ovn: Change ct_label[0] to ct_label.blocked

ovn-northd uses ct_label[0] to keep track of the ACL changes on
existing connections.This patch replaces the usage of ct_label[0]
in the logical flows with a symbolic name ct_label.blocked

Suggested-by: Justin Pettit <jpettit@ovn.org>
Suggested-at: http://openvswitch.org/pipermail/dev/2016-July/075773.html
Signed-off-by: Babu Shanmugam <bschanmu@redhat.com>
Signed-off-by: Russell Bryant <russell@ovn.org>

4 files changed, 23 insertions, 26 deletions

diff --git a/ovn/TODO b/ovn/TODO
index 9a90a4a60..7d071d3c7 100644
--- a/ovn/TODO
+++ b/ovn/TODO
@@ -216,9 +216,3 @@ large.
 ** Support reject action.
 
 ** Support log option.
-
-** We currently use ct_label[0] to mark connections that have been blocked
-   by an ACL.  It would be nice in the logical flow syntax to have a
-   symbolic name like "ct_label.blocked" to make things more clear, as
-   well as to change the implementation of "blocked" in the future
-   if needed.
diff --git a/ovn/lib/logical-fields.c b/ovn/lib/logical-fields.c
index 6dbb4ae44..5229be37e 100644
--- a/ovn/lib/logical-fields.c
+++ b/ovn/lib/logical-fields.c
@@ -91,7 +91,10 @@ ovn_init_symtab(struct shash *symtab)
 
     /* Connection tracking state. */
     expr_symtab_add_field(symtab, "ct_mark", MFF_CT_MARK, NULL, false);
+
     expr_symtab_add_field(symtab, "ct_label", MFF_CT_LABEL, NULL, false);
+    expr_symtab_add_subfield(symtab, "ct_label.blocked", NULL, "ct_label[0]");
+
     expr_symtab_add_field(symtab, "ct_state", MFF_CT_STATE, NULL, false);
 
     struct ct_bit {
diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index 307e8be25..1da633a3f 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -332,12 +332,12 @@
         A priority-65535 flow that allows any traffic in the reply
         direction for a connection that has been committed to the
         connection tracker (i.e., established flows), as long as
-        the committed flow does not have <code>ct_label[0]</code> set.
+        the committed flow does not have <code>ct_label.blocked</code> set.
         We only handle traffic in the reply direction here because
         we want all packets going in the request direction to still
         go through the flows that implement the currently defined
         policy based on ACLs.  If a connection is no longer allowed by
-        policy, <code>ct_label[0]</code> will get set and packets in the
+        policy, <code>ct_label.blocked</code> will get set and packets in the
         reply direction will no longer be allowed, either.
       </li>
 
@@ -345,7 +345,7 @@
         A priority-65535 flow that allows any traffic that is considered
         related to a committed flow in the connection tracker (e.g., an
         ICMP Port Unreachable from a non-listening UDP port), as long
-        as the committed flow does not have <code>ct_label[0]</code> set.
+        as the committed flow does not have <code>ct_label.blocked</code> set.
       </li>
 
       <li>
@@ -355,7 +355,7 @@
 
       <li>
         A priority-65535 flow that drops all trafic in the reply direction
-        with <code>ct_label[0]</code> set meaning that the connection
+        with <code>ct_label.blocked</code> set meaning that the connection
         should no longer be allowed due to a policy change.  Packets
         in the request direction are skipped here to let a newly created
         ACL re-allow this connection.
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index a91e1f89f..2cbbb474e 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2256,18 +2256,18 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)
          * subsequent packets will hit the flow at priority 0 that just
          * uses "next;"
          *
-         * We also check for established connections that have ct_label[0]
+         * We also check for established connections that have ct_label.blocked
          * set on them.  That's a connection that was disallowed, but is
          * now allowed by policy again since it hit this default-allow flow.
-         * We need to set ct_label[0]=0 to let the connection continue,
+         * We need to set ct_label.blocked=0 to let the connection continue,
          * which will be done by ct_commit() in the "stateful" stage.
          * Subsequent packets will hit the flow at priority 0 that just
          * uses "next;". */
         ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, 1,
-                      "ip && (!ct.est || (ct.est && ct_label[0] == 1))",
+                      "ip && (!ct.est || (ct.est && ct_label.blocked == 1))",
                        REGBIT_CONNTRACK_COMMIT" = 1; next;");
         ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, 1,
-                      "ip && (!ct.est || (ct.est && ct_label[0] == 1))",
+                      "ip && (!ct.est || (ct.est && ct_label.blocked == 1))",
                        REGBIT_CONNTRACK_COMMIT" = 1; next;");
 
         /* Ingress and Egress ACL Table (Priority 65535).
@@ -2278,10 +2278,10 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)
          *
          * This is enforced at a higher priority than ACLs can be defined. */
         ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX,
