* Re: [PATCH net-next v10 03/11] vsock: reject bad VSOCK_NET_MODE_LOCAL configuration for G2H
From: Stefano Garzarella @ 2025-11-18 18:10 UTC (permalink / raw)
To: Bobby Eshleman
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Eugenio Pérez, Xuan Zhuo, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, Shuah Khan, linux-kernel,
virtualization, netdev, kvm, linux-hyperv, linux-kselftest,
Sargun Dhillon, berrange, Bobby Eshleman
In-Reply-To: <20251117-vsock-vmtest-v10-3-df08f165bf3e@meta.com>
On Mon, Nov 17, 2025 at 06:00:26PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Reject setting VSOCK_NET_MODE_LOCAL with -EOPNOTSUPP if a G2H transport
>is operational. Additionally, reject G2H transport registration if there
>already exists a namespace in local mode.
>
>G2H sockets break in local mode because the G2H transports don't support
>namespacing yet. The current approach is to coerce packets coming out of
>G2H transports into VSOCK_NET_MODE_GLOBAL mode, but it is not possible
>to coerce sockets in the same way because it cannot be deduced which
>transport will be used by the socket. Specifically, when bound to
>VMADDR_CID_ANY in a nested VM (both G2H and H2G available), it is not
>until a packet is received and matched to the bound socket that we
>assign the transport. This presents a chicken-and-egg problem, because
>we need the namespace to lookup the socket and resolve the transport,
>but we need the transport to know how to use the namespace during
>lookup.
>
>For that reason, this patch prevents VSOCK_NET_MODE_LOCAL from being
>used on systems that support G2H, even nested systems that also have H2G
>transports.
>
>Local mode is blocked based on detecting the presence of G2H devices
>(when possible, as hyperv is special). This means that a host kernel
>with G2H support compiled in (or has the module loaded), will still
>support local mode if there is no G2H (e.g., virtio-vsock) device
>detected. This enables using the same kernel in the host and in the
>guest, as we do in kselftest.
>
>Systems with only namespace-aware transports (vhost-vsock, loopback) can
>still use both VSOCK_NET_MODE_GLOBAL and VSOCK_NET_MODE_LOCAL modes as
>intended.
>
>Add supports_local_mode() transport callback to indicate
>transport-specific local mode support.
>
>These restrictions can be lifted in a future patch series when G2H
>transports gain namespace support.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
>Changes in v10:
>- move this patch before any transports bring online namespacing (Stefano)
>- move vsock_net_mode_string into critical section (Stefano)
>- add ->supports_local_mode() callback to transports (Stefano)
>---
> drivers/vhost/vsock.c | 6 +++++
> include/net/af_vsock.h | 5 ++++
> net/vmw_vsock/af_vsock.c | 50 ++++++++++++++++++++++++++++++++++------
> net/vmw_vsock/hyperv_transport.c | 6 +++++
> net/vmw_vsock/virtio_transport.c | 13 +++++++++++
> net/vmw_vsock/vmci_transport.c | 7 ++++++
> net/vmw_vsock/vsock_loopback.c | 6 +++++
> 7 files changed, 86 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>index 2c937a2df83b..c8319cd1c232 100644
>--- a/drivers/vhost/vsock.c
>+++ b/drivers/vhost/vsock.c
>@@ -64,6 +64,11 @@ static u32 vhost_transport_get_local_cid(void)
> return VHOST_VSOCK_DEFAULT_HOST_CID;
> }
>
>+static bool vhost_transport_supports_local_mode(void)
>+{
>+ return true;
Should we enable this later, when we really add support, or it doesn't
affect anything if vhost-vsock is not really supporting it in this PR
(thinking about bisection issues).
>+}
>+
> /* Callers that dereference the return value must hold vhost_vsock_mutex or the
> * RCU read lock.
> */
>@@ -412,6 +417,7 @@ static struct virtio_transport vhost_transport = {
> .module = THIS_MODULE,
>
> .get_local_cid = vhost_transport_get_local_cid,
>+ .supports_local_mode = vhost_transport_supports_local_mode,
>
> .init = virtio_transport_do_socket_init,
> .destruct = virtio_transport_destruct,
>diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
>index 59d97a143204..824d89657d41 100644
>--- a/include/net/af_vsock.h
>+++ b/include/net/af_vsock.h
>@@ -180,6 +180,11 @@ struct vsock_transport {
> /* Addressing. */
> u32 (*get_local_cid)(void);
>
>+ /* Return true if this transport supports VSOCK_NET_MODE_LOCAL.
nit: Here I would make it clearer that rather than supporting
MODE_LOCAL, the transport is not compatible with it, etc.
A summary of the excellent description we have in the commit.
>+ * Otherwise, return false.
>+ */
>+ bool (*supports_local_mode)(void);
>+
> /* Read a single skb */
> int (*read_skb)(struct vsock_sock *, skb_read_actor_t);
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 54373ae101c3..7a235bb94437 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -91,6 +91,12 @@
> * and locked down by a namespace manager. The default is "global". The mode
> * is set per-namespace.
> *
>+ * Note: LOCAL mode is only supported when using namespace-aware transports
>+ * (vhost-vsock, loopback). If a guest-to-host transport (virtio-vsock,
>+ * hyperv-vsock, vmci-vsock) is loaded, attempts to set LOCAL mode will fail
>+ * with EOPNOTSUPP, as these transports do not support per-namespace
>+ * isolation.
Okay, maybe this is fine, so if you don't need to resend, feel free to
ignore the previous comment.
>+ *
> * The modes affect the allocation and accessibility of CIDs as follows:
> *
> * - global - access and allocation are all system-wide
>@@ -2765,17 +2771,30 @@ static int vsock_net_mode_string(const struct ctl_table *table, int write,
> if (*lenp >= sizeof(data))
> return -EINVAL;
>
>- if (!strncmp(data, VSOCK_NET_MODE_STR_GLOBAL, sizeof(data)))
>+ ret = 0;
IIUC `ret` should already be 0 at this point, no?
>+ mutex_lock(&vsock_register_mutex);
I honestly don't like to mix the parsing, with this new check, so what
about leaving the parsing as before this patch (also without the mutex),
then just (untested):
mutex_lock(&vsock_register_mutex);
if (mode == VSOCK_NET_MODE_LOCAL && transport_g2h &&
transport_g2h->supports_local_mode &&
!transport_g2h->supports_local_mode()) {
ret = -EOPNOTSUPP;
goto out;
}
if (!vsock_net_write_mode(net, mode)) {
ret = -EPERM;
}
out:
mutex_unlock(&vsock_register_mutex);
return ret;
}
>+ if (!strncmp(data, VSOCK_NET_MODE_STR_GLOBAL, sizeof(data))) {
> mode = VSOCK_NET_MODE_GLOBAL;
>- else if (!strncmp(data, VSOCK_NET_MODE_STR_LOCAL, sizeof(data)))
>+ } else if (!strncmp(data, VSOCK_NET_MODE_STR_LOCAL, sizeof(data))) {
>+ if (transport_g2h && transport_g2h->supports_local_mode &&
>+ !transport_g2h->supports_local_mode()) {
>+ ret = -EOPNOTSUPP;
>+ goto out;
>+ }
> mode = VSOCK_NET_MODE_LOCAL;
>- else
>- return -EINVAL;
>+ } else {
>+ ret = -EINVAL;
>+ goto out;
>+ }
>
>- if (!vsock_net_write_mode(net, mode))
>- return -EPERM;
>+ if (!vsock_net_write_mode(net, mode)) {
>+ ret = -EPERM;
>+ goto out;
>+ }
>
>- return 0;
>+out:
>+ mutex_unlock(&vsock_register_mutex);
>+ return ret;
> }
>
> static struct ctl_table vsock_table[] = {
>@@ -2916,6 +2935,7 @@ int vsock_core_register(const struct vsock_transport *t, int features)
> {
> const struct vsock_transport *t_h2g, *t_g2h, *t_dgram, *t_local;
> int err = mutex_lock_interruptible(&vsock_register_mutex);
>+ struct net *net;
>
> if (err)
> return err;
>@@ -2938,6 +2958,22 @@ int vsock_core_register(const struct vsock_transport *t, int features)
> err = -EBUSY;
> goto err_busy;
> }
>+
>+ /* G2H sockets break in LOCAL mode namespaces because G2H
>+ * transports don't support them yet. Block registering new G2H
>+ * transports if we already have local mode namespaces on the
>+ * system.
>+ */
>+ rcu_read_lock();
>+ for_each_net_rcu(net) {
>+ if (vsock_net_mode(net) == VSOCK_NET_MODE_LOCAL) {
>+ rcu_read_unlock();
>+ err = -EOPNOTSUPP;
>+ goto err_busy;
>+ }
>+ }
>+ rcu_read_unlock();
>+
> t_g2h = t;
> }
>
>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>index 432fcbbd14d4..279f04fcd81a 100644
>--- a/net/vmw_vsock/hyperv_transport.c
>+++ b/net/vmw_vsock/hyperv_transport.c
>@@ -833,10 +833,16 @@ int hvs_notify_set_rcvlowat(struct vsock_sock *vsk, int val)
> return -EOPNOTSUPP;
> }
>
>+static bool hvs_supports_local_mode(void)
>+{
>+ return false;
>+}
>+
> static struct vsock_transport hvs_transport = {
> .module = THIS_MODULE,
>
> .get_local_cid = hvs_get_local_cid,
>+ .supports_local_mode = hvs_supports_local_mode,
>
> .init = hvs_sock_init,
> .destruct = hvs_destruct,
>diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
>index 5d379ccf3770..e585cb66c6f5 100644
>--- a/net/vmw_vsock/virtio_transport.c
>+++ b/net/vmw_vsock/virtio_transport.c
>@@ -94,6 +94,18 @@ static u32 virtio_transport_get_local_cid(void)
> return ret;
> }
>
>+static bool virtio_transport_supports_local_mode(void)
>+{
>+ struct virtio_vsock *vsock;
>+
>+ rcu_read_lock();
>+ vsock = rcu_dereference(the_virtio_vsock);
>+ rcu_read_unlock();
>+
>+ /* Local mode is supported only when no G2H device is present. */
>+ return vsock ? false : true;
>+}
>+
> /* Caller need to hold vsock->tx_lock on vq */
> static int virtio_transport_send_skb(struct sk_buff *skb, struct virtqueue *vq,
> struct virtio_vsock *vsock, gfp_t gfp)
>@@ -544,6 +556,7 @@ static struct virtio_transport virtio_transport = {
> .module = THIS_MODULE,
>
> .get_local_cid = virtio_transport_get_local_cid,
>+ .supports_local_mode = virtio_transport_supports_local_mode,
>
> .init = virtio_transport_do_socket_init,
> .destruct = virtio_transport_destruct,
>diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
>index 7eccd6708d66..da7c52ad7b2a 100644
>--- a/net/vmw_vsock/vmci_transport.c
>+++ b/net/vmw_vsock/vmci_transport.c
>@@ -2033,6 +2033,12 @@ static u32 vmci_transport_get_local_cid(void)
> return vmci_get_context_id();
> }
>
>+static bool vmci_transport_supports_local_mode(void)
>+{
>+ /* Local mode is supported only when no device is present. */
>+ return vmci_transport_get_local_cid() == VMCI_INVALID_ID;
IIRC vmci can be registered both as H2G and G2H, so should we filter out
the H2G case?
Also, IMO is better to use VMADDR_CID_ANY with get_local_cid().
>+}
>+
> static struct vsock_transport vmci_transport = {
> .module = THIS_MODULE,
> .init = vmci_transport_socket_init,
>@@ -2062,6 +2068,7 @@ static struct vsock_transport vmci_transport = {
> .notify_send_post_enqueue = vmci_transport_notify_send_post_enqueue,
> .shutdown = vmci_transport_shutdown,
> .get_local_cid = vmci_transport_get_local_cid,
>+ .supports_local_mode = vmci_transport_supports_local_mode,
> };
>
> static bool vmci_check_transport(struct vsock_sock *vsk)
>diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
>index 8722337a4f80..1e25c1a6b43f 100644
>--- a/net/vmw_vsock/vsock_loopback.c
>+++ b/net/vmw_vsock/vsock_loopback.c
>@@ -26,6 +26,11 @@ static u32 vsock_loopback_get_local_cid(void)
> return VMADDR_CID_LOCAL;
> }
>
>+static bool vsock_loopback_supports_local_mode(void)
>+{
>+ return true;
>+}
>+
> static int vsock_loopback_send_pkt(struct sk_buff *skb)
> {
> struct vsock_loopback *vsock = &the_vsock_loopback;
>@@ -58,6 +63,7 @@ static struct virtio_transport loopback_transport = {
> .module = THIS_MODULE,
>
> .get_local_cid = vsock_loopback_get_local_cid,
>+ .supports_local_mode = vsock_loopback_supports_local_mode,
>
> .init = virtio_transport_do_socket_init,
> .destruct = virtio_transport_destruct,
>
>--
>2.47.3
>
^ permalink raw reply
* RE: [PATCH v5 1/3] hyperv: Add definitions for MSHV sleep state configuration
From: Michael Kelley @ 2025-11-18 18:10 UTC (permalink / raw)
To: Praveen K Paladugu, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com, tglx@linutronix.de,
mingo@redhat.com, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, bp@alien8.de,
dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com,
arnd@arndb.de
Cc: anbelski@linux.microsoft.com,
easwar.hariharan@linux.microsoft.com,
nunodasneves@linux.microsoft.com,
skinsburskii@linux.microsoft.com
In-Reply-To: <20251117210855.108126-2-prapal@linux.microsoft.com>
From: Praveen K Paladugu <prapal@linux.microsoft.com> Sent: Monday, November 17, 2025 1:08 PM
>
> Add the definitions required to configure sleep states in mshv hypervsior.
>
> Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
> Co-developed-by: Anatol Belski <anbelski@linux.microsoft.com>
> Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
> Reviewed-by: Easwar Hariharan <easwar.hariharan@linux.microsoft.com>
> Reviewed-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---
> include/hyperv/hvgdk_mini.h | 4 +++-
> include/hyperv/hvhdk_mini.h | 40 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
> index 1d5ce11be8b6..04b18d0e37af 100644
> --- a/include/hyperv/hvgdk_mini.h
> +++ b/include/hyperv/hvgdk_mini.h
> @@ -465,19 +465,21 @@ union hv_vp_assist_msr_contents { /*
> HV_REGISTER_VP_ASSIST_PAGE */
> #define HVCALL_RESET_DEBUG_SESSION 0x006b
> #define HVCALL_MAP_STATS_PAGE 0x006c
> #define HVCALL_UNMAP_STATS_PAGE 0x006d
> +#define HVCALL_SET_SYSTEM_PROPERTY 0x006f
> #define HVCALL_ADD_LOGICAL_PROCESSOR 0x0076
> #define HVCALL_GET_SYSTEM_PROPERTY 0x007b
> #define HVCALL_MAP_DEVICE_INTERRUPT 0x007c
> #define HVCALL_UNMAP_DEVICE_INTERRUPT 0x007d
> #define HVCALL_RETARGET_INTERRUPT 0x007e
> #define HVCALL_NOTIFY_PARTITION_EVENT 0x0087
> +#define HVCALL_ENTER_SLEEP_STATE 0x0084
> #define HVCALL_NOTIFY_PORT_RING_EMPTY 0x008b
> #define HVCALL_REGISTER_INTERCEPT_RESULT 0x0091
> #define HVCALL_ASSERT_VIRTUAL_INTERRUPT 0x0094
> #define HVCALL_CREATE_PORT 0x0095
> #define HVCALL_CONNECT_PORT 0x0096
> #define HVCALL_START_VP 0x0099
> -#define HVCALL_GET_VP_INDEX_FROM_APIC_ID 0x009a
> +#define HVCALL_GET_VP_INDEX_FROM_APIC_ID 0x009a
> #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE 0x00af
> #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST 0x00b0
> #define HVCALL_SIGNAL_EVENT_DIRECT 0x00c0
> diff --git a/include/hyperv/hvhdk_mini.h b/include/hyperv/hvhdk_mini.h
> index f2d7b50de7a4..41a29bf8ec14 100644
> --- a/include/hyperv/hvhdk_mini.h
> +++ b/include/hyperv/hvhdk_mini.h
> @@ -140,6 +140,7 @@ enum hv_snp_status {
>
> enum hv_system_property {
> /* Add more values when needed */
> + HV_SYSTEM_PROPERTY_SLEEP_STATE = 3,
> HV_SYSTEM_PROPERTY_SCHEDULER_TYPE = 15,
> HV_DYNAMIC_PROCESSOR_FEATURE_PROPERTY = 21,
> HV_SYSTEM_PROPERTY_CRASHDUMPAREA = 47,
> @@ -155,6 +156,19 @@ union hv_pfn_range { /* HV_SPA_PAGE_RANGE */
> } __packed;
> };
>
> +enum hv_sleep_state {
> + HV_SLEEP_STATE_S1 = 1,
> + HV_SLEEP_STATE_S2 = 2,
> + HV_SLEEP_STATE_S3 = 3,
> + HV_SLEEP_STATE_S4 = 4,
> + HV_SLEEP_STATE_S5 = 5,
> + /*
> + * After hypervisor has received this, any follow up sleep
> + * state registration requests will be rejected.
> + */
> + HV_SLEEP_STATE_LOCK = 6
> +};
> +
> enum hv_dynamic_processor_feature_property {
> /* Add more values when needed */
> HV_X64_DYNAMIC_PROCESSOR_FEATURE_MAX_ENCRYPTED_PARTITIONS =
> 13,
> @@ -184,6 +198,32 @@ struct hv_output_get_system_property {
> };
> } __packed;
>
> +struct hv_sleep_state_info {
> + u32 sleep_state; /* enum hv_sleep_state */
> + u8 pm1a_slp_typ;
> + u8 pm1b_slp_typ;
> +} __packed;
> +
> +struct hv_input_set_system_property {
> + u32 property_id; /* enum hv_system_property */
> + u32 reserved;
> + union {
> + /* More fields to be filled in when needed */
> + struct hv_sleep_state_info set_sleep_state_info;
> +
> + /*
> + * Add a reserved field to ensure the union is 8-byte aligned as
> + * existing members may not be. This is a temporary measure
> + * until all remaining members are added.
> + */
> + u64 reserved0[8];
I had expected a single u64 to pad out to 64-bit alignment. This is 512 bytes.
> + };
> +} __packed;
> +
> +struct hv_input_enter_sleep_state { /* HV_INPUT_ENTER_SLEEP_STATE */
> + u32 sleep_state; /* enum hv_sleep_state */
> +} __packed;
> +
> struct hv_input_map_stats_page {
> u32 type; /* enum hv_stats_object_type */
> u32 padding;
> --
> 2.51.0
>
^ permalink raw reply
* Re: [PATCH net-next v10 04/11] vsock: add netns support to virtio transports
From: Stefano Garzarella @ 2025-11-18 18:11 UTC (permalink / raw)
To: Bobby Eshleman
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Eugenio Pérez, Xuan Zhuo, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, Shuah Khan, linux-kernel,
virtualization, netdev, kvm, linux-hyperv, linux-kselftest,
Sargun Dhillon, berrange, Bobby Eshleman
In-Reply-To: <20251117-vsock-vmtest-v10-4-df08f165bf3e@meta.com>
On Mon, Nov 17, 2025 at 06:00:27PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Add netns support to loopback and vhost. Keep netns disabled for
>virtio-vsock, but add necessary changes to comply with common API
>updates.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
>Changes in v10:
>- Splitting patches complicates the series with meaningless placeholder values that eventually get replaced anyway,
> so to avoid that this patch combines into one. Links
Yeah, looking at the result, this is better IMO, thanks!
> to previous patches here:
> - Link: https://lore.kernel.org/all/20251111-vsock-vmtest-v9-3-852787a37bed@meta.com/
> - Link: https://lore.kernel.org/all/20251111-vsock-vmtest-v9-6-852787a37bed@meta.com/
> - Link: https://lore.kernel.org/all/20251111-vsock-vmtest-v9-7-852787a37bed@meta.com/
>- remove placeholder values (Stefano)
>- update comment describe net/net_mode for
> virtio_transport_reset_no_sock()
>---
> drivers/vhost/vsock.c | 45 +++++++++++++++++------
> include/linux/virtio_vsock.h | 8 +++--
> net/vmw_vsock/virtio_transport.c | 10 ++++--
> net/vmw_vsock/virtio_transport_common.c | 63 ++++++++++++++++++++++++---------
> net/vmw_vsock/vsock_loopback.c | 8 +++--
> 5 files changed, 102 insertions(+), 32 deletions(-)
>
>diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>index c8319cd1c232..2846076d484f 100644
>--- a/drivers/vhost/vsock.c
>+++ b/drivers/vhost/vsock.c
>@@ -46,6 +46,11 @@ static DEFINE_READ_MOSTLY_HASHTABLE(vhost_vsock_hash, 8);
> struct vhost_vsock {
> struct vhost_dev dev;
> struct vhost_virtqueue vqs[2];
>+ struct net *net;
>+ netns_tracker ns_tracker;
>+
>+ /* The ns mode at the time vhost_vsock was created */
>+ enum vsock_net_mode net_mode;
>
> /* Link to global vhost_vsock_hash, writes use vhost_vsock_mutex */
> struct hlist_node hash;
>@@ -72,7 +77,8 @@ static bool vhost_transport_supports_local_mode(void)
> /* Callers that dereference the return value must hold vhost_vsock_mutex or the
> * RCU read lock.
> */
>-static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
>+static struct vhost_vsock *vhost_vsock_get(u32 guest_cid, struct net *net,
>+ enum vsock_net_mode mode)
> {
> struct vhost_vsock *vsock;
>
>@@ -83,9 +89,10 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
> if (other_cid == 0)
> continue;
>
>- if (other_cid == guest_cid)
>+ if (other_cid == guest_cid &&
>+ vsock_net_check_mode(net, mode, vsock->net,
>+ vsock->net_mode))
> return vsock;
>-
> }
>
> return NULL;
>@@ -274,7 +281,8 @@ static void vhost_transport_send_pkt_work(struct vhost_work *work)
> }
>
> static int
>-vhost_transport_send_pkt(struct sk_buff *skb)
>+vhost_transport_send_pkt(struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode)
> {
> struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> struct vhost_vsock *vsock;
>@@ -283,7 +291,7 @@ vhost_transport_send_pkt(struct sk_buff *skb)
> rcu_read_lock();
>
> /* Find the vhost_vsock according to guest context id */
>- vsock = vhost_vsock_get(le64_to_cpu(hdr->dst_cid));
>+ vsock = vhost_vsock_get(le64_to_cpu(hdr->dst_cid), net, net_mode);
> if (!vsock) {
> rcu_read_unlock();
> kfree_skb(skb);
>@@ -310,7 +318,8 @@ vhost_transport_cancel_pkt(struct vsock_sock *vsk)
> rcu_read_lock();
>
> /* Find the vhost_vsock according to guest context id */
>- vsock = vhost_vsock_get(vsk->remote_addr.svm_cid);
>+ vsock = vhost_vsock_get(vsk->remote_addr.svm_cid,
>+ sock_net(sk_vsock(vsk)), vsk->net_mode);
> if (!vsock)
> goto out;
>
>@@ -470,11 +479,12 @@ static struct virtio_transport vhost_transport = {
> static bool
> vhost_transport_seqpacket_allow(struct vsock_sock *vsk, u32 remote_cid)
> {
>+ struct net *net = sock_net(sk_vsock(vsk));
> struct vhost_vsock *vsock;
> bool seqpacket_allow = false;
>
> rcu_read_lock();
>- vsock = vhost_vsock_get(remote_cid);
>+ vsock = vhost_vsock_get(remote_cid, net, vsk->net_mode);
>
> if (vsock)
> seqpacket_allow = vsock->seqpacket_allow;
>@@ -545,7 +555,8 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work)
> if (le64_to_cpu(hdr->src_cid) == vsock->guest_cid &&
> le64_to_cpu(hdr->dst_cid) ==
> vhost_transport_get_local_cid())
>- virtio_transport_recv_pkt(&vhost_transport, skb);
>+ virtio_transport_recv_pkt(&vhost_transport, skb,
>+ vsock->net, vsock->net_mode);
> else
> kfree_skb(skb);
>
>@@ -662,6 +673,7 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
> {
> struct vhost_virtqueue **vqs;
> struct vhost_vsock *vsock;
>+ struct net *net;
> int ret;
>
> /* This struct is large and allocation could fail, fall back to vmalloc
>@@ -677,6 +689,17 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
> goto out;
> }
>
>+ net = current->nsproxy->net_ns;
>+ vsock->net = get_net_track(net, &vsock->ns_tracker, GFP_KERNEL);
>+
>+ /* Store the mode of the namespace at the time of creation. If this
>+ * namespace later changes from "global" to "local", we want this vsock
>+ * to continue operating normally and not suddenly break. For that
>+ * reason, we save the mode here and later use it when performing
>+ * socket lookups with vsock_net_check_mode() (see vhost_vsock_get()).
>+ */
>+ vsock->net_mode = vsock_net_mode(net);
>+
> vsock->guest_cid = 0; /* no CID assigned yet */
> vsock->seqpacket_allow = false;
>
>@@ -716,7 +739,8 @@ static void vhost_vsock_reset_orphans(struct sock *sk)
> */
>
> /* If the peer is still valid, no need to reset connection */
>- if (vhost_vsock_get(vsk->remote_addr.svm_cid))
>+ if (vhost_vsock_get(vsk->remote_addr.svm_cid, sock_net(sk),
>+ vsk->net_mode))
> return;
>
> /* If the close timeout is pending, let it expire. This avoids races
>@@ -761,6 +785,7 @@ static int vhost_vsock_dev_release(struct inode *inode, struct file *file)
> virtio_vsock_skb_queue_purge(&vsock->send_pkt_queue);
>
> vhost_dev_cleanup(&vsock->dev);
>+ put_net_track(vsock->net, &vsock->ns_tracker);
> kfree(vsock->dev.vqs);
> vhost_vsock_free(vsock);
> return 0;
>@@ -787,7 +812,7 @@ static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 guest_cid)
>
> /* Refuse if CID is already in use */
> mutex_lock(&vhost_vsock_mutex);
>- other = vhost_vsock_get(guest_cid);
>+ other = vhost_vsock_get(guest_cid, vsock->net, vsock->net_mode);
> if (other && other != vsock) {
> mutex_unlock(&vhost_vsock_mutex);
> return -EADDRINUSE;
>diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>index 0c67543a45c8..5ed6136a4ed4 100644
>--- a/include/linux/virtio_vsock.h
>+++ b/include/linux/virtio_vsock.h
>@@ -173,6 +173,8 @@ struct virtio_vsock_pkt_info {
> u32 remote_cid, remote_port;
> struct vsock_sock *vsk;
> struct msghdr *msg;
>+ struct net *net;
>+ enum vsock_net_mode net_mode;
> u32 pkt_len;
> u16 type;
> u16 op;
>@@ -185,7 +187,8 @@ struct virtio_transport {
> struct vsock_transport transport;
>
> /* Takes ownership of the packet */
>- int (*send_pkt)(struct sk_buff *skb);
>+ int (*send_pkt)(struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode);
>
> /* Used in MSG_ZEROCOPY mode. Checks, that provided data
> * (number of buffers) could be transmitted with zerocopy
>@@ -280,7 +283,8 @@ virtio_transport_dgram_enqueue(struct vsock_sock *vsk,
> void virtio_transport_destruct(struct vsock_sock *vsk);
>
> void virtio_transport_recv_pkt(struct virtio_transport *t,
>- struct sk_buff *skb);
>+ struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode);
> void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff *skb);
> u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 wanted);
> void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit);
>diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
>index e585cb66c6f5..bc266bdb7faa 100644
>--- a/net/vmw_vsock/virtio_transport.c
>+++ b/net/vmw_vsock/virtio_transport.c
>@@ -243,7 +243,8 @@ static int virtio_transport_send_skb_fast_path(struct virtio_vsock *vsock, struc
> }
>
> static int
>-virtio_transport_send_pkt(struct sk_buff *skb)
>+virtio_transport_send_pkt(struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode)
> {
> struct virtio_vsock_hdr *hdr;
> struct virtio_vsock *vsock;
>@@ -675,7 +676,12 @@ static void virtio_transport_rx_work(struct work_struct *work)
> virtio_vsock_skb_put(skb, payload_len);
>
> virtio_transport_deliver_tap_pkt(skb);
>- virtio_transport_recv_pkt(&virtio_transport, skb);
>+
>+ /* Force virtio-transport into global mode since it
>+ * does not yet support local-mode namespacing.
>+ */
>+ virtio_transport_recv_pkt(&virtio_transport, skb,
>+ NULL, VSOCK_NET_MODE_GLOBAL);
> }
> } while (!virtqueue_enable_cb(vq));
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index dcc8a1d5851e..168e7517a3f0 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -413,7 +413,7 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>
> virtio_transport_inc_tx_pkt(vvs, skb);
>
>- ret = t_ops->send_pkt(skb);
>+ ret = t_ops->send_pkt(skb, info->net, info->net_mode);
> if (ret < 0)
> break;
>
>@@ -527,6 +527,8 @@ static int virtio_transport_send_credit_update(struct vsock_sock *vsk)
> struct virtio_vsock_pkt_info info = {
> .op = VIRTIO_VSOCK_OP_CREDIT_UPDATE,
> .vsk = vsk,
>+ .net = sock_net(sk_vsock(vsk)),
>+ .net_mode = vsk->net_mode,
> };
>
> return virtio_transport_send_pkt_info(vsk, &info);
>@@ -1067,6 +1069,8 @@ int virtio_transport_connect(struct vsock_sock *vsk)
> struct virtio_vsock_pkt_info info = {
> .op = VIRTIO_VSOCK_OP_REQUEST,
> .vsk = vsk,
>+ .net = sock_net(sk_vsock(vsk)),
>+ .net_mode = vsk->net_mode,
> };
>
> return virtio_transport_send_pkt_info(vsk, &info);
>@@ -1082,6 +1086,8 @@ int virtio_transport_shutdown(struct vsock_sock *vsk, int mode)
> (mode & SEND_SHUTDOWN ?
> VIRTIO_VSOCK_SHUTDOWN_SEND : 0),
> .vsk = vsk,
>+ .net = sock_net(sk_vsock(vsk)),
>+ .net_mode = vsk->net_mode,
> };
>
> return virtio_transport_send_pkt_info(vsk, &info);
>@@ -1108,6 +1114,8 @@ virtio_transport_stream_enqueue(struct vsock_sock *vsk,
> .msg = msg,
> .pkt_len = len,
> .vsk = vsk,
>+ .net = sock_net(sk_vsock(vsk)),
>+ .net_mode = vsk->net_mode,
> };
>
> return virtio_transport_send_pkt_info(vsk, &info);
>@@ -1145,6 +1153,8 @@ static int virtio_transport_reset(struct vsock_sock *vsk,
> .op = VIRTIO_VSOCK_OP_RST,
> .reply = !!skb,
> .vsk = vsk,
>+ .net = sock_net(sk_vsock(vsk)),
>+ .net_mode = vsk->net_mode,
> };
>
> /* Send RST only if the original pkt is not a RST pkt */
>@@ -1156,15 +1166,27 @@ static int virtio_transport_reset(struct vsock_sock *vsk,
>
> /* Normally packets are associated with a socket. There may be no socket if an
> * attempt was made to connect to a socket that does not exist.
>+ *
>+ * net and net_mode refer to the namespace of whoever sent the invalid message.
>+ * For loopback, this is the namespace of the socket. For vhost, this is the
>+ * namespace of the VM (i.e., vhost_vsock).
> */
> static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
>- struct sk_buff *skb)
>+ struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode)
> {
> struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> struct virtio_vsock_pkt_info info = {
> .op = VIRTIO_VSOCK_OP_RST,
> .type = le16_to_cpu(hdr->type),
> .reply = true,
>+
>+ /* net or net_mode are not defined here because we pass
>+ * net and net_mode directly to t->send_pkt(), instead of
>+ * relying on virtio_transport_send_pkt_info() to pass them to
>+ * t->send_pkt(). They are not needed by
>+ * virtio_transport_alloc_skb().
>+ */
> };
> struct sk_buff *reply;
>
>@@ -1183,7 +1205,7 @@ static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
> if (!reply)
> return -ENOMEM;
>
>- return t->send_pkt(reply);
>+ return t->send_pkt(reply, net, net_mode);
> }
>
> /* This function should be called with sk_lock held and SOCK_DONE set */
>@@ -1465,6 +1487,8 @@ virtio_transport_send_response(struct vsock_sock *vsk,
> .remote_port = le32_to_cpu(hdr->src_port),
> .reply = true,
> .vsk = vsk,
>+ .net = sock_net(sk_vsock(vsk)),
>+ .net_mode = vsk->net_mode,
> };
>
> return virtio_transport_send_pkt_info(vsk, &info);
>@@ -1507,12 +1531,14 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> int ret;
>
> if (le16_to_cpu(hdr->op) != VIRTIO_VSOCK_OP_REQUEST) {
>- virtio_transport_reset_no_sock(t, skb);
>+ virtio_transport_reset_no_sock(t, skb, sock_net(sk),
>+ vsk->net_mode);
> return -EINVAL;
> }
>
> if (sk_acceptq_is_full(sk)) {
>- virtio_transport_reset_no_sock(t, skb);
>+ virtio_transport_reset_no_sock(t, skb, sock_net(sk),
>+ vsk->net_mode);
> return -ENOMEM;
> }
>
>@@ -1520,13 +1546,15 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> * Subsequent enqueues would lead to a memory leak.
> */
> if (sk->sk_shutdown == SHUTDOWN_MASK) {
>- virtio_transport_reset_no_sock(t, skb);
>+ virtio_transport_reset_no_sock(t, skb, sock_net(sk),
>+ vsk->net_mode);
> return -ESHUTDOWN;
> }
>
> child = vsock_create_connected(sk);
> if (!child) {
>- virtio_transport_reset_no_sock(t, skb);
>+ virtio_transport_reset_no_sock(t, skb, sock_net(sk),
>+ vsk->net_mode);
> return -ENOMEM;
> }
>
>@@ -1548,7 +1576,8 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> */
> if (ret || vchild->transport != &t->transport) {
> release_sock(child);
>- virtio_transport_reset_no_sock(t, skb);
>+ virtio_transport_reset_no_sock(t, skb, sock_net(sk),
>+ vsk->net_mode);
> sock_put(child);
> return ret;
> }
>@@ -1576,7 +1605,8 @@ static bool virtio_transport_valid_type(u16 type)
> * lock.
> */
> void virtio_transport_recv_pkt(struct virtio_transport *t,
>- struct sk_buff *skb)
>+ struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode)
> {
> struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> struct sockaddr_vm src, dst;
>@@ -1599,24 +1629,25 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> le32_to_cpu(hdr->fwd_cnt));
>
> if (!virtio_transport_valid_type(le16_to_cpu(hdr->type))) {
>- (void)virtio_transport_reset_no_sock(t, skb);
>+ (void)virtio_transport_reset_no_sock(t, skb, net, net_mode);
> goto free_pkt;
> }
>
> /* The socket must be in connected or bound table
> * otherwise send reset back
> */
>- sk = vsock_find_connected_socket(&src, &dst);
>+ sk = vsock_find_connected_socket_net(&src, &dst, net, net_mode);
> if (!sk) {
>- sk = vsock_find_bound_socket(&dst);
>+ sk = vsock_find_bound_socket_net(&dst, net, net_mode);
> if (!sk) {
>- (void)virtio_transport_reset_no_sock(t, skb);
>+ (void)virtio_transport_reset_no_sock(t, skb, net,
>+ net_mode);
> goto free_pkt;
> }
> }
>
> if (virtio_transport_get_type(sk) != le16_to_cpu(hdr->type)) {
>- (void)virtio_transport_reset_no_sock(t, skb);
>+ (void)virtio_transport_reset_no_sock(t, skb, net, net_mode);
> sock_put(sk);
> goto free_pkt;
> }
>@@ -1635,7 +1666,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> */
> if (sock_flag(sk, SOCK_DONE) ||
> (sk->sk_state != TCP_LISTEN && vsk->transport != &t->transport)) {
>- (void)virtio_transport_reset_no_sock(t, skb);
>+ (void)virtio_transport_reset_no_sock(t, skb, net, net_mode);
> release_sock(sk);
> sock_put(sk);
> goto free_pkt;
>@@ -1667,7 +1698,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> kfree_skb(skb);
> break;
> default:
>- (void)virtio_transport_reset_no_sock(t, skb);
>+ (void)virtio_transport_reset_no_sock(t, skb, net, net_mode);
> kfree_skb(skb);
> break;
> }
>diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
>index 1e25c1a6b43f..a730fa74d2d9 100644
>--- a/net/vmw_vsock/vsock_loopback.c
>+++ b/net/vmw_vsock/vsock_loopback.c
>@@ -31,7 +31,8 @@ static bool vsock_loopback_supports_local_mode(void)
> return true;
> }
>
>-static int vsock_loopback_send_pkt(struct sk_buff *skb)
>+static int vsock_loopback_send_pkt(struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode)
> {
> struct vsock_loopback *vsock = &the_vsock_loopback;
> int len = skb->len;
>@@ -138,7 +139,10 @@ static void vsock_loopback_work(struct work_struct *work)
> */
> virtio_transport_consume_skb_sent(skb, false);
> virtio_transport_deliver_tap_pkt(skb);
>- virtio_transport_recv_pkt(&loopback_transport, skb);
>+
>+ virtio_transport_recv_pkt(&loopback_transport, skb,
>+ sock_net(skb->sk),
This will crash without next patch right? (I'll comment also there, but
I think the order of patches here is important).
That said, should we add a WARN_ONCE/WARN_ON_ONCE here with a check on
skb->sk ?
The rest LTGM,
Stefano
>+ vsock_sk(skb->sk)->net_mode);
> }
> }
>
>
>--
>2.47.3
>
^ permalink raw reply
* Re: [PATCH net-next v10 05/11] virtio: set skb owner of virtio_transport_reset_no_sock() reply
From: Stefano Garzarella @ 2025-11-18 18:12 UTC (permalink / raw)
To: Bobby Eshleman
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Eugenio Pérez, Xuan Zhuo, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, Shuah Khan, linux-kernel,
virtualization, netdev, kvm, linux-hyperv, linux-kselftest,
Sargun Dhillon, berrange, Bobby Eshleman
In-Reply-To: <20251117-vsock-vmtest-v10-5-df08f165bf3e@meta.com>
On Mon, Nov 17, 2025 at 06:00:28PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Associate reply packets with the sending socket. When vsock must reply
>with an RST packet and there exists a sending socket (e.g., for
>loopback), setting the skb owner to the socket correctly handles
>reference counting between the skb and sk (i.e., the sk stays alive
>until the skb is freed).
>
>This allows the net namespace to be used for socket lookups for the
>duration of the reply skb's lifetime, preventing race conditions between
>the namespace lifecycle and vsock socket search using the namespace
>pointer.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
>Changes in v10:
>- break this out into its own patch for easy revert (Stefano)
>---
> net/vmw_vsock/virtio_transport_common.c | 6 ++++++
> 1 file changed, 6 insertions(+)
IIUC the previous patch only works well whit this one applied, right?
Please pay more attention to the order; we never want to break the
bisection.
Thanks,
Stefano
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index 168e7517a3f0..5bb498caa19e 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -1181,6 +1181,12 @@ static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
> .type = le16_to_cpu(hdr->type),
> .reply = true,
>
>+ /* Set sk owner to socket we are replying to (may be NULL for
>+ * non-loopback). This keeps a reference to the sock and
>+ * sock_net(sk) until the reply skb is freed.
>+ */
>+ .vsk = vsock_sk(skb->sk),
>+
> /* net or net_mode are not defined here because we pass
> * net and net_mode directly to t->send_pkt(), instead of
> * relying on virtio_transport_send_pkt_info() to pass them to
>
>--
>2.47.3
>
^ permalink raw reply
* Re: [PATCH net-next v10 06/11] selftests/vsock: add namespace helpers to vmtest.sh
From: Stefano Garzarella @ 2025-11-18 18:12 UTC (permalink / raw)
To: Bobby Eshleman
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Eugenio Pérez, Xuan Zhuo, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, Shuah Khan, linux-kernel,
virtualization, netdev, kvm, linux-hyperv, linux-kselftest,
Sargun Dhillon, berrange, Bobby Eshleman
In-Reply-To: <20251117-vsock-vmtest-v10-6-df08f165bf3e@meta.com>
On Mon, Nov 17, 2025 at 06:00:29PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Add functions for initializing namespaces with the different vsock NS
>modes. Callers can use add_namespaces() and del_namespaces() to create
>namespaces global0, global1, local0, and local1.
>
>The init_namespaces() function initializes global0, local0, etc... with
>their respective vsock NS mode. This function is separate so that tests
>that depend on this initialization can use it, while other tests that
>want to test the initialization interface itself can start with a clean
>slate by omitting this call.
>
>Remove namespaces upon exiting the program in cleanup(). This is
>unlikely to be needed for a healthy run, but it is useful for tests that
>are manually killed mid-test. In that case, this patch prevents the
>subsequent test run from finding stale namespaces with
>already-write-once-locked vsock ns modes.
>
>This patch is in preparation for later namespace tests.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
> tools/testing/selftests/vsock/vmtest.sh | 41 +++++++++++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>
>diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
>index c7b270dd77a9..f78cc574c274 100755
>--- a/tools/testing/selftests/vsock/vmtest.sh
>+++ b/tools/testing/selftests/vsock/vmtest.sh
>@@ -49,6 +49,7 @@ readonly TEST_DESCS=(
> )
>
> readonly USE_SHARED_VM=(vm_server_host_client vm_client_host_server vm_loopback)
>+readonly NS_MODES=("local" "global")
>
> VERBOSE=0
>
>@@ -103,6 +104,45 @@ check_result() {
> fi
> }
>
>+add_namespaces() {
>+ # add namespaces local0, local1, global0, and global1
>+ for mode in "${NS_MODES[@]}"; do
>+ ip netns add "${mode}0" 2>/dev/null
>+ ip netns add "${mode}1" 2>/dev/null
>+ done
>+}
>+
>+init_namespaces() {
>+ for mode in "${NS_MODES[@]}"; do
>+ ns_set_mode "${mode}0" "${mode}"
>+ ns_set_mode "${mode}1" "${mode}"
>+
>+ log_host "set ns ${mode}0 to mode ${mode}"
>+ log_host "set ns ${mode}1 to mode ${mode}"
>+
>+ # we need lo for qemu port forwarding
>+ ip netns exec "${mode}0" ip link set dev lo up
>+ ip netns exec "${mode}1" ip link set dev lo up
>+ done
>+}
>+
>+del_namespaces() {
>+ for mode in "${NS_MODES[@]}"; do
>+ ip netns del "${mode}0" &>/dev/null
>+ ip netns del "${mode}1" &>/dev/null
>+ log_host "removed ns ${mode}0"
>+ log_host "removed ns ${mode}1"
>+ done
>+}
>+
>+ns_set_mode() {
>+ local ns=$1
>+ local mode=$2
>+
>+ echo "${mode}" | ip netns exec "${ns}" \
>+ tee /proc/sys/net/vsock/ns_mode &>/dev/null
>+}
>+
> vm_ssh() {
> ssh -q -o UserKnownHostsFile=/dev/null -p ${SSH_HOST_PORT} localhost "$@"
> return $?
>@@ -110,6 +150,7 @@ vm_ssh() {
>
> cleanup() {
> terminate_pidfiles "${!PIDFILES[@]}"
>+ del_namespaces
> }
>
> check_args() {
>
>--
>2.47.3
>
^ permalink raw reply
* Re: [PATCH net-next v10 08/11] selftests/vsock: add tests for proc sys vsock ns_mode
From: Stefano Garzarella @ 2025-11-18 18:13 UTC (permalink / raw)
To: Bobby Eshleman
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Eugenio Pérez, Xuan Zhuo, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, Shuah Khan, linux-kernel,
virtualization, netdev, kvm, linux-hyperv, linux-kselftest,
Sargun Dhillon, berrange, Bobby Eshleman
In-Reply-To: <20251117-vsock-vmtest-v10-8-df08f165bf3e@meta.com>
On Mon, Nov 17, 2025 at 06:00:31PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Add tests for the /proc/sys/net/vsock/ns_mode interface. Namely,
>that it accepts "global" and "local" strings and enforces a write-once
>policy.
>
>Start a convention of commenting the test name over the test
>description. Add test name comments over test descriptions that existed
>before this convention.
>
>Add a check_netns() function that checks if the test requires namespaces
>and if the current kernel supports namespaces. Skip tests that require
>namespaces if the system does not have namespace support.
>
>Add a test to verify that guest VMs with an active G2H transport
>(virtio-vsock) cannot set namespace mode to 'local'. This validates
>the mutual exclusion between G2H transports and LOCAL mode.
>
>This patch is the first to add tests that do *not* re-use the same
>shared VM. For that reason, it adds a run_tests() function to run these
>tests and filter out the shared VM tests.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
>Changes in v10:
>- Remove extraneous add_namespaces/del_namespaces calls.
>- Rename run_tests() to run_ns_tests() since it is designed to only
> run ns tests.
>
>Changes in v9:
>- add test ns_vm_local_mode_rejected to check that guests cannot use
> local mode
>---
> tools/testing/selftests/vsock/vmtest.sh | 140 +++++++++++++++++++++++++++++++-
> 1 file changed, 138 insertions(+), 2 deletions(-)
>
>diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
>index 1a7c810f282f..86483249f490 100755
>--- a/tools/testing/selftests/vsock/vmtest.sh
>+++ b/tools/testing/selftests/vsock/vmtest.sh
>@@ -41,14 +41,40 @@ readonly KERNEL_CMDLINE="\
> virtme.ssh virtme_ssh_channel=tcp virtme_ssh_user=$USER \
> "
> readonly LOG=$(mktemp /tmp/vsock_vmtest_XXXX.log)
>-readonly TEST_NAMES=(vm_server_host_client vm_client_host_server vm_loopback)
>+readonly TEST_NAMES=(
>+ vm_server_host_client
>+ vm_client_host_server
>+ vm_loopback
>+ ns_host_vsock_ns_mode_ok
>+ ns_host_vsock_ns_mode_write_once_ok
>+ ns_vm_local_mode_rejected
>+)
> readonly TEST_DESCS=(
>+ # vm_server_host_client
> "Run vsock_test in server mode on the VM and in client mode on the host."
>+
>+ # vm_client_host_server
> "Run vsock_test in client mode on the VM and in server mode on the host."
>+
>+ # vm_loopback
> "Run vsock_test using the loopback transport in the VM."
>+
>+ # ns_host_vsock_ns_mode_ok
>+ "Check /proc/sys/net/vsock/ns_mode strings on the host."
>+
>+ # ns_host_vsock_ns_mode_write_once_ok
>+ "Check /proc/sys/net/vsock/ns_mode is write-once on the host."
>+
>+ # ns_vm_local_mode_rejected
>+ "Test that guest VM with G2H transport cannot set namespace mode to 'local'"
> )
>
>-readonly USE_SHARED_VM=(vm_server_host_client vm_client_host_server vm_loopback)
>+readonly USE_SHARED_VM=(
>+ vm_server_host_client
>+ vm_client_host_server
>+ vm_loopback
>+ ns_vm_local_mode_rejected
>+)
> readonly NS_MODES=("local" "global")
>
> VERBOSE=0
>@@ -205,6 +231,20 @@ check_deps() {
> fi
> }
>
>+check_netns() {
>+ local tname=$1
>+
>+ # If the test requires NS support, check if NS support exists
>+ # using /proc/self/ns
>+ if [[ "${tname}" =~ ^ns_ ]] &&
If check_netns() is based on the test name, IMO we should document that
on top of TEST_NAMES.
Thanks,
Stefano
>+ [[ ! -e /proc/self/ns ]]; then
>+ log_host "No NS support detected for test ${tname}"
>+ return 1
>+ fi
>+
>+ return 0
>+}
>+
> check_vng() {
> local tested_versions
> local version
>@@ -503,6 +543,32 @@ log_guest() {
> LOG_PREFIX=guest log "$@"
> }
>
>+test_ns_host_vsock_ns_mode_ok() {
>+ for mode in "${NS_MODES[@]}"; do
>+ if ! ns_set_mode "${mode}0" "${mode}"; then
>+ return "${KSFT_FAIL}"
>+ fi
>+ done
>+
>+ return "${KSFT_PASS}"
>+}
>+
>+test_ns_host_vsock_ns_mode_write_once_ok() {
>+ for mode in "${NS_MODES[@]}"; do
>+ local ns="${mode}0"
>+ if ! ns_set_mode "${ns}" "${mode}"; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ # try writing again and expect failure
>+ if ns_set_mode "${ns}" "${mode}"; then
>+ return "${KSFT_FAIL}"
>+ fi
>+ done
>+
>+ return "${KSFT_PASS}"
>+}
>+
> test_vm_server_host_client() {
> if ! vm_vsock_test "init_ns" "server" 2 "${TEST_GUEST_PORT}"; then
> return "${KSFT_FAIL}"
>@@ -544,6 +610,26 @@ test_vm_loopback() {
> return "${KSFT_PASS}"
> }
>
>+test_ns_vm_local_mode_rejected() {
>+ # Guest VMs have a G2H transport (virtio-vsock) active, so they
>+ # should not be able to set namespace mode to 'local'.
>+ # This test verifies that the sysctl write fails as expected.
>+
>+ # Try to set local mode in the guest's init_ns
>+ if vm_ssh init_ns "echo local | tee /proc/sys/net/vsock/ns_mode &>/dev/null"; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ # Verify mode is still 'global'
>+ local mode
>+ mode=$(vm_ssh init_ns "cat /proc/sys/net/vsock/ns_mode")
>+ if [[ "${mode}" != "global" ]]; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ return "${KSFT_PASS}"
>+}
>+
> shared_vm_test() {
> local tname
>
>@@ -576,6 +662,11 @@ run_shared_vm_tests() {
> continue
> fi
>
>+ if ! check_netns "${arg}"; then
>+ check_result "${KSFT_SKIP}" "${arg}"
>+ continue
>+ fi
>+
> run_shared_vm_test "${arg}"
> check_result "$?" "${arg}"
> done
>@@ -629,6 +720,49 @@ run_shared_vm_test() {
> return "${rc}"
> }
>
>+run_ns_tests() {
>+ for arg in "${ARGS[@]}"; do
>+ if shared_vm_test "${arg}"; then
>+ continue
>+ fi
>+
>+ if ! check_netns "${arg}"; then
>+ check_result "${KSFT_SKIP}" "${arg}"
>+ continue
>+ fi
>+
>+ add_namespaces
>+
>+ name=$(echo "${arg}" | awk '{ print $1 }')
>+ log_host "Executing test_${name}"
>+
>+ host_oops_before=$(dmesg 2>/dev/null | grep -c -i 'Oops')
>+ host_warn_before=$(dmesg --level=warn 2>/dev/null | grep -c -i 'vsock')
>+ eval test_"${name}"
>+ rc=$?
>+
>+ host_oops_after=$(dmesg 2>/dev/null | grep -c -i 'Oops')
>+ if [[ "${host_oops_after}" -gt "${host_oops_before}" ]]; then
>+ echo "FAIL: kernel oops detected on host" | log_host
>+ check_result "${KSFT_FAIL}" "${name}"
>+ del_namespaces
>+ continue
>+ fi
>+
>+ host_warn_after=$(dmesg --level=warn 2>/dev/null | grep -c -i 'vsock')
>+ if [[ "${host_warn_after}" -gt "${host_warn_before}" ]]; then
>+ echo "FAIL: kernel warning detected on host" | log_host
>+ check_result "${KSFT_FAIL}" "${name}"
>+ del_namespaces
>+ continue
>+ fi
>+
>+ check_result "${rc}" "${name}"
>+
>+ del_namespaces
>+ done
>+}
>+
> BUILD=0
> QEMU="qemu-system-$(uname -m)"
>
>@@ -674,6 +808,8 @@ if shared_vm_tests_requested "${ARGS[@]}"; then
> terminate_pidfiles "${pidfile}"
> fi
>
>+run_ns_tests "${ARGS[@]}"
>+
> echo "SUMMARY: PASS=${cnt_pass} SKIP=${cnt_skip} FAIL=${cnt_fail}"
> echo "Log: ${LOG}"
>
>
>--
>2.47.3
>
^ permalink raw reply
* Re: [PATCH net-next v10 09/11] selftests/vsock: add namespace tests for CID collisions
From: Stefano Garzarella @ 2025-11-18 18:14 UTC (permalink / raw)
To: Bobby Eshleman
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Eugenio Pérez, Xuan Zhuo, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, Shuah Khan, linux-kernel,
virtualization, netdev, kvm, linux-hyperv, linux-kselftest,
Sargun Dhillon, berrange, Bobby Eshleman
In-Reply-To: <20251117-vsock-vmtest-v10-9-df08f165bf3e@meta.com>
On Mon, Nov 17, 2025 at 06:00:32PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Add tests to verify CID collision rules across different vsock namespace
>modes.
>
>1. Two VMs with the same CID cannot start in different global namespaces
> (ns_global_same_cid_fails)
>2. Two VMs with the same CID can start in different local namespaces
> (ns_local_same_cid_ok)
>3. VMs with the same CID can coexist when one is in a global namespace
> and another is in a local namespace (ns_global_local_same_cid_ok and
> ns_local_global_same_cid_ok)
>
>The tests ns_global_local_same_cid_ok and ns_local_global_same_cid_ok
>make sure that ordering does not matter.
>
>The tests use a shared helper function namespaces_can_boot_same_cid()
>that attempts to start two VMs with identical CIDs in the specified
>namespaces and verifies whether VM initialization failed or succeeded.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
> tools/testing/selftests/vsock/vmtest.sh | 73 +++++++++++++++++++++++++++++++++
> 1 file changed, 73 insertions(+)
>
>diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
>index 86483249f490..a8bf78a5075d 100755
>--- a/tools/testing/selftests/vsock/vmtest.sh
>+++ b/tools/testing/selftests/vsock/vmtest.sh
>@@ -48,6 +48,10 @@ readonly TEST_NAMES=(
> ns_host_vsock_ns_mode_ok
> ns_host_vsock_ns_mode_write_once_ok
> ns_vm_local_mode_rejected
>+ ns_global_same_cid_fails
>+ ns_local_same_cid_ok
>+ ns_global_local_same_cid_ok
>+ ns_local_global_same_cid_ok
> )
> readonly TEST_DESCS=(
> # vm_server_host_client
>@@ -67,6 +71,17 @@ readonly TEST_DESCS=(
>
> # ns_vm_local_mode_rejected
> "Test that guest VM with G2H transport cannot set namespace mode to 'local'"
>+ # ns_global_same_cid_fails
>+ "Check QEMU fails to start two VMs with same CID in two different global namespaces."
>+
>+ # ns_local_same_cid_ok
>+ "Check QEMU successfully starts two VMs with same CID in two different local namespaces."
>+
>+ # ns_global_local_same_cid_ok
>+ "Check QEMU successfully starts one VM in a global ns and then another VM in a local ns with the same CID."
>+
>+ # ns_local_global_same_cid_ok
>+ "Check QEMU successfully starts one VM in a local ns and then another VM in a global ns with the same CID."
> )
>
> readonly USE_SHARED_VM=(
>@@ -553,6 +568,64 @@ test_ns_host_vsock_ns_mode_ok() {
> return "${KSFT_PASS}"
> }
>
>+namespaces_can_boot_same_cid() {
>+ local ns0=$1
>+ local ns1=$2
>+ local pidfile1 pidfile2
>+ local rc
>+
>+ pidfile1="$(create_pidfile)"
>+ vm_start "${pidfile1}" "${ns0}"
Should we check also this return value or return an AND of both?
>+
>+ pidfile2="$(create_pidfile)"
>+ vm_start "${pidfile2}" "${ns1}"
>+
>+ rc=$?
>+ terminate_pidfiles "${pidfile1}" "${pidfile2}"
>+
>+ return "${rc}"
>+}
>+
>+test_ns_global_same_cid_fails() {
>+ init_namespaces
>+
>+ if namespaces_can_boot_same_cid "global0" "global1"; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ return "${KSFT_PASS}"
>+}
>+
>+test_ns_local_global_same_cid_ok() {
>+ init_namespaces
>+
>+ if namespaces_can_boot_same_cid "local0" "global0"; then
>+ return "${KSFT_PASS}"
>+ fi
>+
>+ return "${KSFT_FAIL}"
>+}
>+
>+test_ns_global_local_same_cid_ok() {
>+ init_namespaces
>+
>+ if namespaces_can_boot_same_cid "global0" "local0"; then
>+ return "${KSFT_PASS}"
>+ fi
>+
>+ return "${KSFT_FAIL}"
>+}
>+
>+test_ns_local_same_cid_ok() {
IIUC the naming convention, should this be with _fails() suffix?
Thanks,
Stefano
>+ init_namespaces
>+
>+ if namespaces_can_boot_same_cid "local0" "local0"; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ return "${KSFT_PASS}"
>+}
>+
> test_ns_host_vsock_ns_mode_write_once_ok() {
> for mode in "${NS_MODES[@]}"; do
> local ns="${mode}0"
>
>--
>2.47.3
>
^ permalink raw reply
* Re: [PATCH net-next v10 10/11] selftests/vsock: add tests for host <-> vm connectivity with namespaces
From: Stefano Garzarella @ 2025-11-18 18:15 UTC (permalink / raw)
To: Bobby Eshleman
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Eugenio Pérez, Xuan Zhuo, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, Shuah Khan, linux-kernel,
virtualization, netdev, kvm, linux-hyperv, linux-kselftest,
Sargun Dhillon, berrange, Bobby Eshleman
In-Reply-To: <20251117-vsock-vmtest-v10-10-df08f165bf3e@meta.com>
On Mon, Nov 17, 2025 at 06:00:33PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Add tests to validate namespace correctness using vsock_test and socat.
>The vsock_test tool is used to validate expected success tests, but
>socat is used for expected failure tests. socat is used to ensure that
>connections are rejected outright instead of failing due to some other
>socket behavior (as tested in vsock_test). Additionally, socat is
>already required for tunneling TCP traffic from vsock_test. Using only
>one of the vsock_test tests like 'test_stream_client_close_client' would
>have yielded a similar result, but doing so wouldn't remove the socat
>dependency.
>
>Additionally, check for the dependency socat. socat needs special
>handling beyond just checking if it is on the path because it must be
>compiled with support for both vsock and unix. The function
>check_socat() checks that this support exists.
>
>Add more padding to test name printf strings because the tests added in
>this patch would otherwise overflow.
>
>Add vm_dmesg_start() and vm_dmesg_check() to encapsulate checking dmesg
>for oops and warnings.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
>Changes in v10:
>- add vm_dmesg_start() and vm_dmesg_check()
>
>Changes in v9:
>- consistent variable quoting
>---
> tools/testing/selftests/vsock/vmtest.sh | 558 +++++++++++++++++++++++++++++++-
> 1 file changed, 556 insertions(+), 2 deletions(-)
>
>diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
>index a8bf78a5075d..9c12c1bd1edc 100755
>--- a/tools/testing/selftests/vsock/vmtest.sh
>+++ b/tools/testing/selftests/vsock/vmtest.sh
>@@ -7,6 +7,7 @@
> # * virtme-ng
> # * busybox-static (used by virtme-ng)
> # * qemu (used by virtme-ng)
>+# * socat
> #
> # shellcheck disable=SC2317,SC2119
>
>@@ -52,6 +53,19 @@ readonly TEST_NAMES=(
> ns_local_same_cid_ok
> ns_global_local_same_cid_ok
> ns_local_global_same_cid_ok
>+ ns_diff_global_host_connect_to_global_vm_ok
>+ ns_diff_global_host_connect_to_local_vm_fails
>+ ns_diff_global_vm_connect_to_global_host_ok
>+ ns_diff_global_vm_connect_to_local_host_fails
>+ ns_diff_local_host_connect_to_local_vm_fails
>+ ns_diff_local_vm_connect_to_local_host_fails
>+ ns_diff_global_to_local_loopback_local_fails
>+ ns_diff_local_to_global_loopback_fails
>+ ns_diff_local_to_local_loopback_fails
>+ ns_diff_global_to_global_loopback_ok
>+ ns_same_local_loopback_ok
>+ ns_same_local_host_connect_to_local_vm_ok
>+ ns_same_local_vm_connect_to_local_host_ok
> )
> readonly TEST_DESCS=(
> # vm_server_host_client
>@@ -82,6 +96,45 @@ readonly TEST_DESCS=(
>
> # ns_local_global_same_cid_ok
> "Check QEMU successfully starts one VM in a local ns and then another VM in a global ns with the same CID."
>+
>+ # ns_diff_global_host_connect_to_global_vm_ok
>+ "Run vsock_test client in global ns with server in VM in another global ns."
>+
>+ # ns_diff_global_host_connect_to_local_vm_fails
>+ "Run socat to test a process in a global ns fails to connect to a VM in a local ns."
>+
>+ # ns_diff_global_vm_connect_to_global_host_ok
>+ "Run vsock_test client in VM in a global ns with server in another global ns."
>+
>+ # ns_diff_global_vm_connect_to_local_host_fails
>+ "Run socat to test a VM in a global ns fails to connect to a host process in a local ns."
>+
>+ # ns_diff_local_host_connect_to_local_vm_fails
>+ "Run socat to test a host process in a local ns fails to connect to a VM in another local ns."
>+
>+ # ns_diff_local_vm_connect_to_local_host_fails
>+ "Run socat to test a VM in a local ns fails to connect to a host process in another local ns."
>+
>+ # ns_diff_global_to_local_loopback_local_fails
>+ "Run socat to test a loopback vsock in a global ns fails to connect to a vsock in a local ns."
>+
>+ # ns_diff_local_to_global_loopback_fails
>+ "Run socat to test a loopback vsock in a local ns fails to connect to a vsock in a global ns."
>+
>+ # ns_diff_local_to_local_loopback_fails
>+ "Run socat to test a loopback vsock in a local ns fails to connect to a vsock in another local ns."
>+
>+ # ns_diff_global_to_global_loopback_ok
>+ "Run socat to test a loopback vsock in a global ns successfully connects to a vsock in another global ns."
>+
>+ # ns_same_local_loopback_ok
>+ "Run socat to test a loopback vsock in a local ns successfully connects to a vsock in the same ns."
>+
>+ # ns_same_local_host_connect_to_local_vm_ok
>+ "Run vsock_test client in a local ns with server in VM in same ns."
>+
>+ # ns_same_local_vm_connect_to_local_host_ok
>+ "Run vsock_test client in VM in a local ns with server in same ns."
> )
>
> readonly USE_SHARED_VM=(
>@@ -113,7 +166,7 @@ usage() {
> for ((i = 0; i < ${#TEST_NAMES[@]}; i++)); do
> name=${TEST_NAMES[${i}]}
> desc=${TEST_DESCS[${i}]}
>- printf "\t%-35s%-35s\n" "${name}" "${desc}"
>+ printf "\t%-55s%-35s\n" "${name}" "${desc}"
> done
> echo
>
>@@ -232,7 +285,7 @@ check_args() {
> }
>
> check_deps() {
>- for dep in vng ${QEMU} busybox pkill ssh; do
>+ for dep in vng ${QEMU} busybox pkill ssh socat; do
> if [[ ! -x $(command -v "${dep}") ]]; then
> echo -e "skip: dependency ${dep} not found!\n"
> exit "${KSFT_SKIP}"
>@@ -283,6 +336,20 @@ check_vng() {
> fi
> }
>
>+check_socat() {
>+ local support_string
>+
>+ support_string="$(socat -V)"
>+
>+ if [[ "${support_string}" != *"WITH_VSOCK 1"* ]]; then
>+ die "err: socat is missing vsock support"
>+ fi
>+
>+ if [[ "${support_string}" != *"WITH_UNIX 1"* ]]; then
>+ die "err: socat is missing unix support"
>+ fi
>+}
>+
> handle_build() {
> if [[ ! "${BUILD}" -eq 1 ]]; then
> return
>@@ -331,6 +398,14 @@ terminate_pidfiles() {
> done
> }
>
>+terminate_pids() {
>+ local pid
>+
>+ for pid in "$@"; do
>+ kill -SIGTERM "${pid}" &>/dev/null || :
>+ done
>+}
>+
> vm_start() {
> local pidfile=$1
> local ns=$2
>@@ -444,6 +519,40 @@ host_wait_for_listener() {
> fi
> }
>
>+vm_dmesg_oops_count() {
>+ local ns=$1
>+
>+ vm_ssh "${ns}" -- dmesg 2>/dev/null | grep -c -i 'Oops'
>+}
>+
>+vm_dmesg_warn_count() {
>+ local ns=$1
>+
>+ vm_ssh "${ns}" -- dmesg --level=warn 2>/dev/null | grep -c -i 'vsock'
>+}
>+
>+vm_dmesg_check() {
>+ local pidfile=$1
>+ local ns=$2
>+ local oops_before=$3
>+ local warn_before=$4
>+ local oops_after warn_after
>+
>+ oops_after=$(vm_ssh "${ns}" -- dmesg 2>/dev/null | grep -c -i 'Oops')
>+ if [[ "${oops_after}" -gt "${oops_before}" ]]; then
>+ echo "FAIL: kernel oops detected on vm in ns ${ns}" | log_host
>+ return 1
>+ fi
>+
>+ warn_after=$(vm_ssh "${ns}" -- dmesg --level=warn 2>/dev/null | grep -c -i 'vsock')
>+ if [[ "${warn_after}" -gt "${warn_before}" ]]; then
>+ echo "FAIL: kernel warning detected on vm in ns ${ns}" | log_host
>+ return 1
>+ fi
>+
>+ return 0
>+}
>+
> vm_vsock_test() {
> local ns=$1
> local host=$2
>@@ -568,6 +677,450 @@ test_ns_host_vsock_ns_mode_ok() {
> return "${KSFT_PASS}"
> }
>
>+test_ns_diff_global_host_connect_to_global_vm_ok() {
>+ local oops_before warn_before
>+ local pids pid pidfile
>+ local ns0 ns1 port
>+ declare -a pids
>+ local unixfile
>+ ns0="global0"
>+ ns1="global1"
>+ port=1234
>+ local rc
>+
>+ init_namespaces
>+
>+ pidfile="$(create_pidfile)"
>+
>+ if ! vm_start "${pidfile}" "${ns0}"; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ vm_wait_for_ssh "${ns0}"
>+ oops_before=$(vm_dmesg_oops_count "${ns0}")
>+ warn_before=$(vm_dmesg_warn_count "${ns0}")
>+
>+ unixfile=$(mktemp -u /tmp/XXXX.sock)
Should we remove this file at the end of this test?
>+ ip netns exec "${ns1}" \
>+ socat TCP-LISTEN:"${TEST_HOST_PORT}",fork \
>+ UNIX-CONNECT:"${unixfile}" &
>+ pids+=($!)
>+ host_wait_for_listener "${ns1}" "${TEST_HOST_PORT}"
>+
>+ ip netns exec "${ns0}" socat UNIX-LISTEN:"${unixfile}",fork \
>+ TCP-CONNECT:localhost:"${TEST_HOST_PORT}" &
>+ pids+=($!)
>+
>+ vm_vsock_test "${ns0}" "server" 2 "${TEST_GUEST_PORT}"
>+ vm_wait_for_listener "${ns0}" "${TEST_GUEST_PORT}"
>+ host_vsock_test "${ns1}" "127.0.0.1" "${VSOCK_CID}" "${TEST_HOST_PORT}"
>+ rc=$?
>+
>+ vm_dmesg_check "${pidfile}" "${ns0}" "${oops_before}" "${warn_before}"
>+ dmesg_rc=$?
>+
>+ terminate_pids "${pids[@]}"
>+ terminate_pidfiles "${pidfile}"
>+
>+ if [[ "${rc}" -ne 0 ]] || [[ "${dmesg_rc}" -ne 0 ]]; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ return "${KSFT_PASS}"
>+}
>+
>+test_ns_diff_global_host_connect_to_local_vm_fails() {
>+ local oops_before warn_before
>+ local ns0="global0"
>+ local ns1="local0"
>+ local port=12345
>+ local dmesg_rc
>+ local pidfile
>+ local result
>+ local pid
>+
>+ init_namespaces
>+
>+ outfile=$(mktemp)
>+
>+ pidfile="$(create_pidfile)"
>+ if ! vm_start "${pidfile}" "${ns1}"; then
>+ log_host "failed to start vm (cid=${VSOCK_CID}, ns=${ns0})"
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ vm_wait_for_ssh "${ns1}"
>+ oops_before=$(vm_dmesg_oops_count "${ns1}")
>+ warn_before=$(vm_dmesg_warn_count "${ns1}")
>+
>+ vm_ssh "${ns1}" -- socat VSOCK-LISTEN:"${port}" STDOUT > "${outfile}" &
Should we wait for the listener here, like we do for TCP sockets?
(also in other place where we use VSOCK-LISTEN)
>+ echo TEST | ip netns exec "${ns0}" \
>+ socat STDIN VSOCK-CONNECT:"${VSOCK_CID}":"${port}" 2>/dev/null
>+
>+ vm_dmesg_check "${pidfile}" "${ns1}" "${oops_before}" "${warn_before}"
>+ dmesg_rc=$?
>+
>+ terminate_pidfiles "${pidfile}"
>+ result=$(cat "${outfile}")
>+ rm -f "${outfile}"
>+
>+ if [[ "${result}" == "TEST" ]] || [[ "${dmesg_rc}" -ne 0 ]]; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ return "${KSFT_PASS}"
>+}
>+
>+test_ns_diff_global_vm_connect_to_global_host_ok() {
>+ local oops_before warn_before
>+ local ns0="global0"
>+ local ns1="global1"
>+ local port=12345
>+ local unixfile
>+ local dmesg_rc
>+ local pidfile
>+ local pids
>+ local rc
>+
>+ init_namespaces
>+
>+ declare -a pids
>+
>+ log_host "Setup socat bridge from ns ${ns0} to ns ${ns1} over port ${port}"
>+
>+ unixfile=$(mktemp -u /tmp/XXXX.sock)
>+
>+ ip netns exec "${ns0}" \
>+ socat TCP-LISTEN:"${port}" UNIX-CONNECT:"${unixfile}" &
>+ pids+=($!)
>+
>+ ip netns exec "${ns1}" \
>+ socat UNIX-LISTEN:"${unixfile}" TCP-CONNECT:127.0.0.1:"${port}" &
>+ pids+=($!)
>+
>+ log_host "Launching ${VSOCK_TEST} in ns ${ns1}"
>+ host_vsock_test "${ns1}" "server" "${VSOCK_CID}" "${port}"
>+
>+ pidfile="$(create_pidfile)"
>+ if ! vm_start "${pidfile}" "${ns0}"; then
>+ log_host "failed to start vm (cid=${cid}, ns=${ns0})"
>+ terminate_pids "${pids[@]}"
>+ rm -f "${unixfile}"
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ vm_wait_for_ssh "${ns0}"
>+
>+ oops_before=$(vm_dmesg_oops_count "${ns0}")
>+ warn_before=$(vm_dmesg_warn_count "${ns0}")
>+
>+ vm_vsock_test "${ns0}" "10.0.2.2" 2 "${port}"
>+ rc=$?
>+
>+ vm_dmesg_check "${pidfile}" "${ns0}" "${oops_before}" "${warn_before}"
>+ dmesg_rc=$?
>+
>+ terminate_pidfiles "${pidfile}"
>+ terminate_pids "${pids[@]}"
>+ rm -f "${unixfile}"
>+
>+ if [[ "${rc}" -ne 0 ]] || [[ "${dmesg_rc}" -ne 0 ]]; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ return "${KSFT_PASS}"
>+
>+}
>+
>+test_ns_diff_global_vm_connect_to_local_host_fails() {
>+ local ns0="global0"
>+ local ns1="local0"
>+ local port=12345
>+ local oops_before warn_before
>+ local dmesg_rc
>+ local pidfile
>+ local result
>+ local pid
>+
>+ init_namespaces
>+
>+ log_host "Launching socat in ns ${ns1}"
>+ outfile=$(mktemp)
>+ ip netns exec "${ns1}" socat VSOCK-LISTEN:"${port}" STDOUT &> "${outfile}" &
>+ pid=$!
>+
>+ pidfile="$(create_pidfile)"
>+ if ! vm_start "${pidfile}" "${ns0}"; then
>+ log_host "failed to start vm (cid=${cid}, ns=${ns0})"
>+ terminate_pids "${pid}"
>+ rm -f "${outfile}"
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ vm_wait_for_ssh "${ns0}"
>+
>+ oops_before=$(vm_dmesg_oops_count "${ns0}")
>+ warn_before=$(vm_dmesg_warn_count "${ns0}")
>+
>+ vm_ssh "${ns0}" -- \
>+ bash -c "echo TEST | socat STDIN VSOCK-CONNECT:2:${port}" 2>&1 | log_guest
>+
>+ vm_dmesg_check "${pidfile}" "${ns0}" "${oops_before}" "${warn_before}"
>+ dmesg_rc=$?
>+
>+ terminate_pidfiles "${pidfile}"
>+ terminate_pids "${pid}"
>+
>+ result=$(cat "${outfile}")
>+ rm -f "${outfile}"
>+
>+ if [[ "${result}" != TEST ]] && [[ "${dmesg_rc}" -eq 0 ]]; then
>+ return "${KSFT_PASS}"
>+ fi
>+
>+ return "${KSFT_FAIL}"
>+}
>+
>+test_ns_diff_local_host_connect_to_local_vm_fails() {
>+ local ns0="local0"
>+ local ns1="local1"
>+ local port=12345
>+ local oops_before warn_before
>+ local dmesg_rc
>+ local pidfile
>+ local result
>+ local pid
>+
>+ init_namespaces
>+
>+ outfile=$(mktemp)
>+
>+ pidfile="$(create_pidfile)"
>+ if ! vm_start "${pidfile}" "${ns1}"; then
>+ log_host "failed to start vm (cid=${cid}, ns=${ns0})"
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ vm_wait_for_ssh "${ns1}"
>+ oops_before=$(vm_dmesg_oops_count "${ns1}")
>+ warn_before=$(vm_dmesg_warn_count "${ns1}")
>+
>+ vm_ssh "${ns1}" -- socat VSOCK-LISTEN:"${port}" STDOUT > "${outfile}" &
>+ echo TEST | ip netns exec "${ns0}" \
>+ socat STDIN VSOCK-CONNECT:"${VSOCK_CID}":"${port}" 2>/dev/null
>+
>+ vm_dmesg_check "${pidfile}" "${ns1}" "${oops_before}" "${warn_before}"
>+ dmesg_rc=$?
>+
>+ terminate_pidfiles "${pidfile}"
>+
>+ result=$(cat "${outfile}")
>+ rm -f "${outfile}"
>+
>+ if [[ "${result}" != TEST ]] && [[ "${dmesg_rc}" -eq 0 ]]; then
>+ return "${KSFT_PASS}"
>+ fi
>+
>+ return "${KSFT_FAIL}"
>+}
>+
>+test_ns_diff_local_vm_connect_to_local_host_fails() {
>+ local oops_before warn_before
>+ local ns0="local0"
>+ local ns1="local1"
>+ local port=12345
>+ local dmesg_rc
>+ local pidfile
>+ local result
>+ local pid
>+
>+ init_namespaces
>+
>+ log_host "Launching socat in ns ${ns1}"
>+ outfile=$(mktemp)
>+ ip netns exec "${ns1}" socat VSOCK-LISTEN:"${port}" STDOUT &> "${outfile}" &
>+ pid=$!
>+
>+ pidfile="$(create_pidfile)"
>+ if ! vm_start "${pidfile}" "${ns0}"; then
>+ log_host "failed to start vm (cid=${cid}, ns=${ns0})"
>+ rm -f "${outfile}"
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ vm_wait_for_ssh "${ns0}"
>+ oops_before=$(vm_dmesg_oops_count "${ns0}")
>+ warn_before=$(vm_dmesg_warn_count "${ns0}")
>+
>+ vm_ssh "${ns0}" -- \
>+ bash -c "echo TEST | socat STDIN VSOCK-CONNECT:2:${port}" 2>&1 | log_guest
>+
>+ vm_dmesg_check "${pidfile}" "${ns0}" "${oops_before}" "${warn_before}"
>+ dmesg_rc=$?
>+
>+ terminate_pidfiles "${pidfile}"
>+ terminate_pids "${pid}"
>+
>+ result=$(cat "${outfile}")
>+ rm -f "${outfile}"
>+
>+ if [[ "${result}" != TEST ]] && [[ "${dmesg_rc}" -eq 0 ]]; then
>+ return "${KSFT_PASS}"
>+ fi
>+
>+ return "${KSFT_FAIL}"
>+}
>+
>+__test_loopback_two_netns() {
>+ local ns0=$1
>+ local ns1=$2
>+ local port=12345
>+ local result
>+ local pid
>+
>+ modprobe vsock_loopback &> /dev/null || :
>+
>+ log_host "Launching socat in ns ${ns1}"
>+ outfile=$(mktemp)
>+ ip netns exec "${ns1}" socat VSOCK-LISTEN:"${port}" STDOUT > "${outfile}" 2>/dev/null &
>+ pid=$!
>+
>+ log_host "Launching socat in ns ${ns0}"
>+ echo TEST | ip netns exec "${ns0}" socat STDIN VSOCK-CONNECT:1:"${port}" 2>/dev/null
>+ terminate_pids "${pid}"
>+
>+ result=$(cat "${outfile}")
>+ rm -f "${outfile}"
>+
>+ if [[ "${result}" == TEST ]]; then
>+ return 0
>+ fi
>+
>+ return 1
>+}
>+
>+test_ns_diff_global_to_local_loopback_local_fails() {
>+ init_namespaces
>+
>+ if ! __test_loopback_two_netns "global0" "local0"; then
>+ return "${KSFT_PASS}"
>+ fi
>+
>+ return "${KSFT_FAIL}"
>+}
>+
>+test_ns_diff_local_to_global_loopback_fails() {
>+ init_namespaces
>+
>+ if ! __test_loopback_two_netns "local0" "global0"; then
>+ return "${KSFT_PASS}"
>+ fi
>+
>+ return "${KSFT_FAIL}"
>+}
>+
>+test_ns_diff_local_to_local_loopback_fails() {
>+ init_namespaces
>+
>+ if ! __test_loopback_two_netns "local0" "local1"; then
>+ return "${KSFT_PASS}"
>+ fi
>+
>+ return "${KSFT_FAIL}"
>+}
>+
>+test_ns_diff_global_to_global_loopback_ok() {
>+ init_namespaces
>+
>+ if __test_loopback_two_netns "global0" "global1"; then
>+ return "${KSFT_PASS}"
>+ fi
>+
>+ return "${KSFT_FAIL}"
>+}
>+
>+test_ns_same_local_loopback_ok() {
>+ init_namespaces
>+
>+ if __test_loopback_two_netns "local0" "local0"; then
>+ return "${KSFT_PASS}"
>+ fi
>+
>+ return "${KSFT_FAIL}"
>+}
>+
>+test_ns_same_local_host_connect_to_local_vm_ok() {
>+ local oops_before warn_before
>+ local ns="local0"
>+ local port=1234
>+ local dmesg_rc
>+ local pidfile
>+ local rc
>+
>+ init_namespaces
>+
>+ pidfile="$(create_pidfile)"
>+
>+ if ! vm_start "${pidfile}" "${ns}"; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ vm_wait_for_ssh "${ns}"
>+ oops_before=$(vm_dmesg_oops_count "${ns}")
>+ warn_before=$(vm_dmesg_warn_count "${ns}")
>+
>+ vm_vsock_test "${ns}" "server" 2 "${TEST_GUEST_PORT}"
>+ host_vsock_test "${ns}" "127.0.0.1" "${VSOCK_CID}" "${TEST_HOST_PORT}"
>+ rc=$?
>+
>+ vm_dmesg_check "${pidfile}" "${ns}" "${oops_before}" "${warn_before}"
>+ dmesg_rc=$?
>+
>+ terminate_pidfiles "${pidfile}"
>+
>+ if [[ "${rc}" -ne 0 ]] || [[ "${dmesg_rc}" -ne 0 ]]; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ return "${KSFT_PASS}"
>+}
>+
>+test_ns_same_local_vm_connect_to_local_host_ok() {
I don't understand the difference between this test and the previous one
(test_ns_same_local_host_connect_to_local_vm_ok).
Maybe there is a copy/paste issue and we need to invert server/client.
Can you check?
>+ local oops_before warn_before
>+ local ns="local0"
>+ local port=1234
>+ local dmesg_rc
>+ local pidfile
>+ local rc
>+
>+ init_namespaces
>+
>+ pidfile="$(create_pidfile)"
>+
>+ if ! vm_start "${pidfile}" "${ns}"; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ vm_wait_for_ssh "${ns}"
>+ oops_before=$(vm_dmesg_oops_count "${ns}")
>+ warn_before=$(vm_dmesg_warn_count "${ns}")
>+
>+ vm_vsock_test "${ns}" "server" 2 "${TEST_GUEST_PORT}"
>+ host_vsock_test "${ns}" "127.0.0.1" "${VSOCK_CID}" "${TEST_HOST_PORT}"
>+ rc=$?
>+
>+ vm_dmesg_check "${pidfile}" "${ns}" "${oops_before}" "${warn_before}"
>+ dmesg_rc=$?
>+
>+ terminate_pidfiles "${pidfile}"
>+
>+ if [[ "${rc}" -ne 0 ]] || [[ "${dmesg_rc}" -ne 0 ]]; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ return "${KSFT_PASS}"
>+}
>+
> namespaces_can_boot_same_cid() {
> local ns0=$1
> local ns1=$2
>@@ -861,6 +1414,7 @@ fi
> check_args "${ARGS[@]}"
> check_deps
> check_vng
>+check_socat
> handle_build
>
> echo "1..${#ARGS[@]}"
>
>--
>2.47.3
>
^ permalink raw reply
* Re: [PATCH net-next v10 11/11] selftests/vsock: add tests for namespace deletion and mode changes
From: Stefano Garzarella @ 2025-11-18 18:15 UTC (permalink / raw)
To: Bobby Eshleman
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Eugenio Pérez, Xuan Zhuo, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, Shuah Khan, linux-kernel,
virtualization, netdev, kvm, linux-hyperv, linux-kselftest,
Sargun Dhillon, berrange, Bobby Eshleman
In-Reply-To: <20251117-vsock-vmtest-v10-11-df08f165bf3e@meta.com>
On Mon, Nov 17, 2025 at 06:00:34PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Add tests that validate vsock sockets are resilient to deleting
>namespaces or changing namespace modes from global to local. The vsock
>sockets should still function normally.
>
>The function check_ns_changes_dont_break_connection() is added to re-use
>the step-by-step logic of 1) setup connections, 2) do something that
>would maybe break the connections, 3) check that the connections are
>still ok.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
>Changes in v9:
>- more consistent shell style
>- clarify -u usage comment for pipefile
>---
> tools/testing/selftests/vsock/vmtest.sh | 123 ++++++++++++++++++++++++++++++++
> 1 file changed, 123 insertions(+)
>
>diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
>index 9c12c1bd1edc..2b6e94aafc19 100755
>--- a/tools/testing/selftests/vsock/vmtest.sh
>+++ b/tools/testing/selftests/vsock/vmtest.sh
>@@ -66,6 +66,12 @@ readonly TEST_NAMES=(
> ns_same_local_loopback_ok
> ns_same_local_host_connect_to_local_vm_ok
> ns_same_local_vm_connect_to_local_host_ok
>+ ns_mode_change_connection_continue_vm_ok
>+ ns_mode_change_connection_continue_host_ok
>+ ns_mode_change_connection_continue_both_ok
>+ ns_delete_vm_ok
>+ ns_delete_host_ok
>+ ns_delete_both_ok
> )
> readonly TEST_DESCS=(
> # vm_server_host_client
>@@ -135,6 +141,24 @@ readonly TEST_DESCS=(
>
> # ns_same_local_vm_connect_to_local_host_ok
> "Run vsock_test client in VM in a local ns with server in same ns."
>+
>+ # ns_mode_change_connection_continue_vm_ok
>+ "Check that changing NS mode of VM namespace from global to local after a connection is established doesn't break the connection"
>+
>+ # ns_mode_change_connection_continue_host_ok
>+ "Check that changing NS mode of host namespace from global to
>local after a connection is established doesn't break the connection"
>+
>+ # ns_mode_change_connection_continue_both_ok
>+ "Check that changing NS mode of host and VM namespaces from global to local after a connection is established doesn't break the connection"
>+
>+ # ns_delete_vm_ok
>+ "Check that deleting the VM's namespace does not break the socket connection"
>+
>+ # ns_delete_host_ok
>+ "Check that deleting the host's namespace does not break the socket connection"
>+
>+ # ns_delete_both_ok
>+ "Check that deleting the VM and host's namespaces does not break the socket connection"
> )
>
> readonly USE_SHARED_VM=(
>@@ -1256,6 +1280,105 @@ test_ns_vm_local_mode_rejected() {
> return "${KSFT_PASS}"
> }
>
>+check_ns_changes_dont_break_connection() {
>+ local pipefile pidfile outfile
>+ local ns0="global0"
>+ local ns1="global1"
>+ local port=12345
>+ local pids=()
>+ local rc=0
>+
>+ init_namespaces
>+
>+ pidfile="$(create_pidfile)"
>+ if ! vm_start "${pidfile}" "${ns0}"; then
>+ return "${KSFT_FAIL}"
>+ fi
>+ vm_wait_for_ssh "${ns0}"
>+
>+ outfile=$(mktemp)
>+ vm_ssh "${ns0}" -- \
>+ socat VSOCK-LISTEN:"${port}",fork STDOUT > "${outfile}" 2>/dev/null &
>+ pids+=($!)
>+
>+ # wait_for_listener() does not work for vsock because vsock does not
>+ # export socket state to /proc/net/. Instead, we have no choice but to
>+ # sleep for some hardcoded time.
>+ sleep "${WAIT_PERIOD}"
can we use `ss --vsock --listening` ?
>+
>+ # We use a pipe here so that we can echo into the pipe instead of using
>+ # socat and a unix socket file. We just need a name for the pipe (not a
>+ # regular file) so use -u.
>+ pipefile=$(mktemp -u /tmp/vmtest_pipe_XXXX)
Should we remove this file at the end of the test?
>+ ip netns exec "${ns1}" \
>+ socat PIPE:"${pipefile}" VSOCK-CONNECT:"${VSOCK_CID}":"${port}" &
>+ pids+=($!)
>+
>+ timeout "${WAIT_PERIOD}" \
>+ bash -c 'while [[ ! -e '"${pipefile}"' ]]; do sleep 1; done; exit 0'
>+
>+ if [[ $2 == "delete" ]]; then
>+ if [[ "$1" == "vm" ]]; then
>+ ip netns del "${ns0}"
>+ elif [[ "$1" == "host" ]]; then
>+ ip netns del "${ns1}"
>+ elif [[ "$1" == "both" ]]; then
>+ ip netns del "${ns0}"
>+ ip netns del "${ns1}"
>+ fi
>+ elif [[ $2 == "change_mode" ]]; then
>+ if [[ "$1" == "vm" ]]; then
>+ ns_set_mode "${ns0}" "local"
>+ elif [[ "$1" == "host" ]]; then
>+ ns_set_mode "${ns1}" "local"
>+ elif [[ "$1" == "both" ]]; then
>+ ns_set_mode "${ns0}" "local"
>+ ns_set_mode "${ns1}" "local"
>+ fi
>+ fi
>+
>+ echo "TEST" > "${pipefile}"
>+
>+ timeout "${WAIT_PERIOD}" \
>+ bash -c 'while [[ ! -s '"${outfile}"' ]]; do sleep 1; done; exit 0'
>+
>+ if grep -q "TEST" "${outfile}"; then
>+ rc="${KSFT_PASS}"
>+ else
>+ rc="${KSFT_FAIL}"
>+ fi
>+
>+ terminate_pidfiles "${pidfile}"
>+ terminate_pids "${pids[@]}"
>+ rm -f "${outfile}"
>+
>+ return "${rc}"
>+}
>+
>+test_ns_mode_change_connection_continue_vm_ok() {
>+ check_ns_changes_dont_break_connection "vm" "change_mode"
>+}
>+
>+test_ns_mode_change_connection_continue_host_ok() {
>+ check_ns_changes_dont_break_connection "host" "change_mode"
>+}
>+
>+test_ns_mode_change_connection_continue_both_ok() {
>+ check_ns_changes_dont_break_connection "both" "change_mode"
>+}
>+
>+test_ns_delete_vm_ok() {
>+ check_ns_changes_dont_break_connection "vm" "delete"
>+}
>+
>+test_ns_delete_host_ok() {
>+ check_ns_changes_dont_break_connection "host" "delete"
>+}
>+
>+test_ns_delete_both_ok() {
>+ check_ns_changes_dont_break_connection "both" "delete"
>+}
>+
> shared_vm_test() {
> local tname
>
>
>--
>2.47.3
>
^ permalink raw reply
* RE: [Patch net-next v2] net: mana: Handle hardware recovery events when probing the device
From: Haiyang Zhang @ 2025-11-18 21:18 UTC (permalink / raw)
To: longli@linux.microsoft.com, KY Srinivasan, Wei Liu, Dexuan Cui,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Shradha Gupta, Simon Horman, Konstantin Taranov,
Souradeep Chakrabarti, Erick Archer, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-rdma@vger.kernel.org
Cc: Long Li
In-Reply-To: <1763430724-24719-1-git-send-email-longli@linux.microsoft.com>
> -----Original Message-----
> From: longli@linux.microsoft.com <longli@linux.microsoft.com>
> Sent: Monday, November 17, 2025 8:52 PM
> To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; Wei Liu <wei.liu@kernel.org>; Dexuan Cui
> <DECUI@microsoft.com>; David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Shradha Gupta <shradhagupta@linux.microsoft.com>;
> Simon Horman <horms@kernel.org>; Konstantin Taranov
> <kotaranov@microsoft.com>; Souradeep Chakrabarti
> <schakrabarti@linux.microsoft.com>; Erick Archer
> <erick.archer@outlook.com>; linux-hyperv@vger.kernel.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> rdma@vger.kernel.org
> Cc: Long Li <longli@microsoft.com>
> Subject: [Patch net-next v2] net: mana: Handle hardware recovery events
> when probing the device
>
> From: Long Li <longli@microsoft.com>
>
> When MANA is being probed, it's possible that hardware is in recovery
> mode and the device may get GDMA_EQE_HWC_RESET_REQUEST over HWC in the
> middle of the probe. Detect such condition and go through the recovery
> service procedure.
>
> Fixes: fbe346ce9d62 ("net: mana: Handle Reset Request from MANA NIC")
> Signed-off-by: Long Li <longli@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
^ permalink raw reply
* Re: [PATCH v4] mshv: Extend create partition ioctl to support cpu features
From: Nuno Das Neves @ 2025-11-18 22:49 UTC (permalink / raw)
To: Stanislav Kinsburskii
Cc: linux-hyperv, linux-kernel, wei.liu, mhklinux, kys, haiyangz,
decui, longli, prapal, mrathor, muislam, anrayabh, Jinank Jain
In-Reply-To: <aRu2uC4VVazB_SfV@skinsburskii.localdomain>
On 11/17/2025 3:58 PM, Stanislav Kinsburskii wrote:
> On Tue, Nov 11, 2025 at 03:19:54PM -0800, Nuno Das Neves wrote:
>> From: Muminul Islam <muislam@microsoft.com>
>>
>> The existing mshv create partition ioctl does not provide a way to
>> specify which cpu features are enabled in the guest. Instead, it
>> attempts to enable all features and those that are not supported are
>> silently disabled by the hypervisor.
>>
>> This was done to reduce unnecessary complexity and is sufficient for
>> many cases. However, new scenarios require fine-grained control over
>> these features.
>>
>> Define a new mshv_create_partition_v2 structure which supports
>> passing the disabled processor and xsave feature bits through to the
>> create partition hypercall directly.
>>
>> Introduce a new flag MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES which enables
>> the new structure. If unset, the original mshv_create_partition struct
>> is used, with the old behavior of enabling all features.
>>
>> Co-developed-by: Jinank Jain <jinankjain@microsoft.com>
>> Signed-off-by: Jinank Jain <jinankjain@microsoft.com>
>> Signed-off-by: Muminul Islam <muislam@microsoft.com>
>> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
>> ---
>> Changes in v4:
>> - Change BIT() to BIT_ULL() [Michael Kelley]
>> - Enforce pt_num_cpu_fbanks == MSHV_NUM_CPU_FEATURES_BANKS and expect
>> that number to never change. In future, additional processor banks
>> will be settable as 'early' partition properties. Remove redundant
>> code that set default values for unspecified banks [Michael Kelley]
>> - Set xsave features to 0 on arm64 [Michael Kelley]
>> - Add clarifying comments in a few places
>>
>> Changes in v3:
>> - Remove the new cpu features definitions in hvhdk.h, and retain the
>> old behavior of enabling all features for the old struct. For the v2
>> struct, still disable unspecified feature banks, since that makes it
>> robust to future extensions.
>> - Amend comments and commit message to reflect the above
>> - Fix unused variable on arm64 [kernel test robot]
>>
>> Changes in v2:
>> - Fix exposure of CONFIG_X86_64 to uapi [kernel test robot]
>> - Fix compilation issue on arm64 [kernel test robot]
>> ---
>> drivers/hv/mshv_root_main.c | 113 +++++++++++++++++++++++++++++-------
>> include/uapi/linux/mshv.h | 34 +++++++++++
>> 2 files changed, 126 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
>> index d542a0143bb8..9f9438289b60 100644
>> --- a/drivers/hv/mshv_root_main.c
>> +++ b/drivers/hv/mshv_root_main.c
>> @@ -1900,43 +1900,114 @@ add_partition(struct mshv_partition *partition)
>> return 0;
>> }
>>
>> -static long
>> -mshv_ioctl_create_partition(void __user *user_arg, struct device *module_dev)
>> +static_assert(MSHV_NUM_CPU_FEATURES_BANKS ==
>> + HV_PARTITION_PROCESSOR_FEATURES_BANKS);
>> +
>> +static long mshv_ioctl_process_pt_flags(void __user *user_arg, u64 *pt_flags,
>> + struct hv_partition_creation_properties *cr_props,
>> + union hv_partition_isolation_properties *isol_props)
>> {
>> - struct mshv_create_partition args;
>> - u64 creation_flags;
>> - struct hv_partition_creation_properties creation_properties = {};
>> - union hv_partition_isolation_properties isolation_properties = {};
>> - struct mshv_partition *partition;
>> - struct file *file;
>> - int fd;
>> - long ret;
>> + int i;
>> + struct mshv_create_partition_v2 args;
>> + union hv_partition_processor_features *disabled_procs;
>> + union hv_partition_processor_xsave_features *disabled_xsave;
>>
>> - if (copy_from_user(&args, user_arg, sizeof(args)))
>> + /* First, copy v1 struct in case user is on previous versions */
>> + if (copy_from_user(&args, user_arg,
>> + sizeof(struct mshv_create_partition)))
>> return -EFAULT;
>>
>> if ((args.pt_flags & ~MSHV_PT_FLAGS_MASK) ||
>> args.pt_isolation >= MSHV_PT_ISOLATION_COUNT)
>> return -EINVAL;
>>
>> + disabled_procs = &cr_props->disabled_processor_features;
>> + disabled_xsave = &cr_props->disabled_processor_xsave_features;
>> +
>> + /* Check if user provided newer struct with feature fields */
>> + if (args.pt_flags & BIT_ULL(MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES)) {
>> + if (copy_from_user(&args, user_arg, sizeof(args)))
>> + return -EFAULT;
>> +
>> + if (args.pt_num_cpu_fbanks != MSHV_NUM_CPU_FEATURES_BANKS ||
>> + mshv_field_nonzero(args, pt_rsvd) ||
>> + mshv_field_nonzero(args, pt_rsvd1))
>> + return -EINVAL;
>> +
>> + /*
>> + * Note this assumes MSHV_NUM_CPU_FEATURES_BANKS will never
>> + * change and equals HV_PARTITION_PROCESSOR_FEATURES_BANKS
>> + * (i.e. 2).
>> + *
>> + * Further banks (index >= 2) will be modifiable as 'early'
>> + * properties via the set partition property hypercall.
>> + */
>> + for (i = 0; i < HV_PARTITION_PROCESSOR_FEATURES_BANKS; i++)
>> + disabled_procs->as_uint64[i] = args.pt_cpu_fbanks[i];
>> +
>> +#if IS_ENABLED(CONFIG_X86_64)
>> + disabled_xsave->as_uint64 = args.pt_disabled_xsave;
>> +#else
>> + /*
>> + * In practice this field is ignored on arm64, but safer to
>> + * zero it in case it is ever used.
>> + */
>> + disabled_xsave->as_uint64 = 0;
>
> Why not explicitly treat non-zero value as invalid here instead?
> Isn't it always better than implicitly (and silently) zeroing it?
>
This is setting the hypercall input field to zero, because it looks like
the hypervisor ignores it on arm64. That's not ideal but it isn't under
our control here so all we can do is zero it.
The ioctl argument which we *do* control lacks the xsave field on ARM64,
instead we have a reserved field which is checked below (pt_rsvd2).
>> +
>> + if (mshv_field_nonzero(args, pt_rsvd2))
>> + return -EINVAL;
>> +#endif
>> + } else {
>> + /*
>> + * v1 behavior: try to enable everything. The hypervisor will
>> + * disable features that are not supported. The banks can be
>> + * queried via the get partition property hypercall.
>> + */
>> + for (i = 0; i < HV_PARTITION_PROCESSOR_FEATURES_BANKS; i++)
>> + disabled_procs->as_uint64[i] = 0;
>> +
>> + disabled_xsave->as_uint64 = 0;
>> + }
>> +
>> /* Only support EXO partitions */
>> - creation_flags = HV_PARTITION_CREATION_FLAG_EXO_PARTITION |
>> - HV_PARTITION_CREATION_FLAG_INTERCEPT_MESSAGE_PAGE_ENABLED;
>> + *pt_flags = HV_PARTITION_CREATION_FLAG_EXO_PARTITION |
>> + HV_PARTITION_CREATION_FLAG_INTERCEPT_MESSAGE_PAGE_ENABLED;
>> +
>> + if (args.pt_flags & BIT_ULL(MSHV_PT_BIT_LAPIC))
>> + *pt_flags |= HV_PARTITION_CREATION_FLAG_LAPIC_ENABLED;
>> + if (args.pt_flags & BIT_ULL(MSHV_PT_BIT_X2APIC))
>> + *pt_flags |= HV_PARTITION_CREATION_FLAG_X2APIC_CAPABLE;
>> + if (args.pt_flags & BIT_ULL(MSHV_PT_BIT_GPA_SUPER_PAGES))
>> + *pt_flags |= HV_PARTITION_CREATION_FLAG_GPA_SUPER_PAGES_ENABLED;
>>
>> - if (args.pt_flags & BIT(MSHV_PT_BIT_LAPIC))
>> - creation_flags |= HV_PARTITION_CREATION_FLAG_LAPIC_ENABLED;
>> - if (args.pt_flags & BIT(MSHV_PT_BIT_X2APIC))
>> - creation_flags |= HV_PARTITION_CREATION_FLAG_X2APIC_CAPABLE;
>> - if (args.pt_flags & BIT(MSHV_PT_BIT_GPA_SUPER_PAGES))
>> - creation_flags |= HV_PARTITION_CREATION_FLAG_GPA_SUPER_PAGES_ENABLED;
>> + isol_props->as_uint64 = 0;
>
> These properties are missing the nested one.
> I'd recommend squeezing that change in this patch
>
The nested flag is not introduced yet. It should be in a separate patch.
FYI, there is a v5 of this patch I posted on November 13.
Nuno
> Thanks,
> Stanislav
>
>>
>> switch (args.pt_isolation) {
>> case MSHV_PT_ISOLATION_NONE:
>> - isolation_properties.isolation_type =
>> - HV_PARTITION_ISOLATION_TYPE_NONE;
>> + isol_props->isolation_type = HV_PARTITION_ISOLATION_TYPE_NONE;
>> break;
>> }
>>
>> + return 0;
>> +}
>> +
>> +static long
>> +mshv_ioctl_create_partition(void __user *user_arg, struct device *module_dev)
>> +{
>> + u64 creation_flags;
>> + struct hv_partition_creation_properties creation_properties;
>> + union hv_partition_isolation_properties isolation_properties;
>> + struct mshv_partition *partition;
>> + struct file *file;
>> + int fd;
>> + long ret;
>> +
>> + ret = mshv_ioctl_process_pt_flags(user_arg, &creation_flags,
>> + &creation_properties,
>> + &isolation_properties);
>> + if (ret)
>> + return ret;
>> +
>> partition = kzalloc(sizeof(*partition), GFP_KERNEL);
>> if (!partition)
>> return -ENOMEM;
>> diff --git a/include/uapi/linux/mshv.h b/include/uapi/linux/mshv.h
>> index 876bfe4e4227..cf904f3aa201 100644
>> --- a/include/uapi/linux/mshv.h
>> +++ b/include/uapi/linux/mshv.h
>> @@ -26,6 +26,7 @@ enum {
>> MSHV_PT_BIT_LAPIC,
>> MSHV_PT_BIT_X2APIC,
>> MSHV_PT_BIT_GPA_SUPER_PAGES,
>> + MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES,
>> MSHV_PT_BIT_COUNT,
>> };
>>
>> @@ -41,6 +42,8 @@ enum {
>> * @pt_flags: Bitmask of 1 << MSHV_PT_BIT_*
>> * @pt_isolation: MSHV_PT_ISOLATION_*
>> *
>> + * This is the initial/v1 version for backward compatibility.
>> + *
>> * Returns a file descriptor to act as a handle to a guest partition.
>> * At this point the partition is not yet initialized in the hypervisor.
>> * Some operations must be done with the partition in this state, e.g. setting
>> @@ -52,6 +55,37 @@ struct mshv_create_partition {
>> __u64 pt_isolation;
>> };
>>
>> +#define MSHV_NUM_CPU_FEATURES_BANKS 2
>> +
>> +/**
>> + * struct mshv_create_partition_v2
>> + *
>> + * This is extended version of the above initial MSHV_CREATE_PARTITION
>> + * ioctl and allows for following additional parameters:
>> + *
>> + * @pt_num_cpu_fbanks: Must be set to MSHV_NUM_CPU_FEATURES_BANKS.
>> + * @pt_cpu_fbanks: Disabled processor feature banks array.
>> + * @pt_disabled_xsave: Disabled xsave feature bits.
>> + *
>> + * pt_cpu_fbanks and pt_disabled_xsave are passed through as-is to the create
>> + * partition hypercall.
>> + *
>> + * Returns : same as above original mshv_create_partition
>> + */
>> +struct mshv_create_partition_v2 {
>> + __u64 pt_flags;
>> + __u64 pt_isolation;
>> + __u16 pt_num_cpu_fbanks;
>> + __u8 pt_rsvd[6]; /* MBZ */
>> + __u64 pt_cpu_fbanks[MSHV_NUM_CPU_FEATURES_BANKS];
>> + __u64 pt_rsvd1[2]; /* MBZ */
>> +#if defined(__x86_64__)
>> + __u64 pt_disabled_xsave;
>> +#else
>> + __u64 pt_rsvd2; /* MBZ */
>> +#endif
>> +} __packed;
>> +
>> /* /dev/mshv */
>> #define MSHV_CREATE_PARTITION _IOW(MSHV_IOCTL, 0x00, struct mshv_create_partition)
>>
>> --
>> 2.34.1
^ permalink raw reply
* Re: [PATCH net-next v10 03/11] vsock: reject bad VSOCK_NET_MODE_LOCAL configuration for G2H
From: Bobby Eshleman @ 2025-11-19 1:17 UTC (permalink / raw)
To: Stefano Garzarella
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Eugenio Pérez, Xuan Zhuo, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, Shuah Khan, linux-kernel,
virtualization, netdev, kvm, linux-hyperv, linux-kselftest,
Sargun Dhillon, berrange, Bobby Eshleman
In-Reply-To: <vsyzveqyufaquwx3xgahsh3stb6i5u3xa4kubpvesfzcuj6dry@sn4kx5ctgpbz>
On Tue, Nov 18, 2025 at 07:10:28PM +0100, Stefano Garzarella wrote:
> On Mon, Nov 17, 2025 at 06:00:26PM -0800, Bobby Eshleman wrote:
> > From: Bobby Eshleman <bobbyeshleman@meta.com>
> >
> > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> > index 2c937a2df83b..c8319cd1c232 100644
> > --- a/drivers/vhost/vsock.c
> > +++ b/drivers/vhost/vsock.c
> > @@ -64,6 +64,11 @@ static u32 vhost_transport_get_local_cid(void)
> > return VHOST_VSOCK_DEFAULT_HOST_CID;
> > }
> >
> > +static bool vhost_transport_supports_local_mode(void)
> > +{
> > + return true;
>
> Should we enable this later, when we really add support, or it doesn't
> affect anything if vhost-vsock is not really supporting it in this PR
> (thinking about bisection issues).
sgtm!
>
> > +}
> > +
> > /* Callers that dereference the return value must hold vhost_vsock_mutex or the
> > * RCU read lock.
> > */
> > @@ -412,6 +417,7 @@ static struct virtio_transport vhost_transport = {
> > .module = THIS_MODULE,
> >
> > .get_local_cid = vhost_transport_get_local_cid,
> > + .supports_local_mode = vhost_transport_supports_local_mode,
> >
> > .init = virtio_transport_do_socket_init,
> > .destruct = virtio_transport_destruct,
> > diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> > index 59d97a143204..824d89657d41 100644
> > --- a/include/net/af_vsock.h
> > +++ b/include/net/af_vsock.h
> > @@ -180,6 +180,11 @@ struct vsock_transport {
> > /* Addressing. */
> > u32 (*get_local_cid)(void);
> >
> > + /* Return true if this transport supports VSOCK_NET_MODE_LOCAL.
>
> nit: Here I would make it clearer that rather than supporting MODE_LOCAL,
> the transport is not compatible with it, etc.
> A summary of the excellent description we have in the commit.
>
sounds good!
> > + * Otherwise, return false.
> > + */
> > + bool (*supports_local_mode)(void);
> > +
> > /* Read a single skb */
> > int (*read_skb)(struct vsock_sock *, skb_read_actor_t);
> >
> > diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> > index 54373ae101c3..7a235bb94437 100644
> > --- a/net/vmw_vsock/af_vsock.c
> > +++ b/net/vmw_vsock/af_vsock.c
> > @@ -91,6 +91,12 @@
> > * and locked down by a namespace manager. The default is "global". The mode
> > * is set per-namespace.
> > *
> > + * Note: LOCAL mode is only supported when using namespace-aware transports
> > + * (vhost-vsock, loopback). If a guest-to-host transport (virtio-vsock,
> > + * hyperv-vsock, vmci-vsock) is loaded, attempts to set LOCAL mode will fail
> > + * with EOPNOTSUPP, as these transports do not support per-namespace
> > + * isolation.
>
> Okay, maybe this is fine, so if you don't need to resend, feel free to
> ignore the previous comment.
>
> > + *
> > * The modes affect the allocation and accessibility of CIDs as follows:
> > *
> > * - global - access and allocation are all system-wide
> > @@ -2765,17 +2771,30 @@ static int vsock_net_mode_string(const struct ctl_table *table, int write,
> > if (*lenp >= sizeof(data))
> > return -EINVAL;
> >
> > - if (!strncmp(data, VSOCK_NET_MODE_STR_GLOBAL, sizeof(data)))
> > + ret = 0;
>
> IIUC `ret` should already be 0 at this point, no?
>
> > + mutex_lock(&vsock_register_mutex);
>
> I honestly don't like to mix the parsing, with this new check, so what
> about leaving the parsing as before this patch (also without the mutex),
> then just (untested):
>
> mutex_lock(&vsock_register_mutex);
> if (mode == VSOCK_NET_MODE_LOCAL && transport_g2h &&
> transport_g2h->supports_local_mode &&
> !transport_g2h->supports_local_mode()) {
> ret = -EOPNOTSUPP;
> goto out;
> }
>
> if (!vsock_net_write_mode(net, mode)) {
> ret = -EPERM;
> }
> out:
> mutex_unlock(&vsock_register_mutex);
> return ret;
> }
Makes sense, I can move that around for next rev.
>
> > + if (!strncmp(data, VSOCK_NET_MODE_STR_GLOBAL, sizeof(data))) {
> > mode = VSOCK_NET_MODE_GLOBAL;
> > - else if (!strncmp(data, VSOCK_NET_MODE_STR_LOCAL, sizeof(data)))
> > + } else if (!strncmp(data, VSOCK_NET_MODE_STR_LOCAL, sizeof(data))) {
> > + if (transport_g2h && transport_g2h->supports_local_mode &&
> > + !transport_g2h->supports_local_mode()) {
> > + ret = -EOPNOTSUPP;
> > + goto out;
> > + }
> > mode = VSOCK_NET_MODE_LOCAL;
> > - else
> > - return -EINVAL;
> > + } else {
> > + ret = -EINVAL;
> > + goto out;
> > + }
> >
> > - if (!vsock_net_write_mode(net, mode))
> > - return -EPERM;
> > + if (!vsock_net_write_mode(net, mode)) {
> > + ret = -EPERM;
> > + goto out;
> > + }
> >
> > - return 0;
> > +out:
> > + mutex_unlock(&vsock_register_mutex);
> > + return ret;
> > }
> >
> > static struct ctl_table vsock_table[] = {
> > @@ -2916,6 +2935,7 @@ int vsock_core_register(const struct vsock_transport *t, int features)
> > {
> > const struct vsock_transport *t_h2g, *t_g2h, *t_dgram, *t_local;
> > int err = mutex_lock_interruptible(&vsock_register_mutex);
> > + struct net *net;
> >
> > if (err)
> > return err;
> > @@ -2938,6 +2958,22 @@ int vsock_core_register(const struct vsock_transport *t, int features)
> > err = -EBUSY;
> > goto err_busy;
> > }
> > +
> > + /* G2H sockets break in LOCAL mode namespaces because G2H
> > + * transports don't support them yet. Block registering new G2H
> > + * transports if we already have local mode namespaces on the
> > + * system.
> > + */
> > + rcu_read_lock();
> > + for_each_net_rcu(net) {
> > + if (vsock_net_mode(net) == VSOCK_NET_MODE_LOCAL) {
> > + rcu_read_unlock();
> > + err = -EOPNOTSUPP;
> > + goto err_busy;
> > + }
> > + }
> > + rcu_read_unlock();
> > +
> > t_g2h = t;
> > }
> >
> > diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> > index 432fcbbd14d4..279f04fcd81a 100644
> > --- a/net/vmw_vsock/hyperv_transport.c
> > +++ b/net/vmw_vsock/hyperv_transport.c
> > @@ -833,10 +833,16 @@ int hvs_notify_set_rcvlowat(struct vsock_sock *vsk, int val)
> > return -EOPNOTSUPP;
> > }
> >
> > +static bool hvs_supports_local_mode(void)
> > +{
> > + return false;
> > +}
> > +
> > static struct vsock_transport hvs_transport = {
> > .module = THIS_MODULE,
> >
> > .get_local_cid = hvs_get_local_cid,
> > + .supports_local_mode = hvs_supports_local_mode,
> >
> > .init = hvs_sock_init,
> > .destruct = hvs_destruct,
> > diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> > index 5d379ccf3770..e585cb66c6f5 100644
> > --- a/net/vmw_vsock/virtio_transport.c
> > +++ b/net/vmw_vsock/virtio_transport.c
> > @@ -94,6 +94,18 @@ static u32 virtio_transport_get_local_cid(void)
> > return ret;
> > }
> >
> > +static bool virtio_transport_supports_local_mode(void)
> > +{
> > + struct virtio_vsock *vsock;
> > +
> > + rcu_read_lock();
> > + vsock = rcu_dereference(the_virtio_vsock);
> > + rcu_read_unlock();
> > +
> > + /* Local mode is supported only when no G2H device is present. */
> > + return vsock ? false : true;
> > +}
> > +
> > /* Caller need to hold vsock->tx_lock on vq */
> > static int virtio_transport_send_skb(struct sk_buff *skb, struct virtqueue *vq,
> > struct virtio_vsock *vsock, gfp_t gfp)
> > @@ -544,6 +556,7 @@ static struct virtio_transport virtio_transport = {
> > .module = THIS_MODULE,
> >
> > .get_local_cid = virtio_transport_get_local_cid,
> > + .supports_local_mode = virtio_transport_supports_local_mode,
> >
> > .init = virtio_transport_do_socket_init,
> > .destruct = virtio_transport_destruct,
> > diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
> > index 7eccd6708d66..da7c52ad7b2a 100644
> > --- a/net/vmw_vsock/vmci_transport.c
> > +++ b/net/vmw_vsock/vmci_transport.c
> > @@ -2033,6 +2033,12 @@ static u32 vmci_transport_get_local_cid(void)
> > return vmci_get_context_id();
> > }
> >
> > +static bool vmci_transport_supports_local_mode(void)
> > +{
> > + /* Local mode is supported only when no device is present. */
> > + return vmci_transport_get_local_cid() == VMCI_INVALID_ID;
>
> IIRC vmci can be registered both as H2G and G2H, so should we filter out
> the H2G case?
In fact, I'm realizing now that this should probably just be:
static bool vmci_transport_supports_local_mode(void)
{
return false;
}
... because even for H2G there is no mechanism for attaching a namespace
to a VM (unlike w/ vhost_vsock device open).
Does that seem right?
Best,
Bobby
^ permalink raw reply
* Re: [PATCH net-next v10 03/11] vsock: reject bad VSOCK_NET_MODE_LOCAL configuration for G2H
From: Stefano Garzarella @ 2025-11-19 11:04 UTC (permalink / raw)
To: Bobby Eshleman
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Eugenio Pérez, Xuan Zhuo, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, Shuah Khan, linux-kernel,
virtualization, netdev, kvm, linux-hyperv, linux-kselftest,
Sargun Dhillon, berrange, Bobby Eshleman
In-Reply-To: <aR0arw2F/DmbIrzY@devvm11784.nha0.facebook.com>
On Tue, Nov 18, 2025 at 05:17:35PM -0800, Bobby Eshleman wrote:
>On Tue, Nov 18, 2025 at 07:10:28PM +0100, Stefano Garzarella wrote:
>> On Mon, Nov 17, 2025 at 06:00:26PM -0800, Bobby Eshleman wrote:
>> > From: Bobby Eshleman <bobbyeshleman@meta.com>
[...]
>> > diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
>> > index 7eccd6708d66..da7c52ad7b2a 100644
>> > --- a/net/vmw_vsock/vmci_transport.c
>> > +++ b/net/vmw_vsock/vmci_transport.c
>> > @@ -2033,6 +2033,12 @@ static u32 vmci_transport_get_local_cid(void)
>> > return vmci_get_context_id();
>> > }
>> >
>> > +static bool vmci_transport_supports_local_mode(void)
>> > +{
>> > + /* Local mode is supported only when no device is present. */
>> > + return vmci_transport_get_local_cid() == VMCI_INVALID_ID;
>>
>> IIRC vmci can be registered both as H2G and G2H, so should we filter out
>> the H2G case?
>
>In fact, I'm realizing now that this should probably just be:
>
>static bool vmci_transport_supports_local_mode(void)
>{
> return false;
>}
>
>
>... because even for H2G there is no mechanism for attaching a namespace
>to a VM (unlike w/ vhost_vsock device open).
>
>Does that seem right?
tl;dr yes
vmci_transport.c has MODULE_ALIAS_NETPROTO(PF_VSOCK) for historical
reasons. This means that the module is automatically loaded the first
time PF_VSOCK is requested by the user if af_vsock is not loaded.
This was the case before vsock was generalized to support multiple
transports and has remained so for historical reasons.
So today, we can have that module loaded, registered only for F_DGRAM
but not registered for F_G2H and F_H2G, so maybe it could work for now
and if the H2G is also not supporting it, maybe is the right thing to
do. (with a better comment there on the reason why both G2H and H2G
doesn't support it).
Sorry for the long reply, maybe just `yes` was fine, but I dumped what I
thought because I feel it might be useful to you.
Thanks,
Stefano
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH v3] net: ethernet: fix uninitialized pointers with free attribute
From: Alexander Lobakin @ 2025-11-19 11:25 UTC (permalink / raw)
To: Dan Carpenter
Cc: Ally Heev, Tony Nguyen, Przemek Kitszel, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Aleksandr Loktionov, intel-wired-lan, netdev, linux-kernel,
linux-hyperv
In-Reply-To: <aRtlYIZ2XOQKMGd_@stanley.mountain>
From: Dan Carpenter <dan.carpenter@linaro.org>
Date: Mon, 17 Nov 2025 21:11:44 +0300
> On Mon, Nov 17, 2025 at 03:37:30PM +0100, Alexander Lobakin wrote:
>> From: Dan Carpenter <dan.carpenter@linaro.org>
>> Date: Mon, 17 Nov 2025 16:11:32 +0300
>>
>>> On Thu, Nov 06, 2025 at 03:07:26PM +0100, Alexander Lobakin wrote:
>>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_flow.c b/drivers/net/ethernet/intel/ice/ice_flow.c
>>>>> index 6d5c939dc8a515c252cd2b77d155b69fa264ee92..3590dacf3ee57879b3809d715e40bb290e40c4aa 100644
>>>>> --- a/drivers/net/ethernet/intel/ice/ice_flow.c
>>>>> +++ b/drivers/net/ethernet/intel/ice/ice_flow.c
>>>>> @@ -1573,12 +1573,13 @@ ice_flow_set_parser_prof(struct ice_hw *hw, u16 dest_vsi, u16 fdir_vsi,
>>>>> struct ice_parser_profile *prof, enum ice_block blk)
>>>>> {
>>>>> u64 id = find_first_bit(prof->ptypes, ICE_FLOW_PTYPE_MAX);
>>>>> - struct ice_flow_prof_params *params __free(kfree);
>>>>> u8 fv_words = hw->blk[blk].es.fvw;
>>>>> int status;
>>>>> int i, idx;
>>>>>
>>>>> - params = kzalloc(sizeof(*params), GFP_KERNEL);
>>>>> + struct ice_flow_prof_params *params __free(kfree) =
>>>>> + kzalloc(sizeof(*params), GFP_KERNEL);
>>>>
>>>> Please don't do it that way. It's not C++ with RAII and
>>>> declare-where-you-use.
>>>> Just leave the variable declarations where they are, but initialize them
>>>> with `= NULL`.
>>>>
>>>> Variable declarations must be in one block and sorted from the longest
>>>> to the shortest.
>>>>
>>>
>>> These days, with __free the trend is to say yes this is RAII and we
>>> should declare it where you use it. I personally don't have a strong
>>
>> Sorta, but we can't "declare it where you use it" since we don't allow
>> declaration-after-statement in the kernel.
>
> That changed when we merged cleanup.h. It is allowed now. I still don't
> like to declare variables anywhere unless it's a __free() variable and I
> think almost everyone else agrees. The only subsystem which I know that
> completely moved to declaring variables willy-nilly was bcachefs.
Oops, seems like I completely missed that.
So does it mean we're now allowed to write code in C++ style:
int a = func_a();
func_b();
int c = func_c();
?
Anyway, I have the same preferences as you: to declare everything at the
top of the function (or the scope if it's a loop/if/whatever). And
didn't plan to change this :D
>
> regards,
> dan carpenter
Thanks,
Olek
^ permalink raw reply
* Re: [PATCH 6.12] uio_hv_generic: Set event for all channels on the device
From: Peter Morrow @ 2025-11-19 14:23 UTC (permalink / raw)
To: Naman Jain
Cc: Wei Liu, Dexuan Cui, Greg Kroah-Hartman, Michael Kelley, Long Li,
Saurabh Sengar, K . Y . Srinivasan, Haiyang Zhang, Tianyu Lan,
linux-hyperv, linux-kernel, stable, Salvatore Bonaccorso
In-Reply-To: <dca120db-4751-4de8-961e-05ba701ca100@linux.microsoft.com>
On Sat, 15 Nov 2025 at 09:02, Naman Jain <namjain@linux.microsoft.com> wrote:
>
>
>
> On 11/15/2025 2:27 PM, Naman Jain wrote:
> > From: Long Li <longli@microsoft.com>
> >
> > Hyper-V may offer a non latency sensitive device with subchannels without
> > monitor bit enabled. The decision is entirely on the Hyper-V host not
> > configurable within guest.
> >
> > When a device has subchannels, also signal events for the subchannel
> > if its monitor bit is disabled.
> >
> > This patch also removes the memory barrier when monitor bit is enabled
> > as it is not necessary. The memory barrier is only needed between
> > setting up interrupt mask and calling vmbus_set_event() when monitor
> > bit is disabled.
> >
> > This is a backport of the upstream commit
> > d062463edf17 ("uio_hv_generic: Set event for all channels on the device")
> > with minor modifications to resolve merge conflicts.
> > Original change was not a fix, but it needs to be backported to fix a
> > NULL pointer crash resulting from missing interrupt mask setting.
> >
> > Commit 37bd91f22794 ("uio_hv_generic: Let userspace take care of interrupt mask")
> > removed the default setting of interrupt_mask for channels (including
> > subchannels) in the uio_hv_generic driver, as it relies on the user space
> > to take care of managing it. This approach works fine when user space
> > can control this setting using the irqcontrol interface provided for uio
> > devices. Support for setting the interrupt mask through this interface for
> > subchannels came only after commit d062463edf17 ("uio_hv_generic: Set event
> > for all channels on the device"). On older kernels, this change is not
> > present. With uio_hv_generic no longer setting the interrupt_mask, and
> > userspace not having the capability to set it, it remains unset,
> > and interrupts can come for the subchannels, which can result in a crash
> > in hv_uio_channel_cb. Backport the change to older kernels, where this
> > change was not present, to allow userspace to set the interrupt mask
> > properly for subchannels. Additionally, this patch also adds certain
> > checks for primary vs subchannels in the hv_uio_channel_cb, which can
> > gracefully handle these two cases and prevent the NULL pointer crashes.
> >
> > Signed-off-by: Long Li <longli@microsoft.com>
> > Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> > Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> > Fixes: 37bd91f22794 ("uio_hv_generic: Let userspace take care of interrupt mask")
>
> Sorry for missing this, please add Closes tag, if possible before merging.
>
> Closes: https://bugs.debian.org/1120602
>
> I have kept it in the other patch for 6.6 and prior kernels.
>
> Regards,
> Naman
>
> > Cc: <stable@vger.kernel.org> # 6.12.x
> > Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> > ---
>
> Regards,
> Naman
Tested-by: Peter Morrow <pdmorrow@gmail.com>
^ permalink raw reply
* [PATCH v3] Drivers: hv: ioctl for self targeted passthrough hvcalls
From: Anirudh Rayabharam @ 2025-11-19 17:17 UTC (permalink / raw)
To: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li
Cc: anirudh, linux-hyperv, linux-kernel
From: Anirudh Rayabharam (Microsoft) <anirudh@anirudhrb.com>
Allow MSHV_ROOT_HVCALL IOCTL on the /dev/mshv fd. This IOCTL would
execute a passthrough hypercall targeting the root/parent partition
i.e. HV_PARTITION_ID_SELF.
This will be useful for the VMM to query things like supported
synthetic processor features, supported VMM capabiliites etc.
Since hypercalls targeting the host partition could potentially perform
privileged operations, allow only a limited set of hypercalls. To begin
with, allow only:
HVCALL_GET_PARTITION_PROPERTY
HVCALL_GET_PARTITION_PROPERTY_EX
Signed-off-by: Anirudh Rayabharam (Microsoft) <anirudh@anirudhrb.com>
---
v3: restricted the allowed self-targeted passthrough hypercalls to
smaller, carefully selected list.
v2: rebased on latest hyperv-next
---
drivers/hv/mshv_root_main.c | 47 ++++++++++++++++++++++++++++++-------
1 file changed, 38 insertions(+), 9 deletions(-)
diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
index 45c7a5fea1cf..acbcd9c151a8 100644
--- a/drivers/hv/mshv_root_main.c
+++ b/drivers/hv/mshv_root_main.c
@@ -122,6 +122,7 @@ static struct miscdevice mshv_dev = {
*/
static u16 mshv_passthru_hvcalls[] = {
HVCALL_GET_PARTITION_PROPERTY,
+ HVCALL_GET_PARTITION_PROPERTY_EX,
HVCALL_SET_PARTITION_PROPERTY,
HVCALL_INSTALL_INTERCEPT,
HVCALL_GET_VP_REGISTERS,
@@ -136,6 +137,16 @@ static u16 mshv_passthru_hvcalls[] = {
HVCALL_GET_VP_CPUID_VALUES,
};
+/*
+ * Only allow hypercalls that are safe to be called by the VMM with the host
+ * partition as target (i.e. HV_PARTITION_ID_SELF). Carefully audit that a
+ * hypercall cannot be misused by the VMM before adding it to this list.
+ */
+static u16 mshv_self_passthru_hvcalls[] = {
+ HVCALL_GET_PARTITION_PROPERTY,
+ HVCALL_GET_PARTITION_PROPERTY_EX,
+};
+
static bool mshv_hvcall_is_async(u16 code)
{
switch (code) {
@@ -147,12 +158,30 @@ static bool mshv_hvcall_is_async(u16 code)
return false;
}
+static inline bool mshv_passthru_hvcall_allowed(u16 code, u64 pt_id)
+{
+ int i;
+ int n = ARRAY_SIZE(mshv_passthru_hvcalls);
+ u16 *allowed_hvcalls = mshv_passthru_hvcalls;
+
+ if (pt_id == HV_PARTITION_ID_SELF) {
+ n = ARRAY_SIZE(mshv_self_passthru_hvcalls);
+ allowed_hvcalls = mshv_self_passthru_hvcalls;
+ }
+
+ for (i = 0; i < n; ++i)
+ if (allowed_hvcalls[i] == code)
+ return true;
+
+ return false;
+}
+
static int mshv_ioctl_passthru_hvcall(struct mshv_partition *partition,
bool partition_locked,
void __user *user_args)
{
u64 status;
- int ret = 0, i;
+ int ret = 0;
bool is_async;
struct mshv_root_hvcall args;
struct page *page;
@@ -160,6 +189,7 @@ static int mshv_ioctl_passthru_hvcall(struct mshv_partition *partition,
void *input_pg = NULL;
void *output_pg = NULL;
u16 reps_completed;
+ u64 pt_id = partition ? partition->pt_id : HV_PARTITION_ID_SELF;
if (copy_from_user(&args, user_args, sizeof(args)))
return -EFAULT;
@@ -171,17 +201,13 @@ static int mshv_ioctl_passthru_hvcall(struct mshv_partition *partition,
if (args.out_ptr && (!args.out_sz || args.out_sz > HV_HYP_PAGE_SIZE))
return -EINVAL;
- for (i = 0; i < ARRAY_SIZE(mshv_passthru_hvcalls); ++i)
- if (args.code == mshv_passthru_hvcalls[i])
- break;
-
- if (i >= ARRAY_SIZE(mshv_passthru_hvcalls))
+ if (!mshv_passthru_hvcall_allowed(args.code, pt_id))
return -EINVAL;
is_async = mshv_hvcall_is_async(args.code);
if (is_async) {
/* async hypercalls can only be called from partition fd */
- if (!partition_locked)
+ if (!partition || !partition_locked)
return -EINVAL;
ret = mshv_init_async_handler(partition);
if (ret)
@@ -209,7 +235,7 @@ static int mshv_ioctl_passthru_hvcall(struct mshv_partition *partition,
* NOTE: This only works because all the allowed hypercalls' input
* structs begin with a u64 partition_id field.
*/
- *(u64 *)input_pg = partition->pt_id;
+ *(u64 *)input_pg = pt_id;
reps_completed = 0;
do {
@@ -238,7 +264,7 @@ static int mshv_ioctl_passthru_hvcall(struct mshv_partition *partition,
ret = hv_result_to_errno(status);
else
ret = hv_call_deposit_pages(NUMA_NO_NODE,
- partition->pt_id, 1);
+ pt_id, 1);
} while (!ret);
args.status = hv_result(status);
@@ -2050,6 +2076,9 @@ static long mshv_dev_ioctl(struct file *filp, unsigned int ioctl,
case MSHV_CREATE_PARTITION:
return mshv_ioctl_create_partition((void __user *)arg,
misc->this_device);
+ case MSHV_ROOT_HVCALL:
+ return mshv_ioctl_passthru_hvcall(NULL, false,
+ (void __user *)arg);
}
return -ENOTTY;
base-commit: db7df69995ffbe806d60ad46d5fb9d959da9e549
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v2] Drivers: hv: ioctl for self targeted passthrough hvcalls
From: Anirudh Rayabharam @ 2025-11-19 17:19 UTC (permalink / raw)
To: Nuno Das Neves
Cc: Wei Liu, K. Y. Srinivasan, Haiyang Zhang, Dexuan Cui, Long Li,
linux-hyperv, linux-kernel
In-Reply-To: <d65a1b2d-2fdf-4cd1-bd04-a438205c7a70@linux.microsoft.com>
On Mon, Nov 17, 2025 at 03:42:19PM -0800, Nuno Das Neves wrote:
> On 11/17/2025 11:24 AM, Wei Liu wrote:
> > On Mon, Nov 17, 2025 at 07:18:27PM +0000, Wei Liu wrote:
> >> On Mon, Nov 17, 2025 at 10:16:12AM -0800, Nuno Das Neves wrote:
> >>> On 11/17/2025 1:52 AM, Anirudh Rayabharam wrote:
> >>>> From: Anirudh Rayabharam (Microsoft) <anirudh@anirudhrb.com>
> >>>>
> >>>> Allow MSHV_ROOT_HVCALL IOCTL on the /dev/mshv fd. This IOCTL would
> >>>> execute a passthrough hypercall targeting the root/parent partition
> >>>> i.e. HV_PARTITION_ID_SELF.
> >>>>
> >>>
> >>> I think it's worth taking a moment to check and perhaps explain in
> >>> the commit message/a comment any security implications of the VMM
> >>> process being able to call these hypercalls on the root/parent
> >>> partition.
> >>>
> >>> One implication would be: can the VMM process influence other
> >>> processes in the root partition via these hypercalls,
Thanks for the review! This is a really good point.
> >>> e.g. HVCALL_SET_VP_REGISTERS? I would think that the hypervisor
> >>> itself disallows this but we should check. We can ask the
> >>> hypervisor team what they think, and check the hypervisor code.
> >>>
> >>> Specifically we should check on any hypercall that could possibly
> >>> influence partition state, i.e.:
> >>> HVCALL_SET_PARTITION_PROPERTY
> >>> HVCALL_SET_VP_REGISTERS
> >>> HVCALL_INSTALL_INTERCEPT
> >>> HVCALL_CLEAR_VIRTUAL_INTERRUPT
> >>> HVCALL_REGISTER_INTERCEPT_RESULT
> >>> HVCALL_ASSERT_VIRTUAL_INTERRUPT
> >>> HVCALL_SIGNAL_EVENT_DIRECT
> >>> HVCALL_POST_MESSAGE_DIRECT
> >>>
> >>> If it turns out there is something risky we are enabling here, we can
> >>> introduce a new array of hypercalls to restrict which ones can be
> >>> called on HV_PARTITION_ID_SELF.
> >>>
> >>
> >> This is a good point. Please check with the hypervisor team.
> >
> > I should add: it is always easier to relax restrictions later than to
> > add them back in, so if there is any doubt and we want this code in as
> > quickly as possible, we can start with a new array and expand it later.
> >
>
> Agreed. I think that's a good approach here, we can just enable
> HVCALL_GET_PARTITION_PROPERTY and HVCALL_GET_PARTITION_PROPERTY_EX for
> self-targeted passthru hypercalls.
I've implemented this in v3.
Anirudh
>
> > Wei
>
^ permalink raw reply
* Re: [PATCH v6 3/5] Drivers: hv: Batch GPA unmap operations to improve large region performance
From: Stanislav Kinsburskii @ 2025-11-19 17:42 UTC (permalink / raw)
To: Michael Kelley
Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <SN6PR02MB41574B72041F6AFB1B83DE36D4D6A@SN6PR02MB4157.namprd02.prod.outlook.com>
On Tue, Nov 18, 2025 at 04:28:55PM +0000, Michael Kelley wrote:
> From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Monday, November 17, 2025 8:53 AM
> >
> > Reduce overhead when unmapping large memory regions by batching GPA unmap
> > operations in 2MB-aligned chunks.
> >
> > Use a dedicated constant for batch size to improve code clarity and
> > maintainability.
> >
> > Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
> > Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> > Reviewed-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> > ---
> > drivers/hv/mshv_root.h | 2 ++
> > drivers/hv/mshv_root_hv_call.c | 2 +-
> > drivers/hv/mshv_root_main.c | 36 ++++++++++++++++++++++++++++++------
> > 3 files changed, 33 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
> > index 3eb815011b46..5eece7077f8b 100644
> > --- a/drivers/hv/mshv_root.h
> > +++ b/drivers/hv/mshv_root.h
> > @@ -32,6 +32,8 @@ static_assert(HV_HYP_PAGE_SIZE == MSHV_HV_PAGE_SIZE);
> >
> > #define MSHV_PIN_PAGES_BATCH_SIZE (0x10000000ULL / HV_HYP_PAGE_SIZE)
> >
> > +#define MSHV_MAX_UNMAP_GPA_PAGES 512
> > +
> > struct mshv_vp {
> > u32 vp_index;
> > struct mshv_partition *vp_partition;
> > diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
> > index caf02cfa49c9..8fa983f1109b 100644
> > --- a/drivers/hv/mshv_root_hv_call.c
> > +++ b/drivers/hv/mshv_root_hv_call.c
> > @@ -17,7 +17,7 @@
> > /* Determined empirically */
> > #define HV_INIT_PARTITION_DEPOSIT_PAGES 208
> > #define HV_MAP_GPA_DEPOSIT_PAGES 256
> > -#define HV_UMAP_GPA_PAGES 512
> > +#define HV_UMAP_GPA_PAGES MSHV_MAX_UNMAP_GPA_PAGES
> >
> > #define HV_PAGE_COUNT_2M_ALIGNED(pg_count) (!((pg_count) & (0x200 - 1)))
> >
> > diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> > index a85872b72b1a..ef36d8115d8a 100644
> > --- a/drivers/hv/mshv_root_main.c
> > +++ b/drivers/hv/mshv_root_main.c
> > @@ -1365,7 +1365,7 @@ mshv_map_user_memory(struct mshv_partition *partition,
> > static void mshv_partition_destroy_region(struct mshv_mem_region *region)
> > {
> > struct mshv_partition *partition = region->partition;
> > - u32 unmap_flags = 0;
> > + u64 gfn, gfn_count, start_gfn, end_gfn;
> > int ret;
> >
> > hlist_del(®ion->hnode);
> > @@ -1380,12 +1380,36 @@ static void mshv_partition_destroy_region(struct mshv_mem_region *region)
> > }
> > }
> >
> > - if (region->flags.large_pages)
> > - unmap_flags |= HV_UNMAP_GPA_LARGE_PAGE;
> > + start_gfn = region->start_gfn;
> > + end_gfn = region->start_gfn + region->nr_pages;
> > +
> > + for (gfn = start_gfn; gfn < end_gfn; gfn += gfn_count) {
> > + u32 unmap_flags = 0;
> > +
> > + if (gfn % MSHV_MAX_UNMAP_GPA_PAGES)
> > + gfn_count = ALIGN(gfn, MSHV_MAX_UNMAP_GPA_PAGES) - gfn;
> > + else
> > + gfn_count = MSHV_MAX_UNMAP_GPA_PAGES;
> > +
> > + if (gfn + gfn_count > end_gfn)
> > + gfn_count = end_gfn - gfn;
> >
> > - /* ignore unmap failures and continue as process may be exiting */
> > - hv_call_unmap_gpa_pages(partition->pt_id, region->start_gfn,
> > - region->nr_pages, unmap_flags);
> > + /* Skip all pages in this range if none are mapped */
> > + if (!memchr_inv(region->pages + (gfn - start_gfn), 0,
> > + gfn_count * sizeof(struct page *)))
> > + continue;
> > +
> > + if (region->flags.large_pages &&
> > + VALUE_PMD_ALIGNED(gfn) && VALUE_PMD_ALIGNED(gfn_count))
>
> VALUE_PMD_ALIGNED isn't defined until Patch 4 of this series.
>
> The idea of a page count being PMD aligned occurs a few other places in
> Linux kernel code, and it is usually written as IS_ALIGNED(count, PTRS_PER_PMD),
> though there's one occurrence of !(count % PTRS_PER_PMD).
>
> Also mshv_root_hv_call.c has HV_PAGE_COUNT_2M_ALIGNED() that does
> the same thing. Some macro consolidation is appropriate, or just open code
> as IS_ALIGNED(<cnt>, PTRS_PER_PMD) and eliminate the macros.
>
Thanks for the idea.
I'll update accordingly.
Thanks,
Stanislav
> > + unmap_flags |= HV_UNMAP_GPA_LARGE_PAGE;
> > +
> > + ret = hv_call_unmap_gpa_pages(partition->pt_id, gfn,
> > + gfn_count, unmap_flags);
> > + if (ret)
> > + pt_err(partition,
> > + "Failed to unmap GPA pages %#llx-%#llx: %d\n",
> > + gfn, gfn + gfn_count - 1, ret);
> > + }
> >
> > mshv_region_invalidate(region);
> >
> >
> >
^ permalink raw reply
* Re: [PATCH v6 5/5] Drivers: hv: Add support for movable memory regions
From: kernel test robot @ 2025-11-19 18:06 UTC (permalink / raw)
To: Stanislav Kinsburskii, kys, haiyangz, wei.liu, decui
Cc: oe-kbuild-all, linux-hyperv, linux-kernel
In-Reply-To: <176339837995.27330.14240947043073674139.stgit@skinsburskii-cloud-desktop.internal.cloudapp.net>
Hi Stanislav,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.18-rc6]
[cannot apply to next-20251119]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Stanislav-Kinsburskii/Drivers-hv-Refactor-and-rename-memory-region-handling-functions/20251118-010501
base: linus/master
patch link: https://lore.kernel.org/r/176339837995.27330.14240947043073674139.stgit%40skinsburskii-cloud-desktop.internal.cloudapp.net
patch subject: [PATCH v6 5/5] Drivers: hv: Add support for movable memory regions
config: x86_64-randconfig-076-20251119 (https://download.01.org/0day-ci/archive/20251120/202511200145.iDV1mUnj-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251120/202511200145.iDV1mUnj-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511200145.iDV1mUnj-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/vdso/const.h:5,
from include/linux/const.h:4,
from include/uapi/linux/kernel.h:6,
from include/linux/cache.h:5,
from arch/x86/include/asm/current.h:10,
from include/linux/sched.h:12,
from include/linux/resume_user_mode.h:6,
from include/linux/entry-virt.h:6,
from drivers/hv/mshv_root_main.c:11:
In function 'mshv_region_handle_gfn_fault',
inlined from 'mshv_handle_gpa_intercept' at drivers/hv/mshv_root_main.c:742:9,
inlined from 'mshv_vp_handle_intercept' at drivers/hv/mshv_root_main.c:755:10,
inlined from 'mshv_vp_ioctl_run_vp' at drivers/hv/mshv_root_main.c:769:22,
inlined from 'mshv_vp_ioctl' at drivers/hv/mshv_root_main.c:958:7:
>> include/linux/compiler_types.h:602:45: error: call to '__compiletime_assert_1093' declared with attribute error: BUILD_BUG failed
602 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^
include/uapi/linux/const.h:49:44: note: in definition of macro '__ALIGN_KERNEL_MASK'
49 | #define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask))
| ^
include/vdso/align.h:9:33: note: in expansion of macro '__ALIGN_KERNEL'
9 | #define ALIGN_DOWN(x, a) __ALIGN_KERNEL((x) - ((a) - 1), (a))
| ^~~~~~~~~~~~~~
drivers/hv/mshv_root_main.c:691:23: note: in expansion of macro 'ALIGN_DOWN'
691 | page_offset = ALIGN_DOWN(gfn - region->start_gfn,
| ^~~~~~~~~~
include/linux/compiler_types.h:590:9: note: in expansion of macro '__compiletime_assert'
590 | __compiletime_assert(condition, msg, prefix, suffix)
| ^~~~~~~~~~~~~~~~~~~~
include/linux/compiler_types.h:602:9: note: in expansion of macro '_compiletime_assert'
602 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
| ^~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:59:21: note: in expansion of macro 'BUILD_BUG_ON_MSG'
59 | #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")
| ^~~~~~~~~~~~~~~~
include/linux/huge_mm.h:113:28: note: in expansion of macro 'BUILD_BUG'
113 | #define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
| ^~~~~~~~~
include/linux/huge_mm.h:117:26: note: in expansion of macro 'HPAGE_PMD_SHIFT'
117 | #define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT)
| ^~~~~~~~~~~~~~~
include/linux/huge_mm.h:118:26: note: in expansion of macro 'HPAGE_PMD_ORDER'
118 | #define HPAGE_PMD_NR (1<<HPAGE_PMD_ORDER)
| ^~~~~~~~~~~~~~~
drivers/hv/mshv_root_main.c:40:49: note: in expansion of macro 'HPAGE_PMD_NR'
40 | #define MSHV_MAP_FAULT_IN_PAGES HPAGE_PMD_NR
| ^~~~~~~~~~~~
drivers/hv/mshv_root_main.c:692:34: note: in expansion of macro 'MSHV_MAP_FAULT_IN_PAGES'
692 | MSHV_MAP_FAULT_IN_PAGES);
| ^~~~~~~~~~~~~~~~~~~~~~~
vim +/__compiletime_assert_1093 +602 include/linux/compiler_types.h
eb5c2d4b45e3d2 Will Deacon 2020-07-21 588
eb5c2d4b45e3d2 Will Deacon 2020-07-21 589 #define _compiletime_assert(condition, msg, prefix, suffix) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 590 __compiletime_assert(condition, msg, prefix, suffix)
eb5c2d4b45e3d2 Will Deacon 2020-07-21 591
eb5c2d4b45e3d2 Will Deacon 2020-07-21 592 /**
eb5c2d4b45e3d2 Will Deacon 2020-07-21 593 * compiletime_assert - break build and emit msg if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21 594 * @condition: a compile-time constant condition to check
eb5c2d4b45e3d2 Will Deacon 2020-07-21 595 * @msg: a message to emit if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21 596 *
eb5c2d4b45e3d2 Will Deacon 2020-07-21 597 * In tradition of POSIX assert, this macro will break the build if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21 598 * supplied condition is *false*, emitting the supplied error message if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21 599 * compiler has support to do so.
eb5c2d4b45e3d2 Will Deacon 2020-07-21 600 */
eb5c2d4b45e3d2 Will Deacon 2020-07-21 601 #define compiletime_assert(condition, msg) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 @602 _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
eb5c2d4b45e3d2 Will Deacon 2020-07-21 603
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v3] Drivers: hv: ioctl for self targeted passthrough hvcalls
From: Wei Liu @ 2025-11-19 21:34 UTC (permalink / raw)
To: Anirudh Rayabharam
Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
linux-hyperv, linux-kernel
In-Reply-To: <20251119171708.819072-1-anirudh@anirudhrb.com>
On Wed, Nov 19, 2025 at 05:17:08PM +0000, Anirudh Rayabharam wrote:
[...]
> +static inline bool mshv_passthru_hvcall_allowed(u16 code, u64 pt_id)
There is no need to add inline here. The compiler can decide itself.
I can drop this when committing.
Wei
^ permalink raw reply
* Re: [PATCH 6/9] KVM: SVM: Filter out 64-bit exit codes when invoking exit handlers on bare metal
From: Sean Christopherson @ 2025-11-19 22:05 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Vitaly Kuznetsov, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, kvm, linux-hyperv, linux-kernel, Jim Mattson,
Yosry Ahmed
In-Reply-To: <60f7c9b3-312f-41e2-ab47-c4361df1d825@redhat.com>
On Sat, Nov 15, 2025, Paolo Bonzini wrote:
> On 11/13/25 23:56, Sean Christopherson wrote:
> > Explicitly filter out 64-bit exit codes when invoking exit handlers, as
> > svm_exit_handlers[] will never be sized with entries that use bits 63:32.
> >
> > Processing the non-failing exit code as a 32-bit value will allow tracking
> > exit_code as a single 64-bit value (which it is, architecturally). This
> > will also allow hardening KVM against Spectre-like attacks without needing
> > to do silly things to avoid build failures on 32-bit kernels
> > (array_index_nospec() rightly asserts that the index fits in an "unsigned
> > long").
> >
> > Omit the check when running as a VM, as KVM has historically failed to set
> > bits 63:32 appropriately when synthesizing VM-Exits, i.e. KVM could get
> > false positives when running as a VM on an older, broken KVM/kernel. From
> > a functional perspective, omitting the check is "fine", as any unwanted
> > collision between e.g. VMEXIT_INVALID and a 32-bit exit code will be
> > fatal to KVM-on-KVM regardless of what KVM-as-L1 does.
> >
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> > arch/x86/kvm/svm/svm.c | 18 ++++++++++++++++--
> > 1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index 202a4d8088a2..3b05476296d0 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -3433,8 +3433,22 @@ static void dump_vmcb(struct kvm_vcpu *vcpu)
> > sev_free_decrypted_vmsa(vcpu, save);
> > }
> > -int svm_invoke_exit_handler(struct kvm_vcpu *vcpu, u64 exit_code)
> > +int svm_invoke_exit_handler(struct kvm_vcpu *vcpu, u64 __exit_code)
> > {
> > + u32 exit_code = __exit_code;
> > +
> > + /*
> > + * SVM uses negative values, i.e. 64-bit values, to indicate that VMRUN
> > + * failed. Report all such errors to userspace (note, VMEXIT_INVALID,
> > + * a.k.a. SVM_EXIT_ERR, is special cased by svm_handle_exit()). Skip
> > + * the check when running as a VM, as KVM has historically left garbage
> > + * in bits 63:32, i.e. running KVM-on-KVM would hit false positives if
> > + * the underlying kernel is buggy.
> > + */
> > + if (!cpu_feature_enabled(X86_FEATURE_HYPERVISOR) &&
> > + (u64)exit_code != __exit_code)
> > + goto unexpected_vmexit;
>
> I reviewed the series and it looks good, but with respect to this patch and
> patch 8, is it really worth it? While there is a possibility that code
> 0x00000000ffffffff is used, or that any high 32-bit values other than
> all-zeros or all-ones are used, they'd be presumably enabled by some control
> bits in the VMCB or some paravirt thing in the hypervisor.
Maybe. E.g. TDCALL and SEAMCALL VM-Exits on Intel show up without any enablement
in software (beyond VMXON). I completely agree that it's extremely unlikely that
AMD will add a on-negative exit code with bits 63:32 != 0, i.e. that we could get
a false positive when truncating exit_code to a u32, but it also seems harmless
to be paranoid.
FWIW, I was assuming VMEXIT_INVALID_PMC (-4) was a generic vPMU thing, but it
looks like that one is also SEV-ES+ specific.
As for e57b84699534 ("KVM: SVM: Limit incorrect check on SVM_EXIT_ERR to running
as a VM"), I agree that being paranoid probably doesn't do anything in practice,
but I like being consistent. :-)
> What really matters is that SEV-ES's kvm_get_cached_sw_exit_code() is
> reading the full 64 bits and discarding invalid codes before reaching
> svm_invoke_exit_handler().
No? sev_handle_vmgexit() only handles SVM_VMGEXIT_xxx exit codes, everything
else is punted to svm_invoke_exit_handler()
exit_code = kvm_get_cached_sw_exit_code(control);
switch (exit_code) {
case SVM_VMGEXIT_<0>
...
case SVM_VMGEXIT_<N>
default:
ret = svm_invoke_exit_handler(vcpu, exit_code);
}
And I don't see anything that filters/modifies exit_code_hi.
^ permalink raw reply
* Re: [PATCH v3] Drivers: hv: ioctl for self targeted passthrough hvcalls
From: Nuno Das Neves @ 2025-11-19 23:09 UTC (permalink / raw)
To: Anirudh Rayabharam, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li
Cc: linux-hyperv, linux-kernel
In-Reply-To: <20251119171708.819072-1-anirudh@anirudhrb.com>
On 11/19/2025 9:17 AM, Anirudh Rayabharam wrote:
> From: Anirudh Rayabharam (Microsoft) <anirudh@anirudhrb.com>
>
> Allow MSHV_ROOT_HVCALL IOCTL on the /dev/mshv fd. This IOCTL would
> execute a passthrough hypercall targeting the root/parent partition
> i.e. HV_PARTITION_ID_SELF.
>
> This will be useful for the VMM to query things like supported
> synthetic processor features, supported VMM capabiliites etc.
>
> Since hypercalls targeting the host partition could potentially perform
> privileged operations, allow only a limited set of hypercalls. To begin
> with, allow only:
>
> HVCALL_GET_PARTITION_PROPERTY
> HVCALL_GET_PARTITION_PROPERTY_EX
>
> Signed-off-by: Anirudh Rayabharam (Microsoft) <anirudh@anirudhrb.com>
> ---
>
> v3: restricted the allowed self-targeted passthrough hypercalls to
> smaller, carefully selected list.
> v2: rebased on latest hyperv-next
>
> ---
> drivers/hv/mshv_root_main.c | 47 ++++++++++++++++++++++++++++++-------
> 1 file changed, 38 insertions(+), 9 deletions(-)
>
Reviewed-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
^ permalink raw reply
* Re: [PATCH v6 5/5] Drivers: hv: Add support for movable memory regions
From: Stanislav Kinsburskii @ 2025-11-20 0:45 UTC (permalink / raw)
To: Michael Kelley
Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <SN6PR02MB4157DB4154734C44B5D85A88D4D6A@SN6PR02MB4157.namprd02.prod.outlook.com>
On Tue, Nov 18, 2025 at 04:29:56PM +0000, Michael Kelley wrote:
> From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Monday, November 17, 2025 8:53 AM
> >
> > Introduce support for movable memory regions in the Hyper-V root partition
> > driver, thus improving memory management flexibility and preparing the
> > driver for advanced use cases such as dynamic memory remapping.
> >
> > Integrate mmu_interval_notifier for movable regions, implement functions to
> > handle HMM faults and memory invalidation, and update memory region mapping
> > logic to support movable regions.
> >
> > While MMU notifiers are commonly used in virtualization drivers, this
> > implementation leverages HMM (Heterogeneous Memory Management) for its
> > tailored functionality. HMM provides a ready-made framework for mirroring,
> > invalidation, and fault handling, avoiding the need to reimplement these
> > mechanisms for a single callback. Although MMU notifiers are more generic,
> > using HMM reduces boilerplate and ensures maintainability by utilizing a
> > mechanism specifically designed for such use cases.
>
> I wasn't previously familiar with HMM when reviewing this code, so I had to
> work through the details, and mentally build a high-level view of how this
> case use maps to concepts in the Linux kernel documentation for HMM.
> Here's my take:
>
> In this use case, the goal is what HMM calls "address space mirroring"
> between Linux in the root partition and a guest VM. Linux in the root
> partition is the primary owner of the memory. Each guest VM is a
> "device" from the HMM standpoint, and the device page tables are the
> hypervisor's SLAT entries that are managed using hypercalls to map and
> unmap memory. When a guest VM is using unpinned memory, the guest
> VM faults and generates a VP intercept when it first touches a page in its
> GFN space. MSHV is the device driver for the "device", and it handles the
> VP intercept by calling hmm_range_fault() to populate the pages, and then
> making hypercalls to set up the "device" page tables (i.e., the SLAT entries).
> Linux in the root partition can reclaim ownership of a memory range via
> the HMM "invalidate" callback, which MSHV handles by making hypercalls
> to clear the SLAT entries. After such a invalidate, the guest VM will fault
> again if it touches the memory range.
>
> Is this a correct understanding? If so, including such a summary in the
> commit message or as a code comment would be helpful to people
> in the future who are looking at the code.
>
Yes, this is a correct understanding.
I'll add a description to the commit message.
> >
> > Signed-off-by: Anirudh Rayabharam <anrayabh@linux.microsoft.com>
> > Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
> > ---
> > drivers/hv/Kconfig | 1
> > drivers/hv/mshv_root.h | 8 +
> > drivers/hv/mshv_root_main.c | 328
> > ++++++++++++++++++++++++++++++++++++++++++-
> > 3 files changed, 327 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
> > index 0b8c391a0342..5f1637cbb6e3 100644
> > --- a/drivers/hv/Kconfig
> > +++ b/drivers/hv/Kconfig
> > @@ -75,6 +75,7 @@ config MSHV_ROOT
> > depends on PAGE_SIZE_4KB
> > select EVENTFD
> > select VIRT_XFER_TO_GUEST_WORK
> > + select HMM_MIRROR
>
> Couldn't you also do "select MMU_NOTIFIER" to avoid the #ifdef's
> and stubs for when it isn't selected? There are other Linux kernel
> drivers that select it. Or is the intent to allow building an image that
> doesn't support unpinned memory, and the #ifdef's save space?
>
That's an interesting question. This driver can function without MMU notifiers
by pinning all memory, which might be advantageous for certain real-time
applications.
However, since most other virtualization solutions use MMU_NOTIFIER, there
doesn't appear to be a strong reason for this driver to deviate.
> > default n
> > help
> > Select this option to enable support for booting and running as root
> > diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
> > index 5eece7077f8b..117399dea780 100644
> > --- a/drivers/hv/mshv_root.h
> > +++ b/drivers/hv/mshv_root.h
> > @@ -15,6 +15,7 @@
> > #include <linux/hashtable.h>
> > #include <linux/dev_printk.h>
> > #include <linux/build_bug.h>
> > +#include <linux/mmu_notifier.h>
> > #include <uapi/linux/mshv.h>
> >
> > /*
> > @@ -81,9 +82,14 @@ struct mshv_mem_region {
> > struct {
> > u64 large_pages: 1; /* 2MiB */
> > u64 range_pinned: 1;
> > - u64 reserved: 62;
> > + u64 is_ram : 1; /* mem region can be ram or mmio */
>
> The combination of "range_pinned" and "is_ram" effectively define three
> MSHV region types:
>
> 1) MMIO
> 2) Pinned RAM
> 3) Unpinned RAM
>
> Would it make sense to have an enum instead of 2 booleans? It seems
> like that would simplify the code a bit in a couple places. For example,
> mshv_handle_gpa_intercept() could just do one WARN_ON() instead of two.
> Also you would not need mshv_partition_destroy_region() testing "is_ram",
> and then mshv_region_movable_fini() testing "range_pinned". A single test
> could cover both.
>
> Just a suggestion. Ignore my comment if you prefer the 2 booleans.
>
I's rather keep it as is as it's a legacy pattern people have been used
to. A small runtime optimization doesn't worth the churn from my POV.
> > +/**
> > + * mshv_region_range_fault - Handle memory range faults for a given region.
> > + * @region: Pointer to the memory region structure.
> > + * @page_offset: Offset of the page within the region.
> > + * @page_count: Number of pages to handle.
> > + *
> > + * This function resolves memory faults for a specified range of pages
> > + * within a memory region. It uses HMM (Heterogeneous Memory Management)
> > + * to fault in the required pages and updates the region's page array.
> > + *
> > + * Return: 0 on success, negative error code on failure.
> > + */
> > +static int mshv_region_range_fault(struct mshv_mem_region *region,
> > + u64 page_offset, u64 page_count)
> > +{
> > + struct hmm_range range = {
> > + .notifier = ®ion->mni,
> > + .default_flags = HMM_PFN_REQ_FAULT | HMM_PFN_REQ_WRITE,
> > + };
> > + unsigned long *pfns;
> > + int ret;
> > + u64 i;
> > +
> > + pfns = kmalloc_array(page_count, sizeof(unsigned long), GFP_KERNEL);
> > + if (!pfns)
> > + return -ENOMEM;
> > +
> > + range.hmm_pfns = pfns;
> > + range.start = region->start_uaddr + page_offset * HV_HYP_PAGE_SIZE;
> > + range.end = range.start + page_count * HV_HYP_PAGE_SIZE;
> > +
> > + do {
> > + ret = mshv_region_hmm_fault_and_lock(region, &range);
> > + } while (ret == -EBUSY);
>
> I expected the looping on -EBUSY to be in mshv_region_hmm_fault_and_lock(),
> but I guess it really doesn't matter.
>
> > +
> > + if (ret)
> > + goto out;
> > +
> > + for (i = 0; i < page_count; i++)
> > + region->pages[page_offset + i] = hmm_pfn_to_page(pfns[i]);
>
> The comment with hmm_pfn_to_page() says that the caller is assumed to
> have tested the pfn for HMM_PFN_VALID, which I don't see done anywhere.
> Is there a reason it's not necessary to test?
>
The reason is the HMM_PFN_REQ_FAULT range flag, which requires all PFNs to be
faulted and populated, or mshv_region_hmm_fault_and_lock will return -EFAULT.
Additionally, note that mshv_region_hmm_fault_and_lock returns with
region->mutex held upon success, ensuring that no page can be moved until the
lock is released.
> > +
> > + if (PageHuge(region->pages[page_offset]))
> > + region->flags.large_pages = true;
>
> See comment below in mshv_region_handle_gfn_fault().
>
> > +
> > + ret = mshv_region_remap_pages(region, region->hv_map_flags,
> > + page_offset, page_count);
> > +
> > + mutex_unlock(®ion->mutex);
> > +out:
> > + kfree(pfns);
> > + return ret;
> > +}
> > +#else /* CONFIG_MMU_NOTIFIER */
> > +static int mshv_region_range_fault(struct mshv_mem_region *region,
> > + u64 page_offset, u64 page_count)
> > +{
> > + return -ENODEV;
> > +}
> > +#endif /* CONFIG_MMU_NOTIFIER */
> > +
> > +static bool mshv_region_handle_gfn_fault(struct mshv_mem_region *region, u64 gfn)
> > +{
> > + u64 page_offset, page_count;
> > + int ret;
> > +
> > + if (WARN_ON_ONCE(region->flags.range_pinned))
> > + return false;
> > +
> > + /* Align the page offset to the nearest MSHV_MAP_FAULT_IN_PAGES. */
> > + page_offset = ALIGN_DOWN(gfn - region->start_gfn,
> > + MSHV_MAP_FAULT_IN_PAGES);
> > +
> > + /* Map more pages than requested to reduce the number of faults. */
> > + page_count = min(region->nr_pages - page_offset,
> > + MSHV_MAP_FAULT_IN_PAGES);
>
> These computations make the range defined by page_offset and page_count
> start on a 512 page boundary relative to start_gfn, and have a size that is a
> multiple of 512 pages. But they don't ensure that the range aligns to a large page
> boundary within gfn space since region->start_gfn may not be a multiple of
> 512. Then mshv_region_range_fault() tests the first page of the range for
> being a large page, and if so, sets region->large_pages. This doesn't make
> sense to me if the range doesn't align to a large page boundary.
>
> Does this code need to make sure that the range is aligned to a large
> page boundary in gfn space? Or am I misunderstanding what the
> region->large_pages flag means? Given the fixes in this v6 of the
> patch set, I was thinking that region->large_pages means that every
> large page aligned area within the region is a large page. If region->
> start_gfn and region->nr_pages aren't multiples of 512, then there
> may be an initial range and a final range that aren't large pages,
> but everything in between is. If that's not a correct understanding,
> could you clarify the exact meaning of the region->large_pages
> flag?
>
That's a good catch. Initially, the approach to memory deposit involved pinning
and depositing all pages. The code assumes that if the first page in the region
is huge, it is sufficient to use the "map huge page" flag in the hypercall.
With this series, the region is sparse by default, reducing the likelihood of
huge pages in the region. As a result, using this flag seems neither correct
nor reasonable.
Ideally, whether to use the flag should be determined during each guest memory
map/unmap operation, rather than relying on the flag set during the initial
region mapping.
For now, I will remove the large_pages flag for movable regions in this
series, as it is the least intrusive change. However, I plan to investigate
this further and potentially replace the large_pages flag with a runtime
check in the next series.
> > +
> > + ret = mshv_region_range_fault(region, page_offset, page_count);
> > +
> > + WARN_ONCE(ret,
> > + "p%llu: GPA intercept failed: region %#llx-%#llx, gfn %#llx, page_offset %llu, page_count %llu\n",
> > + region->partition->pt_id, region->start_uaddr,
> > + region->start_uaddr + (region->nr_pages << HV_HYP_PAGE_SHIFT),
> > + gfn, page_offset, page_count);
> > +
> > + return !ret;
> > +}
> > +
> > +/**
> > + * mshv_handle_gpa_intercept - Handle GPA (Guest Physical Address) intercepts.
> > + * @vp: Pointer to the virtual processor structure.
> > + *
> > + * This function processes GPA intercepts by identifying the memory region
> > + * corresponding to the intercepted GPA, aligning the page offset, and
> > + * mapping the required pages. It ensures that the region is valid and
> > + * handles faults efficiently by mapping multiple pages at once.
> > + *
> > + * Return: true if the intercept was handled successfully, false otherwise.
> > + */
> > +static bool mshv_handle_gpa_intercept(struct mshv_vp *vp)
> > +{
> > + struct mshv_partition *p = vp->vp_partition;
> > + struct mshv_mem_region *region;
> > + struct hv_x64_memory_intercept_message *msg;
> > + u64 gfn;
> > +
> > + msg = (struct hv_x64_memory_intercept_message *)
> > + vp->vp_intercept_msg_page->u.payload;
> > +
> > + gfn = HVPFN_DOWN(msg->guest_physical_address);
> > +
> > + region = mshv_partition_region_by_gfn(p, gfn);
> > + if (!region)
> > + return false;
>
> Does it ever happen that the gfn is legitimately not found in any
> region, perhaps due to a race? I think the vp_mutex is held here,
> so maybe that protects the region layout for the VP and "not found"
> should never occur. If so, should there be a WARN_ON here?
>
> If "gfn not found" can be legitimate, perhaps a comment to
> explain the circumstances would be helpful.
>
This is possible, if hypervisor returns some invalid GFN.
But there is also a possibility, that this code can race with region removal from a guest.
I'll address it in the next revision.
> > +
> > + if (WARN_ON_ONCE(!region->flags.is_ram))
> > + return false;
> > +
> > + if (WARN_ON_ONCE(region->flags.range_pinned))
> > + return false;
> > +
> > + return mshv_region_handle_gfn_fault(region, gfn);
> > +}
> > +
> > +#else /* CONFIG_X86_64 */
> > +
> > +static bool mshv_handle_gpa_intercept(struct mshv_vp *vp) { return false; }
> > +
> > +#endif /* CONFIG_X86_64 */
> > +
> > +static bool mshv_vp_handle_intercept(struct mshv_vp *vp)
> > +{
> > + switch (vp->vp_intercept_msg_page->header.message_type) {
> > + case HVMSG_GPA_INTERCEPT:
> > + return mshv_handle_gpa_intercept(vp);
> > + }
> > + return false;
> > +}
> > +
> > static long mshv_vp_ioctl_run_vp(struct mshv_vp *vp, void __user *ret_msg)
> > {
> > long rc;
> >
> > - if (hv_scheduler_type == HV_SCHEDULER_TYPE_ROOT)
> > - rc = mshv_run_vp_with_root_scheduler(vp);
> > - else
> > - rc = mshv_run_vp_with_hyp_scheduler(vp);
> > + do {
> > + if (hv_scheduler_type == HV_SCHEDULER_TYPE_ROOT)
> > + rc = mshv_run_vp_with_root_scheduler(vp);
> > + else
> > + rc = mshv_run_vp_with_hyp_scheduler(vp);
> > + } while (rc == 0 && mshv_vp_handle_intercept(vp));
> >
> > if (rc)
> > return rc;
> > @@ -1194,6 +1385,110 @@ mshv_partition_region_by_gfn(struct mshv_partition *partition, u64 gfn)
> > return NULL;
> > }
> >
> > +#if defined(CONFIG_MMU_NOTIFIER)
> > +static void mshv_region_movable_fini(struct mshv_mem_region *region)
> > +{
> > + if (region->flags.range_pinned)
> > + return;
> > +
> > + mmu_interval_notifier_remove(®ion->mni);
> > +}
> > +
> > +/**
> > + * mshv_region_interval_invalidate - Invalidate a range of memory region
> > + * @mni: Pointer to the mmu_interval_notifier structure
> > + * @range: Pointer to the mmu_notifier_range structure
> > + * @cur_seq: Current sequence number for the interval notifier
> > + *
> > + * This function invalidates a memory region by remapping its pages with
> > + * no access permissions. It locks the region's mutex to ensure thread safety
> > + * and updates the sequence number for the interval notifier. If the range
> > + * is blockable, it uses a blocking lock; otherwise, it attempts a non-blocking
> > + * lock and returns false if unsuccessful.
> > + *
> > + * NOTE: Failure to invalidate a region is a serious error, as the pages will
> > + * be considered freed while they are still mapped by the hypervisor.
> > + * Any attempt to access such pages will likely crash the system.
> > + *
> > + * Return: true if the region was successfully invalidated, false otherwise.
> > + */
> > +static bool
> > +mshv_region_interval_invalidate(struct mmu_interval_notifier *mni,
> > + const struct mmu_notifier_range *range,
> > + unsigned long cur_seq)
> > +{
> > + struct mshv_mem_region *region = container_of(mni,
> > + struct mshv_mem_region,
> > + mni);
> > + u64 page_offset, page_count;
> > + unsigned long mstart, mend;
> > + int ret = -EPERM;
> > +
> > + if (mmu_notifier_range_blockable(range))
> > + mutex_lock(®ion->mutex);
> > + else if (!mutex_trylock(®ion->mutex))
> > + goto out_fail;
> > +
> > + mmu_interval_set_seq(mni, cur_seq);
> > +
> > + mstart = max(range->start, region->start_uaddr);
> > + mend = min(range->end, region->start_uaddr +
> > + (region->nr_pages << HV_HYP_PAGE_SHIFT));
>
> I'm pretty sure region->start_uaddr is always page aligned. But what
> about range->start and range->end? The code here and below assumes
> they are page aligned. It also assumes that range->end is greater than
> range->start so the computation of page_count doesn't wrap and so
> page_count is >= 1. I don't know whether checks for these assumptions
> are appropriate.
>
There is a check for memory region size to be non-zero and page aligned
in mshv_partition_ioct_set_memory function, which is the only caller for
memory region creation. And region start is defined in PFNs.
Thanks,
Stanislav
^ permalink raw reply
* Re: [PATCH net-next v6 0/2] net: mana: Enforce TX SGE limit and fix error cleanup
From: patchwork-bot+netdevbpf @ 2025-11-20 4:30 UTC (permalink / raw)
To: Aditya Garg
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, longli, kotaranov, horms, shradhagupta, ssengar,
ernis, dipayanroy, shirazsaleem, leon, mlevitsk, yury.norov,
sbhatta, linux-hyperv, netdev, linux-kernel, linux-rdma,
gargaditya
In-Reply-To: <1763464269-10431-1-git-send-email-gargaditya@linux.microsoft.com>
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Tue, 18 Nov 2025 03:11:07 -0800 you wrote:
> Add pre-transmission checks to block SKBs that exceed the hardware's SGE
> limit. Force software segmentation for GSO traffic and linearize non-GSO
> packets as needed.
>
> Update TX error handling to drop failed SKBs and unmap resources
> immediately.
>
> [...]
Here is the summary with links:
- [net-next,v6,1/2] net: mana: Handle SKB if TX SGEs exceed hardware limit
https://git.kernel.org/netdev/net-next/c/934fa943b537
- [net-next,v6,2/2] net: mana: Drop TX skb on post_work_request failure and unmap resources
https://git.kernel.org/netdev/net-next/c/45120304e841
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v3] Drivers: hv: ioctl for self targeted passthrough hvcalls
From: Anirudh Rayabharam @ 2025-11-20 6:13 UTC (permalink / raw)
To: Wei Liu
Cc: K. Y. Srinivasan, Haiyang Zhang, Dexuan Cui, Long Li,
linux-hyperv, linux-kernel
In-Reply-To: <20251119213437.GA2848327@liuwe-devbox-debian-v2.local>
On Wed, Nov 19, 2025 at 09:34:37PM +0000, Wei Liu wrote:
> On Wed, Nov 19, 2025 at 05:17:08PM +0000, Anirudh Rayabharam wrote:
> [...]
> > +static inline bool mshv_passthru_hvcall_allowed(u16 code, u64 pt_id)
>
> There is no need to add inline here. The compiler can decide itself.
>
> I can drop this when committing.
Okay, sure! That works.
Anirudh
>
> Wei
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox