[ovs-dev] [PATCH v2] ovn: Add second ACL stage

Mickey Spiegel mickeys.dev at gmail.com
Tue Aug 2 17:08:51 UTC 2016


This patch adds a second logical switch ingress ACL stage, and
correspondingly a second logical switch egress ACL stage.  This
allows for more than one ACL-based feature to be applied in the
ingress and egress logical switch pipelines.  The features
driving the different ACL stages may be configured by different
users, for example an application deployer managing security
groups and a network or security admin configuring network ACLs
or firewall rules.

Each ACL stage is self contained.  The "action" for the
highest-"priority" matching row in an ACL stage determines a
packet's treatment.  A separate "action" will be determined in
each ACL stage, according to the ACL rules configured for that
ACL stage.  The "priority" values are only relevant within the
context of an ACL stage.

The accepted values for "stage" are "acl" and "acl2".  ACL
rules that do not specify an ACL stage are applied to the
default "acl" stage.

Signed-off-by: Mickey Spiegel <mickeys.dev at gmail.com>
---
 ovn/northd/ovn-northd.c   | 319 +++++++++++++++++++++++++++-------------------
 ovn/ovn-nb.ovsschema      |   7 +-
 ovn/ovn-nb.xml            |  26 ++++
 ovn/utilities/ovn-nbctl.c |  35 +++--
 tests/ovn-nbctl.at        |  30 +++--
 5 files changed, 265 insertions(+), 152 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index d6c14cf..9d5a248 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -104,12 +104,13 @@ enum ovn_stage {
     PIPELINE_STAGE(SWITCH, IN,  PRE_LB,         4, "ls_in_pre_lb")         \
     PIPELINE_STAGE(SWITCH, IN,  PRE_STATEFUL,   5, "ls_in_pre_stateful")    \
     PIPELINE_STAGE(SWITCH, IN,  ACL,            6, "ls_in_acl")          \
-    PIPELINE_STAGE(SWITCH, IN,  LB,             7, "ls_in_lb")           \
-    PIPELINE_STAGE(SWITCH, IN,  STATEFUL,       8, "ls_in_stateful")     \
-    PIPELINE_STAGE(SWITCH, IN,  ARP_ND_RSP,     9, "ls_in_arp_rsp")      \
-    PIPELINE_STAGE(SWITCH, IN,  DHCP_OPTIONS,   10, "ls_in_dhcp_options") \
-    PIPELINE_STAGE(SWITCH, IN,  DHCP_RESPONSE,  11, "ls_in_dhcp_response") \
-    PIPELINE_STAGE(SWITCH, IN,  L2_LKUP,        12, "ls_in_l2_lkup")      \
+    PIPELINE_STAGE(SWITCH, IN,  ACL2,           7, "ls_in_acl2")          \
+    PIPELINE_STAGE(SWITCH, IN,  LB,             8, "ls_in_lb")           \
+    PIPELINE_STAGE(SWITCH, IN,  STATEFUL,       9, "ls_in_stateful")     \
+    PIPELINE_STAGE(SWITCH, IN,  ARP_ND_RSP,     10, "ls_in_arp_rsp")      \
+    PIPELINE_STAGE(SWITCH, IN,  DHCP_OPTIONS,   11, "ls_in_dhcp_options") \
+    PIPELINE_STAGE(SWITCH, IN,  DHCP_RESPONSE,  12, "ls_in_dhcp_response") \
+    PIPELINE_STAGE(SWITCH, IN,  L2_LKUP,        13, "ls_in_l2_lkup")      \
                                                                       \
     /* Logical switch egress stages. */                               \
     PIPELINE_STAGE(SWITCH, OUT, PRE_LB,       0, "ls_out_pre_lb")     \
@@ -117,9 +118,10 @@ enum ovn_stage {
     PIPELINE_STAGE(SWITCH, OUT, PRE_STATEFUL, 2, "ls_out_pre_stateful")  \
     PIPELINE_STAGE(SWITCH, OUT, LB,           3, "ls_out_lb")            \
     PIPELINE_STAGE(SWITCH, OUT, ACL,          4, "ls_out_acl")            \
-    PIPELINE_STAGE(SWITCH, OUT, STATEFUL,     5, "ls_out_stateful")       \
-    PIPELINE_STAGE(SWITCH, OUT, PORT_SEC_IP,  6, "ls_out_port_sec_ip")    \
-    PIPELINE_STAGE(SWITCH, OUT, PORT_SEC_L2,  7, "ls_out_port_sec_l2")    \
+    PIPELINE_STAGE(SWITCH, OUT, ACL2,         5, "ls_out_acl2")            \
+    PIPELINE_STAGE(SWITCH, OUT, STATEFUL,     6, "ls_out_stateful")       \
+    PIPELINE_STAGE(SWITCH, OUT, PORT_SEC_IP,  7, "ls_out_port_sec_ip")    \
+    PIPELINE_STAGE(SWITCH, OUT, PORT_SEC_L2,  8, "ls_out_port_sec_l2")    \
                                                                       \
     /* Logical router ingress stages. */                              \
     PIPELINE_STAGE(ROUTER, IN,  ADMISSION,   0, "lr_in_admission")    \
@@ -200,6 +202,48 @@ ovn_stage_to_datapath_type(enum ovn_stage stage)
     default: OVS_NOT_REACHED();
     }
 }
