svn commit: r275326 - in head: sys/dev/cxgbe/tom sys/kern sys/netinet sys/sys usr.bin/bluetooth/btsockstat usr.bin/netstat usr.bin/systat

[Alfred Perlstein]

On 11/30/14, 10:38 AM, Gleb Smirnoff wrote:
>    Adrian & Alfred,
>
> On Sun, Nov 30, 2014 at 09:55:05AM -0800, Adrian Chadd wrote:
> A> I really wished that these commits got individual reviews before they went in.
>
> On Sun, Nov 30, 2014 at 10:10:57AM -0800, Alfred Perlstein wrote:
> A> Agree, this was already discussed as quite possibly an incorrect way
> A> forward some months ago.
> A>
> A> Where is the actual review for this huge change to the networking stack?
> A>
>
> The development was going on in the open branch, and I've been sending the
> diff on the new sendfile constantly during this year. Last time I sent the
> diff, the discussion very quickly moved to Lua in kernel, script(2) and
> other buzz, actually ignoring my review and test request and hijacking
> my thread:
>
> https://lists.freebsd.org/pipermail/freebsd-arch/2014-September/015861.html
>
> Yes, Alfred did ask me why can't this be accomplished with socket
> upcalls, and replied I to him, but discussion ended there:
>
> https://lists.freebsd.org/pipermail/freebsd-arch/2014-September/015858.html
>
> Probably the script(2) topic was more appealing.
>
> If you don't mind, I will make a separate replies on your actual
> technical questions.
>
Gleb,

Maybe I misread your reply to 
'https://lists.freebsd.org/pipermail/freebsd-arch/2014-September/015858.html' 
but my interpretation of your reply was that you would reconsider your 
approach because the suggestion to use socket callback was a valid one.

I am very open to being wrong on this issue, but with all due respect I 
don't see a good reason why not to use socket callbacks to even 
implement what you have right now.

I appreciate what seem to be kind words saying my idea is good, but it 
didn't really answer the question nor address the issue I raised.

I am currently sorting out why this approach was taken and here's what I 
have so far, this is me trying to find the best here:

1) Possible that the socket buffer accounting needs to be done to keep 
TCP window open for send?  (Could this be forced via setsockopt(2) 
option instead)?
2) Lock order issues in the socket callback.
3) There is only a single socket callback func and arg (maybe we need more?)
4) Properly asserting backpressure and actual blocking in sendfile(2) 
becomes very difficult otherwise, meaning that if you don't have the 
complexity in socketbuffer, then you need to sort of implement this 
inside a private control block AND do accounting there.
5) This is a "cool feature" and maybe other things can make use of it 
eventually?  <- hoping this is more of a good thing as opposed to 
(sorry) kse.
6) Alllows one to mix async sendfile write WITH sync write and the 
correct thing happens?  Am I right in assuming that socketbuffers can 
wind up with mbufs as such:  M_NOTREADY, M_READY, M_NOTREADY, M_READY, 
M_NOTREADY, M_READY ?  If so that is pretty interesting and useful.

Again my proposal would be to use socket callback and callback arg would 
basically be something like:

socket_sendfile_cb_arg {
   struct mbuf *head;  /* list of mbufs queued */
}

When the callback is fired just pop off the front of the queue and put 
on the socketbuffer.

This however has the following drawbacks:
1) May have trouble if anyone else is using the sbcallback as well? 
aio?  (might need to allow multiple callbacks now).
2) Can't do M_NOTREADY, M_READY, M_NOTREADY, M_READY, M_NOTREADY, 
M_READY, basically write(2) can get in front of async sendfile.

I know you have put a lot of effort into this and these are just my 
semi-random thoughts on the issue, so I apologize if I'm making you 
repeat a lot of your thinking on the issue, I would just like to 
understand a bit better why this was done and if we really need to do it.

-Alfred