mac80211: sync airtime fairness fixes with updated upstream submission

- fix ath10k latency issues
- reject too large weight values
- code cleanup

Signed-off-by: Felix Fietkau <nbd@nbd.name>
This commit is contained in:
Felix Fietkau 2022-06-14 11:06:25 +02:00
parent dae3927527
commit 958785508c
6 changed files with 239 additions and 90 deletions

View File

@ -12,14 +12,26 @@ Fix this by reordering multiplications/shifts and by reducing unnecessary
intermediate precision (which was lost in a later stage anyway).
The new shift value limits the maximum weight to 4096, which should be more
than enough. Any values bigger than that will be clamped to the upper limit.
than enough. Any values bigger than that will be rejected.
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1602,6 +1602,9 @@ static int sta_apply_parameters(struct i
mask = params->sta_flags_mask;
set = params->sta_flags_set;
+ if (params->airtime_weight > BIT(IEEE80211_RECIPROCAL_SHIFT_STA))
+ return -EINVAL;
+
if (ieee80211_vif_is_mesh(&sdata->vif)) {
/*
* In mesh mode, ASSOCIATED isn't part of the nl80211
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1666,50 +1666,34 @@ static inline struct airtime_info *to_ai
@@ -1666,50 +1666,33 @@ static inline struct airtime_info *to_ai
/* To avoid divisions in the fast path, we keep pre-computed reciprocals for
* airtime weight calculations. There are two different weights to keep track
* of: The per-station weight and the sum of weights per phy.
@ -47,7 +59,6 @@ Signed-off-by: Felix Fietkau <nbd@nbd.name>
-static inline void airtime_weight_set(struct airtime_info *air_info, u16 weight)
+static inline void airtime_weight_set(struct airtime_info *air_info, u32 weight)
{
+ weight = min_t(u32, weight, BIT(IEEE80211_RECIPROCAL_SHIFT_STA));
if (air_info->weight == weight)
return;

View File

@ -0,0 +1,80 @@
From: Felix Fietkau <nbd@nbd.name>
Date: Sat, 11 Jun 2022 16:34:32 +0200
Subject: [PATCH] mac80211: improve AQL tx time estimation
If airtime cannot be calculated because of missing or unsupported rate info,
use the smallest possible non-zero value for estimated tx time.
This improves handling of these cases by preventing queueing of as many packets
as the driver/hardware queue can hold for these stations.
Also slightly improve limiting queueing by explicitly rounding up small values.
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1107,20 +1107,24 @@ struct ieee80211_tx_info {
};
};
+#define IEEE80211_TX_TIME_EST_UNIT 4
+
+static inline u16
+ieee80211_info_get_tx_time_est(struct ieee80211_tx_info *info)
+{
+ return info->tx_time_est * IEEE80211_TX_TIME_EST_UNIT;
+}
+
static inline u16
ieee80211_info_set_tx_time_est(struct ieee80211_tx_info *info, u16 tx_time_est)
{
/* We only have 10 bits in tx_time_est, so store airtime
* in increments of 4us and clamp the maximum to 2**12-1
*/
- info->tx_time_est = min_t(u16, tx_time_est, 4095) >> 2;
- return info->tx_time_est << 2;
-}
+ tx_time_est = DIV_ROUND_UP(tx_time_est, IEEE80211_TX_TIME_EST_UNIT);
+ info->tx_time_est = min_t(u16, tx_time_est, BIT(10) - 1);
-static inline u16
-ieee80211_info_get_tx_time_est(struct ieee80211_tx_info *info)
-{
- return info->tx_time_est << 2;
+ return ieee80211_info_get_tx_time_est(info);
}
/**
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -999,6 +999,8 @@ static void __ieee80211_tx_status(struct
NULL,
skb->len,
false);
+ if (!airtime)
+ airtime = IEEE80211_TX_TIME_EST_UNIT;
ieee80211_register_airtime(txq, airtime, 0);
}
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3798,13 +3798,12 @@ encap_out:
airtime = ieee80211_calc_expected_tx_airtime(hw, vif, txq->sta,
skb->len, ampdu);
- if (airtime) {
- airtime = ieee80211_info_set_tx_time_est(info, airtime);
- ieee80211_sta_update_pending_airtime(local, tx.sta,
- txq->ac,
- airtime,
- false);
- }
+ if (!airtime)
+ airtime = IEEE80211_TX_TIME_EST_UNIT;
+
+ airtime = ieee80211_info_set_tx_time_est(info, airtime);
+ ieee80211_sta_update_pending_airtime(local, tx.sta, txq->ac,
+ airtime, false);
}
return skb;

View File

@ -0,0 +1,91 @@
From: Felix Fietkau <nbd@nbd.name>
Date: Sat, 11 Jun 2022 17:28:02 +0200
Subject: [PATCH] mac80211: fix ieee80211_txq_may_transmit regression
After switching to the virtual time based airtime scheduler, there were reports
that ath10k with tx queueing in push-pull mode was experiencing significant
latency for some stations.
The reason for it is the fact that queues from which the ath10k firmware wants
to pull are getting starved by airtime fairness constraints.
Theoretically the same issue should have been there before the switch to virtual
time, however it seems that in the old round-robin implementation it was simply
looping until the requested txq was considered eligible, which led to it pretty
much ignoring fairness constraints anyway.
In order to fix the immediate regression, let's make bypassing airtime fairness
explicit for now.
Also update the documentation for ieee80211_txq_may_transmit, which was still
referring to implementation details of the old round-robin scheduler
Fixes: 2433647bc8d9 ("mac80211: Switch to a virtual time-based airtime scheduler")
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -6700,22 +6700,11 @@ void ieee80211_return_txq(struct ieee802
/**
* 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 there is remaining AQL budget for the tx queue and %false
+ * if it should be throttled. It will also mark the queue as active for the
+ * airtime scheduler.
*
- * 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 against 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
+ * 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()
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -4100,15 +4100,13 @@ EXPORT_SYMBOL(ieee80211_txq_airtime_chec
bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
struct ieee80211_txq *txq)
{
- struct txq_info *first_txqi = NULL, *txqi = to_txq_info(txq);
struct ieee80211_local *local = hw_to_local(hw);
+ struct txq_info *txqi = to_txq_info(txq);
struct airtime_sched_info *air_sched;
struct airtime_info *air_info;
- struct rb_node *node = NULL;
bool ret = false;
u64 now;
-
if (!ieee80211_txq_airtime_check(hw, txq))
return false;
@@ -4120,19 +4118,6 @@ bool ieee80211_txq_may_transmit(struct i
now = ktime_get_coarse_boottime_ns();
- /* Like in ieee80211_next_txq(), make sure the first station in the
- * scheduling order is eligible for transmission to avoid starvation.
- */
- node = rb_first_cached(&air_sched->active_txqs);
- if (node) {
- first_txqi = container_of(node, struct txq_info,
- schedule_order);
- air_info = to_airtime_info(&first_txqi->txq);
-
- if (air_sched->v_t < air_info->v_t)
- airtime_catchup_v_t(air_sched, air_info->v_t, now);
- }
-
air_info = to_airtime_info(&txqi->txq);
if (air_info->v_t <= air_sched->v_t) {
air_sched->last_schedule_activity = now;

View File

@ -375,7 +375,7 @@ Signed-off-by: Felix Fietkau <nbd@nbd.name>
/* To avoid divisions in the fast path, we keep pre-computed reciprocals for
* airtime weight calculations. There are two different weights to keep track
* of: The per-station weight and the sum of weights per phy.
@@ -1750,6 +1764,7 @@ static inline void init_airtime_info(str
@@ -1749,6 +1763,7 @@ static inline void init_airtime_info(str
air_info->aql_limit_high = air_sched->aql_txq_limit_high;
airtime_weight_set(air_info, IEEE80211_DEFAULT_AIRTIME_WEIGHT);
INIT_LIST_HEAD(&air_info->list);
@ -487,7 +487,7 @@ Signed-off-by: Felix Fietkau <nbd@nbd.name>
WARN_ON_ONCE(softirq_count() == 0);
@@ -3791,21 +3801,26 @@ begin:
@@ -3791,20 +3801,35 @@ begin:
encap_out:
IEEE80211_SKB_CB(skb)->control.vif = vif;
@ -498,13 +498,12 @@ Signed-off-by: Felix Fietkau <nbd@nbd.name>
-
- airtime = ieee80211_calc_expected_tx_airtime(hw, vif, txq->sta,
- skb->len, ampdu);
- if (airtime) {
- airtime = ieee80211_info_set_tx_time_est(info, airtime);
- ieee80211_sta_update_pending_airtime(local, tx.sta,
- txq->ac,
- airtime,
- false);
- }
- if (!airtime)
- airtime = IEEE80211_TX_TIME_EST_UNIT;
-
- airtime = ieee80211_info_set_tx_time_est(info, airtime);
- ieee80211_sta_update_pending_airtime(local, tx.sta, txq->ac,
- airtime, false);
- }
+ if (!vif)
+ return skb;
@ -513,8 +512,17 @@ Signed-off-by: Felix Fietkau <nbd@nbd.name>
+ airtime = ieee80211_calc_expected_tx_airtime(hw, vif, txq->sta,
+ skb->len, ampdu);
+ if (!airtime)
+ return skb;
+ airtime = IEEE80211_TX_TIME_EST_UNIT;
+
+ /*
+ * Tx queue scheduling always happens in airtime order and queues are
+ * sorted by virtual time + pending AQL budget.
+ * If AQL is not supported, pending AQL budget is always zero.
+ * If airtime fairness is not supported, virtual time won't be directly
+ * increased by driver tx completion.
+ * Because of that, we register estimated tx time as airtime if either
+ * AQL or ATF support is missing.
+ */
+ if (!wiphy_ext_feature_isset(local->hw.wiphy, NL80211_EXT_FEATURE_AQL) ||
+ !wiphy_ext_feature_isset(local->hw.wiphy,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
@ -529,17 +537,18 @@ Signed-off-by: Felix Fietkau <nbd@nbd.name>
return skb;
@@ -3816,85 +3831,95 @@ out:
@@ -3815,85 +3840,92 @@ out:
}
EXPORT_SYMBOL(ieee80211_tx_dequeue);
+static void
+static struct ieee80211_txq *
+airtime_info_next_txq_idx(struct airtime_info *air_info)
+{
+ air_info->txq_idx++;
+ if (air_info->txq_idx >= ARRAY_SIZE(air_info->txq) ||
+ !air_info->txq[air_info->txq_idx])
+ air_info->txq_idx = 0;
+ return air_info->txq[air_info->txq_idx];
+}
+
struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac)
@ -587,10 +596,9 @@ Signed-off-by: Felix Fietkau <nbd@nbd.name>
-
- if (!ieee80211_txq_airtime_check(hw, &txqi->txq)) {
- first = false;
+ airtime_info_next_txq_idx(air_info);
+ txq = airtime_info_next_txq_idx(air_info);
+ txq_idx = air_info->txq_idx;
+ txq = air_info->txq[txq_idx];
+ if (!txq || !ieee80211_txq_airtime_check(hw, txq))
+ if (!ieee80211_txq_airtime_check(hw, txq))
goto begin;
+
+ while (1) {
@ -601,17 +609,14 @@ Signed-off-by: Felix Fietkau <nbd@nbd.name>
+ if (txq_has_queue(txq))
+ break;
+
+ airtime_info_next_txq_idx(air_info);
+ txq = air_info->txq[air_info->txq_idx];
+ txq = airtime_info_next_txq_idx(air_info);
+ if (txq_idx == air_info->txq_idx)
+ goto begin;
+ }
+
+ if (air_info->v_t_cur > air_sched->v_t) {
+ if (node == airtime_sched_peek(&air_sched->active_txqs))
+ airtime_catchup_v_t(air_sched, air_info->v_t_cur, now);
}
+ if (air_info->v_t_cur > air_sched->v_t)
+ airtime_catchup_v_t(air_sched, air_info->v_t_cur, now);
+
air_sched->schedule_pos = node;
air_sched->last_schedule_activity = now;
- ret = &txqi->txq;
@ -667,7 +672,7 @@ Signed-off-by: Felix Fietkau <nbd@nbd.name>
struct ieee80211_local *local = hw_to_local(hw);
struct txq_info *txqi = to_txq_info(txq);
struct airtime_sched_info *air_sched;
@@ -3902,41 +3927,7 @@ void ieee80211_resort_txq(struct ieee802
@@ -3901,41 +3933,7 @@ void ieee80211_resort_txq(struct ieee802
air_sched = &local->airtime[txq->ac];
lockdep_assert_held(&air_sched->lock);
@ -710,7 +715,7 @@ Signed-off-by: Felix Fietkau <nbd@nbd.name>
}
void ieee80211_update_airtime_weight(struct ieee80211_local *local,
@@ -3985,7 +3976,7 @@ void ieee80211_schedule_txq(struct ieee8
@@ -3984,7 +3982,7 @@ void ieee80211_schedule_txq(struct ieee8
was_active = airtime_is_active(air_info, now);
airtime_set_active(air_sched, air_info, now);
@ -719,7 +724,7 @@ Signed-off-by: Felix Fietkau <nbd@nbd.name>
goto out;
/* If the station has been inactive for a while, catch up its v_t so it
@@ -3997,7 +3988,7 @@ void ieee80211_schedule_txq(struct ieee8
@@ -3996,7 +3994,7 @@ void ieee80211_schedule_txq(struct ieee8
air_info->v_t = air_sched->v_t;
ieee80211_update_airtime_weight(local, air_sched, now, !was_active);
@ -728,28 +733,35 @@ Signed-off-by: Felix Fietkau <nbd@nbd.name>
out:
spin_unlock_bh(&air_sched->lock);
@@ -4023,19 +4014,10 @@ static void __ieee80211_unschedule_txq(s
ieee80211_update_airtime_weight(local, air_sched, 0, true);
}
@@ -4017,24 +4015,14 @@ static void __ieee80211_unschedule_txq(s
lockdep_assert_held(&air_sched->lock);
+ airtime_sched_delete(&air_sched->active_txqs, &air_info->schedule_order);
if (purge) {
list_del_init(&air_info->list);
ieee80211_update_airtime_weight(local, air_sched, 0, true);
- }
-
- if (RB_EMPTY_NODE(&txqi->schedule_order))
- return;
-
- if (air_sched->schedule_pos == &txqi->schedule_order)
- air_sched->schedule_pos = rb_prev(&txqi->schedule_order);
-
+ airtime_sched_delete(&air_sched->active_txqs, &air_info->schedule_order);
if (!purge)
- if (!purge)
+ } else {
airtime_set_active(air_sched, air_info,
ktime_get_coarse_boottime_ns());
-
- rb_erase_cached(&txqi->schedule_order,
- &air_sched->active_txqs);
- RB_CLEAR_NODE(&txqi->schedule_order);
+ }
}
void ieee80211_unschedule_txq(struct ieee80211_hw *hw,
@@ -4055,14 +4037,24 @@ void ieee80211_return_txq(struct ieee802
@@ -4054,14 +4042,22 @@ void ieee80211_return_txq(struct ieee802
{
struct ieee80211_local *local = hw_to_local(hw);
struct txq_info *txqi = to_txq_info(txq);
@ -768,10 +780,8 @@ Signed-off-by: Felix Fietkau <nbd@nbd.name>
- spin_unlock_bh(&local->airtime[txq->ac].lock);
+ spin_lock_bh(&air_sched->lock);
+ if (!ieee80211_txq_airtime_check(hw, &txqi->txq))
+ airtime_sched_delete(&air_sched->active_txqs,
+ &air_info->schedule_order);
+ else if (txq_has_queue(txq) || force)
+ if (force || (txq_has_queue(txq) &&
+ ieee80211_txq_airtime_check(hw, &txqi->txq)))
+ __ieee80211_insert_txq(local, air_sched, txqi);
+ else
+ __ieee80211_unschedule_txq(hw, txq, false);
@ -779,25 +789,7 @@ Signed-off-by: Felix Fietkau <nbd@nbd.name>
}
EXPORT_SYMBOL(ieee80211_return_txq);
@@ -4101,46 +4093,48 @@ EXPORT_SYMBOL(ieee80211_txq_airtime_chec
bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
struct ieee80211_txq *txq)
{
- struct txq_info *first_txqi = NULL, *txqi = to_txq_info(txq);
+ struct txq_info *txqi = to_txq_info(txq);
struct ieee80211_local *local = hw_to_local(hw);
struct airtime_sched_info *air_sched;
+ struct airtime_sched_node *node = NULL;
struct airtime_info *air_info;
- struct rb_node *node = NULL;
bool ret = false;
+ u32 aql_slack;
u64 now;
-
if (!ieee80211_txq_airtime_check(hw, txq))
return false;
@@ -4113,9 +4109,6 @@ bool ieee80211_txq_may_transmit(struct i
air_sched = &local->airtime[txq->ac];
spin_lock_bh(&air_sched->lock);
@ -806,33 +798,8 @@ Signed-off-by: Felix Fietkau <nbd@nbd.name>
-
now = ktime_get_coarse_boottime_ns();
/* Like in ieee80211_next_txq(), make sure the first station in the
* scheduling order is eligible for transmission to avoid starvation.
*/
- node = rb_first_cached(&air_sched->active_txqs);
+ node = airtime_sched_peek(&air_sched->active_txqs);
if (node) {
- first_txqi = container_of(node, struct txq_info,
- schedule_order);
- air_info = to_airtime_info(&first_txqi->txq);
+ air_info = container_of(node, struct airtime_info,
+ schedule_order);
if (air_sched->v_t < air_info->v_t)
airtime_catchup_v_t(air_sched, air_info->v_t, now);
}
air_info = to_airtime_info(&txqi->txq);
- if (air_info->v_t <= air_sched->v_t) {
+ aql_slack = air_info->aql_limit_low;
+ aql_slack *= air_info->weight_reciprocal;
+ aql_slack >>= IEEE80211_RECIPROCAL_SHIFT_STA - IEEE80211_WEIGHT_SHIFT;
+ /*
+ * add extra slack of aql_limit_low in order to avoid queue
+ * starvation when bypassing normal scheduling order
+ */
+ if (air_info->v_t <= air_sched->v_t + aql_slack) {
air_sched->last_schedule_activity = now;
@@ -4124,7 +4117,6 @@ bool ieee80211_txq_may_transmit(struct i
ret = true;
}
@ -840,7 +807,7 @@ Signed-off-by: Felix Fietkau <nbd@nbd.name>
spin_unlock_bh(&air_sched->lock);
return ret;
}
@@ -4151,9 +4145,7 @@ void ieee80211_txq_schedule_start(struct
@@ -4135,9 +4127,7 @@ void ieee80211_txq_schedule_start(struct
struct ieee80211_local *local = hw_to_local(hw);
struct airtime_sched_info *air_sched = &local->airtime[ac];

View File

@ -26,7 +26,7 @@ Signed-off-by: Johannes Berg <johannes.berg@intel.com>
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2418,6 +2418,9 @@ struct ieee80211_txq {
@@ -2422,6 +2422,9 @@ struct ieee80211_txq {
* usage and 802.11 frames with %RX_FLAG_ONLY_MONITOR set for monitor to
* the stack.
*
@ -36,7 +36,7 @@ Signed-off-by: Johannes Berg <johannes.berg@intel.com>
* @NUM_IEEE80211_HW_FLAGS: number of hardware flags, used for sizing arrays
*/
enum ieee80211_hw_flags {
@@ -2473,6 +2476,7 @@ enum ieee80211_hw_flags {
@@ -2477,6 +2480,7 @@ enum ieee80211_hw_flags {
IEEE80211_HW_SUPPORTS_TX_ENCAP_OFFLOAD,
IEEE80211_HW_SUPPORTS_RX_DECAP_OFFLOAD,
IEEE80211_HW_SUPPORTS_CONC_MON_RX_DECAP,

View File

@ -18,7 +18,7 @@
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1566,6 +1566,7 @@ enum ieee80211_smps_mode {
@@ -1570,6 +1570,7 @@ enum ieee80211_smps_mode {
*
* @power_level: requested transmit power (in dBm), backward compatibility
* value only that is set to the minimum of all interfaces
@ -26,7 +26,7 @@
*
* @chandef: the channel definition to tune to
* @radar_enabled: whether radar detection is enabled
@@ -1586,6 +1587,7 @@ enum ieee80211_smps_mode {
@@ -1590,6 +1591,7 @@ enum ieee80211_smps_mode {
struct ieee80211_conf {
u32 flags;
int power_level, dynamic_ps_timeout;
@ -57,7 +57,7 @@
__NL80211_ATTR_AFTER_LAST,
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2840,6 +2840,19 @@ static int ieee80211_get_tx_power(struct
@@ -2843,6 +2843,19 @@ static int ieee80211_get_tx_power(struct
return 0;
}
@ -77,7 +77,7 @@
static void ieee80211_rfkill_poll(struct wiphy *wiphy)
{
struct ieee80211_local *local = wiphy_priv(wiphy);
@@ -4544,6 +4557,7 @@ const struct cfg80211_ops mac80211_confi
@@ -4547,6 +4560,7 @@ const struct cfg80211_ops mac80211_confi
.set_wiphy_params = ieee80211_set_wiphy_params,
.set_tx_power = ieee80211_set_tx_power,
.get_tx_power = ieee80211_get_tx_power,