From: Stefano Garzarella <sgarzare@redhat.com>
To: "Jiang Wang ." <jiang.wang@bytedance.com>
Cc: cong.wang@bytedance.com, "Michael S. Tsirkin" <mst@redhat.com>,
qemu-devel@nongnu.org, "Yongji Xie" <xieyongji@bytedance.com>,
柴稳 <chaiwen.cc@bytedance.com>,
"Stefan Hajnoczi" <stefanha@redhat.com>,
fam.zheng@bytedance.com
Subject: Re: [RFC v1] virtio/vsock: add two more queues for datagram types
Date: Thu, 24 Jun 2021 16:31:27 +0200 [thread overview]
Message-ID: <20210624143127.fqubmuvw634j44mi@steredhat> (raw)
In-Reply-To: <CAP_N_Z9kFc3pnK0Uwqc-fvfaakAh5VMYBR+9SZkz3w658XRK1g@mail.gmail.com>
On Wed, Jun 23, 2021 at 11:50:33PM -0700, Jiang Wang . wrote:
>Hi Stefano,
>
>I checked virtio_net_set_multiqueue(), which will help with following
>changes in my patch:
>
>#ifdef CONFIG_VHOST_VSOCK_DGRAM
>vvc->dgram_recv_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
>vhost_vsock_common_handle_output);
>vvc->dgram_trans_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
>vhost_vsock_common_handle_output);
>#endif
>
>But I think there is still an issue with the following lines, right?
Yep, I think so.
>
>#ifdef CONFIG_VHOST_VSOCK_DGRAM
>struct vhost_virtqueue vhost_vqs[4];
>#else
>struct vhost_virtqueue vhost_vqs[2];
>#endif
>
>I think the problem with feature bits is that they are set and get after
>vhost_vsock_common_realize() and after vhost_dev_init() in drivers/vhost/vsock.c
>But those virtqueues need to be set up correctly beforehand.
I think we can follow net and scsi vhost devices, so we can set a
VHOST_VSOCK_VQ_MAX(5), allocates all the queues in any case and then use
only the queues acked by the guest.
>
>I tried to test with the host kernel allocating 4 vqs, but qemu only
>allocated 2 vqs, and
>guest kernel will not be able to send even the vsock stream packets. I
>think the host
>kernel and the qemu have to agree on the number of vhost_vqs. Do you agree?
>Did I miss something?
Mmm, I need to check, but for example vhost-net calls vhost_dev_init()
with VHOST_NET_VQ_MAX, but then the guest can decide to use only one
couple of TX and RX queues.
I'm not sure about qemu point of view, but I expected that QEMU can set
less queues then queues allocated by the kernel. `vhost_dev.nvqs` should
be set with the amount of queue that QEMU can handle.
>
>Another idea to make the setting in runtime instead of compiling time
>is to use
>qemu cmd-line options, then qemu can allocate 2 or 4 queues depending
>on
>the cmd line. This will solve the issue when the host kernel is an old
>one( no dgram
>support) and the qemu is a new one.
I don't think this is a good idea, at most we can add an ioctl that qemu
can use to query the kernel about allocated queues, but I still need to
understand better if we really we need this.
>
>But there is still an issue when the host kernel is a new one, while
>the qemu
>is an old one. I am not sure how to make the virtqueues numbers to
>change in run-time
>for the host kernel. In another email thread, you mentioned removing kconfig
>in the linux kernel, I believe that is related to this qemu patch,
>right?
It was related to both, I don't think we should build QEMU and Linux
with or without dgram support.
> If so,
>any ideas that I can make the host kernel change the number of vqs in
>the run-time
>or when starting up vsock? The only way I can think of is to use a
>kernel module parameter
>for the vsock_vhost module. Any other ideas? Thanks.
I need to check better, but we should be able to do all at run time
looking at the features field. As I said, both QEMU and kernel can
allocate the maximum number of queues that they can handle, then enable
only the queues allocated by the guest (e.g. during
vhost_vsock_common_start()).
>
>btw, I searched Linux kernel code but did not find any examples.
>
I'm a bit busy this week, I'll try to write some PoC next week if you
can't find a working solution. (without any #ifdef :-)
Thanks,
Stefano
next prev parent reply other threads:[~2021-06-24 14:35 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-10 0:14 [RFC v1] virtio/vsock: add two more queues for datagram types Jiang Wang
2021-06-10 9:40 ` Stefano Garzarella
2021-06-10 17:29 ` Jiang Wang .
2021-06-24 6:50 ` Jiang Wang .
2021-06-24 14:31 ` Stefano Garzarella [this message]
2021-06-30 22:44 ` [External] " Jiang Wang .
2021-07-01 6:55 ` Stefano Garzarella
2021-07-01 22:09 ` Jiang Wang .
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210624143127.fqubmuvw634j44mi@steredhat \
--to=sgarzare@redhat.com \
--cc=chaiwen.cc@bytedance.com \
--cc=cong.wang@bytedance.com \
--cc=fam.zheng@bytedance.com \
--cc=jiang.wang@bytedance.com \
--cc=mst@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
--cc=xieyongji@bytedance.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).