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