From owner-freebsd-current@FreeBSD.ORG Fri Sep 29 13:07:27 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 DDBC716A40F; Fri, 29 Sep 2006 13:07:27 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from cyrus.watson.org (cyrus.watson.org [209.31.154.42]) by mx1.FreeBSD.org (Postfix) with ESMTP id 5B0D043D4C; Fri, 29 Sep 2006 13:07:27 +0000 (GMT) (envelope-from rwatson@FreeBSD.org) Received: from fledge.watson.org (fledge.watson.org [209.31.154.41]) by cyrus.watson.org (Postfix) with ESMTP id E391946D0B; Fri, 29 Sep 2006 09:07:26 -0400 (EDT) Date: Fri, 29 Sep 2006 14:07:26 +0100 (BST) From: Robert Watson X-X-Sender: robert@fledge.watson.org To: Andre Oppermann In-Reply-To: <451D191C.2050307@freebsd.org> Message-ID: <20060929140545.L66510@fledge.watson.org> References: <451C4850.5030302@freebsd.org> <20060929092813.U73166@fledge.watson.org> <451D191C.2050307@freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed 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:07:28 -0000 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