On Sun, Jun 21, 2026 at 12:47 PM Chanyeol Yoon <[email protected]> wrote:

> The EVPN advertise interface needs the distributed floating IPs that
> ovn-northd publishes in the Advertised_MAC_Binding table both as a FDB
> MAC and as a MAC+IP neighbor, so that FRR emits them as Type-2 routes.
> Until now only port-derived MAC+IP entries were handled, and the FDB
> and IP collectors each walked the Advertised_MAC_Binding table
> separately.
>
> Collect the Advertised_MAC_Binding rows of a datapath in a single pass
> and advertise each row according to its 'type' column:
>
>   - "ip" rows (port addresses) follow the 'fdb'/'ip' tokens, matching
>     the advertisement of the port itself;
>   - "nat" rows (distributed floating IPs) are self-contained: the 'nat'
>     token alone programs both the FDB MAC and the MAC+IP neighbor, so a
>     floating IP is advertised as a Type-2 route without also having to
>     enable 'ip'/'fdb'.
>
> This is the controller-side counterpart of the ovn-northd change in
> "northd: Advertise distributed NAT IPs over EVPN."
>
> Suggested-by: Ales Musil <[email protected]>
> Signed-off-by: Chanyeol Yoon <[email protected]>
> ---
>

Hi Chanyeol,

I think there might have been a misunderstanding when
we should advertise FDB.

So "advertised_mac_binding" should be advertised under the following
conditions:
dynamic-routing-advertise="ip" -> advertise only "ip" type and empty/NULL
type
dynamic-routing-advertise="nat" -> advertise only "nat" type
If both are set advertise both.

For FDB we should advertise only if the fdb is set. In that case we should
advertise
all "advertised_mac_binding" regardless of the type.

 controller/neighbor.c | 81 ++++++++++++++++++++++++++++++++-----------
