[dpdk-dev] [patch] doc: announce API change in ethdev offload flags

[dpdk-dev] [patch] doc: announce API change in ethdev offload flags

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Add new offload flags ``DEV_RX_OFFLOAD_PTYPE``, ``DEV_RX_OFFLOAD_RSS``
and ``DEV_RX_OFFLOAD_FLOW_MARK``.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 doc/guides/rel_notes/deprecation.rst | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 37b8592b6..a10f69e98 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -78,3 +78,12 @@ Deprecation Notices
   to set new power environment if power environment was already initialized.
   In this case the function will return -1 unless the environment is unset first
   (using ``rte_power_unset_env``). Other function usage scenarios will not change.
+
+* ethdev: New offload flags ``DEV_RX_OFFLOAD_PTYPE``, ``DEV_RX_OFFLOAD_RSS``
+  and ``DEV_RX_OFFLOAD_FLOW_MARK`` will be added in 19.11.
+  This will allow application to enable or disable PMDs from updating
+  ``rte_mbuf`` fields ``rte_mbuf::packet_type``, ``rte_mbuf::hash::rss`` and
+  ``rte_mbuf::hash::fdir`` respectively.
+  In 19.11 PMDs will still update the fields even when the offloads are not
+  enabled. This is done as an optimization to avoid writes to ``rte_mbuf`` fields,
+  the exact semantics of the flags will be worked out later.
-- 
2.17.1

Re: [dpdk-dev] [patch] doc: announce API change in ethdev offload flags

[Andrew Rybchenko]

On 8/7/19 7:09 PM, pbhagavatula@marvell.com wrote:
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
> Add new offload flags ``DEV_RX_OFFLOAD_PTYPE``, ``DEV_RX_OFFLOAD_RSS``
> and ``DEV_RX_OFFLOAD_FLOW_MARK``.
>
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
>   doc/guides/rel_notes/deprecation.rst | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 37b8592b6..a10f69e98 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -78,3 +78,12 @@ Deprecation Notices
>     to set new power environment if power environment was already initialized.
>     In this case the function will return -1 unless the environment is unset first
>     (using ``rte_power_unset_env``). Other function usage scenarios will not change.
> +
> +* ethdev: New offload flags ``DEV_RX_OFFLOAD_PTYPE``, ``DEV_RX_OFFLOAD_RSS``

I'd name it DEV_RX_OFFLOAD_RSS_HASH

> +  and ``DEV_RX_OFFLOAD_FLOW_MARK`` will be added in 19.11.
> +  This will allow application to enable or disable PMDs from updating
> +  ``rte_mbuf`` fields ``rte_mbuf::packet_type``, ``rte_mbuf::hash::rss`` and
> +  ``rte_mbuf::hash::fdir`` respectively.
> +  In 19.11 PMDs will still update the fields even when the offloads are not
> +  enabled. This is done as an optimization to avoid writes to ``rte_mbuf`` fields,
> +  the exact semantics of the flags will be worked out later.

Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

since I was going to propose something similar. However, there is a problem
with all three, since they are efficiently enabled by default now and it 
is a
proposal to change it. As Stephen pointed it out it is bad from application
point of view since it is invisible (no API breakage, compilation is 
still OK),
so it is hard for application to notice it. It should be discussed if it 
is acceptable
or should be addressed in a different way (negative offloads? not ideal 
as well).

I think that situation with tso_segsz for LRO, discussed in a separate 
thread [1],
is very similar in fact. No problem with compilation, but semantics change.

[1] https://patches.dpdk.org/patch/57493/

[dpdk-dev] [patch v2] doc: announce API change in ethdev offload flags

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Add new offload flags ``DEV_RX_OFFLOAD_PTYPE``, ``DEV_RX_OFFLOAD_RSS``
and ``DEV_RX_OFFLOAD_FLOW_MARK``.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 v2: Reword for clarity.

 doc/guides/rel_notes/deprecation.rst | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 37b8592b6..79e50a272 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -78,3 +78,16 @@ Deprecation Notices
   to set new power environment if power environment was already initialized.
   In this case the function will return -1 unless the environment is unset first
   (using ``rte_power_unset_env``). Other function usage scenarios will not change.
+
+* ethdev: New offload flags ``DEV_RX_OFFLOAD_PTYPE``, ``DEV_RX_OFFLOAD_RSS``
+  and ``DEV_RX_OFFLOAD_FLOW_MARK`` will be added in 19.11.
+  This will allow application to enable or disable PMDs from updating
+  ``rte_mbuf`` fields ``rte_mbuf::packet_type``, ``rte_mbuf::hash::rss`` and
+  ``rte_mbuf::hash::fdir`` respectively.
+  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields on Rx and
+  thereby improve Rx performance if application wishes do so.
+  In 19.11 PMDs will still update the fields even when the offloads are not
+  enabled.
+  The exact semantics of the flags will be worked out later either by making
+  them negative offloads to avoid application change or positive offload to
+  align with existing offload flag semantics.
--
2.17.1

Re: [dpdk-dev] [patch v2] doc: announce API change in ethdev offload flags

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: pbhagavatula@marvell.com <pbhagavatula@marvell.com>
> Sent: Thursday, August 8, 2019 1:48 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> stephen@networkplumber.org; arybchenko@solarflare.com;
> hemant.agrawal@nxp.com; thomas@monjalon.net; ferruh.yigit@intel.com;
> bruce.richardson@intel.com; Neil Horman <nhorman@tuxdriver.com>; John
> McNamara <john.mcnamara@intel.com>; Marko Kovacevic
> <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org; Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> Subject: [dpdk-dev] [patch v2] doc: announce API change in ethdev offload flags
> 
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> 
> Add new offload flags ``DEV_RX_OFFLOAD_PTYPE``, ``DEV_RX_OFFLOAD_RSS``
> and ``DEV_RX_OFFLOAD_FLOW_MARK``.
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

With DEV_RX_OFFLOAD_RSS to DEV_RX_OFFLOAD_RSS_HASH name change as Andrew suggested.

Acked-by: Jerin Jacob <jerinj@marvell.com>


> ---
>  v2: Reword for clarity.
> 
>  doc/guides/rel_notes/deprecation.rst | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index 37b8592b6..79e50a272 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -78,3 +78,16 @@ Deprecation Notices
>    to set new power environment if power environment was already initialized.
>    In this case the function will return -1 unless the environment is unset first
>    (using ``rte_power_unset_env``). Other function usage scenarios will not
> change.
> +
> +* ethdev: New offload flags ``DEV_RX_OFFLOAD_PTYPE``,
> +``DEV_RX_OFFLOAD_RSS``
> +  and ``DEV_RX_OFFLOAD_FLOW_MARK`` will be added in 19.11.
> +  This will allow application to enable or disable PMDs from updating
> +  ``rte_mbuf`` fields ``rte_mbuf::packet_type``,
> +``rte_mbuf::hash::rss`` and
> +  ``rte_mbuf::hash::fdir`` respectively.
> +  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields on
> +Rx and
> +  thereby improve Rx performance if application wishes do so.
> +  In 19.11 PMDs will still update the fields even when the offloads are
> +not
> +  enabled.
> +  The exact semantics of the flags will be worked out later either by
> +making
> +  them negative offloads to avoid application change or positive
> +offload to
> +  align with existing offload flag semantics.
> --
> 2.17.1

Re: [dpdk-dev] [patch v2] doc: announce API change in ethdev offload flags

[Pavan Nikhilesh Bhagavatula]

>>
>> Add new offload flags ``DEV_RX_OFFLOAD_PTYPE``,
>``DEV_RX_OFFLOAD_RSS``
>> and ``DEV_RX_OFFLOAD_FLOW_MARK``.
>>
>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
>
>With DEV_RX_OFFLOAD_RSS to DEV_RX_OFFLOAD_RSS_HASH name
>change as Andrew suggested.

Sorry I missed it in previous mail. I will send v3.

Thanks for the heads up.
Pavan

>
>Acked-by: Jerin Jacob <jerinj@marvell.com>
>
>
>> ---
>>  v2: Reword for clarity.
>>

[dpdk-dev] [patch v3] doc: announce API change in ethdev offload flags

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Add new offload flags ``DEV_RX_OFFLOAD_PTYPE``, ``DEV_RX_OFFLOAD_RSS``
and ``DEV_RX_OFFLOAD_FLOW_MARK``.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
---
 v3 Changes:
 - DEV_RX_OFFLOAD_RSS -> DEV_RX_OFFLOAD_RSS_HASH (anndrew).

 v2 Changes:
 - Reword for clarity.

 doc/guides/rel_notes/deprecation.rst | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 37b8592b6..056c5709f 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -78,3 +78,16 @@ Deprecation Notices
   to set new power environment if power environment was already initialized.
   In this case the function will return -1 unless the environment is unset first
   (using ``rte_power_unset_env``). Other function usage scenarios will not change.
+
+* ethdev: New offload flags ``DEV_RX_OFFLOAD_PTYPE``, ``DEV_RX_OFFLOAD_RSS_HASH``
+  and ``DEV_RX_OFFLOAD_FLOW_MARK`` will be added in 19.11.
+  This will allow application to enable or disable PMDs from updating
+  ``rte_mbuf`` fields ``rte_mbuf::packet_type``, ``rte_mbuf::hash::rss`` and
+  ``rte_mbuf::hash::fdir`` respectively.
+  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields on Rx and
+  thereby improve Rx performance if application wishes do so.
+  In 19.11 PMDs will still update the fields even when the offloads are not
+  enabled.
+  The exact semantics of the flags will be worked out later either by making
+  them negative offloads to avoid application change or positive offload to
+  align with existing offload flag semantics.
--
2.17.1

Re: [dpdk-dev] [patch v3] doc: announce API change in ethdev offload flags

[Ananyev, Konstantin]

Hi guys,

> 
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> 
> Add new offload flags ``DEV_RX_OFFLOAD_PTYPE``, ``DEV_RX_OFFLOAD_RSS``
> and ``DEV_RX_OFFLOAD_FLOW_MARK``.
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> Acked-by: Jerin Jacob <jerinj@marvell.com>
> ---
>  v3 Changes:
>  - DEV_RX_OFFLOAD_RSS -> DEV_RX_OFFLOAD_RSS_HASH (anndrew).
> 
>  v2 Changes:
>  - Reword for clarity.
> 
>  doc/guides/rel_notes/deprecation.rst | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 37b8592b6..056c5709f 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -78,3 +78,16 @@ Deprecation Notices
>    to set new power environment if power environment was already initialized.
>    In this case the function will return -1 unless the environment is unset first
>    (using ``rte_power_unset_env``). Other function usage scenarios will not change.
> +
> +* ethdev: New offload flags ``DEV_RX_OFFLOAD_PTYPE``, ``DEV_RX_OFFLOAD_RSS_HASH``
> +  and ``DEV_RX_OFFLOAD_FLOW_MARK`` will be added in 19.11.

