diff mbox

[ovs-dev,1/2] ovn-northd: Add hint in lflow to link back to acl

Message ID 1490249716-57081-1-git-send-email-zhouhan@gmail.com
State Superseded
Headers show

Commit Message

Han Zhou March 23, 2017, 6:15 a.m. UTC
It will be helpful for trouble-shooting if we can link a logical flow
back to the ACL that generated it. This patch is to add a stage-hint as
an external-id in lflow. The hint contains stage specific information.
Now only lflows in ACL stages have hint, which is the ACL uuid, though
the same mechanism can be used to add hint for other stages later.

Signed-off-by: Han Zhou <zhouhan@gmail.com>
---
 ovn/northd/ovn-northd.c | 79 +++++++++++++++++++++++++++++++------------------
 1 file changed, 51 insertions(+), 28 deletions(-)
diff mbox

Patch

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index cc9b934..f35e246 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1862,6 +1862,7 @@  struct ovn_lflow {
     uint16_t priority;
     char *match;
     char *actions;
+    char *stage_hint;
     const char *where;
 };
 
@@ -1887,13 +1888,15 @@  ovn_lflow_equal(const struct ovn_lflow *a, const struct ovn_lflow *b)
 static void
 ovn_lflow_init(struct ovn_lflow *lflow, struct ovn_datapath *od,
                enum ovn_stage stage, uint16_t priority,
-               char *match, char *actions, const char *where)
+               char *match, char *actions, char *stage_hint,
+               const char *where)
 {
     lflow->od = od;
     lflow->stage = stage;
     lflow->priority = priority;
     lflow->match = match;
     lflow->actions = actions;
+    lflow->stage_hint = stage_hint;
     lflow->where = where;
 }
 
@@ -1901,20 +1904,28 @@  ovn_lflow_init(struct ovn_lflow *lflow, struct ovn_datapath *od,
 static void
 ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od,
                  enum ovn_stage stage, uint16_t priority,
-                 const char *match, const char *actions, const char *where)
+                 const char *match, const char *actions,
+                 const char *stage_hint, const char *where)
 {
     ovs_assert(ovn_stage_to_datapath_type(stage) == ovn_datapath_get_type(od));
 
     struct ovn_lflow *lflow = xmalloc(sizeof *lflow);
     ovn_lflow_init(lflow, od, stage, priority,
-                   xstrdup(match), xstrdup(actions), where);
+                   xstrdup(match), xstrdup(actions),
+                   stage_hint?xstrdup(stage_hint):NULL,
+                   where);
     hmap_insert(lflow_map, &lflow->hmap_node, ovn_lflow_hash(lflow));
 }
 
 /* Adds a row with the specified contents to the Logical_Flow table. */
+#define ovn_lflow_add_with_hint(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, \
+                                ACTIONS, STAGE_HINT) \
+    ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, \
+                     STAGE_HINT, OVS_SOURCE_LOCATOR)
+
 #define ovn_lflow_add(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS) \
-    ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS,  \
-                     OVS_SOURCE_LOCATOR)
+    ovn_lflow_add_with_hint(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, \
+                            ACTIONS, NULL)
 
 static struct ovn_lflow *
 ovn_lflow_find(struct hmap *lflows, struct ovn_datapath *od,
@@ -1924,7 +1935,7 @@  ovn_lflow_find(struct hmap *lflows, struct ovn_datapath *od,
     struct ovn_lflow target;
     ovn_lflow_init(&target, od, stage, priority,
                    CONST_CAST(char *, match), CONST_CAST(char *, actions),
-                   NULL);
+                   NULL, NULL);
 
     struct ovn_lflow *lflow;
     HMAP_FOR_EACH_WITH_HASH (lflow, hmap_node, ovn_lflow_hash(&target),
@@ -1943,6 +1954,7 @@  ovn_lflow_destroy(struct hmap *lflows, struct ovn_lflow *lflow)
         hmap_remove(lflows, &lflow->hmap_node);
         free(lflow->match);
         free(lflow->actions);
+        free(lflow->stage_hint);
         free(lflow);
     }
 }
@@ -2710,6 +2722,7 @@  build_acls(struct ovn_datapath *od, struct hmap *lflows)
         bool ingress = !strcmp(acl->direction, "from-lport") ? true :false;
         enum ovn_stage stage = ingress ? S_SWITCH_IN_ACL : S_SWITCH_OUT_ACL;
 
