[PATCH] CIFS: Call echo service immediately after socket reconnect

[PATCH] CIFS: Call echo service immediately after socket reconnect

[Pavel Shilovsky]

From: Sachin Prabhu <sprabhu@redhat.com>

Commit b8c600120fc8 ("Call echo service immediately after socket
reconnect") upstream.

Commit 4fcd1813e640 ("Fix reconnect to not defer smb3 session reconnect
long after socket reconnect") changes the behaviour of the SMB2 echo
service and causes it to renegotiate after a socket reconnect. However
under default settings, the echo service could take up to 120 seconds to
be scheduled.

The patch forces the echo service to be called immediately resulting a
negotiate call being made immediately on reconnect.

Cc: <stable@vger.kernel.org> # 3.18.x-4.4.x
Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
Signed-off-by: Steve French <smfrench@gmail.com>
Acked-by: Sachin Prabhu <sprabhu@redhat.com>
Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
---
 fs/cifs/connect.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 156bc18..53a827c 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -412,6 +412,9 @@ cifs_reconnect(struct TCP_Server_Info *server)
 		}
 	} while (server->tcpStatus == CifsNeedReconnect);
 
+	if (server->tcpStatus == CifsNeedNegotiate)
+		mod_delayed_work(cifsiod_wq, &server->echo, 0);
+
 	return rc;
 }
 
@@ -421,18 +424,27 @@ cifs_echo_request(struct work_struct *work)
 	int rc;
 	struct TCP_Server_Info *server = container_of(work,
 					struct TCP_Server_Info, echo.work);
+	unsigned long echo_interval;
+
+	/*
+	 * If we need to renegotiate, set echo interval to zero to
+	 * immediately call echo service where we can renegotiate.
+	 */
+	if (server->tcpStatus == CifsNeedNegotiate)
+		echo_interval = 0;
+	else
+		echo_interval = SMB_ECHO_INTERVAL;
 
 	/*
-	 * We cannot send an echo if it is disabled or until the
-	 * NEGOTIATE_PROTOCOL request is done, which is indicated by
-	 * server->ops->need_neg() == true. Also, no need to ping if
-	 * we got a response recently.
+	 * We cannot send an echo if it is disabled.
+	 * Also, no need to ping if we got a response recently.
 	 */
 
 	if (server->tcpStatus == CifsNeedReconnect ||
-	    server->tcpStatus == CifsExiting || server->tcpStatus == CifsNew ||
+	    server->tcpStatus == CifsExiting ||
+	    server->tcpStatus == CifsNew ||
 	    (server->ops->can_echo && !server->ops->can_echo(server)) ||
-	    time_before(jiffies, server->lstrp + SMB_ECHO_INTERVAL - HZ))
+	    time_before(jiffies, server->lstrp + echo_interval - HZ))
 		goto requeue_echo;
 
 	rc = server->ops->echo ? server->ops->echo(server) : -ENOSYS;
-- 
2.7.4

Re: [PATCH] CIFS: Call echo service immediately after socket reconnect

[Greg KH]

On Mon, Jun 12, 2017 at 12:38:35PM -0700, Pavel Shilovsky wrote:
> From: Sachin Prabhu <sprabhu@redhat.com>
> 
> Commit b8c600120fc8 ("Call echo service immediately after socket
> reconnect") upstream.
> 
> Commit 4fcd1813e640 ("Fix reconnect to not defer smb3 session reconnect
> long after socket reconnect") changes the behaviour of the SMB2 echo
> service and causes it to renegotiate after a socket reconnect. However
> under default settings, the echo service could take up to 120 seconds to
> be scheduled.
> 
> The patch forces the echo service to be called immediately resulting a
> negotiate call being made immediately on reconnect.
> 
> Cc: <stable@vger.kernel.org> # 3.18.x-4.4.x

Thanks for the backport, now applied.

greg k-h

RE: [PATCH] CIFS: Call echo service immediately after socket reconnect

[Pavel Shilovskiy]

2017-06-15 0:23 GMT-07:00 Greg KH <gregkh@linuxfoundation.org>:
> On Mon, Jun 12, 2017 at 12:38:35PM -0700, Pavel Shilovsky wrote:
>> From: Sachin Prabhu <sprabhu@redhat.com>
>>
>> Commit b8c600120fc8 ("Call echo service immediately after socket
>> reconnect") upstream.
>>
>> Commit 4fcd1813e640 ("Fix reconnect to not defer smb3 session reconnect
>> long after socket reconnect") changes the behaviour of the SMB2 echo
>> service and causes it to renegotiate after a socket reconnect. However
>> under default settings, the echo service could take up to 120 seconds to
>> be scheduled.
>>
>> The patch forces the echo service to be called immediately resulting a
>> negotiate call being made immediately on reconnect.
>>
>> Cc: <stable@vger.kernel.org> # 3.18.x-4.4.x
>
> Thanks for the backport, now applied.

Thanks.

(adding Ben Hutchings)

I am preparing a backport of this patch for 3.16.x kernel and have a question. This patch (A) was merged into the mainline without a stable tag. After a problem was discovered with the patch, a new patch 62a6cfddcc0a "cifs: Do not send echoes before Negotiate is complete" (B) was merged into the mainline with a stable tag.

This ended up with the patch B merged into stable kernels (nothing bad about it, it does the right things anyway) before the patch A (which is only merged now). Stable tree 3.16 doesn't have both patches A or B. The question is: what is the best way to deal with this situation? I see 3 options here:

a) Send the patch A, then B which matches the mainline order; this results in stable 3.16.x kernel broken between A and B points.
b) Send the patch B, then A which matches other stable trees order.
c) Squash B into A and send one correct patch.

Thoughts?

--
Best regards,
Pavel Shilovsky

RE: [PATCH] CIFS: Call echo service immediately after socket reconnect

[Pavel Shilovskiy]

2017-06-15 13:43 GMT-07:00 Pavel Shilovskiy <pshilov@microsoft.com>:
> 2017-06-15 0:23 GMT-07:00 Greg KH <gregkh@linuxfoundation.org>:
>> On Mon, Jun 12, 2017 at 12:38:35PM -0700, Pavel Shilovsky wrote:
>>> From: Sachin Prabhu <sprabhu@redhat.com>
>>>
>>> Commit b8c600120fc8 ("Call echo service immediately after socket
>>> reconnect") upstream.
>>>
>>> Commit 4fcd1813e640 ("Fix reconnect to not defer smb3 session reconnect
>>> long after socket reconnect") changes the behaviour of the SMB2 echo
>>> service and causes it to renegotiate after a socket reconnect. However
>>> under default settings, the echo service could take up to 120 seconds to
>>> be scheduled.
>>>
>>> The patch forces the echo service to be called immediately resulting a
>>> negotiate call being made immediately on reconnect.
>>>
>>> Cc: <stable@vger.kernel.org> # 3.18.x-4.4.x
>>
>> Thanks for the backport, now applied.
>
> Thanks.
>
> (adding Ben Hutchings)
>
> I am preparing a backport of this patch for 3.16.x kernel and have a question. This patch (A) was merged into the mainline without a stable tag. After a problem was discovered with the patch, a new patch 62a6cfddcc0a "cifs: Do not send echoes before Negotiate is complete" (B) was merged into the mainline with a stable tag.
>
> This ended up with the patch B merged into stable kernels (nothing bad about it, it does the right things anyway) before the patch A (which is only merged now). Stable tree 3.16 doesn't have both patches A or B. The question is: what is the best way to deal with this situation? I see 3 options here:
>
> a) Send the patch A, then B which matches the mainline order; this results in stable 3.16.x kernel broken between A and B points.
> b) Send the patch B, then A which matches other stable trees order.
> c) Squash B into A and send one correct patch.

