[PULL 0/4] Net patches

[PULL 0/4] Net patches

[Jason Wang]

The following changes since commit 9303ecb658a0194560d1eecde165a1511223c2d8:

  Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20200727' into staging (2020-07-27 17:25:06 +0100)

are available in the git repository at:

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

for you to fetch changes up to 7142cad78d6bf4a1cbcb09d06b39935a7998c24e:

  net: forbid the reentrant RX (2020-07-28 13:50:41 +0800)

----------------------------------------------------------------
Want to send earlier but most patches just come.

- fix vhost-vdpa issues when no peer
- fix virtio-pci queue enabling check
- forbid reentrant RX

----------------------------------------------------------------
Jason Wang (2):
      virtio-net: check the existence of peer before accessing vDPA config
      net: forbid the reentrant RX

Laurent Vivier (1):
      virtio-pci: fix virtio_pci_queue_enabled()

Yuri Benditovich (1):
      virtio-pci: fix wrong index in virtio_pci_queue_enabled

 hw/net/virtio-net.c        | 30 +++++++++++++++++++-----------
 hw/virtio/virtio-pci.c     |  4 ++--
 hw/virtio/virtio.c         |  7 ++++++-
 include/hw/virtio/virtio.h |  1 +
 net/queue.c                |  3 +++
 5 files changed, 31 insertions(+), 14 deletions(-)

[PULL 1/4] virtio-pci: fix wrong index in virtio_pci_queue_enabled

[Jason Wang]

From: Yuri Benditovich <yuri.benditovich@daynix.com>

https://bugzilla.redhat.com/show_bug.cgi?id=1702608

Fixes: f19bcdfedd53 ("virtio-pci: implement queue_enabled method")
Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/virtio/virtio-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index ada1101..2b1f9cc 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1113,7 +1113,7 @@ static bool virtio_pci_queue_enabled(DeviceState *d, int n)
     VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
 
     if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
-        return proxy->vqs[vdev->queue_sel].enabled;
+        return proxy->vqs[n].enabled;
     }
 
     return virtio_queue_enabled(vdev, n);
-- 
2.7.4

[PULL 2/4] virtio-pci: fix virtio_pci_queue_enabled()

[Jason Wang]

From: Laurent Vivier <lvivier@redhat.com>

In legacy mode, virtio_pci_queue_enabled() falls back to
virtio_queue_enabled() to know if the queue is enabled.

But virtio_queue_enabled() calls again virtio_pci_queue_enabled()
if k->queue_enabled is set. This ends in a crash after a stack
overflow.

The problem can be reproduced with
"-device virtio-net-pci,disable-legacy=off,disable-modern=true
 -net tap,vhost=on"

And a look to the backtrace is very explicit:

    ...
    #4  0x000000010029a438 in virtio_queue_enabled ()
    #5  0x0000000100497a9c in virtio_pci_queue_enabled ()
    ...
    #130902 0x000000010029a460 in virtio_queue_enabled ()
    #130903 0x0000000100497a9c in virtio_pci_queue_enabled ()
    #130904 0x000000010029a460 in virtio_queue_enabled ()
    #130905 0x0000000100454a20 in vhost_net_start ()
    ...

This patch fixes the problem by introducing a new function
for the legacy case and calls it from virtio_pci_queue_enabled().
It also calls it from virtio_queue_enabled() to avoid code duplication.

Fixes: f19bcdfedd53 ("virtio-pci: implement queue_enabled method")
Cc: Jason Wang <jasowang@redhat.com>
Cc: Cindy Lu <lulu@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/virtio/virtio-pci.c     | 2 +-
 hw/virtio/virtio.c         | 7 ++++++-
 include/hw/virtio/virtio.h | 1 +
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 2b1f9cc..ccdf54e 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1116,7 +1116,7 @@ static bool virtio_pci_queue_enabled(DeviceState *d, int n)
         return proxy->vqs[n].enabled;
     }
 
-    return virtio_queue_enabled(vdev, n);
+    return virtio_queue_enabled_legacy(vdev, n);
 }
 
 static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 546a198..e983025 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3309,6 +3309,11 @@ hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n)
     return vdev->vq[n].vring.desc;
 }
 
