[RFC PATCH v8 01/21] virtio-net: Expose ctrl virtqueue logic

Eugenio Pérez posted 21 patches 3 years, 8 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
[RFC PATCH v8 01/21] virtio-net: Expose ctrl virtqueue logic
Posted by Eugenio Pérez 3 years, 8 months ago
This allows external vhost-net devices to modify the state of the
VirtIO device model once vhost-vdpa device has acknowledge the control
commands.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 include/hw/virtio/virtio-net.h |  4 ++
 hw/net/virtio-net.c            | 84 ++++++++++++++++++++--------------
 2 files changed, 53 insertions(+), 35 deletions(-)

diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index eb87032627..cd31b7f67d 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -218,6 +218,10 @@ struct VirtIONet {
     struct EBPFRSSContext ebpf_rss;
 };
 
+unsigned virtio_net_handle_ctrl_iov(VirtIODevice *vdev,
+                                    const struct iovec *in_sg, size_t in_num,
+                                    const struct iovec *out_sg,
+                                    unsigned out_num);
 void virtio_net_set_netclient_name(VirtIONet *n, const char *name,
                                    const char *type);
 
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 7ad948ee7c..0e350154ec 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1434,57 +1434,71 @@ static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
     return VIRTIO_NET_OK;
 }
 
-static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
+unsigned virtio_net_handle_ctrl_iov(VirtIODevice *vdev,
+                                    const struct iovec *in_sg, size_t in_num,
+                                    const struct iovec *out_sg,
+                                    unsigned out_num)
 {
     VirtIONet *n = VIRTIO_NET(vdev);
     struct virtio_net_ctrl_hdr ctrl;
     virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
-    VirtQueueElement *elem;
     size_t s;
     struct iovec *iov, *iov2;
-    unsigned int iov_cnt;
+
+    if (iov_size(in_sg, in_num) < sizeof(status) ||
+        iov_size(out_sg, out_num) < sizeof(ctrl)) {
+        virtio_error(vdev, "virtio-net ctrl missing headers");
+        return 0;
+    }
+
+    iov2 = iov = g_memdup2(out_sg, sizeof(struct iovec) * out_num);
+    s = iov_to_buf(iov, out_num, 0, &ctrl, sizeof(ctrl));
+    iov_discard_front(&iov, &out_num, sizeof(ctrl));
+    if (s != sizeof(ctrl)) {
+        status = VIRTIO_NET_ERR;
+    } else if (ctrl.class == VIRTIO_NET_CTRL_RX) {
+        status = virtio_net_handle_rx_mode(n, ctrl.cmd, iov, out_num);
+    } else if (ctrl.class == VIRTIO_NET_CTRL_MAC) {
+        status = virtio_net_handle_mac(n, ctrl.cmd, iov, out_num);
+    } else if (ctrl.class == VIRTIO_NET_CTRL_VLAN) {
+        status = virtio_net_handle_vlan_table(n, ctrl.cmd, iov, out_num);
+    } else if (ctrl.class == VIRTIO_NET_CTRL_ANNOUNCE) {
+        status = virtio_net_handle_announce(n, ctrl.cmd, iov, out_num);
+    } else if (ctrl.class == VIRTIO_NET_CTRL_MQ) {
+        status = virtio_net_handle_mq(n, ctrl.cmd, iov, out_num);
+    } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
+        status = virtio_net_handle_offloads(n, ctrl.cmd, iov, out_num);
+    }
+
+    s = iov_from_buf(in_sg, in_num, 0, &status, sizeof(status));
+    assert(s == sizeof(status));
+
+    g_free(iov2);
+    return sizeof(status);
+}
+
+static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
+{
+    VirtQueueElement *elem;
 
     for (;;) {
+        unsigned written;
         elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
         if (!elem) {
             break;
         }
-        if (iov_size(elem->in_sg, elem->in_num) < sizeof(status) ||
-            iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
-            virtio_error(vdev, "virtio-net ctrl missing headers");
+
+        written = virtio_net_handle_ctrl_iov(vdev, elem->in_sg, elem->in_num,
+                                             elem->out_sg, elem->out_num);
+        if (written > 0) {
+            virtqueue_push(vq, elem, written);
+            virtio_notify(vdev, vq);
+            g_free(elem);
+        } else {
             virtqueue_detach_element(vq, elem, 0);
             g_free(elem);
             break;
         }
-
-        iov_cnt = elem->out_num;
-        iov2 = iov = g_memdup2(elem->out_sg,
-                               sizeof(struct iovec) * elem->out_num);
-        s = iov_to_buf(iov, iov_cnt, 0, &ctrl, sizeof(ctrl));
-        iov_discard_front(&iov, &iov_cnt, sizeof(ctrl));
-        if (s != sizeof(ctrl)) {
-            status = VIRTIO_NET_ERR;
-        } else if (ctrl.class == VIRTIO_NET_CTRL_RX) {
-            status = virtio_net_handle_rx_mode(n, ctrl.cmd, iov, iov_cnt);
-        } else if (ctrl.class == VIRTIO_NET_CTRL_MAC) {
-            status = virtio_net_handle_mac(n, ctrl.cmd, iov, iov_cnt);
-        } else if (ctrl.class == VIRTIO_NET_CTRL_VLAN) {
-            status = virtio_net_handle_vlan_table(n, ctrl.cmd, iov, iov_cnt);
-        } else if (ctrl.class == VIRTIO_NET_CTRL_ANNOUNCE) {
-            status = virtio_net_handle_announce(n, ctrl.cmd, iov, iov_cnt);
-        } else if (ctrl.class == VIRTIO_NET_CTRL_MQ) {
-            status = virtio_net_handle_mq(n, ctrl.cmd, iov, iov_cnt);
-        } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
-            status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
-        }
-
-        s = iov_from_buf(elem->in_sg, elem->in_num, 0, &status, sizeof(status));
-        assert(s == sizeof(status));
-
-        virtqueue_push(vq, elem, sizeof(status));
-        virtio_notify(vdev, vq);
-        g_free(iov2);
-        g_free(elem);
     }
 }
 
-- 
2.27.0


Re: [RFC PATCH v8 01/21] virtio-net: Expose ctrl virtqueue logic
Posted by Jason Wang 3 years, 8 months ago
在 2022/5/20 03:12, Eugenio Pérez 写道:
> This allows external vhost-net devices to modify the state of the
> VirtIO device model once vhost-vdpa device has acknowledge the control
> commands.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>   include/hw/virtio/virtio-net.h |  4 ++
>   hw/net/virtio-net.c            | 84 ++++++++++++++++++++--------------
>   2 files changed, 53 insertions(+), 35 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
> index eb87032627..cd31b7f67d 100644
> --- a/include/hw/virtio/virtio-net.h
> +++ b/include/hw/virtio/virtio-net.h
> @@ -218,6 +218,10 @@ struct VirtIONet {
>       struct EBPFRSSContext ebpf_rss;
>   };
>   
> +unsigned virtio_net_handle_ctrl_iov(VirtIODevice *vdev,
> +                                    const struct iovec *in_sg, size_t in_num,
> +                                    const struct iovec *out_sg,
> +                                    unsigned out_num);
>   void virtio_net_set_netclient_name(VirtIONet *n, const char *name,
>                                      const char *type);
>   
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 7ad948ee7c..0e350154ec 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1434,57 +1434,71 @@ static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
>       return VIRTIO_NET_OK;
>   }
>   
> -static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> +unsigned virtio_net_handle_ctrl_iov(VirtIODevice *vdev,


Should we use size_t here?

Thanks


> +                                    const struct iovec *in_sg, size_t in_num,
> +                                    const struct iovec *out_sg,
> +                                    unsigned out_num)
>   {
>       VirtIONet *n = VIRTIO_NET(vdev);
>       struct virtio_net_ctrl_hdr ctrl;
>       virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
> -    VirtQueueElement *elem;
>       size_t s;
>       struct iovec *iov, *iov2;
> -    unsigned int iov_cnt;
> +
> +    if (iov_size(in_sg, in_num) < sizeof(status) ||
> +        iov_size(out_sg, out_num) < sizeof(ctrl)) {
> +        virtio_error(vdev, "virtio-net ctrl missing headers");
> +        return 0;
> +    }
> +
> +    iov2 = iov = g_memdup2(out_sg, sizeof(struct iovec) * out_num);
> +    s = iov_to_buf(iov, out_num, 0, &ctrl, sizeof(ctrl));
> +    iov_discard_front(&iov, &out_num, sizeof(ctrl));
> +    if (s != sizeof(ctrl)) {
> +        status = VIRTIO_NET_ERR;
> +    } else if (ctrl.class == VIRTIO_NET_CTRL_RX) {
> +        status = virtio_net_handle_rx_mode(n, ctrl.cmd, iov, out_num);
> +    } else if (ctrl.class == VIRTIO_NET_CTRL_MAC) {
> +        status = virtio_net_handle_mac(n, ctrl.cmd, iov, out_num);
> +    } else if (ctrl.class == VIRTIO_NET_CTRL_VLAN) {
> +        status = virtio_net_handle_vlan_table(n, ctrl.cmd, iov, out_num);
> +    } else if (ctrl.class == VIRTIO_NET_CTRL_ANNOUNCE) {
> +        status = virtio_net_handle_announce(n, ctrl.cmd, iov, out_num);
> +    } else if (ctrl.class == VIRTIO_NET_CTRL_MQ) {
> +        status = virtio_net_handle_mq(n, ctrl.cmd, iov, out_num);
> +    } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
> +        status = virtio_net_handle_offloads(n, ctrl.cmd, iov, out_num);
> +    }
> +
> +    s = iov_from_buf(in_sg, in_num, 0, &status, sizeof(status));
> +    assert(s == sizeof(status));
> +
> +    g_free(iov2);
> +    return sizeof(status);
> +}
> +
> +static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> +{
> +    VirtQueueElement *elem;
>   
>       for (;;) {
> +        unsigned written;
>           elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
>           if (!elem) {
>               break;
>           }
> -        if (iov_size(elem->in_sg, elem->in_num) < sizeof(status) ||
> -            iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
> -            virtio_error(vdev, "virtio-net ctrl missing headers");
> +
> +        written = virtio_net_handle_ctrl_iov(vdev, elem->in_sg, elem->in_num,
> +                                             elem->out_sg, elem->out_num);
> +        if (written > 0) {
> +            virtqueue_push(vq, elem, written);
> +            virtio_notify(vdev, vq);
> +            g_free(elem);
> +        } else {
>               virtqueue_detach_element(vq, elem, 0);
>               g_free(elem);
>               break;
>           }
> -
> -        iov_cnt = elem->out_num;
> -        iov2 = iov = g_memdup2(elem->out_sg,
> -                               sizeof(struct iovec) * elem->out_num);
> -        s = iov_to_buf(iov, iov_cnt, 0, &ctrl, sizeof(ctrl));
> -        iov_discard_front(&iov, &iov_cnt, sizeof(ctrl));
> -        if (s != sizeof(ctrl)) {
> -            status = VIRTIO_NET_ERR;
> -        } else if (ctrl.class == VIRTIO_NET_CTRL_RX) {
> -            status = virtio_net_handle_rx_mode(n, ctrl.cmd, iov, iov_cnt);
> -        } else if (ctrl.class == VIRTIO_NET_CTRL_MAC) {
> -            status = virtio_net_handle_mac(n, ctrl.cmd, iov, iov_cnt);
> -        } else if (ctrl.class == VIRTIO_NET_CTRL_VLAN) {
> -            status = virtio_net_handle_vlan_table(n, ctrl.cmd, iov, iov_cnt);
> -        } else if (ctrl.class == VIRTIO_NET_CTRL_ANNOUNCE) {
> -            status = virtio_net_handle_announce(n, ctrl.cmd, iov, iov_cnt);
> -        } else if (ctrl.class == VIRTIO_NET_CTRL_MQ) {
> -            status = virtio_net_handle_mq(n, ctrl.cmd, iov, iov_cnt);
> -        } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
> -            status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
> -        }
> -
> -        s = iov_from_buf(elem->in_sg, elem->in_num, 0, &status, sizeof(status));
> -        assert(s == sizeof(status));
> -
> -        virtqueue_push(vq, elem, sizeof(status));
> -        virtio_notify(vdev, vq);
> -        g_free(iov2);
> -        g_free(elem);
>       }
>   }
>   


