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 DCAD33F9 for ; Mon, 1 Dec 2014 02:57:40 +0100 (CET) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP; 30 Nov 2014 17:57:39 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,489,1413270000"; d="scan'208";a="640430130" Received: from kmsmsx153.gar.corp.intel.com ([172.21.73.88]) by fmsmga002.fm.intel.com with ESMTP; 30 Nov 2014 17:57:37 -0800 Received: from shsmsx103.ccr.corp.intel.com (10.239.110.14) by KMSMSX153.gar.corp.intel.com (172.21.73.88) with Microsoft SMTP Server (TLS) id 14.3.195.1; Mon, 1 Dec 2014 09:57:37 +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; Mon, 1 Dec 2014 09:57:36 +0800 From: "Zhang, Helin" To: Olivier MATZ , "Ananyev, Konstantin" , "dev@dpdk.org" Thread-Topic: [PATCH] i40e: Use one bit flag for all hardware detected RX packet errors Thread-Index: AQHQCWs/YCaH2DMkh0m0YaBNr2E3oZxyY4CAgAAJe4CAA0CEsP//iXmAgATBUXA= Date: Mon, 1 Dec 2014 01:57:36 +0000 Message-ID: References: <1416982032-28519-1-git-send-email-helin.zhang@intel.com> <2601191342CEEE43887BDE71AB977258213BA7CA@IRSMSX105.ger.corp.intel.com> <5475B7EE.4020400@6wind.com> <2601191342CEEE43887BDE71AB977258213BA8CB@IRSMSX105.ger.corp.intel.com> <5475DFB9.7060609@6wind.com> <547836A9.1010008@6wind.com> In-Reply-To: <547836A9.1010008@6wind.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] i40e: Use one bit flag for all hardware detected RX packet errors 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: Mon, 01 Dec 2014 01:57:42 -0000 Hi Olivier > -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > Sent: Friday, November 28, 2014 4:48 PM > To: Zhang, Helin; Ananyev, Konstantin; dev@dpdk.org > Cc: Cao, Waterman; Cao, Min > Subject: Re: [PATCH] i40e: Use one bit flag for all hardware detected RX = packet > errors >=20 > Hi Helin, >=20 > On 11/28/2014 09:07 AM, Zhang, Helin wrote: > > After I have completed another task, I read the datasheet carefully > > again. For those 5 error bits I introduced for a long time, I'd like to= explain one > by one as below. > > > > #define PKT_RX_EIP_CKSUM_BAD (0ULL << 0) /**< External IP header > > checksum error. */ [Helin] Nobody complains it, so we will keep it ther= e, and > just assign a new value to it. >=20 > ok. >=20 > But it would be nice to have a better definition of this flag: does exter= nal mean > outer header? For instance, when you receive a > Ether/IP1/UDP/vxlan/Ether/IP2/xxx, does the flag concerns IP1 or IP2? 'E' means 'external', it indicates the (most) outer IP header checksum erro= r. If you don't think this name is not so clear, I can change it to 'PKT_RX_OUTER_IP_= CHSUM_BAD'. For inner IP header checksum error, it will be indicated by PKT_RX_IP_CKSUM= _BAD. >=20 > If it's IP1, it's really strange compared to the current behavior (the fl= ag > PKT_RX_IP_CKSUM_BAD refers to IP1). >=20 > > #define PKT_RX_OVERSIZE (0ULL << 0) /**< Num of desc of an RX pkt > oversize. */ > > [Helin] I don't think it can be merge with other hardware errors. It > > indicates the packet received needs more descriptors than hardware > > allowed, and the part of packets can still be stored in the mbufs > > provided. It is a good hint for users that larger size of mbuf might > > be needed. If just put it as hardware error, users will lose this infor= mation. So I > prefer to keep it there, and just assign a new value to it. >=20 > Again, a statistic counter would do the job which if it's just to provide= a hint to the > application. It seems that we do not maintain a counter for packets in PMD, if I am not = wrong. Two ways current DPDK is using. One is hardware provide registers to do that, we can read it directly when = needed. The other one is that applications or middle layer sw maintain its own stat= istics. >=20 > I wonder in which case this flag can happen. If you fill the ring with mb= ufs that are > large enough compared to your ethernet network, this should not happen in > normal conditions. I really don't believe that an application receiving a= n mbuf > with this flag would stop the driver, then refill the rings it with large= r mbufs. This is not because of it is lack of available RX descriptors. It is becaus= e of a hardware requirement. FVL hardware requires that it should not use more than 5 rx de= scriptors for receiving a single packet. > Last but not least: If it's really useful, should we have the same behavi= or on other > drivers like ixgbe? I think we really need to care about not having diffe= rent ways > to use the different drivers. I don't see the similar bit in ixgbe datasheet, but this restriction could = be common for some other NICs, as it is reasonable from hardware perspective. >=20 > To me, the only argument in favor of keeping this flag is when the mbuf c= ontains > a part of the data that could be dumped by a user for debug purposes. >=20 > > #define PKT_RX_HBUF_OVERFLOW (0ULL << 0) /**< Header buffer overflow. > > */ [Helin] It indicates the header buff size is not enough, but not > > means hardware cannot process the packet received. It is a good hint > > for the users to provide larger size of header buffers. I also prefer t= o keep it > there, and just assign new value to it. >=20 > Same for this one. It is a bit different from previous one, it always has one header buffer, t= his flag means the buffer size is not enough for the header. These two flags are because of buffer size or number of buffers. The mbufs = are prepared in application or up layer software. If these two flags occur, it is easier= for up layer software to debug, and know different buffers are needed. They do not need to debug = PMD, as they generally don't want to do. >=20 > > #define PKT_RX_RECIP_ERR (0ULL << 0) /**< Hardware processing erro= r. > */ > > [Helin] In the latest data sheet, it is not opened to external users. > > So we can just remove it from here. >=20 > ok >=20 > > #define PKT_RX_MAC_ERR (0ULL << 0) /**< MAC error. */ > > [Helin] This indicates a real hardware error happens. >=20 > And what is the content of the mbuf data in this case? Does the applicati= on really > need an mbuf? Mbuf contains both the data and other information. I prefer to let the up l= ayer software to decide how to deal with the packet, no matter it is correct or bad. In addi= tion, even hardware errors happened, it still can set a special bit to enable storing the whole= packet to the mbuf, for debug purpose. Hardware error bit can be used for all hardware errors. = As we do not have one there, why not add one? >=20 >=20 > Regards, > Olivier Regards, Helin