public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Bobby Eshleman <bobbyeshleman@gmail.com>
To: Stefano Garzarella <sgarzare@redhat.com>
Cc: "Shuah Khan" <shuah@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Simon Horman" <horms@kernel.org>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Xuan Zhuo" <xuanzhuo@linux.alibaba.com>,
	"Eugenio Pérez" <eperezma@redhat.com>,
	"K. Y. Srinivasan" <kys@microsoft.com>,
	"Haiyang Zhang" <haiyangz@microsoft.com>,
	"Wei Liu" <wei.liu@kernel.org>,
	"Dexuan Cui" <decui@microsoft.com>,
	"Bryan Tan" <bryan-bt.tan@broadcom.com>,
	"Vishnu Dasa" <vishnu.dasa@broadcom.com>,
	"Broadcom internal kernel review list"
	<bcm-kernel-feedback-list@broadcom.com>,
	virtualization@lists.linux.dev, netdev@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org, linux-hyperv@vger.kernel.org,
	berrange@redhat.com, "Bobby Eshleman" <bobbyeshleman@meta.com>
Subject: Re: [PATCH net-next v8 06/14] vsock/virtio: add netns to virtio transport common
Date: Fri, 7 Nov 2025 07:47:27 -0800	[thread overview]
Message-ID: <aQ4Uj6z129htVqLk@devvm11784.nha0.facebook.com> (raw)
In-Reply-To: <g34g7deirdtzowtpz5pngfpuzvr62u43psmgct34iliu4bhju4@rkrxdy7n2at3>

