From patchwork Mon Jul 12 08:08:10 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1503821 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: 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=OqXS7xpN; dkim-atps=neutral Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4GNbyZ6KRNz9vGJ for ; Mon, 12 Jul 2021 18:09:26 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 70C3A4047E; Mon, 12 Jul 2021 08:09:23 +0000 (UTC) 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 h4k6YM1-_tFo; Mon, 12 Jul 2021 08:09:22 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id 51BF440324; Mon, 12 Jul 2021 08:09:21 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 316E8C001A; Mon, 12 Jul 2021 08:09:21 +0000 (UTC) X-Original-To: ovs-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 C00BCC000E for ; Mon, 12 Jul 2021 08:09:19 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id ACBF64039E for ; Mon, 12 Jul 2021 08:09:19 +0000 (UTC) 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 o-S3jfBfT2wa for ; Mon, 12 Jul 2021 08:09:18 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by smtp4.osuosl.org (Postfix) with ESMTPS id 69EB740324 for ; Mon, 12 Jul 2021 08:09:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1626077357; 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; bh=oWqPcVU6zAi6jkuTcADSd6aACc/ryjT/o8fYQZ4GHH0=; b=OqXS7xpNU9vUbM7jvNLVpSkAV+/QVae25nNKdQWo4cI8V/VdlNP8nHUfAAP+LtWOYrLKNd 1xf9bkMjF+f6PxxNngkqIqhrzJBduuR6fWcg/rB+gbfT/DGMmHUTYA8E+QzpzNpz+B1ipp 363RawoaTJZEv8q57nLxrrU4TwfLCJ8= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-6-ctaXcmKKN323j79Cyxh7RA-1; Mon, 12 Jul 2021 04:09:15 -0400 X-MC-Unique: ctaXcmKKN323j79Cyxh7RA-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 464D9804323; Mon, 12 Jul 2021 08:09:14 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-0.ams2.redhat.com [10.36.114.0]) by smtp.corp.redhat.com (Postfix) with ESMTP id CA4B85C1D5; Mon, 12 Jul 2021 08:09:04 +0000 (UTC) From: Dumitru Ceara To: ovs-dev@openvswitch.org Date: Mon, 12 Jul 2021 10:08:10 +0200 Message-Id: <20210712080810.26164-1-dceara@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn v2] controller: Avoid unnecessary load balancer flow processing. 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" Whenever a Load_Balancer is updated, e.g., a VIP is added, the following sequence of events happens: 1. The Southbound Load_Balancer record is updated. 2. The Southbound Datapath_Binding records on which the Load_Balancer is applied are updated. 3. Southbound ovsdb-server sends updates about the Load_Balancer and Datapath_Binding records to ovn-controller. 4. The IDL layer in ovn-controller processes the updates at #3, but because of the SB schema references between tables [0] all logical flows referencing the updated Datapath_Binding are marked as "updated". The same is true for Logical_DP_Group records referencing the Datapath_Binding, and also for all logical flows pointing to the new "updated" datapath groups. 5. ovn-controller ends up recomputing (removing/readding) all flows for all these tracked updates. From the SB Schema: "Datapath_Binding": { "columns": { [...] "load_balancers": {"type": {"key": {"type": "uuid", "refTable": "Load_Balancer", "refType": "weak"}, "min": 0, "max": "unlimited"}}, [...] "Load_Balancer": { "columns": { "datapaths": { [...] "type": {"key": {"type": "uuid", "refTable": "Datapath_Binding"}, "min": 0, "max": "unlimited"}}, [...] "Logical_DP_Group": { "columns": { "datapaths": {"type": {"key": {"type": "uuid", "refTable": "Datapath_Binding", "refType": "weak"}, "min": 0, "max": "unlimited"}}}, [...] "Logical_Flow": { "columns": { "logical_datapath": {"type": {"key": {"type": "uuid", "refTable": "Datapath_Binding"}, "min": 0, "max": 1}}, "logical_dp_group": {"type": {"key": {"type": "uuid", "refTable": "Logical_DP_Group"}, In order to avoid this unnecessary Logical_Flow notification storm we now remove the explicit reference from Datapath_Binding to Load_Balancer and instead store raw UUIDs. This means that on the ovn-controller side we need to perform a Load_Balancer table lookup by UUID whenever a new datapath is added, but that doesn't happen too often and the cost of the lookup is negligible compared to the huge cost of processing the unnecessary logical flow updates. This change is backwards compatible because the contents stored in the database are not changed, just that the schema constraints are relaxed a bit. Some performance measurements, on a scale test deployment simulating an ovn-kubernetes deployment with 120 nodes and a large load balancer with 16K VIPs associated to each node's logical switch, the event processing loop time in ovn-controller, when adding a new VIP, is reduced from ~39 seconds to ~8 seconds. There's no need to change the northd DDlog implementation. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1978605 Acked-by: Mark Michelson Signed-off-by: Dumitru Ceara --- controller/lflow.c | 6 ++++-- northd/ovn-northd.c | 14 ++++++-------- ovn-sb.ovsschema | 6 ++---- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index 60aa011ff..c58c4f25c 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -1744,8 +1744,10 @@ lflow_processing_end: /* Add load balancer hairpin flows if the datapath has any load balancers * associated. */ for (size_t i = 0; i < dp->n_load_balancers; i++) { - consider_lb_hairpin_flows(dp->load_balancers[i], - l_ctx_in->local_datapaths, + const struct sbrec_load_balancer *lb = + sbrec_load_balancer_table_get_for_uuid(l_ctx_in->lb_table, + &dp->load_balancers[i]); + consider_lb_hairpin_flows(lb, l_ctx_in->local_datapaths, l_ctx_out->flow_table); } diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 562dc62b2..999c3f482 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -3635,19 +3635,17 @@ build_ovn_lbs(struct northd_context *ctx, struct hmap *datapaths, continue; } - const struct sbrec_load_balancer **sbrec_lbs = - xmalloc(od->nbs->n_load_balancer * sizeof *sbrec_lbs); + struct uuid *lb_uuids = + xmalloc(od->nbs->n_load_balancer * sizeof *lb_uuids); for (size_t i = 0; i < od->nbs->n_load_balancer; i++) { const struct uuid *lb_uuid = &od->nbs->load_balancer[i]->header_.uuid; lb = ovn_northd_lb_find(lbs, lb_uuid); - sbrec_lbs[i] = lb->slb; + lb_uuids[i] = lb->slb->header_.uuid; } - - sbrec_datapath_binding_set_load_balancers( - od->sb, (struct sbrec_load_balancer **)sbrec_lbs, - od->nbs->n_load_balancer); - free(sbrec_lbs); + sbrec_datapath_binding_set_load_balancers(od->sb, lb_uuids, + od->nbs->n_load_balancer); + free(lb_uuids); } } diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema index bbf60781d..57fbc3ca4 100644 --- a/ovn-sb.ovsschema +++ b/ovn-sb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Southbound", "version": "20.18.0", - "cksum": "1816525029 26536", + "cksum": "779623696 26386", "tables": { "SB_Global": { "columns": { @@ -166,9 +166,7 @@ "type": {"key": {"type": "integer", "minInteger": 1, "maxInteger": 16777215}}}, - "load_balancers": {"type": {"key": {"type": "uuid", - "refTable": "Load_Balancer", - "refType": "weak"}, + "load_balancers": {"type": {"key": {"type": "uuid"}, "min": 0, "max": "unlimited"}}, "external_ids": {