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. 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
