[Qemu-devel] vmxnet3, vnet_hdr, and minimum length padding

[Qemu-devel] vmxnet3, vnet_hdr, and minimum length padding

[Brian Kress]

     When running ESXi under qemu there is an issue with the ESXi guest 
discarding packets that are too short.  The guest discards any packets 
under the normal minimum length for an ethernet packet (60).  This 
results in odd behaviour where other hosts or VMs on other hosts can 
communicate with the ESXi guest just fine (since there's a physical NIC 
somewhere doing padding), but VMs on the host and the host itself cannot 
because the ARP request packets are too small for the ESXi host to accept.
     Someone in the past thought this was worth fixing, and added code 
to the vmxnet3 qemu emulation such that if it is receiving packets 
smaller than 60 bytes to pad the packet out to 60. Unfortunately this 
code is wrong (or at least in the wrong place). It does so BEFORE before 
taking into account the vnet_hdr at the front of the packet added by the 
tap device.   As a result, it might add padding, but it never adds 
enough.  Specifically it adds 10 less (the length of the vnet_hdr) than 
it needs to.
     The following (hopefully "obviously correct") patch simply swaps 
the order of processing the vnet header and the padding.  With this 
patch an ESXi guest is able to communicate with the host or other local VMs.


--- a/qemu-2.3.0/hw/net/vmxnet3.c       2015-04-27 10:08:24.000000000 -0400
+++ b/qemu-2.3.0/hw/net/vmxnet3.c       2015-06-23 11:38:48.865728713 -0400
@@ -1879,6 +1879,12 @@
          return -1;
      }

+    if (s->peer_has_vhdr) {
+        vmxnet_rx_pkt_set_vhdr(s->rx_pkt, (struct virtio_net_hdr *)buf);
+        buf += sizeof(struct virtio_net_hdr);
+        size -= sizeof(struct virtio_net_hdr);
+    }
+
      /* Pad to minimum Ethernet frame length */
      if (size < sizeof(min_buf)) {
          memcpy(min_buf, buf, size);
@@ -1887,12 +1893,6 @@
          size = sizeof(min_buf);
      }

-    if (s->peer_has_vhdr) {
-        vmxnet_rx_pkt_set_vhdr(s->rx_pkt, (struct virtio_net_hdr *)buf);
-        buf += sizeof(struct virtio_net_hdr);
-        size -= sizeof(struct virtio_net_hdr);
-    }
-
      vmxnet_rx_pkt_set_packet_type(s->rx_pkt,
          get_eth_packet_type(PKT_GET_ETH_HDR(buf)));

Re: [Qemu-devel] vmxnet3, vnet_hdr, and minimum length padding

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2706 bytes --]

On Tue, Jun 23, 2015 at 11:49:25AM -0400, Brian Kress wrote:

Thanks for sending a patch!

I have CCed the vmxnet3 maintainer and Jason Wang, who looks at net
subsystem patches:

  $ scripts/get_maintainer.pl -f hw/net/vmxnet3.c
  Dmitry Fleytman <dmitry@daynix.com> (maintainer:Vmware)

>     When running ESXi under qemu there is an issue with the ESXi guest
> discarding packets that are too short.  The guest discards any packets under
> the normal minimum length for an ethernet packet (60).  This results in odd
> behaviour where other hosts or VMs on other hosts can communicate with the
> ESXi guest just fine (since there's a physical NIC somewhere doing padding),
> but VMs on the host and the host itself cannot because the ARP request
> packets are too small for the ESXi host to accept.
>     Someone in the past thought this was worth fixing, and added code to the
> vmxnet3 qemu emulation such that if it is receiving packets smaller than 60
> bytes to pad the packet out to 60. Unfortunately this code is wrong (or at
> least in the wrong place). It does so BEFORE before taking into account the
> vnet_hdr at the front of the packet added by the tap device.   As a result,
> it might add padding, but it never adds enough.  Specifically it adds 10
> less (the length of the vnet_hdr) than it needs to.
>     The following (hopefully "obviously correct") patch simply swaps the
> order of processing the vnet header and the padding.  With this patch an
> ESXi guest is able to communicate with the host or other local VMs.
> 
> 

Please add your Signed-off-by.  Details about Signed-off-by are on the
http://qemu-project.org/Contribute/SubmitAPatch page.

> --- a/qemu-2.3.0/hw/net/vmxnet3.c       2015-04-27 10:08:24.000000000 -0400
> +++ b/qemu-2.3.0/hw/net/vmxnet3.c       2015-06-23 11:38:48.865728713 -0400
> @@ -1879,6 +1879,12 @@
>          return -1;
>      }
> 
> +    if (s->peer_has_vhdr) {
> +        vmxnet_rx_pkt_set_vhdr(s->rx_pkt, (struct virtio_net_hdr *)buf);
> +        buf += sizeof(struct virtio_net_hdr);
> +        size -= sizeof(struct virtio_net_hdr);
> +    }
> +
>      /* Pad to minimum Ethernet frame length */
>      if (size < sizeof(min_buf)) {
>          memcpy(min_buf, buf, size);
> @@ -1887,12 +1893,6 @@
>          size = sizeof(min_buf);
>      }
> 
> -    if (s->peer_has_vhdr) {
> -        vmxnet_rx_pkt_set_vhdr(s->rx_pkt, (struct virtio_net_hdr *)buf);
> -        buf += sizeof(struct virtio_net_hdr);
> -        size -= sizeof(struct virtio_net_hdr);
> -    }
> -
>      vmxnet_rx_pkt_set_packet_type(s->rx_pkt,
>          get_eth_packet_type(PKT_GET_ETH_HDR(buf)));
> 
> 
> 
> 
> 

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] vmxnet3, vnet_hdr, and minimum length padding

[Brian Kress]

Resending by request with Signed-off-by:


     When running ESXi under qemu there is an issue with the ESXi guest 
discarding packets that are too short.  The guest discards any packets 
under the normal minimum length for an ethernet packet (60).  This 
results in odd behaviour where other hosts or VMs on other hosts can 
communicate with the ESXi guest just fine (since there's a physical NIC 
somewhere doing padding), but VMs on the host and the host itself cannot 
because the ARP request packets are too small for the ESXi host to accept.
     Someone in the past thought this was worth fixing, and added code 
to the vmxnet3 qemu emulation such that if it is receiving packets 
smaller than 60 bytes to pad the packet out to 60. Unfortunately this 
code is wrong (or at least in the wrong place). It does so BEFORE before 
taking into account the vnet_hdr at the front of the packet added by the 
tap device.   As a result, it might add padding, but it never adds 
enough.  Specifically it adds 10 less (the length of the vnet_hdr) than 
it needs to.
     The following (hopefully "obviously correct") patch simply swaps 
the order of processing the vnet header and the padding.  With this 
patch an ESXi guest is able to communicate with the host or other local 
VMs.


Signed-off-by: Brian Kress <kressb@moose.net>

--- a/qemu-2.3.0/hw/net/vmxnet3.c       2015-04-27 10:08:24.000000000 -0400
+++ b/qemu-2.3.0/hw/net/vmxnet3.c       2015-06-23 11:38:48.865728713 -0400
@@ -1879,6 +1879,12 @@
          return -1;
      }

+    if (s->peer_has_vhdr) {
+        vmxnet_rx_pkt_set_vhdr(s->rx_pkt, (struct virtio_net_hdr *)buf);
+        buf += sizeof(struct virtio_net_hdr);
+        size -= sizeof(struct virtio_net_hdr);
+    }
+
      /* Pad to minimum Ethernet frame length */
      if (size < sizeof(min_buf)) {
          memcpy(min_buf, buf, size);
@@ -1887,12 +1893,6 @@
          size = sizeof(min_buf);
      }

-    if (s->peer_has_vhdr) {
-        vmxnet_rx_pkt_set_vhdr(s->rx_pkt, (struct virtio_net_hdr *)buf);
-        buf += sizeof(struct virtio_net_hdr);
-        size -= sizeof(struct virtio_net_hdr);
-    }
-
      vmxnet_rx_pkt_set_packet_type(s->rx_pkt,
          get_eth_packet_type(PKT_GET_ETH_HDR(buf)));

Re: [Qemu-devel] vmxnet3, vnet_hdr, and minimum length padding

[Paolo Bonzini]

