Here is the consolidated series of mac80211, ath9k and ath10k changes
for moving TXQ scheduling and airtime fairness into mac80211 and support
airtime fairness.
Three main APIs are defined for TXQ scheduling.
- ieee80211_next_txq
- ieee80211_return_txq
- ieee80211_txq_may_transmit
and two APIs for synchronization when the driver wants to iterate
through all TXQs.
- ieee80211_txq_schedule_start
- ieee80211_txq_schedule_end
The earlier discussions:
https://patchwork.kernel.org/project/linux-wireless/list/?series=&submitter=&state=*&q=TXQ+scheduling&archive=both&delegate=
-Rajkumar
v2:
- replaced driver specific airtime default by multiplier
- fixed tab intent
- fixed author and commit log
v3:
- fixed checkpatch warnings and kbuild bot errors.
few of over 80 char warnings are left.
Kan Yan (1):
ath10k: reporting estimated tx airtime for fairness
Toke Høiland-Jørgensen (5):
mac80211: Add TXQ scheduling API
cfg80211: Add airtime statistics and settings
mac80211: Add airtime accounting and scheduling to TXQs
ath9k: Switch to mac80211 TXQ scheduling and airtime APIs
ath10k: migrate to mac80211 txq scheduling
drivers/net/wireless/ath/ath10k/core.c | 2 -
drivers/net/wireless/ath/ath10k/core.h | 8 +-
drivers/net/wireless/ath/ath10k/htc.h | 1 -
drivers/net/wireless/ath/ath10k/htt_rx.c | 9 ++
drivers/net/wireless/ath/ath10k/mac.c | 155 +++++++++++-------
drivers/net/wireless/ath/ath10k/txrx.c | 4 +
drivers/net/wireless/ath/ath9k/ath9k.h | 14 --
drivers/net/wireless/ath/ath9k/debug.c | 3 -
drivers/net/wireless/ath/ath9k/debug.h | 8 -
drivers/net/wireless/ath/ath9k/debug_sta.c | 70 ---------
drivers/net/wireless/ath/ath9k/init.c | 3 +-
drivers/net/wireless/ath/ath9k/recv.c | 9 +-
drivers/net/wireless/ath/ath9k/xmit.c | 244 +++++++++--------------------
include/net/cfg80211.h | 10 +-
include/net/mac80211.h | 121 +++++++++++++-
include/uapi/linux/nl80211.h | 15 ++
net/mac80211/agg-tx.c | 2 +-
net/mac80211/cfg.c | 3 +
net/mac80211/debugfs.c | 3 +
net/mac80211/debugfs_sta.c | 50 +++++-
net/mac80211/driver-ops.h | 9 ++
net/mac80211/ieee80211_i.h | 11 ++
net/mac80211/main.c | 9 ++
net/mac80211/sta_info.c | 46 +++++-
net/mac80211/sta_info.h | 13 ++
net/mac80211/status.c | 6 +
net/mac80211/tx.c | 140 ++++++++++++++++-
net/wireless/nl80211.c | 30 ++++
28 files changed, 644 insertions(+), 354 deletions(-)
--
1.9.1
From: Toke Høiland-Jørgensen <[email protected]>
This adds an API to mac80211 to handle scheduling of TXQs. The interface
between driver and mac80211 for TXQ handling is changed by adding two new
functions: ieee80211_next_txq(), which will return the next TXQ to schedule
in the current round-robin rotation, and ieee80211_return_txq(), which the
driver uses to indicate that it has finished scheduling a TXQ (which will
then be put back in the scheduling rotation if it isn't empty).
The driver must call ieee80211_txq_schedule_start() at the start of each
scheduling session, and ieee80211_txq_schedule_end() at the end. The API
then guarantees that the same TXQ is not returned twice in the same
session (so a driver can loop on ieee80211_next_txq() without worrying
about breaking the loop.
Usage of the new API is optional, so drivers can be ported one at a time.
In this patch, the actual scheduling performed by mac80211 is simple
round-robin, but a subsequent commit adds airtime fairness awareness to the
scheduler.
Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
---
include/net/mac80211.h | 62 +++++++++++++++++++++++++++++++++++++++++++---
net/mac80211/agg-tx.c | 2 +-
net/mac80211/driver-ops.h | 9 +++++++
net/mac80211/ieee80211_i.h | 9 +++++++
net/mac80211/main.c | 5 ++++
net/mac80211/sta_info.c | 2 +-
net/mac80211/tx.c | 58 ++++++++++++++++++++++++++++++++++++++++++-
7 files changed, 140 insertions(+), 7 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 71985e95d2d9..18b11c119b7e 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -108,9 +108,15 @@
* The driver is expected to initialize its private per-queue data for stations
* and interfaces in the .add_interface and .sta_add ops.
*
- * The driver can't access the queue directly. To dequeue a frame, it calls
- * ieee80211_tx_dequeue(). Whenever mac80211 adds a new frame to a queue, it
- * calls the .wake_tx_queue driver op.
+ * The driver can't access the queue directly. To dequeue a frame from a
+ * txq, it calls ieee80211_tx_dequeue(). Whenever mac80211 adds a new frame to a
+ * queue, it calls the .wake_tx_queue driver op.
+ *
+ * Drivers can optionally delegate responsibility for scheduling queues to
+ * mac80211, to take advantage of airtime fairness accounting. In this case, to
+ * obtain the next queue to pull frames from, the driver calls
+ * ieee80211_next_txq(). The driver is then expected to return the txq using
+ * ieee80211_return_txq().
*
* For AP powersave TIM handling, the driver only needs to indicate if it has
* buffered packets in the driver specific data structures by calling
@@ -6088,7 +6094,8 @@ void ieee80211_tdls_oper_request(struct ieee80211_vif *vif, const u8 *peer,
* ieee80211_tx_dequeue - dequeue a packet from a software tx queue
*
* @hw: pointer as obtained from ieee80211_alloc_hw()
- * @txq: pointer obtained from station or virtual interface
+ * @txq: pointer obtained from station or virtual interface, or from
+ * ieee80211_next_txq()
*
* Returns the skb if successful, %NULL if no frame was available.
*/
@@ -6096,6 +6103,53 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
struct ieee80211_txq *txq);
/**
+ * ieee80211_next_txq - get next tx queue to pull packets from
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ * @ac: AC number to return packets from.
+ *
+ * Should only be called between calls to ieee80211_txq_schedule_start()
+ * and ieee80211_txq_schedule_end().
+ * Returns the next txq if successful, %NULL if no queue is eligible. If a txq
+ * is returned, it should be returned with ieee80211_return_txq() after the
+ * driver has finished scheduling it.
+ */
+struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac);
+
+/**
+ * ieee80211_return_txq - return a TXQ previously acquired by
+ * ieee80211_next_txq()
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ * @txq: pointer obtained from station or virtual interface
+ *
+ * Should only be called between calls to ieee80211_txq_schedule_start()
+ * and ieee80211_txq_schedule_end().
+ */
+void ieee80211_return_txq(struct ieee80211_hw *hw, struct ieee80211_txq *txq);
+
+/**
+ * ieee80211_txq_schedule_start - acquire locks for safe scheduling of an AC
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ * @ac: AC number to acquire locks for
+ *
+ * Acquire locks needed to schedule TXQs from the given AC. Should be called
+ * before ieee80211_next_txq() or ieee80211_return_txq().
+ */
+void ieee80211_txq_schedule_start(struct ieee80211_hw *hw, u8 ac);
+
+/**
+ * ieee80211_txq_schedule_end - release locks for safe scheduling of an AC
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ * @ac: AC number to acquire locks for
+ *
+ * Release locks previously acquired by ieee80211_txq_schedule_end().
+ */
+void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8 ac);
+
+/**
* ieee80211_txq_get_depth - get pending frame/byte count of given txq
*
* The values are not guaranteed to be coherent with regard to each other, i.e.
diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index 69e831bc317b..e94b1a0407af 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -229,7 +229,7 @@ static void __releases(agg_queue)
clear_bit(IEEE80211_TXQ_STOP, &txqi->flags);
local_bh_disable();
rcu_read_lock();
- drv_wake_tx_queue(sta->sdata->local, txqi);
+ schedule_and_wake_txq(sta->sdata->local, txqi);
rcu_read_unlock();
local_bh_enable();
}
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 0b1747a2313d..ce4138323820 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -1173,6 +1173,15 @@ static inline void drv_wake_tx_queue(struct ieee80211_local *local,
local->ops->wake_tx_queue(&local->hw, &txq->txq);
}
+static inline void schedule_and_wake_txq(struct ieee80211_local *local,
+ struct txq_info *txqi)
+{
+ spin_lock_bh(&local->active_txq_lock[txqi->txq.ac]);
+ ieee80211_return_txq(&local->hw, &txqi->txq);
+ spin_unlock_bh(&local->active_txq_lock[txqi->txq.ac]);
+ drv_wake_tx_queue(local, txqi);
+}
+
static inline int drv_can_aggregate_in_amsdu(struct ieee80211_local *local,
struct sk_buff *head,
struct sk_buff *skb)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 10a05062e4a0..33763a2eac81 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -830,6 +830,8 @@ enum txq_info_flags {
* a fq_flow which is already owned by a different tin
* @def_cvars: codel vars for @def_flow
* @frags: used to keep fragments created after dequeue
+ * @schedule_order: used with ieee80211_local->active_txqs
+ * @schedule_round: counter to prevent infinite loops on TXQ scheduling
*/
struct txq_info {
struct fq_tin tin;
@@ -837,6 +839,8 @@ struct txq_info {
struct codel_vars def_cvars;
struct codel_stats cstats;
struct sk_buff_head frags;
+ struct list_head schedule_order;
+ u16 schedule_round;
unsigned long flags;
/* keep last! */
@@ -1128,6 +1132,11 @@ struct ieee80211_local {
struct codel_vars *cvars;
struct codel_params cparams;
+ /* protects active_txqs and txqi->schedule_order */
+ spinlock_t active_txq_lock[IEEE80211_NUM_ACS];
+ struct list_head active_txqs[IEEE80211_NUM_ACS];
+ u16 schedule_round[IEEE80211_NUM_ACS];
+
const struct ieee80211_ops *ops;
/*
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 83e71e6b2ebe..ba64757afea4 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -663,6 +663,11 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
spin_lock_init(&local->rx_path_lock);
spin_lock_init(&local->queue_stop_reason_lock);
+ for (i = 0; i < IEEE80211_NUM_ACS; i++) {
+ INIT_LIST_HEAD(&local->active_txqs[i]);
+ spin_lock_init(&local->active_txq_lock[i]);
+ }
+
INIT_LIST_HEAD(&local->chanctx_list);
mutex_init(&local->chanctx_mtx);
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index fb8c2252ac0e..c2f5cb7df54f 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1249,7 +1249,7 @@ void ieee80211_sta_ps_deliver_wakeup(struct sta_info *sta)
if (!sta->sta.txq[i] || !txq_has_queue(sta->sta.txq[i]))
continue;
- drv_wake_tx_queue(local, to_txq_info(sta->sta.txq[i]));
+ schedule_and_wake_txq(local, to_txq_info(sta->sta.txq[i]));
}
skb_queue_head_init(&pending);
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index e0ccee23fbcd..305965283506 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1449,6 +1449,7 @@ void ieee80211_txq_init(struct ieee80211_sub_if_data *sdata,
codel_vars_init(&txqi->def_cvars);
codel_stats_init(&txqi->cstats);
__skb_queue_head_init(&txqi->frags);
+ INIT_LIST_HEAD(&txqi->schedule_order);
txqi->txq.vif = &sdata->vif;
@@ -1489,6 +1490,9 @@ void ieee80211_txq_purge(struct ieee80211_local *local,
fq_tin_reset(fq, tin, fq_skb_free_func);
ieee80211_purge_tx_queue(&local->hw, &txqi->frags);
+ spin_lock_bh(&local->active_txq_lock[txqi->txq.ac]);
+ list_del_init(&txqi->schedule_order);
+ spin_unlock_bh(&local->active_txq_lock[txqi->txq.ac]);
}
void ieee80211_txq_set_params(struct ieee80211_local *local)
@@ -1605,7 +1609,7 @@ static bool ieee80211_queue_skb(struct ieee80211_local *local,
ieee80211_txq_enqueue(local, txqi, skb);
spin_unlock_bh(&fq->lock);
- drv_wake_tx_queue(local, txqi);
+ schedule_and_wake_txq(local, txqi);
return true;
}
@@ -3627,6 +3631,58 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
}
EXPORT_SYMBOL(ieee80211_tx_dequeue);
+struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+ struct txq_info *txqi = NULL;
+
+ lockdep_assert_held(&local->active_txq_lock[ac]);
+
+ txqi = list_first_entry_or_null(&local->active_txqs[ac],
+ struct txq_info,
+ schedule_order);
+
+ if (!txqi || txqi->schedule_round == local->schedule_round[ac])
+ return NULL;
+
+ list_del_init(&txqi->schedule_order);
+ txqi->schedule_round = local->schedule_round[ac];
+ return &txqi->txq;
+}
+EXPORT_SYMBOL(ieee80211_next_txq);
+
+void ieee80211_return_txq(struct ieee80211_hw *hw,
+ struct ieee80211_txq *txq)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+ struct txq_info *txqi = to_txq_info(txq);
+
+ lockdep_assert_held(&local->active_txq_lock[txq->ac]);
+
+ if (list_empty(&txqi->schedule_order) &&
+ (!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets))
+ list_add_tail(&txqi->schedule_order,
+ &local->active_txqs[txq->ac]);
+}
+EXPORT_SYMBOL(ieee80211_return_txq);
+
+void ieee80211_txq_schedule_start(struct ieee80211_hw *hw, u8 ac)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+
+ spin_lock_bh(&local->active_txq_lock[ac]);
+ local->schedule_round[ac]++;
+}
+EXPORT_SYMBOL(ieee80211_txq_schedule_start);
+
+void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8 ac)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+
+ spin_unlock_bh(&local->active_txq_lock[ac]);
+}
+EXPORT_SYMBOL(ieee80211_txq_schedule_end);
+
void __ieee80211_subif_start_xmit(struct sk_buff *skb,
struct net_device *dev,
u32 info_flags)
--
1.9.1
From: Toke Høiland-Jørgensen <[email protected]>
This adds TX airtime statistics to the cfg80211 station dump (to go along
with the RX info already present), and adds a new parameter to set the
airtime weight of each station. The latter allows userspace to implement
policies for different stations by varying their weights.
Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
[[email protected]: fixed checkpatch warnings]
Signed-off-by: Rajkumar Manoharan <[email protected]>
---
include/net/cfg80211.h | 10 +++++++++-
include/uapi/linux/nl80211.h | 15 +++++++++++++++
net/wireless/nl80211.c | 30 ++++++++++++++++++++++++++++++
3 files changed, 54 insertions(+), 1 deletion(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 1fa41b7a1be3..f1312f17fbef 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1001,6 +1001,7 @@ enum station_parameters_apply_mask {
* @support_p2p_ps: information if station supports P2P PS mechanism
* @he_capa: HE capabilities of station
* @he_capa_len: the length of the HE capabilities
+ * @airtime_weight: airtime scheduler weight for this station
*/
struct station_parameters {
const u8 *supported_rates;
@@ -1030,6 +1031,7 @@ struct station_parameters {
int support_p2p_ps;
const struct ieee80211_he_cap_elem *he_capa;
u8 he_capa_len;
+ u16 airtime_weight;
};
/**
@@ -1297,6 +1299,8 @@ struct cfg80211_tid_stats {
* @rx_beacon_signal_avg: signal strength average (in dBm) for beacons received
* from this peer
* @rx_duration: aggregate PPDU duration(usecs) for all the frames from a peer
+ * @tx_duration: aggregate PPDU duration(usecs) for all the frames to a peer
+ * @airtime_weight: current airtime scheduling weight
* @pertid: per-TID statistics, see &struct cfg80211_tid_stats, using the last
* (IEEE80211_NUM_TIDS) index for MSDUs not encapsulated in QoS-MPDUs.
* Note that this doesn't use the @filled bit, but is used if non-NULL.
@@ -1347,10 +1351,12 @@ struct station_info {
u32 expected_throughput;
- u64 rx_beacon;
+ u64 tx_duration;
u64 rx_duration;
+ u64 rx_beacon;
u8 rx_beacon_signal_avg;
struct cfg80211_tid_stats *pertid;
+ u16 airtime_weight;
s8 ack_signal;
s8 avg_ack_signal;
@@ -2381,6 +2387,8 @@ enum wiphy_params_flags {
WIPHY_PARAM_TXQ_QUANTUM = 1 << 8,
};
+#define IEEE80211_DEFAULT_AIRTIME_WEIGHT 256
+
/**
* struct cfg80211_pmksa - PMK Security Association
*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 6d610bae30a9..9428bd0ebb1a 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2254,6 +2254,9 @@ enum nl80211_commands {
* @NL80211_ATTR_FTM_RESPONDER_STATS: Nested attribute with FTM responder
* statistics, see &enum nl80211_ftm_responder_stats.
*
+ * @NL80211_ATTR_AIRTIME_WEIGHT: Station's weight when scheduled by the airtime
+ * scheduler.
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2699,6 +2702,8 @@ enum nl80211_attrs {
NL80211_ATTR_FTM_RESPONDER_STATS,
+ NL80211_ATTR_AIRTIME_WEIGHT,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
@@ -3074,6 +3079,9 @@ enum nl80211_sta_bss_param {
* with an FCS error (u32, from this station). This count may not include
* some packets with an FCS error due to TA corruption. Hence this counter
* might not be fully accurate.
+ * @NL80211_STA_INFO_TX_DURATION: aggregate PPDU duration for all frames
+ * sent to the station (u64, usec)
+ * @NL80211_STA_INFO_AIRTIME_WEIGHT: current airtime weight for station (u16)
* @__NL80211_STA_INFO_AFTER_LAST: internal
* @NL80211_STA_INFO_MAX: highest possible station info attribute
*/
@@ -3116,6 +3124,8 @@ enum nl80211_sta_info {
NL80211_STA_INFO_ACK_SIGNAL_AVG,
NL80211_STA_INFO_RX_MPDUS,
NL80211_STA_INFO_FCS_ERROR_COUNT,
+ NL80211_STA_INFO_TX_DURATION,
+ NL80211_STA_INFO_AIRTIME_WEIGHT,
/* keep last */
__NL80211_STA_INFO_AFTER_LAST,
@@ -5258,6 +5268,10 @@ enum nl80211_feature_flags {
* if this flag is not set. Ignoring this can leak clear text packets and/or
* freeze the connection.
*
+ * @NL80211_EXT_FEATURE_AIRTIME_FAIRNESS: Driver supports getting airtime
+ * fairness for transmitted packets and has enabled airtime fairness
+ * scheduling.
+ *
* @NUM_NL80211_EXT_FEATURES: number of extended features.
* @MAX_NL80211_EXT_FEATURES: highest extended feature index.
*/
@@ -5297,6 +5311,7 @@ enum nl80211_ext_feature_index {
NL80211_EXT_FEATURE_SCAN_MIN_PREQ_CONTENT,
NL80211_EXT_FEATURE_CAN_REPLACE_PTK0,
NL80211_EXT_FEATURE_ENABLE_FTM_RESPONDER,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS,
/* add new features before the definition below */
NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 8d763725498c..ec7c80c01008 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -497,6 +497,7 @@ static int validate_ie_attr(const struct nlattr *attr,
.type = NLA_NESTED,
.validation_data = nl80211_ftm_responder_policy,
},
+ [NL80211_ATTR_AIRTIME_WEIGHT] = { .type = NLA_U16 },
};
/* policy for the key attributes */
@@ -4690,6 +4691,11 @@ static int nl80211_send_station(struct sk_buff *msg, u32 cmd, u32 portid,
PUT_SINFO(PLID, plid, u16);
PUT_SINFO(PLINK_STATE, plink_state, u8);
PUT_SINFO_U64(RX_DURATION, rx_duration);
+ PUT_SINFO_U64(TX_DURATION, tx_duration);
+
+ if (wiphy_ext_feature_isset(&rdev->wiphy,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
+ PUT_SINFO(AIRTIME_WEIGHT, airtime_weight, u16);
switch (rdev->wiphy.signal_type) {
case CFG80211_SIGNAL_TYPE_MBM:
@@ -5308,6 +5314,18 @@ static int nl80211_set_station(struct sk_buff *skb, struct genl_info *info)
nla_get_u8(info->attrs[NL80211_ATTR_OPMODE_NOTIF]);
}
+ if (info->attrs[NL80211_ATTR_AIRTIME_WEIGHT]) {
+ params.airtime_weight =
+ nla_get_u16(info->attrs[NL80211_ATTR_AIRTIME_WEIGHT]);
+ if (!params.airtime_weight)
+ return -EINVAL;
+ }
+
+ if (params.airtime_weight &&
+ !wiphy_ext_feature_isset(&rdev->wiphy,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
+ return -EOPNOTSUPP;
+
/* Include parameters for TDLS peer (will check later) */
err = nl80211_set_station_tdls(info, ¶ms);
if (err)
@@ -5436,6 +5454,18 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
params.plink_action =
nla_get_u8(info->attrs[NL80211_ATTR_STA_PLINK_ACTION]);
+ if (info->attrs[NL80211_ATTR_AIRTIME_WEIGHT]) {
+ params.airtime_weight =
+ nla_get_u16(info->attrs[NL80211_ATTR_AIRTIME_WEIGHT]);
+ if (!params.airtime_weight)
+ return -EINVAL;
+ }
+
+ if (params.airtime_weight &&
+ !wiphy_ext_feature_isset(&rdev->wiphy,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
+ return -EOPNOTSUPP;
+
err = nl80211_parse_sta_channel_info(info, ¶ms);
if (err)
return err;
--
1.9.1
From: Toke Høiland-Jørgensen <[email protected]>
This adds airtime accounting and scheduling to the mac80211 TXQ
scheduler. A new callback, ieee80211_sta_register_airtime(), is added
that drivers can call to report airtime usage for stations.
When airtime information is present, mac80211 will schedule TXQs
(through ieee80211_next_txq()) in a way that enforces airtime fairness
between active stations. This scheduling works the same way as the ath9k
in-driver airtime fairness scheduling. If no airtime usage is reported
by the driver, the scheduler will default to round-robin scheduling.
For drivers that don't control TXQ scheduling in software, a new API
function, ieee80211_txq_may_transmit(), is added which the driver can use
to check if the TXQ is eligible for transmission, or should be throttled to
enforce fairness. Calls to this function must also be enclosed in
ieee80211_txq_schedule_{start,end}() calls to ensure proper locking.
The API ieee80211_txq_may_transmit() also ensures that TXQ list will be
aligned aginst driver's own round-robin scheduler list. i.e it rotates
the TXQ list till it makes the requested node becomes the first entry
in TXQ list. Thus both the TXQ list and driver's list are in sync.
Co-Developed-by: Rajkumar Manoharan <[email protected]>
Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
Signed-off-by: Rajkumar Manoharan <[email protected]>
---
include/net/mac80211.h | 59 ++++++++++++++++++++++++++++++
net/mac80211/cfg.c | 3 ++
net/mac80211/debugfs.c | 3 ++
net/mac80211/debugfs_sta.c | 50 ++++++++++++++++++++++++--
net/mac80211/ieee80211_i.h | 2 ++
net/mac80211/main.c | 4 +++
net/mac80211/sta_info.c | 44 +++++++++++++++++++++--
net/mac80211/sta_info.h | 13 +++++++
net/mac80211/status.c | 6 ++++
net/mac80211/tx.c | 90 +++++++++++++++++++++++++++++++++++++++++++---
10 files changed, 264 insertions(+), 10 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 18b11c119b7e..c43d615ee9b1 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2357,6 +2357,9 @@ enum ieee80211_hw_flags {
* @tx_sk_pacing_shift: Pacing shift to set on TCP sockets when frames from
* them are encountered. The default should typically not be changed,
* unless the driver has good reasons for needing more buffers.
+ *
+ * @weight_multipler: Driver specific airtime weight multiplier used while
+ * refilling deficit of each TXQ.
*/
struct ieee80211_hw {
struct ieee80211_conf conf;
@@ -2393,6 +2396,7 @@ struct ieee80211_hw {
const struct ieee80211_cipher_scheme *cipher_schemes;
u8 max_nan_de_entries;
u8 tx_sk_pacing_shift;
+ u8 weight_multiplier;
};
static inline bool _ieee80211_hw_check(struct ieee80211_hw *hw,
@@ -5393,6 +5397,34 @@ void ieee80211_sta_block_awake(struct ieee80211_hw *hw,
void ieee80211_send_eosp_nullfunc(struct ieee80211_sta *pubsta, int tid);
/**
+ * ieee80211_sta_register_airtime - register airtime usage for a sta/tid
+ *
+ * Register airtime usage for a given sta on a given tid. The driver can call
+ * this function to notify mac80211 that a station used a certain amount of
+ * airtime. This information will be used by the TXQ scheduler to schedule
+ * stations in a way that ensures airtime fairness.
+ *
+ * The reported airtime should as a minimum include all time that is spent
+ * transmitting to the remote station, including overhead and padding, but not
+ * including time spent waiting for a TXOP. If the time is not reported by the
+ * hardware it can in some cases be calculated from the rate and known frame
+ * composition. When possible, the time should include any failed transmission
+ * attempts.
+ *
+ * The driver can either call this function synchronously for every packet or
+ * aggregate, or asynchronously as airtime usage information becomes available.
+ * TX and RX airtime can be reported together, or separately by setting one of
+ * them to 0.
+ *
+ * @pubsta: the station
+ * @tid: the TID to register airtime for
+ * @tx_airtime: airtime used during TX (in usec)
+ * @rx_airtime: airtime used during RX (in usec)
+ */
+void ieee80211_sta_register_airtime(struct ieee80211_sta *pubsta, u8 tid,
+ u32 tx_airtime, u32 rx_airtime);
+
+/**
* ieee80211_iter_keys - iterate keys programmed into the device
* @hw: pointer obtained from ieee80211_alloc_hw()
* @vif: virtual interface to iterate, may be %NULL for all
@@ -6150,6 +6182,33 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8 ac);
/**
+ * ieee80211_txq_may_transmit - check whether TXQ is allowed to transmit
+ *
+ * This function is used to check whether given txq is allowed to transmit by
+ * the airtime scheduler, and can be used by drivers to access the airtime
+ * fairness accounting without going using the scheduling order enfored by
+ * next_txq().
+ *
+ * Returns %true if the airtime scheduler thinks the TXQ should be allowed to
+ * transmit, and %false if it should be throttled. This function can also have
+ * the side effect of rotating the TXQ in the scheduler rotation, which will
+ * eventually bring the deficit to positive and allow the station to transmit
+ * again.
+ *
+ * The API ieee80211_txq_may_transmit() also ensures that TXQ list will be
+ * aligned aginst driver's own round-robin scheduler list. i.e it rotates
+ * the TXQ list till it makes the requested node becomes the first entry
+ * in TXQ list. Thus both the TXQ list and driver's list are in sync. If this
+ * function returns %true, the driver is expected to schedule packets
+ * for transmission, and then return the TXQ through ieee80211_return_txq().
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ * @txq: pointer obtained from station or virtual interface
+ */
+bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
+ struct ieee80211_txq *txq);
+
+/**
* ieee80211_txq_get_depth - get pending frame/byte count of given txq
*
* The values are not guaranteed to be coherent with regard to each other, i.e.
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 818aa0060349..57c59e5ceb98 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1447,6 +1447,9 @@ static int sta_apply_parameters(struct ieee80211_local *local,
if (ieee80211_vif_is_mesh(&sdata->vif))
sta_apply_mesh_params(local, sta, params);
+ if (params->airtime_weight)
+ sta->airtime_weight = params->airtime_weight;
+
/* set the STA state after all sta info from usermode has been set */
if (test_sta_flag(sta, WLAN_STA_TDLS_PEER) ||
set & BIT(NL80211_STA_FLAG_ASSOCIATED)) {
diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index 3fe541e358f3..81c5fec2eae7 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -383,6 +383,9 @@ void debugfs_hw_add(struct ieee80211_local *local)
if (local->ops->wake_tx_queue)
DEBUGFS_ADD_MODE(aqm, 0600);
+ debugfs_create_u16("airtime_flags", 0600,
+ phyd, &local->airtime_flags);
+
statsd = debugfs_create_dir("statistics", phyd);
/* if the dir failed, don't put all the other things into the root! */
diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index af5185a836e5..446908ab3f5d 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -181,9 +181,9 @@ static ssize_t sta_aqm_read(struct file *file, char __user *userbuf,
txqi->tin.tx_bytes,
txqi->tin.tx_packets,
txqi->flags,
- txqi->flags & (1<<IEEE80211_TXQ_STOP) ? "STOP" : "RUN",
- txqi->flags & (1<<IEEE80211_TXQ_AMPDU) ? " AMPDU" : "",
- txqi->flags & (1<<IEEE80211_TXQ_NO_AMSDU) ? " NO-AMSDU" : "");
+ test_bit(IEEE80211_TXQ_STOP, &txqi->flags) ? "STOP" : "RUN",
+ test_bit(IEEE80211_TXQ_AMPDU, &txqi->flags) ? " AMPDU" : "",
+ test_bit(IEEE80211_TXQ_NO_AMSDU, &txqi->flags) ? " NO-AMSDU" : "");
}
rcu_read_unlock();
@@ -195,6 +195,46 @@ static ssize_t sta_aqm_read(struct file *file, char __user *userbuf,
}
STA_OPS(aqm);
+static ssize_t sta_airtime_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 = 200;
+ char *buf = kzalloc(bufsz, GFP_KERNEL), *p = buf;
+ u64 rx_airtime = 0, tx_airtime = 0;
+ s64 deficit[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]);
+ 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;
+}
+STA_OPS(airtime);
+
static ssize_t sta_agg_status_read(struct file *file, char __user *userbuf,
size_t count, loff_t *ppos)
{
@@ -906,6 +946,10 @@ void ieee80211_sta_debugfs_add(struct sta_info *sta)
if (local->ops->wake_tx_queue)
DEBUGFS_ADD(aqm);
+ if (wiphy_ext_feature_isset(local->hw.wiphy,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
+ DEBUGFS_ADD(airtime);
+
if (sizeof(sta->driver_buffered_tids) == sizeof(u32))
debugfs_create_x32("driver_buffered_tids", 0400,
sta->debugfs_dir,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 33763a2eac81..c5b6ba571288 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1137,6 +1137,8 @@ struct ieee80211_local {
struct list_head active_txqs[IEEE80211_NUM_ACS];
u16 schedule_round[IEEE80211_NUM_ACS];
+ u16 airtime_flags;
+
const struct ieee80211_ops *ops;
/*
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index ba64757afea4..3710727fa9c1 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -667,6 +667,7 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
INIT_LIST_HEAD(&local->active_txqs[i]);
spin_lock_init(&local->active_txq_lock[i]);
}
+ local->airtime_flags = AIRTIME_USE_TX | AIRTIME_USE_RX;
INIT_LIST_HEAD(&local->chanctx_list);
mutex_init(&local->chanctx_mtx);
@@ -1153,6 +1154,9 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
if (!local->hw.max_nan_de_entries)
local->hw.max_nan_de_entries = IEEE80211_MAX_NAN_INSTANCE_ID;
+ if (!local->hw.weight_multiplier)
+ local->hw.weight_multiplier = 1;
+
result = ieee80211_wep_init(local);
if (result < 0)
wiphy_debug(local->hw.wiphy, "Failed to initialize wep: %d\n",
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index c2f5cb7df54f..25d47b2af7a9 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -90,7 +90,6 @@ static void __cleanup_single_sta(struct sta_info *sta)
struct tid_ampdu_tx *tid_tx;
struct ieee80211_sub_if_data *sdata = sta->sdata;
struct ieee80211_local *local = sdata->local;
- struct fq *fq = &local->fq;
struct ps_data *ps;
if (test_sta_flag(sta, WLAN_STA_PS_STA) ||
@@ -120,9 +119,7 @@ static void __cleanup_single_sta(struct sta_info *sta)
txqi = to_txq_info(sta->sta.txq[i]);
- spin_lock_bh(&fq->lock);
ieee80211_txq_purge(local, txqi);
- spin_unlock_bh(&fq->lock);
}
}
@@ -387,9 +384,12 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata,
if (sta_prepare_rate_control(local, sta, gfp))
goto free_txq;
+ sta->airtime_weight = IEEE80211_DEFAULT_AIRTIME_WEIGHT;
+
for (i = 0; i < IEEE80211_NUM_ACS; i++) {
skb_queue_head_init(&sta->ps_tx_buf[i]);
skb_queue_head_init(&sta->tx_filtered[i]);
+ sta->airtime[i].deficit = sta->airtime_weight;
}
for (i = 0; i < IEEE80211_NUM_TIDS; i++)
@@ -1826,6 +1826,27 @@ void ieee80211_sta_set_buffered(struct ieee80211_sta *pubsta,
}
EXPORT_SYMBOL(ieee80211_sta_set_buffered);
+void ieee80211_sta_register_airtime(struct ieee80211_sta *pubsta, u8 tid,
+ u32 tx_airtime, u32 rx_airtime)
+{
+ struct sta_info *sta = container_of(pubsta, struct sta_info, sta);
+ struct ieee80211_local *local = sta->sdata->local;
+ u8 ac = ieee80211_ac_from_tid(tid);
+ u32 airtime = 0;
+
+ if (sta->local->airtime_flags & AIRTIME_USE_TX)
+ airtime += tx_airtime;
+ if (sta->local->airtime_flags & AIRTIME_USE_RX)
+ airtime += rx_airtime;
+
+ spin_lock_bh(&local->active_txq_lock[ac]);
+ sta->airtime[ac].tx_airtime += tx_airtime;
+ sta->airtime[ac].rx_airtime += rx_airtime;
+ sta->airtime[ac].deficit -= airtime;
+ spin_unlock_bh(&local->active_txq_lock[ac]);
+}
+EXPORT_SYMBOL(ieee80211_sta_register_airtime);
+
int sta_info_move_state(struct sta_info *sta,
enum ieee80211_sta_state new_state)
{
@@ -2188,6 +2209,23 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_FAILED);
}
+ if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_RX_DURATION))) {
+ for (ac = 0; ac < IEEE80211_NUM_ACS; ac++)
+ sinfo->rx_duration += sta->airtime[ac].rx_airtime;
+ sinfo->filled |= BIT(NL80211_STA_INFO_RX_DURATION);
+ }
+
+ if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_DURATION))) {
+ for (ac = 0; ac < IEEE80211_NUM_ACS; ac++)
+ sinfo->tx_duration += sta->airtime[ac].tx_airtime;
+ sinfo->filled |= BIT(NL80211_STA_INFO_TX_DURATION);
+ }
+
+ if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_AIRTIME_WEIGHT))) {
+ sinfo->airtime_weight = sta->airtime_weight;
+ sinfo->filled |= BIT(NL80211_STA_INFO_AIRTIME_WEIGHT);
+ }
+
sinfo->rx_dropped_misc = sta->rx_stats.dropped;
if (sta->pcpu_rx_stats) {
for_each_possible_cpu(cpu) {
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 9a04327d71d1..b1b0fd6a2e21 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -127,6 +127,16 @@ enum ieee80211_agg_stop_reason {
AGG_STOP_DESTROY_STA,
};
+/* Debugfs flags to enable/disable use of RX/TX airtime in scheduler */
+#define AIRTIME_USE_TX BIT(0)
+#define AIRTIME_USE_RX BIT(1)
+
+struct airtime_info {
+ u64 rx_airtime;
+ u64 tx_airtime;
+ s64 deficit;
+};
+
struct sta_info;
/**
@@ -563,6 +573,9 @@ struct sta_info {
} tx_stats;
u16 tid_seq[IEEE80211_QOS_CTL_TID_MASK + 1];
+ struct airtime_info airtime[IEEE80211_NUM_ACS];
+ u16 airtime_weight;
+
/*
* Aggregation information, locked with lock.
*/
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index aa4afbf0abaf..a1f1256448f5 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -818,6 +818,12 @@ static void __ieee80211_tx_status(struct ieee80211_hw *hw,
ieee80211_sta_tx_notify(sta->sdata, (void *) skb->data,
acked, info->status.tx_time);
+ if (info->status.tx_time &&
+ wiphy_ext_feature_isset(local->hw.wiphy,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
+ ieee80211_sta_register_airtime(&sta->sta, tid,
+ info->status.tx_time, 0);
+
if (ieee80211_hw_check(&local->hw, REPORTS_TX_ACK_STATUS)) {
if (info->flags & IEEE80211_TX_STAT_ACK) {
if (sta->status_stats.lost_packets)
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 305965283506..3f417e80e041 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1488,8 +1488,11 @@ void ieee80211_txq_purge(struct ieee80211_local *local,
struct fq *fq = &local->fq;
struct fq_tin *tin = &txqi->tin;
+ spin_lock_bh(&fq->lock);
fq_tin_reset(fq, tin, fq_skb_free_func);
ieee80211_purge_tx_queue(&local->hw, &txqi->frags);
+ spin_unlock_bh(&fq->lock);
+
spin_lock_bh(&local->active_txq_lock[txqi->txq.ac]);
list_del_init(&txqi->schedule_order);
spin_unlock_bh(&local->active_txq_lock[txqi->txq.ac]);
@@ -3638,11 +3641,28 @@ struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac)
lockdep_assert_held(&local->active_txq_lock[ac]);
+ begin:
txqi = list_first_entry_or_null(&local->active_txqs[ac],
struct txq_info,
schedule_order);
+ if (!txqi)
+ return NULL;
+
+ if (txqi->txq.sta) {
+ struct sta_info *sta = container_of(txqi->txq.sta,
+ struct sta_info, sta);
+
+ if (sta->airtime[txqi->txq.ac].deficit < 0) {
+ sta->airtime[txqi->txq.ac].deficit +=
+ sta->airtime_weight;
+ list_move_tail(&txqi->schedule_order,
+ &local->active_txqs[txqi->txq.ac]);
+ goto begin;
+ }
+ }
+
- if (!txqi || txqi->schedule_round == local->schedule_round[ac])
+ if (txqi->schedule_round == local->schedule_round[ac])
return NULL;
list_del_init(&txqi->schedule_order);
@@ -3660,12 +3680,74 @@ void ieee80211_return_txq(struct ieee80211_hw *hw,
lockdep_assert_held(&local->active_txq_lock[txq->ac]);
if (list_empty(&txqi->schedule_order) &&
- (!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets))
- list_add_tail(&txqi->schedule_order,
- &local->active_txqs[txq->ac]);
+ (!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets)) {
+ /* If airtime accounting is active, always enqueue STAs at the
+ * head of the list to ensure that they only get moved to the
+ * back by the airtime DRR scheduler once they have a negative
+ * deficit. A station that already has a negative deficit will
+ * get immediately moved to the back of the list on the next
+ * call to ieee80211_next_txq().
+ */
+ if (txqi->txq.sta &&
+ wiphy_ext_feature_isset(local->hw.wiphy,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
+ list_add(&txqi->schedule_order,
+ &local->active_txqs[txq->ac]);
+ else
+ list_add_tail(&txqi->schedule_order,
+ &local->active_txqs[txq->ac]);
+ }
}
EXPORT_SYMBOL(ieee80211_return_txq);
+bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
+ struct ieee80211_txq *txq)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+ struct txq_info *iter, *tmp, *txqi = to_txq_info(txq);
+ struct sta_info *sta;
+ u8 ac = txq->ac;
+
+ lockdep_assert_held(&local->active_txq_lock[ac]);
+
+ if (!txqi->txq.sta)
+ goto out;
+
+ if (list_empty(&txqi->schedule_order))
+ goto out;
+
+ list_for_each_entry_safe(iter, tmp, &local->active_txqs[ac],
+ schedule_order) {
+ if (iter == txqi)
+ break;
+
+ if (!iter->txq.sta) {
+ list_move_tail(&iter->schedule_order,
+ &local->active_txqs[ac]);
+ continue;
+ }
+ sta = container_of(iter->txq.sta, struct sta_info, sta);
+ if (sta->airtime[ac].deficit < 0)
+ sta->airtime[ac].deficit += sta->airtime_weight;
+ list_move_tail(&iter->schedule_order, &local->active_txqs[ac]);
+ }
+
+ sta = container_of(txqi->txq.sta, struct sta_info, sta);
+ if (sta->airtime[ac].deficit >= 0)
+ goto out;
+
+ sta->airtime[ac].deficit += sta->airtime_weight;
+ list_move_tail(&txqi->schedule_order, &local->active_txqs[ac]);
+
+ return false;
+out:
+ if (!list_empty(&txqi->schedule_order))
+ list_del_init(&txqi->schedule_order);
+
+ return true;
+}
+EXPORT_SYMBOL(ieee80211_txq_may_transmit);
+
void ieee80211_txq_schedule_start(struct ieee80211_hw *hw, u8 ac)
{
struct ieee80211_local *local = hw_to_local(hw);
--
1.9.1
From: Toke Høiland-Jørgensen <[email protected]>
This moves the ath9k driver to use the mac80211 TXQ scheduling and
airtime accounting APIs, removing the corresponding state tracking
inside the driver.
Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
[[email protected]: fixed checkpatch error and warnings]
Signed-off-by: Rajkumar Manoharan <[email protected]>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 14 --
drivers/net/wireless/ath/ath9k/debug.c | 3 -
drivers/net/wireless/ath/ath9k/debug.h | 8 -
drivers/net/wireless/ath/ath9k/debug_sta.c | 70 ---------
drivers/net/wireless/ath/ath9k/init.c | 3 +-
drivers/net/wireless/ath/ath9k/recv.c | 9 +-
drivers/net/wireless/ath/ath9k/xmit.c | 244 +++++++++--------------------
7 files changed, 75 insertions(+), 276 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 21ba20981a80..90b56766dab1 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -112,8 +112,6 @@ int ath_descdma_setup(struct ath_softc *sc, struct ath_descdma *dd,
#define ATH_TXFIFO_DEPTH 8
#define ATH_TX_ERROR 0x01
-#define ATH_AIRTIME_QUANTUM 300 /* usec */
-
/* Stop tx traffic 1ms before the GO goes away */
#define ATH_P2P_PS_STOP_TIME 1000
@@ -246,10 +244,8 @@ struct ath_atx_tid {
s8 bar_index;
bool active;
bool clear_ps_filter;
- bool has_queued;
};
-void __ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid);
void ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid);
struct ath_node {
@@ -263,12 +259,9 @@ struct ath_node {
bool sleeping;
bool no_ps_filter;
- s64 airtime_deficit[IEEE80211_NUM_ACS];
- u32 airtime_rx_start;
#ifdef CONFIG_ATH9K_STATION_STATISTICS
struct ath_rx_rate_stats rx_rate_stats;
- struct ath_airtime_stats airtime_stats;
#endif
u8 key_idx[4];
@@ -986,11 +979,6 @@ struct ath_ant_comb {
#define ATH9K_NUM_CHANCTX 2 /* supports 2 operating channels */
-#define AIRTIME_USE_TX BIT(0)
-#define AIRTIME_USE_RX BIT(1)
-#define AIRTIME_USE_NEW_QUEUES BIT(2)
-#define AIRTIME_ACTIVE(flags) (!!(flags & (AIRTIME_USE_TX|AIRTIME_USE_RX)))
-
struct ath_softc {
struct ieee80211_hw *hw;
struct device *dev;
@@ -1034,8 +1022,6 @@ struct ath_softc {
short nbcnvifs;
unsigned long ps_usecount;
- u16 airtime_flags; /* AIRTIME_* */
-
struct ath_rx rx;
struct ath_tx tx;
struct ath_beacon beacon;
diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index 4399e9ad058f..0dfea5d6e949 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -1443,9 +1443,6 @@ int ath9k_init_debug(struct ath_hw *ah)
#endif
debugfs_create_file("tpc", 0600, sc->debug.debugfs_phy, sc, &fops_tpc);
- debugfs_create_u16("airtime_flags", 0600,
- sc->debug.debugfs_phy, &sc->airtime_flags);
-
debugfs_create_file("nf_override", 0600,
sc->debug.debugfs_phy, sc, &fops_nf_override);
diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
index 79607db14387..33826aa13687 100644
--- a/drivers/net/wireless/ath/ath9k/debug.h
+++ b/drivers/net/wireless/ath/ath9k/debug.h
@@ -319,20 +319,12 @@ static inline void ath9k_debug_stat_ant(struct ath_softc *sc,
void ath_debug_rate_stats(struct ath_softc *sc,
struct ath_rx_status *rs,
struct sk_buff *skb);
-void ath_debug_airtime(struct ath_softc *sc,
- struct ath_node *an,
- u32 rx, u32 tx);
#else
static inline void ath_debug_rate_stats(struct ath_softc *sc,
struct ath_rx_status *rs,
struct sk_buff *skb)
{
}
-static inline void ath_debug_airtime(struct ath_softc *sc,
- struct ath_node *an,
- u32 rx, u32 tx)
-{
-}
#endif /* CONFIG_ATH9K_STATION_STATISTICS */
#endif /* DEBUG_H */
diff --git a/drivers/net/wireless/ath/ath9k/debug_sta.c b/drivers/net/wireless/ath/ath9k/debug_sta.c
index e8fcd3e1c470..d95cabddce33 100644
--- a/drivers/net/wireless/ath/ath9k/debug_sta.c
+++ b/drivers/net/wireless/ath/ath9k/debug_sta.c
@@ -242,75 +242,6 @@ static ssize_t read_file_node_recv(struct file *file, char __user *user_buf,
.llseek = default_llseek,
};
-void ath_debug_airtime(struct ath_softc *sc,
- struct ath_node *an,
- u32 rx,
- u32 tx)
-{
- struct ath_airtime_stats *astats = &an->airtime_stats;
-
- astats->rx_airtime += rx;
- astats->tx_airtime += tx;
-}
-
-static ssize_t read_airtime(struct file *file, char __user *user_buf,
- size_t count, loff_t *ppos)
-{
- struct ath_node *an = file->private_data;
- struct ath_airtime_stats *astats;
- static const char *qname[4] = {
- "VO", "VI", "BE", "BK"
- };
- u32 len = 0, size = 256;
- char *buf;
- size_t retval;
- int i;
-
- buf = kzalloc(size, GFP_KERNEL);
- if (buf == NULL)
- return -ENOMEM;
-
- astats = &an->airtime_stats;
-
- len += scnprintf(buf + len, size - len, "RX: %u us\n", astats->rx_airtime);
- len += scnprintf(buf + len, size - len, "TX: %u us\n", astats->tx_airtime);
- len += scnprintf(buf + len, size - len, "Deficit: ");
- for (i = 0; i < 4; i++)
- len += scnprintf(buf+len, size - len, "%s: %lld us ", qname[i], an->airtime_deficit[i]);
- if (len < size)
- buf[len++] = '\n';
-
- retval = simple_read_from_buffer(user_buf, count, ppos, buf, len);
- kfree(buf);
-
- return retval;
-}
-
-static ssize_t
-write_airtime_reset_stub(struct file *file, const char __user *ubuf,
- size_t count, loff_t *ppos)
-{
- struct ath_node *an = file->private_data;
- struct ath_airtime_stats *astats;
- int i;
-
- astats = &an->airtime_stats;
- astats->rx_airtime = 0;
- astats->tx_airtime = 0;
- for (i = 0; i < 4; i++)
- an->airtime_deficit[i] = ATH_AIRTIME_QUANTUM;
- return count;
-}
-
-static const struct file_operations fops_airtime = {
- .read = read_airtime,
- .write = write_airtime_reset_stub,
- .open = simple_open,
- .owner = THIS_MODULE,
- .llseek = default_llseek,
-};
-
-
void ath9k_sta_add_debugfs(struct ieee80211_hw *hw,
struct ieee80211_vif *vif,
struct ieee80211_sta *sta,
@@ -320,5 +251,4 @@ void ath9k_sta_add_debugfs(struct ieee80211_hw *hw,
debugfs_create_file("node_aggr", 0444, dir, an, &fops_node_aggr);
debugfs_create_file("node_recv", 0444, dir, an, &fops_node_recv);
- debugfs_create_file("airtime", 0644, dir, an, &fops_airtime);
}
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index c070a9e51ebf..45a31f8a1514 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -676,8 +676,6 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
/* Will be cleared in ath9k_start() */
set_bit(ATH_OP_INVALID, &common->op_flags);
- sc->airtime_flags = (AIRTIME_USE_TX | AIRTIME_USE_RX |
- AIRTIME_USE_NEW_QUEUES);
sc->sc_ah = ah;
sc->dfs_detector = dfs_pattern_detector_init(common, NL80211_DFS_UNSET);
@@ -1013,6 +1011,7 @@ static void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
SET_IEEE80211_PERM_ADDR(hw, common->macaddr);
wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);
+ wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_AIRTIME_FAIRNESS);
}
int ath9k_init_device(u16 devid, struct ath_softc *sc,
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 30d1bd832d90..285a62d3019d 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -1054,14 +1054,7 @@ static void ath_rx_count_airtime(struct ath_softc *sc,
len, rxs->rate_idx, is_sp);
}
- if (!!(sc->airtime_flags & AIRTIME_USE_RX)) {
- spin_lock_bh(&acq->lock);
- an->airtime_deficit[acno] -= airtime;
- if (an->airtime_deficit[acno] <= 0)
- __ath_tx_queue_tid(sc, ATH_AN_2_TID(an, tidno));
- spin_unlock_bh(&acq->lock);
- }
- ath_debug_airtime(sc, an, airtime, 0);
+ ieee80211_sta_register_airtime(sta, tidno, 0, airtime);
exit:
rcu_read_unlock();
}
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 25b3fc82d4ac..d150db99451f 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -113,44 +113,14 @@ void ath_txq_unlock_complete(struct ath_softc *sc, struct ath_txq *txq)
ath_tx_status(hw, skb);
}
-void __ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
-{
- struct ath_vif *avp = (struct ath_vif *) tid->an->vif->drv_priv;
- struct ath_chanctx *ctx = avp->chanctx;
- struct ath_acq *acq;
- struct list_head *tid_list;
- u8 acno = TID_TO_WME_AC(tid->tidno);
-
- if (!ctx || !list_empty(&tid->list))
- return;
-
-
- acq = &ctx->acq[acno];
- if ((sc->airtime_flags & AIRTIME_USE_NEW_QUEUES) &&
- tid->an->airtime_deficit[acno] > 0)
- tid_list = &acq->acq_new;
- else
- tid_list = &acq->acq_old;
-
- list_add_tail(&tid->list, tid_list);
-}
-
void ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
{
- struct ath_vif *avp = (struct ath_vif *) tid->an->vif->drv_priv;
- struct ath_chanctx *ctx = avp->chanctx;
- struct ath_acq *acq;
+ struct ieee80211_txq *queue =
+ container_of((void *)tid, struct ieee80211_txq, drv_priv);
- if (!ctx || !list_empty(&tid->list))
- return;
-
- acq = &ctx->acq[TID_TO_WME_AC(tid->tidno)];
- spin_lock_bh(&acq->lock);
- __ath_tx_queue_tid(sc, tid);
- spin_unlock_bh(&acq->lock);
+ ieee80211_return_txq(sc->hw, queue);
}
-
void ath9k_wake_tx_queue(struct ieee80211_hw *hw, struct ieee80211_txq *queue)
{
struct ath_softc *sc = hw->priv;
@@ -163,11 +133,7 @@ void ath9k_wake_tx_queue(struct ieee80211_hw *hw, struct ieee80211_txq *queue)
tid->tidno);
ath_txq_lock(sc, txq);
-
- tid->has_queued = true;
- ath_tx_queue_tid(sc, tid);
ath_txq_schedule(sc, txq);
-
ath_txq_unlock(sc, txq);
}
@@ -217,8 +183,8 @@ static void ath_txq_skb_done(struct ath_softc *sc, struct ath_txq *txq,
return ATH_AN_2_TID(an, tidno);
}
-static struct sk_buff *
-ath_tid_pull(struct ath_atx_tid *tid)
+static int
+ath_tid_pull(struct ath_atx_tid *tid, struct sk_buff **skbuf)
{
struct ieee80211_txq *txq = container_of((void*)tid, struct ieee80211_txq, drv_priv);
struct ath_softc *sc = tid->an->sc;
@@ -229,20 +195,16 @@ static void ath_txq_skb_done(struct ath_softc *sc, struct ath_txq *txq,
};
struct sk_buff *skb;
struct ath_frame_info *fi;
- int q;
-
- if (!tid->has_queued)
- return NULL;
+ int q, ret;
skb = ieee80211_tx_dequeue(hw, txq);
- if (!skb) {
- tid->has_queued = false;
- return NULL;
- }
+ if (!skb)
+ return -ENOENT;
- if (ath_tx_prepare(hw, skb, &txctl)) {
+ ret = ath_tx_prepare(hw, skb, &txctl);
+ if (ret) {
ieee80211_free_txskb(hw, skb);
- return NULL;
+ return ret;
}
q = skb_get_queue_mapping(skb);
@@ -252,24 +214,19 @@ static void ath_txq_skb_done(struct ath_softc *sc, struct ath_txq *txq,
++tid->txq->pending_frames;
}
- return skb;
-}
-
-
-static bool ath_tid_has_buffered(struct ath_atx_tid *tid)
-{
- return !skb_queue_empty(&tid->retry_q) || tid->has_queued;
+ *skbuf = skb;
+ return 0;
}
-static struct sk_buff *ath_tid_dequeue(struct ath_atx_tid *tid)
+static int ath_tid_dequeue(struct ath_atx_tid *tid,
+ struct sk_buff **skb)
{
- struct sk_buff *skb;
-
- skb = __skb_dequeue(&tid->retry_q);
- if (!skb)
- skb = ath_tid_pull(tid);
+ int ret = 0;
+ *skb = __skb_dequeue(&tid->retry_q);
+ if (!*skb)
+ ret = ath_tid_pull(tid, skb);
- return skb;
+ return ret;
}
static void ath_tx_flush_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
@@ -365,11 +322,12 @@ static void ath_tid_drain(struct ath_softc *sc, struct ath_txq *txq,
struct list_head bf_head;
struct ath_tx_status ts;
struct ath_frame_info *fi;
+ int ret;
memset(&ts, 0, sizeof(ts));
INIT_LIST_HEAD(&bf_head);
- while ((skb = ath_tid_dequeue(tid))) {
+ while ((ret = ath_tid_dequeue(tid, &skb)) == 0) {
fi = get_frame_info(skb);
bf = fi->bf;
@@ -681,7 +639,6 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
skb_queue_splice_tail(&bf_pending, &tid->retry_q);
if (!an->sleeping) {
ath_tx_queue_tid(sc, tid);
-
if (ts->ts_status & (ATH9K_TXERR_FILT | ATH9K_TXERR_XRETRY))
tid->clear_ps_filter = true;
}
@@ -708,11 +665,11 @@ static bool bf_is_ampdu_not_probing(struct ath_buf *bf)
return bf_isampdu(bf) && !(info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE);
}
-static void ath_tx_count_airtime(struct ath_softc *sc, struct ath_node *an,
- struct ath_atx_tid *tid, struct ath_buf *bf,
+static void ath_tx_count_airtime(struct ath_softc *sc,
+ struct ieee80211_sta *sta,
+ struct ath_buf *bf,
struct ath_tx_status *ts)
{
- struct ath_txq *txq = tid->txq;
u32 airtime = 0;
int i;
@@ -722,17 +679,7 @@ static void ath_tx_count_airtime(struct ath_softc *sc, struct ath_node *an,
airtime += rate_dur * bf->rates[i].count;
}
- if (sc->airtime_flags & AIRTIME_USE_TX) {
- int q = txq->mac80211_qnum;
- struct ath_acq *acq = &sc->cur_chan->acq[q];
-
- spin_lock_bh(&acq->lock);
- an->airtime_deficit[q] -= airtime;
- if (an->airtime_deficit[q] <= 0)
- __ath_tx_queue_tid(sc, tid);
- spin_unlock_bh(&acq->lock);
- }
- ath_debug_airtime(sc, an, 0, airtime);
+ ieee80211_sta_register_airtime(sta, ts->tid, airtime, 0);
}
static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
@@ -762,7 +709,7 @@ static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
if (sta) {
struct ath_node *an = (struct ath_node *)sta->drv_priv;
tid = ath_get_skb_tid(sc, an, bf->bf_mpdu);
- ath_tx_count_airtime(sc, an, tid, bf, ts);
+ ath_tx_count_airtime(sc, sta, bf, ts);
if (ts->ts_status & (ATH9K_TXERR_FILT | ATH9K_TXERR_XRETRY))
tid->clear_ps_filter = true;
}
@@ -946,20 +893,21 @@ static int ath_compute_num_delims(struct ath_softc *sc, struct ath_atx_tid *tid,
return ndelim;
}
-static struct ath_buf *
+static int
ath_tx_get_tid_subframe(struct ath_softc *sc, struct ath_txq *txq,
- struct ath_atx_tid *tid)
+ struct ath_atx_tid *tid, struct ath_buf **buf)
{
struct ieee80211_tx_info *tx_info;
struct ath_frame_info *fi;
- struct sk_buff *skb, *first_skb = NULL;
struct ath_buf *bf;
+ struct sk_buff *skb, *first_skb = NULL;
u16 seqno;
+ int ret;
while (1) {
- skb = ath_tid_dequeue(tid);
- if (!skb)
- break;
+ ret = ath_tid_dequeue(tid, &skb);
+ if (ret < 0)
+ return ret;
fi = get_frame_info(skb);
bf = fi->bf;
@@ -991,7 +939,7 @@ static int ath_compute_num_delims(struct ath_softc *sc, struct ath_atx_tid *tid,
if (!(tx_info->flags & IEEE80211_TX_CTL_AMPDU)) {
bf->bf_state.bf_type = 0;
- return bf;
+ break;
}
bf->bf_state.bf_type = BUF_AMPDU | BUF_AGGR;
@@ -1010,7 +958,7 @@ static int ath_compute_num_delims(struct ath_softc *sc, struct ath_atx_tid *tid,
first_skb = skb;
continue;
}
- break;
+ return -EINPROGRESS;
}
if (tid->bar_index > ATH_BA_INDEX(tid->seq_start, seqno)) {
@@ -1027,10 +975,11 @@ static int ath_compute_num_delims(struct ath_softc *sc, struct ath_atx_tid *tid,
if (bf_isampdu(bf))
ath_tx_addto_baw(sc, tid, bf);
- return bf;
+ break;
}
- return NULL;
+ *buf = bf;
+ return 0;
}
static int
@@ -1040,7 +989,7 @@ static int ath_compute_num_delims(struct ath_softc *sc, struct ath_atx_tid *tid,
{
#define PADBYTES(_len) ((4 - ((_len) % 4)) % 4)
struct ath_buf *bf = bf_first, *bf_prev = NULL;
- int nframes = 0, ndelim;
+ int nframes = 0, ndelim, ret;
u16 aggr_limit = 0, al = 0, bpad = 0,
al_delta, h_baw = tid->baw_size / 2;
struct ieee80211_tx_info *tx_info;
@@ -1092,7 +1041,9 @@ static int ath_compute_num_delims(struct ath_softc *sc, struct ath_atx_tid *tid,
bf_prev = bf;
- bf = ath_tx_get_tid_subframe(sc, txq, tid);
+ ret = ath_tx_get_tid_subframe(sc, txq, tid, &bf);
+ if (ret < 0)
+ break;
}
goto finish;
stop:
@@ -1489,7 +1440,7 @@ static void ath_tx_fill_desc(struct ath_softc *sc, struct ath_buf *bf,
struct ath_buf *bf_first)
{
struct ath_buf *bf = bf_first, *bf_prev = NULL;
- int nframes = 0;
+ int nframes = 0, ret;
do {
struct ieee80211_tx_info *tx_info;
@@ -1503,8 +1454,8 @@ static void ath_tx_fill_desc(struct ath_softc *sc, struct ath_buf *bf,
if (nframes >= 2)
break;
- bf = ath_tx_get_tid_subframe(sc, txq, tid);
- if (!bf)
+ ret = ath_tx_get_tid_subframe(sc, txq, tid, &bf);
+ if (ret < 0)
break;
tx_info = IEEE80211_SKB_CB(bf->bf_mpdu);
@@ -1517,30 +1468,27 @@ static void ath_tx_fill_desc(struct ath_softc *sc, struct ath_buf *bf,
} while (1);
}
-static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
- struct ath_atx_tid *tid)
+static int ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
+ struct ath_atx_tid *tid)
{
- struct ath_buf *bf;
+ struct ath_buf *bf = NULL;
struct ieee80211_tx_info *tx_info;
struct list_head bf_q;
- int aggr_len = 0;
+ int aggr_len = 0, ret;
bool aggr;
- if (!ath_tid_has_buffered(tid))
- return false;
-
INIT_LIST_HEAD(&bf_q);
- bf = ath_tx_get_tid_subframe(sc, txq, tid);
- if (!bf)
- return false;
+ ret = ath_tx_get_tid_subframe(sc, txq, tid, &bf);
+ if (ret < 0)
+ return ret;
tx_info = IEEE80211_SKB_CB(bf->bf_mpdu);
aggr = !!(tx_info->flags & IEEE80211_TX_CTL_AMPDU);
if ((aggr && txq->axq_ampdu_depth >= ATH_AGGR_MIN_QDEPTH) ||
(!aggr && txq->axq_depth >= ATH_NON_AGGR_MIN_QDEPTH)) {
__skb_queue_tail(&tid->retry_q, bf->bf_mpdu);
- return false;
+ return -EBUSY;
}
ath_set_rates(tid->an->vif, tid->an->sta, bf);
@@ -1550,7 +1498,7 @@ static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
ath_tx_form_burst(sc, txq, tid, &bf_q, bf);
if (list_empty(&bf_q))
- return false;
+ return -EAGAIN;
if (tid->clear_ps_filter || tid->an->no_ps_filter) {
tid->clear_ps_filter = false;
@@ -1559,7 +1507,7 @@ static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
ath_tx_fill_desc(sc, bf, txq, aggr_len);
ath_tx_txqaddbuf(sc, txq, &bf_q, false);
- return true;
+ return 0;
}
int ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta,
@@ -1622,28 +1570,16 @@ void ath_tx_aggr_sleep(struct ieee80211_sta *sta, struct ath_softc *sc,
{
struct ath_common *common = ath9k_hw_common(sc->sc_ah);
struct ath_atx_tid *tid;
- struct ath_txq *txq;
int tidno;
ath_dbg(common, XMIT, "%s called\n", __func__);
for (tidno = 0; tidno < IEEE80211_NUM_TIDS; tidno++) {
tid = ath_node_to_tid(an, tidno);
- txq = tid->txq;
-
- ath_txq_lock(sc, txq);
-
- if (list_empty(&tid->list)) {
- ath_txq_unlock(sc, txq);
- continue;
- }
if (!skb_queue_empty(&tid->retry_q))
ieee80211_sta_set_buffered(sta, tid->tidno, true);
- list_del_init(&tid->list);
-
- ath_txq_unlock(sc, txq);
}
}
@@ -1662,11 +1598,12 @@ void ath_tx_aggr_wakeup(struct ath_softc *sc, struct ath_node *an)
ath_txq_lock(sc, txq);
tid->clear_ps_filter = true;
- if (ath_tid_has_buffered(tid)) {
+ if (!skb_queue_empty(&tid->retry_q)) {
ath_tx_queue_tid(sc, tid);
ath_txq_schedule(sc, txq);
}
ath_txq_unlock_complete(sc, txq);
+
}
}
@@ -1697,9 +1634,9 @@ void ath9k_release_buffered_frames(struct ieee80211_hw *hw,
struct ath_txq *txq = sc->tx.uapsdq;
struct ieee80211_tx_info *info;
struct list_head bf_q;
- struct ath_buf *bf_tail = NULL, *bf;
+ struct ath_buf *bf_tail = NULL, *bf = NULL;
int sent = 0;
- int i;
+ int i, ret;
INIT_LIST_HEAD(&bf_q);
for (i = 0; tids && nframes; i++, tids >>= 1) {
@@ -1712,8 +1649,9 @@ void ath9k_release_buffered_frames(struct ieee80211_hw *hw,
ath_txq_lock(sc, tid->txq);
while (nframes > 0) {
- bf = ath_tx_get_tid_subframe(sc, sc->tx.uapsdq, tid);
- if (!bf)
+ ret = ath_tx_get_tid_subframe(sc, sc->tx.uapsdq,
+ tid, &bf);
+ if (ret < 0)
break;
ath9k_set_moredata(sc, bf, true);
@@ -1979,11 +1917,11 @@ void ath_tx_cleanupq(struct ath_softc *sc, struct ath_txq *txq)
*/
void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
{
+ struct ieee80211_hw *hw = sc->hw;
struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+ struct ieee80211_txq *queue;
struct ath_atx_tid *tid;
- struct list_head *tid_list;
- struct ath_acq *acq;
- bool active = AIRTIME_ACTIVE(sc->airtime_flags);
+ int ret;
if (txq->mac80211_qnum < 0)
return;
@@ -1991,58 +1929,26 @@ void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
if (test_bit(ATH_OP_HW_RESET, &common->op_flags))
return;
+ ieee80211_txq_schedule_start(hw, txq->mac80211_qnum);
spin_lock_bh(&sc->chan_lock);
rcu_read_lock();
- acq = &sc->cur_chan->acq[txq->mac80211_qnum];
if (sc->cur_chan->stopped)
goto out;
-begin:
- tid_list = &acq->acq_new;
- if (list_empty(tid_list)) {
- tid_list = &acq->acq_old;
- if (list_empty(tid_list))
- goto out;
- }
- tid = list_first_entry(tid_list, struct ath_atx_tid, list);
-
- if (active && tid->an->airtime_deficit[txq->mac80211_qnum] <= 0) {
- spin_lock_bh(&acq->lock);
- tid->an->airtime_deficit[txq->mac80211_qnum] += ATH_AIRTIME_QUANTUM;
- list_move_tail(&tid->list, &acq->acq_old);
- spin_unlock_bh(&acq->lock);
- goto begin;
- }
-
- if (!ath_tid_has_buffered(tid)) {
- spin_lock_bh(&acq->lock);
- if ((tid_list == &acq->acq_new) && !list_empty(&acq->acq_old))
- list_move_tail(&tid->list, &acq->acq_old);
- else {
- list_del_init(&tid->list);
- }
- spin_unlock_bh(&acq->lock);
- goto begin;
- }
+ while ((queue = ieee80211_next_txq(hw, txq->mac80211_qnum))) {
+ tid = (struct ath_atx_tid *)queue->drv_priv;
+ ret = ath_tx_sched_aggr(sc, txq, tid);
+ ath_dbg(common, QUEUE, "ath_tx_sched_aggr returned %d\n", ret);
- /*
- * If we succeed in scheduling something, immediately restart to make
- * sure we keep the HW busy.
- */
- if(ath_tx_sched_aggr(sc, txq, tid)) {
- if (!active) {
- spin_lock_bh(&acq->lock);
- list_move_tail(&tid->list, &acq->acq_old);
- spin_unlock_bh(&acq->lock);
- }
- goto begin;
+ ieee80211_return_txq(hw, queue);
}
out:
rcu_read_unlock();
spin_unlock_bh(&sc->chan_lock);
+ ieee80211_txq_schedule_end(hw, txq->mac80211_qnum);
}
void ath_txq_schedule_all(struct ath_softc *sc)
@@ -2886,9 +2792,6 @@ void ath_tx_node_init(struct ath_softc *sc, struct ath_node *an)
struct ath_atx_tid *tid;
int tidno, acno;
- for (acno = 0; acno < IEEE80211_NUM_ACS; acno++)
- an->airtime_deficit[acno] = ATH_AIRTIME_QUANTUM;
-
for (tidno = 0; tidno < IEEE80211_NUM_TIDS; tidno++) {
tid = ath_node_to_tid(an, tidno);
tid->an = an;
@@ -2898,7 +2801,6 @@ void ath_tx_node_init(struct ath_softc *sc, struct ath_node *an)
tid->baw_head = tid->baw_tail = 0;
tid->active = false;
tid->clear_ps_filter = true;
- tid->has_queued = false;
__skb_queue_head_init(&tid->retry_q);
INIT_LIST_HEAD(&tid->list);
acno = TID_TO_WME_AC(tidno);
--
1.9.1
From: Toke Høiland-Jørgensen <[email protected]>
ath10k maintains common txqs list for all stations. This txq
management can be removed by migrating to mac80211 txq APIs
and let mac80211 handle txqs reordering based on reported airtime.
By doing this, txq fairness maintained in ath10k i.e processing
N frames per txq is removed. By adapting to mac80211 APIs,
ath10k will support mac80211 based airtime fairness algorithm.
Tested-by: Venkateswara Naralasetty <[email protected]>
Co-Developed-by: Rajkumar Manoharan <[email protected]>
Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
Signed-off-by: Rajkumar Manoharan <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.c | 2 -
drivers/net/wireless/ath/ath10k/core.h | 6 +-
drivers/net/wireless/ath/ath10k/htc.h | 1 -
drivers/net/wireless/ath/ath10k/htt_rx.c | 8 +++
drivers/net/wireless/ath/ath10k/mac.c | 98 ++++++++++++++------------------
5 files changed, 54 insertions(+), 61 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index da607febfd82..c8dbbfa901af 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -3065,9 +3065,7 @@ struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
mutex_init(&ar->conf_mutex);
spin_lock_init(&ar->data_lock);
- spin_lock_init(&ar->txqs_lock);
- INIT_LIST_HEAD(&ar->txqs);
INIT_LIST_HEAD(&ar->peers);
init_waitqueue_head(&ar->peer_mapping_wq);
init_waitqueue_head(&ar->htt.empty_tx_wq);
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 042418097cf9..45afa813555c 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -90,6 +90,9 @@
/* The magic used by QCA spec */
#define ATH10K_SMBIOS_BDF_EXT_MAGIC "BDF_"
+/* Default Airtime weight multipler (Tuned for multiclient performance) */
+#define ATH10K_AIRTIME_WEIGHT_MULTIPLIER 4
+
struct ath10k;
static inline const char *ath10k_bus_str(enum ath10k_bus bus)
@@ -1062,10 +1065,7 @@ struct ath10k {
/* protects shared structure data */
spinlock_t data_lock;
- /* protects: ar->txqs, artxq->list */
- spinlock_t txqs_lock;
- struct list_head txqs;
struct list_head arvifs;
struct list_head peers;
struct ath10k_peer *peer_map[ATH10K_MAX_NUM_PEER_IDS];
diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h
index 51fda6c23f69..cb30add7dd33 100644
--- a/drivers/net/wireless/ath/ath10k/htc.h
+++ b/drivers/net/wireless/ath/ath10k/htc.h
@@ -51,7 +51,6 @@
*/
#define HTC_HOST_MAX_MSG_PER_RX_BUNDLE 8
-#define HTC_HOST_MAX_MSG_PER_TX_BUNDLE 16
enum ath10k_htc_tx_flags {
ATH10K_HTC_FLAG_NEED_CREDIT_UPDATE = 0x01,
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index ffec98f7be50..35738fc84271 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2379,6 +2379,7 @@ static void ath10k_htt_rx_tx_fetch_ind(struct ath10k *ar, struct sk_buff *skb)
u8 tid;
int ret;
int i;
+ bool may_tx;
ath10k_dbg(ar, ATH10K_DBG_HTT, "htt rx tx fetch ind\n");
@@ -2451,8 +2452,13 @@ static void ath10k_htt_rx_tx_fetch_ind(struct ath10k *ar, struct sk_buff *skb)
num_msdus = 0;
num_bytes = 0;
+ ieee80211_txq_schedule_start(hw, txq->ac);
+ may_tx = ieee80211_txq_may_transmit(hw, txq);
while (num_msdus < max_num_msdus &&
num_bytes < max_num_bytes) {
+ if (!may_tx)
+ break;
+
ret = ath10k_mac_tx_push_txq(hw, txq);
if (ret < 0)
break;
@@ -2460,6 +2466,8 @@ static void ath10k_htt_rx_tx_fetch_ind(struct ath10k *ar, struct sk_buff *skb)
num_msdus++;
num_bytes += ret;
}
+ ieee80211_return_txq(hw, txq);
+ ieee80211_txq_schedule_end(hw, txq->ac);
record->num_msdus = cpu_to_le16(num_msdus);
record->num_bytes = cpu_to_le32(num_bytes);
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index a1c2801ded10..65ca5717ee0f 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3889,7 +3889,6 @@ static void ath10k_mac_txq_init(struct ieee80211_txq *txq)
static void ath10k_mac_txq_unref(struct ath10k *ar, struct ieee80211_txq *txq)
{
- struct ath10k_txq *artxq;
struct ath10k_skb_cb *cb;
struct sk_buff *msdu;
int msdu_id;
@@ -3897,12 +3896,6 @@ static void ath10k_mac_txq_unref(struct ath10k *ar, struct ieee80211_txq *txq)
if (!txq)
return;
- artxq = (void *)txq->drv_priv;
- spin_lock_bh(&ar->txqs_lock);
- if (!list_empty(&artxq->list))
- list_del_init(&artxq->list);
- spin_unlock_bh(&ar->txqs_lock);
-
spin_lock_bh(&ar->htt.tx_lock);
idr_for_each_entry(&ar->htt.pending_tx, msdu, msdu_id) {
cb = ATH10K_SKB_CB(msdu);
@@ -3942,7 +3935,6 @@ static bool ath10k_mac_tx_can_push(struct ieee80211_hw *hw,
struct ath10k_txq *artxq = (void *)txq->drv_priv;
/* No need to get locks */
-
if (ar->htt.tx_q_state.mode == HTT_TX_MODE_SWITCH_PUSH)
return true;
@@ -4029,48 +4021,45 @@ int ath10k_mac_tx_push_txq(struct ieee80211_hw *hw,
return skb_len;
}
-void ath10k_mac_tx_push_pending(struct ath10k *ar)
+static int ath10k_mac_schedule_txq(struct ieee80211_hw *hw, u32 ac)
{
- struct ieee80211_hw *hw = ar->hw;
struct ieee80211_txq *txq;
- struct ath10k_txq *artxq;
- struct ath10k_txq *last;
- int ret;
- int max;
-
- if (ar->htt.num_pending_tx >= (ar->htt.max_num_pending_tx / 2))
- return;
-
- spin_lock_bh(&ar->txqs_lock);
- rcu_read_lock();
-
- last = list_last_entry(&ar->txqs, struct ath10k_txq, list);
- while (!list_empty(&ar->txqs)) {
- artxq = list_first_entry(&ar->txqs, struct ath10k_txq, list);
- txq = container_of((void *)artxq, struct ieee80211_txq,
- drv_priv);
+ int ret = 0;
- /* Prevent aggressive sta/tid taking over tx queue */
- max = HTC_HOST_MAX_MSG_PER_TX_BUNDLE;
- ret = 0;
- while (ath10k_mac_tx_can_push(hw, txq) && max--) {
+ ieee80211_txq_schedule_start(hw, ac);
+ while ((txq = ieee80211_next_txq(hw, ac))) {
+ while (ath10k_mac_tx_can_push(hw, txq)) {
ret = ath10k_mac_tx_push_txq(hw, txq);
if (ret < 0)
break;
}
+ ieee80211_return_txq(hw, txq);
+ ath10k_htt_tx_txq_update(hw, txq);
+ if (ret == -EBUSY)
+ break;
+ }
+ ieee80211_txq_schedule_end(hw, ac);
+
+ return ret;
+}
- list_del_init(&artxq->list);
- if (ret != -ENOENT)
- list_add_tail(&artxq->list, &ar->txqs);
+void ath10k_mac_tx_push_pending(struct ath10k *ar)
+{
+ struct ieee80211_hw *hw = ar->hw;
+ u32 ac;
- ath10k_htt_tx_txq_update(hw, txq);
+ if (ar->htt.tx_q_state.mode != HTT_TX_MODE_SWITCH_PUSH)
+ return;
- if (artxq == last || (ret < 0 && ret != -ENOENT))
+ if (ar->htt.num_pending_tx >= (ar->htt.max_num_pending_tx / 2))
+ return;
+
+ rcu_read_lock();
+ for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) {
+ if (ath10k_mac_schedule_txq(hw, ac) == -EBUSY)
break;
}
-
rcu_read_unlock();
- spin_unlock_bh(&ar->txqs_lock);
}
EXPORT_SYMBOL(ath10k_mac_tx_push_pending);
@@ -4309,31 +4298,28 @@ static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw,
struct ieee80211_txq *txq)
{
struct ath10k *ar = hw->priv;
- struct ath10k_txq *artxq = (void *)txq->drv_priv;
- struct ieee80211_txq *f_txq;
- struct ath10k_txq *f_artxq;
- int ret = 0;
- int max = HTC_HOST_MAX_MSG_PER_TX_BUNDLE;
+ int ret;
+ u8 ac;
- spin_lock_bh(&ar->txqs_lock);
- if (list_empty(&artxq->list))
- list_add_tail(&artxq->list, &ar->txqs);
+ ath10k_htt_tx_txq_update(hw, txq);
+ if (ar->htt.tx_q_state.mode != HTT_TX_MODE_SWITCH_PUSH)
+ return;
- f_artxq = list_first_entry(&ar->txqs, struct ath10k_txq, list);
- f_txq = container_of((void *)f_artxq, struct ieee80211_txq, drv_priv);
- list_del_init(&f_artxq->list);
+ ac = txq->ac;
+ ieee80211_txq_schedule_start(hw, ac);
+ txq = ieee80211_next_txq(hw, ac);
+ if (!txq)
+ goto out;
- while (ath10k_mac_tx_can_push(hw, f_txq) && max--) {
- ret = ath10k_mac_tx_push_txq(hw, f_txq);
+ while (ath10k_mac_tx_can_push(hw, txq)) {
+ ret = ath10k_mac_tx_push_txq(hw, txq);
if (ret < 0)
break;
}
- if (ret != -ENOENT)
- list_add_tail(&f_artxq->list, &ar->txqs);
- spin_unlock_bh(&ar->txqs_lock);
-
- ath10k_htt_tx_txq_update(hw, f_txq);
+ ieee80211_return_txq(hw, txq);
ath10k_htt_tx_txq_update(hw, txq);
+out:
+ ieee80211_txq_schedule_end(hw, ac);
}
/* Must not be called with conf_mutex held as workers can use that also. */
@@ -8687,6 +8673,8 @@ int ath10k_mac_register(struct ath10k *ar)
wiphy_ext_feature_set(ar->hw->wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);
+ ar->hw->weight_multiplier = ATH10K_AIRTIME_WEIGHT_MULTIPLIER;
+
ret = ieee80211_register_hw(ar->hw);
if (ret) {
ath10k_err(ar, "failed to register ieee80211: %d\n", ret);
--
1.9.1
From: Kan Yan <[email protected]>
Transmit airtime will be estimated from last tx rate used.
Firmware report tx rate by peer stats. Airtime is computed
on tx path and the same will be reported to mac80211 upon
tx completion.
This change is based on Kan's orginal commit in Chromium tree
("CHROMIUM: ath10k: Implementing airtime fairness based TX scheduler")
ref: https://chromium-review.googlesource.com/588190
Signed-off-by: Kan Yan <[email protected]>
[[email protected]: ported only the airtime computation]
Signed-off-by: Rajkumar Manoharan <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.h | 2 ++
drivers/net/wireless/ath/ath10k/htt_rx.c | 1 +
drivers/net/wireless/ath/ath10k/mac.c | 57 ++++++++++++++++++++++++++++++--
drivers/net/wireless/ath/ath10k/txrx.c | 4 +++
4 files changed, 61 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 45afa813555c..883bf4613ab3 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -126,6 +126,7 @@ struct ath10k_skb_cb {
u8 flags;
u8 eid;
u16 msdu_id;
+ u16 airtime_est;
struct ieee80211_vif *vif;
struct ieee80211_txq *txq;
} __packed;
@@ -496,6 +497,7 @@ struct ath10k_sta {
u32 smps;
u16 peer_id;
struct rate_info txrate;
+ u32 last_tx_bitrate;
struct work_struct update_wk;
u64 rx_duration;
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 35738fc84271..26134bed52d2 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2808,6 +2808,7 @@ static inline int ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 rate)
arsta->txrate.nss = txrate.nss;
arsta->txrate.bw = ath10k_bw_to_mac80211_bw(txrate.bw);
+ arsta->last_tx_bitrate = cfg80211_calculate_bitrate(&arsta->txrate);
if (ath10k_debug_is_extd_tx_stats_enabled(ar))
ath10k_accumulate_per_peer_tx_stats(ar, arsta, peer_stats,
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 65ca5717ee0f..e890b475b7fd 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3543,7 +3543,7 @@ static void ath10k_tx_h_add_p2p_noa_ie(struct ath10k *ar,
static void ath10k_mac_tx_h_fill_cb(struct ath10k *ar,
struct ieee80211_vif *vif,
struct ieee80211_txq *txq,
- struct sk_buff *skb)
+ struct sk_buff *skb, u16 airtime)
{
struct ieee80211_hdr *hdr = (void *)skb->data;
struct ath10k_skb_cb *cb = ATH10K_SKB_CB(skb);
@@ -3560,6 +3560,7 @@ static void ath10k_mac_tx_h_fill_cb(struct ath10k *ar,
cb->vif = vif;
cb->txq = txq;
+ cb->airtime_est = airtime;
}
bool ath10k_mac_tx_frm_has_freq(struct ath10k *ar)
@@ -3947,6 +3948,49 @@ static bool ath10k_mac_tx_can_push(struct ieee80211_hw *hw,
return false;
}
+/* Return estimated airtime in microsecond, which is calculated using last
+ * reported TX rate. This is just a rough estimation because host driver has no
+ * knowledge of the actual transmit rate, retries or aggregation. If actual
+ * airtime can be reported by firmware, then delta between estimated and actual
+ * airtime can be adjusted from deficit.
+ */
+#define IEEE80211_ATF_OVERHEAD 100 /* IFS + some slot time */
+#define IEEE80211_ATF_OVERHEAD_IFS 16 /* IFS only */
+static u16 ath10k_mac_update_airtime(struct ath10k *ar,
+ struct ieee80211_txq *txq,
+ struct sk_buff *skb)
+{
+ struct ath10k_sta *arsta;
+ u32 pktlen;
+ u16 airtime = 0;
+
+ if (!txq || !txq->sta)
+ return airtime;
+
+ spin_lock_bh(&ar->data_lock);
+ arsta = (struct ath10k_sta *)txq->sta->drv_priv;
+
+ pktlen = skb->len + 38; /* Assume MAC header 30, SNAP 8 for most case */
+ if (arsta->last_tx_bitrate) {
+ /* airtime in us, last_tx_bitrate in 100kbps */
+ airtime = (pktlen * 8 * (1000 / 100))
+ / arsta->last_tx_bitrate;
+ /* overhead for media access time and IFS */
+ airtime += IEEE80211_ATF_OVERHEAD_IFS;
+ } else {
+ /* This is mostly for throttle excessive BC/MC frames, and the
+ * airtime/rate doesn't need be exact. Airtime of BC/MC frames
+ * in 2G get some discount, which helps prevent very low rate
+ * frames from being blocked for too long.
+ */
+ airtime = (pktlen * 8 * (1000 / 100)) / 60; /* 6M */
+ airtime += IEEE80211_ATF_OVERHEAD;
+ }
+ spin_unlock_bh(&ar->data_lock);
+
+ return airtime;
+}
+
int ath10k_mac_tx_push_txq(struct ieee80211_hw *hw,
struct ieee80211_txq *txq)
{
@@ -3962,6 +4006,7 @@ int ath10k_mac_tx_push_txq(struct ieee80211_hw *hw,
size_t skb_len;
bool is_mgmt, is_presp;
int ret;
+ u16 airtime;
spin_lock_bh(&ar->htt.tx_lock);
ret = ath10k_htt_tx_inc_pending(htt);
@@ -3979,7 +4024,8 @@ int ath10k_mac_tx_push_txq(struct ieee80211_hw *hw,
return -ENOENT;
}
- ath10k_mac_tx_h_fill_cb(ar, vif, txq, skb);
+ airtime = ath10k_mac_update_airtime(ar, txq, skb);
+ ath10k_mac_tx_h_fill_cb(ar, vif, txq, skb, airtime);
skb_len = skb->len;
txmode = ath10k_mac_tx_h_get_txmode(ar, vif, sta, skb);
@@ -4246,8 +4292,10 @@ static void ath10k_mac_op_tx(struct ieee80211_hw *hw,
bool is_mgmt;
bool is_presp;
int ret;
+ u16 airtime;
- ath10k_mac_tx_h_fill_cb(ar, vif, txq, skb);
+ airtime = ath10k_mac_update_airtime(ar, txq, skb);
+ ath10k_mac_tx_h_fill_cb(ar, vif, txq, skb, airtime);
txmode = ath10k_mac_tx_h_get_txmode(ar, vif, sta, skb);
txpath = ath10k_mac_tx_h_get_txpath(ar, skb, txmode);
@@ -8564,6 +8612,9 @@ int ath10k_mac_register(struct ath10k *ar)
wiphy_ext_feature_set(ar->hw->wiphy,
NL80211_EXT_FEATURE_ACK_SIGNAL_SUPPORT);
+ if (ath10k_peer_stats_enabled(ar))
+ wiphy_ext_feature_set(ar->hw->wiphy,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS);
/*
* on LL hardware queues are managed entirely by the FW
* so we only advertise to mac we can do the queues thing
diff --git a/drivers/net/wireless/ath/ath10k/txrx.c b/drivers/net/wireless/ath/ath10k/txrx.c
index 23606b6972d0..8e7c416cd330 100644
--- a/drivers/net/wireless/ath/ath10k/txrx.c
+++ b/drivers/net/wireless/ath/ath10k/txrx.c
@@ -95,6 +95,10 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
wake_up(&htt->empty_tx_wq);
spin_unlock_bh(&htt->tx_lock);
+ if (txq && txq->sta)
+ ieee80211_sta_register_airtime(txq->sta, txq->tid,
+ skb_cb->airtime_est, 0);
+
if (ar->dev_type != ATH10K_DEV_TYPE_HL)
dma_unmap_single(dev, skb_cb->paddr, msdu->len, DMA_TO_DEVICE);
--
1.9.1
On 2018-11-12 23:51, Rajkumar Manoharan wrote:
> From: Toke Høiland-Jørgensen <[email protected]>
>
> This adds airtime accounting and scheduling to the mac80211 TXQ
> scheduler. A new callback, ieee80211_sta_register_airtime(), is added
> that drivers can call to report airtime usage for stations.
>
> When airtime information is present, mac80211 will schedule TXQs
> (through ieee80211_next_txq()) in a way that enforces airtime fairness
> between active stations. This scheduling works the same way as the ath9k
> in-driver airtime fairness scheduling. If no airtime usage is reported
> by the driver, the scheduler will default to round-robin scheduling.
>
> For drivers that don't control TXQ scheduling in software, a new API
> function, ieee80211_txq_may_transmit(), is added which the driver can use
> to check if the TXQ is eligible for transmission, or should be throttled to
> enforce fairness. Calls to this function must also be enclosed in
> ieee80211_txq_schedule_{start,end}() calls to ensure proper locking.
>
> The API ieee80211_txq_may_transmit() also ensures that TXQ list will be
> aligned aginst driver's own round-robin scheduler list. i.e it rotates
> the TXQ list till it makes the requested node becomes the first entry
> in TXQ list. Thus both the TXQ list and driver's list are in sync.
>
> Co-Developed-by: Rajkumar Manoharan <[email protected]>
> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
> Signed-off-by: Rajkumar Manoharan <[email protected]>
> ---
> include/net/mac80211.h | 59 ++++++++++++++++++++++++++++++
> net/mac80211/cfg.c | 3 ++
> net/mac80211/debugfs.c | 3 ++
> net/mac80211/debugfs_sta.c | 50 ++++++++++++++++++++++++--
> net/mac80211/ieee80211_i.h | 2 ++
> net/mac80211/main.c | 4 +++
> net/mac80211/sta_info.c | 44 +++++++++++++++++++++--
> net/mac80211/sta_info.h | 13 +++++++
> net/mac80211/status.c | 6 ++++
> net/mac80211/tx.c | 90 +++++++++++++++++++++++++++++++++++++++++++---
> 10 files changed, 264 insertions(+), 10 deletions(-)
>
> diff --git a/net/mac80211/status.c b/net/mac80211/status.c
> index aa4afbf0abaf..a1f1256448f5 100644
> --- a/net/mac80211/status.c
> +++ b/net/mac80211/status.c
> @@ -818,6 +818,12 @@ static void __ieee80211_tx_status(struct ieee80211_hw *hw,
> ieee80211_sta_tx_notify(sta->sdata, (void *) skb->data,
> acked, info->status.tx_time);
>
> + if (info->status.tx_time &&
> + wiphy_ext_feature_isset(local->hw.wiphy,
> + NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
> + ieee80211_sta_register_airtime(&sta->sta, tid,
> + info->status.tx_time, 0);
> +
> if (ieee80211_hw_check(&local->hw, REPORTS_TX_ACK_STATUS)) {
> if (info->flags & IEEE80211_TX_STAT_ACK) {
> if (sta->status_stats.lost_packets)
I think the same is needed in ieee80211_tx_status_ext.
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index 305965283506..3f417e80e041 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -3660,12 +3680,74 @@ void ieee80211_return_txq(struct ieee80211_hw *hw,
> lockdep_assert_held(&local->active_txq_lock[txq->ac]);
>
> if (list_empty(&txqi->schedule_order) &&
> - (!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets))
> - list_add_tail(&txqi->schedule_order,
> - &local->active_txqs[txq->ac]);
> + (!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets)) {
> + /* If airtime accounting is active, always enqueue STAs at the
> + * head of the list to ensure that they only get moved to the
> + * back by the airtime DRR scheduler once they have a negative
> + * deficit. A station that already has a negative deficit will
> + * get immediately moved to the back of the list on the next
> + * call to ieee80211_next_txq().
> + */
> + if (txqi->txq.sta &&
> + wiphy_ext_feature_isset(local->hw.wiphy,
> + NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
> + list_add(&txqi->schedule_order,
> + &local->active_txqs[txq->ac]);
> + else
> + list_add_tail(&txqi->schedule_order,
> + &local->active_txqs[txq->ac]);
> + }
> }
This part doesn't really make much sense to me, but maybe I'm
misunderstanding how the code works.
Let's assume we have a driver like ath9k or mt76, which tries to keep a
number of aggregates in the hardware queue, and the hardware queue is
currently empty.
If the current txq entry is kept at the head of the schedule list,
wouldn't the code just pull from that one over and over again, until
enough packets are transmitted by the hardware and their tx status
processed?
It seems to me that while fairness is still preserved in the long run,
this could lead to rather bursty scheduling, which may not be
particularly latency friendly.
- Felix
Felix Fietkau <[email protected]> writes:
> On 2018-11-12 23:51, Rajkumar Manoharan wrote:
>> From: Toke Høiland-Jørgensen <[email protected]>
>>
>> This adds airtime accounting and scheduling to the mac80211 TXQ
>> scheduler. A new callback, ieee80211_sta_register_airtime(), is added
>> that drivers can call to report airtime usage for stations.
>>
>> When airtime information is present, mac80211 will schedule TXQs
>> (through ieee80211_next_txq()) in a way that enforces airtime fairness
>> between active stations. This scheduling works the same way as the ath9k
>> in-driver airtime fairness scheduling. If no airtime usage is reported
>> by the driver, the scheduler will default to round-robin scheduling.
>>
>> For drivers that don't control TXQ scheduling in software, a new API
>> function, ieee80211_txq_may_transmit(), is added which the driver can use
>> to check if the TXQ is eligible for transmission, or should be throttled to
>> enforce fairness. Calls to this function must also be enclosed in
>> ieee80211_txq_schedule_{start,end}() calls to ensure proper locking.
>>
>> The API ieee80211_txq_may_transmit() also ensures that TXQ list will be
>> aligned aginst driver's own round-robin scheduler list. i.e it rotates
>> the TXQ list till it makes the requested node becomes the first entry
>> in TXQ list. Thus both the TXQ list and driver's list are in sync.
>>
>> Co-Developed-by: Rajkumar Manoharan <[email protected]>
>> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
>> Signed-off-by: Rajkumar Manoharan <[email protected]>
>> ---
>> include/net/mac80211.h | 59 ++++++++++++++++++++++++++++++
>> net/mac80211/cfg.c | 3 ++
>> net/mac80211/debugfs.c | 3 ++
>> net/mac80211/debugfs_sta.c | 50 ++++++++++++++++++++++++--
>> net/mac80211/ieee80211_i.h | 2 ++
>> net/mac80211/main.c | 4 +++
>> net/mac80211/sta_info.c | 44 +++++++++++++++++++++--
>> net/mac80211/sta_info.h | 13 +++++++
>> net/mac80211/status.c | 6 ++++
>> net/mac80211/tx.c | 90 +++++++++++++++++++++++++++++++++++++++++++---
>> 10 files changed, 264 insertions(+), 10 deletions(-)
>>
>> diff --git a/net/mac80211/status.c b/net/mac80211/status.c
>> index aa4afbf0abaf..a1f1256448f5 100644
>> --- a/net/mac80211/status.c
>> +++ b/net/mac80211/status.c
>> @@ -818,6 +818,12 @@ static void __ieee80211_tx_status(struct ieee80211_hw *hw,
>> ieee80211_sta_tx_notify(sta->sdata, (void *) skb->data,
>> acked, info->status.tx_time);
>>
>> + if (info->status.tx_time &&
>> + wiphy_ext_feature_isset(local->hw.wiphy,
>> + NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
>> + ieee80211_sta_register_airtime(&sta->sta, tid,
>> + info->status.tx_time, 0);
>> +
>> if (ieee80211_hw_check(&local->hw, REPORTS_TX_ACK_STATUS)) {
>> if (info->flags & IEEE80211_TX_STAT_ACK) {
>> if (sta->status_stats.lost_packets)
> I think the same is needed in ieee80211_tx_status_ext.
Right, good point.
>> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
>> index 305965283506..3f417e80e041 100644
>> --- a/net/mac80211/tx.c
>> +++ b/net/mac80211/tx.c
>> @@ -3660,12 +3680,74 @@ void ieee80211_return_txq(struct ieee80211_hw *hw,
>> lockdep_assert_held(&local->active_txq_lock[txq->ac]);
>>
>> if (list_empty(&txqi->schedule_order) &&
>> - (!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets))
>> - list_add_tail(&txqi->schedule_order,
>> - &local->active_txqs[txq->ac]);
>> + (!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets)) {
>> + /* If airtime accounting is active, always enqueue STAs at the
>> + * head of the list to ensure that they only get moved to the
>> + * back by the airtime DRR scheduler once they have a negative
>> + * deficit. A station that already has a negative deficit will
>> + * get immediately moved to the back of the list on the next
>> + * call to ieee80211_next_txq().
>> + */
>> + if (txqi->txq.sta &&
>> + wiphy_ext_feature_isset(local->hw.wiphy,
>> + NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
>> + list_add(&txqi->schedule_order,
>> + &local->active_txqs[txq->ac]);
>> + else
>> + list_add_tail(&txqi->schedule_order,
>> + &local->active_txqs[txq->ac]);
>> + }
>> }
> This part doesn't really make much sense to me, but maybe I'm
> misunderstanding how the code works.
> Let's assume we have a driver like ath9k or mt76, which tries to keep a
> number of aggregates in the hardware queue, and the hardware queue is
> currently empty.
> If the current txq entry is kept at the head of the schedule list,
> wouldn't the code just pull from that one over and over again, until
> enough packets are transmitted by the hardware and their tx status
> processed?
> It seems to me that while fairness is still preserved in the long run,
> this could lead to rather bursty scheduling, which may not be
> particularly latency friendly.
Yes, it'll be a bit more bursty when the hardware queue is completely
empty. However, when a TX completion comes back, that will adjust the
deficit of that sta and cause it to be rotated on the next dequeue. This
obviously relies on the fact that the lower-level hardware queue is
sufficiently shallow to not add a lot of latency. But we want that to be
the case anyway. In practice, it works quite well for ath9k, but not so
well for ath10k because it has a large buffer in firmware.
If we requeue the TXQ at the end of the list, a station that is taking
up too much airtime will fail to be throttled properly, so the
queue-at-head is kinda needed to ensure fairness...
-Toke
Hi Rajkumar, Toke,
I found the series (v3,4/6) remove the debugfs remove reset station's
airtime method, and didn't added at here.
Not sure how to help this kind of situation, do I need a separate
patch to fix this, or posting the patch here is fine?
----
From 3a4a856c397345311c9d7f3679828cadc40e6a80 Mon Sep 17 00:00:00 2001
From: Louie Lu <[email protected]>
Date: Thu, 15 Nov 2018 16:13:57 +0800
Subject: [PATCH] mac80211: Add reset for station's airtime
Let user can reset station airtime status by debugfs, it will
reset all airtime deficit to `sta->airtime_weight` and reset rx/tx
airtime accumulate to 0.
diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index 446908ab3f5d..d84d2369a76e 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -233,7 +233,28 @@ static ssize_t sta_airtime_read(struct file
*file, char __user *userbuf,
kfree(buf);
return rv;
}
-STA_OPS(airtime);
+
+/*
+ * FIXME: This *only* reset station airtime, didn't accept input
+ */
+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_agg_status_read(struct file *file, char __user *userbuf,
size_t count, loff_t *ppos)
--
2.18.0
Rajkumar Manoharan <[email protected]> 於 2018年11月13日 週二 上午6:52寫道：
>
> From: Toke Høiland-Jørgensen <[email protected]>
>
> This adds airtime accounting and scheduling to the mac80211 TXQ
> scheduler. A new callback, ieee80211_sta_register_airtime(), is added
> that drivers can call to report airtime usage for stations.
>
> When airtime information is present, mac80211 will schedule TXQs
> (through ieee80211_next_txq()) in a way that enforces airtime fairness
> between active stations. This scheduling works the same way as the ath9k
> in-driver airtime fairness scheduling. If no airtime usage is reported
> by the driver, the scheduler will default to round-robin scheduling.
>
> For drivers that don't control TXQ scheduling in software, a new API
> function, ieee80211_txq_may_transmit(), is added which the driver can use
> to check if the TXQ is eligible for transmission, or should be throttled to
> enforce fairness. Calls to this function must also be enclosed in
> ieee80211_txq_schedule_{start,end}() calls to ensure proper locking.
>
> The API ieee80211_txq_may_transmit() also ensures that TXQ list will be
> aligned aginst driver's own round-robin scheduler list. i.e it rotates
> the TXQ list till it makes the requested node becomes the first entry
> in TXQ list. Thus both the TXQ list and driver's list are in sync.
>
> Co-Developed-by: Rajkumar Manoharan <[email protected]>
> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
> Signed-off-by: Rajkumar Manoharan <[email protected]>
> ---
> include/net/mac80211.h | 59 ++++++++++++++++++++++++++++++
> net/mac80211/cfg.c | 3 ++
> net/mac80211/debugfs.c | 3 ++
> net/mac80211/debugfs_sta.c | 50 ++++++++++++++++++++++++--
> net/mac80211/ieee80211_i.h | 2 ++
> net/mac80211/main.c | 4 +++
> net/mac80211/sta_info.c | 44 +++++++++++++++++++++--
> net/mac80211/sta_info.h | 13 +++++++
> net/mac80211/status.c | 6 ++++
> net/mac80211/tx.c | 90 +++++++++++++++++++++++++++++++++++++++++++---
> 10 files changed, 264 insertions(+), 10 deletions(-)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 18b11c119b7e..c43d615ee9b1 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -2357,6 +2357,9 @@ enum ieee80211_hw_flags {
> * @tx_sk_pacing_shift: Pacing shift to set on TCP sockets when frames from
> * them are encountered. The default should typically not be changed,
> * unless the driver has good reasons for needing more buffers.
> + *
> + * @weight_multipler: Driver specific airtime weight multiplier used while
> + * refilling deficit of each TXQ.
> */
> struct ieee80211_hw {
> struct ieee80211_conf conf;
> @@ -2393,6 +2396,7 @@ struct ieee80211_hw {
> const struct ieee80211_cipher_scheme *cipher_schemes;
> u8 max_nan_de_entries;
> u8 tx_sk_pacing_shift;
> + u8 weight_multiplier;
> };
>
> static inline bool _ieee80211_hw_check(struct ieee80211_hw *hw,
> @@ -5393,6 +5397,34 @@ void ieee80211_sta_block_awake(struct ieee80211_hw *hw,
> void ieee80211_send_eosp_nullfunc(struct ieee80211_sta *pubsta, int tid);
>
> /**
> + * ieee80211_sta_register_airtime - register airtime usage for a sta/tid
> + *
> + * Register airtime usage for a given sta on a given tid. The driver can call
> + * this function to notify mac80211 that a station used a certain amount of
> + * airtime. This information will be used by the TXQ scheduler to schedule
> + * stations in a way that ensures airtime fairness.
> + *
> + * The reported airtime should as a minimum include all time that is spent
> + * transmitting to the remote station, including overhead and padding, but not
> + * including time spent waiting for a TXOP. If the time is not reported by the
> + * hardware it can in some cases be calculated from the rate and known frame
> + * composition. When possible, the time should include any failed transmission
> + * attempts.
> + *
> + * The driver can either call this function synchronously for every packet or
> + * aggregate, or asynchronously as airtime usage information becomes available.
> + * TX and RX airtime can be reported together, or separately by setting one of
> + * them to 0.
> + *
> + * @pubsta: the station
> + * @tid: the TID to register airtime for
> + * @tx_airtime: airtime used during TX (in usec)
> + * @rx_airtime: airtime used during RX (in usec)
> + */
> +void ieee80211_sta_register_airtime(struct ieee80211_sta *pubsta, u8 tid,
> + u32 tx_airtime, u32 rx_airtime);
> +
> +/**
> * ieee80211_iter_keys - iterate keys programmed into the device
> * @hw: pointer obtained from ieee80211_alloc_hw()
> * @vif: virtual interface to iterate, may be %NULL for all
> @@ -6150,6 +6182,33 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
> void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8 ac);
>
> /**
> + * ieee80211_txq_may_transmit - check whether TXQ is allowed to transmit
> + *
> + * This function is used to check whether given txq is allowed to transmit by
> + * the airtime scheduler, and can be used by drivers to access the airtime
> + * fairness accounting without going using the scheduling order enfored by
> + * next_txq().
> + *
> + * Returns %true if the airtime scheduler thinks the TXQ should be allowed to
> + * transmit, and %false if it should be throttled. This function can also have
> + * the side effect of rotating the TXQ in the scheduler rotation, which will
> + * eventually bring the deficit to positive and allow the station to transmit
> + * again.
> + *
> + * The API ieee80211_txq_may_transmit() also ensures that TXQ list will be
> + * aligned aginst driver's own round-robin scheduler list. i.e it rotates
> + * the TXQ list till it makes the requested node becomes the first entry
> + * in TXQ list. Thus both the TXQ list and driver's list are in sync. If this
> + * function returns %true, the driver is expected to schedule packets
> + * for transmission, and then return the TXQ through ieee80211_return_txq().
> + *
> + * @hw: pointer as obtained from ieee80211_alloc_hw()
> + * @txq: pointer obtained from station or virtual interface
> + */
> +bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
> + struct ieee80211_txq *txq);
> +
> +/**
> * ieee80211_txq_get_depth - get pending frame/byte count of given txq
> *
> * The values are not guaranteed to be coherent with regard to each other, i.e.
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index 818aa0060349..57c59e5ceb98 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -1447,6 +1447,9 @@ static int sta_apply_parameters(struct ieee80211_local *local,
> if (ieee80211_vif_is_mesh(&sdata->vif))
> sta_apply_mesh_params(local, sta, params);
>
> + if (params->airtime_weight)
> + sta->airtime_weight = params->airtime_weight;
> +
> /* set the STA state after all sta info from usermode has been set */
> if (test_sta_flag(sta, WLAN_STA_TDLS_PEER) ||
> set & BIT(NL80211_STA_FLAG_ASSOCIATED)) {
> diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
> index 3fe541e358f3..81c5fec2eae7 100644
> --- a/net/mac80211/debugfs.c
> +++ b/net/mac80211/debugfs.c
> @@ -383,6 +383,9 @@ void debugfs_hw_add(struct ieee80211_local *local)
> if (local->ops->wake_tx_queue)
> DEBUGFS_ADD_MODE(aqm, 0600);
>
> + debugfs_create_u16("airtime_flags", 0600,
> + phyd, &local->airtime_flags);
> +
> statsd = debugfs_create_dir("statistics", phyd);
>
> /* if the dir failed, don't put all the other things into the root! */
> diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
> index af5185a836e5..446908ab3f5d 100644
> --- a/net/mac80211/debugfs_sta.c
> +++ b/net/mac80211/debugfs_sta.c
> @@ -181,9 +181,9 @@ static ssize_t sta_aqm_read(struct file *file, char __user *userbuf,
> txqi->tin.tx_bytes,
> txqi->tin.tx_packets,
> txqi->flags,
> - txqi->flags & (1<<IEEE80211_TXQ_STOP) ? "STOP" : "RUN",
> - txqi->flags & (1<<IEEE80211_TXQ_AMPDU) ? " AMPDU" : "",
> - txqi->flags & (1<<IEEE80211_TXQ_NO_AMSDU) ? " NO-AMSDU" : "");
> + test_bit(IEEE80211_TXQ_STOP, &txqi->flags) ? "STOP" : "RUN",
> + test_bit(IEEE80211_TXQ_AMPDU, &txqi->flags) ? " AMPDU" : "",
> + test_bit(IEEE80211_TXQ_NO_AMSDU, &txqi->flags) ? " NO-AMSDU" : "");
> }
>
> rcu_read_unlock();
> @@ -195,6 +195,46 @@ static ssize_t sta_aqm_read(struct file *file, char __user *userbuf,
> }
> STA_OPS(aqm);
>
> +static ssize_t sta_airtime_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 = 200;
> + char *buf = kzalloc(bufsz, GFP_KERNEL), *p = buf;
> + u64 rx_airtime = 0, tx_airtime = 0;
> + s64 deficit[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]);
> + 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;
> +}
> +STA_OPS(airtime);
> +
> static ssize_t sta_agg_status_read(struct file *file, char __user *userbuf,
> size_t count, loff_t *ppos)
> {
> @@ -906,6 +946,10 @@ void ieee80211_sta_debugfs_add(struct sta_info *sta)
> if (local->ops->wake_tx_queue)
> DEBUGFS_ADD(aqm);
>
> + if (wiphy_ext_feature_isset(local->hw.wiphy,
> + NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
> + DEBUGFS_ADD(airtime);
> +
> if (sizeof(sta->driver_buffered_tids) == sizeof(u32))
> debugfs_create_x32("driver_buffered_tids", 0400,
> sta->debugfs_dir,
> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
> index 33763a2eac81..c5b6ba571288 100644
> --- a/net/mac80211/ieee80211_i.h
> +++ b/net/mac80211/ieee80211_i.h
> @@ -1137,6 +1137,8 @@ struct ieee80211_local {
> struct list_head active_txqs[IEEE80211_NUM_ACS];
> u16 schedule_round[IEEE80211_NUM_ACS];
>
> + u16 airtime_flags;
> +
> const struct ieee80211_ops *ops;
>
> /*
> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
> index ba64757afea4..3710727fa9c1 100644
> --- a/net/mac80211/main.c
> +++ b/net/mac80211/main.c
> @@ -667,6 +667,7 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
> INIT_LIST_HEAD(&local->active_txqs[i]);
> spin_lock_init(&local->active_txq_lock[i]);
> }
> + local->airtime_flags = AIRTIME_USE_TX | AIRTIME_USE_RX;
>
> INIT_LIST_HEAD(&local->chanctx_list);
> mutex_init(&local->chanctx_mtx);
> @@ -1153,6 +1154,9 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
> if (!local->hw.max_nan_de_entries)
> local->hw.max_nan_de_entries = IEEE80211_MAX_NAN_INSTANCE_ID;
>
> + if (!local->hw.weight_multiplier)
> + local->hw.weight_multiplier = 1;
> +
> result = ieee80211_wep_init(local);
> if (result < 0)
> wiphy_debug(local->hw.wiphy, "Failed to initialize wep: %d\n",
> diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
> index c2f5cb7df54f..25d47b2af7a9 100644
> --- a/net/mac80211/sta_info.c
> +++ b/net/mac80211/sta_info.c
> @@ -90,7 +90,6 @@ static void __cleanup_single_sta(struct sta_info *sta)
> struct tid_ampdu_tx *tid_tx;
> struct ieee80211_sub_if_data *sdata = sta->sdata;
> struct ieee80211_local *local = sdata->local;
> - struct fq *fq = &local->fq;
> struct ps_data *ps;
>
> if (test_sta_flag(sta, WLAN_STA_PS_STA) ||
> @@ -120,9 +119,7 @@ static void __cleanup_single_sta(struct sta_info *sta)
>
> txqi = to_txq_info(sta->sta.txq[i]);
>
> - spin_lock_bh(&fq->lock);
> ieee80211_txq_purge(local, txqi);
> - spin_unlock_bh(&fq->lock);
> }
> }
>
> @@ -387,9 +384,12 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata,
> if (sta_prepare_rate_control(local, sta, gfp))
> goto free_txq;
>
> + sta->airtime_weight = IEEE80211_DEFAULT_AIRTIME_WEIGHT;
> +
> for (i = 0; i < IEEE80211_NUM_ACS; i++) {
> skb_queue_head_init(&sta->ps_tx_buf[i]);
> skb_queue_head_init(&sta->tx_filtered[i]);
> + sta->airtime[i].deficit = sta->airtime_weight;
> }
>
> for (i = 0; i < IEEE80211_NUM_TIDS; i++)
> @@ -1826,6 +1826,27 @@ void ieee80211_sta_set_buffered(struct ieee80211_sta *pubsta,
> }
> EXPORT_SYMBOL(ieee80211_sta_set_buffered);
>
> +void ieee80211_sta_register_airtime(struct ieee80211_sta *pubsta, u8 tid,
> + u32 tx_airtime, u32 rx_airtime)
> +{
> + struct sta_info *sta = container_of(pubsta, struct sta_info, sta);
> + struct ieee80211_local *local = sta->sdata->local;
> + u8 ac = ieee80211_ac_from_tid(tid);
> + u32 airtime = 0;
> +
> + if (sta->local->airtime_flags & AIRTIME_USE_TX)
> + airtime += tx_airtime;
> + if (sta->local->airtime_flags & AIRTIME_USE_RX)
> + airtime += rx_airtime;
> +
> + spin_lock_bh(&local->active_txq_lock[ac]);
> + sta->airtime[ac].tx_airtime += tx_airtime;
> + sta->airtime[ac].rx_airtime += rx_airtime;
> + sta->airtime[ac].deficit -= airtime;
> + spin_unlock_bh(&local->active_txq_lock[ac]);
> +}
> +EXPORT_SYMBOL(ieee80211_sta_register_airtime);
> +
> int sta_info_move_state(struct sta_info *sta,
> enum ieee80211_sta_state new_state)
> {
> @@ -2188,6 +2209,23 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
> sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_FAILED);
> }
>
> + if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_RX_DURATION))) {
> + for (ac = 0; ac < IEEE80211_NUM_ACS; ac++)
> + sinfo->rx_duration += sta->airtime[ac].rx_airtime;
> + sinfo->filled |= BIT(NL80211_STA_INFO_RX_DURATION);
> + }
> +
> + if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_DURATION))) {
> + for (ac = 0; ac < IEEE80211_NUM_ACS; ac++)
> + sinfo->tx_duration += sta->airtime[ac].tx_airtime;
> + sinfo->filled |= BIT(NL80211_STA_INFO_TX_DURATION);
> + }
> +
> + if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_AIRTIME_WEIGHT))) {
> + sinfo->airtime_weight = sta->airtime_weight;
> + sinfo->filled |= BIT(NL80211_STA_INFO_AIRTIME_WEIGHT);
> + }
> +
> sinfo->rx_dropped_misc = sta->rx_stats.dropped;
> if (sta->pcpu_rx_stats) {
> for_each_possible_cpu(cpu) {
> diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
> index 9a04327d71d1..b1b0fd6a2e21 100644
> --- a/net/mac80211/sta_info.h
> +++ b/net/mac80211/sta_info.h
> @@ -127,6 +127,16 @@ enum ieee80211_agg_stop_reason {
> AGG_STOP_DESTROY_STA,
> };
>
> +/* Debugfs flags to enable/disable use of RX/TX airtime in scheduler */
> +#define AIRTIME_USE_TX BIT(0)
> +#define AIRTIME_USE_RX BIT(1)
> +
> +struct airtime_info {
> + u64 rx_airtime;
> + u64 tx_airtime;
> + s64 deficit;
> +};
> +
> struct sta_info;
>
> /**
> @@ -563,6 +573,9 @@ struct sta_info {
> } tx_stats;
> u16 tid_seq[IEEE80211_QOS_CTL_TID_MASK + 1];
>
> + struct airtime_info airtime[IEEE80211_NUM_ACS];
> + u16 airtime_weight;
> +
> /*
> * Aggregation information, locked with lock.
> */
> diff --git a/net/mac80211/status.c b/net/mac80211/status.c
> index aa4afbf0abaf..a1f1256448f5 100644
> --- a/net/mac80211/status.c
> +++ b/net/mac80211/status.c
> @@ -818,6 +818,12 @@ static void __ieee80211_tx_status(struct ieee80211_hw *hw,
> ieee80211_sta_tx_notify(sta->sdata, (void *) skb->data,
> acked, info->status.tx_time);
>
> + if (info->status.tx_time &&
> + wiphy_ext_feature_isset(local->hw.wiphy,
> + NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
> + ieee80211_sta_register_airtime(&sta->sta, tid,
> + info->status.tx_time, 0);
> +
> if (ieee80211_hw_check(&local->hw, REPORTS_TX_ACK_STATUS)) {
> if (info->flags & IEEE80211_TX_STAT_ACK) {
> if (sta->status_stats.lost_packets)
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index 305965283506..3f417e80e041 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -1488,8 +1488,11 @@ void ieee80211_txq_purge(struct ieee80211_local *local,
> struct fq *fq = &local->fq;
> struct fq_tin *tin = &txqi->tin;
>
> + spin_lock_bh(&fq->lock);
> fq_tin_reset(fq, tin, fq_skb_free_func);
> ieee80211_purge_tx_queue(&local->hw, &txqi->frags);
> + spin_unlock_bh(&fq->lock);
> +
> spin_lock_bh(&local->active_txq_lock[txqi->txq.ac]);
> list_del_init(&txqi->schedule_order);
> spin_unlock_bh(&local->active_txq_lock[txqi->txq.ac]);
> @@ -3638,11 +3641,28 @@ struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac)
>
> lockdep_assert_held(&local->active_txq_lock[ac]);
>
> + begin:
> txqi = list_first_entry_or_null(&local->active_txqs[ac],
> struct txq_info,
> schedule_order);
> + if (!txqi)
> + return NULL;
> +
> + if (txqi->txq.sta) {
> + struct sta_info *sta = container_of(txqi->txq.sta,
> + struct sta_info, sta);
> +
> + if (sta->airtime[txqi->txq.ac].deficit < 0) {
> + sta->airtime[txqi->txq.ac].deficit +=
> + sta->airtime_weight;
> + list_move_tail(&txqi->schedule_order,
> + &local->active_txqs[txqi->txq.ac]);
> + goto begin;
> + }
> + }
> +
>
> - if (!txqi || txqi->schedule_round == local->schedule_round[ac])
> + if (txqi->schedule_round == local->schedule_round[ac])
> return NULL;
>
> list_del_init(&txqi->schedule_order);
> @@ -3660,12 +3680,74 @@ void ieee80211_return_txq(struct ieee80211_hw *hw,
> lockdep_assert_held(&local->active_txq_lock[txq->ac]);
>
> if (list_empty(&txqi->schedule_order) &&
> - (!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets))
> - list_add_tail(&txqi->schedule_order,
> - &local->active_txqs[txq->ac]);
> + (!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets)) {
> + /* If airtime accounting is active, always enqueue STAs at the
> + * head of the list to ensure that they only get moved to the
> + * back by the airtime DRR scheduler once they have a negative
> + * deficit. A station that already has a negative deficit will
> + * get immediately moved to the back of the list on the next
> + * call to ieee80211_next_txq().
> + */
> + if (txqi->txq.sta &&
> + wiphy_ext_feature_isset(local->hw.wiphy,
> + NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
> + list_add(&txqi->schedule_order,
> + &local->active_txqs[txq->ac]);
> + else
> + list_add_tail(&txqi->schedule_order,
> + &local->active_txqs[txq->ac]);
> + }
> }
> EXPORT_SYMBOL(ieee80211_return_txq);
>
> +bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
> + struct ieee80211_txq *txq)
> +{
> + struct ieee80211_local *local = hw_to_local(hw);
> + struct txq_info *iter, *tmp, *txqi = to_txq_info(txq);
> + struct sta_info *sta;
> + u8 ac = txq->ac;
> +
> + lockdep_assert_held(&local->active_txq_lock[ac]);
> +
> + if (!txqi->txq.sta)
> + goto out;
> +
> + if (list_empty(&txqi->schedule_order))
> + goto out;
> +
> + list_for_each_entry_safe(iter, tmp, &local->active_txqs[ac],
> + schedule_order) {
> + if (iter == txqi)
> + break;
> +
> + if (!iter->txq.sta) {
> + list_move_tail(&iter->schedule_order,
> + &local->active_txqs[ac]);
> + continue;
> + }
> + sta = container_of(iter->txq.sta, struct sta_info, sta);
> + if (sta->airtime[ac].deficit < 0)
> + sta->airtime[ac].deficit += sta->airtime_weight;
> + list_move_tail(&iter->schedule_order, &local->active_txqs[ac]);
> + }
> +
> + sta = container_of(txqi->txq.sta, struct sta_info, sta);
> + if (sta->airtime[ac].deficit >= 0)
> + goto out;
> +
> + sta->airtime[ac].deficit += sta->airtime_weight;
> + list_move_tail(&txqi->schedule_order, &local->active_txqs[ac]);
> +
> + return false;
> +out:
> + if (!list_empty(&txqi->schedule_order))
> + list_del_init(&txqi->schedule_order);
> +
> + return true;
> +}
> +EXPORT_SYMBOL(ieee80211_txq_may_transmit);
> +
> void ieee80211_txq_schedule_start(struct ieee80211_hw *hw, u8 ac)
> {
> struct ieee80211_local *local = hw_to_local(hw);
> --
> 1.9.1
>
> _______________________________________________
> Make-wifi-fast mailing list
> [email protected]
> https://lists.bufferbloat.net/listinfo/make-wifi-fast
On 2018-11-14 18:40, Toke Høiland-Jørgensen wrote:
>> This part doesn't really make much sense to me, but maybe I'm
>> misunderstanding how the code works.
>> Let's assume we have a driver like ath9k or mt76, which tries to keep a
>> number of aggregates in the hardware queue, and the hardware queue is
>> currently empty.
>> If the current txq entry is kept at the head of the schedule list,
>> wouldn't the code just pull from that one over and over again, until
>> enough packets are transmitted by the hardware and their tx status
>> processed?
>> It seems to me that while fairness is still preserved in the long run,
>> this could lead to rather bursty scheduling, which may not be
>> particularly latency friendly.
>
> Yes, it'll be a bit more bursty when the hardware queue is completely
> empty. However, when a TX completion comes back, that will adjust the
> deficit of that sta and cause it to be rotated on the next dequeue. This
> obviously relies on the fact that the lower-level hardware queue is
> sufficiently shallow to not add a lot of latency. But we want that to be
> the case anyway. In practice, it works quite well for ath9k, but not so
> well for ath10k because it has a large buffer in firmware.
>
> If we requeue the TXQ at the end of the list, a station that is taking
> up too much airtime will fail to be throttled properly, so the
> queue-at-head is kinda needed to ensure fairness...
Thanks for the explanation, that makes sense to me. I have an idea on
how to mitigate the burstiness within the driver. I'll write it down in
pseudocode, please let me know if you think that'll work.
do {
struct ieee80211_txq *pending_txq[4];
int n_pending_txq = 0;
int i;
if (hwq->pending < 4)
break;
nframes = 0;
ieee80211_txq_schedule_start(hw, ac)
do {
bool requeue = false;
struct ieee80211_txq *txq;
txq = ieee80211_next_txq(hw, ac);
if (!txq)
break;
nframes += schedule_txq(txq, &requeue);
if (requeue)
pending_txq[n_pending_txq++] = txq;
} while (n_pending_txq < ARRAY_SIZE(pending_txq));
for (i = n_pending_txq; i > 0; i--)
ieee80211_return_txq(hw, pending_txq[i - 1]);
ieee80211_txq_schedule_end(hw, ac)
} while (nframes);
- Felix
Louie Lu <[email protected]> writes:
> Hi Rajkumar, Toke,
>
> I found the series (v3,4/6) remove the debugfs remove reset station's
> airtime method, and didn't added at here.
>
> Not sure how to help this kind of situation, do I need a separate
> patch to fix this, or posting the patch here is fine?
This is fine; we can fold it into the next version. Thanks :)
-Toke
Felix Fietkau <[email protected]> writes:
> On 2018-11-14 18:40, Toke Høiland-Jørgensen wrote:
>>> This part doesn't really make much sense to me, but maybe I'm
>>> misunderstanding how the code works.
>>> Let's assume we have a driver like ath9k or mt76, which tries to keep a
>>> number of aggregates in the hardware queue, and the hardware queue is
>>> currently empty.
>>> If the current txq entry is kept at the head of the schedule list,
>>> wouldn't the code just pull from that one over and over again, until
>>> enough packets are transmitted by the hardware and their tx status
>>> processed?
>>> It seems to me that while fairness is still preserved in the long run,
>>> this could lead to rather bursty scheduling, which may not be
>>> particularly latency friendly.
>>
>> Yes, it'll be a bit more bursty when the hardware queue is completely
>> empty. However, when a TX completion comes back, that will adjust the
>> deficit of that sta and cause it to be rotated on the next dequeue. This
>> obviously relies on the fact that the lower-level hardware queue is
>> sufficiently shallow to not add a lot of latency. But we want that to be
>> the case anyway. In practice, it works quite well for ath9k, but not so
>> well for ath10k because it has a large buffer in firmware.
>>
>> If we requeue the TXQ at the end of the list, a station that is taking
>> up too much airtime will fail to be throttled properly, so the
>> queue-at-head is kinda needed to ensure fairness...
> Thanks for the explanation, that makes sense to me. I have an idea on
> how to mitigate the burstiness within the driver. I'll write it down in
> pseudocode, please let me know if you think that'll work.
I don't think it will, unfortunately. For example, consider the case
where there are two stations queued; one with a large negative deficit
(say, -10ms), and one with a positive deficit.
In this case, we really need to throttle the station with a negative
deficit. But if the driver loops and caches txqs, we'll get something
like the following:
- First driver loop iteration: returns TXQ with positive deficit.
- Second driver loop iteration: Only the negative-deficit TXQ is in the
mac80211 list, so it will loop until that TXQ's deficit turns positive
and return it.
Because of this, the negative-deficit station won't be throttled, and we
won't get fairness.
How many frames will mt76 queue up below the driver point? I.e., how
much burstiness are you expecting this will introduce on that driver?
Taking a step back, it's clear that it would be good to be able to
dequeue packets to multiple STAs at once (we need that for MU-MIMO on
ath10k as well). However, I don't think we can do that with the
round-robin fairness scheduler; so we are going to need a different
algorithm. I *think* it may be possible to do this with a virtual-time
scheduler, but I haven't sat down and worked out the details yet...
-Toke
Toke Høiland-Jørgensen <[email protected]> writes:
> Felix Fietkau <[email protected]> writes:
>
>> On 2018-11-14 18:40, Toke Høiland-Jørgensen wrote:
>>>> This part doesn't really make much sense to me, but maybe I'm
>>>> misunderstanding how the code works.
>>>> Let's assume we have a driver like ath9k or mt76, which tries to keep a
>>>> number of aggregates in the hardware queue, and the hardware queue is
>>>> currently empty.
>>>> If the current txq entry is kept at the head of the schedule list,
>>>> wouldn't the code just pull from that one over and over again, until
>>>> enough packets are transmitted by the hardware and their tx status
>>>> processed?
>>>> It seems to me that while fairness is still preserved in the long run,
>>>> this could lead to rather bursty scheduling, which may not be
>>>> particularly latency friendly.
>>>
>>> Yes, it'll be a bit more bursty when the hardware queue is completely
>>> empty. However, when a TX completion comes back, that will adjust the
>>> deficit of that sta and cause it to be rotated on the next dequeue. This
>>> obviously relies on the fact that the lower-level hardware queue is
>>> sufficiently shallow to not add a lot of latency. But we want that to be
>>> the case anyway. In practice, it works quite well for ath9k, but not so
>>> well for ath10k because it has a large buffer in firmware.
>>>
>>> If we requeue the TXQ at the end of the list, a station that is taking
>>> up too much airtime will fail to be throttled properly, so the
>>> queue-at-head is kinda needed to ensure fairness...
>> Thanks for the explanation, that makes sense to me. I have an idea on
>> how to mitigate the burstiness within the driver. I'll write it down in
>> pseudocode, please let me know if you think that'll work.
>
> I don't think it will, unfortunately. For example, consider the case
> where there are two stations queued; one with a large negative deficit
> (say, -10ms), and one with a positive deficit.
Perhaps a flag for one way or the other?
if(driver->has_absurd_hardware_queue_depth) doitthisway(); else
doitabetterway();
>
> In this case, we really need to throttle the station with a negative
> deficit. But if the driver loops and caches txqs, we'll get something
> like the following:
>
> - First driver loop iteration: returns TXQ with positive deficit.
> - Second driver loop iteration: Only the negative-deficit TXQ is in the
> mac80211 list, so it will loop until that TXQ's deficit turns positive
> and return it.
>
> Because of this, the negative-deficit station won't be throttled, and we
> won't get fairness.
>
> How many frames will mt76 queue up below the driver point? I.e., how
> much burstiness are you expecting this will introduce on that driver?
>
> Taking a step back, it's clear that it would be good to be able to
> dequeue packets to multiple STAs at once (we need that for MU-MIMO on
> ath10k as well). However, I don't think we can do that with the
> round-robin fairness scheduler; so we are going to need a different
> algorithm. I *think* it may be possible to do this with a virtual-time
> scheduler, but I haven't sat down and worked out the details yet...
The answer to which did not fit on the margins of your thesis. :)
I too have been trying to come up with a better means of gang
scheduling... for about 2 years now. In terms of bitmaps it looks a bit
like QFQ, but honestly...
Is there going to be some point where whatever we have here is
significantly better than what we had? Or not significantly worse? Or
handwavy enough to fix the rest once enlightenment arrives?
The perfect is the enemy of the good.
I'd rather like the intel folk to be weighing in on this stuff, too,
trying to get an API right requires use cases.
>
> -Toke
> _______________________________________________
> Make-wifi-fast mailing list
> [email protected]
> https://lists.bufferbloat.net/listinfo/make-wifi-fast
Dave Taht <[email protected]> writes:
> Toke Høiland-Jørgensen <[email protected]> writes:
>
>> Felix Fietkau <[email protected]> writes:
>>
>>> On 2018-11-14 18:40, Toke Høiland-Jørgensen wrote:
>>>>> This part doesn't really make much sense to me, but maybe I'm
>>>>> misunderstanding how the code works.
>>>>> Let's assume we have a driver like ath9k or mt76, which tries to keep a
>>>>> number of aggregates in the hardware queue, and the hardware queue is
>>>>> currently empty.
>>>>> If the current txq entry is kept at the head of the schedule list,
>>>>> wouldn't the code just pull from that one over and over again, until
>>>>> enough packets are transmitted by the hardware and their tx status
>>>>> processed?
>>>>> It seems to me that while fairness is still preserved in the long run,
>>>>> this could lead to rather bursty scheduling, which may not be
>>>>> particularly latency friendly.
>>>>
>>>> Yes, it'll be a bit more bursty when the hardware queue is completely
>>>> empty. However, when a TX completion comes back, that will adjust the
>>>> deficit of that sta and cause it to be rotated on the next dequeue. This
>>>> obviously relies on the fact that the lower-level hardware queue is
>>>> sufficiently shallow to not add a lot of latency. But we want that to be
>>>> the case anyway. In practice, it works quite well for ath9k, but not so
>>>> well for ath10k because it has a large buffer in firmware.
>>>>
>>>> If we requeue the TXQ at the end of the list, a station that is taking
>>>> up too much airtime will fail to be throttled properly, so the
>>>> queue-at-head is kinda needed to ensure fairness...
>>> Thanks for the explanation, that makes sense to me. I have an idea on
>>> how to mitigate the burstiness within the driver. I'll write it down in
>>> pseudocode, please let me know if you think that'll work.
>>
>> I don't think it will, unfortunately. For example, consider the case
>> where there are two stations queued; one with a large negative deficit
>> (say, -10ms), and one with a positive deficit.
>
> Perhaps a flag for one way or the other?
>
> if(driver->has_absurd_hardware_queue_depth) doitthisway(); else
> doitabetterway();
Well, there's going to be a BQL-like queue limit (but for airtime) on
top, which drivers can opt-in to if the hardware has too much queueing.
>> In this case, we really need to throttle the station with a negative
>> deficit. But if the driver loops and caches txqs, we'll get something
>> like the following:
>>
>> - First driver loop iteration: returns TXQ with positive deficit.
>> - Second driver loop iteration: Only the negative-deficit TXQ is in the
>> mac80211 list, so it will loop until that TXQ's deficit turns positive
>> and return it.
>>
>> Because of this, the negative-deficit station won't be throttled, and we
>> won't get fairness.
>>
>> How many frames will mt76 queue up below the driver point? I.e., how
>> much burstiness are you expecting this will introduce on that driver?
>>
>> Taking a step back, it's clear that it would be good to be able to
>> dequeue packets to multiple STAs at once (we need that for MU-MIMO on
>> ath10k as well). However, I don't think we can do that with the
>> round-robin fairness scheduler; so we are going to need a different
>> algorithm. I *think* it may be possible to do this with a virtual-time
>> scheduler, but I haven't sat down and worked out the details yet...
>
> The answer to which did not fit on the margins of your thesis. :)
>
> I too have been trying to come up with a better means of gang
> scheduling... for about 2 years now. In terms of bitmaps it looks a bit
> like QFQ, but honestly...
It's not the gang scheduling we need, deciding which devices to send to
at once is generally done in firmware anyway. We just need to be able to
dequeue packets for more than one station when possible. I don't think
we need the fancy bitmap stuff from QFQ since we don't have that many
stations to schedule at once; so we can probably live with O(log(n)) in
the number of active stations.
> Is there going to be some point where whatever we have here is
> significantly better than what we had? Or not significantly worse? Or
> handwavy enough to fix the rest once enlightenment arrives?
>
> The perfect is the enemy of the good.
Well, what we have now works for ath9k, works reasonably well for ath10k
in pull mode, not so well for ath10k in push mode, and then there's
Felix' comments in this thread...
> I'd rather like the intel folk to be weighing in on this stuff, too,
> trying to get an API right requires use cases.
Johannes has already reviewed a previous version, and I do believe he
said he'd review it again once we have converged on something :)
-Toke
Hi Felix
Thinking a bit more about this, I think that rather than having the
driver work around the API as in your example...
> do {
> struct ieee80211_txq *pending_txq[4];
> int n_pending_txq = 0;
> int i;
>
> if (hwq->pending < 4)
> break;p
>
> nframes = 0;
>
> ieee80211_txq_schedule_start(hw, ac)
> do {
> bool requeue = false;
>
> struct ieee80211_txq *txq;
>
> txq = ieee80211_next_txq(hw, ac);
> if (!txq)
> break;
>
> nframes += schedule_txq(txq, &requeue);
> if (requeue)
> pending_txq[n_pending_txq++] = txq;
>
> } while (n_pending_txq < ARRAY_SIZE(pending_txq));
>
> for (i = n_pending_txq; i > 0; i--)
> ieee80211_return_txq(hw, pending_txq[i - 1]);
>
> ieee80211_txq_schedule_end(hw, ac)
> } while (nframes);
... really what we want is that the driver can just do this:
ieee80211_txq_schedule_start(hw, ac);
while ((txq = ieee80211_next_txq(hw, ac)) {
schedule_txq(txq, &requeue);
return_txq(txq);
}
ieee80211_txq_schedule_end(hw, ac);
and expect so get through all eligible TXQs. Note that there will be
cases where there is only a single eligible TXQ (such as the example I
gave in the other email); in which case the current version is fine. But
there is (probably) also going to be cases where more than one TXQ is
eligible at the same time, which we cannot handle with the current RR
scheduler.
However, I think that assuming we can get the scheduler to guarantee
that it will return all eligible TXQs between each pair of calls to
schedule_start()/schedule_end(), we should be fine with the current API.
Do you agree with this?
-Toke
Toke Høiland-Jørgensen <[email protected]> writes:
> Dave Taht <[email protected]> writes:
>
>> Toke Høiland-Jørgensen <[email protected]> writes:
>>
>>> Felix Fietkau <[email protected]> writes:
>>>
>>>> On 2018-11-14 18:40, Toke Høiland-Jørgensen wrote:
>>>>>> This part doesn't really make much sense to me, but maybe I'm
>>>>>> misunderstanding how the code works.
>>>>>> Let's assume we have a driver like ath9k or mt76, which tries to keep a
>>>>>> number of aggregates in the hardware queue, and the hardware queue is
>>>>>> currently empty.
>>>>>> If the current txq entry is kept at the head of the schedule list,
>>>>>> wouldn't the code just pull from that one over and over again, until
>>>>>> enough packets are transmitted by the hardware and their tx status
>>>>>> processed?
>>>>>> It seems to me that while fairness is still preserved in the long run,
>>>>>> this could lead to rather bursty scheduling, which may not be
>>>>>> particularly latency friendly.
>>>>>
>>>>> Yes, it'll be a bit more bursty when the hardware queue is completely
>>>>> empty. However, when a TX completion comes back, that will adjust the
>>>>> deficit of that sta and cause it to be rotated on the next dequeue. This
>>>>> obviously relies on the fact that the lower-level hardware queue is
>>>>> sufficiently shallow to not add a lot of latency. But we want that to be
>>>>> the case anyway. In practice, it works quite well for ath9k, but not so
>>>>> well for ath10k because it has a large buffer in firmware.
>>>>>
>>>>> If we requeue the TXQ at the end of the list, a station that is taking
>>>>> up too much airtime will fail to be throttled properly, so the
>>>>> queue-at-head is kinda needed to ensure fairness...
>>>> Thanks for the explanation, that makes sense to me. I have an idea on
>>>> how to mitigate the burstiness within the driver. I'll write it down in
>>>> pseudocode, please let me know if you think that'll work.
>>>
>>> I don't think it will, unfortunately. For example, consider the case
>>> where there are two stations queued; one with a large negative deficit
>>> (say, -10ms), and one with a positive deficit.
>>
>> Perhaps a flag for one way or the other?
>>
>> if(driver->has_absurd_hardware_queue_depth) doitthisway(); else
>> doitabetterway();
>
> Well, there's going to be a BQL-like queue limit (but for airtime) on
> top, which drivers can opt-in to if the hardware has too much queueing.
>
>>> In this case, we really need to throttle the station with a negative
>>> deficit. But if the driver loops and caches txqs, we'll get something
>>> like the following:
>>>
>>> - First driver loop iteration: returns TXQ with positive deficit.
>>> - Second driver loop iteration: Only the negative-deficit TXQ is in the
>>> mac80211 list, so it will loop until that TXQ's deficit turns positive
>>> and return it.
>>>
>>> Because of this, the negative-deficit station won't be throttled, and we
>>> won't get fairness.
>>>
>>> How many frames will mt76 queue up below the driver point? I.e., how
>>> much burstiness are you expecting this will introduce on that driver?
>>>
>>> Taking a step back, it's clear that it would be good to be able to
>>> dequeue packets to multiple STAs at once (we need that for MU-MIMO on
>>> ath10k as well). However, I don't think we can do that with the
>>> round-robin fairness scheduler; so we are going to need a different
>>> algorithm. I *think* it may be possible to do this with a virtual-time
>>> scheduler, but I haven't sat down and worked out the details yet...
>>
>> The answer to which did not fit on the margins of your thesis. :)
>>
>> I too have been trying to come up with a better means of gang
>> scheduling... for about 2 years now. In terms of bitmaps it looks a bit
>> like QFQ, but honestly...
>
> It's not the gang scheduling we need, deciding which devices to send to
> at once is generally done in firmware anyway.
I have a long held dream that one day some firmware will be able to send
an interrupt and some information along...
"Hi, I'll be done transmitting/receiving in about 1ms, here's who I
think I can talk to next, and here's who else I maybe could gang schedule".
That would let us get away from 5ms wasted in the "ready to go" portion
of the algo, and share the highest likelyhood "groups" with the higher
layer.
> We just need to be able to
> dequeue packets for more than one station when possible.
And a huge fantasy is in some future 802.11ZZZ standard the on-board firmware and
the linux drivers can be co-designed, even, dare I say, open sourced, to
better evolve to meet real world requirements.
mbox's per station would be nice, with scatter/gather I/O... I
can think of a zillion things I'd want the firmware to handle (other
than buffering)
> I don't think
> we need the fancy bitmap stuff from QFQ since we don't have that many
> stations to schedule at once; so we can probably live with O(log(n)) in
> the number of active stations.
Best of two or three "groups", per above, from the firmware.
>> Is there going to be some point where whatever we have here is
>> significantly better than what we had? Or not significantly worse? Or
>> handwavy enough to fix the rest once enlightenment arrives?
>>
>> The perfect is the enemy of the good.
>
> Well, what we have now works for ath9k, works reasonably well for ath10k
> in pull mode, not so well for ath10k in push mode, and then there's
> Felix' comments in this thread...
So how about, an ath10k in a friggin "co-operative" mode?
What are the performance differences in ath10k in push mode? Why do we
care if this mode works at all?
Perfect, verses "good".
>> I'd rather like the intel folk to be weighing in on this stuff, too,
>> trying to get an API right requires use cases.
>
> Johannes has already reviewed a previous version, and I do believe he
> said he'd review it again once we have converged on something :)
Would intel care if only the pull mode worked well on their hardware? Do
they have a pull or push mode?
Felix Fietkau <[email protected]> writes:
>> diff --git a/net/mac80211/status.c b/net/mac80211/status.c
>> index aa4afbf0abaf..a1f1256448f5 100644
>> --- a/net/mac80211/status.c
>> +++ b/net/mac80211/status.c
>> @@ -818,6 +818,12 @@ static void __ieee80211_tx_status(struct ieee80211_hw *hw,
>> ieee80211_sta_tx_notify(sta->sdata, (void *) skb->data,
>> acked, info->status.tx_time);
>>
>> + if (info->status.tx_time &&
>> + wiphy_ext_feature_isset(local->hw.wiphy,
>> + NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
>> + ieee80211_sta_register_airtime(&sta->sta, tid,
>> + info->status.tx_time, 0);
>> +
>> if (ieee80211_hw_check(&local->hw, REPORTS_TX_ACK_STATUS)) {
>> if (info->flags & IEEE80211_TX_STAT_ACK) {
>> if (sta->status_stats.lost_packets)
> I think the same is needed in ieee80211_tx_status_ext.
So finally circled back to this. In ieee80211_tx_status_ext() we don't
have an skb, so we don't know which TID the packet was sent to; what
airtime information would the driver actually provide in this case? Is
it an aggregate of all ACs, or?
-Toke
On Thu, 2018-11-15 at 09:10 -0800, Toke Høiland-Jørgensen wrote:
> Louie Lu <[email protected]> writes:
>
> > Hi Rajkumar, Toke,
> >
> > I found the series (v3,4/6) remove the debugfs remove reset station's
> > airtime method, and didn't added at here.
> >
> > Not sure how to help this kind of situation, do I need a separate
> > patch to fix this, or posting the patch here is fine?
>
> This is fine; we can fold it into the next version. Thanks :)
Just FYI - I'm going to assume, given this comment and the long
discussion, that there will be a next version :)
johannes
Johannes Berg <[email protected]> writes:
> On Thu, 2018-11-15 at 09:10 -0800, Toke Høiland-Jørgensen wrote:
>> Louie Lu <[email protected]> writes:
>>
>> > Hi Rajkumar, Toke,
>> >
>> > I found the series (v3,4/6) remove the debugfs remove reset
>> > station's
>> > airtime method, and didn't added at here.
>> >
>> > Not sure how to help this kind of situation, do I need a separate
>> > patch to fix this, or posting the patch here is fine?
>>
>> This is fine; we can fold it into the next version. Thanks :)
>
> Just FYI - I'm going to assume, given this comment and the long
> discussion, that there will be a next version :)
I think toke's still sleeping off the post graduation hangover.
>
> johannes
>
> _______________________________________________
> Make-wifi-fast mailing list
> [email protected]
> https://lists.bufferbloat.net/listinfo/make-wifi-fast
Johannes Berg <[email protected]> writes:
> On Thu, 2018-11-15 at 09:10 -0800, Toke Høiland-Jørgensen wrote:
>> Louie Lu <[email protected]> writes:
>>
>> > Hi Rajkumar, Toke,
>> >
>> > I found the series (v3,4/6) remove the debugfs remove reset station's
>> > airtime method, and didn't added at here.
>> >
>> > Not sure how to help this kind of situation, do I need a separate
>> > patch to fix this, or posting the patch here is fine?
>>
>> This is fine; we can fold it into the next version. Thanks :)
>
> Just FYI - I'm going to assume, given this comment and the long
> discussion, that there will be a next version :)
Yes. Got caught up in moving before I managed to get another version
out. Will get around to it eventually, promise! :)
-Toke