Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
💤 Files with no reviewable changes (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughUpdates the DPDK Meson wrap to download dpdk-25.11.1 (directory dpdk-stable-25.11.1) and changes the wrap's diff_files list. Several prior patches were removed (hash RCU dequeue fallback, hash RCU overwrite free, iavf queue-reporting change, and others), while the fib-expose-tbl8-usage-statistics patch is retained. A new patch adds an L2 ptype assignment fix in the VLAN parsing loop and a unit test exercising L2/pkt-type and header-lengths. The TAP driver patch introducing software MAC filtering and related MAC/storage changes is included. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
subprojects/packagefiles/dpdk/net-tap-add-software-MAC-address-filtering.patch (2)
158-167:⚠️ Potential issue | 🟠 Major
rte_reallocfailure leaks the previousmc_addrsbuffer.Assigning the result of
rte_realloc(pmd->mc_addrs, ...)directly back topmd->mc_addrsloses the original pointer when realloc fails. DPDK'srte_reallocdoes not free the old allocation on failure—it returns NULL and leaves the original buffer allocated. The overwrite causes a memory leak with no remaining pointer to the old buffer. Use a temporary variable and only updatepmd->mc_addrsafter confirming success.Proposed fix
- pmd->mc_addrs = rte_realloc(pmd->mc_addrs, - nb_mc_addr * sizeof(*pmd->mc_addrs), 0); - if (pmd->mc_addrs == NULL) { - pmd->nb_mc_addrs = 0; - return -ENOMEM; - } - - memcpy(pmd->mc_addrs, mc_addr_set, - nb_mc_addr * sizeof(*pmd->mc_addrs)); + struct rte_ether_addr *new_addrs = rte_realloc(pmd->mc_addrs, + nb_mc_addr * sizeof(*pmd->mc_addrs), 0); + if (new_addrs == NULL) + return -ENOMEM; + + memcpy(new_addrs, mc_addr_set, nb_mc_addr * sizeof(*new_addrs)); + pmd->mc_addrs = new_addrs; pmd->nb_mc_addrs = nb_mc_addr;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@subprojects/packagefiles/dpdk/net-tap-add-software-MAC-address-filtering.patch` around lines 158 - 167, The current code assigns the result of rte_realloc directly to pmd->mc_addrs which loses the original pointer on failure; change it to use a temporary pointer (e.g., tmp = rte_realloc(pmd->mc_addrs, nb_mc_addr * sizeof(*pmd->mc_addrs), 0)), check tmp for NULL and return -ENOMEM without touching pmd->mc_addrs or pmd->nb_mc_addrs if allocation fails, and only on success assign pmd->mc_addrs = tmp, memcpy from mc_addr_set into pmd->mc_addrs and then set pmd->nb_mc_addrs = nb_mc_addr.
107-125:⚠️ Potential issue | 🔴 CriticalCritical memory leak:
mac_addrsnot freed after allocation change.The patch allocates
data->mac_addrsindependently viarte_calloc_socket()forTAP_MAX_MAC_ADDRSentries but removes its cleanup fromtap_dev_close(). Unlike the old code wheremac_addrspointed intodev_private(and therefore could not be freed alone), the new standalone allocation must be explicitly freed. The normal close path leaks this memory. The error path ineth_dev_tap_create()relies onrte_eth_dev_release_port(), which is an internal function designed for secondary process detach—not full port teardown—and does not document explicit cleanup ofdev->dataor its contents. Addrte_free(dev->data->mac_addrs)before callingrte_eth_dev_release_port()in the error path and intap_dev_close()to prevent leaks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@subprojects/packagefiles/dpdk/net-tap-add-software-MAC-address-filtering.patch` around lines 107 - 125, The mac_addrs buffer allocated via rte_calloc_socket for TAP_MAX_MAC_ADDRS is now standalone and must be freed to avoid leaks; update tap_dev_close (function tap_dev_close) to call rte_free(dev->data->mac_addrs) and set dev->data->mac_addrs = NULL and nb_mc_addrs = 0 (or internals->nb_mc_addrs = 0) before releasing other resources, and also add the same rte_free(dev->data->mac_addrs) cleanup in the error path of eth_dev_tap_create() before calling rte_eth_dev_release_port(); reference the allocation site (rte_calloc_socket) and use rte_free to free the buffer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@subprojects/packagefiles/dpdk/net-tap-add-software-MAC-address-filtering.patch`:
- Around line 158-167: The current code assigns the result of rte_realloc
directly to pmd->mc_addrs which loses the original pointer on failure; change it
to use a temporary pointer (e.g., tmp = rte_realloc(pmd->mc_addrs, nb_mc_addr *
sizeof(*pmd->mc_addrs), 0)), check tmp for NULL and return -ENOMEM without
touching pmd->mc_addrs or pmd->nb_mc_addrs if allocation fails, and only on
success assign pmd->mc_addrs = tmp, memcpy from mc_addr_set into pmd->mc_addrs
and then set pmd->nb_mc_addrs = nb_mc_addr.
- Around line 107-125: The mac_addrs buffer allocated via rte_calloc_socket for
TAP_MAX_MAC_ADDRS is now standalone and must be freed to avoid leaks; update
tap_dev_close (function tap_dev_close) to call rte_free(dev->data->mac_addrs)
and set dev->data->mac_addrs = NULL and nb_mc_addrs = 0 (or
internals->nb_mc_addrs = 0) before releasing other resources, and also add the
same rte_free(dev->data->mac_addrs) cleanup in the error path of
eth_dev_tap_create() before calling rte_eth_dev_release_port(); reference the
allocation site (rte_calloc_socket) and use rte_free to free the buffer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2ca0d4ec-b07e-4aed-9791-e71be437fc3e
📒 Files selected for processing (7)
subprojects/dpdk.wrapsubprojects/packagefiles/dpdk/fib-expose-tbl8-usage-statistics.patchsubprojects/packagefiles/dpdk/fib-fix-prefix-addition-handling.patchsubprojects/packagefiles/dpdk/hash-avoid-leaking-entries-on-RCU-defer-queue-failur.patchsubprojects/packagefiles/dpdk/hash-free-replaced-data-on-overwrite-when-RCU-is-con.patchsubprojects/packagefiles/dpdk/iavf-fix-reported-max-TX-and-RX-queues-in-ethdev-inf.patchsubprojects/packagefiles/dpdk/net-tap-add-software-MAC-address-filtering.patch
💤 Files with no reviewable changes (4)
- subprojects/packagefiles/dpdk/hash-avoid-leaking-entries-on-RCU-defer-queue-failur.patch
- subprojects/packagefiles/dpdk/iavf-fix-reported-max-TX-and-RX-queues-in-ethdev-inf.patch
- subprojects/packagefiles/dpdk/fib-fix-prefix-addition-handling.patch
- subprojects/packagefiles/dpdk/hash-free-replaced-data-on-overwrite-when-RCU-is-con.patch
Update to the latest bugfix release. Remove patches which have been accepted upstream. Rebase the remaining two patches on top of 25.11.1. There is a regression in rte_net_get_ptype(). Include a fix that was sent upstream. Link: https://inbox.dpdk.org/announce/20260421160029.327781-1-ktraynor@redhat.com/ Link: https://patches.dpdk.org/project/dpdk/patch/20260422103838.649578-2-rjarry@redhat.com/ Signed-off-by: Robin Jarry <rjarry@redhat.com>
1 participant
Update to the latest bugfix release. Remove patches which have been accepted upstream. Rebase the remaining two patches on top of 25.11.1.
There is a regression in rte_net_get_ptype(). Include a fix that was sent upstream.
Link: https://inbox.dpdk.org/announce/20260421160029.327781-1-ktraynor@redhat.com/
Link: https://patches.dpdk.org/project/dpdk/patch/20260422103838.649578-2-rjarry@redhat.com/
Update DPDK to 25.11.1 stable
Updated
subprojects/dpdk.wrapto fetch and extractdpdk-25.11.1.tar.xzfrom thedpdk-stable-25.11.1directory with updated source hash and filename.Removed five patches that have been accepted upstream in DPDK 25.11.1:
hash-avoid-leaking-entries-on-RCU-defer-queue-failur.patchhash-free-replaced-data-on-overwrite-when-RCU-is-con.patchiavf-fix-reported-max-TX-and-RX-queues-in-ethdev-inf.patchnet-tap-add-software-MAC-address-filtering.patchRebased two remaining patches on top of 25.11.1:
fib-expose-tbl8-usage-statistics.patch: Adds experimental API functionsrte_fib_tbl8_get_stats()andrte_fib6_tbl8_get_stats()to expose TBL8 group usage statistics for DIR24-8 and trie FIB implementations respectively.fib-fix-prefix-addition-handling.patch: Fixes early-return behavior in FIB prefix insertion to ensure reserved TBL8 counter (dp->rsvd_tbl8s) is consistently updated during prefix addition for both IPv4 and IPv6 FIB implementations.