linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* RFC: ksmbd ABI for ksmbd-tools...
@ 2021-02-12 14:38 ` Stefan Metzmacher
  2021-02-13  2:06   ` [Linux-cifsd-devel] " Sergey Senozhatsky
  2021-02-15  0:59   ` Namjae Jeon
  0 siblings, 2 replies; 3+ messages in thread
From: Stefan Metzmacher @ 2021-02-12 14:38 UTC (permalink / raw)
  To: Namjae Jeon, Namjae Jeon, linux-cifsd-devel, Samba Technical,
	Linux API Mailing List, linux-cifs


[-- Attachment #1.1: Type: text/plain, Size: 2800 bytes --]

Hi Namjae,

I looked through the interfaces used between userspace (ksmbd.mountd and ksmbd.control)
and the kernel module.

After loading the ksmbd.ko module and calling 'ksmbd.mountd', I see
the following related proceses/kernel-threads:

  12200 ?        I      0:00 [kworker/0:0-ksmbd-io]
  12247 ?        Ss     0:00 ksmbd.mountd
  12248 ?        S      0:00 ksmbd.mountd
  12249 ?        S      0:00 [ksmbd-lo]
  12250 ?        S      0:00 [ksmbd-enp0s3]
  12251 ?        S      0:00 [ksmbd-enp0s8]
  12252 ?        S      0:00 [ksmbd-enp0s9]
  12253 ?        S      0:00 [ksmbd-enp0s10]
  12254 ?        I<     0:00 [ksmbd-smb_direc]
  12255 ?        S      0:00 [ksmbd:38794]
  12257 ?        S      0:00 [ksmbd:51579]

I haven't found the exact place, but ksmbd.mountd starts the kernel-part.

ksmbd.mountd also acts as some kind of upcall, for the server part,
that takes care of authentication and some basic DCERPC calls.

I'm wondering why there are two separate ways to kill the running server,
'killall ksmbd.mountd' for the userspace part and
'ksmbd.control -s' (which is just a wrapper for
'echo -n "hard" > /sys/class/ksmbd-control/kill_server') to shutdown the server part.

As it's not useful to run any of these two components on its own,
so I'm wondering why there's no stronger relationship.

As naive admin I'd assume that the kernel part would detect the exit of ksmbd.mountd
and shutdown itself.

It would also be great to bind to specific ip addresses instead of devices
and allow to run more than one instance of ksmbd.mountd (with different config files
and or within containers). That's why I think single global hardcoded path like
'/sys/class/ksmbd-control/kill_server' should be avoided, something like:
'/sys/class/ksmbd-control/<pid-of-ksmbd.mountd>/kill_server' would be better
(if it's needed at all).

I also have ideas how ksmbd{.ok,.mountd} could make use of Samba's winbindd (or authentication)
and Samba's rpc services, but this would require a few changes in the netlink protocol
between ksmbd.ko and ksmbd.mountd. It would be great if a Samba smb.conf option could
cause smbd to start ksmbd.mountd in the background and delegate all raw SMB handling
to the kernel.

So my main big question is how stable would the userspace interface to ksmbd.ko be
treated?

Would it be possible to change the netlink protocol or /sys/class/* behavior in future
in order to improve things?

Can we require that the userspace tool matches the kernel version for a while?

I think iproute2 creates a version for each stable kernel tree and tools like
bpftool, perf even come with each single kernel release.

While others like 'cifs.upcall' try to work with any kernel version.

What do others think?

metze


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Linux-cifsd-devel] RFC: ksmbd ABI for ksmbd-tools...
  2021-02-12 14:38 ` RFC: ksmbd ABI for ksmbd-tools Stefan Metzmacher
@ 2021-02-13  2:06   ` Sergey Senozhatsky
  2021-02-15  0:59   ` Namjae Jeon
  1 sibling, 0 replies; 3+ messages in thread
From: Sergey Senozhatsky @ 2021-02-13  2:06 UTC (permalink / raw)
  To: Stefan Metzmacher
  Cc: Namjae Jeon, Namjae Jeon, linux-cifsd-devel, Samba Technical,
	Linux API Mailing List, linux-cifs

Hi,

On (21/02/12 15:38), Stefan Metzmacher via Linux-cifsd-devel wrote:
> I haven't found the exact place, but ksmbd.mountd starts the kernel-part.
> 
> ksmbd.mountd also acts as some kind of upcall, for the server part,
> that takes care of authentication and some basic DCERPC calls.
> 
> I'm wondering why there are two separate ways to kill the running server,
> 'killall ksmbd.mountd' for the userspace part and
> 'ksmbd.control -s' (which is just a wrapper for
> 'echo -n "hard" > /sys/class/ksmbd-control/kill_server') to shutdown the server part.
> 
> As it's not useful to run any of these two components on its own,
> so I'm wondering why there's no stronger relationship.
> 
> As naive admin I'd assume that the kernel part would detect the exit of ksmbd.mountd
> and shutdown itself.

User-space daemon is just some sort of a database engine, kernel module
queries it when it needs something; otherwise kernel module works without
user-space part just fine and doesn't need it. The goal is that when
user-space crashes or gets restarted after update (critical fix) the server
doesn't panic and doesn't kill itself.

E.g. - when you restart mysql you don't expect httpd to kill itself and
to terminate all existing TCP connections.

[..]
> Can we require that the userspace tool matches the kernel version for a while?

I think such a check exists (or at least it used to). Note that the only
time when version mismatch matters is when sizeof() or layout of the
structures that are used for RPC get updated, or new request commands or
status codes added. IOW, RPC version mismatch is the critical thing.
Otherwise there is no real reason (case by case) to forbid version
mismatch. If user-space gets a memory leak or a NULL pointer dereference
fix then there is no real reasons to force server restart or to force
server module version update, because the fix is completely internal to
the user-space daemon.

	-ss

^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: ksmbd ABI for ksmbd-tools...
  2021-02-12 14:38 ` RFC: ksmbd ABI for ksmbd-tools Stefan Metzmacher
  2021-02-13  2:06   ` [Linux-cifsd-devel] " Sergey Senozhatsky
@ 2021-02-15  0:59   ` Namjae Jeon
  1 sibling, 0 replies; 3+ messages in thread
From: Namjae Jeon @ 2021-02-15  0:59 UTC (permalink / raw)
  To: 'Stefan Metzmacher'
  Cc: 'Namjae Jeon',
	linux-cifsd-devel, 'Samba Technical',
	linux-cifs

> Hi Namjae,
Hi Metze,
> 
> I looked through the interfaces used between userspace (ksmbd.mountd and ksmbd.control) and the kernel
> module.
> 
> After loading the ksmbd.ko module and calling 'ksmbd.mountd', I see the following related
> proceses/kernel-threads:
> 
>   12200 ?        I      0:00 [kworker/0:0-ksmbd-io]
>   12247 ?        Ss     0:00 ksmbd.mountd
>   12248 ?        S      0:00 ksmbd.mountd
>   12249 ?        S      0:00 [ksmbd-lo]
>   12250 ?        S      0:00 [ksmbd-enp0s3]
>   12251 ?        S      0:00 [ksmbd-enp0s8]
>   12252 ?        S      0:00 [ksmbd-enp0s9]
>   12253 ?        S      0:00 [ksmbd-enp0s10]
>   12254 ?        I<     0:00 [ksmbd-smb_direc]
>   12255 ?        S      0:00 [ksmbd:38794]
>   12257 ?        S      0:00 [ksmbd:51579]
> 
> I haven't found the exact place, but ksmbd.mountd starts the kernel-part.
> 
> ksmbd.mountd also acts as some kind of upcall, for the server part, that takes care of authentication
> and some basic DCERPC calls.
> 
> I'm wondering why there are two separate ways to kill the running server, 'killall ksmbd.mountd' for
> the userspace part and 'ksmbd.control -s' (which is just a wrapper for 'echo -n "hard" >
> /sys/class/ksmbd-control/kill_server') to shutdown the server part.
Hm.. We can add the code that kill ksmbd.mountd in ksmbd.control -s.
> 
> As it's not useful to run any of these two components on its own, so I'm wondering why there's no
> stronger relationship.
Sergey answered.
> 
> As naive admin I'd assume that the kernel part would detect the exit of ksmbd.mountd and shutdown
> itself.
Sergey answered.
> 
> It would also be great to bind to specific ip addresses instead of devices and allow to run more than
> one instance of ksmbd.mountd (with different config files and or within containers). That's why I
> think single global hardcoded path like '/sys/class/ksmbd-control/kill_server' should be avoided,
> something like:
> '/sys/class/ksmbd-control/<pid-of-ksmbd.mountd>/kill_server' would be better (if it's needed at all).
Could you please elaborate more why we should do this ?

> 
> I also have ideas how ksmbd{.ok,.mountd} could make use of Samba's winbindd (or authentication) and
> Samba's rpc services, but this would require a few changes in the netlink protocol between ksmbd.ko
> and ksmbd.mountd. It would be great if a Samba smb.conf option could cause smbd to start ksmbd.mountd
> in the background and delegate all raw SMB handling to the kernel.
It's what I plan to do in the long run. It would be great for ksmbd to fully support the function
using samba's library. But I don't think ksmbd should have dependency on such samba's libraries.
i.e. If we change the existing netlink protocol in ksmbd to use samba's winbindd and librpc,
The current users using ksmbd on closed systems may not be able to use ksmbd due to GPLv3. So, This
should be a new netlink protocol addition or extension, not change the existing ones.

> 
> So my main big question is how stable would the userspace interface to ksmbd.ko be treated?
Sergey answered. If his answer is not enough, Let me know it.
> 
> Would it be possible to change the netlink protocol or /sys/class/* behavior in future in order to
> improve things?
Yes.
> 
> Can we require that the userspace tool matches the kernel version for a while?
Sergey answered. If there is a better way than now, please give me your opinion.
> 
> I think iproute2 creates a version for each stable kernel tree and tools like bpftool, perf even come
> with each single kernel release.
Ah. Even if there is no change in source, Does it release according to the kernel version?
It would be better that ksmbd-tools also is merged into kernel/tools like bfptool or perf,
but I am not sure if it is possible. nfs-utils seems to be managed well apart from the kernel version.
> 
Thanks!


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-02-15  1:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20210212143813epcas1p1dcbff2491a1c7cf052c03e57f54e1474@epcas1p1.samsung.com>
2021-02-12 14:38 ` RFC: ksmbd ABI for ksmbd-tools Stefan Metzmacher
2021-02-13  2:06   ` [Linux-cifsd-devel] " Sergey Senozhatsky
2021-02-15  0:59   ` Namjae Jeon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).