| Message ID | 20230629021800.88257-1-hzhou@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] northd.c: Fix memory leak when falling back to recompute during LSP deletion. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On 6/29/23 04:18, Han Zhou wrote: > When multiple LSP deletions are handled in incremental processing, if it > hits a LSP that can't be incrementally processed after incrementally > processing some LSP deletions, it falls back to recompute without > destroying the ovn_port objects that are already handled in the handler, > resulting in memory leaks. See example below, which is detected by the > new test case added by this patch when running with address sanitizer. > > ======================= > > Indirect leak of 936 byte(s) in 3 object(s) allocated from: > #0 0x55bce7 in calloc (/home/hanzhou/src/ovn/_build_as/northd/ovn-northd+0x55bce7) > #1 0x773f4e in xcalloc__ /home/hanzhou/src/ovs/_build/../lib/util.c:121:31 > #2 0x773f4e in xzalloc__ /home/hanzhou/src/ovs/_build/../lib/util.c:131:12 > #3 0x773f4e in xzalloc /home/hanzhou/src/ovs/_build/../lib/util.c:165:12 > #4 0x60106c in en_northd_run /home/hanzhou/src/ovn/_build_as/../northd/en-northd.c:137:5 > #5 0x61c6a8 in engine_recompute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:415:5 > #6 0x61bee0 in engine_compute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:454:17 > #7 0x61bee0 in engine_run_node /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:503:14 > #8 0x61bee0 in engine_run /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:528:9 > #9 0x605e23 in inc_proc_northd_run /home/hanzhou/src/ovn/_build_as/../northd/inc-proc-northd.c:317:9 > #10 0x5fe43b in main /home/hanzhou/src/ovn/_build_as/../northd/ovn-northd.c:934:33 > #11 0x7f473933c1a1 in __libc_start_main (/lib64/libc.so.6+0x281a1) > > Indirect leak of 204 byte(s) in 3 object(s) allocated from: > #0 0x55bea8 in realloc (/home/hanzhou/src/ovn/_build_as/northd/ovn-northd+0x55bea8) > #1 0x773c7d in xrealloc__ /home/hanzhou/src/ovs/_build/../lib/util.c:147:9 > #2 0x773c7d in xrealloc /home/hanzhou/src/ovs/_build/../lib/util.c:179:12 > #3 0x614bd4 in extract_addresses /home/hanzhou/src/ovn/_build_as/../lib/ovn-util.c:228:12 > #4 0x614bd4 in extract_lsp_addresses /home/hanzhou/src/ovn/_build_as/../lib/ovn-util.c:243:20 > #5 0x5c8d90 in parse_lsp_addrs /home/hanzhou/src/ovn/_build_as/../northd/northd.c:2468:21 > #6 0x5b2ebf in join_logical_ports /home/hanzhou/src/ovn/_build_as/../northd/northd.c:2594:13 > #7 0x5b2ebf in build_ports /home/hanzhou/src/ovn/_build_as/../northd/northd.c:4711:5 > #8 0x5b2ebf in ovnnb_db_run /home/hanzhou/src/ovn/_build_as/../northd/northd.c:17376:5 > #9 0x60106c in en_northd_run /home/hanzhou/src/ovn/_build_as/../northd/en-northd.c:137:5 > #10 0x61c6a8 in engine_recompute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:415:5 > #11 0x61bee0 in engine_compute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:454:17 > #12 0x61bee0 in engine_run_node /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:503:14 > #13 0x61bee0 in engine_run /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:528:9 > #14 0x605e23 in inc_proc_northd_run /home/hanzhou/src/ovn/_build_as/../northd/inc-proc-northd.c:317:9 > #15 0x5fe43b in main /home/hanzhou/src/ovn/_build_as/../northd/ovn-northd.c:934:33 > #16 0x7f473933c1a1 in __libc_start_main (/lib64/libc.so.6+0x281a1) > ... > > Fixes: b337750e45be ("northd: Incremental processing of VIF changes in 'northd' node.") > Reported-by: Dumitru Ceara <dceara@redhat.com> > Signed-off-by: Han Zhou <hzhou@ovn.org> > --- Thanks for the fix, Han! > northd/northd.c | 12 +++++++++--- > tests/ovn-northd.at | 45 +++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 54 insertions(+), 3 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index a45c8b53ad4e..dc1271831ab9 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -5037,6 +5037,7 @@ northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, > { > const struct nbrec_logical_switch *changed_ls; > struct ls_change *ls_change = NULL; > + struct ovn_port *op; > > NBREC_LOGICAL_SWITCH_TABLE_FOR_EACH_TRACKED (changed_ls, > ni->nbrec_logical_switch_table) { > @@ -5067,7 +5068,6 @@ northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, > ovs_list_init(&ls_change->deleted_ports); > ovs_list_init(&ls_change->updated_ports); > > - struct ovn_port *op; > HMAP_FOR_EACH (op, dp_node, &od->ports) { > op->visited = false; > } > @@ -5133,12 +5133,12 @@ northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, > HMAP_FOR_EACH_SAFE (op, dp_node, &od->ports) { > if (!op->visited) { > if (!op->lsp_can_be_inc_processed) { > - goto fail; > + goto fail_clean_deleted; > } > if (sset_contains(&nd->svc_monitor_lsps, op->key)) { > /* This port was used for svc monitor, which may be > * impacted by this deletion. Fallback to recompute. */ > - goto fail; > + goto fail_clean_deleted; > } > ovs_list_push_back(&ls_change->deleted_ports, > &op->list); > @@ -5165,6 +5165,12 @@ northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, > } > return true; > > +fail_clean_deleted: > + LIST_FOR_EACH_SAFE (op, list, &ls_change->deleted_ports) { > + ovs_list_remove(&op->list); Nit: I'd use LIST_FOR_EACH_POP here and skip the ovs_list_remove(). > + ovn_port_destroy_orphan(op); > + } > + > fail: > free(ls_change); > destroy_northd_data_tracked_changes(nd); > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index ada2d2a4aa5e..c0d4054413bb 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -9559,6 +9559,51 @@ OVN_CLEANUP([hv1]) > AT_CLEANUP > ]) > > +OVN_FOR_EACH_NORTHD_NO_HV([ > +AT_SETUP([LSP incremental processing fallback to recompute]) > +ovn_start > + > +net_add n1 > +sim_add hv1 > +as hv1 > +ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.11 > + > +check ovn-nbctl --wait=hv ls-add ls0 > + > +# Add ports that should be incrementally processed > +for i in $(seq 9); do Can we use a non-hardcoded value here, e.g.: n_ports=9 > + check ovn-nbctl --wait=hv lsp-add ls0 lsp0-$i -- lsp-set-addresses lsp0-$i "aa:aa:aa:00:00:0$i 192.168.0.1$i" > + ovs-vsctl add-port br-int lsp0-$i -- set interface lsp0-$i external_ids:iface-id=lsp0-$i > +done > + > +# add a port that should not be incrementally processed (because of the Nit: s/add/Add > +# "unknown" that is not supported by i-p for now). Just a note, I'm not sure if we can fix this but if we ever add I-P support for "unknown" this test will not be relevant anymore. > +check ovn-nbctl --wait=hv lsp-add ls0 lsp0-foo -- lsp-set-addresses lsp0-foo "unknown" > +ovs-vsctl add-port br-int lsp0-foo -- set interface lsp0-foo external_ids:iface-id=lsp0-foo > + > +wait_for_ports_up > +check ovn-nbctl --wait=hv sync > +wait_row_count port_binding 10 Same hardcoded comment, we could re-write as: wait_row_count port_binding $((n_ports + 1)) > + > +# Delete multiple ports, and one of them not incrementally processible. This is > +# to trigger partial I-P and then fall back to recompute. > +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats > +args="--wait=hv lsp-del lsp0-foo" > +for i in $(seq 9); do Here too. > + args="$args -- lsp-del lsp0-$i" > +done > +check ovn-nbctl $args > + > +wait_row_count Port_Binding 0 > +northd_recomp=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats northd recompute) > +echo northd_recomp $northd_recomp > +AT_CHECK([test $northd_recomp -ge 1]) > + > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > +]) > + > OVN_FOR_EACH_NORTHD_NO_HV([ > AT_SETUP([check fip and lb flows]) > AT_KEYWORDS([fip-lb-flows]) All the above are minor things, if you address them feel free to add my ack before pushing the patch to main: Acked-by: Dumitru Ceara <dceara@redhat.com> Thanks, Dumitru
On Thu, Jun 29, 2023 at 4:23 AM Dumitru Ceara <dceara@redhat.com> wrote: > > On 6/29/23 04:18, Han Zhou wrote: > > When multiple LSP deletions are handled in incremental processing, if it > > hits a LSP that can't be incrementally processed after incrementally > > processing some LSP deletions, it falls back to recompute without > > destroying the ovn_port objects that are already handled in the handler, > > resulting in memory leaks. See example below, which is detected by the > > new test case added by this patch when running with address sanitizer. > > > > ======================= > > > > Indirect leak of 936 byte(s) in 3 object(s) allocated from: > > #0 0x55bce7 in calloc (/home/hanzhou/src/ovn/_build_as/northd/ovn-northd+0x55bce7) > > #1 0x773f4e in xcalloc__ /home/hanzhou/src/ovs/_build/../lib/util.c:121:31 > > #2 0x773f4e in xzalloc__ /home/hanzhou/src/ovs/_build/../lib/util.c:131:12 > > #3 0x773f4e in xzalloc /home/hanzhou/src/ovs/_build/../lib/util.c:165:12 > > #4 0x60106c in en_northd_run /home/hanzhou/src/ovn/_build_as/../northd/en-northd.c:137:5 > > #5 0x61c6a8 in engine_recompute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:415:5 > > #6 0x61bee0 in engine_compute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:454:17 > > #7 0x61bee0 in engine_run_node /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:503:14 > > #8 0x61bee0 in engine_run /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:528:9 > > #9 0x605e23 in inc_proc_northd_run /home/hanzhou/src/ovn/_build_as/../northd/inc-proc-northd.c:317:9 > > #10 0x5fe43b in main /home/hanzhou/src/ovn/_build_as/../northd/ovn-northd.c:934:33 > > #11 0x7f473933c1a1 in __libc_start_main (/lib64/libc.so.6+0x281a1) > > > > Indirect leak of 204 byte(s) in 3 object(s) allocated from: > > #0 0x55bea8 in realloc (/home/hanzhou/src/ovn/_build_as/northd/ovn-northd+0x55bea8) > > #1 0x773c7d in xrealloc__ /home/hanzhou/src/ovs/_build/../lib/util.c:147:9 > > #2 0x773c7d in xrealloc /home/hanzhou/src/ovs/_build/../lib/util.c:179:12 > > #3 0x614bd4 in extract_addresses /home/hanzhou/src/ovn/_build_as/../lib/ovn-util.c:228:12 > > #4 0x614bd4 in extract_lsp_addresses /home/hanzhou/src/ovn/_build_as/../lib/ovn-util.c:243:20 > > #5 0x5c8d90 in parse_lsp_addrs /home/hanzhou/src/ovn/_build_as/../northd/northd.c:2468:21 > > #6 0x5b2ebf in join_logical_ports /home/hanzhou/src/ovn/_build_as/../northd/northd.c:2594:13 > > #7 0x5b2ebf in build_ports /home/hanzhou/src/ovn/_build_as/../northd/northd.c:4711:5 > > #8 0x5b2ebf in ovnnb_db_run /home/hanzhou/src/ovn/_build_as/../northd/northd.c:17376:5 > > #9 0x60106c in en_northd_run /home/hanzhou/src/ovn/_build_as/../northd/en-northd.c:137:5 > > #10 0x61c6a8 in engine_recompute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:415:5 > > #11 0x61bee0 in engine_compute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:454:17 > > #12 0x61bee0 in engine_run_node /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:503:14 > > #13 0x61bee0 in engine_run /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:528:9 > > #14 0x605e23 in inc_proc_northd_run /home/hanzhou/src/ovn/_build_as/../northd/inc-proc-northd.c:317:9 > > #15 0x5fe43b in main /home/hanzhou/src/ovn/_build_as/../northd/ovn-northd.c:934:33 > > #16 0x7f473933c1a1 in __libc_start_main (/lib64/libc.so.6+0x281a1) > > ... > > > > Fixes: b337750e45be ("northd: Incremental processing of VIF changes in 'northd' node.") > > Reported-by: Dumitru Ceara <dceara@redhat.com> > > Signed-off-by: Han Zhou <hzhou@ovn.org> > > --- > > Thanks for the fix, Han! > > > northd/northd.c | 12 +++++++++--- > > tests/ovn-northd.at | 45 +++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 54 insertions(+), 3 deletions(-) > > > > diff --git a/northd/northd.c b/northd/northd.c > > index a45c8b53ad4e..dc1271831ab9 100644 > > --- a/northd/northd.c > > +++ b/northd/northd.c > > @@ -5037,6 +5037,7 @@ northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, > > { > > const struct nbrec_logical_switch *changed_ls; > > struct ls_change *ls_change = NULL; > > + struct ovn_port *op; > > > > NBREC_LOGICAL_SWITCH_TABLE_FOR_EACH_TRACKED (changed_ls, > > ni->nbrec_logical_switch_table) { > > @@ -5067,7 +5068,6 @@ northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, > > ovs_list_init(&ls_change->deleted_ports); > > ovs_list_init(&ls_change->updated_ports); > > > > - struct ovn_port *op; > > HMAP_FOR_EACH (op, dp_node, &od->ports) { > > op->visited = false; > > } > > @@ -5133,12 +5133,12 @@ northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, > > HMAP_FOR_EACH_SAFE (op, dp_node, &od->ports) { > > if (!op->visited) { > > if (!op->lsp_can_be_inc_processed) { > > - goto fail; > > + goto fail_clean_deleted; > > } > > if (sset_contains(&nd->svc_monitor_lsps, op->key)) { > > /* This port was used for svc monitor, which may be > > * impacted by this deletion. Fallback to recompute. */ > > - goto fail; > > + goto fail_clean_deleted; > > } > > ovs_list_push_back(&ls_change->deleted_ports, > > &op->list); > > @@ -5165,6 +5165,12 @@ northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, > > } > > return true; > > > > +fail_clean_deleted: > > + LIST_FOR_EACH_SAFE (op, list, &ls_change->deleted_ports) { > > + ovs_list_remove(&op->list); > > Nit: I'd use LIST_FOR_EACH_POP here and skip the ovs_list_remove(). > > > + ovn_port_destroy_orphan(op); > > + } > > + > > fail: > > free(ls_change); > > destroy_northd_data_tracked_changes(nd); > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > > index ada2d2a4aa5e..c0d4054413bb 100644 > > --- a/tests/ovn-northd.at > > +++ b/tests/ovn-northd.at > > @@ -9559,6 +9559,51 @@ OVN_CLEANUP([hv1]) > > AT_CLEANUP > > ]) > > > > +OVN_FOR_EACH_NORTHD_NO_HV([ > > +AT_SETUP([LSP incremental processing fallback to recompute]) > > +ovn_start > > + > > +net_add n1 > > +sim_add hv1 > > +as hv1 > > +ovs-vsctl add-br br-phys > > +ovn_attach n1 br-phys 192.168.0.11 > > + > > +check ovn-nbctl --wait=hv ls-add ls0 > > + > > +# Add ports that should be incrementally processed > > +for i in $(seq 9); do > > Can we use a non-hardcoded value here, e.g.: > > n_ports=9 > > > + check ovn-nbctl --wait=hv lsp-add ls0 lsp0-$i -- lsp-set-addresses lsp0-$i "aa:aa:aa:00:00:0$i 192.168.0.1$i" > > + ovs-vsctl add-port br-int lsp0-$i -- set interface lsp0-$i external_ids:iface-id=lsp0-$i > > +done > > + > > +# add a port that should not be incrementally processed (because of the > > Nit: s/add/Add > > > +# "unknown" that is not supported by i-p for now). > > Just a note, I'm not sure if we can fix this but if we ever add I-P > support for "unknown" this test will not be relevant anymore. I had the same concern, and hopefully the comment will raise some attention when "unknown" is supported later :) > > > +check ovn-nbctl --wait=hv lsp-add ls0 lsp0-foo -- lsp-set-addresses lsp0-foo "unknown" > > +ovs-vsctl add-port br-int lsp0-foo -- set interface lsp0-foo external_ids:iface-id=lsp0-foo > > + > > +wait_for_ports_up > > +check ovn-nbctl --wait=hv sync > > +wait_row_count port_binding 10 > > Same hardcoded comment, we could re-write as: > > wait_row_count port_binding $((n_ports + 1)) > > > + > > +# Delete multiple ports, and one of them not incrementally processible. This is > > +# to trigger partial I-P and then fall back to recompute. > > +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats > > +args="--wait=hv lsp-del lsp0-foo" > > +for i in $(seq 9); do > > Here too. > > > + args="$args -- lsp-del lsp0-$i" > > +done > > +check ovn-nbctl $args > > + > > +wait_row_count Port_Binding 0 > > +northd_recomp=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats northd recompute) > > +echo northd_recomp $northd_recomp > > +AT_CHECK([test $northd_recomp -ge 1]) > > + > > +OVN_CLEANUP([hv1]) > > +AT_CLEANUP > > +]) > > + > > OVN_FOR_EACH_NORTHD_NO_HV([ > > AT_SETUP([check fip and lb flows]) > > AT_KEYWORDS([fip-lb-flows]) > > All the above are minor things, if you address them feel free to add my > ack before pushing the patch to main: > > Acked-by: Dumitru Ceara <dceara@redhat.com> Thanks Dumitru! I applied to main. > > Thanks, > Dumitru >
diff --git a/northd/northd.c b/northd/northd.c index a45c8b53ad4e..dc1271831ab9 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -5037,6 +5037,7 @@ northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, { const struct nbrec_logical_switch *changed_ls; struct ls_change *ls_change = NULL; + struct ovn_port *op; NBREC_LOGICAL_SWITCH_TABLE_FOR_EACH_TRACKED (changed_ls, ni->nbrec_logical_switch_table) { @@ -5067,7 +5068,6 @@ northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, ovs_list_init(&ls_change->deleted_ports); ovs_list_init(&ls_change->updated_ports); - struct ovn_port *op; HMAP_FOR_EACH (op, dp_node, &od->ports) { op->visited = false; } @@ -5133,12 +5133,12 @@ northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, HMAP_FOR_EACH_SAFE (op, dp_node, &od->ports) { if (!op->visited) { if (!op->lsp_can_be_inc_processed) { - goto fail; + goto fail_clean_deleted; } if (sset_contains(&nd->svc_monitor_lsps, op->key)) { /* This port was used for svc monitor, which may be * impacted by this deletion. Fallback to recompute. */ - goto fail; + goto fail_clean_deleted; } ovs_list_push_back(&ls_change->deleted_ports, &op->list); @@ -5165,6 +5165,12 @@ northd_handle_ls_changes(struct ovsdb_idl_txn *ovnsb_idl_txn, } return true; +fail_clean_deleted: + LIST_FOR_EACH_SAFE (op, list, &ls_change->deleted_ports) { + ovs_list_remove(&op->list); + ovn_port_destroy_orphan(op); + } + fail: free(ls_change); destroy_northd_data_tracked_changes(nd); diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index ada2d2a4aa5e..c0d4054413bb 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -9559,6 +9559,51 @@ OVN_CLEANUP([hv1]) AT_CLEANUP ]) +OVN_FOR_EACH_NORTHD_NO_HV([ +AT_SETUP([LSP incremental processing fallback to recompute]) +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.11 + +check ovn-nbctl --wait=hv ls-add ls0 + +# Add ports that should be incrementally processed +for i in $(seq 9); do + check ovn-nbctl --wait=hv lsp-add ls0 lsp0-$i -- lsp-set-addresses lsp0-$i "aa:aa:aa:00:00:0$i 192.168.0.1$i" + ovs-vsctl add-port br-int lsp0-$i -- set interface lsp0-$i external_ids:iface-id=lsp0-$i +done + +# add a port that should not be incrementally processed (because of the +# "unknown" that is not supported by i-p for now). +check ovn-nbctl --wait=hv lsp-add ls0 lsp0-foo -- lsp-set-addresses lsp0-foo "unknown" +ovs-vsctl add-port br-int lsp0-foo -- set interface lsp0-foo external_ids:iface-id=lsp0-foo + +wait_for_ports_up +check ovn-nbctl --wait=hv sync +wait_row_count port_binding 10 + +# Delete multiple ports, and one of them not incrementally processible. This is +# to trigger partial I-P and then fall back to recompute. +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +args="--wait=hv lsp-del lsp0-foo" +for i in $(seq 9); do + args="$args -- lsp-del lsp0-$i" +done +check ovn-nbctl $args + +wait_row_count Port_Binding 0 +northd_recomp=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats northd recompute) +echo northd_recomp $northd_recomp +AT_CHECK([test $northd_recomp -ge 1]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP +]) + OVN_FOR_EACH_NORTHD_NO_HV([ AT_SETUP([check fip and lb flows]) AT_KEYWORDS([fip-lb-flows])
When multiple LSP deletions are handled in incremental processing, if it hits a LSP that can't be incrementally processed after incrementally processing some LSP deletions, it falls back to recompute without destroying the ovn_port objects that are already handled in the handler, resulting in memory leaks. See example below, which is detected by the new test case added by this patch when running with address sanitizer. ======================= Indirect leak of 936 byte(s) in 3 object(s) allocated from: #0 0x55bce7 in calloc (/home/hanzhou/src/ovn/_build_as/northd/ovn-northd+0x55bce7) #1 0x773f4e in xcalloc__ /home/hanzhou/src/ovs/_build/../lib/util.c:121:31 #2 0x773f4e in xzalloc__ /home/hanzhou/src/ovs/_build/../lib/util.c:131:12 #3 0x773f4e in xzalloc /home/hanzhou/src/ovs/_build/../lib/util.c:165:12 #4 0x60106c in en_northd_run /home/hanzhou/src/ovn/_build_as/../northd/en-northd.c:137:5 #5 0x61c6a8 in engine_recompute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:415:5 #6 0x61bee0 in engine_compute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:454:17 #7 0x61bee0 in engine_run_node /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:503:14 #8 0x61bee0 in engine_run /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:528:9 #9 0x605e23 in inc_proc_northd_run /home/hanzhou/src/ovn/_build_as/../northd/inc-proc-northd.c:317:9 #10 0x5fe43b in main /home/hanzhou/src/ovn/_build_as/../northd/ovn-northd.c:934:33 #11 0x7f473933c1a1 in __libc_start_main (/lib64/libc.so.6+0x281a1) Indirect leak of 204 byte(s) in 3 object(s) allocated from: #0 0x55bea8 in realloc (/home/hanzhou/src/ovn/_build_as/northd/ovn-northd+0x55bea8) #1 0x773c7d in xrealloc__ /home/hanzhou/src/ovs/_build/../lib/util.c:147:9 #2 0x773c7d in xrealloc /home/hanzhou/src/ovs/_build/../lib/util.c:179:12 #3 0x614bd4 in extract_addresses /home/hanzhou/src/ovn/_build_as/../lib/ovn-util.c:228:12 #4 0x614bd4 in extract_lsp_addresses /home/hanzhou/src/ovn/_build_as/../lib/ovn-util.c:243:20 #5 0x5c8d90 in parse_lsp_addrs /home/hanzhou/src/ovn/_build_as/../northd/northd.c:2468:21 #6 0x5b2ebf in join_logical_ports /home/hanzhou/src/ovn/_build_as/../northd/northd.c:2594:13 #7 0x5b2ebf in build_ports /home/hanzhou/src/ovn/_build_as/../northd/northd.c:4711:5 #8 0x5b2ebf in ovnnb_db_run /home/hanzhou/src/ovn/_build_as/../northd/northd.c:17376:5 #9 0x60106c in en_northd_run /home/hanzhou/src/ovn/_build_as/../northd/en-northd.c:137:5 #10 0x61c6a8 in engine_recompute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:415:5 #11 0x61bee0 in engine_compute /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:454:17 #12 0x61bee0 in engine_run_node /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:503:14 #13 0x61bee0 in engine_run /home/hanzhou/src/ovn/_build_as/../lib/inc-proc-eng.c:528:9 #14 0x605e23 in inc_proc_northd_run /home/hanzhou/src/ovn/_build_as/../northd/inc-proc-northd.c:317:9 #15 0x5fe43b in main /home/hanzhou/src/ovn/_build_as/../northd/ovn-northd.c:934:33 #16 0x7f473933c1a1 in __libc_start_main (/lib64/libc.so.6+0x281a1) ... Fixes: b337750e45be ("northd: Incremental processing of VIF changes in 'northd' node.") Reported-by: Dumitru Ceara <dceara@redhat.com> Signed-off-by: Han Zhou <hzhou@ovn.org> --- northd/northd.c | 12 +++++++++--- tests/ovn-northd.at | 45 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-)