DPDK patches and discussions
 help / color / mirror / Atom feed
From: Olivier Matz <olivier.matz@6wind.com>
To: David Marchand <david.marchand@redhat.com>
Cc: dev <dev@dpdk.org>,
	Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
	"Yigit, Ferruh" <ferruh.yigit@intel.com>,
	Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [PATCH 1/3] mbuf: remove deprecated offload flags
Date: Mon, 4 Oct 2021 11:46:29 +0200	[thread overview]
Message-ID: <YVrNdfg2fM9ON516@platinum> (raw)
In-Reply-To: <CAJFAV8yw+kKVhZ1DH9nYBADeH5Ue6KUpRD1UBqyt02hoNoi7fw@mail.gmail.com>

Hi David,

Thank you for the review, my comments below.

On Mon, Oct 04, 2021 at 10:29:36AM +0200, David Marchand wrote:
> On Wed, Sep 29, 2021 at 11:50 PM Olivier Matz <olivier.matz@6wind.com> wrote:
> >
> > The flags PKT_TX_VLAN_PKT, PKT_TX_QINQ_PKT, PKT_RX_EIP_CKSUM_BAD are
> > marked as deprecated since commit 380a7aab1ae2 ("mbuf: rename deprecated
> > VLAN flags") (2017). Remove their definitions from rte_mbuf_core.h,
> > and replace their usages.
> 
> The patch lgtm except the removal of some "bad checksum" flags, see below.
>
> [snip]
> 
> 
> > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> > index 05fc2fdee7..549e9416c4 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -159,11 +159,6 @@ Deprecation Notices
> >    will be limited to maximum 256 queues.
> >    Also compile time flag ``RTE_ETHDEV_QUEUE_STAT_CNTRS`` will be removed.
> >
> > -* ethdev: The offload flag ``PKT_RX_EIP_CKSUM_BAD`` will be removed and
> > -  replaced by the new flag ``PKT_RX_OUTER_IP_CKSUM_BAD``. The new name is more
> > -  consistent with existing outer header checksum status flag naming, which
> > -  should help in reducing confusion about its usage.
> > -
> >  * i40e: As there are both i40evf and iavf pmd, the functions of them are
> >    duplicated. And now more and more advanced features are developed on iavf.
> >    To keep consistent with kernel driver's name
> 
> Those 3 flags are easy to replace, but some projects are still using them.
> 
> $ git grep-all -El
> '\<(PKT_TX_VLAN_PKT|PKT_TX_QINQ_PKT|PKT_RX_EIP_CKSUM_BAD)\>' |grep -v
> \\.patch$
> DPVS/src/netif.c
> DPVS/src/vlan.c
> FD.io-VPP/src/plugins/dpdk/device/format.c
> gatekeeper/bpf/bpf_mbuf.h
> lagopus/src/dataplane/dpdk/worker.c
> packet-journey/app/main.c
> Trex/src/pal/common/common_mbuf.h
> Trex/src/pal/linux/mbuf.h
> 
> Please update the release notes to announce this API update.

I will add an additional note in the release note.

FYI, the flags PKT_TX_VLAN_PKT and PKT_TX_QINQ_PKT were not marked
RTE_DEPRECATED because their deprecation is older than this macro. If needed, I
can keep them for one more version in the header file.

> [snip]
> 
> > diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
> > index 9d8e3ddc86..93db9292c0 100644
> > --- a/lib/mbuf/rte_mbuf_core.h
> > +++ b/lib/mbuf/rte_mbuf_core.h
> > @@ -55,37 +55,12 @@ extern "C" {
> >   /** RX packet with FDIR match indicate. */
> >  #define PKT_RX_FDIR          (1ULL << 2)
> >
> > -/**
> > - * Deprecated.
> > - * Checking this flag alone is deprecated: check the 2 bits of
> > - * PKT_RX_L4_CKSUM_MASK.
> > - * This flag was set when the L4 checksum of a packet was detected as
> > - * wrong by the hardware.
> > - */
> > -#define PKT_RX_L4_CKSUM_BAD  (1ULL << 3)
> > -
> > -/**
> > - * Deprecated.
> > - * Checking this flag alone is deprecated: check the 2 bits of
> > - * PKT_RX_IP_CKSUM_MASK.
> > - * This flag was set when the IP checksum of a packet was detected as
> > - * wrong by the hardware.
> > - */
> > -#define PKT_RX_IP_CKSUM_BAD  (1ULL << 4)
> > -
> 
> You did not mention PKT_RX_IP_CKSUM_BAD and PKT_RX_L4_CKSUM_BAD in the
> commitlog.
> There was no deprecation notice, and those flags were not marked
> RTE_DEPRECATED (there are still many projects referencing them).
> 
> Is this removal intended?

Yes, actually these flags were defined twice at different places. I'm just
removing one occurence, and the other remains.

Thanks,
Olivier

  reply	other threads:[~2021-10-04  9:46 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-29 21:48 [dpdk-dev] [PATCH 0/3] mbuf: offload flags namespace Olivier Matz
2021-09-29 21:48 ` [dpdk-dev] [PATCH 1/3] mbuf: remove deprecated offload flags Olivier Matz
2021-10-04  8:29   ` David Marchand
2021-10-04  9:46     ` Olivier Matz [this message]
2021-09-29 21:48 ` [dpdk-dev] [PATCH 2/3] cocci: add cocci script to rename mbuf " Olivier Matz
2021-09-29 21:48 ` [dpdk-dev] [PATCH 3/3] mbuf: add rte prefix to " Olivier Matz
2021-10-15 19:24 ` [dpdk-dev] [PATCH v2 0/4] mbuf: offload flags namespace Olivier Matz
2021-10-15 19:24   ` [dpdk-dev] [PATCH v2 1/4] mbuf: remove duplicate definition of cksum offload flags Olivier Matz
2021-10-16  7:47     ` Andrew Rybchenko
2021-10-15 19:24   ` [dpdk-dev] [PATCH v2 2/4] mbuf: mark old VLAN offload flags as deprecated Olivier Matz
2021-10-16  7:50     ` Andrew Rybchenko
2021-10-17 14:46       ` Ajit Khaparde
2021-10-18  8:10       ` Olivier Matz
2021-10-18 11:15         ` Somnath Kotur
2021-10-15 19:24   ` [dpdk-dev] [PATCH v2 3/4] cocci: add cocci script to rename mbuf offload flags Olivier Matz
2021-10-15 19:24   ` [dpdk-dev] [PATCH v2 4/4] mbuf: add rte prefix to " Olivier Matz
2021-10-16  7:57     ` Andrew Rybchenko
2021-10-17 14:45       ` Ajit Khaparde
2021-10-18 11:15         ` Somnath Kotur
2021-10-24 11:43   ` [dpdk-dev] [PATCH v2 0/4] mbuf: offload flags namespace David Marchand

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=YVrNdfg2fM9ON516@platinum \
    --to=olivier.matz@6wind.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=thomas@monjalon.net \
    /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).