> 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.
Hi Ales, thx for the review. > > > > > > Reported-at: https://issues.redhat.com/browse/FDP-2220 > > > Wrong FDP link, it should be https://issues.redhat.com/browse/FDP-2221. ack, I will fix it in v2. > > > > > 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. ack, I will fix it in v2. > > > > 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? ack, I will fix it in v2. > > 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/ ack, I will fix it in v2. > > > > +#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. ack, I will fix it in v2. > > > > } > > > > 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. ack, I will fix it in v2. Regards, Lorenzo > > > > 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
