From patchwork Thu Jun 29 02:18:00 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1801335 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Qs2Dh47TBz20ZC for ; Thu, 29 Jun 2023 12:18:28 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 3F89960AA8; Thu, 29 Jun 2023 02:18:26 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 3F89960AA8 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id PrRaZjaMWfpL; Thu, 29 Jun 2023 02:18:25 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id 4730D605A0; Thu, 29 Jun 2023 02:18:24 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 4730D605A0 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 142DEC007C; Thu, 29 Jun 2023 02:18:24 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 99325C0037 for ; Thu, 29 Jun 2023 02:18:22 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 6C7BE408B0 for ; Thu, 29 Jun 2023 02:18:22 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 6C7BE408B0 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id IGWPM_TIcz9C for ; Thu, 29 Jun 2023 02:18:21 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 82E4840201 Received: from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net [217.70.183.200]) by smtp4.osuosl.org (Postfix) with ESMTPS id 82E4840201 for ; Thu, 29 Jun 2023 02:18:20 +0000 (UTC) X-GND-Sasl: hzhou@ovn.org X-GND-Sasl: hzhou@ovn.org X-GND-Sasl: hzhou@ovn.org Received: by mail.gandi.net (Postfix) with ESMTPSA id 7F47B20002; Thu, 29 Jun 2023 02:18:16 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Wed, 28 Jun 2023 19:18:00 -0700 Message-Id: <20230629021800.88257-1-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Cc: Dumitru Ceara Subject: [ovs-dev] [PATCH ovn] northd.c: Fix memory leak when falling back to recompute during LSP deletion. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" 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 Signed-off-by: Han Zhou Acked-by: Dumitru Ceara --- 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); + 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])