Bug#898165: Regression in [v2] nfs: Fix ugly referral attributes ?
> On May 17, 2018, at 3:53 AM, Moritz Schlarb <schlarbm@uni-mainz.de> wrote:
>
> 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.
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
>
> 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;
>>
> <nfs-referral-broken.pcap><schlarbm.vcf>
--
Chuck Lever
Reply to: