Add proposed extern function add_entry_if()

[jfingerh]

No description provided.

[roop-nvda]

Introduction

There are several use cases for stateful tables in the modern data center networks. These require a table to add an entry when missed, only when certain conditions (such as start of connection) are true and remove an entry when some other conditions indicate that the entry is not needed any more (e.g. end of connection). The PNA spec [1] has defined add_on_miss tables in section 8.1 to allow P4 tables to add entries.

Problem

One commonly cited use case for stateful tables is TCP connection tracking, where we would like an entry to be added when an outgoing TCP SYN is seen, and the entry removed when a TCP FIN is seen. There are several proposals [2][3] that use the PNA defined add_on_miss tables to meet this use case. They use conditional clauses in the match table actions to achieve the selective addition and removal of entries from the add_on_miss table.

However, section 13.1 of the p4_16 language specification [4] mentions portability considerations for switch statements and conditionals in actions – “The body of an action consists of a sequence of statements and declarations. No switch statements are allowed within an action—the grammar permits them, but a semantic check should reject them. Some targets may impose additional restrictions on action bodies—e.g., only allowing straight-line code, with no conditional statements or expressions.”.

In our opinion this restriction is indeed valid for several real world target architectures. As such, we would like to explore further ways to define add_on_miss tables, so that the use cases like the TCP connection tracking could be expressed clearly by the P4 programmer and implemented efficiently in a wide variety of network adapters.

Solution

One of the options to solve this problem is to expand the functionality of the proposed stateful tables externs in PNA.

Add on miss if

For network adapters that do not support conditionals in match actions, an approach that we can consider is adding a check to the extern to add entry, such that a comparison of one or more packet fields is performed inside the extern before inserting the entry in the table. In other words – add entry only if the field matches the value.

extern bool add_entry_if<D, T>(in D data, in D value, in string action_name, in T action_params);

When data == value is true, the return value should be true if the entry was added, false otherwise. When data == value is false, the return value should be false.

For example, to add entry upon receiving TCP SYN, an implementation could call add_entry_if and set the data argument to be the TCP SYN flag and value to be true. Another implementation could use a prior match action table to set a program boolean variable, e.g., do_add. The program can then call the proposed method above as follows: add_entry_if(do_add, true, ..) to add an entry upon miss to an add_on_miss table.

To remove an entry in a timely manner (such as reception of a middle of the connection packet vs. FIN packet in TCP connection tracking) we could set the value of expiry time of the corresponding matched entry to a value only if a field matches the given value.

extern void set_entry_expire_time_if<D>(in D data, in D value, in ExpireTimeProfileId_t expire_time_profile_id);

A more flexible comparison is possible if we use a tuple of fields and values as the first two arguments above.

References

[1] PNA-v0.5.0.pdf (p4.org) version 0.5.0
[2] pna/pna-example-tcp-connection-tracking.p4 at main · p4lang/pna · GitHub
[3] DASH/sirius_conntrack.p4 at main · Azure/DASH · GitHub
[4] P416 Language Specification version 1.2.2.``

[jfingerh]

@roop-nvda Thanks for the comment.

Your comment suggests adding an extern function with signature extern bool add_entry_if<D, T>(in D data, in D value, in string action_name, in T action_params); and the behavior if (data == value) { add_entry(action_name, action_params); }

My PR currently suggests adding an extern function with signature (the text below is copied and pasted from lines added by the current PR as it was after the 1st commit, which I mention in case more commits are added to this PR later):

extern AddEntryErrorStatus_t add_entry_if<T>(
    in bool do_add_entry,
    string action_name,
    in T action_params,
    in ExpireTimeProfileId_t expire_time_profile_id);

with behavior

    (expr) ? add_entry(action_name, action_params, expire_time_profile_id)
            : ADD_ENTRY_NOT_DONE;

As discussed in last week's meeting, it is a bit more general to have a single new bool parameter rather than two type T parameters, where the condition must be data == value, because then an implementation might be able to support either the condition data == value, or other conditions. The complexity of the conditions a target implements might differ from target to target, and while this is not ideal for P4 developers if this varies across target devices, it does give more generality to the supported expressions.

[roop-nvda]

Thanks for your remarks @jfingerh.

The first argument that you propose for add_entry_if is a Boolean. I had read it to mean a p4 elementary type Boolean. From your comment above however, I think you mean the first argument can be an expression that evaluates to a Boolean. I may me missing something, but I am not seeing Boolean cast-able to an expression in the P4 language spec. Can you point me to the relevant section?

Additionally, one of the original motivations of proposing add_entry_if, was to support minimal conditional adds (by means of an extern) in time and compute constrained targets, where a single field could be compared easily, but not much more. Passing an expression, that may in theory be made up of several other expressions, while being indeed more general, is harder for such targets to implement in our opinion. As such we think that the Data filed as first argument is flexible to convey some important conditions while being simple enough to evaluate in a miss action. For example, in the p4 program you have cited, the metadata header can be set by previous tables, and the add on miss table's miss action could capture the condition as -- add_entry_if(meta.other_table_result, true ..).

[jfingerh]

For example, these are all legal call to add_entry_if according to the P4_16 language spec, but some targets could reject some of these for target-specific reasons:

    add_entry_if(true, ...);
    add_entry_if(do_add_on_miss, ...);  // do_add_on_miss is a local variable with type bool
    add_entry_if(hdr.tcp.flags[4:4] == 1, ...);    // #3 Roop believes that BMv2 back end might have difficulty with this.
    add_entry_if((hdr.tcp.flags[5:5] + hdr.tcp.flags[4:4]) == 1, ...);
    add_entry_if(count < 50, ...);

AI Andy: Create a little test program for BMv2 back end that exercises all 5 of these and see whether it handles it, and if so, how. I believe it does. Will use one of the update_checksum or verify_checksum extern functions in BMv2 v1model architecture that already take a bool parameter as first argument, similar to add_entry_if. Add test program and results or link to a comment on this PR.

[jfingerh]

Here is a p4c test program that has been used to test p4c for several years, and runs on the BMv2 simple_switch/simple_switch_grpc software switch:
https://github.com/p4lang/p4c/blob/main/testdata/p4_16_samples/v1model-special-ops-bmv2.p4#L400

The linked call to verify_checksum has a first parameter which is this expression: hdr.ipv4.isValid() && hdr.ipv4.ihl == 5

If you look in the header file v1model.p4, you can see that the definition of verify_checksum's signature is as follows: https://github.com/p4lang/p4c/blob/main/p4include/v1model.p4#L483

extern void verify_checksum<T, O>(in bool condition, in T data, in O checksum, HashAlgorithm algo);

In fact, I believe that having a first parameter with type bool that conditionalizes whether the extern function does something, versus doing nothing, might have originated with verify_checksum and the similar update_checksum extern functions in the v1model architecture.

I would guess that the original motivating reason for having this parameter to verify_checksum and update_checksum was for a similar reason: the desire not to make p4c support recognizing patterns like if (cond1) verify_checksum(...);. However, that implementation slightly precedes my involvement with P4, so I might be wrong on the reason for that implementation choice.

If you compile that test program using a command like p4c --target bmv2 --arch v1model v1model-special-ops-bmv2.p4 it generates a BMv2 JSON file. When I compile it with a recent version of p4c, this section of the BMv2 JSON file defining a BMv2-JSON-checksum-thingy named cksum_0 appears. If you look at the value of the key "if_cond" you can see the BMv2 JSON representation of the condition hdr.ipv4.isValid() && hdr.ipv4.ihl == 5:

    {
      "name" : "cksum_0",
      "id" : 1,
      "source_info" : {
        "filename" : "v1model-special-ops-bmv2.p4",
        "line" : 400,
        "column" : 8,
        "source_fragment" : "verify_checksum(hdr.ipv4.isValid() && hdr.ipv4.ihl == 5, ..."
      },
      "target" : ["ipv4", "hdrChecksum"],
      "type" : "generic",
      "calculation" : "calc_0",
      "verify" : true,
      "update" : false,
      "if_cond" : {
        "type" : "expression",
        "value" : {
          "op" : "and",
          "left" : {
            "type" : "expression",
            "value" : {
              "op" : "d2b",
              "left" : null,
              "right" : {
                "type" : "field",
                "value" : ["ipv4", "$valid$"]
              }
            }
          },
          "right" : {
            "type" : "expression",
            "value" : {
              "op" : "==",
              "left" : {
                "type" : "field",
                "value" : ["ipv4", "ihl"]
              },
              "right" : {
                "type" : "hexstr",
                "value" : "0x05"
              }
            }
          }
        }
      }
    }

[roop-nvda]

I am not sure the verify_checksum example captures the limitations of the table miss actions we are trying to address with add_on_miss_if. My understanding is that verify_checksum is expected to be called as part of parser/deparser blocks, and parser blocks already expect complex conditional expressions.

