Re: [PATCH v2] mac80211: reject/clear user rate mask if not usable
> > Yes but you have to iterate all the stations and check they belong to > > the interface and all that I think? > > > Maybe not need. > if NL80211_IFTYPE_STATION and NON-WLAN_STA_TDLS_PEER, it has only 1 > station for the ieee80211_vif > by my understand. Well, yes, but there's no counter. So you need to iterate to see if there are any additional TDLS peers (in addition to the STA entry for the AP). johannes ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v2] mac80211: reject/clear user rate mask if not usable
On 2020-11-13 17:10, Johannes Berg wrote: On Fri, 2020-11-13 at 17:09 +0800, Wen Gong wrote: On 2020-11-13 16:51, Johannes Berg wrote: > On Fri, 2020-11-13 at 16:51 +0800, Wen Gong wrote: > > > yes. > > It can add check with supp_rates[band] of ieee80211_sta for > > NL80211_IFTYPE_STATION type. > > for others, check with sdata->vif.bss_conf.basic_rates > > Right. > > Though, might need to check that only if there's no TDLS station or > something? > yes, I think it should do like that for TDLS: test_sta_flag(sta, WLAN_STA_TDLS_PEER) Yes but you have to iterate all the stations and check they belong to the interface and all that I think? Maybe not need. if NL80211_IFTYPE_STATION and NON-WLAN_STA_TDLS_PEER, it has only 1 station for the ieee80211_vif by my understand. johannes ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v2] mac80211: reject/clear user rate mask if not usable
On Fri, 2020-11-13 at 17:09 +0800, Wen Gong wrote: > On 2020-11-13 16:51, Johannes Berg wrote: > > On Fri, 2020-11-13 at 16:51 +0800, Wen Gong wrote: > > > > > yes. > > > It can add check with supp_rates[band] of ieee80211_sta for > > > NL80211_IFTYPE_STATION type. > > > for others, check with sdata->vif.bss_conf.basic_rates > > > > Right. > > > > Though, might need to check that only if there's no TDLS station or > > something? > > > yes, I think it should do like that for TDLS: > test_sta_flag(sta, WLAN_STA_TDLS_PEER) Yes but you have to iterate all the stations and check they belong to the interface and all that I think? johannes ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v2] mac80211: reject/clear user rate mask if not usable
On 2020-11-13 16:51, Johannes Berg wrote: On Fri, 2020-11-13 at 16:51 +0800, Wen Gong wrote: yes. It can add check with supp_rates[band] of ieee80211_sta for NL80211_IFTYPE_STATION type. for others, check with sdata->vif.bss_conf.basic_rates Right. Though, might need to check that only if there's no TDLS station or something? yes, I think it should do like that for TDLS: test_sta_flag(sta, WLAN_STA_TDLS_PEER) johannes ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v2] mac80211: reject/clear user rate mask if not usable
On 2020-11-13 16:35, Johannes Berg wrote: On Fri, 2020-11-13 at 16:35 +0800, Wen Gong wrote: > I guess if we really want to redefine the user rate mask to not apply > to > control frames, then we can relax this? > Yes, for AP mode, it is hard to calculate the usable rates over all stations. But for STATION mode, it can set 54M because AP support it, so it should not reject it. If add a check for nl80211_iftype of ieee80211_vif in ieee80211_set_bitrate_mask, it can solve this like this: if (sdata->vif.type != NL80211_IFTYPE_STATION && !(mask->control[band].legacy & basic_rates)) That would forgo the check completely - we'd still need to check against the *supported* rates. yes. It can add check with supp_rates[band] of ieee80211_sta for NL80211_IFTYPE_STATION type. for others, check with sdata->vif.bss_conf.basic_rates johannes ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v2] mac80211: reject/clear user rate mask if not usable
On Fri, 2020-11-13 at 16:51 +0800, Wen Gong wrote: > yes. > It can add check with supp_rates[band] of ieee80211_sta for > NL80211_IFTYPE_STATION type. > for others, check with sdata->vif.bss_conf.basic_rates Right. Though, might need to check that only if there's no TDLS station or something? johannes ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v2] mac80211: reject/clear user rate mask if not usable
On Fri, 2020-11-13 at 16:35 +0800, Wen Gong wrote: > > > I guess if we really want to redefine the user rate mask to not apply > > to > > control frames, then we can relax this? > > > Yes, for AP mode, it is hard to calculate the usable rates over all > stations. > But for STATION mode, it can set 54M because AP support it, so it should > not reject it. > If add a check for nl80211_iftype of ieee80211_vif in > ieee80211_set_bitrate_mask, it can > solve this like this: > if (sdata->vif.type != NL80211_IFTYPE_STATION && > !(mask->control[band].legacy & basic_rates)) That would forgo the check completely - we'd still need to check against the *supported* rates. johannes ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v2] mac80211: reject/clear user rate mask if not usable
On 2020-11-13 16:16, Johannes Berg wrote: On Fri, 2020-11-13 at 16:14 +0800, Wen Gong wrote: On 2020-11-13 15:38, Johannes Berg wrote: > On Fri, 2020-11-13 at 10:08 +0800, Wen Gong wrote: > > > Which was the intent of this change, wasn't it? > > Indeed. Permitting this leads to warnings later. > > > We need to set the tx rate to fixed at a single rate, e.g., > > 54M/48M/36M... for a test case. > > I do not want a clear error message, I want to the 54M rate pass/set > > success to lower wlan driver. > > Then lower wlan driver can handle it. > > No, that will not happen. You should configure your test AP to actually > support 54M. Yes, the AP support 54M, but it is not basic rate, so ieee80211_set_bitrate_mask will reject 54M because fail for check (mask->control[band].legacy & basic_rates). Ah. So this is what I said in the original commit message even: Technically, selecting basic rates as the criterion is a bit too restrictive, but calculating the usable rates over all stations (e.g. in AP mode) is harder, and all stations must support the basic rates. Similarly, in client mode, the basic rates will be used anyway for control frames. I guess if we really want to redefine the user rate mask to not apply to control frames, then we can relax this? Yes, for AP mode, it is hard to calculate the usable rates over all stations. But for STATION mode, it can set 54M because AP support it, so it should not reject it. If add a check for nl80211_iftype of ieee80211_vif in ieee80211_set_bitrate_mask, it can solve this like this: if (sdata->vif.type != NL80211_IFTYPE_STATION && !(mask->control[band].legacy & basic_rates)) johannes ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v2] mac80211: reject/clear user rate mask if not usable
On Fri, 2020-11-13 at 16:14 +0800, Wen Gong wrote: > On 2020-11-13 15:38, Johannes Berg wrote: > > On Fri, 2020-11-13 at 10:08 +0800, Wen Gong wrote: > > > > Which was the intent of this change, wasn't it? > > > > Indeed. Permitting this leads to warnings later. > > > > > We need to set the tx rate to fixed at a single rate, e.g., > > > 54M/48M/36M... for a test case. > > > I do not want a clear error message, I want to the 54M rate pass/set > > > success to lower wlan driver. > > > Then lower wlan driver can handle it. > > > > No, that will not happen. You should configure your test AP to actually > > support 54M. > Yes, the AP support 54M, but it is not basic rate, so > ieee80211_set_bitrate_mask will reject 54M > because fail for check (mask->control[band].legacy & basic_rates). Ah. So this is what I said in the original commit message even: > Technically, selecting basic rates as the criterion is a bit too > restrictive, but calculating the usable rates over all stations > (e.g. in AP mode) is harder, and all stations must support the > basic rates. Similarly, in client mode, the basic rates will be > used anyway for control frames. I guess if we really want to redefine the user rate mask to not apply to control frames, then we can relax this? johannes ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v2] mac80211: reject/clear user rate mask if not usable
On 2020-11-13 15:38, Johannes Berg wrote: On Fri, 2020-11-13 at 10:08 +0800, Wen Gong wrote: > Which was the intent of this change, wasn't it? Indeed. Permitting this leads to warnings later. We need to set the tx rate to fixed at a single rate, e.g., 54M/48M/36M... for a test case. I do not want a clear error message, I want to the 54M rate pass/set success to lower wlan driver. Then lower wlan driver can handle it. No, that will not happen. You should configure your test AP to actually support 54M. Yes, the AP support 54M, but it is not basic rate, so ieee80211_set_bitrate_mask will reject 54M because fail for check (mask->control[band].legacy & basic_rates). suppor rates of my AP: Tag: Supported Rates 6(B), 9, 12(B), 18, 24(B), 36, 48, 54, [Mbit/sec] Tag Number: Supported Rates (1) Tag length: 8 Supported Rates: 6(B) (0x8c) Supported Rates: 9 (0x12) Supported Rates: 12(B) (0x98) Supported Rates: 18 (0x24) Supported Rates: 24(B) (0xb0) Supported Rates: 36 (0x48) Supported Rates: 48 (0x60) Supported Rates: 54 (0x6c) johannes ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v2] mac80211: reject/clear user rate mask if not usable
On Fri, 2020-11-13 at 10:08 +0800, Wen Gong wrote: > > > Which was the intent of this change, wasn't it? Indeed. Permitting this leads to warnings later. > We need to set the tx rate to fixed at a single rate, e.g., > 54M/48M/36M... for a test case. > I do not want a clear error message, I want to the 54M rate pass/set > success to lower wlan driver. > Then lower wlan driver can handle it. No, that will not happen. You should configure your test AP to actually support 54M. johannes ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v2] mac80211: reject/clear user rate mask if not usable
On 2020-11-12 20:49, Arend Van Spriel wrote: On 11/12/2020 11:55 AM, Wen Gong wrote: On 2017-03-08 21:20, Johannes Berg wrote: From: Johannes Berg [...] @@ -2685,6 +2686,21 @@ static int ieee80211_set_bitrate_mask(struct wiphy *wiphy, return ret; } + /* + * If active validate the setting and reject it if it doesn't leave + * at least one basic rate usable, since we really have to be able + * to send something, and if we're an AP we have to be able to do + * so at a basic rate so that all clients can receive it. + */ + if (rcu_access_pointer(sdata->vif.chanctx_conf) && + sdata->vif.bss_conf.chandef.chan) { + u32 basic_rates = sdata->vif.bss_conf.basic_rates; + enum nl80211_band band = sdata->vif.bss_conf.chandef.chan->band; + + if (!(mask->control[band].legacy & basic_rates)) + return -EINVAL; If user want to use “iw wlan0 set bitrates legacy-5 54” to set it to fixed in 54M and 54M is not basic rate in AP's becaon as example of below, then the iw command will fail. Which was the intent of this change, wasn't it? You want to allow anyway or you want a clear error message as to why it fails? We need to set the tx rate to fixed at a single rate, e.g., 54M/48M/36M... for a test case. I do not want a clear error message, I want to the 54M rate pass/set success to lower wlan driver. Then lower wlan driver can handle it. Regards, Arend ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v2] mac80211: reject/clear user rate mask if not usable
On 11/12/2020 11:55 AM, Wen Gong wrote: On 2017-03-08 21:20, Johannes Berg wrote: From: Johannes Berg [...] @@ -2685,6 +2686,21 @@ static int ieee80211_set_bitrate_mask(struct wiphy *wiphy, return ret; } + /* + * If active validate the setting and reject it if it doesn't leave + * at least one basic rate usable, since we really have to be able + * to send something, and if we're an AP we have to be able to do + * so at a basic rate so that all clients can receive it. + */ + if (rcu_access_pointer(sdata->vif.chanctx_conf) && + sdata->vif.bss_conf.chandef.chan) { + u32 basic_rates = sdata->vif.bss_conf.basic_rates; + enum nl80211_band band = sdata->vif.bss_conf.chandef.chan->band; + + if (!(mask->control[band].legacy & basic_rates)) + return -EINVAL; If user want to use “iw wlan0 set bitrates legacy-5 54” to set it to fixed in 54M and 54M is not basic rate in AP's becaon as example of below, then the iw command will fail. Which was the intent of this change, wasn't it? You want to allow anyway or you want a clear error message as to why it fails? Regards, Arend smime.p7s Description: S/MIME Cryptographic Signature ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v2] mac80211: reject/clear user rate mask if not usable
On 2017-03-08 21:20, Johannes Berg wrote: From: Johannes Berg If the user rate mask results in no (basic) rates being usable, clear it. Also, if we're already operating when it's set, reject it instead. Technically, selecting basic rates as the criterion is a bit too restrictive, but calculating the usable rates over all stations (e.g. in AP mode) is harder, and all stations must support the basic rates. Similarly, in client mode, the basic rates will be used anyway for control frames. This fixes the "no supported rates (...) in rate_mask ..." warning that occurs on TX when you've selected a rate mask that's not compatible with the connection (e.g. an AP that enables only the rates 36, 48, 54 and you've selected only 6, 9, 12.) Reported-by: Kirtika Ruchandani Signed-off-by: Johannes Berg --- net/mac80211/cfg.c | 18 +- net/mac80211/mlme.c | 2 ++ net/mac80211/rate.c | 27 +++ net/mac80211/rate.h | 2 ++ 4 files changed, 48 insertions(+), 1 deletion(-) diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index 9c7490cb2243..8bc3d3669348 100644 --- a/net/mac80211/cfg.c +++ b/net/mac80211/cfg.c @@ -3,7 +3,7 @@ * * Copyright 2006-2010 Johannes Berg * Copyright 2013-2015 Intel Mobile Communications GmbH - * Copyright (C) 2015-2016 Intel Deutschland GmbH + * Copyright (C) 2015-2017 Intel Deutschland GmbH * * This file is GPLv2 as found in COPYING. */ @@ -2042,6 +2042,7 @@ static int ieee80211_change_bss(struct wiphy *wiphy, params->basic_rates_len, >vif.bss_conf.basic_rates); changed |= BSS_CHANGED_BASIC_RATES; + ieee80211_check_rate_mask(sdata); } if (params->ap_isolate >= 0) { @@ -2685,6 +2686,21 @@ static int ieee80211_set_bitrate_mask(struct wiphy *wiphy, return ret; } + /* +* If active validate the setting and reject it if it doesn't leave +* at least one basic rate usable, since we really have to be able +* to send something, and if we're an AP we have to be able to do +* so at a basic rate so that all clients can receive it. +*/ + if (rcu_access_pointer(sdata->vif.chanctx_conf) && + sdata->vif.bss_conf.chandef.chan) { + u32 basic_rates = sdata->vif.bss_conf.basic_rates; + enum nl80211_band band = sdata->vif.bss_conf.chandef.chan->band; + + if (!(mask->control[band].legacy & basic_rates)) + return -EINVAL; If user want to use “iw wlan0 set bitrates legacy-5 54” to set it to fixed in 54M and 54M is not basic rate in AP's becaon as example of below, then the iw command will fail. Tag: Supported Rates 6(B), 9, 12(B), 18, 24(B), 36, 48, 54, [Mbit/sec] Tag Number: Supported Rates (1) Tag length: 8 Supported Rates: 6(B) (0x8c) Supported Rates: 9 (0x12) Supported Rates: 12(B) (0x98) Supported Rates: 18 (0x24) Supported Rates: 24(B) (0xb0) Supported Rates: 36 (0x48) Supported Rates: 48 (0x60) Supported Rates: 54 (0x6c) + } + for (i = 0; i < NUM_NL80211_BANDS; i++) { struct ieee80211_supported_band *sband = wiphy->bands[i]; int j; ... ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k