]> git.ipfire.org Git - thirdparty/libvirt.git/commitdiff
Revert "network: add rule to nftables backend that zeroes checksum of DHCP responses" v10.9.0-rc2
authorLaine Stump <laine@redhat.com>
Wed, 30 Oct 2024 03:21:27 +0000 (23:21 -0400)
committerJiri Denemark <jdenemar@redhat.com>
Wed, 30 Oct 2024 10:39:58 +0000 (11:39 +0100)
This reverts commit 42ab0148dd11727f7e3fd31dce4485469af290d5.

This patch was supposed to fix the checksum of dhcp response packets
by setting it to 0 (because having a non-0 but incorrect checksum was
causing the packets to be droppe on FreeBSD guests).

Early testing was positive, but after the patch was pushed upstream
and more people could test it, it turned out that while it fixed the
dhcp checksum problem for virtio-net interfaces on FreeBSD and
OpenBSD, it also *broke* dhcp checksums for the e1000 emulated NIC on
*all* guests (but not e1000e).

So we're reverting this fix and looking for something more universal
to be included in the next release.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
12 files changed:
src/network/network_nftables.c
tests/networkxml2firewalldata/base.nftables
tests/networkxml2firewalldata/forward-dev-linux.nftables
tests/networkxml2firewalldata/isolated-linux.nftables
tests/networkxml2firewalldata/nat-default-linux.nftables
tests/networkxml2firewalldata/nat-ipv6-linux.nftables
tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables
tests/networkxml2firewalldata/nat-many-ips-linux.nftables
tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables
tests/networkxml2firewalldata/nat-port-range-linux.nftables
tests/networkxml2firewalldata/nat-tftp-linux.nftables
tests/networkxml2firewalldata/route-default-linux.nftables

index 5523207269dcf02c033e87e5450620d2f0a0157e..f8b5ab665d3c43e00443d5dbc40fc944ea965c24 100644 (file)
@@ -51,7 +51,6 @@ 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
@@ -107,10 +106,6 @@ 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; }"},
-
 };
 
 
@@ -649,44 +644,6 @@ 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";
@@ -944,30 +901,6 @@ 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,
@@ -1019,8 +952,6 @@ nftablesAddFirewallRules(virNetworkDef *def, virFirewall **fwRemoval)
             return -1;
     }
 
-    nftablesAddChecksumFirewallRules(fw, def);
-
     if (virFirewallApply(fw) < 0)
         return -1;
 
index 6371fc12ddb6e0f11e6f55c6bdce987d007a162b..a064318739b4b98b39ed07d6c0bb801d74cd171f 100644 (file)
@@ -68,13 +68,6 @@ libvirt_network \
 guest_nat \
 '{ type nat hook postrouting priority 100; policy accept; }'
 nft \
-add \
-chain \
-ip \
-libvirt_network \
-postroute_mangle \
-'{ type filter hook postrouting priority 0; policy accept; }'
-nft \
 list \
 table \
 ip6 \
@@ -143,10 +136,3 @@ ip6 \
 libvirt_network \
 guest_nat \
 '{ type nat hook postrouting priority 100; policy accept; }'
-nft \
-add \
-chain \
-ip6 \
-libvirt_network \
-postroute_mangle \
-'{ type filter hook postrouting priority 0; policy accept; }'
index 9dea1a88a4f9a8be05feae305fd720accbae8f9e..8badb74bebec70293f4277f24638de506f7c5b0b 100644 (file)
@@ -156,19 +156,3 @@ 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
index 67ee0a2bf54e3520ba6f65f50c128383d3544bf6..d1b4dac178368d17e3283092fee3fc5d7d7dc20f 100644 (file)
@@ -62,19 +62,3 @@ oif \
 virbr0 \
 counter \
 accept
-nft \
--ae insert \
-rule \
-ip \
-libvirt_network \
-postroute_mangle \
-oif \
-virbr0 \
-udp \
-dport \
-68 \
-counter \
-udp \
-checksum \
-set \
-0
index 951a5a6d6067401763bb68614b88261d287d1bac..28508292f9efe44100077972853ba67fa0d7001e 100644 (file)
@@ -142,19 +142,3 @@ 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
index 617ed8b753a158e13d03c540312e933d5a43992b..d8a9ba706d8b92b1ef4b9c2e36aa8c8fbba88760 100644 (file)
@@ -200,19 +200,3 @@ oif \
 virbr0 \
 counter \
 accept
-nft \
--ae insert \
-rule \
-ip \
-libvirt_network \
-postroute_mangle \
-oif \
-virbr0 \
-udp \
-dport \
-68 \
-counter \
-udp \
-checksum \
-set \
-0
index a710d0e29699eefdead16c51532334cb98743393..a7f09cda59e0385acb48b2238809d3f8ffe3b38a 100644 (file)
@@ -272,19 +272,3 @@ daddr \
 ff02::/16 \
 counter \
 return
-nft \
--ae insert \
-rule \
-ip \
-libvirt_network \
-postroute_mangle \
-oif \
-virbr0 \
-udp \
-dport \
-68 \
-counter \
-udp \
-checksum \
-set \
-0
index 0be5fb7e65080519196d677f525460b8257ce5a2..b826fe6134d64dd0e01f19661787e73ad5e52893 100644 (file)
@@ -366,19 +366,3 @@ 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
index 7574356855672b4f6a3d9fcb0c483b991160a7bd..ceaed6fa409a212073a740316724d7637c5591d0 100644 (file)
@@ -384,19 +384,3 @@ daddr \
 ff02::/16 \
 counter \
 return
-nft \
--ae insert \
-rule \
-ip \
-libvirt_network \
-postroute_mangle \
-oif \
-virbr0 \
-udp \
-dport \
-68 \
-counter \
-udp \
-checksum \
-set \
-0
index 127536e4db7d6c20eb6292421e9372de1c8626de..1dc37a26ec872d4bc78d850aa1bd9d7f43974799 100644 (file)
@@ -312,19 +312,3 @@ oif \
 virbr0 \
 counter \
 accept
-nft \
--ae insert \
-rule \
-ip \
-libvirt_network \
-postroute_mangle \
-oif \
-virbr0 \
-udp \
-dport \
-68 \
-counter \
-udp \
-checksum \
-set \
-0
index 951a5a6d6067401763bb68614b88261d287d1bac..28508292f9efe44100077972853ba67fa0d7001e 100644 (file)
@@ -142,19 +142,3 @@ 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
index be9c4f5439b05b83391369712840c0ecc364297a..282c9542a54d404a18434362b3b3f027e07b12ff 100644 (file)
@@ -56,19 +56,3 @@ oif \
 virbr0 \
 counter \
 accept
-nft \
--ae insert \
-rule \
-ip \
-libvirt_network \
-postroute_mangle \
-oif \
-virbr0 \
-udp \
-dport \
-68 \
-counter \
-udp \
-checksum \
-set \
-0