diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0e9bb78c32..c0c026e242 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2122,7 +2122,7 @@ networkReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup) * but until we untangle the virt driver that's not viable */ if (!driver->privileged) return; - networkPreReloadFirewallRules(startup); + networkPreReloadFirewallRules(driver, startup); virNetworkObjListForEach(driver->networks, networkReloadFirewallRulesHelper, NULL); diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c index 75b34fc317..35459c10d1 100644 --- a/src/network/bridge_driver_linux.c +++ b/src/network/bridge_driver_linux.c @@ -84,16 +84,57 @@ static void networkSetupPrivateChains(void) } } -void networkPreReloadFirewallRules(bool startup) + +static int +networkHasRunningNetworksHelper(virNetworkObjPtr obj, + void *opaque) { - /* We create global rules upfront as we don't want - * the perf hit of conditionally figuring out whether - * to create them each time a network is started. + bool *running = opaque; + + virObjectLock(obj); + if (virNetworkObjIsActive(obj)) + *running = true; + virObjectUnlock(obj); + + return 0; +} + + +static bool +networkHasRunningNetworks(virNetworkDriverStatePtr driver) +{ + bool running = false; + virNetworkObjListForEach(driver->networks, + networkHasRunningNetworksHelper, + &running); + return running; +} + + +void networkPreReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup) +{ + /* + * If there are any running networks, we need to + * create the global rules upfront. This allows us + * convert rules created by old libvirt into the new + * format. + * + * If there are not any running networks, then we + * must not create rules, because the rules will + * cause the conntrack kernel module to be loaded. + * This imposes a significant performance hit on + * the networking stack. Thus we will only create + * rules if a network is later startup. * * Any errors here are saved to be reported at time * of starting the network though as that makes them * more likely to be seen by a human */ + if (!networkHasRunningNetworks(driver)) { + VIR_DEBUG("Delayed global rule setup as no networks are running"); + return; + } + ignore_value(virOnce(&createdOnce, networkSetupPrivateChains)); /* @@ -726,6 +767,9 @@ int networkAddFirewallRules(virNetworkDefPtr def) virFirewallPtr fw = NULL; int ret = -1; + if (virOnce(&createdOnce, networkSetupPrivateChains) < 0) + return -1; + if (errInitV4 && (virNetworkDefGetIPByIndex(def, AF_INET, 0) || virNetworkDefGetRouteByIndex(def, AF_INET, 0))) { diff --git a/src/network/bridge_driver_nop.c b/src/network/bridge_driver_nop.c index ea9db338cb..ec7b1ed8b7 100644 --- a/src/network/bridge_driver_nop.c +++ b/src/network/bridge_driver_nop.c @@ -19,7 +19,8 @@ #include -void networkPreReloadFirewallRules(bool startup ATTRIBUTE_UNUSED) +void networkPreReloadFirewallRules(virNetworkDriverStatePtr driver ATTRIBUTE_UNUSED, + bool startup ATTRIBUTE_UNUSED) { } diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h index 95fd64bdc7..5f534fc132 100644 --- a/src/network/bridge_driver_platform.h +++ b/src/network/bridge_driver_platform.h @@ -58,7 +58,7 @@ struct _virNetworkDriverState { typedef struct _virNetworkDriverState virNetworkDriverState; typedef virNetworkDriverState *virNetworkDriverStatePtr; -void networkPreReloadFirewallRules(bool startup); +void networkPreReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup); void networkPostReloadFirewallRules(bool startup); int networkCheckRouteCollision(virNetworkDefPtr def); diff --git a/tests/networkxml2firewalldata/base.args b/tests/networkxml2firewalldata/base.args new file mode 100644 index 0000000000..0e71bf3a64 --- /dev/null +++ b/tests/networkxml2firewalldata/base.args @@ -0,0 +1,116 @@ +iptables \ +--table filter \ +--list-rules +iptables \ +--table nat \ +--list-rules +iptables \ +--table mangle \ +--list-rules +iptables \ +--table filter \ +--new-chain LIBVIRT_INP +iptables \ +--table filter \ +--insert INPUT \ +--jump LIBVIRT_INP +iptables \ +--table filter \ +--new-chain LIBVIRT_OUT +iptables \ +--table filter \ +--insert OUTPUT \ +--jump LIBVIRT_OUT +iptables \ +--table filter \ +--new-chain LIBVIRT_FWO +iptables \ +--table filter \ +--insert FORWARD \ +--jump LIBVIRT_FWO +iptables \ +--table filter \ +--new-chain LIBVIRT_FWI +iptables \ +--table filter \ +--insert FORWARD \ +--jump LIBVIRT_FWI +iptables \ +--table filter \ +--new-chain LIBVIRT_FWX +iptables \ +--table filter \ +--insert FORWARD \ +--jump LIBVIRT_FWX +iptables \ +--table nat \ +--new-chain LIBVIRT_PRT +iptables \ +--table nat \ +--insert POSTROUTING \ +--jump LIBVIRT_PRT +iptables \ +--table mangle \ +--new-chain LIBVIRT_PRT +iptables \ +--table mangle \ +--insert POSTROUTING \ +--jump LIBVIRT_PRT +ip6tables \ +--table filter \ +--list-rules +ip6tables \ +--table nat \ +--list-rules +ip6tables \ +--table mangle \ +--list-rules +ip6tables \ +--table filter \ +--new-chain LIBVIRT_INP +ip6tables \ +--table filter \ +--insert INPUT \ +--jump LIBVIRT_INP +ip6tables \ +--table filter \ +--new-chain LIBVIRT_OUT +ip6tables \ +--table filter \ +--insert OUTPUT \ +--jump LIBVIRT_OUT +ip6tables \ +--table filter \ +--new-chain LIBVIRT_FWO +ip6tables \ +--table filter \ +--insert FORWARD \ +--jump LIBVIRT_FWO +ip6tables \ +--table filter \ +--new-chain LIBVIRT_FWI +ip6tables \ +--table filter \ +--insert FORWARD \ +--jump LIBVIRT_FWI +ip6tables \ +--table filter \ +--new-chain LIBVIRT_FWX +ip6tables \ +--table filter \ +--insert FORWARD \ +--jump LIBVIRT_FWX +ip6tables \ +--table nat \ +--new-chain LIBVIRT_PRT +ip6tables \ +--table nat \ +--insert POSTROUTING \ +--jump LIBVIRT_PRT +ip6tables \ +--table mangle \ +--new-chain LIBVIRT_PRT +ip6tables \ +--table mangle \ +--insert POSTROUTING \ +--jump LIBVIRT_PRT diff --git a/tests/networkxml2firewalltest.c b/tests/networkxml2firewalltest.c index 575b68379a..c25282ebb1 100644 --- a/tests/networkxml2firewalltest.c +++ b/tests/networkxml2firewalltest.c @@ -22,6 +22,7 @@ #include #include "testutils.h" +#include "viralloc.h" #if defined (__linux__) @@ -57,13 +58,15 @@ testCommandDryRun(const char *const*args ATTRIBUTE_UNUSED, } static int testCompareXMLToArgvFiles(const char *xml, - const char *cmdline) + const char *cmdline, + const char *baseargs) { char *expectargv = NULL; char *actualargv = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; virNetworkDefPtr def = NULL; int ret = -1; + char *actual; virCommandSetDryRun(&buf, testCommandDryRun, NULL); @@ -76,11 +79,18 @@ static int testCompareXMLToArgvFiles(const char *xml, if (virBufferError(&buf)) goto cleanup; - actualargv = virBufferContentAndReset(&buf); + actual = actualargv = virBufferContentAndReset(&buf); virTestClearCommandPath(actualargv); virCommandSetDryRun(NULL, NULL, NULL); - if (virTestCompareToFile(actualargv, cmdline) < 0) + /* The first network to be created populates the + * libvirt global chains. We must skip args for + * that if present + */ + if (STRPREFIX(actual, baseargs)) + actual += strlen(baseargs); + + if (virTestCompareToFile(actual, cmdline) < 0) goto cleanup; ret = 0; @@ -95,6 +105,7 @@ static int testCompareXMLToArgvFiles(const char *xml, struct testInfo { const char *name; + const char *baseargs; }; @@ -112,7 +123,7 @@ testCompareXMLToIPTablesHelper(const void *data) abs_srcdir, info->name, RULESTYPE) < 0) goto cleanup; - result = testCompareXMLToArgvFiles(xml, args); + result = testCompareXMLToArgvFiles(xml, args, info->baseargs); cleanup: VIR_FREE(xml); @@ -133,11 +144,13 @@ static int mymain(void) { int ret = 0; + VIR_AUTOFREE(char *)basefile = NULL; + VIR_AUTOFREE(char *)baseargs = NULL; # define DO_TEST(name) \ do { \ - static struct testInfo info = { \ - name, \ + struct testInfo info = { \ + name, baseargs, \ }; \ if (virTestRun("Network XML-2-iptables " name, \ testCompareXMLToIPTablesHelper, &info) < 0) \ @@ -156,6 +169,17 @@ mymain(void) goto cleanup; } + if (virAsprintf(&basefile, "%s/networkxml2firewalldata/base.args", + abs_srcdir) < 0) { + ret = -1; + goto cleanup; + } + + if (virTestLoadFile(basefile, &baseargs) < 0) { + ret = -1; + goto cleanup; + } + DO_TEST("nat-default"); DO_TEST("nat-tftp"); DO_TEST("nat-many-ips");