[PATCH v3 0/4] Guest announce feature emulation using Shadow VirtQueue

[PATCH v3 0/4] Guest announce feature emulation using Shadow VirtQueue

[Eugenio Pérez]

A gratuitous ARP is recommended after a live migration to reduce the amount of
time needed by the network links to be aware of the new location. A hypervisor
may not have the knowledge of the guest network configuration, and this is
especially true on passthrough devices, so its simpler to ask the guest to
do it.

However, the device control part of this feature can be totally emulated by
qemu and shadow virtqueue, not needing any special feature from the actual
vdpa device.

The vdpa device must offer VIRTIO_NET_F_STATUS for the guest to access the
status of virtio net config where announcement status bit is set. It is
possible to emulate it as always active in case backend does not support it,
but this is left for the future, as there are not many devices not offering it
anyway.

Patch 1 is less useful now that we don't emulate _F_STATUS anymore but QEMU
coding style seems to prefer early return so leaving it in this version.

v3:
* Properly byte swap status field of config space exposed to the guest.
* Fix comment (s/dont/don't)

v2:
* Actually remove VIRTIO_NET_F_STATUS emulation. Comparing with v1, we offer
  the feature with virtio instead of using virtio/vhost-vdpa.

v1:
* Move code from vhost_net_get_config to virtio_net_get_config.

RFC v2:
* Add VIRTIO_NET_F_STATUS emulation.

Eugenio Pérez (4):
  virtio_net: Modify virtio_net_get_config to early return
  virtio_net: copy VIRTIO_NET_S_ANNOUNCE if device model has it
  vdpa: handle VIRTIO_NET_CTRL_ANNOUNCE in
    vhost_vdpa_net_handle_ctrl_avail
  vdpa: do not handle VIRTIO_NET_F_GUEST_ANNOUNCE in vhost-vdpa

 hw/net/virtio-net.c | 30 +++++++++++++++++-------------
 net/vhost-vdpa.c    | 16 ++++++++++++----
 2 files changed, 29 insertions(+), 17 deletions(-)

-- 
2.31.1

[PATCH v3 1/4] virtio_net: Modify virtio_net_get_config to early return

[Eugenio Pérez]

Next patches introduce more code on vhost-vdpa branch, with already have
too much indentation.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/virtio-net.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 9cbdfa5547..b30038d130 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -168,20 +168,22 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
     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) {
-            /*
-             * Some NIC/kernel combinations present 0 as the mac address.  As
-             * that is not a legal address, try to proceed with the
-             * address from the QEMU command line in the hope that the
-             * address has been configured correctly elsewhere - just not
-             * reported by the device.
-             */
-            if (memcmp(&netcfg.mac, &zero, sizeof(zero)) == 0) {
-                info_report("Zero hardware mac address detected. Ignoring.");
-                memcpy(netcfg.mac, n->mac, ETH_ALEN);
-            }
-            memcpy(config, &netcfg, n->config_size);
+        if (ret == -1) {
+            return;
         }
+
+        /*
+         * Some NIC/kernel combinations present 0 as the mac address.  As that
+         * is not a legal address, try to proceed with the address from the
+         * QEMU command line in the hope that the address has been configured
+         * correctly elsewhere - just not reported by the device.
+         */
+        if (memcmp(&netcfg.mac, &zero, sizeof(zero)) == 0) {
+            info_report("Zero hardware mac address detected. Ignoring.");
+            memcpy(netcfg.mac, n->mac, ETH_ALEN);
+        }
+
+        memcpy(config, &netcfg, n->config_size);
     }
 }

--
2.31.1

[PATCH v3 2/4] virtio_net: copy VIRTIO_NET_S_ANNOUNCE if device model has it

[Eugenio Pérez]

Status part of the emulated feature. It will follow device model, so we
must copy it as long as NIC device model has it set.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
v3: Add virtio byte swapping writing net config status.
---
 hw/net/virtio-net.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index b30038d130..122eac25ee 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -183,6 +183,8 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
             memcpy(netcfg.mac, n->mac, ETH_ALEN);
         }
 
+        netcfg.status |= virtio_tswap16(vdev,
+                                        n->status & VIRTIO_NET_S_ANNOUNCE);
         memcpy(config, &netcfg, n->config_size);
     }
 }
-- 
2.31.1

Re: [PATCH v3 2/4] virtio_net: copy VIRTIO_NET_S_ANNOUNCE if device model has it

