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 |
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 >
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);
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);
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 --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);