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

On Tue, Oct 29, 2024 at 05:36:02PM +0000, Daniel P. Berrangé wrote: > 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 :-) 'tc' controls QoS in Linux. 'qdisc's define how congestion should be handled (basically: queue or drop, prioritization, etc). The default qdisc for virtual devices like bridge or veth is "noqueue" - it sets the device's 'enqueue' callback to NULL and __dev_queue_xmit()[1] treats it accordingly (calls dev_hard_start_xmit() after a few checks to make sure the device is working). HTB is a container of classes (for packet classification) which themselves hold qdiscs. On my system at least, it doesn't come with default classes and thus should not do much by itself (apart from running the filter for us which we want. Anything else is overhead. I'm not sure how much detail you need - "as if I know nothing" is a bit like naively typing 'find /' and wondering when it will end. ;) Please shoot if you need more details. For the time being, let me point at some howto[2] I wrote long ago. >>>> 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. OK. From my perspective, attaching the tc qdisc/filter/action to individual guest devices would still be the cleaner solution. If there's no mechanism to attach this to, it might be easier to just stick everything to the bridge, of course.

Cheers, Phil [1] https://github.com/torvalds/linux/blob/master/net/core/dev.c#L4393 [2] http://linux-ip.net/gl/tc-filters/