diff mbox

[dpdk-dev,2/5] net/fm10k: implement new Rx checksum flag

Message ID 1472147299-2376-3-git-send-email-xiao.w.wang@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Xiao Wang Aug. 25, 2016, 5:48 p.m. UTC 
  Add CKSUM_GOOD flag to distinguish a good checksum from an unknown one.

Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
---
 drivers/net/fm10k/fm10k_rxtx.c     | 4 ++++
 drivers/net/fm10k/fm10k_rxtx_vec.c | 7 +++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

Comments

Chen, Jing D Aug. 29, 2016, 9:32 a.m. UTC | #1 
Hi,

>  uint16_t
> diff --git a/drivers/net/fm10k/fm10k_rxtx_vec.c
> b/drivers/net/fm10k/fm10k_rxtx_vec.c
> index 9ea747e..8c08b44 100644
> --- a/drivers/net/fm10k/fm10k_rxtx_vec.c
> +++ b/drivers/net/fm10k/fm10k_rxtx_vec.c
> @@ -95,8 +95,10 @@ fm10k_desc_to_olflags_v(__m128i descs[4], struct rte_mbuf
> **rx_pkts)
>  	const __m128i l3l4cksum_flag = _mm_set_epi8(0, 0, 0, 0,
>  			0, 0, 0, 0,
>  			0, 0, 0, 0,
> -			PKT_RX_IP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD,
> -			PKT_RX_IP_CKSUM_BAD, PKT_RX_L4_CKSUM_BAD, 0);
> +			(PKT_RX_IP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD) >> 1,
> +			(PKT_RX_IP_CKSUM_BAD | PKT_RX_L4_CKSUM_GOOD) >>
> 1,
> +			(PKT_RX_IP_CKSUM_GOOD | PKT_RX_L4_CKSUM_BAD) >>
> 1,
> +			(PKT_RX_IP_CKSUM_GOOD |
> PKT_RX_L4_CKSUM_GOOD) >> 1);

Can we define a macro, like "#define RTE_CKSUM_SHIFT 1" to avoid numeric?

> 
>  	const __m128i rxe_flag = _mm_set_epi8(0, 0, 0, 0,
>  			0, 0, 0, 0,
> @@ -139,6 +141,7 @@ fm10k_desc_to_olflags_v(__m128i descs[4], struct
> rte_mbuf **rx_pkts)
>  	/* Process L4/L3 checksum error flags */
>  	cksumflag = _mm_srli_epi16(cksumflag, L3L4EFLAG_SHIFT);
>  	cksumflag = _mm_shuffle_epi8(l3l4cksum_flag, cksumflag);
> +	cksumflag = _mm_slli_epi16(cksumflag, 1);
>  	vtag1 = _mm_or_si128(cksumflag, vtag1);
> 
>  	vol.dword = _mm_cvtsi128_si64(vtag1);
> --
> 1.9.3

Besides that, just realize we should remove "hw_ip_checksum" check in func
fm10k_rx_vec_condition_check() since we already support it.
Can you help to make the change?
Xiao Wang Aug. 31, 2016, 8:59 a.m. UTC | #2 
Hi Mark,

> -----Original Message-----
> From: Chen, Jing D
> Sent: Monday, August 29, 2016 5:33 PM
> To: Wang, Xiao W <xiao.w.wang@intel.com>; olivier.matz@6wind.com
> Cc: dev@dpdk.org
> Subject: RE: [PATCH 2/5] net/fm10k: implement new Rx checksum flag
> 
> Hi,
> 
> >  uint16_t
> > diff --git a/drivers/net/fm10k/fm10k_rxtx_vec.c
> > b/drivers/net/fm10k/fm10k_rxtx_vec.c
> > index 9ea747e..8c08b44 100644
> > --- a/drivers/net/fm10k/fm10k_rxtx_vec.c
> > +++ b/drivers/net/fm10k/fm10k_rxtx_vec.c
> > @@ -95,8 +95,10 @@ fm10k_desc_to_olflags_v(__m128i descs[4], struct
> rte_mbuf
> > **rx_pkts)
> >  	const __m128i l3l4cksum_flag = _mm_set_epi8(0, 0, 0, 0,
> >  			0, 0, 0, 0,
> >  			0, 0, 0, 0,
> > -			PKT_RX_IP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD,
> > -			PKT_RX_IP_CKSUM_BAD, PKT_RX_L4_CKSUM_BAD, 0);
> > +			(PKT_RX_IP_CKSUM_BAD |
> PKT_RX_L4_CKSUM_BAD) >> 1,
> > +			(PKT_RX_IP_CKSUM_BAD |
> PKT_RX_L4_CKSUM_GOOD) >>
> > 1,
> > +			(PKT_RX_IP_CKSUM_GOOD |
> PKT_RX_L4_CKSUM_BAD) >>
> > 1,
> > +			(PKT_RX_IP_CKSUM_GOOD |
> > PKT_RX_L4_CKSUM_GOOD) >> 1);
> 
> Can we define a macro, like "#define RTE_CKSUM_SHIFT 1" to avoid numeric?

