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 D55D0A0C45 for ; Wed, 28 Jul 2021 16:12:45 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CAFAF410FD; Wed, 28 Jul 2021 16:12:45 +0200 (CEST) Received: from mail-wm1-f45.google.com (mail-wm1-f45.google.com [209.85.128.45]) by mails.dpdk.org (Postfix) with ESMTP id 8BFA5410F0 for ; Wed, 28 Jul 2021 16:12:43 +0200 (CEST) Received: by mail-wm1-f45.google.com with SMTP id n28-20020a05600c3b9cb02902552e60df56so1765275wms.0 for ; Wed, 28 Jul 2021 07:12:43 -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:in-reply-to; bh=OZz9y5Y/i2kR2P/5OPnzN+k81/2AipNg/Yu0dgPveEg=; b=Tuut5SeqYoLepq9CIaIX97xMaNKIGsZb4XnfHJ750Rvg9yIa4APT/SzSfw6TotFp8S 3bT3tg3cu4G3SDcVML35cL5ZdZMUxMMB2ruf/vA4XTqM8aNm1qZFpH9bBZaz4sG1TGPX TeFlkbZ/fUbbAuofdC8UE24rpuOvpDMnyEJVtQGXKDoFJHMLZvEyrtyThHWRAslPSlBu VcUKCcj/NSNXcA/TKwsq7C6vIkPOBkcn7JwxdqIXTM69ekRHPMXNpdY7d5/80iQeJT9v f4FzkWj7GrvbACJti7rfLWDT6hvfodgJE13slgfUzQ66ZS6uHw8wbYveyNEHtp88FCSm l5vg== 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:in-reply-to; bh=OZz9y5Y/i2kR2P/5OPnzN+k81/2AipNg/Yu0dgPveEg=; b=UGoopO8ZkW5W+FHGKm4IspuVxPxjU0fZJk4b+j/3xdtJyUmUtBQW/4mMqonTShWR6A 57ZZpqIyP0a5i99GHaCxLhOKQd38IW3anm4HfOA/a0Dt/CF6cVEt4084fLcZG9+Vl9+y 0z2B7r90kAlISRqfp+TKIVZMhasKm4uQbPvlmCuuKko7iqhoUdx2MYyPCz+LtXQa7iJR 32GTfungf6rLYG7oYjT6Z5zTULbCt6gZUkCw366399+M0qYfHVr98WWz3FlPblAjTPck RFDfxNSpEpFAO1XMs2hQ6Me/xWpNbh9JD1PFvS3CL0CIdNZas+TeX4YMW85w95AvTkmF nkUg== X-Gm-Message-State: AOAM530Jy9P6n7Lrz4aIbtHKwJKzz3Xkphf0oP8kcgVdcVwpt+WcCH2h bgROhzRMuzBrxnHlsLbGF2vn1w== X-Google-Smtp-Source: ABdhPJw+MOICizky/0kqQf3L2bOf8zxGA1cTCEFDErxsiJ0xIZJYC0klUKHZK/9rcygolf5rlacx2w== X-Received: by 2002:a1c:7402:: with SMTP id p2mr9683896wmc.111.1627481563181; Wed, 28 Jul 2021 07:12:43 -0700 (PDT) Received: from 6wind.com ([2a01:e0a:5ac:6460:c065:401d:87eb:9b25]) by smtp.gmail.com with ESMTPSA id b6sm6236542wmj.34.2021.07.28.07.12.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 28 Jul 2021 07:12:42 -0700 (PDT) Date: Wed, 28 Jul 2021 16:12:41 +0200 From: Olivier Matz To: Gregory Etelson Cc: dev@dpdk.org, Ajit Khaparde , Andrew Rybchenko , Ferruh Yigit , Thomas Monjalon , stable@dpdk.org, Xiaoyun Li Message-ID: References: <20210719083309.15428-1-getelson@nvidia.com> <20210727130757.30724-1-getelson@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210727130757.30724-1-getelson@nvidia.com> Subject: Re: [dpdk-stable] [PATCH v2] app/testpmd: fix TX checksum calculation for tunnel X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" Hi Gregory, Few comments below. On Tue, Jul 27, 2021 at 04:07:57PM +0300, Gregory Etelson wrote: > TX checksum of a tunnelled packet can be calculated for outer headers > only or for both outer and inner parts. The calculation method is > determined by application. > If TX checksum calculation can be offloaded, hardware ignores > existing checksum value and replaces it with an updated result. This is not always true. Actually, the checksum value is optionally set by software to the value that is expected by the hardware to offload the checksum correctly. This is done through rte_eth_tx_prepare(), which is called in csumonly test engine. For instance, on an ixgbe NIC, it does: rte_eth_tx_prepare() eth_dev->tx_pkt_prepare() ixgbe_prep_pkts() rte_net_intel_cksum_flags_prepare() if packet is IP, set IP checksum to 0 if packet is TCP or UDP, set L4 checksum to the phdr csum This driver-specific rte_eth_tx_prepare() can indeed do nothing and let the hardware ignore the checksum in the packet. > If TX checksum is calculated by a software, existing value must be > zeroed first. > The testpmd checksum forwarding engine always zeroed inner checksums. > If inner checksum calculation was offloaded, that header was left > with 0 checksum value. > Following outer software checksum calculation produced wrong value. > The patch zeroes inner IPv4 checksum only before software calculation. Sorry, I think I don't understand the issue. Are you trying to compute the inner checksum by hardware and the outer checksum by software? > Fixes: 51f694dd40f5 ("app/testpmd: rework checksum forward engine") I'm not sure the problem origin is this commit (however, I may have misunderstood your issue). At the time this commit was done, it was required to set the TCP/UDP checksum to the pseudo header checksum to offload an L4 checksum. See: https://git.dpdk.org/dpdk/tree/lib/librte_mbuf/rte_mbuf.h?id=51f694dd40f5#n107 The introduction of rte_eth_tx_prepare() API removed this need, see: https://git.dpdk.org/dpdk/commit/?id=6b520d54ebfe Thanks, Olivier > Cc: stable@dpdk.org > > Signed-off-by: Gregory Etelson > --- > v2: > remove blank line between Fixes and Cc > explicitly compare with 0 value in `if ()` > --- > app/test-pmd/csumonly.c | 23 ++++++++++++----------- > 1 file changed, 12 insertions(+), 11 deletions(-) > > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c > index 0161f72175..bd5ad64a57 100644 > --- a/app/test-pmd/csumonly.c > +++ b/app/test-pmd/csumonly.c > @@ -480,17 +480,18 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info, > > if (info->ethertype == _htons(RTE_ETHER_TYPE_IPV4)) { > ipv4_hdr = l3_hdr; > - ipv4_hdr->hdr_checksum = 0; > > ol_flags |= PKT_TX_IPV4; > if (info->l4_proto == IPPROTO_TCP && tso_segsz) { > ol_flags |= PKT_TX_IP_CKSUM; > } else { > - if (tx_offloads & DEV_TX_OFFLOAD_IPV4_CKSUM) > + if (tx_offloads & DEV_TX_OFFLOAD_IPV4_CKSUM) { > ol_flags |= PKT_TX_IP_CKSUM; > - else > + } else if (ipv4_hdr->hdr_checksum != 0) { > + ipv4_hdr->hdr_checksum = 0; > ipv4_hdr->hdr_checksum = > rte_ipv4_cksum(ipv4_hdr); > + } > } > } else if (info->ethertype == _htons(RTE_ETHER_TYPE_IPV6)) > ol_flags |= PKT_TX_IPV6; > @@ -501,10 +502,10 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info, > udp_hdr = (struct rte_udp_hdr *)((char *)l3_hdr + info->l3_len); > /* do not recalculate udp cksum if it was 0 */ > if (udp_hdr->dgram_cksum != 0) { > - udp_hdr->dgram_cksum = 0; > - if (tx_offloads & DEV_TX_OFFLOAD_UDP_CKSUM) > + if (tx_offloads & DEV_TX_OFFLOAD_UDP_CKSUM) { > ol_flags |= PKT_TX_UDP_CKSUM; > - else { > + } else { > + udp_hdr->dgram_cksum = 0; > udp_hdr->dgram_cksum = > get_udptcp_checksum(l3_hdr, udp_hdr, > info->ethertype); > @@ -514,12 +515,12 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info, > ol_flags |= PKT_TX_UDP_SEG; > } else if (info->l4_proto == IPPROTO_TCP) { > tcp_hdr = (struct rte_tcp_hdr *)((char *)l3_hdr + info->l3_len); > - tcp_hdr->cksum = 0; > if (tso_segsz) > ol_flags |= PKT_TX_TCP_SEG; > - else if (tx_offloads & DEV_TX_OFFLOAD_TCP_CKSUM) > + else if (tx_offloads & DEV_TX_OFFLOAD_TCP_CKSUM) { > ol_flags |= PKT_TX_TCP_CKSUM; > - else { > + } else if (tcp_hdr->cksum != 0) { > + tcp_hdr->cksum = 0; > tcp_hdr->cksum = > get_udptcp_checksum(l3_hdr, tcp_hdr, > info->ethertype); > @@ -529,13 +530,13 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info, > } else if (info->l4_proto == IPPROTO_SCTP) { > sctp_hdr = (struct rte_sctp_hdr *) > ((char *)l3_hdr + info->l3_len); > - sctp_hdr->cksum = 0; > /* sctp payload must be a multiple of 4 to be > * offloaded */ > if ((tx_offloads & DEV_TX_OFFLOAD_SCTP_CKSUM) && > ((ipv4_hdr->total_length & 0x3) == 0)) { > ol_flags |= PKT_TX_SCTP_CKSUM; > - } else { > + } else if (sctp_hdr->cksum != 0) { > + sctp_hdr->cksum = 0; > /* XXX implement CRC32c, example available in > * RFC3309 */ > } > -- > 2.32.0 >