[PATCH 0/2] reduce writes in i40e driver

[PATCH 0/2] reduce writes in i40e driver

[Bruce Richardson]

this set is based upon Olivier's mbuf rework patchset, and makes some
improvement to the i40e driver taking account of the rework. It also
removes a build-time option that seems unnecessary.

Bruce Richardson (2):
  net/i40e: eliminate mbuf write on rearm
  net/i40e: remove option to disable offload flags

 config/common_base                      |  1 -
 doc/guides/nics/i40e.rst                |  5 ----
 drivers/net/i40e/i40e_rxtx_vec_common.h |  8 ------
 drivers/net/i40e/i40e_rxtx_vec_neon.c   | 11 --------
 drivers/net/i40e/i40e_rxtx_vec_sse.c    | 50 ++++++++++++++++-----------------
 5 files changed, 24 insertions(+), 51 deletions(-)

-- 
2.9.3

[PATCH 1/2] net/i40e: eliminate mbuf write on rearm

[Bruce Richardson]

With the mbuf rework, we now have 8 contiguous bytes to be rearmed in the
mbuf just before the 8-bytes of olflags. If we don't do the rearm write
inside the descriptor ring replenishment function, and delay it to
receiving the packet, we can do a single 16B write inside the RX function
to set both the rearm data, and the flags together.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/net/i40e/i40e_rxtx_vec_sse.c | 47 +++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/drivers/net/i40e/i40e_rxtx_vec_sse.c b/drivers/net/i40e/i40e_rxtx_vec_sse.c
index e17235a..09a33c3 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_sse.c
+++ b/drivers/net/i40e/i40e_rxtx_vec_sse.c
@@ -82,19 +82,10 @@ i40e_rxq_rearm(struct i40e_rx_queue *rxq)
 	/* Initialize the mbufs in vector, process 2 mbufs in one loop */
 	for (i = 0; i < RTE_I40E_RXQ_REARM_THRESH; i += 2, rxep += 2) {
 		__m128i vaddr0, vaddr1;
-		uintptr_t p0, p1;
 
 		mb0 = rxep[0].mbuf;
 		mb1 = rxep[1].mbuf;
 
-		/* Flush mbuf with pkt template.
-		 * Data to be rearmed is 6 bytes long.
-		 */
-		p0 = (uintptr_t)&mb0->rearm_data;
-		*(uint64_t *)p0 = rxq->mbuf_initializer;
-		p1 = (uintptr_t)&mb1->rearm_data;
-		*(uint64_t *)p1 = rxq->mbuf_initializer;
-
 		/* load buf_addr(lo 64bit) and buf_physaddr(hi 64bit) */
 		vaddr0 = _mm_loadu_si128((__m128i *)&mb0->buf_addr);
 		vaddr1 = _mm_loadu_si128((__m128i *)&mb1->buf_addr);
@@ -125,6 +116,14 @@ i40e_rxq_rearm(struct i40e_rx_queue *rxq)
 	I40E_PCI_REG_WRITE(rxq->qrx_tail, rx_id);
 }
 
