DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v1] net/ixgbe: adjust error for UDP with zero checksum
@ 2021-02-02  7:06 Haiyue Wang
  2021-02-02  9:45 ` David Marchand
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Haiyue Wang @ 2021-02-02  7:06 UTC (permalink / raw)
  To: dev
  Cc: pvalerio, aconole, david.marchand, qi.z.zhang, leyi.rong,
	Lijuan.Tu, Haiyue Wang, stable, Jeff Guo, Bruce Richardson,
	Konstantin Ananyev

There is an 82599 errata that UDP frames with a zero checksum are
incorrectly marked as checksum invalid by the hardware.  This was
leading to misleading PKT_RX_L4_CKSUM_BAD flag. This patch adds a
test around this checksum error flag set for this condition.

1. UDP Test
  sendp(Ether()/IP()/UDP(chksum=0)/Raw("a"*100), iface="ens802f0")
  port 0/queue 0: received 1 packets
    ol_flags: PKT_RX_L4_CKSUM_GOOD PKT_RX_IP_CKSUM_GOOD

2. TCP Test
  sendp(Ether()/IP()/TCP(chksum=0)/Raw("a"*100), iface="ens802f0")
  port 0/queue 0: received 1 packets
    ol_flags: PKT_RX_L4_CKSUM_BAD PKT_RX_IP_CKSUM_GOOD

Bugzilla ID: 629
Cc: stable@dpdk.org

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
 doc/guides/nics/ixgbe.rst              |  6 ++++
 drivers/net/ixgbe/ixgbe_rxtx.c         | 27 +++++++++++---
 drivers/net/ixgbe/ixgbe_rxtx.h         |  2 ++
 drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c | 49 ++++++++++++++++++++------
 4 files changed, 70 insertions(+), 14 deletions(-)

diff --git a/doc/guides/nics/ixgbe.rst b/doc/guides/nics/ixgbe.rst
index 696cbd93b..de210b7b8 100644
--- a/doc/guides/nics/ixgbe.rst
+++ b/doc/guides/nics/ixgbe.rst
@@ -287,6 +287,12 @@ the VFs which are required.::
 Currently hot-plugging of representor ports is not supported so all required
 representors must be specified on the creation of the PF.
 
+Limitations or Known issues
+---------------------------
+The 82599 hardware errata: UDP frames with a zero checksum can be marked as
+checksum errors. To support zero checksum, the UDP checksum is always marked
+as good.
+
 Supported Chipsets and NICs
 ---------------------------
 
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index c1f0c446a..550e3f390 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -1466,7 +1466,8 @@ rx_desc_status_to_pkt_flags(uint32_t rx_status, uint64_t vlan_flags)
 }
 
 static inline uint64_t
-rx_desc_error_to_pkt_flags(uint32_t rx_status)
+rx_desc_error_to_pkt_flags(uint32_t rx_status, uint16_t pkt_info,
+			   uint8_t rx_udp_csum_zero_err)
 {
 	uint64_t pkt_flags;
 
@@ -1480,6 +1481,12 @@ rx_desc_error_to_pkt_flags(uint32_t rx_status)
 		PKT_RX_IP_CKSUM_BAD | PKT_RX_L4_CKSUM_GOOD,
 		PKT_RX_IP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD
 	};
+
+	if ((rx_status & IXGBE_RXDADV_ERR_TCPE) &&
+	    (pkt_info & IXGBE_RXDADV_PKTTYPE_UDP) &&
+	    rx_udp_csum_zero_err)
+		rx_status &= ~IXGBE_RXDADV_ERR_TCPE;
+
 	pkt_flags = error_to_pkt_flags_map[(rx_status >>
 		IXGBE_RXDADV_ERR_CKSUM_BIT) & IXGBE_RXDADV_ERR_CKSUM_MSK];
 
@@ -1569,7 +1576,9 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
 			/* convert descriptor fields to rte mbuf flags */
 			pkt_flags = rx_desc_status_to_pkt_flags(s[j],
 				vlan_flags);
-			pkt_flags |= rx_desc_error_to_pkt_flags(s[j]);
+			pkt_flags |= rx_desc_error_to_pkt_flags(s[j],
+					(uint16_t)pkt_info[j],
+					rxq->rx_udp_csum_zero_err);
 			pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags
 					((uint16_t)pkt_info[j]);
 			mb->ol_flags = pkt_flags;
@@ -1902,7 +1911,9 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		rxm->vlan_tci = rte_le_to_cpu_16(rxd.wb.upper.vlan);
 
 		pkt_flags = rx_desc_status_to_pkt_flags(staterr, vlan_flags);
-		pkt_flags = pkt_flags | rx_desc_error_to_pkt_flags(staterr);
+		pkt_flags = pkt_flags |
+			rx_desc_error_to_pkt_flags(staterr, (uint16_t)pkt_info,
+						   rxq->rx_udp_csum_zero_err);
 		pkt_flags = pkt_flags |
 			ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info);
 		rxm->ol_flags = pkt_flags;
@@ -1995,7 +2006,8 @@ ixgbe_fill_cluster_head_buf(
 	head->vlan_tci = rte_le_to_cpu_16(desc->wb.upper.vlan);
 	pkt_info = rte_le_to_cpu_32(desc->wb.lower.lo_dword.data);
 	pkt_flags = rx_desc_status_to_pkt_flags(staterr, rxq->vlan_flags);
-	pkt_flags |= rx_desc_error_to_pkt_flags(staterr);
+	pkt_flags |= rx_desc_error_to_pkt_flags(staterr, (uint16_t)pkt_info,
+						rxq->rx_udp_csum_zero_err);
 	pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info);
 	head->ol_flags = pkt_flags;
 	head->packet_type =
@@ -3116,6 +3128,13 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	else
 		rxq->pkt_type_mask = IXGBE_PACKET_TYPE_MASK_82599;
 
+	/*
+	 * 82599 errata, UDP frames with a 0 checksum can be marked as checksum
+	 * errors.
+	 */
+	if (hw->mac.type == ixgbe_mac_82599EB)
+		rxq->rx_udp_csum_zero_err = 1;
+
 	/*
 	 * Allocate RX ring hardware descriptors. A memzone large enough to
 	 * handle the maximum ring size is allocated in order to allow for
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
index 8a25e98df..476ef62cf 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.h
+++ b/drivers/net/ixgbe/ixgbe_rxtx.h
@@ -129,6 +129,8 @@ struct ixgbe_rx_queue {
 	uint8_t             crc_len;  /**< 0 if CRC stripped, 4 otherwise. */
 	uint8_t             drop_en;  /**< If not 0, set SRRCTL.Drop_En. */
 	uint8_t             rx_deferred_start; /**< not in global dev start. */
+	/** UDP frames with a 0 checksum can be marked as checksum errors. */
+	uint8_t             rx_udp_csum_zero_err;
 	/** flags to set in mbuf when a vlan is detected. */
 	uint64_t            vlan_flags;
 	uint64_t	    offloads; /**< Rx offloads with DEV_RX_OFFLOAD_* */
diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
index 90c076825..8bffafc71 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
@@ -132,9 +132,9 @@ desc_to_olflags_v_ipsec(__m128i descs[4], struct rte_mbuf **rx_pkts)
 
 static inline void
 desc_to_olflags_v(__m128i descs[4], __m128i mbuf_init, uint8_t vlan_flags,
-	struct rte_mbuf **rx_pkts)
+		  uint16_t udp_p_flag, struct rte_mbuf **rx_pkts)
 {
-	__m128i ptype0, ptype1, vtag0, vtag1, csum;
+	__m128i ptype0, ptype1, vtag0, vtag1, csum, vlan_csum_msk;
 	__m128i rearm0, rearm1, rearm2, rearm3;
 
 	/* mask everything except rss type */
@@ -154,13 +154,29 @@ desc_to_olflags_v(__m128i descs[4], __m128i mbuf_init, uint8_t vlan_flags,
 			PKT_RX_RSS_HASH, PKT_RX_RSS_HASH, PKT_RX_RSS_HASH, 0);
 
 	/* mask everything except vlan present and l4/ip csum error */
-	const __m128i vlan_csum_msk = _mm_set_epi16(
-		(IXGBE_RXDADV_ERR_TCPE | IXGBE_RXDADV_ERR_IPE) >> 16,
-		(IXGBE_RXDADV_ERR_TCPE | IXGBE_RXDADV_ERR_IPE) >> 16,
-		(IXGBE_RXDADV_ERR_TCPE | IXGBE_RXDADV_ERR_IPE) >> 16,
-		(IXGBE_RXDADV_ERR_TCPE | IXGBE_RXDADV_ERR_IPE) >> 16,
-		IXGBE_RXD_STAT_VP, IXGBE_RXD_STAT_VP,
-		IXGBE_RXD_STAT_VP, IXGBE_RXD_STAT_VP);
+	const __m128i vlan_csum_all_msk = _mm_set_epi16
+		((IXGBE_RXDADV_ERR_TCPE | IXGBE_RXDADV_ERR_IPE) >> 16,
+		 (IXGBE_RXDADV_ERR_TCPE | IXGBE_RXDADV_ERR_IPE) >> 16,
+		 (IXGBE_RXDADV_ERR_TCPE | IXGBE_RXDADV_ERR_IPE) >> 16,
+		 (IXGBE_RXDADV_ERR_TCPE | IXGBE_RXDADV_ERR_IPE) >> 16,
+		 IXGBE_RXD_STAT_VP, IXGBE_RXD_STAT_VP,
+		 IXGBE_RXD_STAT_VP, IXGBE_RXD_STAT_VP);
+
+	/* mask everything except UDP header present */
+	const __m128i udptype_msk = _mm_set_epi16
+		(0, 0, 0, 0,
+		 udp_p_flag, udp_p_flag, udp_p_flag, udp_p_flag);
+
+	/* convert UDP header present 16 bits 0x0200 to 8 bits 0x02, then get
+	 * the TCP/UDP checksum error mask 8 bits ~0x40 from 32 bits value of
+	 * 0x40000000.
+	 */
+	const __m128i udp_csum_err_skip = _mm_set_epi8
+		(0, 0, 0, 0,
+		 0, 0, 0, 0,
+		 0, 0, 0, 0,
+		 0, ~(IXGBE_RXDADV_ERR_TCPE >> 24), 0, 0xFF);
+
 	/* map vlan present (0x8), IPE (0x2), L4E (0x1) to ol_flags */
 	const __m128i vlan_csum_map_lo = _mm_set_epi8(
 		0, 0, 0, 0,
@@ -188,9 +204,17 @@ desc_to_olflags_v(__m128i descs[4], __m128i mbuf_init, uint8_t vlan_flags,
 	vtag1 = _mm_unpackhi_epi16(descs[2], descs[3]);
 
 	ptype0 = _mm_unpacklo_epi32(ptype0, ptype1);
+	/* save the UDP header present information */
+	vlan_csum_msk = _mm_and_si128(ptype0, udptype_msk);
 	ptype0 = _mm_and_si128(ptype0, rsstype_msk);
 	ptype0 = _mm_shuffle_epi8(rss_flags, ptype0);
 
+	/* now the most significant 64 bits containing the UDP present */
+	vlan_csum_msk = _mm_slli_si128(vlan_csum_msk, 8);
+	/* use UDP present 0x02 index to get L4 checksum error mask ~0x40 */
+	vlan_csum_msk = _mm_shuffle_epi8(udp_csum_err_skip, vlan_csum_msk);
+	/* then mask out the L4 checksum error bit as needed */
+	vlan_csum_msk = _mm_and_si128(vlan_csum_all_msk, vlan_csum_msk);
 	vtag1 = _mm_unpacklo_epi32(vtag0, vtag1);
 	vtag1 = _mm_and_si128(vtag1, vlan_csum_msk);
 
@@ -341,6 +365,7 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 	__m128i dd_check, eop_check;
 	__m128i mbuf_init;
 	uint8_t vlan_flags;
+	uint16_t udp_p_flag = 0; /* Rx Descriptor UDP header present */
 
 	/* nb_pkts has to be floor-aligned to RTE_IXGBE_DESCS_PER_LOOP */
 	nb_pkts = RTE_ALIGN_FLOOR(nb_pkts, RTE_IXGBE_DESCS_PER_LOOP);
@@ -365,6 +390,9 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 				rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
 		return 0;
 
+	if (rxq->rx_udp_csum_zero_err)
+		udp_p_flag = IXGBE_RXDADV_PKTTYPE_UDP;
+
 	/* 4 packets DD mask */
 	dd_check = _mm_set_epi64x(0x0000000100000001LL, 0x0000000100000001LL);
 
@@ -477,7 +505,8 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 		sterr_tmp1 = _mm_unpackhi_epi32(descs[1], descs[0]);
 
 		/* set ol_flags with vlan packet type */
-		desc_to_olflags_v(descs, mbuf_init, vlan_flags, &rx_pkts[pos]);
+		desc_to_olflags_v(descs, mbuf_init, vlan_flags, udp_p_flag,
+				  &rx_pkts[pos]);
 
 #ifdef RTE_LIB_SECURITY
 		if (unlikely(use_ipsec))
-- 
2.30.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v1] net/ixgbe: adjust error for UDP with zero checksum
  2021-02-02  7:06 [dpdk-dev] [PATCH v1] net/ixgbe: adjust error for UDP with zero checksum Haiyue Wang
@ 2021-02-02  9:45 ` David Marchand
  2021-02-02 12:42   ` Wang, Haiyue
  2021-02-02 12:54   ` Wang, Haiyue
  2021-02-03  2:19 ` [dpdk-dev] [PATCH v2] net/ixgbe: fix UDP zero checksum error Haiyue Wang
  2021-02-04 14:39 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
  2 siblings, 2 replies; 20+ messages in thread
From: David Marchand @ 2021-02-02  9:45 UTC (permalink / raw)
  To: Haiyue Wang
  Cc: dev, pvalerio, Aaron Conole, Qi Zhang, Leyi Rong, Tu, Lijuan,
	dpdk stable, Jeff Guo, Bruce Richardson, Konstantin Ananyev,
	Jerin Jacob Kollanukkaran, Ruifeng Wang (Arm Technology China)

Hello Haiyue,

Thanks for working on it quickly.
Cc: ARM maintainers.

On Tue, Feb 2, 2021 at 8:23 AM Haiyue Wang <haiyue.wang@intel.com> wrote:
>
> There is an 82599 errata that UDP frames with a zero checksum are
> incorrectly marked as checksum invalid by the hardware.  This was

Maybe add a reference to the 82599 hw errata, is this listed in the datasheet?


> leading to misleading PKT_RX_L4_CKSUM_BAD flag. This patch adds a
> test around this checksum error flag set for this condition.
>
> 1. UDP Test
>   sendp(Ether()/IP()/UDP(chksum=0)/Raw("a"*100), iface="ens802f0")
>   port 0/queue 0: received 1 packets
>     ol_flags: PKT_RX_L4_CKSUM_GOOD PKT_RX_IP_CKSUM_GOOD
>
> 2. TCP Test
>   sendp(Ether()/IP()/TCP(chksum=0)/Raw("a"*100), iface="ens802f0")
>   port 0/queue 0: received 1 packets
>     ol_flags: PKT_RX_L4_CKSUM_BAD PKT_RX_IP_CKSUM_GOOD
>
> Bugzilla ID: 629

The problem has always been present, so I would flag:
Fixes: af75078fece3 ("first public release")

> Cc: stable@dpdk.org
>

Reported-by: Paolo Valerio <pvalerio@redhat.com>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> ---
>  doc/guides/nics/ixgbe.rst              |  6 ++++
>  drivers/net/ixgbe/ixgbe_rxtx.c         | 27 +++++++++++---
>  drivers/net/ixgbe/ixgbe_rxtx.h         |  2 ++
>  drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c | 49 ++++++++++++++++++++------
>  4 files changed, 70 insertions(+), 14 deletions(-)
>
> diff --git a/doc/guides/nics/ixgbe.rst b/doc/guides/nics/ixgbe.rst
> index 696cbd93b..de210b7b8 100644
> --- a/doc/guides/nics/ixgbe.rst
> +++ b/doc/guides/nics/ixgbe.rst
> @@ -287,6 +287,12 @@ the VFs which are required.::
>  Currently hot-plugging of representor ports is not supported so all required
>  representors must be specified on the creation of the PF.
>
> +Limitations or Known issues
> +---------------------------
> +The 82599 hardware errata: UDP frames with a zero checksum can be marked as
> +checksum errors. To support zero checksum, the UDP checksum is always marked
> +as good.
> +

If the driver/hw can't report a valid checksum hint, it should
announce it does not know if the checksum is valid (neither bad, nor
good).

So the workaround for udp packets (on this hw model) would be to
report PKT_RX_L4_CKSUM_UNKNOWN.
The sw application will then have to recompute the checksum itself if needed.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v1] net/ixgbe: adjust error for UDP with zero checksum
  2021-02-02  9:45 ` David Marchand
