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

Making sure that the latest P4Runtime spec is compatible with P4_16 v1.2.3 #424

Closed
jafingerhut opened this issue May 12, 2023 · 7 comments
Closed
Labels
p4-language-compatibility An issue related to compatibility between P4_16 language spec and P4Runtime API spec

Comments

@jafingerhut
Copy link
Contributor

This issue is similar to #389, except addresses any changes made to the P4_16 language spec from version 1.2.2 to version 1.2.3, which can be found in bullet list form here: https://p4.org/p4-spec/docs/P4-16-v-1.2.3.html#sec-summary-of-changes-made-in-version-123-released-july-11-2022

I have copied and pasted those brief descriptions below.

Summary of changes made in version 1.2.3, released July 11, 2022.
Extended minSizeInBits and minSizeInBytes to apply to more expressions (Section 9).
Added support for maxSizeInBits and maxSizeInBytes (Section 9).
Added support for empty lists of const entries in tables (Section 14.2.1.4).
Added support for switch statements in actions (Section 14.1).
Added support for direct invocation of controls and parsers (Section 15).
Added parser value_set to list of control-plane visible names (Section 18.3).
Added match_kind as a base type (Section 7.1.3).
Removed structure initializers as they are subsumed by structure-valued expressions (Section 8.13).
Specified operations on values typed as type variables (Section 8.22).
Clarified semantics of compile-time known values (Section 18.1).
Clarified semantics of directionless parameters (Section 6.7).
Clarified semantics of infinite precision integers (Section 7.1.6.5).
Clarified semantics of bit slices, shifts, and concatenation (Section 8.5).
Clarified semantics of optional parameters (Section 6.7.2).
Clarified restrictions on extern method and function invocation (Section F).
Clarified semantics of implicit casts (Section 8.10.2).

I would recommend that a later comment on this issue take all of those bullet items, and make a separate comment on each one as to whether we believe it will have any effect on the P4Runtime API. I suspect that most of them will be "no known effect on P4Runtime API specification", but it would be good to explicitly say that in a later comment, rather than being silent about a language spec change, for possible future reference, and review by others.

@jafingerhut
Copy link
Contributor Author

Here are some item-by-item comments on the list of updates from version 1.2.2 to 1.2.3:

  • Extended minSizeInBits and minSizeInBytes to apply to more expressions (Section 9).

[jafingerhut] minSizeInBits and minSizeInBytes are P4_16 data plane functions that have no control plane API, so there should be nothing that they affect in P4Runtime API.

  • Added support for maxSizeInBits and maxSizeInBytes (Section 9).

[jafingerhut] maxSizeInBits and maxSizeInBytes are P4_16 data plane functions that have no control plane API, so there should be nothing that they affect in P4Runtime API.

  • Added support for empty lists of const entries in tables (Section 14.2.1.4).

[jafingerhut] There is a test case in p4c/testdata/p4_16_samples named issue2905-bmv2.p4 that has a table named t_exact with const entries, and an empty list of entries. The expected output files mentioned here are in p4c/testdata/p4_16_samples_outputs. issue2905-bmv2.p4.p4info.txt contains a P4Info file generated by open source p4c with a definition for t_exact that has is_const_table: true, as all tables with const entries do in their P4Info file. The file issue2905-bmv2.p4.entries.txt is empty, because there are 0 total const entries in all tables of that P4 program. That all looks correct to me, and is tested on every commit to p4c.

  • Added support for switch statements in actions (Section 14.1).

[jafingerhut] P4Runtime API currently defines action names and the directionless action parameters, but as far as I know it says nothing about the bodies of actions, and this change in the spec simply allows more kinds of P4_16 statements to be included in the definition of action bodies, so I cannot imagine any affect on the P4Runtime API spec.

  • Added support for direct invocation of controls and parsers (Section 15).

[jafingerhut] Direct invocation of controls and parsers is simply another way to invoke controls and parsers in a P4_16 source program. I am not aware of any affect on P4Runtime API for any invocations of controls and parsers in the P4_16 source code, regardless of whether it is a direct invocation, or any other kind of invocation.

  • Added parser value_set to list of control-plane visible names (Section 18.3).

[jafingerhut] This appears to be correcting an ommission in the P4_16 language spec. There are already existing test cases in the p4c test suite that test generation of P4Info files for P4 programs containing parser value sets, e.g. issue1955.p4 and many others, and their expected P4Info output files contain definitions of those value_sets, so this was not an omission in the P4Info generation in p4c, but only an omission in the spec.

  • Added match_kind as a base type (Section 7.1.3).

[jafingerhut] This was a clarification in the P4_16 type system. No known affect on the P4Runtime API.

  • Removed structure initializers as they are subsumed by structure-valued expressions (Section 8.13).

[jafingerhut] A clarification in the language spec, removing some redundant definitions. No known affect on the P4Runtime API.

  • Specified operations on values typed as type variables (Section 8.22).

[jafingerhut] No known affect on the P4Runtime API.

  • Clarified semantics of compile-time known values (Section 18.1).

[jafingerhut] No known affect on the P4Runtime API.

  • Clarified semantics of directionless parameters (Section 6.7).

[jafingerhut] No known affect on the P4Runtime API.

  • Clarified semantics of infinite precision integers (Section 7.1.6.5).

[jafingerhut] No known affect on the P4Runtime API.

  • Clarified semantics of bit slices, shifts, and concatenation (Section 8.5).

[jafingerhut] No known affect on the P4Runtime API.

  • Clarified semantics of optional parameters (Section 6.7.2).

[jafingerhut] No known affect on the P4Runtime API.

  • Clarified restrictions on extern method and function invocation (Section F).

[jafingerhut] No known affect on the P4Runtime API.

  • Clarified semantics of implicit casts (Section 8.10.2).

[jafingerhut] No known affect on the P4Runtime API.

Summary: If all of the above evaluations are correct, then none of the changes from version 1.2.2 to 1.2.3 of the language spec have any impact on the P4Runtime API.

@chrispsommers
Copy link
Collaborator

chrispsommers commented May 13, 2023

Thanks @jafingerhut for the timely and thorough review! We'll let this sit for a while to give others a chance to weigh in.

@smolkaj
Copy link
Member

smolkaj commented May 22, 2023

Thanks, @jafingerhut!

@chrispsommers
Copy link
Collaborator

Seems like there have been no objections to Andy's thorough analysis, so I think we should do a PR to the spec declaring it compliant with P4_16 v1.2.3, approve it and close this forthwith.

@jafingerhut
Copy link
Contributor Author

@chrispsommers The main issue with that idea is that depending upon the results of these issues, P4Runtime API spec might not yet be compliant with P4_16 v1.2.2:

The good news is that once those are resolved, then we can jump all the way to being compliant with v1.2.2 to v1.2.3.

@chrispsommers
Copy link
Collaborator

Andy, thanks for reminding me!

@jafingerhut jafingerhut added the p4-language-compatibility An issue related to compatibility between P4_16 language spec and P4Runtime API spec label Aug 15, 2023
@jafingerhut
Copy link
Contributor Author

Closing this issue, as I believe there are no changes required in the P4Runtime API specification due to any of the changes made from version 1.2.2 to version 1.2.3 of the P4_16 language spec, as detailed above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p4-language-compatibility An issue related to compatibility between P4_16 language spec and P4Runtime API spec
Projects
None yet
Development

No branches or pull requests

3 participants