On Mon, Oct 19, 2026 at 11:27 PM Mark Michelson via dev < [email protected]> wrote:
> en_route_policies was mostly defined in en-northd.h and en-northd.c. It > calls out to functions and uses structures that are defined in northd.h > and northd.c. In many cases, these functions were one-off uses that did > not need to be exported in a header file. > > To reduce the annoyingness factor, this commit moves en-route-policies > to its own file. Along with it, this commit moves certain functions out > of northd and into en-route-policies.c as static functions. > > There are no functionality changes in this commit. This only moves > functions around so they are better organized. > > Signed-off-by: Mark Michelson <[email protected]> > --- > northd/automake.mk | 2 + > northd/en-lflow.c | 1 + > northd/en-northd.c | 117 +---------- > northd/en-northd.h | 11 - > northd/en-route-policies.c | 408 +++++++++++++++++++++++++++++++++++++ > northd/en-route-policies.h | 56 +++++ > northd/inc-proc-northd.c | 1 + > northd/northd.c | 313 +--------------------------- > northd/northd.h | 48 +++-- > 9 files changed, 515 insertions(+), 442 deletions(-) > create mode 100644 northd/en-route-policies.c > create mode 100644 northd/en-route-policies.h > > diff --git a/northd/automake.mk b/northd/automake.mk > index 1d3044930..06c35276f 100644 > --- a/northd/automake.mk > +++ b/northd/automake.mk > @@ -34,6 +34,8 @@ northd_ovn_northd_SOURCES = \ > northd/en-northd-output.h \ > northd/en-port-group.c \ > northd/en-port-group.h \ > + northd/en-route-policies.c \ > + northd/en-route-policies.h \ > northd/en-sync-sb.c \ > northd/en-sync-sb.h \ > northd/en-sync-from-sb.c \ > diff --git a/northd/en-lflow.c b/northd/en-lflow.c > index a73d55486..9a186bb50 100644 > --- a/northd/en-lflow.c > +++ b/northd/en-lflow.c > @@ -29,6 +29,7 @@ > #include "en-sampling-app.h" > #include "en-group-ecmp-route.h" > #include "en-datapath-sync.h" > +#include "en-route-policies.h" > #include "lflow-mgr.h" > > #include "lib/inc-proc-eng.h" > diff --git a/northd/en-northd.c b/northd/en-northd.c > index 463383770..bff96ba0a 100644 > --- a/northd/en-northd.c > +++ b/northd/en-northd.c > @@ -1,4 +1,4 @@ > - /* > +/* > * Licensed under the Apache License, Version 2.0 (the "License"); > * you may not use this file except in compliance with the License. > * You may obtain a copy of the License at: > @@ -32,6 +32,7 @@ > #include "lib/util.h" > #include "openvswitch/vlog.h" > #include "en-datapath-logical-router.h" > +#include "en-route-policies.h" > > VLOG_DEFINE_THIS_MODULE(en_northd); > COVERAGE_DEFINE(northd_run); > @@ -278,94 +279,6 @@ northd_nb_port_group_handler(struct engine_node > *node, void *data) > return EN_HANDLED_UNCHANGED; > } > > -enum engine_input_handler_result > -route_policies_northd_change_handler(struct engine_node *node, > - void *data OVS_UNUSED) > -{ > - struct northd_data *northd_data = engine_get_input_data("northd", > node); > - if (!northd_has_tracked_data(&northd_data->trk_data)) { > - return EN_UNHANDLED; > - } > - > - /* This node uses the below data from the en_northd engine node. > - * See (lr_stateful_get_input_data()) > - * 1. northd_data->lr_datapaths > - * This data gets updated when a logical router or logical > router port > - * is created or deleted. > - * Northd engine node presently falls back to full recompute when > - * this happens and so does this node. > - * > - */ > - > - return EN_HANDLED_UNCHANGED; > -} > - > -enum engine_input_handler_result > -route_policies_datapath_synced_logical_router_handler(struct engine_node > *node, > - void *data > OVS_UNUSED) > -{ > - const struct ovn_synced_logical_router_map *synced_lrs = > - engine_get_input_data("datapath_synced_logical_router", node); > - > - if (hmapx_is_empty(&synced_lrs->new) && > - hmapx_is_empty(&synced_lrs->updated) && > - hmapx_is_empty(&synced_lrs->deleted)) { > - return EN_UNHANDLED; > - } > - > - struct hmapx_node *lr_node; > - HMAPX_FOR_EACH (lr_node, &synced_lrs->deleted) { > - const struct ovn_synced_logical_router *lr = lr_node->data; > - if (lr->nb->n_policies > 0) { > - return EN_UNHANDLED; > - } > - } > - > - HMAPX_FOR_EACH (lr_node, &synced_lrs->new) { > - const struct ovn_synced_logical_router *lr = lr_node->data; > - if (lr->nb->n_policies > 0) { > - return EN_UNHANDLED; > - } > - } > - > - HMAPX_FOR_EACH (lr_node, &synced_lrs->updated) { > - const struct ovn_synced_logical_router *lr = lr_node->data; > - if (nbrec_logical_router_is_updated( > - lr->nb, NBREC_LOGICAL_ROUTER_COL_POLICIES)) { > - return EN_UNHANDLED; > - } > - for (size_t i = 0; i < lr->nb->n_policies; i++) { > - if > (nbrec_logical_router_policy_row_get_seqno(lr->nb->policies[i], > - OVSDB_IDL_CHANGE_MODIFY) > 0) { > - return EN_UNHANDLED; > - } > - } > - } > - > - return EN_HANDLED_UNCHANGED; > -} > - > -enum engine_node_state > -en_route_policies_run(struct engine_node *node, void *data) > -{ > - struct northd_data *northd_data = engine_get_input_data("northd", > node); > - struct bfd_data *bfd_data = engine_get_input_data("bfd", node); > - struct route_policies_data *route_policies_data = data; > - > - route_policies_destroy(data); > - route_policies_init(data); > - > - struct ovn_datapath *od; > - HMAP_FOR_EACH (od, key_node, &northd_data->lr_datapaths.datapaths) { > - build_route_policies(od, &bfd_data->bfd_connections, > - &route_policies_data->route_policies, > - &route_policies_data->bfd_active_connections, > - &route_policies_data->chain_ids); > - } > - > - return EN_UPDATED; > -} > - > enum engine_input_handler_result > routes_northd_change_handler(struct engine_node *node, > void *data OVS_UNUSED) > @@ -552,6 +465,12 @@ en_routes_run(struct engine_node *node, void *data) > return EN_UPDATED; > } > > +static void > +destroy_bfd_data(struct bfd_data *data) > +{ > + bfd_destroy(&data->bfd_connections); > +} > + > enum engine_node_state > en_bfd_run(struct engine_node *node, void *data) > { > @@ -561,7 +480,7 @@ en_bfd_run(struct engine_node *node, void *data) > const struct sbrec_bfd_table *sbrec_bfd_table = > EN_OVSDB_GET(engine_get_input("SB_bfd", node)); > > - bfd_destroy(data); > + destroy_bfd_data(data); > bfd_init(data); > build_bfd_map(nbrec_bfd_table, sbrec_bfd_table, > &bfd_data->bfd_connections); > @@ -660,16 +579,6 @@ void > return data; > } > > -void > -*en_route_policies_init(struct engine_node *node OVS_UNUSED, > - struct engine_arg *arg OVS_UNUSED) > -{ > - struct route_policies_data *data = xzalloc(sizeof *data); > - > - route_policies_init(data); > - return data; > -} > - > void > *en_routes_init(struct engine_node *node OVS_UNUSED, > struct engine_arg *arg OVS_UNUSED) > @@ -756,12 +665,6 @@ northd_sb_fdb_change_handler(struct engine_node > *node, void *data) > return EN_HANDLED_UNCHANGED; > } > > -void > -en_route_policies_cleanup(void *data) > -{ > - route_policies_destroy(data); > -} > - > void > en_routes_cleanup(void *data) > { > @@ -777,7 +680,7 @@ en_routes_clear_tracked_data(void *data) > void > en_bfd_cleanup(void *data) > { > - bfd_destroy(data); > + destroy_bfd_data(data); > } > > void > diff --git a/northd/en-northd.h b/northd/en-northd.h > index 706b49e45..6fbf36749 100644 > --- a/northd/en-northd.h > +++ b/northd/en-northd.h > @@ -31,17 +31,6 @@ enum engine_input_handler_result > northd_sb_fdb_change_handler(struct engine_node *node, void *data); > void *en_routes_init(struct engine_node *node OVS_UNUSED, > struct engine_arg *arg OVS_UNUSED); > -void en_route_policies_cleanup(void *data); > -enum engine_input_handler_result > -route_policies_northd_change_handler(struct engine_node *node, > - void *data OVS_UNUSED); > -enum engine_input_handler_result > -route_policies_datapath_synced_logical_router_handler(struct engine_node > *node, > - void *data > OVS_UNUSED); > -enum engine_node_state en_route_policies_run(struct engine_node *node, > - void *data); > -void *en_route_policies_init(struct engine_node *node OVS_UNUSED, > - struct engine_arg *arg OVS_UNUSED); > void en_routes_clear_tracked_data(void *data); > void en_routes_cleanup(void *data); > enum engine_input_handler_result > diff --git a/northd/en-route-policies.c b/northd/en-route-policies.c > new file mode 100644 > index 000000000..dd6d182e8 > --- /dev/null > +++ b/northd/en-route-policies.c > @@ -0,0 +1,408 @@ > +/* > + * Copyright (c) 2026, Red Hat, Inc. > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at: > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or > implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > +#include <config.h> > + > +#include "en-datapath-logical-router.h" > +#include "en-route-policies.h" > +#include "northd.h" > +#include "ovn-nb-idl.h" > + > +#include "openvswitch/vlog.h" > + > +VLOG_DEFINE_THIS_MODULE(en_route_policies); > + > +static struct route_policy * > +route_policies_lookup(struct hmap *route_policies, size_t hash, > + struct route_policy *new_rp) > +{ > + struct route_policy *rp; > + HMAP_FOR_EACH_WITH_HASH (rp, key_node, hash, route_policies) { > + if (rp->rule != new_rp->rule) { > + continue; > + } > + > + if (rp->chain_id != new_rp->chain_id) { > + continue; > + } > + > + if (rp->jump_chain_id != new_rp->jump_chain_id) { > + continue; > + } > + > + if (rp->n_valid_nexthops != new_rp->n_valid_nexthops) { > + continue; > + } > + > + size_t i; > + for (i = 0; i < new_rp->n_valid_nexthops; i++) { > + size_t j; > + > + for (j = 0; j < rp->n_valid_nexthops; j++) { > + if (!strcmp(new_rp->valid_nexthops[i], > + rp->valid_nexthops[j])) { > + break; > + } > + } > + > + if (j == rp->n_valid_nexthops) { > + break; > + } > + } > + > + if (i == new_rp->n_valid_nexthops) { > + return rp; > + } > + } > + > + return NULL; > +} > + > +static bool > +policy_chain_id(struct simap *chain_ids, const char *chain_name, uint32_t > *id) > +{ > + if (chain_name && *chain_name) { > + *id = simap_get(chain_ids, chain_name); > + return true; > + } > + > + return false; > +} > + > +static void > +policy_chain_add(struct simap *chain_ids, const char *chain_name) > +{ > + uint32_t id = simap_count(chain_ids) + 1; > + > + if (id == UINT16_MAX) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_WARN_RL(&rl, "Too many policy chains for Logical Router."); > + return; > + } > + > + if (!simap_put(chain_ids, chain_name, id)) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_WARN_RL(&rl, "Policy chain id unexpectedly appeared"); > + } > +} > + > +static bool > +check_bfd_state(const struct nbrec_logical_router_policy *rule, > + struct ovn_port *out_port, const char *nexthop, > + const struct hmap *bfd_connections, > + struct hmap *bfd_active_connections) > +{ > + struct in6_addr nexthop_v6; > + bool is_nexthop_v6 = ipv6_parse(nexthop, &nexthop_v6); > + > + for (size_t i = 0; i < rule->n_bfd_sessions; i++) { > + /* Check if there is a BFD session associated to the reroute > + * policy. */ > + const struct nbrec_bfd *nb_bt = rule->bfd_sessions[i]; > + struct in6_addr dst_ipv6; > + bool is_dst_v6 = ipv6_parse(nb_bt->dst_ip, &dst_ipv6); > + > + if (is_nexthop_v6 ^ is_dst_v6) { > + continue; > + } > + > + if ((is_nexthop_v6 && !ipv6_addr_equals(&nexthop_v6, &dst_ipv6)) > || > + strcmp(nb_bt->dst_ip, nexthop)) { > + continue; > + } > + > + if (strcmp(nb_bt->logical_port, out_port->key)) { > + continue; > + } > + > + struct bfd_entry *bfd_e = bfd_port_lookup(bfd_connections, > + nb_bt->logical_port, > + nb_bt->dst_ip); > + if (!bfd_e) { > + continue; > + } > + > + /* This route policy is linked to an active bfd session. */ > + struct bfd_entry *bfd_rp = bfd_port_lookup(bfd_active_connections, > + nb_bt->logical_port, > + nb_bt->dst_ip); > + if (!bfd_rp) { > + bfd_rp = bfd_alloc_entry(bfd_active_connections, > + nb_bt->logical_port, nb_bt->dst_ip, > + bfd_e->status); > + } > + > + if (!strcmp(bfd_e->status, "admin_down")) { > + bfd_set_status(bfd_rp, "down"); > + } > + > + return strcmp(bfd_rp->status, "down"); > + } > + > + return true; > +} > + > +static void > +build_route_policies(struct ovn_datapath *od, > + const struct hmap *bfd_connections, > + struct hmap *route_policies, > + struct hmap *bfd_active_connections, > + struct simap *chain_ids) > +{ > + struct route_policy *rp; > + > + /* 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]; > + uint32_t id; > + > + if (policy_chain_id(chain_ids, rule->chain, &id) && id == 0) { > + policy_chain_add(chain_ids, rule->chain); > + } > + } > + > + for (int i = 0; i < od->nbr->n_policies; i++) { > + const struct nbrec_logical_router_policy *rule = > od->nbr->policies[i]; > + > + if (rule->nexthop && rule->nexthop[0]) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > + VLOG_WARN_RL(&rl, "Logical router: %s, policy uses deprecated" > + " column \"nexthop\", this column is ignored. > Please" > + "use \"nexthops\" column instead.", > od->nbr->name); > + continue; > + } > + > + size_t n_valid_nexthops = 0; > + char **valid_nexthops = NULL; > + uint32_t chain_id = 0; > + uint32_t jump_chain_id = 0; > + > + /* Skip policy if chain name is set but id was not created above > */ > + if (policy_chain_id(chain_ids, rule->chain, &chain_id) > + && chain_id == 0) { > + continue; > + } > + > + if (!strcmp(rule->action, "jump")) { > + /* Skip policy if action is 'jump' but no target chain is set > */ > + if (!policy_chain_id(chain_ids, rule->jump_chain, > + &jump_chain_id)) { > + static struct vlog_rate_limit rl = > VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_WARN_RL(&rl, > + "Logical router: %s, policy action 'jump'" > + " has empty target", > + od->nbr->name); > + continue; > + } > + > + /* Skip policy if action is 'jump' but target chain name > + is not resolved to numeric id */ > + if (jump_chain_id == 0) { > + static struct vlog_rate_limit rl = > VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_WARN_RL(&rl, > + "Logical router: %s, policy action 'jump'" > + " follows to non-existent chain %s", > + od->nbr->name, rule->jump_chain); > + continue; > + } > + } > + > + if (simap_is_empty(chain_ids)) { > + chain_id = -1; > + } > + > + if (!strcmp(rule->action, "reroute")) { > + if (rule->output_port && rule->n_nexthops != 1) { > + static struct vlog_rate_limit rl = > VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_WARN_RL(&rl, > + "Logical router: %s, policy " > + "(chain: '%s', match: '%s', priority > %"PRId64"): " > + "output_port only supported on non-ECMP " > + "reroute policies", > + od->nbr->name, > + rule->chain ? rule->chain : "<Default>", > + rule->match, rule->priority); > + continue; > + } > + > + valid_nexthops = xcalloc(rule->n_nexthops, sizeof > *valid_nexthops); > + for (size_t j = 0; j < rule->n_nexthops; j++) { > + char *nexthop = rule->nexthops[j]; > + if (!nexthop || !nexthop[0]) { > + continue; > + } > + > + struct ovn_port *out_port = NULL; > + bool is_ipv4 = strchr(nexthop, '.') ? true : false; > + > + if (!find_policy_outport(od, rule, nexthop, is_ipv4, NULL, > + &out_port)) { > + continue; > + } > + if (!check_bfd_state(rule, out_port, nexthop, > + bfd_connections, > + bfd_active_connections)) { > + continue; > + } > + valid_nexthops[n_valid_nexthops++] = nexthop; > + } > + > + if (!n_valid_nexthops) { > + free(valid_nexthops); > + continue; > + } > + } > + > + struct route_policy *new_rp = xzalloc(sizeof *new_rp); > + new_rp->rule = rule; > + new_rp->n_valid_nexthops = n_valid_nexthops; > + new_rp->valid_nexthops = valid_nexthops; > + new_rp->chain_id = chain_id; > + new_rp->jump_chain_id = jump_chain_id; > + > + size_t hash = uuid_hash(&od->key); > + rp = route_policies_lookup(route_policies, hash, new_rp); > + if (!rp) { > + hmap_insert(route_policies, &new_rp->key_node, hash); > + } else { > + free(valid_nexthops); > + free(new_rp); > + } > + } > +} > + > +static void > +route_policies_init(struct route_policies_data *data) > +{ > + hmap_init(&data->route_policies); > + hmap_init(&data->bfd_active_connections); > + simap_init(&data->chain_ids); > +} > + > +static void > +route_policies_destroy(struct route_policies_data *data) > +{ > + struct route_policy *rp; > + HMAP_FOR_EACH_POP (rp, key_node, &data->route_policies) { > + free(rp->valid_nexthops); > + free(rp); > + }; > + hmap_destroy(&data->route_policies); > + bfd_destroy(&data->bfd_active_connections); > + simap_destroy(&data->chain_ids); > +} > + > +enum engine_node_state > +en_route_policies_run(struct engine_node *node, void *data) > +{ > + struct northd_data *northd_data = engine_get_input_data("northd", > node); > + struct bfd_data *bfd_data = engine_get_input_data("bfd", node); > + struct route_policies_data *route_policies_data = data; > + > + route_policies_destroy(data); > + route_policies_init(data); > + > + struct ovn_datapath *od; > + HMAP_FOR_EACH (od, key_node, &northd_data->lr_datapaths.datapaths) { > + build_route_policies(od, &bfd_data->bfd_connections, > + &route_policies_data->route_policies, > + &route_policies_data->bfd_active_connections, > + &route_policies_data->chain_ids); > + } > + > + return EN_UPDATED; > +} > + > +void > +*en_route_policies_init(struct engine_node *node OVS_UNUSED, > + struct engine_arg *arg OVS_UNUSED) > +{ > + struct route_policies_data *data = xzalloc(sizeof *data); > + > + route_policies_init(data); > + return data; > +} > + > +void > +en_route_policies_cleanup(void *data) > +{ > + route_policies_destroy(data); > +} > + > +enum engine_input_handler_result > +route_policies_northd_change_handler(struct engine_node *node, > + void *data OVS_UNUSED) > +{ > + struct northd_data *northd_data = engine_get_input_data("northd", > node); > + if (!northd_has_tracked_data(&northd_data->trk_data)) { > + return EN_UNHANDLED; > + } > + > + /* This node uses the below data from the en_northd engine node. > + * See (lr_stateful_get_input_data()) > + * 1. northd_data->lr_datapaths > + * This data gets updated when a logical router or logical > router port > + * is created or deleted. > + * Northd engine node presently falls back to full recompute when > + * this happens and so does this node. > + */ > + > + return EN_HANDLED_UNCHANGED; > +} > + > +enum engine_input_handler_result > +route_policies_datapath_synced_logical_router_handler(struct engine_node > *node, > + void *data > OVS_UNUSED) > +{ > + const struct ovn_synced_logical_router_map *synced_lrs = > + engine_get_input_data("datapath_synced_logical_router", node); > + > + if (hmapx_is_empty(&synced_lrs->new) && > + hmapx_is_empty(&synced_lrs->updated) && > + hmapx_is_empty(&synced_lrs->deleted)) { > + return EN_UNHANDLED; > + } > + > + struct hmapx_node *lr_node; > + HMAPX_FOR_EACH (lr_node, &synced_lrs->deleted) { > + const struct ovn_synced_logical_router *lr = lr_node->data; > + if (lr->nb->n_policies > 0) { > + return EN_UNHANDLED; > + } > + } > + > + HMAPX_FOR_EACH (lr_node, &synced_lrs->new) { > + const struct ovn_synced_logical_router *lr = lr_node->data; > + if (lr->nb->n_policies > 0) { > + return EN_UNHANDLED; > + } > + } > + > + HMAPX_FOR_EACH (lr_node, &synced_lrs->updated) { > + const struct ovn_synced_logical_router *lr = lr_node->data; > + if (nbrec_logical_router_is_updated( > + lr->nb, NBREC_LOGICAL_ROUTER_COL_POLICIES)) { > + return EN_UNHANDLED; > + } > + for (size_t i = 0; i < lr->nb->n_policies; i++) { > + if > (nbrec_logical_router_policy_row_get_seqno(lr->nb->policies[i], > + OVSDB_IDL_CHANGE_MODIFY) > 0) { > + return EN_UNHANDLED; > + } > + } > + } > + > + return EN_HANDLED_UNCHANGED; > +} > diff --git a/northd/en-route-policies.h b/northd/en-route-policies.h > new file mode 100644 > index 000000000..1d2288e46 > --- /dev/null > +++ b/northd/en-route-policies.h > @@ -0,0 +1,56 @@ > +/* > + * Copyright (c) 2026, Red Hat, Inc. > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at: > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or > implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > +#ifndef EN_ROUTE_POLICIES_H > +#define EN_ROUTE_POLICIES_H > + > +#include "inc-proc-eng.h" > + > +#include "lib/simap.h" > +#include "openvswitch/hmap.h" > + > +/* Represents the data associated with an instance of a northbound > + * Logical Router Policy for a particular Logical Router. > + */ > +struct route_policy { > + struct hmap_node key_node; > + const struct nbrec_logical_router_policy *rule; > + size_t n_valid_nexthops; > + char **valid_nexthops; > + uint32_t chain_id; > + uint32_t jump_chain_id; > +}; > + > +/* Global route policy data exported by en-route-policies. */ > +struct route_policies_data { > + struct hmap route_policies; > + struct hmap bfd_active_connections; > + struct simap chain_ids; > +}; > + > +void en_route_policies_cleanup(void *data); > +enum engine_input_handler_result > +route_policies_northd_change_handler(struct engine_node *node, > + void *data OVS_UNUSED); > +enum engine_input_handler_result > +route_policies_datapath_synced_logical_router_handler(struct engine_node > *node, > + void *data > OVS_UNUSED); > +enum engine_node_state en_route_policies_run(struct engine_node *node, > + void *data); > +void *en_route_policies_init(struct engine_node *node OVS_UNUSED, > + struct engine_arg *arg OVS_UNUSED); > + > +#endif /* EN_ROUTE_POLICIES_H */ > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > index e28cd9d97..84c40fdd2 100644 > --- a/northd/inc-proc-northd.c > +++ b/northd/inc-proc-northd.c > @@ -51,6 +51,7 @@ > #include "en-datapath-logical-router.h" > #include "en-datapath-logical-switch.h" > #include "en-datapath-sync.h" > +#include "en-route-policies.h" > #include "unixctl.h" > #include "util.h" > > diff --git a/northd/northd.c b/northd/northd.c > index 6fe12641d..eaf8f0b8f 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -54,6 +54,7 @@ > #include "en-sampling-app.h" > #include "en-datapath-logical-switch.h" > #include "en-datapath-logical-router.h" > +#include "en-route-policies.h" > #include "lib/ovn-parallel-hmap.h" > #include "ovn/actions.h" > #include "ovn/features.h" > @@ -12052,19 +12053,7 @@ build_lswitch_ip_unicast_lookup_for_nats( > } > } > > -struct bfd_entry { > - struct hmap_node hmap_node; > - > - const struct nbrec_bfd *nb_bt; > - const struct sbrec_bfd *sb_bt; > - > - char *logical_port; > - char *dst_ip; > - char *status; > - bool stale; > -}; > - > -static struct bfd_entry * > +struct bfd_entry * > bfd_alloc_entry(struct hmap *bfd_connections, > const char *logical_port, const char *dst_ip, > const char *status) > @@ -12080,7 +12069,7 @@ bfd_alloc_entry(struct hmap *bfd_connections, > return bfd_e; > } > > -static void > +void > bfd_erase_entry(struct bfd_entry *bfd_e) > { > free(bfd_e->logical_port); > @@ -12089,14 +12078,14 @@ bfd_erase_entry(struct bfd_entry *bfd_e) > free(bfd_e); > } > > -static void > +void > bfd_set_status(struct bfd_entry *bfd_e, const char *status) > { > free(bfd_e->status); > bfd_e->status = xstrdup(status); > } > > -static struct bfd_entry * > +struct bfd_entry * > bfd_port_lookup(const struct hmap *bfd_map, const char *logical_port, > const char *dst_ip) > { > @@ -12377,7 +12366,7 @@ lrp_find_member_ip(const struct ovn_port *op, > const char *ip_s) > * this policy could be determined. Stores a pointer to the output port > * in 'p_output_port' and a pointer to the router IP address to be used > for > * this policy, in 'p_lrp_addr_s'. */ > -static bool > +bool > find_policy_outport(struct ovn_datapath *od, > const struct nbrec_logical_router_policy *policy, > const char *nexthop, bool is_ipv4, > @@ -12428,61 +12417,6 @@ find_policy_outport(struct ovn_datapath *od, > return true; > } > > -static bool check_bfd_state(const struct nbrec_logical_router_policy > *rule, > - struct ovn_port *out_port, const char > *nexthop, > - const struct hmap *bfd_connections, > - struct hmap *bfd_active_connections) > -{ > - struct in6_addr nexthop_v6; > - bool is_nexthop_v6 = ipv6_parse(nexthop, &nexthop_v6); > - > - for (size_t i = 0; i < rule->n_bfd_sessions; i++) { > - /* Check if there is a BFD session associated to the reroute > - * policy. */ > - const struct nbrec_bfd *nb_bt = rule->bfd_sessions[i]; > - struct in6_addr dst_ipv6; > - bool is_dst_v6 = ipv6_parse(nb_bt->dst_ip, &dst_ipv6); > - > - if (is_nexthop_v6 ^ is_dst_v6) { > - continue; > - } > - > - if ((is_nexthop_v6 && !ipv6_addr_equals(&nexthop_v6, &dst_ipv6)) > || > - strcmp(nb_bt->dst_ip, nexthop)) { > - continue; > - } > - > - if (strcmp(nb_bt->logical_port, out_port->key)) { > - continue; > - } > - > - struct bfd_entry *bfd_e = bfd_port_lookup(bfd_connections, > - nb_bt->logical_port, > - nb_bt->dst_ip); > - if (!bfd_e) { > - continue; > - } > - > - /* This route policy is linked to an active bfd session. */ > - struct bfd_entry *bfd_rp = bfd_port_lookup(bfd_active_connections, > - nb_bt->logical_port, > - nb_bt->dst_ip); > - if (!bfd_rp) { > - bfd_rp = bfd_alloc_entry(bfd_active_connections, > - nb_bt->logical_port, nb_bt->dst_ip, > - bfd_e->status); > - } > - > - if (!strcmp(bfd_e->status, "admin_down")) { > - bfd_set_status(bfd_rp, "down"); > - } > - > - return strcmp(bfd_rp->status, "down"); > - } > - > - return true; > -} > - > static void > build_routing_policy_flow(struct lflow_table *lflows, struct ovn_datapath > *od, > struct route_policy *rp, > @@ -15968,208 +15902,6 @@ build_mcast_lookup_flows_for_lrouter(struct > ovn_datapath *od, > } > } > > -static struct route_policy * > -route_policies_lookup(struct hmap *route_policies, size_t hash, > - struct route_policy *new_rp) > -{ > - struct route_policy *rp; > - HMAP_FOR_EACH_WITH_HASH (rp, key_node, hash, route_policies) { > - if (rp->rule != new_rp->rule) { > - continue; > - } > - > - if (rp->chain_id != new_rp->chain_id) { > - continue; > - } > - > - if (rp->jump_chain_id != new_rp->jump_chain_id) { > - continue; > - } > - > - if (rp->n_valid_nexthops != new_rp->n_valid_nexthops) { > - continue; > - } > - > - size_t i; > - for (i = 0; i < new_rp->n_valid_nexthops; i++) { > - size_t j; > - > - for (j = 0; j < rp->n_valid_nexthops; j++) { > - if (!strcmp(new_rp->valid_nexthops[i], > - rp->valid_nexthops[j])) { > - break; > - } > - } > - > - if (j == rp->n_valid_nexthops) { > - break; > - } > - } > - > - if (i == new_rp->n_valid_nexthops) { > - return rp; > - } > - } > - > - return NULL; > -} > - > -static bool > -policy_chain_id(struct simap *chain_ids, const char *chain_name, uint32_t > *id) > -{ > - if (chain_name && *chain_name) { > - *id = simap_get(chain_ids, chain_name); > - return true; > - } > - > - return false; > -} > - > -static void > -policy_chain_add(struct simap *chain_ids, const char *chain_name) > -{ > - uint32_t id = simap_count(chain_ids) + 1; > - > - if (id == UINT16_MAX) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > - VLOG_WARN_RL(&rl, "Too many policy chains for Logical Router."); > - return; > - } > - > - if (!simap_put(chain_ids, chain_name, id)) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > - VLOG_WARN_RL(&rl, "Policy chain id unexpectedly appeared"); > - } > -} > - > -void > -build_route_policies(struct ovn_datapath *od, > - const struct hmap *bfd_connections, > - struct hmap *route_policies, > - struct hmap *bfd_active_connections, > - struct simap *chain_ids) > -{ > - struct route_policy *rp; > - > - /* 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]; > - uint32_t id; > - > - if (policy_chain_id(chain_ids, rule->chain, &id) && id == 0) { > - policy_chain_add(chain_ids, rule->chain); > - } > - } > - > - for (int i = 0; i < od->nbr->n_policies; i++) { > - const struct nbrec_logical_router_policy *rule = > od->nbr->policies[i]; > - > - if (rule->nexthop && rule->nexthop[0]) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > - VLOG_WARN_RL(&rl, "Logical router: %s, policy uses deprecated" > - " column \"nexthop\", this column is ignored. > Please" > - "use \"nexthops\" column instead.", > od->nbr->name); > - continue; > - } > - > - size_t n_valid_nexthops = 0; > - char **valid_nexthops = NULL; > - uint32_t chain_id = 0; > - uint32_t jump_chain_id = 0; > - > - /* Skip policy if chain name is set but id was not created above > */ > - if (policy_chain_id(chain_ids, rule->chain, &chain_id) > - && chain_id == 0) { > - continue; > - } > - > - if (!strcmp(rule->action, "jump")) { > - /* Skip policy if action is 'jump' but no target chain is set > */ > - if (!policy_chain_id(chain_ids, rule->jump_chain, > - &jump_chain_id)) { > - static struct vlog_rate_limit rl = > VLOG_RATE_LIMIT_INIT(5, 1); > - VLOG_WARN_RL(&rl, > - "Logical router: %s, policy action 'jump'" > - " has empty target", > - od->nbr->name); > - continue; > - } > - > - /* Skip policy if action is 'jump' but target chain name > - is not resolved to numeric id */ > - if (jump_chain_id == 0) { > - static struct vlog_rate_limit rl = > VLOG_RATE_LIMIT_INIT(5, 1); > - VLOG_WARN_RL(&rl, > - "Logical router: %s, policy action 'jump'" > - " follows to non-existent chain %s", > - od->nbr->name, rule->jump_chain); > - continue; > - } > - } > - > - if (simap_is_empty(chain_ids)) { > - chain_id = -1; > - } > - > - if (!strcmp(rule->action, "reroute")) { > - if (rule->output_port && rule->n_nexthops != 1) { > - static struct vlog_rate_limit rl = > VLOG_RATE_LIMIT_INIT(5, 1); > - VLOG_WARN_RL(&rl, > - "Logical router: %s, policy " > - "(chain: '%s', match: '%s', priority > %"PRId64"): " > - "output_port only supported on non-ECMP " > - "reroute policies", > - od->nbr->name, > - rule->chain ? rule->chain : "<Default>", > - rule->match, rule->priority); > - continue; > - } > - > - valid_nexthops = xcalloc(rule->n_nexthops, sizeof > *valid_nexthops); > - for (size_t j = 0; j < rule->n_nexthops; j++) { > - char *nexthop = rule->nexthops[j]; > - if (!nexthop || !nexthop[0]) { > - continue; > - } > - > - struct ovn_port *out_port = NULL; > - bool is_ipv4 = strchr(nexthop, '.') ? true : false; > - > - if (!find_policy_outport(od, rule, nexthop, is_ipv4, NULL, > - &out_port)) { > - continue; > - } > - if (!check_bfd_state(rule, out_port, nexthop, > - bfd_connections, > - bfd_active_connections)) { > - continue; > - } > - valid_nexthops[n_valid_nexthops++] = nexthop; > - } > - > - if (!n_valid_nexthops) { > - free(valid_nexthops); > - continue; > - } > - } > - > - struct route_policy *new_rp = xzalloc(sizeof *new_rp); > - new_rp->rule = rule; > - new_rp->n_valid_nexthops = n_valid_nexthops; > - new_rp->valid_nexthops = valid_nexthops; > - new_rp->chain_id = chain_id; > - new_rp->jump_chain_id = jump_chain_id; > - > - size_t hash = uuid_hash(&od->key); > - rp = route_policies_lookup(route_policies, hash, new_rp); > - if (!rp) { > - hmap_insert(route_policies, &new_rp->key_node, hash); > - } else { > - free(valid_nexthops); > - free(new_rp); > - } > - } > -} > > /* Logical router ingress table POLICY: Policy. > * > @@ -21995,14 +21727,6 @@ northd_init(struct northd_data *data) > init_northd_tracked_data(data); > } > > -void > -route_policies_init(struct route_policies_data *data) > -{ > - hmap_init(&data->route_policies); > - hmap_init(&data->bfd_active_connections); > - simap_init(&data->chain_ids); > -} > - > void > routes_init(struct routes_data *data) > { > @@ -22084,8 +21808,8 @@ northd_destroy(struct northd_data *data) > destroy_northd_tracked_data(data); > } > > -static void > -__bfd_destroy(struct hmap *bfd_connections) > +void > +bfd_destroy(struct hmap *bfd_connections) > { > struct bfd_entry *bfd_e; > > @@ -22095,12 +21819,6 @@ __bfd_destroy(struct hmap *bfd_connections) > hmap_destroy(bfd_connections); > } > > -void > -bfd_destroy(struct bfd_data *data) > -{ > - __bfd_destroy(&data->bfd_connections); > -} > - > static void > __ic_learned_svcs_cleanup(struct hmap *ic_learned_svc_monitors_map) > { > @@ -22118,19 +21836,6 @@ ic_learned_svc_monitors_cleanup(struct > ic_learned_svc_monitors_data *data) > lflow_ref_destroy(data->lflow_ref); > } > > -void > -route_policies_destroy(struct route_policies_data *data) > -{ > - struct route_policy *rp; > - HMAP_FOR_EACH_POP (rp, key_node, &data->route_policies) { > - free(rp->valid_nexthops); > - free(rp); > - }; > - hmap_destroy(&data->route_policies); > - __bfd_destroy(&data->bfd_active_connections); > - simap_destroy(&data->chain_ids); > -} > - > void > routes_destroy(struct routes_data *data) > { > @@ -22141,7 +21846,7 @@ routes_destroy(struct routes_data *data) > hmap_destroy(&data->parsed_routes); > > simap_destroy(&data->route_tables); > - __bfd_destroy(&data->bfd_active_connections); > + bfd_destroy(&data->bfd_active_connections); > hmapx_destroy(&data->trk_data.trk_crupdated_parsed_route); > hmapx_destroy(&data->trk_data.trk_deleted_parsed_route); > } > diff --git a/northd/northd.h b/northd/northd.h > index 4700407bd..f6f6822ad 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -210,15 +210,6 @@ struct northd_data { > struct northd_tracked_data trk_data; > }; > > -struct route_policy { > - struct hmap_node key_node; > - const struct nbrec_logical_router_policy *rule; > - size_t n_valid_nexthops; > - char **valid_nexthops; > - uint32_t chain_id; > - uint32_t jump_chain_id; > -}; > - > struct route_tracked_data { > struct hmapx trk_crupdated_parsed_route; > struct hmapx trk_deleted_parsed_route; > @@ -232,12 +223,6 @@ struct routes_data { > bool tracked; > }; > > -struct route_policies_data { > - struct hmap route_policies; > - struct hmap bfd_active_connections; > - struct simap chain_ids; > -}; > - > struct bfd_data { > struct hmap bfd_connections; > }; > @@ -948,8 +933,6 @@ void northd_init(struct northd_data *data); > void northd_indices_create(struct northd_data *data, > struct ovsdb_idl *ovnsb_idl); > > -void route_policies_init(struct route_policies_data *); > -void route_policies_destroy(struct route_policies_data *); > void build_parsed_routes(const struct ovn_datapath *, const struct hmap *, > struct hmap *, struct simap *, struct hmap *); > uint32_t get_route_table_id(struct simap *, const char *); > @@ -957,8 +940,29 @@ void routes_init(struct routes_data *); > void routes_destroy(struct routes_data *); > void routes_clear_tracked(struct routes_data *); > > +struct bfd_entry { > + struct hmap_node hmap_node; > + > + const struct nbrec_bfd *nb_bt; > + const struct sbrec_bfd *sb_bt; > + > + char *logical_port; > + char *dst_ip; > + char *status; > + bool stale; > +}; > + > +struct bfd_entry *bfd_alloc_entry(struct hmap *bfd_connections, > + const char *logical_port, const char > *dst_ip, > + const char *status); > +void bfd_erase_entry(struct bfd_entry *bfd_e); > +void bfd_set_status(struct bfd_entry *bfd_e, const char *status); > +struct bfd_entry *bfd_port_lookup(const struct hmap *bfd_map, > + const char *logical_port, > + const char *dst_ip); > +void bfd_destroy(struct hmap *bfd_connections); > + > void bfd_init(struct bfd_data *); > -void bfd_destroy(struct bfd_data *); > > void bfd_sync_init(struct bfd_sync_data *); > void bfd_sync_swap(struct bfd_sync_data *, struct sset *bfd_ports); > @@ -1015,8 +1019,12 @@ bool northd_handle_lb_data_changes(struct > tracked_lb_data *, > const struct hmap *lr_lb_map, > struct northd_tracked_data *); > > -void build_route_policies(struct ovn_datapath *, const struct hmap *, > - struct hmap *, struct hmap *, struct simap *); > +bool find_policy_outport(struct ovn_datapath *od, > + const struct nbrec_logical_router_policy *policy, > + const char *nexthop, bool is_ipv4, > + const char **p_lrp_addr_s, > + struct ovn_port **p_out_port); > + > void bfd_table_sync(struct ovsdb_idl_txn *, const struct nbrec_bfd_table > *, > const struct hmap *, const struct hmap *, > const struct hmap *, const struct hmap *, > -- > 2.55.0 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Looks good to me, thanks. Acked-by: Ales Musil <[email protected]> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
