Re: [dpdk-dev] [PATCH] doc: update RSS action with best effort

[Andrew Rybchenko <arybchenko@solarflare.com>]

On 8/3/20 6:47 PM, Ori Kam wrote:
> 
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <arybchenko@solarflare.com>
>>
>> On 8/3/20 6:22 PM, Ori Kam wrote:
>>> Hi Andrew,
>>>
>>>> -----Original Message-----
>>>> From: Andrew Rybchenko <arybchenko@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 <orika@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?

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.

> 
>>>>> +
>>>>>  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.
>>>>>
>