[P4Testgen] Introduce a new Protobuf backend which uses P4 PDPI instead of P4Runtime #4221
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fruffy-g
force-pushed
the
protobuf-ir
branch
3 times, most recently
from
2ae0d88
to
e37ed27
Compare
fruffy-g
force-pushed
the
protobuf-ir
branch
from
264c18f
to
00ff70c
Compare
fruffy-g
changed the title
fruffy-g
force-pushed
the
protobuf-ir
branch
2 times, most recently
from
58131b4
to
bf964c5
Compare
smolkaj
requested changes
Nov 21, 2023
backends/p4tools/modules/testgen/targets/bmv2/test/BMV2ProtobufIrXfail.cmake
Outdated
Show resolved
Hide resolved
backends/p4tools/modules/testgen/targets/bmv2/test_backend/protobuf_ir.h
Show resolved
Hide resolved
backends/p4tools/modules/testgen/targets/bmv2/test_backend/protobuf_ir.cpp
Outdated
Show resolved
Hide resolved
backends/p4tools/modules/testgen/targets/bmv2/test_backend/protobuf_ir.cpp
Outdated
Show resolved
Hide resolved
| # Match field {{r.field_name}} | ||
| matches { | ||
| name: "{{r.field_name}}" | ||
| exact: { hex_str: "{{r.value}}" } |
There was a problem hiding this comment.
Two small points here:
hex_strmust be a string of the form "0x....". Do we need to add an "0x"-prefix here?- This can be
hex_str,ipv4,ipv6,mac, orstr, and PDPI currently enforces that you use the format that is specified in the program using annotations of the form@format(IPV4_ADDRESS)etc (default is hex). So we would need to either branch on the format here (which you can obtain easily from the IrP4Info). Or we would need to add a PDPI post-processing step that corrects the format, or perhaps extend PDPI with an option to be more lenient.
There was a problem hiding this comment.
Yes, the hex string is already prefixed. Not sure about the second point: Do you mean that a generic input hex string is not supported and that we always need some form of ipv4, ipv6, etc assignment?
There was a problem hiding this comment.
Yes, that's right.
PDPI has different formats, and accepts only the one that is specified in the P4 program.
There was a problem hiding this comment.
Perhaps can be solved in a follow up.
There was a problem hiding this comment.
How is that specified? With an annotation?
fruffy-g
force-pushed
the
protobuf-ir
branch
from
a9f7462
to
7590e9a
Compare
smolkaj
approved these changes
Nov 30, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This test back end reuses much of the PTF test back end code, but instead produces a text protobuf file. This text protobuf file uses the p4_pdpi format. This format uses human-readable control-plane names (similar to STF and PTF) instead of requiring P4Runtime IDs for configuration.
A couple changes were necessary for this to work:
ir.prototo the P4Testgen BMv2protofolder since we do not want to depend on the large pins repository.code.prototocontrol-plane/google/rpc/code.protoand refreshstatus.protoPROTOBUF_IRback end and start to deprecate the original PROTOBUF back end. Much of the functionality of PROTOBUF is covered by PROTOBUF_IR.