* [PATCH v4 0/6] net/dpaa2: NAPI-style Rx queue interrupts
From: Maxime Leroy @ 2026-07-10 21:09 UTC (permalink / raw)
To: dev; +Cc: Hemant Agrawal, Jun Yang, Maxime Leroy
In-Reply-To: <20260630144329.457643-1-maxime@leroys.fr>
This series lets a dpaa2 worker sleep on a queue's data-availability
notification instead of busy-polling, exposed through the generic
rte_eth_dev_rx_intr_* API (NAPI-style: poll while frames keep coming,
arm the interrupt and sleep when the queue runs dry).
Why it is not a trivial .rx_queue_intr_enable
----------------------------------------------
A worker wakes on its software portal's DQRI, which fires when the
portal's DQRR holds frames. The default dpaa2 Rx burst pulls frames
from the FQ with a volatile dequeue and cannot be interrupt-driven; to
wake on the DQRI the FQ must instead be pushed to the portal's DQRR.
The natural dpni_set_queue with a notification destination would have to
target the worker's portal, but that portal is only known once a worker
affines, after dev_start, and that MC command holds the global MC lock
long enough to wedge the firmware while traffic runs. So the bind cannot
be done late, against the polling lcore.
Design
------
Each Rx FQ is bound to its own DPCON channel, statically, at
rx_queue_setup while the dpni is still disabled (no knowledge of the
polling lcore). It stays scheduled to that channel for the life of the
port and is never re-bound, because the MC refuses to move a scheduled
FQ back to parked at runtime; a queue-count reconfigure (stop, configure,
setup, start) is therefore supported without a port close. A worker later
subscribes its own ethrx portal to the channel and arms the DQRI in
rx_queue_intr_enable, a one-shot per-portal op, never the wedging
set_queue. On a wakeup the worker drains each of its queues by a volatile
dequeue on the queue's own DPCON channel (one FQ per channel, so no
per-frame demux); it polls all its queues, the same scheduling contract
as plain DPDK polling. A queue can be re-homed to another lcore at
runtime with no set_queue and no port stop.
This reuses the event PMD's pushed/DQRR model but with one DPCON per FQ
and static affinity (no QBMan scheduling), so the DPCON allocator is
moved from the event driver to the fslmc bus and shared.
Patch 1 fixes an existing dpaa2 bug the interrupt path relies on: the
rx_queue_count callback was wired only for the secondary process, so the
generic arm-then-verify (rte_eth_rx_queue_count) misbehaves on a primary
process. Patches 2 to 4 move the DPCON allocator to the fslmc bus, make
the portal DQRI epoll optional, and add the DPCON set-notification MC
command. Patch 5 adds the interrupt support proper; patch 6 pins each
DPIO's MSI to the lcore that arms it, a latency optimisation.
Tested on LX2160A (lx2160acex7).
v4:
- Addressed Hemant's v3 review:
- Dropped the "event/dpaa2: disable channel before closing it" patch.
Pooled DPAA2 objects reprogram on borrow (the sibling DPCI close has
no dpci_disable either), so an enabled DPCON returned to the pool is
inert; the patch was an unnecessary outlier.
- Allocate the rx-interrupt handle once and keep it, instead of calling
rte_intr_vec_list_alloc() again on the existing handle at a second
dev_configure (which leaked the vector list and could size-mismatch).
- Store napi_sub_dpio with rte_atomic_store_explicit() at rx_queue_setup
instead of a plain assignment, for weakly-ordered architectures.
- Re-added the "fix Rx queue count for primary process" patch (patch 1).
v3 dropped it as applied to main, but only the ethdev dummy/keep-fast-
path fixes are upstream; the dpaa2 primary-process wiring is not, and
rte_eth_rx_queue_count returns 0 (defeating arm-then-verify) without it.
- Made the Rx queue count changeable at runtime in interrupt mode: the FQ
is scheduled to its DPCON once, at rx_queue_setup, and never re-bound
(the handle kept above lets the application's rx-intr epoll registration
stay valid across a reconfigure). Switching the interrupt mode on or off
is refused after the first configure.
- Advertise the supported packet types in interrupt mode too:
dpaa2_supported_ptypes_get returned NULL for the channel Rx bursts
(dpaa2_dev_rx_channel, dpaa2_dev_prefetch_rx_channel), so an application
saw no ptypes although they are parsed; the bursts are now recognised.
- Documented the arm-before-register contract (the interrupt fd is bound
at arm time) in doc/guides/nics/dpaa2.rst.
- Rebased on next-net/for-main (v3 was on an older main snapshot).
- Reworded the DPCON set-notification headline to drop the underscore.
v3:
- Reworked the Rx drain. Both versions bind one DPCON per FQ, but v2
drained the shared portal DQRR and demuxed frames to their FQ by
fqd_ctx, stashing foreign frames in a per-queue FIFO. v3 drains each
queue with a volatile dequeue on its own channel (one FQ per channel),
which drops the demux and stash code.
- Dropped the rx_queue_count fix; it is applied to main.
- Dropped the software VLAN strip patch; an independent net/dpaa2 cleanup,
sent standalone and now applied to next-net.
- Dropped the Depends-on: the ethdev fast-path ops fix is now in main.
- Split the dpcon_set_notification MC command into its own patch.
- Added an event/dpaa2 fix to disable the DPCON channel before close,
needed once the allocator is shared.
- Dropped the DQRI holdoff-tuning patch; the immediate-DQRI holdoff is now
set inline in the arm path.
- Added a patch reusing the event driver's MSI-affinity helper (exposed
from its RTE_EVENT_DPAA2 guard) to pin the portal MSI to the lcore that
arms it, so a CDAN wake lands on the worker's own core.
v2:
- Dropped the RSS RETA patch, an independent net/dpaa2 change the
interrupt path does not require; it will be sent as its own series.
- Dropped the ethdev fast-path ops fix; it is now a standalone series.
- Dropped the eal/interrupts -EEXIST fix, applied to main by David
Marchand.
- Declared qbman_swp_interrupt_set_inhibit and qbman_swp_dqrr_size
__rte_internal (David Marchand).
- Minor formatting cleanup in the Rx interrupt setup.
Maxime Leroy (6):
net/dpaa2: fix Rx queue count for primary process
bus/fslmc: move DPCON management from event driver to bus
bus/fslmc/dpio: make the portal DQRI epoll optional
bus/fslmc/mc: implement DPCON set notification
net/dpaa2: support Rx queue interrupts
net/dpaa2: pin Rx queue interrupt to the polling core
doc/guides/nics/dpaa2.rst | 28 ++
doc/guides/nics/features/dpaa2.ini | 1 +
doc/guides/rel_notes/release_26_07.rst | 1 +
drivers/bus/fslmc/mc/dpcon.c | 31 ++
drivers/bus/fslmc/mc/fsl_dpcon.h | 18 +
drivers/bus/fslmc/meson.build | 1 +
.../fslmc/portal}/dpaa2_hw_dpcon.c | 16 +-
drivers/bus/fslmc/portal/dpaa2_hw_dpio.c | 125 ++++--
drivers/bus/fslmc/portal/dpaa2_hw_dpio.h | 12 +
drivers/bus/fslmc/portal/dpaa2_hw_pvt.h | 15 +
.../fslmc/qbman/include/fsl_qbman_portal.h | 5 +
drivers/bus/fslmc/qbman/qbman_portal.c | 5 +
drivers/event/dpaa2/dpaa2_eventdev.h | 3 -
drivers/event/dpaa2/meson.build | 1 -
drivers/net/dpaa2/dpaa2_ethdev.c | 414 +++++++++++++++++-
drivers/net/dpaa2/dpaa2_ethdev.h | 7 +
drivers/net/dpaa2/dpaa2_rxtx.c | 114 ++++-
17 files changed, 725 insertions(+), 72 deletions(-)
rename drivers/{event/dpaa2 => bus/fslmc/portal}/dpaa2_hw_dpcon.c (90%)
--
2.43.0
^ permalink raw reply
* Re: [PATCH v4 2/9] net/gve: delay adding mbuf head to software ring
From: Joshua Washington @ 2026-07-10 20:33 UTC (permalink / raw)
To: Thomas Monjalon
Cc: Jeroen de Borst, Jasper Tran O'Leary, Junfeng Guo, Xiaoyun Li,
stable, dev
In-Reply-To: <l7g2E_cUTtqIHhK0pfFLlQ@monjalon.net>
On Fri, Jul 10, 2026 at 10:45 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 09/07/2026 06:07, Joshua Washington:
> > The GQ TX datapath was set up to write the mbuf head into the sw_ring
> > before writing the descriptors. This poses a problem because it's
> > possible for the packet to be dropped due to lacking the FIFO space to
> > do a proper TX. In such a case, the packet won't be sent, and will lead
> > to leaked mbufs in the subsequent segments.
> >
> > There is also no real reason that the head mbuf must be set in the
> > sw_ring separately from the others; the mbuf chain is not actually
> > walked as part of GQ TX.
> >
> > Fixes: a46583cf43c8 ("net/gve: support Rx/Tx")
> > Cc: stable@dpdk.org
> > Signed-off-by: Joshua Washington <joshwash@google.com>
> > Reviewed-by: Jasper Tran O'Leary <jtranoleary@google.com>
>
> AI found this issue:
>
> **Error:** Resource leak on early return from `gve_tx_burst_qpl()`.
>
> The patch modifies the loop to store mbufs in `sw_ring` only after checking FIFO availability, but introduces a leak when `goto end_of_tx` is taken after the first segment has been processed. At line 306-307:
>
> ```c
> if (!is_fifo_avail(txq, hlen)) {
> gve_tx_clean(txq);
> if (!is_fifo_avail(txq, hlen))
> goto end_of_tx;
> }
> ```
>
> If the second `is_fifo_avail()` check fails and we `goto end_of_tx`, the current `tx_pkt` (which is the mbuf chain head) has not been added to `sw_ring` yet (that happens later in the modified loop at lines 346-351). The function will return without freeing `tx_pkt`, and the mbuf chain is lost.
>
> **Suggested fix:** Free the current packet before `goto end_of_tx`:
>
> ```c
> if (!is_fifo_avail(txq, hlen)) {
> gve_tx_clean(txq);
> if (!is_fifo_avail(txq, hlen)) {
> rte_pktmbuf_free(tx_pkt);
> goto end_of_tx;
> }
> }
> ```
>
> Similarly, for the TSO path at lines 314-316, if the FIFO space check fails, `tx_pkt` should be freed before the goto.
I believe this is a hallucination. The driver does not free the TX
packet in the case of a failed send because it does not count that
packet in the packets sent to the application. It ultimately did not
take ownership of the mbuf for the failing packet from the
application, so it should not free that mbuf. The tx_id pointer is not
incremented until after it is sure that the packet data can be copied
to the DMA area (FIFO buffer), so there is no unintended descriptor
write either, as far as I can tell.
-- Josh
^ permalink raw reply
* Re: [PATCH 0/4] app/testpmd: fix socket id handling with NUMA disabled
From: David Marchand @ 2026-07-10 16:43 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev
In-Reply-To: <20260702161244.363233-1-stephen@networkplumber.org>
On Thu, 2 Jul 2026 at 18:13, Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> Alternative to the patch "app/testpmd: fix multi-pool Rx setup with --no-NUMA"
> from Maayan Kashani, fixing the problem at the source
> rather than remapping inside mbuf_pool_find().
>
> Since commit 835fd4893a31 ("app/testpmd: support cross-NUMA allocations"),
> mbuf pools are created (and named) on SOCKET_ID_ANY
> when NUMA support is disabled, but port->socket_id still holds 0/--socket-num.
> Every name-based pool lookup that does not go through the single workaround in
> start_port() misses. The reported multi-pool Rx failure is one instance; plain
> "testpmd --no-numa --forward-mode=txonly" also crashes with a NULL
> mempool dereference, and --socket-num no longer affects pool allocation.
>
> Patch 1 restores the invariant that port->socket_id equals the
> socket id the pools are created and named with, and removes the
> start_port() workaround. It should go to stable (the regression
> shipped in 25.07 and 25.11); it cherry-picks onto 25.11 with
> trivial context conflicts and was build- and run-tested there.
>
> Patch 2 consolidates the three diverging copies of the port socket
> id policy into one helper; this also fixes hot-attached ports
> ignoring --port-numa-config, --no-numa and --socket-num.
>
> Patch 3 prints SOCKET_ID_ANY as "any" instead of 4294967295.
>
> Patch 4 adds the error string to the interactive queue setup
> command failure message; rte_eth_rx/tx_queue_setup() can return
> -EBUSY silently when the port is started and the PMD lacks
> runtime queue setup support.
>
> Stephen Hemminger (4):
> app/testpmd: fix port socket ID with NUMA disabled
> app/testpmd: consolidate port socket ID computation
> app/testpmd: display any socket ID as text
> app/testpmd: report reason when queue setup command fails
>
> app/test-pmd/cmdline.c | 12 +++--
> app/test-pmd/config.c | 18 ++++---
> app/test-pmd/testpmd.c | 120 +++++++++++++++++++++--------------------
> app/test-pmd/testpmd.h | 11 ++++
> 4 files changed, 92 insertions(+), 69 deletions(-)
LGTM.
Series applied, thanks for the cleanups.
--
David Marchand
^ permalink raw reply
* Re: [PATCH 2/2] net/ixgbe: fix return value of close operation
From: Bruce Richardson @ 2026-07-10 15:26 UTC (permalink / raw)
To: SidAli CHERRATI; +Cc: dev, Anatoly Burakov, Vladimir Medvedkin, stable
In-Reply-To: <20260710134639.108129-2-scherrati1@gmail.com>
On Fri, Jul 10, 2026 at 03:46:39PM +0200, SidAli CHERRATI wrote:
> rte_intr_callback_unregister() returns the number of unregistered
> callbacks on success. Since the close operation started returning
> the ret variable instead of 0, this positive value leaks as the
> return value of ixgbe_dev_close(), so applications checking
> rte_eth_dev_close() != 0 treat a successful close as a failure.
>
> Fixes: 62024eb82756 ("ethdev: change stop operation callback to return int")
> Cc: stable@dpdk.org
> Signed-off-by: SidAli CHERRATI <scherrati1@gmail.com>
> ---
> drivers/net/intel/ixgbe/ixgbe_ethdev.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/intel/ixgbe/ixgbe_ethdev.c b/drivers/net/intel/ixgbe/ixgbe_ethdev.c
> index c5010f623c..6b1799d7a0 100644
> --- a/drivers/net/intel/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/intel/ixgbe/ixgbe_ethdev.c
> @@ -3121,6 +3121,7 @@ ixgbe_dev_close(struct rte_eth_dev *dev)
> ret = rte_intr_callback_unregister(intr_handle,
> ixgbe_dev_interrupt_handler, dev);
> if (ret >= 0 || ret == -ENOENT) {
> + ret = 0;
> break;
This fixes the issue of returning an invalid value from the interrupt
unregister, but it also means losing the previous value of ret from the
stop call above. I'm therefore wondering if a better fix is to use a new
temporary variable inside the do{ }while loop, and assign that to ret in
the case of an actual error, otherwise leaving ret unmodified.
In that case, we probably want to stop retries on an error too, right?
do {
int cb_ret = rte_intr_callback_unregister(intr_handle,
ixgbe_dev_interrupt_handler, dev);
if (cb_ret >= 0 || cb_ret == -ENOENT) {
break;
} else if (cb_ret != -EAGAIN) {
PMD_INIT_LOG(ERR,
"intr callback unregister failed: %d",
cb_ret);
ret = cb_ret;
break; /* on error, don't retry */
}
rte_delay_ms(100);
} while (retries++ < (10 + IXGBE_LINK_UP_TIME));
IF we do need to retry on error, then this approach won't work. Maybe a
modified version of your suggestion might do instead - rather than assigning
0 to ret on success, we assign a saved off version of it from before the
loop?
What do you think?
/Bruce
^ permalink raw reply
* Re: [PATCH 2/2] net/iavf: disable runtime queue setup during queue rate limiting
From: Bruce Richardson @ 2026-07-10 15:14 UTC (permalink / raw)
To: Dawid Wesierski; +Cc: dev, marek.kasiewicz
In-Reply-To: <20260708231926.1550698-3-dawid.wesierski@intel.com>
On Wed, Jul 08, 2026 at 07:19:26PM -0400, Dawid Wesierski wrote:
> Runtime queue setup on E810 VFs causes queue state corruption when
> queues are dynamically reconfigured while the hardware rate limiter
> is actively pacing TX queues. Queue configuration messages to the PF
> via virtchnl can race with ongoing TX operations, leading to undefined
> behavior.
>
> Rather than gating this behind a devarg that an application would have
> to know to set (and could just as easily avoid triggering the race by
> not calling rte_eth_{rx,tx}_queue_setup() on a running port), stop
> advertising RTE_ETH_DEV_CAPA_RUNTIME_RX_QUEUE_SETUP and
> RTE_ETH_DEV_CAPA_RUNTIME_TX_QUEUE_SETUP as soon as the application
> commits a per-queue bandwidth rte_tm hierarchy, i.e. as soon as the
> condition that causes the race actually exists. iavf_dev_info_get() is
> re-queried by the ethdev layer on every rx/tx_queue_setup() call, so
> this is enough for the generic layer to start rejecting runtime queue
> (re)configuration with -EBUSY once queue rate limiting is active, and
> to automatically allow it again once the rte_tm hierarchy is torn
> down.
>
> vf->qtc_map, already used elsewhere to look up a queue's TC mapping,
> is repurposed as the "queue bandwidth committed" signal since it's set
> by iavf_hierarchy_commit() exactly when a per-queue bandwidth mapping
> has been pushed to the PF, regardless of whether the port was stopped
> at the time. Fix two related issues found while making it load-bearing
> for this check:
>
> - iavf_hierarchy_commit() replaced vf->qtc_map on every successful
> commit without freeing the previous allocation, leaking memory.
> - vf->qtc_map was never released on VF teardown, so
> iavf_uninit_vf()/iavf_init_vf() (e.g. across a VF reset) could leave
> a stale pointer referencing freed unrelated memory, and the runtime
> queue setup capability would never be re-advertised after a reset.
>
> Both are fixed by freeing vf->qtc_map before replacing it in
> iavf_hierarchy_commit(), and freeing and clearing it in
> iavf_uninit_vf().
>
> Signed-off-by: Marek Kasiewicz <marek.kasiewicz@intel.com>
> Signed-off-by: Dawid Wesierski <dawid.wesierski@intel.com>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
However, this patch needs a rebase since some of the changes to cleanup
the qtc_map were made in a previously-applied patch.
> doc/guides/nics/intel_vf.rst | 10 ++++++++++
> doc/guides/rel_notes/release_26_07.rst | 3 +++
> drivers/net/intel/iavf/iavf_ethdev.c | 23 ++++++++++++++++++++---
> drivers/net/intel/iavf/iavf_tm.c | 2 ++
> 4 files changed, 35 insertions(+), 3 deletions(-)
>
> diff --git a/doc/guides/nics/intel_vf.rst b/doc/guides/nics/intel_vf.rst
> index e010f852cf..a47e3f6736 100644
> --- a/doc/guides/nics/intel_vf.rst
> +++ b/doc/guides/nics/intel_vf.rst
> @@ -150,6 +150,16 @@ Intel\ |reg| E800 Series Ethernet devices:
> for example: ``-a 18:00.0,quanta_size=2048``.
> The default value is 1024, and quanta size should be set as the product of 64 in legacy host interface mode.
>
> +* Runtime (post-start) Rx/Tx queue setup can race with the hardware Tx rate
> + limiter on E810 VFs and corrupt queue state.
> + Once an application commits a per-queue bandwidth ``rte_tm`` hierarchy,
> + the driver automatically stops advertising
> + ``RTE_ETH_DEV_CAPA_RUNTIME_RX_QUEUE_SETUP`` and
> + ``RTE_ETH_DEV_CAPA_RUNTIME_TX_QUEUE_SETUP``,
> + so ``rte_eth_rx_queue_setup()``/``rte_eth_tx_queue_setup()``
> + are rejected with ``-EBUSY`` on a running port for as long as queue rate
> + limiting is active.
> +
> * When using the Intel out-of-tree "ice" PF/kernel driver v1.13.7 or later,
> to create VFs with >16 queues (aka. "large VFs"),
> it is necessary to change the rss_lut_vf_addr setting in sysfs from the default of 64 to 512.
> diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
> index 6352ef27ab..7ea80112f4 100644
> --- a/doc/guides/rel_notes/release_26_07.rst
> +++ b/doc/guides/rel_notes/release_26_07.rst
> @@ -143,6 +143,9 @@ New Features
>
> * Added support for transmitting LLDP packets based on mbuf packet type.
> * Implemented AVX2 context descriptor transmit paths.
> + * Runtime Rx/Tx queue setup is now automatically disabled while a
> + per-queue bandwidth ``rte_tm`` hierarchy is committed, to avoid
> + corrupting queue state on E810 VFs.
>
> * **Updated Intel ice driver.**
>
> diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
> index 80e740ef29..5d0c361978 100644
> --- a/drivers/net/intel/iavf/iavf_ethdev.c
> +++ b/drivers/net/intel/iavf/iavf_ethdev.c
> @@ -1160,9 +1160,18 @@ iavf_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
> dev_info->reta_size = vf->vf_res->rss_lut_size;
> dev_info->flow_type_rss_offloads = IAVF_RSS_OFFLOAD_ALL;
> dev_info->max_mac_addrs = IAVF_NUM_MACADDR_MAX;
> - dev_info->dev_capa =
> - RTE_ETH_DEV_CAPA_RUNTIME_RX_QUEUE_SETUP |
> - RTE_ETH_DEV_CAPA_RUNTIME_TX_QUEUE_SETUP;
> + /*
> + * Runtime queue setup can race with the hardware Tx rate limiter on
> + * E810 VFs and corrupt queue state. Once a per-queue bandwidth rte_tm
> + * hierarchy has been committed (vf->qtc_map is set), stop advertising
> + * the capability so the ethdev layer rejects further rx/tx_queue_setup()
> + * calls on a running port with -EBUSY. The capability is re-advertised
> + * automatically once the rte_tm hierarchy is torn down.
> + */
> + if (vf->qtc_map == NULL)
> + dev_info->dev_capa =
> + RTE_ETH_DEV_CAPA_RUNTIME_RX_QUEUE_SETUP |
> + RTE_ETH_DEV_CAPA_RUNTIME_TX_QUEUE_SETUP;
> dev_info->rx_offload_capa =
> RTE_ETH_RX_OFFLOAD_VLAN_STRIP |
> RTE_ETH_RX_OFFLOAD_QINQ_STRIP |
> @@ -2756,6 +2765,14 @@ iavf_uninit_vf(struct rte_eth_dev *dev)
> rte_free(vf->qos_cap);
> vf->qos_cap = NULL;
>
> + /*
> + * Drop the committed queue/TC bandwidth mapping so a subsequent
> + * iavf_init_vf() (e.g. after a device reset) starts with runtime
> + * Rx/Tx queue setup available again (see iavf_dev_info_get()).
> + */
> + rte_free(vf->qtc_map);
> + vf->qtc_map = NULL;
> +
> rte_free(vf->rss_lut);
> vf->rss_lut = NULL;
> rte_free(vf->rss_key);
> diff --git a/drivers/net/intel/iavf/iavf_tm.c b/drivers/net/intel/iavf/iavf_tm.c
> index e3492ec491..c9d856e937 100644
> --- a/drivers/net/intel/iavf/iavf_tm.c
> +++ b/drivers/net/intel/iavf/iavf_tm.c
> @@ -905,6 +905,8 @@ static int iavf_hierarchy_commit(struct rte_eth_dev *dev,
> if (ret_val)
> goto fail_clear;
>
> + /* replace the previously committed mapping, if any */
> + rte_free(vf->qtc_map);
> vf->qtc_map = qtc_map;
> if (adapter->stopped == 1)
> vf->tm_conf.committed = true;
> --
> 2.47.3
>
^ permalink raw reply
* Re: [PATCH 1/2] doc: fix ice scheduler rate-limiter burst size description
From: Bruce Richardson @ 2026-07-10 15:07 UTC (permalink / raw)
To: Dawid Wesierski; +Cc: dev, thomas, stephen, marek.kasiewicz
In-Reply-To: <20260708231926.1550698-2-dawid.wesierski@intel.com>
On Wed, Jul 08, 2026 at 07:19:25PM -0400, Dawid Wesierski wrote:
> The paragraph describing the ``rl_burst_size`` devarg was mangled
> (a mid-sentence line break split "favours throughput" across two
> lines with no space),
Are you sure? It looks fine to me in output generated on my machine. Also,
it's unlikely that changing the source text to add a line break between
"favours" and "throughput" will cause a line break to be removed in the
output.
> and it did not mention that out-of-range
> values are rejected by the driver, which can otherwise look like a
> silent no-op to a reader.
>
Yes, that extra note about invalid values being rejected is missing.
However, the rest of the patch rewrapping the text actually goes against
DPDK documentation policy. In order to keep doc diffs small, we try to
always start a sentence on a new line, and to only break lines at
punctuation marks, or between clauses generally. This saves having massive
diffs as the source text is re-wrapped following a change at the start of a
paragraph.
See https://doc.dpdk.org/guides/contributing/documentation.html#line-length-and-wrapping
> Fixes: b3f2afb3b7ea ("net/ice: add scheduler rate-limiter burst size devarg")
>
> Signed-off-by: Dawid Wesierski <dawid.wesierski@intel.com>
> ---
> doc/guides/nics/ice.rst | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst
> index 5589ce934f..6c2063d47e 100644
> --- a/doc/guides/nics/ice.rst
> +++ b/doc/guides/nics/ice.rst
> @@ -160,9 +160,10 @@ Runtime Configuration
>
> - ``Scheduler rate-limiter burst size`` (default ``0``)
>
> - The hardware Tx scheduler uses a default rate-limiter burst size that favours throughput.
> - Time-sensitive applications can lower this value to reduce Tx latency jitter
> - at the cost of throughput by setting the ``rl_burst_size`` devargs parameter, in bytes.
> + The hardware Tx scheduler uses a default rate-limiter burst size that favours
> + throughput. Time-sensitive applications can lower this value to reduce Tx
> + latency jitter at the cost of throughput by setting the ``rl_burst_size``
> + devargs parameter, in bytes. Values that are out of range are rejected.
When they are rejected, is an error just logged, or does the driver fail to
load due to an incorrect value? IF it's just a log message, is it better to
have the driver do a hard-failure so the user can't miss the issue? That
would be better than just noting this in the docs.
Regards,
/Bruce
> A value of ``0`` (the default) keeps the hardware default.
>
> For example::
> --
> 2.47.3
>
^ permalink raw reply
* Re: [PATCH v3] net/iavf: fix single VLAN strip in double VLAN mode
From: Bruce Richardson @ 2026-07-10 14:52 UTC (permalink / raw)
To: Anurag Mandal; +Cc: dev, vladimir.medvedkin, stable, Ye Chen
In-Reply-To: <20260708185847.550732-1-anurag.mandal@intel.com>
On Wed, Jul 08, 2026 at 06:58:47PM +0000, Anurag Mandal wrote:
> In Double VLAN Mode (DVM) i.e. when RTE_ETH_RX_OFFLOAD_VLAN_EXTEND
> is enabled iavf_config_vlan_strip_v2() only toggles the inner
> VLAN strip setting (0x8100) for the RTE_ETH_RX_OFFLOAD_VLAN_STRIP
> offload, while the outer VLAN strip is controlled independently
> through the QinQ strip path.
>
> However, in DVM, most hardwares place a single (non-QinQ) VLAN
> tag in the outer position
> As a result, when a single VLAN packet is received and the
> application disables single VLAN tag stripping, only the inner
> VLAN tag strip gets turned off and the outer VLAN tag strip
> stays enabled.
> Hence, the single VLAN tag still gets stripped.
> This does not honor the application's request.
>
> This patch fixes the aforesaid case.
> When DVM and QinQ stripping are both enabled, also apply the strip
> enable/disable to the outer VLAN so that single VLAN packets received
> in double VLAN mode respect the single VLAN strip configuration.
>
> Fixes: 8599d7604e0a ("net/iavf: support QinQ strip")
> Cc: stable@dpdk.org
>
> Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
> Tested-by: Ye Chen <yex.chen@intel.com>
> ---
> V3: Restricted to VLAN TPID 0x8100
> V2: Spelling correction in commit message
>
Applied to dpdk-next-net-intel.
Thanks,
/Bruce
^ permalink raw reply
* Re: [PATCH v4 2/9] net/gve: delay adding mbuf head to software ring
From: Thomas Monjalon @ 2026-07-10 14:45 UTC (permalink / raw)
To: Jeroen de Borst, Joshua Washington, Jasper Tran O'Leary
Cc: Junfeng Guo, Xiaoyun Li, stable, dev
In-Reply-To: <20260709040739.3184955-3-joshwash@google.com>
09/07/2026 06:07, Joshua Washington:
> The GQ TX datapath was set up to write the mbuf head into the sw_ring
> before writing the descriptors. This poses a problem because it's
> possible for the packet to be dropped due to lacking the FIFO space to
> do a proper TX. In such a case, the packet won't be sent, and will lead
> to leaked mbufs in the subsequent segments.
>
> There is also no real reason that the head mbuf must be set in the
> sw_ring separately from the others; the mbuf chain is not actually
> walked as part of GQ TX.
>
> Fixes: a46583cf43c8 ("net/gve: support Rx/Tx")
> Cc: stable@dpdk.org
> Signed-off-by: Joshua Washington <joshwash@google.com>
> Reviewed-by: Jasper Tran O'Leary <jtranoleary@google.com>
AI found this issue:
**Error:** Resource leak on early return from `gve_tx_burst_qpl()`.
The patch modifies the loop to store mbufs in `sw_ring` only after checking FIFO availability, but introduces a leak when `goto end_of_tx` is taken after the first segment has been processed. At line 306-307:
```c
if (!is_fifo_avail(txq, hlen)) {
gve_tx_clean(txq);
if (!is_fifo_avail(txq, hlen))
goto end_of_tx;
}
```
If the second `is_fifo_avail()` check fails and we `goto end_of_tx`, the current `tx_pkt` (which is the mbuf chain head) has not been added to `sw_ring` yet (that happens later in the modified loop at lines 346-351). The function will return without freeing `tx_pkt`, and the mbuf chain is lost.
**Suggested fix:** Free the current packet before `goto end_of_tx`:
```c
if (!is_fifo_avail(txq, hlen)) {
gve_tx_clean(txq);
if (!is_fifo_avail(txq, hlen)) {
rte_pktmbuf_free(tx_pkt);
goto end_of_tx;
}
}
```
Similarly, for the TSO path at lines 314-316, if the FIFO space check fails, `tx_pkt` should be freed before the goto.
^ permalink raw reply
* Re: [PATCH] net/nfp: fix UB in BAR size shift operations
From: Thomas Monjalon @ 2026-07-10 14:38 UTC (permalink / raw)
To: Alexey Simakov; +Cc: Chaoyong He, Alejandro Lucero, dev, stable
In-Reply-To: <20260707140112.140833-1-bigalex934@gmail.com>
07/07/2026 16:01, Alexey Simakov:
> The literal '1' is a signed 32-bit int. Shifting it by bar->bitsize
> is undefined behavior when bitsize >= 31, and sign-extends when
> bitsize == 31 (producing a wrong upper-bound check). BAR aperture
> sizes from hardware can exceed this range.
>
> Fix by using RTE_BIT64() which produces a 64-bit unsigned value,
> matching the type of the operands (uint64_t base, uint64_t offset).
>
> Fixes: c7e9729da6b5 ("net/nfp: support CPP")
> Fixes: 1fbe51cd9c3a ("net/nfp: extend usage of BAR from 8 to 24")
> Cc: stable@dpdk.org
>
> Signed-off-by: Alexey Simakov <bigalex934@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] net/iavf: check for NULL pointer before dereference
From: Bruce Richardson @ 2026-07-10 14:16 UTC (permalink / raw)
To: Loftus, Ciara; +Cc: dev@dpdk.org
In-Reply-To: <IA4PR11MB9278059BD42DC2B3CF4470DF8EFD2@IA4PR11MB9278.namprd11.prod.outlook.com>
On Fri, Jul 10, 2026 at 02:19:34PM +0100, Loftus, Ciara wrote:
> > Subject: [PATCH] net/iavf: check for NULL pointer before dereference
>
> check-git-log complains NULL --> null
>
> >
> > Static analysis with Coverity flags a possible NULL pointer dereference
> > of "hdr1" when handling IPv4 or IPv6 fragements in
> > iavf_fdir_parse_pattern. This is likely a false positive, since the
> > flow pattern protocol sequences are checked before this function is
> > called, so all paths leading to a hdr1 dereference start with an
> > ethernet protocol, which sets hdr1 to a non-NULL value.
> >
> > However, this is a rather brittle situation, so to prevent any future
> > issues if a new set of allowed protocols is added with IP but no
> > Ethernet matching patterns, add explicit NULL checks for hdr1 before
> > it's used each time.
> >
> > Coverity Id: 503768
>
> "issue" instead of "id"
>
Fixed on apply
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>
> Acked-by: Ciara Loftus <ciara.loftus@intel.com>
>
Applied to dpdk-next-net-intel.
/Bruce
^ permalink raw reply
* Re: [PATCH v3 0/6] net/intel: fix some small memory leaks
From: Bruce Richardson @ 2026-07-10 14:00 UTC (permalink / raw)
To: Loftus, Ciara; +Cc: dev@dpdk.org
In-Reply-To: <IA4PR11MB92780E0AC2DEECB77E23DD7A8EFD2@IA4PR11MB9278.namprd11.prod.outlook.com>
On Fri, Jul 10, 2026 at 11:52:08AM +0100, Loftus, Ciara wrote:
> > Subject: [PATCH v3 0/6] net/intel: fix some small memory leaks
> >
> > AI review of a patch to traffic management code identified a couple of
> > small memory leaks in that function. When fixing those, I then asked AI
> > to identify other similar leaks other Intel drivers, leading to this
> > patchset.
> >
> > V3: update based on feedback on v2:
> > - fix log message on patch 3,
> > - add additional free on close in patch 5
> > V2: expand from 2 patches to 6 to cover some more leaks
> >
> > Bruce Richardson (6):
> > net/iavf: fix local memory leaks in TM hierarchy commit
> > net/iavf: fix leak of queue to traffic class mapping data
> > net/iavf: fix memory leak on error when adding flow parser
> > net/ice: fix buffer leak in config of Tx queue TM node
> > net/iavf: fix leak of flex metadata extraction field
> > net/iavf: fix leak of IPsec crypto capabilities array
> >
> > drivers/net/intel/iavf/iavf_ethdev.c | 7 +++++
> > drivers/net/intel/iavf/iavf_generic_flow.c | 1 +
> > drivers/net/intel/iavf/iavf_ipsec_crypto.c | 2 ++
> > drivers/net/intel/iavf/iavf_tm.c | 31 +++++++++++++++-------
> > drivers/net/intel/ice/ice_tm.c | 6 +++--
> > 5 files changed, 36 insertions(+), 11 deletions(-)
> >
> > --
> > 2.53.0
>
> Series-acked-by: Ciara Loftus <ciara.loftus@intel.com>
>
Applied to dpdk-next-net-intel.
/Bruce
^ permalink raw reply
* [PATCH 2/2] net/ixgbe: fix return value of close operation
From: SidAli CHERRATI @ 2026-07-10 13:46 UTC (permalink / raw)
To: dev
Cc: Bruce Richardson, Anatoly Burakov, Vladimir Medvedkin,
SidAli CHERRATI, stable
In-Reply-To: <20260710134639.108129-1-scherrati1@gmail.com>
rte_intr_callback_unregister() returns the number of unregistered
callbacks on success. Since the close operation started returning
the ret variable instead of 0, this positive value leaks as the
return value of ixgbe_dev_close(), so applications checking
rte_eth_dev_close() != 0 treat a successful close as a failure.
Fixes: 62024eb82756 ("ethdev: change stop operation callback to return int")
Cc: stable@dpdk.org
Signed-off-by: SidAli CHERRATI <scherrati1@gmail.com>
---
drivers/net/intel/ixgbe/ixgbe_ethdev.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/intel/ixgbe/ixgbe_ethdev.c b/drivers/net/intel/ixgbe/ixgbe_ethdev.c
index c5010f623c..6b1799d7a0 100644
--- a/drivers/net/intel/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/intel/ixgbe/ixgbe_ethdev.c
@@ -3121,6 +3121,7 @@ ixgbe_dev_close(struct rte_eth_dev *dev)
ret = rte_intr_callback_unregister(intr_handle,
ixgbe_dev_interrupt_handler, dev);
if (ret >= 0 || ret == -ENOENT) {
+ ret = 0;
break;
} else if (ret != -EAGAIN) {
PMD_INIT_LOG(ERR,
--
2.34.1
^ permalink raw reply related
* [PATCH 1/2] net/i40e: fix return value of close operation
From: SidAli CHERRATI @ 2026-07-10 13:46 UTC (permalink / raw)
To: dev
Cc: Bruce Richardson, Anatoly Burakov, Vladimir Medvedkin,
SidAli CHERRATI, stable
rte_intr_callback_unregister() returns the number of unregistered
callbacks on success. Since the close operation started returning
the ret variable instead of 0, this positive value leaks as the
return value of i40e_dev_close(), so applications checking
rte_eth_dev_close() != 0 treat a successful close as a failure.
Fixes: 62024eb82756 ("ethdev: change stop operation callback to return int")
Cc: stable@dpdk.org
Signed-off-by: SidAli CHERRATI <scherrati1@gmail.com>
---
drivers/net/intel/i40e/i40e_ethdev.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/intel/i40e/i40e_ethdev.c b/drivers/net/intel/i40e/i40e_ethdev.c
index b6b2d291ee..2fe6b1e118 100644
--- a/drivers/net/intel/i40e/i40e_ethdev.c
+++ b/drivers/net/intel/i40e/i40e_ethdev.c
@@ -2764,6 +2764,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
ret = rte_intr_callback_unregister(intr_handle,
i40e_dev_interrupt_handler, dev);
if (ret >= 0 || ret == -ENOENT) {
+ ret = 0;
break;
} else if (ret != -EAGAIN) {
PMD_INIT_LOG(ERR,
--
2.34.1
^ permalink raw reply related
* RE: [PATCH] net/iavf: check for NULL pointer before dereference
From: Loftus, Ciara @ 2026-07-10 13:19 UTC (permalink / raw)
To: Richardson, Bruce, dev@dpdk.org
In-Reply-To: <20260710125940.1572978-1-bruce.richardson@intel.com>
> Subject: [PATCH] net/iavf: check for NULL pointer before dereference
check-git-log complains NULL --> null
>
> Static analysis with Coverity flags a possible NULL pointer dereference
> of "hdr1" when handling IPv4 or IPv6 fragements in
> iavf_fdir_parse_pattern. This is likely a false positive, since the
> flow pattern protocol sequences are checked before this function is
> called, so all paths leading to a hdr1 dereference start with an
> ethernet protocol, which sets hdr1 to a non-NULL value.
>
> However, this is a rather brittle situation, so to prevent any future
> issues if a new set of allowed protocols is added with IP but no
> Ethernet matching patterns, add explicit NULL checks for hdr1 before
> it's used each time.
>
> Coverity Id: 503768
"issue" instead of "id"
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Ciara Loftus <ciara.loftus@intel.com>
> ---
> drivers/net/intel/iavf/iavf_fdir.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/net/intel/iavf/iavf_fdir.c b/drivers/net/intel/iavf/iavf_fdir.c
> index 8940132dff..ea620863ca 100644
> --- a/drivers/net/intel/iavf/iavf_fdir.c
> +++ b/drivers/net/intel/iavf/iavf_fdir.c
> @@ -806,6 +806,14 @@ iavf_fdir_parse_pattern(__rte_unused struct
> iavf_adapter *ad,
> * ethertype, if the spec and mask is valid,
> * set ethertype into input set.
> */
> + /* ETH should be present via pattern pre-
> validation. */
> + if (hdr1 == NULL) {
> + rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ITEM,
> + item,
> + "Missing ETH header before
> IPv4 fragment.");
> + return -rte_errno;
> + }
> input_set |= IAVF_INSET_ETHERTYPE;
>
> VIRTCHNL_ADD_PROTO_HDR_FIELD_BIT(hdr1, ETH,
> ETHERTYPE);
> @@ -911,6 +919,14 @@ iavf_fdir_parse_pattern(__rte_unused struct
> iavf_adapter *ad,
> * ethertype, if the spec and mask is valid,
> * set ethertype into input set.
> */
> + /* ETH should be present via pattern pre-
> validation. */
> + if (hdr1 == NULL) {
> + rte_flow_error_set(error, EINVAL,
> +
> RTE_FLOW_ERROR_TYPE_ITEM,
> + item,
> + "Missing ETH header before
> IPv6 fragment.");
> + return -rte_errno;
> + }
> input_set |= IAVF_INSET_ETHERTYPE;
>
> VIRTCHNL_ADD_PROTO_HDR_FIELD_BIT(hdr1, ETH,
> ETHERTYPE);
> --
> 2.53.0
^ permalink raw reply
* [PATCH] net/iavf: check for NULL pointer before dereference
From: Bruce Richardson @ 2026-07-10 12:59 UTC (permalink / raw)
To: dev; +Cc: ciara.loftus, Bruce Richardson
Static analysis with Coverity flags a possible NULL pointer dereference
of "hdr1" when handling IPv4 or IPv6 fragements in
iavf_fdir_parse_pattern. This is likely a false positive, since the
flow pattern protocol sequences are checked before this function is
called, so all paths leading to a hdr1 dereference start with an
ethernet protocol, which sets hdr1 to a non-NULL value.
However, this is a rather brittle situation, so to prevent any future
issues if a new set of allowed protocols is added with IP but no
Ethernet matching patterns, add explicit NULL checks for hdr1 before
it's used each time.
Coverity Id: 503768
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/iavf/iavf_fdir.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/net/intel/iavf/iavf_fdir.c b/drivers/net/intel/iavf/iavf_fdir.c
index 8940132dff..ea620863ca 100644
--- a/drivers/net/intel/iavf/iavf_fdir.c
+++ b/drivers/net/intel/iavf/iavf_fdir.c
@@ -806,6 +806,14 @@ iavf_fdir_parse_pattern(__rte_unused struct iavf_adapter *ad,
* ethertype, if the spec and mask is valid,
* set ethertype into input set.
*/
+ /* ETH should be present via pattern pre-validation. */
+ if (hdr1 == NULL) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ITEM,
+ item,
+ "Missing ETH header before IPv4 fragment.");
+ return -rte_errno;
+ }
input_set |= IAVF_INSET_ETHERTYPE;
VIRTCHNL_ADD_PROTO_HDR_FIELD_BIT(hdr1, ETH,
ETHERTYPE);
@@ -911,6 +919,14 @@ iavf_fdir_parse_pattern(__rte_unused struct iavf_adapter *ad,
* ethertype, if the spec and mask is valid,
* set ethertype into input set.
*/
+ /* ETH should be present via pattern pre-validation. */
+ if (hdr1 == NULL) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ITEM,
+ item,
+ "Missing ETH header before IPv6 fragment.");
+ return -rte_errno;
+ }
input_set |= IAVF_INSET_ETHERTYPE;
VIRTCHNL_ADD_PROTO_HDR_FIELD_BIT(hdr1, ETH,
ETHERTYPE);
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] net/enic: fix potential null dereference in flow mask check
From: Thomas Monjalon @ 2026-07-10 11:29 UTC (permalink / raw)
To: Alexey Simakov
Cc: John Daley (johndale), Nelson Escobar (neescoba), dev,
stable@dpdk.org, Hyong Youb Kim (hyonkim)
In-Reply-To: <IA3PR11MB8987402CDECFACC6831119E7BFFE2@IA3PR11MB8987.namprd11.prod.outlook.com>
> > The functions enic_copy_item_ipv4_v1(), enic_copy_item_udp_v1(), and
> > enic_copy_item_tcp_v1() each initialize a local 'mask' variable from
> > item->mask and substitute it with a hardcoded mask if NULL. However,
> > the subsequent mask_exact_match() call uses item->mask directly
> > instead of the local 'mask' variable, which will dereference NULL
> > when item->mask is NULL.
> >
> > Use the local 'mask' variable (which has been validated and possibly
> > substituted) instead of item->mask.
> >
> > Fixes: aa3d2ff82198 ("net/enic: flow API for Legacy NICs")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Alexey Simakov <bigalex934@gmail.com>
> > ---
> The enic patch looks like a valid fix.
> Acked-by: Hyong Youb Kim <hyonkim@cisco.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 0/2] net/txgbe: fix coverity issues
From: Thomas Monjalon @ 2026-07-10 11:09 UTC (permalink / raw)
To: Zaiyu Wang; +Cc: dev
In-Reply-To: <20260706104723.25968-1-zaiyuwang@trustnetic.com>
06/07/2026 12:47, Zaiyu Wang:
> This series fixes several Coverity issues reported against the txgbe
> driver in the 26.07-rc2 release.
>
> Patch 1 fixes a REVERSE_INULL issue (CID 504601) in
> txgbe_dev_setup_link_alarm_handler_aml() by moving the null pointer
> check before the pointer dereference.
>
> Patch 2 fixes INTEGER_OVERFLOW (CID 504602) and BAD_SHIFT issues
> (CIDs 504598, 504613) in the e56 backplane code by validating
> lane_num at the source in txgbe_e56_cl72_training() before any
> GENMASK operations are performed.
>
> Zaiyu Wang (2):
> net/txgbe: fix null pointer check order
> net/txgbe: validate lane_num before GENMASK operations
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] net/i40e: fix FDIR flexible payload issues
From: Bruce Richardson @ 2026-07-10 10:58 UTC (permalink / raw)
To: sandeep.penigalapati; +Cc: dev, stable
In-Reply-To: <alDPoD58N7ssSRhh@bricha3-mobl1.ger.corp.intel.com>
On Fri, Jul 10, 2026 at 11:55:28AM +0100, Bruce Richardson wrote:
> On Tue, Jul 07, 2026 at 08:20:45PM -0400, sandeep.penigalapati@intel.com wrote:
> > From: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
> >
> > This fixes two issues in the i40e Flow Director flexible payload path.
> >
> > The bitmask array bound was checked after the write to
> > bitmask[nb_bitmask], allowing a one-slot out-of-bounds write when the
> > number of partial-mask words exceeds I40E_FDIR_BITMASK_NUM_WORD. Move
> > the check before the write and use '>=' so the array is never indexed
> > out of bounds.
> >
> > In addition, i40e_flow_set_fdir_flex_pit() programs the global GLQF_ORT
> > register, which is shared by all PFs on the NIC. It was called before
> > the flex mask was validated, so a rule that is later rejected still left
> > the global register modified, affecting other PFs. Validate the flex
> > mask first and only touch the hardware registers once it has passed.
> >
> > Fixes: 6ced3dd72f5f ("net/i40e: support flexible payload parsing for FDIR")
> > Cc: stable@dpdk.org
> > Signed-off-by: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
> > ---
> > drivers/net/intel/i40e/i40e_fdir.c | 23 +++++++++++++++--------
> > 1 file changed, 15 insertions(+), 8 deletions(-)
> >
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Applied to dpdk-next-net-intel.
Thanks.
^ permalink raw reply
* Re: [PATCH] net/i40e: fix FDIR flexible payload issues
From: Bruce Richardson @ 2026-07-10 10:55 UTC (permalink / raw)
To: sandeep.penigalapati; +Cc: dev, stable
In-Reply-To: <20260708002045.166595-1-sandeep.penigalapati@intel.com>
On Tue, Jul 07, 2026 at 08:20:45PM -0400, sandeep.penigalapati@intel.com wrote:
> From: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
>
> This fixes two issues in the i40e Flow Director flexible payload path.
>
> The bitmask array bound was checked after the write to
> bitmask[nb_bitmask], allowing a one-slot out-of-bounds write when the
> number of partial-mask words exceeds I40E_FDIR_BITMASK_NUM_WORD. Move
> the check before the write and use '>=' so the array is never indexed
> out of bounds.
>
> In addition, i40e_flow_set_fdir_flex_pit() programs the global GLQF_ORT
> register, which is shared by all PFs on the NIC. It was called before
> the flex mask was validated, so a rule that is later rejected still left
> the global register modified, affecting other PFs. Validate the flex
> mask first and only touch the hardware registers once it has passed.
>
> Fixes: 6ced3dd72f5f ("net/i40e: support flexible payload parsing for FDIR")
> Cc: stable@dpdk.org
> Signed-off-by: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
> ---
> drivers/net/intel/i40e/i40e_fdir.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
^ permalink raw reply
* RE: [PATCH v3 0/6] net/intel: fix some small memory leaks
From: Loftus, Ciara @ 2026-07-10 10:52 UTC (permalink / raw)
To: Richardson, Bruce, dev@dpdk.org
In-Reply-To: <20260710103403.1548864-1-bruce.richardson@intel.com>
> Subject: [PATCH v3 0/6] net/intel: fix some small memory leaks
>
> AI review of a patch to traffic management code identified a couple of
> small memory leaks in that function. When fixing those, I then asked AI
> to identify other similar leaks other Intel drivers, leading to this
> patchset.
>
> V3: update based on feedback on v2:
> - fix log message on patch 3,
> - add additional free on close in patch 5
> V2: expand from 2 patches to 6 to cover some more leaks
>
> Bruce Richardson (6):
> net/iavf: fix local memory leaks in TM hierarchy commit
> net/iavf: fix leak of queue to traffic class mapping data
> net/iavf: fix memory leak on error when adding flow parser
> net/ice: fix buffer leak in config of Tx queue TM node
> net/iavf: fix leak of flex metadata extraction field
> net/iavf: fix leak of IPsec crypto capabilities array
>
> drivers/net/intel/iavf/iavf_ethdev.c | 7 +++++
> drivers/net/intel/iavf/iavf_generic_flow.c | 1 +
> drivers/net/intel/iavf/iavf_ipsec_crypto.c | 2 ++
> drivers/net/intel/iavf/iavf_tm.c | 31 +++++++++++++++-------
> drivers/net/intel/ice/ice_tm.c | 6 +++--
> 5 files changed, 36 insertions(+), 11 deletions(-)
>
> --
> 2.53.0
Series-acked-by: Ciara Loftus <ciara.loftus@intel.com>
^ permalink raw reply
* [PATCH v3 6/6] net/iavf: fix leak of IPsec crypto capabilities array
From: Bruce Richardson @ 2026-07-10 10:34 UTC (permalink / raw)
To: dev; +Cc: ciara.loftus, Bruce Richardson, stable
In-Reply-To: <20260710103403.1548864-1-bruce.richardson@intel.com>
The security context setup allocates a crypto capabilities array and
stores it in iavf_sctx->crypto_capabilities. However, the security
context destroy path frees iavf_sctx and sctx directly, without first
freeing that array.
Fix this by freeing iavf_sctx->crypto_capabilities in
iavf_security_ctx_destroy() before freeing the security context itself.
Fixes: 6bc987ecb860 ("net/iavf: support IPsec inline crypto")
Cc: stable@dpdk.org
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/iavf/iavf_ipsec_crypto.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/intel/iavf/iavf_ipsec_crypto.c b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
index b2e08da0aa..9c8f694de5 100644
--- a/drivers/net/intel/iavf/iavf_ipsec_crypto.c
+++ b/drivers/net/intel/iavf/iavf_ipsec_crypto.c
@@ -1572,6 +1572,8 @@ iavf_security_ctx_destroy(struct iavf_adapter *adapter)
return -ENODEV;
/* free and reset security data structures */
+ rte_free(iavf_sctx->crypto_capabilities);
+ iavf_sctx->crypto_capabilities = NULL;
rte_free(iavf_sctx);
rte_free(sctx);
--
2.53.0
^ permalink raw reply related
* [PATCH v3 5/6] net/iavf: fix leak of flex metadata extraction field
From: Bruce Richardson @ 2026-07-10 10:34 UTC (permalink / raw)
To: dev; +Cc: ciara.loftus, Bruce Richardson, stable
In-Reply-To: <20260710103403.1548864-1-bruce.richardson@intel.com>
Function iavf_init_proto_xtr() allocates vf->proto_xtr as part of the
device init sequence, but no shutdown function frees this memory again.
Add appropriate free calls to fix this memory leak.
Fixes: 12b435bf8f2f ("net/iavf: support flex desc metadata extraction")
Cc: stable@dpdk.org
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
V3: add free call to close function to free memory on normal, non-error
path.
---
drivers/net/intel/iavf/iavf_ethdev.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index 1cd8c88384..d601ec3b6a 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -2757,6 +2757,8 @@ iavf_uninit_vf(struct rte_eth_dev *dev)
vf->qos_cap = NULL;
free(vf->qtc_map);
vf->qtc_map = NULL;
+ rte_free(vf->proto_xtr);
+ vf->proto_xtr = NULL;
rte_free(vf->rss_lut);
vf->rss_lut = NULL;
@@ -3255,6 +3257,9 @@ iavf_dev_close(struct rte_eth_dev *dev)
}
}
+ rte_free(vf->proto_xtr);
+ vf->proto_xtr = NULL;
+
rte_free(vf->vf_res);
vf->vsi_res = NULL;
vf->vf_res = NULL;
--
2.53.0
^ permalink raw reply related
* [PATCH v3 4/6] net/ice: fix buffer leak in config of Tx queue TM node
From: Bruce Richardson @ 2026-07-10 10:34 UTC (permalink / raw)
To: dev; +Cc: ciara.loftus, Bruce Richardson, stable
In-Reply-To: <20260710103403.1548864-1-bruce.richardson@intel.com>
Only the error leg handled free of the adminq message buffer when
configuring a Tx queue traffic management node. Fix this by freeing the
buffer unconditionally after the adminq call. Also, remove the use of
dpdk-specific memory allocation, replacing it with generic alloc and
free routines.
Fixes: 715d449a965b ("net/ice: enhance Tx scheduler hierarchy support")
Cc: stable@dpdk.org
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/ice/ice_tm.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/intel/ice/ice_tm.c b/drivers/net/intel/ice/ice_tm.c
index 94ded15fa7..2e6ef9c264 100644
--- a/drivers/net/intel/ice/ice_tm.c
+++ b/drivers/net/intel/ice/ice_tm.c
@@ -1,6 +1,8 @@
/* SPDX-License-Identifier: BSD-3-Clause
* Copyright(c) 2022 Intel Corporation
*/
+#include <stdlib.h>
+
#include <rte_ethdev.h>
#include <rte_tm_driver.h>
@@ -714,7 +716,7 @@ ice_tm_setup_txq_node(struct ice_pf *pf, struct ice_hw *hw, uint16_t qid, uint32
uint8_t txqs_moved = 0;
uint16_t buf_size = ice_struct_size(buf, txqs, 1);
- buf = ice_malloc(hw, buf_size);
+ buf = calloc(1, buf_size);
if (buf == NULL)
return -ENOMEM;
@@ -727,9 +729,9 @@ ice_tm_setup_txq_node(struct ice_pf *pf, struct ice_hw *hw, uint16_t qid, uint32
int ret = ice_aq_move_recfg_lan_txq(hw, 1, true, false, false, false, 50,
NULL, buf, buf_size, &txqs_moved, NULL);
+ free(buf);
if (ret || txqs_moved == 0) {
PMD_DRV_LOG(ERR, "move lan queue %u failed", qid);
- ice_free(hw, buf);
return ICE_ERR_PARAM;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v3 3/6] net/iavf: fix memory leak on error when adding flow parser
From: Bruce Richardson @ 2026-07-10 10:34 UTC (permalink / raw)
To: dev; +Cc: ciara.loftus, Bruce Richardson, stable
In-Reply-To: <20260710103403.1548864-1-bruce.richardson@intel.com>
In iavf_register_parser, the final "else" branch is an error leg which
returns immediately, without adding the newly allocated parser_node to a
TAILQ. Add a free call for that parser_node in the error case to avoid
leaking memory.
Fixes: ff2d0c345c3b ("net/iavf: support generic flow API")
Cc: stable@dpdk.org
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Ciara Loftus <ciara.loftus@intel.com>
---
drivers/net/intel/iavf/iavf_generic_flow.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/intel/iavf/iavf_generic_flow.c b/drivers/net/intel/iavf/iavf_generic_flow.c
index 84bb161bd1..92ca20031c 100644
--- a/drivers/net/intel/iavf/iavf_generic_flow.c
+++ b/drivers/net/intel/iavf/iavf_generic_flow.c
@@ -1911,6 +1911,7 @@ iavf_register_parser(struct iavf_flow_parser *parser,
list = &vf->dist_parser_list;
TAILQ_INSERT_HEAD(list, parser_node, node);
} else {
+ rte_free(parser_node);
return -EINVAL;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v3 2/6] net/iavf: fix leak of queue to traffic class mapping data
From: Bruce Richardson @ 2026-07-10 10:33 UTC (permalink / raw)
To: dev; +Cc: ciara.loftus, Bruce Richardson, stable
In-Reply-To: <20260710103403.1548864-1-bruce.richardson@intel.com>
On iavf TM hierarchy commit, the queue to traffic class mapping is
generated and stored in a pointer from the VF structure. However, that
data is never later freed. The data is also unnecessarily allocated from
hugepage memory.
Fix these issues by replacing rte_zmalloc with calloc, and then
appropriately freeing the memory at a) uninit of the device, b) at
failure of apply of the new settings and c) replacement of old data by
new.
Fixes: 3fd32df381f8 ("net/iavf: check Tx packet with correct UP and queue")
Cc: stable@dpdk.org
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/iavf/iavf_ethdev.c | 2 ++
drivers/net/intel/iavf/iavf_tm.c | 11 ++++++++---
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index 80e740ef29..1cd8c88384 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -2755,6 +2755,8 @@ iavf_uninit_vf(struct rte_eth_dev *dev)
rte_free(vf->qos_cap);
vf->qos_cap = NULL;
+ free(vf->qtc_map);
+ vf->qtc_map = NULL;
rte_free(vf->rss_lut);
vf->rss_lut = NULL;
diff --git a/drivers/net/intel/iavf/iavf_tm.c b/drivers/net/intel/iavf/iavf_tm.c
index 5f888d654f..faa2d4b8a0 100644
--- a/drivers/net/intel/iavf/iavf_tm.c
+++ b/drivers/net/intel/iavf/iavf_tm.c
@@ -97,6 +97,9 @@ iavf_tm_conf_uninit(struct rte_eth_dev *dev)
shaper_profile, node);
rte_free(shaper_profile);
}
+
+ free(vf->qtc_map);
+ vf->qtc_map = NULL;
}
static inline struct iavf_tm_node *
@@ -801,7 +804,8 @@ static int iavf_hierarchy_commit(struct rte_eth_dev *dev,
struct virtchnl_queues_bw_cfg *q_bw = NULL;
struct iavf_tm_node_list *queue_list = &vf->tm_conf.queue_list;
struct iavf_tm_node *tm_node;
- struct iavf_qtc_map *qtc_map;
+ struct iavf_qtc_map *qtc_map = NULL;
+ struct iavf_qtc_map *old_qtc_map = vf->qtc_map; /* to free memory if new map assigned */
uint16_t size, size_q;
int index = 0, node_committed = 0;
int i, ret_val = IAVF_SUCCESS;
@@ -889,8 +893,7 @@ static int iavf_hierarchy_commit(struct rte_eth_dev *dev,
goto fail_clear;
/* store the queue TC mapping info */
- qtc_map = rte_zmalloc("qtc_map",
- sizeof(struct iavf_qtc_map) * q_tc_mapping->num_tc, 0);
+ qtc_map = calloc(q_tc_mapping->num_tc, sizeof(struct iavf_qtc_map));
if (!qtc_map) {
ret_val = IAVF_ERR_NO_MEMORY;
goto fail_clear;
@@ -910,6 +913,7 @@ static int iavf_hierarchy_commit(struct rte_eth_dev *dev,
goto fail_clear;
vf->qtc_map = qtc_map;
+ free(old_qtc_map);
if (adapter->stopped == 1)
vf->tm_conf.committed = true;
free(q_bw);
@@ -925,5 +929,6 @@ static int iavf_hierarchy_commit(struct rte_eth_dev *dev,
err:
free(q_bw);
free(q_tc_mapping);
+ free(qtc_map);
return ret_val;
}
--
2.53.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox