cvs commit: src/sys/sys mbuf.h src/sys/net if_ethersubr.c
jfvogel at gmail.com
Mon Jun 11 20:08:23 UTC 2007
On 6/11/07, Andre Oppermann <andre at 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.
More information about the cvs-src