-                      "ct.inv || (ct.est && ct.rpl && ct_label[0] == 1)",
+                      "ct.inv || (ct.est && ct.rpl && ct_label.blocked == 1)",
                       "drop;");
         ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX,
-                      "ct.inv || (ct.est && ct.rpl && ct_label[0] == 1)",
+                      "ct.inv || (ct.est && ct.rpl && ct_label.blocked == 1)",
                       "drop;");
 
         /* Ingress and Egress ACL Table (Priority 65535).
@@ -2295,11 +2295,11 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)
          * This is enforced at a higher priority than ACLs can be defined. */
         ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX,
                       "ct.est && !ct.rel && !ct.new && !ct.inv "
-                      "&& ct.rpl && ct_label[0] == 0",
+                      "&& ct.rpl && ct_label.blocked == 0",
                       "next;");
         ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX,
                       "ct.est && !ct.rel && !ct.new && !ct.inv "
-                      "&& ct.rpl && ct_label[0] == 0",
+                      "&& ct.rpl && ct_label.blocked == 0",
                       "next;");
 
         /* Ingress and Egress ACL Table (Priority 65535).
@@ -2315,11 +2315,11 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)
          * that's generated from a non-listening UDP port.  */
         ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX,
                       "!ct.est && ct.rel && !ct.new && !ct.inv "
-                      "&& ct_label[0] == 0",
+                      "&& ct_label.blocked == 0",
                       "next;");
         ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX,
                       "!ct.est && ct.rel && !ct.new && !ct.inv "
-                      "&& ct_label[0] == 0",
+                      "&& ct_label.blocked == 0",
                       "next;");
 
         /* Ingress and Egress ACL Table (Priority 65535).
@@ -2357,13 +2357,13 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)
                  * It's also possible that a known connection was marked for
                  * deletion after a policy was deleted, but the policy was
                  * re-added while that connection is still known.  We catch
-                 * that case here and un-set ct_label[0] (which will be done
+                 * that case here and un-set ct_label.blocked (which will be done
                  * by ct_commit in the "stateful" stage) to indicate that the
                  * connection should be allowed to resume.
                  */
                 ds_put_format(&match, "((ct.new && !ct.est)"
                                       " || (!ct.new && ct.est && !ct.rpl "
-                                           "&& ct_label[0] == 1)) "
+                                           "&& ct_label.blocked == 1)) "
                                       "&& (%s)", acl->match);
                 ovn_lflow_add(lflows, od, stage,
                               acl->priority + OVN_ACL_PRI_OFFSET,
@@ -2379,7 +2379,7 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)
                 ds_clear(&match);
                 ds_put_format(&match,
                               "!ct.new && ct.est && !ct.rpl"
-                              " && ct_label[0] == 0 && (%s)",
+                              " && ct_label.blocked == 0 && (%s)",
                               acl->match);
                 ovn_lflow_add(lflows, od, stage,
                               acl->priority + OVN_ACL_PRI_OFFSET,
@@ -2404,7 +2404,7 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)
                 /* If the packet is not part of an established connection, then
                  * we can simply drop it. */
                 ds_put_format(&match,
-                              "(!ct.est || (ct.est && ct_label[0] == 1)) "
+                              "(!ct.est || (ct.est && ct_label.blocked == 1)) "
                               "&& (%s)",
                               acl->match);
                 ovn_lflow_add(lflows, od, stage, acl->priority +
@@ -2422,7 +2422,7 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)
                  * dropping the packet, we go ahead and do it here. */
                 ds_clear(&match);
                 ds_put_format(&match,
-                              "ct.est && ct_label[0] == 0 && (%s)",
+                              "ct.est && ct_label.blocked == 0 && (%s)",
                               acl->match);
                 ovn_lflow_add(lflows, od, stage,
                               acl->priority + OVN_ACL_PRI_OFFSET,
@@ -2526,7 +2526,7 @@ build_stateful(struct ovn_datapath *od, struct hmap *lflows)
     ovn_lflow_add(lflows, od, S_SWITCH_OUT_STATEFUL, 0, "1", "next;");
 
     /* If REGBIT_CONNTRACK_COMMIT is set as 1, then the packets should be
-     * committed to conntrack. We always set ct_label[0] to 0 here as
+     * committed to conntrack. We always set ct_label.blocked to 0 here as
      * any packet that makes it this far is part of a connection we
      * want to allow to continue. */
     ovn_lflow_add(lflows, od, S_SWITCH_IN_STATEFUL, 100,