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.

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

Support for initial entries #432

Merged
merged 24 commits into from
Aug 11, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
c80d691
Define P4Runtime API support for tables with initial entries
jafingerhut May 28, 2023
3db556a
Add TODO asking whether the format for the contents of entries files
jafingerhut May 28, 2023
a280b70
Fix a couple of things found by linter and compiling protobuf
jafingerhut May 28, 2023
6c09da3
Update autogenerated files
jafingerhut May 29, 2023
3b83f50
Document that TableEntry const field must be false in write requests
jafingerhut May 30, 2023
b8a8724
Merge branch 'master' into support-for-initial-entries-v1
jafingerhut Jul 8, 2023
1632245
Add an appendix describing the contents of entries files generated by…
jafingerhut Jul 8, 2023
005df11
Clarify some wording.
jafingerhut Jul 8, 2023
6625dbc
Fix Madoko lint check
jafingerhut Jul 8, 2023
15a5911
Replace TODO with cross reference to new appendix on entries files
jafingerhut Jul 9, 2023
3a10775
Replace TODO with an optimistic footnote.
jafingerhut Jul 14, 2023
0202c3b
Propose that TableEntry has new field const true for const entries
jafingerhut Jul 14, 2023
4264daf
Update auto-generated files
jafingerhut Jul 14, 2023
25aadc5
Define has_initial_entries to be true for tables with `const entries`
jafingerhut Jul 14, 2023
ddc2af1
Update auto-generated files
jafingerhut Jul 14, 2023
2c8b986
Address several review comments
jafingerhut Jul 15, 2023
e26f9c5
Address some more review comments.
jafingerhut Jul 15, 2023
3f6d536
Update auto-generated files again
jafingerhut Jul 15, 2023
4620bb9
Slight change in definition of has_initial_entries flag
jafingerhut Jul 15, 2023
34eb819
Update auto-generated files
jafingerhut Jul 15, 2023
5c82d35
Add "added in 1.4.0" notes to the two new fields
jafingerhut Jul 19, 2023
56e5a1d
Clarify the description of the content of an entries file
jafingerhut Jul 19, 2023
254ad30
Fix a typo, and add is_const field to list of TableEntry fields
jafingerhut Jul 21, 2023
3f0c390
Address review comment in new appendix
jafingerhut Jul 29, 2023
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
75 changes: 72 additions & 3 deletions docs/v1/P4Runtime-Spec.mdk
Original file line number Diff line number Diff line change
Expand Up @@ -3043,6 +3043,10 @@ limitation. It is recommended that, for the sake of portability, P4Runtime
clients do not try to insert additional entries once the size indicated in
P4Info has been reached.

The `const` field must be `false` in any `INSERT`, `MODIFY`, or
`DELETE` write request of a table entry. If it is true, the server
must reject the operation and return an `INVALID_ARGUMENT` error.

### Match Format { #sec-match-format}

The bytes fields in the `FieldMatch` message follow the format described in
Expand Down Expand Up @@ -3270,10 +3274,13 @@ indirect tables --- tables with an ActionProfile or ActionSelector
`implementation` property --- to a constant `NoAction` action entry, with the
hope that it would simplify the implementation of the P4Runtime service.

### Constant Tables
### Constant Tables { #sec-constant-tables }

Constant tables are defined as tables whose match entries are immutable. They
are identified by the `is_const_table` flag in P4Info.
Constant tables are defined as tables whose match entries are
immutable. They are identified by the table property `const entries`
in the P4~16~ source code, and the `is_const_table` flag in P4Info.
For tables declared with the `entries` property, without `const`
before `entries` see Section [#sec-tables-with-initial-entries].

The only write updates which are allowed for constant tables are `MODIFY`
operations on direct resources, and the default action (assuming the default
Expand All @@ -3298,6 +3305,68 @@ entries. When a priority value is required (⪚ for tables including `RANGE`,
`TERNARY` or `OPTIONAL` matches), it is inferred based on the order in which
entries appear in the table declaration.

TODO: With the recent change to the P4~16~ specification for initial
chrispsommers marked this conversation as resolved.
Show resolved Hide resolved
entries, it might no longer be explicitly clear whether `const
entries` can have a priority value specified for them in the P4~16~
source code. Resolve this question and update the language spec and
this specification to match, if changes are required in either or both
of them.

### Tables with initial entries { #sec-tables-with-initial-entries }

Tables with initial entries are those identified by the table property
`entries` in the P4~16~ source code (with no `const` qualifier before
`entries`), and the `has_initial_entries` flag in P4Info. For tables
declared with `const entries`, see Section [#sec-constant-tables].

For every P4 table, it will fall into one of these three categories:
jafingerhut marked this conversation as resolved.
Show resolved Hide resolved

* Neither `entries` nor `const entries` are declared in the source
jafingerhut marked this conversation as resolved.
Show resolved Hide resolved
code, and thus `is_const_table` and `has_initial_entries` will both
be false. This will be called a "normal table" in this section
below.
* The table has `const entries` declared, and thus a separate
jafingerhut marked this conversation as resolved.
Show resolved Hide resolved
`entries` property is not permitted by the language. Such a table
will have `is_const_table` true and `has_initial_entries` false.
* The table has `entries` declared, and thus a separate `const
jafingerhut marked this conversation as resolved.
Show resolved Hide resolved
entries` property is not permitted by the language. Such a table
will have `is_const_table` false and `has_initial_entries` true.

Since a table may not have both `entries` and `const entries`
declared, no table will have `is_const_table` true and
`has_initial_entries` true.

A table with initial entries is allowed to have a mix of some entries
marked `const`, and other entries not marked `const`.

Entries not marked `const` may be modified or deleted, just as a
client may do for any entry in a normal table.

Entries marked `const` behave like entries in a constant table,
&ie; only `MODIFY` operations on direct resources are allowed.

Unlike a table with `is_const_table = true`, a client may insert
entries into a table with `has_initial_entries = true`, subject to
capacity constraints on the number of entries supported by the target
for the table.

The contents of tables with initial entries can be queried by the
client through a `ReadRequest`. When reading entries from a table
with initial entris, the following fields must be set by the server:
Copy link
Member

Choose a reason for hiding this comment

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

Is this any different than for other tables?
If not, perhaps better not to say anything to keep things consistent?

Copy link
Member

Choose a reason for hiding this comment

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

More generally, I would suggest only explaining what is different, so we don't have two sources of truth that may go out of sync.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. My next commit should have changes that avoid this redundancy.

`table_id`, `match`, `action`, `is_default_action`, and `priority` (if
required). This is in addition to any direct resources that are being
queried. Idle timeouts are not supported for such tables.

If the table requires a priority value for entries, the priorities of
the initial entries are determined according to the P4~16~ language
specification. After the P4 program is initially loaded, the entries
not marked `const` can be modified at run time just as table entries
Copy link
Member

Choose a reason for hiding this comment

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

We already have a sentence explaining what we can and cannot do for const entries above -- would it make sense to move one of the two sentences so we can see the difference between const vs non-const in a single place?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe commit 16 removes this redundancy.

in a normal table can.

TODO: Is there already a place in this specification where the
contents of P4Runtime "entries" files are defined? If not, should the
smolkaj marked this conversation as resolved.
Show resolved Hide resolved
contents of files be defined in this spec?

### Wildcard Reads { #sec-table-wildcard-reads}

When performing a `ReadRequest`, the P4Runtime client can select all entries
Expand Down
23 changes: 21 additions & 2 deletions go/p4/config/v1/p4info.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading