From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wg0-f52.google.com (mail-wg0-f52.google.com [74.125.82.52]) by dpdk.org (Postfix) with ESMTP id A8995231C for ; Wed, 26 Nov 2014 15:01:17 +0100 (CET) Received: by mail-wg0-f52.google.com with SMTP id a1so3830742wgh.39 for ; Wed, 26 Nov 2014 06:12:10 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=sm6puCadW3NeUgSbKfSvEfeT2kpTKqz7ObFQrc1dqYk=; b=I9uI69PKyMCIdJx9D/FZW/xg7cE0ZXIa8pkMVFx3+j1/D7xlCKgl1QclhxwMqiH/KI usgGvz7Zho70X42YEluYjb7Cd3+Q1mUU5+dOp4XIHrHGB9c1uWkxfg5aWhSHAvNHQCt6 p0ORFeKVboDB5OvCdJNlKHFcC1oLBpwHWzNl3rhYbAjCMV/gE8ab7EWHeKVjqa2L6Ydg Ee3+XImMSeGpUeetleeY6VNWlmwrJ1yiu5RaXJ4YFAXQ9mi99DqTCQAamyJCK08sXl99 mNTky91lt8l4d2S/gMl8zEhVK4ldPGqIOZFNF+Sci4v9SRdHORt3gMZCMxQ4J31Vq1f8 uuOA== X-Gm-Message-State: ALoCoQkemAIC7FEJ4ZxuhBaws9IVqusgrakjoF7GT3sSEwjFtrDS3AYCzFLz0GWe+FBuuWMW8sIb X-Received: by 10.180.95.201 with SMTP id dm9mr41104398wib.27.1417011130852; Wed, 26 Nov 2014 06:12:10 -0800 (PST) Received: from [10.16.0.195] (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177]) by mx.google.com with ESMTPSA id h2sm7458949wix.5.2014.11.26.06.12.09 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 26 Nov 2014 06:12:10 -0800 (PST) Message-ID: <5475DFB9.7060609@6wind.com> Date: Wed, 26 Nov 2014 15:12:09 +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: "Ananyev, Konstantin" , "Zhang, Helin" , "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> In-Reply-To: <2601191342CEEE43887BDE71AB977258213BA8CB@IRSMSX105.ger.corp.intel.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed 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: Wed, 26 Nov 2014 14:01:17 -0000 Hi Konstantin, On 11/26/2014 02:38 PM, Ananyev, Konstantin wrote: >>> Probably I didn't explain myself clear enough, sorry. >>> I didn't suggest to get rid of setting bits that indicate L3/L4 checksum errors: >>> PKT_RX_IP_CKSUM_BAD, PKT_RX_L4_CKSUM_BAD, PKT_RX_EIP_CKSUM_BAD. >>> I think these flags should be set as before. >>> >>> I was talking only about collapsing only these 4 RX error flags into one: >>> >>> #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. */ >>> >>> From my point of view the difference of these 2 groups are: >>> First - HW was able to receive whole packet without a problem, but L3/L4 checksum check failed. >>> >>> Second - HW was not able to receive whole packet properly by whatever reason. >>> From upper layer SW perspective - there it probably makes little difference, what caused it, >>> as most likely SW has to throw away erroneous packet. >>> And for debugging purposes, we can add PMD_LOG(DEBUG, ...) that would print what exactly HW error happened. >> >> I agree with Konstantin that there are 2 different cases: >> >> a) the packet is properly received by the hardware, but has a bad >> checksum (or another protocol error, for instance an invalid ip len, >> a ip_version == 8 :)) >> >> in this case, it is useful to the application to have the mbuf with >> the data + an error flag. Then using a tcpdump-like tool could help >> to debug what is the cause of the error and what equipment generates >> a bad packet. >> >> b) the packet is not properly received by the hardware. In this case >> the data is invalid in the mbuf and not useable by the application. >> I suggest to only have a stats counter in this case, as receiving the >> mbuf is cpu time consuming and the only thing the application can do >> is to drop the packet. > > So for b) you suggest to drop the packet straight in PMD RX function? > Something like: > if (unlikely(error_bits & ...)) { > PMD_LOG(DEBUG, ...); > rte_pktmbuf_free(mb); > } > ? Yes > That's probably a bit too radical. > Yes, mbuf doesn't contain the whole packet, but it may contain at least part of it, let say in case of 'packet oversize'. > So for debugging purposes the user may still like to examine the mbuf contents. As soon as there is some exploitable data in the mbuf, I agree it can be transfered to the application (ex: bad header, bad len, bad checksum...). But if the hardware is not able to provide any exploitable data, it looks a bit overkill to give an mbuf with an error flag. But grouping the flags as you suggest is already a good clean-up to me, I don't want to be more catholic than the Pope ;) Regards, Olivier