Yes, I'll add a macro for this, but since this shift operation isn't commonly used
by other pmds (igb and i40e don't support cksum offload in vector Rx, ixgbe cannot
do this shift due to VLAN offload), I will make it a local macro definition.

> 
> >
> >  	const __m128i rxe_flag = _mm_set_epi8(0, 0, 0, 0,
> >  			0, 0, 0, 0,
> > @@ -139,6 +141,7 @@ fm10k_desc_to_olflags_v(__m128i descs[4], struct
> > rte_mbuf **rx_pkts)
> >  	/* Process L4/L3 checksum error flags */
> >  	cksumflag = _mm_srli_epi16(cksumflag, L3L4EFLAG_SHIFT);
> >  	cksumflag = _mm_shuffle_epi8(l3l4cksum_flag, cksumflag);
> > +	cksumflag = _mm_slli_epi16(cksumflag, 1);
> >  	vtag1 = _mm_or_si128(cksumflag, vtag1);
> >
> >  	vol.dword = _mm_cvtsi128_si64(vtag1);
> > --
> > 1.9.3
> 
> Besides that, just realize we should remove "hw_ip_checksum" check in func
> fm10k_rx_vec_condition_check() since we already support it.
> Can you help to make the change?

Sure, will remove it in V2.

Thanks for the comments,
Xiao
diff mbox

Patch

diff --git a/drivers/net/fm10k/fm10k_rxtx.c b/drivers/net/fm10k/fm10k_rxtx.c
index bf5888b..32cc7ff 100644
--- a/drivers/net/fm10k/fm10k_rxtx.c
+++ b/drivers/net/fm10k/fm10k_rxtx.c
@@ -101,11 +101,15 @@  rx_desc_to_ol_flags(struct rte_mbuf *m, const union fm10k_rx_desc *d)
 		(FM10K_RXD_STATUS_IPCS | FM10K_RXD_STATUS_IPE)) ==
 		(FM10K_RXD_STATUS_IPCS | FM10K_RXD_STATUS_IPE)))
 		m->ol_flags |= PKT_RX_IP_CKSUM_BAD;
+	else
+		m->ol_flags |= PKT_RX_IP_CKSUM_GOOD;
 
 	if (unlikely((d->d.staterr &
 		(FM10K_RXD_STATUS_L4CS | FM10K_RXD_STATUS_L4E)) ==
 		(FM10K_RXD_STATUS_L4CS | FM10K_RXD_STATUS_L4E)))
 		m->ol_flags |= PKT_RX_L4_CKSUM_BAD;
+	else
+		m->ol_flags |= PKT_RX_L4_CKSUM_GOOD;
 }
 
 uint16_t
diff --git a/drivers/net/fm10k/fm10k_rxtx_vec.c b/drivers/net/fm10k/fm10k_rxtx_vec.c
index 9ea747e..8c08b44 100644
--- a/drivers/net/fm10k/fm10k_rxtx_vec.c
+++ b/drivers/net/fm10k/fm10k_rxtx_vec.c
@@ -95,8 +95,10 @@  fm10k_desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
 	const __m128i l3l4cksum_flag = _mm_set_epi8(0, 0, 0, 0,
 			0, 0, 0, 0,
 			0, 0, 0, 0,
-			PKT_RX_IP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD,
-			PKT_RX_IP_CKSUM_BAD, PKT_RX_L4_CKSUM_BAD, 0);
+			(PKT_RX_IP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD) >> 1,
+			(PKT_RX_IP_CKSUM_BAD | PKT_RX_L4_CKSUM_GOOD) >> 1,
+			(PKT_RX_IP_CKSUM_GOOD | PKT_RX_L4_CKSUM_BAD) >> 1,
+			(PKT_RX_IP_CKSUM_GOOD | PKT_RX_L4_CKSUM_GOOD) >> 1);
 
 	const __m128i rxe_flag = _mm_set_epi8(0, 0, 0, 0,
 			0, 0, 0, 0,
@@ -139,6 +141,7 @@  fm10k_desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
 	/* Process L4/L3 checksum error flags */
 	cksumflag = _mm_srli_epi16(cksumflag, L3L4EFLAG_SHIFT);
 	cksumflag = _mm_shuffle_epi8(l3l4cksum_flag, cksumflag);
+	cksumflag = _mm_slli_epi16(cksumflag, 1);
 	vtag1 = _mm_or_si128(cksumflag, vtag1);
 
 	vol.dword = _mm_cvtsi128_si64(vtag1);