Re: [net-next RFC V5 5/5] virtio_net: support negotiating the numberof queues through ctrl vq

From: Jason Wang
Date: Mon Jul 23 2012 - 01:29:53 EST


On 07/20/2012 08:33 PM, Michael S. Tsirkin wrote:
On Thu, Jul 05, 2012 at 06:29:54PM +0800, Jason Wang wrote:
This patch let the virtio_net driver can negotiate the number of queues it
wishes to use through control virtqueue and export an ethtool interface to let
use tweak it.

As current multiqueue virtio-net implementation has optimizations on per-cpu
virtuqueues, so only two modes were support:

- single queue pair mode
- multiple queue paris mode, the number of queues matches the number of vcpus

The single queue mode were used by default currently due to regression of
multiqueue mode in some test (especially in stream test).

Since virtio core does not support paritially deleting virtqueues, so during
mode switching the whole virtqueue were deleted and the driver would re-create
the virtqueues it would used.

btw. The queue number negotiating were defered to .ndo_open(), this is because
only after feature negotitaion could we send the command to control virtqueue
(as it may also use event index).

Signed-off-by: Jason Wang<jasowang@xxxxxxxxxx>
---
drivers/net/virtio_net.c | 171 ++++++++++++++++++++++++++++++++++---------
include/linux/virtio_net.h | 7 ++
2 files changed, 142 insertions(+), 36 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 7410187..3339eeb 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -88,6 +88,7 @@ struct receive_queue {

struct virtnet_info {
u16 num_queue_pairs; /* # of RX/TX vq pairs */
+ u16 total_queue_pairs;

struct send_queue *sq[MAX_QUEUES] ____cacheline_aligned_in_smp;
struct receive_queue *rq[MAX_QUEUES] ____cacheline_aligned_in_smp;
@@ -137,6 +138,8 @@ struct padded_vnet_hdr {
char padding[6];
};

+static const struct ethtool_ops virtnet_ethtool_ops;
+
static inline int txq_get_qnum(struct virtnet_info *vi, struct virtqueue *vq)
{
int ret = virtqueue_get_queue_index(vq);
@@ -802,22 +805,6 @@ static void virtnet_netpoll(struct net_device *dev)
}
#endif

-static int virtnet_open(struct net_device *dev)
-{
- struct virtnet_info *vi = netdev_priv(dev);
- int i;
-
- for (i = 0; i< vi->num_queue_pairs; i++) {
- /* Make sure we have some buffers: if oom use wq. */
- if (!try_fill_recv(vi->rq[i], GFP_KERNEL))
- queue_delayed_work(system_nrt_wq,
- &vi->rq[i]->refill, 0);
- virtnet_napi_enable(vi->rq[i]);
- }
-
- return 0;
-}
-
/*
* Send command via the control virtqueue and check status. Commands
* supported by the hypervisor, as indicated by feature bits, should
@@ -873,6 +860,43 @@ static void virtnet_ack_link_announce(struct virtnet_info *vi)
rtnl_unlock();
}

+static int virtnet_set_queues(struct virtnet_info *vi)
+{
+ struct scatterlist sg;
+ struct net_device *dev = vi->dev;
+ sg_init_one(&sg,&vi->num_queue_pairs, sizeof(vi->num_queue_pairs));
+
+ if (!vi->has_cvq)
+ return -EINVAL;
+
+ if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MULTIQUEUE,
+ VIRTIO_NET_CTRL_MULTIQUEUE_QNUM,&sg, 1, 0)){
+ dev_warn(&dev->dev, "Fail to set the number of queue pairs to"
+ " %d\n", vi->num_queue_pairs);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int virtnet_open(struct net_device *dev)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+ int i;
+
+ for (i = 0; i< vi->num_queue_pairs; i++) {
+ /* Make sure we have some buffers: if oom use wq. */
+ if (!try_fill_recv(vi->rq[i], GFP_KERNEL))
+ queue_delayed_work(system_nrt_wq,
+ &vi->rq[i]->refill, 0);
+ virtnet_napi_enable(vi->rq[i]);
+ }
+
+ virtnet_set_queues(vi);
+
+ return 0;
+}
+
static int virtnet_close(struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
@@ -1013,12 +1037,6 @@ static void virtnet_get_drvinfo(struct net_device *dev,

}

-static const struct ethtool_ops virtnet_ethtool_ops = {
- .get_drvinfo = virtnet_get_drvinfo,
- .get_link = ethtool_op_get_link,
- .get_ringparam = virtnet_get_ringparam,
-};
-
#define MIN_MTU 68
#define MAX_MTU 65535

@@ -1235,7 +1253,7 @@ static int virtnet_find_vqs(struct virtnet_info *vi)

err:
if (ret&& names)
- for (i = 0; i< vi->num_queue_pairs * 2; i++)
+ for (i = 0; i< total_vqs * 2; i++)
kfree(names[i]);

kfree(names);
@@ -1373,7 +1391,6 @@ static int virtnet_probe(struct virtio_device *vdev)
mutex_init(&vi->config_lock);
vi->config_enable = true;
INIT_WORK(&vi->config_work, virtnet_config_changed_work);
- vi->num_queue_pairs = num_queue_pairs;

/* If we can receive ANY GSO packets, we must allocate large ones. */
if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO4) ||
@@ -1387,6 +1404,10 @@ static int virtnet_probe(struct virtio_device *vdev)
if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
vi->has_cvq = true;

+ /* Use single tx/rx queue pair as default */
+ vi->num_queue_pairs = 1;
+ vi->total_queue_pairs = num_queue_pairs;
+
/* Allocate/initialize the rx/tx queues, and invoke find_vqs */
err = virtnet_setup_vqs(vi);
if (err)
@@ -1396,6 +1417,9 @@ static int virtnet_probe(struct virtio_device *vdev)
virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VLAN))
dev->features |= NETIF_F_HW_VLAN_FILTER;

