All of lore.kernel.org
 help / color / mirror / Atom feed
* mount.nfs: protocol fallback when server doesn't support TCP
@ 2010-08-26  2:06 Neil Brown
  2010-08-26 14:51 ` Chuck Lever
  0 siblings, 1 reply; 14+ messages in thread
From: Neil Brown @ 2010-08-26  2:06 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing list


Currently if the server doesn't support TCP (and so doesn't support NFSv4)
we don't fall-back to NFSv3.  This is because 'ECONNREFUSED' isn't deemed to
be a suitable error for falling back.  Rather we wait for about 2 minutes,
then give up.

There is some justification for this:  ECONNREFUSED could just mean that the
server isn't quite ready yet.

I'm not really sure what the best thing to do here would be.  We don't really
want to try v3 and have fail only because it was just enough later that nfsd
is now responding.

Maybe the ideal would be to do a portmap probe and only fall back to v3 if
portmap confirm that v3 is supported and v4 isn't.

For now I just present this patch which allows fallback to v3 providing tcp
wasn't explicitly requested.

Thoughts?

NeilBrown

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 0241400..fb8c108 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -762,6 +762,25 @@ static int nfs_try_mount(struct nfsmount_info *mi)
 			errno = 0;
 			result = nfs_try_mount_v4(mi);
 			if (errno != EPROTONOSUPPORT) {
+				/* If server only handles UDP, then v4 will have
+				 * received ECONNREFUSED for TCP, so fall through
+				 * to v3v2 which can try udp, but only if tcp
+				 * wasn't explicitly requested
+				 */
+				if (errno == ECONNREFUSED) {
+					static const char *nfs_proto_tbl[] = {
+						"udp",
+						"tcp",
+						NULL
+					};
+					char *p;
+
+					if (po_rightmost(mi->options, nfs_proto_tbl) == 1)
+						break;
+					p = po_get(mi->options, "proto");
+					if (p && strcmp(p, "tcp") == 0)
+						break;
+				} else
 				/* 
 				 * To deal with legacy Linux servers that don't
 				 * automatically export a pseudo root, retry

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: mount.nfs: protocol fallback when server doesn't support TCP
  2010-08-26  2:06 mount.nfs: protocol fallback when server doesn't support TCP Neil Brown
@ 2010-08-26 14:51 ` Chuck Lever
  2010-08-26 15:27   ` Jim Rees
  2010-08-30  0:30   ` Neil Brown
  0 siblings, 2 replies; 14+ messages in thread
From: Chuck Lever @ 2010-08-26 14:51 UTC (permalink / raw)
  To: Neil Brown; +Cc: Linux NFS Mailing list


On Aug 25, 2010, at 10:06 PM, Neil Brown wrote:

> 
> Currently if the server doesn't support TCP (and so doesn't support NFSv4)
> we don't fall-back to NFSv3.  This is because 'ECONNREFUSED' isn't deemed to
> be a suitable error for falling back.  Rather we wait for about 2 minutes,
> then give up.
> 
> There is some justification for this:  ECONNREFUSED could just mean that the
> server isn't quite ready yet.
> 
> I'm not really sure what the best thing to do here would be.  We don't really
> want to try v3 and have fail only because it was just enough later that nfsd
> is now responding.
> 
> Maybe the ideal would be to do a portmap probe and only fall back to v3 if
> portmap confirm that v3 is supported and v4 isn't.
> 
> For now I just present this patch which allows fallback to v3 providing tcp
> wasn't explicitly requested.
> 
> Thoughts?

What are the risks of checking portmap in this case?  That seems like it would give a better chance of a desirable outcome.

