Re: [PATCH] AF_VMCHANNEL address family for guest<->host communication.

2008-12-14 Thread Gleb Natapov
On Sun, Dec 14, 2008 at 10:44:36PM -0800, David Miller wrote:
> From: Gleb Natapov 
> Date: Sun, 14 Dec 2008 13:50:55 +0200
> 
> > It is undesirable to use TCP/IP for this purpose since network
> > connectivity may not exist between host and guest and if it exists the
> > traffic can be not routable between host and guest for security reasons
> > or TCP/IP traffic can be firewalled (by mistake) by unsuspecting VM user.
> 
> I don't really accept this argument, sorry.
> 
> If you can't use TCP because it might be security protected or
> misconfigured, adding this new stream protocol thing is not one
> bit better.  It doesn't make any sense at all.
> 
It can be _accidentally_ misconfigured. Just think about sysadmin that has
a remote access to a VM guest and he doesn't even know that it is a VM.
(He can easily find this out but why should he care?). The sysadmin knows
that the first rule of firewalling is deny everything and than allow
what you want to be allowed, so that what he does and cut communication
between host and guest. The problem with networking is that it is visible
to VM user and perceived to be under full user control.

> Also, if TCP could be "misconfigured" this new thing could just as
> easily be screwed up too.  And I wouldn't be surprised to see a whole
> bunch of SELINUX and netfilter features proposed later for this and
> then we're back to square one.
> 
It not only can be missconfigured it may not exist between guest and
host at all. IP connectivity between guest and host is not mandatory
and we don't want to make it such. It is like saying "who needs
serial console, just use ssh". And what subnet should be used for this
purpose? Who will solve conflicts? I can see why SELINUX features may
be proposed for vmchannel, but netfilter doesn't make sense for it.
And vmchannel also has other advantages over TCP/IP: less overhead and
better "naming". By better naming I mean that guest should not guess
(or require configuration) what ip:port should be used for cut&paste,
it just connects to address "cut&paste".

> You guys really need to rethink this.  Either a stream protocol is a
> workable solution to your problem, or it isn't.
> 
Stream protocol is workable solution for us, but we need it out of band
in regard to networking and as much zero config as possible. If we will
use networking how can it be done without additional configuration (and
reconfiguration can be required after migration BTW)

--
Gleb.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: [PATCH] AF_VMCHANNEL address family for guest<->host communication.

2008-12-14 Thread David Miller
From: Gleb Natapov 
Date: Sun, 14 Dec 2008 13:50:55 +0200

> It is undesirable to use TCP/IP for this purpose since network
> connectivity may not exist between host and guest and if it exists the
> traffic can be not routable between host and guest for security reasons
> or TCP/IP traffic can be firewalled (by mistake) by unsuspecting VM user.

I don't really accept this argument, sorry.

If you can't use TCP because it might be security protected or
misconfigured, adding this new stream protocol thing is not one
bit better.  It doesn't make any sense at all.

Also, if TCP could be "misconfigured" this new thing could just as
easily be screwed up too.  And I wouldn't be surprised to see a whole
bunch of SELINUX and netfilter features proposed later for this and
then we're back to square one.

You guys really need to rethink this.  Either a stream protocol is a
workable solution to your problem, or it isn't.

And don't bring up any "virtualization is special because..."
arguments into your reply because virtualization has nothing to do
with my objections stated above.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


Re: [PATCH] AF_VMCHANNEL address family for guest<->host communication.

2008-12-14 Thread Gleb Natapov
Hi Evgeniy,

