Re: Much improved sosend_*() functions

On Fri, 29 Sep 2006, Andre Oppermann wrote:

>> I like the concept of these changes in principle -- this is the reason I 
>> broke out sosend_copyin(), so that we could start plugging bits of the send 
>> routines more easily for optimization.  However, I think we need to review 
>> this really carefully.  A casual glance brought up one question, and I hope 
>> to get a chance to review this in detail in the next few days.  The 
>> question is with regard to the 'space' variable.  When breaking out 
>> sosend_copyin(), I originally simply passed space in as an argument, which 
>> is what you do currently.  However, I found I had to change it to pass in 
>> the variable by reference so that it could be updated, as later portions of 
>> sosend() depend on it being updated in order to figure out what flags to 
>> pass to pru_send() with respect to PRUS_MORETOCOME, as well as for the 
>> (resid && space > 0) condition for the main loop.  In your revised version, 
>> 'space' isn't updated in sosend() after calling m_uiotombuf(), which on a 
>> casual read, suggests that PRUS_MORETOCOME will no longer get set on the 
>> last pass into pru_send(), and that the loop may go an extra time and pass 
>> more data into TCP than there is room in the socket send buffer.  I may be 
>> reading wrong, I've not had time to look in any detail, but that was my 
>> experience, so you may find you need to pass send by reference, as I do in 
>> sosend_copyin(), for the same reason.
>
> Your observation is correct.  The variable 'space' is no longer updated with 
> my changes.  For sosend_dgram() this is of no consequence as PRUS_ 
> MORETOCOME can't ever be true.  For datagrams the uio must not contain more 
> data than we have space left in the socket buffer.  Otherwise we fail with 
> EMSGSIZE.  For sosend_generic() PRUS_MORETOCOME is no longer correctly set 
> with my changes.  For TCP this is of no consequence either as TCP doesn't 
> care about it.  For correctness I'll change my patch to update 'space' in 
> sosend_generic() and remove the PRUS_MORETOCOME flag from sosend_dgram() 
> completely.

Are you sure TCP doesn't care?  I thought it used PRUS_MORETOCOME as a hint to 
determine whether to immediately output or wait for small segments, but admit 
to never having read the code in detail.  Also, 'space' is not just about 
PRUS_MORETOCOME, it's also about the loop terminating at the right time.

I'll try to give your revised patch a closer reading this weekend.

Thanks,

Robert N M Watson
Computer Laboratory
University of Cambridge