[PATCH RFC net-next v4 09/12] vsock/loopback: add netns support

Bobby Eshleman posted 12 patches 2 months ago
There is a newer version of this series
[PATCH RFC net-next v4 09/12] vsock/loopback: add netns support
Posted by Bobby Eshleman 2 months ago
From: Bobby Eshleman <bobbyeshleman@meta.com>

Add NS support to vsock loopback. Sockets in a global mode netns
communicate with each other, regardless of namespace. Sockets in a local
mode netns may only communicate with other sockets within the same
namespace.

Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
 include/net/af_vsock.h         |  4 +++
 include/net/netns/vsock.h      |  3 +++
 net/vmw_vsock/af_vsock.c       |  8 +++++-
 net/vmw_vsock/vsock_loopback.c | 59 +++++++++++++++++++++++++++++++++++-------
 4 files changed, 63 insertions(+), 11 deletions(-)

diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index 0c0c351394de..aefff6e102e7 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -305,4 +305,8 @@ static inline bool vsock_net_check_mode(struct net *n1, struct net *n2)
 	       (vsock_net_mode(n1) == VSOCK_NET_MODE_GLOBAL &&
 		vsock_net_mode(n2) == VSOCK_NET_MODE_GLOBAL);
 }
+
+int vsock_loopback_init_net(struct net *net);
+void vsock_loopback_exit_net(struct net *net);
+
 #endif /* __AF_VSOCK_H__ */
diff --git a/include/net/netns/vsock.h b/include/net/netns/vsock.h
index 0bad4652815c..4420346e10a8 100644
--- a/include/net/netns/vsock.h
+++ b/include/net/netns/vsock.h
@@ -7,6 +7,8 @@
 #define VSOCK_NET_MODE_GLOBAL	1
 #define VSOCK_NET_MODE_LOCAL	(1 << 1)
 
