doc: announce change in ETH item struct
diff mbox series

Message ID 50e5d2139368c54b414379b18a59ed46c12893f5.1596556981.git.dekelp@mellanox.com
State Superseded, archived
Delegated to: Thomas Monjalon
Headers show
Series
  • doc: announce change in ETH item struct
Related show

Checks

Context Check Description
ci/Intel-compilation fail apply issues
ci/checkpatch success coding style OK

Commit Message

Dekel Peled Aug. 4, 2020, 4:01 p.m. UTC
Struct rte_flow_item_eth will be modified to include additional
values, indicating existence or absence of VLAN headers following
the ETH header, as proposed in RFC
https://mails.dpdk.org/archives/dev/2020-August/177349.html.
Because of ABI break this change is proposed for 20.11.

Signed-off-by: Dekel Peled <dekelp@mellanox.com>
---
 doc/guides/rel_notes/deprecation.rst | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Ajit Khaparde Aug. 5, 2020, 3:39 a.m. UTC | #1
On Tue, Aug 4, 2020 at 9:07 AM Dekel Peled <dekelp@mellanox.com> wrote:

> Struct rte_flow_item_eth will be modified to include additional
> values, indicating existence or absence of VLAN headers following
> the ETH header, as proposed in RFC
> https://mails.dpdk.org/archives/dev/2020-August/177349.html.
> Because of ABI break this change is proposed for 20.11.
>
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>


> ---
>  doc/guides/rel_notes/deprecation.rst | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index 5201142..6241709 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -115,6 +115,11 @@ Deprecation Notices
>    following the IPv6 header, as proposed in RFC
>    https://mails.dpdk.org/archives/dev/2020-August/177257.html.
>
> +* ethdev: The ``struct rte_flow_item_eth`` struct will be modified to
> include
> +  additional values, indicating existence or absence of VLAN headers
> +  following the ETH header, as proposed in RFC
> +  https://mails.dpdk.org/archives/dev/2020-August/177349.html.
> +
>  * traffic manager: All traffic manager API's in ``rte_tm.h`` were
> mistakenly made
>    ABI stable in the v19.11 release. The TM maintainer and other
> contributors have
>    agreed to keep the TM APIs as experimental in expectation of additional
> spec
> --
> 1.8.3.1
>
>
Andrew Rybchenko Aug. 5, 2020, 10:49 a.m. UTC | #2
On 8/4/20 7:01 PM, Dekel Peled wrote:
> Struct rte_flow_item_eth will be modified to include additional
> values, indicating existence or absence of VLAN headers following
> the ETH header, as proposed in RFC
> https://mails.dpdk.org/archives/dev/2020-August/177349.html.
> Because of ABI break this change is proposed for 20.11.
> 
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> ---
>  doc/guides/rel_notes/deprecation.rst | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 5201142..6241709 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -115,6 +115,11 @@ Deprecation Notices
>    following the IPv6 header, as proposed in RFC
>    https://mails.dpdk.org/archives/dev/2020-August/177257.html.
>  
> +* ethdev: The ``struct rte_flow_item_eth`` struct will be modified to include
> +  additional values, indicating existence or absence of VLAN headers
> +  following the ETH header, as proposed in RFC
> +  https://mails.dpdk.org/archives/dev/2020-August/177349.html.

It is unclear how it will coexist with VLAN items in a pattern.
Are you going to add consistency checks on ethdev-layer?

Also it is unclear why both bit fields and a number are
required.

Referenced RFC lacks definition of S-VLAN anc C-VLAN in
the context. Exact definition to avoid ambiguity.

So, it looks required to modify the structure, but I'd
not stick to referenced RFC, since the result could
differ a lot. May be reference it as just an example.
Dekel Peled Aug. 5, 2020, 1:31 p.m. UTC | #3
Thanks, PSB.