+static inline void
+desc_to_olflags_v(struct i40e_rx_queue *rxq, __m128i descs[4] __rte_unused,
+	struct rte_mbuf **rx_pkts)
+{
+
+	const __m128i mbuf_init = _mm_set_epi64x(0, rxq->mbuf_initializer);
+	__m128i rearm0, rearm1, rearm2, rearm3;
+
 /* Handling the offload flags (olflags) field takes computation
  * time when receiving packets. Therefore we provide a flag to disable
  * the processing of the olflags field when they are not needed. This
@@ -133,9 +132,6 @@ i40e_rxq_rearm(struct i40e_rx_queue *rxq)
  */
 #ifdef RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE
 
-static inline void
-desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
-{
 	__m128i vlan0, vlan1, rss, l3_l4e;
 
 	/* mask everything except RSS, flow director and VLAN flags
@@ -203,14 +199,27 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
 	vlan0 = _mm_or_si128(vlan0, rss);
 	vlan0 = _mm_or_si128(vlan0, l3_l4e);
 
-	rx_pkts[0]->ol_flags = _mm_extract_epi16(vlan0, 0);
-	rx_pkts[1]->ol_flags = _mm_extract_epi16(vlan0, 2);
-	rx_pkts[2]->ol_flags = _mm_extract_epi16(vlan0, 4);
-	rx_pkts[3]->ol_flags = _mm_extract_epi16(vlan0, 6);
-}
+	/*
+	 * At this point, we have the 4 sets of flags in the low 16-bits
+	 * of each 32-bit value in vlan0.
+	 * We want to extract these, and merge them with the mbuf init data
+	 * so we can do a single 16-byte write to the mbuf to set the flags
+	 * and all the other initialization fields. Extracting the
+	 * appropriate flags means that we have to do a shift and blend for
+	 * each mbuf before we do the write.
+	 */
+	rearm0 = _mm_blend_epi16(mbuf_init, _mm_slli_si128(vlan0, 8), 0x10);
+	rearm1 = _mm_blend_epi16(mbuf_init, _mm_slli_si128(vlan0, 4), 0x10);
+	rearm2 = _mm_blend_epi16(mbuf_init, vlan0, 0x10);
+	rearm3 = _mm_blend_epi16(mbuf_init, _mm_srli_si128(vlan0, 4), 0x10);
 #else
-#define desc_to_olflags_v(desc, rx_pkts) do {} while (0)
+	rearm0 = rearm1 = rearm2 = rearm3 = mbuf_init;
 #endif
+	_mm_store_si128((__m128i *)&rx_pkts[0]->rearm_data, rearm0);
+	_mm_store_si128((__m128i *)&rx_pkts[1]->rearm_data, rearm1);
+	_mm_store_si128((__m128i *)&rx_pkts[2]->rearm_data, rearm2);
+	_mm_store_si128((__m128i *)&rx_pkts[3]->rearm_data, rearm3);
+}
 
 #define PKTLEN_SHIFT     10
 
@@ -369,7 +378,7 @@ _recv_raw_pkts_vec(struct i40e_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 		/* C.1 4=>2 filter staterr info only */
 		sterr_tmp1 = _mm_unpackhi_epi32(descs[1], descs[0]);
 
-		desc_to_olflags_v(descs, &rx_pkts[pos]);
+		desc_to_olflags_v(rxq, descs, &rx_pkts[pos]);
 
 		/* D.2 pkt 3,4 set in_port/nb_seg and remove crc */
 		pkt_mb4 = _mm_add_epi16(pkt_mb4, crc_adjust);
-- 
2.9.3

[PATCH 2/2] net/i40e: remove option to disable offload flags

[Bruce Richardson]

Having packets received without any offload flags given in the mbuf is not
very useful, and performance tests with testpmd indicates little to no
benefit is got with the current code by turning off the flags. This makes
the build-time option pointless, so we can remove it.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 config/common_base                      |  1 -
 doc/guides/nics/i40e.rst                |  5 -----
 drivers/net/i40e/i40e_rxtx_vec_common.h |  8 --------
 drivers/net/i40e/i40e_rxtx_vec_neon.c   | 11 -----------
 drivers/net/i40e/i40e_rxtx_vec_sse.c    | 11 -----------
 5 files changed, 36 deletions(-)

diff --git a/config/common_base b/config/common_base
index 38e9483..e898f51 100644
--- a/config/common_base
+++ b/config/common_base
@@ -186,7 +186,6 @@ CONFIG_RTE_LIBRTE_I40E_DEBUG_TX_FREE=n
 CONFIG_RTE_LIBRTE_I40E_DEBUG_DRIVER=n
 CONFIG_RTE_LIBRTE_I40E_RX_ALLOW_BULK_ALLOC=y
 CONFIG_RTE_LIBRTE_I40E_INC_VECTOR=y
-CONFIG_RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE=y
 CONFIG_RTE_LIBRTE_I40E_16BYTE_RX_DESC=n
 CONFIG_RTE_LIBRTE_I40E_QUEUE_NUM_PER_PF=64
 CONFIG_RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF=4
diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst
index 043de34..1daf63d 100644
--- a/doc/guides/nics/i40e.rst
+++ b/doc/guides/nics/i40e.rst
@@ -105,11 +105,6 @@ Please note that enabling debugging options may affect system performance.
   Toggle the use of Vector PMD instead of normal RX/TX path.
   To enable vPMD for RX, bulk allocation for Rx must be allowed.
 
-- ``CONFIG_RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE`` (default ``y``)
-
-  Toggle to enable RX ``olflags``.
-  This is only meaningful when Vector PMD is used.
-
 - ``CONFIG_RTE_LIBRTE_I40E_16BYTE_RX_DESC`` (default ``n``)
 
   Toggle to use a 16-byte RX descriptor, by default the RX descriptor is 32 byte.
diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h b/drivers/net/i40e/i40e_rxtx_vec_common.h
index 76031fe..952fd4b 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_common.h
+++ b/drivers/net/i40e/i40e_rxtx_vec_common.h
@@ -224,14 +224,6 @@ i40e_rx_vec_dev_conf_condition_check_default(struct rte_eth_dev *dev)
 	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
 	struct rte_fdir_conf *fconf = &dev->data->dev_conf.fdir_conf;
 
-#ifndef RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE
-	/* whithout rx ol_flags, no VP flag report */
-	if (rxmode->hw_vlan_strip != 0 ||
-	    rxmode->hw_vlan_extend != 0 ||
-	    rxmode->hw_ip_checksum != 0)
-		return -1;
-#endif
-
 	/* no fdir support */
 	if (fconf->mode != RTE_FDIR_MODE_NONE)
 		return -1;
diff --git a/drivers/net/i40e/i40e_rxtx_vec_neon.c b/drivers/net/i40e/i40e_rxtx_vec_neon.c
index 011c54e..adfd3e5 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_neon.c
+++ b/drivers/net/i40e/i40e_rxtx_vec_neon.c
@@ -116,14 +116,6 @@ i40e_rxq_rearm(struct i40e_rx_queue *rxq)
 	I40E_PCI_REG_WRITE(rxq->qrx_tail, rx_id);
 }
 
-/* Handling the offload flags (olflags) field takes computation
- * time when receiving packets. Therefore we provide a flag to disable
- * the processing of the olflags field when they are not needed. This
- * gives improved performance, at the cost of losing the offload info
- * in the received packet
- */
-#ifdef RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE
-
 static inline void
 desc_to_olflags_v(uint64x2_t descs[4], struct rte_mbuf **rx_pkts)
 {
@@ -187,9 +179,6 @@ desc_to_olflags_v(uint64x2_t descs[4], struct rte_mbuf **rx_pkts)
 	rx_pkts[2]->ol_flags = vgetq_lane_u32(vlan0, 2);
 	rx_pkts[3]->ol_flags = vgetq_lane_u32(vlan0, 3);
 }
-#else
-#define desc_to_olflags_v(descs, rx_pkts) do {} while (0)
-#endif
 
 #define PKTLEN_SHIFT     10
 
diff --git a/drivers/net/i40e/i40e_rxtx_vec_sse.c b/drivers/net/i40e/i40e_rxtx_vec_sse.c
index 09a33c3..ace5bd7 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_sse.c
+++ b/drivers/net/i40e/i40e_rxtx_vec_sse.c
@@ -124,14 +124,6 @@ desc_to_olflags_v(struct i40e_rx_queue *rxq, __m128i descs[4] __rte_unused,
 	const __m128i mbuf_init = _mm_set_epi64x(0, rxq->mbuf_initializer);
 	__m128i rearm0, rearm1, rearm2, rearm3;
 
-/* Handling the offload flags (olflags) field takes computation
- * time when receiving packets. Therefore we provide a flag to disable
- * the processing of the olflags field when they are not needed. This
- * gives improved performance, at the cost of losing the offload info
- * in the received packet
- */
-#ifdef RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE
-
 	__m128i vlan0, vlan1, rss, l3_l4e;
 
 	/* mask everything except RSS, flow director and VLAN flags
@@ -212,9 +204,6 @@ desc_to_olflags_v(struct i40e_rx_queue *rxq, __m128i descs[4] __rte_unused,
 	rearm1 = _mm_blend_epi16(mbuf_init, _mm_slli_si128(vlan0, 4), 0x10);
 	rearm2 = _mm_blend_epi16(mbuf_init, vlan0, 0x10);
 	rearm3 = _mm_blend_epi16(mbuf_init, _mm_srli_si128(vlan0, 4), 0x10);
-#else
-	rearm0 = rearm1 = rearm2 = rearm3 = mbuf_init;
-#endif
 	_mm_store_si128((__m128i *)&rx_pkts[0]->rearm_data, rearm0);
 	_mm_store_si128((__m128i *)&rx_pkts[1]->rearm_data, rearm1);
 	_mm_store_si128((__m128i *)&rx_pkts[2]->rearm_data, rearm2);
-- 
2.9.3

Re: [PATCH 2/2] net/i40e: remove option to disable offload flags

[Ferruh Yigit]

On 4/3/2017 3:39 PM, Bruce Richardson wrote:
> Having packets received without any offload flags given in the mbuf is not
> very useful, and performance tests with testpmd indicates little to no
> benefit is got with the current code by turning off the flags. This makes
> the build-time option pointless, so we can remove it.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  config/common_base                      |  1 -
>  doc/guides/nics/i40e.rst                |  5 -----
>  drivers/net/i40e/i40e_rxtx_vec_common.h |  8 --------
>  drivers/net/i40e/i40e_rxtx_vec_neon.c   | 11 -----------
>  drivers/net/i40e/i40e_rxtx_vec_sse.c    | 11 -----------

Should this patch also update powerpc vector PMD (i40e_rxtx_vec_altivec.c) ?

Thanks,
ferruh

Re: [PATCH 2/2] net/i40e: remove option to disable offload flags

[Bruce Richardson]

On Mon, Apr 03, 2017 at 03:45:46PM +0100, Ferruh Yigit wrote:
> On 4/3/2017 3:39 PM, Bruce Richardson wrote:
> > Having packets received without any offload flags given in the mbuf is not
> > very useful, and performance tests with testpmd indicates little to no
> > benefit is got with the current code by turning off the flags. This makes
> > the build-time option pointless, so we can remove it.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  config/common_base                      |  1 -
> >  doc/guides/nics/i40e.rst                |  5 -----
> >  drivers/net/i40e/i40e_rxtx_vec_common.h |  8 --------
> >  drivers/net/i40e/i40e_rxtx_vec_neon.c   | 11 -----------
> >  drivers/net/i40e/i40e_rxtx_vec_sse.c    | 11 -----------
> 
> Should this patch also update powerpc vector PMD (i40e_rxtx_vec_altivec.c) ?
> 
Yes, good catch. I based this work off the main tree rather than
next-net as I was looking at the mbuf changes primarily. Once we get
either next-net or the mbuf patches merged in, I'll do a V2 with all
updates included.

/Bruce

Re: [PATCH 0/2] reduce writes in i40e driver

[Jianbo Liu]

On 3 April 2017 at 22:39, Bruce Richardson <bruce.richardson@intel.com> wrote:
> this set is based upon Olivier's mbuf rework patchset, and makes some
> improvement to the i40e driver taking account of the rework. It also
> removes a build-time option that seems unnecessary.
>
> Bruce Richardson (2):
>   net/i40e: eliminate mbuf write on rearm
>   net/i40e: remove option to disable offload flags
>
>  config/common_base                      |  1 -
>  doc/guides/nics/i40e.rst                |  5 ----
>  drivers/net/i40e/i40e_rxtx_vec_common.h |  8 ------
>  drivers/net/i40e/i40e_rxtx_vec_neon.c   | 11 --------
>  drivers/net/i40e/i40e_rxtx_vec_sse.c    | 50 ++++++++++++++++-----------------
>  5 files changed, 24 insertions(+), 51 deletions(-)
>
> --
> 2.9.3
>

Re: [PATCH 0/2] reduce writes in i40e driver

[Jianbo Liu]

On 3 April 2017 at 22:39, Bruce Richardson <bruce.richardson@intel.com> wrote:
> this set is based upon Olivier's mbuf rework patchset, and makes some
> improvement to the i40e driver taking account of the rework. It also
> removes a build-time option that seems unnecessary.
>
> Bruce Richardson (2):
>   net/i40e: eliminate mbuf write on rearm
>   net/i40e: remove option to disable offload flags
>
>  config/common_base                      |  1 -
>  doc/guides/nics/i40e.rst                |  5 ----
>  drivers/net/i40e/i40e_rxtx_vec_common.h |  8 ------
>  drivers/net/i40e/i40e_rxtx_vec_neon.c   | 11 --------
>  drivers/net/i40e/i40e_rxtx_vec_sse.c    | 50 ++++++++++++++++-----------------
>  5 files changed, 24 insertions(+), 51 deletions(-)

Acked-by: Jianbo Liu <jianbo.liu@linaro.org>

And I'll send a patch to do the same change for i40e neon implementation.

[PATCH v2 0/2] reduce writes in i40e driver

[Bruce Richardson]

this set is based upon Olivier's mbuf rework patchset, and makes some
improvement to the i40e driver taking account of the rework. It also
removes a build-time option that seems unnecessary.

V2: Eliminate one checkpatch warning, and remove #ifdef block from
    new altivec driver code.

Bruce Richardson (2):
  net/i40e: eliminate mbuf write on rearm
  net/i40e: remove option to disable offload flags

 config/common_base                       |  1 -
 doc/guides/nics/i40e.rst                 |  5 ----
 drivers/net/i40e/i40e_rxtx_vec_altivec.c | 11 -------
 drivers/net/i40e/i40e_rxtx_vec_common.h  |  8 ------
 drivers/net/i40e/i40e_rxtx_vec_neon.c    | 11 -------
 drivers/net/i40e/i40e_rxtx_vec_sse.c     | 49 +++++++++++++++-----------------
 6 files changed, 23 insertions(+), 62 deletions(-)

-- 
2.9.3

[PATCH v2 1/2] net/i40e: eliminate mbuf write on rearm

[Bruce Richardson]

With the mbuf rework, we now have 8 contiguous bytes to be rearmed in the
mbuf just before the 8-bytes of olflags. If we don't do the rearm write
inside the descriptor ring replenishment function, and delay it to
receiving the packet, we can do a single 16B write inside the RX function
to set both the rearm data, and the flags together.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
V2: fix a checkpatch warning. One warning remains, which is being left
    as-is as the code line in question is being removed by patch 2.
---
 drivers/net/i40e/i40e_rxtx_vec_sse.c | 46 +++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/drivers/net/i40e/i40e_rxtx_vec_sse.c b/drivers/net/i40e/i40e_rxtx_vec_sse.c
index fdd4a34..c43d1c3 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_sse.c
+++ b/drivers/net/i40e/i40e_rxtx_vec_sse.c
@@ -82,19 +82,10 @@ i40e_rxq_rearm(struct i40e_rx_queue *rxq)
 	/* Initialize the mbufs in vector, process 2 mbufs in one loop */
 	for (i = 0; i < RTE_I40E_RXQ_REARM_THRESH; i += 2, rxep += 2) {
 		__m128i vaddr0, vaddr1;
-		uintptr_t p0, p1;
 
 		mb0 = rxep[0].mbuf;
 		mb1 = rxep[1].mbuf;
 
-		/* Flush mbuf with pkt template.
-		 * Data to be rearmed is 6 bytes long.
-		 */
-		p0 = (uintptr_t)&mb0->rearm_data;
-		*(uint64_t *)p0 = rxq->mbuf_initializer;
-		p1 = (uintptr_t)&mb1->rearm_data;
-		*(uint64_t *)p1 = rxq->mbuf_initializer;
-
 		/* load buf_addr(lo 64bit) and buf_physaddr(hi 64bit) */
 		vaddr0 = _mm_loadu_si128((__m128i *)&mb0->buf_addr);
 		vaddr1 = _mm_loadu_si128((__m128i *)&mb1->buf_addr);
@@ -125,6 +116,13 @@ i40e_rxq_rearm(struct i40e_rx_queue *rxq)
 	I40E_PCI_REG_WRITE(rxq->qrx_tail, rx_id);
 }
 
+static inline void
+desc_to_olflags_v(struct i40e_rx_queue *rxq, __m128i descs[4] __rte_unused,
+	struct rte_mbuf **rx_pkts)
+{
+	const __m128i mbuf_init = _mm_set_epi64x(0, rxq->mbuf_initializer);
+	__m128i rearm0, rearm1, rearm2, rearm3;
+
 /* Handling the offload flags (olflags) field takes computation
  * time when receiving packets. Therefore we provide a flag to disable
  * the processing of the olflags field when they are not needed. This
@@ -133,9 +131,6 @@ i40e_rxq_rearm(struct i40e_rx_queue *rxq)
  */
 #ifdef RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE
 
-static inline void
-desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
-{
 	__m128i vlan0, vlan1, rss, l3_l4e;
 
 	/* mask everything except RSS, flow director and VLAN flags
@@ -203,14 +198,27 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
 	vlan0 = _mm_or_si128(vlan0, rss);
 	vlan0 = _mm_or_si128(vlan0, l3_l4e);
 
-	rx_pkts[0]->ol_flags = _mm_extract_epi16(vlan0, 0);
-	rx_pkts[1]->ol_flags = _mm_extract_epi16(vlan0, 2);
-	rx_pkts[2]->ol_flags = _mm_extract_epi16(vlan0, 4);
-	rx_pkts[3]->ol_flags = _mm_extract_epi16(vlan0, 6);
-}
+	/*
+	 * At this point, we have the 4 sets of flags in the low 16-bits
+	 * of each 32-bit value in vlan0.
+	 * We want to extract these, and merge them with the mbuf init data
+	 * so we can do a single 16-byte write to the mbuf to set the flags
+	 * and all the other initialization fields. Extracting the
+	 * appropriate flags means that we have to do a shift and blend for
+	 * each mbuf before we do the write.
+	 */
+	rearm0 = _mm_blend_epi16(mbuf_init, _mm_slli_si128(vlan0, 8), 0x10);
+	rearm1 = _mm_blend_epi16(mbuf_init, _mm_slli_si128(vlan0, 4), 0x10);
+	rearm2 = _mm_blend_epi16(mbuf_init, vlan0, 0x10);
+	rearm3 = _mm_blend_epi16(mbuf_init, _mm_srli_si128(vlan0, 4), 0x10);
 #else
-#define desc_to_olflags_v(desc, rx_pkts) do {} while (0)
+	rearm0 = rearm1 = rearm2 = rearm3 = mbuf_init;
 #endif
+	_mm_store_si128((__m128i *)&rx_pkts[0]->rearm_data, rearm0);
+	_mm_store_si128((__m128i *)&rx_pkts[1]->rearm_data, rearm1);
+	_mm_store_si128((__m128i *)&rx_pkts[2]->rearm_data, rearm2);
+	_mm_store_si128((__m128i *)&rx_pkts[3]->rearm_data, rearm3);
+}
 
 #define PKTLEN_SHIFT     10
 
@@ -369,7 +377,7 @@ _recv_raw_pkts_vec(struct i40e_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 		/* C.1 4=>2 filter staterr info only */
 		sterr_tmp1 = _mm_unpackhi_epi32(descs[1], descs[0]);
 
-		desc_to_olflags_v(descs, &rx_pkts[pos]);
+		desc_to_olflags_v(rxq, descs, &rx_pkts[pos]);
 
 		/* D.2 pkt 3,4 set in_port/nb_seg and remove crc */
 		pkt_mb4 = _mm_add_epi16(pkt_mb4, crc_adjust);
-- 
2.9.3

[PATCH v2 2/2] net/i40e: remove option to disable offload flags

[Bruce Richardson]

Having packets received without any offload flags given in the mbuf is not
very useful, and performance tests with testpmd indicates little to no
benefit is got with the current code by turning off the flags. This makes
the build-time option pointless, so we can remove it.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
V2: include changes to alitvec as well as neon and sse files
---
 config/common_base                       |  1 -
 doc/guides/nics/i40e.rst                 |  5 -----
 drivers/net/i40e/i40e_rxtx_vec_altivec.c | 11 -----------
 drivers/net/i40e/i40e_rxtx_vec_common.h  |  8 --------
 drivers/net/i40e/i40e_rxtx_vec_neon.c    | 11 -----------
 drivers/net/i40e/i40e_rxtx_vec_sse.c     | 11 -----------
 6 files changed, 47 deletions(-)

diff --git a/config/common_base b/config/common_base
index 7d7290c..434d406 100644
--- a/config/common_base
+++ b/config/common_base
@@ -184,7 +184,6 @@ CONFIG_RTE_LIBRTE_I40E_DEBUG_TX=n
 CONFIG_RTE_LIBRTE_I40E_DEBUG_TX_FREE=n
 CONFIG_RTE_LIBRTE_I40E_RX_ALLOW_BULK_ALLOC=y
 CONFIG_RTE_LIBRTE_I40E_INC_VECTOR=y
-CONFIG_RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE=y
 CONFIG_RTE_LIBRTE_I40E_16BYTE_RX_DESC=n
 CONFIG_RTE_LIBRTE_I40E_QUEUE_NUM_PER_PF=64
 CONFIG_RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF=4
diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst
index 6ae1d46..d5efa3f 100644
--- a/doc/guides/nics/i40e.rst
+++ b/doc/guides/nics/i40e.rst
@@ -105,11 +105,6 @@ Please note that enabling debugging options may affect system performance.
   Toggle the use of Vector PMD instead of normal RX/TX path.
   To enable vPMD for RX, bulk allocation for Rx must be allowed.
 
-- ``CONFIG_RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE`` (default ``y``)
-
-  Toggle to enable RX ``olflags``.
-  This is only meaningful when Vector PMD is used.
-
 - ``CONFIG_RTE_LIBRTE_I40E_16BYTE_RX_DESC`` (default ``n``)
 
   Toggle to use a 16-byte RX descriptor, by default the RX descriptor is 32 byte.
diff --git a/drivers/net/i40e/i40e_rxtx_vec_altivec.c b/drivers/net/i40e/i40e_rxtx_vec_altivec.c
index 2f6f70a..9831438 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_altivec.c
+++ b/drivers/net/i40e/i40e_rxtx_vec_altivec.c
@@ -130,14 +130,6 @@ i40e_rxq_rearm(struct i40e_rx_queue *rxq)
 	I40E_PCI_REG_WRITE(rxq->qrx_tail, rx_id);
 }
 
-/* Handling the offload flags (olflags) field takes computation
- * time when receiving packets. Therefore we provide a flag to disable
- * the processing of the olflags field when they are not needed. This
- * gives improved performance, at the cost of losing the offload info
- * in the received packet
- */
-#ifdef RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE
-
 static inline void
 desc_to_olflags_v(vector unsigned long descs[4], struct rte_mbuf **rx_pkts)
 {
@@ -202,9 +194,6 @@ desc_to_olflags_v(vector unsigned long descs[4], struct rte_mbuf **rx_pkts)
 	rx_pkts[2]->ol_flags = (uint64_t)vlan0[0];
 	rx_pkts[3]->ol_flags = (uint64_t)vlan0[1];
 }
-#else
-#define desc_to_olflags_v(desc, rx_pkts) do {} while (0)
-#endif
 
 #define PKTLEN_SHIFT     10
 
diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h b/drivers/net/i40e/i40e_rxtx_vec_common.h
index 76031fe..952fd4b 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_common.h
+++ b/drivers/net/i40e/i40e_rxtx_vec_common.h
@@ -224,14 +224,6 @@ i40e_rx_vec_dev_conf_condition_check_default(struct rte_eth_dev *dev)
 	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
 	struct rte_fdir_conf *fconf = &dev->data->dev_conf.fdir_conf;
 
-#ifndef RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE
-	/* whithout rx ol_flags, no VP flag report */
-	if (rxmode->hw_vlan_strip != 0 ||
-	    rxmode->hw_vlan_extend != 0 ||
-	    rxmode->hw_ip_checksum != 0)
-		return -1;
-#endif
-
 	/* no fdir support */
 	if (fconf->mode != RTE_FDIR_MODE_NONE)
 		return -1;
diff --git a/drivers/net/i40e/i40e_rxtx_vec_neon.c b/drivers/net/i40e/i40e_rxtx_vec_neon.c
index bd7239b..902fb1f 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_neon.c
+++ b/drivers/net/i40e/i40e_rxtx_vec_neon.c
@@ -116,14 +116,6 @@ i40e_rxq_rearm(struct i40e_rx_queue *rxq)
 	I40E_PCI_REG_WRITE(rxq->qrx_tail, rx_id);
 }
 
-/* Handling the offload flags (olflags) field takes computation
- * time when receiving packets. Therefore we provide a flag to disable
- * the processing of the olflags field when they are not needed. This
- * gives improved performance, at the cost of losing the offload info
- * in the received packet
- */
-#ifdef RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE
-
 static inline void
 desc_to_olflags_v(uint64x2_t descs[4], struct rte_mbuf **rx_pkts)
 {
@@ -187,9 +179,6 @@ desc_to_olflags_v(uint64x2_t descs[4], struct rte_mbuf **rx_pkts)
 	rx_pkts[2]->ol_flags = vgetq_lane_u32(vlan0, 2);
 	rx_pkts[3]->ol_flags = vgetq_lane_u32(vlan0, 3);
 }
-#else
-#define desc_to_olflags_v(descs, rx_pkts) do {} while (0)
-#endif
 
 #define PKTLEN_SHIFT     10
 
diff --git a/drivers/net/i40e/i40e_rxtx_vec_sse.c b/drivers/net/i40e/i40e_rxtx_vec_sse.c
index c43d1c3..f91e32d 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_sse.c
+++ b/drivers/net/i40e/i40e_rxtx_vec_sse.c
@@ -123,14 +123,6 @@ desc_to_olflags_v(struct i40e_rx_queue *rxq, __m128i descs[4] __rte_unused,
 	const __m128i mbuf_init = _mm_set_epi64x(0, rxq->mbuf_initializer);
 	__m128i rearm0, rearm1, rearm2, rearm3;
 
-/* Handling the offload flags (olflags) field takes computation
- * time when receiving packets. Therefore we provide a flag to disable
- * the processing of the olflags field when they are not needed. This
- * gives improved performance, at the cost of losing the offload info
- * in the received packet
- */
-#ifdef RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE
-
 	__m128i vlan0, vlan1, rss, l3_l4e;
 
 	/* mask everything except RSS, flow director and VLAN flags
@@ -211,9 +203,6 @@ desc_to_olflags_v(struct i40e_rx_queue *rxq, __m128i descs[4] __rte_unused,
 	rearm1 = _mm_blend_epi16(mbuf_init, _mm_slli_si128(vlan0, 4), 0x10);
 	rearm2 = _mm_blend_epi16(mbuf_init, vlan0, 0x10);
 	rearm3 = _mm_blend_epi16(mbuf_init, _mm_srli_si128(vlan0, 4), 0x10);
-#else
-	rearm0 = rearm1 = rearm2 = rearm3 = mbuf_init;
-#endif
 	_mm_store_si128((__m128i *)&rx_pkts[0]->rearm_data, rearm0);
 	_mm_store_si128((__m128i *)&rx_pkts[1]->rearm_data, rearm1);
 	_mm_store_si128((__m128i *)&rx_pkts[2]->rearm_data, rearm2);
-- 
2.9.3

Re: [PATCH v2 1/2] net/i40e: eliminate mbuf write on rearm

[Zhang, Qi Z]

Hi Bruce:

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> Sent: Thursday, April 6, 2017 7:32 PM
> To: Zhang, Helin <helin.zhang@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>
> Subject: [dpdk-dev] [PATCH v2 1/2] net/i40e: eliminate mbuf write on rearm
> 
> With the mbuf rework, we now have 8 contiguous bytes to be rearmed in the
> mbuf just before the 8-bytes of olflags. If we don't do the rearm write inside
> the descriptor ring replenishment function, and delay it to receiving the
> packet, we can do a single 16B write inside the RX function to set both the
> rearm data, and the flags together.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> V2: fix a checkpatch warning. One warning remains, which is being left
>     as-is as the code line in question is being removed by patch 2.
> ---
>  drivers/net/i40e/i40e_rxtx_vec_sse.c | 46
> +++++++++++++++++++++---------------
>  1 file changed, 27 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_rxtx_vec_sse.c
> b/drivers/net/i40e/i40e_rxtx_vec_sse.c
> index fdd4a34..c43d1c3 100644
> --- a/drivers/net/i40e/i40e_rxtx_vec_sse.c
> +++ b/drivers/net/i40e/i40e_rxtx_vec_sse.c
> @@ -82,19 +82,10 @@ i40e_rxq_rearm(struct i40e_rx_queue *rxq)
>  	/* Initialize the mbufs in vector, process 2 mbufs in one loop */
>  	for (i = 0; i < RTE_I40E_RXQ_REARM_THRESH; i += 2, rxep += 2) {
>  		__m128i vaddr0, vaddr1;
> -		uintptr_t p0, p1;
> 
>  		mb0 = rxep[0].mbuf;
>  		mb1 = rxep[1].mbuf;
> 
> -		/* Flush mbuf with pkt template.
> -		 * Data to be rearmed is 6 bytes long.
> -		 */
> -		p0 = (uintptr_t)&mb0->rearm_data;
> -		*(uint64_t *)p0 = rxq->mbuf_initializer;
> -		p1 = (uintptr_t)&mb1->rearm_data;
> -		*(uint64_t *)p1 = rxq->mbuf_initializer;
> -
>  		/* load buf_addr(lo 64bit) and buf_physaddr(hi 64bit) */
>  		vaddr0 = _mm_loadu_si128((__m128i *)&mb0->buf_addr);
>  		vaddr1 = _mm_loadu_si128((__m128i *)&mb1->buf_addr); @@
> -125,6 +116,13 @@ i40e_rxq_rearm(struct i40e_rx_queue *rxq)
>  	I40E_PCI_REG_WRITE(rxq->qrx_tail, rx_id);  }
> 
> +static inline void
> +desc_to_olflags_v(struct i40e_rx_queue *rxq, __m128i descs[4]
> __rte_unused,
> +	struct rte_mbuf **rx_pkts)


Should we change the function name? since its scope is changed.
I'm Ok with all the other part.

Thanks
Qi

Re: [PATCH v2 1/2] net/i40e: eliminate mbuf write on rearm

[Bruce Richardson]

On Thu, Apr 06, 2017 at 05:24:44PM +0100, Zhang, Qi Z wrote:
> Hi Bruce:
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> > Sent: Thursday, April 6, 2017 7:32 PM
> > To: Zhang, Helin <helin.zhang@intel.com>
> > Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; Richardson, Bruce
> > <bruce.richardson@intel.com>
> > Subject: [dpdk-dev] [PATCH v2 1/2] net/i40e: eliminate mbuf write on rearm
> > 
> > With the mbuf rework, we now have 8 contiguous bytes to be rearmed in the
> > mbuf just before the 8-bytes of olflags. If we don't do the rearm write inside
> > the descriptor ring replenishment function, and delay it to receiving the
> > packet, we can do a single 16B write inside the RX function to set both the
> > rearm data, and the flags together.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> > V2: fix a checkpatch warning. One warning remains, which is being left
> >     as-is as the code line in question is being removed by patch 2.
> > ---
> >  drivers/net/i40e/i40e_rxtx_vec_sse.c | 46
> > +++++++++++++++++++++---------------
> >  1 file changed, 27 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/net/i40e/i40e_rxtx_vec_sse.c
> > b/drivers/net/i40e/i40e_rxtx_vec_sse.c
> > index fdd4a34..c43d1c3 100644
> > --- a/drivers/net/i40e/i40e_rxtx_vec_sse.c
> > +++ b/drivers/net/i40e/i40e_rxtx_vec_sse.c
> > @@ -82,19 +82,10 @@ i40e_rxq_rearm(struct i40e_rx_queue *rxq)
> >  	/* Initialize the mbufs in vector, process 2 mbufs in one loop */
> >  	for (i = 0; i < RTE_I40E_RXQ_REARM_THRESH; i += 2, rxep += 2) {
> >  		__m128i vaddr0, vaddr1;
> > -		uintptr_t p0, p1;
> > 
> >  		mb0 = rxep[0].mbuf;
> >  		mb1 = rxep[1].mbuf;
> > 
> > -		/* Flush mbuf with pkt template.
> > -		 * Data to be rearmed is 6 bytes long.
> > -		 */
> > -		p0 = (uintptr_t)&mb0->rearm_data;
> > -		*(uint64_t *)p0 = rxq->mbuf_initializer;
> > -		p1 = (uintptr_t)&mb1->rearm_data;
> > -		*(uint64_t *)p1 = rxq->mbuf_initializer;
> > -
> >  		/* load buf_addr(lo 64bit) and buf_physaddr(hi 64bit) */
> >  		vaddr0 = _mm_loadu_si128((__m128i *)&mb0->buf_addr);
> >  		vaddr1 = _mm_loadu_si128((__m128i *)&mb1->buf_addr); @@
> > -125,6 +116,13 @@ i40e_rxq_rearm(struct i40e_rx_queue *rxq)
> >  	I40E_PCI_REG_WRITE(rxq->qrx_tail, rx_id);  }
> > 
> > +static inline void
> > +desc_to_olflags_v(struct i40e_rx_queue *rxq, __m128i descs[4]
> > __rte_unused,
> > +	struct rte_mbuf **rx_pkts)
> 
> 
> Should we change the function name? since its scope is changed.
> I'm Ok with all the other part.
> 
> Thanks
> Qi
>
Yes, it perhaps should, though it's not a big deal IMHO. Alternatively,
the function should be changed to return the flags values as an output
and we do the writes themselves in the main rx function.

/Bruce

Re: [PATCH v2 0/2] reduce writes in i40e driver

[Ferruh Yigit]

On 4/6/2017 12:32 PM, Bruce Richardson wrote:
> this set is based upon Olivier's mbuf rework patchset, and makes some
> improvement to the i40e driver taking account of the rework. It also
> removes a build-time option that seems unnecessary.
> 
> V2: Eliminate one checkpatch warning, and remove #ifdef block from
>     new altivec driver code.
> 
> Bruce Richardson (2):
>   net/i40e: eliminate mbuf write on rearm
>   net/i40e: remove option to disable offload flags

Acked-by: Jianbo Liu <jianbo.liu@linaro.org>

Series applied to dpdk-next-net/master, thanks.