Hi, Adrien Thank you for your so details answer! Let me study your mail first then maybe I will supply some details when I implement PMD code of MOVING rss to rte_flow for ixgbe.
> -----Original Message----- > From: Adrien Mazarguil [mailto:adrien.mazarg...@6wind.com] > Sent: Wednesday, November 1, 2017 1:46 AM > To: Zhao1, Wei <wei.zh...@intel.com> > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 01/25] ethdev: introduce generic flow API > > Hi Wei, > > Sorry for the late answer, see below. > > On Mon, Oct 23, 2017 at 08:53:49AM +0000, Zhao1, Wei wrote: > <snip> > > +/** > > + * RTE_FLOW_ACTION_TYPE_RSS > > + * > > + * Similar to QUEUE, except RSS is additionally performed on packets > > +to > > + * spread them among several queues according to the provided > parameters. > > + * > > + * Note: RSS hash result is normally stored in the hash.rss mbuf > > +field, > > + * however it conflicts with the MARK action as they share the same > > + * space. When both actions are specified, the RSS hash is discarded > > +and > > + * PKT_RX_RSS_HASH is not set in ol_flags. MARK has priority. The > > +mbuf > > + * structure should eventually evolve to store both. > > + * > > + * Terminating by default. > > + */ > > +struct rte_flow_action_rss { > > + const struct rte_eth_rss_conf *rss_conf; /**< RSS parameters. */ > > + uint16_t num; /**< Number of entries in queue[]. */ > > + uint16_t queue[]; /**< Queues indices to use. */ }; > > + > > > > I am plan for moving rss to rte_flow. > > May I ask you some question for this struct of rte_flow_action_rss. > > 1. why do you use pointer mode for rss_conf, why not use " struct > rte_eth_rss_conf rss_conf "? > > we need to fill these rss info which get from CLI to this struct, if we use > > the > pointer mode, how can we fill in these info? > > The original idea was to provide the ability to share a single rss_conf > structure > between many flow rules and avoid redundant allocations. > > It's based on the fact applications currently use a single RSS context for > everything, they can easily make rss_conf point the global context found in > struct rte_eth_dev. > > Currently the testpmd flow command is incomplete, it doesn't support the > configuration of this field and always provides NULL to PMDs instead. This is > not documented in the flow API and may crash PMDs. > > For the time being, a PMD can interpret NULL as a kind of default global > rss_conf parameters, as is done in both mlx5 and mlx4 PMDs. > > That's a problem that needs to be addressed in testpmd. > > > 2. And also why the" const" is not need? We need a const ?How can we > config this parameter? > > It means the allocation is managed outside of rte_flow, where the data may > or may not be const, it's up to the application. The pointer stored in this > structure is only a copy. > > Whether the pointed data remains allocated once the flow rule is created is > unspecified. A PMD cannot assume anything and has to copy these > parameters if needed later. > > It's an API issue I'd like to address by embedding the rss_conf parameters > directly in rte_flow_action_rss instead of doing so through a pointer. > > > 3. what is your expect mode for CLI command for rss config? When I type > in : > > " flow create 0 pattern eth / tcp / end actions rss queues queue 0 /end " > > Or " flow create 0 pattern eth / tcp / end actions rss queues {0 1 2} /end " > > I get " Bad arguments ". > > > > So, the right CLI command is ? > > Basically you need to specify an additional "end" keyword to terminate the > queues list (tab completion shows it): > > flow create 0 ingress pattern eth / tcp / end actions rss queues 0 1 2 end / > end > > There are other design flaws with the RSS action definition: > > - RSS hash algorithm to use is missing, PMDs must rely on global parameters. > - The C99-style flexible queues array is a super pain to manage and is not > supported by C++. I want to make it a normal pointer (the same applies to > the RAW pattern item). > > These changes are planned for the next overhaul of rte_flow, I'll submit a > RFC for them as soon as I get the chance. > > -- > Adrien Mazarguil > 6WIND
