[PATCH 1/2] ksmbd: update documentation

[PATCH 1/2] ksmbd: update documentation

[Namjae Jeon]

configuration.txt in ksmbd-tools moved to smb.conf(5ksmbd) manpage.
update it and more detailed ksmbd-tools build method.

Cc: Tom Talpey <tom@talpey.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 Documentation/filesystems/cifs/ksmbd.rst | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/Documentation/filesystems/cifs/ksmbd.rst b/Documentation/filesystems/cifs/ksmbd.rst
index 1af600db2e70..767e12d2045a 100644
--- a/Documentation/filesystems/cifs/ksmbd.rst
+++ b/Documentation/filesystems/cifs/ksmbd.rst
@@ -121,20 +121,26 @@ How to run
 1. Download ksmbd-tools and compile them.
 	- https://github.com/cifsd-team/ksmbd-tools
 
+        # ./autogen.sh
+        # ./configure --sysconfdir=/etc --with-rundir=/run
+        # make & sudo make install
+
 2. Create user/password for SMB share.
 
 	# mkdir /etc/ksmbd/
 	# ksmbd.adduser -a <Enter USERNAME for SMB share access>
 
 3. Create /etc/ksmbd/smb.conf file, add SMB share in smb.conf file
-	- Refer smb.conf.example and
-          https://github.com/cifsd-team/ksmbd-tools/blob/master/Documentation/configuration.txt
+	- Refer smb.conf.example, See smb.conf(5ksmbd) for details.
+
+        # man smb.conf.5ksmbd
 
 4. Insert ksmbd.ko module
 
 	# insmod ksmbd.ko
 
 5. Start ksmbd user space daemon
+
 	# ksmbd.mountd
 
 6. Access share from Windows or Linux using CIFS
-- 
2.25.1

[PATCH 2/2] ksmbd: remove generic_fillattr use in smb2_open()

[Namjae Jeon]

Removed the use of unneeded generic_fillattr() in smb2_open().

Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/ksmbd/smb2pdu.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index c49f65146ab3..ad6410874b95 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -2761,7 +2761,6 @@ int smb2_open(struct ksmbd_work *work)
 	} else {
 		file_present = true;
 		user_ns = mnt_user_ns(path.mnt);
-		generic_fillattr(user_ns, d_inode(path.dentry), &stat);
 	}
 	if (stream_name) {
 		if (req->CreateOptions & FILE_DIRECTORY_FILE_LE) {
@@ -2770,7 +2769,8 @@ int smb2_open(struct ksmbd_work *work)
 				rsp->hdr.Status = STATUS_NOT_A_DIRECTORY;
 			}
 		} else {
-			if (S_ISDIR(stat.mode) && s_type == DATA_STREAM) {
+			if (file_present && S_ISDIR(d_inode(path.dentry)->i_mode) &&
+			    s_type == DATA_STREAM) {
 				rc = -EIO;
 				rsp->hdr.Status = STATUS_FILE_IS_A_DIRECTORY;
 			}
@@ -2787,7 +2787,8 @@ int smb2_open(struct ksmbd_work *work)
 	}
 
 	if (file_present && req->CreateOptions & FILE_NON_DIRECTORY_FILE_LE &&
-	    S_ISDIR(stat.mode) && !(req->CreateOptions & FILE_DELETE_ON_CLOSE_LE)) {
+	    S_ISDIR(d_inode(path.dentry)->i_mode) &&
+	    !(req->CreateOptions & FILE_DELETE_ON_CLOSE_LE)) {
 		ksmbd_debug(SMB, "open() argument is a directory: %s, %x\n",
 			    name, req->CreateOptions);
 		rsp->hdr.Status = STATUS_FILE_IS_A_DIRECTORY;
@@ -2797,7 +2798,7 @@ int smb2_open(struct ksmbd_work *work)
 
 	if (file_present && (req->CreateOptions & FILE_DIRECTORY_FILE_LE) &&
 	    !(req->CreateDisposition == FILE_CREATE_LE) &&
-	    !S_ISDIR(stat.mode)) {
+	    !S_ISDIR(d_inode(path.dentry)->i_mode)) {
 		rsp->hdr.Status = STATUS_NOT_A_DIRECTORY;
 		rc = -EIO;
 		goto err_out;
-- 
2.25.1

Re: [PATCH 1/2] ksmbd: update documentation

[Tom Talpey]

On 8/30/2022 10:17 AM, Namjae Jeon wrote:
> configuration.txt in ksmbd-tools moved to smb.conf(5ksmbd) manpage.
> update it and more detailed ksmbd-tools build method.
> 
> Cc: Tom Talpey <tom@talpey.com>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> ---
>   Documentation/filesystems/cifs/ksmbd.rst | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/filesystems/cifs/ksmbd.rst b/Documentation/filesystems/cifs/ksmbd.rst
> index 1af600db2e70..767e12d2045a 100644
> --- a/Documentation/filesystems/cifs/ksmbd.rst
> +++ b/Documentation/filesystems/cifs/ksmbd.rst
> @@ -121,20 +121,26 @@ How to run
>   1. Download ksmbd-tools and compile them.
>   	- https://github.com/cifsd-team/ksmbd-tools
>   
> +        # ./autogen.sh
> +        # ./configure --sysconfdir=/etc --with-rundir=/run
> +        # make & sudo make install

I believe you mean "make && sudo make install"? The single & will
kick off two make's in parallel.

>   2. Create user/password for SMB share.
>   
>   	# mkdir /etc/ksmbd/
>   	# ksmbd.adduser -a <Enter USERNAME for SMB share access>

It may be worth mentioning that it's not just single-user access, and
that additional users can be configured.

>   3. Create /etc/ksmbd/smb.conf file, add SMB share in smb.conf file
> -	- Refer smb.conf.example and
> -          https://github.com/cifsd-team/ksmbd-tools/blob/master/Documentation/configuration.txt
> +	- Refer smb.conf.example, See smb.conf(5ksmbd) for details.
> +
> +        # man smb.conf.5ksmbd

I like the new manpage, but that's a strange path. Are you sure
the various maintainers will deploy it that way?

Also, it has always bothered me that the name "smb.conf" is the
same as the Samba server's configuration file, just in a different
directory. If someone enters "man smb.conf", there may be confusion.
I really wish the file was called "ksmbd.conf".

Why not putting this under a simpler manpage title "man ksmbd"?
To me, that's much more logical and it avoids both the confusion
and having to somehow know that weird manpath.

>   4. Insert ksmbd.ko module
>   
>   	# insmod ksmbd.ko

Well, it's worth mentioning that a properly configured and built
kernel is a prerequisite here...

Also, sudo.

>   5. Start ksmbd user space daemon
> +
>   	# ksmbd.mountd

FYI, Ubuntu Jammy pre-configures ksmbd as a service, and there it's
as simple as "sudo service ksmbd start".

Do you not want to mention the other ksmbd.<foo> helpers here?

>   6. Access share from Windows or Linux using CIFS

Pointer to cifs.ko how-to page?

Basically, I'm encouraging these pages to be (much) more user
friendly! They're fine for developers, but way too fiddly IMO
for naive users, or even for admins. It has taken me days to get
this all going on my fresh machines.

Either way, thanks for the cleanup so far!!

Tom.

Re: [PATCH 1/2] ksmbd: update documentation

[Tom Talpey]

BTW, I expected a "2/2" message, but never saw it. Was the
subject a typo?

Anyway, I have a couple of comments on the new file, which are from
going to 
https://github.com/cifsd-team/ksmbd-tools/blob/master/smb.conf.5ksmbd.in

It refers to "SMB1", but ksmbd removed that, right? And because the
SMB2 (and SMB3!) dialects don't make it optional, does the setting
still make sense? Either way, delete "SMB1" and don't restrict the
discussion to "SMB2":

\fBserver signing\fR (G)
This controls whether the client is allowed or required to use SMB1 and 
SMB2 signing.
With \fBserver signing = disabled\fR, SMB1 signing is not offered and 
\fBserver signing = auto\fR applies when negotiating SMB2.
With \fBserver signing = auto\fR, SMB1 signing is offered and SMB2 
signing is required.
With \fBserver signing = mandatory\fR, both SMB1 and SMB2 signing is 
required.


Also, most of the settings start with the words "This controls whether".
I find these words unnecessary, and by the third or fourth one, they
are super-redundant. Would you consider rewording them in a more
active way, for example:

\fBbind interfaces only\fR (G)
This controls whether to only bind to interfaces specified with 
\fBinterfaces\fR.

... could be:

\fBbind interfaces only\fR (G)
Only bind to the interfaces specified in the \fBinterfaces\fR setting.

Tom.


On 8/30/2022 10:17 AM, Namjae Jeon wrote:
> configuration.txt in ksmbd-tools moved to smb.conf(5ksmbd) manpage.
> update it and more detailed ksmbd-tools build method.
> 
> Cc: Tom Talpey <tom@talpey.com>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> ---
>   Documentation/filesystems/cifs/ksmbd.rst | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/filesystems/cifs/ksmbd.rst b/Documentation/filesystems/cifs/ksmbd.rst
> index 1af600db2e70..767e12d2045a 100644
> --- a/Documentation/filesystems/cifs/ksmbd.rst
> +++ b/Documentation/filesystems/cifs/ksmbd.rst
> @@ -121,20 +121,26 @@ How to run
>   1. Download ksmbd-tools and compile them.
>   	- https://github.com/cifsd-team/ksmbd-tools
>   
> +        # ./autogen.sh
> +        # ./configure --sysconfdir=/etc --with-rundir=/run
> +        # make & sudo make install
> +
>   2. Create user/password for SMB share.
>   
>   	# mkdir /etc/ksmbd/
>   	# ksmbd.adduser -a <Enter USERNAME for SMB share access>
>   
>   3. Create /etc/ksmbd/smb.conf file, add SMB share in smb.conf file
> -	- Refer smb.conf.example and
> -          https://github.com/cifsd-team/ksmbd-tools/blob/master/Documentation/configuration.txt
> +	- Refer smb.conf.example, See smb.conf(5ksmbd) for details.
> +
> +        # man smb.conf.5ksmbd
>   
>   4. Insert ksmbd.ko module
>   
>   	# insmod ksmbd.ko
>   
>   5. Start ksmbd user space daemon
> +
>   	# ksmbd.mountd
>   
>   6. Access share from Windows or Linux using CIFS

Re: [PATCH 1/2] ksmbd: update documentation

[Namjae Jeon]

2022-08-31 2:32 GMT+09:00, Tom Talpey <tom@talpey.com>:
> BTW, I expected a "2/2" message, but never saw it. Was the
> subject a typo?
Ah, Yes, 2/2 patch was sent to the list without you cc. I usually send
the patch to the list with maintainers/reviewers, but You have report
the problem about 1/2 patch, So I include you cc tag in the patch.
>
> Anyway, I have a couple of comments on the new file, which are from
> going to
> https://github.com/cifsd-team/ksmbd-tools/blob/master/smb.conf.5ksmbd.in
>
> It refers to "SMB1", but ksmbd removed that, right? And because the
> SMB2 (and SMB3!) dialects don't make it optional, does the setting
> still make sense? Either way, delete "SMB1" and don't restrict the
> discussion to "SMB2":
Okay:)
>
> \fBserver signing\fR (G)
> This controls whether the client is allowed or required to use SMB1 and
> SMB2 signing.
> With \fBserver signing = disabled\fR, SMB1 signing is not offered and
> \fBserver signing = auto\fR applies when negotiating SMB2.
> With \fBserver signing = auto\fR, SMB1 signing is offered and SMB2
> signing is required.
> With \fBserver signing = mandatory\fR, both SMB1 and SMB2 signing is
> required.
>
>
> Also, most of the settings start with the words "This controls whether".
> I find these words unnecessary, and by the third or fourth one, they
> are super-redundant. Would you consider rewording them in a more
> active way, for example:
Okay, I will do that!

Thanks for your review!
>
> \fBbind interfaces only\fR (G)
> This controls whether to only bind to interfaces specified with
> \fBinterfaces\fR.
>
> ... could be:
>
> \fBbind interfaces only\fR (G)
> Only bind to the interfaces specified in the \fBinterfaces\fR setting.
>
> Tom.
>
>
> On 8/30/2022 10:17 AM, Namjae Jeon wrote:
>> configuration.txt in ksmbd-tools moved to smb.conf(5ksmbd) manpage.
>> update it and more detailed ksmbd-tools build method.
>>
>> Cc: Tom Talpey <tom@talpey.com>
>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
>> ---
>>   Documentation/filesystems/cifs/ksmbd.rst | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/filesystems/cifs/ksmbd.rst
>> b/Documentation/filesystems/cifs/ksmbd.rst
>> index 1af600db2e70..767e12d2045a 100644
>> --- a/Documentation/filesystems/cifs/ksmbd.rst
>> +++ b/Documentation/filesystems/cifs/ksmbd.rst
>> @@ -121,20 +121,26 @@ How to run
>>   1. Download ksmbd-tools and compile them.
>>   	- https://github.com/cifsd-team/ksmbd-tools
>>
>> +        # ./autogen.sh
>> +        # ./configure --sysconfdir=/etc --with-rundir=/run
>> +        # make & sudo make install
>> +
>>   2. Create user/password for SMB share.
>>
>>   	# mkdir /etc/ksmbd/
>>   	# ksmbd.adduser -a <Enter USERNAME for SMB share access>
>>
>>   3. Create /etc/ksmbd/smb.conf file, add SMB share in smb.conf file
>> -	- Refer smb.conf.example and
>> -
>> https://github.com/cifsd-team/ksmbd-tools/blob/master/Documentation/configuration.txt
>> +	- Refer smb.conf.example, See smb.conf(5ksmbd) for details.
>> +
>> +        # man smb.conf.5ksmbd
>>
>>   4. Insert ksmbd.ko module
>>
>>   	# insmod ksmbd.ko
>>
>>   5. Start ksmbd user space daemon
>> +
>>   	# ksmbd.mountd
>>
>>   6. Access share from Windows or Linux using CIFS
>

Re: [PATCH 1/2] ksmbd: update documentation

[Namjae Jeon]

2022-08-31 1:13 GMT+09:00, Tom Talpey <tom@talpey.com>:
> On 8/30/2022 10:17 AM, Namjae Jeon wrote:
>> configuration.txt in ksmbd-tools moved to smb.conf(5ksmbd) manpage.
>> update it and more detailed ksmbd-tools build method.
>>
>> Cc: Tom Talpey <tom@talpey.com>
>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
>> ---
>>   Documentation/filesystems/cifs/ksmbd.rst | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/filesystems/cifs/ksmbd.rst
>> b/Documentation/filesystems/cifs/ksmbd.rst
>> index 1af600db2e70..767e12d2045a 100644
>> --- a/Documentation/filesystems/cifs/ksmbd.rst
>> +++ b/Documentation/filesystems/cifs/ksmbd.rst
>> @@ -121,20 +121,26 @@ How to run
>>   1. Download ksmbd-tools and compile them.
>>   	- https://github.com/cifsd-team/ksmbd-tools
>>
>> +        # ./autogen.sh
>> +        # ./configure --sysconfdir=/etc --with-rundir=/run
>> +        # make & sudo make install
>
> I believe you mean "make && sudo make install"? The single & will
> kick off two make's in parallel.
Will fix it.
>
>>   2. Create user/password for SMB share.
>>
>>   	# mkdir /etc/ksmbd/
>>   	# ksmbd.adduser -a <Enter USERNAME for SMB share access>
>
> It may be worth mentioning that it's not just single-user access, and
> that additional users can be configured.
Ah, Okay. Let me think more..
>
>>   3. Create /etc/ksmbd/smb.conf file, add SMB share in smb.conf file
>> -	- Refer smb.conf.example and
>> -
>> https://github.com/cifsd-team/ksmbd-tools/blob/master/Documentation/configuration.txt
>> +	- Refer smb.conf.example, See smb.conf(5ksmbd) for details.
>> +
>> +        # man smb.conf.5ksmbd
>
> I like the new manpage, but that's a strange path. Are you sure
> the various maintainers will deploy it that way?
I was sure there won't be any strong comments, as We've used the MIT
Kerberos (krb5-doc) case as an example. If samba is not installed, man
smb.conf show ksmbd's one.
>
> Also, it has always bothered me that the name "smb.conf" is the
> same as the Samba server's configuration file, just in a different
> directory. If someone enters "man smb.conf", there may be confusion.
> I really wish the file was called "ksmbd.conf".
The initial idea was to make the configuration compatible with samba.
Users copy it to ksmbd directory and reuse the smb.conf file they used
in samba as it is. And I have no strong opinion to renaming it to
ksmbd.conf(i.e. man ksmb.conf).  Atte, Any thought about this ?

>
> Why not putting this under a simpler manpage title "man ksmbd"?
> To me, that's much more logical and it avoids both the confusion
> and having to somehow know that weird manpath.
Is it possible to add it to manpage even though a utility or
configuration file named ksmbd doesn't exist? That is, as if there is
no "man nfs".
>
>>   4. Insert ksmbd.ko module
>>
>>   	# insmod ksmbd.ko
>
> Well, it's worth mentioning that a properly configured and built
> kernel is a prerequisite here...
Okay.
>
> Also, sudo.
>
>>   5. Start ksmbd user space daemon
>> +
>>   	# ksmbd.mountd
>
> FYI, Ubuntu Jammy pre-configures ksmbd as a service, and there it's
> as simple as "sudo service ksmbd start".
Is this command available on all distributions?

>
> Do you not want to mention the other ksmbd.<foo> helpers here?
Other ksmbd utils are described in README of ksmbd-tools github, so I
did not list them here.
>
>>   6. Access share from Windows or Linux using CIFS
>
> Pointer to cifs.ko how-to page?
Do you know of a page link for cifs.ko?
>
> Basically, I'm encouraging these pages to be (much) more user
> friendly! They're fine for developers, but way too fiddly IMO
> for naive users, or even for admins. It has taken me days to get
> this all going on my fresh machines.
okay..
>
> Either way, thanks for the cleanup so far!!
Thanks!
>
> Tom.
>

Re: [PATCH 1/2] ksmbd: update documentation

[atheik]

On 8/30/2022 12:33 PM, Tom Talpey wrote:
>On 8/30/2022 10:17 AM, Namjae Jeon wrote:
>> configuration.txt in ksmbd-tools moved to smb.conf(5ksmbd) manpage.
>> update it and more detailed ksmbd-tools build method.
>> 
>> Cc: Tom Talpey <tom@talpey.com>
>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
>> ---
>>   Documentation/filesystems/cifs/ksmbd.rst | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>> 
>> diff --git a/Documentation/filesystems/cifs/ksmbd.rst b/Documentation/filesystems/cifs/ksmbd.rst
>> index 1af600db2e70..767e12d2045a 100644
>> --- a/Documentation/filesystems/cifs/ksmbd.rst
>> +++ b/Documentation/filesystems/cifs/ksmbd.rst
>> @@ -121,20 +121,26 @@ How to run
>>   1. Download ksmbd-tools and compile them.
>>   	- https://github.com/cifsd-team/ksmbd-tools
>>   
>> +        # ./autogen.sh
>> +        # ./configure --sysconfdir=/etc --with-rundir=/run
>> +        # make & sudo make install
>
>I believe you mean "make && sudo make install"? The single & will
>kick off two make's in parallel.

Also, Jeon, since `#' indicates root prompt, there shouldn't be `sudo'.
If you want to use `sudo' in the instructions, then use `$' prompt.
With that `./configure' invocation, the utilities end up in
`/usr/local/sbin'. Since `sysconfdir=/etc' is associated with the
`/usr' prefix, it might be better to put the utilities in there also.
Meaning that `--prefix=/usr' should be given to `./configure', so
`sbindir' becomes `/usr/sbin'. Alternatively, `sysconfdir=/etc'
should be removed so that `/usr/local/etc' is used. This way the
user-built ksmbd-tools doesn't conflict with ksmbd-tools installed
using the package manager.

>
>>   2. Create user/password for SMB share.
>>   
>>   	# mkdir /etc/ksmbd/

Jeon, the install target already creates `/etc/ksmbd'.

>>   	# ksmbd.adduser -a <Enter USERNAME for SMB share access>
>
>It may be worth mentioning that it's not just single-user access, and
>that additional users can be configured.
>
>>   3. Create /etc/ksmbd/smb.conf file, add SMB share in smb.conf file
>> -	- Refer smb.conf.example and
>> -          https://github.com/cifsd-team/ksmbd-tools/blob/master/Documentation/configuration.txt
>> +	- Refer smb.conf.example, See smb.conf(5ksmbd) for details.
>> +
>> +        # man smb.conf.5ksmbd
>
>I like the new manpage, but that's a strange path. Are you sure
>the various maintainers will deploy it that way?

Jeon, it should be `man 5ksmbd smb.conf'. `5' is the section number and
`ksmbd' further differentiates it from Samba's `smb.conf'. Currently,
just `5k' is enough to do so.

>
>Also, it has always bothered me that the name "smb.conf" is the
>same as the Samba server's configuration file, just in a different
>directory. If someone enters "man smb.conf", there may be confusion.
>I really wish the file was called "ksmbd.conf".

Talpey, in what way is it strange? If both Samba and ksmbd-tools are
installed, `man 5 smb.conf' is the Samba page. Then
`man 5ksmbd smb.conf' (or just `man 5k smb.conf') is the ksmbd-tools
page. If only ksmbd-tools is installed, `man 5 smb.conf' is the
ksmbd-tools page. The same applies for just `man smb.conf' since there
is only a page with that name in section 5. This makes sense since
Samba's `smb.conf' is authoritative and ksmbd-tools tries to be
compatible.

>
>Why not putting this under a simpler manpage title "man ksmbd"?
>To me, that's much more logical and it avoids both the confusion
>and having to somehow know that weird manpath.
>
>>   4. Insert ksmbd.ko module
>>   
>>   	# insmod ksmbd.ko
>
>Well, it's worth mentioning that a properly configured and built
>kernel is a prerequisite here...
>
>Also, sudo.
>
>>   5. Start ksmbd user space daemon
>> +
>>   	# ksmbd.mountd
>
>FYI, Ubuntu Jammy pre-configures ksmbd as a service, and there it's
>as simple as "sudo service ksmbd start".
>
>Do you not want to mention the other ksmbd.<foo> helpers here?
>
>>   6. Access share from Windows or Linux using CIFS
>
>Pointer to cifs.ko how-to page?
>
>Basically, I'm encouraging these pages to be (much) more user
>friendly! They're fine for developers, but way too fiddly IMO
>for naive users, or even for admins. It has taken me days to get
>this all going on my fresh machines.
>
>Either way, thanks for the cleanup so far!!
>
>Tom.
>

Atte Heikkilä

Re: [PATCH 2/2] ksmbd: remove generic_fillattr use in smb2_open()

[Hyunchul Lee]

2022년 8월 30일 (화) 오후 11:17, Namjae Jeon <linkinjeon@kernel.org>님이 작성:
>
> Removed the use of unneeded generic_fillattr() in smb2_open().
>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>

Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>

> ---
>  fs/ksmbd/smb2pdu.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> index c49f65146ab3..ad6410874b95 100644
> --- a/fs/ksmbd/smb2pdu.c
> +++ b/fs/ksmbd/smb2pdu.c
> @@ -2761,7 +2761,6 @@ int smb2_open(struct ksmbd_work *work)
>         } else {
>                 file_present = true;
>                 user_ns = mnt_user_ns(path.mnt);
> -               generic_fillattr(user_ns, d_inode(path.dentry), &stat);
>         }
>         if (stream_name) {
>                 if (req->CreateOptions & FILE_DIRECTORY_FILE_LE) {
> @@ -2770,7 +2769,8 @@ int smb2_open(struct ksmbd_work *work)
>                                 rsp->hdr.Status = STATUS_NOT_A_DIRECTORY;
>                         }
>                 } else {
> -                       if (S_ISDIR(stat.mode) && s_type == DATA_STREAM) {
> +                       if (file_present && S_ISDIR(d_inode(path.dentry)->i_mode) &&
> +                           s_type == DATA_STREAM) {
>                                 rc = -EIO;
>                                 rsp->hdr.Status = STATUS_FILE_IS_A_DIRECTORY;
>                         }
> @@ -2787,7 +2787,8 @@ int smb2_open(struct ksmbd_work *work)
>         }
>
>         if (file_present && req->CreateOptions & FILE_NON_DIRECTORY_FILE_LE &&
> -           S_ISDIR(stat.mode) && !(req->CreateOptions & FILE_DELETE_ON_CLOSE_LE)) {
> +           S_ISDIR(d_inode(path.dentry)->i_mode) &&
> +           !(req->CreateOptions & FILE_DELETE_ON_CLOSE_LE)) {
>                 ksmbd_debug(SMB, "open() argument is a directory: %s, %x\n",
>                             name, req->CreateOptions);
>                 rsp->hdr.Status = STATUS_FILE_IS_A_DIRECTORY;
> @@ -2797,7 +2798,7 @@ int smb2_open(struct ksmbd_work *work)
>
>         if (file_present && (req->CreateOptions & FILE_DIRECTORY_FILE_LE) &&
>             !(req->CreateDisposition == FILE_CREATE_LE) &&
> -           !S_ISDIR(stat.mode)) {
> +           !S_ISDIR(d_inode(path.dentry)->i_mode)) {
>                 rsp->hdr.Status = STATUS_NOT_A_DIRECTORY;
>                 rc = -EIO;
>                 goto err_out;
> --
> 2.25.1
>


-- 
Thanks,
Hyunchul

Re: [PATCH 1/2] ksmbd: update documentation

[Tom Talpey]

On 9/1/2022 1:19 AM, atheik wrote:
> On 8/30/2022 12:33 PM, Tom Talpey wrote:
>> On 8/30/2022 10:17 AM, Namjae Jeon wrote:
>>> configuration.txt in ksmbd-tools moved to smb.conf(5ksmbd) manpage.
>>> update it and more detailed ksmbd-tools build method.
>>>
>>> Cc: Tom Talpey <tom@talpey.com>
>>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
>>> ---
>>>    Documentation/filesystems/cifs/ksmbd.rst | 10 ++++++++--
>>>    1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/filesystems/cifs/ksmbd.rst b/Documentation/filesystems/cifs/ksmbd.rst
>>> index 1af600db2e70..767e12d2045a 100644
>>> --- a/Documentation/filesystems/cifs/ksmbd.rst
>>> +++ b/Documentation/filesystems/cifs/ksmbd.rst
>>> @@ -121,20 +121,26 @@ How to run
>>>    1. Download ksmbd-tools and compile them.
>>>    	- https://github.com/cifsd-team/ksmbd-tools
>>>    
>>> +        # ./autogen.sh
>>> +        # ./configure --sysconfdir=/etc --with-rundir=/run
>>> +        # make & sudo make install
>>
>> I believe you mean "make && sudo make install"? The single & will
>> kick off two make's in parallel.
> 
> Also, Jeon, since `#' indicates root prompt, there shouldn't be `sudo'.
> If you want to use `sudo' in the instructions, then use `$' prompt.
> With that `./configure' invocation, the utilities end up in
> `/usr/local/sbin'. Since `sysconfdir=/etc' is associated with the
> `/usr' prefix, it might be better to put the utilities in there also.
> Meaning that `--prefix=/usr' should be given to `./configure', so
> `sbindir' becomes `/usr/sbin'. Alternatively, `sysconfdir=/etc'
> should be removed so that `/usr/local/etc' is used. This way the
> user-built ksmbd-tools doesn't conflict with ksmbd-tools installed
> using the package manager.
> 
>>
>>>    2. Create user/password for SMB share.
>>>    
>>>    	# mkdir /etc/ksmbd/
> 
> Jeon, the install target already creates `/etc/ksmbd'.
> 
>>>    	# ksmbd.adduser -a <Enter USERNAME for SMB share access>
>>
>> It may be worth mentioning that it's not just single-user access, and
>> that additional users can be configured.
>>
>>>    3. Create /etc/ksmbd/smb.conf file, add SMB share in smb.conf file
>>> -	- Refer smb.conf.example and
>>> -          https://github.com/cifsd-team/ksmbd-tools/blob/master/Documentation/configuration.txt
>>> +	- Refer smb.conf.example, See smb.conf(5ksmbd) for details.
>>> +
>>> +        # man smb.conf.5ksmbd
>>
>> I like the new manpage, but that's a strange path. Are you sure
>> the various maintainers will deploy it that way?
> 
> Jeon, it should be `man 5ksmbd smb.conf'. `5' is the section number and
> `ksmbd' further differentiates it from Samba's `smb.conf'. Currently,
> just `5k' is enough to do so.
> 
>>
>> Also, it has always bothered me that the name "smb.conf" is the
>> same as the Samba server's configuration file, just in a different
>> directory. If someone enters "man smb.conf", there may be confusion.
>> I really wish the file was called "ksmbd.conf".
> 
> Talpey, in what way is it strange? If both Samba and ksmbd-tools are
> installed, `man 5 smb.conf' is the Samba page. Then
> `man 5ksmbd smb.conf' (or just `man 5k smb.conf') is the ksmbd-tools
> page. If only ksmbd-tools is installed, `man 5 smb.conf' is the
> ksmbd-tools page. The same applies for just `man smb.conf' since there
> is only a page with that name in section 5. This makes sense since
> Samba's `smb.conf' is authoritative and ksmbd-tools tries to be
> compatible.

Ok, two things. What I found strange is the "man smb.conf.5ksmbd", and
as you say that should be man 5k smb.conf. Sounds ok to me.

But the second thing I'm concerned about is the reuse of the smb.conf
filename. It's perfectly possible to install both Samba and ksmbd on
a system, they can be configured to use different ports, listen on
different interfaces, or any number of other deployment approaches.

And, Samba provides MUCH more than an SMB server, and configures many
other services in smb.conf. So I feel ksmbd should avoid such filename
conflicts. To me, calling it "ksmbd.conf" is much more logical.

Tom.


>> Why not putting this under a simpler manpage title "man ksmbd"?
>> To me, that's much more logical and it avoids both the confusion
>> and having to somehow know that weird manpath.
>>
>>>    4. Insert ksmbd.ko module
>>>    
>>>    	# insmod ksmbd.ko
>>
>> Well, it's worth mentioning that a properly configured and built
>> kernel is a prerequisite here...
>>
>> Also, sudo.
>>
>>>    5. Start ksmbd user space daemon
>>> +
>>>    	# ksmbd.mountd
>>
>> FYI, Ubuntu Jammy pre-configures ksmbd as a service, and there it's
>> as simple as "sudo service ksmbd start".
>>
>> Do you not want to mention the other ksmbd.<foo> helpers here?
>>
>>>    6. Access share from Windows or Linux using CIFS
>>
>> Pointer to cifs.ko how-to page?
>>
>> Basically, I'm encouraging these pages to be (much) more user
>> friendly! They're fine for developers, but way too fiddly IMO
>> for naive users, or even for admins. It has taken me days to get
>> this all going on my fresh machines.
>>
>> Either way, thanks for the cleanup so far!!
>>
>> Tom.
>>
> 
> Atte Heikkilä
> 

Re: [PATCH 1/2] ksmbd: update documentation

[Jeremy Allison]

On Thu, Sep 01, 2022 at 09:06:07AM -0400, Tom Talpey wrote:
>
>Ok, two things. What I found strange is the "man smb.conf.5ksmbd", and
>as you say that should be man 5k smb.conf. Sounds ok to me.
>
>But the second thing I'm concerned about is the reuse of the smb.conf
>filename. It's perfectly possible to install both Samba and ksmbd on
>a system, they can be configured to use different ports, listen on
>different interfaces, or any number of other deployment approaches.
>
>And, Samba provides MUCH more than an SMB server, and configures many
>other services in smb.conf. So I feel ksmbd should avoid such filename
>conflicts. To me, calling it "ksmbd.conf" is much more logical.

+1 from me. Having 2 conflicting file contents both wanting
to be called smb.conf is a disaster waiting to happen.

Please use ksmbd.conf.

Re: [PATCH 1/2] ksmbd: update documentation

[atheik]

On Thu, 1 Sep 2022 09:14:31 -0700, Jeremy Allison wrote:
>On Thu, Sep 01, 2022 at 09:06:07AM -0400, Tom Talpey wrote:
>>
>>Ok, two things. What I found strange is the "man smb.conf.5ksmbd", and
>>as you say that should be man 5k smb.conf. Sounds ok to me.
>>
>>But the second thing I'm concerned about is the reuse of the smb.conf
>>filename. It's perfectly possible to install both Samba and ksmbd on
>>a system, they can be configured to use different ports, listen on
>>different interfaces, or any number of other deployment approaches.
>>
>>And, Samba provides MUCH more than an SMB server, and configures many
>>other services in smb.conf. So I feel ksmbd should avoid such filename
>>conflicts. To me, calling it "ksmbd.conf" is much more logical.
>
>+1 from me. Having 2 conflicting file contents both wanting
>to be called smb.conf is a disaster waiting to happen.

ksmbd-tools clearly has a goal of being compatible with smb.conf(5) of
Samba when it comes to the common subset of functionality they share.
ksmbd-tools has 7 global parameters that Samba does not have, but other
than, share parameters and global parameters of ksmbd-tools are subset
of those in Samba. Samba and ksmbd-tools do not have any conflicting
file locations. The smb.conf(5ksmbd) man page of ksmbd-tools does not
collide with and never overshadows smb.conf(5) of Samba. Please, help
me understand what sort of disaster this could lead to.

>
>Please use ksmbd.conf.

Atte Heikkilä

Re: [PATCH 1/2] ksmbd: update documentation

[Jeremy Allison]

On Thu, Sep 01, 2022 at 08:41:08PM +0300, atheik wrote:
>On Thu, 1 Sep 2022 09:14:31 -0700, Jeremy Allison wrote:
>>On Thu, Sep 01, 2022 at 09:06:07AM -0400, Tom Talpey wrote:
>>>
>>>Ok, two things. What I found strange is the "man smb.conf.5ksmbd", and
>>>as you say that should be man 5k smb.conf. Sounds ok to me.
>>>
>>>But the second thing I'm concerned about is the reuse of the smb.conf
>>>filename. It's perfectly possible to install both Samba and ksmbd on
>>>a system, they can be configured to use different ports, listen on
>>>different interfaces, or any number of other deployment approaches.
>>>
>>>And, Samba provides MUCH more than an SMB server, and configures many
>>>other services in smb.conf. So I feel ksmbd should avoid such filename
>>>conflicts. To me, calling it "ksmbd.conf" is much more logical.
>>
>>+1 from me. Having 2 conflicting file contents both wanting
>>to be called smb.conf is a disaster waiting to happen.
>
>ksmbd-tools clearly has a goal of being compatible with smb.conf(5) of
>Samba when it comes to the common subset of functionality they share.
>ksmbd-tools has 7 global parameters that Samba does not have, but other
>than, share parameters and global parameters of ksmbd-tools are subset
>of those in Samba. Samba and ksmbd-tools do not have any conflicting
>file locations. The smb.conf(5ksmbd) man page of ksmbd-tools does not
>collide with and never overshadows smb.conf(5) of Samba. Please, help
>me understand what sort of disaster this could lead to.

Samba adds and or changes functionality in smb.conf all
the time, without coordination with ksmbd. If you call
your config file smb.conf then we would have to coordinate
with you before any changes.

Over time, the meaning/use/names of parameters will drift
apart leading to possible conflicts.

Plus it leads to massive user confusion (am I running
smbd or ksmbd ? How do I tell ? etc.).

It is simple hygene to keep these names separate.

Please do so.

Re: [PATCH 1/2] ksmbd: update documentation

[Tom Talpey]

On 9/1/2022 2:30 PM, Jeremy Allison wrote:
> On Thu, Sep 01, 2022 at 08:41:08PM +0300, atheik wrote:
>> On Thu, 1 Sep 2022 09:14:31 -0700, Jeremy Allison wrote:
>>> On Thu, Sep 01, 2022 at 09:06:07AM -0400, Tom Talpey wrote:
>>>>
>>>> Ok, two things. What I found strange is the "man smb.conf.5ksmbd", and
>>>> as you say that should be man 5k smb.conf. Sounds ok to me.
>>>>
>>>> But the second thing I'm concerned about is the reuse of the smb.conf
>>>> filename. It's perfectly possible to install both Samba and ksmbd on
>>>> a system, they can be configured to use different ports, listen on
>>>> different interfaces, or any number of other deployment approaches.
>>>>
>>>> And, Samba provides MUCH more than an SMB server, and configures many
>>>> other services in smb.conf. So I feel ksmbd should avoid such filename
>>>> conflicts. To me, calling it "ksmbd.conf" is much more logical.
>>>
>>> +1 from me. Having 2 conflicting file contents both wanting
>>> to be called smb.conf is a disaster waiting to happen.
>>
>> ksmbd-tools clearly has a goal of being compatible with smb.conf(5) of
>> Samba when it comes to the common subset of functionality they share.
>> ksmbd-tools has 7 global parameters that Samba does not have, but other
>> than, share parameters and global parameters of ksmbd-tools are subset
>> of those in Samba. Samba and ksmbd-tools do not have any conflicting
>> file locations. The smb.conf(5ksmbd) man page of ksmbd-tools does not
>> collide with and never overshadows smb.conf(5) of Samba. Please, help
>> me understand what sort of disaster this could lead to.
> 
> Samba adds and or changes functionality in smb.conf all
> the time, without coordination with ksmbd. If you call
> your config file smb.conf then we would have to coordinate
> with you before any changes.

And vice-versa. For example, ksmbd supports RDMA and can be
configured to use interfaces with kernel-internal names,
for example "enp2s0" or "mlx5/1". These files do not in fact
subset one another, in either direction.

> Over time, the meaning/use/names of parameters will drift
> apart leading to possible conflicts.

Personally I think they're already in conflict, having taken
several days to work them all out wile setting up my new
machines. And, um, I think I know what I'm doing. Heaven
help the newbie.

> Plus it leads to massive user confusion (am I running
> smbd or ksmbd ? How do I tell ? etc.).

+1

Tom.

> It is simple hygene to keep these names separate.
> 
> Please do so.
> 

Re: [PATCH 1/2] ksmbd: update documentation

[atheik]

On Thu, 1 Sep 2022 11:30:32 -0700, Jeremy Allison wrote:
>
>Samba adds and or changes functionality in smb.conf all
>the time, without coordination with ksmbd. If you call
>your config file smb.conf then we would have to coordinate
>with you before any changes.

ksmbd-tools has always called its config file `smb.conf'. Has this
matter come up before? I don't have anything against changing the name,
but I'd be surprised if my writing of the man pages first prompted this
concern.

>
>Over time, the meaning/use/names of parameters will drift
>apart leading to possible conflicts.
>
>Plus it leads to massive user confusion (am I running
>smbd or ksmbd ? How do I tell ? etc.).
>
>It is simple hygene to keep these names separate.
>
>Please do so.

Atte Heikkilä

Re: [PATCH 1/2] ksmbd: update documentation

[Jeremy Allison]

On Thu, Sep 01, 2022 at 10:42:12PM +0300, atheik wrote:
>On Thu, 1 Sep 2022 11:30:32 -0700, Jeremy Allison wrote:
>>
>>Samba adds and or changes functionality in smb.conf all
>>the time, without coordination with ksmbd. If you call
>>your config file smb.conf then we would have to coordinate
>>with you before any changes.
>
>ksmbd-tools has always called its config file `smb.conf'. Has this
>matter come up before? I don't have anything against changing the name,
>but I'd be surprised if my writing of the man pages first prompted this
>concern.

Well I didn't realize it used the same name until
you wrote the man page :-).

Re: [PATCH 1/2] ksmbd: update documentation

[ronnie sahlberg]

On Fri, 2 Sept 2022 at 04:34, Jeremy Allison <jra@samba.org> wrote:
>
> On Thu, Sep 01, 2022 at 08:41:08PM +0300, atheik wrote:
> >On Thu, 1 Sep 2022 09:14:31 -0700, Jeremy Allison wrote:
> >>On Thu, Sep 01, 2022 at 09:06:07AM -0400, Tom Talpey wrote:
> >>>
> >>>Ok, two things. What I found strange is the "man smb.conf.5ksmbd", and
> >>>as you say that should be man 5k smb.conf. Sounds ok to me.
> >>>
> >>>But the second thing I'm concerned about is the reuse of the smb.conf
> >>>filename. It's perfectly possible to install both Samba and ksmbd on
> >>>a system, they can be configured to use different ports, listen on
> >>>different interfaces, or any number of other deployment approaches.
> >>>
> >>>And, Samba provides MUCH more than an SMB server, and configures many
> >>>other services in smb.conf. So I feel ksmbd should avoid such filename
> >>>conflicts. To me, calling it "ksmbd.conf" is much more logical.
> >>
> >>+1 from me. Having 2 conflicting file contents both wanting
> >>to be called smb.conf is a disaster waiting to happen.
> >
> >ksmbd-tools clearly has a goal of being compatible with smb.conf(5) of
> >Samba when it comes to the common subset of functionality they share.
> >ksmbd-tools has 7 global parameters that Samba does not have, but other
> >than, share parameters and global parameters of ksmbd-tools are subset
> >of those in Samba. Samba and ksmbd-tools do not have any conflicting
> >file locations. The smb.conf(5ksmbd) man page of ksmbd-tools does not
> >collide with and never overshadows smb.conf(5) of Samba. Please, help
> >me understand what sort of disaster this could lead to.
>
> Samba adds and or changes functionality in smb.conf all
> the time, without coordination with ksmbd. If you call
> your config file smb.conf then we would have to coordinate
> with you before any changes.
>
> Over time, the meaning/use/names of parameters will drift
> apart leading to possible conflicts.
>
> Plus it leads to massive user confusion (am I running
> smbd or ksmbd ? How do I tell ? etc.).

+1
We should avoid having the same name here since it will cause confusion
and it will lead to files with the same name but with similar but also
different context and syntax.

Any tool that automatically scans/operates on smb.conf is potentially
going to have a bad time.


>
> It is simple hygene to keep these names separate.
>
> Please do so.

Re: [PATCH 1/2] ksmbd: update documentation

[Steve French]

On Thu, Sep 1, 2022 at 4:21 PM ronnie sahlberg <ronniesahlberg@gmail.com> wrote:
>
> On Fri, 2 Sept 2022 at 04:34, Jeremy Allison <jra@samba.org> wrote:
> >
> > On Thu, Sep 01, 2022 at 08:41:08PM +0300, atheik wrote:
> > >On Thu, 1 Sep 2022 09:14:31 -0700, Jeremy Allison wrote:
> > >>On Thu, Sep 01, 2022 at 09:06:07AM -0400, Tom Talpey wrote:
> > >>>
> > >>>Ok, two things. What I found strange is the "man smb.conf.5ksmbd", and
> > >>>as you say that should be man 5k smb.conf. Sounds ok to me.
> > >>>
> > >>>But the second thing I'm concerned about is the reuse of the smb.conf
> > >>>filename. It's perfectly possible to install both Samba and ksmbd on
> > >>>a system, they can be configured to use different ports, listen on
> > >>>different interfaces, or any number of other deployment approaches.
> > >>>
> > >>>And, Samba provides MUCH more than an SMB server, and configures many
> > >>>other services in smb.conf. So I feel ksmbd should avoid such filename
> > >>>conflicts. To me, calling it "ksmbd.conf" is much more logical.
> > >>
> > >>+1 from me. Having 2 conflicting file contents both wanting
> > >>to be called smb.conf is a disaster waiting to happen.
> > >
> > >ksmbd-tools clearly has a goal of being compatible with smb.conf(5) of
> > >Samba when it comes to the common subset of functionality they share.
> > >ksmbd-tools has 7 global parameters that Samba does not have, but other
> > >than, share parameters and global parameters of ksmbd-tools are subset
> > >of those in Samba. Samba and ksmbd-tools do not have any conflicting
> > >file locations. The smb.conf(5ksmbd) man page of ksmbd-tools does not
> > >collide with and never overshadows smb.conf(5) of Samba. Please, help
> > >me understand what sort of disaster this could lead to.
> >
> > Samba adds and or changes functionality in smb.conf all
> > the time, without coordination with ksmbd. If you call
> > your config file smb.conf then we would have to coordinate
> > with you before any changes.
> >
> > Over time, the meaning/use/names of parameters will drift
> > apart leading to possible conflicts.

I do like that Namjae et al made the format of the .conf file very similar
to the format of Samba's smb.conf file though.   I realize there some
users complain that there are too many smb.conf parameters for Samba,
but he seemed to pick a reasonably subset of them for ksmbd.
Samba is a much larger project with many more smb.conf parameters
but it does reduce confusion making the parameter names similar
where possible e.g. "workgroup", "guest account", (share) path, read only, etc.
and fortunately the default directories for the two smb.conf files are
different so at least the daemons don't use the same file.
-- 
Thanks,

Steve

Re: [PATCH 1/2] ksmbd: update documentation

[Jeremy Allison]

On Thu, Sep 01, 2022 at 04:37:21PM -0500, Steve French wrote:
>
>I do like that Namjae et al made the format of the .conf file very similar
>to the format of Samba's smb.conf file though.   I realize there some
>users complain that there are too many smb.conf parameters for Samba,
>but he seemed to pick a reasonably subset of them for ksmbd.
>Samba is a much larger project with many more smb.conf parameters
>but it does reduce confusion making the parameter names similar
>where possible e.g. "workgroup", "guest account", (share) path, read only, etc.
>and fortunately the default directories for the two smb.conf files are
>different so at least the daemons don't use the same file.

Sure, I think the formats and parameter names being as close
as possible is a great idea to allow users to move between
servers as they wish. But making the files the same name
is not a good idea.

Re: [PATCH 1/2] ksmbd: update documentation

[Steve French]

I do think that one obvious thing that is missing is a simple python
script or slightly more complex GUI tool that would allow better
autoconfiguring a share for ksmbd without having to understand the
ksmbd.conf/smb.conf format (and a different tool for Samba - although
to be fair for Samba various vendors and some distros have tools to do
this), but in the short term, a few more example smb.conf/ksmbd.conf
files might help (maybe in the wiki?)

On Thu, Sep 1, 2022 at 1:52 PM Tom Talpey <tom@talpey.com> wrote:
>
> On 9/1/2022 2:30 PM, Jeremy Allison wrote:
> > On Thu, Sep 01, 2022 at 08:41:08PM +0300, atheik wrote:
> >> On Thu, 1 Sep 2022 09:14:31 -0700, Jeremy Allison wrote:
> >>> On Thu, Sep 01, 2022 at 09:06:07AM -0400, Tom Talpey wrote:
> >>>>
> >>>> Ok, two things. What I found strange is the "man smb.conf.5ksmbd", and
> >>>> as you say that should be man 5k smb.conf. Sounds ok to me.
> >>>>
> >>>> But the second thing I'm concerned about is the reuse of the smb.conf
> >>>> filename. It's perfectly possible to install both Samba and ksmbd on
> >>>> a system, they can be configured to use different ports, listen on
> >>>> different interfaces, or any number of other deployment approaches.
> >>>>
> >>>> And, Samba provides MUCH more than an SMB server, and configures many
> >>>> other services in smb.conf. So I feel ksmbd should avoid such filename
> >>>> conflicts. To me, calling it "ksmbd.conf" is much more logical.
> >>>
> >>> +1 from me. Having 2 conflicting file contents both wanting
> >>> to be called smb.conf is a disaster waiting to happen.
> >>
> >> ksmbd-tools clearly has a goal of being compatible with smb.conf(5) of
> >> Samba when it comes to the common subset of functionality they share.
> >> ksmbd-tools has 7 global parameters that Samba does not have, but other
> >> than, share parameters and global parameters of ksmbd-tools are subset
> >> of those in Samba. Samba and ksmbd-tools do not have any conflicting
> >> file locations. The smb.conf(5ksmbd) man page of ksmbd-tools does not
> >> collide with and never overshadows smb.conf(5) of Samba. Please, help
> >> me understand what sort of disaster this could lead to.
> >
> > Samba adds and or changes functionality in smb.conf all
> > the time, without coordination with ksmbd. If you call
> > your config file smb.conf then we would have to coordinate
> > with you before any changes.
>
> And vice-versa. For example, ksmbd supports RDMA and can be
> configured to use interfaces with kernel-internal names,
> for example "enp2s0" or "mlx5/1". These files do not in fact
> subset one another, in either direction.
>
> > Over time, the meaning/use/names of parameters will drift
> > apart leading to possible conflicts.
>
> Personally I think they're already in conflict, having taken
> several days to work them all out wile setting up my new
> machines. And, um, I think I know what I'm doing. Heaven
> help the newbie.
>
> > Plus it leads to massive user confusion (am I running
> > smbd or ksmbd ? How do I tell ? etc.).
>
> +1
>
> Tom.
>
> > It is simple hygene to keep these names separate.
> >
> > Please do so.
> >



-- 
Thanks,

Steve

Re: [PATCH 1/2] ksmbd: update documentation

[Tom Talpey]

I've got a couple, for example I cooked an RDMA-only one which I'm using for testing that ignores regular adapters and therefore can coexist with Samba. Only takes a few lines. Now, if the Ubuntu package manager wouldn't prevent me from installing both Samba and ksmbd-tools, it would be easier to deploy. :(


Sep 1, 2022 6:26:17 PM Steve French <smfrench@gmail.com>:

> I do think that one obvious thing that is missing is a simple python
> script or slightly more complex GUI tool that would allow better
> autoconfiguring a share for ksmbd without having to understand the
> ksmbd.conf/smb.conf format (and a different tool for Samba - although
> to be fair for Samba various vendors and some distros have tools to do
> this), but in the short term, a few more example smb.conf/ksmbd.conf
> files might help (maybe in the wiki?)
> 
> On Thu, Sep 1, 2022 at 1:52 PM Tom Talpey <tom@talpey.com> wrote:
>> 
>> On 9/1/2022 2:30 PM, Jeremy Allison wrote:
>>> On Thu, Sep 01, 2022 at 08:41:08PM +0300, atheik wrote:
>>>> On Thu, 1 Sep 2022 09:14:31 -0700, Jeremy Allison wrote:
>>>>> On Thu, Sep 01, 2022 at 09:06:07AM -0400, Tom Talpey wrote:
>>>>>> …
>>>>> 
>>>>> +1 from me. Having 2 conflicting file contents both wanting
>>>>> to be called smb.conf is a disaster waiting to happen.
>>>> 
>>>> ksmbd-tools clearly has a goal of being compatible with smb.conf(5) of
>>>> Samba when it comes to the common subset of functionality they share.
>>>> ksmbd-tools has 7 global parameters that Samba does not have, but other
>>>> than, share parameters and global parameters of ksmbd-tools are subset
>>>> of those in Samba. Samba and ksmbd-tools do not have any conflicting
>>>> file locations. The smb.conf(5ksmbd) man page of ksmbd-tools does not
>>>> collide with and never overshadows smb.conf(5) of Samba. Please, help
>>>> me understand what sort of disaster this could lead to.
>>> 
>>> Samba adds and or changes functionality in smb.conf all
>>> the time, without coordination with ksmbd. If you call
>>> your config file smb.conf then we would have to coordinate
>>> with you before any changes.
>> 
>> And vice-versa. For example, ksmbd supports RDMA and can be
>> configured to use interfaces with kernel-internal names,
>> for example "enp2s0" or "mlx5/1". These files do not in fact
>> subset one another, in either direction.
>> 
>>> Over time, the meaning/use/names of parameters will drift
>>> apart leading to possible conflicts.
>> 
>> Personally I think they're already in conflict, having taken
>> several days to work them all out wile setting up my new
>> machines. And, um, I think I know what I'm doing. Heaven
>> help the newbie.
>> 
>>> Plus it leads to massive user confusion (am I running
>>> smbd or ksmbd ? How do I tell ? etc.).
>> 
>> +1
>> 
>> Tom.
>> 
>>> It is simple hygene to keep these names separate.
>>> 
>>> Please do so.
>>> 
> 
> 
> 
> -- 
> Thanks,
> 
> Steve

Re: [PATCH 1/2] ksmbd: update documentation

[Namjae Jeon]

2022-09-02 6:48 GMT+09:00, Jeremy Allison <jra@samba.org>:
> On Thu, Sep 01, 2022 at 04:37:21PM -0500, Steve French wrote:
>>
>>I do like that Namjae et al made the format of the .conf file very similar
>>to the format of Samba's smb.conf file though.   I realize there some
>>users complain that there are too many smb.conf parameters for Samba,
>>but he seemed to pick a reasonably subset of them for ksmbd.
>>Samba is a much larger project with many more smb.conf parameters
>>but it does reduce confusion making the parameter names similar
>>where possible e.g. "workgroup", "guest account", (share) path, read only,
>> etc.
>>and fortunately the default directories for the two smb.conf files are
>>different so at least the daemons don't use the same file.
>
> Sure, I think the formats and parameter names being as close
> as possible is a great idea to allow users to move between
> servers as they wish. But making the files the same name
> is not a good idea.
This is the first time I've heard that it is problem that ksmbd's
config filename is same with samba's one. Reading the mail threads, I
don't understand exactly what the real problem is... I thought that
using same smb.conf name make users aware that it was forked from
samba's one. I'm a little surprised, I thought that you will say that
we should use the same name. This seems to contradict your previous
opinion that ksmbd's dos attribute and stream xattr format should be
the same with samba's one. It's not difficult to change config
filename of ksmbd if you agree with it. If we think about adding the
ksmbd integration feature in samba recently, I think it would be
better to use the same name for future.

Re: [PATCH 1/2] ksmbd: update documentation

[Jeremy Allison]

On Fri, Sep 02, 2022 at 09:56:57AM +0900, Namjae Jeon wrote:
>2022-09-02 6:48 GMT+09:00, Jeremy Allison <jra@samba.org>:
>> On Thu, Sep 01, 2022 at 04:37:21PM -0500, Steve French wrote:
>>>
>>>I do like that Namjae et al made the format of the .conf file very similar
>>>to the format of Samba's smb.conf file though.   I realize there some
>>>users complain that there are too many smb.conf parameters for Samba,
>>>but he seemed to pick a reasonably subset of them for ksmbd.
>>>Samba is a much larger project with many more smb.conf parameters
>>>but it does reduce confusion making the parameter names similar
>>>where possible e.g. "workgroup", "guest account", (share) path, read only,
>>> etc.
>>>and fortunately the default directories for the two smb.conf files are
>>>different so at least the daemons don't use the same file.
>>
>> Sure, I think the formats and parameter names being as close
>> as possible is a great idea to allow users to move between
>> servers as they wish. But making the files the same name
>> is not a good idea.
>This is the first time I've heard that it is problem that ksmbd's
>config filename is same with samba's one. Reading the mail threads, I
>don't understand exactly what the real problem is... I thought that
>using same smb.conf name make users aware that it was forked from
>samba's one. I'm a little surprised, I thought that you will say that
>we should use the same name. This seems to contradict your previous
>opinion that ksmbd's dos attribute and stream xattr format should be
>the same with samba's one. It's not difficult to change config
>filename of ksmbd if you agree with it. If we think about adding the
>ksmbd integration feature in samba recently, I think it would be
>better to use the same name for future.

Well as Tom said, it gets very confusing to have the
same name config file.

The file system attributes etc. certainly should be
the same, to allow a user to swap between the two
servers based on need and not lose any meta-data.

But as the features diverge, trying to keep a common
config will get harder and harder.

Better to take the pain now, rather than try and
postpone for later. It'll hurt *worse* later :-).

Re: [PATCH 1/2] ksmbd: update documentation

[Tom Talpey]

On 9/1/2022 10:11 PM, Jeremy Allison wrote:
> On Fri, Sep 02, 2022 at 09:56:57AM +0900, Namjae Jeon wrote:
>> 2022-09-02 6:48 GMT+09:00, Jeremy Allison <jra@samba.org>:
>>> On Thu, Sep 01, 2022 at 04:37:21PM -0500, Steve French wrote:
>>>>
>>>> I do like that Namjae et al made the format of the .conf file very 
>>>> similar
>>>> to the format of Samba's smb.conf file though.   I realize there some
>>>> users complain that there are too many smb.conf parameters for Samba,
>>>> but he seemed to pick a reasonably subset of them for ksmbd.
>>>> Samba is a much larger project with many more smb.conf parameters
>>>> but it does reduce confusion making the parameter names similar
>>>> where possible e.g. "workgroup", "guest account", (share) path, read 
>>>> only,
>>>> etc.
>>>> and fortunately the default directories for the two smb.conf files are
>>>> different so at least the daemons don't use the same file.
>>>
>>> Sure, I think the formats and parameter names being as close
>>> as possible is a great idea to allow users to move between
>>> servers as they wish. But making the files the same name
>>> is not a good idea.
>> This is the first time I've heard that it is problem that ksmbd's
>> config filename is same with samba's one. Reading the mail threads, I
>> don't understand exactly what the real problem is... I thought that
>> using same smb.conf name make users aware that it was forked from
>> samba's one. I'm a little surprised, I thought that you will say that
>> we should use the same name. This seems to contradict your previous
>> opinion that ksmbd's dos attribute and stream xattr format should be
>> the same with samba's one. It's not difficult to change config
>> filename of ksmbd if you agree with it. If we think about adding the
>> ksmbd integration feature in samba recently, I think it would be
>> better to use the same name for future.
> 
> Well as Tom said, it gets very confusing to have the
> same name config file.
> 
> The file system attributes etc. certainly should be
> the same, to allow a user to swap between the two
> servers based on need and not lose any meta-data.
> 
> But as the features diverge, trying to keep a common
> config will get harder and harder.
> 
> Better to take the pain now, rather than try and
> postpone for later. It'll hurt *worse* later :-).

The name collision was always a minor thing for me, but when it
goes front and center in the manpage namespace, potentially
coming up for a user when they expected Samba's, it is working
cross-purposes. Ksmbd should forge its own identity, for clarity
and for the future.

Namjae, you should be excited that this kind of issue comes up.
As interest in ksmbd rises, more eyes will be on it, and more
feedback will come. Remember it's experimental, it is not yet
frozen in stone by being adopted by large numbers of users.
Now is the time to work out the usability.

Tom.
Let's also pop up a level

Re: [PATCH 1/2] ksmbd: update documentation

[Namjae Jeon]

2022-09-02 21:35 GMT+09:00, Tom Talpey <tom@talpey.com>:
> On 9/1/2022 10:11 PM, Jeremy Allison wrote:
>> On Fri, Sep 02, 2022 at 09:56:57AM +0900, Namjae Jeon wrote:
>>> 2022-09-02 6:48 GMT+09:00, Jeremy Allison <jra@samba.org>:
>>>> On Thu, Sep 01, 2022 at 04:37:21PM -0500, Steve French wrote:
>>>>>
>>>>> I do like that Namjae et al made the format of the .conf file very
>>>>> similar
>>>>> to the format of Samba's smb.conf file though.   I realize there some
>>>>> users complain that there are too many smb.conf parameters for Samba,
>>>>> but he seemed to pick a reasonably subset of them for ksmbd.
>>>>> Samba is a much larger project with many more smb.conf parameters
>>>>> but it does reduce confusion making the parameter names similar
>>>>> where possible e.g. "workgroup", "guest account", (share) path, read
>>>>> only,
>>>>> etc.
>>>>> and fortunately the default directories for the two smb.conf files are
>>>>> different so at least the daemons don't use the same file.
>>>>
>>>> Sure, I think the formats and parameter names being as close
>>>> as possible is a great idea to allow users to move between
>>>> servers as they wish. But making the files the same name
>>>> is not a good idea.
>>> This is the first time I've heard that it is problem that ksmbd's
>>> config filename is same with samba's one. Reading the mail threads, I
>>> don't understand exactly what the real problem is... I thought that
>>> using same smb.conf name make users aware that it was forked from
>>> samba's one. I'm a little surprised, I thought that you will say that
>>> we should use the same name. This seems to contradict your previous
>>> opinion that ksmbd's dos attribute and stream xattr format should be
>>> the same with samba's one. It's not difficult to change config
>>> filename of ksmbd if you agree with it. If we think about adding the
>>> ksmbd integration feature in samba recently, I think it would be
>>> better to use the same name for future.
>>
>> Well as Tom said, it gets very confusing to have the
>> same name config file.
>>
>> The file system attributes etc. certainly should be
>> the same, to allow a user to swap between the two
>> servers based on need and not lose any meta-data.
>>
>> But as the features diverge, trying to keep a common
>> config will get harder and harder.
>>
>> Better to take the pain now, rather than try and
>> postpone for later. It'll hurt *worse* later :-).
>
> The name collision was always a minor thing for me, but when it
> goes front and center in the manpage namespace, potentially
> coming up for a user when they expected Samba's, it is working
> cross-purposes. Ksmbd should forge its own identity, for clarity
> and for the future.
Okay, I'm talking to Atte to do that. after changing ksmbd-tools with him,
I will update ksmbd documentation in kernel.
>
> Namjae, you should be excited that this kind of issue comes up.
> As interest in ksmbd rises, more eyes will be on it, and more
> feedback will come. Remember it's experimental, it is not yet
> frozen in stone by being adopted by large numbers of users.
> Now is the time to work out the usability.
Okay. Thank you so much for your good feedbacks as well as
the patch review. Please keep giving feedback like it is now:)
Thanks JRA also.
> Tom.
> Let's also pop up a level
Yup!
>