* [PATCH 0/2] mac80211: Turn AQL into a proper NL80211_EXT_FEATURE flag @ 2019-12-11 14:52 Toke Høiland-Jørgensen 2019-12-11 14:52 ` [PATCH 1/2] mac80211: Turn AQL into an NL80211_EXT_FEATURE Toke Høiland-Jørgensen 2019-12-11 14:52 ` [PATCH 2/2] ath10k: Enable Airtime Queue Limits feature Toke Høiland-Jørgensen 0 siblings, 2 replies; 7+ messages in thread From: Toke Høiland-Jørgensen @ 2019-12-11 14:52 UTC (permalink / raw) To: linux-wireless; +Cc: Toke Høiland-Jørgensen It turns out that there are some issues with just enabling AQL for all drivers; so instead, this series turns it into an NL80211_EXT_FEATURE flag, and only enables it for ath10k. Some coordination to make sure the mac80211 and the driver patch goes into the same -rc would be good. Toke Høiland-Jørgensen (2): mac80211: Turn AQL into an NL80211_EXT_FEATURE ath10k: Enable Airtime Queue Limits feature drivers/net/wireless/ath/ath10k/mac.c | 1 + include/uapi/linux/nl80211.h | 1 + net/mac80211/debugfs_sta.c | 76 ++++++++++++++++++++------- net/mac80211/main.c | 4 +- net/mac80211/sta_info.c | 3 ++ net/mac80211/sta_info.h | 1 - net/mac80211/tx.c | 4 +- 7 files changed, 66 insertions(+), 24 deletions(-) -- 2.24.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] mac80211: Turn AQL into an NL80211_EXT_FEATURE 2019-12-11 14:52 [PATCH 0/2] mac80211: Turn AQL into a proper NL80211_EXT_FEATURE flag Toke Høiland-Jørgensen @ 2019-12-11 14:52 ` Toke Høiland-Jørgensen 2019-12-11 21:21 ` Johannes Berg 2019-12-11 14:52 ` [PATCH 2/2] ath10k: Enable Airtime Queue Limits feature Toke Høiland-Jørgensen 1 sibling, 1 reply; 7+ messages in thread From: Toke Høiland-Jørgensen @ 2019-12-11 14:52 UTC (permalink / raw) To: linux-wireless; +Cc: Toke Høiland-Jørgensen Instead of just having an airtime flag in debugfs, turn AQL into a proper NL80211_EXT_FEATURE, so drivers can turn it on when they are ready, and so we also expose the presence of the feature to userspace. This also has the effect of flipping the default, so drivers have to opt in to using AQL instead of getting it by default with TXQs. A follow-up patch sets the feature for ath10k. While we're at it, split out the debugfs interface so AQL gets its own per-station debugfs file instead of using the 'airtime' file. Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> --- include/uapi/linux/nl80211.h | 1 + net/mac80211/debugfs_sta.c | 76 +++++++++++++++++++++++++++--------- net/mac80211/main.c | 4 +- net/mac80211/sta_info.c | 3 ++ net/mac80211/sta_info.h | 1 - net/mac80211/tx.c | 4 +- 6 files changed, 65 insertions(+), 24 deletions(-) diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h index 341e0e8cae46..1e6f435c709c 100644 --- a/include/uapi/linux/nl80211.h +++ b/include/uapi/linux/nl80211.h @@ -5563,6 +5563,7 @@ enum nl80211_ext_feature_index { NL80211_EXT_FEATURE_STA_TX_PWR, NL80211_EXT_FEATURE_SAE_OFFLOAD, NL80211_EXT_FEATURE_VLAN_OFFLOAD, + NL80211_EXT_FEATURE_AQL, /* add new features before the definition below */ NUM_NL80211_EXT_FEATURES, diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c index b3c9001d1f43..c80b1e163ea4 100644 --- a/net/mac80211/debugfs_sta.c +++ b/net/mac80211/debugfs_sta.c @@ -201,8 +201,6 @@ static ssize_t sta_airtime_read(struct file *file, char __user *userbuf, char *buf = kzalloc(bufsz, GFP_KERNEL), *p = buf; u64 rx_airtime = 0, tx_airtime = 0; s64 deficit[IEEE80211_NUM_ACS]; - u32 q_depth[IEEE80211_NUM_ACS]; - u32 q_limit_l[IEEE80211_NUM_ACS], q_limit_h[IEEE80211_NUM_ACS]; ssize_t rv; int ac; @@ -214,6 +212,56 @@ static ssize_t sta_airtime_read(struct file *file, char __user *userbuf, rx_airtime += sta->airtime[ac].rx_airtime; tx_airtime += sta->airtime[ac].tx_airtime; deficit[ac] = sta->airtime[ac].deficit; + spin_unlock_bh(&local->active_txq_lock[ac]); + } + + p += scnprintf(p, bufsz + buf - p, + "RX: %llu us\nTX: %llu us\nWeight: %u\n" + "Deficit: VO: %lld us VI: %lld us BE: %lld us BK: %lld us\n", + rx_airtime, tx_airtime, sta->airtime_weight, + deficit[0], deficit[1], deficit[2], deficit[3]); + + rv = simple_read_from_buffer(userbuf, count, ppos, buf, p - buf); + kfree(buf); + return rv; +} + +static ssize_t sta_airtime_write(struct file *file, const char __user *userbuf, + size_t count, loff_t *ppos) +{ + struct sta_info *sta = file->private_data; + struct ieee80211_local *local = sta->sdata->local; + int ac; + + for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) { + spin_lock_bh(&local->active_txq_lock[ac]); + sta->airtime[ac].rx_airtime = 0; + sta->airtime[ac].tx_airtime = 0; + sta->airtime[ac].deficit = sta->airtime_weight; + spin_unlock_bh(&local->active_txq_lock[ac]); + } + + return count; +} +STA_OPS_RW(airtime); + +static ssize_t sta_aql_read(struct file *file, char __user *userbuf, + size_t count, loff_t *ppos) +{ + struct sta_info *sta = file->private_data; + struct ieee80211_local *local = sta->sdata->local; + size_t bufsz = 400; + char *buf = kzalloc(bufsz, GFP_KERNEL), *p = buf; + u32 q_depth[IEEE80211_NUM_ACS]; + u32 q_limit_l[IEEE80211_NUM_ACS], q_limit_h[IEEE80211_NUM_ACS]; + ssize_t rv; + int ac; + + if (!buf) + return -ENOMEM; + + for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) { + spin_lock_bh(&local->active_txq_lock[ac]); q_limit_l[ac] = sta->airtime[ac].aql_limit_low; q_limit_h[ac] = sta->airtime[ac].aql_limit_high; spin_unlock_bh(&local->active_txq_lock[ac]); @@ -221,12 +269,8 @@ static ssize_t sta_airtime_read(struct file *file, char __user *userbuf, } p += scnprintf(p, bufsz + buf - p, - "RX: %llu us\nTX: %llu us\nWeight: %u\n" - "Deficit: VO: %lld us VI: %lld us BE: %lld us BK: %lld us\n" "Q depth: VO: %u us VI: %u us BE: %u us BK: %u us\n" "Q limit[low/high]: VO: %u/%u VI: %u/%u BE: %u/%u BK: %u/%u\n", - rx_airtime, tx_airtime, sta->airtime_weight, - deficit[0], deficit[1], deficit[2], deficit[3], q_depth[0], q_depth[1], q_depth[2], q_depth[3], q_limit_l[0], q_limit_h[0], q_limit_l[1], q_limit_h[1], q_limit_l[2], q_limit_h[2], q_limit_l[3], q_limit_h[3]), @@ -236,11 +280,10 @@ static ssize_t sta_airtime_read(struct file *file, char __user *userbuf, return rv; } -static ssize_t sta_airtime_write(struct file *file, const char __user *userbuf, +static ssize_t sta_aql_write(struct file *file, const char __user *userbuf, size_t count, loff_t *ppos) { struct sta_info *sta = file->private_data; - struct ieee80211_local *local = sta->sdata->local; u32 ac, q_limit_l, q_limit_h; char _buf[100] = {}, *buf = _buf; @@ -251,7 +294,7 @@ static ssize_t sta_airtime_write(struct file *file, const char __user *userbuf, return -EFAULT; buf[sizeof(_buf) - 1] = '\0'; - if (sscanf(buf, "queue limit %u %u %u", &ac, &q_limit_l, &q_limit_h) + if (sscanf(buf, "limit %u %u %u", &ac, &q_limit_l, &q_limit_h) != 3) return -EINVAL; @@ -261,17 +304,10 @@ static ssize_t sta_airtime_write(struct file *file, const char __user *userbuf, sta->airtime[ac].aql_limit_low = q_limit_l; sta->airtime[ac].aql_limit_high = q_limit_h; - for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) { - spin_lock_bh(&local->active_txq_lock[ac]); - sta->airtime[ac].rx_airtime = 0; - sta->airtime[ac].tx_airtime = 0; - sta->airtime[ac].deficit = sta->airtime_weight; - spin_unlock_bh(&local->active_txq_lock[ac]); - } - return count; } -STA_OPS_RW(airtime); +STA_OPS_RW(aql); + static ssize_t sta_agg_status_read(struct file *file, char __user *userbuf, size_t count, loff_t *ppos) @@ -996,6 +1032,10 @@ void ieee80211_sta_debugfs_add(struct sta_info *sta) NL80211_EXT_FEATURE_AIRTIME_FAIRNESS)) DEBUGFS_ADD(airtime); + if (wiphy_ext_feature_isset(local->hw.wiphy, + NL80211_EXT_FEATURE_AQL)) + DEBUGFS_ADD(aql); + debugfs_create_xul("driver_buffered_tids", 0400, sta->debugfs_dir, &sta->driver_buffered_tids); diff --git a/net/mac80211/main.c b/net/mac80211/main.c index 6cca0853f183..4c2b5ba3ac09 100644 --- a/net/mac80211/main.c +++ b/net/mac80211/main.c @@ -672,9 +672,7 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len, IEEE80211_DEFAULT_AQL_TXQ_LIMIT_H; } - local->airtime_flags = AIRTIME_USE_TX | - AIRTIME_USE_RX | - AIRTIME_USE_AQL; + local->airtime_flags = AIRTIME_USE_TX | AIRTIME_USE_RX; local->aql_threshold = IEEE80211_AQL_THRESHOLD; atomic_set(&local->aql_total_pending_airtime, 0); diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c index 8eafd81e97b4..0f5f40678885 100644 --- a/net/mac80211/sta_info.c +++ b/net/mac80211/sta_info.c @@ -1916,6 +1916,9 @@ void ieee80211_sta_update_pending_airtime(struct ieee80211_local *local, { int tx_pending; + if (!wiphy_ext_feature_isset(local->hw.wiphy, NL80211_EXT_FEATURE_AQL)) + return; + if (!tx_completed) { if (sta) atomic_add(tx_airtime, diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h index ad5d8a4ae56d..c00e28585f9d 100644 --- a/net/mac80211/sta_info.h +++ b/net/mac80211/sta_info.h @@ -127,7 +127,6 @@ enum ieee80211_agg_stop_reason { /* Debugfs flags to enable/disable use of RX/TX airtime in scheduler */ #define AIRTIME_USE_TX BIT(0) #define AIRTIME_USE_RX BIT(1) -#define AIRTIME_USE_AQL BIT(2) struct airtime_info { u64 rx_airtime; diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index b696b9136f4c..259989a81108 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -3668,7 +3668,7 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw, IEEE80211_SKB_CB(skb)->control.vif = vif; - if (local->airtime_flags & AIRTIME_USE_AQL) { + if (wiphy_ext_feature_isset(local->hw.wiphy, NL80211_EXT_FEATURE_AQL)) { u32 airtime; airtime = ieee80211_calc_expected_tx_airtime(hw, vif, txq->sta, @@ -3790,7 +3790,7 @@ bool ieee80211_txq_airtime_check(struct ieee80211_hw *hw, struct sta_info *sta; struct ieee80211_local *local = hw_to_local(hw); - if (!(local->airtime_flags & AIRTIME_USE_AQL)) + if (!wiphy_ext_feature_isset(local->hw.wiphy, NL80211_EXT_FEATURE_AQL)) return true; if (!txq->sta) -- 2.24.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] mac80211: Turn AQL into an NL80211_EXT_FEATURE 2019-12-11 14:52 ` [PATCH 1/2] mac80211: Turn AQL into an NL80211_EXT_FEATURE Toke Høiland-Jørgensen @ 2019-12-11 21:21 ` Johannes Berg 2019-12-12 6:05 ` Justin Capella 2019-12-12 10:26 ` Toke Høiland-Jørgensen 0 siblings, 2 replies; 7+ messages in thread From: Johannes Berg @ 2019-12-11 21:21 UTC (permalink / raw) To: Toke Høiland-Jørgensen, linux-wireless On Wed, 2019-12-11 at 15:52 +0100, Toke Høiland-Jørgensen wrote: > > diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h > index 341e0e8cae46..1e6f435c709c 100644 > --- a/include/uapi/linux/nl80211.h > +++ b/include/uapi/linux/nl80211.h > @@ -5563,6 +5563,7 @@ enum nl80211_ext_feature_index { > NL80211_EXT_FEATURE_STA_TX_PWR, > NL80211_EXT_FEATURE_SAE_OFFLOAD, > NL80211_EXT_FEATURE_VLAN_OFFLOAD, > + NL80211_EXT_FEATURE_AQL, This is missing kernel-doc. johannes ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] mac80211: Turn AQL into an NL80211_EXT_FEATURE 2019-12-11 21:21 ` Johannes Berg @ 2019-12-12 6:05 ` Justin Capella 2019-12-12 10:29 ` Toke Høiland-Jørgensen 2019-12-12 10:26 ` Toke Høiland-Jørgensen 1 sibling, 1 reply; 7+ messages in thread From: Justin Capella @ 2019-12-12 6:05 UTC (permalink / raw) To: Johannes Berg; +Cc: Toke Høiland-Jørgensen, linux-wireless Disclaimer: I'm new, so don't assume my questions are rhetorical... sta = container_of(txq->sta, struct sta_info, sta); // Is this correct? I see the use of IEEE80211_NUM_ACS seems to be standard, is that AC specific? I learned today that different devs have different numbers of queues... What are the units of the THRESHOLD, iirc there was some bit shifting/masking? Is there supposed to be a queue per station, or just per interface? It seemed like the threshold was meant to pick a higher or lower queue, this seems to maybe just reject if not within the bounds? Sorry for all the questions, I'm still on the gradual side of the learning curve On Wed, Dec 11, 2019 at 1:22 PM Johannes Berg <johannes@sipsolutions.net> wrote: > > On Wed, 2019-12-11 at 15:52 +0100, Toke Høiland-Jørgensen wrote: > > > > diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h > > index 341e0e8cae46..1e6f435c709c 100644 > > --- a/include/uapi/linux/nl80211.h > > +++ b/include/uapi/linux/nl80211.h > > @@ -5563,6 +5563,7 @@ enum nl80211_ext_feature_index { > > NL80211_EXT_FEATURE_STA_TX_PWR, > > NL80211_EXT_FEATURE_SAE_OFFLOAD, > > NL80211_EXT_FEATURE_VLAN_OFFLOAD, > > + NL80211_EXT_FEATURE_AQL, > > This is missing kernel-doc. > > johannes > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] mac80211: Turn AQL into an NL80211_EXT_FEATURE 2019-12-12 6:05 ` Justin Capella @ 2019-12-12 10:29 ` Toke Høiland-Jørgensen 0 siblings, 0 replies; 7+ messages in thread From: Toke Høiland-Jørgensen @ 2019-12-12 10:29 UTC (permalink / raw) To: Justin Capella, Johannes Berg; +Cc: linux-wireless Justin Capella <justincapella@gmail.com> writes: > Disclaimer: I'm new, so don't assume my questions are rhetorical... > > sta = container_of(txq->sta, struct sta_info, sta); // Is this > correct? Not sure what you're referring to here; but generally, yes? :) > I see the use of IEEE80211_NUM_ACS seems to be standard, is that AC > specific? I learned today that different devs have different numbers > of queues... AC numbers are defined in the standard, and there are always four of them. Different hardware can have different numbers of queues, which may or may not be related to NUM_ACS. > What are the units of the THRESHOLD, iirc there was some bit > shifting/masking? Units are logically microseconds, but stored as increments of 4 microseconds because we ran out of bits; this is kept inside the setter/getter function, though, so everywhere else is just microseconds. > Is there supposed to be a queue per station, or just per interface? There's a TXQ per TID (so 16 per station), and an additional one on the interface for multicast. > It seemed like the threshold was meant to pick a higher or lower > queue, this seems to maybe just reject if not within the bounds? There are two thresholds; one for the whole interface, and one per station. If either is exceeded transmission will be throttled. -Toke ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] mac80211: Turn AQL into an NL80211_EXT_FEATURE 2019-12-11 21:21 ` Johannes Berg 2019-12-12 6:05 ` Justin Capella @ 2019-12-12 10:26 ` Toke Høiland-Jørgensen 1 sibling, 0 replies; 7+ messages in thread From: Toke Høiland-Jørgensen @ 2019-12-12 10:26 UTC (permalink / raw) To: Johannes Berg, linux-wireless Johannes Berg <johannes@sipsolutions.net> writes: > On Wed, 2019-12-11 at 15:52 +0100, Toke Høiland-Jørgensen wrote: >> >> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h >> index 341e0e8cae46..1e6f435c709c 100644 >> --- a/include/uapi/linux/nl80211.h >> +++ b/include/uapi/linux/nl80211.h >> @@ -5563,6 +5563,7 @@ enum nl80211_ext_feature_index { >> NL80211_EXT_FEATURE_STA_TX_PWR, >> NL80211_EXT_FEATURE_SAE_OFFLOAD, >> NL80211_EXT_FEATURE_VLAN_OFFLOAD, >> + NL80211_EXT_FEATURE_AQL, > > This is missing kernel-doc. Pfft; since when are we also supposed to *document* our code? ;) Will send a v2... -Toke ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] ath10k: Enable Airtime Queue Limits feature 2019-12-11 14:52 [PATCH 0/2] mac80211: Turn AQL into a proper NL80211_EXT_FEATURE flag Toke Høiland-Jørgensen 2019-12-11 14:52 ` [PATCH 1/2] mac80211: Turn AQL into an NL80211_EXT_FEATURE Toke Høiland-Jørgensen @ 2019-12-11 14:52 ` Toke Høiland-Jørgensen 1 sibling, 0 replies; 7+ messages in thread From: Toke Høiland-Jørgensen @ 2019-12-11 14:52 UTC (permalink / raw) To: linux-wireless; +Cc: Toke Høiland-Jørgensen Since AQL is now a feature flag drivers have to set for the feature to be enabled, set this for ath10k. Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> --- drivers/net/wireless/ath/ath10k/mac.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c index 83cc8778ca1e..978f0037ed52 100644 --- a/drivers/net/wireless/ath/ath10k/mac.c +++ b/drivers/net/wireless/ath/ath10k/mac.c @@ -8958,6 +8958,7 @@ int ath10k_mac_register(struct ath10k *ar) wiphy_ext_feature_set(ar->hw->wiphy, NL80211_EXT_FEATURE_VHT_IBSS); wiphy_ext_feature_set(ar->hw->wiphy, NL80211_EXT_FEATURE_SET_SCAN_DWELL); + wiphy_ext_feature_set(ar->hw->wiphy, NL80211_EXT_FEATURE_AQL); if (test_bit(WMI_SERVICE_TX_DATA_ACK_RSSI, ar->wmi.svc_map) || test_bit(WMI_SERVICE_HTT_MGMT_TX_COMP_VALID_FLAGS, ar->wmi.svc_map)) -- 2.24.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-12-12 10:30 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-11 14:52 [PATCH 0/2] mac80211: Turn AQL into a proper NL80211_EXT_FEATURE flag Toke Høiland-Jørgensen 2019-12-11 14:52 ` [PATCH 1/2] mac80211: Turn AQL into an NL80211_EXT_FEATURE Toke Høiland-Jørgensen 2019-12-11 21:21 ` Johannes Berg 2019-12-12 6:05 ` Justin Capella 2019-12-12 10:29 ` Toke Høiland-Jørgensen 2019-12-12 10:26 ` Toke Høiland-Jørgensen 2019-12-11 14:52 ` [PATCH 2/2] ath10k: Enable Airtime Queue Limits feature Toke Høiland-Jørgensen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).