Bug 9702 - Current crediting algorithm causes problems due to a Win7 client bug.
Current crediting algorithm causes problems due to a Win7 client bug.
Status: RESOLVED FIXED
Product: Samba 3.6
Classification: Unclassified
Component: SMB2
unspecified
All All
: P5 major
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks: 10077
  Show dependency treegraph
 
Reported: 2013-03-07 19:22 UTC by Jeremy Allison
Modified: 2015-07-02 09:11 UTC (History)
5 users (show)

See Also:


Attachments
git-am fix for 3.6.x. (1.15 KB, patch)
2013-03-07 19:23 UTC, Jeremy Allison
ira: review-
Details
replicator for the issue. (20.95 KB, application/octet-stream)
2013-06-18 18:44 UTC, Ira Cooper
no flags Details
Test patch for master (1.91 KB, patch)
2015-01-28 15:00 UTC, Stefan Metzmacher
no flags Details
possible reproducer patches for smbclient -mSMB2_02 -c 'get largefile.dat /dev/null' (2.78 KB, patch)
2015-01-28 15:31 UTC, Stefan Metzmacher
no flags Details
Patches for master (3.61 KB, patch)
2015-01-29 11:36 UTC, Stefan Metzmacher
no flags Details
reproducer patches for smbclient -mSMB2_02 or -mSMB3_00 (2.46 KB, patch)
2015-01-29 11:38 UTC, Stefan Metzmacher
no flags Details
Patches for v4-2-test (3.88 KB, patch)
2015-01-29 15:37 UTC, Stefan Metzmacher
jra: review+
Details
Patches for v4-1-test (3.89 KB, patch)
2015-01-29 15:37 UTC, Stefan Metzmacher
no flags Details
Patches for v4-0-test (3.89 KB, patch)
2015-01-29 15:37 UTC, Stefan Metzmacher
no flags Details
Patches for v4-1-test (3.88 KB, patch)
2015-01-29 16:51 UTC, Stefan Metzmacher
jra: review+
Details
Patches for v4-0-test (3.88 KB, patch)
2015-01-29 16:51 UTC, Stefan Metzmacher
jra: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Allison 2013-03-07 19:22:56 UTC
From Ira.

The client will run itself out of credits, and make mistakes
issuing compound requests.  This code stops the client from
starving itself.  Microsoft likely does something similar,
they document a "minimum credits" option, defaulted to 64.
------------------------------------------------------

Ira. Can you upload a packet capture showing this issue ? That would help. Personally I don't see a problem with this change, but having evidence would really help.

Thanks !

Jeremy.
Comment 1 Jeremy Allison 2013-03-07 19:23:22 UTC
Created attachment 8617 [details]
git-am fix for 3.6.x.
Comment 2 Stefan Metzmacher 2013-03-20 08:10:30 UTC
Comment on attachment 8617 [details]
git-am fix for 3.6.x.

I need more information on this at least a capture file.

From the documentation:
<149> Section 3.3.1.2: Windows Vista SP1, Windows Server 2008, Windows 7, Windows
Server 2008 R2, Windows 8, and Windows Server 2012 SMB2 servers support a configurable minimum credit limit below which the client is unconditionally granted all credits it requests, and a configurable maximum credit limit above which credits are never granted, as follows:

Windows Vista SP1, Windows 7, and Windows 8
=> min default 128 max default 2048
Windows Server 2008,Windows Server 2008 R2, and Windows Server 2012 
=> min default 512 max default 8192

But that applies only for credits the client asked for.

We really need to understand the problem in order to create a useful
fix.
Comment 3 Ira Cooper 2013-03-20 11:28:59 UTC
That patch has a typo.  It isn't something I'd commit to Samba.  (It is something I'd use in local testing.)

I'll see what I can do about getting a reproduction to everyone.  And also a patch against master/4.0 and 3.6.
Comment 4 Jeremy Allison 2013-03-20 16:01:18 UTC
Yep, thanks Metze. I was about to ping Ira asking for the wireshark trace (but I know he's busy :-). The patch was a placeholder so we don't lose the original fix.

Jeremy.
Comment 5 Ira Cooper 2013-06-18 18:44:06 UTC
Created attachment 8984 [details]
replicator for the issue.

This is a replication program for the bug this issue is about.

tFindPerl.exe d:\directory\here 32 0 768

This will cause MS Win7 clients to run out of credits and into issues without a lower bound.

Thanks,

-Ira
Comment 6 Karolin Seeger 2013-12-10 15:49:18 UTC
Any news on this one?
Comment 7 Karolin Seeger 2014-11-27 10:53:04 UTC
Is this a showstopper for 4.2.0?
Comment 8 Stefan Metzmacher 2014-11-29 10:18:21 UTC
No 4.2.0 blocker, remember for 4.3
Comment 9 Stefan Metzmacher 2015-01-28 15:00:54 UTC
Created attachment 10677 [details]
Test patch for master
Comment 10 Stefan Metzmacher 2015-01-28 15:05:32 UTC
I found a likely to be important difference in our credit algorithm.

Windows seems to always grant the credits consumed for the current request,
even if the credits requests field is 0. So the effective credits requests
is always at least 1.
Comment 11 Stefan Metzmacher 2015-01-28 15:31:01 UTC
Created attachment 10680 [details]
possible reproducer patches for smbclient -mSMB2_02 -c 'get largefile.dat /dev/null'

Someone could try smbclient (with these hacks) to reproduce the problem...
Comment 12 Ralph Böhme 2015-01-28 15:45:14 UTC
(In reply to Stefan (metze) Metzmacher from comment #10)
Besides that, were we possibly violating 3.3.1.2

"The server MUST ensure that the number of credits held by the client is never reduced to zero. If the condition occurs, there is no way for the client to send subsequent requests for more credits."

<https://msdn.microsoft.com/en-us/library/cc246704.aspx>
Comment 13 Jeremy Allison 2015-01-28 22:36:56 UTC
(In reply to Stefan (metze) Metzmacher from comment #11)

I've run with these hacks (both #1 and #2 separately) against Win2k12, and I do see 1 credit being returned for the zero-credit asks in wireshark.

Running these hacks against unpatched master returns 0 for the zero-credit asks, and matches Win2k12 in returning 1 credit for the zero-credit asks when the 'Test patch for master' is applied.

I guess what I'm asking is can I push the master patch ? :-).

Jeremy.
Comment 14 Stefan Metzmacher 2015-01-28 22:48:25 UTC
(In reply to Jeremy Allison from comment #13)

Are you able to get smbclient to hang?

Does smbclient reach the point where it always ask for
0 credits when comsuming the last credit?

Can you upload the smbclient output of the 3 runs (against windows, unpatches samba and patches samba)?
(compressed with bz2)

Then I can have a look at the output and see if it's as I expect.
I'll add a useful commit message then and push to master
with out review on it.

Thanks!
Comment 15 Stefan Metzmacher 2015-01-28 22:52:08 UTC
(In reply to Ralph Böhme from comment #12)

-       } else if (xconn->smb2.credits.granted == 0) {
-               /*
-                * Make sure the client has always at least one credit
-                */
-               credits_granted = 1;

Made sure that the client has at least 1 credit, but it seems
that Windows needs more than 1 credit, [MS-SMB2] says most
Windows versions require at least 4 credits. I guess in order to
send oplock break acks. Or do compound directory listings.
Comment 16 Jeremy Allison 2015-01-29 02:36:37 UTC
Metze wrote: 

> Are you able to get smbclient to hang?

No, I never got smbclient to hang. But analysing the wireshark traces clearly show Win2k12 returning 1 credit with a request of zero, whereas currently smbd returns zero in the same situation.

With your master patch we behave identically to Win2k12 in the same situations.
Comment 17 Stefan Metzmacher 2015-01-29 11:36:52 UTC
Created attachment 10691 [details]
Patches for master
Comment 18 Stefan Metzmacher 2015-01-29 11:38:12 UTC
Created attachment 10692 [details]
reproducer patches for smbclient -mSMB2_02 or -mSMB3_00
Comment 19 Stefan Metzmacher 2015-01-29 15:37:00 UTC
Created attachment 10693 [details]
Patches for v4-2-test
Comment 20 Stefan Metzmacher 2015-01-29 15:37:26 UTC
Created attachment 10694 [details]
Patches for v4-1-test
Comment 21 Stefan Metzmacher 2015-01-29 15:37:57 UTC
Created attachment 10695 [details]
Patches for v4-0-test
Comment 22 Stefan Metzmacher 2015-01-29 16:51:03 UTC
Created attachment 10697 [details]
Patches for v4-1-test
Comment 23 Stefan Metzmacher 2015-01-29 16:51:36 UTC
Created attachment 10698 [details]
Patches for v4-0-test
Comment 24 Jeremy Allison 2015-01-29 19:32:24 UTC
Comment on attachment 10693 [details]
Patches for v4-2-test

LGTM.
Comment 25 Jeremy Allison 2015-01-29 20:15:36 UTC
Re-assigning to Karolin for inclusion in 4.2.0, 4.1.next, 4.0.next.

Jeremy.
Comment 26 Stefan Metzmacher 2015-02-01 08:51:45 UTC
Pushed to autobuild-v4-2-test
Comment 27 Stefan Metzmacher 2015-02-01 09:29:18 UTC
Pushed to autobuild-v4-[0|1]-test.
Comment 28 Stefan Metzmacher 2015-02-02 23:22:48 UTC
Pushed to v4-2-test and v4-1-test.
autobuild-v4-0-test is still running...
Comment 29 Karolin Seeger 2015-02-05 20:41:44 UTC
(In reply to Stefan (metze) Metzmacher from comment #28)
Pushed to autobuild-v4-0-test again.
Comment 30 Karolin Seeger 2015-02-09 19:51:50 UTC
Pushed to v4-0-test.
Closing out bug report.

Thanks!
Comment 31 bulin.peter 2015-03-10 11:55:37 UTC
I have a question to this bug... I just installed samba 4.2.0 and I wanted to copy a lot of files from server to client (windows 7 64bit) and after few seconds my client freezes, it does nothing for about one minute and after that copy error is reported and everything is working again. Last messages I see in log on server are for example:

[2015/03/10 11:26:40.560841, 10, pid=18123, effective(32697, 10513), real(32697, 0)] ../source3/smbd/smb2_server.c:898(smb2_set_operation_credit)
  smb2_set_operation_credit: requested 2, charge 1, granted 1, current possible/max 1/512, total granted/max/low/range 512/8192/36313/512

or:

[2015/03/10 12:20:09.291088, 10, pid=18563, effective(32697, 10513), real(32697, 0)] ../source3/smbd/smb2_server.c:898(smb2_set_operation_credit)
smb2_set_operation_credit: requested 1, charge 1, granted 1, current possible/max 2/512, total granted/max/low/range 511/8192/93417/511

is this problem related to this bug report and made changes?
Comment 32 Stefan Metzmacher 2015-07-02 09:11:02 UTC
(In reply to bulin.peter from comment #31)

Can you create a new bug report and
upload a log file with at least the last 600 requests?