Hi Andrew, > -----Original Message----- > From: Andrew Rybchenko <arybche...@solarflare.com> > 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.
Will rephrase. > > > > >>> + > >>> +- 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. > Will update the doc. > > > > > >>> 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. > >>> > >
