Re: [PATCH RFC net-next v13 01/13] vsock: add per-net vsock NS mode state

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Dec 23, 2025 at 04:28:35PM -0800, Bobby Eshleman wrote:
> From: Bobby Eshleman <bobbyeshleman@meta.com>
> 
> Add the per-net vsock NS mode state. This only adds the structure for
> holding the mode and some of the functions for setting/getting and
> checking the mode, but does not integrate the functionality yet.
> 
> Future patches add the uAPI and transport-specific usage of these
> structures and helpers.
> 
> Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>


I do not much like splitting out functionality like this,
myself - there's little to no docs and one can't figure out
whether this code does what it is  supposed to do without
reading the next patch.

I would just squash this with that one.

If you are splitting functionality along some API lines,
you need detailed docs so one can review implementation
separately from use.


> ---
> Changes in v13:
> - remove net_mode because net->vsock.mode becomes immutable, no need to
>   save the mode when vsocks are created.
> - add the new helpers for child_ns_mode to support ns_mode inheriting
>   the mode from child_ns_mode.
> - because ns_mode is immutable and child_ns_mode can be changed multiple
>   times, remove the write-once lock.
> - simplify vsock_net_check_mode() to no longer take mode arguments since
>   the mode can be accessed via the net pointers without fear of the mode
>   changing.
> - add logic in vsock_net_check_mode() to infer VSOCK_NET_MODE_GLOBAL
>   from NULL namespaces in order to allow only net pointers to be passed
>   to vsock_net_check_mode(), while still allowing namespace-unaware
>   transports to force global mode.
> 
> Changes in v10:
> - change mode_locked to int (Stefano)
> 
> Changes in v9:
> - use xchg(), WRITE_ONCE(), READ_ONCE() for mode and mode_locked (Stefano)
> - clarify mode0/mode1 meaning in vsock_net_check_mode() comment
> - remove spin lock in net->vsock (not used anymore)
> - change mode from u8 to enum vsock_net_mode in vsock_net_write_mode()
> 
> Changes in v7:
> - clarify vsock_net_check_mode() comments
> - change to `orig_net_mode == VSOCK_NET_MODE_GLOBAL && orig_net_mode == vsk->orig_net_mode`
> - remove extraneous explanation of `orig_net_mode`
> - rename `written` to `mode_locked`
> - rename `vsock_hdr` to `sysctl_hdr`
> - change `orig_net_mode` to `net_mode`
> - make vsock_net_check_mode() more generic by taking just net pointers
>   and modes, instead of a vsock_sock ptr, for reuse by transports
>   (e.g., vhost_vsock)
> 
> Changes in v6:
> - add orig_net_mode to store mode at creation time which will be used to
>   avoid breakage when namespace changes mode during socket/VM lifespan
> 
> Changes in v5:
> - use /proc/sys/net/vsock/ns_mode instead of /proc/net/vsock_ns_mode
> - change from net->vsock.ns_mode to net->vsock.mode
> - change vsock_net_set_mode() to vsock_net_write_mode()
> - vsock_net_write_mode() returns bool for write success to avoid
>   need to use vsock_net_mode_can_set()
> - remove vsock_net_mode_can_set()
> ---
>  MAINTAINERS                 |  1 +
>  include/net/af_vsock.h      | 42 ++++++++++++++++++++++++++++++++++++++++++
>  include/net/net_namespace.h |  4 ++++
>  include/net/netns/vsock.h   | 17 +++++++++++++++++
>  4 files changed, 64 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 454b8ed119e9..38d24e5a957c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -27516,6 +27516,7 @@ L:	netdev@vger.kernel.org
>  S:	Maintained
>  F:	drivers/vhost/vsock.c
>  F:	include/linux/virtio_vsock.h
> +F:	include/net/netns/vsock.h
>  F:	include/uapi/linux/virtio_vsock.h
>  F:	net/vmw_vsock/virtio_transport.c
>  F:	net/vmw_vsock/virtio_transport_common.c
> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> index d40e978126e3..6f5bc9dbefa5 100644
> --- a/include/net/af_vsock.h
> +++ b/include/net/af_vsock.h
> @@ -10,6 +10,7 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/workqueue.h>
> +#include <net/netns/vsock.h>
>  #include <net/sock.h>
>  #include <uapi/linux/vm_sockets.h>
>  
> @@ -256,4 +257,45 @@ static inline bool vsock_msgzerocopy_allow(const struct vsock_transport *t)
>  {
>  	return t->msgzerocopy_allow && t->msgzerocopy_allow();
>  }
> +
> +static inline enum vsock_net_mode vsock_net_mode(struct net *net)
> +{
> +	return READ_ONCE(net->vsock.mode);
> +}
> +
> +static inline void vsock_net_set_child_mode(struct net *net,
> +					    enum vsock_net_mode mode)
> +{
> +	WRITE_ONCE(net->vsock.child_ns_mode, mode);
> +}
> +
> +static inline enum vsock_net_mode vsock_net_child_mode(struct net *net)
> +{
> +	return READ_ONCE(net->vsock.child_ns_mode);
> +}
> +
> +/* Return true if two namespaces pass the mode rules. Otherwise, return false.
> + *
> + * A NULL namespace is treated as VSOCK_NET_MODE_GLOBAL.
> + *
> + * Read more about modes in the comment header of net/vmw_vsock/af_vsock.c.
> + */
> +static inline bool vsock_net_check_mode(struct net *ns0, struct net *ns1)
> +{
> +	enum vsock_net_mode mode0, mode1;
> +
> +	/* Any vsocks within the same network namespace are always reachable,
> +	 * regardless of the mode.
> +	 */
> +	if (net_eq(ns0, ns1))
> +		return true;
> +
> +	mode0 = ns0 ? vsock_net_mode(ns0) : VSOCK_NET_MODE_GLOBAL;
> +	mode1 = ns1 ? vsock_net_mode(ns1) : VSOCK_NET_MODE_GLOBAL;
> +
> +	/* Different namespaces are only reachable if they are both
> +	 * global mode.
> +	 */
> +	return mode0 == VSOCK_NET_MODE_GLOBAL && mode0 == mode1;
> +}
>  #endif /* __AF_VSOCK_H__ */
> diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
> index cb664f6e3558..66d3de1d935f 100644
> --- a/include/net/net_namespace.h
> +++ b/include/net/net_namespace.h
> @@ -37,6 +37,7 @@
>  #include <net/netns/smc.h>
>  #include <net/netns/bpf.h>
>  #include <net/netns/mctp.h>
> +#include <net/netns/vsock.h>
>  #include <net/net_trackers.h>
>  #include <linux/ns_common.h>
>  #include <linux/idr.h>
> @@ -196,6 +197,9 @@ struct net {
>  	/* Move to a better place when the config guard is removed. */
>  	struct mutex		rtnl_mutex;
>  #endif
> +#if IS_ENABLED(CONFIG_VSOCKETS)
> +	struct netns_vsock	vsock;
> +#endif
>  } __randomize_layout;
>  
>  #include <linux/seq_file_net.h>
> diff --git a/include/net/netns/vsock.h b/include/net/netns/vsock.h
> new file mode 100644
> index 000000000000..e2325e2d6ec5
> --- /dev/null
> +++ b/include/net/netns/vsock.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __NET_NET_NAMESPACE_VSOCK_H
> +#define __NET_NET_NAMESPACE_VSOCK_H
> +
> +#include <linux/types.h>
> +
> +enum vsock_net_mode {
> +	VSOCK_NET_MODE_GLOBAL,
> +	VSOCK_NET_MODE_LOCAL,
> +};
> +
> +struct netns_vsock {
> +	struct ctl_table_header *sysctl_hdr;
> +	enum vsock_net_mode mode;
> +	enum vsock_net_mode child_ns_mode;
> +};
> +#endif /* __NET_NET_NAMESPACE_VSOCK_H */
> 
> -- 
> 2.47.3