Date: Mon, 11 Jun 2007 21:58:44 +0200 From: Andre Oppermann <andre@freebsd.org> To: Andrew Gallatin <gallatin@cs.duke.edu> Cc: cvs-src@freebsd.org, Scott Long <scottl@samsco.org>, src-committers@freebsd.org, 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: <466DA974.8000106@freebsd.org> In-Reply-To: <18029.42579.130017.451610@grasshopper.cs.duke.edu> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
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. -- Andre
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?466DA974.8000106>