@ 2021-02-02 12:42   ` Wang, Haiyue
  2021-02-02 12:53     ` David Marchand
  2021-02-02 12:54   ` Wang, Haiyue
  1 sibling, 1 reply; 20+ messages in thread
From: Wang, Haiyue @ 2021-02-02 12:42 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, pvalerio, Aaron Conole, Zhang, Qi Z, Rong, Leyi, Tu, Lijuan,
	dpdk stable, Guo, Jia, Richardson, Bruce, Ananyev, Konstantin,
	Jerin Jacob Kollanukkaran, Ruifeng Wang (Arm Technology China)

Hi David,

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Tuesday, February 2, 2021 17:45
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dev <dev@dpdk.org>; pvalerio@redhat.com; Aaron Conole <aconole@redhat.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Rong, Leyi <leyi.rong@intel.com>; Tu, Lijuan <lijuan.tu@intel.com>; dpdk
> stable <stable@dpdk.org>; Guo, Jia <jia.guo@intel.com>; Richardson, Bruce <bruce.richardson@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> Ruifeng Wang (Arm Technology China) <ruifeng.wang@arm.com>
> Subject: Re: [PATCH v1] net/ixgbe: adjust error for UDP with zero checksum
> 
> Hello Haiyue,
> 
> Thanks for working on it quickly.
> Cc: ARM maintainers.
> 
> On Tue, Feb 2, 2021 at 8:23 AM Haiyue Wang <haiyue.wang@intel.com> wrote:
> >
> > There is an 82599 errata that UDP frames with a zero checksum are
> > incorrectly marked as checksum invalid by the hardware.  This was
> 
> Maybe add a reference to the 82599 hw errata, is this listed in the datasheet?
> 

I didn't find open and official doc, but on:

https://www.mouser.es/pdfdocs/82599specupdate.pdf

44 Integrity Error Reported for IPv4/UDP Packets
With Zero Checksum
Problem: According to the UDP specification “an all zero transmitted checksum value
means that the transmitter generated no checksum (for debugging or for higher
level protocols that don’t care)”, these packets should be received without a
checksum error notation. The 82599 reports an L4 integrity error if such packets
are received.
Implication: UDP packets without a checksum will have an L4 integrity error indication in the
Rx descriptor.
Workaround: If bits L4E and L4I are set in the Rx descriptor, the software driver should
check if the checksum is zero and then ignore this error.
Status: B0=Yes; No Fix

> 
> > leading to misleading PKT_RX_L4_CKSUM_BAD flag. This patch adds a
> > test around this checksum error flag set for this condition.
> >
> > 1. UDP Test
> >   sendp(Ether()/IP()/UDP(chksum=0)/Raw("a"*100), iface="ens802f0")
> >   port 0/queue 0: received 1 packets
> >     ol_flags: PKT_RX_L4_CKSUM_GOOD PKT_RX_IP_CKSUM_GOOD
> >
> > 2. TCP Test
> >   sendp(Ether()/IP()/TCP(chksum=0)/Raw("a"*100), iface="ens802f0")
> >   port 0/queue 0: received 1 packets
> >     ol_flags: PKT_RX_L4_CKSUM_BAD PKT_RX_IP_CKSUM_GOOD
> >
> > Bugzilla ID: 629
> 
> The problem has always been present, so I would flag:
> Fixes: af75078fece3 ("first public release")
> 
> > Cc: stable@dpdk.org
> >
> 
> Reported-by: Paolo Valerio <pvalerio@redhat.com>
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > ---
> >  doc/guides/nics/ixgbe.rst              |  6 ++++
> >  drivers/net/ixgbe/ixgbe_rxtx.c         | 27 +++++++++++---
> >  drivers/net/ixgbe/ixgbe_rxtx.h         |  2 ++
> >  drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c | 49 ++++++++++++++++++++------
> >  4 files changed, 70 insertions(+), 14 deletions(-)
> >
> > diff --git a/doc/guides/nics/ixgbe.rst b/doc/guides/nics/ixgbe.rst
> > index 696cbd93b..de210b7b8 100644
> > --- a/doc/guides/nics/ixgbe.rst
> > +++ b/doc/guides/nics/ixgbe.rst
> > @@ -287,6 +287,12 @@ the VFs which are required.::
> >  Currently hot-plugging of representor ports is not supported so all required
> >  representors must be specified on the creation of the PF.
> >
> > +Limitations or Known issues
> > +---------------------------
> > +The 82599 hardware errata: UDP frames with a zero checksum can be marked as
> > +checksum errors. To support zero checksum, the UDP checksum is always marked
> > +as good.
> > +
> 
> If the driver/hw can't report a valid checksum hint, it should
> announce it does not know if the checksum is valid (neither bad, nor
> good).
> 
> So the workaround for udp packets (on this hw model) would be to
> report PKT_RX_L4_CKSUM_UNKNOWN.
> The sw application will then have to recompute the checksum itself if needed.
> 

Make sense, but not sure the vector path can handle this more easily. Will try.

> 
> --
> David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v1] net/ixgbe: adjust error for UDP with zero checksum
  2021-02-02 12:42   ` Wang, Haiyue