On 25/06/2015 15:34, Brian Kress wrote:
> Resending by request with Signed-off-by:
> 
> 
>     When running ESXi under qemu there is an issue with the ESXi guest
> discarding packets that are too short.  The guest discards any packets
> under the normal minimum length for an ethernet packet (60).  This
> results in odd behaviour where other hosts or VMs on other hosts can
> communicate with the ESXi guest just fine (since there's a physical NIC
> somewhere doing padding), but VMs on the host and the host itself cannot
> because the ARP request packets are too small for the ESXi host to accept.
>     Someone in the past thought this was worth fixing, and added code to
> the vmxnet3 qemu emulation such that if it is receiving packets smaller
> than 60 bytes to pad the packet out to 60. Unfortunately this code is
> wrong (or at least in the wrong place). It does so BEFORE before taking
> into account the vnet_hdr at the front of the packet added by the tap
> device.   As a result, it might add padding, but it never adds enough. 
> Specifically it adds 10 less (the length of the vnet_hdr) than it needs to.
>     The following (hopefully "obviously correct") patch simply swaps the
> order of processing the vnet header and the padding.  With this patch an
> ESXi guest is able to communicate with the host or other local VMs.

This is not the correct format for a patch:

- the subject should start with [PATCH]

- the subject should describe what the patch does

- the lines of the body of the commit message should be ~72 characters
long at most

- the patch should apply with "patch -p1" (your patch requires "-p2").

For more information, see http://wiki.qemu.org/Contribute/SubmitAPatch.

It's up to the maintainer whether to fix the above; the code however is
fine so:

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks,

Paolo


> 
> Signed-off-by: Brian Kress <kressb@moose.net>
> 
> --- a/qemu-2.3.0/hw/net/vmxnet3.c       2015-04-27 10:08:24.000000000 -0400
> +++ b/qemu-2.3.0/hw/net/vmxnet3.c       2015-06-23 11:38:48.865728713 -0400
> @@ -1879,6 +1879,12 @@
>          return -1;
>      }
> 
> +    if (s->peer_has_vhdr) {
> +        vmxnet_rx_pkt_set_vhdr(s->rx_pkt, (struct virtio_net_hdr *)buf);
> +        buf += sizeof(struct virtio_net_hdr);
> +        size -= sizeof(struct virtio_net_hdr);
> +    }
> +
>      /* Pad to minimum Ethernet frame length */
>      if (size < sizeof(min_buf)) {
>          memcpy(min_buf, buf, size);
> @@ -1887,12 +1893,6 @@
>          size = sizeof(min_buf);
>      }
> 
> -    if (s->peer_has_vhdr) {
> -        vmxnet_rx_pkt_set_vhdr(s->rx_pkt, (struct virtio_net_hdr *)buf);
> -        buf += sizeof(struct virtio_net_hdr);
> -        size -= sizeof(struct virtio_net_hdr);
> -    }
> -
>      vmxnet_rx_pkt_set_packet_type(s->rx_pkt,
>          get_eth_packet_type(PKT_GET_ETH_HDR(buf)));
> 
> 
> 

Re: [Qemu-devel] vmxnet3, vnet_hdr, and minimum length padding

[Dmitry Fleytman]

