On Fri, Jul 18, 2025 at 6:16 PM Mark Michelson via dev < ovs-dev@openvswitch.org> wrote:
> From: Lorenzo Bianconi <lorenzo.bianc...@redhat.com> > > Introduce Incremetal processing logic to datapath-sync node for > en_datapath_logical_switch and en_datapath_logical_router inputs. > > When processing incrementally, the behavior of en-datapath-sync is > different than when run non-incrementally. When new northbound logical > datapath types are added, en-datapath-sync will not propagate the new > logical datapath downstream to other nodes. Instead, en-datapath-sync > waits for the southbound database to report the newly inserted > Datapath_Binding, and then en-datapath-sync reports the new datapath. > See the large comment in northd/datapath-sync.h for more details. > > 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 <lorenzo.bianc...@redhat.com> > --- > Hi Mark and Lorenzo, thank you for the patch. I a couple of comments down below. > northd/datapath-sync.h | 37 +++++ > northd/en-datapath-sync.c | 324 ++++++++++++++++++++++++++++++++++++-- > northd/en-datapath-sync.h | 7 + > northd/en-northd.c | 2 - > northd/inc-proc-northd.c | 12 +- > northd/northd.h | 1 - > tests/ovn-northd.at | 44 +++++- > 7 files changed, 402 insertions(+), 25 deletions(-) > > diff --git a/northd/datapath-sync.h b/northd/datapath-sync.h > index 58f5f1382..879904903 100644 > --- a/northd/datapath-sync.h > +++ b/northd/datapath-sync.h > @@ -36,6 +36,39 @@ > * version (e.g. ovn_synced_logical_router). Later nodes can then consume > * these type-specific synced datapath types in order to perform > * further processing. > + * > + * Incremental processing throws an interesting wrinkle to the mix. > + * When northbound logical datapaths are new, the datapath_sync node > + * inserts a new Datapath_Binding into the southbound database. The > + * new synced datapath is added to a "pending" hmap of synced datapaths. > + * Once the southbound database sends its update showing the inserted > + * Datapath_Binding, then the synced datapath is moved from the "pending" > + * hmap to the "synced_dps" hmap. The engine nodes that process synced > + * datapaths only pay attention to the synced datapaths in the > "synced_dps" > + * hmap. This means that consumers of synced logical switches or synced > + * logical routers will be informed of "new" synced datapaths after the > + * northbound IDL has reported the logical router or logical switch as > + * new. This means that the nbrec_logical_switch_is_new() and > + * nbrec_logical_router_is_new() functions cannot be used to detect if > + * a logical switch or logical router is new. Instead, consumers of > + * synced datapath types can tell if the datapath is new by its > + * presence in the "new" hmapx of the synced logical datapath map. > + * > + * The reason this is done is to ensure that valid southbound > + * datapath binding pointers are provided to all engine nodes. If we > + * provided the southbound datapath binding at the time of insertion, > + * then the pointer would go out of scope when the IDL is run next. By > + * waiting for an updated from the southbound database, we get a more > + * permanent pointer to the southbound datapath binding that is safe > + * to cache. > + * > + * For updated or deleted northbound logical datapaths, there is no > + * such delay. The update or deletion is passed to downstream engine > + * nodes in the same IDL run when they are detected as being updated > + * or deleted in the northbound database. Therefore, the > + * nbrec_logical_switch_is_deleted() and nbrec_logical_router_is_deleted() > + * functions are still valid to call from consumers of synced datapath > + * engine nodes. > */ > > enum ovn_datapath_type { > @@ -74,10 +107,14 @@ struct ovn_synced_datapath { > struct hmap_node hmap_node; > const struct ovsdb_idl_row *nb_row; > const struct sbrec_datapath_binding *sb_dp; > + /* SB datapath_binding must be refreshed with stable > + * pointer to DB row. */ > + bool update_sb_dp; > }; > > struct ovn_synced_datapaths { > struct hmap synced_dps; > + struct hmap pending_dps; > struct hmap dp_tnlids; > > struct hmapx new; > diff --git a/northd/en-datapath-sync.c b/northd/en-datapath-sync.c > index db84448e0..fc4f0a37b 100644 > --- a/northd/en-datapath-sync.c > +++ b/northd/en-datapath-sync.c > @@ -35,7 +35,11 @@ en_datapath_sync_init(struct engine_node *node > OVS_UNUSED, > = xmalloc(sizeof *synced_datapaths); > *synced_datapaths = (struct ovn_synced_datapaths) { > .synced_dps = HMAP_INITIALIZER(&synced_datapaths->synced_dps), > + .pending_dps = HMAP_INITIALIZER(&synced_datapaths->pending_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,8 +74,41 @@ 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->nb_uuid); > + HMAP_FOR_EACH_WITH_HASH (sdp, hmap_node, hash, datapaths) { > + if (uuid_equals(&sdp->nb_row->uuid, sb_dp->nb_uuid)) { > + return sdp; > + } > + } > + > + return NULL; > +} > + > struct candidate_sdp { > struct ovn_synced_datapath *sdp; > + const struct sbrec_datapath_binding *sb_dp; > uint32_t requested_tunnel_key; > uint32_t existing_tunnel_key; > bool tunnel_key_assigned; > @@ -79,21 +116,42 @@ 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 update_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, > + .update_sb_dp = update_sb_dp, > }; > + 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 > @@ -103,7 +161,11 @@ reset_synced_datapaths(struct ovn_synced_datapaths > *synced_datapaths) > HMAP_FOR_EACH_POP (sdp, hmap_node, &synced_datapaths->synced_dps) { > free(sdp); > } > + HMAP_FOR_EACH_POP (sdp, hmap_node, &synced_datapaths->pending_dps) { > + free(sdp); > + } > ovn_destroy_tnlids(&synced_datapaths->dp_tnlids); > + clear_tracked_data(synced_datapaths); > hmap_init(&synced_datapaths->dp_tnlids); > } > > @@ -135,10 +197,12 @@ 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), > + .sb_dp = sb_dp, > .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 +225,12 @@ 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, NULL, true), > + .sb_dp = sb_dp, > .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); > } > } > @@ -188,10 +254,17 @@ assign_requested_tunnel_keys(struct vector > *candidate_sdps, > candidate->requested_tunnel_key); > continue; > } > - sbrec_datapath_binding_set_tunnel_key(candidate->sdp->sb_dp, > + sbrec_datapath_binding_set_tunnel_key(candidate->sb_dp, > > candidate->requested_tunnel_key); > - hmap_insert(&synced_datapaths->synced_dps, > &candidate->sdp->hmap_node, > - uuid_hash(candidate->sdp->sb_dp->nb_uuid)); > + if (candidate->sdp->sb_dp) { > + hmap_insert(&synced_datapaths->synced_dps, > + &candidate->sdp->hmap_node, > + uuid_hash(candidate->sb_dp->nb_uuid)); > + } else { > + hmap_insert(&synced_datapaths->pending_dps, > + &candidate->sdp->hmap_node, > + uuid_hash(candidate->sb_dp->nb_uuid)); > + } > candidate->tunnel_key_assigned = true; > } > } > @@ -211,9 +284,15 @@ assign_existing_tunnel_keys(struct vector > *candidate_sdps, > */ > if (ovn_add_tnlid(&synced_datapaths->dp_tnlids, > candidate->existing_tunnel_key)) { > - hmap_insert(&synced_datapaths->synced_dps, > - &candidate->sdp->hmap_node, > - uuid_hash(candidate->sdp->sb_dp->nb_uuid)); > + if (candidate->sdp->sb_dp) { > + hmap_insert(&synced_datapaths->synced_dps, > + &candidate->sdp->hmap_node, > + uuid_hash(candidate->sb_dp->nb_uuid)); > + } else { > + hmap_insert(&synced_datapaths->pending_dps, > + &candidate->sdp->hmap_node, > + uuid_hash(candidate->sb_dp->nb_uuid)); > + } > candidate->tunnel_key_assigned = true; > } > } > @@ -237,10 +316,17 @@ allocate_tunnel_keys(struct vector *candidate_sdps, > if (!tunnel_key) { > continue; > } > - sbrec_datapath_binding_set_tunnel_key(candidate->sdp->sb_dp, > + sbrec_datapath_binding_set_tunnel_key(candidate->sb_dp, > tunnel_key); > - hmap_insert(&synced_datapaths->synced_dps, > &candidate->sdp->hmap_node, > - uuid_hash(candidate->sdp->sb_dp->nb_uuid)); > + if (candidate->sdp->sb_dp) { > + hmap_insert(&synced_datapaths->synced_dps, > + &candidate->sdp->hmap_node, > + uuid_hash(candidate->sb_dp->nb_uuid)); > + } else { > + hmap_insert(&synced_datapaths->pending_dps, > + &candidate->sdp->hmap_node, > + uuid_hash(candidate->sb_dp->nb_uuid)); > + } > candidate->tunnel_key_assigned = true; > } > } > @@ -253,11 +339,203 @@ delete_unassigned_candidates(struct vector > *candidate_sdps) > if (candidate->tunnel_key_assigned) { > continue; > } > - sbrec_datapath_binding_delete(candidate->sdp->sb_dp); > + sbrec_datapath_binding_delete(candidate->sb_dp); > free(candidate->sdp); > } > } > > +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_count(&map->new) + hmapx_count(&map->deleted) + > + hmapx_count(&map->updated))) { > It would be better to use 'hmapx_is_empty(..)' here. + 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 || sdp->update_sb_dp) { > + 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(ovnsb_idl_txn); > + sbrec_datapath_binding_set_tunnel_key(sb_dp, tunnel_key); > + sdp = synced_datapath_alloc(udp, NULL, true); > + synced_datapath_set_sb_fields(sb_dp, udp); > + hmap_insert(&synced_datapaths->pending_dps, &sdp->hmap_node, > + uuid_hash(sb_dp->nb_uuid)); > + /* Don't mark the node as UPDATED since we are only adding this > + * to the pending datapaths. When the southbound datapath binding > + * handler runs, this will get moved to synced_dps and added to > + * synced_datapaths->new. > + */ > + } > + > + 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_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 already have > + * a non-pending 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 { > + sdp = find_synced_datapath_from_sb( > + &synced_datapaths->pending_dps, sb_dp); > + if (!sdp) { > + return EN_UNHANDLED; > + } > + sdp->sb_dp = sb_dp; > + hmap_remove(&synced_datapaths->pending_dps, > &sdp->hmap_node); > + hmap_insert(&synced_datapaths->synced_dps, > &sdp->hmap_node, > + sdp->hmap_node.hash); > + hmapx_add(&synced_datapaths->new, sdp); > + ret = EN_HANDLED_UPDATED; > + } > + 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) > { > @@ -304,7 +582,11 @@ en_datapath_sync_run(struct engine_node *node , void > *data) > delete_unassigned_candidates(&candidate_sdps); > vector_destroy(&candidate_sdps); > > - return EN_UPDATED; > + if (hmap_count(&synced_datapaths->synced_dps)) { > + return EN_UPDATED; > + } else { > + return EN_UPDATED; > + } > This looks like a mistake, I guess the else should return EN_HANDLED_UNCHANGED. } > > void en_datapath_sync_cleanup(void *data) > @@ -312,9 +594,21 @@ 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); > } > + HMAP_FOR_EACH_POP (sdp, hmap_node, &synced_datapaths->pending_dps) { > + free(sdp); > + } > hmap_destroy(&synced_datapaths->synced_dps); > + hmap_destroy(&synced_datapaths->pending_dps); > ovn_destroy_tnlids(&synced_datapaths->dp_tnlids); > } > diff --git a/northd/en-datapath-sync.h b/northd/en-datapath-sync.h > index 3b3262304..bed2d5a7b 100644 > --- a/northd/en-datapath-sync.h > +++ b/northd/en-datapath-sync.h > @@ -21,7 +21,14 @@ > > 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_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-northd.c b/northd/en-northd.c > index 76e35c97c..60f3f2ef9 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/inc-proc-northd.c b/northd/inc-proc-northd.c > index 0dfbf00b5..166c0d814 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,13 @@ 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_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); > @@ -243,7 +246,6 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > engine_add_input(&en_northd, &en_sb_chassis, NULL); > engine_add_input(&en_northd, &en_sb_mirror, NULL); > engine_add_input(&en_northd, &en_sb_meter, NULL); > - engine_add_input(&en_northd, &en_sb_datapath_binding, NULL); > engine_add_input(&en_northd, &en_sb_dns, NULL); > engine_add_input(&en_northd, &en_sb_ha_chassis_group, NULL); > engine_add_input(&en_northd, &en_sb_ip_multicast, NULL); > diff --git a/northd/northd.h b/northd/northd.h > index 05591c07e..4c142cde0 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; > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 76b59408c..d3fd924fb 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -17446,9 +17446,9 @@ check ovn-nbctl --wait=sb sync > > northd/ovn-northd.log > check as northd ovn-appctl -t ovn-northd vlog/set jsonrpc:dbg > check as northd ovn-appctl -t ovn-northd vlog/set inc_proc_eng:dbg > -# Any change that invoke engine processing, for example add logical > switch. > +# Any change that invoke engine processing, for example add address set. > # nb_cfg bump must be present in order to get feedback as sb_cfg. > -check ovn-nbctl --wait=sb ls-add sw1 > +check ovn-nbctl --wait=sb pg-add pg1 > # > # Get following messages from log file: > # 'unix... transact ... Southbound' --> Indicates the pack of updates has > been > @@ -17806,3 +17806,43 @@ 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 > + > +AT_CLEANUP > +]) > -- > 2.49.0 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > There are also a lot of failing tests, please check it out. Thanks, Ales _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev