[ovs-dev] [PATCH] ovsdb-idl: Mark row "parsed" in ovsdb_idl_txn_write__

Dumitru Ceara dceara at redhat.com
Wed Jul 17 19:05:04 UTC 2019


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>
---
 lib/ovsdb-idl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 5c61096..1a6a4af 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -4487,6 +4487,7 @@ ovsdb_idl_txn_write__(const struct ovsdb_idl_row *row_,
     }
     (column->unparse)(row);
     (column->parse)(row, &row->new_datum[column_idx]);
+    row->parsed = true;
     if (!index_row) {
         ovsdb_idl_add_to_indexes(row);
     }
-- 
1.8.3.1



More information about the dev mailing list