[PULL 0/4] Net patches

[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

[PULL 1/4] net/virtio: fix dev_unplug_pending

[Jason Wang]

From: Jens Freimann <jfreimann@redhat.com>

.dev_unplug_pending is set up by virtio-net code indepent of failover
support was set for the device or not. This gives a wrong result when
we check for existing primary devices in migration code.

Fix this by actually calling dev_unplug_pending() instead of just
checking if the function pointer was set. When the feature was not
negotiated dev_unplug_pending() will always return false. This prevents
us from going into the wait-unplug state when there's no primary device
present.

Fixes: 9711cd0dfc3f ("net/virtio: add failover support")
Signed-off-by: Jens Freimann <jfreimann@redhat.com>
Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/virtio-net.c | 3 +++
 migration/savevm.c  | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 97a5113..946039c 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3124,6 +3124,9 @@ static bool primary_unplug_pending(void *opaque)
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIONet *n = VIRTIO_NET(vdev);
 
+    if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_STANDBY)) {
+        return false;
+    }
     return n->primary_dev ? n->primary_dev->pending_deleted_event : false;
 }
 
diff --git a/migration/savevm.c b/migration/savevm.c
index 966a9c3..a71b930 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1119,7 +1119,8 @@ int qemu_savevm_nr_failover_devices(void)
     int n = 0;
 
     QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
-        if (se->vmsd && se->vmsd->dev_unplug_pending) {
+        if (se->vmsd && se->vmsd->dev_unplug_pending &&
+            se->vmsd->dev_unplug_pending(se->opaque)) {
             n++;
         }
     }
-- 
2.5.0

[PULL 2/4] net/virtio: return early when failover primary alread added

[Jason Wang]

From: Jens Freimann <jfreimann@redhat.com>

Bail out when primary device was already added before.
This avoids printing a wrong warning message during reboot.

