Skip to content

Fix header size to be multiple of 8 bits#5553

Open
pacokwon wants to merge 1 commit intop4lang:mainfrom
pacokwon:custom-type-restricted-fields-bmv2
Open

Fix header size to be multiple of 8 bits#5553
pacokwon wants to merge 1 commit intop4lang:mainfrom
pacokwon:custom-type-restricted-fields-bmv2

Conversation

@pacokwon
Copy link

Resolves #5535

The BMv2 target only supports headers with fields totaling a multiple of 8 bits. However, in testdata/p4_16_samples/custom-type-restricted-fields.p4, the header type packet_out_t had a size of 65 bits and was being rejected by BMv2. Changing queue_id to 7 bits satisfies the header size constraint and preserves the semantics of the program, as queue_id is not being utilized elsewhere in the program.

@jafingerhut
Copy link
Contributor

Resolves #5535

The BMv2 target only supports headers with fields totaling a multiple of 8 bits. However, in testdata/p4_16_samples/custom-type-restricted-fields.p4, the header type packet_out_t had a size of 65 bits and was being rejected by BMv2. Changing queue_id to 7 bits satisfies the header size constraint and preserves the semantics of the program, as queue_id is not being utilized elsewhere in the program.

The changes look good to me, but in order to merge this you need to sign your commit using DCO. See the page for this PR and click on the link labeled "DCO" near the bottom of the page for instructions on how to do so after you have created a PR. Alternately, if you find following those instructions troublesome you can close this PR and create a new one and be sure to use git commit -s . when committing your changes for that PR.

@jafingerhut jafingerhut reopened this Mar 20, 2026
Signed-off-by: pacokwon <haechank@gmail.com>
@pacokwon pacokwon force-pushed the custom-type-restricted-fields-bmv2 branch from 8b5e9a3 to cd3126f Compare March 20, 2026 01:13
@pacokwon
Copy link
Author

Thanks for letting me know. I just pushed a signed commit. Please take a look!

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.

LGTM

@jafingerhut
Copy link
Contributor

@pacokwon You need to update some expected output files in the directory testdata/p4_16_samples_outputs, too, to correspond with the changes you made to the test program, otherwise existing tests that compare the current compiler output against expected fail.

Look in the top level README.md of the p4c repo for example commands using P4TEST_REPLACE, for one way to create locally on your system the new expected output files.

@fruffy fruffy added the infrastructure Topics related to code style and build and test infrastructure. label Mar 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infrastructure Topics related to code style and build and test infrastructure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

custom-type-restricted-fields.p4 is invalid for BMv2

3 participants