[v4] doc: add new tables for rte flow items and actions support

Message ID 1612695128-31878-1-git-send-email-asafp@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v4] doc: add new tables for rte flow items and actions support |

Checks

Context Check Description
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-testing warning Testing issues
ci/travis-robot warning Travis build: failed
ci/checkpatch success coding style OK

Commit Message

Asaf Penso Feb. 7, 2021, 10:52 a.m. UTC
  In http://doc.dpdk.org/guides/nics/overview.html, table 1.1 lists all
supported features.
It has a single line for "Flow API" that refers to rte_flow support.
rte_flow is composed of many items and actions that are not expressed in
this single line.

The following new tables are suggested:
1. rte_flow items
2. rte_flow actions

Also, since each table needs a new section in the pmd ini
file that might not be relevant for all pmds, the print
error message for missing section in conf.py is removed.

Signed-off-by: Asaf Penso <asafp@nvidia.com>
---
v4:
update commit message and remove the note about shared table

v3:
remove the shared action table
update the note for "S" notation

v2: 
update commit message
add .txt to .gitignore
---
 .gitignore                           |   2 +
 doc/guides/conf.py                   |  18 +++---
 doc/guides/nics/features/default.ini | 104 +++++++++++++++++++++++++++++++++++
 doc/guides/nics/features/mlx4.ini    |  15 +++++
 doc/guides/nics/features/mlx5.ini    |  66 ++++++++++++++++++++++
 doc/guides/nics/overview.rst         |  10 ++++
 6 files changed, 208 insertions(+), 7 deletions(-)
  

Comments

Thomas Monjalon Feb. 8, 2021, 12:58 p.m. UTC | #1
07/02/2021 11:52, Asaf Penso:
> In http://doc.dpdk.org/guides/nics/overview.html, table 1.1 lists all
> supported features.
> It has a single line for "Flow API" that refers to rte_flow support.
> rte_flow is composed of many items and actions that are not expressed in
> this single line.

One comment to add to this commit message if you agree:
When the new rte_flow tables are filled for all relevant PMDs,
we could remove the row "Flow API" from the main table.

> The following new tables are suggested:
> 1. rte_flow items
> 2. rte_flow actions
[...]
> +vlan                 =
> +ipv4                 =
> +ipv6                 =
> +icmp                 =
> +udp                  =
> +tcp                  =
> +sctp                 =
> +vxlan                =
[...]
> +set_ipv4_src         =
> +set_ipv4_dst         =

In items and actions above (and others) I understand the names are taken
directly from the enums or structs.
However I could imagine a slight different natural naming,
using some upper cases and spaces. Example:
	set_ipv4_dst -> set IPv4 dst
I see pros and cons. What are other opinions?
  
Ferruh Yigit Feb. 16, 2021, 1:13 p.m. UTC | #2
On 2/7/2021 10:52 AM, Asaf Penso wrote:
> In http://doc.dpdk.org/guides/nics/overview.html, table 1.1 lists all
> supported features.
> It has a single line for "Flow API" that refers to rte_flow support.
> rte_flow is composed of many items and actions that are not expressed in
> this single line.
> 
> The following new tables are suggested:
> 1. rte_flow items
> 2. rte_flow actions
> 

Hi Asaf,

I understand the intention, but I am not sure about this.

The Flow API does not provide a capability or feature list in the API level, by 
design, because it is very hard to do it correct, but this patch tries to do it 
in the documentation level.

This will be missing lots of details, the flow items and actions documented as 
supported may and may not be supported based on the details.

It will be very hard to read this table (when it becomes full), also will be 
very hard to maintain.


Let me start with a question, who do you think will be your consumer?
Who will benefit from this table and how?


> Also, since each table needs a new section in the pmd ini
> file that might not be relevant for all pmds, the print
> error message for missing section in conf.py is removed.
> 
> Signed-off-by: Asaf Penso <asafp@nvidia.com>

<...>
  
Asaf Penso Feb. 17, 2021, 5:57 a.m. UTC | #3
Hello Ferruh, thanks for the reply and please see my below comments.

Regards,
Asaf Penso

>-----Original Message-----
>From: Ferruh Yigit <ferruh.yigit@intel.com>
>Sent: Tuesday, February 16, 2021 3:14 PM
>To: Asaf Penso <asafp@nvidia.com>; dev@dpdk.org
>Cc: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Andrew
>Rybchenko <arybchenko@solarflare.com>
>Subject: Re: [dpdk-dev] [PATCH v4] doc: add new tables for rte flow items and
>actions support
>
>On 2/7/2021 10:52 AM, Asaf Penso wrote:
>> In http://doc.dpdk.org/guides/nics/overview.html, table 1.1 lists all
>> supported features.
>> It has a single line for "Flow API" that refers to rte_flow support.
>> rte_flow is composed of many items and actions that are not expressed
>> in this single line.
>>
>> The following new tables are suggested:
>> 1. rte_flow items
>> 2. rte_flow actions
>>
>
>Hi Asaf,
>
>I understand the intention, but I am not sure about this.
>
>The Flow API does not provide a capability or feature list in the API level, by
>design, because it is very hard to do it correct, but this patch tries to do it in the
>documentation level.
>
>This will be missing lots of details, the flow items and actions documented as
>supported may and may not be supported based on the details.
>

Which missing details are you referring to? All flow items and all actions are listed.

>It will be very hard to read this table (when it becomes full), also will be very hard
>to maintain.

As part of any documentation change in rte_flow the developer would also need to update this table.
Why would it be very hard to maintain?
>
>
>Let me start with a question, who do you think will be your consumer?
>Who will benefit from this table and how?

We get a lot of questions from users regarding rte_flow support and we do not have a single place with proper documentation.
I can ask the same about the overall feature table, right? There is a value to document the support.

>
>
>> Also, since each table needs a new section in the pmd ini
>> file that might not be relevant for all pmds, the print
>> error message for missing section in conf.py is removed.
>>
>> Signed-off-by: Asaf Penso <asafp@nvidia.com>
>
><...>
  
Ferruh Yigit Feb. 17, 2021, 10:37 a.m. UTC | #4
On 2/17/2021 5:57 AM, Asaf Penso wrote:
> Hello Ferruh, thanks for the reply and please see my below comments.
> 
> Regards,
> Asaf Penso
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Tuesday, February 16, 2021 3:14 PM
>> To: Asaf Penso <asafp@nvidia.com>; dev@dpdk.org
>> Cc: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Andrew
>> Rybchenko <arybchenko@solarflare.com>
>> Subject: Re: [dpdk-dev] [PATCH v4] doc: add new tables for rte flow items and
>> actions support
>>
>> On 2/7/2021 10:52 AM, Asaf Penso wrote:
>>> In http://doc.dpdk.org/guides/nics/overview.html, table 1.1 lists all
>>> supported features.
>>> It has a single line for "Flow API" that refers to rte_flow support.
>>> rte_flow is composed of many items and actions that are not expressed
>>> in this single line.
>>>
>>> The following new tables are suggested:
>>> 1. rte_flow items
>>> 2. rte_flow actions
>>>
>>
>> Hi Asaf,
>>
>> I understand the intention, but I am not sure about this.
>>
>> The Flow API does not provide a capability or feature list in the API level, by
>> design, because it is very hard to do it correct, but this patch tries to do it in the
>> documentation level.
>>
>> This will be missing lots of details, the flow items and actions documented as
>> supported may and may not be supported based on the details.
>>
> 
> Which missing details are you referring to? All flow items and all actions are listed.
> 

Patterns are complex, any rule can be valid or invalid based on provided pattern 
values (details), also any rule can be valid or invalid based on previous rules 
or configuration.

In practice this information is much more useful if it is provided by API, but 
we are not able to do it because of its complex nature, it should be same level 
of complexity to provide this information by documentation.

>> It will be very hard to read this table (when it becomes full), also will be very hard
>> to maintain.
> 
> As part of any documentation change in rte_flow the developer would also need to update this table.
> Why would it be very hard to maintain?
 >

Ahh, that sound so simple when you say like this :) In practice even keeping 
feature list requiring lots of effort, developers are 
missing/neglecting/ignoring updating documentation when updating the code.

And for this case is partially correct table a useful information? If this is 
not completely correct people won't rely on it and it will become just useless.
So this feature should come with an automated way to detect if a rule supported 
but not documented, or even better this table should be generated from code 
automatically.

>>
>>
>> Let me start with a question, who do you think will be your consumer?
>> Who will benefit from this table and how?
> 
> We get a lot of questions from users regarding rte_flow support and we do not have a single place with proper documentation.
> I can ask the same about the overall feature table, right? There is a value to document the support.
> 

Let's discuss the feature table separately, I think that is a valid question.

For the rte_flow, who is asking questions? End user, or application developer? 
So is this intended to be a marketing documentation or technical documentation?

And what is the nature of the questions, if it is related to the rte_flow, there 
is already a proper documentation for it:
https://doc.dpdk.org/guides/prog_guide/rte_flow.html

If this question is if any specific rule supported by a specific PMD, right now 
only valid way to say this that I am aware of is, run 'rte_flow_validate()' and see.
Not sure if we can document this properly.

>>
>>
>>> Also, since each table needs a new section in the pmd ini
>>> file that might not be relevant for all pmds, the print
>>> error message for missing section in conf.py is removed.
>>>
>>> Signed-off-by: Asaf Penso <asafp@nvidia.com>
>>
>> <...>
  
