Re: [PATCH 1/1] nfsd: unregister with rpcbind when removing listener

[Jeff Layton <jlayton@kernel.org>]

On Tue, 2025-08-12 at 15:13 -0400, Olga Kornievskaia wrote:
> On Tue, Aug 12, 2025 at 3:08 PM Chuck Lever <chuck.lever@oracle.com> wrote:
> > 
> > On 8/12/25 3:02 PM, Olga Kornievskaia wrote:
> > > When a listener is added, a part of creation of transport also registers
> > > program/port with rpcbind. However, when the listener is removed,
> > > while transport goes away, rpcbind still has the entry for that
> > > port/type.
> > > 
> > > Removal of listeners works by first removing all transports and then
> > > re-adding the ones that were not removed. In addition to destroying
> > > all transports, now also call the function that unregisters everything
> > > with the rpcbind. But we also then need to call the rpcbind setup
> > > function before adding back new transports.
> > 
> > Removing all rpcbind registrations and then re-adding them might
> > cause an outage for clients that attempt to mount the server right
> > at that moment.
> 
> Ok I'll take a look at unregistering elsewhere. But to note, removing
> a listener is only allowed when no threads are running. Thus no mounts
> are possible.
> 

Right, which is why I think this is fine. There is a small chance a
client might see the bogus rpcbind registration, but that's still
better than the status quo.

> > > Fixes: d093c9089260 ("nfsd: fix management of listener transports")
> > > Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> > > ---
> > >  fs/nfsd/nfsctl.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> > > index 2909d70de559..99d06343117b 100644
> > > --- a/fs/nfsd/nfsctl.c
> > > +++ b/fs/nfsd/nfsctl.c
> > > @@ -1998,8 +1998,11 @@ int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info)
> > >        * Since we can't delete an arbitrary llist entry, destroy the
> > >        * remaining listeners and recreate the list.
> > >        */
> > > -     if (delete)
> > > +     if (delete) {
> > >               svc_xprt_destroy_all(serv, net);
> > > +             svc_rpcb_cleanup(serv, net);
> > > +             svc_bind(serv, net);
> > > +     }
> > > 
> > >       /* walk list of addrs again, open any that still don't exist */
> > >       nlmsg_for_each_attr(attr, info->nlhdr, GENL_HDRLEN, rem) {
> > 
> > 
> > --
> > Chuck Lever
> > 

Reviewed-by: Jeff Layton <jlayton@kernel.org>