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@xxxxxxxxxx> 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@xxxxxxxxxx>
>>>> ---
>>>> 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@xxxxxxxxxx>
--
Chuck Lever
