Re: [PATCH v3 00/27] [PATCH v3 00/27] native support for nftables in virtual network driver

On Thu, Apr 25, 2024 at 01:38:06AM -0400, Laine Stump wrote: I deployed on my machine and restarted virtnetworkd, with nft backend active. I have 2 networks running, and got the following result table ip libvirt { chain INPUT { type filter hook input priority filter; policy accept; counter packets 363 bytes 30801 jump LIBVIRT_INP } chain FORWARD { type filter hook forward priority filter; policy accept; counter packets 1 bytes 76 jump LIBVIRT_FWX counter packets 1 bytes 76 jump LIBVIRT_FWI counter packets 1 bytes 76 jump LIBVIRT_FWO } chain OUTPUT { type filter hook output priority filter; policy accept; counter packets 286 bytes 107221 jump LIBVIRT_OUT } chain LIBVIRT_INP { iifname "virbr0" udp dport 53 counter packets 0 bytes 0 accept iifname "virbr0" tcp dport 53 counter packets 0 bytes 0 accept iifname "virbr0" udp dport 67 counter packets 1 bytes 320 accept iifname "virbr0" tcp dport 67 counter packets 0 bytes 0 accept iifname "virbr1" udp dport 53 counter packets 0 bytes 0 accept iifname "virbr1" tcp dport 53 counter packets 0 bytes 0 accept iifname "virbr1" udp dport 67 counter packets 0 bytes 0 accept iifname "virbr1" tcp dport 67 counter packets 0 bytes 0 accept } chain LIBVIRT_OUT { oifname "virbr0" udp dport 53 counter packets 0 bytes 0 accept oifname "virbr0" tcp dport 53 counter packets 0 bytes 0 accept oifname "virbr0" udp dport 68 counter packets 1 bytes 336 accept oifname "virbr0" tcp dport 68 counter packets 0 bytes 0 accept oifname "virbr1" udp dport 53 counter packets 0 bytes 0 accept oifname "virbr1" tcp dport 53 counter packets 0 bytes 0 accept oifname "virbr1" udp dport 68 counter packets 0 bytes 0 accept oifname "virbr1" tcp dport 68 counter packets 0 bytes 0 accept } chain LIBVIRT_FWO { ip saddr 192.168.122.0/24 iifname "virbr0" counter packets 1 bytes 76 accept iifname "virbr0" counter packets 0 bytes 0 reject ip saddr 192.168.100.0/24 iifname "virbr1" counter packets 0 bytes 0 accept iifname "virbr1" counter packets 0 bytes 0 reject } chain LIBVIRT_FWI { oifname "virbr0" ip daddr 192.168.122.0/24 ct state established,related counter packets 0 bytes 0 accept oifname "virbr0" counter packets 0 bytes 0 reject oifname "virbr1" ip daddr 192.168.100.0/24 ct state established,related counter packets 0 bytes 0 accept oifname "virbr1" counter packets 0 bytes 0 reject } chain LIBVIRT_FWX { iifname "virbr0" oifname "virbr0" counter packets 0 bytes 0 accept iifname "virbr1" oifname "virbr1" counter packets 0 bytes 0 accept } chain POSTROUTING { type nat hook postrouting priority srcnat; policy accept; counter packets 2 bytes 136 jump LIBVIRT_PRT } chain LIBVIRT_PRT { ip saddr 192.168.122.0/24 ip daddr 224.0.0.0/24 counter packets 0 bytes 0 return ip saddr 192.168.122.0/24 ip daddr 255.255.255.255 counter packets 0 bytes 0 return meta l4proto tcp ip saddr 192.168.122.0/24 ip daddr != 192.168.122.0/24 counter packets 0 bytes 0 masquerade to :1024-65535 meta l4proto udp ip saddr 192.168.122.0/24 ip daddr != 192.168.122.0/24 counter packets 1 bytes 76 masquerade to :1024-65535 ip saddr 192.168.122.0/24 ip daddr != 192.168.122.0/24 counter packets 0 bytes 0 masquerade ip saddr 192.168.100.0/24 ip daddr 224.0.0.0/24 counter packets 0 bytes 0 return ip saddr 192.168.100.0/24 ip daddr 255.255.255.255 counter packets 0 bytes 0 return meta l4proto tcp ip saddr 192.168.100.0/24 ip daddr != 192.168.100.0/24 counter packets 0 bytes 0 masquerade to :1024-65535 meta l4proto udp ip saddr 192.168.100.0/24 ip daddr != 192.168.100.0/24 counter packets 0 bytes 0 masquerade to :1024-65535 ip saddr 192.168.100.0/24 ip daddr != 192.168.100.0/24 counter packets 0 bytes 0 masquerade } } I also got some 'ip6' rules, but I'll omit that, since my points will be identical for both. 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. The main thought I have is around the chain structure and naming. We have INPUT -> LIBVIRT_INP OUTPUT -> LIBVIRT_OUT FORWARD -> LIBVIRT_FWX -> LIBVIRT_FWI -> LIBVIRT_FWO POSTROUTING -> LIBVIRT_PRT we introduced the top level "LIBVIRT_nnn" chains in the iptables world to cleanly separate ourselves from other users of the top level chains. In the nft world though, this use case is already handled, because we are namespaced in the 'libvirt' table instead. So aside from the FORWARD case, our extra LIBVIRT_nn chains are redundant, and the naming prefix is redundant too. Second observation is that we're not limited to short chain names in the nft world, and uppercase is not a common convention, just a hangover from iptables world. 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. 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. 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 :|