Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Clarify add-on-miss #64

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Clarify add-on-miss #64

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c0773ed
clarify add-on-miss
hesingh Oct 26, 2022
da27d48
address review comments
hesingh Oct 26, 2022
0428fda
address review comment
hesingh Oct 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 32 additions & 6 deletions PNA.mdk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -930,7 +930,10 @@ property is `true` for a table, the P4 developer is allowed to define
a default action for the table that calls the `add_entry` extern
function. `add_entry` adds a new entry to the table whose default
action calls the `add_entry` function. The new entry will have the
same key that was just looked up.
same key that was just looked up. The new entry value, as in a (key,value)
pair, is obtained from sources such as packet headers that caused the
table lookup to miss. Other sources are metadata and results from
prior processing of the P4 program before `add_entry` is called.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree all of this new text is abolutely true, but it seems redundant to me with the definition in the P4 language spec of what the expressions in a table's key expressions are allowed to be. Is it worth listing these possibilities out?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A user may be used to configuring table entries only from the control-plane. I was confused. Such a user will scratch his head and ask where does add-on-miss get its value data from to be able to write a key-value to memory.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the place where the new entry gets its parameter values is from the parameters to add_entry. Why not just say that? Why say "they might be packet header fields, they might be metadata calculated earlier, etc." The rules of P4 say how to evaluate those expressions, without needing to list them all again in the definition of add_entry.


The control plane API is still allowed to add, modify, and delete
entries of such a table, but any entries added via the `add_entry`
Expand All @@ -940,10 +943,14 @@ sustain `add_entry` calls at a large fraction of their line rate, but
it need not be at the same packet rate supported for processing
packets that do not call `add_entry`. The new table entry will be
matchable when the next packet is processed that applies this table.
Use `add_on_miss` with caution because the data plane must have
enough information to verify that adding an entry on miss is correct.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I have said this before, but why a special warning for add-on-miss? Why not for every assignment statement you write? Every action? Every if statement? For all of them, and everything else in a P4 program, the data plane must have enough information to verify that doing that thing is correct. add-on-miss isn't special in that regard.

TCP connection tracking is one application to use `add_on_miss`.

~Begin P4Example
extern bool add_entry<T>(string action_name,
in T action_params);
in T action_params,
in ExpireTimeProfileId_t expire_time_profile_id);
~End P4Example

It is expected that many PNA implementations will restrict
Expand All @@ -960,14 +967,33 @@ It is expected that many PNA implementations will restrict
names must match the hit action parameter names, and their types
must be the same as the corresponding hit action parameters.

The new entry will have the same key field values that were
searched for in the table when the miss occurred, which caused the
table's default action to be executed. The action will be the one
named by the string that is passed as the parameter `action_name`.
The new entry will have the same key that was searched for in the table
when the miss occurred, which caused the table's default action to be
executed. The action will be the one named by the string that is passed
as the parameter `action_name`.

If the attempt to add a table entry succeeds, the return value is
`true`, otherwise `false`.

An example program is included below using `add_entry`.

~Begin P4Example
action hit(bit<8> a0, bit<16> a1) {}
action miss(bit<8> a0, bit<16> a1) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This program won't compile, I'm nearly certain, because no action parameters are provided to the action miss in the definition of the default_action below for table foo.

I would recommend changing the definition of miss so that it has 0 parameters.

You could also consider adding a link to a more complete example program such as the one in the examples directory like https://github.com/p4lang/pna/blob/main/examples/pna-example-tcp-connection-tracking.p4

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. I read the P4-16 working doc and see text in default_action must supply arguments for the control-plane bound parameters because the default_action is synthesized at compilation time

meta.a0 = a0;
meta.a1 = a1;
add_entry("hit", {hdr.a0, hdr.a1}, expire_time_prof_id);
}

table foo {
key = { hdr.ipv6.src_addr : exact;
hdr.ipv6.dst_addr : exact;
}
add_on_miss = true;
actions = { hit; miss;};
default_action = miss;
}
~End P4Example

## Table entry timeout notification {#sec-idle-timeout}

Expand Down