[PATCH net 0/2] tsnep: XDP fixes

[PATCH net 0/2] tsnep: XDP fixes

[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

[PATCH net 1/2] tsnep: Remove FCS for XDP data path

[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

[PATCH net 2/2] tsnep: Fix XDP_RING_NEED_WAKEUP for empty fill ring

[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

Re: [PATCH net 2/2] tsnep: Fix XDP_RING_NEED_WAKEUP for empty fill ring

[Paolo Abeni]

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

Re: [PATCH net 0/2] tsnep: XDP fixes

[patchwork-bot+netdevbpf]

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

Re: [PATCH net 2/2] tsnep: Fix XDP_RING_NEED_WAKEUP for empty fill ring

[Gerhard Engleder]

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!