All of lore.kernel.org
 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 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.