DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Ajit Khaparde <ajit.khaparde@broadcom.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	Asaf Penso <asafp@nvidia.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"Gal Cohen (ProdM)" <galco@nvidia.com>,
	Andrew Rybchenko <arybchenko@solarflare.com>,
	Jerin Jacob Kollanukkaran <jerinj@marvell.com>
Subject: Re: [dpdk-dev] [PATCH v4] doc: add new tables for rte flow items and actions support
Date: Thu, 18 Feb 2021 18:45:38 +0000	[thread overview]
Message-ID: <89ed919a-f45d-3c74-ad62-5402056a612b@intel.com> (raw)
In-Reply-To: <CACZ4nhu6vucdtAh59u-y42Sk3RA5KbpYrr3cBHORCK=vbfnA5g@mail.gmail.com>

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 
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

  reply	other threads:[~2021-02-18 18:46 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-04 17:05 [dpdk-dev] [PATCH] " Asaf Penso
2021-02-04 22:09 ` Thomas Monjalon
2021-02-07  8:46   ` Asaf Penso
2021-02-07  9:44     ` Thomas Monjalon
2021-02-07  9:27 ` [dpdk-dev] [PATCH v2] " Asaf Penso
2021-02-07 10:46   ` [dpdk-dev] [PATCH v3] " Asaf Penso
2021-02-07 10:52     ` [dpdk-dev] [PATCH v4] " Asaf Penso
2021-02-08 12:58       ` Thomas Monjalon
2021-02-16 13:13       ` Ferruh Yigit
2021-02-17  5:57         ` Asaf Penso
2021-02-17 10:37           ` Ferruh Yigit
2021-02-17 10:49             ` Thomas Monjalon
2021-02-18 16:12               ` Asaf Penso
2021-02-18 17:58               ` Ajit Khaparde
2021-02-18 18:45                 ` Ferruh Yigit [this message]
2021-04-06 15:07                   ` Thomas Monjalon
2021-04-07 22:33 ` [dpdk-dev] [PATCH v5 0/3] rte_flow doc matrix Thomas Monjalon
2021-04-07 22:33   ` [dpdk-dev] [PATCH v5 1/3] doc: rename sfc features file Thomas Monjalon
2021-04-07 22:33   ` [dpdk-dev] [PATCH v5 2/3] doc: add flow API features tables Thomas Monjalon
2021-04-08  8:45     ` [dpdk-dev] [EXT] " Kiran Kumar Kokkilagadda
2021-05-14 10:42     ` [dpdk-dev] " Ferruh Yigit
2021-05-14 11:52       ` Thomas Monjalon
2021-04-07 22:33   ` [dpdk-dev] [PATCH v5 3/3] devtools: check flow API doc tables Thomas Monjalon
2021-05-13 18:40     ` Ferruh Yigit
2021-05-18 12:46       ` Thomas Monjalon
2021-05-14 10:51     ` Ferruh Yigit
2021-05-18 10:33       ` Thomas Monjalon
2021-05-18 13:28 ` [dpdk-dev] [PATCH v6 0/3] rte_flow doc matrix Thomas Monjalon
2021-05-18 13:28   ` [dpdk-dev] [PATCH v6 1/3] doc: rename sfc features file Thomas Monjalon
2021-05-18 13:28   ` [dpdk-dev] [PATCH v6 2/3] doc: add flow API features tables Thomas Monjalon
2021-05-18 13:28   ` [dpdk-dev] [PATCH v6 3/3] devtools: check flow API doc tables Thomas Monjalon
2021-05-18 16:54   ` [dpdk-dev] [PATCH v6 0/3] rte_flow doc matrix Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=89ed919a-f45d-3c74-ad62-5402056a612b@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=ajit.khaparde@broadcom.com \
    --cc=arybchenko@solarflare.com \
    --cc=asafp@nvidia.com \
    --cc=dev@dpdk.org \
    --cc=galco@nvidia.com \
    --cc=jerinj@marvell.com \
    --cc=thomas@monjalon.net \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).