From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from proxy.6wind.com (host.76.145.23.62.rev.coltfrance.com [62.23.145.76]) by dpdk.org (Postfix) with ESMTP id EC3482C36 for ; Tue, 2 Apr 2019 10:46:30 +0200 (CEST) Received: from core.dev.6wind.com (unknown [10.0.0.1]) by proxy.6wind.com (Postfix) with ESMTPS id CF17A28F63C; Tue, 2 Apr 2019 10:46:30 +0200 (CEST) Received: from [10.16.0.195] (helo=6wind.com) by core.dev.6wind.com with smtp (Exim 4.84_2) (envelope-from ) id 1hBF3t-0000a7-PL; Tue, 02 Apr 2019 10:46:30 +0200 Received: by 6wind.com (sSMTP sendmail emulation); Tue, 02 Apr 2019 10:46:29 +0200 Date: Tue, 2 Apr 2019 10:46:29 +0200 From: Olivier Matz To: Andrew Rybchenko Cc: "N. Benes" , dev , Stephen Hemminger , Thomas Monjalon Message-ID: <20190402084629.2f3u3iqfkkmrf72x@glumotte.dev.6wind.com> References: <65744706-bc88-f144-f85e-1b0292402cff@eso.org> <105860ec-b4b7-b045-05e0-6fb67bab237a@solarflare.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <105860ec-b4b7-b045-05e0-6fb67bab237a@solarflare.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] Bug in IPv4 header checksum computation? X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Apr 2019 08:46:31 -0000 Hi, On Tue, Apr 02, 2019 at 10:58:52AM +0300, Andrew Rybchenko wrote: > Hi, > > added more people in CC. > > On 4/1/19 6:29 PM, N. Benes wrote: > > Hi, > > > > I wrote to the users list a bit more a week ago concerning the IPv4 > > Header Checksum computation and received no comments on it yet: > > > > https://mails.dpdk.org/archives/users/2019-March/004021.html > > https://mails.dpdk.org/archives/users/2019-March/004022.html > > > > I have the impression that the current implementation wrongly does a > > conditional final inversion of the computed sum. This is correct for > > e.g. UDP, but I think it is not for an IPv4 Header (header checksum is > > not optional). > > > > If it is a bug, this could result in a surprisingly high number of > > invalid/dropped IPv4 packets, when the checksum is calculated manually > > via the DPDK API (not NIC-offloaded) and the IPv4 header has an > > appropriate combination of values e.g. the IP Identification field. > > I agree that it looks like a bug in DPDK. RFC 791 says nothing about > avoid zero value for IPv4 checksum. > > Andrew. Thank you for reporting this. In my opinion, this operation is indeed useless, but not a critical bug: it won't make the checksum verification to fail. Indeed, in one's complement arithmetic, the "0" value can be either 0x0000 or 0xffff. This is why it can be safely replaced in case of UDP. See https://en.wikipedia.org/wiki/Ones%27_complement I will submit a patch to change this. Regards, Olivier From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 8203DA0679 for ; Tue, 2 Apr 2019 10:46:32 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2DB2C34F3; Tue, 2 Apr 2019 10:46:32 +0200 (CEST) Received: from proxy.6wind.com (host.76.145.23.62.rev.coltfrance.com [62.23.145.76]) by dpdk.org (Postfix) with ESMTP id EC3482C36 for ; Tue, 2 Apr 2019 10:46:30 +0200 (CEST) Received: from core.dev.6wind.com (unknown [10.0.0.1]) by proxy.6wind.com (Postfix) with ESMTPS id CF17A28F63C; Tue, 2 Apr 2019 10:46:30 +0200 (CEST) Received: from [10.16.0.195] (helo=6wind.com) by core.dev.6wind.com with smtp (Exim 4.84_2) (envelope-from ) id 1hBF3t-0000a7-PL; Tue, 02 Apr 2019 10:46:30 +0200 Received: by 6wind.com (sSMTP sendmail emulation); Tue, 02 Apr 2019 10:46:29 +0200 Date: Tue, 2 Apr 2019 10:46:29 +0200 From: Olivier Matz To: Andrew Rybchenko Cc: "N. Benes" , dev , Stephen Hemminger , Thomas Monjalon Message-ID: <20190402084629.2f3u3iqfkkmrf72x@glumotte.dev.6wind.com> References: <65744706-bc88-f144-f85e-1b0292402cff@eso.org> <105860ec-b4b7-b045-05e0-6fb67bab237a@solarflare.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline In-Reply-To: <105860ec-b4b7-b045-05e0-6fb67bab237a@solarflare.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] Bug in IPv4 header checksum computation? X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190402084629.6DFlCldEQzerDydtcCDoRQ58_Jq-m5Jpjqwip34pdUM@z> Hi, On Tue, Apr 02, 2019 at 10:58:52AM +0300, Andrew Rybchenko wrote: > Hi, > > added more people in CC. > > On 4/1/19 6:29 PM, N. Benes wrote: > > Hi, > > > > I wrote to the users list a bit more a week ago concerning the IPv4 > > Header Checksum computation and received no comments on it yet: > > > > https://mails.dpdk.org/archives/users/2019-March/004021.html > > https://mails.dpdk.org/archives/users/2019-March/004022.html > > > > I have the impression that the current implementation wrongly does a > > conditional final inversion of the computed sum. This is correct for > > e.g. UDP, but I think it is not for an IPv4 Header (header checksum is > > not optional). > > > > If it is a bug, this could result in a surprisingly high number of > > invalid/dropped IPv4 packets, when the checksum is calculated manually > > via the DPDK API (not NIC-offloaded) and the IPv4 header has an > > appropriate combination of values e.g. the IP Identification field. > > I agree that it looks like a bug in DPDK. RFC 791 says nothing about > avoid zero value for IPv4 checksum. > > Andrew. Thank you for reporting this. In my opinion, this operation is indeed useless, but not a critical bug: it won't make the checksum verification to fail. Indeed, in one's complement arithmetic, the "0" value can be either 0x0000 or 0xffff. This is why it can be safely replaced in case of UDP. See https://en.wikipedia.org/wiki/Ones%27_complement I will submit a patch to change this. Regards, Olivier