[ovs-dev] [PATCH ovn v4] northd: Add ACL label

Priyankar Jain priyankar.jain at nutanix.com
Mon Aug 2 08:32:42 UTC 2021


Allow adding label to an ACL to identify which ACL allowed a particular
flow in the connection tracking table.

The ACL label covers 32 bits at the end of ct_label. Since only allowed
connections are committed, only "allow" and "allow-related" ACLs can
have the label.

If the ACL allowing the connection changes, the label associated with the
new ACL gets updated in the ct_label field. This is achieved by committing
every packet that hits the ACL with the label to the connection tracking
table.
In case the new ACL doesn't have a label, the ct_label field is not
cleared. This is done to prevent any performance change with ACLs that
don't have label set.
For the packets which hits an ACL without label, the behaviour remains the
same as before with respect to the conntrack commit.

Performance:
We used ftrace to measure the time taken by an extra conntrack commit for
the packets hitting the ACL with label. We measured the time taken to
execute the ovs_ct_execute call inside the sock_sendmsg call.

ACL used :-
 from-lport  2000 (tcp && ip4.src == 10.0.0.11 && ip4.dst == 10.0.0.12) allow-related --label=1234

It was observed that the extra ovs_ct_execute call accounted for 1-2%
of the round trip time (sock_sendmsg duration). The actual percentage
is expected to be lesser since it doesn't take into account the tracing
overhead which is substantial for smaller functions.

Signed-off-by: Priyankar Jain <priyankar.jain at nutanix.com>
Acked-by: Numan Siddique <numans at ovn.org>
---

v3 -> v4
* Resolved merge conflicts with master.

v2 -> v3
* Added NEWS entry.

v1 -> v2
* Changed type of label from string (hex) to unsigned integer.
* Added two system tests for checking the correct ct_label programming.
* Updated ovn-northd.8.xml with the new logical flows added as part of
  this patch.
* Rebased with latest master.

 NEWS                      |   3 +
 lib/logical-fields.c      |   2 +
 northd/ovn-northd.8.xml   |  26 +++-
 northd/ovn-northd.c       |  53 +++++++--
 northd/ovn_northd.dl      |  76 +++++++++---
 ovn-nb.ovsschema          |   7 +-
 ovn-nb.xml                |  12 ++
 tests/ovn-nbctl.at        |  19 +++
 tests/ovn-northd.at       | 107 ++++++++++++++++-
 tests/ovn.at              |   1 +
 tests/system-ovn.at       | 244 ++++++++++++++++++++++++++++++++++++++
 utilities/ovn-nbctl.8.xml |   2 +-
 utilities/ovn-nbctl.c     |  38 +++++-
 13 files changed, 548 insertions(+), 42 deletions(-)

diff --git a/NEWS b/NEWS
index f328666da..45dc45353 100644
--- a/NEWS
+++ b/NEWS
@@ -3,6 +3,9 @@ Post-v21.06.0
   - Added Control Plane Protection support (control plane traffic metering).
   - Added path MTU discovery for ingress traffic originated outside of the
     cluster.
+  - Introduced a new "label" field for "allow" and "allow-related" ACLs
+    which helps in debugging/backtracking the ACL which allowed a particular
+    connection.
 
 OVN v21.06.0 - 18 Jun 2021
 -------------------------
diff --git a/lib/logical-fields.c b/lib/logical-fields.c
index 72853013e..7b3d431e0 100644
--- a/lib/logical-fields.c
+++ b/lib/logical-fields.c
@@ -146,6 +146,8 @@ ovn_init_symtab(struct shash *symtab)
                                     "ct_label[32..79]", WR_CT_COMMIT);
     expr_symtab_add_subfield_scoped(symtab, "ct_label.ecmp_reply_port", NULL,
                                     "ct_label[80..95]", WR_CT_COMMIT);
+    expr_symtab_add_subfield_scoped(symtab, "ct_label.label", NULL,
+                                    "ct_label[96..127]", WR_CT_COMMIT);
 
     expr_symtab_add_field(symtab, "ct_state", MFF_CT_STATE, NULL, false);
 
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 08d484760..c5ea15774 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -670,13 +670,19 @@
         the <code>next;</code> action.  If there are any stateful ACLs
         on this datapath, then <code>allow</code> ACLs translate to
         <code>ct_commit; next;</code> (which acts as a hint for the next tables
-        to commit the connection to conntrack),
+        to commit the connection to conntrack). In case the <code>ACL</code>
+        has a label then <code>reg3</code> is loaded with the label value and
+        <code>reg0[13]</code> bit is set to 1 (which acts as a hint for the
+        next tables to commit the label to conntrack).
       </li>
       <li>
         <code>allow-related</code> ACLs translate into logical
         flows with the <code>ct_commit(ct_label=0/1); next;</code> actions
         for new connections and <code>reg0[1] = 1; next;</code> for existing
-        connections.
+        connections.  In case the <code>ACL</code> has a label then
+        <code>reg3</code> is loaded with the label value and
+        <code>reg0[13]</code> bit is set to 1 (which acts as a hint for the
+        next tables to commit the label to conntrack).
       </li>
       <li>
         <code>allow-stateless</code> ACLs translate into logical
@@ -876,9 +882,19 @@
       </li>
 
       <li>
-        A priority-100 flow commits packets to connection tracker using
-        <code>ct_commit; next;</code> action based on a hint provided by
-        the previous tables (with a match for <code>reg0[1] == 1</code>).
+        A priority 100 flow is added which commits the packet to the conntrack
+        and sets the most significant 32-bits of <code>ct_label</code> with the
+        <code>reg3</code> value based on the hint provided by previous tables
+        (with a match for <code>reg0[1] == 1 && reg0[13] == 1</code>).
+        This is used by the <code>ACLs</code> with label to commit the label
+        value to conntrack.
+      </li>
+
+      <li>
+        For <code>ACLs</code> without label, a second priority-100 flow commits
+        packets to connection tracker using <code>ct_commit; next;</code>
+        action based on a hint provided by the previous tables (with a match
+        for <code>reg0[1] == 1 && reg0[13] == 0</code>).
       </li>
       <li>
         A priority-0 flow that simply moves traffic to the next table.
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index a0eaa1247..1ad15b978 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -241,6 +241,7 @@ enum ovn_stage {
 #define REGBIT_ACL_HINT_BLOCK     "reg0[10]"
 #define REGBIT_LKUP_FDB           "reg0[11]"
 #define REGBIT_HAIRPIN_REPLY      "reg0[12]"
+#define REGBIT_ACL_LABEL          "reg0[13]"
 
 #define REG_ORIG_DIP_IPV4         "reg1"
 #define REG_ORIG_DIP_IPV6         "xxreg1"
@@ -273,6 +274,9 @@ enum ovn_stage {
 #define REG_SRC_IPV4 "reg1"
 #define REG_SRC_IPV6 "xxreg1"
 
+/* Register used for setting a label for ACLs in a Logical Switch. */
+#define REG_LABEL "reg3"
+
 #define FLAGBIT_NOT_VXLAN "flags[1] == 0"
 
 /*
@@ -281,14 +285,15 @@ enum ovn_stage {
  * Logical Switch pipeline:
  * +----+----------------------------------------------+---+------------------+
  * | R0 |     REGBIT_{CONNTRACK/DHCP/DNS}              |   |                  |
- * |    |     REGBIT_{HAIRPIN/HAIRPIN_REPLY}           | X |                  |
- * |    | REGBIT_ACL_HINT_{ALLOW_NEW/ALLOW/DROP/BLOCK} | X |                  |
+ * |    |     REGBIT_{HAIRPIN/HAIRPIN_REPLY}           |   |                  |
+ * |    | REGBIT_ACL_HINT_{ALLOW_NEW/ALLOW/DROP/BLOCK} |   |                  |
+ * |    |     REGBIT_ACL_LABEL                         | X |                  |
  * +----+----------------------------------------------+ X |                  |
  * | R1 |         ORIG_DIP_IPV4 (>= IN_STATEFUL)       | R |                  |
  * +----+----------------------------------------------+ E |                  |
  * | R2 |         ORIG_TP_DPORT (>= IN_STATEFUL)       | G |                  |
  * +----+----------------------------------------------+ 0 |                  |
- * | R3 |                   UNUSED                     |   |                  |
+ * | R3 |                  ACL LABEL                   |   |                  |
  * +----+----------------------------------------------+---+------------------+
  * | R4 |                   UNUSED                     |   |                  |
  * +----+----------------------------------------------+ X |   ORIG_DIP_IPV6  |
@@ -5774,7 +5779,12 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od,
             ds_clear(actions);
             ds_put_format(match, REGBIT_ACL_HINT_ALLOW_NEW " == 1 && (%s)",
                           acl->match);
+
             ds_put_cstr(actions, REGBIT_CONNTRACK_COMMIT" = 1; ");
+            if (acl->label) {
+                ds_put_format(actions, REGBIT_ACL_LABEL" = 1; "
+                              REG_LABEL" = %"PRId64"; ", acl->label);
+            }
             build_acl_log(actions, acl, meter_groups);
             ds_put_cstr(actions, "next;");
             ovn_lflow_add_with_hint(lflows, od, stage,
@@ -5785,15 +5795,21 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od,
 
             /* Match on traffic in the request direction for an established
              * connection tracking entry that has not been marked for
-             * deletion.  There is no need to commit here, so we can just
-             * proceed to the next table. We use this to ensure that this
+             * deletion. We use this to ensure that this
              * connection is still allowed by the currently defined
-             * policy. Match untracked packets too. */
+             * policy. Match untracked packets too.
+             * Commit the connection only if the ACL has a label. This is done
+             * to update the connection tracking entry label in case the ACL
+             * allowing the connection changes. */
             ds_clear(match);
             ds_clear(actions);
             ds_put_format(match, REGBIT_ACL_HINT_ALLOW " == 1 && (%s)",
                           acl->match);
-
+            if (acl->label) {
+                ds_put_cstr(actions, REGBIT_CONNTRACK_COMMIT" = 1; ");
+                ds_put_format(actions, REGBIT_ACL_LABEL" = 1; "
+                              REG_LABEL" = %"PRId64"; ", acl->label);
+            }
             build_acl_log(actions, acl, meter_groups);
             ds_put_cstr(actions, "next;");
             ovn_lflow_add_with_hint(lflows, od, stage,
@@ -6292,15 +6308,34 @@ build_stateful(struct ovn_datapath *od, struct hmap *lflows)
     ovn_lflow_add(lflows, od, S_SWITCH_IN_STATEFUL, 0, "1", "next;");
     ovn_lflow_add(lflows, od, S_SWITCH_OUT_STATEFUL, 0, "1", "next;");
 
+    /* If REGBIT_CONNTRACK_COMMIT is set as 1 and
+     * REGBIT_CONNTRACK_SET_LABEL is set to 1, then the packets should be
+     * committed to conntrack.
+     * We always set ct_mark.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,
+                  REGBIT_CONNTRACK_COMMIT" == 1 && "
+                  REGBIT_ACL_LABEL" == 1",
+                  "ct_commit { ct_label.blocked = 0; "
+                  "ct_label.label = " REG_LABEL "; }; next;");
+    ovn_lflow_add(lflows, od, S_SWITCH_OUT_STATEFUL, 100,
+                  REGBIT_CONNTRACK_COMMIT" == 1 && "
+                  REGBIT_ACL_LABEL" == 1",
+                  "ct_commit { ct_label.blocked = 0; "
+                  "ct_label.label = " REG_LABEL "; }; next;");
+
     /* If REGBIT_CONNTRACK_COMMIT is set as 1, then the packets should be
      * 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,
-                  REGBIT_CONNTRACK_COMMIT" == 1",
+                  REGBIT_CONNTRACK_COMMIT" == 1 && "
+                  REGBIT_ACL_LABEL" == 0",
                   "ct_commit { ct_label.blocked = 0; }; next;");
     ovn_lflow_add(lflows, od, S_SWITCH_OUT_STATEFUL, 100,
-                  REGBIT_CONNTRACK_COMMIT" == 1",
+                  REGBIT_CONNTRACK_COMMIT" == 1 && "
+                  REGBIT_ACL_LABEL" == 0",
                   "ct_commit { ct_label.blocked = 0; }; next;");
 }
 
diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
index 091fe10b3..b3d1a9657 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -1505,13 +1505,14 @@ function s_ROUTER_OUT_DELIVERY():       Stage { Stage{ Egress,  4, "lr_out_deliv
  * Logical Switch pipeline:
  * +----+----------------------------------------------+---+------------------+
  * | R0 |     REGBIT_{CONNTRACK/DHCP/DNS}              |   |                  |
- * |    |     REGBIT_{HAIRPIN/HAIRPIN_REPLY}           | X |                  |
+ * |    |     REGBIT_{HAIRPIN/HAIRPIN_REPLY}           |   |                  |
+ * |    |     REGBIT_ACL_LABEL                         | X |                  |
  * +----+----------------------------------------------+ X |                  |
  * | R1 |         ORIG_DIP_IPV4 (>= IN_STATEFUL)       | R |                  |
  * +----+----------------------------------------------+ E |                  |
  * | R2 |         ORIG_TP_DPORT (>= IN_STATEFUL)       | G |                  |
  * +----+----------------------------------------------+ 0 |                  |
- * | R3 |                   UNUSED                     |   |                  |
+ * | R3 |                   ACL_LABEL                  |   |                  |
  * +----+----------------------------------------------+---+------------------+
  * | R4 |                   UNUSED                     |   |                  |
  * +----+----------------------------------------------+ X |   ORIG_DIP_IPV6  |
@@ -1584,6 +1585,7 @@ function rEGBIT_ACL_HINT_DROP()    : string = "reg0[9]"
 function rEGBIT_ACL_HINT_BLOCK()   : string = "reg0[10]"
 function rEGBIT_LKUP_FDB()         : string = "reg0[11]"
 function rEGBIT_HAIRPIN_REPLY()    : string = "reg0[12]"
+function rEGBIT_ACL_LABEL()        : string = "reg0[13]"
 
 function rEG_ORIG_DIP_IPV4()       : string = "reg1"
 function rEG_ORIG_DIP_IPV6()       : string = "xxreg1"
@@ -1610,6 +1612,9 @@ function rEG_INPORT_ETH_ADDR() : string = "xreg0[0..47]"
 function rEG_ECMP_GROUP_ID()  : string = "reg8[0..15]"
 function rEG_ECMP_MEMBER_ID() : string = "reg8[16..31]"
 
+/* Register used for setting a label for ACLs in a Logical Switch. */
+function rEG_LABEL() : string = "reg3"
+
 function fLAGBIT_NOT_VXLAN() : string = "flags[1] == 0"
 
 function mFF_N_LOG_REGS()          : bit<32> = 10
@@ -2698,26 +2703,41 @@ for (&SwitchACL(.sw = sw, .acl = acl, .has_fair_meter = fair_meter)) {
              * 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.
+             * If the ACL has a label, then load REG_LABEL with the label and
+             * set the REGBIT_ACL_LABEL field.
              */
-            Flow(.logical_datapath = sw._uuid,
-                 .stage            = stage,
-                 .priority         = acl.priority + oVN_ACL_PRI_OFFSET(),
-                 .__match          = "${rEGBIT_ACL_HINT_ALLOW_NEW()} == 1 && (${acl.__match})",
-                 .actions          = "${rEGBIT_CONNTRACK_COMMIT()} = 1; ${acl_log}next;",
-                 .external_ids     = stage_hint);
+            var __action = if (acl.label != 0) {
+                "${rEGBIT_CONNTRACK_COMMIT()} = 1; ${rEGBIT_ACL_LABEL()} = 1;\
+                 \ ${rEG_LABEL()} = ${acl.label}; ${acl_log}next;"
+            } else {
+                "${rEGBIT_CONNTRACK_COMMIT()} = 1; ${acl_log}next;"
+            } in Flow(.logical_datapath = sw._uuid,
+                      .stage            = stage,
+                      .priority         = acl.priority + oVN_ACL_PRI_OFFSET(),
+                      .__match          = "${rEGBIT_ACL_HINT_ALLOW_NEW()} == 1 && (${acl.__match})",
+                      .actions          = __action,
+                      .external_ids     = stage_hint);
 
             /* Match on traffic in the request direction for an established
              * connection tracking entry that has not been marked for
-             * deletion.  There is no need to commit here, so we can just
-             * proceed to the next table. We use this to ensure that this
+             * deletion. We use this to ensure that this
              * connection is still allowed by the currently defined
-             * policy. Match untracked packets too. */
-            Flow(.logical_datapath = sw._uuid,
-                 .stage            = stage,
-                 .priority         = acl.priority + oVN_ACL_PRI_OFFSET(),
-                 .__match          = "${rEGBIT_ACL_HINT_ALLOW()} == 1 && (${acl.__match})",
-                 .actions          = "${acl_log}next;",
-                 .external_ids     = stage_hint)
+             * policy. Match untracked packets too.
+             * Commit the connection only if the ACL has a label. This is done to
+             * update the connection tracking entry label in case the ACL
+             * allowing the connection changes.
+             */
+            var __action = if (acl.label != 0) {
+                "${rEGBIT_CONNTRACK_COMMIT()} = 1; ${rEGBIT_ACL_LABEL()} = 1;\
+                 \ ${rEG_LABEL()} = ${acl.label}; ${acl_log}next;"
+            } else {
+                "${acl_log}next;"
+            } in Flow(.logical_datapath = sw._uuid,
+                      .stage            = stage,
+                      .priority         = acl.priority + oVN_ACL_PRI_OFFSET(),
+                      .__match          = "${rEGBIT_ACL_HINT_ALLOW()} == 1 && (${acl.__match})",
+                      .actions          = __action,
+                      .external_ids     = stage_hint)
         }
     } else if (acl.action == "allow-stateless") {
         Flow(.logical_datapath = sw._uuid,
@@ -2934,6 +2954,24 @@ for (&Switch(._uuid = ls_uuid)) {
          .actions          = "next;",
          .external_ids     = map_empty());
 
+    /* If REGBIT_CONNTRACK_COMMIT is set as 1 and REGBIT_CONNTRACK_SET_LABEL
+     * is set to 1, then the packets should be
+     * 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. */
+    Flow(.logical_datapath = ls_uuid,
+         .stage            = s_SWITCH_IN_STATEFUL(),
+         .priority         = 100,
+         .__match          = "${rEGBIT_CONNTRACK_COMMIT()} == 1 && ${rEGBIT_ACL_LABEL()} == 1",
+         .actions          = "ct_commit { ct_label.blocked = 0; ct_label.label = ${rEG_LABEL()}; }; next;",
+         .external_ids     = map_empty());
+    Flow(.logical_datapath = ls_uuid,
+         .stage            = s_SWITCH_OUT_STATEFUL(),
+         .priority         = 100,
+         .__match          = "${rEGBIT_CONNTRACK_COMMIT()} == 1 && ${rEGBIT_ACL_LABEL()} == 1",
+         .actions          = "ct_commit { ct_label.blocked = 0; ct_label.label = ${rEG_LABEL()}; }; next;",
+         .external_ids     = map_empty());
+
     /* If REGBIT_CONNTRACK_COMMIT is set as 1, then the packets should be
      * 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
@@ -2941,13 +2979,13 @@ for (&Switch(._uuid = ls_uuid)) {
     Flow(.logical_datapath = ls_uuid,
          .stage            = s_SWITCH_IN_STATEFUL(),
          .priority         = 100,
-         .__match          = "${rEGBIT_CONNTRACK_COMMIT()} == 1",
+         .__match          = "${rEGBIT_CONNTRACK_COMMIT()} == 1 && ${rEGBIT_ACL_LABEL()} == 0",
          .actions          = "ct_commit { ct_label.blocked = 0; }; next;",
          .external_ids     = map_empty());
     Flow(.logical_datapath = ls_uuid,
          .stage            = s_SWITCH_OUT_STATEFUL(),
          .priority         = 100,
-         .__match          = "${rEGBIT_CONNTRACK_COMMIT()} == 1",
+         .__match          = "${rEGBIT_CONNTRACK_COMMIT()} == 1 && ${rEGBIT_ACL_LABEL()} == 0",
          .actions          = "ct_commit { ct_label.blocked = 0; }; next;",
          .external_ids     = map_empty())
 }
diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
index a53d5e851..2ac8ef3ea 100644
--- a/ovn-nb.ovsschema
+++ b/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
 {
     "name": "OVN_Northbound",
-    "version": "5.32.0",
-    "cksum": "2501921026 29540",
+    "version": "5.32.1",
+    "cksum": "2805328215 29734",
     "tables": {
         "NB_Global": {
             "columns": {
@@ -244,6 +244,9 @@
                                                         "debug"]]},
                                       "min": 0, "max": 1}},
                 "meter": {"type": {"key": "string", "min": 0, "max": 1}},
+                "label": {"type": {"key": {"type": "integer",
+                                           "minInteger": 0,
+                                           "maxInteger": 4294967295}}},
                 "external_ids": {
                     "type": {"key": "string", "value": "string",
                              "min": 0, "max": "unlimited"}}},
diff --git a/ovn-nb.xml b/ovn-nb.xml
index c1176e81f..ebafa3e5e 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -1853,6 +1853,18 @@
       and <code>deny</code> as <ref column="action"/>.)
     </p>
 
+    <column name="label">
+      <p>
+        Associates an identifier with the ACL.
+        The same value will be written to corresponding connection
+        tracker entry. The value should be a valid 32-bit unsigned integer.
+        This value can help in debugging from connection tracker side.
+        For example, through this "label" we can backtrack to the ACL rule
+        which is causing a "leaked" connection. Connection tracker entries are
+        created only for allowed connections so the label is valid only
+        for allow and allow-related actions.
+      </p>
+    </column>
     <column name="priority">
       <p>
         The ACL rule's priority.  Rules with numerically higher priority
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 828777b82..4445ab7c0 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -211,18 +211,36 @@ ovn_nbctl_test_acl() {
    AT_CHECK([ovn-nbctl $2 acl-add $1 to-lport 300 tcp drop])
    AT_CHECK([ovn-nbctl $2 acl-add $1 from-lport 200 ip drop])
    AT_CHECK([ovn-nbctl $2 acl-add $1 to-lport 100 ip drop])
+   AT_CHECK([ovn-nbctl $2 --label=1234 acl-add $1 from-lport 70 icmp allow-related])
+   AT_CHECK([ovn-nbctl $2 --label=1235 acl-add $1 to-lport 70 icmp allow-related])
+
    dnl Add duplicated ACL
    AT_CHECK([ovn-nbctl $2 acl-add $1 to-lport 100 ip drop], [1], [], [stderr])
    AT_CHECK([grep 'already existed' stderr], [0], [ignore])
    AT_CHECK([ovn-nbctl $2 --may-exist acl-add $1 to-lport 100 ip drop])
 
+   dnl Add invalid ACL label
+   AT_CHECK([ovn-nbctl $2 --label=1234 acl-add $1 to-lport 50 ip drop], [1], [], [stderr])
+   AT_CHECK([grep 'can only be set with actions' stderr], [0], [ignore])
+
+   AT_CHECK([ovn-nbctl $2 --label=abcd acl-add $1 to-lport 50 ip allow-related], [1], [], [stderr])
+   AT_CHECK([grep 'label must in range 0...4294967295' stderr], [0], [ignore])
+
+   AT_CHECK([ovn-nbctl $2 --label=-1 acl-add $1 to-lport 50 ip allow-related], [1], [], [stderr])
+   AT_CHECK([grep 'label must in range 0...4294967295' stderr], [0], [ignore])
+
+   AT_CHECK([ovn-nbctl $2 --label=4294967296 acl-add $1 to-lport 50 ip allow-related], [1], [], [stderr])
+   AT_CHECK([grep 'label must in range 0...4294967295' stderr], [0], [ignore])
+
    AT_CHECK([ovn-nbctl $2 acl-list $1], [0], [dnl
 from-lport   600 (udp) drop log()
 from-lport   400 (tcp) drop
 from-lport   200 (ip) drop
+from-lport    70 (icmp) allow-related label=1234
   to-lport   500 (udp) drop log(name=test,severity=info)
   to-lport   300 (tcp) drop
   to-lport   100 (ip) drop
+  to-lport    70 (icmp) allow-related label=1235
 ])
 
    dnl Delete in one direction.
@@ -231,6 +249,7 @@ from-lport   200 (ip) drop
 from-lport   600 (udp) drop log()
 from-lport   400 (tcp) drop
 from-lport   200 (ip) drop
+from-lport    70 (icmp) allow-related label=1234
 ])
 
    dnl Delete all ACLs.
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 474e88021..ba79106a1 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -3608,7 +3608,8 @@ check_stateful_flows() {
 
     AT_CHECK([grep "ls_in_stateful" sw0flows | sort], [0], [dnl
   table=12(ls_in_stateful     ), priority=0    , match=(1), action=(next;)
-  table=12(ls_in_stateful     ), priority=100  , match=(reg0[[1]] == 1), action=(ct_commit { ct_label.blocked = 0; }; next;)
+  table=12(ls_in_stateful     ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;)
+  table=12(ls_in_stateful     ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;)
   table=12(ls_in_stateful     ), priority=120  , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg1 = 10.0.0.10; reg2[[0..15]] = 80; ct_lb(backends=10.0.0.4:8080);)
 ])
 
@@ -3630,7 +3631,8 @@ check_stateful_flows() {
 
     AT_CHECK([grep "ls_out_stateful" sw0flows | sort], [0], [dnl
   table=7 (ls_out_stateful    ), priority=0    , match=(1), action=(next;)
-  table=7 (ls_out_stateful    ), priority=100  , match=(reg0[[1]] == 1), action=(ct_commit { ct_label.blocked = 0; }; next;)
+  table=7 (ls_out_stateful    ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;)
+  table=7 (ls_out_stateful    ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;)
 ])
 }
 
@@ -3669,7 +3671,8 @@ AT_CHECK([grep "ls_in_pre_stateful" sw0flows | sort], [0], [dnl
 
 AT_CHECK([grep "ls_in_stateful" sw0flows | sort], [0], [dnl
   table=12(ls_in_stateful     ), priority=0    , match=(1), action=(next;)
-  table=12(ls_in_stateful     ), priority=100  , match=(reg0[[1]] == 1), action=(ct_commit { ct_label.blocked = 0; }; next;)
+  table=12(ls_in_stateful     ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;)
+  table=12(ls_in_stateful     ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;)
 ])
 
 AT_CHECK([grep "ls_out_pre_lb" sw0flows | sort], [0], [dnl
@@ -3687,14 +3690,108 @@ AT_CHECK([grep "ls_out_pre_stateful" sw0flows | sort], [0], [dnl
 
 AT_CHECK([grep "ls_out_stateful" sw0flows | sort], [0], [dnl
   table=7 (ls_out_stateful    ), priority=0    , match=(1), action=(next;)
-  table=7 (ls_out_stateful    ), priority=100  , match=(reg0[[1]] == 1), action=(ct_commit { ct_label.blocked = 0; }; next;)
+  table=7 (ls_out_stateful    ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;)
+  table=7 (ls_out_stateful    ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;)
 ])
 
 AT_CLEANUP
 ])
 
 OVN_FOR_EACH_NORTHD([
-AT_SETUP([ct.inv usage])
+AT_SETUP([ovn -- ACL label usage])
+ovn_start
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl lsp-add sw0 sw0p1
+
+check ovn-nbctl --wait=sb --label=1234 acl-add sw0 to-lport 1002 tcp allow-related
+check ovn-nbctl --wait=sb --label=1234 acl-add sw0 from-lport 1002 tcp allow-related
+
+ovn-sbctl dump-flows sw0 > sw0flows
+AT_CAPTURE_FILE([sw0flows])
+
+AT_CHECK([grep -w "ls_in_acl" sw0flows | grep 2002 | sort], [0], [dnl
+  table=9 (ls_in_acl          ), priority=2002 , match=(reg0[[7]] == 1 && (tcp)), action=(reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 1234; next;)
+  table=9 (ls_in_acl          ), priority=2002 , match=(reg0[[8]] == 1 && (tcp)), action=(reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 1234; next;)
+])
+AT_CHECK([grep "ls_in_stateful" sw0flows | sort], [0], [dnl
+  table=12(ls_in_stateful     ), priority=0    , match=(1), action=(next;)
+  table=12(ls_in_stateful     ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;)
+  table=12(ls_in_stateful     ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;)
+])
+
+AT_CHECK([grep -w "ls_out_acl" sw0flows | grep 2002 | sort], [0], [dnl
+  table=4 (ls_out_acl         ), priority=2002 , match=(reg0[[7]] == 1 && (tcp)), action=(reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 1234; next;)
+  table=4 (ls_out_acl         ), priority=2002 , match=(reg0[[8]] == 1 && (tcp)), action=(reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 1234; next;)
+])
+AT_CHECK([grep "ls_out_stateful" sw0flows | sort], [0], [dnl
+  table=7 (ls_out_stateful    ), priority=0    , match=(1), action=(next;)
+  table=7 (ls_out_stateful    ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;)
+  table=7 (ls_out_stateful    ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;)
+])
+
+# Add new ACL without label
+check ovn-nbctl --wait=sb acl-add sw0 to-lport 1002 udp allow-related
+check ovn-nbctl --wait=sb acl-add sw0 from-lport 1002 udp allow-related
+
+ovn-sbctl dump-flows sw0 > sw0flows
+AT_CAPTURE_FILE([sw0flows])
+
+AT_CHECK([grep -w "ls_in_acl" sw0flows | grep 2002 | sort], [0], [dnl
+  table=9 (ls_in_acl          ), priority=2002 , match=(reg0[[7]] == 1 && (tcp)), action=(reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 1234; next;)
+  table=9 (ls_in_acl          ), priority=2002 , match=(reg0[[7]] == 1 && (udp)), action=(reg0[[1]] = 1; next;)
+  table=9 (ls_in_acl          ), priority=2002 , match=(reg0[[8]] == 1 && (tcp)), action=(reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 1234; next;)
+  table=9 (ls_in_acl          ), priority=2002 , match=(reg0[[8]] == 1 && (udp)), action=(next;)
+])
+AT_CHECK([grep "ls_in_stateful" sw0flows | sort], [0], [dnl
+  table=12(ls_in_stateful     ), priority=0    , match=(1), action=(next;)
+  table=12(ls_in_stateful     ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;)
+  table=12(ls_in_stateful     ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;)
+])
+
+AT_CHECK([grep -w "ls_out_acl" sw0flows | grep 2002 | sort], [0], [dnl
+  table=4 (ls_out_acl         ), priority=2002 , match=(reg0[[7]] == 1 && (tcp)), action=(reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 1234; next;)
+  table=4 (ls_out_acl         ), priority=2002 , match=(reg0[[7]] == 1 && (udp)), action=(reg0[[1]] = 1; next;)
+  table=4 (ls_out_acl         ), priority=2002 , match=(reg0[[8]] == 1 && (tcp)), action=(reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 1234; next;)
+  table=4 (ls_out_acl         ), priority=2002 , match=(reg0[[8]] == 1 && (udp)), action=(next;)
+])
+AT_CHECK([grep "ls_out_stateful" sw0flows | sort], [0], [dnl
+  table=7 (ls_out_stateful    ), priority=0    , match=(1), action=(next;)
+  table=7 (ls_out_stateful    ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;)
+  table=7 (ls_out_stateful    ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;)
+])
+
+# Delete new ACL with label
+check ovn-nbctl --wait=sb acl-del sw0 to-lport 1002 tcp
+check ovn-nbctl --wait=sb acl-del sw0 from-lport 1002 tcp
+
+ovn-sbctl dump-flows sw0 > sw0flows
+AT_CAPTURE_FILE([sw0flows])
+
+AT_CHECK([grep -w "ls_in_acl" sw0flows | grep 2002 | sort], [0], [dnl
+  table=9 (ls_in_acl          ), priority=2002 , match=(reg0[[7]] == 1 && (udp)), action=(reg0[[1]] = 1; next;)
+  table=9 (ls_in_acl          ), priority=2002 , match=(reg0[[8]] == 1 && (udp)), action=(next;)
+])
+AT_CHECK([grep "ls_in_stateful" sw0flows | sort], [0], [dnl
+  table=12(ls_in_stateful     ), priority=0    , match=(1), action=(next;)
+  table=12(ls_in_stateful     ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;)
+  table=12(ls_in_stateful     ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;)
+])
+
+AT_CHECK([grep -w "ls_out_acl" sw0flows | grep 2002 | sort], [0], [dnl
+  table=4 (ls_out_acl         ), priority=2002 , match=(reg0[[7]] == 1 && (udp)), action=(reg0[[1]] = 1; next;)
+  table=4 (ls_out_acl         ), priority=2002 , match=(reg0[[8]] == 1 && (udp)), action=(next;)
+])
+AT_CHECK([grep "ls_out_stateful" sw0flows | sort], [0], [dnl
+  table=7 (ls_out_stateful    ), priority=0    , match=(1), action=(next;)
+  table=7 (ls_out_stateful    ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;)
+  table=7 (ls_out_stateful    ), priority=100  , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;)
+])
+AT_CLEANUP
+])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn -- ct.inv usage])
 ovn_start
 
 check ovn-nbctl ls-add sw0
diff --git a/tests/ovn.at b/tests/ovn.at
index a207f5e12..e51a94ad8 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -197,6 +197,7 @@ ct_label = NXM_NX_CT_LABEL
 ct_label.blocked = ct_label[0]
 ct_label.ecmp_reply_eth = ct_label[32..79]
 ct_label.ecmp_reply_port = ct_label[80..95]
+ct_label.label = ct_label[96..127]
 ct_label.natted = ct_label[1]
 ct_mark = NXM_NX_CT_MARK
 ct_state = NXM_NX_CT_STATE
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 4288d80e5..7d03bf3f0 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -6745,5 +6745,249 @@ OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
 as
 OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
 /.*terminating with signal 15.*/d"])
+
+AT_CLEANUP
+])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ACL label - conntrack ct_label])
+AT_KEYWORDS([acl label ct_commit])
+
+CHECK_CONNTRACK()
+ovn_start
+
+OVS_TRAFFIC_VSWITCHD_START()
+ADD_BR([br-int])
+
+# Set external-ids in br-int needed for ovn-controller
+ovs-vsctl \
+        -- set Open_vSwitch . external-ids:system-id=hv1 \
+        -- set Open_vSwitch . external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
+        -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
+        -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
+        -- set bridge br-int fail-mode=secure other-config:disable-in-band=true
+
+# Start ovn-controller
+start_daemon ovn-controller
+
+check ovn-nbctl ls-add sw0
+
+check ovn-nbctl lsp-add sw0 sw0-p1
+check ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:02 10.0.0.2"
+check ovn-nbctl lsp-set-port-security sw0-p1 "50:54:00:00:00:02 10.0.0.2"
+
+check ovn-nbctl lsp-add sw0 sw0-p2
+check ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:03 10.0.0.3"
+check ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:03 10.0.0.3"
+
+check ovn-nbctl lsp-add sw0 sw0-p3
+check ovn-nbctl lsp-set-addresses sw0-p3 "50:54:00:00:00:04 10.0.0.4"
+check ovn-nbctl lsp-set-port-security sw0-p3 "50:54:00:00:00:04 10.0.0.4"
+
+# ACLs
+# Case 1: sw0-p1 ---> sw0-p3 allowed, label=1234
+# Case 2: sw0-p3 ---> sw0-p1 allowed, label=1235
+# Case 3: sw0-p1 ---> sw0-p2 allowed, no label
+# Case 4: sw0-p2 ---> sw0-p1 allowed, no label
+
+check ovn-nbctl --label=1234 acl-add sw0 from-lport 1002 'ip4 && inport == "sw0-p1" && ip4.dst == 10.0.0.4' allow-related
+check ovn-nbctl --label=1235 acl-add sw0 to-lport 1002 'ip4 && outport == "sw0-p1" && ip4.src == 10.0.0.4' allow-related
+check ovn-nbctl acl-add sw0 from-lport 1001 "ip" allow-related
+check ovn-nbctl acl-add sw0 to-lport 1001 "ip" allow-related
+
+
+ADD_NAMESPACES(sw0-p1)
+ADD_VETH(sw0-p1, sw0-p1, br-int, "10.0.0.2/24", "50:54:00:00:00:02", \
+         "10.0.0.1")
+ADD_NAMESPACES(sw0-p2)
+ADD_VETH(sw0-p2, sw0-p2, br-int, "10.0.0.3/24", "50:54:00:00:00:03", \
+         "10.0.0.1")
+ADD_NAMESPACES(sw0-p3)
+ADD_VETH(sw0-p3, sw0-p3, br-int, "10.0.0.4/24", "50:54:00:00:00:04", \
+         "10.0.0.1")
+
+# Ensure ovn-controller is caught up
+ovn-nbctl --wait=hv sync
+
+on_exit 'ovn-nbctl acl-list sw0'
+on_exit 'ovn-sbctl lflow-list'
+on_exit 'ovs-ofctl dump-flows br-int'
+
+wait_for_ports_up
+
+AT_CHECK([ovs-appctl dpctl/flush-conntrack])
+# 'sw0-p1' should be able to ping 'sw0-p3'.
+NS_CHECK_EXEC([sw0-p1], [ping -q -c 10 -i 0.3 -w 15 10.0.0.4 | FORMAT_PING], \
+[0], [dnl
+10 packets transmitted, 10 received, 0% packet loss, time 0ms
+])
+
+# Ensure conntrack entry is present and ct_label is set.
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.0.0.4) | \
+sed -e 's/zone=[[0-9]]*/zone=<cleared>/' | \
+sed -e 's/labels=0x4d2[[0-9a-f]]*/labels=0x4d2000000000000000000000000/'], [0], [dnl
+icmp,orig=(src=10.0.0.2,dst=10.0.0.4,id=<cleared>,type=8,code=0),reply=(src=10.0.0.4,dst=10.0.0.2,id=<cleared>,type=0,code=0),zone=<cleared>,labels=0x4d2000000000000000000000000
+icmp,orig=(src=10.0.0.2,dst=10.0.0.4,id=<cleared>,type=8,code=0),reply=(src=10.0.0.4,dst=10.0.0.2,id=<cleared>,type=0,code=0),zone=<cleared>
+])
+
+AT_CHECK([ovs-appctl dpctl/flush-conntrack])
+# 'sw0-p3' should be able to ping 'sw0-p1'.
+NS_CHECK_EXEC([sw0-p3], [ping -q -c 10 -i 0.3 -w 15 10.0.0.2 | FORMAT_PING], \
+[0], [dnl
+10 packets transmitted, 10 received, 0% packet loss, time 0ms
+])
+
+# Ensure conntrack entry is present and ct_label is set.
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.0.0.2) | \
+sed -e 's/zone=[[0-9]]*/zone=<cleared>/' | \
+sed -e 's/labels=0x4d3[[0-9a-f]]*/labels=0x4d3000000000000000000000000/'], [0], [dnl
+icmp,orig=(src=10.0.0.4,dst=10.0.0.2,id=<cleared>,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.4,id=<cleared>,type=0,code=0),zone=<cleared>,labels=0x4d3000000000000000000000000
+icmp,orig=(src=10.0.0.4,dst=10.0.0.2,id=<cleared>,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.4,id=<cleared>,type=0,code=0),zone=<cleared>
+])
+
+AT_CHECK([ovs-appctl dpctl/flush-conntrack])
+# 'sw0-p1' should be able to ping 'sw0-p2'.
+NS_CHECK_EXEC([sw0-p1], [ping -q -c 10 -i 0.3 -w 15 10.0.0.3 | FORMAT_PING], \
+[0], [dnl
+10 packets transmitted, 10 received, 0% packet loss, time 0ms
+])
+
+# Ensure conntrack entry is present and ct_label is not set.
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.0.0.3) | \
+sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
+icmp,orig=(src=10.0.0.2,dst=10.0.0.3,id=<cleared>,type=8,code=0),reply=(src=10.0.0.3,dst=10.0.0.2,id=<cleared>,type=0,code=0),zone=<cleared>
+icmp,orig=(src=10.0.0.2,dst=10.0.0.3,id=<cleared>,type=8,code=0),reply=(src=10.0.0.3,dst=10.0.0.2,id=<cleared>,type=0,code=0),zone=<cleared>
+])
+
+AT_CHECK([ovs-appctl dpctl/flush-conntrack])
+# 'sw0-p2' should be able to ping 'sw0-p1'.
+NS_CHECK_EXEC([sw0-p2], [ping -q -c 10 -i 0.3 -w 15 10.0.0.2 | FORMAT_PING], \
+[0], [dnl
+10 packets transmitted, 10 received, 0% packet loss, time 0ms
+])
+
+# Ensure conntrack entry is present and ct_label is not set.
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.0.0.2) | \
+sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
+icmp,orig=(src=10.0.0.3,dst=10.0.0.2,id=<cleared>,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.3,id=<cleared>,type=0,code=0),zone=<cleared>
+icmp,orig=(src=10.0.0.3,dst=10.0.0.2,id=<cleared>,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.3,id=<cleared>,type=0,code=0),zone=<cleared>
+])
+
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+
+AT_CLEANUP
+])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ACL label - conntrack label change])
+AT_KEYWORDS([acl label ct_commit label change])
+
+CHECK_CONNTRACK()
+ovn_start
+
+OVS_TRAFFIC_VSWITCHD_START()
+ADD_BR([br-int])
+
+# Set external-ids in br-int needed for ovn-controller
+ovs-vsctl \
+        -- set Open_vSwitch . external-ids:system-id=hv1 \
+        -- set Open_vSwitch . external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
+        -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
+        -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
+        -- set bridge br-int fail-mode=secure other-config:disable-in-band=true
+
+# Start ovn-controller
+start_daemon ovn-controller
+
+check ovn-nbctl ls-add sw0
+
+check ovn-nbctl lsp-add sw0 sw0-p1
+check ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:02 10.0.0.2"
+check ovn-nbctl lsp-set-port-security sw0-p1 "50:54:00:00:00:02 10.0.0.2"
+
+check ovn-nbctl lsp-add sw0 sw0-p2
+check ovn-nbctl lsp-set-addresses sw0-p2 "50:54:00:00:00:03 10.0.0.3"
+check ovn-nbctl lsp-set-port-security sw0-p2 "50:54:00:00:00:03 10.0.0.3"
+
+# ACLs
+# sw0-p1 ---> sw0-p2 allowed, label=1234
+
+check ovn-nbctl --label=1234 acl-add sw0 from-lport 1002 'ip4 && inport == "sw0-p1" && ip4.dst == 10.0.0.3' allow-related
+
+ADD_NAMESPACES(sw0-p1)
+ADD_VETH(sw0-p1, sw0-p1, br-int, "10.0.0.2/24", "50:54:00:00:00:02", \
+         "10.0.0.1")
+ADD_NAMESPACES(sw0-p2)
+ADD_VETH(sw0-p2, sw0-p2, br-int, "10.0.0.3/24", "50:54:00:00:00:03", \
+         "10.0.0.1")
+
+# Ensure ovn-controller is caught up
+ovn-nbctl --wait=hv sync
+
+on_exit 'ovn-nbctl acl-list sw0'
+on_exit 'ovn-sbctl lflow-list'
+on_exit 'ovs-ofctl dump-flows br-int'
+
+wait_for_ports_up
+
+AT_CHECK([ovs-appctl dpctl/flush-conntrack])
+
+# start a background ping for ~30 secs.
+NETNS_DAEMONIZE([sw0-p1], [[ping -q -c 100 -i 0.3 -w 15 10.0.0.3]], [ns-sw0-p1.pid])
+
+sleep 3s
+
+# Ensure conntrack entry is present and ct_label is set.
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.0.0.3) | \
+sed -e 's/zone=[[0-9]]*/zone=<cleared>/' | \
+sed -e 's/labels=0x4d2[[0-9a-f]]*/labels=0x4d2000000000000000000000000/'], [0], [dnl
+icmp,orig=(src=10.0.0.2,dst=10.0.0.3,id=<cleared>,type=8,code=0),reply=(src=10.0.0.3,dst=10.0.0.2,id=<cleared>,type=0,code=0),zone=<cleared>,labels=0x4d2000000000000000000000000
+icmp,orig=(src=10.0.0.2,dst=10.0.0.3,id=<cleared>,type=8,code=0),reply=(src=10.0.0.3,dst=10.0.0.2,id=<cleared>,type=0,code=0),zone=<cleared>
+])
+
+# Add a higher priority ACL with different label.
+# This ACL also allows the ping running in background.
+
+check ovn-nbctl --label=1235 acl-add sw0 from-lport 1003 'ip4 && inport == "sw0-p1" && ip4.dst == 10.0.0.3' allow-related
+ovn-nbctl --wait=hv sync
+
+sleep 3s
+
+# Ensure conntrack entry is updated with new ct_label is set.
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.0.0.3) | \
+sed -e 's/zone=[[0-9]]*/zone=<cleared>/' | \
+sed -e 's/labels=0x4d3[[0-9a-f]]*/labels=0x4d3000000000000000000000000/'], [0], [dnl
+icmp,orig=(src=10.0.0.2,dst=10.0.0.3,id=<cleared>,type=8,code=0),reply=(src=10.0.0.3,dst=10.0.0.2,id=<cleared>,type=0,code=0),zone=<cleared>,labels=0x4d3000000000000000000000000
+icmp,orig=(src=10.0.0.2,dst=10.0.0.3,id=<cleared>,type=8,code=0),reply=(src=10.0.0.3,dst=10.0.0.2,id=<cleared>,type=0,code=0),zone=<cleared>
+])
+
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+
 AT_CLEANUP
 ])
diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
index 987797860..80a564660 100644
--- a/utilities/ovn-nbctl.8.xml
+++ b/utilities/ovn-nbctl.8.xml
@@ -399,7 +399,7 @@
       must be either <code>switch</code> or <code>port-group</code>.
     </p>
     <dl>
-      <dt>[<code>--type=</code>{<code>switch</code> | <code>port-group</code>}] [<code>--log</code>] [<code>--meter=</code><var>meter</var>] [<code>--severity=</code><var>severity</var>] [<code>--name=</code><var>name</var>] [<code>--may-exist</code>] <code>acl-add</code> <var>entity</var> <var>direction</var> <var>priority</var> <var>match</var> <var>verdict</var></dt>
+      <dt>[<code>--type=</code>{<code>switch</code> | <code>port-group</code>}] [<code>--log</code>] [<code>--meter=</code><var>meter</var>] [<code>--severity=</code><var>severity</var>] [<code>--name=</code><var>name</var>] [<code>--label=</code><var>label</var>] [<code>--may-exist</code>] <code>acl-add</code> <var>entity</var> <var>direction</var> <var>priority</var> <var>match</var> <var>verdict</var></dt>
       <dd>
         <p>
           Adds the specified ACL to <var>entity</var>.  <var>direction</var>
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index f41238990..ada53b662 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -2011,6 +2011,9 @@ nbctl_acl_list(struct ctl_context *ctx)
             ds_chomp(&ctx->output, ',');
             ds_put_cstr(&ctx->output, ")");
         }
+        if (acl->label) {
+          ds_put_format(&ctx->output, " label=%"PRId64, acl->label);
+        }
         ds_put_cstr(&ctx->output, "\n");
     }
 
@@ -2069,6 +2072,19 @@ parse_priority(const char *arg, int64_t *priority_p)
     return NULL;
 }
 
+static char * OVS_WARN_UNUSED_RESULT
+parse_acl_label(const char *arg, int64_t *label_p)
+{
+    /* Validate label. */
+    int64_t label;
+    if (!ovs_scan(arg, "%"SCNd64, &label)
+        || label < 0 || label > UINT32_MAX) {
+        return xasprintf("%s: label must in range 0...4294967295", arg);
+    }
+    *label_p = label;
+    return NULL;
+}
+
 static void
 nbctl_pre_acl(struct ctl_context *ctx)
 {
@@ -2093,6 +2109,7 @@ nbctl_pre_acl_list(struct ctl_context *ctx)
     ovsdb_idl_add_column(ctx->idl, &nbrec_acl_col_name);
     ovsdb_idl_add_column(ctx->idl, &nbrec_acl_col_severity);
     ovsdb_idl_add_column(ctx->idl, &nbrec_acl_col_meter);
+    ovsdb_idl_add_column(ctx->idl, &nbrec_acl_col_label);
 }
 
 static void
@@ -2160,6 +2177,25 @@ nbctl_acl_add(struct ctl_context *ctx)
         nbrec_acl_set_meter(acl, meter);
     }
 
+    /* Set the ACL label */
+    const char *label = shash_find_data(&ctx->options, "--label");
+    if (label) {
+      /* Ensure that the action is either allow or allow-related */
+      if (strcmp(action, "allow") && strcmp(action, "allow-related")) {
+        ctl_error(ctx, "label can only be set with actions \"allow\" or "
+                  "\"allow-related\"");
+        return;
+      }
+
+      int64_t label_value = 0;
+      error = parse_acl_label(label, &label_value);
+      if (error) {
+        ctx->error = error;
+        return;
+      }
+      nbrec_acl_set_label(acl, label_value);
+    }
+
     /* Check if same acl already exists for the ls/portgroup */
     size_t n_acls = pg ? pg->n_acls : ls->n_acls;
     struct nbrec_acl **acls = pg ? pg->acls : ls->acls;
@@ -6757,7 +6793,7 @@ static const struct ctl_command_syntax nbctl_commands[] = {
     /* acl commands. */
     { "acl-add", 5, 6, "{SWITCH | PORTGROUP} DIRECTION PRIORITY MATCH ACTION",
       nbctl_pre_acl, nbctl_acl_add, NULL,
-      "--log,--may-exist,--type=,--name=,--severity=,--meter=", RW },
+      "--log,--may-exist,--type=,--name=,--severity=,--meter=,--label=", RW },
     { "acl-del", 1, 4, "{SWITCH | PORTGROUP} [DIRECTION [PRIORITY MATCH]]",
       nbctl_pre_acl, nbctl_acl_del, NULL, "--type=", RW },
     { "acl-list", 1, 1, "{SWITCH | PORTGROUP}",
-- 
2.30.1 (Apple Git-130)



More information about the dev mailing list