| Message ID | 20230209160130.1779890-1-larysa.zaremba@intel.com |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [net] ice: xsk: Fix cleaning of XDP_TX frames | expand |
On Thu, 2023-02-09 at 17:01 +0100, Larysa Zaremba wrote: > Incrementation of xsk_frames inside the for-loop produces > infinite loop, if we have both normal AF_XDP-TX and XDP_TXed > buffers to complete. > > Split xsk_frames into 2 variables (xsk_frames and completed_frames) > to eliminate this bug. > > Fixes: 29322791bc8b ("ice: xsk: change batched Tx descriptor cleaning") > Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com> > --- > To Tony: this is urgent and should go directly via net. It's tested and acked. > --- > drivers/net/ethernet/intel/ice/ice_xsk.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c > index 7105de6fb344..374b7f10b549 100644 > --- a/drivers/net/ethernet/intel/ice/ice_xsk.c > +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c > @@ -800,6 +800,7 @@ static void ice_clean_xdp_irq_zc(struct ice_tx_ring *xdp_ring) > struct ice_tx_desc *tx_desc; > u16 cnt = xdp_ring->count; > struct ice_tx_buf *tx_buf; > + u16 completed_frames = 0; > u16 xsk_frames = 0; > u16 last_rs; > int i; > @@ -809,19 +810,21 @@ static void ice_clean_xdp_irq_zc(struct ice_tx_ring *xdp_ring) > if ((tx_desc->cmd_type_offset_bsz & > cpu_to_le64(ICE_TX_DESC_DTYPE_DESC_DONE))) { > if (last_rs >= ntc) > - xsk_frames = last_rs - ntc + 1; > + completed_frames = last_rs - ntc + 1; > else > - xsk_frames = last_rs + cnt - ntc + 1; > + completed_frames = last_rs + cnt - ntc + 1; > } > > - if (!xsk_frames) > + if (!completed_frames) > return; > > - if (likely(!xdp_ring->xdp_tx_active)) > + if (likely(!xdp_ring->xdp_tx_active)) { > + xsk_frames = completed_frames; > goto skip; > + } > > ntc = xdp_ring->next_to_clean; > - for (i = 0; i < xsk_frames; i++) { > + for (i = 0; i < completed_frames; i++) { > tx_buf = &xdp_ring->tx_buf[ntc]; > > if (tx_buf->raw_buf) { > @@ -837,7 +840,7 @@ static void ice_clean_xdp_irq_zc(struct ice_tx_ring *xdp_ring) > } > skip: > tx_desc->cmd_type_offset_bsz = 0; > - xdp_ring->next_to_clean += xsk_frames; > + xdp_ring->next_to_clean += completed_frames; > if (xdp_ring->next_to_clean >= cnt) > xdp_ring->next_to_clean -= cnt; > if (xsk_frames) Looks good to me. Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
On 2/9/2023 8:01 AM, Zaremba, Larysa wrote: > Incrementation of xsk_frames inside the for-loop produces > infinite loop, if we have both normal AF_XDP-TX and XDP_TXed > buffers to complete. > > Split xsk_frames into 2 variables (xsk_frames and completed_frames) > to eliminate this bug. > > Fixes: 29322791bc8b ("ice: xsk: change batched Tx descriptor cleaning") > Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com> > --- > To Tony: this is urgent and should go directly via net. It's tested and acked. netdev maintainers, Since it's been tested and reviewed, did you want to take this directly? Acked-by: Tony Nguyen <anthony.l.nguyen@intel.com>
On Thu, 9 Feb 2023 09:20:27 -0800 Tony Nguyen wrote: > Since it's been tested and reviewed, did you want to take this directly? > > Acked-by: Tony Nguyen <anthony.l.nguyen@intel.com> Sure thing! Already missed today's PR tho.. :(
Hello: This patch was applied to netdev/net.git (master) by Jakub Kicinski <kuba@kernel.org>: On Thu, 9 Feb 2023 17:01:30 +0100 you wrote: > Incrementation of xsk_frames inside the for-loop produces > infinite loop, if we have both normal AF_XDP-TX and XDP_TXed > buffers to complete. > > Split xsk_frames into 2 variables (xsk_frames and completed_frames) > to eliminate this bug. > > [...] Here is the summary with links: - [net] ice: xsk: Fix cleaning of XDP_TX frames https://git.kernel.org/netdev/net/c/1f090494170e You are awesome, thank you!
diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c index 7105de6fb344..374b7f10b549 100644 --- a/drivers/net/ethernet/intel/ice/ice_xsk.c +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c @@ -800,6 +800,7 @@ static void ice_clean_xdp_irq_zc(struct ice_tx_ring *xdp_ring) struct ice_tx_desc *tx_desc; u16 cnt = xdp_ring->count; struct ice_tx_buf *tx_buf; + u16 completed_frames = 0; u16 xsk_frames = 0; u16 last_rs; int i; @@ -809,19 +810,21 @@ static void ice_clean_xdp_irq_zc(struct ice_tx_ring *xdp_ring) if ((tx_desc->cmd_type_offset_bsz & cpu_to_le64(ICE_TX_DESC_DTYPE_DESC_DONE))) { if (last_rs >= ntc) - xsk_frames = last_rs - ntc + 1; + completed_frames = last_rs - ntc + 1; else - xsk_frames = last_rs + cnt - ntc + 1; + completed_frames = last_rs + cnt - ntc + 1; } - if (!xsk_frames) + if (!completed_frames) return; - if (likely(!xdp_ring->xdp_tx_active)) + if (likely(!xdp_ring->xdp_tx_active)) { + xsk_frames = completed_frames; goto skip; + } ntc = xdp_ring->next_to_clean; - for (i = 0; i < xsk_frames; i++) { + for (i = 0; i < completed_frames; i++) { tx_buf = &xdp_ring->tx_buf[ntc]; if (tx_buf->raw_buf) { @@ -837,7 +840,7 @@ static void ice_clean_xdp_irq_zc(struct ice_tx_ring *xdp_ring) } skip: tx_desc->cmd_type_offset_bsz = 0; - xdp_ring->next_to_clean += xsk_frames; + xdp_ring->next_to_clean += completed_frames; if (xdp_ring->next_to_clean >= cnt) xdp_ring->next_to_clean -= cnt; if (xsk_frames)
Incrementation of xsk_frames inside the for-loop produces infinite loop, if we have both normal AF_XDP-TX and XDP_TXed buffers to complete. Split xsk_frames into 2 variables (xsk_frames and completed_frames) to eliminate this bug. Fixes: 29322791bc8b ("ice: xsk: change batched Tx descriptor cleaning") Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com> --- To Tony: this is urgent and should go directly via net. It's tested and acked. --- drivers/net/ethernet/intel/ice/ice_xsk.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)