Re: [PATCH net 4/4] net: ti: icssg: Fix XSK zero copy TX during application wakeup

[Meghana Malladi]

On 6/16/26 20:49, Jakub Kicinski wrote:
> On Tue, 16 Jun 2026 16:41:00 +0530 Meghana Malladi wrote:
> > On 6/16/26 04:51, Jakub Kicinski wrote:
> > > On Fri, 12 Jun 2026 00:27:44 +0530 Meghana Malladi wrote:
> > > > @@ -169,9 +169,6 @@ static int emac_xsk_xmit_zc(struct prueth_emac *emac,
> > > >    
> > > >    		num_tx++;
> > > >    	}
> > > > -
> > > > -	xsk_tx_release(tx_chn->xsk_pool);
> > > > -	return num_tx;
> > >
> > > Why are you deleting this?
> > >    
> >
> > xsk_sendmsg() also calls this without an rcu-lock when transmitting the
> > packets if the xmit was successful, so I was assuming it is not required
> > and I removed this.
>
> I think you still need it. Besides, seems like a separate cleanup.

Okay, I will add it back then.

> > > >    void prueth_xmit_free(struct prueth_tx_chn *tx_chn,
> > > > @@ -279,9 +276,6 @@ int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
> > > >    		num_tx++;
> > > >    	}
> > > >    
> > > > -	if (!num_tx)
> > > > -		return 0;
> > >
> > > Does something prevent us from running all this code if budget is 0?
> > > If budget is 0 we can complete normal Tx with skbs but we must
> > > not touch any AF-XDP related state.
> >
> > Can you elaborate more, I couldn't interpret your comment here
>
> netpoll may call napi from any context, including from IRQ.
> It uses budget of 0 to indicate that it's trying to only reap tx
> completions, without doing any Rx or XDP work. XDPs can't be called
> from IRQ context.

Ah I wasn't aware of this, I will add a check to ensure AF_XDP runs only 
when budget > 0 then.

> > > >    	netif_txq = netdev_get_tx_queue(ndev, chn);
> > > >    	netdev_tx_completed_queue(netif_txq, num_tx, total_bytes);
> > > >    
> > > > @@ -306,7 +300,9 @@ int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
> > > >    
> > > >    		netif_txq = netdev_get_tx_queue(ndev, chn);
> > > >    		txq_trans_cond_update(netif_txq);
> > >
> > > This looks misplaced, now we will hit it even if we didn't complete
> > > or submit any Tx.
> >
> > This code needs to be hit for packet transmission in zero copy mode.
> > emac_xsk_xmit_zc() submits the packets to the DMA in NAPI context,
> > when application wakes up the driver and triggers NAPI. Once DMA
> > transfer is done, irq gets triggered NAPI gets called which will handle
> > the tx packet completion + submit next Tx batch packets to the DMA.
> >
> > if (tx_chn->xsk_pool) -> check ensure this hits and runs for zero copy
> > only. Also above check (!num_tx) returns early during the application
> > wakeup (where budget is zero), hence it is removed.
>
> I'm commenting on txq_trans_cond_update(), you're calling it
> effectively on every NAPI call when XSK is bound, whether
> Tx is making progress or not.

Ok got it, but I wonder if it will hurt in anyway to call this even when 
there are no Tx completions.
Nonetheless, I will move this inside xsk_frames_done check.