On Fri, Nov 07, 2025 at 04:07:39PM +0100, Stefano Garzarella wrote:
> On Fri, Nov 07, 2025 at 06:33:33AM -0800, Bobby Eshleman wrote:
> > > > > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> > > > > index dcc8a1d5851e..b8e52c71920a 100644
> > > > > --- a/net/vmw_vsock/virtio_transport_common.c
> > > > > +++ b/net/vmw_vsock/virtio_transport_common.c
> > > > > @@ -316,6 +316,15 @@ static struct sk_buff *virtio_transport_alloc_skb(struct virtio_vsock_pkt_info *
> > > > > 					 info->flags,
> > > > > 					 zcopy);
> > > > >
> > > > > +	/*
> > > > > +	 * If there is no corresponding socket, then we don't have a
> > > > > +	 * corresponding namespace. This only happens For VIRTIO_VSOCK_OP_RST.
> > > > > +	 */
> > > >
> > > > So, in virtio_transport_recv_pkt() should we check that `net` is not set?
> > > >
> > > > Should we set it to NULL here?
> > > >
> > > 
> > > Sounds good to me.
> > > 
> > > > > +	if (vsk) {
> > > > > +		virtio_vsock_skb_set_net(skb, info->net);
> > > >
> > > > Ditto here about the net refcnt, can the net disappear?
> > > > Should we use get_net() in some way, or the socket will prevent that?
> > > >
> > > 
> > > As long as the socket has an outstanding skb it can't be destroyed and
> > > so will have a reference to the net, that is after skb_set_owner_w() and
> > > freeing... so I think this is okay.
> > > 
> > > But, maybe we could simplify the implied relationship between skb, sk,
> > > and net by removing the VIRTIO_VSOCK_SKB_CB(skb)->net entirely, and only
> > > ever referring to sock_net(skb->sk)? I remember originally having a
> > > reason for adding it to the cb, but my hunch is it that it was probably
> > > some confusion over the !vsk case.
> > > 
> > > WDYT?
> > > 
> > 
> > ... now I remember the reason, because I didn't want two different
> > places for storing the net for RX and TX.
> 
> Yeah, but if we can reuse skb->sk for one path and pass it as parameter to
> the other path (see my prev email), why store it?
> 
> Or even in the TX maybe it can be passed to .send_pkt() in some way, e.g.
> storing it in struct virtio_vsock_sock instead that for each skb.
> 
> Stefano
> 

That's a good point, the rx path only needs to pass to recv_pkt(), it is
not needed after the socket lookup there.

With TX, it does look like we could get rid of it via the
virtio_vsock_sock.

Best,
Bobby

  reply	other threads:[~2025-11-07 15:47 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-23 18:27 [PATCH net-next v8 00/14] vsock: add namespace support to vhost-vsock Bobby Eshleman
2025-10-23 18:27 ` [PATCH net-next v8 01/14] vsock: a per-net vsock NS mode state Bobby Eshleman
2025-11-06 16:16   ` Stefano Garzarella
2025-11-07  1:09     ` Bobby Eshleman
2025-10-23 18:27 ` [PATCH net-next v8 02/14] vsock/virtio: pack struct virtio_vsock_skb_cb Bobby Eshleman
2025-11-06 16:16   ` Stefano Garzarella
2025-10-23 18:27 ` [PATCH net-next v8 03/14] vsock: add netns to vsock skb cb Bobby Eshleman
2025-11-06 16:17   ` Stefano Garzarella
2025-10-23 18:27 ` [PATCH net-next v8 04/14] vsock: add netns to vsock core Bobby Eshleman
2025-11-06 16:18   ` Stefano Garzarella
2025-11-07  2:03     ` Bobby Eshleman
2025-11-07 13:53       ` Stefano Garzarella
2025-10-23 18:27 ` [PATCH net-next v8 05/14] vsock/loopback: add netns support Bobby Eshleman
2025-11-06 16:18   ` Stefano Garzarella
2025-11-07  2:17     ` Bobby Eshleman
2025-10-23 18:27 ` [PATCH net-next v8 06/14] vsock/virtio: add netns to virtio transport common Bobby Eshleman
2025-11-06 16:20   ` Stefano Garzarella
2025-11-07  2:52     ` Bobby Eshleman
2025-11-07 14:30       ` Stefano Garzarella
2025-11-07 14:33       ` Bobby Eshleman
2025-11-07 15:07         ` Stefano Garzarella
2025-11-07 15:47           ` Bobby Eshleman [this message]
2025-10-23 18:27 ` [PATCH net-next v8 07/14] vhost/vsock: add netns support Bobby Eshleman
2025-11-06 16:21   ` Stefano Garzarella
2025-11-07  3:07     ` Bobby Eshleman
2025-10-23 18:27 ` [PATCH net-next v8 08/14] selftests/vsock: add namespace helpers to vmtest.sh Bobby Eshleman
2025-10-23 18:27 ` [PATCH net-next v8 09/14] selftests/vsock: prepare vm management helpers for namespaces Bobby Eshleman
2025-10-23 18:27 ` [PATCH net-next v8 10/14] selftests/vsock: add tests for proc sys vsock ns_mode Bobby Eshleman
2025-10-23 18:27 ` [PATCH net-next v8 11/14] selftests/vsock: add namespace tests for CID collisions Bobby Eshleman
2025-10-23 18:27 ` [PATCH net-next v8 12/14] selftests/vsock: add tests for host <-> vm connectivity with namespaces Bobby Eshleman
2025-10-23 18:27 ` [PATCH net-next v8 13/14] selftests/vsock: add tests for namespace deletion and mode changes Bobby Eshleman
2025-10-23 18:27 ` [PATCH net-next v8 14/14] selftests/vsock: add tests for module loading order Bobby Eshleman
2025-10-27 13:28 ` [PATCH net-next v8 00/14] vsock: add namespace support to vhost-vsock Stefano Garzarella
2025-10-27 17:25   ` Bobby Eshleman
2025-11-06 16:23     ` Stefano Garzarella
2025-11-07  1:00       ` Bobby Eshleman

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=aQ4Uj6z129htVqLk@devvm11784.nha0.facebook.com \
    --to=bobbyeshleman@gmail.com \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=berrange@redhat.com \
    --cc=bobbyeshleman@meta.com \
    --cc=bryan-bt.tan@broadcom.com \
    --cc=davem@davemloft.net \
    --cc=decui@microsoft.com \
    --cc=edumazet@google.com \
    --cc=eperezma@redhat.com \
    --cc=haiyangz@microsoft.com \
    --cc=horms@kernel.org \
    --cc=jasowang@redhat.com \
    --cc=kuba@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=kys@microsoft.com \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sgarzare@redhat.com \
    --cc=shuah@kernel.org \
    --cc=stefanha@redhat.com \
    --cc=virtualization@lists.linux.dev \
    --cc=vishnu.dasa@broadcom.com \
    --cc=wei.liu@kernel.org \
    --cc=xuanzhuo@linux.alibaba.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