[PATCH v5 04/10] vdpa: Get buffers from VhostVDPAState on vhost_vdpa_net_cvq_map_elem

Eugenio Pérez posted 10 patches 3 years, 6 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>
There is a newer version of this series
[PATCH v5 04/10] vdpa: Get buffers from VhostVDPAState on vhost_vdpa_net_cvq_map_elem
Posted by Eugenio Pérez 3 years, 6 months ago
There is no need to get them by parameter, since they're contained in
VhostVDPAState. The only useful information was the written length in
out.

Simplify the function removing those.

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

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index ac1810723c..c6699edfbc 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -303,34 +303,29 @@ dma_map_err:
 
 /**
  * Copy the guest element into a dedicated buffer suitable to be sent to NIC
- *
- * @iov: [0] is the out buffer, [1] is the in one
  */
 static bool vhost_vdpa_net_cvq_map_elem(VhostVDPAState *s,
                                         VirtQueueElement *elem,
-                                        struct iovec *iov)
+                                        size_t *out_len)
 {
     size_t in_copied;
     bool ok;
 
-    iov[0].iov_base = s->cvq_cmd_out_buffer;
     ok = vhost_vdpa_cvq_map_buf(&s->vhost_vdpa, elem->out_sg, elem->out_num,
-                                vhost_vdpa_net_cvq_cmd_len(), iov[0].iov_base,
-                                &iov[0].iov_len, false);
+                                vhost_vdpa_net_cvq_cmd_len(),
+                                s->cvq_cmd_out_buffer, out_len, false);
     if (unlikely(!ok)) {
         return false;
     }
 
-    iov[1].iov_base = s->cvq_cmd_in_buffer;
     ok = vhost_vdpa_cvq_map_buf(&s->vhost_vdpa, NULL, 0,
-                                sizeof(virtio_net_ctrl_ack), iov[1].iov_base,
-                                &in_copied, true);
+                                sizeof(virtio_net_ctrl_ack),
+                                s->cvq_cmd_in_buffer, &in_copied, true);
     if (unlikely(!ok)) {
         vhost_vdpa_cvq_unmap_buf(&s->vhost_vdpa, s->cvq_cmd_out_buffer);
         return false;
     }
 
-    iov[1].iov_len = sizeof(virtio_net_ctrl_ack);
     return true;
 }
 
@@ -395,7 +390,7 @@ static int vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
     int r = -EINVAL;
     bool ok;
 
-    ok = vhost_vdpa_net_cvq_map_elem(s, elem, dev_buffers);
+    ok = vhost_vdpa_net_cvq_map_elem(s, elem, &dev_buffers[0].iov_len);
     if (unlikely(!ok)) {
         goto out;
     }
-- 
2.31.1


Re: [PATCH v5 04/10] vdpa: Get buffers from VhostVDPAState on vhost_vdpa_net_cvq_map_elem
Posted by Jason Wang 3 years, 6 months ago
在 2022/8/3 01:57, Eugenio Pérez 写道:
> There is no need to get them by parameter, since they're contained in
> VhostVDPAState. The only useful information was the written length in
> out.
>
> Simplify the function removing those.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>   net/vhost-vdpa.c | 17 ++++++-----------
>   1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index ac1810723c..c6699edfbc 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -303,34 +303,29 @@ dma_map_err:
>   
>   /**
>    * Copy the guest element into a dedicated buffer suitable to be sent to NIC
> - *
> - * @iov: [0] is the out buffer, [1] is the in one
>    */
>   static bool vhost_vdpa_net_cvq_map_elem(VhostVDPAState *s,
>                                           VirtQueueElement *elem,
> -                                        struct iovec *iov)
> +                                        size_t *out_len)
>   {
>       size_t in_copied;
>       bool ok;
>   
> -    iov[0].iov_base = s->cvq_cmd_out_buffer;
>       ok = vhost_vdpa_cvq_map_buf(&s->vhost_vdpa, elem->out_sg, elem->out_num,
> -                                vhost_vdpa_net_cvq_cmd_len(), iov[0].iov_base,
> -                                &iov[0].iov_len, false);
> +                                vhost_vdpa_net_cvq_cmd_len(),
> +                                s->cvq_cmd_out_buffer, out_len, false);
>       if (unlikely(!ok)) {
>           return false;
>       }
>   
> -    iov[1].iov_base = s->cvq_cmd_in_buffer;
>       ok = vhost_vdpa_cvq_map_buf(&s->vhost_vdpa, NULL, 0,
> -                                sizeof(virtio_net_ctrl_ack), iov[1].iov_base,
> -                                &in_copied, true);
> +                                sizeof(virtio_net_ctrl_ack),
> +                                s->cvq_cmd_in_buffer, &in_copied, true);


I'd suggest to do some tweak to make it easier for the reviewers:

- let vhost_vdpa_cvq_map_buf() and vhost_vdpa_net_cvq_map_elem() return 
ssize_t and drop the confusing written/out_len parameter of those 
functions.
- rename vhost_vdpa_net_cvq_map_elem() to 
vhost_vdpa_net_cvq_bounce_map() since it uses a bounce buffer actually

Thanks


>       if (unlikely(!ok)) {
>           vhost_vdpa_cvq_unmap_buf(&s->vhost_vdpa, s->cvq_cmd_out_buffer);
>           return false;
>       }
>   
> -    iov[1].iov_len = sizeof(virtio_net_ctrl_ack);
>       return true;
>   }
>   
> @@ -395,7 +390,7 @@ static int vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
>       int r = -EINVAL;
>       bool ok;
>   
> -    ok = vhost_vdpa_net_cvq_map_elem(s, elem, dev_buffers);
> +    ok = vhost_vdpa_net_cvq_map_elem(s, elem, &dev_buffers[0].iov_len);
>       if (unlikely(!ok)) {
>           goto out;
>       }