[net-next 0/6][pull request] 40GbE Intel Wired LAN Driver Updates 2020-07-28

[net-next 0/6][pull request] 40GbE Intel Wired LAN Driver Updates 2020-07-28

[Tony Nguyen]

This series contains updates to i40e driver only.

Li RongQing removes binding affinity mask to a fixed CPU and sets
prefetch of Rx buffer page to occur conditionally.

Björn provides AF_XDP performance improvements by not prefetching HW
descriptors, using 16 byte descriptors, increasing budget for AF_XDP
receive, and moving buffer allocation out of Rx processing loop.

The following are changes since commit 5e619d73e6797ed9f2554a1bf996d52d8c91ca50:
  net/mlx4: Use fallthrough pseudo-keyword
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 40GbE

Björn Töpel (4):
  i40e, xsk: remove HW descriptor prefetch in AF_XDP path
  i40e: use 16B HW descriptors instead of 32B
  i40e, xsk: increase budget for AF_XDP path
  i40e, xsk: move buffer allocation out of the Rx processing loop

Li RongQing (2):
  i40e: not compute affinity_mask for IRQ
  i40e: prefetch struct page of Rx buffer conditionally

 drivers/net/ethernet/intel/i40e/i40e.h        |  2 +-
 .../net/ethernet/intel/i40e/i40e_debugfs.c    | 10 +++----
 drivers/net/ethernet/intel/i40e/i40e_main.c   | 16 ++++-------
 drivers/net/ethernet/intel/i40e/i40e_trace.h  |  6 ++--
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 21 ++++++++++++--
 drivers/net/ethernet/intel/i40e/i40e_txrx.h   |  2 +-
 .../ethernet/intel/i40e/i40e_txrx_common.h    | 13 ---------
 drivers/net/ethernet/intel/i40e/i40e_type.h   |  5 +++-
 drivers/net/ethernet/intel/i40e/i40e_xsk.c    | 28 +++++++++++++------
 9 files changed, 55 insertions(+), 48 deletions(-)

-- 
2.26.2

[net-next 1/6] i40e: not compute affinity_mask for IRQ

[Tony Nguyen]

From: Li RongQing <lirongqing@baidu.com>

