Re: [dpdk-dev] [PATCH 1/3] app/flow-perf: support meter policy API

[Wisam Monther <wisamm@nvidia.com>]

Hi guys,

I have some issues w/ this patch, please exclude my ack for now.

Can you please check my comments on the this previous patch in ML?

BRs,
Wisam Jaddo

Get Outlook for iOS<https://aka.ms/o0ukef>
________________________________
From: Jiawei(Jonny) Wang <jiaweiw@nvidia.com>
Sent: Monday, May 10, 2021 7:17:21 PM
To: Matan Azrad <matan@nvidia.com>; Haifei Luo <haifeil@nvidia.com>; NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Wisam Monther <wisamm@nvidia.com>
Cc: dev@dpdk.org <dev@dpdk.org>; Raslan Darawsheh <rasland@nvidia.com>
Subject: [PATCH 1/3] app/flow-perf: support meter policy API

Add option "policy-mtr" to indicate if meter creation will include policy
or not. Meter creation will keep same without it.

With "policy-mtr", policy is introduced. API create_meter_policy
is to create a policy. API create_meter_rule will use it to create
meter.

Signed-off-by: Haifei Luo <haifeil@nvidia.com>
Signed-off-by: Jiawei Wang <jiaweiw@nvidia.com>
Acked-by: Wisam Jaddo <wisamm@nvidia.com>
---
 app/test-flow-perf/main.c      | 87 ++++++++++++++++++++++++++++++++--
 doc/guides/tools/flow-perf.rst |  3 ++
 2 files changed, 86 insertions(+), 4 deletions(-)

diff --git a/app/test-flow-perf/main.c b/app/test-flow-perf/main.c
index 9be8edc31d..7b8b6fb9c4 100644
--- a/app/test-flow-perf/main.c
+++ b/app/test-flow-perf/main.c
@@ -53,6 +53,7 @@ static uint64_t decap_data;
 static uint64_t flow_items[MAX_ITEMS_NUM];
 static uint64_t flow_actions[MAX_ACTIONS_NUM];
 static uint64_t flow_attrs[MAX_ATTRS_NUM];
+static uint32_t g_policy_id[MAX_PORTS];
 static uint8_t items_idx, actions_idx, attrs_idx;

 static uint64_t ports_mask;
@@ -62,6 +63,7 @@ static bool delete_flag;
 static bool dump_socket_mem_flag;
 static bool enable_fwd;
 static bool unique_data;
+static bool policy_mtr;

 static struct rte_mempool *mbuf_mp;
 static uint32_t nb_lcores;
@@ -115,6 +117,13 @@ static struct multi_cores_pool mc_pool = {
         .cores_count = 1,
 };

+/* Storage for struct rte_flow_action_rss including external data. */
+struct action_rss_data {
+       struct rte_flow_action_rss conf;
+       uint8_t key[40];
+       uint16_t queue[128];
+};
+
 static void
 usage(char *progname)
 {
@@ -134,6 +143,7 @@ usage(char *progname)
         printf("  --portmask=N: hexadecimal bitmask of ports used\n");
         printf("  --unique-data: flag to set using unique data for all"
                 " actions that support data, such as header modify and encap actions\n");
+       printf("  --policy-mtr: To create meter with policy\n");

         printf("To set flow attributes:\n");
         printf("  --ingress: set ingress attribute in flows\n");
@@ -573,6 +583,7 @@ args_parse(int argc, char **argv)
                 { "unique-data",                0, 0, 0 },
                 { "portmask",                   1, 0, 0 },
                 { "cores",                      1, 0, 0 },
+               { "policy-mtr",                 0, 0, 0 },
                 /* Attributes */
                 { "ingress",                    0, 0, 0 },
                 { "egress",                     0, 0, 0 },
@@ -802,6 +813,8 @@ args_parse(int argc, char **argv)
                                                 RTE_MAX_LCORE);
                                 }
                         }
+                       if (strcmp(lgopts[opt_idx].name, "policy-mtr") == 0)
+                               policy_mtr = true;
                         break;
                 default:
                         usage(argv[0]);
@@ -912,6 +925,58 @@ has_meter(void)
         return 0;
 }

