Date: Fri, 29 Sep 2006 14:07:26 +0100 (BST) From: Robert Watson <rwatson@FreeBSD.org> To: Andre Oppermann <andre@freebsd.org> Cc: freebsd-net@freebsd.org, freebsd-current@freebsd.org, gallatin@cs.duke.edu Subject: Re: Much improved sosend_*() functions Message-ID: <20060929140545.L66510@fledge.watson.org> In-Reply-To: <451D191C.2050307@freebsd.org> References: <451C4850.5030302@freebsd.org> <20060929092813.U73166@fledge.watson.org> <451D191C.2050307@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060929140545.L66510>