On 4/9/20 8:44 AM, Eric Garver wrote:
On Thu, Apr 09, 2020 at 11:53:46AM +0100, Daniel P. Berrangé wrote:
Copying Eric Garver as a knowledgeable maintainer of firewalld to
confirm a question I have....
On Fri, Mar 20, 2020 at 12:25:49PM +0300, nshirokovskiy wrote:
Hi, all.
Some time ago I posted RFC [1] concerning an issue of unresponsive
libvird during restart if there is large number of VMs that have network
filters on their interfaces. It was identified that in most cases we
don't need actually to reinstall network filter rules on daemon restart.
Thus I proposed patches [2] that check whether we need to reapply rules
or not.
The first version has a drawback that daemon won't reapply rules if
someone mangled them between daemon stop and start (and this can be done
just by restarting firewalld). The second one is just ugly :)
Around that time Florian Westphal in a letter off the mailing list
suggested to use {iptables|ebtables}-restore to apply rules in one
binary call. These binaries has --noflush option so that we won't reset
current state of tables.
Firewalld does exactly this. It's a significant performance gain.
iptables updates are effectively: grab the _entire_ ruleset from kernel,
modify the ruleset in userspace, then push the _entire_ ruleset back to
the kernel. This is very costly. Using iptables-restore means there are
less round trips to the kernel.
We also need one more -L call for
iptables/ebtables to query current filter state to be able to construct
input for restore binaries.
I wonder can we use this approach? I see currently only one issue - we
won't use firealld to spawn rules. But why we need to spawn rules
through firewalld if it present? We use passthrough mode anyway. I tried
to dig history for hints but didn't found anything. Patch [3] introduced
spawning rules thru firewalld-cmd.
For as long as libvirt has done firewall stuff we've have the issue
with other apps on the system breaking / discarding our rules. Originally
this was the "firewall" sysvinit script.
When firewalld came along, libvirt switched to creating rules using the
firwalld passthrough mode API, in the belief that any time firewalld
re-creates its rules, it would preserve any rules we'd created via the
passthrough mode.
The confusion may stem from the fact that there are _three_ different
"direct rule" APIs:
1) direct.addRule()
These are tracked by firewalld and will survive firewalld
reloads. It allows specifying a "priority", but rule indexing is
handled by firewalld.
2) direct.addPassthrough()
These are tracked by firewalld and will survive firewalld
reloads. The user must specify the "index" in the iptables
ruleset.
3) direct.passthrough()
There are _NOT_ tracked by firewalld. They will _NOT_ survive a
firewalld reload. This is a dumb passthrough to iptables.
Firewalld has no memory of their execution immediately after they
complete. I honestly don't understand why it exists as it
provides no value.
Unfortunately, libvirt currently uses #3 [1]. If libvirt used #2 then
libvirt rules would persist firewalld reloads with libvirt taking zero
action. firewalld reinstalls the addPassthrough rules after reload.
The downside to #2 is that if a firewalld user executes "firewall-cmd
--runtime-to-permanent" the libvirt rules will be copied to firewalld's
_permanent_ configuration. This may be the reason libvirt is using
passthrough (#3).
[1] https://gitlab.com/libvirt/libvirt/-/blob/1e2ae2e3/src/util/virfirewalld.c#L293-303
I vaguely recall some recentish discussions though where I think Eric
Garver mentioned we were mistaken, and that firewalld does *nothing*
to preserve passthrough mode rules.
See above.
Eric, from firewalld's POV, is there any functional difference between
an application directly creating rules by spawning "iptables", vs creating
the same rules via the firewalld passthrough API ?
If using direct.passthrough (#3 above), then no.
If there is no difference, then libvirt could stop using the firewalld
passthrough API, and switch to the iptables bulk load tools.
Seem sensible to me.
So in the end the only interaction we should have with firewalld is to
watch for it to restart, and reload all our rules when that happens. Sigh.
It will definitely be nice when firewalld is able to support all the
rules libvirt needs (including output, forward, and nat) via addRule()
[*]. (nudge nudge, wink wink :-)) The lack of a central authority for
managing iptables (and now nftables) rules has been a thorn in the side
for a very long time, and although it's obvious from this exchange that
switching back to always using iptables directly is the right thing to
do for now, it still feels like a step in the wrong direction.
[*] BTW, this points out that there needs to be a way to addRule() rules
such that they don't get moved into the permanent config when
"firewall-cmd --runtime-to-permanent" is run. It would be impossible for
libvirt to keep track of that.
