Interop Issue: SMB2+ async replies, and the kernel, Samba side fix enclosed.

Interop Issue: SMB2+ async replies, and the kernel, Samba side fix enclosed.

[Ira Cooper]

[-- Attachment #1: Type: text/plain, Size: 1229 bytes --]

If the server sends an interim response, then the real response, the real
response, is handled by standard_receive3() in the kernel, instead of the
proper function, and this causes a disconnect.  If there isn't a
disconnect, I believe the reply will just be discarded if I understand the
code correctly.  (That is a big if here ;) )

I've written a patch for Samba to stop sending interim replies on SMB2_READ
and SMB2_WRITE, when non-compounded to stop the impact of this issue.  We
may want to add SMB2_CREATE to the list of ops we don't send async replies
for non-compounded, but I'm not sold either way, I know we CAN go async
there!  I want some opinions here.

This is not hidden behind a flag because compatibility issues that don't
impact protocol correctness usually aren't.

Setting req->async_te = talloc_new(NULL); is just ugly, though it works.
If you have a cleaner approach, I welcome it.

I request you please ASK me before pushing this one, yes, that means you
jra!

For those interested in reproduction: I'd suggest using a server that's
rebuilt with a lower timeout set in smb2_read.c, though we've hit it with
vfs_glusterfs straight up, in our testing.

Thanks,

-Ira / ira@(samba.org|redhat.com|wakeful.net)

[-- Attachment #2: 0001-smbd-Work-around-for-AIO-with-the-Linux-CIFS-client-.patch --]
[-- Type: application/mbox, Size: 1808 bytes --]

Re: Interop Issue: SMB2+ async replies, and the kernel, Samba side fix enclosed.

[Stefan Metzmacher]

[-- Attachment #1: Type: text/plain, Size: 1278 bytes --]

Hi Ira,

> If the server sends an interim response, then the real response, the real
> response, is handled by standard_receive3() in the kernel, instead of the
> proper function, and this causes a disconnect.  If there isn't a
> disconnect, I believe the reply will just be discarded if I understand the
> code correctly.  (That is a big if here ;) )
> 
> I've written a patch for Samba to stop sending interim replies on SMB2_READ
> and SMB2_WRITE, when non-compounded to stop the impact of this issue.  We
> may want to add SMB2_CREATE to the list of ops we don't send async replies
> for non-compounded, but I'm not sold either way, I know we CAN go async
> there!  I want some opinions here.
> 
> This is not hidden behind a flag because compatibility issues that don't
> impact protocol correctness usually aren't.
> 
> Setting req->async_te = talloc_new(NULL); is just ugly, though it works.
> If you have a cleaner approach, I welcome it.

Fix the broken kernel client!

I think this is nothing we should work around in the server.
The situation might be different if Windows clients would be unhappy,
but the linux client is more or less under our control and can be fixed.

People hitting this can just use SMB1 until it's fixed.

metze


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Interop Issue: SMB2+ async replies, and the kernel, Samba side fix enclosed.

[Ira Cooper]

You'll find it easy to reproduce if you:

Set  aio read size = 1 in smb.conf.

And modify line 101 in smb2_read.c or close by to be:

    return smbd_smb2_request_pending_queue(req, subreq, 1);

Writing a 1 GB file, umounting the share, and mounting the share again,
then using dd to read it back with rsize and wsize set to 1MB, should do it.

We can reproduce it as is with Gluster, I suspect that may be enough to do
it with XFS or other filesystems.

Does anyone know what the windows timeout for sending the interim reply
is?  Barring this bug, sending it when we don't have to is wasteful.

Thanks,

-Ira

On Tue, Feb 23, 2016 at 11:11 PM, Steven French <sfrench@samba.org> wrote:

> Do we have a reproduction scenario?  I don’t remember this one and it
> doesn’t send familiar at all - I use smb3 kernel client to Samba every day.
>
> And yes - if we have a bug in the kernel client here let’s fix it.
>
>
>
> On Feb 23, 2016, at 7:05 AM, Stefan Metzmacher <metze@samba.org> wrote:
>
> Hi Ira,
>
> If the server sends an interim response, then the real response, the real
> response, is handled by standard_receive3() in the kernel, instead of the
> proper function, and this causes a disconnect.  If there isn't a
> disconnect, I believe the reply will just be discarded if I understand the
> code correctly.  (That is a big if here ;) )
>
> I've written a patch for Samba to stop sending interim replies on SMB2_READ
> and SMB2_WRITE, when non-compounded to stop the impact of this issue.  We
> may want to add SMB2_CREATE to the list of ops we don't send async replies
> for non-compounded, but I'm not sold either way, I know we CAN go async
> there!  I want some opinions here.
>
> This is not hidden behind a flag because compatibility issues that don't
> impact protocol correctness usually aren't.
>
> Setting req->async_te = talloc_new(NULL); is just ugly, though it works.
> If you have a cleaner approach, I welcome it.
>
>
> Fix the broken kernel client!
>
> I think this is nothing we should work around in the server.
> The situation might be different if Windows clients would be unhappy,
> but the linux client is more or less under our control and can be fixed.
>
> People hitting this can just use SMB1 until it's fixed.
>
> metze
>
>
>

Re: Interop Issue: SMB2+ async replies, and the kernel, Samba side fix enclosed.

[Jeremy Allison]

On Tue, Feb 23, 2016 at 11:41:34PM -0500, Ira Cooper wrote:
> You'll find it easy to reproduce if you:
> 
> Set  aio read size = 1 in smb.conf.
> 
> And modify line 101 in smb2_read.c or close by to be:
> 
>     return smbd_smb2_request_pending_queue(req, subreq, 1);
> 
> Writing a 1 GB file, umounting the share, and mounting the share again,
> then using dd to read it back with rsize and wsize set to 1MB, should do it.
> 
> We can reproduce it as is with Gluster, I suspect that may be enough to do
> it with XFS or other filesystems.
> 
> Does anyone know what the windows timeout for sending the interim reply
> is?  Barring this bug, sending it when we don't have to is wasteful.

Hmmm. We can only test this by causing a Windows read to
take a long time. Any idea how to test this ? Does Win32
have named pipes in the fs we could use for that ? If not
we could test using a program that creates a \\pipe\named_pipe
and then responds slowly... But I don't know if the
timeout replies on np's are the same as in the filesystem.

Re: Interop Issue: SMB2+ async replies, and the kernel, Samba side fix enclosed.

[Volker Lendecke]

On Wed, Feb 24, 2016 at 11:04:08AM -0800, Jeremy Allison wrote:
> Hmmm. We can only test this by causing a Windows read to
> take a long time. Any idea how to test this ? Does Win32
> have named pipes in the fs we could use for that ? If not
> we could test using a program that creates a \\pipe\named_pipe
> and then responds slowly... But I don't know if the
> timeout replies on np's are the same as in the filesystem.

Just use a USB2 stick.

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt@sernet.de

Re: Interop Issue: SMB2+ async replies, and the kernel, Samba side fix enclosed.

[Christian Ambach]

Am 01.03.16 um 14:40 schrieb Volker Lendecke:
> On Wed, Feb 24, 2016 at 11:04:08AM -0800, Jeremy Allison wrote:
>> Hmmm. We can only test this by causing a Windows read to
>> take a long time. Any idea how to test this ? Does Win32
>> have named pipes in the fs we could use for that ? If not
>> we could test using a program that creates a \\pipe\named_pipe
>> and then responds slowly... But I don't know if the
>> timeout replies on np's are the same as in the filesystem.
>
> Just use a USB2 stick.
>
[MS-SMB2] gives some hints when Windows will send out interim replies:
<296> Section 3.3.5.12: Windows SMB2 servers send an interim response to 
the client and handle the read asynchronously if the read is not 
finished in 0.5 milliseconds.
<297> Section 3.3.5.12: Windows-based servers handle the following
commands asynchronously: SMB2 Create (section 2.2.13) when this create would
result in an oplock break, SMB2 IOCTL Request (section 2.2.31) for
FSCTL_PIPE_TRANSCEIVE if it blocks for more than 1 millisecond, SMB2 IOCTL
Request for FSCTL_SRV_COPYCHUNK or FSCTL_SRV_COPYCHUNK_WRITE (section 
2.2.31)
when oplock break happens, SMB2 Change_Notify Request (section 2.2.35) if
it blocks for more than 0.5 milliseconds, SMB2 Read request (section 2.2.19)
for named pipes if it blocks for more than 0.5 milliseconds,
SMB2 Write request (section 2.2.21) for named pipes if it blocks for more
than 0.5 milliseconds, SMB2 Write Request (section 2.2.21) for large 
file write,
SMB2 lock request (section 2.2.26) if the LOCKFLAG_FAIL_IMMEDIATELY flag 
is not set,
and SMB2 FLUSH Request (section 2.2.17) for named pipes.

