When multi-tier ACLs are configured, "ovn-nbctl acl-list" does
not display any tier information, making it difficult to see
which tier each ACL belongs to without querying the database
directly.

Display a "[tier N]" annotation for each ACL showing its tier
value.

Sort ACLs by tier (ascending) before priority so that ACLs
belonging to the same tier are grouped together.  The ascending
order matches the evaluation order of tiers.  Previously, tier
was compared after priority and in descending order.

Reported-at: https://redhat.atlassian.net/browse/FDP-3585
Assisted-by: Claude Opus 4.6, OpenCode
Signed-off-by: Ales Musil <[email protected]>
---
v2: Make tier a higher priority during ordering, change the tier order to be 
ascending to match the evaluation and print the tier every time.
---
 tests/ovn-nbctl.at    | 86 +++++++++++++++++++++++++++----------------
 utilities/ovn-nbctl.c |  7 +++-
 2 files changed, 60 insertions(+), 33 deletions(-)

diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 890d24dbf..44724083b 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -246,29 +246,29 @@ ovn_nbctl_test_acl() {
    AT_CHECK([grep 'label must in range 0...4294967295' stderr], [0], [ignore])
 
    AT_CHECK([ovn-nbctl $2 acl-list $1], [0], [dnl
-from-lport   600 (udp) drop log()
-from-lport   400 (tcp) drop
-from-lport   200 (ip) drop
-from-lport    70 (icmp) allow-related label=1234
-from-lport   500 (tcp) allow [[after-lb]]
-from-lport   300 (tcp) drop [[after-lb]]
-from-lport   300 (udp) allow [[after-lb]]
-  to-lport   500 (udp) drop log(name=test,severity=info)
-  to-lport   300 (tcp) drop
-  to-lport   100 (ip) drop
-  to-lport    70 (icmp) allow-related label=1235
+from-lport   600 (udp) drop log() [[tier 0]]
+from-lport   400 (tcp) drop [[tier 0]]
+from-lport   200 (ip) drop [[tier 0]]
+from-lport    70 (icmp) allow-related label=1234 [[tier 0]]
+from-lport   500 (tcp) allow [[tier 0]] [[after-lb]]
+from-lport   300 (tcp) drop [[tier 0]] [[after-lb]]
+from-lport   300 (udp) allow [[tier 0]] [[after-lb]]
+  to-lport   500 (udp) drop log(name=test,severity=info) [[tier 0]]
+  to-lport   300 (tcp) drop [[tier 0]]
+  to-lport   100 (ip) drop [[tier 0]]
+  to-lport    70 (icmp) allow-related label=1235 [[tier 0]]
 ])
 
    dnl Delete in one direction.
    AT_CHECK([ovn-nbctl $2 acl-del $1 to-lport])
    AT_CHECK([ovn-nbctl $2 acl-list $1], [0], [dnl
-from-lport   600 (udp) drop log()
-from-lport   400 (tcp) drop
-from-lport   200 (ip) drop
-from-lport    70 (icmp) allow-related label=1234
-from-lport   500 (tcp) allow [[after-lb]]
-from-lport   300 (tcp) drop [[after-lb]]
-from-lport   300 (udp) allow [[after-lb]]
+from-lport   600 (udp) drop log() [[tier 0]]
+from-lport   400 (tcp) drop [[tier 0]]
+from-lport   200 (ip) drop [[tier 0]]
+from-lport    70 (icmp) allow-related label=1234 [[tier 0]]
+from-lport   500 (tcp) allow [[tier 0]] [[after-lb]]
+from-lport   300 (tcp) drop [[tier 0]] [[after-lb]]
+from-lport   300 (udp) allow [[tier 0]] [[after-lb]]
 ])
 
    dnl Delete all ACLs.
@@ -283,8 +283,8 @@ from-lport   300 (udp) allow [[after-lb]]
    dnl Delete a single flow.
    AT_CHECK([ovn-nbctl $2 acl-del $1 from-lport 400 tcp])
    AT_CHECK([ovn-nbctl $2 acl-list $1], [0], [dnl
-from-lport   600 (udp) drop
-from-lport   200 (ip) drop
+from-lport   600 (udp) drop [[tier 0]]
+from-lport   200 (ip) drop [[tier 0]]
 ])
 }
 