Thomas Monjalon Feb. 17, 2021, 10:49 a.m. UTC | #5
17/02/2021 11:37, Ferruh Yigit:
> On 2/17/2021 5:57 AM, Asaf Penso wrote:
> > From: Ferruh Yigit <ferruh.yigit@intel.com>
> >> On 2/7/2021 10:52 AM, Asaf Penso wrote:
> >>> In http://doc.dpdk.org/guides/nics/overview.html, table 1.1 lists all
> >>> supported features.
> >>> It has a single line for "Flow API" that refers to rte_flow support.
> >>> rte_flow is composed of many items and actions that are not expressed
> >>> in this single line.
> >>>
> >>> The following new tables are suggested:
> >>> 1. rte_flow items
> >>> 2. rte_flow actions
> >>>
> >>
> >> Hi Asaf,
> >>
> >> I understand the intention, but I am not sure about this.
> >>
> >> The Flow API does not provide a capability or feature list in the API level, by
> >> design, because it is very hard to do it correct, but this patch tries to do it in the
> >> documentation level.
> >>
> >> This will be missing lots of details, the flow items and actions documented as
> >> supported may and may not be supported based on the details.
> >>
> > 
> > Which missing details are you referring to? All flow items and all actions are listed.
> > 
> 
> Patterns are complex, any rule can be valid or invalid based on provided pattern 
> values (details), also any rule can be valid or invalid based on previous rules 
> or configuration.
> 
> In practice this information is much more useful if it is provided by API, but 
> we are not able to do it because of its complex nature, it should be same level 
> of complexity to provide this information by documentation.
> 
> >> It will be very hard to read this table (when it becomes full), also will be very hard
> >> to maintain.
> > 
> > As part of any documentation change in rte_flow the developer would also need to update this table.
> > Why would it be very hard to maintain?
>  >
> 
> Ahh, that sound so simple when you say like this :) In practice even keeping 
> feature list requiring lots of effort, developers are 
> missing/neglecting/ignoring updating documentation when updating the code.
> 
> And for this case is partially correct table a useful information? If this is 
> not completely correct people won't rely on it and it will become just useless.
> So this feature should come with an automated way to detect if a rule supported 
> but not documented, or even better this table should be generated from code 
> automatically.
> 
> >>
> >>
> >> Let me start with a question, who do you think will be your consumer?
> >> Who will benefit from this table and how?
> > 
> > We get a lot of questions from users regarding rte_flow support and we do not have a single place with proper documentation.
> > I can ask the same about the overall feature table, right? There is a value to document the support.
> > 
> 
> Let's discuss the feature table separately, I think that is a valid question.
> 
> For the rte_flow, who is asking questions? End user, or application developer? 
> So is this intended to be a marketing documentation or technical documentation?
> 
> And what is the nature of the questions, if it is related to the rte_flow, there 
> is already a proper documentation for it:
> https://doc.dpdk.org/guides/prog_guide/rte_flow.html
> 
> If this question is if any specific rule supported by a specific PMD, right now 
> only valid way to say this that I am aware of is, run 'rte_flow_validate()' and see.
> Not sure if we can document this properly.

I think in general we are missing a big disclaimer
on top of this overview page:
	Each feature may have some hardware limitations.

Then there is a need, both for application developers and end-users,
to know which feature can be supported by a PMD,
or which PMD can support a feature.
Yes there are complex limitations with hardware offloads in general.
Yes it would be nice to report some tested capabilities with a CI.
But it does not mean we should not try to document it in my opinion.
  
Asaf Penso Feb. 18, 2021, 4:12 p.m. UTC | #6
>-----Original Message-----
>From: Thomas Monjalon <thomas@monjalon.net>
>Sent: Wednesday, February 17, 2021 12:49 PM
>To: Ferruh Yigit <ferruh.yigit@intel.com>
>Cc: Asaf Penso <asafp@nvidia.com>; dev@dpdk.org; Gal Cohen (ProdM)
><galco@nvidia.com>; Andrew Rybchenko <arybchenko@solarflare.com>;
>ajit.khaparde@broadcom.com; jerinj@marvell.com
>Subject: Re: [dpdk-dev] [PATCH v4] doc: add new tables for rte flow items and
>actions support
>
>17/02/2021 11:37, Ferruh Yigit:
>> On 2/17/2021 5:57 AM, Asaf Penso wrote:
>> > From: Ferruh Yigit <ferruh.yigit@intel.com>
>> >> On 2/7/2021 10:52 AM, Asaf Penso wrote:
>> >>> In http://doc.dpdk.org/guides/nics/overview.html, table 1.1 lists
>> >>> all supported features.
>> >>> It has a single line for "Flow API" that refers to rte_flow support.
>> >>> rte_flow is composed of many items and actions that are not
>> >>> expressed in this single line.
>> >>>
>> >>> The following new tables are suggested:
>> >>> 1. rte_flow items
>> >>> 2. rte_flow actions
>> >>>
>> >>
>> >> Hi Asaf,
>> >>
>> >> I understand the intention, but I am not sure about this.
>> >>
>> >> The Flow API does not provide a capability or feature list in the
>> >> API level, by design, because it is very hard to do it correct, but
>> >> this patch tries to do it in the documentation level.
>> >>
>> >> This will be missing lots of details, the flow items and actions
>> >> documented as supported may and may not be supported based on the
>details.
>> >>
>> >
>> > Which missing details are you referring to? All flow items and all actions are
>listed.
>> >
>>
>> Patterns are complex, any rule can be valid or invalid based on
>> provided pattern values (details), also any rule can be valid or
>> invalid based on previous rules or configuration.
>>
>> In practice this information is much more useful if it is provided by
>> API, but we are not able to do it because of its complex nature, it
>> should be same level of complexity to provide this information by
>documentation.
>>
>> >> It will be very hard to read this table (when it becomes full),
>> >> also will be very hard to maintain.
>> >
>> > As part of any documentation change in rte_flow the developer would
>also need to update this table.
>> > Why would it be very hard to maintain?
>>  >
>>
>> Ahh, that sound so simple when you say like this :) In practice even
>> keeping feature list requiring lots of effort, developers are
>> missing/neglecting/ignoring updating documentation when updating the
>code.
>>
>> And for this case is partially correct table a useful information? If
>> this is not completely correct people won't rely on it and it will become just
>useless.
>> So this feature should come with an automated way to detect if a rule
>> supported but not documented, or even better this table should be
>> generated from code automatically.
>>
>> >>
>> >>
>> >> Let me start with a question, who do you think will be your consumer?
>> >> Who will benefit from this table and how?
>> >
>> > We get a lot of questions from users regarding rte_flow support and we
>do not have a single place with proper documentation.
>> > I can ask the same about the overall feature table, right? There is a value
>to document the support.
>> >
>>
>> Let's discuss the feature table separately, I think that is a valid question.
>>
>> For the rte_flow, who is asking questions? End user, or application
>developer?
>> So is this intended to be a marketing documentation or technical
>documentation?
>>
>> And what is the nature of the questions, if it is related to the
>> rte_flow, there is already a proper documentation for it:
>> https://doc.dpdk.org/guides/prog_guide/rte_flow.html
>>
>> If this question is if any specific rule supported by a specific PMD,
>> right now only valid way to say this that I am aware of is, run
>'rte_flow_validate()' and see.
>> Not sure if we can document this properly.
>
>I think in general we are missing a big disclaimer on top of this overview page:
>	Each feature may have some hardware limitations.

Also, the combination of several items and actions is not feasible to be documented. Ferruh, I believe you are referring to this case.
I wanted to have some documentation of the supported items/actions.
The questions come from all users, I didn't bisect to see if they are application engineers or developers or users.
For sure, they can take testpmd and confirm, but before even going there and if they want to evaluate DPDK, they prefer to see a place with this documentation.
We cannot document everything, I know. Also, believe me, that I know how challenging is to track the relevancy of document 😊
Still, it's better to have this, than not at all.

>
>Then there is a need, both for application developers and end-users, to know
>which feature can be supported by a PMD, or which PMD can support a
>feature.
>Yes there are complex limitations with hardware offloads in general.
>Yes it would be nice to report some tested capabilities with a CI.
>But it does not mean we should not try to document it in my opinion.
>
  
Ajit Khaparde Feb. 18, 2021, 5:58 p.m. UTC | #7
On Wed, Feb 17, 2021 at 2:49 AM Thomas Monjalon <thomas@monjalon.net> wrote:

> 17/02/2021 11:37, Ferruh Yigit:
> > On 2/17/2021 5:57 AM, Asaf Penso wrote:
> > > From: Ferruh Yigit <ferruh.yigit@intel.com>
> > >> On 2/7/2021 10:52 AM, Asaf Penso wrote:
> > >>> In http://doc.dpdk.org/guides/nics/overview.html, table 1.1 lists
> all
> > >>> supported features.
> > >>> It has a single line for "Flow API" that refers to rte_flow support.
> > >>> rte_flow is composed of many items and actions that are not expressed
> > >>> in this single line.
> > >>>
> > >>> The following new tables are suggested:
> > >>> 1. rte_flow items
> > >>> 2. rte_flow actions
> > >>>
> > >>
> > >> Hi Asaf,
> > >>
> > >> I understand the intention, but I am not sure about this.
> > >>
> > >> The Flow API does not provide a capability or feature list in the API
> level, by
> > >> design, because it is very hard to do it correct, but this patch
> tries to do it in the
> > >> documentation level.
> > >>
> > >> This will be missing lots of details, the flow items and actions
> documented as
> > >> supported may and may not be supported based on the details.
> > >>
> > >
> > > Which missing details are you referring to? All flow items and all
> actions are listed.
> > >
> >
> > Patterns are complex, any rule can be valid or invalid based on provided
> pattern
> > values (details), also any rule can be valid or invalid based on
> previous rules
> > or configuration.
> >
> > In practice this information is much more useful if it is provided by
> API, but
> > we are not able to do it because of its complex nature, it should be
> same level
> > of complexity to provide this information by documentation.
> >
> > >> It will be very hard to read this table (when it becomes full), also
> will be very hard
> > >> to maintain.
> > >
> > > As part of any documentation change in rte_flow the developer would
> also need to update this table.
> > > Why would it be very hard to maintain?
> >  >
> >
> > Ahh, that sound so simple when you say like this :) In practice even
> keeping
> > feature list requiring lots of effort, developers are
> > missing/neglecting/ignoring updating documentation when updating the
> code.
> >
> > And for this case is partially correct table a useful information? If
> this is
> > not completely correct people won't rely on it and it will become just
> useless.
> > So this feature should come with an automated way to detect if a rule
> supported
> > but not documented, or even better this table should be generated from
> code
> > automatically.
> >
> > >>
> > >>
> > >> Let me start with a question, who do you think will be your consumer?
> > >> Who will benefit from this table and how?
> > >
> > > We get a lot of questions from users regarding rte_flow support and we
> do not have a single place with proper documentation.
> > > I can ask the same about the overall feature table, right? There is a
> value to document the support.
> > >
> >
> > Let's discuss the feature table separately, I think that is a valid
> question.
> >
> > For the rte_flow, who is asking questions? End user, or application
> developer?
> > So is this intended to be a marketing documentation or technical
> documentation?
> >
> > And what is the nature of the questions, if it is related to the
> rte_flow, there
> > is already a proper documentation for it:
> > https://doc.dpdk.org/guides/prog_guide/rte_flow.html
> >
> > If this question is if any specific rule supported by a specific PMD,
> right now
> > only valid way to say this that I am aware of is, run
> 'rte_flow_validate()' and see.
> > Not sure if we can document this properly.
>
> I think in general we are missing a big disclaimer
> on top of this overview page:
>         Each feature may have some hardware limitations.
>
> Then there is a need, both for application developers and end-users,
> to know which feature can be supported by a PMD,
> or which PMD can support a feature.
> Yes there are complex limitations with hardware offloads in general.
> Yes it would be nice to report some tested capabilities with a CI.
> But it does not mean we should not try to document it in my opinion.
>
+1 to all of these.
A document like this will help give an idea on what is possible with the
PMD without looking at the code. Beyond that, the user can check with
the vendor/developer for specific details if needed.
  
