Thanks for the v2, Ales. Acked-by: Mark Michelson <[email protected]>
I pushed this to main, branch-26.09, and branch-26.03. On Fri, Oct 9, 2026 at 4:14 AM Ales Musil via dev <[email protected]> wrote: > > The FDB entries would be cleaned up when the port or datapath > was removed. However, that wasn't enough as there might have been > stale entries when the port changed address from "unknown" or would > be disabled. Make sure we do a proper cleanup for those changes too. > > Fixes: 679d3550303a ("northd: Cleanup stale FDB entries.") > Reported-at: https://redhat.atlassian.net/browse/FDP-4432 > Signed-off-by: Ales Musil <[email protected]> > --- > v2: Fix the wrong type inside vector. > --- > northd/en-northd.c | 29 ++++++++++++++-------------- > northd/northd.c | 32 ++++++++++++++++++++----------- > northd/northd.h | 2 ++ > tests/ovn-northd.at | 46 +++++++++++++++++++++++++++++++++++++-------- > 4 files changed, 75 insertions(+), 34 deletions(-) > > diff --git a/northd/en-northd.c b/northd/en-northd.c > index 480dc61ca..96859e4ae 100644 > --- a/northd/en-northd.c > +++ b/northd/en-northd.c > @@ -694,32 +694,31 @@ northd_sb_fdb_change_handler(struct engine_node *node, > void *data) > const struct sbrec_fdb_table *sbrec_fdb_table = > EN_OVSDB_GET(engine_get_input("SB_fdb", node)); > > + struct vector to_remove = > + VECTOR_EMPTY_INITIALIZER(const struct sbrec_fdb *); > + > /* check if changed rows are stale and delete them */ > - const struct sbrec_fdb *fdb_e, *fdb_prev_del = NULL; > + const struct sbrec_fdb *fdb_e; > SBREC_FDB_TABLE_FOR_EACH_TRACKED (fdb_e, sbrec_fdb_table) { > if (sbrec_fdb_is_deleted(fdb_e)) { > continue; > } > > - if (fdb_prev_del) { > - sbrec_fdb_delete(fdb_prev_del); > - } > - > - fdb_prev_del = fdb_e; > - struct ovn_datapath *od > - = ovn_datapath_find_by_key(&nd->ls_datapaths.datapaths, > - fdb_e->dp_key); > - if (od) { > - if (ovn_tnlid_present(&od->port_tnlids, fdb_e->port_key)) { > - fdb_prev_del = NULL; > - } > + struct ovn_datapath *od = > + ovn_datapath_find_by_key(&nd->ls_datapaths.datapaths, > + fdb_e->dp_key); > + if (!od || ovn_datapath_is_stale(od) || > + !ovn_tnlid_present(&od->fdb_ports_tnlids, fdb_e->port_key)) { > + vector_push(&to_remove, &fdb_e); > } > } > > - if (fdb_prev_del) { > - sbrec_fdb_delete(fdb_prev_del); > + VECTOR_FOR_EACH (&to_remove, fdb_e) { > + sbrec_fdb_delete(fdb_e); > } > > + vector_destroy(&to_remove); > + > return EN_HANDLED_UNCHANGED; > } > > diff --git a/northd/northd.c b/northd/northd.c > index e6a2333b5..7c4cb5a62 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -617,6 +617,7 @@ ovn_datapath_create(struct hmap *datapaths, const struct > uuid *key, > od->sdp = sdp; > od->nbs = nbs; > od->nbr = nbr; > + hmap_init(&od->fdb_ports_tnlids); > hmap_init(&od->port_tnlids); > od->port_key_hint = 0; > hmap_insert(datapaths, &od->key_node, uuid_hash(&od->key)); > @@ -653,6 +654,7 @@ ovn_datapath_destroy(struct ovn_datapath *od) > /* Don't remove od->list. It is used within build_datapaths() as a > * private list and once we've exited that function it is not safe to > * use it. */ > + ovn_destroy_tnlids(&od->fdb_ports_tnlids); > ovn_destroy_tnlids(&od->port_tnlids); > destroy_ipam_info(&od->ipam_info); > vector_destroy(&od->router_ports); > @@ -1163,6 +1165,7 @@ ovn_port_cleanup(struct ovn_port *port) > if (port->tunnel_key) { > ovs_assert(port->od); > ovn_free_tnlid(&port->od->port_tnlids, port->tunnel_key); > + ovn_free_tnlid(&port->od->fdb_ports_tnlids, port->tunnel_key); > port->tunnel_key = 0; > } > for (int i = 0; i < port->n_lsp_addrs; i++) { > @@ -3135,16 +3138,10 @@ cleanup_stale_fdb_entries(const struct > sbrec_fdb_table *sbrec_fdb_table, > { > const struct sbrec_fdb *fdb_e; > SBREC_FDB_TABLE_FOR_EACH_SAFE (fdb_e, sbrec_fdb_table) { > - bool delete = true; > - struct ovn_datapath *od > - = ovn_datapath_find_by_key(ls_datapaths, fdb_e->dp_key); > - if (od) { > - if (ovn_tnlid_present(&od->port_tnlids, fdb_e->port_key)) { > - delete = false; > - } > - } > - > - if (delete) { > + struct ovn_datapath *od = > + ovn_datapath_find_by_key(ls_datapaths, fdb_e->dp_key); > + if (!od || ovn_datapath_is_stale(od) || > + !ovn_tnlid_present(&od->fdb_ports_tnlids, fdb_e->port_key)) { > sbrec_fdb_delete(fdb_e); > } > } > @@ -4369,6 +4366,12 @@ ovn_port_add_tnlid(struct ovn_port *op, uint32_t > tunnel_key) > if (tunnel_key > op->od->port_key_hint) { > op->od->port_key_hint = tunnel_key; > } > + > + /* Track the assigned tunnel_key for enabled LSP > + * with unknown address. */ > + if (op->nbsp && lsp_is_enabled(op->nbsp) && op->has_unknown) { > + ovs_assert(ovn_add_tnlid(&op->od->fdb_ports_tnlids, tunnel_key)); > + } > } > return added; > } > @@ -4421,6 +4424,11 @@ ovn_port_allocate_key(struct ovn_port *op) > if (!op->tunnel_key) { > return false; > } > + > + if (op->nbsp && lsp_is_enabled(op->nbsp) && op->has_unknown) { > + ovs_assert(ovn_add_tnlid(&op->od->fdb_ports_tnlids, > + op->tunnel_key)); > + } > } > return true; > } > @@ -5091,7 +5099,9 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn > *ovnsb_idl_txn, > } > add_op_to_northd_tracked_ports(&trk_lsps->updated, op); > > - if (old_tunnel_key != op->tunnel_key) { > + if (old_tunnel_key != op->tunnel_key || > + !lsp_is_enabled(op->nbsp) || > + !op->has_unknown) { > delete_fdb_entries(ni->sbrec_fdb_by_dp_and_port, > od->tunnel_key, old_tunnel_key); > } > diff --git a/northd/northd.h b/northd/northd.h > index a2b8a0c93..3d1fd2d31 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -421,6 +421,8 @@ struct ovn_datapath { > struct vector router_ports; /* Vector of struct ovn_port *. */ > struct vector switch_ports; /* Vector of struct ovn_port * of > * type 'switch'. */ > + struct hmap fdb_ports_tnlids; /* Tunnel keys for enabled LSP with > + * unknown address. */ > struct hmap port_tnlids; > uint32_t port_key_hint; > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 6866068fa..ec04d4d67 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -5344,28 +5344,38 @@ AT_SETUP([FDB cleanup]) > ovn_start > > check ovn-nbctl ls-add sw0 > -check ovn-nbctl lsp-add sw0 sw0-p1 > -check ovn-nbctl lsp-add sw0 sw0-p2 > -check ovn-nbctl lsp-add sw0 sw0-p3 > +check ovn-nbctl lsp-add sw0 sw0-p1 -- lsp-set-addresses sw0-p1 unknown > +check ovn-nbctl lsp-add sw0 sw0-p2 -- lsp-set-addresses sw0-p2 unknown > +check ovn-nbctl lsp-add sw0 sw0-p3 -- lsp-set-addresses sw0-p3 unknown \ > + -- set Logical_Switch_Port sw0-p3 enabled=false > +check ovn-nbctl lsp-add sw0 sw0-p4 > > check ovn-nbctl ls-add sw1 > -check ovn-nbctl lsp-add sw1 sw1-p1 > -check ovn-nbctl lsp-add sw1 sw1-p2 > -check ovn-nbctl --wait=sb lsp-add sw1 sw1-p3 > +check ovn-nbctl lsp-add sw1 sw1-p1 -- lsp-set-addresses sw1-p1 unknown > +check ovn-nbctl --wait=sb sync > > sw0_key=$(fetch_column datapath_binding tunnel_key external_ids:name=sw0) > sw1_key=$(fetch_column datapath_binding tunnel_key external_ids:name=sw1) > sw0p1_key=$(fetch_column port_binding tunnel_key logical_port=sw0-p1) > sw0p2_key=$(fetch_column port_binding tunnel_key logical_port=sw0-p2) > +sw0p3_key=$(fetch_column port_binding tunnel_key logical_port=sw0-p3) > +sw0p4_key=$(fetch_column port_binding tunnel_key logical_port=sw0-p4) > sw1p1_key=$(fetch_column port_binding tunnel_key logical_port=sw1-p1) > > check_uuid ovn-sbctl create FDB mac="00\:00\:00\:00\:00\:01" dp_key=$sw0_key > port_key=$sw0p1_key > check_uuid ovn-sbctl create FDB mac="00\:00\:00\:00\:00\:02" dp_key=$sw0_key > port_key=$sw0p1_key > check_uuid ovn-sbctl create FDB mac="00\:00\:00\:00\:00\:03" dp_key=$sw0_key > port_key=$sw0p2_key > +check_uuid ovn-sbctl create FDB mac="00\:00\:00\:00\:00\:04" dp_key=$sw0_key > port_key=$sw0p3_key > +check_uuid ovn-sbctl create FDB mac="00\:00\:00\:00\:00\:05" dp_key=$sw0_key > port_key=$sw0p4_key > check_uuid ovn-sbctl create FDB mac="00\:00\:00\:00\:01\:01" dp_key=$sw1_key > port_key=$sw1p1_key > check_uuid ovn-sbctl create FDB mac="00\:00\:00\:00\:01\:02" dp_key=$sw1_key > port_key=$sw1p1_key > check_uuid ovn-sbctl create FDB mac="00\:00\:00\:00\:01\:03" dp_key=$sw1_key > port_key=$sw1p1_key > > +# Disabled port should clear FDB. > +wait_row_count FDB 0 dp_key=$sw0_key port_key=$sw0p3_key > +# Port without "unknown address" should clear FDB. > +wait_row_count FDB 0 dp_key=$sw0_key port_key=$sw0p4_key > + > wait_row_count FDB 6 > > AT_CHECK([ovn-sbctl create fdb mac="00\:00\:00\:00\:01\:03" dp_key=$sw1_key > port_key=10], [1], [ignore], [ignore]) > @@ -5383,12 +5393,32 @@ check ovn-nbctl lsp-del sw0-p1 > wait_row_count FDB 1 > > check_column '00:00:00:00:00:03' FDB mac > -ovn-sbctl list fdb > +ovn-sbctl list FDB > > check_column $sw0_key FDB dp_key > check_column $sw0p2_key FDB port_key > > -check ovn-nbctl --wait=sb lsp-add sw0 sw0-p1 > +check ovn-nbctl --wait=sb lsp-add sw0 sw0-p1 -- lsp-set-addresses sw0-p1 > unknown > +sw0p1_key=$(fetch_column port_binding tunnel_key logical_port=sw0-p1) > +wait_row_count FDB 1 > + > +check_uuid ovn-sbctl create FDB mac="00\:00\:00\:00\:00\:01" dp_key=$sw0_key > port_key=$sw0p1_key > +check_uuid ovn-sbctl create FDB mac="00\:00\:00\:00\:00\:02" dp_key=$sw0_key > port_key=$sw0p1_key > +wait_row_count FDB 3 > + > +# Disabling clears FDB entries. > +check ovn-nbctl --wait=sb set Logical_Switch_Port sw0-p1 enabled=false > +check_uuid ovn-sbctl create FDB mac="00\:00\:00\:00\:00\:01" dp_key=$sw0_key > port_key=$sw0p1_key > +check_uuid ovn-sbctl create FDB mac="00\:00\:00\:00\:00\:02" dp_key=$sw0_key > port_key=$sw0p1_key > +wait_row_count FDB 1 > + > +check ovn-nbctl --wait=sb set Logical_Switch_Port sw0-p1 enabled=true > +check_uuid ovn-sbctl create FDB mac="00\:00\:00\:00\:00\:01" dp_key=$sw0_key > port_key=$sw0p1_key > +check_uuid ovn-sbctl create FDB mac="00\:00\:00\:00\:00\:02" dp_key=$sw0_key > port_key=$sw0p1_key > +wait_row_count FDB 3 > + > +# Removing unknown clears FDB entries. > +check ovn-nbctl --wait=sb lsp-set-addresses sw0-p1 "00:00:00:00:00:10 > 192.168.100.10" > wait_row_count FDB 1 > > check ovn-nbctl lsp-del sw0-p2 > -- > 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
