[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [PATCH v1] virtio-net: enable configurable tx queue size
On 06/05/2017 11:38 PM, Michael S. Tsirkin wrote:
On Mon, Jun 05, 2017 at 04:57:29PM +0800, Wei Wang wrote: /* * Calculate the number of bytes up to and including the given 'field' of @@ -57,6 +62,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_F_MAX_CHAIN_SIZE, + .end = endof(struct virtio_net_config, max_chain_size)}, {} }; Using a generic VIRTIO_F_MAX_CHAIN_SIZE flag, so this should go into the generic virtio section, not virtio_net_config.
Do you mean VIRTIO_PCI_xx under virtio_pci.h? The reason that the config field wasn't added there is because I didn't findspace to put into the new 16-bit field into the existing layout. The last one
is "#define VIRTIO_MSI_QUEUE_VECTOR 22". The per-driver configuration space directly follows that last register by "#define VIRTIO_PCI_CONFIG_OFF(msix_enabled) ((msix_enabled) ? 24:20)" Changing the MACRO to something like VIRTIO_PCI_CONFIG_OFF(max_chain_size_enabled) would be difficult. Do you see a good way to add the new field here?
I think the rx vq doesn't need the limit, because there is no off-by-one issue@@ -84,6 +91,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); } @@ -635,9 +643,33 @@ 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_reconfig_tx_queue_size(VirtIONet *n) +{ + VirtIODevice *vdev = VIRTIO_DEVICE(n); + int i, num_queues = virtio_get_num_queues(vdev); + + /* Return when the queue size is already less than the 1024 */ + 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)) { + n->net_conf.tx_queue_size = VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE / 2; + 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); + NetClientState *nc = qemu_get_queue(n->nic); int i;virtio_net_set_multiqueue(n,@@ -649,6 +681,16 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) virtio_has_feature(features, VIRTIO_F_VERSION_1));+ /*+ * When the traditional QEMU backend is used, using 1024 tx queue size + * requires the driver to support the VIRTIO_F_MAX_CHAIN_SIZE feature. If + * the feature is not supported, reconfigure the tx queue size to 512. + */ + if (!get_vhost_net(nc->peer) && + !virtio_has_feature(features, VIRTIO_F_MAX_CHAIN_SIZE)) { + virtio_net_reconfig_tx_queue_size(n); + } + if (n->has_vnet_hdr) { n->curr_guest_offloads = virtio_net_guest_offloads_by_features(features); @@ -1297,8 +1339,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_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;what about rx vq? we need to limit that as well, do we not?
like the tx side writev().
diff --git a/include/standard-headers/linux/virtio_config.h b/include/standard-headers/linux/virtio_config.hindex b777069..b70cbfe 100644 --- a/include/standard-headers/linux/virtio_config.h +++ b/include/standard-headers/linux/virtio_config.h @@ -60,6 +60,9 @@ #define VIRTIO_F_ANY_LAYOUT 27 #endif /* VIRTIO_CONFIG_NO_LEGACY */+/* Guest chains vring descriptors within a limit */+#define VIRTIO_F_MAX_CHAIN_SIZE 31 + Pls use a flag in the high 32 bit.
I think this should be considered as a transport feature. How about changing VIRTIO_TRANSPORT_F_END to 35 (was 34), and use 34 for VIRTIO_F_MAX_CHAIN_SIZE? Best, Wei
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]