On Tue, Aug 12, 2025 at 3:06 PM Xavier Simonart <xsimo...@redhat.com> wrote:
> Hi Ales, Dumitru > > Thanks for the patch > I only have one nit below > Acked-by: Xavier Simonart <xsimo...@redhat.com> > Thanks > Xavier > Hi Xavier, thank you for the review. > > On Wed, Aug 6, 2025 at 8:29 PM Ales Musil via dev <ovs-dev@openvswitch.org> > wrote: > >> On Wed, Aug 6, 2025 at 12:53 PM Dumitru Ceara <dce...@redhat.com> wrote: >> >> > Hi Ales, >> > >> >> Hi Dumitru, >> >> thank you for the review. >> >> > >> > On 7/25/25 8:03 AM, Ales Musil wrote: >> > > Pair learned remote VTEPs with their respective datapaths and tunnel >> > > ports. This creates binding that can be used for physical flows. In >> > > order to accommodate for multicast groups we need to also create >> > > specific EVPN multicast groups. >> > >> > Maybe just add a small note here too (like in 05/11) that the >> > "dynamic-routing-vni" cannot be set through the NB yet and will be part >> > of a later commit? >> > >> > Otherwise, I only have a few tiny comments below, the rest looks good to >> > me. >> > >> > > >> > > All data are ovn-controller specific due to the nature of EVPN and >> > > also to avoid possible scalability issues within SB database. >> > > >> > > Those bindings and multicast groups are used in later commits to >> > > create physical flows responsible for the delivery. >> > > >> > > Signed-off-by: Ales Musil <amu...@redhat.com> >> > > --- >> > > controller/automake.mk | 2 + >> > > controller/evpn-binding.c | 427 >> ++++++++++++++++++++++++++++++++++++ >> > > controller/evpn-binding.h | 88 ++++++++ >> > > controller/ovn-controller.c | 200 ++++++++++++++++- >> > > lib/ovn-util.c | 16 ++ >> > > lib/ovn-util.h | 5 + >> > > 6 files changed, 727 insertions(+), 11 deletions(-) >> > > create mode 100644 controller/evpn-binding.c >> > > create mode 100644 controller/evpn-binding.h >> > > >> > > diff --git a/controller/automake.mk b/controller/automake.mk >> > > index 37cfd4396..8d94fb646 100644 >> > > --- a/controller/automake.mk >> > > +++ b/controller/automake.mk >> > > @@ -10,6 +10,8 @@ controller_ovn_controller_SOURCES = \ >> > > controller/chassis.h \ >> > > controller/encaps.c \ >> > > controller/encaps.h \ >> > > + controller/evpn-binding.c \ >> > > + controller/evpn-binding.h \ >> > > controller/ha-chassis.c \ >> > > controller/ha-chassis.h \ >> > > controller/if-status.c \ >> > > diff --git a/controller/evpn-binding.c b/controller/evpn-binding.c >> > > new file mode 100644 >> > > index 000000000..86cb7df27 >> > > --- /dev/null >> > > +++ b/controller/evpn-binding.c >> > > @@ -0,0 +1,427 @@ >> > > +/* Copyright (c) 2025, Red Hat, Inc. >> > > + * >> > > + * Licensed under the Apache License, Version 2.0 (the "License"); >> > > + * you may not use this file except in compliance with the License. >> > > + * You may obtain a copy of the License at: >> > > + * >> > > + * http://www.apache.org/licenses/LICENSE-2.0 >> > > + * >> > > + * Unless required by applicable law or agreed to in writing, >> software >> > > + * distributed under the License is distributed on an "AS IS" BASIS, >> > > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >> > implied. >> > > + * See the License for the specific language governing permissions >> and >> > > + * limitations under the License. >> > > + */ >> > > + >> > > +#include <config.h> >> > > + >> > > +#include "flow.h" >> > > +#include "local_data.h" >> > > +#include "neighbor-exchange.h" >> > > +#include "openvswitch/vlog.h" >> > > +#include "ovn-sb-idl.h" >> > > +#include "unixctl.h" >> > > +#include "vec.h" >> > > +#include "vswitch-idl.h" >> > > + >> > > +#include "evpn-binding.h" >> > > + >> > > +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); >> > > + >> > > +struct evpn_tunnel { >> > > + uint16_t dst_port; >> > > + ofp_port_t ofport; >> > > +}; >> > > + >> > > +static struct vector collect_evpn_tunnel_interfaces( >> > > + const struct ovsrec_bridge *br_int); >> > > +static bool is_evpn_tunnel_port(const struct ovsrec_port *port); >> > >> > Nit: 'port' can be omitted. >> > >> > > +static const struct evpn_tunnel *evpn_tunnel_find( >> > > + const struct vector *evpn_tunnels, uint16_t dst_port); >> > > + >> > > +static struct evpn_binding *evpn_binding_add( >> > > + struct hmap *evpn_bindings, const struct evpn_remote_vtep *vtep, >> > > + uint32_t tunnel_key); >> > > +static uint32_t evpn_binding_hash(const struct in6_addr *remote_ip, >> > > + uint32_t vni); >> > > +static struct evpn_multicast_group *evpn_multicast_group_find( >> > > + const struct hmap *evpn_mc_groups, uint32_t vni); >> > > +static struct evpn_multicast_group *evpn_multicast_group_add( >> > > + struct hmap *evpn_mc_groups, uint32_t vni); >> > > + >> > > +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; >> > > + >> > > + struct evpn_binding *binding; >> > > + HMAP_FOR_EACH (binding, hmap_node, b_ctx_out->bindings) { >> > > + hmapx_add(&stale_bindings, binding); >> > > + } >> > > + >> > > + struct evpn_multicast_group *mc_group; >> > > + HMAP_FOR_EACH (mc_group, hmap_node, b_ctx_out->multicast_groups) >> { >> > > + hmapx_add(&stale_mc_groups, mc_group); >> > > + } >> > > + >> > > + const struct evpn_remote_vtep *vtep; >> > > + HMAP_FOR_EACH (vtep, hmap_node, b_ctx_in->remote_vteps) { >> > > + const struct evpn_tunnel *tun = evpn_tunnel_find(&tunnels, >> > vtep->port); >> > > + if (!tun) { >> > > + VLOG_WARN_RL(&rl, "Couldn't find EVPN tunnel for %"PRIu16 >> > > + " destination port.", vtep->port); >> > > + continue; >> > > + } >> > > + >> > > + const struct evpn_datapath *edp = >> evpn_datapath_find(&datapaths, >> > > + >> vtep->vni); >> > > + if (!edp) { >> > > + VLOG_WARN_RL(&rl, "Couldn't find EVPN datapath for >> > %"PRIu16" VNI", >> > > + vtep->vni); >> > > + continue; >> > > + } >> > > + >> > > + binding = evpn_binding_find(b_ctx_out->bindings, &vtep->ip, >> > vtep->vni); >> > > + if (!binding) { >> > > + uint32_t tunnel_key = >> > > + ovn_allocate_tnlid(b_ctx_out->tunnel_keys, >> > "evpn-binding", >> > > + OVN_MIN_EVPN_KEY, >> OVN_MAX_EVPN_KEY, >> > &hint); >> > > + if (!tunnel_key) { >> > > + continue; >> > > + } >> > > + >> > > + binding = evpn_binding_add(b_ctx_out->bindings, vtep, >> > tunnel_key); >> > > + } >> > > + >> > > + mc_group = >> > evpn_multicast_group_find(b_ctx_out->multicast_groups, >> > > + vtep->vni); >> > > + if (!mc_group) { >> > > + mc_group = >> > evpn_multicast_group_add(b_ctx_out->multicast_groups, >> > > + vtep->vni); >> > > + } >> > > + >> > > + bool updated = false; >> > > + if (binding->tunnel_ofport != tun->ofport) { >> > > + binding->tunnel_ofport = tun->ofport; >> > > + updated = true; >> > > + } >> > > + >> > > + if (binding->dp_key != edp->dp_key) { >> > > + binding->dp_key = edp->dp_key; >> > > + updated = true; >> > > + } >> > > + >> > > + if (updated) { >> > > + hmapx_add(b_ctx_out->updated_bindings, binding); >> > > + >> > > + hmapx_add(&mc_group->bindings, binding); >> > > + hmapx_add(b_ctx_out->updated_multicast_groups, mc_group); >> > > + } >> > > + >> > > + hmapx_find_and_delete(&stale_bindings, binding); >> > > + hmapx_find_and_delete(&stale_mc_groups, mc_group); >> > > + } >> > > + >> > > + struct hmapx_node *node; >> > > + HMAPX_FOR_EACH (node, &stale_mc_groups) { >> > > + mc_group = node->data; >> > > + >> > > + uuidset_insert(b_ctx_out->removed_multicast_groups, >> > > + &mc_group->flow_uuid); >> > > + hmap_remove(b_ctx_out->multicast_groups, >> &mc_group->hmap_node); >> > > + free(mc_group); >> > > + } >> > > + >> > > + HMAPX_FOR_EACH (node, &stale_bindings) { >> > > + binding = node->data; >> > > + >> > > + mc_group = >> > evpn_multicast_group_find(b_ctx_out->multicast_groups, >> > > + binding->vni); >> > > + if (mc_group) { >> > > + hmapx_find_and_delete(&mc_group->bindings, binding); >> > > + hmapx_add(b_ctx_out->updated_multicast_groups, mc_group); >> > > + } >> > > + uuidset_insert(b_ctx_out->removed_bindings, >> > &binding->flow_uuid); >> > > + hmap_remove(b_ctx_out->bindings, &binding->hmap_node); >> > > + free(binding); >> > > + } >> > > + >> > > + vector_destroy(&datapaths); >> > > + vector_destroy(&tunnels); >> > > + hmapx_destroy(&stale_bindings); >> > > + hmapx_destroy(&stale_mc_groups); >> > > +} >> > > + >> > > +struct evpn_binding * >> > > +evpn_binding_find(const struct hmap *evpn_bindings, >> > > + const struct in6_addr *remote_ip, uint32_t vni) >> > > +{ >> > > + uint32_t hash = evpn_binding_hash(remote_ip, vni); >> > > + >> > > + struct evpn_binding *binding; >> > > + HMAP_FOR_EACH_WITH_HASH (binding, hmap_node, hash, >> evpn_bindings) { >> > > + if (ipv6_addr_equals(&binding->remote_ip, remote_ip) && >> > > + binding->vni == vni) { >> > > + return binding; >> > > + } >> > > + } >> > > + >> > > + return NULL; >> > > +} >> > > + >> > > +void >> > > +evpn_bindings_destroy(struct hmap *bindings) >> > > +{ >> > > + struct evpn_binding *binding; >> > > + HMAP_FOR_EACH_POP (binding, hmap_node, bindings) { >> > > + free(binding); >> > > + } >> > > + hmap_destroy(bindings); >> > > +} >> > > + >> > > +void >> > > +evpn_vtep_binding_list(struct unixctl_conn *conn, int argc >> OVS_UNUSED, >> > > + const char *argv[] OVS_UNUSED, void *data_) >> > > +{ >> > > + struct hmap *bindings = data_; >> > > + struct ds ds = DS_EMPTY_INITIALIZER; >> > > + >> > > + const struct evpn_binding *binding; >> > > + HMAP_FOR_EACH (binding, hmap_node, bindings) { >> > > + ds_put_format(&ds, "UUID: "UUID_FMT", Remote IP: ", >> > > + UUID_ARGS(&binding->flow_uuid)); >> > > + ipv6_format_mapped(&binding->remote_ip, &ds); >> > > + ds_put_format(&ds, ", vni: %"PRIu32", tunnel_key: >> %#"PRIx32", " >> > > + "ofport: %"PRIu32", dp_key: %"PRIu32, >> > binding->vni, >> > > + binding->tunnel_key, binding->tunnel_ofport, >> > > + binding->dp_key); >> > > + ds_put_char(&ds, '\n'); >> > > + } >> > > + >> > > + unixctl_command_reply(conn, ds_cstr_ro(&ds)); >> > > + ds_destroy(&ds); >> > > +} >> > > + >> > > +void >> > > +evpn_multicast_groups_destroy(struct hmap *multicast_groups) >> > > +{ >> > > + struct evpn_multicast_group *mc_group; >> > > + HMAP_FOR_EACH_POP (mc_group, hmap_node, multicast_groups) { >> > > + free(mc_group); >> > > + } >> > > + hmap_destroy(multicast_groups); >> > > +} >> > > + >> > > + >> > > +void >> > > +evpn_multicast_group_list(struct unixctl_conn *conn, int argc >> > OVS_UNUSED, >> > > + const char *argv[] OVS_UNUSED, void >> *data_) >> > > +{ >> > > + struct hmap *mc_groups = data_; >> > > + struct ds ds = DS_EMPTY_INITIALIZER; >> > > + >> > > + const struct evpn_multicast_group *mc_group; >> > > + HMAP_FOR_EACH (mc_group, hmap_node, mc_groups) { >> > > + ds_put_format(&ds, "UUID: "UUID_FMT", Remote IPs: ", >> > > + UUID_ARGS(&mc_group->flow_uuid)); >> > > + >> > > + struct hmapx_node *node; >> > > + HMAPX_FOR_EACH (node, &mc_group->bindings) { >> > > + const struct evpn_binding *binding = node->data; >> > > + ipv6_format_mapped(&binding->remote_ip, &ds); >> > > + ds_put_cstr(&ds, ", "); >> > > + } >> > > + >> > > + ds_put_format(&ds, "vni: %"PRIu32, mc_group->vni); >> > > + ds_put_char(&ds, '\n'); >> > > + } >> > > + >> > > + unixctl_command_reply(conn, ds_cstr_ro(&ds)); >> > > + ds_destroy(&ds); >> > > +} >> > > + >> > > +static struct vector >> > > +collect_evpn_datapaths(const struct hmap *local_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) { >> > > + continue; >> > > + } >> > > + >> > > + int64_t vni = ovn_smap_get_llong(&ld->datapath->external_ids, >> > > + "dynamic-routing-vni", -1); >> > > + if (!ovn_is_valid_vni(vni)) { >> > > + continue; >> > > + } >> > > + >> > > + 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 = { >> > > + .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; >> > > + } >> > > + } >> > > + >> > > + return NULL; >> > > +} >> > > + >> > > +static struct vector >> > > +collect_evpn_tunnel_interfaces(const struct ovsrec_bridge *br_int) >> > > +{ >> > > + struct vector evpn_tunnels = VECTOR_EMPTY_INITIALIZER(struct >> > evpn_tunnel); >> > > + >> > > + for (size_t i = 0; i < br_int->n_ports; i++) { >> > > + const struct ovsrec_port *port = br_int->ports[i]; >> > > + >> > > + if (!is_evpn_tunnel_port(port)) { >> > > + continue; >> > > + } >> > > + >> > > + const struct ovsrec_interface *iface = port->interfaces[0]; >> > > + if (iface->n_ofport != 1) { >> > > + continue; >> > > + } >> > > + >> > > + const char *dst_port_str = >> > > + smap_get_def(&iface->options, "dst_port", >> > > + OVS_STRINGIZE(DEFAULT_VXLAN_PORT)); >> > > + unsigned int dst_port; >> > > + if (!str_to_uint(dst_port_str, 10, &dst_port)) { >> > > + VLOG_WARN_RL(&rl, "Couldn't parse \"dst_port\" %s for >> > tunnel" >> > > + " interface %s", dst_port_str, iface->name); >> > > + continue; >> > > + } >> > > + >> > > + struct evpn_tunnel tun = { >> > > + .dst_port = dst_port, >> > > + .ofport = u16_to_ofp(iface->ofport[0]), >> > > + }; >> > > + vector_push(&evpn_tunnels, &tun); >> > > + } >> > > + >> > > + return evpn_tunnels; >> > > +} >> > > + >> > > +static bool >> > > +is_evpn_tunnel_port(const struct ovsrec_port *port) >> > > +{ >> > > + if (port->n_interfaces != 1) { >> > > + return false; >> > > + } >> > > + >> > > + return ovn_is_evpn_tunnel_interface(port->interfaces[0]); >> > > +} >> > > + >> > > +static const struct evpn_tunnel * >> > > +evpn_tunnel_find(const struct vector *evpn_tunnels, uint16_t >> dst_port) >> > > +{ >> > > + const struct evpn_tunnel *tun; >> > > + VECTOR_FOR_EACH_PTR (evpn_tunnels, tun) { >> > > + if (tun->dst_port == dst_port) { >> > > + return tun; >> > > + } >> > > + } >> > > + >> > > + return NULL; >> > > +} >> > > + >> > > +static uint32_t >> > > +evpn_binding_hash(const struct in6_addr *remote_ip, uint32_t vni) >> > > +{ >> > > + uint32_t hash = 0; >> > > + hash = hash_add_in6_addr(hash, remote_ip); >> > > + hash = hash_add(hash, vni); >> > > + >> > > + return hash; >> > > +} >> > > + >> > > +static struct evpn_binding * >> > > +evpn_binding_add(struct hmap *evpn_bindings, >> > > + const struct evpn_remote_vtep *vtep, uint32_t >> > tunnel_key) >> > > +{ >> > > + struct evpn_binding *binding = xmalloc(sizeof *binding); >> > > + *binding = (struct evpn_binding) { >> > > + .flow_uuid = uuid_random(), >> > > + .remote_ip = vtep->ip, >> > > + .vni = vtep->vni, >> > > + .tunnel_key = tunnel_key, >> > > + .tunnel_ofport = OFPP_NONE, >> > > + .dp_key = 0, >> > > + }; >> > > + >> > > + uint32_t hash = evpn_binding_hash(&vtep->ip, vtep->vni); >> > > + hmap_insert(evpn_bindings, &binding->hmap_node, hash); >> > > + >> > > + return binding; >> > > +} >> > > + >> > > +static struct evpn_multicast_group * >> > > +evpn_multicast_group_find(const struct hmap *evpn_mc_groups, uint32_t >> > vni) >> > > +{ >> > > + uint32_t hash = hash_add(0, vni); >> > > + >> > > + struct evpn_multicast_group *mc_group; >> > > + HMAP_FOR_EACH_WITH_HASH (mc_group, hmap_node, hash, >> evpn_mc_groups) >> > { >> > > + if (mc_group->vni == vni) { >> > > + return mc_group; >> > > + } >> > > + } >> > > + >> > > + return NULL; >> > > +} >> > > + >> > > +static struct evpn_multicast_group * >> > > +evpn_multicast_group_add(struct hmap *evpn_mc_groups, uint32_t vni) >> > > +{ >> > > + struct evpn_multicast_group *mc_group = xmalloc(sizeof >> *mc_group); >> > > + *mc_group = (struct evpn_multicast_group) { >> > > + .flow_uuid = uuid_random(), >> > > + .bindings = HMAPX_INITIALIZER(&mc_group->bindings), >> > > + .vni = vni, >> > > + }; >> > > + >> > > + uint32_t hash = hash_add(0, vni); >> > > + hmap_insert(evpn_mc_groups, &mc_group->hmap_node, hash); >> > > + >> > > + return mc_group; >> > > +} >> > > diff --git a/controller/evpn-binding.h b/controller/evpn-binding.h >> > > new file mode 100644 >> > > index 000000000..88d8c202c >> > > --- /dev/null >> > > +++ b/controller/evpn-binding.h >> > > @@ -0,0 +1,88 @@ >> > > +/* Copyright (c) 2025, Red Hat, Inc. >> > > + * >> > > + * Licensed under the Apache License, Version 2.0 (the "License"); >> > > + * you may not use this file except in compliance with the License. >> > > + * You may obtain a copy of the License at: >> > > + * >> > > + * http://www.apache.org/licenses/LICENSE-2.0 >> > > + * >> > > + * Unless required by applicable law or agreed to in writing, >> software >> > > + * distributed under the License is distributed on an "AS IS" BASIS, >> > > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >> > implied. >> > > + * See the License for the specific language governing permissions >> and >> > > + * limitations under the License. >> > > + */ >> > > + >> > > +#ifndef EVPN_BINDING_H >> > > +#define EVPN_BINDING_H 1 >> > > + >> > > +#include <stdint.h> >> > > + >> > > +#include "hmapx.h" >> > > +#include "openvswitch/hmap.h" >> > > +#include "uuidset.h" >> > > + >> > > +struct ovsrec_bridge; >> > > +struct unixctl_conn; >> > > + >> > > +struct evpn_binding_ctx_in { >> > > + const struct ovsrec_bridge *br_int; >> > > + /* Contains 'struct local_datapath'. */ >> > > + const struct hmap *local_datapaths; >> > > + /* Contains 'struct evpn_remote_vtep'. */ >> > > + const struct hmap *remote_vteps; >> > > +}; >> > > + >> > > +struct evpn_binding_ctx_out { >> > > + /* Contains 'struct evpn_binding'. */ >> > > + struct hmap *bindings; >> > > + /* Contains pointers to 'struct evpn_binding'. */ >> > > + struct hmapx *updated_bindings; >> > > + /* Contains 'flow_uuid' from removed 'struct evpn_binding'. */ >> > > + struct uuidset *removed_bindings; >> > > + /* Contains 'struct evpn_multicast_group'. */ >> > > + struct hmap *multicast_groups; >> > > + /* Contains pointers to 'struct evpn_multicast_group'. */ >> > > + struct hmapx *updated_multicast_groups; >> > > + /* Contains 'flow_uuid' from removed 'struct >> evpn_multicast_group'. >> > */ >> > > + struct uuidset *removed_multicast_groups; >> > > + /* Contains 'struct tnlid_node". */ >> > > + struct hmap *tunnel_keys; >> > > +}; >> > > + >> > > +struct evpn_binding { >> > > + struct hmap_node hmap_node; >> > > + /* UUID used to identify physical flows related to this binding. >> */ >> > > + struct uuid flow_uuid; >> > > + /* IP address of the remote VTEP. */ >> > > + struct in6_addr remote_ip; >> > > + uint32_t vni; >> > > + /* Local tunnel key to identify the binding. */ >> > > + uint32_t tunnel_key; >> > > + >> > > + ofp_port_t tunnel_ofport; >> > > + uint32_t dp_key; >> > > +}; >> > > + >> > > +struct evpn_multicast_group { >> > > + struct hmap_node hmap_node; >> > > + /* UUID used to identify physical flows related to this mutlicast >> > group. */ >> > > + struct uuid flow_uuid; >> > > + /* Contains pointers to 'struct evpn_bindings'. */ >> > > + struct hmapx bindings; >> > > + uint32_t vni; >> > > +}; >> > > + >> > > +void evpn_binding_run(const struct evpn_binding_ctx_in *, >> > > + struct evpn_binding_ctx_out *); >> > > +struct evpn_binding *evpn_binding_find(const struct hmap >> *evpn_bindings, >> > > + const struct in6_addr >> *remote_ip, >> > > + uint32_t vni); >> > > +void evpn_bindings_destroy(struct hmap *bindings); >> > > +void evpn_vtep_binding_list(struct unixctl_conn *conn, int argc, >> > > + const char *argv[], void *data_); >> > > +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_); >> > > + >> > > +#endif /* EVPN_BINDING_H */ >> > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c >> > > index 90ef6011a..996dcc021 100644 >> > > --- a/controller/ovn-controller.c >> > > +++ b/controller/ovn-controller.c >> > > @@ -98,6 +98,7 @@ >> > > #include "neighbor.h" >> > > #include "neighbor-exchange.h" >> > > #include "neighbor-table-notify.h" >> > > +#include "evpn-binding.h" >> > > >> > > VLOG_DEFINE_THIS_MODULE(main); >> > > >> > > @@ -4558,6 +4559,23 @@ parse_encap_ips(const struct >> > ovsrec_open_vswitch_table *ovs_table, >> > > } >> > > } >> > > >> > > +struct ed_type_evpn_vtep_binding { >> > > + /* Contains 'struct evpn_binding'. */ >> > > + struct hmap bindings; >> > > + /* Contains pointers to 'struct evpn_binding'. */ >> > > + struct hmapx updated_bindings; >> > > + /* Contains 'flow_uuid' from removed 'struct evpn_binding'. */ >> > > + struct uuidset removed_bindings; >> > > + /* Contains 'struct evpn_multicast_group'. */ >> > > + struct hmap multicast_groups; >> > > + /* Contains pointers to 'struct evpn_multicast_group'. */ >> > > + struct hmapx updated_multicast_groups; >> > > + /* Contains 'flow_uuid' from removed 'struct >> evpn_multicast_group'. >> > */ >> > > + struct uuidset removed_multicast_groups; >> > > + /* Contains 'struct tnlid_node". */ >> > > + struct hmap tunnel_keys; >> > > +}; >> > >> > Nit: I'd add some empty lines between the fields here. I think it would >> > be a bit more readable. >> > >> >> >> I kinda prefer them as is, but if you feel like I should >> add empty lines. I can do it in v3. >> >> >> > >> > > + >> > > static void init_physical_ctx(struct engine_node *node, >> > > struct ed_type_runtime_data *rt_data, >> > > struct ed_type_non_vif_data >> *non_vif_data, >> > > @@ -4988,14 +5006,6 @@ controller_output_garp_rarp_handler(struct >> > engine_node *node OVS_UNUSED, >> > > return EN_HANDLED_UPDATED; >> > > } >> > > >> > > -static enum engine_input_handler_result >> > > -controller_output_neighbor_exchange_handler( >> > > - struct engine_node *node OVS_UNUSED, >> > > - void *data OVS_UNUSED) >> > > -{ >> > > - return EN_HANDLED_UPDATED; >> > > -} >> > > - >> > > /* Handles sbrec_chassis changes. >> > > * If a new chassis is added or removed return false, so that >> > > * flows are recomputed. For any updates, there is no need for >> > > @@ -5943,6 +5953,152 @@ en_neighbor_exchange_status_run(struct >> > engine_node *node OVS_UNUSED, >> > > return state; >> > > } >> > > >> > > +static void * >> > > +en_evpn_vtep_binding_init(struct engine_node *node OVS_UNUSED, >> > > + struct engine_arg *arg OVS_UNUSED) >> > > +{ >> > > + struct ed_type_evpn_vtep_binding *data = xmalloc(sizeof *data); >> > > + *data = (struct ed_type_evpn_vtep_binding) { >> > > + .bindings = HMAP_INITIALIZER(&data->bindings), >> > > + .updated_bindings = >> HMAPX_INITIALIZER(&data->updated_bindings), >> > > + .removed_bindings = >> > UUIDSET_INITIALIZER(&data->removed_bindings), >> > > + .multicast_groups = >> HMAP_INITIALIZER(&data->multicast_groups), >> > > + .updated_multicast_groups = >> > > + HMAPX_INITIALIZER(&data->updated_multicast_groups), >> > > + .removed_multicast_groups = >> > > + UUIDSET_INITIALIZER(&data->removed_multicast_groups), >> > > + .tunnel_keys = HMAP_INITIALIZER(&data->tunnel_keys), >> > > + }; >> > > + >> > > + return data; >> > > +} >> > > + >> > > +static void >> > > +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); >> > > + hmapx_clear(&data->updated_multicast_groups); >> > > + uuidset_clear(&data->removed_multicast_groups); >> > > +} >> > > + >> > > +static void >> > > +en_evpn_vtep_binding_cleanup(void *data_) >> > > +{ >> > > + struct ed_type_evpn_vtep_binding *data = data_; >> > > + evpn_bindings_destroy(&data->bindings); >> > > + hmapx_destroy(&data->updated_bindings); >> > > + uuidset_destroy(&data->removed_bindings); >> > > + evpn_multicast_groups_destroy(&data->multicast_groups); >> > > + hmapx_clear(&data->updated_multicast_groups); >> > > + uuidset_clear(&data->removed_multicast_groups); >> > > + ovn_destroy_tnlids(&data->tunnel_keys); >> > > +} >> > > + >> > > +static enum engine_node_state >> > > +en_evpn_vtep_binding_run(struct engine_node *node, void *data_) >> > > +{ >> > > + struct ed_type_evpn_vtep_binding *data = data_; >> > > + const struct ed_type_neighbor_exchange *ne_data = >> > > + engine_get_input_data("neighbor_exchange", node); >> > > + const struct ed_type_runtime_data *rt_data = >> > > + engine_get_input_data("runtime_data", node); >> > > + const struct ovsrec_open_vswitch_table *ovs_table = >> > > + EN_OVSDB_GET(engine_get_input("OVS_open_vswitch", node)); >> > > + const struct ovsrec_bridge_table *bridge_table = >> > > + EN_OVSDB_GET(engine_get_input("OVS_bridge", node)); >> > > + const struct ovsrec_bridge *br_int = get_br_int(bridge_table, >> > ovs_table); >> > > + >> > > + struct evpn_binding_ctx_in b_ctx_in = { >> > > + .br_int = br_int, >> > > + .local_datapaths = &rt_data->local_datapaths, >> > > + .remote_vteps = &ne_data->remote_vteps, >> > > + }; >> > > + >> > > + struct evpn_binding_ctx_out b_ctx_out = { >> > > + .bindings = &data->bindings, >> > > + .updated_bindings = &data->updated_bindings, >> > > + .removed_bindings = &data->removed_bindings, >> > > + .multicast_groups = &data->multicast_groups, >> > > + .updated_multicast_groups = &data->updated_multicast_groups, >> > > + .removed_multicast_groups = &data->removed_multicast_groups, >> > > + .tunnel_keys = &data->tunnel_keys, >> > > + }; >> > > + >> > > + evpn_binding_run(&b_ctx_in, &b_ctx_out); >> > > + >> > > + if (hmapx_count(&data->updated_bindings) || >> > > + uuidset_count(&data->removed_bindings) || >> > > + hmapx_count(&data->updated_multicast_groups) || >> > > + uuidset_count(&data->removed_multicast_groups)) { >> > > + return EN_UPDATED; >> > > + } >> > > + >> > > + return EN_UNCHANGED; >> > > +} >> > > + >> > > +static enum engine_input_handler_result >> > > +evpn_vtep_binding_ovs_interface_handler(struct engine_node *node, >> > > + void *data OVS_UNUSED) >> > > +{ >> > > + const struct ovsrec_interface_table *iface_table = >> > > + EN_OVSDB_GET(engine_get_input("OVS_interface", node)); >> > > + >> > > + const struct ovsrec_interface *iface; >> > > + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface, iface_table) { >> > > + if (!ovn_is_evpn_tunnel_interface(iface)) { >> > > + continue; >> > > + } >> > > + >> > > + if (ovsrec_interface_is_new(iface) || >> > > + ovsrec_interface_is_deleted(iface) || >> > > + ovsrec_interface_is_updated(iface, >> > OVSREC_INTERFACE_COL_OFPORT)) { >> > nit: I would at least add a comment on why handling ovs interface option > change (e.g. handling setting "flow" on local_ip) > is not needed. I think that it is not needed as (among other potential > recomputes) ovs interface changes for vxlan > port w/o iface-id would result in runtime_data recompute (we might also > wonder whether runtime_data recompute > on vxlan changes is really needed, but this can be done in a > separate/later optimization patch). > Right, but this won't actually be needed in v3 because ovn-controller will create the tunnel so the interface option will be stable. > > > + return EN_UNHANDLED; >> > > + } >> > > + } >> > > + >> > > + return EN_HANDLED_UNCHANGED; >> > > +} >> > > + >> > > +static enum engine_input_handler_result >> > > +evpn_vtep_binding_datapath_binding_handler(struct engine_node *node, >> > > + void *data OVS_UNUSED) >> > > +{ >> > > + const struct sbrec_datapath_binding_table *dp_table = >> > > + EN_OVSDB_GET(engine_get_input("SB_datapath_binding", node)); >> > > + struct ed_type_runtime_data *rt_data = >> > > + engine_get_input_data("runtime_data", node); >> > > + >> > > + const struct sbrec_datapath_binding *dp; >> > > + SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_TRACKED (dp, dp_table) { >> > > + if (sbrec_datapath_binding_is_new(dp) || >> > > + sbrec_datapath_binding_is_deleted(dp)) { >> > > + /* The removal and addition is handled via the neighbor. >> */ >> > >> > Nit: maybe the more explicit "via the en_neighbor_exchange I-P node."? >> > >> > > + return EN_HANDLED_UNCHANGED; >> > > + } >> > > + >> > > + struct local_datapath *ld = >> > > + get_local_datapath(&rt_data->local_datapaths, >> > dp->tunnel_key); >> > > + if (!ld || !ld->is_switch) { >> > > + continue; >> > > + } >> > > + >> > > + int64_t vni = ovn_smap_get_llong(&dp->external_ids, >> > > + "dynamic-routing-vni", -1); >> > > + if (!ovn_is_valid_vni(vni)) { >> > > + continue; >> > > + } >> > > + >> > > + if (sbrec_datapath_binding_is_updated( >> > > + dp, SBREC_DATAPATH_BINDING_COL_TUNNEL_KEY)) { >> > > + return EN_UNHANDLED; >> > > + } >> > > + } >> > > + >> > > + return EN_HANDLED_UNCHANGED; >> > > +} >> > > + >> > > /* Returns false if the northd internal version stored in SB_Global >> > > * and ovn-controller internal version don't match. >> > > */ >> > > @@ -6257,6 +6413,7 @@ main(int argc, char *argv[]) >> > > ENGINE_NODE(neighbor_table_notify); >> > > ENGINE_NODE(neighbor_exchange); >> > > ENGINE_NODE(neighbor_exchange_status); >> > > + ENGINE_NODE(evpn_vtep_binding, CLEAR_TRACKED_DATA); >> > > >> > > #define SB_NODE(NAME) ENGINE_NODE_SB(NAME); >> > > SB_NODES >> > > @@ -6492,6 +6649,22 @@ main(int argc, char *argv[]) >> > > engine_add_input(&en_neighbor_exchange, >> > &en_neighbor_exchange_status, >> > > NULL); >> > > >> > > + engine_add_input(&en_evpn_vtep_binding, &en_ovs_open_vswitch, >> NULL); >> > > + engine_add_input(&en_evpn_vtep_binding, &en_ovs_bridge, NULL); >> > > + engine_add_input(&en_evpn_vtep_binding, &en_neighbor_exchange, >> > NULL); >> > > + /* The runtime_data are needed only for local datapaths, any >> update >> > of >> > > + * local datapath will be reflected via en_neighbor_exchange. */ >> > > + engine_add_input(&en_evpn_vtep_binding, &en_runtime_data, >> > > + engine_noop_handler); >> > > + engine_add_input(&en_evpn_vtep_binding, &en_ovs_interface, >> > > + evpn_vtep_binding_ovs_interface_handler); >> > > + engine_add_input(&en_evpn_vtep_binding, &en_sb_datapath_binding, >> > > + evpn_vtep_binding_datapath_binding_handler); >> > > + >> > > + /* TODO Add real handler to process all bindings. */ >> > > + engine_add_input(&en_pflow_output, &en_evpn_vtep_binding, >> > > + engine_noop_handler); >> > > + >> > > engine_add_input(&en_controller_output, &en_dns_cache, >> > > NULL); >> > > engine_add_input(&en_controller_output, &en_lflow_output, >> > > @@ -6511,9 +6684,6 @@ main(int argc, char *argv[]) >> > > engine_add_input(&en_controller_output, &en_acl_id, >> > > controller_output_acl_id_handler); >> > > >> > > - engine_add_input(&en_controller_output, &en_neighbor_exchange, >> > > - controller_output_neighbor_exchange_handler); >> > > - >> > > struct engine_arg engine_arg = { >> > > .sb_idl = ovnsb_idl_loop.idl, >> > > .ovs_idl = ovs_idl_loop.idl, >> > > @@ -6573,6 +6743,8 @@ main(int argc, char *argv[]) >> > > engine_get_internal_data(&en_mac_cache); >> > > struct ed_type_neighbor_exchange *ne_data = >> > > engine_get_internal_data(&en_neighbor_exchange); >> > > + struct ed_type_evpn_vtep_binding *eb_data = >> > > + engine_get_internal_data(&en_evpn_vtep_binding); >> > > >> > > ofctrl_init(&lflow_output_data->group_table, >> > > &lflow_output_data->meter_table); >> > > @@ -6592,6 +6764,12 @@ main(int argc, char *argv[]) >> > > unixctl_command_register("evpn/remote-vtep-list", "", 0, 0, >> > > evpn_remote_vtep_list, >> > > &ne_data->remote_vteps); >> > > + unixctl_command_register("evpn/vtep-binding-list", "", 0, 0, >> > > + evpn_vtep_binding_list, >> > > + &eb_data->bindings); >> > > + unixctl_command_register("evpn/vtep-multicast-group-list", "", >> 0, 0, >> > > + evpn_multicast_group_list, >> > > + &eb_data->multicast_groups); >> > > >> > > struct pending_pkt pending_pkt = { .conn = NULL }; >> > > unixctl_command_register("inject-pkt", "MICROFLOW", 1, 1, >> > inject_pkt, >> > > diff --git a/lib/ovn-util.c b/lib/ovn-util.c >> > > index c12de4da9..e8a32c96a 100644 >> > > --- a/lib/ovn-util.c >> > > +++ b/lib/ovn-util.c >> > > @@ -1490,3 +1490,19 @@ ovn_is_valid_vni(int64_t vni) >> > > { >> > > return vni >= 0 && (vni <= (1 << 24) - 1); >> > > } >> > > + >> > > +bool >> > > +ovn_is_evpn_tunnel_interface(const struct ovsrec_interface *iface) >> > > +{ >> > > + if (strcmp(iface->type, "vxlan")) { >> > > + return false; >> > > + } >> > > + >> > > + if (strcmp(smap_get_def(&iface->options, "local_ip", ""), >> "flow") || >> > > + strcmp(smap_get_def(&iface->options, "remote_ip", ""), >> "flow") >> > || >> > > + strcmp(smap_get_def(&iface->options, "key", ""), "flow")) { >> > > + return false; >> > > + } >> > > + >> > > + return true; >> > > +} >> > > diff --git a/lib/ovn-util.h b/lib/ovn-util.h >> > > index f31eba344..1bdd98675 100644 >> > > --- a/lib/ovn-util.h >> > > +++ b/lib/ovn-util.h >> > > @@ -49,6 +49,7 @@ struct svec; >> > > struct uuid; >> > > struct unixctl_conn; >> > > struct rconn; >> > > +struct ovsrec_interface; >> > > >> > > struct ipv4_netaddr { >> > > ovs_be32 addr; /* 192.168.10.123 */ >> > > @@ -167,6 +168,9 @@ void set_idl_probe_interval(struct ovsdb_idl *idl, >> > const char *remote, >> > > #define OVN_MIN_DP_VXLAN_KEY_GLOBAL (OVN_MAX_DP_VXLAN_KEY_LOCAL + 1) >> > > #define OVN_MAX_DP_VXLAN_KEY_GLOBAL ((1u << 12) - 1) >> > > >> > > +#define OVN_MIN_EVPN_KEY (1u << 31) >> > > +#define OVN_MAX_EVPN_KEY (OVN_MAX_DP_GLOBAL_NUM | OVN_MIN_EVPN_KEY) >> > > + >> > > struct hmap; >> > > void ovn_destroy_tnlids(struct hmap *tnlids); >> > > bool ovn_add_tnlid(struct hmap *set, uint32_t tnlid); >> > > @@ -501,6 +505,7 @@ bool find_prefix_in_set(const struct in6_addr >> > *prefix, unsigned int plen, >> > > void ovn_debug_commands_register(void); >> > > >> > > bool ovn_is_valid_vni(int64_t vni); >> > > +bool ovn_is_evpn_tunnel_interface(const struct ovsrec_interface >> *iface); >> > > >> > > const struct sbrec_port_binding *lport_lookup_by_name( >> > > struct ovsdb_idl_index *sbrec_port_binding_by_name, >> > >> > Regards, >> > Dumitru >> > >> > >> Everything else should be addressed in v2. >> >> Thanks, >> Ales >> _______________________________________________ >> dev mailing list >> d...@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Thanks, Ales _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