Cheers,
Christian

Re: Interop Issue: SMB2+ async replies, and the kernel, Samba side fix enclosed.

[Shirish Pargaonkar]

I think the course of action on cifs client should be realize this is
in interim response
and drop it without percolating up and wait for the real reply.
Looking into that...

Regards,

Shirish

On Tue, Feb 23, 2016 at 6:55 AM, Ira Cooper <ira-mA6sH8tAw6XR7s880joybQ@public.gmane.org> wrote:
> If the server sends an interim response, then the real response, the real
> response, is handled by standard_receive3() in the kernel, instead of the
> proper function, and this causes a disconnect.  If there isn't a
> disconnect, I believe the reply will just be discarded if I understand the
> code correctly.  (That is a big if here ;) )
>
> I've written a patch for Samba to stop sending interim replies on SMB2_READ
> and SMB2_WRITE, when non-compounded to stop the impact of this issue.  We
> may want to add SMB2_CREATE to the list of ops we don't send async replies
> for non-compounded, but I'm not sold either way, I know we CAN go async
> there!  I want some opinions here.
>
> This is not hidden behind a flag because compatibility issues that don't
> impact protocol correctness usually aren't.
>
> Setting req->async_te = talloc_new(NULL); is just ugly, though it works.
> If you have a cleaner approach, I welcome it.
>
> I request you please ASK me before pushing this one, yes, that means you
> jra!
>
> For those interested in reproduction: I'd suggest using a server that's
> rebuilt with a lower timeout set in smb2_read.c, though we've hit it with
> vfs_glusterfs straight up, in our testing.
>
> Thanks,
>
> -Ira / ira@(samba.org|redhat.com|wakeful.net)

Re: Interop Issue: SMB2+ async replies, and the kernel, Samba side fix enclosed.

[Jeremy Allison]

On Tue, Feb 23, 2016 at 07:55:59AM -0500, Ira Cooper wrote:
> If the server sends an interim response, then the real response, the real
> response, is handled by standard_receive3() in the kernel, instead of the
> proper function, and this causes a disconnect.  If there isn't a
> disconnect, I believe the reply will just be discarded if I understand the
> code correctly.  (That is a big if here ;) )
> 
> I've written a patch for Samba to stop sending interim replies on SMB2_READ
> and SMB2_WRITE, when non-compounded to stop the impact of this issue.  We
> may want to add SMB2_CREATE to the list of ops we don't send async replies
> for non-compounded, but I'm not sold either way, I know we CAN go async
> there!  I want some opinions here.
> 
> This is not hidden behind a flag because compatibility issues that don't
> impact protocol correctness usually aren't.
> 
> Setting req->async_te = talloc_new(NULL); is just ugly, though it works.
> If you have a cleaner approach, I welcome it.
> 
> I request you please ASK me before pushing this one, yes, that means you
> jra!

Actually I wasn't planning on pushing at all :-).

I don't think we should stop sending interim replies on SMB2_READ
or SMB2_WRITE, those replies have effects on the client timers
on properly functioning clients.

Linux client needs fixing here I'm afraid (and I know that's
not fun, sorry :-( ).

Jeremy.

Re: Interop Issue: SMB2+ async replies, and the kernel, Samba side fix enclosed.

[Pavel Shilovsky]

2016-02-27 12:11 GMT+03:00 Pavel Shilovsky <pshilovsky@samba.org>:

> 2016-02-23 15:55 GMT+03:00 Ira Cooper <ira@wakeful.net>:
>
>> If the server sends an interim response, then the real response, the real
>> response, is handled by standard_receive3() in the kernel, instead of the
>> proper function, and this causes a disconnect.  If there isn't a
>> disconnect, I believe the reply will just be discarded if I understand the
>> code correctly.  (That is a big if here ;) )
>>
>> I've written a patch for Samba to stop sending interim replies on
>> SMB2_READ
>> and SMB2_WRITE, when non-compounded to stop the impact of this issue.  We
>> may want to add SMB2_CREATE to the list of ops we don't send async replies
>> for non-compounded, but I'm not sold either way, I know we CAN go async
>> there!  I want some opinions here.
>>
>> This is not hidden behind a flag because compatibility issues that don't
>> impact protocol correctness usually aren't.
>>
>> Setting req->async_te = talloc_new(NULL); is just ugly, though it works.
>> If you have a cleaner approach, I welcome it.
>>
>> I request you please ASK me before pushing this one, yes, that means you
>> jra!
>>
>> For those interested in reproduction: I'd suggest using a server that's
>> rebuilt with a lower timeout set in smb2_read.c, though we've hit it with
>> vfs_glusterfs straight up, in our testing.
>>
>> Thanks,
>>
>> -Ira / ira@(samba.org|redhat.com|wakeful.net)
>>
>
>
> Thank you for catching this!
>
> It is the issue in the kernel client - a check for interim responses is
> missed for SMB2_READ command. I created a patch that should fix the problem.
>
> Could you please test it?
>
> --
> Best regards,
> Pavel Shilovsky
>
>
CC'ing @samba-technical from another email address (my original email was
rejected by the server).

