From: Paolo Abeni <pabeni@redhat.com>
To: Stefano Garzarella <sgarzare@redhat.com>,
Alexander Graf <graf@amazon.com>,
mst@redhat.com, kuba@kernel.org
Cc: virtualization@lists.linux.dev, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, kvm@vger.kernel.org, eperezma@redhat.com,
Jason Wang <jasowang@redhat.com>,
Stefan Hajnoczi <stefanha@redhat.com>,
bcm-kernel-feedback-list@broadcom.com,
Arnd Bergmann <arnd@arndb.de>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jonathan Corbet <corbet@lwn.net>,
Bryan Tan <bryan-bt.tan@broadcom.com>,
Vishnu Dasa <vishnu.dasa@broadcom.com>,
nh-open-source@amazon.com, syzbot@syzkaller.appspotmail.com
Subject: Re: [PATCH net-next v4] vsock: add G2H fallback for CIDs not owned by H2G transport
Date: Tue, 10 Mar 2026 10:30:49 +0100 [thread overview]
Message-ID: <8ae37965-ddc8-4ab0-aa95-0de17edf1a3e@redhat.com> (raw)
In-Reply-To: <aalQditGLRMrlyV7@sgarzare-redhat>
On 3/5/26 10:51 AM, Stefano Garzarella wrote:
> On Wed, Mar 04, 2026 at 11:00:27PM +0000, Alexander Graf wrote:
>> When no H2G transport is loaded, vsock currently routes all CIDs to the
>> G2H transport (commit 65b422d9b61b ("vsock: forward all packets to the
>> host when no H2G is registered"). Extend that existing behavior: when
>> an H2G transport is loaded but does not claim a given CID, the
>> connection falls back to G2H in the same way.
>>
>> This matters in environments like Nitro Enclaves, where an instance may
>> run nested VMs via vhost-vsock (H2G) while also needing to reach sibling
>> enclaves at higher CIDs through virtio-vsock-pci (G2H). With the old
>> code, any CID > 2 was unconditionally routed to H2G when vhost was
>> loaded, making those enclaves unreachable without setting
>> VMADDR_FLAG_TO_HOST explicitly on every connect.
>>
>> Requiring every application to set VMADDR_FLAG_TO_HOST creates friction:
>> tools like socat, iperf, and others would all need to learn about it.
>> The flag was introduced 6 years ago and I am still not aware of any tool
>> that supports it. Even if there was support, it would be cumbersome to
>> use. The most natural experience is a single CID address space where H2G
>> only wins for CIDs it actually owns, and everything else falls through to
>> G2H, extending the behavior that already exists when H2G is absent.
>>
>> To give user space at least a hint that the kernel applied this logic,
>> automatically set the VMADDR_FLAG_TO_HOST on the remote address so it
>> can determine the path taken via getpeername().
>>
>> Add a per-network namespace sysctl net.vsock.g2h_fallback (default 1).
>> At 0 it forces strict routing: H2G always wins for CID > VMADDR_CID_HOST,
>> or ENODEV if H2G is not loaded.
>>
>> Signed-off-by: Alexander Graf <graf@amazon.com>
>> Tested-by: syzbot@syzkaller.appspotmail.com
>>
>> ---
>>
>> v1 -> v2:
>>
>> - Rebase on 7.0, include namespace support
>> - Add net.vsock.g2h_fallback sysctl
>> - Rework description
>> - Set VMADDR_FLAG_TO_HOST automatically
>> - Add VMCI support
>> - Update vsock_assign_transport() comment
>>
>> v2 -> v3:
>>
>> - Use has_remote_cid() on G2H transport to gate the fallback. This is
>> used by VMCI to indicate that it never takes G2H CIDs > 2.
>> - Move g2h_fallback into struct netns_vsock to enable namespaces
>> and fix syzbot warning
>> - Gate the !transport_h2g case on g2h_fallback as well, folding the
>> pre-existing no-H2G fallback into the new logic
>> - Remove has_remote_cid() from VMCI again. Instead implement it in
>> virtio.
>>
>> v3 -> v4:
>>
>> - Fix commit reference format (checkpatch)
>> - vhost: use !!vhost_vsock_get() instead of != NULL (checkpatch)
>> - Add braces around final else branch (checkpatch)
>> - Replace 'vhost' with 'H2G transport' (Stefano)
>> ---
>> Documentation/admin-guide/sysctl/net.rst | 28 +++++++++++++++++++
>> drivers/vhost/vsock.c | 13 +++++++++
>> include/net/af_vsock.h | 9 ++++++
>> include/net/netns/vsock.h | 2 ++
>> net/vmw_vsock/af_vsock.c | 35 ++++++++++++++++++++----
>> net/vmw_vsock/virtio_transport.c | 7 +++++
>> 6 files changed, 89 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/sysctl/net.rst b/Documentation/admin-guide/sysctl/net.rst
>> index 3b2ad61995d4..0724a793798f 100644
>> --- a/Documentation/admin-guide/sysctl/net.rst
>> +++ b/Documentation/admin-guide/sysctl/net.rst
>> @@ -602,3 +602,31 @@ it does not modify the current namespace or any existing children.
>>
>> A namespace with ``ns_mode`` set to ``local`` cannot change
>> ``child_ns_mode`` to ``global`` (returns ``-EPERM``).
>> +
>> +g2h_fallback
>> +------------
>> +
>> +Controls whether connections to CIDs not owned by the host-to-guest (H2G)
>> +transport automatically fall back to the guest-to-host (G2H) transport.
>> +
>> +When enabled, if a connect targets a CID that the H2G transport (e.g.
>> +vhost-vsock) does not serve, or if no H2G transport is loaded at all, the
>> +connection is routed via the G2H transport (e.g. virtio-vsock) instead. This
>> +allows a host running both nested VMs (via vhost-vsock) and sibling VMs
>> +reachable through the hypervisor (e.g. Nitro Enclaves) to address both using
>> +a single CID space, without requiring applications to set
>> +``VMADDR_FLAG_TO_HOST``.
>> +
>> +When the fallback is taken, ``VMADDR_FLAG_TO_HOST`` is automatically set on
>> +the remote address so that userspace can determine the path via
>> +``getpeername()``.
>> +
>> +Note: With this sysctl enabled, user space that attempts to talk to a guest
>> +CID which is not implemented by the H2G transport will create host vsock
>> +traffic. Environments that rely on H2G-only isolation should set it to 0.
>> +
>> +Values:
>> +
>> + - 0 - Connections to CIDs <= 2 or with VMADDR_FLAG_TO_HOST use G2H;
>> + all others use H2G (or fail with ENODEV if H2G is not loaded).
>> + - 1 - Connections to CIDs not owned by H2G fall back to G2H. (default)
>> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>> index 054f7a718f50..1d8ec6bed53e 100644
>> --- a/drivers/vhost/vsock.c
>> +++ b/drivers/vhost/vsock.c
>> @@ -91,6 +91,18 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid, struct net *net)
>> return NULL;
>> }
>>
>> +static bool vhost_transport_has_remote_cid(struct vsock_sock *vsk, u32 cid)
>> +{
>> + struct sock *sk = sk_vsock(vsk);
>> + struct net *net = sock_net(sk);
>> + bool found;
>> +
>> + rcu_read_lock();
>> + found = !!vhost_vsock_get(cid, net);
>> + rcu_read_unlock();
>> + return found;
>> +}
>> +
>> static void
>> vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
>> struct vhost_virtqueue *vq)
>> @@ -424,6 +436,7 @@ static struct virtio_transport vhost_transport = {
>> .module = THIS_MODULE,
>>
>> .get_local_cid = vhost_transport_get_local_cid,
>> + .has_remote_cid = vhost_transport_has_remote_cid,
>>
>> .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 533d8e75f7bb..4e40063adab4 100644
>> --- a/include/net/af_vsock.h
>> +++ b/include/net/af_vsock.h
>> @@ -179,6 +179,15 @@ struct vsock_transport {
>> /* Addressing. */
>> u32 (*get_local_cid)(void);
>>
>> + /* Check if this transport serves a specific remote CID.
>> + * For H2G transports: return true if the CID belongs to a registered
>> + * guest. If not implemented, all CIDs > VMADDR_CID_HOST go to H2G.
>> + * For G2H transports: return true if the transport can reach arbitrary
>> + * CIDs via the hypervisor (i.e. supports the fallback overlay). VMCI
>> + * does not implement this as it only serves CIDs 0 and 2.
>> + */
>> + bool (*has_remote_cid)(struct vsock_sock *vsk, u32 remote_cid);
>> +
>> /* Read a single skb */
>> int (*read_skb)(struct vsock_sock *, skb_read_actor_t);
>>
>> diff --git a/include/net/netns/vsock.h b/include/net/netns/vsock.h
>> index dc8cbe45f406..7f84aad92f57 100644
>> --- a/include/net/netns/vsock.h
>> +++ b/include/net/netns/vsock.h
>> @@ -20,5 +20,7 @@ struct netns_vsock {
>>
>> /* 0 = unlocked, 1 = locked to global, 2 = locked to local */
>> int child_ns_mode_locked;
>> +
>> + int g2h_fallback;
>> };
>> #endif /* __NET_NET_NAMESPACE_VSOCK_H */
>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>> index 2f7d94d682cb..50843a977878 100644
>> --- a/net/vmw_vsock/af_vsock.c
>> +++ b/net/vmw_vsock/af_vsock.c
>> @@ -545,9 +545,13 @@ static void vsock_deassign_transport(struct vsock_sock *vsk)
>> * The vsk->remote_addr is used to decide which transport to use:
>> * - remote CID == VMADDR_CID_LOCAL or g2h->local_cid or VMADDR_CID_HOST if
>> * g2h is not loaded, will use local transport;
>> - * - remote CID <= VMADDR_CID_HOST or h2g is not loaded or remote flags field
>> - * includes VMADDR_FLAG_TO_HOST flag value, will use guest->host transport;
>> - * - remote CID > VMADDR_CID_HOST will use host->guest transport;
>> + * - remote CID <= VMADDR_CID_HOST or remote flags field includes
>> + * VMADDR_FLAG_TO_HOST, will use guest->host transport;
>> + * - remote CID > VMADDR_CID_HOST and h2g is loaded and h2g claims that CID,
>> + * will use host->guest transport;
>> + * - h2g not loaded or h2g does not claim that CID and g2h claims the CID via
>> + * has_remote_cid, will use guest->host transport (when g2h_fallback=1)
>> + * - anything else goes to h2g or returns -ENODEV if no h2g is available
>> */
>> int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
>> {
>> @@ -581,11 +585,21 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
>> case SOCK_SEQPACKET:
>> if (vsock_use_local_transport(remote_cid))
>> new_transport = transport_local;
>> - else if (remote_cid <= VMADDR_CID_HOST || !transport_h2g ||
>> + else if (remote_cid <= VMADDR_CID_HOST ||
>> (remote_flags & VMADDR_FLAG_TO_HOST))
>> new_transport = transport_g2h;
>> - else
>> + else if (transport_h2g &&
>> + (!transport_h2g->has_remote_cid ||
>> + transport_h2g->has_remote_cid(vsk, remote_cid)))
>> + new_transport = transport_h2g;
>> + else if (sock_net(sk)->vsock.g2h_fallback &&
>> + transport_g2h && transport_g2h->has_remote_cid &&
>> + transport_g2h->has_remote_cid(vsk, remote_cid)) {
>> + vsk->remote_addr.svm_flags |= VMADDR_FLAG_TO_HOST;
>> + new_transport = transport_g2h;
>> + } else {
>> new_transport = transport_h2g;
>> + }
>> break;
>> default:
>> ret = -ESOCKTNOSUPPORT;
>> @@ -2879,6 +2893,15 @@ static struct ctl_table vsock_table[] = {
>> .mode = 0644,
>> .proc_handler = vsock_net_child_mode_string
>> },
>> + {
>> + .procname = "g2h_fallback",
>> + .data = &init_net.vsock.g2h_fallback,
>> + .maxlen = sizeof(int),
>> + .mode = 0644,
>> + .proc_handler = proc_dointvec_minmax,
>> + .extra1 = SYSCTL_ZERO,
>> + .extra2 = SYSCTL_ONE,
>> + },
>> };
>>
>> static int __net_init vsock_sysctl_register(struct net *net)
>> @@ -2894,6 +2917,7 @@ static int __net_init vsock_sysctl_register(struct net *net)
>>
>> table[0].data = &net->vsock.mode;
>> table[1].data = &net->vsock.child_ns_mode;
>> + table[2].data = &net->vsock.g2h_fallback;
>> }
>>
>> net->vsock.sysctl_hdr = register_net_sysctl_sz(net, "net/vsock", table,
>> @@ -2928,6 +2952,7 @@ static void vsock_net_init(struct net *net)
>> net->vsock.mode = vsock_net_child_mode(current->nsproxy->net_ns);
>>
>> net->vsock.child_ns_mode = net->vsock.mode;
>> + net->vsock.g2h_fallback = 1;
>
> My last concern is what I mentioned in v3 [1].
> Let me quote it here as well:
>
> @Michael @Paolo @Jakub
> I don't know what the sysctl policy is in general in net or virtio.
> Is this fine or should we inherit this from the parent and set the
> default only for init_ns?
AFAICT, there is no geneal policy; depending on the specific value it
should be inherited or be available to configuration on per netns case.
Usually the inherited values are constraints to system-wide resources,
i.e. max memory allocated.
In this specific case I feel like allowing per netns configuration is
correct.
/P
next prev parent reply other threads:[~2026-03-10 9:30 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-04 23:00 [PATCH net-next v4] vsock: add G2H fallback for CIDs not owned by H2G transport Alexander Graf
2026-03-05 9:51 ` Stefano Garzarella
2026-03-10 9:30 ` Paolo Abeni [this message]
2026-03-10 9:18 ` [net-next,v4] " Paolo Abeni
2026-03-10 9:26 ` Paolo Abeni
2026-03-10 11:07 ` [net-next, v4] " Alexander Graf
2026-03-12 10:10 ` [PATCH net-next " patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8ae37965-ddc8-4ab0-aa95-0de17edf1a3e@redhat.com \
--to=pabeni@redhat.com \
--cc=arnd@arndb.de \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=bryan-bt.tan@broadcom.com \
--cc=corbet@lwn.net \
--cc=eperezma@redhat.com \
--cc=graf@amazon.com \
--cc=gregkh@linuxfoundation.org \
--cc=jasowang@redhat.com \
--cc=kuba@kernel.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mst@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=nh-open-source@amazon.com \
--cc=sgarzare@redhat.com \
--cc=stefanha@redhat.com \
--cc=syzbot@syzkaller.appspotmail.com \
--cc=virtualization@lists.linux.dev \
--cc=vishnu.dasa@broadcom.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox