All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] doc: announce header split deprecation
@ 2022-05-23 14:20 xuan.ding
  2022-05-24 14:58 ` Ray Kinsella
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: xuan.ding @ 2022-05-23 14:20 UTC (permalink / raw)
  To: thomas, andrew.rybchenko, mdr; +Cc: stephen, mb, dev, Xuan Ding

From: Xuan Ding <xuan.ding@intel.com>

RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced some time ago to
substitute bit-field header_split in struct rte_eth_rxmode. It allows
to enable header split offload with the header size controlled using
split_hdr_size in the same structure.

Right now, no single PMD actually supports RTE_ETH_RX_OFFLOAD_HEADER_SPLIT
with above definition. Many examples and test apps initialize the field
to 0 explicitly. The most of drivers simply ignore split_hdr_size since
the offload is not advertised, but some double-check that its value is 0.

So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and split_header_size field
will be removed in DPDK 22.11.

Signed-off-by: Xuan Ding <xuan.ding@intel.com>
---
 doc/guides/rel_notes/deprecation.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 4e5b23c53d..b8114f29ed 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -125,3 +125,7 @@ Deprecation Notices
   applications should be updated to use the ``dmadev`` library instead,
   with the underlying HW-functionality being provided by the ``ioat`` or
   ``idxd`` dma drivers
+
+* ethdev: After bit-field header split was removed, the ``RTE_ETH_RX_OFFLOAD_HEADER_SPLIT``
+offload and the ``split_hdr_size`` field in structure ``rte_eth_rxmode`` to enable header
+split offload are not supported in any PMDs. They will be removed in DPDK 22.11.
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] doc: announce header split deprecation
  2022-05-23 14:20 [PATCH] doc: announce header split deprecation xuan.ding
@ 2022-05-24 14:58 ` Ray Kinsella
  2022-07-12 11:43   ` Andrew Rybchenko
  2022-07-13  9:49 ` Thomas Monjalon
  2022-07-15 20:30 ` [PATCH v2] " xuan.ding
  2 siblings, 1 reply; 20+ messages in thread
From: Ray Kinsella @ 2022-05-24 14:58 UTC (permalink / raw)
  To: xuan.ding; +Cc: thomas, andrew.rybchenko, stephen, mb, dev


xuan.ding@intel.com writes:

> From: Xuan Ding <xuan.ding@intel.com>
>
> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced some time ago to
> substitute bit-field header_split in struct rte_eth_rxmode. It allows
> to enable header split offload with the header size controlled using
> split_hdr_size in the same structure.
>
> Right now, no single PMD actually supports RTE_ETH_RX_OFFLOAD_HEADER_SPLIT
> with above definition. Many examples and test apps initialize the field
> to 0 explicitly. The most of drivers simply ignore split_hdr_size since
> the offload is not advertised, but some double-check that its value is 0.
>
> So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and split_header_size field
> will be removed in DPDK 22.11.
>
> Signed-off-by: Xuan Ding <xuan.ding@intel.com>


Acked-by: Ray Kinsella <mdr@ashroe.eu>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] doc: announce header split deprecation
  2022-05-24 14:58 ` Ray Kinsella
@ 2022-07-12 11:43   ` Andrew Rybchenko
  0 siblings, 0 replies; 20+ messages in thread
From: Andrew Rybchenko @ 2022-07-12 11:43 UTC (permalink / raw)
  To: Ray Kinsella, xuan.ding; +Cc: thomas, stephen, mb, dev

On 5/24/22 17:58, Ray Kinsella wrote:
> 
> xuan.ding@intel.com writes:
> 
>> From: Xuan Ding <xuan.ding@intel.com>
>>
>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced some time ago to
>> substitute bit-field header_split in struct rte_eth_rxmode. It allows
>> to enable header split offload with the header size controlled using
>> split_hdr_size in the same structure.
>>
>> Right now, no single PMD actually supports RTE_ETH_RX_OFFLOAD_HEADER_SPLIT
>> with above definition. Many examples and test apps initialize the field
>> to 0 explicitly. The most of drivers simply ignore split_hdr_size since
>> the offload is not advertised, but some double-check that its value is 0.
>>
>> So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and split_header_size field
>> will be removed in DPDK 22.11.
>>
>> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> 
> 
> Acked-by: Ray Kinsella <mdr@ashroe.eu>

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] doc: announce header split deprecation
  2022-05-23 14:20 [PATCH] doc: announce header split deprecation xuan.ding
  2022-05-24 14:58 ` Ray Kinsella
@ 2022-07-13  9:49 ` Thomas Monjalon
  2022-07-14  5:50   ` Ding, Xuan
  2022-07-15 20:30 ` [PATCH v2] " xuan.ding
  2 siblings, 1 reply; 20+ messages in thread
From: Thomas Monjalon @ 2022-07-13  9:49 UTC (permalink / raw)
  To: xuan.ding; +Cc: andrew.rybchenko, mdr, dev, stephen, mb, dev

23/05/2022 16:20, xuan.ding@intel.com:
> From: Xuan Ding <xuan.ding@intel.com>
> 
> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced some time ago to
> substitute bit-field header_split in struct rte_eth_rxmode. It allows
> to enable header split offload with the header size controlled using
> split_hdr_size in the same structure.
> 
> Right now, no single PMD actually supports RTE_ETH_RX_OFFLOAD_HEADER_SPLIT
> with above definition. Many examples and test apps initialize the field
> to 0 explicitly. The most of drivers simply ignore split_hdr_size since
> the offload is not advertised, but some double-check that its value is 0.
> 
> So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and split_header_size field
> will be removed in DPDK 22.11.
> 
> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> ---
>  doc/guides/rel_notes/deprecation.rst | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 4e5b23c53d..b8114f29ed 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -125,3 +125,7 @@ Deprecation Notices
>    applications should be updated to use the ``dmadev`` library instead,
>    with the underlying HW-functionality being provided by the ``ioat`` or
>    ``idxd`` dma drivers
> +
> +* ethdev: After bit-field header split was removed, the ``RTE_ETH_RX_OFFLOAD_HEADER_SPLIT``
> +offload and the ``split_hdr_size`` field in structure ``rte_eth_rxmode`` to enable header
> +split offload are not supported in any PMDs. They will be removed in DPDK 22.11.

It would have been good to talk about rte_eth_rxseg_split
which is similar and configured per-queue.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] doc: announce header split deprecation
  2022-07-13  9:49 ` Thomas Monjalon
@ 2022-07-14  5:50   ` Ding, Xuan
  2022-07-14  8:08     ` Thomas Monjalon
  0 siblings, 1 reply; 20+ messages in thread
From: Ding, Xuan @ 2022-07-14  5:50 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: andrew.rybchenko, mdr, dev, stephen, mb, dev, Zhang, Qi Z,
	asekhar, pbhagavatula, ferruh.yigit, grive

Hi,

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: 2022年7月13日 17:50
> To: Ding, Xuan <xuan.ding@intel.com>
> Cc: andrew.rybchenko@oktetlabs.ru; mdr@ashroe.eu; dev@dpdk.org;
> stephen@networkplumber.org; mb@smartsharesystems.com; dev@dpdk.org
> Subject: Re: [PATCH] doc: announce header split deprecation
> 
> 23/05/2022 16:20, xuan.ding@intel.com:
> > From: Xuan Ding <xuan.ding@intel.com>
> >
> > RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced some time
> ago
> > to substitute bit-field header_split in struct rte_eth_rxmode. It
> > allows to enable header split offload with the header size controlled
> > using split_hdr_size in the same structure.
> >
> > Right now, no single PMD actually supports
> > RTE_ETH_RX_OFFLOAD_HEADER_SPLIT with above definition. Many examples
> > and test apps initialize the field to 0 explicitly. The most of
> > drivers simply ignore split_hdr_size since the offload is not advertised, but
> some double-check that its value is 0.
> >
> > So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and split_header_size field
> > will be removed in DPDK 22.11.
> >
> > Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> > ---
> >  doc/guides/rel_notes/deprecation.rst | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/doc/guides/rel_notes/deprecation.rst
> > b/doc/guides/rel_notes/deprecation.rst
> > index 4e5b23c53d..b8114f29ed 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -125,3 +125,7 @@ Deprecation Notices
> >    applications should be updated to use the ``dmadev`` library instead,
> >    with the underlying HW-functionality being provided by the ``ioat`` or
> >    ``idxd`` dma drivers
> > +
> > +* ethdev: After bit-field header split was removed, the
> > +``RTE_ETH_RX_OFFLOAD_HEADER_SPLIT``
> > +offload and the ``split_hdr_size`` field in structure
> > +``rte_eth_rxmode`` to enable header split offload are not supported in any
> PMDs. They will be removed in DPDK 22.11.
> 
> It would have been good to talk about rte_eth_rxseg_split which is similar and
> configured per-queue.
 
Thanks for your suggestion.

But I'm a little confused, are you referring that I need to involve protocol based buffer split?
About the deprecation of header split, I haven't realized its connection to rte_eth_rxseg_split.

Currently there are 2 acks, add more PMD maintainers to help review this deprecation notice
for header split, thanks a lot!

Regards,
Xuan


> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] doc: announce header split deprecation
  2022-07-14  5:50   ` Ding, Xuan
@ 2022-07-14  8:08     ` Thomas Monjalon
  2022-07-14 12:54       ` Ding, Xuan
  0 siblings, 1 reply; 20+ messages in thread
From: Thomas Monjalon @ 2022-07-14  8:08 UTC (permalink / raw)
  To: Ding, Xuan
  Cc: andrew.rybchenko, mdr, dev, stephen, mb, dev, Zhang, Qi Z,
	asekhar, pbhagavatula, ferruh.yigit, grive

14/07/2022 07:50, Ding, Xuan:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 23/05/2022 16:20, xuan.ding@intel.com:
> > > From: Xuan Ding <xuan.ding@intel.com>
> > >
> > > RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced some time
> > ago
> > > to substitute bit-field header_split in struct rte_eth_rxmode. It
> > > allows to enable header split offload with the header size controlled
> > > using split_hdr_size in the same structure.
> > >
> > > Right now, no single PMD actually supports
> > > RTE_ETH_RX_OFFLOAD_HEADER_SPLIT with above definition. Many examples
> > > and test apps initialize the field to 0 explicitly. The most of
> > > drivers simply ignore split_hdr_size since the offload is not advertised, but
> > some double-check that its value is 0.
> > >
> > > So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and split_header_size field
> > > will be removed in DPDK 22.11.
> > >
> > > Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> > > ---
> > >  doc/guides/rel_notes/deprecation.rst | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/doc/guides/rel_notes/deprecation.rst
> > > b/doc/guides/rel_notes/deprecation.rst
> > > index 4e5b23c53d..b8114f29ed 100644
> > > --- a/doc/guides/rel_notes/deprecation.rst
> > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > @@ -125,3 +125,7 @@ Deprecation Notices
> > >    applications should be updated to use the ``dmadev`` library instead,
> > >    with the underlying HW-functionality being provided by the ``ioat`` or
> > >    ``idxd`` dma drivers
> > > +
> > > +* ethdev: After bit-field header split was removed, the
> > > +``RTE_ETH_RX_OFFLOAD_HEADER_SPLIT``
> > > +offload and the ``split_hdr_size`` field in structure
> > > +``rte_eth_rxmode`` to enable header split offload are not supported in any
> > PMDs. They will be removed in DPDK 22.11.
> > 
> > It would have been good to talk about rte_eth_rxseg_split which is similar and
> > configured per-queue.
>  
> Thanks for your suggestion.
> 
> But I'm a little confused, are you referring that I need to involve protocol based buffer split?
> About the deprecation of header split, I haven't realized its connection to rte_eth_rxseg_split.

What???
In old versions of your patch "ethdev: introduce protocol type based header split"
you wrote:
"
A new proto field is introduced in the
rte_eth_rxseg_split structure reserved field to specify header protocol
type. With Rx offload flag RTE_ETH_RX_OFFLOAD_HEADER_SPLIT enabled and
protocol type configured, PMD will split the ingress packets into two
separate regions.
"

> Currently there are 2 acks, add more PMD maintainers to help review this deprecation notice
> for header split, thanks a lot!

I cannot say my feeling strong enough.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] doc: announce header split deprecation
  2022-07-14  8:08     ` Thomas Monjalon
@ 2022-07-14 12:54       ` Ding, Xuan
  2022-07-14 13:25         ` Thomas Monjalon
  0 siblings, 1 reply; 20+ messages in thread
From: Ding, Xuan @ 2022-07-14 12:54 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: andrew.rybchenko, mdr, dev, stephen, mb, dev, Zhang, Qi Z,
	asekhar, pbhagavatula, ferruh.yigit, grive

