Re: Question about code in rc80211_pid_algo.c

[Mattias Nissler <mattias.nissler@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 1727 bytes --]

Hi Larry,

comments inline.

On Fri, 2011-03-25 at 22:39 -0500, Larry Finger wrote:
> Hi,
> 
> I'm writing to you as the listed authors of the PID code in the mainline kernel.
> 
> I am helping adapt the PID algorithm for an appliance that will use the Realtek 
> vendor driver and the ALFA AWUS036H device. The vendor driver's rate-control 
> mechanism is not very effective. As the target kernel for the appliance is 
> 2.6.21, we are not backporting mac80211 and the rtl8187 driver.
> 
> The package is working quite well, but Mark, the project manager, brought one 
> section to my attention and I am forwarding it.
> 
> In routine rate_control_pid_sample(), the percentage failure is calculated, then 
> if the rate has been changed, the code goes on to "update the rate behaviour 
> info". At that point, the calculation is
> 
>           tmp = (pf - spinfo->last_pf);
>           tmp = RC_PID_DO_ARITH_RIGHT_SHIFT(tmp, RC_PID_ARITH_SHIFT);
> 
>  From our analysis of the code, both pf and last_pf must be in the range 0-100, 
> and the first statement results in tmp between -100 and 100. After the second 
> line does an arithmetic right shift of 8, the resulting value will always be 
> zero. Did we miss something, or is this really a convoluted way to set "tmp" to 
> zero?

I agree with your analysis, the code will effectively set tmp to zero. I
would consider this a bug :) The whole differences computation business
is Stefano's invention though. IIRC, his idea was that we should not
have a static ordering of rates, but rather learn the order of rates in
terms of achievable TX rate. I guess you really want to check with him
in order to find out what to do :)

Mattias


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]