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

Reply via email to