+bool virtio_queue_enabled_legacy(VirtIODevice *vdev, int n)
+{
+    return virtio_queue_get_desc_addr(vdev, n) != 0;
+}
+
 bool virtio_queue_enabled(VirtIODevice *vdev, int n)
 {
     BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
@@ -3317,7 +3322,7 @@ bool virtio_queue_enabled(VirtIODevice *vdev, int n)
     if (k->queue_enabled) {
         return k->queue_enabled(qbus->parent, n);
     }
-    return virtio_queue_get_desc_addr(vdev, n) != 0;
+    return virtio_queue_enabled_legacy(vdev, n);
 }
 
 hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n)
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 198ffc7..e424df1 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -295,6 +295,7 @@ typedef struct VirtIORNGConf VirtIORNGConf;
                       VIRTIO_F_RING_PACKED, false)
 
 hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
+bool virtio_queue_enabled_legacy(VirtIODevice *vdev, int n);
 bool virtio_queue_enabled(VirtIODevice *vdev, int n);
 hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
 hwaddr virtio_queue_get_used_addr(VirtIODevice *vdev, int n);
-- 
2.7.4

[PULL 3/4] virtio-net: check the existence of peer before accessing vDPA config

[Jason Wang]

We try to check whether a peer is VDPA in order to get config from
there - with no peer, this leads to a NULL
pointer dereference. Add a check before trying to access the peer
type. No peer means not VDPA.

Fixes: 108a64818e69b ("vhost-vdpa: introduce vhost-vdpa backend")
Cc: Cindy Lu <lulu@redhat.com>
Tested-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/virtio-net.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 4895af1..a1fe9e9 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -125,6 +125,7 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
 {
     VirtIONet *n = VIRTIO_NET(vdev);
     struct virtio_net_config netcfg;
+    NetClientState *nc = qemu_get_queue(n->nic);
 
     int ret = 0;
     memset(&netcfg, 0 , sizeof(struct virtio_net_config));
@@ -142,13 +143,16 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
                  VIRTIO_NET_RSS_SUPPORTED_HASHES);
     memcpy(config, &netcfg, n->config_size);
 
-    NetClientState *nc = qemu_get_queue(n->nic);
-    if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
+    /*
+     * Is this VDPA? No peer means not VDPA: there's no way to
+     * disconnect/reconnect a VDPA peer.
+     */
+    if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
         ret = vhost_net_get_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
-                             n->config_size);
-    if (ret != -1) {
-        memcpy(config, &netcfg, n->config_size);
-    }
+                                   n->config_size);
+        if (ret != -1) {
+            memcpy(config, &netcfg, n->config_size);
+        }
     }
 }
 
@@ -156,6 +160,7 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
 {
     VirtIONet *n = VIRTIO_NET(vdev);
     struct virtio_net_config netcfg = {};
+    NetClientState *nc = qemu_get_queue(n->nic);
 
     memcpy(&netcfg, config, n->config_size);
 
@@ -166,11 +171,14 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
         qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
     }
 
-    NetClientState *nc = qemu_get_queue(n->nic);
-    if (nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
-        vhost_net_set_config(get_vhost_net(nc->peer), (uint8_t *)&netcfg,
-                               0, n->config_size,
-                        VHOST_SET_CONFIG_TYPE_MASTER);
+    /*
+     * Is this VDPA? No peer means not VDPA: there's no way to
+     * disconnect/reconnect a VDPA peer.
+     */
+    if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
+        vhost_net_set_config(get_vhost_net(nc->peer),
+                             (uint8_t *)&netcfg, 0, n->config_size,
+                             VHOST_SET_CONFIG_TYPE_MASTER);
       }
 }
 
-- 
2.7.4

[PULL 4/4] net: forbid the reentrant RX

[Jason Wang]

