TalAloni / SMBLibrary

Free, Open Source, User-Mode SMB 1.0/CIFS, SMB 2.0, SMB 2.1 and SMB 3.0 server and client library
GNU Lesser General Public License v3.0
712 stars 182 forks source link

Is there a proper way to handle Not Enough Credits as a client? #42

Closed Jo0 closed 3 years ago

Jo0 commented 4 years ago

Hello,

I'm currently running into a consistent Not Enough Credits when using the SMB2Client against a share configured with Samba on a CentOS host.

When trying to copy ~260 96MB files from the samba share to a local directory all at once in parallel, I'm seeing the Not Enough Credits exception fairly regularly. (Sometimes `STATUS_INVALID_SMB as well, but I have a feeling it's because of the amount/rate of which I'm requesting from the share)

I've tried changing the operation to copy in 5-10 file batches, and it's reduced occurrences of the exception, but I still receive the exception.

I think it's good to note that each operation of a file is it's own instance of SMB2Client

I've tried bumping up smb2 max credits in my smb.conf to 15000 and it's significantly reduced the number of occurrences of the exception when operating in batches of 5.

Maybe it's still my lack of understanding of how credits work, but from reading MS-SMB specs and researching on SMB credits, I have a feeling I'm going about it the wrong way.

I'd like to be able to do a fire and forget, and copy all files in parallel or bump up my batch size to something larger, but I'd run the risk of running out of credits.

Is there a suggested/preferred way on how to go about utilizing/consuming SMB2Client in such a way that guards itself from running into Not Enough Credits?

TalAloni commented 4 years ago

Hi Vinh,

  1. In v1.4.1 I have added support for mutl-credit operations / LargeMTU - it's possible that I've missed something while doing so - Can you try v1.4.0?
  2. In v1.4.1+ the assumption is that the server will always grant the requested number of credits, if not, you get the "Not enough credits" exception. perhaps this could be improve by teaching the client to work with less credits when necessary. (e.g. send only 64KB when the server is not willing to accept more)

Tal

Jo0 commented 4 years ago

Tal,

So when trying with v1.4.0, I seem to no longer get "Not Enough Credits", but I consistently get STATUS_INVALID_SMB instead. STATUS_INVALID_SMB was the main motivation for me to move to 1.4.1+.

"Not Enough Credits" (maybe even STATUS_INVALID_SMB in 1.4.0) seems tied to the throughput between the client and server.

If I'm remote to the server (WAN) my upload/download speeds are ~3-5MB/s, and I receive "Not Enough Credits" and/or STATUS_INVALID_SMB much slower and later in the process of those copying those ~260 files.

If I'm local to the server (LAN) my upload/download speeds are ~35-112MB/s, and I receive "Not Enough Credits" and/or STATUS_INVALID_SMB almost instantly.

Jo0 commented 4 years ago

I'll be trying to provide a packet capture, but I'll try to add as much detail as I can while I'm debugging.

my FileStore.ReadFile calls are as follows, 65536 was originally set by me, and even when I bump the maxCount to 1048576, it doesn't seem to make a difference

filestore.ReadFile(out data, SMBLibrary.SMB2.FileID, <start position>, 65536);
Jo0 commented 4 years ago

I think I just found it, I noticed that we were getting these errors when m_availableCredits was off by one of request.Header.CreditCharge

I added the check below of the only two instances where I saw we were off by 1. It's obviously a bainaid, but I think it's a lot better insight as to what's going on.

if (request.Header.CreditCharge == 0)
{
    request.Header.CreditCharge = 1;
}

if((m_availableCredits == 0 && request.Header.CreditCharge == 1) || (m_availableCredits == 15 && request.Header.CreditCharge == 16))
{
    m_availableCredits += 1;
}

if (m_availableCredits < request.Header.CreditCharge)
{
    throw new Exception("Not enough credits");
}

m_availableCredits -= request.Header.CreditCharge;

if (m_availableCredits < DesiredCredits)
{
    request.Header.Credits += (ushort)(DesiredCredits - m_availableCredits);
}

At first I thought it'd be better to just increase m_availableCredits while we set request.Header.CreditCharge = 1 when request.Header.CreditCharge == 0, but that was causing issues to the point where operations would no longer execute, or fail silently.

TalAloni commented 4 years ago

Thanks - Can you test and see if it happens with Windows Server as well?

Jo0 commented 4 years ago

"Not Enough Credits" still occurs when operating against a Windows 10 UNC Share

TalAloni commented 4 years ago