+struct vsock_loopback;
+
 struct netns_vsock {
 	struct ctl_table_header *vsock_hdr;
 	spinlock_t lock;
@@ -14,5 +16,6 @@ struct netns_vsock {
 	/* protected by lock */
 	u8 ns_mode;
 	bool written;
+	struct vsock_loopback *loopback;
 };
 #endif /* __NET_NET_NAMESPACE_VSOCK_H */
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index c69c2db03162..5689ce7d5843 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -2778,9 +2778,12 @@ static __net_init int vsock_sysctl_init_net(struct net *net)
 {
 	vsock_net_init(net);
 
-	if (vsock_sysctl_register(net))
+	if (vsock_loopback_init_net(net))
 		return -ENOMEM;
 
+	if (vsock_sysctl_register(net))
+		goto err_loopback;
+
 #ifdef CONFIG_PROC_FS
 	if (!proc_create_net_single_write("vsock_ns_mode", 0644, net->proc_net,
 					  vsock_proc_ns_mode_show,
@@ -2793,12 +2796,15 @@ static __net_init int vsock_sysctl_init_net(struct net *net)
 
 err_sysctl:
 	vsock_sysctl_unregister(net);
+err_loopback:
+	vsock_loopback_exit_net(net);
 	return -ENOMEM;
 }
 
 static __net_exit void vsock_sysctl_exit_net(struct net *net)
 {
 	vsock_sysctl_unregister(net);
+	vsock_loopback_exit_net(net);
 }
 
 static struct pernet_operations vsock_sysctl_ops __net_initdata = {
diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
index 6e78927a598e..4fc07e3a1d2b 100644
--- a/net/vmw_vsock/vsock_loopback.c
+++ b/net/vmw_vsock/vsock_loopback.c
@@ -28,8 +28,19 @@ static u32 vsock_loopback_get_local_cid(void)
 
 static int vsock_loopback_send_pkt(struct sk_buff *skb)
 {
-	struct vsock_loopback *vsock = &the_vsock_loopback;
+	struct vsock_loopback *vsock;
 	int len = skb->len;
+	struct net *net;
+
+	if (skb->sk)
+		net = sock_net(skb->sk);
+	else
+		net = NULL;
+
+	if (net && net->vsock.ns_mode == VSOCK_NET_MODE_LOCAL)
+		vsock = net->vsock.loopback;
+	else
+		vsock = &the_vsock_loopback;
 
 	virtio_vsock_skb_queue_tail(&vsock->pkt_queue, skb);
 	queue_work(vsock->workqueue, &vsock->pkt_work);
@@ -46,7 +57,7 @@ static int vsock_loopback_cancel_pkt(struct vsock_sock *vsk)
 	return 0;
 }
 
-static bool vsock_loopback_seqpacket_allow(u32 remote_cid);
+static bool vsock_loopback_seqpacket_allow(struct vsock_sock *vsk, u32 remote_cid);
 static bool vsock_loopback_msgzerocopy_allow(void)
 {
 	return true;
@@ -106,7 +117,7 @@ static struct virtio_transport loopback_transport = {
 	.send_pkt = vsock_loopback_send_pkt,
 };
 
-static bool vsock_loopback_seqpacket_allow(u32 remote_cid)
+static bool vsock_loopback_seqpacket_allow(struct vsock_sock *vsk, u32 remote_cid)
 {
 	return true;
 }
@@ -134,27 +145,55 @@ static void vsock_loopback_work(struct work_struct *work)
 	}
 }
 
-static int __init vsock_loopback_init(void)
+static int vsock_loopback_init_vsock(struct vsock_loopback *vsock)
 {
-	struct vsock_loopback *vsock = &the_vsock_loopback;
-	int ret;
-
 	vsock->workqueue = alloc_workqueue("vsock-loopback", 0, 0);
 	if (!vsock->workqueue)
 		return -ENOMEM;
 
 	skb_queue_head_init(&vsock->pkt_queue);
 	INIT_WORK(&vsock->pkt_work, vsock_loopback_work);
+	return 0;
+}
+
+static void vsock_loopback_deinit_vsock(struct vsock_loopback *vsock)
+{
+	destroy_workqueue(vsock->workqueue);
+}
+
+int vsock_loopback_init_net(struct net *net)
+{
+	net->vsock.loopback = kmalloc(GFP_KERNEL, sizeof(struct vsock_loopback));
+	if (!net->vsock.loopback)
+		return -ENOMEM;
+
+	return vsock_loopback_init_vsock(net->vsock.loopback);
+}
+
+void vsock_loopback_exit_net(struct net *net)
+{
+	vsock_loopback_deinit_vsock(net->vsock.loopback);
+	kfree(net->vsock.loopback);
+}
+
+static int __init vsock_loopback_init(void)
+{
+	struct vsock_loopback *vsock = &the_vsock_loopback;
+	int ret;
+
+	ret = vsock_loopback_init_vsock(vsock);
+	if (ret < 0)
+		return ret;
 
 	ret = vsock_core_register(&loopback_transport.transport,
 				  VSOCK_TRANSPORT_F_LOCAL);
 	if (ret)
-		goto out_wq;
+		goto out_deinit;
 
 	return 0;
 
-out_wq:
-	destroy_workqueue(vsock->workqueue);
+out_deinit:
+	vsock_loopback_deinit_vsock(vsock);
 	return ret;
 }
 

-- 
2.47.3
Re: [PATCH RFC net-next v4 09/12] vsock/loopback: add netns support
Posted by Simon Horman 1 month, 4 weeks ago
On Tue, Aug 05, 2025 at 02:49:17PM -0700, Bobby Eshleman wrote:
> From: Bobby Eshleman <bobbyeshleman@meta.com>
> 
> Add NS support to vsock loopback. Sockets in a global mode netns
> communicate with each other, regardless of namespace. Sockets in a local
> mode netns may only communicate with other sockets within the same
> namespace.
> 
> Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>

...

> diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c

...

> @@ -46,7 +57,7 @@ static int vsock_loopback_cancel_pkt(struct vsock_sock *vsk)
>  	return 0;
>  }
>  
> -static bool vsock_loopback_seqpacket_allow(u32 remote_cid);
> +static bool vsock_loopback_seqpacket_allow(struct vsock_sock *vsk, u32 remote_cid);

This change needs to be squashed into
PATCH 3/12 vsock: add netns to af_vsock core

To avoid build breakage.

Likewise with the other change to vsock_loopback_seqpacket_allow below.
And I think also for a number of other changes made by PATCH 3/12.

Please make sure that patches don't introduce transient build failures.
It breaks bisection.


On the topic of vsock_loopback_seqpacket_allow, also:

* Please line wrap this so that the code is 80 columns wide or less,
  as is still preferred for Networking code.

  Flagged by checkpatch.pl --max-line-length=80

* Can we move the definition of vsock_loopback_seqpacket_allow() here?
  The function itself is is trivial. And doing so would avoid a forward
  declaration.

>  static bool vsock_loopback_msgzerocopy_allow(void)
>  {
>  	return true;

...

> +int vsock_loopback_init_net(struct net *net)
> +{
> +	net->vsock.loopback = kmalloc(GFP_KERNEL, sizeof(struct vsock_loopback));
> +	if (!net->vsock.loopback)
> +		return -ENOMEM;
> +
> +	return vsock_loopback_init_vsock(net->vsock.loopback);
> +}
> +
> +void vsock_loopback_exit_net(struct net *net)
> +{
> +	vsock_loopback_deinit_vsock(net->vsock.loopback);
> +	kfree(net->vsock.loopback);
> +}

I think EXPORT_SYMBOL_GPL is needed for both vsock_loopback_exit_net and
vsock_loopback_init_net for the case where CONFIG_VSOCKETS=m

Also, in Kconfig VSOCKETS_LOOPBACK depends on VSOCKETS. But this code adds
a reverse dependency. As it stands it's possible to configure VSOCKETS
without VSOCKETS_LOOPBACK, which will not compile.

Perhaps stub implementations of vsock_loopback_init_net and
vsock_loopback_exit_net should be implemented in af_vsock.h if
VSOCKETS_LOOPBACK is not enabled?

...

-- 
pw-bot: changes-requested
Re: [PATCH RFC net-next v4 09/12] vsock/loopback: add netns support
Posted by Bobby Eshleman 1 month, 4 weeks ago
On Wed, Aug 06, 2025 at 08:12:39PM +0100, Simon Horman wrote:
> On Tue, Aug 05, 2025 at 02:49:17PM -0700, Bobby Eshleman wrote:
> > From: Bobby Eshleman <bobbyeshleman@meta.com>
> > 

...

> 
> This change needs to be squashed into
> PATCH 3/12 vsock: add netns to af_vsock core
> 
> To avoid build breakage.
> 
> Likewise with the other change to vsock_loopback_seqpacket_allow below.
> And I think also for a number of other changes made by PATCH 3/12.
> 
> Please make sure that patches don't introduce transient build failures.
> It breaks bisection.

Will do, thanks!

> 
> 
> On the topic of vsock_loopback_seqpacket_allow, also:
> 
> * Please line wrap this so that the code is 80 columns wide or less,
>   as is still preferred for Networking code.
> 
>   Flagged by checkpatch.pl --max-line-length=80
> 
> * Can we move the definition of vsock_loopback_seqpacket_allow() here?
>   The function itself is is trivial. And doing so would avoid a forward
>   declaration.
> 
> >  static bool vsock_loopback_msgzerocopy_allow(void)
> >  {
> >  	return true;
> 
> ...
> 
> > +int vsock_loopback_init_net(struct net *net)
> > +{
> > +	net->vsock.loopback = kmalloc(GFP_KERNEL, sizeof(struct vsock_loopback));
> > +	if (!net->vsock.loopback)
> > +		return -ENOMEM;
> > +
> > +	return vsock_loopback_init_vsock(net->vsock.loopback);
> > +}
> > +
> > +void vsock_loopback_exit_net(struct net *net)
> > +{
> > +	vsock_loopback_deinit_vsock(net->vsock.loopback);
> > +	kfree(net->vsock.loopback);
> > +}
> 
> I think EXPORT_SYMBOL_GPL is needed for both vsock_loopback_exit_net and
> vsock_loopback_init_net for the case where CONFIG_VSOCKETS=m
> 
> Also, in Kconfig VSOCKETS_LOOPBACK depends on VSOCKETS. But this code adds
> a reverse dependency. As it stands it's possible to configure VSOCKETS
> without VSOCKETS_LOOPBACK, which will not compile.
> 
> Perhaps stub implementations of vsock_loopback_init_net and
> vsock_loopback_exit_net should be implemented in af_vsock.h if
> VSOCKETS_LOOPBACK is not enabled?
> 

Roger that, makes sense. Thanks for the review!

Best,
Bobby