[PATCH] Work around dhclient brokenness (v2)

[PATCH] Work around dhclient brokenness (v2)

[Anthony Liguori]

With the latest GSO/csum offload patches, any guest using an unpatched version
of dhclient (any Ubuntu guest, for instance), will no longer be able to get
a DHCP address.

dhclient is actually at fault here.  It uses AF_PACKET to receive DHCP responses
but does not check auxdata to see if the packet has a valid csum.  This causes
it to throw out the DHCP responses it gets from the virtio interface as there
is not a valid checksum.

Fedora has carried a patch to fix their dhclient (it's needed for Xen too) but
this patch has not made it into a release of dhclient.  AFAIK, the patch is in
the dhclient CVS but I cannot confirm since their CVS is not public.

This patch, suggested by Rusty, looks for UDP packets (of a normal MTU) and
explicitly adds a checksum to them if they are missing one.

This allows unpatched dhclients to continue to work without needing to update
the guest kernels.

Since v1, we refined the search criteria to only consider packets originating
from a DHCP server.  I also added a comment to note that we should disable this
routine when we introduce zero copy.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c
index 61215b1..409960f 100644
--- a/qemu/hw/virtio-net.c
+++ b/qemu/hw/virtio-net.c
@@ -154,6 +154,34 @@ static int virtio_net_can_receive(void *opaque)
     return 1;
 }
 
+/* dhclient uses AF_PACKET but doesn't pass auxdata to the kernel so
+ * it never finds out that the packets don't have valid checksums.  This
+ * causes dhclient to get upset.  Fedora's carried a patch for ages to
+ * fix this with Xen but it hasn't appeared in an upstream release of
+ * dhclient yet.
+ *
+ * To avoid breaking existing guests, we catch udp packets and add
+ * checksums.  This is terrible but it's better than hacking the guest
+ * kernels.
+ *
+ * N.B. if we introduce a zero-copy API, this operation is no longer free so
+ * we should provide a mechanism to disable it to avoid polluting the host
+ * cache.
+ */
+static void work_around_broken_dhclient(struct virtio_net_hdr *hdr,
+                                        const uint8_t *buf, size_t size)
+{
+    if ((hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) && /* missing csum */
+        (size > 27 && size < 1500) && /* normal sized MTU */
+        (buf[12] == 0x08 && buf[13] == 0x00) && /* ethertype == IPv4 */
+        (buf[23] == 17) && /* ip.protocol == UDP */
+        (buf[34] == 0 && buf[35] == 67)) { /* udp.srcport == bootps */
+        /* FIXME this cast is evil */
+        net_checksum_calculate((uint8_t *)buf, size);
+        hdr->flags &= ~VIRTIO_NET_HDR_F_NEEDS_CSUM;
+    }
+}
+
 static void virtio_net_receive(void *opaque, const uint8_t *buf, int size)
 {
     VirtIONet *n = opaque;
@@ -180,6 +208,7 @@ static void virtio_net_receive(void *opaque, const uint8_t *buf, int size)
     if (tap_has_vnet_hdr(n->vc->vlan->first_client)) {
 	memcpy(hdr, buf, sizeof(*hdr));
 	offset += total;
+        work_around_broken_dhclient(hdr, buf + offset, size - offset);
     }
 
     /* copy in packet.  ugh */

Re: [PATCH] Work around dhclient brokenness (v2)

[Avi Kivity]

Anthony Liguori wrote:
> With the latest GSO/csum offload patches, any guest using an unpatched version
> of dhclient (any Ubuntu guest, for instance), will no longer be able to get
> a DHCP address.
>
> dhclient is actually at fault here.  It uses AF_PACKET to receive DHCP responses
> but does not check auxdata to see if the packet has a valid csum.  This causes
> it to throw out the DHCP responses it gets from the virtio interface as there
> is not a valid checksum.
>
> Fedora has carried a patch to fix their dhclient (it's needed for Xen too) but
> this patch has not made it into a release of dhclient.  AFAIK, the patch is in
> the dhclient CVS but I cannot confirm since their CVS is not public.
>
> This patch, suggested by Rusty, looks for UDP packets (of a normal MTU) and
> explicitly adds a checksum to them if they are missing one.
>
> This allows unpatched dhclients to continue to work without needing to update
> the guest kernels.
>
> Since v1, we refined the search criteria to only consider packets originating
> from a DHCP server.  I also added a comment to note that we should disable this
> routine when we introduce zero copy.
>   

Applied, thanks.

-- 
error compiling committee.c: too many arguments to function