From nobody Thu Sep 19 01:42:34 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.libvirt.org designates 8.43.85.245 as permitted sender) client-ip=8.43.85.245; envelope-from=devel-bounces@lists.libvirt.org; helo=lists.libvirt.org; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of lists.libvirt.org designates 8.43.85.245 as permitted sender) smtp.mailfrom=devel-bounces@lists.libvirt.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.libvirt.org (lists.libvirt.org [8.43.85.245]) by mx.zohomail.com with SMTPS id 1715967712457542.8600628130353; Fri, 17 May 2024 10:41:52 -0700 (PDT) Received: by lists.libvirt.org (Postfix, from userid 996) id 6763C1D7F; Fri, 17 May 2024 13:41:51 -0400 (EDT) Received: from lists.libvirt.org (localhost [IPv6:::1]) by lists.libvirt.org (Postfix) with ESMTP id 89E3B1B84; Fri, 17 May 2024 13:30:56 -0400 (EDT) Received: by lists.libvirt.org (Postfix, from userid 996) id 1BFCD1A3E; Fri, 17 May 2024 13:30:16 -0400 (EDT) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.libvirt.org (Postfix) with ESMTPS id AF5C81924 for ; Fri, 17 May 2024 13:30:11 -0400 (EDT) Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-396-KujpXBBuPQSlccQK2HZCFg-1; Fri, 17 May 2024 13:30:10 -0400 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (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 mimecast-mx02.redhat.com (Postfix) with ESMTPS id C737A29ABA16 for ; Fri, 17 May 2024 17:30:09 +0000 (UTC) Received: from vhost3.router.laine.org (unknown [10.22.16.223]) by smtp.corp.redhat.com (Postfix) with ESMTP id B0FE440C6CB7 for ; Fri, 17 May 2024 17:30:09 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on lists.libvirt.org X-Spam-Level: X-Spam-Status: No, score=-0.5 required=5.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE autolearn=unavailable autolearn_force=no version=3.4.4 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1715967011; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=MezisuRiORV4SGtbn1dvCwa4TuD0s4NnnaYOOxXsbVk=; b=cnaGjTXKwHgSRBNBVhXjy8+mGf1Hh7xi02ghsvucclmfDWSvVJ56k1MitWfTk7JPuzWKCL jBhACxrMrEc2SEgSY4tr0aukUVzR0YkaMEQakuymkep1u+Yk0FLFa7RoT2TFIpeoU2bODL MvQ4X8T4w4Gm7/tkl2UE4XwYCMfQEic= X-MC-Unique: KujpXBBuPQSlccQK2HZCFg-1 From: Laine Stump To: devel@lists.libvirt.org Subject: [PATCH v5 13/30] network: framework to call backend-specific function to init private filter chains Date: Fri, 17 May 2024 13:29:50 -0400 Message-ID: <20240517173007.8125-14-laine@redhat.com> In-Reply-To: <20240517173007.8125-1-laine@redhat.com> References: <20240517173007.8125-1-laine@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.2 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Message-ID-Hash: 4JLEBR3V2ZHLTGHBO6IRR5K4MVM6BSXC X-Message-ID-Hash: 4JLEBR3V2ZHLTGHBO6IRR5K4MVM6BSXC X-MailFrom: laine@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; header-match-config-2; header-match-config-3; header-match-devel.lists.libvirt.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header X-Mailman-Version: 3.2.2 Precedence: list List-Id: Development discussions about the libvirt library & tools Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-ZohoMail-DKIM: fail (Header signature does not verify) X-ZM-MESSAGEID: 1715967714564100001 Content-Type: text/plain; charset="utf-8" Modify networkSetupPrivateChains() in the network driver to accept a firewallBackend argument so it will know which backend to call. (right now it always calls the iptables version of the lower level function, but in the future it could instead call the nftables version based on configuration). But networkSetupPrivateChains() was being called with virOnce(), and virOnce() doesn't support calling functions that require an argument (it's based on pthread_once(), which accepts no arguments, so it's not something we can easily fix in our implementation of virOnce()). To solve this dilemma, this patch eliminates use of virOnce() by adding a static lock, and putting all of networkSetupPrivateChains() (including the setting of "chainInitDone") inside a lock guard - now the places that used to call it via virOnce() can safely call it directly instead (adding in the necessary argument to specify backend). (If it turns out to be significant, we could optimize this by checking for chainInitDone outside the lock guard, returning immediately if it's already set, and then moving the setting of chainInitDone up to the top of the guarded section.) Signed-off-by: Laine Stump Reviewed-by: Daniel P. Berrang=C3=A9 --- src/network/bridge_driver_linux.c | 57 +++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_= linux.c index c2ef27f251..988362abef 100644 --- a/src/network/bridge_driver_linux.c +++ b/src/network/bridge_driver_linux.c @@ -34,25 +34,45 @@ VIR_LOG_INIT("network.bridge_driver_linux"); =20 #define PROC_NET_ROUTE "/proc/net/route" =20 -static virOnceControl createdOnce; +static virMutex chainInitLock =3D VIR_MUTEX_INITIALIZER; static bool chainInitDone; /* true iff networkSetupPrivateChains was ever = called */ =20 static virErrorPtr errInitV4; static virErrorPtr errInitV6; =20 -/* Usually only called via virOnce, but can also be called directly in - * response to firewalld reload (if chainInitDone =3D=3D true) - */ -static void networkSetupPrivateChains(void) + +static int +networkFirewallSetupPrivateChains(virFirewallBackend backend, + virFirewallLayer layer) +{ + switch (backend) { + case VIR_FIREWALL_BACKEND_IPTABLES: + return iptablesSetupPrivateChains(layer); + + case VIR_FIREWALL_BACKEND_LAST: + virReportEnumRangeError(virFirewallBackend, backend); + return -1; + } + return 0; +} + + +static void +networkSetupPrivateChains(virFirewallBackend backend, + bool force) { + VIR_LOCK_GUARD lock =3D virLockGuardLock(&chainInitLock); int rc; =20 + if (chainInitDone && !force) + return; + VIR_DEBUG("Setting up global firewall chains"); =20 g_clear_pointer(&errInitV4, virFreeError); g_clear_pointer(&errInitV6, virFreeError); =20 - rc =3D iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV4); + rc =3D networkFirewallSetupPrivateChains(backend, VIR_FIREWALL_LAYER_I= PV4); if (rc < 0) { VIR_DEBUG("Failed to create global IPv4 chains: %s", virGetLastErrorMessage()); @@ -65,7 +85,7 @@ static void networkSetupPrivateChains(void) VIR_DEBUG("Global IPv4 chains already exist"); } =20 - rc =3D iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV6); + rc =3D networkFirewallSetupPrivateChains(backend, VIR_FIREWALL_LAYER_I= PV6); if (rc < 0) { VIR_DEBUG("Failed to create global IPv6 chains: %s", virGetLastErrorMessage()); @@ -138,6 +158,7 @@ networkPreReloadFirewallRules(virNetworkDriverState *dr= iver, bool startup G_GNUC_UNUSED, bool force) { + g_autoptr(virNetworkDriverConfig) cfg =3D virNetworkDriverGetConfig(dr= iver); /* * If there are any running networks, we need to * create the global rules upfront. This allows us @@ -157,14 +178,14 @@ networkPreReloadFirewallRules(virNetworkDriverState *= driver, */ if (chainInitDone && force) { /* The Private chains have already been initialized once - * during this run of libvirtd, so 1) we can't do it again via - * virOnce(), and 2) we need to re-add the private chains even - * if there are currently no running networks, because the - * next time a network is started, libvirt will expect that - * the chains have already been added. So we call directly - * instead of via virOnce(). + * during this run of libvirtd/virtnetworkd (known because + * chainInitDone =3D=3D true) so we need to re-add the private + * chains even if there are currently no running networks, + * because the next time a network is started, libvirt will + * expect that the chains have already been added. So we force + * the init. */ - networkSetupPrivateChains(); + networkSetupPrivateChains(cfg->firewallBackend, true); =20 } else { if (!networkHasRunningNetworksWithFW(driver)) { @@ -172,7 +193,7 @@ networkPreReloadFirewallRules(virNetworkDriverState *dr= iver, return; } =20 - ignore_value(virOnce(&createdOnce, networkSetupPrivateChains)); + networkSetupPrivateChains(cfg->firewallBackend, false); } } =20 @@ -304,10 +325,10 @@ int networkCheckRouteCollision(virNetworkDef *def) =20 int networkAddFirewallRules(virNetworkDef *def, - virFirewallBackend firewallBackend G_GNUC_UNUSED) + virFirewallBackend firewallBackend) { - if (virOnce(&createdOnce, networkSetupPrivateChains) < 0) - return -1; + + networkSetupPrivateChains(firewallBackend, false); =20 if (errInitV4 && (virNetworkDefGetIPByIndex(def, AF_INET, 0) || --=20 2.45.0