@ 2021-02-02 12:53     ` David Marchand
  2021-02-02 12:56       ` Wang, Haiyue
  0 siblings, 1 reply; 20+ messages in thread
From: David Marchand @ 2021-02-02 12:53 UTC (permalink / raw)
  To: Wang, Haiyue
  Cc: dev, pvalerio, Aaron Conole, Zhang, Qi Z, Rong, Leyi, Tu, Lijuan,
	dpdk stable, Guo, Jia, Richardson, Bruce, Ananyev, Konstantin,
	Jerin Jacob Kollanukkaran, Ruifeng Wang (Arm Technology China)

On Tue, Feb 2, 2021 at 1:42 PM Wang, Haiyue <haiyue.wang@intel.com> wrote:
> > If the driver/hw can't report a valid checksum hint, it should
> > announce it does not know if the checksum is valid (neither bad, nor
> > good).
> >
> > So the workaround for udp packets (on this hw model) would be to
> > report PKT_RX_L4_CKSUM_UNKNOWN.
> > The sw application will then have to recompute the checksum itself if needed.
> >
>
> Make sense, but not sure the vector path can handle this more easily. Will try.

Refining this a bit.
It looks like hw correctly reports "good" checksums, so maybe instead
report PKT_RX_L4_CKSUM_UNKNOWN only for reports of "bad" checksums
from the hw?

-- 
David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v1] net/ixgbe: adjust error for UDP with zero checksum
  2021-02-02  9:45 ` David Marchand
  2021-02-02 12:42   ` Wang, Haiyue
@ 2021-02-02 12:54   ` Wang, Haiyue
  1 sibling, 0 replies; 20+ messages in thread
From: Wang, Haiyue @ 2021-02-02 12:54 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, pvalerio, Aaron Conole, Zhang, Qi Z, Rong, Leyi, Tu, Lijuan,
	dpdk stable, Guo, Jia, Richardson, Bruce, Ananyev, Konstantin,
	Jerin Jacob Kollanukkaran, Ruifeng Wang (Arm Technology China)

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Tuesday, February 2, 2021 17:45
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dev <dev@dpdk.org>; pvalerio@redhat.com; Aaron Conole <aconole@redhat.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Rong, Leyi <leyi.rong@intel.com>; Tu, Lijuan <lijuan.tu@intel.com>; dpdk
> stable <stable@dpdk.org>; Guo, Jia <jia.guo@intel.com>; Richardson, Bruce <bruce.richardson@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> Ruifeng Wang (Arm Technology China) <ruifeng.wang@arm.com>
> Subject: Re: [PATCH v1] net/ixgbe: adjust error for UDP with zero checksum
> 


> 
> If the driver/hw can't report a valid checksum hint, it should
> announce it does not know if the checksum is valid (neither bad, nor
> good).
> 
> So the workaround for udp packets (on this hw model) would be to
> report PKT_RX_L4_CKSUM_UNKNOWN.
> The sw application will then have to recompute the checksum itself if needed.
> 

Looks like this workaround will make OVS performance drop a lot, since
every UDP packet needs to do checksum by SW:

            bool  hwol_good_l4_csum = dp_packet_l4_checksum_valid(pkt)
                                      || dp_packet_hwol_tx_l4_checksum(pkt);
            /* Validate the checksum only when hwol is not supported. */
            if (extract_l4(&ctx->key, l4, dp_packet_l4_size(pkt),
                           &ctx->icmp_related, l3, !hwol_good_l4_csum,
                           NULL)) {
                ctx->hash = conn_key_hash(&ctx->key, ct->hash_basis);
                return true;
            }

The lesser of the two rights, marking as good seems a little better.

> 
> --
> David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v1] net/ixgbe: adjust error for UDP with zero checksum
  2021-02-02 12:53     ` David Marchand
@ 2021-02-02 12:56       ` Wang, Haiyue
  2021-02-02 14:28         ` Wang, Haiyue
  0 siblings, 1 reply; 20+ messages in thread
From: Wang, Haiyue @ 2021-02-02 12:56 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, pvalerio, Aaron Conole, Zhang, Qi Z, Rong, Leyi, Tu, Lijuan,
	dpdk stable, Guo, Jia, Richardson, Bruce, Ananyev, Konstantin,
	Jerin Jacob Kollanukkaran, Ruifeng Wang (Arm Technology China)

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Tuesday, February 2, 2021 20:54
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dev <dev@dpdk.org>; pvalerio@redhat.com; Aaron Conole <aconole@redhat.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Rong, Leyi <leyi.rong@intel.com>; Tu, Lijuan <lijuan.tu@intel.com>; dpdk
> stable <stable@dpdk.org>; Guo, Jia <jia.guo@intel.com>; Richardson, Bruce <bruce.richardson@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> Ruifeng Wang (Arm Technology China) <ruifeng.wang@arm.com>
> Subject: Re: [PATCH v1] net/ixgbe: adjust error for UDP with zero checksum
> 
> On Tue, Feb 2, 2021 at 1:42 PM Wang, Haiyue <haiyue.wang@intel.com> wrote:
> > > If the driver/hw can't report a valid checksum hint, it should
> > > announce it does not know if the checksum is valid (neither bad, nor
> > > good).
> > >
> > > So the workaround for udp packets (on this hw model) would be to
> > > report PKT_RX_L4_CKSUM_UNKNOWN.
> > > The sw application will then have to recompute the checksum itself if needed.
> > >
> >
> > Make sense, but not sure the vector path can handle this more easily. Will try.
> 
> Refining this a bit.
> It looks like hw correctly reports "good" checksums, so maybe instead
> report PKT_RX_L4_CKSUM_UNKNOWN only for reports of "bad" checksums
> from the hw?

I guess Paolo will complain about the performance drop for zero checksum
UDP. ;-)

> 
> --
> David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v1] net/ixgbe: adjust error for UDP with zero checksum
  2021-02-02 12:56       ` Wang, Haiyue
@ 2021-02-02 14:28         ` Wang, Haiyue
  2021-02-02 17:42           ` Paolo Valerio
  0 siblings, 1 reply; 20+ messages in thread
From: Wang, Haiyue @ 2021-02-02 14:28 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, pvalerio, Aaron Conole, Zhang, Qi Z, Rong, Leyi, Tu, Lijuan,
	dpdk stable, Guo, Jia, Richardson, Bruce, Ananyev, Konstantin,
	Jerin Jacob Kollanukkaran, Ruifeng Wang (Arm Technology China)

> -----Original Message-----
> From: Wang, Haiyue
> Sent: Tuesday, February 2, 2021 20:57
> To: David Marchand <david.marchand@redhat.com>
> Cc: dev <dev@dpdk.org>; pvalerio@redhat.com; Aaron Conole <aconole@redhat.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Rong, Leyi <leyi.rong@intel.com>; Tu, Lijuan <Lijuan.Tu@intel.com>; dpdk
> stable <stable@dpdk.org>; Guo, Jia <jia.guo@intel.com>; Richardson, Bruce <bruce.richardson@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> Ruifeng Wang (Arm Technology China) <ruifeng.wang@arm.com>
> Subject: RE: [PATCH v1] net/ixgbe: adjust error for UDP with zero checksum
> 
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Tuesday, February 2, 2021 20:54
> > To: Wang, Haiyue <haiyue.wang@intel.com>
> > Cc: dev <dev@dpdk.org>; pvalerio@redhat.com; Aaron Conole <aconole@redhat.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>; Rong, Leyi <leyi.rong@intel.com>; Tu, Lijuan <lijuan.tu@intel.com>; dpdk
> > stable <stable@dpdk.org>; Guo, Jia <jia.guo@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>;
> > Ananyev, Konstantin <konstantin.ananyev@intel.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> > Ruifeng Wang (Arm Technology China) <ruifeng.wang@arm.com>
> > Subject: Re: [PATCH v1] net/ixgbe: adjust error for UDP with zero checksum
> >
> > On Tue, Feb 2, 2021 at 1:42 PM Wang, Haiyue <haiyue.wang@intel.com> wrote:
> > > > If the driver/hw can't report a valid checksum hint, it should
> > > > announce it does not know if the checksum is valid (neither bad, nor
> > > > good).
> > > >
> > > > So the workaround for udp packets (on this hw model) would be to
> > > > report PKT_RX_L4_CKSUM_UNKNOWN.
> > > > The sw application will then have to recompute the checksum itself if needed.
> > > >
> > >
> > > Make sense, but not sure the vector path can handle this more easily. Will try.
> >
> > Refining this a bit.
> > It looks like hw correctly reports "good" checksums, so maybe instead
> > report PKT_RX_L4_CKSUM_UNKNOWN only for reports of "bad" checksums
> > from the hw?
> 
> I guess Paolo will complain about the performance drop for zero checksum
> UDP. ;-)
> 

Deep into OVS for detail, 'PKT_RX_L4_CKSUM_UNKNOWN' is a graceful way. ;-)
Will work for this target.

    /* Validation must be skipped if checksum is 0 on IPv4 packets */
    return (udp->udp_csum == 0 && key->dl_type == htons(ETH_TYPE_IP))
           || (validate_checksum ? checksum_valid(key, data, size, l3) : true);

> >
> > --
> > David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v1] net/ixgbe: adjust error for UDP with zero checksum
  2021-02-02 14:28         ` Wang, Haiyue
@ 2021-02-02 17:42           ` Paolo Valerio
  0 siblings, 0 replies; 20+ messages in thread
From: Paolo Valerio @ 2021-02-02 17:42 UTC (permalink / raw)
  To: Wang, Haiyue, David Marchand
  Cc: dev, Aaron Conole, Zhang, Qi Z, Rong, Leyi, Tu, Lijuan,
	dpdk stable, Guo, Jia, Richardson, Bruce, Ananyev, Konstantin,
	Jerin Jacob Kollanukkaran, Ruifeng Wang (Arm Technology China)

"Wang, Haiyue" <haiyue.wang@intel.com> writes:

>> -----Original Message-----
>> From: Wang, Haiyue
>> Sent: Tuesday, February 2, 2021 20:57
>> To: David Marchand <david.marchand@redhat.com>
>> Cc: dev <dev@dpdk.org>; pvalerio@redhat.com; Aaron Conole <aconole@redhat.com>; Zhang, Qi Z
>> <qi.z.zhang@intel.com>; Rong, Leyi <leyi.rong@intel.com>; Tu, Lijuan <Lijuan.Tu@intel.com>; dpdk
>> stable <stable@dpdk.org>; Guo, Jia <jia.guo@intel.com>; Richardson, Bruce <bruce.richardson@intel.com>;
>> Ananyev, Konstantin <konstantin.ananyev@intel.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
>> Ruifeng Wang (Arm Technology China) <ruifeng.wang@arm.com>
>> Subject: RE: [PATCH v1] net/ixgbe: adjust error for UDP with zero checksum
>> 
>> > -----Original Message-----
>> > From: David Marchand <david.marchand@redhat.com>
>> > Sent: Tuesday, February 2, 2021 20:54
>> > To: Wang, Haiyue <haiyue.wang@intel.com>
>> > Cc: dev <dev@dpdk.org>; pvalerio@redhat.com; Aaron Conole <aconole@redhat.com>; Zhang, Qi Z
>> > <qi.z.zhang@intel.com>; Rong, Leyi <leyi.rong@intel.com>; Tu, Lijuan <lijuan.tu@intel.com>; dpdk
>> > stable <stable@dpdk.org>; Guo, Jia <jia.guo@intel.com>; Richardson, Bruce
>> <bruce.richardson@intel.com>;
>> > Ananyev, Konstantin <konstantin.ananyev@intel.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
>> > Ruifeng Wang (Arm Technology China) <ruifeng.wang@arm.com>
>> > Subject: Re: [PATCH v1] net/ixgbe: adjust error for UDP with zero checksum
>> >
>> > On Tue, Feb 2, 2021 at 1:42 PM Wang, Haiyue <haiyue.wang@intel.com> wrote:
>> > > > If the driver/hw can't report a valid checksum hint, it should
>> > > > announce it does not know if the checksum is valid (neither bad, nor
>> > > > good).
>> > > >
>> > > > So the workaround for udp packets (on this hw model) would be to
>> > > > report PKT_RX_L4_CKSUM_UNKNOWN.
>> > > > The sw application will then have to recompute the checksum itself if needed.
>> > > >
>> > >
>> > > Make sense, but not sure the vector path can handle this more easily. Will try.
>> >
>> > Refining this a bit.
>> > It looks like hw correctly reports "good" checksums, so maybe instead
>> > report PKT_RX_L4_CKSUM_UNKNOWN only for reports of "bad" checksums
>> > from the hw?
>> 
>> I guess Paolo will complain about the performance drop for zero checksum
>> UDP. ;-)
>>

:)

>
> Deep into OVS for detail, 'PKT_RX_L4_CKSUM_UNKNOWN' is a graceful way. ;-)
> Will work for this target.

yes, validation gets skipped in such case.
I'll be happy to test it once posted.

>
>     /* Validation must be skipped if checksum is 0 on IPv4 packets */
>     return (udp->udp_csum == 0 && key->dl_type == htons(ETH_TYPE_IP))
>            || (validate_checksum ? checksum_valid(key, data, size, l3) : true);
>
>> >
>> > --
>> > David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [dpdk-dev] [PATCH v2] net/ixgbe: fix UDP zero checksum error
  2021-02-02  7:06 [dpdk-dev] [PATCH v1] net/ixgbe: adjust error for UDP with zero checksum Haiyue Wang
  2021-02-02  9:45 ` David Marchand
@ 2021-02-03  2:19 ` Haiyue Wang
  2021-02-03  8:07   ` Zhang, Qi Z
  2021-02-04 14:39 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
  2 siblings, 1 reply; 20+ messages in thread
From: Haiyue Wang @ 2021-02-03  2:19 UTC (permalink / raw)
  To: dev
  Cc: pvalerio, aconole, david.marchand, qi.z.zhang, leyi.rong,
	Lijuan.Tu, Ruifeng.Wang, Feifei.Wang2, Haiyue Wang, stable,
	Jeff Guo, Bruce Richardson, Konstantin Ananyev

There is an 82599 errata that UDP frames with a zero checksum are
incorrectly marked as checksum invalid by the hardware.  This was
leading to misleading PKT_RX_L4_CKSUM_BAD flag.

This patch changes the bad UDP checksum to PKT_RX_L4_CKSUM_UNKNOWN,
so the software application will then have to recompute the checksum
itself if needed.

Bugzilla ID: 629

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Reported-by: Paolo Valerio <pvalerio@redhat.com>
Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
v2: Change the always GOOD checksum to UNKOWN if BAD.
---
 doc/guides/nics/ixgbe.rst              |  7 ++++++
 drivers/net/ixgbe/ixgbe_rxtx.c         | 30 ++++++++++++++++++++----
 drivers/net/ixgbe/ixgbe_rxtx.h         |  2 ++
 drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c | 32 +++++++++++++++++++++++---
 4 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/doc/guides/nics/ixgbe.rst b/doc/guides/nics/ixgbe.rst
index 696cbd93ba..41b07b9aff 100644
--- a/doc/guides/nics/ixgbe.rst
+++ b/doc/guides/nics/ixgbe.rst
@@ -287,6 +287,13 @@ the VFs which are required.::
 Currently hot-plugging of representor ports is not supported so all required
 representors must be specified on the creation of the PF.
 
+Limitations or Known issues
+---------------------------
+The 82599 hardware errata: UDP frames with a zero checksum can be marked as
+checksum errors. To support UDP zero checksum, the bad UDP checksum is marked
+as PKT_RX_L4_CKSUM_UNKNOWN, so the application will recompute the checksum to
+validate it again.
+
 Supported Chipsets and NICs
 ---------------------------
 
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 72d27f35ca..950b7894e0 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -1466,7 +1466,8 @@ rx_desc_status_to_pkt_flags(uint32_t rx_status, uint64_t vlan_flags)
 }
 
 static inline uint64_t
-rx_desc_error_to_pkt_flags(uint32_t rx_status)
+rx_desc_error_to_pkt_flags(uint32_t rx_status, uint16_t pkt_info,
+			   uint8_t rx_udp_csum_zero_err)
 {
 	uint64_t pkt_flags;
 
@@ -1483,6 +1484,15 @@ rx_desc_error_to_pkt_flags(uint32_t rx_status)
 	pkt_flags = error_to_pkt_flags_map[(rx_status >>
 		IXGBE_RXDADV_ERR_CKSUM_BIT) & IXGBE_RXDADV_ERR_CKSUM_MSK];
 
+	/* Mask out the bad UDP checksum error if the hardware has UDP zero
+	 * checksum error issue, so that the software application will then
+	 * have to recompute the checksum itself if needed.
+	 */
+	if ((rx_status & IXGBE_RXDADV_ERR_TCPE) &&
+	    (pkt_info & IXGBE_RXDADV_PKTTYPE_UDP) &&
+	    rx_udp_csum_zero_err)
+		pkt_flags &= ~PKT_RX_L4_CKSUM_BAD;
+
 	if ((rx_status & IXGBE_RXD_STAT_OUTERIPCS) &&
 	    (rx_status & IXGBE_RXDADV_ERR_OUTERIPER)) {
 		pkt_flags |= PKT_RX_EIP_CKSUM_BAD;
@@ -1569,7 +1579,9 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
 			/* convert descriptor fields to rte mbuf flags */
 			pkt_flags = rx_desc_status_to_pkt_flags(s[j],
 				vlan_flags);
-			pkt_flags |= rx_desc_error_to_pkt_flags(s[j]);
+			pkt_flags |= rx_desc_error_to_pkt_flags(s[j],
+					(uint16_t)pkt_info[j],
+					rxq->rx_udp_csum_zero_err);
 			pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags
 					((uint16_t)pkt_info[j]);
 			mb->ol_flags = pkt_flags;
@@ -1902,7 +1914,9 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		rxm->vlan_tci = rte_le_to_cpu_16(rxd.wb.upper.vlan);
 
 		pkt_flags = rx_desc_status_to_pkt_flags(staterr, vlan_flags);
-		pkt_flags = pkt_flags | rx_desc_error_to_pkt_flags(staterr);
+		pkt_flags = pkt_flags |
+			rx_desc_error_to_pkt_flags(staterr, (uint16_t)pkt_info,
+						   rxq->rx_udp_csum_zero_err);
 		pkt_flags = pkt_flags |
 			ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info);
 		rxm->ol_flags = pkt_flags;
@@ -1995,7 +2009,8 @@ ixgbe_fill_cluster_head_buf(
 	head->vlan_tci = rte_le_to_cpu_16(desc->wb.upper.vlan);
 	pkt_info = rte_le_to_cpu_32(desc->wb.lower.lo_dword.data);
 	pkt_flags = rx_desc_status_to_pkt_flags(staterr, rxq->vlan_flags);
-	pkt_flags |= rx_desc_error_to_pkt_flags(staterr);
+	pkt_flags |= rx_desc_error_to_pkt_flags(staterr, (uint16_t)pkt_info,
+						rxq->rx_udp_csum_zero_err);
 	pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info);
 	head->ol_flags = pkt_flags;
 	head->packet_type =
@@ -3116,6 +3131,13 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	else
 		rxq->pkt_type_mask = IXGBE_PACKET_TYPE_MASK_82599;
 
+	/*
+	 * 82599 errata, UDP frames with a 0 checksum can be marked as checksum
+	 * errors.
+	 */
+	if (hw->mac.type == ixgbe_mac_82599EB)
+		rxq->rx_udp_csum_zero_err = 1;
+
 	/*
 	 * Allocate RX ring hardware descriptors. A memzone large enough to
 	 * handle the maximum ring size is allocated in order to allow for
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
index 8a25e98df6..476ef62cfd 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.h
+++ b/drivers/net/ixgbe/ixgbe_rxtx.h
@@ -129,6 +129,8 @@ struct ixgbe_rx_queue {
 	uint8_t             crc_len;  /**< 0 if CRC stripped, 4 otherwise. */
 	uint8_t             drop_en;  /**< If not 0, set SRRCTL.Drop_En. */
 	uint8_t             rx_deferred_start; /**< not in global dev start. */
+	/** UDP frames with a 0 checksum can be marked as checksum errors. */
+	uint8_t             rx_udp_csum_zero_err;
 	/** flags to set in mbuf when a vlan is detected. */
 	uint64_t            vlan_flags;
 	uint64_t	    offloads; /**< Rx offloads with DEV_RX_OFFLOAD_* */
diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
index 9bbffe6119..7610fd93db 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
@@ -132,9 +132,9 @@ desc_to_olflags_v_ipsec(__m128i descs[4], struct rte_mbuf **rx_pkts)
 
 static inline void
 desc_to_olflags_v(__m128i descs[4], __m128i mbuf_init, uint8_t vlan_flags,
-	struct rte_mbuf **rx_pkts)
+		  uint16_t udp_p_flag, struct rte_mbuf **rx_pkts)
 {
-	__m128i ptype0, ptype1, vtag0, vtag1, csum;
+	__m128i ptype0, ptype1, vtag0, vtag1, csum, udp_csum_skip;
 	__m128i rearm0, rearm1, rearm2, rearm3;
 
 	/* mask everything except rss type */
@@ -161,6 +161,7 @@ desc_to_olflags_v(__m128i descs[4], __m128i mbuf_init, uint8_t vlan_flags,
 		(IXGBE_RXDADV_ERR_TCPE | IXGBE_RXDADV_ERR_IPE) >> 16,
 		IXGBE_RXD_STAT_VP, IXGBE_RXD_STAT_VP,
 		IXGBE_RXD_STAT_VP, IXGBE_RXD_STAT_VP);
+
 	/* map vlan present (0x8), IPE (0x2), L4E (0x1) to ol_flags */
 	const __m128i vlan_csum_map_lo = _mm_set_epi8(
 		0, 0, 0, 0,
@@ -182,12 +183,23 @@ desc_to_olflags_v(__m128i descs[4], __m128i mbuf_init, uint8_t vlan_flags,
 		0, PKT_RX_L4_CKSUM_GOOD >> sizeof(uint8_t), 0,
 		PKT_RX_L4_CKSUM_GOOD >> sizeof(uint8_t));
 
+	/* mask everything except UDP header present if specified */
+	const __m128i udp_hdr_p_msk = _mm_set_epi16
+		(0, 0, 0, 0,
+		 udp_p_flag, udp_p_flag, udp_p_flag, udp_p_flag);
+
+	const __m128i udp_csum_bad_shuf = _mm_set_epi8
+		(0, 0, 0, 0, 0, 0, 0, 0,
+		 0, 0, 0, 0, 0, 0, ~(uint8_t)PKT_RX_L4_CKSUM_BAD, 0xFF);
+
 	ptype0 = _mm_unpacklo_epi16(descs[0], descs[1]);
 	ptype1 = _mm_unpacklo_epi16(descs[2], descs[3]);
 	vtag0 = _mm_unpackhi_epi16(descs[0], descs[1]);
 	vtag1 = _mm_unpackhi_epi16(descs[2], descs[3]);
 
 	ptype0 = _mm_unpacklo_epi32(ptype0, ptype1);
+	/* save the UDP header present information */
+	udp_csum_skip = _mm_and_si128(ptype0, udp_hdr_p_msk);
 	ptype0 = _mm_and_si128(ptype0, rsstype_msk);
 	ptype0 = _mm_shuffle_epi8(rss_flags, ptype0);
 
@@ -215,6 +227,15 @@ desc_to_olflags_v(__m128i descs[4], __m128i mbuf_init, uint8_t vlan_flags,
 
 	vtag1 = _mm_or_si128(ptype0, vtag1);
 
+	/* convert the UDP header present 0x200 to 0x1 for aligning with each
+	 * PKT_RX_L4_CKSUM_BAD value in low byte of 16 bits word ol_flag in
+	 * vtag1 (4x16). Then mask out the bad checksum value by shuffle and
+	 * bit-mask.
+	 */
+	udp_csum_skip = _mm_srli_epi16(udp_csum_skip, 9);
+	udp_csum_skip = _mm_shuffle_epi8(udp_csum_bad_shuf, udp_csum_skip);
+	vtag1 = _mm_and_si128(vtag1, udp_csum_skip);
+
 	/*
 	 * At this point, we have the 4 sets of flags in the low 64-bits
 	 * of vtag1 (4x16).
@@ -341,6 +362,7 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 	__m128i dd_check, eop_check;
 	__m128i mbuf_init;
 	uint8_t vlan_flags;
+	uint16_t udp_p_flag = 0; /* Rx Descriptor UDP header present */
 
 	/* nb_pkts has to be floor-aligned to RTE_IXGBE_DESCS_PER_LOOP */
 	nb_pkts = RTE_ALIGN_FLOOR(nb_pkts, RTE_IXGBE_DESCS_PER_LOOP);
@@ -365,6 +387,9 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 				rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
 		return 0;
 
+	if (rxq->rx_udp_csum_zero_err)
+		udp_p_flag = IXGBE_RXDADV_PKTTYPE_UDP;
+
 	/* 4 packets DD mask */
 	dd_check = _mm_set_epi64x(0x0000000100000001LL, 0x0000000100000001LL);
 
@@ -477,7 +502,8 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 		sterr_tmp1 = _mm_unpackhi_epi32(descs[1], descs[0]);
 
 		/* set ol_flags with vlan packet type */
-		desc_to_olflags_v(descs, mbuf_init, vlan_flags, &rx_pkts[pos]);
+		desc_to_olflags_v(descs, mbuf_init, vlan_flags, udp_p_flag,
+				  &rx_pkts[pos]);
 
 #ifdef RTE_LIB_SECURITY
 		if (unlikely(use_ipsec))
-- 
2.30.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix UDP zero checksum error
  2021-02-03  2:19 ` [dpdk-dev] [PATCH v2] net/ixgbe: fix UDP zero checksum error Haiyue Wang
@ 2021-02-03  8:07   ` Zhang, Qi Z
  2021-02-03  8:32     ` David Marchand
  0 siblings, 1 reply; 20+ messages in thread
From: Zhang, Qi Z @ 2021-02-03  8:07 UTC (permalink / raw)
  To: Wang, Haiyue, dev
  Cc: pvalerio, aconole, david.marchand, Rong, Leyi, Tu, Lijuan,
	Ruifeng.Wang, Feifei.Wang2, stable, Guo, Jia, Richardson, Bruce,
	Ananyev, Konstantin



> -----Original Message-----
> From: Wang, Haiyue <haiyue.wang@intel.com>
> Sent: Wednesday, February 3, 2021 10:19 AM
> To: dev@dpdk.org
> Cc: pvalerio@redhat.com; aconole@redhat.com;
> david.marchand@redhat.com; Zhang, Qi Z <qi.z.zhang@intel.com>; Rong, Leyi
> <leyi.rong@intel.com>; Tu, Lijuan <lijuan.tu@intel.com>;
> Ruifeng.Wang@arm.com; Feifei.Wang2@arm.com; Wang, Haiyue
> <haiyue.wang@intel.com>; stable@dpdk.org; Guo, Jia <jia.guo@intel.com>;
> Richardson, Bruce <bruce.richardson@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Subject: [PATCH v2] net/ixgbe: fix UDP zero checksum error
> 
> There is an 82599 errata that UDP frames with a zero checksum are incorrectly
> marked as checksum invalid by the hardware.  This was leading to misleading
> PKT_RX_L4_CKSUM_BAD flag.
> 
> This patch changes the bad UDP checksum to PKT_RX_L4_CKSUM_UNKNOWN,
> so the software application will then have to recompute the checksum itself if
> needed.
> 
> Bugzilla ID: 629
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Reported-by: Paolo Valerio <pvalerio@redhat.com>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix UDP zero checksum error
  2021-02-03  8:07   ` Zhang, Qi Z
@ 2021-02-03  8:32     ` David Marchand
  2021-02-04  7:45       ` Zhang, Qi Z
  0 siblings, 1 reply; 20+ messages in thread
From: David Marchand @ 2021-02-03  8:32 UTC (permalink / raw)
  To: Zhang, Qi Z
  Cc: Wang, Haiyue, dev, pvalerio, aconole, Rong, Leyi, Tu, Lijuan,
	Ruifeng.Wang, Feifei.Wang2, stable, Guo, Jia, Richardson, Bruce,
	Ananyev, Konstantin

On Wed, Feb 3, 2021 at 9:07 AM Zhang, Qi Z <qi.z.zhang@intel.com> wrote:
> > -----Original Message-----
> > From: Wang, Haiyue <haiyue.wang@intel.com>
> > Sent: Wednesday, February 3, 2021 10:19 AM
> > To: dev@dpdk.org
> > Cc: pvalerio@redhat.com; aconole@redhat.com;
> > david.marchand@redhat.com; Zhang, Qi Z <qi.z.zhang@intel.com>; Rong, Leyi
> > <leyi.rong@intel.com>; Tu, Lijuan <lijuan.tu@intel.com>;
> > Ruifeng.Wang@arm.com; Feifei.Wang2@arm.com; Wang, Haiyue
> > <haiyue.wang@intel.com>; stable@dpdk.org; Guo, Jia <jia.guo@intel.com>;
> > Richardson, Bruce <bruce.richardson@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>
> > Subject: [PATCH v2] net/ixgbe: fix UDP zero checksum error
> >
> > There is an 82599 errata that UDP frames with a zero checksum are incorrectly
> > marked as checksum invalid by the hardware.  This was leading to misleading
> > PKT_RX_L4_CKSUM_BAD flag.
> >
> > This patch changes the bad UDP checksum to PKT_RX_L4_CKSUM_UNKNOWN,
> > so the software application will then have to recompute the checksum itself if
> > needed.
> >
> > Bugzilla ID: 629
> >
> > Fixes: af75078fece3 ("first public release")
> > Cc: stable@dpdk.org
> >
> > Reported-by: Paolo Valerio <pvalerio@redhat.com>
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
>
> Applied to dpdk-next-net-intel.

Why the rush for applying?
The ARM part is not done, Paolo said he would test the patch and I am
pretty sure a review can't hurt.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix UDP zero checksum error
  2021-02-03  8:32     ` David Marchand
@ 2021-02-04  7:45       ` Zhang, Qi Z
  0 siblings, 0 replies; 20+ messages in thread
From: Zhang, Qi Z @ 2021-02-04  7:45 UTC (permalink / raw)
  To: David Marchand
  Cc: Wang, Haiyue, dev, pvalerio, aconole, Rong, Leyi, Tu, Lijuan,
	Ruifeng.Wang, Feifei.Wang2, stable, Guo, Jia, Richardson, Bruce,
	Ananyev, Konstantin



> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Wednesday, February 3, 2021 4:32 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org;
> pvalerio@redhat.com; aconole@redhat.com; Rong, Leyi
> <leyi.rong@intel.com>; Tu, Lijuan <lijuan.tu@intel.com>;
> Ruifeng.Wang@arm.com; Feifei.Wang2@arm.com; stable@dpdk.org; Guo, Jia
> <jia.guo@intel.com>; Richardson, Bruce <bruce.richardson@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Subject: Re: [PATCH v2] net/ixgbe: fix UDP zero checksum error
> 
> On Wed, Feb 3, 2021 at 9:07 AM Zhang, Qi Z <qi.z.zhang@intel.com> wrote:
> > > -----Original Message-----
> > > From: Wang, Haiyue <haiyue.wang@intel.com>
> > > Sent: Wednesday, February 3, 2021 10:19 AM
> > > To: dev@dpdk.org
> > > Cc: pvalerio@redhat.com; aconole@redhat.com;
> > > david.marchand@redhat.com; Zhang, Qi Z <qi.z.zhang@intel.com>; Rong,
> > > Leyi <leyi.rong@intel.com>; Tu, Lijuan <lijuan.tu@intel.com>;
> > > Ruifeng.Wang@arm.com; Feifei.Wang2@arm.com; Wang, Haiyue
> > > <haiyue.wang@intel.com>; stable@dpdk.org; Guo, Jia
> > > <jia.guo@intel.com>; Richardson, Bruce <bruce.richardson@intel.com>;
> > > Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > > Subject: [PATCH v2] net/ixgbe: fix UDP zero checksum error
> > >
> > > There is an 82599 errata that UDP frames with a zero checksum are
> > > incorrectly marked as checksum invalid by the hardware.  This was
> > > leading to misleading PKT_RX_L4_CKSUM_BAD flag.
> > >
> > > This patch changes the bad UDP checksum to
> PKT_RX_L4_CKSUM_UNKNOWN,
> > > so the software application will then have to recompute the checksum
> > > itself if needed.
> > >
> > > Bugzilla ID: 629
> > >
> > > Fixes: af75078fece3 ("first public release")
> > > Cc: stable@dpdk.org
> > >
> > > Reported-by: Paolo Valerio <pvalerio@redhat.com>
> > > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> >
> > Applied to dpdk-next-net-intel.
> 
> Why the rush for applying?
> The ARM part is not done, Paolo said he would test the patch and I am pretty
> sure a review can't hurt.
> 
> 
> --
> David Marchand

Oops, it should be a mistake, the patch has not been acked and reviewed yet.

Thanks for capture, will revert.
Qi


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [dpdk-dev] [PATCH v3] net/ixgbe: fix UDP zero checksum error
  2021-02-02  7:06 [dpdk-dev] [PATCH v1] net/ixgbe: adjust error for UDP with zero checksum Haiyue Wang
  2021-02-02  9:45 ` David Marchand
  2021-02-03  2:19 ` [dpdk-dev] [PATCH v2] net/ixgbe: fix UDP zero checksum error Haiyue Wang
@ 2021-02-04 14:39 ` Haiyue Wang
  2021-02-05 17:50   ` Paolo Valerio
                     ` (3 more replies)
  2 siblings, 4 replies; 20+ messages in thread
From: Haiyue Wang @ 2021-02-04 14:39 UTC (permalink / raw)
  To: dev
  Cc: pvalerio, aconole, david.marchand, qi.z.zhang, leyi.rong,
	Lijuan.Tu, Ruifeng.Wang, Feifei.Wang2, Haiyue Wang, stable,
	Jeff Guo, Bruce Richardson, Konstantin Ananyev

There is an 82599 errata that UDP frames with a zero checksum are
incorrectly marked as checksum invalid by the hardware.  This was
leading to misleading PKT_RX_L4_CKSUM_BAD flag.

This patch changes the bad UDP checksum to PKT_RX_L4_CKSUM_UNKNOWN,
so the software application will then have to recompute the checksum
itself if needed.

Bugzilla ID: 629
Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Reported-by: Paolo Valerio <pvalerio@redhat.com>
Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
v3: Update the hardware errata doc name and session
v2: Change the always GOOD checksum to UNKOWN if BAD.
---
 doc/guides/nics/ixgbe.rst              | 10 ++++++++
 drivers/net/ixgbe/ixgbe_rxtx.c         | 30 ++++++++++++++++++++----
 drivers/net/ixgbe/ixgbe_rxtx.h         |  2 ++
 drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c | 32 +++++++++++++++++++++++---
 4 files changed, 67 insertions(+), 7 deletions(-)

diff --git a/doc/guides/nics/ixgbe.rst b/doc/guides/nics/ixgbe.rst
index 696cbd93ba..4c644c0e68 100644
--- a/doc/guides/nics/ixgbe.rst
+++ b/doc/guides/nics/ixgbe.rst
@@ -257,6 +257,16 @@ RSS isn't supported when QinQ is enabled
 
 Due to FW limitation, IXGBE doesn't support RSS when QinQ is enabled currently.
 
+UDP with zero checksum is reported as error
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Intel 82599 10 Gigabit Ethernet Controller Specification Update (Revision 2.87)
+Errata: 44 Integrity Error Reported for IPv4/UDP Packets With Zero Checksum
+
+To support UDP zero checksum, the zero and bad UDP checksum packet is marked as
+PKT_RX_L4_CKSUM_UNKNOWN, so the application needs to recompute the checksum to
+validate it.
+
 Inline crypto processing support
 --------------------------------
 
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 72d27f35ca..950b7894e0 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -1466,7 +1466,8 @@ rx_desc_status_to_pkt_flags(uint32_t rx_status, uint64_t vlan_flags)
 }
 
 static inline uint64_t
-rx_desc_error_to_pkt_flags(uint32_t rx_status)
+rx_desc_error_to_pkt_flags(uint32_t rx_status, uint16_t pkt_info,
+			   uint8_t rx_udp_csum_zero_err)
 {
 	uint64_t pkt_flags;
 
@@ -1483,6 +1484,15 @@ rx_desc_error_to_pkt_flags(uint32_t rx_status)
 	pkt_flags = error_to_pkt_flags_map[(rx_status >>
 		IXGBE_RXDADV_ERR_CKSUM_BIT) & IXGBE_RXDADV_ERR_CKSUM_MSK];
 
+	/* Mask out the bad UDP checksum error if the hardware has UDP zero
+	 * checksum error issue, so that the software application will then
+	 * have to recompute the checksum itself if needed.
+	 */
+	if ((rx_status & IXGBE_RXDADV_ERR_TCPE) &&
+	    (pkt_info & IXGBE_RXDADV_PKTTYPE_UDP) &&
+	    rx_udp_csum_zero_err)
+		pkt_flags &= ~PKT_RX_L4_CKSUM_BAD;
+
 	if ((rx_status & IXGBE_RXD_STAT_OUTERIPCS) &&
 	    (rx_status & IXGBE_RXDADV_ERR_OUTERIPER)) {
 		pkt_flags |= PKT_RX_EIP_CKSUM_BAD;
@@ -1569,7 +1579,9 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
 			/* convert descriptor fields to rte mbuf flags */
 			pkt_flags = rx_desc_status_to_pkt_flags(s[j],
 				vlan_flags);
-			pkt_flags |= rx_desc_error_to_pkt_flags(s[j]);
+			pkt_flags |= rx_desc_error_to_pkt_flags(s[j],
+					(uint16_t)pkt_info[j],
+					rxq->rx_udp_csum_zero_err);
 			pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags
 					((uint16_t)pkt_info[j]);
 			mb->ol_flags = pkt_flags;
@@ -1902,7 +1914,9 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		rxm->vlan_tci = rte_le_to_cpu_16(rxd.wb.upper.vlan);
 
 		pkt_flags = rx_desc_status_to_pkt_flags(staterr, vlan_flags);
-		pkt_flags = pkt_flags | rx_desc_error_to_pkt_flags(staterr);
+		pkt_flags = pkt_flags |
+			rx_desc_error_to_pkt_flags(staterr, (uint16_t)pkt_info,
+						   rxq->rx_udp_csum_zero_err);
 		pkt_flags = pkt_flags |
 			ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info);
 		rxm->ol_flags = pkt_flags;
@@ -1995,7 +2009,8 @@ ixgbe_fill_cluster_head_buf(
 	head->vlan_tci = rte_le_to_cpu_16(desc->wb.upper.vlan);
 	pkt_info = rte_le_to_cpu_32(desc->wb.lower.lo_dword.data);
 	pkt_flags = rx_desc_status_to_pkt_flags(staterr, rxq->vlan_flags);
-	pkt_flags |= rx_desc_error_to_pkt_flags(staterr);
+	pkt_flags |= rx_desc_error_to_pkt_flags(staterr, (uint16_t)pkt_info,
+						rxq->rx_udp_csum_zero_err);
 	pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info);
 	head->ol_flags = pkt_flags;
 	head->packet_type =
@@ -3116,6 +3131,13 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	else
 		rxq->pkt_type_mask = IXGBE_PACKET_TYPE_MASK_82599;
 
+	/*
+	 * 82599 errata, UDP frames with a 0 checksum can be marked as checksum
+	 * errors.
+	 */
+	if (hw->mac.type == ixgbe_mac_82599EB)
+		rxq->rx_udp_csum_zero_err = 1;
+
 	/*
 	 * Allocate RX ring hardware descriptors. A memzone large enough to
 	 * handle the maximum ring size is allocated in order to allow for
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
index 8a25e98df6..476ef62cfd 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.h
+++ b/drivers/net/ixgbe/ixgbe_rxtx.h
@@ -129,6 +129,8 @@ struct ixgbe_rx_queue {
 	uint8_t             crc_len;  /**< 0 if CRC stripped, 4 otherwise. */
 	uint8_t             drop_en;  /**< If not 0, set SRRCTL.Drop_En. */
 	uint8_t             rx_deferred_start; /**< not in global dev start. */
+	/** UDP frames with a 0 checksum can be marked as checksum errors. */
+	uint8_t             rx_udp_csum_zero_err;
 	/** flags to set in mbuf when a vlan is detected. */
 	uint64_t            vlan_flags;
 	uint64_t	    offloads; /**< Rx offloads with DEV_RX_OFFLOAD_* */
diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
index 9bbffe6119..7610fd93db 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
@@ -132,9 +132,9 @@ desc_to_olflags_v_ipsec(__m128i descs[4], struct rte_mbuf **rx_pkts)
 
 static inline void
 desc_to_olflags_v(__m128i descs[4], __m128i mbuf_init, uint8_t vlan_flags,
-	struct rte_mbuf **rx_pkts)
+		  uint16_t udp_p_flag, struct rte_mbuf **rx_pkts)
 {
-	__m128i ptype0, ptype1, vtag0, vtag1, csum;
+	__m128i ptype0, ptype1, vtag0, vtag1, csum, udp_csum_skip;
 	__m128i rearm0, rearm1, rearm2, rearm3;
 
 	/* mask everything except rss type */
@@ -161,6 +161,7 @@ desc_to_olflags_v(__m128i descs[4], __m128i mbuf_init, uint8_t vlan_flags,
 		(IXGBE_RXDADV_ERR_TCPE | IXGBE_RXDADV_ERR_IPE) >> 16,
 		IXGBE_RXD_STAT_VP, IXGBE_RXD_STAT_VP,
 		IXGBE_RXD_STAT_VP, IXGBE_RXD_STAT_VP);
+
 	/* map vlan present (0x8), IPE (0x2), L4E (0x1) to ol_flags */
 	const __m128i vlan_csum_map_lo = _mm_set_epi8(
 		0, 0, 0, 0,
@@ -182,12 +183,23 @@ desc_to_olflags_v(__m128i descs[4], __m128i mbuf_init, uint8_t vlan_flags,
 		0, PKT_RX_L4_CKSUM_GOOD >> sizeof(uint8_t), 0,
 		PKT_RX_L4_CKSUM_GOOD >> sizeof(uint8_t));
 
+	/* mask everything except UDP header present if specified */
+	const __m128i udp_hdr_p_msk = _mm_set_epi16
+		(0, 0, 0, 0,
+		 udp_p_flag, udp_p_flag, udp_p_flag, udp_p_flag);
+
+	const __m128i udp_csum_bad_shuf = _mm_set_epi8
+		(0, 0, 0, 0, 0, 0, 0, 0,
+		 0, 0, 0, 0, 0, 0, ~(uint8_t)PKT_RX_L4_CKSUM_BAD, 0xFF);
+
 	ptype0 = _mm_unpacklo_epi16(descs[0], descs[1]);
 	ptype1 = _mm_unpacklo_epi16(descs[2], descs[3]);
 	vtag0 = _mm_unpackhi_epi16(descs[0], descs[1]);
 	vtag1 = _mm_unpackhi_epi16(descs[2], descs[3]);
 
 	ptype0 = _mm_unpacklo_epi32(ptype0, ptype1);
+	/* save the UDP header present information */
+	udp_csum_skip = _mm_and_si128(ptype0, udp_hdr_p_msk);
 	ptype0 = _mm_and_si128(ptype0, rsstype_msk);
 	ptype0 = _mm_shuffle_epi8(rss_flags, ptype0);
 
@@ -215,6 +227,15 @@ desc_to_olflags_v(__m128i descs[4], __m128i mbuf_init, uint8_t vlan_flags,
 
 	vtag1 = _mm_or_si128(ptype0, vtag1);
 
+	/* convert the UDP header present 0x200 to 0x1 for aligning with each
+	 * PKT_RX_L4_CKSUM_BAD value in low byte of 16 bits word ol_flag in
+	 * vtag1 (4x16). Then mask out the bad checksum value by shuffle and
+	 * bit-mask.
+	 */
+	udp_csum_skip = _mm_srli_epi16(udp_csum_skip, 9);
+	udp_csum_skip = _mm_shuffle_epi8(udp_csum_bad_shuf, udp_csum_skip);
+	vtag1 = _mm_and_si128(vtag1, udp_csum_skip);
+
 	/*
 	 * At this point, we have the 4 sets of flags in the low 64-bits
 	 * of vtag1 (4x16).
@@ -341,6 +362,7 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 	__m128i dd_check, eop_check;
 	__m128i mbuf_init;
 	uint8_t vlan_flags;
+	uint16_t udp_p_flag = 0; /* Rx Descriptor UDP header present */
 
 	/* nb_pkts has to be floor-aligned to RTE_IXGBE_DESCS_PER_LOOP */
 	nb_pkts = RTE_ALIGN_FLOOR(nb_pkts, RTE_IXGBE_DESCS_PER_LOOP);
@@ -365,6 +387,9 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 				rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
 		return 0;
 
+	if (rxq->rx_udp_csum_zero_err)
+		udp_p_flag = IXGBE_RXDADV_PKTTYPE_UDP;
+
 	/* 4 packets DD mask */
 	dd_check = _mm_set_epi64x(0x0000000100000001LL, 0x0000000100000001LL);
 
@@ -477,7 +502,8 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 		sterr_tmp1 = _mm_unpackhi_epi32(descs[1], descs[0]);
 
 		/* set ol_flags with vlan packet type */
-		desc_to_olflags_v(descs, mbuf_init, vlan_flags, &rx_pkts[pos]);
+		desc_to_olflags_v(descs, mbuf_init, vlan_flags, udp_p_flag,
+				  &rx_pkts[pos]);
 
 #ifdef RTE_LIB_SECURITY
 		if (unlikely(use_ipsec))
-- 
2.30.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v3] net/ixgbe: fix UDP zero checksum error
  2021-02-04 14:39 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
@ 2021-02-05 17:50   ` Paolo Valerio
  2021-02-07 18:40   ` Paolo Valerio
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 20+ messages in thread
From: Paolo Valerio @ 2021-02-05 17:50 UTC (permalink / raw)
  To: Haiyue Wang, dev
  Cc: aconole, david.marchand, qi.z.zhang, leyi.rong, Lijuan.Tu,
	Ruifeng.Wang, Feifei.Wang2, Haiyue Wang, stable, Jeff Guo,
	Bruce Richardson, Konstantin Ananyev

Hi Haiyue,

Haiyue Wang <haiyue.wang@intel.com> writes:

> There is an 82599 errata that UDP frames with a zero checksum are
> incorrectly marked as checksum invalid by the hardware.  This was
> leading to misleading PKT_RX_L4_CKSUM_BAD flag.
>
> This patch changes the bad UDP checksum to PKT_RX_L4_CKSUM_UNKNOWN,
> so the software application will then have to recompute the checksum
> itself if needed.
>
> Bugzilla ID: 629
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
>
> Reported-by: Paolo Valerio <pvalerio@redhat.com>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> ---
> v3: Update the hardware errata doc name and session
> v2: Change the always GOOD checksum to UNKOWN if BAD.
> ---
>  doc/guides/nics/ixgbe.rst              | 10 ++++++++
>  drivers/net/ixgbe/ixgbe_rxtx.c         | 30 ++++++++++++++++++++----
>  drivers/net/ixgbe/ixgbe_rxtx.h         |  2 ++
>  drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c | 32 +++++++++++++++++++++++---
>  4 files changed, 67 insertions(+), 7 deletions(-)

Tested it successfully with OvS ct() action and testpmd.

Thanks,
Paolo


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v3] net/ixgbe: fix UDP zero checksum error
  2021-02-04 14:39 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
  2021-02-05 17:50   ` Paolo Valerio
@ 2021-02-07 18:40   ` Paolo Valerio
  2021-02-08 13:48   ` Ananyev, Konstantin
  2021-02-25 10:54   ` [dpdk-dev] [dpdk-stable] " David Marchand
  3 siblings, 0 replies; 20+ messages in thread
From: Paolo Valerio @ 2021-02-07 18:40 UTC (permalink / raw)
  To: Haiyue Wang, dev
  Cc: aconole, david.marchand, qi.z.zhang, leyi.rong, Lijuan.Tu,
	Ruifeng.Wang, Feifei.Wang2, Haiyue Wang, stable, Jeff Guo,
	Bruce Richardson, Konstantin Ananyev

Haiyue Wang <haiyue.wang@intel.com> writes:

> There is an 82599 errata that UDP frames with a zero checksum are
> incorrectly marked as checksum invalid by the hardware.  This was
> leading to misleading PKT_RX_L4_CKSUM_BAD flag.
>
> This patch changes the bad UDP checksum to PKT_RX_L4_CKSUM_UNKNOWN,
> so the software application will then have to recompute the checksum
> itself if needed.
>
> Bugzilla ID: 629
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
>
> Reported-by: Paolo Valerio <pvalerio@redhat.com>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> ---
> v3: Update the hardware errata doc name and session
> v2: Change the always GOOD checksum to UNKOWN if BAD.
> ---
>  doc/guides/nics/ixgbe.rst              | 10 ++++++++
>  drivers/net/ixgbe/ixgbe_rxtx.c         | 30 ++++++++++++++++++++----
>  drivers/net/ixgbe/ixgbe_rxtx.h         |  2 ++
>  drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c | 32 +++++++++++++++++++++++---
>  4 files changed, 67 insertions(+), 7 deletions(-)
>

Missed the tag in my previous email

Tested-by: Paolo Valerio <pvalerio@redhat.com>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v3] net/ixgbe: fix UDP zero checksum error
  2021-02-04 14:39 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
  2021-02-05 17:50   ` Paolo Valerio
  2021-02-07 18:40   ` Paolo Valerio
@ 2021-02-08 13:48   ` Ananyev, Konstantin
  2021-02-09  1:23     ` Zhang, Qi Z
  2021-02-25 10:54   ` [dpdk-dev] [dpdk-stable] " David Marchand
  3 siblings, 1 reply; 20+ messages in thread
From: Ananyev, Konstantin @ 2021-02-08 13:48 UTC (permalink / raw)
  To: Wang, Haiyue, dev
  Cc: pvalerio, aconole, david.marchand, Zhang, Qi Z, Rong, Leyi, Tu,
	Lijuan, Ruifeng.Wang, Feifei.Wang2, stable, Guo, Jia, Richardson,
	Bruce

> 
> There is an 82599 errata that UDP frames with a zero checksum are
> incorrectly marked as checksum invalid by the hardware.  This was
> leading to misleading PKT_RX_L4_CKSUM_BAD flag.
> 
> This patch changes the bad UDP checksum to PKT_RX_L4_CKSUM_UNKNOWN,
> so the software application will then have to recompute the checksum
> itself if needed.
> 
> Bugzilla ID: 629
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Reported-by: Paolo Valerio <pvalerio@redhat.com>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> ---
> v3: Update the hardware errata doc name and session
> v2: Change the always GOOD checksum to UNKOWN if BAD.
> ---

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 2.30.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v3] net/ixgbe: fix UDP zero checksum error
  2021-02-08 13:48   ` Ananyev, Konstantin
@ 2021-02-09  1:23     ` Zhang, Qi Z
  0 siblings, 0 replies; 20+ messages in thread
From: Zhang, Qi Z @ 2021-02-09  1:23 UTC (permalink / raw)
  To: Ananyev, Konstantin, Wang, Haiyue, dev
  Cc: pvalerio, aconole, david.marchand, Rong, Leyi, Tu, Lijuan,
	Ruifeng.Wang, Feifei.Wang2, stable, Guo, Jia, Richardson, Bruce



> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Monday, February 8, 2021 9:48 PM
> To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org
> Cc: pvalerio@redhat.com; aconole@redhat.com;
> david.marchand@redhat.com; Zhang, Qi Z <qi.z.zhang@intel.com>; Rong, Leyi
> <leyi.rong@intel.com>; Tu, Lijuan <lijuan.tu@intel.com>;
> Ruifeng.Wang@arm.com; Feifei.Wang2@arm.com; stable@dpdk.org; Guo, Jia
> <jia.guo@intel.com>; Richardson, Bruce <bruce.richardson@intel.com>
> Subject: RE: [PATCH v3] net/ixgbe: fix UDP zero checksum error
> 
> >
> > There is an 82599 errata that UDP frames with a zero checksum are
> > incorrectly marked as checksum invalid by the hardware.  This was
> > leading to misleading PKT_RX_L4_CKSUM_BAD flag.
> >
> > This patch changes the bad UDP checksum to
> PKT_RX_L4_CKSUM_UNKNOWN, so
> > the software application will then have to recompute the checksum
> > itself if needed.
> >
> > Bugzilla ID: 629
> > Fixes: af75078fece3 ("first public release")
> > Cc: stable@dpdk.org
> >
> > Reported-by: Paolo Valerio <pvalerio@redhat.com>
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > ---
> > v3: Update the hardware errata doc name and session
> > v2: Change the always GOOD checksum to UNKOWN if BAD.
> > ---
> 
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> 
> > 2.30.0
> 

Applied to dpdk-next-net-intel.

Thanks
Qi

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH v3] net/ixgbe: fix UDP zero checksum error
  2021-02-04 14:39 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
                     ` (2 preceding siblings ...)
  2021-02-08 13:48   ` Ananyev, Konstantin
@ 2021-02-25 10:54   ` David Marchand
  2021-04-07 11:53     ` David Marchand
  3 siblings, 1 reply; 20+ messages in thread
From: David Marchand @ 2021-02-25 10:54 UTC (permalink / raw)
  To: Haiyue Wang
  Cc: dev, Paolo Valerio, Aaron Conole, Qi Zhang, Leyi Rong, Tu,
	Lijuan, Ruifeng Wang (Arm Technology China),
	Feifei Wang, dpdk stable, Jeff Guo, Bruce Richardson,
	Konstantin Ananyev, Van Haaren Harry, Thomas Monjalon

Hello Haiyue,

On Thu, Feb 4, 2021 at 3:56 PM Haiyue Wang <haiyue.wang@intel.com> wrote:
>
> There is an 82599 errata that UDP frames with a zero checksum are
> incorrectly marked as checksum invalid by the hardware.  This was
> leading to misleading PKT_RX_L4_CKSUM_BAD flag.
>
> This patch changes the bad UDP checksum to PKT_RX_L4_CKSUM_UNKNOWN,
> so the software application will then have to recompute the checksum
> itself if needed.
>
> Bugzilla ID: 629
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
>
> Reported-by: Paolo Valerio <pvalerio@redhat.com>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>

There was a previous mention of this issue in the past and it resulted
in dropping part of the hw statistics.
https://git.dpdk.org/dpdk/commit/?id=256ff05a9cae7484e2197cde4401dfa1f21d5a6f

Does it make sense to restrict this "fix" to 82599 only?


-- 
David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH v3] net/ixgbe: fix UDP zero checksum error
  2021-02-25 10:54   ` [dpdk-dev] [dpdk-stable] " David Marchand
@ 2021-04-07 11:53     ` David Marchand
  2021-04-08  1:12       ` Wang, Haiyue
  0 siblings, 1 reply; 20+ messages in thread
From: David Marchand @ 2021-04-07 11:53 UTC (permalink / raw)
  To: Haiyue Wang, Qi Zhang
  Cc: dev, Paolo Valerio, Aaron Conole, Leyi Rong, Tu, Lijuan,
	Ruifeng Wang (Arm Technology China),
	Feifei Wang, dpdk stable, Jeff Guo, Bruce Richardson,
	Konstantin Ananyev, Van Haaren Harry, Thomas Monjalon, Yigit,
	Ferruh

On Thu, Feb 25, 2021 at 11:54 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> Hello Haiyue,
>
> On Thu, Feb 4, 2021 at 3:56 PM Haiyue Wang <haiyue.wang@intel.com> wrote:
> >
> > There is an 82599 errata that UDP frames with a zero checksum are
> > incorrectly marked as checksum invalid by the hardware.  This was
> > leading to misleading PKT_RX_L4_CKSUM_BAD flag.
> >
> > This patch changes the bad UDP checksum to PKT_RX_L4_CKSUM_UNKNOWN,
> > so the software application will then have to recompute the checksum
> > itself if needed.
> >
> > Bugzilla ID: 629
> > Fixes: af75078fece3 ("first public release")
> > Cc: stable@dpdk.org
> >
> > Reported-by: Paolo Valerio <pvalerio@redhat.com>
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
>
> There was a previous mention of this issue in the past and it resulted
> in dropping part of the hw statistics.
> https://git.dpdk.org/dpdk/commit/?id=256ff05a9cae7484e2197cde4401dfa1f21d5a6f
>
> Does it make sense to restrict this "fix" to 82599 only?

ping.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH v3] net/ixgbe: fix UDP zero checksum error
  2021-04-07 11:53     ` David Marchand
@ 2021-04-08  1:12       ` Wang, Haiyue
  0 siblings, 0 replies; 20+ messages in thread
From: Wang, Haiyue @ 2021-04-08  1:12 UTC (permalink / raw)
  To: David Marchand, Zhang, Qi Z
  Cc: dev, Paolo Valerio, Aaron Conole, Rong, Leyi, Tu, Lijuan,
	Ruifeng Wang (Arm Technology China),
	Feifei Wang, dpdk stable, Guo, Jia, Richardson, Bruce, Ananyev,
	Konstantin, Van Haaren, Harry, Thomas Monjalon, Yigit, Ferruh

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Wednesday, April 7, 2021 19:54
> To: Wang, Haiyue <haiyue.wang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev <dev@dpdk.org>; Paolo Valerio <pvalerio@redhat.com>; Aaron Conole <aconole@redhat.com>; Rong,
> Leyi <leyi.rong@intel.com>; Tu, Lijuan <lijuan.tu@intel.com>; Ruifeng Wang (Arm Technology China)
> <Ruifeng.Wang@arm.com>; Feifei Wang <Feifei.Wang2@arm.com>; dpdk stable <stable@dpdk.org>; Guo, Jia
> <jia.guo@intel.com>; Richardson, Bruce <bruce.richardson@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Van Haaren, Harry <harry.van.haaren@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: Re: [dpdk-stable] [PATCH v3] net/ixgbe: fix UDP zero checksum error
> 
> On Thu, Feb 25, 2021 at 11:54 AM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > Hello Haiyue,
> >
> > On Thu, Feb 4, 2021 at 3:56 PM Haiyue Wang <haiyue.wang@intel.com> wrote:
> > >
> > > There is an 82599 errata that UDP frames with a zero checksum are
> > > incorrectly marked as checksum invalid by the hardware.  This was
> > > leading to misleading PKT_RX_L4_CKSUM_BAD flag.
> > >
> > > This patch changes the bad UDP checksum to PKT_RX_L4_CKSUM_UNKNOWN,
> > > so the software application will then have to recompute the checksum
> > > itself if needed.
> > >
> > > Bugzilla ID: 629
> > > Fixes: af75078fece3 ("first public release")
> > > Cc: stable@dpdk.org
> > >
> > > Reported-by: Paolo Valerio <pvalerio@redhat.com>
> > > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> >
> > There was a previous mention of this issue in the past and it resulted
> > in dropping part of the hw statistics.
> > https://git.dpdk.org/dpdk/commit/?id=256ff05a9cae7484e2197cde4401dfa1f21d5a6f
> >
> > Does it make sense to restrict this "fix" to 82599 only?

Make sense.

> 
> ping.
> 
> 
> --
> David Marchand


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2021-04-08  1:12 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-02  7:06 [dpdk-dev] [PATCH v1] net/ixgbe: adjust error for UDP with zero checksum Haiyue Wang
2021-02-02  9:45 ` David Marchand
2021-02-02 12:42   ` Wang, Haiyue
2021-02-02 12:53     ` David Marchand
2021-02-02 12:56       ` Wang, Haiyue
2021-02-02 14:28         ` Wang, Haiyue
2021-02-02 17:42           ` Paolo Valerio
2021-02-02 12:54   ` Wang, Haiyue
2021-02-03  2:19 ` [dpdk-dev] [PATCH v2] net/ixgbe: fix UDP zero checksum error Haiyue Wang
2021-02-03  8:07   ` Zhang, Qi Z
2021-02-03  8:32     ` David Marchand
2021-02-04  7:45       ` Zhang, Qi Z
2021-02-04 14:39 ` [dpdk-dev] [PATCH v3] " Haiyue Wang
2021-02-05 17:50   ` Paolo Valerio
2021-02-07 18:40   ` Paolo Valerio
2021-02-08 13:48   ` Ananyev, Konstantin
2021-02-09  1:23     ` Zhang, Qi Z
2021-02-25 10:54   ` [dpdk-dev] [dpdk-stable] " David Marchand
2021-04-07 11:53     ` David Marchand
2021-04-08  1:12       ` Wang, Haiyue

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).