From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0A2B0A054D; Thu, 9 Jun 2022 05:25:20 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A1CA440689; Thu, 9 Jun 2022 05:25:19 +0200 (CEST) Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by mails.dpdk.org (Postfix) with ESMTP id EB40140220 for ; Thu, 9 Jun 2022 05:25:16 +0200 (CEST) Received: from dggemv704-chm.china.huawei.com (unknown [172.30.72.54]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4LJTx33NjKz8wyC; Thu, 9 Jun 2022 11:24:55 +0800 (CST) Received: from kwepemm600004.china.huawei.com (7.193.23.242) by dggemv704-chm.china.huawei.com (10.3.19.47) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Thu, 9 Jun 2022 11:25:14 +0800 Received: from [10.67.103.231] (10.67.103.231) by kwepemm600004.china.huawei.com (7.193.23.242) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Thu, 9 Jun 2022 11:25:14 +0800 Message-ID: Date: Thu, 9 Jun 2022 11:25:13 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 Subject: Re: [PATCH V3 app/testpmd 1/4] app/testpmd: fix supported RSS offload display To: Ferruh Yigit , , , , CC: , , References: <20220429102445.23711-1-lihuisong@huawei.com> <20220607083246.12259-1-lihuisong@huawei.com> <20220607083246.12259-2-lihuisong@huawei.com> From: "lihuisong (C)" In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.231] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To kwepemm600004.china.huawei.com (7.193.23.242) X-CFilter-Loop: Reflected X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 在 2022/6/7 23:45, Ferruh Yigit 写道: > On 6/7/2022 9:32 AM, Huisong Li wrote: > >> >> And rte_eth_dev_info.flow_type_rss_offloads is populated in terms of >> RTE_ETH_RSS_* bits. If PMD sets RTE_ETH_RSS_L3_SRC_ONLY to >> dev_info->flow_type_rss_offloads. testpmd will display "user defined 63" >> when run 'show port info 0'. Because testpmd use flowtype_to_str() >> to display the supported RSS offload of PMD. In fact, the function is >> used to display flow type in FDIR commands. This patch uses the >> RTE_ETH_RSS_* bits to display supported RSS offload of PMD. >> >> Fixes: b12964f621dc ("ethdev: unification of RSS offload types") >> Cc: stable@dpdk.org >> >> Signed-off-by: Huisong Li >> Signed-off-by: Ferruh Yigit >> --- >>   app/test-pmd/config.c | 85 ++++++++++++++++++++++++++++++++++++++----- >>   1 file changed, 75 insertions(+), 10 deletions(-) >> >> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c >> index 72d2606d19..d290ca3a06 100644 >> --- a/app/test-pmd/config.c >> +++ b/app/test-pmd/config.c >> @@ -86,6 +86,56 @@ static const struct { >>          }, >>   }; >> >> +const struct rss_type_info rss_offload_table[] = { >> +       {"ipv4", RTE_ETH_RSS_IPV4}, >> +       {"ipv4-frag", RTE_ETH_RSS_FRAG_IPV4}, >> +       {"ipv4-tcp", RTE_ETH_RSS_NONFRAG_IPV4_TCP}, >> +       {"ipv4-udp", RTE_ETH_RSS_NONFRAG_IPV4_UDP}, >> +       {"ipv4-sctp", RTE_ETH_RSS_NONFRAG_IPV4_SCTP}, >> +       {"ipv4-other", RTE_ETH_RSS_NONFRAG_IPV4_OTHER}, >> +       {"ipv6", RTE_ETH_RSS_IPV6}, >> +       {"ipv6-frag", RTE_ETH_RSS_FRAG_IPV6}, >> +       {"ipv6-tcp", RTE_ETH_RSS_NONFRAG_IPV6_TCP}, >> +       {"ipv6-udp", RTE_ETH_RSS_NONFRAG_IPV6_UDP}, >> +       {"ipv6-sctp", RTE_ETH_RSS_NONFRAG_IPV6_SCTP}, >> +       {"ipv6-other", RTE_ETH_RSS_NONFRAG_IPV6_OTHER}, >> +       {"l2_payload", RTE_ETH_RSS_L2_PAYLOAD}, >> +       {"ipv6-ex", RTE_ETH_RSS_IPV6_EX}, >> +       {"ipv6-tcp-ex", RTE_ETH_RSS_IPV6_TCP_EX}, >> +       {"ipv6-udp-ex", RTE_ETH_RSS_IPV6_UDP_EX}, >> +       {"port", RTE_ETH_RSS_PORT}, >> +       {"vxlan", RTE_ETH_RSS_VXLAN}, >> +       {"geneve", RTE_ETH_RSS_GENEVE}, >> +       {"nvgre", RTE_ETH_RSS_NVGRE}, >> +       {"gtpu", RTE_ETH_RSS_GTPU}, >> +       {"eth", RTE_ETH_RSS_ETH}, >> +       {"s-vlan", RTE_ETH_RSS_S_VLAN}, >> +       {"c-vlan", RTE_ETH_RSS_C_VLAN}, >> +       {"esp", RTE_ETH_RSS_ESP}, >> +       {"ah", RTE_ETH_RSS_AH}, >> +       {"l2tpv3", RTE_ETH_RSS_L2TPV3}, >> +       {"pfcp", RTE_ETH_RSS_PFCP}, >> +       {"pppoe", RTE_ETH_RSS_PPPOE}, >> +       {"ecpri", RTE_ETH_RSS_ECPRI}, >> +       {"mpls", RTE_ETH_RSS_MPLS}, >> +       {"ipv4-chksum", RTE_ETH_RSS_IPV4_CHKSUM}, >> +       {"l4-chksum", RTE_ETH_RSS_L4_CHKSUM}, >> +       {"l2tpv2", RTE_ETH_RSS_L2TPV2}, >> +       {"l3-pre96", RTE_ETH_RSS_L3_PRE96}, >> +       {"l3-pre64", RTE_ETH_RSS_L3_PRE64}, >> +       {"l3-pre56", RTE_ETH_RSS_L3_PRE56}, >> +       {"l3-pre48", RTE_ETH_RSS_L3_PRE48}, >> +       {"l3-pre40", RTE_ETH_RSS_L3_PRE40}, >> +       {"l3-pre32", RTE_ETH_RSS_L3_PRE32}, >> +       {"l2-dst-only", RTE_ETH_RSS_L2_DST_ONLY}, >> +       {"l2-src-only", RTE_ETH_RSS_L2_SRC_ONLY}, >> +       {"l4-dst-only", RTE_ETH_RSS_L4_DST_ONLY}, >> +       {"l4-src-only", RTE_ETH_RSS_L4_SRC_ONLY}, >> +       {"l3-dst-only", RTE_ETH_RSS_L3_DST_ONLY}, >> +       {"l3-src-only", RTE_ETH_RSS_L3_SRC_ONLY}, >> +       {NULL, 0}, >> +}; >> + > > Hi Huisong, > > Why not reusing existing 'rss_type_table[]', but adding a new one? > Is it to have each individual RSS type instead of grouping > 'rss_type_table[]' has? If so, since command "port config all rss ..." > using the grouping, I think it makes sense to have grouping. > The 'rss_offload_table[]' includes all defined RSS offloads in ethdev layer, every iterm has one bit, and is part of 'rss_type_table[]'. Some iterms in 'rss_type_table[]' consist of multiple bits. If we want to display all RSS offloads PMD supported and resolve undefined offload, we have to check 'flow_type_rss_offloads' bit by bit. However, "port config all rss ..." and "show port 0 rss-hash" commands supports the configuration and display of multiple offload bits at a time. So it is necessary to introduce this 'rss_offload_table[]' to display 'flow_type_rss_offloads'. That's what I think. > Another thing is having two different array with very similar content > is easy to confuse and can cause diversion on arrays and generate bugs. > Add some notes about the use of the 'rss_offload_table[]'? > > As mentioned from "port config all rss ..." command, it seems it is > also not using 'rss_type_table[]', but all string to RSS type matching > done within the function ('cmd_config_rss_parsed()') duplicating what > we have in 'rss_type_table[]'. Again this has concern to diverge > between set and show functions. > If you have time for it, can you make an additional patch to update > 'cmd_config_rss_parsed()' to use new 'str_to_rsstypes()' function? All right. Let's fix it. > > Thanks, > ferruh > >>   const struct rss_type_info rss_type_table[] = { >>          { "all", RTE_ETH_RSS_ETH | RTE_ETH_RSS_VLAN | RTE_ETH_RSS_IP >> | RTE_ETH_RSS_TCP | >>                  RTE_ETH_RSS_UDP | RTE_ETH_RSS_SCTP | >> RTE_ETH_RSS_L2_PAYLOAD | >> @@ -675,6 +725,19 @@ print_dev_capabilities(uint64_t capabilities) >>          } >>   } >> >> +static const char * >> +rss_offload_to_str(uint64_t rss_offload) >> +{ >> +       uint16_t i; >> + >> +       for (i = 0; rss_offload_table[i].str != NULL; i++) { >> +               if (rss_offload_table[i].rss_type == rss_offload) >> +                       return rss_offload_table[i].str; >> +       } >> + >> +       return NULL; >> +} >> + >>   void >>   port_infos_display(portid_t port_id) >>   { >> @@ -779,19 +842,21 @@ port_infos_display(portid_t port_id) >>          if (!dev_info.flow_type_rss_offloads) >>                  printf("No RSS offload flow type is supported.\n"); >>          else { >> +               uint64_t rss_offload_types = >> dev_info.flow_type_rss_offloads; >>                  uint16_t i; >> -               char *p; >> >>                  printf("Supported RSS offload flow types:\n"); >> -               for (i = RTE_ETH_FLOW_UNKNOWN + 1; >> -                    i < sizeof(dev_info.flow_type_rss_offloads) * >> CHAR_BIT; i++) { >> -                       if (!(dev_info.flow_type_rss_offloads & (1ULL >> << i))) >> -                               continue; >> -                       p = flowtype_to_str(i); >> -                       if (p) >> -                               printf("  %s\n", p); >> -                       else >> -                               printf("  user defined %d\n", i); >> +               for (i = 0; i < sizeof(rss_offload_types) * CHAR_BIT; >> i++) { >> +                       uint64_t rss_offload = RTE_BIT64(i); >> +                       if ((rss_offload_types & rss_offload) != 0) { >> +                               const char *p = >> + rss_offload_to_str(rss_offload); >> +                               if (p) >> +                                       printf("  %s\n", p); >> +                               else >> +                                       printf("  user defined >> 0x%"PRIx64"\n", >> +                                              rss_offload); >> +                       } > > If you go with 'rss_type_table[]', you need to change above logic as > you did in your v2. > >>                  } >>          } >> >> -- >> 2.33.0 >> > > .