When libvirt added support for firewalld, we were unable to use
firewalld's higher level rules, because they weren't detailed enough
and could not be applied to the iptables FORWARD or OUTPUT chains
(only to the INPUT chain). Instead we changed our code so that rather
than running the iptables/ip6tables/ebtables binaries ourselves, we
would send these commands to firewalld as "passthrough commands", and
firewalld would run the appropriate program on our behalf.
This was done under the assumption that firewalld was somehow tracking
all these rules, and that this tracking was benefitting proper
operation of firewalld and the system in general.
Several years later this came up in a discussion on IRC, and we
learned from the firewalld developers that, in fact, adding iptables
and ebtables rules with firewalld's passthrough commands actually has
*no* advantage; firewalld doesn't keep track of these rules in any
way, and doesn't use them to tailor the construction of its own rules.
Meanwhile, users have been complaining for some time that whenever
firewalld is restarted on a system with libvirt virtual networks
and/or nwfilter rules active, the system logs would be flooded with
warning messages whining that [lots of different rules] could not be
deleted because they didn't exist. For example:
firewalld[3536040]: WARNING: COMMAND_FAILED:
'/usr/sbin/iptables -w10 -w --table filter --delete LIBVIRT_OUT
--out-interface virbr4 --protocol udp --destination-port 68
--jump ACCEPT' failed: iptables: Bad rule
(does a matching rule exist in that chain?).
(See https://bugzilla.redhat.com/1790837 for many more examples and a
discussion)
Note that these messages are created by iptables, but are logged by
firewalld - when an iptables/ebtables command fails, firewalld grabs
whatever is in stderr of the program, and spits it out to the system
log as a warning. We've requested that firewalld not do this (and
instead leave it up to the calling application to do the appropriate
logging), but this request has been respectfully denied.
But combining the two problems above ( 1) firewalld doesn't do
anything useful when you use it as a proxy to add/remove iptables
rules, 2) firewalld often insists on logging lots of
annoying/misleading/useless "error" messages when you use it as a
proxy to remove iptables rules that don't already exist), leads to a
solution - simply stop using firewalld to add and remove iptables
rules. Instead, exec iptables/ip6tables/ebtables directly in the same
way we do when firewalld isn't active.
We still need to keep track of whether or not firewalld is active, as
there are some things that must be done, e.g. we need to add some
actual firewalld rules in the firewalld "libvirt" zone, and we need to
take notice when firewalld restarts, so that we can reload all our
rules.
This patch doesn't remove the infrastructure that allows having
different firewall backends that perform their functions in different
ways, as that will very possibly come in handy in the future when we
want to have an nftables direct backend, and possibly a "pure"
firewalld backend (now that firewalld supports more complex rules, and
can add those rules to the FORWARD and OUTPUT chains). Instead, it
just changes the action when the selected backend is "firewalld" so
that it adds rules directly rather than through firewalld, while
leaving as much of the existing code intact as possible.
In order for tests to still pass, virfirewalltest also had to be
modified to behave in a different way (i.e. by capturing the generated
commandline as it does for the DIRECT backend, rather than capturing
dbus messages using a mocked dbus API).
Signed-off-by: Laine Stump <laine@redhat.com>
---
src/util/virfirewall.c | 13 +++++++++++--
tests/virfirewalltest.c | 30 ++++++++++++++++++++----------
2 files changed, 31 insertions(+), 12 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 307825331d..21dea3013a 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -640,7 +640,7 @@ virFirewallApplyRuleDirect(virFirewallRulePtr rule,
}
-static int
+static int G_GNUC_UNUSED
virFirewallApplyRuleFirewallD(virFirewallRulePtr rule,
bool ignoreErrors,
char **output)
@@ -698,7 +698,16 @@ virFirewallApplyRule(virFirewallPtr firewall,
return -1;
break;
case VIR_FIREWALL_BACKEND_FIREWALLD:
- if (virFirewallApplyRuleFirewallD(rule, ignoreErrors, &output) < 0)
+ /* Since we are using raw iptables rules, there is no
+ * advantage to going through firewalld, so instead just add
+ * them directly rather that via dbus calls to firewalld. This
+ * has the useful side effect of eliminating extra unwanted
+ * warning messages in the system logs when trying to delete
+ * rules that don't exist (which is something that happens
+ * often when libvirtd is started, and *always* when firewalld
+ * is restarted)
+ */
+ if (virFirewallApplyRuleDirect(rule, ignoreErrors, &output) < 0)
return -1;
break;
diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c
index fa1838a499..2670eb1561 100644
--- a/tests/virfirewalltest.c
+++ b/tests/virfirewalltest.c
@@ -194,7 +194,8 @@ testFirewallSingleGroup(const void *opaque)
if (virFirewallSetBackend(data->tryBackend) < 0)
goto cleanup;
- if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT)
+ if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
+ data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD)
virCommandSetDryRun(&cmdbuf, NULL, NULL);
else
fwBuf = &cmdbuf;
@@ -247,7 +248,8 @@ testFirewallRemoveRule(const void *opaque)
if (virFirewallSetBackend(data->tryBackend) < 0)
goto cleanup;
- if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT)
+ if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
+ data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD)
virCommandSetDryRun(&cmdbuf, NULL, NULL);
else
fwBuf = &cmdbuf;
@@ -307,7 +309,8 @@ testFirewallManyGroups(const void *opaque G_GNUC_UNUSED)
if (virFirewallSetBackend(data->tryBackend) < 0)
goto cleanup;
- if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT)
+ if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
+ data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD)
virCommandSetDryRun(&cmdbuf, NULL, NULL);
else
fwBuf = &cmdbuf;
@@ -394,7 +397,8 @@ testFirewallIgnoreFailGroup(const void *opaque G_GNUC_UNUSED)
if (virFirewallSetBackend(data->tryBackend) < 0)
goto cleanup;
- if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
+ if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
+ data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD) {
virCommandSetDryRun(&cmdbuf, testFirewallRollbackHook, NULL);
} else {
fwBuf = &cmdbuf;
@@ -462,7 +466,8 @@ testFirewallIgnoreFailRule(const void *opaque G_GNUC_UNUSED)
if (virFirewallSetBackend(data->tryBackend) < 0)
goto cleanup;
- if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
+ if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
+ data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD) {
virCommandSetDryRun(&cmdbuf, testFirewallRollbackHook, NULL);
} else {
fwBuf = &cmdbuf;
@@ -527,7 +532,8 @@ testFirewallNoRollback(const void *opaque G_GNUC_UNUSED)
if (virFirewallSetBackend(data->tryBackend) < 0)
goto cleanup;
- if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
+ if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
+ data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD) {
virCommandSetDryRun(&cmdbuf, testFirewallRollbackHook, NULL);
} else {
fwBuf = &cmdbuf;
@@ -590,7 +596,8 @@ testFirewallSingleRollback(const void *opaque G_GNUC_UNUSED)
if (virFirewallSetBackend(data->tryBackend) < 0)
goto cleanup;
- if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
+ if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
+ data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD) {
virCommandSetDryRun(&cmdbuf, testFirewallRollbackHook, NULL);
} else {
fwError = true;
@@ -669,7 +676,8 @@ testFirewallManyRollback(const void *opaque G_GNUC_UNUSED)
if (virFirewallSetBackend(data->tryBackend) < 0)
goto cleanup;
- if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
+ if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
+ data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD) {
virCommandSetDryRun(&cmdbuf, testFirewallRollbackHook, NULL);
} else {
fwBuf = &cmdbuf;
@@ -756,7 +764,8 @@ testFirewallChainedRollback(const void *opaque G_GNUC_UNUSED)
if (virFirewallSetBackend(data->tryBackend) < 0)
goto cleanup;
- if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
+ if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
+ data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD) {
virCommandSetDryRun(&cmdbuf, testFirewallRollbackHook, NULL);
} else {
fwBuf = &cmdbuf;
@@ -951,7 +960,8 @@ testFirewallQuery(const void *opaque G_GNUC_UNUSED)
if (virFirewallSetBackend(data->tryBackend) < 0)
goto cleanup;
- if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
+ if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
+ data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD) {
virCommandSetDryRun(&cmdbuf, testFirewallQueryHook, NULL);
} else {
fwBuf = &cmdbuf;
--
2.28.0
On 11/24/20 12:30 AM, Laine Stump wrote:
> When libvirt added support for firewalld, we were unable to use
> firewalld's higher level rules, because they weren't detailed enough
> and could not be applied to the iptables FORWARD or OUTPUT chains
> (only to the INPUT chain). Instead we changed our code so that rather
> than running the iptables/ip6tables/ebtables binaries ourselves, we
> would send these commands to firewalld as "passthrough commands", and
> firewalld would run the appropriate program on our behalf.
>
> This was done under the assumption that firewalld was somehow tracking
> all these rules, and that this tracking was benefitting proper
> operation of firewalld and the system in general.
>
> Several years later this came up in a discussion on IRC, and we
> learned from the firewalld developers that, in fact, adding iptables
> and ebtables rules with firewalld's passthrough commands actually has
> *no* advantage; firewalld doesn't keep track of these rules in any
> way, and doesn't use them to tailor the construction of its own rules.
That's odd. Why bother providing the mechanism if you're not building
anything upon it? Might as well drop it entirely and tell users to just
use the command line. At least it'll not cause confusion.
I was going to ask you to mention the man page/doc where it is stated that
the passthrough commands aren't tracked, but I have a hunch that this might
not be officially documented (I just searched in 'man firewalld.direct' and
'man firewalld', haven't found any info about this behavior). So yeah, it
seems like "a guy on IRC told us that ..." is as far as we'll have to
document this, ATM.
>
> Meanwhile, users have been complaining for some time that whenever
> firewalld is restarted on a system with libvirt virtual networks
> and/or nwfilter rules active, the system logs would be flooded with
> warning messages whining that [lots of different rules] could not be
> deleted because they didn't exist. For example:
>
> firewalld[3536040]: WARNING: COMMAND_FAILED:
> '/usr/sbin/iptables -w10 -w --table filter --delete LIBVIRT_OUT
> --out-interface virbr4 --protocol udp --destination-port 68
> --jump ACCEPT' failed: iptables: Bad rule
> (does a matching rule exist in that chain?).
>
> (See https://bugzilla.redhat.com/1790837 for many more examples and a
> discussion)
>
> Note that these messages are created by iptables, but are logged by
> firewalld - when an iptables/ebtables command fails, firewalld grabs
> whatever is in stderr of the program, and spits it out to the system
> log as a warning. We've requested that firewalld not do this (and
> instead leave it up to the calling application to do the appropriate
> logging), but this request has been respectfully denied.
>
> But combining the two problems above ( 1) firewalld doesn't do
> anything useful when you use it as a proxy to add/remove iptables
> rules, 2) firewalld often insists on logging lots of
> annoying/misleading/useless "error" messages when you use it as a
> proxy to remove iptables rules that don't already exist), leads to a
> solution - simply stop using firewalld to add and remove iptables
> rules. Instead, exec iptables/ip6tables/ebtables directly in the same
> way we do when firewalld isn't active.
>
> We still need to keep track of whether or not firewalld is active, as
> there are some things that must be done, e.g. we need to add some
> actual firewalld rules in the firewalld "libvirt" zone, and we need to
> take notice when firewalld restarts, so that we can reload all our
> rules.
>
> This patch doesn't remove the infrastructure that allows having
> different firewall backends that perform their functions in different
> ways, as that will very possibly come in handy in the future when we
> want to have an nftables direct backend, and possibly a "pure"
> firewalld backend (now that firewalld supports more complex rules, and
> can add those rules to the FORWARD and OUTPUT chains). Instead, it
> just changes the action when the selected backend is "firewalld" so
> that it adds rules directly rather than through firewalld, while
> leaving as much of the existing code intact as possible.
After reading all patches I can't help but think that, in the end,
you're having to dance around firewalld ankwardness just to get regular
use case to work properly. Queing 'iptables -V' to not get Libvirt rules
overwritten by firewalld purge protocol in patch 7 represents my 2020
experience: every day getting flabbergasted by random stuff that
shouldn't happen.
Anyway, good story!
DHB
>
> In order for tests to still pass, virfirewalltest also had to be
> modified to behave in a different way (i.e. by capturing the generated
> commandline as it does for the DIRECT backend, rather than capturing
> dbus messages using a mocked dbus API).
>
> Signed-off-by: Laine Stump <laine@redhat.com>
> ---
> src/util/virfirewall.c | 13 +++++++++++--
> tests/virfirewalltest.c | 30 ++++++++++++++++++++----------
> 2 files changed, 31 insertions(+), 12 deletions(-)
>
> diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
> index 307825331d..21dea3013a 100644
> --- a/src/util/virfirewall.c
> +++ b/src/util/virfirewall.c
> @@ -640,7 +640,7 @@ virFirewallApplyRuleDirect(virFirewallRulePtr rule,
> }
>
>
> -static int
> +static int G_GNUC_UNUSED
> virFirewallApplyRuleFirewallD(virFirewallRulePtr rule,
> bool ignoreErrors,
> char **output)
> @@ -698,7 +698,16 @@ virFirewallApplyRule(virFirewallPtr firewall,
> return -1;
> break;
> case VIR_FIREWALL_BACKEND_FIREWALLD:
> - if (virFirewallApplyRuleFirewallD(rule, ignoreErrors, &output) < 0)
> + /* Since we are using raw iptables rules, there is no
> + * advantage to going through firewalld, so instead just add
> + * them directly rather that via dbus calls to firewalld. This
> + * has the useful side effect of eliminating extra unwanted
> + * warning messages in the system logs when trying to delete
> + * rules that don't exist (which is something that happens
> + * often when libvirtd is started, and *always* when firewalld
> + * is restarted)
> + */
> + if (virFirewallApplyRuleDirect(rule, ignoreErrors, &output) < 0)
> return -1;
> break;
>
> diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c
> index fa1838a499..2670eb1561 100644
> --- a/tests/virfirewalltest.c
> +++ b/tests/virfirewalltest.c
> @@ -194,7 +194,8 @@ testFirewallSingleGroup(const void *opaque)
> if (virFirewallSetBackend(data->tryBackend) < 0)
> goto cleanup;
>
> - if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT)
> + if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
> + data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD)
> virCommandSetDryRun(&cmdbuf, NULL, NULL);
> else
> fwBuf = &cmdbuf;
> @@ -247,7 +248,8 @@ testFirewallRemoveRule(const void *opaque)
> if (virFirewallSetBackend(data->tryBackend) < 0)
> goto cleanup;
>
> - if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT)
> + if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
> + data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD)
> virCommandSetDryRun(&cmdbuf, NULL, NULL);
> else
> fwBuf = &cmdbuf;
> @@ -307,7 +309,8 @@ testFirewallManyGroups(const void *opaque G_GNUC_UNUSED)
> if (virFirewallSetBackend(data->tryBackend) < 0)
> goto cleanup;
>
> - if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT)
> + if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
> + data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD)
> virCommandSetDryRun(&cmdbuf, NULL, NULL);
> else
> fwBuf = &cmdbuf;
> @@ -394,7 +397,8 @@ testFirewallIgnoreFailGroup(const void *opaque G_GNUC_UNUSED)
> if (virFirewallSetBackend(data->tryBackend) < 0)
> goto cleanup;
>
> - if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
> + if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
> + data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD) {
> virCommandSetDryRun(&cmdbuf, testFirewallRollbackHook, NULL);
> } else {
> fwBuf = &cmdbuf;
> @@ -462,7 +466,8 @@ testFirewallIgnoreFailRule(const void *opaque G_GNUC_UNUSED)
> if (virFirewallSetBackend(data->tryBackend) < 0)
> goto cleanup;
>
> - if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
> + if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
> + data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD) {
> virCommandSetDryRun(&cmdbuf, testFirewallRollbackHook, NULL);
> } else {
> fwBuf = &cmdbuf;
> @@ -527,7 +532,8 @@ testFirewallNoRollback(const void *opaque G_GNUC_UNUSED)
> if (virFirewallSetBackend(data->tryBackend) < 0)
> goto cleanup;
>
> - if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
> + if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
> + data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD) {
> virCommandSetDryRun(&cmdbuf, testFirewallRollbackHook, NULL);
> } else {
> fwBuf = &cmdbuf;
> @@ -590,7 +596,8 @@ testFirewallSingleRollback(const void *opaque G_GNUC_UNUSED)
> if (virFirewallSetBackend(data->tryBackend) < 0)
> goto cleanup;
>
> - if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
> + if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
> + data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD) {
> virCommandSetDryRun(&cmdbuf, testFirewallRollbackHook, NULL);
> } else {
> fwError = true;
> @@ -669,7 +676,8 @@ testFirewallManyRollback(const void *opaque G_GNUC_UNUSED)
> if (virFirewallSetBackend(data->tryBackend) < 0)
> goto cleanup;
>
> - if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
> + if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
> + data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD) {
> virCommandSetDryRun(&cmdbuf, testFirewallRollbackHook, NULL);
> } else {
> fwBuf = &cmdbuf;
> @@ -756,7 +764,8 @@ testFirewallChainedRollback(const void *opaque G_GNUC_UNUSED)
> if (virFirewallSetBackend(data->tryBackend) < 0)
> goto cleanup;
>
> - if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
> + if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
> + data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD) {
> virCommandSetDryRun(&cmdbuf, testFirewallRollbackHook, NULL);
> } else {
> fwBuf = &cmdbuf;
> @@ -951,7 +960,8 @@ testFirewallQuery(const void *opaque G_GNUC_UNUSED)
> if (virFirewallSetBackend(data->tryBackend) < 0)
> goto cleanup;
>
> - if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT) {
> + if (data->expectBackend == VIR_FIREWALL_BACKEND_DIRECT ||
> + data->expectBackend == VIR_FIREWALL_BACKEND_FIREWALLD) {
> virCommandSetDryRun(&cmdbuf, testFirewallQueryHook, NULL);
> } else {
> fwBuf = &cmdbuf;
>
On 11/24/20 7:01 AM, Daniel Henrique Barboza wrote: > > > On 11/24/20 12:30 AM, Laine Stump wrote: >> When libvirt added support for firewalld, we were unable to use >> firewalld's higher level rules, because they weren't detailed enough >> and could not be applied to the iptables FORWARD or OUTPUT chains >> (only to the INPUT chain). Instead we changed our code so that rather >> than running the iptables/ip6tables/ebtables binaries ourselves, we >> would send these commands to firewalld as "passthrough commands", and >> firewalld would run the appropriate program on our behalf. >> >> This was done under the assumption that firewalld was somehow tracking >> all these rules, and that this tracking was benefitting proper >> operation of firewalld and the system in general. >> >> Several years later this came up in a discussion on IRC, and we >> learned from the firewalld developers that, in fact, adding iptables >> and ebtables rules with firewalld's passthrough commands actually has >> *no* advantage; firewalld doesn't keep track of these rules in any >> way, and doesn't use them to tailor the construction of its own rules. > > > That's odd. Why bother providing the mechanism if you're not building > anything upon it? Sometimes ideas don't work out and/or plans change, and there are multiple levels of stuff based on those earlier ideas that are already in place. > Might as well drop it entirely and tell users to just > use the command line. Actually, when firewalld added in their nftables backend, they did not add support for adding nftables rules via their passthrough command API. But dropping something "entirely" from the API first requires that all users of that part of the API stop using it. This entire dance can take years. > At least it'll not cause confusion. > > I was going to ask you to mention the man page/doc where it is stated > that > the passthrough commands aren't tracked, but I have a hunch that this > might > not be officially documented (I just searched in 'man > firewalld.direct' and > 'man firewalld', haven't found any info about this behavior). So yeah, it > seems like "a guy on IRC told us that ..." is as far as we'll have to > document this, ATM. Well, not just "a guy on IRC", but Eric Garver, the main upstream maintainer/developer of firewalld. > > >> >> Meanwhile, users have been complaining for some time that whenever >> firewalld is restarted on a system with libvirt virtual networks >> and/or nwfilter rules active, the system logs would be flooded with >> warning messages whining that [lots of different rules] could not be >> deleted because they didn't exist. For example: >> >> firewalld[3536040]: WARNING: COMMAND_FAILED: >> '/usr/sbin/iptables -w10 -w --table filter --delete LIBVIRT_OUT >> --out-interface virbr4 --protocol udp --destination-port 68 >> --jump ACCEPT' failed: iptables: Bad rule >> (does a matching rule exist in that chain?). >> >> (See https://bugzilla.redhat.com/1790837 for many more examples and a >> discussion) >> >> Note that these messages are created by iptables, but are logged by >> firewalld - when an iptables/ebtables command fails, firewalld grabs >> whatever is in stderr of the program, and spits it out to the system >> log as a warning. We've requested that firewalld not do this (and >> instead leave it up to the calling application to do the appropriate >> logging), but this request has been respectfully denied. >> >> But combining the two problems above ( 1) firewalld doesn't do >> anything useful when you use it as a proxy to add/remove iptables >> rules, 2) firewalld often insists on logging lots of >> annoying/misleading/useless "error" messages when you use it as a >> proxy to remove iptables rules that don't already exist), leads to a >> solution - simply stop using firewalld to add and remove iptables >> rules. Instead, exec iptables/ip6tables/ebtables directly in the same >> way we do when firewalld isn't active. >> >> We still need to keep track of whether or not firewalld is active, as >> there are some things that must be done, e.g. we need to add some >> actual firewalld rules in the firewalld "libvirt" zone, and we need to >> take notice when firewalld restarts, so that we can reload all our >> rules. >> >> This patch doesn't remove the infrastructure that allows having >> different firewall backends that perform their functions in different >> ways, as that will very possibly come in handy in the future when we >> want to have an nftables direct backend, and possibly a "pure" >> firewalld backend (now that firewalld supports more complex rules, and >> can add those rules to the FORWARD and OUTPUT chains). Instead, it >> just changes the action when the selected backend is "firewalld" so >> that it adds rules directly rather than through firewalld, while >> leaving as much of the existing code intact as possible. > > > After reading all patches I can't help but think that, in the end, > you're having to dance around firewalld ankwardness just to get regular > use case to work properly. Well, firewalld would like it if it was the only arbiter of access to a host's packet filtering engine(s). That's closer to being a possibility (e.g. firewalld can now add rules to the FORWARD and OUTPUT chains) but still not reality, and until it is a reality, other software will need to synchronize itself with firewalld. In the meantime, it would have been nice if firewalld provided a "startup completed" event for people like libvirt to use for synchronization, but it doesn't. We *could* insist that firewalld add such an event, and remain broken until it was provided, but this clever hack gives us the same effects, can be done immediately, and will work with any version of firewalld, so it's a win-win-win. > Queing 'iptables -V' to not get Libvirt rules > overwritten by firewalld purge protocol in patch 7 represents my 2020 > experience: every day getting flabbergasted by random stuff that > shouldn't happen. Yes, "random stuff that shouldn't happen" would be a nice synopsis for this year. > > > > Anyway, good story! ... as would this :-)
© 2016 - 2026 Red Hat, Inc.