Thanks - I'll try to recreate when I have a bit of time - hopefully during this weekend. I'm also curious about that "STATUS_INVALID_SMB" error you have with v1.4.0 - it sounds like a separate issue. if you could provide a WireShark capture for that - I would be interested to into it.

Jo0 commented 4 years ago

I can try to get you a WireShark capture soon, will post it here once I complete it

Jo0 commented 4 years ago

Here's a packet capture where I was experiencing STATUS_INVALID_SMB and Not Enough Credits Packet capture

This was with me being remote to the server (WAN) (upload/download speeds are ~3-5MB/s) and calls to fileStore.ReadFile were

filestore.ReadFile(out data, SMBLibrary.SMB2.FileID, <start position>, 1048576);
TalAloni commented 4 years ago

Is it possible you sent the wrong packet capture or captured the wrong port? the one you sent does not include SMB traffic.

Also, from your description it seems there are two separate issues, and that the first one (STATUS_INVALID_SMB) exists in v1.4.0, so having a packet capture with just this issue will be helpful.

Jo0 commented 4 years ago

Apologies, working on getting two packet captures for 1.4.2 and 1.4.0 right now

Jo0 commented 4 years ago

Getting more leniency with my local machines and am not seeing the issues as described. I'll try to get local to the problem machines as soon as I can and get packet captures for you.

Jo0 commented 4 years ago

I just emailed you the links to the packet captures

TalAloni commented 4 years ago

Vinh, Thanks for the packet capture, The 'STATUS_INVALID_SMB-v1.4.0-maxCount-65536' packet capture suggest that you issue multiple ReadFile requests for a single client at-a-time (instead of one-at-a-time for each client), While from the protocol point-of-view there is nothing wrong with that, the client implementation was never tested for that scenario. The client manages its credits (I have set DesiredCredits to 16) based on the assumption that only a single read is issued at a time.

Jo0 commented 4 years ago

Ah I didn't realize the client was supposed to be utilized in that way. (One read per client?) I'm essentially using one instance of a client per operation. In this case, one client, to read the entirety of a file, per file in a directory

Jo0 commented 4 years ago

I hope you're okay with me asking more questions on this open issue. I don't want to open another one, since we have a dialog here on this one. If you'd like me to, I can do so.

When calling _fileStore.ReadFile on a file handle. I get STATUS_PENDING. Even after I have setup my CreateFile to be synchronous.

var createOptions = CreateOptions.FILE_SYNCHRONOUS_IO_NONALERT | CreateOptions.FILE_NON_DIRECTORY_FILE;

var accessMask = AccessMask.SYNCHRONIZE | AccessMask.GENERIC_READ;
var shareAccess = ShareAccess.Read;

var disposition = CreateDisposition.FILE_OPEN;

object handle;
var status = fileStore.CreateFile(out handle, out FileStatus fileStatus, <relativePath>, accessMask, 0, shareAccess, disposition, createOptions, null);

status = fileStore.GetFileInformation(out FileInformation fileInfo, handle, FileInformationClass.FileStandardInformation);

var fileStandardInfo = (FileStandardInformation)fileInfo;

var position = 0;
byte[] buffer = new byte[1048576];
var offset = 1048576;
var end = false;
while(!end)
{
    var stopwatch = new Stopwatch();

    stopwatch.Start();
    do
    {
        status = fileStore.ReadFile(out byte[] data, handle, position, 4096);

        switch (status)
        {
            case NTStatus.STATUS_SUCCESS:
                for (int i = offset, i2 = 0; i2 < data.Length; i++, i2++)
                {
                    buffer[i] = data[i2];
                }
                Console.WriteLine($"Success - {position} + {data.Length}    {string.Concat(data.Select(b => b.ToString("X2")).ToArray())}");
                position += data.Length;
            case NTStatus.STATUS_END_OF_FILE:
                Console.WriteLine($"End - {position} {fileStandardInfo.EndOfFile}");
                end = true; 
                break;
            case NTStatus.STATUS_PENDING:
                Console.WriteLine($"Pending - {position}");
                break;
            default:
                throw new Exception($"Unable to read file; Status: {status}");
        }
    }
    while (status == NTStatus.STATUS_PENDING && stopwatch.Elapsed.TotalSeconds <= 45);
}

When I go through the retry logic for STATUS_PENDING, I see that my _position exceeds the length returned in fileInfo.EndOfFile NTStatus.EndOfFile. I find it odd, and I'm having a hard time tracking down how to properly setup my CreateFile to not get STATUS_PENDING

TalAloni commented 4 years ago

This is not a good place for a separate issue. See this example. You can also use WireShark to see which parameters are used by Windows.

GambaJo commented 3 years ago

@TalAloni @Jo0 I have the same problem (I'm using SMBLibraryLite 1.4.3.2 https://github.com/jordanlytle/SMBLibrary). Ist this problem solved? What can I do to avoid this error?

TalAloni commented 3 years ago

Not Enough Credits usually means your client is doing something unexpected, make sure you don't send more data than negotiated (MaxReadSize / MaxWriteSize) and don't issue concurrent requests within the context of the same client.

GambaJo commented 3 years ago

It's strange. This problem occurs not everytime. The files are 1-2 MB. It's a windows share. I tried it with only 1 file at a time, so there are no concurrent requests.

TalAloni commented 3 years ago

Do you have a code snipped that demonstrate this? does it happen consistently with various Windows versions?

GambaJo commented 3 years ago

Sorry for my late answer. I made an example: `using SMBLibrary; using SMBLibrary.Client; using System; using System.Collections.Generic; using System.IO;

namespace TestSMBLibrary { class Program { static void Main(string[] args) { UploadFile(); }

    static void UploadFile()
    {
        SMB2Client client = new SMB2Client();
        bool success = client.Connect(System.Net.IPAddress.Parse("172.24.112.20"), SMBTransportType.DirectTCPTransport);
        if (success)
        {
            NTStatus status = client.Login(String.Empty, "testtest", "#Test1234");
            if (status == NTStatus.STATUS_SUCCESS)
            {
                SMB2FileStore fileStore = client.TreeConnect("UncPathTest", out status) as SMB2FileStore;
                if (fileStore != null)
                {
                    object fileHandle = null;
                    status = fileStore.CreateFile(out fileHandle, out FileStatus fileStatus, "test.pdf", AccessMask.SYNCHRONIZE | AccessMask.GENERIC_WRITE, SMBLibrary.FileAttributes.Normal, ShareAccess.None, CreateDisposition.FILE_CREATE, CreateOptions.FILE_SYNCHRONOUS_IO_ALERT | CreateOptions.FILE_NON_DIRECTORY_FILE, null);
                    if (status == NTStatus.STATUS_SUCCESS)
                    {
                        using MemoryStream memoryStream = new MemoryStream();
                        File.OpenRead(@"C:\temp\test.pdf").CopyTo(memoryStream);
                        status = fileStore.WriteFile(out int numberOfBytesWritten, fileHandle, 0, memoryStream.ToArray());
                    }
                    fileStore.Disconnect();
                }
                client.Logoff();
            }
            client.Disconnect();
        }
    }
}

} `

I only can try this with Windows Server 2016. The file was created, but with 0 kB. Then the fileStore.WriteFile() command throws "Not enough credits". But only when the file is > 1 MB. No problems with smaller files so far.

GambaJo commented 3 years ago

@TalAloni Ok, I edited the example because of format. Now it's better. I also did some research: https://docs.microsoft.com/en-us/windows-server/storage/file-server/troubleshoot/troubleshooting-smb But this log only shows: grafik

Jo0 commented 3 years ago

Sorry for the late reply. I did manage to fix this somewhat in SMBLibraryLite. It was mentioned in a reply above here's it is in SMBLibraryLite

It fixed Not Enough Credits when acting on a Linux SMB share, but I would get it occasionally when interacting with a windows share.

It's also helpful to note that I do issue concurrent requests within the context of the same client. Which is against Tal's recommendation on how to use the library/client.

Also, my "fix" isn't really a fix and feels more like a hack/work around.

GambaJo commented 3 years ago

@Jo0 That's strange. I tried my code above with SMBLibraryLite 1.4.3.2 too, but it also throws the same exception everytime I try it. grafik grafik grafik

GambaJo commented 3 years ago

@TalAloni @Jo0 Hey guys, I cloned the solution and used it in my test app. And I debugged it a little bit. So the CreditCharge of my test.pdf is 56 (3664214 / 65536): grafik

So I made a debug output of m_availableCredits and command.Header.Credits: grafik

This is the output: m_availableCredits 0 += command.Header.Credits 1 Connected m_availableCredits 0 += command.Header.Credits 1 m_availableCredits 0 += command.Header.Credits 16 Logged in m_availableCredits 15 += command.Header.Credits 1 Tree connected m_availableCredits 15 += command.Header.Credits 1

When it comes to the exception m_availableCredits is 16 but CreditCharge is still 56 (s.a.): grafik

I hope, this will help you to find the problem. If you have questions, please write me. I will help as much as I can.

EDIT: https://docs.microsoft.com/en-us/windows-server/administration/performance-tuning/role/file-server/smb-file-server grafik

Maybe this is also important: https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-smb2/46256e72-b361-4d73-ac7d-d47c04b32e4b

GambaJo commented 3 years ago

@TalAloni @Jo0 I tried to find the problem today. In my case m_availableCredits is 16 (see screenshot above). 16 x 65536 = 1.048.576. And 1.048.576 is the value of ClientMaxWriteSize. I don't know, if this is true, but for me it seems like the SMBLibrary (and SMBLibraryLite) is limited to files <= 1 MB? If a file is bigger then 1 MB it wont work.

Please guys, can you give me a feedback? I need to know, because if this doesn't work for files bigger then 1 MB, I have to search for another solution for my problem. In the worst case my whole project will die :( If you need other informations or help from me, I will try my best. Please help!

TalAloni commented 3 years ago

GambaJo, you are not honoring MaxWriteSize - that's why your code fail. You must not send more bytes than ClientMaxWriteSize in each request.

GambaJo commented 3 years ago

@TalAloni Sorry, I don't understeand you. Do you mean, that it's not possible to write files larger than the ClientMaxWriteSize? Or do you mean, that there is another way to write a file? I only know the way in my example above. Is it possible to chunk the file and write it in chunks?

TalAloni commented 3 years ago

You must split the file to chunks - each chunk must not exceed ClientMaxWriteSize. See this as a reference - but instead of reading chunks you need to write chunks

GambaJo commented 3 years ago

@TalAloni Ah, this was the missing part for me. I tried it, and it works. Thanks for your support and your work here. Feel free to use my example code for your code examples:

`using SMBLibrary; using SMBLibrary.Client; using System; using System.IO;

namespace TestSMBLibrary { class Program { static string FilePath = @"C:\temp\test.pdf";

    static void Main(string[] args)
    {
        UploadFile();
    }

    static void UploadFile()
    {
        SMB2Client client = new SMB2Client();
        bool success = client.Connect(System.Net.IPAddress.Parse("172.21.67.129"), SMBTransportType.DirectTCPTransport);
        if (success)
        {
            NTStatus status = client.Login(String.Empty, "testtest", "#Test1234");
            if (status == NTStatus.STATUS_SUCCESS)
            {
                SMB2FileStore fileStore = client.TreeConnect("UncPathTest", out status) as SMB2FileStore;
                if (fileStore != null)
                {
                    status = fileStore.CreateFile(out object fileHandle, out FileStatus fileStatus, Path.GetFileName(FilePath), AccessMask.SYNCHRONIZE | AccessMask.GENERIC_WRITE, SMBLibrary.FileAttributes.Normal, ShareAccess.None, CreateDisposition.FILE_CREATE, CreateOptions.FILE_SYNCHRONOUS_IO_ALERT | CreateOptions.FILE_NON_DIRECTORY_FILE, null);
                    if (status == NTStatus.STATUS_SUCCESS)
                    {
                        long bytesWrittenTotal = 0;
                        byte[] fileArray = File.ReadAllBytes(FilePath);
                        do
                        {
                            long chunkArrayLength = fileArray.Length - bytesWrittenTotal < client.MaxWriteSize ? fileArray.Length - bytesWrittenTotal : client.MaxWriteSize;
                            byte[] chunkArray = new byte[chunkArrayLength];
                            Array.Copy(fileArray, bytesWrittenTotal, chunkArray, 0, chunkArrayLength);
                            status = fileStore.WriteFile(out int bytesWrittenChunk, fileHandle, bytesWrittenTotal, chunkArray);
                            if(status != NTStatus.STATUS_SUCCESS)
                            {
                                throw new Exception("Failed to write file");
                            }
                            bytesWrittenTotal += bytesWrittenChunk;
                        } while (bytesWrittenTotal < fileArray.Length);

                        fileStore.CloseFile(fileHandle);
                    }
                    fileStore.Disconnect();
                }
                client.Logoff();
            }
            client.Disconnect();
        }
    }
}

}`

M-Beezy commented 3 years ago

I was facing the same problem. @GambaJo's answer solved it for me, too! Thanks a lot!

I think this would be very useful in the examples

Arnaldo221b commented 2 years ago

I'm having the same issue but for reading files, so I don't know if there is a way of splitting into chunks the amount of data to read.

TalAloni commented 2 years ago

@Arnaldo221b The client decide how much data to read in each request. see: https://github.com/TalAloni/SMBLibrary/blob/master/ClientExamples.md#read-large-file-to-its-end

MarouenBouazzi commented 1 year ago

Hi @TalAloni, I have exception when i try to download 1.7.1 version. this version was released?

TalAloni commented 1 year ago

Hi @TalAloni, I have exception when i try to download 1.7.1 version. this version was released? No such version