SMB2/3 Credits (Multi-Credit) support

[Ira Cooper]

On Wed, Jun 27, 2012 at 5:11 PM, Jeremy Allison <jra at samba.org> wrote:
> On Wed, Jun 27, 2012 at 08:58:21PM +0200, Stefan (metze) Metzmacher wrote:
>> Hi Jeremy,
>>
>> while reviewing the multi-credit code that already went to master
>> a few weeks ago, I noticed some missing details in the multi-credit code.
>>
>> While debugging this I found some bugs in our credit handling code.
>>
>> I spend a lot of time testing against Windows 2012 RC.
>> I think the code in this branch, let us behave almost like a windows server.
>> https://gitweb.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master3-credits
>>
>> The most important thing is that windows 2012 RC allows up to 8192 credits
>> (also our default), but it normally it only grants up to (8192/16) = 512
>> credits.
>
> Interesting - do you know if this is documented anywhere in the MS docs ?
> The credit algorithm they use is not documented as far as I know.
>
>> If the client uses very large credit-charge values (e.g. using all 512
>> (or 256) credits just for one request)
>> I'm able to get up to 8192 credits after a while.
>>
>> Note that it's only useful for a client to use up to 15 credits per
>> request (for 1MB reads/writes).
>>
>> With my patches we only grant up to (8192/16) = 512 credits,
>> adding more dynamic crediting between 512 and 8192 later.
>
> That sounds reasonable.
>
>> I'll do some more testing tomorrow, then I plan to push this to master
>> (it already passed a private autobuild once).
>
> I noticed you removed DEFAULT_SMB2_MAX_CREDIT_BITMAP_FACTOR,
> which is fine, but I need to tell you why that was added.
>
> What I observed back when we were advertising smaller credit
> windows was that Windows clients would request sequence numbers
> *outside* of the valid credit range.
>
> For example, if we have a valid sequence number space of 100-200,
> then Windows clients would use requests with sequence numbers between
> 100 - 300. Note that they'd never use more than the 100 outstanding
> credits they had available, but they'd commonly miss a low
> sequence number and then use numbers above the valid range,
> finally filling in the missing low sequence number just before
> running out of credits.
>
> So DEFAULT_SMB2_MAX_CREDIT_BITMAP_FACTOR was a heuristic added
> to allow us to cope with that. Christian worked with me on
> this and can probably also remember some details.
>
> It sounds like you may have figured out the Windows algorithm,
> and by restricting to 16th you may not hit the problem
> DEFAULT_SMB2_MAX_CREDIT_BITMAP_FACTOR was added to solve.
>
> I'd keep a close eye on this however, as normal testing
> didn't reveal the seuqnce number problem, it took released
> code in the wilds of IBM internal to have the problem be
> seen.
>
> Ira, do you have any comments - have you seen any problems
> with our current released code w.r.t. credits ?

Nope.  The 3.6 code seems fine.

As far as the issue is concerned: We hit it also.  And consistently
enough that I had to remove the check w/o the FACTOR in it.  I
remember complaining to you about this issue :).

I would NOT have the server disconnect on client violation of this
part of the protocol.  We know it is violated alas.  And proving "it
works" is not trivial, even if we implemented MS' exact algorithm.  If
MS' server doesn't disconnect clients due to this issue, then I'd
expect clients to violate it.  This may be a moment for "be
conservative in what you do, be liberal in what you accept from
others."  Unless our implementation, needs this restriction for some
reason.

Thanks,

-Ira