> From: David Marchand [mailto:[email protected]] > Sent: Friday, 27 February 2026 10.24 > > On Thu, 26 Feb 2026 at 20:22, Stephen Hemminger > <[email protected]> wrote: > > > > On Thu, 26 Feb 2026 15:33:35 +0100 > > Maxime Leroy <[email protected]> wrote: > > > > > The sw_td path in dpaa2_dev_tx() reads from the wrong position in > the > > > bufs array. When the goto fires, bufs has already been advanced > past > > > the num_tx successfully sent packets. The first loop then reads > num_tx > > > more entries starting from bufs, going past the end of the input > array. > > > Additionally, the buf_to_free segments for already-enqueued packets > > > are never freed, leaking memory. > > > > > > Replace the buggy sw_td code with the same pattern used in > > > dpaa2_dev_tx_ordered(): free buf_to_free segments first, then use > > > rte_pktmbuf_free_bulk() to drop remaining unsent packets. > > > > > > Not tested, found by code review. > > > > > > Cc: [email protected] > > > Fixes: c3ffe74d85be ("net/dpaa2: support software taildrop") > > > > > > Reported-by: Stephen Hemminger <[email protected]> > > > Signed-off-by: Maxime Leroy <[email protected]> > > > --- > > > drivers/net/dpaa2/dpaa2_rxtx.c | 18 ++++++------------ > > > 1 file changed, 6 insertions(+), 12 deletions(-) > > > > > > diff --git a/drivers/net/dpaa2/dpaa2_rxtx.c > b/drivers/net/dpaa2/dpaa2_rxtx.c > > > index 689e5e7ee7..8275ba9780 100644 > > > --- a/drivers/net/dpaa2/dpaa2_rxtx.c > > > +++ b/drivers/net/dpaa2/dpaa2_rxtx.c > > > @@ -1517,21 +1517,15 @@ dpaa2_dev_tx(void *queue, struct rte_mbuf > **bufs, uint16_t nb_pkts) > > > > > > return num_tx; > > > sw_td: > > > - loop = 0; > > > - while (loop < num_tx) { > > > - if (unlikely(RTE_MBUF_HAS_EXTBUF(*bufs))) > > > - rte_pktmbuf_free(*bufs); > > > - bufs++; > > > - loop++; > > > + for (loop = 0; loop < free_count; loop++) { > > > + if (buf_to_free[loop].pkt_id < num_tx) > > > + rte_pktmbuf_free_seg(buf_to_free[loop].seg); > > > } > > > > > > /* free the pending buffers */ > > > - while (nb_pkts) { > > > - rte_pktmbuf_free(*bufs); > > > - bufs++; > > > - nb_pkts--; > > > - num_tx++; > > > - } > > > + rte_pktmbuf_free_bulk(bufs, nb_pkts); > > > > It is unsafe to use free_bulk on transmit path because packets in > > burst might come from different pools. An example would be a router > > forwarding from two incoming NIC's to one outgoing NIC. > > Is it? > > IIUC, this helper was added specifically to handle mbufs from multiple > mempools. > > static void > __rte_pktmbuf_free_seg_via_array(struct rte_mbuf *m, > struct rte_mbuf ** const pending, unsigned int * const > nb_pending, > const unsigned int pending_sz) > { > m = rte_pktmbuf_prefree_seg(m); > if (likely(m != NULL)) { > if (*nb_pending == pending_sz || > (*nb_pending > 0 && m->pool != pending[0]->pool)) > { > rte_mempool_put_bulk(pending[0]->pool, > (void **)pending, > *nb_pending); > *nb_pending = 0; > } > > pending[(*nb_pending)++] = m; > } > } > > void rte_pktmbuf_free_bulk(struct rte_mbuf **mbufs, unsigned int > count) > for (idx = 0; idx < count; idx++) { > m = mbufs[idx]; > do { > m_next = m->next; > __rte_pktmbuf_free_seg_via_array(m, > pending, &nb_pending, > RTE_PKTMBUF_FREE_PENDING_SZ); > m = m_next; > } while (m != NULL); > } > > > -- > David Marchand
I guess rte_pktmbuf_free_bulk() was mistaken for rte_mbuf_raw_free_bulk(). The first is perfectly good to free from a mix of multiple mempools, the latter takes the mempool as a parameter.
