These patches add a feature to initialize the link rate metric with a guess
based off of RSSI when insufficient traffic has passed to allow actual
calculation of the link rate.
I am submitting this patch set on behalf of our team after
testing them with the wireless-testing tag master-2012-06-08.
Requires the patch series noted below to apply cleanly:
[PATCH 0/2] mac80211: mesh remove unused var and fix naming
formerly sent as 3 and 5 respectively in:
[PATCH v3 0/5] Improvement to initial mesh metric
Javier Cardona (2):
mac80211: Keep tx rate averages for mesh peers for better metric
calculation.
mac80211: Use signal strength as a proxy for expected datarate.
net/mac80211/mesh_hwmp.c | 87 +++++++++++++++++++++++++++++++++++++++++-----
net/mac80211/sta_info.c | 1 +
net/mac80211/sta_info.h | 1 +
3 files changed, 80 insertions(+), 9 deletions(-)
--
1.7.9.5
From: Javier Cardona <[email protected]>
When establishing mesh paths with new stations, there is no historic
datarate information to compute the airtime link metric. Up until now
we were just using a poor initial metric. A better approach is to
use the signal strength as an indicator of how good the new link will
be. Once traffic is exchanged with the station, the average datarate is
computed and the signal strength is not used anymore.
Signed-off-by: Javier Cardona <[email protected]>
Signed-off-by: Thomas Pedersen <[email protected]>
Reviewed-by: Jason Abele <[email protected]>
---
formerly sent as:
[PATCH v3 5/5] mac80211: Use signal strength as a proxy for expected datarate.
net/mac80211/mesh_hwmp.c | 67 ++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 65 insertions(+), 2 deletions(-)
diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
index 22b327a..306b182 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -330,6 +330,66 @@ void ieee80211s_update_metric(struct ieee80211_local *local,
return;
}
+/* estimate link rate as a function of last rssi for sta */
+static unsigned int rssi_get_rate(struct sta_info *sta)
+{
+ struct ieee80211_sub_if_data *sdata = sta->sdata;
+ enum ieee80211_band band = sta->local->oper_channel->band;
+ struct ieee80211_sta_ht_cap *ht_cap = &sta->sta.ht_cap;
+ struct rate_info rinfo;
+ int i, rssi, rate, rate_max, rate_min = 10, maxidx = 0;
+ const int rssi_min = -100;
+ const int rssi_max = -20;
+
+ if (sta->last_signal >= 0)
+ return rate_min;
+
+ memset(&rinfo, 0, sizeof(rinfo));
+
+ if (ht_cap->ht_supported) {
+ for (i = 0; i < IEEE80211_HT_MCS_MASK_LEN; i++) {
+ if (ht_cap->mcs.rx_mask[i])
+ maxidx = i*8 + fls(ht_cap->mcs.rx_mask[i]) - 1;
+ }
+ rinfo.flags |= RATE_INFO_FLAGS_MCS;
+ rinfo.flags |= (sta->sta.ht_cap.cap &
+ IEEE80211_HT_CAP_SUP_WIDTH_20_40) ?
+ RATE_INFO_FLAGS_40_MHZ_WIDTH :
+ 0;
+ rinfo.flags |= (sta->sta.ht_cap.cap &
+ (IEEE80211_HT_CAP_SGI_20 |
+ IEEE80211_HT_CAP_SGI_40)) ?
+ RATE_INFO_FLAGS_SHORT_GI :
+ 0;
+ rinfo.mcs = maxidx;
+ } else {
+ struct ieee80211_supported_band *sband;
+
+ maxidx = fls(sta->sta.supp_rates[band]) - 1;
+ if (WARN_ON(maxidx < 0))
+ return rate_min;
+ sband = sta->local->hw.wiphy->bands[band];
+ rinfo.legacy = sband->bitrates[maxidx].bitrate;
+ }
+
+ rate_max = cfg80211_calculate_bitrate(&rinfo);
+
+ rssi = max(rssi_min, sta->last_signal);
+ rssi = min(rssi_max, rssi);
+
+ rate = ((rssi - rssi_min) * rate_max +
+ (rssi_max - rssi) * rate_min) /
+ (rssi_max - rssi_min);
+
+ if (WARN_ON(!rate))
+ rate = rate_min;
+
+ mhwmp_dbg("est. %d Mbps for %pM with rssi %d",
+ rate / 10, sta->sta.addr, sta->last_signal);
+
+ return rate;
+}
+
static u32 airtime_link_metric_get(struct ieee80211_local *local,
struct sta_info *sta)
{
@@ -345,8 +405,11 @@ static u32 airtime_link_metric_get(struct ieee80211_local *local,
if (sta->fail_avg >= 100)
return MAX_METRIC;
- /* If not enough values accumulated in rate average, assume 1 Mbps */
- rate = max(ewma_read(&sta->avg_rate), 10UL);
+ if (sta->tx_packets > 100)
+ rate = max(ewma_read(&sta->avg_rate), 10UL);
+ else
+ /* Not enough traffic sent, use rx signal strengh as proxy */
+ rate = rssi_get_rate(sta);
/* bitrate is in units of 100 Kbps, while we need rate in units of
* 1Mbps. This will be corrected on tx_time computation.
--
1.7.9.5
Hi Johannes,
On Sat, Jun 9, 2012 at 1:55 AM, Johannes Berg <[email protected]> wrote:
> On Fri, 2012-06-08 at 13:31 -0700, Jason Abele wrote:
>
>> + ? ? if (sta->tx_packets > 100)
>> + ? ? ? ? ? ? rate = max(ewma_read(&sta->avg_rate), 10UL);
>> + ? ? else
>> + ? ? ? ? ? ? /* Not enough traffic sent, use rx signal strengh as proxy */
>> + ? ? ? ? ? ? rate = rssi_get_rate(sta);
>
> This just seems completely wrong to me. There's very little correlation
> between bitrates and RSSI, in fact, we once had 0 dBm power by accident
> and got better performance than with 15 dBm power (due to another bug,
> but still)...
I would say this is an improvement over the existing approach, which
assigns the lowest rate (and therefore a poor metric) to all mesh
paths over new peers. This means that a possibly good path through a
new peer is not used until existing mesh paths degrade below this
initial poor metric.
Alternatively we had considered to initialize the metric with the best
(lowest) metric but that just reverses the problem, as the new peers
would attract traffic away from good established paths.
By using RSSI we can at least choose a sensible initial value for the
airtime link metric. I agree with you in that a high RSSI does not
necessarily mean higher rates, but I would not say that there is "very
little correlation" between the two. SNR is directly correlated with
datarates?. Unfortunately SNR measurements are not available at the
mac80211 layer. But at equal noise levels, which is not unlikely
between two direct peer neighbors, RSSI will correlate with SNR
measurements.
Keep in mind that the RSSI is only used until traffic starts flowing
through that peer. Once valid rate statistics are available, the RSSI
is no longer used to calculate the metric.
> Looking at the code that uses this, I also note:
>
>> ? ? ? /* This should be adjusted for each device */
>> ? ? ? ? int device_constant = 1 << ARITH_SHIFT;
>
> which seems *totally* wrong. Maybe it's time to rethink this code?
Indeed, but the standard defines this constant that "Varies depending
on PHY" (see sect. 13.9). We would have to get each driver to report
the right value.
Cheers,
Javier
[1] For instance, see
http://www.inf.ed.ac.uk/teaching/courses/cn/papers/ygz-infocom08.pdf
> johannes
>
> _______________________________________________
> Devel mailing list
> [email protected]
> http://lists.open80211s.org/cgi-bin/mailman/listinfo/devel
--
Javier Cardona
cozybit Inc.
http://www.cozybit.com
From: Javier Cardona <[email protected]>
The mesh metric is computed every time it was requested by the path
selection framework. At that time only the last_tx_rate was taken into
account when computing the metric. This last frame may not reflect the
average data rate that was possible ver that peer link (it could be a
management frame, a rate control probe, or other outlier).
This patch maintains an average tx rate for each peer link which is used
in metric calculation.
Signed-off-by: Javier Cardona <[email protected]>
Reviewed-by: Jason Abele <[email protected]>
---
formerly sent as:
[PATCH v3 3/5] mac80211: Keep tx rate averages for mesh peers for better metric calculation.
net/mac80211/mesh_hwmp.c | 24 +++++++++++++++---------
net/mac80211/sta_info.c | 1 +
net/mac80211/sta_info.h | 1 +
3 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
index 9b6da2d..22b327a 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -307,7 +307,9 @@ void ieee80211s_update_metric(struct ieee80211_local *local,
{
struct ieee80211_tx_info *txinfo = IEEE80211_SKB_CB(skb);
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
+ struct rate_info rinfo;
int failed;
+ int rate;
if (!ieee80211_is_data(hdr->frame_control))
return;
@@ -318,36 +320,40 @@ void ieee80211s_update_metric(struct ieee80211_local *local,
sta->fail_avg = ((80 * sta->fail_avg + 5) / 100 + 20 * failed);
if (sta->fail_avg > 95)
mesh_plink_broken(sta);
+
+ sta_set_rate_info_tx(sta, &sta->last_tx_rate, &rinfo);
+ rate = cfg80211_calculate_bitrate(&rinfo);
+ if (WARN_ON(!rate))
+ return;
+
+ ewma_add(&sta->avg_rate, rate);
+ return;
}
static u32 airtime_link_metric_get(struct ieee80211_local *local,
struct sta_info *sta)
{
- struct rate_info rinfo;
/* This should be adjusted for each device */
int device_constant = 1 << ARITH_SHIFT;
int test_frame_len = TEST_FRAME_LEN << ARITH_SHIFT;
int s_unit = 1 << ARITH_SHIFT;
- int rate, err;
u32 tx_time, estimated_retx;
u64 result;
+ int rate;
+ int err = (sta->fail_avg << ARITH_SHIFT) / 100;
if (sta->fail_avg >= 100)
return MAX_METRIC;
- sta_set_rate_info_tx(sta, &sta->last_tx_rate, &rinfo);
- rate = cfg80211_calculate_bitrate(&rinfo);
- if (WARN_ON(!rate))
- return MAX_METRIC;
-
- err = (sta->fail_avg << ARITH_SHIFT) / 100;
+ /* If not enough values accumulated in rate average, assume 1 Mbps */
+ rate = max(ewma_read(&sta->avg_rate), 10UL);
/* bitrate is in units of 100 Kbps, while we need rate in units of
* 1Mbps. This will be corrected on tx_time computation.
*/
tx_time = (device_constant + 10 * test_frame_len / rate);
estimated_retx = ((1 << (2 * ARITH_SHIFT)) / (s_unit - err));
- result = (tx_time * estimated_retx) >> (2 * ARITH_SHIFT) ;
+ result = (tx_time * estimated_retx) >> (2 * ARITH_SHIFT);
return (u32)result;
}
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 77dcf2f..ddbec7b 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -256,6 +256,7 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata,
do_posix_clock_monotonic_gettime(&uptime);
sta->last_connected = uptime.tv_sec;
ewma_init(&sta->avg_signal, 1024, 8);
+ ewma_init(&sta->avg_rate, 1, 32);
if (sta_prepare_rate_control(local, sta, gfp)) {
kfree(sta);
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 3bb24a1..0bf888e 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -330,6 +330,7 @@ struct sta_info {
unsigned long tx_retry_failed, tx_retry_count;
/* moving percentage of failed MSDUs */
unsigned int fail_avg;
+ struct ewma avg_rate;
/* Updated from TX path only, no locking requirements */
unsigned long tx_packets;
--
1.7.9.5
On Fri, 2012-06-08 at 13:31 -0700, Jason Abele wrote:
> + if (sta->tx_packets > 100)
> + rate = max(ewma_read(&sta->avg_rate), 10UL);
> + else
> + /* Not enough traffic sent, use rx signal strengh as proxy */
> + rate = rssi_get_rate(sta);
This just seems completely wrong to me. There's very little correlation
between bitrates and RSSI, in fact, we once had 0 dBm power by accident
and got better performance than with 15 dBm power (due to another bug,
but still)...
Looking at the code that uses this, I also note:
> /* This should be adjusted for each device */
> int device_constant = 1 << ARITH_SHIFT;
which seems *totally* wrong. Maybe it's time to rethink this code?
johannes
On Fri, 2012-06-08 at 13:31 -0700, Jason Abele wrote:
> These patches add a feature to initialize the link rate metric with a guess
> based off of RSSI when insufficient traffic has passed to allow actual
> calculation of the link rate.
>
> I am submitting this patch set on behalf of our team after
> testing them with the wireless-testing tag master-2012-06-08.
> Requires the patch series noted below to apply cleanly:
> [PATCH 0/2] mac80211: mesh remove unused var and fix naming
>
> formerly sent as 3 and 5 respectively in:
> [PATCH v3 0/5] Improvement to initial mesh metric
What changed, and why did you split up the patch series? Are you
expecting them to go to different trees?
johannes
On Fri, 2012-06-08 at 13:31 -0700, Jason Abele wrote:
> From: Javier Cardona <[email protected]>
>
> The mesh metric is computed every time it was requested by the path
> selection framework. At that time only the last_tx_rate was taken into
> account when computing the metric. This last frame may not reflect the
> average data rate that was possible ver that peer link (it could be a
> management frame, a rate control probe, or other outlier).
>
> This patch maintains an average tx rate for each peer link which is used
> in metric calculation.
I don't think this makes a lot of sense, the TX rate is supposed to be
the "best rate" that the rate control algorithm decided on, so using
EWMA to weigh the bitrate seems a bit counter-productive.
> +++ b/net/mac80211/sta_info.h
> @@ -330,6 +330,7 @@ struct sta_info {
> unsigned long tx_retry_failed, tx_retry_count;
> /* moving percentage of failed MSDUs */
> unsigned int fail_avg;
> + struct ewma avg_rate;
and in any case this is missing kernel-doc
johannes
Hi Johannes,
On Sat, Jun 9, 2012 at 1:53 AM, Johannes Berg <[email protected]> wrote:
> On Fri, 2012-06-08 at 13:31 -0700, Jason Abele wrote:
>> From: Javier Cardona <[email protected]>
>>
>> The mesh metric is computed every time it was requested by the path
>> selection framework. ?At that time only the last_tx_rate was taken into
>> account when computing the metric. ?This last frame may not reflect the
>> average data rate that was possible ver that peer link (it could be a
>> management frame, a rate control probe, or other outlier).
>>
>> This patch maintains an average tx rate for each peer link which is used
>> in metric calculation.
>
> I don't think this makes a lot of sense, the TX rate is supposed to be
> the "best rate" that the rate control algorithm decided on
Yes, but last_tx_rate is not that. This is a snippet from
ieee80211_tx_h_rate_ctrl():
txrc.skb = tx->skb;
txrc.reported_rate.idx = -1;
(...)
rate_control_get_rate(tx->sdata, tx->sta, &txrc);
(...)
if (txrc.reported_rate.idx < 0) {
txrc.reported_rate = info->control.rates[0];
if (tx->sta && ieee80211_is_data(hdr->frame_control))
tx->sta->last_tx_rate = txrc.reported_rate;
} else if (tx->sta)
tx->sta->last_tx_rate = txrc.reported_rate;
last_tx_rate is just the last rate used to send a data frame, not the
average data rate. For instance, if the last data frame is a
broadcast frame transmitted at the lowest rate, last_tx_rate will
report that low rate and not the much higher rate used for unicast
data frames. Using that low rate to compute the airtime link metric
is an aberration.
Cheers,
Javier
>
>> +++ b/net/mac80211/sta_info.h
>> @@ -330,6 +330,7 @@ struct sta_info {
>> ? ? ? unsigned long tx_retry_failed, tx_retry_count;
>> ? ? ? /* moving percentage of failed MSDUs */
>> ? ? ? unsigned int fail_avg;
>> + ? ? struct ewma avg_rate;
>
> and in any case this is missing kernel-doc
>
> johannes
>
> _______________________________________________
> Devel mailing list
> [email protected]
> http://lists.open80211s.org/cgi-bin/mailman/listinfo/devel
--
Javier Cardona
cozybit Inc.
http://www.cozybit.com