On Tue, 25 Jul 2023 10:30:24 -0700 Alexander H Duyck wrote: > > -In other words, it is recommended to ignore the budget argument when > > -performing TX buffer reclamation to ensure that the reclamation is not > > -arbitrarily bounded; however, it is required to honor the budget argument > > -for RX processing. > > +In other words for Rx processing the ``budget`` argument limits how many > > +packets driver can process in a single poll. Rx specific APIs like page > > +pool or XDP cannot be used at all when ``budget`` is 0. > > +skb Tx processing should happen regardless of the ``budget``, but if > > +the argument is 0 driver cannot call any XDP (or page pool) APIs. > > This isn't accurate, and I would say it is somewhat dangerous advice. > The Tx still needs to be processed regardless of if it is processing > page_pool pages or XDP pages. I agree the Rx should not be processed, > but the Tx must be processed using mechanisms that do NOT make use of > NAPI optimizations when budget is 0. > > So specifically, xdp_return_frame is safe in non-NAPI Tx cleanup. The > xdp_return_frame_rx_napi is not. > > Likewise there is napi_consume_skb which will use either a NAPI or non- > NAPI version of things depending on if budget is 0 or not. > > For the page_pool calls there is the "allow_direct" argument that is > meant to decide between recycling in directly into the page_pool cache > or not. It should only be used in the Rx handler itself when budget is > non-zero. > > I realise this was written up in response to a patch on the Mellanox > driver. Based on the patch in question it looks like they were calling > page_pool_recycle_direct outside of NAPI context. There is an explicit > warning above that function about NOT calling it outside of NAPI > context. Unless I'm missing something budget=0 can be called from hard IRQ context. And page pool takes _bh() locks. So unless we "teach it" not to recycle _anything_ in hard IRQ context, it is not safe to call. > > .. warning:: > > > > - The ``budget`` argument may be 0 if core tries to only process Tx completions > > - and no Rx packets. > > + The ``budget`` argument may be 0 if core tries to only process > > + skb Tx completions and no Rx or XDP packets. > > > > The poll method returns the amount of work done. If the driver still > > has outstanding work to do (e.g. ``budget`` was exhausted) > > We cannot make this distinction if both XDP and skb are processed in > the same Tx queue. Otherwise you will cause the Tx to stall and break > netpoll. If the ring is XDP only then yes, it can be skipped like what > they did in the Mellanox driver, but if it is mixed then the XDP side > of things needs to use the "safe" versions of the calls. IDK, a rare delay in sending of a netpoll message is not a major concern.
