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

[Meghana Malladi <m-malladi@ti.com>]

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.