Re: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Fri, 14 Aug 2009 12:23:46 am Arnd Bergmann wrote: On Thursday 13 August 2009, Michael S. Tsirkin wrote: The best way to do this IMO would be to add zero copy support to raw sockets, vhost will then get it basically for free. Yes, that would be nice. I wonder if that could lead to security problems on TX though. I guess It will only bring significant performance improvements if we leave the data writable in the user space or guest during the operation. If the user finds the right timing, it could modify the frame headers after they have been checked using netfilter, or while the frames are being consumed in the kernel (e.g. an NFS server running in a guest). For this reason, we always linearize parts of packets we're filtering. ie. copy. 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
Re: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Wed, Aug 12, 2009 at 07:59:47PM +0200, Arnd Bergmann wrote: The trick is to swap the virtqueues instead. virtio-net is actually mostly symmetric in just the same way that the physical wires on a twisted pair ethernet are symmetric (I like how that analogy fits). You need to really squint hard for it to look symmetric. For example, for RX, virtio allocates an skb, puts a descriptor on a ring and waits for host to fill it in. Host system can not do the same: guest does not have access to host memory. You can do a copy in transport to hide this fact, but it will kill performance. -- MST -- 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: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Wed, Aug 12, 2009 at 02:27:31PM -0500, Anthony Liguori wrote: Arnd Bergmann wrote: As I pointed out earlier, most code in virtio net is asymmetrical: guest provides buffers, host consumes them. Possibly, one could use virtio rings in a symmetrical way, but support of existing guest virtio net means there's almost no shared code. The trick is to swap the virtqueues instead. virtio-net is actually mostly symmetric in just the same way that the physical wires on a twisted pair ethernet are symmetric (I like how that analogy fits). It's already been done between two guests. See http://article.gmane.org/gmane.linux.kernel.virtualization/5423 Regards, Anthony Liguori Yes, this works by copying data (see PATCH 5/5). Another possibility is page flipping. Either will kill performance. -- MST -- 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: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Wed, Aug 12, 2009 at 02:22:38PM -0500, Anthony Liguori wrote: Michael S. Tsirkin wrote: We discussed this before, and I still think this could be directly derived from struct virtqueue, in the same way that vring_virtqueue is derived from struct virtqueue. I prefer keeping it simple. Much of abstraction in virtio is due to the fact that it needs to work on top of different hardware emulations: lguest,kvm, possibly others in the future. vhost is always working on real hardware, using eventfd as the interface, so it does not need that. Actually, vhost may not always be limited to real hardware. Yes, any ethernet device will do. What I mean is that vhost does not deal with emulation at all. All setup is done in userspace. We may on day use vhost as the basis of a driver domain. There's quite a lot of interest in this for networking. You can use veth for this. This works today. At any rate, I'd like to see performance results before we consider trying to reuse virtio code. Regards, Anthony Liguori -- 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: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Wednesday 12 August 2009, Anthony Liguori wrote: At any rate, I'd like to see performance results before we consider trying to reuse virtio code. Yes, I agree. I'd also like to do more work on the macvlan extensions to see if it works out without involving a socket. Passing the socket into the vhost_net device is a nice feature of the current implementation that we'd have to give up for something else (e.g. making the vhost a real network interface that you can hook up to a bridge) if it were to use virtio. Unless I can come up with a solution that is clearly superior, I'm taking back my objections on that part for now. Arnd -- 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: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Thursday 13 August 2009, Arnd Bergmann wrote: Unfortunately, this also implies that you could no longer simply use the packet socket interface as you do currently, as I realized only now. This obviously has a significant impact on your user space interface. Also, if we do the copy in the transport, it definitely means that we can't get to zero-copy RX/TX from guest space any more. The current vhost_net driver doesn't do that yet, but could be extended in the same way that I'm hoping to do it for macvtap. Arnd -- 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: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Thursday 13 August 2009, Michael S. Tsirkin wrote: On Wed, Aug 12, 2009 at 07:59:47PM +0200, Arnd Bergmann wrote: The trick is to swap the virtqueues instead. virtio-net is actually mostly symmetric in just the same way that the physical wires on a twisted pair ethernet are symmetric (I like how that analogy fits). You need to really squint hard for it to look symmetric. For example, for RX, virtio allocates an skb, puts a descriptor on a ring and waits for host to fill it in. Host system can not do the same: guest does not have access to host memory. You can do a copy in transport to hide this fact, but it will kill performance. Yes, that is what I was suggesting all along. The actual copy operation has to be done by the host transport, which is obviously different from the guest transport that just calls the host using vring_kick(). Right now, the number of copy operations in your code is the same. You are doing the copy a little bit later in skb_copy_datagram_iovec(), which is indeed a very nice hack. Changing to a virtqueue based method would imply that the host needs to add each skb_frag_t to its outbound virtqueue, which then gets copied into the guests inbound virtqueue. Unfortunately, this also implies that you could no longer simply use the packet socket interface as you do currently, as I realized only now. This obviously has a significant impact on your user space interface. Arnd -- 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: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Thu, Aug 13, 2009 at 03:38:43PM +0200, Arnd Bergmann wrote: On Thursday 13 August 2009, Michael S. Tsirkin wrote: On Wed, Aug 12, 2009 at 07:59:47PM +0200, Arnd Bergmann wrote: The trick is to swap the virtqueues instead. virtio-net is actually mostly symmetric in just the same way that the physical wires on a twisted pair ethernet are symmetric (I like how that analogy fits). You need to really squint hard for it to look symmetric. For example, for RX, virtio allocates an skb, puts a descriptor on a ring and waits for host to fill it in. Host system can not do the same: guest does not have access to host memory. You can do a copy in transport to hide this fact, but it will kill performance. Yes, that is what I was suggesting all along. The actual copy operation has to be done by the host transport, which is obviously different from the guest transport that just calls the host using vring_kick(). Right now, the number of copy operations in your code is the same. You are doing the copy a little bit later in skb_copy_datagram_iovec(), which is indeed a very nice hack. Changing to a virtqueue based method would imply that the host needs to add each skb_frag_t to its outbound virtqueue, which then gets copied into the guests inbound virtqueue. Which is a lot more code than just calling skb_copy_datagram_iovec. Unfortunately, this also implies that you could no longer simply use the packet socket interface as you do currently, as I realized only now. This obviously has a significant impact on your user space interface. Arnd And, it will remove our ability to implement zero copy down the road (when raw sockets support it). -- MST -- 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: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Thu, Aug 13, 2009 at 03:48:35PM +0200, Arnd Bergmann wrote: On Thursday 13 August 2009, Arnd Bergmann wrote: Unfortunately, this also implies that you could no longer simply use the packet socket interface as you do currently, as I realized only now. This obviously has a significant impact on your user space interface. Also, if we do the copy in the transport, it definitely means that we can't get to zero-copy RX/TX from guest space any more. The current vhost_net driver doesn't do that yet, but could be extended in the same way that I'm hoping to do it for macvtap. Arnd The best way to do this IMO would be to add zero copy support to raw sockets, vhost will then get it basically for free. -- MST -- 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: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Thursday 13 August 2009, Michael S. Tsirkin wrote: The best way to do this IMO would be to add zero copy support to raw sockets, vhost will then get it basically for free. Yes, that would be nice. I wonder if that could lead to security problems on TX though. I guess It will only bring significant performance improvements if we leave the data writable in the user space or guest during the operation. If the user finds the right timing, it could modify the frame headers after they have been checked using netfilter, or while the frames are being consumed in the kernel (e.g. an NFS server running in a guest). Ardn -- 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: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Thursday 13 August 2009, Michael S. Tsirkin wrote: Right now, the number of copy operations in your code is the same. You are doing the copy a little bit later in skb_copy_datagram_iovec(), which is indeed a very nice hack. Changing to a virtqueue based method would imply that the host needs to add each skb_frag_t to its outbound virtqueue, which then gets copied into the guests inbound virtqueue. Which is a lot more code than just calling skb_copy_datagram_iovec. Well, I don't see this part as much of a problem, because the code already exists in virtio_net. If we really wanted to go down that road, just using virtio_net would solve the problem of frame handling entirely, but create new problems elsewhere, as we have mentioned. Arnd -- 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: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Thu, Aug 13, 2009 at 04:58:06PM +0200, Arnd Bergmann wrote: On Thursday 13 August 2009, Michael S. Tsirkin wrote: Right now, the number of copy operations in your code is the same. You are doing the copy a little bit later in skb_copy_datagram_iovec(), which is indeed a very nice hack. Changing to a virtqueue based method would imply that the host needs to add each skb_frag_t to its outbound virtqueue, which then gets copied into the guests inbound virtqueue. Which is a lot more code than just calling skb_copy_datagram_iovec. Well, I don't see this part as much of a problem, because the code already exists in virtio_net. I am talking about the copying done in low level transport, here. If we really wanted to go down that road, just using virtio_net would solve the problem of frame handling entirely, but create new problems elsewhere, as we have mentioned. Arnd -- 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: [PATCH 2/2] vhost_net: a kernel-level virtio server
On 08/13/2009 05:53 PM, Arnd Bergmann wrote: On Thursday 13 August 2009, Michael S. Tsirkin wrote: The best way to do this IMO would be to add zero copy support to raw sockets, vhost will then get it basically for free. Yes, that would be nice. I wonder if that could lead to security problems on TX though. I guess It will only bring significant performance improvements if we leave the data writable in the user space or guest during the operation. If the user finds the right timing, it could modify the frame headers after they have been checked using netfilter, or while the frames are being consumed in the kernel (e.g. an NFS server running in a guest). IIRC when the kernel consumes data it linearizes the skb. We just need to make sure all the zerocopy data is in the nonlinear part, and the kernel will copy if/when it needs to access packet data. -- 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
Re: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Monday 10 August 2009, Michael S. Tsirkin wrote:
+struct workqueue_struct *vhost_workqueue;
[nitpicking] This could be static.
+/* The virtqueue structure describes a queue attached to a device. */
+struct vhost_virtqueue {
+ struct vhost_dev *dev;
+
+ /* The actual ring of buffers. */
+ struct mutex mutex;
+ unsigned int num;
+ struct vring_desc __user *desc;
+ struct vring_avail __user *avail;
+ struct vring_used __user *used;
+ struct file *kick;
+ struct file *call;
+ struct file *error;
+ struct eventfd_ctx *call_ctx;
+ struct eventfd_ctx *error_ctx;
+
+ struct vhost_poll poll;
+
+ /* The routine to call when the Guest pings us, or timeout. */
+ work_func_t handle_kick;
+
+ /* Last available index we saw. */
+ u16 last_avail_idx;
+
+ /* Last index we used. */
+ u16 last_used_idx;
+
+ /* Outstanding buffers */
+ unsigned int inflight;
+
+ /* Is this blocked? */
+ bool blocked;
+
+ struct iovec iov[VHOST_NET_MAX_SG];
+
+} cacheline_aligned;
We discussed this before, and I still think this could be directly derived
from struct virtqueue, in the same way that vring_virtqueue is derived from
struct virtqueue. That would make it possible for simple device drivers
to use the same driver in both host and guest, similar to how Ira Snyder
used virtqueues to make virtio_net run between two hosts running the
same code [1].
Ideally, I guess you should be able to even make virtio_net work in the
host if you do that, but that could bring other complexities.
Arnd
[1] http://lkml.org/lkml/2009/2/23/353
--
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: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Wed, Aug 12, 2009 at 07:03:22PM +0200, Arnd Bergmann wrote:
On Monday 10 August 2009, Michael S. Tsirkin wrote:
+struct workqueue_struct *vhost_workqueue;
[nitpicking] This could be static.
+/* The virtqueue structure describes a queue attached to a device. */
+struct vhost_virtqueue {
+ struct vhost_dev *dev;
+
+ /* The actual ring of buffers. */
+ struct mutex mutex;
+ unsigned int num;
+ struct vring_desc __user *desc;
+ struct vring_avail __user *avail;
+ struct vring_used __user *used;
+ struct file *kick;
+ struct file *call;
+ struct file *error;
+ struct eventfd_ctx *call_ctx;
+ struct eventfd_ctx *error_ctx;
+
+ struct vhost_poll poll;
+
+ /* The routine to call when the Guest pings us, or timeout. */
+ work_func_t handle_kick;
+
+ /* Last available index we saw. */
+ u16 last_avail_idx;
+
+ /* Last index we used. */
+ u16 last_used_idx;
+
+ /* Outstanding buffers */
+ unsigned int inflight;
+
+ /* Is this blocked? */
+ bool blocked;
+
+ struct iovec iov[VHOST_NET_MAX_SG];
+
+} cacheline_aligned;
We discussed this before, and I still think this could be directly derived
from struct virtqueue, in the same way that vring_virtqueue is derived from
struct virtqueue. That would make it possible for simple device drivers
to use the same driver in both host and guest, similar to how Ira Snyder
used virtqueues to make virtio_net run between two hosts running the
same code [1].
Ideally, I guess you should be able to even make virtio_net work in the
host if you do that, but that could bring other complexities.
I have no comments about the vhost code itself, I haven't reviewed it.
It might be interesting to try using a virtio-net in the host kernel to
communicate with the virtio-net running in the guest kernel. The lack of
a management interface is the biggest problem you will face (setting MAC
addresses, negotiating features, etc. doesn't work intuitively). Getting
the network interfaces talking is relatively easy.
Ira
--
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: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Wed, Aug 12, 2009 at 07:03:22PM +0200, Arnd Bergmann wrote:
On Monday 10 August 2009, Michael S. Tsirkin wrote:
+struct workqueue_struct *vhost_workqueue;
[nitpicking] This could be static.
Good catch. Thanks!
+/* The virtqueue structure describes a queue attached to a device. */
+struct vhost_virtqueue {
+ struct vhost_dev *dev;
+
+ /* The actual ring of buffers. */
+ struct mutex mutex;
+ unsigned int num;
+ struct vring_desc __user *desc;
+ struct vring_avail __user *avail;
+ struct vring_used __user *used;
+ struct file *kick;
+ struct file *call;
+ struct file *error;
+ struct eventfd_ctx *call_ctx;
+ struct eventfd_ctx *error_ctx;
+
+ struct vhost_poll poll;
+
+ /* The routine to call when the Guest pings us, or timeout. */
+ work_func_t handle_kick;
+
+ /* Last available index we saw. */
+ u16 last_avail_idx;
+
+ /* Last index we used. */
+ u16 last_used_idx;
+
+ /* Outstanding buffers */
+ unsigned int inflight;
+
+ /* Is this blocked? */
+ bool blocked;
+
+ struct iovec iov[VHOST_NET_MAX_SG];
+
+} cacheline_aligned;
We discussed this before, and I still think this could be directly derived
from struct virtqueue, in the same way that vring_virtqueue is derived from
struct virtqueue.
I prefer keeping it simple. Much of abstraction in virtio is due to the
fact that it needs to work on top of different hardware emulations:
lguest,kvm, possibly others in the future. vhost is always working on
real hardware, using eventfd as the interface, so it does not need that.
That would make it possible for simple device drivers
to use the same driver in both host and guest,
I don't think so. For example, there's a callback field that gets
invoked in guest when buffers are consumed. It could be overloaded to
mean buffers are available in host but you never handle both
situations in the same way, so what's the point?
similar to how Ira Snyder used virtqueues to make virtio_net run
between two hosts running the same code [1].
Ideally, I guess you should be able to even make virtio_net work in the
host if you do that, but that could bring other complexities.
Arnd
[1] http://lkml.org/lkml/2009/2/23/353
As I pointed out earlier, most code in virtio net is asymmetrical: guest
provides buffers, host consumes them. Possibly, one could use virtio
rings in a symmetrical way, but support of existing guest virtio net
means there's almost no shared code.
--
MST
--
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: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Wed, Aug 12, 2009 at 10:19:22AM -0700, Ira W. Snyder wrote:
On Wed, Aug 12, 2009 at 07:03:22PM +0200, Arnd Bergmann wrote:
On Monday 10 August 2009, Michael S. Tsirkin wrote:
+struct workqueue_struct *vhost_workqueue;
[nitpicking] This could be static.
+/* The virtqueue structure describes a queue attached to a device. */
+struct vhost_virtqueue {
+ struct vhost_dev *dev;
+
+ /* The actual ring of buffers. */
+ struct mutex mutex;
+ unsigned int num;
+ struct vring_desc __user *desc;
+ struct vring_avail __user *avail;
+ struct vring_used __user *used;
+ struct file *kick;
+ struct file *call;
+ struct file *error;
+ struct eventfd_ctx *call_ctx;
+ struct eventfd_ctx *error_ctx;
+
+ struct vhost_poll poll;
+
+ /* The routine to call when the Guest pings us, or timeout. */
+ work_func_t handle_kick;
+
+ /* Last available index we saw. */
+ u16 last_avail_idx;
+
+ /* Last index we used. */
+ u16 last_used_idx;
+
+ /* Outstanding buffers */
+ unsigned int inflight;
+
+ /* Is this blocked? */
+ bool blocked;
+
+ struct iovec iov[VHOST_NET_MAX_SG];
+
+} cacheline_aligned;
We discussed this before, and I still think this could be directly derived
from struct virtqueue, in the same way that vring_virtqueue is derived from
struct virtqueue. That would make it possible for simple device drivers
to use the same driver in both host and guest, similar to how Ira Snyder
used virtqueues to make virtio_net run between two hosts running the
same code [1].
Ideally, I guess you should be able to even make virtio_net work in the
host if you do that, but that could bring other complexities.
I have no comments about the vhost code itself, I haven't reviewed it.
It might be interesting to try using a virtio-net in the host kernel to
communicate with the virtio-net running in the guest kernel. The lack of
a management interface is the biggest problem you will face (setting MAC
addresses, negotiating features, etc. doesn't work intuitively).
That was one of the reasons I decided to move most of code out to
userspace. My kernel driver only handles datapath,
it's much smaller than virtio net.
Getting
the network interfaces talking is relatively easy.
Ira
Tried this, but
- guest memory isn't pinned, so copy_to_user
to access it, errors need to be handled in a sane way
- used/available roles are reversed
- kick/interrupt roles are reversed
So most of the code then looks like
if (host) {
} else {
}
return
The only common part is walking the descriptor list,
but that's like 10 lines of code.
At which point it's better to keep host/guest code separate, IMO.
--
MST
--
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: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Wed, Aug 12, 2009 at 08:31:04PM +0300, Michael S. Tsirkin wrote:
On Wed, Aug 12, 2009 at 10:19:22AM -0700, Ira W. Snyder wrote:
[ snip out code ]
We discussed this before, and I still think this could be directly derived
from struct virtqueue, in the same way that vring_virtqueue is derived
from
struct virtqueue. That would make it possible for simple device drivers
to use the same driver in both host and guest, similar to how Ira Snyder
used virtqueues to make virtio_net run between two hosts running the
same code [1].
Ideally, I guess you should be able to even make virtio_net work in the
host if you do that, but that could bring other complexities.
I have no comments about the vhost code itself, I haven't reviewed it.
It might be interesting to try using a virtio-net in the host kernel to
communicate with the virtio-net running in the guest kernel. The lack of
a management interface is the biggest problem you will face (setting MAC
addresses, negotiating features, etc. doesn't work intuitively).
That was one of the reasons I decided to move most of code out to
userspace. My kernel driver only handles datapath,
it's much smaller than virtio net.
Getting
the network interfaces talking is relatively easy.
Ira
Tried this, but
- guest memory isn't pinned, so copy_to_user
to access it, errors need to be handled in a sane way
- used/available roles are reversed
- kick/interrupt roles are reversed
So most of the code then looks like
if (host) {
} else {
}
return
The only common part is walking the descriptor list,
but that's like 10 lines of code.
At which point it's better to keep host/guest code separate, IMO.
Ok, that makes sense. Let me see if I understand the concept of the
driver. Here's a picture of what makes sense to me:
guest system
-
| userspace applications|
-
| kernel network stack |
-
| virtio-net|
-
| transport (virtio-ring, etc.) |
-
|
|
-
| transport (virtio-ring, etc.) |
-
| some driver (maybe vhost?)| -- [1]
-
| kernel network stack |
-
host system
From the host's network stack, packets can be forwarded out to the
physical network, or be consumed by a normal userspace application on
the host. Just as if this were any other network interface.
In my patch, [1] was the virtio-net driver, completely unmodified.
So, does this patch accomplish the above diagram? If so, why the
copy_to_user(), etc? Maybe I'm confusing this with my system, where the
guest is another physical system, separated by the PCI bus.
Ira
--
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: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Wednesday 12 August 2009, Michael S. Tsirkin wrote: On Wed, Aug 12, 2009 at 07:03:22PM +0200, Arnd Bergmann wrote: We discussed this before, and I still think this could be directly derived from struct virtqueue, in the same way that vring_virtqueue is derived from struct virtqueue. I prefer keeping it simple. Much of abstraction in virtio is due to the fact that it needs to work on top of different hardware emulations: lguest,kvm, possibly others in the future. vhost is always working on real hardware, using eventfd as the interface, so it does not need that. Well, that was my point: virtio can already work on a number of abstractions, so adding one more for vhost should not be too hard. That would make it possible for simple device drivers to use the same driver in both host and guest, I don't think so. For example, there's a callback field that gets invoked in guest when buffers are consumed. It could be overloaded to mean buffers are available in host but you never handle both situations in the same way, so what's the point? ... As I pointed out earlier, most code in virtio net is asymmetrical: guest provides buffers, host consumes them. Possibly, one could use virtio rings in a symmetrical way, but support of existing guest virtio net means there's almost no shared code. The trick is to swap the virtqueues instead. virtio-net is actually mostly symmetric in just the same way that the physical wires on a twisted pair ethernet are symmetric (I like how that analogy fits). virtio_net kicks the transmit virtqueue when it has data and it kicks the receive queue when it has empty buffers to fill, and it has callbacks when the two are done. You can do the same in both the guest and the host, but then the guests input virtqueue is the hosts output virtqueue and vice versa. Once a virtqueue got kicked from both sides, the vhost_virtqueue implementation between the two only needs to do a copy_from_user or copy_to_user (possibly from a thread if it is in atomic context) and then call the two callback functions. This is basically the same thing you do already, except that you use slightly different names for the components. Arnd -- 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: [PATCH 2/2] vhost_net: a kernel-level virtio server
Michael S. Tsirkin wrote: We discussed this before, and I still think this could be directly derived from struct virtqueue, in the same way that vring_virtqueue is derived from struct virtqueue. I prefer keeping it simple. Much of abstraction in virtio is due to the fact that it needs to work on top of different hardware emulations: lguest,kvm, possibly others in the future. vhost is always working on real hardware, using eventfd as the interface, so it does not need that. Actually, vhost may not always be limited to real hardware. We may on day use vhost as the basis of a driver domain. There's quite a lot of interest in this for networking. At any rate, I'd like to see performance results before we consider trying to reuse virtio code. Regards, Anthony Liguori -- 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: [PATCH 2/2] vhost_net: a kernel-level virtio server
Arnd Bergmann wrote: As I pointed out earlier, most code in virtio net is asymmetrical: guest provides buffers, host consumes them. Possibly, one could use virtio rings in a symmetrical way, but support of existing guest virtio net means there's almost no shared code. The trick is to swap the virtqueues instead. virtio-net is actually mostly symmetric in just the same way that the physical wires on a twisted pair ethernet are symmetric (I like how that analogy fits). It's already been done between two guests. See http://article.gmane.org/gmane.linux.kernel.virtualization/5423 Regards, Anthony Liguori -- 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: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Mon, Aug 10, 2009 at 09:53:40PM +0300, Michael S. Tsirkin wrote:
What it is: vhost net is a character device that can be used to reduce
the number of system calls involved in virtio networking.
Existing virtio net code is used in the guest without modification.
There's similarity with vringfd, with some differences and reduced scope
- uses eventfd for signalling
- structures can be moved around in memory at any time (good for migration)
- support memory table and not just an offset (needed for kvm)
common virtio related code has been put in a separate file vhost.c and
can be made into a separate module if/when more backend appear. I used
Rusty's lguest.c as the source for developing this part : this supplied
me with witty comments I wouldn't be able to write myself.
What it is not: vhost net is not a bus, and not a generic new system
call. No assumptions are made on how guest performs hypercalls.
Userspace hypervisors are supported as well as kvm.
How it works: Basically, we connect virtio frontend (configured by
userspace) to a backend. The backend could be a network device, or a
tun-like device. In this version I only support raw socket as a backend,
which can be bound to e.g. SR IOV, or to macvlan device. Backend is
also configured by userspace, including vlan/mac etc.
Status:
This works for me, and I haven't see any crashes.
I have not run any benchmarks yet, compared to userspace, I expect to
see improved latency (as I save up to 4 system calls per packet) but not
yet bandwidth/CPU (as TSO and interrupt mitigation are not yet supported).
Features that I plan to look at in the future:
- TSO
- interrupt mitigation
- zero copy
Much better -- a couple of documentation nits below.
Thanx, Paul
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
MAINTAINERS| 10 +
arch/x86/kvm/Kconfig |1 +
drivers/Makefile |1 +
drivers/block/virtio_blk.c |3 +
drivers/vhost/Kconfig | 11 +
drivers/vhost/Makefile |2 +
drivers/vhost/net.c| 462 ++
drivers/vhost/vhost.c | 663
drivers/vhost/vhost.h | 108 +++
include/linux/Kbuild |1 +
include/linux/miscdevice.h |1 +
include/linux/vhost.h | 100 +++
12 files changed, 1363 insertions(+), 0 deletions(-)
create mode 100644 drivers/vhost/Kconfig
create mode 100644 drivers/vhost/Makefile
create mode 100644 drivers/vhost/net.c
create mode 100644 drivers/vhost/vhost.c
create mode 100644 drivers/vhost/vhost.h
create mode 100644 include/linux/vhost.h
diff --git a/MAINTAINERS b/MAINTAINERS
index ebc2691..eb0c1da 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6312,6 +6312,16 @@ S: Maintained
F: Documentation/filesystems/vfat.txt
F: fs/fat/
+VIRTIO HOST (VHOST)
+P: Michael S. Tsirkin
+M: m...@redhat.com
+L: kvm@vger.kernel.org
+L: virtualizat...@lists.osdl.org
+L: net...@vger.kernel.org
+S: Maintained
+F: drivers/vhost/
+F: include/linux/vhost.h
+
VIA RHINE NETWORK DRIVER
P: Roger Luethi
M: r...@hellgate.ch
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index b84e571..94f44d9 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -64,6 +64,7 @@ config KVM_AMD
# OK, it's a little counter-intuitive to do this, but it puts it neatly under
# the virtualization menu.
+source drivers/vhost/Kconfig
source drivers/lguest/Kconfig
source drivers/virtio/Kconfig
diff --git a/drivers/Makefile b/drivers/Makefile
index bc4205d..1551ae1 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -105,6 +105,7 @@ obj-$(CONFIG_HID) += hid/
obj-$(CONFIG_PPC_PS3)+= ps3/
obj-$(CONFIG_OF) += of/
obj-$(CONFIG_SSB)+= ssb/
+obj-$(CONFIG_VHOST_NET) += vhost/
obj-$(CONFIG_VIRTIO) += virtio/
obj-$(CONFIG_VLYNQ) += vlynq/
obj-$(CONFIG_STAGING)+= staging/
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index aa1a3d5..42e61b0 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -293,6 +293,7 @@ static int __devinit virtblk_probe(struct virtio_device
*vdev)
err = PTR_ERR(vblk-vq);
goto out_free_vblk;
}
+ printk(KERN_ERR vblk-vq = %p\n, vblk-vq);
vblk-pool = mempool_create_kmalloc_pool(1,sizeof(struct virtblk_req));
if (!vblk-pool) {
@@ -383,6 +384,8 @@ static int __devinit virtblk_probe(struct virtio_device
*vdev)
if (!err)
blk_queue_logical_block_size(vblk-disk-queue, blk_size);
+ printk(KERN_ERR virtio_config_val returned %d\n, err);
+
add_disk(vblk-disk);
return 0;
diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
new file mode 100644
Re: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Wed, Aug 12, 2009 at 10:48:21AM -0700, Ira W. Snyder wrote:
On Wed, Aug 12, 2009 at 08:31:04PM +0300, Michael S. Tsirkin wrote:
On Wed, Aug 12, 2009 at 10:19:22AM -0700, Ira W. Snyder wrote:
[ snip out code ]
We discussed this before, and I still think this could be directly
derived
from struct virtqueue, in the same way that vring_virtqueue is derived
from
struct virtqueue. That would make it possible for simple device drivers
to use the same driver in both host and guest, similar to how Ira Snyder
used virtqueues to make virtio_net run between two hosts running the
same code [1].
Ideally, I guess you should be able to even make virtio_net work in the
host if you do that, but that could bring other complexities.
I have no comments about the vhost code itself, I haven't reviewed it.
It might be interesting to try using a virtio-net in the host kernel to
communicate with the virtio-net running in the guest kernel. The lack of
a management interface is the biggest problem you will face (setting MAC
addresses, negotiating features, etc. doesn't work intuitively).
That was one of the reasons I decided to move most of code out to
userspace. My kernel driver only handles datapath,
it's much smaller than virtio net.
Getting
the network interfaces talking is relatively easy.
Ira
Tried this, but
- guest memory isn't pinned, so copy_to_user
to access it, errors need to be handled in a sane way
- used/available roles are reversed
- kick/interrupt roles are reversed
So most of the code then looks like
if (host) {
} else {
}
return
The only common part is walking the descriptor list,
but that's like 10 lines of code.
At which point it's better to keep host/guest code separate, IMO.
Ok, that makes sense. Let me see if I understand the concept of the
driver. Here's a picture of what makes sense to me:
guest system
-
| userspace applications|
-
| kernel network stack |
-
| virtio-net|
-
| transport (virtio-ring, etc.) |
-
|
|
-
| transport (virtio-ring, etc.) |
-
| some driver (maybe vhost?)| -- [1]
-
| kernel network stack |
-
host system
From the host's network stack, packets can be forwarded out to the
physical network, or be consumed by a normal userspace application on
the host. Just as if this were any other network interface.
In my patch, [1] was the virtio-net driver, completely unmodified.
So, does this patch accomplish the above diagram?
Not exactly. vhost passes packets to a physical device,
through a raw socket, not into host network stack.
If so, why the copy_to_user(), etc?
Guest memory is not pinned. Memory access needs to go through
translation process, could cause page faults, etc.
Maybe I'm confusing this with my system, where the
guest is another physical system, separated by the PCI bus.
Ira
Yes, that's different.
--
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
[PATCH 2/2] vhost_net: a kernel-level virtio server
What it is: vhost net is a character device that can be used to reduce
the number of system calls involved in virtio networking.
Existing virtio net code is used in the guest without modification.
There's similarity with vringfd, with some differences and reduced scope
- uses eventfd for signalling
- structures can be moved around in memory at any time (good for migration)
- support memory table and not just an offset (needed for kvm)
common virtio related code has been put in a separate file vhost.c and
can be made into a separate module if/when more backend appear. I used
Rusty's lguest.c as the source for developing this part : this supplied
me with witty comments I wouldn't be able to write myself.
What it is not: vhost net is not a bus, and not a generic new system
call. No assumptions are made on how guest performs hypercalls.
Userspace hypervisors are supported as well as kvm.
How it works: Basically, we connect virtio frontend (configured by
userspace) to a backend. The backend could be a network device, or a
tun-like device. In this version I only support raw socket as a backend,
which can be bound to e.g. SR IOV, or to macvlan device. Backend is
also configured by userspace, including vlan/mac etc.
Status:
This works for me, and I haven't see any crashes.
I have not run any benchmarks yet, compared to userspace, I expect to
see improved latency (as I save up to 4 system calls per packet) but not
yet bandwidth/CPU (as TSO and interrupt mitigation are not yet supported).
Features that I plan to look at in the future:
- TSO
- interrupt mitigation
- zero copy
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
MAINTAINERS| 10 +
arch/x86/kvm/Kconfig |1 +
drivers/Makefile |1 +
drivers/block/virtio_blk.c |3 +
drivers/vhost/Kconfig | 11 +
drivers/vhost/Makefile |2 +
drivers/vhost/net.c| 462 ++
drivers/vhost/vhost.c | 663
drivers/vhost/vhost.h | 108 +++
include/linux/Kbuild |1 +
include/linux/miscdevice.h |1 +
include/linux/vhost.h | 100 +++
12 files changed, 1363 insertions(+), 0 deletions(-)
create mode 100644 drivers/vhost/Kconfig
create mode 100644 drivers/vhost/Makefile
create mode 100644 drivers/vhost/net.c
create mode 100644 drivers/vhost/vhost.c
create mode 100644 drivers/vhost/vhost.h
create mode 100644 include/linux/vhost.h
diff --git a/MAINTAINERS b/MAINTAINERS
index ebc2691..eb0c1da 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6312,6 +6312,16 @@ S: Maintained
F: Documentation/filesystems/vfat.txt
F: fs/fat/
+VIRTIO HOST (VHOST)
+P: Michael S. Tsirkin
+M: m...@redhat.com
+L: kvm@vger.kernel.org
+L: virtualizat...@lists.osdl.org
+L: net...@vger.kernel.org
+S: Maintained
+F: drivers/vhost/
+F: include/linux/vhost.h
+
VIA RHINE NETWORK DRIVER
P: Roger Luethi
M: r...@hellgate.ch
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index b84e571..94f44d9 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -64,6 +64,7 @@ config KVM_AMD
# OK, it's a little counter-intuitive to do this, but it puts it neatly under
# the virtualization menu.
+source drivers/vhost/Kconfig
source drivers/lguest/Kconfig
source drivers/virtio/Kconfig
diff --git a/drivers/Makefile b/drivers/Makefile
index bc4205d..1551ae1 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -105,6 +105,7 @@ obj-$(CONFIG_HID) += hid/
obj-$(CONFIG_PPC_PS3) += ps3/
obj-$(CONFIG_OF) += of/
obj-$(CONFIG_SSB) += ssb/
+obj-$(CONFIG_VHOST_NET)+= vhost/
obj-$(CONFIG_VIRTIO) += virtio/
obj-$(CONFIG_VLYNQ)+= vlynq/
obj-$(CONFIG_STAGING) += staging/
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index aa1a3d5..42e61b0 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -293,6 +293,7 @@ static int __devinit virtblk_probe(struct virtio_device
*vdev)
err = PTR_ERR(vblk-vq);
goto out_free_vblk;
}
+ printk(KERN_ERR vblk-vq = %p\n, vblk-vq);
vblk-pool = mempool_create_kmalloc_pool(1,sizeof(struct virtblk_req));
if (!vblk-pool) {
@@ -383,6 +384,8 @@ static int __devinit virtblk_probe(struct virtio_device
*vdev)
if (!err)
blk_queue_logical_block_size(vblk-disk-queue, blk_size);
+ printk(KERN_ERR virtio_config_val returned %d\n, err);
+
add_disk(vblk-disk);
return 0;
diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
new file mode 100644
index 000..d955406
--- /dev/null
+++ b/drivers/vhost/Kconfig
@@ -0,0 +1,11 @@
+config VHOST_NET
+ tristate Host kernel accelerator for virtio net
+ depends on NET EVENTFD
+ ---help---
+ This kernel module can be loaded in host kernel
Re: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Monday 10 August 2009, Michael S. Tsirkin wrote:
What it is: vhost net is a character device that can be used to reduce
the number of system calls involved in virtio networking.
Existing virtio net code is used in the guest without modification.
Very nice, I loved reading it. It's getting rather late in my time
zone, so this comments only on the network driver. I'll go through
the rest tomorrow.
@@ -293,6 +293,7 @@ static int __devinit virtblk_probe(struct virtio_device
*vdev)
err = PTR_ERR(vblk-vq);
goto out_free_vblk;
}
+ printk(KERN_ERR vblk-vq = %p\n, vblk-vq);
vblk-pool = mempool_create_kmalloc_pool(1,sizeof(struct virtblk_req));
if (!vblk-pool) {
@@ -383,6 +384,8 @@ static int __devinit virtblk_probe(struct virtio_device
*vdev)
if (!err)
blk_queue_logical_block_size(vblk-disk-queue, blk_size);
+ printk(KERN_ERR virtio_config_val returned %d\n, err);
+
add_disk(vblk-disk);
return 0;
I guess you meant to remove these before submitting.
+static void handle_tx_kick(struct work_struct *work);
+static void handle_rx_kick(struct work_struct *work);
+static void handle_tx_net(struct work_struct *work);
+static void handle_rx_net(struct work_struct *work);
[style] I think the code gets more readable if you reorder it
so that you don't need forward declarations for static functions.
+static long vhost_net_reset_owner(struct vhost_net *n)
+{
+ struct socket *sock = NULL;
+ long r;
+ mutex_lock(n-dev.mutex);
+ r = vhost_dev_check_owner(n-dev);
+ if (r)
+ goto done;
+ sock = vhost_net_stop(n);
+ r = vhost_dev_reset_owner(n-dev);
+done:
+ mutex_unlock(n-dev.mutex);
+ if (sock)
+ fput(sock-file);
+ return r;
+}
what is the difference between vhost_net_reset_owner(n)
and vhost_net_set_socket(n, -1)?
+
+static struct file_operations vhost_net_fops = {
+ .owner = THIS_MODULE,
+ .release= vhost_net_release,
+ .unlocked_ioctl = vhost_net_ioctl,
+ .open = vhost_net_open,
+};
This is missing a compat_ioctl pointer. It should simply be
static long vhost_net_compat_ioctl(struct file *f,
unsigned int ioctl, unsigned long arg)
{
return f, ioctl, (unsigned long)compat_ptr(arg);
}
+/* Bits from fs/aio.c. TODO: export and use from there? */
+/*
+ * use_mm
+ * Makes the calling kernel thread take on the specified
+ * mm context.
+ * Called by the retry thread execute retries within the
+ * iocb issuer's mm context, so that copy_from/to_user
+ * operations work seamlessly for aio.
+ * (Note: this routine is intended to be called only
+ * from a kernel thread context)
+ */
+static void use_mm(struct mm_struct *mm)
+{
+ struct mm_struct *active_mm;
+ struct task_struct *tsk = current;
+
+ task_lock(tsk);
+ active_mm = tsk-active_mm;
+ atomic_inc(mm-mm_count);
+ tsk-mm = mm;
+ tsk-active_mm = mm;
+ switch_mm(active_mm, mm, tsk);
+ task_unlock(tsk);
+
+ mmdrop(active_mm);
+}
Why do you need a kernel thread here? If the data transfer functions
all get called from a guest intercept, shouldn't you already be
in the right mm?
+static void handle_tx(struct vhost_net *net)
+{
+ struct vhost_virtqueue *vq = net-dev.vqs[VHOST_NET_VQ_TX];
+ unsigned head, out, in;
+ struct msghdr msg = {
+ .msg_name = NULL,
+ .msg_namelen = 0,
+ .msg_control = NULL,
+ .msg_controllen = 0,
+ .msg_iov = (struct iovec *)vq-iov + 1,
+ .msg_flags = MSG_DONTWAIT,
+ };
+ size_t len;
+ int err;
+ struct socket *sock = rcu_dereference(net-sock);
+ if (!sock || !sock_writeable(sock-sk))
+ return;
+
+ use_mm(net-dev.mm);
+ mutex_lock(vq-mutex);
+ for (;;) {
+ head = vhost_get_vq_desc(net-dev, vq, vq-iov, out, in);
+ if (head == vq-num)
+ break;
+ if (out = 1 || in) {
+ vq_err(vq, Unexpected descriptor format for TX:
+out %d, int %d\n, out, in);
+ break;
+ }
+ /* Sanity check */
+ if (vq-iov-iov_len != sizeof(struct virtio_net_hdr)) {
+ vq_err(vq, Unexpected header len for TX:
+%ld expected %zd\n, vq-iov-iov_len,
+sizeof(struct virtio_net_hdr));
+ break;
+ }
+ /* Skip header. TODO: support TSO. */
+ msg.msg_iovlen = out - 1;
+ len = iov_length(vq-iov + 1, out - 1);
+ /* TODO: Check specific error and bomb out unless ENOBUFS? */
+ err = sock-ops-sendmsg(NULL, sock, msg, len);
+ if (err 0) {
+
Re: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Mon, Aug 10, 2009 at 09:51:18PM +0200, Arnd Bergmann wrote:
On Monday 10 August 2009, Michael S. Tsirkin wrote:
What it is: vhost net is a character device that can be used to reduce
the number of system calls involved in virtio networking.
Existing virtio net code is used in the guest without modification.
Very nice, I loved reading it. It's getting rather late in my time
zone, so this comments only on the network driver. I'll go through
the rest tomorrow.
@@ -293,6 +293,7 @@ static int __devinit virtblk_probe(struct virtio_device
*vdev)
err = PTR_ERR(vblk-vq);
goto out_free_vblk;
}
+ printk(KERN_ERR vblk-vq = %p\n, vblk-vq);
vblk-pool = mempool_create_kmalloc_pool(1,sizeof(struct virtblk_req));
if (!vblk-pool) {
@@ -383,6 +384,8 @@ static int __devinit virtblk_probe(struct virtio_device
*vdev)
if (!err)
blk_queue_logical_block_size(vblk-disk-queue, blk_size);
+ printk(KERN_ERR virtio_config_val returned %d\n, err);
+
add_disk(vblk-disk);
return 0;
I guess you meant to remove these before submitting.
Good catch, thanks!
+static void handle_tx_kick(struct work_struct *work);
+static void handle_rx_kick(struct work_struct *work);
+static void handle_tx_net(struct work_struct *work);
+static void handle_rx_net(struct work_struct *work);
[style] I think the code gets more readable if you reorder it
so that you don't need forward declarations for static functions.
Right.
+static long vhost_net_reset_owner(struct vhost_net *n)
+{
+ struct socket *sock = NULL;
+ long r;
+ mutex_lock(n-dev.mutex);
+ r = vhost_dev_check_owner(n-dev);
+ if (r)
+ goto done;
+ sock = vhost_net_stop(n);
+ r = vhost_dev_reset_owner(n-dev);
+done:
+ mutex_unlock(n-dev.mutex);
+ if (sock)
+ fput(sock-file);
+ return r;
+}
what is the difference between vhost_net_reset_owner(n)
and vhost_net_set_socket(n, -1)?
set socket to -1 will only stop the device.
reset owner will let another process take over the device.
It also needs to reset all parameters to make it safe for that
other process, so in particular the device is stopped.
I tried explaining this in the header vhost.h - does the comment
there help, or do I need to clarify it?
+
+static struct file_operations vhost_net_fops = {
+ .owner = THIS_MODULE,
+ .release= vhost_net_release,
+ .unlocked_ioctl = vhost_net_ioctl,
+ .open = vhost_net_open,
+};
This is missing a compat_ioctl pointer. It should simply be
static long vhost_net_compat_ioctl(struct file *f,
unsigned int ioctl, unsigned long arg)
{
return f, ioctl, (unsigned long)compat_ptr(arg);
}
I had the impression that if there's no compat_ioctl,
unlocked_ioctl will get called automatically. No?
+/* Bits from fs/aio.c. TODO: export and use from there? */
+/*
+ * use_mm
+ * Makes the calling kernel thread take on the specified
+ * mm context.
+ * Called by the retry thread execute retries within the
+ * iocb issuer's mm context, so that copy_from/to_user
+ * operations work seamlessly for aio.
+ * (Note: this routine is intended to be called only
+ * from a kernel thread context)
+ */
+static void use_mm(struct mm_struct *mm)
+{
+ struct mm_struct *active_mm;
+ struct task_struct *tsk = current;
+
+ task_lock(tsk);
+ active_mm = tsk-active_mm;
+ atomic_inc(mm-mm_count);
+ tsk-mm = mm;
+ tsk-active_mm = mm;
+ switch_mm(active_mm, mm, tsk);
+ task_unlock(tsk);
+
+ mmdrop(active_mm);
+}
Why do you need a kernel thread here? If the data transfer functions
all get called from a guest intercept, shouldn't you already be
in the right mm?
several reasons :)
- I get called under lock, so can't block
- eventfd can be passed to another process, and I won't be in guest context at
all
- this also gets called outside guest context from socket poll
- vcpu is blocked while it's doing i/o. it is better to free it up
as all the packet copying might take a while
+static void handle_tx(struct vhost_net *net)
+{
+ struct vhost_virtqueue *vq = net-dev.vqs[VHOST_NET_VQ_TX];
+ unsigned head, out, in;
+ struct msghdr msg = {
+ .msg_name = NULL,
+ .msg_namelen = 0,
+ .msg_control = NULL,
+ .msg_controllen = 0,
+ .msg_iov = (struct iovec *)vq-iov + 1,
+ .msg_flags = MSG_DONTWAIT,
+ };
+ size_t len;
+ int err;
+ struct socket *sock = rcu_dereference(net-sock);
+ if (!sock || !sock_writeable(sock-sk))
+ return;
+
+ use_mm(net-dev.mm);
+ mutex_lock(vq-mutex);
+ for (;;) {
+ head = vhost_get_vq_desc(net-dev, vq, vq-iov, out, in);
+ if (head == vq-num)
+ break;
+ if (out = 1 || in) {
+
Re: [PATCH 2/2] vhost_net: a kernel-level virtio server
On Monday 10 August 2009 20:10:44 Michael S. Tsirkin wrote: On Mon, Aug 10, 2009 at 09:51:18PM +0200, Arnd Bergmann wrote: what is the difference between vhost_net_reset_owner(n) and vhost_net_set_socket(n, -1)? set socket to -1 will only stop the device. reset owner will let another process take over the device. It also needs to reset all parameters to make it safe for that other process, so in particular the device is stopped. ok I tried explaining this in the header vhost.h - does the comment there help, or do I need to clarify it? No, I just didn't get there yet. I had the impression that if there's no compat_ioctl, unlocked_ioctl will get called automatically. No? It will issue a kernel warning but not call unlocked_ioctl, so you need either a compat_ioctl method or list the numbers in fs/compat_ioctl.c, which I try to avoid. Why do you need a kernel thread here? If the data transfer functions all get called from a guest intercept, shouldn't you already be in the right mm? several reasons :) - I get called under lock, so can't block - eventfd can be passed to another process, and I won't be in guest context at all - this also gets called outside guest context from socket poll - vcpu is blocked while it's doing i/o. it is better to free it up as all the packet copying might take a while Ok. I guess that this is where one could plug into macvlan directly, using sock_alloc_send_skb/memcpy_fromiovec/dev_queue_xmit directly, instead of filling a msghdr for each, if we want to combine this with the work I did on that. quite possibly. Or one can just bind a raw socket to macvlan :) Right, that works as well, but may get more complicated once we try to add zero-copy or other optimizations. Arnd -- 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
