All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 00/13] Net patches
@ 2021-03-22 10:07 Jason Wang
  2021-03-22 10:08 ` [PULL 01/13] net: eth: Add a helper to pad a short Ethernet frame Jason Wang
                   ` (13 more replies)
  0 siblings, 14 replies; 30+ messages in thread
From: Jason Wang @ 2021-03-22 10:07 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: Jason Wang

The following changes since commit bdee969c0e65d4d509932b1d70e3a3b2ffbff6d5:

  Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' into staging (2021-03-19 18:01:17 +0000)

are available in the git repository at:

  https://github.com/jasowang/qemu.git tags/net-pull-request

for you to fetch changes up to c7274b5ef43614dd133daec1e2018f71d8744088:

  net/eth: Add an assert() and invert if() statement to simplify code (2021-03-22 17:34:31 +0800)

----------------------------------------------------------------

----------------------------------------------------------------
Bin Meng (4):
      net: eth: Add a helper to pad a short Ethernet frame
      net: Add a 'do_not_pad" to NetClientState
      net: Pad short frames to minimum size before sending from SLiRP/TAP
      hw/net: virtio-net: Initialize nc->do_not_pad to true

Lukas Straub (2):
      net/colo-compare.c: Fix memory leak for non-tcp packet
      net/colo-compare.c: Optimize removal of secondary packet

Philippe Mathieu-Daudé (7):
      net/eth: Use correct in6_address offset in _eth_get_rss_ex_dst_addr()
      net/eth: Simplify _eth_get_rss_ex_dst_addr()
      net/eth: Better describe _eth_get_rss_ex_dst_addr's offset argument
      net/eth: Check size earlier in _eth_get_rss_ex_dst_addr()
      net/eth: Check iovec has enough data earlier
      net/eth: Read ip6_ext_hdr_routing buffer before accessing it
      net/eth: Add an assert() and invert if() statement to simplify code

 MAINTAINERS                    |  1 +
 hw/net/virtio-net.c            |  4 +++
 include/net/eth.h              | 17 ++++++++++++
 include/net/net.h              |  1 +
 net/colo-compare.c             |  3 ++-
 net/eth.c                      | 61 +++++++++++++++++++++++++++---------------
 net/slirp.c                    | 10 +++++++
 net/tap-win32.c                | 10 +++++++
 net/tap.c                      | 10 +++++++
 tests/qtest/fuzz-e1000e-test.c | 53 ++++++++++++++++++++++++++++++++++++
 tests/qtest/meson.build        |  1 +
 11 files changed, 148 insertions(+), 23 deletions(-)
 create mode 100644 tests/qtest/fuzz-e1000e-test.c



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

* [PULL 01/13] net: eth: Add a helper to pad a short Ethernet frame
  2021-03-22 10:07 [PULL 00/13] Net patches Jason Wang
@ 2021-03-22 10:08 ` Jason Wang
  2021-03-22 10:08 ` [PULL 02/13] net: Add a 'do_not_pad" to NetClientState Jason Wang
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Jason Wang @ 2021-03-22 10:08 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: Jason Wang, Bin Meng

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

Add a helper to pad a short Ethernet frame to the minimum required
length, which can be used by backends' code.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 include/net/eth.h | 17 +++++++++++++++++
 net/eth.c         | 17 +++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/include/net/eth.h b/include/net/eth.h
index 0671be6..7767ae8 100644
--- a/include/net/eth.h
+++ b/include/net/eth.h
@@ -31,6 +31,7 @@
 
 #define ETH_ALEN 6
 #define ETH_HLEN 14
