Enhancements to P4Runtime API for P4 table "initial entries" proposal

[jafingerhut]

2023-Aug-08 status of this issue: #432 has several approvals from key people involved, and I hope that it can be merged in during or very soon after the next P4 API work group meeting on 2023-Aug-11.

As of version 1.2.4 of the P4_16 language spec, this PR was merged, enabling P4 programs to specify a list of initial entries in a table, but the runtime is still able to add new entries later during runtime, or to modify or delete any individual entries not marked const: p4lang/p4-spec#1180

On 2023-Jun-07, a partial implementation of this feature has been merged into the open source p4c compiler here: p4lang/p4c@b80b2cb

I say that it is a partial implementation, because for tables with initial entries, it does not mark those tables in P4Info in any way to distinguish them as having initial entries, which is probably desirable to do in some way, and it does not distinguish in the "entries" output file between const vs. non-const entries yet. How to do those things are part of the subject of this issue. When those are decided upon by the P4 API work group, then this p4c issue should be resolved with appropriate changes to the P4Runtime generation code in p4c: p4lang/p4c#4016

This proposed PR on the P4Runtime spec has several TODO items in it that should be discussed and agreed upon in the P4 API work group, in order for it to proceed forward: #432. (alternately, some other PR that addresses this issue in some other way should be created and merged in)

Today, const entries descriptions of table entries are written by p4c front/mid end to a file of entries. None of those contain a priority, nor do they contain a per-entry const qualifier. The 'initial entries' approach will likely need extra properties on each entry for those.

There may be interesting questions of what the various SetForwardingPipelineConfig options should do with initial entries, that never arose with const entries, because the answer for const entries was "they are always there, no matter what". Initial entries are defined to be there when you initially load the P4 program, but what if the same program is reloaded? Should the entries of that table be reinitialized back to what the source code says? Or should they be left as they currently are? Or should there be multiple options that let the controller choose?

[jafingerhut]

Updated status, this PR has now been merged into the language spec, and it seems likely that version 1.2.4 of the spec will be released within a week or so from now: p4lang/p4-spec#1180

[jafingerhut]

Background information on const entries

This comment describes p4c output when a table has const entries today. There is nothing new described in this comment. It is background information on what has been implemented for tables with const entries for about 6 years now.

Here is a test program for p4c that was first added 2017-Mar-15: https://github.com/p4lang/p4c/blob/main/testdata/p4_16_samples/table-entries-ternary-bmv2.p4#L64-L70

When you run p4test or p4c-bm2-ss with these command line options:

cd p4c/testdata/p4_16_samples
mkdir tmp
p4test --arch v1model --p4runtime-files tmp/p4info.txt --p4runtime-entries-files tmp/entries.txt table-entries-ternary-bmv2.p4

Note: The p4c command does NOT support the --p4runtime-entries-files command line option, for reasons that I do not know.

The command above writes not only a P4Info file to tmp/p4info.txt, but it also writes a file named tmp/entries.txt containing a sequence of P4Info WriteRequest messages that, if the table was NOT declared with const entries, would cause a similar table to have the same entries installed into it that the table declared WITH const entries has initially, when the P4 program is loaded, and will never change as long as that P4 program is loaded.

Here are already checked-in versions of these output files for quick reference, used in the automated p4c CI tests to compare p4test's actual output files against these expected contents:

P4Info file: https://github.com/p4lang/p4c/blob/main/testdata/p4_16_samples_outputs/table-entries-ternary-bmv2.p4.p4info.txt
Entries file: https://github.com/p4lang/p4c/blob/main/testdata/p4_16_samples_outputs/table-entries-ternary-bmv2.p4.entries.txt

Note that for this test P4 program, the table with const entries is a table with at least one ternary field. Thus each entry in the "entries" file has a numeric priority value, and they follow the P4Runtime API definition that priorities are positive integers, and larger numerical priority values are higher match priority. The relative match priority of entries declared with const entries is always the same order that the entries appear in the source code today (at least before PR #1180 was merged in, where full support for that will enable the new ability of P4 developers to specify priority values manually in the source file if they wish).

Note also that the P4Info file has the attribute is_const_table equal to the value true: https://github.com/p4lang/p4c/blob/main/testdata/p4_16_samples_outputs/table-entries-ternary-bmv2.p4.p4info.txt#L23 as all tables declared with const entries` are supposed to have.

Reference in P4Info protobuf definition file: https://github.com/p4lang/p4runtime/blob/main/proto/p4/config/v1/p4info.proto#L220-L221

[jafingerhut]

Some comments and questions about how to implement the new support for entries (not const entries, but entries without a const qualifier).

Today tables with const entries have an attribute is_const_table in the P4Info file with value true. I believe we should NOT do this for tables with the new entries property, because there could be many programs out there that process P4Info files and assume that is_const_table means that the table was declared with const entries.

It does seem reasonable to introduce a new way to "mark" that such a table has been declared with entries. Perhaps there could be a new attribute in P4Info for the Table message indicating this, perhaps a boolean-valued attribute with a name like has_non_const_initial_entries that is true for such tables (and only such tables)?

The contents of the entries file written today as described in this comment #426 (comment) seems like a perfectly good format for tables with non-const entries, as long as none of those entries have the const qualifier, which is permitted by the new P4 language feature. See the example in this section of the latest released v1.2.4 of the spec, and notice that there can be a mix of entries marked const and others that are not marked const: https://p4.org/p4-spec/docs/P4-16-v-1.2.4.html#sec-entry-priorities (you must scroll down a bit to find the example in that section).

One way to specify the presence or absence of that const qualifier on a per-entry basis is to add a new boolean attribute const to the P4Runtime message TableEntry defined here: https://github.com/p4lang/p4runtime/blob/main/proto/p4/v1/p4runtime.proto#L152

For the contents of the entries file, that is a very simple generalization to make, and the meaning seems like it should be clear.

Such a new TableEntry attribute is also useful when reading entries of a table, because when reading a table declared with entries, the server can then return separately for each entry read whether it is const or not.

That does raise the question of what the server should do if the client attempts to INSERT/MODIFY/DELETE a TableEntry with const equal to true in the WriteRequest message. One simple approach would be to fail to perform any INSERT, MODIFY, or DELETE operation on all TableEntry messages with const equal to true. This makes some sense semantically for this feature, because it does not make sense to allow a client to add entries that are const that were not already in the table when the P4 program was loaded, nor to MODIFY an entry that was not const so that it becomes const, nor to delete an entry with const.

It should always be an error to attempt to modify or delete an entry that has const equal to true.

It should be supported to insert new entries into a table with the entries property, up to the size limits declared for the table and the details of the target (e.g. P4 targets are allowed to fail to add entries even if they have fewer entries than their size declaration, due to things like hash collisions, and other implementation-specific reasons, and this new feature does not change that).

Except for priority values (discussed in a later comment below), I think these two changes in P4Runtime proto files would enable implementing this new feature.

[jafingerhut]

There are two ways to specify numeric priority values described by this new feature. The default is largest_priority_wins = true. In this situation, the numeric priority values in the source code should be exactly the priority values of the entries initially in the table when the P4 program is loaded, as visible to a P4Runtime API client.

The other is if the P4 developer explicitly chooses to write largest_priority_wins = false as a property of the P4 table. In this case, smaller numeric values in the source code should be higher match priority. When writing an entries file for P4Runtime, however, the priority values should be larger-numeric-priority is higher match priority.

One way to implement this is: for an entry in the P4 source code determine to have priority value P, write to the entries file the priority value (M-P), where M is some maximum supported priority value (perhaps M=2^32 or M=2^32-1 ?)

[chrispsommers]

One way to specify the presence or absence of that const qualifier on a per-entry basis is to add a new boolean attribute const to the P4Runtime message TableEntry defined here: https://github.com/p4lang/p4runtime/blob/main/proto/p4/v1/p4runtime.proto#L152

+1 for this

It does seem reasonable to introduce a new way to "mark" that such a table has been declared with entries. Perhaps there could be a new attribute in P4Info for the Table message indicating this, perhaps a boolean-valued attribute with a name like has_non_const_initial_entries that is true for such tables (and only such tables)?

@jafingerhut This is a reasonable proposal, although this is a rather longish name.

+1 for all the other verbiage describing usage, error-handling, etc.

[jafingerhut]

Yeah, a shorter name would be good. has_initial_entries, perhaps?

[chrispsommers]

I was thinking the same, and the existing is_const_table could signify all constant entries like today. This would result in four valid combinations, see table below.

is_const_table	has_initial_entries	Meaning
absent or false	absent or false	Contains no initial entries, constant or otherwise
absent or false	true	Contains an arbitrary mix of constant and non-constant initial entries, as signified by the new proposed per-entry const attribute
true	absent or false	Contains only constant initial entries (Note - this is the current combination for const entries)
true	true	Contains only constant initial entries

The third row is interesting and corresponds to the current standard: a true value of is_const_table by itself means there are some constant initial entries, even if has_initial_entries is false! This seems questionable. If you agree, perhaps we should use your original proposal of has_non_const_initial_entries despite its length, which would be mutually-exclusive. See below:

is_const_table	has_non_const_initial_entries	Meaning
absent or false	absent or false	Contains no initial entries, constant or otherwise
absent or false	true	Contains an arbitrary mix of constant and non-constant initial entries, as signified by the new proposed per-entry const attribute
true	absent or false	Contains only constant initial entries
true	true	Invalid combination - the two attributes are mutually-exclusive

Thoughts?

[jafingerhut]

My intended meaning of this new attribute is that your second table would list all of the valid combinations. That has the best backwards compatibility, in that is_const_table means exactly what it does today in all cases.

I think a table like that in the P4Runtime spec would make it clear, regardless of the name of the new table property.

[jafingerhut]

Question raised by Sayan: What if a table declares const entries, and also has a direct counter or direct meter associated with the table? Is that supported by P4Runtime API today? If yes, is the P4runtime client able to clear the direct counter with a const entry? Is it allowed to configure the meter parameters of a const entry with a direct meter?

In addition to the questions above:

• Does the P4Runtime spec already address these details?
• What operations does the existing open source P4Runtime implementation support for such a table?

[jafingerhut]

Whenever this issue is resolved, this issue for p4c should be addressed: p4lang/p4c#4016