From: Arik Nemtsov <[email protected]>
Even if the current chandef width is equal to the station's max-BW, it
doesn't mean it's a valid width for TDLS. Make sure to always check
regulatory constraints in these cases.
Fixes: dde4002 ("mac80211: upgrade BW of TDLS peers when possible")
Signed-off-by: Arik Nemtsov <[email protected]>
Signed-off-by: Emmanuel Grumbach <[email protected]>
---
net/mac80211/tdls.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/mac80211/tdls.c b/net/mac80211/tdls.c
index c9eeb3f..43f13abe 100644
--- a/net/mac80211/tdls.c
+++ b/net/mac80211/tdls.c
@@ -332,7 +332,7 @@ ieee80211_tdls_chandef_vht_upgrade(struct ieee80211_sub_if_data *sdata,
return;
/* proceed to downgrade the chandef until usable or the same */
- while (uc.width > max_width &&
+ while (uc.width > max_width ||
!cfg80211_reg_can_beacon_relax(sdata->local->hw.wiphy, &uc,
sdata->wdev.iftype))
ieee80211_chandef_downgrade(&uc);
--
2.5.0
From: Arik Nemtsov <[email protected]>
The previous approach simply ignored chandef restrictions when calculating
the appropriate peer BW for a WIDER_BW peer. This could result in a
regulatory violation if both peers indicated 80MHz support, but the
regdomain forbade it.
Change the approach to setting a WIDER_BW peer's BW. Don't exempt it from
the chandef width at first. If during TDLS negotiation the chandef width
is upgraded, update the peer's BW to match.
Fixes: dde4002 ("mac80211: upgrade BW of TDLS peers when possible")
Signed-off-by: Arik Nemtsov <[email protected]>
Signed-off-by: Emmanuel Grumbach <[email protected]>
---
net/mac80211/ieee80211_i.h | 4 ++++
net/mac80211/tdls.c | 38 ++++++++++++++++++++++++++++++++------
net/mac80211/vht.c | 30 +++++++++++++++++++++++++-----
3 files changed, 61 insertions(+), 11 deletions(-)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 1630975..a602a2a 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1719,6 +1719,10 @@ ieee80211_vht_cap_ie_to_sta_vht_cap(struct ieee80211_sub_if_data *sdata,
enum ieee80211_sta_rx_bandwidth ieee80211_sta_cap_rx_bw(struct sta_info *sta);
enum ieee80211_sta_rx_bandwidth ieee80211_sta_cur_vht_bw(struct sta_info *sta);
void ieee80211_sta_set_rx_nss(struct sta_info *sta);
+enum ieee80211_sta_rx_bandwidth
+ieee80211_chan_width_to_rx_bw(enum nl80211_chan_width width);
+enum nl80211_chan_width ieee80211_sta_cap_chan_bw(struct sta_info *sta);
+void ieee80211_sta_set_rx_nss(struct sta_info *sta);
void ieee80211_process_mu_groups(struct ieee80211_sub_if_data *sdata,
struct ieee80211_mgmt *mgmt);
u32 __ieee80211_vht_handle_opmode(struct ieee80211_sub_if_data *sdata,
diff --git a/net/mac80211/tdls.c b/net/mac80211/tdls.c
index 43f13abe..eed1152 100644
--- a/net/mac80211/tdls.c
+++ b/net/mac80211/tdls.c
@@ -4,7 +4,7 @@
* Copyright 2006-2010 Johannes Berg <[email protected]>
* Copyright 2014, Intel Corporation
* Copyright 2014 Intel Mobile Communications GmbH
- * Copyright 2015 Intel Deutschland GmbH
+ * Copyright 2015 - 2016 Intel Deutschland GmbH
*
* This file is GPLv2 as found in COPYING.
*/
@@ -15,6 +15,7 @@
#include <linux/rtnetlink.h>
#include "ieee80211_i.h"
#include "driver-ops.h"
+#include "rate.h"
/* give usermode some time for retries in setting up the TDLS session */
#define TDLS_PEER_SETUP_TIMEOUT (15 * HZ)
@@ -302,7 +303,7 @@ ieee80211_tdls_chandef_vht_upgrade(struct ieee80211_sub_if_data *sdata,
/* IEEE802.11ac-2013 Table E-4 */
u16 centers_80mhz[] = { 5210, 5290, 5530, 5610, 5690, 5775 };
struct cfg80211_chan_def uc = sta->tdls_chandef;
- enum nl80211_chan_width max_width = ieee80211_get_sta_bw(&sta->sta);
+ enum nl80211_chan_width max_width = ieee80211_sta_cap_chan_bw(sta);
int i;
/* only support upgrading non-narrow channels up to 80Mhz */
@@ -1242,18 +1243,44 @@ int ieee80211_tdls_mgmt(struct wiphy *wiphy, struct net_device *dev,
return ret;
}
-static void iee80211_tdls_recalc_chanctx(struct ieee80211_sub_if_data *sdata)
+static void iee80211_tdls_recalc_chanctx(struct ieee80211_sub_if_data *sdata,
+ struct sta_info *sta)
{
struct ieee80211_local *local = sdata->local;
struct ieee80211_chanctx_conf *conf;
struct ieee80211_chanctx *ctx;
+ enum nl80211_chan_width width;
+ struct ieee80211_supported_band *sband;
mutex_lock(&local->chanctx_mtx);
conf = rcu_dereference_protected(sdata->vif.chanctx_conf,
lockdep_is_held(&local->chanctx_mtx));
if (conf) {
+ width = conf->def.width;
+ sband = local->hw.wiphy->bands[conf->def.chan->band];
ctx = container_of(conf, struct ieee80211_chanctx, conf);
ieee80211_recalc_chanctx_chantype(local, ctx);
+
+ /* if width changed and a peer is given, update its BW */
+ if (width != conf->def.width && sta &&
+ test_sta_flag(sta, WLAN_STA_TDLS_WIDER_BW)) {
+ enum ieee80211_sta_rx_bandwidth bw;
+
+ bw = ieee80211_chan_width_to_rx_bw(conf->def.width);
+ bw = min(bw, ieee80211_sta_cap_rx_bw(sta));
+ if (bw != sta->sta.bandwidth) {
+ sta->sta.bandwidth = bw;
+ rate_control_rate_update(local, sband, sta,
+ IEEE80211_RC_BW_CHANGED);
+ /*
+ * if a TDLS peer BW was updated, we need to
+ * recalc the chandef width again, to get the
+ * correct chanctx min_def
+ */
+ ieee80211_recalc_chanctx_chantype(local, ctx);
+ }
+ }
+
}
mutex_unlock(&local->chanctx_mtx);
}
@@ -1350,8 +1377,6 @@ int ieee80211_tdls_oper(struct wiphy *wiphy, struct net_device *dev,
break;
}
- iee80211_tdls_recalc_chanctx(sdata);
-
mutex_lock(&local->sta_mtx);
sta = sta_info_get(sdata, peer);
if (!sta) {
@@ -1360,6 +1385,7 @@ int ieee80211_tdls_oper(struct wiphy *wiphy, struct net_device *dev,
break;
}
+ iee80211_tdls_recalc_chanctx(sdata, sta);
iee80211_tdls_recalc_ht_protection(sdata, sta);
set_sta_flag(sta, WLAN_STA_TDLS_PEER_AUTH);
@@ -1390,7 +1416,7 @@ int ieee80211_tdls_oper(struct wiphy *wiphy, struct net_device *dev,
iee80211_tdls_recalc_ht_protection(sdata, NULL);
mutex_unlock(&local->sta_mtx);
- iee80211_tdls_recalc_chanctx(sdata);
+ iee80211_tdls_recalc_chanctx(sdata, NULL);
break;
default:
ret = -ENOTSUPP;
diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
index 89e04d5..e590e2e 100644
--- a/net/mac80211/vht.c
+++ b/net/mac80211/vht.c
@@ -319,7 +319,30 @@ enum ieee80211_sta_rx_bandwidth ieee80211_sta_cap_rx_bw(struct sta_info *sta)
return IEEE80211_STA_RX_BW_80;
}
-static enum ieee80211_sta_rx_bandwidth
+enum nl80211_chan_width ieee80211_sta_cap_chan_bw(struct sta_info *sta)
+{
+ struct ieee80211_sta_vht_cap *vht_cap = &sta->sta.vht_cap;
+ u32 cap_width;
+
+ if (!vht_cap->vht_supported) {
+ if (!sta->sta.ht_cap.ht_supported)
+ return NL80211_CHAN_WIDTH_20_NOHT;
+
+ return sta->sta.ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40 ?
+ NL80211_CHAN_WIDTH_40 : NL80211_CHAN_WIDTH_20;
+ }
+
+ cap_width = vht_cap->cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK;
+
+ if (cap_width == IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ)
+ return NL80211_CHAN_WIDTH_160;
+ else if (cap_width == IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ)
+ return NL80211_CHAN_WIDTH_80P80;
+
+ return NL80211_CHAN_WIDTH_80;
+}
+
+enum ieee80211_sta_rx_bandwidth
ieee80211_chan_width_to_rx_bw(enum nl80211_chan_width width)
{
switch (width) {
@@ -347,10 +370,7 @@ enum ieee80211_sta_rx_bandwidth ieee80211_sta_cur_vht_bw(struct sta_info *sta)
bw = ieee80211_sta_cap_rx_bw(sta);
bw = min(bw, sta->cur_max_bandwidth);
-
- /* do not cap the BW of TDLS WIDER_BW peers by the bss */
- if (!test_sta_flag(sta, WLAN_STA_TDLS_WIDER_BW))
- bw = min(bw, ieee80211_chan_width_to_rx_bw(bss_width));
+ bw = min(bw, ieee80211_chan_width_to_rx_bw(bss_width));
return bw;
}
--
2.5.0
On Wed, Mar 02, 2016 at 11:28:31PM +0200, Emmanuel Grumbach wrote:
> Even if the current chandef width is equal to the station's max-BW, it
> doesn't mean it's a valid width for TDLS. Make sure to always check
> regulatory constraints in these cases.
I'm not sure this change is the trigger for this issue, but since I
noticed this for the first time today and this commit went just in into
wireless-testing.git, it sounds quite likely that this was indeed behind
the busy loop I saw here:
> diff --git a/net/mac80211/tdls.c b/net/mac80211/tdls.c
> @@ -332,7 +332,7 @@ ieee80211_tdls_chandef_vht_upgrade(struct ieee80211_sub_if_data *sdata,
> /* proceed to downgrade the chandef until usable or the same */
> - while (uc.width > max_width &&
> + while (uc.width > max_width ||
> !cfg80211_reg_can_beacon_relax(sdata->local->hw.wiphy, &uc,
> sdata->wdev.iftype))
> ieee80211_chandef_downgrade(&uc);
I'm not sure what caused the chandef in uc (sta->tdls_chandef) to be
invalid (actually, I do know now; see below), but when running the
ap_open_tdls_vht80plus80 test case, the VM got stuck in a busy loop
printing warnings about that chandef being invalid and with this while
loop being here, that never stopped.. Well, until couple of hours later
when I noticed this and stopped in manually. That left 1.6 GB of kernel
log entries (and that would have been way more had printk not refused to
print so much, but even the "589 printk messages dropped" prints were
enough to make this huge)..
So if uc is invalid here, it looks like this loop can get into a state
where it never terminates. The iteration hits these warnings:
WARNING: CPU: 2 PID: 623 at net/wireless/chan.c:336 cfg80211_chandef_dfs_required+0xf6/0x100()
WARNING: CPU: 2 PID: 623 at net/wireless/chan.c:608 cfg80211_chandef_usable+0x188/0x190()
WARNING: CPU: 2 PID: 623 at net/mac80211/util.c:2860 ieee80211_chandef_downgrade+0x138/0x170()
The last entry here seems to imply that c->width downgrade happened once
successfully since no other WARN_ON_ONCE() were printed within
ieee80211_chandef_downgrade().
This is followed by:
WARNING: CPU: 2 PID: 623 at net/wireless/chan.c:336 cfg80211_chandef_dfs_required+0xf6/0x100()
WARNING: CPU: 2 PID: 623 at net/wireless/chan.c:608 cfg80211_chandef_usable+0x188/0x190()
WARNING: CPU: 2 PID: 623 at net/wireless/chan.c:336 cfg80211_chandef_dfs_required+0xf6/0x100()
WARNING: CPU: 2 PID: 623 at net/wireless/chan.c:608 cfg80211_chandef_usable+0x188/0x190()
WARNING: CPU: 2 PID: 623 at net/wireless/chan.c:336 cfg80211_chandef_dfs_required+0xf6/0x100()
WARNING: CPU: 2 PID: 623 at net/wireless/chan.c:608 cfg80211_chandef_usable+0x188/0x190()
WARNING: CPU: 2 PID: 623 at net/mac80211/util.c:2848 ieee80211_chandef_downgrade+0x39/0x170()
The last one here is hitting the WARN_ON_ONCE(1) in the default case, so
it looks like there were two more successful downgrades (total three
starting from 80+80) and then we are in a busy loop with every new
iteration hitting that default case warning at util.c:2848. This
continues indefinitely.
Regardless of what caused the chandef to be invalid, this while loop
would benefit of some added robustness to avoid the possibility of an
infinite loop where the channel width cannot be downgraded anymore.
The console log from that run is available here:
http://w1.fi/a/tdls-downgrade-warning-loop.txt
It looks like I can now reproduce this easily with
./vm-run.sh ap_open_tdls_vht80plus80
Reverting this one-liner patch removes that loop and all of the chandef
invalid warnings.
With this patch applied, ieee80211_tdls_chandef_vht_upgrade() shows
uc.width == 4, max_width == 4, and chandef valid at the beginning of the
function. Just before the while loop, uc.width == 3, max_width == 3,
chandef is now invalid. On loop iterations, uc.width is dropped to 2, 1,
and finally 0 where it remains while the loop continues running.
The chandef becomes invalid when going through the centers_80mhz loop
and setting uc.center_freq1 = 5210, uc.width = NL80211_CHAN_WIDTH_80.
The AP was configured with seg0_idx 42 + seg1_idx 155.
--
Jouni Malinen PGP id EFC895FA
All three applied, but I had to fix your Fixes tag commit ID, no idea
what that referred to :)
johannes
On Sun, Mar 6, 2016 at 5:58 PM, Jouni Malinen <[email protected]> wrote:
>
> On Wed, Mar 02, 2016 at 11:28:31PM +0200, Emmanuel Grumbach wrote:
> > Even if the current chandef width is equal to the station's max-BW, it
> > doesn't mean it's a valid width for TDLS. Make sure to always check
> > regulatory constraints in these cases.
>
> I'm not sure this change is the trigger for this issue, but since I
> noticed this for the first time today and this commit went just in into
> wireless-testing.git, it sounds quite likely that this was indeed behind
> the busy loop I saw here:
>
> > diff --git a/net/mac80211/tdls.c b/net/mac80211/tdls.c
> > @@ -332,7 +332,7 @@ ieee80211_tdls_chandef_vht_upgrade(struct ieee80211_sub_if_data *sdata,
> > /* proceed to downgrade the chandef until usable or the same */
> > - while (uc.width > max_width &&
> > + while (uc.width > max_width ||
> > !cfg80211_reg_can_beacon_relax(sdata->local->hw.wiphy, &uc,
> > sdata->wdev.iftype))
> > ieee80211_chandef_downgrade(&uc);
Good catch :)
We actually just noticed this as well and have a suggested fix already
- basically the code was trying to *upgrade* a 80p80 channel to a 80
one.
I guess it will be out in a couple days after some internal testing.
Arik
On Thu, Mar 3, 2016 at 5:40 PM, Johannes Berg <[email protected]> wrote:
> All three applied, but I had to fix your Fixes tag commit ID, no idea
> what that referred to :)
Yea I might have taken it from some internal tree :)
Arik
From: Arik Nemtsov <[email protected]>
The min_def chanctx is affected not only by the current chandef, but
sometimes also by other stations on the vif. There's a valid scenario
where a TDLS peer can widen its BW, thereby causing the min_def
to increase.
Signed-off-by: Arik Nemtsov <[email protected]>
Signed-off-by: Emmanuel Grumbach <[email protected]>
---
net/mac80211/chan.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index 2839811..74142d0 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -343,8 +343,10 @@ static void ieee80211_change_chanctx(struct ieee80211_local *local,
struct ieee80211_chanctx *ctx,
const struct cfg80211_chan_def *chandef)
{
- if (cfg80211_chandef_identical(&ctx->conf.def, chandef))
+ if (cfg80211_chandef_identical(&ctx->conf.def, chandef)) {
+ ieee80211_recalc_chanctx_min_def(local, ctx);
return;
+ }
WARN_ON(!cfg80211_chandef_compatible(&ctx->conf.def, chandef));
--
2.5.0
On Thu, 2016-03-03 at 17:41 +0200, Arik Nemtsov wrote:
> On Thu, Mar 3, 2016 at 5:40 PM, Johannes Berg <johannes@sipsolutions.
> net> wrote:
> > All three applied, but I had to fix your Fixes tag commit ID, no
> > idea
> > what that referred to :)
>
> Yea I might have taken it from some internal tree :)
>
Oh, actually what prompted me to look at all was that it was too short
- should be 12 hex digits.
johannes