Ferruh Yigit Feb. 18, 2021, 6:45 p.m. UTC | #8
On 2/18/2021 5:58 PM, Ajit Khaparde wrote:
> On Wed, Feb 17, 2021 at 2:49 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> 
>> 17/02/2021 11:37, Ferruh Yigit:
>>> On 2/17/2021 5:57 AM, Asaf Penso wrote:
>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>> On 2/7/2021 10:52 AM, Asaf Penso wrote:
>>>>>> In http://doc.dpdk.org/guides/nics/overview.html, table 1.1 lists
>> all
>>>>>> supported features.
>>>>>> It has a single line for "Flow API" that refers to rte_flow support.
>>>>>> rte_flow is composed of many items and actions that are not expressed
>>>>>> in this single line.
>>>>>>
>>>>>> The following new tables are suggested:
>>>>>> 1. rte_flow items
>>>>>> 2. rte_flow actions
>>>>>>
>>>>>
>>>>> Hi Asaf,
>>>>>
>>>>> I understand the intention, but I am not sure about this.
>>>>>
>>>>> The Flow API does not provide a capability or feature list in the API
>> level, by
>>>>> design, because it is very hard to do it correct, but this patch
>> tries to do it in the
>>>>> documentation level.
>>>>>
>>>>> This will be missing lots of details, the flow items and actions
>> documented as
>>>>> supported may and may not be supported based on the details.
>>>>>
>>>>
>>>> Which missing details are you referring to? All flow items and all
>> actions are listed.
>>>>
>>>
>>> Patterns are complex, any rule can be valid or invalid based on provided
>> pattern
>>> values (details), also any rule can be valid or invalid based on
>> previous rules
>>> or configuration.
>>>
>>> In practice this information is much more useful if it is provided by
>> API, but
>>> we are not able to do it because of its complex nature, it should be
>> same level
>>> of complexity to provide this information by documentation.
>>>
>>>>> It will be very hard to read this table (when it becomes full), also
>> will be very hard
>>>>> to maintain.
>>>>
>>>> As part of any documentation change in rte_flow the developer would
>> also need to update this table.
>>>> Why would it be very hard to maintain?
>>>   >
>>>
>>> Ahh, that sound so simple when you say like this :) In practice even
>> keeping
>>> feature list requiring lots of effort, developers are
>>> missing/neglecting/ignoring updating documentation when updating the
>> code.
>>>
>>> And for this case is partially correct table a useful information? If
>> this is
>>> not completely correct people won't rely on it and it will become just
>> useless.
>>> So this feature should come with an automated way to detect if a rule
>> supported
>>> but not documented, or even better this table should be generated from
>> code
>>> automatically.
>>>
>>>>>
>>>>>
>>>>> Let me start with a question, who do you think will be your consumer?
>>>>> Who will benefit from this table and how?
>>>>
>>>> We get a lot of questions from users regarding rte_flow support and we
>> do not have a single place with proper documentation.
>>>> I can ask the same about the overall feature table, right? There is a
>> value to document the support.
>>>>
>>>
>>> Let's discuss the feature table separately, I think that is a valid
>> question.
>>>
>>> For the rte_flow, who is asking questions? End user, or application
>> developer?
>>> So is this intended to be a marketing documentation or technical
>> documentation?
>>>
>>> And what is the nature of the questions, if it is related to the
>> rte_flow, there
>>> is already a proper documentation for it:
>>> https://doc.dpdk.org/guides/prog_guide/rte_flow.html
>>>
>>> If this question is if any specific rule supported by a specific PMD,
>> right now
>>> only valid way to say this that I am aware of is, run
>> 'rte_flow_validate()' and see.
>>> Not sure if we can document this properly.
>>
>> I think in general we are missing a big disclaimer
>> on top of this overview page:
>>          Each feature may have some hardware limitations.
>>
>> Then there is a need, both for application developers and end-users,
>> to know which feature can be supported by a PMD,
>> or which PMD can support a feature.
>> Yes there are complex limitations with hardware offloads in general.
>> Yes it would be nice to report some tested capabilities with a CI.
>> But it does not mean we should not try to document it in my opinion.
>>
> +1 to all of these.
> A document like this will help give an idea on what is possible with the
> PMD without looking at the code. Beyond that, the user can check with
> the vendor/developer for specific details if needed.
> 

