All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/8] net: Pad short frames for network backends
@ 2023-03-01  9:01 bmeng.cn
  2023-03-01  9:01 ` [PATCH v6 1/8] hw/net: e1000: Remove the logic of padding short frames in the receive path bmeng.cn
                   ` (8 more replies)
  0 siblings, 9 replies; 11+ messages in thread
From: bmeng.cn @ 2023-03-01  9:01 UTC (permalink / raw)
  To: Jason Wang, qemu-devel
  Cc: Bin Meng, Dmitry Fleytman, Helge Deller, Richard Henderson

From: Bin Meng <bmeng@tinylab.org>

The minimum Ethernet frame length is 60 bytes. For short frames with
smaller length like ARP packets (only 42 bytes), on a real world NIC
it can choose either padding its length to the minimum required 60
bytes, or sending it out directly to the wire. Such behavior can be
hardcoded or controled by a register bit. Similarly on the receive
path, NICs can choose either dropping such short frames directly or
handing them over to software to handle.

On the other hand, for the network backends like SLiRP/TAP, they
don't expose a way to control the short frame behavior. As of today
they just send/receive data from/to the other end connected to them,
which means any sized packet is acceptable. So they can send and
receive short frames without any problem. It is observed that ARP
packets sent from SLiRP/TAP are 42 bytes, and SLiRP/TAP just send
these ARP packets to the other end which might be a NIC model that
does not allow short frames to pass through.

To provide better compatibility, for packets sent from QEMU network
backends like SLiRP/TAP, we change to pad short frames before sending
it out to the other end, if the other end does not forbid it via the
nc->do_not_pad flag. This ensures a backend as an Ethernet sender
does not violate the spec. But with this change, the behavior of
dropping short frames from SLiRP/TAP interfaces in the NIC model
cannot be emulated because it always receives a packet that is spec
complaint. The capability of sending short frames from NIC models is
still supported and short frames can still pass through SLiRP/TAP.

This series should be able to fix the issue as reported with some
NIC models before, that ARP requests get dropped, preventing the
guest from becoming visible on the network. It was workarounded in
these NIC models on the receive path, that when a short frame is
received, it is padded up to 60 bytes.

Note this is a resend of the v5 [1]. Only the first 4 patches were
applied in QEMU 6.0, and the reset was said to be queued for 6.1
but for some reason they never landed in QEMU mainline.

[1] https://lore.kernel.org/qemu-devel/859cd26a-feb2-ed62-98d5-764841a468cf@redhat.com/

Bin Meng (8):
  hw/net: e1000: Remove the logic of padding short frames in the receive
    path
  hw/net: vmxnet3: Remove the logic of padding short frames in the
    receive path
  hw/net: i82596: Remove the logic of padding short frames in the
    receive path
  hw/net: ne2000: Remove the logic of padding short frames in the
    receive path
  hw/net: pcnet: Remove the logic of padding short frames in the receive
    path
  hw/net: rtl8139: Remove the logic of padding short frames in the
    receive path
  hw/net: sungem: Remove the logic of padding short frames in the
    receive path
  hw/net: sunhme: Remove the logic of padding short frames in the
    receive path

 hw/net/e1000.c   | 11 +----------
 hw/net/i82596.c  | 18 ------------------
 hw/net/ne2000.c  | 12 ------------
 hw/net/pcnet.c   |  9 ---------
 hw/net/rtl8139.c | 12 ------------
 hw/net/sungem.c  | 14 --------------
 hw/net/sunhme.c  | 11 -----------
 hw/net/vmxnet3.c | 10 ----------
 8 files changed, 1 insertion(+), 96 deletions(-)

-- 
2.34.1



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v6 1/8] hw/net: e1000: Remove the logic of padding short frames in the receive path
  2023-03-01  9:01 [PATCH v6 0/8] net: Pad short frames for network backends bmeng.cn
@ 2023-03-01  9:01 ` bmeng.cn
  2023-03-01  9:01 ` [PATCH v6 2/8] hw/net: vmxnet3: " bmeng.cn
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: bmeng.cn @ 2023-03-01  9:01 UTC (permalink / raw)
  To: Jason Wang, qemu-devel; +Cc: Bin Meng, Bin Meng

From: Bin Meng <bmeng.cn@gmail.com>

Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.

This actually reverts commit 78aeb23eded2d0b765bf9145c71f80025b568acd.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---

 hw/net/e1000.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 7efb8a4c52..d9637cbc6d 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -908,7 +908,6 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
     uint16_t vlan_special = 0;
     uint8_t vlan_status = 0;
     uint8_t min_buf[MIN_BUF_SIZE];
-    struct iovec min_iov;
     uint8_t *filter_buf = iov->iov_base;
     size_t size = iov_size(iov, iovcnt);
     size_t iov_ofs = 0;
@@ -924,15 +923,7 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
         return 0;
     }
 
-    /* Pad to minimum Ethernet frame length */
-    if (size < sizeof(min_buf)) {
-        iov_to_buf(iov, iovcnt, 0, min_buf, size);
-        memset(&min_buf[size], 0, sizeof(min_buf) - size);
-        min_iov.iov_base = filter_buf = min_buf;
-        min_iov.iov_len = size = sizeof(min_buf);
-        iovcnt = 1;
-        iov = &min_iov;
-    } else if (iov->iov_len < MAXIMUM_ETHERNET_HDR_LEN) {
+    if (iov->iov_len < MAXIMUM_ETHERNET_HDR_LEN) {
         /* This is very unlikely, but may happen. */
         iov_to_buf(iov, iovcnt, 0, min_buf, MAXIMUM_ETHERNET_HDR_LEN);
         filter_buf = min_buf;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v6 2/8] hw/net: vmxnet3: Remove the logic of padding short frames in the receive path
  2023-03-01  9:01 [PATCH v6 0/8] net: Pad short frames for network backends bmeng.cn
  2023-03-01  9:01 ` [PATCH v6 1/8] hw/net: e1000: Remove the logic of padding short frames in the receive path bmeng.cn
@ 2023-03-01  9:01 ` bmeng.cn
  2023-03-01  9:01 ` [PATCH v6 3/8] hw/net: i82596: " bmeng.cn
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: bmeng.cn @ 2023-03-01  9:01 UTC (permalink / raw)
  To: Jason Wang, qemu-devel; +Cc: Bin Meng, Bin Meng, Dmitry Fleytman

From: Bin Meng <bmeng.cn@gmail.com>

Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.

This actually reverts commit 40a87c6c9b11ef9c14e0301f76abf0eb2582f08e.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---

 hw/net/vmxnet3.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 56559cda24..ea01b1f75f 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -40,7 +40,6 @@
 
 #define PCI_DEVICE_ID_VMWARE_VMXNET3_REVISION 0x1
 #define VMXNET3_MSIX_BAR_SIZE 0x2000
-#define MIN_BUF_SIZE 60
 
 /* Compatibility flags for migration */
 #define VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS_BIT 0
@@ -1979,7 +1978,6 @@ 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");
@@ -1992,14 +1990,6 @@ vmxnet3_receive(NetClientState *nc, const uint8_t *buf, size_t size)
         size -= sizeof(struct virtio_net_hdr);
     }
 
-    /* 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);
-    }
-
     net_rx_pkt_set_packet_type(s->rx_pkt,
         get_eth_packet_type(PKT_GET_ETH_HDR(buf)));
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v6 3/8] hw/net: i82596: Remove the logic of padding short frames in the receive path
  2023-03-01  9:01 [PATCH v6 0/8] net: Pad short frames for network backends bmeng.cn
  2023-03-01  9:01 ` [PATCH v6 1/8] hw/net: e1000: Remove the logic of padding short frames in the receive path bmeng.cn
  2023-03-01  9:01 ` [PATCH v6 2/8] hw/net: vmxnet3: " bmeng.cn
@ 2023-03-01  9:01 ` bmeng.cn
  2023-03-01  9:01 ` [PATCH v6 4/8] hw/net: ne2000: " bmeng.cn
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: bmeng.cn @ 2023-03-01  9:01 UTC (permalink / raw)
  To: Jason Wang, qemu-devel
  Cc: Bin Meng, Bin Meng, Helge Deller, Richard Henderson

From: Bin Meng <bmeng.cn@gmail.com>

Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---

 hw/net/i82596.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/hw/net/i82596.c b/hw/net/i82596.c
index ec21e2699a..ab26f8bea1 100644
--- a/hw/net/i82596.c
+++ b/hw/net/i82596.c
@@ -72,10 +72,6 @@ enum commands {
 #define I596_EOF        0x8000
 #define SIZE_MASK       0x3fff
 
-#define ETHER_TYPE_LEN 2
-#define VLAN_TCI_LEN 2
-#define VLAN_HLEN (ETHER_TYPE_LEN + VLAN_TCI_LEN)
-
 /* various flags in the chip config registers */
 #define I596_PREFETCH   (s->config[0] & 0x80)
 #define I596_PROMISC    (s->config[8] & 0x01)
@@ -488,8 +484,6 @@ bool i82596_can_receive(NetClientState *nc)
     return true;
 }
 
-#define MIN_BUF_SIZE 60
-
 ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz)
 {
     I82596State *s = qemu_get_nic_opaque(nc);
@@ -500,7 +494,6 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz)
     size_t bufsz = sz; /* length of data in buf */
     uint32_t crc;
     uint8_t *crc_ptr;
-    uint8_t buf1[MIN_BUF_SIZE + VLAN_HLEN];
     static const uint8_t broadcast_macaddr[6] = {
                 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 
@@ -583,17 +576,6 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz)
         }
     }
 
-    /* if too small buffer, then expand it */
-    if (len < MIN_BUF_SIZE + VLAN_HLEN) {
-        memcpy(buf1, buf, len);
-        memset(buf1 + len, 0, MIN_BUF_SIZE + VLAN_HLEN - len);
-        buf = buf1;
-        if (len < MIN_BUF_SIZE) {
-            len = MIN_BUF_SIZE;
-        }
-        bufsz = len;
-    }
-
     /* Calculate the ethernet checksum (4 bytes) */
     len += 4;
     crc = cpu_to_be32(crc32(~0, buf, sz));
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v6 4/8] hw/net: ne2000: Remove the logic of padding short frames in the receive path
  2023-03-01  9:01 [PATCH v6 0/8] net: Pad short frames for network backends bmeng.cn
                   ` (2 preceding siblings ...)
  2023-03-01  9:01 ` [PATCH v6 3/8] hw/net: i82596: " bmeng.cn
@ 2023-03-01  9:01 ` bmeng.cn
  2023-03-01  9:02 ` [PATCH v6 5/8] hw/net: pcnet: " bmeng.cn
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: bmeng.cn @ 2023-03-01  9:01 UTC (permalink / raw)
  To: Jason Wang, qemu-devel; +Cc: Bin Meng, Bin Meng

From: Bin Meng <bmeng.cn@gmail.com>

Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---

 hw/net/ne2000.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c
index 3f31d04efb..d79c884d50 100644
--- a/hw/net/ne2000.c
+++ b/hw/net/ne2000.c
@@ -167,15 +167,12 @@ static int ne2000_buffer_full(NE2000State *s)
     return 0;
 }
 
-#define MIN_BUF_SIZE 60
-
 ssize_t ne2000_receive(NetClientState *nc, const uint8_t *buf, size_t size_)
 {
     NE2000State *s = qemu_get_nic_opaque(nc);
     size_t size = size_;
     uint8_t *p;
     unsigned int total_len, next, avail, len, index, mcast_idx;
-    uint8_t buf1[60];
     static const uint8_t broadcast_macaddr[6] =
         { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 
@@ -213,15 +210,6 @@ ssize_t ne2000_receive(NetClientState *nc, const uint8_t *buf, size_t size_)
         }
     }
 
-
-    /* if too small buffer, then expand it */
-    if (size < MIN_BUF_SIZE) {
-        memcpy(buf1, buf, size);
-        memset(buf1 + size, 0, MIN_BUF_SIZE - size);
-        buf = buf1;
-        size = MIN_BUF_SIZE;
-    }
-
     index = s->curpag << 8;
     if (index >= NE2000_PMEM_END) {
         index = s->start;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v6 5/8] hw/net: pcnet: Remove the logic of padding short frames in the receive path
  2023-03-01  9:01 [PATCH v6 0/8] net: Pad short frames for network backends bmeng.cn
                   ` (3 preceding siblings ...)
  2023-03-01  9:01 ` [PATCH v6 4/8] hw/net: ne2000: " bmeng.cn
@ 2023-03-01  9:02 ` bmeng.cn
  2023-03-01  9:02 ` [PATCH v6 6/8] hw/net: rtl8139: " bmeng.cn
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: bmeng.cn @ 2023-03-01  9:02 UTC (permalink / raw)
  To: Jason Wang, qemu-devel; +Cc: Bin Meng, Bin Meng

From: Bin Meng <bmeng.cn@gmail.com>

Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---

 hw/net/pcnet.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index d456094575..02828ae716 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -987,7 +987,6 @@ ssize_t pcnet_receive(NetClientState *nc, const uint8_t *buf, size_t size_)
 {
     PCNetState *s = qemu_get_nic_opaque(nc);
     int is_padr = 0, is_bcast = 0, is_ladr = 0;
-    uint8_t buf1[60];
     int remaining;
     int crc_err = 0;
     size_t size = size_;
@@ -1000,14 +999,6 @@ ssize_t pcnet_receive(NetClientState *nc, const uint8_t *buf, size_t size_)
     printf("pcnet_receive size=%zu\n", size);
 #endif
 
-    /* if too small buffer, then expand it */
-    if (size < MIN_BUF_SIZE) {
-        memcpy(buf1, buf, size);
-        memset(buf1 + size, 0, MIN_BUF_SIZE - size);
-        buf = buf1;
-        size = MIN_BUF_SIZE;
-    }
-
     if (CSR_PROM(s)
         || (is_padr=padr_match(s, buf, size))
         || (is_bcast=padr_bcast(s, buf, size))
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v6 6/8] hw/net: rtl8139: Remove the logic of padding short frames in the receive path
  2023-03-01  9:01 [PATCH v6 0/8] net: Pad short frames for network backends bmeng.cn
                   ` (4 preceding siblings ...)
  2023-03-01  9:02 ` [PATCH v6 5/8] hw/net: pcnet: " bmeng.cn
@ 2023-03-01  9:02 ` bmeng.cn
  2023-03-01  9:02 ` [PATCH v6 7/8] hw/net: sungem: " bmeng.cn
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: bmeng.cn @ 2023-03-01  9:02 UTC (permalink / raw)
  To: Jason Wang, qemu-devel; +Cc: Bin Meng, Bin Meng

From: Bin Meng <bmeng.cn@gmail.com>

Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---

 hw/net/rtl8139.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 5a5aaf868d..a52b961bda 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -826,7 +826,6 @@ static ssize_t rtl8139_do_receive(NetClientState *nc, const uint8_t *buf, size_t
 
     uint32_t packet_header = 0;
 
-    uint8_t buf1[MIN_BUF_SIZE + VLAN_HLEN];
     static const uint8_t broadcast_macaddr[6] =
         { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 
@@ -938,17 +937,6 @@ static ssize_t rtl8139_do_receive(NetClientState *nc, const uint8_t *buf, size_t
         }
     }
 
-    /* if too small buffer, then expand it
-     * Include some tailroom in case a vlan tag is later removed. */
-    if (size < MIN_BUF_SIZE + VLAN_HLEN) {
-        memcpy(buf1, buf, size);
-        memset(buf1 + size, 0, MIN_BUF_SIZE + VLAN_HLEN - size);
-        buf = buf1;
-        if (size < MIN_BUF_SIZE) {
-            size = MIN_BUF_SIZE;
-        }
-    }
-
     if (rtl8139_cp_receiver_enabled(s))
     {
         if (!rtl8139_cp_rx_valid(s)) {
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v6 7/8] hw/net: sungem: Remove the logic of padding short frames in the receive path
  2023-03-01  9:01 [PATCH v6 0/8] net: Pad short frames for network backends bmeng.cn
                   ` (5 preceding siblings ...)
  2023-03-01  9:02 ` [PATCH v6 6/8] hw/net: rtl8139: " bmeng.cn
@ 2023-03-01  9:02 ` bmeng.cn
  2023-03-01  9:02 ` [PATCH v6 8/8] hw/net: sunhme: " bmeng.cn
  2023-03-01  9:12 ` [PATCH v6 0/8] net: Pad short frames for network backends Cédric Le Goater
  8 siblings, 0 replies; 11+ messages in thread
From: bmeng.cn @ 2023-03-01  9:02 UTC (permalink / raw)
  To: Jason Wang, qemu-devel; +Cc: Bin Meng, Bin Meng

From: Bin Meng <bmeng.cn@gmail.com>

Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---

 hw/net/sungem.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/hw/net/sungem.c b/hw/net/sungem.c
index eb01520790..103376c133 100644
--- a/hw/net/sungem.c
+++ b/hw/net/sungem.c
@@ -550,7 +550,6 @@ static ssize_t sungem_receive(NetClientState *nc, const uint8_t *buf,
     PCIDevice *d = PCI_DEVICE(s);
     uint32_t mac_crc, done, kick, max_fsize;
     uint32_t fcs_size, ints, rxdma_cfg, rxmac_cfg, csum, coff;
-    uint8_t smallbuf[60];
     struct gem_rxd desc;
     uint64_t dbase, baddr;
     unsigned int rx_cond;
@@ -584,19 +583,6 @@ static ssize_t sungem_receive(NetClientState *nc, const uint8_t *buf,
         return size;
     }
 
-    /* We don't drop too small frames since we get them in qemu, we pad
-     * them instead. We should probably use the min frame size register
-     * but I don't want to use a variable size staging buffer and I
-     * know both MacOS and Linux use the default 64 anyway. We use 60
-     * here to account for the non-existent FCS.
-     */
-    if (size < 60) {
-        memcpy(smallbuf, buf, size);
-        memset(&smallbuf[size], 0, 60 - size);
-        buf = smallbuf;
-        size = 60;
-    }
-
     /* Get MAC crc */
     mac_crc = net_crc32_le(buf, ETH_ALEN);
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v6 8/8] hw/net: sunhme: Remove the logic of padding short frames in the receive path
  2023-03-01  9:01 [PATCH v6 0/8] net: Pad short frames for network backends bmeng.cn
                   ` (6 preceding siblings ...)
  2023-03-01  9:02 ` [PATCH v6 7/8] hw/net: sungem: " bmeng.cn
@ 2023-03-01  9:02 ` bmeng.cn
  2023-03-01  9:12 ` [PATCH v6 0/8] net: Pad short frames for network backends Cédric Le Goater
  8 siblings, 0 replies; 11+ messages in thread
From: bmeng.cn @ 2023-03-01  9:02 UTC (permalink / raw)
  To: Jason Wang, qemu-devel; +Cc: Bin Meng, Bin Meng

From: Bin Meng <bmeng.cn@gmail.com>

Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Bin Meng <bmeng@tinylab.org>
---

 hw/net/sunhme.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/hw/net/sunhme.c b/hw/net/sunhme.c
index 1f3d8011ae..391d26fb82 100644
--- a/hw/net/sunhme.c
+++ b/hw/net/sunhme.c
@@ -714,8 +714,6 @@ static inline void sunhme_set_rx_ring_nr(SunHMEState *s, int i)
     s->erxregs[HME_ERXI_RING >> 2] = ring;
 }
 