+        char *stage_hint = xasprintf("%08x", acl->header_.uuid.parts[0]);
         if (!strcmp(acl->action, "allow")
             || !strcmp(acl->action, "allow-related")) {
             /* If there are any stateful flows, we must even commit "allow"
@@ -2718,9 +2731,9 @@  build_acls(struct ovn_datapath *od, struct hmap *lflows)
              * may and then its return traffic would not have an
              * associated conntrack entry and would return "+invalid". */
             if (!has_stateful) {
-                ovn_lflow_add(lflows, od, stage,
-                              acl->priority + OVN_ACL_PRI_OFFSET,
-                              acl->match, "next;");
+                ovn_lflow_add_with_hint(lflows, od, stage,
+                                        acl->priority + OVN_ACL_PRI_OFFSET,
+                                        acl->match, "next;", stage_hint);
             } else {
                 struct ds match = DS_EMPTY_INITIALIZER;
 
@@ -2740,10 +2753,11 @@  build_acls(struct ovn_datapath *od, struct hmap *lflows)
                                       " || (!ct.new && ct.est && !ct.rpl "
                                            "&& ct_label.blocked == 1)) "
                                       "&& (%s)", acl->match);
-                ovn_lflow_add(lflows, od, stage,
-                              acl->priority + OVN_ACL_PRI_OFFSET,
-                              ds_cstr(&match),
-                               REGBIT_CONNTRACK_COMMIT" = 1; next;");
+                ovn_lflow_add_with_hint(lflows, od, stage,
+                                        acl->priority + OVN_ACL_PRI_OFFSET,
+                                        ds_cstr(&match),
+                                        REGBIT_CONNTRACK_COMMIT" = 1; next;",
+                                        stage_hint);
 
                 /* Match on traffic in the request direction for an established
                  * connection tracking entry that has not been marked for
@@ -2756,9 +2770,10 @@  build_acls(struct ovn_datapath *od, struct hmap *lflows)
                               "!ct.new && ct.est && !ct.rpl"
                               " && ct_label.blocked == 0 && (%s)",
                               acl->match);
-                ovn_lflow_add(lflows, od, stage,
-                              acl->priority + OVN_ACL_PRI_OFFSET,
-                              ds_cstr(&match), "next;");
+                ovn_lflow_add_with_hint(lflows, od, stage,
+                                        acl->priority + OVN_ACL_PRI_OFFSET,
+                                        ds_cstr(&match), "next;",
+                                        stage_hint);
 
                 ds_destroy(&match);
             }
@@ -2782,8 +2797,10 @@  build_acls(struct ovn_datapath *od, struct hmap *lflows)
                               "(!ct.est || (ct.est && ct_label.blocked == 1)) "
                               "&& (%s)",
                               acl->match);
-                ovn_lflow_add(lflows, od, stage, acl->priority +
-                        OVN_ACL_PRI_OFFSET, ds_cstr(&match), "drop;");
+                ovn_lflow_add_with_hint(lflows, od, stage,
+                                        acl->priority + OVN_ACL_PRI_OFFSET,
+                                        ds_cstr(&match), "drop;",
+                                        stage_hint);
 
                 /* For an existing connection without ct_label set, we've
                  * encountered a policy change. ACLs previously allowed
@@ -2799,21 +2816,24 @@  build_acls(struct ovn_datapath *od, struct hmap *lflows)
                 ds_put_format(&match,
                               "ct.est && ct_label.blocked == 0 && (%s)",
                               acl->match);
-                ovn_lflow_add(lflows, od, stage,
-                              acl->priority + OVN_ACL_PRI_OFFSET,
-                              ds_cstr(&match), "ct_commit(ct_label=1/1);");
+                ovn_lflow_add_with_hint(lflows, od, stage,
+                                        acl->priority + OVN_ACL_PRI_OFFSET,
+                                        ds_cstr(&match),
+                                        "ct_commit(ct_label=1/1);",
+                                        stage_hint);
 
                 ds_destroy(&match);
             } else {
                 /* There are no stateful ACLs in use on this datapath,
                  * so a "drop" ACL is simply the "drop" logical flow action
                  * in all cases. */
-                ovn_lflow_add(lflows, od, stage,
-                              acl->priority + OVN_ACL_PRI_OFFSET,
-                              acl->match, "drop;");
+                ovn_lflow_add_with_hint(lflows, od, stage,
+                                        acl->priority + OVN_ACL_PRI_OFFSET,
+                                        acl->match, "drop;", stage_hint);
                 ds_destroy(&match);
             }
         }
+        free(stage_hint);
     }
 
     /* Add 34000 priority flow to allow DHCP reply from ovn-controller to all
@@ -5168,11 +5188,14 @@  build_lflows(struct northd_context *ctx, struct hmap *datapaths,
 #endif
         const char *where = slash ? slash + 1 : lflow->where;
 
-        const struct smap ids = SMAP_CONST2(
-            &ids,
-            "stage-name", ovn_stage_to_str(lflow->stage),
-            "source", where);
+        struct smap ids = SMAP_INITIALIZER(&ids);
+        smap_add(&ids, "stage-name", ovn_stage_to_str(lflow->stage));
+        smap_add(&ids, "source", where);
+        if (lflow->stage_hint) {
+            smap_add(&ids, "stage-hint", lflow->stage_hint);
+        }
         sbrec_logical_flow_set_external_ids(sbflow, &ids);
+        smap_destroy(&ids);
 
         ovn_lflow_destroy(&lflows, lflow);
     }