>  1 file changed, 60 insertions(+), 21 deletions(-)
>
> diff --git a/controller/neighbor.c b/controller/neighbor.c
> index 72fabe205..8b4743008 100644
> --- a/controller/neighbor.c
> +++ b/controller/neighbor.c
> @@ -46,8 +46,9 @@ 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_ip_mac_to_advertise(
> -    const struct neighbor_ctx_in *,
> +static void neighbor_collect_advertised_mac_bindings(
> +    const struct neighbor_ctx_in *, enum neigh_redistribute_mode mode,
> +    struct hmap *fdb_neighbors,
>      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(
> @@ -176,14 +177,13 @@ neighbor_run(struct neighbor_ctx_in *n_ctx_in,
>                                                n_ctx_out->advertised_pbs,
>                                                ld->datapath);
>          }
> -        if (nrm_mode_IP_is_set(mode) && br_v4 && br_v6) {
> -            neighbor_collect_ip_mac_to_advertise(n_ctx_in,
> -
>  &br_v4->announced_neighbors,
> -
>  &br_v6->announced_neighbors,
> -
>  n_ctx_out->advertised_pbs,
> -                                                 ld->datapath);
> -        }
> -
> +        /* Advertise the SB Advertised_MAC_Binding rows in a single pass;
> per
> +         * row the 'type' column selects which redistribute token gates
> it. */
> +        neighbor_collect_advertised_mac_bindings(
> +            n_ctx_in, mode, lo ? &lo->announced_neighbors : NULL,
> +            br_v4 ? &br_v4->announced_neighbors : NULL,
> +            br_v6 ? &br_v6->announced_neighbors : NULL,
> +            n_ctx_out->advertised_pbs, ld->datapath);
>      }
>  }
>
> @@ -303,9 +303,24 @@ neighbor_collect_mac_to_advertise(const struct
> neighbor_ctx_in *n_ctx_in,
>      sbrec_port_binding_index_destroy_row(target);
>  }
>
> +/* Walks the SB Advertised_MAC_Binding rows of 'dp' exactly once and,
> based on
> + * each row's 'type' column and the datapath's redistribute 'mode',
> advertises:
> + *
> + *   - the IP/MAC pair as an EVPN Type-2 MAC+IP neighbor (into
> 'neighbors_v4'/
> + *     'neighbors_v6'), and
> + *   - the MAC into the FDB ('fdb_neighbors'),
> + *
> + * A "ip" row (a port address) follows the 'ip'/'fdb' tokens, matching the
> + * advertisement of the port itself.  A "nat" row (a distributed floating
> IP)
> + * is self-contained: the 'nat' token alone enables both the MAC+IP
> neighbor
> + * and the FDB entry, so that the floating IP comes out as a Type-2 route
> + * without also having to enable 'ip'/'fdb'.  Any of the target hmaps may
> be
> + * NULL when the corresponding monitored interface does not exist. */
>  static void
> -neighbor_collect_ip_mac_to_advertise(
> +neighbor_collect_advertised_mac_bindings(
>          const struct neighbor_ctx_in *n_ctx_in,
> +        enum neigh_redistribute_mode mode,
> +        struct hmap *fdb_neighbors,
>          struct hmap *neighbors_v4,
>          struct hmap *neighbors_v6,
>          struct sset *advertised_pbs,
> @@ -326,6 +341,15 @@ neighbor_collect_ip_mac_to_advertise(
>              continue;
>          }
>
> +        bool is_nat = adv_mb->type && !strcmp(adv_mb->type, "nat");

+        bool ip_enabled = is_nat ? nrm_mode_NAT_is_set(mode)
> +                                 : nrm_mode_IP_is_set(mode);
> +        bool fdb_enabled = is_nat ? nrm_mode_NAT_is_set(mode)
> +                                  : nrm_mode_FDB_is_set(mode);
> +        if (!ip_enabled && !fdb_enabled) {
> +            continue;
> +        }
>

Thinking about it more we don't actually need the type. Sorry for suggesting
that. The northd does the filtering. We don't even need to check
nrm_mode_NAT_is_set(mode) or  nrm_mode_IP_is_set(mode) in
ovn-controller. The database would be empty for given datapath if it's not
enabled. So this can be actually simplified greatly to just advertise any
advertised_mac_binding if we have them. And if "fdb" is set advertise them
as FDBs too.

+
>          const struct sbrec_port_binding *pb =
>              neighbor_get_relevant_port_binding(n_ctx_in->sbrec_pb_by_name,
>                                                 adv_mb->logical_port);
> @@ -333,11 +357,6 @@ neighbor_collect_ip_mac_to_advertise(
>              continue;
>          }
>
> -        struct in6_addr ip;
> -        if (!ip46_parse(adv_mb->ip, &ip)) {
> -            continue;
> -        }
> -
>          struct eth_addr ea;
>          char *err = str_to_mac(adv_mb->mac, &ea);
>          if (err) {
> @@ -345,12 +364,32 @@ neighbor_collect_ip_mac_to_advertise(
>              continue;
>          }
>
> -        struct hmap *neighbors = IN6_IS_ADDR_V4MAPPED(&ip)
> -                                 ? neighbors_v4 : neighbors_v6;
> -        if (!advertise_neigh_find(neighbors, ea, &ip)) {
> -            advertise_neigh_add(neighbors, ea, ip);
> +        struct in6_addr ip;
> +        if (!ip46_parse(adv_mb->ip, &ip)) {
> +            continue;
> +        }
> +
> +        bool advertised = false;
> +        if (ip_enabled) {
> +            struct hmap *neighbors = IN6_IS_ADDR_V4MAPPED(&ip)
> +                                     ? neighbors_v4 : neighbors_v6;
> +            if (neighbors) {
> +                if (!advertise_neigh_find(neighbors, ea, &ip)) {
> +                    advertise_neigh_add(neighbors, ea, ip);
> +                }
> +                advertised = true;
> +            }
> +        }
> +        if (fdb_enabled && fdb_neighbors) {
> +            if (!advertise_neigh_find(fdb_neighbors, ea, &in6addr_any)) {
> +                advertise_neigh_add(fdb_neighbors, ea, in6addr_any);
> +            }
> +            advertised = true;
> +        }
> +
> +        if (advertised) {
> +            sset_add(advertised_pbs, pb->logical_port);
>          }
> -        sset_add(advertised_pbs, pb->logical_port);
>      }
>
>      sbrec_advertised_mac_binding_index_destroy_row(target);
> --
> 2.54.0 (Apple Git-156)
>
>
Regards,
Ales
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to