On Sat, Oct 10, 2026 at 10:22:43PM +0800, Jia Jia wrote:
> Split virtio_transport_recv_pkt() so socket lookup is separate from the
> receive state machine that runs under the socket lock.
>
> Keep virtio_transport_recv_pkt() on its existing per-packet locking path.
> Pass the source and destination addresses used for lookup to the locked
> receive path so they are decoded only once.
>
> Pass the network namespace and decoded address tuple through a packet
> context to the locked receive path.
>
> Preserve source validation and failure handling. Keep the lookup reference
> separate from the skb owner reference so the locked receive path can manage
> the two lifetimes independently.
>
> Signed-off-by: Jia Jia <[email protected]>
> ---
> net/vmw_vsock/virtio_transport_common.c | 139 ++++++++++++++++--------
> 1 file changed, 96 insertions(+), 43 deletions(-)
>
[...]
> +/*
> + * The caller holds sk's socket lock and must free skb if this returns true.
> + */
> +static bool
> +virtio_transport_recv_pkt_locked(struct virtio_transport *t,
> + struct sk_buff *skb, struct sock *sk,
> + const struct virtio_transport_rx_pkt_ctx *ctx)
> +{
> + const struct sockaddr_vm *src = ctx->src;
> + const struct sockaddr_vm *dst = ctx->dst;
> + struct vsock_sock *vsk = vsock_sk(sk);
> + struct net *net = ctx->net;
> + bool space_available;
>
> - /* Check if sk has been closed or assigned to another transport before
> - * lock_sock (note: listener sockets are not assigned to any transport)
> + /* Check after acquiring the socket lock. Listener sockets accept
> packets
> + * from any source and are not assigned to a transport.
> */
> if (sock_flag(sk, SOCK_DONE) ||
> (sk->sk_state != TCP_LISTEN &&
> - !vsock_check_source(vsk, &t->transport, &src))) {
> + !vsock_check_source(vsk, &t->transport, src))) {
> (void)virtio_transport_reset_no_sock(t, skb, net);
> - release_sock(sk);
> - sock_put(sk);
> - goto free_pkt;
> + return true;
> }
>
> space_available = virtio_transport_space_update(sk, skb);
>
> /* Update CID in case it has changed after a transport reset event */
> if (vsk->local_addr.svm_cid != VMADDR_CID_ANY)
> - vsk->local_addr.svm_cid = dst.svm_cid;
> + vsk->local_addr.svm_cid = dst->svm_cid;
>
> if (space_available)
> sk->sk_write_space(sk);
> @@ -1875,12 +1891,49 @@ void virtio_transport_recv_pkt(struct
> virtio_transport *t,
> break;
> }
>
> + return false;
> +}
> +
> +/* We are under the virtio-vsock's vsock->rx_lock or vhost-vsock's vq->mutex
> + * lock.
> + */
> +void virtio_transport_recv_pkt(struct virtio_transport *t,
> + struct sk_buff *skb, struct net *net)
> +{
> + struct virtio_transport_rx_pkt_ctx ctx;
> + struct sockaddr_vm src, dst;
> + struct sock *sk;
> + bool free_pkt;
> +
> + virtio_transport_recv_pkt_init_addrs(skb, &src, &dst);
> + virtio_transport_trace_recv_pkt(skb, &src, &dst);
> +
> + sk = virtio_transport_recv_pkt_find_socket(skb, &src, &dst, net);
> + if (!sk) {
> + (void)virtio_transport_reset_no_sock(t, skb, net);
> + goto free_pkt;
> + }
> +
> + if (!skb_set_owner_sk_safe(skb, sk)) {
> + WARN_ONCE(1, "receiving vsock socket has sk_refcnt == 0\n");
> + goto free_pkt;
> + }
> +
> + lock_sock(sk);
> + ctx = (struct virtio_transport_rx_pkt_ctx) {
> + .net = net,
> + .src = &src,
> + .dst = &dst,
> + };
> + free_pkt = virtio_transport_recv_pkt_locked(t, skb, sk, &ctx);
At this point the socket lookup succeeded with src/dst/net, why not just
pass sk which already has all of them?
Best,
Bobby
