2018-10-20 11:05:56

by Rajkumar Manoharan

[permalink] [raw]
Subject: [PATCH 0/6] Move TXQ scheduling and airtime fairness into mac80211

Toke,

It is been a while since mac80211 TXQ discussion started. Here is the consolidated
series of mac80211, ath9k and ath10k changes to move TXQ scheduling and airtime
fairness into mac80211. The major changes w.r.t 5th RFC version are in may_transmit()
API. Whenever the driver checks deficit for given TXQ, the list will be
reordered so that driver/firmware RR quickly becomes in sync with mac80211
list. Also airtime tasklet approach cumbersome and causing regression
in multi client performance.

- Removed airtime kickout tasklet

- Reworked on ieee80211_txq_may_transmit() API

- Provided option to configure driver specific default airtime.

- Bundled ath10k changes along with this series.

-Rajkumar

Rajkumar Manoharan (2):
ath10k: migrate to mac80211 txq scheduling
ath10k: reporting estimated tx airtime for fairness

Toke Høiland-Jørgensen (4):
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

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 | 118 +++++++++++++-
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 | 12 ++
net/mac80211/main.c | 9 ++
net/mac80211/sta_info.c | 54 ++++++-
net/mac80211/sta_info.h | 13 ++
net/mac80211/status.c | 6 +
net/mac80211/tx.c | 140 ++++++++++++++++-
net/wireless/nl80211.c | 28 ++++
28 files changed, 646 insertions(+), 356 deletions(-)

--
1.9.1



2018-10-20 11:05:56

by Rajkumar Manoharan

[permalink] [raw]
Subject: [PATCH 1/6] mac80211: Add TXQ scheduling API

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 c4fadbafbf21..2f5c0fbd453c 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
@@ -6045,7 +6051,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.
*/
@@ -6053,6 +6060,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 e42c641b6190..0d47dadee747 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 f40a2167935f..976531717902 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -829,6 +829,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;
@@ -836,6 +838,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! */
@@ -1127,6 +1131,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 e6375d035355..3ea2369e0992 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


2018-10-20 11:05:57

by Rajkumar Manoharan

[permalink] [raw]
Subject: [PATCH 2/6] cfg80211: Add airtime statistics and settings

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]>
---
include/net/cfg80211.h | 10 +++++++++-
include/uapi/linux/nl80211.h | 15 +++++++++++++++
net/wireless/nl80211.c | 28 ++++++++++++++++++++++++++++
3 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 0e16e723dcef..4591cb8a8e5d 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.
@@ -1343,10 +1347,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;
};
@@ -2374,6 +2380,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 dc6d5a1ef470..43591af7178a 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,
@@ -3068,6 +3073,9 @@ enum nl80211_sta_bss_param {
* @NL80211_STA_INFO_PAD: attribute used for padding for 64-bit alignment
* @NL80211_STA_INFO_ACK_SIGNAL: signal strength of the last ACK frame(u8, dBm)
* @NL80211_STA_INFO_ACK_SIGNAL_AVG: avg signal strength of ACK frames (s8, dBm)
+ * @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
*/
@@ -3108,6 +3116,8 @@ enum nl80211_sta_info {
NL80211_STA_INFO_PAD,
NL80211_STA_INFO_ACK_SIGNAL,
NL80211_STA_INFO_ACK_SIGNAL_AVG,
+ NL80211_STA_INFO_TX_DURATION,
+ NL80211_STA_INFO_AIRTIME_WEIGHT,

/* keep last */
__NL80211_STA_INFO_AFTER_LAST,
@@ -5250,6 +5260,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.
*/
@@ -5289,6 +5303,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 9a20c66a1505..09361f75d1df 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -444,6 +444,7 @@ enum nl80211_multicast_groups {
.type = NLA_NESTED,
.validation_data = nl80211_ftm_responder_policy,
},
+ [NL80211_ATTR_AIRTIME_WEIGHT] = { .type = NLA_U16 },
};

/* policy for the key attributes */
@@ -4702,6 +4703,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:
@@ -5339,6 +5345,17 @@ 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]) {
+ if (!wiphy_ext_feature_isset(&rdev->wiphy,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
+ return -EOPNOTSUPP;
+
+ params.airtime_weight =
+ nla_get_u16(info->attrs[NL80211_ATTR_AIRTIME_WEIGHT]);
+ if (!params.airtime_weight)
+ return -EINVAL;
+ }
+
/* Include parameters for TDLS peer (will check later) */
err = nl80211_set_station_tdls(info, &params);
if (err)
@@ -5477,6 +5494,17 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
return -EINVAL;
}

+ if (info->attrs[NL80211_ATTR_AIRTIME_WEIGHT]) {
+ if (!wiphy_ext_feature_isset(&rdev->wiphy,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
+ return -EOPNOTSUPP;
+
+ params.airtime_weight =
+ nla_get_u16(info->attrs[NL80211_ATTR_AIRTIME_WEIGHT]);
+ if (!params.airtime_weight)
+ return -EINVAL;
+ }
+
err = nl80211_parse_sta_channel_info(info, &params);
if (err)
return err;
--
1.9.1


2018-10-20 11:05:57

by Rajkumar Manoharan

[permalink] [raw]
Subject: [PATCH 4/6] ath9k: Switch to mac80211 TXQ scheduling and airtime APIs

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]>
---
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, 73 insertions(+), 278 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..cd49a970dd3a 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,15 @@ 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)) {
+ if ((ret = ath_tx_prepare(hw, skb, &txctl))) {
ieee80211_free_txskb(hw, skb);
- return NULL;
+ return ret;
}

q = skb_get_queue_mapping(skb);
@@ -252,24 +213,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;
+ int ret = 0;
+ *skb = __skb_dequeue(&tid->retry_q);
+ if (!*skb)
+ ret = ath_tid_pull(tid, skb);

- skb = __skb_dequeue(&tid->retry_q);
- if (!skb)
- skb = ath_tid_pull(tid);
-
- return skb;
+ return ret;
}

