[ovs-git] [openvswitch/ovs] 300ac0: ovsdb-idl: Mark row "parsed" in ovsdb_idl_txn_write__

[Dumitru Ceara]

  Branch: refs/heads/master
  Home:   https://github.com/openvswitch/ovs
  Commit: 300ac0e1847d99d4332460002856f38e8102389f
      https://github.com/openvswitch/ovs/commit/300ac0e1847d99d4332460002856f38e8102389f
  Author: Dumitru Ceara <dceara at redhat.com>
  Date:   2019-07-18 (Thu, 18 Jul 2019)

  Changed paths:
    M lib/ovsdb-idl.c

  Log Message:
  -----------
  ovsdb-idl: Mark row "parsed" in ovsdb_idl_txn_write__

Once a column is set in a row using ovsdb_idl_txn_write__ we also mark
the row "parsed". Otherwise the memory allocated by
sbrec_<table>_parse_<col>() will never be freed. After marking the row
"parsed", the ovsdb_idl_txn_disassemble function will properly free the
newly allocated memory for the column (ovsdb_idl_row_unparse).

The problem is present only for rows that are inserted by the
running application because rows that are loaded from the database
will always have row->parsed == true.

One way to detect the leak is to start northd with valgrind:

valgrind --tool=memcheck --leak-check=yes ./ovn-northd

Then create a logical switch and bind a logical port to it:

ovn-nbctl ls-add ls1
ovn-nbctl lsp-add ls1 ls1-vm1

The valgrind report:

==9270== Memcheck, a memory error detector
==9270== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==9270== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright
info
==9270== Command: ./ovn-northd
==9270==

<snip>

==9270==
==9270== 8 bytes in 1 blocks are definitely lost in loss record 30 of
292
==9270==    at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
==9270==    by 0x4D31EF: xmalloc (util.c:138)
==9270==    by 0x45CB8E: sbrec_multicast_group_parse_ports (ovn-sb-idl.c:18141)
==9270==    by 0x4BB12D: ovsdb_idl_txn_write__ (ovsdb-idl.c:4489)
==9270==    by 0x4BB1B5: ovsdb_idl_txn_write (ovsdb-idl.c:4527)
==9270==    by 0x45D167: sbrec_multicast_group_set_ports (ovn-sb-idl.c:18561)
==9270==    by 0x40F416: ovn_multicast_update_sbrec (ovn-northd.c:2947)
==9270==    by 0x41FC55: build_lflows (ovn-northd.c:7981)
==9270==    by 0x421830: ovnnb_db_run (ovn-northd.c:8522)
==9270==    by 0x422B2D: ovn_db_run (ovn-northd.c:9089)
==9270==    by 0x423909: main (ovn-northd.c:9409)
==9270==
==9270== 157 (32 direct, 125 indirect) bytes in 1 blocks are definitely
lost in loss record 199 of 292
==9270==    at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
==9270==    by 0x4D31EF: xmalloc (util.c:138)
==9270==    by 0x471E3D: resize (hmap.c:100)
==9270==    by 0x4720C8: hmap_expand_at (hmap.c:175)
==9270==    by 0x4C74F1: hmap_insert_at (hmap.h:277)
==9270==    by 0x4C825A: smap_add__ (smap.c:392)
==9270==    by 0x4C7783: smap_add (smap.c:55)
==9270==    by 0x451054: sbrec_datapath_binding_parse_external_ids (ovn-sb-idl.c:7181)
==9270==    by 0x4BB12D: ovsdb_idl_txn_write__ (ovsdb-idl.c:4489)
==9270==    by 0x4BB1B5: ovsdb_idl_txn_write (ovsdb-idl.c:4527)
==9270==    by 0x451436: sbrec_datapath_binding_set_external_ids (ovn-sb-idl.c:7444)
==9270==    by 0x4090F1: ovn_datapath_update_external_ids (ovn-northd.c:817)
==9270==

<snip>

==9270==
==9270== LEAK SUMMARY:
==9270==    definitely lost: 1,322 bytes in 47 blocks
==9270==    indirectly lost: 4,653 bytes in 240 blocks
==9270==      possibly lost: 0 bytes in 0 blocks
==9270==    still reachable: 254,004 bytes in 7,780 blocks
==9270==         suppressed: 0 bytes in 0 blocks
==9270== Reachable blocks (those to which a pointer was found) are not
shown.
==9270== To see them, rerun with: --leak-check=full
--show-leak-kinds=all
==9270==
==9270== For counts of detected and suppressed errors, rerun with: -v
==9270== ERROR SUMMARY: 9 errors from 9 contexts (suppressed: 0 from 0)

Signed-off-by: Dumitru Ceara <dceara at redhat.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>