Re: [PATCH 1/2] CIFS: Propagate min offload along with other parameters from primary to secondary channels.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



`srv_sign_required` and `mnt_sign_required` when true, override the
sign value for each session. If the srv_sign_required or
mnt_sign_required are not true, then the sign value will not be copied
from the primary to secondary without our fix.

The logic can be found in fs/smb/client/connect.c:
```
int
cifs_enable_signing(struct TCP_Server_Info *server, bool mnt_sign_required)
{
bool srv_sign_required = server->sec_mode & server->vals->signing_required;
bool srv_sign_enabled = server->sec_mode & server->vals->signing_enabled;
bool mnt_sign_enabled;

/*
* Is signing required by mnt options? If not then check
* global_secflags to see if it is there.
*/
if (!mnt_sign_required)
mnt_sign_required = ((global_secflags & CIFSSEC_MUST_SIGN) ==
CIFSSEC_MUST_SIGN);

/*
* If signing is required then it's automatically enabled too,
* otherwise, check to see if the secflags allow it.
*/
mnt_sign_enabled = mnt_sign_required ? mnt_sign_required :
(global_secflags & CIFSSEC_MAY_SIGN);

/* If server requires signing, does client allow it? */
if (srv_sign_required) {
if (!mnt_sign_enabled) {
cifs_dbg(VFS, "Server requires signing, but it's disabled in SecurityFlags!\n");
return -EOPNOTSUPP;
}
server->sign = true;
}

/* If client requires signing, does server allow it? */
if (mnt_sign_required) {
if (!srv_sign_enabled) {
cifs_dbg(VFS, "Server does not support signing!\n");
return -EOPNOTSUPP;
}
server->sign = true;
}
```


On Tue, Mar 11, 2025 at 8:56 AM Steve French <smfrench@xxxxxxxxx> wrote:
>
> Was doing some testing with this and tried an experiment with
> multichannel and "sign" mount parm - and it looks like even without
> the fix, we do sign on all channels even though tcp_ses->sign was not
> updated on the secondary channels.  Do you have a reproduction
> scenario for the signing issue?
>
> On Thu, Mar 6, 2025 at 11:47 AM <aman1cifs@xxxxxxxxx> wrote:
> >
> > From: Aman <aman1@xxxxxxxxxxxxx>
> >
> > In a multichannel setup, it was observed that a few fields were not being
> > copied over to the secondary channels, which impacted performance in cases
> > where these options were relevant but not properly synchronized. To address
> > this, this patch introduces copying the following parameters from the
> > primary channel to the secondary channels:
> >
> > - min_offload
> > - compression.requested
> > - dfs_conn
> > - ignore_signature
> > - leaf_fullpath
> > - noblockcnt
> > - retrans
> > - sign
> >
> > By copying these parameters, we ensure consistency across channels and
> > prevent performance degradation due to missing or outdated settings.
> >
> > Signed-off-by: Aman <aman1@xxxxxxxxxxxxx>
> > ---
> >  fs/smb/client/connect.c | 1 +
> >  fs/smb/client/sess.c    | 7 +++++++
> >  2 files changed, 8 insertions(+)
> >
> > diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> > index eaa6be445..eb82458eb 100644
> > --- a/fs/smb/client/connect.c
> > +++ b/fs/smb/client/connect.c
> > @@ -1721,6 +1721,7 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx,
> >         /* Grab netns reference for this server. */
> >         cifs_set_net_ns(tcp_ses, get_net(current->nsproxy->net_ns));
> >
> > +       tcp_ses->sign = ctx->sign;
> >         tcp_ses->conn_id = atomic_inc_return(&tcpSesNextId);
> >         tcp_ses->noblockcnt = ctx->rootfs;
> >         tcp_ses->noblocksnd = ctx->noblocksnd || ctx->rootfs;
> > diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
> > index 91d4d409c..b4d76a37a 100644
> > --- a/fs/smb/client/sess.c
> > +++ b/fs/smb/client/sess.c
> > @@ -522,6 +522,13 @@ cifs_ses_add_channel(struct cifs_ses *ses,
> >         ctx->sockopt_tcp_nodelay = ses->server->tcp_nodelay;
> >         ctx->echo_interval = ses->server->echo_interval / HZ;
> >         ctx->max_credits = ses->server->max_credits;
> > +       ctx->min_offload = ses->server->min_offload;
> > +       ctx->compress = ses->server->compression.requested;
> > +       ctx->dfs_conn = ses->server->dfs_conn;
> > +       ctx->ignore_signature = ses->server->ignore_signature;
> > +       ctx->leaf_fullpath = ses->server->leaf_fullpath;
> > +       ctx->rootfs = ses->server->noblockcnt;
> > +       ctx->retrans = ses->server->retrans;
> >
> >         /*
> >          * This will be used for encoding/decoding user/domain/pw
> > --
> > 2.43.0
> >
> >
>
>
> --
> Thanks,
>
> Steve





[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux