All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bobby Eshleman <bobbyeshleman@gmail.com>
To: Simon Horman <horms@kernel.org>
Cc: "Stefano Garzarella" <sgarzare@redhat.com>,
	"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>,
	"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 RFC net-next v4 09/12] vsock/loopback: add netns support
Date: Wed, 6 Aug 2025 14:31:12 -0700	[thread overview]
Message-ID: <aJPJoDo/0MS+thcl@devvm6216.cco0.facebook.com> (raw)
In-Reply-To: <20250806191239.GF61519@horms.kernel.org>

On Wed, Aug 06, 2025 at 08:12:39PM +0100, Simon Horman wrote:
> On Tue, Aug 05, 2025 at 02:49:17PM -0700, Bobby Eshleman wrote:
> > From: Bobby Eshleman <bobbyeshleman@meta.com>
> > 

...

> 
> This change needs to be squashed into
> PATCH 3/12 vsock: add netns to af_vsock core
> 
> To avoid build breakage.
> 
> Likewise with the other change to vsock_loopback_seqpacket_allow below.
> And I think also for a number of other changes made by PATCH 3/12.
> 
> Please make sure that patches don't introduce transient build failures.
> It breaks bisection.

Will do, thanks!

> 
> 
> On the topic of vsock_loopback_seqpacket_allow, also:
> 
> * Please line wrap this so that the code is 80 columns wide or less,
>   as is still preferred for Networking code.
> 
>   Flagged by checkpatch.pl --max-line-length=80
> 
> * Can we move the definition of vsock_loopback_seqpacket_allow() here?
>   The function itself is is trivial. And doing so would avoid a forward
>   declaration.
> 
> >  static bool vsock_loopback_msgzerocopy_allow(void)
> >  {
> >  	return true;
> 
> ...
> 
> > +int vsock_loopback_init_net(struct net *net)
> > +{
> > +	net->vsock.loopback = kmalloc(GFP_KERNEL, sizeof(struct vsock_loopback));
> > +	if (!net->vsock.loopback)
> > +		return -ENOMEM;
> > +
> > +	return vsock_loopback_init_vsock(net->vsock.loopback);
> > +}
> > +
> > +void vsock_loopback_exit_net(struct net *net)
> > +{
> > +	vsock_loopback_deinit_vsock(net->vsock.loopback);
> > +	kfree(net->vsock.loopback);
> > +}
> 
> I think EXPORT_SYMBOL_GPL is needed for both vsock_loopback_exit_net and
> vsock_loopback_init_net for the case where CONFIG_VSOCKETS=m
> 
> Also, in Kconfig VSOCKETS_LOOPBACK depends on VSOCKETS. But this code adds
> a reverse dependency. As it stands it's possible to configure VSOCKETS
> without VSOCKETS_LOOPBACK, which will not compile.
> 
> Perhaps stub implementations of vsock_loopback_init_net and
> vsock_loopback_exit_net should be implemented in af_vsock.h if
> VSOCKETS_LOOPBACK is not enabled?
> 

Roger that, makes sense. Thanks for the review!

Best,
Bobby

  reply	other threads:[~2025-08-06 21:31 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-05 21:49 [PATCH RFC net-next v4 00/12] vsock: add namespace support to vhost-vsock Bobby Eshleman
2025-08-05 21:49 ` [PATCH RFC net-next v4 01/12] vsock: a per-net vsock NS mode state Bobby Eshleman
2025-08-05 21:49 ` [PATCH RFC net-next v4 02/12] vsock: add net to vsock skb cb Bobby Eshleman
2025-08-05 21:49 ` [PATCH RFC net-next v4 03/12] vsock: add netns to af_vsock core Bobby Eshleman
2025-08-07  1:35   ` kernel test robot
2025-08-05 21:49 ` [PATCH RFC net-next v4 04/12] vsock/virtio: add netns to virtio transport common Bobby Eshleman
2025-08-05 21:49 ` [PATCH RFC net-next v4 05/12] vhost/vsock: add netns support Bobby Eshleman
2025-08-05 21:49 ` [PATCH RFC net-next v4 06/12] vsock/virtio: use the global netns Bobby Eshleman
2025-08-05 21:49 ` [PATCH RFC net-next v4 07/12] hv_sock: add netns hooks Bobby Eshleman
2025-08-05 21:49 ` [PATCH RFC net-next v4 08/12] vsock/vmci: " Bobby Eshleman
2025-08-05 21:49 ` [PATCH RFC net-next v4 09/12] vsock/loopback: add netns support Bobby Eshleman
2025-08-06 19:12   ` Simon Horman
2025-08-06 21:31     ` Bobby Eshleman [this message]
2025-08-07  4:13   ` kernel test robot
2025-08-05 21:49 ` [PATCH RFC net-next v4 10/12] selftests/vsock: improve logging in vmtest.sh Bobby Eshleman
2025-08-05 21:49 ` [PATCH RFC net-next v4 11/12] selftests/vsock: invoke vsock_test through helpers Bobby Eshleman
2025-08-05 21:49 ` [PATCH RFC net-next v4 12/12] selftests/vsock: add namespace tests Bobby Eshleman
2025-08-05 22:03 ` [PATCH RFC net-next v4 00/12] vsock: add namespace support to vhost-vsock Bobby Eshleman
2025-08-06 19:13   ` Simon Horman
2025-08-06 21:31     ` Bobby Eshleman
2025-08-07  8:06 ` Stefano Garzarella
2025-08-07 20:24   ` 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=aJPJoDo/0MS+thcl@devvm6216.cco0.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.