From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id C1347A0588; Thu, 16 Apr 2020 09:51:53 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id DAAB21DAD1; Thu, 16 Apr 2020 09:51:52 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 6651D1DACB for ; Thu, 16 Apr 2020 09:51:51 +0200 (CEST) IronPort-SDR: dpibGclfPidyZGz7hdC64x88NEptKl8PNbOcRox4m9TiJpi3A5boQxg4Sg3q7A5a8pUEt3rW4X XT+bkYa0Qpgw== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Apr 2020 00:51:50 -0700 IronPort-SDR: CsudxSD+CgN1nRPsCC4nZRG1Jt8ek4qvLfg1JlP3bAJsDoqgbHpyDk+SvaZgKwl7aT0SusSJWl zFOIvcpKzf8g== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,390,1580803200"; d="scan'208";a="299233356" Received: from jguo15x-mobl.ccr.corp.intel.com (HELO [10.67.68.153]) ([10.67.68.153]) by FMSMGA003.fm.intel.com with ESMTP; 16 Apr 2020 00:51:46 -0700 To: "Iremonger, Bernard" , "orika@mellanox.com" , "Ye, Xiaolong" , "Zhang, Qi Z" Cc: "dev@dpdk.org" , "Wu, Jingjing" , "Cao, Yahui" , "Su, Simei" References: <20200318170401.7938-5-jia.guo@intel.com> <20200415171129.86297-1-jia.guo@intel.com> <20200415171129.86297-4-jia.guo@intel.com> From: Jeff Guo Message-ID: Date: Thu, 16 Apr 2020 15:51:45 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Subject: Re: [dpdk-dev] [dpdk-dev v5 3/3] app/testpmd: add new types to RSS hash commands 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" hi, bernard On 4/15/2020 11:01 PM, Iremonger, Bernard wrote: > Hi Jeff, > >> -----Original Message----- >> From: Guo, Jia >> Sent: Wednesday, April 15, 2020 6:11 PM >> To: Iremonger, Bernard ; >> orika@mellanox.com; Ye, Xiaolong ; Zhang, Qi Z >> >> Cc: dev@dpdk.org; Wu, Jingjing ; Cao, Yahui >> ; Su, Simei ; Guo, Jia >> >> Subject: [dpdk-dev v5 3/3] app/testpmd: add new types to RSS hash >> commands >> >> Add some new types, such as eth/l2-src-only/l2-dst-only/svlan/cvlan/ >> l2tpv3/esp/ah/pfcp types into RSS hash commands, it could be used to >> configure these rss input set by cmdline. >> >> Example testpmd commands are: >> Eth: >> testpmd>flow create 0 ingress pattern eth / ipv4 / end actions rss \ >> types eth l2-src-only end key_len 0 queues end / end >> >> testpmd>flow create 0 ingress pattern eth / ipv4 / end actions rss \ >> types eth l2-dst-only end key_len 0 queues end / end >> >> testpmd>flow create 0 ingress pattern eth / ipv4 / end actions rss \ >> types eth end key_len 0 queues end / end >> >> s-vlan: >> testpmd>flow create 0 ingress pattern eth / ipv4 / end actions rss \ >> types c-vlan end key_len 0 queues end / end >> >> c-vlan: >> testpmd>flow create 0 ingress pattern eth / ipv4 / end actions rss \ >> types c-vlan end key_len 0 queues end / end >> >> l2tpv3: >> testpmd>flow create 0 ingress pattern eth / ipv4 / l2tpv3oip / end \ >> actions rss types ipv4 l2tpv3 end key_len 0 queues end / end >> >> esp: >> testpmd>flow create 0 ingress pattern eth / ipv4 / esp / end actions \ >> rss types ipv4 esp end key_len 0 queues end / end > Should "rss types ipv4 esp" be "rss types esp" ? yes, it should be. >> ah: >> testpmd>flow create 0 ingress pattern eth / ipv4 / ah / end actions \ >> rss types ipv4 ah end key_len 0 queues end / end >> > Should "rss types ah" be "rss types ah" ? yes, the same as above and i think you are right,  all action should be specific, i think the parse logic should be modify to use pattern to distinguish the ipv4 ah or ipv6 ah but not use rss type. >> pfcp: >> testpmd>flow create 0 ingress pattern eth / ipv4 / udp / pfcp / end \ >> actions rss types ipv4-udp pfcp end key_len 0 queues end / end > The above sample commands might be better in a doc file, rather than in the commit message. > For example doc/guides/howto/rte_flow.rst > > The flow create/validate commands are normally parsed in app/testpmd/cmdline_flow.c > This patch is updating the following commands: > "port config all rss all" > "port config rss-hash-key" Since this is not the command initial and just leverage the command which is already exist, so i  think i would suppose that no need to modify the doc if no change, only change the part if the change bring in, and i could briefly introduce a example only in commit log for simplify. >> Signed-off-by: Jeff Guo >> --- >> v5->v4: >> rename eth to l2 and refine commit log >> --- >> app/test-pmd/cmdline.c | 28 +++++++++++++++++++++++++--- app/test- >> pmd/config.c | 9 +++++++++ >> 2 files changed, 34 insertions(+), 3 deletions(-) > >> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index >> 863b567c1..6b688ab66 100644 >> --- a/app/test-pmd/cmdline.c >> +++ b/app/test-pmd/cmdline.c >> @@ -2270,9 +2270,11 @@ cmd_config_rss_parsed(void *parsed_result, >> int ret; >> >> if (!strcmp(res->value, "all")) >> -rss_conf.rss_hf = ETH_RSS_IP | ETH_RSS_TCP | >> +rss_conf.rss_hf = ETH_RSS_ETH | ETH_RSS_IP | >> ETH_RSS_TCP | >> ETH_RSS_UDP | ETH_RSS_SCTP | >> ETH_RSS_L2_PAYLOAD; >> +else if (!strcmp(res->value, "eth")) >> +rss_conf.rss_hf = ETH_RSS_ETH; >> else if (!strcmp(res->value, "ip")) >> rss_conf.rss_hf = ETH_RSS_IP; >> else if (!strcmp(res->value, "udp")) >> @@ -2299,6 +2301,22 @@ cmd_config_rss_parsed(void *parsed_result, >> rss_conf.rss_hf = ETH_RSS_L4_SRC_ONLY; >> else if (!strcmp(res->value, "l4-dst-only")) >> rss_conf.rss_hf = ETH_RSS_L4_DST_ONLY; >> +else if (!strcmp(res->value, "l2-src-only")) >> +rss_conf.rss_hf = ETH_RSS_L2_SRC_ONLY; >> +else if (!strcmp(res->value, "l2-dst-only")) >> +rss_conf.rss_hf = ETH_RSS_L2_DST_ONLY; >> +else if (!strcmp(res->value, "s-vlan")) >> +rss_conf.rss_hf = ETH_RSS_S_VLAN; >> +else if (!strcmp(res->value, "c-vlan")) >> +rss_conf.rss_hf = ETH_RSS_C_VLAN; >> +else if (!strcmp(res->value, "l2tpv3")) >> +rss_conf.rss_hf = ETH_RSS_L2TPV3; >> +else if (!strcmp(res->value, "esp")) >> +rss_conf.rss_hf = ETH_RSS_ESP; >> +else if (!strcmp(res->value, "ah")) >> +rss_conf.rss_hf = ETH_RSS_AH; >> +else if (!strcmp(res->value, "pfcp")) >> +rss_conf.rss_hf = ETH_RSS_PFCP; >> else if (!strcmp(res->value, "none")) >> rss_conf.rss_hf = 0; >> else if (!strcmp(res->value, "default")) @@ -2467,7 +2485,9 @@ >> cmdline_parse_token_string_t cmd_config_rss_hash_key_rss_type = >> "ipv4-other#ipv6#ipv6-frag#ipv6-tcp#ipv6- >> udp#" >> "ipv6-sctp#ipv6-other#l2-payload#ipv6-ex#" >> "ipv6-tcp-ex#ipv6-udp-ex#" >> - "l3-src-only#l3-dst-only#l4-src-only#l4-dst- >> only"); >> + "l3-src-only#l3-dst-only#l4-src-only#l4-dst- >> only#" >> + "l2-src-only#l2-dst-only#s-vlan#c-vlan#" >> + "l2tpv3#esp#ah#pfcp"); >> cmdline_parse_token_string_t cmd_config_rss_hash_key_value = >> TOKEN_STRING_INITIALIZER(struct cmd_config_rss_hash_key, key, >> NULL); >> >> @@ -2478,7 +2498,9 @@ cmdline_parse_inst_t cmd_config_rss_hash_key = >> { >> "ipv4|ipv4-frag|ipv4-tcp|ipv4-udp|ipv4-sctp|ipv4-other|" >> "ipv6|ipv6-frag|ipv6-tcp|ipv6-udp|ipv6-sctp|ipv6-other|" >> "l2-payload|ipv6-ex|ipv6-tcp-ex|ipv6-udp-ex|" >> -"l3-src-only|l3-dst-only|l4-src-only|l4-dst-only " >> +"l3-src-only|l3-dst-only|l4-src-only|l4-dst-only|" >> +"l2-src-only|l2-dst-only|s-vlan|c-vlan|" >> +"l2tpv3|esp|ah|pfcp " >> "", >> .tokens = { >> (void *)&cmd_config_rss_hash_key_port, diff --git >> a/app/test-pmd/config.c b/app/test-pmd/config.c index >> 71aeb5413..e4a97388b 100644 >> --- a/app/test-pmd/config.c >> +++ b/app/test-pmd/config.c >> @@ -79,6 +79,11 @@ const struct rss_type_info rss_type_table[] = { >> ETH_RSS_UDP | ETH_RSS_SCTP | >> ETH_RSS_L2_PAYLOAD }, >> { "none", 0 }, >> +{ "eth", ETH_RSS_ETH }, >> +{ "l2-src-only", ETH_RSS_L2_SRC_ONLY }, >> +{ "l2-dst-only", ETH_RSS_L2_DST_ONLY }, >> +{ "s-vlan", ETH_RSS_S_VLAN }, >> +{ "c-vlan", ETH_RSS_C_VLAN }, > Should these additions be at the bottom of the array with the other additions ? I suppose this is not in a library code or application binary interface, so it would not break ABI/API, and re-order it as up-down layer must be better. >> { "ipv4", ETH_RSS_IPV4 }, >> { "ipv4-frag", ETH_RSS_FRAG_IPV4 }, >> { "ipv4-tcp", ETH_RSS_NONFRAG_IPV4_TCP }, @@ -108,6 +113,10 >> @@ const struct rss_type_info rss_type_table[] = { >> { "l3-dst-only", ETH_RSS_L3_DST_ONLY }, >> { "l4-src-only", ETH_RSS_L4_SRC_ONLY }, >> { "l4-dst-only", ETH_RSS_L4_DST_ONLY }, >> +{ "l2tpv3", ETH_RSS_L2TPV3 }, >> +{ "esp", ETH_RSS_ESP }, >> +{ "ah", ETH_RSS_AH }, >> +{ "pfcp", ETH_RSS_PFCP }, >> { NULL, 0 }, >> }; >> >> -- >> 2.20.1