From owner-freebsd-current@FreeBSD.ORG Fri Sep 29 13:01:20 2006 Return-Path: X-Original-To: freebsd-current@freebsd.org Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 7DC1616A4CA for ; Fri, 29 Sep 2006 13:01:20 +0000 (UTC) (envelope-from andre@freebsd.org) Received: from c00l3r.networx.ch (c00l3r.networx.ch [62.48.2.2]) by mx1.FreeBSD.org (Postfix) with ESMTP id B5B3843D53 for ; Fri, 29 Sep 2006 13:01:17 +0000 (GMT) (envelope-from andre@freebsd.org) Received: (qmail 93219 invoked from network); 29 Sep 2006 13:02:32 -0000 Received: from c00l3r.networx.ch (HELO [127.0.0.1]) ([62.48.2.2]) (envelope-sender ) by c00l3r.networx.ch (qmail-ldap-1.03) with SMTP for ; 29 Sep 2006 13:02:32 -0000 Message-ID: <451D191C.2050307@freebsd.org> Date: Fri, 29 Sep 2006 15:01:16 +0200 From: Andre Oppermann User-Agent: Thunderbird 1.5.0.7 (Windows/20060909) MIME-Version: 1.0 To: Robert Watson References: <451C4850.5030302@freebsd.org> <20060929092813.U73166@fledge.watson.org> In-Reply-To: <20060929092813.U73166@fledge.watson.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: freebsd-net@freebsd.org, freebsd-current@freebsd.org, gallatin@cs.duke.edu Subject: Re: Much improved sosend_*() functions X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Sep 2006 13:01:20 -0000 Robert Watson wrote: > > 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. 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. -- Andre