[PATCH net 0/2] tsnep: XDP fixes

netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH net 0/2] tsnep: XDP fixes
@ 2024-01-23 20:09 Gerhard Engleder
  2024-01-23 20:09 ` [PATCH net 1/2] tsnep: Remove FCS for XDP data path Gerhard Engleder
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Gerhard Engleder @ 2024-01-23 20:09 UTC (permalink / raw)
  To: netdev, bpf
  Cc: davem, kuba, edumazet, pabeni, bjorn, magnus.karlsson,
	maciej.fijalkowski, jonathan.lemon, Gerhard Engleder

Found two driver specific problems during XDP and XSK testing.

Gerhard Engleder (2):
  tsnep: Remove FCS for XDP data path
  tsnep: Fix XDP_RING_NEED_WAKEUP for empty fill ring

 drivers/net/ethernet/engleder/tsnep_main.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH net 1/2] tsnep: Remove FCS for XDP data path
  2024-01-23 20:09 [PATCH net 0/2] tsnep: XDP fixes Gerhard Engleder
@ 2024-01-23 20:09 ` Gerhard Engleder
  2024-01-23 20:09 ` [PATCH net 2/2] tsnep: Fix XDP_RING_NEED_WAKEUP for empty fill ring Gerhard Engleder
  2024-01-25 11:10 ` [PATCH net 0/2] tsnep: XDP fixes patchwork-bot+netdevbpf
  2 siblings, 0 replies; 6+ messages in thread
From: Gerhard Engleder @ 2024-01-23 20:09 UTC (permalink / raw)
  To: netdev, bpf
  Cc: davem, kuba, edumazet, pabeni, bjorn, magnus.karlsson,
	maciej.fijalkowski, jonathan.lemon, Gerhard Engleder

The RX data buffer includes the FCS. The FCS is already stripped for the
normal data path. But for the XDP data path the FCS is included and
acts like additional/useless data.

Remove the FCS from the RX data buffer also for XDP.

Fixes: 65b28c810035 ("tsnep: Add XDP RX support")
Fixes: 3fc2333933fd ("tsnep: Add XDP socket zero-copy RX support")
Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
---
 drivers/net/ethernet/engleder/tsnep_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/engleder/tsnep_main.c b/drivers/net/ethernet/engleder/tsnep_main.c
index 0462834cf6e1..d380a407e175 100644
--- a/drivers/net/ethernet/engleder/tsnep_main.c
+++ b/drivers/net/ethernet/engleder/tsnep_main.c
@@ -1487,7 +1487,7 @@ static int tsnep_rx_poll(struct tsnep_rx *rx, struct napi_struct *napi,
 
 			xdp_prepare_buff(&xdp, page_address(entry->page),
 					 XDP_PACKET_HEADROOM + TSNEP_RX_INLINE_METADATA_SIZE,
-					 length, false);
+					 length - ETH_FCS_LEN, false);
 
 			consume = tsnep_xdp_run_prog(rx, prog, &xdp,
 						     &xdp_status, tx_nq, tx);
