On Thu, Jun 18, 2026 at 12:35 PM Chanyeol Yoon <[email protected]> wrote:
> Hi Ales,
>
> Thanks.
>
> > This is basically a copy from the
> neighbor_collect_ip_mac_to_advertise().
> > That is not wrong per se, but it might be worth trying to find a way
> how we
> > can collect those in a single loop in case both IP + FDB are enabled.
>
> Agreed. I'll merge the two Advertised_MAC_Binding walks into one pass,
> so when
> both IP and FDB are on we go over the table once and fill both the FDB
> (MAC-only) and the v4/v6 neighbor sets.
>
> > It also feels a little strange that we don't check the NAT option here
> at
> > all, because to make things work, the user would have to
> > configure NAT+IP or NAT+IP+FDB. We might need a type column
> > for the advertised_mac_binding to make this work with additional
> > options.
>
> Right -- the row carries no notion of where it came from, so the
> controller
> can't tell a floating IP entry from a port one, and 'nat' only does
> anything if
> 'ip' is set too. Before I go implement it, what did you have in mind for
> the
> type column? Something like a "port"/"nat" string, with the
> controller deciding
> per row what to program, so that enabling 'nat' on its own is enough to
> get a
> floating IP advertised as a Type-2 route? I'll follow whatever shape
> you prefer,
> and can send it as a follow-up in the series to keep this patch focused.
>
Yeah so the column type would basically be something like this:
"type": {"type": {"key": {"type": "string", "enum": ["set", ["ip", "nat"]]}.
With that we can easily filter by type and advertise it only if IP and/or
NAT
is enabled.
> Thanks,
> Chanyeol
>
> --
> Chanyeol Yoon
> KT Cloud
>
> 2026년 6월 18일 (목) 오후 4:28, Ales Musil <[email protected]>님이 작성:
> >
> >
> >
> > On Tue, Jun 16, 2026 at 4:52 AM Chanyeol Yoon <[email protected]> wrote:
> >>
> >> The FDB collector for the EVPN advertise interface only iterates the
> >> Logical_Switch's Port_Bindings, picking MACs out of pb->mac. That works
> >> for VIFs and router ports, where the MAC of interest is always a
> >> Port_Binding address, but it skips MACs that are only known via
> >> Advertised_MAC_Binding (for example, the external_mac of a distributed
> >> dnat_and_snat NAT entry, i.e. a floating IP).
> >>
> >> Without the MAC in the local FDB, FRR cannot match the corresponding
> >> ip neigh entry (which is already programmed by the IP-mode collector)
> >> to a local MAC, and therefore does not emit a Type-2 MAC+IP route for
> >> the floating IP -- only direct VIF and router-port advertisements work.
> >>
> >> Extend neighbor_collect_mac_to_advertise() so that, after walking the
> >> Port_Bindings on the Logical_Switch, it also iterates the
> >> Advertised_MAC_Binding rows attached to the same datapath. The same
> >> chassis-residency filter (via neighbor_get_relevant_port_binding) is
> >> reused so the MAC is only injected on the chassis that owns the
> >> referenced port.
> >>
> >> This is the controller-side counterpart of the ovn-northd change in
> >> "northd: Advertise distributed NAT IPs over EVPN."
> >>
> >> Reported-by: Chanyeol Yoon <[email protected]>
> >> Suggested-by: Ales Musil <[email protected]>
> >> Signed-off-by: Chanyeol Yoon <[email protected]>
> >> ---
> >
> >
> > Hi Chanyeol,
> >
> > thank you for the patch. I have a comment down below.
> >
> >> controller/neighbor.c | 38 ++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 38 insertions(+)
> >>
> >> diff --git a/controller/neighbor.c b/controller/neighbor.c
> >> index 72fabe205..8bbd8e224 100644
> >> --- a/controller/neighbor.c
> >> +++ b/controller/neighbor.c
> >> @@ -301,6 +301,44 @@ neighbor_collect_mac_to_advertise(const struct
> neighbor_ctx_in *n_ctx_in,
> >> }
> >>
> >> sbrec_port_binding_index_destroy_row(target);
> >> +
> >> + /* Some MACs need to be advertised over EVPN even though they are
> not the
> >> + * address of any port_binding on this Logical Switch (for
> example, the
> >> + * external_mac of a distributed dnat_and_snat NAT entry /
> floating IP).
> >> + * For those, ovn-northd populates the Advertised_MAC_Binding
> table on
> >> + * this Logical Switch and we inject them into the FDB here. */
> >> + struct sbrec_advertised_mac_binding *amb_target =
> >> +
> sbrec_advertised_mac_binding_index_init_row(n_ctx_in->sbrec_amb_by_dp);
> >> + sbrec_advertised_mac_binding_index_set_datapath(amb_target, dp);
> >> +
> >> + const struct sbrec_advertised_mac_binding *adv_mb;
> >> + SBREC_ADVERTISED_MAC_BINDING_FOR_EACH_EQUAL (adv_mb, amb_target,
> >> +
> n_ctx_in->sbrec_amb_by_dp) {
> >> + if (!adv_mb->logical_port) {
> >> + continue;
> >> + }
> >> +
> >> + const struct sbrec_port_binding *pb =
> >> +
> neighbor_get_relevant_port_binding(n_ctx_in->sbrec_pb_by_name,
> >> + adv_mb->logical_port);
> >> + if (!lport_pb_is_chassis_resident(n_ctx_in->chassis, pb)) {
> >> + continue;
> >> + }
> >> +
> >> + struct eth_addr ea;
> >> + char *err = str_to_mac(adv_mb->mac, &ea);
> >> + if (err) {
> >> + free(err);
> >> + continue;
> >> + }
> >> +
> >> + if (!advertise_neigh_find(neighbors, ea, &in6addr_any)) {
> >> + advertise_neigh_add(neighbors, ea, in6addr_any);
> >> + }
> >> + sset_add(advertised_pbs, pb->logical_port);
> >> + }
> >> +
> >> + sbrec_advertised_mac_binding_index_destroy_row(amb_target);
> >
> >
> > This is basically a copy from the neighbor_collect_ip_mac_to_advertise().
> > That is not wrong per se, but it might be worth trying to find a way how
> we
> > can collect those in a single loop in case both IP + FDB are enabled.
> >
> > It also feels a little strange that we don't check the NAT option here at
> > all, because to make things work, the user would have to
> > configure NAT+IP or NAT+IP+FDB. We might need a type column
> > for the advertised_mac_binding to make this work with additional
> > options.
> >
> >> }
> >>
> >> static void
> >> --
> >> 2.54.0 (Apple Git-156)
> >>
> >
> > Regards,
> > Ales
>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
