From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <adrien.mazarguil@6wind.com>
Received: from mail-wr0-f194.google.com (mail-wr0-f194.google.com
[209.85.128.194]) by dpdk.org (Postfix) with ESMTP id CE50D7F2D
for <dev@dpdk.org>; Wed, 18 Apr 2018 16:16:55 +0200 (CEST)
Received: by mail-wr0-f194.google.com with SMTP id h3-v6so5319994wrh.5
for <dev@dpdk.org>; Wed, 18 Apr 2018 07:16:55 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=6wind-com.20150623.gappssmtp.com; s=20150623;
h=date:from:to:cc:subject:message-id:references:mime-version
:content-disposition:in-reply-to;
bh=tUSU7HoOQYYyLgmKrha+1fd3Q6w3PRHFDbhFxy12pcE=;
b=WHXN08fsr39pzK1rgNPLE0Y2TGiPNmNGfkl2IBtLwGUrjLF9dCmhDL6a9Gqi7wECvf
DcDN+U2iXJh4k/SG1gdUalVt6XE7b90UmUsHxEUFDxtdvIemqRBccmvDNi7TeywFoBVU
4wRqNjUZnCKCmof9Snup5Lap5Xq6+ye23y61Gur9HuYK5qFaqy+MHv5QYCztbd6IQIVh
4IPaQuFXjQtLtLIscmgNvCEnyxuTji/kP+UmMcBq31bvIAD8oQ1boTNA8lMWb9Zgk6Px
fTnnLzqXAqz2+h1hEI7Rr+rNHG9lhvHQWeFN4Xhv3Ro5a3S+nE66dpFM2CUL2EkrW7Wi
F2sw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20161025;
h=x-gm-message-state:date:from:to:cc:subject:message-id:references
:mime-version:content-disposition:in-reply-to;
bh=tUSU7HoOQYYyLgmKrha+1fd3Q6w3PRHFDbhFxy12pcE=;
b=d6Se5w9ZFdUY7HD7Us3qSjFHY3Rcine0i85vvSb9U8iy9wloABVe4PJpeWR9WvYRjI
hx+83EJhLqoyG69gqF65uc1VAH46yfrFfn/2uPF2hvCpI4kV0QqMlEwCdU8z/xJl0SXn
lQ33a1MOpRCZCBdnBt1aCF4rv+FNXz4nKXmgrJ3UiaO7h79aC8RS0MP6hi7LCU4WC8Yr
0a5YRLMZFK8Yg1Qph6BTx1GPK/v5TOm5PlRMlyrclOjLwCkjCrgQ7EKDCIeYlkUoEGQj
SMmT7WhP5nYxBsWBPQBd2OG3W3f+hgT/vlT7hHL3u6+6s2c1Cno5rfuNcxVDZ6aRfl40
XzpA==
X-Gm-Message-State: ALQs6tC2igC1wQRdEwirvNIvU3OuipIcAwQPDuDJ1RRpyLgGgIkMDYyQ
d2dt+KKkR8LH0jAgJ6reXU9y3A==
X-Google-Smtp-Source: AIpwx4/Ce8nU61LFZPvqUIX2NrGECrJ1XOvD9RQHvwxa6veDDqXFs5MAcEWyYnLCHPB5NWwllctYWw==
X-Received: by 2002:adf:9853:: with SMTP id
v77-v6mr1812240wrb.225.1524061015601;
Wed, 18 Apr 2018 07:16:55 -0700 (PDT)
Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78])
by smtp.gmail.com with ESMTPSA id m17-v6sm1099875wrh.3.2018.04.18.07.16.54
(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
Wed, 18 Apr 2018 07:16:54 -0700 (PDT)
Date: Wed, 18 Apr 2018 16:16:41 +0200
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: "Xueming(Steven) Li" <xuemingl@mellanox.com>
Cc: Shahaf Shuler <shahafs@mellanox.com>,
Nelio Laranjeiro <notifications@github.com>,
Wenzhuo Lu <wenzhuo.lu@intel.com>, Jingjing Wu <jingjing.wu@intel.com>,
Thomas Monjalon <thomas@monjalon.net>, "dev@dpdk.org" <dev@dpdk.org>
Message-ID: <20180418141641.GQ4957@6wind.com>
References: <20180409121035.148813-1-xuemingl@mellanox.com>
<20180418110648.22883-2-xuemingl@mellanox.com>
<20180418132557.GP4957@6wind.com>
<VI1PR05MB1678315C05209982D25FE7C6ACB60@VI1PR05MB1678.eurprd05.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <VI1PR05MB1678315C05209982D25FE7C6ACB60@VI1PR05MB1678.eurprd05.prod.outlook.com>
Subject: Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported
RSS hash types
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 18 Apr 2018 14:16:56 -0000
On Wed, Apr 18, 2018 at 01:54:20PM +0000, Xueming(Steven) Li wrote:
>
>
> > -----Original Message-----
> > From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > Sent: Wednesday, April 18, 2018 9:26 PM
> > To: Xueming(Steven) Li <xuemingl@mellanox.com>
> > Cc: Shahaf Shuler <shahafs@mellanox.com>; Nelio Laranjeiro <notifications@github.com>; Wenzhuo Lu
> > <wenzhuo.lu@intel.com>; Jingjing Wu <jingjing.wu@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
> > dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported RSS hash types
> >
> > Hi Xueming,
> >
> > On Wed, Apr 18, 2018 at 07:06:48PM +0800, Xueming Li wrote:
> > > "port config all rss all" command will fail on PMD that not support
> > > any of hard coding RSS hash types. This patch changed hard coding hash
> > > types to supported types retrieved from device info.
> > >
> > > Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> >
> > Problem is that this patch removes the ability to request "all" RSS types regardless of PMD support.
> >
> > Users will expect "all" to behave as documented, however doing so will only result in the limited set
> > of types reported by PMDs. For instance at least
> > mlx4 doesn't update the flow_type_rss_offloads field since it has never implemented configuration
> > support for the legacy RSS API.
> >
> > You should add an argument with a different name (e.g. "supported" or
> > "default") for that and keep the original meaning for "all".
> >
> > Testpmd documentation has to be updated accordingly.
>
> You must want to have a look at first part of this patchset:
> http://www.dpdk.org/ml/archives/dev/2018-April/097849.html
I did, however this is does not address my previous comment.
In this patch you're preventing testpmd users from requesting something. If
they want to enable "all" RSS types and some of them happen to be
unsupported on some ports, it means they want to get errors. Otherwise they
might think their request succeeded while it was in fact interpreted in an
undocumented manner.
The point is if users want to enable only "supported" RSS types, they
wouldn't write "all" on the command line right? A new parameter is needed
for that.
>
> >
> > One more nit below.
> >
> > > ---
> > > app/test-pmd/cmdline.c | 7 +++++++
> > > 1 file changed, 7 insertions(+)
> > >
> > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > > 512e3b55e..d357de7e6 100644
> > > --- a/app/test-pmd/cmdline.c
> > > +++ b/app/test-pmd/cmdline.c
> > > @@ -1998,6 +1998,7 @@ cmd_config_rss_parsed(void *parsed_result, {
> > > struct cmd_config_rss *res = parsed_result;
> > > struct rte_eth_rss_conf rss_conf = { .rss_key_len = 0, };
> > > + struct rte_eth_dev_info dev_info = {0};
> >
> > This could be declared in the new block where it's used. Also note that "{0}" is nonstandard syntax,
> > use memset() or initialize a field like rss_key_len above.
> >
> > > int diag;
> > > uint8_t i;
> > >
> > > @@ -2034,6 +2035,12 @@ cmd_config_rss_parsed(void *parsed_result,
> > > }
> > > rss_conf.rss_key = NULL;
> > > for (i = 0; i < rte_eth_dev_count(); i++) {
> > > + if (!strcmp(res->value, "all")) {
> > > + rte_eth_dev_info_get(i, &dev_info);
> > > + if (dev_info.flow_type_rss_offloads)
> > > + rss_conf.rss_hf =
> > > + dev_info.flow_type_rss_offloads;
> > > + }
> > > diag = rte_eth_dev_rss_hash_update(i, &rss_conf);
> > > if (diag < 0)
> > > printf("Configuration of RSS hash at ethernet port %d "
> > > --
> > > 2.13.3
> > >
> >
> >
> > --
> > Adrien Mazarguil
> > 6WIND
--
Adrien Mazarguil
6WIND