[dpdk-dev,v2,2/2] i40e: Enable bad checksum flags in i40e vPMD

Message ID 1475712772-105327-2-git-send-email-jeffrey.b.shaw@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Jeff Shaw Oct. 6, 2016, 12:12 a.m. UTC
  From: Damjan Marion <damarion@cisco.com>

Decode the checksum flags from the rx descriptor, setting
the appropriate bit in the mbuf ol_flags field when the flag
indicates a bad checksum.

Signed-off-by: Damjan Marion <damarion@cisco.com>
Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
---
 drivers/net/i40e/i40e_rxtx_vec.c | 48 +++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 20 deletions(-)
  

Comments

Chen, Jing D Oct. 5, 2016, 10:20 p.m. UTC | #1
Hi, 

> -----Original Message-----
> From: Shaw, Jeffrey B
> Sent: Wednesday, October 5, 2016 5:13 PM
> To: dev@dpdk.org
> Cc: Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; damarion@cisco.com; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Chen, Jing D <jing.d.chen@intel.com>
> Subject: [PATCH v2 2/2] i40e: Enable bad checksum flags in i40e vPMD
> 
> From: Damjan Marion <damarion@cisco.com>
> 
> Decode the checksum flags from the rx descriptor, setting the appropriate bit
> in the mbuf ol_flags field when the flag indicates a bad checksum.
> 
> Signed-off-by: Damjan Marion <damarion@cisco.com>
> Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
Acked-by: Jing Chen <jing.d.chen@intel.com>

It seems this patch also fixed a vlan flag bug, should it explain a little bit?
  
Chen, Jing D Oct. 5, 2016, 11:57 p.m. UTC | #2
Hi,

