Repository navigation
[DP-1.11] qos: wait for flow transmission stop and in-flight packet drain in bursty_traffic_test - #6259
[DP-1.11] qos: wait for flow transmission stop and in-flight packet drain in bursty_traffic_test#6259arnavanand-lgtm wants to merge 1 commit into
Conversation
…in in bursty_traffic_test
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 addresses a race condition in the bursty_traffic_test where assertions were failing due to transient in-flight packets being counted before they had fully cleared the network buffers. By introducing explicit state monitoring for transmission status and packet counter stabilization, the test now ensures accurate measurement of traffic drops. Additionally, the removal of an arbitrary sleep timer improves the efficiency of the test suite. 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 #6259 / c0bb10aVirtual 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 optimizes the bursty traffic test by removing a static 30-second sleep before starting traffic and introducing dynamic waiting logic in stopTrafficOnFlows using gnmi.Watch to confirm transmission has stopped and in-flight packets have drained. The review feedback identifies a bug in the packet-draining watch logic where a flow receiving zero packets (e.g., due to a traffic generation failure) will cause the watch to always time out after 15 seconds. A code suggestion is provided to resolve this by tracking the previous sample state with a boolean flag instead of a rx > 0 check.
| var lastRx uint64 | ||
| var unchangedCount int | ||
| _, _ = gnmi.Watch(t, ate.OTG(), gnmi.OTG().Flow(trafficID).Counters().InPkts().State(), 15*time.Second, func(val *ygnmi.Value[uint64]) bool { | ||
| rx, present := val.Val() | ||
| if !present { | ||
| return false | ||
| } | ||
| if rx > 0 && rx == lastRx { | ||
| unchangedCount++ | ||
| if unchangedCount >= 2 { | ||
| return true | ||
| } | ||
| } else { | ||
| lastRx = rx | ||
| unchangedCount = 0 | ||
| } | ||
| return false | ||
| }).Await(t) |
There was a problem hiding this comment.
If a flow does not receive any packets (e.g., due to a traffic generation issue, test failure, or in a negative test scenario), rx will be 0. Because of the rx > 0 guard, the else block will always execute, resetting unchangedCount to 0. This causes the gnmi.Watch to always time out after 15 seconds per flow, unnecessarily delaying the test execution when failures occur.
We can track whether we have received a previous sample using a boolean flag (e.g., hasLastRx) instead of checking rx > 0. This allows the watch to successfully terminate when the counter is stable at 0.
var lastRx uint64
var hasLastRx bool
var unchangedCount int
_, _ = gnmi.Watch(t, ate.OTG(), gnmi.OTG().Flow(trafficID).Counters().InPkts().State(), 15*time.Second, func(val *ygnmi.Value[uint64]) bool {
rx, present := val.Val()
if !present {
return false
}
if hasLastRx && rx == lastRx {
unchangedCount++
if unchangedCount >= 2 {
return true
}
} else {
lastRx = rx
hasLastRx = true
unchangedCount = 0
}
return false
}).Await(t)References
- In gNMI telemetry tests, distinguish between cumulative counters and rate metrics: while zero is valid for counters, rates should often be checked for non-zero values (e.g., using IsZero()) to verify active reporting.
Summary
In #5941,
stopTrafficOnFlowswas introduced to asynchronously stop traffic flows between subtests usinggosnappi.StateTrafficFlowTransmitState.STOP. However,SetControlStatereturns immediately without waiting for transmission to halt or in-flight packets to clear the link and switch buffers.When sampling flow counters immediately after
stopTrafficOnFlows, transient in-flight packets (~90–200 packets / 48–100 KB) causedateOutPkts > ateInPktsat the instantgnmi.Getwas called. Because the physical DUT switch forwarded all traffic without drops (dutDropOctetCounterDiff = 0), the assertion:if dutDropOctetCounterDiff < ateDropOctetCounterDiff { t.Errorf(...) }failed with
got 0, want >= <ateDrop>across all 7 QoS queue subtests, despite 100% throughput verified moments later once counters settled.Changes Made
stopTrafficOnFlows:gnmi.OTG().Flow(id).Transmit().State() == falseto ensure OTG transmission has stopped.gnmi.OTG().Flow(id).Counters().InPkts().State()counter stabilization across consecutive samples to ensure all in-flight packets clear the 100G links and switch buffers before assertions execute.time.Sleep(30 * time.Second)beforestartTrafficOnFlows, reducing test execution time by ~3.5 minutes without impacting traffic stability.Verification
Validated on physical hardware testbeds across two independent Arista DCS-7280 switches (Broadcom Jericho2) connected to Keysight Novus 100G traffic generation modules. All 7 QoS queues (
NC1,AF4,AF3,AF2,AF1,BE0,BE1) passed with 100% throughput and zero drops across 6 consecutive runs.DP-1.11 Pass 1.log
DP-1.11 Pass 2.log
DP-1.11 Pass 3.log
DP-1.11 Pass 4.log
DP-1.11 Pass 5.log
DP-1.11 Pass 6.log