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

[Dumitru Ceara]

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>