hw/net/virtio-net.c | 47 +++++++++++++++++++++++++++----------- hw/virtio/virtio.c | 8 +++++-- include/hw/virtio/virtio.h | 1 + 3 files changed, 41 insertions(+), 15 deletions(-)
From: liuhaiwei <liuhaiwei@inspur.com>
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.
why not enlarge the maximum size (such as 4096) when using the vhost backend?
Signed-off-by: liuhaiwei <liuhaiwei9699@126.com>
Signed-off-by: liuhaiwei <liuhaiwei@inspur.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)
typedef struct VirtQueueElement
{
--
2.27.0On Tue, Sep 20, 2022 at 9:10 AM liuhaiwei <liuhaiwei9699@126.com> wrote:
>
> From: liuhaiwei <liuhaiwei@inspur.com>
>
> 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.
> why not enlarge the maximum size (such as 4096) when using the vhost backend?
As Michael mentioned, there's a limitation in the kernel UIO_MAXIOV.
We need to find way to overcome that limit first.
Thanks
>
> Signed-off-by: liuhaiwei <liuhaiwei9699@126.com>
> Signed-off-by: liuhaiwei <liuhaiwei@inspur.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)
>
> typedef struct VirtQueueElement
> {
> --
> 2.27.0
>
On Tue, Sep 20, 2022 at 9:38 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Tue, Sep 20, 2022 at 9:10 AM liuhaiwei <liuhaiwei9699@126.com> wrote:
> >
> > From: liuhaiwei <liuhaiwei@inspur.com>
> >
> > 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.
> > why not enlarge the maximum size (such as 4096) when using the vhost backend?
>
> As Michael mentioned, there's a limitation in the kernel UIO_MAXIOV.
> We need to find way to overcome that limit first.
Btw, this probably means the skb needs to be built by vhost-net
itself, instead of tuntap.
Thanks
>
> Thanks
>
> >
> > Signed-off-by: liuhaiwei <liuhaiwei9699@126.com>
> > Signed-off-by: liuhaiwei <liuhaiwei@inspur.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)
> >
> > typedef struct VirtQueueElement
> > {
> > --
> > 2.27.0
> >
On Tue, Sep 20, 2022 at 10:03:23AM +0800, Jason Wang wrote:
> On Tue, Sep 20, 2022 at 9:38 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Tue, Sep 20, 2022 at 9:10 AM liuhaiwei <liuhaiwei9699@126.com> wrote:
> > >
> > > From: liuhaiwei <liuhaiwei@inspur.com>
> > >
> > > 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.
> > > why not enlarge the maximum size (such as 4096) when using the vhost backend?
> >
> > As Michael mentioned, there's a limitation in the kernel UIO_MAXIOV.
> > We need to find way to overcome that limit first.
>
> Btw, this probably means the skb needs to be built by vhost-net
> itself, instead of tuntap.
>
> Thanks
That might help vhost-net but it won't help virtio-net.
IMO the right fix is to add a separate limit on s/g size
to the spec. Block and scsi already have it, seems
reasonable to add it for others too.
> >
> > Thanks
> >
> > >
> > > Signed-off-by: liuhaiwei <liuhaiwei9699@126.com>
> > > Signed-off-by: liuhaiwei <liuhaiwei@inspur.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)
> > >
> > > typedef struct VirtQueueElement
> > > {
> > > --
> > > 2.27.0
> > >
On Tue, Sep 20, 2022 at 8:59 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Sep 20, 2022 at 10:03:23AM +0800, Jason Wang wrote:
> > On Tue, Sep 20, 2022 at 9:38 AM Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > On Tue, Sep 20, 2022 at 9:10 AM liuhaiwei <liuhaiwei9699@126.com> wrote:
> > > >
> > > > From: liuhaiwei <liuhaiwei@inspur.com>
> > > >
> > > > 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.
> > > > why not enlarge the maximum size (such as 4096) when using the vhost backend?
> > >
> > > As Michael mentioned, there's a limitation in the kernel UIO_MAXIOV.
> > > We need to find way to overcome that limit first.
> >
> > Btw, this probably means the skb needs to be built by vhost-net
> > itself, instead of tuntap.
> >
> > Thanks
>
> That might help vhost-net but it won't help virtio-net.
>
> IMO the right fix is to add a separate limit on s/g size
> to the spec. Block and scsi already have it, seems
> reasonable to add it for others too.
I wonder if it would be simpler to tie the limit to the virtqueue size.
Having individual limits seems to complicate the migration compatibility anyhow.
Thanks
>
>
>
> > >
> > > Thanks
> > >
> > > >
> > > > Signed-off-by: liuhaiwei <liuhaiwei9699@126.com>
> > > > Signed-off-by: liuhaiwei <liuhaiwei@inspur.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)
> > > >
> > > > typedef struct VirtQueueElement
> > > > {
> > > > --
> > > > 2.27.0
> > > >
>
On Wed, Sep 21, 2022 at 10:59:50AM +0800, Jason Wang wrote:
> On Tue, Sep 20, 2022 at 8:59 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Tue, Sep 20, 2022 at 10:03:23AM +0800, Jason Wang wrote:
> > > On Tue, Sep 20, 2022 at 9:38 AM Jason Wang <jasowang@redhat.com> wrote:
> > > >
> > > > On Tue, Sep 20, 2022 at 9:10 AM liuhaiwei <liuhaiwei9699@126.com> wrote:
> > > > >
> > > > > From: liuhaiwei <liuhaiwei@inspur.com>
> > > > >
> > > > > 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.
> > > > > why not enlarge the maximum size (such as 4096) when using the vhost backend?
> > > >
> > > > As Michael mentioned, there's a limitation in the kernel UIO_MAXIOV.
> > > > We need to find way to overcome that limit first.
> > >
> > > Btw, this probably means the skb needs to be built by vhost-net
> > > itself, instead of tuntap.
> > >
> > > Thanks
> >
> > That might help vhost-net but it won't help virtio-net.
> >
> > IMO the right fix is to add a separate limit on s/g size
> > to the spec. Block and scsi already have it, seems
> > reasonable to add it for others too.
>
> I wonder if it would be simpler to tie the limit to the virtqueue size.
Simpler but wrong I think. As another example for RX we do not need s/g
at all with mergeable. At the same time deep queues are needed
to avoid underruns.
> Having individual limits seems to complicate the migration compatibility anyhow.
>
> Thanks
I don't see how it's materially different from queue size.
We need interfaces for that, that we lack now, userspace is
expected to guess a subset available on both ends.
> >
> >
> >
> > > >
> > > > Thanks
> > > >
> > > > >
> > > > > Signed-off-by: liuhaiwei <liuhaiwei9699@126.com>
> > > > > Signed-off-by: liuhaiwei <liuhaiwei@inspur.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)
> > > > >
> > > > > typedef struct VirtQueueElement
> > > > > {
> > > > > --
> > > > > 2.27.0
> > > > >
> >
On Wed, Sep 21, 2022 at 11:57 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Sep 21, 2022 at 10:59:50AM +0800, Jason Wang wrote:
> > On Tue, Sep 20, 2022 at 8:59 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Tue, Sep 20, 2022 at 10:03:23AM +0800, Jason Wang wrote:
> > > > On Tue, Sep 20, 2022 at 9:38 AM Jason Wang <jasowang@redhat.com> wrote:
> > > > >
> > > > > On Tue, Sep 20, 2022 at 9:10 AM liuhaiwei <liuhaiwei9699@126.com> wrote:
> > > > > >
> > > > > > From: liuhaiwei <liuhaiwei@inspur.com>
> > > > > >
> > > > > > 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.
> > > > > > why not enlarge the maximum size (such as 4096) when using the vhost backend?
> > > > >
> > > > > As Michael mentioned, there's a limitation in the kernel UIO_MAXIOV.
> > > > > We need to find way to overcome that limit first.
> > > >
> > > > Btw, this probably means the skb needs to be built by vhost-net
> > > > itself, instead of tuntap.
> > > >
> > > > Thanks
> > >
> > > That might help vhost-net but it won't help virtio-net.
> > >
> > > IMO the right fix is to add a separate limit on s/g size
> > > to the spec. Block and scsi already have it, seems
> > > reasonable to add it for others too.
> >
> > I wonder if it would be simpler to tie the limit to the virtqueue size.
>
> Simpler but wrong I think. As another example for RX we do not need s/g
> at all with mergeable.
But this is not forbidden by the spec if I was not wrong.
> At the same time deep queues are needed
> to avoid underruns.
>
>
>
> > Having individual limits seems to complicate the migration compatibility anyhow.
> >
> > Thanks
>
> I don't see how it's materially different from queue size.
Yes.
> We need interfaces for that, that we lack now, userspace is
> expected to guess a subset available on both ends.
>
Ok.
Thanks
> > >
> > >
> > >
> > > > >
> > > > > Thanks
> > > > >
> > > > > >
> > > > > > Signed-off-by: liuhaiwei <liuhaiwei9699@126.com>
> > > > > > Signed-off-by: liuhaiwei <liuhaiwei@inspur.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)
> > > > > >
> > > > > > typedef struct VirtQueueElement
> > > > > > {
> > > > > > --
> > > > > > 2.27.0
> > > > > >
> > >
>
On Wed, Sep 21, 2022 at 02:15:41PM +0800, Jason Wang wrote:
> On Wed, Sep 21, 2022 at 11:57 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Wed, Sep 21, 2022 at 10:59:50AM +0800, Jason Wang wrote:
> > > On Tue, Sep 20, 2022 at 8:59 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > >
> > > > On Tue, Sep 20, 2022 at 10:03:23AM +0800, Jason Wang wrote:
> > > > > On Tue, Sep 20, 2022 at 9:38 AM Jason Wang <jasowang@redhat.com> wrote:
> > > > > >
> > > > > > On Tue, Sep 20, 2022 at 9:10 AM liuhaiwei <liuhaiwei9699@126.com> wrote:
> > > > > > >
> > > > > > > From: liuhaiwei <liuhaiwei@inspur.com>
> > > > > > >
> > > > > > > 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.
> > > > > > > why not enlarge the maximum size (such as 4096) when using the vhost backend?
> > > > > >
> > > > > > As Michael mentioned, there's a limitation in the kernel UIO_MAXIOV.
> > > > > > We need to find way to overcome that limit first.
> > > > >
> > > > > Btw, this probably means the skb needs to be built by vhost-net
> > > > > itself, instead of tuntap.
> > > > >
> > > > > Thanks
> > > >
> > > > That might help vhost-net but it won't help virtio-net.
> > > >
> > > > IMO the right fix is to add a separate limit on s/g size
> > > > to the spec. Block and scsi already have it, seems
> > > > reasonable to add it for others too.
> > >
> > > I wonder if it would be simpler to tie the limit to the virtqueue size.
> >
> > Simpler but wrong I think. As another example for RX we do not need s/g
> > at all with mergeable.
>
> But this is not forbidden by the spec if I was not wrong.
Yes, it is forbidden for device not to support S/G on any queues.
There's no way to express such a device limitation in the spec.
> > At the same time deep queues are needed
> > to avoid underruns.
> >
> >
> >
> > > Having individual limits seems to complicate the migration compatibility anyhow.
> > >
> > > Thanks
> >
> > I don't see how it's materially different from queue size.
>
> Yes.
>
> > We need interfaces for that, that we lack now, userspace is
> > expected to guess a subset available on both ends.
> >
>
> Ok.
>
> Thanks
>
> > > >
> > > >
> > > >
> > > > > >
> > > > > > Thanks
> > > > > >
> > > > > > >
> > > > > > > Signed-off-by: liuhaiwei <liuhaiwei9699@126.com>
> > > > > > > Signed-off-by: liuhaiwei <liuhaiwei@inspur.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)
> > > > > > >
> > > > > > > typedef struct VirtQueueElement
> > > > > > > {
> > > > > > > --
> > > > > > > 2.27.0
> > > > > > >
> > > >
> >
© 2016 - 2026 Red Hat, Inc.