Bug#898165: Regression in [v2] nfs: Fix ugly referral attributes ?
> On May 17, 2018, at 5:48 PM, Moritz Schlarbwrote: > > 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 >> AuthorDate: Fri Apr 3 14:35:59 2015 -0400 >> Commit: Trond Myklebust >> 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? 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
Bug#898165: Regression in [v2] nfs: Fix ugly referral attributes ?
Control: tags -1 + upstream patch Control: severity -1 grave Control: summary -1 0 Control: outlook -1 0 3.16.54 introduced a regression by including "nfs: Fix ugly referral attributes" but not "nfs: Fetch MOUNTED_ON_FILEID when updating an inode". Please include that other patch, too so NFS referrals work again. The required patch is https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ea96d1ecbe4fcb1df487d99309d3157b4ff5fc02 Best regards, Moritz 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> AuthorDate: Fri Apr 3 14:35:59 2015 -0400 > Commit: Trond Myklebust > CommitDate: Thu Apr 23 14:43:54 2015 -0400 > > nfs: Fetch MOUNTED_ON_FILEID when updating an inode
Bug#898165: Regression in [v2] nfs: Fix ugly referral attributes ?
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> AuthorDate: Fri Apr 3 14:35:59 2015 -0400 > Commit: Trond Myklebust > 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
Bug#898165: Regression in [v2] nfs: Fix ugly referral attributes ?
> On May 17, 2018, at 3:53 AM, Moritz Schlarbwrote: > > 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 AuthorDate: Fri Apr 3 14:35:59 2015 -0400 Commit: Trond Myklebust 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 >> Signed-off-by: Chuck Lever >> Cc: sta...@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 majord...@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(_locations->fattr); >> fs_locations->server = server; >> > -- Chuck Lever
Bug#898165: Regression in [v2] nfs: Fix ugly referral attributes ?
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> Signed-off-by: Chuck Lever > Cc: sta...@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 majord...@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(_locations->fattr); > fs_locations->server = server; > nfs-referral-broken.pcap Description: application/vnd.tcpdump.pcap <> signature.asc Description: OpenPGP digital signature