This is the promised patch that adds mountproto=tcp to the string
mount options if needed.
We still get a 90second timeout, but at least it works rather than
saying "mount.nfs: internal error".
It seems to me that it would be best to avoid the first call to mount
altogether. Simply always do a probe_both and then do a mount based
on the results of that.
Is there a good reason not to?
BTW, I tested this by running
iptables -A INPUT -p udp --dport 111 -j REJECT
on the NFS server.
Accessing an NFS server over an SSH tunnel would also be a good test
that people are using in real life (IRL as my son says...).
NeilBrown
>From cec4bf512cce4686ed5dd25a9bb489f9e521721d Mon Sep 17 00:00:00 2001
From: Neil Brown <[email protected]>
Date: Tue, 15 Jul 2008 22:38:17 +1000
If an NFS server is only listening on TCP for portmap (as apparently
MS-Windows-Server2003R2SP2 does), mount doesn't cope. There is retry
logic in case the initial choice of version/etc doesn't work, but it
doesn't cope with mountd needing tcp.
So:
Fix probe_port so that a TIMEDOUT error doesn't simply abort
but probes with other protocols (e.g. tcp).
Fix rewrite_mount_options to extract the mountproto option before
doing a probe, then set mountproto (and mount prot) based
on the result.
Enable retry if the mount call returns EIO, as that is what happens
if the UDP requests to portmap get no response.
Signed-off-by: Neil Brown <[email protected]>
---
utils/mount/network.c | 6 ++++--
utils/mount/stropts.c | 31 +++++++++++++++++++++++++++++--
2 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/utils/mount/network.c b/utils/mount/network.c
index ab7f6d0..cde6b2c 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -408,11 +408,10 @@ static int probe_port(clnt_addr_t *server, const unsigned long *versions,
}
if (clnt_ping(saddr, prog, *p_vers, *p_prot, NULL))
goto out_ok;
- if (rpc_createerr.cf_stat == RPC_TIMEDOUT)
- goto out_bad;
}
}
if (rpc_createerr.cf_stat != RPC_PROGNOTREGISTERED &&
+ rpc_createerr.cf_stat != RPC_TIMEDOUT &&
rpc_createerr.cf_stat != RPC_PROGVERSMISMATCH)
goto out_bad;
@@ -421,6 +420,9 @@ static int probe_port(clnt_addr_t *server, const unsigned long *versions,
continue;
p_prot = protos;
}
+ if (rpc_createerr.cf_stat == RPC_TIMEDOUT)
+ goto out_bad;
+
if (vers || !*++p_vers)
break;
}
diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 967fd69..f06e313 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -386,6 +386,17 @@ static struct mount_options *rewrite_mount_options(char *str)
option = po_get(options, "mountvers");
if (option)
mnt_server.pmap.pm_vers = atoi(option);
+ option = po_get(options, "mountproto");
+ if (option) {
+ if (strcmp(option, "tcp") == 0) {
+ mnt_server.pmap.pm_prot = IPPROTO_TCP;
+ po_remove_all(options, "mountproto");
+ }
+ if (strcmp(option, "udp") == 0) {
+ mnt_server.pmap.pm_prot = IPPROTO_UDP;
+ po_remove_all(options, "mountproto");
+ }
+ }
option = po_get(options, "port");
if (option) {
@@ -454,6 +465,20 @@ static struct mount_options *rewrite_mount_options(char *str)
}
+ if (mnt_server.pmap.pm_prot == IPPROTO_TCP)
+ snprintf(new_option, sizeof(new_option) - 1,
+ "mountproto=tcp");
+ else
+ snprintf(new_option, sizeof(new_option) - 1,
+ "mountproto=udp");
+ if (po_append(options, new_option) == PO_FAILED)
+ goto err;
+
+ snprintf(new_option, sizeof(new_option) - 1,
+ "mountport=%lu", mnt_server.pmap.pm_port);
+ if (po_append(options, new_option) == PO_FAILED)
+ goto err;
+
errno = 0;
return options;
@@ -560,9 +585,11 @@ static int nfs_try_nfs23mount(struct nfsmount_info *mi)
/*
* The kernel returns EOPNOTSUPP if the RPC bind failed,
- * and EPROTONOSUPPORT if the version isn't supported.
+ * and EPROTONOSUPPORT if the version isn't supported,
+ * and EIO if the protocol doesn't work.
*/
- if (errno != EOPNOTSUPP && errno != EPROTONOSUPPORT)
+ if (errno != EOPNOTSUPP && errno != EPROTONOSUPPORT &&
+ errno != EIO)
return 0;
return nfs_retry_nfs23mount(mi);
--
1.5.6.2
On Tue, Jul 15, 2008 at 8:56 AM, Neil Brown <[email protected]> wrote:
> This is the promised patch that adds mountproto=tcp to the string
> mount options if needed.
> We still get a 90second timeout, but at least it works rather than
> saying "mount.nfs: internal error".
>
> It seems to me that it would be best to avoid the first call to mount
> altogether. Simply always do a probe_both and then do a mount based
> on the results of that.
> Is there a good reason not to?
If I understand the question correctly, I think it doesn't because in
the most common cases, this isn't necessary. The mount options are
usually adequate, and most servers support all the necessary NFS
versions and transport protocols. This saves ephemeral ports and uses
less network traffic.
> BTW, I tested this by running
> iptables -A INPUT -p udp --dport 111 -j REJECT
>
> on the NFS server.
> Accessing an NFS server over an SSH tunnel would also be a good test
> that people are using in real life (IRL as my son says...).
>
> NeilBrown
>
>
> From cec4bf512cce4686ed5dd25a9bb489f9e521721d Mon Sep 17 00:00:00 2001
> From: Neil Brown <[email protected]>
> Date: Tue, 15 Jul 2008 22:38:17 +1000
>
> If an NFS server is only listening on TCP for portmap (as apparently
> MS-Windows-Server2003R2SP2 does), mount doesn't cope. There is retry
> logic in case the initial choice of version/etc doesn't work, but it
> doesn't cope with mountd needing tcp.
> So:
> Fix probe_port so that a TIMEDOUT error doesn't simply abort
> but probes with other protocols (e.g. tcp).
That seems reasonable and will update the behavior for both legacy and
text-based mounts.
But should you teach connect_to() to specifically handle ECONNREFUSED
as well? I don't see why there would be a long timeout in that case.
> Fix rewrite_mount_options to extract the mountproto option before
> doing a probe, then set mountproto (and mount prot) based
> on the result.
Yes, it should have been doing that anyway. There's a patch queued up
in my IPv6 series that addresses this along with other issues, but
it's reasonable to fix it now.
> Enable retry if the mount call returns EIO, as that is what happens
> if the UDP requests to portmap get no response.
I would rather see this one addressed in the kernel. I think the EIO
return is a kernel bug. If the server doesn't support a particular
transport protocol for portmap, mountd, or NFS, the mount(2) system
call should always return EPROTONOSUPPORT.
>
> Signed-off-by: Neil Brown <[email protected]>
> ---
> utils/mount/network.c | 6 ++++--
> utils/mount/stropts.c | 31 +++++++++++++++++++++++++++++--
> 2 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/utils/mount/network.c b/utils/mount/network.c
> index ab7f6d0..cde6b2c 100644
> --- a/utils/mount/network.c
> +++ b/utils/mount/network.c
> @@ -408,11 +408,10 @@ static int probe_port(clnt_addr_t *server, const unsigned long *versions,
> }
> if (clnt_ping(saddr, prog, *p_vers, *p_prot, NULL))
> goto out_ok;
> - if (rpc_createerr.cf_stat == RPC_TIMEDOUT)
> - goto out_bad;
> }
> }
> if (rpc_createerr.cf_stat != RPC_PROGNOTREGISTERED &&
> + rpc_createerr.cf_stat != RPC_TIMEDOUT &&
> rpc_createerr.cf_stat != RPC_PROGVERSMISMATCH)
> goto out_bad;
>
> @@ -421,6 +420,9 @@ static int probe_port(clnt_addr_t *server, const unsigned long *versions,
> continue;
> p_prot = protos;
> }
> + if (rpc_createerr.cf_stat == RPC_TIMEDOUT)
> + goto out_bad;
> +
> if (vers || !*++p_vers)
> break;
> }
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index 967fd69..f06e313 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -386,6 +386,17 @@ static struct mount_options *rewrite_mount_options(char *str)
> option = po_get(options, "mountvers");
> if (option)
> mnt_server.pmap.pm_vers = atoi(option);
> + option = po_get(options, "mountproto");
> + if (option) {
> + if (strcmp(option, "tcp") == 0) {
> + mnt_server.pmap.pm_prot = IPPROTO_TCP;
> + po_remove_all(options, "mountproto");
> + }
> + if (strcmp(option, "udp") == 0) {
> + mnt_server.pmap.pm_prot = IPPROTO_UDP;
> + po_remove_all(options, "mountproto");
> + }
> + }
>
> option = po_get(options, "port");
> if (option) {
> @@ -454,6 +465,20 @@ static struct mount_options *rewrite_mount_options(char *str)
>
> }
>
> + if (mnt_server.pmap.pm_prot == IPPROTO_TCP)
> + snprintf(new_option, sizeof(new_option) - 1,
> + "mountproto=tcp");
> + else
> + snprintf(new_option, sizeof(new_option) - 1,
> + "mountproto=udp");
> + if (po_append(options, new_option) == PO_FAILED)
> + goto err;
> +
> + snprintf(new_option, sizeof(new_option) - 1,
> + "mountport=%lu", mnt_server.pmap.pm_port);
> + if (po_append(options, new_option) == PO_FAILED)
> + goto err;
> +
> errno = 0;
> return options;
>
> @@ -560,9 +585,11 @@ static int nfs_try_nfs23mount(struct nfsmount_info *mi)
>
> /*
> * The kernel returns EOPNOTSUPP if the RPC bind failed,
> - * and EPROTONOSUPPORT if the version isn't supported.
> + * and EPROTONOSUPPORT if the version isn't supported,
> + * and EIO if the protocol doesn't work.
> */
> - if (errno != EOPNOTSUPP && errno != EPROTONOSUPPORT)
> + if (errno != EOPNOTSUPP && errno != EPROTONOSUPPORT &&
> + errno != EIO)
> return 0;
>
> return nfs_retry_nfs23mount(mi);
> --
> 1.5.6.2
>
> --
> 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
>
--
"Alright guard, begin the unnecessarily slow-moving dipping mechanism."
--Dr. Evil
Chuck Lever wrote:
> On Tue, Jul 15, 2008 at 8:56 AM, Neil Brown <[email protected]> wrote:
>> on the NFS server.
>> Accessing an NFS server over an SSH tunnel would also be a good test
>> that people are using in real life (IRL as my son says...).
>>
>> NeilBrown
>>
>>
>> From cec4bf512cce4686ed5dd25a9bb489f9e521721d Mon Sep 17 00:00:00 2001
>> From: Neil Brown <[email protected]>
>> Date: Tue, 15 Jul 2008 22:38:17 +1000
>>
>> If an NFS server is only listening on TCP for portmap (as apparently
>> MS-Windows-Server2003R2SP2 does), mount doesn't cope. There is retry
>> logic in case the initial choice of version/etc doesn't work, but it
>> doesn't cope with mountd needing tcp.
>> So:
>> Fix probe_port so that a TIMEDOUT error doesn't simply abort
>> but probes with other protocols (e.g. tcp).
>
> That seems reasonable and will update the behavior for both legacy and
> text-based mounts.
I agree...
>
> But should you teach connect_to() to specifically handle ECONNREFUSED
> as well? I don't see why there would be a long timeout in that case.
>
>> Fix rewrite_mount_options to extract the mountproto option before
>> doing a probe, then set mountproto (and mount prot) based
>> on the result.
>
> Yes, it should have been doing that anyway. There's a patch queued up
> in my IPv6 series that addresses this along with other issues, but
> it's reasonable to fix it now.
I agree...
>
>> Enable retry if the mount call returns EIO, as that is what happens
>> if the UDP requests to portmap get no response.
>
> I would rather see this one addressed in the kernel. I think the EIO
> return is a kernel bug. If the server doesn't support a particular
> transport protocol for portmap, mountd, or NFS, the mount(2) system
> call should always return EPROTONOSUPPORT.
Since EIO has so many meanings... I think it should stay as a fatal
error as well...
steved.
On Wednesday July 16, [email protected] wrote:
> >> Enable retry if the mount call returns EIO, as that is what happens
> >> if the UDP requests to portmap get no response.
> >
> > I would rather see this one addressed in the kernel. I think the EIO
> > return is a kernel bug. If the server doesn't support a particular
> > transport protocol for portmap, mountd, or NFS, the mount(2) system
> > call should always return EPROTONOSUPPORT.
> Since EIO has so many meanings... I think it should stay as a fatal
> error as well...
>
That would be best. But the reality is that there are kernels in the
wild where -EIO indicates and error that isn't really fatal.
We already do kernel-detection in mount. How about extending it like
this?
Thanks,
NeilBrown
From: Neil Brown <[email protected]>
Date: Thu, 17 Jul 2008 12:23:03 +1000
Subject: [PATCH] mount: Recognised EIO from early kernels as possibly indicating a protocol error.
In kernels up to and including 2.6.26, mount reports "connection
refused" type messages as EIO rather than EPROTONOSUPPORT.
So detect those kernels and don't treat EIO as so fatal.
Signed-off-by: Neil Brown <[email protected]>
---
utils/mount/mount.c | 8 +++++++-
utils/mount/stropts.c | 7 +++++--
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/utils/mount/mount.c b/utils/mount/mount.c
index 06e2804..2163e4e 100644
--- a/utils/mount/mount.c
+++ b/utils/mount/mount.c
@@ -174,9 +174,15 @@ static void discover_nfs_mount_data_version(void)
}
if (nfs_mount_data_version > NFS_MOUNT_VERSION)
nfs_mount_data_version = NFS_MOUNT_VERSION;
- else
+ else {
if (kernel_version > MAKE_VERSION(2, 6, 22))
string++;
+ /* up to and including 2.6.26, mount might return EIO
+ * when it means EPROTONOSUPPORT
+ */
+ if (kernel_version > MAKE_VERSION(2, 6, 26))
+ string++;
+ }
}
static void print_one(char *spec, char *node, char *type, char *opts)
diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 09fca86..df4d9c1 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -552,9 +552,12 @@ static int nfs_try_nfs23mount(struct nfsmount_info *mi)
/*
* The kernel returns EOPNOTSUPP if the RPC bind failed,
- * and EPROTONOSUPPORT if the version isn't supported.
+ * and EPROTONOSUPPORT if the version isn't supported,
+ * or the protocol isn't accepted.
+ * Up to 2.6.26, the later causes EIO.
*/
- if (errno != EOPNOTSUPP && errno != EPROTONOSUPPORT)
+ if (errno != EOPNOTSUPP && errno != EPROTONOSUPPORT &&
+ (string >= 2 || errno != EIO))
return 0;
return nfs_retry_nfs23mount(mi);
--
1.5.6.2
Neil Brown wrote:
> On Wednesday July 16, [email protected] wrote:
>>>> Enable retry if the mount call returns EIO, as that is what happens
>>>> if the UDP requests to portmap get no response.
>>> I would rather see this one addressed in the kernel. I think the EIO
>>> return is a kernel bug. If the server doesn't support a particular
>>> transport protocol for portmap, mountd, or NFS, the mount(2) system
>>> call should always return EPROTONOSUPPORT.
>> Since EIO has so many meanings... I think it should stay as a fatal
>> error as well...
>>
>
> That would be best. But the reality is that there are kernels in the
> wild where -EIO indicates and error that isn't really fatal.
Yeah I know... and thats the problem.. EIO is basically a catch all errno..
>
> We already do kernel-detection in mount. How about extending it like
> this?
>
> Thanks,
> NeilBrown
>
> From: Neil Brown <[email protected]>
> Date: Thu, 17 Jul 2008 12:23:03 +1000
> Subject: [PATCH] mount: Recognised EIO from early kernels as possibly indicating a protocol error.
>
> In kernels up to and including 2.6.26, mount reports "connection
> refused" type messages as EIO rather than EPROTONOSUPPORT.
> So detect those kernels and don't treat EIO as so fatal.
Should we just fix the kernel to do the right thing?
steved.
Neil Brown wrote:
> On Friday July 18, [email protected] wrote:
>> This is done over an unconnected socket so ICMP errors don't come back
>> (I think) so the timeout is all that clnt_call gets to know there is
>> an error... I wonder what would happen if we just changed that to be a
>> connected socket...
>
> Yep, that was a good idea.
> The patch below causes a connected socket to be used when talking to
> portmap over UDP. That means that we get ICMP errors back.
> And if we handle the errors correctly, we avoid the timeout.
>
> With this added to all my other current patches, attempting to mount
> when udp/portmap is blocked succeeds in 90 seconds rather than 112
> seconds, and the 90 seconds is all in the mount system call.
>
> If the kernel used connected UDP ports to talk to portmap, it might
> benefit too (didn't I see some emails about connected UDP ports in
> net/sunrpc recently - I'm afraid I didn't pay much attention).
>
> NeilBrown
>
>
> From: NeilBrown <[email protected]>
>
> Use a connected port when talking to portmap via UDP.
>
> This allows us to get ICMP errors reported back so we can avoid
> timeouts. Also catch the error (RPC_CANTRECV) properly in getport.
>
> Signed-off-by: NeilBrown <[email protected]>
>
> diff --git a/utils/mount/network.c b/utils/mount/network.c
> index 75354a7..ff50512 100644
> --- a/utils/mount/network.c
> +++ b/utils/mount/network.c
> @@ -447,7 +447,7 @@ static unsigned short getport(struct sockaddr_in *saddr,
> bind_saddr = *saddr;
> bind_saddr.sin_port = htons(PMAPPORT);
>
> - socket = get_socket(&bind_saddr, proto, PMAP_TIMEOUT, FALSE, FALSE);
> + socket = get_socket(&bind_saddr, proto, PMAP_TIMEOUT, FALSE, TRUE);
> if (socket == RPC_ANYSOCK) {
> if (proto == IPPROTO_TCP &&
> rpc_createerr.cf_error.re_errno == ETIMEDOUT)
> @@ -536,6 +536,7 @@ static int probe_port(clnt_addr_t *server, const unsigned long *versions,
> }
> if (rpc_createerr.cf_stat != RPC_PROGNOTREGISTERED &&
> rpc_createerr.cf_stat != RPC_TIMEDOUT &&
> + rpc_createerr.cf_stat != RPC_CANTRECV &&
> rpc_createerr.cf_stat != RPC_PROGVERSMISMATCH)
> goto out_bad;
>
> @@ -544,7 +545,8 @@ static int probe_port(clnt_addr_t *server, const unsigned long *versions,
> continue;
> p_prot = protos;
> }
> - if (rpc_createerr.cf_stat == RPC_TIMEDOUT)
> + if (rpc_createerr.cf_stat == RPC_TIMEDOUT ||
> + rpc_createerr.cf_stat == RPC_CANTRECV)
> goto out_bad;
>
> if (vers || !*++p_vers)
Committed....
steved.