From owner-cvs-all@FreeBSD.ORG Mon Jun 11 20:08:23 2007 Return-Path: X-Original-To: cvs-all@freebsd.org Delivered-To: cvs-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id A746D16A494 for ; Mon, 11 Jun 2007 20:08:23 +0000 (UTC) (envelope-from jfvogel@gmail.com) Received: from wa-out-1112.google.com (wa-out-1112.google.com [209.85.146.179]) by mx1.freebsd.org (Postfix) with ESMTP id 1FF2013C45D for ; Mon, 11 Jun 2007 20:08:23 +0000 (UTC) (envelope-from jfvogel@gmail.com) Received: by wa-out-1112.google.com with SMTP id j37so2462131waf for ; Mon, 11 Jun 2007 13:08:22 -0700 (PDT) DKIM-Signature: a=rsa-sha1; c=relaxed/relaxed; d=gmail.com; s=beta; h=domainkey-signature:received:received:message-id:date:from:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references; b=nd+MxCo3fNevcJ0FphZnOXfFH9H/rVKa3I6pksM9GF7/9n9qeiYElLtjZauAGjXiG6z3937NwDlXFlzKIm3f8Ckp1x4uONiuUobESfxyKiZB6lXGYWVxwxoa/HQz7lTStQl6RtdYb778k/ABkmB6eAAXB2Xxniu3GizaNXlarvo= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:message-id:date:from:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references; b=Nq9henmQeokkUHZhGVTshsniw7W1qEzQZYLte2K1Mckn9j6+Q4u89QEN62TTi5KYggxZj49RpyoF312glVWoblOjb8mO9lZAyocuMLq4WYEQbWY0cCxgk+AzPxmJ8zESg3b2eeNfzxrZJKQAbFv6T29U15gng4BY2tMePMc0eug= Received: by 10.114.176.1 with SMTP id y1mr5842557wae.1181592501972; Mon, 11 Jun 2007 13:08:21 -0700 (PDT) Received: by 10.114.126.10 with HTTP; Mon, 11 Jun 2007 13:08:21 -0700 (PDT) Message-ID: <2a41acea0706111308o5353d7dey377602d8187374cd@mail.gmail.com> Date: Mon, 11 Jun 2007 13:08:21 -0700 From: "Jack Vogel" To: "Andre Oppermann" In-Reply-To: <466DA974.8000106@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <200706111459.l5BExvTp020932@repoman.freebsd.org> <466D9BBB.1060601@freebsd.org> <466D9D94.1020908@samsco.org> <466DA400.6000003@freebsd.org> <18029.42579.130017.451610@grasshopper.cs.duke.edu> <466DA974.8000106@freebsd.org> Cc: cvs-src@freebsd.org, Scott Long , src-committers@freebsd.org, Andrew Gallatin , cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/sys mbuf.h src/sys/net if_ethersubr.c src/sys/dev/mxge mxge_lro.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 11 Jun 2007 20:08:23 -0000 On 6/11/07, Andre Oppermann wrote: > Andrew Gallatin wrote: > > Andre Oppermann writes: > > > Scott Long wrote: > > > > Andre Oppermann wrote: > > > >> Andrew Gallatin wrote: > > > >>> gallatin 2007-06-11 14:59:56 UTC > > > >>> > > > >>> FreeBSD src repository > > > >>> > > > >>> Modified files: > > > >>> sys/sys mbuf.h sys/net > > > >>> if_ethersubr.c sys/dev/mxge mxge_lro.c Log: > > > >>> Allow drivers, such as cxgb and mxge, which support LRO to bypass > > > >>> the MTU check in ether_input() on LRO merged frames. > > > >>> Discussed with: kmacy > > > >> Not discussed with: andre > > > >> > > > >> Your change isn't the right way to make this work. LRO is an interface > > > >> capability (that should have the option to disable it) and the test in > > > >> ether_input() should go on that instead. LRO is not an information > > > >> that is needed beyond ether_input() and thus doesn't have to be a mbuf > > > >> flag. > > > >> > > > >> I've indicated that I'm working in this area as well and at least > > > >> dropping an email or a ping IRC would have been nice. I would have > > > >> told you the above right away. My common version of LRO isn't ready > > > >> yet as I'm a bit short on time and I chose to concentrate on TCP it- > > > >> self. We only have to make sure that we don't exclude a common LRO > > > >> implementation due to API/ABI issues for 7.1R. > > > >> > > > > > > > > Drew's commit looks simple and non-obtrusive enough that it can likely > > > > be replaced once your perfected LRO implementation is done and in the > > > > tree. Until that happens, I can't imagine a good reason to block his > > > > and Kip's work. > > > > > > I'm blocking nothing. Read again what I wrote. I complained about > > > a technically wrong approach to solve a particular side problem. In > > > the meantime it got backed out because someone else complained too. > > > > I specifically *didn't* make it an interface flag *because* of your > > promised generic LRO support. Ie, I didn't want to have any standard, > > documented interface to carry around for what is nothing more than a > > temporary hack to allow 10GbE with standard frames to not suck on > > FreeBSD in the short term. Per-driver LRO support is a nightmare that > > should be ripped out when your generic support is ready. I'm really > > looking forward to your work, and tried to keep the per-driver stuff > > as unobtrusive as possible. > > LRO actually should become an interface capability flag just like > TSO so we can turn it individually on/off for every interface. LRO > must be done inside the driver, even the generic one. Having the > flag doesn't interfere with the common implementation drivers should > use in the future when it's ready. We should do the flag right now > to have it included in the 7.0 API/ABI. > > > As to the other complaint, Sam complained about the scarcity of mbuf > > flags, not about LRO in general. We agreed that, since its basically > > a temporary hack, the best thing to do would be to move the frame > > length check under DIAGNOSTIC, where it should be anyway. > > I didn't (want to) complain about your LRO either. I'm fine with it. > Just it shouldn't have used the mbuf flag as Sam said too but for > different reasons. > > > FWIW, LRO triples receive performance for standard frames (3.xGb/s -> > > 9.3Gb/s) on decent hardware. > > Nice to see that. The problem with LRO at the moment is that it only > works on short RTT links (<1ms) because the TCP stack doesn't do ABC > yet and growing the send window with a LRO receiver is going to be > painfully slow as the RTT goes up. > > Lets add the interface capabilities flag for LRO including the ifconfig > support and be done with this episode. I agree with this approach also. Jack