Re: [RFC PATCH v8 01/21] virtio-net: Expose ctrl virtqueue logic
Posted by Eugenio Perez Martin 3 years, 8 months ago
On Tue, Jun 7, 2022 at 8:13 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2022/5/20 03:12, Eugenio Pérez 写道:
> > This allows external vhost-net devices to modify the state of the
> > VirtIO device model once vhost-vdpa device has acknowledge the control
> > commands.
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> >   include/hw/virtio/virtio-net.h |  4 ++
> >   hw/net/virtio-net.c            | 84 ++++++++++++++++++++--------------
> >   2 files changed, 53 insertions(+), 35 deletions(-)
> >
> > diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
> > index eb87032627..cd31b7f67d 100644
> > --- a/include/hw/virtio/virtio-net.h
> > +++ b/include/hw/virtio/virtio-net.h
> > @@ -218,6 +218,10 @@ struct VirtIONet {
> >       struct EBPFRSSContext ebpf_rss;
> >   };
> >
> > +unsigned virtio_net_handle_ctrl_iov(VirtIODevice *vdev,
> > +                                    const struct iovec *in_sg, size_t in_num,
> > +                                    const struct iovec *out_sg,
> > +                                    unsigned out_num);
> >   void virtio_net_set_netclient_name(VirtIONet *n, const char *name,
> >                                      const char *type);
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index 7ad948ee7c..0e350154ec 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -1434,57 +1434,71 @@ static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
> >       return VIRTIO_NET_OK;
> >   }
> >
> > -static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> > +unsigned virtio_net_handle_ctrl_iov(VirtIODevice *vdev,
>
>
> Should we use size_t here?
>

I think it's a better type, yes. I used "unsigned" because
virtqueue_push uses unsigned for "len", maybe it's a good idea to
replace it there too.

Thanks!

> Thanks
>
>
> > +                                    const struct iovec *in_sg, size_t in_num,
> > +                                    const struct iovec *out_sg,
> > +                                    unsigned out_num)
> >   {
> >       VirtIONet *n = VIRTIO_NET(vdev);
> >       struct virtio_net_ctrl_hdr ctrl;
> >       virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
> > -    VirtQueueElement *elem;
> >       size_t s;
> >       struct iovec *iov, *iov2;
> > -    unsigned int iov_cnt;
> > +
> > +    if (iov_size(in_sg, in_num) < sizeof(status) ||
> > +        iov_size(out_sg, out_num) < sizeof(ctrl)) {
> > +        virtio_error(vdev, "virtio-net ctrl missing headers");
> > +        return 0;
> > +    }
> > +
> > +    iov2 = iov = g_memdup2(out_sg, sizeof(struct iovec) * out_num);
> > +    s = iov_to_buf(iov, out_num, 0, &ctrl, sizeof(ctrl));
> > +    iov_discard_front(&iov, &out_num, sizeof(ctrl));
> > +    if (s != sizeof(ctrl)) {
> > +        status = VIRTIO_NET_ERR;
> > +    } else if (ctrl.class == VIRTIO_NET_CTRL_RX) {
> > +        status = virtio_net_handle_rx_mode(n, ctrl.cmd, iov, out_num);
> > +    } else if (ctrl.class == VIRTIO_NET_CTRL_MAC) {
> > +        status = virtio_net_handle_mac(n, ctrl.cmd, iov, out_num);
> > +    } else if (ctrl.class == VIRTIO_NET_CTRL_VLAN) {
> > +        status = virtio_net_handle_vlan_table(n, ctrl.cmd, iov, out_num);
> > +    } else if (ctrl.class == VIRTIO_NET_CTRL_ANNOUNCE) {
> > +        status = virtio_net_handle_announce(n, ctrl.cmd, iov, out_num);
> > +    } else if (ctrl.class == VIRTIO_NET_CTRL_MQ) {
> > +        status = virtio_net_handle_mq(n, ctrl.cmd, iov, out_num);
> > +    } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
> > +        status = virtio_net_handle_offloads(n, ctrl.cmd, iov, out_num);
> > +    }
> > +
> > +    s = iov_from_buf(in_sg, in_num, 0, &status, sizeof(status));
> > +    assert(s == sizeof(status));
> > +
> > +    g_free(iov2);
> > +    return sizeof(status);
> > +}
> > +
> > +static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> > +{
> > +    VirtQueueElement *elem;
> >
> >       for (;;) {
> > +        unsigned written;
> >           elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> >           if (!elem) {
> >               break;
> >           }
> > -        if (iov_size(elem->in_sg, elem->in_num) < sizeof(status) ||
> > -            iov_size(elem->out_sg, elem->out_num) < sizeof(ctrl)) {
> > -            virtio_error(vdev, "virtio-net ctrl missing headers");
> > +
> > +        written = virtio_net_handle_ctrl_iov(vdev, elem->in_sg, elem->in_num,
> > +                                             elem->out_sg, elem->out_num);
> > +        if (written > 0) {
> > +            virtqueue_push(vq, elem, written);
> > +            virtio_notify(vdev, vq);
> > +            g_free(elem);
> > +        } else {
> >               virtqueue_detach_element(vq, elem, 0);
> >               g_free(elem);
> >               break;
> >           }
> > -
> > -        iov_cnt = elem->out_num;
> > -        iov2 = iov = g_memdup2(elem->out_sg,
> > -                               sizeof(struct iovec) * elem->out_num);
> > -        s = iov_to_buf(iov, iov_cnt, 0, &ctrl, sizeof(ctrl));
> > -        iov_discard_front(&iov, &iov_cnt, sizeof(ctrl));
> > -        if (s != sizeof(ctrl)) {
> > -            status = VIRTIO_NET_ERR;
> > -        } else if (ctrl.class == VIRTIO_NET_CTRL_RX) {
> > -            status = virtio_net_handle_rx_mode(n, ctrl.cmd, iov, iov_cnt);
> > -        } else if (ctrl.class == VIRTIO_NET_CTRL_MAC) {
> > -            status = virtio_net_handle_mac(n, ctrl.cmd, iov, iov_cnt);
> > -        } else if (ctrl.class == VIRTIO_NET_CTRL_VLAN) {
> > -            status = virtio_net_handle_vlan_table(n, ctrl.cmd, iov, iov_cnt);
> > -        } else if (ctrl.class == VIRTIO_NET_CTRL_ANNOUNCE) {
> > -            status = virtio_net_handle_announce(n, ctrl.cmd, iov, iov_cnt);
> > -        } else if (ctrl.class == VIRTIO_NET_CTRL_MQ) {
> > -            status = virtio_net_handle_mq(n, ctrl.cmd, iov, iov_cnt);
> > -        } else if (ctrl.class == VIRTIO_NET_CTRL_GUEST_OFFLOADS) {
> > -            status = virtio_net_handle_offloads(n, ctrl.cmd, iov, iov_cnt);
> > -        }
> > -
> > -        s = iov_from_buf(elem->in_sg, elem->in_num, 0, &status, sizeof(status));
> > -        assert(s == sizeof(status));
> > -
> > -        virtqueue_push(vq, elem, sizeof(status));
> > -        virtio_notify(vdev, vq);
> > -        g_free(iov2);
> > -        g_free(elem);
> >       }
> >   }
> >
>