From: Olivier Matz <olivier.matz@6wind.com>
To: Flavio Leitner <fbl@sysclose.org>
Cc: David Marchand <david.marchand@redhat.com>,
dev@dpdk.org, maxime.coquelin@redhat.com, i.maximets@ovn.org,
Keith Wiles <keith.wiles@intel.com>
Subject: Re: [dpdk-dev] [PATCH 2/5] net/tap: do not touch Tx offload flags
Date: Thu, 8 Apr 2021 14:05:21 +0200 [thread overview]
Message-ID: <20210408120521.GW1650@platinum> (raw)
In-Reply-To: <YG7nVtJ+Ecxf8MNN@p50.lan>
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 <david.marchand@redhat.com>
> > > > ---
> > >
> > > 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.
next prev parent reply other threads:[~2021-04-08 12:05 UTC|newest]
Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-01 9:52 [dpdk-dev] [PATCH 0/5] Offload flags fixes David Marchand
2021-04-01 9:52 ` [dpdk-dev] [PATCH 1/5] mbuf: mark old offload flag as deprecated David Marchand
2021-04-07 20:14 ` Flavio Leitner
2021-04-08 7:23 ` Olivier Matz
2021-04-08 8:41 ` David Marchand
2021-04-01 9:52 ` [dpdk-dev] [PATCH 2/5] net/tap: do not touch Tx offload flags David Marchand
2021-04-07 20:15 ` Flavio Leitner
2021-04-08 7:41 ` Olivier Matz
2021-04-08 11:21 ` Flavio Leitner
2021-04-08 12:05 ` Olivier Matz [this message]
2021-04-08 12:58 ` Flavio Leitner
2021-04-09 13:30 ` Olivier Matz
2021-04-09 16:55 ` Flavio Leitner
2021-04-28 12:17 ` David Marchand
2021-04-08 12:16 ` Ananyev, Konstantin
2021-04-08 7:53 ` Olivier Matz
2021-04-28 12:12 ` David Marchand
2021-04-01 9:52 ` [dpdk-dev] [PATCH 3/5] net/virtio: " David Marchand
2021-04-13 14:17 ` Maxime Coquelin
2021-04-01 9:52 ` [dpdk-dev] [PATCH 4/5] net/virtio: refactor Tx offload helper David Marchand
2021-04-08 13:05 ` Flavio Leitner
2021-04-09 2:31 ` Ruifeng Wang
2021-04-01 9:52 ` [dpdk-dev] [PATCH 5/5] vhost: fix offload flags in Rx path David Marchand
2021-04-08 8:28 ` Olivier Matz
2021-04-08 18:38 ` Flavio Leitner
2021-04-13 15:27 ` Maxime Coquelin
2021-04-27 17:09 ` David Marchand
2021-04-27 17:19 ` David Marchand
2021-04-29 8:04 ` [dpdk-dev] [PATCH v2 0/4] Offload flags fixes David Marchand
2021-04-29 8:04 ` [dpdk-dev] [PATCH v2 1/4] mbuf: mark old offload flag as deprecated David Marchand
2021-04-29 12:14 ` Lance Richardson
2021-04-29 16:45 ` Ajit Khaparde
2021-04-29 8:04 ` [dpdk-dev] [PATCH v2 2/4] net/virtio: do not touch Tx offload flags David Marchand
2021-04-29 13:51 ` Flavio Leitner
2021-04-29 8:04 ` [dpdk-dev] [PATCH v2 3/4] net/virtio: refactor Tx offload helper David Marchand
2021-04-29 12:59 ` Maxime Coquelin
2021-04-29 8:04 ` [dpdk-dev] [PATCH v2 4/4] vhost: fix offload flags in Rx path David Marchand
2021-04-29 13:30 ` Maxime Coquelin
2021-04-29 13:31 ` Maxime Coquelin
2021-04-29 20:21 ` David Marchand
2021-04-30 8:38 ` Maxime Coquelin
2021-04-29 20:09 ` David Marchand
2021-04-29 18:39 ` Flavio Leitner
2021-04-29 19:18 ` David Marchand
2021-05-03 13:26 ` [dpdk-dev] [PATCH v3 0/4] Offload flags fixes David Marchand
2021-05-03 13:26 ` [dpdk-dev] [PATCH v3 1/4] mbuf: mark old offload flag as deprecated David Marchand
2021-05-03 14:02 ` Maxime Coquelin
2021-05-03 14:12 ` David Marchand
2021-05-03 13:26 ` [dpdk-dev] [PATCH v3 2/4] net/virtio: do not touch Tx offload flags David Marchand
2021-05-03 13:26 ` [dpdk-dev] [PATCH v3 3/4] net/virtio: refactor Tx offload helper David Marchand
2021-05-03 13:26 ` [dpdk-dev] [PATCH v3 4/4] vhost: fix offload flags in Rx path David Marchand
2021-05-03 15:24 ` [dpdk-dev] [PATCH v3 0/4] Offload flags fixes Maxime Coquelin
2021-05-03 16:21 ` David Marchand
2021-05-03 16:43 ` [dpdk-dev] [PATCH v4 0/3] " David Marchand
2021-05-03 16:43 ` [dpdk-dev] [PATCH v4 1/3] net/virtio: do not touch Tx offload flags David Marchand
2021-05-03 16:43 ` [dpdk-dev] [PATCH v4 2/3] net/virtio: refactor Tx offload helper David Marchand
2021-05-03 16:43 ` [dpdk-dev] [PATCH v4 3/3] vhost: fix offload flags in Rx path David Marchand
2021-05-04 11:07 ` Flavio Leitner
2021-05-08 6:24 ` Wang, Yinan
2021-05-12 3:29 ` Wang, Yinan
2021-05-12 15:20 ` David Marchand
2021-05-13 6:34 ` Wang, Yinan
2021-05-04 8:29 ` [dpdk-dev] [PATCH v4 0/3] Offload flags fixes Maxime Coquelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210408120521.GW1650@platinum \
--to=olivier.matz@6wind.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=fbl@sysclose.org \
--cc=i.maximets@ovn.org \
--cc=keith.wiles@intel.com \
--cc=maxime.coquelin@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).