Date: Mon, 11 Jun 2007 13:08:21 -0700 From: "Jack Vogel" <jfvogel@gmail.com> To: "Andre Oppermann" <andre@freebsd.org> Cc: cvs-src@freebsd.org, Scott Long <scottl@samsco.org>, src-committers@freebsd.org, Andrew Gallatin <gallatin@cs.duke.edu>, 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 Message-ID: <2a41acea0706111308o5353d7dey377602d8187374cd@mail.gmail.com> In-Reply-To: <466DA974.8000106@freebsd.org> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
On 6/11/07, Andre Oppermann <andre@freebsd.org> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2a41acea0706111308o5353d7dey377602d8187374cd>