Hi,

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, July 14, 2022 4:09 PM
> To: Ding, Xuan <xuan.ding@intel.com>
> Cc: andrew.rybchenko@oktetlabs.ru; mdr@ashroe.eu; dev@dpdk.org;
> stephen@networkplumber.org; mb@smartsharesystems.com;
> dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; asekhar@marvell.com;
> pbhagavatula@marvell.com; ferruh.yigit@xilinx.com; grive@u256.net
> Subject: Re: [PATCH] doc: announce header split deprecation
> 
> 14/07/2022 07:50, Ding, Xuan:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 23/05/2022 16:20, xuan.ding@intel.com:
> > > > From: Xuan Ding <xuan.ding@intel.com>
> > > >
> > > > RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced some
> time
> > > ago
> > > > to substitute bit-field header_split in struct rte_eth_rxmode. It
> > > > allows to enable header split offload with the header size
> > > > controlled using split_hdr_size in the same structure.
> > > >
> > > > Right now, no single PMD actually supports
> > > > RTE_ETH_RX_OFFLOAD_HEADER_SPLIT with above definition. Many
> > > > examples and test apps initialize the field to 0 explicitly. The
> > > > most of drivers simply ignore split_hdr_size since the offload is
> > > > not advertised, but
> > > some double-check that its value is 0.
> > > >
> > > > So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and split_header_size
> field
> > > > will be removed in DPDK 22.11.
> > > >
> > > > Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> > > > ---
> > > >  doc/guides/rel_notes/deprecation.rst | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/doc/guides/rel_notes/deprecation.rst
> > > > b/doc/guides/rel_notes/deprecation.rst
> > > > index 4e5b23c53d..b8114f29ed 100644
> > > > --- a/doc/guides/rel_notes/deprecation.rst
> > > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > > @@ -125,3 +125,7 @@ Deprecation Notices
> > > >    applications should be updated to use the ``dmadev`` library instead,
> > > >    with the underlying HW-functionality being provided by the ``ioat`` or
> > > >    ``idxd`` dma drivers
> > > > +
> > > > +* ethdev: After bit-field header split was removed, the
> > > > +``RTE_ETH_RX_OFFLOAD_HEADER_SPLIT``
> > > > +offload and the ``split_hdr_size`` field in structure
> > > > +``rte_eth_rxmode`` to enable header split offload are not
> > > > +supported in any
> > > PMDs. They will be removed in DPDK 22.11.
> > >
> > > It would have been good to talk about rte_eth_rxseg_split which is
> > > similar and configured per-queue.
> >
> > Thanks for your suggestion.
> >
> > But I'm a little confused, are you referring that I need to involve protocol
> based buffer split?
> > About the deprecation of header split, I haven't realized its connection to
> rte_eth_rxseg_split.
> 
> What???
> In old versions of your patch "ethdev: introduce protocol type based header
> split"
> you wrote:
> "
> A new proto field is introduced in the
> rte_eth_rxseg_split structure reserved field to specify header protocol type.
> With Rx offload flag RTE_ETH_RX_OFFLOAD_HEADER_SPLIT enabled and
> protocol type configured, PMD will split the ingress packets into two separate
> regions.

It has a long history... 
It was corrected in v4 that RTE_ETH_RX_OFFLOAD_HEADER_SPLIT is used to enable header
split offload with the header size controlled using "split_hdr_size".
But no single PMD actually supports RTE_ETH_RX_OFFLOAD_HEADER_SPLIT for this purpose.
So we finally decide to deprecate this flag.

http://patchwork.dpdk.org/project/dpdk/patch/20220402104109.472078-2-wenxuanx.wu@intel.com/

In following series, I use RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT instead. It is for multi-segments packet
split. And it still needs a "proto_hdr" field in rte_eth_rxmode to configure split location.

> "
> 
> > Currently there are 2 acks, add more PMD maintainers to help review
> > this deprecation notice for header split, thanks a lot!
> 
> I cannot say my feeling strong enough.

So IMO the deprecation for header split is not relevant with buffer split. But we can still clean the code.
Hope it make things clearer.

Thanks,
Xuan

> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] doc: announce header split deprecation
  2022-07-14 12:54       ` Ding, Xuan
@ 2022-07-14 13:25         ` Thomas Monjalon
  2022-07-14 14:07           ` Ding, Xuan
  2022-07-15 12:13           ` Andrew Rybchenko
  0 siblings, 2 replies; 20+ messages in thread
From: Thomas Monjalon @ 2022-07-14 13:25 UTC (permalink / raw)
  To: Ding, Xuan, andrew.rybchenko, ferruh.yigit
  Cc: mdr, dev, stephen, mb, dev, Zhang, Qi Z, asekhar, pbhagavatula, grive

14/07/2022 14:54, Ding, Xuan:
> Hi,
> 
> From: Thomas Monjalon <thomas@monjalon.net>
> > 14/07/2022 07:50, Ding, Xuan:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 23/05/2022 16:20, xuan.ding@intel.com:
> > > > > From: Xuan Ding <xuan.ding@intel.com>
> > > > >
> > > > > RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced some
> > time
> > > > ago
> > > > > to substitute bit-field header_split in struct rte_eth_rxmode. It
> > > > > allows to enable header split offload with the header size
> > > > > controlled using split_hdr_size in the same structure.
> > > > >
> > > > > Right now, no single PMD actually supports
> > > > > RTE_ETH_RX_OFFLOAD_HEADER_SPLIT with above definition. Many
> > > > > examples and test apps initialize the field to 0 explicitly. The
> > > > > most of drivers simply ignore split_hdr_size since the offload is
> > > > > not advertised, but
> > > > some double-check that its value is 0.
> > > > >
> > > > > So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and split_header_size
> > field
> > > > > will be removed in DPDK 22.11.
> > > > >
> > > > > Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> > > > > ---
> > > > >  doc/guides/rel_notes/deprecation.rst | 4 ++++
> > > > >  1 file changed, 4 insertions(+)
> > > > >
> > > > > diff --git a/doc/guides/rel_notes/deprecation.rst
> > > > > b/doc/guides/rel_notes/deprecation.rst
> > > > > index 4e5b23c53d..b8114f29ed 100644
> > > > > --- a/doc/guides/rel_notes/deprecation.rst
> > > > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > > > @@ -125,3 +125,7 @@ Deprecation Notices
> > > > >    applications should be updated to use the ``dmadev`` library instead,
> > > > >    with the underlying HW-functionality being provided by the ``ioat`` or
> > > > >    ``idxd`` dma drivers
> > > > > +
> > > > > +* ethdev: After bit-field header split was removed, the
> > > > > +``RTE_ETH_RX_OFFLOAD_HEADER_SPLIT``
> > > > > +offload and the ``split_hdr_size`` field in structure
> > > > > +``rte_eth_rxmode`` to enable header split offload are not
> > > > > +supported in any
> > > > PMDs. They will be removed in DPDK 22.11.
> > > >
> > > > It would have been good to talk about rte_eth_rxseg_split which is
> > > > similar and configured per-queue.
> > >
> > > Thanks for your suggestion.
> > >
> > > But I'm a little confused, are you referring that I need to involve protocol
> > based buffer split?
> > > About the deprecation of header split, I haven't realized its connection to
> > rte_eth_rxseg_split.
> > 
> > What???
> > In old versions of your patch "ethdev: introduce protocol type based header
> > split"
> > you wrote:
> > "
> > A new proto field is introduced in the
> > rte_eth_rxseg_split structure reserved field to specify header protocol type.
> > With Rx offload flag RTE_ETH_RX_OFFLOAD_HEADER_SPLIT enabled and
> > protocol type configured, PMD will split the ingress packets into two separate
> > regions.
> > "
> 
> It has a long history... 
> It was corrected in v4 that RTE_ETH_RX_OFFLOAD_HEADER_SPLIT is used to enable header
> split offload with the header size controlled using "split_hdr_size".
> But no single PMD actually supports RTE_ETH_RX_OFFLOAD_HEADER_SPLIT for this purpose.
> So we finally decide to deprecate this flag.
> 
> http://patchwork.dpdk.org/project/dpdk/patch/20220402104109.472078-2-wenxuanx.wu@intel.com/
> 
> In following series, I use RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT instead. It is for multi-segments packet
> split. And it still needs a "proto_hdr" field in rte_eth_rxmode to configure split location.

I know this history because I was the one asking you to deprecate this.
But it seems you didn't get the big picture.

> > > Currently there are 2 acks, add more PMD maintainers to help review
> > > this deprecation notice for header split, thanks a lot!
> > 
> > I cannot say my feeling strong enough.
> 
> So IMO the deprecation for header split is not relevant with buffer split. But we can still clean the code.
> Hope it make things clearer.

They are almost the same features.
So when deprecating one, it is important to mention what remains.
If needed RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT can still be used
and it is configured per-queue,
while RTE_ETH_RX_OFFLOAD_HEADER_SPLIT was configurable per-port.

Andrew, Ferruh, do you agree to improve this deprecation notice
by adding above information?



^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] doc: announce header split deprecation
  2022-07-14 13:25         ` Thomas Monjalon
@ 2022-07-14 14:07           ` Ding, Xuan
  2022-07-14 15:58             ` Ferruh Yigit
  2022-07-15 12:13           ` Andrew Rybchenko
  1 sibling, 1 reply; 20+ messages in thread
From: Ding, Xuan @ 2022-07-14 14:07 UTC (permalink / raw)
  To: Thomas Monjalon, andrew.rybchenko, ferruh.yigit
  Cc: mdr, dev, stephen, mb, dev, Zhang, Qi Z, asekhar, pbhagavatula, grive

Hi,

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, July 14, 2022 9:25 PM
> To: Ding, Xuan <xuan.ding@intel.com>; andrew.rybchenko@oktetlabs.ru;
> ferruh.yigit@xilinx.com
> Cc: mdr@ashroe.eu; dev@dpdk.org; stephen@networkplumber.org;
> mb@smartsharesystems.com; dev@dpdk.org; Zhang, Qi Z
> <qi.z.zhang@intel.com>; asekhar@marvell.com; pbhagavatula@marvell.com;
> grive@u256.net
> Subject: Re: [PATCH] doc: announce header split deprecation
> 
> 14/07/2022 14:54, Ding, Xuan:
> > Hi,
> >
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 14/07/2022 07:50, Ding, Xuan:
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > 23/05/2022 16:20, xuan.ding@intel.com:
> > > > > > From: Xuan Ding <xuan.ding@intel.com>
> > > > > >
> > > > > > RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced
> some
> > > time
> > > > > ago
> > > > > > to substitute bit-field header_split in struct rte_eth_rxmode.
> > > > > > It allows to enable header split offload with the header size
> > > > > > controlled using split_hdr_size in the same structure.
> > > > > >
> > > > > > Right now, no single PMD actually supports
> > > > > > RTE_ETH_RX_OFFLOAD_HEADER_SPLIT with above definition. Many
> > > > > > examples and test apps initialize the field to 0 explicitly.
> > > > > > The most of drivers simply ignore split_hdr_size since the
> > > > > > offload is not advertised, but
> > > > > some double-check that its value is 0.
> > > > > >
> > > > > > So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and split_header_size
> > > field
> > > > > > will be removed in DPDK 22.11.
> > > > > >
> > > > > > Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> > > > > > ---
> > > > > >  doc/guides/rel_notes/deprecation.rst | 4 ++++
> > > > > >  1 file changed, 4 insertions(+)
> > > > > >
> > > > > > diff --git a/doc/guides/rel_notes/deprecation.rst
> > > > > > b/doc/guides/rel_notes/deprecation.rst
> > > > > > index 4e5b23c53d..b8114f29ed 100644
> > > > > > --- a/doc/guides/rel_notes/deprecation.rst
> > > > > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > > > > @@ -125,3 +125,7 @@ Deprecation Notices
> > > > > >    applications should be updated to use the ``dmadev`` library
> instead,
> > > > > >    with the underlying HW-functionality being provided by the ``ioat``
> or
> > > > > >    ``idxd`` dma drivers
> > > > > > +
> > > > > > +* ethdev: After bit-field header split was removed, the
> > > > > > +``RTE_ETH_RX_OFFLOAD_HEADER_SPLIT``
> > > > > > +offload and the ``split_hdr_size`` field in structure
> > > > > > +``rte_eth_rxmode`` to enable header split offload are not
> > > > > > +supported in any
> > > > > PMDs. They will be removed in DPDK 22.11.
> > > > >
> > > > > It would have been good to talk about rte_eth_rxseg_split which
> > > > > is similar and configured per-queue.
> > > >
> > > > Thanks for your suggestion.
> > > >
> > > > But I'm a little confused, are you referring that I need to
> > > > involve protocol
> > > based buffer split?
> > > > About the deprecation of header split, I haven't realized its
> > > > connection to
> > > rte_eth_rxseg_split.
> > >
> > > What???
> > > In old versions of your patch "ethdev: introduce protocol type based
> > > header split"
> > > you wrote:
> > > "
> > > A new proto field is introduced in the rte_eth_rxseg_split structure
> > > reserved field to specify header protocol type.
> > > With Rx offload flag RTE_ETH_RX_OFFLOAD_HEADER_SPLIT enabled and
> > > protocol type configured, PMD will split the ingress packets into
> > > two separate regions.
> > > "
> >
> > It has a long history...
> > It was corrected in v4 that RTE_ETH_RX_OFFLOAD_HEADER_SPLIT is used to
> > enable header split offload with the header size controlled using
> "split_hdr_size".
> > But no single PMD actually supports RTE_ETH_RX_OFFLOAD_HEADER_SPLIT
> for this purpose.
> > So we finally decide to deprecate this flag.
> >
> > http://patchwork.dpdk.org/project/dpdk/patch/20220402104109.472078-
> 2-w
> > enxuanx.wu@intel.com/
> >
> > In following series, I use RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT instead. It
> > is for multi-segments packet split. And it still needs a "proto_hdr" field in
> rte_eth_rxmode to configure split location.
> 
> I know this history because I was the one asking you to deprecate this.
> But it seems you didn't get the big picture.
> 
> > > > Currently there are 2 acks, add more PMD maintainers to help
> > > > review this deprecation notice for header split, thanks a lot!
> > >
> > > I cannot say my feeling strong enough.
> >
> > So IMO the deprecation for header split is not relevant with buffer split. But
> we can still clean the code.
> > Hope it make things clearer.
> 
> They are almost the same features.
> So when deprecating one, it is important to mention what remains.
> If needed RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT can still be used and it is
> configured per-queue, while RTE_ETH_RX_OFFLOAD_HEADER_SPLIT was
> configurable per-port.

