From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id F3AE01B3B8 for ; Tue, 7 Nov 2017 07:56:04 +0100 (CET) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Nov 2017 22:56:03 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,357,1505804400"; d="scan'208";a="1661819" Received: from kmsmsx152.gar.corp.intel.com ([172.21.73.87]) by orsmga001.jf.intel.com with ESMTP; 06 Nov 2017 22:56:02 -0800 Received: from pgsmsx103.gar.corp.intel.com ([169.254.2.203]) by KMSMSX152.gar.corp.intel.com ([169.254.11.206]) with mapi id 14.03.0319.002; Tue, 7 Nov 2017 14:56:01 +0800 From: "Zhao1, Wei" To: Adrien Mazarguil CC: "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v2 01/25] ethdev: introduce generic flow API Thread-Index: AQHSV7karC67xeKr+0C4PWEofdS44KLzBktggAykVYCACs+kgA== Date: Tue, 7 Nov 2017 06:56:01 +0000 Message-ID: References: <20171031174551.GV26782@6wind.com> In-Reply-To: <20171031174551.GV26782@6wind.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [172.30.20.206] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v2 01/25] ethdev: introduce generic flow API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Nov 2017 06:56:05 -0000 Hi, Adrien Thank you for your so details answer! Let me study your mail first then maybe I will supply some details=20 when I implement PMD code of MOVING rss to rte_flow for ixgbe. > -----Original Message----- > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com] > Sent: Wednesday, November 1, 2017 1:46 AM > To: Zhao1, Wei > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 01/25] ethdev: introduce generic flow A= PI >=20 > Hi Wei, >=20 > Sorry for the late answer, see below. >=20 > On Mon, Oct 23, 2017 at 08:53:49AM +0000, Zhao1, Wei wrote: > > > +/** > > + * 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? >=20 > The original idea was to provide the ability to share a single rss_conf s= tructure > between many flow rules and avoid redundant allocations. >=20 > It's based on the fact applications currently use a single RSS context fo= r > everything, they can easily make rss_conf point the global context found = in > struct rte_eth_dev. >=20 > Currently the testpmd flow command is incomplete, it doesn't support the > configuration of this field and always provides NULL to PMDs instead. Thi= s is > not documented in the flow API and may crash PMDs. >=20 > 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. >=20 > That's a problem that needs to be addressed in testpmd. >=20 > > 2. And also why the" const" is not need? We need a const ?How can we > config this parameter? >=20 > It means the allocation is managed outside of rte_flow, where the data ma= y > or may not be const, it's up to the application. The pointer stored in th= is > structure is only a copy. >=20 > 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. >=20 > It's an API issue I'd like to address by embedding the rss_conf parameter= s > directly in rte_flow_action_rss instead of doing so through a pointer. >=20 > > 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 ? >=20 > Basically you need to specify an additional "end" keyword to terminate th= e > queues list (tab completion shows it): >=20 > flow create 0 ingress pattern eth / tcp / end actions rss queues 0 1 2 e= nd / > end >=20 > There are other design flaws with the RSS action definition: >=20 > - RSS hash algorithm to use is missing, PMDs must rely on global paramete= rs. > - The C99-style flexible queues array is a super pain to manage and is no= t > supported by C++. I want to make it a normal pointer (the same applies = to > the RAW pattern item). >=20 > 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. >=20 > -- > Adrien Mazarguil > 6WIND