From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 759F87E23 for ; Tue, 9 Dec 2014 03:14:38 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga101.fm.intel.com with ESMTP; 08 Dec 2014 18:14:37 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.97,862,1389772800"; d="scan'208";a="426720022" Received: from pgsmsx105.gar.corp.intel.com ([10.221.44.96]) by FMSMGA003.fm.intel.com with ESMTP; 08 Dec 2014 18:04:00 -0800 Received: from kmsmsx154.gar.corp.intel.com (172.21.73.14) by pgsmsx105.gar.corp.intel.com (10.221.44.96) with Microsoft SMTP Server (TLS) id 14.3.195.1; Tue, 9 Dec 2014 10:14:20 +0800 Received: from shsmsx103.ccr.corp.intel.com (10.239.110.14) by KMSMSX154.gar.corp.intel.com (172.21.73.14) with Microsoft SMTP Server (TLS) id 14.3.195.1; Tue, 9 Dec 2014 10:14:20 +0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.182]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.240]) with mapi id 14.03.0195.001; Tue, 9 Dec 2014 10:14:13 +0800 From: "Zhang, Helin" To: Thomas Monjalon Thread-Topic: [dpdk-dev] [PATCH v3] mbuf: fix of enabling all newly added RX error flags Thread-Index: AQHQEtTw36OGroF6YUaHZJ3WjRSbM5yGduvw Date: Tue, 9 Dec 2014 02:14:12 +0000 Message-ID: References: <1417743988-15604-1-git-send-email-helin.zhang@intel.com> <1417829617-7185-1-git-send-email-helin.zhang@intel.com> <5137902.VTrfHmozET@xps13> In-Reply-To: <5137902.VTrfHmozET@xps13> 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 Cc: "dev@dpdk.org" 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:14:39 -0000 Hi Thomas > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Monday, December 8, 2014 6:51 PM > To: Zhang, Helin > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3] mbuf: fix of enabling all newly added = RX > error flags >=20 > Hi Helin, >=20 > 2014-12-06 09:33, Helin Zhang: > > 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 > [...] > > --- 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. */ >=20 > Could PKT_RX_EIP_CKSUM_BAD be aggregated with > PKT_RX_IP_CKSUM_BAD? I tend to say no, but I would listen to comments from others. For tunneling case (e.g. IP over IP), it is a bit similar to the case of L3= /L4 (e.g. UDP over IP). For L3/L4 case, we have PKT_RX_IP_CKSUM_BAD and PKT_RX_L4_CKSUM_BAD to indicate the checksum error is in L3 or L4. So I'd prefer to have PKT_RX_IP_CKSUM_BAD and PKT_RX_EIP_CKSUM_BAD to indic= ate the checksum error is in outer or inner header. Otherwise we have no chance to know where the checksum error is, based on m= buf. > The conclusion is the same: the packet is corrupted. > And some hardwares could not detect the encapsulation and use > PKT_RX_IP_CKSUM_BAD. If the hardware don't know it is a tunneling packet, it will just treat it = as an IP packet. But if hardware supports tunneling, it would be better for apps to know that more = about the packet which can be offloaded by hardware. >=20 > Another interesting improvement would be to have PKT_RX_IP_CKSUM_OK. > I think we'll have to think about this kind of flag for next version. For checksum OK, if no 'BAD' indicated, we can assume it is OK. Any other h= ints from you? >=20 > Note that this patch is an API change and shouldn't be applied for 1.8.0. > But we can do an exception as it has no impact on existing applications a= nd > fixes the 0 flags. Agree with you! >=20 > -- > Thomas Thank you very much for thinking so much about this! Regards, Helin