Thanks for your clarification. It's clearer now.
I was trying to figure out the whole history of header split,
seems it is not enough.

> 
> Andrew, Ferruh, do you agree to improve this deprecation notice by adding
> above information?

Agree. It is better to point out the remaining per queue
rx offload RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT for splitting packets.

Please see v2 after I add more header split background.

Regards,
Xuan

> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] doc: announce header split deprecation
  2022-07-14 14:07           ` Ding, Xuan
@ 2022-07-14 15:58             ` Ferruh Yigit
  2022-07-14 16:03               ` Ferruh Yigit
  2022-07-14 16:56               ` Slava Ovsiienko
  0 siblings, 2 replies; 20+ messages in thread
From: Ferruh Yigit @ 2022-07-14 15:58 UTC (permalink / raw)
  To: Ding, Xuan, Thomas Monjalon, andrew.rybchenko, Viacheslav Ovsiienko
  Cc: mdr, dev, stephen, mb, Zhang, Qi Z, asekhar, pbhagavatula, grive

On 7/14/2022 3:07 PM, Ding, Xuan wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Thomas Monjalon <thomas@monjalon.net>
>> Sent: Thursday, July 14, 2022 9:25 PM
>> To: Ding, Xuan <xuan.ding@intel.com>; andrew.rybchenko@oktetlabs.ru;
>> ferruh.yigit@xilinx.com
>> Cc: mdr@ashroe.eu; dev@dpdk.org; stephen@networkplumber.org;
>> mb@smartsharesystems.com; dev@dpdk.org; Zhang, Qi Z
>> <qi.z.zhang@intel.com>; asekhar@marvell.com; pbhagavatula@marvell.com;
>> grive@u256.net
>> Subject: Re: [PATCH] doc: announce header split deprecation
>>
>> 14/07/2022 14:54, Ding, Xuan:
>>> Hi,
>>>
>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>> 14/07/2022 07:50, Ding, Xuan:
>>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>>>> 23/05/2022 16:20, xuan.ding@intel.com:
>>>>>>> From: Xuan Ding <xuan.ding@intel.com>
>>>>>>>
>>>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced
>> some
>>>> time
>>>>>> ago
>>>>>>> to substitute bit-field header_split in struct rte_eth_rxmode.
>>>>>>> It allows to enable header split offload with the header size
>>>>>>> controlled using split_hdr_size in the same structure.
>>>>>>>
>>>>>>> Right now, no single PMD actually supports
>>>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT with above definition. Many
>>>>>>> examples and test apps initialize the field to 0 explicitly.
>>>>>>> The most of drivers simply ignore split_hdr_size since the
>>>>>>> offload is not advertised, but
>>>>>> some double-check that its value is 0.
>>>>>>>
>>>>>>> So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and split_header_size
>>>> field
>>>>>>> will be removed in DPDK 22.11.
>>>>>>>
>>>>>>> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
>>>>>>> ---
>>>>>>>   doc/guides/rel_notes/deprecation.rst | 4 ++++
>>>>>>>   1 file changed, 4 insertions(+)
>>>>>>>
>>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>>>>>> b/doc/guides/rel_notes/deprecation.rst
>>>>>>> index 4e5b23c53d..b8114f29ed 100644
>>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
>>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>>>>>> @@ -125,3 +125,7 @@ Deprecation Notices
>>>>>>>     applications should be updated to use the ``dmadev`` library
>> instead,
>>>>>>>     with the underlying HW-functionality being provided by the ``ioat``
>> or
>>>>>>>     ``idxd`` dma drivers
>>>>>>> +
>>>>>>> +* ethdev: After bit-field header split was removed, the
>>>>>>> +``RTE_ETH_RX_OFFLOAD_HEADER_SPLIT``
>>>>>>> +offload and the ``split_hdr_size`` field in structure
>>>>>>> +``rte_eth_rxmode`` to enable header split offload are not
>>>>>>> +supported in any
>>>>>> PMDs. They will be removed in DPDK 22.11.
>>>>>>
>>>>>> It would have been good to talk about rte_eth_rxseg_split which
>>>>>> is similar and configured per-queue.
>>>>>
>>>>> Thanks for your suggestion.
>>>>>
>>>>> But I'm a little confused, are you referring that I need to
>>>>> involve protocol
>>>> based buffer split?
>>>>> About the deprecation of header split, I haven't realized its
>>>>> connection to
>>>> rte_eth_rxseg_split.
>>>>
>>>> What???
>>>> In old versions of your patch "ethdev: introduce protocol type based
>>>> header split"
>>>> you wrote:
>>>> "
>>>> A new proto field is introduced in the rte_eth_rxseg_split structure
>>>> reserved field to specify header protocol type.
>>>> With Rx offload flag RTE_ETH_RX_OFFLOAD_HEADER_SPLIT enabled and
>>>> protocol type configured, PMD will split the ingress packets into
>>>> two separate regions.
>>>> "
>>>
>>> It has a long history...
>>> It was corrected in v4 that RTE_ETH_RX_OFFLOAD_HEADER_SPLIT is used to
>>> enable header split offload with the header size controlled using
>> "split_hdr_size".
>>> But no single PMD actually supports RTE_ETH_RX_OFFLOAD_HEADER_SPLIT
>> for this purpose.
>>> So we finally decide to deprecate this flag.
>>>
>>> http://patchwork.dpdk.org/project/dpdk/patch/20220402104109.472078-
>> 2-w
>>> enxuanx.wu@intel.com/
>>>
>>> In following series, I use RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT instead. It
>>> is for multi-segments packet split. And it still needs a "proto_hdr" field in
>> rte_eth_rxmode to configure split location.
>>
>> I know this history because I was the one asking you to deprecate this.
>> But it seems you didn't get the big picture.
>>
>>>>> Currently there are 2 acks, add more PMD maintainers to help
>>>>> review this deprecation notice for header split, thanks a lot!
>>>>
>>>> I cannot say my feeling strong enough.
>>>
>>> So IMO the deprecation for header split is not relevant with buffer split. But
>> we can still clean the code.
>>> Hope it make things clearer.
>>
>> They are almost the same features.
>> So when deprecating one, it is important to mention what remains.
>> If needed RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT can still be used and it is
>> configured per-queue, while RTE_ETH_RX_OFFLOAD_HEADER_SPLIT was
>> configurable per-port.
> 
> Thanks for your clarification. It's clearer now.
> I was trying to figure out the whole history of header split,
> seems it is not enough.
> 

Isn't the intention of 'RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT' & 
'RTE_ETH_RX_OFFLOAD_HEADER_SPLIT' are different?
Cc'ed Slava for more comment.

As far as I understand 'BUFFER_SPLIT' has a target to split package 
based on size, to various size mempools, and it can split the payload as 
well without caring the protocol headers.
Most probably practical usage is to split inner protocol, but protocol 
not needed to be known by NIC, it can still split based on size.


Also as far as I can see mlx5 implemented the feature [1], contradicting 
the claim in the commit log, again Slava can comment better. (Although I 
don't see any PMD claim "Buffer Split on Rx" feature supported.)


[1]
Commit 9f209b59c8b0 ("net/mlx5: support Rx buffer split description")

>>
>> Andrew, Ferruh, do you agree to improve this deprecation notice by adding
>> above information?
> 
> Agree. It is better to point out the remaining per queue
> rx offload RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT for splitting packets.
> 
> Please see v2 after I add more header split background.
> 

I can't see a v2 in patchwork, is a v2 sent? If so, may be something 
went wrong.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] doc: announce header split deprecation
  2022-07-14 15:58             ` Ferruh Yigit
@ 2022-07-14 16:03               ` Ferruh Yigit
  2022-07-15  8:52                 ` Ding, Xuan
  2022-07-14 16:56               ` Slava Ovsiienko
  1 sibling, 1 reply; 20+ messages in thread
From: Ferruh Yigit @ 2022-07-14 16:03 UTC (permalink / raw)
  To: Ding, Xuan, Thomas Monjalon, andrew.rybchenko, Viacheslav Ovsiienko
  Cc: mdr, dev, stephen, mb, Zhang, Qi Z, asekhar, pbhagavatula, grive

On 7/14/2022 4:58 PM, Ferruh Yigit wrote:
> On 7/14/2022 3:07 PM, Ding, Xuan wrote:
>> Hi,
>>
>>> -----Original Message-----
>>> From: Thomas Monjalon <thomas@monjalon.net>
>>> Sent: Thursday, July 14, 2022 9:25 PM
>>> To: Ding, Xuan <xuan.ding@intel.com>; andrew.rybchenko@oktetlabs.ru;
>>> ferruh.yigit@xilinx.com
>>> Cc: mdr@ashroe.eu; dev@dpdk.org; stephen@networkplumber.org;
>>> mb@smartsharesystems.com; dev@dpdk.org; Zhang, Qi Z
>>> <qi.z.zhang@intel.com>; asekhar@marvell.com; pbhagavatula@marvell.com;
>>> grive@u256.net
>>> Subject: Re: [PATCH] doc: announce header split deprecation
>>>
>>> 14/07/2022 14:54, Ding, Xuan:
>>>> Hi,
>>>>
>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>>> 14/07/2022 07:50, Ding, Xuan:
>>>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>>>>> 23/05/2022 16:20, xuan.ding@intel.com:
>>>>>>>> From: Xuan Ding <xuan.ding@intel.com>
>>>>>>>>
>>>>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced
>>> some
>>>>> time
>>>>>>> ago
>>>>>>>> to substitute bit-field header_split in struct rte_eth_rxmode.
>>>>>>>> It allows to enable header split offload with the header size
>>>>>>>> controlled using split_hdr_size in the same structure.
>>>>>>>>
>>>>>>>> Right now, no single PMD actually supports
>>>>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT with above definition. Many
>>>>>>>> examples and test apps initialize the field to 0 explicitly.
>>>>>>>> The most of drivers simply ignore split_hdr_size since the
>>>>>>>> offload is not advertised, but
>>>>>>> some double-check that its value is 0.
>>>>>>>>
>>>>>>>> So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and split_header_size
>>>>> field
>>>>>>>> will be removed in DPDK 22.11.
>>>>>>>>
>>>>>>>> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
>>>>>>>> ---
>>>>>>>>   doc/guides/rel_notes/deprecation.rst | 4 ++++
>>>>>>>>   1 file changed, 4 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>>>>>>> b/doc/guides/rel_notes/deprecation.rst
>>>>>>>> index 4e5b23c53d..b8114f29ed 100644
>>>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
>>>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>>>>>>> @@ -125,3 +125,7 @@ Deprecation Notices
>>>>>>>>     applications should be updated to use the ``dmadev`` library
>>> instead,
>>>>>>>>     with the underlying HW-functionality being provided by the 
>>>>>>>> ``ioat``
>>> or
>>>>>>>>     ``idxd`` dma drivers
>>>>>>>> +
>>>>>>>> +* ethdev: After bit-field header split was removed, the
>>>>>>>> +``RTE_ETH_RX_OFFLOAD_HEADER_SPLIT``
>>>>>>>> +offload and the ``split_hdr_size`` field in structure
>>>>>>>> +``rte_eth_rxmode`` to enable header split offload are not
>>>>>>>> +supported in any
>>>>>>> PMDs. They will be removed in DPDK 22.11.
>>>>>>>
>>>>>>> It would have been good to talk about rte_eth_rxseg_split which
>>>>>>> is similar and configured per-queue.
>>>>>>
>>>>>> Thanks for your suggestion.
>>>>>>
>>>>>> But I'm a little confused, are you referring that I need to
>>>>>> involve protocol
>>>>> based buffer split?
>>>>>> About the deprecation of header split, I haven't realized its
>>>>>> connection to
>>>>> rte_eth_rxseg_split.
>>>>>
>>>>> What???
>>>>> In old versions of your patch "ethdev: introduce protocol type based
>>>>> header split"
>>>>> you wrote:
>>>>> "
>>>>> A new proto field is introduced in the rte_eth_rxseg_split structure
>>>>> reserved field to specify header protocol type.
>>>>> With Rx offload flag RTE_ETH_RX_OFFLOAD_HEADER_SPLIT enabled and
>>>>> protocol type configured, PMD will split the ingress packets into
>>>>> two separate regions.
>>>>> "
>>>>
>>>> It has a long history...
>>>> It was corrected in v4 that RTE_ETH_RX_OFFLOAD_HEADER_SPLIT is used to
>>>> enable header split offload with the header size controlled using
>>> "split_hdr_size".
>>>> But no single PMD actually supports RTE_ETH_RX_OFFLOAD_HEADER_SPLIT
>>> for this purpose.
>>>> So we finally decide to deprecate this flag.
>>>>
>>>> http://patchwork.dpdk.org/project/dpdk/patch/20220402104109.472078-
>>> 2-w
>>>> enxuanx.wu@intel.com/
>>>>
>>>> In following series, I use RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT instead. It
>>>> is for multi-segments packet split. And it still needs a "proto_hdr" 
>>>> field in
>>> rte_eth_rxmode to configure split location.
>>>
>>> I know this history because I was the one asking you to deprecate this.
>>> But it seems you didn't get the big picture.
>>>
>>>>>> Currently there are 2 acks, add more PMD maintainers to help
>>>>>> review this deprecation notice for header split, thanks a lot!
>>>>>
>>>>> I cannot say my feeling strong enough.
>>>>
>>>> So IMO the deprecation for header split is not relevant with buffer 
>>>> split. But
>>> we can still clean the code.
>>>> Hope it make things clearer.
>>>
>>> They are almost the same features.
>>> So when deprecating one, it is important to mention what remains.
>>> If needed RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT can still be used and it is
>>> configured per-queue, while RTE_ETH_RX_OFFLOAD_HEADER_SPLIT was
>>> configurable per-port.
>>
>> Thanks for your clarification. It's clearer now.
>> I was trying to figure out the whole history of header split,
>> seems it is not enough.
>>
> 
> Isn't the intention of 'RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT' & 
> 'RTE_ETH_RX_OFFLOAD_HEADER_SPLIT' are different?
> Cc'ed Slava for more comment.
> 
> As far as I understand 'BUFFER_SPLIT' has a target to split package 
> based on size, to various size mempools, and it can split the payload as 
> well without caring the protocol headers.
> Most probably practical usage is to split inner protocol, but protocol 
> not needed to be known by NIC, it can still split based on size.
> 

