-
Notifications
You must be signed in to change notification settings - Fork 109
[testutils] Optimize the runtime of the method verify_no_packet_any for large port sets #231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[testutils] Optimize the runtime of the method verify_no_packet_any for large port sets #231
Conversation
51712d8 to
6735621
Compare
| return True # PASS - packet not observed within timeout window | ||
|
|
||
| for port in ports: | ||
| result = dp_poll( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you still validate it port-by-port, and not in parallel unless I don't understand the code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, the validation is still per port, but the key change is that the timeout is now global, not per-port.
Each port is polled in a non-blocking way into a loop bounded by a single timeout window.
It's not parallel, but it reduces the runtime from O(N × timeout) to O(timeout) and still detects packets on any port. Parallel way is not really possible here because the poll operates on a shared dataplane. Polling it from multiple threads not providing additional benefits and can lead to race conditions.
… scale ports number Signed-off-by: AntonHryshchuk <antonh@nvidia.com>
6735621 to
3d402ef
Compare
| if port in ports: | ||
| print("verifying packet on port device", device_number, "port", port) | ||
| verify_no_packet(test, pkt, (device, port), timeout=timeout) | ||
| ports = list(ports) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't change the original loop logic:
for device, port in ptf_ports():
if port in ports:
It's possible that a port in ports is not in ptf_ports(). Looks like there will be an exception in dp_poll if the port is not in ptf_ports().
|
|
||
| # Small sleep to avoid busy-looping and excessive CPU usage. | ||
| # Also gives dataplane threads time to enqueue incoming packets. | ||
| time.sleep(0.01) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have a bigger value? 0.01s looks still too short.
|
|
||
|
|
||
| def verify_no_packet_any(test, pkt, ports=[], device_number=0, timeout=None): | ||
| def verify_no_packet_any(test, pkt, ports=[], device_number=0, timeout=1): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default value of timeout was ptf.ptfutils.default_negative_timeout, we should keep it the same.
| while True: | ||
| remaining = timeout - (time.monotonic() - start) | ||
| if remaining <= 0: | ||
| return True # PASS - packet not observed within timeout window |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only "return" is enough here, which is the same as before.
Optimize verify_no_packet_any method for large port sets
The original implementation waits for the full timeout on each port
sequentially, resulting in O(N * timeout) runtime (e.g. 500ports → 50 seconds, with 0.1 second of default timeout).
This change introduces a single global timeout and repeatedly polls all ports
using non-blocking dp_poll(timeout=0), ensuring:
Also adds a small sleep to avoid busy looping and reduce CPU usage.