On 10/16/25 6:51 PM, Lorenzo Bianconi wrote:
> Introduce the capability to ovn-controller to announce local ips/macs
> bindings via EVPN route type2 messages in the EVPN domain.
>
> Signed-off-by: Lorenzo Bianconi <[email protected]>
> ---
Hi Lorenzo,
Thanks for the patch!
> controller/neighbor.c | 62 ++++++++++++++++++++++++++++++++++---
> controller/neighbor.h | 2 ++
> controller/ovn-controller.c | 23 +++++++++++++-
> tests/system-ovn.at | 34 ++++++++++++++++++++
> 4 files changed, 115 insertions(+), 6 deletions(-)
>
> diff --git a/controller/neighbor.c b/controller/neighbor.c
> index 1870159bc..fadec7044 100644
> --- a/controller/neighbor.c
> +++ b/controller/neighbor.c
> @@ -20,6 +20,7 @@
> #include "lib/sset.h"
> #include "local_data.h"
> #include "lport.h"
> +#include "openvswitch/ofp-parse.h"
> #include "ovn-sb-idl.h"
>
> #include "neighbor.h"
> @@ -42,6 +43,10 @@ neighbor_interface_monitor_alloc(enum neighbor_family
> family,
> static void neighbor_collect_mac_to_advertise(
> const struct neighbor_ctx_in *, struct hmap *neighbors,
> struct sset *advertised_pbs, const struct sbrec_datapath_binding *);
> +static void neighbor_collect_route_type2_to_advertise(
As mentioned in patch 2/4, I think "type2" might be too specific. Maybe
just "*_collect_ip_mac_to_advertise()"?
> + const struct neighbor_ctx_in *,
> + struct hmap *neighbors_v4, struct hmap *neighbors_v6,
> + struct sset *advertised_pbs, const struct sbrec_datapath_binding *);
> static const struct sbrec_port_binding *neighbor_get_relevant_port_binding(
> struct ovsdb_idl_index *sbrec_pb_by_name,
> const struct sbrec_port_binding *);
> @@ -114,13 +119,20 @@ neighbor_run(struct neighbor_ctx_in *n_ctx_in,
>
> const char *redistribute = smap_get(&ld->datapath->external_ids,
> "dynamic-routing-redistribute");
> - if (!redistribute || strcmp(redistribute, "fdb")) {
> - continue;
> + if (redistribute && !strcmp(redistribute, "fdb")) {
> + neighbor_collect_mac_to_advertise(n_ctx_in,
> + &lo->announced_neighbors,
> + n_ctx_out->advertised_pbs,
> + ld->datapath);
> + }
> + if (redistribute && !strcmp(redistribute, "ip")) {
> + neighbor_collect_route_type2_to_advertise(n_ctx_in,
> + &br_v4->announced_neighbors,
> + &br_v6->announced_neighbors,
> + n_ctx_out->advertised_pbs,
> + ld->datapath);
> }
>
> - neighbor_collect_mac_to_advertise(n_ctx_in, &lo->announced_neighbors,
> - n_ctx_out->advertised_pbs,
> - ld->datapath);
> }
> }
>
> @@ -236,6 +248,46 @@ neighbor_collect_mac_to_advertise(const struct
> neighbor_ctx_in *n_ctx_in,
> sbrec_port_binding_index_destroy_row(target);
> }
>
> +static void
> +neighbor_collect_route_type2_to_advertise(
> + const struct neighbor_ctx_in *n_ctx_in,
> + struct hmap *neighbors_v4,
> + struct hmap *neighbors_v6,
> + struct sset *advertised_pbs,
> + const struct sbrec_datapath_binding *dp)
> +{
> + struct sbrec_advertised_mac_binding *target =
> + sbrec_advertised_mac_binding_index_init_row(
> + n_ctx_in->sbrec_amb_by_dp);
> + sbrec_advertised_mac_binding_index_set_datapath(target, dp);
> +
> + const struct sbrec_advertised_mac_binding *adv_mb;
> + SBREC_ADVERTISED_MAC_BINDING_FOR_EACH_EQUAL (adv_mb, target,
> + n_ctx_in->sbrec_amb_by_dp) {
We need a similar check to what we do in
neighbor_collect_mac_to_advertise(). We only want to advertise the
mac+ip binding if the port is local. That's because we don't want to
generate suboptimal routing in the fabric. The MAC+IP will be
advertised only on the chassis where the MAC+IP actually resides. So
maybe something like:
if (!lport_pb_is_chassis_resident(n_ctx_in->chassis,
adv_mb->logical_port)) {
continue;
}
We might have to adjust the check for distributed patch ports though.
> + struct in6_addr route;
s/route/ip/
> + unsigned int plen;
> + if (!ip46_parse_cidr(adv_mb->ip, &route, &plen)) {
It's not a route, it's an IP. ip46_parse() is good enough.
> + continue;
> + }
> +
> + struct eth_addr ea;
> + char *err = str_to_mac(adv_mb->mac, &ea);
> + if (err) {
> + free(err);
> + continue;
> + }
> +
> + struct hmap *neighbors = IN6_IS_ADDR_V4MAPPED(&route)
> + ? neighbors_v4 : neighbors_v6;
Nit: according to our coding style guidelines, this should be indented
differently:
struct hmap *neighbors = IN6_IS_ADDR_V4MAPPED(&route)
? neighbors_v4 : neighbors_v6;
> + if (!advertise_neigh_find(neighbors, ea, &route)) {
> + advertise_neigh_add(neighbors, ea, route);
> + }
> + sset_add(advertised_pbs, adv_mb->logical_port->logical_port);
> + }
> +
> + sbrec_advertised_mac_binding_index_destroy_row(target);
> +}
> +
> static const struct sbrec_port_binding *
> neighbor_get_relevant_port_binding(struct ovsdb_idl_index *sbrec_pb_by_name,
> const struct sbrec_port_binding *pb)
> diff --git a/controller/neighbor.h b/controller/neighbor.h
> index ce73dd4c4..e3adc87d1 100644
> --- a/controller/neighbor.h
> +++ b/controller/neighbor.h
> @@ -44,6 +44,8 @@ struct neighbor_ctx_in {
> const struct hmap *local_datapaths;
> /* Index for Port Binding by Datapath. */
> struct ovsdb_idl_index *sbrec_pb_by_dp;
> + /* Index for Advertised_Mac_Binding by Datapath. */
> + struct ovsdb_idl_index *sbrec_amb_by_dp;
> /* Index for Port Binding by name. */
> struct ovsdb_idl_index *sbrec_pb_by_name;
> const struct sbrec_chassis *chassis;
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 6fbf3a227..ab6aba0e4 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -267,6 +267,7 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
> struct ovsdb_idl_condition nh = OVSDB_IDL_CONDITION_INIT(&nh);
> struct ovsdb_idl_condition ar = OVSDB_IDL_CONDITION_INIT(&ar);
> struct ovsdb_idl_condition lr = OVSDB_IDL_CONDITION_INIT(&lr);
> + struct ovsdb_idl_condition amb = OVSDB_IDL_CONDITION_INIT(&amb);
>
> /* Always monitor all logical datapath groups. Otherwise, DPG updates may
> * be received *after* the lflows using it are seen by ovn-controller.
> @@ -301,6 +302,7 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
> ovsdb_idl_condition_add_clause_true(&tv);
> ovsdb_idl_condition_add_clause_true(&nh);
> ovsdb_idl_condition_add_clause_true(&ar);
> + ovsdb_idl_condition_add_clause_true(&amb);
> goto out;
> }
>
> @@ -400,6 +402,8 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
> uuid);
> sbrec_ecmp_nexthop_add_clause_datapath(&nh, OVSDB_F_EQ, uuid);
> sbrec_advertised_route_add_clause_datapath(&ar, OVSDB_F_EQ,
> uuid);
> + sbrec_advertised_mac_binding_add_clause_datapath(&amb,
> OVSDB_F_EQ,
> + uuid);
> }
>
> /* Datapath groups are immutable, which means a new group record is
> @@ -430,6 +434,8 @@ out:;
> sb_table_set_opt_mon_condition(ovnsb_idl, ecmp_nexthop, &nh),
> sb_table_set_opt_mon_condition(ovnsb_idl, advertised_route, &ar),
> sb_table_set_opt_mon_condition(ovnsb_idl, learned_route, &lr),
> + sb_table_set_opt_mon_condition(ovnsb_idl, advertised_mac_binding,
> + &amb),
> };
>
> unsigned int expected_cond_seqno = 0;
> @@ -452,6 +458,7 @@ out:;
> ovsdb_idl_condition_destroy(&nh);
> ovsdb_idl_condition_destroy(&ar);
> ovsdb_idl_condition_destroy(&lr);
> + ovsdb_idl_condition_destroy(&amb);
> return expected_cond_seqno;
> }
>
> @@ -1014,7 +1021,8 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> SB_NODE(chassis_template_var) \
> SB_NODE(acl_id) \
> SB_NODE(advertised_route) \
> - SB_NODE(learned_route)
> + SB_NODE(learned_route) \
> + SB_NODE(advertised_mac_binding)
>
> enum sb_engine_node {
> #define SB_NODE(NAME) SB_##NAME,
> @@ -5872,6 +5880,10 @@ en_neighbor_run(struct engine_node *node OVS_UNUSED,
> void *data)
> engine_ovsdb_node_get_index(
> engine_get_input("SB_chassis", node),
> "name");
> + struct ovsdb_idl_index *sbrec_advertised_mac_binding_by_datapath =
> + engine_ovsdb_node_get_index(
> + engine_get_input("SB_advertised_mac_binding", node),
> + "datapath");
>
> const char *chassis_id = get_ovs_chassis_id(ovs_table);
> ovs_assert(chassis_id);
> @@ -5882,6 +5894,7 @@ en_neighbor_run(struct engine_node *node OVS_UNUSED,
> void *data)
> struct neighbor_ctx_in n_ctx_in = {
> .local_datapaths = &rt_data->local_datapaths,
> .sbrec_pb_by_dp = sbrec_port_binding_by_datapath,
> + .sbrec_amb_by_dp = sbrec_advertised_mac_binding_by_datapath,
> .sbrec_pb_by_name = sbrec_port_binding_by_name,
> .chassis = chassis,
> };
> @@ -6684,6 +6697,9 @@ main(int argc, char *argv[])
> struct ovsdb_idl_index *sbrec_learned_route_index_by_datapath
> = ovsdb_idl_index_create1(ovnsb_idl_loop.idl,
> &sbrec_learned_route_col_datapath);
> + struct ovsdb_idl_index *sbrec_advertised_mac_binding_index_by_dp
> + = ovsdb_idl_index_create1(ovnsb_idl_loop.idl,
> +
> &sbrec_advertised_mac_binding_col_datapath);
>
> ovsdb_idl_track_add_all(ovnsb_idl_loop.idl);
> ovsdb_idl_omit_alert(ovnsb_idl_loop.idl,
> @@ -6718,6 +6734,8 @@ main(int argc, char *argv[])
> &sbrec_advertised_route_col_external_ids);
> ovsdb_idl_omit(ovnsb_idl_loop.idl,
> &sbrec_learned_route_col_external_ids);
> + ovsdb_idl_omit(ovnsb_idl_loop.idl,
> + &sbrec_advertised_mac_binding_col_external_ids);
>
> /* We don't want to monitor Connection table at all. So omit all the
> * columns. */
> @@ -7019,6 +7037,7 @@ main(int argc, char *argv[])
>
> engine_add_input(&en_neighbor, &en_ovs_open_vswitch, NULL);
> engine_add_input(&en_neighbor, &en_sb_chassis, NULL);
> + engine_add_input(&en_neighbor, &en_sb_advertised_mac_binding, NULL);
> engine_add_input(&en_neighbor, &en_runtime_data,
> neighbor_runtime_data_handler);
> engine_add_input(&en_neighbor, &en_sb_datapath_binding,
> @@ -7108,6 +7127,8 @@ main(int argc, char *argv[])
> sbrec_chassis_template_var_index_by_chassis);
> engine_ovsdb_node_add_index(&en_sb_learned_route, "datapath",
> sbrec_learned_route_index_by_datapath);
> + engine_ovsdb_node_add_index(&en_sb_advertised_mac_binding, "datapath",
> + sbrec_advertised_mac_binding_index_by_dp);
> engine_ovsdb_node_add_index(&en_sb_mac_binding, "lport_ip",
> sbrec_mac_binding_by_lport_ip);
> engine_ovsdb_node_add_index(&en_ovs_flow_sample_collector_set, "id",
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 973d2404e..99acae128 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -18759,6 +18759,40 @@ AT_CHECK_UNQUOTED([ovs-ofctl dump-flows br-int
> table=OFTABLE_MAC_LOOKUP | grep p
> awk '{print $7, $8}' | sort], [0], [dnl
> ])
>
> +AS_BOX([L2 EVPN ARP advertising])
> +# Add a router connected to the EVPN logical switch.
> +check ovn-nbctl --wait=hv \
> + -- lr-add lr \
> + -- lrp-add lr lr-ls-evpn f0:00:0f:16:01:01 172.16.1.1/24
> +
> +ls_evpn_uuid=$(fetch_column Datapath_Binding _uuid external_ids:name=ls-evpn)
> +check ovn-nbctl --wait=hv set logical_switch ls-evpn
> other_config:dynamic-routing-redistribute=ip
> +wait_row_count Advertised_Mac_Binding 1 ip=172.16.1.10/32
> mac='f0\:00\:0f\:16\:01\:10' datapath=$ls_evpn_uuid
> +wait_row_count Advertised_Mac_Binding 1 ip=172.16.1.1/32
> mac='f0\:00\:0f\:16\:01\:01' datapath=$ls_evpn_uuid
I think this can be check_row_count.
> +
> +AT_CHECK_UNQUOTED([ip neigh show dev br-10 nud noarp | grep -q '172.16.1.10
> lladdr f0:00:0f:16:01:10 NOARP'])
> +AT_CHECK_UNQUOTED([ip neigh show dev br-10 nud noarp | grep -q '172.16.1.1
> lladdr f0:00:0f:16:01:01 NOARP'])
> +
> +check ovn-nbctl --wait=hv lsp-add ls-evpn workload2 \
> + -- lsp-set-addresses workload2 "f0:00:0f:16:01:20 172.16.1.20"
> +
> +wait_row_count Advertised_Mac_Binding 1 ip=172.16.1.20/32
> mac='f0\:00\:0f\:16\:01\:20' datapath=$ls_evpn_uuid
This too.
> +AT_CHECK_UNQUOTED([ip neigh show dev br-10 nud noarp | grep -q '172.16.1.20
> lladdr f0:00:0f:16:01:20 NOARP'])
> +
> +check ovn-nbctl --wait=hv lsp-del workload2
> +AT_CHECK_UNQUOTED([ip neigh show dev br-10 nud noarp | grep -q '172.16.1.20
> lladdr f0:00:0f:16:01:20 NOARP'], [1])
> +
> +check ovn-nbctl --wait=hv lsp-del workload1
> +wait_row_count Advertised_Mac_Binding 1
This too?
> +AT_CHECK_UNQUOTED([ip neigh show dev br-10 nud noarp | grep -q '172.16.1.10
> lladdr f0:00:0f:16:01:10 NOARP'], [1])
> +
> +check ovn-nbctl --wait=hv lrp-del lr-ls-evpn
> +wait_row_count Advertised_Mac_Binding 0
> +AT_CHECK_UNQUOTED([ip neigh show dev br-10 nud noarp | grep -q '172.16.1.1
> lladdr f0:00:0f:16:01:01 NOARP'], [1])
> +
> +check ovn-nbctl --wait=hv ls-del ls-evpn
> +check ovn-nbctl --wait=hv lr-del lr
> +
> OVN_CLEANUP_CONTROLLER([hv1])
>
> as ovn-sb
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
