Linux NFS development
 help / color / mirror / Atom feed
From: Chuck Lever <chuck.lever@oracle.com>
To: Steve Dickson <SteveD@redhat.com>
Cc: Neil Brown <neilb@suse.de>,
	"'nfs@lists.sourceforge.net'" <nfs@lists.sourceforge.net>,
	Trond Myklebust <Trond.Myklebust@netapp.com>
Subject: Re: [PATCH] NFS: Sign Extentions with Tru64 FSIDs.
Date: Wed, 07 Mar 2007 09:38:44 -0500	[thread overview]
Message-ID: <45EECE74.6070606@oracle.com> (raw)
In-Reply-To: <45EDE060.3020501@RedHat.com>

[-- Attachment #1: Type: text/plain, Size: 5838 bytes --]

A comment...

You've implemented a "negative" -- the usual convention is to implement 
the "positive" option.   This is how Apple/*BSD do it for READDIRPLUS, 
and it might be good to follow existing precedent.

I think you should implement "rddirplus" and make it the default.  That 
way you get "norddirplus" automatically.

You should also include appropriate man page changes in your patch. ;-D

Steve Dickson wrote:
> 
> 
> Steve Dickson wrote:
>>
>> Trond Myklebust wrote:
>>> On Mon, 2007-03-05 at 17:22 -0500, Peter Staubach wrote:
>>>> Or we could just punt and document that READDIRPLUS must be disabled
>>>> on all Tru64 NFS servers if this problem is seen.
>>> That would also be an option.
>> Assuming there is a way to turn off READDIRPLUS  on the
>> server side... I'll look into it... Or are you suggesting
>> that we have a mount option to turn off READDIRPLUS on the
>> client side? (Note: I believe other implementations have
>> this mount option if that means anything)....
> Not too surprising, there does not seem to be away to
> disable READDIRPLUS on the server side.... so...
> Since it was incredibly easy to disable READDIRPLUS
> from the client side... I went ahead came up and tested
> the attached patches...
> 
> Again, I believe there are other implementations, like Apple,
> that have this type of knob so in the end, this might be
> a good thing to have in general... It definitely helps with
> broken Tru64 servers...  So please consider...
> 
> tia,
> 
> steved.
> 
> 
> ------------------------------------------------------------------------
> 
> Added support to turn off the NFSv3 READDIRPLUS RPC.
> 
> Signed-off-by: Steve Dickson <steved@redhat.com>
> 
> ----------------------------------------------
> --- linux-2.6.20.i686/fs/nfs/super.c.orig	2007-03-05 11:26:41.130885000 -0500
> +++ linux-2.6.20.i686/fs/nfs/super.c	2007-03-06 16:10:13.390550000 -0500
> @@ -291,6 +291,7 @@
>  		{ NFS_MOUNT_NOAC, ",noac", "" },
>  		{ NFS_MOUNT_NONLM, ",nolock", "" },
>  		{ NFS_MOUNT_NOACL, ",noacl", "" },
> +		{ NFS_MOUNT_NORDIRPLUS, ",nordirplus", "" },
>  		{ 0, NULL, NULL }
>  	};
>  	const struct proc_nfs_info *nfs_infop;
> --- linux-2.6.20.i686/fs/nfs/client.c.orig	2007-03-05 11:26:40.995885000 -0500
> +++ linux-2.6.20.i686/fs/nfs/client.c	2007-03-06 11:07:23.208538000 -0500
> @@ -618,7 +618,8 @@
>  	if (clp->cl_nfsversion == 3) {
>  		if (server->namelen == 0 || server->namelen > NFS3_MAXNAMLEN)
>  			server->namelen = NFS3_MAXNAMLEN;
> -		server->caps |= NFS_CAP_READDIRPLUS;
> +		if (!(data->flags & NFS_MOUNT_NORDIRPLUS))
> +			server->caps |= NFS_CAP_READDIRPLUS;
>  	} else {
>  		if (server->namelen == 0 || server->namelen > NFS2_MAXNAMLEN)
>  			server->namelen = NFS2_MAXNAMLEN;
> --- linux-2.6.20.i686/include/linux/nfs_mount.h.orig	2007-02-04 13:44:54.000000000 -0500
> +++ linux-2.6.20.i686/include/linux/nfs_mount.h	2007-03-06 10:59:06.457362000 -0500
> @@ -61,6 +61,7 @@
>  #define NFS_MOUNT_NOACL		0x0800	/* 4 */
>  #define NFS_MOUNT_STRICTLOCK	0x1000	/* reserved for NFSv4 */
>  #define NFS_MOUNT_SECFLAVOUR	0x2000	/* 5 */
> +#define NFS_MOUNT_NORDIRPLUS	0x4000	/* 5 */
>  #define NFS_MOUNT_FLAGMASK	0xFFFF
>  
>  #endif
> 
> 
> ------------------------------------------------------------------------
> 
> Adds the -o nordirplus mount option that will disable 
> NFS clients from using the READDIRPLUS RPC.
> 
> Signed-off-by: Steve Dickson <steved@redhat.com>
> 
> ------------------------------
> --- nfs-utils-1.0.12/utils/mount/nfs_mount.h.orig	2007-03-06 15:41:19.557090000 -0500
> +++ nfs-utils-1.0.12/utils/mount/nfs_mount.h	2007-03-06 15:44:22.888922000 -0500
> @@ -63,6 +63,7 @@
>  #define NFS_MOUNT_BROKEN_SUID	0x0400	/* 4 */
>  #define NFS_MOUNT_NOACL     0x0800  /* 4 */
>  #define NFS_MOUNT_SECFLAVOUR	0x2000	/* 5 */
> +#define NFS_MOUNT_NORDIRPLUS	0x4000	/* 5 */
>  
>  /* security pseudoflavors */
>  
> --- nfs-utils-1.0.12/utils/mount/nfsmount.c.orig	2007-03-06 15:41:19.582094000 -0500
> +++ nfs-utils-1.0.12/utils/mount/nfsmount.c	2007-03-06 15:44:53.509748000 -0500
> @@ -788,6 +788,10 @@
>  				data->flags &= ~NFS_MOUNT_NOACL;
>  				if (!val)
>  					data->flags |= NFS_MOUNT_NOACL;
> +			} else if (!strcmp(opt, "rdirplus")) {
> +				data->flags &= ~NFS_MOUNT_NORDIRPLUS;
> +				if (!val)
> +					data->flags |= NFS_MOUNT_NORDIRPLUS;
>  #endif
>  			} else {
>  			bad_option:
> @@ -975,6 +979,7 @@
>  #endif
>  #if NFS_MOUNT_VERSION >= 5
>  	printf("sec = %u ", data.pseudoflavor);
> +	printf("readdirplus = %d ", (data.flags & NFS_MOUNT_NORDIRPLUS) != 0);
>  #endif
>  	printf("\n");
>  #endif
> --- nfs-utils-1.0.12/utils/mount/nfs.man.orig	2007-03-06 15:41:19.431090000 -0500
> +++ nfs-utils-1.0.12/utils/mount/nfs.man	2007-03-06 16:06:29.487656000 -0500
> @@ -272,6 +272,11 @@
>  .I udp
>  Mount the NFS filesystem using the UDP protocol.  This
>  is the default.
> +.TP 1.5i
> +.I nordirplus
> +Disables NFSv3 READDIRPLUS RPCs. Use this options when
> +mounting servers that don't support or have broken
> +READDIRPLUS implementations.
>  .P
>  All of the non-value options have corresponding nooption forms.
>  For example, nointr means don't allow file operations to be
> 
> 
> ------------------------------------------------------------------------
> 
> -------------------------------------------------------------------------
> Take Surveys. Earn Cash. Influence the Future of IT
> Join SourceForge.net's Techsay panel and you'll get the chance to share your
> opinions on IT & business topics through brief surveys-and earn cash
> http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> NFS maillist  -  NFS@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nfs


[-- Attachment #2: chuck.lever.vcf --]
[-- Type: text/x-vcard, Size: 265 bytes --]

begin:vcard
fn:Chuck Lever
n:Lever;Chuck
org:Oracle Corporation;Corporate Architecture Linux Projects Group
email;internet:chuck dot lever at nospam oracle dot com
title:Principal Member of Staff
tel;work:+1 248 614 5091
x-mozilla-html:FALSE
version:2.1
end:vcard


[-- Attachment #3: Type: text/plain, Size: 345 bytes --]

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[-- Attachment #4: Type: text/plain, Size: 140 bytes --]

_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

  parent reply	other threads:[~2007-03-07 14:39 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-03-05 20:35 [PATCH] NFS: Sign Extentions with Tru64 FSIDs Steve Dickson
2007-03-05 21:37 ` Trond Myklebust
2007-03-05 21:46   ` Peter Staubach
2007-03-05 22:03     ` Trond Myklebust
2007-03-05 22:22       ` Peter Staubach
2007-03-06 10:11         ` Peter Åstrand
2007-03-06 13:05         ` Trond Myklebust
2007-03-06 14:56           ` Steve Dickson
2007-03-06 21:42             ` Steve Dickson
2007-03-06 22:00               ` Talpey, Thomas
2007-03-07 14:16                 ` Steve Dickson
2007-03-07  7:45               ` Peter Åstrand
2007-03-07 14:21                 ` Steve Dickson
2007-03-07 14:38               ` Chuck Lever [this message]
2007-03-07 16:29                 ` Peter Staubach
2007-03-08  3:08                   ` Chuck Lever
2007-03-07 19:20                 ` Steve Dickson
2007-03-05 21:54   ` Dan Goetzman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=45EECE74.6070606@oracle.com \
    --to=chuck.lever@oracle.com \
    --cc=SteveD@redhat.com \
    --cc=Trond.Myklebust@netapp.com \
    --cc=neilb@suse.de \
    --cc=nfs@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox