linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts
@ 2020-02-24 13:14 Stefan Metzmacher
  2020-02-24 13:14 ` [PATCH v1 01/13] cifs: call wake_up(&server->response_q) inside of cifs_reconnect() Stefan Metzmacher
                   ` (13 more replies)
  0 siblings, 14 replies; 22+ messages in thread
From: Stefan Metzmacher @ 2020-02-24 13:14 UTC (permalink / raw)
  To: linux-cifs

Commit b0dd940e582b6a6 ("cifs: fail i/o on soft mounts if sessionsetup errors out")
didn't fix the whole problem. New i/o requests still try reconnects on the wire.
But we need to avoid that in order to avoid locking out the user account.

The first patches consolidate the retry/reconnect handling between SMB1
and SMB2/3.

Then we map STATUS_ACCOUNT_LOCKED_OUT also to -EACCES, as it's the
follow up of STATUS_LOGON_FAILURE when the wrong password was used too
often.

Finally if we get -EACCES for a session setup on a soft mount, we'll
turn the session into a CifsInvalidCredentials state and return
-EKEYREVOKED for all i/o on the session, without trying to reconnect
the network connection.

In future we could add ways to recover such sessions.
cifs_demultiplex_thread() is already prepared for that.
I tested this with a trivial /proc/fs/cifs/ResetInvalidCredentials
to reset all sessions, but I don't think that's something that
should be used in production.

For now the only way out is 'umount' (as before).

metze


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v1 01/13] cifs: call wake_up(&server->response_q) inside of cifs_reconnect()
  2020-02-24 13:14 [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Stefan Metzmacher
@ 2020-02-24 13:14 ` Stefan Metzmacher
  2020-02-24 13:14 ` [PATCH v1 02/13] cifs: use mod_delayed_work() for &server->reconnect if already queued Stefan Metzmacher
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: Stefan Metzmacher @ 2020-02-24 13:14 UTC (permalink / raw)
  To: linux-cifs; +Cc: Stefan Metzmacher

This means it's consistently called and the callers don't need to
care about it.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 fs/cifs/cifssmb.c | 1 -
 fs/cifs/connect.c | 7 ++-----
 fs/cifs/smb2ops.c | 3 ---
 3 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 3c89569e7210..01206e12975a 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -1590,7 +1590,6 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 	if (server->ops->is_session_expired &&
 	    server->ops->is_session_expired(buf)) {
 		cifs_reconnect(server);
-		wake_up(&server->response_q);
 		return -1;
 	}
 
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 4804d1df8c1c..25df928b3ca0 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -537,6 +537,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
 		dfs_cache_free_tgts(&tgt_list);
 		put_tcp_super(sb);
 #endif
+		wake_up(&server->response_q);
 		return rc;
 	} else
 		server->tcpStatus = CifsNeedReconnect;
@@ -671,6 +672,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
 	if (server->tcpStatus == CifsNeedNegotiate)
 		mod_delayed_work(cifsiod_wq, &server->echo, 0);
 
+	wake_up(&server->response_q);
 	return rc;
 }
 
@@ -765,7 +767,6 @@ server_unresponsive(struct TCP_Server_Info *server)
 		cifs_server_dbg(VFS, "has not responded in %lu seconds. Reconnecting...\n",
 			 (3 * server->echo_interval) / HZ);
 		cifs_reconnect(server);
-		wake_up(&server->response_q);
 		return true;
 	}
 
@@ -898,7 +899,6 @@ is_smb_response(struct TCP_Server_Info *server, unsigned char type)
 		 */
 		cifs_set_port((struct sockaddr *)&server->dstaddr, CIFS_PORT);
 		cifs_reconnect(server);
-		wake_up(&server->response_q);
 		break;
 	default:
 		cifs_server_dbg(VFS, "RFC 1002 unknown response type 0x%x\n", type);
@@ -1070,7 +1070,6 @@ standard_receive3(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 		server->vals->header_preamble_size) {
 		cifs_server_dbg(VFS, "SMB response too long (%u bytes)\n", pdu_length);
 		cifs_reconnect(server);
-		wake_up(&server->response_q);
 		return -ECONNABORTED;
 	}
 
@@ -1118,7 +1117,6 @@ cifs_handle_standard(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 	if (server->ops->is_session_expired &&
 	    server->ops->is_session_expired(buf)) {
 		cifs_reconnect(server);
-		wake_up(&server->response_q);
 		return -1;
 	}
 
@@ -1212,7 +1210,6 @@ cifs_demultiplex_thread(void *p)
 			cifs_server_dbg(VFS, "SMB response too short (%u bytes)\n",
 				 server->pdu_size);
 			cifs_reconnect(server);
-			wake_up(&server->response_q);
 			continue;
 		}
 
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index e47190cae163..bb2227561f5d 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -4148,7 +4148,6 @@ handle_read_data(struct TCP_Server_Info *server, struct mid_q_entry *mid,
 	if (server->ops->is_session_expired &&
 	    server->ops->is_session_expired(buf)) {
 		cifs_reconnect(server);
-		wake_up(&server->response_q);
 		return -1;
 	}
 
@@ -4512,14 +4511,12 @@ smb3_receive_transform(struct TCP_Server_Info *server,
 		cifs_server_dbg(VFS, "Transform message is too small (%u)\n",
 			 pdu_length);
 		cifs_reconnect(server);
-		wake_up(&server->response_q);
 		return -ECONNABORTED;
 	}
 
 	if (pdu_length < orig_len + sizeof(struct smb2_transform_hdr)) {
 		cifs_server_dbg(VFS, "Transform message is broken\n");
 		cifs_reconnect(server);
-		wake_up(&server->response_q);
 		return -ECONNABORTED;
 	}
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 02/13] cifs: use mod_delayed_work() for &server->reconnect if already queued
  2020-02-24 13:14 [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Stefan Metzmacher
  2020-02-24 13:14 ` [PATCH v1 01/13] cifs: call wake_up(&server->response_q) inside of cifs_reconnect() Stefan Metzmacher
@ 2020-02-24 13:14 ` Stefan Metzmacher
  2020-02-24 13:15 ` [PATCH v1 03/13] cifs: make use of cap_unix(ses) in cifs_reconnect_tcon() Stefan Metzmacher
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: Stefan Metzmacher @ 2020-02-24 13:14 UTC (permalink / raw)
  To: linux-cifs; +Cc: Stefan Metzmacher

mod_delayed_work() is safer than queue_delayed_work() if there's a
chance that the work is already in the queue.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 fs/cifs/smb2pdu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 1234f9ccab03..2495c3021772 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -378,7 +378,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon)
 	}
 
 	if (smb2_command != SMB2_INTERNAL_CMD)
-		queue_delayed_work(cifsiod_wq, &server->reconnect, 0);
+		mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
 
 	atomic_inc(&tconInfoReconnectCount);
 out:
@@ -3558,7 +3558,7 @@ SMB2_echo(struct TCP_Server_Info *server)
 
 	if (server->tcpStatus == CifsNeedNegotiate) {
 		/* No need to send echo on newly established connections */
-		queue_delayed_work(cifsiod_wq, &server->reconnect, 0);
+		mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
 		return rc;
 	}
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 03/13] cifs: make use of cap_unix(ses) in cifs_reconnect_tcon()
  2020-02-24 13:14 [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Stefan Metzmacher
  2020-02-24 13:14 ` [PATCH v1 01/13] cifs: call wake_up(&server->response_q) inside of cifs_reconnect() Stefan Metzmacher
  2020-02-24 13:14 ` [PATCH v1 02/13] cifs: use mod_delayed_work() for &server->reconnect if already queued Stefan Metzmacher
@ 2020-02-24 13:15 ` Stefan Metzmacher
  2020-02-24 18:12   ` Aurélien Aptel
  2020-02-24 13:15 ` [PATCH v1 04/13] cifs: split out a cifs_connect_session_locked() helper function Stefan Metzmacher
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 22+ messages in thread
From: Stefan Metzmacher @ 2020-02-24 13:15 UTC (permalink / raw)
  To: linux-cifs; +Cc: Stefan Metzmacher

cap_unix(ses) defaults to false for SMB2.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 fs/cifs/cifssmb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 01206e12975a..d2b92770384b 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -320,7 +320,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
 	atomic_inc(&tconInfoReconnectCount);
 
 	/* tell server Unix caps we support */
-	if (ses->capabilities & CAP_UNIX)
+	if (cap_unix(ses))
 		reset_cifs_unix_caps(0, tcon, NULL, NULL);
 
 	/*
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 04/13] cifs: split out a cifs_connect_session_locked() helper function
  2020-02-24 13:14 [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Stefan Metzmacher
                   ` (2 preceding siblings ...)
  2020-02-24 13:15 ` [PATCH v1 03/13] cifs: make use of cap_unix(ses) in cifs_reconnect_tcon() Stefan Metzmacher
@ 2020-02-24 13:15 ` Stefan Metzmacher
  2020-03-06 17:05   ` Aurélien Aptel
  2020-02-24 13:15 ` [PATCH v1 05/13] cifs: merge __{cifs,smb2}_reconnect[_tcon]() into cifs_tree_connect() Stefan Metzmacher
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 22+ messages in thread
From: Stefan Metzmacher @ 2020-02-24 13:15 UTC (permalink / raw)
  To: linux-cifs; +Cc: Stefan Metzmacher

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 fs/cifs/cifsproto.h |  8 +++----
 fs/cifs/cifssmb.c   | 13 +++---------
 fs/cifs/connect.c   | 51 +++++++++++++++++++++++++++++----------------
 fs/cifs/sess.c      |  9 ++++----
 fs/cifs/smb2pdu.c   | 21 +++++--------------
 5 files changed, 49 insertions(+), 53 deletions(-)

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 89eaaf46d1ca..e2624ebad189 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -263,10 +263,10 @@ extern void cifs_move_llist(struct list_head *source, struct list_head *dest);
 extern void cifs_free_llist(struct list_head *llist);
 extern void cifs_del_lock_waiters(struct cifsLockInfo *lock);
 
-extern int cifs_negotiate_protocol(const unsigned int xid,
-				   struct cifs_ses *ses);
-extern int cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
-			      struct nls_table *nls_info);
+extern int cifs_connect_session_locked(const unsigned int xid,
+				       struct cifs_ses *ses,
+				       struct nls_table *nls_info,
+				       bool retry);
 extern int cifs_enable_signing(struct TCP_Server_Info *server, bool mnt_sign_required);
 extern int CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses);
 
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index d2b92770384b..dcb009fade8c 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -291,16 +291,9 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
 	 * and the server never sends an answer the socket will be closed
 	 * and tcpStatus set to reconnect.
 	 */
-	if (server->tcpStatus == CifsNeedReconnect) {
-		rc = -EHOSTDOWN;
-		mutex_unlock(&ses->session_mutex);
-		goto out;
-	}
-
-	rc = cifs_negotiate_protocol(0, ses);
-	if (rc == 0 && ses->need_reconnect)
-		rc = cifs_setup_session(0, ses, nls_codepage);
-
+	rc = cifs_connect_session_locked(0, ses,
+					 nls_codepage,
+					 tcon->retry);
 	/* do we need to reconnect tcon? */
 	if (rc || !tcon->need_reconnect) {
 		mutex_unlock(&ses->session_mutex);
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 25df928b3ca0..34269ef40774 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -3288,7 +3288,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
 			 ses->status);
 
 		mutex_lock(&ses->session_mutex);
-		rc = cifs_negotiate_protocol(xid, ses);
+		rc = cifs_connect_session_locked(xid, ses,
+						 volume_info->local_nls,
+						 true /* retry */);
 		if (rc) {
 			mutex_unlock(&ses->session_mutex);
 			/* problem -- put our ses reference */
@@ -3296,18 +3298,6 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
 			free_xid(xid);
 			return ERR_PTR(rc);
 		}
-		if (ses->need_reconnect) {
-			cifs_dbg(FYI, "Session needs reconnect\n");
-			rc = cifs_setup_session(xid, ses,
-						volume_info->local_nls);
-			if (rc) {
-				mutex_unlock(&ses->session_mutex);
-				/* problem -- put our reference */
-				cifs_put_smb_ses(ses);
-				free_xid(xid);
-				return ERR_PTR(rc);
-			}
-		}
 		mutex_unlock(&ses->session_mutex);
 
 		/* existing SMB ses has a server reference already */
@@ -3359,9 +3349,10 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
 	ses->chan_count = 1;
 	ses->chan_max = volume_info->multichannel ? volume_info->max_channels:1;
 
-	rc = cifs_negotiate_protocol(xid, ses);
-	if (!rc)
-		rc = cifs_setup_session(xid, ses, volume_info->local_nls);
+	ses->need_reconnect = true;
+	rc = cifs_connect_session_locked(xid, ses,
+					 volume_info->local_nls,
+					 true /* retry */);
 
 	/* each channel uses a different signing key */
 	memcpy(ses->chans[0].signkey, ses->smb3signingkey,
@@ -5240,7 +5231,7 @@ cifs_umount(struct cifs_sb_info *cifs_sb)
 	call_rcu(&cifs_sb->rcu, delayed_free);
 }
 
-int
+static int
 cifs_negotiate_protocol(const unsigned int xid, struct cifs_ses *ses)
 {
 	int rc = 0;
@@ -5266,7 +5257,7 @@ cifs_negotiate_protocol(const unsigned int xid, struct cifs_ses *ses)
 	return rc;
 }
 
-int
+static int
 cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
 		   struct nls_table *nls_info)
 {
@@ -5299,6 +5290,30 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
 	return rc;
 }
 
+int
+cifs_connect_session_locked(const unsigned int xid,
+			    struct cifs_ses *ses,
+			    struct nls_table *nls_info,
+			    bool retry)
+{
+	int rc;
+
+	if (ses->server->tcpStatus == CifsNeedReconnect) {
+		return -EHOSTDOWN;
+	}
+
+	rc = cifs_negotiate_protocol(xid, ses);
+	if (!rc && (ses->need_reconnect || ses->binding)) {
+		cifs_dbg(FYI, "Session needs reconnect\n");
+		rc = cifs_setup_session(xid, ses, nls_info);
+		if ((rc == -EACCES) && !retry) {
+			return -EHOSTDOWN;
+		}
+	}
+
+	return rc;
+}
+
 static int
 cifs_set_vol_auth(struct smb_vol *vol, struct cifs_ses *ses)
 {
diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index 43a88e26d26b..d38a6769d8aa 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -252,11 +252,10 @@ cifs_ses_add_channel(struct cifs_ses *ses, struct cifs_server_iface *iface)
 	}
 
 	ses->binding = true;
-	rc = cifs_negotiate_protocol(xid, ses);
-	if (rc)
-		goto out;
-
-	rc = cifs_setup_session(xid, ses, vol.local_nls);
+	rc = cifs_connect_session_locked(xid,
+					 ses,
+					 vol.local_nls,
+					 true /* retry */);
 	if (rc)
 		goto out;
 
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 2495c3021772..a15a53d2e808 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -343,21 +343,10 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon)
 	 * and the server never sends an answer the socket will be closed
 	 * and tcpStatus set to reconnect.
 	 */
-	if (server->tcpStatus == CifsNeedReconnect) {
-		rc = -EHOSTDOWN;
-		mutex_unlock(&tcon->ses->session_mutex);
-		goto out;
-	}
-
-	rc = cifs_negotiate_protocol(0, tcon->ses);
-	if (!rc && tcon->ses->need_reconnect) {
-		rc = cifs_setup_session(0, tcon->ses, nls_codepage);
-		if ((rc == -EACCES) && !tcon->retry) {
-			rc = -EHOSTDOWN;
-			mutex_unlock(&tcon->ses->session_mutex);
-			goto failed;
-		}
-	}
+	rc = cifs_connect_session_locked(0, tcon->ses,
+					 nls_codepage,
+					 tcon->retry);
+	/* do we need to reconnect tcon? */
 	if (rc || !tcon->need_reconnect) {
 		mutex_unlock(&tcon->ses->session_mutex);
 		goto out;
@@ -402,7 +391,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon)
 	case SMB2_SET_INFO:
 		rc = -EAGAIN;
 	}
-failed:
+
 	unload_nls(nls_codepage);
 	return rc;
 }
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 05/13] cifs: merge __{cifs,smb2}_reconnect[_tcon]() into cifs_tree_connect()
  2020-02-24 13:14 [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Stefan Metzmacher
                   ` (3 preceding siblings ...)
  2020-02-24 13:15 ` [PATCH v1 04/13] cifs: split out a cifs_connect_session_locked() helper function Stefan Metzmacher
@ 2020-02-24 13:15 ` Stefan Metzmacher
  2020-03-06 16:46   ` Aurélien Aptel
  2020-02-24 13:15 ` [PATCH v1 06/13] cifs: abstract cifs_tcon_reconnect() out of smb2_reconnect() Stefan Metzmacher
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 22+ messages in thread
From: Stefan Metzmacher @ 2020-02-24 13:15 UTC (permalink / raw)
  To: linux-cifs; +Cc: Stefan Metzmacher

They were identical execpt to CIFSTCon() vs. SMB2_tcon().
These are also available via ops->tree_connect().

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 fs/cifs/cifsproto.h |  3 ++
 fs/cifs/cifssmb.c   | 82 +------------------------------------------
 fs/cifs/connect.c   | 84 +++++++++++++++++++++++++++++++++++++++++++++
 fs/cifs/smb2pdu.c   | 82 +------------------------------------------
 4 files changed, 89 insertions(+), 162 deletions(-)

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index e2624ebad189..4c93007e44c0 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -263,6 +263,9 @@ extern void cifs_move_llist(struct list_head *source, struct list_head *dest);
 extern void cifs_free_llist(struct list_head *llist);
 extern void cifs_del_lock_waiters(struct cifsLockInfo *lock);
 
+extern int cifs_tree_connect(const unsigned int xid,
+			     struct cifs_tcon *tcon,
+			     const struct nls_table *nlsc);
 extern int cifs_connect_session_locked(const unsigned int xid,
 				       struct cifs_ses *ses,
 				       struct nls_table *nls_info,
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index dcb009fade8c..412d141e1adc 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -124,86 +124,6 @@ cifs_mark_open_files_invalid(struct cifs_tcon *tcon)
 	 */
 }
 
-#ifdef CONFIG_CIFS_DFS_UPCALL
-static int __cifs_reconnect_tcon(const struct nls_table *nlsc,
-				 struct cifs_tcon *tcon)
-{
-	int rc;
-	struct dfs_cache_tgt_list tl;
-	struct dfs_cache_tgt_iterator *it = NULL;
-	char *tree;
-	const char *tcp_host;
-	size_t tcp_host_len;
-	const char *dfs_host;
-	size_t dfs_host_len;
-
-	tree = kzalloc(MAX_TREE_SIZE, GFP_KERNEL);
-	if (!tree)
-		return -ENOMEM;
-
-	if (tcon->ipc) {
-		scnprintf(tree, MAX_TREE_SIZE, "\\\\%s\\IPC$",
-			  tcon->ses->server->hostname);
-		rc = CIFSTCon(0, tcon->ses, tree, tcon, nlsc);
-		goto out;
-	}
-
-	if (!tcon->dfs_path) {
-		rc = CIFSTCon(0, tcon->ses, tcon->treeName, tcon, nlsc);
-		goto out;
-	}
-
-	rc = dfs_cache_noreq_find(tcon->dfs_path + 1, NULL, &tl);
-	if (rc)
-		goto out;
-
-	extract_unc_hostname(tcon->ses->server->hostname, &tcp_host,
-			     &tcp_host_len);
-
-	for (it = dfs_cache_get_tgt_iterator(&tl); it;
-	     it = dfs_cache_get_next_tgt(&tl, it)) {
-		const char *tgt = dfs_cache_get_tgt_name(it);
-
-		extract_unc_hostname(tgt, &dfs_host, &dfs_host_len);
-
-		if (dfs_host_len != tcp_host_len
-		    || strncasecmp(dfs_host, tcp_host, dfs_host_len) != 0) {
-			cifs_dbg(FYI, "%s: skipping %.*s, doesn't match %.*s",
-				 __func__,
-				 (int)dfs_host_len, dfs_host,
-				 (int)tcp_host_len, tcp_host);
-			continue;
-		}
-
-		scnprintf(tree, MAX_TREE_SIZE, "\\%s", tgt);
-
-		rc = CIFSTCon(0, tcon->ses, tree, tcon, nlsc);
-		if (!rc)
-			break;
-		if (rc == -EREMOTE)
-			break;
-	}
-
-	if (!rc) {
-		if (it)
-			rc = dfs_cache_noreq_update_tgthint(tcon->dfs_path + 1,
-							    it);
-		else
-			rc = -ENOENT;
-	}
-	dfs_cache_free_tgts(&tl);
-out:
-	kfree(tree);
-	return rc;
-}
-#else
-static inline int __cifs_reconnect_tcon(const struct nls_table *nlsc,
-					struct cifs_tcon *tcon)
-{
-	return CIFSTCon(0, tcon->ses, tcon->treeName, tcon, nlsc);
-}
-#endif
-
 /* reconnect the socket, tcon, and smb session if needed */
 static int
 cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
@@ -301,7 +221,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
 	}
 
 	cifs_mark_open_files_invalid(tcon);
-	rc = __cifs_reconnect_tcon(nls_codepage, tcon);
+	rc = cifs_tree_connect(0, tcon, nls_codepage);
 	mutex_unlock(&ses->session_mutex);
 	cifs_dbg(FYI, "reconnect tcon rc = %d\n", rc);
 
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 34269ef40774..c243c9a1b3d4 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -5290,6 +5290,90 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
 	return rc;
 }
 
+#ifdef CONFIG_CIFS_DFS_UPCALL
+int cifs_tree_connect(const unsigned int xid,
+		      struct cifs_tcon *tcon,
+		      const struct nls_table *nlsc)
+{
+	const struct smb_version_operations *ops = tcon->ses->server->ops;
+	int rc;
+	struct dfs_cache_tgt_list tl;
+	struct dfs_cache_tgt_iterator *it = NULL;
+	char *tree;
+	const char *tcp_host;
+	size_t tcp_host_len;
+	const char *dfs_host;
+	size_t dfs_host_len;
+
+	tree = kzalloc(MAX_TREE_SIZE, GFP_KERNEL);
+	if (!tree)
+		return -ENOMEM;
+
+	if (tcon->ipc) {
+		scnprintf(tree, MAX_TREE_SIZE, "\\\\%s\\IPC$",
+			  tcon->ses->server->hostname);
+		rc = ops->tree_connect(xid, tcon->ses, tcon->treeName, tcon, nlsc);
+		goto out;
+	}
+
+	if (!tcon->dfs_path) {
+		rc = ops->tree_connect(xid, tcon->ses, tcon->treeName, tcon, nlsc);
+		goto out;
+	}
+
+	rc = dfs_cache_noreq_find(tcon->dfs_path + 1, NULL, &tl);
+	if (rc)
+		goto out;
+
+	extract_unc_hostname(tcon->ses->server->hostname, &tcp_host,
+			     &tcp_host_len);
+
+	for (it = dfs_cache_get_tgt_iterator(&tl); it;
+	     it = dfs_cache_get_next_tgt(&tl, it)) {
+		const char *tgt = dfs_cache_get_tgt_name(it);
+
+		extract_unc_hostname(tgt, &dfs_host, &dfs_host_len);
+
+		if (dfs_host_len != tcp_host_len
+		    || strncasecmp(dfs_host, tcp_host, dfs_host_len) != 0) {
+			cifs_dbg(FYI, "%s: skipping %.*s, doesn't match %.*s",
+				 __func__,
+				 (int)dfs_host_len, dfs_host,
+				 (int)tcp_host_len, tcp_host);
+			continue;
+		}
+
+		scnprintf(tree, MAX_TREE_SIZE, "\\%s", tgt);
+
+		rc = ops->tree_connect(xid, tcon->ses, tcon->treeName, tcon, nlsc);
+		if (!rc)
+			break;
+		if (rc == -EREMOTE)
+			break;
+	}
+
+	if (!rc) {
+		if (it)
+			rc = dfs_cache_noreq_update_tgthint(tcon->dfs_path + 1,
+							    it);
+		else
+			rc = -ENOENT;
+	}
+	dfs_cache_free_tgts(&tl);
+out:
+	kfree(tree);
+	return rc;
+}
+#else
+int cifs_tree_connect(const unsigned int xid,
+		      struct cifs_tcon *tcon,
+		      const struct nls_table *nlsc)
+{
+	const struct smb_version_operations *ops = tcon->ses->server->ops;
+	return ops->tree_connect(xid, tcon->ses, tcon->treeName, tcon, nlsc);
+}
+#endif
+
 int
 cifs_connect_session_locked(const unsigned int xid,
 			    struct cifs_ses *ses,
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index a15a53d2e808..715a50ffb234 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -155,86 +155,6 @@ smb2_hdr_assemble(struct smb2_sync_hdr *shdr, __le16 smb2_cmd,
 	return;
 }
 
-#ifdef CONFIG_CIFS_DFS_UPCALL
-static int __smb2_reconnect(const struct nls_table *nlsc,
-			    struct cifs_tcon *tcon)
-{
-	int rc;
-	struct dfs_cache_tgt_list tl;
-	struct dfs_cache_tgt_iterator *it = NULL;
-	char *tree;
-	const char *tcp_host;
-	size_t tcp_host_len;
-	const char *dfs_host;
-	size_t dfs_host_len;
-
-	tree = kzalloc(MAX_TREE_SIZE, GFP_KERNEL);
-	if (!tree)
-		return -ENOMEM;
-
-	if (tcon->ipc) {
-		scnprintf(tree, MAX_TREE_SIZE, "\\\\%s\\IPC$",
-			  tcon->ses->server->hostname);
-		rc = SMB2_tcon(0, tcon->ses, tree, tcon, nlsc);
-		goto out;
-	}
-
-	if (!tcon->dfs_path) {
-		rc = SMB2_tcon(0, tcon->ses, tcon->treeName, tcon, nlsc);
-		goto out;
-	}
-
-	rc = dfs_cache_noreq_find(tcon->dfs_path + 1, NULL, &tl);
-	if (rc)
-		goto out;
-
-	extract_unc_hostname(tcon->ses->server->hostname, &tcp_host,
-			     &tcp_host_len);
-
-	for (it = dfs_cache_get_tgt_iterator(&tl); it;
-	     it = dfs_cache_get_next_tgt(&tl, it)) {
-		const char *tgt = dfs_cache_get_tgt_name(it);
-
-		extract_unc_hostname(tgt, &dfs_host, &dfs_host_len);
-
-		if (dfs_host_len != tcp_host_len
-		    || strncasecmp(dfs_host, tcp_host, dfs_host_len) != 0) {
-			cifs_dbg(FYI, "%s: skipping %.*s, doesn't match %.*s",
-				 __func__,
-				 (int)dfs_host_len, dfs_host,
-				 (int)tcp_host_len, tcp_host);
-			continue;
-		}
-
-		scnprintf(tree, MAX_TREE_SIZE, "\\%s", tgt);
-
-		rc = SMB2_tcon(0, tcon->ses, tree, tcon, nlsc);
-		if (!rc)
-			break;
-		if (rc == -EREMOTE)
-			break;
-	}
-
-	if (!rc) {
-		if (it)
-			rc = dfs_cache_noreq_update_tgthint(tcon->dfs_path + 1,
-							    it);
-		else
-			rc = -ENOENT;
-	}
-	dfs_cache_free_tgts(&tl);
-out:
-	kfree(tree);
-	return rc;
-}
-#else
-static inline int __smb2_reconnect(const struct nls_table *nlsc,
-				   struct cifs_tcon *tcon)
-{
-	return SMB2_tcon(0, tcon->ses, tcon->treeName, tcon, nlsc);
-}
-#endif
-
 static int
 smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon)
 {
@@ -356,7 +276,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon)
 	if (tcon->use_persistent)
 		tcon->need_reopen_files = true;
 
-	rc = __smb2_reconnect(nls_codepage, tcon);
+	rc = cifs_tree_connect(0, tcon, nls_codepage);
 	mutex_unlock(&tcon->ses->session_mutex);
 
 	cifs_dbg(FYI, "reconnect tcon rc = %d\n", rc);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 06/13] cifs: abstract cifs_tcon_reconnect() out of smb2_reconnect()
  2020-02-24 13:14 [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Stefan Metzmacher
                   ` (4 preceding siblings ...)
  2020-02-24 13:15 ` [PATCH v1 05/13] cifs: merge __{cifs,smb2}_reconnect[_tcon]() into cifs_tree_connect() Stefan Metzmacher
@ 2020-02-24 13:15 ` Stefan Metzmacher
  2020-03-27 17:31   ` Steve French
  2020-02-24 13:15 ` [PATCH v1 07/13] cifs: cifs_reconnect_tcon() make use of the generic cifs_tcon_reconnect() function Stefan Metzmacher
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 22+ messages in thread
From: Stefan Metzmacher @ 2020-02-24 13:15 UTC (permalink / raw)
  To: linux-cifs; +Cc: Stefan Metzmacher

cifs_tcon_reconnect() will also be used in cifs_reconnect_tcon()
with the next commit.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 fs/cifs/cifsglob.h  |   8 +++
 fs/cifs/cifsproto.h |   3 +
 fs/cifs/connect.c   | 125 ++++++++++++++++++++++++++++++++++++
 fs/cifs/smb2pdu.c   | 151 +++++++++-----------------------------------
 4 files changed, 166 insertions(+), 121 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index de82cfa44b1a..8393ed7ebf96 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1074,6 +1074,14 @@ struct cached_fid {
 	struct smb2_file_all_info file_all_info;
 };
 
+struct cifs_tcon_reconnect_params {
+	bool skip_reconnect;
+	bool exit_nodev;
+	bool early_eagain;
+	bool late_eagain;
+	bool start_timer;
+};
+
 /*
  * there is one of these for each connection to a resource on a particular
  * session
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 4c93007e44c0..64f13affdb15 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -270,6 +270,9 @@ extern int cifs_connect_session_locked(const unsigned int xid,
 				       struct cifs_ses *ses,
 				       struct nls_table *nls_info,
 				       bool retry);
+extern int cifs_tcon_reconnect(struct cifs_tcon *tcon,
+			       const struct cifs_tcon_reconnect_params *params);
+
 extern int cifs_enable_signing(struct TCP_Server_Info *server, bool mnt_sign_required);
 extern int CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses);
 
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index c243c9a1b3d4..67d2ad330f33 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -5398,6 +5398,131 @@ cifs_connect_session_locked(const unsigned int xid,
 	return rc;
 }
 
+int
+cifs_tcon_reconnect(struct cifs_tcon *tcon,
+		    const struct cifs_tcon_reconnect_params *params)
+{
+	int rc;
+	struct nls_table *nls_codepage;
+	struct cifs_ses *ses;
+	struct TCP_Server_Info *server;
+	int retries;
+
+	/*
+	 * SMB2s NegProt, SessSetup, Logoff do not have tcon yet so
+	 * check for tcp and smb session status done differently
+	 * for those three - in the calling routine.
+	 */
+	if (tcon == NULL)
+		return 0;
+
+	ses = tcon->ses;
+	server = ses->server;
+
+	if (params->skip_reconnect)
+		return 0;
+
+	if (tcon->tidStatus == CifsExiting) {
+		if (params->exit_nodev) {
+			cifs_dbg(FYI, "return ENODEV while umounting\n");
+			return -ENODEV;
+		}
+	}
+	if ((!ses) || (ses->status == CifsExiting) || (!server))
+		return -EIO;
+
+	retries = server->nr_targets;
+
+	/*
+	 * Give demultiplex thread up to 10 seconds to each target available for
+	 * reconnect -- should be greater than cifs socket timeout which is 7
+	 * seconds.
+	 */
+	while (server->tcpStatus == CifsNeedReconnect) {
+		if (params->early_eagain) {
+			return -EAGAIN;
+		}
+
+		rc = wait_event_interruptible_timeout(server->response_q,
+						      (server->tcpStatus != CifsNeedReconnect),
+						      10 * HZ);
+		if (rc < 0) {
+			cifs_dbg(FYI, "%s: aborting reconnect due to a received"
+				 " signal by the process\n", __func__);
+			return -ERESTARTSYS;
+		}
+
+		/* are we still trying to reconnect? */
+		if (server->tcpStatus != CifsNeedReconnect)
+			break;
+
+		if (retries && --retries)
+			continue;
+
+		/*
+		 * on "soft" mounts we wait once. Hard mounts keep
+		 * retrying until process is killed or server comes
+		 * back on-line
+		 */
+		if (!tcon->retry) {
+			cifs_dbg(FYI, "gave up waiting on reconnect in smb_init\n");
+			return -EHOSTDOWN;
+		}
+		retries = server->nr_targets;
+	}
+
+	if (!ses->need_reconnect && !tcon->need_reconnect)
+		return 0;
+
+	nls_codepage = load_nls_default();
+
+	/*
+	 * need to prevent multiple threads trying to simultaneously reconnect
+	 * the same SMB session
+	 */
+	mutex_lock(&ses->session_mutex);
+
+	/*
+	 * Recheck after acquire mutex. If another thread is negotiating
+	 * and the server never sends an answer the socket will be closed
+	 * and tcpStatus set to reconnect.
+	 */
+	rc = cifs_connect_session_locked(0, ses,
+					 nls_codepage,
+					 tcon->retry);
+	/* do we need to reconnect tcon? */
+	if (rc || !tcon->need_reconnect) {
+		mutex_unlock(&ses->session_mutex);
+		goto out;
+	}
+
+	cifs_mark_open_files_invalid(tcon);
+	if (tcon->use_persistent)
+		tcon->need_reopen_files = true;
+
+	rc = cifs_tree_connect(0, tcon, nls_codepage);
+	mutex_unlock(&ses->session_mutex);
+
+	cifs_dbg(FYI, "reconnect tcon rc = %d\n", rc);
+	if (rc) {
+		/* If sess reconnected but tcon didn't, something strange ... */
+		printk_once(KERN_WARNING "reconnect tcon failed rc = %d\n", rc);
+		goto out;
+	}
+
+	if (params->start_timer)
+		mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
+
+	atomic_inc(&tconInfoReconnectCount);
+out:
+	if (params->late_eagain) {
+		rc = -EAGAIN;
+	}
+
+	unload_nls(nls_codepage);
+	return rc;
+}
+
 static int
 cifs_set_vol_auth(struct smb_vol *vol, struct cifs_ses *ses)
 {
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 715a50ffb234..162fe3381f4c 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -158,139 +158,48 @@ smb2_hdr_assemble(struct smb2_sync_hdr *shdr, __le16 smb2_cmd,
 static int
 smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon)
 {
-	int rc;
-	struct nls_table *nls_codepage;
-	struct cifs_ses *ses;
-	struct TCP_Server_Info *server;
-	int retries;
-
-	/*
-	 * SMB2s NegProt, SessSetup, Logoff do not have tcon yet so
-	 * check for tcp and smb session status done differently
-	 * for those three - in the calling routine.
-	 */
-	if (tcon == NULL)
-		return 0;
+	struct cifs_tcon_reconnect_params params = {
+		.skip_reconnect = false,
+	};
 
-	if (smb2_command == SMB2_TREE_CONNECT)
-		return 0;
-
-	if (tcon->tidStatus == CifsExiting) {
-		/*
-		 * only tree disconnect, open, and write,
-		 * (and ulogoff which does not have tcon)
-		 * are allowed as we start force umount.
-		 */
-		if ((smb2_command != SMB2_WRITE) &&
-		   (smb2_command != SMB2_CREATE) &&
-		   (smb2_command != SMB2_TREE_DISCONNECT)) {
-			cifs_dbg(FYI, "can not send cmd %d while umounting\n",
-				 smb2_command);
-			return -ENODEV;
-		}
+	switch (smb2_command) {
+	case SMB2_TREE_CONNECT:
+		params.skip_reconnect = true;
+		break;
 	}
-	if ((!tcon->ses) || (tcon->ses->status == CifsExiting) ||
-	    (!tcon->ses->server))
-		return -EIO;
-
-	ses = tcon->ses;
-	server = ses->server;
-
-	retries = server->nr_targets;
 
 	/*
-	 * Give demultiplex thread up to 10 seconds to each target available for
-	 * reconnect -- should be greater than cifs socket timeout which is 7
-	 * seconds.
+	 * only tree disconnect, open, and write,
+	 * (and ulogoff which does not have tcon)
+	 * are allowed as we start force umount.
 	 */
-	while (server->tcpStatus == CifsNeedReconnect) {
-		/*
-		 * Return to caller for TREE_DISCONNECT and LOGOFF and CLOSE
-		 * here since they are implicitly done when session drops.
-		 */
-		switch (smb2_command) {
-		/*
-		 * BB Should we keep oplock break and add flush to exceptions?
-		 */
-		case SMB2_TREE_DISCONNECT:
-		case SMB2_CANCEL:
-		case SMB2_CLOSE:
-		case SMB2_OPLOCK_BREAK:
-			return -EAGAIN;
-		}
-
-		rc = wait_event_interruptible_timeout(server->response_q,
-						      (server->tcpStatus != CifsNeedReconnect),
-						      10 * HZ);
-		if (rc < 0) {
-			cifs_dbg(FYI, "%s: aborting reconnect due to a received"
-				 " signal by the process\n", __func__);
-			return -ERESTARTSYS;
-		}
-
-		/* are we still trying to reconnect? */
-		if (server->tcpStatus != CifsNeedReconnect)
-			break;
-
-		if (retries && --retries)
-			continue;
-
-		/*
-		 * on "soft" mounts we wait once. Hard mounts keep
-		 * retrying until process is killed or server comes
-		 * back on-line
-		 */
-		if (!tcon->retry) {
-			cifs_dbg(FYI, "gave up waiting on reconnect in smb_init\n");
-			return -EHOSTDOWN;
-		}
-		retries = server->nr_targets;
+	switch (smb2_command) {
+	case SMB2_WRITE:
+	case SMB2_CREATE:
+	case SMB2_TREE_DISCONNECT:
+		params.exit_nodev = true;
+		break;
 	}
 
-	if (!tcon->ses->need_reconnect && !tcon->need_reconnect)
-		return 0;
-
-	nls_codepage = load_nls_default();
-
 	/*
-	 * need to prevent multiple threads trying to simultaneously reconnect
-	 * the same SMB session
+	 * Return to caller for TREE_DISCONNECT and LOGOFF and CLOSE
+	 * here since they are implicitly done when session drops.
 	 */
-	mutex_lock(&tcon->ses->session_mutex);
-
+	switch (smb2_command) {
 	/*
-	 * Recheck after acquire mutex. If another thread is negotiating
-	 * and the server never sends an answer the socket will be closed
-	 * and tcpStatus set to reconnect.
+	 * BB Should we keep oplock break and add flush to exceptions?
 	 */
-	rc = cifs_connect_session_locked(0, tcon->ses,
-					 nls_codepage,
-					 tcon->retry);
-	/* do we need to reconnect tcon? */
-	if (rc || !tcon->need_reconnect) {
-		mutex_unlock(&tcon->ses->session_mutex);
-		goto out;
-	}
-
-	cifs_mark_open_files_invalid(tcon);
-	if (tcon->use_persistent)
-		tcon->need_reopen_files = true;
-
-	rc = cifs_tree_connect(0, tcon, nls_codepage);
-	mutex_unlock(&tcon->ses->session_mutex);
-
-	cifs_dbg(FYI, "reconnect tcon rc = %d\n", rc);
-	if (rc) {
-		/* If sess reconnected but tcon didn't, something strange ... */
-		printk_once(KERN_WARNING "reconnect tcon failed rc = %d\n", rc);
-		goto out;
+	case SMB2_TREE_DISCONNECT:
+	case SMB2_CANCEL:
+	case SMB2_CLOSE:
+	case SMB2_OPLOCK_BREAK:
+		params.early_eagain = true;
+		break;
 	}
 
 	if (smb2_command != SMB2_INTERNAL_CMD)
-		mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
+		params.start_timer = true;
 
-	atomic_inc(&tconInfoReconnectCount);
-out:
 	/*
 	 * Check if handle based operation so we know whether we can continue
 	 * or not without returning to caller to reset file handle.
@@ -309,11 +218,11 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon)
 	case SMB2_CHANGE_NOTIFY:
 	case SMB2_QUERY_INFO:
 	case SMB2_SET_INFO:
-		rc = -EAGAIN;
+		params.late_eagain = true;
+		break;
 	}
 
-	unload_nls(nls_codepage);
-	return rc;
+	return cifs_tcon_reconnect(tcon, &params);
 }
 
 static void
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 07/13] cifs: cifs_reconnect_tcon() make use of the generic cifs_tcon_reconnect() function
  2020-02-24 13:14 [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Stefan Metzmacher
                   ` (5 preceding siblings ...)
  2020-02-24 13:15 ` [PATCH v1 06/13] cifs: abstract cifs_tcon_reconnect() out of smb2_reconnect() Stefan Metzmacher
@ 2020-02-24 13:15 ` Stefan Metzmacher
  2020-02-24 13:15 ` [PATCH v1 08/13] cifs: make cifs_tree_connect() static Stefan Metzmacher
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: Stefan Metzmacher @ 2020-02-24 13:15 UTC (permalink / raw)
  To: linux-cifs; +Cc: Stefan Metzmacher

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 fs/cifs/cifssmb.c | 117 +++++-----------------------------------------
 fs/cifs/connect.c |   7 +++
 2 files changed, 19 insertions(+), 105 deletions(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 412d141e1adc..2cf74028ce70 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -128,114 +128,22 @@ cifs_mark_open_files_invalid(struct cifs_tcon *tcon)
 static int
 cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
 {
-	int rc;
-	struct cifs_ses *ses;
-	struct TCP_Server_Info *server;
-	struct nls_table *nls_codepage;
-	int retries;
-
-	/*
-	 * SMBs NegProt, SessSetup, uLogoff do not have tcon yet so check for
-	 * tcp and smb session status done differently for those three - in the
-	 * calling routine
-	 */
-	if (!tcon)
-		return 0;
-
-	ses = tcon->ses;
-	server = ses->server;
+	struct cifs_tcon_reconnect_params params = {
+		.skip_reconnect = false,
+	};
 
 	/*
 	 * only tree disconnect, open, and write, (and ulogoff which does not
 	 * have tcon) are allowed as we start force umount
 	 */
-	if (tcon->tidStatus == CifsExiting) {
-		if (smb_command != SMB_COM_WRITE_ANDX &&
-		    smb_command != SMB_COM_OPEN_ANDX &&
-		    smb_command != SMB_COM_TREE_DISCONNECT) {
-			cifs_dbg(FYI, "can not send cmd %d while umounting\n",
-				 smb_command);
-			return -ENODEV;
-		}
-	}
-
-	retries = server->nr_targets;
-
-	/*
-	 * Give demultiplex thread up to 10 seconds to each target available for
-	 * reconnect -- should be greater than cifs socket timeout which is 7
-	 * seconds.
-	 */
-	while (server->tcpStatus == CifsNeedReconnect) {
-		rc = wait_event_interruptible_timeout(server->response_q,
-						      (server->tcpStatus != CifsNeedReconnect),
-						      10 * HZ);
-		if (rc < 0) {
-			cifs_dbg(FYI, "%s: aborting reconnect due to a received"
-				 " signal by the process\n", __func__);
-			return -ERESTARTSYS;
-		}
-
-		/* are we still trying to reconnect? */
-		if (server->tcpStatus != CifsNeedReconnect)
-			break;
-
-		if (retries && --retries)
-			continue;
-
-		/*
-		 * on "soft" mounts we wait once. Hard mounts keep
-		 * retrying until process is killed or server comes
-		 * back on-line
-		 */
-		if (!tcon->retry) {
-			cifs_dbg(FYI, "gave up waiting on reconnect in smb_init\n");
-			return -EHOSTDOWN;
-		}
-		retries = server->nr_targets;
-	}
-
-	if (!ses->need_reconnect && !tcon->need_reconnect)
-		return 0;
-
-	nls_codepage = load_nls_default();
-
-	/*
-	 * need to prevent multiple threads trying to simultaneously
-	 * reconnect the same SMB session
-	 */
-	mutex_lock(&ses->session_mutex);
-
-	/*
-	 * Recheck after acquire mutex. If another thread is negotiating
-	 * and the server never sends an answer the socket will be closed
-	 * and tcpStatus set to reconnect.
-	 */
-	rc = cifs_connect_session_locked(0, ses,
-					 nls_codepage,
-					 tcon->retry);
-	/* do we need to reconnect tcon? */
-	if (rc || !tcon->need_reconnect) {
-		mutex_unlock(&ses->session_mutex);
-		goto out;
-	}
-
-	cifs_mark_open_files_invalid(tcon);
-	rc = cifs_tree_connect(0, tcon, nls_codepage);
-	mutex_unlock(&ses->session_mutex);
-	cifs_dbg(FYI, "reconnect tcon rc = %d\n", rc);
-
-	if (rc) {
-		printk_once(KERN_WARNING "reconnect tcon failed rc = %d\n", rc);
-		goto out;
+	switch (smb_command) {
+	case SMB_COM_WRITE_ANDX:
+	case SMB_COM_OPEN_ANDX:
+	case SMB_COM_TREE_DISCONNECT:
+		params.exit_nodev = true;
+		break;
 	}
 
-	atomic_inc(&tconInfoReconnectCount);
-
-	/* tell server Unix caps we support */
-	if (cap_unix(ses))
-		reset_cifs_unix_caps(0, tcon, NULL, NULL);
-
 	/*
 	 * Removed call to reopen open files here. It is safer (and faster) to
 	 * reopen files one at a time as needed in read and write.
@@ -243,7 +151,6 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
 	 * FIXME: what about file locks? don't we need to reclaim them ASAP?
 	 */
 
-out:
 	/*
 	 * Check if handle based operation so we know whether we can continue
 	 * or not without returning to caller to reset file handle
@@ -254,11 +161,11 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
 	case SMB_COM_CLOSE:
 	case SMB_COM_FIND_CLOSE2:
 	case SMB_COM_LOCKING_ANDX:
-		rc = -EAGAIN;
+		params.late_eagain = true;
+		break;
 	}
 
-	unload_nls(nls_codepage);
-	return rc;
+	return cifs_tcon_reconnect(tcon, &params);
 }
 
 /* Allocate and return pointer to an SMB request buffer, and set basic
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 67d2ad330f33..e920335384d7 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -5510,6 +5510,13 @@ cifs_tcon_reconnect(struct cifs_tcon *tcon,
 		goto out;
 	}
 
+	/*
+	 * tell server Unix caps we support,
+	 * note it's a noop for SMB2.
+	 */
+	if (cap_unix(ses))
+		reset_cifs_unix_caps(0, tcon, NULL, NULL);
+
 	if (params->start_timer)
 		mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 08/13] cifs: make cifs_tree_connect() static
  2020-02-24 13:14 [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Stefan Metzmacher
                   ` (6 preceding siblings ...)
  2020-02-24 13:15 ` [PATCH v1 07/13] cifs: cifs_reconnect_tcon() make use of the generic cifs_tcon_reconnect() function Stefan Metzmacher
@ 2020-02-24 13:15 ` Stefan Metzmacher
  2020-02-24 13:15 ` [PATCH v1 09/13] cifs: turn smb2_reconnect_server() into a generic cifs_reconnect_server() Stefan Metzmacher
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: Stefan Metzmacher @ 2020-02-24 13:15 UTC (permalink / raw)
  To: linux-cifs; +Cc: Stefan Metzmacher

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 fs/cifs/cifsproto.h |  3 ---
 fs/cifs/connect.c   | 12 ++++++------
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 64f13affdb15..a99e3977645f 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -263,9 +263,6 @@ extern void cifs_move_llist(struct list_head *source, struct list_head *dest);
 extern void cifs_free_llist(struct list_head *llist);
 extern void cifs_del_lock_waiters(struct cifsLockInfo *lock);
 
-extern int cifs_tree_connect(const unsigned int xid,
-			     struct cifs_tcon *tcon,
-			     const struct nls_table *nlsc);
 extern int cifs_connect_session_locked(const unsigned int xid,
 				       struct cifs_ses *ses,
 				       struct nls_table *nls_info,
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index e920335384d7..fc430ba99571 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -5291,9 +5291,9 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
 }
 
 #ifdef CONFIG_CIFS_DFS_UPCALL
-int cifs_tree_connect(const unsigned int xid,
-		      struct cifs_tcon *tcon,
-		      const struct nls_table *nlsc)
+static int cifs_tree_connect(const unsigned int xid,
+			     struct cifs_tcon *tcon,
+			     const struct nls_table *nlsc)
 {
 	const struct smb_version_operations *ops = tcon->ses->server->ops;
 	int rc;
@@ -5365,9 +5365,9 @@ int cifs_tree_connect(const unsigned int xid,
 	return rc;
 }
 #else
-int cifs_tree_connect(const unsigned int xid,
-		      struct cifs_tcon *tcon,
-		      const struct nls_table *nlsc)
+static int cifs_tree_connect(const unsigned int xid,
+			     struct cifs_tcon *tcon,
+			     const struct nls_table *nlsc)
 {
 	const struct smb_version_operations *ops = tcon->ses->server->ops;
 	return ops->tree_connect(xid, tcon->ses, tcon->treeName, tcon, nlsc);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 09/13] cifs: turn smb2_reconnect_server() into a generic cifs_reconnect_server()
  2020-02-24 13:14 [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Stefan Metzmacher
                   ` (7 preceding siblings ...)
  2020-02-24 13:15 ` [PATCH v1 08/13] cifs: make cifs_tree_connect() static Stefan Metzmacher
@ 2020-02-24 13:15 ` Stefan Metzmacher
  2020-02-24 13:15 ` [PATCH v1 10/13] cifs: move cifs_reconnect_tcons() to fs/cifs/connect.c and make it static Stefan Metzmacher
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: Stefan Metzmacher @ 2020-02-24 13:15 UTC (permalink / raw)
  To: linux-cifs; +Cc: Stefan Metzmacher

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 fs/cifs/connect.c   |  2 +-
 fs/cifs/smb2pdu.c   | 12 ++++++------
 fs/cifs/smb2pdu.h   |  2 --
 fs/cifs/smb2proto.h |  2 +-
 4 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index fc430ba99571..6eca37924d9e 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2823,7 +2823,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
 	INIT_LIST_HEAD(&tcp_ses->tcp_ses_list);
 	INIT_LIST_HEAD(&tcp_ses->smb_ses_list);
 	INIT_DELAYED_WORK(&tcp_ses->echo, cifs_echo_request);
-	INIT_DELAYED_WORK(&tcp_ses->reconnect, smb2_reconnect_server);
+	INIT_DELAYED_WORK(&tcp_ses->reconnect, cifs_reconnect_tcons);
 	mutex_init(&tcp_ses->reconnect_mutex);
 	memcpy(&tcp_ses->srcaddr, &volume_info->srcaddr,
 	       sizeof(tcp_ses->srcaddr));
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 162fe3381f4c..6f3c5eb62d51 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -159,7 +159,7 @@ static int
 smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon)
 {
 	struct cifs_tcon_reconnect_params params = {
-		.skip_reconnect = false,
+		.start_timer = true,
 	};
 
 	switch (smb2_command) {
@@ -197,9 +197,6 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon)
 		break;
 	}
 
-	if (smb2_command != SMB2_INTERNAL_CMD)
-		params.start_timer = true;
-
 	/*
 	 * Check if handle based operation so we know whether we can continue
 	 * or not without returning to caller to reset file handle.
@@ -3293,7 +3290,7 @@ smb2_echo_callback(struct mid_q_entry *mid)
 	add_credits(server, &credits, CIFS_ECHO_OP);
 }
 
-void smb2_reconnect_server(struct work_struct *work)
+void cifs_reconnect_tcons(struct work_struct *work)
 {
 	struct TCP_Server_Info *server = container_of(work,
 					struct TCP_Server_Info, reconnect.work);
@@ -3340,7 +3337,10 @@ void smb2_reconnect_server(struct work_struct *work)
 	spin_unlock(&cifs_tcp_ses_lock);
 
 	list_for_each_entry_safe(tcon, tcon2, &tmp_list, rlist) {
-		rc = smb2_reconnect(SMB2_INTERNAL_CMD, tcon);
+		struct cifs_tcon_reconnect_params params = {
+			.start_timer = false,
+		};
+		rc = cifs_tcon_reconnect(tcon, &params);
 		if (!rc)
 			cifs_reopen_persistent_handles(tcon);
 		else
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index fa03df130f1a..330748bd3736 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -81,8 +81,6 @@
 #define SMB2_SET_INFO		cpu_to_le16(SMB2_SET_INFO_HE)
 #define SMB2_OPLOCK_BREAK	cpu_to_le16(SMB2_OPLOCK_BREAK_HE)
 
-#define SMB2_INTERNAL_CMD	cpu_to_le16(0xFFFF)
-
 #define NUMBER_OF_SMB2_COMMANDS	0x0013
 
 /* 52 transform hdr + 64 hdr + 88 create rsp */
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index de6388ef344f..c52be13a374a 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -116,7 +116,7 @@ extern int smb2_open_file(const unsigned int xid,
 extern int smb2_unlock_range(struct cifsFileInfo *cfile,
 			     struct file_lock *flock, const unsigned int xid);
 extern int smb2_push_mandatory_locks(struct cifsFileInfo *cfile);
-extern void smb2_reconnect_server(struct work_struct *work);
+extern void cifs_reconnect_tcons(struct work_struct *work);
 extern int smb3_crypto_aead_allocate(struct TCP_Server_Info *server);
 extern unsigned long smb_rqst_len(struct TCP_Server_Info *server,
 				  struct smb_rqst *rqst);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 10/13] cifs: move cifs_reconnect_tcons() to fs/cifs/connect.c and make it static
  2020-02-24 13:14 [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Stefan Metzmacher
                   ` (8 preceding siblings ...)
  2020-02-24 13:15 ` [PATCH v1 09/13] cifs: turn smb2_reconnect_server() into a generic cifs_reconnect_server() Stefan Metzmacher
@ 2020-02-24 13:15 ` Stefan Metzmacher
  2020-02-24 13:15 ` [PATCH v1 11/13] cifs: use reconnect timer also for SMB1 Stefan Metzmacher
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: Stefan Metzmacher @ 2020-02-24 13:15 UTC (permalink / raw)
  To: linux-cifs; +Cc: Stefan Metzmacher

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 fs/cifs/connect.c   | 72 +++++++++++++++++++++++++++++++++++++++++++++
 fs/cifs/smb2pdu.c   | 72 ---------------------------------------------
 fs/cifs/smb2proto.h |  1 -
 3 files changed, 72 insertions(+), 73 deletions(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 6eca37924d9e..7f4be85b7cc9 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -714,6 +714,78 @@ cifs_echo_request(struct work_struct *work)
 	queue_delayed_work(cifsiod_wq, &server->echo, server->echo_interval);
 }
 
+static void cifs_reconnect_tcons(struct work_struct *work)
+{
+	struct TCP_Server_Info *server = container_of(work,
+					struct TCP_Server_Info, reconnect.work);
+	struct cifs_ses *ses;
+	struct cifs_tcon *tcon, *tcon2;
+	struct list_head tmp_list;
+	int tcon_exist = false;
+	int rc;
+	int resched = false;
+
+
+	/* Prevent simultaneous reconnects that can corrupt tcon->rlist list */
+	mutex_lock(&server->reconnect_mutex);
+
+	INIT_LIST_HEAD(&tmp_list);
+	cifs_dbg(FYI, "Need negotiate, reconnecting tcons\n");
+
+	spin_lock(&cifs_tcp_ses_lock);
+	list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
+		list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {
+			if (tcon->need_reconnect || tcon->need_reopen_files) {
+				tcon->tc_count++;
+				list_add_tail(&tcon->rlist, &tmp_list);
+				tcon_exist = true;
+			}
+		}
+		/*
+		 * IPC has the same lifetime as its session and uses its
+		 * refcount.
+		 */
+		if (ses->tcon_ipc && ses->tcon_ipc->need_reconnect) {
+			list_add_tail(&ses->tcon_ipc->rlist, &tmp_list);
+			tcon_exist = true;
+			ses->ses_count++;
+		}
+	}
+	/*
+	 * Get the reference to server struct to be sure that the last call of
+	 * cifs_put_tcon() in the loop below won't release the server pointer.
+	 */
+	if (tcon_exist)
+		server->srv_count++;
+
+	spin_unlock(&cifs_tcp_ses_lock);
+
+	list_for_each_entry_safe(tcon, tcon2, &tmp_list, rlist) {
+		struct cifs_tcon_reconnect_params params = {
+			.start_timer = false,
+		};
+		rc = cifs_tcon_reconnect(tcon, &params);
+		if (!rc)
+			cifs_reopen_persistent_handles(tcon);
+		else
+			resched = true;
+		list_del_init(&tcon->rlist);
+		if (tcon->ipc)
+			cifs_put_smb_ses(tcon->ses);
+		else
+			cifs_put_tcon(tcon);
+	}
+
+	cifs_dbg(FYI, "Reconnecting tcons finished\n");
+	if (resched)
+		queue_delayed_work(cifsiod_wq, &server->reconnect, 2 * HZ);
+	mutex_unlock(&server->reconnect_mutex);
+
+	/* now we can safely release srv struct */
+	if (tcon_exist)
+		cifs_put_tcp_session(server, 1);
+}
+
 static bool
 allocate_buffers(struct TCP_Server_Info *server)
 {
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 6f3c5eb62d51..b4446ecf4e97 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -3290,78 +3290,6 @@ smb2_echo_callback(struct mid_q_entry *mid)
 	add_credits(server, &credits, CIFS_ECHO_OP);
 }
 
-void cifs_reconnect_tcons(struct work_struct *work)
-{
-	struct TCP_Server_Info *server = container_of(work,
-					struct TCP_Server_Info, reconnect.work);
-	struct cifs_ses *ses;
-	struct cifs_tcon *tcon, *tcon2;
-	struct list_head tmp_list;
-	int tcon_exist = false;
-	int rc;
-	int resched = false;
-
-
-	/* Prevent simultaneous reconnects that can corrupt tcon->rlist list */
-	mutex_lock(&server->reconnect_mutex);
-
-	INIT_LIST_HEAD(&tmp_list);
-	cifs_dbg(FYI, "Need negotiate, reconnecting tcons\n");
-
-	spin_lock(&cifs_tcp_ses_lock);
-	list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
-		list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {
-			if (tcon->need_reconnect || tcon->need_reopen_files) {
-				tcon->tc_count++;
-				list_add_tail(&tcon->rlist, &tmp_list);
-				tcon_exist = true;
-			}
-		}
-		/*
-		 * IPC has the same lifetime as its session and uses its
-		 * refcount.
-		 */
-		if (ses->tcon_ipc && ses->tcon_ipc->need_reconnect) {
-			list_add_tail(&ses->tcon_ipc->rlist, &tmp_list);
-			tcon_exist = true;
-			ses->ses_count++;
-		}
-	}
-	/*
-	 * Get the reference to server struct to be sure that the last call of
-	 * cifs_put_tcon() in the loop below won't release the server pointer.
-	 */
-	if (tcon_exist)
-		server->srv_count++;
-
-	spin_unlock(&cifs_tcp_ses_lock);
-
-	list_for_each_entry_safe(tcon, tcon2, &tmp_list, rlist) {
-		struct cifs_tcon_reconnect_params params = {
-			.start_timer = false,
-		};
-		rc = cifs_tcon_reconnect(tcon, &params);
-		if (!rc)
-			cifs_reopen_persistent_handles(tcon);
-		else
-			resched = true;
-		list_del_init(&tcon->rlist);
-		if (tcon->ipc)
-			cifs_put_smb_ses(tcon->ses);
-		else
-			cifs_put_tcon(tcon);
-	}
-
-	cifs_dbg(FYI, "Reconnecting tcons finished\n");
-	if (resched)
-		queue_delayed_work(cifsiod_wq, &server->reconnect, 2 * HZ);
-	mutex_unlock(&server->reconnect_mutex);
-
-	/* now we can safely release srv struct */
-	if (tcon_exist)
-		cifs_put_tcp_session(server, 1);
-}
-
 int
 SMB2_echo(struct TCP_Server_Info *server)
 {
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index c52be13a374a..fbceb62d355d 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -116,7 +116,6 @@ extern int smb2_open_file(const unsigned int xid,
 extern int smb2_unlock_range(struct cifsFileInfo *cfile,
 			     struct file_lock *flock, const unsigned int xid);
 extern int smb2_push_mandatory_locks(struct cifsFileInfo *cfile);
-extern void cifs_reconnect_tcons(struct work_struct *work);
 extern int smb3_crypto_aead_allocate(struct TCP_Server_Info *server);
 extern unsigned long smb_rqst_len(struct TCP_Server_Info *server,
 				  struct smb_rqst *rqst);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 11/13] cifs: use reconnect timer also for SMB1
  2020-02-24 13:14 [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Stefan Metzmacher
                   ` (9 preceding siblings ...)
  2020-02-24 13:15 ` [PATCH v1 10/13] cifs: move cifs_reconnect_tcons() to fs/cifs/connect.c and make it static Stefan Metzmacher
@ 2020-02-24 13:15 ` Stefan Metzmacher
  2020-02-24 13:15 ` [PATCH v1 12/13] cifs: map STATUS_ACCOUNT_LOCKED_OUT to -EACCES Stefan Metzmacher
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: Stefan Metzmacher @ 2020-02-24 13:15 UTC (permalink / raw)
  To: linux-cifs; +Cc: Stefan Metzmacher

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 fs/cifs/cifssmb.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 2cf74028ce70..1ede3a5c6889 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -129,7 +129,7 @@ static int
 cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
 {
 	struct cifs_tcon_reconnect_params params = {
-		.skip_reconnect = false,
+		.start_timer = true,
 	};
 
 	/*
@@ -662,6 +662,12 @@ CIFSSMBEcho(struct TCP_Server_Info *server)
 
 	cifs_dbg(FYI, "In echo request\n");
 
+	if (server->tcpStatus == CifsNeedNegotiate) {
+		/* No need to send echo on newly established connections */
+		mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
+		return rc;
+	}
+
 	rc = small_smb_init(SMB_COM_ECHO, 0, NULL, (void **)&smb);
 	if (rc)
 		return rc;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 12/13] cifs: map STATUS_ACCOUNT_LOCKED_OUT to -EACCES
  2020-02-24 13:14 [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Stefan Metzmacher
                   ` (10 preceding siblings ...)
  2020-02-24 13:15 ` [PATCH v1 11/13] cifs: use reconnect timer also for SMB1 Stefan Metzmacher
@ 2020-02-24 13:15 ` Stefan Metzmacher
  2020-02-24 13:15 ` [PATCH v1 13/13] cifs: introduce the CifsInvalidCredentials session state Stefan Metzmacher
  2020-02-24 18:38 ` [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Aurélien Aptel
  13 siblings, 0 replies; 22+ messages in thread
From: Stefan Metzmacher @ 2020-02-24 13:15 UTC (permalink / raw)
  To: linux-cifs; +Cc: Stefan Metzmacher

This is basically the same as STATUS_LOGON_FAILURE,
but after the account is locked out.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 fs/cifs/smb2maperror.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/smb2maperror.c b/fs/cifs/smb2maperror.c
index 7fde3775cb57..91af7baed051 100644
--- a/fs/cifs/smb2maperror.c
+++ b/fs/cifs/smb2maperror.c
@@ -814,7 +814,7 @@ static const struct status_to_posix_error smb2_error_map_table[] = {
 	{STATUS_INVALID_VARIANT, -EIO, "STATUS_INVALID_VARIANT"},
 	{STATUS_DOMAIN_CONTROLLER_NOT_FOUND, -EIO,
 	"STATUS_DOMAIN_CONTROLLER_NOT_FOUND"},
-	{STATUS_ACCOUNT_LOCKED_OUT, -EIO, "STATUS_ACCOUNT_LOCKED_OUT"},
+	{STATUS_ACCOUNT_LOCKED_OUT, -EACCES, "STATUS_ACCOUNT_LOCKED_OUT"},
 	{STATUS_HANDLE_NOT_CLOSABLE, -EIO, "STATUS_HANDLE_NOT_CLOSABLE"},
 	{STATUS_CONNECTION_REFUSED, -EIO, "STATUS_CONNECTION_REFUSED"},
 	{STATUS_GRACEFUL_DISCONNECT, -EIO, "STATUS_GRACEFUL_DISCONNECT"},
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v1 13/13] cifs: introduce the CifsInvalidCredentials session state
  2020-02-24 13:14 [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Stefan Metzmacher
                   ` (11 preceding siblings ...)
  2020-02-24 13:15 ` [PATCH v1 12/13] cifs: map STATUS_ACCOUNT_LOCKED_OUT to -EACCES Stefan Metzmacher
@ 2020-02-24 13:15 ` Stefan Metzmacher
  2020-02-24 18:09   ` Aurélien Aptel
  2020-02-24 18:38 ` [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Aurélien Aptel
  13 siblings, 1 reply; 22+ messages in thread
From: Stefan Metzmacher @ 2020-02-24 13:15 UTC (permalink / raw)
  To: linux-cifs; +Cc: Stefan Metzmacher

RHBZ: 1579050

If a session setup gets NT_STATUS_LOGON_FAILURE of
NT_STATUS_ACCOUNT_LOCKED_OUT, we're not able to make any further
progress with the credentials the kernel knowns about.

Instead of retrying for each new request, we should mark
the session with CifsInvalidCredentials (for know we only do that
for soft mounts). We consistently return -EKEYREVOKED for such sessions,
I guess that's better than -EHOSTDOWN.

A future addition would be an upcall to get new credentials from
userspace, or a way to use a magic file per session under /proc/fs/cifs/
to provide new credentials.

But for now we want to avoid that we lock out the users account,
by retrying every 2 seconds.

Note backports also need the other 12 patches before.

Fixes: b0dd940e582b6a6 ("cifs: fail i/o on soft mounts if sessionsetup errors out")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Cc: Stable <stable@vger.kernel.org>
Cc: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/cifs_debug.c |   2 +
 fs/cifs/cifsfs.c     |   1 +
 fs/cifs/cifsglob.h   |   4 +-
 fs/cifs/connect.c    | 145 ++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 148 insertions(+), 4 deletions(-)

diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
index 276e4b5ea8e0..c03a445e95bc 100644
--- a/fs/cifs/cifs_debug.c
+++ b/fs/cifs/cifs_debug.c
@@ -392,6 +392,8 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
 
 			/* dump session id helpful for use with network trace */
 			seq_printf(m, " SessionId: 0x%llx", ses->Suid);
+			if (ses->status == CifsInvalidCredentials)
+				seq_printf(m, " InvalidCredentials!");
 			if (ses->session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA)
 				seq_puts(m, " encrypted");
 			if (ses->sign)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 46ebaf3f0824..9e2e5fa2cdff 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -650,6 +650,7 @@ static void cifs_umount_begin(struct super_block *sb)
 	/* cancel_notify_requests(tcon); */
 	if (tcon->ses && tcon->ses->server) {
 		cifs_dbg(FYI, "wake up tasks now - umount begin not complete\n");
+		wake_up_all(&tcon->ses->server->demultiplex_q);
 		wake_up_all(&tcon->ses->server->request_q);
 		wake_up_all(&tcon->ses->server->response_q);
 		msleep(1); /* yield */
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 8393ed7ebf96..4494b35fa5c5 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -114,7 +114,8 @@ enum statusEnum {
 	CifsGood,
 	CifsExiting,
 	CifsNeedReconnect,
-	CifsNeedNegotiate
+	CifsNeedNegotiate,
+	CifsInvalidCredentials,
 };
 
 enum securityEnum {
@@ -687,6 +688,7 @@ struct TCP_Server_Info {
 #endif
 	wait_queue_head_t response_q;
 	wait_queue_head_t request_q; /* if more than maxmpx to srvr must block*/
+	wait_queue_head_t demultiplex_q;
 	struct list_head pending_mid_q;
 	bool noblocksnd;		/* use blocking sendmsg */
 	bool noautotune;		/* do not autotune send buf sizes */
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 7f4be85b7cc9..39c0a5154eb9 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -480,6 +480,65 @@ static inline int reconn_setup_dfs_targets(struct cifs_sb_info *cifs_sb,
 }
 #endif
 
+static bool
+server_should_poll_for_responses(struct TCP_Server_Info *server)
+{
+	switch (server->tcpStatus) {
+	case CifsNew:
+		return false;
+	case CifsExiting:
+		return false;
+	case CifsGood:
+		return true;
+	case CifsNeedReconnect:
+		return false;
+	case CifsNeedNegotiate:
+		return true;
+	case CifsInvalidCredentials:
+		/*
+		 * Only a session should have this state
+		 */
+		WARN(true, "BUG! server->tcpStatus=CifsInvalidCredentials\n");
+		return false;
+	}
+
+	WARN(true, "Unhandled server->tcpStatus=%u\n", server->tcpStatus);
+	return false;
+}
+
+static bool
+server_should_reconnect(struct TCP_Server_Info *server)
+{
+	struct cifs_ses *ses;
+	size_t num_valid = 0;
+	size_t num_invalid = 0;
+
+	/*
+	 * If we are in disconnected state, waiting for a reconnect,
+	 * we should only try to reconnect if there's a chance
+	 * of success, that's not the case of all sessions
+	 * are have invalid credentials.
+	 */
+	if (server->tcpStatus != CifsNeedReconnect)
+		return false;
+
+	spin_lock(&cifs_tcp_ses_lock);
+	list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
+		if (ses->status != CifsInvalidCredentials) {
+			num_valid++;
+		} else {
+			num_invalid++;
+		}
+	}
+	spin_unlock(&cifs_tcp_ses_lock);
+
+	if (num_invalid == 0 || num_valid != 0) {
+		return true;
+	}
+
+	return false;
+}
+
 /*
  * cifs tcp session reconnection
  *
@@ -617,6 +676,17 @@ cifs_reconnect(struct TCP_Server_Info *server)
 		try_to_freeze();
 
 		mutex_lock(&server->srv_mutex);
+
+		if (!server_should_reconnect(server)) {
+			cifs_dbg(VFS,
+				 "Server %s was disconnected. "
+				 "We should NOT reconnect...\n",
+				 server->hostname);
+			rc = -ECONNABORTED;
+			mutex_unlock(&server->srv_mutex);
+			break;
+		}
+
 		/*
 		 * Set up next DFS target server (if any) for reconnect. If DFS
 		 * feature is disabled, then we will retry last server we
@@ -669,8 +739,17 @@ cifs_reconnect(struct TCP_Server_Info *server)
 
 	put_tcp_super(sb);
 #endif
-	if (server->tcpStatus == CifsNeedNegotiate)
-		mod_delayed_work(cifsiod_wq, &server->echo, 0);
+	if (server->tcpStatus == CifsNeedNegotiate) {
+		/* unfreeze the demultiplex thread */
+		wake_up_all(&server->demultiplex_q);
+		/*
+		 * schedule a immediate reconnect of sessions
+		 * and tcons.
+		 */
+		mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
+		/* reset the echo timer into the future. */
+		mod_delayed_work(cifsiod_wq, &server->echo, server->echo_interval);
+	}
 
 	wake_up(&server->response_q);
 	return rc;
@@ -872,6 +951,19 @@ cifs_readv_from_socket(struct TCP_Server_Info *server, struct msghdr *smb_msg)
 	for (total_read = 0; msg_data_left(smb_msg); total_read += length) {
 		try_to_freeze();
 
+		if (server_should_reconnect(server)) {
+			cifs_dbg(VFS, "Server %s was disconnected. Reconnecting...\n",
+				 server->hostname);
+			cifs_reconnect(server);
+			wake_up(&server->response_q);
+			return -ECONNABORTED;
+		}
+
+		if (!server_should_poll_for_responses(server)) {
+			cifs_dbg(FYI, "Only invalid credential sessions\n");
+			return -ECONNABORTED;
+		}
+
 		/* reconnect if no credits and no requests in flight */
 		if (zero_credits(server)) {
 			cifs_reconnect(server);
@@ -1222,6 +1314,22 @@ smb2_add_credits_from_hdr(char *buffer, struct TCP_Server_Info *server)
 	}
 }
 
+static bool cifs_demultiplex_thread_should_block(struct TCP_Server_Info *server)
+{
+	if (server->tcpStatus == CifsExiting) {
+		return false;
+	}
+
+	if (server_should_reconnect(server)) {
+		return false;
+	}
+
+	if (server_should_poll_for_responses(server)) {
+		return false;
+	}
+
+	return true;
+}
 
 static int
 cifs_demultiplex_thread(void *p)
@@ -1248,6 +1356,11 @@ cifs_demultiplex_thread(void *p)
 		if (try_to_freeze())
 			continue;
 
+		wait_event_freezable(server->demultiplex_q,
+				!cifs_demultiplex_thread_should_block(server));
+		if (server->tcpStatus == CifsExiting)
+			break;
+
 		if (!allocate_buffers(server))
 			continue;
 
@@ -2880,6 +2993,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
 	tcp_ses->credits = 1;
 	init_waitqueue_head(&tcp_ses->response_q);
 	init_waitqueue_head(&tcp_ses->request_q);
+	init_waitqueue_head(&tcp_ses->demultiplex_q);
 	INIT_LIST_HEAD(&tcp_ses->pending_mid_q);
 	mutex_init(&tcp_ses->srv_mutex);
 	memcpy(tcp_ses->workstation_RFC1001_name,
@@ -2967,6 +3081,13 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
 
 	cifs_fscache_get_client_cookie(tcp_ses);
 
+	/* unfreeze the demultiplex thread */
+	wake_up_all(&tcp_ses->demultiplex_q);
+	/*
+	 * we let the caller use cifs_connect_session_locked()
+	 * (directly or via cifs_get_smb_ses() and don't
+	 * schedule the reconnect timer.
+	 */
 	/* queue echo request delayed work */
 	queue_delayed_work(cifsiod_wq, &tcp_ses->echo, tcp_ses->echo_interval);
 
@@ -5454,6 +5575,10 @@ cifs_connect_session_locked(const unsigned int xid,
 {
 	int rc;
 
+	if (ses->status == CifsInvalidCredentials) {
+		return -EKEYREVOKED;
+	}
+
 	if (ses->server->tcpStatus == CifsNeedReconnect) {
 		return -EHOSTDOWN;
 	}
@@ -5463,7 +5588,12 @@ cifs_connect_session_locked(const unsigned int xid,
 		cifs_dbg(FYI, "Session needs reconnect\n");
 		rc = cifs_setup_session(xid, ses, nls_info);
 		if ((rc == -EACCES) && !retry) {
-			return -EHOSTDOWN;
+			cifs_dbg(VFS,
+				 "SessionSetup to Server %s failed, "
+				 "marking as CifsInvalidCredentials/EKEYREVOKED\n",
+				 ses->server->hostname);
+			ses->status = CifsInvalidCredentials;
+			return -EKEYREVOKED;
 		}
 	}
 
@@ -5505,6 +5635,10 @@ cifs_tcon_reconnect(struct cifs_tcon *tcon,
 
 	retries = server->nr_targets;
 
+	if (ses->status == CifsInvalidCredentials) {
+		return -EKEYREVOKED;
+	}
+
 	/*
 	 * Give demultiplex thread up to 10 seconds to each target available for
 	 * reconnect -- should be greater than cifs socket timeout which is 7
@@ -5515,6 +5649,7 @@ cifs_tcon_reconnect(struct cifs_tcon *tcon,
 			return -EAGAIN;
 		}
 
+		wake_up_all(&server->demultiplex_q);
 		rc = wait_event_interruptible_timeout(server->response_q,
 						      (server->tcpStatus != CifsNeedReconnect),
 						      10 * HZ);
@@ -5543,6 +5678,10 @@ cifs_tcon_reconnect(struct cifs_tcon *tcon,
 		retries = server->nr_targets;
 	}
 
+	if (ses->status == CifsInvalidCredentials) {
+		return -EKEYREVOKED;
+	}
+
 	if (!ses->need_reconnect && !tcon->need_reconnect)
 		return 0;
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v1 13/13] cifs: introduce the CifsInvalidCredentials session state
  2020-02-24 13:15 ` [PATCH v1 13/13] cifs: introduce the CifsInvalidCredentials session state Stefan Metzmacher
@ 2020-02-24 18:09   ` Aurélien Aptel
  0 siblings, 0 replies; 22+ messages in thread
From: Aurélien Aptel @ 2020-02-24 18:09 UTC (permalink / raw)
  To: Stefan Metzmacher, linux-cifs; +Cc: Stefan Metzmacher

Stefan Metzmacher <metze@samba.org> writes:
> A future addition would be an upcall to get new credentials from
> userspace, or a way to use a magic file per session under /proc/fs/cifs/
> to provide new credentials.

I've looked into this recently. cifscreds from cifs-utils can already
store/update credentials in keyrings.

I believe this is only used in multiuser mode (-o multiuser).  In that
mode, when a process does a syscall, cifs.ko will try to use a cifs_ses
matching the uid of that process, potentially opening a new one.

To open a new session for that user, cifs.ko looks at the current
process session keyring for that uid credentials. Take a look at
cifs_set_cifscreds(), it's the function that sets the credentials in the
volume about to be connected to.

* the key is of type "logon",
* description is "cifs:<mode>:<host>" where mode determines what host is
  ('a' for an ip address, 'd' for a domain).
* value is "<user>:<password>"

[ side-note on that keyring: it is the process session keyring. So you
need to make sure the keyring is created when the user first logs in the
system (i.e. via pam), otherwise cifscreds will create it, and since it
is the only user, will destroy it when cifscreds exits (refcount reaches
zero).  I don't know why it was decided to use the session keyring, I
feel like we should make this keyring "global" instead of per session,
it would be easier to setup and update but I don't know the security
implications. (If anyone knows please share) ]

In any case, I think we should try to update
cifs_ses->{user_name,password} before re-opening a session by looking at
this keyring.

Cheers,
-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1 03/13] cifs: make use of cap_unix(ses) in cifs_reconnect_tcon()
  2020-02-24 13:15 ` [PATCH v1 03/13] cifs: make use of cap_unix(ses) in cifs_reconnect_tcon() Stefan Metzmacher
@ 2020-02-24 18:12   ` Aurélien Aptel
  2020-02-24 20:39     ` Steve French
  0 siblings, 1 reply; 22+ messages in thread
From: Aurélien Aptel @ 2020-02-24 18:12 UTC (permalink / raw)
  To: Stefan Metzmacher, linux-cifs; +Cc: Stefan Metzmacher

Reviewed-by: Aurelien Aptel <aaptel@suse.com>

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts
  2020-02-24 13:14 [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Stefan Metzmacher
                   ` (12 preceding siblings ...)
  2020-02-24 13:15 ` [PATCH v1 13/13] cifs: introduce the CifsInvalidCredentials session state Stefan Metzmacher
@ 2020-02-24 18:38 ` Aurélien Aptel
  13 siblings, 0 replies; 22+ messages in thread
From: Aurélien Aptel @ 2020-02-24 18:38 UTC (permalink / raw)
  To: Stefan Metzmacher, linux-cifs

Stefan Metzmacher <metze@samba.org> writes:
> The first patches consolidate the retry/reconnect handling between SMB1
> and SMB2/3.

This code is very tricky, we had to tweak it multiple times to deal with
races, deadlocks and DFS (with DFS we have to failover to a different
server and have to update TCP_Server_Info fields) and we can't
test/repro those easily so we need to be very careful. I will start
looking but it will take some time. In any case amazing work in unifying
these things, reconnect code path is horribly complex.

Also, you should try to run checkpatch.pl (from the kernel repo) and
sparse (kernel static analysis). You need a recent version of sparse to
work on the kernel (get&build the git repo somewhere).
I like to use something this when working on a series:

 git rebase -x 'bash -c "./scripts/checkpatch.pl <(git format-patch -1 --stdout)" && touch fs/cifs/*.[ch] && make CHECK=path_to_your_sparse_bin C=1' HEAD~13

(on one line)

Cheers,
-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1 03/13] cifs: make use of cap_unix(ses) in cifs_reconnect_tcon()
  2020-02-24 18:12   ` Aurélien Aptel
@ 2020-02-24 20:39     ` Steve French
  2020-02-24 21:36       ` Pavel Shilovsky
  0 siblings, 1 reply; 22+ messages in thread
From: Steve French @ 2020-02-24 20:39 UTC (permalink / raw)
  To: Aurélien Aptel; +Cc: Stefan Metzmacher, CIFS

First three patches in this series merged into cifs-2.6.git for-next
(and the buildbot's github tree) pending more testing and review

On Mon, Feb 24, 2020 at 12:12 PM Aurélien Aptel <aaptel@samba.org> wrote:
>
> Reviewed-by: Aurelien Aptel <aaptel@suse.com>
>
> --
> Aurélien Aptel / SUSE Labs Samba Team
> GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
> SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)



-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1 03/13] cifs: make use of cap_unix(ses) in cifs_reconnect_tcon()
  2020-02-24 20:39     ` Steve French
@ 2020-02-24 21:36       ` Pavel Shilovsky
  0 siblings, 0 replies; 22+ messages in thread
From: Pavel Shilovsky @ 2020-02-24 21:36 UTC (permalink / raw)
  To: Steve French; +Cc: Aurélien Aptel, Stefan Metzmacher, CIFS

The first tree patches look good.

Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>

--
Best regards,
Pavel Shilovsky

пн, 24 февр. 2020 г. в 12:39, Steve French <smfrench@gmail.com>:
>
> First three patches in this series merged into cifs-2.6.git for-next
> (and the buildbot's github tree) pending more testing and review
>
> On Mon, Feb 24, 2020 at 12:12 PM Aurélien Aptel <aaptel@samba.org> wrote:
> >
> > Reviewed-by: Aurelien Aptel <aaptel@suse.com>
> >
> > --
> > Aurélien Aptel / SUSE Labs Samba Team
> > GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
> > SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
> > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)
>
>
>
> --
> Thanks,
>
> Steve

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1 05/13] cifs: merge __{cifs,smb2}_reconnect[_tcon]() into cifs_tree_connect()
  2020-02-24 13:15 ` [PATCH v1 05/13] cifs: merge __{cifs,smb2}_reconnect[_tcon]() into cifs_tree_connect() Stefan Metzmacher
@ 2020-03-06 16:46   ` Aurélien Aptel
  0 siblings, 0 replies; 22+ messages in thread
From: Aurélien Aptel @ 2020-03-06 16:46 UTC (permalink / raw)
  To: Stefan Metzmacher, linux-cifs; +Cc: Stefan Metzmacher

This one LGTM but doesn't apply anymore.

Cheers,
-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1 04/13] cifs: split out a cifs_connect_session_locked() helper function
  2020-02-24 13:15 ` [PATCH v1 04/13] cifs: split out a cifs_connect_session_locked() helper function Stefan Metzmacher
@ 2020-03-06 17:05   ` Aurélien Aptel
  0 siblings, 0 replies; 22+ messages in thread
From: Aurélien Aptel @ 2020-03-06 17:05 UTC (permalink / raw)
  To: Stefan Metzmacher, linux-cifs; +Cc: Stefan Metzmacher

This LGTM but I think we should really test it on the
buildbot. Especially DFS failover and multichannel.

Stefan Metzmacher <metze@samba.org> writes:
> +int
> +cifs_connect_session_locked(const unsigned int xid,
> +			    struct cifs_ses *ses,
> +			    struct nls_table *nls_info,
> +			    bool retry)
> +{
> +	int rc;
> +
> +	if (ses->server->tcpStatus == CifsNeedReconnect) {
> +		return -EHOSTDOWN;
> +	}

This check is now done everytime. Probably it's correct, but worth
pointing out.

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v1 06/13] cifs: abstract cifs_tcon_reconnect() out of smb2_reconnect()
  2020-02-24 13:15 ` [PATCH v1 06/13] cifs: abstract cifs_tcon_reconnect() out of smb2_reconnect() Stefan Metzmacher
@ 2020-03-27 17:31   ` Steve French
  0 siblings, 0 replies; 22+ messages in thread
From: Steve French @ 2020-03-27 17:31 UTC (permalink / raw)
  To: Stefan Metzmacher; +Cc: CIFS

Any updates on this (rebase) on the remaining 10?

On Mon, Feb 24, 2020 at 7:16 AM Stefan Metzmacher <metze@samba.org> wrote:
>
> cifs_tcon_reconnect() will also be used in cifs_reconnect_tcon()
> with the next commit.
>
> Signed-off-by: Stefan Metzmacher <metze@samba.org>
> ---
>  fs/cifs/cifsglob.h  |   8 +++
>  fs/cifs/cifsproto.h |   3 +
>  fs/cifs/connect.c   | 125 ++++++++++++++++++++++++++++++++++++
>  fs/cifs/smb2pdu.c   | 151 +++++++++-----------------------------------
>  4 files changed, 166 insertions(+), 121 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index de82cfa44b1a..8393ed7ebf96 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -1074,6 +1074,14 @@ struct cached_fid {
>         struct smb2_file_all_info file_all_info;
>  };
>
> +struct cifs_tcon_reconnect_params {
> +       bool skip_reconnect;
> +       bool exit_nodev;
> +       bool early_eagain;
> +       bool late_eagain;
> +       bool start_timer;
> +};
> +
>  /*
>   * there is one of these for each connection to a resource on a particular
>   * session
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index 4c93007e44c0..64f13affdb15 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -270,6 +270,9 @@ extern int cifs_connect_session_locked(const unsigned int xid,
>                                        struct cifs_ses *ses,
>                                        struct nls_table *nls_info,
>                                        bool retry);
> +extern int cifs_tcon_reconnect(struct cifs_tcon *tcon,
> +                              const struct cifs_tcon_reconnect_params *params);
> +
>  extern int cifs_enable_signing(struct TCP_Server_Info *server, bool mnt_sign_required);
>  extern int CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses);
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index c243c9a1b3d4..67d2ad330f33 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -5398,6 +5398,131 @@ cifs_connect_session_locked(const unsigned int xid,
>         return rc;
>  }
>
> +int
> +cifs_tcon_reconnect(struct cifs_tcon *tcon,
> +                   const struct cifs_tcon_reconnect_params *params)
> +{
> +       int rc;
> +       struct nls_table *nls_codepage;
> +       struct cifs_ses *ses;
> +       struct TCP_Server_Info *server;
> +       int retries;
> +
> +       /*
> +        * SMB2s NegProt, SessSetup, Logoff do not have tcon yet so
> +        * check for tcp and smb session status done differently
> +        * for those three - in the calling routine.
> +        */
> +       if (tcon == NULL)
> +               return 0;
> +
> +       ses = tcon->ses;
> +       server = ses->server;
> +
> +       if (params->skip_reconnect)
> +               return 0;
> +
> +       if (tcon->tidStatus == CifsExiting) {
> +               if (params->exit_nodev) {
> +                       cifs_dbg(FYI, "return ENODEV while umounting\n");
> +                       return -ENODEV;
> +               }
> +       }
> +       if ((!ses) || (ses->status == CifsExiting) || (!server))
> +               return -EIO;
> +
> +       retries = server->nr_targets;
> +
> +       /*
> +        * Give demultiplex thread up to 10 seconds to each target available for
> +        * reconnect -- should be greater than cifs socket timeout which is 7
> +        * seconds.
> +        */
> +       while (server->tcpStatus == CifsNeedReconnect) {
> +               if (params->early_eagain) {
> +                       return -EAGAIN;
> +               }
> +
> +               rc = wait_event_interruptible_timeout(server->response_q,
> +                                                     (server->tcpStatus != CifsNeedReconnect),
> +                                                     10 * HZ);
> +               if (rc < 0) {
> +                       cifs_dbg(FYI, "%s: aborting reconnect due to a received"
> +                                " signal by the process\n", __func__);
> +                       return -ERESTARTSYS;
> +               }
> +
> +               /* are we still trying to reconnect? */
> +               if (server->tcpStatus != CifsNeedReconnect)
> +                       break;
> +
> +               if (retries && --retries)
> +                       continue;
> +
> +               /*
> +                * on "soft" mounts we wait once. Hard mounts keep
> +                * retrying until process is killed or server comes
> +                * back on-line
> +                */
> +               if (!tcon->retry) {
> +                       cifs_dbg(FYI, "gave up waiting on reconnect in smb_init\n");
> +                       return -EHOSTDOWN;
> +               }
> +               retries = server->nr_targets;
> +       }
> +
> +       if (!ses->need_reconnect && !tcon->need_reconnect)
> +               return 0;
> +
> +       nls_codepage = load_nls_default();
> +
> +       /*
> +        * need to prevent multiple threads trying to simultaneously reconnect
> +        * the same SMB session
> +        */
> +       mutex_lock(&ses->session_mutex);
> +
> +       /*
> +        * Recheck after acquire mutex. If another thread is negotiating
> +        * and the server never sends an answer the socket will be closed
> +        * and tcpStatus set to reconnect.
> +        */
> +       rc = cifs_connect_session_locked(0, ses,
> +                                        nls_codepage,
> +                                        tcon->retry);
> +       /* do we need to reconnect tcon? */
> +       if (rc || !tcon->need_reconnect) {
> +               mutex_unlock(&ses->session_mutex);
> +               goto out;
> +       }
> +
> +       cifs_mark_open_files_invalid(tcon);
> +       if (tcon->use_persistent)
> +               tcon->need_reopen_files = true;
> +
> +       rc = cifs_tree_connect(0, tcon, nls_codepage);
> +       mutex_unlock(&ses->session_mutex);
> +
> +       cifs_dbg(FYI, "reconnect tcon rc = %d\n", rc);
> +       if (rc) {
> +               /* If sess reconnected but tcon didn't, something strange ... */
> +               printk_once(KERN_WARNING "reconnect tcon failed rc = %d\n", rc);
> +               goto out;
> +       }
> +
> +       if (params->start_timer)
> +               mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
> +
> +       atomic_inc(&tconInfoReconnectCount);
> +out:
> +       if (params->late_eagain) {
> +               rc = -EAGAIN;
> +       }
> +
> +       unload_nls(nls_codepage);
> +       return rc;
> +}
> +
>  static int
>  cifs_set_vol_auth(struct smb_vol *vol, struct cifs_ses *ses)
>  {
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 715a50ffb234..162fe3381f4c 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -158,139 +158,48 @@ smb2_hdr_assemble(struct smb2_sync_hdr *shdr, __le16 smb2_cmd,
>  static int
>  smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon)
>  {
> -       int rc;
> -       struct nls_table *nls_codepage;
> -       struct cifs_ses *ses;
> -       struct TCP_Server_Info *server;
> -       int retries;
> -
> -       /*
> -        * SMB2s NegProt, SessSetup, Logoff do not have tcon yet so
> -        * check for tcp and smb session status done differently
> -        * for those three - in the calling routine.
> -        */
> -       if (tcon == NULL)
> -               return 0;
> +       struct cifs_tcon_reconnect_params params = {
> +               .skip_reconnect = false,
> +       };
>
> -       if (smb2_command == SMB2_TREE_CONNECT)
> -               return 0;
> -
> -       if (tcon->tidStatus == CifsExiting) {
> -               /*
> -                * only tree disconnect, open, and write,
> -                * (and ulogoff which does not have tcon)
> -                * are allowed as we start force umount.
> -                */
> -               if ((smb2_command != SMB2_WRITE) &&
> -                  (smb2_command != SMB2_CREATE) &&
> -                  (smb2_command != SMB2_TREE_DISCONNECT)) {
> -                       cifs_dbg(FYI, "can not send cmd %d while umounting\n",
> -                                smb2_command);
> -                       return -ENODEV;
> -               }
> +       switch (smb2_command) {
> +       case SMB2_TREE_CONNECT:
> +               params.skip_reconnect = true;
> +               break;
>         }
> -       if ((!tcon->ses) || (tcon->ses->status == CifsExiting) ||
> -           (!tcon->ses->server))
> -               return -EIO;
> -
> -       ses = tcon->ses;
> -       server = ses->server;
> -
> -       retries = server->nr_targets;
>
>         /*
> -        * Give demultiplex thread up to 10 seconds to each target available for
> -        * reconnect -- should be greater than cifs socket timeout which is 7
> -        * seconds.
> +        * only tree disconnect, open, and write,
> +        * (and ulogoff which does not have tcon)
> +        * are allowed as we start force umount.
>          */
> -       while (server->tcpStatus == CifsNeedReconnect) {
> -               /*
> -                * Return to caller for TREE_DISCONNECT and LOGOFF and CLOSE
> -                * here since they are implicitly done when session drops.
> -                */
> -               switch (smb2_command) {
> -               /*
> -                * BB Should we keep oplock break and add flush to exceptions?
> -                */
> -               case SMB2_TREE_DISCONNECT:
> -               case SMB2_CANCEL:
> -               case SMB2_CLOSE:
> -               case SMB2_OPLOCK_BREAK:
> -                       return -EAGAIN;
> -               }
> -
> -               rc = wait_event_interruptible_timeout(server->response_q,
> -                                                     (server->tcpStatus != CifsNeedReconnect),
> -                                                     10 * HZ);
> -               if (rc < 0) {
> -                       cifs_dbg(FYI, "%s: aborting reconnect due to a received"
> -                                " signal by the process\n", __func__);
> -                       return -ERESTARTSYS;
> -               }
> -
> -               /* are we still trying to reconnect? */
> -               if (server->tcpStatus != CifsNeedReconnect)
> -                       break;
> -
> -               if (retries && --retries)
> -                       continue;
> -
> -               /*
> -                * on "soft" mounts we wait once. Hard mounts keep
> -                * retrying until process is killed or server comes
> -                * back on-line
> -                */
> -               if (!tcon->retry) {
> -                       cifs_dbg(FYI, "gave up waiting on reconnect in smb_init\n");
> -                       return -EHOSTDOWN;
> -               }
> -               retries = server->nr_targets;
> +       switch (smb2_command) {
> +       case SMB2_WRITE:
> +       case SMB2_CREATE:
> +       case SMB2_TREE_DISCONNECT:
> +               params.exit_nodev = true;
> +               break;
>         }
>
> -       if (!tcon->ses->need_reconnect && !tcon->need_reconnect)
> -               return 0;
> -
> -       nls_codepage = load_nls_default();
> -
>         /*
> -        * need to prevent multiple threads trying to simultaneously reconnect
> -        * the same SMB session
> +        * Return to caller for TREE_DISCONNECT and LOGOFF and CLOSE
> +        * here since they are implicitly done when session drops.
>          */
> -       mutex_lock(&tcon->ses->session_mutex);
> -
> +       switch (smb2_command) {
>         /*
> -        * Recheck after acquire mutex. If another thread is negotiating
> -        * and the server never sends an answer the socket will be closed
> -        * and tcpStatus set to reconnect.
> +        * BB Should we keep oplock break and add flush to exceptions?
>          */
> -       rc = cifs_connect_session_locked(0, tcon->ses,
> -                                        nls_codepage,
> -                                        tcon->retry);
> -       /* do we need to reconnect tcon? */
> -       if (rc || !tcon->need_reconnect) {
> -               mutex_unlock(&tcon->ses->session_mutex);
> -               goto out;
> -       }
> -
> -       cifs_mark_open_files_invalid(tcon);
> -       if (tcon->use_persistent)
> -               tcon->need_reopen_files = true;
> -
> -       rc = cifs_tree_connect(0, tcon, nls_codepage);
> -       mutex_unlock(&tcon->ses->session_mutex);
> -
> -       cifs_dbg(FYI, "reconnect tcon rc = %d\n", rc);
> -       if (rc) {
> -               /* If sess reconnected but tcon didn't, something strange ... */
> -               printk_once(KERN_WARNING "reconnect tcon failed rc = %d\n", rc);
> -               goto out;
> +       case SMB2_TREE_DISCONNECT:
> +       case SMB2_CANCEL:
> +       case SMB2_CLOSE:
> +       case SMB2_OPLOCK_BREAK:
> +               params.early_eagain = true;
> +               break;
>         }
>
>         if (smb2_command != SMB2_INTERNAL_CMD)
> -               mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
> +               params.start_timer = true;
>
> -       atomic_inc(&tconInfoReconnectCount);
> -out:
>         /*
>          * Check if handle based operation so we know whether we can continue
>          * or not without returning to caller to reset file handle.
> @@ -309,11 +218,11 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon)
>         case SMB2_CHANGE_NOTIFY:
>         case SMB2_QUERY_INFO:
>         case SMB2_SET_INFO:
> -               rc = -EAGAIN;
> +               params.late_eagain = true;
> +               break;
>         }
>
> -       unload_nls(nls_codepage);
> -       return rc;
> +       return cifs_tcon_reconnect(tcon, &params);
>  }
>
>  static void
> --
> 2.17.1
>


--
Thanks,

Steve

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2020-03-27 17:32 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-24 13:14 [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Stefan Metzmacher
2020-02-24 13:14 ` [PATCH v1 01/13] cifs: call wake_up(&server->response_q) inside of cifs_reconnect() Stefan Metzmacher
2020-02-24 13:14 ` [PATCH v1 02/13] cifs: use mod_delayed_work() for &server->reconnect if already queued Stefan Metzmacher
2020-02-24 13:15 ` [PATCH v1 03/13] cifs: make use of cap_unix(ses) in cifs_reconnect_tcon() Stefan Metzmacher
2020-02-24 18:12   ` Aurélien Aptel
2020-02-24 20:39     ` Steve French
2020-02-24 21:36       ` Pavel Shilovsky
2020-02-24 13:15 ` [PATCH v1 04/13] cifs: split out a cifs_connect_session_locked() helper function Stefan Metzmacher
2020-03-06 17:05   ` Aurélien Aptel
2020-02-24 13:15 ` [PATCH v1 05/13] cifs: merge __{cifs,smb2}_reconnect[_tcon]() into cifs_tree_connect() Stefan Metzmacher
2020-03-06 16:46   ` Aurélien Aptel
2020-02-24 13:15 ` [PATCH v1 06/13] cifs: abstract cifs_tcon_reconnect() out of smb2_reconnect() Stefan Metzmacher
2020-03-27 17:31   ` Steve French
2020-02-24 13:15 ` [PATCH v1 07/13] cifs: cifs_reconnect_tcon() make use of the generic cifs_tcon_reconnect() function Stefan Metzmacher
2020-02-24 13:15 ` [PATCH v1 08/13] cifs: make cifs_tree_connect() static Stefan Metzmacher
2020-02-24 13:15 ` [PATCH v1 09/13] cifs: turn smb2_reconnect_server() into a generic cifs_reconnect_server() Stefan Metzmacher
2020-02-24 13:15 ` [PATCH v1 10/13] cifs: move cifs_reconnect_tcons() to fs/cifs/connect.c and make it static Stefan Metzmacher
2020-02-24 13:15 ` [PATCH v1 11/13] cifs: use reconnect timer also for SMB1 Stefan Metzmacher
2020-02-24 13:15 ` [PATCH v1 12/13] cifs: map STATUS_ACCOUNT_LOCKED_OUT to -EACCES Stefan Metzmacher
2020-02-24 13:15 ` [PATCH v1 13/13] cifs: introduce the CifsInvalidCredentials session state Stefan Metzmacher
2020-02-24 18:09   ` Aurélien Aptel
2020-02-24 18:38 ` [PATCH v1 00/13] Avoid reconnects of failed session setups on soft mounts Aurélien Aptel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).