Inconsistent error codes between NFSv4 and v3 on network issues

Commit Message

Trond Myklebust March 4, 2013, 10:37 p.m. UTC

On Mon, 2013-03-04 at 20:43 +0000, Myklebust, Trond wrote:
> On Mon, 2013-03-04 at 20:10 +0100, Jan Engelhardt wrote:
> > Just nuke your default route, and it should be easily reproducible.
> > 
> 
> The problem is that call_connect_status() is converting that ENETUNREACH
> into a EIO. We shouldn't be doing that, but should leave it up to the
> caller (i.e. the NFS layer) to perform that kind of mapping.

Could you please check if the attached patch helps.

Comments

Jan Engelhardt March 13, 2013, 2:47 p.m. UTC | #1

On Monday 2013-03-04 23:37, Myklebust, Trond wrote:

>On Mon, 2013-03-04 at 20:43 +0000, Myklebust, Trond wrote:
>> On Mon, 2013-03-04 at 20:10 +0100, Jan Engelhardt wrote:
>> > Just nuke your default route, and it should be easily reproducible.
>> > 
>> 
>> The problem is that call_connect_status() is converting that ENETUNREACH
>> into a EIO. We shouldn't be doing that, but should leave it up to the
>> caller (i.e. the NFS layer) to perform that kind of mapping.
>
>Could you please check if the attached patch helps.

With the patch, I still get EIO from the mount syscall.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

From 724bf7a71b44145811a1fec3b20a26bd1edae51b Mon Sep 17 00:00:00 2001
From: Trond Myklebust <[e-mail]>
Date: Mon, 4 Mar 2013 17:29:33 -0500
Subject: [PATCH] SUNRPC: Report network/connection errors correctly for
 SOFTCONN rpc tasks

In the case of a SOFTCONN rpc task, we really want to ensure that it
reports errors like ENETUNREACH back to the caller. Currently, only
some of these errors are being reported back (connect errors are not),
and they are being converted by the RPC layer into EIO.

Reported-by: Jan Engelhardt <[e-mail]>
Signed-off-by: Trond Myklebust <[e-mail]>
---
 net/sunrpc/clnt.c     | 24 ++++++++++++++----------
 net/sunrpc/xprtsock.c |  8 ++++----
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index dcc446e..b95a0a2 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -1644,22 +1644,26 @@  call_connect_status(struct rpc_task *task)
 
 	dprint_status(task);
 
-	task->tk_status = 0;
-	if (status >= 0 || status == -EAGAIN) {
-		clnt->cl_stats->netreconn++;
-		task->tk_action = call_transmit;
-		return;
-	}
-
 	trace_rpc_connect_status(task, status);
 	switch (status) {
 		/* if soft mounted, test if we've timed out */
 	case -ETIMEDOUT:
 		task->tk_action = call_timeout;
-		break;
-	default:
-		rpc_exit(task, -EIO);
+		return;
+	case -ECONNREFUSED:
+	case -ECONNRESET:
+	case -ENETUNREACH:
+		if (RPC_IS_SOFTCONN(task))
+			break;
+		/* retry with existing socket, after a delay */
+	case 0:
+	case -EAGAIN:
+		task->tk_status = 0;
+		clnt->cl_stats->netreconn++;
+		task->tk_action = call_transmit;
+		return;
 	}
+	rpc_exit(task, status);
 }
 
 /*
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index c1d8476..3081620 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2202,10 +2202,6 @@  static void xs_tcp_setup_socket(struct work_struct *work)
 		 */
 		xs_tcp_force_close(xprt);
 		break;
-	case -ECONNREFUSED:
-	case -ECONNRESET:
-	case -ENETUNREACH:
-		/* retry with existing socket, after a delay */
 	case 0:
 	case -EINPROGRESS:
 	case -EALREADY:
@@ -2216,6 +2212,10 @@  static void xs_tcp_setup_socket(struct work_struct *work)
 		/* Happens, for instance, if the user specified a link
 		 * local IPv6 address without a scope-id.
 		 */
+	case -ECONNREFUSED:
+	case -ECONNRESET:
+	case -ENETUNREACH:
+		/* retry with existing socket, after a delay */
 		goto out;
 	}
 out_eagain:
-- 
1.8.1.4