diff mbox series

[ovs-dev,v5,4/4] acls: Add "pass" ACL action.

Message ID 20230518184842.1681582-4-mmichels@redhat.com
State Accepted
Headers show
Series [ovs-dev,v5,1/4] northd: Break ACLs into two stages. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success

Commit Message

Mark Michelson May 18, 2023, 6:48 p.m. UTC
This allows for evaluating ACLs at the current tier to stop, and to
start evaluating ACLs at the next tier. If not using tiers, or if we
match on the final ACL tier, then a "pass" verdict results in the
default ACL action being applied.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2134138

Signed-off-by: Mark Michelson <mmichels@redhat.com>
Reviewed-by: Ales Musil <amusil@redhat.com>
---
 northd/northd.c         |  8 ++++++-
 northd/ovn-northd.8.xml |  9 ++++++++
 ovn-nb.ovsschema        |  6 +++---
 ovn-nb.xml              | 10 +++++++++
 tests/ovn-northd.at     | 46 +++++++++++++++++++++++++++++++++++++++++
 tests/system-ovn.at     | 40 ++++++++++++++++++++++++++++++++---
 utilities/ovn-nbctl.c   |  2 +-
 7 files changed, 113 insertions(+), 8 deletions(-)
diff mbox series

Patch

diff --git a/northd/northd.c b/northd/northd.c
index 212327b99..07b127cdf 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -6377,6 +6377,8 @@  build_acl_log(struct ds *actions, const struct nbrec_acl *acl,
         ds_put_cstr(actions, "verdict=drop, ");
     } else if (!strcmp(acl->action, "reject")) {
         ds_put_cstr(actions, "verdict=reject, ");
+    } else if (!strcmp(acl->action, "pass")) {
+        ds_put_cstr(actions, "verdict=pass, ");
     } else if (!strcmp(acl->action, "allow")
         || !strcmp(acl->action, "allow-related")
         || !strcmp(acl->action, "allow-stateless")) {
@@ -6415,6 +6417,8 @@  consider_acl(struct hmap *lflows, struct ovn_datapath *od,
         verdict = REGBIT_ACL_VERDICT_DROP " = 1; ";
     } else if (!strcmp(acl->action, "reject")) {
         verdict = REGBIT_ACL_VERDICT_REJECT " = 1; ";
+    } else if (!strcmp(acl->action, "pass")) {
+        verdict = "";
     } else {
         verdict = REGBIT_ACL_VERDICT_ALLOW " = 1; ";
     }
@@ -6434,7 +6438,9 @@  consider_acl(struct hmap *lflows, struct ovn_datapath *od,
         match_tier_len = match->length;
     }
 
-    if (!has_stateful || !strcmp(acl->action, "allow-stateless")) {
+    if (!has_stateful
+        || !strcmp(acl->action, "pass")
+        || !strcmp(acl->action, "allow-stateless")) {
         ds_put_cstr(actions, "next;");
         ds_put_format(match, "(%s)", acl->match);
         ovn_lflow_add_with_hint(lflows, od, stage, priority,
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index c0b7a69f2..540fe03bd 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -743,6 +743,10 @@ 
         <code>reject</code> ACLs translate into logical flows with that set the
         reject bit and advance to the next table.
       </li>
+      <li>
+        <code>pass</code> ACLs translate into logical flows that do not set the
+        allow, drop, or reject bit and advance to the next table.
+      </li>
       <li>
         Other ACLs set the drop bit and advance to the next table for new or
         untracked connections. For known connections, they set the drop bit,
@@ -1223,6 +1227,11 @@ 
         <code>reject</code> apply-after-lb ACLs translate into logical
         flows that set the reject bit and advance to the next table.
       </li>
+      <li>
+        <code>pass</code> apply-after-lb ACLs translate into logical flows that
+        do not set the allow, drop, or reject bit and advance to the next
+        table.
+      </li>
       <li>
         Other apply-after-lb ACLs set the drop bit for new or untracked
         connections and <code>ct_commit(ct_label=1/1);</code> for known
diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
index 789275f37..f8bac5302 100644
--- a/ovn-nb.ovsschema
+++ b/ovn-nb.ovsschema
@@ -1,7 +1,7 @@ 
 {
     "name": "OVN_Northbound",
-    "version": "7.0.3",
-    "cksum": "3667000751 33787",
+    "version": "7.0.4",
+    "cksum": "1676649201 33795",
     "tables": {
         "NB_Global": {
             "columns": {
@@ -260,7 +260,7 @@ 
                                             "enum": ["set",
                                                ["allow", "allow-related",
                                                 "allow-stateless", "drop",
-                                                "reject"]]}}},
+                                                "reject", "pass"]]}}},
                 "log": {"type": "boolean"},
                 "severity": {"type": {"key": {"type": "string",
                                               "enum": ["set",
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 4a8279f6a..9afe3b584 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -2281,6 +2281,16 @@  or
           ICMPv4/ICMPv6 unreachable message for other IPv4/IPv6-based
           protocols.
         </li>
+
+        <li>
+          <code>pass</code>: Pass to the next ACL tier. If using multiple ACL
+          tiers, a match on this ACL will stop evaluating ACLs at the current
+          tier and move to the next one. If not using ACL tiers or if a
+          <code>pass</code> ACL is matched at the final tier, then the
+          <ref column="options" key="default_acl_drop" table="NB_Global" />
+          option from the <ref table="NB_Global" /> table is used to
+          determine how to proceed.
+        </li>
       </ul>
     </column>
 
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 7d94cd6ac..2c4129bf3 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -9379,3 +9379,49 @@  acl_test to-lport "" pg ls_out_acl_eval ls_out_acl_action 4
 
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([ACL "pass" logical flows])
+AT_KEYWORDS([acl])
+
+ovn_start
+check ovn-nbctl ls-add ls
+check ovn-nbctl lsp-add ls lsp
+check ovn-nbctl pg-add pg lsp
+
+m4_define([ACL_FLOWS], [grep -w $1 lflows | grep "$2" | sed 's/table=../table=??/' | sed "s/\($1[[^)]]*\)/$1/" | sort])
+
+acl_test() {
+    direction=$1
+    options=$2
+    thing=$3
+    eval_stage=$4
+
+    # Baseline. Ensure no ACL eval flows are present.
+    ovn-sbctl lflow-list ls > lflows
+    AT_CHECK([ACL_FLOWS([$eval_stage], [priority=2000])], [0], [])
+
+    # Add an ACL with the "pass" verdict. Ensure that it is in the logical flow
+    # table and that it simply moves to the next table without setting a specific
+    # verdict bit.
+    check ovn-nbctl --wait=sb $options acl-add $thing $direction 1000 "ip4.addr == 80.111.111.112" pass
+    ovn-sbctl lflow-list ls > lflows
+    AT_CHECK_UNQUOTED([ACL_FLOWS([$eval_stage], [priority=2000])], [0], [dnl
+  table=??($eval_stage), priority=2000 , match=((ip4.addr == 80.111.111.112)), action=(next;)
+])
+
+    # Remove the ACL with the "pass" verdict. Ensure that no eval flows are present.
+    check ovn-nbctl acl-del $thing
+    ovn-sbctl lflow-list ls > lflows
+    AT_CHECK([ACL_FLOWS([$eval_stage], [priority=2000])], [0], [])
+}
+
+acl_test from-lport "" ls ls_in_acl_eval
+acl_test from-lport "--apply-after-lb" ls ls_in_acl_after_lb_eval
+acl_test to-lport "" ls ls_out_acl_eval
+acl_test from-lport "" pg ls_in_acl_eval
+acl_test from-lport "--apply-after-lb" pg ls_in_acl_after_lb_eval
+acl_test to-lport "" pg ls_out_acl_eval
+
+AT_CLEANUP
+])
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 1b39a320b..68a5e9527 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -11378,20 +11378,54 @@  acl_test() {
 
     # Add a tier-0 ACL that allows the traffic. The priority is only 4, but
     # since it is a higher tier, the traffic should be allowed.
-    check ovn-nbctl --wait=sb $options acl-add $thing $direction 4 "ip4.dst == 10.0.0.2" allow
+    check ovn-nbctl --wait=hv $options acl-add $thing $direction 4 "ip4.dst == 10.0.0.2" allow
     NS_CHECK_EXEC([lsp1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 | PING_PCT], \
 [0], [dnl
 0% packet loss
 ])
 
-    # Removing the 0-tier ACL should make traffic go back to being dropped.
+    # Add a higher-priority tier-0 ACL that passes. This should cause the traffic
+    # to pass over the lower-priority tier-0 "allow" ACL, and move to the tier-3
+    # ACL that drops the traffic.
+    check ovn-nbctl --wait=sb $options acl-add $thing $direction 1000 "ip4.dst == 10.0.0.2" pass
+    NS_CHECK_EXEC([lsp1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 | PING_PCT], \
+[0], [dnl
+100% packet loss
+])
+
+    # Remove the "pass" ACL, and the "allow" rule should kick back in.
+    check ovn-nbctl --wait=sb --tier=0 acl-del $thing $direction 1000 "ip4.dst == 10.0.0.2"
+    NS_CHECK_EXEC([lsp1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 | PING_PCT], \
+[0], [dnl
+0% packet loss
+])
+
+    # Removing the remaining 0-tier ACL should make traffic go back to being dropped.
     check ovn-nbctl --wait=sb acl-del $thing $direction 4 "ip4.dst == 10.0.0.2"
     NS_CHECK_EXEC([lsp1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 | PING_PCT], \
 [0], [dnl
 100% packet loss
 ])
 
-    # Removing all ACLs should make traffic go back to passing.
+    # Adding a higher-priority "pass" ACL at tier 3 should result in using the
+    # default ACL action. Currently, the default is to allow traffic, so the
+    # traffic should be allowed.
+    check ovn-nbctl --wait=sb --tier=3 $options acl-add $thing $direction 2000 "ip4.dst == 10.0.0.2" pass
+    NS_CHECK_EXEC([lsp1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 | PING_PCT], \
+[0], [dnl
+0% packet loss
+])
+
+    # Change the default ACL action to drop, and now the traffic should be dropped.
+    check ovn-nbctl set NB_Global . options:default_acl_drop=true
+    NS_CHECK_EXEC([lsp1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 | PING_PCT], \
+[0], [dnl
+100% packet loss
+])
+
+    # Removing all ACLs (and setting the default acl drop back to false) should
+    # make traffic go back to passing.
+    check ovn-nbctl clear NB_Global . options
     check ovn-nbctl --wait=sb acl-del $thing
     NS_CHECK_EXEC([lsp1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 | PING_PCT], \
 [0], [dnl
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index de03f870c..7a4f6b1b3 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -2348,7 +2348,7 @@  nbctl_acl_add(struct ctl_context *ctx)
     /* Validate action. */
     if (strcmp(action, "allow") && strcmp(action, "allow-related")
         && strcmp(action, "allow-stateless") && strcmp(action, "drop")
-        && strcmp(action, "reject")) {
+        && strcmp(action, "reject") && strcmp(action, "pass")) {
         ctl_error(ctx, "%s: action must be one of \"allow\", "
                   "\"allow-related\", \"allow-stateless\", \"drop\", "
                   "and \"reject\"", action);