* [PATCH net-next 1/6] vsock/virtio_transport_common: remove unused virtio header includes
2019-11-19 11:01 [PATCH net-next 0/6] vsock: add local transport support Stefano Garzarella
@ 2019-11-19 11:01 ` Stefano Garzarella
2019-11-19 11:01 ` [PATCH net-next 2/6] vsock: add VMADDR_CID_LOCAL definition Stefano Garzarella
` (8 subsequent siblings)
9 siblings, 0 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-19 11:01 UTC (permalink / raw)
To: netdev
Cc: virtualization, Dexuan Cui, Stefan Hajnoczi, linux-kernel, kvm,
David S. Miller, Jorgen Hansen
We can remove virtio header includes, because virtio_transport_common
doesn't use virtio API, but provides common functions to interface
virtio/vhost transports with the af_vsock core, and to handle
the protocol.
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
net/vmw_vsock/virtio_transport_common.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index e5ea29c6bca7..0e20b0f6eb65 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -11,9 +11,6 @@
#include <linux/sched/signal.h>
#include <linux/ctype.h>
#include <linux/list.h>
-#include <linux/virtio.h>
-#include <linux/virtio_ids.h>
-#include <linux/virtio_config.h>
#include <linux/virtio_vsock.h>
#include <uapi/linux/vsockmon.h>
--
2.21.0
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH net-next 2/6] vsock: add VMADDR_CID_LOCAL definition
2019-11-19 11:01 [PATCH net-next 0/6] vsock: add local transport support Stefano Garzarella
2019-11-19 11:01 ` [PATCH net-next 1/6] vsock/virtio_transport_common: remove unused virtio header includes Stefano Garzarella
@ 2019-11-19 11:01 ` Stefano Garzarella
2019-11-21 14:49 ` Jorgen Hansen via Virtualization
2019-11-21 14:49 ` Jorgen Hansen
2019-11-19 11:01 ` [PATCH net-next 3/6] vsock: add local transport support in the vsock core Stefano Garzarella
` (7 subsequent siblings)
9 siblings, 2 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-19 11:01 UTC (permalink / raw)
To: netdev
Cc: virtualization, Dexuan Cui, Stefan Hajnoczi, linux-kernel, kvm,
David S. Miller, Jorgen Hansen
The VMADDR_CID_RESERVED (1) was used by VMCI, but now it is not
used anymore, so we can reuse it for local communication
(loopback) adding the new well-know CID: VMADDR_CID_LOCAL.
Cc: Jorgen Hansen <jhansen@vmware.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
include/uapi/linux/vm_sockets.h | 8 +++++---
net/vmw_vsock/vmci_transport.c | 2 +-
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/vm_sockets.h b/include/uapi/linux/vm_sockets.h
index 68d57c5e99bc..fd0ed7221645 100644
--- a/include/uapi/linux/vm_sockets.h
+++ b/include/uapi/linux/vm_sockets.h
@@ -99,11 +99,13 @@
#define VMADDR_CID_HYPERVISOR 0
-/* This CID is specific to VMCI and can be considered reserved (even VMCI
- * doesn't use it anymore, it's a legacy value from an older release).
+/* Use this as the destination CID in an address when referring to the
+ * local communication (loopback).
+ * (This was VMADDR_CID_RESERVED, but even VMCI doesn't use it anymore,
+ * it was a legacy value from an older release).
*/
-#define VMADDR_CID_RESERVED 1
+#define VMADDR_CID_LOCAL 1
/* Use this as the destination CID in an address when referring to the host
* (any process other than the hypervisor). VMCI relies on it being 2, but
diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
index 644d32e43d23..4b8b1150a738 100644
--- a/net/vmw_vsock/vmci_transport.c
+++ b/net/vmw_vsock/vmci_transport.c
@@ -648,7 +648,7 @@ static int vmci_transport_recv_dgram_cb(void *data, struct vmci_datagram *dg)
static bool vmci_transport_stream_allow(u32 cid, u32 port)
{
static const u32 non_socket_contexts[] = {
- VMADDR_CID_RESERVED,
+ VMADDR_CID_LOCAL,
};
int i;
--
2.21.0
^ permalink raw reply related [flat|nested] 43+ messages in thread
* RE: [PATCH net-next 2/6] vsock: add VMADDR_CID_LOCAL definition
2019-11-19 11:01 ` [PATCH net-next 2/6] vsock: add VMADDR_CID_LOCAL definition Stefano Garzarella
@ 2019-11-21 14:49 ` Jorgen Hansen via Virtualization
2019-11-21 14:49 ` Jorgen Hansen
1 sibling, 0 replies; 43+ messages in thread
From: Jorgen Hansen via Virtualization @ 2019-11-21 14:49 UTC (permalink / raw)
To: 'Stefano Garzarella', netdev
Cc: kvm, Dexuan Cui, linux-kernel, virtualization, Stefan Hajnoczi,
David S. Miller
> From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> Sent: Tuesday, November 19, 2019 12:01 PM
>
> The VMADDR_CID_RESERVED (1) was used by VMCI, but now it is not
> used anymore, so we can reuse it for local communication
> (loopback) adding the new well-know CID: VMADDR_CID_LOCAL.
>
> Cc: Jorgen Hansen <jhansen@vmware.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> include/uapi/linux/vm_sockets.h | 8 +++++---
> net/vmw_vsock/vmci_transport.c | 2 +-
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/include/uapi/linux/vm_sockets.h
> b/include/uapi/linux/vm_sockets.h
> index 68d57c5e99bc..fd0ed7221645 100644
> --- a/include/uapi/linux/vm_sockets.h
> +++ b/include/uapi/linux/vm_sockets.h
> @@ -99,11 +99,13 @@
>
> #define VMADDR_CID_HYPERVISOR 0
>
> -/* This CID is specific to VMCI and can be considered reserved (even VMCI
> - * doesn't use it anymore, it's a legacy value from an older release).
> +/* Use this as the destination CID in an address when referring to the
> + * local communication (loopback).
> + * (This was VMADDR_CID_RESERVED, but even VMCI doesn't use it
> anymore,
> + * it was a legacy value from an older release).
> */
>
> -#define VMADDR_CID_RESERVED 1
> +#define VMADDR_CID_LOCAL 1
>
> /* Use this as the destination CID in an address when referring to the host
> * (any process other than the hypervisor). VMCI relies on it being 2, but
> diff --git a/net/vmw_vsock/vmci_transport.c
> b/net/vmw_vsock/vmci_transport.c
> index 644d32e43d23..4b8b1150a738 100644
> --- a/net/vmw_vsock/vmci_transport.c
> +++ b/net/vmw_vsock/vmci_transport.c
> @@ -648,7 +648,7 @@ static int vmci_transport_recv_dgram_cb(void *data,
> struct vmci_datagram *dg)
> static bool vmci_transport_stream_allow(u32 cid, u32 port)
> {
> static const u32 non_socket_contexts[] = {
> - VMADDR_CID_RESERVED,
> + VMADDR_CID_LOCAL,
> };
> int i;
>
> --
> 2.21.0
Reviewed-by: Jorgen Hansen <jhansen@vmware.com>
^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH net-next 2/6] vsock: add VMADDR_CID_LOCAL definition
2019-11-19 11:01 ` [PATCH net-next 2/6] vsock: add VMADDR_CID_LOCAL definition Stefano Garzarella
2019-11-21 14:49 ` Jorgen Hansen via Virtualization
@ 2019-11-21 14:49 ` Jorgen Hansen
1 sibling, 0 replies; 43+ messages in thread
From: Jorgen Hansen @ 2019-11-21 14:49 UTC (permalink / raw)
To: 'Stefano Garzarella', netdev
Cc: virtualization, Dexuan Cui, Stefan Hajnoczi, linux-kernel, kvm,
David S. Miller
> From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> Sent: Tuesday, November 19, 2019 12:01 PM
>
> The VMADDR_CID_RESERVED (1) was used by VMCI, but now it is not
> used anymore, so we can reuse it for local communication
> (loopback) adding the new well-know CID: VMADDR_CID_LOCAL.
>
> Cc: Jorgen Hansen <jhansen@vmware.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> include/uapi/linux/vm_sockets.h | 8 +++++---
> net/vmw_vsock/vmci_transport.c | 2 +-
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/include/uapi/linux/vm_sockets.h
> b/include/uapi/linux/vm_sockets.h
> index 68d57c5e99bc..fd0ed7221645 100644
> --- a/include/uapi/linux/vm_sockets.h
> +++ b/include/uapi/linux/vm_sockets.h
> @@ -99,11 +99,13 @@
>
> #define VMADDR_CID_HYPERVISOR 0
>
> -/* This CID is specific to VMCI and can be considered reserved (even VMCI
> - * doesn't use it anymore, it's a legacy value from an older release).
> +/* Use this as the destination CID in an address when referring to the
> + * local communication (loopback).
> + * (This was VMADDR_CID_RESERVED, but even VMCI doesn't use it
> anymore,
> + * it was a legacy value from an older release).
> */
>
> -#define VMADDR_CID_RESERVED 1
> +#define VMADDR_CID_LOCAL 1
>
> /* Use this as the destination CID in an address when referring to the host
> * (any process other than the hypervisor). VMCI relies on it being 2, but
> diff --git a/net/vmw_vsock/vmci_transport.c
> b/net/vmw_vsock/vmci_transport.c
> index 644d32e43d23..4b8b1150a738 100644
> --- a/net/vmw_vsock/vmci_transport.c
> +++ b/net/vmw_vsock/vmci_transport.c
> @@ -648,7 +648,7 @@ static int vmci_transport_recv_dgram_cb(void *data,
> struct vmci_datagram *dg)
> static bool vmci_transport_stream_allow(u32 cid, u32 port)
> {
> static const u32 non_socket_contexts[] = {
> - VMADDR_CID_RESERVED,
> + VMADDR_CID_LOCAL,
> };
> int i;
>
> --
> 2.21.0
Reviewed-by: Jorgen Hansen <jhansen@vmware.com>
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH net-next 3/6] vsock: add local transport support in the vsock core
2019-11-19 11:01 [PATCH net-next 0/6] vsock: add local transport support Stefano Garzarella
2019-11-19 11:01 ` [PATCH net-next 1/6] vsock/virtio_transport_common: remove unused virtio header includes Stefano Garzarella
2019-11-19 11:01 ` [PATCH net-next 2/6] vsock: add VMADDR_CID_LOCAL definition Stefano Garzarella
@ 2019-11-19 11:01 ` Stefano Garzarella
2019-11-21 15:04 ` Jorgen Hansen
2019-11-21 15:04 ` Jorgen Hansen via Virtualization
2019-11-19 11:01 ` [PATCH net-next 4/6] vsock: add vsock_loopback transport Stefano Garzarella
` (6 subsequent siblings)
9 siblings, 2 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-19 11:01 UTC (permalink / raw)
To: netdev
Cc: virtualization, Dexuan Cui, Stefan Hajnoczi, linux-kernel, kvm,
David S. Miller, Jorgen Hansen
This patch allows to register a transport able to handle
local communication (loopback).
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
include/net/af_vsock.h | 2 ++
net/vmw_vsock/af_vsock.c | 17 ++++++++++++++++-
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index 4206dc6d813f..b1c717286993 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -98,6 +98,8 @@ struct vsock_transport_send_notify_data {
#define VSOCK_TRANSPORT_F_G2H 0x00000002
/* Transport provides DGRAM communication */
#define VSOCK_TRANSPORT_F_DGRAM 0x00000004
+/* Transport provides local (loopback) communication */
+#define VSOCK_TRANSPORT_F_LOCAL 0x00000008
struct vsock_transport {
struct module *module;
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index cc8659838bf2..c9e5bad59dc1 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -136,6 +136,8 @@ static const struct vsock_transport *transport_h2g;
static const struct vsock_transport *transport_g2h;
/* Transport used for DGRAM communication */
static const struct vsock_transport *transport_dgram;
+/* Transport used for local communication */
+static const struct vsock_transport *transport_local;
static DEFINE_MUTEX(vsock_register_mutex);
/**** UTILS ****/
@@ -2130,7 +2132,7 @@ EXPORT_SYMBOL_GPL(vsock_core_get_transport);
int vsock_core_register(const struct vsock_transport *t, int features)
{
- const struct vsock_transport *t_h2g, *t_g2h, *t_dgram;
+ const struct vsock_transport *t_h2g, *t_g2h, *t_dgram, *t_local;
int err = mutex_lock_interruptible(&vsock_register_mutex);
if (err)
@@ -2139,6 +2141,7 @@ int vsock_core_register(const struct vsock_transport *t, int features)
t_h2g = transport_h2g;
t_g2h = transport_g2h;
t_dgram = transport_dgram;
+ t_local = transport_local;
if (features & VSOCK_TRANSPORT_F_H2G) {
if (t_h2g) {
@@ -2164,9 +2167,18 @@ int vsock_core_register(const struct vsock_transport *t, int features)
t_dgram = t;
}
+ if (features & VSOCK_TRANSPORT_F_LOCAL) {
+ if (t_local) {
+ err = -EBUSY;
+ goto err_busy;
+ }
+ t_local = t;
+ }
+
transport_h2g = t_h2g;
transport_g2h = t_g2h;
transport_dgram = t_dgram;
+ transport_local = t_local;
err_busy:
mutex_unlock(&vsock_register_mutex);
@@ -2187,6 +2199,9 @@ void vsock_core_unregister(const struct vsock_transport *t)
if (transport_dgram == t)
transport_dgram = NULL;
+ if (transport_local == t)
+ transport_local = NULL;
+
mutex_unlock(&vsock_register_mutex);
}
EXPORT_SYMBOL_GPL(vsock_core_unregister);
--
2.21.0
^ permalink raw reply related [flat|nested] 43+ messages in thread
* RE: [PATCH net-next 3/6] vsock: add local transport support in the vsock core
2019-11-19 11:01 ` [PATCH net-next 3/6] vsock: add local transport support in the vsock core Stefano Garzarella
@ 2019-11-21 15:04 ` Jorgen Hansen
2019-11-21 15:21 ` Stefano Garzarella
2019-11-21 15:21 ` Stefano Garzarella
2019-11-21 15:04 ` Jorgen Hansen via Virtualization
1 sibling, 2 replies; 43+ messages in thread
From: Jorgen Hansen @ 2019-11-21 15:04 UTC (permalink / raw)
To: 'Stefano Garzarella', netdev
Cc: virtualization, Dexuan Cui, Stefan Hajnoczi, linux-kernel, kvm,
David S. Miller
> From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> Sent: Tuesday, November 19, 2019 12:01 PM
> To: netdev@vger.kernel.org
>
> This patch allows to register a transport able to handle
> local communication (loopback).
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> include/net/af_vsock.h | 2 ++
> net/vmw_vsock/af_vsock.c | 17 ++++++++++++++++-
> 2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> index 4206dc6d813f..b1c717286993 100644
> --- a/include/net/af_vsock.h
> +++ b/include/net/af_vsock.h
> @@ -98,6 +98,8 @@ struct vsock_transport_send_notify_data {
> #define VSOCK_TRANSPORT_F_G2H 0x00000002
> /* Transport provides DGRAM communication */
> #define VSOCK_TRANSPORT_F_DGRAM 0x00000004
> +/* Transport provides local (loopback) communication */
> +#define VSOCK_TRANSPORT_F_LOCAL 0x00000008
>
> struct vsock_transport {
> struct module *module;
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index cc8659838bf2..c9e5bad59dc1 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -136,6 +136,8 @@ static const struct vsock_transport *transport_h2g;
> static const struct vsock_transport *transport_g2h;
> /* Transport used for DGRAM communication */
> static const struct vsock_transport *transport_dgram;
> +/* Transport used for local communication */
> +static const struct vsock_transport *transport_local;
> static DEFINE_MUTEX(vsock_register_mutex);
>
> /**** UTILS ****/
> @@ -2130,7 +2132,7 @@ EXPORT_SYMBOL_GPL(vsock_core_get_transport);
>
> int vsock_core_register(const struct vsock_transport *t, int features)
> {
> - const struct vsock_transport *t_h2g, *t_g2h, *t_dgram;
> + const struct vsock_transport *t_h2g, *t_g2h, *t_dgram, *t_local;
> int err = mutex_lock_interruptible(&vsock_register_mutex);
>
> if (err)
> @@ -2139,6 +2141,7 @@ int vsock_core_register(const struct
> vsock_transport *t, int features)
> t_h2g = transport_h2g;
> t_g2h = transport_g2h;
> t_dgram = transport_dgram;
> + t_local = transport_local;
>
> if (features & VSOCK_TRANSPORT_F_H2G) {
> if (t_h2g) {
> @@ -2164,9 +2167,18 @@ int vsock_core_register(const struct
> vsock_transport *t, int features)
> t_dgram = t;
> }
>
> + if (features & VSOCK_TRANSPORT_F_LOCAL) {
> + if (t_local) {
> + err = -EBUSY;
> + goto err_busy;
> + }
> + t_local = t;
> + }
> +
> transport_h2g = t_h2g;
> transport_g2h = t_g2h;
> transport_dgram = t_dgram;
> + transport_local = t_local;
>
> err_busy:
> mutex_unlock(&vsock_register_mutex);
> @@ -2187,6 +2199,9 @@ void vsock_core_unregister(const struct
> vsock_transport *t)
> if (transport_dgram == t)
> transport_dgram = NULL;
>
> + if (transport_local == t)
> + transport_local = NULL;
> +
> mutex_unlock(&vsock_register_mutex);
> }
> EXPORT_SYMBOL_GPL(vsock_core_unregister);
> --
> 2.21.0
Having loopback support as a separate transport fits nicely, but do we need to support
different variants of loopback? It could just be built in.
Thanks,
Jorgen
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 3/6] vsock: add local transport support in the vsock core
2019-11-21 15:04 ` Jorgen Hansen
@ 2019-11-21 15:21 ` Stefano Garzarella
2019-11-21 15:21 ` Stefano Garzarella
1 sibling, 0 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-21 15:21 UTC (permalink / raw)
To: Jorgen Hansen
Cc: kvm, netdev, Dexuan Cui, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller
On Thu, Nov 21, 2019 at 03:04:18PM +0000, Jorgen Hansen wrote:
> > From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> > Sent: Tuesday, November 19, 2019 12:01 PM
> > To: netdev@vger.kernel.org
> >
> > This patch allows to register a transport able to handle
> > local communication (loopback).
> >
> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > ---
> > include/net/af_vsock.h | 2 ++
> > net/vmw_vsock/af_vsock.c | 17 ++++++++++++++++-
> > 2 files changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> > index 4206dc6d813f..b1c717286993 100644
> > --- a/include/net/af_vsock.h
> > +++ b/include/net/af_vsock.h
> > @@ -98,6 +98,8 @@ struct vsock_transport_send_notify_data {
> > #define VSOCK_TRANSPORT_F_G2H 0x00000002
> > /* Transport provides DGRAM communication */
> > #define VSOCK_TRANSPORT_F_DGRAM 0x00000004
> > +/* Transport provides local (loopback) communication */
> > +#define VSOCK_TRANSPORT_F_LOCAL 0x00000008
> >
> > struct vsock_transport {
> > struct module *module;
> > diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> > index cc8659838bf2..c9e5bad59dc1 100644
> > --- a/net/vmw_vsock/af_vsock.c
> > +++ b/net/vmw_vsock/af_vsock.c
> > @@ -136,6 +136,8 @@ static const struct vsock_transport *transport_h2g;
> > static const struct vsock_transport *transport_g2h;
> > /* Transport used for DGRAM communication */
> > static const struct vsock_transport *transport_dgram;
> > +/* Transport used for local communication */
> > +static const struct vsock_transport *transport_local;
> > static DEFINE_MUTEX(vsock_register_mutex);
> >
> > /**** UTILS ****/
> > @@ -2130,7 +2132,7 @@ EXPORT_SYMBOL_GPL(vsock_core_get_transport);
> >
> > int vsock_core_register(const struct vsock_transport *t, int features)
> > {
> > - const struct vsock_transport *t_h2g, *t_g2h, *t_dgram;
> > + const struct vsock_transport *t_h2g, *t_g2h, *t_dgram, *t_local;
> > int err = mutex_lock_interruptible(&vsock_register_mutex);
> >
> > if (err)
> > @@ -2139,6 +2141,7 @@ int vsock_core_register(const struct
> > vsock_transport *t, int features)
> > t_h2g = transport_h2g;
> > t_g2h = transport_g2h;
> > t_dgram = transport_dgram;
> > + t_local = transport_local;
> >
> > if (features & VSOCK_TRANSPORT_F_H2G) {
> > if (t_h2g) {
> > @@ -2164,9 +2167,18 @@ int vsock_core_register(const struct
> > vsock_transport *t, int features)
> > t_dgram = t;
> > }
> >
> > + if (features & VSOCK_TRANSPORT_F_LOCAL) {
> > + if (t_local) {
> > + err = -EBUSY;
> > + goto err_busy;
> > + }
> > + t_local = t;
> > + }
> > +
> > transport_h2g = t_h2g;
> > transport_g2h = t_g2h;
> > transport_dgram = t_dgram;
> > + transport_local = t_local;
> >
> > err_busy:
> > mutex_unlock(&vsock_register_mutex);
> > @@ -2187,6 +2199,9 @@ void vsock_core_unregister(const struct
> > vsock_transport *t)
> > if (transport_dgram == t)
> > transport_dgram = NULL;
> >
> > + if (transport_local == t)
> > + transport_local = NULL;
> > +
> > mutex_unlock(&vsock_register_mutex);
> > }
> > EXPORT_SYMBOL_GPL(vsock_core_unregister);
> > --
> > 2.21.0
>
> Having loopback support as a separate transport fits nicely, but do we need to support
> different variants of loopback? It could just be built in.
I agree with you, indeed initially I developed it as built in, but
DEPMOD found a cyclic dependency because vsock_transport use
virtio_transport_common that use vsock, so if I include vsock_transport
in the vsock module, DEPMOD is not happy.
I don't know how to break this cyclic dependency, do you have any ideas?
Thanks,
Stefano
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 3/6] vsock: add local transport support in the vsock core
2019-11-21 15:04 ` Jorgen Hansen
2019-11-21 15:21 ` Stefano Garzarella
@ 2019-11-21 15:21 ` Stefano Garzarella
2019-11-21 15:53 ` Jorgen Hansen
2019-11-21 15:53 ` Jorgen Hansen via Virtualization
1 sibling, 2 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-21 15:21 UTC (permalink / raw)
To: Jorgen Hansen
Cc: netdev, virtualization, Dexuan Cui, Stefan Hajnoczi,
linux-kernel, kvm, David S. Miller
On Thu, Nov 21, 2019 at 03:04:18PM +0000, Jorgen Hansen wrote:
> > From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> > Sent: Tuesday, November 19, 2019 12:01 PM
> > To: netdev@vger.kernel.org
> >
> > This patch allows to register a transport able to handle
> > local communication (loopback).
> >
> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > ---
> > include/net/af_vsock.h | 2 ++
> > net/vmw_vsock/af_vsock.c | 17 ++++++++++++++++-
> > 2 files changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> > index 4206dc6d813f..b1c717286993 100644
> > --- a/include/net/af_vsock.h
> > +++ b/include/net/af_vsock.h
> > @@ -98,6 +98,8 @@ struct vsock_transport_send_notify_data {
> > #define VSOCK_TRANSPORT_F_G2H 0x00000002
> > /* Transport provides DGRAM communication */
> > #define VSOCK_TRANSPORT_F_DGRAM 0x00000004
> > +/* Transport provides local (loopback) communication */
> > +#define VSOCK_TRANSPORT_F_LOCAL 0x00000008
> >
> > struct vsock_transport {
> > struct module *module;
> > diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> > index cc8659838bf2..c9e5bad59dc1 100644
> > --- a/net/vmw_vsock/af_vsock.c
> > +++ b/net/vmw_vsock/af_vsock.c
> > @@ -136,6 +136,8 @@ static const struct vsock_transport *transport_h2g;
> > static const struct vsock_transport *transport_g2h;
> > /* Transport used for DGRAM communication */
> > static const struct vsock_transport *transport_dgram;
> > +/* Transport used for local communication */
> > +static const struct vsock_transport *transport_local;
> > static DEFINE_MUTEX(vsock_register_mutex);
> >
> > /**** UTILS ****/
> > @@ -2130,7 +2132,7 @@ EXPORT_SYMBOL_GPL(vsock_core_get_transport);
> >
> > int vsock_core_register(const struct vsock_transport *t, int features)
> > {
> > - const struct vsock_transport *t_h2g, *t_g2h, *t_dgram;
> > + const struct vsock_transport *t_h2g, *t_g2h, *t_dgram, *t_local;
> > int err = mutex_lock_interruptible(&vsock_register_mutex);
> >
> > if (err)
> > @@ -2139,6 +2141,7 @@ int vsock_core_register(const struct
> > vsock_transport *t, int features)
> > t_h2g = transport_h2g;
> > t_g2h = transport_g2h;
> > t_dgram = transport_dgram;
> > + t_local = transport_local;
> >
> > if (features & VSOCK_TRANSPORT_F_H2G) {
> > if (t_h2g) {
> > @@ -2164,9 +2167,18 @@ int vsock_core_register(const struct
> > vsock_transport *t, int features)
> > t_dgram = t;
> > }
> >
> > + if (features & VSOCK_TRANSPORT_F_LOCAL) {
> > + if (t_local) {
> > + err = -EBUSY;
> > + goto err_busy;
> > + }
> > + t_local = t;
> > + }
> > +
> > transport_h2g = t_h2g;
> > transport_g2h = t_g2h;
> > transport_dgram = t_dgram;
> > + transport_local = t_local;
> >
> > err_busy:
> > mutex_unlock(&vsock_register_mutex);
> > @@ -2187,6 +2199,9 @@ void vsock_core_unregister(const struct
> > vsock_transport *t)
> > if (transport_dgram == t)
> > transport_dgram = NULL;
> >
> > + if (transport_local == t)
> > + transport_local = NULL;
> > +
> > mutex_unlock(&vsock_register_mutex);
> > }
> > EXPORT_SYMBOL_GPL(vsock_core_unregister);
> > --
> > 2.21.0
>
> Having loopback support as a separate transport fits nicely, but do we need to support
> different variants of loopback? It could just be built in.
I agree with you, indeed initially I developed it as built in, but
DEPMOD found a cyclic dependency because vsock_transport use
virtio_transport_common that use vsock, so if I include vsock_transport
in the vsock module, DEPMOD is not happy.
I don't know how to break this cyclic dependency, do you have any ideas?
Thanks,
Stefano
^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH net-next 3/6] vsock: add local transport support in the vsock core
2019-11-21 15:21 ` Stefano Garzarella
@ 2019-11-21 15:53 ` Jorgen Hansen
2019-11-21 16:12 ` Stefano Garzarella
2019-11-21 16:12 ` Stefano Garzarella
2019-11-21 15:53 ` Jorgen Hansen via Virtualization
1 sibling, 2 replies; 43+ messages in thread
From: Jorgen Hansen @ 2019-11-21 15:53 UTC (permalink / raw)
To: 'Stefano Garzarella'
Cc: netdev, virtualization, Dexuan Cui, Stefan Hajnoczi,
linux-kernel, kvm, David S. Miller
> From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> Sent: Thursday, November 21, 2019 4:22 PM
>
> On Thu, Nov 21, 2019 at 03:04:18PM +0000, Jorgen Hansen wrote:
> > > From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> > > Sent: Tuesday, November 19, 2019 12:01 PM
> > > To: netdev@vger.kernel.org
> > >
> > > This patch allows to register a transport able to handle
> > > local communication (loopback).
> > >
> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > ---
> > > include/net/af_vsock.h | 2 ++
> > > net/vmw_vsock/af_vsock.c | 17 ++++++++++++++++-
> > > 2 files changed, 18 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> > > index 4206dc6d813f..b1c717286993 100644
> > > --- a/include/net/af_vsock.h
> > > +++ b/include/net/af_vsock.h
> > > @@ -98,6 +98,8 @@ struct vsock_transport_send_notify_data {
> > > #define VSOCK_TRANSPORT_F_G2H 0x00000002
> > > /* Transport provides DGRAM communication */
> > > #define VSOCK_TRANSPORT_F_DGRAM 0x00000004
> > > +/* Transport provides local (loopback) communication */
> > > +#define VSOCK_TRANSPORT_F_LOCAL 0x00000008
> > >
> > > struct vsock_transport {
> > > struct module *module;
> > > diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> > > index cc8659838bf2..c9e5bad59dc1 100644
> > > --- a/net/vmw_vsock/af_vsock.c
> > > +++ b/net/vmw_vsock/af_vsock.c
> > > @@ -136,6 +136,8 @@ static const struct vsock_transport
> *transport_h2g;
> > > static const struct vsock_transport *transport_g2h;
> > > /* Transport used for DGRAM communication */
> > > static const struct vsock_transport *transport_dgram;
> > > +/* Transport used for local communication */
> > > +static const struct vsock_transport *transport_local;
> > > static DEFINE_MUTEX(vsock_register_mutex);
> > >
> > > /**** UTILS ****/
> > > @@ -2130,7 +2132,7 @@
> EXPORT_SYMBOL_GPL(vsock_core_get_transport);
> > >
> > > int vsock_core_register(const struct vsock_transport *t, int features)
> > > {
> > > - const struct vsock_transport *t_h2g, *t_g2h, *t_dgram;
> > > + const struct vsock_transport *t_h2g, *t_g2h, *t_dgram, *t_local;
> > > int err = mutex_lock_interruptible(&vsock_register_mutex);
> > >
> > > if (err)
> > > @@ -2139,6 +2141,7 @@ int vsock_core_register(const struct
> > > vsock_transport *t, int features)
> > > t_h2g = transport_h2g;
> > > t_g2h = transport_g2h;
> > > t_dgram = transport_dgram;
> > > + t_local = transport_local;
> > >
> > > if (features & VSOCK_TRANSPORT_F_H2G) {
> > > if (t_h2g) {
> > > @@ -2164,9 +2167,18 @@ int vsock_core_register(const struct
> > > vsock_transport *t, int features)
> > > t_dgram = t;
> > > }
> > >
> > > + if (features & VSOCK_TRANSPORT_F_LOCAL) {
> > > + if (t_local) {
> > > + err = -EBUSY;
> > > + goto err_busy;
> > > + }
> > > + t_local = t;
> > > + }
> > > +
> > > transport_h2g = t_h2g;
> > > transport_g2h = t_g2h;
> > > transport_dgram = t_dgram;
> > > + transport_local = t_local;
> > >
> > > err_busy:
> > > mutex_unlock(&vsock_register_mutex);
> > > @@ -2187,6 +2199,9 @@ void vsock_core_unregister(const struct
> > > vsock_transport *t)
> > > if (transport_dgram == t)
> > > transport_dgram = NULL;
> > >
> > > + if (transport_local == t)
> > > + transport_local = NULL;
> > > +
> > > mutex_unlock(&vsock_register_mutex);
> > > }
> > > EXPORT_SYMBOL_GPL(vsock_core_unregister);
> > > --
> > > 2.21.0
> >
> > Having loopback support as a separate transport fits nicely, but do we need
> to support
> > different variants of loopback? It could just be built in.
>
> I agree with you, indeed initially I developed it as built in, but
> DEPMOD found a cyclic dependency because vsock_transport use
> virtio_transport_common that use vsock, so if I include vsock_transport
> in the vsock module, DEPMOD is not happy.
>
> I don't know how to break this cyclic dependency, do you have any ideas?
One way to view this would be that the loopback transport and the support
it uses from virtio_transport_common are independent of virtio as such,
and could be part of the af_vsock module if loopback is configured. So
in a way, the virtio g2h and h2g transports would be extensions of the
built in loopback transport. But that brings in quite a bit of code so
it could be better to just keep it as is.
Thanks,
Jorgen
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 3/6] vsock: add local transport support in the vsock core
2019-11-21 15:53 ` Jorgen Hansen
@ 2019-11-21 16:12 ` Stefano Garzarella
2019-11-21 16:19 ` Jorgen Hansen
2019-11-21 16:19 ` Jorgen Hansen via Virtualization
2019-11-21 16:12 ` Stefano Garzarella
1 sibling, 2 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-21 16:12 UTC (permalink / raw)
To: Jorgen Hansen
Cc: netdev, virtualization, Dexuan Cui, Stefan Hajnoczi,
linux-kernel, kvm, David S. Miller
On Thu, Nov 21, 2019 at 03:53:47PM +0000, Jorgen Hansen wrote:
> > From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> > Sent: Thursday, November 21, 2019 4:22 PM
> >
> > On Thu, Nov 21, 2019 at 03:04:18PM +0000, Jorgen Hansen wrote:
> > > > From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> > > > Sent: Tuesday, November 19, 2019 12:01 PM
> > > > To: netdev@vger.kernel.org
> > > >
> > > > This patch allows to register a transport able to handle
> > > > local communication (loopback).
> > > >
> > > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > > ---
> > > > include/net/af_vsock.h | 2 ++
> > > > net/vmw_vsock/af_vsock.c | 17 ++++++++++++++++-
> > > > 2 files changed, 18 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> > > > index 4206dc6d813f..b1c717286993 100644
> > > > --- a/include/net/af_vsock.h
> > > > +++ b/include/net/af_vsock.h
> > > > @@ -98,6 +98,8 @@ struct vsock_transport_send_notify_data {
> > > > #define VSOCK_TRANSPORT_F_G2H 0x00000002
> > > > /* Transport provides DGRAM communication */
> > > > #define VSOCK_TRANSPORT_F_DGRAM 0x00000004
> > > > +/* Transport provides local (loopback) communication */
> > > > +#define VSOCK_TRANSPORT_F_LOCAL 0x00000008
> > > >
> > > > struct vsock_transport {
> > > > struct module *module;
> > > > diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> > > > index cc8659838bf2..c9e5bad59dc1 100644
> > > > --- a/net/vmw_vsock/af_vsock.c
> > > > +++ b/net/vmw_vsock/af_vsock.c
> > > > @@ -136,6 +136,8 @@ static const struct vsock_transport
> > *transport_h2g;
> > > > static const struct vsock_transport *transport_g2h;
> > > > /* Transport used for DGRAM communication */
> > > > static const struct vsock_transport *transport_dgram;
> > > > +/* Transport used for local communication */
> > > > +static const struct vsock_transport *transport_local;
> > > > static DEFINE_MUTEX(vsock_register_mutex);
> > > >
> > > > /**** UTILS ****/
> > > > @@ -2130,7 +2132,7 @@
> > EXPORT_SYMBOL_GPL(vsock_core_get_transport);
> > > >
> > > > int vsock_core_register(const struct vsock_transport *t, int features)
> > > > {
> > > > - const struct vsock_transport *t_h2g, *t_g2h, *t_dgram;
> > > > + const struct vsock_transport *t_h2g, *t_g2h, *t_dgram, *t_local;
> > > > int err = mutex_lock_interruptible(&vsock_register_mutex);
> > > >
> > > > if (err)
> > > > @@ -2139,6 +2141,7 @@ int vsock_core_register(const struct
> > > > vsock_transport *t, int features)
> > > > t_h2g = transport_h2g;
> > > > t_g2h = transport_g2h;
> > > > t_dgram = transport_dgram;
> > > > + t_local = transport_local;
> > > >
> > > > if (features & VSOCK_TRANSPORT_F_H2G) {
> > > > if (t_h2g) {
> > > > @@ -2164,9 +2167,18 @@ int vsock_core_register(const struct
> > > > vsock_transport *t, int features)
> > > > t_dgram = t;
> > > > }
> > > >
> > > > + if (features & VSOCK_TRANSPORT_F_LOCAL) {
> > > > + if (t_local) {
> > > > + err = -EBUSY;
> > > > + goto err_busy;
> > > > + }
> > > > + t_local = t;
> > > > + }
> > > > +
> > > > transport_h2g = t_h2g;
> > > > transport_g2h = t_g2h;
> > > > transport_dgram = t_dgram;
> > > > + transport_local = t_local;
> > > >
> > > > err_busy:
> > > > mutex_unlock(&vsock_register_mutex);
> > > > @@ -2187,6 +2199,9 @@ void vsock_core_unregister(const struct
> > > > vsock_transport *t)
> > > > if (transport_dgram == t)
> > > > transport_dgram = NULL;
> > > >
> > > > + if (transport_local == t)
> > > > + transport_local = NULL;
> > > > +
> > > > mutex_unlock(&vsock_register_mutex);
> > > > }
> > > > EXPORT_SYMBOL_GPL(vsock_core_unregister);
> > > > --
> > > > 2.21.0
> > >
> > > Having loopback support as a separate transport fits nicely, but do we need
> > to support
> > > different variants of loopback? It could just be built in.
> >
> > I agree with you, indeed initially I developed it as built in, but
> > DEPMOD found a cyclic dependency because vsock_transport use
> > virtio_transport_common that use vsock, so if I include vsock_transport
> > in the vsock module, DEPMOD is not happy.
> >
> > I don't know how to break this cyclic dependency, do you have any ideas?
>
> One way to view this would be that the loopback transport and the support
> it uses from virtio_transport_common are independent of virtio as such,
> and could be part of the af_vsock module if loopback is configured. So
> in a way, the virtio g2h and h2g transports would be extensions of the
> built in loopback transport. But that brings in quite a bit of code so
> it could be better to just keep it as is.
Great idea!
Stefan already suggested (as a long-term goal) to rename the generic
functionality in virtio_transport_common.c
Maybe I can do both in another series later on, since it requires enough
changes.
Thanks,
Stefano
^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH net-next 3/6] vsock: add local transport support in the vsock core
2019-11-21 16:12 ` Stefano Garzarella
@ 2019-11-21 16:19 ` Jorgen Hansen
2019-11-21 16:19 ` Jorgen Hansen via Virtualization
1 sibling, 0 replies; 43+ messages in thread
From: Jorgen Hansen @ 2019-11-21 16:19 UTC (permalink / raw)
To: 'Stefano Garzarella'
Cc: netdev, virtualization, Dexuan Cui, Stefan Hajnoczi,
linux-kernel, kvm, David S. Miller
> From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> Sent: Thursday, November 21, 2019 5:13 PM
>
> On Thu, Nov 21, 2019 at 03:53:47PM +0000, Jorgen Hansen wrote:
> > > From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> > > Sent: Thursday, November 21, 2019 4:22 PM
> > >
> > > On Thu, Nov 21, 2019 at 03:04:18PM +0000, Jorgen Hansen wrote:
> > > > > From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> > > > > Sent: Tuesday, November 19, 2019 12:01 PM
> > > > > To: netdev@vger.kernel.org
> > > > >
> > > > > This patch allows to register a transport able to handle
> > > > > local communication (loopback).
> > > > >
> > > > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > > > ---
> > > > > include/net/af_vsock.h | 2 ++
> > > > > net/vmw_vsock/af_vsock.c | 17 ++++++++++++++++-
> > > > > 2 files changed, 18 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> > > > > index 4206dc6d813f..b1c717286993 100644
> > > > > --- a/include/net/af_vsock.h
> > > > > +++ b/include/net/af_vsock.h
> > > > > @@ -98,6 +98,8 @@ struct vsock_transport_send_notify_data {
> > > > > #define VSOCK_TRANSPORT_F_G2H 0x00000002
> > > > > /* Transport provides DGRAM communication */
> > > > > #define VSOCK_TRANSPORT_F_DGRAM 0x00000004
> > > > > +/* Transport provides local (loopback) communication */
> > > > > +#define VSOCK_TRANSPORT_F_LOCAL 0x00000008
> > > > >
> > > > > struct vsock_transport {
> > > > > struct module *module;
> > > > > diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> > > > > index cc8659838bf2..c9e5bad59dc1 100644
> > > > > --- a/net/vmw_vsock/af_vsock.c
> > > > > +++ b/net/vmw_vsock/af_vsock.c
> > > > > @@ -136,6 +136,8 @@ static const struct vsock_transport
> > > *transport_h2g;
> > > > > static const struct vsock_transport *transport_g2h;
> > > > > /* Transport used for DGRAM communication */
> > > > > static const struct vsock_transport *transport_dgram;
> > > > > +/* Transport used for local communication */
> > > > > +static const struct vsock_transport *transport_local;
> > > > > static DEFINE_MUTEX(vsock_register_mutex);
> > > > >
> > > > > /**** UTILS ****/
> > > > > @@ -2130,7 +2132,7 @@
> > > EXPORT_SYMBOL_GPL(vsock_core_get_transport);
> > > > >
> > > > > int vsock_core_register(const struct vsock_transport *t, int features)
> > > > > {
> > > > > - const struct vsock_transport *t_h2g, *t_g2h, *t_dgram;
> > > > > + const struct vsock_transport *t_h2g, *t_g2h, *t_dgram,
> *t_local;
> > > > > int err = mutex_lock_interruptible(&vsock_register_mutex);
> > > > >
> > > > > if (err)
> > > > > @@ -2139,6 +2141,7 @@ int vsock_core_register(const struct
> > > > > vsock_transport *t, int features)
> > > > > t_h2g = transport_h2g;
> > > > > t_g2h = transport_g2h;
> > > > > t_dgram = transport_dgram;
> > > > > + t_local = transport_local;
> > > > >
> > > > > if (features & VSOCK_TRANSPORT_F_H2G) {
> > > > > if (t_h2g) {
> > > > > @@ -2164,9 +2167,18 @@ int vsock_core_register(const struct
> > > > > vsock_transport *t, int features)
> > > > > t_dgram = t;
> > > > > }
> > > > >
> > > > > + if (features & VSOCK_TRANSPORT_F_LOCAL) {
> > > > > + if (t_local) {
> > > > > + err = -EBUSY;
> > > > > + goto err_busy;
> > > > > + }
> > > > > + t_local = t;
> > > > > + }
> > > > > +
> > > > > transport_h2g = t_h2g;
> > > > > transport_g2h = t_g2h;
> > > > > transport_dgram = t_dgram;
> > > > > + transport_local = t_local;
> > > > >
> > > > > err_busy:
> > > > > mutex_unlock(&vsock_register_mutex);
> > > > > @@ -2187,6 +2199,9 @@ void vsock_core_unregister(const struct
> > > > > vsock_transport *t)
> > > > > if (transport_dgram == t)
> > > > > transport_dgram = NULL;
> > > > >
> > > > > + if (transport_local == t)
> > > > > + transport_local = NULL;
> > > > > +
> > > > > mutex_unlock(&vsock_register_mutex);
> > > > > }
> > > > > EXPORT_SYMBOL_GPL(vsock_core_unregister);
> > > > > --
> > > > > 2.21.0
> > > >
> > > > Having loopback support as a separate transport fits nicely, but do we
> need
> > > to support
> > > > different variants of loopback? It could just be built in.
> > >
> > > I agree with you, indeed initially I developed it as built in, but
> > > DEPMOD found a cyclic dependency because vsock_transport use
> > > virtio_transport_common that use vsock, so if I include vsock_transport
> > > in the vsock module, DEPMOD is not happy.
> > >
> > > I don't know how to break this cyclic dependency, do you have any ideas?
> >
> > One way to view this would be that the loopback transport and the support
> > it uses from virtio_transport_common are independent of virtio as such,
> > and could be part of the af_vsock module if loopback is configured. So
> > in a way, the virtio g2h and h2g transports would be extensions of the
> > built in loopback transport. But that brings in quite a bit of code so
> > it could be better to just keep it as is.
>
> Great idea!
>
> Stefan already suggested (as a long-term goal) to rename the generic
> functionality in virtio_transport_common.c
>
> Maybe I can do both in another series later on, since it requires enough
> changes.
Sounds good to me.
Thanks,
Jorgen
^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH net-next 3/6] vsock: add local transport support in the vsock core
2019-11-21 16:12 ` Stefano Garzarella
2019-11-21 16:19 ` Jorgen Hansen
@ 2019-11-21 16:19 ` Jorgen Hansen via Virtualization
1 sibling, 0 replies; 43+ messages in thread
From: Jorgen Hansen via Virtualization @ 2019-11-21 16:19 UTC (permalink / raw)
To: 'Stefano Garzarella'
Cc: kvm, netdev, Dexuan Cui, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller
> From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> Sent: Thursday, November 21, 2019 5:13 PM
>
> On Thu, Nov 21, 2019 at 03:53:47PM +0000, Jorgen Hansen wrote:
> > > From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> > > Sent: Thursday, November 21, 2019 4:22 PM
> > >
> > > On Thu, Nov 21, 2019 at 03:04:18PM +0000, Jorgen Hansen wrote:
> > > > > From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> > > > > Sent: Tuesday, November 19, 2019 12:01 PM
> > > > > To: netdev@vger.kernel.org
> > > > >
> > > > > This patch allows to register a transport able to handle
> > > > > local communication (loopback).
> > > > >
> > > > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > > > ---
> > > > > include/net/af_vsock.h | 2 ++
> > > > > net/vmw_vsock/af_vsock.c | 17 ++++++++++++++++-
> > > > > 2 files changed, 18 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> > > > > index 4206dc6d813f..b1c717286993 100644
> > > > > --- a/include/net/af_vsock.h
> > > > > +++ b/include/net/af_vsock.h
> > > > > @@ -98,6 +98,8 @@ struct vsock_transport_send_notify_data {
> > > > > #define VSOCK_TRANSPORT_F_G2H 0x00000002
> > > > > /* Transport provides DGRAM communication */
> > > > > #define VSOCK_TRANSPORT_F_DGRAM 0x00000004
> > > > > +/* Transport provides local (loopback) communication */
> > > > > +#define VSOCK_TRANSPORT_F_LOCAL 0x00000008
> > > > >
> > > > > struct vsock_transport {
> > > > > struct module *module;
> > > > > diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> > > > > index cc8659838bf2..c9e5bad59dc1 100644
> > > > > --- a/net/vmw_vsock/af_vsock.c
> > > > > +++ b/net/vmw_vsock/af_vsock.c
> > > > > @@ -136,6 +136,8 @@ static const struct vsock_transport
> > > *transport_h2g;
> > > > > static const struct vsock_transport *transport_g2h;
> > > > > /* Transport used for DGRAM communication */
> > > > > static const struct vsock_transport *transport_dgram;
> > > > > +/* Transport used for local communication */
> > > > > +static const struct vsock_transport *transport_local;
> > > > > static DEFINE_MUTEX(vsock_register_mutex);
> > > > >
> > > > > /**** UTILS ****/
> > > > > @@ -2130,7 +2132,7 @@
> > > EXPORT_SYMBOL_GPL(vsock_core_get_transport);
> > > > >
> > > > > int vsock_core_register(const struct vsock_transport *t, int features)
> > > > > {
> > > > > - const struct vsock_transport *t_h2g, *t_g2h, *t_dgram;
> > > > > + const struct vsock_transport *t_h2g, *t_g2h, *t_dgram,
> *t_local;
> > > > > int err = mutex_lock_interruptible(&vsock_register_mutex);
> > > > >
> > > > > if (err)
> > > > > @@ -2139,6 +2141,7 @@ int vsock_core_register(const struct
> > > > > vsock_transport *t, int features)
> > > > > t_h2g = transport_h2g;
> > > > > t_g2h = transport_g2h;
> > > > > t_dgram = transport_dgram;
> > > > > + t_local = transport_local;
> > > > >
> > > > > if (features & VSOCK_TRANSPORT_F_H2G) {
> > > > > if (t_h2g) {
> > > > > @@ -2164,9 +2167,18 @@ int vsock_core_register(const struct
> > > > > vsock_transport *t, int features)
> > > > > t_dgram = t;
> > > > > }
> > > > >
> > > > > + if (features & VSOCK_TRANSPORT_F_LOCAL) {
> > > > > + if (t_local) {
> > > > > + err = -EBUSY;
> > > > > + goto err_busy;
> > > > > + }
> > > > > + t_local = t;
> > > > > + }
> > > > > +
> > > > > transport_h2g = t_h2g;
> > > > > transport_g2h = t_g2h;
> > > > > transport_dgram = t_dgram;
> > > > > + transport_local = t_local;
> > > > >
> > > > > err_busy:
> > > > > mutex_unlock(&vsock_register_mutex);
> > > > > @@ -2187,6 +2199,9 @@ void vsock_core_unregister(const struct
> > > > > vsock_transport *t)
> > > > > if (transport_dgram == t)
> > > > > transport_dgram = NULL;
> > > > >
> > > > > + if (transport_local == t)
> > > > > + transport_local = NULL;
> > > > > +
> > > > > mutex_unlock(&vsock_register_mutex);
> > > > > }
> > > > > EXPORT_SYMBOL_GPL(vsock_core_unregister);
> > > > > --
> > > > > 2.21.0
> > > >
> > > > Having loopback support as a separate transport fits nicely, but do we
> need
> > > to support
> > > > different variants of loopback? It could just be built in.
> > >
> > > I agree with you, indeed initially I developed it as built in, but
> > > DEPMOD found a cyclic dependency because vsock_transport use
> > > virtio_transport_common that use vsock, so if I include vsock_transport
> > > in the vsock module, DEPMOD is not happy.
> > >
> > > I don't know how to break this cyclic dependency, do you have any ideas?
> >
> > One way to view this would be that the loopback transport and the support
> > it uses from virtio_transport_common are independent of virtio as such,
> > and could be part of the af_vsock module if loopback is configured. So
> > in a way, the virtio g2h and h2g transports would be extensions of the
> > built in loopback transport. But that brings in quite a bit of code so
> > it could be better to just keep it as is.
>
> Great idea!
>
> Stefan already suggested (as a long-term goal) to rename the generic
> functionality in virtio_transport_common.c
>
> Maybe I can do both in another series later on, since it requires enough
> changes.
Sounds good to me.
Thanks,
Jorgen
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 3/6] vsock: add local transport support in the vsock core
2019-11-21 15:53 ` Jorgen Hansen
2019-11-21 16:12 ` Stefano Garzarella
@ 2019-11-21 16:12 ` Stefano Garzarella
1 sibling, 0 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-21 16:12 UTC (permalink / raw)
To: Jorgen Hansen
Cc: kvm, netdev, Dexuan Cui, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller
On Thu, Nov 21, 2019 at 03:53:47PM +0000, Jorgen Hansen wrote:
> > From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> > Sent: Thursday, November 21, 2019 4:22 PM
> >
> > On Thu, Nov 21, 2019 at 03:04:18PM +0000, Jorgen Hansen wrote:
> > > > From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> > > > Sent: Tuesday, November 19, 2019 12:01 PM
> > > > To: netdev@vger.kernel.org
> > > >
> > > > This patch allows to register a transport able to handle
> > > > local communication (loopback).
> > > >
> > > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > > ---
> > > > include/net/af_vsock.h | 2 ++
> > > > net/vmw_vsock/af_vsock.c | 17 ++++++++++++++++-
> > > > 2 files changed, 18 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> > > > index 4206dc6d813f..b1c717286993 100644
> > > > --- a/include/net/af_vsock.h
> > > > +++ b/include/net/af_vsock.h
> > > > @@ -98,6 +98,8 @@ struct vsock_transport_send_notify_data {
> > > > #define VSOCK_TRANSPORT_F_G2H 0x00000002
> > > > /* Transport provides DGRAM communication */
> > > > #define VSOCK_TRANSPORT_F_DGRAM 0x00000004
> > > > +/* Transport provides local (loopback) communication */
> > > > +#define VSOCK_TRANSPORT_F_LOCAL 0x00000008
> > > >
> > > > struct vsock_transport {
> > > > struct module *module;
> > > > diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> > > > index cc8659838bf2..c9e5bad59dc1 100644
> > > > --- a/net/vmw_vsock/af_vsock.c
> > > > +++ b/net/vmw_vsock/af_vsock.c
> > > > @@ -136,6 +136,8 @@ static const struct vsock_transport
> > *transport_h2g;
> > > > static const struct vsock_transport *transport_g2h;
> > > > /* Transport used for DGRAM communication */
> > > > static const struct vsock_transport *transport_dgram;
> > > > +/* Transport used for local communication */
> > > > +static const struct vsock_transport *transport_local;
> > > > static DEFINE_MUTEX(vsock_register_mutex);
> > > >
> > > > /**** UTILS ****/
> > > > @@ -2130,7 +2132,7 @@
> > EXPORT_SYMBOL_GPL(vsock_core_get_transport);
> > > >
> > > > int vsock_core_register(const struct vsock_transport *t, int features)
> > > > {
> > > > - const struct vsock_transport *t_h2g, *t_g2h, *t_dgram;
> > > > + const struct vsock_transport *t_h2g, *t_g2h, *t_dgram, *t_local;
> > > > int err = mutex_lock_interruptible(&vsock_register_mutex);
> > > >
> > > > if (err)
> > > > @@ -2139,6 +2141,7 @@ int vsock_core_register(const struct
> > > > vsock_transport *t, int features)
> > > > t_h2g = transport_h2g;
> > > > t_g2h = transport_g2h;
> > > > t_dgram = transport_dgram;
> > > > + t_local = transport_local;
> > > >
> > > > if (features & VSOCK_TRANSPORT_F_H2G) {
> > > > if (t_h2g) {
> > > > @@ -2164,9 +2167,18 @@ int vsock_core_register(const struct
> > > > vsock_transport *t, int features)
> > > > t_dgram = t;
> > > > }
> > > >
> > > > + if (features & VSOCK_TRANSPORT_F_LOCAL) {
> > > > + if (t_local) {
> > > > + err = -EBUSY;
> > > > + goto err_busy;
> > > > + }
> > > > + t_local = t;
> > > > + }
> > > > +
> > > > transport_h2g = t_h2g;
> > > > transport_g2h = t_g2h;
> > > > transport_dgram = t_dgram;
> > > > + transport_local = t_local;
> > > >
> > > > err_busy:
> > > > mutex_unlock(&vsock_register_mutex);
> > > > @@ -2187,6 +2199,9 @@ void vsock_core_unregister(const struct
> > > > vsock_transport *t)
> > > > if (transport_dgram == t)
> > > > transport_dgram = NULL;
> > > >
> > > > + if (transport_local == t)
> > > > + transport_local = NULL;
> > > > +
> > > > mutex_unlock(&vsock_register_mutex);
> > > > }
> > > > EXPORT_SYMBOL_GPL(vsock_core_unregister);
> > > > --
> > > > 2.21.0
> > >
> > > Having loopback support as a separate transport fits nicely, but do we need
> > to support
> > > different variants of loopback? It could just be built in.
> >
> > I agree with you, indeed initially I developed it as built in, but
> > DEPMOD found a cyclic dependency because vsock_transport use
> > virtio_transport_common that use vsock, so if I include vsock_transport
> > in the vsock module, DEPMOD is not happy.
> >
> > I don't know how to break this cyclic dependency, do you have any ideas?
>
> One way to view this would be that the loopback transport and the support
> it uses from virtio_transport_common are independent of virtio as such,
> and could be part of the af_vsock module if loopback is configured. So
> in a way, the virtio g2h and h2g transports would be extensions of the
> built in loopback transport. But that brings in quite a bit of code so
> it could be better to just keep it as is.
Great idea!
Stefan already suggested (as a long-term goal) to rename the generic
functionality in virtio_transport_common.c
Maybe I can do both in another series later on, since it requires enough
changes.
Thanks,
Stefano
^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH net-next 3/6] vsock: add local transport support in the vsock core
2019-11-21 15:21 ` Stefano Garzarella
2019-11-21 15:53 ` Jorgen Hansen
@ 2019-11-21 15:53 ` Jorgen Hansen via Virtualization
1 sibling, 0 replies; 43+ messages in thread
From: Jorgen Hansen via Virtualization @ 2019-11-21 15:53 UTC (permalink / raw)
To: 'Stefano Garzarella'
Cc: kvm, netdev, Dexuan Cui, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller
> From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> Sent: Thursday, November 21, 2019 4:22 PM
>
> On Thu, Nov 21, 2019 at 03:04:18PM +0000, Jorgen Hansen wrote:
> > > From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> > > Sent: Tuesday, November 19, 2019 12:01 PM
> > > To: netdev@vger.kernel.org
> > >
> > > This patch allows to register a transport able to handle
> > > local communication (loopback).
> > >
> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > ---
> > > include/net/af_vsock.h | 2 ++
> > > net/vmw_vsock/af_vsock.c | 17 ++++++++++++++++-
> > > 2 files changed, 18 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> > > index 4206dc6d813f..b1c717286993 100644
> > > --- a/include/net/af_vsock.h
> > > +++ b/include/net/af_vsock.h
> > > @@ -98,6 +98,8 @@ struct vsock_transport_send_notify_data {
> > > #define VSOCK_TRANSPORT_F_G2H 0x00000002
> > > /* Transport provides DGRAM communication */
> > > #define VSOCK_TRANSPORT_F_DGRAM 0x00000004
> > > +/* Transport provides local (loopback) communication */
> > > +#define VSOCK_TRANSPORT_F_LOCAL 0x00000008
> > >
> > > struct vsock_transport {
> > > struct module *module;
> > > diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> > > index cc8659838bf2..c9e5bad59dc1 100644
> > > --- a/net/vmw_vsock/af_vsock.c
> > > +++ b/net/vmw_vsock/af_vsock.c
> > > @@ -136,6 +136,8 @@ static const struct vsock_transport
> *transport_h2g;
> > > static const struct vsock_transport *transport_g2h;
> > > /* Transport used for DGRAM communication */
> > > static const struct vsock_transport *transport_dgram;
> > > +/* Transport used for local communication */
> > > +static const struct vsock_transport *transport_local;
> > > static DEFINE_MUTEX(vsock_register_mutex);
> > >
> > > /**** UTILS ****/
> > > @@ -2130,7 +2132,7 @@
> EXPORT_SYMBOL_GPL(vsock_core_get_transport);
> > >
> > > int vsock_core_register(const struct vsock_transport *t, int features)
> > > {
> > > - const struct vsock_transport *t_h2g, *t_g2h, *t_dgram;
> > > + const struct vsock_transport *t_h2g, *t_g2h, *t_dgram, *t_local;
> > > int err = mutex_lock_interruptible(&vsock_register_mutex);
> > >
> > > if (err)
> > > @@ -2139,6 +2141,7 @@ int vsock_core_register(const struct
> > > vsock_transport *t, int features)
> > > t_h2g = transport_h2g;
> > > t_g2h = transport_g2h;
> > > t_dgram = transport_dgram;
> > > + t_local = transport_local;
> > >
> > > if (features & VSOCK_TRANSPORT_F_H2G) {
> > > if (t_h2g) {
> > > @@ -2164,9 +2167,18 @@ int vsock_core_register(const struct
> > > vsock_transport *t, int features)
> > > t_dgram = t;
> > > }
> > >
> > > + if (features & VSOCK_TRANSPORT_F_LOCAL) {
> > > + if (t_local) {
> > > + err = -EBUSY;
> > > + goto err_busy;
> > > + }
> > > + t_local = t;
> > > + }
> > > +
> > > transport_h2g = t_h2g;
> > > transport_g2h = t_g2h;
> > > transport_dgram = t_dgram;
> > > + transport_local = t_local;
> > >
> > > err_busy:
> > > mutex_unlock(&vsock_register_mutex);
> > > @@ -2187,6 +2199,9 @@ void vsock_core_unregister(const struct
> > > vsock_transport *t)
> > > if (transport_dgram == t)
> > > transport_dgram = NULL;
> > >
> > > + if (transport_local == t)
> > > + transport_local = NULL;
> > > +
> > > mutex_unlock(&vsock_register_mutex);
> > > }
> > > EXPORT_SYMBOL_GPL(vsock_core_unregister);
> > > --
> > > 2.21.0
> >
> > Having loopback support as a separate transport fits nicely, but do we need
> to support
> > different variants of loopback? It could just be built in.
>
> I agree with you, indeed initially I developed it as built in, but
> DEPMOD found a cyclic dependency because vsock_transport use
> virtio_transport_common that use vsock, so if I include vsock_transport
> in the vsock module, DEPMOD is not happy.
>
> I don't know how to break this cyclic dependency, do you have any ideas?
One way to view this would be that the loopback transport and the support
it uses from virtio_transport_common are independent of virtio as such,
and could be part of the af_vsock module if loopback is configured. So
in a way, the virtio g2h and h2g transports would be extensions of the
built in loopback transport. But that brings in quite a bit of code so
it could be better to just keep it as is.
Thanks,
Jorgen
^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH net-next 3/6] vsock: add local transport support in the vsock core
2019-11-19 11:01 ` [PATCH net-next 3/6] vsock: add local transport support in the vsock core Stefano Garzarella
2019-11-21 15:04 ` Jorgen Hansen
@ 2019-11-21 15:04 ` Jorgen Hansen via Virtualization
1 sibling, 0 replies; 43+ messages in thread
From: Jorgen Hansen via Virtualization @ 2019-11-21 15:04 UTC (permalink / raw)
To: 'Stefano Garzarella', netdev
Cc: kvm, Dexuan Cui, linux-kernel, virtualization, Stefan Hajnoczi,
David S. Miller
> From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> Sent: Tuesday, November 19, 2019 12:01 PM
> To: netdev@vger.kernel.org
>
> This patch allows to register a transport able to handle
> local communication (loopback).
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> include/net/af_vsock.h | 2 ++
> net/vmw_vsock/af_vsock.c | 17 ++++++++++++++++-
> 2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> index 4206dc6d813f..b1c717286993 100644
> --- a/include/net/af_vsock.h
> +++ b/include/net/af_vsock.h
> @@ -98,6 +98,8 @@ struct vsock_transport_send_notify_data {
> #define VSOCK_TRANSPORT_F_G2H 0x00000002
> /* Transport provides DGRAM communication */
> #define VSOCK_TRANSPORT_F_DGRAM 0x00000004
> +/* Transport provides local (loopback) communication */
> +#define VSOCK_TRANSPORT_F_LOCAL 0x00000008
>
> struct vsock_transport {
> struct module *module;
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index cc8659838bf2..c9e5bad59dc1 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -136,6 +136,8 @@ static const struct vsock_transport *transport_h2g;
> static const struct vsock_transport *transport_g2h;
> /* Transport used for DGRAM communication */
> static const struct vsock_transport *transport_dgram;
> +/* Transport used for local communication */
> +static const struct vsock_transport *transport_local;
> static DEFINE_MUTEX(vsock_register_mutex);
>
> /**** UTILS ****/
> @@ -2130,7 +2132,7 @@ EXPORT_SYMBOL_GPL(vsock_core_get_transport);
>
> int vsock_core_register(const struct vsock_transport *t, int features)
> {
> - const struct vsock_transport *t_h2g, *t_g2h, *t_dgram;
> + const struct vsock_transport *t_h2g, *t_g2h, *t_dgram, *t_local;
> int err = mutex_lock_interruptible(&vsock_register_mutex);
>
> if (err)
> @@ -2139,6 +2141,7 @@ int vsock_core_register(const struct
> vsock_transport *t, int features)
> t_h2g = transport_h2g;
> t_g2h = transport_g2h;
> t_dgram = transport_dgram;
> + t_local = transport_local;
>
> if (features & VSOCK_TRANSPORT_F_H2G) {
> if (t_h2g) {
> @@ -2164,9 +2167,18 @@ int vsock_core_register(const struct
> vsock_transport *t, int features)
> t_dgram = t;
> }
>
> + if (features & VSOCK_TRANSPORT_F_LOCAL) {
> + if (t_local) {
> + err = -EBUSY;
> + goto err_busy;
> + }
> + t_local = t;
> + }
> +
> transport_h2g = t_h2g;
> transport_g2h = t_g2h;
> transport_dgram = t_dgram;
> + transport_local = t_local;
>
> err_busy:
> mutex_unlock(&vsock_register_mutex);
> @@ -2187,6 +2199,9 @@ void vsock_core_unregister(const struct
> vsock_transport *t)
> if (transport_dgram == t)
> transport_dgram = NULL;
>
> + if (transport_local == t)
> + transport_local = NULL;
> +
> mutex_unlock(&vsock_register_mutex);
> }
> EXPORT_SYMBOL_GPL(vsock_core_unregister);
> --
> 2.21.0
Having loopback support as a separate transport fits nicely, but do we need to support
different variants of loopback? It could just be built in.
Thanks,
Jorgen
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH net-next 4/6] vsock: add vsock_loopback transport
2019-11-19 11:01 [PATCH net-next 0/6] vsock: add local transport support Stefano Garzarella
` (2 preceding siblings ...)
2019-11-19 11:01 ` [PATCH net-next 3/6] vsock: add local transport support in the vsock core Stefano Garzarella
@ 2019-11-19 11:01 ` Stefano Garzarella
2019-11-20 1:15 ` David Miller
` (2 more replies)
2019-11-19 11:01 ` [PATCH net-next 5/6] vsock: use local transport when it is loaded Stefano Garzarella
` (5 subsequent siblings)
9 siblings, 3 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-19 11:01 UTC (permalink / raw)
To: netdev
Cc: virtualization, Dexuan Cui, Stefan Hajnoczi, linux-kernel, kvm,
David S. Miller, Jorgen Hansen
This patch adds a new vsock_loopback transport to handle local
communication.
This transport is based on the loopback implementation of
virtio_transport, so it uses the virtio_transport_common APIs
to interface with the vsock core.
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
MAINTAINERS | 1 +
net/vmw_vsock/Kconfig | 12 ++
net/vmw_vsock/Makefile | 1 +
net/vmw_vsock/vsock_loopback.c | 217 +++++++++++++++++++++++++++++++++
4 files changed, 231 insertions(+)
create mode 100644 net/vmw_vsock/vsock_loopback.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 760049454a23..c2a3dc3113ba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17239,6 +17239,7 @@ F: net/vmw_vsock/diag.c
F: net/vmw_vsock/af_vsock_tap.c
F: net/vmw_vsock/virtio_transport_common.c
F: net/vmw_vsock/virtio_transport.c
+F: net/vmw_vsock/vsock_loopback.c
F: drivers/net/vsockmon.c
F: drivers/vhost/vsock.c
F: tools/testing/vsock/
diff --git a/net/vmw_vsock/Kconfig b/net/vmw_vsock/Kconfig
index 8abcb815af2d..56356d2980c8 100644
--- a/net/vmw_vsock/Kconfig
+++ b/net/vmw_vsock/Kconfig
@@ -26,6 +26,18 @@ config VSOCKETS_DIAG
Enable this module so userspace applications can query open sockets.
+config VSOCKETS_LOOPBACK
+ tristate "Virtual Sockets loopback transport"
+ depends on VSOCKETS
+ default y
+ select VIRTIO_VSOCKETS_COMMON
+ help
+ This module implements a loopback transport for Virtual Sockets,
+ using vmw_vsock_virtio_transport_common.
+
+ To compile this driver as a module, choose M here: the module
+ will be called vsock_loopback. If unsure, say N.
+
config VMWARE_VMCI_VSOCKETS
tristate "VMware VMCI transport for Virtual Sockets"
depends on VSOCKETS && VMWARE_VMCI
diff --git a/net/vmw_vsock/Makefile b/net/vmw_vsock/Makefile
index 7c6f9a0b67b0..6a943ec95c4a 100644
--- a/net/vmw_vsock/Makefile
+++ b/net/vmw_vsock/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_VMWARE_VMCI_VSOCKETS) += vmw_vsock_vmci_transport.o
obj-$(CONFIG_VIRTIO_VSOCKETS) += vmw_vsock_virtio_transport.o
obj-$(CONFIG_VIRTIO_VSOCKETS_COMMON) += vmw_vsock_virtio_transport_common.o
obj-$(CONFIG_HYPERV_VSOCKETS) += hv_sock.o
+obj-$(CONFIG_VSOCKETS_LOOPBACK) += vsock_loopback.o
vsock-y += af_vsock.o af_vsock_tap.o vsock_addr.o
diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
new file mode 100644
index 000000000000..3d1c1a88305f
--- /dev/null
+++ b/net/vmw_vsock/vsock_loopback.c
@@ -0,0 +1,217 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * loopback transport for vsock using virtio_transport_common APIs
+ *
+ * Copyright (C) 2013-2019 Red Hat, Inc.
+ * Author: Asias He <asias@redhat.com>
+ * Stefan Hajnoczi <stefanha@redhat.com>
+ * Stefano Garzarella <sgarzare@redhat.com>
+ *
+ */
+#include <linux/spinlock.h>
+#include <linux/module.h>
+#include <linux/list.h>
+#include <linux/virtio_vsock.h>
+
+static struct workqueue_struct *vsock_loopback_workqueue;
+static struct vsock_loopback *the_vsock_loopback;
+
+struct vsock_loopback {
+ spinlock_t loopback_list_lock; /* protects loopback_list */
+ struct list_head loopback_list;
+ struct work_struct loopback_work;
+};
+
+static u32 vsock_loopback_get_local_cid(void)
+{
+ return VMADDR_CID_LOCAL;
+}
+
+static int vsock_loopback_send_pkt(struct virtio_vsock_pkt *pkt)
+{
+ struct vsock_loopback *vsock;
+ int len = pkt->len;
+
+ rcu_read_lock();
+ vsock = rcu_dereference(the_vsock_loopback);
+ if (!vsock) {
+ virtio_transport_free_pkt(pkt);
+ len = -ENODEV;
+ goto out_rcu;
+ }
+
+ spin_lock_bh(&vsock->loopback_list_lock);
+ list_add_tail(&pkt->list, &vsock->loopback_list);
+ spin_unlock_bh(&vsock->loopback_list_lock);
+
+ queue_work(vsock_loopback_workqueue, &vsock->loopback_work);
+
+out_rcu:
+ rcu_read_unlock();
+ return len;
+}
+
+static int vsock_loopback_cancel_pkt(struct vsock_sock *vsk)
+{
+ struct vsock_loopback *vsock;
+ struct virtio_vsock_pkt *pkt, *n;
+ int ret;
+ LIST_HEAD(freeme);
+
+ rcu_read_lock();
+ vsock = rcu_dereference(the_vsock_loopback);
+ if (!vsock) {
+ ret = -ENODEV;
+ goto out_rcu;
+ }
+
+ spin_lock_bh(&vsock->loopback_list_lock);
+ list_for_each_entry_safe(pkt, n, &vsock->loopback_list, list) {
+ if (pkt->vsk != vsk)
+ continue;
+ list_move(&pkt->list, &freeme);
+ }
+ spin_unlock_bh(&vsock->loopback_list_lock);
+
+ list_for_each_entry_safe(pkt, n, &freeme, list) {
+ list_del(&pkt->list);
+ virtio_transport_free_pkt(pkt);
+ }
+
+ ret = 0;
+
+out_rcu:
+ rcu_read_unlock();
+ return ret;
+}
+
+static struct virtio_transport loopback_transport = {
+ .transport = {
+ .module = THIS_MODULE,
+
+ .get_local_cid = vsock_loopback_get_local_cid,
+
+ .init = virtio_transport_do_socket_init,
+ .destruct = virtio_transport_destruct,
+ .release = virtio_transport_release,
+ .connect = virtio_transport_connect,
+ .shutdown = virtio_transport_shutdown,
+ .cancel_pkt = vsock_loopback_cancel_pkt,
+
+ .dgram_bind = virtio_transport_dgram_bind,
+ .dgram_dequeue = virtio_transport_dgram_dequeue,
+ .dgram_enqueue = virtio_transport_dgram_enqueue,
+ .dgram_allow = virtio_transport_dgram_allow,
+
+ .stream_dequeue = virtio_transport_stream_dequeue,
+ .stream_enqueue = virtio_transport_stream_enqueue,
+ .stream_has_data = virtio_transport_stream_has_data,
+ .stream_has_space = virtio_transport_stream_has_space,
+ .stream_rcvhiwat = virtio_transport_stream_rcvhiwat,
+ .stream_is_active = virtio_transport_stream_is_active,
+ .stream_allow = virtio_transport_stream_allow,
+
+ .notify_poll_in = virtio_transport_notify_poll_in,
+ .notify_poll_out = virtio_transport_notify_poll_out,
+ .notify_recv_init = virtio_transport_notify_recv_init,
+ .notify_recv_pre_block = virtio_transport_notify_recv_pre_block,
+ .notify_recv_pre_dequeue = virtio_transport_notify_recv_pre_dequeue,
+ .notify_recv_post_dequeue = virtio_transport_notify_recv_post_dequeue,
+ .notify_send_init = virtio_transport_notify_send_init,
+ .notify_send_pre_block = virtio_transport_notify_send_pre_block,
+ .notify_send_pre_enqueue = virtio_transport_notify_send_pre_enqueue,
+ .notify_send_post_enqueue = virtio_transport_notify_send_post_enqueue,
+ .notify_buffer_size = virtio_transport_notify_buffer_size,
+ },
+
+ .send_pkt = vsock_loopback_send_pkt,
+};
+
+static void vsock_loopback_work(struct work_struct *work)
+{
+ struct vsock_loopback *vsock =
+ container_of(work, struct vsock_loopback, loopback_work);
+ LIST_HEAD(pkts);
+
+ spin_lock_bh(&vsock->loopback_list_lock);
+ list_splice_init(&vsock->loopback_list, &pkts);
+ spin_unlock_bh(&vsock->loopback_list_lock);
+
+ while (!list_empty(&pkts)) {
+ struct virtio_vsock_pkt *pkt;
+
+ pkt = list_first_entry(&pkts, struct virtio_vsock_pkt, list);
+ list_del_init(&pkt->list);
+
+ virtio_transport_deliver_tap_pkt(pkt);
+ virtio_transport_recv_pkt(&loopback_transport, pkt);
+ }
+}
+
+static int __init vsock_loopback_init(void)
+{
+ struct vsock_loopback *vsock = NULL;
+ int ret;
+
+ vsock_loopback_workqueue = alloc_workqueue("vsock-loopback", 0, 0);
+ if (!vsock_loopback_workqueue)
+ return -ENOMEM;
+
+ vsock = kzalloc(sizeof(*vsock), GFP_KERNEL);
+ if (!vsock) {
+ ret = -ENOMEM;
+ goto out_wq;
+ }
+
+ spin_lock_init(&vsock->loopback_list_lock);
+ INIT_LIST_HEAD(&vsock->loopback_list);
+ INIT_WORK(&vsock->loopback_work, vsock_loopback_work);
+
+ rcu_assign_pointer(the_vsock_loopback, vsock);
+
+ ret = vsock_core_register(&loopback_transport.transport,
+ VSOCK_TRANSPORT_F_LOCAL);
+ if (ret)
+ goto out_free;
+
+ return 0;
+
+out_free:
+ rcu_assign_pointer(the_vsock_loopback, NULL);
+ kfree(vsock);
+out_wq:
+ destroy_workqueue(vsock_loopback_workqueue);
+ return ret;
+}
+
+static void __exit vsock_loopback_exit(void)
+{
+ struct vsock_loopback *vsock = the_vsock_loopback;
+ struct virtio_vsock_pkt *pkt;
+
+ vsock_core_unregister(&loopback_transport.transport);
+
+ rcu_assign_pointer(the_vsock_loopback, NULL);
+ synchronize_rcu();
+
+ spin_lock_bh(&vsock->loopback_list_lock);
+ while (!list_empty(&vsock->loopback_list)) {
+ pkt = list_first_entry(&vsock->loopback_list,
+ struct virtio_vsock_pkt, list);
+ list_del(&pkt->list);
+ virtio_transport_free_pkt(pkt);
+ }
+ spin_unlock_bh(&vsock->loopback_list_lock);
+
+ flush_work(&vsock->loopback_work);
+
+ kfree(vsock);
+ destroy_workqueue(vsock_loopback_workqueue);
+}
+
+module_init(vsock_loopback_init);
+module_exit(vsock_loopback_exit);
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Stefano Garzarella <sgarzare@redhat.com>");
+MODULE_DESCRIPTION("loopback transport for vsock");
+MODULE_ALIAS_NETPROTO(PF_VSOCK);
--
2.21.0
^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 4/6] vsock: add vsock_loopback transport
2019-11-19 11:01 ` [PATCH net-next 4/6] vsock: add vsock_loopback transport Stefano Garzarella
@ 2019-11-20 1:15 ` David Miller
2019-11-20 9:00 ` Stefano Garzarella
2019-11-21 9:34 ` Stefan Hajnoczi
2019-11-21 9:34 ` Stefan Hajnoczi
2 siblings, 1 reply; 43+ messages in thread
From: David Miller @ 2019-11-20 1:15 UTC (permalink / raw)
To: sgarzare
Cc: netdev, virtualization, decui, stefanha, linux-kernel, kvm, jhansen
From: Stefano Garzarella <sgarzare@redhat.com>
Date: Tue, 19 Nov 2019 12:01:19 +0100
> +static int vsock_loopback_cancel_pkt(struct vsock_sock *vsk)
> +{
> + struct vsock_loopback *vsock;
> + struct virtio_vsock_pkt *pkt, *n;
> + int ret;
> + LIST_HEAD(freeme);
Reverse christmas tree ordering of local variables here please.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 4/6] vsock: add vsock_loopback transport
2019-11-20 1:15 ` David Miller
@ 2019-11-20 9:00 ` Stefano Garzarella
0 siblings, 0 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-20 9:00 UTC (permalink / raw)
To: David Miller
Cc: netdev, virtualization, decui, stefanha, linux-kernel, kvm, jhansen
On Tue, Nov 19, 2019 at 05:15:01PM -0800, David Miller wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
> Date: Tue, 19 Nov 2019 12:01:19 +0100
>
> > +static int vsock_loopback_cancel_pkt(struct vsock_sock *vsk)
> > +{
> > + struct vsock_loopback *vsock;
> > + struct virtio_vsock_pkt *pkt, *n;
> > + int ret;
> > + LIST_HEAD(freeme);
>
> Reverse christmas tree ordering of local variables here please.
>
Sure, I'll fix in the v2.
Thanks,
Stefano
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 4/6] vsock: add vsock_loopback transport
2019-11-19 11:01 ` [PATCH net-next 4/6] vsock: add vsock_loopback transport Stefano Garzarella
2019-11-20 1:15 ` David Miller
@ 2019-11-21 9:34 ` Stefan Hajnoczi
2019-11-21 9:34 ` Stefan Hajnoczi
2 siblings, 0 replies; 43+ messages in thread
From: Stefan Hajnoczi @ 2019-11-21 9:34 UTC (permalink / raw)
To: Stefano Garzarella
Cc: kvm, netdev, Dexuan Cui, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller, Jorgen Hansen
[-- Attachment #1.1: Type: text/plain, Size: 2388 bytes --]
On Tue, Nov 19, 2019 at 12:01:19PM +0100, Stefano Garzarella wrote:
Ideas for long-term changes below.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 760049454a23..c2a3dc3113ba 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17239,6 +17239,7 @@ F: net/vmw_vsock/diag.c
> F: net/vmw_vsock/af_vsock_tap.c
> F: net/vmw_vsock/virtio_transport_common.c
> F: net/vmw_vsock/virtio_transport.c
> +F: net/vmw_vsock/vsock_loopback.c
> F: drivers/net/vsockmon.c
> F: drivers/vhost/vsock.c
> F: tools/testing/vsock/
At this point you are most active in virtio-vsock and I am reviewing
patches on a best-effort basis. Feel free to add yourself as
maintainer.
> diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
> new file mode 100644
> index 000000000000..3d1c1a88305f
> --- /dev/null
> +++ b/net/vmw_vsock/vsock_loopback.c
> @@ -0,0 +1,217 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * loopback transport for vsock using virtio_transport_common APIs
> + *
> + * Copyright (C) 2013-2019 Red Hat, Inc.
> + * Author: Asias He <asias@redhat.com>
> + * Stefan Hajnoczi <stefanha@redhat.com>
> + * Stefano Garzarella <sgarzare@redhat.com>
> + *
> + */
> +#include <linux/spinlock.h>
> +#include <linux/module.h>
> +#include <linux/list.h>
> +#include <linux/virtio_vsock.h>
Is it time to rename the generic functionality in
virtio_transport_common.c? This doesn't have anything to do with virtio
:).
> +
> +static struct workqueue_struct *vsock_loopback_workqueue;
> +static struct vsock_loopback *the_vsock_loopback;
the_vsock_loopback could be a static global variable (not a pointer) and
vsock_loopback_workqueue could also be included in the struct.
The RCU pointer is really a way to synchronize vsock_loopback_send_pkt()
and vsock_loopback_cancel_pkt() with module exit. There is no other
reason for using a pointer.
It's cleaner to implement the synchronization once in af_vsock.c (or
virtio_transport_common.c) instead of making each transport do it.
Maybe try_module_get() and related APIs provide the necessary semantics
so that core vsock code can hold the transport module while it's being
used to send/cancel a packet.
> +MODULE_ALIAS_NETPROTO(PF_VSOCK);
Why does this module define the alias for PF_VSOCK? Doesn't another
module already define this alias?
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 4/6] vsock: add vsock_loopback transport
2019-11-19 11:01 ` [PATCH net-next 4/6] vsock: add vsock_loopback transport Stefano Garzarella
2019-11-20 1:15 ` David Miller
2019-11-21 9:34 ` Stefan Hajnoczi
@ 2019-11-21 9:34 ` Stefan Hajnoczi
2019-11-21 9:59 ` Stefano Garzarella
2019-11-21 9:59 ` Stefano Garzarella
2 siblings, 2 replies; 43+ messages in thread
From: Stefan Hajnoczi @ 2019-11-21 9:34 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, virtualization, Dexuan Cui, Stefan Hajnoczi,
linux-kernel, kvm, David S. Miller, Jorgen Hansen
[-- Attachment #1: Type: text/plain, Size: 2388 bytes --]
On Tue, Nov 19, 2019 at 12:01:19PM +0100, Stefano Garzarella wrote:
Ideas for long-term changes below.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 760049454a23..c2a3dc3113ba 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17239,6 +17239,7 @@ F: net/vmw_vsock/diag.c
> F: net/vmw_vsock/af_vsock_tap.c
> F: net/vmw_vsock/virtio_transport_common.c
> F: net/vmw_vsock/virtio_transport.c
> +F: net/vmw_vsock/vsock_loopback.c
> F: drivers/net/vsockmon.c
> F: drivers/vhost/vsock.c
> F: tools/testing/vsock/
At this point you are most active in virtio-vsock and I am reviewing
patches on a best-effort basis. Feel free to add yourself as
maintainer.
> diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
> new file mode 100644
> index 000000000000..3d1c1a88305f
> --- /dev/null
> +++ b/net/vmw_vsock/vsock_loopback.c
> @@ -0,0 +1,217 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * loopback transport for vsock using virtio_transport_common APIs
> + *
> + * Copyright (C) 2013-2019 Red Hat, Inc.
> + * Author: Asias He <asias@redhat.com>
> + * Stefan Hajnoczi <stefanha@redhat.com>
> + * Stefano Garzarella <sgarzare@redhat.com>
> + *
> + */
> +#include <linux/spinlock.h>
> +#include <linux/module.h>
> +#include <linux/list.h>
> +#include <linux/virtio_vsock.h>
Is it time to rename the generic functionality in
virtio_transport_common.c? This doesn't have anything to do with virtio
:).
> +
> +static struct workqueue_struct *vsock_loopback_workqueue;
> +static struct vsock_loopback *the_vsock_loopback;
the_vsock_loopback could be a static global variable (not a pointer) and
vsock_loopback_workqueue could also be included in the struct.
The RCU pointer is really a way to synchronize vsock_loopback_send_pkt()
and vsock_loopback_cancel_pkt() with module exit. There is no other
reason for using a pointer.
It's cleaner to implement the synchronization once in af_vsock.c (or
virtio_transport_common.c) instead of making each transport do it.
Maybe try_module_get() and related APIs provide the necessary semantics
so that core vsock code can hold the transport module while it's being
used to send/cancel a packet.
> +MODULE_ALIAS_NETPROTO(PF_VSOCK);
Why does this module define the alias for PF_VSOCK? Doesn't another
module already define this alias?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 4/6] vsock: add vsock_loopback transport
2019-11-21 9:34 ` Stefan Hajnoczi
@ 2019-11-21 9:59 ` Stefano Garzarella
2019-11-21 9:59 ` Stefano Garzarella
1 sibling, 0 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-21 9:59 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: kvm, netdev, Dexuan Cui, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller, Jorgen Hansen
On Thu, Nov 21, 2019 at 09:34:58AM +0000, Stefan Hajnoczi wrote:
> On Tue, Nov 19, 2019 at 12:01:19PM +0100, Stefano Garzarella wrote:
>
> Ideas for long-term changes below.
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>
Thanks for reviewing!
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 760049454a23..c2a3dc3113ba 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -17239,6 +17239,7 @@ F: net/vmw_vsock/diag.c
> > F: net/vmw_vsock/af_vsock_tap.c
> > F: net/vmw_vsock/virtio_transport_common.c
> > F: net/vmw_vsock/virtio_transport.c
> > +F: net/vmw_vsock/vsock_loopback.c
> > F: drivers/net/vsockmon.c
> > F: drivers/vhost/vsock.c
> > F: tools/testing/vsock/
>
> At this point you are most active in virtio-vsock and I am reviewing
> patches on a best-effort basis. Feel free to add yourself as
> maintainer.
>
Sure, I'd be happy to maintain it.
> > diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
> > new file mode 100644
> > index 000000000000..3d1c1a88305f
> > --- /dev/null
> > +++ b/net/vmw_vsock/vsock_loopback.c
> > @@ -0,0 +1,217 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * loopback transport for vsock using virtio_transport_common APIs
> > + *
> > + * Copyright (C) 2013-2019 Red Hat, Inc.
> > + * Author: Asias He <asias@redhat.com>
> > + * Stefan Hajnoczi <stefanha@redhat.com>
> > + * Stefano Garzarella <sgarzare@redhat.com>
> > + *
> > + */
> > +#include <linux/spinlock.h>
> > +#include <linux/module.h>
> > +#include <linux/list.h>
> > +#include <linux/virtio_vsock.h>
>
> Is it time to rename the generic functionality in
> virtio_transport_common.c? This doesn't have anything to do with virtio
> :).
>
Completely agree, new transports could use it to handle the protocol without
reimplementing things already done.
> > +
> > +static struct workqueue_struct *vsock_loopback_workqueue;
> > +static struct vsock_loopback *the_vsock_loopback;
>
> the_vsock_loopback could be a static global variable (not a pointer) and
> vsock_loopback_workqueue could also be included in the struct.
>
> The RCU pointer is really a way to synchronize vsock_loopback_send_pkt()
> and vsock_loopback_cancel_pkt() with module exit. There is no other
> reason for using a pointer.
>
> It's cleaner to implement the synchronization once in af_vsock.c (or
> virtio_transport_common.c) instead of making each transport do it.
> Maybe try_module_get() and related APIs provide the necessary semantics
> so that core vsock code can hold the transport module while it's being
> used to send/cancel a packet.
Right, the module cannot be unloaded until open sockets, so here the
synchronization is not needed.
The synchronization come from virtio-vsock device that can be
hot-unplugged while sockets are still open, but that can't happen here.
I will remove the pointers and RCU in the v2.
Can I keep your R-b or do you prefer to watch v2 first?
>
> > +MODULE_ALIAS_NETPROTO(PF_VSOCK);
>
> Why does this module define the alias for PF_VSOCK? Doesn't another
> module already define this alias?
It is a way to load this module when PF_VSOCK is starting to be used.
MODULE_ALIAS_NETPROTO(PF_VSOCK) is already defined in vmci_transport
and hyperv_transport. IIUC it is used for the same reason.
In virtio_transport we don't need it because it will be loaded when
the PCI device is discovered.
Do you think there's a better way?
Should I include the vsock_loopback transport directly in af_vsock
without creating a new module?
Thanks,
Stefano
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 4/6] vsock: add vsock_loopback transport
2019-11-21 9:34 ` Stefan Hajnoczi
2019-11-21 9:59 ` Stefano Garzarella
@ 2019-11-21 9:59 ` Stefano Garzarella
2019-11-21 15:25 ` Stefano Garzarella
2019-11-21 15:25 ` Stefano Garzarella
1 sibling, 2 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-21 9:59 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: netdev, virtualization, Dexuan Cui, Stefan Hajnoczi,
linux-kernel, kvm, David S. Miller, Jorgen Hansen
On Thu, Nov 21, 2019 at 09:34:58AM +0000, Stefan Hajnoczi wrote:
> On Tue, Nov 19, 2019 at 12:01:19PM +0100, Stefano Garzarella wrote:
>
> Ideas for long-term changes below.
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>
Thanks for reviewing!
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 760049454a23..c2a3dc3113ba 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -17239,6 +17239,7 @@ F: net/vmw_vsock/diag.c
> > F: net/vmw_vsock/af_vsock_tap.c
> > F: net/vmw_vsock/virtio_transport_common.c
> > F: net/vmw_vsock/virtio_transport.c
> > +F: net/vmw_vsock/vsock_loopback.c
> > F: drivers/net/vsockmon.c
> > F: drivers/vhost/vsock.c
> > F: tools/testing/vsock/
>
> At this point you are most active in virtio-vsock and I am reviewing
> patches on a best-effort basis. Feel free to add yourself as
> maintainer.
>
Sure, I'd be happy to maintain it.
> > diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
> > new file mode 100644
> > index 000000000000..3d1c1a88305f
> > --- /dev/null
> > +++ b/net/vmw_vsock/vsock_loopback.c
> > @@ -0,0 +1,217 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * loopback transport for vsock using virtio_transport_common APIs
> > + *
> > + * Copyright (C) 2013-2019 Red Hat, Inc.
> > + * Author: Asias He <asias@redhat.com>
> > + * Stefan Hajnoczi <stefanha@redhat.com>
> > + * Stefano Garzarella <sgarzare@redhat.com>
> > + *
> > + */
> > +#include <linux/spinlock.h>
> > +#include <linux/module.h>
> > +#include <linux/list.h>
> > +#include <linux/virtio_vsock.h>
>
> Is it time to rename the generic functionality in
> virtio_transport_common.c? This doesn't have anything to do with virtio
> :).
>
Completely agree, new transports could use it to handle the protocol without
reimplementing things already done.
> > +
> > +static struct workqueue_struct *vsock_loopback_workqueue;
> > +static struct vsock_loopback *the_vsock_loopback;
>
> the_vsock_loopback could be a static global variable (not a pointer) and
> vsock_loopback_workqueue could also be included in the struct.
>
> The RCU pointer is really a way to synchronize vsock_loopback_send_pkt()
> and vsock_loopback_cancel_pkt() with module exit. There is no other
> reason for using a pointer.
>
> It's cleaner to implement the synchronization once in af_vsock.c (or
> virtio_transport_common.c) instead of making each transport do it.
> Maybe try_module_get() and related APIs provide the necessary semantics
> so that core vsock code can hold the transport module while it's being
> used to send/cancel a packet.
Right, the module cannot be unloaded until open sockets, so here the
synchronization is not needed.
The synchronization come from virtio-vsock device that can be
hot-unplugged while sockets are still open, but that can't happen here.
I will remove the pointers and RCU in the v2.
Can I keep your R-b or do you prefer to watch v2 first?
>
> > +MODULE_ALIAS_NETPROTO(PF_VSOCK);
>
> Why does this module define the alias for PF_VSOCK? Doesn't another
> module already define this alias?
It is a way to load this module when PF_VSOCK is starting to be used.
MODULE_ALIAS_NETPROTO(PF_VSOCK) is already defined in vmci_transport
and hyperv_transport. IIUC it is used for the same reason.
In virtio_transport we don't need it because it will be loaded when
the PCI device is discovered.
Do you think there's a better way?
Should I include the vsock_loopback transport directly in af_vsock
without creating a new module?
Thanks,
Stefano
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 4/6] vsock: add vsock_loopback transport
2019-11-21 9:59 ` Stefano Garzarella
@ 2019-11-21 15:25 ` Stefano Garzarella
2019-11-22 9:25 ` Stefan Hajnoczi
2019-11-22 9:25 ` Stefan Hajnoczi
2019-11-21 15:25 ` Stefano Garzarella
1 sibling, 2 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-21 15:25 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: netdev, virtualization, Dexuan Cui, Stefan Hajnoczi,
linux-kernel, kvm, David S. Miller, Jorgen Hansen
On Thu, Nov 21, 2019 at 10:59:48AM +0100, Stefano Garzarella wrote:
> On Thu, Nov 21, 2019 at 09:34:58AM +0000, Stefan Hajnoczi wrote:
> > On Tue, Nov 19, 2019 at 12:01:19PM +0100, Stefano Garzarella wrote:
> >
> > Ideas for long-term changes below.
> >
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> >
>
> Thanks for reviewing!
>
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 760049454a23..c2a3dc3113ba 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -17239,6 +17239,7 @@ F: net/vmw_vsock/diag.c
> > > F: net/vmw_vsock/af_vsock_tap.c
> > > F: net/vmw_vsock/virtio_transport_common.c
> > > F: net/vmw_vsock/virtio_transport.c
> > > +F: net/vmw_vsock/vsock_loopback.c
> > > F: drivers/net/vsockmon.c
> > > F: drivers/vhost/vsock.c
> > > F: tools/testing/vsock/
> >
> > At this point you are most active in virtio-vsock and I am reviewing
> > patches on a best-effort basis. Feel free to add yourself as
> > maintainer.
> >
>
> Sure, I'd be happy to maintain it.
>
> > > diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
> > > new file mode 100644
> > > index 000000000000..3d1c1a88305f
> > > --- /dev/null
> > > +++ b/net/vmw_vsock/vsock_loopback.c
> > > @@ -0,0 +1,217 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * loopback transport for vsock using virtio_transport_common APIs
> > > + *
> > > + * Copyright (C) 2013-2019 Red Hat, Inc.
> > > + * Author: Asias He <asias@redhat.com>
> > > + * Stefan Hajnoczi <stefanha@redhat.com>
> > > + * Stefano Garzarella <sgarzare@redhat.com>
> > > + *
> > > + */
> > > +#include <linux/spinlock.h>
> > > +#include <linux/module.h>
> > > +#include <linux/list.h>
> > > +#include <linux/virtio_vsock.h>
> >
> > Is it time to rename the generic functionality in
> > virtio_transport_common.c? This doesn't have anything to do with virtio
> > :).
> >
>
> Completely agree, new transports could use it to handle the protocol without
> reimplementing things already done.
>
> > > +
> > > +static struct workqueue_struct *vsock_loopback_workqueue;
> > > +static struct vsock_loopback *the_vsock_loopback;
> >
> > the_vsock_loopback could be a static global variable (not a pointer) and
> > vsock_loopback_workqueue could also be included in the struct.
> >
> > The RCU pointer is really a way to synchronize vsock_loopback_send_pkt()
> > and vsock_loopback_cancel_pkt() with module exit. There is no other
> > reason for using a pointer.
> >
> > It's cleaner to implement the synchronization once in af_vsock.c (or
> > virtio_transport_common.c) instead of making each transport do it.
> > Maybe try_module_get() and related APIs provide the necessary semantics
> > so that core vsock code can hold the transport module while it's being
> > used to send/cancel a packet.
>
> Right, the module cannot be unloaded until open sockets, so here the
> synchronization is not needed.
>
> The synchronization come from virtio-vsock device that can be
> hot-unplugged while sockets are still open, but that can't happen here.
>
> I will remove the pointers and RCU in the v2.
>
> Can I keep your R-b or do you prefer to watch v2 first?
>
> >
> > > +MODULE_ALIAS_NETPROTO(PF_VSOCK);
> >
> > Why does this module define the alias for PF_VSOCK? Doesn't another
> > module already define this alias?
>
> It is a way to load this module when PF_VSOCK is starting to be used.
> MODULE_ALIAS_NETPROTO(PF_VSOCK) is already defined in vmci_transport
> and hyperv_transport. IIUC it is used for the same reason.
>
> In virtio_transport we don't need it because it will be loaded when
> the PCI device is discovered.
>
> Do you think there's a better way?
> Should I include the vsock_loopback transport directly in af_vsock
> without creating a new module?
>
That last thing I said may not be possible:
I remembered that I tried, but DEPMOD found a cyclic dependency because
vsock_transport use virtio_transport_common that use vsock, so if I
include vsock_transport in the vsock module, DEPMOD is not happy.
Do you think it's okay in this case to keep MODULE_ALIAS_NETPROTO(PF_VSOCK)
or is there a better way?
Thanks,
Stefano
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 4/6] vsock: add vsock_loopback transport
2019-11-21 15:25 ` Stefano Garzarella
@ 2019-11-22 9:25 ` Stefan Hajnoczi
2019-11-22 9:25 ` Stefan Hajnoczi
1 sibling, 0 replies; 43+ messages in thread
From: Stefan Hajnoczi @ 2019-11-22 9:25 UTC (permalink / raw)
To: Stefano Garzarella
Cc: kvm, Dexuan Cui, linux-kernel, virtualization, netdev,
David S. Miller, Jorgen Hansen
[-- Attachment #1.1: Type: text/plain, Size: 2762 bytes --]
On Thu, Nov 21, 2019 at 04:25:17PM +0100, Stefano Garzarella wrote:
> On Thu, Nov 21, 2019 at 10:59:48AM +0100, Stefano Garzarella wrote:
> > On Thu, Nov 21, 2019 at 09:34:58AM +0000, Stefan Hajnoczi wrote:
> > > On Tue, Nov 19, 2019 at 12:01:19PM +0100, Stefano Garzarella wrote:
> > > > +static struct workqueue_struct *vsock_loopback_workqueue;
> > > > +static struct vsock_loopback *the_vsock_loopback;
> > >
> > > the_vsock_loopback could be a static global variable (not a pointer) and
> > > vsock_loopback_workqueue could also be included in the struct.
> > >
> > > The RCU pointer is really a way to synchronize vsock_loopback_send_pkt()
> > > and vsock_loopback_cancel_pkt() with module exit. There is no other
> > > reason for using a pointer.
> > >
> > > It's cleaner to implement the synchronization once in af_vsock.c (or
> > > virtio_transport_common.c) instead of making each transport do it.
> > > Maybe try_module_get() and related APIs provide the necessary semantics
> > > so that core vsock code can hold the transport module while it's being
> > > used to send/cancel a packet.
> >
> > Right, the module cannot be unloaded until open sockets, so here the
> > synchronization is not needed.
> >
> > The synchronization come from virtio-vsock device that can be
> > hot-unplugged while sockets are still open, but that can't happen here.
> >
> > I will remove the pointers and RCU in the v2.
> >
> > Can I keep your R-b or do you prefer to watch v2 first?
I'd like to review v2.
> > > > +MODULE_ALIAS_NETPROTO(PF_VSOCK);
> > >
> > > Why does this module define the alias for PF_VSOCK? Doesn't another
> > > module already define this alias?
> >
> > It is a way to load this module when PF_VSOCK is starting to be used.
> > MODULE_ALIAS_NETPROTO(PF_VSOCK) is already defined in vmci_transport
> > and hyperv_transport. IIUC it is used for the same reason.
> >
> > In virtio_transport we don't need it because it will be loaded when
> > the PCI device is discovered.
> >
> > Do you think there's a better way?
> > Should I include the vsock_loopback transport directly in af_vsock
> > without creating a new module?
> >
>
> That last thing I said may not be possible:
> I remembered that I tried, but DEPMOD found a cyclic dependency because
> vsock_transport use virtio_transport_common that use vsock, so if I
> include vsock_transport in the vsock module, DEPMOD is not happy.
>
> Do you think it's okay in this case to keep MODULE_ALIAS_NETPROTO(PF_VSOCK)
> or is there a better way?
The reason I asked is because the semantics of duplicate module aliases
aren't clear to me. Do all modules with the same alias get loaded?
Or just the first? Or ...?
Stefan
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 4/6] vsock: add vsock_loopback transport
2019-11-21 15:25 ` Stefano Garzarella
2019-11-22 9:25 ` Stefan Hajnoczi
@ 2019-11-22 9:25 ` Stefan Hajnoczi
2019-11-22 10:02 ` Stefano Garzarella
1 sibling, 1 reply; 43+ messages in thread
From: Stefan Hajnoczi @ 2019-11-22 9:25 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Stefan Hajnoczi, netdev, virtualization, Dexuan Cui,
linux-kernel, kvm, David S. Miller, Jorgen Hansen
[-- Attachment #1: Type: text/plain, Size: 2762 bytes --]
On Thu, Nov 21, 2019 at 04:25:17PM +0100, Stefano Garzarella wrote:
> On Thu, Nov 21, 2019 at 10:59:48AM +0100, Stefano Garzarella wrote:
> > On Thu, Nov 21, 2019 at 09:34:58AM +0000, Stefan Hajnoczi wrote:
> > > On Tue, Nov 19, 2019 at 12:01:19PM +0100, Stefano Garzarella wrote:
> > > > +static struct workqueue_struct *vsock_loopback_workqueue;
> > > > +static struct vsock_loopback *the_vsock_loopback;
> > >
> > > the_vsock_loopback could be a static global variable (not a pointer) and
> > > vsock_loopback_workqueue could also be included in the struct.
> > >
> > > The RCU pointer is really a way to synchronize vsock_loopback_send_pkt()
> > > and vsock_loopback_cancel_pkt() with module exit. There is no other
> > > reason for using a pointer.
> > >
> > > It's cleaner to implement the synchronization once in af_vsock.c (or
> > > virtio_transport_common.c) instead of making each transport do it.
> > > Maybe try_module_get() and related APIs provide the necessary semantics
> > > so that core vsock code can hold the transport module while it's being
> > > used to send/cancel a packet.
> >
> > Right, the module cannot be unloaded until open sockets, so here the
> > synchronization is not needed.
> >
> > The synchronization come from virtio-vsock device that can be
> > hot-unplugged while sockets are still open, but that can't happen here.
> >
> > I will remove the pointers and RCU in the v2.
> >
> > Can I keep your R-b or do you prefer to watch v2 first?
I'd like to review v2.
> > > > +MODULE_ALIAS_NETPROTO(PF_VSOCK);
> > >
> > > Why does this module define the alias for PF_VSOCK? Doesn't another
> > > module already define this alias?
> >
> > It is a way to load this module when PF_VSOCK is starting to be used.
> > MODULE_ALIAS_NETPROTO(PF_VSOCK) is already defined in vmci_transport
> > and hyperv_transport. IIUC it is used for the same reason.
> >
> > In virtio_transport we don't need it because it will be loaded when
> > the PCI device is discovered.
> >
> > Do you think there's a better way?
> > Should I include the vsock_loopback transport directly in af_vsock
> > without creating a new module?
> >
>
> That last thing I said may not be possible:
> I remembered that I tried, but DEPMOD found a cyclic dependency because
> vsock_transport use virtio_transport_common that use vsock, so if I
> include vsock_transport in the vsock module, DEPMOD is not happy.
>
> Do you think it's okay in this case to keep MODULE_ALIAS_NETPROTO(PF_VSOCK)
> or is there a better way?
The reason I asked is because the semantics of duplicate module aliases
aren't clear to me. Do all modules with the same alias get loaded?
Or just the first? Or ...?
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 4/6] vsock: add vsock_loopback transport
2019-11-22 9:25 ` Stefan Hajnoczi
@ 2019-11-22 10:02 ` Stefano Garzarella
0 siblings, 0 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-22 10:02 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Stefan Hajnoczi, netdev, virtualization, Dexuan Cui,
linux-kernel, kvm, David S. Miller, Jorgen Hansen
On Fri, Nov 22, 2019 at 09:25:46AM +0000, Stefan Hajnoczi wrote:
> On Thu, Nov 21, 2019 at 04:25:17PM +0100, Stefano Garzarella wrote:
> > On Thu, Nov 21, 2019 at 10:59:48AM +0100, Stefano Garzarella wrote:
> > > On Thu, Nov 21, 2019 at 09:34:58AM +0000, Stefan Hajnoczi wrote:
> > > > On Tue, Nov 19, 2019 at 12:01:19PM +0100, Stefano Garzarella wrote:
> > > > > +static struct workqueue_struct *vsock_loopback_workqueue;
> > > > > +static struct vsock_loopback *the_vsock_loopback;
> > > >
> > > > the_vsock_loopback could be a static global variable (not a pointer) and
> > > > vsock_loopback_workqueue could also be included in the struct.
> > > >
> > > > The RCU pointer is really a way to synchronize vsock_loopback_send_pkt()
> > > > and vsock_loopback_cancel_pkt() with module exit. There is no other
> > > > reason for using a pointer.
> > > >
> > > > It's cleaner to implement the synchronization once in af_vsock.c (or
> > > > virtio_transport_common.c) instead of making each transport do it.
> > > > Maybe try_module_get() and related APIs provide the necessary semantics
> > > > so that core vsock code can hold the transport module while it's being
> > > > used to send/cancel a packet.
> > >
> > > Right, the module cannot be unloaded until open sockets, so here the
> > > synchronization is not needed.
> > >
> > > The synchronization come from virtio-vsock device that can be
> > > hot-unplugged while sockets are still open, but that can't happen here.
> > >
> > > I will remove the pointers and RCU in the v2.
> > >
> > > Can I keep your R-b or do you prefer to watch v2 first?
>
> I'd like to review v2.
>
Sure!
> > > > > +MODULE_ALIAS_NETPROTO(PF_VSOCK);
> > > >
> > > > Why does this module define the alias for PF_VSOCK? Doesn't another
> > > > module already define this alias?
> > >
> > > It is a way to load this module when PF_VSOCK is starting to be used.
> > > MODULE_ALIAS_NETPROTO(PF_VSOCK) is already defined in vmci_transport
> > > and hyperv_transport. IIUC it is used for the same reason.
> > >
> > > In virtio_transport we don't need it because it will be loaded when
> > > the PCI device is discovered.
> > >
> > > Do you think there's a better way?
> > > Should I include the vsock_loopback transport directly in af_vsock
> > > without creating a new module?
> > >
> >
> > That last thing I said may not be possible:
> > I remembered that I tried, but DEPMOD found a cyclic dependency because
> > vsock_transport use virtio_transport_common that use vsock, so if I
> > include vsock_transport in the vsock module, DEPMOD is not happy.
> >
> > Do you think it's okay in this case to keep MODULE_ALIAS_NETPROTO(PF_VSOCK)
> > or is there a better way?
>
> The reason I asked is because the semantics of duplicate module aliases
> aren't clear to me. Do all modules with the same alias get loaded?
> Or just the first? Or ...?
It wasn't clear to me either, but when I tried, I saw that all modules
with the same alias got loaded.
Stefano
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 4/6] vsock: add vsock_loopback transport
@ 2019-11-22 10:02 ` Stefano Garzarella
0 siblings, 0 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-22 10:02 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: kvm, Dexuan Cui, linux-kernel, virtualization, netdev,
David S. Miller, Jorgen Hansen
On Fri, Nov 22, 2019 at 09:25:46AM +0000, Stefan Hajnoczi wrote:
> On Thu, Nov 21, 2019 at 04:25:17PM +0100, Stefano Garzarella wrote:
> > On Thu, Nov 21, 2019 at 10:59:48AM +0100, Stefano Garzarella wrote:
> > > On Thu, Nov 21, 2019 at 09:34:58AM +0000, Stefan Hajnoczi wrote:
> > > > On Tue, Nov 19, 2019 at 12:01:19PM +0100, Stefano Garzarella wrote:
> > > > > +static struct workqueue_struct *vsock_loopback_workqueue;
> > > > > +static struct vsock_loopback *the_vsock_loopback;
> > > >
> > > > the_vsock_loopback could be a static global variable (not a pointer) and
> > > > vsock_loopback_workqueue could also be included in the struct.
> > > >
> > > > The RCU pointer is really a way to synchronize vsock_loopback_send_pkt()
> > > > and vsock_loopback_cancel_pkt() with module exit. There is no other
> > > > reason for using a pointer.
> > > >
> > > > It's cleaner to implement the synchronization once in af_vsock.c (or
> > > > virtio_transport_common.c) instead of making each transport do it.
> > > > Maybe try_module_get() and related APIs provide the necessary semantics
> > > > so that core vsock code can hold the transport module while it's being
> > > > used to send/cancel a packet.
> > >
> > > Right, the module cannot be unloaded until open sockets, so here the
> > > synchronization is not needed.
> > >
> > > The synchronization come from virtio-vsock device that can be
> > > hot-unplugged while sockets are still open, but that can't happen here.
> > >
> > > I will remove the pointers and RCU in the v2.
> > >
> > > Can I keep your R-b or do you prefer to watch v2 first?
>
> I'd like to review v2.
>
Sure!
> > > > > +MODULE_ALIAS_NETPROTO(PF_VSOCK);
> > > >
> > > > Why does this module define the alias for PF_VSOCK? Doesn't another
> > > > module already define this alias?
> > >
> > > It is a way to load this module when PF_VSOCK is starting to be used.
> > > MODULE_ALIAS_NETPROTO(PF_VSOCK) is already defined in vmci_transport
> > > and hyperv_transport. IIUC it is used for the same reason.
> > >
> > > In virtio_transport we don't need it because it will be loaded when
> > > the PCI device is discovered.
> > >
> > > Do you think there's a better way?
> > > Should I include the vsock_loopback transport directly in af_vsock
> > > without creating a new module?
> > >
> >
> > That last thing I said may not be possible:
> > I remembered that I tried, but DEPMOD found a cyclic dependency because
> > vsock_transport use virtio_transport_common that use vsock, so if I
> > include vsock_transport in the vsock module, DEPMOD is not happy.
> >
> > Do you think it's okay in this case to keep MODULE_ALIAS_NETPROTO(PF_VSOCK)
> > or is there a better way?
>
> The reason I asked is because the semantics of duplicate module aliases
> aren't clear to me. Do all modules with the same alias get loaded?
> Or just the first? Or ...?
It wasn't clear to me either, but when I tried, I saw that all modules
with the same alias got loaded.
Stefano
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 4/6] vsock: add vsock_loopback transport
2019-11-21 9:59 ` Stefano Garzarella
2019-11-21 15:25 ` Stefano Garzarella
@ 2019-11-21 15:25 ` Stefano Garzarella
1 sibling, 0 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-21 15:25 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: kvm, netdev, Dexuan Cui, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller, Jorgen Hansen
On Thu, Nov 21, 2019 at 10:59:48AM +0100, Stefano Garzarella wrote:
> On Thu, Nov 21, 2019 at 09:34:58AM +0000, Stefan Hajnoczi wrote:
> > On Tue, Nov 19, 2019 at 12:01:19PM +0100, Stefano Garzarella wrote:
> >
> > Ideas for long-term changes below.
> >
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> >
>
> Thanks for reviewing!
>
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 760049454a23..c2a3dc3113ba 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -17239,6 +17239,7 @@ F: net/vmw_vsock/diag.c
> > > F: net/vmw_vsock/af_vsock_tap.c
> > > F: net/vmw_vsock/virtio_transport_common.c
> > > F: net/vmw_vsock/virtio_transport.c
> > > +F: net/vmw_vsock/vsock_loopback.c
> > > F: drivers/net/vsockmon.c
> > > F: drivers/vhost/vsock.c
> > > F: tools/testing/vsock/
> >
> > At this point you are most active in virtio-vsock and I am reviewing
> > patches on a best-effort basis. Feel free to add yourself as
> > maintainer.
> >
>
> Sure, I'd be happy to maintain it.
>
> > > diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
> > > new file mode 100644
> > > index 000000000000..3d1c1a88305f
> > > --- /dev/null
> > > +++ b/net/vmw_vsock/vsock_loopback.c
> > > @@ -0,0 +1,217 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * loopback transport for vsock using virtio_transport_common APIs
> > > + *
> > > + * Copyright (C) 2013-2019 Red Hat, Inc.
> > > + * Author: Asias He <asias@redhat.com>
> > > + * Stefan Hajnoczi <stefanha@redhat.com>
> > > + * Stefano Garzarella <sgarzare@redhat.com>
> > > + *
> > > + */
> > > +#include <linux/spinlock.h>
> > > +#include <linux/module.h>
> > > +#include <linux/list.h>
> > > +#include <linux/virtio_vsock.h>
> >
> > Is it time to rename the generic functionality in
> > virtio_transport_common.c? This doesn't have anything to do with virtio
> > :).
> >
>
> Completely agree, new transports could use it to handle the protocol without
> reimplementing things already done.
>
> > > +
> > > +static struct workqueue_struct *vsock_loopback_workqueue;
> > > +static struct vsock_loopback *the_vsock_loopback;
> >
> > the_vsock_loopback could be a static global variable (not a pointer) and
> > vsock_loopback_workqueue could also be included in the struct.
> >
> > The RCU pointer is really a way to synchronize vsock_loopback_send_pkt()
> > and vsock_loopback_cancel_pkt() with module exit. There is no other
> > reason for using a pointer.
> >
> > It's cleaner to implement the synchronization once in af_vsock.c (or
> > virtio_transport_common.c) instead of making each transport do it.
> > Maybe try_module_get() and related APIs provide the necessary semantics
> > so that core vsock code can hold the transport module while it's being
> > used to send/cancel a packet.
>
> Right, the module cannot be unloaded until open sockets, so here the
> synchronization is not needed.
>
> The synchronization come from virtio-vsock device that can be
> hot-unplugged while sockets are still open, but that can't happen here.
>
> I will remove the pointers and RCU in the v2.
>
> Can I keep your R-b or do you prefer to watch v2 first?
>
> >
> > > +MODULE_ALIAS_NETPROTO(PF_VSOCK);
> >
> > Why does this module define the alias for PF_VSOCK? Doesn't another
> > module already define this alias?
>
> It is a way to load this module when PF_VSOCK is starting to be used.
> MODULE_ALIAS_NETPROTO(PF_VSOCK) is already defined in vmci_transport
> and hyperv_transport. IIUC it is used for the same reason.
>
> In virtio_transport we don't need it because it will be loaded when
> the PCI device is discovered.
>
> Do you think there's a better way?
> Should I include the vsock_loopback transport directly in af_vsock
> without creating a new module?
>
That last thing I said may not be possible:
I remembered that I tried, but DEPMOD found a cyclic dependency because
vsock_transport use virtio_transport_common that use vsock, so if I
include vsock_transport in the vsock module, DEPMOD is not happy.
Do you think it's okay in this case to keep MODULE_ALIAS_NETPROTO(PF_VSOCK)
or is there a better way?
Thanks,
Stefano
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH net-next 5/6] vsock: use local transport when it is loaded
2019-11-19 11:01 [PATCH net-next 0/6] vsock: add local transport support Stefano Garzarella
` (3 preceding siblings ...)
2019-11-19 11:01 ` [PATCH net-next 4/6] vsock: add vsock_loopback transport Stefano Garzarella
@ 2019-11-19 11:01 ` Stefano Garzarella
2019-11-21 9:46 ` Stefan Hajnoczi
2019-11-21 9:46 ` Stefan Hajnoczi
2019-11-19 11:01 ` [PATCH net-next 6/6] vsock/virtio: remove loopback handling Stefano Garzarella
` (4 subsequent siblings)
9 siblings, 2 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-19 11:01 UTC (permalink / raw)
To: netdev
Cc: virtualization, Dexuan Cui, Stefan Hajnoczi, linux-kernel, kvm,
David S. Miller, Jorgen Hansen
Now that we have a transport that can handle the local communication,
we can use it when it is loaded.
A socket will use the local transport (loopback) when the remote
CID is:
- equal to VMADDR_CID_LOCAL
- or equal to transport_g2h->get_local_cid(), if transport_g2h
is loaded (this allows us to keep the same behavior implemented
by virtio and vmci transports)
- or equal to VMADDR_CID_HOST, if transport_g2h is not loaded
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
net/vmw_vsock/af_vsock.c | 32 ++++++++++++++++++++++++++------
1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index c9e5bad59dc1..40bbb2a17e3d 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -388,6 +388,21 @@ void vsock_enqueue_accept(struct sock *listener, struct sock *connected)
}
EXPORT_SYMBOL_GPL(vsock_enqueue_accept);
+static bool vsock_use_local_transport(unsigned int remote_cid)
+{
+ if (!transport_local)
+ return false;
+
+ if (remote_cid == VMADDR_CID_LOCAL)
+ return true;
+
+ if (transport_g2h) {
+ return remote_cid == transport_g2h->get_local_cid();
+ } else {
+ return remote_cid == VMADDR_CID_HOST;
+ }
+}
+
static void vsock_deassign_transport(struct vsock_sock *vsk)
{
if (!vsk->transport)
@@ -404,9 +419,10 @@ static void vsock_deassign_transport(struct vsock_sock *vsk)
* (e.g. during the connect() or when a connection request on a listener
* socket is received).
* The vsk->remote_addr is used to decide which transport to use:
- * - remote CID <= VMADDR_CID_HOST will use guest->host transport;
- * - remote CID == local_cid (guest->host transport) will use guest->host
- * transport for loopback (host->guest transports don't support loopback);
+ * - remote CID == VMADDR_CID_LOCAL or g2h->local_cid or VMADDR_CID_HOST if
+ * g2h is not loaded, will use local transport;
+ * - remote CID == VMADDR_CID_HOST or VMADDR_CID_HYPERVISOR, will use
+ * guest->host transport;
* - remote CID > VMADDR_CID_HOST will use host->guest transport;
*/
int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
@@ -420,9 +436,10 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
new_transport = transport_dgram;
break;
case SOCK_STREAM:
- if (remote_cid <= VMADDR_CID_HOST ||
- (transport_g2h &&
- remote_cid == transport_g2h->get_local_cid()))
+ if (vsock_use_local_transport(remote_cid))
+ new_transport = transport_local;
+ else if (remote_cid == VMADDR_CID_HOST ||
+ remote_cid == VMADDR_CID_HYPERVISOR)
new_transport = transport_g2h;
else
new_transport = transport_h2g;
@@ -459,6 +476,9 @@ bool vsock_find_cid(unsigned int cid)
if (transport_h2g && cid == VMADDR_CID_HOST)
return true;
+ if (transport_local && cid == VMADDR_CID_LOCAL)
+ return true;
+
return false;
}
EXPORT_SYMBOL_GPL(vsock_find_cid);
--
2.21.0
^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 5/6] vsock: use local transport when it is loaded
2019-11-19 11:01 ` [PATCH net-next 5/6] vsock: use local transport when it is loaded Stefano Garzarella
@ 2019-11-21 9:46 ` Stefan Hajnoczi
2019-11-21 9:46 ` Stefan Hajnoczi
1 sibling, 0 replies; 43+ messages in thread
From: Stefan Hajnoczi @ 2019-11-21 9:46 UTC (permalink / raw)
To: Stefano Garzarella
Cc: kvm, netdev, Dexuan Cui, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller, Jorgen Hansen
[-- Attachment #1.1: Type: text/plain, Size: 880 bytes --]
On Tue, Nov 19, 2019 at 12:01:20PM +0100, Stefano Garzarella wrote:
> @@ -420,9 +436,10 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
> new_transport = transport_dgram;
> break;
> case SOCK_STREAM:
> - if (remote_cid <= VMADDR_CID_HOST ||
> - (transport_g2h &&
> - remote_cid == transport_g2h->get_local_cid()))
> + if (vsock_use_local_transport(remote_cid))
> + new_transport = transport_local;
> + else if (remote_cid == VMADDR_CID_HOST ||
> + remote_cid == VMADDR_CID_HYPERVISOR)
> new_transport = transport_g2h;
> else
> new_transport = transport_h2g;
We used to send VMADDR_CID_RESERVED to the host. Now we send
VMADDR_CID_RESERVED (LOCAL) to the guest when there is no
transport_local loaded?
If this is correct, is there a justification for this change? It seems
safest to retain existing behavior.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 5/6] vsock: use local transport when it is loaded
2019-11-19 11:01 ` [PATCH net-next 5/6] vsock: use local transport when it is loaded Stefano Garzarella
2019-11-21 9:46 ` Stefan Hajnoczi
@ 2019-11-21 9:46 ` Stefan Hajnoczi
2019-11-21 10:01 ` Stefano Garzarella
2019-11-21 10:01 ` Stefano Garzarella
1 sibling, 2 replies; 43+ messages in thread
From: Stefan Hajnoczi @ 2019-11-21 9:46 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, virtualization, Dexuan Cui, Stefan Hajnoczi,
linux-kernel, kvm, David S. Miller, Jorgen Hansen
[-- Attachment #1: Type: text/plain, Size: 880 bytes --]
On Tue, Nov 19, 2019 at 12:01:20PM +0100, Stefano Garzarella wrote:
> @@ -420,9 +436,10 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
> new_transport = transport_dgram;
> break;
> case SOCK_STREAM:
> - if (remote_cid <= VMADDR_CID_HOST ||
> - (transport_g2h &&
> - remote_cid == transport_g2h->get_local_cid()))
> + if (vsock_use_local_transport(remote_cid))
> + new_transport = transport_local;
> + else if (remote_cid == VMADDR_CID_HOST ||
> + remote_cid == VMADDR_CID_HYPERVISOR)
> new_transport = transport_g2h;
> else
> new_transport = transport_h2g;
We used to send VMADDR_CID_RESERVED to the host. Now we send
VMADDR_CID_RESERVED (LOCAL) to the guest when there is no
transport_local loaded?
If this is correct, is there a justification for this change? It seems
safest to retain existing behavior.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 5/6] vsock: use local transport when it is loaded
2019-11-21 9:46 ` Stefan Hajnoczi
@ 2019-11-21 10:01 ` Stefano Garzarella
2019-11-21 10:01 ` Stefano Garzarella
1 sibling, 0 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-21 10:01 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: kvm, netdev, Dexuan Cui, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller, Jorgen Hansen
On Thu, Nov 21, 2019 at 09:46:14AM +0000, Stefan Hajnoczi wrote:
> On Tue, Nov 19, 2019 at 12:01:20PM +0100, Stefano Garzarella wrote:
> > @@ -420,9 +436,10 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
> > new_transport = transport_dgram;
> > break;
> > case SOCK_STREAM:
> > - if (remote_cid <= VMADDR_CID_HOST ||
> > - (transport_g2h &&
> > - remote_cid == transport_g2h->get_local_cid()))
> > + if (vsock_use_local_transport(remote_cid))
> > + new_transport = transport_local;
> > + else if (remote_cid == VMADDR_CID_HOST ||
> > + remote_cid == VMADDR_CID_HYPERVISOR)
> > new_transport = transport_g2h;
> > else
> > new_transport = transport_h2g;
>
> We used to send VMADDR_CID_RESERVED to the host. Now we send
> VMADDR_CID_RESERVED (LOCAL) to the guest when there is no
> transport_local loaded?
>
> If this is correct, is there a justification for this change? It seems
> safest to retain existing behavior.
You're right, I'll revert this change in v2.
Thanks,
Stefano
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 5/6] vsock: use local transport when it is loaded
2019-11-21 9:46 ` Stefan Hajnoczi
2019-11-21 10:01 ` Stefano Garzarella
@ 2019-11-21 10:01 ` Stefano Garzarella
1 sibling, 0 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-21 10:01 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: netdev, virtualization, Dexuan Cui, Stefan Hajnoczi,
linux-kernel, kvm, David S. Miller, Jorgen Hansen
On Thu, Nov 21, 2019 at 09:46:14AM +0000, Stefan Hajnoczi wrote:
> On Tue, Nov 19, 2019 at 12:01:20PM +0100, Stefano Garzarella wrote:
> > @@ -420,9 +436,10 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
> > new_transport = transport_dgram;
> > break;
> > case SOCK_STREAM:
> > - if (remote_cid <= VMADDR_CID_HOST ||
> > - (transport_g2h &&
> > - remote_cid == transport_g2h->get_local_cid()))
> > + if (vsock_use_local_transport(remote_cid))
> > + new_transport = transport_local;
> > + else if (remote_cid == VMADDR_CID_HOST ||
> > + remote_cid == VMADDR_CID_HYPERVISOR)
> > new_transport = transport_g2h;
> > else
> > new_transport = transport_h2g;
>
> We used to send VMADDR_CID_RESERVED to the host. Now we send
> VMADDR_CID_RESERVED (LOCAL) to the guest when there is no
> transport_local loaded?
>
> If this is correct, is there a justification for this change? It seems
> safest to retain existing behavior.
You're right, I'll revert this change in v2.
Thanks,
Stefano
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH net-next 6/6] vsock/virtio: remove loopback handling
2019-11-19 11:01 [PATCH net-next 0/6] vsock: add local transport support Stefano Garzarella
` (4 preceding siblings ...)
2019-11-19 11:01 ` [PATCH net-next 5/6] vsock: use local transport when it is loaded Stefano Garzarella
@ 2019-11-19 11:01 ` Stefano Garzarella
2019-11-21 9:46 ` [PATCH net-next 0/6] vsock: add local transport support Stefan Hajnoczi
` (3 subsequent siblings)
9 siblings, 0 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-19 11:01 UTC (permalink / raw)
To: netdev
Cc: virtualization, Dexuan Cui, Stefan Hajnoczi, linux-kernel, kvm,
David S. Miller, Jorgen Hansen
We can remove the loopback handling from virtio_transport,
because now the vsock core is able to handle local communication
using the new vsock_loopback device.
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
net/vmw_vsock/virtio_transport.c | 61 ++------------------------------
1 file changed, 2 insertions(+), 59 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 1458c5c8b64d..dfbaf6bd8b1c 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -44,10 +44,6 @@ struct virtio_vsock {
spinlock_t send_pkt_list_lock;
struct list_head send_pkt_list;
- struct work_struct loopback_work;
- spinlock_t loopback_list_lock; /* protects loopback_list */
- struct list_head loopback_list;
-
atomic_t queued_replies;
/* The following fields are protected by rx_lock. vqs[VSOCK_VQ_RX]
@@ -86,20 +82,6 @@ static u32 virtio_transport_get_local_cid(void)
return ret;
}
-static int virtio_transport_send_pkt_loopback(struct virtio_vsock *vsock,
- struct virtio_vsock_pkt *pkt)
-{
- int len = pkt->len;
-
- spin_lock_bh(&vsock->loopback_list_lock);
- list_add_tail(&pkt->list, &vsock->loopback_list);
- spin_unlock_bh(&vsock->loopback_list_lock);
-
- queue_work(virtio_vsock_workqueue, &vsock->loopback_work);
-
- return len;
-}
-
static void
virtio_transport_send_pkt_work(struct work_struct *work)
{
@@ -194,7 +176,8 @@ virtio_transport_send_pkt(struct virtio_vsock_pkt *pkt)
}
if (le64_to_cpu(pkt->hdr.dst_cid) == vsock->guest_cid) {
- len = virtio_transport_send_pkt_loopback(vsock, pkt);
+ virtio_transport_free_pkt(pkt);
+ len = -ENODEV;
goto out_rcu;
}
@@ -502,33 +485,6 @@ static struct virtio_transport virtio_transport = {
.send_pkt = virtio_transport_send_pkt,
};
-static void virtio_transport_loopback_work(struct work_struct *work)
-{
- struct virtio_vsock *vsock =
- container_of(work, struct virtio_vsock, loopback_work);
- LIST_HEAD(pkts);
-
- spin_lock_bh(&vsock->loopback_list_lock);
- list_splice_init(&vsock->loopback_list, &pkts);
- spin_unlock_bh(&vsock->loopback_list_lock);
-
- mutex_lock(&vsock->rx_lock);
-
- if (!vsock->rx_run)
- goto out;
-
- while (!list_empty(&pkts)) {
- struct virtio_vsock_pkt *pkt;
-
- pkt = list_first_entry(&pkts, struct virtio_vsock_pkt, list);
- list_del_init(&pkt->list);
-
- virtio_transport_recv_pkt(&virtio_transport, pkt);
- }
-out:
- mutex_unlock(&vsock->rx_lock);
-}
-
static void virtio_transport_rx_work(struct work_struct *work)
{
struct virtio_vsock *vsock =
@@ -633,13 +589,10 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
mutex_init(&vsock->event_lock);
spin_lock_init(&vsock->send_pkt_list_lock);
INIT_LIST_HEAD(&vsock->send_pkt_list);
- spin_lock_init(&vsock->loopback_list_lock);
- INIT_LIST_HEAD(&vsock->loopback_list);
INIT_WORK(&vsock->rx_work, virtio_transport_rx_work);
INIT_WORK(&vsock->tx_work, virtio_transport_tx_work);
INIT_WORK(&vsock->event_work, virtio_transport_event_work);
INIT_WORK(&vsock->send_pkt_work, virtio_transport_send_pkt_work);
- INIT_WORK(&vsock->loopback_work, virtio_transport_loopback_work);
mutex_lock(&vsock->tx_lock);
vsock->tx_run = true;
@@ -720,22 +673,12 @@ static void virtio_vsock_remove(struct virtio_device *vdev)
}
spin_unlock_bh(&vsock->send_pkt_list_lock);
- spin_lock_bh(&vsock->loopback_list_lock);
- while (!list_empty(&vsock->loopback_list)) {
- pkt = list_first_entry(&vsock->loopback_list,
- struct virtio_vsock_pkt, list);
- list_del(&pkt->list);
- virtio_transport_free_pkt(pkt);
- }
- spin_unlock_bh(&vsock->loopback_list_lock);
-
/* Delete virtqueues and flush outstanding callbacks if any */
vdev->config->del_vqs(vdev);
/* Other works can be queued before 'config->del_vqs()', so we flush
* all works before to free the vsock object to avoid use after free.
*/
- flush_work(&vsock->loopback_work);
flush_work(&vsock->rx_work);
flush_work(&vsock->tx_work);
flush_work(&vsock->event_work);
--
2.21.0
^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 0/6] vsock: add local transport support
2019-11-19 11:01 [PATCH net-next 0/6] vsock: add local transport support Stefano Garzarella
` (5 preceding siblings ...)
2019-11-19 11:01 ` [PATCH net-next 6/6] vsock/virtio: remove loopback handling Stefano Garzarella
@ 2019-11-21 9:46 ` Stefan Hajnoczi
2019-11-21 10:05 ` Stefano Garzarella
2019-11-21 10:05 ` Stefano Garzarella
2019-11-21 9:46 ` Stefan Hajnoczi
` (2 subsequent siblings)
9 siblings, 2 replies; 43+ messages in thread
From: Stefan Hajnoczi @ 2019-11-21 9:46 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, virtualization, Dexuan Cui, Stefan Hajnoczi,
linux-kernel, kvm, David S. Miller, Jorgen Hansen
[-- Attachment #1: Type: text/plain, Size: 2680 bytes --]
On Tue, Nov 19, 2019 at 12:01:15PM +0100, Stefano Garzarella wrote:
> This series introduces a new transport (vsock_loopback) to handle
> local communication.
> This could be useful to test vsock core itself and to allow developers
> to test their applications without launching a VM.
>
> Before this series, vmci and virtio transports allowed this behavior,
> but only in the guest.
> We are moving the loopback handling in a new transport, because it
> might be useful to provide this feature also in the host or when
> no H2G/G2H transports (hyperv, virtio, vmci) are loaded.
>
> The user can use the loopback with the new VMADDR_CID_LOCAL (that
> replaces VMADDR_CID_RESERVED) in any condition.
> Otherwise, if the G2H transport is loaded, it can also use the guest
> local CID as previously supported by vmci and virtio transports.
> If G2H transport is not loaded, the user can also use VMADDR_CID_HOST
> for local communication.
>
> Patch 1 is a cleanup to build virtio_transport_common without virtio
> Patch 2 adds the new VMADDR_CID_LOCAL, replacing VMADDR_CID_RESERVED
> Patch 3 adds a new feature flag to register a loopback transport
> Patch 4 adds the new vsock_loopback transport based on the loopback
> implementation of virtio_transport
> Patch 5 implements the logic to use the local transport for loopback
> communication
> Patch 6 removes the loopback from virtio_transport
>
> @Jorgen: Do you think it might be a problem to replace
> VMADDR_CID_RESERVED with VMADDR_CID_LOCAL?
>
> Thanks,
> Stefano
>
> Stefano Garzarella (6):
> vsock/virtio_transport_common: remove unused virtio header includes
> vsock: add VMADDR_CID_LOCAL definition
> vsock: add local transport support in the vsock core
> vsock: add vsock_loopback transport
> vsock: use local transport when it is loaded
> vsock/virtio: remove loopback handling
>
> MAINTAINERS | 1 +
> include/net/af_vsock.h | 2 +
> include/uapi/linux/vm_sockets.h | 8 +-
> net/vmw_vsock/Kconfig | 12 ++
> net/vmw_vsock/Makefile | 1 +
> net/vmw_vsock/af_vsock.c | 49 +++++-
> net/vmw_vsock/virtio_transport.c | 61 +------
> net/vmw_vsock/virtio_transport_common.c | 3 -
> net/vmw_vsock/vmci_transport.c | 2 +-
> net/vmw_vsock/vsock_loopback.c | 217 ++++++++++++++++++++++++
> 10 files changed, 283 insertions(+), 73 deletions(-)
> create mode 100644 net/vmw_vsock/vsock_loopback.c
Please see my comments. Otherwise:
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 0/6] vsock: add local transport support
2019-11-21 9:46 ` [PATCH net-next 0/6] vsock: add local transport support Stefan Hajnoczi
@ 2019-11-21 10:05 ` Stefano Garzarella
2019-11-21 10:05 ` Stefano Garzarella
1 sibling, 0 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-21 10:05 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: netdev, virtualization, Dexuan Cui, Stefan Hajnoczi,
linux-kernel, kvm, David S. Miller, Jorgen Hansen
On Thu, Nov 21, 2019 at 09:46:43AM +0000, Stefan Hajnoczi wrote:
> On Tue, Nov 19, 2019 at 12:01:15PM +0100, Stefano Garzarella wrote:
> > This series introduces a new transport (vsock_loopback) to handle
> > local communication.
> > This could be useful to test vsock core itself and to allow developers
> > to test their applications without launching a VM.
> >
> > Before this series, vmci and virtio transports allowed this behavior,
> > but only in the guest.
> > We are moving the loopback handling in a new transport, because it
> > might be useful to provide this feature also in the host or when
> > no H2G/G2H transports (hyperv, virtio, vmci) are loaded.
> >
> > The user can use the loopback with the new VMADDR_CID_LOCAL (that
> > replaces VMADDR_CID_RESERVED) in any condition.
> > Otherwise, if the G2H transport is loaded, it can also use the guest
> > local CID as previously supported by vmci and virtio transports.
> > If G2H transport is not loaded, the user can also use VMADDR_CID_HOST
> > for local communication.
> >
> > Patch 1 is a cleanup to build virtio_transport_common without virtio
> > Patch 2 adds the new VMADDR_CID_LOCAL, replacing VMADDR_CID_RESERVED
> > Patch 3 adds a new feature flag to register a loopback transport
> > Patch 4 adds the new vsock_loopback transport based on the loopback
> > implementation of virtio_transport
> > Patch 5 implements the logic to use the local transport for loopback
> > communication
> > Patch 6 removes the loopback from virtio_transport
> >
> > @Jorgen: Do you think it might be a problem to replace
> > VMADDR_CID_RESERVED with VMADDR_CID_LOCAL?
> >
> > Thanks,
> > Stefano
> >
> > Stefano Garzarella (6):
> > vsock/virtio_transport_common: remove unused virtio header includes
> > vsock: add VMADDR_CID_LOCAL definition
> > vsock: add local transport support in the vsock core
> > vsock: add vsock_loopback transport
> > vsock: use local transport when it is loaded
> > vsock/virtio: remove loopback handling
> >
> > MAINTAINERS | 1 +
> > include/net/af_vsock.h | 2 +
> > include/uapi/linux/vm_sockets.h | 8 +-
> > net/vmw_vsock/Kconfig | 12 ++
> > net/vmw_vsock/Makefile | 1 +
> > net/vmw_vsock/af_vsock.c | 49 +++++-
> > net/vmw_vsock/virtio_transport.c | 61 +------
> > net/vmw_vsock/virtio_transport_common.c | 3 -
> > net/vmw_vsock/vmci_transport.c | 2 +-
> > net/vmw_vsock/vsock_loopback.c | 217 ++++++++++++++++++++++++
> > 10 files changed, 283 insertions(+), 73 deletions(-)
> > create mode 100644 net/vmw_vsock/vsock_loopback.c
>
> Please see my comments. Otherwise:
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Thanks!
I'll send a v2 following your comments.
Stefano
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 0/6] vsock: add local transport support
2019-11-21 9:46 ` [PATCH net-next 0/6] vsock: add local transport support Stefan Hajnoczi
2019-11-21 10:05 ` Stefano Garzarella
@ 2019-11-21 10:05 ` Stefano Garzarella
1 sibling, 0 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-21 10:05 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: kvm, netdev, Dexuan Cui, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller, Jorgen Hansen
On Thu, Nov 21, 2019 at 09:46:43AM +0000, Stefan Hajnoczi wrote:
> On Tue, Nov 19, 2019 at 12:01:15PM +0100, Stefano Garzarella wrote:
> > This series introduces a new transport (vsock_loopback) to handle
> > local communication.
> > This could be useful to test vsock core itself and to allow developers
> > to test their applications without launching a VM.
> >
> > Before this series, vmci and virtio transports allowed this behavior,
> > but only in the guest.
> > We are moving the loopback handling in a new transport, because it
> > might be useful to provide this feature also in the host or when
> > no H2G/G2H transports (hyperv, virtio, vmci) are loaded.
> >
> > The user can use the loopback with the new VMADDR_CID_LOCAL (that
> > replaces VMADDR_CID_RESERVED) in any condition.
> > Otherwise, if the G2H transport is loaded, it can also use the guest
> > local CID as previously supported by vmci and virtio transports.
> > If G2H transport is not loaded, the user can also use VMADDR_CID_HOST
> > for local communication.
> >
> > Patch 1 is a cleanup to build virtio_transport_common without virtio
> > Patch 2 adds the new VMADDR_CID_LOCAL, replacing VMADDR_CID_RESERVED
> > Patch 3 adds a new feature flag to register a loopback transport
> > Patch 4 adds the new vsock_loopback transport based on the loopback
> > implementation of virtio_transport
> > Patch 5 implements the logic to use the local transport for loopback
> > communication
> > Patch 6 removes the loopback from virtio_transport
> >
> > @Jorgen: Do you think it might be a problem to replace
> > VMADDR_CID_RESERVED with VMADDR_CID_LOCAL?
> >
> > Thanks,
> > Stefano
> >
> > Stefano Garzarella (6):
> > vsock/virtio_transport_common: remove unused virtio header includes
> > vsock: add VMADDR_CID_LOCAL definition
> > vsock: add local transport support in the vsock core
> > vsock: add vsock_loopback transport
> > vsock: use local transport when it is loaded
> > vsock/virtio: remove loopback handling
> >
> > MAINTAINERS | 1 +
> > include/net/af_vsock.h | 2 +
> > include/uapi/linux/vm_sockets.h | 8 +-
> > net/vmw_vsock/Kconfig | 12 ++
> > net/vmw_vsock/Makefile | 1 +
> > net/vmw_vsock/af_vsock.c | 49 +++++-
> > net/vmw_vsock/virtio_transport.c | 61 +------
> > net/vmw_vsock/virtio_transport_common.c | 3 -
> > net/vmw_vsock/vmci_transport.c | 2 +-
> > net/vmw_vsock/vsock_loopback.c | 217 ++++++++++++++++++++++++
> > 10 files changed, 283 insertions(+), 73 deletions(-)
> > create mode 100644 net/vmw_vsock/vsock_loopback.c
>
> Please see my comments. Otherwise:
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Thanks!
I'll send a v2 following your comments.
Stefano
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 0/6] vsock: add local transport support
2019-11-19 11:01 [PATCH net-next 0/6] vsock: add local transport support Stefano Garzarella
` (6 preceding siblings ...)
2019-11-21 9:46 ` [PATCH net-next 0/6] vsock: add local transport support Stefan Hajnoczi
@ 2019-11-21 9:46 ` Stefan Hajnoczi
2019-11-21 14:45 ` Jorgen Hansen via Virtualization
2019-11-21 14:45 ` Jorgen Hansen
9 siblings, 0 replies; 43+ messages in thread
From: Stefan Hajnoczi @ 2019-11-21 9:46 UTC (permalink / raw)
To: Stefano Garzarella
Cc: kvm, netdev, Dexuan Cui, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller, Jorgen Hansen
[-- Attachment #1.1: Type: text/plain, Size: 2680 bytes --]
On Tue, Nov 19, 2019 at 12:01:15PM +0100, Stefano Garzarella wrote:
> This series introduces a new transport (vsock_loopback) to handle
> local communication.
> This could be useful to test vsock core itself and to allow developers
> to test their applications without launching a VM.
>
> Before this series, vmci and virtio transports allowed this behavior,
> but only in the guest.
> We are moving the loopback handling in a new transport, because it
> might be useful to provide this feature also in the host or when
> no H2G/G2H transports (hyperv, virtio, vmci) are loaded.
>
> The user can use the loopback with the new VMADDR_CID_LOCAL (that
> replaces VMADDR_CID_RESERVED) in any condition.
> Otherwise, if the G2H transport is loaded, it can also use the guest
> local CID as previously supported by vmci and virtio transports.
> If G2H transport is not loaded, the user can also use VMADDR_CID_HOST
> for local communication.
>
> Patch 1 is a cleanup to build virtio_transport_common without virtio
> Patch 2 adds the new VMADDR_CID_LOCAL, replacing VMADDR_CID_RESERVED
> Patch 3 adds a new feature flag to register a loopback transport
> Patch 4 adds the new vsock_loopback transport based on the loopback
> implementation of virtio_transport
> Patch 5 implements the logic to use the local transport for loopback
> communication
> Patch 6 removes the loopback from virtio_transport
>
> @Jorgen: Do you think it might be a problem to replace
> VMADDR_CID_RESERVED with VMADDR_CID_LOCAL?
>
> Thanks,
> Stefano
>
> Stefano Garzarella (6):
> vsock/virtio_transport_common: remove unused virtio header includes
> vsock: add VMADDR_CID_LOCAL definition
> vsock: add local transport support in the vsock core
> vsock: add vsock_loopback transport
> vsock: use local transport when it is loaded
> vsock/virtio: remove loopback handling
>
> MAINTAINERS | 1 +
> include/net/af_vsock.h | 2 +
> include/uapi/linux/vm_sockets.h | 8 +-
> net/vmw_vsock/Kconfig | 12 ++
> net/vmw_vsock/Makefile | 1 +
> net/vmw_vsock/af_vsock.c | 49 +++++-
> net/vmw_vsock/virtio_transport.c | 61 +------
> net/vmw_vsock/virtio_transport_common.c | 3 -
> net/vmw_vsock/vmci_transport.c | 2 +-
> net/vmw_vsock/vsock_loopback.c | 217 ++++++++++++++++++++++++
> 10 files changed, 283 insertions(+), 73 deletions(-)
> create mode 100644 net/vmw_vsock/vsock_loopback.c
Please see my comments. Otherwise:
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH net-next 0/6] vsock: add local transport support
2019-11-19 11:01 [PATCH net-next 0/6] vsock: add local transport support Stefano Garzarella
` (7 preceding siblings ...)
2019-11-21 9:46 ` Stefan Hajnoczi
@ 2019-11-21 14:45 ` Jorgen Hansen via Virtualization
2019-11-21 14:45 ` Jorgen Hansen
9 siblings, 0 replies; 43+ messages in thread
From: Jorgen Hansen via Virtualization @ 2019-11-21 14:45 UTC (permalink / raw)
To: 'Stefano Garzarella', netdev
Cc: kvm, Dexuan Cui, linux-kernel, virtualization, Stefan Hajnoczi,
David S. Miller
> From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> Sent: Tuesday, November 19, 2019 12:01 PM
> This series introduces a new transport (vsock_loopback) to handle
> local communication.
> This could be useful to test vsock core itself and to allow developers
> to test their applications without launching a VM.
>
> Before this series, vmci and virtio transports allowed this behavior,
> but only in the guest.
> We are moving the loopback handling in a new transport, because it
> might be useful to provide this feature also in the host or when
> no H2G/G2H transports (hyperv, virtio, vmci) are loaded.
>
> The user can use the loopback with the new VMADDR_CID_LOCAL (that
> replaces VMADDR_CID_RESERVED) in any condition.
> Otherwise, if the G2H transport is loaded, it can also use the guest
> local CID as previously supported by vmci and virtio transports.
> If G2H transport is not loaded, the user can also use VMADDR_CID_HOST
> for local communication.
>
> Patch 1 is a cleanup to build virtio_transport_common without virtio
> Patch 2 adds the new VMADDR_CID_LOCAL, replacing
> VMADDR_CID_RESERVED
> Patch 3 adds a new feature flag to register a loopback transport
> Patch 4 adds the new vsock_loopback transport based on the loopback
> implementation of virtio_transport
> Patch 5 implements the logic to use the local transport for loopback
> communication
> Patch 6 removes the loopback from virtio_transport
>
> @Jorgen: Do you think it might be a problem to replace
> VMADDR_CID_RESERVED with VMADDR_CID_LOCAL?
No, that should be fine. It has never allowed for use with stream sockets in
AF_VSOCK. The only potential use would be for datagram sockets, but that
side appears to be unaffected by your changes, since loopback is only
introduced for SOCK_STREAM.
>
> Thanks,
> Stefano
>
> Stefano Garzarella (6):
> vsock/virtio_transport_common: remove unused virtio header includes
> vsock: add VMADDR_CID_LOCAL definition
> vsock: add local transport support in the vsock core
> vsock: add vsock_loopback transport
> vsock: use local transport when it is loaded
> vsock/virtio: remove loopback handling
>
> MAINTAINERS | 1 +
> include/net/af_vsock.h | 2 +
> include/uapi/linux/vm_sockets.h | 8 +-
> net/vmw_vsock/Kconfig | 12 ++
> net/vmw_vsock/Makefile | 1 +
> net/vmw_vsock/af_vsock.c | 49 +++++-
> net/vmw_vsock/virtio_transport.c | 61 +------
> net/vmw_vsock/virtio_transport_common.c | 3 -
> net/vmw_vsock/vmci_transport.c | 2 +-
> net/vmw_vsock/vsock_loopback.c | 217
> ++++++++++++++++++++++++
> 10 files changed, 283 insertions(+), 73 deletions(-)
> create mode 100644 net/vmw_vsock/vsock_loopback.c
>
> --
> 2.21.0
^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH net-next 0/6] vsock: add local transport support
2019-11-19 11:01 [PATCH net-next 0/6] vsock: add local transport support Stefano Garzarella
` (8 preceding siblings ...)
2019-11-21 14:45 ` Jorgen Hansen via Virtualization
@ 2019-11-21 14:45 ` Jorgen Hansen
2019-11-21 15:13 ` Stefano Garzarella
2019-11-21 15:13 ` Stefano Garzarella
9 siblings, 2 replies; 43+ messages in thread
From: Jorgen Hansen @ 2019-11-21 14:45 UTC (permalink / raw)
To: 'Stefano Garzarella', netdev
Cc: virtualization, Dexuan Cui, Stefan Hajnoczi, linux-kernel, kvm,
David S. Miller
> From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> Sent: Tuesday, November 19, 2019 12:01 PM
> This series introduces a new transport (vsock_loopback) to handle
> local communication.
> This could be useful to test vsock core itself and to allow developers
> to test their applications without launching a VM.
>
> Before this series, vmci and virtio transports allowed this behavior,
> but only in the guest.
> We are moving the loopback handling in a new transport, because it
> might be useful to provide this feature also in the host or when
> no H2G/G2H transports (hyperv, virtio, vmci) are loaded.
>
> The user can use the loopback with the new VMADDR_CID_LOCAL (that
> replaces VMADDR_CID_RESERVED) in any condition.
> Otherwise, if the G2H transport is loaded, it can also use the guest
> local CID as previously supported by vmci and virtio transports.
> If G2H transport is not loaded, the user can also use VMADDR_CID_HOST
> for local communication.
>
> Patch 1 is a cleanup to build virtio_transport_common without virtio
> Patch 2 adds the new VMADDR_CID_LOCAL, replacing
> VMADDR_CID_RESERVED
> Patch 3 adds a new feature flag to register a loopback transport
> Patch 4 adds the new vsock_loopback transport based on the loopback
> implementation of virtio_transport
> Patch 5 implements the logic to use the local transport for loopback
> communication
> Patch 6 removes the loopback from virtio_transport
>
> @Jorgen: Do you think it might be a problem to replace
> VMADDR_CID_RESERVED with VMADDR_CID_LOCAL?
No, that should be fine. It has never allowed for use with stream sockets in
AF_VSOCK. The only potential use would be for datagram sockets, but that
side appears to be unaffected by your changes, since loopback is only
introduced for SOCK_STREAM.
>
> Thanks,
> Stefano
>
> Stefano Garzarella (6):
> vsock/virtio_transport_common: remove unused virtio header includes
> vsock: add VMADDR_CID_LOCAL definition
> vsock: add local transport support in the vsock core
> vsock: add vsock_loopback transport
> vsock: use local transport when it is loaded
> vsock/virtio: remove loopback handling
>
> MAINTAINERS | 1 +
> include/net/af_vsock.h | 2 +
> include/uapi/linux/vm_sockets.h | 8 +-
> net/vmw_vsock/Kconfig | 12 ++
> net/vmw_vsock/Makefile | 1 +
> net/vmw_vsock/af_vsock.c | 49 +++++-
> net/vmw_vsock/virtio_transport.c | 61 +------
> net/vmw_vsock/virtio_transport_common.c | 3 -
> net/vmw_vsock/vmci_transport.c | 2 +-
> net/vmw_vsock/vsock_loopback.c | 217
> ++++++++++++++++++++++++
> 10 files changed, 283 insertions(+), 73 deletions(-)
> create mode 100644 net/vmw_vsock/vsock_loopback.c
>
> --
> 2.21.0
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 0/6] vsock: add local transport support
2019-11-21 14:45 ` Jorgen Hansen
@ 2019-11-21 15:13 ` Stefano Garzarella
2019-11-21 15:13 ` Stefano Garzarella
1 sibling, 0 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-21 15:13 UTC (permalink / raw)
To: Jorgen Hansen
Cc: netdev, virtualization, Dexuan Cui, Stefan Hajnoczi,
linux-kernel, kvm, David S. Miller
On Thu, Nov 21, 2019 at 02:45:32PM +0000, Jorgen Hansen wrote:
> > From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> > Sent: Tuesday, November 19, 2019 12:01 PM
> > This series introduces a new transport (vsock_loopback) to handle
> > local communication.
> > This could be useful to test vsock core itself and to allow developers
> > to test their applications without launching a VM.
> >
> > Before this series, vmci and virtio transports allowed this behavior,
> > but only in the guest.
> > We are moving the loopback handling in a new transport, because it
> > might be useful to provide this feature also in the host or when
> > no H2G/G2H transports (hyperv, virtio, vmci) are loaded.
> >
> > The user can use the loopback with the new VMADDR_CID_LOCAL (that
> > replaces VMADDR_CID_RESERVED) in any condition.
> > Otherwise, if the G2H transport is loaded, it can also use the guest
> > local CID as previously supported by vmci and virtio transports.
> > If G2H transport is not loaded, the user can also use VMADDR_CID_HOST
> > for local communication.
> >
> > Patch 1 is a cleanup to build virtio_transport_common without virtio
> > Patch 2 adds the new VMADDR_CID_LOCAL, replacing
> > VMADDR_CID_RESERVED
> > Patch 3 adds a new feature flag to register a loopback transport
> > Patch 4 adds the new vsock_loopback transport based on the loopback
> > implementation of virtio_transport
> > Patch 5 implements the logic to use the local transport for loopback
> > communication
> > Patch 6 removes the loopback from virtio_transport
> >
> > @Jorgen: Do you think it might be a problem to replace
> > VMADDR_CID_RESERVED with VMADDR_CID_LOCAL?
>
> No, that should be fine. It has never allowed for use with stream sockets in
> AF_VSOCK. The only potential use would be for datagram sockets, but that
> side appears to be unaffected by your changes, since loopback is only
> introduced for SOCK_STREAM.
>
Yes, datagram sockets are not affected.
Thanks for the clarification,
Stefano
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH net-next 0/6] vsock: add local transport support
2019-11-21 14:45 ` Jorgen Hansen
2019-11-21 15:13 ` Stefano Garzarella
@ 2019-11-21 15:13 ` Stefano Garzarella
1 sibling, 0 replies; 43+ messages in thread
From: Stefano Garzarella @ 2019-11-21 15:13 UTC (permalink / raw)
To: Jorgen Hansen
Cc: kvm, netdev, Dexuan Cui, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller
On Thu, Nov 21, 2019 at 02:45:32PM +0000, Jorgen Hansen wrote:
> > From: Stefano Garzarella [mailto:sgarzare@redhat.com]
> > Sent: Tuesday, November 19, 2019 12:01 PM
> > This series introduces a new transport (vsock_loopback) to handle
> > local communication.
> > This could be useful to test vsock core itself and to allow developers
> > to test their applications without launching a VM.
> >
> > Before this series, vmci and virtio transports allowed this behavior,
> > but only in the guest.
> > We are moving the loopback handling in a new transport, because it
> > might be useful to provide this feature also in the host or when
> > no H2G/G2H transports (hyperv, virtio, vmci) are loaded.
> >
> > The user can use the loopback with the new VMADDR_CID_LOCAL (that
> > replaces VMADDR_CID_RESERVED) in any condition.
> > Otherwise, if the G2H transport is loaded, it can also use the guest
> > local CID as previously supported by vmci and virtio transports.
> > If G2H transport is not loaded, the user can also use VMADDR_CID_HOST
> > for local communication.
> >
> > Patch 1 is a cleanup to build virtio_transport_common without virtio
> > Patch 2 adds the new VMADDR_CID_LOCAL, replacing
> > VMADDR_CID_RESERVED
> > Patch 3 adds a new feature flag to register a loopback transport
> > Patch 4 adds the new vsock_loopback transport based on the loopback
> > implementation of virtio_transport
> > Patch 5 implements the logic to use the local transport for loopback
> > communication
> > Patch 6 removes the loopback from virtio_transport
> >
> > @Jorgen: Do you think it might be a problem to replace
> > VMADDR_CID_RESERVED with VMADDR_CID_LOCAL?
>
> No, that should be fine. It has never allowed for use with stream sockets in
> AF_VSOCK. The only potential use would be for datagram sockets, but that
> side appears to be unaffected by your changes, since loopback is only
> introduced for SOCK_STREAM.
>
Yes, datagram sockets are not affected.
Thanks for the clarification,
Stefano
^ permalink raw reply [flat|nested] 43+ messages in thread