diff mbox series

[ovs-dev,v2,1/3] northd: Avoid recompute of lflow from ACLs without meters.

Message ID 20250514081246.985906-2-amusil@redhat.com
State Superseded
Headers show
Series northd: Add ACL I-P | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Ales Musil May 14, 2025, 8:12 a.m. UTC 
The lflow has dependency on sync_meters node so any change in
sync_meters will cause full lflow recompute. Add handler for ACLs
into sync_meters node that will check if the ACL actually requires
any meters. In case it does it will trigger recompute. However, in
most cases the ACLs are without metering which should avoid expensive
recomopute of lflow node.

Signed-off-by: Ales Musil <amusil@redhat.com>
---
v2: Rebase on top of latest main.
---
 northd/en-meters.c       | 24 ++++++++++++
 northd/en-meters.h       |  2 +
 northd/inc-proc-northd.c |  2 +-
 tests/ovn-northd.at      | 80 +++++++++++++++++++++++++++++++++++++++-
 4 files changed, 105 insertions(+), 3 deletions(-)
diff mbox series

Patch

diff --git a/northd/en-meters.c b/northd/en-meters.c
index a0352c34b..288134108 100644
--- a/northd/en-meters.c
+++ b/northd/en-meters.c
@@ -79,6 +79,30 @@  en_sync_meters_run(struct engine_node *node, void *data_)
     return EN_UPDATED;
 }
 
+enum engine_input_handler_result
+sync_meters_nb_acl_handler(struct engine_node *node, void *data OVS_UNUSED)
+{
+    const struct nbrec_acl_table *acl_table =
+        EN_OVSDB_GET(engine_get_input("NB_acl", node));
+
+    const struct nbrec_acl *nb_acl;
+    NBREC_ACL_TABLE_FOR_EACH_TRACKED (nb_acl, acl_table) {
+        /* New or deleted ACL with meter needs to be recomputed. */
+        if ((nbrec_acl_is_new(nb_acl) || nbrec_acl_is_deleted(nb_acl)) &&
+            (nb_acl->log || nb_acl->meter)) {
+            return EN_UNHANDLED;
+        }
+
+        /* Addition or removal of meter requires recompute. */
+        if (nbrec_acl_is_updated(nb_acl, NBREC_ACL_COL_LOG) ||
+            nbrec_acl_is_updated(nb_acl, NBREC_ACL_COL_METER)) {
+            return EN_UNHANDLED;
+        }
+    }
+
+    return EN_HANDLED_UNCHANGED;
+}
+
 const struct nbrec_meter*
 fair_meter_lookup_by_name(const struct shash *meter_groups,
                           const char *meter_name)
diff --git a/northd/en-meters.h b/northd/en-meters.h
index e0ef07fad..e1deb5daf 100644
--- a/northd/en-meters.h
+++ b/northd/en-meters.h
@@ -29,6 +29,8 @@  struct sync_meters_data {
 void *en_sync_meters_init(struct engine_node *, struct engine_arg *);
 void en_sync_meters_cleanup(void *data);
 enum engine_node_state en_sync_meters_run(struct engine_node *, void *data);
+enum engine_input_handler_result
+sync_meters_nb_acl_handler(struct engine_node *, void *data);
 
 const struct nbrec_meter *fair_meter_lookup_by_name(
     const struct shash *meter_groups,
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index b1e4994a4..5905462ec 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -319,7 +319,7 @@  void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
     engine_add_input(&en_group_ecmp_route, &en_learned_route_sync,
                      group_ecmp_route_learned_route_change_handler);
 
-    engine_add_input(&en_sync_meters, &en_nb_acl, NULL);
+    engine_add_input(&en_sync_meters, &en_nb_acl, sync_meters_nb_acl_handler);
     engine_add_input(&en_sync_meters, &en_nb_meter, NULL);
     engine_add_input(&en_sync_meters, &en_sb_meter, NULL);
 
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 69b75fe9d..39f27772b 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -11352,14 +11352,14 @@  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb meter-add m drop 1 pktps
 check ovn-nbctl --wait=sb acl-add ls from-lport 1 1 allow
 dnl Only triggers recompute of the sync_meters and lflow nodes.
-check_recompute_counter 0 2 2
+check_recompute_counter 0 2 1
 CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
 
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb meter-del m
 check ovn-nbctl --wait=sb acl-del ls
 dnl Only triggers recompute of the sync_meters and lflow nodes.
-check_recompute_counter 0 2 2
+check_recompute_counter 0 2 1
 CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
 
 AT_CLEANUP
@@ -17290,3 +17290,79 @@  AT_CHECK([cat trace | grep output], [0], [dnl
 
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([ACL incremental processing])
+ovn_start
+
+check ovn-nbctl ls-add ls
+check ovn-nbctl meter-add meter1 drop 10 kbps
+check ovn-nbctl meter-add meter2 drop 20 kbps
+
+# Wait for sb to be connected before clearing stats.
+check ovn-nbctl --wait=sb sync
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+
+AS_BOX([ACL with log])
+check ovn-nbctl --wait=sb --log acl-add ls from-lport 100 tcp drop
+acl_id=$(fetch_column nb:Acl _uuid action=drop)
+check_engine_stats northd norecompute compute
+check_engine_stats lflow recompute nocompute
+check_engine_stats sync_meters recompute nocompute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+
+check ovn-nbctl --wait=sb set acl $acl_id match=udp
+check_engine_stats northd norecompute compute
+check_engine_stats lflow recompute nocompute
+check_engine_stats sync_meters norecompute compute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+
+check ovn-nbctl --wait=sb set acl $acl_id log=false
+check_engine_stats northd norecompute compute
+check_engine_stats lflow recompute nocompute
+check_engine_stats sync_meters recompute nocompute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+check ovn-nbctl --wait=sb set acl $acl_id log=true
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+
+check ovn-nbctl --wait=sb acl-del ls
+check_engine_stats northd norecompute compute
+check_engine_stats lflow recompute nocompute
+check_engine_stats sync_meters recompute nocompute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+
+AS_BOX([ACL with meter])
+check ovn-nbctl --wait=sb --meter=meter1 acl-add ls from-lport 100 tcp drop
+acl_id=$(fetch_column nb:Acl _uuid action=drop)
+check_engine_stats northd norecompute compute
+check_engine_stats lflow recompute nocompute
+check_engine_stats sync_meters recompute nocompute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+
+check ovn-nbctl --wait=sb set acl $acl_id match=udp
+check_engine_stats northd norecompute compute
+check_engine_stats lflow recompute nocompute
+check_engine_stats sync_meters norecompute compute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+
+check ovn-nbctl --wait=sb set acl $acl_id meter=meter2
+check_engine_stats northd norecompute compute
+check_engine_stats lflow recompute nocompute
+check_engine_stats sync_meters recompute nocompute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+
+check ovn-nbctl --wait=sb acl-del ls
+check_engine_stats northd norecompute compute
+check_engine_stats lflow recompute nocompute
+check_engine_stats sync_meters recompute nocompute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+
+AT_CLEANUP
+])