-#define MIN_BUF_SIZE 60
-
 static ssize_t sunhme_receive(NetClientState *nc, const uint8_t *buf,
                               size_t size)
 {
@@ -724,7 +722,6 @@ static ssize_t sunhme_receive(NetClientState *nc, const uint8_t *buf,
     dma_addr_t rb, addr;
     uint32_t intstatus, status, buffer, buffersize, sum;
     uint16_t csum;
-    uint8_t buf1[60];
     int nr, cr, len, rxoffset, csum_offset;
 
     trace_sunhme_rx_incoming(size);
@@ -775,14 +772,6 @@ static ssize_t sunhme_receive(NetClientState *nc, const uint8_t *buf,
 
     trace_sunhme_rx_filter_accept();
 
-    /* If too small buffer, then expand it */
-    if (size < MIN_BUF_SIZE) {
-        memcpy(buf1, buf, size);
-        memset(buf1 + size, 0, MIN_BUF_SIZE - size);
-        buf = buf1;
-        size = MIN_BUF_SIZE;
-    }
-
     rb = s->erxregs[HME_ERXI_RING >> 2] & HME_ERXI_RING_ADDR;
     nr = sunhme_get_rx_ring_count(s);
     cr = sunhme_get_rx_ring_nr(s);
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v6 0/8] net: Pad short frames for network backends
  2023-03-01  9:01 [PATCH v6 0/8] net: Pad short frames for network backends bmeng.cn
                   ` (7 preceding siblings ...)
  2023-03-01  9:02 ` [PATCH v6 8/8] hw/net: sunhme: " bmeng.cn
@ 2023-03-01  9:12 ` Cédric Le Goater
  2023-03-01 10:20   ` Bin Meng
  8 siblings, 1 reply; 11+ messages in thread
From: Cédric Le Goater @ 2023-03-01  9:12 UTC (permalink / raw)
  To: bmeng.cn, Jason Wang, qemu-devel
  Cc: Bin Meng, Dmitry Fleytman, Helge Deller, Richard Henderson

Hello Bin,

On 3/1/23 10:01, bmeng.cn@gmail.com wrote:
> From: Bin Meng <bmeng@tinylab.org>
> 
> The minimum Ethernet frame length is 60 bytes. For short frames with
> smaller length like ARP packets (only 42 bytes), on a real world NIC
> it can choose either padding its length to the minimum required 60
> bytes, or sending it out directly to the wire. Such behavior can be
> hardcoded or controled by a register bit. Similarly on the receive
> path, NICs can choose either dropping such short frames directly or
> handing them over to software to handle.
> 
> On the other hand, for the network backends like SLiRP/TAP, they
> don't expose a way to control the short frame behavior. As of today
> they just send/receive data from/to the other end connected to them,
> which means any sized packet is acceptable. So they can send and
> receive short frames without any problem. It is observed that ARP
> packets sent from SLiRP/TAP are 42 bytes, and SLiRP/TAP just send
> these ARP packets to the other end which might be a NIC model that
> does not allow short frames to pass through.
> 
> To provide better compatibility, for packets sent from QEMU network
> backends like SLiRP/TAP, we change to pad short frames before sending
> it out to the other end, if the other end does not forbid it via the
> nc->do_not_pad flag. This ensures a backend as an Ethernet sender
> does not violate the spec. But with this change, the behavior of
> dropping short frames from SLiRP/TAP interfaces in the NIC model
> cannot be emulated because it always receives a packet that is spec
> complaint. The capability of sending short frames from NIC models is
> still supported and short frames can still pass through SLiRP/TAP.
> 
> This series should be able to fix the issue as reported with some
> NIC models before, that ARP requests get dropped, preventing the
> guest from becoming visible on the network. It was workarounded in
> these NIC models on the receive path, that when a short frame is
> received, it is padded up to 60 bytes.

I guess we can drop this code in ftgmac100.c also then :

     /* TODO : Pad to minimum Ethernet frame length */
     /* handle small packets.  */
     if (size < 10) {
         qemu_log_mask(LOG_GUEST_ERROR, "%s: dropped frame of %zd bytes\n",
                       __func__, size);
         return size;
     }

Correct ? No need to resend. I can take care of it.

Thanks,

C.

> Note this is a resend of the v5 [1]. Only the first 4 patches were
> applied in QEMU 6.0, and the reset was said to be queued for 6.1
> but for some reason they never landed in QEMU mainline.
> 
> [1] https://lore.kernel.org/qemu-devel/859cd26a-feb2-ed62-98d5-764841a468cf@redhat.com/
> 
> Bin Meng (8):
>    hw/net: e1000: Remove the logic of padding short frames in the receive
>      path
>    hw/net: vmxnet3: Remove the logic of padding short frames in the
>      receive path
>    hw/net: i82596: Remove the logic of padding short frames in the
>      receive path
>    hw/net: ne2000: Remove the logic of padding short frames in the
>      receive path
>    hw/net: pcnet: Remove the logic of padding short frames in the receive
>      path
>    hw/net: rtl8139: Remove the logic of padding short frames in the
>      receive path
>    hw/net: sungem: Remove the logic of padding short frames in the
>      receive path
>    hw/net: sunhme: Remove the logic of padding short frames in the
>      receive path
> 
>   hw/net/e1000.c   | 11 +----------
>   hw/net/i82596.c  | 18 ------------------
>   hw/net/ne2000.c  | 12 ------------
>   hw/net/pcnet.c   |  9 ---------
>   hw/net/rtl8139.c | 12 ------------
>   hw/net/sungem.c  | 14 --------------
>   hw/net/sunhme.c  | 11 -----------
>   hw/net/vmxnet3.c | 10 ----------
>   8 files changed, 1 insertion(+), 96 deletions(-)
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v6 0/8] net: Pad short frames for network backends
  2023-03-01  9:12 ` [PATCH v6 0/8] net: Pad short frames for network backends Cédric Le Goater
@ 2023-03-01 10:20   ` Bin Meng
  0 siblings, 0 replies; 11+ messages in thread
From: Bin Meng @ 2023-03-01 10:20 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: Jason Wang, qemu-devel, Bin Meng, Dmitry Fleytman, Helge Deller,
	Richard Henderson

Hi Cédric,

On Wed, Mar 1, 2023 at 5:13 PM Cédric Le Goater <clg@kaod.org> wrote:
>
> Hello Bin,
>
> On 3/1/23 10:01, bmeng.cn@gmail.com wrote:
> > From: Bin Meng <bmeng@tinylab.org>
> >
> > The minimum Ethernet frame length is 60 bytes. For short frames with
> > smaller length like ARP packets (only 42 bytes), on a real world NIC
> > it can choose either padding its length to the minimum required 60
> > bytes, or sending it out directly to the wire. Such behavior can be
> > hardcoded or controled by a register bit. Similarly on the receive
> > path, NICs can choose either dropping such short frames directly or
> > handing them over to software to handle.
> >
> > On the other hand, for the network backends like SLiRP/TAP, they
> > don't expose a way to control the short frame behavior. As of today
> > they just send/receive data from/to the other end connected to them,
> > which means any sized packet is acceptable. So they can send and
> > receive short frames without any problem. It is observed that ARP
> > packets sent from SLiRP/TAP are 42 bytes, and SLiRP/TAP just send
> > these ARP packets to the other end which might be a NIC model that
> > does not allow short frames to pass through.
> >
> > To provide better compatibility, for packets sent from QEMU network
> > backends like SLiRP/TAP, we change to pad short frames before sending
> > it out to the other end, if the other end does not forbid it via the
> > nc->do_not_pad flag. This ensures a backend as an Ethernet sender
> > does not violate the spec. But with this change, the behavior of
> > dropping short frames from SLiRP/TAP interfaces in the NIC model
> > cannot be emulated because it always receives a packet that is spec
> > complaint. The capability of sending short frames from NIC models is
> > still supported and short frames can still pass through SLiRP/TAP.
> >
> > This series should be able to fix the issue as reported with some
> > NIC models before, that ARP requests get dropped, preventing the
> > guest from becoming visible on the network. It was workarounded in
> > these NIC models on the receive path, that when a short frame is
> > received, it is padded up to 60 bytes.
>
> I guess we can drop this code in ftgmac100.c also then :
>
>      /* TODO : Pad to minimum Ethernet frame length */
>      /* handle small packets.  */
>      if (size < 10) {
>          qemu_log_mask(LOG_GUEST_ERROR, "%s: dropped frame of %zd bytes\n",
>                        __func__, size);
>          return size;
>      }
>
> Correct ? No need to resend. I can take care of it.
>

Yes, I think so. Thanks!

Regards,
Bin


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-03-01 10:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-01  9:01 [PATCH v6 0/8] net: Pad short frames for network backends bmeng.cn
2023-03-01  9:01 ` [PATCH v6 1/8] hw/net: e1000: Remove the logic of padding short frames in the receive path bmeng.cn
2023-03-01  9:01 ` [PATCH v6 2/8] hw/net: vmxnet3: " bmeng.cn
2023-03-01  9:01 ` [PATCH v6 3/8] hw/net: i82596: " bmeng.cn
2023-03-01  9:01 ` [PATCH v6 4/8] hw/net: ne2000: " bmeng.cn
2023-03-01  9:02 ` [PATCH v6 5/8] hw/net: pcnet: " bmeng.cn
2023-03-01  9:02 ` [PATCH v6 6/8] hw/net: rtl8139: " bmeng.cn
2023-03-01  9:02 ` [PATCH v6 7/8] hw/net: sungem: " bmeng.cn
2023-03-01  9:02 ` [PATCH v6 8/8] hw/net: sunhme: " bmeng.cn
2023-03-01  9:12 ` [PATCH v6 0/8] net: Pad short frames for network backends Cédric Le Goater
2023-03-01 10:20   ` Bin Meng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.