In the PSA spec, table 5, and in the PNA spec, table 2, specify that the checksum externs can only be called from the parser and deparser blocks.

Some thinking behind this was kindly captured by @jfingerh here p4lang/p4-spec#360:

All (or at least most) checking of header checksums is expected to be performed in the parsers, setting some kind of result for use in the following Ingress or Egress control block, such as the parser error. The Ingress and Egress control blocks are expected to check for parser errors, if desired, and then also perform the desired actions on packets that experienced errors during parsing.

In v1model.p4, the checksum externs are only callable from checksum controls.
https://github.com/p4lang/p4c/blob/6d16a1867e5e3252a6524e2d584bd2f861126109/p4include/v1model.p4#L467
The checksum controls are defined as before and after ingress and egress pipelines of the v1switch.
https://github.com/p4lang/p4c/blob/6d16a1867e5e3252a6524e2d584bd2f861126109/p4include/v1model.p4#L760

I tried inserting a verify_checksum call in an action block, and it does not compile with target bmv2; but the same call compiles fine in the parser block.

[jafingerhut]

@roop-nvda Check out the attached P4 program, which invokes this action from a table:

    action update_ingress_pkt_count () {
        per_inport_protocol_counter.count(
            ((IngressProtoCounterIndex_t) stdmeta.ingress_port << LOG2_NUM_PROTO_IDS) +
            (IngressProtoCounterIndex_t) proto_id);
    }

In the BMv2 JSON file, it appears that this expression is assigned to a temporary variable first in the representation of the action, and then that temporary variable is passed as the parameter to the count method. That introduction of a temporary variable might be simply an implementation detail of the p4c BMv2 back end -- I am not sure.
indexed-counter-update-example.p4.txt

[jfingerh]

@roop-nvda Are the changes in this PR still desired? I will add it to the list of topics for live discussion in PNA meetings.

[roop-nvda]

The PR looks good to me. Thanks @jfingerh fingerh!

[shirshyad]

Is this needed ? If BMV2 does not support if handling , then we should get BMV2 support it instead of adding a new extern.

[roop-nvda]

Section 13.1 of the p4_16 language specification [4] mentions portability considerations for switch statements and conditionals in actions – “The body of an action consists of a sequence of statements and declarations. No switch statements are allowed within an action—the grammar permits them, but a semantic check should reject them. Some targets may impose additional restrictions on action bodies—e.g., only allowing straight-line code, with no conditional statements or expressions.”.

[jfingerh]

@shirshyad I agree that it is best if P4 compilers for programmable NICs more often support if statements within actions, as compared to BMv2's support. I believe the DPDK target DOES support more general if statements within actions already today.

BMv2 could be generalized, of course, but unless someone volunteers (or more likely, is paid) to do that work, it won't happen. In this case, the BMv2 support is already there -- it requires enhancing the BMv2 back end for the open source p4c compiler to add support for more general if statements within P4 actions.

I would definitely prefer if we could simply assume that all P4 targets support if statements within actions, but the unfortunate current reality is they do not. The add_extern_if is ADMITTEDLY a workaround for this limitation.

[jafingerhut]

Following up, I have created this public issue to record some details about BMv2 and DPDK and their support (or not) for if conditions within P4 actions. See there if you are interested: #63

[shirshyad]

should we not show in the example, the usage of the property 'add_on_miss = true' ?

[jfingerh]

There is such an example, here: https://github.com/p4lang/pna/blob/main/examples/pna-example-tcp-connection-tracking.p4#L273

Is there some other example program you are referring to that is missing add_on_miss = true?

[shirshyad]

Should we not say "default_action" table property versus annotation "@deafult_only" ?

[jfingerh]

I tried to be very precise about what I meant there, and I think what I wrote is what I meant, but see if the explanation below clarifies anything:

Some actions can be both hit actions and miss actions for a table, i.e. if they have neither a @tableonly nor a @defaultonly annotation them. According to the text, it is OK for such an action to be the first parameter of an add_entry call. That is true EVEN IF such an action is ALSO currently the default (miss) action of the table. Why should we disallow that? It might be unusual for an action to be a table's default action, and also installed as the action of one or more table entries, but the P4 language has allowed this since it was defined.

[shirshyad]

Then may be description can also include both "@defaultonly" annotation or has table property "default_action".

[jfingerh]

What if a table is defined like this?

table t1 {
    key = { /* key fields not important for this example, so omitted */ }
    actions = {
        a1;
        a2;
    }
    default_action = a2;
}

