From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f170.google.com (mail-wr0-f170.google.com [209.85.128.170]) by dpdk.org (Postfix) with ESMTP id 8C0407F58 for ; Wed, 18 Apr 2018 16:47:45 +0200 (CEST) Received: by mail-wr0-f170.google.com with SMTP id s18-v6so5591963wrg.9 for ; Wed, 18 Apr 2018 07:47:45 -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=ujaB55Va0xxIe5WDeBSPm71sXeO4R4Sy1zr9YjyAukk=; b=mUi/aSiKFFmAzFXjKmdVJs6VTE9o4ovFb/cwBTsUuthMliSe5I/VWLBQZXX9qTuv3b Ne8S9wUU3/vWEBlrOJotVHR+J0mQRi4sABYLxok9F9HojCXTS48XrGkX2LC07Ai1Cc1e TsnhpQVCZj95Oq7rAFISul3ZF6HfwGHgqKrrTnSWhD5XyBrLpH8f9l6BSeYSYwsvHARJ vtUNn0YZZcC3fwxqhgD9x6cCUxE0vM+c3w2igphkh+0TeLp0wWtyyFjqRIqIDe4vQsey /xsBtiy3BySAfq7e87w3WhWfIKoe+o9PZLNGIfrmF6mAD+NdCQ1o9gyfw+prXwA2mu7R 9FJA== 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=ujaB55Va0xxIe5WDeBSPm71sXeO4R4Sy1zr9YjyAukk=; b=dfSqaKgCsNGfKgvX2RrOvaNB5+DQt87r6K1arRZEu4ZMG/x02f1YCTYKdpYdis5KDh iXGEPtY9jxLk/uZWe2nMsFIpAOiUAh9cmRtNRpsdFrYK5BoHhIXLEFiaUqNZVA3DxHuv DNrdH8GeZtq+HlLSfFMWYBVAHTRnsZkazu1Tp/0Ky6/HMiJIpvOrdqH6S74m2Bf44aeC /byXiAG7SN1wEOhs2GkBaHH5vL/xjHLh+5e4pHkaTC3kNToJ/YyX9bIFpd77rvdewRa1 585ZvycVzbYnIE4EiX+t0qAqAtpTDBqBT+bXC6Ni91MHTQss/cz4VAS2mRtBvDBx0+PW ENeQ== X-Gm-Message-State: ALQs6tC4OETGH+NtDKG+DdVclwDEcP9Y4Sip67FB1kPwbby+vscRAnC5 viD+CzaVe5oGpi4hU3mP6fzu1Q== X-Google-Smtp-Source: AIpwx4/AkOI4hl150aSPw548SAEaTFFpd3ws5810KtbTdmjUVq6qRe8WESnxbIbIxxqmH2bIlc57dw== X-Received: by 10.28.198.77 with SMTP id w74mr2054269wmf.36.1524062865302; Wed, 18 Apr 2018 07:47:45 -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 2sm1619334wml.2.2018.04.18.07.47.44 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 18 Apr 2018 07:47:44 -0700 (PDT) Date: Wed, 18 Apr 2018 16:47:31 +0200 From: Adrien Mazarguil To: "Xueming(Steven) Li" Cc: Shahaf Shuler , Nelio Laranjeiro , Wenzhuo Lu , Jingjing Wu , Thomas Monjalon , "dev@dpdk.org" , Olivier Matz Message-ID: <20180418144731.GT4957@6wind.com> References: <20180409121035.148813-1-xuemingl@mellanox.com> <20180418110648.22883-2-xuemingl@mellanox.com> <20180418132557.GP4957@6wind.com> <20180418141641.GQ4957@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 Apr 2018 14:47:45 -0000 On Wed, Apr 18, 2018 at 02:26:30PM +0000, Xueming(Steven) Li wrote: > +Olivier > > > -----Original Message----- > > From: Adrien Mazarguil > > Sent: Wednesday, April 18, 2018 10:17 PM > > To: Xueming(Steven) Li > > Cc: Shahaf Shuler ; Nelio Laranjeiro ; Wenzhuo Lu > > ; Jingjing Wu ; Thomas Monjalon ; > > dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v3 2/2] app/testpmd: only config supported RSS hash types > > > > On Wed, Apr 18, 2018 at 01:54:20PM +0000, Xueming(Steven) Li wrote: > > > > > > > > > > -----Original Message----- > > > > From: Adrien Mazarguil > > > > Sent: Wednesday, April 18, 2018 9:26 PM > > > > To: Xueming(Steven) Li > > > > Cc: Shahaf Shuler ; Nelio Laranjeiro > > > > ; Wenzhuo Lu ; > > > > Jingjing Wu ; Thomas Monjalon > > > > ; 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 > > > > > > > > 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: > > > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww. > > > dpdk.org%2Fml%2Farchives%2Fdev%2F2018-April%2F097849.html&data=02%7C01 > > > %7Cxuemingl%40mellanox.com%7C497583093d324a6861f708d5a5370a8f%7Ca65297 > > > 1c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636596578186582002&sdata=uNVw2egf > > > 0rcTHazCuoMelNrY44lZVfd6Ado1bJfzg8s%3D&reserved=0 > > > > I did, however this is does not address my previous comment. > > In first patch, if flow_type_rss_offloads not set, any RSS will not be allowed. > We need to clarify definition of empty flow_type_rss_offloads, not set or no rss support. It's fine, mlx4 does not implement rte_eth_dev_rss_hash_update() either (legacy RSS configuration is not implemented at all). With mlx4, the command: port config all rss (all|anything else) Always returns an error. It's a good thing because it's not implemented and users must be made aware of that fact. No need to lie about it. > > 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 -- Adrien Mazarguil 6WIND