[PATCH (RFC and a half?)] network: add rule to nftables backend that zeroes checksum of DHCP responses
Many long years ago (April 2010), soon after "vhost" in-kernel packet processing was added to the virtio-net driver, people running RHEL5 virtual machines with a virtio-net interface connected via a libvirt virtual network noticed that when vhost packet processing was enabled, their VMs could no longer get an IP address via DHCP - the guest was ignoring the DHCP response packets sent by the host. The (as danpb calls them) "gory details" of this are chronicled here: https://lists.isc.org/pipermail/dhcp-hackers/2010-April/001835.html but basically it was because the checksum of packets wasn't being fully computed on the host side (because the host had checksum offloading enabled and thought that it would be taken care of later, e.g. with NIC hardware), while these packets going from a tap device to a virtio-net NIC in a guest wouldn't get that service, and the packets would arrive with a "bad checksum". The "fix" for this ended up being that iptables added a new "--checksum-fill" action, and libvirt added an iptables rule for each virtual network to match DHCP response packets and perform --checksum-fill. In the meantime, the ISC DHCP package (which contains the dhclient program that had been rejecting the bad checksum packets) made a separate fix to their dhclient which caused it to accept packets anyway even if they didn't have a proper checksum (NB: that's not a full explanation, and possibly not accurate). The word at the time from those "in the know" was that the bad checksum problem was really specific to ISC's dhclient, and so once their fix was in use everywhere dhclient was used, the problem would be a thing of the past and the checksum fixup iptables rules would no longer be needed (but would otherwise be harmless if it was still there). Based on this information (and also due to the opinion that fixing the problem by having iptables modify the packet checksum was the wrong way to fix things), the nftables developers made the decision to not implement an equivalent to --checksum-fill in nftables. As a result, when I wrote the nftables firewall backend for libvirt virtual networks, it didn't add in any rule to "fix" broken UDP checksums (after all, that was fixed somewhere else 14 years ago, right???) Cut to last week, when Rich Jones was doing routine testing using Fedora 40 (the first Fedora release to use the nftables backend of libvirt's network driver by default) and a FreeBSD guest - for "some strange reason", the FreeBSD guest was unable to get an IP address from DHCP!! https://www.spinics.net/linux/fedora/libvirt-users/msg14356.html A few quick tests proved that it was the same old "bad checksum" problem from 2010 come back to haunt us. After some discussion with Phil Sutter and Eric Garver (nftables people), they suggested that, while nftables doesn't have an action that will *compute* the checksum of a packet, it *does* have an action that will set the checksum to 0, and that maybe we should try that. Then Phil tried it himself by manually adding such a rule to a running system, and verified that it did fix the issue at least for FreeBSD guests. So over the weekend I came up with a patch to add a checksum 0 rule to the rules setup for each virtual network. This is that patch. I have so far verified that this patch enables FreeBSD to receive the DHCP response and get an IP address, and that it hasn't *broken* this functionality for a random old Fedora image I had (Fedora 27!?!?! I really need to update my test images!!). Before pushing it I would like to verify that zeroing the checksum of DHCP response packets doesn't break any other guest, so I would appreciate the help of anyone who could build and install libvirt with this patch and let me know of both successes and failures of any guest to acquire an IP address with DHCP. Once I've received enough positive reports (and 0 negative reports!) then we can think about pushing this patch (and also backporting it downstream to Fedora 40) Signed-off-by: Laine Stump <laine@redhat.com> --- src/network/network_nftables.c | 69 +++++++++++++++++++ .../forward-dev-linux.nftables | 16 +++++ .../isolated-linux.nftables | 16 +++++ .../nat-default-linux.nftables | 16 +++++ .../nat-ipv6-linux.nftables | 16 +++++ .../nat-ipv6-masquerade-linux.nftables | 16 +++++ .../nat-many-ips-linux.nftables | 16 +++++ .../nat-port-range-ipv6-linux.nftables | 16 +++++ .../nat-port-range-linux.nftables | 16 +++++ .../nat-tftp-linux.nftables | 16 +++++ .../route-default-linux.nftables | 16 +++++ 11 files changed, 229 insertions(+) diff --git a/src/network/network_nftables.c b/src/network/network_nftables.c index f8b5ab665d..5523207269 100644 --- a/src/network/network_nftables.c +++ b/src/network/network_nftables.c @@ -51,6 +51,7 @@ VIR_LOG_INIT("network.nftables"); #define VIR_NFTABLES_FWD_OUT_CHAIN "guest_output" #define VIR_NFTABLES_FWD_X_CHAIN "guest_cross" #define VIR_NFTABLES_NAT_POSTROUTE_CHAIN "guest_nat" +#define VIR_NFTABLES_MANGLE_POSTROUTE_CHAIN "postroute_mangle" /* we must avoid using the standard "filter" table as used by * iptables, as any subsequent attempts to use iptables commands will @@ -106,6 +107,10 @@ nftablesGlobalChain nftablesChains[] = { /* chains for NAT rules */ {NULL, VIR_NFTABLES_NAT_POSTROUTE_CHAIN, "{ type nat hook postrouting priority 100; policy accept; }"}, + + /* chain for "mangle" rules that modify packets (e.g. 0 out UDP checksums) */ + {NULL, VIR_NFTABLES_MANGLE_POSTROUTE_CHAIN, "{ type filter hook postrouting priority 0; policy accept; }"}, + }; @@ -644,6 +649,44 @@ nftablesAddDontMasquerade(virFirewall *fw, } +/** + * nftablesAddOutputFixUdpChecksum: + * + * Add a rule to @fw that will 0 out the checksum of udp packets + * output from @iface with destination port @port. + + * Zeroing the checksum of a UDP packet tells the receiving end "you + * don't need to validate the checksum", which is useful in cases + * where the host (sender) thinks that packet checksums will be + * computed elsewhere (and so leaves a partially computed checksum in + * the packet header) while the guest (receiver) thinks that the + * checksum has already been fully computed; in the meantime none of + * the code in between has actually finished computing the + * checksum. + * + * An example of this is DHCP response packets from host to + * guest. If the checksum of each of these packets isn't zeroed, then + * many guests (e.g. FreeBSD) will drop them with reason BAD CHECKSUM; + * if the packets arrive at those guests with a checksum of 0, they + * will happily accept the packet. + */ +static void +nftablesAddOutputFixUdpChecksum(virFirewall *fw, + const char *iface, + int port) +{ + g_autofree char *portstr = g_strdup_printf("%d", port); + + virFirewallAddCmd(fw, VIR_FIREWALL_LAYER_IPV4, + "insert", "rule", "ip", + VIR_NFTABLES_PRIVATE_TABLE, + VIR_NFTABLES_MANGLE_POSTROUTE_CHAIN, + "oif", iface, "udp", "dport", portstr, + "counter", "udp", "checksum", "set", "0", + NULL); +} + + static const char networkLocalMulticastIPv4[] = "224.0.0.0/24"; static const char networkLocalMulticastIPv6[] = "ff02::/16"; static const char networkLocalBroadcast[] = "255.255.255.255/32"; @@ -901,6 +944,30 @@ nftablesAddGeneralFirewallRules(virFirewall *fw, } +static void +nftablesAddChecksumFirewallRules(virFirewall *fw, + virNetworkDef *def) +{ + size_t i; + virNetworkIPDef *ipv4def; + + /* Look for the first IPv4 address that has dhcp or tftpboot + * defined. We support dhcp config on 1 IPv4 interface only. + */ + for (i = 0; (ipv4def = virNetworkDefGetIPByIndex(def, AF_INET, i)); i++) { + if (ipv4def->nranges || ipv4def->nhosts) + break; + } + + /* If we are doing local DHCP service on this network, add a rule + * that will fixup the checksum of DHCP response packets back to + * the guests. + */ + if (ipv4def) + nftablesAddOutputFixUdpChecksum(fw, def->bridge, 68); +} + + static int nftablesAddIPSpecificFirewallRules(virFirewall *fw, virNetworkDef *def, @@ -952,6 +1019,8 @@ nftablesAddFirewallRules(virNetworkDef *def, virFirewall **fwRemoval) return -1; } + nftablesAddChecksumFirewallRules(fw, def); + if (virFirewallApply(fw) < 0) return -1; diff --git a/tests/networkxml2firewalldata/forward-dev-linux.nftables b/tests/networkxml2firewalldata/forward-dev-linux.nftables index 8badb74beb..9dea1a88a4 100644 --- a/tests/networkxml2firewalldata/forward-dev-linux.nftables +++ b/tests/networkxml2firewalldata/forward-dev-linux.nftables @@ -156,3 +156,19 @@ daddr \ 224.0.0.0/24 \ counter \ return +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0 diff --git a/tests/networkxml2firewalldata/isolated-linux.nftables b/tests/networkxml2firewalldata/isolated-linux.nftables index d1b4dac178..67ee0a2bf5 100644 --- a/tests/networkxml2firewalldata/isolated-linux.nftables +++ b/tests/networkxml2firewalldata/isolated-linux.nftables @@ -62,3 +62,19 @@ oif \ virbr0 \ counter \ accept +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0 diff --git a/tests/networkxml2firewalldata/nat-default-linux.nftables b/tests/networkxml2firewalldata/nat-default-linux.nftables index 28508292f9..951a5a6d60 100644 --- a/tests/networkxml2firewalldata/nat-default-linux.nftables +++ b/tests/networkxml2firewalldata/nat-default-linux.nftables @@ -142,3 +142,19 @@ daddr \ 224.0.0.0/24 \ counter \ return +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0 diff --git a/tests/networkxml2firewalldata/nat-ipv6-linux.nftables b/tests/networkxml2firewalldata/nat-ipv6-linux.nftables index d8a9ba706d..617ed8b753 100644 --- a/tests/networkxml2firewalldata/nat-ipv6-linux.nftables +++ b/tests/networkxml2firewalldata/nat-ipv6-linux.nftables @@ -200,3 +200,19 @@ oif \ virbr0 \ counter \ accept +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0 diff --git a/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables b/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables index a7f09cda59..a710d0e296 100644 --- a/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables +++ b/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables @@ -272,3 +272,19 @@ daddr \ ff02::/16 \ counter \ return +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0 diff --git a/tests/networkxml2firewalldata/nat-many-ips-linux.nftables b/tests/networkxml2firewalldata/nat-many-ips-linux.nftables index b826fe6134..0be5fb7e65 100644 --- a/tests/networkxml2firewalldata/nat-many-ips-linux.nftables +++ b/tests/networkxml2firewalldata/nat-many-ips-linux.nftables @@ -366,3 +366,19 @@ daddr \ 224.0.0.0/24 \ counter \ return +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0 diff --git a/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables b/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables index ceaed6fa40..7574356855 100644 --- a/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables +++ b/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables @@ -384,3 +384,19 @@ daddr \ ff02::/16 \ counter \ return +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0 diff --git a/tests/networkxml2firewalldata/nat-port-range-linux.nftables b/tests/networkxml2firewalldata/nat-port-range-linux.nftables index 1dc37a26ec..127536e4db 100644 --- a/tests/networkxml2firewalldata/nat-port-range-linux.nftables +++ b/tests/networkxml2firewalldata/nat-port-range-linux.nftables @@ -312,3 +312,19 @@ oif \ virbr0 \ counter \ accept +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0 diff --git a/tests/networkxml2firewalldata/nat-tftp-linux.nftables b/tests/networkxml2firewalldata/nat-tftp-linux.nftables index 28508292f9..951a5a6d60 100644 --- a/tests/networkxml2firewalldata/nat-tftp-linux.nftables +++ b/tests/networkxml2firewalldata/nat-tftp-linux.nftables @@ -142,3 +142,19 @@ daddr \ 224.0.0.0/24 \ counter \ return +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0 diff --git a/tests/networkxml2firewalldata/route-default-linux.nftables b/tests/networkxml2firewalldata/route-default-linux.nftables index 282c9542a5..be9c4f5439 100644 --- a/tests/networkxml2firewalldata/route-default-linux.nftables +++ b/tests/networkxml2firewalldata/route-default-linux.nftables @@ -56,3 +56,19 @@ oif \ virbr0 \ counter \ accept +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0 -- 2.47.0
FYI I've also tested this with Win10 and it works fine. It would be nice to get this into the upcoming release if possible (nudge nudge, wink wink) On 10/21/24 12:14 AM, Laine Stump wrote:
Many long years ago (April 2010), soon after "vhost" in-kernel packet processing was added to the virtio-net driver, people running RHEL5 virtual machines with a virtio-net interface connected via a libvirt virtual network noticed that when vhost packet processing was enabled, their VMs could no longer get an IP address via DHCP - the guest was ignoring the DHCP response packets sent by the host.
The (as danpb calls them) "gory details" of this are chronicled here:
https://lists.isc.org/pipermail/dhcp-hackers/2010-April/001835.html
but basically it was because the checksum of packets wasn't being fully computed on the host side (because the host had checksum offloading enabled and thought that it would be taken care of later, e.g. with NIC hardware), while these packets going from a tap device to a virtio-net NIC in a guest wouldn't get that service, and the packets would arrive with a "bad checksum".
The "fix" for this ended up being that iptables added a new "--checksum-fill" action, and libvirt added an iptables rule for each virtual network to match DHCP response packets and perform --checksum-fill.
In the meantime, the ISC DHCP package (which contains the dhclient program that had been rejecting the bad checksum packets) made a separate fix to their dhclient which caused it to accept packets anyway even if they didn't have a proper checksum (NB: that's not a full explanation, and possibly not accurate). The word at the time from those "in the know" was that the bad checksum problem was really specific to ISC's dhclient, and so once their fix was in use everywhere dhclient was used, the problem would be a thing of the past and the checksum fixup iptables rules would no longer be needed (but would otherwise be harmless if it was still there).
Based on this information (and also due to the opinion that fixing the problem by having iptables modify the packet checksum was the wrong way to fix things), the nftables developers made the decision to not implement an equivalent to --checksum-fill in nftables. As a result, when I wrote the nftables firewall backend for libvirt virtual networks, it didn't add in any rule to "fix" broken UDP checksums (after all, that was fixed somewhere else 14 years ago, right???)
Cut to last week, when Rich Jones was doing routine testing using Fedora 40 (the first Fedora release to use the nftables backend of libvirt's network driver by default) and a FreeBSD guest - for "some strange reason", the FreeBSD guest was unable to get an IP address from DHCP!!
https://www.spinics.net/linux/fedora/libvirt-users/msg14356.html
A few quick tests proved that it was the same old "bad checksum" problem from 2010 come back to haunt us.
After some discussion with Phil Sutter and Eric Garver (nftables people), they suggested that, while nftables doesn't have an action that will *compute* the checksum of a packet, it *does* have an action that will set the checksum to 0, and that maybe we should try that. Then Phil tried it himself by manually adding such a rule to a running system, and verified that it did fix the issue at least for FreeBSD guests.
So over the weekend I came up with a patch to add a checksum 0 rule to the rules setup for each virtual network. This is that patch.
I have so far verified that this patch enables FreeBSD to receive the DHCP response and get an IP address, and that it hasn't *broken* this functionality for a random old Fedora image I had (Fedora 27!?!?! I really need to update my test images!!). Before pushing it I would like to verify that zeroing the checksum of DHCP response packets doesn't break any other guest, so I would appreciate the help of anyone who could build and install libvirt with this patch and let me know of both successes and failures of any guest to acquire an IP address with DHCP. Once I've received enough positive reports (and 0 negative reports!) then we can think about pushing this patch (and also backporting it downstream to Fedora 40)
Signed-off-by: Laine Stump <laine@redhat.com> --- src/network/network_nftables.c | 69 +++++++++++++++++++ .../forward-dev-linux.nftables | 16 +++++ .../isolated-linux.nftables | 16 +++++ .../nat-default-linux.nftables | 16 +++++ .../nat-ipv6-linux.nftables | 16 +++++ .../nat-ipv6-masquerade-linux.nftables | 16 +++++ .../nat-many-ips-linux.nftables | 16 +++++ .../nat-port-range-ipv6-linux.nftables | 16 +++++ .../nat-port-range-linux.nftables | 16 +++++ .../nat-tftp-linux.nftables | 16 +++++ .../route-default-linux.nftables | 16 +++++ 11 files changed, 229 insertions(+)
diff --git a/src/network/network_nftables.c b/src/network/network_nftables.c index f8b5ab665d..5523207269 100644 --- a/src/network/network_nftables.c +++ b/src/network/network_nftables.c @@ -51,6 +51,7 @@ VIR_LOG_INIT("network.nftables"); #define VIR_NFTABLES_FWD_OUT_CHAIN "guest_output" #define VIR_NFTABLES_FWD_X_CHAIN "guest_cross" #define VIR_NFTABLES_NAT_POSTROUTE_CHAIN "guest_nat" +#define VIR_NFTABLES_MANGLE_POSTROUTE_CHAIN "postroute_mangle"
/* we must avoid using the standard "filter" table as used by * iptables, as any subsequent attempts to use iptables commands will @@ -106,6 +107,10 @@ nftablesGlobalChain nftablesChains[] = {
/* chains for NAT rules */ {NULL, VIR_NFTABLES_NAT_POSTROUTE_CHAIN, "{ type nat hook postrouting priority 100; policy accept; }"}, + + /* chain for "mangle" rules that modify packets (e.g. 0 out UDP checksums) */ + {NULL, VIR_NFTABLES_MANGLE_POSTROUTE_CHAIN, "{ type filter hook postrouting priority 0; policy accept; }"}, + };
@@ -644,6 +649,44 @@ nftablesAddDontMasquerade(virFirewall *fw, }
+/** + * nftablesAddOutputFixUdpChecksum: + * + * Add a rule to @fw that will 0 out the checksum of udp packets + * output from @iface with destination port @port. + + * Zeroing the checksum of a UDP packet tells the receiving end "you + * don't need to validate the checksum", which is useful in cases + * where the host (sender) thinks that packet checksums will be + * computed elsewhere (and so leaves a partially computed checksum in + * the packet header) while the guest (receiver) thinks that the + * checksum has already been fully computed; in the meantime none of + * the code in between has actually finished computing the + * checksum. + * + * An example of this is DHCP response packets from host to + * guest. If the checksum of each of these packets isn't zeroed, then + * many guests (e.g. FreeBSD) will drop them with reason BAD CHECKSUM; + * if the packets arrive at those guests with a checksum of 0, they + * will happily accept the packet. + */ +static void +nftablesAddOutputFixUdpChecksum(virFirewall *fw, + const char *iface, + int port) +{ + g_autofree char *portstr = g_strdup_printf("%d", port); + + virFirewallAddCmd(fw, VIR_FIREWALL_LAYER_IPV4, + "insert", "rule", "ip", + VIR_NFTABLES_PRIVATE_TABLE, + VIR_NFTABLES_MANGLE_POSTROUTE_CHAIN, + "oif", iface, "udp", "dport", portstr, + "counter", "udp", "checksum", "set", "0", + NULL); +} + + static const char networkLocalMulticastIPv4[] = "224.0.0.0/24"; static const char networkLocalMulticastIPv6[] = "ff02::/16"; static const char networkLocalBroadcast[] = "255.255.255.255/32"; @@ -901,6 +944,30 @@ nftablesAddGeneralFirewallRules(virFirewall *fw, }
+static void +nftablesAddChecksumFirewallRules(virFirewall *fw, + virNetworkDef *def) +{ + size_t i; + virNetworkIPDef *ipv4def; + + /* Look for the first IPv4 address that has dhcp or tftpboot + * defined. We support dhcp config on 1 IPv4 interface only. + */ + for (i = 0; (ipv4def = virNetworkDefGetIPByIndex(def, AF_INET, i)); i++) { + if (ipv4def->nranges || ipv4def->nhosts) + break; + } + + /* If we are doing local DHCP service on this network, add a rule + * that will fixup the checksum of DHCP response packets back to + * the guests. + */ + if (ipv4def) + nftablesAddOutputFixUdpChecksum(fw, def->bridge, 68); +} + + static int nftablesAddIPSpecificFirewallRules(virFirewall *fw, virNetworkDef *def, @@ -952,6 +1019,8 @@ nftablesAddFirewallRules(virNetworkDef *def, virFirewall **fwRemoval) return -1; }
+ nftablesAddChecksumFirewallRules(fw, def); + if (virFirewallApply(fw) < 0) return -1;
diff --git a/tests/networkxml2firewalldata/forward-dev-linux.nftables b/tests/networkxml2firewalldata/forward-dev-linux.nftables index 8badb74beb..9dea1a88a4 100644 --- a/tests/networkxml2firewalldata/forward-dev-linux.nftables +++ b/tests/networkxml2firewalldata/forward-dev-linux.nftables @@ -156,3 +156,19 @@ daddr \ 224.0.0.0/24 \ counter \ return +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0 diff --git a/tests/networkxml2firewalldata/isolated-linux.nftables b/tests/networkxml2firewalldata/isolated-linux.nftables index d1b4dac178..67ee0a2bf5 100644 --- a/tests/networkxml2firewalldata/isolated-linux.nftables +++ b/tests/networkxml2firewalldata/isolated-linux.nftables @@ -62,3 +62,19 @@ oif \ virbr0 \ counter \ accept +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0 diff --git a/tests/networkxml2firewalldata/nat-default-linux.nftables b/tests/networkxml2firewalldata/nat-default-linux.nftables index 28508292f9..951a5a6d60 100644 --- a/tests/networkxml2firewalldata/nat-default-linux.nftables +++ b/tests/networkxml2firewalldata/nat-default-linux.nftables @@ -142,3 +142,19 @@ daddr \ 224.0.0.0/24 \ counter \ return +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0 diff --git a/tests/networkxml2firewalldata/nat-ipv6-linux.nftables b/tests/networkxml2firewalldata/nat-ipv6-linux.nftables index d8a9ba706d..617ed8b753 100644 --- a/tests/networkxml2firewalldata/nat-ipv6-linux.nftables +++ b/tests/networkxml2firewalldata/nat-ipv6-linux.nftables @@ -200,3 +200,19 @@ oif \ virbr0 \ counter \ accept +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0 diff --git a/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables b/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables index a7f09cda59..a710d0e296 100644 --- a/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables +++ b/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables @@ -272,3 +272,19 @@ daddr \ ff02::/16 \ counter \ return +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0 diff --git a/tests/networkxml2firewalldata/nat-many-ips-linux.nftables b/tests/networkxml2firewalldata/nat-many-ips-linux.nftables index b826fe6134..0be5fb7e65 100644 --- a/tests/networkxml2firewalldata/nat-many-ips-linux.nftables +++ b/tests/networkxml2firewalldata/nat-many-ips-linux.nftables @@ -366,3 +366,19 @@ daddr \ 224.0.0.0/24 \ counter \ return +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0 diff --git a/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables b/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables index ceaed6fa40..7574356855 100644 --- a/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables +++ b/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables @@ -384,3 +384,19 @@ daddr \ ff02::/16 \ counter \ return +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0 diff --git a/tests/networkxml2firewalldata/nat-port-range-linux.nftables b/tests/networkxml2firewalldata/nat-port-range-linux.nftables index 1dc37a26ec..127536e4db 100644 --- a/tests/networkxml2firewalldata/nat-port-range-linux.nftables +++ b/tests/networkxml2firewalldata/nat-port-range-linux.nftables @@ -312,3 +312,19 @@ oif \ virbr0 \ counter \ accept +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0 diff --git a/tests/networkxml2firewalldata/nat-tftp-linux.nftables b/tests/networkxml2firewalldata/nat-tftp-linux.nftables index 28508292f9..951a5a6d60 100644 --- a/tests/networkxml2firewalldata/nat-tftp-linux.nftables +++ b/tests/networkxml2firewalldata/nat-tftp-linux.nftables @@ -142,3 +142,19 @@ daddr \ 224.0.0.0/24 \ counter \ return +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0 diff --git a/tests/networkxml2firewalldata/route-default-linux.nftables b/tests/networkxml2firewalldata/route-default-linux.nftables index 282c9542a5..be9c4f5439 100644 --- a/tests/networkxml2firewalldata/route-default-linux.nftables +++ b/tests/networkxml2firewalldata/route-default-linux.nftables @@ -56,3 +56,19 @@ oif \ virbr0 \ counter \ accept +nft \ +-ae insert \ +rule \ +ip \ +libvirt_network \ +postroute_mangle \ +oif \ +virbr0 \ +udp \ +dport \ +68 \ +counter \ +udp \ +checksum \ +set \ +0
On Mon, Oct 21, 2024 at 12:14:38AM -0400, Laine Stump wrote:
Many long years ago (April 2010), soon after "vhost" in-kernel packet processing was added to the virtio-net driver, people running RHEL5 virtual machines with a virtio-net interface connected via a libvirt virtual network noticed that when vhost packet processing was enabled, their VMs could no longer get an IP address via DHCP - the guest was ignoring the DHCP response packets sent by the host.
The (as danpb calls them) "gory details" of this are chronicled here:
https://lists.isc.org/pipermail/dhcp-hackers/2010-April/001835.html
but basically it was because the checksum of packets wasn't being fully computed on the host side (because the host had checksum offloading enabled and thought that it would be taken care of later, e.g. with NIC hardware), while these packets going from a tap device to a virtio-net NIC in a guest wouldn't get that service, and the packets would arrive with a "bad checksum".
AFAIR, it isn't actually a bug with virtio-net usage as this last bit suggests. Rather it is a result of feature negotiation with QEMU on the host, whereby the guest & QEMU mutually agree to turn off checksums because they are redundant when the "link" is just local memory not a physical cable. IOW, packets don't arrive in the guest with a bad checksum. They arrive in the guest with no checksum *as requested* by the guest. The DHCP client decides this is a bad checksum, as it wasn't aware of the checksum offload usage.
The "fix" for this ended up being that iptables added a new "--checksum-fill" action, and libvirt added an iptables rule for each virtual network to match DHCP response packets and perform --checksum-fill.
In the meantime, the ISC DHCP package (which contains the dhclient program that had been rejecting the bad checksum packets) made a separate fix to their dhclient which caused it to accept packets anyway even if they didn't have a proper checksum (NB: that's not a full explanation, and possibly not accurate). The word at the time> q from those "in the know" was that the bad checksum problem was really specific to ISC's dhclient, and so once their fix was in use everywhere dhclient was used, the problem would be a thing of the past and the checksum fixup iptables rules would no longer be needed (but would otherwise be harmless if it was still there).
The fix did indeed work correctly for dhclient.... on linux ! The fix relied on a Linux specific sockets API extension, and thus wasn't applicable to non-Linux codepaths in dhclient AFAICT.
Based on this information (and also due to the opinion that fixing the problem by having iptables modify the packet checksum was the wrong way to fix things), the nftables developers made the decision to not implement an equivalent to --checksum-fill in nftables. As a result, when I wrote the nftables firewall backend for libvirt virtual networks, it didn't add in any rule to "fix" broken UDP checksums (after all, that was fixed somewhere else 14 years ago, right???)
....and in Fedora/RHEL context it was fixed 18 years ago, as we first hit this when working on Xen integration in 2006 :-)
A few quick tests proved that it was the same old "bad checksum" problem from 2010 come back to haunt us.
2006 :-)
After some discussion with Phil Sutter and Eric Garver (nftables people), they suggested that, while nftables doesn't have an action that will *compute* the checksum of a packet, it *does* have an action that will set the checksum to 0, and that maybe we should try that. Then Phil tried it himself by manually adding such a rule to a running system, and verified that it did fix the issue at least for FreeBSD guests.
So over the weekend I came up with a patch to add a checksum 0 rule to the rules setup for each virtual network. This is that patch.
I have so far verified that this patch enables FreeBSD to receive the DHCP response and get an IP address, and that it hasn't *broken* this functionality for a random old Fedora image I had (Fedora 27!?!?! I really need to update my test images!!). Before pushing it I would like to verify that zeroing the checksum of DHCP response packets doesn't break any other guest, so I would appreciate the help of anyone who could build and install libvirt with this patch and let me know of both successes and failures of any guest to acquire an IP address with DHCP. Once I've received enough positive reports (and 0 negative reports!) then we can think about pushing this patch (and also backporting it downstream to Fedora 40)
On the one hand it is good that you test this and found it to to work. What concerns me is a lack of understanding of /why/ it works. AFAICT there is nothing in the TCP RFC documenting all-zeros as a special case for indicating absent checksums. I'd really like to know /why/ it works, so we can be confident we're relying on intentional behaviour, as opposed to a happy accident. Functionally your patch does what it claims to do, so codewise I'm happy to say Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>, but I'd rather not merge it without a deeper understandnig. 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 :|
On Thu, Oct 24, 2024 at 05:36:10PM +0100, Daniel P. Berrangé wrote:
On Mon, Oct 21, 2024 at 12:14:38AM -0400, Laine Stump wrote:
After some discussion with Phil Sutter and Eric Garver (nftables people), they suggested that, while nftables doesn't have an action that will *compute* the checksum of a packet, it *does* have an action that will set the checksum to 0, and that maybe we should try that. Then Phil tried it himself by manually adding such a rule to a running system, and verified that it did fix the issue at least for FreeBSD guests.
So over the weekend I came up with a patch to add a checksum 0 rule to the rules setup for each virtual network. This is that patch.
I have so far verified that this patch enables FreeBSD to receive the DHCP response and get an IP address, and that it hasn't *broken* this functionality for a random old Fedora image I had (Fedora 27!?!?! I really need to update my test images!!). Before pushing it I would like to verify that zeroing the checksum of DHCP response packets doesn't break any other guest, so I would appreciate the help of anyone who could build and install libvirt with this patch and let me know of both successes and failures of any guest to acquire an IP address with DHCP. Once I've received enough positive reports (and 0 negative reports!) then we can think about pushing this patch (and also backporting it downstream to Fedora 40)
On the one hand it is good that you test this and found it to to work.
What concerns me is a lack of understanding of /why/ it works.
AFAICT there is nothing in the TCP RFC documenting all-zeros as a special case for indicating absent checksums.
Doh, we're dealing with UDP for DHCP, not TCP /facepalm The UDP RFC 768 says [quote] If the computed checksum is zero, it is transmitted as all ones (the equivalent in one's complement arithmetic). An all zero transmitted checksum value means that the transmitter generated no checksum (for debugging or for higher level protocols that don't care). [/quote]
I'd really like to know /why/ it works, so we can be confident we're relying on intentional behaviour, as opposed to a happy accident.
I've finally found the dhclient code that makes this work https://github.com/isc-projects/dhcp/blob/master/common/packet.c#L365 /* UDP check sum may be optional (udp.uh_sum == 0) or not ready if checksum * offloading is in use */ udp_packets_seen++; if (udp.uh_sum && csum_ready) { /* Check the UDP header checksum - since the received packet header * contains the UDP checksum calculated by the transmitter, calculating * it now should come out to zero. */ .... IOW, if 'uh_sum' is all zeroes, then it skips checksum validation, which is exactly what we want.
Functionally your patch does what it claims to do, so codewise I'm happy to say Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>, but I'd rather not merge it without a deeper understandnig.
We can go ahead and merge this, as its matching specified UDP behaviour. 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 :|
On Thu, Oct 24, 2024 at 06:42:55PM +0100, Daniel P. Berrangé wrote:
On Thu, Oct 24, 2024 at 05:36:10PM +0100, Daniel P. Berrangé wrote:
On Mon, Oct 21, 2024 at 12:14:38AM -0400, Laine Stump wrote:
After some discussion with Phil Sutter and Eric Garver (nftables people), they suggested that, while nftables doesn't have an action that will *compute* the checksum of a packet, it *does* have an action that will set the checksum to 0, and that maybe we should try that. Then Phil tried it himself by manually adding such a rule to a running system, and verified that it did fix the issue at least for FreeBSD guests.
So over the weekend I came up with a patch to add a checksum 0 rule to the rules setup for each virtual network. This is that patch.
I have so far verified that this patch enables FreeBSD to receive the DHCP response and get an IP address, and that it hasn't *broken* this functionality for a random old Fedora image I had (Fedora 27!?!?! I really need to update my test images!!). Before pushing it I would like to verify that zeroing the checksum of DHCP response packets doesn't break any other guest, so I would appreciate the help of anyone who could build and install libvirt with this patch and let me know of both successes and failures of any guest to acquire an IP address with DHCP. Once I've received enough positive reports (and 0 negative reports!) then we can think about pushing this patch (and also backporting it downstream to Fedora 40)
On the one hand it is good that you test this and found it to to work.
What concerns me is a lack of understanding of /why/ it works.
AFAICT there is nothing in the TCP RFC documenting all-zeros as a special case for indicating absent checksums.
Doh, we're dealing with UDP for DHCP, not TCP /facepalm
The UDP RFC 768 says
[quote] If the computed checksum is zero, it is transmitted as all ones (the equivalent in one's complement arithmetic). An all zero transmitted checksum value means that the transmitter generated no checksum (for debugging or for higher level protocols that don't care). [/quote]
I'd really like to know /why/ it works, so we can be confident we're relying on intentional behaviour, as opposed to a happy accident.
I've finally found the dhclient code that makes this work
https://github.com/isc-projects/dhcp/blob/master/common/packet.c#L365
/* UDP check sum may be optional (udp.uh_sum == 0) or not ready if checksum * offloading is in use */ udp_packets_seen++; if (udp.uh_sum && csum_ready) { /* Check the UDP header checksum - since the received packet header * contains the UDP checksum calculated by the transmitter, calculating * it now should come out to zero. */ ....
IOW, if 'uh_sum' is all zeroes, then it skips checksum validation, which is exactly what we want.
For another example, OpenBSD does NOT use ISC dhclient, and also explicitly skips treating an all-zeros checksum as an error https://github.com/openbsd/src/blob/master/sbin/dhcpleased/engine.c#L855 if (usum != 0 && usum != sum) { log_warnx("%s: bad UDP checksum", __func__); return; }
Functionally your patch does what it claims to do, so codewise I'm happy to say Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>, but I'd rather not merge it without a deeper understandnig.
We can go ahead and merge this, as its matching specified UDP behaviour.
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 :|
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 :|
On 10/24/24 12:36 PM, Daniel P. Berrangé wrote:
On Mon, Oct 21, 2024 at 12:14:38AM -0400, Laine Stump wrote:
Many long years ago (April 2010), soon after "vhost" in-kernel packet processing was added to the virtio-net driver, people running RHEL5 virtual machines with a virtio-net interface connected via a libvirt virtual network noticed that when vhost packet processing was enabled, their VMs could no longer get an IP address via DHCP - the guest was ignoring the DHCP response packets sent by the host.
The (as danpb calls them) "gory details" of this are chronicled here:
https://lists.isc.org/pipermail/dhcp-hackers/2010-April/001835.html
but basically it was because the checksum of packets wasn't being fully computed on the host side (because the host had checksum offloading enabled and thought that it would be taken care of later, e.g. with NIC hardware), while these packets going from a tap device to a virtio-net NIC in a guest wouldn't get that service, and the packets would arrive with a "bad checksum".
AFAIR, it isn't actually a bug with virtio-net usage as this last bit suggests. Rather it is a result of feature negotiation with QEMU on the host, whereby the guest & QEMU mutually agree to turn off checksums because they are redundant when the "link" is just local memory not a physical cable.
IOW, packets don't arrive in the guest with a bad checksum. They arrive in the guest with no checksum *as requested* by the guest.
The DHCP client decides this is a bad checksum, as it wasn't aware of the checksum offload usage.
The "fix" for this ended up being that iptables added a new "--checksum-fill" action, and libvirt added an iptables rule for each virtual network to match DHCP response packets and perform --checksum-fill.
In the meantime, the ISC DHCP package (which contains the dhclient program that had been rejecting the bad checksum packets) made a separate fix to their dhclient which caused it to accept packets anyway even if they didn't have a proper checksum (NB: that's not a full explanation, and possibly not accurate). The word at the time> q from those "in the know" was that the bad checksum problem was really specific to ISC's dhclient, and so once their fix was in use everywhere dhclient was used, the problem would be a thing of the past and the checksum fixup iptables rules would no longer be needed (but would otherwise be harmless if it was still there).
The fix did indeed work correctly for dhclient.... on linux !
The fix relied on a Linux specific sockets API extension, and thus wasn't applicable to non-Linux codepaths in dhclient AFAICT.
Based on this information (and also due to the opinion that fixing the problem by having iptables modify the packet checksum was the wrong way to fix things), the nftables developers made the decision to not implement an equivalent to --checksum-fill in nftables. As a result, when I wrote the nftables firewall backend for libvirt virtual networks, it didn't add in any rule to "fix" broken UDP checksums (after all, that was fixed somewhere else 14 years ago, right???)
....and in Fedora/RHEL context it was fixed 18 years ago, as we first hit this when working on Xen integration in 2006 :-)
A few quick tests proved that it was the same old "bad checksum" problem from 2010 come back to haunt us.
2006 :-)
After some discussion with Phil Sutter and Eric Garver (nftables people), they suggested that, while nftables doesn't have an action that will *compute* the checksum of a packet, it *does* have an action that will set the checksum to 0, and that maybe we should try that. Then Phil tried it himself by manually adding such a rule to a running system, and verified that it did fix the issue at least for FreeBSD guests.
So over the weekend I came up with a patch to add a checksum 0 rule to the rules setup for each virtual network. This is that patch.
I have so far verified that this patch enables FreeBSD to receive the DHCP response and get an IP address, and that it hasn't *broken* this functionality for a random old Fedora image I had (Fedora 27!?!?! I really need to update my test images!!). Before pushing it I would like to verify that zeroing the checksum of DHCP response packets doesn't break any other guest, so I would appreciate the help of anyone who could build and install libvirt with this patch and let me know of both successes and failures of any guest to acquire an IP address with DHCP. Once I've received enough positive reports (and 0 negative reports!) then we can think about pushing this patch (and also backporting it downstream to Fedora 40)
On the one hand it is good that you test this and found it to to work.
What concerns me is a lack of understanding of /why/ it works.
AFAICT there is nothing in the TCP RFC documenting all-zeros as a special case for indicating absent checksums.
Well, RFC768 says this:
If the computed checksum is zero, it is transmitted as all ones (the equivalent in one's complement arithmetic). An all zero transmitted checksum value means that the transmitter generated no checksum (for debugging or for higher level protocols that don't care).
(so a checksum of 0 as an actual computed checksum is never possible, and there are cases where a sender might no compute a checksum and the receiver *could* still accept the packet as valid). The Wikipedia entry for UDP says this:
UDP checksum computation is optional for IPv4. If a checksum is not used it should be set to the value zero.
Again implying that 0 can be used to indicate that a checksum wasn't computed but the packet can still be accepted. Another indicator is that tcpdump will display "[udp sum ok]" if the checksum in the packet matches what is computed, and "[no cksum]" if the checksum is 0, but "[bad udp cksum 0xXXXX -> 0xYYYY!]" if the checksum is any other value and not correct. So this *implies* special treatment of a checksum of 0. Also there is an entire RFC (6936) dedicated to the topic "Applicability Statement for the Use of IPv6 UDP Datagrams with Zero Checksums" but of course that's not applicable here. I didn't find a similar RFC for IPv4.
I'd really like to know /why/ it works, so we can be confident we're relying on intentional behaviour, as opposed to a happy accident.
Many searches have led me to statements that in IPv4 UDP packets, a checksum of 0 means "checksum not done/needed", but no official document that says "if a UDP packet has a checksum of 0 it MUST be accepted as valid". So .... *shrug*.
Functionally your patch does what it claims to do, so codewise I'm happy to say Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>, but I'd rather not merge it without a deeper understandnig.
Just as I was hitting send jdenemar notified me you'd responded to yourself with one of the same quotes :-). I want to fix the commit log message based on your more-correct info, so I'll try to do that and re-send so you can verify and we can get history correct, but I might not get it done until later tonight. If that happens, I'll leave a message requesting that you push it if the log message is okay (or just correct it and push that) Thanks for investigating !! :-)
On 10/24/24 12:36 PM, Daniel P. Berrangé wrote:
[...]
AFAIR, it isn't actually a bug with virtio-net usage as this last bit suggests. Rather it is a result of feature negotiation with QEMU on the host, whereby the guest & QEMU mutually agree to turn off checksums because they are redundant when the "link" is just local memory not a physical cable.
IOW, packets don't arrive in the guest with a bad checksum. They arrive in the guest with no checksum *as requested* by the guest.
The DHCP client decides this is a bad checksum, as it wasn't aware of the checksum offload usage.
[...]
....and in Fedora/RHEL context it was fixed 18 years ago, as we first hit this when working on Xen integration in 2006 :-)
A few quick tests proved that it was the same old "bad checksum" problem from 2010 come back to haunt us.
2006 :-)
Interesting - so my origin story is at least partially a false memory :-/ But if you had this problem with Xen in 2006, how was it fixed then (and why was it a surprised when it came up again in 2010 after vhost processing was turned on? Or maybe it wasn't a surprise, and I just thought so because I wasn't around in 2006 :-) (The commit adding --checksum-fill rules to libvirt was fd5b15ff1, from July 2010)
On 10/24/24 2:12 PM, Laine Stump wrote:
On 10/24/24 12:36 PM, Daniel P. Berrangé wrote:
[...]
AFAIR, it isn't actually a bug with virtio-net usage as this last bit suggests. Rather it is a result of feature negotiation with QEMU on the host, whereby the guest & QEMU mutually agree to turn off checksums because they are redundant when the "link" is just local memory not a physical cable.
IOW, packets don't arrive in the guest with a bad checksum. They arrive in the guest with no checksum *as requested* by the guest.
The DHCP client decides this is a bad checksum, as it wasn't aware of the checksum offload usage.
[...]
....and in Fedora/RHEL context it was fixed 18 years ago, as we first hit this when working on Xen integration in 2006 :-)
I think you would have to say "in Fedora/RHEL+Xen context it was fixed 18 years ago", since the specific test case that I recall working with was a RHEL5 guest that couldn't get an address from DHCP. So RHEL still had the problem, it just took switching to QEMU + virtio + vhost packet processing to make it visible :-)
A few quick tests proved that it was the same old "bad checksum" problem from 2010 come back to haunt us.
2006 :-)
Interesting - so my origin story is at least partially a false memory :-/
But if you had this problem with Xen in 2006, how was it fixed then (and why was it a surprised when it came up again in 2010 after vhost processing was turned on? Or maybe it wasn't a surprise, and I just thought so because I wasn't around in 2006 :-)
(The commit adding --checksum-fill rules to libvirt was fd5b15ff1, from July 2010)
participants (3)
-
Daniel P. Berrangé -
Laine Stump -
Laine Stump