Skip to content

Commit a94d548

Browse files
dcearanumansiddique
authored andcommitted
northd: Avoid useless iterations when recomputing policies.
When recomputing router policies we can just mark everything that we already had as "potentially stale", build the new set of parsed policies and then remove what actually is stale in one go for all router datapaths. The old code was achieving the same thing but in a very inefficient way. It used to: - walk all routers, for each router R: - walk all parsed policies (for all routers) and mark as potentially stale the ones that correspond to R - reparse R's policies (making the ones that are still relevant "not stale") - remove all actually stale policies for R But that means O(N x M) iterations, where N is the number of routers and M is the number of router policies in the NB database. But it turns out we never had any stale policies because we always do a recompute of the route_policies node. So we can remove all the stale checking and looking up of parsed routes. This makes it that we can rebuild the parsed policies in a single pass, with a complexity of O(M). Some test numbers from a scaled setup, 1.6K routers with a total number of 4.8K policies: Before: node: route_policies, recompute (forced) took 254ms After: node: route_policies, recompute (forced) took 6ms Fixes: 15c9c9f ("northd: Add bfd, static_routes, route_policies and bfd_sync nodes to I-P engine.") Reported-at: https://redhat.atlassian.net/browse/FDP-3996 Assisted-by: Claude Opus 4.6, Claude Code Acked-by: Ales Musil <amusil@redhat.com> Signed-off-by: Dumitru Ceara <dceara@redhat.com> (cherry picked from commit 24ed825)
1 parent ca0dd14 commit a94d548

2 files changed

Lines changed: 0 additions & 20 deletions

File tree

northd/northd.c

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14356,12 +14356,6 @@ build_route_policies(struct ovn_datapath *od, const struct hmap *lr_ports,
1435614356
{
1435714357
struct route_policy *rp;
1435814358

14359-
HMAP_FOR_EACH (rp, key_node, route_policies) {
14360-
if (rp->nbr == od->nbr) {
14361-
rp->stale = true;
14362-
}
14363-
}
14364-
1436514359
/* Create chain numeric ids for policies with chain name set */
1436614360
for (int i = 0; i < od->nbr->n_policies; i++) {
1436714361
const struct nbrec_logical_router_policy *rule = od->nbr->policies[i];
@@ -14441,7 +14435,6 @@ build_route_policies(struct ovn_datapath *od, const struct hmap *lr_ports,
1444114435
new_rp->rule = rule;
1444214436
new_rp->n_valid_nexthops = n_valid_nexthops;
1444314437
new_rp->valid_nexthops = valid_nexthops;
14444-
new_rp->nbr = od->nbr;
1444514438
new_rp->chain_id = chain_id;
1444614439
new_rp->jump_chain_id = jump_chain_id;
1444714440

@@ -14450,21 +14443,10 @@ build_route_policies(struct ovn_datapath *od, const struct hmap *lr_ports,
1445014443
if (!rp) {
1445114444
hmap_insert(route_policies, &new_rp->key_node, hash);
1445214445
} else {
14453-
rp->stale = false;
1445414446
free(valid_nexthops);
1445514447
free(new_rp);
1445614448
}
1445714449
}
14458-
14459-
HMAP_FOR_EACH_SAFE (rp, key_node, route_policies) {
14460-
if (!rp->stale) {
14461-
continue;
14462-
}
14463-
14464-
hmap_remove(route_policies, &rp->key_node);
14465-
free(rp->valid_nexthops);
14466-
free(rp);
14467-
}
1446814450
}
1446914451

1447014452
/* Logical router ingress table POLICY: Policy.

northd/northd.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,8 +192,6 @@ struct route_policy {
192192
const struct nbrec_logical_router_policy *rule;
193193
size_t n_valid_nexthops;
194194
char **valid_nexthops;
195-
const struct nbrec_logical_router *nbr;
196-
bool stale;
197195
uint32_t chain_id;
198196
uint32_t jump_chain_id;
199197
};

0 commit comments

Comments
 (0)