cvs commit: src/sys/sys mbuf.h src/sys/net if_ethersubr.c src/sys/dev/mxge mxge_lro.c

[Andrew Gallatin]

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.

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.

FWIW, LRO triples receive performance for standard frames (3.xGb/s ->
9.3Gb/s) on decent hardware.

Drew