On Fri, May 10, 2019 at 04:02:07PM -0600, Jim Fehlig wrote:
On 5/7/19 4:36 AM, Daniel P. Berrangé wrote:
On Tue, Apr 30, 2019 at 02:34:44PM -0600, Jim Fehlig wrote:
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@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.
The test file hackyness is something we had before, so not a big deal imho.
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);
This is problematic. It means that when upgrading libvirt we will never delete the legacy rules from the built-in chains.
We needed to create the chains upfront, so that when we recreate rules for existing running networks, we'll upgrade them to use the libvirt chains instead of built-in chains.
So I think we'll need to keep the code to create libvirt chains in this networkPreReloadFirewallRules, but *only* run it if we have existing virtual networks that are active.
That way when libvirt starts on fresh boot, chains will be crated only when a network is started. If libvirt is upgraded on running host, then we'll still do thoings early.
Thanks for the comments. I didn't have time to work on a V2 after travel and before a vacation. I'll be gone next week but will pick this up the following week after returning.
Don't worry about it, I'll probably get time to do an updated patch this week. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|