[1/7] cifs: handle servers that still advertise multichannel after disabling

From: Shyam Prasad N <sprasad@microsoft.com>

Some servers like Azure SMB servers always advertise multichannel
capability in server capabilities list. Such servers return error
STATUS_NOT_IMPLEMENTED for ioctl calls to query server interfaces,
and expect clients to consider that as a sign that they do not support
multichannel.

We already handled this at mount time. Soon after the tree connect,
we query server interfaces. And when server returned STATUS_NOT_IMPLEMENTED,
we kept interface list as empty. When cifs_try_adding_channels gets
called, it would not find any interfaces, so will not add channels.

For the case where an active multichannel mount exists, and multichannel
is disabled by such a server, this change will now allow the client
to disable secondary channels on the mount. It will check the return
status of query server interfaces call soon after a tree reconnect.
If the return status is EOPNOTSUPP, then instead of the check to add
more channels, we'll disable the secondary channels instead.

For better code reuse, this change also moves the common code for
disabling multichannel to a helper function.

Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
---
 fs/smb/client/smb2pdu.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Has a duplicate label warning that I corrected by removing the
following part of patch 1

@@ -441,6 +439,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
        }
 skip_add_channels:

+skip_add_channels:
        if (smb2_command != SMB2_INTERNAL_CMD)
                mod_delayed_work(cifsiod_wq, &server->reconnect, 0);

On Sat, Jan 20, 2024 at 9:33 PM <nspmangalore@gmail.com> wrote:
>
> From: Shyam Prasad N <sprasad@microsoft.com>
>
> Some servers like Azure SMB servers always advertise multichannel
> capability in server capabilities list. Such servers return error
> STATUS_NOT_IMPLEMENTED for ioctl calls to query server interfaces,
> and expect clients to consider that as a sign that they do not support
> multichannel.
>
> We already handled this at mount time. Soon after the tree connect,
> we query server interfaces. And when server returned STATUS_NOT_IMPLEMENTED,
> we kept interface list as empty. When cifs_try_adding_channels gets
> called, it would not find any interfaces, so will not add channels.
>
> For the case where an active multichannel mount exists, and multichannel
> is disabled by such a server, this change will now allow the client
> to disable secondary channels on the mount. It will check the return
> status of query server interfaces call soon after a tree reconnect.
> If the return status is EOPNOTSUPP, then instead of the check to add
> more channels, we'll disable the secondary channels instead.
>
> For better code reuse, this change also moves the common code for
> disabling multichannel to a helper function.
>
> Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
> ---
>  fs/smb/client/smb2pdu.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
> index 288199f0b987..5126f5f97969 100644
> --- a/fs/smb/client/smb2pdu.c
> +++ b/fs/smb/client/smb2pdu.c
> @@ -167,7 +167,7 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses,
>
>         if (SERVER_IS_CHAN(server)) {
>                 cifs_dbg(VFS,
> -                       "server %s does not support multichannel anymore. Skip secondary channel\n",
> +                        "server %s does not support multichannel anymore. Skip secondary channel\n",
>                          ses->server->hostname);
>
>                 spin_lock(&ses->chan_lock);
> @@ -195,15 +195,13 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses,
>                 pserver = server->primary_server;
>                 cifs_signal_cifsd_for_reconnect(pserver, false);
>  skip_terminate:
> -               mutex_unlock(&ses->session_mutex);
>                 return -EHOSTDOWN;
>         }
>
>         cifs_server_dbg(VFS,
> -               "server does not support multichannel anymore. Disable all other channels\n");
> +                       "server does not support multichannel anymore. Disable all other channels\n");
>         cifs_disable_secondary_channels(ses);
>
> -
>         return 0;
>  }
>
> @@ -441,6 +439,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
>         }
>  skip_add_channels:
>
> +skip_add_channels:
>         if (smb2_command != SMB2_INTERNAL_CMD)
>                 mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
>
> --
> 2.34.1
>

On Sun, Jan 21, 2024 at 9:32 AM Steve French <smfrench@gmail.com> wrote:
>
> Has a duplicate label warning that I corrected by removing the
> following part of patch 1
>
> @@ -441,6 +439,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
>         }
>  skip_add_channels:
>
> +skip_add_channels:
>         if (smb2_command != SMB2_INTERNAL_CMD)
>                 mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
>
> On Sat, Jan 20, 2024 at 9:33 PM <nspmangalore@gmail.com> wrote:
> >
> > From: Shyam Prasad N <sprasad@microsoft.com>
> >
> > Some servers like Azure SMB servers always advertise multichannel
> > capability in server capabilities list. Such servers return error
> > STATUS_NOT_IMPLEMENTED for ioctl calls to query server interfaces,
> > and expect clients to consider that as a sign that they do not support
> > multichannel.
> >
> > We already handled this at mount time. Soon after the tree connect,
> > we query server interfaces. And when server returned STATUS_NOT_IMPLEMENTED,
> > we kept interface list as empty. When cifs_try_adding_channels gets
> > called, it would not find any interfaces, so will not add channels.
> >
> > For the case where an active multichannel mount exists, and multichannel
> > is disabled by such a server, this change will now allow the client
> > to disable secondary channels on the mount. It will check the return
> > status of query server interfaces call soon after a tree reconnect.
> > If the return status is EOPNOTSUPP, then instead of the check to add
> > more channels, we'll disable the secondary channels instead.
> >
> > For better code reuse, this change also moves the common code for
> > disabling multichannel to a helper function.
> >
> > Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
> > ---
> >  fs/smb/client/smb2pdu.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
> > index 288199f0b987..5126f5f97969 100644
> > --- a/fs/smb/client/smb2pdu.c
> > +++ b/fs/smb/client/smb2pdu.c
> > @@ -167,7 +167,7 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses,
> >
> >         if (SERVER_IS_CHAN(server)) {
> >                 cifs_dbg(VFS,
> > -                       "server %s does not support multichannel anymore. Skip secondary channel\n",
> > +                        "server %s does not support multichannel anymore. Skip secondary channel\n",
> >                          ses->server->hostname);
> >
> >                 spin_lock(&ses->chan_lock);
> > @@ -195,15 +195,13 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses,
> >                 pserver = server->primary_server;
> >                 cifs_signal_cifsd_for_reconnect(pserver, false);
> >  skip_terminate:
> > -               mutex_unlock(&ses->session_mutex);
> >                 return -EHOSTDOWN;
> >         }
> >
> >         cifs_server_dbg(VFS,
> > -               "server does not support multichannel anymore. Disable all other channels\n");
> > +                       "server does not support multichannel anymore. Disable all other channels\n");
> >         cifs_disable_secondary_channels(ses);
> >
> > -
> >         return 0;
> >  }
> >
> > @@ -441,6 +439,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
> >         }
> >  skip_add_channels:
> >
> > +skip_add_channels:
> >         if (smb2_command != SMB2_INTERNAL_CMD)
> >                 mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
> >
> > --
> > 2.34.1
> >
>
>
> --
> Thanks,
>
> Steve

Please ignore this one. I'll send an updated patch.