@@ -2590,6 +2590,87 @@ 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 acl-add ls from-lport 1000 "ip" drop
+#check_column 0 nb:ACL tier priority=1000
+#
+#check ovn-nbctl acl-del 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`])
@@ -48,6 +48,7 @@
#include "unixctl.h"
#include "util.h"
#include "openvswitch/vlog.h"
+#include "bitmap.h"
VLOG_DEFINE_THIS_MODULE(nbctl);
@@ -2100,6 +2101,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);
}
@@ -2283,6 +2286,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
@@ -2390,6 +2394,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) {
+ int64_t 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;
@@ -2418,6 +2432,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");
+ int64_t tier;
+ unsigned long *bitmaps[3];
+ size_t n_bitmaps = 0;
char *error = acl_cmd_get_pg_or_ls(ctx, &ls, &pg);
if (error) {
@@ -2425,8 +2443,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);
@@ -2438,55 +2461,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]);
}
}
@@ -7658,9 +7709,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 },
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 <mmichels@redhat.com> --- tests/ovn-nbctl.at | 81 ++++++++++++++++++++++++++ utilities/ovn-nbctl.c | 131 +++++++++++++++++++++++++++++------------- 2 files changed, 172 insertions(+), 40 deletions(-)