One question about DEV_RX_OFFLOAD_PTYPE:
Does it mean that new ol_flags value (PKT_RX_PTYPE) will be introduced to
indicate that mbuf.packet_type value is set?
Or PMD will have to set  mbuf.packet_type to zero,
when  DEV_RX_OFFLOAD_PTYPE was not enabled by user?
If so, what is the advantage?
Again in that case, would it be more plausible to introduce something like:
rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t ptype_mask);
instead of DEV_RX_OFFLOAD_PTYPE?
Konstantin

> +  This will allow application to enable or disable PMDs from updating
> +  ``rte_mbuf`` fields ``rte_mbuf::packet_type``, ``rte_mbuf::hash::rss`` and
> +  ``rte_mbuf::hash::fdir`` respectively.
> +  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields on Rx and
> +  thereby improve Rx performance if application wishes do so.
> +  In 19.11 PMDs will still update the fields even when the offloads are not
> +  enabled.
> +  The exact semantics of the flags will be worked out later either by making
> +  them negative offloads to avoid application change or positive offload to
> +  align with existing offload flag semantics.
> --
> 2.17.1

Re: [dpdk-dev] [patch v3] doc: announce API change in ethdev offload flags

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Thursday, August 8, 2019 2:53 PM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Jerin Jacob
> Kollanukkaran <jerinj@marvell.com>; stephen@networkplumber.org;
> arybchenko@solarflare.com; hemant.agrawal@nxp.com;
> thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>; Richardson,
> Bruce <bruce.richardson@intel.com>; Neil Horman
> <nhorman@tuxdriver.com>; Mcnamara, John <john.mcnamara@intel.com>;
> Kovacevic, Marko <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org
> Subject: [EXT] RE: [dpdk-dev] [patch v3] doc: announce API change in ethdev
> offload flags
> 
> External Email
> 
> ----------------------------------------------------------------------
> 
> Hi guys,
> 
> >
> > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >
> > Add new offload flags ``DEV_RX_OFFLOAD_PTYPE``,
> ``DEV_RX_OFFLOAD_RSS``
> > and ``DEV_RX_OFFLOAD_FLOW_MARK``.
> >
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > Acked-by: Jerin Jacob <jerinj@marvell.com>
> > ---
> >  v3 Changes:
> >  - DEV_RX_OFFLOAD_RSS -> DEV_RX_OFFLOAD_RSS_HASH (anndrew).
> >
> >  v2 Changes:
> >  - Reword for clarity.
> >
> >  doc/guides/rel_notes/deprecation.rst | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/doc/guides/rel_notes/deprecation.rst
> > b/doc/guides/rel_notes/deprecation.rst
> > index 37b8592b6..056c5709f 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -78,3 +78,16 @@ Deprecation Notices
> >    to set new power environment if power environment was already
> initialized.
> >    In this case the function will return -1 unless the environment is unset
> first
> >    (using ``rte_power_unset_env``). Other function usage scenarios will not
> change.
> > +
> > +* ethdev: New offload flags ``DEV_RX_OFFLOAD_PTYPE``,
> > +``DEV_RX_OFFLOAD_RSS_HASH``
> > +  and ``DEV_RX_OFFLOAD_FLOW_MARK`` will be added in 19.11.
> 
> One question about DEV_RX_OFFLOAD_PTYPE:
> Does it mean that new ol_flags value (PKT_RX_PTYPE) will be introduced to
> indicate that mbuf.packet_type value is set?
> Or PMD will have to set  mbuf.packet_type to zero, when
> DEV_RX_OFFLOAD_PTYPE was not enabled by user?

I was thinking when DEV_RX_OFFLOAD_PTYPE is set
- mbuf.packet_type will be valid and mbuf.packet_type will have parsed packet type.
If not set
- mbuf.packet_type can be anything application should not use mbuf.packet_type field.

This will avoid writes 0 to mbuf.packet_type and packet_type parsing if offload is not set.


> If so, what is the advantage?
> Again in that case, would it be more plausible to introduce something like:
> rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t
> ptype_mask); instead of DEV_RX_OFFLOAD_PTYPE?

Any scheme is fine where we can skip the  write 0 to mbuf.packet_type and packet_type parsing
If application is NOT interested in packet_type.

> Konstantin
> 
> > +  This will allow application to enable or disable PMDs from updating
> > + ``rte_mbuf`` fields ``rte_mbuf::packet_type``,
> > + ``rte_mbuf::hash::rss`` and  ``rte_mbuf::hash::fdir`` respectively.
> > +  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields
> > + on Rx and  thereby improve Rx performance if application wishes do so.
> > +  In 19.11 PMDs will still update the fields even when the offloads
> > + are not  enabled.
> > +  The exact semantics of the flags will be worked out later either by
> > + making  them negative offloads to avoid application change or
> > + positive offload to  align with existing offload flag semantics.
> > --
> > 2.17.1

Re: [dpdk-dev] [patch v3] doc: announce API change in ethdev offload flags

[Ananyev, Konstantin]

Hi Jerin,

> >
> > Hi guys,
> >
> > >
> > > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > >
> > > Add new offload flags ``DEV_RX_OFFLOAD_PTYPE``,
> > ``DEV_RX_OFFLOAD_RSS``
> > > and ``DEV_RX_OFFLOAD_FLOW_MARK``.
> > >
> > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > > Acked-by: Jerin Jacob <jerinj@marvell.com>
> > > ---
> > >  v3 Changes:
> > >  - DEV_RX_OFFLOAD_RSS -> DEV_RX_OFFLOAD_RSS_HASH (anndrew).
> > >
> > >  v2 Changes:
> > >  - Reword for clarity.
> > >
> > >  doc/guides/rel_notes/deprecation.rst | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > >
> > > diff --git a/doc/guides/rel_notes/deprecation.rst
> > > b/doc/guides/rel_notes/deprecation.rst
> > > index 37b8592b6..056c5709f 100644
> > > --- a/doc/guides/rel_notes/deprecation.rst
> > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > @@ -78,3 +78,16 @@ Deprecation Notices
> > >    to set new power environment if power environment was already
> > initialized.
> > >    In this case the function will return -1 unless the environment is unset
> > first
> > >    (using ``rte_power_unset_env``). Other function usage scenarios will not
> > change.
> > > +
> > > +* ethdev: New offload flags ``DEV_RX_OFFLOAD_PTYPE``,
> > > +``DEV_RX_OFFLOAD_RSS_HASH``
> > > +  and ``DEV_RX_OFFLOAD_FLOW_MARK`` will be added in 19.11.
> >
> > One question about DEV_RX_OFFLOAD_PTYPE:
> > Does it mean that new ol_flags value (PKT_RX_PTYPE) will be introduced to
> > indicate that mbuf.packet_type value is set?
> > Or PMD will have to set  mbuf.packet_type to zero, when
> > DEV_RX_OFFLOAD_PTYPE was not enabled by user?
> 
> I was thinking when DEV_RX_OFFLOAD_PTYPE is set
> - mbuf.packet_type will be valid and mbuf.packet_type will have parsed packet type.
> If not set
> - mbuf.packet_type can be anything application should not use mbuf.packet_type field.

But in that case, we do need a new value for ol_flags, PKT_RX_PTYPE or so, right?

> 
> This will avoid writes 0 to mbuf.packet_type and packet_type parsing if offload is not set.
> 
> 
> > If so, what is the advantage?
> > Again in that case, would it be more plausible to introduce something like:
> > rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t
> > ptype_mask); instead of DEV_RX_OFFLOAD_PTYPE?
> 
> Any scheme is fine where we can skip the  write 0 to mbuf.packet_type and packet_type parsing
> If application is NOT interested in packet_type.
> 
> > Konstantin
> >
> > > +  This will allow application to enable or disable PMDs from updating
> > > + ``rte_mbuf`` fields ``rte_mbuf::packet_type``,
> > > + ``rte_mbuf::hash::rss`` and  ``rte_mbuf::hash::fdir`` respectively.
> > > +  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields
> > > + on Rx and  thereby improve Rx performance if application wishes do so.
> > > +  In 19.11 PMDs will still update the fields even when the offloads
> > > + are not  enabled.
> > > +  The exact semantics of the flags will be worked out later either by
> > > + making  them negative offloads to avoid application change or
> > > + positive offload to  align with existing offload flag semantics.
> > > --
> > > 2.17.1

Re: [dpdk-dev] [patch v3] doc: announce API change in ethdev offload flags

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Thursday, August 8, 2019 3:39 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Pavan Nikhilesh
> Bhagavatula <pbhagavatula@marvell.com>; stephen@networkplumber.org;
> arybchenko@solarflare.com; hemant.agrawal@nxp.com;
> thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>; Richardson,
> Bruce <bruce.richardson@intel.com>; Neil Horman
> <nhorman@tuxdriver.com>; Mcnamara, John <john.mcnamara@intel.com>;
> Kovacevic, Marko <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org
> Subject: [EXT] RE: [dpdk-dev] [patch v3] doc: announce API change in ethdev
> offload flags
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi Jerin,

Hi Konstantin,


> 
> > >
> > > Hi guys,
> > >
> > > >
> > > > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > >
> > > > Add new offload flags ``DEV_RX_OFFLOAD_PTYPE``,
> > > ``DEV_RX_OFFLOAD_RSS``
> > > > and ``DEV_RX_OFFLOAD_FLOW_MARK``.
> > > >
> > > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > > Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > > > Acked-by: Jerin Jacob <jerinj@marvell.com>
> > > > ---
> > > >  v3 Changes:
> > > >  - DEV_RX_OFFLOAD_RSS -> DEV_RX_OFFLOAD_RSS_HASH (anndrew).
> > > >
> > > >  v2 Changes:
> > > >  - Reword for clarity.
> > > >
> > > >  doc/guides/rel_notes/deprecation.rst | 13 +++++++++++++
> > > >  1 file changed, 13 insertions(+)
> > > >
> > > > diff --git a/doc/guides/rel_notes/deprecation.rst
> > > > b/doc/guides/rel_notes/deprecation.rst
> > > > index 37b8592b6..056c5709f 100644
> > > > --- a/doc/guides/rel_notes/deprecation.rst
> > > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > > @@ -78,3 +78,16 @@ Deprecation Notices
> > > >    to set new power environment if power environment was already
> > > initialized.
> > > >    In this case the function will return -1 unless the environment
> > > > is unset
> > > first
> > > >    (using ``rte_power_unset_env``). Other function usage scenarios
> > > > will not
> > > change.
> > > > +
> > > > +* ethdev: New offload flags ``DEV_RX_OFFLOAD_PTYPE``,
> > > > +``DEV_RX_OFFLOAD_RSS_HASH``
> > > > +  and ``DEV_RX_OFFLOAD_FLOW_MARK`` will be added in 19.11.
> > >
> > > One question about DEV_RX_OFFLOAD_PTYPE:
> > > Does it mean that new ol_flags value (PKT_RX_PTYPE) will be
> > > introduced to indicate that mbuf.packet_type value is set?
> > > Or PMD will have to set  mbuf.packet_type to zero, when
> > > DEV_RX_OFFLOAD_PTYPE was not enabled by user?
> >
> > I was thinking when DEV_RX_OFFLOAD_PTYPE is set
> > - mbuf.packet_type will be valid and mbuf.packet_type will have parsed
> packet type.
> > If not set
> > - mbuf.packet_type can be anything application should not use
> mbuf.packet_type field.
> 
> But in that case, we do need a new value for ol_flags, PKT_RX_PTYPE or so,
> right?