+static void
+create_meter_policy(void)
+{
+       struct rte_mtr_error error;
+       uint32_t policy_id;
+       int ret, i, port_id;
+       struct rte_mtr_meter_policy_params policy;
+       struct action_rss_data rss_data;
+       struct rte_flow_action g_actions[2], r_actions[2];
+       uint16_t nr_ports;
+
+       memset(&rss_data, 0, sizeof(rss_data));
+       memset(&policy, 0, sizeof(policy));
+       rss_data.conf.func = RTE_ETH_HASH_FUNCTION_DEFAULT;
+       rss_data.conf.level = 0;
+       rss_data.conf.types = GET_RSS_HF();
+       rss_data.conf.key_len = 0;
+       rss_data.conf.key = NULL;
+       rss_data.conf.queue_num  = RXQ_NUM;
+       uint16_t q_data[RXQ_NUM];
+       rss_data.conf.queue = q_data;
+
+       for (i = 0; i < RXQ_NUM; i++)
+               q_data[i] = i;
+       for (i = 0; i < RXQ_NUM; i++)
+               rss_data.queue[i] = i;
+
+       g_actions[0].type = RTE_FLOW_ACTION_TYPE_RSS;
+       g_actions[0].conf = &(rss_data.conf);
+       g_actions[1].type = RTE_FLOW_ACTION_TYPE_END;
+       g_actions[1].conf = NULL;
+
+       r_actions[0].type = RTE_FLOW_ACTION_TYPE_DROP;
+       r_actions[0].conf = NULL;
+       r_actions[1].type = RTE_FLOW_ACTION_TYPE_END;
+       r_actions[1].conf = NULL;
+
+       policy.actions[RTE_COLOR_GREEN] = &g_actions[0];
+       policy.actions[RTE_COLOR_YELLOW] = NULL;
+       policy.actions[RTE_COLOR_RED] = &r_actions[0];
+
+       nr_ports = rte_eth_dev_count_avail();
+       for (port_id = 0; port_id < nr_ports; port_id++) {
+               policy_id = port_id + 10;
+               ret = rte_mtr_meter_policy_add(port_id, policy_id,
+                                              &policy, &error);
+               if (ret)
+                       printf("meter add failed port_id  %d\n", port_id);
+               g_policy_id[port_id] = policy_id;
+       }
+}
+
 static void
 create_meter_rule(int port_id, uint32_t counter)
 {
@@ -928,7 +993,14 @@ create_meter_rule(int port_id, uint32_t counter)

         /*create meter*/
         params.meter_profile_id = default_prof_id;
-       ret = rte_mtr_create(port_id, counter, &params, 1, &error);
+
+       if (!policy_mtr) {
+               ret = rte_mtr_create(port_id, counter, &params, 1, &error);
+       } else {
+               params.meter_policy_id = g_policy_id[port_id];
+               ret = rte_mtr_create(port_id, counter, &params, 0, &error);
+       }
+
         if (ret != 0) {
                 printf("Port %u create meter idx(%d) error(%d) message: %s\n",
                         port_id, counter, error.type,
@@ -942,11 +1014,16 @@ destroy_meter_rule(int port_id, uint32_t counter)
 {
         struct rte_mtr_error error;

-       if (rte_mtr_destroy(port_id, counter, &error)) {
-               printf("Port %u destroy meter(%d) error(%d) message: %s\n",
+       if (policy_mtr) {
+               if (rte_mtr_meter_policy_delete(port_id, counter+1, &error))
+                       printf("erro delete policy %d\n", counter+1);
+       } else {
+               if (rte_mtr_destroy(port_id, counter, &error)) {
+                       printf("Port %u destroy meter(%d) error(%d) message: %s\n",
                         port_id, counter, error.type,
                         error.message ? error.message : "(no stated reason)");
-               rte_exit(EXIT_FAILURE, "Error in deleting meter rule\n");
+                       rte_exit(EXIT_FAILURE, "Error in deleting meter rule");
+               }
         }
 }

@@ -1894,6 +1971,8 @@ main(int argc, char **argv)

         if (has_meter())
                 create_meter_profile();
+       if (policy_mtr)
+               create_meter_policy();
         rte_eal_mp_remote_launch(run_rte_flow_handler_cores, NULL, CALL_MAIN);

         if (enable_fwd) {
diff --git a/doc/guides/tools/flow-perf.rst b/doc/guides/tools/flow-perf.rst
index 280bf7e0e0..4c6480d70a 100644
--- a/doc/guides/tools/flow-perf.rst
+++ b/doc/guides/tools/flow-perf.rst
@@ -354,3 +354,6 @@ Actions:
 *       ``--meter``
         Add meter action to all flows actions.
         Currently, 1 meter profile -> N meter rules -> N rte flows.
+
+*       ``--policy-mtr``
+        Add policy-mtr to create meter with policy.
--
2.18.1