Messages in this thread | | | From | Wei Fang <> | Subject | RE: [PATCH net] net: fec: tx processing does not call XDP APIs if budget is 0 | Date | Wed, 26 Jul 2023 03:40:54 +0000 |
| |
Hi Alexander,
> > @@ -1416,6 +1416,14 @@ fec_enet_tx_queue(struct net_device *ndev, > u16 queue_id) > > if (!skb) > > goto tx_buf_done; > > } else { > > + /* Tx processing cannot call any XDP (or page pool) APIs if > > + * the "budget" is 0. Because NAPI is called with budget of > > + * 0 (such as netpoll) indicates we may be in an IRQ context, > > + * however, we can't use the page pool from IRQ context. > > + */ > > + if (unlikely(!budget)) > > + break; > > + > > xdpf = txq->tx_buf[index].xdp; > > if (bdp->cbd_bufaddr) > > dma_unmap_single(&fep->pdev->dev, > > This statement isn't correct. There are napi enabled and non-napi > versions of these calls. This is the reason for things like the > "allow_direct" parameter in page_pool_put_full_page and the > "napi_direct" parameter in __xdp_return. > > By blocking on these cases you can end up hanging the Tx queue which is > going to break netpoll as you are going to stall the ring on XDP > packets if they are already in the queue. > > From what I can tell your driver is using xdp_return_frame in the case > of an XDP frame which doesn't make use of the NAPI optimizations in > freeing from what I can tell. The NAPI optimized version is > xdp_return_frame_rx. > So you mean it is safe to use xdp_return_frame no matter in NAPI context or non-NAPI context? And xdp_return_frame_rx_napi must be used in NAPI context? If so, I think I must have misunderstood, then this patch is not necessary.
> > @@ -1508,14 +1516,14 @@ fec_enet_tx_queue(struct net_device *ndev, > u16 queue_id) > > writel(0, txq->bd.reg_desc_active); > > } > > > > -static void fec_enet_tx(struct net_device *ndev) > > +static void fec_enet_tx(struct net_device *ndev, int budget) > > { > > struct fec_enet_private *fep = netdev_priv(ndev); > > int i; > > > > /* Make sure that AVB queues are processed first. */ > > for (i = fep->num_tx_queues - 1; i >= 0; i--) > > - fec_enet_tx_queue(ndev, i); > > + fec_enet_tx_queue(ndev, i, budget); > > } > > > > static void fec_enet_update_cbd(struct fec_enet_priv_rx_q *rxq, > > @@ -1858,7 +1866,7 @@ static int fec_enet_rx_napi(struct napi_struct > *napi, int budget) > > > > do { > > done += fec_enet_rx(ndev, budget - done); > > - fec_enet_tx(ndev); > > + fec_enet_tx(ndev, budget); > > } while ((done < budget) && fec_enet_collect_events(fep)); > > > > if (done < budget) { > > Since you are passing budget, one optimization you could make use of > would be napi_consume_skb in your Tx path instead of dev_kfree_skb_any. That's good suggestion, I think I can add this optimization in my XDP_TX support patch. Thanks!
| |