All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] nfs: Fix ugly referral attributes
@ 2017-11-05 20:45 Chuck Lever
  2017-11-06 17:34 ` Frank Filz
  2018-05-17  7:53 ` Regression in [v2] nfs: Fix ugly referral attributes ? Moritz Schlarb
  0 siblings, 2 replies; 7+ messages in thread
From: Chuck Lever @ 2017-11-05 20:45 UTC (permalink / raw)
  To: trond.myklebust, anna.schumaker; +Cc: linux-nfs

Before traversing a referral and performing a mount, the mounted-on
directory looks strange:

dr-xr-xr-x. 2 4294967294 4294967294 0 Dec 31  1969 dir.0

nfs4_get_referral is wiping out any cached attributes with what was
returned via GETATTR(fs_locations), but the bit mask for that
operation does not request any file attributes.

Retrieve owner and timestamp information so that the memcpy in
nfs4_get_referral fills in more attributes.

Changes since v1:
- Don't request attributes that the client unconditionally replaces
- Request only MOUNTED_ON_FILEID or FILEID attribute, not both
- encode_fs_locations() doesn't use the third bitmask word

Fixes: 6b97fd3da1ea ("NFSv4: Follow a referral")
Suggested-by: Pradeep Thomas <pradeepthomas@gmail.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Cc: stable@vger.kernel.org
---
 fs/nfs/nfs4proc.c |   18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

