| Message ID | 20230109164146.1910-1-pc@cjr.nz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cifs: do not query ifaces on smb1 mounts | expand |
On 1/9/2023 11:41 AM, 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> > --- > fs/cifs/cifsglob.h | 1 + > fs/cifs/connect.c | 18 ++++++++++++------ > 2 files changed, 13 insertions(+), 6 deletions(-) > > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h > index cfdd5bf701a1..931e9d5b21f4 100644 > --- a/fs/cifs/cifsglob.h > +++ b/fs/cifs/cifsglob.h > @@ -1240,6 +1240,7 @@ struct cifs_tcon { > #ifdef CONFIG_CIFS_DFS_UPCALL > struct list_head ulist; /* cache update list */ > #endif > + bool iface_query_poll:1; Why add such a special-case flag, instead of simply checking the dialect, or (betyter) the server's multichannel capability attribute? It seems fragile and untestable to set a flag like this, especially in the tcon, which has nothing to do with supporting the multichannel fsctl. Tom. > struct delayed_work query_interfaces; /* query interfaces workqueue job */ > }; > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index d371259d6808..164beb365bfe 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -2366,7 +2366,10 @@ cifs_put_tcon(struct cifs_tcon *tcon) > spin_unlock(&cifs_tcp_ses_lock); > > /* cancel polling of interfaces */ > - cancel_delayed_work_sync(&tcon->query_interfaces); > + if (tcon->iface_query_poll) { > + tcon->iface_query_poll = false; > + cancel_delayed_work_sync(&tcon->query_interfaces); > + } > > if (tcon->use_witness) { > int rc; > @@ -2606,11 +2609,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 (!is_smb1_server(ses->server)) { > + /* schedule query interfaces poll */ > + tcon->iface_query_poll = true; > + INIT_DELAYED_WORK(&tcon->query_interfaces, > + smb2_query_server_interfaces); > + 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 <tom@talpey.com> writes: > On 1/9/2023 11:41 AM, 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> >> --- >> fs/cifs/cifsglob.h | 1 + >> fs/cifs/connect.c | 18 ++++++++++++------ >> 2 files changed, 13 insertions(+), 6 deletions(-) >> >> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h >> index cfdd5bf701a1..931e9d5b21f4 100644 >> --- a/fs/cifs/cifsglob.h >> +++ b/fs/cifs/cifsglob.h >> @@ -1240,6 +1240,7 @@ struct cifs_tcon { >> #ifdef CONFIG_CIFS_DFS_UPCALL >> struct list_head ulist; /* cache update list */ >> #endif >> + bool iface_query_poll:1; > > Why add such a special-case flag, instead of simply checking the > dialect, or (betyter) the server's multichannel capability attribute? > > It seems fragile and untestable to set a flag like this, especially > in the tcon, which has nothing to do with supporting the multichannel > fsctl. Makes sense. I'll fix it in v2. Thanks.
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index cfdd5bf701a1..931e9d5b21f4 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -1240,6 +1240,7 @@ struct cifs_tcon { #ifdef CONFIG_CIFS_DFS_UPCALL struct list_head ulist; /* cache update list */ #endif + bool iface_query_poll:1; struct delayed_work query_interfaces; /* query interfaces workqueue job */ }; diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index d371259d6808..164beb365bfe 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -2366,7 +2366,10 @@ cifs_put_tcon(struct cifs_tcon *tcon) spin_unlock(&cifs_tcp_ses_lock); /* cancel polling of interfaces */ - cancel_delayed_work_sync(&tcon->query_interfaces); + if (tcon->iface_query_poll) { + tcon->iface_query_poll = false; + cancel_delayed_work_sync(&tcon->query_interfaces); + } if (tcon->use_witness) { int rc; @@ -2606,11 +2609,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 (!is_smb1_server(ses->server)) { + /* schedule query interfaces poll */ + tcon->iface_query_poll = true; + INIT_DELAYED_WORK(&tcon->query_interfaces, + smb2_query_server_interfaces); + 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);
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> --- fs/cifs/cifsglob.h | 1 + fs/cifs/connect.c | 18 ++++++++++++------ 2 files changed, 13 insertions(+), 6 deletions(-)