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 E1EFCA00C5; Tue, 21 Jun 2022 14:21:47 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D2F524069C; Tue, 21 Jun 2022 14:21:47 +0200 (CEST) Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by mails.dpdk.org (Postfix) with ESMTP id C517740151 for ; Tue, 21 Jun 2022 14:21:45 +0200 (CEST) Received: from dggemv711-chm.china.huawei.com (unknown [172.30.72.55]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4LS5GH0k72zDqsK; Tue, 21 Jun 2022 20:21:11 +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 20:21:43 +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 20:21:42 +0800 Message-ID: Date: Tue, 21 Jun 2022 20:21:42 +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> <1f205d2f-ec36-d471-265e-94d05f31fb7f@xilinx.com> From: "lihuisong (C)" In-Reply-To: <1f205d2f-ec36-d471-265e-94d05f31fb7f@xilinx.com> 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/21 15:51, Ferruh Yigit 写道: > On 6/21/2022 3:18 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/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? > > There are two set of arrays, one moved to i40e folder, other not, right? > > I think better to move 'str2flowtype()' back to testpmd and unify > arrays. As discussed above it is not i40e specific. Ok. >>> >>>>> >>>>>> +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 >>>>>> >>>>> >>>>> . >>> >>> . > > .