I would argue that action a2 is a perfectly acceptable argument to a call to add_entry, because it does not have a @defaultonly annotation.

The fact that it is the value of the default_action property does not stop the control plane from adding entries with action a2, so why should add_entry be disallowed from adding entries with action a2?

[shirshyad]

From the description I understood as if default_action adds entry with a1 as action, then it should be accepted. However if default_action adds entry with "a2" as match-action for the new entry , then when "a2" happens to be default_action (@default_action annotation), then it should not be allowed. Let me re-read the description.

[jfingerh]

Would it be clearer if the restriction were documented in the following way instead?

action_name is the name of an action that must meet the following conditions:

• it must be an action in the actions property for the table to which it is adding the entry
• it must be allowed for the action to be an action associated with a table entry, i.e. it must not have the @defaultonly attribute
• it must be an action that does not contain a call to add_entry

[jfingerh]

@shirshyad Do you have any concerns or questions remaining on this PR? Any suggested changes before you would be willing to approve it? If folks were happy with using the DPDK software switch for PNA (which fully supports if in P4 actions), or enhancing the BMv2 one to support if fully in P4 actions, I am happy never defining add_entry_if, by the way. The only reason this PR exists is because several people were concerned that some P4 targets don't support if` statements within actions.

[shirshyad]

The earlier description is clear. I cannot close/resolve comments. Pls consider any comments with "like" from my end are resolved from my end.

[jfingerh]

Sorry, do you mind copying and pasting what you consider to be the "earlier description"?

[jfingerh]

Never mind this question, as I believe you have answered it in this comment: #43 (comment) I will update the PR to reflect that updated description.

[shirshyad]

If we mean the action associated with add_entry is an action that is NOT default action, then we can say "action_name" is the name of non default action of this table.

[shirshyad]

Is this single action parameter ? Can we provide a list of action parameters here ? if not, then we should call action_param ?

[jfingerh]

The type T is intended to be a struct containing as many fields as the corresponding action being added has action parameters, and for those struct fields to match one-to-one with the names and types of the action parameters.

There are likely other ways this could be defined in P4, but I do not know another good way. It is not straightforward to define the signature of a function or method that takes any number of parameters today in P4, e.g. unlike Python, and probably C++.

[shirshyad]

@jfingerh I get that. Hence the suggestion/wording to be "action_param" versus "action_params"

[jfingerh]

What if the action has 3 parameters? You would not object if it was called action_param in that case, even though that one parameter to add_entry represents 3 parameters to the action?

[shirshyad]

Its not clear where do_add_on_miss is evaluated ?

[jfingerh]

I do not understand why it is unclear. do_add_on_miss is the expression that is the first parameter of add_entry_if. In P4, for any call to an action, function, extern function, or extern method, expressions that are the parameters to the call are first evaluated, then once their values (or references, if it is an L-value for an out or inout parameter) are determined, then the call is made.

If that is unclear, please try asking your question in a different way, or feel free to email me at [email protected] and we can schedule time to talk live.

[jafingerhut]

@shirshyad Perhaps you might not have seen the syntax in P4 that this example uses, where when you write an action like action foo(bit<4> a, bit<8> b) you are allowed to call it with syntax like this foo(a = 5, b = 7). It is much more common to write foo(5, 7), but both are allowed and have the same effect. I used the first style of syntax in this example to try to make it clearer what each parameter meant, but may have ended up confusing readers in the process. We can remove the parameter_name = occurrences from this example if you think it would be clearer.

[shirshyad]

I am aware of foo(a=5, b=7) usage. At line 248, I see do_add_on_miss. I was searching for do_add_on_miss as a action local variable, action parameter etc. Hence the question.

[jfingerh]

do_add_on_miss in that example P4 program is a variable defined inside the control, before the action where it is used. P4 actions can refer to any variable that is in scope, including local variables declared earlier in the control that they are defined within (if the action is defined within a control), or any parameters of the control, as well as parameters of the action.

[shirshyad]

Other than modifying description as stated in #43 (comment) the PR LGTM

Thank you answering Qs and addressing comments. PR is approved now assuming some extern description clarification will be done before merge.

[jafingerhut]

@shirshyad I added another commit to this PR that attempts to address your comments for clarifying what the restrictions on the action added by an add_entry() call are allowed to be. I am going ahead and merging this since it has a couple of approvals, but please do consider reading my modified version of those comments for add_entry to see if it addresses your concerns, and file an issue if it does not.