[libvirt] [PATCH RFC] network: Delay creating private chains until starting network

[Jim Fehlig]

Automated performance tests found that network-centric workloads suffered
a 20 percent decrease when the host libvirt was updated from 5.0.0 to
5.1.0. On the test hosts libvirtd is enabled to start at boot and the
"default" network is defined, but it is not set to autostart.

libvirt 5.1.0 introduced private firewall chains with commit 5f1e6a7d.
The chains are created at libvirtd start, which triggers loading the
conntrack kernel module. Subsequent tracking and processing of
connections resulted in the performance hit. Prior to commit 5f1e6a7d
the conntrack module would not be loaded until starting a network,
when libvirt added rules to the builtin chains.

Restore the behavior of previous libvirt versions by delaying
the creation of private chains until the first network is started.

Signed-off-by: Jim Fehlig <jfehlig at suse.com>
---

I briefly discussed this issue with Daniel on IRC and just now finding
time to bring it to the list for broader discussion. The adjustment to
the test file feels hacky. The whole approach might by hacky, hence the
RFC.

 src/network/bridge_driver_linux.c             |  64 +++-------
 .../nat-default-linux.args                    | 116 ++++++++++++++++++
 2 files changed, 131 insertions(+), 49 deletions(-)

diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
index f2827543ca..a3a09caeba 100644
--- a/src/network/bridge_driver_linux.c
+++ b/src/network/bridge_driver_linux.c
@@ -35,44 +35,10 @@ VIR_LOG_INIT("network.bridge_driver_linux");
 
 #define PROC_NET_ROUTE "/proc/net/route"
 
-static virErrorPtr errInitV4;
-static virErrorPtr errInitV6;
+static bool pvtChainsCreated;
 
 void networkPreReloadFirewallRules(bool startup)
 {
-    bool created = false;
-    int rc;
-
-    /* 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.
-     *
-     * 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
-     */
-    rc = iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV4);
-    if (rc < 0) {
-        errInitV4 = virSaveLastError();
-        virResetLastError();
-    } else {
-        virFreeError(errInitV4);
-        errInitV4 = NULL;
-    }
-    if (rc)
-        created = true;
-
-    rc = iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV6);
-    if (rc < 0) {
-        errInitV6 = virSaveLastError();
-        virResetLastError();
-    } else {
-        virFreeError(errInitV6);
-        errInitV6 = NULL;
-    }
-    if (rc)
-        created = true;
-
     /*
      * If this is initial startup, and we just created the
      * top level private chains we either
@@ -86,8 +52,8 @@ void networkPreReloadFirewallRules(bool startup)
      * rules will be present. Thus we can safely just tell it
      * to always delete from the builin chain
      */
-    if (startup && created)
-        iptablesSetDeletePrivate(false);
+    if (startup)
+        iptablesSetDeletePrivate(true);
 }
 
 
@@ -701,19 +667,19 @@ int networkAddFirewallRules(virNetworkDefPtr def)
     virFirewallPtr fw = NULL;
     int ret = -1;
 
-    if (errInitV4 &&
-        (virNetworkDefGetIPByIndex(def, AF_INET, 0) ||
-         virNetworkDefGetRouteByIndex(def, AF_INET, 0))) {
-        virSetError(errInitV4);
-        return -1;
-    }
+    if (!pvtChainsCreated) {
+        if (iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV4) < 0 &&
+            (virNetworkDefGetIPByIndex(def, AF_INET, 0) ||
+             virNetworkDefGetRouteByIndex(def, AF_INET, 0)))
+            return -1;
 
-    if (errInitV6 &&
-        (virNetworkDefGetIPByIndex(def, AF_INET6, 0) ||
-         virNetworkDefGetRouteByIndex(def, AF_INET6, 0) ||
-         def->ipv6nogw)) {
-        virSetError(errInitV6);
-        return -1;
+        if (iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV6) < 0 &&
+            (virNetworkDefGetIPByIndex(def, AF_INET6, 0) ||
+             virNetworkDefGetRouteByIndex(def, AF_INET6, 0) ||
+             def->ipv6nogw))
+            return -1;
+
+        pvtChainsCreated = true;
     }
 
     if (def->bridgeZone) {
diff --git a/tests/networkxml2firewalldata/nat-default-linux.args b/tests/networkxml2firewalldata/nat-default-linux.args
index c9d523d043..61d620b592 100644
--- a/tests/networkxml2firewalldata/nat-default-linux.args
+++ b/tests/networkxml2firewalldata/nat-default-linux.args
@@ -1,5 +1,121 @@
 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
+iptables \
+--table filter \
 --insert LIBVIRT_INP \
 --in-interface virbr0 \
 --protocol tcp \
-- 
2.21.0