Since application has two knobs rte_eth_dev_get_supported_ptypes() and 
DEV_RX_OFFLOAD_PTYPE. We may not need to new ol_flags for this change. Right?
i.e if application sets the DEV_RX_OFFLOAD_PTYPE, The application will
get the parsed ptypes by the driver(= rte_eth_dev_get_supported_ptypes()).
So there is no scope ambiguity. Right?


> 
> >
> > This will avoid writes 0 to mbuf.packet_type and packet_type parsing if
> offload is not set.
> >
> >
> > > If so, what is the advantage?
> > > Again in that case, would it be more plausible to introduce something like:
> > > rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t
> > > ptype_mask); instead of DEV_RX_OFFLOAD_PTYPE?
> >
> > Any scheme is fine where we can skip the  write 0 to mbuf.packet_type
> > and packet_type parsing If application is NOT interested in packet_type.
> >
> > > Konstantin
> > >
> > > > +  This will allow application to enable or disable PMDs from
> > > > + updating ``rte_mbuf`` fields ``rte_mbuf::packet_type``,
> > > > + ``rte_mbuf::hash::rss`` and  ``rte_mbuf::hash::fdir`` respectively.
> > > > +  This scheme will allow PMDs to avoid writes to ``rte_mbuf``
> > > > + fields on Rx and  thereby improve Rx performance if application
> wishes do so.
> > > > +  In 19.11 PMDs will still update the fields even when the
> > > > + offloads are not  enabled.
> > > > +  The exact semantics of the flags will be worked out later
> > > > + either by making  them negative offloads to avoid application
> > > > + change or positive offload to  align with existing offload flag
> semantics.
> > > > --
> > > > 2.17.1

Re: [dpdk-dev] [patch v3] doc: announce API change in ethdev offload flags

[Ananyev, Konstantin]

> -----Original Message-----
> From: Jerin Jacob Kollanukkaran [mailto:jerinj@marvell.com]
> Sent: Thursday, August 8, 2019 11:23 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
> stephen@networkplumber.org; arybchenko@solarflare.com; hemant.agrawal@nxp.com; thomas@monjalon.net; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Richardson, Bruce <bruce.richardson@intel.com>; Neil Horman <nhorman@tuxdriver.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Kovacevic, Marko <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [patch v3] doc: announce API change in ethdev offload flags
> 
> > -----Original Message-----
> > From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > Sent: Thursday, August 8, 2019 3:39 PM
> > To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Pavan Nikhilesh
> > Bhagavatula <pbhagavatula@marvell.com>; stephen@networkplumber.org;
> > arybchenko@solarflare.com; hemant.agrawal@nxp.com;
> > thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>; Richardson,
> > Bruce <bruce.richardson@intel.com>; Neil Horman
> > <nhorman@tuxdriver.com>; Mcnamara, John <john.mcnamara@intel.com>;
> > Kovacevic, Marko <marko.kovacevic@intel.com>
> > Cc: dev@dpdk.org
> > Subject: [EXT] RE: [dpdk-dev] [patch v3] doc: announce API change in ethdev
> > offload flags
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > Hi Jerin,
> 
> Hi Konstantin,
> 
> 
> >
> > > >
> > > > Hi guys,
> > > >
> > > > >
> > > > > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > > >
> > > > > Add new offload flags ``DEV_RX_OFFLOAD_PTYPE``,
> > > > ``DEV_RX_OFFLOAD_RSS``
> > > > > and ``DEV_RX_OFFLOAD_FLOW_MARK``.
> > > > >
> > > > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > > > Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > > > > Acked-by: Jerin Jacob <jerinj@marvell.com>
> > > > > ---
> > > > >  v3 Changes:
> > > > >  - DEV_RX_OFFLOAD_RSS -> DEV_RX_OFFLOAD_RSS_HASH (anndrew).
> > > > >
> > > > >  v2 Changes:
> > > > >  - Reword for clarity.
> > > > >
> > > > >  doc/guides/rel_notes/deprecation.rst | 13 +++++++++++++
> > > > >  1 file changed, 13 insertions(+)
> > > > >
> > > > > diff --git a/doc/guides/rel_notes/deprecation.rst
> > > > > b/doc/guides/rel_notes/deprecation.rst
> > > > > index 37b8592b6..056c5709f 100644
> > > > > --- a/doc/guides/rel_notes/deprecation.rst
> > > > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > > > @@ -78,3 +78,16 @@ Deprecation Notices
> > > > >    to set new power environment if power environment was already
> > > > initialized.
> > > > >    In this case the function will return -1 unless the environment
> > > > > is unset
> > > > first
> > > > >    (using ``rte_power_unset_env``). Other function usage scenarios
> > > > > will not
> > > > change.
> > > > > +
> > > > > +* ethdev: New offload flags ``DEV_RX_OFFLOAD_PTYPE``,
> > > > > +``DEV_RX_OFFLOAD_RSS_HASH``
> > > > > +  and ``DEV_RX_OFFLOAD_FLOW_MARK`` will be added in 19.11.
> > > >
> > > > One question about DEV_RX_OFFLOAD_PTYPE:
> > > > Does it mean that new ol_flags value (PKT_RX_PTYPE) will be
> > > > introduced to indicate that mbuf.packet_type value is set?
> > > > Or PMD will have to set  mbuf.packet_type to zero, when
> > > > DEV_RX_OFFLOAD_PTYPE was not enabled by user?
> > >
> > > I was thinking when DEV_RX_OFFLOAD_PTYPE is set
> > > - mbuf.packet_type will be valid and mbuf.packet_type will have parsed
> > packet type.
> > > If not set
> > > - mbuf.packet_type can be anything application should not use
> > mbuf.packet_type field.
> >
> > But in that case, we do need a new value for ol_flags, PKT_RX_PTYPE or so,
> > right?
> 
> Since application has two knobs rte_eth_dev_get_supported_ptypes() and
> DEV_RX_OFFLOAD_PTYPE. We may not need to new ol_flags for this change. Right?
> i.e if application sets the DEV_RX_OFFLOAD_PTYPE, The application will
> get the parsed ptypes by the driver(= rte_eth_dev_get_supported_ptypes()).
> So there is no scope ambiguity. Right?

I still think there is:
Imagine user has 2 eth devices, one does support DEV_RX_OFFLOAD_PTYPE,
second doesn't.  Now he has a mix of packets from both devices, that you want t process.
How would he figure out for which of them ptype values are valid, and for each are not?
Trace back from what port he has received them? 
Not very convenient, and not always possible.
I think we need either to introduce new ol_flag value (as we usually do for other RX offloads),
or force PMD to always set ptype value.   
Konstantin

> 
> 
> >
> > >
> > > This will avoid writes 0 to mbuf.packet_type and packet_type parsing if
> > offload is not set.
> > >
> > >
> > > > If so, what is the advantage?
> > > > Again in that case, would it be more plausible to introduce something like:
> > > > rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t
> > > > ptype_mask); instead of DEV_RX_OFFLOAD_PTYPE?
> > >
> > > Any scheme is fine where we can skip the  write 0 to mbuf.packet_type
> > > and packet_type parsing If application is NOT interested in packet_type.
> > >
> > > > Konstantin
> > > >
> > > > > +  This will allow application to enable or disable PMDs from
> > > > > + updating ``rte_mbuf`` fields ``rte_mbuf::packet_type``,
> > > > > + ``rte_mbuf::hash::rss`` and  ``rte_mbuf::hash::fdir`` respectively.
> > > > > +  This scheme will allow PMDs to avoid writes to ``rte_mbuf``
> > > > > + fields on Rx and  thereby improve Rx performance if application
> > wishes do so.
> > > > > +  In 19.11 PMDs will still update the fields even when the
> > > > > + offloads are not  enabled.
> > > > > +  The exact semantics of the flags will be worked out later
> > > > > + either by making  them negative offloads to avoid application
> > > > > + change or positive offload to  align with existing offload flag
> > semantics.
> > > > > --
> > > > > 2.17.1