> -----Original Message-----
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> Sent: Wednesday, August 5, 2020 1:49 PM
> To: Dekel Peled <dekelp@mellanox.com>; dev@dpdk.org
> Cc: jerinjacobk@gmail.com; stephen@networkplumber.org;
> ajit.khaparde@broadcom.com; maxime.coquelin@redhat.com;
> olivier.matz@6wind.com; david.marchand@redhat.com;
> ferruh.yigit@intel.com; Asaf Penso <asafp@mellanox.com>
> Subject: Re: [dpdk-dev] [PATCH] doc: announce change in ETH item struct
> 
> On 8/4/20 7:01 PM, Dekel Peled wrote:
> > Struct rte_flow_item_eth will be modified to include additional
> > values, indicating existence or absence of VLAN headers following the
> > ETH header, as proposed in RFC
> >
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.
> dpdk.org%2Farchives%2Fdev%2F2020-
> August%2F177349.html&amp;data=02%7C01%7Cdekelp%40mellanox.com%7
> C7d12c0524d434c10f35a08d8392d3f53%7Ca652971c7d2e4d9ba6a4d149256f46
> 1b%7C0%7C0%7C637322213783925660&amp;sdata=4rEXCifnCHd2%2FA6AU4
> F3vjBcD7CfoTpT0traJ2z1fBk%3D&amp;reserved=0.
> > Because of ABI break this change is proposed for 20.11.
> >
> > Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> > ---
> >  doc/guides/rel_notes/deprecation.rst | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/doc/guides/rel_notes/deprecation.rst
> > b/doc/guides/rel_notes/deprecation.rst
> > index 5201142..6241709 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -115,6 +115,11 @@ Deprecation Notices
> >    following the IPv6 header, as proposed in RFC
> >
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.
> dpdk.org%2Farchives%2Fdev%2F2020-
> August%2F177257.html&amp;data=02%7C01%7Cdekelp%40mellanox.com%7
> C7d12c0524d434c10f35a08d8392d3f53%7Ca652971c7d2e4d9ba6a4d149256f46
> 1b%7C0%7C0%7C637322213783925660&amp;sdata=Ix4Y2vEXMoBek%2BkXw
> QazY11a9mkc3aiZRJDX9cbUiZk%3D&amp;reserved=0.
> >
> > +* ethdev: The ``struct rte_flow_item_eth`` struct will be modified to
> > +include
> > +  additional values, indicating existence or absence of VLAN headers
> > +  following the ETH header, as proposed in RFC
> > +
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.
> dpdk.org%2Farchives%2Fdev%2F2020-
> August%2F177349.html&amp;data=02%7C01%7Cdekelp%40mellanox.com%7
> C7d12c0524d434c10f35a08d8392d3f53%7Ca652971c7d2e4d9ba6a4d149256f46
> 1b%7C0%7C0%7C637322213783925660&amp;sdata=4rEXCifnCHd2%2FA6AU4
> F3vjBcD7CfoTpT0traJ2z1fBk%3D&amp;reserved=0.
> 
> It is unclear how it will coexist with VLAN items in a pattern.
Same as the existing proto field coexist with following VLAN items.

> Are you going to add consistency checks on ethdev-layer?
Not planned currently.

> 
> Also it is unclear why both bit fields and a number are required.
I agree it is redundancy, added for flexibility, but can leave num_of_vlans only.

> 
> Referenced RFC lacks definition of S-VLAN anc C-VLAN in the context. Exact
> definition to avoid ambiguity.
These are well defined terms, I will add reference to spec.

