2008-07-21 15:32:41

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH] mount: enable retry for nfs23 to set the correct protocol for mount.

On Mon, Jul 21, 2008 at 2:06 AM, Neil Brown <[email protected]> wrote:
> On Monday July 21, [email protected] wrote:
>> The workaround for older nfs-utils has been to specify
>> "mountproto=udp/tcp" explicitly.
> Yes, that's a usable workaround.
>> I think this works around the problem of having a server who's
>> portmapper listens only on TCP. The underlying portmap client in the
>> mount command (and in the kernel) should be able to deal with that
>> without tying the rpcbind transport to the transport that is being
>> queried, and that is what is truly broken here.
>> So I think I would rather see this problem addressed in the
>> portmap/rpcbind client implementations and not in the upper level
>> mountd clients. If a portmap query doesn't work over UDP, it seems
>> like it should try it over TCP instead, and then remember that for
>> later queries.
>> I think your objection would be that this doesn't address the
>> regression for particular combinations of nfs-utils and kernel
>> versions, but I don't have a better suggestion at the moment.
> My other objection is the you are pushing too much functionality into
> the kernel.


> The portmap lookup can be done in userspace, and so "should" be.

Well, the rpcbind step comes quite automatically for the mountd client
in the kernel. It's part of the RPC state machine for all client
connections. There is no extra logic in the kernel to do it for the
mountd client. It's really quite natural for it to do.

> I'm not even convinced that the mount lookup belongs in the kernel,
> though passing a (hex encoded) file handle down might be a bit clumsy.

The mountd client was already in the kernel (used for NFSROOT). We
simply adopted it for normal mounts too. User space has absolutely no
need to know the root file handle, but the kernel does. So I think it
makes more sense for the kernel to handle the mountd lookup.

> But I've decided that there are bigger fish in the sea and I'm going
> to let this one swim away. As you say, the enterprise releases are
> more important, and it looks like enough fixes will make their way in
> before SLES11 that this won't be an issue any more.
> I still hope the other two nfs-utils patches will get in.
>> Again, I may not be clear on the original requirements here. My
>> understanding is that the legacy mount command uses default settings
>> if "proto/mountproto" are not specified. It does not attempt to use
>> "the best" settings unless the default settings don't work. This is
>> what the text-based mount logic does -- it uses default settings
>> (NFSv3 over TCP, mountd v3 over UDP), then punts if it needs service
>> discovery.
> "requirements"? that would be nice?
> The legacy code tries every possibly combination of values that
> weren't explicitly given until it finds a combination that "works".
> It goes from most desirable to least desirable.

Yes, and the text-based mount command should do that as well if the
initial defaults don't work.

> The text-based mount logic doesn't do what you said any more.
> It default to mountd over TCP thanks to
> nfs_set_mount_transport_protocol().

I will have to look at it again. My version of this fix, at least,
made the kernel use TCP for mountd and NFS if "proto=tcp" is
specified, UDP for both if "proto=udp" is specified, and TCP for NFS
and UDP for mountd if none were specified. This is exactly how the
legacy mount command starts off.

If Trond's version of the fix doesn't do that, then that is a behavior

> Will this be a regression for someone else I wonder ....

Chuck Lever