Re: [PATCH v2] network: add rule to nftables backend that zeroes checksum of DHCP responses

On Tue, Oct 29, 2024 at 06:29:55PM +0100, Phil Sutter wrote: > On Tue, Oct 29, 2024 at 03:38:08PM +0000, Daniel P. Berrangé wrote: > > On Tue, Oct 29, 2024 at 04:12:16PM +0100, Phil Sutter wrote: > > > Hi, > > > > > > On Tue, Oct 29, 2024 at 09:30:27AM -0400, Laine Stump wrote: > > > > So when the extra rules are removed, then those same guests begin > > > > working? (You can easily remove the checksum rules with: > > > > > > > > nft delete chain ip libvirt_network postroute_mangle > > > > > > > > BTW, I just now tried an e1000e NIC on Fedora guest and it continues to > > > > work with the 0-checksum rules removed. In this case tcpdump on virbr0 > > > > shows "bad cksum", but when I look at tcpdump on the guest, it shows > > > > "udp cksum ok" though, so something else somewhere is setting the > > > > checksum to the correct value. > > > > > > FWIW, I just tested an alternative workaround using tc. This works for > > > me with a FreeBSD guest and NIC switched to either e1000 or virtio: > > > > > > # tc qd add dev vnetbr0 root handle 1: htb > > > # tc filter add dev vnetbr0 prio 1 protocol ip parent 1: \ > > > u32 match ip sport 67 ffff match ip dport 68 ffff \ > > > action csum ip and udp > > > > This feels like it is functionally closest to what we've had historically, > > even though it is annoying to have to deal with 'tc' tool, in addition > > to 'nft'. So I'm thinking this is probably the way we'll have to go. > > Another ugly detail (inherent to 'tc') is that you have to attach a > classful qdisc to the interface since otherwise you can't add a filter > with attached action. While this may not be a problem in practice, there > is this side-effect of setting up a HTB on the bridge which by default > runs a "noqueue" qdisc. I'm not that familiar with 'tc'. Can you explain the functional effect of those 'qdisc' settings on virbr0, as if I know nothing :-)

> > > Another alternative might be to add the nftables rule for virtio-based > > > guests only. > > > > The firewall rules are in a chain that's applied to all guests, > > so we have no where to add a per-guest rule. > > With nftables, you may create a chain in netdev family which binds to > the specific device(s) needing the hack. It needs maintenance after > guest startup and shutdown, though. > > BTW: libvirt supports configurations which don't involve a 'vnetbr0' > bridge. In this case, you will have to setup tc on the actual tap > device, right? In those cases, we haven't historically set firewall rules, so users were on their own, so in that sense, it isn't a regression we need to solve. Also in those cases, the DHCP daemon would be off-host, and so packets we're getting back from it ought to have a checksum present, as they've been over a physical link.