On 4/28/23 07:46, Ales Musil wrote:
On Fri, Apr 21, 2023 at 9:52 PM Mark Michelson <[email protected] <mailto:[email protected]>> wrote: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 <https://bugzilla.redhat.com/show_bug.cgi?id=2134138> Signed-off-by: Mark Michelson <[email protected] <mailto:[email protected]>> Hi Mark, there is one unrelated change other than that it looks good. Reviewed-by: Ales Musil <[email protected] <mailto:[email protected]>> --- northd/northd.c | 8 +++++++- ovn-nb.ovsschema | 4 ++-- ovn-nb.xml | 10 ++++++++++ tests/ovn-northd.at <http://ovn-northd.at> | 46 +++++++++++++++++++++++++++++++++++++++++++ tests/system-ovn.at <http://system-ovn.at> | 41 ++++++++++++++++++++++++++++++++++---- utilities/ovn-nbctl.c | 2 +- 6 files changed, 103 insertions(+), 8 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index 647e70a0d..9358887e3 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -6414,6 +6414,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")) { @@ -6452,6 +6454,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; "; } @@ -6471,7 +6475,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/ovn-nb.ovsschema b/ovn-nb.ovsschema index f12d39542..e713cce46 100644 --- a/ovn-nb.ovsschema +++ b/ovn-nb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Northbound", "version": "7.0.0", - "cksum": "3195094080 33650", + "cksum": "2504399077 33658", "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 d5606ce7d..0144e0934 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -2263,6 +2263,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 <http://ovn-northd.at> b/tests/ovn-northd.at <http://ovn-northd.at> index d8562c9f1..b6717a4eb 100644 --- a/tests/ovn-northd.at <http://ovn-northd.at> +++ b/tests/ovn-northd.at <http://ovn-northd.at> @@ -9267,3 +9267,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 <http://system-ovn.at> b/tests/system-ovn.at <http://system-ovn.at> index 7488d0a53..dabef9e35 100644 --- a/tests/system-ovn.at <http://system-ovn.at> +++ b/tests/system-ovn.at <http://system-ovn.at> @@ -10714,7 +10714,6 @@ acl_test() { # Add an untiered drop ACL. This should cause pings to fail. check ovn-nbctl --wait=sb $options acl-add $thing $direction 1000 "ip4.dst == 10.0.0.2" drop acl1_uuid=$(ovn-nbctl --bare --columns _uuid find ACL priority=1000) - nit: Unrelated change
In v4 I address this by not introducing this blank line in patch 3 of the series.
NS_CHECK_EXEC([lsp1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 | PING_PCT], \ [0], [dnl 100% packet loss @@ -10730,20 +10729,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 +]) + + # 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 0-tier ACL should make traffic go back to being dropped. + # 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 104f492a5..897f60900 100644 --- a/utilities/ovn-nbctl.c +++ b/utilities/ovn-nbctl.c @@ -2332,7 +2332,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);-- 2.39.2_______________________________________________ dev mailing list [email protected] <mailto:[email protected]> https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> Thanks, Ales -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> [email protected] <mailto:[email protected]> IM: amusil <https://red.ht/sig>
_______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