Ahh, deprecation notice is to deprecate 'HEADER_SPLIT', not other way 
around as I got it wrongly.

I think both may have use case, specially for NICs that parse the 
protocols, header split can be easier to use.
But since there is no user for a long time, perhaps that is not a real 
life need, and we can always add it back when needed, hence:

Acked-by: Ferruh Yigit <ferruh.yigit@xilinx.com>


> 
> Also as far as I can see mlx5 implemented the feature [1], contradicting 
> the claim in the commit log, again Slava can comment better. (Although I 
> don't see any PMD claim "Buffer Split on Rx" feature supported.)
> 
> 
> [1]
> Commit 9f209b59c8b0 ("net/mlx5: support Rx buffer split description")
> 
>>>
>>> Andrew, Ferruh, do you agree to improve this deprecation notice by 
>>> adding
>>> above information?
>>
>> Agree. It is better to point out the remaining per queue
>> rx offload RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT for splitting packets.
>>
>> Please see v2 after I add more header split background.
>>
> 
> I can't see a v2 in patchwork, is a v2 sent? If so, may be something 
> went wrong.
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] doc: announce header split deprecation
  2022-07-14 15:58             ` Ferruh Yigit
  2022-07-14 16:03               ` Ferruh Yigit
@ 2022-07-14 16:56               ` Slava Ovsiienko
  2022-07-15  8:28                 ` Ding, Xuan
  1 sibling, 1 reply; 20+ messages in thread
From: Slava Ovsiienko @ 2022-07-14 16:56 UTC (permalink / raw)
  To: Ferruh Yigit, Ding, Xuan, NBU-Contact-Thomas Monjalon (EXTERNAL),
	andrew.rybchenko
  Cc: mdr, dev, stephen, mb, Zhang, Qi Z, asekhar, pbhagavatula, grive

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@xilinx.com>
> Sent: Thursday, July 14, 2022 18:58
> To: Ding, Xuan <xuan.ding@intel.com>; NBU-Contact-Thomas Monjalon (EXTERNAL)
> <thomas@monjalon.net>; andrew.rybchenko@oktetlabs.ru; Slava Ovsiienko
> <viacheslavo@nvidia.com>
> Cc: mdr@ashroe.eu; dev@dpdk.org; stephen@networkplumber.org;
> mb@smartsharesystems.com; Zhang, Qi Z <qi.z.zhang@intel.com>;
> asekhar@marvell.com; pbhagavatula@marvell.com; grive@u256.net
> Subject: Re: [PATCH] doc: announce header split deprecation
> 
> On 7/14/2022 3:07 PM, Ding, Xuan wrote:
> > Hi,
> >
> >> -----Original Message-----
> >> From: Thomas Monjalon <thomas@monjalon.net>
> >> Sent: Thursday, July 14, 2022 9:25 PM
> >> To: Ding, Xuan <xuan.ding@intel.com>; andrew.rybchenko@oktetlabs.ru;
> >> ferruh.yigit@xilinx.com
> >> Cc: mdr@ashroe.eu; dev@dpdk.org; stephen@networkplumber.org;
> >> mb@smartsharesystems.com; dev@dpdk.org; Zhang, Qi Z
> >> <qi.z.zhang@intel.com>; asekhar@marvell.com;
> >> pbhagavatula@marvell.com; grive@u256.net
> >> Subject: Re: [PATCH] doc: announce header split deprecation
> >>
> >> 14/07/2022 14:54, Ding, Xuan:
> >>> Hi,
> >>>
> >>> From: Thomas Monjalon <thomas@monjalon.net>
> >>>> 14/07/2022 07:50, Ding, Xuan:
> >>>>> From: Thomas Monjalon <thomas@monjalon.net>
> >>>>>> 23/05/2022 16:20, xuan.ding@intel.com:
> >>>>>>> From: Xuan Ding <xuan.ding@intel.com>
> >>>>>>>
> >>>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced
> >> some
> >>>> time
> >>>>>> ago
> >>>>>>> to substitute bit-field header_split in struct rte_eth_rxmode.
> >>>>>>> It allows to enable header split offload with the header size
> >>>>>>> controlled using split_hdr_size in the same structure.
> >>>>>>>
> >>>>>>> Right now, no single PMD actually supports
> >>>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT with above definition. Many
> >>>>>>> examples and test apps initialize the field to 0 explicitly.
> >>>>>>> The most of drivers simply ignore split_hdr_size since the
> >>>>>>> offload is not advertised, but
> >>>>>> some double-check that its value is 0.
> >>>>>>>
> >>>>>>> So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and split_header_size
> >>>> field
> >>>>>>> will be removed in DPDK 22.11.
> >>>>>>>
> >>>>>>> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> >>>>>>> ---
> >>>>>>>   doc/guides/rel_notes/deprecation.rst | 4 ++++
> >>>>>>>   1 file changed, 4 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
> >>>>>>> b/doc/guides/rel_notes/deprecation.rst
> >>>>>>> index 4e5b23c53d..b8114f29ed 100644
> >>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
> >>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
> >>>>>>> @@ -125,3 +125,7 @@ Deprecation Notices
> >>>>>>>     applications should be updated to use the ``dmadev`` library
> >> instead,
> >>>>>>>     with the underlying HW-functionality being provided by the
> >>>>>>> ``ioat``
> >> or
> >>>>>>>     ``idxd`` dma drivers
> >>>>>>> +
> >>>>>>> +* ethdev: After bit-field header split was removed, the
> >>>>>>> +``RTE_ETH_RX_OFFLOAD_HEADER_SPLIT``
> >>>>>>> +offload and the ``split_hdr_size`` field in structure
> >>>>>>> +``rte_eth_rxmode`` to enable header split offload are not
> >>>>>>> +supported in any
> >>>>>> PMDs. They will be removed in DPDK 22.11.
> >>>>>>
> >>>>>> It would have been good to talk about rte_eth_rxseg_split which
> >>>>>> is similar and configured per-queue.
> >>>>>
> >>>>> Thanks for your suggestion.
> >>>>>
> >>>>> But I'm a little confused, are you referring that I need to
> >>>>> involve protocol
> >>>> based buffer split?
> >>>>> About the deprecation of header split, I haven't realized its
> >>>>> connection to
> >>>> rte_eth_rxseg_split.
> >>>>
> >>>> What???
> >>>> In old versions of your patch "ethdev: introduce protocol type
> >>>> based header split"
> >>>> you wrote:
> >>>> "
> >>>> A new proto field is introduced in the rte_eth_rxseg_split
> >>>> structure reserved field to specify header protocol type.
> >>>> With Rx offload flag RTE_ETH_RX_OFFLOAD_HEADER_SPLIT enabled and
> >>>> protocol type configured, PMD will split the ingress packets into
> >>>> two separate regions.
> >>>> "
> >>>
> >>> It has a long history...
> >>> It was corrected in v4 that RTE_ETH_RX_OFFLOAD_HEADER_SPLIT is used
> >>> to enable header split offload with the header size controlled using
> >> "split_hdr_size".
> >>> But no single PMD actually supports RTE_ETH_RX_OFFLOAD_HEADER_SPLIT
> >> for this purpose.
> >>> So we finally decide to deprecate this flag.
> >>>
> >>> http://patchwork.dpdk.org/project/dpdk/patch/20220402104109.472078-
> >> 2-w
> >>> enxuanx.wu@intel.com/
> >>>
> >>> In following series, I use RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT instead.
> >>> It is for multi-segments packet split. And it still needs a
> >>> "proto_hdr" field in
> >> rte_eth_rxmode to configure split location.
> >>
> >> I know this history because I was the one asking you to deprecate this.
> >> But it seems you didn't get the big picture.
> >>
> >>>>> Currently there are 2 acks, add more PMD maintainers to help
> >>>>> review this deprecation notice for header split, thanks a lot!
> >>>>
> >>>> I cannot say my feeling strong enough.
> >>>
> >>> So IMO the deprecation for header split is not relevant with buffer
> >>> split. But
> >> we can still clean the code.
> >>> Hope it make things clearer.
> >>
> >> They are almost the same features.
> >> So when deprecating one, it is important to mention what remains.
> >> If needed RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT can still be used and it is
> >> configured per-queue, while RTE_ETH_RX_OFFLOAD_HEADER_SPLIT was
> >> configurable per-port.
> >
> > Thanks for your clarification. It's clearer now.
> > I was trying to figure out the whole history of header split, seems it
> > is not enough.
> >
> 
> Isn't the intention of 'RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT' &
> 'RTE_ETH_RX_OFFLOAD_HEADER_SPLIT' are different?
> Cc'ed Slava for more comment.

Hi, thank you for Cc'ing

Yes, you are right, we have two splitting offloads, and these ones have
the different intentions. As there are no PMDs actually handling
RTE_ETH_RX_OFFLOAD_HEADER_SPLIT, there should be no objections for this deprecation.

Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>



