diff mbox series

[ovs-dev] ovn-northd: Simplify iteration through ACLs.

Message ID 20210205195046.3759076-1-blp@ovn.org
State Accepted
Headers show
Series [ovs-dev] ovn-northd: Simplify iteration through ACLs. | expand

Commit Message

Ben Pfaff Feb. 5, 2021, 7:50 p.m. UTC
The existing code here seemed like an expensive way to find every ACL.
This simplifies it.

There is one behavioral change here: if an ACL is only referred to by
a Port_Group that itself has no references, then the old code will not
process it and the new code will.  I don't know whether that was why
the code was written the way it was.

Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 northd/ovn-northd.c | 27 ++++++---------------------
 1 file changed, 6 insertions(+), 21 deletions(-)

Comments

Dumitru Ceara Feb. 5, 2021, 10:50 p.m. UTC | #1
On 2/5/21 8:50 PM, Ben Pfaff wrote:
> The existing code here seemed like an expensive way to find every ACL.

It is :)

> This simplifies it.
> 
> There is one behavioral change here: if an ACL is only referred to by
> a Port_Group that itself has no references, then the old code will not
> process it and the new code will.  I don't know whether that was why
> the code was written the way it was.

It wasn't.. it was just a mistake.

> 
> Signed-off-by: Ben Pfaff <blp@ovn.org>
> ---

Acked-by: Dumitru Ceara <dceara@redhat.com>

Thanks,
Dumitru
Ben Pfaff Feb. 5, 2021, 11:18 p.m. UTC | #2
On Fri, Feb 05, 2021 at 11:50:21PM +0100, Dumitru Ceara wrote:
> On 2/5/21 8:50 PM, Ben Pfaff wrote:
> > The existing code here seemed like an expensive way to find every ACL.
> 
> It is :)
> 
> > This simplifies it.
> > 
> > There is one behavioral change here: if an ACL is only referred to by
> > a Port_Group that itself has no references, then the old code will not
> > process it and the new code will.  I don't know whether that was why
> > the code was written the way it was.
> 
> It wasn't.. it was just a mistake.
> 
> > 
> > Signed-off-by: Ben Pfaff <blp@ovn.org>
> > ---
> 
> Acked-by: Dumitru Ceara <dceara@redhat.com>

OK, cool, I updated the commit message and applied this to master.
diff mbox series

Patch

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 63966ef22a21..d6e921bbb2ac 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -12123,8 +12123,7 @@  sync_acl_fair_meter(struct northd_context *ctx, struct shash *meter_groups,
  * a private copy of its meter in the SB table.
  */
 static void
-sync_meters(struct northd_context *ctx, struct hmap *datapaths,
-            struct shash *meter_groups, struct hmap *port_groups)
+sync_meters(struct northd_context *ctx, struct shash *meter_groups)
 {
     struct shash sb_meters = SHASH_INITIALIZER(&sb_meters);
     struct sset used_sb_meters = SSET_INITIALIZER(&used_sb_meters);
@@ -12145,24 +12144,10 @@  sync_meters(struct northd_context *ctx, struct hmap *datapaths,
      * and see if additional rows are needed to get ACLs logs individually
      * rate-limited.
      */
-    struct ovn_datapath *od;
-    HMAP_FOR_EACH (od, key_node, datapaths) {
-        if (!od->nbs) {
-            continue;
-        }
-        for (size_t i = 0; i < od->nbs->n_acls; i++) {
-            sync_acl_fair_meter(ctx, meter_groups, od->nbs->acls[i],
-                                &sb_meters, &used_sb_meters);
-        }
-        struct ovn_port_group *pg;
-        HMAP_FOR_EACH (pg, key_node, port_groups) {
-            if (ovn_port_group_ls_find(pg, &od->nbs->header_.uuid)) {
-                for (size_t i = 0; i < pg->nb_pg->n_acls; i++) {
-                    sync_acl_fair_meter(ctx, meter_groups, pg->nb_pg->acls[i],
-                                        &sb_meters, &used_sb_meters);
-                }
-            }
-        }
+    const struct nbrec_acl *acl;
+    NBREC_ACL_FOR_EACH (acl, ctx->ovnnb_idl) {
+        sync_acl_fair_meter(ctx, meter_groups, acl,
+                            &sb_meters, &used_sb_meters);
     }
 
     const char *used_meter;
@@ -12655,7 +12640,7 @@  ovnnb_db_run(struct northd_context *ctx,
 
     sync_address_sets(ctx);
     sync_port_groups(ctx, &port_groups);
-    sync_meters(ctx, datapaths, &meter_groups, &port_groups);
+    sync_meters(ctx, &meter_groups);
     sync_dns_entries(ctx, datapaths);
 
     struct ovn_northd_lb *lb;