Re: [rfc net-next v6 2/3] virtio_net: multiqueue support
On Sat, Nov 17, 2012 at 12:35:29AM +, Ben Hutchings wrote:
On Tue, 2012-11-13 at 08:40 +0200, Michael S. Tsirkin wrote:
On Mon, Nov 05, 2012 at 11:38:39AM +1030, Rusty Russell wrote:
@@ -924,11 +1032,10 @@ static void virtnet_get_ringparam(struct
net_device *dev,
{
struct virtnet_info *vi = netdev_priv(dev);
- ring-rx_max_pending = virtqueue_get_vring_size(vi-rvq);
- ring-tx_max_pending = virtqueue_get_vring_size(vi-svq);
+ ring-rx_max_pending = virtqueue_get_vring_size(vi-rq[0].vq);
+ ring-tx_max_pending = virtqueue_get_vring_size(vi-sq[0].vq);
ring-rx_pending = ring-rx_max_pending;
ring-tx_pending = ring-tx_max_pending;
-
}
This assumes all vqs are the same size. I think this should probably
check: for mq mode, use the first vq, otherewise use the 0th.
For rx_pending/tx_pending I think what is required here is the
actual number of outstanding buffers.
Dave, Eric - right?
So this should be the total over all rings and to be useful,
rx_max_pending/tx_max_pending should be the total too.
So far as I know, all current implementations use the number of
descriptors per ring here. virtio_net should be consistent with this.
Ben.
Problem is, it could in theory be different between rings. I guess we
could use the maximum.
What's the right thing to do for rx_pending - I am guessing
we want the current outstanding packets right?
For bonus points, check this assertion at probe time.
Looks like we can easily support different queues too.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug 42980] BUG in gfn_to_pfn_prot
On 11/18/2012 12:10 AM, bugzilla-dae...@bugzilla.kernel.org wrote: https://bugzilla.kernel.org/show_bug.cgi?id=42980 --- Comment #18 from Ian Pilcher arequip...@gmail.com 2012-11-17 22:10:45 --- (In reply to comment #11) A patch referencing this bug report has been merged in Linux v3.5-rc1: commit d8368af8b46b904def42a0f341d2f4f29001fa77 Author: Avi Kivity a...@redhat.com Date: Mon May 14 18:07:56 2012 +0300 KVM: Fix mmu_reload() clash with nested vmx event injection Silly question. Is this patch applicable to the physical host, the L1 guest (virtualized hypervisor), or both? The physical host. If you want to run a hypervisor in L2, you need to apply it to L1 as well. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 42980] BUG in gfn_to_pfn_prot
https://bugzilla.kernel.org/show_bug.cgi?id=42980 --- Comment #19 from Avi Kivity a...@redhat.com 2012-11-18 14:15:41 --- On 11/18/2012 12:10 AM, bugzilla-dae...@bugzilla.kernel.org wrote: https://bugzilla.kernel.org/show_bug.cgi?id=42980 --- Comment #18 from Ian Pilcher arequip...@gmail.com 2012-11-17 22:10:45 --- (In reply to comment #11) A patch referencing this bug report has been merged in Linux v3.5-rc1: commit d8368af8b46b904def42a0f341d2f4f29001fa77 Author: Avi Kivity a...@redhat.com Date: Mon May 14 18:07:56 2012 +0300 KVM: Fix mmu_reload() clash with nested vmx event injection Silly question. Is this patch applicable to the physical host, the L1 guest (virtualized hypervisor), or both? The physical host. If you want to run a hypervisor in L2, you need to apply it to L1 as well. -- Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are watching the assignee of the bug. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: vmexit cycle cost
On 11/16/2012 06:56 PM, Christoffer Dall wrote: Hi all, just a quick question: using the kvm-unittest vmexit test, it seems using a VMCALL for exit, a round-trip to the host takes roughly 1000 cycles on a modern processor. This number sounds quite low to me. Can anyone verify if it's reasonable or have more data on this number has developed over time given newer processors? It's reasonable, not low (this number can never be low enough). -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 42980] BUG in gfn_to_pfn_prot
https://bugzilla.kernel.org/show_bug.cgi?id=42980 --- Comment #20 from Ian Pilcher arequip...@gmail.com 2012-11-18 17:06:36 --- (In reply to comment #19) The physical host. If you want to run a hypervisor in L2, you need to apply it to L1 as well. OK. If I'm parsing that correctly, it sounds like backporting the patch to the RHEL 6 kernel, so I could run it in the L1 hypervisors, wouldn't help anything. Bummer. Any ideas on how I can make this environment stable? I see that Luke-Jr is also on a DQ67SW, and he's doing PCI passthrough. I do have VT-d enabled, although I'm not actually doing any PCI-passthrough. I that something that could be related to this? -- Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are watching the assignee of the bug. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [rfc net-next v6 2/3] virtio_net: multiqueue support
On 11/05/2012 07:16 AM, Rusty Russell wrote:
Jason Wang jasow...@redhat.com writes:
This addes multiqueue support to virtio_net driver. There's two mode supported:
single queue pair mode and multiple queue pairs mode. An obvious
difference compared with a physical mq card is that virtio-net reserve
first two virtqueues when it is working in multiqueue mode, this is
used for implementing adaptive mode switching in the future. The
virtqueues that were in both mq and sq mode were initialized and only
one queue pair (single queue mode) were used at default. User could
use ethtool -L to switch to multiqueue mode withe the next patch.
Hi Jason,
This first patch looks good, but conflates three things
together:
(1) Separate per-queue structures from struct virtnet_info to allow
multiple queues. This is the mechanical part of the patch.
(2) An annotation bugfix, see below.
(3) Enabling mq using a new feature and negotiation.
Hi Rusty:
Sorry for the late response, just back from vacation.
For 1 and 3, I will split the patch as you suggested.
For 2, will fix it.
Thanks
@@ -700,7 +767,8 @@ static struct rtnl_link_stats64 *virtnet_stats(struct
net_device *dev,
unsigned int start;
for_each_possible_cpu(cpu) {
- struct virtnet_stats *stats = per_cpu_ptr(vi-stats, cpu);
+ struct virtnet_stats __percpu *stats
+ = per_cpu_ptr(vi-stats, cpu);
u64 tpackets, tbytes, rpackets, rbytes;
do {
Cheers,
Rusty.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [rfc net-next v6 3/3] virtio-net: change the number of queues through ethtool
On 11/05/2012 07:46 AM, Rusty Russell wrote:
Jason Wang jasow...@redhat.com writes:
This patch implement the {set|get}_channels method of ethool to allow user to
change the number of queues dymaically when the device is running. This would
let the user to tune the device for specific applications.
...
+ /* Only two modes were support currently */
+ if (queue_pairs == 0)
+ return -EINVAL;
+ if (queue_pairs != vi-total_queue_pairs - 1 queue_pairs != 1)
+ return -EINVAL;
OK, so you let them do all or nothing, but this three-way test is
pretty unclear.
True, looks like the first check could be removed.
In fact, the whole total_queue_pairs/num_queue_pairs thing is weird (and
uncommented). I think for total you mean max; the maximum possible
queue pair number.
Yes, total means max, will add a comment or change the name to
max_queue_pairs/current_queue_pairs.
Let me go back and review the previous patch again...
Cheers,
Rusty.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [rfc net-next v6 2/3] virtio_net: multiqueue support
On 11/05/2012 09:08 AM, Rusty Russell wrote:
Jason Wang jasow...@redhat.com writes:
+struct virtnet_info {
+ u16 num_queue_pairs;/* # of RX/TX vq pairs */
+ u16 total_queue_pairs;
+
+ struct send_queue *sq;
+ struct receive_queue *rq;
+ struct virtqueue *cvq;
+
+ struct virtio_device *vdev;
+ struct net_device *dev;
+ unsigned int status;
status seems unused?
It's used for tacking the status of the device (e.g in
virtnet_config_changed_work() ).
+static const struct ethtool_ops virtnet_ethtool_ops;
Strange hoist, but I can't tell from the patch if this is necessary.
Assume it is.
Sorry, this line should belong to patch 3/3.
+static inline int vq2txq(struct virtqueue *vq)
+{
+ int index = virtqueue_get_queue_index(vq);
+ return index == 1 ? 0 : (index - 3) / 2;
+}
+
+static inline int txq2vq(int txq)
+{
+ return txq ? 2 * txq + 3 : 1;
+}
+
+static inline int vq2rxq(struct virtqueue *vq)
+{
+ int index = virtqueue_get_queue_index(vq);
+ return index ? (index - 2) / 2 : 0;
+}
+
+static inline int rxq2vq(int rxq)
+{
+ return rxq ? 2 * rxq + 2 : 0;
+}
+
static inline struct skb_vnet_hdr *skb_vnet_hdr(struct sk_buff *skb)
I know skb_vnet_hdr() does it, but I generally dislike inline in C
files; gcc is generally smart enough these days, and inline suppresses
unused function warnings.
Ok, I will remove the inline here.
I guess these mappings have to work even when we're switching from mq to
single queue mode; otherwise we could simplify them using a 'bool mq'
flag.
Yes, it still work when switching to sq. And what makes it looks strange
is because we reserve the virtqueues for single queue mode and also
reserve vq 3. But it does not bring much benefit, need more thought.
+static int virtnet_set_queues(struct virtnet_info *vi)
+{
+ struct scatterlist sg;
+ struct virtio_net_ctrl_steering s;
+ struct net_device *dev = vi-dev;
+
+ if (vi-num_queue_pairs == 1) {
+ s.current_steering_rule = VIRTIO_NET_CTRL_STEERING_SINGLE;
+ s.current_steering_param = 1;
+ } else {
+ s.current_steering_rule =
+ VIRTIO_NET_CTRL_STEERING_RX_FOLLOWS_TX;
+ s.current_steering_param = vi-num_queue_pairs;
+ }
(BTW, VIRTIO_NET_CTRL_STEERING_RX_FOLLOWS_TX etc not defined anywhere?)
It's defined in include/uapi/linux/virtio_net.h
Hmm, it's not clear that anything other than RX_FOLLOWS_TX will ever
make sense, so this is really just turning mq on and off.
Currently, when multiqueue is enabled for tuntap, it does tx follow rx.
So when guest driver specify the RX_FOLLOWS_TX, qemu would just enable
multiqueue for tuntap and this policy could be done by tuntap.
Unfortunately, we can't turn feature bits on and off after startup, so
if we want this level of control (and I think we do), there does need to
be a mechanism.
Michael? I'd prefer this to be further simplfied, to just
disable/enable. We can extend it later, but for now the second
parameter is redundant, ie.:
struct virtio_net_ctrl_steering {
u8 mode; /* 0 == off, 1 == on */
} __attribute__((packed));
We may need more policy in the future, so maybe a
VIRTIO_NET_CTRL_STEERING_NONE is ok?
@@ -924,11 +1032,10 @@ static void virtnet_get_ringparam(struct net_device *dev,
{
struct virtnet_info *vi = netdev_priv(dev);
- ring-rx_max_pending = virtqueue_get_vring_size(vi-rvq);
- ring-tx_max_pending = virtqueue_get_vring_size(vi-svq);
+ ring-rx_max_pending = virtqueue_get_vring_size(vi-rq[0].vq);
+ ring-tx_max_pending = virtqueue_get_vring_size(vi-sq[0].vq);
ring-rx_pending = ring-rx_max_pending;
ring-tx_pending = ring-tx_max_pending;
-
}
This assumes all vqs are the same size. I think this should probably
check: for mq mode, use the first vq, otherewise use the 0th.
Ok, but I don't see the reason that we need different size for mq.
For bonus points, check this assertion at probe time.
+ /*
+* We expect 1 RX virtqueue followed by 1 TX virtqueue, followd by
+* possible control virtqueue, followed by 1 reserved vq, followed
+* by RX/TX queue pairs used in multiqueue mode.
+*/
+ if (vi-total_queue_pairs == 1)
+ total_vqs = 2 + virtio_has_feature(vi-vdev,
+ VIRTIO_NET_F_CTRL_VQ);
+ else
+ total_vqs = 2 * vi-total_queue_pairs + 2;
What's the allergy to odd numbers? Why the reserved queue?
It was suggested by Michael to let the vq calculation easier, but it
seems does not help much. So it's better not reserve virtqueue in next
version.
+ 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;
+
