-
Notifications
You must be signed in to change notification settings - Fork 4
[PWCI] "[v2] net/nbl: add VLAN offload set interface" #499
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>
Implement the vlan_offload_set device operation to allow enabling or disabling VLAN offloads through the API. rte_eth_dev_set_vlan_offload() calls the driver to update the hardware configuration. Because this driver performs VLAN stripping entirely in software, only a dummy implementation is needed to avoid returning an error. Validation of unsupported or invalid flags is already handled in ethdev. Fixes: 9d7757d ("net/nbl: simulate VLAN offload") Signed-off-by: Dimon Zhao <dimon.zhao@nebula-matrix.com> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org> Signed-off-by: 0-day Robot <robot@bytheb.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.
WalkthroughVersion bumped to 25.11.0-rc4. Documentation updated with new platform support, copyright years, and NIC models. MLX5 driver refactored with structure changes, steering logic modifications, and capability handling simplification. Multiple drivers standardized on tx_pkt_prepare dummy handlers. L3fwd example refactored to split RX/TX burst sizes as separate parameters. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ 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.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
drivers/net/mlx5/mlx5.c (1)
1050-1153: SRH flex parser header-length WA looks reasonable; fix existing refcnt error pathThe new distinction between legacy firmware (
header_length_field_mode_watrue, offset derived fromheader_length_mask_width) and newer firmware (offset fixed at 12) is self‑consistent and keeps behavior identical to old code when the WA flag is false; no correctness issues stand out around the new offset math by itself.However, there is an existing lifetime bug in this function: if any of the error paths after incrementing
srh_flex_parser.refcntare hit (e.g.,mlx5_devx_cmd_create_flex_parserormlx5_devx_cmd_query_parse_samplesfails),refcntstays at 1 whileflex.devx_fpmay be freed, so the next call will early‑return on the refcount and skip re‑allocation, leaving a “success” state with a NULLdevx_fp.A minimal fix is to decrement the refcount on the
error:path and null outflex.devx_fp, e.g.:int mlx5_alloc_srh_flex_parser(struct rte_eth_dev *dev) { @@ error: - if (fp) - mlx5_devx_cmd_destroy(fp); - if (priv->sh->srh_flex_parser.flex.devx_fp) - mlx5_free(priv->sh->srh_flex_parser.flex.devx_fp); - return (rte_errno == 0) ? -ENODEV : -rte_errno; + if (fp) + mlx5_devx_cmd_destroy(fp); + if (priv->sh->srh_flex_parser.flex.devx_fp) { + mlx5_free(priv->sh->srh_flex_parser.flex.devx_fp); + priv->sh->srh_flex_parser.flex.devx_fp = NULL; + } + rte_atomic_fetch_sub_explicit(&priv->sh->srh_flex_parser.refcnt, 1, + rte_memory_order_relaxed); + return (rte_errno == 0) ? -ENODEV : -rte_errno; }
🧹 Nitpick comments (7)
doc/guides/freebsd_gsg/build_dpdk.rst (1)
30-36: Clarify FreeBSD version selection in kernel sources example.The example references FreeBSD 12.1 (released 2018), which is quite dated. While the "like the following" language indicates this is an example, readers may not know how to adapt it for their system. Consider adding guidance on determining the appropriate FreeBSD release and architecture, or reference a more recent version as the example.
For example:
To compile the required kernel modules for memory management and working with physical NIC devices, the kernel sources for FreeBSD also need to be installed. If not already present on the system, these can be -installed via commands like the following, for FreeBSD 12.1 on x86_64:: +installed via commands like the following. This example is for FreeBSD 12.1 on +x86_64; replace the version and architecture as appropriate for your system::Alternatively, update to reference a more recent FreeBSD release as the example baseline.
examples/server_node_efd/efd_server/main.c (1)
67-88: MAC address buffer sizing fix is correct; newline-in-buffer is a minor style nitThe change to:
static char addresses[RTE_MAX_ETHPORTS][RTE_ETHER_ADDR_FMT_SIZE + 1];(Line 71)
combined withsnprintf(addresses[port], sizeof(addresses[port]), RTE_ETHER_ADDR_PRT_FMT "\n", ...)(Lines 85–87)properly aligns the buffer size with the format string: the original
RTE_ETHER_ADDR_FMT_SIZEbuffer for the formatted MAC plus an extra byte for the appended'\n'. Usingsizeof(addresses[port])keeps the call robust if the macro is ever adjusted.One minor (optional) refinement: since the MAC string is printed inside quotes and combined with other layout, you might consider dropping the embedded
"\n"from the cached string and letting the caller handle newlines, to avoid surprises if this helper is reused elsewhere. That’s purely stylistic and not required for correctness.doc/guides/windows_gsg/build_dpdk.rst (1)
28-33: Windows build doc updates improve clarity and modernize referencesThe switch to HTTPS for LLVM and Meson installer links, newer LLVM example, and explicit Developer Command Prompt / PowerShell guidance all look good. Optionally, you could also update the first Meson “Getting-meson.html” link to HTTPS for consistency with the MSI installer link.
Also applies to: 48-52, 66-70, 125-147
drivers/net/nbl/nbl_dev/nbl_dev.c (2)
761-768: VLAN offload setter is a no-op; consider basic validation or documenting the contractReturning 0 unconditionally and ignoring
maskis fine if VLAN strip/insert are implemented purely in software based ondev->data->dev_conf.rxmode.offloadsand that state is already maintained by ethdev, but it does mean:
- Unsupported
maskcombinations will silently “succeed”.- This callback does not guard against future changes that might start relying on hardware programming here.
If you intend this to be a pure-ACK hook, it would help either to:
- Validate
maskagainst the VLAN-related bits you actually honor (fromdev_info->{rx,tx}_offload_capa) and return-ENOTSUPwhen mismatched, or- Add a brief comment noting that
maskis intentionally ignored and that VLAN behavior is controlled solely byrxmode.offloadsin the rx/tx paths.
482-486: Use ofRTE_ETHDEV_QUEUE_STAT_CNTRSafter its public removal
nbl_stats_get()still checksi < RTE_ETHDEV_QUEUE_STAT_CNTRSbefore fillingq_errors. Since 25.11 release notes state this macro is removed from public headers, please confirm it remains available for drivers (e.g. via an internal header) or consider migrating this path fully to the xstats-based mechanism rather than relying on that macro.doc/guides/nics/mlx5.rst (2)
648-654: Scope of dv_xmeta_en=3 tunnel offload limitationThe new note about tunnel offload not being supported in the synchronous flow API with HW steering (dv_xmeta_en=3) is clear; if this restriction applies only to mode 3, consider rephrasing to “In this mode, tunnel offload…” to make the scope explicit.
1366-1372: Synchronous/HWS flow API limitations are well captured; minor clarity nitThe new note about requiring
rte_flow_configure()for async API and recommending it for sync API, plus the detailed sync‑HWS limitations (match field size, update semantics, default port attributes, partial mask behavior) are good and actionable. The bullet about “zeroedrte_flow_port_attr(zero flow queues, one created by default)” is slightly confusing; consider clarifying that one internal queue is created even whennb_queue = 0so readers don’t misinterpret this.Also applies to: 1410-1443
📜 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 (39)
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/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/nbl/nbl_dev/nbl_dev.c(1 hunks)drivers/net/nbl/nbl_dev/nbl_dev.h(1 hunks)drivers/net/nbl/nbl_ethdev.c(1 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
🧰 Additional context used
🧬 Code graph analysis (2)
drivers/net/nbl/nbl_dev/nbl_dev.h (1)
drivers/net/nbl/nbl_dev/nbl_dev.c (1)
nbl_vlan_offload_set(761-768)
drivers/net/nbl/nbl_ethdev.c (1)
drivers/net/nbl/nbl_dev/nbl_dev.c (1)
nbl_vlan_offload_set(761-768)
🔇 Additional comments (50)
doc/guides/linux_gsg/amd_platform.rst (1)
25-27: Documentation updates are well-structured.The addition of TURIN references, the update from SIENNA to SIENA, and the new Uncore subsection are all well-documented and consistently integrated into the file structure. The prerequisites and tuning guidance for Solarflare NICs are clear and actionable.
Also applies to: 91-92, 99-104
doc/guides/freebsd_gsg/build_dpdk.rst (2)
19-19: Verify py311-pyelftools availability across FreeBSD versions.The change from
py38-pyelftoolstopy311-pyelftoolsassumes Python 3.11 packages are available. Since FreeBSD package availability varies by version, confirm thatpy311-pyelftoolsis widely available in supported FreeBSD releases, or add a note about minimum FreeBSD version requirements.Also applies to: 28-28
21-24: Note directive format is now correct.The change from
.. note:(single colon) to.. note::(double colon) aligns with ReStructuredText specification and is consistent with other note blocks in the file (lines 59, 98, 134, 161, 176). Indentation is also correct.examples/vdpa/main.c (3)
25-25: LGTM! Well-designed macro for buffer size calculation.The macro correctly calculates the string length needed for the maximum device count, accounting for both digits and null terminator. This approach is future-proof—if
MAX_VDPA_SAMPLE_PORTSchanges, the calculation automatically adjusts.
40-40: LGTM! Buffer size correctly accounts for device count concatenation.Reducing the
ifacebuffer size toMAX_PATH_LEN - MAX_VDPA_STR_LENensures that when the device count is appended at line 501-502, the result fits withinMAX_PATH_LENwithout truncation. This prevents potential buffer overflow.
78-81: LGTM! Excellent improvements to option parsing.Two important fixes here:
- Line 78: Using
strcmpinstead ofstrncmpwithMAX_PATH_LENis correct for comparing fixed option names. The original usage was semantically incorrect.- Line 79: Switching to
rte_strscpywithsizeof(iface)is safer—it guarantees null termination and uses the correct buffer size.These changes improve both correctness and safety.
doc/guides/sample_app_ug/l3_forward.rst (1)
80-81: LGTM - Clear documentation for new burst size options.The new
--rx-burstand--tx-burstcommand-line options are well-documented with clear descriptions and default values (32 packets). This allows users to independently configure RX and TX burst sizes.Also applies to: 118-121
examples/l3fwd/l3fwd_common.h (2)
28-30: LGTM - Proper external declarations for burst sizes.The external declarations for
rx_burst_sizeandtx_burst_sizeare appropriately placed and will be used across the l3fwd modules.
77-77: LGTM - Correct use of tx_burst_size for TX path.The
send_packetsx4function correctly usestx_burst_sizeas the threshold for deciding whether to send packets immediately or buffer them. This is appropriate for the TX path.examples/l3fwd-power/main.c (1)
1958-1970: Verify that eth-link-speed feature is intentionally included in this PR.The implementation adds a new
--eth-link-speedoption with proper validation (rejecting unsupported 10M/100M speeds) and conversion usingrte_eth_speed_bitflag. The logic appears correct, but this feature seems unrelated to the PR title "[v2] net/nbl: add VLAN offload set interface" and the main burst size changes.Please confirm whether the eth-link-speed feature addition is intentionally part of this PR or if it was inadvertently included.
examples/l3fwd/l3fwd_fib.c (1)
241-242: LGTM - Correct use of rx_burst_size for RX burst.The replacement of
nb_pkt_per_burstwithrx_burst_sizeis correct for the RX path in the FIB main loop.examples/l3fwd/l3fwd_acl.c (1)
1138-1139: LGTM - Correct use of rx_burst_size for RX burst.The replacement of
nb_pkt_per_burstwithrx_burst_sizeis correct for the RX path in the ACL main loop.examples/l3fwd/l3fwd_lpm.c (1)
207-208: LGTM - Correct use of rx_burst_size for RX burst.The replacement of
nb_pkt_per_burstwithrx_burst_sizeis correct for the RX path in the LPM main loop.examples/l3fwd/l3fwd_em.c (1)
646-647: LGTM - Correct use of rx_burst_size for RX burst.The replacement of
nb_pkt_per_burstwithrx_burst_sizeis correct for the RX path in the EM main loop.examples/l3fwd/l3fwd.h (1)
115-115: LGTM - Variable renamed consistently.The external declaration has been correctly updated from
nb_pkt_per_bursttorx_burst_size.examples/l3fwd/main.c (6)
60-62: LGTM!The split of burst sizes into separate RX and TX configuration variables is a logical improvement over the previous single burst size parameter.
404-436: LGTM!The usage documentation clearly describes the new options and their default values.
468-469: LGTM!The default burst size is correctly passed twice for both RX and TX burst documentation.
762-762: LGTM!The CLI option constants and table entries are properly defined and follow the existing conventions.
Also applies to: 784-785, 795-795, 817-818, 824-824, 847-848
949-955: LGTM!The parsing correctly differentiates between RX and TX burst options, passing the appropriate boolean flag and burst size reference.
Note: This code is affected by the hardcoded port 0 issue in
parse_pkt_burstflagged separately.
1690-1691: LGTM!The log message clearly reports the chosen RX and TX burst sizes, which is helpful for debugging and verification.
doc/guides/meson.build (1)
15-26: install_tag addition for html_guides looks correctTagging this custom_target with
install_tag: 'doc'is consistent with the rest of the documentation Meson targets and should not affect build behavior beyond install tagging.doc/guides/nics/ionic.rst (1)
2-3: IONIC NIC docs update is consistent and well-formedYear extension and Pollara 400 model entry (with PDF link) match the existing style and structure of this guide; no issues from a docs or formatting perspective.
Also applies to: 18-19
doc/guides/cryptodevs/ionic.rst (1)
2-3: Crypto IONIC docs remain aligned with NIC docsThe updated year range and Pollara 400 entry mirror the NIC guide and keep device support documentation consistent.
Also applies to: 19-20
VERSION (1)
1-1: Version bump is straightforwardUpdating
VERSIONto25.11.0-rc4is consistent with a new RC; no technical concerns here.drivers/net/ngbe/ngbe_rxtx.c (1)
1920-1925: Simple TX path now opts into dummy tx_pkt_prepareSetting
dev->tx_pkt_prepare = rte_eth_tx_pkt_prepare_dummy;in the no-offload “simple” path makes ngbe’s behavior consistent with other drivers and guarantees a non-NULL preparer, but it does slightly change semantics for applications that callrte_eth_tx_prepare()and may have relied on the previous implicit behavior. Please double-check that this matches current ethdev expectations (default vs dummy preparer) and that no callers depend ontx_pkt_preparebeing NULL in this mode.doc/api/meson.build (1)
24-31: Meson install_tag usage for API docs is consistentAdding
install_tag: 'doc'to the examples, HTML, and manpage Doxygen targets cleanly aligns API docs with the new documentation install tagging scheme and should be safe for existing Meson setups.Also applies to: 71-81, 86-96
drivers/net/txgbe/txgbe_rxtx.c (1)
2340-2348: Simple TX path now explicitly uses dummy tx_pkt_prepareAssigning
dev->tx_pkt_prepare = rte_eth_tx_pkt_prepare_dummy;in the no-offload / no-IPsec simple TX path makes txgbe consistent with other PMDs and guarantees a defined, no-op preparer. Please verify this matches current ethdev expectations for simple TX queues and that no callers relied ontx_pkt_preparebeing left at its previous value in this configuration.drivers/net/hns3/hns3_rxtx.c (1)
4552-4564: tx_pkt_prepare now consistently reset to dummy in disabled TX pathsUsing
rte_eth_tx_pkt_prepare_dummyinstead ofNULLfortx_pkt_preparein bothhns3_set_rxtx_function()’s non-started branch andhns3_stop_tx_datapath()keeps the fastpath callback non-null and aligned with the dummytx_pkt_burstusage; behavior looks correct. Please just confirm there is no remaining code (in this driver or callers) that interpretstx_pkt_prepare == NULLspecially for TX-disable detection.Also applies to: 4917-4921
doc/guides/nics/i40e.rst (1)
104-108: 25.11 kernel/FW matrix entries are consistent with existing tablesThe added 25.11 rows for X710/XL710/XXV710 and X722 keep the table structure, alignment, and version ordering intact; no issues spotted.
Also applies to: 137-141
doc/guides/nics/ice.rst (1)
61-92: E810/E830 25.11 compatibility rows look correctThe new 25.11 entries extend the existing E810 and E830 matching lists cleanly, with consistent column formatting and ordering; nothing problematic here.
Also applies to: 93-101
doc/guides/rel_notes/release_25_11.rst (1)
349-600: Tested Platforms matrix is comprehensive and well-structuredThe newly added platform/NIC matrices for Intel, NVIDIA, and IBM Power 9 are consistently formatted and detailed enough for release documentation; I don’t see structural or formatting issues.
drivers/net/nbl/nbl_dev/nbl_dev.h (1)
73-99: VLAN offload setter prototype aligns with ethdev ops usageThe added declaration of
nbl_vlan_offload_set()matches the ethdev ops callback signature and the implementation innbl_dev.c, with__rte_unusedcorrectly reflecting the current no-op behavior.drivers/net/nbl/nbl_ethdev.c (1)
29-49: Wiringvlan_offload_setinto eth_dev_ops looks correctHooking
.vlan_offload_set = nbl_vlan_offload_setcleanly exposes the VLAN offload control entrypoint for this PMD and is consistent with the no-op implementation innbl_dev.cfor software-only VLAN handling.examples/server_node_efd/shared/common.h (1)
61-63: Buffer sizing correctly usesRTE_RING_NAMESIZEconstant — change is sound.Verification confirms the analysis in the original review is accurate:
- RTE_RING_NAMESIZE = 30 bytes (defined as
RTE_MEMZONE_NAMESIZE - sizeof(RTE_RING_MZ_PREFIX) + 1 = 32 - 3 + 1)- Maximum formatted string:
"MProc_Node_4294967295_RX\0"requires 26 bytes (25 chars + null terminator)- Buffer is sufficient: 30 ≥ 26 ✓
The old calculation (referenced in the outdated code comment) would have allocated only ~19–20 bytes, which is insufficient. Using the appropriate
RTE_RING_NAMESIZEconstant is the correct fix.Note: The code comment at lines 59–60 should be updated to reflect that unsigned integers can have up to 10 digits, not 3.
drivers/net/mlx5/mlx5_flow_flex.c (1)
557-575: Correct capability-based offset mode handling.The conditional guard on line 557 properly gates the legacy offset adjustment to only apply when
header_length_field_mode_wais true (workaround needed for older hardware). The inverse relationship on line 575 (offset_mode = !wa) correctly signals to firmware which behavior to use, aligning with the capability query logic inmlx5_devx_cmd_query_hca_parse_graph_node_cap.drivers/common/mlx5/mlx5_devx_cmds.c (2)
787-788: Correct propagation of offset mode to flex parser.The new
header_length_field_offset_modefield is correctly propagated to the DevX flex parser configuration using the standard MLX5_SET pattern.
916-917: Correct capability query with inverse workaround flag.The workaround flag is correctly computed as the inverse of the hardware-reported capability. When hardware supports
header_length_field_offset_mode(returns 1), no workaround is needed (wa=0); when hardware doesn't support it (returns 0), the workaround is required (wa=1).drivers/common/mlx5/mlx5_devx_cmds.h (2)
122-122: Appropriate addition of workaround capability flag.The new
header_length_field_mode_wabitfield is correctly positioned within the existing bitfield group inmlx5_hca_flex_attr. This flag properly captures whether the hardware requires legacy offset handling behavior.
652-652: No code dependencies on removedsw_ownerfields detected.The verification confirms that the removed
sw_ownerfields (rx_sw_owner,tx_sw_owner,esw_sw_owner, and their_v2variants) are not referenced anywhere in the codebase. The field removal does not break any existing functionality.doc/guides/nics/mlx5.rst (6)
689-701: dv_flow_en default now clearly matches implementationDocumenting dv_flow_en as defaulting to 1 (DV) when supported, and 0 otherwise, matches the new default initialization in
mlx5_shared_dev_ctx_args_config, so this looks consistent.
824-837: allow_duplicate_pattern default documentedStating that
allow_duplicate_patterndefaults to 1 aligns with the code default; no issues here.
2771-2772: Root-table COUNT HWS requirements noteThe added Linux/rdma-core/DOCA requirements for using count actions on root tables under HWS are precise and useful; no issues from a docs standpoint.
2812-2814: Age action: sync HWS “reported only once” behaviorDocumenting that aged flows are reported only once when using sync API with HWS is important for application expectations; the wording is clear.
2947-2949: Metering: clarify HWS only supports METER_MARKThe limitation that HWS supports only
RTE_FLOW_ACTION_TYPE_METER_MARKand notRTE_FLOW_ACTION_TYPE_METERis clearly stated and matches the usual HWS design, so this looks good.
3819-3821: GRE sync HWS group‑0 limitationThe note that, with synchronous flow API and HWS, matching
c_rsvd0_veris not supported in group 0 is precise; no further clarification seems necessary.drivers/net/mlx5/mlx5.c (1)
1490-1499: dv_flow_en and allow_duplicate_pattern defaults now match documented behaviorSetting
config->dv_flow_en = 1andconfig->allow_duplicate_pattern = 1in the shared-config defaults, combined with the existing capability gate:if (config->dv_flow_en && !sh->dev_cap.dv_flow_en) { DRV_LOG(WARNING, "DV flow is not supported."); config->dv_flow_en = 0; }gives exactly the documented behavior: default to DV flow steering when supported, else fall back to legacy Verbs with duplicate patterns allowed by default.
drivers/common/mlx5/mlx5_prm.h (3)
1600-1602: I'll help you verify the enum to macro conversion in the review comment. Let me search the codebase for related enum declarations and usage patterns.
<func_calls>
#!/bin/bashSearch for any enum declarations related to steering format
echo "=== Searching for enum declarations related to steering format ==="
rg -n -C5 'enum.*steering|enum.*MLX5_STEERING_LOGIC_FORMAT' --type=c --type=hecho ""
#!/bin/bash # Check for any enum usage patterns that might break (e.g., enum iteration, sizeof, type checking) echo "=== Checking for potential enum-specific usage patterns ===" rg -n 'CONNECTX_5|CONNECTX_6DX' --type=c --type=h | head -50 #!/bin/bash # Look at the actual file around lines 1600-1602 echo "=== Content around lines 1600-1602 in mlx5_prm.h ===" sed -n '1595,1610p' drivers/common/mlx5/mlx5_prm.h
echo "=== Searching for macro usage of steering logic format ==="
rg -n -C3 'MLX5_STEERING_LOGIC_FORMAT' --type=c --type=h | head -100
</function_calls>Also check if there's any original enum definition further up in the file
echo ""
#!/bin/bash # Search for any type-related usages that would break with macro conversion echo "=== Searching for potential type-related usages ===" rg -n 'MLX5_STEERING_LOGIC_FORMAT|steering_logic_format' --type=c --type=h -A2 -B2 | head -150
echo "=== Searching for steering format definitions in the entire file ==="
grep -n 'steering.*logic|STEERING_LOGIC_FORMAT' drivers/common/mlx5/mlx5_prm.h
</function_calls>
4990-4992: Field is consistently handled across both structures with proper synchronization.The
header_length_field_offset_modefield appears in both capability and flex structures with correct synchronized handling:
- Capability read (mlx5_devx_cmds.c:916-917): Hardware capability
parse_graph_node_cap.header_length_field_offset_modeis read and inverted intoheader_length_field_mode_waworkaround flag- Runtime usage (mlx5_flow_flex.c:575): The workaround flag is inverted back to match hardware capability state:
node->header_length_field_offset_mode = !attr->header_length_field_mode_wa- Device command (mlx5_devx_cmds.c:787-788): The runtime value is written to
parse_graph_flex.header_length_field_offset_modevia MLX5_SET macroThis inversion pattern is intentional—the hardware capability value flows through a workaround flag layer and returns to the original state when written to the flex parser.
Note: The AI summary's reference to "FIELD_MODE_OFFSET" is unrelated—that's an enum for field length determination mode, not the hardware register field.
2118-2120: All verification checks passed—the field is properly integrated.The
header_length_field_offset_modefield is correctly implemented:
Properly queried in capability detection:
mlx5_devx_cmds.clines 916-917 extract this field from hardware capabilities usingMLX5_GET(parse_graph_node_cap, hcattr, header_length_field_offset_mode)Field meaning is documented through usage: It's a 1-bit mode flag stored in the hardware capability structure and reflected as
header_length_field_mode_wa(workaround flag) in the flex attributes structConsistently used in related code:
mlx5_devx_cmds.h:652defines the field in the graph node attribute structuremlx5_flow_flex.c:575properly initializes it based on the queried capabilitymlx5_devx_cmds.c:787-788properly sets it when configuring the deviceThe field follows the established pattern for hardware capability exposure and is fully integrated with the driver's flex parser operations.
| 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. |
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 typo: "at type of writing" → "at time of writing"
Line 23 contains a typographical error that should be corrected.
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.
+ Python 3.11 being the version at time of writing, hence the ``py311`` prefix.📝 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.
| 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. | |
| 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. |
🤖 Prompt for AI Agents
In doc/guides/freebsd_gsg/build_dpdk.rst around lines 23 to 24, there's a
typographical error "at type of writing" — replace it with "at time of writing"
to correct the sentence about Python 3.11 and the py311 prefix.
| .. note:: | ||
|
|
||
| * Power libraries are supported on Linux only. | ||
| * DPDK uncore support on Linux is work in progress. | ||
|
|
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.
Remove or update outdated note about uncore support.
The note at line 98 states that "DPDK uncore support on Linux is work in progress," but the new Uncore subsection (lines 99–104) documents uncore as a supported feature. Please either remove this note or update it to reflect that uncore is now supported from 25.03 onwards.
Apply this diff to remove the outdated note:
.. note::
* Power libraries are supported on Linux only.
- * DPDK uncore support on Linux is work in progress.📝 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.
| .. note:: | |
| * Power libraries are supported on Linux only. | |
| * DPDK uncore support on Linux is work in progress. | |
| .. note:: | |
| * Power libraries are supported on Linux only. |
🤖 Prompt for AI Agents
In doc/guides/linux_gsg/amd_platform.rst around lines 94 to 98, the second
bullet saying "DPDK uncore support on Linux is work in progress" is outdated;
either remove that bullet entirely or update it to reflect current status (for
example: "DPDK uncore support on Linux is supported since 25.03"). Ensure the
note matches the new Uncore subsection (lines 99–104) and keep phrasing
consistent with existing release/version style used elsewhere in the document.
| NIC | ||
| --- | ||
|
|
||
| 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``. |
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.
Clarify consistency in driver descriptions between Solarflare x4 and x2.
The instructions for Solarflare x4 (line 116) specify "out-of-tree sfc (contact Solarflare)" while the x2 instructions (line 127) state only "sfc" without clarifying whether it is out-of-tree or in-tree. For consistency and clarity, please specify which driver variant is required for x2.
Suggested fix for line 127:
-#. Enable the hardware with ``sfc`` Linux Kernel Driver first;
+#. Enable the hardware with out-of-tree sfc (contact Solarflare) Linux kernel driver first;(Or alternatively, clarify that x2 uses the in-tree driver if that is the case.)
📝 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.
| NIC | |
| --- | |
| 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``. | |
| NIC | |
| --- | |
| 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 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``. |
🤖 Prompt for AI Agents
In doc/guides/linux_gsg/amd_platform.rst around lines 107 to 130, the Solarflare
x2 driver instruction on line ~127 is ambiguous compared to the x4 entry (which
explicitly says "out-of-tree sfc (contact Solarflare)"); update the x2 entry to
clearly state which driver variant is required — either change the text to
"Enable the hardware with out-of-tree sfc (contact Solarflare) Linux kernel
driver first; then bind with ``vfio-pci`` next." to match x4, or explicitly
state "in-tree sfc" if the in-tree driver is acceptable, ensuring wording and
phrasing mirror the x4 section for consistency.
| #. When using synchronous flow API with :ref:`HW steering <mlx5_hws>`: | ||
|
|
||
| - ``RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR`` is not supported. | ||
| ``RTE_FLOW_ITEM_TYPE_TX_QUEUE`` can be used with a rule for each queue. | ||
| - Transfer rules are not supported on representor ports. | ||
| - Rules created on proxy ports without explicit represented port matching | ||
| will match packets from all ports. | ||
|
|
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.
Update capability tables to reflect new sync HWS restrictions
The new bullets state that with synchronous flow API and HW steering, RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR and RTE_FLOW_ITEM_TYPE_MARK (16‑bit match) are unsupported, recommending TX_QUEUE and META instead. However, the “Flow Matching” capability table above still marks port representor and mark as supported for sync HWS, which can mislead users.
Consider either adjusting the sync HWS column for these rows or adding a footnote/remark in the table pointing to these limitations so the summary view matches the detailed behavior.
Also applies to: 2593-2596
🤖 Prompt for AI Agents
In doc/guides/nics/mlx5.rst around lines 2396-2403 (and also update the related
table area at 2593-2596), the “Flow Matching” capability table incorrectly shows
port representor and mark as supported for synchronous HW steering; update the
sync HWS column to reflect that RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR and 16-bit
RTE_FLOW_ITEM_TYPE_MARK are unsupported (or add a clear footnote/remark linking
to the new bullets stating the restrictions and recommending TX_QUEUE and META
instead) so the summary table matches the detailed behavior.
| u8 reserved_at_0[0x200]; | ||
| struct mlx5_ifc_flow_table_prop_layout_bits flow_table_properties_nic_esw_fdb; | ||
| u8 reserved_at_400[0x400]; | ||
| u8 reserved_at_0[0x800]; |
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.
Verify significant structure size expansion is intentional.
The reserved area at the start of mlx5_ifc_flow_table_esw_cap_bits has been expanded from 0x200 (512 bytes) to 0x800 (2048 bytes), quadrupling the structure size. This is a substantial change that could impact:
- ABI compatibility: Code compiled with the old structure size won't be compatible
- Memory allocation: Buffers sized for this structure may need updates
- Hardware expectations: Ensure this matches the actual hardware specification version
Please verify:
- This expansion aligns with the target hardware specification
- All code that allocates or copies this structure uses sizeof() rather than hardcoded sizes
- The change is documented in release notes as a breaking change if it affects external interfaces
#!/bin/bash
# Check for hardcoded structure size references
rg -n '0x200|512' drivers/common/mlx5/ drivers/net/mlx5/ -g '*.c' -C2 | rg -i 'flow_table_esw'
# Find all allocations and usage of this structure
ast-grep --pattern 'mlx5_ifc_flow_table_esw_cap_bits'🤖 Prompt for AI Agents
drivers/common/mlx5/mlx5_prm.h around line 2351: the reserved field was
increased from 0x200 to 0x800 which quadruples the structure size; verify this
change matches the target hardware specification/version and is intentional,
then update all callers to use sizeof(mlx5_ifc_flow_table_esw_cap_bits) (not
hardcoded 0x200/512) for allocations, copies, and ABI checks, search and replace
any literal 0x200/512 uses related to this struct, ensure consumers compiled
against older size are handled or rebuilt, and add a release note documenting
this breaking change if it affects external interfaces.
| RTE_LOG(ERR, IP_RSMBL, "invalid lcore %u or queue %u", | ||
| lcore, queue); |
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.
Add missing newline character to error message.
The error log message lacks a trailing newline, inconsistent with all other RTE_LOG calls in this file (see lines 440, 449, 780, 865).
Apply this diff:
- RTE_LOG(ERR, IP_RSMBL, "invalid lcore %u or queue %u",
+ RTE_LOG(ERR, IP_RSMBL, "invalid lcore %u or queue %u\n",📝 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.
| RTE_LOG(ERR, IP_RSMBL, "invalid lcore %u or queue %u", | |
| lcore, queue); | |
| RTE_LOG(ERR, IP_RSMBL, "invalid lcore %u or queue %u\n", | |
| lcore, queue); |
🤖 Prompt for AI Agents
In examples/ip_reassembly/main.c around lines 889-890, the RTE_LOG error message
"invalid lcore %u or queue %u" is missing a trailing newline; update the format
string to include a "\n" at the end so the log call matches the file's other
RTE_LOG entries and 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
Script executed:
#!/bin/bash
# Find and examine the l3fwd.h file
fd -t f "l3fwd.h" examples/Repository: ovsrobot/dpdk
<shell_result>
Repository: ovsrobot/dpdk
Exit code: 0
stdout:
examples/l3fwd/l3fwd.h
</shell_result>
Script executed:
#!/bin/bash
# Find and examine the l3fwd_common.h file
fd -t f "l3fwd_common.h"Repository: ovsrobot/dpdk
<shell_result>
Repository: ovsrobot/dpdk
Exit code: 0
stdout:
examples/l3fwd/l3fwd_common.h
</shell_result>
Script executed:
#!/bin/bash
# Read the send_single_packet function in l3fwd.h around lines 151-152
head -n 170 examples/l3fwd/l3fwd.h | tail -n 40Repository: ovsrobot/dpdk
<shell_result>
Repository: ovsrobot/dpdk
Exit code: 0
stdout:
do {
rte_pktmbuf_free(m_table[ret]);
} while (++ret < n);
}
return 0;
}
/* Enqueue a single packet, and send burst if queue is filled */
static inline int
send_single_packet(struct lcore_conf *qconf,
struct rte_mbuf *m, uint16_t port)
{
uint16_t len;
len = qconf->tx_mbufs[port].len;
qconf->tx_mbufs[port].m_table[len] = m;
len++;
/* enough pkts to be sent */
if (unlikely(len == rx_burst_size)) {
send_burst(qconf, rx_burst_size, port);
len = 0;
}
qconf->tx_mbufs[port].len = len;
return 0;
}
#ifdef DO_RFC_1812_CHECKS
static inline int
is_valid_ipv4_pkt(struct rte_ipv4_hdr *pkt, uint32_t link_len, uint64_t ol_flags)
{
/* From http://www.rfc-editor.org/rfc/rfc1812.txt section 5.2.2 */
/*
* 1. The packet length reported by the Link Layer must be large
* enough to hold the minimum length legal IP datagram (20 bytes).
*/
if (link_len < sizeof(struct rte_ipv4_hdr))
return -1;
</shell_result>
Script executed:
#!/bin/bash
# Search for all declarations and usages of tx_burst_size and rx_burst_size
rg -n "tx_burst_size|rx_burst_size" examples/l3fwd/ -A 2 -B 2Repository: ovsrobot/dpdk
<shell_result>
Repository: ovsrobot/dpdk
Exit code: 0
stdout:
examples/l3fwd/l3fwd_lpm.c-206- queueid = qconf->rx_queue_list[i].queue_id;
examples/l3fwd/l3fwd_lpm.c-207- nb_rx = rte_eth_rx_burst(portid, queueid, pkts_burst,
examples/l3fwd/l3fwd_lpm.c:208: rx_burst_size);
examples/l3fwd/l3fwd_lpm.c-209- if (nb_rx == 0)
examples/l3fwd/l3fwd_lpm.c-210- continue;
--
examples/l3fwd/main.c-58-uint16_t nb_rxd = RX_DESC_DEFAULT;
examples/l3fwd/main.c-59-uint16_t nb_txd = TX_DESC_DEFAULT;
examples/l3fwd/main.c:60:uint32_t rx_burst_size = DEFAULT_PKT_BURST;
examples/l3fwd/main.c-61-uint32_t mb_mempool_cache_size = MEMPOOL_CACHE_SIZE;
examples/l3fwd/main.c:62:uint32_t tx_burst_size = DEFAULT_PKT_BURST;
examples/l3fwd/main.c-63-
examples/l3fwd/main.c-64-/**< Ports set in promiscuous mode off by default. */
--
examples/l3fwd/main.c-734- RTE_LOG(INFO, L3FWD, "PMD does not recommend a burst size. Using default value %d. "
examples/l3fwd/main.c-735- "User provided value must be in [1, %d]\n",
examples/l3fwd/main.c:736: rx_burst_size, MAX_PKT_BURST);
examples/l3fwd/main.c-737- return;
examples/l3fwd/main.c-738- } else if (burst_size > MAX_PKT_BURST) {
examples/l3fwd/main.c-739- RTE_LOG(INFO, L3FWD, "PMD recommended burst size %d exceeds maximum value %d. "
examples/l3fwd/main.c-740- "Using default value %d\n",
examples/l3fwd/main.c:741: burst_size, MAX_PKT_BURST, rx_burst_size);
examples/l3fwd/main.c-742- return;
examples/l3fwd/main.c-743- }
--
examples/l3fwd/main.c-948-
examples/l3fwd/main.c-949- case CMD_LINE_OPT_PKT_RX_BURST_NUM:
examples/l3fwd/main.c:950: parse_pkt_burst(optarg, true, &rx_burst_size);
examples/l3fwd/main.c-951- break;
examples/l3fwd/main.c-952-
examples/l3fwd/main.c-953- case CMD_LINE_OPT_PKT_TX_BURST_NUM:
examples/l3fwd/main.c:954: parse_pkt_burst(optarg, false, &tx_burst_size);
examples/l3fwd/main.c-955- break;
examples/l3fwd/main.c-956-
--
examples/l3fwd/main.c-1688- rte_exit(EXIT_FAILURE, "Invalid L3FWD parameters\n");
examples/l3fwd/main.c-1689-
examples/l3fwd/main.c:1690: RTE_LOG(INFO, L3FWD, "Using Rx burst %u Tx burst %u\n", rx_burst_size, tx_burst_size);
examples/l3fwd/main.c-1691-
examples/l3fwd/main.c-1692- /* Setup function pointers for lookup method. */
--
examples/l3fwd/l3fwd_em.c-645- queueid = qconf->rx_queue_list[i].queue_id;
examples/l3fwd/l3fwd_em.c-646- nb_rx = rte_eth_rx_burst(portid, queueid, pkts_burst,
examples/l3fwd/l3fwd_em.c:647: rx_burst_size);
examples/l3fwd/l3fwd_em.c-648- if (nb_rx == 0)
examples/l3fwd/l3fwd_em.c-649- continue;
--
examples/l3fwd/l3fwd_fib.c-240- queueid = qconf->rx_queue_list[i].queue_id;
examples/l3fwd/l3fwd_fib.c-241- nb_rx = rte_eth_rx_burst(portid, queueid, pkts_burst,
examples/l3fwd/l3fwd_fib.c:242: rx_burst_size);
examples/l3fwd/l3fwd_fib.c-243- if (nb_rx == 0)
examples/l3fwd/l3fwd_fib.c-244- continue;
--
examples/l3fwd/l3fwd_acl.c-1137- queueid = qconf->rx_queue_list[i].queue_id;
examples/l3fwd/l3fwd_acl.c-1138- nb_rx = rte_eth_rx_burst(portid, queueid,
examples/l3fwd/l3fwd_acl.c:1139: pkts_burst, rx_burst_size);
examples/l3fwd/l3fwd_acl.c-1140-
examples/l3fwd/l3fwd_acl.c-1141- if (nb_rx > 0) {
--
examples/l3fwd/l3fwd.h-113-extern uint32_t max_pkt_len;
examples/l3fwd/l3fwd.h-114-
examples/l3fwd/l3fwd.h:115:extern uint32_t rx_burst_size;
examples/l3fwd/l3fwd.h-116-extern uint32_t mb_mempool_cache_size;
examples/l3fwd/l3fwd.h-117-
--
examples/l3fwd/l3fwd.h-149-
examples/l3fwd/l3fwd.h-150- /* enough pkts to be sent */
examples/l3fwd/l3fwd.h:151: if (unlikely(len == rx_burst_size)) {
examples/l3fwd/l3fwd.h:152: send_burst(qconf, rx_burst_size, port);
examples/l3fwd/l3fwd.h-153- len = 0;
examples/l3fwd/l3fwd.h-154- }
--
examples/l3fwd/l3fwd_common.h-26-#define SENDM_PORT_OVERHEAD(x) (x)
examples/l3fwd/l3fwd_common.h-27-
examples/l3fwd/l3fwd_common.h:28:extern uint32_t rx_burst_size;
examples/l3fwd/l3fwd_common.h:29:extern uint32_t tx_burst_size;
examples/l3fwd/l3fwd_common.h-30-
examples/l3fwd/l3fwd_common.h-31-/*
--
examples/l3fwd/l3fwd_common.h-75- * then send them straightway.
examples/l3fwd/l3fwd_common.h-76- */
examples/l3fwd/l3fwd_common.h:77: if (num >= tx_burst_size && len == 0) {
examples/l3fwd/l3fwd_common.h-78- n = rte_eth_tx_burst(port, qconf->tx_queue_id[port], m, num);
examples/l3fwd/l3fwd_common.h-79- if (unlikely(n < num)) {
</shell_result>
Script executed:
#!/bin/bash
# Check what l3fwd.h includes and its complete extern declarations
head -n 130 examples/l3fwd/l3fwd.hRepository: ovsrobot/dpdk
<shell_result>
Repository: ovsrobot/dpdk
Exit code: 0
stdout:
/* SPDX-License-Identifier: BSD-3-Clause
* Copyright(c) 2010-2021 Intel Corporation
*/
#ifndef __L3_FWD_H__
#define __L3_FWD_H__
#include <rte_ethdev.h>
#include <rte_vect.h>
#include <rte_acl.h>
#define DO_RFC_1812_CHECKS
#define RTE_LOGTYPE_L3FWD RTE_LOGTYPE_USER1
#if !defined(NO_HASH_MULTI_LOOKUP) && defined(__ARM_NEON)
#define NO_HASH_MULTI_LOOKUP 1
#endif
/*
* Configurable number of RX/TX ring descriptors
*/
#define RX_DESC_DEFAULT 1024
#define TX_DESC_DEFAULT 1024
#define DEFAULT_PKT_BURST 32
#define MAX_PKT_BURST 512
#define BURST_TX_DRAIN_US 100 /* TX drain every ~100us */
#define MEMPOOL_CACHE_SIZE RTE_MEMPOOL_CACHE_MAX_SIZE
#define MAX_RX_QUEUE_PER_LCORE 16
#define VECTOR_SIZE_DEFAULT MAX_PKT_BURST
#define VECTOR_TMO_NS_DEFAULT 1E6 /* 1ms */
#define NB_SOCKETS 8
/* Configure how many packets ahead to prefetch, when reading packets */
#define PREFETCH_OFFSET 3
/* Used to mark destination port as 'invalid'. */
#define BAD_PORT ((uint16_t)-1)
/* replace first 12B of the ethernet header. */
#define MASK_ETH 0x3f
/* Hash parameters. */
#ifdef RTE_ARCH_64
/* default to 4 million hash entries (approx) */
#define L3FWD_HASH_ENTRIES (1024*1024*4)
#else
/* 32-bit has less address-space for hugepage memory, limit to 1M entries */
#define L3FWD_HASH_ENTRIES (1024*1024*1)
#endif
struct parm_cfg {
const char *rule_ipv4_name;
const char *rule_ipv6_name;
enum rte_acl_classify_alg alg;
};
struct acl_algorithms {
const char *name;
enum rte_acl_classify_alg alg;
};
struct mbuf_table {
uint16_t len;
struct rte_mbuf *m_table[MAX_PKT_BURST];
};
struct __rte_cache_aligned lcore_rx_queue {
uint16_t port_id;
uint16_t queue_id;
};
struct __rte_cache_aligned lcore_conf {
uint16_t n_rx_queue;
struct lcore_rx_queue rx_queue_list[MAX_RX_QUEUE_PER_LCORE];
uint16_t n_tx_port;
uint16_t tx_port_id[RTE_MAX_ETHPORTS];
uint16_t tx_queue_id[RTE_MAX_ETHPORTS];
struct mbuf_table tx_mbufs[RTE_MAX_ETHPORTS];
void *ipv4_lookup_struct;
void *ipv6_lookup_struct;
};
extern volatile bool force_quit;
/* RX and TX queue depths */
extern uint16_t nb_rxd;
extern uint16_t nb_txd;
/* ethernet addresses of ports */
extern uint64_t dest_eth_addr[RTE_MAX_ETHPORTS];
extern struct rte_ether_addr ports_eth_addr[RTE_MAX_ETHPORTS];
/* mask of enabled ports */
extern uint32_t enabled_port_mask;
/* Used only in exact match mode. */
extern int ipv6; /**< ipv6 is false by default. */
extern uint32_t hash_entry_number;
extern xmm_t val_eth[RTE_MAX_ETHPORTS];
extern struct lcore_conf lcore_conf[RTE_MAX_LCORE];
extern struct parm_cfg parm_config;
extern struct acl_algorithms acl_alg[];
extern uint32_t max_pkt_len;
extern uint32_t rx_burst_size;
extern uint32_t mb_mempool_cache_size;
/* Send burst of packets on an output interface */
static inline int
send_burst(struct lcore_conf *qconf, uint16_t n, uint16_t port)
{
struct rte_mbuf **m_table;
int ret;
uint16_t queueid;
queueid = qconf->tx_queue_id[port];
m_table = (struct rte_mbuf **)qconf->tx_mbufs[port].m_table;
ret = rte_eth_tx_burst(port, queueid, m_table, n);
if (unlikely(ret < n)) {
</shell_result>
Script executed:
#!/bin/bash
# Verify the parse_pkt_burst function to understand rx vs tx configuration
rg -n "parse_pkt_burst" examples/l3fwd/ -A 10Repository: ovsrobot/dpdk
<shell_result>
Repository: ovsrobot/dpdk
Exit code: 0
stdout:
examples/l3fwd/main.c:703:parse_pkt_burst(const char *optarg, bool is_rx_burst, uint32_t *burst_sz)
examples/l3fwd/main.c-704-{
examples/l3fwd/main.c-705- struct rte_eth_dev_info dev_info;
examples/l3fwd/main.c-706- unsigned long pkt_burst;
examples/l3fwd/main.c-707- uint16_t burst_size;
examples/l3fwd/main.c-708- char *end = NULL;
examples/l3fwd/main.c-709- int ret;
examples/l3fwd/main.c-710-
examples/l3fwd/main.c-711- /* parse decimal string */
examples/l3fwd/main.c-712- pkt_burst = strtoul(optarg, &end, 10);
examples/l3fwd/main.c-713- if ((optarg[0] == '\0') || (end == NULL) || (*end != '\0'))
--
examples/l3fwd/main.c:950: parse_pkt_burst(optarg, true, &rx_burst_size);
examples/l3fwd/main.c-951- break;
examples/l3fwd/main.c-952-
examples/l3fwd/main.c-953- case CMD_LINE_OPT_PKT_TX_BURST_NUM:
examples/l3fwd/main.c:954: parse_pkt_burst(optarg, false, &tx_burst_size);
examples/l3fwd/main.c-955- break;
examples/l3fwd/main.c-956-
examples/l3fwd/main.c-957- case CMD_LINE_OPT_MB_CACHE_SIZE_NUM:
examples/l3fwd/main.c-958- parse_mbcache_size(optarg);
examples/l3fwd/main.c-959- break;
examples/l3fwd/main.c-960-
examples/l3fwd/main.c-961- case CMD_LINE_OPT_ETH_DEST_NUM:
examples/l3fwd/main.c-962- parse_eth_dest(optarg);
examples/l3fwd/main.c-963- break;
examples/l3fwd/main.c-964-
</shell_result>
Based on my verification of the codebase, I can confirm the review comment is accurate. Here's what I found:
Verification Results:
- ✓
send_single_packetinl3fwd.h(lines 151-152) does userx_burst_sizein the TX path - ✓
send_packetsx4inl3fwd_common.h(line 77) correctly usestx_burst_size - ✓ Both
rx_burst_sizeandtx_burst_sizeare separate, independently configurable variables (main.c lines 950, 954) - ✓
l3fwd.honly declaresrx_burst_sizeas extern, nottx_burst_size(line 115) - ✓
l3fwd_common.hcorrectly declares both (lines 28-29) - ✓ The proposed diff is valid and necessary
Inconsistent use of rx_burst_size in TX path—should use tx_burst_size.
The send_single_packet function is part of the TX path and buffers packets before transmission. However, it uses rx_burst_size as the threshold for when to flush the buffer (lines 151-152):
if (unlikely(len == rx_burst_size)) {
send_burst(qconf, rx_burst_size, port);This is inconsistent with the analogous TX function send_packetsx4 in l3fwd_common.h (line 77), which correctly uses tx_burst_size:
if (num >= tx_burst_size && len == 0) {Additionally, rx_burst_size and tx_burst_size are independently configurable via command-line arguments, so using the wrong one could cause incorrect TX buffer flushing behavior.
Apply the proposed diff to declare and use tx_burst_size in the TX path:
extern uint32_t rx_burst_size;
+extern uint32_t tx_burst_size;
extern uint32_t mb_mempool_cache_size;
/* Send burst of packets on an output interface */
@@ -148,8 +149,8 @@ send_single_packet(struct lcore_conf *qconf,
len++;
/* enough pkts to be sent */
- if (unlikely(len == rx_burst_size)) {
- send_burst(qconf, rx_burst_size, port);
+ if (unlikely(len == tx_burst_size)) {
+ send_burst(qconf, tx_burst_size, port);
len = 0;
}🤖 Prompt for AI Agents
In examples/l3fwd/l3fwd.h around lines 151-152, the TX path function
send_single_packet incorrectly uses rx_burst_size as the flush threshold;
declare tx_burst_size as extern in this file (matching l3fwd_common.h) and
replace uses of rx_burst_size in the TX path with tx_burst_size so the
transmitter flushes according to the configured TX burst size.
| if (is_rx_burst) { | ||
| /* If user gives a value of zero, query the PMD for its recommended | ||
| * Rx burst size. | ||
| */ | ||
| ret = rte_eth_dev_info_get(0, &dev_info); | ||
| if (ret != 0) | ||
| return; | ||
| burst_size = dev_info.default_rxportconf.burst_size; | ||
| if (burst_size == 0) { | ||
| RTE_LOG(INFO, L3FWD, "PMD does not recommend a burst size. Using default value %d. " | ||
| "User provided value must be in [1, %d]\n", | ||
| rx_burst_size, MAX_PKT_BURST); | ||
| return; | ||
| } else if (burst_size > MAX_PKT_BURST) { | ||
| RTE_LOG(INFO, L3FWD, "PMD recommended burst size %d exceeds maximum value %d. " | ||
| "Using default value %d\n", | ||
| burst_size, MAX_PKT_BURST, rx_burst_size); | ||
| return; | ||
| } | ||
| *burst_sz = burst_size; | ||
| RTE_LOG(INFO, L3FWD, "Using PMD-provided RX burst value %d\n", burst_size); |
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.
Hardcoded port 0 in PMD burst size query.
The function queries device info from port 0 (line 729) to determine the recommended RX burst size, but this value is applied globally to all ports via the rx_burst_size parameter. Different PMDs or ports may have different recommended burst sizes, leading to suboptimal performance or incorrect configuration for non-zero ports.
Apply this diff to accept a portid parameter and query the correct port:
static void
-parse_pkt_burst(const char *optarg, bool is_rx_burst, uint32_t *burst_sz)
+parse_pkt_burst(const char *optarg, bool is_rx_burst, uint32_t *burst_sz, uint16_t portid)
{
struct rte_eth_dev_info dev_info;
unsigned long pkt_burst;
uint16_t burst_size;
char *end = NULL;
int ret;
/* parse decimal string */
pkt_burst = strtoul(optarg, &end, 10);
if ((optarg[0] == '\0') || (end == NULL) || (*end != '\0'))
return;
if (pkt_burst > MAX_PKT_BURST) {
RTE_LOG(INFO, L3FWD, "User provided burst must be <= %d. Using default value %d\n",
MAX_PKT_BURST, *burst_sz);
return;
} else if (pkt_burst > 0) {
*burst_sz = (uint32_t)pkt_burst;
return;
}
if (is_rx_burst) {
/* If user gives a value of zero, query the PMD for its recommended
* Rx burst size.
*/
- ret = rte_eth_dev_info_get(0, &dev_info);
+ ret = rte_eth_dev_info_get(portid, &dev_info);
if (ret != 0)
return;
burst_size = dev_info.default_rxportconf.burst_size;
if (burst_size == 0) {
RTE_LOG(INFO, L3FWD, "PMD does not recommend a burst size. Using default value %d. "
"User provided value must be in [1, %d]\n",
rx_burst_size, MAX_PKT_BURST);
return;
} else if (burst_size > MAX_PKT_BURST) {
RTE_LOG(INFO, L3FWD, "PMD recommended burst size %d exceeds maximum value %d. "
"Using default value %d\n",
burst_size, MAX_PKT_BURST, rx_burst_size);
return;
}
*burst_sz = burst_size;
RTE_LOG(INFO, L3FWD, "Using PMD-provided RX burst value %d\n", burst_size);
} else {
RTE_LOG(INFO, L3FWD, "User provided TX burst is 0. Using default value %d\n",
*burst_sz);
}
}Note: You'll need to update the call sites (lines 950, 954) to determine the appropriate portid to pass.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In examples/l3fwd/main.c around lines 725 to 745, the code queries PMD device
info using a hardcoded port 0 which is then applied globally; change the
function to accept a portid parameter and use that portid in
rte_eth_dev_info_get instead of 0, propagate that portid when calling the
function (update call sites around lines 950 and 954 to pass the appropriate
port id), and adjust the function signature and any related variable
names/comments to reflect per-port burst size retrieval so each port queries its
own dev_info before setting burst_sz.
| case CMD_LINK_OPT_ETH_LINK_SPEED_NUM: | ||
| speed_num = atoi(optarg); | ||
| if ((speed_num == RTE_ETH_SPEED_NUM_10M) || | ||
| (speed_num == RTE_ETH_SPEED_NUM_100M)) { | ||
| fprintf(stderr, "Unsupported fixed speed\n"); | ||
| print_usage(prgname); | ||
| return -1; | ||
| } | ||
| if (speed_num >= 0 && rte_eth_speed_bitflag(speed_num, 0) > 0) | ||
| port_conf.link_speeds = rte_eth_speed_bitflag(speed_num, 0); | ||
| break; |
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.
Incomplete speed validation allows silent failures.
The implementation explicitly rejects 10M and 100M speeds but silently ignores other invalid speeds. If rte_eth_speed_bitflag returns 0 (indicating an invalid speed), link_speeds remains unset without any error message. Additionally, atoi does not handle parsing errors robustly.
Consider improving the validation:
case CMD_LINK_OPT_ETH_LINK_SPEED_NUM:
speed_num = atoi(optarg);
if ((speed_num == RTE_ETH_SPEED_NUM_10M) ||
(speed_num == RTE_ETH_SPEED_NUM_100M)) {
fprintf(stderr, "Unsupported fixed speed\n");
print_usage(prgname);
return -1;
}
- if (speed_num >= 0 && rte_eth_speed_bitflag(speed_num, 0) > 0)
+ if (speed_num > 0) {
+ uint32_t speed_flag = rte_eth_speed_bitflag(speed_num, 0);
+ if (speed_flag == 0) {
+ fprintf(stderr, "Invalid link speed: %d\n", speed_num);
+ print_usage(prgname);
+ return -1;
+ }
- port_conf.link_speeds = rte_eth_speed_bitflag(speed_num, 0);
+ port_conf.link_speeds = speed_flag;
+ }
break;🤖 Prompt for AI Agents
In examples/l3fwd/main.c around lines 930 to 940, the speed parsing and
validation is incomplete: use a robust parse (strtol) and check for conversion
errors and range, keep the existing explicit rejection for RTE_ETH_SPEED_NUM_10M
and RTE_ETH_SPEED_NUM_100M, then call rte_eth_speed_bitflag and verify its
result is non‑zero; if it returns zero (invalid speed) print an error and usage
and return -1, otherwise assign port_conf.link_speeds to the returned bitflag.
Ensure to handle errno from strtol and reject non‑numeric or out‑of‑range
inputs.
|
|
||
| default_cflags = machine_args | ||
| if cc.has_argument('-Wno-format-truncation') | ||
| default_cflags += '-Wno-format-truncation' | ||
| endif | ||
|
|
||
| default_ldflags = dpdk_extra_ldflags |
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.
Dropping -Wno-format-truncation for examples changes compiler warning behavior
Previously, example builds conditionally disabled -Wformat-truncation when the compiler supported that flag; now default_cflags is just machine_args, so those warnings will surface again and may turn into build failures in -Werror environments. If this is intentional (to catch real truncation bugs), it would be good to confirm CI coverage and, if needed, call this out in release notes; otherwise consider restoring the conditional flag or handling it per-example.
🤖 Prompt for AI Agents
In examples/meson.build around lines 81 to 84, removing the conditional addition
of -Wno-format-truncation causes format-truncation warnings to reappear and may
break builds under -Werror; restore the previous behavior by reintroducing a
compiler-feature check and only add -Wno-format-truncation when the compiler
supports it (or alternatively add the flag per-example where needed), or
explicitly document and ensure CI covers the new stricter warnings; update the
meson logic to detect support for -Wno-format-truncation and conditionally
append it to default_cflags (or set example-specific cflags) so builds remain
consistent.
NOTE: This is an auto submission for "[v2] net/nbl: add VLAN offload set interface".
See "http://patchwork.dpdk.org/project/dpdk/list/?series=36804" for details.
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.