> 
> So, it looks required to modify the structure, but I'd not stick to referenced
> RFC, since the result could differ a lot. May be reference it as just an
> example.
Thank you.
Andrew Rybchenko Aug. 5, 2020, 1:44 p.m. UTC | #4
On 8/5/20 4:31 PM, Dekel Peled wrote:
> Thanks, PSB.
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <arybchenko@solarflare.com>
>> Sent: Wednesday, August 5, 2020 1:49 PM
>> To: Dekel Peled <dekelp@mellanox.com>; dev@dpdk.org
>> Cc: jerinjacobk@gmail.com; stephen@networkplumber.org;
>> ajit.khaparde@broadcom.com; maxime.coquelin@redhat.com;
>> olivier.matz@6wind.com; david.marchand@redhat.com;
>> ferruh.yigit@intel.com; Asaf Penso <asafp@mellanox.com>
>> Subject: Re: [dpdk-dev] [PATCH] doc: announce change in ETH item struct
>>
>> On 8/4/20 7:01 PM, Dekel Peled wrote:
>>> Struct rte_flow_item_eth will be modified to include additional
>>> values, indicating existence or absence of VLAN headers following the
>>> ETH header, as proposed in RFC
>>>
>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.
>> dpdk.org%2Farchives%2Fdev%2F2020-
>> August%2F177349.html&amp;data=02%7C01%7Cdekelp%40mellanox.com%7
>> C7d12c0524d434c10f35a08d8392d3f53%7Ca652971c7d2e4d9ba6a4d149256f46
>> 1b%7C0%7C0%7C637322213783925660&amp;sdata=4rEXCifnCHd2%2FA6AU4
>> F3vjBcD7CfoTpT0traJ2z1fBk%3D&amp;reserved=0.
>>> Because of ABI break this change is proposed for 20.11.
>>>
>>> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
>>> ---
>>>  doc/guides/rel_notes/deprecation.rst | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>> b/doc/guides/rel_notes/deprecation.rst
>>> index 5201142..6241709 100644
>>> --- a/doc/guides/rel_notes/deprecation.rst
>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>> @@ -115,6 +115,11 @@ Deprecation Notices
>>>    following the IPv6 header, as proposed in RFC
>>>
>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.
>> dpdk.org%2Farchives%2Fdev%2F2020-
>> August%2F177257.html&amp;data=02%7C01%7Cdekelp%40mellanox.com%7
>> C7d12c0524d434c10f35a08d8392d3f53%7Ca652971c7d2e4d9ba6a4d149256f46
>> 1b%7C0%7C0%7C637322213783925660&amp;sdata=Ix4Y2vEXMoBek%2BkXw
>> QazY11a9mkc3aiZRJDX9cbUiZk%3D&amp;reserved=0.
>>>
>>> +* ethdev: The ``struct rte_flow_item_eth`` struct will be modified to
>>> +include
>>> +  additional values, indicating existence or absence of VLAN headers
>>> +  following the ETH header, as proposed in RFC
>>> +
>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.
>> dpdk.org%2Farchives%2Fdev%2F2020-
>> August%2F177349.html&amp;data=02%7C01%7Cdekelp%40mellanox.com%7
>> C7d12c0524d434c10f35a08d8392d3f53%7Ca652971c7d2e4d9ba6a4d149256f46
>> 1b%7C0%7C0%7C637322213783925660&amp;sdata=4rEXCifnCHd2%2FA6AU4
>> F3vjBcD7CfoTpT0traJ2z1fBk%3D&amp;reserved=0.
>>
>> It is unclear how it will coexist with VLAN items in a pattern.
> Same as the existing proto field coexist with following VLAN items.
> 
>> Are you going to add consistency checks on ethdev-layer?
> Not planned currently.

IMHO, it is a must requirement if you introduce interface which
has inter-dependencies and requires generic consistency check.
It must be a part of ethdev, not every PMD.

> 
>>
>> Also it is unclear why both bit fields and a number are required.
> I agree it is redundancy, added for flexibility, but can leave num_of_vlans only.
> 
>>
>> Referenced RFC lacks definition of S-VLAN anc C-VLAN in the context. Exact
>> definition to avoid ambiguity.
> These are well defined terms, I will add reference to spec.

Basically, I agree that changes in the ETH item may be
required and OK to acknowledge the deprecation notice
without deep technical details. The only requirement is
API backward compatibility (i.e. just recompiled code
to take ABI changes into account must work).

