Dpdk: hotplug reliability, ready platform support, and nic selection fixes - #4662
Dpdk: hotplug reliability, ready platform support, and nic selection fixes#4662mcgov (mcgov) wants to merge 13 commits into
Conversation
7fc873f to
141df8b
Compare
141df8b to
a8c3ba7
Compare
There was a problem hiding this comment.
Pull request overview
This PR is the final part of a stacked DPDK SRIOV hotplug rework, focusing on making hotplug transitions deterministic (uevent-driven), improving NIC selection robustness (subnet-based rather than ordering), and tightening DPDK helper behavior for reliability across Azure and ReadyPlatform environments.
Changes:
- Replaces sleep-based hotplug orchestration with sysfs PCI remove/rescan plus uevent listener synchronization, and updates send/receive flows to use explicit subnet-based NIC selection.
- Refactors MTU configuration/validation and adds environment cleanup utilities to reset any leftover
uio_hv_genericbindings back tohv_netvsc. - Updates DPDK tooling behavior (testpmd include generation now considers PMD selection; devname app stops ports on exit).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| lisa/microsoft/testsuites/dpdk/dpdkutil.py | Adds uevent-driven PCI hotplug flow, subnet-based NIC selection, and netvsc rebind utilities; refactors MTU handling. |
| lisa/microsoft/testsuites/dpdk/dpdktestpmd.py | Extends testpmd command/include generation to be PMD-aware; improves logging and drop-rate thresholds. |
| lisa/microsoft/testsuites/dpdk/dpdksuite.py | Updates suite entrypoints to use new hotplug runner and resets bindings before tests; fixes renamed multiple-ports helper usage. |
| lisa/microsoft/testsuites/dpdk/devname/main.c | Stops started ports before EAL cleanup to avoid leaking active ports on exit. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| results = dict() | ||
| results[sender] = sender.testpmd.process_testpmd_output(sender_result.wait_result()) | ||
| results[sender] = sender.testpmd.process_testpmd_output(sender_proc.wait_result()) | ||
| results[receiver] = receiver.testpmd.process_testpmd_output( | ||
| receive_result.wait_result() | ||
| receiver_proc.wait_result() | ||
| ) |
a8c3ba7 to
4bd9967
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Suppressed comments (5)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:460
validate_mtu_size_for_nic_typecan crash when there are no SRIOV devices (UnboundLocalError onvalidated/vendor/info) or when an SRIOV PCI device can’t be mapped to a NIC (IndexError on[0]). This turns MTU tests into hard failures instead of clean skips.
# verify mtu is not too large for the NIC
# there should only be a single item in this list
ethdev = [
dev.lower for dev in node.nics.nics.values() if dev.pci_slot == nic.slot
][0]
lisa/microsoft/testsuites/dpdk/dpdkutil.py:712
get_vmbus_network_device_idswill currently fail the test when grep finds no matches (exit code 1) becauseexpected_exit_code=0is enforced. It also buildsfor i in <empty>; do ...whendevice_idsis empty, which is a shell syntax error. Both cases should return an empty list so callers can no-op safely (especially on non-Hyper-V/ReadyPlatform nodes).
device_ids = node.execute(
"grep -l f8615163-df3e-46c5-913f-f2d2f965ed0e "
"/sys/bus/vmbus/devices/*/class_id "
"| cut -f 1-6 -d / ",
shell=True,
lisa/microsoft/testsuites/dpdk/dpdkutil.py:751
rebind_uio_devices_to_hv_netvscignores itsdevicesparameter and re-queries uio_hv_generic devices itself. This makesreset_node_netvsc_bindings’s earlier query redundant and risks rebinding a different set than the caller intended.
def rebind_uio_devices_to_hv_netvsc(node: Node, devices: List[str]) -> None:
# unbind any uio_hv_generic devices and re-bind them to hv_netvsc
device_ids = get_vmbus_network_device_ids(node, filter_driver="uio_hv_generic")
if not device_ids:
# there were no devices bound to uio_hv_generic, so we can
return
lisa/microsoft/testsuites/dpdk/dpdkutil.py:843
- The hugepage minimum was increased to
8 * numa_nodesbut the rationale isn’t captured inline. This value controls whether tests skip due to memory pressure, so it should be documented per the project’s "magic numbers" guideline.
hugepages.init_hugepages(hugepage_size, minimum_gb=8 * numa_nodes)
lisa/microsoft/testsuites/dpdk/dpdktestpmd.py:865
check_rx_packet_dropsrelaxes the allowed RX drop rate from 1% to 20%/50% based on a core-count heuristic. This can allow severe regressions to pass unnoticed, and the core-count cutoff (192) is not a reliable proxy for isolation across SKUs. Consider keying this to an explicit feature/capability (e.g., IsolatedResource) or making the threshold configurable per test/VM type.
core_count = self.node.tools[Lscpu].get_core_count()
# tag of 'isolated resource' is being deprecated,
# so best effort to gauge this is going to be core count.
# we'll assume e192 or above is isolated,
# may need to adjust in the future.
| sample_rules_v4 += [ | ||
| f"R {ipv4_to_lpm(subnet_b_nics[receiver].ip_addr)} {dpdk_port_b}", | ||
| f"R {ipv4_to_lpm(subnet_a_nics[sender].ip_addr)} {dpdk_port_a}", | ||
| f"R {ipv4_to_lpm(subnet_b_nics[receiver].ip_addr)} {dpdk_port_a}", | ||
| f"R {ipv4_to_lpm(subnet_a_nics[sender].ip_addr)} {dpdk_port_b}", | ||
| ] |
4bd9967 to
12e18f0
Compare
12e18f0 to
d454088
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (8)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:456
validate_mtu_size_for_nic_type()usesvalidated/vendor/infoafter the loop, but they are only set inside the loop. Ifget_devices_by_type()returns an empty list (e.g. SRIOV disabled / no devices), this will raiseUnboundLocalErrorinstead of skipping cleanly. Also, the current logic overwritesvalidatedper-device rather than combining results across devices.
# then verify the MTU size is supported
for nic in node.tools[Lspci].get_devices_by_type(DEVICE_TYPE_SRIOV, force_run=True):
# verify mtu is not too large for the NIC
lisa/microsoft/testsuites/dpdk/dpdkutil.py:460
validate_mtu_size_for_nic_type()assumes every SRIOV PCI slot maps to a NIC entry and indexes[0], which can raiseIndexError(e.g. ifnode.nicsis stale or if a device has nolower). This should fail/skip with a clear message instead of crashing.
ethdev = [
dev.lower for dev in node.nics.nics.values() if dev.pci_slot == nic.slot
][0]
lisa/microsoft/testsuites/dpdk/dpdkutil.py:721
get_vmbus_network_device_ids()builds a shellfor i in ...; do ...; doneeven whendevice_idsis empty, which produces a shell syntax error and can breakreset_environment_netvsc_binding()on nodes where the grep finds no matching devices. Return early when nothing matches.
).stdout.splitlines()
drivers = node.execute(
f"for i in {' '.join(device_ids)}; do readlink -f $i/driver; done", shell=True
).stdout.splitlines()
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1857
- Same issue as IPv4 rules: IPv6 LPM rules route subnet_b to
dpdk_port_aand subnet_a todpdk_port_b, which looks reversed given howdpdk_port_a/dpdk_port_bare derived.
sample_rules_v6 += [
f"R {ipv4_to_ipv6_lpm(subnet_b_nics[receiver].ip_addr)} {dpdk_port_a}",
f"R {ipv4_to_ipv6_lpm(subnet_a_nics[sender].ip_addr)} {dpdk_port_b}",
]
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1068
verify_dpdk_send_receive()waits for testpmd processes with the defaultProcess.wait_result()timeout (600s). Since the processes have just been SIGINT/SIGKILLed viakill_previous_testpmd_command(), this can unnecessarily stall failures for up to 10 minutes. Use an explicit, tighter timeout based on the test duration.
results = dict()
results[sender] = sender.testpmd.process_testpmd_output(sender_proc.wait_result())
results[receiver] = receiver.testpmd.process_testpmd_output(
receiver_proc.wait_result()
)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1849
- In
create_l3fwd_rules_files(), the LPM rules map the receiver subnet (subnet_b) todpdk_port_aand the sender subnet (subnet_a) todpdk_port_b. Sincedpdk_port_a/dpdk_port_bare explicitly looked up from the forwarder NICs for subnet_a/subnet_b respectively, this appears reversed and would route traffic out the wrong DPDK port.
sample_rules_v4 += [
f"R {ipv4_to_lpm(subnet_b_nics[receiver].ip_addr)} {dpdk_port_a}",
f"R {ipv4_to_lpm(subnet_a_nics[sender].ip_addr)} {dpdk_port_b}",
]
lisa/microsoft/testsuites/dpdk/dpdkutil.py:498
validate_mtu_size_for_nic_type()currently overwritesvalidatedfor each NIC, so on multi-SRIOV-NIC VMs the last NIC decides whether the MTU is accepted. This can incorrectly pass validation when any earlier NIC is incompatible.
if "Mellanox" in vendor:
if "connect-x3" in info:
validated = mtu == 1500
else:
validated = mtu in [1400, 1500, 4000]
lisa/microsoft/testsuites/dpdk/dpdksuite.py:709
initialize_node_resources()already forcestestpmd.install(), which installs DPDK. Callingtestpmd.installer.do_installation()again here can trigger an unnecessary rebuild/reinstall (slow and costly), depending on installer behavior. Guard it to only run when not installed.
testpmd.installer.do_installation()
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (8)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:460
- validate_mtu_size_for_nic_type can raise IndexError/UnboundLocalError: the NIC->interface mapping uses
[0]even when no matching NIC exists, andvalidated/vendor/infomay be referenced after the loop if no SRIOV devices are returned. This can crash MTU tests instead of skipping with a clear reason.
# verify mtu is not too large for the NIC
# there should only be a single item in this list
ethdev = [
dev.lower for dev in node.nics.nics.values() if dev.pci_slot == nic.slot
][0]
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1849
- create_l3fwd_rules_files currently routes the receiver (subnet_b) prefix to dpdk_port_a and the sender (subnet_a) prefix to dpdk_port_b. Since dpdk_port_a is derived from subnet_a_nics[forwarder] and dpdk_port_b from subnet_b_nics[forwarder], this inverts the forwarding decision and will send traffic out the wrong port.
sample_rules_v4 += [
f"R {ipv4_to_lpm(subnet_b_nics[receiver].ip_addr)} {dpdk_port_a}",
f"R {ipv4_to_lpm(subnet_a_nics[sender].ip_addr)} {dpdk_port_b}",
]
lisa/microsoft/testsuites/dpdk/dpdkutil.py:843
- The new hugepages minimum (8 * numa_nodes) changes test behavior and can increase skip rate on smaller SKUs. Per repo guidelines, magic numbers that control test behavior should have an inline explanation so future maintainers know why this threshold is required.
hugepages.init_hugepages(hugepage_size, minimum_gb=8 * numa_nodes)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:750
- rebind_uio_devices_to_hv_netvsc takes a
devicesparameter but ignores it and re-scans sysfs again, and reset_node_netvsc_bindings reloads nics redundantly after rebind. Using the passed device list avoids extra shelling out and makes intent clearer.
def rebind_uio_devices_to_hv_netvsc(node: Node, devices: List[str]) -> None:
# unbind any uio_hv_generic devices and re-bind them to hv_netvsc
device_ids = get_vmbus_network_device_ids(node, filter_driver="uio_hv_generic")
if not device_ids:
# there were no devices bound to uio_hv_generic, so we can
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1494
- This test modifies node networking state (enables IP forwarding, changes routes, and brings interfaces down via reroute_traffic_and_disable_nic). Per LISA guidelines, nodes with network config changes should be marked dirty so they aren't reused in a potentially altered state if cleanup is incomplete.
# enable ip forwarding on secondary and tertiary nics
run_in_parallel(
[partial(__enable_ip_forwarding, node) for node in environment.nodes.list()]
)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:473
- Typo in comment: "restict" → "restrict".
# restict jumbo frame to supported sizes for that nic
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1049
- verify_dpdk_send_receive waits for "start packet forwarding" using Process.wait_output() with the default 300s timeout. On failures to start testpmd, this can stall the test much longer than intended. Consider using an explicit, shorter timeout (similar to testpmd_start_process) so failures surface quickly.
receiver_proc = receiver.node.execute_async(
kit_cmd_pairs[receiver],
sudo=True,
)
receiver_proc.wait_output("start packet forwarding")
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1473
- The architecture/Ubuntu version compatibility guard is commented out, but the suite requirement only constrains OS family (Ubuntu) and core/NIC count. If l3fwd truly requires x64 and Ubuntu >= 22.04, this should be enforced either here or via the TestCase requirement to avoid running in unsupported environments and producing noisy failures.
# if not (
# forwarder.tools[Lscpu].get_architecture() == CpuArchitecture.X64
# and isinstance(forwarder.os, Ubuntu)
# and forwarder.os.information.version >= "22.4.0"
# ):
578ab6c to
e3be5d8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (4)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:503
validate_mtu_size_for_nic_type()can raiseUnboundLocalErrorwhenget_devices_by_type()returns no SRIOV devices becausevalidatedis never assigned beforeif not validated:. This can happen on nodes without SRIOV (or when SRIOV is not active) while running MTU-related scenarios.
# if there is an unknown nic, we should skip the mtu tests until it's added
if not validated:
raise SkippedException(
f"This MTU test size ({mtu}) is not supported for this nic: {vendor, info}"
)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:721
get_vmbus_network_device_ids()builds a shellforloop fromdevice_ids. Whendevice_idsis empty, the command becomesfor i in ; do ...; done, which can fail and potentially raise fromnode.execute(). Add an early return when no matching vmbus network devices are present.
).stdout.splitlines()
drivers = node.execute(
f"for i in {' '.join(device_ids)}; do readlink -f $i/driver; done", shell=True
).stdout.splitlines()
lisa/microsoft/testsuites/dpdk/dpdkutil.py:751
rebind_uio_devices_to_hv_netvsc()takes adevicesparameter but ignores it and re-queries sysfs again. This makes the function harder to reason about and does redundant work; it should operate on the provided device list.
def rebind_uio_devices_to_hv_netvsc(node: Node, devices: List[str]) -> None:
# unbind any uio_hv_generic devices and re-bind them to hv_netvsc
device_ids = get_vmbus_network_device_ids(node, filter_driver="uio_hv_generic")
if not device_ids:
# there were no devices bound to uio_hv_generic, so we can
return
lisa/microsoft/testsuites/dpdk/dpdkutil.py:844
minimum_gb=8 * numa_nodeschanges a key test-sizing parameter but the rationale isn’t documented. Since this directly affects whether the test skips (NotEnoughMemory) and impacts cost/runtime, add an inline comment explaining why 8GB/NUMA is required (or make it a named constant).
# init and enable hugepages (required by dpdk)
hugepages = node.tools[Hugepages]
numa_nodes = node.tools[Lscpu].get_numa_node_count()
try:
hugepages.init_hugepages(hugepage_size, minimum_gb=8 * numa_nodes)
except NotEnoughMemoryException as err:
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (5)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:760
- rebind_uio_devices_to_hv_netvsc() takes a
devicesparameter but ignores it and re-derives the device list. This makes the API misleading and does redundant sysfs scans.
Use the passed-in devices list (the caller already computed it) so behavior matches the signature.
def rebind_uio_devices_to_hv_netvsc(node: Node, devices: List[str]) -> None:
# unbind any uio_hv_generic devices and re-bind them to hv_netvsc
device_ids = get_vmbus_network_device_ids(node, filter_driver="uio_hv_generic")
if not device_ids:
lisa/microsoft/testsuites/dpdk/dpdkutil.py:854
- The change to require
minimum_gb=8 * numa_nodesincreases memory requirements significantly but doesn’t explain why 8GB/NUMA is needed. This is a test-behavior “magic number”; please document the rationale so future maintainers can adjust it safely (or make it configurable via variables).
hugepages.init_hugepages(hugepage_size, minimum_gb=8 * numa_nodes)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1860
- The l3fwd LPM rules appear to route each destination subnet to the wrong DPDK port. Since dpdk_port_a is derived from subnet_a_nics[forwarder] and dpdk_port_b from subnet_b_nics[forwarder], traffic destined for subnet_b (the receiver side) should egress via dpdk_port_b (NIC_B), not dpdk_port_a.
sample_rules_v4 += [
f"R {ipv4_to_lpm(subnet_b_nics[receiver].ip_addr)} {dpdk_port_a}",
f"R {ipv4_to_lpm(subnet_a_nics[sender].ip_addr)} {dpdk_port_b}",
]
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1868
- Same as the IPv4 rules: the IPv6 LPM rules should map the receiver subnet to dpdk_port_b (subnet_b_nics[forwarder]) and the sender subnet to dpdk_port_a (subnet_a_nics[forwarder]).
sample_rules_v6 += [
f"R {ipv4_to_ipv6_lpm(subnet_b_nics[receiver].ip_addr)} {dpdk_port_a}",
f"R {ipv4_to_ipv6_lpm(subnet_a_nics[sender].ip_addr)} {dpdk_port_b}",
]
lisa/microsoft/testsuites/dpdk/dpdkutil.py:464
- In validate_mtu_size_for_nic_type(), the loop breaks on the first SRIOV PCI device that doesn’t map to a netdev (ethdevs empty). That prevents checking any remaining SRIOV NICs and can incorrectly skip MTU tests on nodes where the first lspci SRIOV entry isn’t associated with a Linux interface yet.
Use continue instead so the function evaluates other SRIOV devices before deciding validation failed.
dev.lower for dev in node.nics.nics.values() if dev.pci_slot == nic.slot
]
if not ethdevs:
break
ethdev = ethdevs[0]
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (8)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:465
- In validate_mtu_size_for_nic_type(), if the first SR-IOV PCI device has no matching ethdevs, the code breaks out of the loop. That prevents checking any remaining SR-IOV devices and can incorrectly skip MTU tests on multi-device systems. Use
continueso the loop can evaluate the next device instead of aborting validation early.
ethdevs = [
dev.lower for dev in node.nics.nics.values() if dev.pci_slot == nic.slot
]
if not ethdevs:
break
ethdev = ethdevs[0]
lisa/microsoft/testsuites/dpdk/dpdkutil.py:479
- Typo in comment: "restict" should be "restrict".
info = nic.device_info.lower()
vendor = nic.vendor
# restict jumbo frame to supported sizes for that nic
# I'm attempting to strike a balance between not breaking when a new nic
lisa/microsoft/testsuites/dpdk/dpdkutil.py:762
- rebind_uio_devices_to_hv_netvsc() takes a
devicesargument but ignores it and re-queries the device IDs. This is confusing and causes redundant sysfs scanning. Use the passed-in list as the source of truth (callers already queried it).
def rebind_uio_devices_to_hv_netvsc(node: Node, devices: List[str]) -> None:
# unbind any uio_hv_generic devices and re-bind them to hv_netvsc
device_ids = get_vmbus_network_device_ids(node, filter_driver="uio_hv_generic")
if not device_ids:
# there were no devices bound to uio_hv_generic, so we can
lisa/microsoft/testsuites/dpdk/dpdkutil.py:855
- The new
minimum_gb=8 * numa_nodesintroduces a behavior-affecting magic number. Add an inline comment explaining why 8 GiB per NUMA node is required so future changes don't accidentally regress stability or coverage.
hugepages.init_hugepages(hugepage_size, minimum_gb=8 * numa_nodes)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1080
- verify_dpdk_send_receive() now waits for testpmd processes to exit with the default wait_result timeout (600s). If testpmd hangs after the SIGINT/SIGKILL sequence, this can stall the test run for up to 10 minutes. Pass an explicit timeout derived from test_duration so failures surface promptly.
sleep(5)
results = dict()
results[sender] = sender.testpmd.process_testpmd_output(sender_proc.wait_result())
results[receiver] = receiver.testpmd.process_testpmd_output(
receiver_proc.wait_result()
)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:290
- The fixed
sleep(30)controls test behavior but doesn’t document why 30s is sufficient. Add an inline comment (or make it configurable) to clarify what this delay is intended to cover (baseline forwarding/stats collection).
This issue also appears in the following locations of the same file:
- line 460
- line 476
sleep(30)
lisa/microsoft/testsuites/dpdk/dpdktestpmd.py:870
- This change increases the allowed RX drop rate from 1% to 20%/50% based on core count, which materially reduces test sensitivity. Please justify these new thresholds (or make them configurable) so we don’t mask real regressions.
if core_count >= 192:
allowable_drop_rate = 0.2
else:
allowable_drop_rate = 0.5
self.packet_drop_rate = self.rx_packet_drops / self.rx_total_packets
lisa/microsoft/testsuites/dpdk/dpdksuite.py:709
- initialize_node_resources() already calls testpmd.install(). Calling testpmd.installer.do_installation() again here is redundant and can add significant time to the ring_ping test (especially for source builds).
testpmd.installer.do_installation()
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (5)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:851
- The new
minimum_gb=8 * numa_nodesintroduces a magic constant that materially changes hugepage reservation behavior, but there's no inline explanation of why 8GB/NUMA is required. Add a brief comment (or named constant) so future maintainers understand the rationale and can tune it safely.
hugepages.init_hugepages(hugepage_size, minimum_gb=8 * numa_nodes)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1857
- In the L3FWD LPM rules, the destination subnet should map to the egress port on the same subnet. With
dpdk_port_abound to subnet A anddpdk_port_bbound to subnet B (as computed just above), routingsubnet_btraffic todpdk_port_awill send packets out the wrong port.
sample_rules_v4 += [
f"R {ipv4_to_lpm(subnet_b_nics[receiver].ip_addr)} {dpdk_port_a}",
f"R {ipv4_to_lpm(subnet_a_nics[sender].ip_addr)} {dpdk_port_b}",
]
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1865
- Same issue for the IPv6 placeholder LPM rules:
subnet_bshould map todpdk_port_bandsubnet_atodpdk_port_ato keep egress ports consistent with the destination subnets.
sample_rules_v6 += [
f"R {ipv4_to_ipv6_lpm(subnet_b_nics[receiver].ip_addr)} {dpdk_port_a}",
f"R {ipv4_to_ipv6_lpm(subnet_a_nics[sender].ip_addr)} {dpdk_port_b}",
]
lisa/microsoft/testsuites/dpdk/dpdkutil.py:464
validate_mtu_size_for_nic_typestops scanning SRIOV PCI devices when the first device doesn't map to an interface (break). If there are multiple SRIOV devices, this can incorrectly skip MTU validation for later devices and cause false SkippedException results. Usecontinueso all candidate devices are checked.
ethdevs = [
dev.lower for dev in node.nics.nics.values() if dev.pci_slot == nic.slot
]
if not ethdevs:
break
lisa/microsoft/testsuites/dpdk/dpdksuite.py:709
initialize_node_resources()already installs DPDK viatestpmd.install(). Callingtestpmd.installer.do_installation()again here is redundant and can significantly slow the test by rebuilding/reinstalling unnecessarily. Prefer the tool-levelinstall()(idempotent) if you need to re-assert installation.
testpmd.installer.do_installation()
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (6)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1865
- The L3FWD route rules appear to forward each subnet to the wrong DPDK port.
dpdk_port_ais derived from the forwarder NIC on subnet 10.0.1.0/24 anddpdk_port_bfrom 10.0.2.0/24, so traffic destined for subnet B should egress on port B (and vice versa). As written, packets for the receiver subnet are sent outdpdk_port_a, which likely breaks forwarding.
sample_rules_v4 += [
f"R {ipv4_to_lpm(subnet_b_nics[receiver].ip_addr)} {dpdk_port_a}",
f"R {ipv4_to_lpm(subnet_a_nics[sender].ip_addr)} {dpdk_port_b}",
]
# Need to map ipv4 to ipv6 addresses, unused but the rules must be
# provided. A valid ipv6 address needs to be in the ipv6 rules, but
# ipv6 is not enabled in azure.
sample_rules_v6 += [
f"R {ipv4_to_ipv6_lpm(subnet_b_nics[receiver].ip_addr)} {dpdk_port_a}",
f"R {ipv4_to_ipv6_lpm(subnet_a_nics[sender].ip_addr)} {dpdk_port_b}",
]
lisa/microsoft/testsuites/dpdk/dpdkutil.py:852
minimum_gbwas increased from4 * numa_nodesto8 * numa_nodeswithout any inline justification. This is a magic-number change that can materially increase skip rates on smaller VMs and should be documented inline so future changes/tests understand why 8GiB/NUMA is required.
# init and enable hugepages (required by dpdk)
hugepages = node.tools[Hugepages]
numa_nodes = node.tools[Lscpu].get_numa_node_count()
try:
hugepages.init_hugepages(hugepage_size, minimum_gb=8 * numa_nodes)
except NotEnoughMemoryException as err:
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1076
verify_dpdk_send_receive()starts testpmd asynchronously, then callswait_result()without an explicit timeout.Process.wait_result()defaults to 600s, so if testpmd doesn’t terminate cleanly after SIGINT/SIGKILL, this can stall the test for up to 10 minutes. It’s safer to bound the wait based ontest_durationplus a small buffer (the old implementation had tighter timeouts).
receiver_proc = receiver.node.execute_async(
kit_cmd_pairs[receiver],
sudo=True,
)
receiver_proc.wait_output("start packet forwarding")
sender_proc = sender.node.execute_async(
kit_cmd_pairs[sender],
sudo=True,
)
sender_proc.wait_output("start packet forwarding")
sleep(test_duration)
sender.testpmd.kill_previous_testpmd_command()
receiver.testpmd.kill_previous_testpmd_command()
sleep(5)
results = dict()
results[sender] = sender.testpmd.process_testpmd_output(sender_proc.wait_result())
results[receiver] = receiver.testpmd.process_testpmd_output(
receiver_proc.wait_result()
)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:465
validate_mtu_size_for_nic_type()breaks out of the SR-IOV PCI scan when it encounters a device whose PCI slot doesn't currently map to anynode.nicsentry. On multi-VF setups (or during hotplug) this can happen transiently and will cause the function to incorrectly skip MTU tests even if other VFs are valid.
for nic in node.tools[Lspci].get_devices_by_type(DEVICE_TYPE_SRIOV, force_run=True):
# verify mtu is not too large for the NIC
# there should only be a single item in this list
ethdevs = [
dev.lower for dev in node.nics.nics.values() if dev.pci_slot == nic.slot
]
if not ethdevs:
break
ethdev = ethdevs[0]
lisa/microsoft/testsuites/dpdk/dpdkutil.py:296
run_testpmd_hotplug()hardcodessleep(30)as the total run time. Previously the hotplug tests ran testpmd forDPDK_VF_REMOVAL_MAX_TEST_TIME(10 minutes). This is a significant reduction in stress/coverage; if intentional, it should be configurable (e.g., parameter/variable-driven) and/or justified in code.
# run the send/receive hotplug test.
def run_testpmd_hotplug(
kit_cmd_pairs: Dict[DpdkTestResources, str],
sender: DpdkTestResources,
receiver: Optional[DpdkTestResources] = None,
hotplug: bool = True,
) -> None:
processes: Dict[DpdkTestResources, Process] = {}
collect_from = receiver if receiver else sender
all_kits = [sender]
if receiver:
all_kits += [receiver]
processes[receiver] = testpmd_start_process(receiver, kit_cmd_pairs[receiver])
processes[sender] = testpmd_start_process(sender, kit_cmd_pairs[sender])
if hotplug:
node = collect_from.node
# gather the VF pci slot up front, the uevent match criteria are
# built from it. The slot is stable across a remove/rescan cycle.
test_nic = node.nics.get_nic_by_subnet("10.0.1.0/24")
switch_sriov_for_nic(node, test_nic)
# let it run for a bit
sleep(30)
# kill testpmd and process the output
for kit in all_kits:
kit.testpmd.kill_previous_testpmd_command()
# allow time for SIGINT/SIGKILL shutdown and stats flush
kit.testpmd.process_testpmd_output(processes[kit].wait_result(timeout=120))
lisa/microsoft/testsuites/dpdk/dpdktestpmd.py:1026
DpdkTestpmd._install()is reaching into the installer’s private API (_check_if_installed). This makes the tool more brittle to refactors and can skip installer setup steps.Installer.do_installation()already decides whether work is needed, so it can be called directly.
if not self.installer._check_if_installed():
self.installer.do_installation()
Add a Phase 1 boot validation case for the Microsoft.AKS Compute.AKS.Linux.Billing extension. Reuse the shared VM extension boot flow to validate provisioning, exact patch version, VM reachability, and cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4eb9bc67-81fc-4158-96dc-c3b3f4ec2195
* test(vm_extensions): add AKS Linux AKSNode boot validation Add Phase 1 package and handler lifecycle coverage for the Microsoft.AKS Compute.AKS.Linux.AKSNode extension. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4eb9bc67-81fc-4158-96dc-c3b3f4ec2195 * refactor(vm_extensions): move AKS extension tests Place the AKSNode and Billing extension suites at the VM extension suite root because they are not owned by the runtime extensions team. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4eb9bc67-81fc-4158-96dc-c3b3f4ec2195 --------- Copilot-Session: 4eb9bc67-81fc-4158-96dc-c3b3f4ec2195
…fixes Rework the SRIOV hot plug tests around the uevent listener so they wait for the kernel events that mark a VF being removed and re-added instead of sleeping and hoping the device settled. run_testpmd_concurrent is replaced by run_testpmd_hotplug, which drives the disable and enable cycle, watches for the matching uevents on each node, and collects the pps data around the transition. Tests now pick their nics explicitly by test subnet rather than taking the secondary nic, and reset any leftover uio bindings back to hv_netvsc before starting, so a node reused after a failed hot plug run is usable. This also makes the suite work on ready platform nodes, where nic ordering is not guaranteed. Also fix the verify_dpdk_mutliple_ports spelling and stop the devname example app leaking started ports on exit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d5f58ad-b9df-4420-ad37-22caee78e925
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
apt-get apparently doesn't raise a failure code if it can't update only one or two repos. The apt mirror has been flaky recently, so it surfaced this bug in our logic. This change checks for warnings about not being able to connect and retries the apt-get update a few times. It raises a new exception LisaRetryableException to indicate that the test issue was due to a retryable operation that just timed out. This should avoid confusion about LisaExceptions being raised when the issue is just an infrastructure issue unrelated to the test case.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (5)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1860
- The l3fwd LPM rules appear to route destination subnet_b traffic to dpdk_port_a and subnet_a traffic to dpdk_port_b, which is the opposite of the intended NIC mapping (dpdk_port_a is derived from subnet_a_nics[forwarder] and dpdk_port_b from subnet_b_nics[forwarder]). This would send packets out the wrong port and likely break forwarding.
sample_rules_v4 += [
f"R {ipv4_to_lpm(subnet_b_nics[receiver].ip_addr)} {dpdk_port_a}",
f"R {ipv4_to_lpm(subnet_a_nics[sender].ip_addr)} {dpdk_port_b}",
]
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1868
- Same issue as IPv4 rules above: the IPv6 LPM rules route subnet_b to dpdk_port_a and subnet_a to dpdk_port_b, which appears reversed relative to the dpdk_port_a/subnet_a and dpdk_port_b/subnet_b mapping.
sample_rules_v6 += [
f"R {ipv4_to_ipv6_lpm(subnet_b_nics[receiver].ip_addr)} {dpdk_port_a}",
f"R {ipv4_to_ipv6_lpm(subnet_a_nics[sender].ip_addr)} {dpdk_port_b}",
]
lisa/util/init.py:388
- LisaRetryableException doesn't call LisaException/Exception.init, so the exception's args are empty and any code that relies on the base exception message/args (or masking behavior in LisaException.init) won't see the message. It should pass the message to super().init.
def __init__(self, operation_type: str = "") -> None:
self.message = f"{operation_type} failed but can be retried."
def __str__(self) -> str:
return self.message
lisa/microsoft/testsuites/dpdk/dpdkutil.py:478
- Typo in comment: "restict" -> "restrict".
# restict jumbo frame to supported sizes for that nic
lisa/microsoft/testsuites/dpdk/dpdkutil.py:290
- run_testpmd_hotplug hard-codes the run duration (sleep(30)) rather than taking a duration parameter (previously callers passed DPDK_VF_REMOVAL_MAX_TEST_TIME=10min). This makes the test runtime/coverage effectively fixed and ignores the existing configurability/constant.
# let it run for a bit
sleep(30)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (3)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:854
- minimum_gb was changed from 4numa_nodes to 8numa_nodes without any inline justification or configuration knob. Per LISA test guidelines, magic numbers that materially affect test behavior (resource requirements / skip rate) should be explained or made configurable.
hugepages.init_hugepages(hugepage_size, minimum_gb=8 * numa_nodes)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:464
- validate_mtu_size_for_nic_type() breaks out of the SR-IOV device scan when a PCI slot has no matching netdev (ethdevs is empty). That can cause a false SkippedException even if later SR-IOV devices in the list do have a netdev and support the requested MTU. This should continue scanning instead of aborting early.
if not ethdevs:
break
lisa/microsoft/testsuites/dpdk/dpdkutil.py:722
- get_vmbus_network_device_ids() introduces hardcoded POSIX paths and a shell pipeline. LISA test guidelines prefer using node.get_pure_path() for path composition and avoiding hardcoded separators/pipelines where possible for portability and maintainability.
result = node.execute(
"grep -l f8615163-df3e-46c5-913f-f2d2f965ed0e "
"/sys/bus/vmbus/devices/*/class_id "
"| cut -f 1-6 -d / ",
shell=True,
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (6)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:855
- Uncommented magic number:
minimum_gb=8 * numa_nodesmaterially changes test resource requirements and can increase skip rate on smaller SKUs. Per repo guidelines, test-behavior magic numbers should have an inline rationale (and ideally be configurable viavariables).
numa_nodes = node.tools[Lscpu].get_numa_node_count()
try:
hugepages.init_hugepages(hugepage_size, minimum_gb=8 * numa_nodes)
except NotEnoughMemoryException as err:
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1860
- The IPv4 LPM rules map destination subnets to the wrong DPDK port IDs.
dpdk_port_ais derived fromsubnet_a_nics[forwarder]anddpdk_port_bfromsubnet_b_nics[forwarder](see above), so traffic destined for subnet B should egress via port B, not port A (and vice versa).
sample_rules_v4 += [
f"R {ipv4_to_lpm(subnet_b_nics[receiver].ip_addr)} {dpdk_port_a}",
f"R {ipv4_to_lpm(subnet_a_nics[sender].ip_addr)} {dpdk_port_b}",
]
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1868
- Same issue as IPv4 rules: the IPv6 LPM rules currently route subnet B destinations to
dpdk_port_aand subnet A destinations todpdk_port_b, which conflicts with howdpdk_port_a/dpdk_port_bare assigned from the forwarder NICs.
sample_rules_v6 += [
f"R {ipv4_to_ipv6_lpm(subnet_b_nics[receiver].ip_addr)} {dpdk_port_a}",
f"R {ipv4_to_ipv6_lpm(subnet_a_nics[sender].ip_addr)} {dpdk_port_b}",
]
lisa/util/init.py:382
- The docstring says to “add it to the retry decorator filter”, but
retry_without_exceptionsusesskipped_exceptionsas the filter for non-retriable exceptions. Adding this exception there would prevent retries, which is the opposite of what this exception is for.
class LisaRetryableException(LisaException):
"""
This exception is used to indicate that an operation should be retried.
Connection issues, http retryable errors, etc.
Raise this exception and add it to the retry decorator filter.
"""
lisa/microsoft/testsuites/dpdk/dpdkutil.py:465
validate_mtu_size_for_nic_typebreaks out of the loop when it can’t map the current SRIOV PCI device to an ethdev (if not ethdevs: break). This can incorrectly skip checking remaining SRIOV devices and then fail validation with missing vendor/info.
for nic in node.tools[Lspci].get_devices_by_type(DEVICE_TYPE_SRIOV, force_run=True):
# verify mtu is not too large for the NIC
# there should only be a single item in this list
ethdevs = [
dev.lower for dev in node.nics.nics.values() if dev.pci_slot == nic.slot
]
if not ethdevs:
break
ethdev = ethdevs[0]
lisa/microsoft/testsuites/dpdk/dpdkutil.py:377
- When
set_mtuis enabled,maxmtu_intfalls back to 0 ifip linkdoesn’t reportmaxmtu. That value is then used as--mbuf-size=0ingenerate_testpmd_command, which is very likely invalid and turns a “can’t validate MTU” situation into a harder-to-debug runtime failure.
check_nic = snd_nic.lower if snd_nic.lower else snd_nic.name
maxmtu = sender.node.tools[Ip].get_detail(check_nic, "maxmtu")
maxmtu_int = int(maxmtu) if maxmtu else 0
…ore tries but adding backoff.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (7)
lisa/microsoft/testsuites/dpdk/dpdkutil.py:855
- hugepages.init_hugepages now hard-codes 8 GiB per NUMA node (previously 4). This is a test-behavior tuning constant; add an inline comment explaining why 8 is required (e.g. DPDK hotplug stability / memory needs) to avoid it becoming an unexplained magic number.
numa_nodes = node.tools[Lscpu].get_numa_node_count()
try:
hugepages.init_hugepages(hugepage_size, minimum_gb=8 * numa_nodes)
except NotEnoughMemoryException as err:
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1860
- The L3FWD LPM rules now route traffic for subnet_b/receiver out of dpdk_port_a and subnet_a/sender out of dpdk_port_b, which is the opposite of the port naming established earlier (dpdk_port_a is derived from subnet_a NIC MAC, dpdk_port_b from subnet_b NIC MAC). This will forward packets to the wrong port and break connectivity. The same swap is needed in the IPv6 rules below.
sample_rules_v4 += [
f"R {ipv4_to_lpm(subnet_b_nics[receiver].ip_addr)} {dpdk_port_a}",
f"R {ipv4_to_lpm(subnet_a_nics[sender].ip_addr)} {dpdk_port_b}",
]
lisa/microsoft/testsuites/dpdk/dpdkutil.py:1073
- verify_dpdk_send_receive uses time-based sleeps (sleep(test_duration) and sleep(5)) to control testpmd runtime and shutdown. Fixed sleeps are discouraged in LISA tests because they make runs flaky/slow and can hide hangs until the default 600s Process.wait_result timeout. Prefer a bounded wait/timeout helper that sends SIGINT and enforces a predictable max runtime (e.g. the existing Timeout.start_with_timeout pattern used previously).
receiver_proc = receiver.node.execute_async(
kit_cmd_pairs[receiver],
sudo=True,
)
receiver_proc.wait_output("start packet forwarding")
sender_proc = sender.node.execute_async(
kit_cmd_pairs[sender],
sudo=True,
)
sender_proc.wait_output("start packet forwarding")
sleep(test_duration)
sender.testpmd.kill_previous_testpmd_command()
receiver.testpmd.kill_previous_testpmd_command()
sleep(5)
lisa/util/init.py:388
- LisaRetryableException doesn't call the LisaException/Exception constructor, so the message isn't stored in Exception args (and won't get secret-masked by LisaException). This can lead to inconsistent logging/serialization compared to other LisaException types.
def __init__(self, operation_type: str = "") -> None:
self.message = f"{operation_type} failed but can be retried."
def __str__(self) -> str:
return self.message
lisa/microsoft/testsuites/dpdk/dpdkutil.py:465
- validate_mtu_size_for_nic_type stops scanning on the first SRIOV PCI device that doesn't map to a NIC in node.nics (it uses
break). That can incorrectly skip MTU validation on nodes where the first probed PCI device isn't the one we care about, causing false SkippedExceptions.
for nic in node.tools[Lspci].get_devices_by_type(DEVICE_TYPE_SRIOV, force_run=True):
# verify mtu is not too large for the NIC
# there should only be a single item in this list
ethdevs = [
dev.lower for dev in node.nics.nics.values() if dev.pci_slot == nic.slot
]
if not ethdevs:
break
ethdev = ethdevs[0]
lisa/microsoft/testsuites/dpdk/dpdksuite.py:709
- verify_dpdk_ring_ping calls initialize_node_resources(), which already installs DPDK via testpmd.install(), but then unconditionally calls testpmd.installer.do_installation() again. This can trigger redundant rebuild/reinstall and slow down the test; guard it so it only runs when not already installed.
if isinstance(testpmd.installer, PackageManagerInstall):
# The Testpmd tool doesn't get re-initialized
# even if you invoke it with new arguments.
raise SkippedException(
"DPDK ring_ping test is not implemented for "
" package manager installation."
)
testpmd.installer.do_installation()
# grab a nic and run testpmd
lisa/microsoft/testsuites/dpdk/dpdkutil.py:676
- set_mtu_for_nics uses assert_success=False for nic.name but defaults to assert_success=True for nic.lower. If nic.lower isn't present on the guest (or temporarily missing during hotplug), this will raise and fail the test even though the first call intentionally tolerates missing devices.
validate_mtu_size_for_nic_type(node=node, mtu=mtu)
ip_tool = node.tools[Ip]
for nic in nics:
ip_tool.set_mtu(nic.name, mtu, assert_success=False)
if nic.lower:
ip_tool.set_mtu(nic.lower, mtu)
node.log.debug(f"Set MTU to {mtu} for interface {nic.name}")
Part 9 of 9 of a stacked series that reworks the DPDK SRIOV hot plug tests. Stacked on #4661, review only the last commit.
run_testpmd_concurrentis replaced byrun_testpmd_hotplug, which drives the disable and enable cycle, watches for the matching uevents on each node, and collects the pps data around the transition.verify_dpdk_mutliple_portsspelling and stop the devname example app leaking started ports on exit.Key Test Cases:
verify_dpdk_sriov_rescind_failover_send_only|verify_dpdk_sriov_rescind_failover_send_receive|verify_dpdk_multiple_ports_netvsc|verify_dpdk_send_receive_netvsc|verify_dpdk_build_netvsc
Impacted LISA Features:
Sriov, NetworkInterface, IsolatedResource, SerialConsole
Tested Azure Marketplace Images:
canonical 0001-com-ubuntu-server-jammy 22_04-lts latestcanonical ubuntu-24_04-lts server latestmicrosoftcblmariner azure-linux-3 azure-linux-3 latestredhat rhel 9_5 latest