^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] doc: announce header split deprecation
  2022-07-14 16:56               ` Slava Ovsiienko
@ 2022-07-15  8:28                 ` Ding, Xuan
  2022-07-15 11:34                   ` Ferruh Yigit
  0 siblings, 1 reply; 20+ messages in thread
From: Ding, Xuan @ 2022-07-15  8:28 UTC (permalink / raw)
  To: Slava Ovsiienko, Ferruh Yigit,
	NBU-Contact-Thomas Monjalon (EXTERNAL),
	andrew.rybchenko
  Cc: mdr, dev, stephen, mb, Zhang, Qi Z, asekhar, pbhagavatula, grive

Hi,

> -----Original Message-----
> From: Slava Ovsiienko <viacheslavo@nvidia.com>
> Sent: Friday, July 15, 2022 12:56 AM
> To: Ferruh Yigit <ferruh.yigit@xilinx.com>; Ding, Xuan
> <xuan.ding@intel.com>; NBU-Contact-Thomas Monjalon (EXTERNAL)
> <thomas@monjalon.net>; andrew.rybchenko@oktetlabs.ru
> Cc: mdr@ashroe.eu; dev@dpdk.org; stephen@networkplumber.org;
> mb@smartsharesystems.com; Zhang, Qi Z <qi.z.zhang@intel.com>;
> asekhar@marvell.com; pbhagavatula@marvell.com; grive@u256.net
> Subject: RE: [PATCH] doc: announce header split deprecation
> 
> > -----Original Message-----
> > From: Ferruh Yigit <ferruh.yigit@xilinx.com>
> > Sent: Thursday, July 14, 2022 18:58
> > To: Ding, Xuan <xuan.ding@intel.com>; NBU-Contact-Thomas Monjalon
> > (EXTERNAL) <thomas@monjalon.net>; andrew.rybchenko@oktetlabs.ru;
> Slava
> > Ovsiienko <viacheslavo@nvidia.com>
> > Cc: mdr@ashroe.eu; dev@dpdk.org; stephen@networkplumber.org;
> > mb@smartsharesystems.com; Zhang, Qi Z <qi.z.zhang@intel.com>;
> > asekhar@marvell.com; pbhagavatula@marvell.com; grive@u256.net
> > Subject: Re: [PATCH] doc: announce header split deprecation
> >
> > On 7/14/2022 3:07 PM, Ding, Xuan wrote:
> > > Hi,
> > >
> > >> -----Original Message-----
> > >> From: Thomas Monjalon <thomas@monjalon.net>
> > >> Sent: Thursday, July 14, 2022 9:25 PM
> > >> To: Ding, Xuan <xuan.ding@intel.com>;
> > >> andrew.rybchenko@oktetlabs.ru; ferruh.yigit@xilinx.com
> > >> Cc: mdr@ashroe.eu; dev@dpdk.org; stephen@networkplumber.org;
> > >> mb@smartsharesystems.com; dev@dpdk.org; Zhang, Qi Z
> > >> <qi.z.zhang@intel.com>; asekhar@marvell.com;
> > >> pbhagavatula@marvell.com; grive@u256.net
> > >> Subject: Re: [PATCH] doc: announce header split deprecation
> > >>
> > >> 14/07/2022 14:54, Ding, Xuan:
> > >>> Hi,
> > >>>
> > >>> From: Thomas Monjalon <thomas@monjalon.net>
> > >>>> 14/07/2022 07:50, Ding, Xuan:
> > >>>>> From: Thomas Monjalon <thomas@monjalon.net>
> > >>>>>> 23/05/2022 16:20, xuan.ding@intel.com:
> > >>>>>>> From: Xuan Ding <xuan.ding@intel.com>
> > >>>>>>>
> > >>>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced
> > >> some
> > >>>> time
> > >>>>>> ago
> > >>>>>>> to substitute bit-field header_split in struct rte_eth_rxmode.
> > >>>>>>> It allows to enable header split offload with the header size
> > >>>>>>> controlled using split_hdr_size in the same structure.
> > >>>>>>>
> > >>>>>>> Right now, no single PMD actually supports
> > >>>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT with above definition.
> Many
> > >>>>>>> examples and test apps initialize the field to 0 explicitly.
> > >>>>>>> The most of drivers simply ignore split_hdr_size since the
> > >>>>>>> offload is not advertised, but
> > >>>>>> some double-check that its value is 0.
> > >>>>>>>
> > >>>>>>> So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and
> split_header_size
> > >>>> field
> > >>>>>>> will be removed in DPDK 22.11.
> > >>>>>>>
> > >>>>>>> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> > >>>>>>> ---
> > >>>>>>>   doc/guides/rel_notes/deprecation.rst | 4 ++++
> > >>>>>>>   1 file changed, 4 insertions(+)
> > >>>>>>>
> > >>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
> > >>>>>>> b/doc/guides/rel_notes/deprecation.rst
> > >>>>>>> index 4e5b23c53d..b8114f29ed 100644
> > >>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
> > >>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
> > >>>>>>> @@ -125,3 +125,7 @@ Deprecation Notices
> > >>>>>>>     applications should be updated to use the ``dmadev``
> > >>>>>>> library
> > >> instead,
> > >>>>>>>     with the underlying HW-functionality being provided by the
> > >>>>>>> ``ioat``
> > >> or
> > >>>>>>>     ``idxd`` dma drivers
> > >>>>>>> +
> > >>>>>>> +* ethdev: After bit-field header split was removed, the
> > >>>>>>> +``RTE_ETH_RX_OFFLOAD_HEADER_SPLIT``
> > >>>>>>> +offload and the ``split_hdr_size`` field in structure
> > >>>>>>> +``rte_eth_rxmode`` to enable header split offload are not
> > >>>>>>> +supported in any
> > >>>>>> PMDs. They will be removed in DPDK 22.11.
> > >>>>>>
> > >>>>>> It would have been good to talk about rte_eth_rxseg_split which
> > >>>>>> is similar and configured per-queue.
> > >>>>>
> > >>>>> Thanks for your suggestion.
> > >>>>>
> > >>>>> But I'm a little confused, are you referring that I need to
> > >>>>> involve protocol
> > >>>> based buffer split?
> > >>>>> About the deprecation of header split, I haven't realized its
> > >>>>> connection to
> > >>>> rte_eth_rxseg_split.
> > >>>>
> > >>>> What???
> > >>>> In old versions of your patch "ethdev: introduce protocol type
> > >>>> based header split"
> > >>>> you wrote:
> > >>>> "
> > >>>> A new proto field is introduced in the rte_eth_rxseg_split
> > >>>> structure reserved field to specify header protocol type.
> > >>>> With Rx offload flag RTE_ETH_RX_OFFLOAD_HEADER_SPLIT enabled
> and
> > >>>> protocol type configured, PMD will split the ingress packets into
> > >>>> two separate regions.
> > >>>> "
> > >>>
> > >>> It has a long history...
> > >>> It was corrected in v4 that RTE_ETH_RX_OFFLOAD_HEADER_SPLIT is
> > >>> used to enable header split offload with the header size
> > >>> controlled using
> > >> "split_hdr_size".
> > >>> But no single PMD actually supports
> > >>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT
> > >> for this purpose.
> > >>> So we finally decide to deprecate this flag.
> > >>>
> > >>>
> http://patchwork.dpdk.org/project/dpdk/patch/20220402104109.472078
> > >>> -
> > >> 2-w
> > >>> enxuanx.wu@intel.com/
> > >>>
> > >>> In following series, I use RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT instead.
> > >>> It is for multi-segments packet split. And it still needs a
> > >>> "proto_hdr" field in
> > >> rte_eth_rxmode to configure split location.
> > >>
> > >> I know this history because I was the one asking you to deprecate this.
> > >> But it seems you didn't get the big picture.
> > >>
> > >>>>> Currently there are 2 acks, add more PMD maintainers to help
> > >>>>> review this deprecation notice for header split, thanks a lot!
> > >>>>
> > >>>> I cannot say my feeling strong enough.
> > >>>
> > >>> So IMO the deprecation for header split is not relevant with
> > >>> buffer split. But
> > >> we can still clean the code.
> > >>> Hope it make things clearer.
> > >>
> > >> They are almost the same features.
> > >> So when deprecating one, it is important to mention what remains.
> > >> If needed RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT can still be used and it
> > >> is configured per-queue, while RTE_ETH_RX_OFFLOAD_HEADER_SPLIT
> was
> > >> configurable per-port.
> > >
> > > Thanks for your clarification. It's clearer now.
> > > I was trying to figure out the whole history of header split, seems
> > > it is not enough.
> > >
> >
> > Isn't the intention of 'RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT' &
> > 'RTE_ETH_RX_OFFLOAD_HEADER_SPLIT' are different?
> > Cc'ed Slava for more comment.
> 
> Hi, thank you for Cc'ing
> 
> Yes, you are right, we have two splitting offloads, and these ones have the
> different intentions. As there are no PMDs actually handling
> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT, there should be no objections for
> this deprecation.

Thanks for helping review.

For 'BUFFER_SPLIT', I think it is clear. As Ferruh explains, it is used for splitting packets into multi-segments and multi-mempools based on rte_eth_rxseg_split.
Right now only mlx5 supports this offload.

For 'HEADER_SPLIT', IMO it is used for splitting packets into two segments based on the split_hdr_size in rte_eth_rxmode.
And header split does not support split into multi-mempools(the same mempool).

So at this level, we can say that header split and buffer split are the same intention (split packets).
The functions supported by header split have been covered by buffer split. And no PMD actually supports 'HEADER_SPLIT'.

Please corrects me if my understanding is wrong.

Thanks,
Xuan


> 
> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] doc: announce header split deprecation
  2022-07-14 16:03               ` Ferruh Yigit
@ 2022-07-15  8:52                 ` Ding, Xuan
  0 siblings, 0 replies; 20+ messages in thread
From: Ding, Xuan @ 2022-07-15  8:52 UTC (permalink / raw)
  To: Ferruh Yigit, Thomas Monjalon, andrew.rybchenko, Viacheslav Ovsiienko
  Cc: mdr, dev, stephen, mb, Zhang, Qi Z, asekhar, pbhagavatula, grive

Hi Ferruh,

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@xilinx.com>
> Sent: Friday, July 15, 2022 12:03 AM
> To: Ding, Xuan <xuan.ding@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>; andrew.rybchenko@oktetlabs.ru; Viacheslav
> Ovsiienko <viacheslavo@nvidia.com>
> Cc: mdr@ashroe.eu; dev@dpdk.org; stephen@networkplumber.org;
> mb@smartsharesystems.com; Zhang, Qi Z <qi.z.zhang@intel.com>;
> asekhar@marvell.com; pbhagavatula@marvell.com; grive@u256.net
> Subject: Re: [PATCH] doc: announce header split deprecation
> 
> On 7/14/2022 4:58 PM, Ferruh Yigit wrote:
> > On 7/14/2022 3:07 PM, Ding, Xuan wrote:
> >> Hi,
> >>
> >>> -----Original Message-----
> >>> From: Thomas Monjalon <thomas@monjalon.net>
> >>> Sent: Thursday, July 14, 2022 9:25 PM
> >>> To: Ding, Xuan <xuan.ding@intel.com>; andrew.rybchenko@oktetlabs.ru;
> >>> ferruh.yigit@xilinx.com
> >>> Cc: mdr@ashroe.eu; dev@dpdk.org; stephen@networkplumber.org;
> >>> mb@smartsharesystems.com; dev@dpdk.org; Zhang, Qi Z
> >>> <qi.z.zhang@intel.com>; asekhar@marvell.com;
> >>> pbhagavatula@marvell.com; grive@u256.net
> >>> Subject: Re: [PATCH] doc: announce header split deprecation
> >>>
> >>> 14/07/2022 14:54, Ding, Xuan:
> >>>> Hi,
> >>>>
> >>>> From: Thomas Monjalon <thomas@monjalon.net>
> >>>>> 14/07/2022 07:50, Ding, Xuan:
> >>>>>> From: Thomas Monjalon <thomas@monjalon.net>
> >>>>>>> 23/05/2022 16:20, xuan.ding@intel.com:
> >>>>>>>> From: Xuan Ding <xuan.ding@intel.com>
> >>>>>>>>
> >>>>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced
> >>> some
> >>>>> time
> >>>>>>> ago
> >>>>>>>> to substitute bit-field header_split in struct rte_eth_rxmode.
> >>>>>>>> It allows to enable header split offload with the header size
> >>>>>>>> controlled using split_hdr_size in the same structure.
> >>>>>>>>
> >>>>>>>> Right now, no single PMD actually supports
> >>>>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT with above definition. Many
> >>>>>>>> examples and test apps initialize the field to 0 explicitly.
> >>>>>>>> The most of drivers simply ignore split_hdr_size since the
> >>>>>>>> offload is not advertised, but
> >>>>>>> some double-check that its value is 0.
> >>>>>>>>
> >>>>>>>> So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and split_header_size
> >>>>> field
> >>>>>>>> will be removed in DPDK 22.11.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> >>>>>>>> ---
> >>>>>>>>   doc/guides/rel_notes/deprecation.rst | 4 ++++
> >>>>>>>>   1 file changed, 4 insertions(+)
> >>>>>>>>
> >>>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
> >>>>>>>> b/doc/guides/rel_notes/deprecation.rst
> >>>>>>>> index 4e5b23c53d..b8114f29ed 100644
> >>>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
> >>>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
> >>>>>>>> @@ -125,3 +125,7 @@ Deprecation Notices
> >>>>>>>>     applications should be updated to use the ``dmadev``
> >>>>>>>> library
> >>> instead,
> >>>>>>>>     with the underlying HW-functionality being provided by the
> >>>>>>>> ``ioat``
> >>> or
> >>>>>>>>     ``idxd`` dma drivers
> >>>>>>>> +
> >>>>>>>> +* ethdev: After bit-field header split was removed, the
> >>>>>>>> +``RTE_ETH_RX_OFFLOAD_HEADER_SPLIT``
> >>>>>>>> +offload and the ``split_hdr_size`` field in structure
> >>>>>>>> +``rte_eth_rxmode`` to enable header split offload are not
> >>>>>>>> +supported in any
> >>>>>>> PMDs. They will be removed in DPDK 22.11.
> >>>>>>>
> >>>>>>> It would have been good to talk about rte_eth_rxseg_split which
> >>>>>>> is similar and configured per-queue.
> >>>>>>
> >>>>>> Thanks for your suggestion.
> >>>>>>
> >>>>>> But I'm a little confused, are you referring that I need to
> >>>>>> involve protocol
> >>>>> based buffer split?
> >>>>>> About the deprecation of header split, I haven't realized its
> >>>>>> connection to
> >>>>> rte_eth_rxseg_split.
> >>>>>
> >>>>> What???
> >>>>> In old versions of your patch "ethdev: introduce protocol type
> >>>>> based header split"
> >>>>> you wrote:
> >>>>> "
> >>>>> A new proto field is introduced in the rte_eth_rxseg_split
> >>>>> structure reserved field to specify header protocol type.
> >>>>> With Rx offload flag RTE_ETH_RX_OFFLOAD_HEADER_SPLIT enabled and
> >>>>> protocol type configured, PMD will split the ingress packets into
> >>>>> two separate regions.
> >>>>> "
> >>>>
> >>>> It has a long history...
> >>>> It was corrected in v4 that RTE_ETH_RX_OFFLOAD_HEADER_SPLIT is used
> >>>> to enable header split offload with the header size controlled
> >>>> using
> >>> "split_hdr_size".
> >>>> But no single PMD actually supports
> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT
> >>> for this purpose.
> >>>> So we finally decide to deprecate this flag.
> >>>>
> >>>> http://patchwork.dpdk.org/project/dpdk/patch/20220402104109.472078-
> >>> 2-w
> >>>> enxuanx.wu@intel.com/
> >>>>
> >>>> In following series, I use RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT instead.
> >>>> It is for multi-segments packet split. And it still needs a "proto_hdr"
> >>>> field in
> >>> rte_eth_rxmode to configure split location.
> >>>
> >>> I know this history because I was the one asking you to deprecate this.
> >>> But it seems you didn't get the big picture.
> >>>
> >>>>>> Currently there are 2 acks, add more PMD maintainers to help
> >>>>>> review this deprecation notice for header split, thanks a lot!
> >>>>>
> >>>>> I cannot say my feeling strong enough.
> >>>>
> >>>> So IMO the deprecation for header split is not relevant with buffer
> >>>> split. But
> >>> we can still clean the code.
> >>>> Hope it make things clearer.
> >>>
> >>> They are almost the same features.
> >>> So when deprecating one, it is important to mention what remains.
> >>> If needed RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT can still be used and it
> >>> is configured per-queue, while RTE_ETH_RX_OFFLOAD_HEADER_SPLIT was
> >>> configurable per-port.
> >>
> >> Thanks for your clarification. It's clearer now.
> >> I was trying to figure out the whole history of header split, seems
> >> it is not enough.
> >>
> >
> > Isn't the intention of 'RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT' &
> > 'RTE_ETH_RX_OFFLOAD_HEADER_SPLIT' are different?
> > Cc'ed Slava for more comment.
> >
> > As far as I understand 'BUFFER_SPLIT' has a target to split package
> > based on size, to various size mempools, and it can split the payload
> > as well without caring the protocol headers.
> > Most probably practical usage is to split inner protocol, but protocol
> > not needed to be known by NIC, it can still split based on size.
> >
> 
> Ahh, deprecation notice is to deprecate 'HEADER_SPLIT', not other way around
> as I got it wrongly.
> 
> I think both may have use case, specially for NICs that parse the protocols,
> header split can be easier to use.

Thanks for your comments.

Does it supports to split packets into different mempools?
Actually in DPDK22.07, we send another series to extend current buffer split to support
split by protocol header.

http://patchwork.dpdk.org/project/dpdk/cover/20220613102550.241759-1-wenxuanx.wu@intel.com/

Since no PMD supports 'HEADER_SPLIT', I cannot find a reference.
IMO the 'HEADER SPLIT' is not used for splitting by protocol headers, it still requires 'split_hdr_size'.

> But since there is no user for a long time, perhaps that is not a real life need, and
> we can always add it back when needed, hence:
> 
> Acked-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
> 
> 
> >
> > Also as far as I can see mlx5 implemented the feature [1],
> > contradicting the claim in the commit log, again Slava can comment
> > better. (Although I don't see any PMD claim "Buffer Split on Rx"
> > feature supported.)

Yes, you are right.

> >
> >
> > [1]
> > Commit 9f209b59c8b0 ("net/mlx5: support Rx buffer split description")
> >
> >>>
> >>> Andrew, Ferruh, do you agree to improve this deprecation notice by
> >>> adding above information?
> >>
> >> Agree. It is better to point out the remaining per queue rx offload
> >> RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT for splitting packets.
> >>
> >> Please see v2 after I add more header split background.
> >>
> >
> > I can't see a v2 in patchwork, is a v2 sent? If so, may be something
> > went wrong.

Sorry, the v2 has not been sent yet since there are doubts about the relationship header split
and buffer split.

Please see last email. Hope to get your insights.

Regards,
Xuan

> >


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] doc: announce header split deprecation
  2022-07-15  8:28                 ` Ding, Xuan
