mir/ovs
2
0
Fork 0
mirror of https://github.com/openvswitch/ovs synced 2025-08-31 06:15:47 +00:00
Code Issues Releases Wiki Activity

ovsdb-idl: Fix iteration over tracked rows with no actual data.

Browse Source 
When idl removes orphan rows, those rows are inserted into the
'track_list'.  This allows iterators such as *_FOR_EACH_TRACKED () to
return orphan rows that never had any data to the IDL user.  In this
case, it is difficult for the user to understand whether it is a row
with no data (there was no "insert" / "modify" for this row) or it is
a row with zero data (columns were cleared by DB transaction).

The main problem with this condition is that rows without data will
have NULL pointers instead of references that should be there according
to the database schema.  For example, ovn-controller might crash:

 ERROR: AddressSanitizer: SEGV on unknown address 0x000000000100
       (pc 0x00000055e9b2 bp 0x7ffef6180880 sp 0x7ffef6180860 T0)
 The signal is caused by a READ memory access.
 Hint: address points to the zero page.
    #0 0x55e9b1 in handle_deleted_lport /controller/binding.c
    #1 0x55e903 in handle_deleted_vif_lport /controller/binding.c:2072:5
    #2 0x55e059 in binding_handle_port_binding_changes /controller/binding.c:2155:23
    #3 0x5a6395 in runtime_data_sb_port_binding_handler /controller/ovn-controller.c:1454:10
    #4 0x5e15b3 in engine_compute /lib/inc-proc-eng.c:306:18
    #5 0x5e0faf in engine_run_node /lib/inc-proc-eng.c:352:14
    #6 0x5e0e04 in engine_run /lib/inc-proc-eng.c:377:9
    #7 0x5a03de in main /controller/ovn-controller.c
    #8 0x7f4fd9c991a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)
    #9 0x483f0d in _start (/controller/ovn-controller+0x483f0d)

It doesn't make much sense to return non-real rows to the user, so it's
best to exclude them from iteration.

Test included.  Without the fix, provided test will print empty orphan
rows that was never received by idl as tracked changes.

Fixes: 932104f483 ("ovsdb-idl: Add support for change tracking.")
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Dumitru Ceara <dceara@redhat.com>
This commit is contained in:
Ilya Maximets
2020-11-23 09:37:47 +01:00
parent 37423e4dc8
commit f0d23f6795
4 changed files with 140 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
lib/ovsdb-idl.c
View File

@@ -1892,29 +1892,37 @@ ovsdb_idl_track_is_set(struct ovsdb_idl_table *table)
}
/* Returns the first tracked row in table with class 'table_class'
* for the specified 'idl'. Returns NULL if there are no tracked rows */
* for the specified 'idl'. Returns NULL if there are no tracked rows.
* Pure orphan rows, i.e. rows that never had any datum, are skipped. */
const struct ovsdb_idl_row *
ovsdb_idl_track_get_first(const struct ovsdb_idl *idl,
const struct ovsdb_idl_table_class *table_class)
{
struct ovsdb_idl_table *table
= ovsdb_idl_db_table_from_class(&idl->data, table_class);
struct ovsdb_idl_row *row;
if (!ovs_list_is_empty(&table->track_list)) {
return CONTAINER_OF(ovs_list_front(&table->track_list), struct ovsdb_idl_row, track_node);
LIST_FOR_EACH (row, track_node, &table->track_list) {
if (!ovsdb_idl_row_is_orphan(row) || row->tracked_old_datum) {
return row;
}
}
return NULL;
}
/* Returns the next tracked row in table after the specified 'row'
* (in no particular order). Returns NULL if there are no tracked rows */
* (in no particular order). Returns NULL if there are no tracked rows.
* Pure orphan rows, i.e. rows that never had any datum, are skipped.*/
const struct ovsdb_idl_row *
ovsdb_idl_track_get_next(const struct ovsdb_idl_row *row)
{
if (row->track_node.next != &row->table->track_list) {
return CONTAINER_OF(row->track_node.next, struct ovsdb_idl_row, track_node);
}
struct ovsdb_idl_table *table = row->table;
LIST_FOR_EACH_CONTINUE (row, track_node, &table->track_list) {
if (!ovsdb_idl_row_is_orphan(row) || row->tracked_old_datum) {
return row;
}
}
return NULL;
}

15
tests/idltest.ovsschema
View File

@@ -195,6 +195,21 @@
},
"isRoot": true
},
"simple6": {
"columns" : {
"name": {"type": "string"},
"weak_ref": {
"type": {
"key": {"type": "uuid",
"refTable": "simple",
"refType": "weak"},
"min": 0,
"max": "unlimited"
}
}
},
"isRoot": true
},
"singleton" : {
"columns" : {
"name" : {

55
tests/ovsdb-idl.at
View File

@@ -967,6 +967,7 @@ AT_CHECK([grep ovsdb_idl stderr | sort], [0], [dnl
test-ovsdb|ovsdb_idl|idltest database lacks indexed table (database needs upgrade?)
test-ovsdb|ovsdb_idl|idltest database lacks link2 table (database needs upgrade?)
test-ovsdb|ovsdb_idl|idltest database lacks simple5 table (database needs upgrade?)
test-ovsdb|ovsdb_idl|idltest database lacks simple6 table (database needs upgrade?)
test-ovsdb|ovsdb_idl|idltest database lacks singleton table (database needs upgrade?)
test-ovsdb|ovsdb_idl|link1 table in idltest database lacks l2 column (database needs upgrade?)
])
@@ -1171,6 +1172,59 @@ OVSDB_CHECK_IDL_TRACK([track, simple idl, initially populated],
003: done
]])
dnl This test creates database with weak references and checks that orphan
dnl rows created for weak references are not available for iteration via
dnl list of tracked changes.
OVSDB_CHECK_IDL_TRACK([track, simple idl, initially populated, orphan weak references],
[['["idltest",
{"op": "insert",
"table": "simple",
"row": {"s": "row0_s"},
"uuid-name": "weak_row0"},
{"op": "insert",
"table": "simple",
"row": {"s": "row1_s"},
"uuid-name": "weak_row1"},
{"op": "insert",
"table": "simple",
"row": {"s": "row2_s"},
"uuid-name": "weak_row2"},
{"op": "insert",
"table": "simple6",
"row": {"name": "first_row",
"weak_ref": ["set",
[["named-uuid", "weak_row0"],
["named-uuid", "weak_row1"],
["named-uuid", "weak_row2"]]
]}}]']],
[['condition simple []' \
'condition simple [["s","==","row1_s"]]' \
'["idltest",
{"op": "update",
"table": "simple6",
"where": [],
"row": {"name": "new_name"}}]' \
'["idltest",
{"op": "delete",
"table": "simple6",
"where": []}]']],
[[000: change conditions
001: inserted row: uuid=<0>
001: name=first_row weak_ref=[] uuid=<0>
001: updated columns: name weak_ref
002: change conditions
003: i=0 r=0 b=false s=row1_s u=<1> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<2>
003: inserted row: uuid=<2>
003: name=first_row weak_ref=[<2>] uuid=<0>
003: updated columns: s
004: {"error":null,"result":[{"count":1}]}
005: name=new_name weak_ref=[<2>] uuid=<0>
005: updated columns: name
006: {"error":null,"result":[{"count":1}]}
007: i=0 r=0 b=false s=row1_s u=<1> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<2>
008: done
]])
OVSDB_CHECK_IDL_TRACK([track, simple idl, initially empty, various ops],
[],
[['["idltest",
@@ -1246,6 +1300,7 @@ OVSDB_CHECK_IDL_TRACK([track, simple idl, initially empty, various ops],
010: updated columns: s
011: {"error":null,"result":[{"count":1}]}
012: deleted row: uuid=<1>
012: i=0 r=123.5 b=true s=newstring u=<5> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1>
013: reconnect
014: i=-1 r=125 b=false s=newstring u=<5> ia=[1] ra=[1.5] ba=[false] sa=[] ua=[] uuid=<6>
014: i=1 r=123.5 b=true s=mystring u=<2> ia=[1 2 3] ra=[-0.5] ba=[true] sa=[abc def] ua=[<3> <4>] uuid=<0>

61
tests/test-ovsdb.c
View File

@@ -1904,6 +1904,26 @@ print_idl_row_updated_link2(const struct idltest_link2 *l2, int step)
}
}
static void
print_idl_row_updated_simple6(const struct idltest_simple6 *s6, int step)
{
size_t i;
bool updated = false;
for (i = 0; i < IDLTEST_SIMPLE6_N_COLUMNS; i++) {
if (idltest_simple6_is_updated(s6, i)) {
if (!updated) {
printf("%03d: updated columns:", step);
updated = true;
}
printf(" %s", idltest_simple6_columns[i].name);
}
}
if (updated) {
printf("\n");
}
}
static void
print_idl_row_updated_singleton(const struct idltest_singleton *sng, int step)
{
@@ -1991,6 +2011,22 @@ print_idl_row_link2(const struct idltest_link2 *l2, int step)
print_idl_row_updated_link2(l2, step);
}
static void
print_idl_row_simple6(const struct idltest_simple6 *s6, int step)
{
int i;
printf("%03d: name=%s ", step, s6->name);
printf("weak_ref=[");
for (i = 0; i < s6->n_weak_ref; i++) {
printf("%s"UUID_FMT, i ? " " : "",
UUID_ARGS(&s6->weak_ref[i]->header_.uuid));
}
printf("] uuid="UUID_FMT"\n", UUID_ARGS(&s6->header_.uuid));
print_idl_row_updated_simple6(s6, step);
}
static void
print_idl_row_singleton(const struct idltest_singleton *sng, int step)
{
@@ -2032,21 +2068,20 @@ print_idl(struct ovsdb_idl *idl, int step)
static void
print_idl_track(struct ovsdb_idl *idl, int step)
{
const struct idltest_simple6 *s6;
const struct idltest_simple *s;
const struct idltest_link1 *l1;
const struct idltest_link2 *l2;
int n = 0;
IDLTEST_SIMPLE_FOR_EACH_TRACKED (s, idl) {
print_idl_row_simple(s, step);
if (idltest_simple_is_deleted(s)) {
printf("%03d: deleted row: uuid="UUID_FMT"\n", step,
UUID_ARGS(&s->header_.uuid));
} else {
print_idl_row_simple(s, step);
if (idltest_simple_is_new(s)) {
printf("%03d: inserted row: uuid="UUID_FMT"\n", step,
UUID_ARGS(&s->header_.uuid));
}
} else if (idltest_simple_is_new(s)) {
printf("%03d: inserted row: uuid="UUID_FMT"\n", step,
UUID_ARGS(&s->header_.uuid));
}
n++;
}
@@ -2077,6 +2112,18 @@ print_idl_track(struct ovsdb_idl *idl, int step)
}
n++;
}
IDLTEST_SIMPLE6_FOR_EACH_TRACKED (s6, idl) {
print_idl_row_simple6(s6, step);
if (idltest_simple6_is_deleted(s6)) {
printf("%03d: deleted row: uuid="UUID_FMT"\n", step,
UUID_ARGS(&s6->header_.uuid));
} else if (idltest_simple6_is_new(s6)) {
printf("%03d: inserted row: uuid="UUID_FMT"\n", step,
UUID_ARGS(&s6->header_.uuid));
}
n++;
}
if (!n) {
printf("%03d: empty\n", step);
}
@@ -2298,6 +2345,8 @@ find_table_class(const char *name)
return &idltest_table_link1;
} else if (!strcmp(name, "link2")) {
return &idltest_table_link2;
} else if (!strcmp(name, "simple6")) {
return &idltest_table_simple6;
}
return NULL;
}