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