[Qemu-devel] [PATCH] vmxnet3: Pad short frames to minimum size (60 bytes)

[Qemu-devel] [PATCH] vmxnet3: Pad short frames to minimum size (60 bytes)

[Ben Draper]

When running VMware ESXi under qemu-kvm the guest discards frames
that are too short. Short ARP Requests will be dropped, this prevents
guests on the same bridge as VMware ESXi from communicating. This patch
simply adds the padding on the network device itself.

Signed-off-by: Ben Draper <ben@xrsa.net>
---
 hw/net/vmxnet3.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 791321f..f246fa1 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -34,6 +34,7 @@
 
 #define PCI_DEVICE_ID_VMWARE_VMXNET3_REVISION 0x1
 #define VMXNET3_MSIX_BAR_SIZE 0x2000
+#define MIN_BUF_SIZE 60
 
 #define VMXNET3_BAR0_IDX      (0)
 #define VMXNET3_BAR1_IDX      (1)
@@ -1871,12 +1872,21 @@ vmxnet3_receive(NetClientState *nc, const uint8_t *buf, size_t size)
 {
     VMXNET3State *s = qemu_get_nic_opaque(nc);
     size_t bytes_indicated;
+    uint8_t min_buf[MIN_BUF_SIZE];
 
     if (!vmxnet3_can_receive(nc)) {
         VMW_PKPRN("Cannot receive now");
         return -1;
     }
 
+    /* Pad to minimum Ethernet frame length */
+    if (size < sizeof(min_buf)) {
+        memcpy(min_buf, buf, size);
+        memset(&min_buf[size], 0, sizeof(min_buf) - size);
+        buf = min_buf;
+        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);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH] vmxnet3: Pad short frames to minimum size (60 bytes)

[Ben Draper]

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

Interestingly this does not appear to affect Windows guests on the same
host/bridge as the ESXi guest, as
Windows pads the frames before sending them out. However it does affect
Linux guests on the same host/bridge
from communicating with the ESXi guest itself and the guests ESXi hosts.


--

*View My LinkedIn Profile*

http://uk.linkedin.com/in/bendraper00


On 20 August 2014 13:27, Ben Draper <ben@xrsa.net> wrote:

> When running VMware ESXi under qemu-kvm the guest discards frames
> that are too short. Short ARP Requests will be dropped, this prevents
> guests on the same bridge as VMware ESXi from communicating. This patch
> simply adds the padding on the network device itself.
>
> Signed-off-by: Ben Draper <ben@xrsa.net>
> ---
>  hw/net/vmxnet3.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 791321f..f246fa1 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -34,6 +34,7 @@
>
>  #define PCI_DEVICE_ID_VMWARE_VMXNET3_REVISION 0x1
>  #define VMXNET3_MSIX_BAR_SIZE 0x2000
> +#define MIN_BUF_SIZE 60
>
>  #define VMXNET3_BAR0_IDX      (0)
>  #define VMXNET3_BAR1_IDX      (1)
> @@ -1871,12 +1872,21 @@ vmxnet3_receive(NetClientState *nc, const uint8_t
> *buf, size_t size)
>  {
>      VMXNET3State *s = qemu_get_nic_opaque(nc);
>      size_t bytes_indicated;
> +    uint8_t min_buf[MIN_BUF_SIZE];
>
>      if (!vmxnet3_can_receive(nc)) {
>          VMW_PKPRN("Cannot receive now");
>          return -1;
>      }
>
> +    /* Pad to minimum Ethernet frame length */
> +    if (size < sizeof(min_buf)) {
> +        memcpy(min_buf, buf, size);
> +        memset(&min_buf[size], 0, sizeof(min_buf) - size);
> +        buf = min_buf;
> +        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);
> --
> 1.7.10.4
>
>

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

Re: [Qemu-devel] [PATCH] vmxnet3: Pad short frames to minimum size (60 bytes)

[Michael Tokarev]

20.08.2014 16:27, Ben Draper wrote:
> When running VMware ESXi under qemu-kvm the guest discards frames
> that are too short. Short ARP Requests will be dropped, this prevents
> guests on the same bridge as VMware ESXi from communicating. This patch
> simply adds the padding on the network device itself.

I'm not sure it is "trivial enough", so to say.  Do we have a maintainer
for vmxnet?  It's been written and updated several times by vmware (Daynix)
people, maybe they can comment on this somehow?  I mean, if we don't have
a maintainer for this device, it is okay to go to -trivial, but maybe it's
a good idea to try to reach the author(s) first?  (Adding Cc).

Especially since this change is only required in certain cases, not
generally.

Thanks,

/mjt

> Signed-off-by: Ben Draper <ben@xrsa.net>
> ---
>  hw/net/vmxnet3.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 791321f..f246fa1 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -34,6 +34,7 @@
>  
>  #define PCI_DEVICE_ID_VMWARE_VMXNET3_REVISION 0x1
>  #define VMXNET3_MSIX_BAR_SIZE 0x2000
> +#define MIN_BUF_SIZE 60
>  
>  #define VMXNET3_BAR0_IDX      (0)
>  #define VMXNET3_BAR1_IDX      (1)
> @@ -1871,12 +1872,21 @@ vmxnet3_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>  {
>      VMXNET3State *s = qemu_get_nic_opaque(nc);
>      size_t bytes_indicated;
> +    uint8_t min_buf[MIN_BUF_SIZE];
>  
>      if (!vmxnet3_can_receive(nc)) {
>          VMW_PKPRN("Cannot receive now");
>          return -1;
>      }
>  
> +    /* Pad to minimum Ethernet frame length */
> +    if (size < sizeof(min_buf)) {
> +        memcpy(min_buf, buf, size);
> +        memset(&min_buf[size], 0, sizeof(min_buf) - size);
> +        buf = min_buf;
> +        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);
> 

Re: [Qemu-devel] [PATCH] vmxnet3: Pad short frames to minimum size (60 bytes)

[Dmitry Fleytman]

On Aug 24, 2014, at 15:06 PM, Michael Tokarev <mjt@tls.msk.ru> wrote:

> 20.08.2014 16:27, Ben Draper wrote:
>> When running VMware ESXi under qemu-kvm the guest discards frames
>> that are too short. Short ARP Requests will be dropped, this prevents
>> guests on the same bridge as VMware ESXi from communicating. This patch
>> simply adds the padding on the network device itself.
> 
> I'm not sure it is "trivial enough", so to say.  Do we have a maintainer
> for vmxnet?  It's been written and updated several times by vmware (Daynix)
> people, maybe they can comment on this somehow?  I mean, if we don't have
> a maintainer for this device, it is okay to go to -trivial, but maybe it's
> a good idea to try to reach the author(s) first?  (Adding Cc).
> 
> Especially since this change is only required in certain cases, not
> generally.


Hi Michael,

I’m the maintainer of vmxnet3/pvscsi devices in QEMU. Thanks for CC’ing me.

I think this patch is correct and needed.

As we saw a few times already on different operating systems,
vmware drivers expect short packets to be padded as required
by corresponding RFC. Therefore this patch fixes a real bug.

Reviewed-by: Dmitry Fleytman <dmitry@daynix.com>

> 
> Thanks,
> 
> /mjt
> 
>> Signed-off-by: Ben Draper <ben@xrsa.net>
>> ---
>> hw/net/vmxnet3.c |   10 ++++++++++
>> 1 file changed, 10 insertions(+)
>> 
>> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
>> index 791321f..f246fa1 100644
>> --- a/hw/net/vmxnet3.c
>> +++ b/hw/net/vmxnet3.c
>> @@ -34,6 +34,7 @@
>> 
>> #define PCI_DEVICE_ID_VMWARE_VMXNET3_REVISION 0x1
>> #define VMXNET3_MSIX_BAR_SIZE 0x2000
>> +#define MIN_BUF_SIZE 60
>> 
>> #define VMXNET3_BAR0_IDX      (0)
>> #define VMXNET3_BAR1_IDX      (1)
>> @@ -1871,12 +1872,21 @@ vmxnet3_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>> {
>>     VMXNET3State *s = qemu_get_nic_opaque(nc);
>>     size_t bytes_indicated;
>> +    uint8_t min_buf[MIN_BUF_SIZE];
>> 
>>     if (!vmxnet3_can_receive(nc)) {
>>         VMW_PKPRN("Cannot receive now");
>>         return -1;
>>     }
>> 
>> +    /* Pad to minimum Ethernet frame length */
>> +    if (size < sizeof(min_buf)) {
>> +        memcpy(min_buf, buf, size);
>> +        memset(&min_buf[size], 0, sizeof(min_buf) - size);
>> +        buf = min_buf;
>> +        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);
>> 
> 

Re: [Qemu-devel] [PATCH] vmxnet3: Pad short frames to minimum size (60 bytes)

[Michael Tokarev]

24.08.2014 16:28, Dmitry Fleytman wrote:

> Hi Michael,
> 
> I’m the maintainer of vmxnet3/pvscsi devices in QEMU. Thanks for CC’ing me.

Maybe you can add yourself to MAINTAINERS file as well? :)
I dunno if that's actually needed, but at least this should
stop "strain" patches like this to be sent to -trivial alone... ;)

> I think this patch is correct and needed.
> 
> As we saw a few times already on different operating systems,
> vmware drivers expect short packets to be padded as required
> by corresponding RFC. Therefore this patch fixes a real bug.

Okay, since there's no entry for vmxnet in MAINTAINERS, and with
your blessing, and since this is a rather specific device which
is not in common use, I'll apply it to -trivial, for now, unless
you want to pick it up and send a pull request for it.

Given your description, I think it should be Cc: qemu-stable@.

> Reviewed-by: Dmitry Fleytman <dmitry@daynix.com>

Thanks,

/mjt

Re: [Qemu-devel] [PATCH] vmxnet3: Pad short frames to minimum size (60 bytes)

[Dmitry Fleytman]

On Aug 24, 2014, at 16:10 PM, Michael Tokarev <mjt@tls.msk.ru> wrote:

> 24.08.2014 16:28, Dmitry Fleytman wrote:
> 
>> Hi Michael,
>> 
>> I’m the maintainer of vmxnet3/pvscsi devices in QEMU. Thanks for CC’ing me.
> 
> Maybe you can add yourself to MAINTAINERS file as well? :)

