On Mon, Aug 18, 2025 at 2:55 PM Olga Kornievskaia <aglo@xxxxxxxxx> wrote:
>
> On Mon, Aug 18, 2025 at 2:48 PM Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:
> >
> > Hi Olga -
> >
> > On 8/18/25 2:25 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.
> > >
> > > When deleting the transport, unregister with rpcbind when appropriate.
> >
> > The patch description needs to explain why this is needed. Did you
> > mention to me there was a crash or other malfunction?
>
> Malfunction is that nfsdctl removed a listener (nfsdctl listener
> -udp::2049) but rpcinfo query still shows it (rpcinfo -p |grep -w
> nfs).
>
> > > Fixes: d093c9089260 ("nfsd: fix management of listener transports")
> > > Signed-off-by: Olga Kornievskaia <okorniev@xxxxxxxxxx>
> > > ---
> > > net/sunrpc/svc_xprt.c | 17 +++++++++++++++++
> > > 1 file changed, 17 insertions(+)
> > >
> > > diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> > > index 8b1837228799..223737fac95d 100644
> > > --- a/net/sunrpc/svc_xprt.c
> > > +++ b/net/sunrpc/svc_xprt.c
> > > @@ -1014,6 +1014,23 @@ static void svc_delete_xprt(struct svc_xprt *xprt)
> > > struct svc_serv *serv = xprt->xpt_server;
> > > struct svc_deferred_req *dr;
> > >
> > > + /* unregister with rpcbind for when transport type is TCP or UDP.
> > > + * Only TCP and RDMA sockets are marked as LISTENER sockets, so
> > > + * check for UDP separately.
> > > + */
> > > + if ((test_bit(XPT_LISTENER, &xprt->xpt_flags) &&
> > > + xprt->xpt_class->xcl_ident != XPRT_TRANSPORT_RDMA) ||
> > > + xprt->xpt_class->xcl_ident == XPRT_TRANSPORT_UDP) {
> >
> > Now I thought that UDP also had a rpcbind registration ... ?
>
> Correct.
>
> > So I don't
> > quite understand why gating the unregistration is necessary.
>
> We are sending unregister for when the transport xprt is of type
> LISTENER (which covers TCP but not UDP) so to also send unregister for
> UDP we need to check for it separately. RDMA listener transport is
> also marked as listener but we do not want to trigger unregister here
> because rpcbind knows nothing about rdma type.
>
> Transports are not necessarily of listener type and thus we don't want
> to trigger rpcbind unregister for that.
I still feel that unregistering "all" with rpcbind in nfsctl after we
call svc_xprt_destroy_all() seems cleaner (single call vs a call per
registered transport). But this approach would go for when listeners
are allowed to be deleted while the server is running. Perhaps both
patches can be considered for inclusion.
>
> > > + struct svc_sock *svsk = container_of(xprt, struct svc_sock,
> > > + sk_xprt);
> > > + struct socket *sock = svsk->sk_sock;
> > > +
> > > + if (svc_register(serv, xprt->xpt_net, sock->sk->sk_family,
> > > + sock->sk->sk_protocol, 0) < 0)
> > > + pr_warn("failed to unregister %s with rpcbind\n",
> > > + xprt->xpt_class->xcl_name);
> > > + }
> > > +
> > > if (test_and_set_bit(XPT_DEAD, &xprt->xpt_flags))
> > > return;
> > >
> >
> >
> > --
> > Chuck Lever
> >
>
