[ovs-dev] [PATCH] ovsdb-idl: Fix iteration over tracked rows with no actual data.

[Ilya Maximets]

On 11/23/20 10:49 AM, Dumitru Ceara wrote:
> On 11/23/20 9:37 AM, Ilya Maximets wrote:
>> 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: 932104f483ef ("ovsdb-idl: Add support for change tracking.")
>> Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
>> ---
> 
> Looks good to me, thanks!
> 
> Acked-by: Dumitru Ceara <dceara at redhat.com>
> 

Thanks!

Applied to master and backported down to 2.12.  It doesn't apply cleanly
on branches below 2.12 and there is some missing functionality on these
branches.  But OVN 2.11 doesn't use iterators over tracked changes, so
it should be fine.

Best regards, Ilya Maximets.