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> --- tests/ovn-nbctl.at | 77 ++++++++++++++++++++++ utilities/ovn-nbctl.8.xml | 29 ++++++--- utilities/ovn-nbctl.c | 131 ++++++++++++++++++++++++++------------ 3 files changed, 189 insertions(+), 48 deletions(-) diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index aa5baade4..52c98fd5e 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/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml index 54dbdb791..1fc00a927 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 22313ecd5..104f492a5 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); @@ -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) { + 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; @@ -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"); + long 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 }, -- 2.39.2 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev