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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 24.04.23 14:03, Maciej Fijalkowski wrote:
On Fri, Apr 21, 2023 at 09:02:56PM +0200, Gerhard Engleder wrote:
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@xxxxxxxxxxxxxxxxxxxxx>
---
   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().

i was only referring to tsnep_xdp_xmit_zc() being void on return. Thing is
that there is currently no way you would tell to napi there is more work
to be done. you should do so if desc_available == batch. That would mean
there might be still descriptors on XSK Tx ring ready to be consumed. NAPI
budget is out of the picture here.

Now I got it. Pending zero-copy work shall be signaled to napi directly.
In current implementation it would be processed if TX interrupt signals
completed descriptors. By signaling napi directly, the TX interrupt
could be prevented. In my opinion both options are possible, but the
performance may differ. I will check if telling napi that there is more
work to be done would improve performance.



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux