-
Notifications
You must be signed in to change notification settings - Fork 23
dpdk: update to dpdk 25.11 #422
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
Conversation
5f4c7b6 to
aa41815
Compare
b36b940 to
49f4293
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis pull request upgrades DPDK from version 24.11.1 to 25.11 and updates the corresponding build configuration. It standardizes struct packing attributes across multiple datapath headers from Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
Upgrade dpdk version to 25.11.
- remove vendor patches.
- update meson.build file
- update the minimum required version
- enable_kmods option has been removed
- drivers: "common/iavf" is removed as well
- treewide: replace deprecated __rte_packed
Use standard __attribute__((packed)) instead of the new
__rte_packed_begin and __rte_packed_end macros.
Signed-off-by: Christophe Fontaine <[email protected]>
Now that we have DPDK 25.11 we can use rte_node_free(). Signed-off-by: Robin Jarry <[email protected]>
49f4293 to
c7f9e72
Compare
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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
meson.build(1 hunks)modules/dhcp/control/client.h(1 hunks)modules/infra/control/graph.c(1 hunks)modules/infra/datapath/gr_icmp6.h(5 hunks)modules/infra/datapath/gr_lacp.h(2 hunks)subprojects/dpdk.wrap(1 hunks)subprojects/packagefiles/dpdk/0001-net-tap-add-netlink-helpers.patch(0 hunks)subprojects/packagefiles/dpdk/0002-net-tap-replace-ioctl-with-netlink.patch(0 hunks)subprojects/packagefiles/dpdk/0003-net-tap-detect-namespace-change.patch(0 hunks)subprojects/packagefiles/dpdk/0004-net-tap-configure-link-carrier.patch(0 hunks)subprojects/packagefiles/dpdk/net-ipv6-link-local-compliance-with-rfc-4291.diff(0 hunks)
💤 Files with no reviewable changes (5)
- subprojects/packagefiles/dpdk/0004-net-tap-configure-link-carrier.patch
- subprojects/packagefiles/dpdk/0002-net-tap-replace-ioctl-with-netlink.patch
- subprojects/packagefiles/dpdk/0001-net-tap-add-netlink-helpers.patch
- subprojects/packagefiles/dpdk/net-ipv6-link-local-compliance-with-rfc-4291.diff
- subprojects/packagefiles/dpdk/0003-net-tap-detect-namespace-change.patch
🚧 Files skipped from review as they are similar to previous changes (1)
- subprojects/dpdk.wrap
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{c,h}
⚙️ CodeRabbit configuration file
**/*.{c,h}: -gr_vec_*()functions cannot fail. No need to check their return value.
gr_vec_free(x)always setsx = NULL. There is no risk of double free.ec_node_*()functions consume theirec_nodearguments. No leaks on error.rte_node->ctxis an uint8_t array of size 16, not a pointer.- Don't suggest to replace
assert()with graceful error checking.- We compile with
-std=gnu2x. Unnamed parameters in function definitions are valid.
Files:
modules/infra/control/graph.cmodules/dhcp/control/client.hmodules/infra/datapath/gr_icmp6.hmodules/infra/datapath/gr_lacp.h
🧠 Learnings (6)
📚 Learning: 2025-11-03T13:28:19.489Z
Learnt from: rjarry
Repo: DPDK/grout PR: 379
File: modules/infra/datapath/port_tx.c:36-46
Timestamp: 2025-11-03T13:28:19.489Z
Learning: In DPDK graph node process callbacks, the return value is only used for statistics and does not affect packet flow or scheduling through the graph. Nodes can return 0 when they haven't processed packets (e.g., when dropping or redirecting due to port state).
Applied to files:
modules/infra/control/graph.c
📚 Learning: 2025-09-09T09:22:31.596Z
Learnt from: maxime-leroy
Repo: DPDK/grout PR: 309
File: modules/srv6/datapath/srv6_local.c:97-101
Timestamp: 2025-09-09T09:22:31.596Z
Learning: In SRv6 IPv6 extension header parsing, when is_ipv6_ext[proto] is true, rte_ipv6_get_next_ext() will not return a negative value, making error checking unnecessary. An assert can be used as a defensive measure for future DPDK compatibility.
Applied to files:
modules/infra/datapath/gr_icmp6.h
📚 Learning: 2025-11-24T12:50:09.310Z
Learnt from: rjarry
Repo: DPDK/grout PR: 408
File: api/gr_net_types.h:78-84
Timestamp: 2025-11-24T12:50:09.310Z
Learning: In api/gr_net_types.h, ip4_addr_t values are stored in network byte order (big-endian). When using a union to cast to a byte array (uint8_t u8[4]), accessing u8[0] correctly reads the first octet (most significant byte) of the IP address on all architectures, because it directly accesses the memory layout rather than performing integer operations. This pattern is valid for checking properties like multicast (first octet in range 224-239).
Applied to files:
modules/infra/datapath/gr_icmp6.h
📚 Learning: 2025-11-05T14:28:28.817Z
Learnt from: rjarry
Repo: DPDK/grout PR: 372
File: modules/infra/datapath/loop_input.c:53-56
Timestamp: 2025-11-05T14:28:28.817Z
Learning: In modules/infra/datapath/loop_input.c, the l3_edges array is intentionally indexed using network byte order (rte_be16_t) for EtherType values. Both loopback_input_add_type() registration and loopback_input_process() lookup use network byte order consistently, so no byte order conversion is needed when accessing l3_edges.
Applied to files:
modules/infra/datapath/gr_icmp6.hmodules/infra/datapath/gr_lacp.h
📚 Learning: 2025-09-09T06:02:47.703Z
Learnt from: maxime-leroy
Repo: DPDK/grout PR: 309
File: modules/srv6/datapath/srv6_local.c:88-122
Timestamp: 2025-09-09T06:02:47.703Z
Learning: When parsing IPv6 routing headers in SRv6 processing, bounds checking for the full extension header length and validation of the routing header structure is needed before accessing fields like hdr_len, last_entry, segments_left, and type.
Applied to files:
modules/infra/datapath/gr_icmp6.h
📚 Learning: 2025-09-09T06:02:47.703Z
Learnt from: maxime-leroy
Repo: DPDK/grout PR: 309
File: modules/srv6/datapath/srv6_local.c:88-122
Timestamp: 2025-09-09T06:02:47.703Z
Learning: The DPDK function rte_ipv6_get_next_ext() only reads 2 bytes from the extension header, so a 2-byte precheck is sufficient before calling it.
Applied to files:
modules/infra/datapath/gr_icmp6.h
⏰ 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). (7)
- GitHub Check: build-and-tests (clang-15, none, debugoptimized, ubuntu-22.04, false)
- GitHub Check: build-and-tests (clang-18, none, debugoptimized, ubuntu-24.04, false)
- GitHub Check: build-and-tests (clang-16, none, debugoptimized, ubuntu-24.04, false)
- GitHub Check: build-and-tests (gcc-13, none, debugoptimized, ubuntu-24.04, -Dfrr=enabled, false)
- GitHub Check: build-and-tests (gcc-14, address, debug, ubuntu-24.04, -Dfrr=enabled, true)
- GitHub Check: deb
- GitHub Check: rpm
🔇 Additional comments (8)
modules/infra/control/graph.c (1)
317-318: Verifyrte_node_free()error handling in DPDK 25.11.
rte_node_free()is available in DPDK 25.11 and returns 0 on success or a negative value (errno-style code) on failure. If this function can fail in your use case, consider adding error logging for consistency with other DPDK resource cleanup in this file.modules/infra/datapath/gr_lacp.h (2)
73-73: LGTM!The packing attribute syntax is updated while maintaining the same attribute order (packed before aligned).
99-99: LGTM!Consistent with the struct lacp_participant change above.
meson.build (3)
61-61: LGTM!Version requirement correctly updated to match DPDK 25.11 upgrade.
67-67: Verify that all required drivers are included.The driver list has been simplified. Ensure all drivers needed for supported hardware are present. Clarify whether
common/iavf(mentioned in the change summary) is distinct fromnet/iavf(currently in the enabled list) and confirm whether its removal, if applicable, is intentional and does not impact functionality.
71-71: Thedeveloper_mode=disabledoption is a valid DPDK Meson build feature. DPDK supports this option with three values: auto (default, enabled when .git present), enabled, and disabled. Disabling it for production builds is appropriate as it disables extra developer-only build checks.modules/infra/datapath/gr_icmp6.h (2)
39-39: LGTM!Packing attribute syntax updated consistently across all ICMPv6 structs.
Also applies to: 44-44, 49-49, 54-54, 59-59, 65-65, 71-71, 76-76, 90-90, 96-96, 110-110, 126-126
135-135: Attribute order change is functionally equivalent.The reordering from
__rte_aligned(2) __rte_packedto__attribute__((packed)) __rte_aligned(2)does not affect struct layout. Both__attribute__((packed))and__rte_aligned()are independent attributes; the compiler applies them regardless of declaration order. The result in both cases is packed member layout with 2-byte struct alignment. No verification needed.Likely an incorrect or invalid review comment.
| rte_be32_t magic; | ||
| uint8_t options[]; | ||
| } __rte_packed; | ||
| } __attribute__((packed)); |
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
🌐 Web query:
DPDK 25.11 release notes __rte_packed removed
💡 Result:
Yes — the old __rte_packed macro was removed (replaced by the begin/end pair). DPDK now uses __rte_packed_begin / __rte_packed_end (change announced in release notes/patches and reflected in 25.11 headers). [1][2][3]
Sources
- DPDK release notes (noting replacement of __rte_packed with __rte_packed_begin / __rte_packed_end). [1]
- Dev mailing-list patch discussion and plan to remove __rte_packed (targeted for 25.11). [2]
- DPDK API/header (rte_common.h) showing __rte_packed deprecated and the begin/end macros in 25.11 tree. [3]
Replace __attribute__((packed)) with DPDK's __rte_packed_begin / __rte_packed_end macros.
While __rte_packed was removed in DPDK 25.11, the replacement should use __rte_packed_begin and __rte_packed_end as recommended in DPDK release notes. Using raw __attribute__((packed)) bypasses DPDK's macro layer and may cause portability issues.
🤖 Prompt for AI Agents
In modules/dhcp/control/client.h around line 82, the struct closure uses the raw
GCC attribute "__attribute__((packed))"; replace it with DPDK's recommended
packing macros by wrapping the struct declaration with __rte_packed_begin before
the struct and __rte_packed_end after the struct (or use the pair around the
typedef) so the compiler packing is applied via DPDK's portability layer; update
the surrounding include/guards if necessary to ensure those macros are
available.
Upgrade to dpdk 25.11
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.