On 8/5/20 5:08 PM, Ori Kam wrote: > Hi Andrew, > >> -----Original Message----- >> From: Andrew Rybchenko <arybche...@solarflare.com> >> >> On 8/4/20 11:13 AM, Ori Kam wrote: >>> Using the rte_flow action RSS types field, >>> may result in 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> >>> --- >>> v2: >>> * Based on ML, update that using only unsupported hash type >>> may cause the flow to be rejected by PMD. >>> --- >>> doc/guides/prog_guide/rte_flow.rst | 11 +++++++++++ >>> 1 file changed, 11 insertions(+) >>> >>> diff --git a/doc/guides/prog_guide/rte_flow.rst >> b/doc/guides/prog_guide/rte_flow.rst >>> index 3e5cd1e..d730b66 100644 >>> --- a/doc/guides/prog_guide/rte_flow.rst >>> +++ b/doc/guides/prog_guide/rte_flow.rst >>> @@ -1735,6 +1735,17 @@ 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. >> >> Thinking more about it, I see no conflict at all. It is common >> to specify both TCP and UDP in hash function if user wants to >> take TCP ports for TCP packets and UDP ports for UDP into >> account. >> > I fully agree with you that it is common to use both UDP and TCP and > expect it to work based on the traffic. this is the point of this patch. > To clarify that this is valid input and the PMD will work in best effort mode.
Just don't name it "Conflicting RSS types", may be "Non-applicable RSS types", but it collapses two bullets into one. > >>> + >>> +- Conflicting between RSS types and the requested pattern to match, >>> + for example matching on UDP and hashing RSS on TCP. >> >> I'd not name it a conflict either. It is just common rule when >> applicable fields are used only. >> > Just like my comment from above. > >>> + >>> +If requested RSS hash type is not supported, >>> +and no supported hash type is requested then the PMD may reject the flow. >>> + >> >> I disagree with such description. If requested RSS hash type is >> not supported (not present in dev_info.flow_type_rss_offloads), >> it must be rejected and error returned. >> If requested RSS hash type is not supported for a packet >> matching the rule, but supported in general (present in >> dev_info.flow_type_rss_offloads), part of RSS hash type >> specification may be ignored and only applicable bits are used. >> If result is empty, PMD may reject the flow rule. >> > The flow should be rejected even if it is used with some type that is > supported? Yes, if user ask for something what is not supported at all, it is a problem and user should be notified. May be it is too restrictive, but I'd prefer this way. User has all required information in dev_info and can remove unsupported hash types from request. IMHO, it should enforce user check the result to be not empty and handle it properly. > > >>> 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. >>> >
