Steven Stallion wrote:
>> 1) enable_interurpts() used to conditionally enable tx interrutps.  Now
>> its unconditional.  Its probably for the best (simpler logic, less
>> likely to result in stalls, etc.)  You might want to note something
>> somewhere, though, as it potentially has a small performance impact.
>>     
>
> enable_tx_interrupts did originally have the conditional, however every
> usage of the function enabled the interrupts. The conditional was removed
> for clarity.
>
> I can certainly add a comment and/or re-add the original conditional logic
> if this is troublesome.
>   

I just looked again.  I recall now that the conditional to clear that 
was #ifdef'd out.  So, just leave it out.  We'll never need to enable 
this.  Its not like dnet is a high performance NIC anyway.

>   
>> 2) much worse, your transmit logic only seems to transmit a single
>> packet linked by mp->b_cont.  With GLDv3, you can get multiple packets,
>> linked together with mp->b_next.  You need to have some logic to handle
>> this in a loop, or somesuch.  If you can't find any example code, let me
>> know.
>>     
>
> That's a big one (I didn't realize that GLDv3 supported this). I'll
> definitely add this in.
>   

Thanks.  I'll watch for your updated webrev.  Once you get some good 
testing in, then I can start the process for the RTI.  It would be nice 
to get at least one other name besides mine on the code review list, 
only because I'm also the one filing the RTI.  But it isn't critical.

    -- Garrett
>   
>> Have you had a chance to do any testing yet?
>>     
>
> I ended up having to go out of town this weekend, but I did manage to get
> nicdrv setup on both the server and client (direct link to a bge; the
> client also has more resources available per the documentation).
>
> I will be running the tests this evening. Depending on the results, I
> should have an amended webrev ready this evening or at the latest,
> tomorrow which will include the fixed/improved tx logic.
>
> Steve
>
>   

_______________________________________________
driver-discuss mailing list
driver-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/driver-discuss

Reply via email to