I am still feeling we are trying to workaround flow API design constrain with 
documentation, although we know it won't be complete.
And not really clear who will benefit from this in what way.

Anyway, as mentioned above I am concerned the maintenance cost, can this series 
investigate:
1) A way to automatically fill the table from source code
2) A way to check if a patch is adding a new flow support but not documenting it

Also can you please propose a maintainer for it (can be documented in MAINTAINER 
file) who will be responsible of the correctness of the table, which means:
- Will verify a claimed support by a PMD is really supported
- All flow API features are documented for a PMD
- Changes in the code are reflected to the documentation
- Trace PMD maintainers for missing data
  
Thomas Monjalon April 6, 2021, 3:07 p.m. UTC | #9
18/02/2021 19:45, Ferruh Yigit:
> On 2/18/2021 5:58 PM, Ajit Khaparde wrote:
> > On Wed, Feb 17, 2021 at 2:49 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> >> 17/02/2021 11:37, Ferruh Yigit:
> >>> On 2/17/2021 5:57 AM, Asaf Penso wrote:
> >>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >>>>> On 2/7/2021 10:52 AM, Asaf Penso wrote:
> >>>>>> The following new tables are suggested:
> >>>>>> 1. rte_flow items
> >>>>>> 2. rte_flow actions
> >>>>>
> >>>>> Hi Asaf,
> >>>>>
> >>>>> I understand the intention, but I am not sure about this.
> >>>>>
> >>>>> The Flow API does not provide a capability or feature list in the API
> >>>>> level, by
> >>>>> design, because it is very hard to do it correct, but this patch
> >>>>> tries to do it in the
> >>>>> documentation level.
> >>>>>
> >>>>> This will be missing lots of details, the flow items and actions
> >>>>> documented as
> >>>>> supported may and may not be supported based on the details.
> >>>>>
> >>>>
> >>>> Which missing details are you referring to? All flow items and all
> >>>> actions are listed.
> >>>
> >>> Patterns are complex, any rule can be valid or invalid based on provided
> >>> pattern
> >>> values (details), also any rule can be valid or invalid based on
> >>> previous rules
> >>> or configuration.
> >>>
> >>> In practice this information is much more useful if it is provided by
> >>> API, but
> >>> we are not able to do it because of its complex nature, it should be
> >>> same level
> >>> of complexity to provide this information by documentation.
> >>>
> >>>>> It will be very hard to read this table (when it becomes full), also
> >>>>> will be very hard to maintain.
> >>>>
> >>>> As part of any documentation change in rte_flow the developer would
> >>>> also need to update this table.
> >>>> Why would it be very hard to maintain?
> >>>
> >>> Ahh, that sound so simple when you say like this :) In practice even
> >>> keeping
> >>> feature list requiring lots of effort, developers are
> >>> missing/neglecting/ignoring updating documentation when updating the
> >>> code.
> >>>
> >>> And for this case is partially correct table a useful information? If
> >>> this is
> >>> not completely correct people won't rely on it and it will become just
> >>> useless.
> >>> So this feature should come with an automated way to detect if a rule
> >>> supported
> >>> but not documented, or even better this table should be generated from
> >>> code automatically.
> >>>
> >>>>> Let me start with a question, who do you think will be your consumer?
> >>>>> Who will benefit from this table and how?
> >>>>
> >>>> We get a lot of questions from users regarding rte_flow support and we
> >>>> do not have a single place with proper documentation.
> >>>> I can ask the same about the overall feature table, right? There is a
> >>>> value to document the support.
> >>>>
> >>>
> >>> Let's discuss the feature table separately, I think that is a valid
> >>> question.
> >>>
> >>> For the rte_flow, who is asking questions? End user, or application
> >>> developer?
> >>> So is this intended to be a marketing documentation or technical
> >>> documentation?
> >>>
> >>> And what is the nature of the questions, if it is related to the
> >>> rte_flow, there
> >>> is already a proper documentation for it:
> >>> https://doc.dpdk.org/guides/prog_guide/rte_flow.html
> >>>
> >>> If this question is if any specific rule supported by a specific PMD,
> >>> right now
> >>> only valid way to say this that I am aware of is, run
> >>> 'rte_flow_validate()' and see.
> >>> Not sure if we can document this properly.
> >>
> >> I think in general we are missing a big disclaimer
> >> on top of this overview page:
> >>          Each feature may have some hardware limitations.
> >>
> >> Then there is a need, both for application developers and end-users,
> >> to know which feature can be supported by a PMD,
> >> or which PMD can support a feature.
> >> Yes there are complex limitations with hardware offloads in general.
> >> Yes it would be nice to report some tested capabilities with a CI.
> >> But it does not mean we should not try to document it in my opinion.
> >>
> > +1 to all of these.
> > A document like this will help give an idea on what is possible with the
> > PMD without looking at the code. Beyond that, the user can check with
> > the vendor/developer for specific details if needed.
> 
> I am still feeling we are trying to workaround flow API design constrain with 
> documentation, although we know it won't be complete.

