[PATCH v2] ksmbd: fix incorrect handling of iterate_dir

[PATCH v2] ksmbd: fix incorrect handling of iterate_dir

[Namjae Jeon]

if iterate_dir() returns non-negative value, caller has to treat it
as normal and check there is any error while populating dentry
information. ksmbd doesn't have to do anything because ksmbd already
checks too small OutputBufferLength to store one file information.

And because ctx->pos is set to file->f_pos when iterative_dir is called,
remove restart_ctx(). And if iterate_dir() return -EIO, which mean
directory entry is corrupted, return STATUS_FILE_CORRUPT_ERROR error
response.

This patch fixes some failure of SMB2_QUERY_DIRECTORY, which happens when
ntfs3 is local filesystem.

Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 v2:
   - remove unneeded restart_ctx().
   - If directory entry is corrupted, return STATUS_FILE_CORRUPT_ERROR
     error response.

 fs/ksmbd/smb2pdu.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index ba74aba2f1d3..634e21bba770 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -3809,11 +3809,6 @@ static int __query_dir(struct dir_context *ctx, const char *name, int namlen,
 	return 0;
 }
 
-static void restart_ctx(struct dir_context *ctx)
-{
-	ctx->pos = 0;
-}
-
 static int verify_info_level(int info_level)
 {
 	switch (info_level) {
@@ -3921,7 +3916,6 @@ int smb2_query_dir(struct ksmbd_work *work)
 	if (srch_flag & SMB2_REOPEN || srch_flag & SMB2_RESTART_SCANS) {
 		ksmbd_debug(SMB, "Restart directory scan\n");
 		generic_file_llseek(dir_fp->filp, 0, SEEK_SET);
-		restart_ctx(&dir_fp->readdir_data.ctx);
 	}
 
 	memset(&d_info, 0, sizeof(struct ksmbd_dir_info));
@@ -3968,11 +3962,9 @@ int smb2_query_dir(struct ksmbd_work *work)
 	 */
 	if (!d_info.out_buf_len && !d_info.num_entry)
 		goto no_buf_len;
-	if (rc == 0)
-		restart_ctx(&dir_fp->readdir_data.ctx);
-	if (rc == -ENOSPC)
+	if (rc > 0 || rc == -ENOSPC)
 		rc = 0;
-	if (rc)
+	else if (rc)
 		goto err_out;
 
 	d_info.wptr = d_info.rptr;
@@ -4029,6 +4021,8 @@ int smb2_query_dir(struct ksmbd_work *work)
 		rsp->hdr.Status = STATUS_NO_MEMORY;
 	else if (rc == -EFAULT)
 		rsp->hdr.Status = STATUS_INVALID_INFO_CLASS;
+	else if (rc == -EIO)
+		rsp->hdr.Status = STATUS_FILE_CORRUPT_ERROR;
 	if (!rsp->hdr.Status)
 		rsp->hdr.Status = STATUS_UNEXPECTED_IO_ERROR;
 
-- 
2.25.1

[PATCH v3] ksmbd: update documentation

[Namjae Jeon]

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

Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 v3:
   - replace CIFS with SMB3 clients.
   - update ksmbd built-in case.
   - replace smb.conf leftover with ksmbd.conf.
   - use full name of utils in ksmbd-tools instead of <foo>.
   - fix the warnings from make htlmdocs build reported by kernel test
     robot.
 v2:
   - rename smb.conf to ksmbd.conf.
   - add how to set ksmbd module in menuconfig
   - remove --syscondir option for configure, instead change ksmbd
     directory to /usr/local/etc/ksmbd.
   - change the prompt to '$'.

 Documentation/filesystems/cifs/ksmbd.rst | 40 +++++++++++++++++-------
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/Documentation/filesystems/cifs/ksmbd.rst b/Documentation/filesystems/cifs/ksmbd.rst
index 1af600db2e70..4284341c89f3 100644
--- a/Documentation/filesystems/cifs/ksmbd.rst
+++ b/Documentation/filesystems/cifs/ksmbd.rst
@@ -118,26 +118,44 @@ ksmbd/nfsd interoperability    Planned for future. The features that ksmbd
 How to run
 ==========
 
-1. Download ksmbd-tools and compile them.
-	- https://github.com/cifsd-team/ksmbd-tools
+1. Download ksmbd-tools(https://github.com/cifsd-team/ksmbd-tools/releases) and
+   compile them.
+
+   - Refer README(https://github.com/cifsd-team/ksmbd-tools/blob/master/README.md)
+     to know how to use ksmbd.mountd/adduser/addshare/control utils
+
+     $ ./autogen.sh
+     $ ./configure --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>
+   - See ksmbd.adduser manpage.
+
+     $ man ksmbd.adduser
+     $ sudo ksmbd.adduser -a <Enter USERNAME for SMB share access>
+
+3. Create /usr/local/etc/ksmbd/ksmbd.conf file, add SMB share in ksmbd.conf file.
 
-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 ksmbd.conf.example in ksmbd-utils, See ksmbd.conf manpage
+     for details to configure shares.
 
-4. Insert ksmbd.ko module
+        $ man ksmbd.conf
 
-	# insmod ksmbd.ko
+4. Insert ksmbd.ko module after build your kernel. No need to load module
+   if ksmbd is built into the kernel.
+
+   - Set ksmbd in menuconfig(e.g. $ make menuconfig)
+       [*] Network File Systems  --->
+           <M> SMB3 server support (EXPERIMENTAL)
+
+	$ sudo modprobe ksmbd.ko
 
 5. Start ksmbd user space daemon
-	# ksmbd.mountd
 
-6. Access share from Windows or Linux using CIFS
+	$ sudo ksmbd.mountd
+
+6. Access share from Windows or Linux SMB3 clients (cifs.ko or smbclient of samba)
 
 Shutdown KSMBD
 ==============
-- 
2.25.1

Re: [PATCH v3] ksmbd: update documentation

[Tom Talpey]

On 9/9/2022 5:25 AM, Namjae Jeon wrote:
> configuration.txt in ksmbd-tools moved to ksmbd.conf manpage.
> update it and more detailed ksmbd-tools build method.
> 
> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> ---
>   v3:
>     - replace CIFS with SMB3 clients.
>     - update ksmbd built-in case.
>     - replace smb.conf leftover with ksmbd.conf.
>     - use full name of utils in ksmbd-tools instead of <foo>.
>     - fix the warnings from make htlmdocs build reported by kernel test
>       robot.
>   v2:
>     - rename smb.conf to ksmbd.conf.
>     - add how to set ksmbd module in menuconfig
>     - remove --syscondir option for configure, instead change ksmbd
>       directory to /usr/local/etc/ksmbd.
>     - change the prompt to '$'.
> 
>   Documentation/filesystems/cifs/ksmbd.rst | 40 +++++++++++++++++-------
>   1 file changed, 29 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/filesystems/cifs/ksmbd.rst b/Documentation/filesystems/cifs/ksmbd.rst
> index 1af600db2e70..4284341c89f3 100644
> --- a/Documentation/filesystems/cifs/ksmbd.rst
> +++ b/Documentation/filesystems/cifs/ksmbd.rst
> @@ -118,26 +118,44 @@ ksmbd/nfsd interoperability    Planned for future. The features that ksmbd
>   How to run
>   ==========
>   
> -1. Download ksmbd-tools and compile them.
> -	- https://github.com/cifsd-team/ksmbd-tools
> +1. Download ksmbd-tools(https://github.com/cifsd-team/ksmbd-tools/releases) and
> +   compile them.
> +
> +   - Refer README(https://github.com/cifsd-team/ksmbd-tools/blob/master/README.md)
> +     to know how to use ksmbd.mountd/adduser/addshare/control utils
> +
> +     $ ./autogen.sh
> +     $ ./configure --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>
> +   - See ksmbd.adduser manpage.
> +
> +     $ man ksmbd.adduser
> +     $ sudo ksmbd.adduser -a <Enter USERNAME for SMB share access>
> +
> +3. Create /usr/local/etc/ksmbd/ksmbd.conf file, add SMB share in ksmbd.conf file.

I missed this in the v2 match - are you intentionally moving the
ksmbd.conf file to /usr/local/etc? That seems a very mysterious
location. Nothing on my vanilla installed system places anything
in there.

Also, doesn't this file need to exist before step 2??

Tom.


> -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 ksmbd.conf.example in ksmbd-utils, See ksmbd.conf manpage
> +     for details to configure shares.
>   
> -4. Insert ksmbd.ko module
> +        $ man ksmbd.conf
>   
> -	# insmod ksmbd.ko
> +4. Insert ksmbd.ko module after build your kernel. No need to load module
> +   if ksmbd is built into the kernel.
> +
> +   - Set ksmbd in menuconfig(e.g. $ make menuconfig)
> +       [*] Network File Systems  --->
> +           <M> SMB3 server support (EXPERIMENTAL)
> +
> +	$ sudo modprobe ksmbd.ko
>   
>   5. Start ksmbd user space daemon
> -	# ksmbd.mountd
>   
> -6. Access share from Windows or Linux using CIFS
> +	$ sudo ksmbd.mountd
> +
> +6. Access share from Windows or Linux SMB3 clients (cifs.ko or smbclient of samba)
>   
>   Shutdown KSMBD
>   ==============

Re: [PATCH v3] ksmbd: update documentation

[Namjae Jeon]

2022-09-13 8:38 GMT+09:00, Tom Talpey <tom@talpey.com>:
> On 9/9/2022 5:25 AM, Namjae Jeon wrote:
>> configuration.txt in ksmbd-tools moved to ksmbd.conf manpage.
>> update it and more detailed ksmbd-tools build method.
>>
>> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
>> ---
>>   v3:
>>     - replace CIFS with SMB3 clients.
>>     - update ksmbd built-in case.
>>     - replace smb.conf leftover with ksmbd.conf.
>>     - use full name of utils in ksmbd-tools instead of <foo>.
>>     - fix the warnings from make htlmdocs build reported by kernel test
>>       robot.
>>   v2:
>>     - rename smb.conf to ksmbd.conf.
>>     - add how to set ksmbd module in menuconfig
>>     - remove --syscondir option for configure, instead change ksmbd
>>       directory to /usr/local/etc/ksmbd.
>>     - change the prompt to '$'.
>>
>>   Documentation/filesystems/cifs/ksmbd.rst | 40 +++++++++++++++++-------
>>   1 file changed, 29 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/filesystems/cifs/ksmbd.rst
>> b/Documentation/filesystems/cifs/ksmbd.rst
>> index 1af600db2e70..4284341c89f3 100644
>> --- a/Documentation/filesystems/cifs/ksmbd.rst
>> +++ b/Documentation/filesystems/cifs/ksmbd.rst
>> @@ -118,26 +118,44 @@ ksmbd/nfsd interoperability    Planned for future.
>> The features that ksmbd
>>   How to run
>>   ==========
>>
>> -1. Download ksmbd-tools and compile them.
>> -	- https://github.com/cifsd-team/ksmbd-tools
>> +1. Download
>> ksmbd-tools(https://github.com/cifsd-team/ksmbd-tools/releases) and
>> +   compile them.
>> +
>> +   - Refer
>> README(https://github.com/cifsd-team/ksmbd-tools/blob/master/README.md)
>> +     to know how to use ksmbd.mountd/adduser/addshare/control utils
>> +
>> +     $ ./autogen.sh
>> +     $ ./configure --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>
>> +   - See ksmbd.adduser manpage.
>> +
>> +     $ man ksmbd.adduser
>> +     $ sudo ksmbd.adduser -a <Enter USERNAME for SMB share access>
>> +
>> +3. Create /usr/local/etc/ksmbd/ksmbd.conf file, add SMB share in
>> ksmbd.conf file.
>
> I missed this in the v2 match - are you intentionally moving the
> ksmbd.conf file to /usr/local/etc? That seems a very mysterious
> location. Nothing on my vanilla installed system places anything
> in there.
To avoid conflicts with the existing distribution package, the default
location as far as I know is /usr/local/etc. And it can be changed
with --sysconfdir. It is same with samba.
>
> Also, doesn't this file need to exist before step 2??
Ah, Yes. Will switch them.

Thanks for your review!
>
> Tom.
>
>
>> -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 ksmbd.conf.example in ksmbd-utils, See ksmbd.conf manpage
>> +     for details to configure shares.
>>
>> -4. Insert ksmbd.ko module
>> +        $ man ksmbd.conf
>>
>> -	# insmod ksmbd.ko
>> +4. Insert ksmbd.ko module after build your kernel. No need to load
>> module
>> +   if ksmbd is built into the kernel.
>> +
>> +   - Set ksmbd in menuconfig(e.g. $ make menuconfig)
>> +       [*] Network File Systems  --->
>> +           <M> SMB3 server support (EXPERIMENTAL)
>> +
>> +	$ sudo modprobe ksmbd.ko
>>
>>   5. Start ksmbd user space daemon
>> -	# ksmbd.mountd
>>
>> -6. Access share from Windows or Linux using CIFS
>> +	$ sudo ksmbd.mountd
>> +
>> +6. Access share from Windows or Linux SMB3 clients (cifs.ko or smbclient
>> of samba)
>>
>>   Shutdown KSMBD
>>   ==============
>

Re: [PATCH v3] ksmbd: update documentation

[Tom Talpey]

On 9/12/2022 4:54 PM, Namjae Jeon wrote:
> 2022-09-13 8:38 GMT+09:00, Tom Talpey <tom@talpey.com>:
>> On 9/9/2022 5:25 AM, Namjae Jeon wrote:
>>> configuration.txt in ksmbd-tools moved to ksmbd.conf manpage.
>>> update it and more detailed ksmbd-tools build method.
>>>
>>> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
>>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
>>> ---
>>>    v3:
>>>      - replace CIFS with SMB3 clients.
>>>      - update ksmbd built-in case.
>>>      - replace smb.conf leftover with ksmbd.conf.
>>>      - use full name of utils in ksmbd-tools instead of <foo>.
>>>      - fix the warnings from make htlmdocs build reported by kernel test
>>>        robot.
>>>    v2:
>>>      - rename smb.conf to ksmbd.conf.
>>>      - add how to set ksmbd module in menuconfig
>>>      - remove --syscondir option for configure, instead change ksmbd
>>>        directory to /usr/local/etc/ksmbd.
>>>      - change the prompt to '$'.
>>>
>>>    Documentation/filesystems/cifs/ksmbd.rst | 40 +++++++++++++++++-------
>>>    1 file changed, 29 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/Documentation/filesystems/cifs/ksmbd.rst
>>> b/Documentation/filesystems/cifs/ksmbd.rst
>>> index 1af600db2e70..4284341c89f3 100644
>>> --- a/Documentation/filesystems/cifs/ksmbd.rst
>>> +++ b/Documentation/filesystems/cifs/ksmbd.rst
>>> @@ -118,26 +118,44 @@ ksmbd/nfsd interoperability    Planned for future.
>>> The features that ksmbd
>>>    How to run
>>>    ==========
>>>
>>> -1. Download ksmbd-tools and compile them.
>>> -	- https://github.com/cifsd-team/ksmbd-tools
>>> +1. Download
>>> ksmbd-tools(https://github.com/cifsd-team/ksmbd-tools/releases) and
>>> +   compile them.
>>> +
>>> +   - Refer
>>> README(https://github.com/cifsd-team/ksmbd-tools/blob/master/README.md)
>>> +     to know how to use ksmbd.mountd/adduser/addshare/control utils
>>> +
>>> +     $ ./autogen.sh
>>> +     $ ./configure --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>
>>> +   - See ksmbd.adduser manpage.
>>> +
>>> +     $ man ksmbd.adduser
>>> +     $ sudo ksmbd.adduser -a <Enter USERNAME for SMB share access>
>>> +
>>> +3. Create /usr/local/etc/ksmbd/ksmbd.conf file, add SMB share in
>>> ksmbd.conf file.
>>
>> I missed this in the v2 match - are you intentionally moving the
>> ksmbd.conf file to /usr/local/etc? That seems a very mysterious
>> location. Nothing on my vanilla installed system places anything
>> in there.
> To avoid conflicts with the existing distribution package, the default
> location as far as I know is /usr/local/etc. And it can be changed
> with --sysconfdir. It is same with samba.

I totally disagree with this. The kernel server is part of, well,
the kernel, and loading the kernel should not depend on a path like
/usr/local/etc. Also, nothing I know, including Samba, is deployed
with such a directory in my experience. I find smb.conf in /etc/samba.

Where are the ksmbd.<foo> helpers installed by default? /usr/local/sbin?
On my standard Ubuntu install (and presumably Debian?) they are in
/sbin.

Tom.

>> Also, doesn't this file need to exist before step 2??
> Ah, Yes. Will switch them.
> 
> Thanks for your review!
>>
>> Tom.
>>
>>
>>> -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 ksmbd.conf.example in ksmbd-utils, See ksmbd.conf manpage
>>> +     for details to configure shares.
>>>
>>> -4. Insert ksmbd.ko module
>>> +        $ man ksmbd.conf
>>>
>>> -	# insmod ksmbd.ko
>>> +4. Insert ksmbd.ko module after build your kernel. No need to load
>>> module
>>> +   if ksmbd is built into the kernel.
>>> +
>>> +   - Set ksmbd in menuconfig(e.g. $ make menuconfig)
>>> +       [*] Network File Systems  --->
>>> +           <M> SMB3 server support (EXPERIMENTAL)
>>> +
>>> +	$ sudo modprobe ksmbd.ko
>>>
>>>    5. Start ksmbd user space daemon
>>> -	# ksmbd.mountd
>>>
>>> -6. Access share from Windows or Linux using CIFS
>>> +	$ sudo ksmbd.mountd
>>> +
>>> +6. Access share from Windows or Linux SMB3 clients (cifs.ko or smbclient
>>> of samba)
>>>
>>>    Shutdown KSMBD
>>>    ==============
>>
> 

Re: [PATCH v3] ksmbd: update documentation

[Namjae Jeon]

2022-09-14 4:02 GMT+09:00, Tom Talpey <tom@talpey.com>:
> On 9/12/2022 4:54 PM, Namjae Jeon wrote:
>> 2022-09-13 8:38 GMT+09:00, Tom Talpey <tom@talpey.com>:
>>> On 9/9/2022 5:25 AM, Namjae Jeon wrote:
>>>> configuration.txt in ksmbd-tools moved to ksmbd.conf manpage.
>>>> update it and more detailed ksmbd-tools build method.
>>>>
>>>> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
>>>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
>>>> ---
>>>>    v3:
>>>>      - replace CIFS with SMB3 clients.
>>>>      - update ksmbd built-in case.
>>>>      - replace smb.conf leftover with ksmbd.conf.
>>>>      - use full name of utils in ksmbd-tools instead of <foo>.
>>>>      - fix the warnings from make htlmdocs build reported by kernel
>>>> test
>>>>        robot.
>>>>    v2:
>>>>      - rename smb.conf to ksmbd.conf.
>>>>      - add how to set ksmbd module in menuconfig
>>>>      - remove --syscondir option for configure, instead change ksmbd
>>>>        directory to /usr/local/etc/ksmbd.
>>>>      - change the prompt to '$'.
>>>>
>>>>    Documentation/filesystems/cifs/ksmbd.rst | 40
>>>> +++++++++++++++++-------
>>>>    1 file changed, 29 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/Documentation/filesystems/cifs/ksmbd.rst
>>>> b/Documentation/filesystems/cifs/ksmbd.rst
>>>> index 1af600db2e70..4284341c89f3 100644
>>>> --- a/Documentation/filesystems/cifs/ksmbd.rst
>>>> +++ b/Documentation/filesystems/cifs/ksmbd.rst
>>>> @@ -118,26 +118,44 @@ ksmbd/nfsd interoperability    Planned for
>>>> future.
>>>> The features that ksmbd
>>>>    How to run
>>>>    ==========
>>>>
>>>> -1. Download ksmbd-tools and compile them.
>>>> -	- https://github.com/cifsd-team/ksmbd-tools
>>>> +1. Download
>>>> ksmbd-tools(https://github.com/cifsd-team/ksmbd-tools/releases) and
>>>> +   compile them.
>>>> +
>>>> +   - Refer
>>>> README(https://github.com/cifsd-team/ksmbd-tools/blob/master/README.md)
>>>> +     to know how to use ksmbd.mountd/adduser/addshare/control utils
>>>> +
>>>> +     $ ./autogen.sh
>>>> +     $ ./configure --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>
>>>> +   - See ksmbd.adduser manpage.
>>>> +
>>>> +     $ man ksmbd.adduser
>>>> +     $ sudo ksmbd.adduser -a <Enter USERNAME for SMB share access>
>>>> +
>>>> +3. Create /usr/local/etc/ksmbd/ksmbd.conf file, add SMB share in
>>>> ksmbd.conf file.
>>>
>>> I missed this in the v2 match - are you intentionally moving the
>>> ksmbd.conf file to /usr/local/etc? That seems a very mysterious
>>> location. Nothing on my vanilla installed system places anything
>>> in there.
>> To avoid conflicts with the existing distribution package, the default
>> location as far as I know is /usr/local/etc. And it can be changed
>> with --sysconfdir. It is same with samba.
>
> I totally disagree with this. The kernel server is part of, well,
> the kernel, and loading the kernel should not depend on a path like
> /usr/local/etc.
You should consider ksmbd-tools included ksmbd.mountd also.
To start running ksmbd server, ksmbd.mountd should be launched.
> Also, nothing I know, including Samba, is deployed
> with such a directory in my experience. I find smb.conf in /etc/samba.
If you build samba by default, it is installed into /usr/local.
/etc/samba/ location you are saying is installed by package manager.

>
> Where are the ksmbd.<foo> helpers installed by default? /usr/local/sbin?
> On my standard Ubuntu install (and presumably Debian?) they are in
> /sbin.
I think that you are confusing binary of ksmbd-tools which installed
by package manager.
make & make install in ksmbd-tools directory and can see them in
/usr/local/sbin/ .

>
> Tom.
>
>>> Also, doesn't this file need to exist before step 2??
>> Ah, Yes. Will switch them.
>>
>> Thanks for your review!
>>>
>>> Tom.
>>>
>>>
>>>> -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 ksmbd.conf.example in ksmbd-utils, See ksmbd.conf manpage
>>>> +     for details to configure shares.
>>>>
>>>> -4. Insert ksmbd.ko module
>>>> +        $ man ksmbd.conf
>>>>
>>>> -	# insmod ksmbd.ko
>>>> +4. Insert ksmbd.ko module after build your kernel. No need to load
>>>> module
>>>> +   if ksmbd is built into the kernel.
>>>> +
>>>> +   - Set ksmbd in menuconfig(e.g. $ make menuconfig)
>>>> +       [*] Network File Systems  --->
>>>> +           <M> SMB3 server support (EXPERIMENTAL)
>>>> +
>>>> +	$ sudo modprobe ksmbd.ko
>>>>
>>>>    5. Start ksmbd user space daemon
>>>> -	# ksmbd.mountd
>>>>
>>>> -6. Access share from Windows or Linux using CIFS
>>>> +	$ sudo ksmbd.mountd
>>>> +
>>>> +6. Access share from Windows or Linux SMB3 clients (cifs.ko or
>>>> smbclient
>>>> of samba)
>>>>
>>>>    Shutdown KSMBD
>>>>    ==============
>>>
>>
>

Re: [PATCH v3] ksmbd: update documentation

[Atte Heikkilä]

On Tue, 13 Sep 2022 12:02:47 -0700, Tom Talpey wrote:
> On 9/12/2022 4:54 PM, Namjae Jeon wrote:
>> 2022-09-13 8:38 GMT+09:00, Tom Talpey <tom@talpey.com>:
>>>
>>> I missed this in the v2 match - are you intentionally moving the
>>> ksmbd.conf file to /usr/local/etc? That seems a very mysterious
>>> location. Nothing on my vanilla installed system places anything
>>> in there.
>> To avoid conflicts with the existing distribution package, the default
>> location as far as I know is /usr/local/etc. And it can be changed
>> with --sysconfdir. It is same with samba.
> 
> I totally disagree with this. The kernel server is part of, well,
> the kernel, and loading the kernel should not depend on a path like
> /usr/local/etc.

I really don't understand what this means. The dependency to the
sysconfdir path isn't ksmbd's, it's ksmbd-tools'.

> Also, nothing I know, including Samba, is deployed
> with such a directory in my experience. I find smb.conf in /etc/samba.

Yes, that is because your distribution builds it for you. If you build it
yourself, and don't want to collide with your distribution's packaged
version of it, then you choose some prefix other than /usr.

> 
> Where are the ksmbd.<foo> helpers installed by default? /usr/local/sbin?
> On my standard Ubuntu install (and presumably Debian?) they are in
> /sbin.

Yes, the GNU autoconf default sbindir is /usr/local/sbin since the default
prefix is /usr/local. It is distinct from the sbindir your distribution's
packages use. Your /sbin is (likely) a symlink to /usr/sbin and the
distribution's packages install in the /usr prefix. The /etc sysconfdir
is associated with the /usr prefix. You can also check what FHS has to
say about /usr/local if you'd like.

Namjae's way of running configure is correct. It's either this or colliding
with file paths used by the packaged ksmbd-tools, which isn't a good idea.

> 
> Tom.
> 
>>> Also, doesn't this file need to exist before step 2??
>> Ah, Yes. Will switch them.
>> 
>> Thanks for your review!
>>>
>>> Tom.
>>>
>>>
>>>
>> 
> 

Re: [PATCH v3] ksmbd: update documentation

[Tom Talpey]

On 9/14/2022 4:09 AM, Atte Heikkilä wrote:
> On Tue, 13 Sep 2022 12:02:47 -0700, Tom Talpey wrote:
>> On 9/12/2022 4:54 PM, Namjae Jeon wrote:
>>> 2022-09-13 8:38 GMT+09:00, Tom Talpey <tom@talpey.com>:
>>>>
>>>> I missed this in the v2 match - are you intentionally moving the
>>>> ksmbd.conf file to /usr/local/etc? That seems a very mysterious
>>>> location. Nothing on my vanilla installed system places anything
>>>> in there.
>>> To avoid conflicts with the existing distribution package, the default
>>> location as far as I know is /usr/local/etc. And it can be changed
>>> with --sysconfdir. It is same with samba.
>>
>> I totally disagree with this. The kernel server is part of, well,
>> the kernel, and loading the kernel should not depend on a path like
>> /usr/local/etc.
> 
> I really don't understand what this means. The dependency to the
> sysconfdir path isn't ksmbd's, it's ksmbd-tools'.

But ksmbd depends on the ksmbd-tools to provide the configuration
to the kernel module, and to manage the service. So, the user/admin
will view them as one subsystem.

>> Also, nothing I know, including Samba, is deployed
>> with such a directory in my experience. I find smb.conf in /etc/samba.
> 
> Yes, that is because your distribution builds it for you. If you build it
> yourself, and don't want to collide with your distribution's packaged
> version of it, then you choose some prefix other than /usr.

I understand that. My point is that distributions have already
built ksmbd-tools with sysconfdir==/etc, which IMO is appropriate
and correct.

>> Where are the ksmbd.<foo> helpers installed by default? /usr/local/sbin?
>> On my standard Ubuntu install (and presumably Debian?) they are in
>> /sbin.
> 
> Yes, the GNU autoconf default sbindir is /usr/local/sbin since the default
> prefix is /usr/local. It is distinct from the sbindir your distribution's
> packages use. Your /sbin is (likely) a symlink to /usr/sbin and the
> distribution's packages install in the /usr prefix. The /etc sysconfdir
> is associated with the /usr prefix. You can also check what FHS has to
> say about /usr/local if you'd like.
> 
> Namjae's way of running configure is correct. It's either this or colliding
> with file paths used by the packaged ksmbd-tools, which isn't a good idea.

Ok, so if overriding the default sysconfdir builds the manpage
output to follow, we're good here. Out of step, perhaps, but good.

Tom.

> 
>>
>> Tom.
>>
>>>> Also, doesn't this file need to exist before step 2??
>>> Ah, Yes. Will switch them.
>>>
>>> Thanks for your review!
>>>>
>>>> Tom.
>>>>
>>>>
>>>>
>>>
>>
>