Socket draining for sys_recvfile() (patch for 'reverse sendfile')

[Jeremy Allison]

On Fri, Dec 30, 2011 at 05:57:42PM -0800, Jeremy Allison wrote:
> On Fri, Dec 30, 2011 at 05:37:25PM -0800, Jeremy Allison wrote:
> > On Fri, Dec 30, 2011 at 02:13:34PM +1100, Andrew Bartlett wrote:
> > > On Mon, 2011-12-26 at 08:38 +1100, Andrew Bartlett wrote:
> > > > On Sat, 2011-12-24 at 21:02 -0800, Jeremy Allison wrote:
> > > > 
> > > > > Completely correct. You've found a bug in the splice() codepath !
> > > > > 
> > > > > Well done & thanks.
> > > > 
> > > > No worries!  Now that is fixed up, I'm much more confident to include it
> > > > in the patch for reverse sendfile.
> > > > 
> > > > > The reason this hasn't been an issue up until now is that
> > > > > the splice() codepath is (a) slower than the read/write path,
> > > > > so was never used by default, (b) the Isilon code didn't exercise
> > > > > this codepath at all - they used a custom written sendfile()
> > > > > implementation and (c) it's in an error path rarely taken
> > > > > (probably only on disk full).
> > > > 
> > > > Yeah, and that is the error case I need to test.
> > > 
> > > I've been testing the disk full semantics, and in 3.5.11, they are
> > > certainly not correct, even when just using the fallback:
> > > 
> > > Using a small flash drive attached to my NAS, with about 1MB free, with
> > > 'min recvfile size = 1':
> > > 
> > > [abartlet at ruth netgear-src]$ /data/samba/git/samba/bin/smbclient3 //nas-02-22-94/USB_FLASH_1 -Uabartlet%penguin
> > > Domain=[NETGEAR] OS=[Unix] Server=[Samba 3.5.11]
> > > smb: \> rm centos.iso2
> > > smb: \> put /data/vm/cdroms/CentOS-5.5-x86_64-bin-DVD-1of2.iso centos.iso2
> > > cli_push returned NT_STATUS_INVALID_NETWORK_RESPONSE
> > > putting file /data/vm/cdroms/CentOS-5.5-x86_64-bin-DVD-1of2.iso as \centos.iso2 (66.0 kb/s) (average 66.0 kb/s)
> > > smb: \> ^C
> > > 
> > > with 'min recvfile size = 0':
> > > [abartlet at ruth netgear-src]$ /data/samba/git/samba/bin/smbclient3 //nas-02-22-94/USB_FLASH_1 -Uabartlet%penguin
> > > Domain=[NETGEAR] OS=[Unix] Server=[Samba 3.5.11]
> > > smb: \> rm centos.iso2
> > > smb: \> put /data/vm/cdroms/CentOS-5.5-x86_64-bin-DVD-1of2.iso centos.iso2
> > > cli_push returned NT_STATUS_DISK_FULL
> > > putting file /data/vm/cdroms/CentOS-5.5-x86_64-bin-DVD-1of2.iso as \centos.iso2 (76.0 kb/s) (average 76.0 kb/s)
> > > smb: \> 
> > > 
> > > I have a capture of both at: http://abartlet.net/recvfile-broken.cap.gz  (I also get similar behaviour with my patch)
> > > 
> > > Are there any changes to this behaviour since 3.5 I should be aware of?
> > > 
> > > I'll try out master once I sort out some unrelated loadparm issues on this Sparc based platform
> > 
> > Thanks (got your message about master also being broken).
> > 
> > A debug level 10 log would be more help than a capture file
> > (although both would be better). I'll take a look at this
> > on Tues (when I'm back at work). Hope that's ok.
> 
> I took a quick look at your capture and it seems to be returning successes
> to all the writes in the first test case. Not sure where the client is
> detecting NT_STATUS_INVALID_NETWORK_RESPONSE in the replies.
> 
> Can you split out the success and fail captures into 2 different files ?
> 
> (I'll try and take a look at this, family time permitting - over the
> next few days).

Just FYI for anyone following - I set up the
same (small usb drive nearly full, min recvfile size = 1
set in smb.conf) and the problem doesn't happen in
the normal (non-splice) codepath - smbclient gets
STATUS_DISK_FULL on write.

Phew. I didn't think we were *that* broken :-). I'll
look into enabling the non-default splice codepath
next and see if I can reproduce with that.

Jeremy.