em network issues

[Jack Vogel]

On 10/25/06, Scott Long <scottl at samsco.org> wrote:
> Jack Vogel wrote:
> > On 10/25/06, Doug Ambrisko <ambrisko at ambrisko.com> wrote:
> >
> >>      3) In em_process_receive_interrupts/em_rxeof always decrement
> >>         the count on every run through the loop.  If you notice
> >>         count is an is an int that starts at the passed in value
> >>         of -1.  It then count-- until count==0.  Doing -1, -2, -3
> >>         takes awhile until the int rolls over to 0.   Passing 100
> >>         limits it more :-)  So this can run 3 * 100 versuses
> >>         infinite * int roll over assuming we don't skip a count--.
> >
> > Been chatting with Jesse Brandeburg (one of our senior Linux guys) about
> > receive side cleaning. Gave me a number of things to think about. First
> > off,
> > this change you mention is problematic. The reason it doesnt increment
> > every time thru the while loop is its meant as a packet counter, NOT a
> > descriptor counter. If we just fix this number at 100, and have it only
> > counting descriptors you could get all but the EOP descriptor of a packet
> > and then exit without finishing it and calling the stack, not a good
> > tactic.
> >
> > Having a limited count is still a good idea, but I think we still want
> > to base
> > it on packets and not just descriptors.
> >
> > Jesse also talked about their experience with the Linux driver, deciding
> > where to update the RDT, my current code doesnt do it til after the whole
> > while loop is completed (havent looked at CURRENT again today yet),
> > Jesse suggested doing it but not EVERY pass in the loop, maybe making
> > it mod the number of rx descriptors. Having that AND a fixed limit on the
> > number of total packets cleaned in a pass might be good.
>
> Good idea.  Though for 1518 byte frames, I think you'll only have one
> descriptor per packet.  Definitely need to do the right thing for jumbo
> frames, though.
>
> >
> > I was also thinking, maybe having the taskqueue code be selectable, but
> > not just a POLL vs TASKQUEUE, rather keep a legacy intr option which
> > has a POLL option within it.
> >
> > My motivation for doing that is I can take the TASKQUEUE code into the
> > Intel code base, but make it backward compatible, the default would have
> > it optioned off.
> >
> > Jack
>
> I had it that way initially, and I think you commented that it was ugly
> ;-)

Naaahhhh, couldnt be, I'd never do anything like that :)

OHHHH, I know what you're talking about. When I first started this job a year
ago the driver was just PEPPERED with all these #if _FreeBSD_Version < BladdyFoo
or something like that. I think the Intel code base was even worse cuz Tony was
trying to make a single source base for 4.X and 5.X at that point. It
was a major
pain to look at that code :)

What I'm talking about is a simple #ifdef EM_FASTINTR or something like that,
no defines that remind me of POSIX header files please :)

Jack