Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 29 Sep 2006 09:35:25 +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:  <20060929092813.U73166@fledge.watson.org>
In-Reply-To: <451C4850.5030302@freebsd.org>
References:  <451C4850.5030302@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

On Fri, 29 Sep 2006, Andre Oppermann wrote:

> The patch is available here: 
> http://people.freebsd.org/~andre/sosend+m_uiotombuf-20060928.diff

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.

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?20060929092813.U73166>