On Thu, 2016-06-30 at 18:30 +0300, Maxim Altshul wrote: > Mesh HWMP module will be able to rely on the HW > RC algorithm if it exists, for path metric calculations. > > This allows the metric calculation mechanism to calculate > a correct metric, based on PER and last TX rate both via > HW RC algorithm if it exists or via parameters collected > by the SW. > > Signed-off-by: Maxim Altshul <maxim.alts...@ti.com> > --- > Changed the function to return u32, I agree that this > is much clearer. > As for the rate, two things: > 1. I had to divide the returned value by 100, since > drv_get_expected_throughput returns values in units of Kbps. > On the contrary, the function cfg80211_calculate_bitrate > returns in units of 100Kbps, so a correction is needed. > 2. Why return the value into rate? > As I understand, rate here is actually bitrate, > and so, we have two possible outcomes: > - A SW/HW RC algo does exist, and an estimated throughput is > returned. err is set to 0 (as it is already included in the RC algo) > and the airtime is calculated using the estimated throughput. > - A SW/HW RC algo does not exist, and thus the regular calculation > takes place, in which an estimated throughput is calculated > using the bitrate and the err parameter. > From this calculation the airtime is calculated. > > net/mac80211/mesh_hwmp.c | 25 +++++++++++++++++-------- > net/mac80211/sta_info.c | 23 +++++++++++++++++++---- > net/mac80211/sta_info.h | 2 ++ > 3 files changed, 38 insertions(+), 12 deletions(-) > > diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c > index c6be0b4..ad67f46 100644 > --- a/net/mac80211/mesh_hwmp.c > +++ b/net/mac80211/mesh_hwmp.c > @@ -322,19 +322,28 @@ static u32 airtime_link_metric_get(struct > ieee80211_local *local, > int device_constant = 1 << ARITH_SHIFT; > int test_frame_len = TEST_FRAME_LEN << ARITH_SHIFT; > int s_unit = 1 << ARITH_SHIFT; > - int rate, err; > + int rate = 0, err = 0;
The rate init is wrong - you overwrite it immediately. The err init is questionable - I think it might be better to write if (rate) { err = 0; } else { ... } below? > u32 tx_time, estimated_retx; > u64 result; > > - if (sta->mesh->fail_avg >= 100) > - return MAX_METRIC; > + /* Try to get rate based on HW/SW RC algorithm. > + * Rate is returned in units of Kbps, correct this > + * to comply with airtime calculation units > + */ > + rate = sta_get_expected_throughput(sta) / 100; > + > + /* if we did not get a rate */ > + if (!rate) { Maybe you want DIV_ROUND_UP to account for getting a very lot estimated rate (< 100Kbps) returned, which isn't "no rate"? Ohterwise looks fine. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html