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

[Jerin Jacob <jerinjacobk@gmail.com>]

On Wed, 16 Oct, 2019, 3:46 PM Ferruh Yigit, <ferruh.yigit@intel.com> wrote:

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

Ok


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

Yes. I meant the list of supported types.
Ok. I will check the feasibility.


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