From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 425CA1518 for ; Fri, 28 Nov 2014 09:47:48 +0100 (CET) Received: from was59-1-82-226-113-214.fbx.proxad.net ([82.226.113.214] helo=[192.168.0.10]) by mail.droids-corp.org with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.80) (envelope-from ) id 1XuHGR-0004du-Ob; Fri, 28 Nov 2014 09:50:59 +0100 Message-ID: <547836A9.1010008@6wind.com> Date: Fri, 28 Nov 2014 09:47:37 +0100 From: Olivier MATZ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Icedove/24.5.0 MIME-Version: 1.0 To: "Zhang, Helin" , "Ananyev, Konstantin" , "dev@dpdk.org" 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> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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: Fri, 28 Nov 2014 08:47:48 -0000 Hi Helin, 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 there, and just assign a new value to it. ok. But it would be nice to have a better definition of this flag: does external mean outer header? For instance, when you receive a Ether/IP1/UDP/vxlan/Ether/IP2/xxx, does the flag concerns IP1 or IP2? If it's IP1, it's really strange compared to the current behavior (the flag PKT_RX_IP_CKSUM_BAD refers to IP1). > #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 information. So I > prefer to keep it there, and just assign a new value to it. Again, a statistic counter would do the job which if it's just to provide a hint to the application. I wonder in which case this flag can happen. If you fill the ring with mbufs 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 an mbuf with this flag would stop the driver, then refill the rings it with larger mbufs. Last but not least: If it's really useful, should we have the same behavior on other drivers like ixgbe? I think we really need to care about not having different ways to use the different drivers. To me, the only argument in favor of keeping this flag is when the mbuf contains a part of the data that could be dumped by a user for debug purposes. > #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 to keep it there, and just assign new value to it. Same for this one. > #define PKT_RX_RECIP_ERR (0ULL << 0) /**< Hardware processing error. */ > [Helin] In the latest data sheet, it is not opened to external users. So we can just remove > it from here. ok > #define PKT_RX_MAC_ERR (0ULL << 0) /**< MAC error. */ > [Helin] This indicates a real hardware error happens. And what is the content of the mbuf data in this case? Does the application really need an mbuf? Regards, Olivier