diff mbox series

[v2] cifs: do not query ifaces on smb1 mounts

Message ID 20230110222321.30860-1-pc@cjr.nz (mailing list archive)
State New, archived
Headers show
Series [v2] cifs: do not query ifaces on smb1 mounts | expand

Commit Message

Paulo Alcantara Jan. 10, 2023, 10:23 p.m. UTC
Users have reported the following error on every 600 seconds
(SMB_INTERFACE_POLL_INTERVAL) when mounting SMB1 shares:

	CIFS: VFS: \\srv\share error -5 on ioctl to get interface list

It's supported only by SMB2+, so do not query network interfaces on
SMB1 mounts.

Fixes: 6e1c1c08cdf3 ("cifs: periodically query network interfaces from server")
Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
v1 -> v2:
	remove cifs_tcon::iface_query_poll and then check version and
	server's capability multichannel

 fs/cifs/connect.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Steve French Jan. 10, 2023, 10:31 p.m. UTC | #1
tentatively merged into cifs-2.6.git for-next pending testing and
additional reviews

On Tue, Jan 10, 2023 at 4:23 PM Paulo Alcantara <pc@cjr.nz> wrote:
>
> Users have reported the following error on every 600 seconds
> (SMB_INTERFACE_POLL_INTERVAL) when mounting SMB1 shares:
>
>         CIFS: VFS: \\srv\share error -5 on ioctl to get interface list
>
> It's supported only by SMB2+, so do not query network interfaces on
> SMB1 mounts.
>
> Fixes: 6e1c1c08cdf3 ("cifs: periodically query network interfaces from server")
> Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
> Signed-off-by: Steve French <stfrench@microsoft.com>
> ---
> v1 -> v2:
>         remove cifs_tcon::iface_query_poll and then check version and
>         server's capability multichannel
>
>  fs/cifs/connect.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index d371259d6808..b2a04b4e89a5 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2606,11 +2606,14 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
>         INIT_LIST_HEAD(&tcon->pending_opens);
>         tcon->status = TID_GOOD;
>
> -       /* schedule query interfaces poll */
>         INIT_DELAYED_WORK(&tcon->query_interfaces,
>                           smb2_query_server_interfaces);
> -       queue_delayed_work(cifsiod_wq, &tcon->query_interfaces,
> -                          (SMB_INTERFACE_POLL_INTERVAL * HZ));
> +       if (ses->server->dialect >= SMB30_PROT_ID &&
> +           (ses->server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) {
> +               /* schedule query interfaces poll */
> +               queue_delayed_work(cifsiod_wq, &tcon->query_interfaces,
> +                                  (SMB_INTERFACE_POLL_INTERVAL * HZ));
> +       }
>
>         spin_lock(&cifs_tcp_ses_lock);
>         list_add(&tcon->tcon_list, &ses->tcon_list);
> --
> 2.39.0
>
Tom Talpey Jan. 11, 2023, 1:42 a.m. UTC | #2
On 1/10/2023 5:23 PM, Paulo Alcantara wrote:
> Users have reported the following error on every 600 seconds
> (SMB_INTERFACE_POLL_INTERVAL) when mounting SMB1 shares:
> 
> 	CIFS: VFS: \\srv\share error -5 on ioctl to get interface list
> 
> It's supported only by SMB2+, so do not query network interfaces on
> SMB1 mounts.
> 
> Fixes: 6e1c1c08cdf3 ("cifs: periodically query network interfaces from server")
> Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
> Signed-off-by: Steve French <stfrench@microsoft.com>
> ---
> v1 -> v2:
> 	remove cifs_tcon::iface_query_poll and then check version and
> 	server's capability multichannel
> 
>   fs/cifs/connect.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index d371259d6808..b2a04b4e89a5 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2606,11 +2606,14 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
>   	INIT_LIST_HEAD(&tcon->pending_opens);
>   	tcon->status = TID_GOOD;
>   
> -	/* schedule query interfaces poll */
>   	INIT_DELAYED_WORK(&tcon->query_interfaces,
>   			  smb2_query_server_interfaces);
> -	queue_delayed_work(cifsiod_wq, &tcon->query_interfaces,
> -			   (SMB_INTERFACE_POLL_INTERVAL * HZ));
> +	if (ses->server->dialect >= SMB30_PROT_ID &&

The dialect test is actually unnecessary, because the server
global capability, indicating the support, is what's important.
But it's harmless to be explicit, so...

Reviewed-by: Tom Talpey <tom@talpey.com>

LGTM.

> +	    (ses->server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) {
> +		/* schedule query interfaces poll */
> +		queue_delayed_work(cifsiod_wq, &tcon->query_interfaces,
> +				   (SMB_INTERFACE_POLL_INTERVAL * HZ));
> +	}
>   
>   	spin_lock(&cifs_tcp_ses_lock);
>   	list_add(&tcon->tcon_list, &ses->tcon_list);
Paulo Alcantara Jan. 11, 2023, 2:24 a.m. UTC | #3
On 10 January 2023 22:42:33 GMT-03:00, Tom Talpey <tom@talpey.com> wrote:
>On 1/10/2023 5:23 PM, Paulo Alcantara wrote:
>> Users have reported the following error on every 600 seconds
>> (SMB_INTERFACE_POLL_INTERVAL) when mounting SMB1 shares:
>> 
>> 	CIFS: VFS: \\srv\share error -5 on ioctl to get interface list
>> 
>> It's supported only by SMB2+, so do not query network interfaces on
>> SMB1 mounts.
>> 
>> Fixes: 6e1c1c08cdf3 ("cifs: periodically query network interfaces from server")
>> Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
>> Signed-off-by: Steve French <stfrench@microsoft.com>
>> ---
>> v1 -> v2:
>> 	remove cifs_tcon::iface_query_poll and then check version and
>> 	server's capability multichannel
>> 
>>   fs/cifs/connect.c | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>> 
>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> index d371259d6808..b2a04b4e89a5 100644
>> --- a/fs/cifs/connect.c
>> +++ b/fs/cifs/connect.c
>> @@ -2606,11 +2606,14 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
>>   	INIT_LIST_HEAD(&tcon->pending_opens);
>>   	tcon->status = TID_GOOD;
>>   -	/* schedule query interfaces poll */
>>   	INIT_DELAYED_WORK(&tcon->query_interfaces,
>>   			  smb2_query_server_interfaces);
>> -	queue_delayed_work(cifsiod_wq, &tcon->query_interfaces,
>> -			   (SMB_INTERFACE_POLL_INTERVAL * HZ));
>> +	if (ses->server->dialect >= SMB30_PROT_ID &&
>
>The dialect test is actually unnecessary, because the server
>global capability, indicating the support, is what's important.
>But it's harmless to be explicit, so..

The dialect test is still necessary, otherwise we'd end up mixing SMB2_GLOBAL_CAP_MULTI_CHANNEL with CAP_LARGE_FILES[1] and then scheduling the worker for smb1 mounts.

I quickly tested it and the global cap test passed for smb1 mount due to CAP_LARGE_FILES being set.

[1] https://git.cjr.nz/linux.git/tree/fs/cifs/cifspdu.h#n533


>
>Reviewed-by: Tom Talpey <tom@talpey.com>
>
>LGTM.
>
>> +	    (ses->server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) {
>> +		/* schedule query interfaces poll */
>> +		queue_delayed_work(cifsiod_wq, &tcon->query_interfaces,
>> +				   (SMB_INTERFACE_POLL_INTERVAL * HZ));
>> +	}
>>     	spin_lock(&cifs_tcp_ses_lock);
>>   	list_add(&tcon->tcon_list, &ses->tcon_list);
Tom Talpey Jan. 11, 2023, 2:32 a.m. UTC | #4
On 1/10/2023 9:24 PM, Paulo Alcantara wrote:
> On 10 January 2023 22:42:33 GMT-03:00, Tom Talpey <tom@talpey.com> wrote:
>> On 1/10/2023 5:23 PM, Paulo Alcantara wrote:
>>> Users have reported the following error on every 600 seconds
>>> (SMB_INTERFACE_POLL_INTERVAL) when mounting SMB1 shares:
>>>
>>> 	CIFS: VFS: \\srv\share error -5 on ioctl to get interface list
>>>
>>> It's supported only by SMB2+, so do not query network interfaces on
>>> SMB1 mounts.
>>>
>>> Fixes: 6e1c1c08cdf3 ("cifs: periodically query network interfaces from server")
>>> Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
>>> Signed-off-by: Steve French <stfrench@microsoft.com>
>>> ---
>>> v1 -> v2:
>>> 	remove cifs_tcon::iface_query_poll and then check version and
>>> 	server's capability multichannel
>>>
>>>    fs/cifs/connect.c | 9 ++++++---
>>>    1 file changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>>> index d371259d6808..b2a04b4e89a5 100644
>>> --- a/fs/cifs/connect.c
>>> +++ b/fs/cifs/connect.c
>>> @@ -2606,11 +2606,14 @@ cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
>>>    	INIT_LIST_HEAD(&tcon->pending_opens);
>>>    	tcon->status = TID_GOOD;
>>>    -	/* schedule query interfaces poll */
>>>    	INIT_DELAYED_WORK(&tcon->query_interfaces,
>>>    			  smb2_query_server_interfaces);
>>> -	queue_delayed_work(cifsiod_wq, &tcon->query_interfaces,
>>> -			   (SMB_INTERFACE_POLL_INTERVAL * HZ));
>>> +	if (ses->server->dialect >= SMB30_PROT_ID &&
>>
>> The dialect test is actually unnecessary, because the server
>> global capability, indicating the support, is what's important.
>> But it's harmless to be explicit, so..
> 
> The dialect test is still necessary, otherwise we'd end up mixing SMB2_GLOBAL_CAP_MULTI_CHANNEL with CAP_LARGE_FILES[1] and then scheduling the worker for smb1 mounts.

Oh, yuck.

OK.

> I quickly tested it and the global cap test passed for smb1 mount due to CAP_LARGE_FILES being set.
> 
> [1] https://git.cjr.nz/linux.git/tree/fs/cifs/cifspdu.h#n533
> 
> 
>>
>> Reviewed-by: Tom Talpey <tom@talpey.com>
>>
>> LGTM.
>>
>>> +	    (ses->server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) {
>>> +		/* schedule query interfaces poll */
>>> +		queue_delayed_work(cifsiod_wq, &tcon->query_interfaces,
>>> +				   (SMB_INTERFACE_POLL_INTERVAL * HZ));
>>> +	}
>>>      	spin_lock(&cifs_tcp_ses_lock);
>>>    	list_add(&tcon->tcon_list, &ses->tcon_list);
> 
>
diff mbox series

Patch

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index d371259d6808..b2a04b4e89a5 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2606,11 +2606,14 @@  cifs_get_tcon(struct cifs_ses *ses, struct smb3_fs_context *ctx)
 	INIT_LIST_HEAD(&tcon->pending_opens);
 	tcon->status = TID_GOOD;
 
-	/* schedule query interfaces poll */
 	INIT_DELAYED_WORK(&tcon->query_interfaces,
 			  smb2_query_server_interfaces);
-	queue_delayed_work(cifsiod_wq, &tcon->query_interfaces,
-			   (SMB_INTERFACE_POLL_INTERVAL * HZ));
+	if (ses->server->dialect >= SMB30_PROT_ID &&
+	    (ses->server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL)) {
+		/* schedule query interfaces poll */
+		queue_delayed_work(cifsiod_wq, &tcon->query_interfaces,
+				   (SMB_INTERFACE_POLL_INTERVAL * HZ));
+	}
 
 	spin_lock(&cifs_tcp_ses_lock);
 	list_add(&tcon->tcon_list, &ses->tcon_list);