Skip to content

Commit

Permalink
mac80211: add airtime fairness improvements
Browse files Browse the repository at this point in the history
This reverts the airtime scheduler back from the virtual-time based scheduler
to the deficit round robin scheduler implementation.
This reduces burstiness and improves fairness by improving interaction with AQL.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
  • Loading branch information
nbd168 committed Jun 25, 2022
1 parent 4d1fc89 commit 6d49a25
Show file tree
Hide file tree
Showing 9 changed files with 1,698 additions and 6 deletions.

Large diffs are not rendered by default.

@@ -0,0 +1,52 @@
From: Felix Fietkau <nbd@nbd.name>
Date: Mon, 20 Jun 2022 14:53:04 +0200
Subject: [PATCH] mac80211: make sta airtime deficit field s32 instead of
s64

32 bit is more than enough range for the airtime deficit

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---

--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -202,7 +202,7 @@ static ssize_t sta_airtime_read(struct f
size_t bufsz = 400;
char *buf = kzalloc(bufsz, GFP_KERNEL), *p = buf;
u64 rx_airtime = 0, tx_airtime = 0;
- s64 deficit[IEEE80211_NUM_ACS];
+ s32 deficit[IEEE80211_NUM_ACS];
ssize_t rv;
int ac;

@@ -219,7 +219,7 @@ static ssize_t sta_airtime_read(struct f

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",
+ "Deficit: VO: %d us VI: %d us BE: %d us BK: %d us\n",
rx_airtime, tx_airtime, sta->airtime_weight,
deficit[0], deficit[1], deficit[2], deficit[3]);

--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -138,7 +138,7 @@ enum ieee80211_agg_stop_reason {
struct airtime_info {
u64 rx_airtime;
u64 tx_airtime;
- s64 deficit;
+ s32 deficit;
atomic_t aql_tx_pending; /* Estimated airtime for frames pending */
u32 aql_limit_low;
u32 aql_limit_high;
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3847,7 +3847,7 @@ struct ieee80211_txq *ieee80211_next_txq
struct sta_info *sta = container_of(txqi->txq.sta,
struct sta_info, sta);
bool aql_check = ieee80211_txq_airtime_check(hw, &txqi->txq);
- s64 deficit = sta->airtime[txqi->txq.ac].deficit;
+ s32 deficit = sta->airtime[txqi->txq.ac].deficit;

if (aql_check)
found_eligible_txq = true;
@@ -0,0 +1,48 @@
From: Felix Fietkau <nbd@nbd.name>
Date: Mon, 20 Jun 2022 14:59:09 +0200
Subject: [PATCH] mac80211: consider aql_tx_pending when checking airtime
deficit

When queueing packets for a station, deficit only gets added once the packets
have been transmitted, which could be much later. During that time, a lot of
temporary unfairness could happen, which could lead to bursty behavior.
Fix this by subtracting the aql_tx_pending when checking the deficit in tx
scheduling.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---

--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3817,6 +3817,13 @@ out:
}
EXPORT_SYMBOL(ieee80211_tx_dequeue);

+static inline s32 ieee80211_sta_deficit(struct sta_info *sta, u8 ac)
+{
+ struct airtime_info *air_info = &sta->airtime[ac];
+
+ return air_info->deficit - atomic_read(&air_info->aql_tx_pending);
+}
+
struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac)
{
struct ieee80211_local *local = hw_to_local(hw);
@@ -3847,7 +3854,7 @@ struct ieee80211_txq *ieee80211_next_txq
struct sta_info *sta = container_of(txqi->txq.sta,
struct sta_info, sta);
bool aql_check = ieee80211_txq_airtime_check(hw, &txqi->txq);
- s32 deficit = sta->airtime[txqi->txq.ac].deficit;
+ s32 deficit = ieee80211_sta_deficit(sta, txqi->txq.ac);

if (aql_check)
found_eligible_txq = true;
@@ -3972,7 +3979,7 @@ bool ieee80211_txq_may_transmit(struct i
continue;
}
sta = container_of(iter->txq.sta, struct sta_info, sta);
- if (sta->airtime[ac].deficit < 0)
+ if (ieee80211_sta_deficit(sta, ac) < 0)
sta->airtime[ac].deficit += sta->airtime_weight;
list_move_tail(&iter->schedule_order, &local->active_txqs[ac]);
}
@@ -0,0 +1,118 @@
From: Felix Fietkau <nbd@nbd.name>
Date: Mon, 20 Jun 2022 20:52:50 +0200
Subject: [PATCH] mac80211: keep recently active tx queues in scheduling
list

This allows proper deficit accounting to ensure that they don't carry their
deficit until the next time they become active

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---

--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -83,6 +83,13 @@ extern const u8 ieee80211_ac_to_qos_mask

#define IEEE80211_MAX_NAN_INSTANCE_ID 255

+
+/*
+ * Keep a station's queues on the active list for deficit accounting purposes
+ * if it was active or queued during the last 100ms
+ */
+#define AIRTIME_ACTIVE_DURATION (HZ / 10)
+
struct ieee80211_bss {
u32 device_ts_beacon, device_ts_presp;

--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -138,6 +138,7 @@ enum ieee80211_agg_stop_reason {
struct airtime_info {
u64 rx_airtime;
u64 tx_airtime;
+ u32 last_active;
s32 deficit;
atomic_t aql_tx_pending; /* Estimated airtime for frames pending */
u32 aql_limit_low;
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3824,6 +3824,36 @@ static inline s32 ieee80211_sta_deficit(
return air_info->deficit - atomic_read(&air_info->aql_tx_pending);
}

+static void
+ieee80211_txq_set_active(struct txq_info *txqi)
+{
+ struct sta_info *sta;
+
+ if (!txqi->txq.sta)
+ return;
+
+ sta = container_of(txqi->txq.sta, struct sta_info, sta);
+ sta->airtime[txqi->txq.ac].last_active = (u32)jiffies;
+}
+
+static bool
+ieee80211_txq_keep_active(struct txq_info *txqi)
+{
+ struct sta_info *sta;
+ u32 diff;
+
+ if (!txqi->txq.sta)
+ return false;
+
+ sta = container_of(txqi->txq.sta, struct sta_info, sta);
+ if (ieee80211_sta_deficit(sta, txqi->txq.ac) >= 0)
+ return false;
+
+ diff = (u32)jiffies - sta->airtime[txqi->txq.ac].last_active;
+
+ return diff <= AIRTIME_ACTIVE_DURATION;
+}
+
struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac)
{
struct ieee80211_local *local = hw_to_local(hw);
@@ -3870,7 +3900,6 @@ struct ieee80211_txq *ieee80211_next_txq
}
}

-
if (txqi->schedule_round == local->schedule_round[ac])
goto out;

@@ -3890,12 +3919,13 @@ void __ieee80211_schedule_txq(struct iee
{
struct ieee80211_local *local = hw_to_local(hw);
struct txq_info *txqi = to_txq_info(txq);
+ bool has_queue;

spin_lock_bh(&local->active_txq_lock[txq->ac]);

+ has_queue = force || txq_has_queue(txq);
if (list_empty(&txqi->schedule_order) &&
- (force || !skb_queue_empty(&txqi->frags) ||
- txqi->tin.backlog_packets)) {
+ (has_queue || ieee80211_txq_keep_active(txqi))) {
/* 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
@@ -3903,7 +3933,7 @@ void __ieee80211_schedule_txq(struct iee
* get immediately moved to the back of the list on the next
* call to ieee80211_next_txq().
*/
- if (txqi->txq.sta && local->airtime_flags &&
+ if (txqi->txq.sta && local->airtime_flags && has_queue &&
wiphy_ext_feature_isset(local->hw.wiphy,
NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
list_add(&txqi->schedule_order,
@@ -3911,6 +3941,8 @@ void __ieee80211_schedule_txq(struct iee
else
list_add_tail(&txqi->schedule_order,
&local->active_txqs[txq->ac]);
+ if (has_queue)
+ ieee80211_txq_set_active(txqi);
}

spin_unlock_bh(&local->active_txq_lock[txq->ac]);
@@ -0,0 +1,131 @@
From: Felix Fietkau <nbd@nbd.name>
Date: Mon, 20 Jun 2022 21:26:34 +0200
Subject: [PATCH] mac80211: add a per-PHY AQL limit to improve fairness

In order to maintain fairness, the amount of queueing needs to be limited
beyond the simple per-station AQL budget, otherwise the driver can simply
repeatedly do scheduling rounds until all queues that have not used their
AQL budget become eligble.

To be conservative, use the high AQL limit for the first txq and add half
of the low AQL for each subsequent queue.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---

--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1211,6 +1211,7 @@ struct ieee80211_local {
u32 aql_txq_limit_high[IEEE80211_NUM_ACS];
u32 aql_threshold;
atomic_t aql_total_pending_airtime;
+ atomic_t aql_ac_pending_airtime[IEEE80211_NUM_ACS];

const struct ieee80211_ops *ops;

--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -712,6 +712,7 @@ struct ieee80211_hw *ieee80211_alloc_hw_
local->aql_txq_limit_low[i] = IEEE80211_DEFAULT_AQL_TXQ_LIMIT_L;
local->aql_txq_limit_high[i] =
IEEE80211_DEFAULT_AQL_TXQ_LIMIT_H;
+ atomic_set(&local->aql_ac_pending_airtime[i], 0);
}

local->airtime_flags = AIRTIME_USE_TX | AIRTIME_USE_RX;
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1929,6 +1929,7 @@ void ieee80211_sta_update_pending_airtim
&sta->airtime[ac].aql_tx_pending);

atomic_add(tx_airtime, &local->aql_total_pending_airtime);
+ atomic_add(tx_airtime, &local->aql_ac_pending_airtime[ac]);
return;
}

@@ -1940,14 +1941,17 @@ void ieee80211_sta_update_pending_airtim
tx_pending, 0);
}

+ atomic_sub(tx_airtime, &local->aql_total_pending_airtime);
tx_pending = atomic_sub_return(tx_airtime,
- &local->aql_total_pending_airtime);
+ &local->aql_ac_pending_airtime[ac]);
if (WARN_ONCE(tx_pending < 0,
"Device %s AC %d pending airtime underflow: %u, %u",
wiphy_name(local->hw.wiphy), ac, tx_pending,
- tx_airtime))
- atomic_cmpxchg(&local->aql_total_pending_airtime,
+ tx_airtime)) {
+ atomic_cmpxchg(&local->aql_ac_pending_airtime[ac],
tx_pending, 0);
+ atomic_sub(tx_pending, &local->aql_total_pending_airtime);
+ }
}

int sta_info_move_state(struct sta_info *sta,
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3863,6 +3863,9 @@ struct ieee80211_txq *ieee80211_next_txq

spin_lock_bh(&local->active_txq_lock[ac]);

+ if (!local->schedule_round[ac])
+ goto out;
+
begin:
txqi = list_first_entry_or_null(&local->active_txqs[ac],
struct txq_info,
@@ -3984,6 +3987,25 @@ bool ieee80211_txq_airtime_check(struct
}
EXPORT_SYMBOL(ieee80211_txq_airtime_check);

+static bool
+ieee80211_txq_schedule_airtime_check(struct ieee80211_local *local, u8 ac)
+{
+ unsigned int num_txq = 0;
+ struct txq_info *txq;
+ u32 aql_limit;
+
+ if (!wiphy_ext_feature_isset(local->hw.wiphy, NL80211_EXT_FEATURE_AQL))
+ return true;
+
+ list_for_each_entry(txq, &local->active_txqs[ac], schedule_order)
+ num_txq++;
+
+ aql_limit = (num_txq - 1) * local->aql_txq_limit_low[ac] / 2 +
+ local->aql_txq_limit_high[ac];
+
+ return atomic_read(&local->aql_ac_pending_airtime[ac]) < aql_limit;
+}
+
bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
struct ieee80211_txq *txq)
{
@@ -4000,6 +4022,9 @@ bool ieee80211_txq_may_transmit(struct i
if (list_empty(&txqi->schedule_order))
goto out;

+ if (!ieee80211_txq_schedule_airtime_check(local, ac))
+ goto out;
+
list_for_each_entry_safe(iter, tmp, &local->active_txqs[ac],
schedule_order) {
if (iter == txqi)
@@ -4039,7 +4064,15 @@ void ieee80211_txq_schedule_start(struct
struct ieee80211_local *local = hw_to_local(hw);

spin_lock_bh(&local->active_txq_lock[ac]);
- local->schedule_round[ac]++;
+
+ if (ieee80211_txq_schedule_airtime_check(local, ac)) {
+ local->schedule_round[ac]++;
+ if (!local->schedule_round[ac])
+ local->schedule_round[ac]++;
+ } else {
+ local->schedule_round[ac] = 0;
+ }
+
spin_unlock_bh(&local->active_txq_lock[ac]);
}
EXPORT_SYMBOL(ieee80211_txq_schedule_start);

0 comments on commit 6d49a25

Please sign in to comment.