Re: [GIT PULL] Networking for v6.17

[Christian Brauner <brauner@kernel.org>]

On Wed, Jul 30, 2025 at 09:20:46AM -0700, Linus Torvalds wrote:
> On Sat, 26 Jul 2025 at 18:35, Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > Networking changes for 6.17.
> 
> So while merging this, there was a trivial conflict with commit
> 9b0240b3ccc3 ("netns: use stable inode number for initial mount ns")
> from the vfs side (acked by networking people).
> 
> And the conflict wasn't hard to resolve, but while looking at it, I
> got very unhappy with that conflicting commit from the vfs tree.
> 
> Christian - when the "use stable inode number" code triggers, it
> bypasses ns_alloc_inum() entirely. Fine - except that function *also*
> does that
> 
>         WRITE_ONCE(ns->stashed, NULL);
> 
> so now ns->stashed isn't initialized any more.
> 
> Now, that shouldn't matter here because this only triggers for
> 'init_net' that is a global data structure and thus initialized to all
> zeroes anyway, but it makes me very unhappy about that pattern that
> ends up being about allocating the pid, but also almost incidentally
> initializing that 'stashed' entry.
> 
> I ended up re-organizing the net_ns_net_init() code a bit (because it
> now does that debugfs setup on success, so the old "return 0" didn't
> work), and I think the merge is fine, but I think this "don't call
> ns_alloc_inum()" pattern is wrong.
> 
> IOW, I don't think this is a bug, but I think it's not great.

I think we should not be initializing ns->stashed in ns_alloc_inum().
The function name is already wrong for that purpose:

static inline int ns_alloc_inum(struct ns_common *ns)
{
	WRITE_ONCE(ns->stashed, NULL);
	return proc_alloc_inum(&ns->inum);
}

That was done a long time ago via atomic_long_set() and I just changed
it to WRITE_ONCE() when I reworked both nsfs and pidfs.

We let all callers initialize the fields of struct ns_common embedded in
their respective namespace types already. I see no reason to not just do
the same thing for ns->stashed and drop that implicit initialization
from ns_alloc_inum().

But aside from that I think my patch should have probably been:

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 1b6f3826dd0e..5c39fb544f93 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -815,7 +815,6 @@ static __net_init int net_ns_net_init(struct net *net)
 #ifdef CONFIG_NET_NS
        net->ns.ops = &netns_operations;
 #endif
-       net->ns.inum = PROC_NET_INIT_INO;
        if (net != &init_net) {
                int ret = ns_alloc_inum(&net->ns);
                if (ret)
@@ -1283,6 +1282,8 @@ void __init net_ns_init(void)
        init_net.key_domain = &init_net_key_domain;
 #endif
        preinit_net(&init_net, &init_user_ns);
+       init_net.ns.inum = PROC_NET_INIT_INO;
+       init_net.ns.stashed = NULL;

        down_write(&pernet_ops_rwsem);
        if (setup_net(&init_net))

so the setup for the initial network namespce happens right where it is
explicitly initialized.