> -----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
@@ -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
---------------------------
@@ -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
@@ -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_* */
@@ -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))