On Tue, Nov 4, 2025 at 5:33 PM Lorenzo Bianconi via dev < [email protected]> wrote:
> Fix evpn neighbors advertisement when dynamic-routing-redistribute > option is set to "fdb,ip" > Hi Lorenzo, thank you for the patch, I have a couple of comments down below. > > Reported-at: https://issues.redhat.com/browse/FDP-2220 Wrong FDP link, it should be https://issues.redhat.com/browse/FDP-2221. > > Fixes: 499ddafbf160 ("controller: Advertise local EVPN type2 routes in > ovn-controller.") > Fixes: 3b500540552f ("northd: Collect vif and router port IPs/MAC bindings > in the SB Advertised_Mac_Binding table.") > Signed-off-by: Lorenzo Bianconi <[email protected]> > --- > controller/neighbor.c | 8 ++++---- > controller/ovn-controller.c | 9 +++------ > lib/ovn-util.c | 25 +++++++++++++++++++++++++ > lib/ovn-util.h | 30 ++++++++++++++++++++++++++++++ > northd/en-advertised-route-sync.c | 6 +++--- > tests/system-ovn.at | 2 +- > 6 files changed, 66 insertions(+), 14 deletions(-) > > diff --git a/controller/neighbor.c b/controller/neighbor.c > index 0bad7e019..dc1f0894e 100644 > --- a/controller/neighbor.c > +++ b/controller/neighbor.c > @@ -117,15 +117,15 @@ neighbor_run(struct neighbor_ctx_in *n_ctx_in, > NEIGH_IFACE_BRIDGE, vni); > vector_push(n_ctx_out->monitored_interfaces, &br_v6); > > - const char *redistribute = smap_get(&ld->datapath->external_ids, > - > "dynamic-routing-redistribute"); > - if (redistribute && !strcmp(redistribute, "fdb")) { > + enum neigh_redistribute_mode mode = > + parse_neigh_dynamic_redistribute(&ld->datapath->external_ids); > + if (mode & NRM_FDB) { > neighbor_collect_mac_to_advertise(n_ctx_in, > &lo->announced_neighbors, > n_ctx_out->advertised_pbs, > ld->datapath); > } > - if (redistribute && !strcmp(redistribute, "ip")) { > + if (mode & NRM_IP) { > Please use the nrm_mode_*_is_set() functions. > neighbor_collect_ip_mac_to_advertise(n_ctx_in, > > &br_v4->announced_neighbors, > > &br_v6->announced_neighbors, > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index c2dab41c1..9006e3d56 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -5983,12 +5983,9 @@ neighbor_runtime_data_handler(struct engine_node > *node, void *data) > return EN_UNHANDLED; > } > > - const char *redistribute = smap_get(&ld->datapath->external_ids, > - > "dynamic-routing-redistribute"); > - if (!redistribute) { > - continue; > - } > - if (strcmp(redistribute, "fdb") && strcmp(redistribute, "ip")) { > + enum neigh_redistribute_mode mode = > + parse_neigh_dynamic_redistribute(&ld->datapath->external_ids); > + if (!(mode & (NRM_FDB | NRM_IP))) { > Isn't it enough to check if mode == NRM_NONE? continue; > } > > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > index d27983d1e..f17173c31 100644 > --- a/lib/ovn-util.c > +++ b/lib/ovn-util.c > @@ -1648,3 +1648,28 @@ normalize_addr_str(const char *orig_addr) > > return ret; > } > + > +enum neigh_redistribute_mode > +parse_neigh_dynamic_redistribute(const struct smap *options) > +{ > + const char *redistribute = smap_get(options, > + "dynamic-routing-redistribute"); > + if (!redistribute) { > + return NRM_NODE; > + } > + > + enum neigh_redistribute_mode mode = NRM_NODE; > + char *save_ptr = NULL, *tokstr = xstrdup(redistribute); > + for (char *token = strtok_r(tokstr, ",", &save_ptr); > + token; token = strtok_r(NULL, ",", &save_ptr)) { > + if (!strcmp(token, "fdb")) { > + mode |= NRM_FDB; > + } > + if (!strcmp(token, "ip")) { > + mode |= NRM_IP; > + } > + } > + free(tokstr); > + > + return mode; > +} > diff --git a/lib/ovn-util.h b/lib/ovn-util.h > index 7a5bb9559..7b57f5664 100644 > --- a/lib/ovn-util.h > +++ b/lib/ovn-util.h > @@ -686,4 +686,34 @@ char *normalize_ipv6_addr_str(const char *orig_addr); > > char *normalize_addr_str(const char *orig_addr); > > +#define NEIGH_REDISTRIBUTE_MODES \ > + NEIGH_REDISTRIBUTE_MODE(FDB, 0) \ > + NEIGH_REDISTRIBUTE_MODE(IP, 1) > + > +enum neigh_redistribute_mode_bits { > +#define NEIGH_REDISTRIBUTE_MODE(PROTOCOL, BIT) NRM_##PROTOCOL##_BIT = BIT, > + NEIGH_REDISTRIBUTE_MODES > +#undef NEIGH_REDISTRIBUTE_MODE > +}; > + > +enum neigh_redistribute_mode { > + NRM_NODE = 0, > nit: Typo, s/NRM_NODE/NRM_NONE/ > +#define NEIGH_REDISTRIBUTE_MODE(PROTOCOL, BIT) \ > + NRM_##PROTOCOL = (1 << NRM_##PROTOCOL##_BIT), > + NEIGH_REDISTRIBUTE_MODES > +#undef NEIGH_REDISTRIBUTE_MODE > +}; > + > +#define NEIGH_REDISTRIBUTE_MODE(PROTOCOL, BIT) \ > + static inline bool nrm_mode_##PROTOCOL##_is_set( \ > + enum neigh_redistribute_mode value) \ > + { \ > + return !!(value & NRM_##PROTOCOL); \ > + } > +NEIGH_REDISTRIBUTE_MODES > +#undef NEIGH_REDISTRIBUTE_MODE > + > +enum neigh_redistribute_mode > +parse_neigh_dynamic_redistribute(const struct smap *options); > + > #endif /* OVN_UTIL_H */ > diff --git a/northd/en-advertised-route-sync.c > b/northd/en-advertised-route-sync.c > index c737376ae..30a90c348 100644 > --- a/northd/en-advertised-route-sync.c > +++ b/northd/en-advertised-route-sync.c > @@ -851,9 +851,9 @@ evpn_ip_redistribution_enabled(const struct > ovn_datapath *od) > return false; > } > > - const char *redistribute = smap_get(&od->nbs->other_config, > - "dynamic-routing-redistribute"); > - return redistribute && !strcmp(redistribute, "ip"); > + enum neigh_redistribute_mode mode = > + parse_neigh_dynamic_redistribute(&od->nbs->other_config); > + return !!(mode & NRM_IP); > Please use the nrm_mode_*_is_set() functions. > } > > static uint32_t > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index 373a87657..f9ae3a503 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -18534,7 +18534,7 @@ check ovn-nbctl --wait=hv > \ > -- 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 > +check ovn-nbctl --wait=hv set logical_switch ls-evpn > other_config:dynamic-routing-redistribute=fdb,ip > With both options set we should check that the FDBs are also advertised. > check_row_count Advertised_MAC_Binding 1 ip=172.16.1.10 > mac='f0\:00\:0f\:16\:01\:10' datapath=$ls_evpn_uuid > check_row_count Advertised_MAC_Binding 1 ip=172.16.1.1 > mac='f0\:00\:0f\:16\:01\:01' datapath=$ls_evpn_uuid > > -- > 2.51.1 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Regards, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