I disagree, it is not a workaround.
API is for application to work with any hardware.
Doc is for users to imagine which feature they could use
in which config (HW, DPDK version, etc).

> And not really clear who will benefit from this in what way.

End users will be the main users of this doc,
but it will help a lot more people to track what is implemented.

> Anyway, as mentioned above I am concerned the maintenance cost, can this series 
> investigate:
> 1) A way to automatically fill the table from source code

I am looking at it.

> 2) A way to check if a patch is adding a new flow support but not documenting it

Looking into it as well.

> Also can you please propose a maintainer for it (can be documented in MAINTAINER 
> file) who will be responsible of the correctness of the table, which means:
> - Will verify a claimed support by a PMD is really supported
> - All flow API features are documented for a PMD
> - Changes in the code are reflected to the documentation
> - Trace PMD maintainers for missing data

I'm not sure we should have a maintainer for this single file.
With the help of checkpatch script, the responsibility can be shared
in the next-net trees, isn't it?
  

Patch

diff --git a/.gitignore b/.gitignore
index f73d93c..b19c071 100644
--- a/.gitignore
+++ b/.gitignore
@@ -3,6 +3,8 @@ 
 
 # ignore generated documentation tables
 doc/guides/nics/overview_table.txt
+doc/guides/nics/rte_flow_actions_table.txt
+doc/guides/nics/rte_flow_items_table.txt
 doc/guides/cryptodevs/overview_feature_table.txt
 doc/guides/cryptodevs/overview_cipher_table.txt
 doc/guides/cryptodevs/overview_auth_table.txt
diff --git a/doc/guides/conf.py b/doc/guides/conf.py
index aceeb62..4eea58f 100644
--- a/doc/guides/conf.py
+++ b/doc/guides/conf.py
@@ -217,14 +217,8 @@  def generate_overview_table(output_filename, table_id, section, table_name, titl
         # Initialize the dict with the default.ini value.
         ini_data[ini_filename] = valid_features.copy()
 
-        # Check for a valid ini section.
+        # Check for a section.
         if not config.has_section(section):
-            print("{}: File '{}' has no [{}] secton".format(warning,
-                                                            ini_filename,
-                                                            section),
-                                                            file=stderr)
-            if stop_on_error:
-                raise Exception('Warning is treated as a failure')
             continue
 
         # Check for valid features names.
@@ -380,6 +374,16 @@  def setup(app):
                             'Features',
                             'Features availability in networking drivers',
                             'Feature')
