[ovs-dev] [PATCH/RFC 6/7] ofproto-dpif: add offloaded meter revalidatation

Simon Horman simon.horman at corigine.com
Wed Nov 10 16:28:57 UTC 2021


From: Baowen Zheng <baowen.zheng at corigine.com>

When meter deleted the deletion of the corresponding TC police
action, used to facilitate offload via OVS-TC, may fail. This
is occurs when the TC police action instance still in use by a
TC flower classifiers (flows added to the TC datapath).

In this commit, i add a revalidate process to clean the police
action that offloaded relatewith a meter.

Signed-off-by: Baowen Zheng <baowen.zheng at corigine.com>
Signed-off-by: Tianyu Yuan <tianyu.yuan at corigine.com>
Signed-off-by: Simon Horman <simon.horman at corigine.com>
---
 lib/dpif-netdev.c             |   1 +
 lib/dpif-netlink.c            | 172 ++++++++++++++++++++++++++++++++++
 lib/dpif-provider.h           |   4 +
 lib/dpif.c                    |   6 ++
 lib/id-pool.c                 |   6 ++
 lib/id-pool.h                 |   1 +
 lib/tc.c                      |   6 --
 lib/tc.h                      |   7 ++
 ofproto/ofproto-dpif-upcall.c |   1 +
 ofproto/ofproto-dpif.h        |   2 +
 10 files changed, 200 insertions(+), 6 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index bf4de3cf4..0c0a0249e 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -8809,6 +8809,7 @@ const struct dpif_class dpif_netdev_class = {
     dpif_netdev_meter_set,
     dpif_netdev_meter_get,
     dpif_netdev_meter_del,
+    NULL,                       /* meter_revalidate */
     dpif_netdev_bond_add,
     dpif_netdev_bond_del,
     dpif_netdev_bond_stats_get,
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index d270dbbd2..aff78abcc 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -49,6 +49,7 @@
 #include "netlink.h"
 #include "netnsid.h"
 #include "odp-util.h"
+#include "ofproto/ofproto-dpif.h"
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/flow.h"
 #include "openvswitch/hmap.h"
@@ -4035,6 +4036,12 @@ dpif_netlink_meter_transact(struct ofpbuf *request, struct ofpbuf **replyp,
     return 0;
 }
 
+static const struct nl_policy police_policy[] = {
+    [TCA_POLICE_TBF] = { .type = NL_A_UNSPEC,
+                         .min_len = sizeof(struct tc_police),
+                         .optional = false, },
+};
+
 static void
 dpif_netlink_meter_get_features(const struct dpif *dpif_,
                                 struct ofputil_meter_features *features)
@@ -4294,6 +4301,170 @@ dpif_netlink_meter_set(struct dpif *dpif_, ofproto_meter_id meter_id,
     return dpif_netlink_meter_set__(dpif_, meter_id, add, config);
 }
 
+static const struct nl_policy tca_root_policy[] = {
+    [TCA_ACT_TAB] = { .type = NL_A_NESTED, .optional = false },
+    [TCA_ROOT_COUNT] = { .type = NL_A_U32, .optional = false },
+};
+
+static const struct nl_policy act_policy[] = {
+    [TCA_ACT_KIND] = { .type = NL_A_STRING, .optional = false, },
+    [TCA_ACT_COOKIE] = { .type = NL_A_UNSPEC, .optional = true, },
+    [TCA_ACT_OPTIONS] = { .type = NL_A_NESTED, .optional = true, },
+    [TCA_ACT_STATS] = { .type = NL_A_NESTED, .optional = false, },
+};
+
+static bool dpif_netlink_meter_should_revalidate(struct dpif_backer *backer,
+                                                 uint32_t meter_id)
+{
+    return !id_pool_id_exist(backer->meter_ids, meter_id);
+}
+
+static void
+dpif_tc_meter_revalidate(struct dpif *dpif_ OVS_UNUSED,
+                         struct dpif_backer *backer, struct ofpbuf *reply)
+{
+    static struct nl_policy actions_orders_policy[ACT_MAX_NUM + 1] = {};
+    struct nlattr *actions_orders[ARRAY_SIZE(actions_orders_policy)];
+    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
+    struct nlattr *action_root_attrs[ARRAY_SIZE(tca_root_policy)];
+    struct nlattr *action_police_tab[ARRAY_SIZE(police_policy)];
+    struct nlattr *action_police_attrs[ARRAY_SIZE(act_policy)];
+    const int max_size = ARRAY_SIZE(actions_orders_policy);
+    const struct tc_police *tc_police = NULL;
+    struct ofputil_meter_stats stats;
+    ofproto_meter_id meter_id;
+    size_t revalidate_num;
+    size_t act_count;
+    uint32_t index;
+    int i;
+
+    if (!reply) {
+        VLOG_ERR_RL(&rl, "get null reply message when meter revalidate \n");
+        return;
+    }
+
+    if (!nl_policy_parse(reply, NLMSG_HDRLEN + sizeof(struct tcamsg),
+                         tca_root_policy, action_root_attrs,
+                         ARRAY_SIZE(action_root_attrs))) {
+        VLOG_ERR_RL(&rl, "failed to parse reply message when meter "
+                    "revalidate \n");
+        return;
+    }
+
+    act_count = nl_attr_get_u32(action_root_attrs[TCA_ROOT_COUNT]);
+    if (!act_count) {
+        VLOG_ERR_RL(&rl, "there is no police action returned in message when "
+                    "meter revalidate \n");
+        return;
+    }
+
+    for (i = 0; i < max_size; i++) {
+        actions_orders_policy[i].type = NL_A_NESTED;
+        actions_orders_policy[i].optional = true;
+    }
+
+    revalidate_num = act_count > ACT_MAX_NUM ?
+                                (ACT_MAX_NUM + 1) : (act_count + 1);
+
+    if (!nl_parse_nested(action_root_attrs[TCA_ACT_TAB], actions_orders_policy,
+                         actions_orders, revalidate_num) ) {
+        VLOG_ERR_RL(&rl, "failed to parse TCA_ACT_TAB when meter revalidate "
+                    "for act_count %lu", act_count);
+        return;
+    }
+
+    for (i = 0; i < revalidate_num; i++) {
+        if (!actions_orders[i]) {
+            continue;
+        }
+
+        if (!nl_parse_nested(actions_orders[i], act_policy,
+                             action_police_attrs, ARRAY_SIZE(act_policy))) {
+            VLOG_ERR_RL(&rl, "failed to parse police action when meter "
+                      "revalidate\n");
+            return;
+        }
+        if (strcmp(nl_attr_get_string(action_police_attrs[TCA_KIND]),
+                                      "police")) {
+            VLOG_EMER("get none police action when meter revalidate\n");
+            continue;
+        }
+        if (!nl_parse_nested(action_police_attrs[TCA_ACT_OPTIONS],
+                             police_policy, action_police_tab,
+                             ARRAY_SIZE(action_police_tab))) {
+            VLOG_ERR_RL(&rl, "failed to parse the single police action when "
+                        "meter revalidate\n");
+            return;
+        }
+        tc_police = nl_attr_get_unspec(action_police_tab[TCA_POLICE_TBF],
+                                       sizeof *tc_police);
+        if (!tc_police) {
+            VLOG_ERR_RL(&rl, "can not get police struct in police order %u "
+                        "when meter revalidate\n", i);
+            continue;
+        }
+        index = tc_police->index;
+        if (UNLIKELY_METER_ACTION(index)) {
+            continue;
+        }
+
+        index = POLICY_INDEX_TO_METER_ID(index);
+        if (dpif_netlink_meter_should_revalidate(backer, index)) {
+            meter_id.uint32 = index;
+            VLOG_ERR_RL(&rl, "revalidate the meter id %u for police index "
+                        "%08x\n", index, tc_police->index);
+            dpif_netlink_meter_del_police(meter_id, &stats, 1);
+        }
+    }
+}
+
+static void
+dpif_netlink_meter_revalidate__(struct dpif *dpif_ OVS_UNUSED,
+                                struct dpif_backer *backer)
+{
+    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
+    struct nla_bitfield32 dump_flags = { TCA_DUMP_FLAGS_TERSE,
+                                         TCA_DUMP_FLAGS_TERSE };
+    struct ofpbuf request;
+    struct ofpbuf *reply;
+    struct tcamsg *tcmsg;
+    size_t total_offset;
+    size_t act_offset;
+    int prio = 0;
+    int error;
+
+    if (!netdev_is_flow_api_enabled()) {
+        return;
+    }
+    tcmsg = tc_act_make_request(RTM_GETACTION, NLM_F_DUMP, &request);
+    if (!tcmsg) {
+        return;
+    }
+    dpif_netlink_police_start_nested(&request, &prio, &total_offset,
+                                     &act_offset);
+    nl_msg_put_string(&request, TCA_KIND, "police");
+    dpif_netlink_police_end_nested(&request, &total_offset, &act_offset);
+    nl_msg_put_unspec(&request, TCA_ROOT_FLAGS, &dump_flags,
+                      sizeof dump_flags);
+    error = tc_transact(&request, &reply);
+    if (error) {
+        VLOG_ERR_RL(&rl, "failed to send dump netlink msg for revalidate "
+                    "error %d\n", error);
+        return;
+    }
+    dpif_tc_meter_revalidate(dpif_, backer, reply);
+    ofpbuf_delete(reply);
+}
+
+static void
+dpif_netlink_meter_revalidate(struct dpif *dpif_, struct dpif_backer *backer)
+{
+    if (probe_broken_meters(dpif_)) {
+        return;
+    }
+    dpif_netlink_meter_revalidate__(dpif_, backer);
+}
+
 /* Retrieve statistics and/or delete meter 'meter_id'.  Statistics are
  * stored in 'stats', if it is not null.  If 'command' is
  * OVS_METER_CMD_DEL, the meter is deleted and statistics are optionally
@@ -4617,6 +4788,7 @@ const struct dpif_class dpif_netlink_class = {
     dpif_netlink_meter_set,
     dpif_netlink_meter_get,
     dpif_netlink_meter_del,
+    dpif_netlink_meter_revalidate,
     NULL,                       /* bond_add */
     NULL,                       /* bond_del */
     NULL,                       /* bond_stats_get */
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 0d092de9b..f3893b2d1 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -25,6 +25,7 @@
 #include "openflow/openflow.h"
 #include "dpif.h"
 #include "util.h"
+#include "ofproto/ofproto-dpif.h"
 
 #ifdef  __cplusplus
 extern "C" {
@@ -624,6 +625,9 @@ struct dpif_class {
     int (*meter_del)(struct dpif *, ofproto_meter_id meter_id,
                      struct ofputil_meter_stats *, uint16_t n_bands);
 
+    /* revalidate meter entries in offload cases */
+    void (*meter_revalidate)(struct dpif *, struct dpif_backer *);
+
     /* Adds a bond with 'bond_id' and the member-map to 'dpif'. */
     int (*bond_add)(struct dpif *dpif, uint32_t bond_id,
                     odp_port_t *member_map);
diff --git a/lib/dpif.c b/lib/dpif.c
index 3d4eae1a6..984b20fef 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -2090,3 +2090,9 @@ dpif_cache_set_size(struct dpif *dpif, uint32_t level, uint32_t size)
            ? dpif->dpif_class->cache_set_size(dpif, level, size)
            : EOPNOTSUPP;
 }
+
+void
+dpif_meter_revalidate(struct dpif *dpif, struct dpif_backer *backer)
+{
+    dpif->dpif_class->meter_revalidate(dpif, backer);
+}
diff --git a/lib/id-pool.c b/lib/id-pool.c
index 69910ad08..50999a096 100644
--- a/lib/id-pool.c
+++ b/lib/id-pool.c
@@ -155,3 +155,9 @@ id_pool_free_id(struct id_pool *pool, uint32_t id)
         }
     }
 }
+
+bool
+id_pool_id_exist(struct id_pool *pool, uint32_t id)
+{
+    return !!id_pool_find(pool, id);
+}
diff --git a/lib/id-pool.h b/lib/id-pool.h
index 8721f8793..08fba6d43 100644
--- a/lib/id-pool.h
+++ b/lib/id-pool.h
@@ -29,6 +29,7 @@ void id_pool_destroy(struct id_pool *);
 bool id_pool_alloc_id(struct id_pool *, uint32_t *id);
 void id_pool_free_id(struct id_pool *, uint32_t id);
 void id_pool_add(struct id_pool *, uint32_t id);
+bool id_pool_id_exist(struct id_pool *pool, uint32_t id);
 
 /*
  * ID pool.
diff --git a/lib/tc.c b/lib/tc.c
index 3993498b0..5d89560f8 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -51,10 +51,6 @@
 #define TCM_IFINDEX_MAGIC_BLOCK (0xFFFFFFFFU)
 #endif
 
-#ifndef TCA_DUMP_FLAGS_TERSE
-#define TCA_DUMP_FLAGS_TERSE (1 << 0)
-#endif
-
 #if TCA_MAX < 15
 #define TCA_CHAIN 11
 #define TCA_INGRESS_BLOCK 13
@@ -1821,8 +1817,6 @@ nl_parse_single_action(struct nlattr *action, struct tc_flower *flower,
     return 0;
 }
 
-#define TCA_ACT_MIN_PRIO 1
-
 static int
 nl_parse_flower_actions(struct nlattr **attrs, struct tc_flower *flower,
                         bool terse)
diff --git a/lib/tc.h b/lib/tc.h
index 0f7de8677..5b1c934d8 100644
--- a/lib/tc.h
+++ b/lib/tc.h
@@ -58,6 +58,8 @@ enum tc_flower_reserved_prio {
 #define METER_ID_TO_POLICY_INDEX(meter_id) 0xff << 24 | (meter_id + 1) << 8
 /* Mapping policy_index to meter_id */
 #define POLICY_INDEX_TO_METER_ID(index) (((index >> 8) & 0xffff) - 1)
