Currently the parameter "min receivefile size" has no effect on the SMB2 server, only the SMB1 server. Patch for 3.6.x (and 4.0.x and master) to follow to fix this. Jeremy.
Created attachment 8216 [details] test git-am fix for 3.6.x Compiles, but not yet tested. Not asking for review yet. Jeremy.
W00t. Tested patch and it works. I'll work on getting this into 3.6.next, 4.0.0 and master. Jeremy.
Comment on attachment 8216 [details] test git-am fix for 3.6.x Asking Metze to take a look at this. Metze, I'll create a forward port for master and 4.0.0 and submit for review also. Cheers, Jeremy.
Comment on attachment 8216 [details] test git-am fix for 3.6.x Michael should also have a look :-).
Hello Jeremy, I'm a crazy guy ;) who is very interested in getting this patch working on samba 4 master trunk so that I can start working on a write optimization for a Samba server based on openWRT. This why I'm putting some effort right now on it. However, I noted that samba 4 SMB2 server code is sligthly different than samba 3 one. So let me ask you whether you did get the chance to create such as port as mentioned on comment #3? And, if so, whether you can share a link to the repository? Thanks, Jose M. Prieto (In reply to comment #3) > Comment on attachment 8216 [details] > test git-am fix for 3.6.x > > Asking Metze to take a look at this. > > Metze, I'll create a forward port for master and 4.0.0 and submit for review > also. > > Cheers, > > Jeremy.
I haven't moved this to master yet as I'm waiting for confirmation from an OEM that this actually does help and improve performance in real world conditions. If you work for such an OEM and are willing to do such testing please email me directly at jra@samba.org and let's discuss it. Jeremy.
I am such an OEM. :-) I can verify that we consistently get ~18% write speed improvement on a slow-ish platform (1.2GHz ARMv7) with this patch. Our kernel does have a receivefile (splice from socket to file) implementation. I'm happy to test again once a 4.0.x patch is ready; but the results on 3.6 are very good. I'd love to see this change get pulled in.
Ok I'll forward port to master/4.0.x. Not until Monday though :-). Jeremy.
Ouch. The vector reading code has changed significantly between 3.6.x and 4.0.x (for the better, so we aren't doing as many separate read requests and also to support the SMB3 transport level encryption) to make it much harder to do the RECVFILE port to 4.0.x. I'm still looking at what I'd need to change to do this. Jeremy.
Am 19.03.2013 23:54, schrieb samba-bugs@samba.org: > https://bugzilla.samba.org/show_bug.cgi?id=9412 > > --- Comment #9 from Jeremy Allison <jra@samba.org> 2013-03-19 22:54:03 UTC --- > Ouch. The vector reading code has changed significantly between 3.6.x and 4.0.x > (for the better, so we aren't doing as many separate read requests and also to > support the SMB3 transport level encryption) to make it much harder to do the > RECVFILE port to 4.0.x. I'm still looking at what I'd need to change to do > this. That's why I were so sceptical about the 3.6 patch, we can't have that in 3.6 but not in newer releases... If you can come with a similar small patch for master and 4.0 we might be able to bring it upstream, but it might cause some more work later when we try to implement SMB-Direct (RDMA). metze
I'm assuming in the previous comment "we can't have that in 3.6 but not in newer releases" you mean "we *can* have that in 3.6 but not in newer releases", yes ? I agree. I'm going to look at getting a minimal patch working in master/4.0.x. I don't think it'll cause trouble for RDMA, as it should be orthogonal to that logic. Jeremy.
(In reply to comment #11) > I'm assuming in the previous comment "we can't have that in 3.6 but not in > newer releases" > > you mean "we *can* have that in 3.6 but not in newer releases", yes ? > > I agree. I'm going to look at getting a minimal patch working in master/4.0.x. > I don't think it'll cause trouble for RDMA, as it should be orthogonal to that > logic. I mean if we add that at all, we need it in master and maybe 4.0. If that's done, we can start to think about 3.6. metze
I have hard numbers from Justin that shows he needs this for their low-end platforms, so I'm planning to code this up in the next week or so. Jeremy.
Finally figured out how to do this (relatively, ha ha) cleanly in 4.0.x, master. Patch should be ready for test early next week. Jeremy.
Hi, I got RECVFILE working on samba 4 and SMB2 based on your patch for 3.6.x. As you said the new way the next vector function is working as of 4.x was the most challenging for me. If you want I can share the patch with you. Regards, Jose M. Prieto (In reply to comment #14) > Finally figured out how to do this (relatively, ha ha) cleanly in 4.0.x, > master. > > Patch should be ready for test early next week. > > Jeremy.
Sure I'll take a look - thanks ! I already have that part working though :-). It'll be good to compare the two to see if there's anything I might have missed. Cheers, Jeremy.
Created attachment 8702 [details] samba4-smb2-recvfile-v4.patch
Hi, You can find the patch attached to the this thread. My work is a fork from branch v4-0-stable. Regards, Jose (In reply to comment #16) > Sure I'll take a look - thanks ! I already have that part working though :-). > It'll be good to compare the two to see if there's anything I might have > missed. > > Cheers, > > Jeremy.
Created attachment 8706 [details] Raw patch for 4.0.next/master The changes to smbd_smb2_request_next_vector() in master actually make it easier to detect a recvfile write, without having to check the tid/fid explicitly (the smbd_smb2_inbuf_parse_compound() takes care of errors in that nicely). Justin, can you test with this raw patch, and if it works for you I'll split up into smaller commits we can try and get into 4.0.next. Cheers, Jeremy.
Found the bug.... doing a check on IVAL(state->pktbuf, SMB2_HDR_OPCODE), this should be an SVAL instead. New patch in a min or so... Jeremy.
Created attachment 8707 [details] Fixed raw-patch for 4.0.x / master Fixed the opcode read IVAL -> SVAL. Jeremy.
Created attachment 8716 [details] git-am fix for master and 4.0.x Patch for 4.0.x and master currently undergoing test.
This latest patch is working well for me so far. I'm seeing that nice 18% write speed improvement on my ARM machine, which finally makes SMB2 actually faster than SMB1. :-)
Created attachment 8728 [details] git-am fix for master and 4.0.x Updated with Metze's change requests. Jeremy.
Created attachment 8737 [details] git-am fix for master and 4.0.next Updated with Metze's comments. 3 new patches added at the end to ensure the socket is in blocking state inside drain_socket() and before any RECVFILE call. (It's possible these last 2 might be moved into the default recvfile() code, if the kernel recvfile patch already correctly copes with the non-blocking socket reads). Justin if you could test this I'd really appreciate it. Cheers, Jeremy.
Update via email from Justin: Fortunately, the updated patch set doesn't appear to tangibly affect write performance in my testing today. So everything looks good from my end.
Jeremy, can you upload new patches with cherry-pick information from master? Then I can give my review+ for the releases. Thanks!
Created attachment 8803 [details] git-am patchset that went into master. Here you go Metze, thanks ! Jeremy.
(In reply to comment #28) > Created attachment 8803 [details] > git-am patchset that went into master. > > Here you go Metze, thanks ! I don't see (cherry picked from commit .... lines in the attachment. in v4-0-test git cherry-pick -x -13 95f7fc83b251efefcc2a603b936b55e2f0308a72 git format-patch --stdout -13 > tmp40.diff should do it, I'll upload that and ask you for the review flag.
Oh sorry, I just did the 'git format-patch --stdout <start ref>..<end ref>" from master. Thought that's what you required.
Created attachment 8804 [details] Patches for v4-0-test
(In reply to comment #30) > Oh sorry, I just did the 'git format-patch --stdout <start ref>..<end ref>" > from master. Thought that's what you required. No, problem it would be cool to have a git format-patch -x --stdout but on the other hand it's not the worst thing, if it's required to really apply the patches to the target branch first, then it's less likely to generate conflicts for Karolin...
Comment on attachment 8216 [details] test git-am fix for 3.6.x I guess this patch needs to be updated, correct?
Yes the 3.6.x patch needs to be re-done. Let's ignore it for now and just leave SMB2 recvfile as 4.0.x. Jeremy.
Comment on attachment 8804 [details] Patches for v4-0-test LGTM.
Re-assigning to Karolin for inclusion in 4.0.next. Jeremy.
(In reply to comment #36) > Re-assigning to Karolin for inclusion in 4.0.next. > Jeremy. Pushed to autobuild-v4-0-test.
(In reply to comment #34) > Yes the 3.6.x patch needs to be re-done. Let's ignore it for now and just leave > SMB2 recvfile as 4.0.x. > Jeremy. Re-assigning to Jeremy to re-do the patch for 3.6.
(In reply to comment #37) > (In reply to comment #36) > > Re-assigning to Karolin for inclusion in 4.0.next. > > Jeremy. > > Pushed to autobuild-v4-0-test. Pushed to v4-0-test.
SMB2 recvfile is a 4.x only feature.