RE: [v4,net-next,13/15] net: fec: add fec_alloc_rxq_buffers_pp() to allocate buffers from page pool

Wei Fang
2026-01-26T02:37:03Z

> net: fec: add AF_XDP zero-copy support
>
> This patch adds AF_XDP zero-copy support for both TX and RX on the FEC
> ethernet driver. RX buffers are allocated from the XSK pool instead of
> the page pool, and TX supports both frames from AF_XDP sockets and
> XDP_TX action.
>
> > diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> > index c19ae60b04d7..d3e54dcf6a4c 100644
>
> [ ... ]
>
> > +static int fec_setup_xsk_pool(struct net_device *ndev,
> > + struct xsk_buff_pool *pool,
> > + u16 queue)
> > +{
> > + struct fec_enet_private *fep = netdev_priv(ndev);
> > + bool is_run = netif_running(ndev);
> > + struct fec_enet_priv_rx_q *rxq;
> > + struct fec_enet_priv_tx_q *txq;
> > + bool enable = !!pool;
> > + int err;
> > +
> > + if (queue >= fep->num_rx_queues || queue >= fep->num_tx_queues)
> > + return -ERANGE;
> > +
> > + if (is_run) {
> > + napi_disable(&fep->napi);
> > + netif_tx_disable(ndev);
> > + synchronize_rcu();
> > + fec_enet_free_buffers(ndev);
> > + }
> > +
> > + rxq = fep->rx_queue[queue];
> > + txq = fep->tx_queue[queue];
> > +
> > + if (enable) {
> > + err = xsk_pool_dma_map(pool, &fep->pdev->dev, 0);
> > + if (err) {
> > + netdev_err(ndev, "Failed to map xsk pool\n");
> > + return err;
> > + }
>
> If xsk_pool_dma_map() fails here when is_run was true, NAPI has already
> been disabled, TX has been disabled, and buffers have been freed. The
> early return leaves the device in a non-functional state with no way to
> recover except manually bringing the interface down and up.
>
> Should this error path restore the device state by calling fec_restart(),
> napi_enable(), and netif_tx_start_all_queues() before returning, similar
> to what is done after fec_enet_alloc_buffers() succeeds?

I will fix this issue, thanks.

>
> > +
> > + rxq->xsk_pool = pool;
> > + txq->xsk_pool = pool;
> > + } else {
> > + xsk_pool_dma_unmap(rxq->xsk_pool, 0);
> > + rxq->xsk_pool = NULL;
> > + txq->xsk_pool = NULL;
> > + }
> > +
> > + if (is_run) {
> > + err = fec_enet_alloc_buffers(ndev);
> > + if (err) {
> > + netdev_err(ndev, "Failed to alloc buffers\n");
> > + goto err_alloc_buffers;
> > + }
> > +
> > + fec_restart(ndev);
> > + napi_enable(&fep->napi);
> > + netif_tx_start_all_queues(ndev);
> > + }
> > +
> > + return 0;
> > +
> > +err_alloc_buffers:
> > + if (enable) {
> > + xsk_pool_dma_unmap(pool, 0);
> > + rxq->xsk_pool = NULL;
> > + txq->xsk_pool = NULL;
> > + }
> > +
> > + return err;
>
> Similarly, the err_alloc_buffers path cleans up the XSK pool state but
> does not restore NAPI/TX or restart the device when is_run was true.
> Does this leave the device in the same non-functional state?
>

The current driver does not support allocating the buffers first and then
replacing the old buffers, so calling fec_enet_alloc_buffers() again in the
err_alloc_buffers path may still fail and potentially make the device
unusable. I will think about how to change this situation. Thanks.