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