From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D7D17A0C40; Wed, 28 Apr 2021 14:21:56 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5E2EB40697; Wed, 28 Apr 2021 14:21:56 +0200 (CEST) Received: from mail-wr1-f53.google.com (mail-wr1-f53.google.com [209.85.221.53]) by mails.dpdk.org (Postfix) with ESMTP id B494240147 for ; Wed, 28 Apr 2021 14:21:54 +0200 (CEST) Received: by mail-wr1-f53.google.com with SMTP id l2so10492959wrm.9 for ; Wed, 28 Apr 2021 05:21:54 -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=ISj3wxhNtkboVYNWfQbvaCQS8PxfWlz8Kku5/jQH1xs=; b=i5u0bTPBNgDe+EHknF7RSW3nbvz0L0BvaYo/ZnD/ILpPlaAcXjr736MeatuV1ULw9Q 6C6pPn8mWTXuOWN2WETIOlvX74sIVkrKqArL3MBOINbGFU7XOBN1VUhMljeh+pIU1IZ4 5Dj19A8Nc4pcWPjqkBKlGOhVLvXbiMdHrV7h3jZVZSbobVByzH1FO4Rabs47++CXwUR8 adTrNX5eJ0TUiiEo9abTQ2f1Ylap263kzq9M23EiiEXIYa5o5L4Ru97je8Aesy+HpMtT Ps1uHUf/LfHKY1NIpEkll+ZlsisSpo3K85SXJnsnsjSGxvo2N/0g8gWfGCEvkl0ttKVm N5tA== 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=ISj3wxhNtkboVYNWfQbvaCQS8PxfWlz8Kku5/jQH1xs=; b=gIAdn85rTOtZG5hBol20ocGn5LLBLqPs9aOm30gucR2GoE/ChTe52OPKbWABsHbrzM g1huwk0hX1/aYlA9lyexazeViX+llApAqDguf29nuW3fkdjfJ+g5nMBDPkjXFjYCiVgG i5BEEZHlruPcZcGFRw11e1+Wh+kocRZiLFhsXMmuBGXFZBL1odabfGsVXsyIRGZZGq+g cptXWtYuANGGuqQpPJNsEcKHpfvQbHUf/Tg0kk27eGZJsLMjVrgqUicM0d9c+Y0U2JtH VhdOfipEOicL+bDMvQrcW9ivOrFWxmpu0G63i9+EuN2Pv5queWP63xhlHWnOcwHjR+sa uvRw== X-Gm-Message-State: AOAM532G0NyPe2cG0ZyIvB4/Jq58d511utHRdhK22hPiQ3PFxHTi9cyg 9di0PqDLJkvBvRLaZdmVxc01MQ== X-Google-Smtp-Source: ABdhPJxfsuxV9aM4s2xo0ckOW8FftxHurXRTh5Nkm7IGt8QkOIv6tF51MqefiUbaMdWJg9tLcAVFOA== X-Received: by 2002:a5d:610d:: with SMTP id v13mr6075081wrt.173.1619612514359; Wed, 28 Apr 2021 05:21:54 -0700 (PDT) Received: from 6wind.com ([2a01:e0a:5ac:6460:c065:401d:87eb:9b25]) by smtp.gmail.com with ESMTPSA id b202sm6572697wmb.5.2021.04.28.05.21.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 28 Apr 2021 05:21:53 -0700 (PDT) Date: Wed, 28 Apr 2021 14:21:53 +0200 From: Olivier Matz To: Morten =?iso-8859-1?Q?Br=F8rup?= Cc: dev@dpdk.org, Keith Wiles , Hongzhi Guo , Thomas Monjalon Message-ID: <20210428122153.GU1726@platinum> References: <20210427135755.927-1-olivier.matz@6wind.com> <20210427135755.927-4-olivier.matz@6wind.com> <98CBD80474FA8B44BF855DF32C47DC35C61718@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: <98CBD80474FA8B44BF855DF32C47DC35C61718@smartserver.smartshare.dk> User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [PATCH 3/4] net: introduce functions to verify L4 checksums X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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 Morten, Thank you for the review. <...> On Tue, Apr 27, 2021 at 05:07:04PM +0200, Morten Brørup wrote: > > +static inline uint16_t > > +rte_ipv4_udptcp_cksum(const struct rte_ipv4_hdr *ipv4_hdr, const void > > *l4_hdr) > > +{ > > + uint16_t cksum = __rte_ipv4_udptcp_cksum(ipv4_hdr, l4_hdr); > > + > > + cksum = ~cksum; > > + > > /* > > - * Per RFC 768:If the computed checksum is zero for UDP, > > + * Per RFC 768: If the computed checksum is zero for UDP, > > * it is transmitted as all ones > > * (the equivalent in one's complement arithmetic). > > */ > > if (cksum == 0 && ipv4_hdr->next_proto_id == IPPROTO_UDP) > > cksum = 0xffff; > > > > - return (uint16_t)cksum; > > + return cksum; > > +} > > The GCC static branch predictor treats the above comparison as likely. Playing around with Godbolt, I came up with this alternative: > > if (likely(cksum != 0)) return cksum; > if (ipv4_hdr->next_proto_id == IPPROTO_UDP) return 0xffff; > return 0; Good idea, this is indeed an unlikely branch. However this code was already present before this patch, so I suggest to add it as a specific optimization patch. > > + > > +/** > > + * Validate the IPv4 UDP or TCP checksum. > > + * > > + * @param ipv4_hdr > > + * The pointer to the contiguous IPv4 header. > > + * @param l4_hdr > > + * The pointer to the beginning of the L4 header. > > + * @return > > + * Return 0 if the checksum is correct, else -1. > > + */ > > +__rte_experimental > > +static inline int > > +rte_ipv4_udptcp_cksum_verify(const struct rte_ipv4_hdr *ipv4_hdr, > > + const void *l4_hdr) > > +{ > > + uint16_t cksum = __rte_ipv4_udptcp_cksum(ipv4_hdr, l4_hdr); > > + > > + if (cksum != 0xffff) > > + return -1; > > The GCC static branch predictor treats the above comparison as likely, so I would prefer unlikely() around it. For this one, I'm less convinced: should we decide here whether the good or the bad checksum is more likely than the other? Given it's a static inline function, wouldn't it be better to let the application call it this way: if (likely(rte_ipv4_udptcp_cksum_verify(...) == 0)) ? Regards, Olivier