+#define ETH_ZLEN 60     /* Min. octets in frame without FCS */
 
 struct eth_header {
     uint8_t  h_dest[ETH_ALEN];   /* destination eth addr */
@@ -422,4 +423,20 @@ bool
 eth_parse_ipv6_hdr(const struct iovec *pkt, int pkt_frags,
                    size_t ip6hdr_off, eth_ip6_hdr_info *info);
 
+/**
+ * eth_pad_short_frame - pad a short frame to the minimum Ethernet frame length
+ *
+ * If the Ethernet frame size is shorter than 60 bytes, it will be padded to
+ * 60 bytes at the address @padded_pkt.
+ *
+ * @padded_pkt: buffer address to hold the padded frame
+ * @padded_buflen: pointer holding length of @padded_pkt. If the frame is
+ *                 padded, the length will be updated to the padded one.
+ * @pkt: address to hold the original Ethernet frame
+ * @pkt_size: size of the original Ethernet frame
+ * @return true if the frame is padded, otherwise false
+ */
+bool eth_pad_short_frame(uint8_t *padded_pkt, size_t *padded_buflen,
+                         const void *pkt, size_t pkt_size);
+
 #endif
diff --git a/net/eth.c b/net/eth.c
index 1e0821c..f913e43 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -548,3 +548,20 @@ bool eth_parse_ipv6_hdr(const struct iovec *pkt, int pkt_frags,
     info->l4proto = ext_hdr.ip6r_nxt;
     return true;
 }
+
+bool eth_pad_short_frame(uint8_t *padded_pkt, size_t *padded_buflen,
+                         const void *pkt, size_t pkt_size)
+{
+    assert(padded_buflen && *padded_buflen >= ETH_ZLEN);
+
+    if (pkt_size >= ETH_ZLEN) {
+        return false;
+    }
+
+    /* pad to minimum Ethernet frame length */
+    memcpy(padded_pkt, pkt, pkt_size);
+    memset(&padded_pkt[pkt_size], 0, ETH_ZLEN - pkt_size);
+    *padded_buflen = ETH_ZLEN;
+
+    return true;
+}
-- 
2.7.4



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

* [PULL 02/13] net: Add a 'do_not_pad" to NetClientState
  2021-03-22 10:07 [PULL 00/13] Net patches Jason Wang
  2021-03-22 10:08 ` [PULL 01/13] net: eth: Add a helper to pad a short Ethernet frame Jason Wang
@ 2021-03-22 10:08 ` Jason Wang
  2021-03-22 10:08 ` [PULL 03/13] net: Pad short frames to minimum size before sending from SLiRP/TAP Jason Wang
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Jason Wang @ 2021-03-22 10:08 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: Jason Wang, Bin Meng

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

This adds a flag in NetClientState, so that a net client can tell
its peer that the packets do not need to be padded to the minimum
size of an Ethernet frame (60 bytes) before sending to it.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 include/net/net.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/net/net.h b/include/net/net.h
index a02949f..3559f3c 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -103,6 +103,7 @@ struct NetClientState {
     int vring_enable;
     int vnet_hdr_len;
     bool is_netdev;
+    bool do_not_pad; /* do not pad to the minimum ethernet frame length */
     QTAILQ_HEAD(, NetFilterState) filters;
 };
 
-- 
2.7.4



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

* [PULL 03/13] net: Pad short frames to minimum size before sending from SLiRP/TAP
  2021-03-22 10:07 [PULL 00/13] Net patches Jason Wang
  2021-03-22 10:08 ` [PULL 01/13] net: eth: Add a helper to pad a short Ethernet frame Jason Wang
  2021-03-22 10:08 ` [PULL 02/13] net: Add a 'do_not_pad" to NetClientState Jason Wang
@ 2021-03-22 10:08 ` Jason Wang
  2021-03-22 10:08 ` [PULL 04/13] hw/net: virtio-net: Initialize nc->do_not_pad to true Jason Wang
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Jason Wang @ 2021-03-22 10:08 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: Jason Wang, Bin Meng

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

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 commit 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.

The following 2 commits seem to be the one to workaround this issue
in e1000 and vmxenet3 before, and should probably 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 <bmeng.cn@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/slirp.c     | 10 ++++++++++
 net/tap-win32.c | 10 ++++++++++
 net/tap.c       | 10 ++++++++++
 3 files changed, 30 insertions(+)

diff --git a/net/slirp.c b/net/slirp.c
index 9454a67..a9fdc7a 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -31,6 +31,7 @@
 #include <pwd.h>
 #include <sys/wait.h>
 #endif
+#include "net/eth.h"
 #include "net/net.h"
 #include "clients.h"
 #include "hub.h"
@@ -115,6 +116,15 @@ static ssize_t net_slirp_send_packet(const void *pkt, size_t pkt_len,
                                      void *opaque)
 {
     SlirpState *s = opaque;
+    uint8_t min_pkt[ETH_ZLEN];
+    size_t min_pktsz = sizeof(min_pkt);
+
+    if (!s->nc.peer->do_not_pad) {
+        if (eth_pad_short_frame(min_pkt, &min_pktsz, pkt, pkt_len)) {
+            pkt = min_pkt;
+            pkt_len = min_pktsz;
+        }
+    }
 
     return qemu_send_packet(&s->nc, pkt, pkt_len);
 }
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 21e4511..d7c2a87 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -31,6 +31,7 @@
 
 #include "qemu-common.h"
 #include "clients.h"            /* net_init_tap */
+#include "net/eth.h"
 #include "net/net.h"
 #include "net/tap.h"            /* tap_has_ufo, ... */
 #include "qemu/error-report.h"
@@ -688,9 +689,18 @@ static void tap_win32_send(void *opaque)
     uint8_t *buf;
     int max_size = 4096;
     int size;
+    uint8_t min_pkt[ETH_ZLEN];
+    size_t min_pktsz = sizeof(min_pkt);
 
     size = tap_win32_read(s->handle, &buf, max_size);
     if (size > 0) {
+        if (!s->nc.peer->do_not_pad) {
+            if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
+                buf = min_pkt;
+                size = min_pktsz;
+            }
+        }
+
         qemu_send_packet(&s->nc, buf, size);
         tap_win32_free_buffer(s->handle, buf);
     }
diff --git a/net/tap.c b/net/tap.c
index 12a08d5..d6d8456 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -32,6 +32,7 @@
 #include <sys/socket.h>
 #include <net/if.h>
 
+#include "net/eth.h"
 #include "net/net.h"
 #include "clients.h"
 #include "monitor/monitor.h"
@@ -189,6 +190,8 @@ static void tap_send(void *opaque)
 
     while (true) {
         uint8_t *buf = s->buf;
+        uint8_t min_pkt[ETH_ZLEN];
+        size_t min_pktsz = sizeof(min_pkt);
 
         size = tap_read_packet(s->fd, s->buf, sizeof(s->buf));
         if (size <= 0) {
@@ -200,6 +203,13 @@ static void tap_send(void *opaque)
             size -= s->host_vnet_hdr_len;
         }
 
+        if (!s->nc.peer->do_not_pad) {
+            if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
+                buf = min_pkt;
+                size = min_pktsz;
+            }
+        }
+
         size = qemu_send_packet_async(&s->nc, buf, size, tap_send_completed);
         if (size == 0) {
             tap_read_poll(s, false);
-- 
2.7.4



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

* [PULL 04/13] hw/net: virtio-net: Initialize nc->do_not_pad to true
  2021-03-22 10:07 [PULL 00/13] Net patches Jason Wang
                   ` (2 preceding siblings ...)
  2021-03-22 10:08 ` [PULL 03/13] net: Pad short frames to minimum size before sending from SLiRP/TAP Jason Wang
@ 2021-03-22 10:08 ` Jason Wang
  2021-03-22 10:08 ` [PULL 05/13] net/colo-compare.c: Fix memory leak for non-tcp packet Jason Wang
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Jason Wang @ 2021-03-22 10:08 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: Jason Wang, Bin Meng

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

For virtio-net, there is no need to pad the Ethernet frame size to
60 bytes before sending to it.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/virtio-net.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 96a3cc8..66b9ff4 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3314,6 +3314,10 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
                               object_get_typename(OBJECT(dev)), dev->id, n);
     }
 
+    for (i = 0; i < n->max_queues; i++) {
+        n->nic->ncs[i].do_not_pad = true;
+    }
+
     peer_test_vnet_hdr(n);
     if (peer_has_vnet_hdr(n)) {
         for (i = 0; i < n->max_queues; i++) {
-- 
2.7.4



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

* [PULL 05/13] net/colo-compare.c: Fix memory leak for non-tcp packet
  2021-03-22 10:07 [PULL 00/13] Net patches Jason Wang
                   ` (3 preceding siblings ...)
  2021-03-22 10:08 ` [PULL 04/13] hw/net: virtio-net: Initialize nc->do_not_pad to true Jason Wang
@ 2021-03-22 10:08 ` Jason Wang
  2021-03-22 10:08 ` [PULL 06/13] net/colo-compare.c: Optimize removal of secondary packet Jason Wang
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Jason Wang @ 2021-03-22 10:08 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: Zhang Chen, Jason Wang, Lukas Straub

From: Lukas Straub <lukasstraub2@web.de>

Additional to removing the packet from the secondary queue,
we also need to free it.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/colo-compare.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 84db497..2e819ff 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -690,6 +690,7 @@ static void colo_compare_packet(CompareState *s, Connection *conn,
 
         if (result) {
             colo_release_primary_pkt(s, pkt);
+            packet_destroy(result->data, NULL);
             g_queue_remove(&conn->secondary_list, result->data);
         } else {
             /*
-- 
2.7.4



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

* [PULL 06/13] net/colo-compare.c: Optimize removal of secondary packet
  2021-03-22 10:07 [PULL 00/13] Net patches Jason Wang
                   ` (4 preceding siblings ...)
  2021-03-22 10:08 ` [PULL 05/13] net/colo-compare.c: Fix memory leak for non-tcp packet Jason Wang
@ 2021-03-22 10:08 ` Jason Wang
  2021-03-22 10:08 ` [PULL 07/13] net/eth: Use correct in6_address offset in _eth_get_rss_ex_dst_addr() Jason Wang
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Jason Wang @ 2021-03-22 10:08 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: Zhang Chen, Jason Wang, Lukas Straub

From: Lukas Straub <lukasstraub2@web.de>

g_queue_remove needs to look up the list entry first, but we
already have it as result and can remove it directly with
g_queue_delete_link.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/colo-compare.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 2e819ff..9d1ad99 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -691,7 +691,7 @@ static void colo_compare_packet(CompareState *s, Connection *conn,
         if (result) {
             colo_release_primary_pkt(s, pkt);
             packet_destroy(result->data, NULL);
-            g_queue_remove(&conn->secondary_list, result->data);
+            g_queue_delete_link(&conn->secondary_list, result);
         } else {
             /*
              * If one packet arrive late, the secondary_list or
-- 
2.7.4



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

* [PULL 07/13] net/eth: Use correct in6_address offset in _eth_get_rss_ex_dst_addr()
  2021-03-22 10:07 [PULL 00/13] Net patches Jason Wang
                   ` (5 preceding siblings ...)
  2021-03-22 10:08 ` [PULL 06/13] net/colo-compare.c: Optimize removal of secondary packet Jason Wang
@ 2021-03-22 10:08 ` Jason Wang
  2021-03-22 10:08 ` [PULL 08/13] net/eth: Simplify _eth_get_rss_ex_dst_addr() Jason Wang
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Jason Wang @ 2021-03-22 10:08 UTC (permalink / raw)
  To: qemu-devel, peter.maydell
  Cc: Jason Wang, Philippe Mathieu-Daudé, qemu-stable

From: Philippe Mathieu-Daudé <philmd@redhat.com>

The in6_address comes after the ip6_ext_hdr_routing header,
not after the ip6_ext_hdr one. Fix the offset.

Cc: qemu-stable@nongnu.org
Reported-by: Stefano Garzarella <sgarzare@redhat.com>
Fixes: eb700029c78 ("net_pkt: Extend packet abstraction as required by e1000e functionality")
Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/eth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/eth.c b/net/eth.c
index f913e43..c8babfa 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -419,7 +419,7 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt, int pkt_frags,
         }
 
         bytes_read = iov_to_buf(pkt, pkt_frags,
-                                rthdr_offset + sizeof(*ext_hdr),
+                                rthdr_offset + sizeof(*rthdr),
                                 dst_addr, sizeof(*dst_addr));
 
         return bytes_read == sizeof(*dst_addr);
-- 
2.7.4



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

* [PULL 08/13] net/eth: Simplify _eth_get_rss_ex_dst_addr()
  2021-03-22 10:07 [PULL 00/13] Net patches Jason Wang
                   ` (6 preceding siblings ...)
  2021-03-22 10:08 ` [PULL 07/13] net/eth: Use correct in6_address offset in _eth_get_rss_ex_dst_addr() Jason Wang
@ 2021-03-22 10:08 ` Jason Wang
  2021-03-22 10:08 ` [PULL 09/13] net/eth: Better describe _eth_get_rss_ex_dst_addr's offset argument Jason Wang
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Jason Wang @ 2021-03-22 10:08 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: Jason Wang, Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

The length field is already contained in the ip6_ext_hdr structure.
Check it direcly in eth_parse_ipv6_hdr() before calling
_eth_get_rss_ex_dst_addr(), which gets a bit simplified.

Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/eth.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/eth.c b/net/eth.c
index c8babfa..06badd1 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -407,9 +407,7 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt, int pkt_frags,
 {
     struct ip6_ext_hdr_routing *rthdr = (struct ip6_ext_hdr_routing *) ext_hdr;
 
-    if ((rthdr->rtype == 2) &&
-        (rthdr->len == sizeof(struct in6_address) / 8) &&
-        (rthdr->segleft == 1)) {
+    if ((rthdr->rtype == 2) && (rthdr->segleft == 1)) {
 
         size_t input_size = iov_size(pkt, pkt_frags);
         size_t bytes_read;
@@ -528,10 +526,12 @@ bool eth_parse_ipv6_hdr(const struct iovec *pkt, int pkt_frags,
         }
 
         if (curr_ext_hdr_type == IP6_ROUTING) {
-            info->rss_ex_dst_valid =
-                _eth_get_rss_ex_dst_addr(pkt, pkt_frags,
-                                         ip6hdr_off + info->full_hdr_len,
-                                         &ext_hdr, &info->rss_ex_dst);
+            if (ext_hdr.ip6r_len == sizeof(struct in6_address) / 8) {
+                info->rss_ex_dst_valid =
+                    _eth_get_rss_ex_dst_addr(pkt, pkt_frags,
+                                             ip6hdr_off + info->full_hdr_len,
+                                             &ext_hdr, &info->rss_ex_dst);
+            }
         } else if (curr_ext_hdr_type == IP6_DESTINATON) {
             info->rss_ex_src_valid =
                 _eth_get_rss_ex_src_addr(pkt, pkt_frags,
-- 
2.7.4



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

* [PULL 09/13] net/eth: Better describe _eth_get_rss_ex_dst_addr's offset argument
  2021-03-22 10:07 [PULL 00/13] Net patches Jason Wang
                   ` (7 preceding siblings ...)
  2021-03-22 10:08 ` [PULL 08/13] net/eth: Simplify _eth_get_rss_ex_dst_addr() Jason Wang
@ 2021-03-22 10:08 ` Jason Wang
  2021-03-22 10:08 ` [PULL 10/13] net/eth: Check size earlier in _eth_get_rss_ex_dst_addr() Jason Wang
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Jason Wang @ 2021-03-22 10:08 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: Jason Wang, Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

The 'offset' argument represents the offset to the ip6_ext_hdr
header, rename it as 'ext_hdr_offset'.

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/eth.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/eth.c b/net/eth.c
index 06badd1..5eb05bd 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -401,7 +401,7 @@ eth_is_ip6_extension_header_type(uint8_t hdr_type)
 
 static bool
 _eth_get_rss_ex_dst_addr(const struct iovec *pkt, int pkt_frags,
-                        size_t rthdr_offset,
+                        size_t ext_hdr_offset,
                         struct ip6_ext_hdr *ext_hdr,
                         struct in6_address *dst_addr)
 {
@@ -412,12 +412,12 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt, int pkt_frags,
         size_t input_size = iov_size(pkt, pkt_frags);
         size_t bytes_read;
 
-        if (input_size < rthdr_offset + sizeof(*ext_hdr)) {
+        if (input_size < ext_hdr_offset + sizeof(*ext_hdr)) {
             return false;
         }
 
         bytes_read = iov_to_buf(pkt, pkt_frags,
-                                rthdr_offset + sizeof(*rthdr),
+                                ext_hdr_offset + sizeof(*rthdr),
                                 dst_addr, sizeof(*dst_addr));
 
         return bytes_read == sizeof(*dst_addr);
-- 
2.7.4



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

* [PULL 10/13] net/eth: Check size earlier in _eth_get_rss_ex_dst_addr()
  2021-03-22 10:07 [PULL 00/13] Net patches Jason Wang
                   ` (8 preceding siblings ...)
  2021-03-22 10:08 ` [PULL 09/13] net/eth: Better describe _eth_get_rss_ex_dst_addr's offset argument Jason Wang
@ 2021-03-22 10:08 ` Jason Wang
  2021-03-22 10:08 ` [PULL 11/13] net/eth: Check iovec has enough data earlier Jason Wang
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Jason Wang @ 2021-03-22 10:08 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: Jason Wang, Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/eth.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/net/eth.c b/net/eth.c
index 5eb05bd..087aa71 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -406,16 +406,14 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt, int pkt_frags,
                         struct in6_address *dst_addr)
 {
     struct ip6_ext_hdr_routing *rthdr = (struct ip6_ext_hdr_routing *) ext_hdr;
+    size_t input_size = iov_size(pkt, pkt_frags);
+    size_t bytes_read;
 
-    if ((rthdr->rtype == 2) && (rthdr->segleft == 1)) {
-
-        size_t input_size = iov_size(pkt, pkt_frags);
-        size_t bytes_read;
-
-        if (input_size < ext_hdr_offset + sizeof(*ext_hdr)) {
-            return false;
-        }
+    if (input_size < ext_hdr_offset + sizeof(*ext_hdr)) {
+        return false;
+    }
 
+    if ((rthdr->rtype == 2) && (rthdr->segleft == 1)) {
         bytes_read = iov_to_buf(pkt, pkt_frags,
                                 ext_hdr_offset + sizeof(*rthdr),
                                 dst_addr, sizeof(*dst_addr));
-- 
2.7.4



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

* [PULL 11/13] net/eth: Check iovec has enough data earlier
  2021-03-22 10:07 [PULL 00/13] Net patches Jason Wang
                   ` (9 preceding siblings ...)
  2021-03-22 10:08 ` [PULL 10/13] net/eth: Check size earlier in _eth_get_rss_ex_dst_addr() Jason Wang
@ 2021-03-22 10:08 ` Jason Wang
  2021-03-22 10:08 ` [PULL 12/13] net/eth: Read ip6_ext_hdr_routing buffer before accessing it Jason Wang
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Jason Wang @ 2021-03-22 10:08 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: Jason Wang, Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

We want to check fields from ip6_ext_hdr_routing structure
and if correct read the full in6_address. Let's directly check
if our iovec contains enough data for everything, else return
early.

Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/eth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/eth.c b/net/eth.c
index 087aa71..6db943d 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -409,7 +409,7 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt, int pkt_frags,
     size_t input_size = iov_size(pkt, pkt_frags);
     size_t bytes_read;
 
-    if (input_size < ext_hdr_offset + sizeof(*ext_hdr)) {
+    if (input_size < ext_hdr_offset + sizeof(*rthdr) + sizeof(*dst_addr)) {
         return false;
     }
 
-- 
2.7.4



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

* [PULL 12/13] net/eth: Read ip6_ext_hdr_routing buffer before accessing it
  2021-03-22 10:07 [PULL 00/13] Net patches Jason Wang
                   ` (10 preceding siblings ...)
  2021-03-22 10:08 ` [PULL 11/13] net/eth: Check iovec has enough data earlier Jason Wang
@ 2021-03-22 10:08 ` Jason Wang
  2021-03-22 10:08 ` [PULL 13/13] net/eth: Add an assert() and invert if() statement to simplify code Jason Wang
  2021-03-22 14:13 ` [PULL 00/13] Net patches Peter Maydell
  13 siblings, 0 replies; 30+ messages in thread
From: Jason Wang @ 2021-03-22 10:08 UTC (permalink / raw)
  To: qemu-devel, peter.maydell
  Cc: Jason Wang, Philippe Mathieu-Daudé, qemu-stable

From: Philippe Mathieu-Daudé <philmd@redhat.com>

We can't know the caller read enough data in the memory pointed
by ext_hdr to cast it as a ip6_ext_hdr_routing.
Declare rt_hdr on the stack and fill it again from the iovec.

Since we already checked there is enough data in the iovec buffer,
simply add an assert() call to consume the bytes_read variable.

This fix a 2 bytes buffer overrun in eth_parse_ipv6_hdr() reported
by QEMU fuzzer:

  $ cat << EOF | ./qemu-system-i386 -M pc-q35-5.0 \
    -accel qtest -monitor none \
    -serial none -nographic -qtest stdio
  outl 0xcf8 0x80001010
  outl 0xcfc 0xe1020000
  outl 0xcf8 0x80001004
  outw 0xcfc 0x7
  write 0x25 0x1 0x86
  write 0x26 0x1 0xdd
  write 0x4f 0x1 0x2b
  write 0xe1020030 0x4 0x190002e1
  write 0xe102003a 0x2 0x0807
  write 0xe1020048 0x4 0x12077cdd
  write 0xe1020400 0x4 0xba077cdd
  write 0xe1020420 0x4 0x190002e1
  write 0xe1020428 0x4 0x3509d807
  write 0xe1020438 0x1 0xe2
  EOF
  =================================================================
  ==2859770==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffdef904902 at pc 0x561ceefa78de bp 0x7ffdef904820 sp 0x7ffdef904818
  READ of size 1 at 0x7ffdef904902 thread T0
      #0 0x561ceefa78dd in _eth_get_rss_ex_dst_addr net/eth.c:410:17
      #1 0x561ceefa41fb in eth_parse_ipv6_hdr net/eth.c:532:17
      #2 0x561cef7de639 in net_tx_pkt_parse_headers hw/net/net_tx_pkt.c:228:14
      #3 0x561cef7dbef4 in net_tx_pkt_parse hw/net/net_tx_pkt.c:273:9
      #4 0x561ceec29f22 in e1000e_process_tx_desc hw/net/e1000e_core.c:730:29
      #5 0x561ceec28eac in e1000e_start_xmit hw/net/e1000e_core.c:927:9
      #6 0x561ceec1baab in e1000e_set_tdt hw/net/e1000e_core.c:2444:9
      #7 0x561ceebf300e in e1000e_core_write hw/net/e1000e_core.c:3256:9
      #8 0x561cef3cd4cd in e1000e_mmio_write hw/net/e1000e.c:110:5

  Address 0x7ffdef904902 is located in stack of thread T0 at offset 34 in frame
      #0 0x561ceefa320f in eth_parse_ipv6_hdr net/eth.c:486

    This frame has 1 object(s):
      [32, 34) 'ext_hdr' (line 487) <== Memory access at offset 34 overflows this variable
  HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
        (longjmp and C++ exceptions *are* supported)
  SUMMARY: AddressSanitizer: stack-buffer-overflow net/eth.c:410:17 in _eth_get_rss_ex_dst_addr
  Shadow bytes around the buggy address:
    0x10003df188d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x10003df188e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x10003df188f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x10003df18900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x10003df18910: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
  =>0x10003df18920:[02]f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
    0x10003df18930: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x10003df18940: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x10003df18950: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x10003df18960: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x10003df18970: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  Shadow byte legend (one shadow byte represents 8 application bytes):
    Addressable:           00
    Partially addressable: 01 02 03 04 05 06 07
    Stack left redzone:      f1
    Stack right redzone:     f3
  ==2859770==ABORTING

Add the corresponding qtest case with the fuzzer reproducer.

FWIW GCC 11 similarly reported:

  net/eth.c: In function 'eth_parse_ipv6_hdr':
  net/eth.c:410:15: error: array subscript 'struct ip6_ext_hdr_routing[0]' is partly outside array bounds of 'struct ip6_ext_hdr[1]' [-Werror=array-bounds]
    410 |     if ((rthdr->rtype == 2) && (rthdr->segleft == 1)) {
        |          ~~~~~^~~~~~~
  net/eth.c:485:24: note: while referencing 'ext_hdr'
    485 |     struct ip6_ext_hdr ext_hdr;
        |                        ^~~~~~~
  net/eth.c:410:38: error: array subscript 'struct ip6_ext_hdr_routing[0]' is partly outside array bounds of 'struct ip6_ext_hdr[1]' [-Werror=array-bounds]
    410 |     if ((rthdr->rtype == 2) && (rthdr->segleft == 1)) {
        |                                 ~~~~~^~~~~~~~~
  net/eth.c:485:24: note: while referencing 'ext_hdr'
    485 |     struct ip6_ext_hdr ext_hdr;
        |                        ^~~~~~~

Cc: qemu-stable@nongnu.org
Buglink: https://bugs.launchpad.net/qemu/+bug/1879531
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Reported-by: Miroslav Rezanina <mrezanin@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
Fixes: eb700029c78 ("net_pkt: Extend packet abstraction as required by e1000e functionality")
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 MAINTAINERS                    |  1 +
 net/eth.c                      | 13 +++++++----
 tests/qtest/fuzz-e1000e-test.c | 53 ++++++++++++++++++++++++++++++++++++++++++
 tests/qtest/meson.build        |  1 +
 4 files changed, 63 insertions(+), 5 deletions(-)
 create mode 100644 tests/qtest/fuzz-e1000e-test.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 25fc49d..9147e9a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2027,6 +2027,7 @@ e1000e
 M: Dmitry Fleytman <dmitry.fleytman@gmail.com>
 S: Maintained
 F: hw/net/e1000e*
+F: tests/qtest/fuzz-e1000e-test.c
 
 eepro100
 M: Stefan Weil <sw@weilnetz.de>
diff --git a/net/eth.c b/net/eth.c
index 6db943d..b2704fb 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -405,17 +405,20 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt, int pkt_frags,
                         struct ip6_ext_hdr *ext_hdr,
                         struct in6_address *dst_addr)
 {
-    struct ip6_ext_hdr_routing *rthdr = (struct ip6_ext_hdr_routing *) ext_hdr;
+    struct ip6_ext_hdr_routing rt_hdr;
     size_t input_size = iov_size(pkt, pkt_frags);
     size_t bytes_read;
 
-    if (input_size < ext_hdr_offset + sizeof(*rthdr) + sizeof(*dst_addr)) {
+    if (input_size < ext_hdr_offset + sizeof(rt_hdr) + sizeof(*dst_addr)) {
         return false;
     }
 
-    if ((rthdr->rtype == 2) && (rthdr->segleft == 1)) {
-        bytes_read = iov_to_buf(pkt, pkt_frags,
-                                ext_hdr_offset + sizeof(*rthdr),
+    bytes_read = iov_to_buf(pkt, pkt_frags, ext_hdr_offset,
+                            &rt_hdr, sizeof(rt_hdr));
+    assert(bytes_read == sizeof(rt_hdr));
+
+    if ((rt_hdr.rtype == 2) && (rt_hdr.segleft == 1)) {
+        bytes_read = iov_to_buf(pkt, pkt_frags, ext_hdr_offset + sizeof(rt_hdr),
                                 dst_addr, sizeof(*dst_addr));
 
         return bytes_read == sizeof(*dst_addr);
diff --git a/tests/qtest/fuzz-e1000e-test.c b/tests/qtest/fuzz-e1000e-test.c
new file mode 100644
index 0000000..66229e6
--- /dev/null
+++ b/tests/qtest/fuzz-e1000e-test.c
@@ -0,0 +1,53 @@
+/*
+ * QTest testcase for e1000e device generated by fuzzer
+ *
+ * Copyright (c) 2021 Red Hat, Inc.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+
+#include "libqos/libqtest.h"
+
+/*
+ * https://bugs.launchpad.net/qemu/+bug/1879531
+ */
+static void test_lp1879531_eth_get_rss_ex_dst_addr(void)
+{
+    QTestState *s;
+
+    s = qtest_init("-nographic -monitor none -serial none -M pc-q35-5.0");
+
+    qtest_outl(s, 0xcf8, 0x80001010);
+    qtest_outl(s, 0xcfc, 0xe1020000);
+    qtest_outl(s, 0xcf8, 0x80001004);
+    qtest_outw(s, 0xcfc, 0x7);
+    qtest_writeb(s, 0x25, 0x86);
+    qtest_writeb(s, 0x26, 0xdd);
+    qtest_writeb(s, 0x4f, 0x2b);
+
+    qtest_writel(s, 0xe1020030, 0x190002e1);
+    qtest_writew(s, 0xe102003a, 0x0807);
+    qtest_writel(s, 0xe1020048, 0x12077cdd);
+    qtest_writel(s, 0xe1020400, 0xba077cdd);
+    qtest_writel(s, 0xe1020420, 0x190002e1);
+    qtest_writel(s, 0xe1020428, 0x3509d807);
+    qtest_writeb(s, 0xe1020438, 0xe2);
+    qtest_writeb(s, 0x4f, 0x2b);
+    qtest_quit(s);
+}
+
+int main(int argc, char **argv)
+{
+    const char *arch = qtest_get_arch();
+
+    g_test_init(&argc, &argv, NULL);
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        qtest_add_func("fuzz/test_lp1879531_eth_get_rss_ex_dst_addr",
+                       test_lp1879531_eth_get_rss_ex_dst_addr);
+    }
+
+    return g_test_run();
+}
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 9731606..902cfef 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -67,6 +67,7 @@ qtests_i386 = \
   (config_all_devices.has_key('CONFIG_TPM_TIS_ISA') ? ['tpm-tis-test'] : []) +              \
   (config_all_devices.has_key('CONFIG_TPM_TIS_ISA') ? ['tpm-tis-swtpm-test'] : []) +        \
   (config_all_devices.has_key('CONFIG_RTL8139_PCI') ? ['rtl8139-test'] : []) +              \
+  (config_all_devices.has_key('CONFIG_E1000E_PCI_EXPRESS') ? ['fuzz-e1000e-test'] : []) +   \
   qtests_pci +                                                                              \
   ['fdc-test',
    'ide-test',
-- 
2.7.4



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

* [PULL 13/13] net/eth: Add an assert() and invert if() statement to simplify code
  2021-03-22 10:07 [PULL 00/13] Net patches Jason Wang
                   ` (11 preceding siblings ...)
  2021-03-22 10:08 ` [PULL 12/13] net/eth: Read ip6_ext_hdr_routing buffer before accessing it Jason Wang
@ 2021-03-22 10:08 ` Jason Wang
  2021-03-22 14:13 ` [PULL 00/13] Net patches Peter Maydell
  13 siblings, 0 replies; 30+ messages in thread
From: Jason Wang @ 2021-03-22 10:08 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: Jason Wang, Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

To simplify the function body, invert the if() statement, returning
earlier.
Since we already checked there is enough data in the iovec buffer,
simply add an assert() call to consume the bytes_read variable.

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/eth.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/net/eth.c b/net/eth.c
index b2704fb..fe876d1 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -416,15 +416,14 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt, int pkt_frags,
     bytes_read = iov_to_buf(pkt, pkt_frags, ext_hdr_offset,
                             &rt_hdr, sizeof(rt_hdr));
     assert(bytes_read == sizeof(rt_hdr));
-
-    if ((rt_hdr.rtype == 2) && (rt_hdr.segleft == 1)) {
-        bytes_read = iov_to_buf(pkt, pkt_frags, ext_hdr_offset + sizeof(rt_hdr),
-                                dst_addr, sizeof(*dst_addr));
-
-        return bytes_read == sizeof(*dst_addr);
+    if ((rt_hdr.rtype != 2) || (rt_hdr.segleft != 1)) {
+        return false;
     }
+    bytes_read = iov_to_buf(pkt, pkt_frags, ext_hdr_offset + sizeof(rt_hdr),
+                            dst_addr, sizeof(*dst_addr));
+    assert(bytes_read == sizeof(*dst_addr));
 
-    return false;
+    return true;
 }
 
 static bool
-- 
2.7.4



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

* Re: [PULL 00/13] Net patches
  2021-03-22 10:07 [PULL 00/13] Net patches Jason Wang
                   ` (12 preceding siblings ...)
  2021-03-22 10:08 ` [PULL 13/13] net/eth: Add an assert() and invert if() statement to simplify code Jason Wang
@ 2021-03-22 14:13 ` Peter Maydell
  13 siblings, 0 replies; 30+ messages in thread
From: Peter Maydell @ 2021-03-22 14:13 UTC (permalink / raw)
  To: Jason Wang; +Cc: QEMU Developers

On Mon, 22 Mar 2021 at 10:08, Jason Wang <jasowang@redhat.com> wrote:
>
> The following changes since commit bdee969c0e65d4d509932b1d70e3a3b2ffbff6d5:
>
>   Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' into staging (2021-03-19 18:01:17 +0000)
>
> are available in the git repository at:
>
>   https://github.com/jasowang/qemu.git tags/net-pull-request
>
> for you to fetch changes up to c7274b5ef43614dd133daec1e2018f71d8744088:
>
>   net/eth: Add an assert() and invert if() statement to simplify code (2021-03-22 17:34:31 +0800)
>
> ----------------------------------------------------------------
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0
for any user-visible changes.

-- PMM


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

* Re: [PULL 00/13] Net patches
  2022-01-12  7:51           ` Jason Wang
@ 2022-01-12 13:16             ` Vladislav Yaroshchuk
  0 siblings, 0 replies; 30+ messages in thread
From: Vladislav Yaroshchuk @ 2022-01-12 13:16 UTC (permalink / raw)
  To: Jason Wang; +Cc: Roman Bolshakov, qemu-devel, Peter Maydell

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

ср, 12 янв. 2022 г. в 10:51, Jason Wang <jasowang@redhat.com>:

> On Wed, Jan 12, 2022 at 3:10 PM Roman Bolshakov <roman@roolebo.dev> wrote:
> >
> > On Wed, Jan 12, 2022 at 01:39:28PM +0800, Jason Wang wrote:
> > >
> > > 在 2022/1/12 上午6:02, Vladislav Yaroshchuk 写道:
> > > >
> > > >
> > > > вт, 11 янв. 2022 г., 5:10 AM Jason Wang <jasowang@redhat.com>:
> > > >
> > > >     On Tue, Jan 11, 2022 at 12:49 AM Peter Maydell
> > > >     <peter.maydell@linaro.org> wrote:
> > > >     >
> > > >     > On Mon, 10 Jan 2022 at 03:40, Jason Wang <jasowang@redhat.com>
> > > >     wrote:
> > > >     > >
> > > >     > > The following changes since commit
> > > >     df722e33d5da26ea8604500ca8f509245a0ea524:
> > > >     > >
> > > >     > >   Merge tag 'bsd-user-arm-pull-request' of
> > > >     gitlab.com:bsdimp/qemu into staging (2022-01-08 09:37:59 -0800)
> > > >     > >
> > > >     > > are available in the git repository at:
> > > >     > >
> > > >     > > https://github.com/jasowang/qemu.git tags/net-pull-request
> > > >     > >
> > > >     > > for you to fetch changes up to
> > > >     5136cc6d3b8b74f4fa572f0874656947a401330e:
> > > >     > >
> > > >     > >   net/vmnet: update MAINTAINERS list (2022-01-10 11:30:55
> +0800)
> > > >     > >
> > > >     > >
> ----------------------------------------------------------------
> > > >     > >
> > > >     > >
> ----------------------------------------------------------------
> > > >     >
> > > >     > Fails to build on OSX Catalina:
> > > >     >
> > > >     > ../../net/vmnet-common.m:165:10: error: use of undeclared
> identifier
> > > >     > 'VMNET_SHARING_SERVICE_BUSY'
> > > >     >     case VMNET_SHARING_SERVICE_BUSY:
> > > >     >          ^
> > > >     >
> > > >     > This constant only got added in macOS 11.0. I guess that
> technically
> > > >     > our supported-platforms policy only requires us to support 11
> > > >     (Big Sur)
> > > >     > and 12 (Monterey) at this point, but it would be nice to still
> > > >     be able
> > > >     > to build on Catalina (10.15).
> > > >
> > > >     Yes, it was only supported by the vmnet framework starting from
> > > >     Catalyst according to
> > > >     https://developer.apple.com/documentation/vmnet?language=objc.
> > > >
> > > >
> > > > Yes, there are some symbols from macOS >= 11.0 new backend
> > > > uses, not only this one, ex. vmnet_enable_isolation_key:
> > > >
> https://developer.apple.com/documentation/vmnet/vmnet_enable_isolation_key
> > > >
> > > >     >
> > > >     > (Personally I would like Catalina still to work at least for a
> > > >     little
> > > >     > while, because my x86 Mac is old enough that it is not
> supported by
> > > >     > Big Sur. I'll have to dump it once Apple stops doing security
> > > >     support
> > > >     > for Catalina, but they haven't done that quite yet.)
> > > >
> > > >
> > > > Sure, broken builds on old macOSes are bad. For this case I think
> > > > it's enough to disable vmnet for macOS < 11.0 with a probe while
> > > > configure build step. Especially given that Apple supports ~three
> > > > latest macOS versions, support for Catalina is expected to end
> > > > in 2022, when QEMU releases 7.0.
> > >
> > >
> > > That should be fine.
> > >
> >
> > I agree with Peter on this,
> >
> > There's a lot of hardware running with Catalina. I think it's useful to
> > support it a little longer.
>
> Right and Vladislav have disabled vmnet on the old versions.
>
> Thanks
>
>
Roman requested vmnet support for Catalina (10.15) also:
https://patchew.org/QEMU/20220111211422.21789-1-yaroshchuk2000@gmail.com/

After some thought I also found it important to support 10.15.
I've found a free hour to update patches and submit as v11
(still not displayed on Patchew for some reason).

Tested on Catalina 10.15 and Big Sur 11.5.
Built with no errors under Ubuntu 20.04 5.4.0-94-generic.

>
> > Regards,
> > Roman
> >
> > >
> > > >
> > > > If this workaround is not suitable and it's required to support vmnet
> > > > in Catalina 10.15 with a subset of available features, it can be
> done.
> > > > But I'll be ready to handle this in approximately two-three weeks
> only.
> > > >
> > > >     Sure, Vladislav please fix this and send a new version.
> > > >
> > > >
> > > > Quick fix as described above is available in v10:
> > > >
> https://patchew.org/QEMU/20220111211422.21789-1-yaroshchuk2000@gmail.com/
> > >
> > >
> > > Have you got chance to test that for macOS < 11.0?
> > >
> > > Thanks
> > >
> > >
> > > >     Thanks
> > > >
> > > >     >
> > > >     > -- PMM
> > > >     >
> > > >
> > > >
> > > >
> > > >
> > > > --
> > > > Best Regards,
> > > >
> > > > Vladislav Yaroshchuk
> > >
> > >
> >
>
>

-- 
Best Regards,

Vladislav Yaroshchuk

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

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

* Re: [PULL 00/13] Net patches
  2022-01-12  7:10         ` Roman Bolshakov
@ 2022-01-12  7:51           ` Jason Wang
  2022-01-12 13:16             ` Vladislav Yaroshchuk
  0 siblings, 1 reply; 30+ messages in thread
From: Jason Wang @ 2022-01-12  7:51 UTC (permalink / raw)
  To: Roman Bolshakov; +Cc: Peter Maydell, qemu-devel, Vladislav Yaroshchuk

On Wed, Jan 12, 2022 at 3:10 PM Roman Bolshakov <roman@roolebo.dev> wrote:
>
> On Wed, Jan 12, 2022 at 01:39:28PM +0800, Jason Wang wrote:
> >
> > 在 2022/1/12 上午6:02, Vladislav Yaroshchuk 写道:
> > >
> > >
> > > вт, 11 янв. 2022 г., 5:10 AM Jason Wang <jasowang@redhat.com>:
> > >
> > >     On Tue, Jan 11, 2022 at 12:49 AM Peter Maydell
> > >     <peter.maydell@linaro.org> wrote:
> > >     >
> > >     > On Mon, 10 Jan 2022 at 03:40, Jason Wang <jasowang@redhat.com>
> > >     wrote:
> > >     > >
> > >     > > The following changes since commit
> > >     df722e33d5da26ea8604500ca8f509245a0ea524:
> > >     > >
> > >     > >   Merge tag 'bsd-user-arm-pull-request' of
> > >     gitlab.com:bsdimp/qemu into staging (2022-01-08 09:37:59 -0800)
> > >     > >
> > >     > > are available in the git repository at:
> > >     > >
> > >     > > https://github.com/jasowang/qemu.git tags/net-pull-request
> > >     > >
> > >     > > for you to fetch changes up to
> > >     5136cc6d3b8b74f4fa572f0874656947a401330e:
> > >     > >
> > >     > >   net/vmnet: update MAINTAINERS list (2022-01-10 11:30:55 +0800)
> > >     > >
> > >     > > ----------------------------------------------------------------
> > >     > >
> > >     > > ----------------------------------------------------------------
> > >     >
> > >     > Fails to build on OSX Catalina:
> > >     >
> > >     > ../../net/vmnet-common.m:165:10: error: use of undeclared identifier
> > >     > 'VMNET_SHARING_SERVICE_BUSY'
> > >     >     case VMNET_SHARING_SERVICE_BUSY:
> > >     >          ^
> > >     >
> > >     > This constant only got added in macOS 11.0. I guess that technically
> > >     > our supported-platforms policy only requires us to support 11
> > >     (Big Sur)
> > >     > and 12 (Monterey) at this point, but it would be nice to still
> > >     be able
> > >     > to build on Catalina (10.15).
> > >
> > >     Yes, it was only supported by the vmnet framework starting from
> > >     Catalyst according to
> > >     https://developer.apple.com/documentation/vmnet?language=objc.
> > >
> > >
> > > Yes, there are some symbols from macOS >= 11.0 new backend
> > > uses, not only this one, ex. vmnet_enable_isolation_key:
> > > https://developer.apple.com/documentation/vmnet/vmnet_enable_isolation_key
> > >
> > >     >
> > >     > (Personally I would like Catalina still to work at least for a
> > >     little
> > >     > while, because my x86 Mac is old enough that it is not supported by
> > >     > Big Sur. I'll have to dump it once Apple stops doing security
> > >     support
> > >     > for Catalina, but they haven't done that quite yet.)
> > >
> > >
> > > Sure, broken builds on old macOSes are bad. For this case I think
> > > it's enough to disable vmnet for macOS < 11.0 with a probe while
> > > configure build step. Especially given that Apple supports ~three
> > > latest macOS versions, support for Catalina is expected to end
> > > in 2022, when QEMU releases 7.0.
> >
> >
> > That should be fine.
> >
>
> I agree with Peter on this,
>
> There's a lot of hardware running with Catalina. I think it's useful to
> support it a little longer.

Right and Vladislav have disabled vmnet on the old versions.

Thanks

>
> Regards,
> Roman
>
> >
> > >
> > > If this workaround is not suitable and it's required to support vmnet
> > > in Catalina 10.15 with a subset of available features, it can be done.
> > > But I'll be ready to handle this in approximately two-three weeks only.
> > >
> > >     Sure, Vladislav please fix this and send a new version.
> > >
> > >
> > > Quick fix as described above is available in v10:
> > > https://patchew.org/QEMU/20220111211422.21789-1-yaroshchuk2000@gmail.com/
> >
> >
> > Have you got chance to test that for macOS < 11.0?
> >
> > Thanks
> >
> >
> > >     Thanks
> > >
> > >     >
> > >     > -- PMM
> > >     >
> > >
> > >
> > >
> > >
> > > --
> > > Best Regards,
> > >
> > > Vladislav Yaroshchuk
> >
> >
>



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

* Re: [PULL 00/13] Net patches
  2022-01-12  6:19         ` Vladislav Yaroshchuk
@ 2022-01-12  7:49           ` Jason Wang
  0 siblings, 0 replies; 30+ messages in thread
From: Jason Wang @ 2022-01-12  7:49 UTC (permalink / raw)
  To: Vladislav Yaroshchuk; +Cc: Peter Maydell, qemu-devel

On Wed, Jan 12, 2022 at 2:19 PM Vladislav Yaroshchuk
<yaroshchuk2000@gmail.com> wrote:
>
>
>
> ср, 12 янв. 2022 г., 8:39 AM Jason Wang <jasowang@redhat.com>:
>>
>>
>> 在 2022/1/12 上午6:02, Vladislav Yaroshchuk 写道:
>> >
>> >
>> > вт, 11 янв. 2022 г., 5:10 AM Jason Wang <jasowang@redhat.com>:
>> >
>> >     On Tue, Jan 11, 2022 at 12:49 AM Peter Maydell
>> >     <peter.maydell@linaro.org> wrote:
>> >     >
>> >     > On Mon, 10 Jan 2022 at 03:40, Jason Wang <jasowang@redhat.com>
>> >     wrote:
>> >     > >
>> >     > > The following changes since commit
>> >     df722e33d5da26ea8604500ca8f509245a0ea524:
>> >     > >
>> >     > >   Merge tag 'bsd-user-arm-pull-request' of
>> >     gitlab.com:bsdimp/qemu into staging (2022-01-08 09:37:59 -0800)
>> >     > >
>> >     > > are available in the git repository at:
>> >     > >
>> >     > > https://github.com/jasowang/qemu.git tags/net-pull-request
>> >     > >
>> >     > > for you to fetch changes up to
>> >     5136cc6d3b8b74f4fa572f0874656947a401330e:
>> >     > >
>> >     > >   net/vmnet: update MAINTAINERS list (2022-01-10 11:30:55 +0800)
>> >     > >
>> >     > > ----------------------------------------------------------------
>> >     > >
>> >     > > ----------------------------------------------------------------
>> >     >
>> >     > Fails to build on OSX Catalina:
>> >     >
>> >     > ../../net/vmnet-common.m:165:10: error: use of undeclared identifier
>> >     > 'VMNET_SHARING_SERVICE_BUSY'
>> >     >     case VMNET_SHARING_SERVICE_BUSY:
>> >     >          ^
>> >     >
>> >     > This constant only got added in macOS 11.0. I guess that technically
>> >     > our supported-platforms policy only requires us to support 11
>> >     (Big Sur)
>> >     > and 12 (Monterey) at this point, but it would be nice to still
>> >     be able
>> >     > to build on Catalina (10.15).
>> >
>> >     Yes, it was only supported by the vmnet framework starting from
>> >     Catalyst according to
>> >     https://developer.apple.com/documentation/vmnet?language=objc.
>> >
>> >
>> > Yes, there are some symbols from macOS >= 11.0 new backend
>> > uses, not only this one, ex. vmnet_enable_isolation_key:
>> > https://developer.apple.com/documentation/vmnet/vmnet_enable_isolation_key
>> >
>> >     >
>> >     > (Personally I would like Catalina still to work at least for a
>> >     little
>> >     > while, because my x86 Mac is old enough that it is not supported by
>> >     > Big Sur. I'll have to dump it once Apple stops doing security
>> >     support
>> >     > for Catalina, but they haven't done that quite yet.)
>> >
>> >
>> > Sure, broken builds on old macOSes are bad. For this case I think
>> > it's enough to disable vmnet for macOS < 11.0 with a probe while
>> > configure build step. Especially given that Apple supports ~three
>> > latest macOS versions, support for Catalina is expected to end
>> > in 2022, when QEMU releases 7.0.
>>
>>
>> That should be fine.
>>
>>
>> >
>> > If this workaround is not suitable and it's required to support vmnet
>> > in Catalina 10.15 with a subset of available features, it can be done.
>> > But I'll be ready to handle this in approximately two-three weeks only.
>> >
>> >     Sure, Vladislav please fix this and send a new version.
>> >
>> >
>> > Quick fix as described above is available in v10:
>> > https://patchew.org/QEMU/20220111211422.21789-1-yaroshchuk2000@gmail.com/
>>
>>
>> Have you got chance to test that for macOS < 11.0?
>
>
> Yes, tested on Catalina 10.15.Works as expected.

Cool.

Thanks

>
>> Thanks
>>
>>
>> >     Thanks
>> >
>> >     >
>> >     > -- PMM
>> >     >
>> >
>> >
>> >
>> >
>> > --
>> > Best Regards,
>> >
>> > Vladislav Yaroshchuk
>>
>>
>>



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

* Re: [PULL 00/13] Net patches
  2022-01-12  5:39       ` Jason Wang
  2022-01-12  6:19         ` Vladislav Yaroshchuk
@ 2022-01-12  7:10         ` Roman Bolshakov
  2022-01-12  7:51           ` Jason Wang
  1 sibling, 1 reply; 30+ messages in thread
From: Roman Bolshakov @ 2022-01-12  7:10 UTC (permalink / raw)
  To: Jason Wang; +Cc: Peter Maydell, qemu-devel, Vladislav Yaroshchuk

On Wed, Jan 12, 2022 at 01:39:28PM +0800, Jason Wang wrote:
> 
> 在 2022/1/12 上午6:02, Vladislav Yaroshchuk 写道:
> > 
> > 
> > вт, 11 янв. 2022 г., 5:10 AM Jason Wang <jasowang@redhat.com>:
> > 
> >     On Tue, Jan 11, 2022 at 12:49 AM Peter Maydell
> >     <peter.maydell@linaro.org> wrote:
> >     >
> >     > On Mon, 10 Jan 2022 at 03:40, Jason Wang <jasowang@redhat.com>
> >     wrote:
> >     > >
> >     > > The following changes since commit
> >     df722e33d5da26ea8604500ca8f509245a0ea524:
> >     > >
> >     > >   Merge tag 'bsd-user-arm-pull-request' of
> >     gitlab.com:bsdimp/qemu into staging (2022-01-08 09:37:59 -0800)
> >     > >
> >     > > are available in the git repository at:
> >     > >
> >     > > https://github.com/jasowang/qemu.git tags/net-pull-request
> >     > >
> >     > > for you to fetch changes up to
> >     5136cc6d3b8b74f4fa572f0874656947a401330e:
> >     > >
> >     > >   net/vmnet: update MAINTAINERS list (2022-01-10 11:30:55 +0800)
> >     > >
> >     > > ----------------------------------------------------------------
> >     > >
> >     > > ----------------------------------------------------------------
> >     >
> >     > Fails to build on OSX Catalina:
> >     >
> >     > ../../net/vmnet-common.m:165:10: error: use of undeclared identifier
> >     > 'VMNET_SHARING_SERVICE_BUSY'
> >     >     case VMNET_SHARING_SERVICE_BUSY:
> >     >          ^
> >     >
> >     > This constant only got added in macOS 11.0. I guess that technically
> >     > our supported-platforms policy only requires us to support 11
> >     (Big Sur)
> >     > and 12 (Monterey) at this point, but it would be nice to still
> >     be able
> >     > to build on Catalina (10.15).
> > 
> >     Yes, it was only supported by the vmnet framework starting from
> >     Catalyst according to
> >     https://developer.apple.com/documentation/vmnet?language=objc.
> > 
> > 
> > Yes, there are some symbols from macOS >= 11.0 new backend
> > uses, not only this one, ex. vmnet_enable_isolation_key:
> > https://developer.apple.com/documentation/vmnet/vmnet_enable_isolation_key
> > 
> >     >
> >     > (Personally I would like Catalina still to work at least for a
> >     little
> >     > while, because my x86 Mac is old enough that it is not supported by
> >     > Big Sur. I'll have to dump it once Apple stops doing security
> >     support
> >     > for Catalina, but they haven't done that quite yet.)
> > 
> > 
> > Sure, broken builds on old macOSes are bad. For this case I think
> > it's enough to disable vmnet for macOS < 11.0 with a probe while
> > configure build step. Especially given that Apple supports ~three
> > latest macOS versions, support for Catalina is expected to end
> > in 2022, when QEMU releases 7.0.
> 
> 
> That should be fine.
> 

I agree with Peter on this,

There's a lot of hardware running with Catalina. I think it's useful to
support it a little longer.

Regards,
Roman

> 
> > 
> > If this workaround is not suitable and it's required to support vmnet
> > in Catalina 10.15 with a subset of available features, it can be done.
> > But I'll be ready to handle this in approximately two-three weeks only.
> > 
> >     Sure, Vladislav please fix this and send a new version.
> > 
> > 
> > Quick fix as described above is available in v10:
> > https://patchew.org/QEMU/20220111211422.21789-1-yaroshchuk2000@gmail.com/
> 
> 
> Have you got chance to test that for macOS < 11.0?
> 
> Thanks
> 
> 
> >     Thanks
> > 
> >     >
> >     > -- PMM
> >     >
> > 
> > 
> > 
> > 
> > -- 
> > Best Regards,
> > 
> > Vladislav Yaroshchuk
> 
> 


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

* Re: [PULL 00/13] Net patches
  2022-01-12  5:39       ` Jason Wang
@ 2022-01-12  6:19         ` Vladislav Yaroshchuk
  2022-01-12  7:49           ` Jason Wang
  2022-01-12  7:10         ` Roman Bolshakov
  1 sibling, 1 reply; 30+ messages in thread
From: Vladislav Yaroshchuk @ 2022-01-12  6:19 UTC (permalink / raw)
  To: Jason Wang; +Cc: Peter Maydell, qemu-devel

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

ср, 12 янв. 2022 г., 8:39 AM Jason Wang <jasowang@redhat.com>:

>
> 在 2022/1/12 上午6:02, Vladislav Yaroshchuk 写道:
> >
> >
> > вт, 11 янв. 2022 г., 5:10 AM Jason Wang <jasowang@redhat.com>:
> >
> >     On Tue, Jan 11, 2022 at 12:49 AM Peter Maydell
> >     <peter.maydell@linaro.org> wrote:
> >     >
> >     > On Mon, 10 Jan 2022 at 03:40, Jason Wang <jasowang@redhat.com>
> >     wrote:
> >     > >
> >     > > The following changes since commit
> >     df722e33d5da26ea8604500ca8f509245a0ea524:
> >     > >
> >     > >   Merge tag 'bsd-user-arm-pull-request' of
> >     gitlab.com:bsdimp/qemu into staging (2022-01-08 09:37:59 -0800)
> >     > >
> >     > > are available in the git repository at:
> >     > >
> >     > > https://github.com/jasowang/qemu.git tags/net-pull-request
> >     > >
> >     > > for you to fetch changes up to
> >     5136cc6d3b8b74f4fa572f0874656947a401330e:
> >     > >
> >     > >   net/vmnet: update MAINTAINERS list (2022-01-10 11:30:55 +0800)
> >     > >
> >     > > ----------------------------------------------------------------
> >     > >
> >     > > ----------------------------------------------------------------
> >     >
> >     > Fails to build on OSX Catalina:
> >     >
> >     > ../../net/vmnet-common.m:165:10: error: use of undeclared
> identifier
> >     > 'VMNET_SHARING_SERVICE_BUSY'
> >     >     case VMNET_SHARING_SERVICE_BUSY:
> >     >          ^
> >     >
> >     > This constant only got added in macOS 11.0. I guess that
> technically
> >     > our supported-platforms policy only requires us to support 11
> >     (Big Sur)
> >     > and 12 (Monterey) at this point, but it would be nice to still
> >     be able
> >     > to build on Catalina (10.15).
> >
> >     Yes, it was only supported by the vmnet framework starting from
> >     Catalyst according to
> >     https://developer.apple.com/documentation/vmnet?language=objc.
> >
> >
> > Yes, there are some symbols from macOS >= 11.0 new backend
> > uses, not only this one, ex. vmnet_enable_isolation_key:
> >
> https://developer.apple.com/documentation/vmnet/vmnet_enable_isolation_key
> >
> >     >
> >     > (Personally I would like Catalina still to work at least for a
> >     little
> >     > while, because my x86 Mac is old enough that it is not supported by
> >     > Big Sur. I'll have to dump it once Apple stops doing security
> >     support
> >     > for Catalina, but they haven't done that quite yet.)
> >
> >
> > Sure, broken builds on old macOSes are bad. For this case I think
> > it's enough to disable vmnet for macOS < 11.0 with a probe while
> > configure build step. Especially given that Apple supports ~three
> > latest macOS versions, support for Catalina is expected to end
> > in 2022, when QEMU releases 7.0.
>
>
> That should be fine.
>
>
> >
> > If this workaround is not suitable and it's required to support vmnet
> > in Catalina 10.15 with a subset of available features, it can be done.
> > But I'll be ready to handle this in approximately two-three weeks only.
> >
> >     Sure, Vladislav please fix this and send a new version.
> >
> >
> > Quick fix as described above is available in v10:
> >
> https://patchew.org/QEMU/20220111211422.21789-1-yaroshchuk2000@gmail.com/
>
>
> Have you got chance to test that for macOS < 11.0?
>

Yes, tested on Catalina 10.15.Works as expected.

Thanks
>
>
> >     Thanks
> >
> >     >
> >     > -- PMM
> >     >
> >
> >
> >
> >
> > --
> > Best Regards,
> >
> > Vladislav Yaroshchuk


>
>

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

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

* Re: [PULL 00/13] Net patches
  2022-01-11 22:02     ` Vladislav Yaroshchuk
@ 2022-01-12  5:39       ` Jason Wang
  2022-01-12  6:19         ` Vladislav Yaroshchuk
  2022-01-12  7:10         ` Roman Bolshakov
  0 siblings, 2 replies; 30+ messages in thread
From: Jason Wang @ 2022-01-12  5:39 UTC (permalink / raw)
  To: Vladislav Yaroshchuk; +Cc: Peter Maydell, qemu-devel


在 2022/1/12 上午6:02, Vladislav Yaroshchuk 写道:
>
>
> вт, 11 янв. 2022 г., 5:10 AM Jason Wang <jasowang@redhat.com>:
>
>     On Tue, Jan 11, 2022 at 12:49 AM Peter Maydell
>     <peter.maydell@linaro.org> wrote:
>     >
>     > On Mon, 10 Jan 2022 at 03:40, Jason Wang <jasowang@redhat.com>
>     wrote:
>     > >
>     > > The following changes since commit
>     df722e33d5da26ea8604500ca8f509245a0ea524:
>     > >
>     > >   Merge tag 'bsd-user-arm-pull-request' of
>     gitlab.com:bsdimp/qemu into staging (2022-01-08 09:37:59 -0800)
>     > >
>     > > are available in the git repository at:
>     > >
>     > > https://github.com/jasowang/qemu.git tags/net-pull-request
>     > >
>     > > for you to fetch changes up to
>     5136cc6d3b8b74f4fa572f0874656947a401330e:
>     > >
>     > >   net/vmnet: update MAINTAINERS list (2022-01-10 11:30:55 +0800)
>     > >
>     > > ----------------------------------------------------------------
>     > >
>     > > ----------------------------------------------------------------
>     >
>     > Fails to build on OSX Catalina:
>     >
>     > ../../net/vmnet-common.m:165:10: error: use of undeclared identifier
>     > 'VMNET_SHARING_SERVICE_BUSY'
>     >     case VMNET_SHARING_SERVICE_BUSY:
>     >          ^
>     >
>     > This constant only got added in macOS 11.0. I guess that technically
>     > our supported-platforms policy only requires us to support 11
>     (Big Sur)
>     > and 12 (Monterey) at this point, but it would be nice to still
>     be able
>     > to build on Catalina (10.15).
>
>     Yes, it was only supported by the vmnet framework starting from
>     Catalyst according to
>     https://developer.apple.com/documentation/vmnet?language=objc.
>
>
> Yes, there are some symbols from macOS >= 11.0 new backend
> uses, not only this one, ex. vmnet_enable_isolation_key:
> https://developer.apple.com/documentation/vmnet/vmnet_enable_isolation_key
>
>     >
>     > (Personally I would like Catalina still to work at least for a
>     little
>     > while, because my x86 Mac is old enough that it is not supported by
>     > Big Sur. I'll have to dump it once Apple stops doing security
>     support
>     > for Catalina, but they haven't done that quite yet.)
>
>
> Sure, broken builds on old macOSes are bad. For this case I think
> it's enough to disable vmnet for macOS < 11.0 with a probe while
> configure build step. Especially given that Apple supports ~three
> latest macOS versions, support for Catalina is expected to end
> in 2022, when QEMU releases 7.0.


That should be fine.


>
> If this workaround is not suitable and it's required to support vmnet
> in Catalina 10.15 with a subset of available features, it can be done.
> But I'll be ready to handle this in approximately two-three weeks only.
>
>     Sure, Vladislav please fix this and send a new version.
>
>
> Quick fix as described above is available in v10:
> https://patchew.org/QEMU/20220111211422.21789-1-yaroshchuk2000@gmail.com/


Have you got chance to test that for macOS < 11.0?

Thanks


>     Thanks
>
>     >
>     > -- PMM
>     >
>
>
>
>
> -- 
> Best Regards,
>
> Vladislav Yaroshchuk



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

* Re: [PULL 00/13] Net patches
  2022-01-11  2:09   ` Jason Wang
@ 2022-01-11 22:02     ` Vladislav Yaroshchuk
  2022-01-12  5:39       ` Jason Wang
  0 siblings, 1 reply; 30+ messages in thread
From: Vladislav Yaroshchuk @ 2022-01-11 22:02 UTC (permalink / raw)
  To: Jason Wang; +Cc: Peter Maydell, qemu-devel

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

вт, 11 янв. 2022 г., 5:10 AM Jason Wang <jasowang@redhat.com>:

> On Tue, Jan 11, 2022 at 12:49 AM Peter Maydell <peter.maydell@linaro.org>
> wrote:
> >
> > On Mon, 10 Jan 2022 at 03:40, Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > The following changes since commit
> df722e33d5da26ea8604500ca8f509245a0ea524:
> > >
> > >   Merge tag 'bsd-user-arm-pull-request' of gitlab.com:bsdimp/qemu
> into staging (2022-01-08 09:37:59 -0800)
> > >
> > > are available in the git repository at:
> > >
> > >   https://github.com/jasowang/qemu.git tags/net-pull-request
> > >
> > > for you to fetch changes up to
> 5136cc6d3b8b74f4fa572f0874656947a401330e:
> > >
> > >   net/vmnet: update MAINTAINERS list (2022-01-10 11:30:55 +0800)
> > >
> > > ----------------------------------------------------------------
> > >
> > > ----------------------------------------------------------------
> >
> > Fails to build on OSX Catalina:
> >
> > ../../net/vmnet-common.m:165:10: error: use of undeclared identifier
> > 'VMNET_SHARING_SERVICE_BUSY'
> >     case VMNET_SHARING_SERVICE_BUSY:
> >          ^
> >
> > This constant only got added in macOS 11.0. I guess that technically
> > our supported-platforms policy only requires us to support 11 (Big Sur)
> > and 12 (Monterey) at this point, but it would be nice to still be able
> > to build on Catalina (10.15).
>
Yes, it was only supported by the vmnet framework starting from
> Catalyst according to
> https://developer.apple.com/documentation/vmnet?language=objc.
>
>
Yes, there are some symbols from macOS >= 11.0 new backend
uses, not only this one, ex. vmnet_enable_isolation_key:
https://developer.apple.com/documentation/vmnet/vmnet_enable_isolation_key

>
> > (Personally I would like Catalina still to work at least for a little
> > while, because my x86 Mac is old enough that it is not supported by
> > Big Sur. I'll have to dump it once Apple stops doing security support
> > for Catalina, but they haven't done that quite yet.)
>
>
Sure, broken builds on old macOSes are bad. For this case I think
it's enough to disable vmnet for macOS < 11.0 with a probe while
configure build step. Especially given that Apple supports ~three
latest macOS versions, support for Catalina is expected to end
in 2022, when QEMU releases 7.0.

If this workaround is not suitable and it's required to support vmnet
in Catalina 10.15 with a subset of available features, it can be done.
But I'll be ready to handle this in approximately two-three weeks only.


> Sure, Vladislav please fix this and send a new version.
>
>
Quick fix as described above is available in v10:
https://patchew.org/QEMU/20220111211422.21789-1-yaroshchuk2000@gmail.com/

> Thanks
>
> >
> > -- PMM
> >
>
>
>

-- 
Best Regards,

Vladislav Yaroshchuk

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

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

* Re: [PULL 00/13] Net patches
  2022-01-10 16:49 ` Peter Maydell
@ 2022-01-11  2:09   ` Jason Wang
  2022-01-11 22:02     ` Vladislav Yaroshchuk
  0 siblings, 1 reply; 30+ messages in thread
From: Jason Wang @ 2022-01-11  2:09 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-devel, Vladislav Yaroshchuk

On Tue, Jan 11, 2022 at 12:49 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 10 Jan 2022 at 03:40, Jason Wang <jasowang@redhat.com> wrote:
> >
> > The following changes since commit df722e33d5da26ea8604500ca8f509245a0ea524:
> >
> >   Merge tag 'bsd-user-arm-pull-request' of gitlab.com:bsdimp/qemu into staging (2022-01-08 09:37:59 -0800)
> >
> > are available in the git repository at:
> >
> >   https://github.com/jasowang/qemu.git tags/net-pull-request
> >
> > for you to fetch changes up to 5136cc6d3b8b74f4fa572f0874656947a401330e:
> >
> >   net/vmnet: update MAINTAINERS list (2022-01-10 11:30:55 +0800)
> >
> > ----------------------------------------------------------------
> >
> > ----------------------------------------------------------------
>
> Fails to build on OSX Catalina:
>
> ../../net/vmnet-common.m:165:10: error: use of undeclared identifier
> 'VMNET_SHARING_SERVICE_BUSY'
>     case VMNET_SHARING_SERVICE_BUSY:
>          ^
>
> This constant only got added in macOS 11.0. I guess that technically
> our supported-platforms policy only requires us to support 11 (Big Sur)
> and 12 (Monterey) at this point, but it would be nice to still be able
> to build on Catalina (10.15).

Yes, it was only supported by the vmnet framework starting from
Catalyst according to
https://developer.apple.com/documentation/vmnet?language=objc.

>
> (Personally I would like Catalina still to work at least for a little
> while, because my x86 Mac is old enough that it is not supported by
> Big Sur. I'll have to dump it once Apple stops doing security support
> for Catalina, but they haven't done that quite yet.)

Sure, Vladislav please fix this and send a new version.

Thanks

>
> -- PMM
>



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

* Re: [PULL 00/13] Net patches
  2022-01-10  3:39 Jason Wang
@ 2022-01-10 16:49 ` Peter Maydell
  2022-01-11  2:09   ` Jason Wang
  0 siblings, 1 reply; 30+ messages in thread
From: Peter Maydell @ 2022-01-10 16:49 UTC (permalink / raw)
  To: Jason Wang; +Cc: qemu-devel

On Mon, 10 Jan 2022 at 03:40, Jason Wang <jasowang@redhat.com> wrote:
>
> The following changes since commit df722e33d5da26ea8604500ca8f509245a0ea524:
>
>   Merge tag 'bsd-user-arm-pull-request' of gitlab.com:bsdimp/qemu into staging (2022-01-08 09:37:59 -0800)
>
> are available in the git repository at:
>
>   https://github.com/jasowang/qemu.git tags/net-pull-request
>
> for you to fetch changes up to 5136cc6d3b8b74f4fa572f0874656947a401330e:
>
>   net/vmnet: update MAINTAINERS list (2022-01-10 11:30:55 +0800)
>
> ----------------------------------------------------------------
>
> ----------------------------------------------------------------

Fails to build on OSX Catalina:

../../net/vmnet-common.m:165:10: error: use of undeclared identifier
'VMNET_SHARING_SERVICE_BUSY'
    case VMNET_SHARING_SERVICE_BUSY:
         ^

This constant only got added in macOS 11.0. I guess that technically
our supported-platforms policy only requires us to support 11 (Big Sur)
and 12 (Monterey) at this point, but it would be nice to still be able
to build on Catalina (10.15).

(Personally I would like Catalina still to work at least for a little
while, because my x86 Mac is old enough that it is not supported by
Big Sur. I'll have to dump it once Apple stops doing security support
for Catalina, but they haven't done that quite yet.)

-- PMM


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

* [PULL 00/13] Net patches
@ 2022-01-10  3:39 Jason Wang
  2022-01-10 16:49 ` Peter Maydell
  0 siblings, 1 reply; 30+ messages in thread
From: Jason Wang @ 2022-01-10  3:39 UTC (permalink / raw)
  To: peter.maydell; +Cc: Jason Wang, qemu-devel

The following changes since commit df722e33d5da26ea8604500ca8f509245a0ea524:

  Merge tag 'bsd-user-arm-pull-request' of gitlab.com:bsdimp/qemu into staging (2022-01-08 09:37:59 -0800)

are available in the git repository at:

  https://github.com/jasowang/qemu.git tags/net-pull-request

for you to fetch changes up to 5136cc6d3b8b74f4fa572f0874656947a401330e:

  net/vmnet: update MAINTAINERS list (2022-01-10 11:30:55 +0800)

----------------------------------------------------------------

----------------------------------------------------------------
Peter Foley (2):
      net/tap: Set return code on failure
      net: Fix uninitialized data usage

Philippe Mathieu-Daudé (1):
      hw/net/vmxnet3: Log guest-triggerable errors using LOG_GUEST_ERROR

Rao Lei (1):
      net/filter: Optimize filter_send to coroutine

Vladislav Yaroshchuk (7):
      net/vmnet: add vmnet dependency and customizable option
      net/vmnet: add vmnet backends to qapi/net
      net/vmnet: implement shared mode (vmnet-shared)
      net/vmnet: implement host mode (vmnet-host)
      net/vmnet: implement bridged mode (vmnet-bridged)
      net/vmnet: update qemu-options.hx
      net/vmnet: update MAINTAINERS list

Zhang Chen (2):
      net/colo-compare.c: Optimize compare order for performance
      net/colo-compare.c: Update the default value comments

 MAINTAINERS                   |   5 +
 hw/net/vmxnet3.c              |   4 +-
 meson.build                   |   4 +
 meson_options.txt             |   2 +
 net/clients.h                 |  11 ++
 net/colo-compare.c            |  28 ++--
 net/filter-mirror.c           |  66 +++++++--
 net/meson.build               |   7 +
 net/net.c                     |  10 ++
 net/tap-linux.c               |   1 +
 net/tap.c                     |   1 +
 net/vmnet-bridged.m           | 111 ++++++++++++++
 net/vmnet-common.m            | 330 ++++++++++++++++++++++++++++++++++++++++++
 net/vmnet-host.c              | 105 ++++++++++++++
 net/vmnet-shared.c            |  91 ++++++++++++
 net/vmnet_int.h               |  48 ++++++
 qapi/net.json                 | 132 ++++++++++++++++-
 qemu-options.hx               |  25 ++++
 scripts/meson-buildoptions.sh |   3 +
 19 files changed, 954 insertions(+), 30 deletions(-)
 create mode 100644 net/vmnet-bridged.m
 create mode 100644 net/vmnet-common.m
 create mode 100644 net/vmnet-host.c
 create mode 100644 net/vmnet-shared.c
 create mode 100644 net/vmnet_int.h




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

* Re: [PULL 00/13] Net patches
  2020-03-27 11:36 ` Peter Maydell
@ 2020-03-30  9:47   ` Jason Wang
  0 siblings, 0 replies; 30+ messages in thread
From: Jason Wang @ 2020-03-30  9:47 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers


On 2020/3/27 下午7:36, Peter Maydell wrote:
> On Fri, 27 Mar 2020 at 11:14, Jason Wang <jasowang@redhat.com> wrote:
>> The following changes since commit cfe68ae025f704f336d7dd3d1903ce37b445831d:
>>
>>    Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-5.0-pull-request' into staging (2020-03-26 20:55:54 +0000)
>>
>> are available in the git repository at:
>>
>>    https://github.com/jasowang/qemu.git tags/net-pull-request
>>
>> for you to fetch changes up to f3b364f4f77fcb24cec468f518bf5e093dc27cb7:
>>
>>    hw/net/allwinner-sun8i-emac.c: Fix REG_ADDR_HIGH/LOW reads (2020-03-27 18:59:47 +0800)
>>
>> ----------------------------------------------------------------
>>
>> ----------------------------------------------------------------
> Hi; this fails to compile (all platforms):


My bad, forget to run full docker test before sending the pull request.


>
> /home/petmay01/qemu-for-merges/hw/net/allwinner-sun8i-emac.c:773:20:
> error: initialization from incompatible pointer type
> [-Werror=incompatible-pointer-types]
>       .can_receive = allwinner_sun8i_emac_can_receive,
>                      ^
> /home/petmay01/qemu-for-merges/hw/net/allwinner-sun8i-emac.c:773:20:
> note: (near initialization for
> 'net_allwinner_sun8i_emac_info.can_receive')
>
>
> There's also this one, though not every compiler picked it up:
>
> /home/peter.maydell/qemu/hw/net/i82596.c: In function 'i82596_receive':
> /home/peter.maydell/qemu/hw/net/i82596.c:657:30: error: comparison of
> unsigned expression >= 0 is always true [-Werror=type-limits]
>                   assert(bufsz >= 0);
>                                ^
> /home/peter.maydell/qemu/hw/net/i82596.c:657:30: error: comparison of
> unsigned expression >= 0 is always true [-Werror=type-limits]
>                   assert(bufsz >= 0);
>                                ^
>
>
> For the first error, I think this needs squashing into
> "hw/net: Make NetCanReceive() return a boolean":
>
> diff --git a/hw/net/allwinner-sun8i-emac.c b/hw/net/allwinner-sun8i-emac.c
> index fc67a1be70..28637ff4c1 100644
> --- a/hw/net/allwinner-sun8i-emac.c
> +++ b/hw/net/allwinner-sun8i-emac.c
> @@ -395,7 +395,7 @@ static void
> allwinner_sun8i_emac_flush_desc(FrameDescriptor *desc,
>       cpu_physical_memory_write(phys_addr, desc, sizeof(*desc));
>   }
>
> -static int allwinner_sun8i_emac_can_receive(NetClientState *nc)
> +static bool allwinner_sun8i_emac_can_receive(NetClientState *nc)
>   {
>       AwSun8iEmacState *s = qemu_get_nic_opaque(nc);
>       FrameDescriptor desc;
>
>
> Squashing this into my
> "hw/net/i82596.c: Avoid reading off end of buffer in i82596_receive()"
> commit fixes the second error.
>
> diff --git a/hw/net/i82596.c b/hw/net/i82596.c
> index a9bdbac339..055c3a1470 100644
> --- a/hw/net/i82596.c
> +++ b/hw/net/i82596.c
> @@ -653,8 +653,8 @@ ssize_t i82596_receive(NetClientState *nc, const
> uint8_t *buf, size_t sz)
>
>               if (bufcount > 0) {
>                   /* Still some of the actual data buffer to transfer */
> +                assert(bufsz >= bufcount);
>                   bufsz -= bufcount;
> -                assert(bufsz >= 0);
>                   address_space_write(&address_space_memory, rba,
>                                       MEMTXATTRS_UNSPECIFIED, buf, bufcount);
>                   rba += bufcount;
>
> thanks
> -- PMM
>

Right, will fix them.

Thanks




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

* Re: [PULL 00/13] Net patches
  2020-03-27 11:13 Jason Wang
  2020-03-27 11:36 ` Peter Maydell
  2020-03-27 12:03 ` no-reply
@ 2020-03-27 12:05 ` no-reply
  2 siblings, 0 replies; 30+ messages in thread
From: no-reply @ 2020-03-27 12:05 UTC (permalink / raw)
  To: jasowang; +Cc: peter.maydell, jasowang, qemu-devel

Patchew URL: https://patchew.org/QEMU/1585307647-24456-1-git-send-email-jasowang@redhat.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      hw/net/rocker/rocker_world.o
  CC      hw/net/rocker/rocker_of_dpa.o
  CC      hw/net/can/can_sja1000.o
/tmp/qemu-test/src/hw/net/allwinner-sun8i-emac.c:773:20: error: initialization of '_Bool (*)(NetClientState *)' {aka '_Bool (*)(struct NetClientState *)'} from incompatible pointer type 'int (*)(NetClientState *)' {aka 'int (*)(struct NetClientState *)'} [-Werror=incompatible-pointer-types]
     .can_receive = allwinner_sun8i_emac_can_receive,
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/qemu-test/src/hw/net/allwinner-sun8i-emac.c:773:20: note: (near initialization for 'net_allwinner_sun8i_emac_info.can_receive')
cc1: all warnings being treated as errors
make: *** [/tmp/qemu-test/src/rules.mak:69: hw/net/allwinner-sun8i-emac.o] Error 1
make: *** Waiting for unfinished jobs....
  CC      hw/net/can/can_kvaser_pci.o
Traceback (most recent call last):
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=5f98328ddc9546e3a22732b69d331039', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-25jm6435/src/docker-src.2020-03-27-08.03.37.24872:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=5f98328ddc9546e3a22732b69d331039
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-25jm6435/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    2m1.238s
user    0m8.179s


The full log is available at
http://patchew.org/logs/1585307647-24456-1-git-send-email-jasowang@redhat.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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

* Re: [PULL 00/13] Net patches
  2020-03-27 11:13 Jason Wang
  2020-03-27 11:36 ` Peter Maydell
@ 2020-03-27 12:03 ` no-reply
  2020-03-27 12:05 ` no-reply
  2 siblings, 0 replies; 30+ messages in thread
From: no-reply @ 2020-03-27 12:03 UTC (permalink / raw)
  To: jasowang; +Cc: peter.maydell, jasowang, qemu-devel

Patchew URL: https://patchew.org/QEMU/1585307647-24456-1-git-send-email-jasowang@redhat.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      hw/net/can/can_pcm3680_pci.o
  CC      hw/net/can/can_mioe3680_pci.o
  CC      hw/nvram/eeprom93xx.o
/tmp/qemu-test/src/hw/net/allwinner-sun8i-emac.c:773:5: error: initialization from incompatible pointer type [-Werror]
     .can_receive = allwinner_sun8i_emac_can_receive,
     ^
/tmp/qemu-test/src/hw/net/allwinner-sun8i-emac.c:773:5: error: (near initialization for 'net_allwinner_sun8i_emac_info.can_receive') [-Werror]
cc1: all warnings being treated as errors
make: *** [hw/net/allwinner-sun8i-emac.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=9de76a7b115b4ab8a94068ca9c2cad7f', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-tnhlej87/src/docker-src.2020-03-27-08.00.45.17095:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=9de76a7b115b4ab8a94068ca9c2cad7f
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-tnhlej87/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    2m20.630s
user    0m8.048s


The full log is available at
http://patchew.org/logs/1585307647-24456-1-git-send-email-jasowang@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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

* Re: [PULL 00/13] Net patches
  2020-03-27 11:13 Jason Wang
@ 2020-03-27 11:36 ` Peter Maydell
  2020-03-30  9:47   ` Jason Wang
  2020-03-27 12:03 ` no-reply
  2020-03-27 12:05 ` no-reply
  2 siblings, 1 reply; 30+ messages in thread
From: Peter Maydell @ 2020-03-27 11:36 UTC (permalink / raw)
  To: Jason Wang; +Cc: QEMU Developers

On Fri, 27 Mar 2020 at 11:14, Jason Wang <jasowang@redhat.com> wrote:
>
> The following changes since commit cfe68ae025f704f336d7dd3d1903ce37b445831d:
>
>   Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-5.0-pull-request' into staging (2020-03-26 20:55:54 +0000)
>
> are available in the git repository at:
>
>   https://github.com/jasowang/qemu.git tags/net-pull-request
>
> for you to fetch changes up to f3b364f4f77fcb24cec468f518bf5e093dc27cb7:
>
>   hw/net/allwinner-sun8i-emac.c: Fix REG_ADDR_HIGH/LOW reads (2020-03-27 18:59:47 +0800)
>
> ----------------------------------------------------------------
>
> ----------------------------------------------------------------

Hi; this fails to compile (all platforms):

/home/petmay01/qemu-for-merges/hw/net/allwinner-sun8i-emac.c:773:20:
error: initialization from incompatible pointer type
[-Werror=incompatible-pointer-types]
     .can_receive = allwinner_sun8i_emac_can_receive,
                    ^
/home/petmay01/qemu-for-merges/hw/net/allwinner-sun8i-emac.c:773:20:
note: (near initialization for
'net_allwinner_sun8i_emac_info.can_receive')


There's also this one, though not every compiler picked it up:

/home/peter.maydell/qemu/hw/net/i82596.c: In function 'i82596_receive':
/home/peter.maydell/qemu/hw/net/i82596.c:657:30: error: comparison of
unsigned expression >= 0 is always true [-Werror=type-limits]
                 assert(bufsz >= 0);
                              ^
/home/peter.maydell/qemu/hw/net/i82596.c:657:30: error: comparison of
unsigned expression >= 0 is always true [-Werror=type-limits]
                 assert(bufsz >= 0);
                              ^


For the first error, I think this needs squashing into
"hw/net: Make NetCanReceive() return a boolean":

diff --git a/hw/net/allwinner-sun8i-emac.c b/hw/net/allwinner-sun8i-emac.c
index fc67a1be70..28637ff4c1 100644
--- a/hw/net/allwinner-sun8i-emac.c
+++ b/hw/net/allwinner-sun8i-emac.c
@@ -395,7 +395,7 @@ static void
allwinner_sun8i_emac_flush_desc(FrameDescriptor *desc,
     cpu_physical_memory_write(phys_addr, desc, sizeof(*desc));
 }

-static int allwinner_sun8i_emac_can_receive(NetClientState *nc)
+static bool allwinner_sun8i_emac_can_receive(NetClientState *nc)
 {
     AwSun8iEmacState *s = qemu_get_nic_opaque(nc);
     FrameDescriptor desc;


Squashing this into my
"hw/net/i82596.c: Avoid reading off end of buffer in i82596_receive()"
commit fixes the second error.

diff --git a/hw/net/i82596.c b/hw/net/i82596.c
index a9bdbac339..055c3a1470 100644
--- a/hw/net/i82596.c
+++ b/hw/net/i82596.c
@@ -653,8 +653,8 @@ ssize_t i82596_receive(NetClientState *nc, const
uint8_t *buf, size_t sz)

             if (bufcount > 0) {
                 /* Still some of the actual data buffer to transfer */
+                assert(bufsz >= bufcount);
                 bufsz -= bufcount;
-                assert(bufsz >= 0);
                 address_space_write(&address_space_memory, rba,
                                     MEMTXATTRS_UNSPECIFIED, buf, bufcount);
                 rba += bufcount;

thanks
-- PMM


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

* [PULL 00/13] Net patches
@ 2020-03-27 11:13 Jason Wang
  2020-03-27 11:36 ` Peter Maydell
                   ` (2 more replies)
  0 siblings, 3 replies; 30+ messages in thread
From: Jason Wang @ 2020-03-27 11:13 UTC (permalink / raw)
  To: qemu-devel, peter.maydell; +Cc: Jason Wang

The following changes since commit cfe68ae025f704f336d7dd3d1903ce37b445831d:

  Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-5.0-pull-request' into staging (2020-03-26 20:55:54 +0000)

are available in the git repository at:

  https://github.com/jasowang/qemu.git tags/net-pull-request

for you to fetch changes up to f3b364f4f77fcb24cec468f518bf5e093dc27cb7:

  hw/net/allwinner-sun8i-emac.c: Fix REG_ADDR_HIGH/LOW reads (2020-03-27 18:59:47 +0800)

----------------------------------------------------------------

----------------------------------------------------------------
Andrew Melnychenko (1):
      Fixed integer overflow in e1000e

Peter Maydell (2):
      hw/net/i82596.c: Avoid reading off end of buffer in i82596_receive()
      hw/net/allwinner-sun8i-emac.c: Fix REG_ADDR_HIGH/LOW reads

Philippe Mathieu-Daudé (7):
      hw/net/i82596: Correct command bitmask (CID 1419392)
      hw/net/e1000e_core: Let e1000e_can_receive() return a boolean
      hw/net/smc91c111: Let smc91c111_can_receive() return a boolean
      hw/net/rtl8139: Simplify if/else statement
      hw/net/rtl8139: Update coding style to make checkpatch.pl happy
      hw/net: Make NetCanReceive() return a boolean
      hw/net/can: Make CanBusClientInfo::can_receive() return a boolean

Prasad J Pandit (1):
      net: tulip: check frame size and r/w data length

Zhang Chen (2):
      net/colo-compare.c: Expose "compare_timeout" to users
      net/colo-compare.c: Expose "expired_scan_cycle" to users

 hw/net/allwinner-sun8i-emac.c | 12 ++----
 hw/net/allwinner_emac.c       |  2 +-
 hw/net/cadence_gem.c          |  8 ++--
 hw/net/can/can_sja1000.c      |  8 ++--
 hw/net/can/can_sja1000.h      |  2 +-
 hw/net/dp8393x.c              |  8 ++--
 hw/net/e1000.c                |  2 +-
 hw/net/e1000e.c               |  4 +-
 hw/net/e1000e_core.c          |  2 +-
 hw/net/e1000e_core.h          |  2 +-
 hw/net/ftgmac100.c            |  6 +--
 hw/net/i82596.c               | 66 ++++++++++++++++++++----------
 hw/net/i82596.h               |  2 +-
 hw/net/imx_fec.c              |  2 +-
 hw/net/opencores_eth.c        |  5 +--
 hw/net/rtl8139.c              | 22 +++++-----
 hw/net/smc91c111.c            | 10 ++---
 hw/net/spapr_llan.c           |  4 +-
 hw/net/sungem.c               |  6 +--
 hw/net/sunhme.c               |  4 +-
 hw/net/tulip.c                | 36 ++++++++++++----
 hw/net/virtio-net.c           | 10 ++---
 hw/net/xilinx_ethlite.c       |  2 +-
 include/net/can_emu.h         |  2 +-
 include/net/net.h             |  2 +-
 net/can/can_socketcan.c       |  4 +-
 net/colo-compare.c            | 95 ++++++++++++++++++++++++++++++++++++++++---
 net/filter-buffer.c           |  2 +-
 net/hub.c                     |  6 +--
 qemu-options.hx               | 10 +++--
 30 files changed, 235 insertions(+), 111 deletions(-)




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

end of thread, other threads:[~2022-01-12 14:03 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-22 10:07 [PULL 00/13] Net patches Jason Wang
2021-03-22 10:08 ` [PULL 01/13] net: eth: Add a helper to pad a short Ethernet frame Jason Wang
2021-03-22 10:08 ` [PULL 02/13] net: Add a 'do_not_pad" to NetClientState Jason Wang
2021-03-22 10:08 ` [PULL 03/13] net: Pad short frames to minimum size before sending from SLiRP/TAP Jason Wang
2021-03-22 10:08 ` [PULL 04/13] hw/net: virtio-net: Initialize nc->do_not_pad to true Jason Wang
2021-03-22 10:08 ` [PULL 05/13] net/colo-compare.c: Fix memory leak for non-tcp packet Jason Wang
2021-03-22 10:08 ` [PULL 06/13] net/colo-compare.c: Optimize removal of secondary packet Jason Wang
2021-03-22 10:08 ` [PULL 07/13] net/eth: Use correct in6_address offset in _eth_get_rss_ex_dst_addr() Jason Wang
2021-03-22 10:08 ` [PULL 08/13] net/eth: Simplify _eth_get_rss_ex_dst_addr() Jason Wang
2021-03-22 10:08 ` [PULL 09/13] net/eth: Better describe _eth_get_rss_ex_dst_addr's offset argument Jason Wang
2021-03-22 10:08 ` [PULL 10/13] net/eth: Check size earlier in _eth_get_rss_ex_dst_addr() Jason Wang
2021-03-22 10:08 ` [PULL 11/13] net/eth: Check iovec has enough data earlier Jason Wang
2021-03-22 10:08 ` [PULL 12/13] net/eth: Read ip6_ext_hdr_routing buffer before accessing it Jason Wang
2021-03-22 10:08 ` [PULL 13/13] net/eth: Add an assert() and invert if() statement to simplify code Jason Wang
2021-03-22 14:13 ` [PULL 00/13] Net patches Peter Maydell
  -- strict thread matches above, loose matches on Subject: below --
2022-01-10  3:39 Jason Wang
2022-01-10 16:49 ` Peter Maydell
2022-01-11  2:09   ` Jason Wang
2022-01-11 22:02     ` Vladislav Yaroshchuk
2022-01-12  5:39       ` Jason Wang
2022-01-12  6:19         ` Vladislav Yaroshchuk
2022-01-12  7:49           ` Jason Wang
2022-01-12  7:10         ` Roman Bolshakov
2022-01-12  7:51           ` Jason Wang
2022-01-12 13:16             ` Vladislav Yaroshchuk
2020-03-27 11:13 Jason Wang
2020-03-27 11:36 ` Peter Maydell
2020-03-30  9:47   ` Jason Wang
2020-03-27 12:03 ` no-reply
2020-03-27 12:05 ` no-reply

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.