@@ -1570,7 +1570,7 @@ static int tsnep_rx_poll_zc(struct tsnep_rx *rx, struct napi_struct *napi,
 		prefetch(entry->xdp->data);
 		length = __le32_to_cpu(entry->desc_wb->properties) &
 			 TSNEP_DESC_LENGTH_MASK;
-		xsk_buff_set_size(entry->xdp, length);
+		xsk_buff_set_size(entry->xdp, length - ETH_FCS_LEN);
 		xsk_buff_dma_sync_for_cpu(entry->xdp, rx->xsk_pool);
 
 		/* RX metadata with timestamps is in front of actual data,
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH net 2/2] tsnep: Fix XDP_RING_NEED_WAKEUP for empty fill ring
  2024-01-23 20:09 [PATCH net 0/2] tsnep: XDP fixes Gerhard Engleder
  2024-01-23 20:09 ` [PATCH net 1/2] tsnep: Remove FCS for XDP data path Gerhard Engleder
@ 2024-01-23 20:09 ` Gerhard Engleder
  2024-01-25 11:06   ` Paolo Abeni
  2024-01-25 11:10 ` [PATCH net 0/2] tsnep: XDP fixes patchwork-bot+netdevbpf
  2 siblings, 1 reply; 6+ messages in thread
From: Gerhard Engleder @ 2024-01-23 20:09 UTC (permalink / raw)
  To: netdev, bpf
  Cc: davem, kuba, edumazet, pabeni, bjorn, magnus.karlsson,
	maciej.fijalkowski, jonathan.lemon, Gerhard Engleder

The fill ring of the XDP socket may contain not enough buffers to
completey fill the RX queue during socket creation. In this case the
flag XDP_RING_NEED_WAKEUP is not set as this flag is only set if the RX
queue is not completely filled during polling.

Set XDP_RING_NEED_WAKEUP flag also if RX queue is not completely filled
during XDP socket creation.

Fixes: 3fc2333933fd ("tsnep: Add XDP socket zero-copy RX support")
Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
---
 drivers/net/ethernet/engleder/tsnep_main.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/net/ethernet/engleder/tsnep_main.c b/drivers/net/ethernet/engleder/tsnep_main.c
index d380a407e175..ae0b8b37b9bf 100644
--- a/drivers/net/ethernet/engleder/tsnep_main.c
+++ b/drivers/net/ethernet/engleder/tsnep_main.c
@@ -1764,6 +1764,19 @@ static void tsnep_rx_reopen_xsk(struct tsnep_rx *rx)
 			allocated--;
 		}
 	}
+
+	/* set need wakeup flag immediately if ring is not filled completely,
+	 * first polling would be too late as need wakeup signalisation would
+	 * be delayed for an indefinite time
+	 */
+	if (xsk_uses_need_wakeup(rx->xsk_pool)) {
+		int desc_available = tsnep_rx_desc_available(rx);
+
+		if (desc_available)
+			xsk_set_rx_need_wakeup(rx->xsk_pool);
+		else
+			xsk_clear_rx_need_wakeup(rx->xsk_pool);
+	}
 }
 
 static bool tsnep_pending(struct tsnep_queue *queue)
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net 2/2] tsnep: Fix XDP_RING_NEED_WAKEUP for empty fill ring
  2024-01-23 20:09 ` [PATCH net 2/2] tsnep: Fix XDP_RING_NEED_WAKEUP for empty fill ring Gerhard Engleder
@ 2024-01-25 11:06   ` Paolo Abeni
  2024-01-25 21:50     ` Gerhard Engleder
  0 siblings, 1 reply; 6+ messages in thread
From: Paolo Abeni @ 2024-01-25 11:06 UTC (permalink / raw)
  To: Gerhard Engleder, netdev, bpf
  Cc: davem, kuba, edumazet, bjorn, magnus.karlsson, maciej.fijalkowski,
	jonathan.lemon

On Tue, 2024-01-23 at 21:09 +0100, Gerhard Engleder wrote:
> The fill ring of the XDP socket may contain not enough buffers to
> completey fill the RX queue during socket creation. In this case the
> flag XDP_RING_NEED_WAKEUP is not set as this flag is only set if the RX
> queue is not completely filled during polling.
> 
> Set XDP_RING_NEED_WAKEUP flag also if RX queue is not completely filled
> during XDP socket creation.
> 
> Fixes: 3fc2333933fd ("tsnep: Add XDP socket zero-copy RX support")
> Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
> ---
>  drivers/net/ethernet/engleder/tsnep_main.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/net/ethernet/engleder/tsnep_main.c b/drivers/net/ethernet/engleder/tsnep_main.c
> index d380a407e175..ae0b8b37b9bf 100644
> --- a/drivers/net/ethernet/engleder/tsnep_main.c
> +++ b/drivers/net/ethernet/engleder/tsnep_main.c
> @@ -1764,6 +1764,19 @@ static void tsnep_rx_reopen_xsk(struct tsnep_rx *rx)
>  			allocated--;
>  		}
>  	}
> +
> +	/* set need wakeup flag immediately if ring is not filled completely,
> +	 * first polling would be too late as need wakeup signalisation would
> +	 * be delayed for an indefinite time
> +	 */
> +	if (xsk_uses_need_wakeup(rx->xsk_pool)) {
> +		int desc_available = tsnep_rx_desc_available(rx);
> +
> +		if (desc_available)
> +			xsk_set_rx_need_wakeup(rx->xsk_pool);
> +		else
> +			xsk_clear_rx_need_wakeup(rx->xsk_pool);
> +	}
>  }
>  
>  static bool tsnep_pending(struct tsnep_queue *queue)

The patch LGTM, but there is a very similar chunk of code in
tsnep_rx_poll_zc(). You should consider a net-next follow-up
consolidating the code in a common helper.

Cheers,

Paolo


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net 2/2] tsnep: Fix XDP_RING_NEED_WAKEUP for empty fill ring
  2024-01-25 11:06   ` Paolo Abeni