static void ath_tx_flush_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
@@ -365,11 +321,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 +638,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 +664,9 @@ 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,
- struct ath_tx_status *ts)
+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 +676,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 +706,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 +890,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 +936,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 +955,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 +972,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 +986,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 +1038,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 +1437,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 +1451,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 +1465,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 +1495,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 +1504,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 +1567,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 +1595,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 +1631,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 +1646,8 @@ 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 +1913,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 +1925,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 +2788,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 +2797,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


2018-10-20 11:05:57

by Rajkumar Manoharan

[permalink] [raw]
Subject: [PATCH 3/6] mac80211: Add airtime accounting and scheduling to TXQs

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.

Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
Signed-off-by: Rajkumar Manoharan <[email protected]>
---
include/net/mac80211.h | 58 ++++++++++++++++++++++++++++++
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 | 45 +++++++++++++++++++++--
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 2f5c0fbd453c..0ced3adb09ac 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2334,6 +2334,8 @@ 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.
+ *
+ * @airtime_weight: Default airtime weight preferred by driver.
*/
struct ieee80211_hw {
struct ieee80211_conf conf;
@@ -2370,6 +2372,7 @@ struct ieee80211_hw {
const struct ieee80211_cipher_scheme *cipher_schemes;
u8 max_nan_de_entries;
u8 tx_sk_pacing_shift;
+ u32 airtime_weight;
};

static inline bool _ieee80211_hw_check(struct ieee80211_hw *hw,
@@ -5350,6 +5353,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
@@ -6107,6 +6138,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 914aef7e7afd..337ac2b57e59 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1388,6 +1388,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 976531717902..7abc20c0e47c 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1136,6 +1136,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 3ea2369e0992..3d79f42042dd 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.airtime_weight)
+ local->hw.airtime_weight = IEEE80211_DEFAULT_AIRTIME_WEIGHT;
+
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..5a6d05063341 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 = hw->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,28 @@ 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;
+ struct txq_info *txqi;
+ 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 +2210,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(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(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(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 91d7c0cd1882..cd4582f80fea 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..dd2354188357 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 (wiphy_ext_feature_isset(local->hw.wiphy,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS)
+ && txqi->txq.sta)
+ 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


2018-10-20 11:05:58

by Rajkumar Manoharan

[permalink] [raw]
Subject: [PATCH 5/6] ath10k: migrate to mac80211 txq scheduling

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.

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..571289c7821d 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 deficit (Tuned for multiclient performance) */
+#define ATH10K_DEFAULT_AIRTIME_WEIGHT 4096
+
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..d78b83ac5c7b 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->airtime_weight = ATH10K_DEFAULT_AIRTIME_WEIGHT;
+
ret = ieee80211_register_hw(ar->hw);
if (ret) {
ath10k_err(ar, "failed to register ieee80211: %d\n", ret);
--
1.9.1


2018-10-20 11:06:01

by Rajkumar Manoharan

[permalink] [raw]
Subject: [PATCH 6/6] ath10k: reporting estimated tx airtime for fairness

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.

Signed-off-by: Kan Yan <[email protected]>
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 571289c7821d..e6f597d9f226 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 d78b83ac5c7b..d74b55ba9914 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


2018-10-21 11:28:04

by Toke Høiland-Jørgensen

[permalink] [raw]
Subject: Re: [PATCH 0/6] Move TXQ scheduling and airtime fairness into mac80211

Rajkumar Manoharan <[email protected]> writes:

> Toke,
>
> It is been a while since mac80211 TXQ discussion started. Here is the consolidated
> series of mac80211, ath9k and ath10k changes to move TXQ scheduling and airtime
> fairness into mac80211. The major changes w.r.t 5th RFC version are in may_transmit()
> API. Whenever the driver checks deficit for given TXQ, the list will be
> reordered so that driver/firmware RR quickly becomes in sync with mac80211
> list. Also airtime tasklet approach cumbersome and causing regression
> in multi client performance.

Cool, thanks. I'll look at this in more detail later, but in the
meantime, do you have any test results to share? Specifically, how did
you measure that the fairness part actually works? :)

-Toke

2018-10-24 08:33:37

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 5/6] ath10k: migrate to mac80211 txq scheduling

Rajkumar Manoharan <[email protected]> writes:

> 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.
>
> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
> Signed-off-by: Rajkumar Manoharan <[email protected]>

Is Toke the original author? Then you should add "From:" tag to mark
that. And you might want to use Co-developed-by to mark yourself as
co-developer.

--
Kalle Valo

2018-10-24 08:35:28

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 6/6] ath10k: reporting estimated tx airtime for fairness

Rajkumar Manoharan <[email protected]> writes:

> 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.
>
> Signed-off-by: Kan Yan <[email protected]>
> Signed-off-by: Rajkumar Manoharan <[email protected]>

Same comments as in patch 5 about "From:" and Co-developed-by tags.

--
Kalle Valo

2018-10-24 18:55:27

by Rajkumar Manoharan

[permalink] [raw]
Subject: Re: [PATCH 5/6] ath10k: migrate to mac80211 txq scheduling

On 2018-10-24 01:33, Kalle Valo wrote:
> Rajkumar Manoharan <[email protected]> writes:
>
>> 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.
>>
>> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
>> Signed-off-by: Rajkumar Manoharan <[email protected]>
>
> Is Toke the original author? Then you should add "From:" tag to mark
> that. And you might want to use Co-developed-by to mark yourself as
> co-developer.
>
Toke submitted ath10k changes in RFC v1 but those were dropped
in follow up series. When I reworked on ath10k, I retaining his s-o-b.

Will fix it.

-Rajkumar

2018-10-26 14:16:17

by Toke Høiland-Jørgensen

[permalink] [raw]
Subject: Re: [PATCH 3/6] mac80211: Add airtime accounting and scheduling to TXQs

Rajkumar Manoharan <[email protected]> writes:

> 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.
>
> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
> Signed-off-by: Rajkumar Manoharan <[email protected]>
> ---
> include/net/mac80211.h | 58 ++++++++++++++++++++++++++++++
> 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 | 45 +++++++++++++++++++++--
> 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 2f5c0fbd453c..0ced3adb09ac 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -2334,6 +2334,8 @@ 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.
> + *
> + * @airtime_weight: Default airtime weight preferred by driver.
> */
> struct ieee80211_hw {
> struct ieee80211_conf conf;
> @@ -2370,6 +2372,7 @@ struct ieee80211_hw {
> const struct ieee80211_cipher_scheme *cipher_schemes;
> u8 max_nan_de_entries;
> u8 tx_sk_pacing_shift;
> + u32 airtime_weight;
> };

This doesn't make sense. Airtime weights can be set by userspace, so
even if a driver sets another default it is not guaranteed to be
honoured. So what's the point?

> +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]);
> + }

So since we're rotating the queue on every call to the function, I'm
wondering if this actually succeeds in throttling the slow station's
airtime usage enough to achieve fairness? So I'll ask again: Did you
test the fairness performance, and how?


Also, taking a step back:

With this, we're doing lots of work to make sure that the hardware's
round-robin scheduling queue lines up with mac80211; so if we do this
anyway, why can't we just control the order directly from mac80211
(which is what we do with the other next_txq() API)?

-Toke

2018-10-26 23:04:26

by Rajkumar Manoharan

[permalink] [raw]
Subject: Re: [PATCH 3/6] mac80211: Add airtime accounting and scheduling to TXQs

On 2018-10-26 07:16, Toke Høiland-Jørgensen wrote:
> Rajkumar Manoharan <[email protected]> writes:
>
>> From: Toke Høiland-Jørgensen <[email protected]>
[...]
>> u8 max_nan_de_entries;
>> u8 tx_sk_pacing_shift;
>> + u32 airtime_weight;
>> };
>
> This doesn't make sense. Airtime weights can be set by userspace, so
> even if a driver sets another default it is not guaranteed to be
> honoured. So what's the point?
>
The reason for driver specific default is to avoid performance impact
in ath10k when the user is using vanilla ath10k with default airtime.
As I mentioned earlier, mac80211 default (256us) is too low for 11ac
devices
especially with driver is bursting aggregation.

Yes. I do understand the user can change airtime at anytime but It must
be
noted that different airtime weight will result in different throughput.
IMHO the defaults should not impact current benchmark. Otherwise it will
be
alarmed as regression later. isn't it?


> So since we're rotating the queue on every call to the function, I'm
> wondering if this actually succeeds in throttling the slow station's
> airtime usage enough to achieve fairness? So I'll ask again: Did you
> test the fairness performance, and how?
>
We are collecting data of mixed clients (11ac, 11n and legacy) keeping
them
at same distance and different distance. So that lower rate clients will
interfere higher MCS rate station. Also configuring different airtime
weight
for each stations so that throttling low rate clients more should help
higher
performing(better MCS) clients.

By allowing different airtime for each stations, the user can control
guest
network over primary network. Also It helped to improve performance of
preferred
station and algo. to control station is given to cloud or user
application.

As of now, We are testing with 4 11ac clients of same distance. And
collecting
the performance data in multiple iteration. Below are current data of
station's performance (Mbps) against airtime weight.

airtime station1(%airtime) station2 station3 station4
(Mbps)

No ATF 182 168 166 169

4ms 170 (100%) 164 (100%) 185 (100%) 175 (100%)

4ms 277 (70%) 115 (10%) 103 (10%) 105 (10%)

4ms 223 (40%) 214 (40%) 109 (10%) 94 (10%)

4ms 337 (90%) 182 (8%) 23 (1%) 30 (1%)



STA1(11ac) STA2 (11n) STA3(11a)
========== ========== =========

No ATF 225 166 3.5

ATF (4ms) 234 151 3.5

>
> Also, taking a step back:
>
> With this, we're doing lots of work to make sure that the hardware's
> round-robin scheduling queue lines up with mac80211; so if we do this
> anyway, why can't we just control the order directly from mac80211
> (which is what we do with the other next_txq() API)?
>
The only way to enforce mac80211 ordering in ath10k is to disable pull
mode in firmware and always operates in push mode similar to ath9k.

-Rajkumar

2018-10-28 15:48:14

by Toke Høiland-Jørgensen

[permalink] [raw]
Subject: Re: [PATCH 3/6] mac80211: Add airtime accounting and scheduling to TXQs

Rajkumar Manoharan <[email protected]> writes:

> On 2018-10-26 07:16, Toke Høiland-Jørgensen wrote:
>> Rajkumar Manoharan <[email protected]> writes:
>>
>>> From: Toke Høiland-Jørgensen <[email protected]>
> [...]
>>> u8 max_nan_de_entries;
>>> u8 tx_sk_pacing_shift;
>>> + u32 airtime_weight;
>>> };
>>
>> This doesn't make sense. Airtime weights can be set by userspace, so
>> even if a driver sets another default it is not guaranteed to be
>> honoured. So what's the point?
>>
> The reason for driver specific default is to avoid performance impact
> in ath10k when the user is using vanilla ath10k with default airtime.
> As I mentioned earlier, mac80211 default (256us) is too low for 11ac
> devices especially with driver is bursting aggregation.
>
> Yes. I do understand the user can change airtime at anytime but It
> must be noted that different airtime weight will result in different
> throughput. IMHO the defaults should not impact current benchmark.
> Otherwise it will be alarmed as regression later. isn't it?

