Re: [PATCH] virtio-net: set the max of queue size to 4096

[Jason Wang]

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
> >