[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 4/8] vdpa: Avoid using vhost_vdpa_net_load_*() outside vho
From: |
Eugenio Perez Martin |
Subject: |
Re: [PATCH v4 4/8] vdpa: Avoid using vhost_vdpa_net_load_*() outside vhost_vdpa_net_load() |
Date: |
Tue, 3 Oct 2023 19:48:10 +0200 |
On Tue, Aug 29, 2023 at 7:55 AM Hawkins Jiawei <yin31149@gmail.com> wrote:
>
> Next patches in this series will refactor vhost_vdpa_net_load_cmd()
> to iterate through the control commands shadow buffers, allowing QEMU
> to send CVQ state load commands in parallel at device startup.
>
> Considering that QEMU always forwards the CVQ command serialized
> outside of vhost_vdpa_net_load(), it is more elegant to send the
> CVQ commands directly without invoking vhost_vdpa_net_load_*() helpers.
>
> Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
> ---
> v4:
> - pack CVQ command by iov_from_buf() instead of accessing
> `out` directly suggested by Eugenio
>
> v3:
> https://lore.kernel.org/all/428a8fac2a29b37757fa15ca747be93c0226cb1f.1689748694.git.yin31149@gmail.com/
>
> net/vhost-vdpa.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index e6342b213f..7c67063469 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -1097,12 +1097,14 @@ static NetClientInfo net_vhost_vdpa_cvq_info = {
> */
> static int vhost_vdpa_net_excessive_mac_filter_cvq_add(VhostVDPAState *s,
> VirtQueueElement
> *elem,
> - struct iovec *out)
> + struct iovec *out,
> + const struct iovec
> *in)
> {
> struct virtio_net_ctrl_mac mac_data, *mac_ptr;
> struct virtio_net_ctrl_hdr *hdr_ptr;
> uint32_t cursor;
> ssize_t r;
> + uint8_t on = 1;
>
> /* parse the non-multicast MAC address entries from CVQ command */
> cursor = sizeof(*hdr_ptr);
> @@ -1150,7 +1152,15 @@ static int
> vhost_vdpa_net_excessive_mac_filter_cvq_add(VhostVDPAState *s,
> * filter table to the vdpa device, it should send the
> * VIRTIO_NET_CTRL_RX_PROMISC CVQ command to enable promiscuous mode
> */
> - r = vhost_vdpa_net_load_rx_mode(s, VIRTIO_NET_CTRL_RX_PROMISC, 1);
> + cursor = 0;
I think this is redundant, as "cursor" is not used by the new code and
it is already set to 0 before its usage, isn't it?
> + hdr_ptr = out->iov_base;
> + out->iov_len = sizeof(*hdr_ptr) + sizeof(on);
> + assert(out->iov_len < vhost_vdpa_net_cvq_cmd_page_len());
> +
I think we can assume this assertion is never hit, as out->iov_len is
controlled by this function.
Apart from these nits,
Acked-by: Eugenio Pérez <eperezma@redhat.com>
> + hdr_ptr->class = VIRTIO_NET_CTRL_RX;
> + hdr_ptr->cmd = VIRTIO_NET_CTRL_RX_PROMISC;
> + iov_from_buf(out, 1, sizeof(*hdr_ptr), &on, sizeof(on));
> + r = vhost_vdpa_net_cvq_add(s, out, 1, in, 1);
> if (unlikely(r < 0)) {
> return r;
> }
> @@ -1268,7 +1278,7 @@ static int
> vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
> * the CVQ command direclty.
> */
> dev_written = vhost_vdpa_net_excessive_mac_filter_cvq_add(s, elem,
> - &out);
> + &out, &vdpa_in);
> if (unlikely(dev_written < 0)) {
> goto out;
> }
> --
> 2.25.1
>
- Re: [PATCH v4 4/8] vdpa: Avoid using vhost_vdpa_net_load_*() outside vhost_vdpa_net_load(),
Eugenio Perez Martin <=