-
Notifications
You must be signed in to change notification settings - Fork 111
[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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3328,21 +3328,47 @@ def verify_packets(test, pkt, ports=[], device_number=0, timeout=None, n_timeout | |
| verify_no_other_packets(test, device_number=device_number, timeout=n_timeout) | ||
|
|
||
|
|
||
| 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): | ||
| """ | ||
| Check that a packet is NOT received on _any_ of the specified ports belonging to | ||
| the given device (default device_number is 0). | ||
| Verify that a packet is NOT received on any of the specified ports belonging to | ||
| the given device within the given timeout. Uses a single global timeout and repeatedly | ||
| polls all ports with zero per-port timeout. | ||
| Raises: | ||
| test.fail if the packet is received on any of the specified ports. | ||
| """ | ||
| test.assertTrue( | ||
| len(ports) != 0, | ||
| "No port available to validate receiving packet on device %d, " % device_number, | ||
| ) | ||
| for device, port in ptf_ports(): | ||
| if device != device_number: | ||
| continue | ||
| 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) | ||
AntonHryshchuk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| logging.debug("Negative check for pkt on device %d, ports %s", device_number, ports) | ||
| start = time.monotonic() | ||
| while True: | ||
| remaining = timeout - (time.monotonic() - start) | ||
| if remaining <= 0: | ||
| return # PASS - packet not observed within timeout window | ||
|
|
||
| for device, port in ptf_ports(): | ||
| if device != device_number or port not in ports: | ||
| continue | ||
|
|
||
| result = dp_poll( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| test, | ||
| device_number=device_number, | ||
| port_number=port, | ||
| timeout=0, # non-blocking poll | ||
| exp_pkt=pkt, | ||
| ) | ||
|
|
||
| if isinstance(result, test.dataplane.PollSuccess): | ||
| test.fail( | ||
| "Unexpected packet received on device %d, port %d" | ||
| % (device_number, port) | ||
| ) | ||
|
|
||
| # Small sleep to avoid busy-looping and excessive CPU usage. | ||
| # Also gives dataplane threads time to enqueue incoming packets. | ||
| time.sleep(0.05) | ||
|
|
||
|
|
||
| def verify_packets_any( | ||
|
|
||
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.
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't be used with default_negative_timeout as before.
Now, it's a global timeout of the method.
Previously, it was the timeout for a single dp_poll call.
This value(0) is too small for the global timeout.