-
Notifications
You must be signed in to change notification settings - Fork 4
[PWCI] "test/debug: fix test failure on ppc64" #488
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?
Conversation
Enhance the documentation by adding information about limitations specific to the synchronous API when used with hardware steering. This update provides users with a more comprehensive understanding of the constraints in this configuration. Signed-off-by: Maayan Kashani <mkashani@nvidia.com> Signed-off-by: Bing Zhao <bingz@nvidia.com>
Add tested, and hence recommended, firmware and other versions for i40e and ice drivers. In the process remove version inform for obsolete DPDK releases. Signed-off-by: Hailin Xu <hailinx.xu@intel.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
There is the updated firmware providing the new capability bit "header_length_field_offset_mode". If this bit is set the length field offset in flex parser configuration should not be adjusted by the supported field mask left margin, and the bit "header_length_field_offset_mode" should be set in configuration command on the flex parser creation firmware call. Fixes: b04b06f ("net/mlx5: fix flex item header length field translation") Cc: stable@dpdk.org Signed-off-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
The update of flow engine devargs introduced a regression with untrusted VFs/SFs. Using HWS engine caused a degradation in probing 1K SFs, because memzone segments exceeds maximum 2560. Fixes: 53fdc23 ("common/mlx5: read SW steering capability bits") Fixes: d1ac7b6 ("net/mlx5: update flow devargs handling for future HW") Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
Count flow action is supported on root table with HW Steering flow engine with upstream drivers/libraries and drivers/libraries packaged with recently released DOCA. This patch updates relevant docs with minimum DOCA version. Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com> Acked-by: Bing Zhao <bingz@nvidia.com>
Previously, the Tx burst size in l3fwd was fixed at 256, which could lead to suboptimal performance in certain scenarios. This patch introduces separate --rx-burst and --tx-burst options to explicitly configure Rx and Tx burst sizes. By default, the Tx burst size now matches the Rx burst size for better efficiency and pipeline balance. Fixes: d5c4897 ("examples/l3fwd: add option to set Rx burst size") Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com> Tested-by: Venkat Kumar Ande <venkatkumar.ande@amd.com> Tested-by: Dengdui Huang <huangdengdui@huawei.com> Tested-by: Pavan Nikhilesh <pbhagavatula@marvell.com> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Currently, l3fwd starts in auto-negotiation mode, but it may fail to link up when auto-negotiation is not supported. Therefore, it is necessary to support starting with a specified speed for port. Additionally, this patch does not support changing the duplex mode. So speeds like 10M, 100M are not configurable using this method. Signed-off-by: Dengdui Huang <huangdengdui@huawei.com> Reviewed-by: Chengwen Feng <fengchengwen@huawei.com>
Currently, l3fwd-power starts in auto-negotiation mode, but it may fail to link up when auto-negotiation is not supported. Therefore, it is necessary to support starting with a specified speed for port. Additionally, this patch does not support changing the duplex mode. So speeds like 10M, 100M are not configurable using this method. Signed-off-by: Dengdui Huang <huangdengdui@huawei.com> Reviewed-by: Chengwen Feng <fengchengwen@huawei.com>
If format-truncation is enabled, the compiler detects a string overflow
since the snprintf() is putting ethernet address and new line
in buffer only sized for the address.
Since get_rx_queue_name() is used to create a ring name.
The buffer should be sized to be a valid ring name.
This fixes some format-overflow warnings and also corrects
for future errors.
In file included from ../examples/server_node_efd/efd_server/main.c:31:
In function ‘get_printable_mac_addr’,
inlined from ‘do_stats_display’ at ../examples/server_node_efd/efd_server/main.c:136:3:
../lib/net/rte_ether.h:248:36: warning: ‘snprintf’ output truncated before the last format character [-Wformat-truncation=]
248 | #define RTE_ETHER_ADDR_PRT_FMT "%02X:%02X:%02X:%02X:%02X:%02X"
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../examples/server_node_efd/efd_server/main.c:86:33: note: in expansion of macro ‘RTE_ETHER_ADDR_PRT_FMT’
86 | RTE_ETHER_ADDR_PRT_FMT "\n",
| ^~~~~~~~~~~~~~~~~~~~~~
../examples/server_node_efd/efd_server/main.c: In function ‘do_stats_display’:
../examples/server_node_efd/efd_server/main.c:86:59: note: format string is defined here
86 | RTE_ETHER_ADDR_PRT_FMT "\n",
|
Fixes: 39aad0e ("examples/flow_distributor: new example to demonstrate EFD")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
The ifname is limited to 128 characters, but it would allow up
to 128 characters as prefix then could overflow creating ifname.
Change to limit path prefix (iface) to 123 (128 - sizeof("1024"))
to avoid possible format overflow
../examples/vdpa/main.c:501:76: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
501 | snprintf(vports[devcnt].ifname, MAX_PATH_LEN, "%s%d",
| ^
../examples/vdpa/main.c:501:25: note: ‘snprintf’ output between 2 and 139 bytes into a destination of size 128
501 | snprintf(vports[devcnt].ifname, MAX_PATH_LEN, "%s%d",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
502 | iface, devcnt);
|
Fixes: 38f8ab0 ("vhost: make vDPA framework bus agnostic")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
In theory, lcore and queue could be so large that mbuf pool name could overflow. But that can never happen since lcore and queue will be in range. Add a check so that static tools know that. Format overflow warnings seen on older versions of Gcc only. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
There were several hidden bugs in examples because format truncation warning was disabled. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
After removing callback checks from ethdev fast path, some drivers crashed because tx_pkt_prepare was set to NULL. Some drivers (hns3, ngbe, txgbe) need to use rte_eth_tx_pkt_prepare_dummy when configuring queues. Other drivers (ntnic, softnic) does not need to set tx_pkt_prepare as it was set by eth_dev_set_dummy_fops() called by rte_eth_dev_allocate(). Bugzilla ID: 1834 Fixes: 066f3d9 ("ethdev: remove callback checks from fast path") Reported-by: Jiawen Wu <jiawenwu@trustnetic.com> Signed-off-by: Thomas Monjalon <thomas@monjalon.net> Reviewed-by: Morten Brørup <mb@smartsharesystems.com> Tested-by: Jiawen Wu <jiawenwu@trustnetic.com>
The Pollara 400 AI NIC is supported without any PMD changes. Add a link to the product brief. Cc: stable@dpdk.org Signed-off-by: Andrew Boyer <andrew.boyer@amd.com>
changes: 1. fix spelling from `SIENNA` to `SIENA` 2. add tuning guide for zen5 AMD Epyc 9005 3. add uncore power details 4. update on using AMD Solarflare X4 and X2 Signed-off-by: Vipin Varghese <vipin.varghese@amd.com> Tested-by: Thiyagarajan P <thiyagarajan.p@amd.com>
Updated some external links and removed a note which does not apply to MSVC builds anymore. Also reworded a sentence clarifying how to open a Visual Studio Developer Command Prompt from CMD and Powershell, as the original text could induce people to run VsDevCmd.bat from a Powershell prompt, which does not work. Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
The note about the prefix of the package pyelftools was not showed in the documentation output because the syntax was missing a colon, so it was considered as a simple comment in the source file. Fixes: 9e6f75e ("doc: update build section of FreeBSD guide") Cc: stable@dpdk.org Signed-off-by: Thomas Monjalon <thomas@monjalon.net> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
The version python used in current FreeBSD stable release 14.3 is now 3.11. Update documentation accordingly. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
KNI was removed from DPDK but the diagrams in the documentation were left behind. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Chengwen Feng <fengchengwen@huawei.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
This allows building and installing only the documentation, without recompiling the whole project, using: meson build -Denable_docs=true meson compile -C build doc meson install -C build --no-rebuild --tags doc In Debian/Ubuntu the documentation is built separately from the binaries, in a separate architecture-independent build job, so that it has to be done only once, instead of once per supported architecture. Signed-off-by: Luca Boccassi <bluca@debian.org> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Add tested platforms with NVIDIA NICs to the 25.11 release notes. Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
Add tested Intel platforms with Intel NICs to v25.11 release note. Signed-off-by: Yu Jiang <yux.jiang@intel.com>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
1001s Running binary with argv[]:'/tmp/autopkgtest.LvdXhc/build.6vT/src/build/app/dpdk-test' '--file-prefix=debug' '-l' '0' '--no-huge' '-m' '2048' <...> 1001s EAL: Cannot use IOVA as 'PA' since physical addresses are not available This is similar to 6b07762, on ppc64 iova-mode=pa is the default, but it doesn't work with --no-huge, so add --iova-mode=va to the test arguments. Fixes: 2b403dd ("test/debug: fix crash with mlx5 devices") Cc: stable@dpdk.org Signed-off-by: Luca Boccassi <bluca@debian.org> Signed-off-by: 0-day Robot <robot@bytheb.org>
Reviewer's GuideThis PR fixes a ppc64 test failure and multiple portability issues by refining the l3fwd burst configuration (separate RX/TX burst sizes and link-speed option), updating several drivers (mlx5 flex parser capabilities and flow config, hns3/ngbe/txgbe/ntnic/softnic tx_pkt_prepare behavior, misc limits and name sizes), adjusting sample apps (vdpa, ip_reassembly, server_node_efd), improving documentation install behavior, and adding a ppc64-specific iova-mode workaround in test_debug. Sequence diagram for l3fwd argument parsing and burst configurationsequenceDiagram
actor User
participant L3fwd_Main as L3fwd_main
participant Parser as parse_args
participant BurstParser as parse_pkt_burst
participant PMD as rte_eth_dev_info_get
User->>L3fwd_Main: start application with CLI options
L3fwd_Main->>Parser: parse_args(argc, argv)
alt rx-burst option
Parser->>BurstParser: parse_pkt_burst(optarg, true, &rx_burst_size)
BurstParser->>BurstParser: convert optarg to pkt_burst
alt pkt_burst > MAX_PKT_BURST
BurstParser-->>Parser: log and keep rx_burst_size
else pkt_burst > 0
BurstParser->>BurstParser: rx_burst_size = pkt_burst
BurstParser-->>Parser: return
else pkt_burst == 0
BurstParser->>PMD: rte_eth_dev_info_get(0, &dev_info)
PMD-->>BurstParser: dev_info with default_rxportconf
BurstParser->>BurstParser: burst_size = dev_info.default_rxportconf.burst_size
alt burst_size == 0 or burst_size > MAX_PKT_BURST
BurstParser-->>Parser: log and keep rx_burst_size
else valid burst_size
BurstParser->>BurstParser: rx_burst_size = burst_size
BurstParser-->>Parser: log PMD-provided RX burst
end
end
end
alt tx-burst option
Parser->>BurstParser: parse_pkt_burst(optarg, false, &tx_burst_size)
BurstParser->>BurstParser: convert optarg to pkt_burst
alt pkt_burst > MAX_PKT_BURST
BurstParser-->>Parser: log and keep tx_burst_size
else pkt_burst > 0
BurstParser->>BurstParser: tx_burst_size = pkt_burst
BurstParser-->>Parser: return
else pkt_burst == 0
BurstParser-->>Parser: log use default tx_burst_size
end
end
Parser-->>L3fwd_Main: return configured rx_burst_size, tx_burst_size
L3fwd_Main->>L3fwd_Main: RTE_LOG Using Rx burst rx_burst_size Tx burst tx_burst_size
L3fwd_Main-->>User: run dataplane using configured burst sizes
Flow diagram for mlx5 flex parser offset mode configurationflowchart TD
A[Start flex parser setup] --> B[Query HCA parse graph node capabilities]
B --> C[Set attr.header_length_mask_width
and attr.header_length_field_mode_wa
from HCA caps]
C --> D[Translate flex length
mlx5_flex_translate_length]
D --> E[Compute msb and offset
from field]
E --> F{attr.header_length_field_mode_wa?}
F -->|Yes legacy firmware| G{msb < header_length_mask_width?}
G -->|Yes| H{header_length_mask_width - msb > offset?}
H -->|Yes| I[Return error
invalid mask and shift]
H -->|No| J[Continue
with adjusted offset]
G -->|No| J
F -->|No new firmware| K[Use computed offset directly]
J --> L[Set node.header_length_field_mask
node.header_length_field_shift
node.header_length_field_offset]
K --> L
L --> M[Set node.header_length_field_offset_mode
based on !attr.header_length_field_mode_wa]
M --> N[Create flex parser object
mlx5_devx_cmd_create_flex_parser]
N --> O[Program hardware flex parser]
O --> P[End]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughVersion bump from 25.11.0-rc3 to 25.11.0-rc4. Updates to platform documentation (AMD EPYC TURIN support, Intel i40e/ice compatibility tables, Solarflare NICs). MLX5 driver enhancements for header length field offset handling. Network driver TX prepare callback refactoring. L3FWD example application refactored to support independent RX and TX burst sizes via command-line options. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
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.
Hey there - I've reviewed your changes - here's some feedback:
- In ip_reassembly setup_queue_tbl(), the bounds check on lcore/queue uses
>rather than>=, so an index equal to RTE_MAX_LCORE or RTE_MAX_QUEUES_PER_PORT will still be accepted and may result in an overlong pool name; consider changing the checks to>=to enforce the intended limits. - The new
--eth-link-speedhandling is implemented differently in l3fwd (via an enum-based long option) and l3fwd-power (via string comparisons inside the long-option handler); consider refactoring this into a shared helper or aligning the patterns to reduce duplication and the risk of future divergence in validation behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In ip_reassembly setup_queue_tbl(), the bounds check on lcore/queue uses `>` rather than `>=`, so an index equal to RTE_MAX_LCORE or RTE_MAX_QUEUES_PER_PORT will still be accepted and may result in an overlong pool name; consider changing the checks to `>=` to enforce the intended limits.
- The new `--eth-link-speed` handling is implemented differently in l3fwd (via an enum-based long option) and l3fwd-power (via string comparisons inside the long-option handler); consider refactoring this into a shared helper or aligning the patterns to reduce duplication and the risk of future divergence in validation behavior.
## Individual Comments
### Comment 1
<location> `examples/l3fwd/l3fwd.h:151-152` </location>
<code_context>
/* enough pkts to be sent */
- if (unlikely(len == MAX_PKT_BURST)) {
- send_burst(qconf, MAX_PKT_BURST, port);
+ if (unlikely(len == rx_burst_size)) {
+ send_burst(qconf, rx_burst_size, port);
len = 0;
}
</code_context>
<issue_to_address>
**suggestion (performance):** TX flush threshold now tied to RX burst size and no longer uses a separate, smaller MAX_TX_BURST, which may unintentionally increase buffering/latency.
This effectively removes the earlier TX flush behavior that was based on MAX_TX_BURST (MAX_PKT_BURST / 2) and instead flushes only when len reaches rx_burst_size. That can increase TX buffering/latency when RX bursts are large. If the goal is to keep TX tuning independent, consider basing the threshold on tx_burst_size or reintroducing a separate TX-specific threshold rather than tying it directly to RX burst size.
Suggested implementation:
```c
extern uint32_t rx_burst_size;
extern uint32_t tx_burst_size;
extern uint32_t mb_mempool_cache_size;
```
```c
/* Send burst of packets on an output interface */
len++;
/*
* enough pkts to be sent
* use separate TX threshold to keep TX tuning independent of RX burst size
*/
if (unlikely(len == tx_burst_size)) {
send_burst(qconf, len, port);
len = 0;
}
```
To fully implement this:
1) Define and initialize tx_burst_size in the appropriate C source file (likely examples/l3fwd/l3fwd.c) as a configurable TX tuning parameter, e.g.:
- Set a default such as tx_burst_size = MAX_TX_BURST (where MAX_TX_BURST was previously defined as MAX_PKT_BURST / 2), or
- Derive it from existing constants while keeping it independent of rx_burst_size.
2) If l3fwd supports command-line/runtime configuration for rx_burst_size, consider adding a similar option for tx_burst_size so users can tune TX flushing behavior separately from RX.
3) Ensure tx_burst_size is never configured larger than the TX ring or any MAX_PKT_BURST-equivalent limit to avoid overruns, and update any documentation/comments that previously referred only to MAX_TX_BURST or RX-driven flushing.
</issue_to_address>
### Comment 2
<location> `examples/ip_reassembly/main.c:887-888` </location>
<code_context>
nb_mbuf = RTE_MAX(nb_mbuf, (uint32_t)NB_MBUF);
+ /* Should never happen but check so that pool name won't be too long. */
+ if (lcore > RTE_MAX_LCORE || queue > RTE_MAX_QUEUES_PER_PORT) {
+ RTE_LOG(ERR, IP_RSMBL, "invalid lcore %u or queue %u",
+ lcore, queue);
</code_context>
<issue_to_address>
**issue (bug_risk):** The bounds check on lcore/queue is likely off-by-one and still allows out-of-range indices equal to the max constants.
RTE_MAX_LCORE and RTE_MAX_QUEUES_PER_PORT represent counts, so valid indices are [0, max-1]. With the current check, lcore == RTE_MAX_LCORE or queue == RTE_MAX_QUEUES_PER_PORT would still pass and be out of range if used as indices. Please change the condition to use >=: `lcore >= RTE_MAX_LCORE || queue >= RTE_MAX_QUEUES_PER_PORT`.
</issue_to_address>
### Comment 3
<location> `doc/guides/rel_notes/release_25_11.rst:374-376` </location>
<code_context>
+ * Red Hat Enterprise Linux Server release 9.6
+ * Ubuntu 24.04 LTS
+ * Ubuntu 24.04.3 LTS
+ * Vmware ESXi 8.0.3
+
+ * NICs:
</code_context>
<issue_to_address>
**suggestion (typo):** Use the correct capitalization for VMware.
It should be written as "VMware ESXi 8.0.3" with capital V and W.
```suggestion
* Ubuntu 24.04 LTS
* Ubuntu 24.04.3 LTS
* VMware ESXi 8.0.3
```
</issue_to_address>
### Comment 4
<location> `doc/guides/nics/mlx5.rst:1413` </location>
<code_context>
+#. When using synchronous flow API,
+ the following limitations and considerations apply:
+
+ - There are limitations on size of match fields.
+ Exceeding these limitations will result in an error,
+ unlike other flow engines (``dv_flow_en`` < 2)
</code_context>
<issue_to_address>
**suggestion (typo):** Improve grammar in the sentence about limitations on match fields.
Change this to: "There are limitations on the size of match fields." (adding "the").
```suggestion
- There are limitations on the size of match fields.
```
</issue_to_address>
### Comment 5
<location> `doc/guides/nics/mlx5.rst:1368-1371` </location>
<code_context>
+ The application must call ``rte_flow_configure()``
+ before creating any flow rules
+ when using :ref:`asynchronous flow API <flow_async_api>`.
+ It is also recommended for synchronous API.
+
Limitations
</code_context>
<issue_to_address>
**suggestion (typo):** Clarify wording by adding 'the' before 'synchronous API'.
Consider: "It is also recommended for the synchronous API."
```suggestion
The application must call ``rte_flow_configure()``
before creating any flow rules
when using :ref:`asynchronous flow API <flow_async_api>`.
It is also recommended for the synchronous API.
```
</issue_to_address>
### Comment 6
<location> `doc/guides/nics/mlx5.rst:1443` </location>
<code_context>
+ If the application needs to modify flow queue settings,
+ it should also use ``rte_flow_configure``.
+
+ - When creating a rule with a partial `mask` provided in the flow item,
+ the `spec` value must be calculated after the "AND" operation with the `mask`.
+ If more significant bits are present in the `spec` than in the `mask`,
+ the rule will be created without any error
+ but the packets will not hit as expected.
+ Such limitation will be removed in future.
+
</code_context>
<issue_to_address>
**suggestion (typo):** Tighten the grammar in the sentence about this limitation being removed.
Consider rephrasing the last sentence to: "This limitation will be removed in the future."
```suggestion
This limitation will be removed in the future.
```
</issue_to_address>
### Comment 7
<location> `doc/guides/linux_gsg/amd_platform.rst:102` </location>
<code_context>
+Uncore
+~~~~~~
+
+AMD EPYC SoC support UNCORE power functions via ``rte_power_uncore`` from `25.03` onwards.
+These are tested and validated on GENOA, SIENA and TURIN.
+Please refer the tuning guides to enable ``HSMP`` and DPDK power management guide.
</code_context>
<issue_to_address>
**issue (typo):** Fix subject-verb agreement in the Uncore power functions sentence.
Replace “support” with “supports” in the sentence: “AMD EPYC SoC supports UNCORE power functions via ``rte_power_uncore`` from `25.03` onwards.”
```suggestion
AMD EPYC SoC supports UNCORE power functions via ``rte_power_uncore`` from `25.03` onwards.
```
</issue_to_address>
### Comment 8
<location> `doc/guides/freebsd_gsg/build_dpdk.rst:23-24` </location>
<code_context>
-.. note:
+.. note::
- The specific package for pyelftools is dependent on the version of python in use,
- Python 3.8 being the version at type of writing, hence the ``py38`` prefix.
+ The specific package for pyelftools is dependent on the version of python in use,
+ Python 3.11 being the version at type of writing, hence the ``py311`` prefix.
These can be installed using (as root)::
</code_context>
<issue_to_address>
**issue (typo):** Correct 'type of writing' to 'time of writing'.
This looks like a typo; "time of writing" is the standard phrasing in this context.
```suggestion
The specific package for pyelftools is dependent on the version of python in use,
Python 3.11 being the version at time of writing, hence the ``py311`` prefix.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/l3fwd/main.c (1)
703-750: Fix: Hardcoded port 0 in PMD query requires enabled port or deferred initialization.The
parse_pkt_burst()function queriesrte_eth_dev_info_get(0, ...)during command-line parsing (line 729), before ports are initialized and beforeenabled_port_maskcan be used. This will fail if port 0 is not available or not in the user's port mask (e.g.,-p 0x2).The call stack is:
main()→parse_args()(line 1686) →parse_pkt_burst()→rte_eth_dev_info_get(0)(line 729), which occurs before port resource setup (line 1714) and port initialization (line 1717+).Options to fix:
- Pass
enabled_port_masktoparse_pkt_burst()and query the first enabled port instead of hardcoded 0- Query the first available port from all ports (using
RTE_ETH_FOREACH_DEV())- Defer PMD-based defaults to after ports are configured, or fall back gracefully if the query fails
🧹 Nitpick comments (7)
doc/guides/linux_gsg/amd_platform.rst (1)
107-130: NIC section setup is clear but contains redundant instructions between x4 and x2 subsections.The setup instructions for Solarflare x4 and x2 are nearly identical, with only the kernel driver reference differing (out-of-tree sfc vs sfc). Consider consolidating these to reduce duplication while highlighting the driver difference.
Example refactored structure:
-To use Solarflare x4 on AMD EPYC TURIN, please ensure the following - -#. Upgrade Linux or distro version to 6.13 or higher. - -#. Disable PCIe power option via ``vfio-pci.disable_idle_d3=1`` in GRUB. - -#. Enable the hardware with out-of-tree sfc (contact Solarflare) Linux kernel driver first; - then bind with ``vfio-pci`` next. - -#. Please use PMD args ``fw_variant=ultra-low-latency,perf_profile=throughput``. - -To use Solarflare x2 on AMD EPYC TURIN, please ensure the following - -#. Upgrade Linux or distro version to 6.13 or higher. - -#. Disable PCIe power option via ``vfio-pci.disable_idle_d3=1`` in GRUB. - -#. Enable the hardware with ``sfc`` Linux Kernel Driver first; - then bind with ``vfio-pci`` next. - -#. Please use PMD args ``fw_variant=ultra-low-latency,perf_profile=throughput``. +To use Solarflare NICs (x4 or x2) on AMD EPYC TURIN, please ensure the following + +#. Upgrade Linux or distro version to 6.13 or higher. + +#. Disable PCIe power option via ``vfio-pci.disable_idle_d3=1`` in GRUB. + +#. Enable the hardware with the appropriate Linux kernel driver: + - For Solarflare x4: use the out-of-tree sfc driver (contact Solarflare) + - For Solarflare x2: use the in-tree ``sfc`` driver + Then bind with ``vfio-pci``. + +#. Use PMD args ``fw_variant=ultra-low-latency,perf_profile=throughput``.doc/guides/freebsd_gsg/build_dpdk.rst (1)
19-29: Python/pyelftools dependency bump looks good; consider more generic wordingSwitching to
py311-pyelftoolsand mentioning Python 3.11 matches current port naming. You might consider phrasing the note as “e.g.py311-pyelftoolsfor Python 3.11; use thepyXYprefix matching your Python version” to avoid future edits when the default Python changes.Please double-check against the current FreeBSD ports tree that
py311-pyelftoolsis indeed the correct package name on all supported FreeBSD versions.examples/l3fwd-power/main.c (2)
1490-1507: Clarify--eth-link-speedusage in help textThe new
--eth-link-speedline doesn’t indicate expected units or valid values (e.g., “1000 for 1G, 10000 for 10G, …”), which could confuse users. Consider briefly documenting the units and supported speeds to align withrte_eth_speed_bitflag()semantics.
1746-1774:--eth-link-speedoption wiring is correct but argument validation could be stricterThe option is correctly added to
lgoptsand mapped inparse_args, and the resulting bitflag is applied toport_conf.link_speeds, so the basic wiring is sound.Two robustness points you may want to tighten up:
atoi()and invalid/garbage values: non-numeric or out-of-range strings become0and are silently ignored (no error, app just uses default autoneg). Usingstrtolwith end-pointer checks, and rejecting inputs that don’t parse cleanly, would give clearer feedback.- Unknown positive speeds: if
rte_eth_speed_bitflag(speed_num, 0)returns 0 for an unsupported speed, the option is again silently ignored. It may be better to treat “non-negative but maps to no bitflag” as an error (print message + fail parsing), similar to how you handle the explicitly unsupported 10M/100M case.Functionally this is fine as-is, but tightening validation would make the CLI behavior less surprising.
Please double-check against your DPDK version’s
rte_eth_speed_bitflag()documentation that:
- 10M and 100M speeds are indeed the only ones you intend to reject outright here, and
- returning
0fromrte_eth_speed_bitflag()is the canonical way to detect an unsupported speed.Also applies to: 1958-1971
doc/guides/nics/mlx5.rst (1)
1410-1444: Sync API + HWS limitations block is valuable; consider a small wording tweakThe new subsection clearly calls out HWS-specific behavior differences for the synchronous API (match-field size limits, rule update semantics, default port/queue attributes, and partial-mask handling), which is important context for users porting from SW steering.
One minor readability suggestion: the sentence
“By default, the port is configured with zeroed
rte_flow_port_attr: there are zero flow queues (one is created by default), no actions, and no flags set.”can be misread. Consider separating “requested” vs “effective” defaults:
- - By default, the port is configured with zeroed ``rte_flow_port_attr``: - there are zero flow queues (one is created by default), - no actions, and no flags set. - The default flow queue size for ``rte_flow_queue_attr`` is 32 - (used for the internal flow queue). + - By default, the port is configured with a zeroed ``rte_flow_port_attr``: + no flow queues, actions or flags are *requested*. + The driver still creates a single internal flow queue + with the default ``rte_flow_queue_attr`` size of 32.Functionally it’s the same, just a bit clearer for readers.
drivers/net/mlx5/mlx5.c (1)
1077-1092: SRv6 flex parser offset split by header_length_field_mode_wa; consider also setting offset-mode flagThe new
if (attr->header_length_field_mode_wa) { ... } else { ... }split fornode.header_length_field_offsetcleanly separates legacy WA vs new firmware behavior and matches the new capability flag.For consistency with the generic flex-item path (where
header_length_field_offset_modeis set from the same capability), consider explicitly initializing it here as well, e.g.:node.header_length_field_offset_mode = !attr->header_length_field_mode_wa;This would make SRv6 nodes follow the same mode bookkeeping and avoid ambiguity about which offset interpretation is active on newer HCAs.
drivers/common/mlx5/mlx5_devx_cmds.c (1)
914-918: Deriving header_length_field_mode_wa by inverting header_length_field_offset_modeHere
attr->header_length_field_mode_wais defined as:attr->header_length_field_mode_wa = !MLX5_GET(parse_graph_node_cap, hcattr, header_length_field_offset_mode);which encodes “WA required” as the inverse of the HCA’s
header_length_field_offset_modebit. That’s subtle and worth making explicit (e.g., a short comment that0 == legacy WA semantics, 1 == direct offset semantics), and it’s important this matches the PRM definition of that field.Please verify against the HW spec that this inversion is correct for all relevant devices, especially around the ConnectX‑8 transition.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
doc/guides/prog_guide/img/kni_traffic_flow.pngis excluded by!**/*.pngdoc/guides/prog_guide/img/pkt_flow_kni.pngis excluded by!**/*.png
📒 Files selected for processing (37)
VERSION(1 hunks)app/test/test_debug.c(2 hunks)doc/api/meson.build(3 hunks)doc/guides/cryptodevs/ionic.rst(2 hunks)doc/guides/freebsd_gsg/build_dpdk.rst(1 hunks)doc/guides/linux_gsg/amd_platform.rst(2 hunks)doc/guides/meson.build(1 hunks)doc/guides/nics/i40e.rst(2 hunks)doc/guides/nics/ice.rst(2 hunks)doc/guides/nics/ionic.rst(2 hunks)doc/guides/nics/mlx5.rst(11 hunks)doc/guides/rel_notes/release_25_11.rst(1 hunks)doc/guides/sample_app_ug/l3_forward.rst(2 hunks)doc/guides/windows_gsg/build_dpdk.rst(4 hunks)drivers/common/mlx5/mlx5_devx_cmds.c(2 hunks)drivers/common/mlx5/mlx5_devx_cmds.h(2 hunks)drivers/common/mlx5/mlx5_prm.h(4 hunks)drivers/net/hns3/hns3_rxtx.c(2 hunks)drivers/net/mlx5/mlx5.c(2 hunks)drivers/net/mlx5/mlx5_flow_flex.c(2 hunks)drivers/net/ngbe/ngbe_rxtx.c(1 hunks)drivers/net/ntnic/ntnic_ethdev.c(0 hunks)drivers/net/softnic/rte_eth_softnic.c(0 hunks)drivers/net/txgbe/txgbe_rxtx.c(1 hunks)examples/ip_reassembly/main.c(1 hunks)examples/l3fwd-power/main.c(4 hunks)examples/l3fwd/l3fwd.h(2 hunks)examples/l3fwd/l3fwd_acl.c(1 hunks)examples/l3fwd/l3fwd_common.h(2 hunks)examples/l3fwd/l3fwd_em.c(1 hunks)examples/l3fwd/l3fwd_fib.c(1 hunks)examples/l3fwd/l3fwd_lpm.c(1 hunks)examples/l3fwd/main.c(15 hunks)examples/meson.build(1 hunks)examples/server_node_efd/efd_server/main.c(1 hunks)examples/server_node_efd/shared/common.h(1 hunks)examples/vdpa/main.c(3 hunks)
💤 Files with no reviewable changes (2)
- drivers/net/ntnic/ntnic_ethdev.c
- drivers/net/softnic/rte_eth_softnic.c
⏰ 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 (57)
doc/guides/nics/ionic.rst (2)
2-2: Copyright year update is appropriate.Updated copyright year to 2025, which is consistent with current date.
18-19: New Pollara 400 model entry added with correct formatting.The new model entry for "Pollara 400 single-port 400G AI NIC" is properly formatted and follows the established pattern of existing hardware entries. The PDF documentation link uses the correct AMD Pensando documentation URL structure.
doc/guides/cryptodevs/ionic.rst (2)
2-2: Copyright year updated consistently.Updated copyright year from 2021-2024 to 2021-2025, maintaining consistency with the NIC driver documentation.
19-20: Model entry is consistent with NIC driver documentation.The Pollara 400 model entry matches exactly with the entry added in
doc/guides/nics/ionic.rst, ensuring consistent documentation across both the NIC and crypto driver guides. The model name, description, and PDF documentation link are all identical.doc/guides/nics/ice.rst (1)
90-91: Documentation for DPDK 25.11 compatibility correctly added.The new 25.11 entries for both E810 and E830 adapters are properly formatted and maintain consistency with existing table structure. Kernel driver and DDP version specifications are consistent between both device families.
Also applies to: 100-101
doc/guides/nics/i40e.rst (2)
107-108: Documentation for DPDK 25.11 compatibility correctly added.The new 25.11 entries for both X710/XL710/XXV710 and X722 families are properly formatted and maintain consistency with existing table structure. Kernel driver versioning (2.28.13) is consistent across both device families, with appropriately differentiated firmware versions.
Also applies to: 140-141
1-1: Scope mismatch: PR description vs. provided files.The PR description references "test/debug: fix test failure on ppc64," but the provided files are documentation updates for NIC driver compatibility matrices. While these updates align with the AI-generated summary mentioning a version bump, the actual test failure fix is not visible in the files under review. Please confirm whether the ppc64 fix is in a separate file not provided, or clarify the relationship between this documentation update and the test debugging scope.
doc/guides/linux_gsg/amd_platform.rst (3)
25-27: Documentation additions for TURIN are consistent and properly referenced.The updates to the tuning guides list align with the new TURIN-specific content added throughout the document. PDF links follow the established pattern and reference official AMD documentation.
91-92: Power/Core section correctly updated with TURIN and amd_pstate passive mode specification.The addition of TURIN to the tested platforms and specification of passive mode aligns with DPDK power management best practices.
107-130: Verify technical specifications for Solarflare and Linux kernel requirements.The specification of Linux kernel 6.13 and the particular PMD args should be verified to ensure they are current and accurate.
Please verify the following:
- Is Linux kernel 6.13 the correct minimum version for Solarflare x4/x2 on TURIN?
- Are the PMD args
fw_variant=ultra-low-latencyandperf_profile=throughputcorrect for these NICs?- Is there documentation from Solarflare or AMD that details these specific requirements?
You may search the Solarflare and AMD documentation or contact the respective teams to confirm these specifications.
doc/guides/windows_gsg/build_dpdk.rst (2)
48-48: Text rewording is clear and appropriate.The phrasing "latest installer version" is slightly more precise than the original. No issues here.
133-146: No issues identified.The Developer Command Prompt section is well-documented with accurate RST formatting. The referenced Microsoft Learn documentation is currently accessible and actively maintained (last updated 2025-11-18), so the version-specific URL parameter is appropriate and poses no staleness concerns.
examples/vdpa/main.c (3)
25-25: LGTM - Buffer overflow fix!The introduction of
MAX_VDPA_STR_LENand the reducedifacebuffer size fix a potential buffer overflow at line 501. Previously, if a user provided a 127-character path anddevcntreached 1023, thesnprintfat line 501 would write 132 bytes into the 128-byteifnamebuffer. The new sizing ensuresiface(max 122 chars) + port number (max 4 chars) + null terminator fit safely withinMAX_PATH_LEN.Also applies to: 40-40
78-79: Improved safety withsizeof(iface).Using
sizeof(iface)instead of the hardcodedMAX_PATH_LENcorrectly reflects the actual buffer size (123 bytes). This ensuresrte_strscpyproperly limits the copied string length. Note that this reduces the maximum accepted path length from 127 to 122 characters, which is necessary to prevent the buffer overflow at line 501.
78-79: Review comment is incorrect—strcmp at line 78 checks the option name, not path prefix behavior.The strcmp at line 78 parses command-line option names during argument processing, which correctly requires exact matching (e.g., "iface" vs other long options). This is unrelated to the prefix-matching behavior documented for the
--ifaceparameter.The actual prefix behavior persists at line 501–502, where
snprintf(vports[devcnt].ifname, MAX_PATH_LEN, "%s%d", iface, devcnt)concatenates the iface prefix with a device number (0–1023)—socket path construction is unchanged. Documentation remains accurate:--ifaceaccepts a path prefix like/tmp/vhost-user-which becomes/tmp/vhost-user-0,/tmp/vhost-user-1, etc.No behavioral change to the documented API exists. The buffer size calculation (128 - 4 = 124 bytes) also does not match the "122 instead of 127" claim in the review.
Likely an incorrect or invalid review comment.
VERSION (1)
1-1: Version bump to 25.11.0-rc4 is fineSingle-line metadata update; no behavioral implications.
doc/guides/meson.build (1)
15-26: Addinginstall_tag: 'doc'tohtml_guidesis appropriateTagging this custom_target as docs is consistent with API doc targets and should not affect build behavior beyond tagged installs.
examples/meson.build (1)
81-83: Re-enabling-Wformat-truncationdiagnostics for examples is reasonableSetting
default_cflags = machine_argsand dropping any-Wno-format-truncationsuppression tightens warning coverage; just be aware this may expose existing truncation warnings in some examples when building with strict-Werrorconfigurations.examples/server_node_efd/efd_server/main.c (1)
67-89: MAC buffer resized correctly for formatted address + newlineUsing
RTE_ETHER_ADDR_FMT_SIZE + 1foraddressesensures the printed MAC string including the trailing newline and NUL fits without truncation while leaving existing behavior unchanged.doc/guides/sample_app_ug/l3_forward.rst (1)
72-95:--rx-burst/--tx-burstoption docs are clear; verify they match implementation defaultsDocumenting
--rx-burst NPKTSand--tx-burst NPKTSas optional decimal burst sizes with default 32 looks consistent with the described l3fwd refactor to separate Rx/Tx burst sizes. Please just confirm that the l3fwd implementation uses the same option names and a default of 32 for both directions so the doc and code stay in sync.To verify, you can grep the l3fwd source for these options and check the default constants.
Also applies to: 118-121
app/test/test_debug.c (1)
49-49: PPC64--iova-mode=vaaddition and test_args resize look correctExpanding
test_argsto 8 elements and initializingtest_args[7]in both hugepage and no-huge branches avoids uninitialized entries. The PPC64-specific--iova-mode=vafor the--no-hugecase matches the comment about PA failing there, while leaving the hugepage case untouched.It’s worth re-running
debug_autoteston PPC64 with both hugepages enabled and with--no-hugeto confirm the test now passes in both configurations.Also applies to: 157-172
doc/api/meson.build (1)
24-32: Doc install tagging for API Doxygen targets is consistent and low-riskAdding
install_tag: 'doc'to theexamples.dox,doxygen-html, anddoxygen-mantargets aligns API docs with the guides’ tagging and should only affect tagged installs, not build behavior.Also applies to: 70-82, 86-96
drivers/net/ngbe/ngbe_rxtx.c (1)
1920-1945: Simple TX path now correctly sets a non-NULLtx_pkt_preparecallbackAssigning
dev->tx_pkt_prepare = rte_eth_tx_pkt_prepare_dummyfor the simple/no-offload TX path makes the driver conform to the ethdev expectations without adding prep overhead, and avoids NULL-callback issues in tests or generic code paths. This is safe given the simple-path assumptions and keeps behavior unchanged for the full-featured path.doc/guides/rel_notes/release_25_11.rst (1)
349-600: Tested platforms section reads well and follows the documented structureThe new tested-platform matrices (Intel + Intel NICs, Intel + NVIDIA NICs, IBM Power9 + NVIDIA NICs) are well structured, use existing RST conventions (
|reg|etc.), and should render correctly. No blocking issues from a docs perspective.drivers/net/hns3/hns3_rxtx.c (1)
4547-4565: Make tx_pkt_prepare explicitly safe when TX is disabledSetting
eth_dev->tx_pkt_prepare/dev->tx_pkt_preparetorte_eth_tx_pkt_prepare_dummyin the non-started path and inhns3_stop_tx_datapath()removes any risk of a NULL or stale prepare callback while TX is stopped, without affecting the started path (which still useshns3_get_tx_prepare()for real offloads). This is a clean, consistent change with no obvious regressions.If you haven’t already, please confirm against your target DPDK version that
rte_eth_tx_pkt_prepare_dummyis the recommended non-NULL placeholder for drivers when TX is disabled.Also applies to: 4917-4921
drivers/net/txgbe/txgbe_rxtx.c (1)
2337-2368: Explicit dummy tx_pkt_prepare for simple TX path looks correctWiring
dev->tx_pkt_prepare = rte_eth_tx_pkt_prepare_dummyin the simple (offload-free) TX path is consistent with using a no-op prepare function when no TX offloads are in play and avoids leavingtx_pkt_prepareNULL or stale when switching between simple and full-featured paths. The full-featured path continues to usetxgbe_prep_pkts, so behavior with offloads is unchanged.Please double-check in your target DPDK version that using
rte_eth_tx_pkt_prepare_dummyfor offload-free/simple TX paths aligns with the core ethdev API guidance.examples/l3fwd/l3fwd_lpm.c (1)
207-208: LGTM! RX burst size correctly updated.The change from
nb_pkt_per_bursttorx_burst_sizeproperly implements the per-direction burst configuration.examples/l3fwd/l3fwd_em.c (1)
646-647: LGTM! RX burst size correctly updated.The change from
nb_pkt_per_bursttorx_burst_sizeis consistent with the per-direction burst configuration changes.examples/l3fwd/l3fwd_acl.c (1)
1138-1139: LGTM! RX burst size correctly updated.The change from
nb_pkt_per_bursttorx_burst_sizealigns with the per-direction burst configuration.examples/l3fwd/l3fwd_fib.c (1)
241-242: LGTM! RX burst size correctly updated.The change from
nb_pkt_per_bursttorx_burst_sizeis consistent with the per-direction burst configuration approach.examples/l3fwd/l3fwd_common.h (2)
28-29: LGTM! External declarations properly added.The extern declarations for
rx_burst_sizeandtx_burst_sizecorrectly expose the new per-direction burst size globals.
77-77: LGTM! TX burst threshold correctly updated.The change from
MAX_TX_BURSTtotx_burst_sizeproperly implements runtime-configurable TX burst behavior.examples/l3fwd/l3fwd.h (1)
115-115: LGTM! Declaration correctly updated.The replacement of
nb_pkt_per_burstwithrx_burst_sizealigns with the per-direction burst configuration.examples/l3fwd/main.c (6)
60-62: LGTM! Per-direction burst sizes properly introduced.The new
rx_burst_sizeandtx_burst_sizeglobals with default initialization replace the singlenb_pkt_per_burst, enabling independent RX/TX burst configuration.
762-762: LGTM! Command-line options properly added.The new options (
--rx-burst,--tx-burst,--eth-link-speed) are correctly integrated with proper constant definitions, enums, and long options table entries.Also applies to: 784-785, 795-795, 817-818, 824-824, 847-848
404-406: LGTM! Usage text correctly updated.The help text properly documents the new per-direction burst options and eth-link-speed option with clear descriptions and default values.
Also applies to: 428-436, 468-469
930-940: LGTM! Link speed option properly validated.The eth-link-speed parsing correctly validates against unsupported fixed speeds and uses
rte_eth_speed_bitflagto set the configuration.
949-955: LGTM! Burst size parsing correctly invoked.The RX and TX burst options are parsed with the appropriate
is_rx_burstflag, routing to the correct burst size variable.
1690-1690: LGTM! Runtime logging added.The log message provides useful visibility into the configured RX and TX burst sizes at startup.
drivers/common/mlx5/mlx5_devx_cmds.h (1)
116-127: Header-length flex/graph flags extension looks correctThe new
header_length_field_mode_waandheader_length_field_offset_modebitfields integrate cleanly with the existing flex/graph attributes and keep prior members’ ordering and semantics intact. No structural or API concerns from this header alone.Also applies to: 646-653
doc/guides/nics/mlx5.rst (10)
648-651: dv_xmeta_en mode 3 tunnel-offload limitation is clearly documentedThe added note that tunnel offload isn’t available via the synchronous flow API when using HW steering makes the interaction between
dv_xmeta_en=3, HWS, and API choice explicit and should help avoid misconfiguration.
689-700: dv_flow_en default behavior clarification matches expected semanticsDocumenting that
dv_flow_endefaults to1(DV steering) when supported, and otherwise falls back to0(legacy Verbs), aligns the text with the driver’s typical behavior and makes the implicit default explicit.
824-837: allow_duplicate_pattern default of 1 is now explicitStating that
allow_duplicate_patterndefaults to1is useful for users reasoning about rule insertion behavior on non-root tables; the description remains internally consistent.
1366-1372: rte_flow_configure() requirement for async API is well capturedThe new note that applications must call
rte_flow_configure()before creating rules with the asynchronous flow API (and that doing so is recommended for the synchronous API) reflects how HWS is configured in practice and should prevent subtle misuses.
2396-2403: Sync API + HWS representor limitations are clearly spelled outThe added bullets for synchronous API with HWS (no
PORT_REPRESENTORitem support, no transfer rules on representor ports, and proxy-port rules matching all ports without explicit represented-port matching) accurately set expectations for representor use-cases under HWS.
2593-2596: Flow MARK item restriction under sync API + HWS is well documentedClarifying that
RTE_FLOW_ITEM_TYPE_MARK(16‑bit match) isn’t supported with the synchronous API under HWS, and thatRTE_FLOW_ITEM_TYPE_META(32‑bit match) should be used instead, is an important nuance and is described succinctly.
2812-2814: One-shot reporting of aged flows under sync API + HWS is clearStating that, with the synchronous API and HWS, aged flows are reported only once is an important behavioral detail for applications relying on periodic polling; the wording is straightforward.
2947-2950: HWS metering limitations and 16e6-object cap are now explicitDocumenting that only
RTE_FLOW_ACTION_TYPE_METER_MARK(notMETER) is supported under HWS, and that HW quota + meter objects are capped at16e6, makes the constraints around advanced metering much clearer to users configuring large rule sets.
3816-3821: GRE c_rsvd0_ver limitation under sync API + HWS is well scopedThe added limitation that
c_rsvd0_vercannot be matched in group 0 when using the synchronous API with HWS is precise and localized, and fits consistently with other HWS-specific constraints documented above.
2769-2772: Documentation version requirements confirmed accurate — no changes neededThe web search confirms that the documented version thresholds (Linux ≥ 6.4, rdma-core ≥ 60.0 for upstream drivers, or DOCA ≥ 3.2.0 for NVIDIA/DOCA-provided drivers) match the mlx5 PMD implementation requirements for using
RTE_FLOW_ACTION_TYPE_COUNTon root tables in the 25.11 cycle. The documentation is properly aligned with the implementation and does not require modification.drivers/common/mlx5/mlx5_prm.h (4)
1600-1602: Steering logic format constants move to macros looks fineSwitching
MLX5_STEERING_LOGIC_FORMAT_CONNECTX_5/6DXfrom an enum to#defines preserves the values and keeps them usable as plain integer constants; no layout or ABI impact inside this header.
2118-2120: Newheader_length_field_offset_modebit keeps node-cap layout intactSplitting
reserved_at_78[0x3]intoreserved_at_78[0x1],header_length_field_offset_mode[0x1], andreserved_at_79[0x1]adds the new capability bit without changing the total 3‑bit span, so following fields keep their original offsets.
2351-2357: Flow-table ESW caps reserved-region enlargement is structurally safeOnly reserved ranges around
ft_header_modify_esw_fdb/ft_field_support_2_esw_fdbare adjusted (reserved_at_0andreserved_at_C00), with no semantic fields moved or resized, so user-visible capability bits remain at stable offsets. This should be safe as long as it matches the underlying PRM, which it appears to.
4979-4992: Flex-parse caps gainheader_length_field_offset_modewithout shifting layoutLike the node-cap struct, the flex-parse struct replaces a 3‑bit reserved range at 0x178 with
reserved_at_178[0x1],header_length_field_offset_mode[0x1], andreserved_at_17a[0x1]. Total width is unchanged, so existing offsets for subsequent fields are preserved while exposing the new capability.drivers/net/mlx5/mlx5_flow_flex.c (1)
555-576: Offset WA gating and mode bookkeeping look consistentConditioning the WA block on
attr->header_length_field_mode_waand recording the chosen mode vianode->header_length_field_offset_mode = !attr->header_length_field_mode_wa;correctly ties the header-length offset handling to the HCA capabilities. No functional issues spotted.drivers/net/mlx5/mlx5.c (1)
1490-1499: Defaulting dv_flow_en and allow_duplicate_pattern to 1 changes behavior
mlx5_shared_dev_ctx_args_config()now unconditionally defaults:config->dv_flow_en = 1; config->allow_duplicate_pattern = 1;before applying devargs and capability-based adjustments. This effectively enables DV flow steering and duplicate-pattern allowance by default wherever supported.
Please double-check that this behavioral change is intentional and that no existing deployments relied on the previous defaults (e.g., verbs-only flows unless
dv_flow_enwas explicitly requested).drivers/common/mlx5/mlx5_devx_cmds.c (1)
785-795: FLEX_PARSE_GRAPH now receives header_length_field_offset_mode from graph-node attrProgramming
header_length_field_offset_modeintoparse_graph_flexfromdata->header_length_field_offset_modekeeps the DevX object in sync with the higher-level flex configuration and the HCA capability wiring. Callers that don’t care about the distinction still see a safe default of 0 from zero-initialized structs.
|
|
||
| AMD EPYC SoC support UNCORE power functions via ``rte_power_uncore`` from `25.03` onwards. | ||
| These are tested and validated on GENOA, SIENA and TURIN. | ||
| Please refer the tuning guides to enable ``HSMP`` and DPDK power management guide. |
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.
Fix grammar and provide clearer guidance for Uncore setup.
Line 104 has a grammatical error ("refer the" → "refer to the") and the instruction is vague. Instead of only directing readers to external documentation, provide actionable next steps or a brief summary of what needs to be configured.
Apply this diff to fix the grammar:
-Please refer the tuning guides to enable ``HSMP`` and DPDK power management guide.
+Please refer to the tuning guides to enable ``HSMP`` and DPDK power management.Consider expanding this with a brief actionable summary, e.g.:
+AMD EPYC SoC support UNCORE power functions via ``rte_power_uncore`` from `25.03` onwards.
+These are tested and validated on GENOA, SIENA and TURIN.
+To use this feature, enable HSMP in BIOS and configure DPDK power management as detailed in the power management guide.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Please refer the tuning guides to enable ``HSMP`` and DPDK power management guide. | |
| Please refer to the tuning guides to enable ``HSMP`` and DPDK power management. |
🤖 Prompt for AI Agents
In doc/guides/linux_gsg/amd_platform.rst at line 104, fix the grammar ("refer
the" → "refer to the") and replace the vague instruction with a concise
actionable note: change the sentence to refer to the tuning guides and add a
short summary of the exact Uncore/HSMP and DPDK power-management steps (e.g.,
enable HSMP in BIOS/kernel, set required sysfs knobs or driver params, and
enable DPDK PM settings or link to the specific DPDK power-management section).
Ensure the new line reads clearly and includes both the corrected phrasing and
1–2 brief actionable steps or pointers for what to configure.
|
|
||
| Download and install the clang compiler from | ||
| `LLVM website <http://releases.llvm.org/download.html>`_. | ||
| `LLVM website <https://releases.llvm.org>`_. |
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.
🧩 Analysis chain
Verify LLVM version update and GitHub URL validity.
The update from LLVM 7.0.1 to 19.1.0 is a significant version jump. While it exceeds the minimum requirement (14.0.0), please confirm:
- The GitHub releases URL is the canonical and stable download source for LLVM Windows binaries.
- LLVM 19.1.0 is compatible with DPDK Windows builds and has no breaking changes compared to earlier versions.
- Whether version 19.1.0 is the recommended version or if documentation should specify a broader range.
Also applies to: 32-32
🌐 Web query:
Is LLVM 19.1.0 the current recommended version for Windows development? Are there any known compatibility issues with DPDK?
💡 Result:
Short answer: No — LLVM 19.1.0 is not the current recommended release for Windows development. LLVM 20 (20.1.0) is available and is the newer stable release; check the LLVM release notes/announcements and pick the latest stable LLVM/Clang for new projects. [1][2]
DPDK-specific: DPDK supports building on Windows with Clang/LLVM (and either the MSVC linker or MinGW), but the Windows port historically tests with a range of Clang versions (older test reports commonly show Clang 14–15) and the project has a number of Windows-specific caveats (meson build options, clang vs clang-cl differences, PDB/debug-info handling, and required linker/toolchain choices). There are no widely‑reported, fundamental incompatibilities that single out LLVM 19.1.0 as unsupported, but users have run into practical issues (meson/flag mismatches, clang-cl vs clang behavior for debug/PDB, and linker/CRT handling) that require specific build flags or MSVC/MinGW choices. Test reports and developer discussion recommend using the toolchain/version that DPDK CI/test reports target or the latest supported Clang and to follow the DPDK Windows build notes. [3][4][5][6]
Recommendations:
- Use the latest stable LLVM/Clang (e.g., LLVM 20.x as of the 2025 releases) unless you have a specific reason to pin 19.1.0. [1]
- When building DPDK on Windows, follow DPDK Windows build documentation and CI/test-reported compiler versions; prefer the Clang flavour the DPDK Windows docs/meson files expect (and be careful about using clang vs clang-cl and the MSVC linker). Test your chosen LLVM/Clang with your DPDK version before rolling it out. [3][4][5]
Sources:
- LLVM 20.1.0 release notes / announcements (latest stable release). [1]
- LLVM 19.1.0 release notes. [2]
- DPDK Windows support / Windows port historical notes. [3]
- DPDK dev threads about Windows toolchain (clang/clang-cl, MinGW, linker, meson). [4][5]
- DPDK test reports showing compilers used in CI (examples). [6]
Update LLVM version to current stable release (20.1.0 instead of 19.1.0).
The documentation recommends LLVM 19.1.0, but LLVM 20.1.0 is the current stable release as of 2025. While there are no widely-reported fundamental incompatibilities between LLVM 19.1.0 and DPDK, the documentation should reflect current best practices. The recommendation is to use the latest stable LLVM/Clang unless you have a specific reason to pin a version.
Update lines 29 and 32 to reference LLVM 20.1.0 or specify "latest stable version" to keep the documentation current without frequent updates.
🤖 Prompt for AI Agents
In doc/guides/windows_gsg/build_dpdk.rst around lines 29 to 32, the guide
currently references LLVM 19.1.0; update the text at lines 29 and 32 to
reference LLVM 20.1.0 (or change the phrasing to "latest stable LLVM/Clang") so
the recommended LLVM version is current; ensure the LLVM website link remains
and adjust any version-specific examples or download instructions to match
20.1.0 if present.
| A good option to choose is the MSI installer for both meson and ninja together:: | ||
|
|
||
| http://mesonbuild.com/Getting-meson.html#installing-meson-and-ninja-with-the-msi-installer%22 | ||
| http://mesonbuild.com/Getting-meson.html#installing-meson-and-ninja-with-the-msi-installer |
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.
🧩 Analysis chain
Verify Meson MSI installer URL is still valid.
Please confirm the Meson download page URL remains correct and that the MSI installer option is still available at this location.
🌐 Web query:
Is the current Meson MSI installer download page still available at mesonbuild.com?
💡 Result:
Short answer: No — mesonbuild.com no longer hosts the MSI file itself; the site directs Windows users to the project's GitHub Releases page for the MSI installer. [1][2]
Sources:
- Meson "Getting Meson" (states the MSI installer is available on the GitHub release page). [1]
- Meson "SimpleStart"/Windows (says the Windows .msi installer can be downloaded from the Releases page). [2]
Update Meson MSI installer documentation to reflect current distribution method.
The MSI installer is no longer hosted directly at mesonbuild.com; the site now directs Windows users to the project's GitHub Releases page. Update the URL in line 69 to point to the GitHub Releases page instead of mesonbuild.com/Getting-meson.html, or revise the documentation text to reflect the current download process.
🤖 Prompt for AI Agents
In doc/guides/windows_gsg/build_dpdk.rst around line 69, the link currently
points to mesonbuild.com/Getting-meson.html which no longer hosts the MSI;
update the URL to Meson's GitHub Releases page
(https://github.com/mesonbuild/meson/releases) or change the sentence to state
that the Meson MSI installer is distributed via the project's GitHub Releases
page and provide that link.
| /* Should never happen but check so that pool name won't be too long. */ | ||
| if (lcore > RTE_MAX_LCORE || queue > RTE_MAX_QUEUES_PER_PORT) { | ||
| RTE_LOG(ERR, IP_RSMBL, "invalid lcore %u or queue %u", | ||
| lcore, queue); | ||
| return -1; | ||
| } |
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.
🧩 Analysis chain
Fix the comparison operator and verify the constant.
This validation has several concerns:
-
Comparison operator: The check uses
>, but ifRTE_MAX_LCORErepresents the maximum count of lcores (typical in DPDK), valid lcore IDs range from 0 toRTE_MAX_LCORE - 1. The current check allowslcore == RTE_MAX_LCORE, which would be invalid. Use>=instead. -
Missing newline: The error message at line 889 is missing a trailing newline character, inconsistent with other
RTE_LOGerror messages in this file (see lines 780, 804, 835, 865, 899). -
Constant verification needed: The constant
RTE_MAX_QUEUES_PER_PORTneeds verification—it may not be a standard DPDK constant.
Apply this diff:
- if (lcore > RTE_MAX_LCORE || queue > RTE_MAX_QUEUES_PER_PORT) {
- RTE_LOG(ERR, IP_RSMBL, "invalid lcore %u or queue %u",
+ if (lcore >= RTE_MAX_LCORE || queue >= RTE_MAX_QUEUES_PER_PORT) {
+ RTE_LOG(ERR, IP_RSMBL, "invalid lcore %u or queue %u\n",
lcore, queue);
return -1;
}Verify the existence and correct usage of RTE_MAX_QUEUES_PER_PORT:
🏁 Script executed:
#!/bin/bash
# Description: Verify RTE_MAX_QUEUES_PER_PORT constant definition and usage
# Search for the constant definition in DPDK headers
rg -n "define\s+RTE_MAX_QUEUES_PER_PORT" --type c --type h
# Search for other usages of this constant to understand the intended semantics
rg -n "RTE_MAX_QUEUES_PER_PORT" --type c --type h -C2Length of output: 25047
Fix the comparison operators to match DPDK conventions and add missing newline.
The validation logic uses > operators, but throughout the DPDK codebase—including library code and drivers—the standard pattern for bounds checking against RTE_MAX_QUEUES_PER_PORT and RTE_MAX_LCORE is >=. Since RTE_MAX_QUEUES_PER_PORT is 1024, valid queue IDs are 0–1023, so queue > 1024 would incorrectly allow queue == 1024. The operators should be >= to reject invalid indices at or beyond the maximum.
Additionally, the error message at line 889 is missing a trailing newline character, inconsistent with other RTE_LOG error calls in this file.
Apply this diff:
- if (lcore > RTE_MAX_LCORE || queue > RTE_MAX_QUEUES_PER_PORT) {
- RTE_LOG(ERR, IP_RSMBL, "invalid lcore %u or queue %u",
+ if (lcore >= RTE_MAX_LCORE || queue >= RTE_MAX_QUEUES_PER_PORT) {
+ RTE_LOG(ERR, IP_RSMBL, "invalid lcore %u or queue %u\n",
lcore, queue);
return -1;
}🤖 Prompt for AI Agents
In examples/ip_reassembly/main.c around lines 887 to 892, the bounds check uses
'>' but should use '>=' to follow DPDK conventions (reject indices equal to the
max), and the RTE_LOG message is missing a trailing newline; change both
comparisons to '>=' and append "\n" to the RTE_LOG format string so the invalid
lcore/queue case is correctly rejected and the log line ends with a newline.
| if (unlikely(len == rx_burst_size)) { | ||
| send_burst(qconf, rx_burst_size, port); |
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.
🧩 Analysis chain
Verify: Using RX burst size for TX buffering threshold.
The code uses rx_burst_size as the threshold for flushing the TX buffer. While this maintains backward compatibility with the previous nb_pkt_per_burst behavior, consider whether tx_burst_size would be more semantically appropriate for controlling TX buffer flushing.
🏁 Script executed:
#!/bin/bash
# Search for other uses of tx_burst_size to understand the intended design
rg -n -C3 'tx_burst_size' --type=cLength of output: 3088
I need to examine the context of the code in l3fwd.h to understand the function and verify the semantic concern.
Based on the search results and the shell script output, the review comment's concern is valid and supported by recent DPDK changes.
Recent patches change the condition from if (unlikely(len == MAX_PKT_BURST)) to if (unlikely(len == tx_pkt_burst)), confirming that tx_burst_size is the more semantically appropriate choice for controlling TX buffer flushing.
While backward compatibility is maintained (both rx_burst_size and tx_burst_size initialize to DEFAULT_PKT_BURST by default), using rx_burst_size for a TX buffer operation is semantically incorrect. The code should use tx_burst_size to properly control when the TX buffer is flushed, as rx_burst_size governs RX operations and should not be mixed with TX logic.
Replace rx_burst_size with tx_burst_size in the condition at lines 151-152.
if (unlikely(len == tx_burst_size)) {
send_burst(qconf, tx_burst_size, port);🤖 Prompt for AI Agents
In examples/l3fwd/l3fwd.h around lines 151-152 the TX buffer-flush condition
incorrectly uses rx_burst_size (an RX parameter) causing a semantic mismatch;
replace rx_burst_size with tx_burst_size in the if condition and in the
send_burst call so the TX buffer is flushed based on the TX burst size (i.e.,
use tx_burst_size for both the comparison and the send_burst length).
| * Used as ring name, so upper limit is ring name size. | ||
| */ | ||
| static char buffer[sizeof(MP_NODE_RXQ_NAME) + 2]; | ||
| static char buffer[RTE_RING_NAMESIZE]; |
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.
🧩 Analysis chain
Good fix: Using RTE_RING_NAMESIZE properly aligns the buffer with its intended use.
The buffer is used to generate ring names, so using the ring name size constant is the correct approach. This ensures the buffer is appropriately sized regardless of the MP_NODE_RXQ_NAME template.
However, verify that RTE_RING_NAMESIZE is available. It's defined in rte_ring_core.h (typically via rte_ring.h), which isn't explicitly included here.
🏁 Script executed:
#!/bin/bash
# Check if RTE_RING_NAMESIZE is transitively available through existing includes
# or if rte_ring.h needs to be explicitly added
echo "=== Checking if rte_ring.h is transitively included ==="
# Check rte_hash.h for ring includes
fd -t f 'rte_hash\.h$' --exec grep -l 'rte_ring' {} \; 2>/dev/null || echo "No ring include in rte_hash.h"
# Check rte_hash_crc.h for ring includes
fd -t f 'rte_hash_crc\.h$' --exec grep -l 'rte_ring' {} \; 2>/dev/null || echo "No ring include in rte_hash_crc.h"
echo ""
echo "=== Checking how other files using RTE_RING_NAMESIZE handle includes ==="
rg -l 'RTE_RING_NAMESIZE' --type c --type-add 'c:*.h' | head -5 | xargs -I {} sh -c 'echo "--- {} ---" && grep -E "^#include.*rte_ring" {} || echo "(no rte_ring include)"'
echo ""
echo "=== Verifying this file compiles (check if it is used in build) ==="
rg -l 'common\.h' examples/server_node_efd/ 2>/dev/null | head -5Length of output: 721
Add missing include: #include <rte_ring.h> required for RTE_RING_NAMESIZE
The buffer size change to RTE_RING_NAMESIZE is correct for the ring name use case, but verification shows that RTE_RING_NAMESIZE is not available through the current includes (rte_hash.h and rte_hash_crc.h do not transitively include ring headers). Other DPDK files using RTE_RING_NAMESIZE (e.g., rte_mempool.h, rte_rcu_qsbr.h) explicitly include <rte_ring.h>. This file will fail to compile without it.
🤖 Prompt for AI Agents
In examples/server_node_efd/shared/common.h around lines 61 to 63, the code uses
RTE_RING_NAMESIZE but does not include the header that defines it; add an
explicit #include <rte_ring.h> at the top of the file (with the other rte
includes) so RTE_RING_NAMESIZE is available and the file compiles cleanly.
NOTE: This is an auto submission for "test/debug: fix test failure on ppc64".
See "http://patchwork.dpdk.org/project/dpdk/list/?series=36786" for details.
Summary by Sourcery
Adjust burst configuration, device capabilities handling, and documentation to improve robustness and platform compatibility across examples and Mellanox/other PMDs, including fixes for debug tests on ppc64.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Summary by CodeRabbit
New Features
--rx-burst,--tx-burstcommand-line options for l3fwd sample app to control packet burst sizes.--eth-link-speedoption for l3fwd and l3fwd-power apps to enforce Ethernet link speeds.Bug Fixes
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.