Re: [dpdk-dev] [patch v3] doc: announce API change in ethdev offload flags

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Thursday, August 8, 2019 4:04 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Pavan Nikhilesh
> Bhagavatula <pbhagavatula@marvell.com>; stephen@networkplumber.org;
> arybchenko@solarflare.com; hemant.agrawal@nxp.com;
> thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>; Richardson,
> Bruce <bruce.richardson@intel.com>; Neil Horman
> <nhorman@tuxdriver.com>; Mcnamara, John <john.mcnamara@intel.com>;
> Kovacevic, Marko <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org
> Subject: [EXT] RE: [dpdk-dev] [patch v3] doc: announce API change in ethdev
> offload flags
> 
> External Email
> 
> ----------------------------------------------------------------------
> 
> 
> > -----Original Message-----
> > From: Jerin Jacob Kollanukkaran [mailto:jerinj@marvell.com]
> > Sent: Thursday, August 8, 2019 11:23 AM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Pavan
> > Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
> > stephen@networkplumber.org; arybchenko@solarflare.com;
> > hemant.agrawal@nxp.com; thomas@monjalon.net; Yigit, Ferruh
> > <ferruh.yigit@intel.com>; Richardson, Bruce
> > <bruce.richardson@intel.com>; Neil Horman <nhorman@tuxdriver.com>;
> > Mcnamara, John <john.mcnamara@intel.com>; Kovacevic, Marko
> > <marko.kovacevic@intel.com>
> > Cc: dev@dpdk.org
> > Subject: RE: [dpdk-dev] [patch v3] doc: announce API change in ethdev
> > offload flags
> >
> > > -----Original Message-----
> > > From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > > Sent: Thursday, August 8, 2019 3:39 PM
> > > To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Pavan Nikhilesh
> > > Bhagavatula <pbhagavatula@marvell.com>;
> stephen@networkplumber.org;
> > > arybchenko@solarflare.com; hemant.agrawal@nxp.com;
> > > thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>;
> > > Richardson, Bruce <bruce.richardson@intel.com>; Neil Horman
> > > <nhorman@tuxdriver.com>; Mcnamara, John
> <john.mcnamara@intel.com>;
> > > Kovacevic, Marko <marko.kovacevic@intel.com>
> > > Cc: dev@dpdk.org
> > > Subject: [EXT] RE: [dpdk-dev] [patch v3] doc: announce API change in
> > > ethdev offload flags
> > >
> > > External Email
> > >
> > > --------------------------------------------------------------------
> > > --
> > > Hi Jerin,
> >
> > Hi Konstantin,
> >
> >
> > >
> > > > >
> > > > > Hi guys,
> > > > >
> > > > > >
> > > > > > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > > > >
> > > > > > Add new offload flags ``DEV_RX_OFFLOAD_PTYPE``,
> > > > > ``DEV_RX_OFFLOAD_RSS``
> > > > > > and ``DEV_RX_OFFLOAD_FLOW_MARK``.
> > > > > >
> > > > > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > > > > Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > > > > > Acked-by: Jerin Jacob <jerinj@marvell.com>
> > > > > > ---
> > > > > >  v3 Changes:
> > > > > >  - DEV_RX_OFFLOAD_RSS -> DEV_RX_OFFLOAD_RSS_HASH
> (anndrew).
> > > > > >
> > > > > >  v2 Changes:
> > > > > >  - Reword for clarity.
> > > > > >
> > > > > >  doc/guides/rel_notes/deprecation.rst | 13 +++++++++++++
> > > > > >  1 file changed, 13 insertions(+)
> > > > > >
> > > > > > diff --git a/doc/guides/rel_notes/deprecation.rst
> > > > > > b/doc/guides/rel_notes/deprecation.rst
> > > > > > index 37b8592b6..056c5709f 100644
> > > > > > --- a/doc/guides/rel_notes/deprecation.rst
> > > > > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > > > > @@ -78,3 +78,16 @@ Deprecation Notices
> > > > > >    to set new power environment if power environment was
> > > > > > already
> > > > > initialized.
> > > > > >    In this case the function will return -1 unless the
> > > > > > environment is unset
> > > > > first
> > > > > >    (using ``rte_power_unset_env``). Other function usage
> > > > > > scenarios will not
> > > > > change.
> > > > > > +
> > > > > > +* ethdev: New offload flags ``DEV_RX_OFFLOAD_PTYPE``,
> > > > > > +``DEV_RX_OFFLOAD_RSS_HASH``
> > > > > > +  and ``DEV_RX_OFFLOAD_FLOW_MARK`` will be added in 19.11.
> > > > >
> > > > > One question about DEV_RX_OFFLOAD_PTYPE:
> > > > > Does it mean that new ol_flags value (PKT_RX_PTYPE) will be
> > > > > introduced to indicate that mbuf.packet_type value is set?
> > > > > Or PMD will have to set  mbuf.packet_type to zero, when
> > > > > DEV_RX_OFFLOAD_PTYPE was not enabled by user?
> > > >
> > > > I was thinking when DEV_RX_OFFLOAD_PTYPE is set
> > > > - mbuf.packet_type will be valid and mbuf.packet_type will have
> > > > parsed
> > > packet type.
> > > > If not set
> > > > - mbuf.packet_type can be anything application should not use
> > > mbuf.packet_type field.
> > >
> > > But in that case, we do need a new value for ol_flags, PKT_RX_PTYPE
> > > or so, right?
> >
> > Since application has two knobs rte_eth_dev_get_supported_ptypes() and
> > DEV_RX_OFFLOAD_PTYPE. We may not need to new ol_flags for this
> change. Right?
> > i.e if application sets the DEV_RX_OFFLOAD_PTYPE, The application will
> > get the parsed ptypes by the driver(=
> rte_eth_dev_get_supported_ptypes()).
> > So there is no scope ambiguity. Right?
> 
> I still think there is:
> Imagine user has 2 eth devices, one does support DEV_RX_OFFLOAD_PTYPE,
> second doesn't.  Now he has a mix of packets from both devices, that you
> want t process.
> How would he figure out for which of them ptype values are valid, and for
> each are not?
> Trace back from what port he has received them?
> Not very convenient, and not always possible.

I thought so. But in that case, application can always set DEV_RX_OFFLOAD_PTYPE
Flags for all the ethdev ports. Right? Rather having any complicated ol_flags
or port based parsing. If limit the _contract_ to following, we are good. Right?
# when DEV_RX_OFFLOAD_PTYPE is set, mbuf.packet_type will be valid
and mbuf.packet_type will have parsed packet type

or the negative offload(This contract is pretty clear, I don't think any ambiguity at all)
# when DEV_RX_OFFLOAD_NO_PTYPE(something similar) is set,
mbuf.packet_type will be invalid. 

> I think we need either to introduce new ol_flag value (as we usually do for
> other RX offloads),
> or force PMD to always set ptype value.

Setting new  ol_flag value may effect performance for existing drivers
which don't planning to use this offload and it complicates the 
application to have additional check based on ol_flag. If you see any corner case with above section,

How about just setting as ptype as 0 incase it is not parsed by driver.
Actual lookup is the costly one, writing 0 to pytpe is not costly
as there are plenty of writes in Rx and it will be write merged(No CPU stall)

I did not get the complete picture of "rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t
ptype_mask); instead of DEV_RX_OFFLOAD_PTYPE? scheme", Does it help?

> Konstantin
> 
> >
> >
> > >
> > > >
> > > > This will avoid writes 0 to mbuf.packet_type and packet_type
> > > > parsing if
> > > offload is not set.
> > > >
> > > >
> > > > > If so, what is the advantage?
> > > > > Again in that case, would it be more plausible to introduce something
> like:
> > > > > rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t
> > > > > ptype_mask); instead of DEV_RX_OFFLOAD_PTYPE?
> > > >
> > > > Any scheme is fine where we can skip the  write 0 to
> > > > mbuf.packet_type and packet_type parsing If application is NOT
> interested in packet_type.
> > > >
> > > > > Konstantin
> > > > >
> > > > > > +  This will allow application to enable or disable PMDs from
> > > > > > + updating ``rte_mbuf`` fields ``rte_mbuf::packet_type``,
> > > > > > + ``rte_mbuf::hash::rss`` and  ``rte_mbuf::hash::fdir`` respectively.
> > > > > > +  This scheme will allow PMDs to avoid writes to ``rte_mbuf``
> > > > > > + fields on Rx and  thereby improve Rx performance if
> > > > > > + application
> > > wishes do so.
> > > > > > +  In 19.11 PMDs will still update the fields even when the
> > > > > > + offloads are not  enabled.
> > > > > > +  The exact semantics of the flags will be worked out later
> > > > > > + either by making  them negative offloads to avoid
> > > > > > + application change or positive offload to  align with
> > > > > > + existing offload flag
> > > semantics.
> > > > > > --
> > > > > > 2.17.1

Re: [dpdk-dev] [patch v3] doc: announce API change in ethdev offload flags

[Thomas Monjalon]

08/08/2019 12:59, Jerin Jacob Kollanukkaran:
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > From: Jerin Jacob Kollanukkaran [mailto:jerinj@marvell.com]
> > > From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > > > > > > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > > > > One question about DEV_RX_OFFLOAD_PTYPE:
> > > > > > Does it mean that new ol_flags value (PKT_RX_PTYPE) will be
> > > > > > introduced to indicate that mbuf.packet_type value is set?
> > > > > > Or PMD will have to set  mbuf.packet_type to zero, when
> > > > > > DEV_RX_OFFLOAD_PTYPE was not enabled by user?
> > > > >
> > > > > I was thinking when DEV_RX_OFFLOAD_PTYPE is set
> > > > > - mbuf.packet_type will be valid and mbuf.packet_type will have
> > > > > parsed
> > > > packet type.
> > > > > If not set
> > > > > - mbuf.packet_type can be anything application should not use
> > > > mbuf.packet_type field.
> > > >
> > > > But in that case, we do need a new value for ol_flags, PKT_RX_PTYPE
> > > > or so, right?
> > >
> > > Since application has two knobs rte_eth_dev_get_supported_ptypes() and
> > > DEV_RX_OFFLOAD_PTYPE. We may not need to new ol_flags for this
> > change. Right?
> > > i.e if application sets the DEV_RX_OFFLOAD_PTYPE, The application will
> > > get the parsed ptypes by the driver(=
> > rte_eth_dev_get_supported_ptypes()).
> > > So there is no scope ambiguity. Right?
> > 
> > I still think there is:
> > Imagine user has 2 eth devices, one does support DEV_RX_OFFLOAD_PTYPE,
> > second doesn't.  Now he has a mix of packets from both devices, that you
> > want t process.
> > How would he figure out for which of them ptype values are valid, and for
> > each are not?
> > Trace back from what port he has received them?
> > Not very convenient, and not always possible.
> 
> I thought so. But in that case, application can always set DEV_RX_OFFLOAD_PTYPE
> Flags for all the ethdev ports. Right? Rather having any complicated ol_flags
> or port based parsing. If limit the _contract_ to following, we are good. Right?
> # when DEV_RX_OFFLOAD_PTYPE is set, mbuf.packet_type will be valid
> and mbuf.packet_type will have parsed packet type
> 
> or the negative offload(This contract is pretty clear, I don't think any ambiguity at all)
> # when DEV_RX_OFFLOAD_NO_PTYPE(something similar) is set,
> mbuf.packet_type will be invalid.

Just a note here: I am clearly against negative flags.
We recently cleaned up the flags to all be positive.

Re: [dpdk-dev] [patch v3] doc: announce API change in ethdev offload flags

[Ananyev, Konstantin]

> > > > > > > Add new offload flags ``DEV_RX_OFFLOAD_PTYPE``,
> > > > > > ``DEV_RX_OFFLOAD_RSS``
> > > > > > > and ``DEV_RX_OFFLOAD_FLOW_MARK``.
> > > > > > >
> > > > > > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > > > > > Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > > > > > > Acked-by: Jerin Jacob <jerinj@marvell.com>
> > > > > > > ---
> > > > > > >  v3 Changes:
> > > > > > >  - DEV_RX_OFFLOAD_RSS -> DEV_RX_OFFLOAD_RSS_HASH
> > (anndrew).
> > > > > > >
> > > > > > >  v2 Changes:
> > > > > > >  - Reword for clarity.
> > > > > > >
> > > > > > >  doc/guides/rel_notes/deprecation.rst | 13 +++++++++++++
> > > > > > >  1 file changed, 13 insertions(+)
> > > > > > >
> > > > > > > diff --git a/doc/guides/rel_notes/deprecation.rst
> > > > > > > b/doc/guides/rel_notes/deprecation.rst
> > > > > > > index 37b8592b6..056c5709f 100644
> > > > > > > --- a/doc/guides/rel_notes/deprecation.rst
> > > > > > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > > > > > @@ -78,3 +78,16 @@ Deprecation Notices
> > > > > > >    to set new power environment if power environment was
> > > > > > > already
> > > > > > initialized.
> > > > > > >    In this case the function will return -1 unless the
> > > > > > > environment is unset
> > > > > > first
> > > > > > >    (using ``rte_power_unset_env``). Other function usage
> > > > > > > scenarios will not
> > > > > > change.
> > > > > > > +
> > > > > > > +* ethdev: New offload flags ``DEV_RX_OFFLOAD_PTYPE``,
> > > > > > > +``DEV_RX_OFFLOAD_RSS_HASH``
> > > > > > > +  and ``DEV_RX_OFFLOAD_FLOW_MARK`` will be added in 19.11.
> > > > > >
> > > > > > One question about DEV_RX_OFFLOAD_PTYPE:
> > > > > > Does it mean that new ol_flags value (PKT_RX_PTYPE) will be
> > > > > > introduced to indicate that mbuf.packet_type value is set?
> > > > > > Or PMD will have to set  mbuf.packet_type to zero, when
> > > > > > DEV_RX_OFFLOAD_PTYPE was not enabled by user?
> > > > >
> > > > > I was thinking when DEV_RX_OFFLOAD_PTYPE is set
> > > > > - mbuf.packet_type will be valid and mbuf.packet_type will have
> > > > > parsed
> > > > packet type.
> > > > > If not set
> > > > > - mbuf.packet_type can be anything application should not use
> > > > mbuf.packet_type field.
> > > >
> > > > But in that case, we do need a new value for ol_flags, PKT_RX_PTYPE
> > > > or so, right?
> > >
> > > Since application has two knobs rte_eth_dev_get_supported_ptypes() and
> > > DEV_RX_OFFLOAD_PTYPE. We may not need to new ol_flags for this
> > change. Right?
> > > i.e if application sets the DEV_RX_OFFLOAD_PTYPE, The application will
> > > get the parsed ptypes by the driver(=
> > rte_eth_dev_get_supported_ptypes()).
> > > So there is no scope ambiguity. Right?
> >
> > I still think there is:
> > Imagine user has 2 eth devices, one does support DEV_RX_OFFLOAD_PTYPE,
> > second doesn't.  Now he has a mix of packets from both devices, that you
> > want t process.
> > How would he figure out for which of them ptype values are valid, and for
> > each are not?
> > Trace back from what port he has received them?
> > Not very convenient, and not always possible.
> 
> I thought so. But in that case, application can always set DEV_RX_OFFLOAD_PTYPE
> Flags for all the ethdev ports. Right? Rather having any complicated ol_flags
> or port based parsing. If limit the _contract_ to following, we are good. Right?
> # when DEV_RX_OFFLOAD_PTYPE is set, mbuf.packet_type will be valid
> and mbuf.packet_type will have parsed packet type

Yes sure in principle user can calculate smallest common subset of RX offloads
supported by all devs in the system and use only  them.
Then he can use some global value for ol_flags that will be setup at initialization time,
instead of checking ol_flags for every mbuf. 
Though inside DPDK we don't use that method for other offloads (cksum, vlan, rss).
Why we should do different here?
Again how to deal with hot-plugged devices with such approach?

> 
> or the negative offload(This contract is pretty clear, I don't think any ambiguity at all)
> # when DEV_RX_OFFLOAD_NO_PTYPE(something similar) is set,
> mbuf.packet_type will be invalid.
> 
> > I think we need either to introduce new ol_flag value (as we usually do for
> > other RX offloads),
> > or force PMD to always set ptype value.
> 
> Setting new  ol_flag value may effect performance for existing drivers
> which don't planning to use this offload

If the driver doesn't support DEV_RX_OFFLOAD_PTYPE, it wouldn't need to set anything
(neither ol_flags, neither packet_type).

> and it complicates the
> application to have additional check based on ol_flag. If you see any corner case with above section,
> 
> How about just setting as ptype as 0 incase it is not parsed by driver.

As I said above - ok by me.
AFAIK, this is current behavior, so no changes in PMD will be required.

> Actual lookup is the costly one, writing 0 to pytpe is not costly
> as there are plenty of writes in Rx and it will be write merged(No CPU stall)

Yes packet_type is at first 64B, so shouldn't cause any extra overhead.

> 
> I did not get the complete picture of "rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t
> ptype_mask); instead of DEV_RX_OFFLOAD_PTYPE? scheme", Does it help?

I thought about it as just a different way to disable(/limit) requested by user PTYPE support.
If let say user is not interested in ptype information at all, he can ask PMD to just 
always set ptype value to 0:
rte_eth_dev_set_supported_ptypes(port, RTE_PTYPE_UNKNOWN);

if he is interested just in L2/L3 layer info,
he can ask PMD to provide ptype information only for L2/L3:
rte_eth_dev_set_supported_ptypes(port, RTE_PTYPE_L2_MASK | RTE_PTYPE_L3_MASK);

Or to enable all supported by PMD ptypes: 
rte_eth_dev_set_supported_ptypes(port, UINT32_MAX)

Re: [dpdk-dev] [patch v3] doc: announce API change in ethdev offload flags

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Thursday, August 8, 2019 10:24 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Pavan Nikhilesh
> Bhagavatula <pbhagavatula@marvell.com>; stephen@networkplumber.org;
> arybchenko@solarflare.com; hemant.agrawal@nxp.com;
> thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>; Richardson,
> Bruce <bruce.richardson@intel.com>; Neil Horman
> <nhorman@tuxdriver.com>; Mcnamara, John <john.mcnamara@intel.com>;
> Kovacevic, Marko <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org
> Subject: [EXT] RE: [dpdk-dev] [patch v3] doc: announce API change in ethdev
> offload flags
> > > > Since application has two knobs rte_eth_dev_get_supported_ptypes()
> > > > and DEV_RX_OFFLOAD_PTYPE. We may not need to new ol_flags for
> this
> > > change. Right?
> > > > i.e if application sets the DEV_RX_OFFLOAD_PTYPE, The application
> > > > will get the parsed ptypes by the driver(=
> > > rte_eth_dev_get_supported_ptypes()).
> > > > So there is no scope ambiguity. Right?
> > >
> > > I still think there is:
> > > Imagine user has 2 eth devices, one does support
> > > DEV_RX_OFFLOAD_PTYPE, second doesn't.  Now he has a mix of packets
> > > from both devices, that you want t process.
> > > How would he figure out for which of them ptype values are valid,
> > > and for each are not?
> > > Trace back from what port he has received them?
> > > Not very convenient, and not always possible.
> >
> > I thought so. But in that case, application can always set
> > DEV_RX_OFFLOAD_PTYPE Flags for all the ethdev ports. Right? Rather
> > having any complicated ol_flags or port based parsing. If limit the
> _contract_ to following, we are good. Right?
> > # when DEV_RX_OFFLOAD_PTYPE is set, mbuf.packet_type will be valid
> and
> > mbuf.packet_type will have parsed packet type
> 
> Yes sure in principle user can calculate smallest common subset of RX
> offloads supported by all devs in the system and use only  them.
> Then he can use some global value for ol_flags that will be setup at
> initialization time, instead of checking ol_flags for every mbuf.
> Though inside DPDK we don't use that method for other offloads (cksum,
> vlan, rss).
> Why we should do different here?

I agree. We don't need to.

> Again how to deal with hot-plugged devices with such approach?
> 
> >
> > or the negative offload(This contract is pretty clear, I don't think
> > any ambiguity at all) # when DEV_RX_OFFLOAD_NO_PTYPE(something
> > similar) is set, mbuf.packet_type will be invalid.
> >
> > > I think we need either to introduce new ol_flag value (as we usually
> > > do for other RX offloads), or force PMD to always set ptype value.
> >
> > Setting new  ol_flag value may effect performance for existing drivers
> > which don't planning to use this offload
> 
> If the driver doesn't support DEV_RX_OFFLOAD_PTYPE, it wouldn't need to
> set anything (neither ol_flags, neither packet_type).

Yes

> 
> > and it complicates the
> > application to have additional check based on ol_flag. If you see any
> > corner case with above section,
> >
> > How about just setting as ptype as 0 incase it is not parsed by driver.
> 
> As I said above - ok by me.
> AFAIK, this is current behavior, so no changes in PMD will be required.
> 
> > Actual lookup is the costly one, writing 0 to pytpe is not costly as
> > there are plenty of writes in Rx and it will be write merged(No CPU
> > stall)
> 
> Yes packet_type is at first 64B, so shouldn't cause any extra overhead.
> 
> >
> > I did not get the complete picture of
> > "rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t
> ptype_mask); instead of DEV_RX_OFFLOAD_PTYPE? scheme", Does it help?
> 
> I thought about it as just a different way to disable(/limit) requested by user
> PTYPE support.
> If let say user is not interested in ptype information at all, he can ask PMD to
> just always set ptype value to 0:
> rte_eth_dev_set_supported_ptypes(port, RTE_PTYPE_UNKNOWN);
> 
> if he is interested just in L2/L3 layer info, he can ask PMD to provide ptype
> information only for L2/L3:
> rte_eth_dev_set_supported_ptypes(port, RTE_PTYPE_L2_MASK |
> RTE_PTYPE_L3_MASK);
> 
> Or to enable all supported by PMD ptypes:
> rte_eth_dev_set_supported_ptypes(port, UINT32_MAX)


The API looks good to me. We need to document the  rte_eth_dev_set_supported_ptypes()
must  be called when device is in stop state to allow PMD do slow path configuration.

To summarize:
Two options to control PTYPE lookup:
Option 1:
- If DEV_RX_OFFLOAD_PTYPE set, PMD returns mbuf->packet_type with rte_eth_dev_get_supported_ptypes()
- If DEV_RX_OFFLOAD_PTYPE is not set, PMD still can return  mbuf->packet_type with rte_eth_dev_get_supported_ptypes()
But if PMD can do some optimization, it can avoid ptype lookup and return mbuf->packet_type as zero.

Option 2:
- Introduce rte_eth_dev_set_supported_ptypes(port, needed_ptypes).

I think, rte_eth_dev_set_supported_ptypes() is better option As Konstantain suggested to
have selective control of ptype parsing by PMD at the cost of adding new API.

I think, we can avoid breaking exiting application by, If rte_eth_dev_set_supported_ptypes() is not called,
PMD must return mbuf->packet_type with rte_eth_dev_get_supported_ptypes().
If rte_eth_dev_set_supported_ptypes() and successful, PMD must return
mbuf->packet_type with rte_eth_dev_set_supported_ptypes()

If there no objection to this API, We can send updated deprecation notice.

> 

Re: [dpdk-dev] [patch v3] doc: announce API change in ethdev offload flags

[Ananyev, Konstantin]

Hi Jerin,

