On 9/29/25 10:36 AM, Ales Musil wrote: > On Sat, Sep 27, 2025 at 12:20 AM Dumitru Ceara <[email protected]> wrote: > >> Store the evpn_datapaths in I-P node data as it will be used by >> upcoming patches, as input for new I-P nodes. We also store a pointer >> to 'struct local_datapath' objects instead of just the datapath key in >> order to avoid looking up the datapath multiple time when reading the >> (SB) EVPN datapath configuration is needed. Instead of storing EVPN >> datapaths as a vector, store them in a hashmap to avoid linear lookups. >> >> Signed-off-by: Dumitru Ceara <[email protected]> >> --- >> controller/evpn-binding.c | 84 ++++++++++++++++++++----------------- >> controller/evpn-binding.h | 13 ++++++ >> controller/ovn-controller.c | 6 +++ >> 3 files changed, 64 insertions(+), 39 deletions(-) >> >> diff --git a/controller/evpn-binding.c b/controller/evpn-binding.c >> index 4bd67b618c..4170239e1b 100644 >> --- a/controller/evpn-binding.c >> +++ b/controller/evpn-binding.c >> @@ -30,15 +30,8 @@ VLOG_DEFINE_THIS_MODULE(evpn_binding); >> >> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); >> >> -struct evpn_datapath { >> - uint32_t vni; >> - uint32_t dp_key; >> -}; >> - >> -static struct vector collect_evpn_datapaths( >> - const struct hmap *local_datapaths); >> -static const struct evpn_datapath *evpn_datapath_find( >> - const struct vector *evpn_datapaths, uint32_t vni); >> +static void collect_evpn_datapaths(const struct hmap *local_datapaths, >> + struct hmap *evpn_datapaths); >> >> struct evpn_tunnel { >> uint16_t dst_port; >> @@ -64,13 +57,13 @@ void >> evpn_binding_run(const struct evpn_binding_ctx_in *b_ctx_in, >> struct evpn_binding_ctx_out *b_ctx_out) >> { >> - struct vector datapaths = >> - collect_evpn_datapaths(b_ctx_in->local_datapaths); >> struct vector tunnels = >> collect_evpn_tunnel_interfaces(b_ctx_in->br_int); >> struct hmapx stale_bindings = HMAPX_INITIALIZER(&stale_bindings); >> struct hmapx stale_mc_groups = HMAPX_INITIALIZER(&stale_mc_groups); >> uint32_t hint = OVN_MIN_EVPN_KEY; >> >> + collect_evpn_datapaths(b_ctx_in->local_datapaths, >> b_ctx_out->datapaths); >> + >> struct evpn_binding *binding; >> HMAP_FOR_EACH (binding, hmap_node, b_ctx_out->bindings) { >> hmapx_add(&stale_bindings, binding); >> @@ -90,8 +83,8 @@ evpn_binding_run(const struct evpn_binding_ctx_in >> *b_ctx_in, >> continue; >> } >> >> - const struct evpn_datapath *edp = evpn_datapath_find(&datapaths, >> - vtep->vni); >> + const struct evpn_datapath *edp = >> + evpn_datapath_find(b_ctx_out->datapaths, vtep->vni); >> if (!edp) { >> VLOG_WARN_RL(&rl, "Couldn't find EVPN datapath for %"PRIu16" >> VNI", >> vtep->vni); >> @@ -123,8 +116,8 @@ evpn_binding_run(const struct evpn_binding_ctx_in >> *b_ctx_in, >> updated = true; >> } >> >> - if (binding->dp_key != edp->dp_key) { >> - binding->dp_key = edp->dp_key; >> + if (binding->dp_key != edp->ldp->datapath->tunnel_key) { >> + binding->dp_key = edp->ldp->datapath->tunnel_key; >> updated = true; >> } >> >> @@ -163,7 +156,6 @@ evpn_binding_run(const struct evpn_binding_ctx_in >> *b_ctx_in, >> free(binding); >> } >> >> - vector_destroy(&datapaths); >> vector_destroy(&tunnels); >> hmapx_destroy(&stale_bindings); >> hmapx_destroy(&stale_mc_groups); >> @@ -219,6 +211,35 @@ evpn_vtep_binding_list(struct unixctl_conn *conn, int >> argc OVS_UNUSED, >> ds_destroy(&ds); >> } >> >> +const struct evpn_datapath * >> +evpn_datapath_find(const struct hmap *evpn_datapaths, uint32_t vni) >> +{ >> + const struct evpn_datapath *edp; >> + HMAP_FOR_EACH_WITH_HASH (edp, hmap_node, vni, evpn_datapaths) { >> + if (edp->vni == vni) { >> + return edp; >> + } >> + } >> + >> + return NULL; >> +} >> + >> +void >> +evpn_datapaths_clear(struct hmap *evpn_datapaths) >> +{ >> + struct evpn_datapath *edp; >> + HMAP_FOR_EACH_POP (edp, hmap_node, evpn_datapaths) { >> + free(edp); >> + } >> +} >> + >> +void >> +evpn_datapaths_destroy(struct hmap *evpn_datapaths) >> +{ >> + evpn_datapaths_clear(evpn_datapaths); >> + hmap_destroy(evpn_datapaths); >> +} >> + >> void >> evpn_multicast_groups_destroy(struct hmap *multicast_groups) >> { >> @@ -257,12 +278,10 @@ evpn_multicast_group_list(struct unixctl_conn *conn, >> int argc OVS_UNUSED, >> ds_destroy(&ds); >> } >> >> -static struct vector >> -collect_evpn_datapaths(const struct hmap *local_datapaths) >> +static void >> +collect_evpn_datapaths(const struct hmap *local_datapaths, >> + struct hmap *evpn_datapaths) >> { >> - struct vector evpn_datapaths = >> - VECTOR_EMPTY_INITIALIZER(struct evpn_datapath); >> - >> struct local_datapath *ld; >> HMAP_FOR_EACH (ld, hmap_node, local_datapaths) { >> if (!ld->is_switch) { >> @@ -275,33 +294,20 @@ collect_evpn_datapaths(const struct hmap >> *local_datapaths) >> continue; >> } >> >> - if (evpn_datapath_find(&evpn_datapaths, vni)) { >> + if (evpn_datapath_find(evpn_datapaths, vni)) { >> VLOG_WARN_RL(&rl, "Datapath "UUID_FMT" with duplicate VNI >> %"PRIi64, >> UUID_ARGS(&ld->datapath->header_.uuid), vni); >> continue; >> } >> >> - struct evpn_datapath edp = { >> + struct evpn_datapath *edp = xmalloc(sizeof *edp); >> + *edp = (struct evpn_datapath) { >> + .ldp = ld, >> .vni = vni, >> - .dp_key = ld->datapath->tunnel_key, >> }; >> - vector_push(&evpn_datapaths, &edp); >> - } >> >> - return evpn_datapaths; >> -} >> - >> -static const struct evpn_datapath * >> -evpn_datapath_find(const struct vector *evpn_datapaths, uint32_t vni) >> -{ >> - const struct evpn_datapath *edp; >> - VECTOR_FOR_EACH_PTR (evpn_datapaths, edp) { >> - if (edp->vni == vni) { >> - return edp; >> - } >> + hmap_insert(evpn_datapaths, &edp->hmap_node, edp->vni); >> } >> - >> - return NULL; >> } >> >> static struct vector >> diff --git a/controller/evpn-binding.h b/controller/evpn-binding.h >> index 229d29b09c..38fbea0d2a 100644 >> --- a/controller/evpn-binding.h >> +++ b/controller/evpn-binding.h >> @@ -36,6 +36,8 @@ struct evpn_binding_ctx_in { >> struct evpn_binding_ctx_out { >> /* Contains 'struct evpn_binding'. */ >> struct hmap *bindings; >> + /* Contains 'struct evpn_datapath'. */ >> + struct hmap *datapaths; >> /* Contains pointers to 'struct evpn_binding'. */ >> struct hmapx *updated_bindings; >> /* Contains 'flow_uuid' from removed 'struct evpn_binding'. */ >> @@ -64,6 +66,13 @@ struct evpn_binding { >> uint32_t dp_key; >> }; >> >> +struct evpn_datapath { >> + struct hmap_node hmap_node; >> + >> + const struct local_datapath *ldp; >> + uint32_t vni; >> +}; >> + >> struct evpn_multicast_group { >> struct hmap_node hmap_node; >> /* UUID used to identify physical flows related to this mutlicast >> group. */ >> @@ -81,6 +90,10 @@ struct evpn_binding *evpn_binding_find(const struct >> hmap *evpn_bindings, >> void evpn_bindings_destroy(struct hmap *bindings); >> void evpn_vtep_binding_list(struct unixctl_conn *conn, int argc, >> const char *argv[], void *data_); >> +const struct evpn_datapath *evpn_datapath_find( >> + const struct hmap *evpn_datapaths, uint32_t vni); >> +void evpn_datapaths_clear(struct hmap *evpn_datapaths); >> +void evpn_datapaths_destroy(struct hmap *evpn_datapaths); >> void evpn_multicast_groups_destroy(struct hmap *multicast_groups); >> void evpn_multicast_group_list(struct unixctl_conn *conn, int argc, >> const char *argv[], void *data_); >> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c >> index e2793c8837..c8b76feb52 100644 >> --- a/controller/ovn-controller.c >> +++ b/controller/ovn-controller.c >> @@ -4568,6 +4568,8 @@ struct ed_type_evpn_vtep_binding { >> struct hmapx updated_bindings; >> /* Contains 'flow_uuid' from removed 'struct evpn_binding'. */ >> struct uuidset removed_bindings; >> + /* Contains 'struct evpn_datapath'. */ >> + struct hmap datapaths; >> /* Contains 'struct evpn_multicast_group'. */ >> struct hmap multicast_groups; >> /* Contains pointers to 'struct evpn_multicast_group'. */ >> @@ -6136,6 +6138,7 @@ en_evpn_vtep_binding_init(struct engine_node *node >> OVS_UNUSED, >> .bindings = HMAP_INITIALIZER(&data->bindings), >> .updated_bindings = HMAPX_INITIALIZER(&data->updated_bindings), >> .removed_bindings = UUIDSET_INITIALIZER(&data->removed_bindings), >> + .datapaths = HMAP_INITIALIZER(&data->datapaths), >> .multicast_groups = HMAP_INITIALIZER(&data->multicast_groups), >> .updated_multicast_groups = >> HMAPX_INITIALIZER(&data->updated_multicast_groups), >> @@ -6153,6 +6156,7 @@ en_evpn_vtep_binding_clear_tracked_data(void *data_) >> struct ed_type_evpn_vtep_binding *data = data_; >> hmapx_clear(&data->updated_bindings); >> uuidset_clear(&data->removed_bindings); >> + evpn_datapaths_clear(&data->datapaths); >> hmapx_clear(&data->updated_multicast_groups); >> uuidset_clear(&data->removed_multicast_groups); >> } >> @@ -6164,6 +6168,7 @@ en_evpn_vtep_binding_cleanup(void *data_) >> evpn_bindings_destroy(&data->bindings); >> hmapx_destroy(&data->updated_bindings); >> uuidset_destroy(&data->removed_bindings); >> + evpn_datapaths_destroy(&data->datapaths); >> evpn_multicast_groups_destroy(&data->multicast_groups); >> hmapx_clear(&data->updated_multicast_groups); >> uuidset_clear(&data->removed_multicast_groups); >> @@ -6194,6 +6199,7 @@ en_evpn_vtep_binding_run(struct engine_node *node, >> void *data_) >> .bindings = &data->bindings, >> .updated_bindings = &data->updated_bindings, >> .removed_bindings = &data->removed_bindings, >> + .datapaths = &data->datapaths, >> .multicast_groups = &data->multicast_groups, >> .updated_multicast_groups = &data->updated_multicast_groups, >> .removed_multicast_groups = &data->removed_multicast_groups, >> -- >> 2.51.0 >> >> > Looks good to me, thanks. > > Acked-by: Ales Musil <[email protected]> >
Hi Ales, Thanks for the review! Applied to main. Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
