On 10/25/2017 11:44 AM, Ferruh Yigit wrote: > On 9/23/2017 10:55 PM, Matan Azrad wrote: >> Hi Ferruh >> >>> -----Original Message----- >>> From: Ferruh Yigit [mailto:ferruh.yi...@intel.com] >>> Sent: Friday, September 22, 2017 1:32 PM >>> To: Matan Azrad <ma...@mellanox.com>; Gaetan Rivet >>> <gaetan.ri...@6wind.com> >>> Cc: dev@dpdk.org; john.mcnam...@intel.com >>> Subject: Re: [dpdk-dev] [PATCH] doc: update failsafe feature list >>> >>> On 9/19/2017 12:39 PM, Matan Azrad wrote: >>>> Hi Ferruh >>>> >>>>> -----Original Message----- >>>>> From: Ferruh Yigit [mailto:ferruh.yi...@intel.com] >>>>> Sent: Tuesday, September 19, 2017 2:00 PM >>>>> To: Matan Azrad <ma...@mellanox.com>; Gaetan Rivet >>>>> <gaetan.ri...@6wind.com> >>>>> Cc: dev@dpdk.org; john.mcnam...@intel.com >>>>> Subject: Re: [dpdk-dev] [PATCH] doc: update failsafe feature list >>>>> >>>>> On 9/19/2017 11:04 AM, Matan Azrad wrote: >>>>>> >>>>>> Hi Ferruh >>>>>> >>>>>>> -----Original Message----- >>>>>>> From: Ferruh Yigit [mailto:ferruh.yi...@intel.com] >>>>>>> Sent: Tuesday, September 19, 2017 12:27 PM >>>>>>> To: Matan Azrad <ma...@mellanox.com>; Gaetan Rivet >>>>>>> <gaetan.ri...@6wind.com> >>>>>>> Cc: dev@dpdk.org >>>>>>> Subject: Re: [dpdk-dev] [PATCH] doc: update failsafe feature list >>>>>>> >>>>>>> On 9/14/2017 4:32 PM, Matan Azrad wrote: >>>>>>>> Add supported failsafe features to feature list. >>>>>>>> Remove stats per queue feature from failsafe feature list since >>>>>>>> queue_stats_mapping_set dev op has not implemented yet. >>>>>>>> >>>>>>>> Signed-off-by: Matan Azrad <ma...@mellanox.com> >>>>>>>> --- >>>>>>>> doc/guides/nics/features/failsafe.ini | 15 ++++++++++++++- >>>>>>>> 1 file changed, 14 insertions(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/doc/guides/nics/features/failsafe.ini >>>>>>>> b/doc/guides/nics/features/failsafe.ini >>>>>>>> index a42e344..9f48455 100644 >>>>>>>> --- a/doc/guides/nics/features/failsafe.ini >>>>>>>> +++ b/doc/guides/nics/features/failsafe.ini >>>>>>>> @@ -4,20 +4,33 @@ >>>>>>>> ; Refer to default.ini for the full list of available PMD features. >>>>>>>> ; >>>>>>>> [Features] >>>>>>>> +Speed capabilities = Y >>>>>>>> Link status = Y >>>>>>>> Link status event = Y >>>>>>>> MTU update = Y >>>>>>>> Jumbo frame = Y >>>>>>>> +Scattered Rx = Y >>>>>>>> +LRO = Y >>>>>>>> +TSO = Y >>>>>>>> Promiscuous mode = Y >>>>>>>> Allmulticast mode = Y >>>>>>>> Unicast MAC filter = Y >>>>>>>> Multicast MAC filter = Y >>>>>>>> VLAN filter = Y >>>>>>>> +Ethertype filter = Y >>>>>>>> +N-tuple filter = Y >>>>>>>> +SYN filter = Y >>>>>>>> +Tunnel filter = Y >>>>>>>> +Flexible filter = Y >>>>>>>> +Hash filter = Y >>>>>>>> +Flow director = Y >>>>>>>> Flow control = Y >>>>>>>> Flow API = Y >>>>>>>> +QinQ offload = Y >>>>>>>> +L3 checksum offload = Y >>>>>>>> +L4 checksum offload = Y >>>>>>>> Packet type parsing = Y >>>>>>>> Basic stats = Y >>>>>>>> -Stats per queue = Y >>>>>>>> ARMv7 = Y >>>>>>>> ARMv8 = Y >>>>>>>> Power8 = Y >>>>>>> >>>>>>> I am not sure if claiming support for these features is correct. >>>>>>> Failsafe itself doesn't provide these features, but relies >>>>>>> underlying hardware which we don't really know what they supports >>>>>>> or not in this >>>>> stage. >>>>>>> >>>>>> >>>>>> Don't you think that almost all failsafe features rely underlying >>>>>> hardware or >>>>> sub PMDs? >>>>> >>>>> You are right, perhaps we should remove all. This is helpful to show >>>>> what device features are supported. For failsafe, is this information >>> useful? >>>>> >>>> Since there are features that failsafe cannot support without any sub >>>> PMD dependences (for example "Stats per queue") it is useful. >>> >>> Sorry, I missed your point. >>> >>> Device feature list documentation is good for: >>> - End user can easily see what to expect from a device/driver. >>> - To trace what features implemented for a device. >>> - To find out which device has a specific desired feature. >>> >>> For failsafe, it is a virtual overlay device on other physical devices. >>> >>> The supported architectures and provided documents features can be >>> useful. But why/how NIC related features can be useful since all they are >>> coming form underlay devices? >>> >> My point is that someone can understand from this list all failsafe PMD >> features which are not >> supported even if the failsafe sub devices PMD may support them. > > It may be possible to document them as "Feature = N" if you sure they are not > supported, > instead of saying the features that may or may not be supported as supported.
Nak for the patch, its status updated in patchwork. > >> Please read section 31.1 in failsafe documentation: >> http://dpdk.org/doc/guides/nics/fail_safe.html >> Actually, the failsafe supported features is the logical AND between all its >> sub devices supported features and failsafe default features. >> I think this table should reflect the failsafe default features. > > So if any PMD doesn't support that feature, it won't be supported, right? If > so > why we are documented it as supported. > > Agree to document default features independent from what PMD supports, as I > said > before "supported architectures and provided documents" etc.. > >> It is very useful for failsafe user to compare failsafe features and sub >> devices features to infer which feature is going to be supported with >> failsafe combination. >> >> In addition, >> Even if the PMD part is only to set capability bit (NIC related features >> capability) user must know that failsafe is going to set it. > > Still this depends on if PMD supports it or not. > > Out of curiosity, how failsafe set the underlying PMD features, automatically > or > based on user request? I guess it checks that all PMDs support feature before > setting it. > >> >>>> >>>>>> >>>>>>> OK for dropping "Stats per queue" >>>>>>> >>>>>>>> >>>>>> >>>> >> >
