Re: [dpdk-dev] [PATCH 1/3] mbuf: remove deprecated offload flags

[David Marchand <david.marchand@redhat.com>]

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.


[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?


-- 
David Marchand