+ netif_set_real_num_tx_queues(dev, 1);
+ netif_set_real_num_rx_queues(dev, 1);
+
err = register_netdev(dev);
if (err) {
pr_debug("virtio_net: registering device failed\n");
@@ -1403,7 +1427,7 @@ static int virtnet_probe(struct virtio_device *vdev)
}

/* Last of all, set up some receive buffers. */
- for (i = 0; i< num_queue_pairs; i++) {
+ for (i = 0; i< vi->num_queue_pairs; i++) {
try_fill_recv(vi->rq[i], GFP_KERNEL);

/* If we didn't even get one input buffer, we're useless. */
@@ -1474,10 +1498,8 @@ static void __devexit virtnet_remove(struct virtio_device *vdev)
free_netdev(vi->dev);
}

-#ifdef CONFIG_PM
-static int virtnet_freeze(struct virtio_device *vdev)
+static void virtnet_stop(struct virtnet_info *vi)
{
- struct virtnet_info *vi = vdev->priv;
int i;

/* Prevent config work handler from accessing the device */
@@ -1493,17 +1515,10 @@ static int virtnet_freeze(struct virtio_device *vdev)
for (i = 0; i< vi->num_queue_pairs; i++)
napi_disable(&vi->rq[i]->napi);

-
- remove_vq_common(vi);
-
- flush_work(&vi->config_work);
-
- return 0;
}

-static int virtnet_restore(struct virtio_device *vdev)
+static int virtnet_start(struct virtnet_info *vi)
{
- struct virtnet_info *vi = vdev->priv;
int err, i;

err = virtnet_setup_vqs(vi);
@@ -1527,6 +1542,29 @@ static int virtnet_restore(struct virtio_device *vdev)

return 0;
}
+
+#ifdef CONFIG_PM
+static int virtnet_freeze(struct virtio_device *vdev)
+{
+ struct virtnet_info *vi = vdev->priv;
+
+ virtnet_stop(vi);
+
+ remove_vq_common(vi);
+
+ flush_work(&vi->config_work);
+
+ return 0;
+}
+
+static int virtnet_restore(struct virtio_device *vdev)
+{
+ struct virtnet_info *vi = vdev->priv;
+
+ virtnet_start(vi);
+
+ return 0;
+}
#endif

static struct virtio_device_id id_table[] = {
@@ -1560,6 +1598,67 @@ static struct virtio_driver virtio_net_driver = {
#endif
};

+static int virtnet_set_channels(struct net_device *dev,
+ struct ethtool_channels *channels)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+ u16 queues = channels->rx_count;
+ unsigned status = VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER;
+
+ if (channels->rx_count != channels->tx_count)
+ return -EINVAL;
+ /* Only two modes were support currently */
s/were/are/ ?

Ok.

+ if (queues != vi->total_queue_pairs&& queues != 1)
+ return -EINVAL;
So userspace has to get queue number right. How does it know
what the valid value is?

Usespace could query the number through ethtool -l (virtnet_get_channels()).

+ if (!vi->has_cvq)
+ return -EINVAL;
+
+ virtnet_stop(vi);
+
+ netif_set_real_num_tx_queues(dev, queues);
+ netif_set_real_num_rx_queues(dev, queues);
+
+ remove_vq_common(vi);
+ flush_work(&vi->config_work);
+
+ vi->num_queue_pairs = queues;
+ virtnet_start(vi);
+
+ vi->vdev->config->finalize_features(vi->vdev);
+
+ if (virtnet_set_queues(vi))
+ status |= VIRTIO_CONFIG_S_FAILED;
+ else
+ status |= VIRTIO_CONFIG_S_DRIVER_OK;
+
+ vi->vdev->config->set_status(vi->vdev, status);
+
Why do we need to tweak status like that?

Because remove_vq_common() reset the device. Since virtio core api does not support remove a specified number of virtqueues, it's the only method when we change the number of queues.
Can we maybe just roll changes back on error?

Not easy, we reset and detroy previous virtqueues and create new ones.
+ return 0;
+}
+
+static void virtnet_get_channels(struct net_device *dev,
+ struct ethtool_channels *channels)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+
+ channels->max_rx = vi->total_queue_pairs;
+ channels->max_tx = vi->total_queue_pairs;
+ channels->max_other = 0;
+ channels->max_combined = 0;
+ channels->rx_count = vi->num_queue_pairs;
+ channels->tx_count = vi->num_queue_pairs;
+ channels->other_count = 0;
+ channels->combined_count = 0;
+}
+
+static const struct ethtool_ops virtnet_ethtool_ops = {
+ .get_drvinfo = virtnet_get_drvinfo,
+ .get_link = ethtool_op_get_link,
+ .get_ringparam = virtnet_get_ringparam,
+ .set_channels = virtnet_set_channels,
+ .get_channels = virtnet_get_channels,
+};
+
static int __init init(void)
{
return register_virtio_driver(&virtio_net_driver);
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 60f09ff..0d21e08 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -169,4 +169,11 @@ struct virtio_net_ctrl_mac {
#define VIRTIO_NET_CTRL_ANNOUNCE 3
#define VIRTIO_NET_CTRL_ANNOUNCE_ACK 0

+/*
+ * Control multiqueue
+ *
+ */
+#define VIRTIO_NET_CTRL_MULTIQUEUE 4
+ #define VIRTIO_NET_CTRL_MULTIQUEUE_QNUM 0
+
#endif /* _LINUX_VIRTIO_NET_H */
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/