RE: [v4,net-next,12/15] net: fec: move xdp_rxq_info* APIs out of fec_enet_create_page_pool()

Wei Fang
2026-01-26T02:23:00Z

> > +static int
> > +fec_enet_alloc_rxq_buffers(struct net_device *ndev, unsigned int queue)
> > +{
> > + struct fec_enet_private *fep = netdev_priv(ndev);
> > + struct fec_enet_priv_rx_q *rxq;
> > + int err;
> > +
> > + rxq = fep->rx_queue[queue];
> > + err = fec_alloc_rxq_buffers_pp(fep, rxq);
> > + if (err)
> > + return err;
> > +
> > + err = fec_xdp_rxq_info_reg(fep, rxq);
> > + if (err) {
> > + fec_free_rxq_buffers(rxq);
> > +
> > + return err;
> > + }
> > +
> > + return 0;
> > +}
>
> Could there be a memory leak here on systems with multiple RX queues? The
> old code had:
>
> err_alloc:
> fec_enet_free_buffers(ndev);
> return -ENOMEM;
>
> which freed all queues (both RX and TX). The new code calls
> fec_free_rxq_buffers(rxq), which only frees the current queue.
>
> If fec_enet_alloc_buffers() calls this function in a loop for multiple
> queues and queue N fails after queues 0 through N-1 succeeded, only queue
> N gets cleaned up. The caller fec_enet_alloc_buffers() just returns
> -ENOMEM without freeing the previously allocated queues:
>
> for (i = 0; i < fep->num_rx_queues; i++)
> if (fec_enet_alloc_rxq_buffers(ndev, i))
> return -ENOMEM;
>
> And fec_enet_open() jumps to err_enet_alloc which does not call
> fec_enet_free_buffers(), leaving the page pools and pages from the
> successfully allocated queues leaked.

Yes, this is real a potential bug, I will fix it.