On Sun, Dec 14, 2008 at 03:23:20PM +0300, Evgeniy Polyakov wrote:
> On Sun, Dec 14, 2008 at 01:50:55PM +0200, Gleb Natapov (g...@redhat.com) 
> wrote:
> > There is a need for communication channel between host and various
> > agents that are running inside a VM guest. The channel will be used
> > for statistic gathering, logging, cut & paste, host screen resolution
> > changes notifications, guest configuration etc.
> > 
> > It is undesirable to use TCP/IP for this purpose since network
> > connectivity may not exist between host and guest and if it exists the
> > traffic can be not routable between host and guest for security reasons
> > or TCP/IP traffic can be firewalled (by mistake) by unsuspecting VM user.
> > 
> > This patch implement new address family AF_VMCHANNEL that is used
> > for communication between guest and host. Channels are created at VM
> > start time. Each channel has a name. Agent, that runs on a guest, can
> > send/receive data to/from a channel by creating AF_VMCHANNEL socket and
> > connecting to a channel using channels name as an address.
> > 
> > Only stream sockets are supported by this implementation. Also only
> > connect, sendmsg and recvmsg socket ops are implemented which is enough
> > to allow application running in a guest to connect to a channel created
> > by a host and read/write from/to the channel. This can be extended to
> > allow channel creation from inside a guest by creating listen socket and
> > accepting on it if the need will arise and thus even allow guest<->guest
> > communication in the future (but TCP/IP may be preferable for this).
> 
> Couple of comments on this.
> First, there is only single virtio device initialized at probe time,
> how this will work on the host system with multiple guests? Is it
> possible to have multiple virtual devices?
The module is loaded only inside a guest not host and it manages all
existing channels. What would be the value to have multiple vmchannel
PCI devices in a single guest?

> Second, each virtual device has an array of names, and each socket can
> be bound to one of them, but it is not allowed to have multiple sockets
> bound to the same name, so it looks like there is no possibility to have
> several sockets communicating via signel channel, was this intentional?
Yes, this is intentional as it matches our usage model. It is possible
to change this in the future if needed. All sockets bound to the same
channel will receive the same data.

> And third, tasklet callbacks do not use bh socket locking, and while it
> is not something bad, but rt folks want (dream) to replace it with
> process context, so this at least requires some note in comments.
> 
This is something I need to understand better. I though that socket
lock guards socket state change. The patch only access socket state
from bh context in the vmchannel_socket_recv() and even if state of the
socket will change after function validates it nothing bad can happen.
Is this the case? I it is I will add comment explaining this.

> Except that about questions, this patch looks good.
Thanks for the review.

--
Gleb.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization


[PATCH] AF_VMCHANNEL address family for guest<->host communication.

2008-12-14 Thread Gleb Natapov
There is a need for communication channel between host and various
agents that are running inside a VM guest. The channel will be used
for statistic gathering, logging, cut & paste, host screen resolution
changes notifications, guest configuration etc.

It is undesirable to use TCP/IP for this purpose since network
connectivity may not exist between host and guest and if it exists the
traffic can be not routable between host and guest for security reasons
or TCP/IP traffic can be firewalled (by mistake) by unsuspecting VM user.

This patch implement new address family AF_VMCHANNEL that is used
for communication between guest and host. Channels are created at VM
start time. Each channel has a name. Agent, that runs on a guest, can
send/receive data to/from a channel by creating AF_VMCHANNEL socket and
connecting to a channel using channels name as an address.

Only stream sockets are supported by this implementation. Also only
connect, sendmsg and recvmsg socket ops are implemented which is enough
to allow application running in a guest to connect to a channel created
by a host and read/write from/to the channel. This can be extended to
allow channel creation from inside a guest by creating listen socket and
accepting on it if the need will arise and thus even allow guest<->guest
communication in the future (but TCP/IP may be preferable for this).

Signed-off-by: Gleb Natapov 
---

 include/linux/socket.h   |4 
 include/linux/vmchannel.h|   54 +++
 net/Kconfig  |1 
 net/Makefile |1 
 net/vmchannel/Kconfig|   11 +
 net/vmchannel/Makefile   |5 
 net/vmchannel/af_vmchannel.c |  769 ++
 7 files changed, 844 insertions(+), 1 deletions(-)
 create mode 100644 include/linux/vmchannel.h
 create mode 100644 net/vmchannel/Kconfig
 create mode 100644 net/vmchannel/Makefile
 create mode 100644 net/vmchannel/af_vmchannel.c

diff --git a/include/linux/socket.h b/include/linux/socket.h
index 20fc4bb..e65834c 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -191,7 +191,8 @@ struct ucred {
 #define AF_RXRPC   33  /* RxRPC sockets*/
 #define AF_ISDN34  /* mISDN sockets*/
 #define AF_PHONET  35  /* Phonet sockets   */
-#define AF_MAX 36  /* For now.. */
+#define AF_VMCHANNEL   36  /* Vmchannel sockets*/
+#define AF_MAX 37  /* For now.. */
 
 /* Protocol families, same as address families. */
 #define PF_UNSPEC  AF_UNSPEC
@@ -229,6 +230,7 @@ struct ucred {
 #define PF_RXRPC   AF_RXRPC
 #define PF_ISDNAF_ISDN
 #define PF_PHONET  AF_PHONET
+#define PF_VMCHANNEL   AF_VMCHANNEL
 #define PF_MAX AF_MAX
 
 /* Maximum queue length specifiable by listen.  */
diff --git a/include/linux/vmchannel.h b/include/linux/vmchannel.h
new file mode 100644
index 000..27c1f94
--- /dev/null
+++ b/include/linux/vmchannel.h
@@ -0,0 +1,54 @@
+/*
+ *  Copyright 2008 Red Hat, Inc --- All Rights Reserved
+ *
+ *  Author(s): Gleb Natapov 
+ */
+
+#ifndef VMCHANNEL_H
+#define VMCHANNEL_H
+
+#define VMCHANNEL_NAME_MAX 80
+struct sockaddr_vmchannel {
+   sa_family_t svmchannel_family;
+   char svmchannel_name[VMCHANNEL_NAME_MAX];
+};
+
+#ifdef __KERNEL__
+
+#define VIRTIO_ID_VMCHANNEL 6
+#define VMCHANNEL_BAD_ID (~(__u32)0)
+
+#define vmchannel_sk(__sk) ((struct vmchannel_sock *) __sk)
+
+struct vmchannel_sock {
+   struct sock sk;
+   char name[VMCHANNEL_NAME_MAX];
+   __u32 id;
+   struct sk_buff_head backlog_skb_q;
+};
+
+struct vmchannel_info {
+   __u32 id;
+   char *name;
+};
+
+struct vmchannel_dev {
+   struct virtio_device *vdev;
+   struct virtqueue *rq;
+   struct virtqueue *sq;
+   struct tasklet_struct rx_tasklet;
+   struct tasklet_struct tx_tasklet;
+   __u32 channel_count;
+   struct vmchannel_info *channels;
+   struct sk_buff_head rx_skbuff_q;
+   struct sk_buff_head tx_skbuff_q;
+   atomic_t recv_posted;
+};
+
+struct vmchannel_desc {
+   __u32 id;
+   __le32 len;
+};
+
+#endif /* __KERNEL__ */
+#endif
diff --git a/net/Kconfig b/net/Kconfig
index d789d79..d01f135 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -36,6 +36,7 @@ source "net/packet/Kconfig"
 source "net/unix/Kconfig"
 source "net/xfrm/Kconfig"
 source "net/iucv/Kconfig"
+source "net/vmchannel/Kconfig"
 
 config INET
bool "TCP/IP networking"
diff --git a/net/Makefile b/net/Makefile
index 27d1f10..ddc89dc 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_IEEE80211)   += ieee80211/
 obj-$(CONFIG_TIPC) += tipc/
 obj-$(CONFIG_NETLABEL) += netlabel/
 obj-$(CONFIG_IUCV) += iucv/
+obj-$(CONFIG_VMCHANNEL)+= vmchannel/
 obj-$(CONFIG_RFKILL)   += rfkill/
 obj-$(CONFIG_NET_9P)   += 9p/
 
diff --git a/net/vmchannel/Kconfig b/n