Hi Adrien: > -----Original Message----- > From: Adrien Mazarguil [mailto:adrien.mazarg...@6wind.com] > Sent: Thursday, July 4, 2019 10:09 PM > To: Zhang, Qi Z <qi.z.zh...@intel.com> > Cc: Su, Simei <simei...@intel.com>; Wu, Jingjing <jingjing...@intel.com>; > Xing, Beilei <beilei.x...@intel.com>; Yang, Qiming <qiming.y...@intel.com>; > dev@dpdk.org > Subject: Re: [dpdk-dev] [RFC] ethdev: support input set change by RSS action > > On Thu, Jul 04, 2019 at 01:55:14PM +0000, Zhang, Qi Z wrote: > > > > > > > -----Original Message----- > > > From: Adrien Mazarguil [mailto:adrien.mazarg...@6wind.com] > > > Sent: Thursday, July 4, 2019 5:07 PM > > > To: Su, Simei <simei...@intel.com> > > > Cc: Zhang, Qi Z <qi.z.zh...@intel.com>; Wu, Jingjing > > > <jingjing...@intel.com>; Xing, Beilei <beilei.x...@intel.com>; Yang, > > > Qiming <qiming.y...@intel.com>; dev@dpdk.org > > > Subject: Re: [dpdk-dev] [RFC] ethdev: support input set change by > > > RSS action > > > > > > On Thu, Jul 04, 2019 at 12:47:09PM +0800, simei wrote: > > > > From: Simei Su <simei...@intel.com> > > > > > > > > This RFC introduces inputset structure to rte_flow_action_rss to > > > > support input set specific configuration by rte_flow RSS action. > > > > > > > > We can give an testpmd command line example to make it more clear. > > > > > > > > For example, below flow selects the l4 port as inputset for any > > > > eth/ipv4/tcp packet: #flow create 0 ingress pattern eth / ipv4 / > > > > tcp / end actions rss inputset tcp src mask 0xffff dst mask 0xffff > > > > /end > > > > > > > > Signed-off-by: Simei Su <simei...@intel.com> > > > > --- > > > > lib/librte_ethdev/rte_flow.h | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/lib/librte_ethdev/rte_flow.h > > > > b/lib/librte_ethdev/rte_flow.h index f3a8fb1..2a455b6 100644 > > > > --- a/lib/librte_ethdev/rte_flow.h > > > > +++ b/lib/librte_ethdev/rte_flow.h > > > > @@ -1796,6 +1796,9 @@ struct rte_flow_action_rss { > > > > uint32_t queue_num; /**< Number of entries in @p queue. */ > > > > const uint8_t *key; /**< Hash key. */ > > > > const uint16_t *queue; /**< Queue indices to use. */ > > > > + struct rte_flow_item *inputset; /** Provide more specific > > > > +inputset > > > configuration. > > > > + * ignore spec, only mask. > > > > + */ > > > > }; > > > > > > > > /** > > > > > > To make sure I understand, is this kind of a more flexible version > > > of rte_flow_action_rss.types? > > > > Yes > > > > > > For instance while specifying .types = ETH_RSS_IPV4 normally covers > > > both source and destination addresses, does this approach enable > > > users to perform RSS on source IP only? > > > > Yes, .it is the case to select any subset of 5 tuples or even tunnel > > header's id for hash > > > > > In which case, what value does the Toeplitz algorithm assume for the > > > destination, 0x0? (note: must be documented) > > > > My understanding is src/dst pair is only required for a symmetric case > > But for Toeplitz, it is just a hash function, it process a serial of > > data with specific algorithm, have no idea about which part is src and dst > > , So > for ip src only with Toeplitz, dst is not required to be a placeholder.. > > Right, I had symmetric Toeplitz in mind and wondered what would happen > when users would not select the required fields. I guess the PMD would have > to reject unsupported combinations. > > > anything I missed, would you share more insight? > > No, that answers the question, thanks. > > Now what about my suggestion below? In short: extending ETH_RSS_* > assuming there's enough bits left in there, instead of adding a whole new > framework and breaking ABI in the process.
Since the hardware can support any combination of input set (not just 5 tuples), we'd like to make it more generic. Those will be some pain due to ABI break, but we think its worth for long term. Thanks Qi > > > > My opinion is that, unless you know of a hardware which can perform > > > RSS on random bytes of a packet, this approach is a bit overkill at this > point. > > > > > > How about simply adding the needed ETH_RSS_* definitions (e.g. > > > ETH_RSS_IPV4_(SRC|DST))? How many are needed? > > > > > > There are currently 20 used bits and struct > > > rte_flow_action_rss.types is 64-bit wide. I'm sure we can manage > > > something without harming the ABI. Even better, you wouldn't need a > deprecation notice. > > > > > > If you use the suggested approach, please update testpmd and its > > > documentation as part of the same patch, thanks. > > -- > Adrien Mazarguil > 6WIND
