On Fri, May 19, 2017 at 10:32:19AM +0800, Wei Wang wrote:
> This patch enables the virtio-net tx queue size to be configurable
> between 256 (the default queue size) and 1024 by the user. The queue
> size specified by the user should be power of 2.
>
> Setting the tx queue size to be 1024 requires the guest driver to
> support the VIRTIO_NET_F_MAX_CHAIN_SIZE feature.
This should be a generic ring feature, not one specific to virtio net.
> This feature restricts
> the guest driver from chaining 1024 vring descriptors, which may cause
> the device side implementation to send more than 1024 iov to writev.
> Currently, the max chain size allowed for the guest driver is set to
> 1023.
>
> In the case that the tx queue size is set to 1024 and the
> VIRTIO_NET_F_MAX_CHAIN_SIZE feature is not supported by the guest driver,
> the default tx queue size (256) will be used.
>
> Signed-off-by: Wei Wang <wei.w.w...@intel.com>
> ---
> hw/net/virtio-net.c | 71
> +++++++++++++++++++++++++++--
> include/hw/virtio/virtio-net.h | 1 +
> include/standard-headers/linux/virtio_net.h | 3 ++
> 3 files changed, 71 insertions(+), 4 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 7d091c9..ef38cb1 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -33,8 +33,12 @@
>
> /* previously fixed value */
> #define VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE 256
> +#define VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE 256
> /* for now, only allow larger queues; 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_MAX_CHAIN_SIZE 1023
>
> /*
> * Calculate the number of bytes up to and including the given 'field' of
> @@ -57,6 +61,8 @@ static VirtIOFeature feature_sizes[] = {
> .end = endof(struct virtio_net_config, max_virtqueue_pairs)},
> {.flags = 1 << VIRTIO_NET_F_MTU,
> .end = endof(struct virtio_net_config, mtu)},
> + {.flags = 1 << VIRTIO_NET_F_MAX_CHAIN_SIZE,
> + .end = endof(struct virtio_net_config, max_chain_size)},
> {}
> };
>
> @@ -84,6 +90,7 @@ static void virtio_net_get_config(VirtIODevice *vdev,
> uint8_t *config)
> virtio_stw_p(vdev, &netcfg.status, n->status);
> virtio_stw_p(vdev, &netcfg.max_virtqueue_pairs, n->max_queues);
> virtio_stw_p(vdev, &netcfg.mtu, n->net_conf.mtu);
> + virtio_stw_p(vdev, &netcfg.max_chain_size, VIRTIO_NET_MAX_CHAIN_SIZE);
> memcpy(netcfg.mac, n->mac, ETH_ALEN);
> memcpy(config, &netcfg, n->config_size);
> }
> @@ -568,6 +575,7 @@ static uint64_t virtio_net_get_features(VirtIODevice
> *vdev, uint64_t features,
> features |= n->host_features;
>
> virtio_add_feature(&features, VIRTIO_NET_F_MAC);
> + virtio_add_feature(&features, VIRTIO_NET_F_MAX_CHAIN_SIZE);
>
> if (!peer_has_vnet_hdr(n)) {
> virtio_clear_feature(&features, VIRTIO_NET_F_CSUM);
> @@ -603,6 +611,7 @@ static uint64_t virtio_net_bad_features(VirtIODevice
> *vdev)
> virtio_add_feature(&features, VIRTIO_NET_F_HOST_TSO4);
> virtio_add_feature(&features, VIRTIO_NET_F_HOST_TSO6);
> virtio_add_feature(&features, VIRTIO_NET_F_HOST_ECN);
> + virtio_add_feature(&features, VIRTIO_NET_F_MAX_CHAIN_SIZE);
>
> return features;
> }
> @@ -635,6 +644,27 @@ static inline uint64_t
> virtio_net_supported_guest_offloads(VirtIONet *n)
> return virtio_net_guest_offloads_by_features(vdev->guest_features);
> }
>
> +static bool is_tx(int queue_index)
> +{
> + return queue_index % 2 == 1;
> +}
> +
> +static void virtio_net_change_tx_queue_size(VirtIONet *n)
> +{
> + VirtIODevice *vdev = VIRTIO_DEVICE(n);
> + int i, num_queues = virtio_get_num_queues(vdev);
> +
> + if (n->net_conf.tx_queue_size == VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE) {
> + return;
> + }
> +
> + for (i = 0; i < num_queues; i++) {
> + if (is_tx(i)) {
> + virtio_queue_set_num(vdev, i, n->net_conf.tx_queue_size);
> + }
> + }
> +}
> +
> static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
> {
> VirtIONet *n = VIRTIO_NET(vdev);
> @@ -649,6 +679,16 @@ static void virtio_net_set_features(VirtIODevice *vdev,
> uint64_t features)
> virtio_has_feature(features,
> VIRTIO_F_VERSION_1));
>
> + /*
> + * Change the tx queue size if the guest supports
> + * VIRTIO_NET_F_MAX_CHAIN_SIZE. This will restrict the guest from sending
> + * a very large chain of vring descriptors (e.g. 1024), which may cause
> + * 1025 iov to be written to writev.
> + */
> + if (virtio_has_feature(features, VIRTIO_NET_F_MAX_CHAIN_SIZE)) {
> + virtio_net_change_tx_queue_size(n);
> + }
> +
> if (n->has_vnet_hdr) {
> n->curr_guest_offloads =
> virtio_net_guest_offloads_by_features(features);
> @@ -1297,8 +1337,8 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
>
> out_num = elem->out_num;
> out_sg = elem->out_sg;
> - if (out_num < 1) {
> - virtio_error(vdev, "virtio-net header not in first element");
> + if (out_num < 1 || out_num > VIRTIO_NET_F_MAX_CHAIN_SIZE) {
> + virtio_error(vdev, "no packet or too large vring desc chain");
> virtqueue_detach_element(q->tx_vq, elem, 0);
> g_free(elem);
> return -EINVAL;
> @@ -1491,18 +1531,27 @@ static void virtio_net_tx_bh(void *opaque)
> static void virtio_net_add_queue(VirtIONet *n, int index)
> {
> VirtIODevice *vdev = VIRTIO_DEVICE(n);
> + /*
> + * If the user specified tx queue size is less than IOV_MAX (e.g. 512),
> + * it is safe to use the specified queue size here. Otherwise, use the
> + * default queue size here, and change it when the guest confirms that
> + * it supports the VIRTIO_NET_F_MAX_CHAIN_SIZE feature.
> + */
> + uint16_t tx_queue_size = n->net_conf.tx_queue_size < IOV_MAX ?
> + n->net_conf.tx_queue_size :
> + VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE;
>
> n->vqs[index].rx_vq = virtio_add_queue(vdev, n->net_conf.rx_queue_size,
> virtio_net_handle_rx);
> if (n->net_conf.tx && !strcmp(n->net_conf.tx, "timer")) {
> n->vqs[index].tx_vq =
> - virtio_add_queue(vdev, 256, virtio_net_handle_tx_timer);
> + virtio_add_queue(vdev, tx_queue_size,
> virtio_net_handle_tx_timer);
> n->vqs[index].tx_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> virtio_net_tx_timer,
> &n->vqs[index]);
> } else {
> n->vqs[index].tx_vq =
> - virtio_add_queue(vdev, 256, virtio_net_handle_tx_bh);
> + virtio_add_queue(vdev, tx_queue_size, virtio_net_handle_tx_bh);
> n->vqs[index].tx_bh = qemu_bh_new(virtio_net_tx_bh, &n->vqs[index]);
> }
>
> @@ -1857,6 +1906,7 @@ static void virtio_net_set_config_size(VirtIONet *n,
> uint64_t host_features)
> {
> int i, config_size = 0;
> virtio_add_feature(&host_features, VIRTIO_NET_F_MAC);
> + virtio_add_feature(&host_features, VIRTIO_NET_F_MAX_CHAIN_SIZE);
You can't really change the default features - this will
break migration to/from old QEMU versions.
You need a flag for this and fix it up with the compat machinery.
>
> for (i = 0; feature_sizes[i].flags != 0; i++) {
> if (host_features & feature_sizes[i].flags) {
> @@ -1910,6 +1960,17 @@ static void virtio_net_device_realize(DeviceState
> *dev, Error **errp)
> 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 & (n->net_conf.tx_queue_size - 1))) {
> + 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_cleanup(vdev);
> + return;
> + }
> +
> n->max_queues = MAX(n->nic_conf.peers.queues, 1);
> if (n->max_queues * 2 + 1 > VIRTIO_QUEUE_MAX) {
> error_setg(errp, "Invalid number of queues (= %" PRIu32 "), "
> @@ -2089,6 +2150,8 @@ static Property virtio_net_properties[] = {
> 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),
> + DEFINE_PROP_UINT16("tx_queue_size", VirtIONet, net_conf.tx_queue_size,
> + VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE),
> DEFINE_PROP_UINT16("host_mtu", VirtIONet, net_conf.mtu, 0),
> DEFINE_PROP_END_OF_LIST(),
> };
> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
> index 1eec9a2..fd944ba 100644
> --- a/include/hw/virtio/virtio-net.h
> +++ b/include/hw/virtio/virtio-net.h
> @@ -36,6 +36,7 @@ typedef struct virtio_net_conf
> int32_t txburst;
> char *tx;
> uint16_t rx_queue_size;
> + uint16_t tx_queue_size;
> uint16_t mtu;
> } virtio_net_conf;
>
> diff --git a/include/standard-headers/linux/virtio_net.h
> b/include/standard-headers/linux/virtio_net.h
> index 30ff249..0bc1c52 100644
> --- a/include/standard-headers/linux/virtio_net.h
> +++ b/include/standard-headers/linux/virtio_net.h
> @@ -56,6 +56,7 @@
> #define VIRTIO_NET_F_MQ 22 /* Device supports Receive Flow
> * Steering */
> #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
> +#define VIRTIO_NET_F_MAX_CHAIN_SIZE 25 /* Guest chains desc within a
> limit */
>
> #ifndef VIRTIO_NET_NO_LEGACY
> #define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
> @@ -76,6 +77,8 @@ struct virtio_net_config {
> uint16_t max_virtqueue_pairs;
> /* Default maximum transmit unit advice */
> uint16_t mtu;
> + /* Maximum number of vring descriptors that can be chained */
> + uint16_t max_chain_size;
> } QEMU_PACKED;
>
> /*
> --
> 2.7.4
