Fix pktgen tx test netdst conversion, mac address and install issue #6765
Conversation
WalkthroughMake VM startup conditional in a pktgen burst-mode test: call Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@provider/virtual_network/pktgen_utils.py`:
- Line 44: The code calls libvirt.get_ifname_host(...) inside configure_pktgen
(when pkt_cate == "tx") but libvirt is not imported; add the missing import
alongside the other virttest imports at the top of the file, e.g. add "from
virttest import libvirt" (or the existing project import style for
virttest.libvirt) so that libvirt.get_ifname_host and any other libvirt
references resolve at runtime.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libvirt/tests/src/virtual_network/qemu/pktgen_burst_mode_test.pyprovider/virtual_network/pktgen_utils.py
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: qiankehan
Repo: autotest/tp-libvirt PR: 6652
File: libvirt/tests/cfg/virtual_network/passt/passt_function.cfg:106-106
Timestamp: 2025-11-24T10:44:47.801Z
Learning: In libvirt/tests/cfg/virtual_network/passt/passt_function.cfg, passt test cases have only one interface, so running `dhcpcd` without specifying an interface is safe and doesn't cause network disruption.
📚 Learning: 2025-11-24T10:44:47.801Z
Learnt from: qiankehan
Repo: autotest/tp-libvirt PR: 6652
File: libvirt/tests/cfg/virtual_network/passt/passt_function.cfg:106-106
Timestamp: 2025-11-24T10:44:47.801Z
Learning: In libvirt/tests/cfg/virtual_network/passt/passt_function.cfg, passt test cases have only one interface, so running `dhcpcd` without specifying an interface is safe and doesn't cause network disruption.
Applied to files:
provider/virtual_network/pktgen_utils.pylibvirt/tests/src/virtual_network/qemu/pktgen_burst_mode_test.py
📚 Learning: 2025-11-18T08:47:14.465Z
Learnt from: smitterl
Repo: autotest/tp-libvirt PR: 6072
File: libvirt/tests/src/passthrough/pci/libvirt_pci_passthrough_hotplug.py:239-241
Timestamp: 2025-11-18T08:47:14.465Z
Learning: In tp-libvirt tests, `utils_test.ping` is a valid function that exists in the avocado-vt framework (virttest.utils_test module). It is an alias to `utils_net.ping` and supports parameters including dest, count, interface, timeout, and session.
Applied to files:
libvirt/tests/src/virtual_network/qemu/pktgen_burst_mode_test.py
🪛 Ruff (0.14.11)
provider/virtual_network/pktgen_utils.py
44-44: Undefined name libvirt
(F821)
🔇 Additional comments (2)
libvirt/tests/src/virtual_network/qemu/pktgen_burst_mode_test.py (2)
64-66: LGTM!Good defensive check to ensure the VM is running before proceeding with the test. The flow correctly handles the case where the VM might not be alive by starting it and then verifying.
101-104: LGTM!Good approach to handle the "private" netdst case. Creating a copy of params avoids side effects on the original configuration, and the fallback to "atbr0" provides a sensible default.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
quanwenli
changed the title
|
@nanli1 could you help review this patch? thanks a lot. |
quanwenli
force-pushed
the
pktgen
branch
2 times, most recently
from
61e7385 to
c19bc0e
Compare
| pktgen_utils.install_package(host_ver) | ||
|
|
||
| test.log.debug("TEST_STEP 2: Test with guest and host connectivity") | ||
| vm.start() |
There was a problem hiding this comment.
The latest code already removed this step, please according to the latest code to update your patch.
There was a problem hiding this comment.
pktgen_burst_mode_test.py file was modified after I submitted the pr, I have remove it.
| vm.copy_files_to(local_path, remote_path) | ||
| utils_misc.cmd_status_output(cmd_install, session=session_serial) | ||
| utils_misc.cmd_status_output(cmd_clean, session=session_serial) | ||
| utils_misc.cmd_status_output(cmd_install, session=session_serial) |
There was a problem hiding this comment.
But according to the code design session_serial will not be none. VM can not copy file if it is None, because the package download to host, you will copy the package via vm.copy_files_to(local_path, remote_path). Therefore, if this section code have a problem, you need to consider re-design this part.
There was a problem hiding this comment.
This is required because the host-side pktgen/rx workflow needs the module on host, while guest-side pktgen needs it in guest. otherwise pktgen_utils.install_package(host_ver) will fail. please check issue LIBVIRTAT-22256
1. Fix netdst conversion: convert 'private' to 'atbr0' for host IP lookup 2. Fix mac address: use tap device mac instead of default gateway mac 3. Fix pktgen package installation on host when session_serial is none Signed-off-by: Wenli Quan <[email protected]>
quanwenli
changed the title
|
We need to add test result for failed reason before fix, and case result after this patch |
|
The failed results were in the LIBVIRTAT-22252, LIBVIRTAT-22254, LIBVIRTAT-22256 The passed result: avocado run --vt-type libvirt --vt-machine-type q35 --vt-guest-os RHEL.9.7 --vt-extra-params os_variant=rhel-unknown --vt-extra-params "$(cat /root/extra_params_pktgen.cfg)" virtual_network.qemu_test.pktgen_burst_mode_testJOB ID : a6433bc954e6b86d7fc638d46760570b839b288a |
3 participants
ID: LIBVIRTAT-22252, LIBVIRTAT-22254, LIBVIRTAT-22256
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.