The en-bfd incremental engine node creates a hashmap of bfd_entry structures, which in turn is used by en-routes, en-route-policies, and en-bfd-sync. There are some issues with this approach:
1) bfd_entry structures that are taken as input from an incremental node are treated as immutable (good!). However, each node wants to make some sort of change to the bfd_entries, so they must make clones of the bfd_entries and then update those clones. This means we are doing a lot of looking up, cloning, and modifying of bfd_entries. 2) The en-bfd node does not provide tracking data. Therefore, any node that cares about BFD entries cannot incrementally process BFD changes. In this commit, we remove the en-bfd incremental engine node completely. Now, en-routes and en-route-policies use the direct NB BFD entries' statuses in order to make reachability decisions. en-routes and en-route-policies supply a uuidset of northbound BFD records that they care about. Then finally, en-bfd-sync takes care of syncing the state of the BFD records. Part of the duties of en-bfd, en-routes, and en-route-policies has been moved to en-bfd-sync instead. This paves the way for en-routes and en-route-policies to be able to incrementally handle BFD changes. Later in this patch series, we will add incremental handling of BFD changes to en-route-policies, but en-routes is outside the scope of this series. An item has been added to TODO.rst about addressing this in en-routes in the future. This commit also makes it so that en-routes and en-route-policies no longer recompute based on southbound BFD changes. Only northbound BFD changes will trigger a recompute. Before this change, a southbound BFD change would trigger a recompute on all en-bfd and all its dependent nodes. en-bfd-sync would write status updates to the NB BFD nodes, which would then trigger a second recompute of the nodes with the same data again. Now we only trigger one recompute per BFD state change. Signed-off-by: Mark Michelson <[email protected]> --- TODO.rst | 1 + northd/en-northd.c | 94 ++++++++--------- northd/en-northd.h | 4 - northd/en-route-policies.c | 40 ++----- northd/en-route-policies.h | 3 +- northd/inc-proc-northd.c | 8 +- northd/northd.c | 174 +++++++++++++------------------ northd/northd.h | 26 ++--- tests/ovn-inc-proc-graph-dump.at | 7 +- tests/ovn-northd.at | 21 ++-- 10 files changed, 146 insertions(+), 232 deletions(-) diff --git a/TODO.rst b/TODO.rst index 023eb27f6..16278ec0f 100644 --- a/TODO.rst +++ b/TODO.rst @@ -100,6 +100,7 @@ OVN To-do List * Implement I-P for datapath groups. * Implement I-P for route exchange relevant ports. + * Implement I-P in en-routes based on BFD changes. * ovn-northd parallel logical flow processing diff --git a/northd/en-northd.c b/northd/en-northd.c index db0d4e413..2ae4838d2 100644 --- a/northd/en-northd.c +++ b/northd/en-northd.c @@ -339,6 +339,22 @@ static_route_lookup_parsed(struct routes_data *routes_data, return pr; } +static bool +static_route_bfd_is_updated(const struct nbrec_logical_router_static_route *sr) +{ + if (nbrec_logical_router_static_route_is_updated(sr, + NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_BFD)) { + return true; + } + + if (sr->bfd && + nbrec_bfd_row_get_seqno(sr->bfd, OVSDB_IDL_CHANGE_MODIFY) > 0) { + return true; + } + + return false; +} + enum engine_input_handler_result routes_static_route_change_handler(struct engine_node *node, void *data) @@ -348,7 +364,6 @@ routes_static_route_change_handler(struct engine_node *node, nb_lr_static_route_table = EN_OVSDB_GET(engine_get_input("NB_logical_router_static_route", node)); struct northd_data *northd_data = engine_get_input_data("northd", node); - struct bfd_data *bfd_data = engine_get_input_data("bfd", node); struct parsed_route *pr; routes_data->tracked = true; @@ -366,7 +381,6 @@ routes_static_route_change_handler(struct engine_node *node, if (nbrec_logical_router_static_route_is_new(sr)) { pr = parsed_routes_add_static(od, sr, - &bfd_data->bfd_connections, &routes_data->parsed_routes, &routes_data->route_tables, &routes_data->bfd_active_connections); @@ -379,8 +393,7 @@ routes_static_route_change_handler(struct engine_node *node, } /* A BFD column change requires a full recompute. */ - if (nbrec_logical_router_static_route_is_updated(sr, - NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_BFD)) { + if (static_route_bfd_is_updated(sr)) { return EN_UNHANDLED; } @@ -394,8 +407,7 @@ routes_static_route_change_handler(struct engine_node *node, } hmapx_add(&routes_data->trk_data.trk_deleted_parsed_route, pr); hmap_remove(&routes_data->parsed_routes, &pr->key_node); - pr = parsed_routes_add_static(od, sr, &bfd_data->bfd_connections, - &routes_data->parsed_routes, + pr = parsed_routes_add_static(od, sr, &routes_data->parsed_routes, &routes_data->route_tables, &routes_data->bfd_active_connections); if (!pr) { @@ -442,7 +454,6 @@ enum engine_node_state en_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 routes_data *routes_data = data; routes_destroy(data); @@ -457,8 +468,7 @@ en_routes_run(struct engine_node *node, void *data) route_table_name); } - build_parsed_routes(od, &bfd_data->bfd_connections, - &routes_data->parsed_routes, + build_parsed_routes(od, &routes_data->parsed_routes, &routes_data->route_tables, &routes_data->bfd_active_connections); } @@ -466,28 +476,6 @@ en_routes_run(struct engine_node *node, void *data) return EN_UPDATED; } -static void -destroy_bfd_data(struct bfd_data *data) -{ - bfd_destroy(&data->bfd_connections); -} - -enum engine_node_state -en_bfd_run(struct engine_node *node, void *data) -{ - 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)); - - destroy_bfd_data(data); - bfd_init(data); - build_bfd_map(nbrec_bfd_table, sbrec_bfd_table, - &bfd_data->bfd_connections); - return EN_UPDATED; -} - enum engine_input_handler_result bfd_sync_northd_change_handler(struct engine_node *node, void *data OVS_UNUSED) { @@ -525,21 +513,37 @@ en_bfd_sync_run(struct engine_node *node, void *data) { struct northd_data *northd_data = engine_get_input_data("northd", node); 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 routes_data *routes_data = engine_get_input_data("routes", node); 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)); struct bfd_sync_data *bfd_sync_data = data; + struct uuidset bfd_active_connections = + UUIDSET_INITIALIZER(&bfd_active_connections); + struct uuidset_node *uuid_node; + UUIDSET_FOR_EACH (uuid_node, + &route_policies_data->bfd_active_connections) { + uuidset_insert(&bfd_active_connections, &uuid_node->uuid); + } + UUIDSET_FOR_EACH (uuid_node, &routes_data->bfd_active_connections) { + uuidset_insert(&bfd_active_connections, &uuid_node->uuid); + } + + struct hmap bfd_connections = HMAP_INITIALIZER(&bfd_connections); + build_bfd_map(nbrec_bfd_table, sbrec_bfd_table, &bfd_connections, + &bfd_active_connections); + struct sset new_bfd_ports = SSET_INITIALIZER(&new_bfd_ports); - bfd_table_sync(eng_ctx->ovnsb_idl_txn, nbrec_bfd_table, - &northd_data->lr_ports, &bfd_data->bfd_connections, - &route_policies_data->bfd_active_connections, - &routes_data->bfd_active_connections, - &new_bfd_ports); + bfd_table_sync(eng_ctx->ovnsb_idl_txn, &northd_data->lr_ports, + &bfd_connections, &new_bfd_ports); + + bfd_destroy(&bfd_connections); + uuidset_destroy(&bfd_active_connections); enum engine_node_state new_state = sset_equals(&new_bfd_ports, &bfd_sync_data->bfd_ports) @@ -590,16 +594,6 @@ void 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) @@ -679,12 +673,6 @@ en_routes_clear_tracked_data(void *data) routes_clear_tracked(data); } -void -en_bfd_cleanup(void *data) -{ - destroy_bfd_data(data); -} - void en_bfd_sync_cleanup(void *data) { diff --git a/northd/en-northd.h b/northd/en-northd.h index 6fbf36749..dc55897af 100644 --- a/northd/en-northd.h +++ b/northd/en-northd.h @@ -38,10 +38,6 @@ routes_northd_change_handler(struct engine_node *node, void *data OVS_UNUSED); enum engine_input_handler_result routes_static_route_change_handler(struct engine_node *node, void *data); enum engine_node_state en_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); -enum engine_node_state 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); enum engine_input_handler_result diff --git a/northd/en-route-policies.c b/northd/en-route-policies.c index dc33edb53..86a2818f4 100644 --- a/northd/en-route-policies.c +++ b/northd/en-route-policies.c @@ -124,8 +124,7 @@ find_policy_outport(struct ovn_datapath *od, static bool check_bfd_state(const struct nbrec_logical_router_policy *rule, struct ovn_port *out_port, const char *nexthop, - const struct hmap *bfd_connections, - struct hmap *bfd_active_connections) + struct uuidset *bfd_active_connections) { struct in6_addr nexthop_v6; bool is_nexthop_v6 = ipv6_parse(nexthop, &nexthop_v6); @@ -150,28 +149,11 @@ check_bfd_state(const struct nbrec_logical_router_policy *rule, continue; } - struct bfd_entry *bfd_e = bfd_port_lookup(bfd_connections, - nb_bt->logical_port, - nb_bt->dst_ip); - if (!bfd_e) { - continue; - } - - /* This route policy is linked to an active bfd session. */ - struct bfd_entry *bfd_rp = bfd_port_lookup(bfd_active_connections, - nb_bt->logical_port, - nb_bt->dst_ip); - if (!bfd_rp) { - bfd_rp = bfd_alloc_entry(bfd_active_connections, - nb_bt->logical_port, nb_bt->dst_ip, - bfd_e->status); - } - - if (!strcmp(bfd_e->status, "admin_down")) { - bfd_set_status(bfd_rp, "down"); - } + uuidset_insert(bfd_active_connections, &nb_bt->header_.uuid); - return strcmp(bfd_rp->status, "down"); + const char *nb_status = bfd_get_status(nb_bt->status); + return strcmp(nb_status, "down") && + strcmp(nb_status, "admin_down"); } return true; @@ -179,9 +161,8 @@ check_bfd_state(const struct nbrec_logical_router_policy *rule, static void build_route_policies(struct ovn_datapath *od, - const struct hmap *bfd_connections, struct hmap *route_policies, - struct hmap *bfd_active_connections, + struct uuidset *bfd_active_connections, struct simap *chain_ids) { /* Create chain numeric ids for policies with chain name set */ @@ -305,7 +286,6 @@ build_route_policies(struct ovn_datapath *od, continue; } if (!check_bfd_state(rule, out_port, nexthop, - bfd_connections, bfd_active_connections)) { continue; } @@ -344,7 +324,7 @@ static void route_policies_init(struct route_policies_data *data) { hmap_init(&data->route_policies); - hmap_init(&data->bfd_active_connections); + uuidset_init(&data->bfd_active_connections); } static void @@ -356,14 +336,13 @@ route_policies_destroy(struct route_policies_data *data) free(rp); }; hmap_destroy(&data->route_policies); - bfd_destroy(&data->bfd_active_connections); + uuidset_destroy(&data->bfd_active_connections); } enum engine_node_state en_route_policies_run(struct engine_node *node, void *data) { struct northd_data *northd_data = engine_get_input_data("northd", node); - struct bfd_data *bfd_data = engine_get_input_data("bfd", node); struct route_policies_data *route_policies_data = data; route_policies_destroy(data); @@ -373,8 +352,7 @@ en_route_policies_run(struct engine_node *node, void *data) HMAP_FOR_EACH (od, key_node, &northd_data->lr_datapaths.datapaths) { struct simap chain_ids = SIMAP_INITIALIZER(&chain_ids); - build_route_policies(od, &bfd_data->bfd_connections, - &route_policies_data->route_policies, + build_route_policies(od, &route_policies_data->route_policies, &route_policies_data->bfd_active_connections, &chain_ids); simap_destroy(&chain_ids); diff --git a/northd/en-route-policies.h b/northd/en-route-policies.h index dba0fc2b6..9daaa826f 100644 --- a/northd/en-route-policies.h +++ b/northd/en-route-policies.h @@ -23,6 +23,7 @@ #include "openvswitch/hmap.h" #include "vec.h" +#include "uuidset.h" /* Each instance of this represents a nexthop for a router * policy with "reroute" action. The fields are used for @@ -52,7 +53,7 @@ struct route_policy { /* Global route policy data exported by en-route-policies. */ struct route_policies_data { struct hmap route_policies; - struct hmap bfd_active_connections; + struct uuidset bfd_active_connections; }; void en_route_policies_cleanup(void *data); diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index 84c40fdd2..7e92f7fec 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -182,7 +182,6 @@ static ENGINE_NODE(lr_stateful, CLEAR_TRACKED_DATA); static ENGINE_NODE(ls_stateful, CLEAR_TRACKED_DATA); static ENGINE_NODE(route_policies); static ENGINE_NODE(routes, CLEAR_TRACKED_DATA); -static ENGINE_NODE(bfd); static ENGINE_NODE(bfd_sync, SB_WRITE); static ENGINE_NODE(ecmp_nexthop, SB_WRITE); static ENGINE_NODE(multicast_igmp, SB_WRITE); @@ -330,23 +329,18 @@ 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_route_policies, &en_bfd, NULL); engine_add_input(&en_route_policies, &en_datapath_synced_logical_router, route_policies_datapath_synced_logical_router_handler); engine_add_input(&en_route_policies, &en_northd, route_policies_northd_change_handler); - engine_add_input(&en_routes, &en_bfd, NULL); engine_add_input(&en_routes, &en_northd, routes_northd_change_handler); engine_add_input(&en_routes, &en_nb_logical_router_static_route, routes_static_route_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_sb_bfd, NULL); engine_add_input(&en_bfd_sync, &en_routes, bfd_sync_routes_change_handler); engine_add_input(&en_bfd_sync, &en_route_policies, NULL); engine_add_input(&en_bfd_sync, &en_northd, bfd_sync_northd_change_handler); diff --git a/northd/northd.c b/northd/northd.c index 15921d525..170f3266a 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -11938,6 +11938,20 @@ bfd_is_port_running(const struct sset *bfd_ports, const char *port) return !!sset_find(bfd_ports, port); } +/* Returns the configured BFD status, or "admin_down" if the BFD + * status is NULL or zero-length. This is useful when trying to + * access BFD status from a database when it is not clear if the + * status actually exists. + */ +const char * +bfd_get_status(const char *db_status) +{ + if (!db_status || !db_status[0]) { + return "admin_down"; + } + return db_status; +} + #define BFD_DEF_MINTX 1000 /* 1s */ #define BFD_DEF_MINRX 1000 /* 1s */ #define BFD_DEF_DETECT_MULT 5 @@ -11954,8 +11968,10 @@ build_bfd_update_sb_conf(const struct nbrec_bfd *nb_bt, sbrec_bfd_set_logical_port(sb_bt, nb_bt->logical_port); } - if (strcmp(nb_bt->status, sb_bt->status)) { - sbrec_bfd_set_status(sb_bt, nb_bt->status); + const char *nb_status = bfd_get_status(nb_bt->status); + const char *sb_status = bfd_get_status(sb_bt->status); + if (strcmp(nb_status, sb_status)) { + sbrec_bfd_set_status(sb_bt, nb_status); } int detect_mult = nb_bt->n_detect_mult ? nb_bt->detect_mult[0] @@ -11998,46 +12014,16 @@ static int bfd_get_unused_port(unsigned long *bfd_src_ports) return port + BFD_UDP_SRC_PORT_START; } -static char * -bfd_get_connection_status(const struct nbrec_bfd *nb_bt, - const struct hmap *rp_bfd_connections, - const struct hmap *sr_bfd_connections) -{ - struct bfd_entry *bfd_rp, *bfd_sr; - - bfd_rp = bfd_port_lookup(rp_bfd_connections, nb_bt->logical_port, - nb_bt->dst_ip); - if (!bfd_rp) { - bfd_sr = bfd_port_lookup(sr_bfd_connections, nb_bt->logical_port, - nb_bt->dst_ip); - if (!bfd_sr) { - return "admin_down"; - } - } - - return bfd_rp ? bfd_rp->status : bfd_sr->status; -} - void bfd_table_sync(struct ovsdb_idl_txn *ovnsb_txn, - const struct nbrec_bfd_table *nbrec_bfd_table, const struct hmap *lr_ports, - const struct hmap *bfd_connections, - const struct hmap *rp_bfd_connections, - const struct hmap *sr_bfd_connections, + struct hmap *bfd_connections, struct sset *bfd_ports) { unsigned long *bfd_src_ports = bitmap_allocate(BFD_UDP_SRC_PORT_LEN); - struct hmap sync_bfd_connections = HMAP_INITIALIZER(&sync_bfd_connections); struct bfd_entry *bfd_e; HMAP_FOR_EACH (bfd_e, hmap_node, bfd_connections) { - struct bfd_entry *e = bfd_alloc_entry(&sync_bfd_connections, - bfd_e->logical_port, - bfd_e->dst_ip, bfd_e->status); - e->nb_bt = bfd_e->nb_bt; - e->sb_bt = bfd_e->sb_bt; - 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, @@ -12045,24 +12031,29 @@ bfd_table_sync(struct ovsdb_idl_txn *ovnsb_txn, } } - const struct nbrec_bfd *nb_bt; - NBREC_BFD_TABLE_FOR_EACH (nb_bt, nbrec_bfd_table) { - bfd_e = bfd_port_lookup(&sync_bfd_connections, nb_bt->logical_port, - nb_bt->dst_ip); - if (!bfd_e) { + HMAP_FOR_EACH_SAFE (bfd_e, hmap_node, bfd_connections) { + if (!bfd_e->nb_bt) { + /* Northbound entry was removed or altered. Get rid of the + * old SB entry since we'll be creating a new one based on + * the NB entry's changes. + */ + if (bfd_e->sb_bt) { + sbrec_bfd_delete(bfd_e->sb_bt); + } + hmap_remove(bfd_connections, &bfd_e->hmap_node); + bfd_erase_entry(bfd_e); continue; } - struct ovn_port *op = ovn_port_find(lr_ports, nb_bt->logical_port); + struct ovn_port *op = ovn_port_find(lr_ports, + bfd_e->nb_bt->logical_port); if (!op || !op->sb) { /* skip not bounded ports */ continue; } - nbrec_bfd_set_status(nb_bt, - bfd_get_connection_status(nb_bt, - rp_bfd_connections, - sr_bfd_connections)); + nbrec_bfd_set_status(bfd_e->nb_bt, bfd_e->status); + if (!bfd_e->sb_bt) { int udp_src = bfd_get_unused_port(bfd_src_ports); if (udp_src < 0) { @@ -12071,33 +12062,38 @@ bfd_table_sync(struct ovsdb_idl_txn *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_logical_port(sb_bt, bfd_e->nb_bt->logical_port); + sbrec_bfd_set_dst_ip(sb_bt, bfd_e->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); + sbrec_bfd_set_status(sb_bt, bfd_e->status); if (op->sb->chassis) { sbrec_bfd_set_chassis_name(sb_bt, op->sb->chassis->name); } - int min_tx = nb_bt->n_min_tx ? nb_bt->min_tx[0] : BFD_DEF_MINTX; + int min_tx = bfd_e->nb_bt->n_min_tx + ? bfd_e->nb_bt->min_tx[0] + : BFD_DEF_MINTX; sbrec_bfd_set_min_tx(sb_bt, min_tx); - int min_rx = nb_bt->n_min_rx ? nb_bt->min_rx[0] : BFD_DEF_MINRX; + int min_rx = bfd_e->nb_bt->n_min_rx + ? bfd_e->nb_bt->min_rx[0] + : BFD_DEF_MINRX; sbrec_bfd_set_min_rx(sb_bt, min_rx); - int d_mult = nb_bt->n_detect_mult ? nb_bt->detect_mult[0] - : BFD_DEF_DETECT_MULT; + int d_mult = bfd_e->nb_bt->n_detect_mult + ? bfd_e->nb_bt->detect_mult[0] + : BFD_DEF_DETECT_MULT; sbrec_bfd_set_detect_mult(sb_bt, d_mult); } else { - if (strcmp(bfd_e->sb_bt->status, nb_bt->status)) { - if (!strcmp(nb_bt->status, "admin_down") || + if (strcmp(bfd_e->sb_bt->status, bfd_e->nb_bt->status)) { + if (!strcmp(bfd_e->nb_bt->status, "admin_down") || !strcmp(bfd_e->sb_bt->status, "admin_down")) { - sbrec_bfd_set_status(bfd_e->sb_bt, nb_bt->status); + sbrec_bfd_set_status(bfd_e->sb_bt, bfd_e->nb_bt->status); } else { - nbrec_bfd_set_status(nb_bt, bfd_e->sb_bt->status); + nbrec_bfd_set_status(bfd_e->nb_bt, bfd_e->sb_bt->status); } } - build_bfd_update_sb_conf(nb_bt, bfd_e->sb_bt); + build_bfd_update_sb_conf(bfd_e->nb_bt, bfd_e->sb_bt); if (op->sb->chassis && !strcmp(op->sb->chassis->name, bfd_e->sb_bt->chassis_name)) { sbrec_bfd_set_chassis_name(bfd_e->sb_bt, @@ -12105,25 +12101,17 @@ bfd_table_sync(struct ovsdb_idl_txn *ovnsb_txn, } } - sset_add(bfd_ports, nb_bt->logical_port); - bfd_e->stale = false; + sset_add(bfd_ports, bfd_e->nb_bt->logical_port); } - HMAP_FOR_EACH_POP (bfd_e, hmap_node, &sync_bfd_connections) { - if (bfd_e->stale && bfd_e->sb_bt) { - sbrec_bfd_delete(bfd_e->sb_bt); - } - bfd_erase_entry(bfd_e); - } - hmap_destroy(&sync_bfd_connections); - 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, - struct hmap *bfd_connections) + struct hmap *bfd_connections, + const struct uuidset *bfd_active_connections) { struct bfd_entry *bfd_e; @@ -12144,10 +12132,16 @@ build_bfd_map(const struct nbrec_bfd_table *nbrec_bfd_table, 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"); } + if (uuidset_contains(bfd_active_connections, &nb_bt->header_.uuid)) { + if (!strcmp(bfd_e->status, "admin_down")) { + bfd_set_status(bfd_e, "down"); + } + } else { + bfd_set_status(bfd_e, "admin_down"); + } bfd_e->nb_bt = nb_bt; } } @@ -12660,9 +12654,8 @@ parsed_route_add(const struct ovn_datapath *od, struct parsed_route * parsed_routes_add_static(const struct ovn_datapath *od, const struct nbrec_logical_router_static_route *route, - const struct hmap *bfd_connections, struct hmap *routes, struct simap *route_tables, - struct hmap *bfd_active_connections) + struct uuidset *bfd_active_connections) { /* Verify that the next hop is an IP address with an all-ones mask. */ struct in6_addr *nexthop = NULL; @@ -12716,29 +12709,10 @@ parsed_routes_add_static(const struct ovn_datapath *od, const struct nbrec_bfd *nb_bt = route->bfd; if (nb_bt && !strcmp(nb_bt->dst_ip, route->nexthop)) { - struct bfd_entry *bfd_e = bfd_port_lookup(bfd_connections, - nb_bt->logical_port, - nb_bt->dst_ip); - if (!bfd_e) { - free(nexthop); - return NULL; - } - - /* This static route is linked to an active bfd session. */ - struct bfd_entry *bfd_sr = bfd_port_lookup(bfd_active_connections, - nb_bt->logical_port, - nb_bt->dst_ip); - if (!bfd_sr) { - bfd_sr = bfd_alloc_entry(bfd_active_connections, - nb_bt->logical_port, nb_bt->dst_ip, - bfd_e->status); - } - - if (!strcmp(bfd_e->status, "admin_down")) { - bfd_set_status(bfd_sr, "down"); - } - - if (!strcmp(bfd_sr->status, "down")) { + uuidset_insert(bfd_active_connections, &nb_bt->header_.uuid); + const char *nb_status = bfd_get_status(nb_bt->status); + if (!strcmp(nb_status, "down") || + !strcmp(nb_status, "admin_down")) { free(nexthop); return NULL; } @@ -12825,13 +12799,13 @@ parsed_routes_add_connected(const struct ovn_datapath *od, void build_parsed_routes(const struct ovn_datapath *od, - const struct hmap *bfd_connections, struct hmap *routes, + struct hmap *routes, struct simap *route_tables, - struct hmap *bfd_active_connections) + struct uuidset *bfd_active_connections) { for (size_t i = 0; i < od->nbr->n_static_routes; i++) { parsed_routes_add_static(od, od->nbr->static_routes[i], - bfd_connections, routes, route_tables, + routes, route_tables, bfd_active_connections); } @@ -21482,18 +21456,12 @@ routes_init(struct routes_data *data) { hmap_init(&data->parsed_routes); simap_init(&data->route_tables); - hmap_init(&data->bfd_active_connections); + uuidset_init(&data->bfd_active_connections); hmapx_init(&data->trk_data.trk_deleted_parsed_route); hmapx_init(&data->trk_data.trk_crupdated_parsed_route); data->tracked = false; } -void -bfd_init(struct bfd_data *data) -{ - hmap_init(&data->bfd_connections); -} - void bfd_sync_init(struct bfd_sync_data *data) { @@ -21596,7 +21564,7 @@ routes_destroy(struct routes_data *data) hmap_destroy(&data->parsed_routes); simap_destroy(&data->route_tables); - bfd_destroy(&data->bfd_active_connections); + uuidset_destroy(&data->bfd_active_connections); hmapx_destroy(&data->trk_data.trk_crupdated_parsed_route); hmapx_destroy(&data->trk_data.trk_deleted_parsed_route); } diff --git a/northd/northd.h b/northd/northd.h index 30264e6be..9c7ca1363 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -32,6 +32,7 @@ #include "vec.h" #include "datapath-sync.h" #include "sparse-array.h" +#include "uuidset.h" struct northd_input { /* Northbound table references */ @@ -218,15 +219,11 @@ struct route_tracked_data { struct routes_data { struct hmap parsed_routes; /* Stores struct parsed_route. */ struct simap route_tables; - struct hmap bfd_active_connections; + struct uuidset bfd_active_connections; struct route_tracked_data trk_data; bool tracked; }; -struct bfd_data { - struct hmap bfd_connections; -}; - struct bfd_sync_data { struct sset bfd_ports; }; @@ -894,9 +891,8 @@ struct parsed_route *parsed_route_add( struct parsed_route *parsed_routes_add_static( const struct ovn_datapath *od, const struct nbrec_logical_router_static_route *route, - const struct hmap *bfd_connections, struct hmap *routes, struct simap *route_tables, - struct hmap *bfd_active_connections); + struct uuidset *bfd_active_connections); struct svc_monitors_map_data { const struct hmap *local_svc_monitors_map; @@ -934,8 +930,8 @@ void northd_init(struct northd_data *data); void northd_indices_create(struct northd_data *data, struct ovsdb_idl *ovnsb_idl); -void build_parsed_routes(const struct ovn_datapath *, const struct hmap *, - struct hmap *, struct simap *, struct hmap *); +void build_parsed_routes(const struct ovn_datapath *, struct hmap *, + struct simap *, struct uuidset *); uint32_t get_route_table_id(struct simap *, const char *); void routes_init(struct routes_data *); void routes_destroy(struct routes_data *); @@ -950,7 +946,6 @@ struct bfd_entry { char *logical_port; char *dst_ip; char *status; - bool stale; }; struct bfd_entry *bfd_alloc_entry(struct hmap *bfd_connections, @@ -958,13 +953,12 @@ struct bfd_entry *bfd_alloc_entry(struct hmap *bfd_connections, const char *status); void bfd_erase_entry(struct bfd_entry *bfd_e); void bfd_set_status(struct bfd_entry *bfd_e, const char *status); +const char *bfd_get_status(const char *db_status); struct bfd_entry *bfd_port_lookup(const struct hmap *bfd_map, const char *logical_port, const char *dst_ip); void bfd_destroy(struct hmap *bfd_connections); -void bfd_init(struct bfd_data *); - void bfd_sync_init(struct bfd_sync_data *); void bfd_sync_swap(struct bfd_sync_data *, struct sset *bfd_ports); void bfd_sync_destroy(struct bfd_sync_data *); @@ -1020,12 +1014,12 @@ bool northd_handle_lb_data_changes(struct tracked_lb_data *, const struct hmap *lr_lb_map, struct northd_tracked_data *); -void bfd_table_sync(struct ovsdb_idl_txn *, const struct nbrec_bfd_table *, - const struct hmap *, const struct hmap *, - const struct hmap *, const struct hmap *, +void bfd_table_sync(struct ovsdb_idl_txn *, + const struct hmap *, struct hmap *, struct sset *); void build_bfd_map(const struct nbrec_bfd_table *, - const struct sbrec_bfd_table *, struct hmap *); + const struct sbrec_bfd_table *, struct hmap *, + const struct uuidset *); void build_ic_learned_svc_monitors_map( struct hmap *ic_learned_svc_monitors_map, diff --git a/tests/ovn-inc-proc-graph-dump.at b/tests/ovn-inc-proc-graph-dump.at index e763fa189..5686e0208 100644 --- a/tests/ovn-inc-proc-graph-dump.at +++ b/tests/ovn-inc-proc-graph-dump.at @@ -146,21 +146,16 @@ digraph "Incremental-Processing-Engine" { SB_multicast_group [[style=filled, shape=box, fillcolor=white, label="SB_multicast_group"]]; NB_bfd [[style=filled, shape=box, fillcolor=white, label="NB_bfd"]]; SB_bfd [[style=filled, shape=box, fillcolor=white, label="SB_bfd"]]; - bfd [[style=filled, shape=box, fillcolor=white, label="bfd"]]; - NB_bfd -> bfd [[label=""]]; - SB_bfd -> bfd [[label=""]]; NB_logical_router_static_route [[style=filled, shape=box, fillcolor=white, label="NB_logical_router_static_route"]]; routes [[style=filled, shape=box, fillcolor=white, label="routes"]]; - bfd -> routes [[label=""]]; northd -> routes [[label="routes_northd_change_handler"]]; NB_logical_router_static_route -> routes [[label="routes_static_route_change_handler"]]; route_policies [[style=filled, shape=box, fillcolor=white, label="route_policies"]]; - bfd -> route_policies [[label=""]]; datapath_synced_logical_router -> route_policies [[label="route_policies_datapath_synced_logical_router_handler"]]; northd -> route_policies [[label="route_policies_northd_change_handler"]]; bfd_sync [[style=filled, shape=box, fillcolor=white, label="bfd_sync"]]; - bfd -> bfd_sync [[label=""]]; NB_bfd -> bfd_sync [[label=""]]; + SB_bfd -> bfd_sync [[label=""]]; routes -> bfd_sync [[label="bfd_sync_routes_change_handler"]]; route_policies -> bfd_sync [[label=""]]; northd -> bfd_sync [[label="bfd_sync_northd_change_handler"]]; diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 192c74f1b..81e571d36 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -4605,7 +4605,6 @@ 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 @@ -4620,9 +4619,8 @@ 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_engine_stats lflow norecompute nocompute +check_engine_stats northd_output norecompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats @@ -4631,7 +4629,6 @@ 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], [], [ignore]) check_engine_stats northd norecompute compute -check_engine_stats bfd recompute nocompute check_engine_stats routes recompute nocompute check_engine_stats lflow recompute nocompute check_engine_stats northd_output norecompute compute @@ -4647,7 +4644,6 @@ 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], [], [ignore]) check_engine_stats northd norecompute compute -check_engine_stats bfd recompute nocompute check_engine_stats routes recompute nocompute check_engine_stats lflow recompute nocompute check_engine_stats northd_output norecompute compute @@ -4659,8 +4655,7 @@ 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], [], [ignore]) check_engine_stats northd norecompute compute -check_engine_stats bfd recompute nocompute -check_engine_stats route_policies recompute nocompute +check_engine_stats routes recompute nocompute check_engine_stats lflow recompute nocompute check_engine_stats northd_output norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -4694,8 +4689,10 @@ 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 norecompute incremental -check_engine_stats bfd recompute nocompute -check_engine_stats routes recompute nocompute +# route_policies was able to compute when the static route was +# added earlier, but has to recompute as a result of the policy +# being added. +check_engine_stats route_policies recompute compute check_engine_stats lflow recompute nocompute check_engine_stats northd_output norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE @@ -4709,7 +4706,9 @@ 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 norecompute compute -check_engine_stats bfd recompute nocompute +# In this case, though, the only operation that happened since +# clearing incremental stats is that a policy was added. In this +# case, route_policies has recomputed but has not computed. check_engine_stats route_policies recompute nocompute check_engine_stats lflow recompute nocompute check_engine_stats northd_output norecompute compute -- 2.55.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
