On 11/1/2017 10:53 PM, Matan Azrad wrote: > Hi Ferruh, Gaetan > >> -----Original Message----- >> From: Ferruh Yigit [mailto:ferruh.yi...@intel.com] >> Sent: Thursday, November 2, 2017 12:34 AM >> 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 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. >> > Why? > It should be superseded later, the stats per queue should be removed any way. > Moreover, I think we should solve it in a different way if you don't agree > to mark them as supported. > Maybe we can add special sign to this table or add star in failsafe > name(maybe change color) and explain there that any supported feature by > failsafe depends on sub devices support. > User must know all the features allowed by failsafe otherwise we should > remove all of the failsafe feature in this table(because all of them depend > on sub devices support ). > What do you think?
This version of the patch was not moving, no more comments no updates, that is why I would like take it out off list. Please feel free to send a new version and we can continue to discuss other options as well. > >>> >>>> Please read section 31.1 in failsafe documentation: >>>> >> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdpd >>>> >> k.org%2Fdoc%2Fguides%2Fnics%2Ffail_safe.html&data=02%7C01%7Cmatan >> %40m >>>> >> ellanox.com%7C3ad1faf403e14c85ad6508d5217899ea%7Ca652971c7d2e4d9b >> a6a4 >>>> >> d149256f461b%7C0%7C0%7C636451724233800703&sdata=i1si8wQIGS3GSqG >> WI2LyK >>>> 19N6oUMeVA07rzkmSkgZXk%3D&reserved=0 >>>> 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" >>>>>>>>> >>>>>>>>>> >>>>>>>> >>>>>> >>>> >>> >