Do you have any preferences in a way how above patches should be backported and submitted to stable 3.16.x tree?

--
Best regards,
Pavel Shilovsky

[PATCH] CIFS: Call echo service immediately after socket reconnect

[Pavel Shilovsky]

From: Sachin Prabhu <sprabhu@redhat.com>

Commit b8c600120fc8 ("Call echo service immediately after socket
reconnect") upstream.

Commit 4fcd1813e640 ("Fix reconnect to not defer smb3 session reconnect
long after socket reconnect") changes the behaviour of the SMB2 echo
service and causes it to renegotiate after a socket reconnect. However
under default settings, the echo service could take up to 120 seconds to
be scheduled.

The patch forces the echo service to be called immediately resulting a
negotiate call being made immediately on reconnect.

Cc: <stable@vger.kernel.org> # 3.16.x
Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
Signed-off-by: Steve French <smfrench@gmail.com>
Acked-by: Sachin Prabhu <sprabhu@redhat.com>
Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
---
 fs/cifs/connect.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 95a9573..ac042a0 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -397,6 +397,9 @@ cifs_reconnect(struct TCP_Server_Info *server)
 		mutex_unlock(&server->srv_mutex);
 	} while (server->tcpStatus == CifsNeedReconnect);
 
+	if (server->tcpStatus == CifsNeedNegotiate)
+		mod_delayed_work(cifsiod_wq, &server->echo, 0);
+
 	return rc;
 }
 
@@ -406,18 +409,27 @@ cifs_echo_request(struct work_struct *work)
 	int rc;
 	struct TCP_Server_Info *server = container_of(work,
 					struct TCP_Server_Info, echo.work);
+	unsigned long echo_interval;
+
+	/*
+	 * If we need to renegotiate, set echo interval to zero to
+	 * immediately call echo service where we can renegotiate.
+	 */
+	if (server->tcpStatus == CifsNeedNegotiate)
+		echo_interval = 0;
+	else
+		echo_interval = SMB_ECHO_INTERVAL;
 
 	/*
-	 * We cannot send an echo if it is disabled or until the
-	 * NEGOTIATE_PROTOCOL request is done, which is indicated by
-	 * server->ops->need_neg() == true. Also, no need to ping if
-	 * we got a response recently.
+	 * We cannot send an echo if it is disabled.
+	 * Also, no need to ping if we got a response recently.
 	 */
 
 	if (server->tcpStatus == CifsNeedReconnect ||
-	    server->tcpStatus == CifsExiting || server->tcpStatus == CifsNew ||
+	    server->tcpStatus == CifsExiting ||
+	    server->tcpStatus == CifsNew ||
 	    (server->ops->can_echo && !server->ops->can_echo(server)) ||
-	    time_before(jiffies, server->lstrp + SMB_ECHO_INTERVAL - HZ))
+	    time_before(jiffies, server->lstrp + echo_interval - HZ))
 		goto requeue_echo;
 
 	rc = server->ops->echo ? server->ops->echo(server) : -ENOSYS;
-- 
2.7.4