[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 06/10] vdpa: Make vhost_vdpa_net_cvq_map_elem accept any o
From: |
Eugenio Perez Martin |
Subject: |
Re: [PATCH v5 06/10] vdpa: Make vhost_vdpa_net_cvq_map_elem accept any out sg |
Date: |
Thu, 4 Aug 2022 09:38:36 +0200 |
On Thu, Aug 4, 2022 at 6:17 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2022/8/3 01:57, Eugenio Pérez 写道:
> > So its generic enough to accept any out sg buffer and we can inject
> > NIC state messages.
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> > v5: Accept out sg instead of dev_buffers[]
> > ---
> > net/vhost-vdpa.c | 13 +++++++------
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> > index 33bf3d6409..2421bca347 100644
> > --- a/net/vhost-vdpa.c
> > +++ b/net/vhost-vdpa.c
> > @@ -302,16 +302,16 @@ dma_map_err:
> > }
> >
> > /**
> > - * Copy the guest element into a dedicated buffer suitable to be sent to
> > NIC
> > + * Maps out sg and in buffer into dedicated buffers suitable to be sent to
> > NIC
> > */
> > -static bool vhost_vdpa_net_cvq_map_elem(VhostVDPAState *s,
> > - VirtQueueElement *elem,
> > - size_t *out_len)
> > +static bool vhost_vdpa_net_cvq_map_sg(VhostVDPAState *s,
> > + const struct iovec *out, size_t
> > out_num,
> > + size_t *out_len)
>
>
> This still looks not genreal as there's no guarantee that we won't have
> command-in-specific-data. One example is that Ali is working on the
> virtio-net statistics fetching from the control virtqueue.
>
> So it looks to me we'd better have a general bounce_map here that accepts:
>
> 1) out_sg and out_num
> 2) in_sg and in_num
>
We don't need to pass in_sg for that: The only useful information is
its size. Since the exposed buffer is an in one, it's enough to expose
the s->cvq_cmd_in_buffer buffer in full. The caller already knows the
device will write to it, so the only missing piece is to return the
written length at vhost_vdpa_net_cvq_add.
Is one page the right buffer size for the in buffer? Is it worth
worrying about it before implementing the stat control command in qemu
virtio-net?
> In this level, we'd better not have any special care about the in as the
> ack.
We need to care about it. If a property has not been updated in the
vdpa device (it returned VIRTIO_NET_ERR), we must not update the
device model.
We can move the processing from vhost_vdpa_net_cvq_add to
vhost_vdpa_net_load and vhost_vdpa_net_handle_ctrl_avail, but the code
gets duplicated then.
> And we need do bouncing:
>
> 1) for out buffer, during map
> 2) for in buffer during unmap
>
We can move the copy of the in_buffer to the unmap for sure.
Thanks!
> Thanks
>
>
> > {
> > size_t in_copied;
> > bool ok;
> >
> > - ok = vhost_vdpa_cvq_map_buf(&s->vhost_vdpa, elem->out_sg,
> > elem->out_num,
> > + ok = vhost_vdpa_cvq_map_buf(&s->vhost_vdpa, out, out_num,
> > vhost_vdpa_net_cvq_cmd_len(),
> > s->cvq_cmd_out_buffer, out_len, false);
> > if (unlikely(!ok)) {
> > @@ -435,7 +435,8 @@ static int
> > vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
> > };
> > bool ok;
> >
> > - ok = vhost_vdpa_net_cvq_map_elem(s, elem, &dev_buffers[0].iov_len);
> > + ok = vhost_vdpa_net_cvq_map_sg(s, elem->out_sg, elem->out_num,
> > + &dev_buffers[0].iov_len);
> > if (unlikely(!ok)) {
> > goto out;
> > }
>
[PATCH v5 07/10] vdpa: add NetClientState->load() callback, Eugenio Pérez, 2022/08/02
[PATCH v5 08/10] vdpa: add net_vhost_vdpa_cvq_info NetClientInfo, Eugenio Pérez, 2022/08/02
[PATCH v5 09/10] vdpa: Add virtio-net mac address via CVQ at start, Eugenio Pérez, 2022/08/02
[PATCH v5 10/10] vdpa: Delete CVQ migration blocker, Eugenio Pérez, 2022/08/02
Re: [PATCH v5 00/10] NIC vhost-vdpa state restore via Shadow CVQ, Jason Wang, 2022/08/04