Add iptables rule to fixup DHCP response checksum.

This patch attempts to take advantage of a newly added netfilter
module to correct for a problem with some guest DHCP client
implementations when used in conjunction with a DHCP server run on the
host systems with packet checksum offloading enabled.

The problem is that, when the guest uses a RAW socket to read the DHCP
response packets, the checksum hasn't yet been fixed by the IP stack,
so it is incorrect.

The fix implemented here is to add a rule to the POSTROUTING chain of
the mangle table in iptables that fixes up the checksum for packets on
the virtual network's bridge that are destined for the bootpc port (ie
"dhcpc", ie port 68) port on the guest.

Only very new versions of iptables will have this support (it will be
in the next upstream release), so a failure to add this rule only
results in a warning message. The iptables patch is here:

  http://patchwork.ozlabs.org/patch/58525/

A corresponding kernel module patch is also required (the backend of
the iptables patch) and that will be in the next release of the
kernel.

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b5944a799bfea25c372a0464607b5e2b0041ba94..d5a7a735629f59ddbdbc74f63f26b09d67712b9c 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -332,6 +332,7 @@ iptablesAddForwardAllowRelatedIn;
 iptablesAddForwardMasquerade;
 iptablesAddForwardRejectIn;
 iptablesAddForwardRejectOut;
+iptablesAddOutputFixUdpChecksum;
 iptablesAddTcpInput;
 iptablesAddUdpInput;
 iptablesContextFree;
@@ -343,6 +344,7 @@ iptablesRemoveForwardAllowRelatedIn;
 iptablesRemoveForwardMasquerade;
 iptablesRemoveForwardRejectIn;
 iptablesRemoveForwardRejectOut;
+iptablesRemoveOutputFixUdpChecksum;
 iptablesRemoveTcpInput;
 iptablesRemoveUdpInput;
 

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 217cdcca2500faf44b986e5c6e56dce8fb289f5b..59e02b1f1b3a94ad7730df536350d40411755493 100644 (file)
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -837,6 +837,20 @@ networkAddIptablesRules(struct network_driver *driver,
              !networkAddRoutingIptablesRules(driver, network))
         goto err8;
 
+    /* If we are doing local DHCP service on this network, attempt to
+     * add a rule that will fixup the checksum of DHCP response
+     * packets back to the guests (but report failure without
+     * aborting, since not all iptables implementations support it).
+     */
+
+    if ((network->def->ipAddress || network->def->nranges) &&
+        (iptablesAddOutputFixUdpChecksum(driver->iptables,
+                                         network->def->bridge, 68) != 0)) {
+        VIR_WARN("Could not add rule to fixup DHCP response checksums "
+                 "on network '%s'.", network->def->name);
+        VIR_WARN0("May need to update iptables package & kernel to support CHECKSUM rule.");
+    }
+
     return 1;
 
  err8:
@@ -867,6 +881,10 @@ networkAddIptablesRules(struct network_driver *driver,
 static void
 networkRemoveIptablesRules(struct network_driver *driver,
                          virNetworkObjPtr network) {
+    if (network->def->ipAddress || network->def->nranges) {
+        iptablesRemoveOutputFixUdpChecksum(driver->iptables,
+                                           network->def->bridge, 68);
+    }
     if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE) {
         if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) {
             iptablesRemoveForwardMasquerade(driver->iptables,

diff --git a/src/util/iptables.c b/src/util/iptables.c
index f63e8c64d99efe2abc82bcd15c5ce89b4012b241..1b905bb58721075e65baeb33e6ba91a080b00cc4 100644 (file)
--- a/src/util/iptables.c
+++ b/src/util/iptables.c
@@ -60,6 +60,7 @@ struct _iptablesContext
     iptRules *input_filter;
     iptRules *forward_filter;
     iptRules *nat_postrouting;
+    iptRules *mangle_postrouting;
 };
 
 static void
@@ -188,6 +189,9 @@ iptablesContextNew(void)
     if (!(ctx->nat_postrouting = iptRulesNew("nat", "POSTROUTING")))
         goto error;
 
+    if (!(ctx->mangle_postrouting = iptRulesNew("mangle", "POSTROUTING")))
+        goto error;
+
     return ctx;
 
  error:
@@ -210,6 +214,8 @@ iptablesContextFree(iptablesContext *ctx)
         iptRulesFree(ctx->forward_filter);
     if (ctx->nat_postrouting)
         iptRulesFree(ctx->nat_postrouting);
+    if (ctx->mangle_postrouting)
+        iptRulesFree(ctx->mangle_postrouting);
     VIR_FREE(ctx);
 }
 
@@ -781,3 +787,68 @@ iptablesRemoveForwardMasquerade(iptablesContext *ctx,
 {
     return iptablesForwardMasquerade(ctx, network, physdev, protocol, REMOVE);
 }
+
+
+static int
+iptablesOutputFixUdpChecksum(iptablesContext *ctx,
+                             const char *iface,
+                             int port,
+                             int action)
+{
+    char portstr[32];
+
+    snprintf(portstr, sizeof(portstr), "%d", port);
+    portstr[sizeof(portstr) - 1] = '\0';
+
+    return iptablesAddRemoveRule(ctx->mangle_postrouting,
+                                 action,
+                                 "--out-interface", iface,
+                                 "--protocol", "udp",
+                                 "--destination-port", portstr,
+                                 "--jump", "CHECKSUM", "--checksum-fill",
+                                 NULL);
+}
+
+/**
+ * iptablesAddOutputFixUdpChecksum:
+ * @ctx: pointer to the IP table context
+ * @iface: the interface name
+ * @port: the UDP port to match
+ *
+ * Add an rule to the mangle table's POSTROUTING chain that fixes up the
+ * checksum of packets with the given destination @port.
+ * the given @iface interface for TCP packets.
+ *
+ * Returns 0 in case of success or an error code in case of error.
+ * (NB: if the system's iptables does not support checksum mangling,
+ * this will return an error, which should be ignored.)
+ */
+
+int
+iptablesAddOutputFixUdpChecksum(iptablesContext *ctx,
+                                const char *iface,
+                                int port)
+{
+    return iptablesOutputFixUdpChecksum(ctx, iface, port, ADD);
+}
+
+/**
+ * iptablesRemoveOutputFixUdpChecksum:
+ * @ctx: pointer to the IP table context
+ * @iface: the interface name
+ * @port: the UDP port of the rule to remove
+ *
+ * Removes the checksum fixup rule that was previous added with
+ * iptablesAddOutputFixUdpChecksum.
+ *
+ * Returns 0 in case of success or an error code in case of error
+ * (again, if iptables doesn't support checksum fixup, this will
+ * return an error, which should be ignored)
+ */
+int
+iptablesRemoveOutputFixUdpChecksum(iptablesContext *ctx,
+                                   const char *iface,
+                                   int port)
+{
+    return iptablesOutputFixUdpChecksum(ctx, iface, port, REMOVE);
+}

diff --git a/src/util/iptables.h b/src/util/iptables.h
index b47d8543d96ca90d4a18fe50b6eb407a3c4e24ee..21ba667074c67d5bf4a17f344f13c546f64cd813 100644 (file)
--- a/src/util/iptables.h
+++ b/src/util/iptables.h
@@ -91,5 +91,11 @@ int              iptablesRemoveForwardMasquerade (iptablesContext *ctx,
                                                   const char *network,
                                                   const char *physdev,
                                                   const char *protocol);
+int              iptablesAddOutputFixUdpChecksum (iptablesContext *ctx,
+                                                  const char *iface,
+                                                  int port);
+int              iptablesRemoveOutputFixUdpChecksum (iptablesContext *ctx,
+                                                     const char *iface,
+                                                     int port);
 
 #endif /* __QEMUD_IPTABLES_H__ */