Message ID | 20210205090904.20794-1-magnus.karlsson@gmail.com |
---|---|
State | Accepted |
Delegated to: | Anthony Nguyen |
Headers | show |
Series | [intel-net] ice: fix napi work done reporting in xsk path | expand |
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of > Magnus Karlsson > Sent: Friday, February 5, 2021 2:39 PM > To: Karlsson, Magnus <magnus.karlsson@intel.com>; Topel, Bjorn > <bjorn.topel@intel.com>; intel-wired-lan@lists.osuosl.org; Nguyen, Anthony > L <anthony.l.nguyen@intel.com>; Fijalkowski, Maciej > <maciej.fijalkowski@intel.com>; maciejromanfijalkowski@gmail.com > Cc: netdev@vger.kernel.org > Subject: [Intel-wired-lan] [PATCH intel-net] ice: fix napi work done reporting > in xsk path > > From: Magnus Karlsson <magnus.karlsson@intel.com> > > Fix the wrong napi work done reporting in the xsk path of the ice driver. The > code in the main Rx processing loop was written to assume that the buffer > allocation code returns true if all allocations where successful and false if > not. In contrast with all other Intel NIC xsk drivers, the ice_alloc_rx_bufs_zc() > has the inverted logic messing up the work done reporting in the napi loop. > > This can be fixed either by inverting the return value from > ice_alloc_rx_bufs_zc() in the function that uses this in an incorrect way, or by > changing the return value of ice_alloc_rx_bufs_zc(). We chose the latter as it > makes all the xsk allocation functions for Intel NICs behave in the same way. > My guess is that it was this unexpected discrepancy that gave rise to this bug > in the first place. > > Fixes: 5bb0c4b5eb61 ("ice, xsk: Move Rx allocation out of while-loop") > Reported-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> > --- > drivers/net/ethernet/intel/ice/ice_base.c | 6 ++++-- > drivers/net/ethernet/intel/ice/ice_xsk.c | 10 +++++----- > 2 files changed, 9 insertions(+), 7 deletions(-) > Tested-by: Kiran Bhandare <kiranx.bhandare@intel.com> A Contingent Worker at Intel
diff --git a/drivers/net/ethernet/intel/ice/ice_base.c b/drivers/net/ethernet/intel/ice/ice_base.c index 3124a3bf519a..952e41a1e001 100644 --- a/drivers/net/ethernet/intel/ice/ice_base.c +++ b/drivers/net/ethernet/intel/ice/ice_base.c @@ -418,6 +418,8 @@ int ice_setup_rx_ctx(struct ice_ring *ring) writel(0, ring->tail); if (ring->xsk_pool) { + bool ok; + if (!xsk_buff_can_alloc(ring->xsk_pool, num_bufs)) { dev_warn(dev, "XSK buffer pool does not provide enough addresses to fill %d buffers on Rx ring %d\n", num_bufs, ring->q_index); @@ -426,8 +428,8 @@ int ice_setup_rx_ctx(struct ice_ring *ring) return 0; } - err = ice_alloc_rx_bufs_zc(ring, num_bufs); - if (err) + ok = ice_alloc_rx_bufs_zc(ring, num_bufs); + if (!ok) dev_info(dev, "Failed to allocate some buffers on XSK buffer pool enabled Rx ring %d (pf_q %d)\n", ring->q_index, pf_q); return 0; diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c index 1782146db644..69ee1a8e87ab 100644 --- a/drivers/net/ethernet/intel/ice/ice_xsk.c +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c @@ -408,18 +408,18 @@ int ice_xsk_pool_setup(struct ice_vsi *vsi, struct xsk_buff_pool *pool, u16 qid) * This function allocates a number of Rx buffers from the fill ring * or the internal recycle mechanism and places them on the Rx ring. * - * Returns false if all allocations were successful, true if any fail. + * Returns true if all allocations were successful, false if any fail. */ bool ice_alloc_rx_bufs_zc(struct ice_ring *rx_ring, u16 count) { union ice_32b_rx_flex_desc *rx_desc; u16 ntu = rx_ring->next_to_use; struct ice_rx_buf *rx_buf; - bool ret = false; + bool ok = true; dma_addr_t dma; if (!count) - return false; + return true; rx_desc = ICE_RX_DESC(rx_ring, ntu); rx_buf = &rx_ring->rx_buf[ntu]; @@ -427,7 +427,7 @@ bool ice_alloc_rx_bufs_zc(struct ice_ring *rx_ring, u16 count) do { rx_buf->xdp = xsk_buff_alloc(rx_ring->xsk_pool); if (!rx_buf->xdp) { - ret = true; + ok = false; break; } @@ -452,7 +452,7 @@ bool ice_alloc_rx_bufs_zc(struct ice_ring *rx_ring, u16 count) ice_release_rx_desc(rx_ring, ntu); } - return ret; + return ok; } /**