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 E9695A0544; Tue, 21 Jun 2022 04:19:03 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A55114069C; Tue, 21 Jun 2022 04:19:03 +0200 (CEST) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by mails.dpdk.org (Postfix) with ESMTP id 8846340151 for ; Tue, 21 Jun 2022 04:19:01 +0200 (CEST) Received: from dggemv711-chm.china.huawei.com (unknown [172.30.72.57]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4LRqs12ktHzhYcK; Tue, 21 Jun 2022 10:16:53 +0800 (CST) Received: from kwepemm600004.china.huawei.com (7.193.23.242) by dggemv711-chm.china.huawei.com (10.1.198.66) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Tue, 21 Jun 2022 10:18:53 +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; Tue, 21 Jun 2022 10:18:53 +0800 Message-ID: Date: Tue, 21 Jun 2022 10:18:52 +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 4/4] app/testpmd: remove duplicated flow type to string table To: Ferruh Yigit , , , , CC: , , References: <20220429102445.23711-1-lihuisong@huawei.com> <20220607083246.12259-1-lihuisong@huawei.com> <20220607083246.12259-5-lihuisong@huawei.com> <20a69b54-40ca-f1be-3253-4f5aed741919@huawei.com> <1b05f308-d313-6e62-0a4f-76f2f6dfc918@xilinx.com> From: "lihuisong (C)" In-Reply-To: <1b05f308-d313-6e62-0a4f-76f2f6dfc918@xilinx.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.231] X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) 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/20 20:38, Ferruh Yigit 写道: > On 6/9/2022 4:26 AM, lihuisong (C) wrote: >> CAUTION: This message has originated from an External Source. Please >> use proper judgment and caution when opening attachments, clicking >> links, or responding to this email. >> >> >> 在 2022/6/7 23:47, Ferruh Yigit 写道: >>> On 6/7/2022 9:32 AM, Huisong Li wrote: >>> >>>> >>>> From: Ferruh Yigit >>>> >>>> Flow type table has two instance, one is used for flow type to string >>>> conversion, and other is used for string to flow type conversion. >>>> And tables are diverged by time. >>>> >>>> Unifying tables to prevent maintaining two different tables. >>>> >>>> Note: made 'flowtype_to_str()' non-static to prevent build error >>>> for the >>>> case PMDs using it disables. Making function generic, not for some >>>> PMDs. >>>> >>>> Signed-off-by: Ferruh Yigit >>>> Signed-off-by: Huisong Li >>>> --- >>>>   app/test-pmd/cmdline.c | 41 +------------------ >>>>   app/test-pmd/config.c  | 92 >>>> +++++++++++++++++++++++++++--------------- >>>>   app/test-pmd/testpmd.h |  5 +++ >>>>   3 files changed, 65 insertions(+), 73 deletions(-) >>>> >>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c >>>> index fdd0cada3b..e44bb3f475 100644 >>>> --- a/app/test-pmd/cmdline.c >>>> +++ b/app/test-pmd/cmdline.c >>>> @@ -10591,45 +10591,6 @@ do { \ >>>> >>>>   #ifdef RTE_NET_I40E >>>> >>>> -static uint16_t >>>> -str2flowtype(char *string) >>>> -{ >>>> -       uint8_t i = 0; >>>> -       static const struct { >>>> -               char str[32]; >>>> -               uint16_t type; >>>> -       } flowtype_str[] = { >>>> -               {"raw", RTE_ETH_FLOW_RAW}, >>>> -               {"ipv4", RTE_ETH_FLOW_IPV4}, >>>> -               {"ipv4-frag", RTE_ETH_FLOW_FRAG_IPV4}, >>>> -               {"ipv4-tcp", RTE_ETH_FLOW_NONFRAG_IPV4_TCP}, >>>> -               {"ipv4-udp", RTE_ETH_FLOW_NONFRAG_IPV4_UDP}, >>>> -               {"ipv4-sctp", RTE_ETH_FLOW_NONFRAG_IPV4_SCTP}, >>>> -               {"ipv4-other", RTE_ETH_FLOW_NONFRAG_IPV4_OTHER}, >>>> -               {"ipv6", RTE_ETH_FLOW_IPV6}, >>>> -               {"ipv6-frag", RTE_ETH_FLOW_FRAG_IPV6}, >>>> -               {"ipv6-tcp", RTE_ETH_FLOW_NONFRAG_IPV6_TCP}, >>>> -               {"ipv6-udp", RTE_ETH_FLOW_NONFRAG_IPV6_UDP}, >>>> -               {"ipv6-sctp", RTE_ETH_FLOW_NONFRAG_IPV6_SCTP}, >>>> -               {"ipv6-other", RTE_ETH_FLOW_NONFRAG_IPV6_OTHER}, >>>> -               {"l2_payload", RTE_ETH_FLOW_L2_PAYLOAD}, >>>> -               {"ipv6-ex", RTE_ETH_FLOW_IPV6_EX}, >>>> -               {"ipv6-tcp-ex", RTE_ETH_FLOW_IPV6_TCP_EX}, >>>> -               {"ipv6-udp-ex", RTE_ETH_FLOW_IPV6_UDP_EX}, >>>> -               {"gtpu", RTE_ETH_FLOW_GTPU}, >>>> -       }; >>>> - >>>> -       for (i = 0; i < RTE_DIM(flowtype_str); i++) { >>>> -               if (!strcmp(flowtype_str[i].str, string)) >>>> -                       return flowtype_str[i].type; >>>> -       } >>>> - >>>> -       if (isdigit(string[0]) && atoi(string) > 0 && atoi(string) >>>> < 64) >>>> -               return (uint16_t)atoi(string); >>>> - >>>> -       return RTE_ETH_FLOW_UNKNOWN; >>>> -} >>>> - >>>>   /* *** deal with flow director filter *** */ >>>>   struct cmd_flow_director_result { >>>>          cmdline_fixed_string_t flow_director_filter; >>>> @@ -10658,7 +10619,7 @@ cmd_flow_director_filter_parsed(void >>>> *parsed_result, >>>>          struct rte_pmd_i40e_flow_type_mapping >>>> mapping[RTE_PMD_I40E_FLOW_TYPE_MAX]; >>>>          struct rte_pmd_i40e_pkt_template_conf conf; >>>> -       uint16_t flow_type = str2flowtype(res->flow_type); >>>> +       uint16_t flow_type = str_to_flowtype(res->flow_type); >>>>          uint16_t i, port = res->port_id; >>>>          uint8_t add; >>>> >>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c >>>> index 209cbcc514..ed0be8036b 100644 >>>> --- a/app/test-pmd/config.c >>>> +++ b/app/test-pmd/config.c >>>> @@ -86,6 +86,38 @@ static const struct { >>>>          }, >>>>   }; >>>> >>>> +#if defined(RTE_NET_I40E) || defined(RTE_NET_IXGBE) >>>> + >>> >>> Although right now 'flowtype_to_str()' & 'str_to_flowtype()' are used >>> by code for above PMDs, the functionality is nothing special to any >>> PMD. >>> What about making above functions as non-static functions and get rid >>> of all related #ifdef? >>> >> Currently, they are not used anywhere but here. If remove #ifdef, a >> warning will encounter. > > If you make functions non-static, I think there won't be any warnings. > > It is better to make functionality more generic, instead of PMD specific. These codes has been moved to i40e PMD. Can we remove this patch from the set? > >>> >>>> +static const struct { >>>> +       char str[32]; >>>> +       uint16_t ftype; >>>> +} flowtype_str_table[] = { >>>> +       {"raw", RTE_ETH_FLOW_RAW}, >>>> +       {"ipv4", RTE_ETH_FLOW_IPV4}, >>>> +       {"ipv4-frag", RTE_ETH_FLOW_FRAG_IPV4}, >>>> +       {"ipv4-tcp", RTE_ETH_FLOW_NONFRAG_IPV4_TCP}, >>>> +       {"ipv4-udp", RTE_ETH_FLOW_NONFRAG_IPV4_UDP}, >>>> +       {"ipv4-sctp", RTE_ETH_FLOW_NONFRAG_IPV4_SCTP}, >>>> +       {"ipv4-other", RTE_ETH_FLOW_NONFRAG_IPV4_OTHER}, >>>> +       {"ipv6", RTE_ETH_FLOW_IPV6}, >>>> +       {"ipv6-frag", RTE_ETH_FLOW_FRAG_IPV6}, >>>> +       {"ipv6-tcp", RTE_ETH_FLOW_NONFRAG_IPV6_TCP}, >>>> +       {"ipv6-udp", RTE_ETH_FLOW_NONFRAG_IPV6_UDP}, >>>> +       {"ipv6-sctp", RTE_ETH_FLOW_NONFRAG_IPV6_SCTP}, >>>> +       {"ipv6-other", RTE_ETH_FLOW_NONFRAG_IPV6_OTHER}, >>>> +       {"l2_payload", RTE_ETH_FLOW_L2_PAYLOAD}, >>>> +       {"ipv6-ex", RTE_ETH_FLOW_IPV6_EX}, >>>> +       {"ipv6-tcp-ex", RTE_ETH_FLOW_IPV6_TCP_EX}, >>>> +       {"ipv6-udp-ex", RTE_ETH_FLOW_IPV6_UDP_EX}, >>>> +       {"port", RTE_ETH_FLOW_PORT}, >>>> +       {"vxlan", RTE_ETH_FLOW_VXLAN}, >>>> +       {"geneve", RTE_ETH_FLOW_GENEVE}, >>>> +       {"nvgre", RTE_ETH_FLOW_NVGRE}, >>>> +       {"vxlan-gpe", RTE_ETH_FLOW_VXLAN_GPE}, >>>> +       {"gtpu", RTE_ETH_FLOW_GTPU}, >>>> +}; >>>> +#endif >>>> + >>>>   const struct rss_type_info rss_offload_table[] = { >>>>          {"ipv4", RTE_ETH_RSS_IPV4}, >>>>          {"ipv4-frag", RTE_ETH_RSS_FRAG_IPV4}, >>>> @@ -5713,41 +5745,35 @@ set_record_burst_stats(uint8_t on_off) >>>>          record_burst_stats = on_off; >>>>   } >>>> >>>> +#ifdef RTE_NET_I40E >>>> + >>>> +uint16_t >>>> +str_to_flowtype(const char *string) >>>> +{ >>>> +       uint8_t i; >>>> + >>>> +       for (i = 0; i < RTE_DIM(flowtype_str_table); i++) { >>>> +               if (!strcmp(flowtype_str_table[i].str, string)) >>>> +                       return flowtype_str_table[i].ftype; >>>> +       } >>>> + >>>> +       if (isdigit(string[0])) { >>>> +               int val = atoi(string); >>>> +               if (val > 0 && val < 64) >>>> +                       return (uint16_t)val; >>>> +       } >>>> + >>>> +       return RTE_ETH_FLOW_UNKNOWN; >>>> +} >>>> + >>>> +#endif /* RTE_NET_I40E */ >>>> + >>>>   #if defined(RTE_NET_I40E) || defined(RTE_NET_IXGBE) >>>> -static char* >>>> + >>>> +static const char* >>>>   flowtype_to_str(uint16_t flow_type) >>>>   { >>>> -       struct flow_type_info { >>>> -               char str[32]; >>>> -               uint16_t ftype; >>>> -       }; >>>> - >>>>          uint8_t i; >>>> -       static struct flow_type_info flowtype_str_table[] = { >>>> -               {"raw", RTE_ETH_FLOW_RAW}, >>>> -               {"ipv4", RTE_ETH_FLOW_IPV4}, >>>> -               {"ipv4-frag", RTE_ETH_FLOW_FRAG_IPV4}, >>>> -               {"ipv4-tcp", RTE_ETH_FLOW_NONFRAG_IPV4_TCP}, >>>> -               {"ipv4-udp", RTE_ETH_FLOW_NONFRAG_IPV4_UDP}, >>>> -               {"ipv4-sctp", RTE_ETH_FLOW_NONFRAG_IPV4_SCTP}, >>>> -               {"ipv4-other", RTE_ETH_FLOW_NONFRAG_IPV4_OTHER}, >>>> -               {"ipv6", RTE_ETH_FLOW_IPV6}, >>>> -               {"ipv6-frag", RTE_ETH_FLOW_FRAG_IPV6}, >>>> -               {"ipv6-tcp", RTE_ETH_FLOW_NONFRAG_IPV6_TCP}, >>>> -               {"ipv6-udp", RTE_ETH_FLOW_NONFRAG_IPV6_UDP}, >>>> -               {"ipv6-sctp", RTE_ETH_FLOW_NONFRAG_IPV6_SCTP}, >>>> -               {"ipv6-other", RTE_ETH_FLOW_NONFRAG_IPV6_OTHER}, >>>> -               {"l2_payload", RTE_ETH_FLOW_L2_PAYLOAD}, >>>> -               {"ipv6-ex", RTE_ETH_FLOW_IPV6_EX}, >>>> -               {"ipv6-tcp-ex", RTE_ETH_FLOW_IPV6_TCP_EX}, >>>> -               {"ipv6-udp-ex", RTE_ETH_FLOW_IPV6_UDP_EX}, >>>> -               {"port", RTE_ETH_FLOW_PORT}, >>>> -               {"vxlan", RTE_ETH_FLOW_VXLAN}, >>>> -               {"geneve", RTE_ETH_FLOW_GENEVE}, >>>> -               {"nvgre", RTE_ETH_FLOW_NVGRE}, >>>> -               {"vxlan-gpe", RTE_ETH_FLOW_VXLAN_GPE}, >>>> -               {"gtpu", RTE_ETH_FLOW_GTPU}, >>>> -       }; >>>> >>>>          for (i = 0; i < RTE_DIM(flowtype_str_table); i++) { >>>>                  if (flowtype_str_table[i].ftype == flow_type) >>>> @@ -5821,7 +5847,7 @@ print_fdir_flex_mask(struct >>>> rte_eth_fdir_flex_conf *flex_conf, uint32_t num) >>>>   { >>>>          struct rte_eth_fdir_flex_mask *mask; >>>>          uint32_t i, j; >>>> -       char *p; >>>> +       const char *p; >>>> >>>>          for (i = 0; i < flex_conf->nb_flexmasks; i++) { >>>>                  mask = &flex_conf->flex_mask[i]; >>>> @@ -5837,7 +5863,7 @@ static inline void >>>>   print_fdir_flow_type(uint32_t flow_types_mask) >>>>   { >>>>          int i; >>>> -       char *p; >>>> +       const char *p; >>>> >>>>          for (i = RTE_ETH_FLOW_UNKNOWN; i < RTE_ETH_FLOW_MAX; i++) { >>>>                  if (!(flow_types_mask & (1 << i))) >>>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h >>>> index 6693813dda..b7931a2bee 100644 >>>> --- a/app/test-pmd/testpmd.h >>>> +++ b/app/test-pmd/testpmd.h >>>> @@ -1086,6 +1086,11 @@ void pmd_test_exit(void); >>>>   #if defined(RTE_NET_I40E) || defined(RTE_NET_IXGBE) >>>>   void fdir_get_infos(portid_t port_id); >>>>   #endif >>>> + >>>> +#ifdef RTE_NET_I40E >>>> +uint16_t str_to_flowtype(const char *string); >>>> +#endif >>>> + >>>>   void fdir_set_flex_mask(portid_t port_id, >>>>                             struct rte_eth_fdir_flex_mask *cfg); >>>>   void fdir_set_flex_payload(portid_t port_id, >>>> -- >>>> 2.33.0 >>>> >>> >>> . > > .