[PWCI] "bus/fslmc: fix warning in device scan" #489
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>
With latest glibc (Fedora Rawhide):
../drivers/bus/fslmc/fslmc_bus.c: In function ‘rte_fslmc_parse’:
../drivers/bus/fslmc/fslmc_bus.c:298:15: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
298 | t_ptr = strchr(sep, '.');
| ^
Fixes: e67a616 ("bus/fslmc: support device iteration")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
Signed-off-by: 0-day Robot <robot@bytheb.org>
Reviewer's GuideThis PR makes multiple correctness and robustness fixes across DPDK examples and drivers, introduces explicit RX/TX burst-size configuration and link-speed forcing in l3fwd apps, updates mlx5 flex-parser handling for new firmware capabilities while simplifying steering configuration, tightens several buffer-size and naming constraints, and standardizes tx_pkt_prepare initialization in multiple PMDs, while also fixing a const-correctness warning in the fslmc bus and improving documentation install tagging. Sequence diagram for l3fwd argument parsing and burst/link speed setupsequenceDiagram
actor User
participant L3fwdMain as L3fwd_main
participant CliParser as parse_args
participant PortConf as port_conf
User->>L3fwdMain: run l3fwd with CLI options
L3fwdMain->>CliParser: parse_args(argc, argv)
loop process_long_options
CliParser->>CliParser: handle CMD_LINE_OPT_RX_QUEUE_SIZE_NUM
CliParser->>CliParser: parse_queue_size(optarg, &nb_rxd, 1)
CliParser->>CliParser: handle CMD_LINE_OPT_TX_QUEUE_SIZE_NUM
CliParser->>CliParser: parse_queue_size(optarg, &nb_txd, 0)
CliParser->>CliParser: handle CMD_LINE_OPT_PKT_RX_BURST_NUM
CliParser->>CliParser: parse_pkt_burst(optarg, true, &rx_burst_size)
CliParser->>CliParser: handle CMD_LINE_OPT_PKT_TX_BURST_NUM
CliParser->>CliParser: parse_pkt_burst(optarg, false, &tx_burst_size)
CliParser->>CliParser: handle CMD_LINK_OPT_ETH_LINK_SPEED_NUM
CliParser->>PortConf: apply_link_speed(speed_num)
PortConf->>PortConf: port_conf.link_speeds = rte_eth_speed_bitflag(speed_num, 0)
end
CliParser-->>L3fwdMain: return success or error
L3fwdMain->>L3fwdMain: log Using Rx burst rx_burst_size Tx burst tx_burst_size
par RX_loops
L3fwdMain->>RxLoopACL: acl_main_loop
loop per_queue_acl
RxLoopACL->>RxLoopACL: rte_eth_rx_burst(..., rx_burst_size)
end
L3fwdMain->>RxLoopEM: em_main_loop
loop per_queue_em
RxLoopEM->>RxLoopEM: rte_eth_rx_burst(..., rx_burst_size)
end
L3fwdMain->>RxLoopFIB: fib_main_loop
loop per_queue_fib
RxLoopFIB->>RxLoopFIB: rte_eth_rx_burst(..., rx_burst_size)
end
L3fwdMain->>RxLoopLPM: lpm_main_loop
loop per_queue_lpm
RxLoopLPM->>RxLoopLPM: rte_eth_rx_burst(..., rx_burst_size)
end
and TX_helpers
L3fwdMain->>TxHelperSingle: send_single_packet
TxHelperSingle->>TxHelperSingle: if len == rx_burst_size then send_burst(..., rx_burst_size)
L3fwdMain->>TxHelperX4: send_packetsx4
TxHelperX4->>TxHelperX4: if num >= tx_burst_size and len == 0 then rte_eth_tx_burst(...)
end
Sequence diagram for mlx5 flex parser setup with new header length offset modesequenceDiagram
participant App
participant QueryNodeCap as mlx5_devx_cmd_query_hca_parse_graph_node_cap
participant FlexAttr as mlx5_hca_flex_attr
participant FlexTranslate as mlx5_flex_translate_length
participant AllocSRH as mlx5_alloc_srh_flex_parser
participant CreateFlex as mlx5_devx_cmd_create_flex_parser
App->>QueryNodeCap: mlx5_devx_cmd_query_hca_parse_graph_node_cap(ctx, attr)
QueryNodeCap->>FlexAttr: set header_length_mask_width
QueryNodeCap->>FlexAttr: set header_length_field_mode_wa = !header_length_field_offset_mode
QueryNodeCap-->>App: return
App->>FlexTranslate: mlx5_flex_translate_length(attr, field, node, error)
alt FIELD_MODE_NORMAL
FlexTranslate->>FlexTranslate: compute offset and msb
alt attr.header_length_field_mode_wa && msb < header_length_mask_width
FlexTranslate-->>App: error if unsupported combination
else attr.header_length_field_mode_wa == 0 or valid
FlexTranslate->>FlexTranslate: node.header_length_field_mask = mask
FlexTranslate->>FlexTranslate: node.header_length_field_shift = shift
FlexTranslate->>FlexTranslate: node.header_length_field_offset = offset
FlexTranslate->>FlexTranslate: node.header_length_field_offset_mode = !attr.header_length_field_mode_wa
FlexTranslate-->>App: success
end
else FIELD_MODE_BITMASK
FlexTranslate->>FlexTranslate: handle bitmask mode
FlexTranslate->>FlexTranslate: node.header_length_field_offset_mode = !attr.header_length_field_mode_wa
FlexTranslate-->>App: success or error
end
App->>AllocSRH: mlx5_alloc_srh_flex_parser(dev)
alt attr.header_length_field_mode_wa
AllocSRH->>AllocSRH: legacy firmware path
AllocSRH->>AllocSRH: node.header_length_field_offset = 8 adjusted by header_length_mask_width
else !attr.header_length_field_mode_wa
AllocSRH->>AllocSRH: new firmware path
AllocSRH->>AllocSRH: node.header_length_field_offset = 12
end
AllocSRH-->>App: configured node
App->>CreateFlex: mlx5_devx_cmd_create_flex_parser(ctx, data)
CreateFlex->>CreateFlex: set header_length_mode
CreateFlex->>CreateFlex: set header_length_field_offset_mode from data
CreateFlex-->>App: flex parser object
Updated class diagram for l3fwd burst size configurationclassDiagram
class L3fwdGlobals {
+uint32_t rx_burst_size
+uint32_t tx_burst_size
+uint32_t mb_mempool_cache_size
+uint16_t nb_rxd
+uint16_t nb_txd
}
class CliParser {
+int parse_args(int argc, char** argv)
+void parse_queue_size(const char* optarg, uint16_t* nb_desc, int is_rx)
+void parse_mbcache_size(const char* optarg)
+void parse_pkt_burst(const char* optarg, bool is_rx_burst, uint32_t* burst_sz)
+void print_usage(const char* prgname)
}
class PortConfig {
+struct rte_eth_conf port_conf
+void apply_link_speed(int speed_num)
}
class RxTxLoops {
+void acl_main_loop(void* dummy)
+void em_main_loop(void* dummy)
+void fib_main_loop(void* dummy)
+void lpm_main_loop(void* dummy)
}
class TxHelpers {
+void send_single_packet(struct lcore_conf* qconf, struct rte_mbuf* m, uint16_t port)
+void send_packetsx4(struct lcore_conf* qconf, uint16_t port, struct rte_mbuf** m, uint16_t nb_pkts)
}
L3fwdGlobals <.. CliParser : updates
L3fwdGlobals <.. RxTxLoops : reads
L3fwdGlobals <.. TxHelpers : reads
PortConfig <.. CliParser : configured_by
CliParser ..> PortConfig : set link_speeds via apply_link_speed
CliParser ..> L3fwdGlobals : set rx_burst_size
CliParser ..> L3fwdGlobals : set tx_burst_size
RxTxLoops ..> L3fwdGlobals : use rx_burst_size for rte_eth_rx_burst
TxHelpers ..> L3fwdGlobals : use rx_burst_size and tx_burst_size
Updated class diagram for mlx5 flex parser and HCA capability structuresclassDiagram
class mlx5_hca_flex_attr {
+uint8_t sample_tunnel_outer:1
+uint8_t sample_tunnel_inner:1
+uint8_t sample_tunnel_inner2:1
+uint8_t zero_size_supported:1
+uint8_t sample_id_in_out:1
+uint8_t header_length_field_mode_wa:1
+uint16_t max_base_header_length
+uint8_t max_sample_base_offset
+uint16_t max_next_header_offset
+uint8_t header_length_mask_width
}
class mlx5_devx_graph_node_attr {
+uint16_t header_length_base_value
+uint8_t header_length_field_shift
+uint16_t header_length_field_offset
+uint8_t header_length_field_offset_mode:1
+uint32_t header_length_field_mask
+uint16_t next_header_field_offset
+uint8_t next_header_field_size
}
class mlx5_ifc_parse_graph_node_cap_bits {
+uint8_t max_num_arc_in
+uint8_t max_num_arc_out
+uint8_t max_num_sample
+uint8_t header_length_field_offset_mode:1
}
class mlx5_ifc_parse_graph_flex_bits {
+uint8_t header_length_mode
+uint16_t header_length_base_value
+uint8_t header_length_field_shift
+uint16_t header_length_field_offset
+uint8_t header_length_field_offset_mode:1
+uint8_t next_header_field_size
+uint32_t header_length_field_mask
}
class mlx5_devx_cmd_query_hca_parse_graph_node_cap {
+int mlx5_devx_cmd_query_hca_parse_graph_node_cap(void* ctx, struct mlx5_hca_flex_attr* attr)
}
class mlx5_flex_translate_length {
+int mlx5_flex_translate_length(struct mlx5_hca_flex_attr* attr, struct rte_flow_item_flex_field* field, struct mlx5_devx_graph_node_attr* node, struct rte_flow_error* error)
}
class mlx5_devx_cmd_create_flex_parser {
+void* mlx5_devx_cmd_create_flex_parser(void* ctx, struct mlx5_devx_graph_node_attr* data)
}
class mlx5_alloc_srh_flex_parser {
+int mlx5_alloc_srh_flex_parser(struct rte_eth_dev* dev)
}
mlx5_ifc_parse_graph_node_cap_bits <.. mlx5_devx_cmd_query_hca_parse_graph_node_cap : reads_from
mlx5_devx_cmd_query_hca_parse_graph_node_cap ..> mlx5_hca_flex_attr : populates
mlx5_hca_flex_attr <.. mlx5_flex_translate_length : reads
mlx5_flex_translate_length ..> mlx5_devx_graph_node_attr : configures
mlx5_devx_graph_node_attr <.. mlx5_devx_cmd_create_flex_parser : reads
mlx5_devx_cmd_create_flex_parser ..> mlx5_ifc_parse_graph_flex_bits : writes_fields
mlx5_hca_flex_attr <.. mlx5_alloc_srh_flex_parser : reads header_length_field_mode_wa
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughA version bump from 25.11.0-rc3 to 25.11.0-rc4 accompanies documentation updates (copyright years, platform support, hardware tables), MLX5 flex parser and hardware steering field additions, consistent tx_pkt_prepare callback refactoring across multiple drivers, and L3 forwarding sample app burst size configuration refactoring. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
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 |
![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 and found some issues that need to be addressed.
- In l3fwd, send_single_packet now uses rx_burst_size as the TX flush threshold instead of the previous MAX_TX_BURST (MAX_PKT_BURST/2), which changes the batching behavior; consider whether this semantic change is intentional or if a separate tx threshold should be kept.
- In ip_reassembly setup_queue_tbl(), the bounds check on lcore and queue should likely use >= instead of > against RTE_MAX_LCORE and RTE_MAX_QUEUES_PER_PORT, since those macros represent limits and the current check still allows an out-of-range index equal to the max.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In l3fwd, send_single_packet now uses rx_burst_size as the TX flush threshold instead of the previous MAX_TX_BURST (MAX_PKT_BURST/2), which changes the batching behavior; consider whether this semantic change is intentional or if a separate tx threshold should be kept.
- In ip_reassembly setup_queue_tbl(), the bounds check on lcore and queue should likely use >= instead of > against RTE_MAX_LCORE and RTE_MAX_QUEUES_PER_PORT, since those macros represent limits and the current check still allows an out-of-range index equal to the max.
## Individual Comments
### Comment 1
<location> `examples/l3fwd/l3fwd.h:115` </location>
<code_context>
extern uint32_t max_pkt_len;
-extern uint32_t nb_pkt_per_burst;
+extern uint32_t rx_burst_size;
extern uint32_t mb_mempool_cache_size;
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Tx flush threshold in send_single_packet should likely use tx_burst_size instead of rx_burst_size to match the new CLI separation.
With separate rx_burst_size and tx_burst_size now introduced, send_single_packet() still uses rx_burst_size as the TX flush threshold, while send_packetsx4() uses tx_burst_size. As a result, the single-packet path’s “enough packets to send” decision is driven by the RX setting rather than the user-configured --tx-burst value, which is misleading. Please switch send_single_packet() to use tx_burst_size so both paths consistently respect the TX burst configuration.
Suggested implementation:
```c
extern uint32_t max_pkt_len;
```
To fully implement your suggestion, you will also need to:
1) Declare the tx_burst_size variable in the corresponding .c file where rx_burst_size is defined, for example:
- Replace the old definition:
- uint32_t nb_pkt_per_burst;
- With:
- uint32_t rx_burst_size;
- uint32_t tx_burst_size;
2) Update send_single_packet() (likely in examples/l3fwd/l3fwd_event.c or l3fwd_lpm.c / l3fwd_em.c depending on implementation) so that:
- Any condition like:
- if (nb_tx == rx_burst_size) { ... }
or
- if (tx_buffer->count >= rx_burst_size) { ... }
or any “TX flush threshold” check currently comparing against rx_burst_size
- Is changed to compare against tx_burst_size instead:
- if (nb_tx == tx_burst_size) { ... }
or
- if (tx_buffer->count >= tx_burst_size) { ... }
3) Ensure that the CLI parsing code that currently sets the TX burst parameter:
- Stores the user-provided --tx-burst value into tx_burst_size (instead of nb_pkt_per_burst or rx_burst_size).
4) Verify that send_packetsx4() is already using tx_burst_size, so both send_single_packet() and send_packetsx4() consistently respect the TX burst configuration.
</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 lcore/queue bounds check is likely off by one and the log message could be more informative.
The guard should likely use `>=` since `RTE_MAX_LCORE` and `RTE_MAX_QUEUES_PER_PORT` are counts and valid indices are typically `[0, N-1]`. As written, `lcore == RTE_MAX_LCORE` or `queue == RTE_MAX_QUEUES_PER_PORT` would still pass the check and may be out of range:
if (lcore >= RTE_MAX_LCORE || queue >= RTE_MAX_QUEUES_PER_PORT) {
Also, consider adding a newline to the log message for consistency with other logs:
RTE_LOG(ERR, IP_RSMBL, "invalid lcore %u or queue %u\n", lcore, queue);
</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>
**issue (typo):** Correct vendor name capitalization for VMware.
Use "VMware ESXi 8.0.3" here to match the vendor’s official capitalization.
```suggestion
* Ubuntu 24.04 LTS
* Ubuntu 24.04.3 LTS
* VMware ESXi 8.0.3
```
</issue_to_address>
### Comment 4
<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 "AMD EPYC SoC supports UNCORE".
Please change the sentence to: "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 5
<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):** Fix typo "type of writing" to "time of writing".
Please change "at type of writing" to "at time of writing" in this note.
```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>
### Comment 6
<location> `doc/guides/nics/mlx5.rst:1438-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):** Use code-style backticks consistently and fix "in future" phrasing.
Please update this block to:
- Use double backticks for code identifiers (``mask`` and ``spec``) for consistency with the rest of the document.
- Replace “in future” with the more idiomatic “in the future.”
```suggestion
- 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 the future.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
examples/l3fwd/main.c (1)
57-63: RX/TX burst CLI and parsing are coherent;NB_MBUFstill tied toMAX_PKT_BURSTThe introduction of
rx_burst_size/tx_burst_sizeplus:
- updated usage (
--rx-burst/--tx-burst),parse_pkt_burst(including the PMD-recommended RX burst path for value 0), and- wiring in
parse_args,is internally consistent and safely bounded by
MAX_PKT_BURST. The log at startup clearly reports the effective RX/TX bursts.One behavioral note: the
NB_MBUFmacro still usesMAX_PKT_BURSTrather than the configuredrx_burst_size, so reducing the burst size won’t reduce the mempool footprint. That’s safe but may slightly over-allocate; consider following up with a separate change if you want pool sizing to track the configured RX burst.Also applies to: 388-437, 468-470, 703-750, 784-786, 817-819, 949-955, 1690-1691
📜 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)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/bus/fslmc/fslmc_bus.c(1 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/softnic/rte_eth_softnic.c
- drivers/net/ntnic/ntnic_ethdev.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/ice.rst (3)
1-2: PR scope mismatch: Title references bus/fslmc, but file is ICE NIC documentation.The PR title states "[PWCI] bus/fslmc: fix warning in device scan", but the only file provided for review is
doc/guides/nics/ice.rst, which documents the Intel ICE Network Interface Card (NIC) driver. These are unrelated components.Please clarify: Is this the correct file for this PR, or was the wrong file included in the review?
90-91: Version table updates for 25.11 release are clear and follow existing formatting.The new entries for DPDK 25.11 have been added to both E810 and E830 compatibility tables with appropriate kernel driver, DDP, and firmware versions. The formatting is consistent with existing entries.
Also applies to: 100-101
632-705: Comprehensive raw pattern documentation with clear examples and actionable guidance.The new sections on "Protocol Agnostic Filtering" provide well-structured documentation including:
- Clear distinction between Pattern Spec and Pattern Mask concepts
- Step-by-step instructions for generating raw patterns
- Both CLI and C API examples
- References to external tools (VPP's flow_parse.py)
- Clear constraints and limitations
The documentation follows the existing reStructuredText style and integrates well with the surrounding content. The note at line 699 about hardware parser 16-bit boundaries is particularly helpful for avoiding common mistakes.
Please verify that the VPP project link referenced at lines 684-685 remains current and accessible. Also confirm that the examples provided (particularly the flow_parse.py usage at lines 690-697) are accurate with the latest VPP implementation.
doc/guides/sample_app_ug/l3_forward.rst (1)
80-81: Documentation additions are well-formatted and consistent.The new
--rx-burstand--tx-burstoptions are properly integrated into both the command-line syntax section and option descriptions with formatting and style consistent with existing options. Defaults (32) align for both options, and descriptions clearly indicate these are optional parameters accepting decimal values.Also applies to: 118-120
examples/server_node_efd/efd_server/main.c (1)
70-88: MAC buffer size fix correctly accounts for newline and terminatorUpdating
addressestoRTE_MAX_ETHPORTS][RTE_ETHER_ADDR_FMT_SIZE + 1]aligns the buffer size withRTE_ETHER_ADDR_PRT_FMT "\n"and avoids the truncation warning while keeping behavior unchanged. Looks good.VERSION (1)
1-1: Version bump is consistent with release progressionUpdating to
25.11.0-rc4is self-contained and has no behavioral side effects.doc/guides/nics/ionic.rst (1)
2-19: Ionic NIC guide updates look correct and consistentYear range and the Pollara 400 AI NIC entry (text + PDF link) match the existing list structure and branding; no issues spotted.
doc/guides/cryptodevs/ionic.rst (1)
2-20: Crypto driver doc changes align with NIC guideUpdated copyright dates and Pollara 400 entry mirror the NIC guide and use consistent formatting; looks good.
drivers/bus/fslmc/fslmc_bus.c (1)
245-303: Const-correctness fix inrte_fslmc_parseis appropriateMaking
t_ptraconst char *matchesstrchr/sscanfusage and removes the discarded-qualifiers warning without altering behavior.doc/guides/meson.build (1)
15-26: Mesonhtml_guidesinstall tagging is well-scopedAdding
install_tag: 'doc'cleanly integrates this target into the doc install group and doesn’t affect build behavior otherwise.examples/server_node_efd/shared/common.h (1)
58-66: Ring queue name buffer sizing is now aligned with API constraintsUsing
RTE_RING_NAMESIZEfor the static buffer (and documenting the ring-name limit) is safer than a hand-computed size and avoids potential truncation/overflow mismatches.drivers/net/ngbe/ngbe_rxtx.c (1)
1918-1945: Simple TX path now exposes a safetx_pkt_preparecallbackAssigning
rte_eth_tx_pkt_prepare_dummyin the simple TX path guarantees a non-NULL prepare callback while leaving existing full-featured offload preparation (ngbe_prep_pkts) unchanged.drivers/net/txgbe/txgbe_rxtx.c (1)
2337-2368: TXGBE simple TX path now provides a dummytx_pkt_prepareUsing
rte_eth_tx_pkt_prepare_dummyfor the no-offload/simple path standardizes behavior with other drivers and avoids a NULLtx_pkt_preparewithout changing offloaded TX handling.doc/guides/freebsd_gsg/build_dpdk.rst (1)
19-28: LGTM - Python version update is consistent.The documentation correctly updates all references from Python 3.8 (py38-pyelftools) to Python 3.11 (py311-pyelftools), including the package name, version reference in the note, and installation command.
doc/api/meson.build (1)
30-30: LGTM - Documentation installation tagging.Adding
install_tag: 'doc'to custom targets is a metadata change that enables selective installation of documentation artifacts. This aligns with the broader documentation packaging strategy visible across other Meson build files in this PR.examples/vdpa/main.c (2)
78-79: LGTM - Improved string handling safety.The change from substring detection to explicit
strcmpmakes the intent clearer, and usingsizeof(iface)instead of the hardcodedMAX_PATH_LENconstant improves safety by automatically adapting to the actual buffer size.
25-25: Buffer size calculation is correct; no action required.MAX_VDPA_SAMPLE_PORTS is 1024, which stringifies to "1024" (4 characters + null terminator = 5 bytes). The iface buffer reduction from MAX_PATH_LEN (128) to MAX_PATH_LEN - MAX_VDPA_STR_LEN (123) correctly reserves space for the port number suffix. At line 501-502, snprintf appends the device count (port number) as "%s%d" format, which can be up to 1023 (4 digits), requiring up to 126 characters total plus null terminator—fitting within the 128-byte destination buffer. This is a legitimate fix for the compiler truncation warning.
doc/guides/linux_gsg/amd_platform.rst (2)
25-27: LGTM - Typo fix and TURIN platform addition.The correction of "SIENNA" to "SIENA" fixes a typo, and the addition of TURIN platform documentation with its tuning guide link is appropriate for expanding platform support coverage.
91-130: LGTM - Comprehensive platform documentation expansion.The additions provide valuable information about:
- Power management support across MILAN, GENOA, BERGAMO, SIENA, and TURIN platforms with passive mode specification
- New UNCORE power functions documentation starting from 25.03
- Detailed NIC-specific guidance for Solarflare x4 and x2 on TURIN
The documentation is well-structured and provides actionable configuration guidance.
doc/guides/rel_notes/release_25_11.rst (1)
349-600: LGTM - Comprehensive tested platforms documentation.The addition of detailed tested platform information significantly enhances release documentation by providing:
- Multiple Intel platforms with various NICs (E810, E830, E825, E610 series, X710, XXV710, XL710, etc.)
- NVIDIA ConnectX and BlueField configurations
- IBM Power 9 platforms
- Detailed firmware versions, driver versions, and compatibility information
This level of detail is valuable for users validating their hardware configurations.
drivers/net/hns3/hns3_rxtx.c (2)
4564-4564: LGTM - Consistent NULL pointer safety improvement.Assigning
rte_eth_tx_pkt_prepare_dummyinstead ofNULLwhen the NIC is not started ensures a valid function pointer is always available, preventing potential NULL pointer dereferences. This pattern is consistent with similar changes across multiple drivers (ngbe, txgbe) in this codebase.
4920-4920: LGTM - Safe function pointer assignment in TX stop path.Using
rte_eth_tx_pkt_prepare_dummyinstead of leaving the pointer asNULLwhen stopping the TX datapath maintains consistency and safety. The dummy function acts as a no-op that can be safely called, avoiding potential crashes from NULL dereferences.doc/guides/nics/i40e.rst (1)
107-108: LGTM - Hardware compatibility documentation update.The addition of DPDK 25.11 compatibility entries with kernel driver 2.28.13 and updated firmware versions (9.55 for X710 series, 6.51 for X722) maintains the documentation's currency. The version progression is consistent with the historical pattern.
Also applies to: 140-141
examples/meson.build (1)
81-83: No action required—snprintf calls are correctly bounded and safe.Verification confirms that the removal of
-Wno-format-truncationwill not expose actual warnings. Thenamebuffer inexamples/vhost_crypto/main.cis declared aschar name[128](line 494), and snprintf calls correctly specify a 127-byte limit, leaving room for the null terminator. All format strings produce output well under 127 bytes (e.g., "SYM_SESS_POOL_%u" with auint32_tlcore_id generates at most ~26 bytes). Other snprintf calls in the examples directory use safe patterns (sizeofoperator or bounds checks). The flag removal is safe.doc/guides/windows_gsg/build_dpdk.rst (4)
48-48: Minor wording improvement.The rephrasing from "latest version installer" to "latest installer version" improves clarity and conciseness.
69-69: Code-block formatting alignment applied.Leading spaces added for alignment within the code block; URL content and reference remain unchanged.
125-146: I'll verify the Microsoft documentation URL mentioned in the review comment.Microsoft documentation link is valid and current.
The Microsoft Learn page for Visual Studio Developer Command Prompt and Developer PowerShell is accessible and up-to-date for Visual Studio 2022, including details about command-line arguments like
-archfor 64-bit builds. The URL correctly directs to official Microsoft documentation.
28-32: Ignore this review comment—the file was not changed in this PR.The PR only modifies
drivers/bus/fslmc/fslmc_bus.c(pergit diff). The documentation filedoc/guides/windows_gsg/build_dpdk.rstreferenced in this review was not modified and should not be reviewed as part of this change.Likely an incorrect or invalid review comment.
doc/guides/nics/mlx5.rst (12)
1-4: Copyright year update is appropriate for the release.Standard maintenance addition.
650-651: Clarification about tunnel offload limitations is well-placed.The note appropriately documents the synchronous flow API constraint for HW steering.
699-700: Default dv_flow_en behavior documentation is now explicit.The new description clearly states the default is DV flow steering (value 1) if supported, removing ambiguity about capability-based selection.
This documentation change should be verified to align with corresponding code changes in the mlx5 driver that implement this new default behavior.
1366-1371: Precondition note about rte_flow_configure() is clear and appropriately placed.The distinction between mandatory (asynchronous) and recommended (synchronous) usage is helpful for users.
1410-1443: Synchronous flow API limitations are thoroughly documented.The new subsection covers important behavioral differences (match field size limits, rule update ordering, port configuration optimization, partial mask requirements) that developers need to understand when using synchronous API with HW steering.
2396-2402: Port representor limitations for synchronous HW steering are clearly stated.The section appropriately documents constraints and provides practical alternatives (using TX_QUEUE with per-queue rules).
2593-2595: MARK item limitation and META alternative are clearly documented.The note helpfully explains the constraint and provides a direct alternative for users migrating to synchronous HW steering API.
2812-2813: Aged flows reporting limitation is concisely documented.The constraint that aged flows report only once in synchronous HW steering is clearly stated.
2947-2948: Meter action type support in HW steering is clearly distinguished.The documentation correctly specifies that only METER_MARK (not METER) is supported in HW steering mode.
3816-3820: GRE field matching limitation for root table is clearly documented.The constraint on matching c_rsvd0_ver field in group 0 is appropriately specified for synchronous HW steering.
1-4800: Overall documentation consistency is maintained across all changes.The additions consistently document synchronous flow API constraints for HW steering mode, with proper cross-references and placement within existing sections. No contradictions or inconsistencies detected between sections.
1-4800: Inconsistency between PR objectives and reviewed file.The PR title references "bus/fslmc: fix warning in device scan", but the file under review is
doc/guides/nics/mlx5.rst(MLX5 NIC driver documentation). The AI summary suggests this is a broad release update (25.11.0-rc3 to 25.11.0-rc4) affecting multiple components. Verify that the correct files are included in this PR review, or clarify if this is intended as a multi-component release update.drivers/common/mlx5/mlx5_devx_cmds.h (2)
122-122: LGTM: New firmware workaround flag added.The addition of
header_length_field_mode_wabitfield follows the existing pattern in the structure and is appropriately sized as a 1-bit field for a boolean flag.
652-652: LGTM: Offset mode flag appropriately positioned.The
header_length_field_offset_modebitfield is well-placed adjacent to the relatedheader_length_field_offsetfield, making the relationship clear.drivers/net/mlx5/mlx5_flow_flex.c (1)
557-575: Theheader_length_field_mode_wafield is properly initialized from hardware capabilities during device setup. The script output shows it's set indrivers/common/mlx5/mlx5_devx_cmds.c:916-917viaMLX5_GET(parse_graph_node_cap, hcattr, header_length_field_offset_mode), and theattrstructure passed tomlx5_flex_translate_lengthis obtained from the device's configuration (priv->sh->cdev->config.hca_attr.flex). The gate condition at line 557 is intentional firmware compatibility logic, not indicative of an uninitialized variable issue—it deliberately applies the offset adjustment only for legacy firmware mode.Likely an incorrect or invalid review comment.
drivers/net/mlx5/mlx5.c (1)
1083-1091: No actionable issue found—the field is properly initialized.The
attr->header_length_field_mode_wafield is initialized during device capability query (viamlx5_devx_cmd_query_hca_parse_graph_node_capindrivers/common/mlx5/mlx5_devx_cmds.c), stored in persistent device config (priv->sh->cdev->config.hca_attr.flex), and accessed only after capability checks confirm the feature is supported. This is standard MLX5 device initialization; the field is safe to use.Likely an incorrect or invalid review comment.
drivers/common/mlx5/mlx5_prm.h (4)
1600-1602: Steering format macros look consistent and non-breakingDefining
MLX5_STEERING_LOGIC_FORMAT_CONNECTX_5/_CONNECTX_6DXas 0/1 aligns with the 4‑bitsteering_format_versionfield and introduces no ABI or layout changes.
2110-2132: Newheader_length_field_offset_modebit is wired correctlySplitting
reserved_at_78[0x3]intoreserved_at_78[0x1],header_length_field_offset_mode[0x1], andreserved_at_79[0x1]preserves the total bit width, so downstream offsets forparse_graph_anchoret al stay unchanged while enablingMLX5_GET(parse_graph_node_cap, …, header_length_field_offset_mode)in devx.
2350-2357: Expandedreserved_at_0range in ESW FT capsChanging
reserved_at_0from[0x200]to[0x800]shiftsft_header_modify_esw_fdbto a later offset to match updated PRM layout. Since only the reserved area moved and consumers accessft_header_modify_esw_fdbviaMLX5_GET(flow_table_esw_cap, …), this should be layout‑compatible with existing code.
4978-4995: Flex parser header-length offset mode bit is consistent with capsAdding
header_length_field_offset_modebetweenhead_anchor_idandnext_header_field_sizemirrors the new capability bit inparse_graph_node_capwhile keeping the total bit count of that region unchanged. This enablesMLX5_SET(parse_graph_flex, …, header_length_field_offset_mode, …)in the devx flex‑parser create path without disturbing existing fields.drivers/common/mlx5/mlx5_devx_cmds.c (2)
781-802: Flex parser create correctly propagates new offset-mode bitThe additional
MLX5_SET(parse_graph_flex, flex, header_length_field_offset_mode, data->header_length_field_offset_mode);wires the new bit frommlx5_devx_graph_node_attrinto the firmware object, matching the newparse_graph_flexlayout. This keeps all other fields untouched and is compatible with existing flex-graph creation.
884-917: Workaround flag derived from HCA cap is logically sound
attr->header_length_field_mode_wa = !MLX5_GET(parse_graph_node_cap, hcattr, header_length_field_offset_mode);makes the “workaround” flag true exactly when the HCA does not advertiseheader_length_field_offset_mode. That allows higher layers to use!header_length_field_mode_wawhen deciding whether to enable offset‑mode in flex nodes and avoids setting unsupported bits on older firmware.examples/l3fwd-power/main.c (1)
1490-1507: Eth link speed CLI wiring looks consistentUsage text, option definition, getopt_long entry, and the handler that validates and maps the speed into
port_conf.link_speedsare coherent; nothing blocking here.Also applies to: 1746-1754, 1771-1775, 1958-1970
examples/l3fwd/l3fwd_acl.c (1)
1134-1142: RX burst size switch torx_burst_sizeis safeUsing
rx_burst_sizefor the ACL RX loop matches the new configuration model, and withpkts_burst[MAX_PKT_BURST]plus validation in the parser there’s no risk of overruns.examples/l3fwd/l3fwd_fib.c (1)
238-244: FIB RX path now correctly uses configurablerx_burst_sizeThe change to pass
rx_burst_sizeintorte_eth_rx_burstis consistent with the new burst configuration and safe given theMAX_PKT_BURST-sized buffer.examples/l3fwd/l3fwd_lpm.c (1)
204-211: LPM RX loop aligned withrx_burst_sizeSwitching the LPM main loop to use
rx_burst_sizeforrte_eth_rx_burstis consistent with the rest of the app and safe given the fixedMAX_PKT_BURSTbuffer.examples/l3fwd/l3fwd_em.c (1)
643-649: Exact-match RX loop now usesrx_burst_sizeThe EM main loop correctly switches to the configurable
rx_burst_sizeand remains within theMAX_PKT_BURSTbuffer limit.examples/l3fwd/l3fwd_common.h (1)
28-30: TX fast path correctly parameterized bytx_burst_sizeMaking
tx_burst_size/rx_burst_sizeexterns and usingtx_burst_sizeas the threshold for the directrte_eth_tx_burstpath insend_packetsx4is consistent with the new CLI knobs; the hard cap viaMAX_PKT_BURSTremains intact.Also applies to: 77-79
examples/l3fwd/main.c (1)
761-763: Eth-link-speed option in l3fwd matches the intended behaviorAdding
--eth-link-speedhere mirrors the testpmd semantics: usage text, option constants, long-option table entry, and theCMD_LINK_OPT_ETH_LINK_SPEED_NUMcase that validates and maps speeds intoport_conf.link_speedsare all wired correctly, and 10M/100M are explicitly rejected as unsupported.Also applies to: 788-821, 822-851, 881-882, 930-941
| config->dv_flow_en = 1; | ||
| config->decap_en = 1; | ||
| if (config->dv_flow_en == 2) | ||
| config->allow_duplicate_pattern = 0; | ||
| else | ||
| config->allow_duplicate_pattern = 1; | ||
| config->allow_duplicate_pattern = 1; |
There was a problem hiding this comment.
🧩 Analysis chain
Unconditional defaults: verify validation catches unsupported configurations.
The default values are now unconditionally set to:
dv_flow_en = 1(DV flow steering enabled)allow_duplicate_pattern = 1(duplicate patterns allowed)
While validation logic at lines 1512-1515 resets dv_flow_en if unsupported, ensure this validation comprehensively handles all device configurations where these defaults may not be appropriate.
Run the following script to verify validation coverage:
🏁 Script executed:
#!/bin/bash
# Description: Verify validation logic handles all cases where default dv_flow_en=1 or allow_duplicate_pattern=1 may be unsupported
# Find validation checks for dv_flow_en capability
rg -nP --type=c 'dev_cap\.dv_flow_en|sh->dev_cap\.dv_flow_en' drivers/net/mlx5/ -A3 -B3
# Find any other places where allow_duplicate_pattern is validated
rg -nP --type=c 'allow_duplicate_pattern' drivers/net/mlx5/ -A3 -B3Length of output: 5987
Based on verification, the review comment's concern is validated. The web search reveals dv_flow_en and allow_duplicate_pattern devargs need updates based on SWS support, indicating both parameters have capability dependencies. However, the script output shows validation exists only for dv_flow_en (lines 1512-1515), while allow_duplicate_pattern lacks equivalent capability checking.
Asymmetric validation: add capability check for allow_duplicate_pattern.
SWS will be disabled in future HW generation, and both dv_flow_en and allow_duplicate_pattern devargs should be updated according to SWS support. The current code sets both unconditionally to 1 at lines 1494-1496 and includes validation for dv_flow_en (lines 1512-1515) but no corresponding validation for allow_duplicate_pattern. This creates a gap where unsupported configurations may not be properly reset. Add validation similar to dv_flow_en to reset allow_duplicate_pattern when hardware does not support it, ensuring consistency and preventing runtime issues on unsupported platforms.
🤖 Prompt for AI Agents
In drivers/net/mlx5/mlx5.c around lines 1494-1496 and the validation block at
1512-1515, the code unconditionally enables dv_flow_en and
allow_duplicate_pattern but only validates dv_flow_en against SWS/hardware
capability; add the same capability check for allow_duplicate_pattern as is done
for dv_flow_en (check SWS/hw support, and if unsupported, reset
config->allow_duplicate_pattern to 0 and log/handle accordingly) so both devargs
are gated by the same hardware capability logic and remain consistent.
| /* 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.
🧩 Analysis chain
Fix off-by-one error in bounds check and verify the correct constant.
The bounds validation has several issues:
-
Off-by-one error: The check uses
lcore > RTE_MAX_LCORE, but sincelcore_queue_confis declared as[RTE_MAX_LCORE](line 159), valid indices are 0 toRTE_MAX_LCORE-1. The condition should belcore >= RTE_MAX_LCORE. The same issue applies to the queue check. -
Possibly incorrect constant: The code checks
queue > RTE_MAX_QUEUES_PER_PORT, but the queue parameter indexes into structures sized byMAX_RX_QUEUE_PER_LCORE(see line 153:rx_queue_list[MAX_RX_QUEUE_PER_LCORE]). Verify thatRTE_MAX_QUEUES_PER_PORTis the appropriate constant here, or useMAX_RX_QUEUE_PER_LCOREinstead. -
Missing newline: The error message should end with
\nfor proper log formatting.
Apply this diff to fix the off-by-one error and add the missing newline:
- 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;
}Run the following script to verify whether RTE_MAX_QUEUES_PER_PORT or MAX_RX_QUEUE_PER_LCORE should be used for queue validation:
🏁 Script executed:
#!/bin/bash
# Description: Check usage context and definitions of queue-related constants
# Find where RTE_MAX_QUEUES_PER_PORT is defined
echo "=== Searching for RTE_MAX_QUEUES_PER_PORT definition ==="
rg -n 'define\s+RTE_MAX_QUEUES_PER_PORT'
# Find usages of setup_queue_tbl to understand queue parameter context
echo -e "\n=== Searching for setup_queue_tbl calls ==="
rg -nC3 'setup_queue_tbl\s*\('
# Check MAX_RX_QUEUE_PER_LCORE usage context
echo -e "\n=== Searching for MAX_RX_QUEUE_PER_LCORE usage ==="
rg -n 'MAX_RX_QUEUE_PER_LCORE'
# Compare definitions
echo -e "\n=== Searching for both constant definitions ==="
rg -n 'define\s+(RTE_MAX_QUEUES_PER_PORT|MAX_RX_QUEUE_PER_LCORE)'Length of output: 5632
Fix critical off-by-one errors and use correct constant for queue validation.
The bounds validation contains critical off-by-one errors that allow out-of-bounds array access:
-
Off-by-one on
lcore: The checklcore > RTE_MAX_LCOREallowslcore == RTE_MAX_LCOREthrough, butlcore_queue_conf[RTE_MAX_LCORE]is out of bounds (valid indices: 0 toRTE_MAX_LCORE-1). Should belcore >= RTE_MAX_LCORE. -
Wrong constant on
queue: The check usesRTE_MAX_QUEUES_PER_PORT(1024), but thequeueparameter indexes intorx_queue_list[MAX_RX_QUEUE_PER_LCORE](size 16, line 153). Should bequeue >= MAX_RX_QUEUE_PER_LCORE. -
Missing newline: Error message should end with
\nfor proper log formatting.
- 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 >= MAX_RX_QUEUE_PER_LCORE) {
+ RTE_LOG(ERR, IP_RSMBL, "invalid lcore %u or queue %u\n",
lcore, queue);
return -1;
}📝 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.
| /* 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; | |
| } | |
| /* Should never happen but check so that pool name won't be too long. */ | |
| if (lcore >= RTE_MAX_LCORE || queue >= MAX_RX_QUEUE_PER_LCORE) { | |
| 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
allows out-of-range indices: change the lcore comparison from > RTE_MAX_LCORE to
>= RTE_MAX_LCORE (valid indices are 0..RTE_MAX_LCORE-1), change the queue
comparison from > RTE_MAX_QUEUES_PER_PORT to >= MAX_RX_QUEUE_PER_LCORE
(rx_queue_list size is MAX_RX_QUEUE_PER_LCORE), and append a trailing "\n" to
the RTE_LOG message so the error is properly terminated.
| extern uint32_t rx_burst_size; | ||
| extern uint32_t mb_mempool_cache_size; | ||
|
|
There was a problem hiding this comment.
send_single_packet ignores tx_burst_size and incorrectly keys off rx_burst_size
The TX path now has a dedicated tx_burst_size, but send_single_packet still:
- Flushes when
len == rx_burst_size, and - Calls
send_burst(qconf, rx_burst_size, port),
so the --tx-burst option is effectively ignored for this common path and TX behavior is coupled to the RX burst size.
You likely want send_single_packet to honor tx_burst_size instead:
-extern uint32_t rx_burst_size;
+extern uint32_t rx_burst_size;
+extern uint32_t tx_burst_size;
@@
- /* enough pkts to be sent */
- if (unlikely(len == rx_burst_size)) {
- send_burst(qconf, rx_burst_size, port);
- len = 0;
- }
+ /* enough pkts to be sent */
+ if (unlikely(len >= tx_burst_size)) {
+ /* len and tx_burst_size should normally be equal here */
+ send_burst(qconf, len, port);
+ len = 0;
+ }This way:
- TX burst sizing is controlled solely by
tx_burst_size, --tx-bursthas a visible effect, and- You avoid hard-coding RX semantics into the TX flush condition.
Also applies to: 151-153
🤖 Prompt for AI Agents
In examples/l3fwd/l3fwd.h around lines 115-117 (and also lines 151-153),
send_single_packet is incorrectly using rx_burst_size for its TX flush condition
and burst length; change those checks and the send_burst call to use
tx_burst_size instead of rx_burst_size so the function honors the dedicated
tx_burst_size option (i.e., flush when len == tx_burst_size and call send_burst
with tx_burst_size), ensuring --tx-burst affects this path and TX behavior is
decoupled from RX sizing.
NOTE: This is an auto submission for "bus/fslmc: fix warning in device scan".
See "http://patchwork.dpdk.org/project/dpdk/list/?series=36789" for details.
Summary by Sourcery
Add new configuration options and safety fixes across examples and drivers, while updating mlx5 flex parser capabilities and improving documentation and install metadata.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Chores:
Summary by CodeRabbit
New Features
--rx-burstand--tx-burstcommand-line options to L3 forwarding sample application.--eth-link-speedoption to L3fwd-power sample application.Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.