Re: [PATCH net] ice: xsk: clear status_error0 for each allocated desc

Re: [PATCH net] ice: xsk: clear status_error0 for each allocated desc

[Alexander Lobakin]

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Date: Fri, 26 Nov 2021 15:38:48 +0100

> Fix a bug in which the receiving of packets can stop in the zero-copy
> driver. Ice HW ignores 3 lower bits from QRX_TAIL register, which means
> that tail is bumped only on intervals of 8. Currently with XSK RX
> batching in place, ice_alloc_rx_bufs_zc() clears the status_error0 only
> of the last descriptor that has been allocated/taken from the XSK buffer
> pool. status_error0 includes DD bit that is looked upon by the
> ice_clean_rx_irq_zc() to tell if a descriptor can be processed.
> 
> The bug can be triggered when driver updates the ntu but not the
> QRX_TAIL, so HW wouldn't have a chance to write to the ready
> descriptors. Later on driver moves the ntc to the mentioned set of
> descriptors and interprets them as a ready to be processed, since
> corresponding DD bits were not cleared nor any writeback has happened
> that would clear it. This can then lead to ntc == ntu case which means
> that ring is empty and no further packet processing.
> 
> Fix the XSK traffic hang that can be observed when l2fwd scenario from
> xdpsock is used by making sure that status_error0 is cleared for each
> descriptor that is fed to HW and therefore we are sure that driver will
> not processed non-valid DD bits. This will also prevent the driver from
> processing the descriptors that were allocated in favor of the
> previously processed ones, but writeback didn't happen yet.
> 
> Fixes: db804cfc21e9 ("ice: Use the xsk batched rx allocation interface")
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Reviewed-by: Alexander Lobakin <alexandr.lobakin@intel.com>

Thanks!

> ---
>  drivers/net/ethernet/intel/ice/ice_xsk.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
> index ff55cb415b11..bb9a80847298 100644
> --- a/drivers/net/ethernet/intel/ice/ice_xsk.c
> +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
> @@ -383,6 +383,7 @@ bool ice_alloc_rx_bufs_zc(struct ice_rx_ring *rx_ring, u16 count)
>  	while (i--) {
>  		dma = xsk_buff_xdp_get_dma(*xdp);
>  		rx_desc->read.pkt_addr = cpu_to_le64(dma);
> +		rx_desc->wb.status_error0 = 0;
>  
>  		rx_desc++;
>  		xdp++;
> -- 
> 2.31.1

Al

[PATCH net] ice: xsk: clear status_error0 for each allocated desc

[Maciej Fijalkowski]

Fix a bug in which the receiving of packets can stop in the zero-copy
driver. Ice HW ignores 3 lower bits from QRX_TAIL register, which means
that tail is bumped only on intervals of 8. Currently with XSK RX
batching in place, ice_alloc_rx_bufs_zc() clears the status_error0 only
of the last descriptor that has been allocated/taken from the XSK buffer
pool. status_error0 includes DD bit that is looked upon by the
ice_clean_rx_irq_zc() to tell if a descriptor can be processed.

The bug can be triggered when driver updates the ntu but not the
QRX_TAIL, so HW wouldn't have a chance to write to the ready
descriptors. Later on driver moves the ntc to the mentioned set of
descriptors and interprets them as a ready to be processed, since
corresponding DD bits were not cleared nor any writeback has happened
that would clear it. This can then lead to ntc == ntu case which means
that ring is empty and no further packet processing.

Fix the XSK traffic hang that can be observed when l2fwd scenario from
xdpsock is used by making sure that status_error0 is cleared for each
descriptor that is fed to HW and therefore we are sure that driver will
not processed non-valid DD bits. This will also prevent the driver from
processing the descriptors that were allocated in favor of the
previously processed ones, but writeback didn't happen yet.

Fixes: db804cfc21e9 ("ice: Use the xsk batched rx allocation interface")
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_xsk.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
index ff55cb415b11..bb9a80847298 100644
--- a/drivers/net/ethernet/intel/ice/ice_xsk.c
+++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
@@ -383,6 +383,7 @@ bool ice_alloc_rx_bufs_zc(struct ice_rx_ring *rx_ring, u16 count)
 	while (i--) {
 		dma = xsk_buff_xdp_get_dma(*xdp);
 		rx_desc->read.pkt_addr = cpu_to_le64(dma);
+		rx_desc->wb.status_error0 = 0;
 
 		rx_desc++;
 		xdp++;
-- 
2.31.1