Re: [PATCH v2 4/8] SUNRPC: undo partial rpcbind registrations when svc_register() fails

[Jeff Layton <jlayton@kernel.org>]

On Tue, 2026-08-11 at 15:19 -0400, Chuck Lever wrote:
> 
> On Tue, Aug 11, 2026, at 8:03 AM, Jeff Layton wrote:
> > svc_register() registers each [program, version] in turn. On failure the
> > caller tears the listener down -- svc_setup_socket() frees the svc_sock and
> > svc_create_socket() releases the socket -- but the entries that were already
> > set stay in rpcbind, now pointing at a closed port. XPT_RPCB_UNREG is set
> > later, in svc_udp_init()/svc_tcp_init(), so svc_delete_xprt() never runs for
> > this transport and nothing clears them.
> > 
> > Unwind on failure, and stop the walk there rather than registering the
> > programs after it. rpcbind matches RPCBPROC_UNSET on
> > [program, version, netid] and ignores the address, so the unwind clears the
> > netid, not just the port -- the same granularity svc_delete_xprt() already
> > unregisters at.
> > 
> > Assisted-by: LLM
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  net/sunrpc/svc.c | 31 ++++++++++++++++++++++++++++++-
> >  1 file changed, 30 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> > index 4f402bbf97ba..9ffa87007004 100644
> > --- a/net/sunrpc/svc.c
> > +++ b/net/sunrpc/svc.c
> > @@ -1183,6 +1183,29 @@ int svc_generic_rpcbind_set(struct net *net,
> >  }
> >  EXPORT_SYMBOL_GPL(svc_generic_rpcbind_set);
> > 
> > +/*
> > + * Undo the [program, version] registrations that this svc_register() 
> > call
> > + * already made, stopping at version @nvers of program @nprog.
> > + *
> > + * Note that rpcbind matches RPCBPROC_UNSET on [program, version, 
> > netid] and
> > + * ignores the address, so this clears the netid rather than the one 
> > port.
> > + * That is the granularity svc_delete_xprt() unregisters at as well.
> > + */
> > +static void svc_unwind_register(const struct svc_serv *serv, struct 
> > net *net,
> > +				const int family, const unsigned short proto,
> > +				unsigned int nprog, unsigned int nvers)
> > +{
> > +	unsigned int p, i;
> > +
> > +	for (p = 0; p <= nprog; p++) {
> > +		struct svc_program *progp = &serv->sv_programs[p];
> > +		unsigned int last = p < nprog ? progp->pg_nvers : nvers;
> > +
> > +		for (i = 0; i < last; i++)
> > +			progp->pg_rpcbind_set(net, progp, i, family, proto, 0);
> > +	}
> > +}
> 
> For an IPv4 listener, the port-zero callback falls back through
> __svc_rpcb_register4() to rpcb_register(). PMAPPROC_UNSET ignores
> its protocol argument, so unwinding a partially successful TCP
> registration also removes the mappings for existing listeners on
> other transports, I would think.
> 
> It might be that the best the kernel can do here is tear everything
> down if one registration fails.
> 

What I was thinking for NFSv2/3 was to just have the listener set
netlink call wait for registration to complete before returning to
userland. That would mean we'd have to block even longer to try and
unregister things if things fail.

Alternate proposal: let's just declare rpcbind reg errors to be non-
fatal: do a pr_warn() and just leave it up to the admin to sort it out
if that happens instead of trying to fail the startup.

The resulting situation for the server is no worse off (it's just
running instead of being down), and I move that we're better off
leaving it up to a human to clean up the mess instead of trying to fix
things up from the kernel.

Thoughts?
-- 
Jeff Layton <jlayton@kernel.org>