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

[Andrew Bartlett]

On Fri, 2011-12-23 at 08:14 +1100, Andrew Bartlett wrote:
> On Thu, 2011-12-22 at 11:12 -0800, Jeremy Allison wrote:

> > > In particular, what is the purpose of the 'socket draining' code?  I'm a
> > > little confused, because the comments at the callers
> > > (source3/smbd/vfs.c) indicate it is expected, but it is only explicitly
> > > in the splice() code path in source3/lib/recvfile.c and not in the
> > > read/write 'fallback' (which is actually what we always use). 
> > 
> > Ok, when recvfile() returns we *must* have drained the incoming
> > socket of the remainder of the SMB packet, else we'll lose packet
> > boundary coherence.
> > 
> > The socket draining code was added as it is (theoretically?) possible
> > for the splice() code path to return a short read, leaving data
> > in the socket buffers (the aforementioned packet boundary coherence
> > problem) so we need to drain the socket in this case.

I've been looking at it given your comments, and I'm not convinced by
the current code.  In particular, I'm thinking 	if (total_written <
count) {

is not the correct test for draining the socket.  Why does draining the
socket have anything to do with total_written?  Shouldn't it be:

if (count) {

That is, during a multi-part recvfile of 48k of data (due to the 16k
chunks) I think that we could get a pattern like:

count = 48000
total_written = 0

nread = 16000

end loop 1:
total_written = 16000
count = 32000

error:
total_written is < count
drain 16000 from fromfd, leaving 16000 in fromfd

or 

start loop 2:
nread = 16000
total_written = 32000
count = 16000

error:

total_written is > count
no drain, but 16000 still in fromfd

(sorry if my notes are a little cryptic)

> > FYI: I'm on Holiday from now until Jan 3rd so email response
> > times can be a little sketchy !

Don't feel pressured to get back to me, I'm just keen to get important
code like this right for all of us in the new year.

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: recvfile-fix-drain.patch
Type: text/x-patch
Size: 470 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20111224/cc1458ea/attachment.bin>