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 > 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 >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
