fs/smb/client/cifsfs.c | 1 - fs/smb/client/fs_context.c | 65 ++++++++++++++++++++++++++++---------- fs/smb/client/fs_context.h | 2 ++ 3 files changed, 50 insertions(+), 18 deletions(-)
From: Rajasi Mandal <rajasimandal@microsoft.com>
Previously, the behavior of the multichannel and max_channels mount
options was inconsistent and order-dependent. For example, specifying
"multichannel,max_channels=1" would result in 2 channels, while
"max_channels=1,multichannel" would result in 1 channel. Additionally,
conflicting combinations such as "nomultichannel,max_channels=3" or
"multichannel,max_channels=1" did not produce errors and could lead to
unexpected channel counts.
This commit introduces two new fields in smb3_fs_context to explicitly
track whether multichannel and max_channels were specified during
mount. The option parsing and validation logic is updated to ensure:
- The outcome is no longer dependent on the order of options.
- Conflicting combinations (e.g., "nomultichannel,max_channels=3" or
"multichannel,max_channels=1") are detected and result in an error.
- The number of channels created is consistent with the specified
options.
This improves the reliability and predictability of mount option
handling for SMB3 multichannel support.
Reviewed-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Rajasi Mandal <rajasimandal@microsoft.com>
---
fs/smb/client/cifsfs.c | 1 -
fs/smb/client/fs_context.c | 65 ++++++++++++++++++++++++++++----------
fs/smb/client/fs_context.h | 2 ++
3 files changed, 50 insertions(+), 18 deletions(-)
diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index 185ac41bd7e9..4d53ec53d8db 100644
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -1016,7 +1016,6 @@ cifs_smb3_do_mount(struct file_system_type *fs_type,
} else {
cifs_info("Attempting to mount %s\n", old_ctx->source);
}
-
cifs_sb = kzalloc(sizeof(*cifs_sb), GFP_KERNEL);
if (!cifs_sb)
return ERR_PTR(-ENOMEM);
diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
index 46d1eaae62da..1794a31541fe 100644
--- a/fs/smb/client/fs_context.c
+++ b/fs/smb/client/fs_context.c
@@ -711,6 +711,47 @@ smb3_parse_devname(const char *devname, struct smb3_fs_context *ctx)
return 0;
}
+static int smb3_handle_conflicting_options(struct fs_context *fc)
+{
+ struct smb3_fs_context *ctx = smb3_fc2context(fc);
+
+ if (ctx->multichannel_specified) {
+ if (ctx->multichannel) {
+ if (!ctx->max_channels_specified) {
+ ctx->max_channels = 2;
+ } else if (ctx->max_channels == 1) {
+ cifs_errorf(fc,
+ "max_channels must be greater than 1 when multichannel is enabled\n");
+ return -EINVAL;
+ }
+ } else {
+ if (!ctx->max_channels_specified) {
+ ctx->max_channels = 1;
+ } else if (ctx->max_channels > 1) {
+ cifs_errorf(fc,
+ "max_channels must be equal to 1 when multichannel is disabled\n");
+ return -EINVAL;
+ }
+ }
+ } else {
+ if (ctx->max_channels_specified) {
+ if (ctx->max_channels > 1)
+ ctx->multichannel = true;
+ else
+ ctx->multichannel = false;
+ } else {
+ ctx->multichannel = false;
+ ctx->max_channels = 1;
+ }
+ }
+
+ //resetting default values as remount doesn't initialize fs_context again
+ ctx->multichannel_specified = false;
+ ctx->max_channels_specified = false;
+
+ return 0;
+}
+
static void smb3_fs_context_free(struct fs_context *fc);
static int smb3_fs_context_parse_param(struct fs_context *fc,
struct fs_parameter *param);
@@ -785,6 +826,7 @@ static int smb3_fs_context_parse_monolithic(struct fs_context *fc,
if (ret < 0)
break;
}
+ ret = smb3_handle_conflicting_options(fc);
return ret;
}
@@ -1296,15 +1338,11 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
ctx->nodelete = 1;
break;
case Opt_multichannel:
- if (result.negated) {
+ ctx->multichannel_specified = true;
+ if (result.negated)
ctx->multichannel = false;
- ctx->max_channels = 1;
- } else {
+ else
ctx->multichannel = true;
- /* if number of channels not specified, default to 2 */
- if (ctx->max_channels < 2)
- ctx->max_channels = 2;
- }
break;
case Opt_uid:
ctx->linux_uid = result.uid;
@@ -1440,15 +1478,13 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
ctx->max_credits = result.uint_32;
break;
case Opt_max_channels:
+ ctx->max_channels_specified = true;
if (result.uint_32 < 1 || result.uint_32 > CIFS_MAX_CHANNELS) {
cifs_errorf(fc, "%s: Invalid max_channels value, needs to be 1-%d\n",
__func__, CIFS_MAX_CHANNELS);
goto cifs_parse_mount_err;
}
ctx->max_channels = result.uint_32;
- /* If more than one channel requested ... they want multichan */
- if (result.uint_32 > 1)
- ctx->multichannel = true;
break;
case Opt_max_cached_dirs:
if (result.uint_32 < 1) {
@@ -1866,13 +1902,6 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
goto cifs_parse_mount_err;
}
- /*
- * Multichannel is not meaningful if max_channels is 1.
- * Force multichannel to false to ensure consistent configuration.
- */
- if (ctx->multichannel && ctx->max_channels == 1)
- ctx->multichannel = false;
-
return 0;
cifs_parse_mount_err:
@@ -1955,6 +1984,8 @@ int smb3_init_fs_context(struct fs_context *fc)
/* default to no multichannel (single server connection) */
ctx->multichannel = false;
+ ctx->multichannel_specified = false;
+ ctx->max_channels_specified = false;
ctx->max_channels = 1;
ctx->backupuid_specified = false; /* no backup intent for a user */
diff --git a/fs/smb/client/fs_context.h b/fs/smb/client/fs_context.h
index b0fec6b9a23b..7af7cbbe4208 100644
--- a/fs/smb/client/fs_context.h
+++ b/fs/smb/client/fs_context.h
@@ -294,6 +294,8 @@ struct smb3_fs_context {
bool domainauto:1;
bool rdma:1;
bool multichannel:1;
+ bool multichannel_specified:1; /* true if user specified multichannel or nomultichannel */
+ bool max_channels_specified:1; /* true if user specified max_channels */
bool use_client_guid:1;
/* reuse existing guid for multichannel */
u8 client_guid[SMB2_CLIENT_GUID_SIZE];
--
2.43.0On 11/18, Rajasi Mandal wrote:
>From: Rajasi Mandal <rajasimandal@microsoft.com>
>
>Previously, the behavior of the multichannel and max_channels mount
>options was inconsistent and order-dependent. For example, specifying
>"multichannel,max_channels=1" would result in 2 channels, while
>"max_channels=1,multichannel" would result in 1 channel. Additionally,
>conflicting combinations such as "nomultichannel,max_channels=3" or
>"multichannel,max_channels=1" did not produce errors and could lead to
>unexpected channel counts.
>
>This commit introduces two new fields in smb3_fs_context to explicitly
>track whether multichannel and max_channels were specified during
>mount. The option parsing and validation logic is updated to ensure:
>- The outcome is no longer dependent on the order of options.
>- Conflicting combinations (e.g., "nomultichannel,max_channels=3" or
> "multichannel,max_channels=1") are detected and result in an error.
>- The number of channels created is consistent with the specified
> options.
>
>This improves the reliability and predictability of mount option
>handling for SMB3 multichannel support.
>
>Reviewed-by: Shyam Prasad N <sprasad@microsoft.com>
>Signed-off-by: Rajasi Mandal <rajasimandal@microsoft.com>
It's conflicting because it's already too complex for something that
should've been simple. This patch introduces a new field + unnecessary
logic on top if it all.
cf. a PoC patch I sent a while ago, we can (ab)use fsparam with same key
name, but different key types, so we could only deal with:
'nomultichannel', 'multichannel={0,1,off,no}' as multichannel disabled
'multichannel' as ctx->max_channels=2 (multichannel enabled, obviously)
'multichannel=X' as ctx->max_channels=X (ditto)
Makes 0 sense to have both multichannel and max_channels mount options.
>---
> fs/smb/client/cifsfs.c | 1 -
> fs/smb/client/fs_context.c | 65 ++++++++++++++++++++++++++++----------
> fs/smb/client/fs_context.h | 2 ++
> 3 files changed, 50 insertions(+), 18 deletions(-)
>
>diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
>index 185ac41bd7e9..4d53ec53d8db 100644
>--- a/fs/smb/client/cifsfs.c
>+++ b/fs/smb/client/cifsfs.c
>@@ -1016,7 +1016,6 @@ cifs_smb3_do_mount(struct file_system_type *fs_type,
> } else {
> cifs_info("Attempting to mount %s\n", old_ctx->source);
> }
>-
> cifs_sb = kzalloc(sizeof(*cifs_sb), GFP_KERNEL);
> if (!cifs_sb)
> return ERR_PTR(-ENOMEM);
>diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
>index 46d1eaae62da..1794a31541fe 100644
>--- a/fs/smb/client/fs_context.c
>+++ b/fs/smb/client/fs_context.c
>@@ -711,6 +711,47 @@ smb3_parse_devname(const char *devname, struct smb3_fs_context *ctx)
> return 0;
> }
>
>+static int smb3_handle_conflicting_options(struct fs_context *fc)
>+{
>+ struct smb3_fs_context *ctx = smb3_fc2context(fc);
>+
>+ if (ctx->multichannel_specified) {
>+ if (ctx->multichannel) {
>+ if (!ctx->max_channels_specified) {
>+ ctx->max_channels = 2;
>+ } else if (ctx->max_channels == 1) {
>+ cifs_errorf(fc,
>+ "max_channels must be greater than 1 when multichannel is enabled\n");
>+ return -EINVAL;
>+ }
>+ } else {
>+ if (!ctx->max_channels_specified) {
>+ ctx->max_channels = 1;
>+ } else if (ctx->max_channels > 1) {
>+ cifs_errorf(fc,
>+ "max_channels must be equal to 1 when multichannel is disabled\n");
>+ return -EINVAL;
>+ }
>+ }
>+ } else {
>+ if (ctx->max_channels_specified) {
>+ if (ctx->max_channels > 1)
>+ ctx->multichannel = true;
>+ else
>+ ctx->multichannel = false;
>+ } else {
>+ ctx->multichannel = false;
>+ ctx->max_channels = 1;
>+ }
>+ }
>+
>+ //resetting default values as remount doesn't initialize fs_context again
Please stick to /* ... */ comments style.
>+ ctx->multichannel_specified = false;
>+ ctx->max_channels_specified = false;
>+
>+ return 0;
>+}
>+
> static void smb3_fs_context_free(struct fs_context *fc);
> static int smb3_fs_context_parse_param(struct fs_context *fc,
> struct fs_parameter *param);
>@@ -785,6 +826,7 @@ static int smb3_fs_context_parse_monolithic(struct fs_context *fc,
> if (ret < 0)
> break;
> }
>+ ret = smb3_handle_conflicting_options(fc);
>
> return ret;
> }
>@@ -1296,15 +1338,11 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
> ctx->nodelete = 1;
> break;
> case Opt_multichannel:
>- if (result.negated) {
>+ ctx->multichannel_specified = true;
>+ if (result.negated)
> ctx->multichannel = false;
>- ctx->max_channels = 1;
>- } else {
>+ else
> ctx->multichannel = true;
>- /* if number of channels not specified, default to 2 */
>- if (ctx->max_channels < 2)
>- ctx->max_channels = 2;
>- }
> break;
> case Opt_uid:
> ctx->linux_uid = result.uid;
>@@ -1440,15 +1478,13 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
> ctx->max_credits = result.uint_32;
> break;
> case Opt_max_channels:
>+ ctx->max_channels_specified = true;
> if (result.uint_32 < 1 || result.uint_32 > CIFS_MAX_CHANNELS) {
> cifs_errorf(fc, "%s: Invalid max_channels value, needs to be 1-%d\n",
> __func__, CIFS_MAX_CHANNELS);
> goto cifs_parse_mount_err;
> }
> ctx->max_channels = result.uint_32;
>- /* If more than one channel requested ... they want multichan */
>- if (result.uint_32 > 1)
>- ctx->multichannel = true;
> break;
> case Opt_max_cached_dirs:
> if (result.uint_32 < 1) {
>@@ -1866,13 +1902,6 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
> goto cifs_parse_mount_err;
> }
>
>- /*
>- * Multichannel is not meaningful if max_channels is 1.
>- * Force multichannel to false to ensure consistent configuration.
>- */
>- if (ctx->multichannel && ctx->max_channels == 1)
>- ctx->multichannel = false;
>-
> return 0;
>
> cifs_parse_mount_err:
>@@ -1955,6 +1984,8 @@ int smb3_init_fs_context(struct fs_context *fc)
>
> /* default to no multichannel (single server connection) */
> ctx->multichannel = false;
>+ ctx->multichannel_specified = false;
>+ ctx->max_channels_specified = false;
> ctx->max_channels = 1;
>
> ctx->backupuid_specified = false; /* no backup intent for a user */
>diff --git a/fs/smb/client/fs_context.h b/fs/smb/client/fs_context.h
>index b0fec6b9a23b..7af7cbbe4208 100644
>--- a/fs/smb/client/fs_context.h
>+++ b/fs/smb/client/fs_context.h
>@@ -294,6 +294,8 @@ struct smb3_fs_context {
> bool domainauto:1;
> bool rdma:1;
> bool multichannel:1;
>+ bool multichannel_specified:1; /* true if user specified multichannel or nomultichannel */
>+ bool max_channels_specified:1; /* true if user specified max_channels */
> bool use_client_guid:1;
> /* reuse existing guid for multichannel */
> u8 client_guid[SMB2_CLIENT_GUID_SIZE];
>--
>2.43.0
Cheers,
Enzo
On Tue, Nov 18, 2025 at 8:01 AM Enzo Matsumiya via samba-technical
<samba-technical@lists.samba.org> wrote:
>
> On 11/18, Rajasi Mandal wrote:
> >From: Rajasi Mandal <rajasimandal@microsoft.com>
> >
> >Previously, the behavior of the multichannel and max_channels mount
> >options was inconsistent and order-dependent. For example, specifying
> >"multichannel,max_channels=1" would result in 2 channels, while
> >"max_channels=1,multichannel" would result in 1 channel. Additionally,
> >conflicting combinations such as "nomultichannel,max_channels=3" or
> >"multichannel,max_channels=1" did not produce errors and could lead to
> >unexpected channel counts.
> >
> >This commit introduces two new fields in smb3_fs_context to explicitly
> >track whether multichannel and max_channels were specified during
> >mount. The option parsing and validation logic is updated to ensure:
> >- The outcome is no longer dependent on the order of options.
> >- Conflicting combinations (e.g., "nomultichannel,max_channels=3" or
> > "multichannel,max_channels=1") are detected and result in an error.
> >- The number of channels created is consistent with the specified
> > options.
> >
> >This improves the reliability and predictability of mount option
> >handling for SMB3 multichannel support.
> >
> >Reviewed-by: Shyam Prasad N <sprasad@microsoft.com>
> >Signed-off-by: Rajasi Mandal <rajasimandal@microsoft.com>
>
> It's conflicting because it's already too complex for something that
> should've been simple. This patch introduces a new field + unnecessary
> logic on top if it all.
>
> cf. a PoC patch I sent a while ago, we can (ab)use fsparam with same key
> name, but different key types, so we could only deal with:
>
> 'nomultichannel', 'multichannel={0,1,off,no}' as multichannel disabled
> 'multichannel' as ctx->max_channels=2 (multichannel enabled, obviously)
> 'multichannel=X' as ctx->max_channels=X (ditto)
>
> Makes 0 sense to have both multichannel and max_channels mount options.
We can't regress customers who use common mount options without
warning them for multiple releases that parm is going to be removed.
I don't object to changing Opt_max_channels parsing so
ctx->max_channels in fs_context fgoes rom a # to a combination of
number and something which can be mapped to on/off (for on client
picks default on the fly while for off sets channels to 1) and
removing ctx->multichannel - so if you specify "multichannel" it sets
ctx->max_channels to something like -1 (or whatever max # is) and if
you set nomultichannel it sets ctx->max_channels to 1
--
Thanks,
Steve
© 2016 - 2025 Red Hat, Inc.