Message ID | 7422f898b852d8377755f806bcd45b470318bc39.1722356842.git.lorenzo.bianconi@redhat.com |
---|---|
State | Superseded |
Headers | show |
Series | [ovs-dev,v9] northd: Add bfd, static_routes, route_policies and bfd_sync nodes to I-P engine. | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | warning | apply and check: warning |
ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Tue, Jul 30, 2024 at 9:34 AM Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote: > > Introduce bfd, static_routes, route_policies and bfd_sync nodes to northd I-P > engine to track bfd connections and northd static_route/policy_route > changes. > > Reported-at: https://issues.redhat.com/browse/FDP-600 > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > Changes since v8: > - rebase on top of ovn main branch and fix conflicts > Changes since v7: > - rework node dependencies and move all NB-BFD table update in bfd_sync node > - fix some more comments > Changes since v6: > - remove unnecessary node inputs > - fix comment > Changes since v5: > - remove bfd_mutex > Changes since v4: > - introduce bfd_nb_sync node and ger rid of bfd_consumer routine > Changes since v3: > - fix bfd_northd_change_handler logic > - fix route_policies_northd_change_handler logic > - fix static_routes_northd_change_handler logic > - add unit-tests > - cosmetics > Changes since v2: > - add incremental processing routines > - split bfd_consumer node in static_routes and route_policies nodes > Changes since v1: > - add incremental processing logic for bfd_consumer node to avoid a full > recompute > --- > northd/en-lflow.c | 25 +- > northd/en-northd.c | 225 +++++++++++++ > northd/en-northd.h | 23 ++ > northd/inc-proc-northd.c | 27 +- > northd/northd.c | 702 +++++++++++++++++++++++++++------------ > northd/northd.h | 73 +++- > tests/ovn-northd.at | 56 ++++ > 7 files changed, 894 insertions(+), 237 deletions(-) > > diff --git a/northd/en-lflow.c b/northd/en-lflow.c > index c4b927fb8..3dba5034b 100644 > --- a/northd/en-lflow.c > +++ b/northd/en-lflow.c > @@ -41,6 +41,11 @@ lflow_get_input_data(struct engine_node *node, > struct lflow_input *lflow_input) > { > struct northd_data *northd_data = engine_get_input_data("northd", node); > + struct bfd_data *bfd_data = engine_get_input_data("bfd", node); > + struct static_routes_data *static_routes_data = > + engine_get_input_data("static_routes", node); > + struct route_policies_data *route_policies_data = > + engine_get_input_data("route_policies", node); > struct port_group_data *pg_data = > engine_get_input_data("port_group", node); > struct sync_meters_data *sync_meters_data = > @@ -50,10 +55,6 @@ lflow_get_input_data(struct engine_node *node, > struct ed_type_ls_stateful *ls_stateful_data = > engine_get_input_data("ls_stateful", node); > > - lflow_input->nbrec_bfd_table = > - EN_OVSDB_GET(engine_get_input("NB_bfd", node)); > - lflow_input->sbrec_bfd_table = > - EN_OVSDB_GET(engine_get_input("SB_bfd", node)); > lflow_input->sbrec_logical_flow_table = > EN_OVSDB_GET(engine_get_input("SB_logical_flow", node)); > lflow_input->sbrec_multicast_group_table = > @@ -78,7 +79,10 @@ lflow_get_input_data(struct engine_node *node, > lflow_input->meter_groups = &sync_meters_data->meter_groups; > lflow_input->lb_datapaths_map = &northd_data->lb_datapaths_map; > lflow_input->svc_monitor_map = &northd_data->svc_monitor_map; > - lflow_input->bfd_connections = NULL; > + lflow_input->bfd_connections = &bfd_data->bfd_connections; > + lflow_input->parsed_routes = &static_routes_data->parsed_routes; > + lflow_input->route_tables = &static_routes_data->route_tables; > + lflow_input->route_policies = &route_policies_data->route_policies; > > struct ed_type_global_config *global_config = > engine_get_input_data("global_config", node); > @@ -95,25 +99,14 @@ void en_lflow_run(struct engine_node *node, void *data) > struct lflow_input lflow_input; > lflow_get_input_data(node, &lflow_input); > > - struct hmap bfd_connections = HMAP_INITIALIZER(&bfd_connections); > - lflow_input.bfd_connections = &bfd_connections; > - > stopwatch_start(BUILD_LFLOWS_STOPWATCH_NAME, time_msec()); > > struct lflow_data *lflow_data = data; > lflow_table_clear(lflow_data->lflow_table); > lflow_reset_northd_refs(&lflow_input); > > - build_bfd_table(eng_ctx->ovnsb_idl_txn, > - lflow_input.nbrec_bfd_table, > - lflow_input.sbrec_bfd_table, > - lflow_input.lr_ports, > - &bfd_connections); > build_lflows(eng_ctx->ovnsb_idl_txn, &lflow_input, > lflow_data->lflow_table); > - bfd_cleanup_connections(lflow_input.nbrec_bfd_table, > - &bfd_connections); > - hmap_destroy(&bfd_connections); > stopwatch_stop(BUILD_LFLOWS_STOPWATCH_NAME, time_msec()); > > engine_set_node_state(node, EN_UPDATED); > diff --git a/northd/en-northd.c b/northd/en-northd.c > index 4479b4aff..817a4736c 100644 > --- a/northd/en-northd.c > +++ b/northd/en-northd.c > @@ -236,6 +236,172 @@ northd_global_config_handler(struct engine_node *node, void *data OVS_UNUSED) > return true; > } > > +bool > +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 false; > + } > + > + /* This node uses the below data from the en_northd engine node. > + * See (lr_stateful_get_input_data()) > + * 1. northd_data->lr_datapaths > + * 2. northd_data->lr_ports > + * 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. > + * Note: When we add I-P to the created/deleted logical routers or > + * logical router ports, we need to revisit this handler. > + * > + * This node also accesses the route policies of the logical router. > + * When these route policies get updated, en_northd engine recomputes > + * and so does this node. > + * Note: When we add I-P to handle route policies changes, we need > + * to revisit this handler. > + */ > + return true; > +} > + > +void > +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, &northd_data->lr_ports, > + &route_policies_data->route_policies, > + &route_policies_data->bfd_active_connections, > + &bfd_data->bfd_connections); > + } > + > + engine_set_node_state(node, EN_UPDATED); > +} > + > +bool > +static_routes_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 false; > + } > + > + /* This node uses the below data from the en_northd engine node. > + * See (lr_stateful_get_input_data()) > + * 1. northd_data->lr_datapaths > + * 2. northd_data->lr_ports > + * 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. > + * Note: When we add I-P to the created/deleted logical routers or > + * logical router ports, we need to revisit this handler. > + * > + * This node also accesses the static routes of the logical router. > + * When these static routes gets updated, en_northd engine recomputes > + * and so does this node. > + * Note: When we add I-P to handle static routes changes, we need > + * to revisit this handler. > + */ > + return true; > +} > + > +void > +en_static_routes_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 static_routes_data *static_routes_data = data; > + > + static_routes_destroy(data); > + static_routes_init(data); > + > + struct ovn_datapath *od; > + HMAP_FOR_EACH (od, key_node, &northd_data->lr_datapaths.datapaths) { > + for (int i = 0; i < od->nbr->n_ports; i++) { > + const char *route_table_name = > + smap_get(&od->nbr->ports[i]->options, "route_table"); > + get_route_table_id(&static_routes_data->route_tables, > + route_table_name); > + } > + > + build_parsed_routes(od, &northd_data->lr_ports, > + &static_routes_data->parsed_routes, > + &static_routes_data->route_tables, > + &static_routes_data->bfd_active_connections, > + &bfd_data->bfd_connections); > + } > + > + engine_set_node_state(node, EN_UPDATED); > +} > + > +bool > +bfd_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 false; > + } > + > + /* This node uses the below data from the en_northd engine node. > + * See (lr_stateful_get_input_data()) > + * 1. northd_data->lr_ports > + * This data gets updated when a logical router port is created or > + * deleted. Northd engine node presently falls back to full recompute > + * when this happens and so does this node. > + * Note: When we add I-P to the created/deleted logical router ports, > + * we need to revisit this handler. > + */ > + return true; > +} > + > +void > +en_bfd_run(struct engine_node *node, void *data) > +{ > + struct northd_data *northd_data = engine_get_input_data("northd", node); > + struct bfd_data *bfd_data = data; > + const struct nbrec_bfd_table *nbrec_bfd_table = > + EN_OVSDB_GET(engine_get_input("NB_bfd", node)); > + const struct sbrec_bfd_table *sbrec_bfd_table = > + EN_OVSDB_GET(engine_get_input("SB_bfd", node)); > + > + bfd_destroy(data); > + bfd_init(data); > + build_bfd_map(nbrec_bfd_table, sbrec_bfd_table, > + &northd_data->lr_ports, &bfd_data->bfd_connections); > + engine_set_node_state(node, EN_UPDATED); > +} > + > +void > +en_bfd_sync_run(struct engine_node *node OVS_UNUSED, > + void *data OVS_UNUSED) > +{ > + const struct engine_context *eng_ctx = engine_get_context(); > + struct bfd_data *bfd_data = engine_get_input_data("bfd", node); > + struct route_policies_data *route_policies_data > + = engine_get_input_data("route_policies", node); > + struct static_routes_data *static_routes_data > + = engine_get_input_data("static_routes", node); > + const struct nbrec_bfd_table *nbrec_bfd_table = > + EN_OVSDB_GET(engine_get_input("NB_bfd", node)); > + > + bfd_table_sync(eng_ctx->ovnsb_idl_txn, nbrec_bfd_table, > + &bfd_data->bfd_connections, > + &route_policies_data->bfd_active_connections, > + &static_routes_data->bfd_active_connections); > + engine_set_node_state(node, EN_UPDATED); > +} > + > void > *en_northd_init(struct engine_node *node OVS_UNUSED, > struct engine_arg *arg OVS_UNUSED) > @@ -247,6 +413,42 @@ 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_static_routes_init(struct engine_node *node OVS_UNUSED, > + struct engine_arg *arg OVS_UNUSED) > +{ > + struct static_routes_data *data = xzalloc(sizeof *data); > + > + static_routes_init(data); > + return data; > +} > + > +void > +*en_bfd_init(struct engine_node *node OVS_UNUSED, > + struct engine_arg *arg OVS_UNUSED) > +{ > + struct bfd_data *data = xzalloc(sizeof *data); > + > + bfd_init(data); > + return data; > +} > + > +void *en_bfd_sync_init(struct engine_node *node OVS_UNUSED, > + struct engine_arg *arg OVS_UNUSED) > +{ > + return NULL; > +} > + > void > en_northd_cleanup(void *data) > { > @@ -259,3 +461,26 @@ en_northd_clear_tracked_data(void *data_) > struct northd_data *data = data_; > destroy_northd_data_tracked_changes(data); > } > + > +void > +en_route_policies_cleanup(void *data) > +{ > + route_policies_destroy(data); > +} > + > +void > +en_static_routes_cleanup(void *data) > +{ > + static_routes_destroy(data); > +} > + > +void > +en_bfd_cleanup(void *data) > +{ > + bfd_destroy(data); > +} > + > +void > +en_bfd_sync_cleanup(void *data OVS_UNUSED) > +{ > +} > diff --git a/northd/en-northd.h b/northd/en-northd.h > index 9b7bda32a..68f7755f8 100644 > --- a/northd/en-northd.h > +++ b/northd/en-northd.h > @@ -19,5 +19,28 @@ bool northd_nb_logical_switch_handler(struct engine_node *, void *data); > bool northd_nb_logical_router_handler(struct engine_node *, void *data); > bool northd_sb_port_binding_handler(struct engine_node *, void *data); > bool northd_lb_data_handler(struct engine_node *, void *data); > +void *en_static_routes_init(struct engine_node *node OVS_UNUSED, > + struct engine_arg *arg OVS_UNUSED); > +void en_route_policies_cleanup(void *data); > +bool route_policies_northd_change_handler(struct engine_node *node, > + void *data OVS_UNUSED); > +void 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_static_routes_cleanup(void *data); > +bool static_routes_northd_change_handler(struct engine_node *node, > + void *data OVS_UNUSED); > +void en_static_routes_run(struct engine_node *node, void *data); > +void *en_bfd_init(struct engine_node *node OVS_UNUSED, > + struct engine_arg *arg OVS_UNUSED); > +void en_bfd_cleanup(void *data); > +bool bfd_northd_change_handler(struct engine_node *node, > + void *data OVS_UNUSED); > +void en_bfd_run(struct engine_node *node, void *data); > +void *en_bfd_sync_init(struct engine_node *node OVS_UNUSED, > + struct engine_arg *arg OVS_UNUSED); > +void en_bfd_sync_run(struct engine_node *node OVS_UNUSED, > + void *data OVS_UNUSED); > +void en_bfd_sync_cleanup(void *data OVS_UNUSED); > > #endif /* EN_NORTHD_H */ > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > index 522236ad2..77610cdd8 100644 > --- a/northd/inc-proc-northd.c > +++ b/northd/inc-proc-northd.c > @@ -155,6 +155,10 @@ static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lb_data, "lb_data"); > static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lr_nat, "lr_nat"); > static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lr_stateful, "lr_stateful"); > static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(ls_stateful, "ls_stateful"); > +static ENGINE_NODE(route_policies, "route_policies"); > +static ENGINE_NODE(static_routes, "static_routes"); > +static ENGINE_NODE(bfd, "bfd"); > +static ENGINE_NODE(bfd_sync, "bfd_sync"); > > void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > struct ovsdb_idl_loop *sb) > @@ -237,18 +241,37 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > engine_add_input(&en_fdb_aging, &en_global_config, > node_global_config_handler); > > + engine_add_input(&en_bfd, &en_nb_bfd, NULL); > + engine_add_input(&en_bfd, &en_sb_bfd, NULL); > + engine_add_input(&en_bfd, &en_northd, bfd_northd_change_handler); > + > + engine_add_input(&en_route_policies, &en_bfd, NULL); > + engine_add_input(&en_route_policies, &en_northd, > + route_policies_northd_change_handler); > + > + engine_add_input(&en_static_routes, &en_bfd, NULL); > + engine_add_input(&en_static_routes, &en_northd, > + static_routes_northd_change_handler); > + > + engine_add_input(&en_bfd_sync, &en_bfd, NULL); > + engine_add_input(&en_bfd_sync, &en_nb_bfd, NULL); > + engine_add_input(&en_bfd_sync, &en_static_routes, NULL); > + engine_add_input(&en_bfd_sync, &en_route_policies, NULL); > + > engine_add_input(&en_sync_meters, &en_nb_acl, NULL); > engine_add_input(&en_sync_meters, &en_nb_meter, NULL); > engine_add_input(&en_sync_meters, &en_sb_meter, NULL); > > - engine_add_input(&en_lflow, &en_nb_bfd, NULL); > engine_add_input(&en_lflow, &en_nb_acl, NULL); > engine_add_input(&en_lflow, &en_sync_meters, NULL); > - engine_add_input(&en_lflow, &en_sb_bfd, NULL); > engine_add_input(&en_lflow, &en_sb_logical_flow, NULL); > engine_add_input(&en_lflow, &en_sb_multicast_group, NULL); > engine_add_input(&en_lflow, &en_sb_igmp_group, NULL); > engine_add_input(&en_lflow, &en_sb_logical_dp_group, NULL); > + engine_add_input(&en_lflow, &en_bfd, NULL); > + engine_add_input(&en_lflow, &en_bfd_sync, NULL); It seems en_lflow node doesn't depend on en_bfd_sync. In theory the en_bfd_sync can be added as an input of en_northd_output. What's the reason if it has to be added as an input of en_lflow? > + engine_add_input(&en_lflow, &en_route_policies, NULL); > + engine_add_input(&en_lflow, &en_static_routes, NULL); > engine_add_input(&en_lflow, &en_global_config, > node_global_config_handler); > engine_add_input(&en_lflow, &en_northd, lflow_northd_handler); > diff --git a/northd/northd.c b/northd/northd.c > index 5c2fd74ff..18fe2b48a 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -9689,11 +9689,49 @@ 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; > > - bool ref; > + struct ovn_port *op; > + > + char *logical_port; > + char *dst_ip; > + char *status; > + bool stale; > }; > > +static struct bfd_entry * > +bfd_alloc_entry(struct hmap *bfd_connections, > + const char *logical_port, const char *dst_ip, > + const char *status) > +{ > + struct bfd_entry *bfd_e = xzalloc(sizeof *bfd_e); > + bfd_e->logical_port = xstrdup(logical_port); > + bfd_e->dst_ip = xstrdup(dst_ip); > + bfd_e->status = xstrdup(status); > + uint32_t hash = hash_string(dst_ip, 0); > + hash = hash_string(logical_port, hash); > + hmap_insert(bfd_connections, &bfd_e->hmap_node, hash); > + > + return bfd_e; > +} > + > +static void > +bfd_erase_entry(struct bfd_entry *bfd_e) > +{ > + free(bfd_e->logical_port); > + free(bfd_e->dst_ip); > + free(bfd_e->status); > + free(bfd_e); > +} > + > +static 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 * > bfd_port_lookup(const struct hmap *bfd_map, const char *logical_port, > const char *dst_ip) > @@ -9704,36 +9742,13 @@ bfd_port_lookup(const struct hmap *bfd_map, const char *logical_port, > hash = hash_string(dst_ip, 0); > hash = hash_string(logical_port, hash); > HMAP_FOR_EACH_WITH_HASH (bfd_e, hmap_node, hash, bfd_map) { > - if (!strcmp(bfd_e->sb_bt->logical_port, logical_port) && > - !strcmp(bfd_e->sb_bt->dst_ip, dst_ip)) { > + if (!strcmp(bfd_e->logical_port, logical_port) && > + !strcmp(bfd_e->dst_ip, dst_ip)) { > return bfd_e; > } > } > - return NULL; > -} > > -void > -bfd_cleanup_connections(const struct nbrec_bfd_table *nbrec_bfd_table, > - struct hmap *bfd_map) > -{ > - const struct nbrec_bfd *nb_bt; > - struct bfd_entry *bfd_e; > - > - NBREC_BFD_TABLE_FOR_EACH (nb_bt, nbrec_bfd_table) { > - bfd_e = bfd_port_lookup(bfd_map, nb_bt->logical_port, nb_bt->dst_ip); > - if (!bfd_e) { > - continue; > - } > - > - if (!bfd_e->ref && strcmp(nb_bt->status, "admin_down")) { > - /* no user for this bfd connection */ > - nbrec_bfd_set_status(nb_bt, "admin_down"); > - } > - } > - > - HMAP_FOR_EACH_POP (bfd_e, hmap_node, bfd_map) { > - free(bfd_e); > - } > + return NULL; > } > > #define BFD_DEF_MINTX 1000 /* 1s */ > @@ -9797,50 +9812,64 @@ static int bfd_get_unused_port(unsigned long *bfd_src_ports) > } > > void > -build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn, > - const struct nbrec_bfd_table *nbrec_bfd_table, > - const struct sbrec_bfd_table *sbrec_bfd_table, > - const struct hmap *lr_ports, struct hmap *bfd_connections) > +bfd_table_sync(struct ovsdb_idl_txn *ovnsb_txn, > + const struct nbrec_bfd_table *nbrec_bfd_table, > + struct hmap *bfd_connections, > + struct hmap *rp_bfd_connections, > + struct hmap *sr_bfd_connections) > { > - struct hmap sb_only = HMAP_INITIALIZER(&sb_only); > - const struct sbrec_bfd *sb_bt; > - unsigned long *bfd_src_ports; > - struct bfd_entry *bfd_e; > - uint32_t hash; > + if (!ovnsb_txn) { > + return; > + } > > - bfd_src_ports = bitmap_allocate(BFD_UDP_SRC_PORT_LEN); > + unsigned long *bfd_src_ports = bitmap_allocate(BFD_UDP_SRC_PORT_LEN); > > - SBREC_BFD_TABLE_FOR_EACH (sb_bt, sbrec_bfd_table) { > - bfd_e = xmalloc(sizeof *bfd_e); > - bfd_e->sb_bt = sb_bt; > - hash = hash_string(sb_bt->dst_ip, 0); > - hash = hash_string(sb_bt->logical_port, hash); > - hmap_insert(&sb_only, &bfd_e->hmap_node, hash); > - bitmap_set1(bfd_src_ports, sb_bt->src_port - BFD_UDP_SRC_PORT_START); > + struct bfd_entry *bfd_e; > + HMAP_FOR_EACH (bfd_e, hmap_node, bfd_connections) { > + bfd_e->stale = true; > + /* we need to check if this entry is even in the BFD nb db table */ > + if (bfd_e->sb_bt) { > + bitmap_set1(bfd_src_ports, > + bfd_e->sb_bt->src_port - BFD_UDP_SRC_PORT_START); > + } > } > > const struct nbrec_bfd *nb_bt; > NBREC_BFD_TABLE_FOR_EACH (nb_bt, nbrec_bfd_table) { > - if (!nb_bt->status) { > - /* default state is admin_down */ > - nbrec_bfd_set_status(nb_bt, "admin_down"); > + bfd_e = bfd_port_lookup(bfd_connections, nb_bt->logical_port, > + nb_bt->dst_ip); > + if (!bfd_e) { > + continue; > } > > - struct ovn_port *op = ovn_port_find(lr_ports, nb_bt->logical_port); > - bfd_e = bfd_port_lookup(&sb_only, nb_bt->logical_port, nb_bt->dst_ip); > - if (!bfd_e) { > + struct ovn_port *op = bfd_e->op; > + if (!op) { > + continue; > + } > + > + if (!bfd_port_lookup(rp_bfd_connections, nb_bt->logical_port, > + nb_bt->dst_ip) && > + !bfd_port_lookup(sr_bfd_connections, nb_bt->logical_port, > + nb_bt->dst_ip)) { > + /* no consumer for this bfd connection, mark it as admin_down. */ > + bfd_set_status(bfd_e, "admin_down"); > + } > + nbrec_bfd_set_status(nb_bt, bfd_e->status); > + > + if (!bfd_e->sb_bt) { > int udp_src = bfd_get_unused_port(bfd_src_ports); > if (udp_src < 0) { > continue; > } > > - sb_bt = sbrec_bfd_insert(ovnsb_txn); > + /* Add entry to bfd sb table. */ > + const struct sbrec_bfd *sb_bt = sbrec_bfd_insert(ovnsb_txn); > sbrec_bfd_set_logical_port(sb_bt, nb_bt->logical_port); > sbrec_bfd_set_dst_ip(sb_bt, nb_bt->dst_ip); > sbrec_bfd_set_disc(sb_bt, 1 + random_uint32()); > sbrec_bfd_set_src_port(sb_bt, udp_src); > sbrec_bfd_set_status(sb_bt, nb_bt->status); > - if (op && op->sb && op->sb->chassis) { > + if (op->sb->chassis) { > sbrec_bfd_set_chassis_name(sb_bt, op->sb->chassis->name); > } > > @@ -9858,41 +9887,77 @@ build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn, > sbrec_bfd_set_status(bfd_e->sb_bt, nb_bt->status); > } else { > nbrec_bfd_set_status(nb_bt, bfd_e->sb_bt->status); > + bfd_set_status(bfd_e, bfd_e->sb_bt->status); > } > } > + > build_bfd_update_sb_conf(nb_bt, bfd_e->sb_bt); > - if (op && op->sb && op->sb->chassis && > - strcmp(op->sb->chassis->name, bfd_e->sb_bt->chassis_name)) { > + if (op->sb->chassis && !strcmp(op->sb->chassis->name, > + bfd_e->sb_bt->chassis_name)) { > sbrec_bfd_set_chassis_name(bfd_e->sb_bt, > op->sb->chassis->name); > } > - > - hmap_remove(&sb_only, &bfd_e->hmap_node); > - bfd_e->ref = false; > - hash = hash_string(bfd_e->sb_bt->dst_ip, 0); > - hash = hash_string(bfd_e->sb_bt->logical_port, hash); > - hmap_insert(bfd_connections, &bfd_e->hmap_node, hash); > } > + bfd_e->stale = false; > + } > > - if (op) { > - op->has_bfd = true; > + HMAP_FOR_EACH (bfd_e, hmap_node, bfd_connections) { > + if (!bfd_e->stale) { > + continue; > } > - } > > - HMAP_FOR_EACH_POP (bfd_e, hmap_node, &sb_only) { > - struct ovn_port *op = ovn_port_find(lr_ports, > - bfd_e->sb_bt->logical_port); > - if (op) { > - op->has_bfd = false; > + if (bfd_e->op) { > + bfd_e->op->has_bfd = false; > } > sbrec_bfd_delete(bfd_e->sb_bt); > - free(bfd_e); > } > - hmap_destroy(&sb_only); > > bitmap_free(bfd_src_ports); > } > > +void > +build_bfd_map(const struct nbrec_bfd_table *nbrec_bfd_table, > + const struct sbrec_bfd_table *sbrec_bfd_table, > + const struct hmap *lr_ports, struct hmap *bfd_connections) > +{ > + struct bfd_entry *bfd_e; > + > + /* align bfd_map to sb db */ > + const struct sbrec_bfd *sb_bt; > + SBREC_BFD_TABLE_FOR_EACH (sb_bt, sbrec_bfd_table) { > + bfd_e = bfd_port_lookup(bfd_connections, sb_bt->logical_port, > + sb_bt->dst_ip); > + if (!bfd_e) { > + bfd_e = bfd_alloc_entry(bfd_connections, sb_bt->logical_port, > + sb_bt->dst_ip, sb_bt->status); > + } > + bfd_e->sb_bt = sb_bt; > + } > + > + const struct nbrec_bfd *nb_bt; > + NBREC_BFD_TABLE_FOR_EACH (nb_bt, nbrec_bfd_table) { > + struct ovn_port *op = ovn_port_find(lr_ports, nb_bt->logical_port); > + if (!op || !op->sb) { > + /* skip not bounded ports */ > + continue; > + } > + > + bfd_e = bfd_port_lookup(bfd_connections, nb_bt->logical_port, > + nb_bt->dst_ip); > + if (!bfd_e) { > + /* brand new entry. */ > + bfd_e = bfd_alloc_entry(bfd_connections, nb_bt->logical_port, > + nb_bt->dst_ip, "admin_down"); > + } > + > + bfd_e->nb_bt = nb_bt; > + bfd_e->op = op; > + if (op) { > + op->has_bfd = true; > + } > + } > +} > + > /* Returns a string of the IP address of the router port 'op' that > * overlaps with 'ip_s". If one is not found, returns NULL. > * > @@ -9928,17 +9993,13 @@ get_outport_for_routing_policy_nexthop(struct ovn_datapath *od, > return NULL; > } > > -static struct ovs_mutex bfd_lock = OVS_MUTEX_INITIALIZER; > - > -static bool check_bfd_state( > - const struct nbrec_logical_router_policy *rule, > - const struct hmap *bfd_connections, > - struct ovn_port *out_port, > - const char *nexthop) > +static bool check_bfd_state(const struct nbrec_logical_router_policy *rule, > + struct ovn_port *out_port, const char *nexthop, > + struct hmap *bfd_active_connections, > + struct hmap *bfd_connections) > { > struct in6_addr nexthop_v6; > bool is_nexthop_v6 = ipv6_parse(nexthop, &nexthop_v6); > - bool ret = true; > > for (size_t i = 0; i < rule->n_bfd_sessions; i++) { > /* Check if there is a BFD session associated to the reroute > @@ -9963,39 +10024,45 @@ static bool check_bfd_state( > struct bfd_entry *bfd_e = bfd_port_lookup(bfd_connections, > nb_bt->logical_port, > nb_bt->dst_ip); > - ovs_mutex_lock(&bfd_lock); > - if (bfd_e) { > - bfd_e->ref = true; > + if (!bfd_e) { > + continue; > } > > - if (!strcmp(nb_bt->status, "admin_down")) { > - nbrec_bfd_set_status(nb_bt, "down"); > + if (!strcmp(bfd_e->status, "admin_down")) { > + bfd_set_status(bfd_e, "down"); > } This function is called by build_route_policies() which belongs to the en_route_polices node. The bfd_connections is input data so it should be readonly to this node and never be modified here. Otherwise, if a node modifies its input data, it may impact other nodes that depend on the same input node, and it is hard to get things correct in the I-P engine. Similarly, I see that the bfd_table_sync() function is modifying the en_bfd data, which is not good. en_bfd data should be modified in en_bfd node only, when consolidating status between SB DB and NB DB, and bfd_sync node should only update the DB, according to the data of en_bfd, en_route_policies and en_static_routes. Please let me know if this is challenging. Thanks, Han > > - ret = strcmp(nb_bt->status, "down"); > - ovs_mutex_unlock(&bfd_lock); > - break; > + /* This route policy is linked to an active bfd session. */ > + if (!bfd_port_lookup(bfd_active_connections, nb_bt->logical_port, > + nb_bt->dst_ip)) { > + bfd_alloc_entry(bfd_active_connections, nb_bt->logical_port, > + nb_bt->dst_ip, bfd_e->status); > + } > + > + return strcmp(bfd_e->status, "down"); > } > > - return ret; > + return true; > } > > static void > build_routing_policy_flow(struct lflow_table *lflows, struct ovn_datapath *od, > - const struct hmap *lr_ports, > - const struct nbrec_logical_router_policy *rule, > - const struct hmap *bfd_connections, > + const struct hmap *lr_ports, struct route_policy *rp, > const struct ovsdb_idl_row *stage_hint, > struct lflow_ref *lflow_ref) > { > + const struct nbrec_logical_router_policy *rule = rp->rule; > struct ds match = DS_EMPTY_INITIALIZER; > struct ds actions = DS_EMPTY_INITIALIZER; > > if (!strcmp(rule->action, "reroute")) { > ovs_assert(rule->n_nexthops <= 1); > > - char *nexthop = > - (rule->n_nexthops == 1 ? rule->nexthops[0] : rule->nexthop); > + if (!rp->n_valid_nexthops) { > + return; > + } > + > + char *nexthop = rp->valid_nexthops[0]; > struct ovn_port *out_port = get_outport_for_routing_policy_nexthop( > od, lr_ports, rule->priority, nexthop); > if (!out_port) { > @@ -10011,10 +10078,6 @@ build_routing_policy_flow(struct lflow_table *lflows, struct ovn_datapath *od, > return; > } > > - if (!check_bfd_state(rule, bfd_connections, out_port, nexthop)) { > - return; > - } > - > uint32_t pkt_mark = smap_get_uint(&rule->options, "pkt_mark", 0); > if (pkt_mark) { > ds_put_format(&actions, "pkt.mark = %u; ", pkt_mark); > @@ -10057,19 +10120,18 @@ static void > build_ecmp_routing_policy_flows(struct lflow_table *lflows, > struct ovn_datapath *od, > const struct hmap *lr_ports, > - const struct nbrec_logical_router_policy *rule, > - const struct hmap *bfd_connections, > + struct route_policy *rp, > uint16_t ecmp_group_id, > struct lflow_ref *lflow_ref) > { > - ovs_assert(rule->n_nexthops > 1); > - > bool nexthops_is_ipv4 = true; > + const struct nbrec_logical_router_policy *rule = rp->rule; > + ovs_assert(rule->n_nexthops > 1); > > /* Check that all the nexthops belong to the same addr family before > * adding logical flows. */ > - for (uint16_t i = 0; i < rule->n_nexthops; i++) { > - bool is_ipv4 = strchr(rule->nexthops[i], '.') ? true : false; > + for (uint16_t i = 0; i < rp->n_valid_nexthops; i++) { > + bool is_ipv4 = strchr(rp->valid_nexthops[i], '.') ? true : false; > > if (i == 0) { > nexthops_is_ipv4 = is_ipv4; > @@ -10080,7 +10142,7 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, > VLOG_WARN_RL(&rl, "nexthop [%s] of the router policy with " > "the match [%s] do not belong to the same address " > "family as other next hops", > - rule->nexthops[i], rule->match); > + rp->valid_nexthops[i], rule->match); > return; > } > } > @@ -10088,40 +10150,30 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, > struct ds match = DS_EMPTY_INITIALIZER; > struct ds actions = DS_EMPTY_INITIALIZER; > > - size_t *valid_nexthops = xcalloc(rule->n_nexthops, sizeof *valid_nexthops); > - size_t n_valid_nexthops = 0; > - > - for (size_t i = 0; i < rule->n_nexthops; i++) { > + for (size_t i = 0; i < rp->n_valid_nexthops; i++) { > struct ovn_port *out_port = get_outport_for_routing_policy_nexthop( > - od, lr_ports, rule->priority, rule->nexthops[i]); > + od, lr_ports, rule->priority, rp->valid_nexthops[i]); > if (!out_port) { > goto cleanup; > } > > const char *lrp_addr_s = > - find_lrp_member_ip(out_port, rule->nexthops[i]); > + find_lrp_member_ip(out_port, rp->valid_nexthops[i]); > if (!lrp_addr_s) { > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > VLOG_WARN_RL(&rl, "lrp_addr not found for routing policy " > " priority %"PRId64" nexthop %s", > - rule->priority, rule->nexthops[i]); > + rule->priority, rp->valid_nexthops[i]); > goto cleanup; > } > > - if (!check_bfd_state(rule, bfd_connections, out_port, > - rule->nexthops[i])) { > - continue; > - } > - > - valid_nexthops[n_valid_nexthops++] = i + 1; > - > ds_clear(&actions); > uint32_t pkt_mark = smap_get_uint(&rule->options, "pkt_mark", 0); > if (pkt_mark) { > ds_put_format(&actions, "pkt.mark = %u; ", pkt_mark); > } > > - bool is_ipv4 = strchr(rule->nexthops[i], '.') ? true : false; > + bool is_ipv4 = strchr(rp->valid_nexthops[i], '.') ? true : false; > > ds_put_format(&actions, "%s = %s; " > "%s = %s; " > @@ -10130,7 +10182,7 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, > "flags.loopback = 1; " > "next;", > is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6, > - rule->nexthops[i], > + rp->valid_nexthops[i], > is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6, > lrp_addr_s, > out_port->lrp_networks.ea_s, > @@ -10146,37 +10198,30 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, > lflow_ref); > } > > - if (!n_valid_nexthops) { > - goto cleanup; > - } > - > ds_clear(&actions); > - if (n_valid_nexthops > 1) { > + if (rp->n_valid_nexthops > 1) { > ds_put_format(&actions, "%s = %"PRIu16 > "; %s = select(", REG_ECMP_GROUP_ID, ecmp_group_id, > REG_ECMP_MEMBER_ID); > > - for (size_t i = 0; i < n_valid_nexthops; i++) { > + for (size_t i = 0; i < rp->n_valid_nexthops; i++) { > if (i > 0) { > ds_put_cstr(&actions, ", "); > } > > - ds_put_format(&actions, "%"PRIuSIZE, valid_nexthops[i]); > + ds_put_format(&actions, "%"PRIuSIZE, i + 1); > } > ds_put_cstr(&actions, ");"); > } else { > ds_put_format(&actions, "%s = %"PRIu16 > - "; %s = %"PRIuSIZE"; next;", REG_ECMP_GROUP_ID, > - ecmp_group_id, REG_ECMP_MEMBER_ID, > - valid_nexthops[0]); > + "; %s = 1; next;", REG_ECMP_GROUP_ID, > + ecmp_group_id, REG_ECMP_MEMBER_ID); > } > ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_POLICY, > rule->priority, rule->match, > ds_cstr(&actions), &rule->header_, > lflow_ref); > - > cleanup: > - free(valid_nexthops); > ds_destroy(&match); > ds_destroy(&actions); > } > @@ -10201,7 +10246,7 @@ route_table_add(struct simap *route_tables, const char *route_table_name) > return rtb_id; > } > > -static uint32_t > +uint32_t > get_route_table_id(struct simap *route_tables, const char *route_table_name) > { > if (!route_table_name || !route_table_name[0]) { > @@ -10242,18 +10287,6 @@ build_route_table_lflow(struct ovn_datapath *od, struct lflow_table *lflows, > ds_destroy(&actions); > } > > -struct parsed_route { > - struct ovs_list list_node; > - struct in6_addr prefix; > - unsigned int plen; > - bool is_src_route; > - uint32_t route_table_id; > - uint32_t hash; > - const struct nbrec_logical_router_static_route *route; > - bool ecmp_symmetric_reply; > - bool is_discard_route; > -}; > - > static uint32_t > route_hash(struct parsed_route *route) > { > @@ -10268,11 +10301,53 @@ find_static_route_outport(struct ovn_datapath *od, const struct hmap *lr_ports, > > /* Parse and validate the route. Return the parsed route if successful. > * Otherwise return NULL. */ > + > static struct parsed_route * > +parsed_route_lookup(struct hmap *routes, size_t hash, > + struct parsed_route *new_pr) > +{ > + struct parsed_route *pr; > + HMAP_FOR_EACH_WITH_HASH (pr, key_node, hash, routes) { > + if (pr->plen != new_pr->plen) { > + continue; > + } > + > + if (memcmp(&pr->prefix, &new_pr->prefix, sizeof(struct in6_addr))) { > + continue; > + } > + > + if (pr->is_src_route != new_pr->is_src_route) { > + continue; > + } > + > + if (pr->route_table_id != new_pr->route_table_id) { > + continue; > + } > + > + if (pr->route != new_pr->route) { > + continue; > + } > + > + if (pr->ecmp_symmetric_reply != new_pr->ecmp_symmetric_reply) { > + continue; > + } > + > + if (pr->is_discard_route != new_pr->is_discard_route) { > + continue; > + } > + > + return pr; > + } > + > + return NULL; > +} > + > +static void > parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > - struct ovs_list *routes, struct simap *route_tables, > + struct hmap *routes, struct simap *route_tables, > const struct nbrec_logical_router_static_route *route, > - const struct hmap *bfd_connections) > + struct hmap *bfd_active_connections, > + struct hmap *bfd_connections) > { > /* Verify that the next hop is an IP address with an all-ones mask. */ > struct in6_addr nexthop; > @@ -10285,7 +10360,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > VLOG_WARN_RL(&rl, "bad 'nexthop' %s in static route " > UUID_FMT, route->nexthop, > UUID_ARGS(&route->header_.uuid)); > - return NULL; > + return; > } > if ((IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 32) || > (!IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 128)) { > @@ -10293,7 +10368,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > VLOG_WARN_RL(&rl, "bad next hop mask %s in static route " > UUID_FMT, route->nexthop, > UUID_ARGS(&route->header_.uuid)); > - return NULL; > + return; > } > } > > @@ -10304,7 +10379,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > VLOG_WARN_RL(&rl, "bad 'ip_prefix' %s in static route " > UUID_FMT, route->ip_prefix, > UUID_ARGS(&route->header_.uuid)); > - return NULL; > + return; > } > > /* Verify that ip_prefix and nexthop have same address familiy. */ > @@ -10315,7 +10390,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > " %s and 'nexthop' %s in static route "UUID_FMT, > route->ip_prefix, route->nexthop, > UUID_ARGS(&route->header_.uuid)); > - return NULL; > + return; > } > } > > @@ -10324,52 +10399,85 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > !find_static_route_outport(od, lr_ports, route, > IN6_IS_ADDR_V4MAPPED(&prefix), > NULL, NULL)) { > - return NULL; > + return; > } > > const struct nbrec_bfd *nb_bt = route->bfd; > if (nb_bt && !strcmp(nb_bt->dst_ip, route->nexthop)) { > - struct bfd_entry *bfd_e; > + struct bfd_entry *bfd_e = bfd_port_lookup(bfd_connections, > + nb_bt->logical_port, > + nb_bt->dst_ip); > + if (!bfd_e) { > + return; > + } > > - bfd_e = bfd_port_lookup(bfd_connections, nb_bt->logical_port, > - nb_bt->dst_ip); > - ovs_mutex_lock(&bfd_lock); > - if (bfd_e) { > - bfd_e->ref = true; > + if (!strcmp(bfd_e->status, "admin_down")) { > + bfd_set_status(bfd_e, "down"); > } > > - if (!strcmp(nb_bt->status, "admin_down")) { > - nbrec_bfd_set_status(nb_bt, "down"); > + /* This static route is linked to an active bfd session. */ > + if (!bfd_port_lookup(bfd_active_connections, nb_bt->logical_port, > + nb_bt->dst_ip)) { > + bfd_alloc_entry(bfd_active_connections, nb_bt->logical_port, > + nb_bt->dst_ip, bfd_e->status); > } > > - if (!strcmp(nb_bt->status, "down")) { > - ovs_mutex_unlock(&bfd_lock); > - return NULL; > + if (!strcmp(bfd_e->status, "down")) { > + return; > } > - ovs_mutex_unlock(&bfd_lock); > } > > - struct parsed_route *pr = xzalloc(sizeof *pr); > - pr->prefix = prefix; > - pr->plen = plen; > - pr->route_table_id = get_route_table_id(route_tables, route->route_table); > - pr->is_src_route = (route->policy && !strcmp(route->policy, > - "src-ip")); > - pr->hash = route_hash(pr); > - pr->route = route; > - pr->ecmp_symmetric_reply = smap_get_bool(&route->options, > - "ecmp_symmetric_reply", false); > - pr->is_discard_route = is_discard_route; > - ovs_list_insert(routes, &pr->list_node); > - return pr; > + struct parsed_route *new_pr = xzalloc(sizeof *new_pr); > + new_pr->prefix = prefix; > + new_pr->plen = plen; > + new_pr->route_table_id = get_route_table_id(route_tables, > + route->route_table); > + new_pr->is_src_route = (route->policy && > + !strcmp(route->policy, "src-ip")); > + new_pr->hash = route_hash(new_pr); > + new_pr->route = route; > + new_pr->nbr = od->nbr; > + new_pr->ecmp_symmetric_reply = smap_get_bool(&route->options, > + "ecmp_symmetric_reply", > + false); > + new_pr->is_discard_route = is_discard_route; > + > + size_t hash = uuid_hash(&od->key); > + struct parsed_route *pr = parsed_route_lookup(routes, hash, new_pr); > + if (!pr) { > + hmap_insert(routes, &new_pr->key_node, hash); > + } else { > + pr->stale = false; > + free(new_pr); > + } > } > > -static void > -parsed_routes_destroy(struct ovs_list *routes) > +void > +build_parsed_routes(struct ovn_datapath *od, const struct hmap *lr_ports, > + struct hmap *routes, struct simap *route_tables, > + struct hmap *bfd_active_connections, > + struct hmap *bfd_connections) > { > struct parsed_route *pr; > - LIST_FOR_EACH_SAFE (pr, list_node, routes) { > - ovs_list_remove(&pr->list_node); > + HMAP_FOR_EACH (pr, key_node, routes) { > + if (pr->nbr == od->nbr) { > + pr->stale = true; > + } > + } > + > + for (int i = 0; i < od->nbr->n_static_routes; i++) { > + parsed_routes_add(od, lr_ports, routes, route_tables, > + od->nbr->static_routes[i], > + bfd_active_connections, > + bfd_connections); > + } > + > + HMAP_FOR_EACH_SAFE (pr, key_node, routes) { > + if (!pr->stale) { > + continue; > + } > + > + hmap_remove(routes, &pr->key_node); > free(pr); > } > } > @@ -12671,8 +12779,8 @@ build_ip_routing_flows_for_lrp( > static void > build_static_route_flows_for_lrouter( > struct ovn_datapath *od, struct lflow_table *lflows, > - const struct hmap *lr_ports, const struct hmap *bfd_connections, > - struct lflow_ref *lflow_ref) > + const struct hmap *lr_ports, struct hmap *parsed_routes, > + struct simap *route_tables, struct lflow_ref *lflow_ref) > { > ovs_assert(od->nbr); > ovn_lflow_add_default_drop(lflows, od, S_ROUTER_IN_IP_ROUTING_ECMP, > @@ -12685,22 +12793,16 @@ build_static_route_flows_for_lrouter( > > struct hmap ecmp_groups = HMAP_INITIALIZER(&ecmp_groups); > struct hmap unique_routes = HMAP_INITIALIZER(&unique_routes); > - struct ovs_list parsed_routes = OVS_LIST_INITIALIZER(&parsed_routes); > - struct simap route_tables = SIMAP_INITIALIZER(&route_tables); > struct ecmp_groups_node *group; > > for (int i = 0; i < od->nbr->n_ports; i++) { > build_route_table_lflow(od, lflows, od->nbr->ports[i], > - &route_tables, lflow_ref); > + route_tables, lflow_ref); > } > > - for (int i = 0; i < od->nbr->n_static_routes; i++) { > - struct parsed_route *route = > - parsed_routes_add(od, lr_ports, &parsed_routes, &route_tables, > - od->nbr->static_routes[i], bfd_connections); > - if (!route) { > - continue; > - } > + struct parsed_route *route; > + HMAP_FOR_EACH_WITH_HASH (route, key_node, uuid_hash(&od->key), > + parsed_routes) { > group = ecmp_groups_find(&ecmp_groups, route); > if (group) { > ecmp_groups_add_route(group, route); > @@ -12728,8 +12830,6 @@ build_static_route_flows_for_lrouter( > } > ecmp_groups_destroy(&ecmp_groups); > unique_routes_destroy(&unique_routes); > - parsed_routes_destroy(&parsed_routes); > - simap_destroy(&route_tables); > } > > /* IP Multicast lookup. Here we set the output port, adjust TTL and > @@ -12836,6 +12936,115 @@ build_mcast_lookup_flows_for_lrouter( > } > } > > +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->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; > +} > + > +void > +build_route_policies(struct ovn_datapath *od, struct hmap *lr_ports, > + struct hmap *route_policies, > + struct hmap *bfd_active_connections, > + struct hmap *bfd_connections) > +{ > + struct route_policy *rp; > + > + HMAP_FOR_EACH (rp, key_node, route_policies) { > + if (rp->nbr == od->nbr) { > + rp->stale = true; > + } > + } > + > + for (int i = 0; i < od->nbr->n_policies; i++) { > + const struct nbrec_logical_router_policy *rule = od->nbr->policies[i]; > + size_t n_valid_nexthops = 0; > + char **valid_nexthops = NULL; > + > + if (!strcmp(rule->action, "reroute")) { > + size_t n_nexthops = rule->n_nexthops ? rule->n_nexthops : 1; > + > + valid_nexthops = xcalloc(n_nexthops, sizeof *valid_nexthops); > + for (size_t j = 0; j < n_nexthops; j++) { > + char *nexthop = rule->n_nexthops > + ? rule->nexthops[j] : rule->nexthop; > + struct ovn_port *out_port = > + get_outport_for_routing_policy_nexthop( > + od, lr_ports, rule->priority, nexthop); > + if (!out_port || !check_bfd_state(rule, out_port, nexthop, > + bfd_active_connections, > + bfd_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->nbr = od->nbr; > + > + 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 { > + 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. > * > * A packet that arrives at this table is an IP packet that should be > @@ -12849,7 +13058,7 @@ static void > build_ingress_policy_flows_for_lrouter( > struct ovn_datapath *od, struct lflow_table *lflows, > const struct hmap *lr_ports, > - const struct hmap *bfd_connections, > + struct hmap *route_policies, > struct lflow_ref *lflow_ref) > { > ovs_assert(od->nbr); > @@ -12866,21 +13075,20 @@ build_ingress_policy_flows_for_lrouter( > > /* Convert routing policies to flows. */ > uint16_t ecmp_group_id = 1; > - for (int i = 0; i < od->nbr->n_policies; i++) { > - const struct nbrec_logical_router_policy *rule > - = od->nbr->policies[i]; > + struct route_policy *rp; > + HMAP_FOR_EACH_WITH_HASH (rp, key_node, uuid_hash(&od->key), > + route_policies) { > + const struct nbrec_logical_router_policy *rule = rp->rule; > bool is_ecmp_reroute = > (!strcmp(rule->action, "reroute") && rule->n_nexthops > 1); > > if (is_ecmp_reroute) { > - build_ecmp_routing_policy_flows(lflows, od, lr_ports, rule, > - bfd_connections, ecmp_group_id, > - lflow_ref); > + build_ecmp_routing_policy_flows(lflows, od, lr_ports, rp, > + ecmp_group_id, lflow_ref); > ecmp_group_id++; > } else { > - build_routing_policy_flow(lflows, od, lr_ports, rule, > - bfd_connections, &rule->header_, > - lflow_ref); > + build_routing_policy_flow(lflows, od, lr_ports, rp, > + &rule->header_, lflow_ref); > } > } > } > @@ -15805,6 +16013,9 @@ struct lswitch_flow_build_info { > struct ds actions; > size_t thread_lflow_counter; > const char *svc_monitor_mac; > + struct hmap *parsed_routes; > + struct hmap *route_policies; > + struct simap *route_tables; > }; > > /* Helper function to combine all lflow generation which is iterated by > @@ -15850,12 +16061,12 @@ build_lswitch_and_lrouter_iterate_by_lr(struct ovn_datapath *od, > build_ND_RA_flows_for_lrouter(od, lsi->lflows, NULL); > build_ip_routing_pre_flows_for_lrouter(od, lsi->lflows, NULL); > build_static_route_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports, > - lsi->bfd_connections, > + lsi->parsed_routes, lsi->route_tables, > NULL); > build_mcast_lookup_flows_for_lrouter(od, lsi->lflows, &lsi->match, > &lsi->actions, NULL); > build_ingress_policy_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports, > - lsi->bfd_connections, NULL); > + lsi->route_policies, NULL); > build_arp_resolve_flows_for_lrouter(od, lsi->lflows, NULL); > build_check_pkt_len_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports, > &lsi->match, &lsi->actions, > @@ -16169,7 +16380,10 @@ build_lswitch_and_lrouter_flows( > const struct hmap *svc_monitor_map, > const struct hmap *bfd_connections, > const struct chassis_features *features, > - const char *svc_monitor_mac) > + const char *svc_monitor_mac, > + struct hmap *parsed_routes, > + struct hmap *route_policies, > + struct simap *route_tables) > { > > char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac); > @@ -16203,6 +16417,9 @@ build_lswitch_and_lrouter_flows( > lsiv[index].svc_check_match = svc_check_match; > lsiv[index].thread_lflow_counter = 0; > lsiv[index].svc_monitor_mac = svc_monitor_mac; > + lsiv[index].parsed_routes = parsed_routes; > + lsiv[index].route_tables = route_tables; > + lsiv[index].route_policies = route_policies; > ds_init(&lsiv[index].match); > ds_init(&lsiv[index].actions); > > @@ -16243,6 +16460,9 @@ build_lswitch_and_lrouter_flows( > .features = features, > .svc_check_match = svc_check_match, > .svc_monitor_mac = svc_monitor_mac, > + .parsed_routes = parsed_routes, > + .route_tables = route_tables, > + .route_policies = route_policies, > .match = DS_EMPTY_INITIALIZER, > .actions = DS_EMPTY_INITIALIZER, > }; > @@ -16404,7 +16624,10 @@ void build_lflows(struct ovsdb_idl_txn *ovnsb_txn, > input_data->svc_monitor_map, > input_data->bfd_connections, > input_data->features, > - input_data->svc_monitor_mac); > + input_data->svc_monitor_mac, > + input_data->parsed_routes, > + input_data->route_policies, > + input_data->route_tables); > > if (parallelization_state == STATE_INIT_HASH_SIZES) { > parallelization_state = STATE_USE_PARALLELIZATION; > @@ -17477,6 +17700,27 @@ 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); > +} > + > +void > +static_routes_init(struct static_routes_data *data) > +{ > + hmap_init(&data->parsed_routes); > + simap_init(&data->route_tables); > + hmap_init(&data->bfd_active_connections); > +} > + > +void > +bfd_init(struct bfd_data *data) > +{ > + hmap_init(&data->bfd_connections); > +} > + > void > northd_destroy(struct northd_data *data) > { > @@ -17516,6 +17760,48 @@ northd_destroy(struct northd_data *data) > destroy_northd_tracked_data(data); > } > > +static void > +__bfd_destroy(struct hmap *bfd_connections) > +{ > + struct bfd_entry *bfd_e; > + > + HMAP_FOR_EACH_POP (bfd_e, hmap_node, bfd_connections) { > + bfd_erase_entry(bfd_e); > + } > + hmap_destroy(bfd_connections); > +} > + > +void > +bfd_destroy(struct bfd_data *data) > +{ > + __bfd_destroy(&data->bfd_connections); > +} > + > +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); > +} > + > +void > +static_routes_destroy(struct static_routes_data *data) > +{ > + struct parsed_route *r; > + HMAP_FOR_EACH_POP (r, key_node, &data->parsed_routes) { > + free(r); > + } > + hmap_destroy(&data->parsed_routes); > + > + simap_destroy(&data->route_tables); > + __bfd_destroy(&data->bfd_active_connections); > +} > + > void > ovnnb_db_run(struct northd_input *input_data, > struct northd_data *data, > diff --git a/northd/northd.h b/northd/northd.h > index d4a8d75ab..7ddb72eb7 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -23,6 +23,7 @@ > #include "northd/en-port-group.h" > #include "northd/ipam.h" > #include "openvswitch/hmap.h" > +#include "simap.h" > #include "ovs-thread.h" > > struct northd_input { > @@ -160,14 +161,34 @@ 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; > + const struct nbrec_logical_router *nbr; > + bool stale; > +}; > + > +struct static_routes_data { > + struct hmap parsed_routes; > + struct simap route_tables; > + struct hmap bfd_active_connections; > +}; > + > +struct route_policies_data { > + struct hmap route_policies; > + struct hmap bfd_active_connections; > +}; > + > +struct bfd_data { > + struct hmap bfd_connections; > +}; > + > struct lr_nat_table; > > struct lflow_input { > - /* Northbound table references */ > - const struct nbrec_bfd_table *nbrec_bfd_table; > - > /* Southbound table references */ > - const struct sbrec_bfd_table *sbrec_bfd_table; > const struct sbrec_logical_flow_table *sbrec_logical_flow_table; > const struct sbrec_multicast_group_table *sbrec_multicast_group_table; > const struct sbrec_igmp_group_table *sbrec_igmp_group_table; > @@ -190,6 +211,9 @@ struct lflow_input { > const struct hmap *svc_monitor_map; > bool ovn_internal_version_changed; > const char *svc_monitor_mac; > + struct hmap *parsed_routes; > + struct hmap *route_policies; > + struct simap *route_tables; > }; > > extern int parallelization_state; > @@ -653,6 +677,20 @@ struct ovn_port { > struct lflow_ref *stateful_lflow_ref; > }; > > +struct parsed_route { > + struct hmap_node key_node; > + struct in6_addr prefix; > + unsigned int plen; > + bool is_src_route; > + uint32_t route_table_id; > + uint32_t hash; > + const struct nbrec_logical_router_static_route *route; > + bool ecmp_symmetric_reply; > + bool is_discard_route; > + const struct nbrec_logical_router *nbr; > + bool stale; > +}; > + > void ovnnb_db_run(struct northd_input *input_data, > struct northd_data *data, > struct ovsdb_idl_txn *ovnnb_txn, > @@ -674,6 +712,18 @@ 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(struct ovn_datapath *, const struct hmap *, > + struct hmap *, struct simap *, struct hmap *, > + struct hmap *); > +uint32_t get_route_table_id(struct simap *, const char *); > +void static_routes_init(struct static_routes_data *); > +void static_routes_destroy(struct static_routes_data *); > + > +void bfd_init(struct bfd_data *); > +void bfd_destroy(struct bfd_data *); > + > struct lflow_table; > struct lr_stateful_tracked_data; > struct ls_stateful_tracked_data; > @@ -711,13 +761,14 @@ bool northd_handle_lb_data_changes(struct tracked_lb_data *, > struct hmap *lbgrp_datapaths_map, > struct northd_tracked_data *); > > -void build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn, > - const struct nbrec_bfd_table *, > - const struct sbrec_bfd_table *, > - const struct hmap *lr_ports, > - struct hmap *bfd_connections); > -void bfd_cleanup_connections(const struct nbrec_bfd_table *, > - struct hmap *bfd_map); > +void build_route_policies(struct ovn_datapath *, struct hmap *, struct hmap *, > + struct hmap *, struct hmap *); > +void bfd_table_sync(struct ovsdb_idl_txn *, const struct nbrec_bfd_table *, > + struct hmap *, struct hmap *, struct hmap *); > +void build_bfd_map(const struct nbrec_bfd_table *, > + const struct sbrec_bfd_table *, > + const struct hmap *lr_ports, > + struct hmap *bfd_connections); > void run_update_worker_pool(int n_threads); > > const struct ovn_datapath *northd_get_datapath_for_port( > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 57f89a774..1d7f55e4f 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -12,6 +12,7 @@ m4_define([_DUMP_DB_TABLES], [ > ovn-sbctl list meter >> $1 > ovn-sbctl list meter_band >> $1 > ovn-sbctl list port_group >> $1 > + ovn-sbctl list bfd >> $1 > ovn-sbctl dump-flows > lflows_$1 > ]) > > @@ -3861,6 +3862,7 @@ for i in $(seq 1 9); do > check ovn-nbctl --wait=sb lsp-set-addresses sw$i-r0 00:00:00:00:00:0$i > done > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > uuid=$(ovn-nbctl create bfd logical_port=r0-sw1 dst_ip=192.168.1.2 status=down min_tx=250 min_rx=250 detect_mult=10) > ovn-nbctl create bfd logical_port=r0-sw2 dst_ip=192.168.2.2 status=down min_tx=500 min_rx=500 detect_mult=20 > ovn-nbctl create bfd logical_port=r0-sw3 dst_ip=192.168.3.2 status=down > @@ -3873,6 +3875,13 @@ wait_row_count bfd 1 logical_port=r0-sw2 detect_mult=20 dst_ip=192.168.2.2 \ > wait_row_count bfd 1 logical_port=r0-sw3 detect_mult=5 dst_ip=192.168.3.2 \ > min_rx=1000 min_tx=1000 status=admin_down > > +check_engine_stats northd norecompute nocompute > +check_engine_stats bfd recompute nocompute > +check_engine_stats lflow recompute nocompute > +check_engine_stats northd_output norecompute compute > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > + > uuid=$(fetch_column nb:bfd _uuid logical_port=r0-sw1) > check ovn-nbctl set bfd $uuid min_tx=1000 min_rx=1000 detect_mult=100 > > @@ -3881,10 +3890,25 @@ check ovn-nbctl clear bfd $uuid_2 min_rx > wait_row_count bfd 1 logical_port=r0-sw2 min_rx=1000 > wait_row_count bfd 1 logical_port=r0-sw1 min_rx=1000 min_tx=1000 detect_mult=100 > > +check_engine_stats northd norecompute nocompute > +check_engine_stats bfd recompute nocompute > +check_engine_stats lflow recompute nocompute > +check_engine_stats northd_output norecompute compute > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > + > check ovn-nbctl --bfd=$uuid lr-route-add r0 100.0.0.0/8 192.168.1.2 > wait_column down bfd status logical_port=r0-sw1 > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.1.2 | grep -q bfd],[0]) > > +check_engine_stats northd recompute nocompute > +check_engine_stats bfd recompute nocompute > +check_engine_stats static_routes recompute nocompute > +check_engine_stats lflow recompute nocompute > +check_engine_stats northd_output norecompute compute > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > + > check ovn-nbctl --bfd lr-route-add r0 200.0.0.0/8 192.168.2.2 > wait_column down bfd status logical_port=r0-sw2 > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.2.2 | grep -q bfd],[0]) > @@ -3893,10 +3917,26 @@ check ovn-nbctl --bfd lr-route-add r0 240.0.0.0/8 192.168.5.2 r0-sw5 > wait_column down bfd status logical_port=r0-sw5 > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.5.2 | grep -q bfd],[0]) > > +check_engine_stats northd recompute nocompute > +check_engine_stats bfd recompute nocompute > +check_engine_stats static_routes recompute nocompute > +check_engine_stats lflow recompute nocompute > +check_engine_stats northd_output norecompute compute > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > + > check ovn-nbctl --bfd --policy=src-ip lr-route-add r0 192.168.6.1/32 192.168.10.10 r0-sw6 > wait_column down bfd status logical_port=r0-sw6 > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.6.1 | grep -q bfd],[0]) > > +check_engine_stats northd recompute nocompute > +check_engine_stats bfd recompute nocompute > +check_engine_stats route_policies recompute nocompute > +check_engine_stats lflow recompute nocompute > +check_engine_stats northd_output norecompute compute > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > + > check ovn-nbctl --bfd --policy=src-ip lr-route-add r0 192.168.7.1/32 192.168.10.10 r0-sw7 > wait_column down bfd status logical_port=r0-sw7 > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.7.1 | grep -q bfd],[0]) > @@ -3924,6 +3964,14 @@ wait_column down bfd status logical_port=r0-sw8 > bfd_route_policy_uuid=$(fetch_column nb:bfd _uuid logical_port=r0-sw8) > AT_CHECK([ovn-nbctl list logical_router_policy | grep -q $bfd_route_policy_uuid]) > > +check_engine_stats northd recompute nocompute > +check_engine_stats bfd recompute nocompute > +check_engine_stats static_routes recompute nocompute > +check_engine_stats lflow recompute nocompute > +check_engine_stats northd_output norecompute compute > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > + > check ovn-nbctl lr-policy-del r0 > check ovn-nbctl --bfd lr-policy-add r0 100 "ip4.src == 2.3.4.0/24" reroute 192.168.9.2,192.168.9.3,192.168.9.4 > > @@ -3931,6 +3979,14 @@ wait_column down bfd status dst_ip=192.168.9.2 > wait_column down bfd status dst_ip=192.168.9.3 > wait_column down bfd status dst_ip=192.168.9.4 > > +check_engine_stats northd recompute nocompute > +check_engine_stats bfd recompute nocompute > +check_engine_stats route_policies recompute nocompute > +check_engine_stats lflow recompute nocompute > +check_engine_stats northd_output norecompute compute > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > + > bfd_route_policy_uuid=$(fetch_column nb:bfd _uuid logical_port=r0-sw9) > AT_CHECK([ovn-nbctl list logical_router_policy | sed s/,//g | grep -q "$bfd_route_policy_uuid"]) > > -- > 2.45.2 >
[...] > > #endif /* EN_NORTHD_H */ > > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > > index 522236ad2..77610cdd8 100644 > > --- a/northd/inc-proc-northd.c > > +++ b/northd/inc-proc-northd.c > > @@ -155,6 +155,10 @@ static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lb_data, "lb_data"); > > static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lr_nat, "lr_nat"); > > static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lr_stateful, "lr_stateful"); > > static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(ls_stateful, "ls_stateful"); > > +static ENGINE_NODE(route_policies, "route_policies"); > > +static ENGINE_NODE(static_routes, "static_routes"); > > +static ENGINE_NODE(bfd, "bfd"); > > +static ENGINE_NODE(bfd_sync, "bfd_sync"); > > > > void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > > struct ovsdb_idl_loop *sb) > > @@ -237,18 +241,37 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > > engine_add_input(&en_fdb_aging, &en_global_config, > > node_global_config_handler); > > > > + engine_add_input(&en_bfd, &en_nb_bfd, NULL); > > + engine_add_input(&en_bfd, &en_sb_bfd, NULL); > > + engine_add_input(&en_bfd, &en_northd, bfd_northd_change_handler); > > + > > + engine_add_input(&en_route_policies, &en_bfd, NULL); > > + engine_add_input(&en_route_policies, &en_northd, > > + route_policies_northd_change_handler); > > + > > + engine_add_input(&en_static_routes, &en_bfd, NULL); > > + engine_add_input(&en_static_routes, &en_northd, > > + static_routes_northd_change_handler); > > + > > + engine_add_input(&en_bfd_sync, &en_bfd, NULL); > > + engine_add_input(&en_bfd_sync, &en_nb_bfd, NULL); > > + engine_add_input(&en_bfd_sync, &en_static_routes, NULL); > > + engine_add_input(&en_bfd_sync, &en_route_policies, NULL); > > + > > engine_add_input(&en_sync_meters, &en_nb_acl, NULL); > > engine_add_input(&en_sync_meters, &en_nb_meter, NULL); > > engine_add_input(&en_sync_meters, &en_sb_meter, NULL); > > > > - engine_add_input(&en_lflow, &en_nb_bfd, NULL); > > engine_add_input(&en_lflow, &en_nb_acl, NULL); > > engine_add_input(&en_lflow, &en_sync_meters, NULL); > > - engine_add_input(&en_lflow, &en_sb_bfd, NULL); > > engine_add_input(&en_lflow, &en_sb_logical_flow, NULL); > > engine_add_input(&en_lflow, &en_sb_multicast_group, NULL); > > engine_add_input(&en_lflow, &en_sb_igmp_group, NULL); > > engine_add_input(&en_lflow, &en_sb_logical_dp_group, NULL); > > + engine_add_input(&en_lflow, &en_bfd, NULL); > > + engine_add_input(&en_lflow, &en_bfd_sync, NULL); > > It seems en_lflow node doesn't depend on en_bfd_sync. In theory the > en_bfd_sync can be added as an input of en_northd_output. What's the > reason if it has to be added as an input of en_lflow? ack, I will fix it. I guess we can have en_bfd_sync as input of en_northd_output. > > > + engine_add_input(&en_lflow, &en_route_policies, NULL); > > + engine_add_input(&en_lflow, &en_static_routes, NULL); > > engine_add_input(&en_lflow, &en_global_config, > > node_global_config_handler); > > engine_add_input(&en_lflow, &en_northd, lflow_northd_handler); > > diff --git a/northd/northd.c b/northd/northd.c > > index 5c2fd74ff..18fe2b48a 100644 > > --- a/northd/northd.c > > +++ b/northd/northd.c > > @@ -9689,11 +9689,49 @@ 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; [...] > > This function is called by build_route_policies() which belongs to the > en_route_polices node. The bfd_connections is input data so it should > be readonly to this node and never be modified here. Otherwise, if a > node modifies its input data, it may impact other nodes that depend on > the same input node, and it is hard to get things correct in the I-P > engine. > > Similarly, I see that the bfd_table_sync() function is modifying the > en_bfd data, which is not good. en_bfd data should be modified in > en_bfd node only, when consolidating status between SB DB and NB DB, > and bfd_sync node should only update the DB, according to the data of > en_bfd, en_route_policies and en_static_routes. > > Please let me know if this is challenging. ack, I guess we can set bfd_connections entry status in check_bfd_state() (route_policies) and parsed_routes_add() (static_routes) based on en_bfd bfd hashmap entry and set NB BFD status in bfd_table_sync() according to the status value we can get from rp_bfd_connections and sr_bfd_connections (bfd_connection consumers - route_policies and static_routes). Doing so we do need to set write into structures belonging to other IP nodes. I will fix it in v10. Regards, Lorenzo > > Thanks, > Han > > > > > - ret = strcmp(nb_bt->status, "down"); > > - ovs_mutex_unlock(&bfd_lock); > > - break; > > + /* This route policy is linked to an active bfd session. */ > > + if (!bfd_port_lookup(bfd_active_connections, nb_bt->logical_port, > > + nb_bt->dst_ip)) { > > + bfd_alloc_entry(bfd_active_connections, nb_bt->logical_port, > > + nb_bt->dst_ip, bfd_e->status); > > + } > > + > > + return strcmp(bfd_e->status, "down"); > > } > > > > - return ret; > > + return true; > > } > > > > static void > > build_routing_policy_flow(struct lflow_table *lflows, struct ovn_datapath *od, > > - const struct hmap *lr_ports, > > - const struct nbrec_logical_router_policy *rule, > > - const struct hmap *bfd_connections, > > + const struct hmap *lr_ports, struct route_policy *rp, > > const struct ovsdb_idl_row *stage_hint, > > struct lflow_ref *lflow_ref) > > { > > + const struct nbrec_logical_router_policy *rule = rp->rule; > > struct ds match = DS_EMPTY_INITIALIZER; > > struct ds actions = DS_EMPTY_INITIALIZER; > > > > if (!strcmp(rule->action, "reroute")) { > > ovs_assert(rule->n_nexthops <= 1); > > > > - char *nexthop = > > - (rule->n_nexthops == 1 ? rule->nexthops[0] : rule->nexthop); > > + if (!rp->n_valid_nexthops) { > > + return; > > + } > > + > > + char *nexthop = rp->valid_nexthops[0]; > > struct ovn_port *out_port = get_outport_for_routing_policy_nexthop( > > od, lr_ports, rule->priority, nexthop); > > if (!out_port) { > > @@ -10011,10 +10078,6 @@ build_routing_policy_flow(struct lflow_table *lflows, struct ovn_datapath *od, > > return; > > } > > > > - if (!check_bfd_state(rule, bfd_connections, out_port, nexthop)) { > > - return; > > - } > > - > > uint32_t pkt_mark = smap_get_uint(&rule->options, "pkt_mark", 0); > > if (pkt_mark) { > > ds_put_format(&actions, "pkt.mark = %u; ", pkt_mark); > > @@ -10057,19 +10120,18 @@ static void > > build_ecmp_routing_policy_flows(struct lflow_table *lflows, > > struct ovn_datapath *od, > > const struct hmap *lr_ports, > > - const struct nbrec_logical_router_policy *rule, > > - const struct hmap *bfd_connections, > > + struct route_policy *rp, > > uint16_t ecmp_group_id, > > struct lflow_ref *lflow_ref) > > { > > - ovs_assert(rule->n_nexthops > 1); > > - > > bool nexthops_is_ipv4 = true; > > + const struct nbrec_logical_router_policy *rule = rp->rule; > > + ovs_assert(rule->n_nexthops > 1); > > > > /* Check that all the nexthops belong to the same addr family before > > * adding logical flows. */ > > - for (uint16_t i = 0; i < rule->n_nexthops; i++) { > > - bool is_ipv4 = strchr(rule->nexthops[i], '.') ? true : false; > > + for (uint16_t i = 0; i < rp->n_valid_nexthops; i++) { > > + bool is_ipv4 = strchr(rp->valid_nexthops[i], '.') ? true : false; > > > > if (i == 0) { > > nexthops_is_ipv4 = is_ipv4; > > @@ -10080,7 +10142,7 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, > > VLOG_WARN_RL(&rl, "nexthop [%s] of the router policy with " > > "the match [%s] do not belong to the same address " > > "family as other next hops", > > - rule->nexthops[i], rule->match); > > + rp->valid_nexthops[i], rule->match); > > return; > > } > > } > > @@ -10088,40 +10150,30 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, > > struct ds match = DS_EMPTY_INITIALIZER; > > struct ds actions = DS_EMPTY_INITIALIZER; > > > > - size_t *valid_nexthops = xcalloc(rule->n_nexthops, sizeof *valid_nexthops); > > - size_t n_valid_nexthops = 0; > > - > > - for (size_t i = 0; i < rule->n_nexthops; i++) { > > + for (size_t i = 0; i < rp->n_valid_nexthops; i++) { > > struct ovn_port *out_port = get_outport_for_routing_policy_nexthop( > > - od, lr_ports, rule->priority, rule->nexthops[i]); > > + od, lr_ports, rule->priority, rp->valid_nexthops[i]); > > if (!out_port) { > > goto cleanup; > > } > > > > const char *lrp_addr_s = > > - find_lrp_member_ip(out_port, rule->nexthops[i]); > > + find_lrp_member_ip(out_port, rp->valid_nexthops[i]); > > if (!lrp_addr_s) { > > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > > VLOG_WARN_RL(&rl, "lrp_addr not found for routing policy " > > " priority %"PRId64" nexthop %s", > > - rule->priority, rule->nexthops[i]); > > + rule->priority, rp->valid_nexthops[i]); > > goto cleanup; > > } > > > > - if (!check_bfd_state(rule, bfd_connections, out_port, > > - rule->nexthops[i])) { > > - continue; > > - } > > - > > - valid_nexthops[n_valid_nexthops++] = i + 1; > > - > > ds_clear(&actions); > > uint32_t pkt_mark = smap_get_uint(&rule->options, "pkt_mark", 0); > > if (pkt_mark) { > > ds_put_format(&actions, "pkt.mark = %u; ", pkt_mark); > > } > > > > - bool is_ipv4 = strchr(rule->nexthops[i], '.') ? true : false; > > + bool is_ipv4 = strchr(rp->valid_nexthops[i], '.') ? true : false; > > > > ds_put_format(&actions, "%s = %s; " > > "%s = %s; " > > @@ -10130,7 +10182,7 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, > > "flags.loopback = 1; " > > "next;", > > is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6, > > - rule->nexthops[i], > > + rp->valid_nexthops[i], > > is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6, > > lrp_addr_s, > > out_port->lrp_networks.ea_s, > > @@ -10146,37 +10198,30 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, > > lflow_ref); > > } > > > > - if (!n_valid_nexthops) { > > - goto cleanup; > > - } > > - > > ds_clear(&actions); > > - if (n_valid_nexthops > 1) { > > + if (rp->n_valid_nexthops > 1) { > > ds_put_format(&actions, "%s = %"PRIu16 > > "; %s = select(", REG_ECMP_GROUP_ID, ecmp_group_id, > > REG_ECMP_MEMBER_ID); > > > > - for (size_t i = 0; i < n_valid_nexthops; i++) { > > + for (size_t i = 0; i < rp->n_valid_nexthops; i++) { > > if (i > 0) { > > ds_put_cstr(&actions, ", "); > > } > > > > - ds_put_format(&actions, "%"PRIuSIZE, valid_nexthops[i]); > > + ds_put_format(&actions, "%"PRIuSIZE, i + 1); > > } > > ds_put_cstr(&actions, ");"); > > } else { > > ds_put_format(&actions, "%s = %"PRIu16 > > - "; %s = %"PRIuSIZE"; next;", REG_ECMP_GROUP_ID, > > - ecmp_group_id, REG_ECMP_MEMBER_ID, > > - valid_nexthops[0]); > > + "; %s = 1; next;", REG_ECMP_GROUP_ID, > > + ecmp_group_id, REG_ECMP_MEMBER_ID); > > } > > ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_POLICY, > > rule->priority, rule->match, > > ds_cstr(&actions), &rule->header_, > > lflow_ref); > > - > > cleanup: > > - free(valid_nexthops); > > ds_destroy(&match); > > ds_destroy(&actions); > > } > > @@ -10201,7 +10246,7 @@ route_table_add(struct simap *route_tables, const char *route_table_name) > > return rtb_id; > > } > > > > -static uint32_t > > +uint32_t > > get_route_table_id(struct simap *route_tables, const char *route_table_name) > > { > > if (!route_table_name || !route_table_name[0]) { > > @@ -10242,18 +10287,6 @@ build_route_table_lflow(struct ovn_datapath *od, struct lflow_table *lflows, > > ds_destroy(&actions); > > } > > > > -struct parsed_route { > > - struct ovs_list list_node; > > - struct in6_addr prefix; > > - unsigned int plen; > > - bool is_src_route; > > - uint32_t route_table_id; > > - uint32_t hash; > > - const struct nbrec_logical_router_static_route *route; > > - bool ecmp_symmetric_reply; > > - bool is_discard_route; > > -}; > > - > > static uint32_t > > route_hash(struct parsed_route *route) > > { > > @@ -10268,11 +10301,53 @@ find_static_route_outport(struct ovn_datapath *od, const struct hmap *lr_ports, > > > > /* Parse and validate the route. Return the parsed route if successful. > > * Otherwise return NULL. */ > > + > > static struct parsed_route * > > +parsed_route_lookup(struct hmap *routes, size_t hash, > > + struct parsed_route *new_pr) > > +{ > > + struct parsed_route *pr; > > + HMAP_FOR_EACH_WITH_HASH (pr, key_node, hash, routes) { > > + if (pr->plen != new_pr->plen) { > > + continue; > > + } > > + > > + if (memcmp(&pr->prefix, &new_pr->prefix, sizeof(struct in6_addr))) { > > + continue; > > + } > > + > > + if (pr->is_src_route != new_pr->is_src_route) { > > + continue; > > + } > > + > > + if (pr->route_table_id != new_pr->route_table_id) { > > + continue; > > + } > > + > > + if (pr->route != new_pr->route) { > > + continue; > > + } > > + > > + if (pr->ecmp_symmetric_reply != new_pr->ecmp_symmetric_reply) { > > + continue; > > + } > > + > > + if (pr->is_discard_route != new_pr->is_discard_route) { > > + continue; > > + } > > + > > + return pr; > > + } > > + > > + return NULL; > > +} > > + > > +static void > > parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > > - struct ovs_list *routes, struct simap *route_tables, > > + struct hmap *routes, struct simap *route_tables, > > const struct nbrec_logical_router_static_route *route, > > - const struct hmap *bfd_connections) > > + struct hmap *bfd_active_connections, > > + struct hmap *bfd_connections) > > { > > /* Verify that the next hop is an IP address with an all-ones mask. */ > > struct in6_addr nexthop; > > @@ -10285,7 +10360,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > > VLOG_WARN_RL(&rl, "bad 'nexthop' %s in static route " > > UUID_FMT, route->nexthop, > > UUID_ARGS(&route->header_.uuid)); > > - return NULL; > > + return; > > } > > if ((IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 32) || > > (!IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 128)) { > > @@ -10293,7 +10368,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > > VLOG_WARN_RL(&rl, "bad next hop mask %s in static route " > > UUID_FMT, route->nexthop, > > UUID_ARGS(&route->header_.uuid)); > > - return NULL; > > + return; > > } > > } > > > > @@ -10304,7 +10379,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > > VLOG_WARN_RL(&rl, "bad 'ip_prefix' %s in static route " > > UUID_FMT, route->ip_prefix, > > UUID_ARGS(&route->header_.uuid)); > > - return NULL; > > + return; > > } > > > > /* Verify that ip_prefix and nexthop have same address familiy. */ > > @@ -10315,7 +10390,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > > " %s and 'nexthop' %s in static route "UUID_FMT, > > route->ip_prefix, route->nexthop, > > UUID_ARGS(&route->header_.uuid)); > > - return NULL; > > + return; > > } > > } > > > > @@ -10324,52 +10399,85 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > > !find_static_route_outport(od, lr_ports, route, > > IN6_IS_ADDR_V4MAPPED(&prefix), > > NULL, NULL)) { > > - return NULL; > > + return; > > } > > > > const struct nbrec_bfd *nb_bt = route->bfd; > > if (nb_bt && !strcmp(nb_bt->dst_ip, route->nexthop)) { > > - struct bfd_entry *bfd_e; > > + struct bfd_entry *bfd_e = bfd_port_lookup(bfd_connections, > > + nb_bt->logical_port, > > + nb_bt->dst_ip); > > + if (!bfd_e) { > > + return; > > + } > > > > - bfd_e = bfd_port_lookup(bfd_connections, nb_bt->logical_port, > > - nb_bt->dst_ip); > > - ovs_mutex_lock(&bfd_lock); > > - if (bfd_e) { > > - bfd_e->ref = true; > > + if (!strcmp(bfd_e->status, "admin_down")) { > > + bfd_set_status(bfd_e, "down"); > > } > > > > - if (!strcmp(nb_bt->status, "admin_down")) { > > - nbrec_bfd_set_status(nb_bt, "down"); > > + /* This static route is linked to an active bfd session. */ > > + if (!bfd_port_lookup(bfd_active_connections, nb_bt->logical_port, > > + nb_bt->dst_ip)) { > > + bfd_alloc_entry(bfd_active_connections, nb_bt->logical_port, > > + nb_bt->dst_ip, bfd_e->status); > > } > > > > - if (!strcmp(nb_bt->status, "down")) { > > - ovs_mutex_unlock(&bfd_lock); > > - return NULL; > > + if (!strcmp(bfd_e->status, "down")) { > > + return; > > } > > - ovs_mutex_unlock(&bfd_lock); > > } > > > > - struct parsed_route *pr = xzalloc(sizeof *pr); > > - pr->prefix = prefix; > > - pr->plen = plen; > > - pr->route_table_id = get_route_table_id(route_tables, route->route_table); > > - pr->is_src_route = (route->policy && !strcmp(route->policy, > > - "src-ip")); > > - pr->hash = route_hash(pr); > > - pr->route = route; > > - pr->ecmp_symmetric_reply = smap_get_bool(&route->options, > > - "ecmp_symmetric_reply", false); > > - pr->is_discard_route = is_discard_route; > > - ovs_list_insert(routes, &pr->list_node); > > - return pr; > > + struct parsed_route *new_pr = xzalloc(sizeof *new_pr); > > + new_pr->prefix = prefix; > > + new_pr->plen = plen; > > + new_pr->route_table_id = get_route_table_id(route_tables, > > + route->route_table); > > + new_pr->is_src_route = (route->policy && > > + !strcmp(route->policy, "src-ip")); > > + new_pr->hash = route_hash(new_pr); > > + new_pr->route = route; > > + new_pr->nbr = od->nbr; > > + new_pr->ecmp_symmetric_reply = smap_get_bool(&route->options, > > + "ecmp_symmetric_reply", > > + false); > > + new_pr->is_discard_route = is_discard_route; > > + > > + size_t hash = uuid_hash(&od->key); > > + struct parsed_route *pr = parsed_route_lookup(routes, hash, new_pr); > > + if (!pr) { > > + hmap_insert(routes, &new_pr->key_node, hash); > > + } else { > > + pr->stale = false; > > + free(new_pr); > > + } > > } > > > > -static void > > -parsed_routes_destroy(struct ovs_list *routes) > > +void > > +build_parsed_routes(struct ovn_datapath *od, const struct hmap *lr_ports, > > + struct hmap *routes, struct simap *route_tables, > > + struct hmap *bfd_active_connections, > > + struct hmap *bfd_connections) > > { > > struct parsed_route *pr; > > - LIST_FOR_EACH_SAFE (pr, list_node, routes) { > > - ovs_list_remove(&pr->list_node); > > + HMAP_FOR_EACH (pr, key_node, routes) { > > + if (pr->nbr == od->nbr) { > > + pr->stale = true; > > + } > > + } > > + > > + for (int i = 0; i < od->nbr->n_static_routes; i++) { > > + parsed_routes_add(od, lr_ports, routes, route_tables, > > + od->nbr->static_routes[i], > > + bfd_active_connections, > > + bfd_connections); > > + } > > + > > + HMAP_FOR_EACH_SAFE (pr, key_node, routes) { > > + if (!pr->stale) { > > + continue; > > + } > > + > > + hmap_remove(routes, &pr->key_node); > > free(pr); > > } > > } > > @@ -12671,8 +12779,8 @@ build_ip_routing_flows_for_lrp( > > static void > > build_static_route_flows_for_lrouter( > > struct ovn_datapath *od, struct lflow_table *lflows, > > - const struct hmap *lr_ports, const struct hmap *bfd_connections, > > - struct lflow_ref *lflow_ref) > > + const struct hmap *lr_ports, struct hmap *parsed_routes, > > + struct simap *route_tables, struct lflow_ref *lflow_ref) > > { > > ovs_assert(od->nbr); > > ovn_lflow_add_default_drop(lflows, od, S_ROUTER_IN_IP_ROUTING_ECMP, > > @@ -12685,22 +12793,16 @@ build_static_route_flows_for_lrouter( > > > > struct hmap ecmp_groups = HMAP_INITIALIZER(&ecmp_groups); > > struct hmap unique_routes = HMAP_INITIALIZER(&unique_routes); > > - struct ovs_list parsed_routes = OVS_LIST_INITIALIZER(&parsed_routes); > > - struct simap route_tables = SIMAP_INITIALIZER(&route_tables); > > struct ecmp_groups_node *group; > > > > for (int i = 0; i < od->nbr->n_ports; i++) { > > build_route_table_lflow(od, lflows, od->nbr->ports[i], > > - &route_tables, lflow_ref); > > + route_tables, lflow_ref); > > } > > > > - for (int i = 0; i < od->nbr->n_static_routes; i++) { > > - struct parsed_route *route = > > - parsed_routes_add(od, lr_ports, &parsed_routes, &route_tables, > > - od->nbr->static_routes[i], bfd_connections); > > - if (!route) { > > - continue; > > - } > > + struct parsed_route *route; > > + HMAP_FOR_EACH_WITH_HASH (route, key_node, uuid_hash(&od->key), > > + parsed_routes) { > > group = ecmp_groups_find(&ecmp_groups, route); > > if (group) { > > ecmp_groups_add_route(group, route); > > @@ -12728,8 +12830,6 @@ build_static_route_flows_for_lrouter( > > } > > ecmp_groups_destroy(&ecmp_groups); > > unique_routes_destroy(&unique_routes); > > - parsed_routes_destroy(&parsed_routes); > > - simap_destroy(&route_tables); > > } > > > > /* IP Multicast lookup. Here we set the output port, adjust TTL and > > @@ -12836,6 +12936,115 @@ build_mcast_lookup_flows_for_lrouter( > > } > > } > > > > +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->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; > > +} > > + > > +void > > +build_route_policies(struct ovn_datapath *od, struct hmap *lr_ports, > > + struct hmap *route_policies, > > + struct hmap *bfd_active_connections, > > + struct hmap *bfd_connections) > > +{ > > + struct route_policy *rp; > > + > > + HMAP_FOR_EACH (rp, key_node, route_policies) { > > + if (rp->nbr == od->nbr) { > > + rp->stale = true; > > + } > > + } > > + > > + for (int i = 0; i < od->nbr->n_policies; i++) { > > + const struct nbrec_logical_router_policy *rule = od->nbr->policies[i]; > > + size_t n_valid_nexthops = 0; > > + char **valid_nexthops = NULL; > > + > > + if (!strcmp(rule->action, "reroute")) { > > + size_t n_nexthops = rule->n_nexthops ? rule->n_nexthops : 1; > > + > > + valid_nexthops = xcalloc(n_nexthops, sizeof *valid_nexthops); > > + for (size_t j = 0; j < n_nexthops; j++) { > > + char *nexthop = rule->n_nexthops > > + ? rule->nexthops[j] : rule->nexthop; > > + struct ovn_port *out_port = > > + get_outport_for_routing_policy_nexthop( > > + od, lr_ports, rule->priority, nexthop); > > + if (!out_port || !check_bfd_state(rule, out_port, nexthop, > > + bfd_active_connections, > > + bfd_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->nbr = od->nbr; > > + > > + 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 { > > + 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. > > * > > * A packet that arrives at this table is an IP packet that should be > > @@ -12849,7 +13058,7 @@ static void > > build_ingress_policy_flows_for_lrouter( > > struct ovn_datapath *od, struct lflow_table *lflows, > > const struct hmap *lr_ports, > > - const struct hmap *bfd_connections, > > + struct hmap *route_policies, > > struct lflow_ref *lflow_ref) > > { > > ovs_assert(od->nbr); > > @@ -12866,21 +13075,20 @@ build_ingress_policy_flows_for_lrouter( > > > > /* Convert routing policies to flows. */ > > uint16_t ecmp_group_id = 1; > > - for (int i = 0; i < od->nbr->n_policies; i++) { > > - const struct nbrec_logical_router_policy *rule > > - = od->nbr->policies[i]; > > + struct route_policy *rp; > > + HMAP_FOR_EACH_WITH_HASH (rp, key_node, uuid_hash(&od->key), > > + route_policies) { > > + const struct nbrec_logical_router_policy *rule = rp->rule; > > bool is_ecmp_reroute = > > (!strcmp(rule->action, "reroute") && rule->n_nexthops > 1); > > > > if (is_ecmp_reroute) { > > - build_ecmp_routing_policy_flows(lflows, od, lr_ports, rule, > > - bfd_connections, ecmp_group_id, > > - lflow_ref); > > + build_ecmp_routing_policy_flows(lflows, od, lr_ports, rp, > > + ecmp_group_id, lflow_ref); > > ecmp_group_id++; > > } else { > > - build_routing_policy_flow(lflows, od, lr_ports, rule, > > - bfd_connections, &rule->header_, > > - lflow_ref); > > + build_routing_policy_flow(lflows, od, lr_ports, rp, > > + &rule->header_, lflow_ref); > > } > > } > > } > > @@ -15805,6 +16013,9 @@ struct lswitch_flow_build_info { > > struct ds actions; > > size_t thread_lflow_counter; > > const char *svc_monitor_mac; > > + struct hmap *parsed_routes; > > + struct hmap *route_policies; > > + struct simap *route_tables; > > }; > > > > /* Helper function to combine all lflow generation which is iterated by > > @@ -15850,12 +16061,12 @@ build_lswitch_and_lrouter_iterate_by_lr(struct ovn_datapath *od, > > build_ND_RA_flows_for_lrouter(od, lsi->lflows, NULL); > > build_ip_routing_pre_flows_for_lrouter(od, lsi->lflows, NULL); > > build_static_route_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports, > > - lsi->bfd_connections, > > + lsi->parsed_routes, lsi->route_tables, > > NULL); > > build_mcast_lookup_flows_for_lrouter(od, lsi->lflows, &lsi->match, > > &lsi->actions, NULL); > > build_ingress_policy_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports, > > - lsi->bfd_connections, NULL); > > + lsi->route_policies, NULL); > > build_arp_resolve_flows_for_lrouter(od, lsi->lflows, NULL); > > build_check_pkt_len_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports, > > &lsi->match, &lsi->actions, > > @@ -16169,7 +16380,10 @@ build_lswitch_and_lrouter_flows( > > const struct hmap *svc_monitor_map, > > const struct hmap *bfd_connections, > > const struct chassis_features *features, > > - const char *svc_monitor_mac) > > + const char *svc_monitor_mac, > > + struct hmap *parsed_routes, > > + struct hmap *route_policies, > > + struct simap *route_tables) > > { > > > > char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac); > > @@ -16203,6 +16417,9 @@ build_lswitch_and_lrouter_flows( > > lsiv[index].svc_check_match = svc_check_match; > > lsiv[index].thread_lflow_counter = 0; > > lsiv[index].svc_monitor_mac = svc_monitor_mac; > > + lsiv[index].parsed_routes = parsed_routes; > > + lsiv[index].route_tables = route_tables; > > + lsiv[index].route_policies = route_policies; > > ds_init(&lsiv[index].match); > > ds_init(&lsiv[index].actions); > > > > @@ -16243,6 +16460,9 @@ build_lswitch_and_lrouter_flows( > > .features = features, > > .svc_check_match = svc_check_match, > > .svc_monitor_mac = svc_monitor_mac, > > + .parsed_routes = parsed_routes, > > + .route_tables = route_tables, > > + .route_policies = route_policies, > > .match = DS_EMPTY_INITIALIZER, > > .actions = DS_EMPTY_INITIALIZER, > > }; > > @@ -16404,7 +16624,10 @@ void build_lflows(struct ovsdb_idl_txn *ovnsb_txn, > > input_data->svc_monitor_map, > > input_data->bfd_connections, > > input_data->features, > > - input_data->svc_monitor_mac); > > + input_data->svc_monitor_mac, > > + input_data->parsed_routes, > > + input_data->route_policies, > > + input_data->route_tables); > > > > if (parallelization_state == STATE_INIT_HASH_SIZES) { > > parallelization_state = STATE_USE_PARALLELIZATION; > > @@ -17477,6 +17700,27 @@ 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); > > +} > > + > > +void > > +static_routes_init(struct static_routes_data *data) > > +{ > > + hmap_init(&data->parsed_routes); > > + simap_init(&data->route_tables); > > + hmap_init(&data->bfd_active_connections); > > +} > > + > > +void > > +bfd_init(struct bfd_data *data) > > +{ > > + hmap_init(&data->bfd_connections); > > +} > > + > > void > > northd_destroy(struct northd_data *data) > > { > > @@ -17516,6 +17760,48 @@ northd_destroy(struct northd_data *data) > > destroy_northd_tracked_data(data); > > } > > > > +static void > > +__bfd_destroy(struct hmap *bfd_connections) > > +{ > > + struct bfd_entry *bfd_e; > > + > > + HMAP_FOR_EACH_POP (bfd_e, hmap_node, bfd_connections) { > > + bfd_erase_entry(bfd_e); > > + } > > + hmap_destroy(bfd_connections); > > +} > > + > > +void > > +bfd_destroy(struct bfd_data *data) > > +{ > > + __bfd_destroy(&data->bfd_connections); > > +} > > + > > +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); > > +} > > + > > +void > > +static_routes_destroy(struct static_routes_data *data) > > +{ > > + struct parsed_route *r; > > + HMAP_FOR_EACH_POP (r, key_node, &data->parsed_routes) { > > + free(r); > > + } > > + hmap_destroy(&data->parsed_routes); > > + > > + simap_destroy(&data->route_tables); > > + __bfd_destroy(&data->bfd_active_connections); > > +} > > + > > void > > ovnnb_db_run(struct northd_input *input_data, > > struct northd_data *data, > > diff --git a/northd/northd.h b/northd/northd.h > > index d4a8d75ab..7ddb72eb7 100644 > > --- a/northd/northd.h > > +++ b/northd/northd.h > > @@ -23,6 +23,7 @@ > > #include "northd/en-port-group.h" > > #include "northd/ipam.h" > > #include "openvswitch/hmap.h" > > +#include "simap.h" > > #include "ovs-thread.h" > > > > struct northd_input { > > @@ -160,14 +161,34 @@ 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; > > + const struct nbrec_logical_router *nbr; > > + bool stale; > > +}; > > + > > +struct static_routes_data { > > + struct hmap parsed_routes; > > + struct simap route_tables; > > + struct hmap bfd_active_connections; > > +}; > > + > > +struct route_policies_data { > > + struct hmap route_policies; > > + struct hmap bfd_active_connections; > > +}; > > + > > +struct bfd_data { > > + struct hmap bfd_connections; > > +}; > > + > > struct lr_nat_table; > > > > struct lflow_input { > > - /* Northbound table references */ > > - const struct nbrec_bfd_table *nbrec_bfd_table; > > - > > /* Southbound table references */ > > - const struct sbrec_bfd_table *sbrec_bfd_table; > > const struct sbrec_logical_flow_table *sbrec_logical_flow_table; > > const struct sbrec_multicast_group_table *sbrec_multicast_group_table; > > const struct sbrec_igmp_group_table *sbrec_igmp_group_table; > > @@ -190,6 +211,9 @@ struct lflow_input { > > const struct hmap *svc_monitor_map; > > bool ovn_internal_version_changed; > > const char *svc_monitor_mac; > > + struct hmap *parsed_routes; > > + struct hmap *route_policies; > > + struct simap *route_tables; > > }; > > > > extern int parallelization_state; > > @@ -653,6 +677,20 @@ struct ovn_port { > > struct lflow_ref *stateful_lflow_ref; > > }; > > > > +struct parsed_route { > > + struct hmap_node key_node; > > + struct in6_addr prefix; > > + unsigned int plen; > > + bool is_src_route; > > + uint32_t route_table_id; > > + uint32_t hash; > > + const struct nbrec_logical_router_static_route *route; > > + bool ecmp_symmetric_reply; > > + bool is_discard_route; > > + const struct nbrec_logical_router *nbr; > > + bool stale; > > +}; > > + > > void ovnnb_db_run(struct northd_input *input_data, > > struct northd_data *data, > > struct ovsdb_idl_txn *ovnnb_txn, > > @@ -674,6 +712,18 @@ 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(struct ovn_datapath *, const struct hmap *, > > + struct hmap *, struct simap *, struct hmap *, > > + struct hmap *); > > +uint32_t get_route_table_id(struct simap *, const char *); > > +void static_routes_init(struct static_routes_data *); > > +void static_routes_destroy(struct static_routes_data *); > > + > > +void bfd_init(struct bfd_data *); > > +void bfd_destroy(struct bfd_data *); > > + > > struct lflow_table; > > struct lr_stateful_tracked_data; > > struct ls_stateful_tracked_data; > > @@ -711,13 +761,14 @@ bool northd_handle_lb_data_changes(struct tracked_lb_data *, > > struct hmap *lbgrp_datapaths_map, > > struct northd_tracked_data *); > > > > -void build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn, > > - const struct nbrec_bfd_table *, > > - const struct sbrec_bfd_table *, > > - const struct hmap *lr_ports, > > - struct hmap *bfd_connections); > > -void bfd_cleanup_connections(const struct nbrec_bfd_table *, > > - struct hmap *bfd_map); > > +void build_route_policies(struct ovn_datapath *, struct hmap *, struct hmap *, > > + struct hmap *, struct hmap *); > > +void bfd_table_sync(struct ovsdb_idl_txn *, const struct nbrec_bfd_table *, > > + struct hmap *, struct hmap *, struct hmap *); > > +void build_bfd_map(const struct nbrec_bfd_table *, > > + const struct sbrec_bfd_table *, > > + const struct hmap *lr_ports, > > + struct hmap *bfd_connections); > > void run_update_worker_pool(int n_threads); > > > > const struct ovn_datapath *northd_get_datapath_for_port( > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > > index 57f89a774..1d7f55e4f 100644 > > --- a/tests/ovn-northd.at > > +++ b/tests/ovn-northd.at > > @@ -12,6 +12,7 @@ m4_define([_DUMP_DB_TABLES], [ > > ovn-sbctl list meter >> $1 > > ovn-sbctl list meter_band >> $1 > > ovn-sbctl list port_group >> $1 > > + ovn-sbctl list bfd >> $1 > > ovn-sbctl dump-flows > lflows_$1 > > ]) > > > > @@ -3861,6 +3862,7 @@ for i in $(seq 1 9); do > > check ovn-nbctl --wait=sb lsp-set-addresses sw$i-r0 00:00:00:00:00:0$i > > done > > > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > uuid=$(ovn-nbctl create bfd logical_port=r0-sw1 dst_ip=192.168.1.2 status=down min_tx=250 min_rx=250 detect_mult=10) > > ovn-nbctl create bfd logical_port=r0-sw2 dst_ip=192.168.2.2 status=down min_tx=500 min_rx=500 detect_mult=20 > > ovn-nbctl create bfd logical_port=r0-sw3 dst_ip=192.168.3.2 status=down > > @@ -3873,6 +3875,13 @@ wait_row_count bfd 1 logical_port=r0-sw2 detect_mult=20 dst_ip=192.168.2.2 \ > > wait_row_count bfd 1 logical_port=r0-sw3 detect_mult=5 dst_ip=192.168.3.2 \ > > min_rx=1000 min_tx=1000 status=admin_down > > > > +check_engine_stats northd norecompute nocompute > > +check_engine_stats bfd recompute nocompute > > +check_engine_stats lflow recompute nocompute > > +check_engine_stats northd_output norecompute compute > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > + > > uuid=$(fetch_column nb:bfd _uuid logical_port=r0-sw1) > > check ovn-nbctl set bfd $uuid min_tx=1000 min_rx=1000 detect_mult=100 > > > > @@ -3881,10 +3890,25 @@ check ovn-nbctl clear bfd $uuid_2 min_rx > > wait_row_count bfd 1 logical_port=r0-sw2 min_rx=1000 > > wait_row_count bfd 1 logical_port=r0-sw1 min_rx=1000 min_tx=1000 detect_mult=100 > > > > +check_engine_stats northd norecompute nocompute > > +check_engine_stats bfd recompute nocompute > > +check_engine_stats lflow recompute nocompute > > +check_engine_stats northd_output norecompute compute > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > + > > check ovn-nbctl --bfd=$uuid lr-route-add r0 100.0.0.0/8 192.168.1.2 > > wait_column down bfd status logical_port=r0-sw1 > > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.1.2 | grep -q bfd],[0]) > > > > +check_engine_stats northd recompute nocompute > > +check_engine_stats bfd recompute nocompute > > +check_engine_stats static_routes recompute nocompute > > +check_engine_stats lflow recompute nocompute > > +check_engine_stats northd_output norecompute compute > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > + > > check ovn-nbctl --bfd lr-route-add r0 200.0.0.0/8 192.168.2.2 > > wait_column down bfd status logical_port=r0-sw2 > > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.2.2 | grep -q bfd],[0]) > > @@ -3893,10 +3917,26 @@ check ovn-nbctl --bfd lr-route-add r0 240.0.0.0/8 192.168.5.2 r0-sw5 > > wait_column down bfd status logical_port=r0-sw5 > > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.5.2 | grep -q bfd],[0]) > > > > +check_engine_stats northd recompute nocompute > > +check_engine_stats bfd recompute nocompute > > +check_engine_stats static_routes recompute nocompute > > +check_engine_stats lflow recompute nocompute > > +check_engine_stats northd_output norecompute compute > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > + > > check ovn-nbctl --bfd --policy=src-ip lr-route-add r0 192.168.6.1/32 192.168.10.10 r0-sw6 > > wait_column down bfd status logical_port=r0-sw6 > > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.6.1 | grep -q bfd],[0]) > > > > +check_engine_stats northd recompute nocompute > > +check_engine_stats bfd recompute nocompute > > +check_engine_stats route_policies recompute nocompute > > +check_engine_stats lflow recompute nocompute > > +check_engine_stats northd_output norecompute compute > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > + > > check ovn-nbctl --bfd --policy=src-ip lr-route-add r0 192.168.7.1/32 192.168.10.10 r0-sw7 > > wait_column down bfd status logical_port=r0-sw7 > > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.7.1 | grep -q bfd],[0]) > > @@ -3924,6 +3964,14 @@ wait_column down bfd status logical_port=r0-sw8 > > bfd_route_policy_uuid=$(fetch_column nb:bfd _uuid logical_port=r0-sw8) > > AT_CHECK([ovn-nbctl list logical_router_policy | grep -q $bfd_route_policy_uuid]) > > > > +check_engine_stats northd recompute nocompute > > +check_engine_stats bfd recompute nocompute > > +check_engine_stats static_routes recompute nocompute > > +check_engine_stats lflow recompute nocompute > > +check_engine_stats northd_output norecompute compute > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > + > > check ovn-nbctl lr-policy-del r0 > > check ovn-nbctl --bfd lr-policy-add r0 100 "ip4.src == 2.3.4.0/24" reroute 192.168.9.2,192.168.9.3,192.168.9.4 > > > > @@ -3931,6 +3979,14 @@ wait_column down bfd status dst_ip=192.168.9.2 > > wait_column down bfd status dst_ip=192.168.9.3 > > wait_column down bfd status dst_ip=192.168.9.4 > > > > +check_engine_stats northd recompute nocompute > > +check_engine_stats bfd recompute nocompute > > +check_engine_stats route_policies recompute nocompute > > +check_engine_stats lflow recompute nocompute > > +check_engine_stats northd_output norecompute compute > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > + > > bfd_route_policy_uuid=$(fetch_column nb:bfd _uuid logical_port=r0-sw9) > > AT_CHECK([ovn-nbctl list logical_router_policy | sed s/,//g | grep -q "$bfd_route_policy_uuid"]) > > > > -- > > 2.45.2 > > >
On Wed, Jul 31, 2024 at 8:42 AM Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote: > > [...] > > > #endif /* EN_NORTHD_H */ > > > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > > > index 522236ad2..77610cdd8 100644 > > > --- a/northd/inc-proc-northd.c > > > +++ b/northd/inc-proc-northd.c > > > @@ -155,6 +155,10 @@ static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lb_data, "lb_data"); > > > static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lr_nat, "lr_nat"); > > > static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lr_stateful, "lr_stateful"); > > > static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(ls_stateful, "ls_stateful"); > > > +static ENGINE_NODE(route_policies, "route_policies"); > > > +static ENGINE_NODE(static_routes, "static_routes"); > > > +static ENGINE_NODE(bfd, "bfd"); > > > +static ENGINE_NODE(bfd_sync, "bfd_sync"); > > > > > > void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > > > struct ovsdb_idl_loop *sb) > > > @@ -237,18 +241,37 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > > > engine_add_input(&en_fdb_aging, &en_global_config, > > > node_global_config_handler); > > > > > > + engine_add_input(&en_bfd, &en_nb_bfd, NULL); > > > + engine_add_input(&en_bfd, &en_sb_bfd, NULL); > > > + engine_add_input(&en_bfd, &en_northd, bfd_northd_change_handler); > > > + > > > + engine_add_input(&en_route_policies, &en_bfd, NULL); > > > + engine_add_input(&en_route_policies, &en_northd, > > > + route_policies_northd_change_handler); > > > + > > > + engine_add_input(&en_static_routes, &en_bfd, NULL); > > > + engine_add_input(&en_static_routes, &en_northd, > > > + static_routes_northd_change_handler); > > > + > > > + engine_add_input(&en_bfd_sync, &en_bfd, NULL); > > > + engine_add_input(&en_bfd_sync, &en_nb_bfd, NULL); > > > + engine_add_input(&en_bfd_sync, &en_static_routes, NULL); > > > + engine_add_input(&en_bfd_sync, &en_route_policies, NULL); > > > + > > > engine_add_input(&en_sync_meters, &en_nb_acl, NULL); > > > engine_add_input(&en_sync_meters, &en_nb_meter, NULL); > > > engine_add_input(&en_sync_meters, &en_sb_meter, NULL); > > > > > > - engine_add_input(&en_lflow, &en_nb_bfd, NULL); > > > engine_add_input(&en_lflow, &en_nb_acl, NULL); > > > engine_add_input(&en_lflow, &en_sync_meters, NULL); > > > - engine_add_input(&en_lflow, &en_sb_bfd, NULL); > > > engine_add_input(&en_lflow, &en_sb_logical_flow, NULL); > > > engine_add_input(&en_lflow, &en_sb_multicast_group, NULL); > > > engine_add_input(&en_lflow, &en_sb_igmp_group, NULL); > > > engine_add_input(&en_lflow, &en_sb_logical_dp_group, NULL); > > > + engine_add_input(&en_lflow, &en_bfd, NULL); > > > + engine_add_input(&en_lflow, &en_bfd_sync, NULL); > > > > It seems en_lflow node doesn't depend on en_bfd_sync. In theory the > > en_bfd_sync can be added as an input of en_northd_output. What's the > > reason if it has to be added as an input of en_lflow? > > ack, I will fix it. I guess we can have en_bfd_sync as input of > en_northd_output. I tried it but at least one test case failed: check BFD config propagation to SBDB I didn't debug the failure yet. Probably some other hidden dependency that we need to sort out. > > > > > > + engine_add_input(&en_lflow, &en_route_policies, NULL); > > > + engine_add_input(&en_lflow, &en_static_routes, NULL); > > > engine_add_input(&en_lflow, &en_global_config, > > > node_global_config_handler); > > > engine_add_input(&en_lflow, &en_northd, lflow_northd_handler); > > > diff --git a/northd/northd.c b/northd/northd.c > > > index 5c2fd74ff..18fe2b48a 100644 > > > --- a/northd/northd.c > > > +++ b/northd/northd.c > > > @@ -9689,11 +9689,49 @@ 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; > > [...] > > > > > This function is called by build_route_policies() which belongs to the > > en_route_polices node. The bfd_connections is input data so it should > > be readonly to this node and never be modified here. Otherwise, if a > > node modifies its input data, it may impact other nodes that depend on > > the same input node, and it is hard to get things correct in the I-P > > engine. > > > > Similarly, I see that the bfd_table_sync() function is modifying the > > en_bfd data, which is not good. en_bfd data should be modified in > > en_bfd node only, when consolidating status between SB DB and NB DB, > > and bfd_sync node should only update the DB, according to the data of > > en_bfd, en_route_policies and en_static_routes. > > > > Please let me know if this is challenging. > > ack, I guess we can set bfd_connections entry status in check_bfd_state() I guess you are saying we can set rp_bfd_connections and sr_bfd_connections entry status instead of directly setting bfd_connections entry status in those functions that don't belong to en_bfd? > (route_policies) and parsed_routes_add() (static_routes) based on en_bfd > bfd hashmap entry and set NB BFD status in bfd_table_sync() according to > the status value we can get from rp_bfd_connections and sr_bfd_connections > (bfd_connection consumers - route_policies and static_routes). Doing so we > do need to set write into structures belonging to other IP nodes. Just to confirm, are you saying s/do need to/don't need to? :) > I will fix it in v10. Thanks! In addition, could you try to reorder the related function parameters so that input (readonly) parameters are in front of output, such as: build_route_policies(), build_parsed_routes(), check_bfd_state(), parsed_routes_add(). In these functions bfd_data->bfd_connections are put at last but it is better to put it in front of the output parameters of the functions because bfd_data->bfd_connections is input and readonly to those functions. (This has been an agreement but may not be strictly followed in many places.) Thanks, Han > > Regards, > Lorenzo > > > > Thanks, > > Han > > > > > > > > - ret = strcmp(nb_bt->status, "down"); > > > - ovs_mutex_unlock(&bfd_lock); > > > - break; > > > + /* This route policy is linked to an active bfd session. */ > > > + if (!bfd_port_lookup(bfd_active_connections, nb_bt->logical_port, > > > + nb_bt->dst_ip)) { > > > + bfd_alloc_entry(bfd_active_connections, nb_bt->logical_port, > > > + nb_bt->dst_ip, bfd_e->status); > > > + } > > > + > > > + return strcmp(bfd_e->status, "down"); > > > } > > > > > > - return ret; > > > + return true; > > > } > > > > > > static void > > > build_routing_policy_flow(struct lflow_table *lflows, struct ovn_datapath *od, > > > - const struct hmap *lr_ports, > > > - const struct nbrec_logical_router_policy *rule, > > > - const struct hmap *bfd_connections, > > > + const struct hmap *lr_ports, struct route_policy *rp, > > > const struct ovsdb_idl_row *stage_hint, > > > struct lflow_ref *lflow_ref) > > > { > > > + const struct nbrec_logical_router_policy *rule = rp->rule; > > > struct ds match = DS_EMPTY_INITIALIZER; > > > struct ds actions = DS_EMPTY_INITIALIZER; > > > > > > if (!strcmp(rule->action, "reroute")) { > > > ovs_assert(rule->n_nexthops <= 1); > > > > > > - char *nexthop = > > > - (rule->n_nexthops == 1 ? rule->nexthops[0] : rule->nexthop); > > > + if (!rp->n_valid_nexthops) { > > > + return; > > > + } > > > + > > > + char *nexthop = rp->valid_nexthops[0]; > > > struct ovn_port *out_port = get_outport_for_routing_policy_nexthop( > > > od, lr_ports, rule->priority, nexthop); > > > if (!out_port) { > > > @@ -10011,10 +10078,6 @@ build_routing_policy_flow(struct lflow_table *lflows, struct ovn_datapath *od, > > > return; > > > } > > > > > > - if (!check_bfd_state(rule, bfd_connections, out_port, nexthop)) { > > > - return; > > > - } > > > - > > > uint32_t pkt_mark = smap_get_uint(&rule->options, "pkt_mark", 0); > > > if (pkt_mark) { > > > ds_put_format(&actions, "pkt.mark = %u; ", pkt_mark); > > > @@ -10057,19 +10120,18 @@ static void > > > build_ecmp_routing_policy_flows(struct lflow_table *lflows, > > > struct ovn_datapath *od, > > > const struct hmap *lr_ports, > > > - const struct nbrec_logical_router_policy *rule, > > > - const struct hmap *bfd_connections, > > > + struct route_policy *rp, > > > uint16_t ecmp_group_id, > > > struct lflow_ref *lflow_ref) > > > { > > > - ovs_assert(rule->n_nexthops > 1); > > > - > > > bool nexthops_is_ipv4 = true; > > > + const struct nbrec_logical_router_policy *rule = rp->rule; > > > + ovs_assert(rule->n_nexthops > 1); > > > > > > /* Check that all the nexthops belong to the same addr family before > > > * adding logical flows. */ > > > - for (uint16_t i = 0; i < rule->n_nexthops; i++) { > > > - bool is_ipv4 = strchr(rule->nexthops[i], '.') ? true : false; > > > + for (uint16_t i = 0; i < rp->n_valid_nexthops; i++) { > > > + bool is_ipv4 = strchr(rp->valid_nexthops[i], '.') ? true : false; > > > > > > if (i == 0) { > > > nexthops_is_ipv4 = is_ipv4; > > > @@ -10080,7 +10142,7 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, > > > VLOG_WARN_RL(&rl, "nexthop [%s] of the router policy with " > > > "the match [%s] do not belong to the same address " > > > "family as other next hops", > > > - rule->nexthops[i], rule->match); > > > + rp->valid_nexthops[i], rule->match); > > > return; > > > } > > > } > > > @@ -10088,40 +10150,30 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, > > > struct ds match = DS_EMPTY_INITIALIZER; > > > struct ds actions = DS_EMPTY_INITIALIZER; > > > > > > - size_t *valid_nexthops = xcalloc(rule->n_nexthops, sizeof *valid_nexthops); > > > - size_t n_valid_nexthops = 0; > > > - > > > - for (size_t i = 0; i < rule->n_nexthops; i++) { > > > + for (size_t i = 0; i < rp->n_valid_nexthops; i++) { > > > struct ovn_port *out_port = get_outport_for_routing_policy_nexthop( > > > - od, lr_ports, rule->priority, rule->nexthops[i]); > > > + od, lr_ports, rule->priority, rp->valid_nexthops[i]); > > > if (!out_port) { > > > goto cleanup; > > > } > > > > > > const char *lrp_addr_s = > > > - find_lrp_member_ip(out_port, rule->nexthops[i]); > > > + find_lrp_member_ip(out_port, rp->valid_nexthops[i]); > > > if (!lrp_addr_s) { > > > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > > > VLOG_WARN_RL(&rl, "lrp_addr not found for routing policy " > > > " priority %"PRId64" nexthop %s", > > > - rule->priority, rule->nexthops[i]); > > > + rule->priority, rp->valid_nexthops[i]); > > > goto cleanup; > > > } > > > > > > - if (!check_bfd_state(rule, bfd_connections, out_port, > > > - rule->nexthops[i])) { > > > - continue; > > > - } > > > - > > > - valid_nexthops[n_valid_nexthops++] = i + 1; > > > - > > > ds_clear(&actions); > > > uint32_t pkt_mark = smap_get_uint(&rule->options, "pkt_mark", 0); > > > if (pkt_mark) { > > > ds_put_format(&actions, "pkt.mark = %u; ", pkt_mark); > > > } > > > > > > - bool is_ipv4 = strchr(rule->nexthops[i], '.') ? true : false; > > > + bool is_ipv4 = strchr(rp->valid_nexthops[i], '.') ? true : false; > > > > > > ds_put_format(&actions, "%s = %s; " > > > "%s = %s; " > > > @@ -10130,7 +10182,7 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, > > > "flags.loopback = 1; " > > > "next;", > > > is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6, > > > - rule->nexthops[i], > > > + rp->valid_nexthops[i], > > > is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6, > > > lrp_addr_s, > > > out_port->lrp_networks.ea_s, > > > @@ -10146,37 +10198,30 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, > > > lflow_ref); > > > } > > > > > > - if (!n_valid_nexthops) { > > > - goto cleanup; > > > - } > > > - > > > ds_clear(&actions); > > > - if (n_valid_nexthops > 1) { > > > + if (rp->n_valid_nexthops > 1) { > > > ds_put_format(&actions, "%s = %"PRIu16 > > > "; %s = select(", REG_ECMP_GROUP_ID, ecmp_group_id, > > > REG_ECMP_MEMBER_ID); > > > > > > - for (size_t i = 0; i < n_valid_nexthops; i++) { > > > + for (size_t i = 0; i < rp->n_valid_nexthops; i++) { > > > if (i > 0) { > > > ds_put_cstr(&actions, ", "); > > > } > > > > > > - ds_put_format(&actions, "%"PRIuSIZE, valid_nexthops[i]); > > > + ds_put_format(&actions, "%"PRIuSIZE, i + 1); > > > } > > > ds_put_cstr(&actions, ");"); > > > } else { > > > ds_put_format(&actions, "%s = %"PRIu16 > > > - "; %s = %"PRIuSIZE"; next;", REG_ECMP_GROUP_ID, > > > - ecmp_group_id, REG_ECMP_MEMBER_ID, > > > - valid_nexthops[0]); > > > + "; %s = 1; next;", REG_ECMP_GROUP_ID, > > > + ecmp_group_id, REG_ECMP_MEMBER_ID); > > > } > > > ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_POLICY, > > > rule->priority, rule->match, > > > ds_cstr(&actions), &rule->header_, > > > lflow_ref); > > > - > > > cleanup: > > > - free(valid_nexthops); > > > ds_destroy(&match); > > > ds_destroy(&actions); > > > } > > > @@ -10201,7 +10246,7 @@ route_table_add(struct simap *route_tables, const char *route_table_name) > > > return rtb_id; > > > } > > > > > > -static uint32_t > > > +uint32_t > > > get_route_table_id(struct simap *route_tables, const char *route_table_name) > > > { > > > if (!route_table_name || !route_table_name[0]) { > > > @@ -10242,18 +10287,6 @@ build_route_table_lflow(struct ovn_datapath *od, struct lflow_table *lflows, > > > ds_destroy(&actions); > > > } > > > > > > -struct parsed_route { > > > - struct ovs_list list_node; > > > - struct in6_addr prefix; > > > - unsigned int plen; > > > - bool is_src_route; > > > - uint32_t route_table_id; > > > - uint32_t hash; > > > - const struct nbrec_logical_router_static_route *route; > > > - bool ecmp_symmetric_reply; > > > - bool is_discard_route; > > > -}; > > > - > > > static uint32_t > > > route_hash(struct parsed_route *route) > > > { > > > @@ -10268,11 +10301,53 @@ find_static_route_outport(struct ovn_datapath *od, const struct hmap *lr_ports, > > > > > > /* Parse and validate the route. Return the parsed route if successful. > > > * Otherwise return NULL. */ > > > + > > > static struct parsed_route * > > > +parsed_route_lookup(struct hmap *routes, size_t hash, > > > + struct parsed_route *new_pr) > > > +{ > > > + struct parsed_route *pr; > > > + HMAP_FOR_EACH_WITH_HASH (pr, key_node, hash, routes) { > > > + if (pr->plen != new_pr->plen) { > > > + continue; > > > + } > > > + > > > + if (memcmp(&pr->prefix, &new_pr->prefix, sizeof(struct in6_addr))) { > > > + continue; > > > + } > > > + > > > + if (pr->is_src_route != new_pr->is_src_route) { > > > + continue; > > > + } > > > + > > > + if (pr->route_table_id != new_pr->route_table_id) { > > > + continue; > > > + } > > > + > > > + if (pr->route != new_pr->route) { > > > + continue; > > > + } > > > + > > > + if (pr->ecmp_symmetric_reply != new_pr->ecmp_symmetric_reply) { > > > + continue; > > > + } > > > + > > > + if (pr->is_discard_route != new_pr->is_discard_route) { > > > + continue; > > > + } > > > + > > > + return pr; > > > + } > > > + > > > + return NULL; > > > +} > > > + > > > +static void > > > parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > > > - struct ovs_list *routes, struct simap *route_tables, > > > + struct hmap *routes, struct simap *route_tables, > > > const struct nbrec_logical_router_static_route *route, > > > - const struct hmap *bfd_connections) > > > + struct hmap *bfd_active_connections, > > > + struct hmap *bfd_connections) > > > { > > > /* Verify that the next hop is an IP address with an all-ones mask. */ > > > struct in6_addr nexthop; > > > @@ -10285,7 +10360,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > > > VLOG_WARN_RL(&rl, "bad 'nexthop' %s in static route " > > > UUID_FMT, route->nexthop, > > > UUID_ARGS(&route->header_.uuid)); > > > - return NULL; > > > + return; > > > } > > > if ((IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 32) || > > > (!IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 128)) { > > > @@ -10293,7 +10368,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > > > VLOG_WARN_RL(&rl, "bad next hop mask %s in static route " > > > UUID_FMT, route->nexthop, > > > UUID_ARGS(&route->header_.uuid)); > > > - return NULL; > > > + return; > > > } > > > } > > > > > > @@ -10304,7 +10379,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > > > VLOG_WARN_RL(&rl, "bad 'ip_prefix' %s in static route " > > > UUID_FMT, route->ip_prefix, > > > UUID_ARGS(&route->header_.uuid)); > > > - return NULL; > > > + return; > > > } > > > > > > /* Verify that ip_prefix and nexthop have same address familiy. */ > > > @@ -10315,7 +10390,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > > > " %s and 'nexthop' %s in static route "UUID_FMT, > > > route->ip_prefix, route->nexthop, > > > UUID_ARGS(&route->header_.uuid)); > > > - return NULL; > > > + return; > > > } > > > } > > > > > > @@ -10324,52 +10399,85 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > > > !find_static_route_outport(od, lr_ports, route, > > > IN6_IS_ADDR_V4MAPPED(&prefix), > > > NULL, NULL)) { > > > - return NULL; > > > + return; > > > } > > > > > > const struct nbrec_bfd *nb_bt = route->bfd; > > > if (nb_bt && !strcmp(nb_bt->dst_ip, route->nexthop)) { > > > - struct bfd_entry *bfd_e; > > > + struct bfd_entry *bfd_e = bfd_port_lookup(bfd_connections, > > > + nb_bt->logical_port, > > > + nb_bt->dst_ip); > > > + if (!bfd_e) { > > > + return; > > > + } > > > > > > - bfd_e = bfd_port_lookup(bfd_connections, nb_bt->logical_port, > > > - nb_bt->dst_ip); > > > - ovs_mutex_lock(&bfd_lock); > > > - if (bfd_e) { > > > - bfd_e->ref = true; > > > + if (!strcmp(bfd_e->status, "admin_down")) { > > > + bfd_set_status(bfd_e, "down"); > > > } > > > > > > - if (!strcmp(nb_bt->status, "admin_down")) { > > > - nbrec_bfd_set_status(nb_bt, "down"); > > > + /* This static route is linked to an active bfd session. */ > > > + if (!bfd_port_lookup(bfd_active_connections, nb_bt->logical_port, > > > + nb_bt->dst_ip)) { > > > + bfd_alloc_entry(bfd_active_connections, nb_bt->logical_port, > > > + nb_bt->dst_ip, bfd_e->status); > > > } > > > > > > - if (!strcmp(nb_bt->status, "down")) { > > > - ovs_mutex_unlock(&bfd_lock); > > > - return NULL; > > > + if (!strcmp(bfd_e->status, "down")) { > > > + return; > > > } > > > - ovs_mutex_unlock(&bfd_lock); > > > } > > > > > > - struct parsed_route *pr = xzalloc(sizeof *pr); > > > - pr->prefix = prefix; > > > - pr->plen = plen; > > > - pr->route_table_id = get_route_table_id(route_tables, route->route_table); > > > - pr->is_src_route = (route->policy && !strcmp(route->policy, > > > - "src-ip")); > > > - pr->hash = route_hash(pr); > > > - pr->route = route; > > > - pr->ecmp_symmetric_reply = smap_get_bool(&route->options, > > > - "ecmp_symmetric_reply", false); > > > - pr->is_discard_route = is_discard_route; > > > - ovs_list_insert(routes, &pr->list_node); > > > - return pr; > > > + struct parsed_route *new_pr = xzalloc(sizeof *new_pr); > > > + new_pr->prefix = prefix; > > > + new_pr->plen = plen; > > > + new_pr->route_table_id = get_route_table_id(route_tables, > > > + route->route_table); > > > + new_pr->is_src_route = (route->policy && > > > + !strcmp(route->policy, "src-ip")); > > > + new_pr->hash = route_hash(new_pr); > > > + new_pr->route = route; > > > + new_pr->nbr = od->nbr; > > > + new_pr->ecmp_symmetric_reply = smap_get_bool(&route->options, > > > + "ecmp_symmetric_reply", > > > + false); > > > + new_pr->is_discard_route = is_discard_route; > > > + > > > + size_t hash = uuid_hash(&od->key); > > > + struct parsed_route *pr = parsed_route_lookup(routes, hash, new_pr); > > > + if (!pr) { > > > + hmap_insert(routes, &new_pr->key_node, hash); > > > + } else { > > > + pr->stale = false; > > > + free(new_pr); > > > + } > > > } > > > > > > -static void > > > -parsed_routes_destroy(struct ovs_list *routes) > > > +void > > > +build_parsed_routes(struct ovn_datapath *od, const struct hmap *lr_ports, > > > + struct hmap *routes, struct simap *route_tables, > > > + struct hmap *bfd_active_connections, > > > + struct hmap *bfd_connections) > > > { > > > struct parsed_route *pr; > > > - LIST_FOR_EACH_SAFE (pr, list_node, routes) { > > > - ovs_list_remove(&pr->list_node); > > > + HMAP_FOR_EACH (pr, key_node, routes) { > > > + if (pr->nbr == od->nbr) { > > > + pr->stale = true; > > > + } > > > + } > > > + > > > + for (int i = 0; i < od->nbr->n_static_routes; i++) { > > > + parsed_routes_add(od, lr_ports, routes, route_tables, > > > + od->nbr->static_routes[i], > > > + bfd_active_connections, > > > + bfd_connections); > > > + } > > > + > > > + HMAP_FOR_EACH_SAFE (pr, key_node, routes) { > > > + if (!pr->stale) { > > > + continue; > > > + } > > > + > > > + hmap_remove(routes, &pr->key_node); > > > free(pr); > > > } > > > } > > > @@ -12671,8 +12779,8 @@ build_ip_routing_flows_for_lrp( > > > static void > > > build_static_route_flows_for_lrouter( > > > struct ovn_datapath *od, struct lflow_table *lflows, > > > - const struct hmap *lr_ports, const struct hmap *bfd_connections, > > > - struct lflow_ref *lflow_ref) > > > + const struct hmap *lr_ports, struct hmap *parsed_routes, > > > + struct simap *route_tables, struct lflow_ref *lflow_ref) > > > { > > > ovs_assert(od->nbr); > > > ovn_lflow_add_default_drop(lflows, od, S_ROUTER_IN_IP_ROUTING_ECMP, > > > @@ -12685,22 +12793,16 @@ build_static_route_flows_for_lrouter( > > > > > > struct hmap ecmp_groups = HMAP_INITIALIZER(&ecmp_groups); > > > struct hmap unique_routes = HMAP_INITIALIZER(&unique_routes); > > > - struct ovs_list parsed_routes = OVS_LIST_INITIALIZER(&parsed_routes); > > > - struct simap route_tables = SIMAP_INITIALIZER(&route_tables); > > > struct ecmp_groups_node *group; > > > > > > for (int i = 0; i < od->nbr->n_ports; i++) { > > > build_route_table_lflow(od, lflows, od->nbr->ports[i], > > > - &route_tables, lflow_ref); > > > + route_tables, lflow_ref); > > > } > > > > > > - for (int i = 0; i < od->nbr->n_static_routes; i++) { > > > - struct parsed_route *route = > > > - parsed_routes_add(od, lr_ports, &parsed_routes, &route_tables, > > > - od->nbr->static_routes[i], bfd_connections); > > > - if (!route) { > > > - continue; > > > - } > > > + struct parsed_route *route; > > > + HMAP_FOR_EACH_WITH_HASH (route, key_node, uuid_hash(&od->key), > > > + parsed_routes) { > > > group = ecmp_groups_find(&ecmp_groups, route); > > > if (group) { > > > ecmp_groups_add_route(group, route); > > > @@ -12728,8 +12830,6 @@ build_static_route_flows_for_lrouter( > > > } > > > ecmp_groups_destroy(&ecmp_groups); > > > unique_routes_destroy(&unique_routes); > > > - parsed_routes_destroy(&parsed_routes); > > > - simap_destroy(&route_tables); > > > } > > > > > > /* IP Multicast lookup. Here we set the output port, adjust TTL and > > > @@ -12836,6 +12936,115 @@ build_mcast_lookup_flows_for_lrouter( > > > } > > > } > > > > > > +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->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; > > > +} > > > + > > > +void > > > +build_route_policies(struct ovn_datapath *od, struct hmap *lr_ports, > > > + struct hmap *route_policies, > > > + struct hmap *bfd_active_connections, > > > + struct hmap *bfd_connections) > > > +{ > > > + struct route_policy *rp; > > > + > > > + HMAP_FOR_EACH (rp, key_node, route_policies) { > > > + if (rp->nbr == od->nbr) { > > > + rp->stale = true; > > > + } > > > + } > > > + > > > + for (int i = 0; i < od->nbr->n_policies; i++) { > > > + const struct nbrec_logical_router_policy *rule = od->nbr->policies[i]; > > > + size_t n_valid_nexthops = 0; > > > + char **valid_nexthops = NULL; > > > + > > > + if (!strcmp(rule->action, "reroute")) { > > > + size_t n_nexthops = rule->n_nexthops ? rule->n_nexthops : 1; > > > + > > > + valid_nexthops = xcalloc(n_nexthops, sizeof *valid_nexthops); > > > + for (size_t j = 0; j < n_nexthops; j++) { > > > + char *nexthop = rule->n_nexthops > > > + ? rule->nexthops[j] : rule->nexthop; > > > + struct ovn_port *out_port = > > > + get_outport_for_routing_policy_nexthop( > > > + od, lr_ports, rule->priority, nexthop); > > > + if (!out_port || !check_bfd_state(rule, out_port, nexthop, > > > + bfd_active_connections, > > > + bfd_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->nbr = od->nbr; > > > + > > > + 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 { > > > + 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. > > > * > > > * A packet that arrives at this table is an IP packet that should be > > > @@ -12849,7 +13058,7 @@ static void > > > build_ingress_policy_flows_for_lrouter( > > > struct ovn_datapath *od, struct lflow_table *lflows, > > > const struct hmap *lr_ports, > > > - const struct hmap *bfd_connections, > > > + struct hmap *route_policies, > > > struct lflow_ref *lflow_ref) > > > { > > > ovs_assert(od->nbr); > > > @@ -12866,21 +13075,20 @@ build_ingress_policy_flows_for_lrouter( > > > > > > /* Convert routing policies to flows. */ > > > uint16_t ecmp_group_id = 1; > > > - for (int i = 0; i < od->nbr->n_policies; i++) { > > > - const struct nbrec_logical_router_policy *rule > > > - = od->nbr->policies[i]; > > > + struct route_policy *rp; > > > + HMAP_FOR_EACH_WITH_HASH (rp, key_node, uuid_hash(&od->key), > > > + route_policies) { > > > + const struct nbrec_logical_router_policy *rule = rp->rule; > > > bool is_ecmp_reroute = > > > (!strcmp(rule->action, "reroute") && rule->n_nexthops > 1); > > > > > > if (is_ecmp_reroute) { > > > - build_ecmp_routing_policy_flows(lflows, od, lr_ports, rule, > > > - bfd_connections, ecmp_group_id, > > > - lflow_ref); > > > + build_ecmp_routing_policy_flows(lflows, od, lr_ports, rp, > > > + ecmp_group_id, lflow_ref); > > > ecmp_group_id++; > > > } else { > > > - build_routing_policy_flow(lflows, od, lr_ports, rule, > > > - bfd_connections, &rule->header_, > > > - lflow_ref); > > > + build_routing_policy_flow(lflows, od, lr_ports, rp, > > > + &rule->header_, lflow_ref); > > > } > > > } > > > } > > > @@ -15805,6 +16013,9 @@ struct lswitch_flow_build_info { > > > struct ds actions; > > > size_t thread_lflow_counter; > > > const char *svc_monitor_mac; > > > + struct hmap *parsed_routes; > > > + struct hmap *route_policies; > > > + struct simap *route_tables; > > > }; > > > > > > /* Helper function to combine all lflow generation which is iterated by > > > @@ -15850,12 +16061,12 @@ build_lswitch_and_lrouter_iterate_by_lr(struct ovn_datapath *od, > > > build_ND_RA_flows_for_lrouter(od, lsi->lflows, NULL); > > > build_ip_routing_pre_flows_for_lrouter(od, lsi->lflows, NULL); > > > build_static_route_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports, > > > - lsi->bfd_connections, > > > + lsi->parsed_routes, lsi->route_tables, > > > NULL); > > > build_mcast_lookup_flows_for_lrouter(od, lsi->lflows, &lsi->match, > > > &lsi->actions, NULL); > > > build_ingress_policy_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports, > > > - lsi->bfd_connections, NULL); > > > + lsi->route_policies, NULL); > > > build_arp_resolve_flows_for_lrouter(od, lsi->lflows, NULL); > > > build_check_pkt_len_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports, > > > &lsi->match, &lsi->actions, > > > @@ -16169,7 +16380,10 @@ build_lswitch_and_lrouter_flows( > > > const struct hmap *svc_monitor_map, > > > const struct hmap *bfd_connections, > > > const struct chassis_features *features, > > > - const char *svc_monitor_mac) > > > + const char *svc_monitor_mac, > > > + struct hmap *parsed_routes, > > > + struct hmap *route_policies, > > > + struct simap *route_tables) > > > { > > > > > > char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac); > > > @@ -16203,6 +16417,9 @@ build_lswitch_and_lrouter_flows( > > > lsiv[index].svc_check_match = svc_check_match; > > > lsiv[index].thread_lflow_counter = 0; > > > lsiv[index].svc_monitor_mac = svc_monitor_mac; > > > + lsiv[index].parsed_routes = parsed_routes; > > > + lsiv[index].route_tables = route_tables; > > > + lsiv[index].route_policies = route_policies; > > > ds_init(&lsiv[index].match); > > > ds_init(&lsiv[index].actions); > > > > > > @@ -16243,6 +16460,9 @@ build_lswitch_and_lrouter_flows( > > > .features = features, > > > .svc_check_match = svc_check_match, > > > .svc_monitor_mac = svc_monitor_mac, > > > + .parsed_routes = parsed_routes, > > > + .route_tables = route_tables, > > > + .route_policies = route_policies, > > > .match = DS_EMPTY_INITIALIZER, > > > .actions = DS_EMPTY_INITIALIZER, > > > }; > > > @@ -16404,7 +16624,10 @@ void build_lflows(struct ovsdb_idl_txn *ovnsb_txn, > > > input_data->svc_monitor_map, > > > input_data->bfd_connections, > > > input_data->features, > > > - input_data->svc_monitor_mac); > > > + input_data->svc_monitor_mac, > > > + input_data->parsed_routes, > > > + input_data->route_policies, > > > + input_data->route_tables); > > > > > > if (parallelization_state == STATE_INIT_HASH_SIZES) { > > > parallelization_state = STATE_USE_PARALLELIZATION; > > > @@ -17477,6 +17700,27 @@ 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); > > > +} > > > + > > > +void > > > +static_routes_init(struct static_routes_data *data) > > > +{ > > > + hmap_init(&data->parsed_routes); > > > + simap_init(&data->route_tables); > > > + hmap_init(&data->bfd_active_connections); > > > +} > > > + > > > +void > > > +bfd_init(struct bfd_data *data) > > > +{ > > > + hmap_init(&data->bfd_connections); > > > +} > > > + > > > void > > > northd_destroy(struct northd_data *data) > > > { > > > @@ -17516,6 +17760,48 @@ northd_destroy(struct northd_data *data) > > > destroy_northd_tracked_data(data); > > > } > > > > > > +static void > > > +__bfd_destroy(struct hmap *bfd_connections) > > > +{ > > > + struct bfd_entry *bfd_e; > > > + > > > + HMAP_FOR_EACH_POP (bfd_e, hmap_node, bfd_connections) { > > > + bfd_erase_entry(bfd_e); > > > + } > > > + hmap_destroy(bfd_connections); > > > +} > > > + > > > +void > > > +bfd_destroy(struct bfd_data *data) > > > +{ > > > + __bfd_destroy(&data->bfd_connections); > > > +} > > > + > > > +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); > > > +} > > > + > > > +void > > > +static_routes_destroy(struct static_routes_data *data) > > > +{ > > > + struct parsed_route *r; > > > + HMAP_FOR_EACH_POP (r, key_node, &data->parsed_routes) { > > > + free(r); > > > + } > > > + hmap_destroy(&data->parsed_routes); > > > + > > > + simap_destroy(&data->route_tables); > > > + __bfd_destroy(&data->bfd_active_connections); > > > +} > > > + > > > void > > > ovnnb_db_run(struct northd_input *input_data, > > > struct northd_data *data, > > > diff --git a/northd/northd.h b/northd/northd.h > > > index d4a8d75ab..7ddb72eb7 100644 > > > --- a/northd/northd.h > > > +++ b/northd/northd.h > > > @@ -23,6 +23,7 @@ > > > #include "northd/en-port-group.h" > > > #include "northd/ipam.h" > > > #include "openvswitch/hmap.h" > > > +#include "simap.h" > > > #include "ovs-thread.h" > > > > > > struct northd_input { > > > @@ -160,14 +161,34 @@ 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; > > > + const struct nbrec_logical_router *nbr; > > > + bool stale; > > > +}; > > > + > > > +struct static_routes_data { > > > + struct hmap parsed_routes; > > > + struct simap route_tables; > > > + struct hmap bfd_active_connections; > > > +}; > > > + > > > +struct route_policies_data { > > > + struct hmap route_policies; > > > + struct hmap bfd_active_connections; > > > +}; > > > + > > > +struct bfd_data { > > > + struct hmap bfd_connections; > > > +}; > > > + > > > struct lr_nat_table; > > > > > > struct lflow_input { > > > - /* Northbound table references */ > > > - const struct nbrec_bfd_table *nbrec_bfd_table; > > > - > > > /* Southbound table references */ > > > - const struct sbrec_bfd_table *sbrec_bfd_table; > > > const struct sbrec_logical_flow_table *sbrec_logical_flow_table; > > > const struct sbrec_multicast_group_table *sbrec_multicast_group_table; > > > const struct sbrec_igmp_group_table *sbrec_igmp_group_table; > > > @@ -190,6 +211,9 @@ struct lflow_input { > > > const struct hmap *svc_monitor_map; > > > bool ovn_internal_version_changed; > > > const char *svc_monitor_mac; > > > + struct hmap *parsed_routes; > > > + struct hmap *route_policies; > > > + struct simap *route_tables; > > > }; > > > > > > extern int parallelization_state; > > > @@ -653,6 +677,20 @@ struct ovn_port { > > > struct lflow_ref *stateful_lflow_ref; > > > }; > > > > > > +struct parsed_route { > > > + struct hmap_node key_node; > > > + struct in6_addr prefix; > > > + unsigned int plen; > > > + bool is_src_route; > > > + uint32_t route_table_id; > > > + uint32_t hash; > > > + const struct nbrec_logical_router_static_route *route; > > > + bool ecmp_symmetric_reply; > > > + bool is_discard_route; > > > + const struct nbrec_logical_router *nbr; > > > + bool stale; > > > +}; > > > + > > > void ovnnb_db_run(struct northd_input *input_data, > > > struct northd_data *data, > > > struct ovsdb_idl_txn *ovnnb_txn, > > > @@ -674,6 +712,18 @@ 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(struct ovn_datapath *, const struct hmap *, > > > + struct hmap *, struct simap *, struct hmap *, > > > + struct hmap *); > > > +uint32_t get_route_table_id(struct simap *, const char *); > > > +void static_routes_init(struct static_routes_data *); > > > +void static_routes_destroy(struct static_routes_data *); > > > + > > > +void bfd_init(struct bfd_data *); > > > +void bfd_destroy(struct bfd_data *); > > > + > > > struct lflow_table; > > > struct lr_stateful_tracked_data; > > > struct ls_stateful_tracked_data; > > > @@ -711,13 +761,14 @@ bool northd_handle_lb_data_changes(struct tracked_lb_data *, > > > struct hmap *lbgrp_datapaths_map, > > > struct northd_tracked_data *); > > > > > > -void build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn, > > > - const struct nbrec_bfd_table *, > > > - const struct sbrec_bfd_table *, > > > - const struct hmap *lr_ports, > > > - struct hmap *bfd_connections); > > > -void bfd_cleanup_connections(const struct nbrec_bfd_table *, > > > - struct hmap *bfd_map); > > > +void build_route_policies(struct ovn_datapath *, struct hmap *, struct hmap *, > > > + struct hmap *, struct hmap *); > > > +void bfd_table_sync(struct ovsdb_idl_txn *, const struct nbrec_bfd_table *, > > > + struct hmap *, struct hmap *, struct hmap *); > > > +void build_bfd_map(const struct nbrec_bfd_table *, > > > + const struct sbrec_bfd_table *, > > > + const struct hmap *lr_ports, > > > + struct hmap *bfd_connections); > > > void run_update_worker_pool(int n_threads); > > > > > > const struct ovn_datapath *northd_get_datapath_for_port( > > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > > > index 57f89a774..1d7f55e4f 100644 > > > --- a/tests/ovn-northd.at > > > +++ b/tests/ovn-northd.at > > > @@ -12,6 +12,7 @@ m4_define([_DUMP_DB_TABLES], [ > > > ovn-sbctl list meter >> $1 > > > ovn-sbctl list meter_band >> $1 > > > ovn-sbctl list port_group >> $1 > > > + ovn-sbctl list bfd >> $1 > > > ovn-sbctl dump-flows > lflows_$1 > > > ]) > > > > > > @@ -3861,6 +3862,7 @@ for i in $(seq 1 9); do > > > check ovn-nbctl --wait=sb lsp-set-addresses sw$i-r0 00:00:00:00:00:0$i > > > done > > > > > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > > uuid=$(ovn-nbctl create bfd logical_port=r0-sw1 dst_ip=192.168.1.2 status=down min_tx=250 min_rx=250 detect_mult=10) > > > ovn-nbctl create bfd logical_port=r0-sw2 dst_ip=192.168.2.2 status=down min_tx=500 min_rx=500 detect_mult=20 > > > ovn-nbctl create bfd logical_port=r0-sw3 dst_ip=192.168.3.2 status=down > > > @@ -3873,6 +3875,13 @@ wait_row_count bfd 1 logical_port=r0-sw2 detect_mult=20 dst_ip=192.168.2.2 \ > > > wait_row_count bfd 1 logical_port=r0-sw3 detect_mult=5 dst_ip=192.168.3.2 \ > > > min_rx=1000 min_tx=1000 status=admin_down > > > > > > +check_engine_stats northd norecompute nocompute > > > +check_engine_stats bfd recompute nocompute > > > +check_engine_stats lflow recompute nocompute > > > +check_engine_stats northd_output norecompute compute > > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > > + > > > uuid=$(fetch_column nb:bfd _uuid logical_port=r0-sw1) > > > check ovn-nbctl set bfd $uuid min_tx=1000 min_rx=1000 detect_mult=100 > > > > > > @@ -3881,10 +3890,25 @@ check ovn-nbctl clear bfd $uuid_2 min_rx > > > wait_row_count bfd 1 logical_port=r0-sw2 min_rx=1000 > > > wait_row_count bfd 1 logical_port=r0-sw1 min_rx=1000 min_tx=1000 detect_mult=100 > > > > > > +check_engine_stats northd norecompute nocompute > > > +check_engine_stats bfd recompute nocompute > > > +check_engine_stats lflow recompute nocompute > > > +check_engine_stats northd_output norecompute compute > > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > > + > > > check ovn-nbctl --bfd=$uuid lr-route-add r0 100.0.0.0/8 192.168.1.2 > > > wait_column down bfd status logical_port=r0-sw1 > > > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.1.2 | grep -q bfd],[0]) > > > > > > +check_engine_stats northd recompute nocompute > > > +check_engine_stats bfd recompute nocompute > > > +check_engine_stats static_routes recompute nocompute > > > +check_engine_stats lflow recompute nocompute > > > +check_engine_stats northd_output norecompute compute > > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > > + > > > check ovn-nbctl --bfd lr-route-add r0 200.0.0.0/8 192.168.2.2 > > > wait_column down bfd status logical_port=r0-sw2 > > > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.2.2 | grep -q bfd],[0]) > > > @@ -3893,10 +3917,26 @@ check ovn-nbctl --bfd lr-route-add r0 240.0.0.0/8 192.168.5.2 r0-sw5 > > > wait_column down bfd status logical_port=r0-sw5 > > > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.5.2 | grep -q bfd],[0]) > > > > > > +check_engine_stats northd recompute nocompute > > > +check_engine_stats bfd recompute nocompute > > > +check_engine_stats static_routes recompute nocompute > > > +check_engine_stats lflow recompute nocompute > > > +check_engine_stats northd_output norecompute compute > > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > > + > > > check ovn-nbctl --bfd --policy=src-ip lr-route-add r0 192.168.6.1/32 192.168.10.10 r0-sw6 > > > wait_column down bfd status logical_port=r0-sw6 > > > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.6.1 | grep -q bfd],[0]) > > > > > > +check_engine_stats northd recompute nocompute > > > +check_engine_stats bfd recompute nocompute > > > +check_engine_stats route_policies recompute nocompute > > > +check_engine_stats lflow recompute nocompute > > > +check_engine_stats northd_output norecompute compute > > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > > + > > > check ovn-nbctl --bfd --policy=src-ip lr-route-add r0 192.168.7.1/32 192.168.10.10 r0-sw7 > > > wait_column down bfd status logical_port=r0-sw7 > > > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.7.1 | grep -q bfd],[0]) > > > @@ -3924,6 +3964,14 @@ wait_column down bfd status logical_port=r0-sw8 > > > bfd_route_policy_uuid=$(fetch_column nb:bfd _uuid logical_port=r0-sw8) > > > AT_CHECK([ovn-nbctl list logical_router_policy | grep -q $bfd_route_policy_uuid]) > > > > > > +check_engine_stats northd recompute nocompute > > > +check_engine_stats bfd recompute nocompute > > > +check_engine_stats static_routes recompute nocompute > > > +check_engine_stats lflow recompute nocompute > > > +check_engine_stats northd_output norecompute compute > > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > > + > > > check ovn-nbctl lr-policy-del r0 > > > check ovn-nbctl --bfd lr-policy-add r0 100 "ip4.src == 2.3.4.0/24" reroute 192.168.9.2,192.168.9.3,192.168.9.4 > > > > > > @@ -3931,6 +3979,14 @@ wait_column down bfd status dst_ip=192.168.9.2 > > > wait_column down bfd status dst_ip=192.168.9.3 > > > wait_column down bfd status dst_ip=192.168.9.4 > > > > > > +check_engine_stats northd recompute nocompute > > > +check_engine_stats bfd recompute nocompute > > > +check_engine_stats route_policies recompute nocompute > > > +check_engine_stats lflow recompute nocompute > > > +check_engine_stats northd_output norecompute compute > > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > > + > > > bfd_route_policy_uuid=$(fetch_column nb:bfd _uuid logical_port=r0-sw9) > > > AT_CHECK([ovn-nbctl list logical_router_policy | sed s/,//g | grep -q "$bfd_route_policy_uuid"]) > > > > > > -- > > > 2.45.2 > > > > >
On Jul 31, Han Zhou wrote: > On Wed, Jul 31, 2024 at 8:42 AM Lorenzo Bianconi > <lorenzo.bianconi@redhat.com> wrote: > > > > [...] > > > > #endif /* EN_NORTHD_H */ > > > > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > > > > index 522236ad2..77610cdd8 100644 > > > > --- a/northd/inc-proc-northd.c > > > > +++ b/northd/inc-proc-northd.c > > > > @@ -155,6 +155,10 @@ static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lb_data, "lb_data"); > > > > static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lr_nat, "lr_nat"); > > > > static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lr_stateful, "lr_stateful"); > > > > static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(ls_stateful, "ls_stateful"); > > > > +static ENGINE_NODE(route_policies, "route_policies"); > > > > +static ENGINE_NODE(static_routes, "static_routes"); > > > > +static ENGINE_NODE(bfd, "bfd"); > > > > +static ENGINE_NODE(bfd_sync, "bfd_sync"); > > > > > > > > void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > > > > struct ovsdb_idl_loop *sb) > > > > @@ -237,18 +241,37 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > > > > engine_add_input(&en_fdb_aging, &en_global_config, > > > > node_global_config_handler); > > > > > > > > + engine_add_input(&en_bfd, &en_nb_bfd, NULL); > > > > + engine_add_input(&en_bfd, &en_sb_bfd, NULL); > > > > + engine_add_input(&en_bfd, &en_northd, bfd_northd_change_handler); > > > > + > > > > + engine_add_input(&en_route_policies, &en_bfd, NULL); > > > > + engine_add_input(&en_route_policies, &en_northd, > > > > + route_policies_northd_change_handler); > > > > + > > > > + engine_add_input(&en_static_routes, &en_bfd, NULL); > > > > + engine_add_input(&en_static_routes, &en_northd, > > > > + static_routes_northd_change_handler); > > > > + > > > > + engine_add_input(&en_bfd_sync, &en_bfd, NULL); > > > > + engine_add_input(&en_bfd_sync, &en_nb_bfd, NULL); > > > > + engine_add_input(&en_bfd_sync, &en_static_routes, NULL); > > > > + engine_add_input(&en_bfd_sync, &en_route_policies, NULL); > > > > + > > > > engine_add_input(&en_sync_meters, &en_nb_acl, NULL); > > > > engine_add_input(&en_sync_meters, &en_nb_meter, NULL); > > > > engine_add_input(&en_sync_meters, &en_sb_meter, NULL); > > > > > > > > - engine_add_input(&en_lflow, &en_nb_bfd, NULL); > > > > engine_add_input(&en_lflow, &en_nb_acl, NULL); > > > > engine_add_input(&en_lflow, &en_sync_meters, NULL); > > > > - engine_add_input(&en_lflow, &en_sb_bfd, NULL); > > > > engine_add_input(&en_lflow, &en_sb_logical_flow, NULL); > > > > engine_add_input(&en_lflow, &en_sb_multicast_group, NULL); > > > > engine_add_input(&en_lflow, &en_sb_igmp_group, NULL); > > > > engine_add_input(&en_lflow, &en_sb_logical_dp_group, NULL); > > > > + engine_add_input(&en_lflow, &en_bfd, NULL); > > > > + engine_add_input(&en_lflow, &en_bfd_sync, NULL); > > > > > > It seems en_lflow node doesn't depend on en_bfd_sync. In theory the > > > en_bfd_sync can be added as an input of en_northd_output. What's the > > > reason if it has to be added as an input of en_lflow? > > > > ack, I will fix it. I guess we can have en_bfd_sync as input of > > en_northd_output. > > I tried it but at least one test case failed: check BFD config > propagation to SBDB > I didn't debug the failure yet. Probably some other hidden dependency > that we need to sort out. ack, looking again at the code I guess we can move ovn_port lookup just in bfd_table_sync() but in this way we need to keep the en_lflow dependency since this will affect lflow generation. > > > > > > > > > > + engine_add_input(&en_lflow, &en_route_policies, NULL); > > > > + engine_add_input(&en_lflow, &en_static_routes, NULL); > > > > engine_add_input(&en_lflow, &en_global_config, > > > > node_global_config_handler); > > > > engine_add_input(&en_lflow, &en_northd, lflow_northd_handler); > > > > diff --git a/northd/northd.c b/northd/northd.c > > > > index 5c2fd74ff..18fe2b48a 100644 > > > > --- a/northd/northd.c > > > > +++ b/northd/northd.c > > > > @@ -9689,11 +9689,49 @@ 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; > > > > [...] > > > > > > > > This function is called by build_route_policies() which belongs to the > > > en_route_polices node. The bfd_connections is input data so it should > > > be readonly to this node and never be modified here. Otherwise, if a > > > node modifies its input data, it may impact other nodes that depend on > > > the same input node, and it is hard to get things correct in the I-P > > > engine. > > > > > > Similarly, I see that the bfd_table_sync() function is modifying the > > > en_bfd data, which is not good. en_bfd data should be modified in > > > en_bfd node only, when consolidating status between SB DB and NB DB, > > > and bfd_sync node should only update the DB, according to the data of > > > en_bfd, en_route_policies and en_static_routes. > > > > > > Please let me know if this is challenging. > > > > ack, I guess we can set bfd_connections entry status in check_bfd_state() > > I guess you are saying we can set rp_bfd_connections and > sr_bfd_connections entry status instead of directly setting > bfd_connections entry status in those functions that don't belong to > en_bfd? correct > > > (route_policies) and parsed_routes_add() (static_routes) based on en_bfd > > bfd hashmap entry and set NB BFD status in bfd_table_sync() according to > > the status value we can get from rp_bfd_connections and sr_bfd_connections > > (bfd_connection consumers - route_policies and static_routes). Doing so we > > do need to set write into structures belonging to other IP nodes. > > Just to confirm, are you saying s/do need to/don't need to? :) sorry I forgot a 'not', we do not need.. > > > I will fix it in v10. > > Thanks! In addition, could you try to reorder the related function > parameters so that input (readonly) parameters are in front of output, > such as: > build_route_policies(), build_parsed_routes(), check_bfd_state(), > parsed_routes_add(). In these functions bfd_data->bfd_connections are > put at last but it is better to put it in front of the output > parameters of the functions because bfd_data->bfd_connections is input > and readonly to those functions. (This has been an agreement but may > not be strictly followed in many places.) ack, I will fix it in v10. Regards, Lorenzo > > Thanks, > Han > > > > > Regards, > > Lorenzo > > > > > > Thanks, > > > Han > > > > > > > > > > > - ret = strcmp(nb_bt->status, "down"); > > > > - ovs_mutex_unlock(&bfd_lock); > > > > - break; > > > > + /* This route policy is linked to an active bfd session. */ > > > > + if (!bfd_port_lookup(bfd_active_connections, nb_bt->logical_port, > > > > + nb_bt->dst_ip)) { > > > > + bfd_alloc_entry(bfd_active_connections, nb_bt->logical_port, > > > > + nb_bt->dst_ip, bfd_e->status); > > > > + } > > > > + > > > > + return strcmp(bfd_e->status, "down"); > > > > } > > > > > > > > - return ret; > > > > + return true; > > > > } > > > > > > > > static void > > > > build_routing_policy_flow(struct lflow_table *lflows, struct ovn_datapath *od, > > > > - const struct hmap *lr_ports, > > > > - const struct nbrec_logical_router_policy *rule, > > > > - const struct hmap *bfd_connections, > > > > + const struct hmap *lr_ports, struct route_policy *rp, > > > > const struct ovsdb_idl_row *stage_hint, > > > > struct lflow_ref *lflow_ref) > > > > { > > > > + const struct nbrec_logical_router_policy *rule = rp->rule; > > > > struct ds match = DS_EMPTY_INITIALIZER; > > > > struct ds actions = DS_EMPTY_INITIALIZER; > > > > > > > > if (!strcmp(rule->action, "reroute")) { > > > > ovs_assert(rule->n_nexthops <= 1); > > > > > > > > - char *nexthop = > > > > - (rule->n_nexthops == 1 ? rule->nexthops[0] : rule->nexthop); > > > > + if (!rp->n_valid_nexthops) { > > > > + return; > > > > + } > > > > + > > > > + char *nexthop = rp->valid_nexthops[0]; > > > > struct ovn_port *out_port = get_outport_for_routing_policy_nexthop( > > > > od, lr_ports, rule->priority, nexthop); > > > > if (!out_port) { > > > > @@ -10011,10 +10078,6 @@ build_routing_policy_flow(struct lflow_table *lflows, struct ovn_datapath *od, > > > > return; > > > > } > > > > > > > > - if (!check_bfd_state(rule, bfd_connections, out_port, nexthop)) { > > > > - return; > > > > - } > > > > - > > > > uint32_t pkt_mark = smap_get_uint(&rule->options, "pkt_mark", 0); > > > > if (pkt_mark) { > > > > ds_put_format(&actions, "pkt.mark = %u; ", pkt_mark); > > > > @@ -10057,19 +10120,18 @@ static void > > > > build_ecmp_routing_policy_flows(struct lflow_table *lflows, > > > > struct ovn_datapath *od, > > > > const struct hmap *lr_ports, > > > > - const struct nbrec_logical_router_policy *rule, > > > > - const struct hmap *bfd_connections, > > > > + struct route_policy *rp, > > > > uint16_t ecmp_group_id, > > > > struct lflow_ref *lflow_ref) > > > > { > > > > - ovs_assert(rule->n_nexthops > 1); > > > > - > > > > bool nexthops_is_ipv4 = true; > > > > + const struct nbrec_logical_router_policy *rule = rp->rule; > > > > + ovs_assert(rule->n_nexthops > 1); > > > > > > > > /* Check that all the nexthops belong to the same addr family before > > > > * adding logical flows. */ > > > > - for (uint16_t i = 0; i < rule->n_nexthops; i++) { > > > > - bool is_ipv4 = strchr(rule->nexthops[i], '.') ? true : false; > > > > + for (uint16_t i = 0; i < rp->n_valid_nexthops; i++) { > > > > + bool is_ipv4 = strchr(rp->valid_nexthops[i], '.') ? true : false; > > > > > > > > if (i == 0) { > > > > nexthops_is_ipv4 = is_ipv4; > > > > @@ -10080,7 +10142,7 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, > > > > VLOG_WARN_RL(&rl, "nexthop [%s] of the router policy with " > > > > "the match [%s] do not belong to the same address " > > > > "family as other next hops", > > > > - rule->nexthops[i], rule->match); > > > > + rp->valid_nexthops[i], rule->match); > > > > return; > > > > } > > > > } > > > > @@ -10088,40 +10150,30 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, > > > > struct ds match = DS_EMPTY_INITIALIZER; > > > > struct ds actions = DS_EMPTY_INITIALIZER; > > > > > > > > - size_t *valid_nexthops = xcalloc(rule->n_nexthops, sizeof *valid_nexthops); > > > > - size_t n_valid_nexthops = 0; > > > > - > > > > - for (size_t i = 0; i < rule->n_nexthops; i++) { > > > > + for (size_t i = 0; i < rp->n_valid_nexthops; i++) { > > > > struct ovn_port *out_port = get_outport_for_routing_policy_nexthop( > > > > - od, lr_ports, rule->priority, rule->nexthops[i]); > > > > + od, lr_ports, rule->priority, rp->valid_nexthops[i]); > > > > if (!out_port) { > > > > goto cleanup; > > > > } > > > > > > > > const char *lrp_addr_s = > > > > - find_lrp_member_ip(out_port, rule->nexthops[i]); > > > > + find_lrp_member_ip(out_port, rp->valid_nexthops[i]); > > > > if (!lrp_addr_s) { > > > > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > > > > VLOG_WARN_RL(&rl, "lrp_addr not found for routing policy " > > > > " priority %"PRId64" nexthop %s", > > > > - rule->priority, rule->nexthops[i]); > > > > + rule->priority, rp->valid_nexthops[i]); > > > > goto cleanup; > > > > } > > > > > > > > - if (!check_bfd_state(rule, bfd_connections, out_port, > > > > - rule->nexthops[i])) { > > > > - continue; > > > > - } > > > > - > > > > - valid_nexthops[n_valid_nexthops++] = i + 1; > > > > - > > > > ds_clear(&actions); > > > > uint32_t pkt_mark = smap_get_uint(&rule->options, "pkt_mark", 0); > > > > if (pkt_mark) { > > > > ds_put_format(&actions, "pkt.mark = %u; ", pkt_mark); > > > > } > > > > > > > > - bool is_ipv4 = strchr(rule->nexthops[i], '.') ? true : false; > > > > + bool is_ipv4 = strchr(rp->valid_nexthops[i], '.') ? true : false; > > > > > > > > ds_put_format(&actions, "%s = %s; " > > > > "%s = %s; " > > > > @@ -10130,7 +10182,7 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, > > > > "flags.loopback = 1; " > > > > "next;", > > > > is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6, > > > > - rule->nexthops[i], > > > > + rp->valid_nexthops[i], > > > > is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6, > > > > lrp_addr_s, > > > > out_port->lrp_networks.ea_s, > > > > @@ -10146,37 +10198,30 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, > > > > lflow_ref); > > > > } > > > > > > > > - if (!n_valid_nexthops) { > > > > - goto cleanup; > > > > - } > > > > - > > > > ds_clear(&actions); > > > > - if (n_valid_nexthops > 1) { > > > > + if (rp->n_valid_nexthops > 1) { > > > > ds_put_format(&actions, "%s = %"PRIu16 > > > > "; %s = select(", REG_ECMP_GROUP_ID, ecmp_group_id, > > > > REG_ECMP_MEMBER_ID); > > > > > > > > - for (size_t i = 0; i < n_valid_nexthops; i++) { > > > > + for (size_t i = 0; i < rp->n_valid_nexthops; i++) { > > > > if (i > 0) { > > > > ds_put_cstr(&actions, ", "); > > > > } > > > > > > > > - ds_put_format(&actions, "%"PRIuSIZE, valid_nexthops[i]); > > > > + ds_put_format(&actions, "%"PRIuSIZE, i + 1); > > > > } > > > > ds_put_cstr(&actions, ");"); > > > > } else { > > > > ds_put_format(&actions, "%s = %"PRIu16 > > > > - "; %s = %"PRIuSIZE"; next;", REG_ECMP_GROUP_ID, > > > > - ecmp_group_id, REG_ECMP_MEMBER_ID, > > > > - valid_nexthops[0]); > > > > + "; %s = 1; next;", REG_ECMP_GROUP_ID, > > > > + ecmp_group_id, REG_ECMP_MEMBER_ID); > > > > } > > > > ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_POLICY, > > > > rule->priority, rule->match, > > > > ds_cstr(&actions), &rule->header_, > > > > lflow_ref); > > > > - > > > > cleanup: > > > > - free(valid_nexthops); > > > > ds_destroy(&match); > > > > ds_destroy(&actions); > > > > } > > > > @@ -10201,7 +10246,7 @@ route_table_add(struct simap *route_tables, const char *route_table_name) > > > > return rtb_id; > > > > } > > > > > > > > -static uint32_t > > > > +uint32_t > > > > get_route_table_id(struct simap *route_tables, const char *route_table_name) > > > > { > > > > if (!route_table_name || !route_table_name[0]) { > > > > @@ -10242,18 +10287,6 @@ build_route_table_lflow(struct ovn_datapath *od, struct lflow_table *lflows, > > > > ds_destroy(&actions); > > > > } > > > > > > > > -struct parsed_route { > > > > - struct ovs_list list_node; > > > > - struct in6_addr prefix; > > > > - unsigned int plen; > > > > - bool is_src_route; > > > > - uint32_t route_table_id; > > > > - uint32_t hash; > > > > - const struct nbrec_logical_router_static_route *route; > > > > - bool ecmp_symmetric_reply; > > > > - bool is_discard_route; > > > > -}; > > > > - > > > > static uint32_t > > > > route_hash(struct parsed_route *route) > > > > { > > > > @@ -10268,11 +10301,53 @@ find_static_route_outport(struct ovn_datapath *od, const struct hmap *lr_ports, > > > > > > > > /* Parse and validate the route. Return the parsed route if successful. > > > > * Otherwise return NULL. */ > > > > + > > > > static struct parsed_route * > > > > +parsed_route_lookup(struct hmap *routes, size_t hash, > > > > + struct parsed_route *new_pr) > > > > +{ > > > > + struct parsed_route *pr; > > > > + HMAP_FOR_EACH_WITH_HASH (pr, key_node, hash, routes) { > > > > + if (pr->plen != new_pr->plen) { > > > > + continue; > > > > + } > > > > + > > > > + if (memcmp(&pr->prefix, &new_pr->prefix, sizeof(struct in6_addr))) { > > > > + continue; > > > > + } > > > > + > > > > + if (pr->is_src_route != new_pr->is_src_route) { > > > > + continue; > > > > + } > > > > + > > > > + if (pr->route_table_id != new_pr->route_table_id) { > > > > + continue; > > > > + } > > > > + > > > > + if (pr->route != new_pr->route) { > > > > + continue; > > > > + } > > > > + > > > > + if (pr->ecmp_symmetric_reply != new_pr->ecmp_symmetric_reply) { > > > > + continue; > > > > + } > > > > + > > > > + if (pr->is_discard_route != new_pr->is_discard_route) { > > > > + continue; > > > > + } > > > > + > > > > + return pr; > > > > + } > > > > + > > > > + return NULL; > > > > +} > > > > + > > > > +static void > > > > parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > > > > - struct ovs_list *routes, struct simap *route_tables, > > > > + struct hmap *routes, struct simap *route_tables, > > > > const struct nbrec_logical_router_static_route *route, > > > > - const struct hmap *bfd_connections) > > > > + struct hmap *bfd_active_connections, > > > > + struct hmap *bfd_connections) > > > > { > > > > /* Verify that the next hop is an IP address with an all-ones mask. */ > > > > struct in6_addr nexthop; > > > > @@ -10285,7 +10360,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > > > > VLOG_WARN_RL(&rl, "bad 'nexthop' %s in static route " > > > > UUID_FMT, route->nexthop, > > > > UUID_ARGS(&route->header_.uuid)); > > > > - return NULL; > > > > + return; > > > > } > > > > if ((IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 32) || > > > > (!IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 128)) { > > > > @@ -10293,7 +10368,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > > > > VLOG_WARN_RL(&rl, "bad next hop mask %s in static route " > > > > UUID_FMT, route->nexthop, > > > > UUID_ARGS(&route->header_.uuid)); > > > > - return NULL; > > > > + return; > > > > } > > > > } > > > > > > > > @@ -10304,7 +10379,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > > > > VLOG_WARN_RL(&rl, "bad 'ip_prefix' %s in static route " > > > > UUID_FMT, route->ip_prefix, > > > > UUID_ARGS(&route->header_.uuid)); > > > > - return NULL; > > > > + return; > > > > } > > > > > > > > /* Verify that ip_prefix and nexthop have same address familiy. */ > > > > @@ -10315,7 +10390,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > > > > " %s and 'nexthop' %s in static route "UUID_FMT, > > > > route->ip_prefix, route->nexthop, > > > > UUID_ARGS(&route->header_.uuid)); > > > > - return NULL; > > > > + return; > > > > } > > > > } > > > > > > > > @@ -10324,52 +10399,85 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, > > > > !find_static_route_outport(od, lr_ports, route, > > > > IN6_IS_ADDR_V4MAPPED(&prefix), > > > > NULL, NULL)) { > > > > - return NULL; > > > > + return; > > > > } > > > > > > > > const struct nbrec_bfd *nb_bt = route->bfd; > > > > if (nb_bt && !strcmp(nb_bt->dst_ip, route->nexthop)) { > > > > - struct bfd_entry *bfd_e; > > > > + struct bfd_entry *bfd_e = bfd_port_lookup(bfd_connections, > > > > + nb_bt->logical_port, > > > > + nb_bt->dst_ip); > > > > + if (!bfd_e) { > > > > + return; > > > > + } > > > > > > > > - bfd_e = bfd_port_lookup(bfd_connections, nb_bt->logical_port, > > > > - nb_bt->dst_ip); > > > > - ovs_mutex_lock(&bfd_lock); > > > > - if (bfd_e) { > > > > - bfd_e->ref = true; > > > > + if (!strcmp(bfd_e->status, "admin_down")) { > > > > + bfd_set_status(bfd_e, "down"); > > > > } > > > > > > > > - if (!strcmp(nb_bt->status, "admin_down")) { > > > > - nbrec_bfd_set_status(nb_bt, "down"); > > > > + /* This static route is linked to an active bfd session. */ > > > > + if (!bfd_port_lookup(bfd_active_connections, nb_bt->logical_port, > > > > + nb_bt->dst_ip)) { > > > > + bfd_alloc_entry(bfd_active_connections, nb_bt->logical_port, > > > > + nb_bt->dst_ip, bfd_e->status); > > > > } > > > > > > > > - if (!strcmp(nb_bt->status, "down")) { > > > > - ovs_mutex_unlock(&bfd_lock); > > > > - return NULL; > > > > + if (!strcmp(bfd_e->status, "down")) { > > > > + return; > > > > } > > > > - ovs_mutex_unlock(&bfd_lock); > > > > } > > > > > > > > - struct parsed_route *pr = xzalloc(sizeof *pr); > > > > - pr->prefix = prefix; > > > > - pr->plen = plen; > > > > - pr->route_table_id = get_route_table_id(route_tables, route->route_table); > > > > - pr->is_src_route = (route->policy && !strcmp(route->policy, > > > > - "src-ip")); > > > > - pr->hash = route_hash(pr); > > > > - pr->route = route; > > > > - pr->ecmp_symmetric_reply = smap_get_bool(&route->options, > > > > - "ecmp_symmetric_reply", false); > > > > - pr->is_discard_route = is_discard_route; > > > > - ovs_list_insert(routes, &pr->list_node); > > > > - return pr; > > > > + struct parsed_route *new_pr = xzalloc(sizeof *new_pr); > > > > + new_pr->prefix = prefix; > > > > + new_pr->plen = plen; > > > > + new_pr->route_table_id = get_route_table_id(route_tables, > > > > + route->route_table); > > > > + new_pr->is_src_route = (route->policy && > > > > + !strcmp(route->policy, "src-ip")); > > > > + new_pr->hash = route_hash(new_pr); > > > > + new_pr->route = route; > > > > + new_pr->nbr = od->nbr; > > > > + new_pr->ecmp_symmetric_reply = smap_get_bool(&route->options, > > > > + "ecmp_symmetric_reply", > > > > + false); > > > > + new_pr->is_discard_route = is_discard_route; > > > > + > > > > + size_t hash = uuid_hash(&od->key); > > > > + struct parsed_route *pr = parsed_route_lookup(routes, hash, new_pr); > > > > + if (!pr) { > > > > + hmap_insert(routes, &new_pr->key_node, hash); > > > > + } else { > > > > + pr->stale = false; > > > > + free(new_pr); > > > > + } > > > > } > > > > > > > > -static void > > > > -parsed_routes_destroy(struct ovs_list *routes) > > > > +void > > > > +build_parsed_routes(struct ovn_datapath *od, const struct hmap *lr_ports, > > > > + struct hmap *routes, struct simap *route_tables, > > > > + struct hmap *bfd_active_connections, > > > > + struct hmap *bfd_connections) > > > > { > > > > struct parsed_route *pr; > > > > - LIST_FOR_EACH_SAFE (pr, list_node, routes) { > > > > - ovs_list_remove(&pr->list_node); > > > > + HMAP_FOR_EACH (pr, key_node, routes) { > > > > + if (pr->nbr == od->nbr) { > > > > + pr->stale = true; > > > > + } > > > > + } > > > > + > > > > + for (int i = 0; i < od->nbr->n_static_routes; i++) { > > > > + parsed_routes_add(od, lr_ports, routes, route_tables, > > > > + od->nbr->static_routes[i], > > > > + bfd_active_connections, > > > > + bfd_connections); > > > > + } > > > > + > > > > + HMAP_FOR_EACH_SAFE (pr, key_node, routes) { > > > > + if (!pr->stale) { > > > > + continue; > > > > + } > > > > + > > > > + hmap_remove(routes, &pr->key_node); > > > > free(pr); > > > > } > > > > } > > > > @@ -12671,8 +12779,8 @@ build_ip_routing_flows_for_lrp( > > > > static void > > > > build_static_route_flows_for_lrouter( > > > > struct ovn_datapath *od, struct lflow_table *lflows, > > > > - const struct hmap *lr_ports, const struct hmap *bfd_connections, > > > > - struct lflow_ref *lflow_ref) > > > > + const struct hmap *lr_ports, struct hmap *parsed_routes, > > > > + struct simap *route_tables, struct lflow_ref *lflow_ref) > > > > { > > > > ovs_assert(od->nbr); > > > > ovn_lflow_add_default_drop(lflows, od, S_ROUTER_IN_IP_ROUTING_ECMP, > > > > @@ -12685,22 +12793,16 @@ build_static_route_flows_for_lrouter( > > > > > > > > struct hmap ecmp_groups = HMAP_INITIALIZER(&ecmp_groups); > > > > struct hmap unique_routes = HMAP_INITIALIZER(&unique_routes); > > > > - struct ovs_list parsed_routes = OVS_LIST_INITIALIZER(&parsed_routes); > > > > - struct simap route_tables = SIMAP_INITIALIZER(&route_tables); > > > > struct ecmp_groups_node *group; > > > > > > > > for (int i = 0; i < od->nbr->n_ports; i++) { > > > > build_route_table_lflow(od, lflows, od->nbr->ports[i], > > > > - &route_tables, lflow_ref); > > > > + route_tables, lflow_ref); > > > > } > > > > > > > > - for (int i = 0; i < od->nbr->n_static_routes; i++) { > > > > - struct parsed_route *route = > > > > - parsed_routes_add(od, lr_ports, &parsed_routes, &route_tables, > > > > - od->nbr->static_routes[i], bfd_connections); > > > > - if (!route) { > > > > - continue; > > > > - } > > > > + struct parsed_route *route; > > > > + HMAP_FOR_EACH_WITH_HASH (route, key_node, uuid_hash(&od->key), > > > > + parsed_routes) { > > > > group = ecmp_groups_find(&ecmp_groups, route); > > > > if (group) { > > > > ecmp_groups_add_route(group, route); > > > > @@ -12728,8 +12830,6 @@ build_static_route_flows_for_lrouter( > > > > } > > > > ecmp_groups_destroy(&ecmp_groups); > > > > unique_routes_destroy(&unique_routes); > > > > - parsed_routes_destroy(&parsed_routes); > > > > - simap_destroy(&route_tables); > > > > } > > > > > > > > /* IP Multicast lookup. Here we set the output port, adjust TTL and > > > > @@ -12836,6 +12936,115 @@ build_mcast_lookup_flows_for_lrouter( > > > > } > > > > } > > > > > > > > +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->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; > > > > +} > > > > + > > > > +void > > > > +build_route_policies(struct ovn_datapath *od, struct hmap *lr_ports, > > > > + struct hmap *route_policies, > > > > + struct hmap *bfd_active_connections, > > > > + struct hmap *bfd_connections) > > > > +{ > > > > + struct route_policy *rp; > > > > + > > > > + HMAP_FOR_EACH (rp, key_node, route_policies) { > > > > + if (rp->nbr == od->nbr) { > > > > + rp->stale = true; > > > > + } > > > > + } > > > > + > > > > + for (int i = 0; i < od->nbr->n_policies; i++) { > > > > + const struct nbrec_logical_router_policy *rule = od->nbr->policies[i]; > > > > + size_t n_valid_nexthops = 0; > > > > + char **valid_nexthops = NULL; > > > > + > > > > + if (!strcmp(rule->action, "reroute")) { > > > > + size_t n_nexthops = rule->n_nexthops ? rule->n_nexthops : 1; > > > > + > > > > + valid_nexthops = xcalloc(n_nexthops, sizeof *valid_nexthops); > > > > + for (size_t j = 0; j < n_nexthops; j++) { > > > > + char *nexthop = rule->n_nexthops > > > > + ? rule->nexthops[j] : rule->nexthop; > > > > + struct ovn_port *out_port = > > > > + get_outport_for_routing_policy_nexthop( > > > > + od, lr_ports, rule->priority, nexthop); > > > > + if (!out_port || !check_bfd_state(rule, out_port, nexthop, > > > > + bfd_active_connections, > > > > + bfd_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->nbr = od->nbr; > > > > + > > > > + 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 { > > > > + 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. > > > > * > > > > * A packet that arrives at this table is an IP packet that should be > > > > @@ -12849,7 +13058,7 @@ static void > > > > build_ingress_policy_flows_for_lrouter( > > > > struct ovn_datapath *od, struct lflow_table *lflows, > > > > const struct hmap *lr_ports, > > > > - const struct hmap *bfd_connections, > > > > + struct hmap *route_policies, > > > > struct lflow_ref *lflow_ref) > > > > { > > > > ovs_assert(od->nbr); > > > > @@ -12866,21 +13075,20 @@ build_ingress_policy_flows_for_lrouter( > > > > > > > > /* Convert routing policies to flows. */ > > > > uint16_t ecmp_group_id = 1; > > > > - for (int i = 0; i < od->nbr->n_policies; i++) { > > > > - const struct nbrec_logical_router_policy *rule > > > > - = od->nbr->policies[i]; > > > > + struct route_policy *rp; > > > > + HMAP_FOR_EACH_WITH_HASH (rp, key_node, uuid_hash(&od->key), > > > > + route_policies) { > > > > + const struct nbrec_logical_router_policy *rule = rp->rule; > > > > bool is_ecmp_reroute = > > > > (!strcmp(rule->action, "reroute") && rule->n_nexthops > 1); > > > > > > > > if (is_ecmp_reroute) { > > > > - build_ecmp_routing_policy_flows(lflows, od, lr_ports, rule, > > > > - bfd_connections, ecmp_group_id, > > > > - lflow_ref); > > > > + build_ecmp_routing_policy_flows(lflows, od, lr_ports, rp, > > > > + ecmp_group_id, lflow_ref); > > > > ecmp_group_id++; > > > > } else { > > > > - build_routing_policy_flow(lflows, od, lr_ports, rule, > > > > - bfd_connections, &rule->header_, > > > > - lflow_ref); > > > > + build_routing_policy_flow(lflows, od, lr_ports, rp, > > > > + &rule->header_, lflow_ref); > > > > } > > > > } > > > > } > > > > @@ -15805,6 +16013,9 @@ struct lswitch_flow_build_info { > > > > struct ds actions; > > > > size_t thread_lflow_counter; > > > > const char *svc_monitor_mac; > > > > + struct hmap *parsed_routes; > > > > + struct hmap *route_policies; > > > > + struct simap *route_tables; > > > > }; > > > > > > > > /* Helper function to combine all lflow generation which is iterated by > > > > @@ -15850,12 +16061,12 @@ build_lswitch_and_lrouter_iterate_by_lr(struct ovn_datapath *od, > > > > build_ND_RA_flows_for_lrouter(od, lsi->lflows, NULL); > > > > build_ip_routing_pre_flows_for_lrouter(od, lsi->lflows, NULL); > > > > build_static_route_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports, > > > > - lsi->bfd_connections, > > > > + lsi->parsed_routes, lsi->route_tables, > > > > NULL); > > > > build_mcast_lookup_flows_for_lrouter(od, lsi->lflows, &lsi->match, > > > > &lsi->actions, NULL); > > > > build_ingress_policy_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports, > > > > - lsi->bfd_connections, NULL); > > > > + lsi->route_policies, NULL); > > > > build_arp_resolve_flows_for_lrouter(od, lsi->lflows, NULL); > > > > build_check_pkt_len_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports, > > > > &lsi->match, &lsi->actions, > > > > @@ -16169,7 +16380,10 @@ build_lswitch_and_lrouter_flows( > > > > const struct hmap *svc_monitor_map, > > > > const struct hmap *bfd_connections, > > > > const struct chassis_features *features, > > > > - const char *svc_monitor_mac) > > > > + const char *svc_monitor_mac, > > > > + struct hmap *parsed_routes, > > > > + struct hmap *route_policies, > > > > + struct simap *route_tables) > > > > { > > > > > > > > char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac); > > > > @@ -16203,6 +16417,9 @@ build_lswitch_and_lrouter_flows( > > > > lsiv[index].svc_check_match = svc_check_match; > > > > lsiv[index].thread_lflow_counter = 0; > > > > lsiv[index].svc_monitor_mac = svc_monitor_mac; > > > > + lsiv[index].parsed_routes = parsed_routes; > > > > + lsiv[index].route_tables = route_tables; > > > > + lsiv[index].route_policies = route_policies; > > > > ds_init(&lsiv[index].match); > > > > ds_init(&lsiv[index].actions); > > > > > > > > @@ -16243,6 +16460,9 @@ build_lswitch_and_lrouter_flows( > > > > .features = features, > > > > .svc_check_match = svc_check_match, > > > > .svc_monitor_mac = svc_monitor_mac, > > > > + .parsed_routes = parsed_routes, > > > > + .route_tables = route_tables, > > > > + .route_policies = route_policies, > > > > .match = DS_EMPTY_INITIALIZER, > > > > .actions = DS_EMPTY_INITIALIZER, > > > > }; > > > > @@ -16404,7 +16624,10 @@ void build_lflows(struct ovsdb_idl_txn *ovnsb_txn, > > > > input_data->svc_monitor_map, > > > > input_data->bfd_connections, > > > > input_data->features, > > > > - input_data->svc_monitor_mac); > > > > + input_data->svc_monitor_mac, > > > > + input_data->parsed_routes, > > > > + input_data->route_policies, > > > > + input_data->route_tables); > > > > > > > > if (parallelization_state == STATE_INIT_HASH_SIZES) { > > > > parallelization_state = STATE_USE_PARALLELIZATION; > > > > @@ -17477,6 +17700,27 @@ 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); > > > > +} > > > > + > > > > +void > > > > +static_routes_init(struct static_routes_data *data) > > > > +{ > > > > + hmap_init(&data->parsed_routes); > > > > + simap_init(&data->route_tables); > > > > + hmap_init(&data->bfd_active_connections); > > > > +} > > > > + > > > > +void > > > > +bfd_init(struct bfd_data *data) > > > > +{ > > > > + hmap_init(&data->bfd_connections); > > > > +} > > > > + > > > > void > > > > northd_destroy(struct northd_data *data) > > > > { > > > > @@ -17516,6 +17760,48 @@ northd_destroy(struct northd_data *data) > > > > destroy_northd_tracked_data(data); > > > > } > > > > > > > > +static void > > > > +__bfd_destroy(struct hmap *bfd_connections) > > > > +{ > > > > + struct bfd_entry *bfd_e; > > > > + > > > > + HMAP_FOR_EACH_POP (bfd_e, hmap_node, bfd_connections) { > > > > + bfd_erase_entry(bfd_e); > > > > + } > > > > + hmap_destroy(bfd_connections); > > > > +} > > > > + > > > > +void > > > > +bfd_destroy(struct bfd_data *data) > > > > +{ > > > > + __bfd_destroy(&data->bfd_connections); > > > > +} > > > > + > > > > +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); > > > > +} > > > > + > > > > +void > > > > +static_routes_destroy(struct static_routes_data *data) > > > > +{ > > > > + struct parsed_route *r; > > > > + HMAP_FOR_EACH_POP (r, key_node, &data->parsed_routes) { > > > > + free(r); > > > > + } > > > > + hmap_destroy(&data->parsed_routes); > > > > + > > > > + simap_destroy(&data->route_tables); > > > > + __bfd_destroy(&data->bfd_active_connections); > > > > +} > > > > + > > > > void > > > > ovnnb_db_run(struct northd_input *input_data, > > > > struct northd_data *data, > > > > diff --git a/northd/northd.h b/northd/northd.h > > > > index d4a8d75ab..7ddb72eb7 100644 > > > > --- a/northd/northd.h > > > > +++ b/northd/northd.h > > > > @@ -23,6 +23,7 @@ > > > > #include "northd/en-port-group.h" > > > > #include "northd/ipam.h" > > > > #include "openvswitch/hmap.h" > > > > +#include "simap.h" > > > > #include "ovs-thread.h" > > > > > > > > struct northd_input { > > > > @@ -160,14 +161,34 @@ 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; > > > > + const struct nbrec_logical_router *nbr; > > > > + bool stale; > > > > +}; > > > > + > > > > +struct static_routes_data { > > > > + struct hmap parsed_routes; > > > > + struct simap route_tables; > > > > + struct hmap bfd_active_connections; > > > > +}; > > > > + > > > > +struct route_policies_data { > > > > + struct hmap route_policies; > > > > + struct hmap bfd_active_connections; > > > > +}; > > > > + > > > > +struct bfd_data { > > > > + struct hmap bfd_connections; > > > > +}; > > > > + > > > > struct lr_nat_table; > > > > > > > > struct lflow_input { > > > > - /* Northbound table references */ > > > > - const struct nbrec_bfd_table *nbrec_bfd_table; > > > > - > > > > /* Southbound table references */ > > > > - const struct sbrec_bfd_table *sbrec_bfd_table; > > > > const struct sbrec_logical_flow_table *sbrec_logical_flow_table; > > > > const struct sbrec_multicast_group_table *sbrec_multicast_group_table; > > > > const struct sbrec_igmp_group_table *sbrec_igmp_group_table; > > > > @@ -190,6 +211,9 @@ struct lflow_input { > > > > const struct hmap *svc_monitor_map; > > > > bool ovn_internal_version_changed; > > > > const char *svc_monitor_mac; > > > > + struct hmap *parsed_routes; > > > > + struct hmap *route_policies; > > > > + struct simap *route_tables; > > > > }; > > > > > > > > extern int parallelization_state; > > > > @@ -653,6 +677,20 @@ struct ovn_port { > > > > struct lflow_ref *stateful_lflow_ref; > > > > }; > > > > > > > > +struct parsed_route { > > > > + struct hmap_node key_node; > > > > + struct in6_addr prefix; > > > > + unsigned int plen; > > > > + bool is_src_route; > > > > + uint32_t route_table_id; > > > > + uint32_t hash; > > > > + const struct nbrec_logical_router_static_route *route; > > > > + bool ecmp_symmetric_reply; > > > > + bool is_discard_route; > > > > + const struct nbrec_logical_router *nbr; > > > > + bool stale; > > > > +}; > > > > + > > > > void ovnnb_db_run(struct northd_input *input_data, > > > > struct northd_data *data, > > > > struct ovsdb_idl_txn *ovnnb_txn, > > > > @@ -674,6 +712,18 @@ 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(struct ovn_datapath *, const struct hmap *, > > > > + struct hmap *, struct simap *, struct hmap *, > > > > + struct hmap *); > > > > +uint32_t get_route_table_id(struct simap *, const char *); > > > > +void static_routes_init(struct static_routes_data *); > > > > +void static_routes_destroy(struct static_routes_data *); > > > > + > > > > +void bfd_init(struct bfd_data *); > > > > +void bfd_destroy(struct bfd_data *); > > > > + > > > > struct lflow_table; > > > > struct lr_stateful_tracked_data; > > > > struct ls_stateful_tracked_data; > > > > @@ -711,13 +761,14 @@ bool northd_handle_lb_data_changes(struct tracked_lb_data *, > > > > struct hmap *lbgrp_datapaths_map, > > > > struct northd_tracked_data *); > > > > > > > > -void build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn, > > > > - const struct nbrec_bfd_table *, > > > > - const struct sbrec_bfd_table *, > > > > - const struct hmap *lr_ports, > > > > - struct hmap *bfd_connections); > > > > -void bfd_cleanup_connections(const struct nbrec_bfd_table *, > > > > - struct hmap *bfd_map); > > > > +void build_route_policies(struct ovn_datapath *, struct hmap *, struct hmap *, > > > > + struct hmap *, struct hmap *); > > > > +void bfd_table_sync(struct ovsdb_idl_txn *, const struct nbrec_bfd_table *, > > > > + struct hmap *, struct hmap *, struct hmap *); > > > > +void build_bfd_map(const struct nbrec_bfd_table *, > > > > + const struct sbrec_bfd_table *, > > > > + const struct hmap *lr_ports, > > > > + struct hmap *bfd_connections); > > > > void run_update_worker_pool(int n_threads); > > > > > > > > const struct ovn_datapath *northd_get_datapath_for_port( > > > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > > > > index 57f89a774..1d7f55e4f 100644 > > > > --- a/tests/ovn-northd.at > > > > +++ b/tests/ovn-northd.at > > > > @@ -12,6 +12,7 @@ m4_define([_DUMP_DB_TABLES], [ > > > > ovn-sbctl list meter >> $1 > > > > ovn-sbctl list meter_band >> $1 > > > > ovn-sbctl list port_group >> $1 > > > > + ovn-sbctl list bfd >> $1 > > > > ovn-sbctl dump-flows > lflows_$1 > > > > ]) > > > > > > > > @@ -3861,6 +3862,7 @@ for i in $(seq 1 9); do > > > > check ovn-nbctl --wait=sb lsp-set-addresses sw$i-r0 00:00:00:00:00:0$i > > > > done > > > > > > > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > > > uuid=$(ovn-nbctl create bfd logical_port=r0-sw1 dst_ip=192.168.1.2 status=down min_tx=250 min_rx=250 detect_mult=10) > > > > ovn-nbctl create bfd logical_port=r0-sw2 dst_ip=192.168.2.2 status=down min_tx=500 min_rx=500 detect_mult=20 > > > > ovn-nbctl create bfd logical_port=r0-sw3 dst_ip=192.168.3.2 status=down > > > > @@ -3873,6 +3875,13 @@ wait_row_count bfd 1 logical_port=r0-sw2 detect_mult=20 dst_ip=192.168.2.2 \ > > > > wait_row_count bfd 1 logical_port=r0-sw3 detect_mult=5 dst_ip=192.168.3.2 \ > > > > min_rx=1000 min_tx=1000 status=admin_down > > > > > > > > +check_engine_stats northd norecompute nocompute > > > > +check_engine_stats bfd recompute nocompute > > > > +check_engine_stats lflow recompute nocompute > > > > +check_engine_stats northd_output norecompute compute > > > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > > > + > > > > uuid=$(fetch_column nb:bfd _uuid logical_port=r0-sw1) > > > > check ovn-nbctl set bfd $uuid min_tx=1000 min_rx=1000 detect_mult=100 > > > > > > > > @@ -3881,10 +3890,25 @@ check ovn-nbctl clear bfd $uuid_2 min_rx > > > > wait_row_count bfd 1 logical_port=r0-sw2 min_rx=1000 > > > > wait_row_count bfd 1 logical_port=r0-sw1 min_rx=1000 min_tx=1000 detect_mult=100 > > > > > > > > +check_engine_stats northd norecompute nocompute > > > > +check_engine_stats bfd recompute nocompute > > > > +check_engine_stats lflow recompute nocompute > > > > +check_engine_stats northd_output norecompute compute > > > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > > > + > > > > check ovn-nbctl --bfd=$uuid lr-route-add r0 100.0.0.0/8 192.168.1.2 > > > > wait_column down bfd status logical_port=r0-sw1 > > > > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.1.2 | grep -q bfd],[0]) > > > > > > > > +check_engine_stats northd recompute nocompute > > > > +check_engine_stats bfd recompute nocompute > > > > +check_engine_stats static_routes recompute nocompute > > > > +check_engine_stats lflow recompute nocompute > > > > +check_engine_stats northd_output norecompute compute > > > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > > > + > > > > check ovn-nbctl --bfd lr-route-add r0 200.0.0.0/8 192.168.2.2 > > > > wait_column down bfd status logical_port=r0-sw2 > > > > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.2.2 | grep -q bfd],[0]) > > > > @@ -3893,10 +3917,26 @@ check ovn-nbctl --bfd lr-route-add r0 240.0.0.0/8 192.168.5.2 r0-sw5 > > > > wait_column down bfd status logical_port=r0-sw5 > > > > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.5.2 | grep -q bfd],[0]) > > > > > > > > +check_engine_stats northd recompute nocompute > > > > +check_engine_stats bfd recompute nocompute > > > > +check_engine_stats static_routes recompute nocompute > > > > +check_engine_stats lflow recompute nocompute > > > > +check_engine_stats northd_output norecompute compute > > > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > > > + > > > > check ovn-nbctl --bfd --policy=src-ip lr-route-add r0 192.168.6.1/32 192.168.10.10 r0-sw6 > > > > wait_column down bfd status logical_port=r0-sw6 > > > > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.6.1 | grep -q bfd],[0]) > > > > > > > > +check_engine_stats northd recompute nocompute > > > > +check_engine_stats bfd recompute nocompute > > > > +check_engine_stats route_policies recompute nocompute > > > > +check_engine_stats lflow recompute nocompute > > > > +check_engine_stats northd_output norecompute compute > > > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > > > + > > > > check ovn-nbctl --bfd --policy=src-ip lr-route-add r0 192.168.7.1/32 192.168.10.10 r0-sw7 > > > > wait_column down bfd status logical_port=r0-sw7 > > > > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.7.1 | grep -q bfd],[0]) > > > > @@ -3924,6 +3964,14 @@ wait_column down bfd status logical_port=r0-sw8 > > > > bfd_route_policy_uuid=$(fetch_column nb:bfd _uuid logical_port=r0-sw8) > > > > AT_CHECK([ovn-nbctl list logical_router_policy | grep -q $bfd_route_policy_uuid]) > > > > > > > > +check_engine_stats northd recompute nocompute > > > > +check_engine_stats bfd recompute nocompute > > > > +check_engine_stats static_routes recompute nocompute > > > > +check_engine_stats lflow recompute nocompute > > > > +check_engine_stats northd_output norecompute compute > > > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > > > + > > > > check ovn-nbctl lr-policy-del r0 > > > > check ovn-nbctl --bfd lr-policy-add r0 100 "ip4.src == 2.3.4.0/24" reroute 192.168.9.2,192.168.9.3,192.168.9.4 > > > > > > > > @@ -3931,6 +3979,14 @@ wait_column down bfd status dst_ip=192.168.9.2 > > > > wait_column down bfd status dst_ip=192.168.9.3 > > > > wait_column down bfd status dst_ip=192.168.9.4 > > > > > > > > +check_engine_stats northd recompute nocompute > > > > +check_engine_stats bfd recompute nocompute > > > > +check_engine_stats route_policies recompute nocompute > > > > +check_engine_stats lflow recompute nocompute > > > > +check_engine_stats northd_output norecompute compute > > > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > > > > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > > > > + > > > > bfd_route_policy_uuid=$(fetch_column nb:bfd _uuid logical_port=r0-sw9) > > > > AT_CHECK([ovn-nbctl list logical_router_policy | sed s/,//g | grep -q "$bfd_route_policy_uuid"]) > > > > > > > > -- > > > > 2.45.2 > > > > > > > >
diff --git a/northd/en-lflow.c b/northd/en-lflow.c index c4b927fb8..3dba5034b 100644 --- a/northd/en-lflow.c +++ b/northd/en-lflow.c @@ -41,6 +41,11 @@ lflow_get_input_data(struct engine_node *node, struct lflow_input *lflow_input) { struct northd_data *northd_data = engine_get_input_data("northd", node); + struct bfd_data *bfd_data = engine_get_input_data("bfd", node); + struct static_routes_data *static_routes_data = + engine_get_input_data("static_routes", node); + struct route_policies_data *route_policies_data = + engine_get_input_data("route_policies", node); struct port_group_data *pg_data = engine_get_input_data("port_group", node); struct sync_meters_data *sync_meters_data = @@ -50,10 +55,6 @@ lflow_get_input_data(struct engine_node *node, struct ed_type_ls_stateful *ls_stateful_data = engine_get_input_data("ls_stateful", node); - lflow_input->nbrec_bfd_table = - EN_OVSDB_GET(engine_get_input("NB_bfd", node)); - lflow_input->sbrec_bfd_table = - EN_OVSDB_GET(engine_get_input("SB_bfd", node)); lflow_input->sbrec_logical_flow_table = EN_OVSDB_GET(engine_get_input("SB_logical_flow", node)); lflow_input->sbrec_multicast_group_table = @@ -78,7 +79,10 @@ lflow_get_input_data(struct engine_node *node, lflow_input->meter_groups = &sync_meters_data->meter_groups; lflow_input->lb_datapaths_map = &northd_data->lb_datapaths_map; lflow_input->svc_monitor_map = &northd_data->svc_monitor_map; - lflow_input->bfd_connections = NULL; + lflow_input->bfd_connections = &bfd_data->bfd_connections; + lflow_input->parsed_routes = &static_routes_data->parsed_routes; + lflow_input->route_tables = &static_routes_data->route_tables; + lflow_input->route_policies = &route_policies_data->route_policies; struct ed_type_global_config *global_config = engine_get_input_data("global_config", node); @@ -95,25 +99,14 @@ void en_lflow_run(struct engine_node *node, void *data) struct lflow_input lflow_input; lflow_get_input_data(node, &lflow_input); - struct hmap bfd_connections = HMAP_INITIALIZER(&bfd_connections); - lflow_input.bfd_connections = &bfd_connections; - stopwatch_start(BUILD_LFLOWS_STOPWATCH_NAME, time_msec()); struct lflow_data *lflow_data = data; lflow_table_clear(lflow_data->lflow_table); lflow_reset_northd_refs(&lflow_input); - build_bfd_table(eng_ctx->ovnsb_idl_txn, - lflow_input.nbrec_bfd_table, - lflow_input.sbrec_bfd_table, - lflow_input.lr_ports, - &bfd_connections); build_lflows(eng_ctx->ovnsb_idl_txn, &lflow_input, lflow_data->lflow_table); - bfd_cleanup_connections(lflow_input.nbrec_bfd_table, - &bfd_connections); - hmap_destroy(&bfd_connections); stopwatch_stop(BUILD_LFLOWS_STOPWATCH_NAME, time_msec()); engine_set_node_state(node, EN_UPDATED); diff --git a/northd/en-northd.c b/northd/en-northd.c index 4479b4aff..817a4736c 100644 --- a/northd/en-northd.c +++ b/northd/en-northd.c @@ -236,6 +236,172 @@ northd_global_config_handler(struct engine_node *node, void *data OVS_UNUSED) return true; } +bool +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 false; + } + + /* This node uses the below data from the en_northd engine node. + * See (lr_stateful_get_input_data()) + * 1. northd_data->lr_datapaths + * 2. northd_data->lr_ports + * 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. + * Note: When we add I-P to the created/deleted logical routers or + * logical router ports, we need to revisit this handler. + * + * This node also accesses the route policies of the logical router. + * When these route policies get updated, en_northd engine recomputes + * and so does this node. + * Note: When we add I-P to handle route policies changes, we need + * to revisit this handler. + */ + return true; +} + +void +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, &northd_data->lr_ports, + &route_policies_data->route_policies, + &route_policies_data->bfd_active_connections, + &bfd_data->bfd_connections); + } + + engine_set_node_state(node, EN_UPDATED); +} + +bool +static_routes_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 false; + } + + /* This node uses the below data from the en_northd engine node. + * See (lr_stateful_get_input_data()) + * 1. northd_data->lr_datapaths + * 2. northd_data->lr_ports + * 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. + * Note: When we add I-P to the created/deleted logical routers or + * logical router ports, we need to revisit this handler. + * + * This node also accesses the static routes of the logical router. + * When these static routes gets updated, en_northd engine recomputes + * and so does this node. + * Note: When we add I-P to handle static routes changes, we need + * to revisit this handler. + */ + return true; +} + +void +en_static_routes_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 static_routes_data *static_routes_data = data; + + static_routes_destroy(data); + static_routes_init(data); + + struct ovn_datapath *od; + HMAP_FOR_EACH (od, key_node, &northd_data->lr_datapaths.datapaths) { + for (int i = 0; i < od->nbr->n_ports; i++) { + const char *route_table_name = + smap_get(&od->nbr->ports[i]->options, "route_table"); + get_route_table_id(&static_routes_data->route_tables, + route_table_name); + } + + build_parsed_routes(od, &northd_data->lr_ports, + &static_routes_data->parsed_routes, + &static_routes_data->route_tables, + &static_routes_data->bfd_active_connections, + &bfd_data->bfd_connections); + } + + engine_set_node_state(node, EN_UPDATED); +} + +bool +bfd_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 false; + } + + /* This node uses the below data from the en_northd engine node. + * See (lr_stateful_get_input_data()) + * 1. northd_data->lr_ports + * This data gets updated when a logical router port is created or + * deleted. Northd engine node presently falls back to full recompute + * when this happens and so does this node. + * Note: When we add I-P to the created/deleted logical router ports, + * we need to revisit this handler. + */ + return true; +} + +void +en_bfd_run(struct engine_node *node, void *data) +{ + struct northd_data *northd_data = engine_get_input_data("northd", node); + struct bfd_data *bfd_data = data; + const struct nbrec_bfd_table *nbrec_bfd_table = + EN_OVSDB_GET(engine_get_input("NB_bfd", node)); + const struct sbrec_bfd_table *sbrec_bfd_table = + EN_OVSDB_GET(engine_get_input("SB_bfd", node)); + + bfd_destroy(data); + bfd_init(data); + build_bfd_map(nbrec_bfd_table, sbrec_bfd_table, + &northd_data->lr_ports, &bfd_data->bfd_connections); + engine_set_node_state(node, EN_UPDATED); +} + +void +en_bfd_sync_run(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED) +{ + const struct engine_context *eng_ctx = engine_get_context(); + struct bfd_data *bfd_data = engine_get_input_data("bfd", node); + struct route_policies_data *route_policies_data + = engine_get_input_data("route_policies", node); + struct static_routes_data *static_routes_data + = engine_get_input_data("static_routes", node); + const struct nbrec_bfd_table *nbrec_bfd_table = + EN_OVSDB_GET(engine_get_input("NB_bfd", node)); + + bfd_table_sync(eng_ctx->ovnsb_idl_txn, nbrec_bfd_table, + &bfd_data->bfd_connections, + &route_policies_data->bfd_active_connections, + &static_routes_data->bfd_active_connections); + engine_set_node_state(node, EN_UPDATED); +} + void *en_northd_init(struct engine_node *node OVS_UNUSED, struct engine_arg *arg OVS_UNUSED) @@ -247,6 +413,42 @@ 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_static_routes_init(struct engine_node *node OVS_UNUSED, + struct engine_arg *arg OVS_UNUSED) +{ + struct static_routes_data *data = xzalloc(sizeof *data); + + static_routes_init(data); + return data; +} + +void +*en_bfd_init(struct engine_node *node OVS_UNUSED, + struct engine_arg *arg OVS_UNUSED) +{ + struct bfd_data *data = xzalloc(sizeof *data); + + bfd_init(data); + return data; +} + +void *en_bfd_sync_init(struct engine_node *node OVS_UNUSED, + struct engine_arg *arg OVS_UNUSED) +{ + return NULL; +} + void en_northd_cleanup(void *data) { @@ -259,3 +461,26 @@ en_northd_clear_tracked_data(void *data_) struct northd_data *data = data_; destroy_northd_data_tracked_changes(data); } + +void +en_route_policies_cleanup(void *data) +{ + route_policies_destroy(data); +} + +void +en_static_routes_cleanup(void *data) +{ + static_routes_destroy(data); +} + +void +en_bfd_cleanup(void *data) +{ + bfd_destroy(data); +} + +void +en_bfd_sync_cleanup(void *data OVS_UNUSED) +{ +} diff --git a/northd/en-northd.h b/northd/en-northd.h index 9b7bda32a..68f7755f8 100644 --- a/northd/en-northd.h +++ b/northd/en-northd.h @@ -19,5 +19,28 @@ bool northd_nb_logical_switch_handler(struct engine_node *, void *data); bool northd_nb_logical_router_handler(struct engine_node *, void *data); bool northd_sb_port_binding_handler(struct engine_node *, void *data); bool northd_lb_data_handler(struct engine_node *, void *data); +void *en_static_routes_init(struct engine_node *node OVS_UNUSED, + struct engine_arg *arg OVS_UNUSED); +void en_route_policies_cleanup(void *data); +bool route_policies_northd_change_handler(struct engine_node *node, + void *data OVS_UNUSED); +void 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_static_routes_cleanup(void *data); +bool static_routes_northd_change_handler(struct engine_node *node, + void *data OVS_UNUSED); +void en_static_routes_run(struct engine_node *node, void *data); +void *en_bfd_init(struct engine_node *node OVS_UNUSED, + struct engine_arg *arg OVS_UNUSED); +void en_bfd_cleanup(void *data); +bool bfd_northd_change_handler(struct engine_node *node, + void *data OVS_UNUSED); +void en_bfd_run(struct engine_node *node, void *data); +void *en_bfd_sync_init(struct engine_node *node OVS_UNUSED, + struct engine_arg *arg OVS_UNUSED); +void en_bfd_sync_run(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED); +void en_bfd_sync_cleanup(void *data OVS_UNUSED); #endif /* EN_NORTHD_H */ diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index 522236ad2..77610cdd8 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -155,6 +155,10 @@ static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lb_data, "lb_data"); static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lr_nat, "lr_nat"); static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lr_stateful, "lr_stateful"); static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(ls_stateful, "ls_stateful"); +static ENGINE_NODE(route_policies, "route_policies"); +static ENGINE_NODE(static_routes, "static_routes"); +static ENGINE_NODE(bfd, "bfd"); +static ENGINE_NODE(bfd_sync, "bfd_sync"); void inc_proc_northd_init(struct ovsdb_idl_loop *nb, struct ovsdb_idl_loop *sb) @@ -237,18 +241,37 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, engine_add_input(&en_fdb_aging, &en_global_config, node_global_config_handler); + engine_add_input(&en_bfd, &en_nb_bfd, NULL); + engine_add_input(&en_bfd, &en_sb_bfd, NULL); + engine_add_input(&en_bfd, &en_northd, bfd_northd_change_handler); + + engine_add_input(&en_route_policies, &en_bfd, NULL); + engine_add_input(&en_route_policies, &en_northd, + route_policies_northd_change_handler); + + engine_add_input(&en_static_routes, &en_bfd, NULL); + engine_add_input(&en_static_routes, &en_northd, + static_routes_northd_change_handler); + + engine_add_input(&en_bfd_sync, &en_bfd, NULL); + engine_add_input(&en_bfd_sync, &en_nb_bfd, NULL); + engine_add_input(&en_bfd_sync, &en_static_routes, NULL); + engine_add_input(&en_bfd_sync, &en_route_policies, NULL); + engine_add_input(&en_sync_meters, &en_nb_acl, NULL); engine_add_input(&en_sync_meters, &en_nb_meter, NULL); engine_add_input(&en_sync_meters, &en_sb_meter, NULL); - engine_add_input(&en_lflow, &en_nb_bfd, NULL); engine_add_input(&en_lflow, &en_nb_acl, NULL); engine_add_input(&en_lflow, &en_sync_meters, NULL); - engine_add_input(&en_lflow, &en_sb_bfd, NULL); engine_add_input(&en_lflow, &en_sb_logical_flow, NULL); engine_add_input(&en_lflow, &en_sb_multicast_group, NULL); engine_add_input(&en_lflow, &en_sb_igmp_group, NULL); engine_add_input(&en_lflow, &en_sb_logical_dp_group, NULL); + engine_add_input(&en_lflow, &en_bfd, NULL); + engine_add_input(&en_lflow, &en_bfd_sync, NULL); + engine_add_input(&en_lflow, &en_route_policies, NULL); + engine_add_input(&en_lflow, &en_static_routes, NULL); engine_add_input(&en_lflow, &en_global_config, node_global_config_handler); engine_add_input(&en_lflow, &en_northd, lflow_northd_handler); diff --git a/northd/northd.c b/northd/northd.c index 5c2fd74ff..18fe2b48a 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -9689,11 +9689,49 @@ 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; - bool ref; + struct ovn_port *op; + + char *logical_port; + char *dst_ip; + char *status; + bool stale; }; +static struct bfd_entry * +bfd_alloc_entry(struct hmap *bfd_connections, + const char *logical_port, const char *dst_ip, + const char *status) +{ + struct bfd_entry *bfd_e = xzalloc(sizeof *bfd_e); + bfd_e->logical_port = xstrdup(logical_port); + bfd_e->dst_ip = xstrdup(dst_ip); + bfd_e->status = xstrdup(status); + uint32_t hash = hash_string(dst_ip, 0); + hash = hash_string(logical_port, hash); + hmap_insert(bfd_connections, &bfd_e->hmap_node, hash); + + return bfd_e; +} + +static void +bfd_erase_entry(struct bfd_entry *bfd_e) +{ + free(bfd_e->logical_port); + free(bfd_e->dst_ip); + free(bfd_e->status); + free(bfd_e); +} + +static 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 * bfd_port_lookup(const struct hmap *bfd_map, const char *logical_port, const char *dst_ip) @@ -9704,36 +9742,13 @@ bfd_port_lookup(const struct hmap *bfd_map, const char *logical_port, hash = hash_string(dst_ip, 0); hash = hash_string(logical_port, hash); HMAP_FOR_EACH_WITH_HASH (bfd_e, hmap_node, hash, bfd_map) { - if (!strcmp(bfd_e->sb_bt->logical_port, logical_port) && - !strcmp(bfd_e->sb_bt->dst_ip, dst_ip)) { + if (!strcmp(bfd_e->logical_port, logical_port) && + !strcmp(bfd_e->dst_ip, dst_ip)) { return bfd_e; } } - return NULL; -} -void -bfd_cleanup_connections(const struct nbrec_bfd_table *nbrec_bfd_table, - struct hmap *bfd_map) -{ - const struct nbrec_bfd *nb_bt; - struct bfd_entry *bfd_e; - - NBREC_BFD_TABLE_FOR_EACH (nb_bt, nbrec_bfd_table) { - bfd_e = bfd_port_lookup(bfd_map, nb_bt->logical_port, nb_bt->dst_ip); - if (!bfd_e) { - continue; - } - - if (!bfd_e->ref && strcmp(nb_bt->status, "admin_down")) { - /* no user for this bfd connection */ - nbrec_bfd_set_status(nb_bt, "admin_down"); - } - } - - HMAP_FOR_EACH_POP (bfd_e, hmap_node, bfd_map) { - free(bfd_e); - } + return NULL; } #define BFD_DEF_MINTX 1000 /* 1s */ @@ -9797,50 +9812,64 @@ static int bfd_get_unused_port(unsigned long *bfd_src_ports) } void -build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn, - const struct nbrec_bfd_table *nbrec_bfd_table, - const struct sbrec_bfd_table *sbrec_bfd_table, - const struct hmap *lr_ports, struct hmap *bfd_connections) +bfd_table_sync(struct ovsdb_idl_txn *ovnsb_txn, + const struct nbrec_bfd_table *nbrec_bfd_table, + struct hmap *bfd_connections, + struct hmap *rp_bfd_connections, + struct hmap *sr_bfd_connections) { - struct hmap sb_only = HMAP_INITIALIZER(&sb_only); - const struct sbrec_bfd *sb_bt; - unsigned long *bfd_src_ports; - struct bfd_entry *bfd_e; - uint32_t hash; + if (!ovnsb_txn) { + return; + } - bfd_src_ports = bitmap_allocate(BFD_UDP_SRC_PORT_LEN); + unsigned long *bfd_src_ports = bitmap_allocate(BFD_UDP_SRC_PORT_LEN); - SBREC_BFD_TABLE_FOR_EACH (sb_bt, sbrec_bfd_table) { - bfd_e = xmalloc(sizeof *bfd_e); - bfd_e->sb_bt = sb_bt; - hash = hash_string(sb_bt->dst_ip, 0); - hash = hash_string(sb_bt->logical_port, hash); - hmap_insert(&sb_only, &bfd_e->hmap_node, hash); - bitmap_set1(bfd_src_ports, sb_bt->src_port - BFD_UDP_SRC_PORT_START); + struct bfd_entry *bfd_e; + HMAP_FOR_EACH (bfd_e, hmap_node, bfd_connections) { + bfd_e->stale = true; + /* we need to check if this entry is even in the BFD nb db table */ + if (bfd_e->sb_bt) { + bitmap_set1(bfd_src_ports, + bfd_e->sb_bt->src_port - BFD_UDP_SRC_PORT_START); + } } const struct nbrec_bfd *nb_bt; NBREC_BFD_TABLE_FOR_EACH (nb_bt, nbrec_bfd_table) { - if (!nb_bt->status) { - /* default state is admin_down */ - nbrec_bfd_set_status(nb_bt, "admin_down"); + bfd_e = bfd_port_lookup(bfd_connections, nb_bt->logical_port, + nb_bt->dst_ip); + if (!bfd_e) { + continue; } - struct ovn_port *op = ovn_port_find(lr_ports, nb_bt->logical_port); - bfd_e = bfd_port_lookup(&sb_only, nb_bt->logical_port, nb_bt->dst_ip); - if (!bfd_e) { + struct ovn_port *op = bfd_e->op; + if (!op) { + continue; + } + + if (!bfd_port_lookup(rp_bfd_connections, nb_bt->logical_port, + nb_bt->dst_ip) && + !bfd_port_lookup(sr_bfd_connections, nb_bt->logical_port, + nb_bt->dst_ip)) { + /* no consumer for this bfd connection, mark it as admin_down. */ + bfd_set_status(bfd_e, "admin_down"); + } + nbrec_bfd_set_status(nb_bt, bfd_e->status); + + if (!bfd_e->sb_bt) { int udp_src = bfd_get_unused_port(bfd_src_ports); if (udp_src < 0) { continue; } - sb_bt = sbrec_bfd_insert(ovnsb_txn); + /* Add entry to bfd sb table. */ + const struct sbrec_bfd *sb_bt = sbrec_bfd_insert(ovnsb_txn); sbrec_bfd_set_logical_port(sb_bt, nb_bt->logical_port); sbrec_bfd_set_dst_ip(sb_bt, nb_bt->dst_ip); sbrec_bfd_set_disc(sb_bt, 1 + random_uint32()); sbrec_bfd_set_src_port(sb_bt, udp_src); sbrec_bfd_set_status(sb_bt, nb_bt->status); - if (op && op->sb && op->sb->chassis) { + if (op->sb->chassis) { sbrec_bfd_set_chassis_name(sb_bt, op->sb->chassis->name); } @@ -9858,41 +9887,77 @@ build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn, sbrec_bfd_set_status(bfd_e->sb_bt, nb_bt->status); } else { nbrec_bfd_set_status(nb_bt, bfd_e->sb_bt->status); + bfd_set_status(bfd_e, bfd_e->sb_bt->status); } } + build_bfd_update_sb_conf(nb_bt, bfd_e->sb_bt); - if (op && op->sb && op->sb->chassis && - strcmp(op->sb->chassis->name, bfd_e->sb_bt->chassis_name)) { + if (op->sb->chassis && !strcmp(op->sb->chassis->name, + bfd_e->sb_bt->chassis_name)) { sbrec_bfd_set_chassis_name(bfd_e->sb_bt, op->sb->chassis->name); } - - hmap_remove(&sb_only, &bfd_e->hmap_node); - bfd_e->ref = false; - hash = hash_string(bfd_e->sb_bt->dst_ip, 0); - hash = hash_string(bfd_e->sb_bt->logical_port, hash); - hmap_insert(bfd_connections, &bfd_e->hmap_node, hash); } + bfd_e->stale = false; + } - if (op) { - op->has_bfd = true; + HMAP_FOR_EACH (bfd_e, hmap_node, bfd_connections) { + if (!bfd_e->stale) { + continue; } - } - HMAP_FOR_EACH_POP (bfd_e, hmap_node, &sb_only) { - struct ovn_port *op = ovn_port_find(lr_ports, - bfd_e->sb_bt->logical_port); - if (op) { - op->has_bfd = false; + if (bfd_e->op) { + bfd_e->op->has_bfd = false; } sbrec_bfd_delete(bfd_e->sb_bt); - free(bfd_e); } - hmap_destroy(&sb_only); bitmap_free(bfd_src_ports); } +void +build_bfd_map(const struct nbrec_bfd_table *nbrec_bfd_table, + const struct sbrec_bfd_table *sbrec_bfd_table, + const struct hmap *lr_ports, struct hmap *bfd_connections) +{ + struct bfd_entry *bfd_e; + + /* align bfd_map to sb db */ + const struct sbrec_bfd *sb_bt; + SBREC_BFD_TABLE_FOR_EACH (sb_bt, sbrec_bfd_table) { + bfd_e = bfd_port_lookup(bfd_connections, sb_bt->logical_port, + sb_bt->dst_ip); + if (!bfd_e) { + bfd_e = bfd_alloc_entry(bfd_connections, sb_bt->logical_port, + sb_bt->dst_ip, sb_bt->status); + } + bfd_e->sb_bt = sb_bt; + } + + const struct nbrec_bfd *nb_bt; + NBREC_BFD_TABLE_FOR_EACH (nb_bt, nbrec_bfd_table) { + struct ovn_port *op = ovn_port_find(lr_ports, nb_bt->logical_port); + if (!op || !op->sb) { + /* skip not bounded ports */ + continue; + } + + bfd_e = bfd_port_lookup(bfd_connections, nb_bt->logical_port, + nb_bt->dst_ip); + if (!bfd_e) { + /* brand new entry. */ + bfd_e = bfd_alloc_entry(bfd_connections, nb_bt->logical_port, + nb_bt->dst_ip, "admin_down"); + } + + bfd_e->nb_bt = nb_bt; + bfd_e->op = op; + if (op) { + op->has_bfd = true; + } + } +} + /* Returns a string of the IP address of the router port 'op' that * overlaps with 'ip_s". If one is not found, returns NULL. * @@ -9928,17 +9993,13 @@ get_outport_for_routing_policy_nexthop(struct ovn_datapath *od, return NULL; } -static struct ovs_mutex bfd_lock = OVS_MUTEX_INITIALIZER; - -static bool check_bfd_state( - const struct nbrec_logical_router_policy *rule, - const struct hmap *bfd_connections, - struct ovn_port *out_port, - const char *nexthop) +static bool check_bfd_state(const struct nbrec_logical_router_policy *rule, + struct ovn_port *out_port, const char *nexthop, + struct hmap *bfd_active_connections, + struct hmap *bfd_connections) { struct in6_addr nexthop_v6; bool is_nexthop_v6 = ipv6_parse(nexthop, &nexthop_v6); - bool ret = true; for (size_t i = 0; i < rule->n_bfd_sessions; i++) { /* Check if there is a BFD session associated to the reroute @@ -9963,39 +10024,45 @@ static bool check_bfd_state( struct bfd_entry *bfd_e = bfd_port_lookup(bfd_connections, nb_bt->logical_port, nb_bt->dst_ip); - ovs_mutex_lock(&bfd_lock); - if (bfd_e) { - bfd_e->ref = true; + if (!bfd_e) { + continue; } - if (!strcmp(nb_bt->status, "admin_down")) { - nbrec_bfd_set_status(nb_bt, "down"); + if (!strcmp(bfd_e->status, "admin_down")) { + bfd_set_status(bfd_e, "down"); } - ret = strcmp(nb_bt->status, "down"); - ovs_mutex_unlock(&bfd_lock); - break; + /* This route policy is linked to an active bfd session. */ + if (!bfd_port_lookup(bfd_active_connections, nb_bt->logical_port, + nb_bt->dst_ip)) { + bfd_alloc_entry(bfd_active_connections, nb_bt->logical_port, + nb_bt->dst_ip, bfd_e->status); + } + + return strcmp(bfd_e->status, "down"); } - return ret; + return true; } static void build_routing_policy_flow(struct lflow_table *lflows, struct ovn_datapath *od, - const struct hmap *lr_ports, - const struct nbrec_logical_router_policy *rule, - const struct hmap *bfd_connections, + const struct hmap *lr_ports, struct route_policy *rp, const struct ovsdb_idl_row *stage_hint, struct lflow_ref *lflow_ref) { + const struct nbrec_logical_router_policy *rule = rp->rule; struct ds match = DS_EMPTY_INITIALIZER; struct ds actions = DS_EMPTY_INITIALIZER; if (!strcmp(rule->action, "reroute")) { ovs_assert(rule->n_nexthops <= 1); - char *nexthop = - (rule->n_nexthops == 1 ? rule->nexthops[0] : rule->nexthop); + if (!rp->n_valid_nexthops) { + return; + } + + char *nexthop = rp->valid_nexthops[0]; struct ovn_port *out_port = get_outport_for_routing_policy_nexthop( od, lr_ports, rule->priority, nexthop); if (!out_port) { @@ -10011,10 +10078,6 @@ build_routing_policy_flow(struct lflow_table *lflows, struct ovn_datapath *od, return; } - if (!check_bfd_state(rule, bfd_connections, out_port, nexthop)) { - return; - } - uint32_t pkt_mark = smap_get_uint(&rule->options, "pkt_mark", 0); if (pkt_mark) { ds_put_format(&actions, "pkt.mark = %u; ", pkt_mark); @@ -10057,19 +10120,18 @@ static void build_ecmp_routing_policy_flows(struct lflow_table *lflows, struct ovn_datapath *od, const struct hmap *lr_ports, - const struct nbrec_logical_router_policy *rule, - const struct hmap *bfd_connections, + struct route_policy *rp, uint16_t ecmp_group_id, struct lflow_ref *lflow_ref) { - ovs_assert(rule->n_nexthops > 1); - bool nexthops_is_ipv4 = true; + const struct nbrec_logical_router_policy *rule = rp->rule; + ovs_assert(rule->n_nexthops > 1); /* Check that all the nexthops belong to the same addr family before * adding logical flows. */ - for (uint16_t i = 0; i < rule->n_nexthops; i++) { - bool is_ipv4 = strchr(rule->nexthops[i], '.') ? true : false; + for (uint16_t i = 0; i < rp->n_valid_nexthops; i++) { + bool is_ipv4 = strchr(rp->valid_nexthops[i], '.') ? true : false; if (i == 0) { nexthops_is_ipv4 = is_ipv4; @@ -10080,7 +10142,7 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, VLOG_WARN_RL(&rl, "nexthop [%s] of the router policy with " "the match [%s] do not belong to the same address " "family as other next hops", - rule->nexthops[i], rule->match); + rp->valid_nexthops[i], rule->match); return; } } @@ -10088,40 +10150,30 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, struct ds match = DS_EMPTY_INITIALIZER; struct ds actions = DS_EMPTY_INITIALIZER; - size_t *valid_nexthops = xcalloc(rule->n_nexthops, sizeof *valid_nexthops); - size_t n_valid_nexthops = 0; - - for (size_t i = 0; i < rule->n_nexthops; i++) { + for (size_t i = 0; i < rp->n_valid_nexthops; i++) { struct ovn_port *out_port = get_outport_for_routing_policy_nexthop( - od, lr_ports, rule->priority, rule->nexthops[i]); + od, lr_ports, rule->priority, rp->valid_nexthops[i]); if (!out_port) { goto cleanup; } const char *lrp_addr_s = - find_lrp_member_ip(out_port, rule->nexthops[i]); + find_lrp_member_ip(out_port, rp->valid_nexthops[i]); if (!lrp_addr_s) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); VLOG_WARN_RL(&rl, "lrp_addr not found for routing policy " " priority %"PRId64" nexthop %s", - rule->priority, rule->nexthops[i]); + rule->priority, rp->valid_nexthops[i]); goto cleanup; } - if (!check_bfd_state(rule, bfd_connections, out_port, - rule->nexthops[i])) { - continue; - } - - valid_nexthops[n_valid_nexthops++] = i + 1; - ds_clear(&actions); uint32_t pkt_mark = smap_get_uint(&rule->options, "pkt_mark", 0); if (pkt_mark) { ds_put_format(&actions, "pkt.mark = %u; ", pkt_mark); } - bool is_ipv4 = strchr(rule->nexthops[i], '.') ? true : false; + bool is_ipv4 = strchr(rp->valid_nexthops[i], '.') ? true : false; ds_put_format(&actions, "%s = %s; " "%s = %s; " @@ -10130,7 +10182,7 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, "flags.loopback = 1; " "next;", is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6, - rule->nexthops[i], + rp->valid_nexthops[i], is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6, lrp_addr_s, out_port->lrp_networks.ea_s, @@ -10146,37 +10198,30 @@ build_ecmp_routing_policy_flows(struct lflow_table *lflows, lflow_ref); } - if (!n_valid_nexthops) { - goto cleanup; - } - ds_clear(&actions); - if (n_valid_nexthops > 1) { + if (rp->n_valid_nexthops > 1) { ds_put_format(&actions, "%s = %"PRIu16 "; %s = select(", REG_ECMP_GROUP_ID, ecmp_group_id, REG_ECMP_MEMBER_ID); - for (size_t i = 0; i < n_valid_nexthops; i++) { + for (size_t i = 0; i < rp->n_valid_nexthops; i++) { if (i > 0) { ds_put_cstr(&actions, ", "); } - ds_put_format(&actions, "%"PRIuSIZE, valid_nexthops[i]); + ds_put_format(&actions, "%"PRIuSIZE, i + 1); } ds_put_cstr(&actions, ");"); } else { ds_put_format(&actions, "%s = %"PRIu16 - "; %s = %"PRIuSIZE"; next;", REG_ECMP_GROUP_ID, - ecmp_group_id, REG_ECMP_MEMBER_ID, - valid_nexthops[0]); + "; %s = 1; next;", REG_ECMP_GROUP_ID, + ecmp_group_id, REG_ECMP_MEMBER_ID); } ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_POLICY, rule->priority, rule->match, ds_cstr(&actions), &rule->header_, lflow_ref); - cleanup: - free(valid_nexthops); ds_destroy(&match); ds_destroy(&actions); } @@ -10201,7 +10246,7 @@ route_table_add(struct simap *route_tables, const char *route_table_name) return rtb_id; } -static uint32_t +uint32_t get_route_table_id(struct simap *route_tables, const char *route_table_name) { if (!route_table_name || !route_table_name[0]) { @@ -10242,18 +10287,6 @@ build_route_table_lflow(struct ovn_datapath *od, struct lflow_table *lflows, ds_destroy(&actions); } -struct parsed_route { - struct ovs_list list_node; - struct in6_addr prefix; - unsigned int plen; - bool is_src_route; - uint32_t route_table_id; - uint32_t hash; - const struct nbrec_logical_router_static_route *route; - bool ecmp_symmetric_reply; - bool is_discard_route; -}; - static uint32_t route_hash(struct parsed_route *route) { @@ -10268,11 +10301,53 @@ find_static_route_outport(struct ovn_datapath *od, const struct hmap *lr_ports, /* Parse and validate the route. Return the parsed route if successful. * Otherwise return NULL. */ + static struct parsed_route * +parsed_route_lookup(struct hmap *routes, size_t hash, + struct parsed_route *new_pr) +{ + struct parsed_route *pr; + HMAP_FOR_EACH_WITH_HASH (pr, key_node, hash, routes) { + if (pr->plen != new_pr->plen) { + continue; + } + + if (memcmp(&pr->prefix, &new_pr->prefix, sizeof(struct in6_addr))) { + continue; + } + + if (pr->is_src_route != new_pr->is_src_route) { + continue; + } + + if (pr->route_table_id != new_pr->route_table_id) { + continue; + } + + if (pr->route != new_pr->route) { + continue; + } + + if (pr->ecmp_symmetric_reply != new_pr->ecmp_symmetric_reply) { + continue; + } + + if (pr->is_discard_route != new_pr->is_discard_route) { + continue; + } + + return pr; + } + + return NULL; +} + +static void parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, - struct ovs_list *routes, struct simap *route_tables, + struct hmap *routes, struct simap *route_tables, const struct nbrec_logical_router_static_route *route, - const struct hmap *bfd_connections) + struct hmap *bfd_active_connections, + struct hmap *bfd_connections) { /* Verify that the next hop is an IP address with an all-ones mask. */ struct in6_addr nexthop; @@ -10285,7 +10360,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, VLOG_WARN_RL(&rl, "bad 'nexthop' %s in static route " UUID_FMT, route->nexthop, UUID_ARGS(&route->header_.uuid)); - return NULL; + return; } if ((IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 32) || (!IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 128)) { @@ -10293,7 +10368,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, VLOG_WARN_RL(&rl, "bad next hop mask %s in static route " UUID_FMT, route->nexthop, UUID_ARGS(&route->header_.uuid)); - return NULL; + return; } } @@ -10304,7 +10379,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, VLOG_WARN_RL(&rl, "bad 'ip_prefix' %s in static route " UUID_FMT, route->ip_prefix, UUID_ARGS(&route->header_.uuid)); - return NULL; + return; } /* Verify that ip_prefix and nexthop have same address familiy. */ @@ -10315,7 +10390,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, " %s and 'nexthop' %s in static route "UUID_FMT, route->ip_prefix, route->nexthop, UUID_ARGS(&route->header_.uuid)); - return NULL; + return; } } @@ -10324,52 +10399,85 @@ parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports, !find_static_route_outport(od, lr_ports, route, IN6_IS_ADDR_V4MAPPED(&prefix), NULL, NULL)) { - return NULL; + return; } const struct nbrec_bfd *nb_bt = route->bfd; if (nb_bt && !strcmp(nb_bt->dst_ip, route->nexthop)) { - struct bfd_entry *bfd_e; + struct bfd_entry *bfd_e = bfd_port_lookup(bfd_connections, + nb_bt->logical_port, + nb_bt->dst_ip); + if (!bfd_e) { + return; + } - bfd_e = bfd_port_lookup(bfd_connections, nb_bt->logical_port, - nb_bt->dst_ip); - ovs_mutex_lock(&bfd_lock); - if (bfd_e) { - bfd_e->ref = true; + if (!strcmp(bfd_e->status, "admin_down")) { + bfd_set_status(bfd_e, "down"); } - if (!strcmp(nb_bt->status, "admin_down")) { - nbrec_bfd_set_status(nb_bt, "down"); + /* This static route is linked to an active bfd session. */ + if (!bfd_port_lookup(bfd_active_connections, nb_bt->logical_port, + nb_bt->dst_ip)) { + bfd_alloc_entry(bfd_active_connections, nb_bt->logical_port, + nb_bt->dst_ip, bfd_e->status); } - if (!strcmp(nb_bt->status, "down")) { - ovs_mutex_unlock(&bfd_lock); - return NULL; + if (!strcmp(bfd_e->status, "down")) { + return; } - ovs_mutex_unlock(&bfd_lock); } - struct parsed_route *pr = xzalloc(sizeof *pr); - pr->prefix = prefix; - pr->plen = plen; - pr->route_table_id = get_route_table_id(route_tables, route->route_table); - pr->is_src_route = (route->policy && !strcmp(route->policy, - "src-ip")); - pr->hash = route_hash(pr); - pr->route = route; - pr->ecmp_symmetric_reply = smap_get_bool(&route->options, - "ecmp_symmetric_reply", false); - pr->is_discard_route = is_discard_route; - ovs_list_insert(routes, &pr->list_node); - return pr; + struct parsed_route *new_pr = xzalloc(sizeof *new_pr); + new_pr->prefix = prefix; + new_pr->plen = plen; + new_pr->route_table_id = get_route_table_id(route_tables, + route->route_table); + new_pr->is_src_route = (route->policy && + !strcmp(route->policy, "src-ip")); + new_pr->hash = route_hash(new_pr); + new_pr->route = route; + new_pr->nbr = od->nbr; + new_pr->ecmp_symmetric_reply = smap_get_bool(&route->options, + "ecmp_symmetric_reply", + false); + new_pr->is_discard_route = is_discard_route; + + size_t hash = uuid_hash(&od->key); + struct parsed_route *pr = parsed_route_lookup(routes, hash, new_pr); + if (!pr) { + hmap_insert(routes, &new_pr->key_node, hash); + } else { + pr->stale = false; + free(new_pr); + } } -static void -parsed_routes_destroy(struct ovs_list *routes) +void +build_parsed_routes(struct ovn_datapath *od, const struct hmap *lr_ports, + struct hmap *routes, struct simap *route_tables, + struct hmap *bfd_active_connections, + struct hmap *bfd_connections) { struct parsed_route *pr; - LIST_FOR_EACH_SAFE (pr, list_node, routes) { - ovs_list_remove(&pr->list_node); + HMAP_FOR_EACH (pr, key_node, routes) { + if (pr->nbr == od->nbr) { + pr->stale = true; + } + } + + for (int i = 0; i < od->nbr->n_static_routes; i++) { + parsed_routes_add(od, lr_ports, routes, route_tables, + od->nbr->static_routes[i], + bfd_active_connections, + bfd_connections); + } + + HMAP_FOR_EACH_SAFE (pr, key_node, routes) { + if (!pr->stale) { + continue; + } + + hmap_remove(routes, &pr->key_node); free(pr); } } @@ -12671,8 +12779,8 @@ build_ip_routing_flows_for_lrp( static void build_static_route_flows_for_lrouter( struct ovn_datapath *od, struct lflow_table *lflows, - const struct hmap *lr_ports, const struct hmap *bfd_connections, - struct lflow_ref *lflow_ref) + const struct hmap *lr_ports, struct hmap *parsed_routes, + struct simap *route_tables, struct lflow_ref *lflow_ref) { ovs_assert(od->nbr); ovn_lflow_add_default_drop(lflows, od, S_ROUTER_IN_IP_ROUTING_ECMP, @@ -12685,22 +12793,16 @@ build_static_route_flows_for_lrouter( struct hmap ecmp_groups = HMAP_INITIALIZER(&ecmp_groups); struct hmap unique_routes = HMAP_INITIALIZER(&unique_routes); - struct ovs_list parsed_routes = OVS_LIST_INITIALIZER(&parsed_routes); - struct simap route_tables = SIMAP_INITIALIZER(&route_tables); struct ecmp_groups_node *group; for (int i = 0; i < od->nbr->n_ports; i++) { build_route_table_lflow(od, lflows, od->nbr->ports[i], - &route_tables, lflow_ref); + route_tables, lflow_ref); } - for (int i = 0; i < od->nbr->n_static_routes; i++) { - struct parsed_route *route = - parsed_routes_add(od, lr_ports, &parsed_routes, &route_tables, - od->nbr->static_routes[i], bfd_connections); - if (!route) { - continue; - } + struct parsed_route *route; + HMAP_FOR_EACH_WITH_HASH (route, key_node, uuid_hash(&od->key), + parsed_routes) { group = ecmp_groups_find(&ecmp_groups, route); if (group) { ecmp_groups_add_route(group, route); @@ -12728,8 +12830,6 @@ build_static_route_flows_for_lrouter( } ecmp_groups_destroy(&ecmp_groups); unique_routes_destroy(&unique_routes); - parsed_routes_destroy(&parsed_routes); - simap_destroy(&route_tables); } /* IP Multicast lookup. Here we set the output port, adjust TTL and @@ -12836,6 +12936,115 @@ build_mcast_lookup_flows_for_lrouter( } } +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->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; +} + +void +build_route_policies(struct ovn_datapath *od, struct hmap *lr_ports, + struct hmap *route_policies, + struct hmap *bfd_active_connections, + struct hmap *bfd_connections) +{ + struct route_policy *rp; + + HMAP_FOR_EACH (rp, key_node, route_policies) { + if (rp->nbr == od->nbr) { + rp->stale = true; + } + } + + for (int i = 0; i < od->nbr->n_policies; i++) { + const struct nbrec_logical_router_policy *rule = od->nbr->policies[i]; + size_t n_valid_nexthops = 0; + char **valid_nexthops = NULL; + + if (!strcmp(rule->action, "reroute")) { + size_t n_nexthops = rule->n_nexthops ? rule->n_nexthops : 1; + + valid_nexthops = xcalloc(n_nexthops, sizeof *valid_nexthops); + for (size_t j = 0; j < n_nexthops; j++) { + char *nexthop = rule->n_nexthops + ? rule->nexthops[j] : rule->nexthop; + struct ovn_port *out_port = + get_outport_for_routing_policy_nexthop( + od, lr_ports, rule->priority, nexthop); + if (!out_port || !check_bfd_state(rule, out_port, nexthop, + bfd_active_connections, + bfd_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->nbr = od->nbr; + + 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 { + 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. * * A packet that arrives at this table is an IP packet that should be @@ -12849,7 +13058,7 @@ static void build_ingress_policy_flows_for_lrouter( struct ovn_datapath *od, struct lflow_table *lflows, const struct hmap *lr_ports, - const struct hmap *bfd_connections, + struct hmap *route_policies, struct lflow_ref *lflow_ref) { ovs_assert(od->nbr); @@ -12866,21 +13075,20 @@ build_ingress_policy_flows_for_lrouter( /* Convert routing policies to flows. */ uint16_t ecmp_group_id = 1; - for (int i = 0; i < od->nbr->n_policies; i++) { - const struct nbrec_logical_router_policy *rule - = od->nbr->policies[i]; + struct route_policy *rp; + HMAP_FOR_EACH_WITH_HASH (rp, key_node, uuid_hash(&od->key), + route_policies) { + const struct nbrec_logical_router_policy *rule = rp->rule; bool is_ecmp_reroute = (!strcmp(rule->action, "reroute") && rule->n_nexthops > 1); if (is_ecmp_reroute) { - build_ecmp_routing_policy_flows(lflows, od, lr_ports, rule, - bfd_connections, ecmp_group_id, - lflow_ref); + build_ecmp_routing_policy_flows(lflows, od, lr_ports, rp, + ecmp_group_id, lflow_ref); ecmp_group_id++; } else { - build_routing_policy_flow(lflows, od, lr_ports, rule, - bfd_connections, &rule->header_, - lflow_ref); + build_routing_policy_flow(lflows, od, lr_ports, rp, + &rule->header_, lflow_ref); } } } @@ -15805,6 +16013,9 @@ struct lswitch_flow_build_info { struct ds actions; size_t thread_lflow_counter; const char *svc_monitor_mac; + struct hmap *parsed_routes; + struct hmap *route_policies; + struct simap *route_tables; }; /* Helper function to combine all lflow generation which is iterated by @@ -15850,12 +16061,12 @@ build_lswitch_and_lrouter_iterate_by_lr(struct ovn_datapath *od, build_ND_RA_flows_for_lrouter(od, lsi->lflows, NULL); build_ip_routing_pre_flows_for_lrouter(od, lsi->lflows, NULL); build_static_route_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports, - lsi->bfd_connections, + lsi->parsed_routes, lsi->route_tables, NULL); build_mcast_lookup_flows_for_lrouter(od, lsi->lflows, &lsi->match, &lsi->actions, NULL); build_ingress_policy_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports, - lsi->bfd_connections, NULL); + lsi->route_policies, NULL); build_arp_resolve_flows_for_lrouter(od, lsi->lflows, NULL); build_check_pkt_len_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports, &lsi->match, &lsi->actions, @@ -16169,7 +16380,10 @@ build_lswitch_and_lrouter_flows( const struct hmap *svc_monitor_map, const struct hmap *bfd_connections, const struct chassis_features *features, - const char *svc_monitor_mac) + const char *svc_monitor_mac, + struct hmap *parsed_routes, + struct hmap *route_policies, + struct simap *route_tables) { char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac); @@ -16203,6 +16417,9 @@ build_lswitch_and_lrouter_flows( lsiv[index].svc_check_match = svc_check_match; lsiv[index].thread_lflow_counter = 0; lsiv[index].svc_monitor_mac = svc_monitor_mac; + lsiv[index].parsed_routes = parsed_routes; + lsiv[index].route_tables = route_tables; + lsiv[index].route_policies = route_policies; ds_init(&lsiv[index].match); ds_init(&lsiv[index].actions); @@ -16243,6 +16460,9 @@ build_lswitch_and_lrouter_flows( .features = features, .svc_check_match = svc_check_match, .svc_monitor_mac = svc_monitor_mac, + .parsed_routes = parsed_routes, + .route_tables = route_tables, + .route_policies = route_policies, .match = DS_EMPTY_INITIALIZER, .actions = DS_EMPTY_INITIALIZER, }; @@ -16404,7 +16624,10 @@ void build_lflows(struct ovsdb_idl_txn *ovnsb_txn, input_data->svc_monitor_map, input_data->bfd_connections, input_data->features, - input_data->svc_monitor_mac); + input_data->svc_monitor_mac, + input_data->parsed_routes, + input_data->route_policies, + input_data->route_tables); if (parallelization_state == STATE_INIT_HASH_SIZES) { parallelization_state = STATE_USE_PARALLELIZATION; @@ -17477,6 +17700,27 @@ 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); +} + +void +static_routes_init(struct static_routes_data *data) +{ + hmap_init(&data->parsed_routes); + simap_init(&data->route_tables); + hmap_init(&data->bfd_active_connections); +} + +void +bfd_init(struct bfd_data *data) +{ + hmap_init(&data->bfd_connections); +} + void northd_destroy(struct northd_data *data) { @@ -17516,6 +17760,48 @@ northd_destroy(struct northd_data *data) destroy_northd_tracked_data(data); } +static void +__bfd_destroy(struct hmap *bfd_connections) +{ + struct bfd_entry *bfd_e; + + HMAP_FOR_EACH_POP (bfd_e, hmap_node, bfd_connections) { + bfd_erase_entry(bfd_e); + } + hmap_destroy(bfd_connections); +} + +void +bfd_destroy(struct bfd_data *data) +{ + __bfd_destroy(&data->bfd_connections); +} + +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); +} + +void +static_routes_destroy(struct static_routes_data *data) +{ + struct parsed_route *r; + HMAP_FOR_EACH_POP (r, key_node, &data->parsed_routes) { + free(r); + } + hmap_destroy(&data->parsed_routes); + + simap_destroy(&data->route_tables); + __bfd_destroy(&data->bfd_active_connections); +} + void ovnnb_db_run(struct northd_input *input_data, struct northd_data *data, diff --git a/northd/northd.h b/northd/northd.h index d4a8d75ab..7ddb72eb7 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -23,6 +23,7 @@ #include "northd/en-port-group.h" #include "northd/ipam.h" #include "openvswitch/hmap.h" +#include "simap.h" #include "ovs-thread.h" struct northd_input { @@ -160,14 +161,34 @@ 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; + const struct nbrec_logical_router *nbr; + bool stale; +}; + +struct static_routes_data { + struct hmap parsed_routes; + struct simap route_tables; + struct hmap bfd_active_connections; +}; + +struct route_policies_data { + struct hmap route_policies; + struct hmap bfd_active_connections; +}; + +struct bfd_data { + struct hmap bfd_connections; +}; + struct lr_nat_table; struct lflow_input { - /* Northbound table references */ - const struct nbrec_bfd_table *nbrec_bfd_table; - /* Southbound table references */ - const struct sbrec_bfd_table *sbrec_bfd_table; const struct sbrec_logical_flow_table *sbrec_logical_flow_table; const struct sbrec_multicast_group_table *sbrec_multicast_group_table; const struct sbrec_igmp_group_table *sbrec_igmp_group_table; @@ -190,6 +211,9 @@ struct lflow_input { const struct hmap *svc_monitor_map; bool ovn_internal_version_changed; const char *svc_monitor_mac; + struct hmap *parsed_routes; + struct hmap *route_policies; + struct simap *route_tables; }; extern int parallelization_state; @@ -653,6 +677,20 @@ struct ovn_port { struct lflow_ref *stateful_lflow_ref; }; +struct parsed_route { + struct hmap_node key_node; + struct in6_addr prefix; + unsigned int plen; + bool is_src_route; + uint32_t route_table_id; + uint32_t hash; + const struct nbrec_logical_router_static_route *route; + bool ecmp_symmetric_reply; + bool is_discard_route; + const struct nbrec_logical_router *nbr; + bool stale; +}; + void ovnnb_db_run(struct northd_input *input_data, struct northd_data *data, struct ovsdb_idl_txn *ovnnb_txn, @@ -674,6 +712,18 @@ 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(struct ovn_datapath *, const struct hmap *, + struct hmap *, struct simap *, struct hmap *, + struct hmap *); +uint32_t get_route_table_id(struct simap *, const char *); +void static_routes_init(struct static_routes_data *); +void static_routes_destroy(struct static_routes_data *); + +void bfd_init(struct bfd_data *); +void bfd_destroy(struct bfd_data *); + struct lflow_table; struct lr_stateful_tracked_data; struct ls_stateful_tracked_data; @@ -711,13 +761,14 @@ bool northd_handle_lb_data_changes(struct tracked_lb_data *, struct hmap *lbgrp_datapaths_map, struct northd_tracked_data *); -void build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn, - const struct nbrec_bfd_table *, - const struct sbrec_bfd_table *, - const struct hmap *lr_ports, - struct hmap *bfd_connections); -void bfd_cleanup_connections(const struct nbrec_bfd_table *, - struct hmap *bfd_map); +void build_route_policies(struct ovn_datapath *, struct hmap *, struct hmap *, + struct hmap *, struct hmap *); +void bfd_table_sync(struct ovsdb_idl_txn *, const struct nbrec_bfd_table *, + struct hmap *, struct hmap *, struct hmap *); +void build_bfd_map(const struct nbrec_bfd_table *, + const struct sbrec_bfd_table *, + const struct hmap *lr_ports, + struct hmap *bfd_connections); void run_update_worker_pool(int n_threads); const struct ovn_datapath *northd_get_datapath_for_port( diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 57f89a774..1d7f55e4f 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -12,6 +12,7 @@ m4_define([_DUMP_DB_TABLES], [ ovn-sbctl list meter >> $1 ovn-sbctl list meter_band >> $1 ovn-sbctl list port_group >> $1 + ovn-sbctl list bfd >> $1 ovn-sbctl dump-flows > lflows_$1 ]) @@ -3861,6 +3862,7 @@ for i in $(seq 1 9); do check ovn-nbctl --wait=sb lsp-set-addresses sw$i-r0 00:00:00:00:00:0$i done +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats uuid=$(ovn-nbctl create bfd logical_port=r0-sw1 dst_ip=192.168.1.2 status=down min_tx=250 min_rx=250 detect_mult=10) ovn-nbctl create bfd logical_port=r0-sw2 dst_ip=192.168.2.2 status=down min_tx=500 min_rx=500 detect_mult=20 ovn-nbctl create bfd logical_port=r0-sw3 dst_ip=192.168.3.2 status=down @@ -3873,6 +3875,13 @@ wait_row_count bfd 1 logical_port=r0-sw2 detect_mult=20 dst_ip=192.168.2.2 \ wait_row_count bfd 1 logical_port=r0-sw3 detect_mult=5 dst_ip=192.168.3.2 \ min_rx=1000 min_tx=1000 status=admin_down +check_engine_stats northd norecompute nocompute +check_engine_stats bfd recompute nocompute +check_engine_stats lflow recompute nocompute +check_engine_stats northd_output norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + uuid=$(fetch_column nb:bfd _uuid logical_port=r0-sw1) check ovn-nbctl set bfd $uuid min_tx=1000 min_rx=1000 detect_mult=100 @@ -3881,10 +3890,25 @@ check ovn-nbctl clear bfd $uuid_2 min_rx wait_row_count bfd 1 logical_port=r0-sw2 min_rx=1000 wait_row_count bfd 1 logical_port=r0-sw1 min_rx=1000 min_tx=1000 detect_mult=100 +check_engine_stats northd norecompute nocompute +check_engine_stats bfd recompute nocompute +check_engine_stats lflow recompute nocompute +check_engine_stats northd_output norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + check ovn-nbctl --bfd=$uuid lr-route-add r0 100.0.0.0/8 192.168.1.2 wait_column down bfd status logical_port=r0-sw1 AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.1.2 | grep -q bfd],[0]) +check_engine_stats northd recompute nocompute +check_engine_stats bfd recompute nocompute +check_engine_stats static_routes recompute nocompute +check_engine_stats lflow recompute nocompute +check_engine_stats northd_output norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + check ovn-nbctl --bfd lr-route-add r0 200.0.0.0/8 192.168.2.2 wait_column down bfd status logical_port=r0-sw2 AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.2.2 | grep -q bfd],[0]) @@ -3893,10 +3917,26 @@ check ovn-nbctl --bfd lr-route-add r0 240.0.0.0/8 192.168.5.2 r0-sw5 wait_column down bfd status logical_port=r0-sw5 AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.5.2 | grep -q bfd],[0]) +check_engine_stats northd recompute nocompute +check_engine_stats bfd recompute nocompute +check_engine_stats static_routes recompute nocompute +check_engine_stats lflow recompute nocompute +check_engine_stats northd_output norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + check ovn-nbctl --bfd --policy=src-ip lr-route-add r0 192.168.6.1/32 192.168.10.10 r0-sw6 wait_column down bfd status logical_port=r0-sw6 AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.6.1 | grep -q bfd],[0]) +check_engine_stats northd recompute nocompute +check_engine_stats bfd recompute nocompute +check_engine_stats route_policies recompute nocompute +check_engine_stats lflow recompute nocompute +check_engine_stats northd_output norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + check ovn-nbctl --bfd --policy=src-ip lr-route-add r0 192.168.7.1/32 192.168.10.10 r0-sw7 wait_column down bfd status logical_port=r0-sw7 AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.7.1 | grep -q bfd],[0]) @@ -3924,6 +3964,14 @@ wait_column down bfd status logical_port=r0-sw8 bfd_route_policy_uuid=$(fetch_column nb:bfd _uuid logical_port=r0-sw8) AT_CHECK([ovn-nbctl list logical_router_policy | grep -q $bfd_route_policy_uuid]) +check_engine_stats northd recompute nocompute +check_engine_stats bfd recompute nocompute +check_engine_stats static_routes recompute nocompute +check_engine_stats lflow recompute nocompute +check_engine_stats northd_output norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + check ovn-nbctl lr-policy-del r0 check ovn-nbctl --bfd lr-policy-add r0 100 "ip4.src == 2.3.4.0/24" reroute 192.168.9.2,192.168.9.3,192.168.9.4 @@ -3931,6 +3979,14 @@ wait_column down bfd status dst_ip=192.168.9.2 wait_column down bfd status dst_ip=192.168.9.3 wait_column down bfd status dst_ip=192.168.9.4 +check_engine_stats northd recompute nocompute +check_engine_stats bfd recompute nocompute +check_engine_stats route_policies recompute nocompute +check_engine_stats lflow recompute nocompute +check_engine_stats northd_output norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats + bfd_route_policy_uuid=$(fetch_column nb:bfd _uuid logical_port=r0-sw9) AT_CHECK([ovn-nbctl list logical_router_policy | sed s/,//g | grep -q "$bfd_route_policy_uuid"])
Introduce bfd, static_routes, route_policies and bfd_sync nodes to northd I-P engine to track bfd connections and northd static_route/policy_route changes. Reported-at: https://issues.redhat.com/browse/FDP-600 Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- Changes since v8: - rebase on top of ovn main branch and fix conflicts Changes since v7: - rework node dependencies and move all NB-BFD table update in bfd_sync node - fix some more comments Changes since v6: - remove unnecessary node inputs - fix comment Changes since v5: - remove bfd_mutex Changes since v4: - introduce bfd_nb_sync node and ger rid of bfd_consumer routine Changes since v3: - fix bfd_northd_change_handler logic - fix route_policies_northd_change_handler logic - fix static_routes_northd_change_handler logic - add unit-tests - cosmetics Changes since v2: - add incremental processing routines - split bfd_consumer node in static_routes and route_policies nodes Changes since v1: - add incremental processing logic for bfd_consumer node to avoid a full recompute --- northd/en-lflow.c | 25 +- northd/en-northd.c | 225 +++++++++++++ northd/en-northd.h | 23 ++ northd/inc-proc-northd.c | 27 +- northd/northd.c | 702 +++++++++++++++++++++++++++------------ northd/northd.h | 73 +++- tests/ovn-northd.at | 56 ++++ 7 files changed, 894 insertions(+), 237 deletions(-)