On 10/22/25 4:49 PM, Lorenzo Bianconi wrote:
> Collect vif and router ports IPs/MAC bindings in SB Advertised_Mac_Binding
> table in
> order to announce them as EVPN Type2 Route messages.
>
> Reported-at: https://issues.redhat.com/browse/FDP-2019
> Signed-off-by: Lorenzo Bianconi <[email protected]>
> ---
Hi Lorenzo,
Thanks for v2!
> NEWS | 3 +
> northd/en-advertised-route-sync.c | 200 ++++++++++++++++++++++++++++++
> northd/en-advertised-route-sync.h | 6 +
> northd/inc-proc-northd.c | 12 +-
> northd/ovn-northd.c | 4 +
> ovn-nb.xml | 6 +
> ovn-sb.ovsschema | 22 +++-
> ovn-sb.xml | 30 +++++
> tests/ovn-northd.at | 38 ++++++
> 9 files changed, 317 insertions(+), 4 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index fb4d63194..77a329669 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -14,6 +14,9 @@ Post v25.09.0
> Switches. If set to "true" ovn-controller will give preference to SB
> (Static_)MAC_Bindings of adjacent logical routers over ARPs learned
> through EVPN on the switch.
> + * Add the "other_config:dynamic-routing-redistribute=ip" to Logical
> + Switches to announce local IPs/MAC bindings belonging to the same
> + EVPN domain.
> - Add support for Network Function insertion in OVN with stateful traffic
> redirection capability in Logical Switch datapath. The feature
> introduces
> three new NB database tables:
> diff --git a/northd/en-advertised-route-sync.c
> b/northd/en-advertised-route-sync.c
> index 423baaacf..7b89e0351 100644
> --- a/northd/en-advertised-route-sync.c
> +++ b/northd/en-advertised-route-sync.c
> @@ -834,3 +834,203 @@ advertised_route_table_sync(
> hmap_destroy(&sync_routes);
> }
>
> +struct advertised_mac_binding {
> + struct hmap_node hmap_node;
> +
> + const struct sbrec_port_binding *sb;
> + const struct sbrec_datapath_binding *dp;
Nit: I'd move this one line higher. Then fields will be in the same
order as in the SB schema and will also be in reverse x-mas tree order.
> +
> + char *ip;
> + char *mac;
> +};
> +
> +static bool
> +evpn_ip_redistribution_enabled(const struct ovn_datapath *od)
> +{
> + if (!od->has_evpn_vni) {
> + return false;
> + }
> +
> + const char *redistribute = smap_get(&od->nbs->other_config,
> + "dynamic-routing-redistribute");
> + return redistribute && !strcmp(redistribute, "ip");
> +}
> +
> +static uint32_t
> +advertised_mac_binding_get_hash(const struct sbrec_port_binding *sb,
> + const struct sbrec_datapath_binding *dp,
Same nit about order of arguments, dp first.
> + const char *ip, const char *mac)
> +{
> + uint32_t hash = uuid_hash(&dp->header_.uuid);
> + hash = hash_string(sb->logical_port, hash);
> + hash = hash_string(ip, hash);
> + hash = hash_string(mac, hash);
> +
> + return hash;
> +}
> +
> +static struct advertised_mac_binding *
> +advertised_mac_binding_entry_find(struct hmap *map,
> + const struct sbrec_port_binding *sb,
> + const struct sbrec_datapath_binding *dp,
Nit about order of arguments.
> + const char *ip, const char *mac)
> +{
> + uint32_t hash = advertised_mac_binding_get_hash(sb, dp, ip, mac);
> + struct advertised_mac_binding *e;
> + HMAP_FOR_EACH_WITH_HASH (e, hmap_node, hash, map) {
> + if (uuid_equals(&sb->header_.uuid, &e->sb->header_.uuid) &&
> + uuid_equals(&dp->header_.uuid, &e->dp->header_.uuid) &&
> + !strcmp(e->ip, ip) && !strcmp(e->mac, mac)) {
> + return e;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +static void
> +advertised_mac_binding_entry_add(struct hmap *map,
> + const struct sbrec_port_binding *sb,
> + const struct sbrec_datapath_binding *dp,
> + const char *ip, const char *mac)
> +{
> + struct advertised_mac_binding *e = xmalloc(sizeof *e);
> + e->ip = xstrdup(ip);
> + e->mac = xstrdup(mac);
> + e->sb = sb;
> + e->dp = dp;
> +
> + uint32_t hash = advertised_mac_binding_get_hash(sb, dp, ip, mac);
> + hmap_insert(map, &e->hmap_node, hash);
> +}
> +
> +static void
> +advertised_mac_binding_entry_destroy(struct advertised_mac_binding *e)
> +{
> + free(e->ip);
> + free(e->mac);
> + free(e);
> +}
> +
> +static void
> +advertised_mac_binding_add(struct hmap *map,
> + const struct sbrec_port_binding *sb,
> + const struct sbrec_datapath_binding *dp,
Nit: order of arguments.
> + struct lport_addresses *addr)
> +{
> + if (!addr) {
> + return;
> + }
> +
> + for (size_t i = 0; i < addr->n_ipv4_addrs; i++) {
> + if (!advertised_mac_binding_entry_find(map, sb, dp,
> + addr->ipv4_addrs[i].addr_s,
> + addr->ea_s)) {
> + advertised_mac_binding_entry_add(map, sb, dp,
> + addr->ipv4_addrs[i].addr_s,
> + addr->ea_s);
> + }
> + }
> +
> + for (size_t i = 0; i < addr->n_ipv6_addrs; i++) {
> + if (prefix_is_link_local(&addr->ipv6_addrs[i].addr, 128)) {
> + continue;
> + }
> +
> + if (!advertised_mac_binding_entry_find(map, sb, dp,
> + addr->ipv6_addrs[i].addr_s,
> + addr->ea_s)) {
> + advertised_mac_binding_entry_add(map, sb, dp,
> + addr->ipv6_addrs[i].addr_s,
> + addr->ea_s);
> + }
> + }
> +}
> +
> +static void
> +build_advertised_mac_binding(const struct ovn_datapath *od, struct hmap *map)
> +{
> + ovs_assert(od->nbs);
> +
> + if (!evpn_ip_redistribution_enabled(od)) {
> + return;
> + }
> +
> + struct ovn_port *op;
> + HMAP_FOR_EACH (op, dp_node, &od->ports) {
> + if (!op->sb) {
> + continue;
> + }
> +
> + if (lsp_is_router(op->nbsp) && op->peer) {
> + advertised_mac_binding_add(map, op->sb, od->sdp->sb_dp,
> + &op->peer->lrp_networks);
> + }
> +
> + if (!strcmp(op->nbsp->type, "")) { /* LSP */
> + advertised_mac_binding_add(map, op->sb, od->sdp->sb_dp,
> + op->lsp_addrs);
> + }
> + }
> +}
> +
> +void *
> +en_advertised_mac_binding_sync_init(struct engine_node *node OVS_UNUSED,
> + struct engine_arg *arg OVS_UNUSED)
> +{
> + return NULL;
> +}
> +
> +enum engine_node_state
> +en_advertised_mac_binding_sync_run(struct engine_node *node,
> + void *data OVS_UNUSED)
> +{
> + struct northd_data *northd_data = engine_get_input_data("northd", node);
> + const struct sbrec_advertised_mac_binding_table *sbrec_adv_mb_table =
> + EN_OVSDB_GET(engine_get_input("SB_advertised_mac_binding", node));
> + const struct engine_context *eng_ctx = engine_get_context();
> +
> + struct hmap advertised_mac_binding_map =
> + HMAP_INITIALIZER(&advertised_mac_binding_map);
> +
> + struct ovn_datapath *od;
> + HMAP_FOR_EACH (od, key_node, &northd_data->ls_datapaths.datapaths) {
> + build_advertised_mac_binding(od, &advertised_mac_binding_map);
> + }
> +
> + struct advertised_mac_binding *e;
> + const struct sbrec_advertised_mac_binding *sb_adv_mb;
> + SBREC_ADVERTISED_MAC_BINDING_TABLE_FOR_EACH_SAFE (sb_adv_mb,
> + sbrec_adv_mb_table) {
> + e = advertised_mac_binding_entry_find(&advertised_mac_binding_map,
> + sb_adv_mb->logical_port,
> + sb_adv_mb->datapath,
> + sb_adv_mb->ip, sb_adv_mb->mac);
> + if (!e) {
> + sbrec_advertised_mac_binding_delete(sb_adv_mb);
> + } else {
> + hmap_remove(&advertised_mac_binding_map, &e->hmap_node);
> + advertised_mac_binding_entry_destroy(e);
> + }
> + }
> +
> + HMAP_FOR_EACH_POP (e, hmap_node, &advertised_mac_binding_map) {
> + sb_adv_mb =
> + sbrec_advertised_mac_binding_insert(eng_ctx->ovnsb_idl_txn);
> + sbrec_advertised_mac_binding_set_datapath(sb_adv_mb,
> e->sb->datapath);
> + sbrec_advertised_mac_binding_set_logical_port(sb_adv_mb, e->sb);
> + sbrec_advertised_mac_binding_set_ip(sb_adv_mb, e->ip);
> + sbrec_advertised_mac_binding_set_mac(sb_adv_mb, e->mac);
> + advertised_mac_binding_entry_destroy(e);
> + }
> +
> + hmap_destroy(&advertised_mac_binding_map);
> +
> + return EN_UPDATED;
> +}
> +
> +void
> +en_advertised_mac_binding_sync_cleanup(void *data OVS_UNUSED)
> +{
> +}
> +
> diff --git a/northd/en-advertised-route-sync.h
> b/northd/en-advertised-route-sync.h
> index 7a2927000..ced0de49f 100644
> --- a/northd/en-advertised-route-sync.h
> +++ b/northd/en-advertised-route-sync.h
> @@ -42,4 +42,10 @@ enum engine_node_state en_advertised_route_sync_run(struct
> engine_node *,
> void *en_dynamic_routes_init(struct engine_node *, struct engine_arg *);
> void en_dynamic_routes_cleanup(void *data);
> enum engine_node_state en_dynamic_routes_run(struct engine_node *, void
> *data);
> +
> +void *en_advertised_mac_binding_sync_init(struct engine_node *,
> + struct engine_arg *);
> +void en_advertised_mac_binding_sync_cleanup(void *data);
> +enum engine_node_state
> +en_advertised_mac_binding_sync_run(struct engine_node *, void *data);
> #endif /* EN_ADVERTISED_ROUTE_SYNC_H */
> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> index fdea550d7..0cf2b0936 100644
> --- a/northd/inc-proc-northd.c
> +++ b/northd/inc-proc-northd.c
> @@ -118,7 +118,8 @@ static unixctl_cb_func chassis_features_list;
> SB_NODE(ecmp_nexthop) \
> 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,
> @@ -181,6 +182,7 @@ static ENGINE_NODE(ecmp_nexthop);
> static ENGINE_NODE(multicast_igmp);
> static ENGINE_NODE(acl_id);
> static ENGINE_NODE(advertised_route_sync);
> +static ENGINE_NODE(advertised_mac_binding_sync);
This needs a small rebase after the recent changes to main. It needs to be:
static ENGINE_NODE(advertised_mac_binding_sync, SB_WRITE);
> static ENGINE_NODE(learned_route_sync, CLEAR_TRACKED_DATA);
> static ENGINE_NODE(dynamic_routes);
> static ENGINE_NODE(group_ecmp_route, CLEAR_TRACKED_DATA);
> @@ -355,6 +357,13 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> engine_add_input(&en_advertised_route_sync, &en_northd,
> advertised_route_sync_northd_change_handler);
>
> + engine_add_input(&en_advertised_mac_binding_sync, &en_sb_port_binding,
> + NULL);
> + engine_add_input(&en_advertised_mac_binding_sync,
> + &en_sb_advertised_mac_binding, engine_noop_handler);
This can be NULL instead of engine_noop_handler AFAICT, right? We
ignore the SB.Avertised_MAC_Binding IDL updates in ovn-northd.c
> + engine_add_input(&en_advertised_mac_binding_sync, &en_northd,
> + engine_noop_handler);
We should try to always add a comment when using an engine_noop_handler.
E.g., the comment above:
engine_add_input(&en_dynamic_routes, &en_northd, engine_noop_handler);
> +
> engine_add_input(&en_learned_route_sync, &en_sb_learned_route,
> learned_route_sync_sb_learned_route_change_handler);
> engine_add_input(&en_learned_route_sync, &en_northd,
> @@ -452,6 +461,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> engine_add_input(&en_sync_from_sb, &en_sb_ha_chassis_group, NULL);
>
> engine_add_input(&en_northd_output, &en_acl_id, NULL);
> + engine_add_input(&en_northd_output, &en_advertised_mac_binding_sync,
> NULL);
We should be in line with the rest of the en_northd_output and add a
northd_output_advertised_mac_binding_sync_handler().
> engine_add_input(&en_northd_output, &en_sync_from_sb, NULL);
> engine_add_input(&en_northd_output, &en_sync_to_sb,
> northd_output_sync_to_sb_handler);
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 25c4cd5cb..c9d88dd22 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -968,6 +968,10 @@ main(int argc, char *argv[])
> ovsdb_idl_omit_alert(ovnsb_idl_loop.idl,
> &sbrec_static_mac_binding_columns[i]);
> }
> + for (size_t i = 0; i < SBREC_ADVERTISED_MAC_BINDING_N_COLUMNS; i++) {
> + ovsdb_idl_omit_alert(ovnsb_idl_loop.idl,
> + &sbrec_advertised_mac_binding_columns[i]);
> + }
>
> unixctl_command_register("sb-connection-status", "", 0, 0,
> ovn_conn_show, ovnsb_idl_loop.idl);
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 73b5f213f..c2d16f5eb 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -977,6 +977,12 @@
> routers.
> </p>
>
> + <p>
> + If <code>ip</code> is specified then ovn-controller will advertise
> + all IPs/MACs bindings that are local to the chassis. The applies to
> + VIFs and router ports.
> + </p>
> +
> <p>
> NOTE: this feature is experimental and may be subject to
> removal/change in the future.
> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> index f22141489..156d013ef 100644
> --- a/ovn-sb.ovsschema
> +++ b/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_Southbound",
> - "version": "21.6.0",
> - "cksum": "1200327755 35814",
> + "version": "21.7.0",
> + "cksum": "1583541293 36671",
> "tables": {
> "SB_Global": {
> "columns": {
> @@ -695,6 +695,22 @@
> "id": {"type": {"key": {"type": "integer",
> "minInteger": 0,
> "maxInteger": 32767}}}},
> - "isRoot": true}
> + "isRoot": true},
> + "Advertised_MAC_Binding": {
> + "columns": {
> + "datapath": {"type": {"key": {"type": "uuid",
> + "refTable": "Datapath_Binding",
> + "refType": "strong"}}},
> + "logical_port": {"type": {"key": {"type": "uuid",
> + "refTable": "Port_Binding",
> + "refType": "strong"}}},
> + "ip": {"type": "string"},
> + "mac": {"type": "string"},
> + "external_ids": {
> + "type": {"key": "string", "value": "string",
> + "min": 0, "max": "unlimited"}}},
> + "indexes": [["datapath", "logical_port",
> + "ip", "mac"]],
Nit: these fit on one line.
> + "isRoot": true}
> }
> }
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index a781129b7..a1e4f01ee 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -5457,4 +5457,34 @@ tcp.flags = RST;
> <code>allow-established</code> ACL.
> </column>
> </table>
> +
> + <table name="Advertised_MAC_Binding">
> + <p>
> + Each record represents an ip/mac binding the
> + <code>OVN_Northbound.Logical_Switch</code> is announcing outside
> + the network fabric if <code>EVPN</code> is enabled on the datapath.
> + </p>
> +
> + <column name="datapath">
> + The datapath belonging to the
> + <code>OVN_Northbound.Logical_Switch</code> this ip/mac binding
> + belongs to.
> + </column>
> +
> + <column name="logical_port">
> + This is the <ref table="Port_Binding"/> this ip/mac binding belongs to.
> + </column>
> +
> + <column name="ip">
> + Announced IP (e.g. 192.168.100.1).
> + </column>
> +
> + <column name="mac">
> + Announced mac address.
> + </column>
> +
> + <column name="external_ids">
> + See <em>External IDs</em> at the beginning of this document.
> + </column>
> + </table>
> </database>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index fd572a023..d26ac8507 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -18859,3 +18859,41 @@ AT_CHECK([ovn-sbctl lflow-list ls1 | grep
> ls_in_arp_rsp | grep "ip6.dst == fd01:
>
> AT_CLEANUP
> ])
> +
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([LS EVPN Route Type2 sync])
> +ovn_start ovn-northd
> +
> +AS_BOX([Create logical switch.])
> +check ovn-nbctl ls-add ls-evpn \
> + -- set logical_switch ls-evpn other_config:dynamic-routing-vni=10 \
> + -- lsp-add ls-evpn vm1 \
> + -- lsp-set-addresses vm1 "00:00:00:00:01:00 10.0.0.2"
> +check ovn-nbctl --wait=sb sync
> +check_row_count Advertised_MAC_Binding 0
> +ls_evpn_uuid=$(fetch_column Datapath_Binding _uuid external_ids:name=ls-evpn)
> +
> +check ovn-nbctl --wait=sb set logical_switch ls-evpn
> other_config:dynamic-routing-redistribute=ip
> +check_row_count Advertised_MAC_Binding 1 ip=10.0.0.2 datapath=$ls_evpn_uuid
> mac='00\:00\:00\:00\:01\:00'
> +
> +check ovn-nbctl lr-add lr \
> + -- lrp-add lr lrp 00:00:00:00:01:01 10.0.0.1/24 \
> + -- lsp-add ls-evpn ls-evpn-lrp \
> + -- lsp-set-type ls-evpn-lrp router \
> + -- lsp-set-addresses ls-evpn-lrp 00:00:00:00:01:01 \
> + -- lsp-set-options ls-evpn-lrp router-port=lrp
> +check ovn-nbctl --wait=sb sync
> +
> +check_row_count Advertised_MAC_Binding 1 ip=10.0.0.2 datapath=$ls_evpn_uuid
> mac='00\:00\:00\:00\:01\:00'
> +check_row_count Advertised_MAC_Binding 1 ip=10.0.0.1 datapath=$ls_evpn_uuid
> mac='00\:00\:00\:00\:01\:01'
> +check_row_count Advertised_MAC_Binding 2
> +
> +check ovn-nbctl --wait=sb lsp-del vm1
> +check_row_count Advertised_MAC_Binding 1 ip=10.0.0.1 datapath=$ls_evpn_uuid
> mac='00\:00\:00\:00\:01\:01'
> +check_row_count Advertised_MAC_Binding 1
> +
> +check ovn-nbctl --wait=sb clear logical_switch ls-evpn other_config
> +check_row_count Advertised_MAC_Binding 0
> +
> +AT_CLEANUP
> +])
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
