From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 07849A04DB; Wed, 2 Dec 2020 07:39:21 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 599D3C9A8; Wed, 2 Dec 2020 07:39:20 +0100 (CET) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 1A8EB2C12 for ; Wed, 2 Dec 2020 07:39:17 +0100 (CET) IronPort-SDR: EHGzWrd6k6w2ZN0gphnTLg3Qr2GvqAhgZJvXXqjTnf0IEMPTCmkxMDvLMC+Xzm0jIXvhZqr9sy bUFbIrruOwpg== X-IronPort-AV: E=McAfee;i="6000,8403,9822"; a="160732609" X-IronPort-AV: E=Sophos;i="5.78,385,1599548400"; d="scan'208";a="160732609" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Dec 2020 22:39:04 -0800 IronPort-SDR: wXkW67DiT8DroLMtyqxhS0a8O41K++43CYfzOmdFkxOtHdr+OabATdrlecHezJq/Rtx0dlYl8t OMyYzgFYknRw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.78,385,1599548400"; d="scan'208";a="367886511" Received: from fmsmsx604.amr.corp.intel.com ([10.18.126.84]) by fmsmga002.fm.intel.com with ESMTP; 01 Dec 2020 22:39:03 -0800 Received: from shsmsx601.ccr.corp.intel.com (10.109.6.141) by fmsmsx604.amr.corp.intel.com (10.18.126.84) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Tue, 1 Dec 2020 22:39:02 -0800 Received: from shsmsx601.ccr.corp.intel.com (10.109.6.141) by SHSMSX601.ccr.corp.intel.com (10.109.6.141) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Wed, 2 Dec 2020 14:39:00 +0800 Received: from shsmsx601.ccr.corp.intel.com ([10.109.6.141]) by SHSMSX601.ccr.corp.intel.com ([10.109.6.141]) with mapi id 15.01.1713.004; Wed, 2 Dec 2020 14:39:00 +0800 From: "Guo, Jia" To: "Yang, MurphyX" , "dev@dpdk.org" CC: "Yang, Qiming" , "Yang, SteveX" , "Xing, Beilei" , "Yang, MurphyX" Thread-Topic: [PATCH] net/i40e: fix incorrect checksum flag of L4 checksum Thread-Index: AQHWuArih4Ybw7BEo0ST8U1hMAMdfanjeZwQ Date: Wed, 2 Dec 2020 06:39:00 +0000 Message-ID: <014569e9fe21419fbd39238ddc3fcd65@intel.com> References: <20201111091112.12606-1-murphyx.yang@intel.com> In-Reply-To: <20201111091112.12606-1-murphyx.yang@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.5.1.3 x-originating-ip: [10.239.127.36] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix incorrect checksum flag of L4 checksum X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi, murphy > -----Original Message----- > From: Murphy Yang > Sent: Wednesday, November 11, 2020 5:11 PM > To: dev@dpdk.org > Cc: Yang, Qiming ; Yang, SteveX > ; Xing, Beilei ; Guo, Jia > ; Yang, MurphyX > Subject: [PATCH] net/i40e: fix incorrect checksum flag of L4 checksum >=20 > When send tunneled packet that inner L4 checksum value is correct, the > test_pmd output log shows 'ol_flags' value is > 'PKT_RX_L4_CKSUM_UNKNOWN', but expected value is > 'PKT_RX_L4_CKSUM_GOOD'. >=20 > Add the 'PKT_RX_L4_CKSUM_GOOD' to 'l3_l4e_flags' for sse and > 'l3_l4_flags_shuf' for avx2 to ensure that the 'ol_flags' can match corre= ct flags. >=20 Seems that 'PKT_RX_L4_CKSUM_GOOD' is previous there but not set correctly, = so maybe it should not say " Add the 'PKT_RX_L4_CKSUM_GOOD' to 'l3_l4e_flags' .... " Add more, could you please to check if the other rx vec path also need it, = such as vec_altivec and vec_neon? > Fixes: 9966a00a0688 ("net/i40e: enable bad checksum flags in vector Rx") > Fixes: dafadd73762e ("net/i40e: add AVX2 Rx function") >=20 > Signed-off-by: Murphy Yang > --- > drivers/net/i40e/i40e_rxtx_vec_avx2.c | 40 ++++++++++++++++----------- > drivers/net/i40e/i40e_rxtx_vec_sse.c | 20 ++++++++------ > 2 files changed, 35 insertions(+), 25 deletions(-) >=20 > diff --git a/drivers/net/i40e/i40e_rxtx_vec_avx2.c > b/drivers/net/i40e/i40e_rxtx_vec_avx2.c > index 7a558fc73a..fe6ec7deef 100644 > --- a/drivers/net/i40e/i40e_rxtx_vec_avx2.c > +++ b/drivers/net/i40e/i40e_rxtx_vec_avx2.c > @@ -342,24 +342,32 @@ _recv_raw_pkts_vec_avx2(struct i40e_rx_queue > *rxq, struct rte_mbuf **rx_pkts, > */ > const __m256i l3_l4_flags_shuf =3D _mm256_set_epi8(0, 0, 0, 0, 0, 0, 0, > 0, > /* shift right 1 bit to make sure it not exceed 255 */ > - (PKT_RX_EIP_CKSUM_BAD | > PKT_RX_L4_CKSUM_BAD | PKT_RX_IP_CKSUM_BAD) >> 1, > - (PKT_RX_IP_CKSUM_GOOD | > PKT_RX_EIP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD) >> 1, > - (PKT_RX_EIP_CKSUM_BAD | > PKT_RX_IP_CKSUM_BAD) >> 1, > - (PKT_RX_IP_CKSUM_GOOD | > PKT_RX_EIP_CKSUM_BAD) >> 1, > - (PKT_RX_L4_CKSUM_BAD | > PKT_RX_IP_CKSUM_BAD) >> 1, > - (PKT_RX_IP_CKSUM_GOOD | > PKT_RX_L4_CKSUM_BAD) >> 1, > - PKT_RX_IP_CKSUM_BAD >> 1, > - (PKT_RX_IP_CKSUM_GOOD | > PKT_RX_L4_CKSUM_GOOD) >> 1, > + (PKT_RX_EIP_CKSUM_BAD | > PKT_RX_L4_CKSUM_BAD | > + PKT_RX_IP_CKSUM_BAD) >> 1, > + (PKT_RX_EIP_CKSUM_BAD | > PKT_RX_L4_CKSUM_BAD | > + PKT_RX_IP_CKSUM_GOOD) >> 1, > + (PKT_RX_EIP_CKSUM_BAD | > PKT_RX_L4_CKSUM_GOOD | > + PKT_RX_IP_CKSUM_BAD) >> 1, > + (PKT_RX_EIP_CKSUM_BAD | > PKT_RX_L4_CKSUM_GOOD | > + PKT_RX_IP_CKSUM_GOOD) >> 1, > + (PKT_RX_L4_CKSUM_BAD | > PKT_RX_IP_CKSUM_BAD) >> 1, > + (PKT_RX_L4_CKSUM_BAD | > PKT_RX_IP_CKSUM_GOOD) >> 1, > + (PKT_RX_L4_CKSUM_GOOD | > PKT_RX_IP_CKSUM_BAD) >> 1, > + (PKT_RX_L4_CKSUM_GOOD | > PKT_RX_IP_CKSUM_GOOD) >> 1, > /* second 128-bits */ > 0, 0, 0, 0, 0, 0, 0, 0, > - (PKT_RX_EIP_CKSUM_BAD | > PKT_RX_L4_CKSUM_BAD | PKT_RX_IP_CKSUM_BAD) >> 1, > - (PKT_RX_IP_CKSUM_GOOD | > PKT_RX_EIP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD) >> 1, > - (PKT_RX_EIP_CKSUM_BAD | > PKT_RX_IP_CKSUM_BAD) >> 1, > - (PKT_RX_IP_CKSUM_GOOD | > PKT_RX_EIP_CKSUM_BAD) >> 1, > - (PKT_RX_L4_CKSUM_BAD | > PKT_RX_IP_CKSUM_BAD) >> 1, > - (PKT_RX_IP_CKSUM_GOOD | > PKT_RX_L4_CKSUM_BAD) >> 1, > - PKT_RX_IP_CKSUM_BAD >> 1, > - (PKT_RX_IP_CKSUM_GOOD | > PKT_RX_L4_CKSUM_GOOD) >> 1); > + (PKT_RX_EIP_CKSUM_BAD | > PKT_RX_L4_CKSUM_BAD | > + PKT_RX_IP_CKSUM_BAD) >> 1, > + (PKT_RX_EIP_CKSUM_BAD | > PKT_RX_L4_CKSUM_BAD | > + PKT_RX_IP_CKSUM_GOOD) >> 1, > + (PKT_RX_EIP_CKSUM_BAD | > PKT_RX_L4_CKSUM_GOOD | > + PKT_RX_IP_CKSUM_BAD) >> 1, > + (PKT_RX_EIP_CKSUM_BAD | > PKT_RX_L4_CKSUM_GOOD | > + PKT_RX_IP_CKSUM_GOOD) >> 1, > + (PKT_RX_L4_CKSUM_BAD | > PKT_RX_IP_CKSUM_BAD) >> 1, > + (PKT_RX_L4_CKSUM_BAD | > PKT_RX_IP_CKSUM_GOOD) >> 1, > + (PKT_RX_L4_CKSUM_GOOD | > PKT_RX_IP_CKSUM_BAD) >> 1, > + (PKT_RX_L4_CKSUM_GOOD | > PKT_RX_IP_CKSUM_GOOD) >> 1); >=20 Could you double check if it is reasonable that the " PKT_RX_EIP_CKSUM_BAD"= is always be set, but no " PKT_RX_EIP_CKSUM_GOOD "? > const __m256i cksum_mask =3D _mm256_set1_epi32( > PKT_RX_IP_CKSUM_GOOD | > PKT_RX_IP_CKSUM_BAD | diff --git a/drivers/net/i40e/i40e_rxtx_vec_sse.c > b/drivers/net/i40e/i40e_rxtx_vec_sse.c > index 4b2b6a28fc..0bcb48e24e 100644 > --- a/drivers/net/i40e/i40e_rxtx_vec_sse.c > +++ b/drivers/net/i40e/i40e_rxtx_vec_sse.c > @@ -254,16 +254,18 @@ desc_to_olflags_v(struct i40e_rx_queue *rxq, > volatile union i40e_rx_desc *rxdp, >=20 > const __m128i l3_l4e_flags =3D _mm_set_epi8(0, 0, 0, 0, 0, 0, 0, 0, > /* shift right 1 bit to make sure it not exceed 255 */ > - (PKT_RX_EIP_CKSUM_BAD | > PKT_RX_L4_CKSUM_BAD | > + (PKT_RX_EIP_CKSUM_BAD | > PKT_RX_L4_CKSUM_BAD | > PKT_RX_IP_CKSUM_BAD) >> 1, > - (PKT_RX_IP_CKSUM_GOOD | > PKT_RX_EIP_CKSUM_BAD | > - PKT_RX_L4_CKSUM_BAD) >> 1, > - (PKT_RX_EIP_CKSUM_BAD | > PKT_RX_IP_CKSUM_BAD) >> 1, > - (PKT_RX_IP_CKSUM_GOOD | > PKT_RX_EIP_CKSUM_BAD) >> 1, > - (PKT_RX_L4_CKSUM_BAD | > PKT_RX_IP_CKSUM_BAD) >> 1, > - (PKT_RX_IP_CKSUM_GOOD | > PKT_RX_L4_CKSUM_BAD) >> 1, > - PKT_RX_IP_CKSUM_BAD >> 1, > - (PKT_RX_IP_CKSUM_GOOD | > PKT_RX_L4_CKSUM_GOOD) >> 1); > + (PKT_RX_EIP_CKSUM_BAD | > PKT_RX_L4_CKSUM_BAD | > + PKT_RX_IP_CKSUM_GOOD) >> 1, > + (PKT_RX_EIP_CKSUM_BAD | > PKT_RX_L4_CKSUM_GOOD | > + PKT_RX_IP_CKSUM_BAD) >> 1, > + (PKT_RX_EIP_CKSUM_BAD | > PKT_RX_L4_CKSUM_GOOD | > + PKT_RX_IP_CKSUM_GOOD) >> 1, > + (PKT_RX_L4_CKSUM_BAD | > PKT_RX_IP_CKSUM_BAD) >> 1, > + (PKT_RX_L4_CKSUM_BAD | > PKT_RX_IP_CKSUM_GOOD) >> 1, > + (PKT_RX_L4_CKSUM_GOOD | > PKT_RX_IP_CKSUM_BAD) >> 1, > + (PKT_RX_L4_CKSUM_GOOD | > PKT_RX_IP_CKSUM_GOOD) >> 1); >=20 > /* Unpack "status" from quadword 1, bits 0:32 */ > vlan0 =3D _mm_unpackhi_epi32(descs[0], descs[1]); > -- > 2.17.1