SYS-1.1 Add strata platform support to default_copp_test#5460
SYS-1.1 Add strata platform support to default_copp_test#5460keil-arista wants to merge 2 commits into
Conversation
The default_copp_test hardcodes "sand" in the OpenConfig vendor path and
uses sand-specific counter names. Strata platform devices (e.g.
DCS-7050CX4M-48D8-F) require different counter names and a different
vendor path element. This change detects the platform dynamically from
the gNMI response and selects the appropriate counter mapping.
L2 unicast CoPP tests are skipped on strata platforms via a new
deviation because strata drops L2 packets destined to the system MAC
in the pipeline before they reach the CPU.
* (M) feature/system/control_plane_traffic/otg_tests/default_copp_test/default_copp_test.go
- Add platformConfig struct and counter name maps for sand vs strata.
- Detect platform dynamically from gNMI Get response on the vendor path.
- Replace hardcoded "sand" path element with detected platform name.
- Skip L2 unicast test cases on strata via CoppL2UnicastUnsupported deviation.
* (M) feature/system/control_plane_traffic/otg_tests/default_copp_test/metadata.textproto
- Add hardware_model_regex to existing Arista sand platform_exceptions entry.
- Add new platform_exceptions entry for strata (DCS-7050CX4.*) with
copp_l2_unicast_unsupported deviation.
* (M) proto/metadata.proto
- Add copp_l2_unicast_unsupported deviation field (422).
* (M) proto/metadata_go_proto/metadata.pb.go
- Regenerated from metadata.proto.
* (M) internal/deviations/deviations.go
- Add CoppL2UnicastUnsupported accessor.
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 enhances the default_copp_test to support the Strata platform alongside the existing Sand platform. By dynamically detecting the hardware platform via gNMI, the test now correctly selects the appropriate counter mappings for control plane traffic. Additionally, it introduces a new deviation to gracefully handle platform-specific limitations where L2 unicast packets are dropped before reaching the CPU, ensuring test reliability across different Arista hardware models. 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 #5460 / 95e5146Virtual Devices
Hardware Devices
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces platform-specific COPP (Control Plane Policing) counter validation by dynamically detecting the platform type and mapping logical counters to platform-specific names. It also adds a new deviation, CoppL2UnicastUnsupported, to handle hardware that does not support L2 unicast counter validation. Feedback includes a requirement to add issue tracker links to deviation functions per the style guide, and recommendations to use the test context in gNMI requests within helper functions to ensure proper timeout and cancellation handling.
| // CoppL2UnicastUnsupported returns true if the device does not support L2 unicast COPP counter validation. | ||
| func CoppL2UnicastUnsupported(dut *ondatra.DUTDevice) bool { |
There was a problem hiding this comment.
The deviation accessor function is missing a comment with a URL link to an issue tracker, which is required by the repository style guide.
| // CoppL2UnicastUnsupported returns true if the device does not support L2 unicast COPP counter validation. | |
| func CoppL2UnicastUnsupported(dut *ondatra.DUTDevice) bool { | |
| // CoppL2UnicastUnsupported returns true if the device does not support L2 unicast COPP counter validation. | |
| // https://issuetracker.google.com/issues/xxxxx | |
| func CoppL2UnicastUnsupported(dut *ondatra.DUTDevice) bool { |
References
- Add a comment to the accessor function containing a URL link to an issue tracker which tracks removal of the deviation. The format should be https://issuetracker.google.com/issues/xxxxx. (link)
| func getPlatformConfig(t *testing.T, gnmiClient gpb.GNMIClient) *platformConfig { | ||
| t.Helper() | ||
| resp, err := gnmiClient.Get(context.Background(), &gpb.GetRequest{ |
There was a problem hiding this comment.
It is recommended to pass the test context to getPlatformConfig and use it in the gNMI Get request instead of context.Background(). This ensures that the gNMI operation respects the test's timeout and cancellation. Additionally, ensure t.Helper() is called as the first statement in the helper function.
| func getPlatformConfig(t *testing.T, gnmiClient gpb.GNMIClient) *platformConfig { | |
| t.Helper() | |
| resp, err := gnmiClient.Get(context.Background(), &gpb.GetRequest{ | |
| func getPlatformConfig(t *testing.T, ctx context.Context, gnmiClient gpb.GNMIClient) *platformConfig { | |
| t.Helper() | |
| resp, err := gnmiClient.Get(ctx, &gpb.GetRequest{ |
References
- Helper functions that report test errors should call t.Helper() as the first statement to ensure error logs point to the caller's location.
| return | ||
| } | ||
|
|
||
| platCfg := getPlatformConfig(t, gnmiClient) |
There was a problem hiding this comment.
Pass the test context ctx to getPlatformConfig.
| platCfg := getPlatformConfig(t, gnmiClient) | |
| platCfg := getPlatformConfig(t, ctx, gnmiClient) |
2 participants
The default_copp_test hardcodes "sand" in the OpenConfig vendor path and uses sand-specific counter names. Strata platform devices (e.g. DCS-7050CX4M-48D8-F) require different counter names and a different vendor path element. This change detects the platform dynamically from the gNMI response and selects the appropriate counter mapping.
L2 unicast CoPP tests are skipped on strata platforms via a new deviation because strata drops L2 packets destined to the system MAC in the pipeline before they reach the CPU.