Fixes: 9711cd0dfc3f ("net/virtio: add failover support")
Signed-off-by: Jens Freimann <jfreimann@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.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 946039c..ac4d191 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -759,6 +759,10 @@ static void failover_add_primary(VirtIONet *n, Error **errp)
 {
     Error *err = NULL;
 
+    if (n->primary_dev) {
+        return;
+    }
+
     n->primary_device_opts = qemu_opts_find(qemu_find_opts("device"),
             n->primary_device_id);
     if (n->primary_device_opts) {
-- 
2.5.0

[PULL 3/4] net/virtio: fix re-plugging of primary device

[Jason Wang]

From: Jens Freimann <jfreimann@redhat.com>

failover_replug_primary was returning true on failure which lead to
re-plug not working when a migration failed.  Fix this by returning
success when hotplug worked.  This is a bug that was missed in last
round of testing but was tested succesfully with this version.  Also
make sure we don't pass NULL to qdev_set_parent_bus().

This fixes CID 1407224.

Fixes: 9711cd0dfc3f ("net/virtio: add failover support")
Signed-off-by: Jens Freimann <jfreimann@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/virtio-net.c | 42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index ac4d191..565dea0 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -2805,25 +2805,33 @@ static bool failover_replug_primary(VirtIONet *n, Error **errp)
         n->primary_device_opts = qemu_opts_from_qdict(
                 qemu_find_opts("device"),
                 n->primary_device_dict, errp);
-    }
-    if (n->primary_device_opts) {
-        if (n->primary_dev) {
-            n->primary_bus = n->primary_dev->parent_bus;
-        }
-        qdev_set_parent_bus(n->primary_dev, n->primary_bus);
-        n->primary_should_be_hidden = false;
-        qemu_opt_set_bool(n->primary_device_opts,
-                "partially_hotplugged", true, errp);
-        hotplug_ctrl = qdev_get_hotplug_handler(n->primary_dev);
-        if (hotplug_ctrl) {
-            hotplug_handler_pre_plug(hotplug_ctrl, n->primary_dev, errp);
-            hotplug_handler_plug(hotplug_ctrl, n->primary_dev, errp);
+        if (!n->primary_device_opts) {
+            error_setg(errp, "virtio_net: couldn't find primary device opts");
+            goto out;
         }
-        if (!n->primary_dev) {
+    }
+    if (!n->primary_dev) {
             error_setg(errp, "virtio_net: couldn't find primary device");
-        }
+            goto out;
     }
-    return *errp != NULL;
+
+    n->primary_bus = n->primary_dev->parent_bus;
+    if (!n->primary_bus) {
+        error_setg(errp, "virtio_net: couldn't find primary bus");
+        goto out;
+    }
+    qdev_set_parent_bus(n->primary_dev, n->primary_bus);
+    n->primary_should_be_hidden = false;
+    qemu_opt_set_bool(n->primary_device_opts,
+                      "partially_hotplugged", true, errp);
+    hotplug_ctrl = qdev_get_hotplug_handler(n->primary_dev);
+    if (hotplug_ctrl) {
+        hotplug_handler_pre_plug(hotplug_ctrl, n->primary_dev, errp);
+        hotplug_handler_plug(hotplug_ctrl, n->primary_dev, errp);
+    }
+
+out:
+    return *errp == NULL;
 }
 
 static void virtio_net_handle_migration_primary(VirtIONet *n,
@@ -2852,7 +2860,7 @@ static void virtio_net_handle_migration_primary(VirtIONet *n,
             warn_report("couldn't unplug primary device");
         }
     } else if (migration_has_failed(s)) {
-        /* We already unplugged the device let's plugged it back */
+        /* We already unplugged the device let's plug it back */
         if (!failover_replug_primary(n, &err)) {
             if (err) {
                 error_report_err(err);
-- 
2.5.0

[PULL 4/4] net/virtio: return error when device_opts arg is NULL

[Jason Wang]

From: Jens Freimann <jfreimann@redhat.com>

This fixes CID 1407222.

Fixes: 9711cd0dfc3f ("net/virtio: add failover support")
Signed-off-by: Jens Freimann <jfreimann@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/virtio-net.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 565dea0..3c31471 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -2880,9 +2880,12 @@ static int virtio_net_primary_should_be_hidden(DeviceListener *listener,
             QemuOpts *device_opts)
 {
     VirtIONet *n = container_of(listener, VirtIONet, primary_listener);
-    bool match_found;
-    bool hide;
+    bool match_found = false;
+    bool hide = false;
 
+    if (!device_opts) {
+        return -1;
+    }
     n->primary_device_dict = qemu_opts_to_qdict(device_opts,
             n->primary_device_dict);
     if (n->primary_device_dict) {
@@ -2890,7 +2893,7 @@ static int virtio_net_primary_should_be_hidden(DeviceListener *listener,
         n->standby_id = g_strdup(qdict_get_try_str(n->primary_device_dict,
                     "failover_pair_id"));
     }
-    if (device_opts && g_strcmp0(n->standby_id, n->netclient_name) == 0) {
+    if (g_strcmp0(n->standby_id, n->netclient_name) == 0) {
         match_found = true;
     } else {
         match_found = false;
-- 
2.5.0

Re: [PULL 4/4] net/virtio: return error when device_opts arg is NULL

[Paolo Bonzini]

On 25/11/19 16:40, Jason Wang wrote:
> From: Jens Freimann <jfreimann@redhat.com>
> 
> This fixes CID 1407222.
> 
> Fixes: 9711cd0dfc3f ("net/virtio: add failover support")
> Signed-off-by: Jens Freimann <jfreimann@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  hw/net/virtio-net.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 565dea0..3c31471 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -2880,9 +2880,12 @@ static int virtio_net_primary_should_be_hidden(DeviceListener *listener,
>              QemuOpts *device_opts)
>  {
>      VirtIONet *n = container_of(listener, VirtIONet, primary_listener);
> -    bool match_found;
> -    bool hide;
> +    bool match_found = false;
> +    bool hide = false;
>  
> +    if (!device_opts) {
> +        return -1;
> +    }
>      n->primary_device_dict = qemu_opts_to_qdict(device_opts,
>              n->primary_device_dict);
>      if (n->primary_device_dict) {
> @@ -2890,7 +2893,7 @@ static int virtio_net_primary_should_be_hidden(DeviceListener *listener,
>          n->standby_id = g_strdup(qdict_get_try_str(n->primary_device_dict,
>                      "failover_pair_id"));
>      }
> -    if (device_opts && g_strcmp0(n->standby_id, n->netclient_name) == 0) {
> +    if (g_strcmp0(n->standby_id, n->netclient_name) == 0) {
>          match_found = true;
>      } else {
>          match_found = false;
> 

It can't be NULL though, can it?  It is called from a qemu_foreach_opt
callback on device_opts itself.  This can be "re-fixed" in 5.0 though,
no hurry.

Paolo

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 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(-)

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(-)
>
>

[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

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

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
>