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

Reply via email to