My point is that if the user has to know the implementation-specific
limitations of each driver before setting a weight, then it's not a
particularly friendly API. I think we should be able to do better than
that...

>> So since we're rotating the queue on every call to the function, I'm
>> wondering if this actually succeeds in throttling the slow station's
>> airtime usage enough to achieve fairness? So I'll ask again: Did you
>> test the fairness performance, and how?
>>
> We are collecting data of mixed clients (11ac, 11n and legacy) keeping
> them at same distance and different distance. So that lower rate
> clients will interfere higher MCS rate station. Also configuring
> different airtime weight for each stations so that throttling low rate
> clients more should help higher performing(better MCS) clients.
>
> By allowing different airtime for each stations, the user can control
> guest network over primary network. Also It helped to improve
> performance of preferred station and algo. to control station is given
> to cloud or user application.
>
> As of now, We are testing with 4 11ac clients of same distance. And
> collecting the performance data in multiple iteration. Below are
> current data of station's performance (Mbps) against airtime weight.
>
> airtime station1(%airtime) station2 station3 station4
> (Mbps)
>
> No ATF 182 168 166 169
>
> 4ms 170 (100%) 164 (100%) 185 (100%) 175 (100%)
>
> 4ms 277 (70%) 115 (10%) 103 (10%) 105 (10%)
>
> 4ms 223 (40%) 214 (40%) 109 (10%) 94 (10%)
>
> 4ms 337 (90%) 182 (8%) 23 (1%) 30 (1%)

So this looks like it's doing *something*, but not like it's succeeding
in achieving the set percentages. Did you check if the actual airtime
values (in debugfs) corresponds to the configured weights?

>
> STA1(11ac) STA2 (11n) STA3(11a)
> ========== ========== =========
>
> No ATF 225 166 3.5
>
> ATF (4ms) 234 151 3.5

This also shows like ATF has no effect?

>> Also, taking a step back:
>>
>> With this, we're doing lots of work to make sure that the hardware's
>> round-robin scheduling queue lines up with mac80211; so if we do this
>> anyway, why can't we just control the order directly from mac80211
>> (which is what we do with the other next_txq() API)?
>>
> The only way to enforce mac80211 ordering in ath10k is to disable pull
> mode in firmware and always operates in push mode similar to ath9k.

And I assume that is not too likely to happen, or? What is the benefit
of pull mode at the firmware level?

-Toke

2018-10-28 22:01:44

by Rajkumar Manoharan

[permalink] [raw]
Subject: Re: [PATCH 3/6] mac80211: Add airtime accounting and scheduling to TXQs

On 2018-10-28 08:48, Toke Høiland-Jørgensen wrote:
> Rajkumar Manoharan <[email protected]> writes:
>
>> Yes. I do understand the user can change airtime at anytime but It
>> must be noted that different airtime weight will result in different
>> throughput. IMHO the defaults should not impact current benchmark.
>> Otherwise it will be alarmed as regression later. isn't it?
>
> My point is that if the user has to know the implementation-specific
> limitations of each driver before setting a weight, then it's not a
> particularly friendly API. I think we should be able to do better than
> that...
>
Hmm.. I was trying to balance throughput vs CPU usage. Keeping lower
threshold is causing more lock contention and impacting throughput.
I thought of configuring default airtime_weight from hostapd for
legacy, 11n and 11ac stations.

>> airtime station1(%airtime) station2 station3 station4
>> (Mbps)
>>
>> No ATF 182 168 166 169
>>
>> 4ms 170 (100%) 164 (100%) 185 (100%) 175 (100%)
>>
>> 4ms 277 (70%) 115 (10%) 103 (10%) 105 (10%)
>>
>> 4ms 223 (40%) 214 (40%) 109 (10%) 94 (10%)
>>
>> 4ms 337 (90%) 182 (8%) 23 (1%) 30 (1%)
>
> So this looks like it's doing *something*, but not like it's succeeding
> in achieving the set percentages. Did you check if the actual airtime
> values (in debugfs) corresponds to the configured weights?
>
No. Will check that.

>> The only way to enforce mac80211 ordering in ath10k is to disable pull
>> mode in firmware and always operates in push mode similar to ath9k.
>
> And I assume that is not too likely to happen, or? What is the benefit
> of pull mode at the firmware level?
>
Pull mode mainly required to support MU-MIMO. IMHO both MUMIMO & ATF can
not
co-exist unless both are implemented at host driver. Fixing tx mode in
ath10k
needs firmware reload. By introducing modparam, pull-mode can be
disabled.

-Rajkumar

2018-10-29 23:50:53

by Rajkumar Manoharan

[permalink] [raw]
Subject: Re: [PATCH 3/6] mac80211: Add airtime accounting and scheduling to TXQs

On 2018-10-28 15:01, Rajkumar Manoharan wrote:
> On 2018-10-28 08:48, Toke Høiland-Jørgensen wrote:
>> Rajkumar Manoharan <[email protected]> writes:
>>
>>>
>>> 4ms 223 (40%) 214 (40%) 109 (10%) 94 (10%)
>>>
>>> 4ms 337 (90%) 182 (8%) 23 (1%) 30 (1%)
>>
>> So this looks like it's doing *something*, but not like it's
>> succeeding
>> in achieving the set percentages. Did you check if the actual airtime
>> values (in debugfs) corresponds to the configured weights?
>>
> No. Will check that.
>
Toke,

From above results, different airtime for each station is reflecting on
output performance. Unfortunately I don't see such tput difference, when
the tx mode is fixed in push-only. Even low weight station is giving
same
performance. Are you also seeing the same behavior in your setup? Could
you please share your results?

Not sure why low weight station (26us) is consuming more airtime than
higher airtime station. Below result is taken in push-only mode that
means only next_txq() ordering is followed.

