Add metadata to multicast group entry #446
Conversation
|
Looks like you have some trailing whitespace in the .mdk and you need to regenerate the protobuf code, see instructions at bottom of this section: https://github.com/p4lang/p4runtime#detailed-processes. Thanks. |
There was a problem hiding this comment.
LGTM from my side.
Let's also give others a chance to weigh in, e.g. @saynb @antoninbas @jafingerhut.
|
@verios-google @smolkaj Do you use Stratum P4Runtime API implementation code in your work? If yes, am I correct in guessing that implementing these changes would require some (probably small) changes in Stratum to accomplish this? Are you willing to do that? |
|
Same question for whether these changes might require implementation code changes in this repo: https://github.com/p4lang/PI |
No, we do not use Stratum. So we'd kindly ask that people interested in that project extend it if desired. Any idea who the main people working on Stratum these days might be? By the way, our P4Runtime server is part of SONIC and you can find it here: https://github.com/sonic-net/sonic-pins. We will be extending it to support this. We do use BMv2, so extending https://github.com/p4lang/PI seems like a reasonable ask. @antoninbas any ideas how much work this may be? |
|
@saynb Are you the right person to ask about Stratum developer contacts? @smolkaj I think it is perfectly reasonable to ask Antonin about development effort required to make such a change in p4lang/PI, but I would recommend that serious thought be given to the idea that someone besides Antonin actually implements it. Otherwise, the base of developers who know how to make such changes will not be growing. |
This is very simple to implement. Should be about the same scope as your recent PR (p4lang/PI#598). The |
Sure, I agree. I imlemented p4lang/PI#598 just a few days ago. Thanks @antoninbas for the quick effort estimation & implementation idea. |
I am not the primary person but I can help connect the right people in addition to me. ONF stratum : Probably @pudelkoM ? |
|
While most of us don't work at ONF anymore, I can certainly take a look at any potential PR and merge it. |
|
It appears that automated CI checks for this PR never completed. I do not know why. Likely one of those checks that would fail if it had completed is one for checking in the auto-generated output files from the modified .proto files in your PR. Instructions for regenerating those files can be found near the end of this section of the README: https://github.com/p4lang/p4runtime#detailed-processes Committing such updated files should trigger CI checks to re-run (or any other commit to this PR). |
See my comment #446 (comment) |
* Update Go dependencies (#438) * Fixes #439 (#440) Change CI workflow to skip publishing if PR spawned by dependabot * Adding dev branches to workflows (#443) * Support for initial entries (#432) * Define P4Runtime API support for tables with initial entries * Add TODO asking whether the format for the contents of entries files should be specified in the P4Runtime spec. * Fix a couple of things found by linter and compiling protobuf * Update autogenerated files * Document that TableEntry const field must be false in write requests * Add an appendix describing the contents of entries files generated by p4c * Clarify some wording. * Fix Madoko lint check * Replace TODO with cross reference to new appendix on entries files and clean up Madoko formatting in that appendix. * Replace TODO with an optimistic footnote. * Propose that TableEntry has new field const true for const entries and also for const default_action * Update auto-generated files * Define has_initial_entries to be true for tables with `const entries` Also fix a couple of spelling typos. * Update auto-generated files * Address several review comments * Address some more review comments. * Update auto-generated files again * Slight change in definition of has_initial_entries flag Propose that it is true if and only if the table property `entries` is present, _and_ the list of entries is not empty. * Update auto-generated files * Add "added in 1.4.0" notes to the two new fields * Clarify the description of the content of an entries file * Fix a typo, and add is_const field to list of TableEntry fields * Address review comment in new appendix * Fix #434: Remove obsolete TODO section in README (#447) * Fix #434: Remove obsolete TODO section in README Update the link to the auto-generated versions of the P4Runtime specification on the P4.org web site. Update the section "P4 Language Version Applicability" to version 1.2.4 of the P4_16 language specification, but list 3 known exceptions of features that have not been explicitly addressed yet. * Add P4_16 v1.2.4 language spec features that may need addressing in a future version of the P4Runtime API specification. * Update discussion of entry priorities in constant tables (#457) * Update discussion of entry priorities in constant tables * Correct description of entry priority for constant tables * Bump golang.org/x/net from 0.9.0 to 0.17.0 (#461) Bumps [golang.org/x/net](https://github.com/golang/net) from 0.9.0 to 0.17.0. - [Commits](golang/net@v0.9.0...v0.17.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Remove 4 P4 language spec compatibility issues from the list (#459) * Remove 4 P4 language spec compatibility issues from the list During 2023-Sep-08 P4.org API work group meeting, it was agreed that there are no changes required to the P4Runtime API specification to be compatible with these updates in the language spec. * Add clarifying behavior of table with no `key` property back in since there are potentially open issues around p4c implementation and how it generates size field of tables in P4Info files that should be considered before considering that issue resolved. * Add metadata to multicast group entry (#446) Same role as the metadata field for table entry * Add proto_build_test rule that tests building the protos defined in the workspace. (#460) * Update license kind to Apache2.0 instead of generic notice (#464) * Bump google.golang.org/grpc from 1.56.1 to 1.56.3 (#465) Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.56.1 to 1.56.3. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](grpc/grpc-go@v1.56.1...v1.56.3) --- updated-dependencies: - dependency-name: google.golang.org/grpc dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Antonin Bas <[email protected]> Co-authored-by: Chris Sommers <[email protected]> Co-authored-by: Andy Fingerhut <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: verios-google <[email protected]> Co-authored-by: anksaiki <[email protected]> Co-authored-by: anksaiki <[email protected]>
* Update Go dependencies (#438) * Fixes #439 (#440) Change CI workflow to skip publishing if PR spawned by dependabot * Adding dev branches to workflows (#443) * Support for initial entries (#432) * Define P4Runtime API support for tables with initial entries * Add TODO asking whether the format for the contents of entries files should be specified in the P4Runtime spec. * Fix a couple of things found by linter and compiling protobuf * Update autogenerated files * Document that TableEntry const field must be false in write requests * Add an appendix describing the contents of entries files generated by p4c * Clarify some wording. * Fix Madoko lint check * Replace TODO with cross reference to new appendix on entries files and clean up Madoko formatting in that appendix. * Replace TODO with an optimistic footnote. * Propose that TableEntry has new field const true for const entries and also for const default_action * Update auto-generated files * Define has_initial_entries to be true for tables with `const entries` Also fix a couple of spelling typos. * Update auto-generated files * Address several review comments * Address some more review comments. * Update auto-generated files again * Slight change in definition of has_initial_entries flag Propose that it is true if and only if the table property `entries` is present, _and_ the list of entries is not empty. * Update auto-generated files * Add "added in 1.4.0" notes to the two new fields * Clarify the description of the content of an entries file * Fix a typo, and add is_const field to list of TableEntry fields * Address review comment in new appendix * Fix #434: Remove obsolete TODO section in README (#447) * Fix #434: Remove obsolete TODO section in README Update the link to the auto-generated versions of the P4Runtime specification on the P4.org web site. Update the section "P4 Language Version Applicability" to version 1.2.4 of the P4_16 language specification, but list 3 known exceptions of features that have not been explicitly addressed yet. * Add P4_16 v1.2.4 language spec features that may need addressing in a future version of the P4Runtime API specification. * Update discussion of entry priorities in constant tables (#457) * Update discussion of entry priorities in constant tables * Correct description of entry priority for constant tables * Bump golang.org/x/net from 0.9.0 to 0.17.0 (#461) Bumps [golang.org/x/net](https://github.com/golang/net) from 0.9.0 to 0.17.0. - [Commits](golang/net@v0.9.0...v0.17.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Remove 4 P4 language spec compatibility issues from the list (#459) * Remove 4 P4 language spec compatibility issues from the list During 2023-Sep-08 P4.org API work group meeting, it was agreed that there are no changes required to the P4Runtime API specification to be compatible with these updates in the language spec. * Add clarifying behavior of table with no `key` property back in since there are potentially open issues around p4c implementation and how it generates size field of tables in P4Info files that should be considered before considering that issue resolved. * Add metadata to multicast group entry (#446) Same role as the metadata field for table entry * Add proto_build_test rule that tests building the protos defined in the workspace. (#460) * Update license kind to Apache2.0 instead of generic notice (#464) * Bump google.golang.org/grpc from 1.56.1 to 1.56.3 (#465) Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.56.1 to 1.56.3. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](grpc/grpc-go@v1.56.1...v1.56.3) --- updated-dependencies: - dependency-name: google.golang.org/grpc dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Pin Bazel version to 6.4.0 to fix regression (#471) --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Antonin Bas <[email protected]> Co-authored-by: Chris Sommers <[email protected]> Co-authored-by: Andy Fingerhut <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: verios-google <[email protected]> Co-authored-by: anksaiki <[email protected]> Co-authored-by: anksaiki <[email protected]> Co-authored-by: Steffen Smolka <[email protected]>
We have a use case for this. The change is small, backwards compatible, and consistent with table entries.