[PWCI] "app/test: remove use of coremasks" #512
+53
−87
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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>
A number of test cases spawned off secondary processes and used the "-c", or coremask, EAL option to do so. This option is deprecated, so replace it with "-l" in tests. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by: Stephen Hemminger <stephen@networkplumber.org> Signed-off-by: 0-day Robot <robot@bytheb.org>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideReplace deprecated coremask-based EAL arguments with core list syntax in unit tests, plus miscellaneous whitespace/format cleanups and asset updates across drivers, docs, and data files. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- For the secondary test apps, consider guarding the switch from
-cto-lwith a brief note in the commit message or code comment if there are any environments or scripts that still rely on coremask semantics, since this changes the expected EAL argument format for spawned processes. - The PR mixes a small functional change in the test apps with a large number of whitespace-only cleanups across drivers and docs; splitting the formatting-only edits into a separate patch/commit would make review and potential backports of the behavioral change easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- For the secondary test apps, consider guarding the switch from `-c` to `-l` with a brief note in the commit message or code comment if there are any environments or scripts that still rely on coremask semantics, since this changes the expected EAL argument format for spawned processes.
- The PR mixes a small functional change in the test apps with a large number of whitespace-only cleanups across drivers and docs; splitting the formatting-only edits into a separate patch/commit would make review and potential backports of the behavioral change easier.
## Individual Comments
### Comment 1
<location> `doc/guides/nics/mlx5.rst:1342-1344` </location>
<code_context>
Faster than software steering (SWS),
-hardware steering (HWS) is the only mode supporting the flow template async API.
+hardware steering (HWS) is the only mode supporting the flow template async API,
+and the only mode supported on device ConnectX-9 and later.
Flow rules are managed by the hardware,
</code_context>
<issue_to_address>
**suggestion (typo):** The phrase "on device ConnectX-9 and later" is grammatically awkward; suggest pluralizing or rephrasing.
Consider: "and the only mode supported on ConnectX-9 and later devices," which avoids the awkward "on device … and later" construction.
```suggestion
Faster than software steering (SWS),
hardware steering (HWS) is the only mode supporting the flow template async API,
and the only mode supported on ConnectX-9 and later devices.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
WalkthroughVersion string updated from 25.11.0-rc4 to 25.11.0 (prerelease removal). Test infrastructure and core selection flags changed from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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
🧹 Nitpick comments (4)
doc/guides/nics/mlx5.rst (3)
691-702: Clarify whether dv_flow_en=1 is backward compatible with ConnectX-9 devices.The revised documentation states that dv_flow_en value 1 (Direct Verbs) "is available for devices older than ConnectX-9" (line 695), yet line 701 says the driver "is configured by default to 1 (DV flow steering) if supported." This phrasing creates ambiguity about backward compatibility: Can ConnectX-9+ devices still use value 1 as a fallback, or is it strictly forbidden?
Consider rewording to clarify whether:
- Value 1 is recommended only for pre-ConnectX-9 devices (but still functional on newer hardware), or
- Value 1 is not supported on ConnectX-9+ devices at all.
For example: "Value 1 enables Direct Verbs flow steering, available on ConnectX-6 Dx and earlier, and requires rdma-core 24 or later."
1343-1344: Verify consistency of HWS availability claim with dv_flow_en documentation.The statement that hardware steering (HWS) "is the only mode supported on device ConnectX-9 and later" (line 1343-1344) should align with the dv_flow_en parameter description. If ConnectX-9+ devices must use HWS exclusively, this should be made explicit in the dv_flow_en section to prevent users from attempting unsupported configurations.
Consider adding a note in the dv_flow_en=2 description or in the default behavior explanation (lines 701-702) to clarify that ConnectX-9+ devices require dv_flow_en=2.
691-702: Document rdma-core version requirement scope.Line 696 specifies "requires rdma-core 24 or later" for dv_flow_en=1, but this requirement is now more of a historical note since the documentation restricts value 1 to pre-ConnectX-9 devices. Consider clarifying whether rdma-core 24 is a hard requirement for all dv_flow_en=1 deployments, or whether newer rdma-core versions also work (to aid users upgrading libraries without changing hardware).
app/test/test_mp_secondary.c (1)
69-105: LGTM! Clean refactoring from coremask to core list format.The changes correctly transition from the deprecated
-c(coremask) option to the-l(core list) option:
- Variable renamed from
coremasktocore_strfor clarity- Format specifier changed from
%x(hexadecimal) to%u(decimal) to match core list expectations- Logic simplified from
1 << rte_get_main_lcore()(bitmask) torte_get_main_lcore()(direct core ID), which is correct for the-lflag- All four argv arrays (argv1, argv2, argv3, argv4) updated consistently
Optionally, consider increasing the buffer size from 10 to 16 or 32 bytes for extra safety, though core IDs in practice rarely exceed three digits.
- char core_str[10]; + char core_str[16];
📜 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 (52)
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/test_mp_secondary.c(2 hunks)app/test/test_pdump.c(2 hunks)app/test/test_timer_secondary.c(2 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/virtio/virtio_cvq.c
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_act.c
- drivers/net/gve/gve_rss.c
- drivers/net/txgbe/base/txgbe_eeprom.c
- drivers/net/nfp/nfp_ipsec.c
- drivers/net/hinic/base/hinic_pmd_mbox.c
- drivers/net/nfp/nfpcore/nfp_elf.c
- drivers/net/octeontx/base/octeontx_bgx.h
- drivers/net/gve/gve_rss.h
- drivers/net/hinic/base/meson.build
- drivers/net/intel/i40e/base/i40e_adminq.c
- drivers/net/ngbe/base/ngbe_phy.c
- doc/guides/nics/cxgbe.rst
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_thor_class.c
- drivers/raw/cnxk_rvu_lf/cnxk_rvu_lf_selftest.c
- drivers/net/intel/ice/base/ice_flex_pipe.c
- license/Linux-syscall-note
- drivers/net/txgbe/base/txgbe_dummy.h
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_tbl.c
- drivers/net/bnxt/tf_core/tf_em_common.c
- drivers/net/txgbe/base/txgbe_dcb.c
- drivers/net/txgbe/base/txgbe_dcb_hw.c
- drivers/net/intel/ixgbe/base/ixgbe_x540.h
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_defs.h
- drivers/net/hinic/base/hinic_pmd_niccfg.c
- drivers/net/intel/e1000/base/e1000_vf.c
- drivers/net/intel/ixgbe/base/ixgbe_dcb_82599.c
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_wh_plus_class.c
- drivers/net/intel/e1000/base/e1000_mbx.c
- drivers/vdpa/mlx5/mlx5_vdpa_event.c
- drivers/event/dlb2/dlb2_iface.c
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_class.c
- drivers/net/intel/e1000/base/e1000_82575.c
- drivers/net/mlx5/hws/mlx5dr_buddy.c
- doc/guides/nics/ipn3ke.rst
- drivers/net/ngbe/base/ngbe_dummy.h
- drivers/net/bnxt/tf_ulp/generic_templates/ulp_template_db_thor2_class.c
- drivers/net/gve/base/gve_adminq.c
- drivers/net/intel/ice/base/ice_hw_autogen.h
🧰 Additional context used
🧬 Code graph analysis (2)
app/test/test_mp_secondary.c (1)
lib/eal/common/eal_common_lcore.c (1)
rte_get_main_lcore(23-26)
app/test/test_pdump.c (1)
lib/eal/common/eal_common_lcore.c (1)
rte_get_main_lcore(23-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (11)
dts/configurations/tests_config.example.yaml (1)
17-17: Formatting improvement—no functional changes.Adding a trailing newline at EOF aligns with POSIX standards and best practices. The configuration content remains unchanged.
app/test-bbdev/test_vectors/turbo_enc_c3_k4800_r2_e14412_crc24b.data (1)
152-153: Good formatting consistency fix.Adding the trailing newline aligns with POSIX conventions and ensures consistent file formatting across the test vector data files.
app/test-bbdev/test_vectors/turbo_enc_c1_k6144_r0_e18448_crc24a.data (1)
158-159: Formatting improvement approved.The trailing newline addition to the
expected_statussection aligns with standard Unix text file conventions and the PR's broader formatting consistency improvements across test vectors. No functional impact to test execution or validation.VERSION (1)
1-1: Version string update is finePure metadata bump; no functional impact here.
app/test-bbdev/test_vectors/turbo_dec_c1_k6144_r0_e10376_crc24b_sbd_negllr_low_snr.data (1)
639-640: EOF newline normalization only
expected_status = OKis unchanged; added trailing newline is harmless and improves consistency across vectors.app/test-bbdev/test_vectors/turbo_dec_c1_k6144_r0_e34560_sbd_posllr.data (1)
1221-1222: EOF newline normalization only
expected_statusremainsOK; trailing newline change has no effect on test semantics.app/test-bbdev/test_vectors/bbdev_null.data (1)
4-5: No semantic change in op_type
op_type = RTE_BBDEV_OP_NONEis unchanged; any diff here is formatting-only.app/test-bbdev/test_vectors/turbo_dec_c1_k6144_r0_e10376_crc24b_sbd_negllr_high_snr.data (1)
639-640: EOF newline normalization only
expected_status = OKis unchanged; added newline just standardizes file termination.app/test-bbdev/test_vectors/turbo_dec_c2_k3136_r0_e4920_sbd_negllr_crc24b.data (1)
676-677: EOF newline normalization only
expected_statusremainsOK; change is strictly formatting.app/test/test_pdump.c (1)
176-195: Switch to-lcore list withcore_strlooks correctUsing
core_strand passing-l <main_lcore>(decimal viarte_get_main_lcore()and%u) is consistent and safe;core_str’s lifetime covers thelaunch_p(argv1)call.app/test/test_timer_secondary.c (1)
53-72: Updated secondary spawn to use-lwith decimal lcore idFormatting the target lcore into
core_strand passing it via-linargvmatches the updated core-list interface and is used safely within the lifetime of the stack buffer.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
NOTE: This is an auto submission for "app/test: remove use of coremasks".
See "http://patchwork.dpdk.org/project/dpdk/list/?series=36823" for details.
Summary by Sourcery
Update test applications to stop using coremask-based EAL options and perform minor codebase cleanups.
Enhancements:
Chores:
Summary by CodeRabbit
Release Notes
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.