Re: [Qemu-devel] [PATCH] eepro100: pad to ensure minimum packet size

[Stefan Weil <weil@mail.berlios.de>]

Am 11.02.2011 20:36, schrieb Bruce Rogers:
> Recent gpxe e100pro drivers will drop small packets because the emulated
> nic will report an error for small frames. In the qemu model we should
> instead have the e100pro pad out the received frames to be the minimum
> size and not report this case as an error.
>
> Signed-off-by: Bruce Rogers <brogers@novell.com>
> ---
> hw/eepro100.c | 23 +++++++++++++----------
> 1 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/hw/eepro100.c b/hw/eepro100.c
> index edf48f6..03b6934 100644
> --- a/hw/eepro100.c
> +++ b/hw/eepro100.c
> @@ -1645,6 +1645,8 @@ static int nic_can_receive(VLANClientState *nc)
> #endif
> }
>
> +#define MIN_BUF_SIZE 60 /* Min. octets in an ethernet frame sans FCS */
> +
> static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, 
> size_t size)
> {
> /* TODO:
> @@ -1653,6 +1655,7 @@ 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;
> + uint8_t min_buf[MIN_BUF_SIZE];
> static const uint8_t broadcast_macaddr[6] =
> { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
>
> @@ -1660,15 +1663,15 @@ static ssize_t nic_receive(VLANClientState 
> *nc, const uint8_t * buf, size_t size
> /* CSMA is disabled. */
> logout("%p received while CSMA is disabled\n", s);
> return -1;
> - } 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))) {
> + }
> + /* 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 ((size > MAX_ETH_FRAME_SIZE + 4) && !(s->configuration[18] & 
> BIT(3))) {
> /* Long frame and configuration byte 18/3 (long receive ok) not set:
> * Long frames are discarded. */
> logout("%p received long frame (%zu byte), ignored\n", s, size);
> @@ -1744,7 +1747,7 @@ 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 (size < 64) {
> + if (size < MIN_BUF_SIZE) {
> rfd_status |= 0x0080;
> }
> TRACE(OTHER, logout("command 0x%04x, link 0x%08x, addr 0x%08x, size %u\n",


Could you please give more details of the test scenario which fails?
I'd like to reproduce it here and find a better solution.

The configuration bit "discard short frame" exists in real hardware,
so removing the code which emulates this behavior is not the correct
solution - even if it helps in a special case.

No acknowledge for this patch from me.

Regards,
Stefan Weil