> > > > > Since application has two knobs rte_eth_dev_get_supported_ptypes()
> > > > > and DEV_RX_OFFLOAD_PTYPE. We may not need to new ol_flags for
> > this
> > > > change. Right?
> > > > > i.e if application sets the DEV_RX_OFFLOAD_PTYPE, The application
> > > > > will get the parsed ptypes by the driver(=
> > > > rte_eth_dev_get_supported_ptypes()).
> > > > > So there is no scope ambiguity. Right?
> > > >
> > > > I still think there is:
> > > > Imagine user has 2 eth devices, one does support
> > > > DEV_RX_OFFLOAD_PTYPE, second doesn't.  Now he has a mix of packets
> > > > from both devices, that you want t process.
> > > > How would he figure out for which of them ptype values are valid,
> > > > and for each are not?
> > > > Trace back from what port he has received them?
> > > > Not very convenient, and not always possible.
> > >
> > > I thought so. But in that case, application can always set
> > > DEV_RX_OFFLOAD_PTYPE Flags for all the ethdev ports. Right? Rather
> > > having any complicated ol_flags or port based parsing. If limit the
> > _contract_ to following, we are good. Right?
> > > # when DEV_RX_OFFLOAD_PTYPE is set, mbuf.packet_type will be valid
> > and
> > > mbuf.packet_type will have parsed packet type
> >
> > Yes sure in principle user can calculate smallest common subset of RX
> > offloads supported by all devs in the system and use only  them.
> > Then he can use some global value for ol_flags that will be setup at
> > initialization time, instead of checking ol_flags for every mbuf.
> > Though inside DPDK we don't use that method for other offloads (cksum,
> > vlan, rss).
> > Why we should do different here?
> 
> I agree. We don't need to.
> 
> > Again how to deal with hot-plugged devices with such approach?
> >
> > >
> > > or the negative offload(This contract is pretty clear, I don't think
> > > any ambiguity at all) # when DEV_RX_OFFLOAD_NO_PTYPE(something
> > > similar) is set, mbuf.packet_type will be invalid.
> > >
> > > > I think we need either to introduce new ol_flag value (as we usually
> > > > do for other RX offloads), or force PMD to always set ptype value.
> > >
> > > Setting new  ol_flag value may effect performance for existing drivers
> > > which don't planning to use this offload
> >
> > If the driver doesn't support DEV_RX_OFFLOAD_PTYPE, it wouldn't need to
> > set anything (neither ol_flags, neither packet_type).
> 
> Yes
> 
> >
> > > and it complicates the
> > > application to have additional check based on ol_flag. If you see any
> > > corner case with above section,
> > >
> > > How about just setting as ptype as 0 incase it is not parsed by driver.
> >
> > As I said above - ok by me.
> > AFAIK, this is current behavior, so no changes in PMD will be required.
> >
> > > Actual lookup is the costly one, writing 0 to pytpe is not costly as
> > > there are plenty of writes in Rx and it will be write merged(No CPU
> > > stall)
> >
> > Yes packet_type is at first 64B, so shouldn't cause any extra overhead.
> >
> > >
> > > I did not get the complete picture of
> > > "rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t
> > ptype_mask); instead of DEV_RX_OFFLOAD_PTYPE? scheme", Does it help?
> >
> > I thought about it as just a different way to disable(/limit) requested by user
> > PTYPE support.
> > If let say user is not interested in ptype information at all, he can ask PMD to
> > just always set ptype value to 0:
> > rte_eth_dev_set_supported_ptypes(port, RTE_PTYPE_UNKNOWN);
> >
> > if he is interested just in L2/L3 layer info, he can ask PMD to provide ptype
> > information only for L2/L3:
> > rte_eth_dev_set_supported_ptypes(port, RTE_PTYPE_L2_MASK |
> > RTE_PTYPE_L3_MASK);
> >
> > Or to enable all supported by PMD ptypes:
> > rte_eth_dev_set_supported_ptypes(port, UINT32_MAX)
> 
> 
> The API looks good to me. We need to document the  rte_eth_dev_set_supported_ptypes()
> must  be called when device is in stop state to allow PMD do slow path configuration.
> 
> To summarize:
> Two options to control PTYPE lookup:
> Option 1:
> - If DEV_RX_OFFLOAD_PTYPE set, PMD returns mbuf->packet_type with rte_eth_dev_get_supported_ptypes()
> - If DEV_RX_OFFLOAD_PTYPE is not set, PMD still can return  mbuf->packet_type with rte_eth_dev_get_supported_ptypes()
> But if PMD can do some optimization, it can avoid ptype lookup and return mbuf->packet_type as zero.
> 
> Option 2:
> - Introduce rte_eth_dev_set_supported_ptypes(port, needed_ptypes).

Yes.

> 
> I think, rte_eth_dev_set_supported_ptypes() is better option As Konstantain suggested to
> have selective control of ptype parsing by PMD at the cost of adding new API.
> 
> I think, we can avoid breaking exiting application by, If rte_eth_dev_set_supported_ptypes() is not called,
> PMD must return mbuf->packet_type with rte_eth_dev_get_supported_ptypes().
> If rte_eth_dev_set_supported_ptypes() and successful, PMD must return
> mbuf->packet_type with rte_eth_dev_set_supported_ptypes()

+1

> 
> If there no objection to this API, We can send updated deprecation notice.
> 

None from me.
Konstantin

Re: [dpdk-dev] [patch v3] doc: announce API change in ethdev offload flags

[Hemant Agrawal]

> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> 
> Add new offload flags ``DEV_RX_OFFLOAD_PTYPE``,
> ``DEV_RX_OFFLOAD_RSS`` and ``DEV_RX_OFFLOAD_FLOW_MARK``.
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> Acked-by: Jerin Jacob <jerinj@marvell.com>

Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>

[dpdk-dev] [patch v4] doc: announce API change in ethdev offload flags

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Add new offload flags ``DEV_RX_OFFLOAD_RSS`` and ``DEV_RX_OFFLOAD_FLOW_MARK``.
Add new function ``rte_eth_dev_set_supported_ptypes`` to allow application to
set specific ptypes to be updated in ``rte_mbuf::packet_type``

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 doc/guides/rel_notes/deprecation.rst | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 37b8592b6..e4e2a85d7 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -78,3 +78,26 @@ Deprecation Notices
   to set new power environment if power environment was already initialized.
   In this case the function will return -1 unless the environment is unset first
   (using ``rte_power_unset_env``). Other function usage scenarios will not change.
+
+* ethdev: New offload flags ``DEV_RX_OFFLOAD_RSS_HASH`` and ``DEV_RX_OFFLOAD_FLOW_MARK``
+  will be added in 19.11.
+  This will allow application to enable or disable PMDs from updating
+  ``rte_mbuf::hash::rss`` and ``rte_mbuf::hash::fdir`` respectively.
+  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields on Rx and
+  thereby improve Rx performance if application wishes do so.
+  In 19.11 PMDs will still update the fields even when the offloads are not
+  enabled.
+
+* ethdev: New function ``rte_eth_dev_set_supported_ptypes`` will be added in
+  19.11.
+  This will allow application to request PMD to set specific ptypes defined
+  through ``rte_eth_dev_set_supported_ptypes`` in ``rte_mbuf::packet_type``.
+  If application doesn't want any ptype information it can call
+  ``rte_eth_dev_set_supported_ptypes(ethdev_id, RTE_PTYPE_UNKNOWN)``
+  If application doesn't call ``rte_eth_dev_set_supported_ptypes`` PMD can
+  return ``rte_mbuf::packet_type`` with ``rte_eth_dev_get_supported_ptypes``.
+  If application is interested only in L2/L3 layer, it can inform the PMD to update
+ ``rte_mbuf::packet_type`` with L2/L3 ptype by calling
+ ``rte_eth_dev_set_supported_ptypes(ethdev_id, RTE_PTYPE_L2_MASK | RTE_PTYPE_L3_MASK)``.
+  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields on Rx and
+  thereby improve Rx performance if application wishes do so.
--
2.17.1

Re: [dpdk-dev] [patch v4] doc: announce API change in ethdev offload flags

[Ananyev, Konstantin]

Hi 

> 
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> 
> Add new offload flags ``DEV_RX_OFFLOAD_RSS`` and ``DEV_RX_OFFLOAD_FLOW_MARK``.
> Add new function ``rte_eth_dev_set_supported_ptypes`` to allow application to
> set specific ptypes to be updated in ``rte_mbuf::packet_type``
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> Acked-by: Jerin Jacob <jerinj@marvell.com>
> Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
>  doc/guides/rel_notes/deprecation.rst | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 37b8592b6..e4e2a85d7 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -78,3 +78,26 @@ Deprecation Notices
>    to set new power environment if power environment was already initialized.
>    In this case the function will return -1 unless the environment is unset first
>    (using ``rte_power_unset_env``). Other function usage scenarios will not change.
> +
> +* ethdev: New offload flags ``DEV_RX_OFFLOAD_RSS_HASH`` and ``DEV_RX_OFFLOAD_FLOW_MARK``
> +  will be added in 19.11.
> +  This will allow application to enable or disable PMDs from updating
> +  ``rte_mbuf::hash::rss`` and ``rte_mbuf::hash::fdir`` respectively.
> +  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields on Rx and
> +  thereby improve Rx performance if application wishes do so.
> +  In 19.11 PMDs will still update the fields even when the offloads are not
> +  enabled.
> +
> +* ethdev: New function ``rte_eth_dev_set_supported_ptypes`` will be added in
> +  19.11.

As I said in my previous mail, I do support the idea, though few nits below.

> +  This will allow application to request PMD to set specific ptypes defined
> +  through ``rte_eth_dev_set_supported_ptypes`` in ``rte_mbuf::packet_type``.
> +  If application doesn't want any ptype information it can call
> +  ``rte_eth_dev_set_supported_ptypes(ethdev_id, RTE_PTYPE_UNKNOWN)``

Probably worth to mention that in that case packet_type will be set to 0
(PMD still need to set the value).

> +  If application doesn't call ``rte_eth_dev_set_supported_ptypes`` PMD can
> +  return ``rte_mbuf::packet_type`` with ``rte_eth_dev_get_supported_ptypes``.
> +  If application is interested only in L2/L3 layer, it can inform the PMD to update
> + ``rte_mbuf::packet_type`` with L2/L3 ptype by calling
> + ``rte_eth_dev_set_supported_ptypes(ethdev_id, RTE_PTYPE_L2_MASK | RTE_PTYPE_L3_MASK)``.
> +  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields on Rx and

As I understand, PMD will still need to write ptype value.
Though it will help PMD to avoid figuring out ptype values, when user don't need that information.

> +  thereby improve Rx performance if application wishes do so.
> --
> 2.17.1

Re: [dpdk-dev] [patch v4] doc: announce API change in ethdev offload flags

[Pavan Nikhilesh Bhagavatula]

Hi,

