The mount system call path does not expect such errors as ECONNREFUSED
to be returned from failed transport connection attempts, otherwise it
prints simply "internal error". Translate all such errors to ENOTCONN
from RPC/RDMA to match sockets behavior.
Signed-off-by: Tom Talpey <[email protected]>
---
net/sunrpc/xprtrdma/rpc_rdma.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c
index c4b8011..11ea8da 100644
--- a/net/sunrpc/xprtrdma/rpc_rdma.c
+++ b/net/sunrpc/xprtrdma/rpc_rdma.c
@@ -700,7 +700,7 @@ rpcrdma_conn_func(struct rpcrdma_ep *ep)
xprt_wake_pending_tasks(xprt, 0);
} else {
if (xprt_test_and_clear_connected(xprt))
- xprt_wake_pending_tasks(xprt, ep->rep_connected);
+ xprt_wake_pending_tasks(xprt, -ENOTCONN);
}
spin_unlock_bh(&xprt->transport_lock);
}
On Wed, 2008-10-08 at 11:48 -0400, Tom Talpey wrote:
> The mount system call path does not expect such errors as ECONNREFUSED
> to be returned from failed transport connection attempts, otherwise it
> prints simply "internal error". Translate all such errors to ENOTCONN
> from RPC/RDMA to match sockets behavior.
Hmm... Shouldn't we be passing the ECONNREFUSED error here, and rather
fix the downstream error paths?
> Signed-off-by: Tom Talpey <[email protected]>
> ---
>
> net/sunrpc/xprtrdma/rpc_rdma.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c
> index c4b8011..11ea8da 100644
> --- a/net/sunrpc/xprtrdma/rpc_rdma.c
> +++ b/net/sunrpc/xprtrdma/rpc_rdma.c
> @@ -700,7 +700,7 @@ rpcrdma_conn_func(struct rpcrdma_ep *ep)
> xprt_wake_pending_tasks(xprt, 0);
> } else {
> if (xprt_test_and_clear_connected(xprt))
> - xprt_wake_pending_tasks(xprt, ep->rep_connected);
> + xprt_wake_pending_tasks(xprt, -ENOTCONN);
> }
> spin_unlock_bh(&xprt->transport_lock);
> }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
At 01:31 PM 10/8/2008, Trond Myklebust wrote:
>On Wed, 2008-10-08 at 11:48 -0400, Tom Talpey wrote:
>> The mount system call path does not expect such errors as ECONNREFUSED
>> to be returned from failed transport connection attempts, otherwise it
>> prints simply "internal error". Translate all such errors to ENOTCONN
>> from RPC/RDMA to match sockets behavior.
>
>Hmm... Shouldn't we be passing the ECONNREFUSED error here, and rather
>fix the downstream error paths?
That means fixing /sbin/mount.nfs, and an earlier conversation concluded that
"doing what TCP does" was preferred. The error path from NFS and RPC is,
frankly, more than a little tortuous. The error is translated and filtered in
both layers, after being returned from the transport. Then, the mount command
makes up its own diagnostic from what comes back from the syscall. Well beyond
the scope of RDMA.
Your call. As proposed, it is more compatible with current practice, IMO.
Tom.
>> Signed-off-by: Tom Talpey <[email protected]>
>> ---
>>
>> net/sunrpc/xprtrdma/rpc_rdma.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c
>> index c4b8011..11ea8da 100644
>> --- a/net/sunrpc/xprtrdma/rpc_rdma.c
>> +++ b/net/sunrpc/xprtrdma/rpc_rdma.c
>> @@ -700,7 +700,7 @@ rpcrdma_conn_func(struct rpcrdma_ep *ep)
>> xprt_wake_pending_tasks(xprt, 0);
>> } else {
>> if (xprt_test_and_clear_connected(xprt))
>> - xprt_wake_pending_tasks(xprt, ep->rep_connected);
>> + xprt_wake_pending_tasks(xprt, -ENOTCONN);
>> }
>> spin_unlock_bh(&xprt->transport_lock);
>> }
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2008-10-08 at 13:40 -0400, Talpey, Thomas wrote:
> At 01:31 PM 10/8/2008, Trond Myklebust wrote:
> >On Wed, 2008-10-08 at 11:48 -0400, Tom Talpey wrote:
> >> The mount system call path does not expect such errors as ECONNREFUSED
> >> to be returned from failed transport connection attempts, otherwise it
> >> prints simply "internal error". Translate all such errors to ENOTCONN
> >> from RPC/RDMA to match sockets behavior.
> >
> >Hmm... Shouldn't we be passing the ECONNREFUSED error here, and rather
> >fix the downstream error paths?
>
> That means fixing /sbin/mount.nfs, and an earlier conversation concluded that
> "doing what TCP does" was preferred. The error path from NFS and RPC is,
> frankly, more than a little tortuous. The error is translated and filtered in
> both layers, after being returned from the transport. Then, the mount command
> makes up its own diagnostic from what comes back from the syscall. Well beyond
> the scope of RDMA.
>
> Your call. As proposed, it is more compatible with current practice, IMO.
Are you saying that mount.nfs translates 'ECONNREFUSED' as 'internal
error'? That would be a bug...
Trond
At 01:43 PM 10/8/2008, Trond Myklebust wrote:
>On Wed, 2008-10-08 at 13:40 -0400, Talpey, Thomas wrote:
>> At 01:31 PM 10/8/2008, Trond Myklebust wrote:
>> >On Wed, 2008-10-08 at 11:48 -0400, Tom Talpey wrote:
>> >> The mount system call path does not expect such errors as ECONNREFUSED
>> >> to be returned from failed transport connection attempts, otherwise it
>> >> prints simply "internal error". Translate all such errors to ENOTCONN
>> >> from RPC/RDMA to match sockets behavior.
>> >
>> >Hmm... Shouldn't we be passing the ECONNREFUSED error here, and rather
>> >fix the downstream error paths?
>>
>> That means fixing /sbin/mount.nfs, and an earlier conversation concluded that
>> "doing what TCP does" was preferred. The error path from NFS and RPC is,
>> frankly, more than a little tortuous. The error is translated and filtered in
>> both layers, after being returned from the transport. Then, the mount command
>> makes up its own diagnostic from what comes back from the syscall.
>Well beyond
>> the scope of RDMA.
>>
>> Your call. As proposed, it is more compatible with current practice, IMO.
>
>Are you saying that mount.nfs translates 'ECONNREFUSED' as 'internal
>error'? That would be a bug...
No, unfortunately it's a good bit more complicated than that. Sorry for
oversimplifying. Mount.nfs would need to change, but the kernel too.
What happens is, the XYZ transport returns a connect status, which xprt.c's
xprt_connect_status() looks at and if non-zero decides what to dprintk, and
whether to retry.
xprt_connect_status() only parses two errors: ENOTCONN and ETIMEDOUT.
These result in various attempts to rebind and retry, as appropriate.
If any other error is returned, the status is changed to EIO and the call
is aborted. When the caller is mount, this results in EIO popping out of
the kernel as the return of sys_mount().
The EIO is then handled by mount.nfs in various unhelpful ways. Mount
pretty much never sees ECONNREFUSED from this call (though its userspace
stuff such as looking up ports and pinging servers does).
So, I just decided to return ENOTCONN like the other transports. I could
add new error arms to this code, but IMO they'd be unnecessary, for TCP
and UDP anyway.
Tom.
>
>Trond