From: Lorenzo Bianconi <[email protected]> Introduce Incremetal processing logic to datapath-sync node for en_datapath_logical_switch and en_datapath_logical_router inputs.
One test change that may seem unrelated is the propage sb_cfg test in ovn-northd.at. That test checked for a specific sequence of events to happen based on northbound database manipulation. The test had used the insertion of a logical switch to test this. Now, inserting a logical switch causes more than one engine run to occur because of the way en-datapath-sync works. The easiest way to fix the test was to insert a different type of row into the database, so this commit changes to using a port group now. Reported-at: https://issues.redhat.com/browse/FDP-1519 Signed-off-by: Lorenzo Bianconi <[email protected]> --- V17: - cherry picked from v16 - cherry picked some of the v16 patch 8/8 to avoid use after free - fix I-P bug in en_datapath_sync_run() --- northd/aging.c | 8 +- northd/datapath-sync.h | 24 +++ northd/en-advertised-route-sync.c | 12 +- northd/en-datapath-logical-router.c | 2 +- northd/en-datapath-logical-router.h | 2 +- northd/en-datapath-logical-switch.c | 2 +- northd/en-datapath-logical-switch.h | 2 +- northd/en-datapath-sync.c | 289 +++++++++++++++++++++++++++- northd/en-datapath-sync.h | 9 + northd/en-multicast.c | 4 +- northd/en-northd.c | 2 - northd/en-port-group.c | 3 +- northd/inc-proc-northd.c | 14 +- northd/lflow-mgr.c | 4 +- northd/northd.c | 50 ++--- northd/northd.h | 3 +- tests/ovn-northd.at | 56 ++++++ 17 files changed, 428 insertions(+), 58 deletions(-) diff --git a/northd/aging.c b/northd/aging.c index aa62a90dc..62e25f6cb 100644 --- a/northd/aging.c +++ b/northd/aging.c @@ -412,7 +412,7 @@ en_mac_binding_aging_run(struct engine_node *node, void *data OVS_UNUSED) HMAP_FOR_EACH (od, key_node, &northd_data->lr_datapaths.datapaths) { ovs_assert(od->nbr); - if (!od->sb) { + if (!od->sdp->sb_dp) { continue; } @@ -425,7 +425,7 @@ en_mac_binding_aging_run(struct engine_node *node, void *data OVS_UNUSED) aging_context_set_threshold(&ctx, &threshold_config); - mac_binding_aging_run_for_datapath(od->sb, + mac_binding_aging_run_for_datapath(od->sdp->sb_dp, sbrec_mac_binding_by_datapath, &ctx); threshold_config_destroy(&threshold_config); @@ -544,7 +544,7 @@ en_fdb_aging_run(struct engine_node *node, void *data OVS_UNUSED) HMAP_FOR_EACH (od, key_node, &northd_data->ls_datapaths.datapaths) { ovs_assert(od->nbs); - if (!od->sb) { + if (!od->sdp->sb_dp) { continue; } @@ -553,7 +553,7 @@ en_fdb_aging_run(struct engine_node *node, void *data OVS_UNUSED) threshold_config.default_threshold = smap_get_uint(&od->nbs->other_config, "fdb_age_threshold", 0); aging_context_set_threshold(&ctx, &threshold_config); - fdb_run_for_datapath(od->sb, sbrec_fdb_by_dp_key, &ctx); + fdb_run_for_datapath(od->sdp->sb_dp, sbrec_fdb_by_dp_key, &ctx); if (aging_context_is_at_limit(&ctx)) { /* Schedule the next run after specified delay. */ diff --git a/northd/datapath-sync.h b/northd/datapath-sync.h index e90ec09a5..594b43b02 100644 --- a/northd/datapath-sync.h +++ b/northd/datapath-sync.h @@ -74,11 +74,35 @@ struct ovn_synced_datapath { struct hmap_node hmap_node; const struct ovsdb_idl_row *nb_row; const struct sbrec_datapath_binding *sb_dp; + /* This boolean indicates if the synced datapath + * has a transient sb_dp pointer. If "true", then + * it means the sb_dp field is the return value of + * sbrec_datapath_binding_insert(). If "false", then + * it means the sb_dp field was provided by an IDL + * update. + * + * This field is only necessary in order to distinguish + * duplicate southbound datapath binding records. If a + * mischievous agent inserts a datapath binding that walks, + * looks, and quacks like an existing datapath binding, + * this is what helps us distinguish that the inserted + * datapath is actually a duplicate of a known datapath. + */ + bool pending_sb_dp; }; struct ovn_synced_datapaths { struct hmap synced_dps; struct hmap dp_tnlids; + + struct hmapx new; + struct hmapx updated; + struct hmapx deleted; + + /* Cache the vxlan mode setting. This is an easy way for us to detect if + * the global configuration resulted in a change to this value. + */ + bool vxlan_mode; }; struct ovn_unsynced_datapath *ovn_unsynced_datapath_alloc( diff --git a/northd/en-advertised-route-sync.c b/northd/en-advertised-route-sync.c index e75ab15c5..dfa9f9c44 100644 --- a/northd/en-advertised-route-sync.c +++ b/northd/en-advertised-route-sync.c @@ -56,7 +56,7 @@ ar_entry_add_nocopy(struct hmap *routes, const struct ovn_datapath *od, route_e->ip_prefix = ip_prefix; route_e->tracked_port = tracked_port; route_e->source = source; - uint32_t hash = uuid_hash(&od->sb->header_.uuid); + uint32_t hash = uuid_hash(&od->sdp->sb_dp->header_.uuid); hash = hash_string(op->sb->logical_port, hash); hash = hash_string(ip_prefix, hash); hmap_insert(routes, &route_e->hmap_node, hash); @@ -92,7 +92,7 @@ ar_entry_find(struct hmap *route_map, HMAP_FOR_EACH_WITH_HASH (route_e, hmap_node, hash, route_map) { if (!uuid_equals(&sb_db->header_.uuid, - &route_e->od->sb->header_.uuid)) { + &route_e->od->sdp->sb_dp->header_.uuid)) { continue; } if (!uuid_equals(&logical_port->header_.uuid, @@ -281,7 +281,8 @@ build_nat_route_for_port(const struct ovn_port *advertising_op, ? ovn_port_find(ls_ports, nat->nb->logical_port) : nat->l3dgw_port; - if (!ar_entry_find(routes, advertising_od->sb, advertising_op->sb, + if (!ar_entry_find(routes, advertising_od->sdp->sb_dp, + advertising_op->sb, nat->nb->external_ip, tracked_port ? tracked_port->sb : NULL)) { ar_entry_add(routes, advertising_od, advertising_op, @@ -767,7 +768,8 @@ advertised_route_table_sync( const struct sbrec_port_binding *tracked_pb = route_e->tracked_port ? route_e->tracked_port->sb : NULL; - if (ar_entry_find(&sync_routes, route_e->od->sb, route_e->op->sb, + if (ar_entry_find(&sync_routes, route_e->od->sdp->sb_dp, + route_e->op->sb, route_e->ip_prefix, tracked_pb)) { /* We could already have advertised route entry for LRP IP that * corresponds to "snat" when "connected-as-host" is combined @@ -802,7 +804,7 @@ advertised_route_table_sync( HMAP_FOR_EACH_POP (route_e, hmap_node, &sync_routes) { const struct sbrec_advertised_route *sr = sbrec_advertised_route_insert(ovnsb_txn); - sbrec_advertised_route_set_datapath(sr, route_e->od->sb); + sbrec_advertised_route_set_datapath(sr, route_e->od->sdp->sb_dp); sbrec_advertised_route_set_logical_port(sr, route_e->op->sb); sbrec_advertised_route_set_ip_prefix(sr, route_e->ip_prefix); if (route_e->tracked_port) { diff --git a/northd/en-datapath-logical-router.c b/northd/en-datapath-logical-router.c index a033e4520..ec974ec53 100644 --- a/northd/en-datapath-logical-router.c +++ b/northd/en-datapath-logical-router.c @@ -252,7 +252,7 @@ en_datapath_synced_logical_router_run(struct engine_node *node , void *data) *lr = (struct ovn_synced_logical_router) { .nb = CONTAINER_OF(sdp->nb_row, struct nbrec_logical_router, header_), - .sb = sdp->sb_dp, + .sdp = sdp, }; hmap_insert(&router_map->synced_routers, &lr->hmap_node, uuid_hash(&lr->nb->header_.uuid)); diff --git a/northd/en-datapath-logical-router.h b/northd/en-datapath-logical-router.h index fa2bf798f..de405dc36 100644 --- a/northd/en-datapath-logical-router.h +++ b/northd/en-datapath-logical-router.h @@ -31,7 +31,7 @@ void en_datapath_logical_router_cleanup(void *data); struct ovn_synced_logical_router { struct hmap_node hmap_node; const struct nbrec_logical_router *nb; - const struct sbrec_datapath_binding *sb; + const struct ovn_synced_datapath *sdp; }; struct ovn_synced_logical_router_map { diff --git a/northd/en-datapath-logical-switch.c b/northd/en-datapath-logical-switch.c index 4141843aa..29a64ed43 100644 --- a/northd/en-datapath-logical-switch.c +++ b/northd/en-datapath-logical-switch.c @@ -250,7 +250,7 @@ en_datapath_synced_logical_switch_run(struct engine_node *node , void *data) *lsw = (struct ovn_synced_logical_switch) { .nb = CONTAINER_OF(sdp->nb_row, struct nbrec_logical_switch, header_), - .sb = sdp->sb_dp, + .sdp = sdp, }; hmap_insert(&switch_map->synced_switches, &lsw->hmap_node, uuid_hash(&lsw->nb->header_.uuid)); diff --git a/northd/en-datapath-logical-switch.h b/northd/en-datapath-logical-switch.h index 2a5a97b18..6abedbd30 100644 --- a/northd/en-datapath-logical-switch.h +++ b/northd/en-datapath-logical-switch.h @@ -33,7 +33,7 @@ void en_datapath_logical_switch_clear_tracked_data(void *data); struct ovn_synced_logical_switch { struct hmap_node hmap_node; const struct nbrec_logical_switch *nb; - const struct sbrec_datapath_binding *sb; + const struct ovn_synced_datapath *sdp; }; struct ovn_synced_logical_switch_map { diff --git a/northd/en-datapath-sync.c b/northd/en-datapath-sync.c index db84448e0..5d0b7c0fc 100644 --- a/northd/en-datapath-sync.c +++ b/northd/en-datapath-sync.c @@ -36,6 +36,9 @@ en_datapath_sync_init(struct engine_node *node OVS_UNUSED, *synced_datapaths = (struct ovn_synced_datapaths) { .synced_dps = HMAP_INITIALIZER(&synced_datapaths->synced_dps), .dp_tnlids = HMAP_INITIALIZER(&synced_datapaths->dp_tnlids), + .new = HMAPX_INITIALIZER(&synced_datapaths->new), + .deleted = HMAPX_INITIALIZER(&synced_datapaths->deleted), + .updated = HMAPX_INITIALIZER(&synced_datapaths->updated), }; return synced_datapaths; @@ -70,6 +73,38 @@ find_unsynced_datapath(const struct ovn_unsynced_datapath_map **maps, return NULL; } +static struct ovn_synced_datapath * +find_synced_datapath_from_udp( + const struct ovn_synced_datapaths *synced_datapaths, + const struct ovn_unsynced_datapath *udp) +{ + struct ovn_synced_datapath *sdp; + uint32_t hash = uuid_hash(&udp->nb_row->uuid); + HMAP_FOR_EACH_WITH_HASH (sdp, hmap_node, hash, + &synced_datapaths->synced_dps) { + if (uuid_equals(&sdp->nb_row->uuid, &udp->nb_row->uuid)) { + return sdp; + } + } + + return NULL; +} + +static struct ovn_synced_datapath * +find_synced_datapath_from_sb(const struct hmap *datapaths, + const struct sbrec_datapath_binding *sb_dp) +{ + struct ovn_synced_datapath *sdp; + uint32_t hash = uuid_hash(&sb_dp->header_.uuid); + HMAP_FOR_EACH_WITH_HASH (sdp, hmap_node, hash, datapaths) { + if (uuid_equals(&sdp->nb_row->uuid, &sb_dp->header_.uuid)) { + return sdp; + } + } + + return NULL; +} + struct candidate_sdp { struct ovn_synced_datapath *sdp; uint32_t requested_tunnel_key; @@ -79,21 +114,40 @@ struct candidate_sdp { static struct ovn_synced_datapath * synced_datapath_alloc(const struct ovn_unsynced_datapath *udp, - const struct sbrec_datapath_binding *sb_dp) + const struct sbrec_datapath_binding *sb_dp, + bool pending_sb_dp) { struct ovn_synced_datapath *sdp; sdp = xmalloc(sizeof *sdp); *sdp = (struct ovn_synced_datapath) { .sb_dp = sb_dp, .nb_row = udp->nb_row, + .pending_sb_dp = pending_sb_dp }; - sbrec_datapath_binding_set_external_ids(sb_dp, &udp->external_ids); + return sdp; +} +static void +synced_datapath_set_sb_fields(const struct sbrec_datapath_binding *sb_dp, + const struct ovn_unsynced_datapath *udp) +{ + sbrec_datapath_binding_set_external_ids(sb_dp, &udp->external_ids); sbrec_datapath_binding_set_nb_uuid(sb_dp, &udp->nb_row->uuid, 1); - sbrec_datapath_binding_set_type(sb_dp, ovn_datapath_type_to_string(udp->type)); - return sdp; +} + +static void +clear_tracked_data(struct ovn_synced_datapaths *synced_datapaths) +{ + hmapx_clear(&synced_datapaths->new); + hmapx_clear(&synced_datapaths->updated); + + struct hmapx_node *node; + HMAPX_FOR_EACH_SAFE (node, &synced_datapaths->deleted) { + free(node->data); + hmapx_delete(&synced_datapaths->deleted, node); + } } static void @@ -104,6 +158,7 @@ reset_synced_datapaths(struct ovn_synced_datapaths *synced_datapaths) free(sdp); } ovn_destroy_tnlids(&synced_datapaths->dp_tnlids); + clear_tracked_data(synced_datapaths); hmap_init(&synced_datapaths->dp_tnlids); } @@ -135,10 +190,11 @@ create_synced_datapath_candidates_from_sb( } struct candidate_sdp candidate = { - .sdp = synced_datapath_alloc(udp, sb_dp), + .sdp = synced_datapath_alloc(udp, sb_dp, false), .requested_tunnel_key = udp->requested_tunnel_key, .existing_tunnel_key = sb_dp->tunnel_key, }; + synced_datapath_set_sb_fields(sb_dp, udp); vector_push(candidate_sdps, &candidate); uuidset_insert(visited, &udp->nb_row->uuid); } @@ -161,10 +217,11 @@ create_synced_datapath_candidates_from_nb( struct sbrec_datapath_binding *sb_dp; sb_dp = sbrec_datapath_binding_insert(ovnsb_idl_txn); struct candidate_sdp candidate = { - .sdp = synced_datapath_alloc(udp, sb_dp), + .sdp = synced_datapath_alloc(udp, sb_dp, true), .requested_tunnel_key = udp->requested_tunnel_key, .existing_tunnel_key = sb_dp->tunnel_key, }; + synced_datapath_set_sb_fields(sb_dp, udp); vector_push(candidate_sdps, &candidate); } } @@ -258,6 +315,216 @@ delete_unassigned_candidates(struct vector *candidate_sdps) } } +static enum engine_input_handler_result +datapath_sync_unsynced_datapath_handler( + const struct ovn_unsynced_datapath_map *map, + const struct ed_type_global_config *global_config, + struct ovsdb_idl_txn *ovnsb_idl_txn, void *data) +{ + enum engine_input_handler_result ret = EN_HANDLED_UNCHANGED; + struct ovn_synced_datapaths *synced_datapaths = data; + struct ovn_unsynced_datapath *udp; + struct ovn_synced_datapath *sdp; + + if (hmapx_is_empty(&map->new) && + hmapx_is_empty(&map->deleted) && + hmapx_is_empty(&map->updated)) { + return EN_UNHANDLED; + } + + struct hmapx_node *n; + HMAPX_FOR_EACH (n, &map->deleted) { + udp = n->data; + sdp = find_synced_datapath_from_udp(synced_datapaths, udp); + if (!sdp) { + return EN_UNHANDLED; + } + hmap_remove(&synced_datapaths->synced_dps, &sdp->hmap_node); + hmapx_add(&synced_datapaths->deleted, sdp); + ovn_free_tnlid(&synced_datapaths->dp_tnlids, + sdp->sb_dp->tunnel_key); + sbrec_datapath_binding_delete(sdp->sb_dp); + ret = EN_HANDLED_UPDATED; + } + + HMAPX_FOR_EACH (n, &map->new) { + udp = n->data; + uint32_t tunnel_key; + + if (find_synced_datapath_from_udp(synced_datapaths, udp)) { + return EN_UNHANDLED; + } + + if (udp->requested_tunnel_key) { + tunnel_key = udp->requested_tunnel_key; + if (!ovn_add_tnlid(&synced_datapaths->dp_tnlids, tunnel_key)) { + return EN_UNHANDLED; + } + } else { + uint32_t hint = 0; + tunnel_key = ovn_allocate_tnlid(&synced_datapaths->dp_tnlids, + "datapath", OVN_MIN_DP_KEY_LOCAL, + global_config->max_dp_tunnel_id, + &hint); + if (!tunnel_key) { + return EN_UNHANDLED; + } + } + + struct sbrec_datapath_binding *sb_dp = + sbrec_datapath_binding_insert_persist_uuid(ovnsb_idl_txn, + &udp->nb_row->uuid); + sbrec_datapath_binding_set_tunnel_key(sb_dp, tunnel_key); + sdp = synced_datapath_alloc(udp, sb_dp, true); + synced_datapath_set_sb_fields(sb_dp, udp); + hmap_insert(&synced_datapaths->synced_dps, &sdp->hmap_node, + uuid_hash(&udp->nb_row->uuid)); + hmapx_add(&synced_datapaths->new, sdp); + ret = EN_HANDLED_UPDATED; + } + + HMAPX_FOR_EACH (n, &map->updated) { + udp = n->data; + sdp = find_synced_datapath_from_udp(synced_datapaths, udp); + if (!sdp || !sdp->sb_dp) { + return EN_UNHANDLED; + } + if (udp->requested_tunnel_key && + udp->requested_tunnel_key != sdp->sb_dp->tunnel_key) { + ovn_free_tnlid(&synced_datapaths->dp_tnlids, + sdp->sb_dp->tunnel_key); + if (!ovn_add_tnlid(&synced_datapaths->dp_tnlids, + udp->requested_tunnel_key)) { + return EN_UNHANDLED; + } + sbrec_datapath_binding_set_tunnel_key(sdp->sb_dp, + udp->requested_tunnel_key); + } + if (!smap_equal(&udp->external_ids, &sdp->sb_dp->external_ids)) { + sbrec_datapath_binding_set_external_ids(sdp->sb_dp, + &udp->external_ids); + } + hmapx_add(&synced_datapaths->updated, sdp); + ret = EN_HANDLED_UPDATED; + } + + return ret; +} + +enum engine_input_handler_result +datapath_sync_logical_switch_handler(struct engine_node *node, void *data) +{ + const struct ovn_unsynced_datapath_map *map = + engine_get_input_data("datapath_logical_switch", node); + const struct engine_context *eng_ctx = engine_get_context(); + const struct ed_type_global_config *global_config = + engine_get_input_data("global_config", node); + + return datapath_sync_unsynced_datapath_handler(map, global_config, + eng_ctx->ovnsb_idl_txn, + data); +} + +enum engine_input_handler_result +datapath_sync_logical_router_handler(struct engine_node *node, void *data) +{ + const struct ovn_unsynced_datapath_map *map = + engine_get_input_data("datapath_logical_router", node); + const struct engine_context *eng_ctx = engine_get_context(); + const struct ed_type_global_config *global_config = + engine_get_input_data("global_config", node); + + return datapath_sync_unsynced_datapath_handler(map, global_config, + eng_ctx->ovnsb_idl_txn, + data); +} + +enum engine_input_handler_result +datapath_sync_global_config_handler(struct engine_node *node, void *data) +{ + const struct ed_type_global_config *global_config = + engine_get_input_data("global_config", node); + struct ovn_synced_datapaths *synced_datapaths = data; + + if (synced_datapaths->vxlan_mode != + global_config->vxlan_mode) { + /* If VXLAN mode changes, then the range of datapath tunnel IDs + * has completely been upended and we need to recompute. + */ + return EN_UNHANDLED; + } + + return EN_HANDLED_UNCHANGED; +} + +enum engine_input_handler_result +datapath_sync_sb_datapath_binding(struct engine_node *node, void *data) +{ + const struct sbrec_datapath_binding_table *sb_dp_table = + EN_OVSDB_GET(engine_get_input("SB_datapath_binding", node)); + enum engine_input_handler_result ret = EN_HANDLED_UNCHANGED; + struct ovn_synced_datapaths *synced_datapaths = data; + + const struct sbrec_datapath_binding *sb_dp; + SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_TRACKED (sb_dp, sb_dp_table) { + struct ovn_synced_datapath *sdp = + find_synced_datapath_from_sb(&synced_datapaths->synced_dps, sb_dp); + if (sbrec_datapath_binding_is_deleted(sb_dp)) { + if (sdp) { + /* The SB datapath binding was deleted, but we still have a + * record of it locally. This implies the SB datapath binding + * was deleted by something other than ovn-northd. We need + * to recompute in this case. + */ + return EN_UNHANDLED; + } + continue; + } + + if (sbrec_datapath_binding_is_new(sb_dp)) { + if (!sdp) { + /* There is a new datapath binding, but we have no record + * of the synced datapath. This indicates that + * something other than ovn-northd added this datapath + * binding to the database, and we need to recompute. + */ + return EN_UNHANDLED; + } else { + if (sdp->pending_sb_dp) { + /* Update the existing synced datapath pointer to the safer + * version to cache. + */ + sdp->sb_dp = sb_dp; + sdp->pending_sb_dp = false; + } else { + /* Some jerk has inserted a duplicate datapath to try to + * fool us. Not on my watch! + */ + return EN_UNHANDLED; + } + } + continue; + } + + /* Datapath binding is updated. This happens if the northbound + * logical datapath is updated and we make changes to the existing + * southbound datapath binding. In this case, the handler for the + * unsynced datapath will add sdp to synced_datapaths->updated, so + * we don't need to do anything here. + */ + } + + return ret; +} + +void +en_datapath_sync_clear_tracked_data(void *data) +{ + struct ovn_synced_datapaths *synced_datapaths = data; + + clear_tracked_data(synced_datapaths); +} + enum engine_node_state en_datapath_sync_run(struct engine_node *node , void *data) { @@ -284,6 +551,8 @@ en_datapath_sync_run(struct engine_node *node , void *data) reset_synced_datapaths(synced_datapaths); + synced_datapaths->vxlan_mode = global_config->vxlan_mode; + struct uuidset visited = UUIDSET_INITIALIZER(&visited); struct vector candidate_sdps = VECTOR_CAPACITY_INITIALIZER(struct candidate_sdp, num_datapaths); @@ -312,6 +581,14 @@ void en_datapath_sync_cleanup(void *data) struct ovn_synced_datapaths *synced_datapaths = data; struct ovn_synced_datapath *sdp; + hmapx_destroy(&synced_datapaths->new); + hmapx_destroy(&synced_datapaths->updated); + struct hmapx_node *node; + HMAPX_FOR_EACH_SAFE (node, &synced_datapaths->deleted) { + free(node->data); + } + hmapx_destroy(&synced_datapaths->deleted); + HMAP_FOR_EACH_POP (sdp, hmap_node, &synced_datapaths->synced_dps) { free(sdp); } diff --git a/northd/en-datapath-sync.h b/northd/en-datapath-sync.h index 3b3262304..0f7a85d1d 100644 --- a/northd/en-datapath-sync.h +++ b/northd/en-datapath-sync.h @@ -21,7 +21,16 @@ void *en_datapath_sync_init(struct engine_node *, struct engine_arg *); +enum engine_input_handler_result +datapath_sync_logical_switch_handler(struct engine_node *, void *data); +enum engine_input_handler_result +datapath_sync_logical_router_handler(struct engine_node *, void *data); +enum engine_input_handler_result +datapath_sync_sb_datapath_binding(struct engine_node *, void *data); +enum engine_input_handler_result +datapath_sync_global_config_handler(struct engine_node *, void *data); enum engine_node_state en_datapath_sync_run(struct engine_node *, void *data); void en_datapath_sync_cleanup(void *data); +void en_datapath_sync_clear_tracked_data(void *data); #endif /* EN_DATAPATH_SYNC_H */ diff --git a/northd/en-multicast.c b/northd/en-multicast.c index d51db557e..6613bdaa9 100644 --- a/northd/en-multicast.c +++ b/northd/en-multicast.c @@ -434,7 +434,7 @@ sync_multicast_groups_to_sb( continue; } - sbmc = create_sb_multicast_group(ovnsb_txn, mc->datapath->sb, + sbmc = create_sb_multicast_group(ovnsb_txn, mc->datapath->sdp->sb_dp, mc->group->name, mc->group->key); ovn_multicast_update_sbrec(mc, sbmc); } @@ -579,7 +579,7 @@ ovn_igmp_group_add(struct ovsdb_idl_index *sbrec_mcast_group_by_name_dp, const struct sbrec_multicast_group *mcgroup = mcast_group_lookup(sbrec_mcast_group_by_name_dp, address_s, - datapath->sb); + datapath->sdp->sb_dp); igmp_group->datapath = datapath; igmp_group->address = *address; diff --git a/northd/en-northd.c b/northd/en-northd.c index b991d3a1c..b2986edf1 100644 --- a/northd/en-northd.c +++ b/northd/en-northd.c @@ -81,8 +81,6 @@ northd_get_input_data(struct engine_node *node, input_data->nbrec_port_group_table = EN_OVSDB_GET(engine_get_input("NB_port_group", node)); - input_data->sbrec_datapath_binding_table = - EN_OVSDB_GET(engine_get_input("SB_datapath_binding", node)); input_data->sbrec_port_binding_table = EN_OVSDB_GET(engine_get_input("SB_port_binding", node)); input_data->sbrec_mac_binding_table = diff --git a/northd/en-port-group.c b/northd/en-port-group.c index 504b5befd..476c0a18d 100644 --- a/northd/en-port-group.c +++ b/northd/en-port-group.c @@ -270,7 +270,8 @@ ls_port_group_process(struct ls_port_group_table *ls_port_groups, struct ls_port_group *ls_pg = ls_port_group_table_find(ls_port_groups, od->nbs); if (!ls_pg) { - ls_pg = ls_port_group_create(ls_port_groups, od->nbs, od->sb); + ls_pg = ls_port_group_create(ls_port_groups, od->nbs, + od->sdp->sb_dp); } struct ls_port_group_record *ls_pg_rec = diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index 2c31a245e..c43c35118 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -184,7 +184,7 @@ static ENGINE_NODE(dynamic_routes); static ENGINE_NODE(group_ecmp_route, CLEAR_TRACKED_DATA); static ENGINE_NODE(datapath_logical_router, CLEAR_TRACKED_DATA); static ENGINE_NODE(datapath_logical_switch, CLEAR_TRACKED_DATA); -static ENGINE_NODE(datapath_sync); +static ENGINE_NODE(datapath_sync, CLEAR_TRACKED_DATA); static ENGINE_NODE(datapath_synced_logical_router); static ENGINE_NODE(datapath_synced_logical_switch); @@ -225,10 +225,14 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, engine_add_input(&en_datapath_logical_router, &en_nb_logical_router, en_datapath_logical_router_logical_router_handler); - engine_add_input(&en_datapath_sync, &en_datapath_logical_switch, NULL); - engine_add_input(&en_datapath_sync, &en_datapath_logical_router, NULL); - engine_add_input(&en_datapath_sync, &en_sb_datapath_binding, NULL); - engine_add_input(&en_datapath_sync, &en_global_config, NULL); + engine_add_input(&en_datapath_sync, &en_global_config, + datapath_sync_global_config_handler); + engine_add_input(&en_datapath_sync, &en_sb_datapath_binding, + datapath_sync_sb_datapath_binding); + engine_add_input(&en_datapath_sync, &en_datapath_logical_switch, + datapath_sync_logical_switch_handler); + engine_add_input(&en_datapath_sync, &en_datapath_logical_router, + datapath_sync_logical_router_handler); engine_add_input(&en_datapath_synced_logical_router, &en_datapath_sync, NULL); diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c index 18b88cf9e..31476286a 100644 --- a/northd/lflow-mgr.c +++ b/northd/lflow-mgr.c @@ -1118,7 +1118,7 @@ sync_lflow_to_sb(struct ovn_lflow *lflow, } if (lflow->od) { - sbrec_logical_flow_set_logical_datapath(sbflow, lflow->od->sb); + sbrec_logical_flow_set_logical_datapath(sbflow, lflow->od->sdp->sb_dp); sbrec_logical_flow_set_logical_dp_group(sbflow, NULL); } else { sbrec_logical_flow_set_logical_datapath(sbflow, NULL); @@ -1226,7 +1226,7 @@ ovn_sb_insert_or_update_logical_dp_group( sb = xmalloc(bitmap_count1(dpg_bitmap, ods_size(datapaths)) * sizeof *sb); BITMAP_FOR_EACH_1 (index, ods_size(datapaths), dpg_bitmap) { - sb[n++] = datapaths->array[index]->sb; + sb[n++] = datapaths->array[index]->sdp->sb_dp; } if (!dp_group) { struct uuid dpg_uuid = uuid_random(); diff --git a/northd/northd.c b/northd/northd.c index 4d5ed6701..2b6b80750 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -518,11 +518,11 @@ static struct ovn_datapath * ovn_datapath_create(struct hmap *datapaths, const struct uuid *key, const struct nbrec_logical_switch *nbs, const struct nbrec_logical_router *nbr, - const struct sbrec_datapath_binding *sb) + const struct ovn_synced_datapath *sdp) { struct ovn_datapath *od = xzalloc(sizeof *od); od->key = *key; - od->sb = sb; + od->sdp = sdp; od->nbs = nbs; od->nbr = nbr; hmap_init(&od->port_tnlids); @@ -537,7 +537,7 @@ ovn_datapath_create(struct hmap *datapaths, const struct uuid *key, od->localnet_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *); od->lb_with_stateless_mode = false; od->ipam_info_initialized = false; - od->tunnel_key = sb->tunnel_key; + od->tunnel_key = sdp->sb_dp->tunnel_key; init_mcast_info_for_datapath(od); return od; } @@ -628,7 +628,7 @@ ovn_datapath_from_sbrec(const struct hmap *ls_datapaths, } struct ovn_datapath *od = ovn_datapath_find_(dps, &key); - if (od && (od->sb == sb)) { + if (od && (od->sdp->sb_dp == sb)) { return od; } @@ -747,7 +747,7 @@ store_mcast_info_for_switch_datapath(const struct sbrec_ip_multicast *sb, { struct mcast_switch_info *mcast_sw_info = &od->mcast_info.sw; - sbrec_ip_multicast_set_datapath(sb, od->sb); + sbrec_ip_multicast_set_datapath(sb, od->sdp->sb_dp); sbrec_ip_multicast_set_enabled(sb, &mcast_sw_info->enabled, 1); sbrec_ip_multicast_set_querier(sb, &mcast_sw_info->querier, 1); sbrec_ip_multicast_set_table_size(sb, &mcast_sw_info->table_size, 1); @@ -856,7 +856,7 @@ build_datapaths(const struct ovn_synced_logical_switch_map *ls_map, struct ovn_datapath *od = ovn_datapath_create(&ls_datapaths->datapaths, &ls->nb->header_.uuid, - ls->nb, NULL, ls->sb); + ls->nb, NULL, ls->sdp); init_ipam_info_for_datapath(od); if (smap_get_bool(&od->nbs->other_config, "enable-stateless-acl-with-lb", @@ -870,7 +870,7 @@ build_datapaths(const struct ovn_synced_logical_switch_map *ls_map, struct ovn_datapath *od = ovn_datapath_create(&lr_datapaths->datapaths, &lr->nb->header_.uuid, - NULL, lr->nb, lr->sb); + NULL, lr->nb, lr->sdp); if (smap_get(&od->nbr->options, "chassis")) { od->is_gw_router = true; } @@ -1441,7 +1441,7 @@ create_mirror_port(struct ovn_port *op, struct hmap *ports, struct ovs_list *both_dbs, struct ovs_list *nb_only, const struct nbrec_mirror *nb_mirror) { - char *mp_name = ovn_mirror_port_name(ovn_datapath_name(op->od->sb), + char *mp_name = ovn_mirror_port_name(ovn_datapath_name(op->od->sdp->sb_dp), nb_mirror->sink); struct ovn_port *mp = ovn_port_find(ports, mp_name); struct ovn_port *target_port = ovn_port_find(ports, nb_mirror->sink); @@ -1485,7 +1485,7 @@ join_logical_ports_lsp(struct hmap *ports, = VLOG_RATE_LIMIT_INIT(5, 1); VLOG_WARN_RL(&rl, "duplicate logical port %s", name); return NULL; - } else if (op && (!op->sb || op->sb->datapath == od->sb)) { + } else if (op && (!op->sb || op->sb->datapath == od->sdp->sb_dp)) { /* * Handle cases where lport type was explicitly changed * in the NBDB, in such cases: @@ -1582,7 +1582,7 @@ join_logical_ports_lrp(struct hmap *ports, name); destroy_lport_addresses(lrp_networks); return NULL; - } else if (op && (!op->sb || op->sb->datapath == od->sb)) { + } else if (op && (!op->sb || op->sb->datapath == od->sdp->sb_dp)) { ovn_port_set_nb(op, NULL, nbrp); ovs_list_remove(&op->list); ovs_list_push_back(both, &op->list); @@ -1655,7 +1655,7 @@ create_cr_port(struct ovn_port *op, struct hmap *ports, op->nbsp ? op->nbsp->name : op->nbrp->name); struct ovn_port *crp = ovn_port_find(ports, redirect_name); - if (crp && crp->sb && crp->sb->datapath == op->od->sb) { + if (crp && crp->sb && crp->sb->datapath == op->od->sdp->sb_dp) { ovn_port_set_nb(crp, op->nbsp, op->nbrp); ovs_list_remove(&crp->list); ovs_list_push_back(both_dbs, &crp->list); @@ -2513,7 +2513,7 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn *ovnsb_txn, unsigned long *queue_id_bitmap, struct sset *active_ha_chassis_grps) { - sbrec_port_binding_set_datapath(op->sb, op->od->sb); + sbrec_port_binding_set_datapath(op->sb, op->od->sdp->sb_dp); if (op->nbrp) { /* Note: SB port binding options for router ports are set in * sync_pbs(). */ @@ -3829,7 +3829,7 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn, /* When reusing stale Port_Bindings, make sure that stale * Mac_Bindings are purged. */ - if (op->od->sb != op->sb->datapath) { + if (op->od->sdp->sb_dp != op->sb->datapath) { remove_mac_bindings = true; } if (op->nbsp) { @@ -5340,7 +5340,7 @@ build_mirror_lflows(struct ovn_port *op, } char *serving_port_name = ovn_mirror_port_name( - ovn_datapath_name(op->od->sb), + ovn_datapath_name(op->od->sdp->sb_dp), mirror->sink); struct ovn_port *serving_port = ovn_port_find(ls_ports, @@ -18037,13 +18037,13 @@ lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn, const struct sbrec_multicast_group *sbmc_flood = mcast_group_lookup(lflow_input->sbrec_mcast_group_by_name_dp, - MC_FLOOD, op->od->sb); + MC_FLOOD, op->od->sdp->sb_dp); const struct sbrec_multicast_group *sbmc_flood_l2 = mcast_group_lookup(lflow_input->sbrec_mcast_group_by_name_dp, - MC_FLOOD_L2, op->od->sb); + MC_FLOOD_L2, op->od->sdp->sb_dp); const struct sbrec_multicast_group *sbmc_unknown = mcast_group_lookup(lflow_input->sbrec_mcast_group_by_name_dp, - MC_UNKNOWN, op->od->sb); + MC_UNKNOWN, op->od->sdp->sb_dp); struct ds match = DS_EMPTY_INITIALIZER; struct ds actions = DS_EMPTY_INITIALIZER; @@ -18083,13 +18083,13 @@ lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn, /* Update SB multicast groups for the new port. */ if (!sbmc_flood) { sbmc_flood = create_sb_multicast_group(ovnsb_txn, - op->od->sb, MC_FLOOD, OVN_MCAST_FLOOD_TUNNEL_KEY); + op->od->sdp->sb_dp, MC_FLOOD, OVN_MCAST_FLOOD_TUNNEL_KEY); } sbrec_multicast_group_update_ports_addvalue(sbmc_flood, op->sb); if (!sbmc_flood_l2) { sbmc_flood_l2 = create_sb_multicast_group(ovnsb_txn, - op->od->sb, MC_FLOOD_L2, + op->od->sdp->sb_dp, MC_FLOOD_L2, OVN_MCAST_FLOOD_L2_TUNNEL_KEY); } sbrec_multicast_group_update_ports_addvalue(sbmc_flood_l2, op->sb); @@ -18097,7 +18097,7 @@ lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn, if (op->has_unknown) { if (!sbmc_unknown) { sbmc_unknown = create_sb_multicast_group(ovnsb_txn, - op->od->sb, MC_UNKNOWN, + op->od->sdp->sb_dp, MC_UNKNOWN, OVN_MCAST_UNKNOWN_TUNNEL_KEY); } sbrec_multicast_group_update_ports_addvalue(sbmc_unknown, @@ -18418,7 +18418,7 @@ sync_dns_entries(struct ovsdb_idl_txn *ovnsb_txn, dns_info->n_sbs++; dns_info->sbs = xrealloc(dns_info->sbs, dns_info->n_sbs * sizeof *dns_info->sbs); - dns_info->sbs[dns_info->n_sbs - 1] = od->sb; + dns_info->sbs[dns_info->n_sbs - 1] = od->sdp->sb_dp; } } @@ -18535,7 +18535,7 @@ build_ip_mcast(struct ovsdb_idl_txn *ovnsb_txn, ovs_assert(od->nbs); const struct sbrec_ip_multicast *ip_mcast = - ip_mcast_lookup(sbrec_ip_mcast_by_dp, od->sb); + ip_mcast_lookup(sbrec_ip_mcast_by_dp, od->sdp->sb_dp); if (!ip_mcast) { ip_mcast = sbrec_ip_multicast_insert(ovnsb_txn); @@ -18576,7 +18576,7 @@ build_static_mac_binding_table( } struct ovn_port *op = ovn_port_find(lr_ports, nb_smb->logical_port); - if (!op || !op->nbrp || !op->od || !op->od->sb) { + if (!op || !op->nbrp || !op->od || !op->od->sdp->sb_dp) { sbrec_static_mac_binding_delete(sb_smb); } } @@ -18588,7 +18588,7 @@ build_static_mac_binding_table( struct ovn_port *op = ovn_port_find(lr_ports, nb_smb->logical_port); if (op && op->nbrp) { struct ovn_datapath *od = op->od; - if (od && od->sb) { + if (od && od->sdp->sb_dp) { const struct uuid *nb_uuid = &nb_smb->header_.uuid; const struct sbrec_static_mac_binding *mb = sbrec_static_mac_binding_table_get_for_uuid( @@ -18603,7 +18603,7 @@ build_static_mac_binding_table( sbrec_static_mac_binding_set_mac(mb, nb_smb->mac); sbrec_static_mac_binding_set_override_dynamic_mac(mb, nb_smb->override_dynamic_mac); - sbrec_static_mac_binding_set_datapath(mb, od->sb); + sbrec_static_mac_binding_set_datapath(mb, od->sdp->sb_dp); } else { /* Update existing entry if there is a change*/ if (strcmp(mb->mac, nb_smb->mac)) { diff --git a/northd/northd.h b/northd/northd.h index 2f1b8ceb5..5c809f95a 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -42,7 +42,6 @@ struct northd_input { const struct nbrec_port_group_table *nbrec_port_group_table; /* Southbound table references */ - const struct sbrec_datapath_binding_table *sbrec_datapath_binding_table; const struct sbrec_port_binding_table *sbrec_port_binding_table; const struct sbrec_mac_binding_table *sbrec_mac_binding_table; const struct sbrec_ha_chassis_group_table *sbrec_ha_chassis_group_table; @@ -369,7 +368,7 @@ struct ovn_datapath { const struct nbrec_logical_switch *nbs; /* May be NULL. */ const struct nbrec_logical_router *nbr; /* May be NULL. */ - const struct sbrec_datapath_binding *sb; /* May be NULL. */ + const struct ovn_synced_datapath *sdp; /* May be NULL. */ struct ovs_list list; /* In list of similar records. */ diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index a1a9ad77e..f8f4839c0 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -17714,3 +17714,59 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE AT_CLEANUP ]) +OVN_FOR_EACH_NORTHD_NO_HV([ +AT_SETUP([Datapath Sync incremental processing]) +ovn_start + +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb ls-add sw0 +check_engine_stats datapath_sync norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb set logical_switch sw0 other_config:fdb_age_threshold=5 +check_engine_stats datapath_sync norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb set logical_switch sw0 other_config:requested-tnl-key=123 +check_engine_stats datapath_sync norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb ls-del sw0 +check_engine_stats datapath_sync norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lr-add lr0 +check_engine_stats datapath_sync norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl set Logical_Router lr0 options:ct-zone-limit=10 +check_engine_stats datapath_sync norecompute compute + +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb lr-del lr0 +check_engine_stats datapath_sync norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +# Global changes mostly should not trigger recomputes... +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check ovn-nbctl --wait=sb set NB_Global . options:foo=bar +check_engine_stats datapath_sync norecompute compute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +#...unless we're changing vxlan mode. +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats +check_uuid ovn-sbctl \ + --id=@e create encap chassis_name=hv1 ip="192.168.0.1" type="vxlan" \ + -- --id=@c create chassis name=hv1 encaps=@e +# Sync just to be sure the encap change has propagated to northd +check ovn-nbctl --wait=sb sync +check_engine_stats datapath_sync recompute nocompute +CHECK_NO_CHANGE_AFTER_RECOMPUTE + +AT_CLEANUP +]) -- 2.49.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
