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

Updating generic-table-dev branch with p4lang:p4runtime:main #468

Merged
merged 13 commits into from
Jan 26, 2024

Conversation

saynb
Copy link
Contributor

@saynb saynb commented Dec 7, 2023

No description provided.

antoninbas and others added 13 commits July 6, 2023 11:51
Change CI  workflow to skip publishing if PR spawned by dependabot
* 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 p4lang#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

* Correct description of entry priority for constant tables
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
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.
Same role as the metadata field for table entry
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>
@chrispsommers
Copy link
Collaborator

Hi @saynb I am a bit puzzled why this PR isn't just "sync main to generic-table-dev." It would make the lineage clearer.

@saynb
Copy link
Contributor Author

saynb commented Dec 7, 2023

There were some merge conflicts on that #469. So decided to do it this way instead

@chrispsommers
Copy link
Collaborator

Understood.

Copy link
Collaborator

@chrispsommers chrispsommers left a comment

Choose a reason for hiding this comment

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

So assuming this makes generic-tables-dev the same as main (hard to tell from the commit chain), LGTM.

@chrispsommers
Copy link
Collaborator

Merging. @saynb please confirm it resolves to be same as main, otherwise we can fall back to a rebase and force-merge.

@chrispsommers chrispsommers merged commit a6b4cf6 into p4lang:generic-table-dev Jan 26, 2024
5 checks passed
@chrispsommers
Copy link
Collaborator

@saynb
Copy link
Contributor Author

saynb commented Jan 26, 2024

@chrispsommers I can confirm that the generic-table-dev branch has no diff with main even if changes show up in a PR if you try and create one from main->generic-table-dev.
Also, this CI job has somehow been failing on main branch itself since last month
https://github.com/p4lang/p4runtime/actions/workflows/ci-build-proto.yml

@chrispsommers
Copy link
Collaborator

Hi @antoninbas do you have any ideas about the CI failures cited above?

@antoninbas
Copy link
Member

It's a bazel build issue that I haven't had time to look into.
Looks like the failure is because https://mirror.bazel.build/github.com/bazelbuild/rules_proto/archive/9e4c622ba8c2178b71420ed3d14fb8874beee3a5.tar.gz no longer exists. Maybe @smolkaj knows what we should be using instead?

@smolkaj
Copy link
Member

smolkaj commented Jan 27, 2024

Interesting!

Looks like a regression on the main branch.
Last passing run (10 Dec 2023): https://github.com/p4lang/p4runtime/actions/runs/7161170241
First failing run (11 Dec 2023): https://github.com/p4lang/p4runtime/actions/runs/7174991901

@smolkaj
Copy link
Member

smolkaj commented Jan 27, 2024

Here is what changed:

2023/12/11 00:10:44 Downloading https://releases.bazel.build/6.4.0/release/bazel-6.4.0-linux-x86_64...

vs

2023/12/12 00:10:26 Downloading https://releases.bazel.build/7.0.0/release/bazel-7.0.0-linux-x86_64...

Looks like we're not yet ready for Bazel 7.x. This is easy to fix by adding a .bazelversion file. Let me send a PR.

@chrispsommers
Copy link
Collaborator

@saynb Could you do another PR to sync to main and gain the benefit of the Bazel fix? Thx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants