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

[Jerin Jacob <jerinjacobk@gmail.com>]

On Wed, 16 Oct, 2019, 3:32 PM Ferruh Yigit, <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>
> 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

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


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