Re: [PATCH v3 00/27] [PATCH v3 00/27] native support for nftables in virtual network driver
On 4/25/24 1:22 PM, Daniel P. Berrangé wrote:
On Thu, Apr 25, 2024 at 01:38:06AM -0400, Laine Stump wrote:
> V2:
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/5R...
>
> This patch series enables libvirt to use nftables rules rather than
> iptables *when setting up virtual networks* (it does *not* add
> nftables support to the nwfilter driver).
I deployed on my machine and restarted virtnetworkd, with nft backend
active. I have 2 networks running, and got the following result
[...]
I compared these rules, against the nft rules created by the
iptables
compat shim. They were identical, aside from the expected difference
with checksum rules, so that's good.
It's good to know that the result is reproducible, and that I apparently
didn't miss anything.
The main thought I have is around the chain structure and naming.
[...]
If we consider what each chain is used for
* LIBVIRT_INPUT - incoming dnsmasq DHCP/DNS to the host
* LIBVIRT_OUTPUT - outgoing dnsmasq DHCP/DNS from the host
* LIBVIRT_FWX - crossing traffic between guests on a network
* LIBVIRT_FWI - incoming traffic to guests
* LIBVIRT_FWO - outgoing traffic from guests
* LIBVIRT_PRT - outgoing NAT from guest
Then I think we can change names thus:
input -> host_input
output -> host_output
forward -> guest_cross
-> guest_input
-> guest_output
postrouting -> guest_nat
so the names better reflect the purpose of each chain.
In theory we could eliminate the 'input', 'output' and
'postrouting' top
level chains, since they only have 1 rule in them. We can't eliminate the
'forward' chain though, as we need explicit ordernig of traversal for the
3 chains it links to. So on balance, its OK to keep them all I guess.
So in the end i'm only really asking for some chain renaming.
Sure - readability always gets extra points, and now that we've
established a working baseline that's identical to previous we can
modify with abandon :-)
Oh, actually one final thing. We called the tables
'libvirt'.
We have 2 possible users of nft - virtnetworkd and virnwfilterd.
I see no reason for them to use the same table. Cleanly separating
them would be nice, given they're totally indepedent daemons.
IOW, change the table names from 'libvirt' to 'libvirt_network'.
Thus in future we can also have 'libvirt_nwfilter' tables.
I hadn't thought of that, but it makes sense. I'm trying to think of a
way where it would be an advantage to have both use the same table, and
(without some serious active cooperation between the two drivers) I
can't come up with anything.
(and anyway, if we later decide that we want both to be in the same
table we can easily change it, and reloading virtnetworkd will create
the new table, remove the rules from the old table, and add them in the
new table (ie. it will be completely painless for the user)
I think I'll do this (and the change to remove the DNS/DHCP rules) as
separate patches rather than squasing them into the initial patch for
the nft backend. That way it will be obvious from git history what's
different in the rules, and will be easier to revert in case we later
find some reason that requires that (seems doubtful, but just in case)