+    table_file = dirname(__file__) + '/nics/rte_flow_items_table.txt'
+    generate_overview_table(table_file, 2,
+                            'rte_flow items',
+                            'rte_flow items features availability in networking drivers',
+                            'Items')
+    table_file = dirname(__file__) + '/nics/rte_flow_actions_table.txt'
+    generate_overview_table(table_file, 3,
+                            'rte_flow actions',
+                            'rte_flow actions features availability in networking drivers',
+                            'Actions')
     table_file = dirname(__file__) + '/cryptodevs/overview_feature_table.txt'
     generate_overview_table(table_file, 1,
                             'Features',
diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
index 8046bd1..968aee3 100644
--- a/doc/guides/nics/features/default.ini
+++ b/doc/guides/nics/features/default.ini
@@ -77,3 +77,107 @@  x86-64               =
 Usage doc            =
 Design doc           =
 Perf doc             =
+
+[rte_flow items]
+any                  =
+raw                  =
+eth                  =
+vlan                 =
+ipv4                 =
+ipv6                 =
+icmp                 =
+udp                  =
+tcp                  =
+sctp                 =
+vxlan                =
+e_tag                =
+nvgre                =
+mpls                 =
+gre                  =
+gre_key              =
+fuzzy                =
+gtp                  =
+gtpc                 =
+gtpu                 =
+esp                  =
+geneve               =
+vxlan-gpe            =
+arp_eth_ipv4         =
+ipv6_ext             =
+ipv6_frag_ext        =
+icmp6                =
+icmp6_nd_ns          =
+icmp6_nd_na          =
+icmp6_nd_opt         =
+icmp6_nd_opt_sla_eth =
+icmp6_nd_opt_tla_eth =
+meta                 =
+gtp_psc              =
+pppoes               =
+pppoed               =
+pppoe_proto_id       =
+nsh                  =
+igmp                 =
+ah                   =
+higig2               =
+l2tpv3oip            =
+pfcp                 =
+ecpri                =
+
+[rte_flow actions]
+end                  =
+void                 =
+passthru             =
+jump                 =
+mark                 =
+flag                 =
+queue                =
+drop                 =
+count                =
+rss                  =
+pf                   =
+vf                   =
+phy_port             =
+port_id              =
+meter                =
+security             =
+of_set_mpls_ttl      =
+of_dec_mpls_ttl      =
+of_set_nw_ttl        =
+of_dec_nw_ttl        =
+of_copy_ttl_out      =
+of_copy_ttl_in       =
+of_pop_vlan          =
+of_push_vlan         =
+of_set_vlan_vid      =
+of_set_vlan_pcp      =
+of_pop_mpls          =
+of_push_mpls         =
+vxlan_encap          =
+vxlan_decap          =
+nvgre_encap          =
+nvgre_decap          =
+raw_encap            =
+raw_decap            =
+set_ipv4_src         =
+set_ipv4_dst         =
+set_ipv6_src         =
+set_ipv6_dst         =
+set_tp_src           =
+set_tp_dst           =
+mac_swap             =
+dec_ttl              =
+set_ttl              =
+set_mac_src          =
+set_mac_dst          =
+inc_tcp_seq          =
+dec_tcp_seq          =
+inc_tcp_ack          =
+dec_tcp_ack          =
+set_tag              =
+set_meta             =
+set_ipv4_dscp        =
+set_ipv6_dscp        =
+age                  =
+sample               =
+modify_field         =
diff --git a/doc/guides/nics/features/mlx4.ini b/doc/guides/nics/features/mlx4.ini
index ebb9ccf..5e352f5 100644
--- a/doc/guides/nics/features/mlx4.ini
+++ b/doc/guides/nics/features/mlx4.ini
@@ -38,3 +38,18 @@  Power8               = Y
 x86-32               = Y
 x86-64               = Y
 Usage doc            = Y
+
+[rte_flow items]
+eth                  = Y
+vlan                 = Y
+ipv4                 = Y
+ipv6                 = Y
+udp                  = Y
+tcp                  = Y
+
+[rte_flow actions]
+end                  = Y
+void                 = Y
+queue                = Y
+drop                 = Y
+rss                  = Y
diff --git a/doc/guides/nics/features/mlx5.ini b/doc/guides/nics/features/mlx5.ini
index ddd131d..fcf934b 100644
--- a/doc/guides/nics/features/mlx5.ini
+++ b/doc/guides/nics/features/mlx5.ini
@@ -52,3 +52,69 @@  Power8               = Y
 x86-32               = Y
 x86-64               = Y
 Usage doc            = Y
+
+[rte_flow items]
+eth                  = Y
+vlan                 = Y
+ipv4                 = Y
+ipv6                 = Y
+icmp                 = Y
+udp                  = Y
+tcp                  = Y
+vxlan                = Y
+nvgre                = Y
+mpls                 = Y
+gre                  = Y
+gre_key              = Y
+gtp                  = Y
+geneve               = Y
+vxlan-gpe            = Y
+ipv6_ext             = Y
+ipv6_frag_ext        = Y
+icmp6                = Y
+meta                 = Y
+gtp_psc              = Y
+ecpri                = Y
+
+[rte_flow actions]
+end                  = Y
+void                 = Y
+jump                 = Y
+mark                 = Y
+flag                 = Y
+queue                = Y
+drop                 = Y
+count                = Y
+rss                  = S
+port_id              = Y
+meter                = Y
+of_pop_vlan          = Y
+of_push_vlan         = Y
+of_set_vlan_vid      = Y
+of_set_vlan_pcp      = Y
+vxlan_encap          = Y
+vxlan_decap          = Y
+raw_encap            = Y
+raw_decap            = Y
+set_ipv4_src         = Y
+set_ipv4_dst         = Y
+set_ipv6_src         = Y
+set_ipv6_dst         = Y
+set_tp_src           = Y
+set_tp_dst           = Y
+dec_ttl              = Y
+set_ttl              = Y
+set_mac_src          = Y
+set_mac_dst          = Y
+inc_tcp_seq          = Y
+dec_tcp_seq          = Y
+inc_tcp_ack          = Y
+dec_tcp_ack          = Y
+set_tag              = Y
+set_meta             = Y
+set_ipv4_dscp        = Y
+set_ipv6_dscp        = Y
+age                  = S
+sample               = Y
+modify_field         = Y
+
diff --git a/doc/guides/nics/overview.rst b/doc/guides/nics/overview.rst
index 20cd52b..86b5718 100644
--- a/doc/guides/nics/overview.rst
+++ b/doc/guides/nics/overview.rst
@@ -28,7 +28,17 @@  More details about features can be found in :doc:`features`.
 
 .. include:: overview_table.txt
 
+.. include:: rte_flow_items_table.txt
+
+.. include:: rte_flow_actions_table.txt
+
 .. Note::
 
    Features marked with "P" are partially supported. Refer to the appropriate
    NIC guide in the following sections for details.
+
+.. Note::
+
+   rte_flow action that is marked with "S" means it is supported as shared
+   and non-shared action.
+