IDL may contain deleted or orphan rows that should never be visible
to the user. However, ovsdb_idl_get_row_for_uuid() function simply
looks up the row by UUID and returns it without checking if the row
actually exists in the IDL. This is causing a crash on assertion
failure in ovn-controller when it looks up and finds port binding
records that were already deleted:
5 vlog_abort at lib/vlog.c:1325
6 ovs_assert_failure at lib/util.c:90
7 ovsdb_idl_txn_write__.constprop.0 at lib/ovsdb-idl.c:3650
8 ovsdb_idl_txn_write at lib/ovsdb-idl.c:3742
9 sbrec_port_binding_set_up at lib/ovn-sb-idl.c:39665
10 port_binding_set_down at controller/binding.c:3700
11 if_status_mgr_update at controller/if-status.c:645
12 main at controller/ovn-controller.c:7544
7 ovsdb_idl_txn_write__.constprop.0 at lib/ovsdb-idl.c:3650
3650 ovs_assert(row->new_datum != NULL);
Can be easily reproduced with ovs-vsctl:
$ ovs-vsctl add-br br-int
$ ovs-vsctl del-br br-int \
-- set bridge $(ovs-vsctl get bridge br-int _uuid) other-config:a=b
ovs-vsctl: lib/ovsdb-idl.c:2673:
assertion row->new_datum != NULL failed in ovsdb_idl_read()
Aborted (core dumped)
Fix that by adding an extra check for row existence like in IDL
iterators, so deleted or orphan rows can no longer be found.
Fixes: 979821c0a6b0 ("ovsdb-idl: Allow clients to modify records without using
structs.")
Reported-by: Dumitru Ceara <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
---
lib/ovsdb-idl.c | 5 ++++-
tests/ovs-vsctl.at | 16 ++++++++++++++++
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 7f49e922d..d8094458d 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -2602,7 +2602,10 @@ ovsdb_idl_get_row_for_uuid(const struct ovsdb_idl *idl,
const struct ovsdb_idl_table_class *tc,
const struct uuid *uuid)
{
- return ovsdb_idl_get_row(ovsdb_idl_table_from_class(idl, tc), uuid);
+ const struct ovsdb_idl_row *row;
+
+ row = ovsdb_idl_get_row(ovsdb_idl_table_from_class(idl, tc), uuid);
+ return (row && ovsdb_idl_row_exists(row)) ? row : NULL;
}
static struct ovsdb_idl_row *
diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index abc102510..4ca5261c2 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -417,6 +417,22 @@ CHECK_PORTS([a])
OVS_VSCTL_CLEANUP
AT_CLEANUP
+AT_SETUP([add-br a, del-br a + set external-ids on deleted bridge])
+AT_KEYWORDS([ovs-vsctl])
+OVS_VSCTL_SETUP
+AT_CHECK([RUN_OVS_VSCTL([add-br a])])
+AT_CHECK([RUN_OVS_VSCTL_TOGETHER(
+ [del-br a],
+ [set bridge $(RUN_OVS_VSCTL([get bridge a _uuid]))
external-ids:key0=value0])],
+ [1], [], [stderr])
+AT_CHECK([uuidfilt stderr], [0], [dnl
+ovs-vsctl: no row "<0>" in table Bridge
+])
+CHECK_BRIDGES([a, a, 0])
+CHECK_PORTS([a])
+OVS_VSCTL_CLEANUP
+AT_CLEANUP
+
AT_SETUP([external IDs])
AT_KEYWORDS([ovs-vsctl])
OVS_VSCTL_SETUP
--
2.51.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