>-----Original Message-----
>From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
>Sent: Friday, August 9, 2019 2:17 PM
>To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Jerin
>Jacob Kollanukkaran <jerinj@marvell.com>;
>stephen@networkplumber.org; arybchenko@solarflare.com;
>hemant.agrawal@nxp.com; thomas@monjalon.net; Yigit, Ferruh
><ferruh.yigit@intel.com>; Richardson, Bruce
><bruce.richardson@intel.com>; Neil Horman
><nhorman@tuxdriver.com>; Mcnamara, John
><john.mcnamara@intel.com>; Kovacevic, Marko
><marko.kovacevic@intel.com>
>Cc: dev@dpdk.org
>Subject: RE: [dpdk-dev] [patch v4] doc: announce API change in
>ethdev offload flags
>Hi
>
>>
>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>
>> Add new offload flags ``DEV_RX_OFFLOAD_RSS`` and
>``DEV_RX_OFFLOAD_FLOW_MARK``.
>> Add new function ``rte_eth_dev_set_supported_ptypes`` to allow
>application to
>> set specific ptypes to be updated in ``rte_mbuf::packet_type``
>>
>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> Acked-by: Jerin Jacob <jerinj@marvell.com>
>> Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> ---
>>  doc/guides/rel_notes/deprecation.rst | 23
>+++++++++++++++++++++++
>>  1 file changed, 23 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/deprecation.rst
>b/doc/guides/rel_notes/deprecation.rst
>> index 37b8592b6..e4e2a85d7 100644
>> --- a/doc/guides/rel_notes/deprecation.rst
>> +++ b/doc/guides/rel_notes/deprecation.rst
>> @@ -78,3 +78,26 @@ Deprecation Notices
>>    to set new power environment if power environment was already
>initialized.
>>    In this case the function will return -1 unless the environment is
>unset first
>>    (using ``rte_power_unset_env``). Other function usage scenarios
>will not change.
>> +
>> +* ethdev: New offload flags ``DEV_RX_OFFLOAD_RSS_HASH`` and
>``DEV_RX_OFFLOAD_FLOW_MARK``
>> +  will be added in 19.11.
>> +  This will allow application to enable or disable PMDs from updating
>> +  ``rte_mbuf::hash::rss`` and ``rte_mbuf::hash::fdir`` respectively.
>> +  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields
>on Rx and
>> +  thereby improve Rx performance if application wishes do so.
>> +  In 19.11 PMDs will still update the fields even when the offloads are
>not
>> +  enabled.
>> +
>> +* ethdev: New function ``rte_eth_dev_set_supported_ptypes`` will
>be added in
>> +  19.11.
>
>As I said in my previous mail, I do support the idea, though few nits
>below.
>
>> +  This will allow application to request PMD to set specific ptypes
>defined
>> +  through ``rte_eth_dev_set_supported_ptypes`` in
>``rte_mbuf::packet_type``.
>> +  If application doesn't want any ptype information it can call
>> +  ``rte_eth_dev_set_supported_ptypes(ethdev_id,
>RTE_PTYPE_UNKNOWN)``
>
>Probably worth to mention that in that case packet_type will be set to 0
>(PMD still need to set the value).

Ack, will update in v5.

>
>> +  If application doesn't call ``rte_eth_dev_set_supported_ptypes``
>PMD can
>> +  return ``rte_mbuf::packet_type`` with
>``rte_eth_dev_get_supported_ptypes``.
>> +  If application is interested only in L2/L3 layer, it can inform the PMD
>to update
>> + ``rte_mbuf::packet_type`` with L2/L3 ptype by calling
>> + ``rte_eth_dev_set_supported_ptypes(ethdev_id,
>RTE_PTYPE_L2_MASK | RTE_PTYPE_L3_MASK)``.
>> +  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields
>on Rx and
>
>As I understand, PMD will still need to write ptype value.
>Though it will help PMD to avoid figuring out ptype values, when user
>don't need that information.

I will reword to ``will allow PMDs to avoid lookup to internal ptype table``.

>
>> +  thereby improve Rx performance if application wishes do so.
>> --
>> 2.17.1

Thanks,
Pavan.

Re: [dpdk-dev] [patch v4] doc: announce API change in ethdev offload flags

[Tom Barbette]

I think the silent breaking is still not solved for 
DEV_RX_OFFLOAD_RSS_HASH and DEV_RX_OFFLOAD_FLOW_MARK.

An old application will still compile without any problem, but the RSS 
hash will not be written and the app will break... They should be 
negative. Eg DEV_RX_OFFLOAD_NO_RSS_HASH and DEV_RX_OFFLOAD_NO_FLOW_MARK?

Then, regarding the idea, are we sure it's better to add a configuration 
check/a branch than always copying a few bytes from a warm cache line to 
a warm cache line? Or some HW could free some internal resources? But 
drivers are free to ignore it so it is not a problem as opposed to 
silent breaking.

Tom

On 2019-08-09 10:17, pbhagavatula@marvell.com wrote:
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> 
> Add new offload flags ``DEV_RX_OFFLOAD_RSS`` and ``DEV_RX_OFFLOAD_FLOW_MARK``.
> Add new function ``rte_eth_dev_set_supported_ptypes`` to allow application to
> set specific ptypes to be updated in ``rte_mbuf::packet_type``
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> Acked-by: Jerin Jacob <jerinj@marvell.com>
> Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
>   doc/guides/rel_notes/deprecation.rst | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 37b8592b6..e4e2a85d7 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -78,3 +78,26 @@ Deprecation Notices
>     to set new power environment if power environment was already initialized.
>     In this case the function will return -1 unless the environment is unset first
>     (using ``rte_power_unset_env``). Other function usage scenarios will not change.
> +
> +* ethdev: New offload flags ``DEV_RX_OFFLOAD_RSS_HASH`` and ``DEV_RX_OFFLOAD_FLOW_MARK``
> +  will be added in 19.11.
> +  This will allow application to enable or disable PMDs from updating
> +  ``rte_mbuf::hash::rss`` and ``rte_mbuf::hash::fdir`` respectively.
> +  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields on Rx and
> +  thereby improve Rx performance if application wishes do so.
> +  In 19.11 PMDs will still update the fields even when the offloads are not
> +  enabled.
> +
> +* ethdev: New function ``rte_eth_dev_set_supported_ptypes`` will be added in
> +  19.11.
> +  This will allow application to request PMD to set specific ptypes defined
> +  through ``rte_eth_dev_set_supported_ptypes`` in ``rte_mbuf::packet_type``.
> +  If application doesn't want any ptype information it can call
> +  ``rte_eth_dev_set_supported_ptypes(ethdev_id, RTE_PTYPE_UNKNOWN)``
> +  If application doesn't call ``rte_eth_dev_set_supported_ptypes`` PMD can
> +  return ``rte_mbuf::packet_type`` with ``rte_eth_dev_get_supported_ptypes``.
> +  If application is interested only in L2/L3 layer, it can inform the PMD to update
> + ``rte_mbuf::packet_type`` with L2/L3 ptype by calling
> + ``rte_eth_dev_set_supported_ptypes(ethdev_id, RTE_PTYPE_L2_MASK | RTE_PTYPE_L3_MASK)``.
> +  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields on Rx and
> +  thereby improve Rx performance if application wishes do so.
> --
> 2.17.1
> 

Re: [dpdk-dev] [patch v4] doc: announce API change in ethdev offload flags

[Andrew Rybchenko]

On 8/9/19 12:13 PM, Tom Barbette wrote:
> I think the silent breaking is still not solved for 
> DEV_RX_OFFLOAD_RSS_HASH and DEV_RX_OFFLOAD_FLOW_MARK.
>
> An old application will still compile without any problem, but the RSS 
> hash will not be written and the app will break... They should be 
> negative. Eg DEV_RX_OFFLOAD_NO_RSS_HASH and DEV_RX_OFFLOAD_NO_FLOW_MARK?
>
> Then, regarding the idea, are we sure it's better to add a 
> configuration check/a branch than always copying a few bytes from a 
> warm cache line to a warm cache line? Or some HW could free some 
> internal resources? But drivers are free to ignore it so it is not a 
> problem as opposed to silent breaking.

It could be not delivered from NIC to host saving PCIe bandwidth. E.g. 
flow mark plus RSS hash is 8 byte and it is more than 10% for small packet.

> Tom
>
> On 2019-08-09 10:17, pbhagavatula@marvell.com wrote:
>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>
>> Add new offload flags ``DEV_RX_OFFLOAD_RSS`` and 
>> ``DEV_RX_OFFLOAD_FLOW_MARK``.
>> Add new function ``rte_eth_dev_set_supported_ptypes`` to allow 
>> application to
>> set specific ptypes to be updated in ``rte_mbuf::packet_type``
>>
>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> Acked-by: Jerin Jacob <jerinj@marvell.com>
>> Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> ---
>>   doc/guides/rel_notes/deprecation.rst | 23 +++++++++++++++++++++++
>>   1 file changed, 23 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/deprecation.rst 
>> b/doc/guides/rel_notes/deprecation.rst
>> index 37b8592b6..e4e2a85d7 100644
>> --- a/doc/guides/rel_notes/deprecation.rst
>> +++ b/doc/guides/rel_notes/deprecation.rst
>> @@ -78,3 +78,26 @@ Deprecation Notices
>>     to set new power environment if power environment was already 
>> initialized.
>>     In this case the function will return -1 unless the environment 
>> is unset first
>>     (using ``rte_power_unset_env``). Other function usage scenarios 
>> will not change.
>> +
>> +* ethdev: New offload flags ``DEV_RX_OFFLOAD_RSS_HASH`` and 
>> ``DEV_RX_OFFLOAD_FLOW_MARK``
>> +  will be added in 19.11.
>> +  This will allow application to enable or disable PMDs from updating
>> +  ``rte_mbuf::hash::rss`` and ``rte_mbuf::hash::fdir`` respectively.
>> +  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields 
>> on Rx and
>> +  thereby improve Rx performance if application wishes do so.
>> +  In 19.11 PMDs will still update the fields even when the offloads 
>> are not
>> +  enabled.
>> +
>> +* ethdev: New function ``rte_eth_dev_set_supported_ptypes`` will be 
>> added in
>> +  19.11.
>> +  This will allow application to request PMD to set specific ptypes 
>> defined
>> +  through ``rte_eth_dev_set_supported_ptypes`` in 
>> ``rte_mbuf::packet_type``.
>> +  If application doesn't want any ptype information it can call
>> +  ``rte_eth_dev_set_supported_ptypes(ethdev_id, RTE_PTYPE_UNKNOWN)``
>> +  If application doesn't call ``rte_eth_dev_set_supported_ptypes`` 
>> PMD can
>> +  return ``rte_mbuf::packet_type`` with 
>> ``rte_eth_dev_get_supported_ptypes``.
>> +  If application is interested only in L2/L3 layer, it can inform 
>> the PMD to update
>> + ``rte_mbuf::packet_type`` with L2/L3 ptype by calling
>> + ``rte_eth_dev_set_supported_ptypes(ethdev_id, RTE_PTYPE_L2_MASK | 
>> RTE_PTYPE_L3_MASK)``.
>> +  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields 
>> on Rx and
>> +  thereby improve Rx performance if application wishes do so.
>> -- 
>> 2.17.1
>>

