From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f194.google.com (mail-wr0-f194.google.com [209.85.128.194]) by dpdk.org (Postfix) with ESMTP id E25FD6CCA for ; Wed, 18 Apr 2018 16:30:22 +0200 (CEST) Received: by mail-wr0-f194.google.com with SMTP id s18-v6so5430041wrg.9 for ; Wed, 18 Apr 2018 07:30:22 -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=6Eo7Jsaz5PNHPl9zkyInJc798qNgHFVtM6JdC5BXssI=; b=rO3eTSc4jyQsZzT079zrjTzYkDRDE/kUAGAMtb5q+TwsHR/9I5GsQVjdIkr6O4K/1W ScfSAGSCskI/2OdeYI0wCjAXkasAukYx05X25iHtSRBAnMrXpGWw2Fh34wH7eYLAKjv+ 1DYHHlgaiWnR/lsWY613Ak5DQaUz6mNwmRnYlJhqtDZ/HtOmsz/EFjrg1TL9lwXunleE zmEJ5M5FvqAtT5fJZ/LJJ9yNMFoJ1MXE3X0941zEHcwO5fdNk/wcytCYfawW+Dfn10Ba 1ZkN7G0Uk3D+2hFwyUjuv94eUVps52Kobg6uK6NXU3Kyikk8KNQ/yY1JembIkrSl9Jdg UMsw== 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=6Eo7Jsaz5PNHPl9zkyInJc798qNgHFVtM6JdC5BXssI=; b=YSJhsmeTPo+uYA7he/VDjT1jPFio27CFxQEdoGXJlibFharers7sNxdu29kO7elOu1 jkX/vhqzXdIJ55WzSlQO7W8kCTAZNG7+wKfbV7eqtKCUqepBXkDLcYAGvWzjD51qlJgu pvjKzHjFznV+R2N78WThFOTE9YJZ90qY8ESuX5vs8TtbQbE8MNI29clXQ2LIRHEDALvf 89Z+96vM+eQMzVjc61QgCY47HWyAJqVh58NqCNQQeGTW2N04EINgYPiLMYverrgSYqUW ZSrNehdVUcvzzDXUXOzr2hm416b2h70thnFRuqExzmb2Va8iYC6IYjSXN7ZkjaK12okP r4Uw== X-Gm-Message-State: ALQs6tDCYp6jmKewNQZ22FhIIhIf1SGFZ7fULVXeb9TCqgIMhikcH+cp YwCGr8f6BuulrIBgDmpP3wWHfg== X-Google-Smtp-Source: AIpwx48MpSkOf2slNKIHMFLp8U2xEh1iLVsrDNbCjJMO4SVzOSA9eTFaeu3eEJaWwrFljK7qNN/+3Q== X-Received: by 10.28.22.198 with SMTP id 189mr1841986wmw.5.1524061822693; Wed, 18 Apr 2018 07:30:22 -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 c141sm2624282wmh.21.2018.04.18.07.30.21 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 18 Apr 2018 07:30:21 -0700 (PDT) Date: Wed, 18 Apr 2018 16:30:08 +0200 From: Adrien Mazarguil To: "Xueming(Steven) Li" Cc: Shahaf Shuler , Nelio Laranjeiro , Wenzhuo Lu , Jingjing Wu , Thomas Monjalon , "dev@dpdk.org" Message-ID: <20180418143008.GR4957@6wind.com> References: <20180409121035.148813-1-xuemingl@mellanox.com> <20180418110648.22883-2-xuemingl@mellanox.com> <20180418132557.GP4957@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:30:23 -0000 On Wed, Apr 18, 2018 at 02:10:45PM +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. > > Keeping it "all" as it is will result in error for PMDs not completely support "all" RSS types and this > always confuse people. You're redefining the commonly accepted meaning of "all". The first instance of "all" in the "port config all rss all" command means "all ports", not "only ports that happen to implement the RSS configuration callback". Same for all other testpmd commands accepting "all" as a parameter anywhere. Those that don't will return errors, it's fine. > How about adding a check, if flow_type_rss_offloads not set, ignore and set default RSS "all" types? Results would be even less predictable. Keep it simple. Testpmd is a testing tool, if PMD developers want to check error-spitting abilities of their PMDs, they must be able to. Adding a dedicated parameter is one way to satisfy everyone, dropping this patch is another. > > 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. > > > > 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