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
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