[Jason Wang]

On Wed, Dec 21, 2022 at 7:50 PM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> Status part of the emulated feature. It will follow device model, so we
> must copy it as long as NIC device model has it set.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

> ---
> v3: Add virtio byte swapping writing net config status.
> ---
>  hw/net/virtio-net.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index b30038d130..122eac25ee 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -183,6 +183,8 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
>              memcpy(netcfg.mac, n->mac, ETH_ALEN);
>          }
>
> +        netcfg.status |= virtio_tswap16(vdev,
> +                                        n->status & VIRTIO_NET_S_ANNOUNCE);
>          memcpy(config, &netcfg, n->config_size);
>      }
>  }
> --
> 2.31.1
>

[PATCH v3 3/4] vdpa: handle VIRTIO_NET_CTRL_ANNOUNCE in vhost_vdpa_net_handle_ctrl_avail

[Eugenio Pérez]

Since this capability is emulated by qemu shadowed CVQ cannot forward it
to the device. Process all that command within qemu.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 net/vhost-vdpa.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 260e474863..0b0712cd8a 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -489,9 +489,18 @@ static int vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
     out.iov_len = iov_to_buf(elem->out_sg, elem->out_num, 0,
                              s->cvq_cmd_out_buffer,
                              vhost_vdpa_net_cvq_cmd_len());
-    dev_written = vhost_vdpa_net_cvq_add(s, out.iov_len, sizeof(status));
-    if (unlikely(dev_written < 0)) {
-        goto out;
+    if (*(uint8_t *)s->cvq_cmd_out_buffer == VIRTIO_NET_CTRL_ANNOUNCE) {
+        /*
+         * Guest announce capability is emulated by qemu, so don't forward to
+         * the device.
+         */
+        dev_written = sizeof(status);
+        *s->status = VIRTIO_NET_OK;
+    } else {
+        dev_written = vhost_vdpa_net_cvq_add(s, out.iov_len, sizeof(status));
+        if (unlikely(dev_written < 0)) {
+            goto out;
+        }
     }
 
     if (unlikely(dev_written < sizeof(status))) {
-- 
2.31.1

Re: [PATCH v3 3/4] vdpa: handle VIRTIO_NET_CTRL_ANNOUNCE in vhost_vdpa_net_handle_ctrl_avail

[Jason Wang]

On Wed, Dec 21, 2022 at 7:50 PM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> Since this capability is emulated by qemu shadowed CVQ cannot forward it
> to the device. Process all that command within qemu.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

> ---
>  net/vhost-vdpa.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 260e474863..0b0712cd8a 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -489,9 +489,18 @@ static int vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
>      out.iov_len = iov_to_buf(elem->out_sg, elem->out_num, 0,
>                               s->cvq_cmd_out_buffer,
>                               vhost_vdpa_net_cvq_cmd_len());
> -    dev_written = vhost_vdpa_net_cvq_add(s, out.iov_len, sizeof(status));
> -    if (unlikely(dev_written < 0)) {
> -        goto out;
> +    if (*(uint8_t *)s->cvq_cmd_out_buffer == VIRTIO_NET_CTRL_ANNOUNCE) {
> +        /*
> +         * Guest announce capability is emulated by qemu, so don't forward to
> +         * the device.
> +         */
> +        dev_written = sizeof(status);
> +        *s->status = VIRTIO_NET_OK;
> +    } else {
> +        dev_written = vhost_vdpa_net_cvq_add(s, out.iov_len, sizeof(status));
> +        if (unlikely(dev_written < 0)) {
> +            goto out;
> +        }
>      }
>
>      if (unlikely(dev_written < sizeof(status))) {
> --
> 2.31.1
>

[PATCH v3 4/4] vdpa: do not handle VIRTIO_NET_F_GUEST_ANNOUNCE in vhost-vdpa

[Eugenio Pérez]

So qemu emulates it even in case the device does not support it.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
 net/vhost-vdpa.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 0b0712cd8a..6e71c74da2 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -72,7 +72,6 @@ const int vdpa_feature_bits[] = {
     VIRTIO_F_RING_RESET,
     VIRTIO_NET_F_RSS,
     VIRTIO_NET_F_HASH_REPORT,
-    VIRTIO_NET_F_GUEST_ANNOUNCE,
     VIRTIO_NET_F_STATUS,
     VHOST_INVALID_FEATURE_BIT
 };
-- 
2.31.1