virtio-net: Copy all for dhclient workaround

Commit Message

Akihiko Odaki April 5, 2025, 8:04 a.m. UTC

The goal of commit 7987d2be5a8b ("virtio-net: Copy received header to
buffer") was to remove the need to patch the (const) input buffer with a
recomputed UDP checksum by copying headers to a RW region and inject the
checksum there. The patch computed the checksum only from the header
fields (missing the rest of the payload) producing an invalid one
and making guests fail to acquire a DHCP lease.

Fix the issue by copying the entire packet instead of only copying the
headers.

Fixes: 7987d2be5a8b ("virtio-net: Copy received header to buffer")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2727
Cc: [e-mail]
Signed-off-by: Akihiko Odaki <[e-mail]>
---
This patch aims to resolves the issue the following one also does:
https://lore.kernel.org/qemu-devel/[e-mail]

The difference from the mentioned patch is that this patch also
preserves that the original intent of regressing change, which is to
remove the need to patch the (const) input buffer with a recomputed UDP
checksum.

To Antoine Damhet:
I confirmed that DHCP is currently not working and this patch fixes the
issue, but I would appreciate if you also confirm the fix as I already
have done testing badly for the regressing patch.
---
 hw/net/virtio-net.c | 35 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 19 deletions(-)


---
base-commit: 825b96dbcee23d134b691fc75618b59c5f53da32
change-id: 20250405-mtu-834d4c62c93c

Best regards,

Patch

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index de87cfadffe1..a920358a89c5 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1687,6 +1687,11 @@  static void virtio_net_hdr_swap(VirtIODevice *vdev, struct virtio_net_hdr *hdr)
     virtio_tswap16s(vdev, &hdr->csum_offset);
 }
 
+typedef struct Header {
+    struct virtio_net_hdr_v1_hash virtio_net;
+    uint8_t payload[1500];
+} Header;
+
 /* 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
@@ -1701,7 +1706,7 @@  static void virtio_net_hdr_swap(VirtIODevice *vdev, struct virtio_net_hdr *hdr)
  * 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,
+static void work_around_broken_dhclient(struct Header *hdr,
                                         size_t *hdr_len, const uint8_t *buf,
                                         size_t buf_size, size_t *buf_offset)
 {
@@ -1711,20 +1716,20 @@  static void work_around_broken_dhclient(struct virtio_net_hdr *hdr,
     buf += *buf_offset;
     buf_size -= *buf_offset;
 
-    if ((hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) && /* missing csum */
-        (buf_size >= csum_size && buf_size < 1500) && /* normal sized MTU */
+    if ((hdr->virtio_net.hdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) && /* missing csum */
+        (buf_size >= csum_size && buf_size < sizeof(hdr->payload)) && /* 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 */
-        memcpy((uint8_t *)hdr + *hdr_len, buf, csum_size);
-        net_checksum_calculate((uint8_t *)hdr + *hdr_len, csum_size, CSUM_UDP);
-        hdr->flags &= ~VIRTIO_NET_HDR_F_NEEDS_CSUM;
-        *hdr_len += csum_size;
-        *buf_offset += csum_size;
+        memcpy((uint8_t *)hdr + *hdr_len, buf, buf_size);
+        net_checksum_calculate((uint8_t *)hdr + *hdr_len, buf_size, CSUM_UDP);
+        hdr->virtio_net.hdr.flags &= ~VIRTIO_NET_HDR_F_NEEDS_CSUM;
+        *hdr_len += buf_size;
+        *buf_offset += buf_size;
     }
 }
 
-static size_t receive_header(VirtIONet *n, struct virtio_net_hdr *hdr,
+static size_t receive_header(VirtIONet *n, Header *hdr,
                              const void *buf, size_t buf_size,
                              size_t *buf_offset)
 {
@@ -1736,7 +1741,7 @@  static size_t receive_header(VirtIONet *n, struct virtio_net_hdr *hdr,
     work_around_broken_dhclient(hdr, &hdr_len, buf, buf_size, buf_offset);
 
     if (n->needs_vnet_hdr_swap) {
-        virtio_net_hdr_swap(VIRTIO_DEVICE(n), hdr);
+        virtio_net_hdr_swap(VIRTIO_DEVICE(n), (struct virtio_net_hdr *)hdr);
     }
 
     return hdr_len;
@@ -1907,13 +1912,6 @@  static int virtio_net_process_rss(NetClientState *nc, const uint8_t *buf,
     return (index == new_index) ? -1 : new_index;
 }
 
-typedef struct Header {
-    struct virtio_net_hdr_v1_hash virtio_net;
-    struct eth_header eth;
-    struct ip_header ip;
-    struct udp_header udp;
-} Header;
-
 static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
                                       size_t size)
 {
@@ -2002,8 +2000,7 @@  static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
             }
 
             guest_offset = n->has_vnet_hdr ?
-                           receive_header(n, (struct virtio_net_hdr *)&hdr,
-                                          buf, size, &offset) :
+                           receive_header(n, &hdr, buf, size, &offset) :
                            n->guest_hdr_len;
 
             iov_from_buf(sg, elem->in_num, 0, &hdr, guest_offset);