Minimal e2e implementation of packet trace. by c8ef · Pull Request #1433 · p4lang/behavioral-model

c8ef · 2026-07-28T14:09:00Z

With this patch we will have a minimal and workable implementation of packet trace with testcases.

c8ef · 2026-07-28T15:15:42Z

cc @matthewtlam @fruffy @jafingerhut @grg

fruffy · 2026-07-29T15:19:43Z

How could we test this? E.g., for the compiler we have golden tests to ensure that the output matches our expectations. We could also use gtests and pattern match, but that is harder to maintain and review and might not capture the full picture of a trace.

c8ef · 2026-07-29T15:46:42Z

How could we test this? E.g., for the compiler we have golden tests to ensure that the output matches our expectations. We could also use gtests and pattern match, but that is harder to maintain and review and might not capture the full picture of a trace.

During development I have a local smoke test script(which generates the trace in the pr description). Though I haven't think very thoroughly on how to test this. Since it is using existing infrastructure I hope we could reuse the existing nanomsg backend testing, but not sure if that exists.

fruffy · 2026-07-29T15:56:40Z

How could we test this? E.g., for the compiler we have golden tests to ensure that the output matches our expectations. We could also use gtests and pattern match, but that is harder to maintain and review and might not capture the full picture of a trace.

During development I have a local smoke test script(which generates the trace in the pr description). Though I haven't think very thoroughly on how to test this. Since it is using existing infrastructure I hope we could reuse the existing nanomsg backend testing, but not sure if that exists.

Basically you want avoid that this feature slowly bitrots. Ideally, we have infra for E2E tests that run BMv2, generate a trace and then we compare outputs.

matthewtlam · 2026-07-29T19:37:33Z

@c8ef I think we should be producing golden tests for this and adding appropriate test cases so that we can see if the behavior is as we expect. Ideally, we would iterate over the different P4 pipelines that we have define and send the corresponding input packets through BMv2 and then we output the packet trace in a golden file (.expected file). Since you added the Bazel support, we can do some golden tests. Here are a couple of examples:

This is the corresponding test runner: https://github.com/google/p4-infra/blob/f107159078db6ba189a413d7308f00b4c0d1d9fe/p4_pdpi/references_test_runner.cc

This is a sample expected file: https://github.com/google/p4-infra/blob/f107159078db6ba189a413d7308f00b4c0d1d9fe/p4_pdpi/references.expected

We can also add the golden tests to the Github workflow.

c8ef · 2026-07-30T14:34:49Z

@c8ef I think we should be producing golden tests for this and adding appropriate test cases so that we can see if the behavior is as we expect. Ideally, we would iterate over the different P4 pipelines that we have define and send the corresponding input packets through BMv2 and then we output the packet trace in a golden file (.expected file). Since you added the Bazel support, we can do some golden tests. Here are a couple of examples:

This is the corresponding test runner: https://github.com/google/p4-infra/blob/f107159078db6ba189a413d7308f00b4c0d1d9fe/p4_pdpi/references_test_runner.cc

This is a sample expected file: https://github.com/google/p4-infra/blob/f107159078db6ba189a413d7308f00b4c0d1d9fe/p4_pdpi/references.expected

We can also add the golden tests to the Github workflow.

@fruffy @matthewtlam

Thanks for the pointers on the test runners! Following that pattern, I added a similar test runner for BMv2 and the structured packet trace test. Since the test isn't necessarily related to gRPC, I added it to the simple_switch target directory.

Signed-off-by: c8ef <c8ef@outlook.com>

matthewtlam

Sending some initial comments. Sorry for the late review, I've been on vacation and haven't had a good chance to take a look at things. Also the PR is big so it will take sometime. Will try to have faster reviews. LOoking good as a start!

matthewtlam · 2026-08-09T10:06:25Z

+Output packets:
+  port=1 len=6 bytes=000000010000
+Packet traces:
+--- trace_1.txtpb


The numbering of the traces should be unique and ideally start at 0 for each packet test. It will be hard to keep track of the correct number of expected outputs for cases like WCMP where we want to make sure that we output the correct number of traces

In current design the trace filename closely related to packet_id, which in the parser_error case there are precisely 3 packets here. I think we could use this to test the number logic here, and if there is special case like WCMP, maybe we could limit to one packet, and focus on the single trace?

I think for WCMP we would not want to only show a single packet nor a single packet trace but show a comprehensive set of packets despite being lengthy as it helps provide all the debug info needed. We should adjust the output of this golden test to account for this

Understood. Now in the latest version, the traces and packet_ids have been renamed, so we can clearly see the expected number of packet or packet_id.

matthewtlam · 2026-08-09T10:11:15Z

+-- INPUT ----------------------------------------------------------------
+P4 pipeline: recirc.json
+Table programming:
+  (none)


