Adding readme and new topology file required for dataplane hashing test#5570
Adding readme and new topology file required for dataplane hashing test#5570harprsie-g wants to merge 1 commit into
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces the necessary infrastructure and documentation to support a new dataplane hashing test. The changes define a complex network topology involving multiple physical and software loopbacks, enabling the verification of traffic distribution across various network instances. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Pull Request Functional Test Report for #5570 / b11e690Virtual Devices
Hardware Devices
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new dataplane hashing test using physical and software loopbacks across multiple network instances, including a test plan README, metadata configuration, and a 10-link testbed topology. Feedback on these changes includes adding the required OpenConfig Path and RPC Coverage section to the README, assigning the lc2_p9 interface to the EGRESS VRF to ensure proper traffic routing, and correcting the testbed enum in the metadata file to match the 10-link topology instead of 34 links.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| - Verify packet distribution: | ||
| - **Stage 1**: ~70% to `lc1_p3`, ~10% to each of the 3 soft loops. | ||
| - **Stage 2 (Transit VRF)**: ~12.5% to each of the 8 members of NHG 2 (within tolerance). | ||
| - **Stage 3 (Self-Site VRF)**: ~12.5% to each of the 8 members of NHG 3 (within tolerance). No newline at end of file |
There was a problem hiding this comment.
The test README.md is missing the OpenConfig Path and RPC Coverage section, which is required by the repository's test plan template. This section is important for tracking the telemetry and configuration paths covered by the test. Additionally, when adding new telemetry paths (such as /interfaces/interface/state/counters) to the coverage section, please ensure the test procedure section is updated to describe how these counters are verified, and that the test implementation code is also updated to perform this verification.
| - **Stage 3 (Self-Site VRF)**: ~12.5% to each of the 8 members of NHG 3 (within tolerance). | |
| - **Stage 3 (Self-Site VRF)**: ~12.5% to each of the 8 members of NHG 3 (within tolerance). | |
| ## OpenConfig Path and RPC Coverage | |
| The following OpenConfig paths and RPCs are covered by this test: | |
| ### OpenConfig Paths | |
| * /interfaces/interface/config/name | |
| * /interfaces/interface/config/enabled | |
| * /interfaces/interface/state/counters | |
| * /network-instances/network-instance/config/name | |
| * /network-instances/network-instance/config/type | |
| * /network-instances/network-instance/interfaces/interface/config/id | |
| * /network-instances/network-instance/interfaces/interface/config/interface | |
| ### gRIBI RPCs | |
| * gRIBI.Modify |
References
- The test README.md should be structured following the test plan template, which includes an OpenConfig Path and RPC Coverage section. (link)
- When adding new telemetry paths to the coverage section of a test's README.md, ensure the test procedure section is updated to describe how these counters are verified, and that the test implementation code is also updated to perform this verification.
| - Default VRF: Ingress `lc2_p10`, Stage 1 Soft Loops. | ||
| - `TRANSIT` VRF: `lc2_p3` (Loop 1 RX), `lc1_p4` (Loop 2 TX), `lc1_p5` (Loop 3 TX), `lc1_p6` (Loop 4 TX), `lc1_p1` (Loop 5 TX), Stage 2 Soft Loops. | ||
| - `SELF_SITE` VRF: `lc2_p6` (Loop 4 RX), `lc2_p1` (Loop 5 RX), `lc2_p8` (Loop 6 TX), `lc2_p7` (Loop 7 TX), `lc2_p2` (Loop 8 TX), Stage 3 Soft Loops. | ||
| - `EGRESS` VRF: `lc2_p4` (Loop 2 RX), `lc2_p5` (Loop 3 RX), `lc1_p8` (Loop 6 RX), `lc1_p7` (Loop 7 RX), `lc1_p2` (Loop 8 RX). |
There was a problem hiding this comment.
The interface lc2_p9 (which is connected to ate:ixia1 in the testbed topology topologies/dut_8_loop_2_ate.testbed) is not assigned to any network instance in the baseline configuration. To ensure that egress traffic can be successfully routed back to the ATE, lc2_p9 should be assigned to the EGRESS VRF.
There was a problem hiding this comment.
Interface belong to Default VRF if it's not explicitly assigned to any VRF.
| uuid: "b6f845b2-597d-5eb8-c48b-96ffc6082d71" | ||
| plan_id: "Hashing" | ||
| description: "Hashing with Physical/Software Loopbacks" | ||
| testbed: TESTBED_DUT_ATE_34LINKS |
There was a problem hiding this comment.
The testbed is specified as TESTBED_DUT_ATE_34LINKS, but the corresponding testbed file topologies/dut_8_loop_2_ate.testbed defines a topology with only 10 links (8 loopback links and 2 ATE links). Please verify and update the testbed field to the correct enum value that matches this 10-link topology.
The PR add a readme file and specific testbed topology required for hashing test.