On Wed, Oct 25, 2017 at 11:44:39AM -0700, 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. > > > 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. >
Hi Ferruh, I won't comment on the rest of the discussion, as I see merit to both positions and I am still undecided about what is most interesting to the user. On your last question: For ether ops, the fail-safe relies on the public ether API of the sub-device eth_dev. For interrupts, the fail-safe relies on user request or on specific PMD support for RMV_EVENT: it doesn't matter if the other sub-device does not support the RMV event, if one PMD supports it it will be enabled. For offloads, the fail-safe will do a logical AND of offload amongst the sub-device PMDs and restrict those to this subset. If a plugged-in PMD has a different set of supported offloads, the fail-safe throws an error. Generally, sub-device PMDs are expected to closely match feature-wise. > > > >>> > >>>>> > >>>>>> OK for dropping "Stats per queue" > >>>>>> > >>>>>>> > >>>>> > >>> > > > -- Gaëtan Rivet 6WIND