@ 2022-07-15 11:34                   ` Ferruh Yigit
  2022-07-15 12:43                     ` Ding, Xuan
  0 siblings, 1 reply; 20+ messages in thread
From: Ferruh Yigit @ 2022-07-15 11:34 UTC (permalink / raw)
  To: Ding, Xuan, Slava Ovsiienko,
	NBU-Contact-Thomas Monjalon (EXTERNAL),
	andrew.rybchenko
  Cc: mdr, dev, stephen, mb, Zhang, Qi Z, asekhar, pbhagavatula, grive

On 7/15/2022 9:28 AM, Ding, Xuan wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Slava Ovsiienko <viacheslavo@nvidia.com>
>> Sent: Friday, July 15, 2022 12:56 AM
>> To: Ferruh Yigit <ferruh.yigit@xilinx.com>; Ding, Xuan
>> <xuan.ding@intel.com>; NBU-Contact-Thomas Monjalon (EXTERNAL)
>> <thomas@monjalon.net>; andrew.rybchenko@oktetlabs.ru
>> Cc: mdr@ashroe.eu; dev@dpdk.org; stephen@networkplumber.org;
>> mb@smartsharesystems.com; Zhang, Qi Z <qi.z.zhang@intel.com>;
>> asekhar@marvell.com; pbhagavatula@marvell.com; grive@u256.net
>> Subject: RE: [PATCH] doc: announce header split deprecation
>>
>>> -----Original Message-----
>>> From: Ferruh Yigit <ferruh.yigit@xilinx.com>
>>> Sent: Thursday, July 14, 2022 18:58
>>> To: Ding, Xuan <xuan.ding@intel.com>; NBU-Contact-Thomas Monjalon
>>> (EXTERNAL) <thomas@monjalon.net>; andrew.rybchenko@oktetlabs.ru;
>> Slava
>>> Ovsiienko <viacheslavo@nvidia.com>
>>> Cc: mdr@ashroe.eu; dev@dpdk.org; stephen@networkplumber.org;
>>> mb@smartsharesystems.com; Zhang, Qi Z <qi.z.zhang@intel.com>;
>>> asekhar@marvell.com; pbhagavatula@marvell.com; grive@u256.net
>>> Subject: Re: [PATCH] doc: announce header split deprecation
>>>
>>> On 7/14/2022 3:07 PM, Ding, Xuan wrote:
>>>> Hi,
>>>>
>>>>> -----Original Message-----
>>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>>> Sent: Thursday, July 14, 2022 9:25 PM
>>>>> To: Ding, Xuan <xuan.ding@intel.com>;
>>>>> andrew.rybchenko@oktetlabs.ru; ferruh.yigit@xilinx.com
>>>>> Cc: mdr@ashroe.eu; dev@dpdk.org; stephen@networkplumber.org;
>>>>> mb@smartsharesystems.com; dev@dpdk.org; Zhang, Qi Z
>>>>> <qi.z.zhang@intel.com>; asekhar@marvell.com;
>>>>> pbhagavatula@marvell.com; grive@u256.net
>>>>> Subject: Re: [PATCH] doc: announce header split deprecation
>>>>>
>>>>> 14/07/2022 14:54, Ding, Xuan:
>>>>>> Hi,
>>>>>>
>>>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>>>>> 14/07/2022 07:50, Ding, Xuan:
>>>>>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>>>>>>> 23/05/2022 16:20, xuan.ding@intel.com:
>>>>>>>>>> From: Xuan Ding <xuan.ding@intel.com>
>>>>>>>>>>
>>>>>>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced
>>>>> some
>>>>>>> time
>>>>>>>>> ago
>>>>>>>>>> to substitute bit-field header_split in struct rte_eth_rxmode.
>>>>>>>>>> It allows to enable header split offload with the header size
>>>>>>>>>> controlled using split_hdr_size in the same structure.
>>>>>>>>>>
>>>>>>>>>> Right now, no single PMD actually supports
>>>>>>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT with above definition.
>> Many
>>>>>>>>>> examples and test apps initialize the field to 0 explicitly.
>>>>>>>>>> The most of drivers simply ignore split_hdr_size since the
>>>>>>>>>> offload is not advertised, but
>>>>>>>>> some double-check that its value is 0.
>>>>>>>>>>
>>>>>>>>>> So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and
>> split_header_size
>>>>>>> field
>>>>>>>>>> will be removed in DPDK 22.11.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
>>>>>>>>>> ---
>>>>>>>>>>    doc/guides/rel_notes/deprecation.rst | 4 ++++
>>>>>>>>>>    1 file changed, 4 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>>>>>>>>> b/doc/guides/rel_notes/deprecation.rst
>>>>>>>>>> index 4e5b23c53d..b8114f29ed 100644
>>>>>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
>>>>>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>>>>>>>>> @@ -125,3 +125,7 @@ Deprecation Notices
>>>>>>>>>>      applications should be updated to use the ``dmadev``
>>>>>>>>>> library
>>>>> instead,
>>>>>>>>>>      with the underlying HW-functionality being provided by the
>>>>>>>>>> ``ioat``
>>>>> or
>>>>>>>>>>      ``idxd`` dma drivers
>>>>>>>>>> +
>>>>>>>>>> +* ethdev: After bit-field header split was removed, the
>>>>>>>>>> +``RTE_ETH_RX_OFFLOAD_HEADER_SPLIT``
>>>>>>>>>> +offload and the ``split_hdr_size`` field in structure
>>>>>>>>>> +``rte_eth_rxmode`` to enable header split offload are not
>>>>>>>>>> +supported in any
>>>>>>>>> PMDs. They will be removed in DPDK 22.11.
>>>>>>>>>
>>>>>>>>> It would have been good to talk about rte_eth_rxseg_split which
>>>>>>>>> is similar and configured per-queue.
>>>>>>>>
>>>>>>>> Thanks for your suggestion.
>>>>>>>>
>>>>>>>> But I'm a little confused, are you referring that I need to
>>>>>>>> involve protocol
>>>>>>> based buffer split?
>>>>>>>> About the deprecation of header split, I haven't realized its
>>>>>>>> connection to
>>>>>>> rte_eth_rxseg_split.
>>>>>>>
>>>>>>> What???
>>>>>>> In old versions of your patch "ethdev: introduce protocol type
>>>>>>> based header split"
>>>>>>> you wrote:
>>>>>>> "
>>>>>>> A new proto field is introduced in the rte_eth_rxseg_split
>>>>>>> structure reserved field to specify header protocol type.
>>>>>>> With Rx offload flag RTE_ETH_RX_OFFLOAD_HEADER_SPLIT enabled
>> and
>>>>>>> protocol type configured, PMD will split the ingress packets into
>>>>>>> two separate regions.
>>>>>>> "
>>>>>>
>>>>>> It has a long history...
>>>>>> It was corrected in v4 that RTE_ETH_RX_OFFLOAD_HEADER_SPLIT is
>>>>>> used to enable header split offload with the header size
>>>>>> controlled using
>>>>> "split_hdr_size".
>>>>>> But no single PMD actually supports
>>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT
>>>>> for this purpose.
>>>>>> So we finally decide to deprecate this flag.
>>>>>>
>>>>>>
>> http://patchwork.dpdk.org/project/dpdk/patch/20220402104109.472078
>>>>>> -
>>>>> 2-w
>>>>>> enxuanx.wu@intel.com/
>>>>>>
>>>>>> In following series, I use RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT instead.
>>>>>> It is for multi-segments packet split. And it still needs a
>>>>>> "proto_hdr" field in
>>>>> rte_eth_rxmode to configure split location.
>>>>>
>>>>> I know this history because I was the one asking you to deprecate this.
>>>>> But it seems you didn't get the big picture.
>>>>>
>>>>>>>> Currently there are 2 acks, add more PMD maintainers to help
>>>>>>>> review this deprecation notice for header split, thanks a lot!
>>>>>>>
>>>>>>> I cannot say my feeling strong enough.
>>>>>>
>>>>>> So IMO the deprecation for header split is not relevant with
>>>>>> buffer split. But
>>>>> we can still clean the code.
>>>>>> Hope it make things clearer.
>>>>>
>>>>> They are almost the same features.
>>>>> So when deprecating one, it is important to mention what remains.
>>>>> If needed RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT can still be used and it
>>>>> is configured per-queue, while RTE_ETH_RX_OFFLOAD_HEADER_SPLIT
>> was
>>>>> configurable per-port.
>>>>
>>>> Thanks for your clarification. It's clearer now.
>>>> I was trying to figure out the whole history of header split, seems
>>>> it is not enough.
>>>>
>>>
>>> Isn't the intention of 'RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT' &
>>> 'RTE_ETH_RX_OFFLOAD_HEADER_SPLIT' are different?
>>> Cc'ed Slava for more comment.
>>
>> Hi, thank you for Cc'ing
>>
>> Yes, you are right, we have two splitting offloads, and these ones have the
>> different intentions. As there are no PMDs actually handling
>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT, there should be no objections for
>> this deprecation.
> 
> Thanks for helping review.
> 
> For 'BUFFER_SPLIT', I think it is clear. As Ferruh explains, it is used for splitting packets into multi-segments and multi-mempools based on rte_eth_rxseg_split.
> Right now only mlx5 supports this offload.
> 
> For 'HEADER_SPLIT', IMO it is used for splitting packets into two segments based on the split_hdr_size in rte_eth_rxmode.
> And header split does not support split into multi-mempools(the same mempool).
> 

I looks like we don't have much details on the intention of the 
'HEADER_SPLIT', it is not well documented.

> So at this level, we can say that header split and buffer split are the same intention (split packets).
> The functions supported by header split have been covered by buffer split. And no PMD actually supports 'HEADER_SPLIT'.
> 
> Please corrects me if my understanding is wrong.
> 
> Thanks,
> Xuan
> 
> 
>>
>> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
>>
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] doc: announce header split deprecation
  2022-07-14 13:25         ` Thomas Monjalon
  2022-07-14 14:07           ` Ding, Xuan
@ 2022-07-15 12:13           ` Andrew Rybchenko
  2022-07-15 12:44             ` Ding, Xuan
  1 sibling, 1 reply; 20+ messages in thread
From: Andrew Rybchenko @ 2022-07-15 12:13 UTC (permalink / raw)
  To: Thomas Monjalon, Ding, Xuan, ferruh.yigit
  Cc: mdr, dev, stephen, mb, Zhang, Qi Z, asekhar, pbhagavatula, grive