I could send this as an incremental, but that just seems to piss
off distributors, who will just squash them all together anyway.

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 6c61e2b..2662879 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -254,15 +254,12 @@ static int nfs4_map_errors(int err)
 };
 
 const u32 nfs4_fs_locations_bitmap[3] = {
-	FATTR4_WORD0_TYPE
-	| FATTR4_WORD0_CHANGE
+	FATTR4_WORD0_CHANGE
 	| FATTR4_WORD0_SIZE
 	| FATTR4_WORD0_FSID
 	| FATTR4_WORD0_FILEID
 	| FATTR4_WORD0_FS_LOCATIONS,
-	FATTR4_WORD1_MODE
-	| FATTR4_WORD1_NUMLINKS
-	| FATTR4_WORD1_OWNER
+	FATTR4_WORD1_OWNER
 	| FATTR4_WORD1_OWNER_GROUP
 	| FATTR4_WORD1_RAWDEV
 	| FATTR4_WORD1_SPACE_USED
@@ -6763,9 +6760,7 @@ static int _nfs4_proc_fs_locations(struct rpc_clnt *client, struct inode *dir,
 				   struct page *page)
 {
 	struct nfs_server *server = NFS_SERVER(dir);
-	u32 bitmask[3] = {
-		[0] = FATTR4_WORD0_FSID | FATTR4_WORD0_FS_LOCATIONS,
-	};
+	u32 bitmask[3];
 	struct nfs4_fs_locations_arg args = {
 		.dir_fh = NFS_FH(dir),
 		.name = name,
@@ -6784,12 +6779,15 @@ static int _nfs4_proc_fs_locations(struct rpc_clnt *client, struct inode *dir,
 
 	dprintk("%s: start\n", __func__);
 
+	bitmask[0] = nfs4_fattr_bitmap[0] | FATTR4_WORD0_FS_LOCATIONS;
+	bitmask[1] = nfs4_fattr_bitmap[1];
+
 	/* Ask for the fileid of the absent filesystem if mounted_on_fileid
 	 * is not supported */
 	if (NFS_SERVER(dir)->attr_bitmask[1] & FATTR4_WORD1_MOUNTED_ON_FILEID)
-		bitmask[1] |= FATTR4_WORD1_MOUNTED_ON_FILEID;
+		bitmask[0] &= ~FATTR4_WORD0_FILEID;
 	else
-		bitmask[0] |= FATTR4_WORD0_FILEID;
+		bitmask[1] &= ~FATTR4_WORD1_MOUNTED_ON_FILEID;
 
 	nfs_fattr_init(&fs_locations->fattr);
 	fs_locations->server = server;


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

* RE: [PATCH v2] nfs: Fix ugly referral attributes
  2017-11-05 20:45 [PATCH v2] nfs: Fix ugly referral attributes Chuck Lever
@ 2017-11-06 17:34 ` Frank Filz
  2017-11-06 20:39   ` Pradeep
  2018-05-17  7:53 ` Regression in [v2] nfs: Fix ugly referral attributes ? Moritz Schlarb
  1 sibling, 1 reply; 7+ messages in thread
From: Frank Filz @ 2017-11-06 17:34 UTC (permalink / raw)
  To: 'Chuck Lever', trond.myklebust, anna.schumaker
  Cc: linux-nfs, 'Pradeep', nfs-ganesha-devel

Pradeep,

Could you verify this patch with nfs-ganesha? Looking at the code, it looks=
 like we will supply the attributes requested.

Thanks

Frank

> -----Original Message-----
> From: linux-nfs-owner@vger.kernel.org [mailto:linux-nfs-
> owner@vger.kernel.org] On Behalf Of Chuck Lever
> Sent: Sunday, November 5, 2017 12:45 PM
> To: trond.myklebust@primarydata.com; anna.schumaker@netapp.com
> Cc: linux-nfs@vger.kernel.org
> Subject: [PATCH v2] nfs: Fix ugly referral attributes
> 
> Before traversing a referral and performing a mount, the mounted-on
> directory looks strange:
> 
> dr-xr-xr-x. 2 4294967294 4294967294 0 Dec 31  1969 dir.0
> 
> nfs4_get_referral is wiping out any cached attributes with what was retur=
ned
> via GETATTR(fs_locations), but the bit mask for that operation does not
> request any file attributes.
> 
> Retrieve owner and timestamp information so that the memcpy in
> nfs4_get_referral fills in more attributes.
> 
> Changes since v1:
> - Don't request attributes that the client unconditionally replaces
> - Request only MOUNTED_ON_FILEID or FILEID attribute, not both
> - encode_fs_locations() doesn't use the third bitmask word
> 
> Fixes: 6b97fd3da1ea ("NFSv4: Follow a referral")
> Suggested-by: Pradeep Thomas <pradeepthomas@gmail.com>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> Cc: stable@vger.kernel.org
> ---
>  fs/nfs/nfs4proc.c |   18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> I could send this as an incremental, but that just seems to piss off
> distributors, who will just squash them all together anyway.
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 6c61e2b..2662879=

> 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -254,15 +254,12 @@ static int nfs4_map_errors(int err)  };
> 
>  const u32 nfs4_fs_locations_bitmap[3] =3D {
> -	FATTR4_WORD0_TYPE
> -	| FATTR4_WORD0_CHANGE
> +	FATTR4_WORD0_CHANGE
>  	| FATTR4_WORD0_SIZE
>  	| FATTR4_WORD0_FSID
>  	| FATTR4_WORD0_FILEID
>  	| FATTR4_WORD0_FS_LOCATIONS,
> -	FATTR4_WORD1_MODE
> -	| FATTR4_WORD1_NUMLINKS
> -	| FATTR4_WORD1_OWNER
> +	FATTR4_WORD1_OWNER
>  	| FATTR4_WORD1_OWNER_GROUP
>  	| FATTR4_WORD1_RAWDEV
>  	| FATTR4_WORD1_SPACE_USED
> @@ -6763,9 +6760,7 @@ static int _nfs4_proc_fs_locations(struct rpc_clnt
> *client, struct inode *dir,
>  				   struct page *page)
>  {
>  	struct nfs_server *server =3D NFS_SERVER(dir);
> -	u32 bitmask[3] =3D {
> -		[0] =3D FATTR4_WORD0_FSID |
> FATTR4_WORD0_FS_LOCATIONS,
> -	};
> +	u32 bitmask[3];
>  	struct nfs4_fs_locations_arg args =3D {
>  		.dir_fh =3D NFS_FH(dir),
>  		.name =3D name,
> @@ -6784,12 +6779,15 @@ static int _nfs4_proc_fs_locations(struct rpc_cln=
t
> *client, struct inode *dir,
> 
>  	dprintk("%s: start\n", __func__);
> 
> +	bitmask[0] =3D nfs4_fattr_bitmap[0] |
> FATTR4_WORD0_FS_LOCATIONS;
> +	bitmask[1] =3D nfs4_fattr_bitmap[1];
> +
>  	/* Ask for the fileid of the absent filesystem if mounted_on_fileid
>  	 * is not supported */
>  	if (NFS_SERVER(dir)->attr_bitmask[1] &
> FATTR4_WORD1_MOUNTED_ON_FILEID)
> -		bitmask[1] |=3D FATTR4_WORD1_MOUNTED_ON_FILEID;
> +		bitmask[0] &=3D ~FATTR4_WORD0_FILEID;
>  	else
> -		bitmask[0] |=3D FATTR4_WORD0_FILEID;
> +		bitmask[1] &=3D ~FATTR4_WORD1_MOUNTED_ON_FILEID;
> 
>  	nfs_fattr_init(&fs_locations->fattr);
>  	fs_locations->server =3D server;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in t=
he
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html


---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus


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

* Re: [PATCH v2] nfs: Fix ugly referral attributes
  2017-11-06 17:34 ` Frank Filz
@ 2017-11-06 20:39   ` Pradeep
  0 siblings, 0 replies; 7+ messages in thread
From: Pradeep @ 2017-11-06 20:39 UTC (permalink / raw)
  To: Frank Filz
  Cc: Chuck Lever, trond.myklebust, anna.schumaker,
	Linux NFS Mailing List, nfs-ganesha-devel

Hi Frank,

I should be able to do that. Thanks Chuck for the fix.

On Mon, Nov 6, 2017 at 9:34 AM, Frank Filz <ffilzlnx@mindspring.com> wrote:
> Pradeep,
>
> Could you verify this patch with nfs-ganesha? Looking at the code, it looks like we will supply the attributes requested.
>
> Thanks
>
> Frank
>
>> -----Original Message-----
>> From: linux-nfs-owner@vger.kernel.org [mailto:linux-nfs-
>> owner@vger.kernel.org] On Behalf Of Chuck Lever
>> Sent: Sunday, November 5, 2017 12:45 PM
>> To: trond.myklebust@primarydata.com; anna.schumaker@netapp.com
>> Cc: linux-nfs@vger.kernel.org
>> Subject: [PATCH v2] nfs: Fix ugly referral attributes
>>
>> Before traversing a referral and performing a mount, the mounted-on
>> directory looks strange:
>>
>> dr-xr-xr-x. 2 4294967294 4294967294 0 Dec 31  1969 dir.0
>>
>> nfs4_get_referral is wiping out any cached attributes with what was returned
>> via GETATTR(fs_locations), but the bit mask for that operation does not
>> request any file attributes.
>>
>> Retrieve owner and timestamp information so that the memcpy in
>> nfs4_get_referral fills in more attributes.
>>
>> Changes since v1:
>> - Don't request attributes that the client unconditionally replaces
>> - Request only MOUNTED_ON_FILEID or FILEID attribute, not both
>> - encode_fs_locations() doesn't use the third bitmask word
>>
>> Fixes: 6b97fd3da1ea ("NFSv4: Follow a referral")
>> Suggested-by: Pradeep Thomas <pradeepthomas@gmail.com>
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> Cc: stable@vger.kernel.org
>> ---
>>  fs/nfs/nfs4proc.c |   18 ++++++++----------
>>  1 file changed, 8 insertions(+), 10 deletions(-)
>>
>> I could send this as an incremental, but that just seems to piss off
>> distributors, who will just squash them all together anyway.
>>
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 6c61e2b..2662879
>> 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -254,15 +254,12 @@ static int nfs4_map_errors(int err)  };
>>
>>  const u32 nfs4_fs_locations_bitmap[3] = {
>> -     FATTR4_WORD0_TYPE
>> -     | FATTR4_WORD0_CHANGE
>> +     FATTR4_WORD0_CHANGE
>>       | FATTR4_WORD0_SIZE
>>       | FATTR4_WORD0_FSID
>>       | FATTR4_WORD0_FILEID
>>       | FATTR4_WORD0_FS_LOCATIONS,
>> -     FATTR4_WORD1_MODE
>> -     | FATTR4_WORD1_NUMLINKS
>> -     | FATTR4_WORD1_OWNER
>> +     FATTR4_WORD1_OWNER
>>       | FATTR4_WORD1_OWNER_GROUP
>>       | FATTR4_WORD1_RAWDEV
>>       | FATTR4_WORD1_SPACE_USED
>> @@ -6763,9 +6760,7 @@ static int _nfs4_proc_fs_locations(struct rpc_clnt
>> *client, struct inode *dir,
>>                                  struct page *page)
>>  {
>>       struct nfs_server *server = NFS_SERVER(dir);
>> -     u32 bitmask[3] = {
>> -             [0] = FATTR4_WORD0_FSID |
>> FATTR4_WORD0_FS_LOCATIONS,
>> -     };
>> +     u32 bitmask[3];
>>       struct nfs4_fs_locations_arg args = {
>>               .dir_fh = NFS_FH(dir),
>>               .name = name,
>> @@ -6784,12 +6779,15 @@ static int _nfs4_proc_fs_locations(struct rpc_clnt
>> *client, struct inode *dir,
>>
>>       dprintk("%s: start\n", __func__);
>>
>> +     bitmask[0] = nfs4_fattr_bitmap[0] |
>> FATTR4_WORD0_FS_LOCATIONS;
>> +     bitmask[1] = nfs4_fattr_bitmap[1];
>> +
>>       /* Ask for the fileid of the absent filesystem if mounted_on_fileid
>>        * is not supported */
>>       if (NFS_SERVER(dir)->attr_bitmask[1] &
>> FATTR4_WORD1_MOUNTED_ON_FILEID)
>> -             bitmask[1] |= FATTR4_WORD1_MOUNTED_ON_FILEID;
>> +             bitmask[0] &= ~FATTR4_WORD0_FILEID;
>>       else
>> -             bitmask[0] |= FATTR4_WORD0_FILEID;
>> +             bitmask[1] &= ~FATTR4_WORD1_MOUNTED_ON_FILEID;
>>
>>       nfs_fattr_init(&fs_locations->fattr);
>>       fs_locations->server = server;
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the
>> body of a message to majordomo@vger.kernel.org More majordomo info at
>> http://vger.kernel.org/majordomo-info.html
>
>
> ---
> This email has been checked for viruses by Avast antivirus software.
> https://www.avast.com/antivirus
>

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

* Re: Regression in [v2] nfs: Fix ugly referral attributes ?
  2017-11-05 20:45 [PATCH v2] nfs: Fix ugly referral attributes Chuck Lever
  2017-11-06 17:34 ` Frank Filz
@ 2018-05-17  7:53 ` Moritz Schlarb
  2018-05-17 14:15   ` Chuck Lever
  1 sibling, 1 reply; 7+ messages in thread
From: Moritz Schlarb @ 2018-05-17  7:53 UTC (permalink / raw)
  To: linux-nfs
  Cc: Chuck Lever, trond.myklebust, anna.schumaker, 898165-forwarded,
	898165, ben, Pradeep


[-- Attachment #1.1.1: Type: text/plain, Size: 4248 bytes --]

Hi everyone,

there might be a regression coming from this patch:
Since it got included in 3.16.54, our clients running a recent 3.16
kernel (like from Debian jessie-security) did not follow NFS 4.1
referrals (issued by nfs-ganesha) anymore.
I have built that exact Debian kernel package with just this patch
reversed and it worked again, so I got pretty confident that this patch
is at least strongly related to the problem.
Pradeep also confirmed the problem happening in 3.16.54 but not in 3.16.51.
Interestingly, this does *not* happen with 4.9 kernels, although the
patch was part of 4.9.80...

I have attached a pcap file of a machine running 3.16.56-1+deb8u1 in
which I try to login as a user where my home directory is
/uni-mainz.de/homes/schlarbm (with nfsrefer.zdv.uni-mainz.de:/ on
/uni-mainz.de) which is then referred to
fs02.uni-mainz.de:/vol/ma17/homes/schlarbm but that referral is not
followed by the client.

Please let me know if you need additional information to reproduce or
have suggestions on what we could try.

Best regards,
Moritz

On 05.11.2017 21:45, Chuck Lever wrote:
> Before traversing a referral and performing a mount, the mounted-on
> directory looks strange:
> 
> dr-xr-xr-x. 2 4294967294 4294967294 0 Dec 31  1969 dir.0
> 
> nfs4_get_referral is wiping out any cached attributes with what was
> returned via GETATTR(fs_locations), but the bit mask for that
> operation does not request any file attributes.
> 
> Retrieve owner and timestamp information so that the memcpy in
> nfs4_get_referral fills in more attributes.
> 
> Changes since v1:
> - Don't request attributes that the client unconditionally replaces
> - Request only MOUNTED_ON_FILEID or FILEID attribute, not both
> - encode_fs_locations() doesn't use the third bitmask word
> 
> Fixes: 6b97fd3da1ea ("NFSv4: Follow a referral")
> Suggested-by: Pradeep Thomas <pradeepthomas@gmail.com>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> Cc: stable@vger.kernel.org
> ---
>  fs/nfs/nfs4proc.c |   18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> I could send this as an incremental, but that just seems to piss
> off distributors, who will just squash them all together anyway.
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 6c61e2b..2662879 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -254,15 +254,12 @@ static int nfs4_map_errors(int err)
>  };
>  
>  const u32 nfs4_fs_locations_bitmap[3] = {
> -	FATTR4_WORD0_TYPE
> -	| FATTR4_WORD0_CHANGE
> +	FATTR4_WORD0_CHANGE
>  	| FATTR4_WORD0_SIZE
>  	| FATTR4_WORD0_FSID
>  	| FATTR4_WORD0_FILEID
>  	| FATTR4_WORD0_FS_LOCATIONS,
> -	FATTR4_WORD1_MODE
> -	| FATTR4_WORD1_NUMLINKS
> -	| FATTR4_WORD1_OWNER
> +	FATTR4_WORD1_OWNER
>  	| FATTR4_WORD1_OWNER_GROUP
>  	| FATTR4_WORD1_RAWDEV
>  	| FATTR4_WORD1_SPACE_USED
> @@ -6763,9 +6760,7 @@ static int _nfs4_proc_fs_locations(struct rpc_clnt *client, struct inode *dir,
>  				   struct page *page)
>  {
>  	struct nfs_server *server = NFS_SERVER(dir);
> -	u32 bitmask[3] = {
> -		[0] = FATTR4_WORD0_FSID | FATTR4_WORD0_FS_LOCATIONS,
> -	};
> +	u32 bitmask[3];
>  	struct nfs4_fs_locations_arg args = {
>  		.dir_fh = NFS_FH(dir),
>  		.name = name,
> @@ -6784,12 +6779,15 @@ static int _nfs4_proc_fs_locations(struct rpc_clnt *client, struct inode *dir,
>  
>  	dprintk("%s: start\n", __func__);
>  
> +	bitmask[0] = nfs4_fattr_bitmap[0] | FATTR4_WORD0_FS_LOCATIONS;
> +	bitmask[1] = nfs4_fattr_bitmap[1];
> +
>  	/* Ask for the fileid of the absent filesystem if mounted_on_fileid
>  	 * is not supported */
>  	if (NFS_SERVER(dir)->attr_bitmask[1] & FATTR4_WORD1_MOUNTED_ON_FILEID)
> -		bitmask[1] |= FATTR4_WORD1_MOUNTED_ON_FILEID;
> +		bitmask[0] &= ~FATTR4_WORD0_FILEID;
>  	else
> -		bitmask[0] |= FATTR4_WORD0_FILEID;
> +		bitmask[1] &= ~FATTR4_WORD1_MOUNTED_ON_FILEID;
>  
>  	nfs_fattr_init(&fs_locations->fattr);
>  	fs_locations->server = server;
> 

[-- Attachment #1.1.2: nfs-referral-broken.pcap --]
[-- Type: application/vnd.tcpdump.pcap, Size: 16892 bytes --]

[-- Attachment #1.1.3: schlarbm.vcf --]
[-- Type: text/x-vcard, Size: 379 bytes --]

begin:vcard
fn:Moritz Schlarb
n:Schlarb;Moritz
org;quoted-printable;quoted-printable:Johannes Gutenberg-Universit=C3=A4t Mainz;Zentrum f=C3=BCr Datenverarbeitung
adr;dom:;;;Mainz
email;internet:schlarbm@uni-mainz.de
tel;work:+49 6131 39 29441
note;quoted-printable:OpenPGP Fingerprint: DF01 2247 BFC6=0D=0A=
	5501 AFF2 8445 0C24 B841 C7DD BAAF
version:2.1
end:vcard


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: Regression in [v2] nfs: Fix ugly referral attributes ?
  2018-05-17  7:53 ` Regression in [v2] nfs: Fix ugly referral attributes ? Moritz Schlarb
@ 2018-05-17 14:15   ` Chuck Lever
  2018-05-17 21:48     ` Moritz Schlarb
  0 siblings, 1 reply; 7+ messages in thread
From: Chuck Lever @ 2018-05-17 14:15 UTC (permalink / raw)
  To: Moritz Schlarb
  Cc: Linux NFS Mailing List, Trond Myklebust, Anna Schumaker,
	898165-forwarded, 898165, ben, Pradeep



> On May 17, 2018, at 3:53 AM, Moritz Schlarb <schlarbm@uni-mainz.de> =
wrote:
>=20
> Hi everyone,
>=20
> there might be a regression coming from this patch:
> Since it got included in 3.16.54, our clients running a recent 3.16
> kernel (like from Debian jessie-security) did not follow NFS 4.1
> referrals (issued by nfs-ganesha) anymore.
> I have built that exact Debian kernel package with just this patch
> reversed and it worked again, so I got pretty confident that this =
patch
> is at least strongly related to the problem.
> Pradeep also confirmed the problem happening in 3.16.54 but not in =
3.16.51.
> Interestingly, this does *not* happen with 4.9 kernels, although the
> patch was part of 4.9.80...
>=20
> I have attached a pcap file of a machine running 3.16.56-1+deb8u1 in
> which I try to login as a user where my home directory is
> /uni-mainz.de/homes/schlarbm (with nfsrefer.zdv.uni-mainz.de:/ on
> /uni-mainz.de) which is then referred to
> fs02.uni-mainz.de:/vol/ma17/homes/schlarbm but that referral is not
> followed by the client.
>=20
> Please let me know if you need additional information to reproduce or
> have suggestions on what we could try.

Just a shot in the dark: Wondering if v3.16 needs

commit ea96d1ecbe4fcb1df487d99309d3157b4ff5fc02
Author:     Anna Schumaker <Anna.Schumaker@netapp.com>
AuthorDate: Fri Apr 3 14:35:59 2015 -0400
Commit:     Trond Myklebust <trond.myklebust@primarydata.com>
CommitDate: Thu Apr 23 14:43:54 2015 -0400

    nfs: Fetch MOUNTED_ON_FILEID when updating an inode


> Best regards,
> Moritz
>=20
> On 05.11.2017 21:45, Chuck Lever wrote:
>> Before traversing a referral and performing a mount, the mounted-on
>> directory looks strange:
>>=20
>> dr-xr-xr-x. 2 4294967294 4294967294 0 Dec 31  1969 dir.0
>>=20
>> nfs4_get_referral is wiping out any cached attributes with what was
>> returned via GETATTR(fs_locations), but the bit mask for that
>> operation does not request any file attributes.
>>=20
>> Retrieve owner and timestamp information so that the memcpy in
>> nfs4_get_referral fills in more attributes.
>>=20
>> Changes since v1:
>> - Don't request attributes that the client unconditionally replaces
>> - Request only MOUNTED_ON_FILEID or FILEID attribute, not both
>> - encode_fs_locations() doesn't use the third bitmask word
>>=20
>> Fixes: 6b97fd3da1ea ("NFSv4: Follow a referral")
>> Suggested-by: Pradeep Thomas <pradeepthomas@gmail.com>
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> Cc: stable@vger.kernel.org
>> ---
>> fs/nfs/nfs4proc.c |   18 ++++++++----------
>> 1 file changed, 8 insertions(+), 10 deletions(-)
>>=20
>> I could send this as an incremental, but that just seems to piss
>> off distributors, who will just squash them all together anyway.
>>=20
>>=20
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" =
in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>=20
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 6c61e2b..2662879 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -254,15 +254,12 @@ static int nfs4_map_errors(int err)
>> };
>>=20
>> const u32 nfs4_fs_locations_bitmap[3] =3D {
>> -	FATTR4_WORD0_TYPE
>> -	| FATTR4_WORD0_CHANGE
>> +	FATTR4_WORD0_CHANGE
>> 	| FATTR4_WORD0_SIZE
>> 	| FATTR4_WORD0_FSID
>> 	| FATTR4_WORD0_FILEID
>> 	| FATTR4_WORD0_FS_LOCATIONS,
>> -	FATTR4_WORD1_MODE
>> -	| FATTR4_WORD1_NUMLINKS
>> -	| FATTR4_WORD1_OWNER
>> +	FATTR4_WORD1_OWNER
>> 	| FATTR4_WORD1_OWNER_GROUP
>> 	| FATTR4_WORD1_RAWDEV
>> 	| FATTR4_WORD1_SPACE_USED
>> @@ -6763,9 +6760,7 @@ static int _nfs4_proc_fs_locations(struct =
rpc_clnt *client, struct inode *dir,
>> 				   struct page *page)
>> {
>> 	struct nfs_server *server =3D NFS_SERVER(dir);
>> -	u32 bitmask[3] =3D {
>> -		[0] =3D FATTR4_WORD0_FSID | FATTR4_WORD0_FS_LOCATIONS,
>> -	};
>> +	u32 bitmask[3];
>> 	struct nfs4_fs_locations_arg args =3D {
>> 		.dir_fh =3D NFS_FH(dir),
>> 		.name =3D name,
>> @@ -6784,12 +6779,15 @@ static int _nfs4_proc_fs_locations(struct =
rpc_clnt *client, struct inode *dir,
>>=20
>> 	dprintk("%s: start\n", __func__);
>>=20
>> +	bitmask[0] =3D nfs4_fattr_bitmap[0] | FATTR4_WORD0_FS_LOCATIONS;
>> +	bitmask[1] =3D nfs4_fattr_bitmap[1];
>> +
>> 	/* Ask for the fileid of the absent filesystem if =
mounted_on_fileid
>> 	 * is not supported */
>> 	if (NFS_SERVER(dir)->attr_bitmask[1] & =
FATTR4_WORD1_MOUNTED_ON_FILEID)
>> -		bitmask[1] |=3D FATTR4_WORD1_MOUNTED_ON_FILEID;
>> +		bitmask[0] &=3D ~FATTR4_WORD0_FILEID;
>> 	else
>> -		bitmask[0] |=3D FATTR4_WORD0_FILEID;
>> +		bitmask[1] &=3D ~FATTR4_WORD1_MOUNTED_ON_FILEID;
>>=20
>> 	nfs_fattr_init(&fs_locations->fattr);
>> 	fs_locations->server =3D server;
>>=20
> <nfs-referral-broken.pcap><schlarbm.vcf>

--
Chuck Lever




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

* Re: Regression in [v2] nfs: Fix ugly referral attributes ?
  2018-05-17 14:15   ` Chuck Lever
@ 2018-05-17 21:48     ` Moritz Schlarb
  2018-05-18 14:35       ` Chuck Lever
  0 siblings, 1 reply; 7+ messages in thread
From: Moritz Schlarb @ 2018-05-17 21:48 UTC (permalink / raw)
  To: Chuck Lever
  Cc: Linux NFS Mailing List, Trond Myklebust, Anna Schumaker,
	898165-forwarded, 898165, ben, Pradeep, Christoph Martin

Hi Chuck,

On 17.05.2018 16:15, Chuck Lever wrote:

> Just a shot in the dark: Wondering if v3.16 needs
> 
> commit ea96d1ecbe4fcb1df487d99309d3157b4ff5fc02
> Author:     Anna Schumaker <Anna.Schumaker@netapp.com>
> AuthorDate: Fri Apr 3 14:35:59 2015 -0400
> Commit:     Trond Myklebust <trond.myklebust@primarydata.com>
> CommitDate: Thu Apr 23 14:43:54 2015 -0400
> 
>     nfs: Fetch MOUNTED_ON_FILEID when updating an inode

Gosh, it seems you're right!
When I take that patch and apply it, the referrals are being followed again!

Thanks for your idea!
Now how do we make sure it gets applied soonish?

Regards,
Moritz

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

* Re: Regression in [v2] nfs: Fix ugly referral attributes ?
  2018-05-17 21:48     ` Moritz Schlarb
@ 2018-05-18 14:35       ` Chuck Lever
  0 siblings, 0 replies; 7+ messages in thread
From: Chuck Lever @ 2018-05-18 14:35 UTC (permalink / raw)
  To: Moritz Schlarb
  Cc: Linux NFS Mailing List, Trond Myklebust, Anna Schumaker,
	898165-forwarded, 898165, ben, Pradeep, Christoph Martin



> On May 17, 2018, at 5:48 PM, Moritz Schlarb <schlarbm@uni-mainz.de> =
wrote:
>=20
> Hi Chuck,
>=20
> On 17.05.2018 16:15, Chuck Lever wrote:
>=20
>> Just a shot in the dark: Wondering if v3.16 needs
>>=20
>> commit ea96d1ecbe4fcb1df487d99309d3157b4ff5fc02
>> Author:     Anna Schumaker <Anna.Schumaker@netapp.com>
>> AuthorDate: Fri Apr 3 14:35:59 2015 -0400
>> Commit:     Trond Myklebust <trond.myklebust@primarydata.com>
>> CommitDate: Thu Apr 23 14:43:54 2015 -0400
>>=20
>>    nfs: Fetch MOUNTED_ON_FILEID when updating an inode
>=20
> Gosh, it seems you're right!
> When I take that patch and apply it, the referrals are being followed =
again!
>=20
> Thanks for your idea!
> Now how do we make sure it gets applied soonish?

Hi Moritz-

Anyone (including you or your distributor) can request that this
patch be applied to the upstream v3.16 LTS branch. Since you have
confirmed that it applies and fixes your problem, it would be
appropriate to report that along with your backport request.


--
Chuck Lever




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

end of thread, other threads:[~2018-05-18 14:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-05 20:45 [PATCH v2] nfs: Fix ugly referral attributes Chuck Lever
2017-11-06 17:34 ` Frank Filz
2017-11-06 20:39   ` Pradeep
2018-05-17  7:53 ` Regression in [v2] nfs: Fix ugly referral attributes ? Moritz Schlarb
2018-05-17 14:15   ` Chuck Lever
2018-05-17 21:48     ` Moritz Schlarb
2018-05-18 14:35       ` Chuck Lever

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.