On Tue, Sep 8, 2026 at 6:32 AM Ales Musil via dev <[email protected]>
wrote:
> We create meters only if both log and meter is set for given
> ACL. Do not recompute if the ACL is created with only one
> of those. This should make it more efficient especially for
> ovn-kubernetes which always creates ACLs with meters set, but
> enables logging only when needed.
>
> Fixes: b2ac717fcec2 ("northd: Avoid recompute of lflow from ACLs without
> meters.")
> Reported-at: https://redhat.atlassian.net/browse/FDP-3976
> Signed-off-by: Ales Musil <[email protected]>
> ---
> northd/en-meters.c | 11 +++++++----
> tests/ovn-northd.at | 30 ++++++++++++++++++++++++------
> 2 files changed, 31 insertions(+), 10 deletions(-)
>
> diff --git a/northd/en-meters.c b/northd/en-meters.c
> index ef0da25c9..18db9a1fc 100644
> --- a/northd/en-meters.c
> +++ b/northd/en-meters.c
> @@ -81,10 +81,13 @@ sync_meters_nb_acl_handler(struct engine_node *node,
> void *data OVS_UNUSED)
>
> 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;
> + /* New or deleted ACL with meter and log needs to be recomputed.
> */
> + if (nbrec_acl_is_new(nb_acl) || nbrec_acl_is_deleted(nb_acl)) {
> + if (nb_acl->log && nb_acl->meter) {
> + return EN_UNHANDLED;
> + }
> +
> + continue;
> }
>
> /* Addition or removal of meter requires recompute. */
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 1e567704d..fd94b7c8d 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -20553,12 +20553,12 @@ check ovn-nbctl meter-add meter2 drop 20 kbps
> check ovn-nbctl --wait=sb sync
> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
>
> -AS_BOX([ACL with log])
> +AS_BOX([ACL with log only])
> 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_engine_stats lflow norecompute compute
> +check_engine_stats sync_meters norecompute compute
> CHECK_NO_CHANGE_AFTER_RECOMPUTE
> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
>
> @@ -20579,12 +20579,12 @@ 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_engine_stats lflow norecompute compute
> +check_engine_stats sync_meters norecompute compute
> CHECK_NO_CHANGE_AFTER_RECOMPUTE
> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
>
> -AS_BOX([ACL with meter])
> +AS_BOX([ACL with log and 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
> @@ -20614,6 +20614,24 @@ 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])
> +m1=$(fetch_column nb:Meter _uuid name=meter1)
> +check_uuid ovn-nbctl --wait=sb --id=@id create ACL action=drop \
> + direction=from-lport match=tcp priority=1001 meter=$m1 \
>
This might be pedantic but in the ACL table the meter column is the name of
the meter not the UUID, in this case, since log=false, the meter isn't
resolved, but it would be clearer if "meter=meter1" here
+ -- set logical_switch ls acls=@id
> +check_engine_stats northd norecompute compute
> +check_engine_stats lflow norecompute compute
> +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 acl-del ls
> +check_engine_stats northd norecompute compute
> +check_engine_stats lflow norecompute compute
> +check_engine_stats sync_meters norecompute compute
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +
> AS_BOX([ACLs attached to LS])
> check ovn-nbctl --wait=sb acl-add ls from-lport 100 tcp drop
> acl_id=$(fetch_column nb:Acl _uuid match=tcp action=drop)
> --
> 2.55.0
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
This is a good patch with just a minor change suggestion
Jacob
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