For recirculate, we should have table entries such that we expect to see the packet entering the ingress + egress stage at least twice. Perhaps add another test case here to show this property off

Actually it has the default table entry (see the related json file for more detail), so here (none) means no manually added table programming. I should make it clearer here.

Fixed the trace file description and removed the redundant manually defined table entry in e5ebb0e

Ack sounds good. Though we should have more comprehensive tests in itself for the recirculate behavior in itself. Perhaps we should open an issue to track this

I think when packet trace relates to recirculate is implemented, current test will be sufficient for the recirculate testing? If not we can add more test then.

Signed-off-by: c8ef <c8ef@outlook.com>

matthewtlam · 2026-08-09T14:30:52Z

+Output packets:
+  port=1 len=6 bytes=000000010000
+Packet traces:
+--- trace_1.txtpb


I think for WCMP we would not want to only show a single packet nor a single packet trace but show a comprehensive set of packets despite being lengthy as it helps provide all the debug info needed. We should adjust the output of this golden test to account for this

matthewtlam · 2026-08-09T14:32:05Z

+-- INPUT ----------------------------------------------------------------
+P4 pipeline: recirc.json
+Table programming:
+  (none)


Ack sounds good. Though we should have more comprehensive tests in itself for the recirculate behavior in itself. Perhaps we should open an issue to track this

matthewtlam · 2026-08-09T14:42:58Z

+
+  std::cout << kOutputBanner << "\n";
+  std::cout << "Output packets:\n";
+  if (outputs.empty()) std::cout << "  (none)\n";


May be nice to specify the packets get dropped, but we can handle that in a follow up PR as well

I think if the packets get dropped in Packet traces: section we can clearly see the traces relate to drop. Adding the information here is also acceptable.

Signed-off-by: c8ef <c8ef@outlook.com>

fruffy · 2026-08-15T22:47:33Z


      - name: Test
-        run: bazel test //targets/simple_switch_grpc/tests/...
+        run: |


My one request is also to add test support for cmake here. Bazel-only is not great for the wider community

Since this PR is already huge I suggest this could be done in follow up PRs.

I prefer to merge this only when it is complete. This PR only changes 19 files, most of which are test files.

Okay. But I'm not familiar with all these build systems so it might take a while.

c8ef force-pushed the next branch from 69e2312 to d608fe2 Compare July 28, 2026 14:47

c8ef marked this pull request as ready for review July 28, 2026 15:12

c8ef changed the title ~~Draft~~ Minimal e2e implementation of packet trace. Jul 28, 2026

c8ef force-pushed the next branch from d608fe2 to e6514ed Compare July 29, 2026 12:22

c8ef force-pushed the next branch 2 times, most recently from e97f11d to 2712f9c Compare July 30, 2026 14:30

minimal MVP

2c82470

Signed-off-by: c8ef <c8ef@outlook.com>

c8ef force-pushed the next branch from 2712f9c to 2c82470 Compare July 30, 2026 17:08

Merge branch 'main' into next

b6dd50c

matthewtlam reviewed Aug 9, 2026

View reviewed changes

c8ef added 2 commits August 9, 2026 19:57

git rid of auto type

350877b

Signed-off-by: c8ef <c8ef@outlook.com>

tweak test

1339367

Signed-off-by: c8ef <c8ef@outlook.com>

c8ef requested a review from matthewtlam August 9, 2026 13:30

refine

e5ebb0e

Signed-off-by: c8ef <c8ef@outlook.com>

matthewtlam reviewed Aug 9, 2026

View reviewed changes

c8ef added 2 commits August 10, 2026 23:01

rename hex_dump

50cef99

Signed-off-by: c8ef <c8ef@outlook.com>

simplify wait traces

528a27c

Signed-off-by: c8ef <c8ef@outlook.com>

c8ef force-pushed the next branch from e90d748 to 528a27c Compare August 10, 2026 15:24

rename packet trace

83437df

Signed-off-by: c8ef <c8ef@outlook.com>

c8ef requested a review from matthewtlam August 10, 2026 15:51

fruffy requested review from grg and removed request for matthewtlam August 15, 2026 22:46

fruffy reviewed Aug 15, 2026

View reviewed changes

Uh oh!

Conversation

c8ef commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

c8ef commented Jul 28, 2026

Uh oh!

fruffy commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

c8ef commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fruffy commented Jul 29, 2026

Uh oh!

matthewtlam commented Jul 29, 2026

Uh oh!

c8ef commented Jul 30, 2026

Uh oh!

matthewtlam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

c8ef commented Jul 28, 2026 •

edited

Loading

fruffy commented Jul 29, 2026 •

edited

Loading

c8ef commented Jul 29, 2026 •

edited

Loading