On 7/14/22 16:25, Thomas Monjalon wrote:
> 14/07/2022 14:54, Ding, Xuan:
>> Hi,
>>
>> From: Thomas Monjalon <thomas@monjalon.net>
>>> 14/07/2022 07:50, Ding, Xuan:
>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>>> 23/05/2022 16:20, xuan.ding@intel.com:
>>>>>> From: Xuan Ding <xuan.ding@intel.com>
>>>>>>
>>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced some
>>> time
>>>>> ago
>>>>>> to substitute bit-field header_split in struct rte_eth_rxmode. It
>>>>>> allows to enable header split offload with the header size
>>>>>> controlled using split_hdr_size in the same structure.
>>>>>>
>>>>>> Right now, no single PMD actually supports
>>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT with above definition. Many
>>>>>> examples and test apps initialize the field to 0 explicitly. The
>>>>>> most of drivers simply ignore split_hdr_size since the offload is
>>>>>> not advertised, but
>>>>> some double-check that its value is 0.
>>>>>>
>>>>>> So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and split_header_size
>>> field
>>>>>> will be removed in DPDK 22.11.
>>>>>>
>>>>>> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
>>>>>> ---
>>>>>>   doc/guides/rel_notes/deprecation.rst | 4 ++++
>>>>>>   1 file changed, 4 insertions(+)
>>>>>>
>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>>>>> b/doc/guides/rel_notes/deprecation.rst
>>>>>> index 4e5b23c53d..b8114f29ed 100644
>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>>>>> @@ -125,3 +125,7 @@ Deprecation Notices
>>>>>>     applications should be updated to use the ``dmadev`` library instead,
>>>>>>     with the underlying HW-functionality being provided by the ``ioat`` or
>>>>>>     ``idxd`` dma drivers
>>>>>> +
>>>>>> +* ethdev: After bit-field header split was removed, the
>>>>>> +``RTE_ETH_RX_OFFLOAD_HEADER_SPLIT``
>>>>>> +offload and the ``split_hdr_size`` field in structure
>>>>>> +``rte_eth_rxmode`` to enable header split offload are not
>>>>>> +supported in any
>>>>> PMDs. They will be removed in DPDK 22.11.
>>>>>
>>>>> It would have been good to talk about rte_eth_rxseg_split which is
>>>>> similar and configured per-queue.
>>>>
>>>> Thanks for your suggestion.
>>>>
>>>> But I'm a little confused, are you referring that I need to involve protocol
>>> based buffer split?
>>>> About the deprecation of header split, I haven't realized its connection to
>>> rte_eth_rxseg_split.
>>>
>>> What???
>>> In old versions of your patch "ethdev: introduce protocol type based header
>>> split"
>>> you wrote:
>>> "
>>> A new proto field is introduced in the
>>> rte_eth_rxseg_split structure reserved field to specify header protocol type.
>>> With Rx offload flag RTE_ETH_RX_OFFLOAD_HEADER_SPLIT enabled and
>>> protocol type configured, PMD will split the ingress packets into two separate
>>> regions.
>>> "
>>
>> It has a long history...
>> It was corrected in v4 that RTE_ETH_RX_OFFLOAD_HEADER_SPLIT is used to enable header
>> split offload with the header size controlled using "split_hdr_size".
>> But no single PMD actually supports RTE_ETH_RX_OFFLOAD_HEADER_SPLIT for this purpose.
>> So we finally decide to deprecate this flag.
>>
>> http://patchwork.dpdk.org/project/dpdk/patch/20220402104109.472078-2-wenxuanx.wu@intel.com/
>>
>> In following series, I use RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT instead. It is for multi-segments packet
>> split. And it still needs a "proto_hdr" field in rte_eth_rxmode to configure split location.
> 
> I know this history because I was the one asking you to deprecate this.
> But it seems you didn't get the big picture.
> 
>>>> Currently there are 2 acks, add more PMD maintainers to help review
>>>> this deprecation notice for header split, thanks a lot!
>>>
>>> I cannot say my feeling strong enough.
>>
>> So IMO the deprecation for header split is not relevant with buffer split. But we can still clean the code.
>> Hope it make things clearer.
> 
> They are almost the same features.
> So when deprecating one, it is important to mention what remains.
> If needed RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT can still be used
> and it is configured per-queue,
> while RTE_ETH_RX_OFFLOAD_HEADER_SPLIT was configurable per-port.
> 
> Andrew, Ferruh, do you agree to improve this deprecation notice
> by adding above information?

+1 tt is definitely a very good idea.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] doc: announce header split deprecation
  2022-07-15 11:34                   ` Ferruh Yigit
@ 2022-07-15 12:43                     ` Ding, Xuan
  0 siblings, 0 replies; 20+ messages in thread
From: Ding, Xuan @ 2022-07-15 12:43 UTC (permalink / raw)
  To: Ferruh Yigit, Slava Ovsiienko,
	NBU-Contact-Thomas Monjalon (EXTERNAL),
	andrew.rybchenko
  Cc: mdr, dev, stephen, mb, Zhang, Qi Z, asekhar, pbhagavatula, grive



> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@xilinx.com>
> Sent: Friday, July 15, 2022 7:34 PM
> To: Ding, Xuan <xuan.ding@intel.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>; NBU-Contact-Thomas Monjalon (EXTERNAL)
> <thomas@monjalon.net>; andrew.rybchenko@oktetlabs.ru
> Cc: mdr@ashroe.eu; dev@dpdk.org; stephen@networkplumber.org;
> mb@smartsharesystems.com; Zhang, Qi Z <qi.z.zhang@intel.com>;
> asekhar@marvell.com; pbhagavatula@marvell.com; grive@u256.net
> Subject: Re: [PATCH] doc: announce header split deprecation
> 
> On 7/15/2022 9:28 AM, Ding, Xuan wrote:
> > Hi,
> >
> >> -----Original Message-----
> >> From: Slava Ovsiienko <viacheslavo@nvidia.com>
> >> Sent: Friday, July 15, 2022 12:56 AM
> >> To: Ferruh Yigit <ferruh.yigit@xilinx.com>; Ding, Xuan
> >> <xuan.ding@intel.com>; NBU-Contact-Thomas Monjalon (EXTERNAL)
> >> <thomas@monjalon.net>; andrew.rybchenko@oktetlabs.ru
> >> Cc: mdr@ashroe.eu; dev@dpdk.org; stephen@networkplumber.org;
> >> mb@smartsharesystems.com; Zhang, Qi Z <qi.z.zhang@intel.com>;
> >> asekhar@marvell.com; pbhagavatula@marvell.com; grive@u256.net
> >> Subject: RE: [PATCH] doc: announce header split deprecation
> >>
> >>> -----Original Message-----
> >>> From: Ferruh Yigit <ferruh.yigit@xilinx.com>
> >>> Sent: Thursday, July 14, 2022 18:58
> >>> To: Ding, Xuan <xuan.ding@intel.com>; NBU-Contact-Thomas Monjalon
> >>> (EXTERNAL) <thomas@monjalon.net>; andrew.rybchenko@oktetlabs.ru;
> >> Slava
> >>> Ovsiienko <viacheslavo@nvidia.com>
> >>> Cc: mdr@ashroe.eu; dev@dpdk.org; stephen@networkplumber.org;
> >>> mb@smartsharesystems.com; Zhang, Qi Z <qi.z.zhang@intel.com>;
> >>> asekhar@marvell.com; pbhagavatula@marvell.com; grive@u256.net
> >>> Subject: Re: [PATCH] doc: announce header split deprecation
> >>>
> >>> On 7/14/2022 3:07 PM, Ding, Xuan wrote:
> >>>> Hi,
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Thomas Monjalon <thomas@monjalon.net>
> >>>>> Sent: Thursday, July 14, 2022 9:25 PM
> >>>>> To: Ding, Xuan <xuan.ding@intel.com>;
> >>>>> andrew.rybchenko@oktetlabs.ru; ferruh.yigit@xilinx.com
> >>>>> Cc: mdr@ashroe.eu; dev@dpdk.org; stephen@networkplumber.org;
> >>>>> mb@smartsharesystems.com; dev@dpdk.org; Zhang, Qi Z
> >>>>> <qi.z.zhang@intel.com>; asekhar@marvell.com;
> >>>>> pbhagavatula@marvell.com; grive@u256.net
> >>>>> Subject: Re: [PATCH] doc: announce header split deprecation
> >>>>>
> >>>>> 14/07/2022 14:54, Ding, Xuan:
> >>>>>> Hi,
> >>>>>>
> >>>>>> From: Thomas Monjalon <thomas@monjalon.net>
> >>>>>>> 14/07/2022 07:50, Ding, Xuan:
> >>>>>>>> From: Thomas Monjalon <thomas@monjalon.net>
> >>>>>>>>> 23/05/2022 16:20, xuan.ding@intel.com:
> >>>>>>>>>> From: Xuan Ding <xuan.ding@intel.com>
> >>>>>>>>>>
> >>>>>>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced
> >>>>> some
> >>>>>>> time
> >>>>>>>>> ago
> >>>>>>>>>> to substitute bit-field header_split in struct rte_eth_rxmode.
> >>>>>>>>>> It allows to enable header split offload with the header size
> >>>>>>>>>> controlled using split_hdr_size in the same structure.
> >>>>>>>>>>
> >>>>>>>>>> Right now, no single PMD actually supports
> >>>>>>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT with above definition.
> >> Many
> >>>>>>>>>> examples and test apps initialize the field to 0 explicitly.
> >>>>>>>>>> The most of drivers simply ignore split_hdr_size since the
> >>>>>>>>>> offload is not advertised, but
> >>>>>>>>> some double-check that its value is 0.
> >>>>>>>>>>
> >>>>>>>>>> So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and
> >> split_header_size
> >>>>>>> field
> >>>>>>>>>> will be removed in DPDK 22.11.
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> >>>>>>>>>> ---
> >>>>>>>>>>    doc/guides/rel_notes/deprecation.rst | 4 ++++
> >>>>>>>>>>    1 file changed, 4 insertions(+)
> >>>>>>>>>>
> >>>>>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
> >>>>>>>>>> b/doc/guides/rel_notes/deprecation.rst
> >>>>>>>>>> index 4e5b23c53d..b8114f29ed 100644
> >>>>>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
> >>>>>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
> >>>>>>>>>> @@ -125,3 +125,7 @@ Deprecation Notices
> >>>>>>>>>>      applications should be updated to use the ``dmadev``
> >>>>>>>>>> library
> >>>>> instead,
> >>>>>>>>>>      with the underlying HW-functionality being provided by
> >>>>>>>>>> the ``ioat``
> >>>>> or
> >>>>>>>>>>      ``idxd`` dma drivers
> >>>>>>>>>> +
> >>>>>>>>>> +* ethdev: After bit-field header split was removed, the
> >>>>>>>>>> +``RTE_ETH_RX_OFFLOAD_HEADER_SPLIT``
> >>>>>>>>>> +offload and the ``split_hdr_size`` field in structure
> >>>>>>>>>> +``rte_eth_rxmode`` to enable header split offload are not
> >>>>>>>>>> +supported in any
> >>>>>>>>> PMDs. They will be removed in DPDK 22.11.
> >>>>>>>>>
> >>>>>>>>> It would have been good to talk about rte_eth_rxseg_split
> >>>>>>>>> which is similar and configured per-queue.
> >>>>>>>>
> >>>>>>>> Thanks for your suggestion.
> >>>>>>>>
> >>>>>>>> But I'm a little confused, are you referring that I need to
> >>>>>>>> involve protocol
> >>>>>>> based buffer split?
> >>>>>>>> About the deprecation of header split, I haven't realized its
> >>>>>>>> connection to
> >>>>>>> rte_eth_rxseg_split.
> >>>>>>>
> >>>>>>> What???
> >>>>>>> In old versions of your patch "ethdev: introduce protocol type
> >>>>>>> based header split"
> >>>>>>> you wrote:
> >>>>>>> "
> >>>>>>> A new proto field is introduced in the rte_eth_rxseg_split
> >>>>>>> structure reserved field to specify header protocol type.
> >>>>>>> With Rx offload flag RTE_ETH_RX_OFFLOAD_HEADER_SPLIT enabled
> >> and
> >>>>>>> protocol type configured, PMD will split the ingress packets
> >>>>>>> into two separate regions.
> >>>>>>> "
> >>>>>>
> >>>>>> It has a long history...
> >>>>>> It was corrected in v4 that RTE_ETH_RX_OFFLOAD_HEADER_SPLIT is
> >>>>>> used to enable header split offload with the header size
> >>>>>> controlled using
> >>>>> "split_hdr_size".
> >>>>>> But no single PMD actually supports
> >>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT
> >>>>> for this purpose.
> >>>>>> So we finally decide to deprecate this flag.
> >>>>>>
> >>>>>>
> >> http://patchwork.dpdk.org/project/dpdk/patch/20220402104109.472078
> >>>>>> -
> >>>>> 2-w
> >>>>>> enxuanx.wu@intel.com/
> >>>>>>
> >>>>>> In following series, I use RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT
> instead.
> >>>>>> It is for multi-segments packet split. And it still needs a
> >>>>>> "proto_hdr" field in
> >>>>> rte_eth_rxmode to configure split location.
> >>>>>
> >>>>> I know this history because I was the one asking you to deprecate this.
> >>>>> But it seems you didn't get the big picture.
> >>>>>
> >>>>>>>> Currently there are 2 acks, add more PMD maintainers to help
> >>>>>>>> review this deprecation notice for header split, thanks a lot!
> >>>>>>>
> >>>>>>> I cannot say my feeling strong enough.
> >>>>>>
> >>>>>> So IMO the deprecation for header split is not relevant with
> >>>>>> buffer split. But
> >>>>> we can still clean the code.
> >>>>>> Hope it make things clearer.
> >>>>>
> >>>>> They are almost the same features.
> >>>>> So when deprecating one, it is important to mention what remains.
> >>>>> If needed RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT can still be used and
> it
> >>>>> is configured per-queue, while RTE_ETH_RX_OFFLOAD_HEADER_SPLIT
> >> was
> >>>>> configurable per-port.
> >>>>
> >>>> Thanks for your clarification. It's clearer now.
> >>>> I was trying to figure out the whole history of header split, seems
> >>>> it is not enough.
> >>>>
> >>>
> >>> Isn't the intention of 'RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT' &
> >>> 'RTE_ETH_RX_OFFLOAD_HEADER_SPLIT' are different?
> >>> Cc'ed Slava for more comment.
> >>
> >> Hi, thank you for Cc'ing
> >>
> >> Yes, you are right, we have two splitting offloads, and these ones
> >> have the different intentions. As there are no PMDs actually handling
> >> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT, there should be no objections for
> >> this deprecation.
> >
> > Thanks for helping review.
> >
> > For 'BUFFER_SPLIT', I think it is clear. As Ferruh explains, it is used for
> splitting packets into multi-segments and multi-mempools based on
> rte_eth_rxseg_split.
> > Right now only mlx5 supports this offload.
> >
> > For 'HEADER_SPLIT', IMO it is used for splitting packets into two segments
> based on the split_hdr_size in rte_eth_rxmode.
> > And header split does not support split into multi-mempools(the same
> mempool).
> >
> 
> I looks like we don't have much details on the intention of the
> 'HEADER_SPLIT', it is not well documented.

