From: "NeilBrown" <neilb@xxxxxxx>
Date: Tue, 12 Nov 2024 10:52:34 +1100
> > diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> > index 6f272013fd9b..d4330aaadc23 100644
> > --- a/net/sunrpc/svcsock.c
> > +++ b/net/sunrpc/svcsock.c
> > @@ -1551,6 +1551,10 @@ static struct svc_xprt *svc_create_socket(struct svc_serv *serv,
> > newlen = error;
> >
> > if (protocol == IPPROTO_TCP) {
> > + __netns_tracker_free(net, &sock->sk->ns_tracker, false);
> > + sock->sk->sk_net_refcnt = 1;
> > + get_net_track(net, &sock->sk->ns_tracker, GFP_KERNEL);
> > + sock_inuse_add(net, 1);
>
> This is really ugly. These internal details of the network layer have
> no place in sunrpc code. There must be a better way.
I asked to do this way. I agree this way is really ugly. Similar
code exists in MPTCP, SMC, CIFS, etc, so I plan to add a new API for
this case, but this requires huge change adding a new parameter for
->create() prototype and the changes are not backportable.
https://github.com/q2ven/linux/commit/bb8b8814a73b3f50c3fef5eaf8d30d8c1df43e7b
https://github.com/q2ven/linux/commits/427_2
After my series, we can use the following but cannot backport it to
stable.
sock_create_net(net, family, type, protocol);
e.g. commit for MPTCP
https://github.com/q2ven/linux/commit/24a4647561272c1e67a685d8403e27eb863398cf
That's why I suggested to go with the ugly way and I will clean them
up in the next cycle.
So, finally the sunrpc code will be much cleaner and the netns refcnt
will be touched only in the core code.
>
> Can we pass '0' for the kern arg to __sock_create()? That should fix
> the refcounting issues, but might mess up security labelling.
This should be avoided as it's confusing for BPF programs, LSMs, and
LOCKDEP.
>
> Can we wait for something before we call put_net() to release the net.
>
> Maybe we want to split the "kern" arg t __sock_create() and have
> "kern" which affects labeling and "refnet" with affects refcounting the
> net.
This is exactly what my series does, but again, it's not backport
friendly.
https://github.com/q2ven/linux/commit/413e867b4aee9e9f60f3c33fb38d2004aeb29c40
>
> I had a quick look and very nearly every caller of __sock_create()
> outside of net/core really does want refcount. Many callers of
> sock_create_kern() possibly don't.
Actually, since sock_create_kern() is added, we no longer need to
export __sock_create(), so I have a patch to convert them to
sock_create_kern().
And most of TCP socket does need refcnt, but non-TCP won't.
Also, handshake one is exception, which uses TCP but only in init_net,
where we need not take care of netns refcnt.
https://github.com/q2ven/linux/commit/b56888bbbf327d57ea25a6b97275d6b9b8ad043a
>
> So I really think this needs to be cleaned up in net/core, not in all
> the different network clients in the kernel.
Yes, will be done in the next cycle.
