em network issues
Scott Long
scottl at samsco.org
Thu Oct 26 05:20:23 UTC 2006
Jack Vogel wrote:
> 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
Yes, that's almost exactly what was there! It was
#ifndef NO_EM_FASTINTR
Anywho...
More information about the freebsd-net
mailing list