[PWCI] "[v3] app/testpmd: fail on shared rxq switch mismatch" #507
Conversation
Starting with NVIDIA ConnectX-9, the future devices will support only hardware steering (HWS) flow engine. The software steering options (legacy Verbs and Direct Verbs) have lower performances, and won't be available for new devices. Both flow APIs (sync and async template) will still be supported with the hardware steering flow engine. Fixes: 1b55eeb ("common/mlx5: add ConnectX-9 SuperNIC") Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
The tool ripgrep allows to find files not ending with a line break: rg -Ul '[^\n]\z' The files with a trailing blank lines are shown with this command: rg -Ul '\n\n\z' Files are fixed to end with a single line break. Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Shared Rx queues do not work with every combination of ports. Besides requiring the corresponding device capability, shared Rx queues also require all ports of one share group and queue ID have the same switch domain and Rx domain. When these fields do not match, shared Rx queues are not properly set up and queue sharing may fail silently. This can happen even in some less intuitive cases like multiple VFs of one physical NIC and may cause packets not to be read by the application, depending on how the data path is implemented. To help with debugging issues with shared Rx queue configuration when using testpmd, this commit introduces simple checks and error messages for members of a share_group and share_qid to fail whenever there is a mismatch in the switch and Rx domain. Signed-off-by: Adrian Schollmeyer <a.schollmeyer@syseleven.de> Signed-off-by: 0-day Robot <robot@bytheb.org>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
WalkthroughRelease version 25.11.0 finalized with trailing blank line cleanup across driver files, test data formatting standardization, RX queue validation logic added to testpmd, documentation updates clarifying device support, array declaration corrections in Bnxt templates, and removal of PFC hardware configuration call in Txgbe. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
doc/guides/nics/mlx5.rst (1)
692-703: Clarify dv_flow_en defaults for ConnectX-9 and fix HWS grammarThe documentation currently leaves the default
dv_flow_enbehavior for ConnectX-9+ unclear. Lines 701–702 state that the default is 1 (DV flow steering) or 0 (legacy Verbs), but both values 0 and 1 are documented as available only for devices older than ConnectX-9. For ConnectX-9 and later, value 2 (WQE-based hardware steering) is the only supported mode, yet the default statement doesn't mention what the default actually is on these newer devices. Explicitly stating thatdv_flow_en=2is the default for ConnectX-9+ will remove this ambiguity.Additionally, at line 1344, change "the only mode supported on device ConnectX-9 and later" to "devices ConnectX-9 and later" for proper grammar.
🧹 Nitpick comments (1)
app/test-pmd/testpmd.c (1)
2678-2718: Shared RXQ switch-domain check is sound; consider small refinementsThe new guard correctly detects cases where a queue in the same
share_group/share_qiduses a differentswitch_info.domain_idorswitch_info.rx_domainand fails early with a clear error, which aligns with the intent of rejecting invalid shared-RXQ topologies.Two optional tweaks you might consider:
- Once a mismatch is detected, break out of the outer
RTE_ETH_FOREACH_DEVloop as well to avoid scanning remaining ports unnecessarily.- If there are devices that expose
share_group > 0but have no meaningful switch info (e.g., invalid/uninitializeddomain_id/rx_domain), it may be safer to either reject shared-RXQ for those devices up front or explicitly skip them here, to avoid surprising “switch domain” errors coming from partially populateddev_info.Functionally this looks correct as-is; these are mainly for efficiency and robustness in edge configurations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
doc/guides/contributing/img/abi_stability_policy.svgis excluded by!**/*.svgdoc/guides/contributing/img/what_is_an_abi.svgis excluded by!**/*.svgdoc/guides/howto/img/flow_bifurcation_overview.svgis excluded by!**/*.svgdoc/guides/howto/img/virtio_user_for_container_networking.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/bond-mode-0.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/bond-mode-1.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/bond-mode-2.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/bond-mode-3.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/bond-mode-4.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/bond-mode-5.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/lcore_var_mem_layout.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/pdcp_functional_overview.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/predictable_snat_1.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/predictable_snat_2.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/rss_queue_assign.svgis excluded by!**/*.svgdoc/guides/prog_guide/img/static_array_mem_layout.svgis excluded by!**/*.svgdoc/guides/sample_app_ug/img/overlay_networking.svgis excluded by!**/*.svg
📒 Files selected for processing (50)
VERSION(1 hunks)app/test-bbdev/test_vectors/bbdev_null.data(1 hunks)app/test-bbdev/test_vectors/turbo_dec_c1_k6144_r0_e10376_crc24b_sbd_negllr_high_snr.data(1 hunks)app/test-bbdev/test_vectors/turbo_dec_c1_k6144_r0_e10376_crc24b_sbd_negllr_low_snr.data(1 hunks)app/test-bbdev/test_vectors/turbo_dec_c1_k6144_r0_e34560_sbd_posllr.data(1 hunks)app/test-bbdev/test_vectors/turbo_dec_c2_k3136_r0_e4920_sbd_negllr_crc24b.data(1 hunks)app/test-bbdev/test_vectors/turbo_enc_c1_k6144_r0_e18448_crc24a.data(1 hunks)app/test-bbdev/test_vectors/turbo_enc_c3_k4800_r2_e14412_crc24b.data(1 hunks)app/test-pmd/testpmd.c(1 hunks)doc/guides/nics/cxgbe.rst(0 hunks)doc/guides/nics/ipn3ke.rst(0 hunks)doc/guides/nics/mlx5.rst(2 hunks)drivers/event/dlb2/dlb2_iface.c(0 hunks)drivers/net/bnxt/tf_core/tf_em_common.c(0 hunks)drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_act.c(0 hunks)drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_class.c(0 hunks)drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_defs.h(0 hunks)drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_tbl.c(0 hunks)drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_thor2_class.c(0 hunks)drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_thor_class.c(0 hunks)drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_wh_plus_class.c(0 hunks)drivers/net/gve/base/gve_adminq.c(0 hunks)drivers/net/gve/gve_rss.c(0 hunks)drivers/net/gve/gve_rss.h(0 hunks)drivers/net/hinic/base/hinic_pmd_mbox.c(0 hunks)drivers/net/hinic/base/hinic_pmd_niccfg.c(0 hunks)drivers/net/hinic/base/meson.build(0 hunks)drivers/net/intel/e1000/base/e1000_82575.c(0 hunks)drivers/net/intel/e1000/base/e1000_mbx.c(0 hunks)drivers/net/intel/e1000/base/e1000_vf.c(0 hunks)drivers/net/intel/i40e/base/i40e_adminq.c(0 hunks)drivers/net/intel/ice/base/ice_flex_pipe.c(0 hunks)drivers/net/intel/ice/base/ice_hw_autogen.h(0 hunks)drivers/net/intel/ixgbe/base/ixgbe_dcb_82599.c(0 hunks)drivers/net/intel/ixgbe/base/ixgbe_x540.h(0 hunks)drivers/net/mlx5/hws/mlx5dr_buddy.c(0 hunks)drivers/net/nfp/nfp_ipsec.c(0 hunks)drivers/net/nfp/nfpcore/nfp_elf.c(0 hunks)drivers/net/ngbe/base/ngbe_dummy.h(0 hunks)drivers/net/ngbe/base/ngbe_phy.c(0 hunks)drivers/net/octeontx/base/octeontx_bgx.h(0 hunks)drivers/net/txgbe/base/txgbe_dcb.c(0 hunks)drivers/net/txgbe/base/txgbe_dcb_hw.c(0 hunks)drivers/net/txgbe/base/txgbe_dummy.h(0 hunks)drivers/net/txgbe/base/txgbe_eeprom.c(0 hunks)drivers/net/virtio/virtio_cvq.c(0 hunks)drivers/raw/cnxk_rvu_lf/cnxk_rvu_lf_selftest.c(0 hunks)drivers/vdpa/mlx5/mlx5_vdpa_event.c(0 hunks)dts/configurations/tests_config.example.yaml(1 hunks)license/Linux-syscall-note(0 hunks)
💤 Files with no reviewable changes (39)
- drivers/net/ngbe/base/ngbe_dummy.h
- doc/guides/nics/cxgbe.rst
- drivers/net/gve/gve_rss.c
- drivers/net/intel/ixgbe/base/ixgbe_x540.h
- drivers/net/txgbe/base/txgbe_eeprom.c
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_defs.h
- drivers/net/hinic/base/hinic_pmd_mbox.c
- drivers/net/intel/i40e/base/i40e_adminq.c
- drivers/net/ngbe/base/ngbe_phy.c
- drivers/net/mlx5/hws/mlx5dr_buddy.c
- drivers/net/octeontx/base/octeontx_bgx.h
- drivers/net/txgbe/base/txgbe_dummy.h
- drivers/net/virtio/virtio_cvq.c
- drivers/net/gve/base/gve_adminq.c
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_class.c
- doc/guides/nics/ipn3ke.rst
- license/Linux-syscall-note
- drivers/net/hinic/base/meson.build
- drivers/net/intel/e1000/base/e1000_vf.c
- drivers/net/gve/gve_rss.h
- drivers/net/intel/ice/base/ice_flex_pipe.c
- drivers/net/nfp/nfp_ipsec.c
- drivers/raw/cnxk_rvu_lf/cnxk_rvu_lf_selftest.c
- drivers/net/intel/e1000/base/e1000_82575.c
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_wh_plus_class.c
- drivers/net/hinic/base/hinic_pmd_niccfg.c
- drivers/net/nfp/nfpcore/nfp_elf.c
- drivers/net/txgbe/base/txgbe_dcb_hw.c
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_tbl.c
- drivers/net/intel/ixgbe/base/ixgbe_dcb_82599.c
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_act.c
- drivers/net/txgbe/base/txgbe_dcb.c
- drivers/net/intel/e1000/base/e1000_mbx.c
- drivers/net/bnxt/tf_core/tf_em_common.c
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_thor2_class.c
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_thor_class.c
- drivers/net/intel/ice/base/ice_hw_autogen.h
- drivers/event/dlb2/dlb2_iface.c
- drivers/vdpa/mlx5/mlx5_vdpa_event.c
🧰 Additional context used
🧬 Code graph analysis (1)
app/test-pmd/testpmd.c (1)
app/test-pmd/cmdline.c (5)
pid(1481-1483)pid(1747-1750)pid(1864-1866)pid(2148-2161)pid(14455-14462)
🔇 Additional comments (9)
dts/configurations/tests_config.example.yaml (1)
17-17: No concerns with formatting update.This is a benign formatting/whitespace change in a commented-out example configuration line. No functional impact on active configurations.
app/test-bbdev/test_vectors/turbo_dec_c1_k6144_r0_e10376_crc24b_sbd_negllr_low_snr.data (1)
639-640: Aligningexpected_statusformatting with other test vectors looks goodThis is a formatting-only change (explicit
OKon its own line) and does not affect test semantics.app/test-bbdev/test_vectors/turbo_dec_c2_k3136_r0_e4920_sbd_negllr_crc24b.data (1)
676-677: Consistentexpected_statusformatting
expected_status = OKformatting is now consistent with the rest of the bbdev turbo-dec vectors; no functional impact.app/test-bbdev/test_vectors/turbo_dec_c1_k6144_r0_e34560_sbd_posllr.data (1)
1221-1222: EOF newline /expected_statusnormalization is fineAdding the trailing newline and explicit
OKkeeps this vector in line with others and should not affect parsing or behavior.app/test-bbdev/test_vectors/bbdev_null.data (1)
4-5:bbdev_nullvector remains semantically unchanged
op_typestill usesRTE_BBDEV_OP_NONE; the only change is formatting/newline, which is harmless and improves consistency.app/test-bbdev/test_vectors/turbo_enc_c3_k4800_r2_e14412_crc24b.data (1)
152-153: Encoder test vectorexpected_statusformatting is OKThe change is limited to how
expected_statusis written; encoder data and parameters are untouched.app/test-bbdev/test_vectors/turbo_dec_c1_k6144_r0_e10376_crc24b_sbd_negllr_high_snr.data (1)
639-640: High-SNR turbo-dec vector formatting change is benignNormalizing
expected_status = OKhere mirrors the low-SNR file and remains purely cosmetic.app/test-bbdev/test_vectors/turbo_enc_c1_k6144_r0_e18448_crc24a.data (1)
158-159: EOF newline addition for encoder vector is fineAdding the newline after
expected_status = OKaligns this file with the others and doesn’t alter test behavior.VERSION (1)
1-1: Version bumped to 25.11.0 (final)The version string correctly reflects a final release; just ensure any external packaging or documentation references are updated in parallel if needed.
NOTE: This is an auto submission for "[v3] app/testpmd: fail on shared rxq switch mismatch".
See "http://patchwork.dpdk.org/project/dpdk/list/?series=36815" for details.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.