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 E18A0A034F; Thu, 29 Jul 2021 18:02:11 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 61D9C40687; Thu, 29 Jul 2021 18:02:11 +0200 (CEST) Received: from mail-wr1-f44.google.com (mail-wr1-f44.google.com [209.85.221.44]) by mails.dpdk.org (Postfix) with ESMTP id E70CC4067A for ; Thu, 29 Jul 2021 18:02:09 +0200 (CEST) Received: by mail-wr1-f44.google.com with SMTP id z4so7554814wrv.11 for ; Thu, 29 Jul 2021 09:02:09 -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=n9zwqkGgCeUa7TtiKT6IYwn21KwN1IaSQdeVXBKlW2Y=; b=aBjX5vxSW81Jncw+eZRSHMCM7M/ygjUiwmrUlXdzqYwBPOkx+ZgleEVt15IAxkowIp C1qM6vM/M+4HU9kN/0xsLxwLHmb086l4bgqJn9FZFQYp+rSqeKKO615RKbfWrFLJqaPE ixlAxDWHOwoJudIlkt8q/zLXovGNM9nYyBwm+QbqFs3JXpI0h+hmH/oFZyMzlr8UcPnR P8PoEuXGR4nSCc58pybpabEb6Q3gyEK2EKvO3ZBBgIIomWRAV9tRJL2hwBGibQhpJ+qT noCH8TMqvc3zHz2IUfEB0DwardWx3l/Jn7WV9P5IMT+1M2+kcuPUszduBPcPRVr5mmrq xszA== 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=n9zwqkGgCeUa7TtiKT6IYwn21KwN1IaSQdeVXBKlW2Y=; b=Am1tE0/MHjmfSy1Xde1YBo3OIKdEMgiBn+GaEEbtpdoFixtvVYO/mVqatZ+q/NqZXC 0TrfQysM9EZtAP3BJ/mOild3ErTxhNcKlRSnKiLXvktyxriSptpzTWNY0ItuE5yXbq82 uoZHbgcKcL0CViE4DPdDo/xDEbF8QlDRqdCyts6Uo1WpxJMOYCxjPA2mCNIFgxg3l8V3 bn4pwHpQ0uTjvmTZLobRqRfP3n8jJir6p3QLi9Tt2PXCHLIvdkV4lmgwDP81eeFav2w6 J3VxKB3LSRN55HKZOnWVHXVBtdDBJFcLCqpMAZAIEIE+eMYM4rT8013o4LRwyjjoV0TC QN0g== X-Gm-Message-State: AOAM530yU5vzprId7R/GbEnOrlTE3/i++xUi/6AMR9tunYPVHdACB5tA H5nHip+U1dAxBiIYAf7UfVupNg== X-Google-Smtp-Source: ABdhPJyFbhEUCnmseE9rA3a37FLoa9vz66//p7Ha+oX992M/V1x9CYDXW6qO7IUQvbPFfYjpTJPpNQ== X-Received: by 2002:adf:e3d2:: with SMTP id k18mr5655466wrm.212.1627574529586; Thu, 29 Jul 2021 09:02:09 -0700 (PDT) Received: from 6wind.com ([2a01:e0a:5ac:6460:c065:401d:87eb:9b25]) by smtp.gmail.com with ESMTPSA id c10sm7141623wml.12.2021.07.29.09.02.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 29 Jul 2021 09:02:08 -0700 (PDT) Date: Thu, 29 Jul 2021 18:02:07 +0200 From: Olivier Matz To: Gregory Etelson Cc: "dev@dpdk.org" , Ajit Khaparde , Andrew Rybchenko , Ferruh Yigit , NBU-Contact-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: Subject: Re: [dpdk-dev] [PATCH v2] app/testpmd: fix TX checksum calculation for tunnel 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" On Thu, Jul 29, 2021 at 10:31:45AM +0000, Gregory Etelson wrote: > Hello Olivier, > > [:snip:] > > > > > > Correct. Inner checksum is offloaded and outer computed in software. > > > > I think this approach is not sane: the value of the outer checksum depends on > > the inner checksum, so it has to be calculated after. There is a comment in the > > code about this: > > > > /* Then process outer headers if any. Note that the software > > * checksum will be wrong if one of the inner checksums is > > * processed in hardware. */ > > if (info.is_tunnel == 1) { > > tx_ol_flags |= process_outer_cksums(outer_l3_hdr, &info, > > tx_offloads, > > !!(tx_ol_flags & PKT_TX_TCP_SEG)); > > } > > I agree. That computation order led to error in my case. > What if the engine could analyze port TX offload options and select > processing order according to existing configuration ? I really think hardware inner checksum + software outer checksum is broken by design, because outer checksum depends on inner checksum. > > > Consider this example: > > > Tunneled packet arrived at port A and being forwarded through port B. > > > The packet arrived at port A with correct inner checksums - L3 and L4. > > > Port B TX offloads inner L3 only. > > > > > > process_inner_cksums() sets "ipv4_hdr->hdr_checksum = 0;" > > unconditionally. > > > Inner L3 checksum value will be restored by port B TX checksum > > > offload, but when > > > process_outer_cksums() runs software calculation on outer L4 it will use 0 > > and produce wrong result. > > > > > > Therefore, the patch zeros inner checksum values only before actual > > software calculations. > > > > I better understand your use case, thanks. > > > > However, with your patch, if the inner L4 checksum is wrong when it arrives > > on port A, I think it will result in a packet with a wrong outer > > L4 checksum and a correct inner L4 checksum. Is it what you expect? > > > > Wrong checksum reflects ongoing issue that must be investigated and fixed. > I don't expect forwarding engine to fix wrong checksum because it can hide > a real problem. Yes and no :) We should keep in mind that this engine is a demo / test program for checksum API. A real-world application should detect and drop a packet with a wrong checksum. > > I don't argue against the patch itself. What you suggest better matches the > > offload API than what we have today. Can you please send another version > > that better explains the use-case? > > > > I posted v3 with updated comment. > > > One more suggestion, maybe for later. Currently, the csumonly engine can be > > configured to do the checksum in sw or in hw. Maybe we could add a "dont- > > touch" option, to keep the value in the packet. Would it help for your use- > > case? > > > > That's a very good idea. > Packets can arrive with pre-calculated correct checksums. Keeping these values > can save processing time. > > [:snip:] > > Regards, > Gregory