Re: [PATCH net-next v3 6/6] tsnep: Add XDP socket zero-copy TX support

[Gerhard Engleder <gerhard@engleder-embedded.com>]

On 20.04.23 22:17, Maciej Fijalkowski wrote:
> On Tue, Apr 18, 2023 at 09:04:59PM +0200, Gerhard Engleder wrote:
>> Send and complete XSK pool frames within TX NAPI context. NAPI context
>> is triggered by ndo_xsk_wakeup.
>>
>> Test results with A53 1.2GHz:
>>
>> xdpsock txonly copy mode:
>>                     pps            pkts           1.00
>> tx                 284,409        11,398,144
>> Two CPUs with 100% and 10% utilization.
>>
>> xdpsock txonly zero-copy mode:
>>                     pps            pkts           1.00
>> tx                 511,929        5,890,368
>> Two CPUs with 100% and 1% utilization.
> 
> Hmm, I think l2fwd ZC numbers should be included here not in the previous
> patch?

Will be done.

>>
>> Packet rate increases and CPU utilization is reduced.
>>
>> Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
>> ---
>>   drivers/net/ethernet/engleder/tsnep.h      |   2 +
>>   drivers/net/ethernet/engleder/tsnep_main.c | 127 +++++++++++++++++++--
>>   2 files changed, 119 insertions(+), 10 deletions(-)
>>

(...)

>>   static bool tsnep_tx_poll(struct tsnep_tx *tx, int napi_budget)
>>   {
>>   	struct tsnep_tx_entry *entry;
>>   	struct netdev_queue *nq;
>> +	int xsk_frames = 0;
>>   	int budget = 128;
>>   	int length;
>>   	int count;
>> @@ -676,7 +771,7 @@ static bool tsnep_tx_poll(struct tsnep_tx *tx, int napi_budget)
>>   		if ((entry->type & TSNEP_TX_TYPE_SKB) &&
>>   		    skb_shinfo(entry->skb)->nr_frags > 0)
>>   			count += skb_shinfo(entry->skb)->nr_frags;
>> -		else if (!(entry->type & TSNEP_TX_TYPE_SKB) &&
>> +		else if ((entry->type & TSNEP_TX_TYPE_XDP) &&
>>   			 xdp_frame_has_frags(entry->xdpf))
>>   			count += xdp_get_shared_info_from_frame(entry->xdpf)->nr_frags;
>>   
>> @@ -705,9 +800,11 @@ static bool tsnep_tx_poll(struct tsnep_tx *tx, int napi_budget)
>>   
>>   		if (entry->type & TSNEP_TX_TYPE_SKB)
>>   			napi_consume_skb(entry->skb, napi_budget);
>> -		else
>> +		else if (entry->type & TSNEP_TX_TYPE_XDP)
>>   			xdp_return_frame_rx_napi(entry->xdpf);
>> -		/* xdpf is union with skb */
>> +		else
>> +			xsk_frames++;
>> +		/* xdpf and zc are union with skb */
>>   		entry->skb = NULL;
>>   
>>   		tx->read = (tx->read + count) & TSNEP_RING_MASK;
>> @@ -718,6 +815,14 @@ static bool tsnep_tx_poll(struct tsnep_tx *tx, int napi_budget)
>>   		budget--;
>>   	} while (likely(budget));
>>   
>> +	if (tx->xsk_pool) {
>> +		if (xsk_frames)
>> +			xsk_tx_completed(tx->xsk_pool, xsk_frames);
>> +		if (xsk_uses_need_wakeup(tx->xsk_pool))
>> +			xsk_set_tx_need_wakeup(tx->xsk_pool);
>> +		tsnep_xdp_xmit_zc(tx);
> 
> would be good to signal to NAPI if we are done with the work or is there a
> need to be rescheduled (when you didn't manage to consume all of the descs
> from XSK Tx ring).

In my opinion this is already done. If some budget is left, then we are
done and tsnep_tx_poll() returns true to signal work is complete. If
buget gets zero, then tsnep_tx_poll() returns false to signal work is
not complete. This return value is considered for the NAPI signaling
by tsnep_poll().