>

Re: Interop Issue: SMB2+ async replies, and the kernel, Samba side fix enclosed.

[Pavel Shilovsky]

[-- Attachment #1: Type: text/plain, Size: 1885 bytes --]

2016-02-27 12:11 GMT+03:00 Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>:
>
> 2016-02-23 15:55 GMT+03:00 Ira Cooper <ira-mA6sH8tAw6XR7s880joybQ@public.gmane.org>:
>>
>> If the server sends an interim response, then the real response, the real
>> response, is handled by standard_receive3() in the kernel, instead of the
>> proper function, and this causes a disconnect.  If there isn't a
>> disconnect, I believe the reply will just be discarded if I understand the
>> code correctly.  (That is a big if here ;) )
>>
>> I've written a patch for Samba to stop sending interim replies on SMB2_READ
>> and SMB2_WRITE, when non-compounded to stop the impact of this issue.  We
>> may want to add SMB2_CREATE to the list of ops we don't send async replies
>> for non-compounded, but I'm not sold either way, I know we CAN go async
>> there!  I want some opinions here.
>>
>> This is not hidden behind a flag because compatibility issues that don't
>> impact protocol correctness usually aren't.
>>
>> Setting req->async_te = talloc_new(NULL); is just ugly, though it works.
>> If you have a cleaner approach, I welcome it.
>>
>> I request you please ASK me before pushing this one, yes, that means you
>> jra!
>>
>> For those interested in reproduction: I'd suggest using a server that's
>> rebuilt with a lower timeout set in smb2_read.c, though we've hit it with
>> vfs_glusterfs straight up, in our testing.
>>
>> Thanks,
>>
>> -Ira / ira@(samba.org|redhat.com|wakeful.net)
>
>
>
> Thank you for catching this!
>
> It is the issue in the kernel client - a check for interim responses is missed for SMB2_READ command. I created a patch that should fix the problem.
>
> Could you please test it?
>
> --
> Best regards,
> Pavel Shilovsky

CC'ing @samba-technical (my original email was rejected by the server).

Sorry for the spam.

-- 
Best regards,
Pavel Shilovsky

