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]>
---
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..c8c57ea5b 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(struct sbrec_fdb_table *);
+
/* 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