After commit 759dc4a7e605 ("i40e: initialize our affinity_mask
based on cpu_possible_mask"), NAPI IRQ affinity_mask is bind to
all possible CPUs, not a fixed CPU

Signed-off-by: Li RongQing <lirongqing@baidu.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index dadbfb3d2a2b..523c07961b4b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -11185,11 +11185,10 @@ static int i40e_init_msix(struct i40e_pf *pf)
  * i40e_vsi_alloc_q_vector - Allocate memory for a single interrupt vector
  * @vsi: the VSI being configured
  * @v_idx: index of the vector in the vsi struct
- * @cpu: cpu to be used on affinity_mask
  *
  * We allocate one q_vector.  If allocation fails we return -ENOMEM.
  **/
-static int i40e_vsi_alloc_q_vector(struct i40e_vsi *vsi, int v_idx, int cpu)
+static int i40e_vsi_alloc_q_vector(struct i40e_vsi *vsi, int v_idx)
 {
 	struct i40e_q_vector *q_vector;
 
@@ -11222,7 +11221,7 @@ static int i40e_vsi_alloc_q_vector(struct i40e_vsi *vsi, int v_idx, int cpu)
 static int i40e_vsi_alloc_q_vectors(struct i40e_vsi *vsi)
 {
 	struct i40e_pf *pf = vsi->back;
-	int err, v_idx, num_q_vectors, current_cpu;
+	int err, v_idx, num_q_vectors;
 
 	/* if not MSIX, give the one vector only to the LAN VSI */
 	if (pf->flags & I40E_FLAG_MSIX_ENABLED)
@@ -11232,15 +11231,10 @@ static int i40e_vsi_alloc_q_vectors(struct i40e_vsi *vsi)
 	else
 		return -EINVAL;
 
-	current_cpu = cpumask_first(cpu_online_mask);
-
 	for (v_idx = 0; v_idx < num_q_vectors; v_idx++) {
-		err = i40e_vsi_alloc_q_vector(vsi, v_idx, current_cpu);
+		err = i40e_vsi_alloc_q_vector(vsi, v_idx);
 		if (err)
 			goto err_out;
-		current_cpu = cpumask_next(current_cpu, cpu_online_mask);
-		if (unlikely(current_cpu >= nr_cpu_ids))
-			current_cpu = cpumask_first(cpu_online_mask);
 	}
 
 	return 0;
-- 
2.26.2

[net-next 2/6] i40e: prefetch struct page of Rx buffer conditionally

[Tony Nguyen]

From: Li RongQing <lirongqing@baidu.com>

page_address() accesses struct page only when WANT_PAGE_VIRTUAL
or HASHED_PAGE_VIRTUAL is defined, otherwise it returns address
based on offset, so we prefetch it conditionally

Signed-off-by: Li RongQing <lirongqing@baidu.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 3e5c566ceb01..5d408fe26063 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1953,7 +1953,9 @@ static struct i40e_rx_buffer *i40e_get_rx_buffer(struct i40e_ring *rx_ring,
 	struct i40e_rx_buffer *rx_buffer;
 
 	rx_buffer = i40e_rx_bi(rx_ring, rx_ring->next_to_clean);
+#if defined(WANT_PAGE_VIRTUAL) || defined(HASHED_PAGE_VIRTUAL)
 	prefetchw(rx_buffer->page);
+#endif
 
 	/* we are reusing so sync this buffer for CPU use */
 	dma_sync_single_range_for_cpu(rx_ring->dev,
-- 
2.26.2

[net-next 3/6] i40e, xsk: remove HW descriptor prefetch in AF_XDP path

[Tony Nguyen]

From: Björn Töpel <bjorn.topel@intel.com>

The software prefetching of HW descriptors has a negative impact on
the performance. Therefore, it is now removed.

Performance for the rx_drop benchmark increased with 2%.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c        | 13 +++++++++++++
 drivers/net/ethernet/intel/i40e/i40e_txrx_common.h | 13 -------------
 drivers/net/ethernet/intel/i40e/i40e_xsk.c         | 12 ++++++++++++
 3 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 5d408fe26063..31dc1a011b2a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2301,6 +2301,19 @@ void i40e_finalize_xdp_rx(struct i40e_ring *rx_ring, unsigned int xdp_res)
 	}
 }
 
+/**
+ * i40e_inc_ntc: Advance the next_to_clean index
+ * @rx_ring: Rx ring
+ **/
+static void i40e_inc_ntc(struct i40e_ring *rx_ring)
+{
+	u32 ntc = rx_ring->next_to_clean + 1;
+
+	ntc = (ntc < rx_ring->count) ? ntc : 0;
+	rx_ring->next_to_clean = ntc;
+	prefetch(I40E_RX_DESC(rx_ring, ntc));
+}
+
 /**
  * i40e_clean_rx_irq - Clean completed descriptors from Rx ring - bounce buf
  * @rx_ring: rx descriptor ring to transact packets on
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx_common.h b/drivers/net/ethernet/intel/i40e/i40e_txrx_common.h
index 667c4dc4b39f..1397dd3c1c57 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx_common.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx_common.h
@@ -99,19 +99,6 @@ static inline bool i40e_rx_is_programming_status(u64 qword1)
 	return qword1 & I40E_RXD_QW1_LENGTH_SPH_MASK;
 }
 
-/**
- * i40e_inc_ntc: Advance the next_to_clean index
- * @rx_ring: Rx ring
- **/
-static inline void i40e_inc_ntc(struct i40e_ring *rx_ring)
-{
-	u32 ntc = rx_ring->next_to_clean + 1;
-
-	ntc = (ntc < rx_ring->count) ? ntc : 0;
-	rx_ring->next_to_clean = ntc;
-	prefetch(I40E_RX_DESC(rx_ring, ntc));
-}
-
 void i40e_xsk_clean_rx_ring(struct i40e_ring *rx_ring);
 void i40e_xsk_clean_tx_ring(struct i40e_ring *tx_ring);
 bool i40e_xsk_any_rx_ring_enabled(struct i40e_vsi *vsi);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 8ce57b507a21..1f2dd591dbf1 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -253,6 +253,18 @@ static struct sk_buff *i40e_construct_skb_zc(struct i40e_ring *rx_ring,
 	return skb;
 }
 
+/**
+ * i40e_inc_ntc: Advance the next_to_clean index
+ * @rx_ring: Rx ring
+ **/
+static void i40e_inc_ntc(struct i40e_ring *rx_ring)
+{
+	u32 ntc = rx_ring->next_to_clean + 1;
+
+	ntc = (ntc < rx_ring->count) ? ntc : 0;
+	rx_ring->next_to_clean = ntc;
+}
+
 /**
  * i40e_clean_rx_irq_zc - Consumes Rx packets from the hardware ring
  * @rx_ring: Rx ring
-- 
2.26.2

[net-next 4/6] i40e: use 16B HW descriptors instead of 32B

[Tony Nguyen]

From: Björn Töpel <bjorn.topel@intel.com>

The i40e NIC supports two flavors of HW descriptors, 16 and 32
byte. The latter has, obviously, room for more offloading
information. However, the only fields of the 32B HW descriptor that is
being used by the driver, is also available in the 16B descriptor.

In other words; Reading and writing 32 bytes instead of 16 byte is a
waste of bus bandwidth.

This commit starts using 16 byte descriptors instead of 32 byte
descriptors.

For AF_XDP the rx_drop benchmark was improved by 2%.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e.h         |  2 +-
 drivers/net/ethernet/intel/i40e/i40e_debugfs.c | 10 ++++------
 drivers/net/ethernet/intel/i40e/i40e_main.c    |  4 ++--
 drivers/net/ethernet/intel/i40e/i40e_trace.h   |  6 +++---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c    |  6 +++---
 drivers/net/ethernet/intel/i40e/i40e_txrx.h    |  2 +-
 drivers/net/ethernet/intel/i40e/i40e_type.h    |  5 ++++-
 7 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index a7e212d1caa2..ada0e93c38f0 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -90,7 +90,7 @@
 #define I40E_OEM_RELEASE_MASK		0x0000ffff
 
 #define I40E_RX_DESC(R, i)	\
-	(&(((union i40e_32byte_rx_desc *)((R)->desc))[i]))
+	(&(((union i40e_rx_desc *)((R)->desc))[i]))
 #define I40E_TX_DESC(R, i)	\
 	(&(((struct i40e_tx_desc *)((R)->desc))[i]))
 #define I40E_TX_CTXTDESC(R, i)	\
diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index d3ad2e3aa838..d7c13ca9be7d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
@@ -604,10 +604,9 @@ static void i40e_dbg_dump_desc(int cnt, int vsi_seid, int ring_id, int desc_n,
 			} else {
 				rxd = I40E_RX_DESC(ring, i);
 				dev_info(&pf->pdev->dev,
-					 "   d[%03x] = 0x%016llx 0x%016llx 0x%016llx 0x%016llx\n",
+					 "   d[%03x] = 0x%016llx 0x%016llx\n",
 					 i, rxd->read.pkt_addr,
-					 rxd->read.hdr_addr,
-					 rxd->read.rsvd1, rxd->read.rsvd2);
+					 rxd->read.hdr_addr);
 			}
 		}
 	} else if (cnt == 3) {
@@ -625,10 +624,9 @@ static void i40e_dbg_dump_desc(int cnt, int vsi_seid, int ring_id, int desc_n,
 		} else {
 			rxd = I40E_RX_DESC(ring, desc_n);
 			dev_info(&pf->pdev->dev,
-				 "vsi = %02i rx ring = %02i d[%03x] = 0x%016llx 0x%016llx 0x%016llx 0x%016llx\n",
+				 "vsi = %02i rx ring = %02i d[%03x] = 0x%016llx 0x%016llx\n",
 				 vsi_seid, ring_id, desc_n,
-				 rxd->read.pkt_addr, rxd->read.hdr_addr,
-				 rxd->read.rsvd1, rxd->read.rsvd2);
+				 rxd->read.pkt_addr, rxd->read.hdr_addr);
 		}
 	} else {
 		dev_info(&pf->pdev->dev, "dump desc rx/tx/xdp <vsi_seid> <ring_id> [<desc_n>]\n");
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 523c07961b4b..cc4d7c03bdd5 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -3320,8 +3320,8 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
 	rx_ctx.base = (ring->dma / 128);
 	rx_ctx.qlen = ring->count;
 
-	/* use 32 byte descriptors */
-	rx_ctx.dsize = 1;
+	/* use 16 byte descriptors */
+	rx_ctx.dsize = 0;
 
 	/* descriptor type is always zero
 	 * rx_ctx.dtype = 0;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_trace.h b/drivers/net/ethernet/intel/i40e/i40e_trace.h
index 424f02077e2e..983f8b98b275 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_trace.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_trace.h
@@ -112,7 +112,7 @@ DECLARE_EVENT_CLASS(
 	i40e_rx_template,
 
 	TP_PROTO(struct i40e_ring *ring,
-		 union i40e_32byte_rx_desc *desc,
+		 union i40e_16byte_rx_desc *desc,
 		 struct sk_buff *skb),
 
 	TP_ARGS(ring, desc, skb),
@@ -140,7 +140,7 @@ DECLARE_EVENT_CLASS(
 DEFINE_EVENT(
 	i40e_rx_template, i40e_clean_rx_irq,
 	TP_PROTO(struct i40e_ring *ring,
-		 union i40e_32byte_rx_desc *desc,
+		 union i40e_16byte_rx_desc *desc,
 		 struct sk_buff *skb),
 
 	TP_ARGS(ring, desc, skb));
@@ -148,7 +148,7 @@ DEFINE_EVENT(
 DEFINE_EVENT(
 	i40e_rx_template, i40e_clean_rx_irq_rx,
 	TP_PROTO(struct i40e_ring *ring,
-		 union i40e_32byte_rx_desc *desc,
+		 union i40e_16byte_rx_desc *desc,
 		 struct sk_buff *skb),
 
 	TP_ARGS(ring, desc, skb));
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 31dc1a011b2a..2fb6dddf9106 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -533,11 +533,11 @@ static void i40e_fd_handle_status(struct i40e_ring *rx_ring, u64 qword0_raw,
 {
 	struct i40e_pf *pf = rx_ring->vsi->back;
 	struct pci_dev *pdev = pf->pdev;
-	struct i40e_32b_rx_wb_qw0 *qw0;
+	struct i40e_16b_rx_wb_qw0 *qw0;
 	u32 fcnt_prog, fcnt_avail;
 	u32 error;
 
-	qw0 = (struct i40e_32b_rx_wb_qw0 *)&qword0_raw;
+	qw0 = (struct i40e_16b_rx_wb_qw0 *)&qword0_raw;
 	error = (qword1 & I40E_RX_PROG_STATUS_DESC_QW1_ERROR_MASK) >>
 		I40E_RX_PROG_STATUS_DESC_QW1_ERROR_SHIFT;
 
@@ -1418,7 +1418,7 @@ int i40e_setup_rx_descriptors(struct i40e_ring *rx_ring)
 	u64_stats_init(&rx_ring->syncp);
 
 	/* Round up to nearest 4K */
-	rx_ring->size = rx_ring->count * sizeof(union i40e_32byte_rx_desc);
+	rx_ring->size = rx_ring->count * sizeof(union i40e_rx_desc);
 	rx_ring->size = ALIGN(rx_ring->size, 4096);
 	rx_ring->desc = dma_alloc_coherent(dev, rx_ring->size,
 					   &rx_ring->dma, GFP_KERNEL);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
index 4036893d6825..0eacd5f21e9d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
@@ -110,7 +110,7 @@ enum i40e_dyn_idx_t {
  */
 #define I40E_RX_HDR_SIZE I40E_RXBUFFER_256
 #define I40E_PACKET_HDR_PAD (ETH_HLEN + ETH_FCS_LEN + (VLAN_HLEN * 2))
-#define i40e_rx_desc i40e_32byte_rx_desc
+#define i40e_rx_desc i40e_16byte_rx_desc
 
 #define I40E_RX_DMA_ATTR \
 	(DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h
index 52410d609ba1..97d29df65f9e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_type.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_type.h
@@ -628,7 +628,7 @@ union i40e_16byte_rx_desc {
 		__le64 hdr_addr; /* Header buffer address */
 	} read;
 	struct {
-		struct {
+		struct i40e_16b_rx_wb_qw0 {
 			struct {
 				union {
 					__le16 mirroring_status;
@@ -647,6 +647,9 @@ union i40e_16byte_rx_desc {
 			__le64 status_error_len;
 		} qword1;
 	} wb;  /* writeback */
+	struct {
+		u64 qword[2];
+	} raw;
 };
 
 union i40e_32byte_rx_desc {
-- 
2.26.2

[net-next 5/6] i40e, xsk: increase budget for AF_XDP path

[Tony Nguyen]

From: Björn Töpel <bjorn.topel@intel.com>

The napi_budget, meaning the number of received packets that are
allowed to be processed for each napi invocation, takes into
consideration that each received packet is aimed for the kernel
networking stack.

That is not the case for the AF_XDP receive path, where the cost of
each packet is significantly less. Therefore, this commit disregards
the napi budget and increases it to 256. Processing 256 packets
targeted for AF_XDP is still less work than 64 (napi budget) packets
going to the kernel networking stack.

The performance for the rx_drop scenario is up 7%.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_xsk.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 1f2dd591dbf1..99f4afdc403d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -265,6 +265,8 @@ static void i40e_inc_ntc(struct i40e_ring *rx_ring)
 	rx_ring->next_to_clean = ntc;
 }
 
+#define I40E_XSK_CLEAN_RX_BUDGET 256U
+
 /**
  * i40e_clean_rx_irq_zc - Consumes Rx packets from the hardware ring
  * @rx_ring: Rx ring
@@ -280,7 +282,7 @@ int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget)
 	bool failure = false;
 	struct sk_buff *skb;
 
-	while (likely(total_rx_packets < (unsigned int)budget)) {
+	while (likely(total_rx_packets < I40E_XSK_CLEAN_RX_BUDGET)) {
 		union i40e_rx_desc *rx_desc;
 		struct xdp_buff **bi;
 		unsigned int size;
-- 
2.26.2

[net-next 6/6] i40e, xsk: move buffer allocation out of the Rx processing loop

[Tony Nguyen]

From: Björn Töpel <bjorn.topel@intel.com>

Instead of checking in each iteration of the Rx packet processing
loop, move the allocation out of the loop and do it once for each napi
activation.

For AF_XDP the rx_drop benchmark was improved by 6%.

Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_xsk.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 99f4afdc403d..91aee16fbe72 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -279,8 +279,8 @@ int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget)
 	unsigned int total_rx_bytes = 0, total_rx_packets = 0;
 	u16 cleaned_count = I40E_DESC_UNUSED(rx_ring);
 	unsigned int xdp_res, xdp_xmit = 0;
-	bool failure = false;
 	struct sk_buff *skb;
+	bool failure;
 
 	while (likely(total_rx_packets < I40E_XSK_CLEAN_RX_BUDGET)) {
 		union i40e_rx_desc *rx_desc;
@@ -288,13 +288,6 @@ int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget)
 		unsigned int size;
 		u64 qword;
 
-		if (cleaned_count >= I40E_RX_BUFFER_WRITE) {
-			failure = failure ||
-				  !i40e_alloc_rx_buffers_zc(rx_ring,
-							    cleaned_count);
-			cleaned_count = 0;
-		}
-
 		rx_desc = I40E_RX_DESC(rx_ring, rx_ring->next_to_clean);
 		qword = le64_to_cpu(rx_desc->wb.qword1.status_error_len);
 
@@ -369,6 +362,9 @@ int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget)
 		napi_gro_receive(&rx_ring->q_vector->napi, skb);
 	}
 
+	if (cleaned_count >= I40E_RX_BUFFER_WRITE)
+		failure = !i40e_alloc_rx_buffers_zc(rx_ring, cleaned_count);
+
 	i40e_finalize_xdp_rx(rx_ring, xdp_xmit);
 	i40e_update_rx_stats(rx_ring, total_rx_bytes, total_rx_packets);
 
-- 
2.26.2

Re: [net-next 2/6] i40e: prefetch struct page of Rx buffer conditionally

[Jakub Kicinski]

On Tue, 28 Jul 2020 12:08:38 -0700 Tony Nguyen wrote:
> From: Li RongQing <lirongqing@baidu.com>
> 
> page_address() accesses struct page only when WANT_PAGE_VIRTUAL
> or HASHED_PAGE_VIRTUAL is defined, otherwise it returns address
> based on offset, so we prefetch it conditionally
> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> index 3e5c566ceb01..5d408fe26063 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> @@ -1953,7 +1953,9 @@ static struct i40e_rx_buffer *i40e_get_rx_buffer(struct i40e_ring *rx_ring,
>  	struct i40e_rx_buffer *rx_buffer;
>  
>  	rx_buffer = i40e_rx_bi(rx_ring, rx_ring->next_to_clean);
> +#if defined(WANT_PAGE_VIRTUAL) || defined(HASHED_PAGE_VIRTUAL)
>  	prefetchw(rx_buffer->page);
> +#endif

Looks like something that belongs in a common header not (potentially
multiple) C sources.

Re: [net-next 5/6] i40e, xsk: increase budget for AF_XDP path

[Jakub Kicinski]

On Tue, 28 Jul 2020 12:08:41 -0700 Tony Nguyen wrote:
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> index 1f2dd591dbf1..99f4afdc403d 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> @@ -265,6 +265,8 @@ static void i40e_inc_ntc(struct i40e_ring *rx_ring)
>  	rx_ring->next_to_clean = ntc;
>  }
>  
> +#define I40E_XSK_CLEAN_RX_BUDGET 256U
> +
>  /**
>   * i40e_clean_rx_irq_zc - Consumes Rx packets from the hardware ring
>   * @rx_ring: Rx ring
> @@ -280,7 +282,7 @@ int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget)
>  	bool failure = false;
>  	struct sk_buff *skb;
>  
> -	while (likely(total_rx_packets < (unsigned int)budget)) {
> +	while (likely(total_rx_packets < I40E_XSK_CLEAN_RX_BUDGET)) {
>  		union i40e_rx_desc *rx_desc;
>  		struct xdp_buff **bi;
>  		unsigned int size;

Should this perhaps be a common things that drivers do?

Should we define a "XSK_NAPI_WEIGHT_MULT 4" instead of hard coding the
value in a driver?

Re: [net-next 0/6][pull request] 40GbE Intel Wired LAN Driver Updates 2020-07-28

[Jakub Kicinski]

On Tue, 28 Jul 2020 12:08:36 -0700 Tony Nguyen wrote:
> This series contains updates to i40e driver only.
> 
> Li RongQing removes binding affinity mask to a fixed CPU and sets
> prefetch of Rx buffer page to occur conditionally.
> 
> Björn provides AF_XDP performance improvements by not prefetching HW
> descriptors, using 16 byte descriptors, increasing budget for AF_XDP
> receive, and moving buffer allocation out of Rx processing loop.

My comment on patch #2 is really a nit, but for patch 5 I think we
should consider carefully a common path rather than "tweak" the drivers
like this.

答复: [net-next 2/6] i40e: prefetch struct page of Rx buffer conditionally

[Li,Rongqing]

> -----邮件原件-----
> 发件人: Jakub Kicinski [mailto:kuba@kernel.org]
> 发送时间: 2020年7月29日 4:14
> 收件人: Tony Nguyen <anthony.l.nguyen@intel.com>
> 抄送: davem@davemloft.net; Li,Rongqing <lirongqing@baidu.com>;
> netdev@vger.kernel.org; nhorman@redhat.com; sassmann@redhat.com;
> jeffrey.t.kirsher@intel.com; Andrew Bowers <andrewx.bowers@intel.com>
> 主题: Re: [net-next 2/6] i40e: prefetch struct page of Rx buffer conditionally
> 
> On Tue, 28 Jul 2020 12:08:38 -0700 Tony Nguyen wrote:
> > From: Li RongQing <lirongqing@baidu.com>
> >
> > page_address() accesses struct page only when WANT_PAGE_VIRTUAL or
> > HASHED_PAGE_VIRTUAL is defined, otherwise it returns address based on
> > offset, so we prefetch it conditionally
> >
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> > Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> > ---
> >  drivers/net/ethernet/intel/i40e/i40e_txrx.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> > b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> > index 3e5c566ceb01..5d408fe26063 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> > @@ -1953,7 +1953,9 @@ static struct i40e_rx_buffer
> *i40e_get_rx_buffer(struct i40e_ring *rx_ring,
> >  	struct i40e_rx_buffer *rx_buffer;
> >
> >  	rx_buffer = i40e_rx_bi(rx_ring, rx_ring->next_to_clean);
> > +#if defined(WANT_PAGE_VIRTUAL) || defined(HASHED_PAGE_VIRTUAL)
> >  	prefetchw(rx_buffer->page);

Maybe I change prefetchw to prefetch.

refcount of rx_buffer page will be added here originally,
so prefetchw is needed, but after commit 1793668c3b8c
("i40e/i40evf: Update code to better handle incrementing page count"),
and refcount is not added everytime, so change prefetchw
as prefetch,

> > +#endif
> 
> Looks like something that belongs in a common header not (potentially
> multiple) C sources.

Not clear, how should I change?

-Li

Re: [net-next 5/6] i40e, xsk: increase budget for AF_XDP path

[Björn Töpel]

On 2020-07-28 22:15, Jakub Kicinski wrote:
> On Tue, 28 Jul 2020 12:08:41 -0700 Tony Nguyen wrote:
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
>> index 1f2dd591dbf1..99f4afdc403d 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
>> @@ -265,6 +265,8 @@ static void i40e_inc_ntc(struct i40e_ring *rx_ring)
>>   	rx_ring->next_to_clean = ntc;
>>   }
>>   
>> +#define I40E_XSK_CLEAN_RX_BUDGET 256U
>> +
>>   /**
>>    * i40e_clean_rx_irq_zc - Consumes Rx packets from the hardware ring
>>    * @rx_ring: Rx ring
>> @@ -280,7 +282,7 @@ int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget)
>>   	bool failure = false;
>>   	struct sk_buff *skb;
>>   
>> -	while (likely(total_rx_packets < (unsigned int)budget)) {
>> +	while (likely(total_rx_packets < I40E_XSK_CLEAN_RX_BUDGET)) {
>>   		union i40e_rx_desc *rx_desc;
>>   		struct xdp_buff **bi;
>>   		unsigned int size;
> 
> Should this perhaps be a common things that drivers do?
> 
> Should we define a "XSK_NAPI_WEIGHT_MULT 4" instead of hard coding the
> value in a driver?
>

Yes, that's a good idea. I can generalize for the AF_XDP paths in the 
other drivers as a follow up!


Cheers,
Björn (in vacation mode)

Re: [net-next 2/6] i40e: prefetch struct page of Rx buffer conditionally

[Jakub Kicinski]

On Wed, 29 Jul 2020 06:20:47 +0000 Li,Rongqing wrote:
> > Looks like something that belongs in a common header not (potentially
> > multiple) C sources.  
> 
> Not clear, how should I change?

Can you add something like:

static inline void prefetch_page_address(struct page *page)
{
#if defined(WANT_PAGE_VIRTUAL) || defined(HASHED_PAGE_VIRTUAL)
	prefetch(page);
#endif
}

to mm.h or prefetch.h or i40e.h or some other header? That's preferred
over adding #ifs directly in the source code.

Re: [net-next 5/6] i40e, xsk: increase budget for AF_XDP path

[Jakub Kicinski]

On Wed, 29 Jul 2020 12:43:46 +0200 Björn Töpel wrote:
> > Should this perhaps be a common things that drivers do?
> > 
> > Should we define a "XSK_NAPI_WEIGHT_MULT 4" instead of hard coding the
> > value in a driver?
> 
> Yes, that's a good idea. I can generalize for the AF_XDP paths in the 
> other drivers as a follow up!

I don't like followups 'cause there's no way I can track if everyone
actually does what I asked them to - but since you're in vacation mode,
let's say this one's fine as a follow up ;)

答复: [net-next 2/6] i40e: prefetch struct page of Rx buffer conditionally

[Li,Rongqing]

> -----邮件原件-----
> 发件人: Jakub Kicinski [mailto:kuba@kernel.org]
> 发送时间: 2020年7月30日 5:20
> 收件人: Li,Rongqing <lirongqing@baidu.com>
> 抄送: Tony Nguyen <anthony.l.nguyen@intel.com>; davem@davemloft.net;
> netdev@vger.kernel.org; nhorman@redhat.com; sassmann@redhat.com;
> jeffrey.t.kirsher@intel.com; Andrew Bowers <andrewx.bowers@intel.com>
> 主题: Re: [net-next 2/6] i40e: prefetch struct page of Rx buffer conditionally
> 
> On Wed, 29 Jul 2020 06:20:47 +0000 Li,Rongqing wrote:
> > > Looks like something that belongs in a common header not
> > > (potentially
> > > multiple) C sources.
> >
> > Not clear, how should I change?
> 
> Can you add something like:
> 
> static inline void prefetch_page_address(struct page *page) { #if
> defined(WANT_PAGE_VIRTUAL) || defined(HASHED_PAGE_VIRTUAL)
> 	prefetch(page);
> #endif
> }
> 
> to mm.h or prefetch.h or i40e.h or some other header? That's preferred over
> adding #ifs directly in the source code.


Thanks, I will send a new version

-Li

Re: [net-next 0/6][pull request] 40GbE Intel Wired LAN Driver Updates 2020-07-28

[Björn Töpel]

On Tue, 28 Jul 2020 at 22:20, Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 28 Jul 2020 12:08:36 -0700 Tony Nguyen wrote:
> > This series contains updates to i40e driver only.
> >
> > Li RongQing removes binding affinity mask to a fixed CPU and sets
> > prefetch of Rx buffer page to occur conditionally.
> >
> > Björn provides AF_XDP performance improvements by not prefetching HW
> > descriptors, using 16 byte descriptors, increasing budget for AF_XDP
> > receive, and moving buffer allocation out of Rx processing loop.
>
> My comment on patch #2 is really a nit, but for patch 5 I think we
> should consider carefully a common path rather than "tweak" the drivers
> like this.

Yup, I agree that tweaks like this should be avoided, and I said that
I'll address it in a follow up. However, I was under the impression
that this series would still be pulled (discussed in patch 5), but I
can't find it in the tree.

Thanks,
Björn

Re: [net-next 0/6][pull request] 40GbE Intel Wired LAN Driver Updates 2020-07-28

[Björn Töpel]

On Mon, 10 Aug 2020 at 13:47, Björn Töpel <bjorn.topel@gmail.com> wrote:
>
> On Tue, 28 Jul 2020 at 22:20, Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Tue, 28 Jul 2020 12:08:36 -0700 Tony Nguyen wrote:
> > > This series contains updates to i40e driver only.
> > >
> > > Li RongQing removes binding affinity mask to a fixed CPU and sets
> > > prefetch of Rx buffer page to occur conditionally.
> > >
> > > Björn provides AF_XDP performance improvements by not prefetching HW
> > > descriptors, using 16 byte descriptors, increasing budget for AF_XDP
> > > receive, and moving buffer allocation out of Rx processing loop.
> >
> > My comment on patch #2 is really a nit, but for patch 5 I think we
> > should consider carefully a common path rather than "tweak" the drivers
> > like this.
>
> Yup, I agree that tweaks like this should be avoided, and I said that
> I'll address it in a follow up. However, I was under the impression
> that this series would still be pulled (discussed in patch 5), but I
> can't find it in the tree.
>

Ugh. Please ignore the noise. Patchwork says "Changes Requested".

I'll rework the series!


Cheers,
Björn