[-- Attachment #2: 0001-CIFS-Fix-SMB2-interim-response-processing-for-read-r.patch --]
[-- Type: text/x-patch, Size: 2082 bytes --]

From 09904bf0c6a1ecc7f61d4755db33b762b53176d6 Mon Sep 17 00:00:00 2001
From: Pavel Shilovsky <pshilovsky@samba.org>
Date: Sat, 27 Feb 2016 11:58:18 +0300
Subject: [PATCH] CIFS: Fix SMB2+ interim response processing for read requests

For interim responses we only need to parse a header and update
a number credits. Now it is done for all SMB2+ command except
SMB2_READ which is wrong. Fix this by adding such processing.

Signed-off-by: Pavel Shilovsky <pshilovsky@samba.org>
---
 fs/cifs/cifssmb.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 90b4f9f..76fcb50 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -1396,11 +1396,10 @@ openRetry:
  * current bigbuf.
  */
 static int
-cifs_readv_discard(struct TCP_Server_Info *server, struct mid_q_entry *mid)
+discard_remaining_data(struct TCP_Server_Info *server)
 {
 	unsigned int rfclen = get_rfc1002_length(server->smallbuf);
 	int remaining = rfclen + 4 - server->total_read;
-	struct cifs_readdata *rdata = mid->callback_data;
 
 	while (remaining > 0) {
 		int length;
@@ -1414,10 +1413,20 @@ cifs_readv_discard(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 		remaining -= length;
 	}
 
-	dequeue_mid(mid, rdata->result);
 	return 0;
 }
 
+static int
+cifs_readv_discard(struct TCP_Server_Info *server, struct mid_q_entry *mid)
+{
+	int length;
+	struct cifs_readdata *rdata = mid->callback_data;
+
+	length = discard_remaining_data(server);
+	dequeue_mid(mid, rdata->result);
+	return length;
+}
+
 int
 cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 {
@@ -1446,6 +1455,12 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 		return length;
 	server->total_read += length;
 
+	if (server->ops->is_status_pending &&
+	    server->ops->is_status_pending(buf, server, 0)) {
+		discard_remaining_data(server);
+		return -1;
+	}
+
 	/* Was the SMB read successful? */
 	rdata->result = server->ops->map_error(buf, false);
 	if (rdata->result != 0) {
-- 
2.1.4

Re: Interop Issue: SMB2+ async replies, and the kernel, Samba side fix enclosed.

[Pavel Shilovsky]

[-- Attachment #1: Type: text/plain, Size: 1841 bytes --]

2016-02-27 12:11 GMT+03:00 Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>:
> 2016-02-23 15:55 GMT+03:00 Ira Cooper <ira-mA6sH8tAw6XR7s880joybQ@public.gmane.org>:
>>
>> If the server sends an interim response, then the real response, the real
>> response, is handled by standard_receive3() in the kernel, instead of the
>> proper function, and this causes a disconnect.  If there isn't a
>> disconnect, I believe the reply will just be discarded if I understand the
>> code correctly.  (That is a big if here ;) )
>>
>> I've written a patch for Samba to stop sending interim replies on
>> SMB2_READ
>> and SMB2_WRITE, when non-compounded to stop the impact of this issue.  We
>> may want to add SMB2_CREATE to the list of ops we don't send async replies
>> for non-compounded, but I'm not sold either way, I know we CAN go async
>> there!  I want some opinions here.
>>
>> This is not hidden behind a flag because compatibility issues that don't
>> impact protocol correctness usually aren't.
>>
>> Setting req->async_te = talloc_new(NULL); is just ugly, though it works.
>> If you have a cleaner approach, I welcome it.
>>
>> I request you please ASK me before pushing this one, yes, that means you
>> jra!
>>
>> For those interested in reproduction: I'd suggest using a server that's
>> rebuilt with a lower timeout set in smb2_read.c, though we've hit it with
>> vfs_glusterfs straight up, in our testing.
>>
>> Thanks,
>>
>> -Ira / ira@(samba.org|redhat.com|wakeful.net)
>
>
>
> Thank you for catching this!
>
> It is the issue in the kernel client - a check for interim responses is
> missed for SMB2_READ command. I created a patch that should fix the problem.
>
> Could you please test it?
>
> --
> Best regards,
> Pavel Shilovsky

CC'ing @samba-technical.

Sorry for the spam.

-- 
Best regards,
Pavel Shilovsky

[-- Attachment #2: 0001-CIFS-Fix-SMB2-interim-response-processing-for-read-r.patch --]
[-- Type: text/x-patch, Size: 2082 bytes --]

From 09904bf0c6a1ecc7f61d4755db33b762b53176d6 Mon Sep 17 00:00:00 2001
From: Pavel Shilovsky <pshilovsky@samba.org>
Date: Sat, 27 Feb 2016 11:58:18 +0300
Subject: [PATCH] CIFS: Fix SMB2+ interim response processing for read requests

For interim responses we only need to parse a header and update
a number credits. Now it is done for all SMB2+ command except
SMB2_READ which is wrong. Fix this by adding such processing.

Signed-off-by: Pavel Shilovsky <pshilovsky@samba.org>
---
 fs/cifs/cifssmb.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 90b4f9f..76fcb50 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -1396,11 +1396,10 @@ openRetry:
  * current bigbuf.
  */
 static int
-cifs_readv_discard(struct TCP_Server_Info *server, struct mid_q_entry *mid)
+discard_remaining_data(struct TCP_Server_Info *server)
 {
 	unsigned int rfclen = get_rfc1002_length(server->smallbuf);
 	int remaining = rfclen + 4 - server->total_read;
-	struct cifs_readdata *rdata = mid->callback_data;
 
 	while (remaining > 0) {
 		int length;
@@ -1414,10 +1413,20 @@ cifs_readv_discard(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 		remaining -= length;
 	}
 
-	dequeue_mid(mid, rdata->result);
 	return 0;
 }
 
+static int
+cifs_readv_discard(struct TCP_Server_Info *server, struct mid_q_entry *mid)
+{
+	int length;
+	struct cifs_readdata *rdata = mid->callback_data;
+
+	length = discard_remaining_data(server);
+	dequeue_mid(mid, rdata->result);
+	return length;
+}
+
 int
 cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 {
@@ -1446,6 +1455,12 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 		return length;
 	server->total_read += length;
 
+	if (server->ops->is_status_pending &&
+	    server->ops->is_status_pending(buf, server, 0)) {
+		discard_remaining_data(server);
+		return -1;
+	}
+
 	/* Was the SMB read successful? */
 	rdata->result = server->ops->map_error(buf, false);
 	if (rdata->result != 0) {
-- 
2.1.4

Re: Interop Issue: SMB2+ async replies, and the kernel, Samba side fix enclosed.

[Shirish Pargaonkar]

Pavel, briefly tested this, it looks good. No error (messages) logged.

Tested-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Sat, Feb 27, 2016 at 3:31 AM, Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 2016-02-27 12:11 GMT+03:00 Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>:
>> 2016-02-23 15:55 GMT+03:00 Ira Cooper <ira-mA6sH8tAw6XR7s880joybQ@public.gmane.org>:
>>>
>>> If the server sends an interim response, then the real response, the real
>>> response, is handled by standard_receive3() in the kernel, instead of the
>>> proper function, and this causes a disconnect.  If there isn't a
>>> disconnect, I believe the reply will just be discarded if I understand the
>>> code correctly.  (That is a big if here ;) )
>>>
>>> I've written a patch for Samba to stop sending interim replies on
>>> SMB2_READ
>>> and SMB2_WRITE, when non-compounded to stop the impact of this issue.  We
>>> may want to add SMB2_CREATE to the list of ops we don't send async replies
>>> for non-compounded, but I'm not sold either way, I know we CAN go async
>>> there!  I want some opinions here.
>>>
>>> This is not hidden behind a flag because compatibility issues that don't
>>> impact protocol correctness usually aren't.
>>>
>>> Setting req->async_te = talloc_new(NULL); is just ugly, though it works.
>>> If you have a cleaner approach, I welcome it.
>>>
>>> I request you please ASK me before pushing this one, yes, that means you
>>> jra!
>>>
>>> For those interested in reproduction: I'd suggest using a server that's
>>> rebuilt with a lower timeout set in smb2_read.c, though we've hit it with
>>> vfs_glusterfs straight up, in our testing.
>>>
>>> Thanks,
>>>
>>> -Ira / ira@(samba.org|redhat.com|wakeful.net)
>>
>>
>>
>> Thank you for catching this!
>>
>> It is the issue in the kernel client - a check for interim responses is
>> missed for SMB2_READ command. I created a patch that should fix the problem.
>>
>> Could you please test it?
>>
>> --
>> Best regards,
>> Pavel Shilovsky
>
> CC'ing @samba-technical.
>
> Sorry for the spam.
>
> --
> Best regards,
> Pavel Shilovsky

Re: Interop Issue: SMB2+ async replies, and the kernel, Samba side fix enclosed.

[Pavel Shilovsky]

2016-02-27 16:50 GMT+03:00 Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> Pavel, briefly tested this, it looks good. No error (messages) logged.
>
> Tested-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Thanks!

-- 
Best regards,
Pavel Shilovsky

Re: Interop Issue: SMB2+ async replies, and the kernel, Samba side fix enclosed.

[Steve French]

merged into cifs-2.6.git

Added Shirish's tested-by and cc: stable

Let me know if any objections to going to stable etc

On Sun, Feb 28, 2016 at 12:22 AM, Pavel Shilovsky <piastryyy@gmail.com>
wrote:

> 2016-02-27 16:50 GMT+03:00 Shirish Pargaonkar <shirishpargaonkar@gmail.com
> >:
> > Pavel, briefly tested this, it looks good. No error (messages) logged.
> >
> > Tested-by: Shirish Pargaonkar <shirishpargaonkar@gmail.com>
>
> Thanks!
>
> --
> Best regards,
> Pavel Shilovsky
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Thanks,

Steve

Re: Interop Issue: SMB2+ async replies, and the kernel, Samba side fix enclosed.

[Steve French]

merged into cifs-2.6.git

Added Shirish's tested-by and cc: stable

Let me know if any objections to going to stable etc

On Sun, Feb 28, 2016 at 12:22 AM, Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 2016-02-27 16:50 GMT+03:00 Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>> Pavel, briefly tested this, it looks good. No error (messages) logged.
>>
>> Tested-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Thanks!
>
> --
> Best regards,
> Pavel Shilovsky
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Thanks,

Steve