Re: [PATCH V3 app/testpmd 4/4] app/testpmd: remove duplicated flow type to string table

Mon, 20 Jun 2022 05:39:06 -0700 

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 <ferruh.yi...@xilinx.com>

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 <ferruh.yi...@xilinx.com>
Signed-off-by: Huisong Li <lihuis...@huawei.com>
---
  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.




+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



.
























					Reply via email to