[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] virtio-net: set the max of queue size to 4096 according to t
|
From: |
Michael S. Tsirkin |
|
Subject: |
Re: [PATCH] virtio-net: set the max of queue size to 4096 according to the virtio specs, The maximum Queue Size value is 32768 : https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.html#x1-240006 |
|
Date: |
Mon, 19 Sep 2022 09:01:42 -0400 |
subject is unreasonably long.
limit it to 50 characters. esp links do not belong there.
Looks like this is a repost, ignoring most comments
on the previous version.
This kind of thing is ok as a conversation starter but then
pls add RFC in the subject.
On Mon, Sep 19, 2022 at 08:28:22AM -0400, liuhaiwei wrote:
> the limit of maximum of rx_queue_size and tx_queue to 1024 is so small as to
> affect our network performance when using the virtio-net and vhost ,
> we cannot set the maximum size beyond 1k.
don't use such long lines in the commit log. The line length for the
body text of a commit message should be constrained to 80 characters.
> why not enlarge the maximum size (such as 4096) when using the vhost backend?
because we might then theoretically get a single s/g exceeding 1k,
and that does not fit in an iovec.
>
> Signed-off-by: liuhaiwei <liuhaiwei9699@126.com>
> ---
> hw/net/virtio-net.c | 47 +++++++++++++++++++++++++++-----------
> hw/virtio/virtio.c | 8 +++++--
> include/hw/virtio/virtio.h | 1 +
> 3 files changed, 41 insertions(+), 15 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index dd0d056fde..4b56484855 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -52,12 +52,11 @@
> #define MAX_VLAN (1 << 12) /* Per 802.1Q definition */
>
> /* previously fixed value */
> -#define VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE 256
> -#define VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE 256
> +#define VIRTIO_NET_VHOST_USER_DEFAULT_SIZE 2048
>
> /* for now, only allow larger queue_pairs; with virtio-1, guest can downsize
> */
> -#define VIRTIO_NET_RX_QUEUE_MIN_SIZE VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE
> -#define VIRTIO_NET_TX_QUEUE_MIN_SIZE VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE
> +#define VIRTIO_NET_RX_QUEUE_MIN_SIZE 256
> +#define VIRTIO_NET_TX_QUEUE_MIN_SIZE 256
>
> #define VIRTIO_NET_IP4_ADDR_SIZE 8 /* ipv4 saddr + daddr */
>
> @@ -594,6 +593,28 @@ static int peer_has_ufo(VirtIONet *n)
> return n->has_ufo;
> }
>
> +static void virtio_net_set_default_queue_size(VirtIONet *n)
> +{
> + NetClientState *peer = n->nic_conf.peers.ncs[0];
> +
> + /* Default value is 0 if not set */
> + if (n->net_conf.rx_queue_size == 0) {
> + if (peer && peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> + n->net_conf.rx_queue_size = VIRTIO_NET_VHOST_USER_DEFAULT_SIZE;
> + } else {
> + n->net_conf.rx_queue_size = VIRTIO_NET_VQ_MAX_SIZE;
> + }
> + }
> +
> + if (n->net_conf.tx_queue_size == 0) {
> + if (peer && peer->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> + n->net_conf.tx_queue_size = VIRTIO_NET_VHOST_USER_DEFAULT_SIZE;
> + } else {
> + n->net_conf.tx_queue_size = VIRTIO_NET_VQ_MAX_SIZE;
> + }
> + }
> +}
> +
> static void virtio_net_set_mrg_rx_bufs(VirtIONet *n, int mergeable_rx_bufs,
> int version_1, int hash_report)
> {
> @@ -633,7 +654,7 @@ static int virtio_net_max_tx_queue_size(VirtIONet *n)
> * size.
> */
> if (!peer) {
> - return VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE;
> + return VIRTIO_NET_VQ_MAX_SIZE;
> }
>
> switch(peer->info->type) {
> @@ -641,7 +662,7 @@ static int virtio_net_max_tx_queue_size(VirtIONet *n)
> case NET_CLIENT_DRIVER_VHOST_VDPA:
> return VIRTQUEUE_MAX_SIZE;
> default:
> - return VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE;
> + return VIRTIO_NET_VQ_MAX_SIZE;
> };
> }
>
> @@ -3450,30 +3471,30 @@ static void virtio_net_device_realize(DeviceState
> *dev, Error **errp)
>
> virtio_net_set_config_size(n, n->host_features);
> virtio_init(vdev, VIRTIO_ID_NET, n->config_size);
> -
> + virtio_net_set_default_queue_size(n);
> /*
> * We set a lower limit on RX queue size to what it always was.
> * Guests that want a smaller ring can always resize it without
> * help from us (using virtio 1 and up).
> */
> if (n->net_conf.rx_queue_size < VIRTIO_NET_RX_QUEUE_MIN_SIZE ||
> - n->net_conf.rx_queue_size > VIRTQUEUE_MAX_SIZE ||
> + n->net_conf.rx_queue_size > VIRTIO_NET_VQ_MAX_SIZE ||
> !is_power_of_2(n->net_conf.rx_queue_size)) {
> error_setg(errp, "Invalid rx_queue_size (= %" PRIu16 "), "
> "must be a power of 2 between %d and %d.",
> n->net_conf.rx_queue_size, VIRTIO_NET_RX_QUEUE_MIN_SIZE,
> - VIRTQUEUE_MAX_SIZE);
> + VIRTIO_NET_VQ_MAX_SIZE );
> virtio_cleanup(vdev);
> return;
> }
>
> if (n->net_conf.tx_queue_size < VIRTIO_NET_TX_QUEUE_MIN_SIZE ||
> - n->net_conf.tx_queue_size > VIRTQUEUE_MAX_SIZE ||
> + n->net_conf.tx_queue_size > VIRTIO_NET_VQ_MAX_SIZE ||
> !is_power_of_2(n->net_conf.tx_queue_size)) {
> error_setg(errp, "Invalid tx_queue_size (= %" PRIu16 "), "
> "must be a power of 2 between %d and %d",
> n->net_conf.tx_queue_size, VIRTIO_NET_TX_QUEUE_MIN_SIZE,
> - VIRTQUEUE_MAX_SIZE);
> + VIRTIO_NET_VQ_MAX_SIZE);
> virtio_cleanup(vdev);
> return;
> }
> @@ -3751,9 +3772,9 @@ static Property virtio_net_properties[] = {
> DEFINE_PROP_INT32("x-txburst", VirtIONet, net_conf.txburst, TX_BURST),
> DEFINE_PROP_STRING("tx", VirtIONet, net_conf.tx),
> DEFINE_PROP_UINT16("rx_queue_size", VirtIONet, net_conf.rx_queue_size,
> - VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE),
> + 0),
> DEFINE_PROP_UINT16("tx_queue_size", VirtIONet, net_conf.tx_queue_size,
> - VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE),
> + 0),
> DEFINE_PROP_UINT16("host_mtu", VirtIONet, net_conf.mtu, 0),
> DEFINE_PROP_BOOL("x-mtu-bypass-backend", VirtIONet, mtu_bypass_backend,
> true),
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 5d607aeaa0..ad9dfa20e7 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2286,11 +2286,15 @@ void virtio_queue_set_rings(VirtIODevice *vdev, int
> n, hwaddr desc,
>
> void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
> {
> + int vq_max_size = VIRTQUEUE_MAX_SIZE;
> + if (!strcmp(vdev->name, "virtio-net")) {
> + vq_max_size = VIRTIO_NET_VQ_MAX_SIZE;
> + }
> /* Don't allow guest to flip queue between existent and
> * nonexistent states, or to set it to an invalid size.
> */
> if (!!num != !!vdev->vq[n].vring.num ||
> - num > VIRTQUEUE_MAX_SIZE ||
> + num > vq_max_size ||
> num < 0) {
> return;
> }
> @@ -2423,7 +2427,7 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int
> queue_size,
> break;
> }
>
> - if (i == VIRTIO_QUEUE_MAX || queue_size > VIRTQUEUE_MAX_SIZE)
> + if (i == VIRTIO_QUEUE_MAX )
> abort();
>
> vdev->vq[i].vring.num = queue_size;
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index db1c0ddf6b..1f4d2eb5d7 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -50,6 +50,7 @@ size_t virtio_feature_get_config_size(const VirtIOFeature
> *features,
> typedef struct VirtQueue VirtQueue;
>
> #define VIRTQUEUE_MAX_SIZE 1024
> +#define VIRTIO_NET_VQ_MAX_SIZE (4096)
() not needed
> typedef struct VirtQueueElement
> {
> --
> 2.27.0