Hi Andrew, > -----Original Message----- > From: Andrew Rybchenko <arybche...@solarflare.com> > > On 8/3/20 6:47 PM, Ori Kam wrote: > > > > > >> -----Original Message----- > >> From: Andrew Rybchenko <arybche...@solarflare.com> > >> > >> On 8/3/20 6:22 PM, Ori Kam wrote: > >>> Hi Andrew, > >>> > >>>> -----Original Message----- > >>>> From: Andrew Rybchenko <arybche...@solarflare.com> > >>>> > >>>> On 8/3/20 5:28 PM, Ori Kam wrote: > >>>>> Using the rte_flow action RSS types field, > >>>>> may result in an undefined outcome. > >>>>> > >>>>> For example selecting both UDP and TCP, > >>>>> selecting TCP RSS type but the pattern is targeting UDP traffic. > >>>>> another option is that the PMD doesn't support all requested types. > >>>>> > >>>>> Until now, it wasn't clear what will happen in such cases. > >>>>> This commit clarify this issue by stating that the PMD > >>>>> will work in the best-effort mode. > >>>>> > >>>>> Signed-off-by: Ori Kam <or...@mellanox.com> > >>>>> --- > >>>>> doc/guides/prog_guide/rte_flow.rst | 10 ++++++++++ > >>>>> 1 file changed, 10 insertions(+) > >>>>> > >>>>> diff --git a/doc/guides/prog_guide/rte_flow.rst > >>>> b/doc/guides/prog_guide/rte_flow.rst > >>>>> index 3e5cd1e..d000560 100644 > >>>>> --- a/doc/guides/prog_guide/rte_flow.rst > >>>>> +++ b/doc/guides/prog_guide/rte_flow.rst > >>>>> @@ -1735,6 +1735,16 @@ unspecified "best-effort" settings from the > >>>> underlying PMD, which depending > >>>>> on the flow rule, may result in anything ranging from empty (single > queue) > >>>>> to all-inclusive RSS. > >>>>> > >>>>> +Best effort will be used, in case there is a conflict inside the rule. > >>>>> +The conflict can be the result of: > >>>>> + > >>>>> +- Conflicting RSS types, for example setting both UDP and TCP. > >>>>> + > >>>>> +- Conflicting between RSS types and the requested pattern to match, > >>>>> + for example matching on UDP and hashing RSS on TCP. > >>>> IMHO it is not a problem at all, but good to clarify anyway. > >>>> > >>> Agree this patch is just for clarification. > >>> > >>>>> + > >>>>> +- Hashing on types that are not supported by the PMD. > >>>> Shouldn't it return error to the caller? > >>>> > >>> That’s depends, if for example the application requested eth and IPv4, > >>> and the PMD can do only IPv4 so according to this, the PMD will just do > IPv4. > >> > >> I think it is a bad behaviour. Application has required information to > >> provide correct parameters and therefore incorrect should be rejected. > >> Am I missing something? > >> > > In RTE flow you can't know what are the parameters that are supported. > > One option is to fail the rule, but this means failing the rule even if > > some of > them are supported. > > Or we can choose to fail if all the types are unsupported, but this will > > result in > miss match between > > adding two types and adding just one type. For example the PMD doesn't > support RSS on eth > > and the application request RSS on ETH and IPv4 so the PMD will do IPv4 RSS, > > while if the user selects only ETH the rule will fail. > > Same this is exactly the same case as miss match the application requests > something > > that the PMD can't supply. > > In any case I think this is the current behavior of the PMD so this just > > clarify it. > > I'm sorry, but the PMD is hardly applicable here. > Do you mean the "mlx5" PMD? Or have you reviewed all PMDs? > I assume that each PMD as its own limitations, I'm sure that no PMD can support all possible RSS. But maybe there is such perfect PMD.
> No I understand that the topic is more complicated, but > covering complex cases should not spoil simple. > I think that information provided in dev_info should > specify all supported protocols/fields. > Yes, it is acceptable to behave best effort if requested > field is supported in general, but it not for the flow. > Please, make it a bit clearer in the suggested description. > So what about the following wording: (after the current part) If requested RSS hash type is not supported, and no supported hash function is requested then the PMD may reject the flow. > > > >>>>> + > >>>>> Note: RSS hash result is stored in the ``hash.rss`` mbuf field which > >>>>> overlaps ``hash.fdir.lo``. Since `Action: MARK`_ sets the > >>>>> ``hash.fdir.hi`` > >>>>> field only, both can be requested simultaneously. > >>>>> > >