Yes, since no PMD supports this offload, we should remove it
to make code cleaner.

Thanks,
Xuan

> 
> > So at this level, we can say that header split and buffer split are the same
> intention (split packets).
> > The functions supported by header split have been covered by buffer split.
> And no PMD actually supports 'HEADER_SPLIT'.
> >
> > Please corrects me if my understanding is wrong.
> >
> > Thanks,
> > Xuan
> >
> >
> >>
> >> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> >>
> >


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] doc: announce header split deprecation
  2022-07-15 12:13           ` Andrew Rybchenko
@ 2022-07-15 12:44             ` Ding, Xuan
  0 siblings, 0 replies; 20+ messages in thread
From: Ding, Xuan @ 2022-07-15 12:44 UTC (permalink / raw)
  To: Andrew Rybchenko, Thomas Monjalon, ferruh.yigit
  Cc: mdr, dev, stephen, mb, Zhang, Qi Z, asekhar, pbhagavatula, grive

Hi,

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Friday, July 15, 2022 8:14 PM
> To: Thomas Monjalon <thomas@monjalon.net>; Ding, Xuan
> <xuan.ding@intel.com>; ferruh.yigit@xilinx.com
> Cc: mdr@ashroe.eu; dev@dpdk.org; stephen@networkplumber.org;
> mb@smartsharesystems.com; Zhang, Qi Z <qi.z.zhang@intel.com>;
> asekhar@marvell.com; pbhagavatula@marvell.com; grive@u256.net
> Subject: Re: [PATCH] doc: announce header split deprecation
> 
> On 7/14/22 16:25, Thomas Monjalon wrote:
> > 14/07/2022 14:54, Ding, Xuan:
> >> Hi,
> >>
> >> From: Thomas Monjalon <thomas@monjalon.net>
> >>> 14/07/2022 07:50, Ding, Xuan:
> >>>> From: Thomas Monjalon <thomas@monjalon.net>
> >>>>> 23/05/2022 16:20, xuan.ding@intel.com:
> >>>>>> From: Xuan Ding <xuan.ding@intel.com>
> >>>>>>
> >>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced some
> >>> time
> >>>>> ago
> >>>>>> to substitute bit-field header_split in struct rte_eth_rxmode. It
> >>>>>> allows to enable header split offload with the header size
> >>>>>> controlled using split_hdr_size in the same structure.
> >>>>>>
> >>>>>> Right now, no single PMD actually supports
> >>>>>> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT with above definition. Many
> >>>>>> examples and test apps initialize the field to 0 explicitly. The
> >>>>>> most of drivers simply ignore split_hdr_size since the offload is
> >>>>>> not advertised, but
> >>>>> some double-check that its value is 0.
> >>>>>>
> >>>>>> So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and split_header_size
> >>> field
> >>>>>> will be removed in DPDK 22.11.
> >>>>>>
> >>>>>> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> >>>>>> ---
> >>>>>>   doc/guides/rel_notes/deprecation.rst | 4 ++++
> >>>>>>   1 file changed, 4 insertions(+)
> >>>>>>
> >>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
> >>>>>> b/doc/guides/rel_notes/deprecation.rst
> >>>>>> index 4e5b23c53d..b8114f29ed 100644
> >>>>>> --- a/doc/guides/rel_notes/deprecation.rst
> >>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
> >>>>>> @@ -125,3 +125,7 @@ Deprecation Notices
> >>>>>>     applications should be updated to use the ``dmadev`` library
> instead,
> >>>>>>     with the underlying HW-functionality being provided by the ``ioat``
> or
> >>>>>>     ``idxd`` dma drivers
> >>>>>> +
> >>>>>> +* ethdev: After bit-field header split was removed, the
> >>>>>> +``RTE_ETH_RX_OFFLOAD_HEADER_SPLIT``
> >>>>>> +offload and the ``split_hdr_size`` field in structure
> >>>>>> +``rte_eth_rxmode`` to enable header split offload are not
> >>>>>> +supported in any
> >>>>> PMDs. They will be removed in DPDK 22.11.
> >>>>>
> >>>>> It would have been good to talk about rte_eth_rxseg_split which is
> >>>>> similar and configured per-queue.
> >>>>
> >>>> Thanks for your suggestion.
> >>>>
> >>>> But I'm a little confused, are you referring that I need to involve
> >>>> protocol
> >>> based buffer split?
> >>>> About the deprecation of header split, I haven't realized its
> >>>> connection to
> >>> rte_eth_rxseg_split.
> >>>
> >>> What???
> >>> In old versions of your patch "ethdev: introduce protocol type based
> >>> header split"
> >>> you wrote:
> >>> "
> >>> A new proto field is introduced in the rte_eth_rxseg_split structure
> >>> reserved field to specify header protocol type.
> >>> With Rx offload flag RTE_ETH_RX_OFFLOAD_HEADER_SPLIT enabled and
> >>> protocol type configured, PMD will split the ingress packets into
> >>> two separate regions.
> >>> "
> >>
> >> It has a long history...
> >> It was corrected in v4 that RTE_ETH_RX_OFFLOAD_HEADER_SPLIT is used
> >> to enable header split offload with the header size controlled using
> "split_hdr_size".
> >> But no single PMD actually supports
> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT for this purpose.
> >> So we finally decide to deprecate this flag.
> >>
> >> http://patchwork.dpdk.org/project/dpdk/patch/20220402104109.472078-
> 2-
> >> wenxuanx.wu@intel.com/
> >>
> >> In following series, I use RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT instead.
> >> It is for multi-segments packet split. And it still needs a "proto_hdr" field
> in rte_eth_rxmode to configure split location.
> >
> > I know this history because I was the one asking you to deprecate this.
> > But it seems you didn't get the big picture.
> >
> >>>> Currently there are 2 acks, add more PMD maintainers to help review
> >>>> this deprecation notice for header split, thanks a lot!
> >>>
> >>> I cannot say my feeling strong enough.
> >>
> >> So IMO the deprecation for header split is not relevant with buffer split.
> But we can still clean the code.
> >> Hope it make things clearer.
> >
> > They are almost the same features.
> > So when deprecating one, it is important to mention what remains.
> > If needed RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT can still be used and it is
> > configured per-queue, while RTE_ETH_RX_OFFLOAD_HEADER_SPLIT was
> > configurable per-port.
> >
> > Andrew, Ferruh, do you agree to improve this deprecation notice by
> > adding above information?
> 
> +1 tt is definitely a very good idea.

Thanks, v2 is on the way.

Regards,
Xuan

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2] doc: announce header split deprecation
  2022-05-23 14:20 [PATCH] doc: announce header split deprecation xuan.ding
  2022-05-24 14:58 ` Ray Kinsella
  2022-07-13  9:49 ` Thomas Monjalon
@ 2022-07-15 20:30 ` xuan.ding
  2022-07-16 21:28   ` Thomas Monjalon
  2 siblings, 1 reply; 20+ messages in thread
From: xuan.ding @ 2022-07-15 20:30 UTC (permalink / raw)
  To: thomas, mdr, andrew.rybchenko, ferruh.yigit, viacheslavo
  Cc: dev, stephen, mb, qi.z.zhang, asekhar, pbhagavatula, Xuan Ding

From: Xuan Ding <xuan.ding@intel.com>

RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced some time ago to
substitute bit-field header_split in struct rte_eth_rxmode. It allows
to enable per-port header split offload with the header size controlled
using split_hdr_size in the same structure.

Right now, no single PMD actually supports RTE_ETH_RX_OFFLOAD_HEADER_SPLIT
with above definition. Many examples and test apps initialize the field
to 0 explicitly. The most of drivers simply ignore split_hdr_size since
the offload is not advertised, but some double-check that its value is 0.

So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and split_header_size field
will be removed in DPDK 22.11. After DPDK 22.11 LTS, the
RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT can still be used for per-queue Rx
packet split offload, which is configured by rte_eth_rxseg_split.

Signed-off-by: Xuan Ding <xuan.ding@intel.com>
Acked-by: Ray Kinsella <mdr@ashroe.eu>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Acked-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
---
v2:
* Refine doc to mention remained RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT.
---
 doc/guides/rel_notes/deprecation.rst | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 1e5e532e60..ae37559275 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -158,3 +158,10 @@ Deprecation Notices
   applications should be updated to use the ``dmadev`` library instead,
   with the underlying HW-functionality being provided by the ``ioat`` or
   ``idxd`` dma drivers
+
+* ethdev: Since no single PMD supports ``RTE_ETH_RX_OFFLOAD_HEADER_SPLIT`` offload
+and the ``split_hdr_size`` field in structure ``rte_eth_rxmode`` to enable per-port
+header split as they were designed, they will be removed in DPDK 22.11.
+After DPDK 22.11 LTS, the ``RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT`` can still be
+used for per-queue Rx packet split offload, and it is configured
+by ``rte_eth_rxseg_split``.
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2] doc: announce header split deprecation
  2022-07-15 20:30 ` [PATCH v2] " xuan.ding
@ 2022-07-16 21:28   ` Thomas Monjalon
  0 siblings, 0 replies; 20+ messages in thread
From: Thomas Monjalon @ 2022-07-16 21:28 UTC (permalink / raw)
  To: Xuan Ding
  Cc: mdr, andrew.rybchenko, ferruh.yigit, viacheslavo, dev, stephen,
	mb, qi.z.zhang, asekhar, pbhagavatula

15/07/2022 22:30, xuan.ding@intel.com:
> From: Xuan Ding <xuan.ding@intel.com>
> 
> RTE_ETH_RX_OFFLOAD_HEADER_SPLIT offload was introduced some time ago to
> substitute bit-field header_split in struct rte_eth_rxmode. It allows
> to enable per-port header split offload with the header size controlled
> using split_hdr_size in the same structure.
> 
> Right now, no single PMD actually supports RTE_ETH_RX_OFFLOAD_HEADER_SPLIT
> with above definition. Many examples and test apps initialize the field
> to 0 explicitly. The most of drivers simply ignore split_hdr_size since
> the offload is not advertised, but some double-check that its value is 0.
> 
> So the RTE_ETH_RX_OFFLOAD_HEADER_SPLIT and split_header_size field
> will be removed in DPDK 22.11. After DPDK 22.11 LTS, the
> RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT can still be used for per-queue Rx
> packet split offload, which is configured by rte_eth_rxseg_split.
> 
> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> Acked-by: Ray Kinsella <mdr@ashroe.eu>
> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Acked-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
Acked-by: Thomas Monjalon <thomas@monjalon.net>

This v2 is a lot better for the users to understand what happens.
Fixed the indent and moved few words:

* ethdev: Since no single PMD supports ``RTE_ETH_RX_OFFLOAD_HEADER_SPLIT``
  offload and the ``split_hdr_size`` field in structure ``rte_eth_rxmode``
  to enable per-port header split, they will be removed in DPDK 22.11.
  The per-queue Rx packet split offload ``RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT``
  can still be used, and it is configured by ``rte_eth_rxseg_split``.

Applied, thanks.



^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2022-07-16 21:28 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-23 14:20 [PATCH] doc: announce header split deprecation xuan.ding
2022-05-24 14:58 ` Ray Kinsella
2022-07-12 11:43   ` Andrew Rybchenko
2022-07-13  9:49 ` Thomas Monjalon
2022-07-14  5:50   ` Ding, Xuan
2022-07-14  8:08     ` Thomas Monjalon
2022-07-14 12:54       ` Ding, Xuan
2022-07-14 13:25         ` Thomas Monjalon
2022-07-14 14:07           ` Ding, Xuan
2022-07-14 15:58             ` Ferruh Yigit
2022-07-14 16:03               ` Ferruh Yigit
2022-07-15  8:52                 ` Ding, Xuan
2022-07-14 16:56               ` Slava Ovsiienko
2022-07-15  8:28                 ` Ding, Xuan
2022-07-15 11:34                   ` Ferruh Yigit
2022-07-15 12:43                     ` Ding, Xuan
2022-07-15 12:13           ` Andrew Rybchenko
2022-07-15 12:44             ` Ding, Xuan
2022-07-15 20:30 ` [PATCH v2] " xuan.ding
2022-07-16 21:28   ` Thomas Monjalon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.