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] Represent bits of width zero as an empty string. #4852

Merged
merged 1 commit into from
Aug 12, 2024

Conversation

fruffy
Copy link
Collaborator

@fruffy fruffy commented Aug 5, 2024

Do not return "something" when the bitwidth is zero. Instead handle this case separately. This fixes some problems when trying to test for 0 bit packets emitted in the packet test framework.

@fruffy fruffy added the p4tools Topics related to the P4Tools back end label Aug 5, 2024
@fruffy fruffy marked this pull request as ready for review August 5, 2024 07:57
@fruffy fruffy requested a review from vlstill August 5, 2024 07:58
@fruffy
Copy link
Collaborator Author

fruffy commented Aug 12, 2024

Required for #4697. @vlstill Could you give this a review. This fixes some packet mismatches when generating PTF tests.

@vlstill
Copy link
Contributor

vlstill commented Aug 12, 2024

This seems quite weird to me, formatting a number I would not expect to get an empty string. Especially in the case the base prefix is expected. How does an example output in PTF look (a snippet whith the empty packet suffices)? Would it make more sense to handle this special case in the PTF formatter only?

@fruffy
Copy link
Collaborator Author

fruffy commented Aug 12, 2024

This seems quite weird to me, formatting a number I would not expect to get an empty string. Especially in the case the base prefix is expected. How does an example output in PTF look (a snippet whith the empty packet suffices)? Would it make more sense to handle this special case in the PTF formatter only?

How do you represent a number with width zero and no value? "0x0" also does not seem correct to me and you can distinguish between an expression with width and one that has none. The empty string signals that you requested a value that returns nothing and you can process this as needed. I can also make this a std::optional<> to distinguish between errors and an empty return.

In PTF this would look like:

exp_pkt = Mask(b'')

In STF a packet that is 0 actually leads to garbled output (02000000). Because we set the expected packet top this value in STF the problem has been masked there. 0x0 also would be incorrect in STF because it is interpreted as a malformed 4 bit output.

@vlstill
Copy link
Contributor

vlstill commented Aug 12, 2024

Hmm, that is really a bad design on the testing frameworks :-/, they really should take width with the packet (that is why in my backend, which uses a custom format, I always use the P4 sized notation for literals e.g. it would be 0w0b0). I guess this can't be really applied to existing frameworks though :-/.

Copy link
Contributor

@vlstill vlstill left a comment

Choose a reason for hiding this comment

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

OK, let's go for this to make PTF work, even though it is not very nice.

@fruffy fruffy added this pull request to the merge queue Aug 12, 2024
Merged via the queue into main with commit 9c98084 Aug 12, 2024
17 checks passed
@fruffy fruffy deleted the fruffy/testgen_format_correction branch August 12, 2024 19:41
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.

2 participants