[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <tuclqrdg5p2uzfvczhcdig7jlifvhqtlafe4xcqy4x4p3vrya6@jq5mujdluze5>
Date: Thu, 6 Nov 2025 17:18:36 +0100
From: Stefano Garzarella <sgarzare@...hat.com>
To: Bobby Eshleman <bobbyeshleman@...il.com>
Cc: Shuah Khan <shuah@...nel.org>, "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
Stefan Hajnoczi <stefanha@...hat.com>, "Michael S. Tsirkin" <mst@...hat.com>,
Jason Wang <jasowang@...hat.com>, Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
Eugenio Pérez <eperezma@...hat.com>, "K. Y. Srinivasan" <kys@...rosoft.com>,
Haiyang Zhang <haiyangz@...rosoft.com>, Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>,
Bryan Tan <bryan-bt.tan@...adcom.com>, Vishnu Dasa <vishnu.dasa@...adcom.com>,
Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>, virtualization@...ts.linux.dev, netdev@...r.kernel.org,
linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
linux-hyperv@...r.kernel.org, berrange@...hat.com, Bobby Eshleman <bobbyeshleman@...a.com>
Subject: Re: [PATCH net-next v8 05/14] vsock/loopback: add netns support
On Thu, Oct 23, 2025 at 11:27:44AM -0700, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@...a.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.
>
>Use pernet_ops to install a vsock_loopback for every namespace that is
>created (to be used if local mode is enabled).
>
>Retroactively call init/exit on every namespace when the vsock_loopback
>module is loaded in order to initialize the per-ns device.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@...a.com>
>---
I'm a bit confused, should we move this after the next patch that add
support of netns in the virtio common module?
Or this is a pre-requisite?
>Changes in v7:
>- drop for_each_net() init/exit, drop net_rwsem, the pernet registration
> handles this automatically and race-free
>- flush workqueue before destruction, purge pkt list
>- remember net_mode instead of current net mode
>- keep space after INIT_WORK()
>- change vsock_loopback in netns_vsock to ->priv void ptr
>- rename `orig_net_mode` to `net_mode`
>- remove useless comment
>- protect `register_pernet_subsys()` with `net_rwsem`
>- do cleanup before releasing `net_rwsem` when failure happens
>- call `unregister_pernet_subsys()` in `vsock_loopback_exit()`
>- call `vsock_loopback_deinit_vsock()` in `vsock_loopback_exit()`
>
>Changes in v6:
>- init pernet ops for vsock_loopback module
>- vsock_loopback: add space in struct to clarify lock protection
>- do proper cleanup/unregister on vsock_loopback_exit()
>- vsock_loopback: use virtio_vsock_skb_net()
>
>Changes in v5:
>- add callbacks code to avoid reverse dependency
>- add logic for handling vsock_loopback setup for already existing
> namespaces
>---
> include/net/netns/vsock.h | 2 +
> net/vmw_vsock/vsock_loopback.c | 85 ++++++++++++++++++++++++++++++++++++------
> 2 files changed, 75 insertions(+), 12 deletions(-)
>
>diff --git a/include/net/netns/vsock.h b/include/net/netns/vsock.h
>index c9a438ad52f2..9d0d8e2fbc37 100644
>--- a/include/net/netns/vsock.h
>+++ b/include/net/netns/vsock.h
>@@ -16,5 +16,7 @@ struct netns_vsock {
> /* protected by lock */
> enum vsock_net_mode mode;
> bool mode_locked;
>+
>+ void *priv;
> };
> #endif /* __NET_NET_NAMESPACE_VSOCK_H */
>diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
>index a8f218f0c5a3..474083d4cfcb 100644
>--- a/net/vmw_vsock/vsock_loopback.c
>+++ b/net/vmw_vsock/vsock_loopback.c
>@@ -28,8 +28,16 @@ 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;
>+
>+ net = virtio_vsock_skb_net(skb);
>+
>+ if (virtio_vsock_skb_net_mode(skb) == VSOCK_NET_MODE_LOCAL)
>+ vsock = (struct vsock_loopback *)net->vsock.priv;
Is there some kind of refcount on the net?
What I mean is, are we sure this pointer is still valid? Could the net
disappear in the meantime?
The rest LGTM!
Thanks,
Stefano
>+ else
>+ vsock = &the_vsock_loopback;
>
> virtio_vsock_skb_queue_tail(&vsock->pkt_queue, skb);
> queue_work(vsock->workqueue, &vsock->pkt_work);
>@@ -134,11 +142,8 @@ 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", WQ_PERCPU, 0);
> if (!vsock->workqueue)
> return -ENOMEM;
>@@ -146,15 +151,73 @@ static int __init vsock_loopback_init(void)
> 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)
>+{
>+ if (vsock->workqueue) {
>+ flush_work(&vsock->pkt_work);
>+ virtio_vsock_skb_queue_purge(&vsock->pkt_queue);
>+ destroy_workqueue(vsock->workqueue);
>+ vsock->workqueue = NULL;
>+ }
>+}
>+
>+static int vsock_loopback_init_net(struct net *net)
>+{
>+ int ret;
>+
>+ net->vsock.priv = kzalloc(sizeof(struct vsock_loopback), GFP_KERNEL);
>+ if (!net->vsock.priv)
>+ return -ENOMEM;
>+
>+ ret = vsock_loopback_init_vsock((struct vsock_loopback *)net->vsock.priv);
>+ if (ret < 0) {
>+ kfree(net->vsock.priv);
>+ net->vsock.priv = NULL;
>+ return ret;
>+ }
>+
>+ return 0;
>+}
>+
>+static void vsock_loopback_exit_net(struct net *net)
>+{
>+ vsock_loopback_deinit_vsock(net->vsock.priv);
>+ kfree(net->vsock.priv);
>+ net->vsock.priv = NULL;
>+}
>+
>+static struct pernet_operations vsock_loopback_net_ops = {
>+ .init = vsock_loopback_init_net,
>+ .exit = vsock_loopback_exit_net,
>+};
>+
>+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 = register_pernet_subsys(&vsock_loopback_net_ops);
>+ if (ret < 0)
>+ goto out_deinit_vsock;
>+
> ret = vsock_core_register(&loopback_transport.transport,
> VSOCK_TRANSPORT_F_LOCAL);
> if (ret)
>- goto out_wq;
>+ goto out_unregister_pernet_subsys;
>
> return 0;
>
>-out_wq:
>- destroy_workqueue(vsock->workqueue);
>+out_unregister_pernet_subsys:
>+ unregister_pernet_subsys(&vsock_loopback_net_ops);
>+out_deinit_vsock:
>+ vsock_loopback_deinit_vsock(vsock);
> return ret;
> }
>
>@@ -164,11 +227,9 @@ static void __exit vsock_loopback_exit(void)
>
> vsock_core_unregister(&loopback_transport.transport);
>
>- flush_work(&vsock->pkt_work);
>-
>- virtio_vsock_skb_queue_purge(&vsock->pkt_queue);
>+ unregister_pernet_subsys(&vsock_loopback_net_ops);
>
>- destroy_workqueue(vsock->workqueue);
>+ vsock_loopback_deinit_vsock(vsock);
> }
>
> module_init(vsock_loopback_init);
>
>--
>2.47.3
>
Powered by blists - more mailing lists