Thread (32 messages) 32 messages, 5 authors, 2021-05-18

Re: [dpdk-dev] [PATCH v4] doc: add new tables for rte flow items and actions support

From: Thomas Monjalon <hidden>
Date: 2021-04-06 15:07:16

18/02/2021 19:45, Ferruh Yigit:
On 2/18/2021 5:58 PM, Ajit Khaparde wrote:
quoted
On Wed, Feb 17, 2021 at 2:49 AM Thomas Monjalon [off-list ref] wrote:
quoted
17/02/2021 11:37, Ferruh Yigit:
quoted
On 2/17/2021 5:57 AM, Asaf Penso wrote:
quoted
From: Ferruh Yigit <redacted>
quoted
On 2/7/2021 10:52 AM, Asaf Penso wrote:
quoted
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.
quoted
quoted
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.
quoted
quoted
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?



Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help