diff mbox

[v2,04/10] eepro100: Pad received short frames

Message ID 1304196013-14432-5-git-send-email-weil@mail.berlios.de
State Accepted
Headers show

Commit Message

Stefan Weil April 30, 2011, 8:40 p.m. UTC
QEMU sends frames smaller than 60 bytes to ethernet nics.
This should be fixed in the networking code because normally
such frames are rejected by real NICs and their emulations.
To avoid this behaviour, other NIC emulations pad received
frames. This patch enables this workaround for eepro100, too.

All related code is marked with CONFIG_PAD_RECEIVED_FRAMES,
so emulation of the correct handling for short frames can
be restored as soon as QEMU's networking code is fixed.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 hw/eepro100.c |   25 ++++++++++++++++++++++++-
 1 files changed, 24 insertions(+), 1 deletions(-)

Comments

Michael S. Tsirkin May 5, 2011, 1 p.m. UTC | #1
On Sat, Apr 30, 2011 at 10:40:07PM +0200, Stefan Weil wrote:
> QEMU sends frames smaller than 60 bytes to ethernet nics.
> This should be fixed in the networking code because normally
> such frames are rejected by real NICs and their emulations.
> To avoid this behaviour, other NIC emulations pad received
> frames. This patch enables this workaround for eepro100, too.
> 
> All related code is marked with CONFIG_PAD_RECEIVED_FRAMES,
> so emulation of the correct handling for short frames can
> be restored as soon as QEMU's networking code is fixed.
> 
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>

Applied, I tweaked the comment a bit as we don't intend
to change it in qemu. But go ahead and keep the ifdef around
if you like.

