This modifies the acl-add and acl-del commands so that an ACL
tier can be specified when adding or deleting ACLs.
For acl-add, if the tier is specified, then the ACL created by the
command will have that tier set.
For acl-del, if the tier is specified, then the tier will be one of the
criteria used when deciding which ACLs to delete. Because the tier is
not any more or less specific than the other criteria used for deleting
ACLs, a bitmap approach is used to determine the final set of ACLs that
should be deleted.
Signed-off-by: Mark Michelson <mmich...@redhat.com>
Reviewed-by: Ales Musil <amu...@redhat.com>
---
NEWS | 3 +
tests/ovn-nbctl.at | 77 ++++++++++++++++++++++
tests/system-ovn.at | 1 -
utilities/ovn-nbctl.8.xml | 29 ++++++---
utilities/ovn-nbctl.c | 131 ++++++++++++++++++++++++++------------
5 files changed, 192 insertions(+), 49 deletions(-)
diff --git a/NEWS b/NEWS
index a7a11061f..e29193104 100644
--- a/NEWS
+++ b/NEWS
@@ -31,6 +31,9 @@ Post v23.03.0
port is possible to define QoS rules to apply to the local egress localnet
port. Please note now the QoS will be applied just to the local localnet
port and not to all localnet port marked with ovn-egress iface.
+ - Support for tiered ACLs has been added. This allows for ACLs to be layered
+ into separate tiers of priority. For more information, please see the
+ ovn-nb and ovn-northd manpages.
OVN v23.03.0 - 03 Mar 2023
--------------------------
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 478a32f5a..fde3a28ee 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -2616,6 +2616,83 @@ ovn-nbctl: no row "foo1" in table Logical_Switch
dnl ---------------------------------------------------------------------
+OVN_NBCTL_TEST([acl_tiers], [ACL tier operations], [
+check ovn-nbctl ls-add ls
+check ovn-nbctl --tier=3 acl-add ls from-lport 1000 "ip" drop
+check_column 3 nb:ACL tier priority=1000
+
+check ovn-nbctl --tier=3 acl-add ls from-lport 1001 "ip" drop
+check_column 3 nb:ACL tier priority=1001
+
+check ovn-nbctl --tier=2 acl-add ls from-lport 1002 "ip" drop
+check_column 2 nb:ACL tier priority=1002
+
+# Removing the tier 3 acls from ls should result in 1 ACL
+# remaining.
+check ovn-nbctl --tier=3 acl-del ls
+check_row_count nb:ACL 1
+check_column 2 nb:ACL tier priority=1002
+
+# Add two egress ACLs at tier 2.
+check ovn-nbctl --tier=2 acl-add ls to-lport 1000 "ip" drop
+check ovn-nbctl --tier=2 acl-add ls to-lport 1001 "ip" drop
+
+check_row_count nb:ACL 3 tier=2
+
+# This should remove the egress tier 2 ACLs and leave the
+# ingress tier 2 ACL
+check ovn-nbctl --tier=2 acl-del ls to-lport
+check_row_count nb:ACL 1
+check_column 2 nb:ACL tier priority=1002
+check_column from-lport nb:ACL direction priority=1002
+
+# Re-add two ingress ACLs at tier 2.
+check ovn-nbctl --tier=2 acl-add ls from-lport 1000 "ip" drop
+check ovn-nbctl --tier=2 acl-add ls from-lport 1001 "ip" drop
+
+check_row_count nb:ACL 3
+
+# Attempt to remove all tier 3 ACLs. All three ACLs are tier 2
+# so this shouldn't have any effect.
+check ovn-nbctl --tier=3 acl-del ls
+check_row_count nb:ACL 3
+
+# Attempt to remove all ingress tier 3 ACLs. All three ACLs are tier
+# 2, so this shouldn't have any effect.
+check ovn-nbctl --tier=3 acl-del ls from-lport
+check_row_count nb:ACL 3
+
+# Attempt to remove the 1000 priority ACL but specify tier 3. Since
+# all ACLs are tier 2, this should have no effect.
+check ovn-nbctl --tier=3 acl-del ls from-lport 1000 "ip"
+check_row_count nb:ACL 3
+
+# Specifying the proper tier should result in all ACLs being deleted.
+check ovn-nbctl --tier=2 acl-del ls
+check_row_count nb:ACL 0
+
+# Now let's experiment with identical ACLs at different tiers.
+check ovn-nbctl --tier=1 acl-add ls from-lport 1000 "ip" drop
+check ovn-nbctl --tier=2 acl-add ls from-lport 1000 "ip" drop
+check ovn-nbctl --tier=3 acl-add ls from-lport 1000 "ip" drop
+check_row_count nb:ACL 3
+check_row_count nb:ACL 1 tier=1
+check_row_count nb:ACL 1 tier=2
+check_row_count nb:ACL 1 tier=3
+
+# Specifying tier 1 should result in only one ACL being deleted.
+check ovn-nbctl --tier=1 acl-del ls from-lport 1000 "ip"
+check_row_count nb:ACL 2
+check_row_count nb:ACL 1 tier=2
+check_row_count nb:ACL 1 tier=3
+
+# Not specifying a tier should result in all ACLs being deleted.
+check ovn-nbctl acl-del ls from-lport 1000 "ip"
+check_row_count nb:ACL 0
+])
+
+dnl ---------------------------------------------------------------------
+
AT_SETUP([ovn-nbctl - daemon retry connection])
OVN_NBCTL_TEST_START daemon
AT_CHECK([kill `cat ovsdb-server.pid`])
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index b9288c24f..1b39a320b 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -11363,7 +11363,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)
-
NS_CHECK_EXEC([lsp1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 | PING_PCT], \
[0], [dnl
100% packet loss
diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
index 8b4c3f29a..b4af43185 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>--label=</code><var>label</var>]
[<code>--may-exist</code>] [<code>--apply-after-lb</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>--apply-after-lb</code>]
[<code>--tier</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>
@@ -430,16 +430,29 @@
of the <code>ACL</code> table. As the option name suggests, the ACL
will be applied after the logical switch load balancer stage.
</p>
+ <p>
+ The <code>--tier</code> option sets the ACL's tier to the specified
+ value. For more information about ACL tiers, see the documentation
+ for the <code>ovn-nb</code>(5) database.
+ </p>
</dd>
- <dt>[<code>--type=</code>{<code>switch</code> |
<code>port-group</code>}] <code>acl-del</code> <var>entity</var>
[<var>direction</var> [<var>priority</var> <var>match</var>]]</dt>
+ <dt>[<code>--type=</code>{<code>switch</code> |
<code>port-group</code>}] [<code>--tier</code>] <code>acl-del</code>
<var>entity</var> [<var>direction</var> [<var>priority</var>
<var>match</var>]]</dt>
<dd>
- Deletes ACLs from <var>entity</var>. If only <var>entity</var> is
- supplied, all the ACLs from the <var>entity</var> are deleted. If
- <var>direction</var> is also specified, then all the flows in that
- direction will be deleted from the <var>entity</var>. If all the
- fields are given, then a single flow that matches all the fields will
- be deleted.
+ <p>
+ Deletes ACLs from <var>entity</var>. If only <var>entity</var> is
+ supplied, all the ACLs from the <var>entity</var> are deleted. If
+ <var>direction</var> is also specified, then all the flows in that
+ direction will be deleted from the <var>entity</var>. If all the
+ fields are given, then a single flow that matches all the fields will
+ be deleted.
+ </p>
+
+ <p>
+ If the <code>--tier</code> option is provided, then only ACLs of the
+ given tier value will be deleted, in addition to whatever other
+ criteria have been provided.
+ </p>
</dd>
<dt>[<code>--type=</code>{<code>switch</code> |
<code>port-group</code>}] <code>acl-list</code> <var>entity</var> </dt>
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 279088aaf..de03f870c 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -48,6 +48,7 @@
#include "unixctl.h"
#include "util.h"
#include "openvswitch/vlog.h"
+#include "bitmap.h"
VLOG_DEFINE_THIS_MODULE(nbctl);
@@ -2116,6 +2117,8 @@ acl_cmp(const void *acl1_, const void *acl2_)
return after_lb2 ? -1 : 1;
} else if (acl1->priority != acl2->priority) {
return acl1->priority > acl2->priority ? -1 : 1;
+ } else if (acl1->tier != acl2->tier) {
+ return acl1->tier > acl2->tier ? -1 : 1;
} else {
return strcmp(acl1->match, acl2->match);
}
@@ -2299,6 +2302,7 @@ nbctl_pre_acl(struct ctl_context *ctx)
ovsdb_idl_add_column(ctx->idl, &nbrec_acl_col_priority);
ovsdb_idl_add_column(ctx->idl, &nbrec_acl_col_match);
ovsdb_idl_add_column(ctx->idl, &nbrec_acl_col_options);
+ ovsdb_idl_add_column(ctx->idl, &nbrec_acl_col_tier);
}
static void
@@ -2406,6 +2410,16 @@ nbctl_acl_add(struct ctl_context *ctx)
nbrec_acl_set_options(acl, &options);
}
+ const char *tier_s = shash_find_data(&ctx->options, "--tier");
+ if (tier_s) {
+ long tier;
+ if (!str_to_long(tier_s, 10, &tier)) {
+ ctl_error(ctx, "Invalid tier %s", tier_s);
+ return;
+ }
+ nbrec_acl_set_tier(acl, tier);
+ }
+
/* 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;
@@ -2434,6 +2448,10 @@ nbctl_acl_del(struct ctl_context *ctx)
{
const struct nbrec_logical_switch *ls = NULL;
const struct nbrec_port_group *pg = NULL;
+ const char *tier_s = shash_find_data(&ctx->options, "--tier");
+ long tier;
+ unsigned long *bitmaps[3];
+ size_t n_bitmaps = 0;
char *error = acl_cmd_get_pg_or_ls(ctx, &ls, &pg);
if (error) {
@@ -2441,8 +2459,13 @@ nbctl_acl_del(struct ctl_context *ctx)
return;
}
- if (ctx->argc == 2) {
- /* If direction, priority, and match are not specified, delete
+ if (tier_s && !str_to_long(tier_s, 10, &tier)) {
+ ctl_error(ctx, "Invalid tier %s", tier_s);
+ return;
+ }
+
+ if (ctx->argc == 2 && !tier_s) {
+ /* If direction, priority, tier, and match are not specified, delete
* all ACLs. */
if (pg) {
nbrec_port_group_verify_acls(pg);
@@ -2454,55 +2477,83 @@ nbctl_acl_del(struct ctl_context *ctx)
return;
}
- const char *direction;
- error = parse_direction(ctx->argv[2], &direction);
- if (error) {
- ctx->error = error;
- return;
- }
-
size_t n_acls = pg ? pg->n_acls : ls->n_acls;
struct nbrec_acl **acls = pg ? pg->acls : ls->acls;
- /* If priority and match are not specified, delete all ACLs with the
- * specified direction. */
- if (ctx->argc == 3) {
+
+ if (tier_s) {
+ bitmaps[n_bitmaps] = bitmap_allocate(n_acls);
for (size_t i = 0; i < n_acls; i++) {
- if (!strcmp(direction, acls[i]->direction)) {
- if (pg) {
- nbrec_port_group_update_acls_delvalue(pg, acls[i]);
- } else {
- nbrec_logical_switch_update_acls_delvalue(ls, acls[i]);
- }
+ if (acls[i]->tier == tier) {
+ bitmap_set1(bitmaps[n_bitmaps], i);
}
}
- return;
+ n_bitmaps++;
}
- int64_t priority;
- error = parse_priority(ctx->argv[3], &priority);
- if (error) {
- ctx->error = error;
- return;
- }
+ if (ctx->argc >= 3) {
+ const char *direction;
+ error = parse_direction(ctx->argv[2], &direction);
+ if (error) {
+ ctx->error = error;
+ goto cleanup;
+ }
- if (ctx->argc == 4) {
- ctl_error(ctx, "cannot specify priority without match");
- return;
+ /* If priority and match are not specified, delete all ACLs with the
+ * specified direction. */
+ bitmaps[n_bitmaps] = bitmap_allocate(n_acls);
+ for (size_t i = 0; i < n_acls; i++) {
+ if (!strcmp(direction, acls[i]->direction)) {
+ bitmap_set1(bitmaps[n_bitmaps], i);
+ }
+ }
+ n_bitmaps++;
}
- /* Remove the matching rule. */
- for (size_t i = 0; i < n_acls; i++) {
- struct nbrec_acl *acl = acls[i];
+ if (ctx->argc >= 4) {
+ int64_t priority;
+ error = parse_priority(ctx->argv[3], &priority);
+ if (error) {
+ ctx->error = error;
+ goto cleanup;
+ }
- if (priority == acl->priority && !strcmp(ctx->argv[4], acl->match) &&
- !strcmp(direction, acl->direction)) {
- if (pg) {
- nbrec_port_group_update_acls_delvalue(pg, acl);
- } else {
- nbrec_logical_switch_update_acls_delvalue(ls, acl);
+ if (ctx->argc == 4) {
+ ctl_error(ctx, "cannot specify priority without match");
+ goto cleanup;
+ }
+
+ /* Remove the matching rule. */
+ bitmaps[n_bitmaps] = bitmap_allocate(n_acls);
+ for (size_t i = 0; i < n_acls; i++) {
+ struct nbrec_acl *acl = acls[i];
+
+ if (priority == acl->priority &&
+ !strcmp(ctx->argv[4], acl->match)) {
+ bitmap_set1(bitmaps[n_bitmaps], i);
}
- return;
}
+ n_bitmaps++;
+ }
+
+ unsigned long *bitmap_result = bitmap_allocate1(n_acls);
+ for (size_t i = 0; i < n_bitmaps; i++) {
+ bitmap_result = bitmap_and(bitmap_result, bitmaps[i], n_acls);
+ }
+
+ size_t index;
+ BITMAP_FOR_EACH_1 (index, n_acls, bitmap_result) {
+ if (pg) {
+ nbrec_port_group_update_acls_delvalue(pg, acls[index]);
+ } else {
+ nbrec_logical_switch_update_acls_delvalue(ls, acls[index]);
+ }
+ }
+
+ free(bitmap_result);
+
+cleanup:
+ for (size_t i = 0; i < n_bitmaps; i++) {
+ free(bitmaps[i]);
}
}
@@ -7680,9 +7731,9 @@ static const struct ctl_command_syntax nbctl_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=,--label=,"
- "--apply-after-lb", RW },
+ "--apply-after-lb,--tier=", RW },
{ "acl-del", 1, 4, "{SWITCH | PORTGROUP} [DIRECTION [PRIORITY MATCH]]",
- nbctl_pre_acl, nbctl_acl_del, NULL, "--type=", RW },
+ nbctl_pre_acl, nbctl_acl_del, NULL, "--type=,--tier=", RW },
{ "acl-list", 1, 1, "{SWITCH | PORTGROUP}",
nbctl_pre_acl_list, nbctl_acl_list, NULL, "--type=", RO },
--
2.39.2
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