+/* Ckeck if given policy action is meter action*/
+#define UNLIKELY_METER_ACTION(index) !(index & (0xff << 24)) || !((index >> 8) & 0xffff)
 
 enum tc_qdisc_hook {
     TC_INGRESS,
@@ -284,7 +286,12 @@ enum tc_offloaded_state {
     TC_OFFLOADED_STATE_NOT_IN_HW,
 };
 
+#ifndef TCA_DUMP_FLAGS_TERSE
+#define TCA_DUMP_FLAGS_TERSE (1 << 0)
+#endif
+#define ACT_MAX_NUM 1024
 #define TCA_ACT_MAX_NUM 16
+#define TCA_ACT_MIN_PRIO 1
 
 struct tcf_id {
     enum tc_qdisc_hook hook;
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 1c9c720f0..091b0a626 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -2794,6 +2794,7 @@ revalidate(struct revalidator *revalidator)
         ovsrcu_quiesce();
     }
     dpif_flow_dump_thread_destroy(dump_thread);
+    dpif_meter_revalidate(udpif->dpif, udpif->backer);
     ofpbuf_uninit(&odp_actions);
 }
 
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 191cfcb0d..cf7ebc29d 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -401,4 +401,6 @@ bool ofproto_dpif_ct_zone_timeout_policy_get_name(
 
 bool ovs_explicit_drop_action_supported(struct ofproto_dpif *);
 
+void dpif_meter_revalidate(struct dpif *dpif, struct dpif_backer *backer);
+
 #endif /* ofproto-dpif.h */
-- 
2.20.1



More information about the dev mailing list