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>
>
> This is a preliminary patch to introduce Incremental Processing to
> Datapath-sync node.
>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com>
> ---
>
Hi Mark and Lorenzo,
thank you for the patch. I have one minor comment down below.
northd/datapath-sync.h | 5 +++++
> northd/en-datapath-sync.c | 28 ++++++++++++++--------------
> 2 files changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/northd/datapath-sync.h b/northd/datapath-sync.h
> index 081d0af93..58f5f1382 100644
> --- a/northd/datapath-sync.h
> +++ b/northd/datapath-sync.h
> @@ -78,6 +78,11 @@ struct ovn_synced_datapath {
>
> struct ovn_synced_datapaths {
> struct hmap synced_dps;
> + struct hmap dp_tnlids;
> +
> + struct hmapx new;
> + struct hmapx updated;
> + struct hmapx deleted;
>
This is unused in this patch, it should be defined in the next one.
> };
>
> struct ovn_unsynced_datapath *ovn_unsynced_datapath_alloc(
> diff --git a/northd/en-datapath-sync.c b/northd/en-datapath-sync.c
> index ec25b0b2c..db84448e0 100644
> --- a/northd/en-datapath-sync.c
> +++ b/northd/en-datapath-sync.c
> @@ -35,6 +35,7 @@ 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),
> + .dp_tnlids = HMAP_INITIALIZER(&synced_datapaths->dp_tnlids),
> };
>
> return synced_datapaths;
> @@ -102,6 +103,8 @@ reset_synced_datapaths(struct ovn_synced_datapaths
> *synced_datapaths)
> HMAP_FOR_EACH_POP (sdp, hmap_node, &synced_datapaths->synced_dps) {
> free(sdp);
> }
> + ovn_destroy_tnlids(&synced_datapaths->dp_tnlids);
> + hmap_init(&synced_datapaths->dp_tnlids);
> }
>
> static void
> @@ -169,7 +172,6 @@ create_synced_datapath_candidates_from_nb(
>
> static void
> assign_requested_tunnel_keys(struct vector *candidate_sdps,
> - struct hmap *dp_tnlids,
> struct ovn_synced_datapaths
> *synced_datapaths)
> {
> struct candidate_sdp *candidate;
> @@ -177,7 +179,8 @@ assign_requested_tunnel_keys(struct vector
> *candidate_sdps,
> if (!candidate->requested_tunnel_key) {
> continue;
> }
> - if (!ovn_add_tnlid(dp_tnlids, candidate->requested_tunnel_key)) {
> + if (!ovn_add_tnlid(&synced_datapaths->dp_tnlids,
> + candidate->requested_tunnel_key)) {
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> VLOG_WARN_RL(&rl, "Logical datapath "UUID_FMT" requests same "
> "tunnel key %"PRIu32" as another logical
> datapath",
> @@ -195,7 +198,6 @@ assign_requested_tunnel_keys(struct vector
> *candidate_sdps,
>
> static void
> assign_existing_tunnel_keys(struct vector *candidate_sdps,
> - struct hmap *dp_tnlids,
> struct ovn_synced_datapaths *synced_datapaths)
> {
> struct candidate_sdp *candidate;
> @@ -207,7 +209,8 @@ assign_existing_tunnel_keys(struct vector
> *candidate_sdps,
> /* Existing southbound DP. If this key is available,
> * reuse it.
> */
> - if (ovn_add_tnlid(dp_tnlids, candidate->existing_tunnel_key)) {
> + 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));
> @@ -218,7 +221,6 @@ assign_existing_tunnel_keys(struct vector
> *candidate_sdps,
>
> static void
> allocate_tunnel_keys(struct vector *candidate_sdps,
> - struct hmap *dp_tnlids,
> uint32_t max_dp_tunnel_id,
> struct ovn_synced_datapaths *synced_datapaths)
> {
> @@ -229,7 +231,8 @@ allocate_tunnel_keys(struct vector *candidate_sdps,
> continue;
> }
> uint32_t tunnel_key =
> - ovn_allocate_tnlid(dp_tnlids, "datapath",
> OVN_MIN_DP_KEY_LOCAL,
> + ovn_allocate_tnlid(&synced_datapaths->dp_tnlids, "datapath",
> + OVN_MIN_DP_KEY_LOCAL,
> max_dp_tunnel_id, &hint);
> if (!tunnel_key) {
> continue;
> @@ -293,18 +296,14 @@ en_datapath_sync_run(struct engine_node *node , void
> *data)
> &candidate_sdps);
> uuidset_destroy(&visited);
>
> - struct hmap dp_tnlids = HMAP_INITIALIZER(&dp_tnlids);
> - assign_requested_tunnel_keys(&candidate_sdps, &dp_tnlids,
> - synced_datapaths);
> - assign_existing_tunnel_keys(&candidate_sdps, &dp_tnlids,
> synced_datapaths);
> - allocate_tunnel_keys(&candidate_sdps, &dp_tnlids,
> - global_config->max_dp_tunnel_id,
> synced_datapaths);
> + assign_requested_tunnel_keys(&candidate_sdps, synced_datapaths);
> + assign_existing_tunnel_keys(&candidate_sdps, synced_datapaths);
> + allocate_tunnel_keys(&candidate_sdps, global_config->max_dp_tunnel_id,
> + synced_datapaths);
>
> delete_unassigned_candidates(&candidate_sdps);
> vector_destroy(&candidate_sdps);
>
> - ovn_destroy_tnlids(&dp_tnlids);
> -
> return EN_UPDATED;
> }
>
> @@ -317,4 +316,5 @@ void en_datapath_sync_cleanup(void *data)
> free(sdp);
> }
> hmap_destroy(&synced_datapaths->synced_dps);
> + ovn_destroy_tnlids(&synced_datapaths->dp_tnlids);
> }
> --
> 2.49.0
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
With that addressed:
Acked-by: Ales Musil <amu...@redhat.com>
Thanks,
Ales
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
