On Thu, Aug 14, 2025 at 2:34 PM Ilya Maximets <i.maxim...@ovn.org> wrote:
> On 8/12/25 4:56 PM, Ales Musil via dev wrote:
> > Add support for a new option in local OvS database
> > "external_ids:ovn-evpn-vxlan-ports" which is comma separated list
> > of VXLAN destination ports. Create VXLAN tunnel for each port
> > in the list which can then be used by OVN for EVPN traffic.
> >
> > Signed-off-by: Ales Musil <amu...@redhat.com>
> > ---
> > v3: Newly added commit.
> > ---
>
Hi Ilya,
thank you for the review.
> > NEWS | 3 +
> > controller/chassis.c | 19 ++++++
> > controller/encaps.c | 115 +++++++++++++++++++++++++++++++-
> > controller/ovn-controller.8.xml | 20 ++++++
> > lib/ovn-util.c | 14 ++++
> > lib/ovn-util.h | 1 +
> > tests/ovn-controller.at | 57 ++++++++++++++++
> > 7 files changed, 228 insertions(+), 1 deletion(-)
> >
> > diff --git a/NEWS b/NEWS
> > index 54d676be8..ec52f0d90 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -46,6 +46,9 @@ Post v25.03.0
> > Routers and Logical Router Ports which refines the way in which
> > chassis-specific Advertised_Routes (e.g., for NAT and LB IPs) are
> > advertised.
> > + * Add the option "external_ids:ovn-evpn-vxlan-ports" into OvS
> datapath.
>
> This is worded strangely. s/into OvS datapath/for the Open_vSwitch
> database/.
> I'd also suggest using dashes, i.e. "external-ids" for the public-facing
> documentation, but also in tests.
>
Addressed in v4.
>
> > + This option allows CMS to set list of UDP destination ports that
> will be
> > + used to create the VXLAN tunnels.
> >
> > OVN v25.03.0 - 07 Mar 2025
> > --------------------------
> > diff --git a/controller/chassis.c b/controller/chassis.c
> > index 7616069b7..34adc98be 100644
> > --- a/controller/chassis.c
> > +++ b/controller/chassis.c
> > @@ -58,6 +58,7 @@ struct ovs_chassis_cfg {
> > const char *trim_limit_lflow_cache;
> > const char *trim_wmark_perc_lflow_cache;
> > const char *trim_timeout_ms;
> > + const char *evpn_vxlan_port;
> >
> > /* Set of encap types parsed from the 'ovn-encap-type' external-id.
> */
> > struct sset encap_type_set;
> > @@ -198,6 +199,13 @@ get_encap_csum(const struct smap *ext_ids, const
> char *chassis_id)
> > "ovn-encap-csum", "true");
> > }
> >
> > +static const char *
> > +get_evpn_vxlan_port(const struct smap *ext_ids, const char *chassis_id)
> > +{
> > + return get_chassis_external_id_value(ext_ids, chassis_id,
> > + "ovn-evpn-vxlan-ports", "");
> > +}
> > +
> > static const char *
> > get_datapath_type(const struct ovsrec_bridge *br_int)
> > {
> > @@ -332,6 +340,8 @@ chassis_parse_ovs_config(const struct
> ovsrec_open_vswitch_table *ovs_table,
> > get_trim_wmark_perc_lflow_cache(&cfg->external_ids, chassis_id);
> > ovs_cfg->trim_timeout_ms =
> > get_trim_timeout(&cfg->external_ids, chassis_id);
> > + ovs_cfg->evpn_vxlan_port =
> > + get_evpn_vxlan_port(&cfg->external_ids, chassis_id);
> >
> > chassis_parse_ovs_encap_type(encap_type, &ovs_cfg->encap_type_set);
> >
> > @@ -384,6 +394,7 @@ chassis_build_other_config(const struct
> ovs_chassis_cfg *ovs_cfg,
> > smap_replace(config, "ovn-trim-timeout-ms",
> ovs_cfg->trim_timeout_ms);
> > smap_replace(config, "iface-types",
> ds_cstr_ro(&ovs_cfg->iface_types));
> > smap_replace(config, "ovn-chassis-mac-mappings",
> ovs_cfg->chassis_macs);
> > + smap_replace(config, "ovn-evpn-vxlan-ports",
> ovs_cfg->evpn_vxlan_port);
> > smap_replace(config, "is-interconn",
> > ovs_cfg->is_interconn ? "true" : "false");
> > smap_replace(config, OVN_FEATURE_PORT_UP_NOTIF, "true");
> > @@ -504,6 +515,13 @@ chassis_other_config_changed(const struct
> ovs_chassis_cfg *ovs_cfg,
> > return true;
> > }
> >
> > + const char *chassis_evpn_vxlan_port =
> > + get_evpn_vxlan_port(&chassis_rec->other_config,
> chassis_rec->name);
> > + if (strcmp(ovs_cfg->evpn_vxlan_port, chassis_evpn_vxlan_port)) {
> > + return true;
> > + }
> > +
> > +
> > if (!smap_get_bool(&chassis_rec->other_config,
> OVN_FEATURE_PORT_UP_NOTIF,
> > false)) {
> > return true;
> > @@ -722,6 +740,7 @@ update_supported_sset(struct sset *supported)
> > sset_add(supported, "iface-types");
> > sset_add(supported, "ovn-chassis-mac-mappings");
> > sset_add(supported, "is-interconn");
> > + sset_add(supported, "ovn-evpn-vxlan-ports");
> >
> > /* Internal options. */
> > sset_add(supported, "is-vtep");
> > diff --git a/controller/encaps.c b/controller/encaps.c
> > index b5ef66371..67f8c9c9c 100644
> > --- a/controller/encaps.c
> > +++ b/controller/encaps.c
> > @@ -64,6 +64,9 @@ struct tunnel_ctx {
> > * adding a new tunnel. */
> > struct sset port_names;
> >
> > + /* Contains 'struct ovsrec_port' by name if it's evpn tunnel. */
> > + struct shash evpn_tunnels;
> > +
> > struct ovsdb_idl_txn *ovs_txn;
> > const struct ovsrec_open_vswitch_table *ovs_table;
> > const struct ovsrec_bridge *br_int;
> > @@ -442,7 +445,10 @@ clear_old_tunnels(const struct ovsrec_bridge
> *old_br_int, const char *prefix,
> > for (size_t i = 0; i < old_br_int->n_ports; i++) {
> > const struct ovsrec_port *port = old_br_int->ports[i];
> > const char *id = smap_get(&port->external_ids, OVN_TUNNEL_ID);
> > - if (id && !strncmp(port->name, prefix, prefix_len)) {
> > + const bool evpn_tunnel =
> > + smap_get_bool(&port->external_ids, "ovn-evpn-tunnel",
> false);
> > + if (!strncmp(port->name, prefix, prefix_len) &&
> > + (id || evpn_tunnel)) {
> > VLOG_DBG("Clearing old tunnel port \"%s\" (%s) from bridge "
> > "\"%s\".", port->name, id, old_br_int->name);
> > ovsrec_bridge_update_ports_delvalue(old_br_int, port);
> > @@ -450,6 +456,97 @@ clear_old_tunnels(const struct ovsrec_bridge
> *old_br_int, const char *prefix,
> > }
> > }
> >
> > +static bool
> > +is_evpn_tunnel_port(const struct ovsrec_port *port, const char
> *dst_port)
> > +{
> > + if (!smap_get_bool(&port->external_ids, "ovn-evpn-tunnel", false)) {
> > + return false;
> > + }
> > +
> > + if (port->n_interfaces != 1) {
> > + return false;
> > + }
> > +
> > + const struct ovsrec_interface *iface = port->interfaces[0];
> > + 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") ||
> > + strcmp(smap_get_def(&iface->options, "dst_port", ""),
> dst_port)) {
> > + return false;
> > + }
> > +
> > + return true;
> > +}
> > +
> > +static void
> > +create_evpn_tunnels(struct tunnel_ctx *tc)
> > +{
> > + const char *evpn_vxlan_ports =
> > + smap_get(&tc->this_chassis->other_config,
> "ovn-evpn-vxlan-ports");
> > + if (!evpn_vxlan_ports) {
> > + return;
> > + }
> > +
> > + /* Create smap of common tunnel options. */
> > + struct smap options = SMAP_INITIALIZER(&options);
> > + smap_add(&options, "local_ip", "flow");
> > + smap_add(&options, "remote_ip", "flow");
> > + smap_add(&options, "key", "flow");
> > +
> > + struct sset vxlan_ports;
> > + sset_from_delimited_string(&vxlan_ports, evpn_vxlan_ports, ",");
> > + const char *idx = get_chassis_idx(tc->ovs_table);
> > +
> > + const char *vxlan_port;
> > + SSET_FOR_EACH (vxlan_port, &vxlan_ports) {
> > + unsigned short us;
> > + if (!ovn_str_to_ushort(vxlan_port, 10, &us) || !us) {
> > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
> 1);
> > + VLOG_WARN_RL(&rl, "Invalid VXLAN port number '%s'",
> vxlan_port);
> > + continue;
> > + }
> > +
> > + char *name = xasprintf("ovn%s-evpn-%s", idx, vxlan_port);
> > + const struct ovsrec_port *port =
> > + shash_find_and_delete(&tc->evpn_tunnels, name);
> > +
> > + if (!port) {
> > + port = ovsrec_port_insert(tc->ovs_txn);
> > + ovsrec_port_set_name(port, name);
> > +
> > + const struct smap id = SMAP_CONST1(&id, "ovn-evpn-tunnel",
> "true");
> > + ovsrec_port_set_external_ids(port, &id);
> > +
> > + ovsrec_bridge_update_ports_addvalue(tc->br_int, port);
> > + }
> > +
> > + if (!is_evpn_tunnel_port(port, vxlan_port)) {
> > + struct ovsrec_interface *iface =
> > + ovsrec_interface_insert(tc->ovs_txn);
> > + ovsrec_interface_set_name(iface, name);
> > + ovsrec_interface_set_type(iface, "vxlan");
> > +
> > + smap_replace(&options, "dst_port", vxlan_port);
> > + ovsrec_interface_set_options(iface, &options);
> > +
> > + const struct smap id = SMAP_CONST1(&id, "ovn-evpn-tunnel",
> "true");
> > + ovsrec_port_set_external_ids(port, &id);
> > +
> > + ovsrec_port_set_interfaces(port, &iface, 1);
> > + }
> > +
> > + free(name);
> > + }
> > +
> > + smap_destroy(&options);
> > + sset_destroy(&vxlan_ports);
> > +}
> > +
> > +
> > void
> > encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
> > const struct ovsrec_bridge *br_int,
> > @@ -498,6 +595,7 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
> > struct tunnel_ctx tc = {
> > .tunnel = SHASH_INITIALIZER(&tc.tunnel),
> > .port_names = SSET_INITIALIZER(&tc.port_names),
> > + .evpn_tunnels = SHASH_INITIALIZER(&tc.evpn_tunnels),
> > .br_int = br_int,
> > .this_chassis = this_chassis,
> > .ovs_table = ovs_table,
> > @@ -528,6 +626,10 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
> > ovsrec_bridge_update_ports_delvalue(br_int, port);
> > }
> > }
> > +
> > + if (smap_get_bool(&port->external_ids, "ovn-evpn-tunnel",
> false)) {
> > + shash_add(&tc.evpn_tunnels, port->name, port);
> > + }
> > }
> >
> > SBREC_CHASSIS_TABLE_FOR_EACH (chassis_rec, chassis_table) {
> > @@ -558,6 +660,8 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
> > }
> > }
> >
> > + create_evpn_tunnels(&tc);
> > +
> > /* Delete any existing OVN tunnels that were not still around. */
> > struct shash_node *node;
> > SHASH_FOR_EACH_SAFE (node, &tc.tunnel) {
> > @@ -566,8 +670,17 @@ encaps_run(struct ovsdb_idl_txn *ovs_idl_txn,
> > shash_delete(&tc.tunnel, node);
> > free(tunnel);
> > }
> > +
> > + /* Delete any stale EVPN tunnels. */
> > + SHASH_FOR_EACH_SAFE (node, &tc.evpn_tunnels) {
> > + const struct ovsrec_port *port = node->data;
> > + ovsrec_bridge_update_ports_delvalue(br_int, port);
> > + shash_delete(&tc.evpn_tunnels, node);
> > + }
> > +
> > shash_destroy(&tc.tunnel);
> > sset_destroy(&tc.port_names);
> > + shash_destroy(&tc.evpn_tunnels);
> > }
> >
> > /* Returns true if the database is all cleaned up, false if more work is
> > diff --git a/controller/ovn-controller.8.xml
> b/controller/ovn-controller.8.xml
> > index 3f2cf5bae..e602ebca8 100644
> > --- a/controller/ovn-controller.8.xml
> > +++ b/controller/ovn-controller.8.xml
> > @@ -392,6 +392,13 @@
> > exit. In order to keep backward compatibility the
> > <code>--restart</code> exit flag has priority over this flag.
> > </dd>
> > +
> > + <dt><code>external_ids:ovn-evpn-vxlan-ports</code></dt>
> > + <dd>
> > + Comma separated list of UDP ports used as a destination port for
> > + the EVPN tunnels created by this ovn-controller. NOTE: this
> feature is
> > + experimental and may be subject to removal/change in the future.
> > + </dd>
> > </dl>
> >
> > <p>
> > @@ -626,6 +633,19 @@
> > <code>external_ids:ovn-installed-ts</code>.
> > </p>
> > </dd>
> > +
> > + <dt>
> > + <code>external_ids:ovn-evpn-tunnel</code> in the
> > + <code>Port</code> and <code>Interface</code> table
> > + </dt>
> > +
> > + <dd>
> > + <p>
> > + Presence of this key indicates that the
> > + <code>Port/Interface</code> was created by ovn-controller to
> > + be used as tunnel port for EVPN traffic.
> > + </p>
> > + </dd>
> > </dl>
> >
> > <h1>OVN Southbound Database Usage</h1>
> > diff --git a/lib/ovn-util.c b/lib/ovn-util.c
> > index c2a73db9b..70a1c9404 100644
> > --- a/lib/ovn-util.c
> > +++ b/lib/ovn-util.c
> > @@ -865,6 +865,20 @@ ovn_smap_get_llong(const struct smap *smap, const
> char *key, long long def)
> > return ll_value;
> > }
> >
> > +bool
> > +ovn_str_to_ushort(const char *s, int base, unsigned short *u)
>
> I wonder why we can't use well-sized types and the str_to_u16.
> 'short' can be larger than 16 bits, while the port number can't.
>
It doesn't seem right to use a function that is in ofp-parse for this.
I have added an upper limit check though.
> > +{
> > + long long ll;
> > + bool ok = str_to_llong(s, base, &ll);
> > + if (!ok || ll < 0 || ll > USHRT_MAX) {
> > + *u = 0;
> > + return false;
> > + } else {
> > + *u = ll;
> > + return true;
> > + }
> > +}
> > +
> > /* For a 'key' of the form "IP:port" or just "IP", sets 'port',
> > * 'ip_address' and 'ip' ('struct in6_addr' IPv6 or IPv4 mapped
> address).
> > * The caller must free() the memory allocated for 'ip_address'.
> > diff --git a/lib/ovn-util.h b/lib/ovn-util.h
> > index 77dd0ffa4..e8c2cedc5 100644
> > --- a/lib/ovn-util.h
> > +++ b/lib/ovn-util.h
> > @@ -221,6 +221,7 @@ char *str_tolower(const char *orig);
> >
> > long long ovn_smap_get_llong(const struct smap *smap, const char *key,
> > long long def);
> > +bool ovn_str_to_ushort(const char *, int base, unsigned short *);
> >
> > /* OVN daemon options. Taken from ovs/lib/daemon.h. */
> > #define OVN_DAEMON_OPTION_ENUMS \
> > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> > index afdc76a32..d3e317b9c 100644
> > --- a/tests/ovn-controller.at
> > +++ b/tests/ovn-controller.at
> > @@ -3799,3 +3799,60 @@ check ovn-nbctl --wait=hv sync
> >
> > OVN_CLEANUP([hv1])
> > AT_CLEANUP
> > +
> > +AT_SETUP([ovn-controller - EVPN tunnel])
> > +ovn_start
> > +
> > +net_add n1
> > +sim_add hv1
> > +as hv1
> > +check ovs-vsctl add-br br-phys
> > +ovn_attach n1 br-phys 192.168.0.11 24 geneve,vxlan
> > +check ovn-nbctl --wait=hv sync
> > +
> > +wait_ports() {
> > + local n_ports=$1
> > +
> > + OVS_WAIT_UNTIL([
> > + test "$(ovs-vsctl --format=table --no-headings find port
> external_ids:ovn-evpn-tunnel="true" | wc -l)" = "$n_ports"
> > + ])
> > +}
> > +
> > +check_tunnel_port() {
> > + local port=$1
> > +
> > + local tunnel_id=$(ovs-vsctl --bare --columns _uuid find port
> name="ovn-evpn-$port")
> > + AT_CHECK([ovs-vsctl --bare --columns ports find bridge
> name="br-int" | grep -q "$tunnel_id"])
> > +}
> > +
> > +check ovs-vsctl set Open_vSwitch .
> external_ids:ovn-evpn-vxlan-ports="4789"
> > +wait_ports 1
> > +check_tunnel_port 4789
> > +
> > +check ovs-vsctl set Open_vSwitch .
> external_ids:ovn-evpn-vxlan-ports="4789,4790"
> > +wait_ports 2
> > +check_tunnel_port 4789
> > +check_tunnel_port 4790
> > +
> > +# Stop ovn-controller
> > +OVN_CONTROLLER_EXIT([hv1],[])
> > +start_daemon ovn-controller
> > +check ovn-nbctl --wait=hv sync
> > +
> > +wait_ports 2
> > +check_tunnel_port 4789
> > +check_tunnel_port 4790
> > +
> > +check ovs-vsctl set Open_vSwitch .
> external_ids:ovn-evpn-vxlan-ports="4790"
> > +wait_ports 1
> > +check_tunnel_port 4790
> > +
> > +ovs-vsctl remove Open_vSwitch . external_ids ovn-evpn-vxlan-ports
> > +wait_ports 0
> > +
> > +check ovs-vsctl set Open_vSwitch .
> external_ids:ovn-evpn-vxlan-ports="wrong"
> > +wait_ports 0
> > +
> > +OVN_CLEANUP([hv1
> > +/Invalid VXLAN port number.*/d])
> > +AT_CLEANUP
>
>
Thanks,
Ales
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