> -----Original Message-----
> From: Shaw, Jeffrey B
> Sent: Wednesday, October 5, 2016 5:13 PM
> To: dev@dpdk.org
> Cc: Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; damarion@cisco.com; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Chen, Jing D <jing.d.chen@intel.com>
> Subject: [PATCH v2 2/2] i40e: Enable bad checksum flags in i40e vPMD
> 
> From: Damjan Marion <damarion@cisco.com>
> 
> Decode the checksum flags from the rx descriptor, setting the appropriate bit
> in the mbuf ol_flags field when the flag indicates a bad checksum.
> 
> Signed-off-by: Damjan Marion <damarion@cisco.com>
> Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
> ---
>  drivers/net/i40e/i40e_rxtx_vec.c | 48 +++++++++++++++++++++++---------------
> --
>  1 file changed, 28 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_rxtx_vec.c
> b/drivers/net/i40e/i40e_rxtx_vec.c
> index 6c63141..d2267ad 100644
> --- a/drivers/net/i40e/i40e_rxtx_vec.c
> +++ b/drivers/net/i40e/i40e_rxtx_vec.c
> @@ -138,19 +138,14 @@ i40e_rxq_rearm(struct i40e_rx_queue *rxq)  static
> inline void  desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)  {
> -	__m128i vlan0, vlan1, rss;
> -	union {
> -		uint16_t e[4];
> -		uint64_t dword;
> -	} vol;
> +	__m128i vlan0, vlan1, rss, l3_l4e;
> 
>  	/* mask everything except RSS, flow director and VLAN flags
>  	 * bit2 is for VLAN tag, bit11 for flow director indication
>  	 * bit13:12 for RSS indication.
>  	 */
> -	const __m128i rss_vlan_msk = _mm_set_epi16(
> -			0x0000, 0x0000, 0x0000, 0x0000,
> -			0x3804, 0x3804, 0x3804, 0x3804);
> +	const __m128i rss_vlan_msk = _mm_set_epi32(
> +			0x1c03004, 0x1c03004, 0x1c03004, 0x1c03004);
> 
>  	/* map rss and vlan type to rss hash and vlan flag */
>  	const __m128i vlan_flags = _mm_set_epi8(0, 0, 0, 0, @@ -163,23
> +158,36 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
>  			PKT_RX_RSS_HASH | PKT_RX_FDIR,
> PKT_RX_RSS_HASH, 0, 0,
>  			0, 0, PKT_RX_FDIR, 0);
> 
> -	vlan0 = _mm_unpackhi_epi16(descs[0], descs[1]);
> -	vlan1 = _mm_unpackhi_epi16(descs[2], descs[3]);
> -	vlan0 = _mm_unpacklo_epi32(vlan0, vlan1);
> +	const __m128i l3_l4e_flags = _mm_set_epi8(0, 0, 0, 0, 0, 0, 0, 0,
> +			PKT_RX_EIP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD
> | PKT_RX_IP_CKSUM_BAD,
> +			PKT_RX_EIP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD,
> +			PKT_RX_EIP_CKSUM_BAD | PKT_RX_IP_CKSUM_BAD,
> +			PKT_RX_EIP_CKSUM_BAD,
> +			PKT_RX_L4_CKSUM_BAD | PKT_RX_IP_CKSUM_BAD,
> +			PKT_RX_L4_CKSUM_BAD,
> +			PKT_RX_IP_CKSUM_BAD,
> +			0);
> +
> +	vlan0 = _mm_unpackhi_epi32(descs[0], descs[1]);
> +	vlan1 = _mm_unpackhi_epi32(descs[2], descs[3]);
> +	vlan0 = _mm_unpacklo_epi64(vlan0, vlan1);
> 
>  	vlan1 = _mm_and_si128(vlan0, rss_vlan_msk);
>  	vlan0 = _mm_shuffle_epi8(vlan_flags, vlan1);
> 
> -	rss = _mm_srli_epi16(vlan1, 11);
> +	rss = _mm_srli_epi32(vlan1, 12);
> 	rss = _mm_shuffle_epi8(rss_flags, rss);

My bad. Original code will use bit[13:11] to identify RSS and FDIR flag. Now 
It masked bit 11 out when creating " rss_vlan_msk" and doing shift above,
while it still try to use  original "rss_flags"?
  
Jeff Shaw Oct. 6, 2016, 2:19 a.m. UTC | #3
On Wed, Oct 05, 2016 at 04:57:28PM -0700, Chen, Jing D wrote:
> Hi,
> 
> > -----Original Message-----
> > From: Shaw, Jeffrey B
> > Sent: Wednesday, October 5, 2016 5:13 PM
> > To: dev@dpdk.org
> > Cc: Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing
> > <jingjing.wu@intel.com>; damarion@cisco.com; Zhang, Qi Z
> > <qi.z.zhang@intel.com>; Chen, Jing D <jing.d.chen@intel.com>
> > Subject: [PATCH v2 2/2] i40e: Enable bad checksum flags in i40e vPMD
> > 
> > From: Damjan Marion <damarion@cisco.com>
> > 
> > Decode the checksum flags from the rx descriptor, setting the appropriate bit
> > in the mbuf ol_flags field when the flag indicates a bad checksum.
> > 
> > Signed-off-by: Damjan Marion <damarion@cisco.com>
> > Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
> > ---
> >  drivers/net/i40e/i40e_rxtx_vec.c | 48 +++++++++++++++++++++++---------------
> > --
> >  1 file changed, 28 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/net/i40e/i40e_rxtx_vec.c
> > b/drivers/net/i40e/i40e_rxtx_vec.c
> > index 6c63141..d2267ad 100644
> > --- a/drivers/net/i40e/i40e_rxtx_vec.c
> > +++ b/drivers/net/i40e/i40e_rxtx_vec.c
> > @@ -138,19 +138,14 @@ i40e_rxq_rearm(struct i40e_rx_queue *rxq)  static
> > inline void  desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)  {
> > -	__m128i vlan0, vlan1, rss;
> > -	union {
> > -		uint16_t e[4];
> > -		uint64_t dword;
> > -	} vol;
> > +	__m128i vlan0, vlan1, rss, l3_l4e;
> > 
> >  	/* mask everything except RSS, flow director and VLAN flags
> >  	 * bit2 is for VLAN tag, bit11 for flow director indication
> >  	 * bit13:12 for RSS indication.
> >  	 */
> > -	const __m128i rss_vlan_msk = _mm_set_epi16(
> > -			0x0000, 0x0000, 0x0000, 0x0000,
> > -			0x3804, 0x3804, 0x3804, 0x3804);
> > +	const __m128i rss_vlan_msk = _mm_set_epi32(
> > +			0x1c03004, 0x1c03004, 0x1c03004, 0x1c03004);

Mask is wrong here. Should be 0x1c03804, ..., etc.

> > 
> >  	/* map rss and vlan type to rss hash and vlan flag */
> >  	const __m128i vlan_flags = _mm_set_epi8(0, 0, 0, 0, @@ -163,23
> > +158,36 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
> >  			PKT_RX_RSS_HASH | PKT_RX_FDIR,
> > PKT_RX_RSS_HASH, 0, 0,
> >  			0, 0, PKT_RX_FDIR, 0);
> > 
> > -	vlan0 = _mm_unpackhi_epi16(descs[0], descs[1]);
> > -	vlan1 = _mm_unpackhi_epi16(descs[2], descs[3]);
> > -	vlan0 = _mm_unpacklo_epi32(vlan0, vlan1);
> > +	const __m128i l3_l4e_flags = _mm_set_epi8(0, 0, 0, 0, 0, 0, 0, 0,
> > +			PKT_RX_EIP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD
> > | PKT_RX_IP_CKSUM_BAD,
> > +			PKT_RX_EIP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD,
> > +			PKT_RX_EIP_CKSUM_BAD | PKT_RX_IP_CKSUM_BAD,
> > +			PKT_RX_EIP_CKSUM_BAD,
> > +			PKT_RX_L4_CKSUM_BAD | PKT_RX_IP_CKSUM_BAD,
> > +			PKT_RX_L4_CKSUM_BAD,
> > +			PKT_RX_IP_CKSUM_BAD,
> > +			0);
> > +
> > +	vlan0 = _mm_unpackhi_epi32(descs[0], descs[1]);
> > +	vlan1 = _mm_unpackhi_epi32(descs[2], descs[3]);
> > +	vlan0 = _mm_unpacklo_epi64(vlan0, vlan1);
> > 
> >  	vlan1 = _mm_and_si128(vlan0, rss_vlan_msk);
> >  	vlan0 = _mm_shuffle_epi8(vlan_flags, vlan1);
> > 
> > -	rss = _mm_srli_epi16(vlan1, 11);
> > +	rss = _mm_srli_epi32(vlan1, 12);
> > 	rss = _mm_shuffle_epi8(rss_flags, rss);
> 
> My bad. Original code will use bit[13:11] to identify RSS and FDIR flag. Now 
> It masked bit 11 out when creating " rss_vlan_msk" and doing shift above,
> while it still try to use  original "rss_flags"?

Good catch.  I have no idea how you spotted that, and you're right, we should
be shifting by 11, not 12. Also the mask needs to be updated (as you
mentioned to me offline) which I noted above.

Damjan, unless you object I'll send a v3 with an updated rss_vlan_msk and
the 11 bit shift so we also get the Flow Director Filter Match (FLM)
indication.

>
  

Patch

diff --git a/drivers/net/i40e/i40e_rxtx_vec.c b/drivers/net/i40e/i40e_rxtx_vec.c
index 6c63141..d2267ad 100644
--- a/drivers/net/i40e/i40e_rxtx_vec.c
+++ b/drivers/net/i40e/i40e_rxtx_vec.c
@@ -138,19 +138,14 @@  i40e_rxq_rearm(struct i40e_rx_queue *rxq)
 static inline void
 desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
 {
-	__m128i vlan0, vlan1, rss;
-	union {
-		uint16_t e[4];
-		uint64_t dword;
-	} vol;
+	__m128i vlan0, vlan1, rss, l3_l4e;
 
 	/* mask everything except RSS, flow director and VLAN flags
 	 * bit2 is for VLAN tag, bit11 for flow director indication
 	 * bit13:12 for RSS indication.
 	 */
-	const __m128i rss_vlan_msk = _mm_set_epi16(
-			0x0000, 0x0000, 0x0000, 0x0000,
-			0x3804, 0x3804, 0x3804, 0x3804);
+	const __m128i rss_vlan_msk = _mm_set_epi32(
+			0x1c03004, 0x1c03004, 0x1c03004, 0x1c03004);
 
 	/* map rss and vlan type to rss hash and vlan flag */
 	const __m128i vlan_flags = _mm_set_epi8(0, 0, 0, 0,
@@ -163,23 +158,36 @@  desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
 			PKT_RX_RSS_HASH | PKT_RX_FDIR, PKT_RX_RSS_HASH, 0, 0,
 			0, 0, PKT_RX_FDIR, 0);
 
-	vlan0 = _mm_unpackhi_epi16(descs[0], descs[1]);
-	vlan1 = _mm_unpackhi_epi16(descs[2], descs[3]);
-	vlan0 = _mm_unpacklo_epi32(vlan0, vlan1);
+	const __m128i l3_l4e_flags = _mm_set_epi8(0, 0, 0, 0, 0, 0, 0, 0,
+			PKT_RX_EIP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD | PKT_RX_IP_CKSUM_BAD,
+			PKT_RX_EIP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD,
+			PKT_RX_EIP_CKSUM_BAD | PKT_RX_IP_CKSUM_BAD,
+			PKT_RX_EIP_CKSUM_BAD,
+			PKT_RX_L4_CKSUM_BAD | PKT_RX_IP_CKSUM_BAD,
+			PKT_RX_L4_CKSUM_BAD,
+			PKT_RX_IP_CKSUM_BAD,
+			0);
+
+	vlan0 = _mm_unpackhi_epi32(descs[0], descs[1]);
+	vlan1 = _mm_unpackhi_epi32(descs[2], descs[3]);
+	vlan0 = _mm_unpacklo_epi64(vlan0, vlan1);
 
 	vlan1 = _mm_and_si128(vlan0, rss_vlan_msk);
 	vlan0 = _mm_shuffle_epi8(vlan_flags, vlan1);
 
-	rss = _mm_srli_epi16(vlan1, 11);
+	rss = _mm_srli_epi32(vlan1, 12);
 	rss = _mm_shuffle_epi8(rss_flags, rss);
 
+	l3_l4e = _mm_srli_epi32(vlan1, 22);
+	l3_l4e = _mm_shuffle_epi8(l3_l4e_flags, l3_l4e);
+
 	vlan0 = _mm_or_si128(vlan0, rss);
-	vol.dword = _mm_cvtsi128_si64(vlan0);
+	vlan0 = _mm_or_si128(vlan0, l3_l4e);
 
-	rx_pkts[0]->ol_flags = vol.e[0];
-	rx_pkts[1]->ol_flags = vol.e[1];
-	rx_pkts[2]->ol_flags = vol.e[2];
-	rx_pkts[3]->ol_flags = vol.e[3];
+	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);
 }
 #else
 #define desc_to_olflags_v(desc, rx_pkts) do {} while (0)
@@ -754,7 +762,8 @@  i40e_rx_vec_dev_conf_condition_check(struct rte_eth_dev *dev)
 #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_vlan_extend != 0 ||
+	    rxmode->hw_ip_checksum != 0)
 		return -1;
 #endif
 
@@ -765,8 +774,7 @@  i40e_rx_vec_dev_conf_condition_check(struct rte_eth_dev *dev)
 	 /* - no csum error report support
 	 * - no header split support
 	 */
-	if (rxmode->hw_ip_checksum == 1 ||
-	    rxmode->header_split == 1)
+	if (rxmode->header_split == 1)
 		return -1;
 
 	return 0;