On 10/11/2017 01:02 AM, Johannes Berg wrote:
Hi,
#iw dev vap206 set bitrates legacy-5 ht-mcs-5 0 vht-mcs-5
command failed: Invalid argument (-22)
But, it actually *does* successfully set the rate in the driver
first, which is confusing at best.
Huh?
The call to set the rate in the driver comes before the error
check.
if (ieee80211_hw_check(&local->hw, HAS_RATE_CONTROL)) {
ret = drv_set_bitrate_mask(local, sdata, mask);
if (ret) {
pr_err("%s: drv-set-bitrate-mask had error return: %d\n",
sdata->dev->name, ret);
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)) {
#### I changed this code so I could set a single rate... --Ben
pr_err("%s: WARNING: no legacy rates for band[%d] in set-bitrate-mask.\n",
sdata->dev->name, band);
}
}
So, I think we should relax this check, at least for ath10k.
Well, yes and no. I don't think we should make ath10k special here, and
this fixes a real problem - namely that you can set up the system so
that you have no usable rates at all, and then you just get a WARN_ON
and start using the lowest possible rate...
Well, there are a million ways to set up a broken system, and setting a single
rate has a useful purpose, especially with ath10k since it has such limited
rate-setting capabilities.
There is basically never going to be a case where setting a single tx-rate on an
AP is a good idea in a general production environment, so maybe a possible WARN-ON is fine?
If you *do* set up an AP with a limited rateset, then it should simply fail to
allow a station to connect if it does not have any matching rates. This might go
back to a previous idea I had (and patches I posted and carry in my tree) to allow setting a different
advertise rateset vs usable tx-rateset.
For existing stations that might not match the new fixed rate, we could purposefully kick
them off I guess, but seems like a lot of work for a case that seems pretty irrelevant.
commit e8e4f5280ddd0a7b43a795f90a0758e3c99df6a6
Author: Johannes Berg <johannes.berg@xxxxxxxxx>
Date: Wed Mar 8 11:12:10 2017 +0100
mac80211: reject/clear user rate mask if not usable
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.
I guess you could implement this part? I.e. iterating the clients and
checking that they all support the rate that is set. However, then you
also need to implement that this gets reset when a new client that
doesn't support this rate connects.
Overall, this isn't very well defined for AP mode...
Perhaps it'd be better - as you pointed out in the other thread - to
have API to force a rate per station? We already have that for iwlwifi
in debugfs, so perhaps that'd be something to consider for this too,
I'm not sure there would be a real need to have it in nl80211?
I looked closely at the ath10k firmware yesterday, and it has no way to set a specific
single rate per peer. Sure, I could hack something into my CT firmware, but that
still leaves all the stock driver/firmware people out of luck, and my patches
won't make it upstream in the main kernel...
Thanks,
Ben
--
Ben Greear <greearb@xxxxxxxxxxxxxxx>
Candela Technologies Inc http://www.candelatech.com
