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

Add support for new platform property annotations for P4Runtime. #4611

Merged
merged 11 commits into from
May 31, 2024

Conversation

jonathan-dilorenzo
Copy link
Contributor

See this PR for more information: p4lang/p4runtime#472

@fruffy fruffy added the control-plane Topics related to the control-plane or P4Runtime. label Apr 16, 2024
@fruffy
Copy link
Collaborator

fruffy commented May 15, 2024

p4lang/p4runtime#472 has been merged. Is this PR ready for review?

@jonathan-dilorenzo jonathan-dilorenzo changed the title [NOT READY] Add tentative support for new platform property annotations for P4Runtime. Add tentative support for new platform property annotations for P4Runtime. May 30, 2024
@jonathan-dilorenzo jonathan-dilorenzo marked this pull request as ready for review May 30, 2024 18:46
@jonathan-dilorenzo jonathan-dilorenzo changed the title Add tentative support for new platform property annotations for P4Runtime. Add support for new platform property annotations for P4Runtime. May 30, 2024
control-plane/p4RuntimeSerializer.cpp Outdated Show resolved Hide resolved
control-plane/p4RuntimeSerializer.cpp Outdated Show resolved Hide resolved
control-plane/p4RuntimeSerializer.cpp Show resolved Hide resolved
Copy link
Member

@smolkaj smolkaj left a comment

Choose a reason for hiding this comment

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

Very cool!

@platform_property(
multicast_group_table_size = 10,
multicast_group_table_total_replicas = 20,
multicast_group_table_max_replicas_per_entry = 30
Copy link
Member

Choose a reason for hiding this comment

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

It would be really wonderful (but certainly not a blocker) if we could support optional, trailing commas here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, but I believe not currently how the KV parser works, which I just call without modification. @fruffy might know better though?

@jonathan-dilorenzo jonathan-dilorenzo added this pull request to the merge queue May 31, 2024
Merged via the queue into main with commit cc3a4a0 May 31, 2024
17 checks passed
@jonathan-dilorenzo jonathan-dilorenzo deleted the support_new_p4runtime_annotations branch May 31, 2024 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
control-plane Topics related to the control-plane or P4Runtime.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants