10:19 p.m.
Instead of calling iptableSetupPrivateChains(), the network driver now
calls virNetfilterSetupPrivateChains() (which right now always calls
the iptables version of the function, but in the future might instead
call the nftables version).
virNetFilterSetupPrivateChains() needs an argument to know which
backend to call, and that means that networkSetupPrivateChains() has
to take an argument (we can't rely on getting the setting from the
driver config, because the unit tests don't initialize the network
driver).
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()). So
instead this patch changes things to handle the "do it once"
functionality 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(), just call it directly instead.
(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 <laine(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/network/bridge_driver_linux.c | 30 +++++++++++++++---------------
src/util/viriptables.h | 7 ++++---
src/util/virnetfilter.c | 16 ++++++++++++++++
src/util/virnetfilter.h | 3 +++
5 files changed, 39 insertions(+), 18 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a09e5ae871..a93143638f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2966,6 +2966,7 @@ virNetfilterRemoveTcpInput;
virNetfilterRemoveTcpOutput;
virNetfilterRemoveUdpInput;
virNetfilterRemoveUdpOutput;
+virNetfilterSetupPrivateChains;
# util/virnetlink.h
diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
index 3efb669789..058cfa1d80 100644
--- a/src/network/bridge_driver_linux.c
+++ b/src/network/bridge_driver_linux.c
@@ -35,25 +35,26 @@ VIR_LOG_INIT("network.bridge_driver_linux");
#define PROC_NET_ROUTE "/proc/net/route"
-static virOnceControl createdOnce;
+static virMutex chainInitLock = VIR_MUTEX_INITIALIZER;
static bool chainInitDone; /* true iff networkSetupPrivateChains was ever called */
static virErrorPtr errInitV4;
static virErrorPtr errInitV6;
-/* Usually only called via virOnce, but can also be called directly in
- * response to firewalld reload (if chainInitDone == true)
- */
-static void networkSetupPrivateChains(void)
+static void networkSetupPrivateChains(virFirewallBackend backend, bool force)
{
+ VIR_LOCK_GUARD lock = virLockGuardLock(&chainInitLock);
int rc;
+ if (chainInitDone && !force)
+ return;
+
VIR_DEBUG("Setting up global firewall chains");
g_clear_pointer(&errInitV4, virFreeError);
g_clear_pointer(&errInitV6, virFreeError);
- rc = iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV4);
+ rc = virNetfilterSetupPrivateChains(backend, VIR_FIREWALL_LAYER_IPV4);
if (rc < 0) {
VIR_DEBUG("Failed to create global IPv4 chains: %s",
virGetLastErrorMessage());
@@ -66,7 +67,7 @@ static void networkSetupPrivateChains(void)
VIR_DEBUG("Global IPv4 chains already exist");
}
- rc = iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV6);
+ rc = virNetfilterSetupPrivateChains(backend, VIR_FIREWALL_LAYER_IPV6);
if (rc < 0) {
VIR_DEBUG("Failed to create global IPv6 chains: %s",
virGetLastErrorMessage());
@@ -139,6 +140,7 @@ networkPreReloadFirewallRules(virNetworkDriverState *driver,
bool startup G_GNUC_UNUSED,
bool force)
{
+ g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver);
/*
* If there are any running networks, we need to
* create the global rules upfront. This allows us
@@ -158,14 +160,13 @@ 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
+ * during this run of libvirtd (known because chainInitDone == 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 call directly
- * instead of via virOnce().
+ * the chains have already been added. So we force the init.
*/
- networkSetupPrivateChains();
+ networkSetupPrivateChains(cfg->firewallBackend, true);
} else {
if (!networkHasRunningNetworksWithFW(driver)) {
@@ -173,7 +174,7 @@ networkPreReloadFirewallRules(virNetworkDriverState *driver,
return;
}
- ignore_value(virOnce(&createdOnce, networkSetupPrivateChains));
+ networkSetupPrivateChains(cfg->firewallBackend, false);
}
}
@@ -823,8 +824,7 @@ networkAddFirewallRules(virNetworkDef *def,
virNetworkIPDef *ipdef;
g_autoptr(virFirewall) fw = virFirewallNew(firewallBackend);
- if (virOnce(&createdOnce, networkSetupPrivateChains) < 0)
- return -1;
+ networkSetupPrivateChains(firewallBackend, false);
if (errInitV4 &&
(virNetworkDefGetIPByIndex(def, AF_INET, 0) ||
diff --git a/src/util/viriptables.h b/src/util/viriptables.h
index 990cb2e25d..496c6eaf51 100644
--- a/src/util/viriptables.h
+++ b/src/util/viriptables.h
@@ -37,8 +37,6 @@ virIptablesApplyFirewallRule(virFirewall *firewall,
* requires untangling all the special cases for setting up private
* chains that are necessitated by firewalld reloads).
*/
-int iptablesSetupPrivateChains (virFirewallLayer layer);
-
void iptablesAddOutputFixUdpChecksum (virFirewall *fw,
const char *iface,
int port);
@@ -46,12 +44,15 @@ void iptablesRemoveOutputFixUdpChecksum (virFirewall *fw,
const char *iface,
int port);
-/* These functions are only called from virnetfilter.c. Each can be
+/* These functions are only called from virnetfilter.c. Most can be
* called with an action of VIR_NETFILTER_INSERT or
* VIR_NETFILTER_DELETE, to add or remove the described rule(s) in the
* appropriate chain.
*/
+int
+iptablesSetupPrivateChains(virFirewallLayer layer);
+
void
iptablesInput(virFirewall *fw,
virFirewallLayer layer,
diff --git a/src/util/virnetfilter.c b/src/util/virnetfilter.c
index ba0f292ea9..f0fa0d5cd2 100644
--- a/src/util/virnetfilter.c
+++ b/src/util/virnetfilter.c
@@ -63,6 +63,22 @@ virNetfilterApplyFirewallRule(virFirewall *fw,
}
+/**
+ * virNetFilterSetupPrivateChains:
+ * @layer: VIR_NETFILTER_LAYER_IPV(4|6)
+ *
+ * Check if the private tables/chains needed for libvirt virtual
+ * networks exist in the systems filters, and add them if they're not
+ * already there.
+ *
+ */
+int
+virNetfilterSetupPrivateChains(virFirewallBackend backend G_GNUC_UNUSED,
+ virFirewallLayer layer)
+{
+ return iptablesSetupPrivateChains(layer);
+}
+
/**
* virNetfilterAddTcpInput:
* @ctx: pointer to the IP table context
diff --git a/src/util/virnetfilter.h b/src/util/virnetfilter.h
index eff047cde0..70dede3c3f 100644
--- a/src/util/virnetfilter.h
+++ b/src/util/virnetfilter.h
@@ -33,6 +33,9 @@
int virNetfilterApplyFirewallRule (virFirewall *fw,
virFirewallRule *rule,
char **output);
+
+int virNetfilterSetupPrivateChains (virFirewallBackend backend,
+ virFirewallLayer layer);
void virNetfilterAddTcpInput (virFirewall *fw,
virFirewallLayer layer,
const char *iface,
--
2.39.2