qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH 1/3] net: Pad short frames to minimum size (60 bytes)


From: Mark Cave-Ayland
Subject: Re: [RFC PATCH 1/3] net: Pad short frames to minimum size (60 bytes)
Date: Fri, 26 Feb 2021 11:51:37 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0

On 26/02/2021 10:29, Philippe Mathieu-Daudé wrote:

On 2/26/21 11:03 AM, Bin Meng wrote:
From: Bin Meng <bin.meng@windriver.com>

The minimum Ethernet frame length is 60 bytes, and we should pad
frames whose length is smaller to the minimum size.

This commit fixes the issue as seen with various ethernet models,
that ARP requests get dropped, preventing the guest from becoming
visible on the network.

Is it also used in commit 18995b9808d ("Send a RARP packet after
migration.")?

The following 2 commits that attempted to workaround this issue
in e1000 and vmxenet3 before, should be reverted.

   commit 78aeb23eded2 ("e1000: Pad short frames to minimum size (60 bytes)")
   commit 40a87c6c9b11 ("vmxnet3: Pad short frames to minimum size (60 bytes)")

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

  net/net.c | 9 +++++++++
  1 file changed, 9 insertions(+)

diff --git a/net/net.c b/net/net.c
index b038370..34004da 100644
--- a/net/net.c
+++ b/net/net.c
@@ -638,6 +638,7 @@ static ssize_t 
qemu_send_packet_async_with_flags(NetClientState *sender,
                                                   NetPacketSent *sent_cb)
  {
      NetQueue *queue;
+    uint8_t min_buf[60];

Can you add a definition instead of a magic value?
Maybe ETH_FRAME_MIN_LEN in "net/eth.h"?

      int ret;
#ifdef DEBUG_NET
@@ -649,6 +650,14 @@ static ssize_t 
qemu_send_packet_async_with_flags(NetClientState *sender,
          return size;
      }
+ /* 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);
+    }
+
      /* Let filters handle the packet first */
      ret = filter_receive(sender, NET_FILTER_DIRECTION_TX,
                           sender, flags, buf, size, sent_cb);


Looks like this pattern is also used in a few other cards. From the SPARC/PPC 
world:

https://gitlab.com/qemu-project/qemu/-/blob/master/hw/net/pcnet.c#L1003
https://gitlab.com/qemu-project/qemu/-/blob/master/hw/net/sungem.c#L587
https://gitlab.com/qemu-project/qemu/-/blob/master/hw/net/sunhme.c#L778

I can easily review/test patches that remove the same code from the cards.


ATB,

Mark.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]