cat /sys/kernel/debug/ieee80211/phy0/netdev\:wlan0/stations/*/airtime
RX: 0 us
TX: 980443 us
Weight: 176
Deficit: VO: 256 us VI: 256 us BE: -91 us BK: 256 us
RX: 0 us
TX: 2008512 us
Weight: 26
Deficit: VO: 238 us VI: 256 us BE: 24 us BK: 256 us
RX: 0 us
TX: 513287 us
Weight: 26
Deficit: VO: 256 us VI: 256 us BE: 1 us BK: 256 us
RX: 0 us
TX: 576746 us
Weight: 26
Deficit: VO: 256 us VI: 256 us BE: 10 us BK: 256 us

-Rajkumar

2018-10-31 06:17:43

by Yibo Zhao

[permalink] [raw]
Subject: Re: [PATCH 3/6] mac80211: Add airtime accounting and scheduling to TXQs

在 2018-10-28 23:48,Toke Høiland-Jørgensen 写道:
> Rajkumar Manoharan <[email protected]> writes:
>
>> On 2018-10-26 07:16, Toke Høiland-Jørgensen wrote:
>>> Rajkumar Manoharan <[email protected]> writes:
>>>
>>>> From: Toke Høiland-Jørgensen <[email protected]>
>> [...]
>>>> u8 max_nan_de_entries;
>>>> u8 tx_sk_pacing_shift;
>>>> + u32 airtime_weight;
>>>> };
>>>
>>> This doesn't make sense. Airtime weights can be set by userspace, so
>>> even if a driver sets another default it is not guaranteed to be
>>> honoured. So what's the point?
>>>
>> The reason for driver specific default is to avoid performance impact
>> in ath10k when the user is using vanilla ath10k with default airtime.
>> As I mentioned earlier, mac80211 default (256us) is too low for 11ac
>> devices especially with driver is bursting aggregation.
>>
>> Yes. I do understand the user can change airtime at anytime but It
>> must be noted that different airtime weight will result in different
>> throughput. IMHO the defaults should not impact current benchmark.
>> Otherwise it will be alarmed as regression later. isn't it?
>
> My point is that if the user has to know the implementation-specific
> limitations of each driver before setting a weight, then it's not a
> particularly friendly API. I think we should be able to do better than
> that...
>
>>> So since we're rotating the queue on every call to the function, I'm
>>> wondering if this actually succeeds in throttling the slow station's
>>> airtime usage enough to achieve fairness? So I'll ask again: Did you
>>> test the fairness performance, and how?
>>>
>> We are collecting data of mixed clients (11ac, 11n and legacy) keeping
>> them at same distance and different distance. So that lower rate
>> clients will interfere higher MCS rate station. Also configuring
>> different airtime weight for each stations so that throttling low rate
>> clients more should help higher performing(better MCS) clients.
>>
>> By allowing different airtime for each stations, the user can control
>> guest network over primary network. Also It helped to improve
>> performance of preferred station and algo. to control station is given
>> to cloud or user application.
>>
>> As of now, We are testing with 4 11ac clients of same distance. And
>> collecting the performance data in multiple iteration. Below are
>> current data of station's performance (Mbps) against airtime weight.
>>
>> airtime station1(%airtime) station2 station3 station4
>> (Mbps)
>>
>> No ATF 182 168 166 169
>>
>> 4ms 170 (100%) 164 (100%) 185 (100%) 175 (100%)
>>
>> 4ms 277 (70%) 115 (10%) 103 (10%) 105 (10%)
>>
>> 4ms 223 (40%) 214 (40%) 109 (10%) 94 (10%)
>>
>> 4ms 337 (90%) 182 (8%) 23 (1%) 30 (1%)
>
> So this looks like it's doing *something*, but not like it's succeeding
> in achieving the set percentages. Did you check if the actual airtime
> values (in debugfs) corresponds to the configured weights?
>
>>
>> STA1(11ac) STA2 (11n) STA3(11a)
>> ========== ========== =========
>>
>> No ATF 225 166 3.5
>>
>> ATF (4ms) 234 151 3.5
>
> This also shows like ATF has no effect?
>
>>> Also, taking a step back:
>>>
>>> With this, we're doing lots of work to make sure that the hardware's
>>> round-robin scheduling queue lines up with mac80211; so if we do this
>>> anyway, why can't we just control the order directly from mac80211
>>> (which is what we do with the other next_txq() API)?
>>>
Toke and Raj,

How about treating any txqs before the txq that FW asked for in
push-pull mode
as pending txqs? Then we can dequeue and reorder them.And airtime weight
need
to be tuned to make sure it won't cost too much time. After that, check
the txq
FW wishes to fetch in txq_may_transmit.

Is this way able to achieve fairness and line up with mac80211?
Also, we may need to consider if FW supports in this way.

>> The only way to enforce mac80211 ordering in ath10k is to disable pull
>> mode in firmware and always operates in push mode similar to ath9k.
>
> And I assume that is not too likely to happen, or? What is the benefit
> of pull mode at the firmware level?
>
> -Toke

2018-11-02 10:30:28

by Toke Høiland-Jørgensen

[permalink] [raw]
Subject: Re: [PATCH 3/6] mac80211: Add airtime accounting and scheduling to TXQs

Rajkumar Manoharan <[email protected]> writes:

> On 2018-10-28 15:01, Rajkumar Manoharan wrote:
>> On 2018-10-28 08:48, Toke Høiland-Jørgensen wrote:
>>> Rajkumar Manoharan <[email protected]> writes:
>>>
>>>>
>>>> 4ms 223 (40%) 214 (40%) 109 (10%) 94 (10%)
>>>>
>>>> 4ms 337 (90%) 182 (8%) 23 (1%) 30 (1%)
>>>
>>> So this looks like it's doing *something*, but not like it's
>>> succeeding
>>> in achieving the set percentages. Did you check if the actual airtime
>>> values (in debugfs) corresponds to the configured weights?
>>>
>> No. Will check that.
>>
> Toke,
>
> From above results, different airtime for each station is reflecting on
> output performance. Unfortunately I don't see such tput difference, when
> the tx mode is fixed in push-only. Even low weight station is giving
> same
> performance. Are you also seeing the same behavior in your setup? Could
> you please share your results?

Sorry, I've been travelling this week; I'll be back in the office next
week and can run some tests then. I may also have an idea for a
different algorithm that will work better in pull mode, but need to see
if it works at all first :)

How do I force ath10k into push mode?

-Toke

2018-11-05 08:39:27

by Rajkumar Manoharan

[permalink] [raw]
Subject: Re: [PATCH 3/6] mac80211: Add airtime accounting and scheduling to TXQs

On 2018-11-02 03:30, Toke Høiland-Jørgensen wrote:
> Rajkumar Manoharan <[email protected]> writes:
>
>> On 2018-10-28 15:01, Rajkumar Manoharan wrote:
>>> On 2018-10-28 08:48, Toke Høiland-Jørgensen wrote:
>>>> Rajkumar Manoharan <[email protected]> writes:
>>>>
>>>>>
>>>>> 4ms 223 (40%) 214 (40%) 109 (10%) 94 (10%)
>>>>>
>>>>> 4ms 337 (90%) 182 (8%) 23 (1%) 30 (1%)
>>>>
>>>> So this looks like it's doing *something*, but not like it's
>>>> succeeding
>>>> in achieving the set percentages. Did you check if the actual
>>>> airtime
>>>> values (in debugfs) corresponds to the configured weights?
>>>>
>>> No. Will check that.
>>>
>> Toke,
>>
>> From above results, different airtime for each station is reflecting
>> on
>> output performance. Unfortunately I don't see such tput difference,
>> when
>> the tx mode is fixed in push-only. Even low weight station is giving
>> same
>> performance. Are you also seeing the same behavior in your setup?
>> Could
>> you please share your results?
>
> Sorry, I've been travelling this week; I'll be back in the office next
> week and can run some tests then. I may also have an idea for a
> different algorithm that will work better in pull mode, but need to see
> if it works at all first :)
>
Wow... :)

Meanwhile we did some more experiments with both modes. The experiment
was
done in open environment and fixed rate and UDP traffic ran for 60
seconds.

Seems like push mode not honoring the configured weight. Always the
airtime
was almost same whereas in pull-mode airtime is changing based on
configured
weight. Hence I would like to know your results.

sta1 sta2 sta3 sta4
pull-mode 8s(205us) 18s(3.2ms) 8s(205us) 14s(410us)
12s(256us) 12s(256us) 13s(256us) 12s(256us)
14s(4ms) 13s(4ms) 14s(4ms) 13s(4ms)

push-mode 15s(205us) 12s(3.2ms) 16s(205us) 12s(410us)
15s(256us) 12s(256us) 16s(256us) 12s(256us)
14s(4ms) 13s(4ms) 16s(4ms) 12s(4ms)


> How do I force ath10k into push mode?
>
Attaching the change to fix push mode.

-Rajkumar


Attachments:
push-only.patch (2.19 kB)

2018-11-07 14:53:11

by Toke Høiland-Jørgensen

[permalink] [raw]
Subject: Re: [PATCH 3/6] mac80211: Add airtime accounting and scheduling to TXQs

Rajkumar Manoharan <[email protected]> writes:

> On 2018-11-02 03:30, Toke Høiland-Jørgensen wrote:
>> Rajkumar Manoharan <[email protected]> writes:
>>
>>> On 2018-10-28 15:01, Rajkumar Manoharan wrote:
>>>> On 2018-10-28 08:48, Toke Høiland-Jørgensen wrote:
>>>>> Rajkumar Manoharan <[email protected]> writes:
>>>>>
>>>>>>
>>>>>> 4ms 223 (40%) 214 (40%) 109 (10%) 94 (10%)
>>>>>>
>>>>>> 4ms 337 (90%) 182 (8%) 23 (1%) 30 (1%)
>>>>>
>>>>> So this looks like it's doing *something*, but not like it's
>>>>> succeeding
>>>>> in achieving the set percentages. Did you check if the actual
>>>>> airtime
>>>>> values (in debugfs) corresponds to the configured weights?
>>>>>
>>>> No. Will check that.
>>>>
>>> Toke,
>>>
>>> From above results, different airtime for each station is reflecting
>>> on
>>> output performance. Unfortunately I don't see such tput difference,
>>> when
>>> the tx mode is fixed in push-only. Even low weight station is giving
>>> same
>>> performance. Are you also seeing the same behavior in your setup?
>>> Could
>>> you please share your results?
>>
>> Sorry, I've been travelling this week; I'll be back in the office next
>> week and can run some tests then. I may also have an idea for a
>> different algorithm that will work better in pull mode, but need to see
>> if it works at all first :)
>>
> Wow... :)
>
> Meanwhile we did some more experiments with both modes. The experiment
> was done in open environment and fixed rate and UDP traffic ran for 60
> seconds.
>
> Seems like push mode not honoring the configured weight. Always the
> airtime was almost same whereas in pull-mode airtime is changing based
> on configured weight. Hence I would like to know your results.

Right, so I verified that the current version of the patch set still
works with ath9k. However, the ath10k card I have doesn't seem to
support peer stats, so I can't test ath10k.

$ lspci | grep Qualcomm
03:00.0 Network controller: Qualcomm Atheros QCA986x/988x 802.11ac Wireless Network Adapter

$ cat /sys/kernel/debug/ieee80211/phy1/ath10k/chip_id
0x043202ff

$ cat /sys/kernel/debug/ieee80211/phy1/ath10k/wmi_services | grep PEER
WMI_SERVICE_PEER_CACHING -
WMI_SERVICE_PEER_STATS -


Is there a way to force-enable airtime support, is this a hardware issue?

> sta1 sta2 sta3 sta4
> pull-mode 8s(205us) 18s(3.2ms) 8s(205us) 14s(410us)
> 12s(256us) 12s(256us) 13s(256us) 12s(256us)
> 14s(4ms) 13s(4ms) 14s(4ms) 13s(4ms)
>
> push-mode 15s(205us) 12s(3.2ms) 16s(205us) 12s(410us)
> 15s(256us) 12s(256us) 16s(256us) 12s(256us)
> 14s(4ms) 13s(4ms) 16s(4ms) 12s(4ms)

Right, so the pull-mode results are encouraging! *Something* is
happening, at least, even though the aggregate airtime doesn't quite
match the ratios of the configured weights.

Are you running the UDP generator on the AP itself, or on a separate
device, BTW? If it's on the AP, the userspace socket can get throttled,
which will interfere with results, so it's better to have it on a
separate device (connected via ethernet).

As for push-mode, could this be because of bad buffer management? I.e.,
because there's a lag between the time airtime is registered, and the
time that airtime usage is reported, the driver just pushes a whole
bunch of packets to the firmware when it gets the chance, which prevents
the scheduler from throttling properly. This could also explain the
better, but not quite perfect, results in pull mode, assuming that pull
mode results in better firmware buffer management which reduces, but
doesn't quite remove, the lag.

If this is indeed the reason, the queue limit patches should hopefully
be a solution. So guess we need to get those working as well :)

-Toke

2018-11-07 22:35:33

by Rajkumar Manoharan

[permalink] [raw]
Subject: Re: [PATCH 3/6] mac80211: Add airtime accounting and scheduling to TXQs

On 2018-11-07 06:53, Toke Høiland-Jørgensen wrote:
> Rajkumar Manoharan <[email protected]> writes:
>
>> Meanwhile we did some more experiments with both modes. The experiment
>> was done in open environment and fixed rate and UDP traffic ran for 60
>> seconds.
>>
>> Seems like push mode not honoring the configured weight. Always the
>> airtime was almost same whereas in pull-mode airtime is changing based
>> on configured weight. Hence I would like to know your results.
>
> Right, so I verified that the current version of the patch set still
> works with ath9k. However, the ath10k card I have doesn't seem to
> support peer stats, so I can't test ath10k.
>
> $ lspci | grep Qualcomm
> 03:00.0 Network controller: Qualcomm Atheros QCA986x/988x 802.11ac
> Wireless Network Adapter
>
> $ cat /sys/kernel/debug/ieee80211/phy1/ath10k/chip_id
> 0x043202ff
>
> $ cat /sys/kernel/debug/ieee80211/phy1/ath10k/wmi_services | grep PEER
> WMI_SERVICE_PEER_CACHING -
> WMI_SERVICE_PEER_STATS -
>

Oops... Yeah 988x firmware (10.2.4) does not have peer stats support.

>
> Is there a way to force-enable airtime support, is this a hardware
> issue?
>
Unfortunately not. There is one more pending change that handles airtime
report
from HTT tx-compl. Again it depends firmware support. These experiments
are
taken with this f/w interface. Will post the change.

>> sta1 sta2 sta3 sta4
>> pull-mode 8s(205us) 18s(3.2ms) 8s(205us) 14s(410us)
>> 12s(256us) 12s(256us) 13s(256us) 12s(256us)
>> 14s(4ms) 13s(4ms) 14s(4ms) 13s(4ms)
>>
>> push-mode 15s(205us) 12s(3.2ms) 16s(205us) 12s(410us)
>> 15s(256us) 12s(256us) 16s(256us) 12s(256us)
>> 14s(4ms) 13s(4ms) 16s(4ms) 12s(4ms)
>
> Right, so the pull-mode results are encouraging! *Something* is
> happening, at least, even though the aggregate airtime doesn't quite
> match the ratios of the configured weights.
>
> Are you running the UDP generator on the AP itself, or on a separate
> device, BTW? If it's on the AP, the userspace socket can get throttled,
> which will interfere with results, so it's better to have it on a
> separate device (connected via ethernet).
>
Traffic b/w wired client (via ethernet) and wireless clients.

> As for push-mode, could this be because of bad buffer management? I.e.,
> because there's a lag between the time airtime is registered, and the
> time that airtime usage is reported, the driver just pushes a whole
> bunch of packets to the firmware when it gets the chance, which
> prevents
> the scheduler from throttling properly. This could also explain the
> better, but not quite perfect, results in pull mode, assuming that pull
> mode results in better firmware buffer management which reduces, but
> doesn't quite remove, the lag.
>
Hmm... I agree that lag in reporting airtime may cause more data push to
hw.
Right now both tx and tx-compl are serialized by active_txq_lock. So
once
lock acquired by tx path, it will download all frames. i.e it is even
true for
ath9k driver. Hence I am wondering how it is working only with ath9k.

In ath10k, The airtime always be reported in tx-completion. I dont see
much lag
from my experiments.

> If this is indeed the reason, the queue limit patches should hopefully
> be a solution. So guess we need to get those working as well :)
>
I would prefer to baseline the basic infra into upstream first and do
enhancement
on top of that. I request you to revisit maintaining per driver default.
Otherwise
there would be performance impact with 256us. :(

-Rajkumar

2018-11-08 13:46:10

by Toke Høiland-Jørgensen

[permalink] [raw]
Subject: Re: [PATCH 3/6] mac80211: Add airtime accounting and scheduling to TXQs

Rajkumar Manoharan <[email protected]> writes:

> On 2018-11-07 06:53, Toke Høiland-Jørgensen wrote:
>> Rajkumar Manoharan <[email protected]> writes:
>>
>>> Meanwhile we did some more experiments with both modes. The experiment
>>> was done in open environment and fixed rate and UDP traffic ran for 60
>>> seconds.
>>>
>>> Seems like push mode not honoring the configured weight. Always the
>>> airtime was almost same whereas in pull-mode airtime is changing based
>>> on configured weight. Hence I would like to know your results.
>>
>> Right, so I verified that the current version of the patch set still
>> works with ath9k. However, the ath10k card I have doesn't seem to
>> support peer stats, so I can't test ath10k.
>>
>> $ lspci | grep Qualcomm
>> 03:00.0 Network controller: Qualcomm Atheros QCA986x/988x 802.11ac
>> Wireless Network Adapter
>>
>> $ cat /sys/kernel/debug/ieee80211/phy1/ath10k/chip_id
>> 0x043202ff
>>
>> $ cat /sys/kernel/debug/ieee80211/phy1/ath10k/wmi_services | grep PEER
>> WMI_SERVICE_PEER_CACHING -
>> WMI_SERVICE_PEER_STATS -
>>
>
> Oops... Yeah 988x firmware (10.2.4) does not have peer stats support.
>
>>
>> Is there a way to force-enable airtime support, is this a hardware
>> issue?
>>
> Unfortunately not. There is one more pending change that handles
> airtime report from HTT tx-compl. Again it depends firmware support.
> These experiments are taken with this f/w interface. Will post the
> change.

Right, thought so. Too bad. Guess you are doing all the ath10k testing,
then ;)

>>> sta1 sta2 sta3 sta4
>>> pull-mode 8s(205us) 18s(3.2ms) 8s(205us) 14s(410us)
>>> 12s(256us) 12s(256us) 13s(256us) 12s(256us)
>>> 14s(4ms) 13s(4ms) 14s(4ms) 13s(4ms)
>>>
>>> push-mode 15s(205us) 12s(3.2ms) 16s(205us) 12s(410us)
>>> 15s(256us) 12s(256us) 16s(256us) 12s(256us)
>>> 14s(4ms) 13s(4ms) 16s(4ms) 12s(4ms)
>>
>> Right, so the pull-mode results are encouraging! *Something* is
>> happening, at least, even though the aggregate airtime doesn't quite
>> match the ratios of the configured weights.
>>
>> Are you running the UDP generator on the AP itself, or on a separate
>> device, BTW? If it's on the AP, the userspace socket can get throttled,
>> which will interfere with results, so it's better to have it on a
>> separate device (connected via ethernet).
>>
> Traffic b/w wired client (via ethernet) and wireless clients.

Cool.

>> As for push-mode, could this be because of bad buffer management? I.e.,
>> because there's a lag between the time airtime is registered, and the
>> time that airtime usage is reported, the driver just pushes a whole
>> bunch of packets to the firmware when it gets the chance, which
>> prevents
>> the scheduler from throttling properly. This could also explain the
>> better, but not quite perfect, results in pull mode, assuming that pull
>> mode results in better firmware buffer management which reduces, but
>> doesn't quite remove, the lag.
>>
> Hmm... I agree that lag in reporting airtime may cause more data push
> to hw. Right now both tx and tx-compl are serialized by
> active_txq_lock. So once lock acquired by tx path, it will download
> all frames. i.e it is even true for ath9k driver. Hence I am wondering
> how it is working only with ath9k.

If enough packets are dequeued at once that the TXQ empties and is not
put back on the scheduling loop, the next_txq() loop is just going to
loop through the remaining TXQs and immediately increase their quantum.
In ath9k, there's a maximum of two outstanding aggregates below the TXQ
level, but I think you mentioned that ath10k can queue thousands in
firmware?

Your patch removes the 'max 16 packets at a time' check before the call
to ath10k_mac_tx_push_txq(); try adding that back and see if it helps?

>> If this is indeed the reason, the queue limit patches should hopefully
>> be a solution. So guess we need to get those working as well :)
>>
> I would prefer to baseline the basic infra into upstream first and do
> enhancement on top of that.

Sure, I'm fine with merging this first and building on top.

> I request you to revisit maintaining per driver default. Otherwise
> there would be performance impact with 256us. :(

Hmm, how about we make it a driver-specified multiplier instead (which
could be 4, 8 or 16 for ath10k)? That way it would still work even if
userspace changes the weights. It would affect the minimum possible
granularity, but that is probably acceptable; and we would be free to
change the mechanism later, without affecting the userspace API.

-Toke

2018-11-09 12:00:11

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/6] mac80211: Add TXQ scheduling API

On Sat, 2018-10-20 at 04:05 -0700, Rajkumar Manoharan wrote:
>
> + * @txq: pointer obtained from station or virtual interface, or from
> + * ieee80211_next_txq()

nit: please just indent by a single tab for continuation, trying to line
it all up doesn't really make it more readable

I see you're still discussing all this, so I'm going to assume you'll
resend this series eventually :-)

johannes


2018-11-09 12:39:13

by Toke Høiland-Jørgensen

[permalink] [raw]
Subject: Re: [PATCH 1/6] mac80211: Add TXQ scheduling API



On 9 November 2018 13:00:04 CET, Johannes Berg <[email protected]> wrote:
>On Sat, 2018-10-20 at 04:05 -0700, Rajkumar Manoharan wrote:
>>
>> + * @txq: pointer obtained from station or virtual interface, or from
>> + * ieee80211_next_txq()
>
>nit: please just indent by a single tab for continuation, trying to
>line
>it all up doesn't really make it more readable

Not sure I agree about the "not more readable" part, but OK.

>I see you're still discussing all this, so I'm going to assume you'll
>resend this series eventually :-)

Yup. Think we're getting close to something that might be mergeable. Will send another version then :)

-Toke