> ---
>  hw/eepro100.c |   25 ++++++++++++++++++++++++-
>  1 files changed, 24 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/eepro100.c b/hw/eepro100.c
> index 82c6369..2104bc0 100644
> --- a/hw/eepro100.c
> +++ b/hw/eepro100.c
> @@ -48,6 +48,14 @@
>  #include "eeprom93xx.h"
>  #include "sysemu.h"
>  
> +/* QEMU sends frames smaller than 60 bytes to ethernet nics.
> + * This should be fixed in the networking code because normally
> + * such frames are rejected by real nics and their emulations.
> + * To avoid this behaviour, other nic emulations pad received
> + * frames. The following definition enables this workaround for
> + * eepro100, too. */
> +#define CONFIG_PAD_RECEIVED_FRAMES
> +
>  #define KiB 1024
>  
>  /* Debug EEPRO100 card. */
> @@ -1656,19 +1664,32 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
>       */
>      EEPRO100State *s = DO_UPCAST(NICState, nc, nc)->opaque;
>      uint16_t rfd_status = 0xa000;
> +#if defined(CONFIG_PAD_RECEIVED_FRAMES)
> +    uint8_t min_buf[60];
> +#endif
>      static const uint8_t broadcast_macaddr[6] =
>          { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
>  
> +#if defined(CONFIG_PAD_RECEIVED_FRAMES)
> +    /* 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);
> +    }
> +#endif
> +
>      if (s->configuration[8] & 0x80) {
>          /* CSMA is disabled. */
>          logout("%p received while CSMA is disabled\n", s);
>          return -1;
> +#if !defined(CONFIG_PAD_RECEIVED_FRAMES)
>      } else if (size < 64 && (s->configuration[7] & BIT(0))) {
>          /* Short frame and configuration byte 7/0 (discard short receive) set:
>           * Short frame is discarded */
>          logout("%p received short frame (%zu byte)\n", s, size);
>          s->statistics.rx_short_frame_errors++;
> -#if 0
>          return -1;
>  #endif
>      } else if ((size > MAX_ETH_FRAME_SIZE + 4) && !(s->configuration[18] & BIT(3))) {
> @@ -1747,9 +1768,11 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
>              "(%zu bytes); data truncated\n", rfd_size, size);
>          size = rfd_size;
>      }
> +#if !defined(CONFIG_PAD_RECEIVED_FRAMES)
>      if (size < 64) {
>          rfd_status |= 0x0080;
>      }
> +#endif
>      TRACE(OTHER, logout("command 0x%04x, link 0x%08x, addr 0x%08x, size %u\n",
>            rfd_command, rx.link, rx.rx_buf_addr, rfd_size));
>      stw_phys(s->ru_base + s->ru_offset + offsetof(eepro100_rx_t, status),
> -- 
> 1.7.2.5
Stefan Weil May 5, 2011, 4:19 p.m. UTC | #2
Am 05.05.2011 15:00, schrieb Michael S. Tsirkin:
> On Sat, Apr 30, 2011 at 10:40:07PM +0200, Stefan Weil wrote:
>> QEMU sends frames smaller than 60 bytes to ethernet nics.
>> This should be fixed in the networking code because normally
>> such frames are rejected by real NICs and their emulations.
>> To avoid this behaviour, other NIC emulations pad received
>> frames. This patch enables this workaround for eepro100, too.
>>
>> All related code is marked with CONFIG_PAD_RECEIVED_FRAMES,
>> so emulation of the correct handling for short frames can
>> be restored as soon as QEMU's networking code is fixed.
>>
>> Signed-off-by: Stefan Weil<weil@mail.berlios.de>
> Applied, I tweaked the comment a bit as we don't intend
> to change it in qemu. But go ahead and keep the ifdef around
> if you like.

The new comment is ok, thanks.

Cheers,
Stefan W.
diff mbox

Patch

diff --git a/hw/eepro100.c b/hw/eepro100.c
index 82c6369..2104bc0 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -48,6 +48,14 @@ 
 #include "eeprom93xx.h"
 #include "sysemu.h"
 
+/* QEMU sends frames smaller than 60 bytes to ethernet nics.
+ * This should be fixed in the networking code because normally
+ * such frames are rejected by real nics and their emulations.
+ * To avoid this behaviour, other nic emulations pad received
+ * frames. The following definition enables this workaround for
+ * eepro100, too. */
+#define CONFIG_PAD_RECEIVED_FRAMES
+
 #define KiB 1024
 
 /* Debug EEPRO100 card. */
@@ -1656,19 +1664,32 @@  static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
      */
     EEPRO100State *s = DO_UPCAST(NICState, nc, nc)->opaque;
     uint16_t rfd_status = 0xa000;
+#if defined(CONFIG_PAD_RECEIVED_FRAMES)
+    uint8_t min_buf[60];
+#endif
     static const uint8_t broadcast_macaddr[6] =
         { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 
+#if defined(CONFIG_PAD_RECEIVED_FRAMES)
+    /* 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);
+    }
+#endif
+
     if (s->configuration[8] & 0x80) {
         /* CSMA is disabled. */
         logout("%p received while CSMA is disabled\n", s);
         return -1;
+#if !defined(CONFIG_PAD_RECEIVED_FRAMES)
     } else if (size < 64 && (s->configuration[7] & BIT(0))) {
         /* Short frame and configuration byte 7/0 (discard short receive) set:
          * Short frame is discarded */
         logout("%p received short frame (%zu byte)\n", s, size);
         s->statistics.rx_short_frame_errors++;
-#if 0
         return -1;
 #endif
     } else if ((size > MAX_ETH_FRAME_SIZE + 4) && !(s->configuration[18] & BIT(3))) {
@@ -1747,9 +1768,11 @@  static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
             "(%zu bytes); data truncated\n", rfd_size, size);
         size = rfd_size;
     }
+#if !defined(CONFIG_PAD_RECEIVED_FRAMES)
     if (size < 64) {
         rfd_status |= 0x0080;
     }
+#endif
     TRACE(OTHER, logout("command 0x%04x, link 0x%08x, addr 0x%08x, size %u\n",
           rfd_command, rx.link, rx.rx_buf_addr, rfd_size));
     stw_phys(s->ru_base + s->ru_offset + offsetof(eepro100_rx_t, status),