Hi Ales, Dumitru Thanks for the patch I only have one nit below Acked-by: Xavier Simonart <xsimo...@redhat.com> Thanks Xavier
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). > > > + 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 > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev