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 18A9DA0579; Fri, 9 Apr 2021 15:30:21 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id ED9AF4068B; Fri, 9 Apr 2021 15:30:20 +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 9B0314014D for ; Fri, 9 Apr 2021 15:30:19 +0200 (CEST) Received: by mail-wr1-f44.google.com with SMTP id q26so5645539wrz.9 for ; Fri, 09 Apr 2021 06:30:19 -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:user-agent; bh=IgXnUANVxYXaqXyN+kgG28d0pwOJxcHs/akEutj+6Y4=; b=FpuugO+UrLLh2zWFWBPgjT2C14Y/UZl+MpewLWM3mMlLDraKXGP4W28CtP9WhrNT25 PBnFpRakFp4TD/9yqKgH9/r4Kiv1iyKfu1X25G6BAVuG5IBrHhJhHYSMycIfvaV/Ubbz Efu+KJI1q+DLGsZTsuuFsdRBCjwpOCtBU/JdfrEsOv6fNecnYBfO0byJ8DEYZE129o2B hXEOj35CjmGcvBhhEJoDMMVYbZoXCq/RKsEK6L1gj7wg20jho9YZ40Ss6GuBP9bYEEo8 Up0VV8kA24xM3aG6CzDMkwQch0vASBPzEe1mEdjfWtJZkp1RSJJnX56ZPLdr4cLUACP3 bcyQ== 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:user-agent; bh=IgXnUANVxYXaqXyN+kgG28d0pwOJxcHs/akEutj+6Y4=; b=bbpTCAkJVMG6A3pTlO+VZD6rTWnQKkoRFEctNaMLr4QsaniqsXxMynrMH7f4QsTNXv qYU96B22TLsK9Fz17HBeA9UFNbHVcYjhuh3eDr80upDzZ0bCFYWu6m0xsJEDKNNhIPmF Kxom6VfzVNIcizeM/60PUNQ6cOG9Tc/OWp2WhWP4dMndIr9Y27CxNMAUKs3TYRRVwup9 sE/WMG1HUNTKPmCo8zgKtZNRkYZ4vlwsSHNKpuiDbUJyeROvOSOu2SZ6S5iRCsJ5pyex sCA6QHr6J7T3aVbosQBdELsTfqE5d4Gf5rkigQsDt9DoYu+GToq6cqbgDekxBHB3ll+A rjgA== X-Gm-Message-State: AOAM533/EkrdxTzcbtQJXs5S5YgWxfLSrrhvLKRefn0MecgDdZO1k8Wj NvNYxO5B3ETf1z7tkajs9itMvg== X-Google-Smtp-Source: ABdhPJxqc05ob1EuSmrD9XpOUb4p/9hXo4wosZ/UE7pgwLmBP8E5yVEf9jJBYVpQpNpdl3a2BkWhVg== X-Received: by 2002:adf:ebcc:: with SMTP id v12mr18017789wrn.389.1617975019366; Fri, 09 Apr 2021 06:30:19 -0700 (PDT) Received: from 6wind.com ([2a01:e0a:5ac:6460:c065:401d:87eb:9b25]) by smtp.gmail.com with ESMTPSA id r5sm4679397wrx.87.2021.04.09.06.30.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 09 Apr 2021 06:30:18 -0700 (PDT) Date: Fri, 9 Apr 2021 15:30:18 +0200 From: Olivier Matz To: Flavio Leitner Cc: David Marchand , dev@dpdk.org, maxime.coquelin@redhat.com, i.maximets@ovn.org, Keith Wiles Message-ID: <20210409133018.GB1650@platinum> References: <20210401095243.18211-1-david.marchand@redhat.com> <20210401095243.18211-3-david.marchand@redhat.com> <20210408074159.GQ1650@platinum> <20210408120521.GW1650@platinum> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [PATCH 2/5] net/tap: do not touch Tx offload flags 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, Apr 08, 2021 at 09:58:35AM -0300, Flavio Leitner wrote: > On Thu, Apr 08, 2021 at 02:05:21PM +0200, Olivier Matz wrote: > > On Thu, Apr 08, 2021 at 08:21:58AM -0300, Flavio Leitner wrote: > > > On Thu, Apr 08, 2021 at 09:41:59AM +0200, Olivier Matz wrote: > > > > On Wed, Apr 07, 2021 at 05:15:39PM -0300, Flavio Leitner wrote: > > > > > On Thu, Apr 01, 2021 at 11:52:40AM +0200, David Marchand wrote: > > > > > > Tx offload flags are of the application responsibility. > > > > > > Leave the mbuf alone and check for TSO where needed. > > > > > > > > > > > > Signed-off-by: David Marchand > > > > > > --- > > > > > > > > > > The patch looks good, but maybe a better approach would be > > > > > to change the documentation to require the TCP_CKSUM flag > > > > > when TCP_SEG is used, otherwise this flag adjusting needs > > > > > to be replicated every time TCP_SEG is used. > > > > > > > > > > The above could break existing applications, so perhaps doing > > > > > something like below would be better and backwards compatible? > > > > > Then we can remove those places tweaking the flags completely. > > > > > > > > As a first step, I suggest to document that: > > > > - applications must set TCP_CKSUM when setting TCP_SEG > > > > > > That's what I suggest above. > > > > > > > - pmds must suppose that TCP_CKSUM is set when TCP_SEG is set > > > > > > But that keeps the problem of implying the TCP_CKSUM flag in > > > various places. > > > > Yes. What I propose is just a first step: better document what is the > > current expected behavior, before doing something else. > > > > > > This is clearer that what we have today, and I think it does not break > > > > anything. This will guide apps in the correct direction, facilitating > > > > an eventual future PMD change. > > > > > > > > > diff --git a/lib/librte_mbuf/rte_mbuf_core.h b/lib/librte_mbuf/rte_mbuf_core.h > > > > > index c17dc95c5..6a0c2cdd9 100644 > > > > > --- a/lib/librte_mbuf/rte_mbuf_core.h > > > > > +++ b/lib/librte_mbuf/rte_mbuf_core.h > > > > > @@ -298,7 +298,7 @@ extern "C" { > > > > > * - if it's IPv4, set the PKT_TX_IP_CKSUM flag > > > > > * - fill the mbuf offload information: l2_len, l3_len, l4_len, tso_segsz > > > > > */ > > > > > -#define PKT_TX_TCP_SEG (1ULL << 50) > > > > > +#define PKT_TX_TCP_SEG (1ULL << 50) | PKT_TX_TCP_CKSUM > > > > > > > > > > /** TX IEEE1588 packet to timestamp. */ > > > > > #define PKT_TX_IEEE1588_TMST (1ULL << 51) > > > > > > > > I'm afraid some applications or drivers use extended bit manipulations > > > > to do the conversion from/to another domain (like hardware descriptors > > > > or application-specific flags). They may expect this constant to be a > > > > uniq flag. > > > > > > Interesting, do you have an example? Because each flag still has an > > > separate meaning. > > > > Honnestly no, I don't have any good example, just a (maybe unfounded) doubt. > > > > I have in mind operations that are done with tables or vector > > instructions inside the drivers, but this is mainly done for Rx, not Tx. > > You can look at Tx functions like mlx5_set_cksum_table() or > > nix_xmit_pkts_vector(), or Rx functions like desc_to_olflags_v() or > > enic_noscatter_vec_recv_pkts() to see what kind of stuff I'm talking > > about. > > I see your point. Going back to improving the documentation as a > first step, what would be the next steps? Are we going to wait few > releases and then remove the flag tweaking code assuming that PMDs > and apps are ok? After this documentation step, in few releases, we could relax the constraint on PMD: applications will be expected to set TCP_CKSUM when TCP_SEG is set, so no need for the PMD to force TCP_CKSUM to 1 if TCP_SEG is set. The documentation will be updated again. This plan can be described in the deprecation notice, and later in the release note. How does it sound?