Hi,

Any comments or thoughts on this patch?

Thanks,
Roi


On 2021-01-26 9:17 AM, Roi Dayan wrote:
From: Jianbo Liu <jian...@nvidia.com>

Previously, only chain 0 is deleted before attach the ingress block.
If there are rules on the chain other than 0, these rules are not flushed.
In this case, the recreation of qdisc also fails. To fix this issue, flush
rules from all chains.

Signed-off-by: Jianbo Liu <jian...@nvidia.com>
Reviewed-by: Roi Dayan <r...@nvidia.com>
---
  lib/netdev-offload-tc.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++---
  lib/tc.c                | 39 +++++++++++++++++++++++
  lib/tc.h                |  2 ++
  3 files changed, 121 insertions(+), 5 deletions(-)

diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 717a987d14d8..bd7aa7725ad3 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -55,6 +55,11 @@ struct netlink_field {
      int size;
  };
+struct chain_node {
+    struct hmap_node node;
+    uint32_t chain;
+};
+
  static bool
  is_internal_port(const char *type)
  {
@@ -345,6 +350,69 @@ get_block_id_from_netdev(struct netdev *netdev)
  }
static int
+get_chains_from_netdev(struct netdev *netdev, struct tcf_id *id,
+                       struct hmap *map)
+{
+    struct netdev_flow_dump *dump;
+    struct chain_node *chain_node;
+    struct ofpbuf rbuffer, reply;
+    uint32_t chain;
+    size_t hash;
+    int err;
+
+    dump = xzalloc(sizeof *dump);
+    dump->nl_dump = xzalloc(sizeof *dump->nl_dump);
+    dump->netdev = netdev_ref(netdev);
+
+    ofpbuf_init(&rbuffer, NL_DUMP_BUFSIZE);
+    tc_dump_tc_chain_start(id, dump->nl_dump);
+
+    while (nl_dump_next(dump->nl_dump, &reply, &rbuffer)) {
+        if (parse_netlink_to_tc_chain(&reply, &chain)) {
+            continue;
+        }
+
+        chain_node = xzalloc(sizeof *chain_node);
+        chain_node->chain = chain;
+        hash = hash_int(chain, 0);
+        hmap_insert(map, &chain_node->node, hash);
+    }
+
+    err = nl_dump_done(dump->nl_dump);
+    ofpbuf_uninit(&rbuffer);
+    netdev_close(netdev);
+    free(dump->nl_dump);
+    free(dump);
+
+    return err;
+}
+
+static int
+delete_chains_from_netdev(struct netdev *netdev, struct tcf_id *id)
+{
+    struct chain_node *chain_node;
+    struct hmap map;
+    int error;
+
+    hmap_init(&map);
+    error = get_chains_from_netdev(netdev, id, &map);
+
+    if (!error) {
+        /* Flush rules explicitly needed when we work with ingress_block,
+         * so we will not fail with reattaching block to bond iface, for ex.
+         */
+        HMAP_FOR_EACH_POP (chain_node, node, &map) {
+            id->chain = chain_node->chain;
+            tc_del_filter(id);
+            free(chain_node);
+        }
+    }
+
+    hmap_destroy(&map);
+    return error;
+}
+
+static int
  netdev_tc_flow_flush(struct netdev *netdev)
  {
      struct ufid_tc_data *data, *next;
@@ -2008,6 +2076,7 @@ netdev_tc_init_flow_api(struct netdev *netdev)
  {
      static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
      enum tc_qdisc_hook hook = get_tc_qdisc_hook(netdev);
+    static bool get_chain_supported = true;
      uint32_t block_id = 0;
      struct tcf_id id;
      int ifindex;
@@ -2021,12 +2090,18 @@ netdev_tc_init_flow_api(struct netdev *netdev)
      }
block_id = get_block_id_from_netdev(netdev);
-
-    /* Flush rules explicitly needed when we work with ingress_block,
-     * so we will not fail with reattaching block to bond iface, for ex.
-     */
      id = tc_make_tcf_id(ifindex, block_id, 0, hook);
-    tc_del_filter(&id);
+
+    if (get_chain_supported) {
+        if (delete_chains_from_netdev(netdev, &id)) {
+            get_chain_supported = false;
+        }
+    }
+
+    /* fallback here if delete chains fail */
+    if (!get_chain_supported) {
+        tc_del_filter(&id);
+    }
/* make sure there is no ingress/egress qdisc */
      tc_add_del_qdisc(ifindex, false, 0, hook);
diff --git a/lib/tc.c b/lib/tc.c
index c2de78bfe347..74a8113eaa0f 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -61,6 +61,10 @@
  #define TCA_DUMP_FLAGS 15
  #endif
+#ifndef RTM_GETCHAIN
+#define RTM_GETCHAIN 102
+#endif
+
  VLOG_DEFINE_THIS_MODULE(tc);
static struct vlog_rate_limit error_rl = VLOG_RATE_LIMIT_INIT(60, 5);
@@ -297,6 +301,10 @@ static const struct nl_policy tca_policy[] = {
      [TCA_STATS2] = { .type = NL_A_NESTED, .optional = true, },
  };
+static const struct nl_policy tca_chain_policy[] = {
+    [TCA_CHAIN] = { .type = NL_A_U32, .optional = false, },
+};
+
  static const struct nl_policy tca_flower_policy[] = {
      [TCA_FLOWER_CLASSID] = { .type = NL_A_U32, .optional = true, },
      [TCA_FLOWER_INDEV] = { .type = NL_A_STRING, .max_len = IFNAMSIZ,
@@ -1864,6 +1872,25 @@ parse_netlink_to_tc_flower(struct ofpbuf *reply, struct 
tcf_id *id,
  }
int
+parse_netlink_to_tc_chain(struct ofpbuf *reply, uint32_t *chain)
+{
+    struct nlattr *ta[ARRAY_SIZE(tca_chain_policy)];
+    struct tcmsg *tc;
+
+    tc = ofpbuf_at_assert(reply, NLMSG_HDRLEN, sizeof *tc);
+
+    if (!nl_policy_parse(reply, NLMSG_HDRLEN + sizeof *tc,
+                         tca_chain_policy, ta, ARRAY_SIZE(ta))) {
+        VLOG_ERR_RL(&error_rl, "failed to parse tca chain policy");
+        return EINVAL;
+    }
+
+   *chain = nl_attr_get_u32(ta[TCA_CHAIN]);
+
+    return 0;
+}
+
+int
  tc_dump_flower_start(struct tcf_id *id, struct nl_dump *dump, bool terse)
  {
      struct ofpbuf request;
@@ -1883,6 +1910,18 @@ tc_dump_flower_start(struct tcf_id *id, struct nl_dump 
*dump, bool terse)
  }
int
+tc_dump_tc_chain_start(struct tcf_id *id, struct nl_dump *dump)
+{
+    struct ofpbuf request;
+
+    request_from_tcf_id(id, 0, RTM_GETCHAIN, NLM_F_DUMP, &request);
+    nl_dump_start(dump, NETLINK_ROUTE, &request);
+    ofpbuf_uninit(&request);
+
+    return 0;
+}
+
+int
  tc_del_filter(struct tcf_id *id)
  {
      struct ofpbuf request;
diff --git a/lib/tc.h b/lib/tc.h
index 281231c0d3f1..d4a304ea6faa 100644
--- a/lib/tc.h
+++ b/lib/tc.h
@@ -360,10 +360,12 @@ int tc_replace_flower(struct tcf_id *id, struct tc_flower 
*flower);
  int tc_del_filter(struct tcf_id *id);
  int tc_get_flower(struct tcf_id *id, struct tc_flower *flower);
  int tc_dump_flower_start(struct tcf_id *id, struct nl_dump *dump, bool terse);
+int tc_dump_tc_chain_start(struct tcf_id *id, struct nl_dump *dump);
  int parse_netlink_to_tc_flower(struct ofpbuf *reply,
                                 struct tcf_id *id,
                                 struct tc_flower *flower,
                                 bool terse);
+int parse_netlink_to_tc_chain(struct ofpbuf *reply, uint32_t *chain);
  void tc_set_policy(const char *policy);
#endif /* tc.h */

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to