| Message ID | 458d3314-2010-4271-bb73-bff47e9de358@samba.org |
|---|---|
| State | New |
| Headers | show |
| Series | Special files broken against Samba master | expand |
Ralph Boehme <slow@samba.org> writes: > I'm also optimizing away unneeded reparse point queries > (create+fsctl+close) for nfs reparse points where we only need the file > type which we now get and use via the smb3 posix type. Awesome! Yeah, we don't need to revalidate the special files with SMB3.1.1 POSIX as we can already parse file type directly from query dir response. We could that in separate patches, so no worries. This will definitely help with getdents(2). > I'm sure I'm doing things wrong. Can you help me getting this across the > line? Patches look good, thanks. It would be great making sure it didn't regress against NFS server on Windows, but Steve can do that. Thanks Ralph!
On 12/2/24 11:40 PM, Paulo Alcantara wrote: > Ralph Boehme <slow@samba.org> writes: >> I'm sure I'm doing things wrong. Can you help me getting this across the >> line? > > Patches look good, thanks. It would be great making sure it didn't > regress against NFS server on Windows, but Steve can do that. great, thanks for taking a look! I was expecting I was likely doing some stupid beginner mistakes, but if you think the changes look good, here's a v2 with just my +1 added, without code changes. Thanks! -slow
Looks like a big improvement - fewer roundtrips to Samba. Haven't tried to Windows yet, but it does look like it breaks mounts to ksmbd so I may need a minor change to cifs.ko or ksmbd. On Tue, Dec 3, 2024 at 4:04 AM Ralph Boehme <slow@samba.org> wrote: > > On 12/2/24 11:40 PM, Paulo Alcantara wrote: > > Ralph Boehme <slow@samba.org> writes: > >> I'm sure I'm doing things wrong. Can you help me getting this across the > >> line? > > > > Patches look good, thanks. It would be great making sure it didn't > > regress against NFS server on Windows, but Steve can do that. > > great, thanks for taking a look! I was expecting I was likely doing some > stupid beginner mistakes, but if you think the changes look good, here's > a v2 with just my +1 added, without code changes. > > Thanks! > -slow
Hi Steve! Thanks for testing! ksmbd needs to be updated to implement the current spec. The next major Samba version 4.22 will ship this stuff, so I'd be good if cifs.ko follows along asap. Thanks! -slow On 12/3/24 8:56 PM, Steve French wrote: > Looks like a big improvement - fewer roundtrips to Samba. Haven't > tried to Windows yet, but it does look like it breaks mounts to ksmbd > so I may need a minor change to cifs.ko or ksmbd. > > On Tue, Dec 3, 2024 at 4:04 AM Ralph Boehme <slow@samba.org> wrote: >> >> On 12/2/24 11:40 PM, Paulo Alcantara wrote: >>> Ralph Boehme <slow@samba.org> writes: >>>> I'm sure I'm doing things wrong. Can you help me getting this across the >>>> line? >>> >>> Patches look good, thanks. It would be great making sure it didn't >>> regress against NFS server on Windows, but Steve can do that. >> >> great, thanks for taking a look! I was expecting I was likely doing some >> stupid beginner mistakes, but if you think the changes look good, here's >> a v2 with just my +1 added, without code changes. >> >> Thanks! >> -slow > > >
I updated patch 2 (0002-fs-smb-client-Implement-new-SMB3-POSIX-type.patch) to address various checkpatch warnings, and added one small changeset to fix mounts to older servers which don't fill in the file type in the level 100 Mode field (0004-smb3.1.1-fix-posix-mounts-to-older-servers.patch), and updated them in cifs-2.6.git for-next. Let me know if any objections. It looks like it does make a big improvement in reporting special files when mounted to Samba, but want to figure out why the dentry cache (revalidate call) still requires the extra roundtrip per file when doing "ls -l" (there may be a way that this enhanced query dir output could be recognized as still valid, and reduce need for extra queries). See attached. On Tue, Dec 3, 2024 at 4:04 AM Ralph Boehme <slow@samba.org> wrote: > > On 12/2/24 11:40 PM, Paulo Alcantara wrote: > > Ralph Boehme <slow@samba.org> writes: > >> I'm sure I'm doing things wrong. Can you help me getting this across the > >> line? > > > > Patches look good, thanks. It would be great making sure it didn't > > regress against NFS server on Windows, but Steve can do that. > > great, thanks for taking a look! I was expecting I was likely doing some > stupid beginner mistakes, but if you think the changes look good, here's > a v2 with just my +1 added, without code changes. > > Thanks! > -slow
Hi Steve thanks for digging into this! On 12/5/24 1:06 AM, Steve French wrote: > I updated patch 2 > (0002-fs-smb-client-Implement-new-SMB3-POSIX-type.patch) to address > various checkpatch warnings, oh, sorry for those! I'll try to remember to run it myself next time. > and added one small changeset to fix > mounts to older servers which don't fill in the file type in the level > 100 Mode field (0004-smb3.1.1-fix-posix-mounts-to-older-servers.patch), > and updated them in cifs-2.6.git for-next. Let me know if any > objections. hm, not sure I like adding code to support non compliant SMB3 POSIX server implementations at this early stage of implementing them across various projects, clients and servers. Can't Namjae just fix it in ksmbd? > It looks like it does make a big improvement in reporting > special files when mounted to Samba, but want to figure out why the > dentry cache (revalidate call) still requires the extra roundtrip per > file when doing "ls -l" (there may be a way that this enhanced query > dir output could be recognized as still valid, and reduce need for > extra queries). it should only do an extra roundtrip when needed for symlinks, character and block devices as for those we wee need additional info. But for fifos and sockets the SMB2-FIND POSIX-infolevel is sufficient and in my testing I didn't see extra roundtrips for those. Thanks! -slow
On Fri, Dec 6, 2024 at 6:11 AM Ralph Boehme <slow@samba.org> wrote: > > Hi Steve > > thanks for digging into this! > > On 12/5/24 1:06 AM, Steve French wrote: > > I updated patch 2 > > (0002-fs-smb-client-Implement-new-SMB3-POSIX-type.patch) to address > > various checkpatch warnings, > > oh, sorry for those! I'll try to remember to run it myself next time. > > > and added one small changeset to fix > > mounts to older servers which don't fill in the file type in the level > > 100 Mode field (0004-smb3.1.1-fix-posix-mounts-to-older-servers.patch), > > and updated them in cifs-2.6.git for-next. Let me know if any > > objections. > > hm, not sure I like adding code to support non compliant SMB3 POSIX > server implementations at this early stage of implementing them across > various projects, clients and servers. Can't Namjae just fix it in ksmbd? We will make sure ksmbd is changed to fill in the file type, but we should reduce risks (where fix is easy) of breaking mounts to existing servers, especially since this has been in for more than three years and since the sanity check is safe and easy (ATTRIBUTE_DIRECTORY will always be set correctly by a server). There is also at least one other server that implemented earlier version of the SMB3.1.1 POSIX Extensions so better to be safe when we can do a minor check like this on the client so we don't risk any regressions, and it also will reduce risk of a future server bug breaking mounts.
Hello, this patch is incomplete and still does not fix the main problem that SMB2_OP_QUERY_WSL_EA command does not work with any Windows SMB server except the last Windows Server version. On non-recent Windows versions it is not possible to set both EAs and reparse point at the same time and Windows SMB server is returning error when trying to query EAs on file with reparse point. Which basically means that it is not possible to query data about the special files from Windows SMB server (except 2022 version). More details are in the email which I wrote in September: https://lore.kernel.org/linux-cifs/20240928140939.vjndryndfngzq7x4@pali/ I proposed similar but extended patch which skips asking for EAs based on reparse point: https://lore.kernel.org/linux-cifs/20240913200204.10660-1-pali@kernel.org/ But it was somehow rejected as the proper solution should be different: https://lore.kernel.org/linux-cifs/20240917210707.4lt4obty7wlmm42j@pali/ And that is why I'm surprised that the batch below was accepted... Are you planning to implement a proper solution? If not that I would propose to reconsider my original idea, which will workaround also Windows SMB server, and not only Samba server. As I wrote in previous emails, I think that this is a serious issue as it disallow to use any reparse points against non-recent Windows Server systems. On Wednesday 27 November 2024 20:23:29 Ralph Boehme wrote: > From a57c32da874f285af266b7fbbaefb7940991d049 Mon Sep 17 00:00:00 2001 > From: Ralph Boehme <slow@samba.org> > Date: Fri, 15 Nov 2024 13:15:50 +0100 > Subject: [PATCH 1/3] fs/smb/client: avoid querying SMB2_OP_QUERY_WSL_EA for > SMB3 POSIX > > --- > fs/smb/client/smb2inode.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c > index e49d0c25eb03..ab069d5285c5 100644 > --- a/fs/smb/client/smb2inode.c > +++ b/fs/smb/client/smb2inode.c > @@ -941,7 +941,8 @@ int smb2_query_path_info(const unsigned int xid, > if (rc || !data->reparse_point) > goto out; > > - cmds[num_cmds++] = SMB2_OP_QUERY_WSL_EA; > + if (!tcon->posix_extensions) > + cmds[num_cmds++] = SMB2_OP_QUERY_WSL_EA; > /* > * Skip SMB2_OP_GET_REPARSE if symlink already parsed in create > * response. > -- > 2.47.0
Pali Rohár <pali@kernel.org> writes: > Hello, this patch is incomplete and still does not fix the main problem > that SMB2_OP_QUERY_WSL_EA command does not work with any Windows SMB > server except the last Windows Server version. On non-recent Windows > versions it is not possible to set both EAs and reparse point at the > same time and Windows SMB server is returning error when trying to query > EAs on file with reparse point. No, Ralph's patch has nothing to do with your problem. SMB3.1.1 POSIX will support NFS reparse points with no EAs, so makes sense skipping SMB2_OP_QUERY_WSL_EA altogether for posix case. > Which basically means that it is not possible to query data about the > special files from Windows SMB server (except 2022 version). > > More details are in the email which I wrote in September: > https://lore.kernel.org/linux-cifs/20240928140939.vjndryndfngzq7x4@pali/ > > I proposed similar but extended patch which skips asking for EAs based > on reparse point: > https://lore.kernel.org/linux-cifs/20240913200204.10660-1-pali@kernel.org/ Yes, that patch looks incorrect and untested. Can you tell me how is @data->reparse.tag supposed to be set, for non-readdir case, if the compound request wasn't sent yet? Have you tried to stat(2) those files with your patch? > But it was somehow rejected as the proper solution should be different: > https://lore.kernel.org/linux-cifs/20240917210707.4lt4obty7wlmm42j@pali/ Yes. Have you sent a patch with the proposed solution yet?
On Monday 09 December 2024 16:28:08 Paulo Alcantara wrote: > Pali Rohár <pali@kernel.org> writes: > > > Hello, this patch is incomplete and still does not fix the main problem > > that SMB2_OP_QUERY_WSL_EA command does not work with any Windows SMB > > server except the last Windows Server version. On non-recent Windows > > versions it is not possible to set both EAs and reparse point at the > > same time and Windows SMB server is returning error when trying to query > > EAs on file with reparse point. > > No, Ralph's patch has nothing to do with your problem. SMB3.1.1 POSIX > will support NFS reparse points with no EAs, so makes sense skipping > SMB2_OP_QUERY_WSL_EA altogether for posix case. Ok, so this is just a coincidence that skip was added at the same place. And I though that it is the same thing (which now I see that is not). > > Which basically means that it is not possible to query data about the > > special files from Windows SMB server (except 2022 version). > > > > More details are in the email which I wrote in September: > > https://lore.kernel.org/linux-cifs/20240928140939.vjndryndfngzq7x4@pali/ > > > > I proposed similar but extended patch which skips asking for EAs based > > on reparse point: > > https://lore.kernel.org/linux-cifs/20240913200204.10660-1-pali@kernel.org/ > > Yes, that patch looks incorrect and untested. Can you tell me how is > @data->reparse.tag supposed to be set, for non-readdir case, if the > compound request wasn't sent yet? Have you tried to stat(2) those files > with your patch? I need to rethink about it. Basically I already dropped this patch as I was expecting the proper solution. > > But it was somehow rejected as the proper solution should be different: > > https://lore.kernel.org/linux-cifs/20240917210707.4lt4obty7wlmm42j@pali/ > > Yes. Have you sent a patch with the proposed solution yet? I have not sent, I was not working on it. I was in impression that you are going to implement the proper solution.
Pali Rohár <pali@kernel.org> writes: > On Monday 09 December 2024 16:28:08 Paulo Alcantara wrote: >> >> Yes. Have you sent a patch with the proposed solution yet? > > I have not sent, I was not working on it. I was in impression that you > are going to implement the proper solution. Yes, but priorities have changed, unfortunately.
On Monday 09 December 2024 16:42:54 Paulo Alcantara wrote: > Pali Rohár <pali@kernel.org> writes: > > > On Monday 09 December 2024 16:28:08 Paulo Alcantara wrote: > >> > >> Yes. Have you sent a patch with the proposed solution yet? > > > > I have not sent, I was not working on it. I was in impression that you > > are going to implement the proper solution. > > Yes, but priorities have changed, unfortunately. Ok. Would you have a time to prepare a patch? I have still feeling that the smb2_compound_op() code and EAs code is rather complicated for me. I would like if somebody else could look at it, as I have feeling if I try to do it, it can end up with something more broken...
Pali Rohár <pali@kernel.org> writes: > Ok. Would you have a time to prepare a patch? I have still feeling that > the smb2_compound_op() code and EAs code is rather complicated for me. > I would like if somebody else could look at it, as I have feeling if I > try to do it, it can end up with something more broken... Yeah, it's a bit tricky. I'll let you know when I have patch so I can try it against your old servers.
On Tuesday 10 December 2024 10:19:26 Paulo Alcantara wrote: > Pali Rohár <pali@kernel.org> writes: > > > Ok. Would you have a time to prepare a patch? I have still feeling that > > the smb2_compound_op() code and EAs code is rather complicated for me. > > I would like if somebody else could look at it, as I have feeling if I > > try to do it, it can end up with something more broken... > > Yeah, it's a bit tricky. I'll let you know when I have patch so I can > try it against your old servers. I looked at that code and it looks to be really more complicated to implement partial parsing of the reply, as compound code is bound to many places and code paths. Meanwhile I have tried to implement a new version to retry the call without querying EAs when server returns that EAs on specific paths are unsupported: https://patchwork.kernel.org/project/cifs-client/patch/20241224131859.3457-1-pali@kernel.org/ I think that this approach should now work correctly.
From a57c32da874f285af266b7fbbaefb7940991d049 Mon Sep 17 00:00:00 2001 From: Ralph Boehme <slow@samba.org> Date: Fri, 15 Nov 2024 13:15:50 +0100 Subject: [PATCH 1/3] fs/smb/client: avoid querying SMB2_OP_QUERY_WSL_EA for SMB3 POSIX --- fs/smb/client/smb2inode.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c index e49d0c25eb03..ab069d5285c5 100644 --- a/fs/smb/client/smb2inode.c +++ b/fs/smb/client/smb2inode.c @@ -941,7 +941,8 @@ int smb2_query_path_info(const unsigned int xid, if (rc || !data->reparse_point) goto out; - cmds[num_cmds++] = SMB2_OP_QUERY_WSL_EA; + if (!tcon->posix_extensions) + cmds[num_cmds++] = SMB2_OP_QUERY_WSL_EA; /* * Skip SMB2_OP_GET_REPARSE if symlink already parsed in create * response. -- 2.47.0 From afd2dce84b1ae970b2ec4421948194746f542cf6 Mon Sep 17 00:00:00 2001 From: Ralph Boehme <slow@samba.org> Date: Fri, 15 Nov 2024 19:21:04 +0100 Subject: [PATCH 2/3] fs/smb/client: Implement new SMB3 POSIX type Fixes special files against current Samba. On the Samba server: insgesamt 20 131958 brw-r--r-- 1 root root 0, 0 15. Nov 12:04 blockdev 131965 crw-r--r-- 1 root root 1, 1 15. Nov 12:04 chardev 131966 prw-r--r-- 1 samba samba 0 15. Nov 12:05 fifo 131953 -rw-rwxrw-+ 2 samba samba 4 18. Nov 11:37 file 131953 -rw-rwxrw-+ 2 samba samba 4 18. Nov 11:37 hardlink 131957 lrwxrwxrwx 1 samba samba 4 15. Nov 12:03 symlink -> file 131954 -rwxrwxr-x+ 1 samba samba 0 18. Nov 15:28 symlinkoversmb Before: ls: cannot access '/mnt/smb3unix/posix/blockdev': No data available ls: cannot access '/mnt/smb3unix/posix/chardev': No data available ls: cannot access '/mnt/smb3unix/posix/symlinkoversmb': No data available ls: cannot access '/mnt/smb3unix/posix/fifo': No data available ls: cannot access '/mnt/smb3unix/posix/symlink': No data available total 16 ? -????????? ? ? ? ? ? blockdev ? -????????? ? ? ? ? ? chardev ? -????????? ? ? ? ? ? fifo 131953 -rw-rwxrw- 2 root samba 4 Nov 18 11:37 file 131953 -rw-rwxrw- 2 root samba 4 Nov 18 11:37 hardlink ? -????????? ? ? ? ? ? symlink ? -????????? ? ? ? ? ? symlinkoversmb After: insgesamt 21 131958 brw-r--r-- 1 root root 0, 0 15. Nov 12:04 blockdev 131965 crw-r--r-- 1 root root 1, 1 15. Nov 12:04 chardev 131966 prw-r--r-- 1 root samba 0 15. Nov 12:05 fifo 131953 -rw-rwxrw- 2 root samba 4 18. Nov 11:37 file 131953 -rw-rwxrw- 2 root samba 4 18. Nov 11:37 hardlink 131957 lrwxrwxrwx 1 root samba 4 15. Nov 12:03 symlink -> file 131954 lrwxrwxr-x 1 root samba 23 18. Nov 15:28 symlinkoversmb -> mnt/smb3unix/posix/file --- fs/smb/client/cifsproto.h | 1 + fs/smb/client/inode.c | 89 +++++++++++++++++++++++++++++++++++---- fs/smb/client/readdir.c | 35 +++++++-------- fs/smb/client/reparse.c | 84 ++++++++++++++++++++++-------------- 4 files changed, 149 insertions(+), 60 deletions(-) diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h index d312ea9776ce..20ee7feb9c49 100644 --- a/fs/smb/client/cifsproto.h +++ b/fs/smb/client/cifsproto.h @@ -676,6 +676,7 @@ int __cifs_sfu_make_node(unsigned int xid, struct inode *inode, int cifs_sfu_make_node(unsigned int xid, struct inode *inode, struct dentry *dentry, struct cifs_tcon *tcon, const char *full_path, umode_t mode, dev_t dev); +umode_t wire_mode_to_posix(u32 wire); #ifdef CONFIG_CIFS_DFS_UPCALL static inline int get_dfs_path(const unsigned int xid, struct cifs_ses *ses, diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c index 72ebd72dd02b..0dcf9e3635ff 100644 --- a/fs/smb/client/inode.c +++ b/fs/smb/client/inode.c @@ -724,6 +724,84 @@ static int cifs_sfu_mode(struct cifs_fattr *fattr, const unsigned char *path, #endif } +#define POSIX_TYPE_FILE 0 +#define POSIX_TYPE_DIR 1 +#define POSIX_TYPE_SYMLINK 2 +#define POSIX_TYPE_CHARDEV 3 +#define POSIX_TYPE_BLKDEV 4 +#define POSIX_TYPE_FIFO 5 +#define POSIX_TYPE_SOCKET 6 + +#define POSIX_X_OTH 0000001 +#define POSIX_W_OTH 0000002 +#define POSIX_R_OTH 0000004 +#define POSIX_X_GRP 0000010 +#define POSIX_W_GRP 0000020 +#define POSIX_R_GRP 0000040 +#define POSIX_X_USR 0000100 +#define POSIX_W_USR 0000200 +#define POSIX_R_USR 0000400 +#define POSIX_STICKY 0001000 +#define POSIX_SET_GID 0002000 +#define POSIX_SET_UID 0004000 + +#define POSIX_OTH_MASK 0000007 +#define POSIX_GRP_MASK 0000070 +#define POSIX_USR_MASK 0000700 +#define POSIX_PERM_MASK 0000777 +#define POSIX_FILETYPE_MASK 0070000 + +#define POSIX_FILETYPE_SHIFT 12 + +static u32 wire_perms_to_posix(u32 wire) +{ + u32 mode = 0; + + mode |= (wire & POSIX_X_OTH) ? S_IXOTH : 0; + mode |= (wire & POSIX_W_OTH) ? S_IWOTH : 0; + mode |= (wire & POSIX_R_OTH) ? S_IROTH : 0; + mode |= (wire & POSIX_X_GRP) ? S_IXGRP : 0; + mode |= (wire & POSIX_W_GRP) ? S_IWGRP : 0; + mode |= (wire & POSIX_R_GRP) ? S_IRGRP : 0; + mode |= (wire & POSIX_X_USR) ? S_IXUSR : 0; + mode |= (wire & POSIX_W_USR) ? S_IWUSR : 0; + mode |= (wire & POSIX_R_USR) ? S_IRUSR : 0; + mode |= (wire & POSIX_STICKY) ? S_ISVTX : 0; + mode |= (wire & POSIX_SET_GID) ? S_ISGID : 0; + mode |= (wire & POSIX_SET_UID) ? S_ISUID : 0; + + return mode; +} + +static u32 posix_filetypes[] = { + S_IFREG, + S_IFDIR, + S_IFLNK, + S_IFCHR, + S_IFBLK, + S_IFIFO, + S_IFSOCK +}; + +static u32 wire_filetype_to_posix(u32 wire_type) +{ + if (wire_type >= ARRAY_SIZE(posix_filetypes)) { + pr_warn("Unexpected type %u", wire_type); + return 0; + } + return posix_filetypes[wire_type]; +} + +umode_t wire_mode_to_posix(u32 wire) +{ + u32 wire_type; + u32 mode; + + wire_type = (wire & POSIX_FILETYPE_MASK) >> POSIX_FILETYPE_SHIFT; + mode = (wire_perms_to_posix(wire) | wire_filetype_to_posix(wire_type)); + return (umode_t)mode; +} + /* Fill a cifs_fattr struct with info from POSIX info struct */ static void smb311_posix_info_to_fattr(struct cifs_fattr *fattr, struct cifs_open_info_data *data, @@ -760,20 +838,13 @@ static void smb311_posix_info_to_fattr(struct cifs_fattr *fattr, fattr->cf_bytes = le64_to_cpu(info->AllocationSize); fattr->cf_createtime = le64_to_cpu(info->CreationTime); fattr->cf_nlink = le32_to_cpu(info->HardLinks); - fattr->cf_mode = (umode_t) le32_to_cpu(info->Mode); + fattr->cf_mode = wire_mode_to_posix(le32_to_cpu(info->Mode)); if (cifs_open_data_reparse(data) && cifs_reparse_point_to_fattr(cifs_sb, fattr, data)) goto out_reparse; - fattr->cf_mode &= ~S_IFMT; - if (fattr->cf_cifsattrs & ATTR_DIRECTORY) { - fattr->cf_mode |= S_IFDIR; - fattr->cf_dtype = DT_DIR; - } else { /* file */ - fattr->cf_mode |= S_IFREG; - fattr->cf_dtype = DT_REG; - } + fattr->cf_dtype = S_DT(fattr->cf_mode); out_reparse: if (S_ISLNK(fattr->cf_mode)) { diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c index b3a8f9c6fcff..28071b2be88c 100644 --- a/fs/smb/client/readdir.c +++ b/fs/smb/client/readdir.c @@ -241,31 +241,28 @@ cifs_posix_to_fattr(struct cifs_fattr *fattr, struct smb2_posix_info *info, fattr->cf_nlink = le32_to_cpu(info->HardLinks); fattr->cf_cifsattrs = le32_to_cpu(info->DosAttributes); - /* - * Since we set the inode type below we need to mask off - * to avoid strange results if bits set above. - * XXX: why not make server&client use the type bits? - */ - fattr->cf_mode = le32_to_cpu(info->Mode) & ~S_IFMT; + if (fattr->cf_cifsattrs & ATTR_REPARSE) { + fattr->cf_cifstag = le32_to_cpu(info->ReparseTag); + } + + fattr->cf_mode = wire_mode_to_posix(le32_to_cpu(info->Mode)); + fattr->cf_dtype = S_DT(le32_to_cpu(info->Mode)); + + switch (fattr->cf_mode & S_IFMT) { + case S_IFLNK: + case S_IFBLK: + case S_IFCHR: + fattr->cf_flags |= CIFS_FATTR_NEED_REVAL; + break; + default: + break; + } cifs_dbg(FYI, "posix fattr: dev %d, reparse %d, mode %o\n", le32_to_cpu(info->DeviceId), le32_to_cpu(info->ReparseTag), le32_to_cpu(info->Mode)); - if (fattr->cf_cifsattrs & ATTR_DIRECTORY) { - fattr->cf_mode |= S_IFDIR; - fattr->cf_dtype = DT_DIR; - } else { - /* - * mark anything that is not a dir as regular - * file. special files should have the REPARSE - * attribute and will be marked as needing revaluation - */ - fattr->cf_mode |= S_IFREG; - fattr->cf_dtype = DT_REG; - } - sid_to_id(cifs_sb, &parsed.owner, fattr, SIDOWNER); sid_to_id(cifs_sb, &parsed.group, fattr, SIDGROUP); } diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c index 74abbdf5026c..d392b9c53e43 100644 --- a/fs/smb/client/reparse.c +++ b/fs/smb/client/reparse.c @@ -661,44 +661,60 @@ static void wsl_to_fattr(struct cifs_open_info_data *data, fattr->cf_dtype = S_DT(fattr->cf_mode); } -bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb, - struct cifs_fattr *fattr, - struct cifs_open_info_data *data) +static bool posix_reparse_to_fattr(struct cifs_sb_info *cifs_sb, + struct cifs_fattr *fattr, + struct cifs_open_info_data *data) { struct reparse_posix_data *buf = data->reparse.posix; - u32 tag = data->reparse.tag; - if (tag == IO_REPARSE_TAG_NFS && buf) { - if (le16_to_cpu(buf->ReparseDataLength) < sizeof(buf->InodeType)) + if (buf == NULL) { + return true; + } + + if (le16_to_cpu(buf->ReparseDataLength) < sizeof(buf->InodeType)) { + WARN_ON_ONCE(1); + return false; + } + + switch (le64_to_cpu(buf->InodeType)) { + case NFS_SPECFILE_CHR: + if (le16_to_cpu(buf->ReparseDataLength) != sizeof(buf->InodeType) + 8) { + WARN_ON_ONCE(1); return false; - switch (le64_to_cpu(buf->InodeType)) { - case NFS_SPECFILE_CHR: - if (le16_to_cpu(buf->ReparseDataLength) != sizeof(buf->InodeType) + 8) - return false; - fattr->cf_mode |= S_IFCHR; - fattr->cf_rdev = reparse_mkdev(buf->DataBuffer); - break; - case NFS_SPECFILE_BLK: - if (le16_to_cpu(buf->ReparseDataLength) != sizeof(buf->InodeType) + 8) - return false; - fattr->cf_mode |= S_IFBLK; - fattr->cf_rdev = reparse_mkdev(buf->DataBuffer); - break; - case NFS_SPECFILE_FIFO: - fattr->cf_mode |= S_IFIFO; - break; - case NFS_SPECFILE_SOCK: - fattr->cf_mode |= S_IFSOCK; - break; - case NFS_SPECFILE_LNK: - fattr->cf_mode |= S_IFLNK; - break; - default: + } + fattr->cf_mode |= S_IFCHR; + fattr->cf_rdev = reparse_mkdev(buf->DataBuffer); + break; + case NFS_SPECFILE_BLK: + if (le16_to_cpu(buf->ReparseDataLength) != sizeof(buf->InodeType) + 8) { WARN_ON_ONCE(1); return false; } - goto out; + fattr->cf_mode |= S_IFBLK; + fattr->cf_rdev = reparse_mkdev(buf->DataBuffer); + break; + case NFS_SPECFILE_FIFO: + fattr->cf_mode |= S_IFIFO; + break; + case NFS_SPECFILE_SOCK: + fattr->cf_mode |= S_IFSOCK; + break; + case NFS_SPECFILE_LNK: + fattr->cf_mode |= S_IFLNK; + break; + default: + WARN_ON_ONCE(1); + return false; } + return true; +} + +bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb, + struct cifs_fattr *fattr, + struct cifs_open_info_data *data) +{ + u32 tag = data->reparse.tag; + bool ok; switch (tag) { case IO_REPARSE_TAG_INTERNAL: @@ -718,15 +734,19 @@ bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb, case IO_REPARSE_TAG_LX_BLK: wsl_to_fattr(data, cifs_sb, tag, fattr); break; + case IO_REPARSE_TAG_NFS: + ok = posix_reparse_to_fattr(cifs_sb, fattr, data); + if (!ok) + return false; + break; case 0: /* SMB1 symlink */ case IO_REPARSE_TAG_SYMLINK: - case IO_REPARSE_TAG_NFS: fattr->cf_mode |= S_IFLNK; break; default: return false; } -out: + fattr->cf_dtype = S_DT(fattr->cf_mode); return true; } -- 2.47.0 From 436e2dd352980f0be7f43e02c8c24d558a7d54bc Mon Sep 17 00:00:00 2001 From: Ralph Boehme <slow@samba.org> Date: Mon, 25 Nov 2024 16:19:56 +0100 Subject: [PATCH 3/3] fs/smb/client: cifs_prime_dcache() for SMB3 POSIX reparse points --- fs/smb/client/readdir.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c index 28071b2be88c..d2137bcb8d87 100644 --- a/fs/smb/client/readdir.c +++ b/fs/smb/client/readdir.c @@ -71,6 +71,8 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name, struct inode *inode; struct super_block *sb = parent->d_sb; struct cifs_sb_info *cifs_sb = CIFS_SB(sb); + bool posix = cifs_sb_master_tcon(cifs_sb)->posix_extensions; + bool reparse_need_reval = false; DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq); int rc; @@ -85,7 +87,21 @@ cifs_prime_dcache(struct dentry *parent, struct qstr *name, * this spares us an invalidation. */ retry: - if ((fattr->cf_cifsattrs & ATTR_REPARSE) || + if (posix) { + switch (fattr->cf_mode & S_IFMT) { + case S_IFLNK: + case S_IFBLK: + case S_IFCHR: + reparse_need_reval = true; + break; + default: + break; + } + } else if (fattr->cf_cifsattrs & ATTR_REPARSE) { + reparse_need_reval = true; + } + + if (reparse_need_reval || (fattr->cf_flags & CIFS_FATTR_NEED_REVAL)) return; -- 2.47.0
Hi! Against Samba master special files are broken with posix extensions. On the Samba server: root@smb3-posix:/home/samba# ls -li /srv/samba/test/posix/ insgesamt 20 131958 brw-r--r-- 1 root root 0, 0 15. Nov 12:04 blockdev 131965 crw-r--r-- 1 root root 1, 1 15. Nov 12:04 chardev 131966 prw-r--r-- 1 samba samba 0 15. Nov 12:05 fif 131953 -rw-rwxrw-+ 2 samba samba 4 18. Nov 11:37 file 131953 -rw-rwxrw-+ 2 samba samba 4 18. Nov 11:37 hardlink 131957 lrwxrwxrwx 1 samba samba 4 15. Nov 12:03 symlink -> file 131954 -rwxrwxr-x+ 1 samba samba 0 18. Nov 15:28 symlinkoversmb # mount.cifs //127.0.0.1/posix /mnt/smb3unix/ -o username=samba,password=x,posix # ls -li /mnt/smb3unix/posix/ ls: cannot access '/mnt/smb3unix/posix/blockdev': No data available ls: cannot access '/mnt/smb3unix/posix/chardev': No data available ls: cannot access '/mnt/smb3unix/posix/symlinkoversmb': No data available ls: cannot access '/mnt/smb3unix/posix/fifo': No data available ls: cannot access '/mnt/smb3unix/posix/symlink': No data available total 16 ? -????????? ? ? ? ? ? blockdev ? -????????? ? ? ? ? ? chardev ? -????????? ? ? ? ? ? fifo 131953 -rw-rwxrw- 2 root samba 4 Nov 18 11:37 file 131953 -rw-rwxrw- 2 root samba 4 Nov 18 11:37 hardlink ? -????????? ? ? ? ? ? symlink ? -????????? ? ? ? ? ? symlinkoversmb I have two WIP patches that fix this as a side effect of adding support to using the new POSIX type as discussed at SDC, patches attached: root@smb3-posix:/home/samba# ls -li /mnt/smb3unix/posix/ insgesamt 21 131958 brw-r--r-- 1 root root 0, 0 15. Nov 12:04 blockdev 131965 crw-r--r-- 1 root root 1, 1 15. Nov 12:04 chardev 131966 prw-r--r-- 1 root samba 0 15. Nov 12:05 fifo 131953 -rw-rwxrw- 2 root samba 4 18. Nov 11:37 file 131953 -rw-rwxrw- 2 root samba 4 18. Nov 11:37 hardlink 131957 lrwxrwxrwx 1 root samba 4 15. Nov 12:03 symlink -> file 131954 lrwxrwxr-x 1 root samba 23 18. Nov 15:28 symlinkoversmb -> mnt/smb3unix/posix/file I'm also optimizing away unneeded reparse point queries (create+fsctl+close) for nfs reparse points where we only need the file type which we now get and use via the smb3 posix type. I'm sure I'm doing things wrong. Can you help me getting this across the line? -slow [1] <https://www.samba.org/~slow/SMB3_POSIX/fscc_posix_extensions.html#posix-file-type-definition>