+
+static enum ovn_stage
+ovn_stage_from_acl_stage(const char *acl_stage, enum ovn_pipeline pipeline) {
+    if ((pipeline == P_IN) && !acl_stage) {
+        return S_SWITCH_IN_ACL;
+    } else if ((pipeline == P_OUT) && !acl_stage) {
+        return S_SWITCH_OUT_ACL;
+    } else if ((pipeline == P_IN) && !strcmp(acl_stage, "acl")) {
+        return S_SWITCH_IN_ACL;
+    } else if ((pipeline == P_OUT) && !strcmp(acl_stage, "acl")) {
+        return S_SWITCH_OUT_ACL;
+    } else if ((pipeline == P_IN) && !strcmp(acl_stage, "acl2")) {
+        return S_SWITCH_IN_ACL2;
+    } else if ((pipeline == P_OUT) && !strcmp(acl_stage, "acl2")) {
+        return S_SWITCH_OUT_ACL2;
+    } else if (pipeline == P_IN) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+        VLOG_WARN_RL(&rl, "Bad configuration: invalid ACL stage %s",
+                     acl_stage);
+        return S_SWITCH_IN_ACL;
+    } else {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+        VLOG_WARN_RL(&rl, "Bad configuration: invalid ACL stage %s",
+                     acl_stage);
+        return S_SWITCH_OUT_ACL;
+    }
+}
+
+enum acl_stage_enum {
+    acl,
+    acl2
+};
+
+static const char *
+acl_stage_str_from_enum(enum acl_stage_enum stage)
+{
+    switch (stage) {
+        case acl: return "acl";
+        case acl2: return "acl2";
+        default: return "<unknown>";
+    }
+}
 
 static void
 usage(void)
@@ -2010,105 +2054,149 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)
 {
     bool has_stateful = has_stateful_acl(od);
 
-    /* Ingress and Egress ACL Table (Priority 0): Packets are allowed by
-     * default.  A related rule at priority 1 is added below if there
-     * are any stateful ACLs in this datapath. */
-    ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, 0, "1", "next;");
-    ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, 0, "1", "next;");
+    for (int i = acl; i <= acl2; i++) {
+        const char *acl_stage = acl_stage_str_from_enum(i);
+        enum ovn_stage in_stage = ovn_stage_from_acl_stage(acl_stage, P_IN);
+        enum ovn_stage out_stage = ovn_stage_from_acl_stage(acl_stage, P_OUT);
 
-    if (has_stateful) {
-        /* Ingress and Egress ACL Table (Priority 1).
-         *
-         * By default, traffic is allowed.  This is partially handled by
-         * the Priority 0 ACL flows added earlier, but we also need to
-         * commit IP flows.  This is because, while the initiater's
-         * direction may not have any stateful rules, the server's may
-         * and then its return traffic would not have an associated
-         * conntrack entry and would return "+invalid".
-         *
-         * We use "ct_commit" for a connection that is not already known
-         * by the connection tracker.  Once a connection is committed,
-         * subsequent packets will hit the flow at priority 0 that just
-         * uses "next;"
-         *
-         * We also check for established connections that have ct_label[0]
-         * 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,
-         * 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))",
-                       REGBIT_CONNTRACK_COMMIT" = 1; next;");
-        ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, 1,
-                      "ip && (!ct.est || (ct.est && ct_label[0] == 1))",
-                       REGBIT_CONNTRACK_COMMIT" = 1; next;");
-
-        /* Ingress and Egress ACL Table (Priority 65535).
-         *
-         * Always drop traffic that's in an invalid state.  Also drop
-         * reply direction packets for connections that have been marked
-         * for deletion (bit 0 of ct_label is set).
-         *
-         * 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)",
-                      "drop;");
-        ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX,
-                      "ct.inv || (ct.est && ct.rpl && ct_label[0] == 1)",
-                      "drop;");
+        /* Ingress and Egress ACL Table (Priority 0): Packets are allowed by
+         * default.  A related rule at priority 1 is added below if there
+         * are any stateful ACLs in this datapath. */
+        ovn_lflow_add(lflows, od, in_stage, 0, "1", "next;");
+        ovn_lflow_add(lflows, od, out_stage, 0, "1", "next;");
 
-        /* Ingress and Egress ACL Table (Priority 65535).
-         *
-         * Allow reply traffic that is part of an established
-         * conntrack entry that has not been marked for deletion
-         * (bit 0 of ct_label).  We only match traffic in the
-         * reply direction because we want traffic in the request
-         * direction to hit the currently defined policy from ACLs.
-         *
-         * 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",
-                      "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",
-                      "next;");
-
-        /* Ingress and Egress ACL Table (Priority 65535).
-         *
-         * Allow traffic that is related to an existing conntrack entry that
-         * has not been marked for deletion (bit 0 of ct_label).
-         *
-         * This is enforced at a higher priority than ACLs can be defined.
-         *
-         * NOTE: This does not support related data sessions (eg,
-         * a dynamically negotiated FTP data channel), but will allow
-         * related traffic such as an ICMP Port Unreachable through
-         * 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",
-                      "next;");
-        ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX,
-                      "!ct.est && ct.rel && !ct.new && !ct.inv "
-                      "&& ct_label[0] == 0",
-                      "next;");
-
-        /* Ingress and Egress ACL Table (Priority 65535).
-         *
-         * Not to do conntrack on ND packets. */
-        ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX, "nd", "next;");
-        ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX, "nd", "next;");
+        if (has_stateful) {
+            /* Ingress and Egress ACL Table (Priority 1).
+             *
+             * By default, traffic is allowed.  This is partially handled by
+             * the Priority 0 ACL flows added earlier, but we also need to
+             * commit IP flows.  This is because, while the initiater's
+             * direction may not have any stateful rules, the server's may
+             * and then its return traffic would not have an associated
+             * conntrack entry and would return "+invalid".
+             *
+             * We use "ct_commit" for a connection that is not already known
+             * by the connection tracker.  Once a connection is committed,
+             * subsequent packets will hit the flow at priority 0 that just
+             * uses "next;"
+             *
+             * We also check for established connections that have ct_label[0]
+             * 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, 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, in_stage, 1,
+                          "ip && (!ct.est || (ct.est && ct_label[0] == 1))",
+                           REGBIT_CONNTRACK_COMMIT" = 1; next;");
+            ovn_lflow_add(lflows, od, out_stage, 1,
+                          "ip && (!ct.est || (ct.est && ct_label[0] == 1))",
+                           REGBIT_CONNTRACK_COMMIT" = 1; next;");
+
+            /* Ingress and Egress ACL Table (Priority 65535).
+             *
+             * Always drop traffic that's in an invalid state.  Also drop
+             * reply direction packets for connections that have been marked
+             * for deletion (bit 0 of ct_label is set).
+             *
+             * This is enforced at a higher priority than ACLs can be
+             * defined. */
+            ovn_lflow_add(lflows, od, in_stage, UINT16_MAX,
+                          "ct.inv || (ct.est && ct.rpl && ct_label[0] == 1)",
+                          "drop;");
+            ovn_lflow_add(lflows, od, out_stage, UINT16_MAX,
+                          "ct.inv || (ct.est && ct.rpl && ct_label[0] == 1)",
+                          "drop;");
+
+            /* Ingress and Egress ACL Table (Priority 65535).
+             *
+             * Allow reply traffic that is part of an established
+             * conntrack entry that has not been marked for deletion
+             * (bit 0 of ct_label).  We only match traffic in the
+             * reply direction because we want traffic in the request
+             * direction to hit the currently defined policy from ACLs.
+             *
+             * This is enforced at a higher priority than ACLs can be
+             * defined. */
+            ovn_lflow_add(lflows, od, in_stage, UINT16_MAX,
+                          "ct.est && !ct.rel && !ct.new && !ct.inv "
+                          "&& ct.rpl && ct_label[0] == 0",
+                          "next;");
+            ovn_lflow_add(lflows, od, out_stage, UINT16_MAX,
+                          "ct.est && !ct.rel && !ct.new && !ct.inv "
+                          "&& ct.rpl && ct_label[0] == 0",
+                          "next;");
+
+            /* Ingress and Egress ACL Table (Priority 65535).
+             *
+             * Allow traffic that is related to an existing conntrack entry
+             * that has not been marked for deletion (bit 0 of ct_label).
+             *
+             * This is enforced at a higher priority than ACLs can be defined.
+             *
+             * NOTE: This does not support related data sessions (eg,
+             * a dynamically negotiated FTP data channel), but will allow
+             * related traffic such as an ICMP Port Unreachable through
+             * that's generated from a non-listening UDP port.  */
+            ovn_lflow_add(lflows, od, in_stage, UINT16_MAX,
+                          "!ct.est && ct.rel && !ct.new && !ct.inv "
+                          "&& ct_label[0] == 0",
+                          "next;");
+            ovn_lflow_add(lflows, od, out_stage, UINT16_MAX,
+                          "!ct.est && ct.rel && !ct.new && !ct.inv "
+                          "&& ct_label[0] == 0",
+                          "next;");
+
+            /* Ingress and Egress ACL Table (Priority 65535).
+             *
+             * Not to do conntrack on ND packets. */
+            ovn_lflow_add(lflows, od, in_stage, UINT16_MAX, "nd", "next;");
+            ovn_lflow_add(lflows, od, out_stage, UINT16_MAX, "nd", "next;");
+        }
+
+        /* Add 34000 priority flow to allow DHCP reply from ovn-controller to
+         * all logical ports of the datapath if the CMS has configured DHCPv4
+         * options*/
+        if (od->nbs && od->nbs->n_ports) {
+            for (size_t i = 0; i < od->nbs->n_ports; i++) {
+                if (od->nbs->ports[i]->dhcpv4_options) {
+                    const char *server_id = smap_get(
+                        &od->nbs->ports[i]->dhcpv4_options->options,
+                        "server_id");
+                    const char *server_mac = smap_get(
+                        &od->nbs->ports[i]->dhcpv4_options->options,
+                        "server_mac");
+                    const char *lease_time = smap_get(
+                        &od->nbs->ports[i]->dhcpv4_options->options,
+                        "lease_time");
+                    const char *router = smap_get(
+                        &od->nbs->ports[i]->dhcpv4_options->options, "router");
+                    if (server_id && server_mac && lease_time && router) {
+                        struct ds match = DS_EMPTY_INITIALIZER;
+                        const char *actions =
+                            has_stateful ? "ct_commit; next;" : "next;";
+                        ds_put_format(&match,
+                                      "outport == \"%s\" && eth.src == %s "
+                                      "&& ip4.src == %s && udp "
+                                      "&& udp.src == 67 && udp.dst == 68",
+                                      od->nbs->ports[i]->name,
+                                      server_mac, server_id);
+                        ovn_lflow_add(
+                            lflows, od, out_stage, 34000, ds_cstr(&match),
+                            actions);
+                    }
+                }
+            }
+        }
     }
 
     /* Ingress or Egress ACL Table (Various priorities). */
     for (size_t i = 0; i < od->nbs->n_acls; i++) {
         struct nbrec_acl *acl = od->nbs->acls[i];
-        bool ingress = !strcmp(acl->direction, "from-lport") ? true :false;
-        enum ovn_stage stage = ingress ? S_SWITCH_IN_ACL : S_SWITCH_OUT_ACL;
+        enum ovn_pipeline pipeline
+            = !strcmp(acl->direction, "from-lport") ? P_IN : P_OUT;
+        enum ovn_stage stage = ovn_stage_from_acl_stage(acl->stage, pipeline);
 
         if (!strcmp(acl->action, "allow")
             || !strcmp(acl->action, "allow-related")) {
@@ -2214,35 +2302,6 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)
             }
         }
     }
-
-    /* Add 34000 priority flow to allow DHCP reply from ovn-controller to all
-     * logical ports of the datapath if the CMS has configured DHCPv4 options*/
-    if (od->nbs && od->nbs->n_ports) {
-        for (size_t i = 0; i < od->nbs->n_ports; i++) {
-            if (od->nbs->ports[i]->dhcpv4_options) {
-                const char *server_id = smap_get(
-                    &od->nbs->ports[i]->dhcpv4_options->options, "server_id");
-                const char *server_mac = smap_get(
-                    &od->nbs->ports[i]->dhcpv4_options->options, "server_mac");
-                const char *lease_time = smap_get(
-                    &od->nbs->ports[i]->dhcpv4_options->options, "lease_time");
-                const char *router = smap_get(
-                    &od->nbs->ports[i]->dhcpv4_options->options, "router");
-                if (server_id && server_mac && lease_time && router) {
-                    struct ds match = DS_EMPTY_INITIALIZER;
-                    const char *actions =
-                        has_stateful ? "ct_commit; next;" : "next;";
-                    ds_put_format(&match, "outport == \"%s\" && eth.src == %s "
-                                  "&& ip4.src == %s && udp && udp.src == 67 "
-                                  "&& udp.dst == 68", od->nbs->ports[i]->name,
-                                  server_mac, server_id);
-                    ovn_lflow_add(
-                        lflows, od, S_SWITCH_OUT_ACL, 34000, ds_cstr(&match),
-                        actions);
-                }
-            }
-        }
-    }
 }
 
 static void
diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
index 660db76..a901910 100644
--- a/ovn/ovn-nb.ovsschema
+++ b/ovn/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
 {
     "name": "OVN_Northbound",
-    "version": "5.3.0",
-    "cksum": "1305504870 9051",
+    "version": "5.4.0",
+    "cksum": "2152914871 9247",
     "tables": {
         "NB_Global": {
             "columns": {
@@ -108,6 +108,9 @@
                 "match": {"type": "string"},
                 "action": {"type": {"key": {"type": "string",
                                             "enum": ["set", ["allow", "allow-related", "drop", "reject"]]}}},
+                "stage": {"type": {"key": {"type": "string",
+                                           "enum": ["set", ["acl", "acl2"]]},
+                                   "min": 0, "max": 1}},
                 "log": {"type": "boolean"},
                 "external_ids": {
                     "type": {"key": "string", "value": "string",
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 4ce295a..0eef8f2 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -772,6 +772,32 @@
       </ul>
     </column>
 
+    <column name="stage">
+      <p>
+        OVN supports the use of two ACL stages.  This allows for more than one
+        ACL-based feature to be applied in the ingress and egress logical
+        switch pipelines.  The features driving the different ACL stages may be
+        configured by different users, for example an application deployer
+        managing security groups and a network or security admin configuring
+        network ACLs or firewall rules.
+      </p>
+
+      <p>
+        Each ACL stage is self contained.  The <ref column="action"/> column
+        for the highest-<ref column="priority"/> matching row in an ACL stage
+        determines a packet's treatment.  A separate <ref column="action"/>
+        will be determined in each ACL stage, according to the ACL rules
+        configured for that ACL stage.  The <ref column="priority"/> values
+        are only relevant within the context of an ACL stage.
+      </p>
+
+      <p>
+        The accepted values for <ref column="stage"/> are <code>acl</code> and
+        and <code>acl2</code>.  ACL rules that do not specify an ACL stage are
+        applied to the default <code>acl</code> stage.
+      </p>
+    </column>
+
     <column name="log">
       <p>
         If set to <code>true</code>, packets that match the ACL will trigger a
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 3d74779..e6affce 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -334,9 +334,9 @@ Logical switch commands:\n\
   ls-list                   print the names of all logical switches\n\
 \n\
 ACL commands:\n\
-  acl-add SWITCH DIRECTION PRIORITY MATCH ACTION [log]\n\
+  acl-add SWITCH DIRECTION PRIORITY MATCH ACTION [STAGE] [log]\n\
                             add an ACL to SWITCH\n\
-  acl-del SWITCH [DIRECTION [PRIORITY MATCH]]\n\
+  acl-del SWITCH [DIRECTION [PRIORITY MATCH [STAGE]]]\n\
                             remove ACLs from SWITCH\n\
   acl-list SWITCH           print ACLs for SWITCH\n\
 \n\
@@ -1150,7 +1150,14 @@ acl_cmp(const void *acl1_, const void *acl2_)
     int dir1 = dir_encode(acl1->direction);
     int dir2 = dir_encode(acl2->direction);
 
-    if (dir1 != dir2) {
+    /* Normalize stage==NULL and stage=="acl". */
+    const char *stage1 = (acl1->stage) ? acl1->stage : "acl";
+    const char *stage2 = (acl2->stage) ? acl2->stage : "acl";
+
+    const int stage_cmp = strcmp(stage1, stage2);
+    if (stage_cmp) {
+        return stage_cmp;
+    } else if (dir1 != dir2) {
         return dir1 < dir2 ? -1 : 1;
     } else if (acl1->priority != acl2->priority) {
         return acl1->priority > acl2->priority ? -1 : 1;
@@ -1177,7 +1184,8 @@ nbctl_acl_list(struct ctl_context *ctx)
 
     for (i = 0; i < ls->n_acls; i++) {
         const struct nbrec_acl *acl = acls[i];
-        ds_put_format(&ctx->output, "%10s %5"PRId64" (%s) %s%s\n",
+        ds_put_format(&ctx->output, "%-4.4s %10s %5"PRId64" (%s) %s%s\n",
+                      acl->stage ? acl->stage : "",
                       acl->direction, acl->priority,
                       acl->match, acl->action, acl->log ? " log" : "");
     }
@@ -1229,12 +1237,22 @@ nbctl_acl_add(struct ctl_context *ctx)
         return;
     }
 
+    /* Validate stage. */
+    const char *stage = ctx->argc == 7 ? ctx->argv[6] : NULL;
+    if (stage && strcmp(stage, "acl") && strcmp(stage, "acl2")) {
+        ctl_fatal("%s: stage must be one of \"acl\" and \"acl2\"", stage);
+        return;
+    }
+
     /* Create the acl. */
     struct nbrec_acl *acl = nbrec_acl_insert(ctx->txn);
     nbrec_acl_set_priority(acl, priority);
     nbrec_acl_set_direction(acl, direction);
     nbrec_acl_set_match(acl, ctx->argv[4]);
     nbrec_acl_set_action(acl, action);
+    if (stage) {
+        nbrec_acl_set_stage(acl, stage);
+    }
     if (shash_find(&ctx->options, "--log") != NULL) {
         nbrec_acl_set_log(acl, true);
     }
@@ -1254,7 +1272,7 @@ nbctl_acl_del(struct ctl_context *ctx)
     const struct nbrec_logical_switch *ls;
     ls = ls_by_name_or_uuid(ctx, ctx->argv[1], true);
 
-    if (ctx->argc != 2 && ctx->argc != 3 && ctx->argc != 5) {
+    if (ctx->argc != 2 && ctx->argc != 3 && ctx->argc != 5 && ctx->argc != 6) {
         ctl_fatal("cannot specify priority without match");
     }
 
@@ -1293,7 +1311,8 @@ nbctl_acl_del(struct ctl_context *ctx)
         struct nbrec_acl *acl = ls->acls[i];
 
         if (priority == acl->priority && !strcmp(ctx->argv[4], acl->match) &&
-             !strcmp(direction, acl->direction)) {
+             !strcmp(direction, acl->direction) && (ctx->argc == 5 ||
+             !strcmp(ctx->argv[5], acl->stage))) {
             struct nbrec_acl **new_acls
                 = xmemdup(ls->acls, sizeof *new_acls * ls->n_acls);
             new_acls[i] = ls->acls[ls->n_acls - 1];
@@ -2403,9 +2422,9 @@ static const struct ctl_command_syntax nbctl_commands[] = {
     { "ls-list", 0, 0, "", NULL, nbctl_ls_list, NULL, "", RO },
 
     /* acl commands. */
-    { "acl-add", 5, 5, "SWITCH DIRECTION PRIORITY MATCH ACTION", NULL,
+    { "acl-add", 5, 6, "SWITCH DIRECTION PRIORITY MATCH ACTION [STAGE]", NULL,
       nbctl_acl_add, NULL, "--log", RW },
-    { "acl-del", 1, 4, "SWITCH [DIRECTION [PRIORITY MATCH]]", NULL,
+    { "acl-del", 1, 5, "SWITCH [DIRECTION [PRIORITY MATCH [STAGE]]]", NULL,
       nbctl_acl_del, NULL, "", RW },
     { "acl-list", 1, 1, "SWITCH", NULL, nbctl_acl_list, NULL, "", RO },
 
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 5357ced..13d55e7 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -196,26 +196,32 @@ OVN_NBCTL_TEST_START
 AT_CHECK([ovn-nbctl ls-add ls0])
 AT_CHECK([ovn-nbctl --log acl-add ls0 from-lport 600 udp drop])
 AT_CHECK([ovn-nbctl --log acl-add ls0 to-lport 500 udp drop])
-AT_CHECK([ovn-nbctl acl-add ls0 from-lport 400 tcp drop])
+AT_CHECK([ovn-nbctl acl-add ls0 from-lport 400 tcp drop acl])
 AT_CHECK([ovn-nbctl acl-add ls0 to-lport 300 tcp drop])
+AT_CHECK([ovn-nbctl acl-add ls0 from-lport 300 'ip4.dst == 10.0.0.0/24' allow acl2])
 AT_CHECK([ovn-nbctl acl-add ls0 from-lport 200 ip drop])
 AT_CHECK([ovn-nbctl acl-add ls0 to-lport 100 ip drop])
+AT_CHECK([ovn-nbctl acl-add ls0 from-lport 0 ip drop acl2])
 
 AT_CHECK([ovn-nbctl acl-list ls0], [0], [dnl
-from-lport   600 (udp) drop log
-from-lport   400 (tcp) drop
-from-lport   200 (ip) drop
-  to-lport   500 (udp) drop log
-  to-lport   300 (tcp) drop
-  to-lport   100 (ip) drop
+     from-lport   600 (udp) drop log
+acl  from-lport   400 (tcp) drop
+     from-lport   200 (ip) drop
+       to-lport   500 (udp) drop log
+       to-lport   300 (tcp) drop
+       to-lport   100 (ip) drop
+acl2 from-lport   300 (ip4.dst == 10.0.0.0/24) allow
+acl2 from-lport     0 (ip) drop
 ])
 
 dnl Delete in one direction.
 AT_CHECK([ovn-nbctl acl-del ls0 to-lport])
 AT_CHECK([ovn-nbctl acl-list ls0], [0], [dnl
-from-lport   600 (udp) drop log
-from-lport   400 (tcp) drop
-from-lport   200 (ip) drop
+     from-lport   600 (udp) drop log
+acl  from-lport   400 (tcp) drop
+     from-lport   200 (ip) drop
+acl2 from-lport   300 (ip4.dst == 10.0.0.0/24) allow
+acl2 from-lport     0 (ip) drop
 ])
 
 dnl Delete all ACLs.
@@ -230,8 +236,8 @@ AT_CHECK([ovn-nbctl acl-add ls0 from-lport 200 ip drop])
 dnl Delete a single flow.
 AT_CHECK([ovn-nbctl acl-del ls0 from-lport 400 tcp])
 AT_CHECK([ovn-nbctl acl-list ls0], [0], [dnl
-from-lport   600 (udp) drop
-from-lport   200 (ip) drop
+     from-lport   600 (udp) drop
+     from-lport   200 (ip) drop
 ])
 
 OVN_NBCTL_TEST_STOP
-- 
1.9.1




More information about the dev mailing list