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

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

[Olga Kornievskaia]

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.

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) {
-- 
2.47.1

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

[Chuck Lever]

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.


> 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

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

[Olga Kornievskaia]

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.

> > 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
>

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

[Jeff Layton]

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>

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

[Chuck Lever]

On 8/12/25 3:57 PM, Jeff Layton wrote:
> 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.

I think Olga's proposed solution is "fine for now" but IMO it adds a bit
of technical debt that we don't want, long term.

A better solution is to make NFSD listener creation and destruction
complementary, if that's possible.


> 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>


-- 
Chuck Lever

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

[Jeff Layton]

On Tue, 2025-08-12 at 16:00 -0400, Chuck Lever wrote:
> On 8/12/25 3:57 PM, Jeff Layton wrote:
> > 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.
> 
> I think Olga's proposed solution is "fine for now" but IMO it adds a bit
> of technical debt that we don't want, long term.
> 
> A better solution is to make NFSD listener creation and destruction
> complementary, if that's possible.
>

Agreed, but that's a bigger project. Note that the patch below is just
adding rpcbind dereg/reg to what the netlink interface is already
doing. It already blows away all of the listeners and recreates them
when one is removed.

Changing that is a bigger project, and at that point we might as well
also allow the removal of listeners while the server is up. I'm not
opposed to that, but it may be considerably larger change.


> 
> > 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>
> 

-- 
Jeff Layton <jlayton@kernel.org>

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

[Tom Talpey]

On 8/12/2025 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.

The rpcbind and portmap protocols both define PROC_UNSET procedures
to selectively remove the triplets. Is it not feasible to code a new
client stub to simply invoke them, instead of the legacy big-hammer?

Tom.

> 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) {

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

[Chuck Lever]

On 8/12/25 4:30 PM, Jeff Layton wrote:
> On Tue, 2025-08-12 at 16:00 -0400, Chuck Lever wrote:
>> On 8/12/25 3:57 PM, Jeff Layton wrote:
>>> 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.
>>
>> I think Olga's proposed solution is "fine for now" but IMO it adds a bit
>> of technical debt that we don't want, long term.
>>
>> A better solution is to make NFSD listener creation and destruction
>> complementary, if that's possible.
>>
> 
> Agreed, but that's a bigger project. Note that the patch below is just
> adding rpcbind dereg/reg to what the netlink interface is already
> doing. It already blows away all of the listeners and recreates them
> when one is removed.
> 
> Changing that is a bigger project, and at that point we might as well
> also allow the removal of listeners while the server is up. I'm not
> opposed to that, but it may be considerably larger change.

Change size doesn't worry me that much (it will need some strategy,
though).

But two practical questions, then:

1. Does Olga's fix need to be backported to one or more LTS kernels? If
   it does, the narrower fix is probably better for now.

2. Is there a concern that the "larger" change will result in ABI
   incompatibility?

IMO both of the current behaviors (replacing all registrations and
requiring the server to be shutdown) have negative impact on server
availability. Adding or removing additional listeners should be
seamless.


>>> 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>
>>
> 


-- 
Chuck Lever