On Thu, Nov 10, 2022 at 11:31:47AM -0500, Eric Garver wrote:
> The firewalld backend for routed networks can now use a native
> implementation. The hybrid of iptables + firewalld is no longer
> necessary. When full native firewalld is in use there are zero iptables
> rules add by libvirt.
>
> This is accomplished by returning early in networkAddFirewallRules() and
> avoiding calls to networkSetupPrivateChains().
>
> Signed-off-by: Eric Garver <eric@xxxxxxxxxxx>
> ---
> src/network/bridge_driver_linux.c | 51 +++++++++++++++++++++++++------
> 1 file changed, 42 insertions(+), 9 deletions(-)
>
> diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
> index 88a8e9c5fa27..42f098ff1f9b 100644
> --- a/src/network/bridge_driver_linux.c
> +++ b/src/network/bridge_driver_linux.c
> @@ -133,6 +133,21 @@ networkHasRunningNetworksWithFW(virNetworkDriverState *driver)
> }
>
>
> +static bool
> +networkUseOnlyFirewallDRules(void)
> +{
> + if (virFirewallDIsRegistered() < 0)
> + return false;
> +
> + if (virFirewallDPolicyExists("libvirt-routed-out") &&
> + virFirewallDZoneExists("libvirt-routed")) {
> + return true;
> + }
> +
> + return false;
> +}
> +
> +
> void
> networkPreReloadFirewallRules(virNetworkDriverState *driver,
> bool startup G_GNUC_UNUSED,
> @@ -172,6 +187,9 @@ networkPreReloadFirewallRules(virNetworkDriverState *driver,
> return;
> }
>
> + if (!chainInitDone && networkUseOnlyFirewallDRules())
> + return;
> +
> ignore_value(virOnce(&createdOnce, networkSetupPrivateChains));
> }
> }
> @@ -801,6 +819,18 @@ networkRemoveIPSpecificFirewallRules(virFirewall *fw,
> }
>
>
> +static int
> +networkAddOnlyFirewallDRules(virNetworkDef *def)
> +{
> + if (def->forward.type == VIR_NETWORK_FORWARD_ROUTE) {
> + if (virFirewallDInterfaceSetZone(def->bridge, "libvirt-routed") < 0)
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +
> static int
> networkAddHybridFirewallDRules(virNetworkDef *def)
> {
> @@ -860,6 +890,11 @@ int networkAddFirewallRules(virNetworkDef *def)
> virNetworkIPDef *ipdef;
> g_autoptr(virFirewall) fw = virFirewallNew();
>
> + if (!def->bridgeZone && networkUseOnlyFirewallDRules() &&
> + def->forward.type == VIR_NETWORK_FORWARD_ROUTE) {
> + return networkAddOnlyFirewallDRules(def);
> + }
> +
> if (virOnce(&createdOnce, networkSetupPrivateChains) < 0)
> return -1;
>
> @@ -895,15 +930,8 @@ int networkAddFirewallRules(virNetworkDef *def)
> return -1;
>
> } else if (virFirewallDIsRegistered() == 0) {
> - if (def->forward.type == VIR_NETWORK_FORWARD_ROUTE &&
> - virFirewallDPolicyExists("libvirt-routed-out") &&
> - virFirewallDZoneExists("libvirt-routed")) {
> - if (virFirewallDInterfaceSetZone(def->bridge, "libvirt-routed") < 0)
> - return -1;
> - } else {
> - if (networkAddHybridFirewallDRules(def) < 0)
> - return -1;
> - }
> + if (networkAddHybridFirewallDRules(def) < 0)
> + return -1;
> }
>
> virFirewallStartTransaction(fw, 0);
> @@ -940,6 +968,11 @@ void networkRemoveFirewallRules(virNetworkDef *def)
> virNetworkIPDef *ipdef;
> g_autoptr(virFirewall) fw = virFirewallNew();
>
> + if (!def->bridgeZone && networkUseOnlyFirewallDRules() &&
> + def->forward.type == VIR_NETWORK_FORWARD_ROUTE) {
> + return;
> + }
> +
This logic doesn't work well in the upgrade scenario.
Consider that we have existing running virtual networks, and
we upgrade libvirt in-place on the host.
During virtnetworkd startup, we tear down old firwall rules
and create the new ones. Except that we need to teardown the
old iptables rules, and this skips that because it decided we
need to use firewalld instead. So we're left with dangling
iptables rules on upgrade
> virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS);
> networkRemoveChecksumFirewallRules(fw, def);
With 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 :|