The memory API allows DMA into NIC's MMIO area. This means the NIC's
RX routine must be reentrant. Instead of auditing all the NIC, we can
simply detect the reentrancy and return early. The queue->delivering
is set and cleared by qemu_net_queue_deliver() for other queue helpers
to know whether the delivering in on going (NIC's receive is being
called). We can check it and return early in qemu_net_queue_flush() to
forbid reentrant RX.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/queue.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/queue.c b/net/queue.c
index 0164727..19e32c8 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -250,6 +250,9 @@ void qemu_net_queue_purge(NetQueue *queue, NetClientState *from)
 
 bool qemu_net_queue_flush(NetQueue *queue)
 {
+    if (queue->delivering)
+        return false;
+
     while (!QTAILQ_EMPTY(&queue->packets)) {
         NetPacket *packet;
         int ret;
-- 
2.7.4

Re: [PULL 0/4] Net patches

[Jason Wang]

On 2020/7/28 下午1:57, Jason Wang wrote:
> The following changes since commit 9303ecb658a0194560d1eecde165a1511223c2d8:
>
>    Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20200727' into staging (2020-07-27 17:25:06 +0100)
>
> are available in the git repository at:
>
>    https://github.com/jasowang/qemu.git tags/net-pull-request
>
> for you to fetch changes up to 7142cad78d6bf4a1cbcb09d06b39935a7998c24e:
>
>    net: forbid the reentrant RX (2020-07-28 13:50:41 +0800)
>
> ----------------------------------------------------------------
> Want to send earlier but most patches just come.
>
> - fix vhost-vdpa issues when no peer
> - fix virtio-pci queue enabling check
> - forbid reentrant RX
>
> ----------------------------------------------------------------
> Jason Wang (2):
>        virtio-net: check the existence of peer before accessing vDPA config
>        net: forbid the reentrant RX
>
> Laurent Vivier (1):
>        virtio-pci: fix virtio_pci_queue_enabled()


Just notice this has been merged.

Will send a V2.

Thanks


>
> Yuri Benditovich (1):
>        virtio-pci: fix wrong index in virtio_pci_queue_enabled
>
>   hw/net/virtio-net.c        | 30 +++++++++++++++++++-----------
>   hw/virtio/virtio-pci.c     |  4 ++--
>   hw/virtio/virtio.c         |  7 ++++++-
>   include/hw/virtio/virtio.h |  1 +
>   net/queue.c                |  3 +++
>   5 files changed, 31 insertions(+), 14 deletions(-)
>
>

Re: [PULL 0/4] Net patches

[Peter Maydell]

On Mon, 25 Nov 2019 at 15:40, Jason Wang <jasowang@redhat.com> wrote:
>
> The following changes since commit 122e6d2a9c1bf8aa1d51409c15809a82621515b1:
>
>   Merge remote-tracking branch 'remotes/gkurz/tags/9p-fix-2019-11-23' into staging (2019-11-25 13:39:45 +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 4d0e59ace29277b2faa5b33c719be9baaa659093:
>
>   net/virtio: return error when device_opts arg is NULL (2019-11-25 23:30:29 +0800)
>
> ----------------------------------------------------------------
>
> ----------------------------------------------------------------
> Jens Freimann (4):
>       net/virtio: fix dev_unplug_pending
>       net/virtio: return early when failover primary alread added
>       net/virtio: fix re-plugging of primary device
>       net/virtio: return error when device_opts arg is NULL
>
>  hw/net/virtio-net.c | 58 +++++++++++++++++++++++++++++++++++------------------
>  migration/savevm.c  |  3 ++-
>  2 files changed, 40 insertions(+), 21 deletions(-)


Applied, thanks.

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

-- PMM

[PULL 0/4] Net patches

[Jason Wang]

The following changes since commit 122e6d2a9c1bf8aa1d51409c15809a82621515b1:

  Merge remote-tracking branch 'remotes/gkurz/tags/9p-fix-2019-11-23' into staging (2019-11-25 13:39:45 +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 4d0e59ace29277b2faa5b33c719be9baaa659093:

  net/virtio: return error when device_opts arg is NULL (2019-11-25 23:30:29 +0800)

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

----------------------------------------------------------------
Jens Freimann (4):
      net/virtio: fix dev_unplug_pending
      net/virtio: return early when failover primary alread added
      net/virtio: fix re-plugging of primary device
      net/virtio: return error when device_opts arg is NULL

 hw/net/virtio-net.c | 58 +++++++++++++++++++++++++++++++++++------------------
 migration/savevm.c  |  3 ++-
 2 files changed, 40 insertions(+), 21 deletions(-)

2.5.0

Re: [PULL 0/4] Net patches

[Jason Wang]

On 2019/10/29 上午5:42, Peter Maydell wrote:
> On Mon, 28 Oct 2019 at 06:35, Jason Wang <jasowang@redhat.com> wrote:
>> The following changes since commit 187f35512106501fe9a11057f4d8705431e0026d:
>>
>>    Merge remote-tracking branch 'remotes/stsquad/tags/pull-testing-next-251019-3' into staging (2019-10-26 10:13:48 +0100)
>>
>> are available in the git repository at:
>>
>>    https://github.com/jasowang/qemu.git tags/net-pull-request
>>
>> for you to fetch changes up to 5af982a47cf6b6cd9beb872e5a9b940e43df5bf9:
>>
>>    COLO-compare: Fix incorrect `if` logic (2019-10-28 14:28:31 +0800)
>>
>> ----------------------------------------------------------------
>>
>> ----------------------------------------------------------------
> Hi -- I get this compile failure on some hosts (s390, aarch64,
> aarch32, ppc):
>
> /home/pm215/qemu/hw/net/tulip.c: In function ‘tulip_idblock_crc’:
> /home/pm215/qemu/hw/net/tulip.c:859:9: error: comparison is always
> true due to limited range of data type [-Werror=type-limits]
>           for (bit = 15; bit >= 0; bit--) {
>           ^
>
> 'bit' here is of type char, which can be either signed
> or unsigned depending on the host. If it's unsigned
> (as is the case on the above architectures), then the
> loop will never terminate because "bit >= 0" is always true.
> Plain old 'int' is probably a better choice anyway.


Yes, will fix and post V2.

Thanks


>
> thanks
> -- PMM
>

Re: [PULL 0/4] Net patches

[Peter Maydell]

On Mon, 28 Oct 2019 at 06:35, Jason Wang <jasowang@redhat.com> wrote:
>
> The following changes since commit 187f35512106501fe9a11057f4d8705431e0026d:
>
>   Merge remote-tracking branch 'remotes/stsquad/tags/pull-testing-next-251019-3' into staging (2019-10-26 10:13:48 +0100)
>
> are available in the git repository at:
>
>   https://github.com/jasowang/qemu.git tags/net-pull-request
>
> for you to fetch changes up to 5af982a47cf6b6cd9beb872e5a9b940e43df5bf9:
>
>   COLO-compare: Fix incorrect `if` logic (2019-10-28 14:28:31 +0800)
>
> ----------------------------------------------------------------
>
> ----------------------------------------------------------------

Hi -- I get this compile failure on some hosts (s390, aarch64,
aarch32, ppc):

/home/pm215/qemu/hw/net/tulip.c: In function ‘tulip_idblock_crc’:
/home/pm215/qemu/hw/net/tulip.c:859:9: error: comparison is always
true due to limited range of data type [-Werror=type-limits]
         for (bit = 15; bit >= 0; bit--) {
         ^

'bit' here is of type char, which can be either signed
or unsigned depending on the host. If it's unsigned
(as is the case on the above architectures), then the
loop will never terminate because "bit >= 0" is always true.
Plain old 'int' is probably a better choice anyway.

thanks
-- PMM

[PULL 0/4] Net patches

[Jason Wang]

The following changes since commit 187f35512106501fe9a11057f4d8705431e0026d:

  Merge remote-tracking branch 'remotes/stsquad/tags/pull-testing-next-251019-3' into staging (2019-10-26 10:13:48 +0100)

are available in the git repository at:

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

for you to fetch changes up to 5af982a47cf6b6cd9beb872e5a9b940e43df5bf9:

  COLO-compare: Fix incorrect `if` logic (2019-10-28 14:28:31 +0800)

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

----------------------------------------------------------------
Fan Yang (1):
      COLO-compare: Fix incorrect `if` logic

Michael S. Tsirkin (1):
      virtio: new post_load hook

Mikhail Sennikovsky (1):
      virtio-net: prevent offloads reset on migration

Sven Schnelle (1):
      net: add tulip (dec21143) driver

 MAINTAINERS                    |    6 +
 hw/net/Kconfig                 |    5 +
 hw/net/Makefile.objs           |    1 +
 hw/net/trace-events            |   14 +
 hw/net/tulip.c                 | 1029 ++++++++++++++++++++++++++++++++++++++++
 hw/net/tulip.h                 |  267 +++++++++++
 hw/net/virtio-net.c            |   27 +-
 hw/virtio/virtio.c             |    7 +
 include/hw/pci/pci_ids.h       |    1 +
 include/hw/virtio/virtio-net.h |    2 +
 include/hw/virtio/virtio.h     |    6 +
 net/colo-compare.c             |    6 +-
 12 files changed, 1365 insertions(+), 6 deletions(-)
 create mode 100644 hw/net/tulip.c
 create mode 100644 hw/net/tulip.h