@ 2024-01-25 21:50     ` Gerhard Engleder
  0 siblings, 0 replies; 6+ messages in thread
From: Gerhard Engleder @ 2024-01-25 21:50 UTC (permalink / raw)
  To: Paolo Abeni, netdev, bpf
  Cc: davem, kuba, edumazet, bjorn, magnus.karlsson, maciej.fijalkowski,
	jonathan.lemon

On 25.01.24 12:06, Paolo Abeni wrote:
> On Tue, 2024-01-23 at 21:09 +0100, Gerhard Engleder wrote:
>> +	if (xsk_uses_need_wakeup(rx->xsk_pool)) {
>> +		int desc_available = tsnep_rx_desc_available(rx);
>> +
>> +		if (desc_available)
>> +			xsk_set_rx_need_wakeup(rx->xsk_pool);
>> +		else
>> +			xsk_clear_rx_need_wakeup(rx->xsk_pool);
>> +	}
>>   }
>>   
>>   static bool tsnep_pending(struct tsnep_queue *queue)
> 
> The patch LGTM, but there is a very similar chunk of code in
> tsnep_rx_poll_zc(). You should consider a net-next follow-up
> consolidating the code in a common helper.

I will do. Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net 0/2] tsnep: XDP fixes
  2024-01-23 20:09 [PATCH net 0/2] tsnep: XDP fixes Gerhard Engleder
  2024-01-23 20:09 ` [PATCH net 1/2] tsnep: Remove FCS for XDP data path Gerhard Engleder
  2024-01-23 20:09 ` [PATCH net 2/2] tsnep: Fix XDP_RING_NEED_WAKEUP for empty fill ring Gerhard Engleder
@ 2024-01-25 11:10 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-01-25 11:10 UTC (permalink / raw)
  To: Gerhard Engleder
  Cc: netdev, bpf, davem, kuba, edumazet, pabeni, bjorn,
	magnus.karlsson, maciej.fijalkowski, jonathan.lemon

Hello:

This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Tue, 23 Jan 2024 21:09:16 +0100 you wrote:
> Found two driver specific problems during XDP and XSK testing.
> 
> Gerhard Engleder (2):
>   tsnep: Remove FCS for XDP data path
>   tsnep: Fix XDP_RING_NEED_WAKEUP for empty fill ring
> 
>  drivers/net/ethernet/engleder/tsnep_main.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)

Here is the summary with links:
  - [net,1/2] tsnep: Remove FCS for XDP data path
    https://git.kernel.org/netdev/net/c/50bad6f797d4
  - [net,2/2] tsnep: Fix XDP_RING_NEED_WAKEUP for empty fill ring
    https://git.kernel.org/netdev/net/c/9a91c05f4bd6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-01-25 22:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-23 20:09 [PATCH net 0/2] tsnep: XDP fixes Gerhard Engleder
2024-01-23 20:09 ` [PATCH net 1/2] tsnep: Remove FCS for XDP data path Gerhard Engleder
2024-01-23 20:09 ` [PATCH net 2/2] tsnep: Fix XDP_RING_NEED_WAKEUP for empty fill ring Gerhard Engleder
2024-01-25 11:06   ` Paolo Abeni
2024-01-25 21:50     ` Gerhard Engleder
2024-01-25 11:10 ` [PATCH net 0/2] tsnep: XDP fixes patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).