From: Johannes Berg <johannes@sipsolutions.net>
To: Maxim Altshul <maxim.altshul@ti.com>, linux-kernel@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH v2] mac80211: mesh: Add support for HW RC implementation
Date: Wed, 06 Jul 2016 14:35:25 +0200 [thread overview]
Message-ID: <1467808525.5460.4.camel@sipsolutions.net> (raw)
In-Reply-To: <20160630153055.26497-1-maxim.altshul@ti.com>
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.altshul@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
next prev parent reply other threads:[~2016-07-06 12:35 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-30 15:30 [PATCH v2] mac80211: mesh: Add support for HW RC implementation Maxim Altshul
2016-07-06 12:35 ` Johannes Berg [this message]
-- strict thread matches above, loose matches on Subject: below --
2016-06-23 13:53 Maxim Altshul
2016-06-23 19:26 ` Johannes Berg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1467808525.5460.4.camel@sipsolutions.net \
--to=johannes@sipsolutions.net \
--cc=davem@davemloft.net \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=maxim.altshul@ti.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).