> NeilBrown
> 
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index 0241400..fb8c108 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -762,6 +762,25 @@ static int nfs_try_mount(struct nfsmount_info *mi)
> 			errno = 0;
> 			result = nfs_try_mount_v4(mi);
> 			if (errno != EPROTONOSUPPORT) {
> +				/* If server only handles UDP, then v4 will have
> +				 * received ECONNREFUSED for TCP, so fall through
> +				 * to v3v2 which can try udp, but only if tcp
> +				 * wasn't explicitly requested
> +				 */
> +				if (errno == ECONNREFUSED) {
> +					static const char *nfs_proto_tbl[] = {
> +						"udp",
> +						"tcp",
> +						NULL
> +					};
> +					char *p;
> +
> +					if (po_rightmost(mi->options, nfs_proto_tbl) == 1)
> +						break;
> +					p = po_get(mi->options, "proto");
> +					if (p && strcmp(p, "tcp") == 0)
> +						break;

There are all kinds of things to worry about when looking for what protocol was specified.  For example, you should check for udp6 or tcp6 here (and of course, you shouldn't check for those if IPV6_SUPPORTED is not enabled).

utils/mount/network.c:nfs_nfs_protocol() does most or all of what you need.  It will also return zero if the user didn't specify a protocol, which is, I think, the case you're trying to detect here.

> +				} else
> 				/* 
> 				 * To deal with legacy Linux servers that don't
> 				 * automatically export a pseudo root, retry

-- 
chuck[dot]lever[at]oracle[dot]com





^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: mount.nfs: protocol fallback when server doesn't support TCP
  2010-08-26 14:51 ` Chuck Lever
@ 2010-08-26 15:27   ` Jim Rees
  2010-08-26 16:09     ` Chuck Lever
  2010-08-30  0:30   ` Neil Brown
  1 sibling, 1 reply; 14+ messages in thread
From: Jim Rees @ 2010-08-26 15:27 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Neil Brown, Linux NFS Mailing list

Chuck Lever wrote:

  On Aug 25, 2010, at 10:06 PM, Neil Brown wrote:
  > Maybe the ideal would be to do a portmap probe and only fall back to v3 if
  > portmap confirm that v3 is supported and v4 isn't.

  What are the risks of checking portmap in this case?  That seems like it
  would give a better chance of a desirable outcome.

In general, portmap can't confirm that v4 isn't available, only that v3 is.
Right?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: mount.nfs: protocol fallback when server doesn't support TCP
  2010-08-26 15:27   ` Jim Rees
@ 2010-08-26 16:09     ` Chuck Lever
  0 siblings, 0 replies; 14+ messages in thread
From: Chuck Lever @ 2010-08-26 16:09 UTC (permalink / raw)
  To: Jim Rees; +Cc: Neil Brown, Linux NFS Mailing list


On Aug 26, 2010, at 11:27 AM, Jim Rees wrote:

> Chuck Lever wrote:
> 
>  On Aug 25, 2010, at 10:06 PM, Neil Brown wrote:
>> Maybe the ideal would be to do a portmap probe and only fall back to v3 if
>> portmap confirm that v3 is supported and v4 isn't.
> 
>  What are the risks of checking portmap in this case?  That seems like it
>  would give a better chance of a desirable outcome.
> 
> In general, portmap can't confirm that v4 isn't available, only that v3 is.
> Right?

Registering an NFSv4 service with the local rpcbind is a SHOULD at best, true.  But, if no version-specific mount options are specified, I think establishing any kind of connection is better than failing.

-- 
chuck[dot]lever[at]oracle[dot]com





^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: mount.nfs: protocol fallback when server doesn't support TCP
  2010-08-26 14:51 ` Chuck Lever
  2010-08-26 15:27   ` Jim Rees
@ 2010-08-30  0:30   ` Neil Brown
  2010-08-30 19:29     ` Chuck Lever
  1 sibling, 1 reply; 14+ messages in thread
From: Neil Brown @ 2010-08-30  0:30 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing list

On Thu, 26 Aug 2010 10:51:23 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:

> 
> On Aug 25, 2010, at 10:06 PM, Neil Brown wrote:
> 
> > 
> > Currently if the server doesn't support TCP (and so doesn't support NFSv4)
> > we don't fall-back to NFSv3.  This is because 'ECONNREFUSED' isn't deemed to
> > be a suitable error for falling back.  Rather we wait for about 2 minutes,
> > then give up.
> > 
> > There is some justification for this:  ECONNREFUSED could just mean that the
> > server isn't quite ready yet.
> > 
> > I'm not really sure what the best thing to do here would be.  We don't really
> > want to try v3 and have fail only because it was just enough later that nfsd
> > is now responding.
> > 
> > Maybe the ideal would be to do a portmap probe and only fall back to v3 if
> > portmap confirm that v3 is supported and v4 isn't.
> > 
> > For now I just present this patch which allows fallback to v3 providing tcp
> > wasn't explicitly requested.
> > 
> > Thoughts?
> 
> What are the risks of checking portmap in this case?  That seems like it would give a better chance of a desirable outcome.

True, I was just too lazy at the time.

How about this:
  If nfs_try_mount_v4 return ECONNREFUSED we do a portmap lookup and see
  which versions and protocols are supported.
  If nothing is registered, or if any version support TCP, then we take the
  current approach or waiting and trying again on the assumption that the
  server is still starting up and wasn't quite ready yet when we got the
  ECONNREFUSED.
  However if UDP support is available but TCP isn't, and if the mount options
  don't specify a protocol, then fall back to v2v3.

So here is my second draft.  The 'verbose' results will be a bit confusing
but I think the functionality is close to what I want - it just does the pmap
lookups twice.

NeilBrown


diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 0241400..913b563 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -93,6 +93,7 @@ struct nfsmount_info {
 	char			**extra_opts;	/* string for /etc/mtab */
 
 	unsigned long		version;	/* NFS version */
+	unsigned long		proto;		/* protocol chosen */
 	int			flags,		/* MS_ flags */
 				fake,		/* actually do the mount? */
 				child;		/* forked bg child? */
@@ -625,6 +626,9 @@ static int nfs_do_mount_v3v2(struct nfsmount_info *mi,
 	if (!nfs_rewrite_pmap_mount_options(options))
 		goto out_fail;
 
+	/* record which protocol was chosen */
+	nfs_nfs_protocol(options, &mi->proto);
+
 	result = nfs_sys_mount(mi, options);
 
 out_fail:
@@ -762,6 +766,29 @@ static int nfs_try_mount(struct nfsmount_info *mi)
 			errno = 0;
 			result = nfs_try_mount_v4(mi);
 			if (errno != EPROTONOSUPPORT) {
+				/* If server only handles UDP, then v4 will have
+				 * received ECONNREFUSED for TCP, so fall through
+				 * to v3v2 which can try udp, but only if tcp
+				 * wasn't explicitly requested
+				 */
+				if (errno == ECONNREFUSED) {
+					unsigned long proto;
+					if (nfs_nfs_protocol(mi->options, &proto) == 1 &&
+					    proto == 0) {
+						/* OK to try different protocols. Lets see
+						 * what portmap thinks.
+						 */
+						int oldfake = mi->fake;
+						int res;
+						mi->fake = 1;
+						res = nfs_try_mount_v3v2(mi);
+						mi->fake = oldfake;
+						if (!res || mi->proto != IPPROTO_UDP)
+							/* time out and try again */
+							break;
+					} else
+						break;
+				} else
 				/* 
 				 * To deal with legacy Linux servers that don't
 				 * automatically export a pseudo root, retry



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: mount.nfs: protocol fallback when server doesn't support TCP
  2010-08-30  0:30   ` Neil Brown
@ 2010-08-30 19:29     ` Chuck Lever
  2010-08-31  1:00       ` Neil Brown
  0 siblings, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2010-08-30 19:29 UTC (permalink / raw)
  To: Neil Brown; +Cc: Linux NFS Mailing list


On Aug 29, 2010, at 8:30 PM, Neil Brown wrote:

> On Thu, 26 Aug 2010 10:51:23 -0400
> Chuck Lever <chuck.lever@oracle.com> wrote:
> 
>> 
>> On Aug 25, 2010, at 10:06 PM, Neil Brown wrote:
>> 
>>> 
>>> Currently if the server doesn't support TCP (and so doesn't support NFSv4)
>>> we don't fall-back to NFSv3.  This is because 'ECONNREFUSED' isn't deemed to
>>> be a suitable error for falling back.  Rather we wait for about 2 minutes,
>>> then give up.
>>> 
>>> There is some justification for this:  ECONNREFUSED could just mean that the
>>> server isn't quite ready yet.
>>> 
>>> I'm not really sure what the best thing to do here would be.  We don't really
>>> want to try v3 and have fail only because it was just enough later that nfsd
>>> is now responding.
>>> 
>>> Maybe the ideal would be to do a portmap probe and only fall back to v3 if
>>> portmap confirm that v3 is supported and v4 isn't.
>>> 
>>> For now I just present this patch which allows fallback to v3 providing tcp
>>> wasn't explicitly requested.
>>> 
>>> Thoughts?
>> 
>> What are the risks of checking portmap in this case?  That seems like it would give a better chance of a desirable outcome.
> 
> True, I was just too lazy at the time.
> 
> How about this:
>  If nfs_try_mount_v4 return ECONNREFUSED we do a portmap lookup and see
>  which versions and protocols are supported.
>  If nothing is registered, or if any version support TCP, then we take the
>  current approach or waiting and trying again on the assumption that the
>  server is still starting up and wasn't quite ready yet when we got the
>  ECONNREFUSED.
>  However if UDP support is available but TCP isn't, and if the mount options
>  don't specify a protocol, then fall back to v2v3.

That doesn't sound too bad.

> So here is my second draft.  The 'verbose' results will be a bit confusing
> but I think the functionality is close to what I want - it just does the pmap
> lookups twice.
> 
> NeilBrown
> 
> 
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index 0241400..913b563 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -93,6 +93,7 @@ struct nfsmount_info {
> 	char			**extra_opts;	/* string for /etc/mtab */
> 
> 	unsigned long		version;	/* NFS version */
> +	unsigned long		proto;		/* protocol chosen */
> 	int			flags,		/* MS_ flags */
> 				fake,		/* actually do the mount? */
> 				child;		/* forked bg child? */
> @@ -625,6 +626,9 @@ static int nfs_do_mount_v3v2(struct nfsmount_info *mi,
> 	if (!nfs_rewrite_pmap_mount_options(options))
> 		goto out_fail;
> 
> +	/* record which protocol was chosen */
> +	nfs_nfs_protocol(options, &mi->proto);

Don't see why this is necessary...?

The mount retry loop operates on a copy of the command line options; each iteration gets a fresh copy of the original options.  Maybe it's lunchtime and I'm hungry enough that I just don't understand how you are using mi->proto.

> +
> 	result = nfs_sys_mount(mi, options);
> 
> out_fail:
> @@ -762,6 +766,29 @@ static int nfs_try_mount(struct nfsmount_info *mi)
> 			errno = 0;
> 			result = nfs_try_mount_v4(mi);
> 			if (errno != EPROTONOSUPPORT) {
> +				/* If server only handles UDP, then v4 will have
> +				 * received ECONNREFUSED for TCP, so fall through
> +				 * to v3v2 which can try udp, but only if tcp
> +				 * wasn't explicitly requested
> +				 */
> +				if (errno == ECONNREFUSED) {
> +					unsigned long proto;
> +					if (nfs_nfs_protocol(mi->options, &proto) == 1 &&
> +					    proto == 0) {

Is the protocol setting loop-invariant?  Shouldn't you use mi->proto here?

> +						/* OK to try different protocols. Lets see
> +						 * what portmap thinks.
> +						 */
> +						int oldfake = mi->fake;
> +						int res;
> +						mi->fake = 1;
> +						res = nfs_try_mount_v3v2(mi);

If you just want to probe the server's portmapper, I think you can use nfs_probe_bothports() directly.

> +						mi->fake = oldfake;
> +						if (!res || mi->proto != IPPROTO_UDP)
> +							/* time out and try again */
> +							break;
> +					} else
> +						break;
> +				} else
> 				/* 
> 				 * To deal with legacy Linux servers that don't
> 				 * automatically export a pseudo root, retry
> 
> 

This is a lot of logic to stuff in here, where it is already fairly congested.  I would encourage you to move it to a helper function if that's practical.

-- 
chuck[dot]lever[at]oracle[dot]com





^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: mount.nfs: protocol fallback when server doesn't support TCP
  2010-08-30 19:29     ` Chuck Lever
@ 2010-08-31  1:00       ` Neil Brown
  2010-08-31 16:38         ` Chuck Lever
  2010-08-31 20:29         ` Chuck Lever
  0 siblings, 2 replies; 14+ messages in thread
From: Neil Brown @ 2010-08-31  1:00 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing list

On Mon, 30 Aug 2010 15:29:03 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:

> 
> On Aug 29, 2010, at 8:30 PM, Neil Brown wrote:
> 
> > On Thu, 26 Aug 2010 10:51:23 -0400
> > Chuck Lever <chuck.lever@oracle.com> wrote:
> > 
> >> 
> >> On Aug 25, 2010, at 10:06 PM, Neil Brown wrote:
> >> 
> > How about this:
> >  If nfs_try_mount_v4 return ECONNREFUSED we do a portmap lookup and see
> >  which versions and protocols are supported.
> >  If nothing is registered, or if any version support TCP, then we take the
> >  current approach or waiting and trying again on the assumption that the
> >  server is still starting up and wasn't quite ready yet when we got the
> >  ECONNREFUSED.
> >  However if UDP support is available but TCP isn't, and if the mount options
> >  don't specify a protocol, then fall back to v2v3.
> 
> That doesn't sound too bad.

Good.  Thanks.

> 
> > So here is my second draft.  The 'verbose' results will be a bit confusing
> > but I think the functionality is close to what I want - it just does the pmap
> > lookups twice.
> > 
> > NeilBrown
> > 
> > 
> > diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> > index 0241400..913b563 100644
> > --- a/utils/mount/stropts.c
> > +++ b/utils/mount/stropts.c
> > @@ -93,6 +93,7 @@ struct nfsmount_info {
> > 	char			**extra_opts;	/* string for /etc/mtab */
> > 
> > 	unsigned long		version;	/* NFS version */
> > +	unsigned long		proto;		/* protocol chosen */
> > 	int			flags,		/* MS_ flags */
> > 				fake,		/* actually do the mount? */
> > 				child;		/* forked bg child? */
> > @@ -625,6 +626,9 @@ static int nfs_do_mount_v3v2(struct nfsmount_info *mi,
> > 	if (!nfs_rewrite_pmap_mount_options(options))
> > 		goto out_fail;
> > 
> > +	/* record which protocol was chosen */
> > +	nfs_nfs_protocol(options, &mi->proto);
> 
> Don't see why this is necessary...?
> 
> The mount retry loop operates on a copy of the command line options; each iteration gets a fresh copy of the original options.  Maybe it's lunchtime and I'm hungry enough that I just don't understand how you are using mi->proto.

At the interesting moment when an NFSv4 attempt returned ECONNREFUSED we only
want to try a V3 mount if it would use UDP (or UDP6 I guess).

The easiest way to determine this is to go through the motions with a 'fake'
mount attempt and see what protocol ends up being used.
mi->proto is used simply to pass the final decision back up to the caller so
we can know what was decided.
The string options which also would record this are not passed back up.


> 
> > +
> > 	result = nfs_sys_mount(mi, options);
> > 
> > out_fail:
> > @@ -762,6 +766,29 @@ static int nfs_try_mount(struct nfsmount_info *mi)
> > 			errno = 0;
> > 			result = nfs_try_mount_v4(mi);
> > 			if (errno != EPROTONOSUPPORT) {
> > +				/* If server only handles UDP, then v4 will have
> > +				 * received ECONNREFUSED for TCP, so fall through
> > +				 * to v3v2 which can try udp, but only if tcp
> > +				 * wasn't explicitly requested
> > +				 */
> > +				if (errno == ECONNREFUSED) {
> > +					unsigned long proto;
> > +					if (nfs_nfs_protocol(mi->options, &proto) == 1 &&
> > +					    proto == 0) {
> 
> Is the protocol setting loop-invariant?  Shouldn't you use mi->proto here?

This is happening before any call to nfs_try_mountv3v2 so mi->proto would not
be set.  At this point we just want a quick look a the options to see if a
protocol was specified.  If it was the fallback from v4 over tcp to v3 over
udp would clearly be wrong and can be avoided.  So we really just want a
temporary variable here to examine the options.
Yes, it is a loop-invariant, but there would not be a lot to gain by making
'proto' a more global variable.


> 
> > +						/* OK to try different protocols. Lets see
> > +						 * what portmap thinks.
> > +						 */
> > +						int oldfake = mi->fake;
> > +						int res;
> > +						mi->fake = 1;
> > +						res = nfs_try_mount_v3v2(mi);
> 
> If you just want to probe the server's portmapper, I think you can use nfs_probe_bothports() directly.
> 

Not quite.  You would need to wrap it in a loop over all addresses in
md->address, and would need to worry about the mounthost option.  It seems
easier to just call nfs_try_mount_v3v2 which already does this.




> > +						mi->fake = oldfake;
> > +						if (!res || mi->proto != IPPROTO_UDP)
> > +							/* time out and try again */
> > +							break;
> > +					} else
> > +						break;
> > +				} else
> > 				/* 
> > 				 * To deal with legacy Linux servers that don't
> > 				 * automatically export a pseudo root, retry
> > 
> > 
> 
> This is a lot of logic to stuff in here, where it is already fairly congested.  I would encourage you to move it to a helper function if that's practical.
> 

That is a fairly common (and valid) complaint about my coding style.
My post-hoc excuse is that I was going to clean it up once I got the logic
sorted out properly.

Two observations I've made while exploring which are only tangential to the
current issue:

 1/ if I give an invalid proto name
          mount -o proto=fred .....
    the mount command fails (as it should) but gives no error message.

 2/ The options saved in /etc/mtab does not include any automatically
    determined options.  For mountport that is reasonable.  For proto that is
    possibly justified though I am less sure, but for vers it seems wrong.
    /etc/mtab needs to reccord if an unmount request shold be sent which
    means it needs to know which of v2/3 or v4 was used.

NeilBrown

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: mount.nfs: protocol fallback when server doesn't support TCP
  2010-08-31  1:00       ` Neil Brown
@ 2010-08-31 16:38         ` Chuck Lever
  2010-09-07  1:32           ` Neil Brown
  2010-08-31 20:29         ` Chuck Lever
  1 sibling, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2010-08-31 16:38 UTC (permalink / raw)
  To: Neil Brown; +Cc: Linux NFS Mailing list


On Aug 30, 2010, at 9:00 PM, Neil Brown wrote:

> On Mon, 30 Aug 2010 15:29:03 -0400
> Chuck Lever <chuck.lever@oracle.com> wrote:
> 
>> 
>> On Aug 29, 2010, at 8:30 PM, Neil Brown wrote:
>> 
>>> On Thu, 26 Aug 2010 10:51:23 -0400
>>> Chuck Lever <chuck.lever@oracle.com> wrote:
>>> 
>>>> 
>>>> On Aug 25, 2010, at 10:06 PM, Neil Brown wrote:
>>>> 
>>> How about this:
>>> If nfs_try_mount_v4 return ECONNREFUSED we do a portmap lookup and see
>>> which versions and protocols are supported.
>>> If nothing is registered, or if any version support TCP, then we take the
>>> current approach or waiting and trying again on the assumption that the
>>> server is still starting up and wasn't quite ready yet when we got the
>>> ECONNREFUSED.
>>> However if UDP support is available but TCP isn't, and if the mount options
>>> don't specify a protocol, then fall back to v2v3.
>> 
>> That doesn't sound too bad.
> 
> Good.  Thanks.
> 
>> 
>>> So here is my second draft.  The 'verbose' results will be a bit confusing
>>> but I think the functionality is close to what I want - it just does the pmap
>>> lookups twice.
>>> 
>>> NeilBrown
>>> 
>>> 
>>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
>>> index 0241400..913b563 100644
>>> --- a/utils/mount/stropts.c
>>> +++ b/utils/mount/stropts.c
>>> @@ -93,6 +93,7 @@ struct nfsmount_info {
>>> 	char			**extra_opts;	/* string for /etc/mtab */
>>> 
>>> 	unsigned long		version;	/* NFS version */
>>> +	unsigned long		proto;		/* protocol chosen */
>>> 	int			flags,		/* MS_ flags */
>>> 				fake,		/* actually do the mount? */
>>> 				child;		/* forked bg child? */
>>> @@ -625,6 +626,9 @@ static int nfs_do_mount_v3v2(struct nfsmount_info *mi,
>>> 	if (!nfs_rewrite_pmap_mount_options(options))
>>> 		goto out_fail;
>>> 
>>> +	/* record which protocol was chosen */
>>> +	nfs_nfs_protocol(options, &mi->proto);
>> 
>> Don't see why this is necessary...?
>> 
>> The mount retry loop operates on a copy of the command line options; each iteration gets a fresh copy of the original options.  Maybe it's lunchtime and I'm hungry enough that I just don't understand how you are using mi->proto.
> 
> At the interesting moment when an NFSv4 attempt returned ECONNREFUSED we only
> want to try a V3 mount if it would use UDP (or UDP6 I guess).
> 
> The easiest way to determine this is to go through the motions with a 'fake'
> mount attempt and see what protocol ends up being used.
> mi->proto is used simply to pass the final decision back up to the caller so
> we can know what was decided.
> The string options which also would record this are not passed back up.
> 
> 
>> 
>>> +
>>> 	result = nfs_sys_mount(mi, options);
>>> 
>>> out_fail:
>>> @@ -762,6 +766,29 @@ static int nfs_try_mount(struct nfsmount_info *mi)
>>> 			errno = 0;
>>> 			result = nfs_try_mount_v4(mi);
>>> 			if (errno != EPROTONOSUPPORT) {
>>> +				/* If server only handles UDP, then v4 will have
>>> +				 * received ECONNREFUSED for TCP, so fall through
>>> +				 * to v3v2 which can try udp, but only if tcp
>>> +				 * wasn't explicitly requested
>>> +				 */
>>> +				if (errno == ECONNREFUSED) {
>>> +					unsigned long proto;
>>> +					if (nfs_nfs_protocol(mi->options, &proto) == 1 &&
>>> +					    proto == 0) {
>> 
>> Is the protocol setting loop-invariant?  Shouldn't you use mi->proto here?
> 
> This is happening before any call to nfs_try_mountv3v2 so mi->proto would not
> be set.  At this point we just want a quick look a the options to see if a
> protocol was specified.  If it was the fallback from v4 over tcp to v3 over
> udp would clearly be wrong and can be avoided.  So we really just want a
> temporary variable here to examine the options.
> Yes, it is a loop-invariant, but there would not be a lot to gain by making
> 'proto' a more global variable.

The context saved in the mount_info struct is probably not quite logically appropriate for storing temporary values across retry iterations.  That could be a pseudo-rationale for grabbing the command-line specified protocol value at the same time as the command-line specified NFS version, in nfs_validate_options(), before the retry loop starts.  Another rationale might be making the logic in here a little simpler (not that I'm complaining, just making an observation).

>> 
>>> +						/* OK to try different protocols. Lets see
>>> +						 * what portmap thinks.
>>> +						 */
>>> +						int oldfake = mi->fake;
>>> +						int res;
>>> +						mi->fake = 1;
>>> +						res = nfs_try_mount_v3v2(mi);
>> 
>> If you just want to probe the server's portmapper, I think you can use nfs_probe_bothports() directly.
>> 
> 
> Not quite.  You would need to wrap it in a loop over all addresses in
> md->address, and would need to worry about the mounthost option.  It seems
> easier to just call nfs_try_mount_v3v2 which already does this.

/me smacks forehead

Complexity that I guess we have to live with.  A comment that explains the oldfake hack would be helpful.

>>> +						mi->fake = oldfake;
>>> +						if (!res || mi->proto != IPPROTO_UDP)
>>> +							/* time out and try again */
>>> +							break;
>>> +					} else
>>> +						break;
>>> +				} else
>>> 				/* 
>>> 				 * To deal with legacy Linux servers that don't
>>> 				 * automatically export a pseudo root, retry
>>> 
>>> 
>> 
>> This is a lot of logic to stuff in here, where it is already fairly congested.  I would encourage you to move it to a helper function if that's practical.
>> 
> 
> That is a fairly common (and valid) complaint about my coding style.
> My post-hoc excuse is that I was going to clean it up once I got the logic
> sorted out properly.

Sorry for the nudge.

> Two observations I've made while exploring which are only tangential to the
> current issue:
> 
> 1/ if I give an invalid proto name
>          mount -o proto=fred .....
>    the mount command fails (as it should) but gives no error message.

Thanks for the report.  Does this occur with both a legacy glibc RPC build and a libtirpc-enabled build?

> 2/ The options saved in /etc/mtab does not include any automatically
>    determined options.  For mountport that is reasonable.  For proto that is
>    possibly justified though I am less sure, but for vers it seems wrong.
>    /etc/mtab needs to reccord if an unmount request shold be sent which
>    means it needs to know which of v2/3 or v4 was used.

When deciding what mount options should go in /etc/mtab, the guide I use is: if I want to reproduce the same behavior as when the original mount was done, what options would I put in the saved mount option string?  In this case, I think the NFS version setting ostensibly should be added.

However, when mount option negotiation is in effect, it becomes a little less clear.  Basically in that case, we should put in the options specified on the command line, and let the next mount command instantiation renegotiate if needed.

I suspect the problem here is that the logic that updates /etc/mtab assumes the legacy world where the file system type was the sole determinant of whether NFS version 4 is in use.  So the NFSv4 mtab updating code doesn't bother to stick a version option in when needed (the new "-t nfs vers=4" case).

This probably has some consequences for umount, since it decides whether to send a UMNT to the server based on what NFS version is listed in /etc/mtab.

-- 
chuck[dot]lever[at]oracle[dot]com





^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: mount.nfs: protocol fallback when server doesn't support TCP
  2010-08-31  1:00       ` Neil Brown
  2010-08-31 16:38         ` Chuck Lever
@ 2010-08-31 20:29         ` Chuck Lever
  2010-09-07  0:02           ` Neil Brown
  1 sibling, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2010-08-31 20:29 UTC (permalink / raw)
  To: Neil Brown; +Cc: Linux NFS Mailing list


On Aug 30, 2010, at 9:00 PM, Neil Brown wrote:

> Two observations I've made while exploring which are only tangential to the
> current issue:
> 
> 1/ if I give an invalid proto name
>          mount -o proto=fred .....
>    the mount command fails (as it should) but gives no error message.

I was not able to reproduce this.  I get an appropriate error message from mount.nfs when built either with TI-RPC or without.  I'm using the latest upstream nfs-utils + my mountd patches.

-- 
chuck[dot]lever[at]oracle[dot]com





^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: mount.nfs: protocol fallback when server doesn't support TCP
  2010-08-31 20:29         ` Chuck Lever
@ 2010-09-07  0:02           ` Neil Brown
  0 siblings, 0 replies; 14+ messages in thread
From: Neil Brown @ 2010-09-07  0:02 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing list

On Tue, 31 Aug 2010 16:29:59 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:

> 
> On Aug 30, 2010, at 9:00 PM, Neil Brown wrote:
> 
> > Two observations I've made while exploring which are only tangential to the
> > current issue:
> > 
> > 1/ if I give an invalid proto name
> >          mount -o proto=fred .....
> >    the mount command fails (as it should) but gives no error message.
> 
> I was not able to reproduce this.  I get an appropriate error message from mount.nfs when built either with TI-RPC or without.  I'm using the latest upstream nfs-utils + my mountd patches.
> 

Ahh yes, fixed by

commit 9ac7a15017b876d4d8d3a4502ebaf954f36f7f54
Author: Steve Dickson <steved@redhat.com>
Date:   Thu Jun 3 08:53:22 2010 -0400

    mount.nfs: silently fails when the network protocol is not found
    
    mount.nfs should display some type of error diagnostics when
    the network protocol can not be determined.
    
    Signed-off-by: Steve Dickson <steved@redhat.com>


Thanks,
NeilBrown

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: mount.nfs: protocol fallback when server doesn't support TCP
  2010-08-31 16:38         ` Chuck Lever
@ 2010-09-07  1:32           ` Neil Brown
  2010-09-07 17:37             ` Chuck Lever
  0 siblings, 1 reply; 14+ messages in thread
From: Neil Brown @ 2010-09-07  1:32 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing list

On Tue, 31 Aug 2010 12:38:09 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:


> >> 
> >>> +						/* OK to try different protocols. Lets see
> >>> +						 * what portmap thinks.
> >>> +						 */
> >>> +						int oldfake = mi->fake;
> >>> +						int res;
> >>> +						mi->fake = 1;
> >>> +						res = nfs_try_mount_v3v2(mi);
> >> 
> >> If you just want to probe the server's portmapper, I think you can use nfs_probe_bothports() directly.
> >> 
> > 
> > Not quite.  You would need to wrap it in a loop over all addresses in
> > md->address, and would need to worry about the mounthost option.  It seems
> > easier to just call nfs_try_mount_v3v2 which already does this.
> 
> /me smacks forehead
> 
> Complexity that I guess we have to live with.  A comment that explains the oldfake hack would be helpful.

And often trying to write such a comment can make it clear why the code was a
bad idea in the first place.... :-)

Rather than write a comment I wouldn't believe myself, here is something
quite different - my third draft.
This is on top of the nice tidy-up patches you posted earlier.

It uses nfs_getport to make very focussed requests to portmap.

Two things I'm still not really sure about:

1/ RDMA.  How does that interact with fallback.  Do we only support v4 over
   RDMA?  That would be nice was we could simply avoid fallback in that case.

2/ IPv6.  I should possibly be checking if v3v2 are available via UDP6 as well
   as UDP - and similarly if v4 has become available via TCP6?? Or is that
   all magically handled somewhere under the hood?

Thanks,
NeilBrown

>From 60dae9b9c14869ae630db683e2524ad230064b19 Mon Sep 17 00:00:00 2001
From: Neil Brown <neilb@suse.de>
Date: Tue, 7 Sep 2010 11:23:02 +1000
Subject: [PATCH] mount: correctly handle fallback for  ECONNREFUSED during mount attempt.

If we get ECONNREFUSED when attempting a v4 mount, it could be that
the server just isn't ready yet (the current assumption) or it could
be that the server only support UDP - and thus only v3/v2.  The latter
possibility isn't handled currently.

So if we get ECONNREFUSED when fallback is an option, check with
portmap/rpcbind to see if it could be that case that v2/v3 are
supported over UDP, and v4 still is not.  In that case, fall back to v2v3.

I'm not at all sure if IPv6 possibilities are RDMA are handled correctly..

Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 9e29a19..5ebcf3e 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -750,6 +750,7 @@ static int nfs_autonegotiate(struct nfsmount_info *mi)
 {
 	unsigned long protocol;
 	int result;
+	struct addrinfo *ai;
 
 	/*
 	 * If UDP was requested from the command line, try
@@ -796,6 +797,33 @@ static int nfs_autonegotiate(struct nfsmount_info *mi)
 		/* Linux servers prior to 2.6.25 may return
 		 * EPERM when NFS version 4 is not supported. */
 		goto fall_back;
+	case ECONNREFUSED:
+		/* Either the server doesn't accept TCP connections,
+		 * or it just isn't quite ready yet.
+		 * In the first case, fall back to v3v2, in the second
+		 * return failure - as this isn't a permanent error
+		 * the attempt will be retried.
+		 * To determine which, we need to check with portmap/rpcbind.
+		 * If v2 or v3 are supported on UDP, but v4 isn't on TCP then fall-back.
+		 */
+		for (ai = mi->address; ai != NULL; ai = ai->ai_next) {
+			if (nfs_getport(ai->ai_addr, ai->ai_addrlen,
+					NFS_PROGRAM, 3, IPPROTO_UDP) != 0 ||
+			    nfs_getport(ai->ai_addr, ai->ai_addrlen,
+					NFS_PROGRAM, 2, IPPROTO_UDP) != 0) {
+				if (nfs_getport(ai->ai_addr, ai->ai_addrlen,
+						NFS_PROGRAM, 4, IPPROTO_TCP) != 0) {
+					/* v4 support is almost online, wait for it. */
+					errno = ECONNREFUSED;
+					return result;
+				} else
+					/* v2/v3 is supported, but v4 isn't, fall_back */
+					goto fall_back;
+			}
+		}
+		/* portmap is not responding yet either, so just try again */
+		errno = ECONNREFUSED;
+		return result;
 	default:
 		return result;
 	}

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: mount.nfs: protocol fallback when server doesn't support TCP
  2010-09-07  1:32           ` Neil Brown
@ 2010-09-07 17:37             ` Chuck Lever
  2010-09-08  2:35               ` Neil Brown
  0 siblings, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2010-09-07 17:37 UTC (permalink / raw)
  To: Neil Brown; +Cc: Linux NFS Mailing list


On Sep 6, 2010, at 9:32 PM, Neil Brown wrote:

> On Tue, 31 Aug 2010 12:38:09 -0400
> Chuck Lever <chuck.lever@oracle.com> wrote:
> 
> 
>>>> 
>>>>> +						/* OK to try different protocols. Lets see
>>>>> +						 * what portmap thinks.
>>>>> +						 */
>>>>> +						int oldfake = mi->fake;
>>>>> +						int res;
>>>>> +						mi->fake = 1;
>>>>> +						res = nfs_try_mount_v3v2(mi);
>>>> 
>>>> If you just want to probe the server's portmapper, I think you can use nfs_probe_bothports() directly.
>>>> 
>>> 
>>> Not quite.  You would need to wrap it in a loop over all addresses in
>>> md->address, and would need to worry about the mounthost option.  It seems
>>> easier to just call nfs_try_mount_v3v2 which already does this.
>> 
>> /me smacks forehead
>> 
>> Complexity that I guess we have to live with.  A comment that explains the oldfake hack would be helpful.
> 
> And often trying to write such a comment can make it clear why the code was a
> bad idea in the first place.... :-)
> 
> Rather than write a comment I wouldn't believe myself, here is something
> quite different - my third draft.
> This is on top of the nice tidy-up patches you posted earlier.
> 
> It uses nfs_getport to make very focussed requests to portmap.
> 
> Two things I'm still not really sure about:
> 
> 1/ RDMA.  How does that interact with fallback.  Do we only support v4 over
>   RDMA?  That would be nice was we could simply avoid fallback in that case.

My impression is that today RDMA is stable only on vers=3; someday it should be stable with vers=4 too.

I think Steve and I need to agree on the final form of the RDMA patches before we can say with authority how fallback will work.

> 2/ IPv6.  I should possibly be checking if v3v2 are available via UDP6 as well
>   as UDP - and similarly if v4 has become available via TCP6?? Or is that
>   all magically handled somewhere under the hood?

nfs_try_mount_v4() will try all address families provided by getaddrinfo(3).  So when ECONNREFUSED finally bubbles up to nfs_try_mount(), mount.nfs should have already tried NFSv4 via both tcp and tcp6.

Anyway, this seems to answer my original question about "what are the risks of trying the portmapper-based approach?"  It looks like a complex solution no matter what.

I'm not suggesting you abandon this direction, but if the nfs_try_mount() logic does nothing more but fallback to v2/v3 if the NFSv4 mount attempt fails with ECONNREFUSED, what harm could that cause?

> From 60dae9b9c14869ae630db683e2524ad230064b19 Mon Sep 17 00:00:00 2001
> From: Neil Brown <neilb@suse.de>
> Date: Tue, 7 Sep 2010 11:23:02 +1000
> Subject: [PATCH] mount: correctly handle fallback for  ECONNREFUSED during mount attempt.
> 
> If we get ECONNREFUSED when attempting a v4 mount, it could be that
> the server just isn't ready yet (the current assumption) or it could
> be that the server only support UDP - and thus only v3/v2.  The latter
> possibility isn't handled currently.
> 
> So if we get ECONNREFUSED when fallback is an option, check with
> portmap/rpcbind to see if it could be that case that v2/v3 are
> supported over UDP, and v4 still is not.  In that case, fall back to v2v3.
> 
> I'm not at all sure if IPv6 possibilities are RDMA are handled correctly..
> 
> Signed-off-by: NeilBrown <neilb@suse.de>
> 
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index 9e29a19..5ebcf3e 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -750,6 +750,7 @@ static int nfs_autonegotiate(struct nfsmount_info *mi)
> {
> 	unsigned long protocol;
> 	int result;
> +	struct addrinfo *ai;
> 
> 	/*
> 	 * If UDP was requested from the command line, try
> @@ -796,6 +797,33 @@ static int nfs_autonegotiate(struct nfsmount_info *mi)
> 		/* Linux servers prior to 2.6.25 may return
> 		 * EPERM when NFS version 4 is not supported. */
> 		goto fall_back;
> +	case ECONNREFUSED:
> +		/* Either the server doesn't accept TCP connections,
> +		 * or it just isn't quite ready yet.
> +		 * In the first case, fall back to v3v2, in the second
> +		 * return failure - as this isn't a permanent error
> +		 * the attempt will be retried.
> +		 * To determine which, we need to check with portmap/rpcbind.
> +		 * If v2 or v3 are supported on UDP, but v4 isn't on TCP then fall-back.
> +		 */
> +		for (ai = mi->address; ai != NULL; ai = ai->ai_next) {
> +			if (nfs_getport(ai->ai_addr, ai->ai_addrlen,
> +					NFS_PROGRAM, 3, IPPROTO_UDP) != 0 ||
> +			    nfs_getport(ai->ai_addr, ai->ai_addrlen,
> +					NFS_PROGRAM, 2, IPPROTO_UDP) != 0) {
> +				if (nfs_getport(ai->ai_addr, ai->ai_addrlen,
> +						NFS_PROGRAM, 4, IPPROTO_TCP) != 0) {
> +					/* v4 support is almost online, wait for it. */
> +					errno = ECONNREFUSED;
> +					return result;
> +				} else
> +					/* v2/v3 is supported, but v4 isn't, fall_back */
> +					goto fall_back;
> +			}
> +		}
> +		/* portmap is not responding yet either, so just try again */
> +		errno = ECONNREFUSED;
> +		return result;
> 	default:
> 		return result;
> 	}

-- 
chuck[dot]lever[at]oracle[dot]com





^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: mount.nfs: protocol fallback when server doesn't support TCP
  2010-09-07 17:37             ` Chuck Lever
@ 2010-09-08  2:35               ` Neil Brown
  2010-09-08 18:52                 ` Chuck Lever
  0 siblings, 1 reply; 14+ messages in thread
From: Neil Brown @ 2010-09-08  2:35 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing list

On Tue, 7 Sep 2010 13:37:40 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:

> 
> On Sep 6, 2010, at 9:32 PM, Neil Brown wrote:
> 
> > On Tue, 31 Aug 2010 12:38:09 -0400
> > Chuck Lever <chuck.lever@oracle.com> wrote:
> > 
> > 
> >>>> 
> >>>>> +						/* OK to try different protocols. Lets see
> >>>>> +						 * what portmap thinks.
> >>>>> +						 */
> >>>>> +						int oldfake = mi->fake;
> >>>>> +						int res;
> >>>>> +						mi->fake = 1;
> >>>>> +						res = nfs_try_mount_v3v2(mi);
> >>>> 
> >>>> If you just want to probe the server's portmapper, I think you can use nfs_probe_bothports() directly.
> >>>> 
> >>> 
> >>> Not quite.  You would need to wrap it in a loop over all addresses in
> >>> md->address, and would need to worry about the mounthost option.  It seems
> >>> easier to just call nfs_try_mount_v3v2 which already does this.
> >> 
> >> /me smacks forehead
> >> 
> >> Complexity that I guess we have to live with.  A comment that explains the oldfake hack would be helpful.
> > 
> > And often trying to write such a comment can make it clear why the code was a
> > bad idea in the first place.... :-)
> > 
> > Rather than write a comment I wouldn't believe myself, here is something
> > quite different - my third draft.
> > This is on top of the nice tidy-up patches you posted earlier.
> > 
> > It uses nfs_getport to make very focussed requests to portmap.
> > 
> > Two things I'm still not really sure about:
> > 
> > 1/ RDMA.  How does that interact with fallback.  Do we only support v4 over
> >   RDMA?  That would be nice was we could simply avoid fallback in that case.
> 
> My impression is that today RDMA is stable only on vers=3; someday it should be stable with vers=4 too.
> 
> I think Steve and I need to agree on the final form of the RDMA patches before we can say with authority how fallback will work.
> 
> > 2/ IPv6.  I should possibly be checking if v3v2 are available via UDP6 as well
> >   as UDP - and similarly if v4 has become available via TCP6?? Or is that
> >   all magically handled somewhere under the hood?
> 
> nfs_try_mount_v4() will try all address families provided by getaddrinfo(3).  So when ECONNREFUSED finally bubbles up to nfs_try_mount(), mount.nfs should have already tried NFSv4 via both tcp and tcp6.
> 
> Anyway, this seems to answer my original question about "what are the risks of trying the portmapper-based approach?"  It looks like a complex solution no matter what.
> 
> I'm not suggesting you abandon this direction, but if the nfs_try_mount() logic does nothing more but fallback to v2/v3 if the NFSv4 mount attempt fails with ECONNREFUSED, what harm could that cause?

The risk is slightly unpredictable behaviour.
If we just fall-back it should normally do "the right thing".
However it could sometimes end up mounting with v3 even though the server
supports v4.  This would happen if the server and client were starting up at
much the same time and when the client attempts a v4 mount the server hasn't
started nfsd yet, but when the client then tries v3, the server has started
nfsd.

If the server supports v4 it would seem reasonable for the sysadmin not to
explicitly request v4 in fstab, and could be quite annoying and very hard to
debug if very occasionally clients ended up with a v3 mount.

Dunno - maybe I'm trying too hard.  Maybe just a comment in the man page that
"if version and/or protocol are not specified, mount.nfs will make
best-effort to mount something but does not guarantee to always provide the
same result".

Certainly just fall-through on ECONNREFUSED with be much the easiest option.

NeilBrown



> 
> > From 60dae9b9c14869ae630db683e2524ad230064b19 Mon Sep 17 00:00:00 2001
> > From: Neil Brown <neilb@suse.de>
> > Date: Tue, 7 Sep 2010 11:23:02 +1000
> > Subject: [PATCH] mount: correctly handle fallback for  ECONNREFUSED during mount attempt.
> > 
> > If we get ECONNREFUSED when attempting a v4 mount, it could be that
> > the server just isn't ready yet (the current assumption) or it could
> > be that the server only support UDP - and thus only v3/v2.  The latter
> > possibility isn't handled currently.
> > 
> > So if we get ECONNREFUSED when fallback is an option, check with
> > portmap/rpcbind to see if it could be that case that v2/v3 are
> > supported over UDP, and v4 still is not.  In that case, fall back to v2v3.
> > 
> > I'm not at all sure if IPv6 possibilities are RDMA are handled correctly..
> > 
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > 
> > diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> > index 9e29a19..5ebcf3e 100644
> > --- a/utils/mount/stropts.c
> > +++ b/utils/mount/stropts.c
> > @@ -750,6 +750,7 @@ static int nfs_autonegotiate(struct nfsmount_info *mi)
> > {
> > 	unsigned long protocol;
> > 	int result;
> > +	struct addrinfo *ai;
> > 
> > 	/*
> > 	 * If UDP was requested from the command line, try
> > @@ -796,6 +797,33 @@ static int nfs_autonegotiate(struct nfsmount_info *mi)
> > 		/* Linux servers prior to 2.6.25 may return
> > 		 * EPERM when NFS version 4 is not supported. */
> > 		goto fall_back;
> > +	case ECONNREFUSED:
> > +		/* Either the server doesn't accept TCP connections,
> > +		 * or it just isn't quite ready yet.
> > +		 * In the first case, fall back to v3v2, in the second
> > +		 * return failure - as this isn't a permanent error
> > +		 * the attempt will be retried.
> > +		 * To determine which, we need to check with portmap/rpcbind.
> > +		 * If v2 or v3 are supported on UDP, but v4 isn't on TCP then fall-back.
> > +		 */
> > +		for (ai = mi->address; ai != NULL; ai = ai->ai_next) {
> > +			if (nfs_getport(ai->ai_addr, ai->ai_addrlen,
> > +					NFS_PROGRAM, 3, IPPROTO_UDP) != 0 ||
> > +			    nfs_getport(ai->ai_addr, ai->ai_addrlen,
> > +					NFS_PROGRAM, 2, IPPROTO_UDP) != 0) {
> > +				if (nfs_getport(ai->ai_addr, ai->ai_addrlen,
> > +						NFS_PROGRAM, 4, IPPROTO_TCP) != 0) {
> > +					/* v4 support is almost online, wait for it. */
> > +					errno = ECONNREFUSED;
> > +					return result;
> > +				} else
> > +					/* v2/v3 is supported, but v4 isn't, fall_back */
> > +					goto fall_back;
> > +			}
> > +		}
> > +		/* portmap is not responding yet either, so just try again */
> > +		errno = ECONNREFUSED;
> > +		return result;
> > 	default:
> > 		return result;
> > 	}
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: mount.nfs: protocol fallback when server doesn't support TCP
  2010-09-08  2:35               ` Neil Brown
@ 2010-09-08 18:52                 ` Chuck Lever
  0 siblings, 0 replies; 14+ messages in thread
From: Chuck Lever @ 2010-09-08 18:52 UTC (permalink / raw)
  To: Neil Brown; +Cc: Linux NFS Mailing list


On Sep 7, 2010, at 10:35 PM, Neil Brown wrote:

> On Tue, 7 Sep 2010 13:37:40 -0400
> Chuck Lever <chuck.lever@oracle.com> wrote:
> 
>> 
>> On Sep 6, 2010, at 9:32 PM, Neil Brown wrote:
>> 
>>> On Tue, 31 Aug 2010 12:38:09 -0400
>>> Chuck Lever <chuck.lever@oracle.com> wrote:
>>> 
>>> 
>>>>>> 
>>>>>>> +						/* OK to try different protocols. Lets see
>>>>>>> +						 * what portmap thinks.
>>>>>>> +						 */
>>>>>>> +						int oldfake = mi->fake;
>>>>>>> +						int res;
>>>>>>> +						mi->fake = 1;
>>>>>>> +						res = nfs_try_mount_v3v2(mi);
>>>>>> 
>>>>>> If you just want to probe the server's portmapper, I think you can use nfs_probe_bothports() directly.
>>>>>> 
>>>>> 
>>>>> Not quite.  You would need to wrap it in a loop over all addresses in
>>>>> md->address, and would need to worry about the mounthost option.  It seems
>>>>> easier to just call nfs_try_mount_v3v2 which already does this.
>>>> 
>>>> /me smacks forehead
>>>> 
>>>> Complexity that I guess we have to live with.  A comment that explains the oldfake hack would be helpful.
>>> 
>>> And often trying to write such a comment can make it clear why the code was a
>>> bad idea in the first place.... :-)
>>> 
>>> Rather than write a comment I wouldn't believe myself, here is something
>>> quite different - my third draft.
>>> This is on top of the nice tidy-up patches you posted earlier.
>>> 
>>> It uses nfs_getport to make very focussed requests to portmap.
>>> 
>>> Two things I'm still not really sure about:
>>> 
>>> 1/ RDMA.  How does that interact with fallback.  Do we only support v4 over
>>>  RDMA?  That would be nice was we could simply avoid fallback in that case.
>> 
>> My impression is that today RDMA is stable only on vers=3; someday it should be stable with vers=4 too.
>> 
>> I think Steve and I need to agree on the final form of the RDMA patches before we can say with authority how fallback will work.
>> 
>>> 2/ IPv6.  I should possibly be checking if v3v2 are available via UDP6 as well
>>>  as UDP - and similarly if v4 has become available via TCP6?? Or is that
>>>  all magically handled somewhere under the hood?
>> 
>> nfs_try_mount_v4() will try all address families provided by getaddrinfo(3).  So when ECONNREFUSED finally bubbles up to nfs_try_mount(), mount.nfs should have already tried NFSv4 via both tcp and tcp6.
>> 
>> Anyway, this seems to answer my original question about "what are the risks of trying the portmapper-based approach?"  It looks like a complex solution no matter what.
>> 
>> I'm not suggesting you abandon this direction, but if the nfs_try_mount() logic does nothing more but fallback to v2/v3 if the NFSv4 mount attempt fails with ECONNREFUSED, what harm could that cause?
> 
> The risk is slightly unpredictable behaviour.
> If we just fall-back it should normally do "the right thing".
> However it could sometimes end up mounting with v3 even though the server
> supports v4.  This would happen if the server and client were starting up at
> much the same time and when the client attempts a v4 mount the server hasn't
> started nfsd yet, but when the client then tries v3, the server has started
> nfsd.
> 
> If the server supports v4 it would seem reasonable for the sysadmin not to
> explicitly request v4 in fstab, and could be quite annoying and very hard to
> debug if very occasionally clients ended up with a v3 mount.
> 
> Dunno - maybe I'm trying too hard.  Maybe just a comment in the man page that
> "if version and/or protocol are not specified, mount.nfs will make
> best-effort to mount something but does not guarantee to always provide the
> same result".

Yeah, I think that describes already the behavior of NFS autonegotiation.  If the user didn't bother to specify a version, then she shouldn't care if sometimes the mount command gets a lower than expected version.  To force the version setting, the admin can either disable v3 on the server, or specify "vers=4" on the client.

The problem with that kind of flexibility, of course, is that we assume that an NFSv4 mount will behave identically to an NFSv3 mount.

> Certainly just fall-through on ECONNREFUSED with be much the easiest option.

That might be appropriate for at least a short-term fix.  Not mounting a UDP-only server is clearly a regression.

> 
> NeilBrown
> 
> 
> 
>> 
>>> From 60dae9b9c14869ae630db683e2524ad230064b19 Mon Sep 17 00:00:00 2001
>>> From: Neil Brown <neilb@suse.de>
>>> Date: Tue, 7 Sep 2010 11:23:02 +1000
>>> Subject: [PATCH] mount: correctly handle fallback for  ECONNREFUSED during mount attempt.
>>> 
>>> If we get ECONNREFUSED when attempting a v4 mount, it could be that
>>> the server just isn't ready yet (the current assumption) or it could
>>> be that the server only support UDP - and thus only v3/v2.  The latter
>>> possibility isn't handled currently.
>>> 
>>> So if we get ECONNREFUSED when fallback is an option, check with
>>> portmap/rpcbind to see if it could be that case that v2/v3 are
>>> supported over UDP, and v4 still is not.  In that case, fall back to v2v3.
>>> 
>>> I'm not at all sure if IPv6 possibilities are RDMA are handled correctly..
>>> 
>>> Signed-off-by: NeilBrown <neilb@suse.de>
>>> 
>>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
>>> index 9e29a19..5ebcf3e 100644
>>> --- a/utils/mount/stropts.c
>>> +++ b/utils/mount/stropts.c
>>> @@ -750,6 +750,7 @@ static int nfs_autonegotiate(struct nfsmount_info *mi)
>>> {
>>> 	unsigned long protocol;
>>> 	int result;
>>> +	struct addrinfo *ai;
>>> 
>>> 	/*
>>> 	 * If UDP was requested from the command line, try
>>> @@ -796,6 +797,33 @@ static int nfs_autonegotiate(struct nfsmount_info *mi)
>>> 		/* Linux servers prior to 2.6.25 may return
>>> 		 * EPERM when NFS version 4 is not supported. */
>>> 		goto fall_back;
>>> +	case ECONNREFUSED:
>>> +		/* Either the server doesn't accept TCP connections,
>>> +		 * or it just isn't quite ready yet.
>>> +		 * In the first case, fall back to v3v2, in the second
>>> +		 * return failure - as this isn't a permanent error
>>> +		 * the attempt will be retried.
>>> +		 * To determine which, we need to check with portmap/rpcbind.
>>> +		 * If v2 or v3 are supported on UDP, but v4 isn't on TCP then fall-back.
>>> +		 */
>>> +		for (ai = mi->address; ai != NULL; ai = ai->ai_next) {
>>> +			if (nfs_getport(ai->ai_addr, ai->ai_addrlen,
>>> +					NFS_PROGRAM, 3, IPPROTO_UDP) != 0 ||
>>> +			    nfs_getport(ai->ai_addr, ai->ai_addrlen,
>>> +					NFS_PROGRAM, 2, IPPROTO_UDP) != 0) {
>>> +				if (nfs_getport(ai->ai_addr, ai->ai_addrlen,
>>> +						NFS_PROGRAM, 4, IPPROTO_TCP) != 0) {
>>> +					/* v4 support is almost online, wait for it. */
>>> +					errno = ECONNREFUSED;
>>> +					return result;
>>> +				} else
>>> +					/* v2/v3 is supported, but v4 isn't, fall_back */
>>> +					goto fall_back;
>>> +			}
>>> +		}
>>> +		/* portmap is not responding yet either, so just try again */
>>> +		errno = ECONNREFUSED;
>>> +		return result;
>>> 	default:
>>> 		return result;
>>> 	}
>> 
> 

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com




^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2010-09-08 18:53 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-26  2:06 mount.nfs: protocol fallback when server doesn't support TCP Neil Brown
2010-08-26 14:51 ` Chuck Lever
2010-08-26 15:27   ` Jim Rees
2010-08-26 16:09     ` Chuck Lever
2010-08-30  0:30   ` Neil Brown
2010-08-30 19:29     ` Chuck Lever
2010-08-31  1:00       ` Neil Brown
2010-08-31 16:38         ` Chuck Lever
2010-09-07  1:32           ` Neil Brown
2010-09-07 17:37             ` Chuck Lever
2010-09-08  2:35               ` Neil Brown
2010-09-08 18:52                 ` Chuck Lever
2010-08-31 20:29         ` Chuck Lever
2010-09-07  0:02           ` Neil Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.