diff mbox series

ksmbd-tools: add max connections parameter to global section

Message ID 20221227150213.9842-1-linkinjeon@kernel.org
State New
Headers show
Series ksmbd-tools: add max connections parameter to global section | expand

Commit Message

Namjae Jeon Dec. 27, 2022, 3:02 p.m. UTC
Add max connections parameter to limit number of maximum simultaneous
connections.

Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 include/linux/ksmbd_server.h | 3 ++-
 include/tools.h              | 1 +
 ksmbd.conf.5.in              | 2 +-
 ksmbd.conf.example           | 1 +
 mountd/ipc.c                 | 1 +
 tools/config_parser.c        | 5 +++++
 6 files changed, 11 insertions(+), 2 deletions(-)

Comments

Sergey Senozhatsky Dec. 30, 2022, 2:54 a.m. UTC | #1
On (22/12/28 00:02), Namjae Jeon wrote:
[..]
> @@ -175,6 +175,7 @@ static int ipc_ksmbd_starting_up(void)
>  	ev->smb2_max_write = global_conf.smb2_max_write;
>  	ev->smb2_max_trans = global_conf.smb2_max_trans;
>  	ev->smbd_max_io_size = global_conf.smbd_max_io_size;
> +	ev->max_connections = global_conf.max_connections;
>  	ev->share_fake_fscaps = global_conf.share_fake_fscaps;
>  	memcpy(ev->sub_auth, global_conf.gen_subauth, sizeof(ev->sub_auth));
>  	ev->smb2_max_credits = global_conf.smb2_max_credits;
> diff --git a/tools/config_parser.c b/tools/config_parser.c
> index 2dc6b34..5f36606 100644
> --- a/tools/config_parser.c
> +++ b/tools/config_parser.c
> @@ -548,6 +548,11 @@ static gboolean global_group_kv(gpointer _k, gpointer _v, gpointer user_data)
>  		return TRUE;
>  	}
>  
> +	if (!cp_key_cmp(_k, "max connections")) {
> +		global_conf.max_connections = memparse(_v);
> +		return TRUE;
> +	}
> +

I'd say that it'll make sense to me if ksmbd will impose a default
limit on the number of connections, which people can overwrite. Yes,
I know that samba doesn't limit by default, but ksmbd is a kernel
module and the price of unlimited resource consumption is higher.
We can't probably easily apply the "samba does it" rule here. What
do you think?

How about:
- default `max connections`, say, of 512. max possible value, say, 64k?
- `max connections` cannot be zero
Namjae Jeon Dec. 30, 2022, 3:56 a.m. UTC | #2
2022-12-30 11:54 GMT+09:00, Sergey Senozhatsky <senozhatsky@chromium.org>:
> On (22/12/28 00:02), Namjae Jeon wrote:
> [..]
>> @@ -175,6 +175,7 @@ static int ipc_ksmbd_starting_up(void)
>>  	ev->smb2_max_write = global_conf.smb2_max_write;
>>  	ev->smb2_max_trans = global_conf.smb2_max_trans;
>>  	ev->smbd_max_io_size = global_conf.smbd_max_io_size;
>> +	ev->max_connections = global_conf.max_connections;
>>  	ev->share_fake_fscaps = global_conf.share_fake_fscaps;
>>  	memcpy(ev->sub_auth, global_conf.gen_subauth, sizeof(ev->sub_auth));
>>  	ev->smb2_max_credits = global_conf.smb2_max_credits;
>> diff --git a/tools/config_parser.c b/tools/config_parser.c
>> index 2dc6b34..5f36606 100644
>> --- a/tools/config_parser.c
>> +++ b/tools/config_parser.c
>> @@ -548,6 +548,11 @@ static gboolean global_group_kv(gpointer _k, gpointer
>> _v, gpointer user_data)
>>  		return TRUE;
>>  	}
>>
>> +	if (!cp_key_cmp(_k, "max connections")) {
>> +		global_conf.max_connections = memparse(_v);
>> +		return TRUE;
>> +	}
>> +
>
> I'd say that it'll make sense to me if ksmbd will impose a default
> limit on the number of connections, which people can overwrite. Yes,
> I know that samba doesn't limit by default, but ksmbd is a kernel
> module and the price of unlimited resource consumption is higher.
> We can't probably easily apply the "samba does it" rule here. What
> do you think?
>
> How about:
> - default `max connections`, say, of 512. max possible value, say, 64k?
> - `max connections` cannot be zero
Make sense. I will update it on v2.
Thanks for your review:)
>
diff mbox series

Patch

diff --git a/include/linux/ksmbd_server.h b/include/linux/ksmbd_server.h
index 8ec004f..64099f2 100644
--- a/include/linux/ksmbd_server.h
+++ b/include/linux/ksmbd_server.h
@@ -49,7 +49,8 @@  struct ksmbd_startup_request {
 	__u32	sub_auth[3];
 	__u32	smb2_max_credits;
 	__u32	smbd_max_io_size;	/* smbd read write size */
-	__u32   reserved[127];		/* Reserved room */
+	__u32	max_connections;	/* Number of maximum simultaneous connections */
+	__u32   reserved[126];		/* Reserved room */
 	__u32	ifc_list_sz;
 	__s8	____payload[];
 };
diff --git a/include/tools.h b/include/tools.h
index f6f51f8..6ff77b9 100644
--- a/include/tools.h
+++ b/include/tools.h
@@ -53,6 +53,7 @@  struct smbconf_global {
 	unsigned int		smb2_max_trans;
 	unsigned int		smb2_max_credits;
 	unsigned int		smbd_max_io_size;
+	unsigned int		max_connections;
 	unsigned int		share_fake_fscaps;
 	unsigned int		gen_subauth[3];
 	char			*krb5_keytab_file;
diff --git a/ksmbd.conf.5.in b/ksmbd.conf.5.in
index a1dfb4a..3cb237d 100644
--- a/ksmbd.conf.5.in
+++ b/ksmbd.conf.5.in
@@ -172,7 +172,7 @@  Maximum number of simultaneous sessions to all shares.
 
 Default: \fBmax active sessions = 1024\fR \" KSMBD_CONF_DEFAULT_SESS_CAP
 .TP
-\fBmax connections\fR (S)
+\fBmax connections\fR (G)
 Maximum number of simultaneous connections to the share.
 With \fBmax connections = 0\fR, any number of connections may be made.
 
diff --git a/ksmbd.conf.example b/ksmbd.conf.example
index 6ce4ec7..6bfc965 100644
--- a/ksmbd.conf.example
+++ b/ksmbd.conf.example
@@ -30,6 +30,7 @@ 
 	smbd max io size = 8MB
 	tcp port = 445
 	workgroup = WORKGROUP
+	max connections = 0
 
 	; share parameters for all sections
 	browseable = yes
diff --git a/mountd/ipc.c b/mountd/ipc.c
index 9d4c1ca..382f5ed 100644
--- a/mountd/ipc.c
+++ b/mountd/ipc.c
@@ -175,6 +175,7 @@  static int ipc_ksmbd_starting_up(void)
 	ev->smb2_max_write = global_conf.smb2_max_write;
 	ev->smb2_max_trans = global_conf.smb2_max_trans;
 	ev->smbd_max_io_size = global_conf.smbd_max_io_size;
+	ev->max_connections = global_conf.max_connections;
 	ev->share_fake_fscaps = global_conf.share_fake_fscaps;
 	memcpy(ev->sub_auth, global_conf.gen_subauth, sizeof(ev->sub_auth));
 	ev->smb2_max_credits = global_conf.smb2_max_credits;
diff --git a/tools/config_parser.c b/tools/config_parser.c
index 2dc6b34..5f36606 100644
--- a/tools/config_parser.c
+++ b/tools/config_parser.c
@@ -548,6 +548,11 @@  static gboolean global_group_kv(gpointer _k, gpointer _v, gpointer user_data)
 		return TRUE;
 	}
 
+	if (!cp_key_cmp(_k, "max connections")) {
+		global_conf.max_connections = memparse(_v);
+		return TRUE;
+	}
+
 	/* At this point, this is an option that must be applied to all shares */
 	return FALSE;
 }