From patchwork Fri Mar 12 17:47:51 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tony Nguyen X-Patchwork-Id: 399399 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4045FC4332B for ; Fri, 12 Mar 2021 17:47:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1A37264F0F for ; Fri, 12 Mar 2021 17:47:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231557AbhCLRrK (ORCPT ); Fri, 12 Mar 2021 12:47:10 -0500 Received: from mga06.intel.com ([134.134.136.31]:9116 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231392AbhCLRqi (ORCPT ); Fri, 12 Mar 2021 12:46:38 -0500 IronPort-SDR: AoTZ9Nbpjgjtd8SpgfE5kZnfk4lGPipyVarhrunm1eB3IY7ICl1icAmsunY3alvzXLt+ON4hGg vzCRewQgUgNg== X-IronPort-AV: E=McAfee;i="6000,8403,9921"; a="250232654" X-IronPort-AV: E=Sophos;i="5.81,244,1610438400"; d="scan'208";a="250232654" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Mar 2021 09:46:38 -0800 IronPort-SDR: yHdnIH1PsYKnXfVx8L9Tz5NF0/sHC6/MC+6+g4dBAp9sfHrDF6YHDQ351Cu6ygDuJt6rdfzxfK KbYSwY9C5kXA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.81,244,1610438400"; d="scan'208";a="589615846" Received: from anguy11-desk2.jf.intel.com ([10.166.244.147]) by orsmga005.jf.intel.com with ESMTP; 12 Mar 2021 09:46:37 -0800 From: Tony Nguyen To: davem@davemloft.net, kuba@kernel.org Cc: Magnus Karlsson , netdev@vger.kernel.org, sassmann@redhat.com, anthony.l.nguyen@intel.com, bjorn.topel@intel.com, maciej.fijalkowski@intel.com, Kiran Bhandare Subject: [PATCH net 1/5] ice: fix napi work done reporting in xsk path Date: Fri, 12 Mar 2021 09:47:51 -0800 Message-Id: <20210312174755.2103336-2-anthony.l.nguyen@intel.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210312174755.2103336-1-anthony.l.nguyen@intel.com> References: <20210312174755.2103336-1-anthony.l.nguyen@intel.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Magnus Karlsson 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 Signed-off-by: Magnus Karlsson Tested-by: Kiran Bhandare Signed-off-by: Tony Nguyen --- 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(-) 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 83f3c9574ed1..9f94d9159acd 100644 --- a/drivers/net/ethernet/intel/ice/ice_xsk.c +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c @@ -358,18 +358,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]; @@ -377,7 +377,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; } @@ -402,7 +402,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; } /** From patchwork Fri Mar 12 17:47:52 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tony Nguyen X-Patchwork-Id: 399400 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 91516C433DB for ; Fri, 12 Mar 2021 17:47:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5A15464F0E for ; Fri, 12 Mar 2021 17:47:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232468AbhCLRrJ (ORCPT ); Fri, 12 Mar 2021 12:47:09 -0500 Received: from mga06.intel.com ([134.134.136.31]:9116 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232233AbhCLRqi (ORCPT ); Fri, 12 Mar 2021 12:46:38 -0500 IronPort-SDR: FQxB7mO4UcvBTA4oYkvJASVDVgwCS9QVWWHGAd9owJCqY/mZ/pilHH2mFXRmL5Exh8QovZWNoa /Yd4olSnSJsA== X-IronPort-AV: E=McAfee;i="6000,8403,9921"; a="250232655" X-IronPort-AV: E=Sophos;i="5.81,244,1610438400"; d="scan'208";a="250232655" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Mar 2021 09:46:38 -0800 IronPort-SDR: /HBHtw9GWVepyl3BgYEwkiehjB998RG+8KWFVFAqwYTIRkEUTmvHFwjdcnqdSejC4RCexw8iKi BSquv+qYmG7Q== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.81,244,1610438400"; d="scan'208";a="589615848" Received: from anguy11-desk2.jf.intel.com ([10.166.244.147]) by orsmga005.jf.intel.com with ESMTP; 12 Mar 2021 09:46:38 -0800 From: Tony Nguyen To: davem@davemloft.net, kuba@kernel.org Cc: Maciej Fijalkowski , netdev@vger.kernel.org, sassmann@redhat.com, anthony.l.nguyen@intel.com, bjorn.topel@intel.com, magnus.karlsson@intel.com, Jesper Dangaard Brouer , Kiran Bhandare Subject: [PATCH net 2/5] i40e: move headroom initialization to i40e_configure_rx_ring Date: Fri, 12 Mar 2021 09:47:52 -0800 Message-Id: <20210312174755.2103336-3-anthony.l.nguyen@intel.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20210312174755.2103336-1-anthony.l.nguyen@intel.com> References: <20210312174755.2103336-1-anthony.l.nguyen@intel.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Maciej Fijalkowski i40e_rx_offset(), that is supposed to initialize the Rx buffer headroom, relies on I40E_RXR_FLAGS_BUILD_SKB_ENABLED flag. Currently, the callsite of mentioned function is placed incorrectly within i40e_setup_rx_descriptors() where Rx ring's build skb flag is not set yet. This causes the XDP_REDIRECT to be partially broken due to inability to create xdp_frame in the headroom space, as the headroom is 0. For the record, below is the call graph: i40e_vsi_open i40e_vsi_setup_rx_resources i40e_setup_rx_descriptors i40e_rx_offset() <-- sets offset to 0 as build_skb flag is set below i40e_vsi_configure_rx i40e_configure_rx_ring set_ring_build_skb_enabled(ring) <-- set build_skb flag Fix this by moving i40e_rx_offset() to i40e_configure_rx_ring() after the flag setting. Fixes: f7bb0d71d658 ("i40e: store the result of i40e_rx_offset() onto i40e_ring") Reported-by: Jesper Dangaard Brouer Co-developed-by: Jesper Dangaard Brouer Signed-off-by: Jesper Dangaard Brouer Signed-off-by: Maciej Fijalkowski Acked-by: Jesper Dangaard Brouer Tested-by: Jesper Dangaard Brouer Tested-by: Kiran Bhandare Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/i40e/i40e_main.c | 13 +++++++++++++ drivers/net/ethernet/intel/i40e/i40e_txrx.c | 12 ------------ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index 353deae139f9..17f3b800640e 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -3258,6 +3258,17 @@ static int i40e_configure_tx_ring(struct i40e_ring *ring) return 0; } +/** + * i40e_rx_offset - Return expected offset into page to access data + * @rx_ring: Ring we are requesting offset of + * + * Returns the offset value for ring into the data buffer. + */ +static unsigned int i40e_rx_offset(struct i40e_ring *rx_ring) +{ + return ring_uses_build_skb(rx_ring) ? I40E_SKB_PAD : 0; +} + /** * i40e_configure_rx_ring - Configure a receive ring context * @ring: The Rx ring to configure @@ -3369,6 +3380,8 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring) else set_ring_build_skb_enabled(ring); + ring->rx_offset = i40e_rx_offset(ring); + /* cache tail for quicker writes, and clear the reg before use */ ring->tail = hw->hw_addr + I40E_QRX_TAIL(pf_q); writel(0, ring->tail); diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c index 627794b31e33..5747a99122fb 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c @@ -1569,17 +1569,6 @@ void i40e_free_rx_resources(struct i40e_ring *rx_ring) } } -/** - * i40e_rx_offset - Return expected offset into page to access data - * @rx_ring: Ring we are requesting offset of - * - * Returns the offset value for ring into the data buffer. - */ -static unsigned int i40e_rx_offset(struct i40e_ring *rx_ring) -{ - return ring_uses_build_skb(rx_ring) ? I40E_SKB_PAD : 0; -} - /** * i40e_setup_rx_descriptors - Allocate Rx descriptors * @rx_ring: Rx descriptor ring (for a specific queue) to setup @@ -1608,7 +1597,6 @@ int i40e_setup_rx_descriptors(struct i40e_ring *rx_ring) rx_ring->next_to_alloc = 0; rx_ring->next_to_clean = 0; rx_ring->next_to_use = 0; - rx_ring->rx_offset = i40e_rx_offset(rx_ring); /* XDP RX-queue info only needed for RX rings exposed to XDP */ if (rx_ring->vsi->type == I40E_VSI_MAIN) {