Re: [dpdk-dev] [PATCH] doc: remove deprecated ethdev features

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 10/15/2019 5:19 PM, Jerin Jacob wrote:
> On Tue, Oct 15, 2019 at 9:26 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> On 10/15/2019 3:16 PM, Jerin Jacob wrote:
>>>>>>> @@ -36,13 +36,6 @@ VMDq                 =
>>>>>>>   SR-IOV               =
>>>>>>>   DCB                  =
>>>>>>>   VLAN filter          =
>>>>>>> -Ethertype filter     =
>>>>>>> -N-tuple filter       =
>>>>>>> -SYN filter           =
>>>>>>> -Tunnel filter        =
>>>>>>> -Flexible filter      =
>>>>>>> -Hash filter          =
>>>>>>> -Flow director        =
>>>>>>>   Flow control         =
>>>>>>>   Flow API             =
>>>>>>>   Rate limitation      =
>>>>>> I suggest adding these features back!
>>>>>>
>>>>>> "Flow director" and other filters are features that device/driver supports.
>>>>>>
>>>>>> And "Flow API" and "filter_ctrl" are methods used to implement these features.
>>>>>> Indeed they are only different APIs to get input from application/user.
>>>>>>
>>>>>> It doesn't really mean much to say "Flow API" is supported? So what is really
>>>>>> supported? It matters more what feature is supported.
>>>>>>
>>>>>> Since we are saying old method is deprecated, we can update the feature list of
>>>>>> drivers which implements filtering features using old method as not supported.
>>>>>> And that is the case with this patch since old APIs are marked as deprecated,
>>>>>> users can't use them to enable a filtering feature.
>>>>>>
>>>>>> Indeed I am for removing the "Flow API" from feature list, first it is not a
>>>>>> feature, second if it is only method to enable a filtering, and if filtering is
>>>>>> enabled in a driver, what is the point of redundant "Flow API" listing?
>>>>>>
>>>>>> I can make a quick patch if there is no objection, thanks.
>>>>>
>>>>> As I understand it was a decision to avoid details about flow API support
>>>>> in features matrix. Mainly because matrix would be really huge in
>>>>> attempt to represent it. The question is why filters/patterns mentioned
>>>>> above are better than others and should be mentioned.
>>>>> I'm not against adding some details, just want to understand criteria.
>>>>> Flexible and hash are definitely not well defined.
>>>>> What is flow director and which features should be supported to say yes?
>>>>>
>>>
>>>>
>>>> The criteria I have is what users will be interested about a device/driver.
>>>>
>>>> Will it be really huge to list filtering capabilities of the devices? I believe
>>>> we can group them into a few groups like above.
>>>> Or at least keep existing one and improve it by time and +1 to clarify them more
>>>> but that is something else.
>>>>
>>>> A device can have capabilities but it is not easy to find if that capability has
>>>> been enabled via DPDK, this kind of feature matrix works for it, and all
>>>> features together makes it much easier than digging datasheets and code.
>>>>
>>>> Saying that "Flow API" is enabled for a driver doesn't really gives any
>>>> information to the user if they are interested what kind of filtering features
>>>> are supported by that device/driver.
>>>
>>> I agree. I think, we need to enumerate rte flow patterns and actions
>>> supported by the PMD.
>>> Since there was no single place such documentation, we added the same
>>> in PMD documentation
>>> See 39.8. RTE Flow Support at https://doc.dpdk.org/guides/nics/octeontx2.html
>>>
>>> And IMO, We should not add deprecated features in the features matrix as
>>> new PMDs are not planning to implement the deprecated APIs. That
>>> makes, matrix looks
>>> new PMDs do not implement a lot of features, but in reality, those are
>>> deprecated and never planning to implement if even though HW supports it.
>>>
>>
>> +1 to not add deprecated features to the matrix, but those removed ones [1] are
>> not deprecated. Implementing them via "filter_ctrl" is deprecated. Below
>> features still can be implemented via "Flow API", that is why I am for adding
>> them back to default.ini.
> 
> Got it. Instead of [1], Can we document it as in the form of rte_flow
> semantics(patterns and actions) so
> that for the end-user it is very clear. Reason being:
> # Expressing "Tunnel filter" or "N-tupe filter" or "Flexible filter"
> or "Flow director" etc is very vague in rte_flow semantics
> and function is not just limited with above-fixed functions
> #  The new PMDs also can express the rte_flow aka "Flow API" support
> in the rte_flow semantics.

rte_flow is implementation detail, as well as 'filter_ctrl', I believe listing
rte_flow semantic will be too much detail for the feature table.

And end user may be interested in features, as if that drive/device support
"Flow Director" or not, instead of rte_flow semantic.

But I can see feature being vague is also problem, perhaps we can have rte_flow
level details in features.rst file, will it work?

> 
> 
>> And announce them as supported per PMD only if they are implemented via Flow API.
>>
>> [1]
>>  Ethertype filter     =
>>  N-tuple filter       =
>>  SYN filter           =
>>  Tunnel filter        =
>>  Flexible filter      =
>>  Hash filter          =
>>  Flow director        =