From patchwork Tue Aug 9 18:25:03 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ihar Hrachyshka X-Patchwork-Id: 1665029 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Gg7nRBT8; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4M2M1y5C8lz9sGZ for ; Wed, 10 Aug 2022 04:25:26 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 9DDED40591; Tue, 9 Aug 2022 18:25:24 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 9DDED40591 Authentication-Results: smtp4.osuosl.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Gg7nRBT8 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 BHtOMuZuFdjL; Tue, 9 Aug 2022 18:25:22 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id DDC1E408A9; Tue, 9 Aug 2022 18:25:19 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org DDC1E408A9 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id D62AAC0033; Tue, 9 Aug 2022 18:25:18 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 01F3BC0085 for ; Tue, 9 Aug 2022 18:25:17 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id C3A2580BEA for ; Tue, 9 Aug 2022 18:25:16 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org C3A2580BEA Authentication-Results: smtp1.osuosl.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Gg7nRBT8 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id lvwYWRrDriw9 for ; Tue, 9 Aug 2022 18:25:14 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 8A16A80BCD Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by smtp1.osuosl.org (Postfix) with ESMTPS id 8A16A80BCD for ; Tue, 9 Aug 2022 18:25:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1660069512; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=X0CszMNo73ltZMENaAP5OhYLEAqd1hpaBS0uJgGGm5Q=; b=Gg7nRBT8ruoFAwudPr8pglRvv/vDrLRhF5McsxcghMoyLiGoqn0MiM80h53KCZmpZ0nHJT RxbB3QywN8TqVT878xstSYL0wJu9q/66XIJkpbmNV9HUT6NN/WlwhcXSJ8H5YojlDKXD1k WLfXZo7Y62uybCc8Gb9Yfq76TLkfOwA= Received: from mail-qv1-f71.google.com (mail-qv1-f71.google.com [209.85.219.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-351-N8wKXZ9RM3OtnoqKhpURmw-1; Tue, 09 Aug 2022 14:25:12 -0400 X-MC-Unique: N8wKXZ9RM3OtnoqKhpURmw-1 Received: by mail-qv1-f71.google.com with SMTP id cz12-20020a056214088c00b004763e7e7d81so6739534qvb.21 for ; Tue, 09 Aug 2022 11:25:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc; bh=X0CszMNo73ltZMENaAP5OhYLEAqd1hpaBS0uJgGGm5Q=; b=GKwS09K9bkfn23WKWR3j0w8+KOK84OBnZYzMcSE0I7MujevbH7VVUwN+FahdL/Xvva 1VwdnZK65hCXgI4jYaN9h/YrJ63rBn9tKtdID6kkUssIgAw62YHOmb60ycy58oJ+KwdV yLHSzTNGRMQE6MTwRB2++TquMo64yojc9tBqKrY+AxKCH2wNDDNSEpUcrD+xrHa9ekZ5 TAWuHkjO7oT563zDBmQidyWkta3qkI6tZDKfjcOl/0cZryI7BpwqRWaFw9P0PvNqCWc0 aOS8mNvjZEbFIWNfAsKzjNa2Gjb8xwkho65dF78DVXspFCQDbeXo5LZ58OjcSCKpadcX dSPA== X-Gm-Message-State: ACgBeo3QVRcbAhr58LKzJIZoVii8nuidF5HKAJgQ+nEPmfS54p8ABeJ7 3kip2q2ilPR6RZa4ZtcHCK0LiPNo3+FTlO2+VXlxMTDkoLQrEqjSivUmxnUw9zIwXef83lDgwQW 1pbQPpDYBovdRpzzuQcQUlCdRnRQSmcAQiWWz1TVAaUygr5lkDCqa+ncIHVU6iNOT X-Received: by 2002:a0c:ca84:0:b0:474:7480:4114 with SMTP id a4-20020a0cca84000000b0047474804114mr21420946qvk.78.1660069511140; Tue, 09 Aug 2022 11:25:11 -0700 (PDT) X-Google-Smtp-Source: AA6agR4/Fvjk021yu+Jia9wM8niX263dAfyu0mKstoT4KqtBIgmPEgy8rh7yijIRH58cPBgGfKUxQQ== X-Received: by 2002:a0c:ca84:0:b0:474:7480:4114 with SMTP id a4-20020a0cca84000000b0047474804114mr21420922qvk.78.1660069510706; Tue, 09 Aug 2022 11:25:10 -0700 (PDT) Received: from fedora34.localdomain.com (cpe-172-73-180-250.carolina.res.rr.com. [172.73.180.250]) by smtp.gmail.com with ESMTPSA id w6-20020ac86b06000000b00342f05defd1sm6354757qts.66.2022.08.09.11.25.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 09 Aug 2022 11:25:09 -0700 (PDT) From: Ihar Hrachyshka To: dev@openvswitch.org Date: Tue, 9 Aug 2022 18:25:03 +0000 Message-Id: <20220809182503.955660-2-ihrachys@redhat.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20220809182503.955660-1-ihrachys@redhat.com> References: <20220809182503.955660-1-ihrachys@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH v6 ovn 2/2] controller: throttle port claim attempts 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 chassis are fighting for the same port (requested-chassis is not set, e.g. for gateway ports), they may produce an unreasonable number of chassis field updates in a very short time frame (hundreds of updates in several seconds). This puts unnecessary load on OVN as well as any db notification consumers trying to keep up with the barrage. This patch throttles port claim attempts so that they don't happen more frequently than once per 0.5 seconds. Reported: https://bugzilla.redhat.com/show_bug.cgi?id=1974898 Signed-off-by: Ihar Hrachyshka Acked-by: Mark Michelson --- v1: initial version v2: don't postpone claim when port is unclaimed (chassis == nil) v2: don't postpone claim as an additional chassis for a multichassis port v2: fixed memory corruption when modifying sset while iterating over it v3: rebased to resolve a git conflict v4: added opportunistic cleanup for claimed_ports shash v4: made a debug message in the new test case more intelligible v5: fixed a memleak in cleanup_claimed_port_timestamps (node->data not freed) v6: rebased, added Mark's ack. v6: removed poll_wait calls from engine handler, moved them to binding_wait. --- controller/binding.c | 127 ++++++++++++++++++++++++++++++++++-- controller/binding.h | 10 +++ controller/ovn-controller.c | 49 ++++++++++++++ tests/ovn.at | 41 ++++++++++++ 4 files changed, 222 insertions(+), 5 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index 96a158225..9f5393a92 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -48,6 +48,67 @@ VLOG_DEFINE_THIS_MODULE(binding); #define OVN_QOS_TYPE "linux-htb" +#define CLAIM_TIME_THRESHOLD_MS 500 + +struct claimed_port { + long long int last_claimed; +}; + +static struct shash _claimed_ports = SHASH_INITIALIZER(&_claimed_ports); +static struct sset _postponed_ports = SSET_INITIALIZER(&_postponed_ports); + +struct sset * +get_postponed_ports(void) +{ + return &_postponed_ports; +} + +static long long int +get_claim_timestamp(const char *port_name) +{ + struct claimed_port *cp = shash_find_data(&_claimed_ports, port_name); + return cp ? cp->last_claimed : 0; +} + +static void +register_claim_timestamp(const char *port_name, long long int t) +{ + struct claimed_port *cp = shash_find_data(&_claimed_ports, port_name); + if (!cp) { + cp = xzalloc(sizeof *cp); + shash_add(&_claimed_ports, port_name, cp); + } + cp->last_claimed = t; +} + +static void +cleanup_claimed_port_timestamps(void) +{ + long long int now = time_msec(); + struct shash_node *node; + SHASH_FOR_EACH_SAFE (node, &_claimed_ports) { + struct claimed_port *cp = (struct claimed_port *) node->data; + if (now - cp->last_claimed >= 5 * CLAIM_TIME_THRESHOLD_MS) { + free(cp); + shash_delete(&_claimed_ports, node); + } + } +} + +/* Schedule any pending binding work. Runs with in the main ovn-controller + * thread context.*/ +void +binding_wait(void) +{ + const char *port_name; + SSET_FOR_EACH (port_name, &_postponed_ports) { + long long int t = get_claim_timestamp(port_name); + if (t) { + poll_timer_wait_until(t + CLAIM_TIME_THRESHOLD_MS); + } + } +} + struct qos_queue { struct hmap_node node; uint32_t queue_id; @@ -996,6 +1057,21 @@ remove_additional_chassis(const struct sbrec_port_binding *pb, remove_additional_encap_for_chassis(pb, chassis_rec); } +static bool +lport_maybe_postpone(const char *port_name, long long int now, + struct sset *postponed_ports) +{ + long long int last_claimed = get_claim_timestamp(port_name); + if (now - last_claimed >= CLAIM_TIME_THRESHOLD_MS) { + return false; + } + + sset_add(postponed_ports, port_name); + VLOG_DBG("Postponed claim on logical port %s.", port_name); + + return true; +} + /* Returns false if lport is not claimed due to 'sb_readonly'. * Returns true otherwise. */ @@ -1006,7 +1082,8 @@ claim_lport(const struct sbrec_port_binding *pb, const struct ovsrec_interface *iface_rec, bool sb_readonly, bool notify_up, struct hmap *tracked_datapaths, - struct if_status_mgr *if_mgr) + struct if_status_mgr *if_mgr, + struct sset *postponed_ports) { if (!sb_readonly) { claimed_lport_set_up(pb, parent_pb, chassis_rec, notify_up, if_mgr); @@ -1021,7 +1098,12 @@ claim_lport(const struct sbrec_port_binding *pb, return false; } + long long int now = time_msec(); if (pb->chassis) { + if (lport_maybe_postpone(pb->logical_port, now, + postponed_ports)) { + return true; + } VLOG_INFO("Changing chassis for lport %s from %s to %s.", pb->logical_port, pb->chassis->name, chassis_rec->name); @@ -1038,6 +1120,9 @@ claim_lport(const struct sbrec_port_binding *pb, remove_additional_chassis(pb, chassis_rec); } update_tracked = true; + + register_claim_timestamp(pb->logical_port, now); + sset_find_and_delete(postponed_ports, pb->logical_port); } } else if (can_bind == CAN_BIND_AS_ADDITIONAL) { if (!is_additional_chassis(pb, chassis_rec)) { @@ -1060,8 +1145,10 @@ claim_lport(const struct sbrec_port_binding *pb, } } - if (update_tracked && tracked_datapaths) { - update_lport_tracking(pb, tracked_datapaths, true); + if (update_tracked) { + if (tracked_datapaths) { + update_lport_tracking(pb, tracked_datapaths, true); + } } /* Check if the port encap binding, if any, has changed */ @@ -1223,7 +1310,8 @@ consider_vif_lport_(const struct sbrec_port_binding *pb, b_lport->lbinding->iface, !b_ctx_in->ovnsb_idl_txn, !parent_pb, b_ctx_out->tracked_dp_bindings, - b_ctx_out->if_mgr)){ + b_ctx_out->if_mgr, + b_ctx_out->postponed_ports)) { return false; } @@ -1519,7 +1607,8 @@ consider_nonvif_lport_(const struct sbrec_port_binding *pb, return claim_lport(pb, NULL, b_ctx_in->chassis_rec, NULL, !b_ctx_in->ovnsb_idl_txn, false, b_ctx_out->tracked_dp_bindings, - b_ctx_out->if_mgr); + b_ctx_out->if_mgr, + b_ctx_out->postponed_ports); } if (pb->chassis == b_ctx_in->chassis_rec || @@ -1843,6 +1932,8 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) } destroy_qos_map(&qos_map); + + cleanup_claimed_port_timestamps(); } /* Returns true if the database is all cleaned up, false if more work is @@ -2740,6 +2831,25 @@ delete_done: } } + /* Also handle any postponed (throttled) ports. */ + const char *port_name; + struct sset postponed_ports = SSET_INITIALIZER(&postponed_ports); + sset_clone(&postponed_ports, b_ctx_out->postponed_ports); + SSET_FOR_EACH (port_name, &postponed_ports) { + pb = lport_lookup_by_name(b_ctx_in->sbrec_port_binding_by_name, + port_name); + if (!pb) { + sset_find_and_delete(b_ctx_out->postponed_ports, port_name); + continue; + } + handled = handle_updated_port(b_ctx_in, b_ctx_out, pb, qos_map_ptr); + if (!handled) { + break; + } + } + sset_destroy(&postponed_ports); + cleanup_claimed_port_timestamps(); + if (handled && qos_map_ptr && set_noop_qos(b_ctx_in->ovs_idl_txn, b_ctx_in->port_table, b_ctx_in->qos_table, @@ -3182,3 +3292,10 @@ ovs_iface_matches_lport_iface_id_ver(const struct ovsrec_interface *iface, return true; } + +void +binding_destroy(void) +{ + shash_destroy_free_data(&_claimed_ports); + sset_clear(&_postponed_ports); +} diff --git a/controller/binding.h b/controller/binding.h index 1fed06674..b2360bac2 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -103,6 +103,8 @@ struct binding_ctx_out { struct hmap *tracked_dp_bindings; struct if_status_mgr *if_mgr; + + struct sset *postponed_ports; }; /* Local bindings. binding.c module binds the logical port (represented by @@ -219,4 +221,12 @@ struct binding_lport { size_t n_port_security; }; +struct sset *get_postponed_ports(void); + +/* Schedule any pending binding work. */ +void binding_wait(void); + +/* Clean up module state. */ +void binding_destroy(void); + #endif /* controller/binding.h */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 5449743e8..8268726e6 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1175,6 +1175,41 @@ en_activated_ports_run(struct engine_node *node, void *data_) engine_set_node_state(node, state); } +struct ed_type_postponed_ports { + struct sset *postponed_ports; +}; + +static void * +en_postponed_ports_init(struct engine_node *node OVS_UNUSED, + struct engine_arg *arg OVS_UNUSED) +{ + struct ed_type_postponed_ports *data = xzalloc(sizeof *data); + data->postponed_ports = get_postponed_ports(); + return data; +} + +static void +en_postponed_ports_cleanup(void *data_) +{ + struct ed_type_postponed_ports *data = data_; + if (!data->postponed_ports) { + return; + } + data->postponed_ports = NULL; +} + +static void +en_postponed_ports_run(struct engine_node *node, void *data_) +{ + struct ed_type_postponed_ports *data = data_; + enum engine_node_state state = EN_UNCHANGED; + data->postponed_ports = get_postponed_ports(); + if (!sset_is_empty(data->postponed_ports)) { + state = EN_UPDATED; + } + engine_set_node_state(node, state); +} + struct ed_type_runtime_data { /* Contains "struct local_datapath" nodes. */ struct hmap local_datapaths; @@ -1205,6 +1240,8 @@ struct ed_type_runtime_data { struct shash local_active_ports_ipv6_pd; struct shash local_active_ports_ras; + + struct sset *postponed_ports; }; /* struct ed_type_runtime_data has the below members for tracking the @@ -1405,6 +1442,7 @@ init_binding_ctx(struct engine_node *node, b_ctx_out->egress_ifaces = &rt_data->egress_ifaces; b_ctx_out->lbinding_data = &rt_data->lbinding_data; b_ctx_out->local_iface_ids = &rt_data->local_iface_ids; + b_ctx_out->postponed_ports = rt_data->postponed_ports; b_ctx_out->tracked_dp_bindings = NULL; b_ctx_out->if_mgr = ctrl_ctx->if_mgr; } @@ -1442,6 +1480,10 @@ en_runtime_data_run(struct engine_node *node, void *data) local_binding_data_init(&rt_data->lbinding_data); } + struct ed_type_postponed_ports *pp_data = + engine_get_input_data("postponed_ports", node); + rt_data->postponed_ports = pp_data->postponed_ports; + struct binding_ctx_in b_ctx_in; struct binding_ctx_out b_ctx_out; init_binding_ctx(node, rt_data, &b_ctx_in, &b_ctx_out); @@ -3542,6 +3584,7 @@ main(int argc, char *argv[]) ENGINE_NODE(mff_ovn_geneve, "mff_ovn_geneve"); ENGINE_NODE(ofctrl_is_connected, "ofctrl_is_connected"); ENGINE_NODE_WITH_CLEAR_TRACK_DATA(activated_ports, "activated_ports"); + ENGINE_NODE(postponed_ports, "postponed_ports"); ENGINE_NODE(pflow_output, "physical_flow_output"); ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lflow_output, "logical_flow_output"); ENGINE_NODE(flow_output, "flow_output"); @@ -3681,6 +3724,9 @@ main(int argc, char *argv[]) runtime_data_sb_datapath_binding_handler); engine_add_input(&en_runtime_data, &en_sb_port_binding, runtime_data_sb_port_binding_handler); + /* Reuse the same handler for any previously postponed ports. */ + engine_add_input(&en_runtime_data, &en_postponed_ports, + runtime_data_sb_port_binding_handler); /* The OVS interface handler for runtime_data changes MUST be executed * after the sb_port_binding_handler as port_binding deletes must be @@ -4191,6 +4237,8 @@ main(int argc, char *argv[]) ofctrl_wait(); pinctrl_wait(ovnsb_idl_txn); } + + binding_wait(); } if (!northd_version_match && br_int) { @@ -4318,6 +4366,7 @@ loop_done: lflow_destroy(); ofctrl_destroy(); pinctrl_destroy(); + binding_destroy(); patch_destroy(); if_status_mgr_destroy(if_mgr); shash_destroy(&vif_plug_deleted_iface_ids); diff --git a/tests/ovn.at b/tests/ovn.at index 23b205791..c8cc8cde4 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -15274,6 +15274,47 @@ OVN_CLEANUP([hv1],[hv2]) AT_CLEANUP ]) +OVN_FOR_EACH_NORTHD([ +AT_SETUP([tug-of-war between two chassis for the same port]) +ovn_start + +ovn-nbctl ls-add ls0 +ovn-nbctl lsp-add ls0 lsp0 + +net_add n1 +for i in 1 2; do + sim_add hv$i + as hv$i + ovs-vsctl add-br br-phys + ovn_attach n1 br-phys 192.168.0.$i +done + +for i in 1 2; do + as hv$i + ovs-vsctl -- add-port br-int vif \ + -- set Interface vif external-ids:iface-id=lsp0 +done + +# give controllers some time to fight for the port binding +sleep 3 + +# calculate the number of port claims registered by each fighting chassis +hv1_claims=$(as hv1 grep -c 'Claiming\|Changing chassis' hv1/ovn-controller.log) +hv2_claims=$(as hv2 grep -c 'Claiming\|Changing chassis' hv2/ovn-controller.log) + +echo "hv1 claimed ${hv1_claims} times" +echo "hv2 claimed ${hv2_claims} times" + +# check that neither registered an outrageous number of port claims +max_claims=10 +AT_CHECK([test "${hv1_claims}" -le "${max_claims}"], [0], []) +AT_CHECK([test "${hv2_claims}" -le "${max_claims}"], [0], []) + +OVN_CLEANUP([hv1],[hv2]) + +AT_CLEANUP +]) + OVN_FOR_EACH_NORTHD([ AT_SETUP([options:requested-chassis with hostname])