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]> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
