Subject: Re: ural(4) for NetBSD 3.0, now sync'ed with OpenBSD ural driver
To: David Young <tech-net@NetBSD.org>
From: Damien Bergamini <damien.bergamini@free.fr>
List: tech-kern
Date: 11/29/2005 18:56:13
| On Mon, Nov 28, 2005 at 06:07:57PM +0100, Damien Bergamini wrote:
| > | AMRR is already in the NetBSD tree, although it is coupled with ath(4);
| > | see src/sys/dev/ic/athrate-amrr.[ch].  I prefer that the code isn't
| > | duplicated in ural(4).
| > 
| > People always talk about making things generic but never step in or show
| > any code.  It's not generic because you don't want it to be generic.
| > You want to exploit the different h/w capabilities.  The AMRR algorithm
| > is about 30 lines long by itself.  It's not that big.
| 
| I'm not concerned with the size, I'm concerned with avoiding a "cut
| and paste" programming methodology.

My implementation is fortunately not a copy&paste of what is in ath(4).
The ath(4) implementation of AMRR seems obviously broken to me.
I don't think node_reset() should be called at the end of ath_rate_update().

Otherwise, amn_recovery will always be equal to 0.
Each time it is set to 1, you increase the rate then you call ath_rate_update()
which calls node_reset() which sets amn_recovery back to 0.
The "recovery failure" path is thus never taken (dead code).

The same goes for amn_success_threshold (and maybe amn_success).

Damien