On Thu, Jul 30, 2026 at 10:16 AM Dumitru Ceara <[email protected]> wrote:
> Hi Mark, Ales, > > On 7/29/26 7:48 PM, Mark Michelson wrote: > > Thanks for v2. I added my ack and pushed this to main. Should this > > also be backported, or is advertisement of dynamic FDB entries > > considered just a new feature? > > > > In my opinion this is a bug fix and should be backported to 26.03. > > Without it we risk inefficient unknown-unicast traffic forwarding (flood > in the EVPN overlay) if the destination MAC is an OVN workload whose MAC > is learnt dynamically in OVN's FDB. > > Regards, > Dumitru > Thank Mark and Dumitru, indeed it should be backported to 26.03, I took care of that. > > On Wed, Jul 29, 2026 at 3:10 AM Ales Musil via dev > > <[email protected]> wrote: > >> > >> When a logical switch has dynamic-routing-redistribute=fdb > >> set, neighbor_collect_mac_to_advertise() only iterated > >> statically configured port MAC addresses. Dynamically > >> learned FDB entries from the SB FDB table were completely > >> ignored. > >> > >> Add neighbor_collect_fdb_to_advertise() which iterates FDB > >> entries for the datapath and advertises each learned MAC as > >> an EVPN Type-2 MAC-only neighbor. Only FDB entries learned > >> on VIF ports bound to this chassis or on localnet ports are > >> included. The per-port dynamic-routing-advertise option is > >> honored. > >> > >> An incremental processing handler for SB FDB changes is > >> added to trigger a recompute of the neighbor node only when > >> an FDB entry changes on a datapath that has FDB > >> advertisement enabled. A set of relevant datapath pointers > >> is maintained to filter out irrelevant FDB changes. > >> > >> Fixes: 32d5252ceb65 ("controller, northd: Add an option to advertise > FDB over EVPN.") > >> Reported-at: https://redhat.atlassian.net/browse/FDP-4149 > >> Assisted-by: Claude Opus 4.6, OpenCode > >> Acked-by: Mairtin O'Loingsigh <[email protected]> > >> Signed-off-by: Ales Musil <[email protected]> > >> --- > >> v2: Rebase on top of latest main. > >> Add comment into the handler. > >> Address Mairtin's nit and add his ack. > >> --- > >> controller/neighbor.c | 61 +++++++++++++++++++++++++++++++ > >> controller/neighbor.h | 8 +++++ > >> controller/ovn-controller.c | 47 ++++++++++++++++++++++++ > >> ovn-nb.xml | 8 +++-- > >> tests/ovn-inc-proc-graph-dump.at | 1 + > >> tests/system-ovn.at | 62 ++++++++++++++++++++++++++++++++ > >> 6 files changed, 185 insertions(+), 2 deletions(-) > >> > >> diff --git a/controller/neighbor.c b/controller/neighbor.c > >> index ada9254e0..c06183063 100644 > >> --- a/controller/neighbor.c > >> +++ b/controller/neighbor.c > >> @@ -46,6 +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_fdb_to_advertise( > >> + const struct neighbor_ctx_in *, struct hmap *neighbors, > >> + struct hmapx *fdb_datapaths, struct local_datapath *); > >> static void neighbor_collect_advertised_mac_bindings( > >> const struct neighbor_ctx_in *, enum neigh_redistribute_mode mode, > >> struct hmap *fdb_neighbors, > >> @@ -176,6 +179,10 @@ neighbor_run(struct neighbor_ctx_in *n_ctx_in, > >> &lo->announced_neighbors, > >> > n_ctx_out->advertised_pbs, > >> ld->datapath); > >> + neighbor_collect_fdb_to_advertise(n_ctx_in, > >> + &lo->announced_neighbors, > >> + n_ctx_out->fdb_datapaths, > >> + ld); > >> } > >> /* Advertise every SB Advertised_MAC_Binding row of the > datapath as an > >> * EVPN Type-2 MAC+IP neighbor; ovn-northd only populates rows > that are > >> @@ -305,6 +312,60 @@ neighbor_collect_mac_to_advertise(const struct > neighbor_ctx_in *n_ctx_in, > >> sbrec_port_binding_index_destroy_row(target); > >> } > >> > >> +/* Walks the SB FDB entries for 'dp' and advertises each learned MAC > as an > >> + * EVPN Type-2 MAC-only neighbor (into 'neighbors'). Only FDB entries > >> + * learned on VIF ports bound to this chassis or on localnet ports are > >> + * included. The per-port 'dynamic-routing-advertise' option is > honored. */ > >> +static void > >> +neighbor_collect_fdb_to_advertise(const struct neighbor_ctx_in > *n_ctx_in, > >> + struct hmap *neighbors, > >> + struct hmapx *fdb_datapaths, > >> + struct local_datapath *ld) > >> +{ > >> + hmapx_add(fdb_datapaths, ld); > >> + > >> + const struct sbrec_datapath_binding *dp = ld->datapath; > >> + struct sbrec_fdb *fdb_target = > >> + sbrec_fdb_index_init_row(n_ctx_in->sbrec_fdb_by_dp_key); > >> + sbrec_fdb_index_set_dp_key(fdb_target, dp->tunnel_key); > >> + > >> + const struct sbrec_fdb *fdb; > >> + SBREC_FDB_FOR_EACH_EQUAL (fdb, fdb_target, > >> + n_ctx_in->sbrec_fdb_by_dp_key) { > >> + const struct sbrec_port_binding *pb = > >> + lport_lookup_by_key_with_dp(n_ctx_in->sbrec_pb_by_key, > >> + dp, fdb->port_key); > >> + if (!pb) { > >> + continue; > >> + } > >> + > >> + enum en_lport_type type = get_lport_type(pb); > >> + if (type == LP_VIF && > >> + !lport_pb_is_chassis_resident(n_ctx_in->chassis, pb)) { > >> + continue; > >> + } > >> + > >> + if (type != LP_VIF && type != LP_LOCALNET) { > >> + continue; > >> + } > >> + > >> + if (!smap_get_bool(&pb->options, "dynamic-routing-advertise", > true)) { > >> + continue; > >> + } > >> + > >> + struct eth_addr ea; > >> + if (!eth_addr_from_string(fdb->mac, &ea)) { > >> + continue; > >> + } > >> + > >> + if (!advertise_neigh_find(neighbors, ea, &in6addr_any)) { > >> + advertise_neigh_add(neighbors, ea, in6addr_any); > >> + } > >> + } > >> + > >> + sbrec_fdb_index_destroy_row(fdb_target); > >> +} > >> + > >> /* Walks the SB Advertised_MAC_Binding rows of 'dp' and advertises > each as an > >> * EVPN Type-2 MAC+IP neighbor (into 'neighbors_v4'/'neighbors_v6'). > >> * ovn-northd only populates a row when the datapath opted into > advertising it > >> diff --git a/controller/neighbor.h b/controller/neighbor.h > >> index e3adc87d1..cd158a0f1 100644 > >> --- a/controller/neighbor.h > >> +++ b/controller/neighbor.h > >> @@ -21,6 +21,7 @@ > >> #include <net/if.h> > >> #include <stdint.h> > >> > >> +#include "hmapx.h" > >> #include "lib/sset.h" > >> #include "openvswitch/hmap.h" > >> > >> @@ -48,6 +49,10 @@ struct neighbor_ctx_in { > >> struct ovsdb_idl_index *sbrec_amb_by_dp; > >> /* Index for Port Binding by name. */ > >> struct ovsdb_idl_index *sbrec_pb_by_name; > >> + /* Index for Port Binding by datapath and tunnel key. */ > >> + struct ovsdb_idl_index *sbrec_pb_by_key; > >> + /* Index for FDB by dp_key. */ > >> + struct ovsdb_idl_index *sbrec_fdb_by_dp_key; > >> const struct sbrec_chassis *chassis; > >> }; > >> > >> @@ -56,6 +61,9 @@ struct neighbor_ctx_out { > >> struct vector *monitored_interfaces; > >> /* Contains set of port binding names that are currently > advertised. */ > >> struct sset *advertised_pbs; > >> + /* Contains 'struct local_datapath' pointers for datapaths with FDB > >> + * advertisement enabled. */ > >> + struct hmapx *fdb_datapaths; > >> }; > >> > >> enum neighbor_interface_type { > >> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > >> index 5f63c5fec..ea9952a19 100644 > >> --- a/controller/ovn-controller.c > >> +++ b/controller/ovn-controller.c > >> @@ -6081,6 +6081,9 @@ struct ed_type_neighbor { > >> struct vector monitored_interfaces; > >> /* Contains set of PB names that are currently advertised. */ > >> struct sset advertised_pbs; > >> + /* Contains 'struct local_datapath' pointers for datapaths with FDB > >> + * advertisement enabled. */ > >> + struct hmapx fdb_datapaths; > >> }; > >> > >> static void * > >> @@ -6093,6 +6096,7 @@ en_neighbor_init(struct engine_node *node > OVS_UNUSED, > >> .monitored_interfaces = > >> VECTOR_EMPTY_INITIALIZER(struct neighbor_interface_monitor > *), > >> .advertised_pbs = SSET_INITIALIZER(&data->advertised_pbs), > >> + .fdb_datapaths = HMAPX_INITIALIZER(&data->fdb_datapaths), > >> }; > >> return data; > >> } > >> @@ -6105,6 +6109,7 @@ en_neighbor_cleanup(void *data) > >> neighbor_cleanup(&ne_data->monitored_interfaces); > >> vector_destroy(&ne_data->monitored_interfaces); > >> sset_destroy(&ne_data->advertised_pbs); > >> + hmapx_destroy(&ne_data->fdb_datapaths); > >> } > >> > >> static enum engine_node_state > >> @@ -6131,6 +6136,14 @@ en_neighbor_run(struct engine_node *node > OVS_UNUSED, void *data) > >> engine_ovsdb_node_get_index( > >> engine_get_input("SB_advertised_mac_binding", node), > >> "datapath"); > >> + struct ovsdb_idl_index *sbrec_port_binding_by_key = > >> + engine_ovsdb_node_get_index( > >> + engine_get_input("SB_port_binding", node), > >> + "key"); > >> + struct ovsdb_idl_index *sbrec_fdb_by_dp_key = > >> + engine_ovsdb_node_get_index( > >> + engine_get_input("SB_fdb", node), > >> + "dp_key"); > >> > >> const char *chassis_id = get_ovs_chassis_id(ovs_table); > >> ovs_assert(chassis_id); > >> @@ -6143,16 +6156,20 @@ en_neighbor_run(struct engine_node *node > OVS_UNUSED, void *data) > >> .sbrec_pb_by_dp = sbrec_port_binding_by_datapath, > >> .sbrec_amb_by_dp = sbrec_advertised_mac_binding_by_datapath, > >> .sbrec_pb_by_name = sbrec_port_binding_by_name, > >> + .sbrec_pb_by_key = sbrec_port_binding_by_key, > >> + .sbrec_fdb_by_dp_key = sbrec_fdb_by_dp_key, > >> .chassis = chassis, > >> }; > >> > >> struct neighbor_ctx_out n_ctx_out = { > >> .monitored_interfaces = &ne_data->monitored_interfaces, > >> .advertised_pbs = &ne_data->advertised_pbs, > >> + .fdb_datapaths = &ne_data->fdb_datapaths, > >> }; > >> > >> neighbor_cleanup(&ne_data->monitored_interfaces); > >> sset_clear(&ne_data->advertised_pbs); > >> + hmapx_clear(&ne_data->fdb_datapaths); > >> neighbor_run(&n_ctx_in, &n_ctx_out); > >> > >> return EN_UPDATED; > >> @@ -6283,6 +6300,35 @@ neighbor_sb_port_binding_handler(struct > engine_node *node, void *data) > >> return EN_HANDLED_UNCHANGED; > >> } > >> > >> +static enum engine_input_handler_result > >> +neighbor_sb_fdb_handler(struct engine_node *node, void *data) > >> +{ > >> + /* This handler assumes that local_datapaths have not been added or > >> + * removed in this engine run. This is guaranteed because > >> + * neighbor_runtime_data_handler() returns EN_UNHANDLED when it > detects > >> + * a new or removed datapath (TRACKED_RESOURCE_NEW / _REMOVED), > which > >> + * forces a full recompute of en_neighbor before this handler can > run. > >> + * If neighbor_runtime_data_handler() is ever changed to handle > those > >> + * cases incrementally, this handler must be updated to account for > >> + * datapaths that were not present when it looked up FDB entries. > */ > >> + struct ed_type_neighbor *ne_data = data; > >> + struct ed_type_runtime_data *rt_data = > >> + engine_get_input_data("runtime_data", node); > >> + const struct sbrec_fdb_table *fdb_table = > >> + EN_OVSDB_GET(engine_get_input("SB_fdb", node)); > >> + > >> + const struct sbrec_fdb *fdb; > >> + SBREC_FDB_TABLE_FOR_EACH_TRACKED (fdb, fdb_table) { > >> + struct local_datapath *ld = > >> + get_local_datapath(&rt_data->local_datapaths, fdb->dp_key); > >> + if (ld && hmapx_contains(&ne_data->fdb_datapaths, ld)) { > >> + return EN_UNHANDLED; > >> + } > >> + } > >> + > >> + return EN_HANDLED_UNCHANGED; > >> +} > >> + > >> static int > >> if_index_cmp(const void *a_, const void *b_) > >> { > >> @@ -7212,6 +7258,7 @@ inc_proc_ovn_controller_init( > >> neighbor_sb_datapath_binding_handler); > >> engine_add_input(&en_neighbor, &en_sb_port_binding, > >> neighbor_sb_port_binding_handler); > >> + engine_add_input(&en_neighbor, &en_sb_fdb, > neighbor_sb_fdb_handler); > >> engine_add_input(&en_neighbor_exchange, &en_neighbor, NULL); > >> engine_add_input(&en_neighbor_exchange, &en_host_if_monitor, NULL); > >> engine_add_input(&en_neighbor_exchange, &en_neighbor_table_notify, > NULL); > >> diff --git a/ovn-nb.xml b/ovn-nb.xml > >> index 9657c849d..68d4237c0 100644 > >> --- a/ovn-nb.xml > >> +++ b/ovn-nb.xml > >> @@ -1041,9 +1041,13 @@ > >> > >> <p> > >> If <code>fdb</code> is specified then ovn-controller will > advertise > >> - all workloads that are local to the chassis. The applies to > VIFs, > >> + all workloads that are local to the chassis. This applies to > VIFs, > >> container ports, virtual ports, connected DGPs and connected > GW > >> - routers. > >> + routers. Additionally, dynamically learned FDB entries > (from ports > >> + with <code>unknown</code> addresses) are also advertised. > For VIF > >> + ports, only entries learned on the local chassis are > advertised; > >> + for localnet ports, entries are advertised on all chassis > where the > >> + datapath is local. > >> </p> > >> > >> <p> > >> diff --git a/tests/ovn-inc-proc-graph-dump.at b/tests/ > ovn-inc-proc-graph-dump.at > >> index 9a1483ee0..52bccf06d 100644 > >> --- a/tests/ovn-inc-proc-graph-dump.at > >> +++ b/tests/ovn-inc-proc-graph-dump.at > >> @@ -395,6 +395,7 @@ digraph "Incremental-Processing-Engine" { > >> runtime_data -> neighbor > [[label="neighbor_runtime_data_handler"]]; > >> SB_datapath_binding -> neighbor > [[label="neighbor_sb_datapath_binding_handler"]]; > >> SB_port_binding -> neighbor > [[label="neighbor_sb_port_binding_handler"]]; > >> + SB_fdb -> neighbor [[label="neighbor_sb_fdb_handler"]]; > >> host_if_monitor [[style=filled, shape=box, fillcolor=white, > label="host_if_monitor"]]; > >> neighbor_table_notify [[style=filled, shape=box, > fillcolor=white, label="neighbor_table_notify"]]; > >> neighbor_exchange_status [[style=filled, shape=box, > fillcolor=white, label="neighbor_exchange_status"]]; > >> diff --git a/tests/system-ovn.at b/tests/system-ovn.at > >> index 67e1f76f1..5ebda085f 100644 > >> --- a/tests/system-ovn.at > >> +++ b/tests/system-ovn.at > >> @@ -18397,6 +18397,68 @@ f0:00:0f:16:01:20 dev $LO_NAME master $BR_NAME > static > >> f0:00:0f:16:01:20 dev $LO_NAME vlan 1 master $BR_NAME static > >> ]) > >> > >> +AS_BOX([L2 EVPN learned FDB advertising]) > >> + > >> +# Add a port with "unknown" addresses and FDB learning enabled. > >> +# redistribute=fdb is already set from the previous section. > >> +check ovn-nbctl --wait=hv \ > >> + -- lsp-add ls-evpn wl-unknown \ > >> + -- lsp-set-addresses wl-unknown "unknown" > >> + > >> +ADD_NAMESPACES(wl-unknown) > >> +ADD_VETH(wl-unknown, wl-unknown, br-int, "172.16.1.30/24", > "f0:00:0f:16:01:30") > >> +check ovn-nbctl --wait=hv sync > >> +wait_for_ports_up wl-unknown > >> + > >> +# Trigger FDB learning by sending traffic from the namespace. > >> +NS_CHECK_EXEC([wl-unknown], [ping -c 1 -W 1 172.16.1.10 > /dev/null > 2>&1 || true]) > >> + > >> +# Wait for the FDB entry to appear in the SB database. > >> +learned_mac=f0:00:0f:16:01:30 > >> +wait_row_count fdb 1 mac='"'$learned_mac'"' > >> + > >> +# The dynamically learned FDB MAC should be advertised on the loopback. > >> +OVS_WAIT_FOR_OUTPUT_UNQUOTED([bridge fdb show | grep $LO_NAME | grep > "$learned_mac" | sort], [0], [dnl > >> +$learned_mac dev $LO_NAME master $BR_NAME static > >> +$learned_mac dev $LO_NAME vlan 1 master $BR_NAME static > >> +]) > >> + > >> +# Disabling dynamic-routing-advertise on the port should remove it. > >> +check ovn-nbctl --wait=hv set Logical_Switch_Port wl-unknown \ > >> + options:dynamic-routing-advertise=false > >> +OVS_WAIT_FOR_OUTPUT_UNQUOTED([bridge fdb show | grep $LO_NAME | grep > "$learned_mac" | sort], [0], [dnl > >> +]) > >> + > >> +# Re-enabling should bring it back. > >> +check ovn-nbctl --wait=hv set Logical_Switch_Port wl-unknown \ > >> + options:dynamic-routing-advertise=true > >> +OVS_WAIT_FOR_OUTPUT_UNQUOTED([bridge fdb show | grep $LO_NAME | grep > "$learned_mac" | sort], [0], [dnl > >> +$learned_mac dev $LO_NAME master $BR_NAME static > >> +$learned_mac dev $LO_NAME vlan 1 master $BR_NAME static > >> +]) > >> + > >> +# Also verify that FDB entries learned on localnet ports are > advertised. > >> +# The localnet port already has addresses="unknown" from > lsp-add-localnet-port. > >> +check ovn-nbctl --wait=hv set Logical_Switch_Port ln_port \ > >> + options:localnet_learn_fdb=true > >> + > >> +# Send traffic from the physical side to trigger localnet FDB learning. > >> +ADD_NAMESPACES(ext-host) > >> +ADD_VETH(ext-host, ext-host, br-ext, "172.16.1.40/24", > "aa:bb:cc:dd:ee:ff") > >> +NS_CHECK_EXEC([ext-host], [ping -c 1 -W 1 172.16.1.10 > /dev/null 2>&1 > || true]) > >> + > >> +ln_learned_mac=aa:bb:cc:dd:ee:ff > >> +wait_row_count fdb 1 mac='"'$ln_learned_mac'"' > >> +OVS_WAIT_FOR_OUTPUT_UNQUOTED([bridge fdb show | grep $LO_NAME | grep > "$ln_learned_mac" | sort], [0], [dnl > >> +$ln_learned_mac dev $LO_NAME master $BR_NAME static > >> +$ln_learned_mac dev $LO_NAME vlan 1 master $BR_NAME static > >> +]) > >> + > >> +# Cleanup. > >> +check ovn-sbctl --all destroy FDB > >> +check ovn-nbctl --wait=hv lsp-del wl-unknown \ > >> + -- remove Logical_Switch_Port ln_port options localnet_learn_fdb > >> + > >> check ovn-nbctl --wait=hv lsp-del workload2 > >> OVS_WAIT_FOR_OUTPUT_UNQUOTED([bridge fdb show | grep $LO_NAME | grep > "f0:00:0f:16:01" | sort], [0], [dnl > >> f0:00:0f:16:01:10 dev $LO_NAME master $BR_NAME static > >> -- > >> 2.55.0 > >> > >> _______________________________________________ > >> 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