> 
>>
>> So, it looks required to modify the structure, but I'd not stick to referenced
>> RFC, since the result could differ a lot. May be reference it as just an
>> example.
> Thank you.
> 
>
Dekel Peled Aug. 5, 2020, 2:25 p.m. UTC | #5
> -----Original Message-----
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> Sent: Wednesday, August 5, 2020 4:44 PM
> To: Dekel Peled <dekelp@mellanox.com>; dev@dpdk.org
> Cc: jerinjacobk@gmail.com; stephen@networkplumber.org;
> ajit.khaparde@broadcom.com; maxime.coquelin@redhat.com;
> olivier.matz@6wind.com; david.marchand@redhat.com;
> ferruh.yigit@intel.com; Asaf Penso <asafp@mellanox.com>
> Subject: Re: [dpdk-dev] [PATCH] doc: announce change in ETH item struct
> 
> On 8/5/20 4:31 PM, Dekel Peled wrote:
> > Thanks, PSB.
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <arybchenko@solarflare.com>
> >> Sent: Wednesday, August 5, 2020 1:49 PM
> >> To: Dekel Peled <dekelp@mellanox.com>; dev@dpdk.org
> >> Cc: jerinjacobk@gmail.com; stephen@networkplumber.org;
> >> ajit.khaparde@broadcom.com; maxime.coquelin@redhat.com;
> >> olivier.matz@6wind.com; david.marchand@redhat.com;
> >> ferruh.yigit@intel.com; Asaf Penso <asafp@mellanox.com>
> >> Subject: Re: [dpdk-dev] [PATCH] doc: announce change in ETH item
> >> struct
> >>
> >> On 8/4/20 7:01 PM, Dekel Peled wrote:
> >>> Struct rte_flow_item_eth will be modified to include additional
> >>> values, indicating existence or absence of VLAN headers following
> >>> the ETH header, as proposed in RFC
> >>>
> >>
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.
> >> dpdk.org%2Farchives%2Fdev%2F2020-
> >>
> August%2F177349.html&amp;data=02%7C01%7Cdekelp%40mellanox.com%7
> >>
> C7d12c0524d434c10f35a08d8392d3f53%7Ca652971c7d2e4d9ba6a4d149256f46
> >>
> 1b%7C0%7C0%7C637322213783925660&amp;sdata=4rEXCifnCHd2%2FA6AU4
> >> F3vjBcD7CfoTpT0traJ2z1fBk%3D&amp;reserved=0.
> >>> Because of ABI break this change is proposed for 20.11.
> >>>
> >>> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> >>> ---
> >>>  doc/guides/rel_notes/deprecation.rst | 5 +++++
> >>>  1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/doc/guides/rel_notes/deprecation.rst
> >>> b/doc/guides/rel_notes/deprecation.rst
> >>> index 5201142..6241709 100644
> >>> --- a/doc/guides/rel_notes/deprecation.rst
> >>> +++ b/doc/guides/rel_notes/deprecation.rst
> >>> @@ -115,6 +115,11 @@ Deprecation Notices
> >>>    following the IPv6 header, as proposed in RFC
> >>>
> >>
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.
> >> dpdk.org%2Farchives%2Fdev%2F2020-
> >>
> August%2F177257.html&amp;data=02%7C01%7Cdekelp%40mellanox.com%7
> >>
> C7d12c0524d434c10f35a08d8392d3f53%7Ca652971c7d2e4d9ba6a4d149256f46
> >>
> 1b%7C0%7C0%7C637322213783925660&amp;sdata=Ix4Y2vEXMoBek%2BkXw
> >> QazY11a9mkc3aiZRJDX9cbUiZk%3D&amp;reserved=0.
> >>>
> >>> +* ethdev: The ``struct rte_flow_item_eth`` struct will be modified
> >>> +to include
> >>> +  additional values, indicating existence or absence of VLAN
> >>> +headers
> >>> +  following the ETH header, as proposed in RFC
> >>> +
> >>
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.
> >> dpdk.org%2Farchives%2Fdev%2F2020-
> >>
> August%2F177349.html&amp;data=02%7C01%7Cdekelp%40mellanox.com%7
> >>
> C7d12c0524d434c10f35a08d8392d3f53%7Ca652971c7d2e4d9ba6a4d149256f46
> >>
> 1b%7C0%7C0%7C637322213783925660&amp;sdata=4rEXCifnCHd2%2FA6AU4
> >> F3vjBcD7CfoTpT0traJ2z1fBk%3D&amp;reserved=0.
> >>
> >> It is unclear how it will coexist with VLAN items in a pattern.
> > Same as the existing proto field coexist with following VLAN items.
> >
> >> Are you going to add consistency checks on ethdev-layer?
> > Not planned currently.
> 
> IMHO, it is a must requirement if you introduce interface which has inter-
> dependencies and requires generic consistency check.
> It must be a part of ethdev, not every PMD.
OK, will take it into account in the implementation.

> 
> >
> >>
> >> Also it is unclear why both bit fields and a number are required.
> > I agree it is redundancy, added for flexibility, but can leave num_of_vlans
> only.
> >
> >>
> >> Referenced RFC lacks definition of S-VLAN anc C-VLAN in the context.
> >> Exact definition to avoid ambiguity.
> > These are well defined terms, I will add reference to spec.
> 
> Basically, I agree that changes in the ETH item may be required and OK to
> acknowledge the deprecation notice without deep technical details. The only
> requirement is API backward compatibility (i.e. just recompiled code to take
> ABI changes into account must work).
Of course, code including proposed change is built and runs.

> 
> >
> >>
> >> So, it looks required to modify the structure, but I'd not stick to
> >> referenced RFC, since the result could differ a lot. May be reference
> >> it as just an example.
> > Thank you.
> >
> >

Patch
diff mbox series

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 5201142..6241709 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -115,6 +115,11 @@  Deprecation Notices
   following the IPv6 header, as proposed in RFC
   https://mails.dpdk.org/archives/dev/2020-August/177257.html.
 
+* ethdev: The ``struct rte_flow_item_eth`` struct will be modified to include
+  additional values, indicating existence or absence of VLAN headers
+  following the ETH header, as proposed in RFC
+  https://mails.dpdk.org/archives/dev/2020-August/177349.html.
+
 * traffic manager: All traffic manager API's in ``rte_tm.h`` were mistakenly made
   ABI stable in the v19.11 release. The TM maintainer and other contributors have
   agreed to keep the TM APIs as experimental in expectation of additional spec