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: 15c9c9f42ad8 ("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
Signed-off-by: Dumitru Ceara <[email protected]>
---
northd/northd.c | 18 ------------------
northd/northd.h | 2 --
2 files changed, 20 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index 6c93208a84..53b433007f 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -15425,12 +15425,6 @@ build_route_policies(struct ovn_datapath *od, const
struct hmap *lr_ports,
{
struct route_policy *rp;
- HMAP_FOR_EACH (rp, key_node, route_policies) {
- if (rp->nbr == od->nbr) {
- rp->stale = true;
- }
- }
-
/* Create chain numeric ids for policies with chain name set */
for (int i = 0; i < od->nbr->n_policies; i++) {
const struct nbrec_logical_router_policy *rule = od->nbr->policies[i];
@@ -15537,7 +15531,6 @@ build_route_policies(struct ovn_datapath *od, const
struct hmap *lr_ports,
new_rp->rule = rule;
new_rp->n_valid_nexthops = n_valid_nexthops;
new_rp->valid_nexthops = valid_nexthops;
- new_rp->nbr = od->nbr;
new_rp->chain_id = chain_id;
new_rp->jump_chain_id = jump_chain_id;
@@ -15546,21 +15539,10 @@ build_route_policies(struct ovn_datapath *od, const
struct hmap *lr_ports,
if (!rp) {
hmap_insert(route_policies, &new_rp->key_node, hash);
} else {
- rp->stale = false;
free(valid_nexthops);
free(new_rp);
}
}
-
- HMAP_FOR_EACH_SAFE (rp, key_node, route_policies) {
- if (!rp->stale) {
- continue;
- }
-
- hmap_remove(route_policies, &rp->key_node);
- free(rp->valid_nexthops);
- free(rp);
- }
}
/* Logical router ingress table POLICY: Policy.
diff --git a/northd/northd.h b/northd/northd.h
index ede9e81bc6..bb86d3dac2 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -210,8 +210,6 @@ struct route_policy {
const struct nbrec_logical_router_policy *rule;
size_t n_valid_nexthops;
char **valid_nexthops;
- const struct nbrec_logical_router *nbr;
- bool stale;
uint32_t chain_id;
uint32_t jump_chain_id;
};
--
2.54.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
