Re: [net-next,v4] vsock: add G2H fallback for CIDs not owned by H2G transport

[Paolo Abeni <pabeni@redhat.com>]

Hi,

On 3/10/26 10:18 AM, Paolo Abeni wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> 
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
>> @@ -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;
> 
> The old code had `!transport_h2g` in the second else-if condition at line
> -583, which routed all CID > VMADDR_CID_HOST traffic to transport_g2h when
> no H2G transport was loaded. The new code removes this check.
> 
> When transport_h2g is NULL (no H2G transport loaded) and the G2H transport
> does not implement has_remote_cid (as is the case for VMCI and Hyper-V
> transports), what happens?
> 
> The G2H fallback branch at line +596 requires
> `transport_g2h->has_remote_cid` to be non-NULL. If it's NULL, the
> short-circuit evaluation skips this branch and executes the final else at
> line +601, setting `new_transport = transport_h2g` (which is NULL),
> ultimately returning -ENODEV.
> 
> The commit message explicitly states the intent to preserve the existing
> no-H2G fallback behavior. Does this implementation break it for G2H
> transports that do not implement the new optional has_remote_cid callback?
> 
> This affects VMCI and Hyper-V environments: when no H2G transport is
> loaded, CID > 2 connections that previously fell back to G2H now get
> ENODEV.
Other comments from AI looked like nit picking to me, but the above one
looks relevant. I forwarded verbatim all the feedback for completeness.

/P