From 21bef0a23e2ba9aa417ffc0d21cfe60f840ce352 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Tue, 17 May 2022 12:08:42 -0400 Subject: [PATCH 1/2] Add proposed extern function add_entry_if() --- .../pna-example-tcp-connection-tracking.p4 | 27 +++++- pna.p4 | 82 ++++++++++++++++--- 2 files changed, 94 insertions(+), 15 deletions(-) diff --git a/examples/pna-example-tcp-connection-tracking.p4 b/examples/pna-example-tcp-connection-tracking.p4 index 7b8ea33..8375fe3 100644 --- a/examples/pna-example-tcp-connection-tracking.p4 +++ b/examples/pna-example-tcp-connection-tracking.p4 @@ -150,7 +150,7 @@ control MainControlImpl( ExpireTimeProfileId_t new_expire_time_profile_id; // Outputs from actions of ct_tcp_table - bool add_succeeded; + AddEntryErrorStatus_t add_status; action tcp_syn_packet () { do_add_on_miss = true; @@ -159,11 +159,13 @@ control MainControlImpl( new_expire_time_profile_id = EXPIRE_TIME_PROFILE_TCP_NEW; } action tcp_fin_or_rst_packet () { + do_add_on_miss = false; update_aging_info = true; update_expire_time = true; new_expire_time_profile_id = EXPIRE_TIME_PROFILE_TCP_NOW; } action tcp_other_packets () { + do_add_on_miss = false; update_aging_info = true; update_expire_time = true; new_expire_time_profile_id = EXPIRE_TIME_PROFILE_TCP_ESTABLISHED; @@ -229,11 +231,31 @@ control MainControlImpl( } action ct_tcp_table_miss() { +#ifdef AVOID_IF_INSIDE_ACTION + // Note that this code is NOT completely equivalent to the + // code in the #else branch below, because it does not call + // drop_packet() if do_add_on_miss is false. One way to make + // the two versions of the program equivalent, not shown here, + // would be to assign a value to a metadata variable here such + // as 'ct_tcp_table_got_miss = true;', and then write + // additional code after ct_tcp_table.apply() equivalent in + // behavior to this: + // + // if (ct_tcp_table_got_miss && !do_add_on_miss) { + // drop_packet(); + // } + add_status = + add_entry_if(do_add_entry = do_add_on_miss, + action_name = "ct_tcp_table_hit", // name of action + action_params = (ct_tcp_table_hit_params_t) + {}, + expire_time_profile_id = new_expire_time_profile_id); +#else if (do_add_on_miss) { // This example does not need to use allocate_flow_id(), // because no later part of the P4 program uses its return // value for anything. - add_succeeded = + add_status = add_entry(action_name = "ct_tcp_table_hit", // name of action action_params = (ct_tcp_table_hit_params_t) {}, @@ -241,6 +263,7 @@ control MainControlImpl( } else { drop_packet(); } +#endif // AVOID_IF_INSIDE_ACTION // a target might also support additional statements here, e.g. // mirror the packet // update a counter diff --git a/pna.p4 b/pna.p4 index 98c50fe..6b6041e 100644 --- a/pna.p4 +++ b/pna.p4 @@ -665,19 +665,75 @@ extern void mirror_packet(MirrorSlotId_t mirror_slot_id, // probably be a parameter that specifies the new entry's initial // expire_time_profile_id. -extern bool add_entry(string action_name, - in T action_params, - in ExpireTimeProfileId_t expire_time_profile_id); - -// If we support the following variant of add_entry, with no -// expire_time_profile_id parameter, then we need to define its -// behavior in terms of what the added entry's value of -// expire_time_profile_id is. One option would be to define the -// behavior to be the same as the 3-parameter version of add_entry -// above, with expire_time_profile_id equal to 0. - -//extern bool add_entry(string action_name, -// in T action_params); +// The bit width of this type is allowed to be different for different +// target devices. It must be at least a 1-bit wide type. + +typedef bit<1> AddEntryErrorStatus_t; + +const AddEntryErrorStatus_t ADD_ENTRY_SUCCESS = 0; +const AddEntryErrorStatus_t ADD_ENTRY_NOT_DONE = 1; + +// Targets may define target-specific non-0 constants of type +// AddEntryErrorStatus_t if they wish. + +// add_entry() must be called within the body of a P4 action that is +// the default_action of a table with the table property `add_on_miss` +// equal to true. +// +// The key of the entry added will always be the same as the key that +// was just looked up in the table, and experienced a miss. +// +// `action_name` is the name of an action of this table. It must be +// possible for this action to be the action of an entry added to the +// table, i.e. it is an error if the action has the annotation +// `@defaultonly`. +// +// Type T must be a struct type whose field names have the same name +// as the parameters of the action being added, in the same order, and +// have the same type as the corresponding action parameters. +// +// `action_params` will become the action parameters of the new entry +// to be added. +// +// `expire_time_profile_id` is the initial expire time profile id of +// the entry added. +// +// The return value will be ADD_ENTRY_SUCCESS if the entry was +// successfully added, otherwise it will be some other value not equal +// to ADD_ENTRY_SUCCESS. Targets are allowed to define only one +// failure return value, or several if they wish to provide more +// detail on the reason for the failure to add the entry. + +extern AddEntryErrorStatus_t add_entry( + string action_name, + in T action_params, + in ExpireTimeProfileId_t expire_time_profile_id); + +// The following call to add_entry_if(): +// +// add_entry_if(expr, action_name, action_params, expire_time_profile_id); +// +// has exactly the same behavior as the following expression: +// +// (expr) ? add_entry(action_name, action_params, expire_time_profile_id) +// : ADD_ENTRY_NOT_DONE; +// +// and it has the same restrictions on where it can appear in a P4 +// program as that equivalent code. +// +// Rationale: At the time PNA was being designed in 2022, there were +// P4 targets, including the BMv2 software switch in the repository +// https://github.com/p4lang/behavioral-model, that did not fully +// support `if` statements within P4 actions. add_entry_if() enables +// writing P4 code without `if` statements within P4 actions that +// would otherwise require an `if` statement to express the desired +// behavior, if only the extern function add_entry() were provided. + +extern AddEntryErrorStatus_t add_entry_if( + in bool do_add_entry, + string action_name, + in T action_params, + in ExpireTimeProfileId_t expire_time_profile_id); extern FlowId_t allocate_flow_id(); From 68d09957aa5ab4bd15f458ec57d0eaaf96bd19e6 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Thu, 27 Oct 2022 22:49:23 -0400 Subject: [PATCH 2/2] Address comments on restrictions to add_entry() calls --- pna.p4 | 42 ++++++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/pna.p4 b/pna.p4 index 6b6041e..38a73ff 100644 --- a/pna.p4 +++ b/pna.p4 @@ -676,17 +676,24 @@ const AddEntryErrorStatus_t ADD_ENTRY_NOT_DONE = 1; // Targets may define target-specific non-0 constants of type // AddEntryErrorStatus_t if they wish. -// add_entry() must be called within the body of a P4 action that is -// the default_action of a table with the table property `add_on_miss` -// equal to true. +// The add_entry() extern function causes an entry, i.e. a key and its +// corresponding action and action parameter values, to be added to a +// table from the data plane, i.e. without the control plane having to +// take any action at all to cause the table entry to be added. // -// The key of the entry added will always be the same as the key that -// was just looked up in the table, and experienced a miss. +// The key of the new entry added will always be the same as the key +// that was just looked up in the table, and experienced a miss. // -// `action_name` is the name of an action of this table. It must be -// possible for this action to be the action of an entry added to the -// table, i.e. it is an error if the action has the annotation -// `@defaultonly`. +// `action_name` is the name of an action that must satisfy these +// restrictions: +// + It must be an action that is in the list specified as the +// `actions` property of the table. +// + It must be possible for this action to be the action of an entry +// added to the table, e.g. it is an error if the action has the +// annotation `@defaultonly`. +// + The action to be added must not itself contain a call to +// add_entry(), or anything else that is not supported in a table's +// "hit action". // // Type T must be a struct type whose field names have the same name // as the parameters of the action being added, in the same order, and @@ -703,6 +710,19 @@ const AddEntryErrorStatus_t ADD_ENTRY_NOT_DONE = 1; // to ADD_ENTRY_SUCCESS. Targets are allowed to define only one // failure return value, or several if they wish to provide more // detail on the reason for the failure to add the entry. +// +// It is NOT defined by PNA, and need not be supported by PNA +// implementations, to call add_entry() within an action that is added +// as an entry of a table, i.e. as a "hit action". It is only defined +// if called within an action that is the default_action, i.e. a "miss +// action" of a table. +// +// For tables with `add_on_miss = true`, some PNA implementations +// might only support `default_action` with the `const` qualifier. +// However, if a PNA implementation can support run-time modifiable +// default actions for such a table, some of which call add_entry() +// and some of which do not, the behavior of such an implementation is +// defined by PNA, and this may be a useful feature. extern AddEntryErrorStatus_t add_entry( string action_name, @@ -727,7 +747,9 @@ extern AddEntryErrorStatus_t add_entry( // support `if` statements within P4 actions. add_entry_if() enables // writing P4 code without `if` statements within P4 actions that // would otherwise require an `if` statement to express the desired -// behavior, if only the extern function add_entry() were provided. +// behavior. Admittedly, this is a work-around for targets with +// limited support for `if` statements within P4 actions. See +// https://github.com/p4lang/pna/issues/63 for more details. extern AddEntryErrorStatus_t add_entry_if( in bool do_add_entry,