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

Load table action from json without 'name' field for BMV2 #3435

Merged
merged 5 commits into from
Sep 2, 2022

Conversation

VolodymyrPeschanenkoIntel
Copy link
Contributor

This small changes in script allow to load table action from JSON file without name field.
Without this changes this example finished with the message:
Exception 'name'

name = ""
if "name" in k:
name = k["name"]
else:
Copy link
Contributor

Choose a reason for hiding this comment

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

Given the documentation for the name and target keys of a table definition in the BMv2 JSON file that you can find here https://github.com/p4lang/behavioral-model/blob/main/docs/JSON_format.md, then search for the only occurrence of "name: this attribute", this change appears like a correct one to me, in order to handle a larger set of correct BMv2 JSON files. Pinging @antoninbas to see if he has any comment on this change, but I will approve this change regardless, in case he is busy.

Copy link
Contributor

@jafingerhut jafingerhut left a comment

Choose a reason for hiding this comment

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

Looks good to me. It is a bit odd to have a test case that intentionally uses fields of a header that is known to be invalid, but I suspect it might be difficult to find another test case that triggers the same issue, and the compiler does give good warnings for the source program.

Copy link
Member

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

I didn't look at the test programs, and I would expect p4c to always generate the "name" attribute, but I agree with the change to bmv2stf.py. The "name" attribute is indeed optional.

@fruffy fruffy merged commit ead1595 into p4lang:main Sep 2, 2022
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.

4 participants