@@ -329,14 +329,14 @@ check ovn-nbctl --type=port-group acl-add pg2 from-lport 
100 ip drop
 check ovn-nbctl --type=port-group acl-add pg2 from-lport 98 ip drop
 
 AT_CHECK([ovn-nbctl --all acl-list ls1], [0], [dnl
-from-lport   500 (ip) drop
-  to-lport   100 (ip) drop
-  to-lport   144 (ip) drop [[pg0]]
-  to-lport    11 (ip) drop [[pg0]]
-  to-lport    12 (ip) drop [[pg1]]
-from-lport   100 (ip) drop [[pg2]]
-from-lport    98 (ip) drop [[pg2]]
-  to-lport    99 (ip) drop [[pg2]]
+from-lport   500 (ip) drop [[tier 0]]
+  to-lport   100 (ip) drop [[tier 0]]
+  to-lport   144 (ip) drop [[tier 0]] [[pg0]]
+  to-lport    11 (ip) drop [[tier 0]] [[pg0]]
+  to-lport    12 (ip) drop [[tier 0]] [[pg1]]
+from-lport   100 (ip) drop [[tier 0]] [[pg2]]
+from-lport    98 (ip) drop [[tier 0]] [[pg2]]
+  to-lport    99 (ip) drop [[tier 0]] [[pg2]]
 ])
 
 check ovn-nbctl acl-del ls1
@@ -3019,12 +3019,25 @@ 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
 
+check ovn-nbctl acl-add ls from-lport 999 "ip" allow
+AT_CHECK([ovn-nbctl acl-list ls], [0], [dnl
+from-lport   999 (ip) allow [[tier 0]]
+from-lport  1002 (ip) drop [[tier 2]]
+from-lport  1001 (ip) drop [[tier 3]]
+from-lport  1000 (ip) drop [[tier 3]]
+])
+check ovn-nbctl acl-del ls from-lport 999 "ip"
+
 # 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
 
+AT_CHECK([ovn-nbctl acl-list ls], [0], [dnl
+from-lport  1002 (ip) drop [[tier 2]]
+])
+
 # 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
@@ -3072,12 +3085,23 @@ check_row_count nb:ACL 1 tier=1
 check_row_count nb:ACL 1 tier=2
 check_row_count nb:ACL 1 tier=3
 
+AT_CHECK([ovn-nbctl acl-list ls], [0], [dnl
+from-lport  1000 (ip) drop [[tier 1]]
+from-lport  1000 (ip) drop [[tier 2]]
+from-lport  1000 (ip) drop [[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
 
+AT_CHECK([ovn-nbctl acl-list ls], [0], [dnl
+from-lport  1000 (ip) drop [[tier 2]]
+from-lport  1000 (ip) drop [[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
@@ -3319,12 +3343,12 @@ AT_CHECK(ovn-nbctl pg-add pg0 p0 p1)
 check_uuid ovn-nbctl create Address_Set name=as0 addresses=\"10.1.1.5\"
 AT_CHECK([ovn-nbctl acl-add pg0 from-lport 100 'inport == @pg0 && ip4.dst == 
$as2' allow-related nfg0])
 AT_CHECK([ovn-nbctl acl-list pg0], [0], [dnl
-from-lport   100 (inport == @pg0 && ip4.dst == $as2) allow-related 
network-function-group=nfg0
+from-lport   100 (inport == @pg0 && ip4.dst == $as2) allow-related 
network-function-group=nfg0 [[tier 0]]
 ])
 AT_CHECK([ovn-nbctl acl-add pg0 to-lport 200 'outport == @pg0 && ip4.src == 
$as2' allow-related nfg1])
 AT_CHECK([ovn-nbctl acl-list pg0], [0], [dnl
-from-lport   100 (inport == @pg0 && ip4.dst == $as2) allow-related 
network-function-group=nfg0
-  to-lport   200 (outport == @pg0 && ip4.src == $as2) allow-related 
network-function-group=nfg1
+from-lport   100 (inport == @pg0 && ip4.dst == $as2) allow-related 
network-function-group=nfg0 [[tier 0]]
+  to-lport   200 (outport == @pg0 && ip4.src == $as2) allow-related 
network-function-group=nfg1 [[tier 0]]
 ])
 AT_CHECK([ovn-nbctl acl-del pg0 to-lport 200 'outport == @pg0 && ip4.src == 
$as2'])
 AT_CHECK([ovn-nbctl acl-del pg0 from-lport 100 'inport == @pg0 && ip4.dst == 
$as2'])
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 0900254c4..373c14793 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -2699,10 +2699,10 @@ acl_cmp(const void *acl1_, const void *acl2_)
         return dir1 < dir2 ? -1 : 1;
     } else if (after_lb1 != after_lb2) {
         return after_lb2 ? -1 : 1;
+    } else if (acl1->tier != acl2->tier) {
+        return acl1->tier < acl2->tier ? -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);
     }
@@ -2783,6 +2783,9 @@ nbctl_acl_print(struct ctl_context *ctx, const struct 
nbrec_acl **acls,
         if (acl->label) {
           ds_put_format(&ctx->output, " label=%"PRId64, acl->label);
         }
+
+        ds_put_format(&ctx->output, " [tier %"PRId64"]", acl->tier);
+
         if (smap_get_bool(&acl->options, "apply-after-lb", false)) {
             ds_put_cstr(&ctx->output, " [after-lb]");
         }
-- 
2.54.0

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to