Message ID | 20210422002824.12677-1-namjae.jeon@samsung.com (mailing list archive) |
---|---|
Headers | show |
Series | cifsd: introduce new SMB3 kernel server | expand |
On Thu, Apr 22, 2021 at 09:28:14AM +0900, Namjae Jeon wrote: > This is the patch series for cifsd(ksmbd) kernel server. > > What is cifsd(ksmbd) ? > ====================== > > The SMB family of protocols is the most widely deployed > network filesystem protocol, the default on Windows and Macs (and even > on many phones and tablets), with clients and servers on all major > operating systems, but lacked a kernel server for Linux. For many > cases the current userspace server choices were suboptimal > either due to memory footprint, performance or difficulty integrating > well with advanced Linux features. > > ksmbd is a new kernel module which implements the server-side of the SMB3 protocol. > The target is to provide optimized performance, GPLv2 SMB server, better > lease handling (distributed caching). The bigger goal is to add new > features more rapidly (e.g. RDMA aka "smbdirect", and recent encryption > and signing improvements to the protocol) which are easier to develop > on a smaller, more tightly optimized kernel server than for example > in Samba. The Samba project is much broader in scope (tools, security services, > LDAP, Active Directory Domain Controller, and a cross platform file server > for a wider variety of purposes) but the user space file server portion > of Samba has proved hard to optimize for some Linux workloads, including > for smaller devices. This is not meant to replace Samba, but rather be > an extension to allow better optimizing for Linux, and will continue to > integrate well with Samba user space tools and libraries where appropriate. > Working with the Samba team we have already made sure that the configuration > files and xattrs are in a compatible format between the kernel and > user space server. > > > Architecture > ============ > > |--- ... > --------|--- ksmbd/3 - Client 3 > |-------|--- ksmbd/2 - Client 2 > | | ____________________________________________________ > | | |- Client 1 | > <--- Socket ---|--- ksmbd/1 <<= Authentication : NTLM/NTLM2, Kerberos | > | | | | <<= SMB engine : SMB2, SMB2.1, SMB3, SMB3.0.2, | > | | | | SMB3.1.1 | > | | | |____________________________________________________| > | | | > | | |--- VFS --- Local Filesystem > | | > KERNEL |--- ksmbd/0(forker kthread) > ---------------||--------------------------------------------------------------- > USER || > || communication using NETLINK > || ______________________________________________ > || | | > ksmbd.mountd <<= DCE/RPC(srvsvc, wkssvc, samr, lsarpc) | > ^ | <<= configure shares setting, user accounts | > | |______________________________________________| > | > |------ smb.conf(config file) > | > |------ ksmbdpwd.db(user account/password file) > ^ > ksmbd.adduser ---------------| > > The subset of performance related operations(open/read/write/close etc.) belong > in kernelspace(ksmbd) and the other subset which belong to operations(DCE/RPC, > user account/share database) which are not really related with performance are > handled in userspace(ksmbd.mountd). > > When the ksmbd.mountd is started, It starts up a forker thread at initialization > time and opens a dedicated port 445 for listening to SMB requests. Whenever new > clients make request, Forker thread will accept the client connection and fork > a new thread for dedicated communication channel between the client and > the server. Judging from the diagram above, all those threads are kernel threads, is that right? So a kernel thread gets each call first, then uses netlink to get help from ksmbd.mountd if necessary, is that right? --b.
2021-04-28 5:53 GMT+09:00, J. Bruce Fields <bfields@fieldses.org>: > On Thu, Apr 22, 2021 at 09:28:14AM +0900, Namjae Jeon wrote: >> This is the patch series for cifsd(ksmbd) kernel server. >> >> What is cifsd(ksmbd) ? >> ====================== >> >> The SMB family of protocols is the most widely deployed >> network filesystem protocol, the default on Windows and Macs (and even >> on many phones and tablets), with clients and servers on all major >> operating systems, but lacked a kernel server for Linux. For many >> cases the current userspace server choices were suboptimal >> either due to memory footprint, performance or difficulty integrating >> well with advanced Linux features. >> >> ksmbd is a new kernel module which implements the server-side of the SMB3 >> protocol. >> The target is to provide optimized performance, GPLv2 SMB server, better >> lease handling (distributed caching). The bigger goal is to add new >> features more rapidly (e.g. RDMA aka "smbdirect", and recent encryption >> and signing improvements to the protocol) which are easier to develop >> on a smaller, more tightly optimized kernel server than for example >> in Samba. The Samba project is much broader in scope (tools, security >> services, >> LDAP, Active Directory Domain Controller, and a cross platform file >> server >> for a wider variety of purposes) but the user space file server portion >> of Samba has proved hard to optimize for some Linux workloads, including >> for smaller devices. This is not meant to replace Samba, but rather be >> an extension to allow better optimizing for Linux, and will continue to >> integrate well with Samba user space tools and libraries where >> appropriate. >> Working with the Samba team we have already made sure that the >> configuration >> files and xattrs are in a compatible format between the kernel and >> user space server. >> >> >> Architecture >> ============ >> >> |--- ... >> --------|--- ksmbd/3 - Client 3 >> |-------|--- ksmbd/2 - Client 2 >> | | >> ____________________________________________________ >> | | |- Client 1 >> | >> <--- Socket ---|--- ksmbd/1 <<= Authentication : NTLM/NTLM2, Kerberos >> | >> | | | | <<= SMB engine : SMB2, SMB2.1, SMB3, >> SMB3.0.2, | >> | | | | SMB3.1.1 >> | >> | | | >> |____________________________________________________| >> | | | >> | | |--- VFS --- Local Filesystem >> | | >> KERNEL |--- ksmbd/0(forker kthread) >> ---------------||--------------------------------------------------------------- >> USER || >> || communication using NETLINK >> || ______________________________________________ >> || | | >> ksmbd.mountd <<= DCE/RPC(srvsvc, wkssvc, samr, lsarpc) | >> ^ | <<= configure shares setting, user accounts | >> | |______________________________________________| >> | >> |------ smb.conf(config file) >> | >> |------ ksmbdpwd.db(user account/password file) >> ^ >> ksmbd.adduser ---------------| >> >> The subset of performance related operations(open/read/write/close etc.) >> belong >> in kernelspace(ksmbd) and the other subset which belong to >> operations(DCE/RPC, >> user account/share database) which are not really related with performance >> are >> handled in userspace(ksmbd.mountd). >> >> When the ksmbd.mountd is started, It starts up a forker thread at >> initialization >> time and opens a dedicated port 445 for listening to SMB requests. >> Whenever new >> clients make request, Forker thread will accept the client connection and >> fork >> a new thread for dedicated communication channel between the client and >> the server. > > Judging from the diagram above, all those threads are kernel threads, is > that right? So a kernel thread gets each call first, then uses netlink > to get help from ksmbd.mountd if necessary, is that right? Yes, That's right. > > --b. >
On Thu, Apr 22, 2021 at 09:28:14AM +0900, Namjae Jeon wrote: > ACLs Partially Supported. only DACLs available, SACLs > (auditing) is planned for the future. For > ownership (SIDs) ksmbd generates random subauth > values(then store it to disk) and use uid/gid > get from inode as RID for local domain SID. > The current acl implementation is limited to > standalone server, not a domain member. > Integration with Samba tools is being worked on to > allow future support for running as a domain member. How exactly is this implementing ACLs? I grepped through the code a bit and couldn't quite figure it out--it looked like maybe it's both converting to a POSIX ACL and storing the full SBM ACL in an xattr, is that correct? When you read an ACL, and both are present, which do you use? Also it looked like there's some code from fs/nfsd/nfs4acl.c, could we share that somehow instead of copying? --b.
On Thu, Apr 22, 2021 at 09:28:14AM +0900, Namjae Jeon wrote: > This is the patch series for cifsd(ksmbd) kernel server. Looks like this series probably isn't bisectable. E.g. while looking at the ACL code I noticed ksmbd_vfs_setxattr is defined in a later patch than it's first used in. I know the requirement that everything build and run after each individual patch in the series is tough to meet when introducing a whole new subsystem. I'm sure it's still possible, but I don't know what the usual practice is in these cases. --b. > > What is cifsd(ksmbd) ? > ====================== > > The SMB family of protocols is the most widely deployed > network filesystem protocol, the default on Windows and Macs (and even > on many phones and tablets), with clients and servers on all major > operating systems, but lacked a kernel server for Linux. For many > cases the current userspace server choices were suboptimal > either due to memory footprint, performance or difficulty integrating > well with advanced Linux features. > > ksmbd is a new kernel module which implements the server-side of the SMB3 protocol. > The target is to provide optimized performance, GPLv2 SMB server, better > lease handling (distributed caching). The bigger goal is to add new > features more rapidly (e.g. RDMA aka "smbdirect", and recent encryption > and signing improvements to the protocol) which are easier to develop > on a smaller, more tightly optimized kernel server than for example > in Samba. The Samba project is much broader in scope (tools, security services, > LDAP, Active Directory Domain Controller, and a cross platform file server > for a wider variety of purposes) but the user space file server portion > of Samba has proved hard to optimize for some Linux workloads, including > for smaller devices. This is not meant to replace Samba, but rather be > an extension to allow better optimizing for Linux, and will continue to > integrate well with Samba user space tools and libraries where appropriate. > Working with the Samba team we have already made sure that the configuration > files and xattrs are in a compatible format between the kernel and > user space server. > > > Architecture > ============ > > |--- ... > --------|--- ksmbd/3 - Client 3 > |-------|--- ksmbd/2 - Client 2 > | | ____________________________________________________ > | | |- Client 1 | > <--- Socket ---|--- ksmbd/1 <<= Authentication : NTLM/NTLM2, Kerberos | > | | | | <<= SMB engine : SMB2, SMB2.1, SMB3, SMB3.0.2, | > | | | | SMB3.1.1 | > | | | |____________________________________________________| > | | | > | | |--- VFS --- Local Filesystem > | | > KERNEL |--- ksmbd/0(forker kthread) > ---------------||--------------------------------------------------------------- > USER || > || communication using NETLINK > || ______________________________________________ > || | | > ksmbd.mountd <<= DCE/RPC(srvsvc, wkssvc, samr, lsarpc) | > ^ | <<= configure shares setting, user accounts | > | |______________________________________________| > | > |------ smb.conf(config file) > | > |------ ksmbdpwd.db(user account/password file) > ^ > ksmbd.adduser ---------------| > > The subset of performance related operations(open/read/write/close etc.) belong > in kernelspace(ksmbd) and the other subset which belong to operations(DCE/RPC, > user account/share database) which are not really related with performance are > handled in userspace(ksmbd.mountd). > > When the ksmbd.mountd is started, It starts up a forker thread at initialization > time and opens a dedicated port 445 for listening to SMB requests. Whenever new > clients make request, Forker thread will accept the client connection and fork > a new thread for dedicated communication channel between the client and > the server. > > > ksmbd feature status > ==================== > > ============================== ================================================= > Feature name Status > ============================== ================================================= > Dialects Supported. SMB2.1 SMB3.0, SMB3.1.1 dialects > (intentionally excludes security vulnerable SMB1 dialect). > Auto Negotiation Supported. > Compound Request Supported. > Oplock Cache Mechanism Supported. > SMB2 leases(v1 lease) Supported. > Directory leases(v2 lease) Planned for future. > Multi-credits Supported. > NTLM/NTLMv2 Supported. > HMAC-SHA256 Signing Supported. > Secure negotiate Supported. > Signing Update Supported. > Pre-authentication integrity Supported. > SMB3 encryption(CCM, GCM) Supported. (CCM and GCM128 supported, GCM256 in progress) > SMB direct(RDMA) Partially Supported. SMB3 Multi-channel is required > to connect to Windows client. > SMB3 Multi-channel In Progress. > SMB3.1.1 POSIX extension Supported. > ACLs Partially Supported. only DACLs available, SACLs > (auditing) is planned for the future. For > ownership (SIDs) ksmbd generates random subauth > values(then store it to disk) and use uid/gid > get from inode as RID for local domain SID. > The current acl implementation is limited to > standalone server, not a domain member. > Integration with Samba tools is being worked on to > allow future support for running as a domain member. > Kerberos Supported. > Durable handle v1,v2 Planned for future. > Persistent handle Planned for future. > SMB2 notify Planned for future. > Sparse file support Supported. > DCE/RPC support Partially Supported. a few calls(NetShareEnumAll, > NetServerGetInfo, SAMR, LSARPC) that are needed > for file server handled via netlink interface from > ksmbd.mountd. Additional integration with Samba > tools and libraries via upcall is being investigated > to allow support for additional DCE/RPC management > calls (and future support for Witness protocol e.g.) > ============================== ================================================= > > All features required as file server are currently implemented in ksmbd. > In particular, the implementation of SMB Direct(RDMA) is only currently > possible with ksmbd (among Linux servers) > > > Stability > ========= > > It has been proved to be stable. A significant amount of xfstests pass and > are run regularly from Linux to Linux: > > http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/8/builds/32 > > In addition regression tests using the broadest SMB3 functional test suite > (Samba's "smbtorture") are run on every checkin. > It has already been used by many other open source toolkits and commercial companies > that need NAS functionality. Their issues have been fixed and contributions are > applied into ksmbd. Ksmbd has been well tested and verified in the field and market. > > > Mailing list and repositories > ============================= > - linux-cifsd-devel@lists.sourceforge.net > - https://github.com/smfrench/smb3-kernel/tree/cifsd-for-next > - https://github.com/cifsd-team/cifsd (out-of-tree) > - https://github.com/cifsd-team/ksmbd-tools > > > How to run ksmbd > ================ > > a. Download ksmbd-tools and compile them. > - https://github.com/cifsd-team/ksmbd-tools > > b. Create user/password for SMB share. > > # mkdir /etc/ksmbd/ > # ksmbd.adduser -a <Enter USERNAME for SMB share access> > > c. Create /etc/ksmbd/smb.conf file, add SMB share in smb.conf file > - Refer smb.conf.example and Documentation/configuration.txt > in ksmbd-tools > > d. Insert ksmbd.ko module > > # insmod ksmbd.ko > > e. Start ksmbd user space daemon > # ksmbd.mountd > > f. Access share from Windows or Linux using SMB > e.g. "mount -t cifs //server/share /mnt ..." > > > v2: > - fix an error code in smb2_read(). (Dan Carpenter) > - fix error handling in ksmbd_server_init() (Dan Carpenter) > - remove redundant assignment to variable err. (Colin Ian King) > - remove unneeded macros. > - fix wrong use of rw semaphore in __session_create(). > - use kmalloc() for small allocations. > - add the check to work file lock and rename behaviors like Windows > unless POSIX extensions are negotiated. > - clean-up codes using chechpatch.pl --strict. > - merge time_wrappers.h into smb_common.h. > - fix wrong prototype in comment (kernel test robot). > - fix implicit declaration of function 'groups_alloc' (kernel test robot). > - fix implicit declaration of function 'locks_alloc_lock' (kernel test robot). > - remove smack inherit leftovers. > - remove calling d_path in error paths. > - handle unhashed dentry in ksmbd_vfs_mkdir. > - use file_inode() instead of d_inode(). > - remove useless error handling in ksmbd_vfs_read. > - use xarray instead of linked list for tree connect list. > - remove stale prototype and variables. > - fix memory leak when loop ends (coverity-bot, Muhammad Usama Anjum). > - use kfree to free memory allocated by kmalloc or kzalloc (Muhammad Usama Anjum). > - fix memdup.cocci warnings (kernel test robot) > - remove wrappers of kvmalloc/kvfree. > - change the reference to configuration.txt (Mauro Carvalho Chehab). > - prevent a integer overflow in wm_alloc(). > - select SG_POOL for SMB_SERVER_SMBDIRECT. (Zhang Xiaoxu). > - remove unused including <linux/version.h> (Tian Tao). > - declare ida statically. > - add the check if parent is stable by unexpected rename. > - get parent dentry from child in ksmbd_vfs_remove_file(). > - re-implement ksmbd_vfs_kern_path. > - fix reference count decrement of unclaimed file in __ksmbd_lookup_fd. > - remove smb2_put_name(). (Marios Makassikis). > - remove unused smberr.h, nterr.c and netmisc.c. > - fix potential null-ptr-deref in smb2_open() (Marios Makassikis). > - use d_inode(). > - remove the dead code of unimplemented durable handle. > - use the generic one in lib/asn1_decoder.c > > v1: > - fix a handful of spelling mistakes (Colin Ian King) > - fix a precedence bug in parse_dacl() (Dan Carpenter) > - fix a IS_ERR() vs NULL bug (Dan Carpenter) > - fix a use after free on error path (Dan Carpenter) > - update cifsd.rst Documentation > - remove unneeded FIXME comments > - fix static checker warnings (Dan Carpenter) > - fix WARNING: unmet direct dependencies detected for CRYPTO_ARC4 (Randy Dunlap) > - uniquify extract_sharename() (Stephen Rothwell) > - fix WARNING: document isn't included in any toctree (Stephen Rothwell) > - fix WARNING: Title overline too short (Stephen Rothwell) > - fix warning: variable 'total_ace_size' and 'posix_ccontext'set but not used (kernel test rotbot) > - fix incorrect function comments (kernel test robot) > > Namjae Jeon (10): > cifsd: add document > cifsd: add server handler > cifsd: add trasport layers > cifsd: add authentication > cifsd: add smb3 engine part 1 > cifsd: add smb3 engine part 2 > cifsd: add oplock/lease cache mechanism > cifsd: add file operations > cifsd: add Kconfig and Makefile > MAINTAINERS: add cifsd kernel server > > Documentation/filesystems/cifs/cifsd.rst | 152 + > Documentation/filesystems/cifs/index.rst | 10 + > Documentation/filesystems/index.rst | 2 +- > MAINTAINERS | 12 +- > fs/Kconfig | 1 + > fs/Makefile | 1 + > fs/cifsd/Kconfig | 67 + > fs/cifsd/Makefile | 17 + > fs/cifsd/asn1.c | 352 + > fs/cifsd/asn1.h | 29 + > fs/cifsd/auth.c | 1323 ++++ > fs/cifsd/auth.h | 90 + > fs/cifsd/buffer_pool.c | 264 + > fs/cifsd/buffer_pool.h | 20 + > fs/cifsd/connection.c | 411 ++ > fs/cifsd/connection.h | 208 + > fs/cifsd/crypto_ctx.c | 286 + > fs/cifsd/crypto_ctx.h | 77 + > fs/cifsd/glob.h | 64 + > fs/cifsd/ksmbd_server.h | 283 + > fs/cifsd/ksmbd_work.c | 93 + > fs/cifsd/ksmbd_work.h | 110 + > fs/cifsd/mgmt/ksmbd_ida.c | 48 + > fs/cifsd/mgmt/ksmbd_ida.h | 34 + > fs/cifsd/mgmt/share_config.c | 239 + > fs/cifsd/mgmt/share_config.h | 81 + > fs/cifsd/mgmt/tree_connect.c | 122 + > fs/cifsd/mgmt/tree_connect.h | 56 + > fs/cifsd/mgmt/user_config.c | 70 + > fs/cifsd/mgmt/user_config.h | 66 + > fs/cifsd/mgmt/user_session.c | 328 + > fs/cifsd/mgmt/user_session.h | 103 + > fs/cifsd/misc.c | 340 + > fs/cifsd/misc.h | 44 + > fs/cifsd/ndr.c | 347 + > fs/cifsd/ndr.h | 21 + > fs/cifsd/nterr.h | 545 ++ > fs/cifsd/ntlmssp.h | 169 + > fs/cifsd/oplock.c | 1667 +++++ > fs/cifsd/oplock.h | 133 + > fs/cifsd/server.c | 631 ++ > fs/cifsd/server.h | 60 + > fs/cifsd/smb2misc.c | 435 ++ > fs/cifsd/smb2ops.c | 300 + > fs/cifsd/smb2pdu.c | 8069 ++++++++++++++++++++++ > fs/cifsd/smb2pdu.h | 1646 +++++ > fs/cifsd/smb_common.c | 652 ++ > fs/cifsd/smb_common.h | 544 ++ > fs/cifsd/smbacl.c | 1317 ++++ > fs/cifsd/smbacl.h | 201 + > fs/cifsd/smbfsctl.h | 90 + > fs/cifsd/smbstatus.h | 1822 +++++ > fs/cifsd/spnego_negtokeninit.asn1 | 43 + > fs/cifsd/spnego_negtokentarg.asn1 | 19 + > fs/cifsd/transport_ipc.c | 881 +++ > fs/cifsd/transport_ipc.h | 54 + > fs/cifsd/transport_rdma.c | 2034 ++++++ > fs/cifsd/transport_rdma.h | 61 + > fs/cifsd/transport_tcp.c | 618 ++ > fs/cifsd/transport_tcp.h | 13 + > fs/cifsd/unicode.c | 383 + > fs/cifsd/unicode.h | 356 + > fs/cifsd/uniupr.h | 268 + > fs/cifsd/vfs.c | 1995 ++++++ > fs/cifsd/vfs.h | 273 + > fs/cifsd/vfs_cache.c | 683 ++ > fs/cifsd/vfs_cache.h | 185 + > 67 files changed, 31916 insertions(+), 2 deletions(-) > create mode 100644 Documentation/filesystems/cifs/cifsd.rst > create mode 100644 Documentation/filesystems/cifs/index.rst > create mode 100644 fs/cifsd/Kconfig > create mode 100644 fs/cifsd/Makefile > create mode 100644 fs/cifsd/asn1.c > create mode 100644 fs/cifsd/asn1.h > create mode 100644 fs/cifsd/auth.c > create mode 100644 fs/cifsd/auth.h > create mode 100644 fs/cifsd/buffer_pool.c > create mode 100644 fs/cifsd/buffer_pool.h > create mode 100644 fs/cifsd/connection.c > create mode 100644 fs/cifsd/connection.h > create mode 100644 fs/cifsd/crypto_ctx.c > create mode 100644 fs/cifsd/crypto_ctx.h > create mode 100644 fs/cifsd/glob.h > create mode 100644 fs/cifsd/ksmbd_server.h > create mode 100644 fs/cifsd/ksmbd_work.c > create mode 100644 fs/cifsd/ksmbd_work.h > create mode 100644 fs/cifsd/mgmt/ksmbd_ida.c > create mode 100644 fs/cifsd/mgmt/ksmbd_ida.h > create mode 100644 fs/cifsd/mgmt/share_config.c > create mode 100644 fs/cifsd/mgmt/share_config.h > create mode 100644 fs/cifsd/mgmt/tree_connect.c > create mode 100644 fs/cifsd/mgmt/tree_connect.h > create mode 100644 fs/cifsd/mgmt/user_config.c > create mode 100644 fs/cifsd/mgmt/user_config.h > create mode 100644 fs/cifsd/mgmt/user_session.c > create mode 100644 fs/cifsd/mgmt/user_session.h > create mode 100644 fs/cifsd/misc.c > create mode 100644 fs/cifsd/misc.h > create mode 100644 fs/cifsd/ndr.c > create mode 100644 fs/cifsd/ndr.h > create mode 100644 fs/cifsd/nterr.h > create mode 100644 fs/cifsd/ntlmssp.h > create mode 100644 fs/cifsd/oplock.c > create mode 100644 fs/cifsd/oplock.h > create mode 100644 fs/cifsd/server.c > create mode 100644 fs/cifsd/server.h > create mode 100644 fs/cifsd/smb2misc.c > create mode 100644 fs/cifsd/smb2ops.c > create mode 100644 fs/cifsd/smb2pdu.c > create mode 100644 fs/cifsd/smb2pdu.h > create mode 100644 fs/cifsd/smb_common.c > create mode 100644 fs/cifsd/smb_common.h > create mode 100644 fs/cifsd/smbacl.c > create mode 100644 fs/cifsd/smbacl.h > create mode 100644 fs/cifsd/smbfsctl.h > create mode 100644 fs/cifsd/smbstatus.h > create mode 100644 fs/cifsd/spnego_negtokeninit.asn1 > create mode 100644 fs/cifsd/spnego_negtokentarg.asn1 > create mode 100644 fs/cifsd/transport_ipc.c > create mode 100644 fs/cifsd/transport_ipc.h > create mode 100644 fs/cifsd/transport_rdma.c > create mode 100644 fs/cifsd/transport_rdma.h > create mode 100644 fs/cifsd/transport_tcp.c > create mode 100644 fs/cifsd/transport_tcp.h > create mode 100644 fs/cifsd/unicode.c > create mode 100644 fs/cifsd/unicode.h > create mode 100644 fs/cifsd/uniupr.h > create mode 100644 fs/cifsd/vfs.c > create mode 100644 fs/cifsd/vfs.h > create mode 100644 fs/cifsd/vfs_cache.c > create mode 100644 fs/cifsd/vfs_cache.h > > -- > 2.17.1 >
bfields@fieldses.org (J. Bruce Fields) writes: > On Thu, Apr 22, 2021 at 09:28:14AM +0900, Namjae Jeon wrote: >> This is the patch series for cifsd(ksmbd) kernel server. > > Looks like this series probably isn't bisectable. E.g. while looking at > the ACL code I noticed ksmbd_vfs_setxattr is defined in a later patch > than it's first used in. The Kconfig and Makefile are added in the last patch so it should be ok. Cheers,
On Wed, Apr 28, 2021 at 10:19:58PM +0200, Aurélien Aptel wrote: > bfields@fieldses.org (J. Bruce Fields) writes: > > On Thu, Apr 22, 2021 at 09:28:14AM +0900, Namjae Jeon wrote: > >> This is the patch series for cifsd(ksmbd) kernel server. > > > > Looks like this series probably isn't bisectable. E.g. while looking at > > the ACL code I noticed ksmbd_vfs_setxattr is defined in a later patch > > than it's first used in. > > The Kconfig and Makefile are added in the last patch so it should be ok. I'm not sure if doing that way is really any better than making it one big patch. I'd rather see multiple patches that were actually functional at each stage: e.g., start with a server that responds to some sort of rpc-level ping but does nothing else, then add basic file IO, etc. I don't know if that's practical. --b.
"J. Bruce Fields" <bfields@fieldses.org> writes: > I'd rather see multiple patches that were actually functional at each > stage: e.g., start with a server that responds to some sort of rpc-level > ping but does nothing else, then add basic file IO, etc. > > I don't know if that's practical. Although it would certainly be nice I don't think it's realistic to expect this kind of retro-logical-rewriting. AFAIK the other new fs-related addition (ntfs patchset) is using the same trick of adding the Makefile at the end after it was suggested on the mailing list. So there's a precedent. Cheers,
On Thu, Apr 29, 2021 at 12:24:17AM +0200, Aurélien Aptel wrote: > "J. Bruce Fields" <bfields@fieldses.org> writes: > > I'd rather see multiple patches that were actually functional at each > > stage: e.g., start with a server that responds to some sort of rpc-level > > ping but does nothing else, then add basic file IO, etc. > > > > I don't know if that's practical. > > Although it would certainly be nice I don't think it's realistic to > expect this kind of retro-logical-rewriting. AFAIK the other new > fs-related addition (ntfs patchset) is using the same trick of adding > the Makefile at the end after it was suggested on the mailing list. So > there's a precedent. OK, I wondered if that might be the case. I don't love it, but, fair enough, maybe that's the best compromise. --b.
> On Thu, Apr 22, 2021 at 09:28:14AM +0900, Namjae Jeon wrote: > > ACLs Partially Supported. only DACLs available, SACLs > > (auditing) is planned for the future. For > > ownership (SIDs) ksmbd generates random subauth > > values(then store it to disk) and use uid/gid > > get from inode as RID for local domain SID. > > The current acl implementation is limited to > > standalone server, not a domain member. > > Integration with Samba tools is being worked on to > > allow future support for running as a domain member. > Hi Bruce, > How exactly is this implementing ACLs? I grepped through the code a bit and couldn't quite figure it > out--it looked like maybe it's both converting to a POSIX ACL and storing the full SBM ACL in an xattr, > is that correct? When you read an ACL, and both are present, which do you use? If 'vfs objects = acl_xattr' parameter is defined in smb.conf, ksmbd store both. If not, only posix acl will be stored. To avoid translation from posix acl to ntacl from request of client, ksmbd use ntacl in xattr first. > > Also it looked like there's some code from fs/nfsd/nfs4acl.c, could we share that somehow instead of > copying? Hm.. I do not know how to share the code with nfsd at present. Maybe we can discuss it again after upstream ? Any thought ? > > --b.
> On Thu, Apr 29, 2021 at 12:24:17AM +0200, Aurélien Aptel wrote: > > "J. Bruce Fields" <bfields@fieldses.org> writes: > > > I'd rather see multiple patches that were actually functional at > > > each > > > stage: e.g., start with a server that responds to some sort of > > > rpc-level ping but does nothing else, then add basic file IO, etc. > > > > > > I don't know if that's practical. > > > > Although it would certainly be nice I don't think it's realistic to > > expect this kind of retro-logical-rewriting. AFAIK the other new > > fs-related addition (ntfs patchset) is using the same trick of adding > > the Makefile at the end after it was suggested on the mailing list. So > > there's a precedent. > > OK, I wondered if that might be the case. > > I don't love it, but, fair enough, maybe that's the best compromise. Thank you for your understanding. One big patch seems to be unable to review because the code seem to be cut off in the mail. So it does not exceed 300KB per patch. So we split patches out by layer and added Makefile, Kconfig at the end. Thanks! > > --b.