Re: [PATCH 0/8] network: firewalld: native support for NAT/routed

On Wed, Nov 16, 2022 at 09:40:41AM +0100, Michal Prívozník wrote: > On 11/15/22 23:16, Eric Garver wrote: >> On Tue, Nov 15, 2022 at 11:03:21AM -0500, Laine Stump wrote: >>> On 11/15/22 5:21 AM, Michal Prívozník wrote: >>>> On 11/10/22 17:31, Eric Garver wrote: >>>>> This series further improves the firewalld backend by converting to a >>>>> fully native implementation for NAT and routed networks. That is, there >>>>> are no iptables rules added by libvirt when the running firewalld is >>>>> 0.9.0 or later. >>>>> >>>>> The major advantage is that firewalld users can use firewall-cmd to >>>>> filter the VM traffic and apply their own policies. >>>>> >>>>> When firewalld < 0.9.0 is present only the "libvirt" zone will be used. >>>>> The new "libvirt-nat" and "libvirt-routed" zones are not used. This >>>>> maintains compatibility for older distributions (e.g. Ubuntu 20.04). >>>>> >>>>> Patch 1 is a bug fix for my previous series to avoid a bogus error log. >>>>> >>>>> Patches 2-3 converts the routed network to native firewalld. >>>>> >>>>> Patches 4-8 converts the NAT network to native firewalld. It also >>>>> introduces the "libvirt-nat" zone. >>>>> >>>>> Eric Garver (8): >>>>> util: virFirewallDGetPolicies: gracefully handle older firewalld >>>>> network: firewalld: add networkAddHybridFirewallDRules() >>>>> network: firewalld: use native routed networks >>>>> util: add virFirewallDSourceSetZone() >>>>> util: add virFirewallDApplyPolicyRichRules() >>>>> network: firewalld: add zone for NAT networks >>>>> network: firewalld: add policies for NAT networks >>>>> network: firewalld: use native NAT networks >>>>> >>>>> libvirt.spec.in | 2 + >>>>> src/libvirt_private.syms | 2 + >>>>> src/network/bridge_driver_linux.c | 193 ++++++++++++++++++++--------- >>>>> src/network/libvirt-nat-out.policy | 13 ++ >>>>> src/network/libvirt-nat.zone | 10 ++ >>>>> src/network/libvirt-to-host.policy | 1 + >>>>> src/network/meson.build | 10 ++ >>>>> src/util/virfirewalld.c | 79 +++++++++++- >>>>> src/util/virfirewalld.h | 6 + >>>>> 9 files changed, 258 insertions(+), 58 deletions(-) >>>>> create mode 100644 src/network/libvirt-nat-out.policy >>>>> create mode 100644 src/network/libvirt-nat.zone >>>>> >>>> >>>> Patches look good to me. You have my: >>>> >>>> Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com&gt; >>>> >>>> but I'll wait a bit for Laine, if he wants to express his opinion. >>> >>> This series has been on my list of things I need to get to since it arrived, >>> but I've been purposefully not responding in order to avoid distracting my >>> brain from something else I'm working on that is more urgent (supporting >>> passt as a guest interface connection mode). >>> >>> I have pending stuff (in-process on and off for many months now) that adds a >>> separate (configurable) backend for raw nftables that this firewalld-backend >>> mode needs to mesh with. In particular, I don't think it's safe to >>> automatically switch to using a pure firewalld backend any time firewalld is >>> running, because behavior isn't exactly the same as the standard iptables >>> backend (the first example that comes to mind is those horrible dhcp >>> checksum munging rules that are added by libvirt's iptables backend). >>> >>> Probably most of the patches in this series will be untouched by mine, or >>> should be prerequisites to mine, but some will need to be re-jiggered to use >>> my conf-file option and to deal with my other reorganizations. I'll look at >>> it in more detail as soon as I have a first version of passt patches posted, >>> which I'm hoping will happen sometime this week. >>> >>> So please don't push these patches (yet). >> >> Please take the first patch now. I can resend individually if you'd >> like. >> >> The rest we can sort out and re-spin after your series. > > Yeah, the first patch is independent of the rest so unless there's any > objection from Laine or Dan I'll push it later today. Yes, it looks fine.