Yes, this should be done.
How we do this? Should I send a patch for MAINTAINETRS?


> I dunno if that's actually needed, but at least this should
> stop "strain" patches like this to be sent to -trivial alone... ;)
> 
>> I think this patch is correct and needed.
>> 
>> As we saw a few times already on different operating systems,
>> vmware drivers expect short packets to be padded as required
>> by corresponding RFC. Therefore this patch fixes a real bug.
> 
> Okay, since there's no entry for vmxnet in MAINTAINERS, and with
> your blessing, and since this is a rather specific device which
> is not in common use, I'll apply it to -trivial, for now, unless
> you want to pick it up and send a pull request for it.


-trivial is good enough for this patch.

> 
> Given your description, I think it should be Cc: qemu-stable@.
> 
>> Reviewed-by: Dmitry Fleytman <dmitry@daynix.com>
> 
> Thanks,
> 
> /mjt

Re: [Qemu-devel] [PATCH] vmxnet3: Pad short frames to minimum size (60 bytes)

[Markus Armbruster]

Dmitry Fleytman <dmitry@daynix.com> writes:

> On Aug 24, 2014, at 16:10 PM, Michael Tokarev <mjt@tls.msk.ru> wrote:
>
>> 24.08.2014 16:28, Dmitry Fleytman wrote:
>> 
>>> Hi Michael,
>>> 
>>> I’m the maintainer of vmxnet3/pvscsi devices in QEMU. Thanks for CC’ing me.
>> 
>> Maybe you can add yourself to MAINTAINERS file as well? :)
>
> Yes, this should be done.
> How we do this? Should I send a patch for MAINTAINETRS?

Yes.