From patchwork Tue Feb 27 16:05:02 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Maximets X-Patchwork-Id: 1905285 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=patchwork.ozlabs.org) 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 (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Tkj3m5WK7z1yX0 for ; Wed, 28 Feb 2024 03:04:36 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 9B81A60BC5; Tue, 27 Feb 2024 16:04:34 +0000 (UTC) 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 pfd0-5Ftb5kK; Tue, 27 Feb 2024 16:04:33 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=140.211.9.56; helo=lists.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 8627C605EA Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTPS id 8627C605EA; Tue, 27 Feb 2024 16:04:33 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 579EDC0072; Tue, 27 Feb 2024 16:04:33 +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 B940BC0037 for ; Tue, 27 Feb 2024 16:04:31 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 9BAD940341 for ; Tue, 27 Feb 2024 16:04:31 +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 lxYB8rrJNcN0 for ; Tue, 27 Feb 2024 16:04:30 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=2001:4b98:dc4:8::226; helo=relay6-d.mail.gandi.net; envelope-from=i.maximets@ovn.org; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp4.osuosl.org 4D0A0404F5 Authentication-Results: smtp4.osuosl.org; dmarc=none (p=none dis=none) header.from=ovn.org DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 4D0A0404F5 Received: from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::226]) by smtp4.osuosl.org (Postfix) with ESMTPS id 4D0A0404F5 for ; Tue, 27 Feb 2024 16:04:29 +0000 (UTC) Received: by mail.gandi.net (Postfix) with ESMTPSA id BFE1FC0003; Tue, 27 Feb 2024 16:04:26 +0000 (UTC) From: Ilya Maximets To: ovs-dev@openvswitch.org Date: Tue, 27 Feb 2024 17:05:02 +0100 Message-ID: <20240227160508.3530181-1-i.maximets@ovn.org> X-Mailer: git-send-email 2.43.0 MIME-Version: 1.0 X-GND-Sasl: i.maximets@ovn.org Cc: Dumitru Ceara , Ilya Maximets Subject: [ovs-dev] [PATCH ovn] northd: Don't create fair Sb meters for ACLs with logging disabled. 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" If the ACL.log is false for a fair meter, but ACL.meter is set in the Northbound database, northd will create a unique meter for this ACL in a Southbound database, even though it will never be used. Normal ovn-nbctl acl-add command can't create such a record, but it is possible with a plain 'ovn-nbctl set' or a direct database transaction. And, in practice, ovn-kubernetes always sets the ACL.meter column even if the logging is not enabled in the namespace. This creates extra unnecessary load on the Southbound database and the ovn-controller that performs a linear iteration over the Southbound Meter table on every ofctrl_put(). Logging is also not a default option, so only a fraction of ACLs will actually need meters under normal circumstances. Stop generating these unnecessary meters. In an ovn-kubernetes setup with 90K ACLs 1K of which has logging enabled this saves ~20 MB of the Southbound database file size and about 30% of the RSS on ovsdb-server (with 1 ovn-controller connected). Should make ofctrl_put() in ovn-controller much faster as well. Arguably, CMS should not set ACL.meter without ACL.log, but the behavior of the ovn-northd is not correct either, so should be fixed anyway. Fixes: 880dca99eaf7 ("northd: Enhance the implementation of ACL log meters (pre-ddlog merge).") Reported-at: https://issues.redhat.com/browse/FDP-401 Signed-off-by: Ilya Maximets Acked-by: Mark Michelson Acked-by: Ales Musil --- northd/en-meters.c | 8 ++++++-- tests/ovn-northd.at | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/northd/en-meters.c b/northd/en-meters.c index aabd002b6..793a46335 100644 --- a/northd/en-meters.c +++ b/northd/en-meters.c @@ -203,9 +203,13 @@ sync_acl_fair_meter(struct ovsdb_idl_txn *ovnsb_txn, const struct nbrec_acl *acl, struct shash *sb_meters, struct sset *used_sb_meters) { - const struct nbrec_meter *nb_meter = - fair_meter_lookup_by_name(meter_groups, acl->meter); + const struct nbrec_meter *nb_meter; + + if (!acl->log || !acl->meter) { + return; + } + nb_meter = fair_meter_lookup_by_name(meter_groups, acl->meter); if (!nb_meter) { return; } diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 6fdd761da..0732486f3 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -2432,6 +2432,24 @@ check_acl_lflow acl_two meter_me__${acl2} sw0 check_acl_lflow acl_three meter_me__${acl3} sw0 check_acl_lflow acl_three meter_me__${acl3} sw1 +AS_BOX([Disable/enable logging for acl3 while still referencing the meter]) +check_row_count meter 4 +check ovn-nbctl --wait=sb set acl $acl3 log=false +check_row_count meter 3 +check_meter_by_name meter_me meter_me__${acl1} meter_me__${acl2} +check_meter_by_name NOT meter_me__${acl3} +check_acl_lflow acl_one meter_me__${acl1} sw0 +check_acl_lflow acl_two meter_me__${acl2} sw0 + +check ovn-nbctl --wait=sb set acl $acl3 log=true +check_row_count meter 4 +check_meter_by_name \ + meter_me meter_me__${acl1} meter_me__${acl2} meter_me__${acl3} +check_acl_lflow acl_one meter_me__${acl1} sw0 +check_acl_lflow acl_two meter_me__${acl2} sw0 +check_acl_lflow acl_three meter_me__${acl3} sw0 +check_acl_lflow acl_three meter_me__${acl3} sw1 + AS_BOX([Stop using meter for acl1]) check ovn-nbctl --wait=sb clear acl $acl1 meter check_meter_by_name meter_me meter_me__${acl2}