[-- Attachment #1: Type: text/plain, Size: 2484 bytes --]


> On Jun 23, 2015, at 18:49 PM, Brian Kress <kressb@moose.net> wrote:
> 
>    When running ESXi under qemu there is an issue with the ESXi guest discarding packets that are too short.  The guest discards any packets under the normal minimum length for an ethernet packet (60).  This results in odd behaviour where other hosts or VMs on other hosts can communicate with the ESXi guest just fine (since there's a physical NIC somewhere doing padding), but VMs on the host and the host itself cannot because the ARP request packets are too small for the ESXi host to accept.
>    Someone in the past thought this was worth fixing, and added code to the vmxnet3 qemu emulation such that if it is receiving packets smaller than 60 bytes to pad the packet out to 60. Unfortunately this code is wrong (or at least in the wrong place). It does so BEFORE before taking into account the vnet_hdr at the front of the packet added by the tap device.   As a result, it might add padding, but it never adds enough.  Specifically it adds 10 less (the length of the vnet_hdr) than it needs to.
>    The following (hopefully "obviously correct") patch simply swaps the order of processing the vnet header and the padding.  With this patch an ESXi guest is able to communicate with the host or other local VMs.
> 
> 
> --- a/qemu-2.3.0/hw/net/vmxnet3.c       2015-04-27 10:08:24.000000000 -0400
> +++ b/qemu-2.3.0/hw/net/vmxnet3.c       2015-06-23 11:38:48.865728713 -0400
> @@ -1879,6 +1879,12 @@
>         return -1;
>     }
> 
> +    if (s->peer_has_vhdr) {
> +        vmxnet_rx_pkt_set_vhdr(s->rx_pkt, (struct virtio_net_hdr *)buf);
> +        buf += sizeof(struct virtio_net_hdr);
> +        size -= sizeof(struct virtio_net_hdr);
> +    }
> +
>     /* Pad to minimum Ethernet frame length */
>     if (size < sizeof(min_buf)) {
>         memcpy(min_buf, buf, size);
> @@ -1887,12 +1893,6 @@
>         size = sizeof(min_buf);
>     }
> 
> -    if (s->peer_has_vhdr) {
> -        vmxnet_rx_pkt_set_vhdr(s->rx_pkt, (struct virtio_net_hdr *)buf);
> -        buf += sizeof(struct virtio_net_hdr);
> -        size -= sizeof(struct virtio_net_hdr);
> -    }
> -

Reviewed-by: Dmitry Fleytman <dmitry@daynix.com <mailto:dmitry@daynix.com>>
The code is fine, thanks!

Please fix the patch according to Paolo comments.

Regards,
Dmitry.

>     vmxnet_rx_pkt_set_packet_type(s->rx_pkt,
>         get_eth_packet_type(PKT_GET_ETH_HDR(buf)));
> 
> 
> 
> 
> 


[-- Attachment #2: Type: text/html, Size: 4225 bytes --]

Re: [Qemu-devel] vmxnet3, vnet_hdr, and minimum length padding

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2410 bytes --]

On Tue, Jun 23, 2015 at 11:49:25AM -0400, Brian Kress wrote:
>     When running ESXi under qemu there is an issue with the ESXi guest
> discarding packets that are too short.  The guest discards any packets under
> the normal minimum length for an ethernet packet (60).  This results in odd
> behaviour where other hosts or VMs on other hosts can communicate with the
> ESXi guest just fine (since there's a physical NIC somewhere doing padding),
> but VMs on the host and the host itself cannot because the ARP request
> packets are too small for the ESXi host to accept.
>     Someone in the past thought this was worth fixing, and added code to the
> vmxnet3 qemu emulation such that if it is receiving packets smaller than 60
> bytes to pad the packet out to 60. Unfortunately this code is wrong (or at
> least in the wrong place). It does so BEFORE before taking into account the
> vnet_hdr at the front of the packet added by the tap device.   As a result,
> it might add padding, but it never adds enough.  Specifically it adds 10
> less (the length of the vnet_hdr) than it needs to.
>     The following (hopefully "obviously correct") patch simply swaps the
> order of processing the vnet header and the padding.  With this patch an
> ESXi guest is able to communicate with the host or other local VMs.
> 
> 
> --- a/qemu-2.3.0/hw/net/vmxnet3.c       2015-04-27 10:08:24.000000000 -0400
> +++ b/qemu-2.3.0/hw/net/vmxnet3.c       2015-06-23 11:38:48.865728713 -0400
> @@ -1879,6 +1879,12 @@
>          return -1;
>      }
> 
> +    if (s->peer_has_vhdr) {
> +        vmxnet_rx_pkt_set_vhdr(s->rx_pkt, (struct virtio_net_hdr *)buf);
> +        buf += sizeof(struct virtio_net_hdr);
> +        size -= sizeof(struct virtio_net_hdr);
> +    }
> +
>      /* Pad to minimum Ethernet frame length */
>      if (size < sizeof(min_buf)) {
>          memcpy(min_buf, buf, size);
> @@ -1887,12 +1893,6 @@
>          size = sizeof(min_buf);
>      }
> 
> -    if (s->peer_has_vhdr) {
> -        vmxnet_rx_pkt_set_vhdr(s->rx_pkt, (struct virtio_net_hdr *)buf);
> -        buf += sizeof(struct virtio_net_hdr);
> -        size -= sizeof(struct virtio_net_hdr);
> -    }
> -
>      vmxnet_rx_pkt_set_packet_type(s->rx_pkt,
>          get_eth_packet_type(PKT_GET_ETH_HDR(buf)));

Thanks, applied to my net tree:
https://github.com/stefanha/qemu/commits/net

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]