| Message ID | 20190115233156.225963-1-julanhsu@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | mac80211: mesh: use average bitrate for link metric calculation | expand |
Hi, Am 16.01.2019 um 00:31 schrieb julanhsu@google.com: > From: Julan Hsu <julanhsu@google.com> > > Use bitrate moving average to smooth out link metric and stablize path > selection. We had some investigations on the same field, but I didn't had time to publish anything of that. The general idea is good and we implemented it similar. One Issue is the different rate of multi cast and management frames in contrast to uni cast data frames. AFAIK, the rate for management frames, especially the beacon frames of mesh, are always sent with a low mandatory rates (although they are configurable). If the link is less used, the beacon frames may slowly degrade the average. But anyhow, this problem also exist in the current implementation. The other problem is, as you may expected already, the actual used information for the metrics are highly driver dependent. So e.g. the ath9k driver had some issue in the past, were the update metric function was not called for some runtime modes (no-SKB vs SKB ect.). Or the update_metric function is not called for all kind of frames. But in general I think it is a first good step for better metrics.
Altough the patch is already accepted, I have a question regarding the current semantics of the whole function airtime_link_metric_get: My Impression was, that rate = DIV_ROUND_UP(sta_get_expected_throughput(sta), 100); will get the expected throughput from the rate control, so e.g. for ath9k it will receive the already averaged rate from minstrel(_ht). I don't know which hardware you use, but we have only mesh hardware in our lab, which use minstrel, so this code wouldn't be triggered. Unfortunately, we are on a quite old kernel in the lab, where this code is not present, so I'm not able to proof my assumption. Nevertheless, my former comment is still valid, as the averaged rate could be calculated over several kinds of frames (management/multicast) which have a inherently lower rate. This could have the inverse effect to slowly change the path metric, while this patch should stabilize it, isn't it ?
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index 818aa0060349..31a47d3a122d 100644 --- a/net/mac80211/cfg.c +++ b/net/mac80211/cfg.c @@ -1231,6 +1231,11 @@ static void sta_apply_mesh_params(struct ieee80211_local *local, ieee80211_mps_sta_status_update(sta); changed |= ieee80211_mps_set_sta_local_pm(sta, sdata->u.mesh.mshcfg.power_mode); + + ewma_mesh_tx_rate_avg_init(&sta->mesh->tx_rate_avg); + /* init at low value */ + ewma_mesh_tx_rate_avg_add(&sta->mesh->tx_rate_avg, 10); + break; case NL80211_PLINK_LISTEN: case NL80211_PLINK_BLOCKED: diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c index 6950cd0bf594..34f11be04e64 100644 --- a/net/mac80211/mesh_hwmp.c +++ b/net/mac80211/mesh_hwmp.c @@ -300,6 +300,7 @@ void ieee80211s_update_metric(struct ieee80211_local *local, { struct ieee80211_tx_info *txinfo = st->info; int failed; + struct rate_info rinfo; failed = !(txinfo->flags & IEEE80211_TX_STAT_ACK); @@ -310,12 +311,15 @@ void ieee80211s_update_metric(struct ieee80211_local *local, if (ewma_mesh_fail_avg_read(&sta->mesh->fail_avg) > LINK_FAIL_THRESH) mesh_plink_broken(sta); + + sta_set_rate_info_tx(sta, &sta->tx_stats.last_rate, &rinfo); + ewma_mesh_tx_rate_avg_add(&sta->mesh->tx_rate_avg, + cfg80211_calculate_bitrate(&rinfo)); } 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; @@ -339,8 +343,7 @@ static u32 airtime_link_metric_get(struct ieee80211_local *local, if (fail_avg > LINK_FAIL_THRESH) return MAX_METRIC; - sta_set_rate_info_tx(sta, &sta->tx_stats.last_rate, &rinfo); - rate = cfg80211_calculate_bitrate(&rinfo); + rate = ewma_mesh_tx_rate_avg_read(&sta->mesh->tx_rate_avg); if (WARN_ON(!rate)) return MAX_METRIC; diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h index 9a04327d71d1..b94aeb680d2d 100644 --- a/net/mac80211/sta_info.h +++ b/net/mac80211/sta_info.h @@ -343,6 +343,7 @@ struct ieee80211_fast_rx { /* we use only values in the range 0-100, so pick a large precision */ DECLARE_EWMA(mesh_fail_avg, 20, 8) +DECLARE_EWMA(mesh_tx_rate_avg, 8, 16) /** * struct mesh_sta - mesh STA information @@ -365,6 +366,7 @@ DECLARE_EWMA(mesh_fail_avg, 20, 8) * @processed_beacon: set to true after peer rates and capabilities are * processed * @fail_avg: moving percentage of failed MSDUs + * @tx_rate_avg: moving average of tx bitrate */ struct mesh_sta { struct timer_list plink_timer; @@ -392,6 +394,8 @@ struct mesh_sta { /* moving percentage of failed MSDUs */ struct ewma_mesh_fail_avg fail_avg; + /* moving average of tx bitrate */ + struct ewma_mesh_tx_rate_avg tx_rate_avg; }; DECLARE_EWMA(signal, 10, 8)