Re: [PATCH net] igc: return XSK frame copies on XDP_TX failure

[Paul Moses]

On Tuesday, October 27th, 2026 at 3:19 AM, Maciej Fijalkowski <maciej.fijalkowski@xxxxxxxxx> wrote:

> On Mon, Oct 26, 2026 at 10:18:03PM +0000, Paul Moses wrote:
> > On Monday, October 26th, 2026 at 3:07 PM, Maciej Fijalkowski <maciej.fijalkowski@xxxxxxxxx> wrote:
> >
> > > On Fri, Oct 23, 2026 at 02:13:42PM +0000, Paul Moses wrote:
> > > > Converting an XSK buffer to an XDP frame allocates a page and returns
> > > > the original buffer to its pool. If the TX ring is full or DMA mapping
> > > > fails, igc_xdp_xmit_back() returns an error without freeing the copy.
> > > > The RX cleaner cannot release it, so the page leaks.
> > > >
> > > > Return the copied frame on TX failure. Save the RX memory type before
> > > > conversion so native frames remain RX-owned on failure.
> > > >
> > > > Fixes: fc9df2a0b520 ("igc: Enable RX via AF_XDP zero-copy")
> > > > Cc: stable@xxxxxxxxxxxxxxx
> > > > Signed-off-by: Paul Moses <p@xxxxxxx>
> > > > ---
> > > > Tested on Intel I225-V with baseline and patched kernels. Physical
> > > > pause-frame backpressure caused 46 leaked copies on the baseline.
> > > > With the fix, all 72 rejected copies were returned once and freed,
> > > > and all 192 copies were accounted for.
> > > >
> > > >  drivers/net/ethernet/intel/igc/igc_main.c | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> > > > index 1fb5f3cbe93c..39263fd04aa9 100644
> > > > --- a/drivers/net/ethernet/intel/igc/igc_main.c
> > > > +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> > > > @@ -2487,6 +2487,7 @@ struct igc_ring *igc_get_tx_ring(struct igc_adapter *adapter, int cpu)
> > > >
> > > >  static int igc_xdp_xmit_back(struct igc_adapter *adapter, struct xdp_buff *xdp)
> > > >  {
> > > > +	enum xdp_mem_type mem_type = xdp->rxq->mem.type;
> > >
> > > small nit is we require a concept called RCT (reverse christmas tree)
> > > which makes you to order variable declarations from longest line to
> > > shortest.
> > >
> > > >  	struct xdp_frame *xdpf = xdp_convert_buff_to_frame(xdp);
> > > >  	int cpu = smp_processor_id();
> > > >  	struct netdev_queue *nq;
> > > > @@ -2504,6 +2505,10 @@ static int igc_xdp_xmit_back(struct igc_adapter *adapter, struct xdp_buff *xdp)
> > > >  	txq_trans_cond_update(nq);
> > > >  	res = igc_xdp_init_tx_descriptor(ring, xdpf);
> > > >  	__netif_tx_unlock(nq);
> > > > +
> > > > +	if (res && mem_type == MEM_TYPE_XSK_BUFF_POOL)
> > > > +		xdp_return_frame(xdpf);
> > >
> > > Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@xxxxxxxxx>
> > >
> > > > +
> > > >  	return res;
> > > >  }
> > > >
> > > > --
> > > > 2.56.GIT
> > > >
> > > >
> > > >
> > >
> >
> > Never know what to do in these cases, order matters here, so
> > do I touch more lines to move the assignment down below?
> 
> 	struct xdp_frame *xdpf = xdp_convert_buff_to_frame(xdp);
> +	enum xdp_mem_type mem_type = xdp->rxq->mem.type;
> 	int cpu = smp_processor_id();
> 	struct netdev_queue *nq;
> 
> this would satisfy the RCT rule.
> 

Ok I'll send soon.