Re: [dpdk-dev] [patch v4] doc: announce API change in ethdev offload flags

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Tom Barbette <barbette@kth.se>
> Sent: Friday, August 9, 2019 2:44 PM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Jerin Jacob
> Kollanukkaran <jerinj@marvell.com>; stephen@networkplumber.org;
> arybchenko@solarflare.com; hemant.agrawal@nxp.com;
> thomas@monjalon.net; ferruh.yigit@intel.com;
> bruce.richardson@intel.com; Neil Horman <nhorman@tuxdriver.com>; John
> McNamara <john.mcnamara@intel.com>; Marko Kovacevic
> <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [patch v4] doc: announce API change in ethdev
> offload flags
> 
> I think the silent breaking is still not solved for DEV_RX_OFFLOAD_RSS_HASH
> and DEV_RX_OFFLOAD_FLOW_MARK.

Unlike ptype, presence of  rss hash and flow_mark will be marked in ol_flags
as PKT_RX_RSS_HASH  and PKT_RX_FDIR_ID. So NO application contract breakage.

> 
> An old application will still compile without any problem, but the RSS hash will
> not be written and the app will break... They should be negative. Eg
> DEV_RX_OFFLOAD_NO_RSS_HASH and
> DEV_RX_OFFLOAD_NO_FLOW_MARK?
> 
> Then, regarding the idea, are we sure it's better to add a configuration
> check/a branch than always copying a few bytes from a warm cache line to a
> warm cache line? Or some HW could free some internal resources? But
> drivers are free to ignore it so it is not a problem as opposed to silent
> breaking.
> 
> Tom
> 
> On 2019-08-09 10:17, pbhagavatula@marvell.com wrote:
> > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >
> > Add new offload flags ``DEV_RX_OFFLOAD_RSS`` and
> ``DEV_RX_OFFLOAD_FLOW_MARK``.
> > Add new function ``rte_eth_dev_set_supported_ptypes`` to allow
> application to
> > set specific ptypes to be updated in ``rte_mbuf::packet_type``
> >
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > Acked-by: Jerin Jacob <jerinj@marvell.com>
> > Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> > ---
> >   doc/guides/rel_notes/deprecation.rst | 23 +++++++++++++++++++++++
> >   1 file changed, 23 insertions(+)
> >
> > diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> > index 37b8592b6..e4e2a85d7 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -78,3 +78,26 @@ Deprecation Notices
> >     to set new power environment if power environment was already
> initialized.
> >     In this case the function will return -1 unless the environment is unset
> first
> >     (using ``rte_power_unset_env``). Other function usage scenarios will not
> change.
> > +
> > +* ethdev: New offload flags ``DEV_RX_OFFLOAD_RSS_HASH`` and
> ``DEV_RX_OFFLOAD_FLOW_MARK``
> > +  will be added in 19.11.
> > +  This will allow application to enable or disable PMDs from updating
> > +  ``rte_mbuf::hash::rss`` and ``rte_mbuf::hash::fdir`` respectively.
> > +  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields on Rx
> and
> > +  thereby improve Rx performance if application wishes do so.
> > +  In 19.11 PMDs will still update the fields even when the offloads are not
> > +  enabled.
> > +
> > +* ethdev: New function ``rte_eth_dev_set_supported_ptypes`` will be
> added in
> > +  19.11.
> > +  This will allow application to request PMD to set specific ptypes defined
> > +  through ``rte_eth_dev_set_supported_ptypes`` in
> ``rte_mbuf::packet_type``.
> > +  If application doesn't want any ptype information it can call
> > +  ``rte_eth_dev_set_supported_ptypes(ethdev_id,
> RTE_PTYPE_UNKNOWN)``
> > +  If application doesn't call ``rte_eth_dev_set_supported_ptypes`` PMD
> can
> > +  return ``rte_mbuf::packet_type`` with
> ``rte_eth_dev_get_supported_ptypes``.
> > +  If application is interested only in L2/L3 layer, it can inform the PMD to
> update
> > + ``rte_mbuf::packet_type`` with L2/L3 ptype by calling
> > + ``rte_eth_dev_set_supported_ptypes(ethdev_id, RTE_PTYPE_L2_MASK
> | RTE_PTYPE_L3_MASK)``.
> > +  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields on Rx
> and
> > +  thereby improve Rx performance if application wishes do so.
> > --
> > 2.17.1
> >

[dpdk-dev] [patch v5] doc: announce API change in ethdev offload flags

[pbhagavatula]

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Add new offload flags ``DEV_RX_OFFLOAD_RSS_HASH`` and
``DEV_RX_OFFLOAD_FLOW_MARK``.
Add new function ``rte_eth_dev_set_supported_ptypes`` to allow application
to set specific ptypes to be updated in ``rte_mbuf::packet_type``

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 v5 Changes:
 - Reword for clarity.

 v4 Changes:
 - Remove DEV_RX_OFFLOAD_PTYPE proposition and replace it with
 rte_eth_dev_set_supported_ptypes (Konstantin).

 v3 Changes:
 - DEV_RX_OFFLOAD_RSS -> DEV_RX_OFFLOAD_RSS_HASH (andrew).

 v2 Changes:
 - Reword for clarity.

 doc/guides/rel_notes/deprecation.rst | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 37b8592b6..b0391366c 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -78,3 +78,27 @@ Deprecation Notices
   to set new power environment if power environment was already initialized.
   In this case the function will return -1 unless the environment is unset first
   (using ``rte_power_unset_env``). Other function usage scenarios will not change.
+
+* ethdev: New offload flags ``DEV_RX_OFFLOAD_RSS_HASH`` and ``DEV_RX_OFFLOAD_FLOW_MARK``
+  will be added in 19.11.
+  This will allow application to enable or disable PMDs from updating
+  ``rte_mbuf::hash::rss`` and ``rte_mbuf::hash::fdir`` respectively.
+  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields on Rx and
+  thereby improve Rx performance if application wishes do so.
+  In 19.11 PMDs will still update the fields even when the offloads are not
+  enabled.
+
+* ethdev: New function ``rte_eth_dev_set_supported_ptypes`` will be added in
+  19.11.
+  This will allow application to request PMD to set specific ptypes defined
+  through ``rte_eth_dev_set_supported_ptypes`` in ``rte_mbuf::packet_type``.
+  If application doesn't want any ptype information it can call
+  ``rte_eth_dev_set_supported_ptypes(ethdev_id, RTE_PTYPE_UNKNOWN)`` and PMD
+  will set ``rte_mbuf::packet_type`` to ``0``.
+  If application doesn't call ``rte_eth_dev_set_supported_ptypes`` PMD can
+  return ``rte_mbuf::packet_type`` with ``rte_eth_dev_get_supported_ptypes``.
+  If application is interested only in L2/L3 layer, it can inform the PMD to update
+ ``rte_mbuf::packet_type`` with L2/L3 ptype by calling
+ ``rte_eth_dev_set_supported_ptypes(ethdev_id, RTE_PTYPE_L2_MASK | RTE_PTYPE_L3_MASK)``.
+  This scheme will allow PMDs to avoid lookup to internal ptype table on Rx and
+  thereby improve Rx performance if application wishes do so.
--
2.17.1

Re: [dpdk-dev] [patch v5] doc: announce API change in ethdev offload flags

[Ananyev, Konstantin]

> Add new offload flags ``DEV_RX_OFFLOAD_RSS_HASH`` and
> ``DEV_RX_OFFLOAD_FLOW_MARK``.
> Add new function ``rte_eth_dev_set_supported_ptypes`` to allow application
> to set specific ptypes to be updated in ``rte_mbuf::packet_type``
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> Acked-by: Jerin Jacob <jerinj@marvell.com>
> Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
>  v5 Changes:
>  - Reword for clarity.
> 
>  v4 Changes:
>  - Remove DEV_RX_OFFLOAD_PTYPE proposition and replace it with
>  rte_eth_dev_set_supported_ptypes (Konstantin).
> 
>  v3 Changes:
>  - DEV_RX_OFFLOAD_RSS -> DEV_RX_OFFLOAD_RSS_HASH (andrew).
> 
>  v2 Changes:
>  - Reword for clarity.
> 
>  doc/guides/rel_notes/deprecation.rst | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 37b8592b6..b0391366c 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -78,3 +78,27 @@ Deprecation Notices
>    to set new power environment if power environment was already initialized.
>    In this case the function will return -1 unless the environment is unset first
>    (using ``rte_power_unset_env``). Other function usage scenarios will not change.
> +
> +* ethdev: New offload flags ``DEV_RX_OFFLOAD_RSS_HASH`` and ``DEV_RX_OFFLOAD_FLOW_MARK``
> +  will be added in 19.11.
> +  This will allow application to enable or disable PMDs from updating
> +  ``rte_mbuf::hash::rss`` and ``rte_mbuf::hash::fdir`` respectively.
> +  This scheme will allow PMDs to avoid writes to ``rte_mbuf`` fields on Rx and
> +  thereby improve Rx performance if application wishes do so.
> +  In 19.11 PMDs will still update the fields even when the offloads are not
> +  enabled.
> +
> +* ethdev: New function ``rte_eth_dev_set_supported_ptypes`` will be added in
> +  19.11.
> +  This will allow application to request PMD to set specific ptypes defined
> +  through ``rte_eth_dev_set_supported_ptypes`` in ``rte_mbuf::packet_type``.
> +  If application doesn't want any ptype information it can call
> +  ``rte_eth_dev_set_supported_ptypes(ethdev_id, RTE_PTYPE_UNKNOWN)`` and PMD
> +  will set ``rte_mbuf::packet_type`` to ``0``.
> +  If application doesn't call ``rte_eth_dev_set_supported_ptypes`` PMD can
> +  return ``rte_mbuf::packet_type`` with ``rte_eth_dev_get_supported_ptypes``.
> +  If application is interested only in L2/L3 layer, it can inform the PMD to update
> + ``rte_mbuf::packet_type`` with L2/L3 ptype by calling
> + ``rte_eth_dev_set_supported_ptypes(ethdev_id, RTE_PTYPE_L2_MASK | RTE_PTYPE_L3_MASK)``.
> +  This scheme will allow PMDs to avoid lookup to internal ptype table on Rx and
> +  thereby improve Rx performance if application wishes do so.
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 2.17.1

Re: [dpdk-dev] [patch v5] doc: announce API change in ethdev offload flags

[Thomas Monjalon]

> > Add new offload flags ``DEV_RX_OFFLOAD_RSS_HASH`` and
> > ``DEV_RX_OFFLOAD_FLOW_MARK``.
> > Add new function ``rte_eth_dev_set_supported_ptypes`` to allow application
> > to set specific ptypes to be updated in ``rte_mbuf::packet_type``
> > 
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > Acked-by: Jerin Jacob <jerinj@marvell.com>
> > Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

We'll probably have to discuss more the details of these new APIs,
but the overall idea looks good.
I am not sure there is any API or ABI breakage here,
so the announce may not be required.

Anyway applied, thanks.