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

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

On 10/16/2019 11:08 AM, Jerin Jacob wrote:
> 
> 
> On Wed, 16 Oct, 2019, 3:32 PM Ferruh Yigit, <ferruh.yigit@intel.com
> <mailto:ferruh.yigit@intel.com>> wrote:
> 
>     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
>     <mailto: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?
> 
> 
> 
> +1 for adding rte_flow level level details in features.rst

OK, let me check this

> 
> IMO, Supported packet types(ptype) also good addition in features list.

"Packet type parsing" feature is already there,
http://lxr.dpdk.org/dpdk/v19.08/source/doc/guides/nics/features/default.ini#L53

If you mean the list of supported types, it is possible to get list on runtime
via an API, it will be hard to maintain that list in documentation.

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