From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4F392A00C5; Mon, 6 Jul 2020 09:58:38 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 63F7C1C1E5; Mon, 6 Jul 2020 09:58:37 +0200 (CEST) Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by dpdk.org (Postfix) with ESMTP id B1FF81C0B3 for ; Mon, 6 Jul 2020 09:58:35 +0200 (CEST) Received: by mail-wr1-f68.google.com with SMTP id s10so39688272wrw.12 for ; Mon, 06 Jul 2020 00:58:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=4RJUuyfHSJas9Scv2Mo/wKY0ibtDJZdkrn8YWrrPRK8=; b=MeVXF/hajMUfl7QF56dlBZlWG0kbI8DNORRjOVCDLowSqgnxbiGL6ObpKVtW6KExc2 0SOpa1GqMXEKgzUpWQ2lGZT/l+HaSzxEdstyCGCo6eDlStmr+vMUFMqSuyLbSOgKi43x lWUeqsX1dwkDdNy8o6COSMoEFdm9vgckn6lL/+J33+HGOOXEXHLHD6RY+63MqQd7FoSK 65xlzfxYBD1DFimdEZO2MIq9Hp+hk0H19lH46B28+Bhs9h4ACgpUef62REmMil4tpyO8 rpfUTi7W6rYExlzxHLIzqcg+HZP5rL8Dqvsusuwim34EyUlPinruCOJFNRYrwZ8aEaWD Ny/Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=4RJUuyfHSJas9Scv2Mo/wKY0ibtDJZdkrn8YWrrPRK8=; b=sSMs+Cg93lDjALXIRkcG6b3BXdX79PmfRPzoBnEJj3x9muC99tC5nKT5+yo6M5GteU HsjYzK6CghCMoyqtprks20rO0G0Be5GXwPgCLcjJ4SLLGw4menPeyEY7VdjL1hBFVnGj D4EShGzhuYtMYKNn+PKV1w1OjA/Isiwx4cwVIdrh+qZtGOTLv2JEtF1YQzKzamsfOH4A nHENZFjAHClzH/b0O67AGfRUDK2zK+iYTBVs/Jt4zBwhhXhasbINCNPhj0ztdYX8YqQC S00RpO0/RAVMAqL/t1nJqvhDUl66+4fzZ2o3hFj6a1uvPvdO1ajAZ6o5bKVTlML7SiEH 4s/g== X-Gm-Message-State: AOAM5320lY8aynw1RSYKcUmeUVpOdqhdI8zSyfpAYU2R5yNyw+Yb4sSh 6HfOkCpHjNgDgDLAhJhI/INc9w== X-Google-Smtp-Source: ABdhPJxAS66UaaXk1Q4T29lscJepc/8JEE5GiUMzKKCk+YouHx2wbQLeuusfoR6a1TCBDaHQ+1Dfrw== X-Received: by 2002:adf:f984:: with SMTP id f4mr46931620wrr.221.1594022315406; Mon, 06 Jul 2020 00:58:35 -0700 (PDT) Received: from 6wind.com (2a01cb0c0005a600345636f7e65ed1a0.ipv6.abo.wanadoo.fr. [2a01:cb0c:5:a600:3456:36f7:e65e:d1a0]) by smtp.gmail.com with ESMTPSA id v12sm10560446wrt.31.2020.07.06.00.58.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Jul 2020 00:58:34 -0700 (PDT) Date: Mon, 6 Jul 2020 09:58:34 +0200 From: Olivier Matz To: Morten =?iso-8859-1?Q?Br=F8rup?= Cc: Stephen Hemminger , guohongzhi , dev@dpdk.org, stable@dpdk.org, konstantin.ananyev@intel.com, jiayu.hu@intel.com, ferruh.yigit@intel.com, nicolas.chautru@intel.com, cristian.dumitrescu@intel.com, zhoujingbin@huawei.com, chenchanghu@huawei.com, jerry.lilijun@huawei.com, haifeng.lin@huawei.com Message-ID: <20200706075834.GC5869@platinum> References: <20200527144127.5792-1-guohongzhi1@huawei.com> <20200527080242.16dceeae@hermes.lan> <98CBD80474FA8B44BF855DF32C47DC35C6100E@smartserver.smartshare.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35C6100E@smartserver.smartshare.dk> User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [PATCH] bugfix: udptcp_checksum should tread tcp and udp differently 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" Hi, Here is a suggestion for the title: net: fix uneeded replacement of 0 by ffff for TCP checksum The commit log looks good to me, but you can add: Fixes: 6006818cfb26 ("net: new checksum functions") Cc: stable@dpdk.org On Wed, May 27, 2020 at 05:36:59PM +0200, Morten Brørup wrote: > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger > > Sent: Wednesday, May 27, 2020 5:03 PM > > > > On Wed, 27 May 2020 22:41:27 +0800 > > guohongzhi wrote: > > > > > --- a/lib/librte_net/rte_ip.h > > > +++ b/lib/librte_net/rte_ip.h > > > @@ -324,8 +324,7 @@ rte_ipv4_phdr_cksum(const struct rte_ipv4_hdr *ipv4_hdr, uint64_t ol_flags) > > > * @param l4_hdr > > > * The pointer to the beginning of the L4 header. > > > * @return > > > - * The complemented checksum to set in the IP packet > > > - * or 0 on error > > > + * The complemented checksum to set in the IP packet. > > > */ > > > static inline uint16_t > > > rte_ipv4_udptcp_cksum(const struct rte_ipv4_hdr *ipv4_hdr, const void *l4_hdr) It still returns 0 when ipv4_hdr->total_length < sizeof(struct rte_ipv4_hdr). I suggest to keep this case in the API comment, maybe like this: The complemented checksum to set in the IP packet or 0 if the IP length is invalid in the header. > > > + /* For Udp, if the computed checksum is zero, > > > + * it is transmitted as all ones.RFC768 > > > + */ > > > + if (cksum == 0 && ipv4_hdr->next_proto_id == IPPROTO_UDP) > > > cksum = 0xffff; > > > > > > > > > The comment should be reformatted to be clearer. > > > > Maybe: > > /* > > * Per RFC768: > > * If the computed checksum is zero,it is transmitted as all > > ones > > * (the equivalent in one's complement arithmetic). > > */ > > > > But without the double spaces. :-) I agree, Stephen's wording looks better, can you please use it? > > There is no special case required here, it is true for TCP as well. > > I disagree. I researched this topic when Hongzhi Guo initially submitted the patches, and have only seen the special exception mentioned in RFC 768 (describing UDP), where a transmitted value of 0x0000 means that the checksum has not been generated. None of the RFCs regarding Internet Checksum or TCP checksum mentions this special case. > > > In TCP it appears 0 is allowed but not generally used. Most > > implementations > > use common checksum for both TCP and UDP > > Then most implementations are wrong. > > Jon Postel famously wrote: "Be liberal in what you accept, and conservative in what you send." > > This function is in the transmission path, so we should calculate it correctly. > > In the receive path, when checking the checksum as described in RFC 1071, any of the two values (0x0000 or 0xFFFF) in the Checksum field will yield the correct result. Which is why most implementations can get away with doing it wrong. I agree with Morten, it looks more strict like this.