On 8/19/2025 11:28 AM, Chuck Lever wrote:
On 8/19/25 11:14 AM, Olga Kornievskaia wrote:
On Mon, Aug 18, 2025 at 3:36 PM Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:
On 8/18/25 3:04 PM, Olga Kornievskaia wrote:
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.
rpcbind is supposed to know about the "rdma" and "rdma6" netids. The
convention though is not to register them. Registering those transports
should work just fine.
Question is: should nfsd have been registering rdma with rpcbind as well?
Solaris doesn't register it's RDMA services, so I didn't implement it
for Linux. There's no reason we couldn't, though. But probably not worth
spending time on if there isn't a practical need for it.
My opinion is definitely no to registering RDMA, until there's
a desire to use it on unregistered ports. And even then, the
clients allow specifying the port as a mount option.
Really, portmap and rpcbind are (IMO) artifacts of the 1990's
Sun Microsystems computing ecosystem. Let's try to innovate
in other ways.
Tom.
__svc_rpcb_register4() takes in: program (i'm assuming nfs, acl, etc),
version, protocol, and port. Protocol is supposed to be a number
(below). I don't see how "rdma" can be specified as a protocol/type.
switch (protocol) {
case IPPROTO_UDP:
netid = RPCBIND_NETID_UDP;
break;
case IPPROTO_TCP:
netid = RPCBIND_NETID_TCP;
break;
default:
return -ENOPROTOOPT;
We can register nfs, tcp, port 20049 but nothing that would indicate
that it's rdma. I have grepped thru the rpcbind source code and didn't
find occurrences of rdma.
Transports are not necessarily of listener type and thus we don't want
to trigger rpcbind unregister for that.
Ah. Maybe svc_delete_xprt() is not the right place for unregistration.
The "listener" check here doesn't seem like the correct approach, but
I don't know enough about how UDP set-up works to understand how that
transport is registered.
A xpo_register and a xpo_unregister method can be added to the
svc_xprt_ops structure to partially handle the differences. The question
is where should those methods be called?
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.
You and Jeff both seem to want to punt on this, but...
People will see that a transport can be removed, but having to shut down
the whole NFS service to do that seems... unexpected and rather useless.
At the very least, it would indicate to me as a sysadmin that the
"remove transport" feature is not finished, and is thus unusable in its
current form.
An alternative is to simply disable the "remove transport" API until it
can be implemented correctly.
+ 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
--
Chuck Lever
