[P4Testgen] Clean up the implementation of the BMv2 clone externs. #3976

fruffy · 2023-04-10T17:17:39Z

Calling clone again in the ingress control will override any previous clone calls. We were not modelling this behavior. Now, we use a clone info test object which contains the execution state at the time the clone call was made. This clone info object is converted into a clone info spec after the deparser.
We were also not respecting the session ID. If we are able to control the session ID, we have to limit ourselves to a particular range of values.
Introduce a weak notion of a "traffic manager" extern, which handles calls such as recirculate or clone.
Refactor the clone class in BMV2's expression stepper, add more comments, and clean up the code.
Factor the test object class out of the test objects file and introduce a test object map.
Various little fixes here and there.

With this pull request, pins_middleblock.p4 should be testable without any errors.

fruffy · 2023-04-11T20:46:32Z

testdata/p4_16_samples/pins/pins_fabric.p4

@@ -372,7 +372,7 @@ control routing(in headers_t headers, inout local_metadata_t local_metadata, ino
        local_metadata.route_metadata = route_metadata;
    }
    @id(0x0100000F) action trap() {
-        clone(CloneType.I2E, 1024);


@smolkaj Just a quick clarification question. Is the session ID value here intentional? I had to change it to 511 because the open-source P4Runtime server only allows session IDs numbering lower than 512.

Good catch!

This is a pretty old version of our programs. In the latest version, we use

#define COPY_TO_CPU_SESSION_ID 255

Ah great, that makes things easier. Do you know whether or when there is a refresh coming to the files at
https://github.com/sonic-net/sonic-pins/tree/main/sai_p4/instantiations/google
?

@rhalstea do you know the answer?

Generally we would be happy to have our up-to-date programs upstreamed to p4c, we should just keep a copyright notice.

We usually only update the programs in https://github.com/sonic-net/sonic-pins/tree/main/sai_p4/instantiations/google when we're upstreaming new features into a SONiC release which have around a 6 month cadence. However, we don't always upstream new P4 features each release.

If we want to have up-to-date programs I like @smolkaj's suggestion of p4c. The copies in sonic-pins need to be kept in sync with the P4RT App so as to not break the component tests, and that dependency would be the bottleneck.

I guess I didn't explicitly answer @fruffy's question about the next refresh ... It won't be anytime soon. The feature's we're targeting for the May SONiC release don't require a P4 program updates, and we haven't decided on the features to upstream for the next November release.

Thanks, that makes sense. In the meantime we can fix issues manually until the next refresh. Or we could update out-of-band (directly from the private repo to p4c's versions) , if necessary. Currently, there is no pressing need.

fruffy · 2023-04-14T18:06:33Z

@nikil21 @abhinav2sri

This will make the implementation of recirculation functions easier.

…licit info objects to maintain state.

fruffy added the p4tools Topics related to the P4Tools back end label Apr 10, 2023

fruffy force-pushed the fruffy/clone_fixes branch 3 times, most recently from c09f4d5 to c59311f Compare April 10, 2023 23:19

fruffy marked this pull request as ready for review April 11, 2023 14:19

fruffy requested review from pkotikal, jnfoster and mihaibudiu as code owners April 11, 2023 14:19

fruffy commented Apr 11, 2023

View reviewed changes

fruffy force-pushed the fruffy/clone_fixes branch from c59311f to 5953941 Compare April 11, 2023 23:11

fruffy changed the title ~~[P4Testgen] Clean up the implementation of the BMv2 clone implementation.~~ [P4Testgen] Clean up the implementation of the BMv2 clone externs. Apr 12, 2023

fruffy force-pushed the fruffy/clone_fixes branch from 642d842 to 366132a Compare April 14, 2023 18:05

fruffy force-pushed the fruffy/clone_fixes branch 2 times, most recently from 7742e13 to d6c6d7d Compare April 19, 2023 00:22

fruffy added 4 commits April 28, 2023 18:09

Clean up the implementation of the BMv2 clone implementation. Use exp…

1e8e1b3

…licit info objects to maintain state.

A different clone id is not needed. Instead, we should fix PI.

5b09f91

Update the session ID maximum.

4405261

Merge fixes.

d56c569

fruffy force-pushed the fruffy/clone_fixes branch from d6c6d7d to d56c569 Compare April 28, 2023 22:18

pkotikal approved these changes May 2, 2023

View reviewed changes

fruffy merged commit 7be200d into main May 2, 2023

fruffy deleted the fruffy/clone_fixes branch May 2, 2023 12:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[P4Testgen] Clean up the implementation of the BMv2 clone externs. #3976

[P4Testgen] Clean up the implementation of the BMv2 clone externs. #3976

fruffy commented Apr 10, 2023 •

edited

Loading

fruffy Apr 11, 2023

smolkaj Apr 11, 2023

fruffy Apr 12, 2023

smolkaj Apr 12, 2023

smolkaj Apr 12, 2023

rhalstea Apr 12, 2023

rhalstea Apr 12, 2023 •

edited

Loading

fruffy Apr 12, 2023

fruffy commented Apr 14, 2023

[P4Testgen] Clean up the implementation of the BMv2 clone externs. #3976

[P4Testgen] Clean up the implementation of the BMv2 clone externs. #3976

Conversation

fruffy commented Apr 10, 2023 • edited Loading

fruffy Apr 11, 2023

Choose a reason for hiding this comment

smolkaj Apr 11, 2023

Choose a reason for hiding this comment

fruffy Apr 12, 2023

Choose a reason for hiding this comment

smolkaj Apr 12, 2023

Choose a reason for hiding this comment

smolkaj Apr 12, 2023

Choose a reason for hiding this comment

rhalstea Apr 12, 2023

Choose a reason for hiding this comment

rhalstea Apr 12, 2023 • edited Loading

Choose a reason for hiding this comment

fruffy Apr 12, 2023

Choose a reason for hiding this comment

fruffy commented Apr 14, 2023

fruffy commented Apr 10, 2023 •

edited

Loading

rhalstea Apr 12, 2023 •

edited

Loading