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

[P4Testgen] Encode more P4Runtime constraints for the behavioral model #4103

Merged
merged 1 commit into from
Oct 3, 2023

Conversation

fruffy
Copy link
Collaborator

@fruffy fruffy commented Aug 9, 2023

@fruffy fruffy added the p4tools Topics related to the P4Tools back end label Aug 9, 2023
@fruffy fruffy force-pushed the fruffy/p4runtime_encodings branch from d14ce6f to afd3598 Compare August 26, 2023 18:31
@fruffy fruffy requested a review from jafingerhut September 13, 2023 15:00
@jafingerhut
Copy link
Contributor

Is there a brief description of this change you can give? From scanning the most dense part of the code changes, it appears that you are adding support for ternary fields, and perhaps also adding the ability of optional match_kind keys to be all-wildcard, whereas before this change you only ever created all-exact-match table entries for optional match_kind?

@fruffy
Copy link
Collaborator Author

fruffy commented Sep 13, 2023

This change has been prompted by p4lang/p4-constraints#100. We are trying to align more with the p4runtime specification in terms of how we generate our table entries. This required some changes to the optional matches we are making and how we produce ternary matches. This PR encodes this in the form of additional constraints.

@jafingerhut
Copy link
Contributor

@fruffy So I do not have the necessary context to do what I would think of as a proper code review for this kind of change in p4testgen, and I am not sure I have the time required to gain that context any time soon. By proper code review, I mean thinking about the logic of the changes, and understanding whether they seem correct, or have some bug in them. I know the P4Runtime API constraints pretty well, but I don't think that is enough.

Would you prefer a rubber-stamp LGTM that means "yeah, I see CI tests did not break", or someone who can give a proper review?

@fruffy fruffy force-pushed the fruffy/p4runtime_encodings branch from afd3598 to a9f53c1 Compare October 1, 2023 14:39
@fruffy fruffy merged commit d12db66 into main Oct 3, 2023
@fruffy fruffy deleted the fruffy/p4runtime_encodings branch October 3, 2023 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p4tools Topics related to the P4Tools back end
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants