From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 1A5A02A9 for ; Tue, 9 Dec 2014 03:23:44 +0100 (CET) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP; 08 Dec 2014 18:21:50 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,542,1413270000"; d="scan'208";a="650645298" Received: from pgsmsx104.gar.corp.intel.com ([10.221.44.91]) by orsmga002.jf.intel.com with ESMTP; 08 Dec 2014 18:23:42 -0800 Received: from pgsmsx103.gar.corp.intel.com (10.221.44.82) by PGSMSX104.gar.corp.intel.com (10.221.44.91) with Microsoft SMTP Server (TLS) id 14.3.195.1; Tue, 9 Dec 2014 10:23:41 +0800 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by PGSMSX103.gar.corp.intel.com (10.221.44.82) with Microsoft SMTP Server (TLS) id 14.3.195.1; Tue, 9 Dec 2014 10:23:41 +0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.182]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.110]) with mapi id 14.03.0195.001; Tue, 9 Dec 2014 10:23:39 +0800 From: "Zhang, Helin" To: "Ananyev, Konstantin" , "dev@dpdk.org" Thread-Topic: [PATCH v3] mbuf: fix of enabling all newly added RX error flags Thread-Index: AQHQEPS0haWW4BN5h0WG5HkjEp/AbZyFg8YwgAEHSGA= Date: Tue, 9 Dec 2014 02:23:39 +0000 Message-ID: References: <1417743988-15604-1-git-send-email-helin.zhang@intel.com> <1417829617-7185-1-git-send-email-helin.zhang@intel.com> <2601191342CEEE43887BDE71AB977258213BD9ED@IRSMSX105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB977258213BD9ED@IRSMSX105.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v3] mbuf: fix of enabling all newly added RX error flags X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Dec 2014 02:23:51 -0000 > -----Original Message----- > From: Ananyev, Konstantin > Sent: Monday, December 8, 2014 6:45 PM > To: Zhang, Helin; dev@dpdk.org > Cc: Cao, Waterman; Cao, Min; olivier.matz@6wind.com > Subject: RE: [PATCH v3] mbuf: fix of enabling all newly added RX error fl= ags >=20 > Hi Helin, >=20 > > -----Original Message----- > > From: Zhang, Helin > > Sent: Saturday, December 06, 2014 1:34 AM > > To: dev@dpdk.org > > Cc: Cao, Waterman; Cao, Min; olivier.matz@6wind.com; Ananyev, > > Konstantin; Zhang, Helin > > Subject: [PATCH v3] mbuf: fix of enabling all newly added RX error > > flags > > > > Before redefining mbuf structure, there was lack of free bits in 'ol_fl= ags' > > (32 bits in total) for new RX or TX flags. So it tried to reuse > > existant bits as most as possible, or even assigning 0 to some of bit > > flags. After new mbuf structure defined, there are quite a lot of free > > bits. So those newly added bit flags should be assigned with correct > > and valid bit values, and getting their names should be enabled as > > well. Note that 'RECIP' should be removed, as nowhere will use it. > > 'PKT_RX_ERR' is defined to replace all other error bit flags, e.g. MAC > > error, Oversize error, header buffer overflow error. > > > > Signed-off-by: Helin Zhang > > --- > > lib/librte_mbuf/rte_mbuf.c | 7 ++----- > > lib/librte_mbuf/rte_mbuf.h | 7 ++----- > > lib/librte_pmd_i40e/i40e_rxtx.c | 15 ++++----------- > > 3 files changed, 8 insertions(+), 21 deletions(-) > > > > v2 changes: > > * Removed error flag of 'ECIPE' processing only in both i40e PMD and mb= uf. > All > > other error flags were added back. > > * Assigned error flags with correct and valid values, as their previous= values > > were invalid. > > * Enabled getting all error flag names. > > > > v3 changes: > > * 'PKT_RX_ERR' is defined to replace error bit flags of MAC error, Over= size > > error, header buffer overflow error. > > * Removed assigning values to PKT_TX_* bit flags, as it has already bee= n > done > > in another packet set. > > > > diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c > > index 1b14e02..5e6b5d0 100644 > > --- a/lib/librte_mbuf/rte_mbuf.c > > +++ b/lib/librte_mbuf/rte_mbuf.c > > @@ -210,11 +210,8 @@ const char *rte_get_rx_ol_flag_name(uint64_t > mask) > > case PKT_RX_FDIR: return "PKT_RX_FDIR"; > > case PKT_RX_L4_CKSUM_BAD: return "PKT_RX_L4_CKSUM_BAD"; > > case PKT_RX_IP_CKSUM_BAD: return "PKT_RX_IP_CKSUM_BAD"; > > - /* case PKT_RX_EIP_CKSUM_BAD: return "PKT_RX_EIP_CKSUM_BAD"; */ > > - /* case PKT_RX_OVERSIZE: return "PKT_RX_OVERSIZE"; */ > > - /* case PKT_RX_HBUF_OVERFLOW: return "PKT_RX_HBUF_OVERFLOW"; > */ > > - /* case PKT_RX_RECIP_ERR: return "PKT_RX_RECIP_ERR"; */ > > - /* case PKT_RX_MAC_ERR: return "PKT_RX_MAC_ERR"; */ > > + case PKT_RX_EIP_CKSUM_BAD: return "PKT_RX_EIP_CKSUM_BAD"; > > + case PKT_RX_ERR: return "PKT_RX_ERR"; > > case PKT_RX_IPV4_HDR: return "PKT_RX_IPV4_HDR"; > > case PKT_RX_IPV4_HDR_EXT: return "PKT_RX_IPV4_HDR_EXT"; > > case PKT_RX_IPV6_HDR: return "PKT_RX_IPV6_HDR"; diff --git > > a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index > > efdefc4..5e647a9 100644 > > --- a/lib/librte_mbuf/rte_mbuf.h > > +++ b/lib/librte_mbuf/rte_mbuf.h > > @@ -84,11 +84,6 @@ extern "C" { > > #define PKT_RX_FDIR (1ULL << 2) /**< RX packet with FDIR > match indicate. */ > > #define PKT_RX_L4_CKSUM_BAD (1ULL << 3) /**< L4 cksum of RX pkt. > is > > not OK. */ #define PKT_RX_IP_CKSUM_BAD (1ULL << 4) /**< IP cksum > of > > RX pkt. is not OK. */ -#define PKT_RX_EIP_CKSUM_BAD (0ULL << 0) /**< > External IP header checksum error. */ > > -#define PKT_RX_OVERSIZE (0ULL << 0) /**< Num of desc of an RX > pkt oversize. */ > > -#define PKT_RX_HBUF_OVERFLOW (0ULL << 0) /**< Header buffer > overflow. */ > > -#define PKT_RX_RECIP_ERR (0ULL << 0) /**< Hardware processing > error. */ > > -#define PKT_RX_MAC_ERR (0ULL << 0) /**< MAC error. */ > > #define PKT_RX_IPV4_HDR (1ULL << 5) /**< RX packet with IPv4 > header. */ > > #define PKT_RX_IPV4_HDR_EXT (1ULL << 6) /**< RX packet with > extended IPv4 header. */ > > #define PKT_RX_IPV6_HDR (1ULL << 7) /**< RX packet with IPv6 > header. */ > > @@ -99,6 +94,8 @@ extern "C" { > > #define PKT_RX_TUNNEL_IPV6_HDR (1ULL << 12) /**< RX tunnel packet > with IPv6 header. */ > > #define PKT_RX_FDIR_ID (1ULL << 13) /**< FD id reported if FDIR > match. */ > > #define PKT_RX_FDIR_FLX (1ULL << 14) /**< Flexible bytes reported > if FDIR match. */ > > +#define PKT_RX_EIP_CKSUM_BAD (1ULL << 15) /**< External IP header > checksum error. */ > > +#define PKT_RX_ERR (1ULL << 16) /**< Other errors, e.g. > MAC error. */ > > /* add new RX flags here */ > > > > /* add new TX flags here */ > > diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c > > b/lib/librte_pmd_i40e/i40e_rxtx.c index 2beae3c..5d99bd2 100644 > > --- a/lib/librte_pmd_i40e/i40e_rxtx.c > > +++ b/lib/librte_pmd_i40e/i40e_rxtx.c > > @@ -123,25 +123,18 @@ i40e_rxd_error_to_pkt_flags(uint64_t qword) > > return flags; > > /* If RXE bit set, all other status bits are meaningless */ > > if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_RXE_SHIFT))) { > > - flags |=3D PKT_RX_MAC_ERR; > > + flags |=3D PKT_RX_ERR; > > return flags; > > } > > - > > - /* If RECIPE bit set, all other status indications should be ignored = */ > > - if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_RECIPE_SHIFT))) { > > - flags |=3D PKT_RX_RECIP_ERR; > > - return flags; > > - } > > - if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_HBO_SHIFT))) > > - flags |=3D PKT_RX_HBUF_OVERFLOW; > > if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_IPE_SHIFT))) > > flags |=3D PKT_RX_IP_CKSUM_BAD; > > if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_L4E_SHIFT))) > > flags |=3D PKT_RX_L4_CKSUM_BAD; > > if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_EIPE_SHIFT))) > > flags |=3D PKT_RX_EIP_CKSUM_BAD; > > - if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_OVERSIZE_SHIFT))) > > - flags |=3D PKT_RX_OVERSIZE; > > + if (unlikely(error_bits & ((1 << I40E_RX_DESC_ERROR_OVERSIZE_SHIFT) |= | > > + (1 << I40E_RX_DESC_ERROR_HBO_SHIFT)))) > > + flags |=3D PKT_RX_ERR; >=20 > It should be: > error_bits & (1 << I40E_RX_DESC_ERROR_OVERSIZE_SHIFT | 1 << > I40E_RX_DESC_ERROR_HBO_SHIFT) Ohm, stupid fault! I will fix it right now. >=20 > Another 2 questions: > - Why not to put all these 3 bits processing together: > error_bits & (1 << I40E_RX_DESC_ERROR_OVERSIZE_SHIFT | 1 << > I40E_RX_DESC_ERROR_HBO_SHIFT | 1 << > I40E_RX_DESC_ERROR_RXE_SHIFT) ? If RECIPE, other bits are meaningless, said by EAS. If HBO or OVERSIZE, IPE, L4E, EIPE could be meaningful correct or not. > - Is there any reason why you don't want to put PMD_RX_LOG(DEBUG, ...) fo= r > them? > I think it might be usefull for debugging. For all error bits or just HBO and OVERSIZE? >=20 > Konstantin >=20 >=20 > > > > return flags; > > } > > -- > > 1.9.3 Regards, Helin