* [PATCH 1/4] wifi: mac80211: factor out part of ieee80211_calc_expected_tx_airtime
@ 2026-03-23 10:19 Felix Fietkau
2026-03-23 10:19 ` [PATCH 2/4] wifi: mac80211: estimate expected throughput if not provided by driver/rc Felix Fietkau
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Felix Fietkau @ 2026-03-23 10:19 UTC (permalink / raw)
To: linux-wireless; +Cc: johannes
Create ieee80211_rate_expected_tx_airtime helper function, which returns
the expected tx airtime for a given rate and packet length in units of
1024 usec, for more accuracy.
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
net/mac80211/airtime.c | 87 ++++++++++++++++++++++----------------
net/mac80211/ieee80211_i.h | 5 +++
2 files changed, 56 insertions(+), 36 deletions(-)
diff --git a/net/mac80211/airtime.c b/net/mac80211/airtime.c
index c61df637232a..0c54cdbd753c 100644
--- a/net/mac80211/airtime.c
+++ b/net/mac80211/airtime.c
@@ -685,7 +685,7 @@ static int ieee80211_fill_rx_status(struct ieee80211_rx_status *stat,
if (ieee80211_fill_rate_info(hw, stat, band, ri))
return 0;
- if (!ieee80211_rate_valid(rate))
+ if (!rate || !ieee80211_rate_valid(rate))
return -1;
if (rate->flags & IEEE80211_TX_RC_160_MHZ_WIDTH)
@@ -753,6 +753,53 @@ u32 ieee80211_calc_tx_airtime(struct ieee80211_hw *hw,
}
EXPORT_SYMBOL_GPL(ieee80211_calc_tx_airtime);
+u32 ieee80211_rate_expected_tx_airtime(struct ieee80211_hw *hw,
+ struct ieee80211_tx_rate *tx_rate,
+ struct rate_info *ri,
+ enum nl80211_band band,
+ bool ampdu, int len)
+{
+ struct ieee80211_rx_status stat;
+ u32 duration, overhead;
+ u8 agg_shift;
+
+ if (ieee80211_fill_rx_status(&stat, hw, tx_rate, ri, band, len))
+ return 0;
+
+ if (stat.encoding == RX_ENC_LEGACY || !ampdu)
+ return ieee80211_calc_rx_airtime(hw, &stat, len) * 1024;
+
+ duration = ieee80211_get_rate_duration(hw, &stat, &overhead);
+
+ /*
+ * Assume that HT/VHT transmission on any AC except VO will
+ * use aggregation. Since we don't have reliable reporting
+ * of aggregation length, assume an average size based on the
+ * tx rate.
+ * This will not be very accurate, but much better than simply
+ * assuming un-aggregated tx in all cases.
+ */
+ if (duration > 400 * 1024) /* <= VHT20 MCS2 1S */
+ agg_shift = 1;
+ else if (duration > 250 * 1024) /* <= VHT20 MCS3 1S or MCS1 2S */
+ agg_shift = 2;
+ else if (duration > 150 * 1024) /* <= VHT20 MCS5 1S or MCS2 2S */
+ agg_shift = 3;
+ else if (duration > 70 * 1024) /* <= VHT20 MCS5 2S */
+ agg_shift = 4;
+ else if (stat.encoding != RX_ENC_HE ||
+ duration > 20 * 1024) /* <= HE40 MCS6 2S */
+ agg_shift = 5;
+ else
+ agg_shift = 6;
+
+ duration *= len;
+ duration /= AVG_PKT_SIZE;
+ duration += (overhead * 1024 >> agg_shift);
+
+ return duration;
+}
+
u32 ieee80211_calc_expected_tx_airtime(struct ieee80211_hw *hw,
struct ieee80211_vif *vif,
struct ieee80211_sta *pubsta,
@@ -775,45 +822,13 @@ u32 ieee80211_calc_expected_tx_airtime(struct ieee80211_hw *hw,
if (pubsta) {
struct sta_info *sta = container_of(pubsta, struct sta_info,
sta);
- struct ieee80211_rx_status stat;
struct ieee80211_tx_rate *tx_rate = &sta->deflink.tx_stats.last_rate;
struct rate_info *ri = &sta->deflink.tx_stats.last_rate_info;
- u32 duration, overhead;
- u8 agg_shift;
+ u32 duration;
- if (ieee80211_fill_rx_status(&stat, hw, tx_rate, ri, band, len))
- return 0;
-
- if (stat.encoding == RX_ENC_LEGACY || !ampdu)
- return ieee80211_calc_rx_airtime(hw, &stat, len);
-
- duration = ieee80211_get_rate_duration(hw, &stat, &overhead);
- /*
- * Assume that HT/VHT transmission on any AC except VO will
- * use aggregation. Since we don't have reliable reporting
- * of aggregation length, assume an average size based on the
- * tx rate.
- * This will not be very accurate, but much better than simply
- * assuming un-aggregated tx in all cases.
- */
- if (duration > 400 * 1024) /* <= VHT20 MCS2 1S */
- agg_shift = 1;
- else if (duration > 250 * 1024) /* <= VHT20 MCS3 1S or MCS1 2S */
- agg_shift = 2;
- else if (duration > 150 * 1024) /* <= VHT20 MCS5 1S or MCS2 2S */
- agg_shift = 3;
- else if (duration > 70 * 1024) /* <= VHT20 MCS5 2S */
- agg_shift = 4;
- else if (stat.encoding != RX_ENC_HE ||
- duration > 20 * 1024) /* <= HE40 MCS6 2S */
- agg_shift = 5;
- else
- agg_shift = 6;
-
- duration *= len;
- duration /= AVG_PKT_SIZE;
+ duration = ieee80211_rate_expected_tx_airtime(hw, tx_rate, ri,
+ band, true, len);
duration /= 1024;
- duration += (overhead >> agg_shift);
return max_t(u32, duration, 4);
}
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index d71e0c6d2165..b0dc93399e95 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -2834,6 +2834,11 @@ u8 *ieee80211_get_bssid(struct ieee80211_hdr *hdr, size_t len,
extern const struct ethtool_ops ieee80211_ethtool_ops;
+u32 ieee80211_rate_expected_tx_airtime(struct ieee80211_hw *hw,
+ struct ieee80211_tx_rate *tx_rate,
+ struct rate_info *ri,
+ enum nl80211_band band,
+ bool ampdu, int len);
u32 ieee80211_calc_expected_tx_airtime(struct ieee80211_hw *hw,
struct ieee80211_vif *vif,
struct ieee80211_sta *pubsta,
--
2.51.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/4] wifi: mac80211: estimate expected throughput if not provided by driver/rc
2026-03-23 10:19 [PATCH 1/4] wifi: mac80211: factor out part of ieee80211_calc_expected_tx_airtime Felix Fietkau
@ 2026-03-23 10:19 ` Felix Fietkau
2026-03-23 10:35 ` Johannes Berg
2026-03-23 10:19 ` [PATCH 3/4] wifi: mac80211: add AQL support for broadcast packets Felix Fietkau
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Felix Fietkau @ 2026-03-23 10:19 UTC (permalink / raw)
To: linux-wireless; +Cc: johannes
Estimate the tx throughput based on the expected per-packet tx time.
This is useful for mesh implementations that rely on expected throughput,
e.g. 802.11s or batman-adv.
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
net/mac80211/sta_info.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 4259e9c13ed7..912f00d905b8 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -2978,6 +2978,27 @@ static void sta_set_link_sinfo(struct sta_info *sta,
}
}
+static u32 sta_estimate_expected_throughput(struct sta_info *sta,
+ struct station_info *sinfo)
+{
+ struct ieee80211_sub_if_data *sdata = sta->sdata;
+ struct ieee80211_local *local = sdata->local;
+ struct rate_info *ri = &sinfo->txrate;
+ struct ieee80211_hw *hw = &local->hw;
+ struct ieee80211_chanctx_conf *conf;
+ u32 duration;
+ u8 band = 0;
+
+ conf = rcu_dereference(sdata->vif.bss_conf.chanctx_conf);
+ if (conf)
+ band = conf->def.chan->band;
+
+ duration = ieee80211_rate_expected_tx_airtime(hw, NULL, ri, band, true, 1024);
+ duration += duration >> 4; /* add assumed packet error rate of ~6% */
+
+ return ((1024 * USEC_PER_SEC) / duration) * 8;
+}
+
void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
bool tidstats)
{
@@ -3202,6 +3223,8 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
sinfo->sta_flags.set |= BIT(NL80211_STA_FLAG_TDLS_PEER);
thr = sta_get_expected_throughput(sta);
+ if (!thr && (sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_BITRATE)))
+ thr = sta_estimate_expected_throughput(sta, sinfo);
if (thr != 0) {
sinfo->filled |= BIT_ULL(NL80211_STA_INFO_EXPECTED_THROUGHPUT);
--
2.51.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/4] wifi: mac80211: add AQL support for broadcast packets
2026-03-23 10:19 [PATCH 1/4] wifi: mac80211: factor out part of ieee80211_calc_expected_tx_airtime Felix Fietkau
2026-03-23 10:19 ` [PATCH 2/4] wifi: mac80211: estimate expected throughput if not provided by driver/rc Felix Fietkau
@ 2026-03-23 10:19 ` Felix Fietkau
2026-03-23 10:38 ` Johannes Berg
2026-03-23 10:19 ` [PATCH 4/4] wifi: mac80211: add ieee80211_txq_aql_pending() Felix Fietkau
2026-03-23 16:00 ` [PATCH 1/4] wifi: mac80211: factor out part of ieee80211_calc_expected_tx_airtime Pablo MARTIN-GOMEZ
3 siblings, 1 reply; 14+ messages in thread
From: Felix Fietkau @ 2026-03-23 10:19 UTC (permalink / raw)
To: linux-wireless; +Cc: johannes
Excessive broadcast traffic with little competing unicast traffic can easily
flood hardware queues, leading to throughput issues. Additionally, filling
the hardware queues with too many packets breaks FQ for broadcast data.
Fix this by enabling AQL for broadcast packets.
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
include/net/cfg80211.h | 1 +
include/net/mac80211.h | 2 +-
net/mac80211/debugfs.c | 13 ++++++++--
net/mac80211/ieee80211_i.h | 2 ++
net/mac80211/main.c | 1 +
net/mac80211/sta_info.c | 17 ++++++++++++-
net/mac80211/sta_info.h | 3 ++-
net/mac80211/status.c | 5 ++--
net/mac80211/tx.c | 52 ++++++++++++++++++++------------------
9 files changed, 65 insertions(+), 31 deletions(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 8cd870ece351..2607b800ada5 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -3656,6 +3656,7 @@ enum wiphy_params_flags {
/* The per TXQ device queue limit in airtime */
#define IEEE80211_DEFAULT_AQL_TXQ_LIMIT_L 5000
#define IEEE80211_DEFAULT_AQL_TXQ_LIMIT_H 12000
+#define IEEE80211_DEFAULT_AQL_TXQ_LIMIT_BC 50000
/* The per interface airtime threshold to switch to lower queue limit */
#define IEEE80211_AQL_THRESHOLD 24000
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 9cc482191ab9..310546d4fca6 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1252,8 +1252,8 @@ struct ieee80211_tx_info {
status_data_idr:1,
status_data:13,
hw_queue:4,
+ tx_time_mc:1,
tx_time_est:10;
- /* 1 free bit */
union {
struct {
diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index e8d0a8b71d59..a97d146f1445 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -210,11 +210,13 @@ static ssize_t aql_pending_read(struct file *file,
"VI %u us\n"
"BE %u us\n"
"BK %u us\n"
+ "BC/MC %u us\n"
"total %u us\n",
atomic_read(&local->aql_ac_pending_airtime[IEEE80211_AC_VO]),
atomic_read(&local->aql_ac_pending_airtime[IEEE80211_AC_VI]),
atomic_read(&local->aql_ac_pending_airtime[IEEE80211_AC_BE]),
atomic_read(&local->aql_ac_pending_airtime[IEEE80211_AC_BK]),
+ atomic_read(&local->aql_bc_pending_airtime),
atomic_read(&local->aql_total_pending_airtime));
return simple_read_from_buffer(user_buf, count, ppos,
buf, len);
@@ -239,7 +241,8 @@ static ssize_t aql_txq_limit_read(struct file *file,
"VO %u %u\n"
"VI %u %u\n"
"BE %u %u\n"
- "BK %u %u\n",
+ "BK %u %u\n"
+ "BC/MC %u\n",
local->aql_txq_limit_low[IEEE80211_AC_VO],
local->aql_txq_limit_high[IEEE80211_AC_VO],
local->aql_txq_limit_low[IEEE80211_AC_VI],
@@ -247,7 +250,8 @@ static ssize_t aql_txq_limit_read(struct file *file,
local->aql_txq_limit_low[IEEE80211_AC_BE],
local->aql_txq_limit_high[IEEE80211_AC_BE],
local->aql_txq_limit_low[IEEE80211_AC_BK],
- local->aql_txq_limit_high[IEEE80211_AC_BK]);
+ local->aql_txq_limit_high[IEEE80211_AC_BK],
+ local->aql_txq_limit_bc);
return simple_read_from_buffer(user_buf, count, ppos,
buf, len);
}
@@ -273,6 +277,11 @@ static ssize_t aql_txq_limit_write(struct file *file,
else
buf[count] = '\0';
+ if (sscanf(buf, "mcast %u", &q_limit_low) == 1) {
+ local->aql_txq_limit_bc = q_limit_low;
+ return count;
+ }
+
if (sscanf(buf, "%u %u %u", &ac, &q_limit_low, &q_limit_high) != 3)
return -EINVAL;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index b0dc93399e95..7ce39d19274f 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1427,10 +1427,12 @@ struct ieee80211_local {
spinlock_t handle_wake_tx_queue_lock;
u16 airtime_flags;
+ u32 aql_txq_limit_bc;
u32 aql_txq_limit_low[IEEE80211_NUM_ACS];
u32 aql_txq_limit_high[IEEE80211_NUM_ACS];
u32 aql_threshold;
atomic_t aql_total_pending_airtime;
+ atomic_t aql_bc_pending_airtime;
atomic_t aql_ac_pending_airtime[IEEE80211_NUM_ACS];
const struct ieee80211_ops *ops;
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index d1bb6353908d..a05e3f6cb43c 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -984,6 +984,7 @@ 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);
+ local->aql_txq_limit_bc = IEEE80211_DEFAULT_AQL_TXQ_LIMIT_BC;
for (i = 0; i < IEEE80211_NUM_ACS; i++) {
INIT_LIST_HEAD(&local->active_txqs[i]);
spin_lock_init(&local->active_txq_lock[i]);
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 912f00d905b8..12696c242537 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -2443,13 +2443,28 @@ EXPORT_SYMBOL(ieee80211_sta_recalc_aggregates);
void ieee80211_sta_update_pending_airtime(struct ieee80211_local *local,
struct sta_info *sta, u8 ac,
- u16 tx_airtime, bool tx_completed)
+ u16 tx_airtime, bool tx_completed,
+ bool mcast)
{
int tx_pending;
if (!wiphy_ext_feature_isset(local->hw.wiphy, NL80211_EXT_FEATURE_AQL))
return;
+ if (mcast) {
+ if (!tx_completed) {
+ atomic_add(tx_airtime, &local->aql_bc_pending_airtime);
+ return;
+ }
+
+ tx_pending = atomic_sub_return(tx_airtime,
+ &local->aql_bc_pending_airtime);
+ if (tx_pending < 0)
+ atomic_cmpxchg(&local->aql_bc_pending_airtime,
+ tx_pending, 0);
+ return;
+ }
+
if (!tx_completed) {
if (sta)
atomic_add(tx_airtime,
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 58ccbea7f6f6..eea7eff35463 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -147,7 +147,8 @@ struct airtime_info {
void ieee80211_sta_update_pending_airtime(struct ieee80211_local *local,
struct sta_info *sta, u8 ac,
- u16 tx_airtime, bool tx_completed);
+ u16 tx_airtime, bool tx_completed,
+ bool mcast);
struct sta_info;
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 4b38aa0e902a..ccc37c4d843d 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -751,7 +751,7 @@ static void ieee80211_report_used_skb(struct ieee80211_local *local,
ieee80211_sta_update_pending_airtime(local, sta,
skb_get_queue_mapping(skb),
tx_time_est,
- true);
+ true, info->tx_time_mc);
rcu_read_unlock();
}
@@ -1160,10 +1160,11 @@ void ieee80211_tx_status_ext(struct ieee80211_hw *hw,
/* Do this here to avoid the expensive lookup of the sta
* in ieee80211_report_used_skb().
*/
+ bool mcast = IEEE80211_SKB_CB(skb)->tx_time_mc;
ieee80211_sta_update_pending_airtime(local, sta,
skb_get_queue_mapping(skb),
tx_time_est,
- true);
+ true, mcast);
ieee80211_info_set_tx_time_est(IEEE80211_SKB_CB(skb), 0);
}
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 3844c7fbb8a8..04a3ea9beae5 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3987,20 +3987,20 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
encap_out:
info->control.vif = vif;
- if (tx.sta &&
- wiphy_ext_feature_isset(local->hw.wiphy, NL80211_EXT_FEATURE_AQL)) {
- bool ampdu = txq->ac != IEEE80211_AC_VO;
+ if (wiphy_ext_feature_isset(local->hw.wiphy, NL80211_EXT_FEATURE_AQL)) {
+ bool ampdu = txq->sta && txq->ac != IEEE80211_AC_VO;
u32 airtime;
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)
+ return skb;
+
+ airtime = ieee80211_info_set_tx_time_est(info, airtime);
+ info->tx_time_mc = !tx.sta;
+ ieee80211_sta_update_pending_airtime(local, tx.sta, txq->ac,
+ airtime, false,
+ info->tx_time_mc);
}
return skb;
@@ -4052,6 +4052,7 @@ struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac)
struct ieee80211_txq *ret = NULL;
struct txq_info *txqi = NULL, *head = NULL;
bool found_eligible_txq = false;
+ bool aql_check;
spin_lock_bh(&local->active_txq_lock[ac]);
@@ -4075,26 +4076,28 @@ struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac)
if (!head)
head = txqi;
+ aql_check = ieee80211_txq_airtime_check(hw, &txqi->txq);
+ if (aql_check)
+ found_eligible_txq = true;
+
if (txqi->txq.sta) {
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 = ieee80211_sta_deficit(sta, txqi->txq.ac);
- if (aql_check)
- found_eligible_txq = true;
-
- if (deficit < 0)
+ if (ieee80211_sta_deficit(sta, txqi->txq.ac) < 0) {
sta->airtime[txqi->txq.ac].deficit +=
sta->airtime_weight;
- if (deficit < 0 || !aql_check) {
- list_move_tail(&txqi->schedule_order,
- &local->active_txqs[txqi->txq.ac]);
- goto begin;
+ aql_check = false;
}
}
+ if (!aql_check) {
+ list_move_tail(&txqi->schedule_order,
+ &local->active_txqs[txqi->txq.ac]);
+ goto begin;
+ }
+
if (txqi->schedule_round == local->schedule_round[ac])
goto out;
@@ -4161,7 +4164,8 @@ bool ieee80211_txq_airtime_check(struct ieee80211_hw *hw,
return true;
if (!txq->sta)
- return true;
+ return atomic_read(&local->aql_bc_pending_airtime) <
+ local->aql_txq_limit_bc;
if (unlikely(txq->tid == IEEE80211_NUM_TIDS))
return true;
@@ -4210,15 +4214,15 @@ bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
spin_lock_bh(&local->active_txq_lock[ac]);
- if (!txqi->txq.sta)
- goto out;
-
if (list_empty(&txqi->schedule_order))
goto out;
if (!ieee80211_txq_schedule_airtime_check(local, ac))
goto out;
+ if (!txqi->txq.sta)
+ goto out;
+
list_for_each_entry_safe(iter, tmp, &local->active_txqs[ac],
schedule_order) {
if (iter == txqi)
--
2.51.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/4] wifi: mac80211: add ieee80211_txq_aql_pending()
2026-03-23 10:19 [PATCH 1/4] wifi: mac80211: factor out part of ieee80211_calc_expected_tx_airtime Felix Fietkau
2026-03-23 10:19 ` [PATCH 2/4] wifi: mac80211: estimate expected throughput if not provided by driver/rc Felix Fietkau
2026-03-23 10:19 ` [PATCH 3/4] wifi: mac80211: add AQL support for broadcast packets Felix Fietkau
@ 2026-03-23 10:19 ` Felix Fietkau
2026-03-23 10:39 ` Johannes Berg
2026-03-23 10:55 ` Johannes Berg
2026-03-23 16:00 ` [PATCH 1/4] wifi: mac80211: factor out part of ieee80211_calc_expected_tx_airtime Pablo MARTIN-GOMEZ
3 siblings, 2 replies; 14+ messages in thread
From: Felix Fietkau @ 2026-03-23 10:19 UTC (permalink / raw)
To: linux-wireless; +Cc: johannes
Add a function to allow drivers to query the pending AQL airtime
for a given txq, for both unicast and broadcast.
This will be used for mt76 to limit buffering in AP mode for power-save
stations.
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
include/net/mac80211.h | 11 +++++++++++
net/mac80211/tx.c | 18 ++++++++++++++++++
2 files changed, 29 insertions(+)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 310546d4fca6..f260017cc858 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -6710,6 +6710,17 @@ void ieee80211_sta_register_airtime(struct ieee80211_sta *pubsta, u8 tid,
bool
ieee80211_txq_airtime_check(struct ieee80211_hw *hw, struct ieee80211_txq *txq);
+/**
+ * ieee80211_txq_aql_pending - get pending AQL airtime for a txq
+ *
+ * @hw: pointer obtained from ieee80211_alloc_hw()
+ * @txq: pointer obtained from station or virtual interface
+ *
+ * Return: pending airtime (in usec) for the given txq.
+ */
+u32 ieee80211_txq_aql_pending(struct ieee80211_hw *hw,
+ struct ieee80211_txq *txq);
+
/**
* ieee80211_iter_keys - iterate keys programmed into the device
* @hw: pointer obtained from ieee80211_alloc_hw()
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 04a3ea9beae5..07a65e14d637 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -4185,6 +4185,24 @@ bool ieee80211_txq_airtime_check(struct ieee80211_hw *hw,
}
EXPORT_SYMBOL(ieee80211_txq_airtime_check);
+u32 ieee80211_txq_aql_pending(struct ieee80211_hw *hw,
+ struct ieee80211_txq *txq)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+ struct sta_info *sta;
+
+ if (!txq->sta)
+ return atomic_read(&local->aql_bc_pending_airtime);
+
+ sta = container_of(txq->sta, struct sta_info, sta);
+
+ if (unlikely(txq->tid == IEEE80211_NUM_TIDS))
+ return 0;
+
+ return atomic_read(&sta->airtime[txq->ac].aql_tx_pending);
+}
+EXPORT_SYMBOL(ieee80211_txq_aql_pending);
+
static bool
ieee80211_txq_schedule_airtime_check(struct ieee80211_local *local, u8 ac)
{
--
2.51.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] wifi: mac80211: estimate expected throughput if not provided by driver/rc
2026-03-23 10:19 ` [PATCH 2/4] wifi: mac80211: estimate expected throughput if not provided by driver/rc Felix Fietkau
@ 2026-03-23 10:35 ` Johannes Berg
0 siblings, 0 replies; 14+ messages in thread
From: Johannes Berg @ 2026-03-23 10:35 UTC (permalink / raw)
To: Felix Fietkau, linux-wireless
On Mon, 2026-03-23 at 10:19 +0000, Felix Fietkau wrote:
> Estimate the tx throughput based on the expected per-packet tx time.
> This is useful for mesh implementations that rely on expected throughput,
> e.g. 802.11s or batman-adv.
>
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> ---
> net/mac80211/sta_info.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
> index 4259e9c13ed7..912f00d905b8 100644
> --- a/net/mac80211/sta_info.c
> +++ b/net/mac80211/sta_info.c
> @@ -2978,6 +2978,27 @@ static void sta_set_link_sinfo(struct sta_info *sta,
> }
> }
>
> +static u32 sta_estimate_expected_throughput(struct sta_info *sta,
> + struct station_info *sinfo)
> +{
> + struct ieee80211_sub_if_data *sdata = sta->sdata;
> + struct ieee80211_local *local = sdata->local;
> + struct rate_info *ri = &sinfo->txrate;
> + struct ieee80211_hw *hw = &local->hw;
> + struct ieee80211_chanctx_conf *conf;
> + u32 duration;
> + u8 band = 0;
> +
> + conf = rcu_dereference(sdata->vif.bss_conf.chanctx_conf);
> + if (conf)
> + band = conf->def.chan->band;
Double indentation, but 'u8 band = 0' is also really strange. Enum? And
should have a justification for assuming =0?
> + duration = ieee80211_rate_expected_tx_airtime(hw, NULL, ri, band, true, 1024);
> + duration += duration >> 4; /* add assumed packet error rate of ~6% */
> +
> + return ((1024 * USEC_PER_SEC) / duration) * 8;
this divides by zero.
> @@ -3202,6 +3223,8 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
> sinfo->sta_flags.set |= BIT(NL80211_STA_FLAG_TDLS_PEER);
>
> thr = sta_get_expected_throughput(sta);
> + if (!thr && (sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_BITRATE)))
> + thr = sta_estimate_expected_throughput(sta, sinfo);
Wrong indentation too ...
johannes
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] wifi: mac80211: add AQL support for broadcast packets
2026-03-23 10:19 ` [PATCH 3/4] wifi: mac80211: add AQL support for broadcast packets Felix Fietkau
@ 2026-03-23 10:38 ` Johannes Berg
0 siblings, 0 replies; 14+ messages in thread
From: Johannes Berg @ 2026-03-23 10:38 UTC (permalink / raw)
To: Felix Fietkau, linux-wireless
On Mon, 2026-03-23 at 10:19 +0000, Felix Fietkau wrote:
>
> +++ b/include/net/mac80211.h
> @@ -1252,8 +1252,8 @@ struct ieee80211_tx_info {
> status_data_idr:1,
> status_data:13,
> hw_queue:4,
> + tx_time_mc:1,
> tx_time_est:10;
> + "BC/MC %u us\n"
> + atomic_read(&local->aql_bc_pending_airtime),
> + local->aql_txq_limit_bc);
> return simple_read_from_buffer(user_buf, count, ppos,
> buf, len);
> }
> @@ -273,6 +277,11 @@ static ssize_t aql_txq_limit_write(struct file *file,
> else
> buf[count] = '\0';
>
> + if (sscanf(buf, "mcast %u", &q_limit_low) == 1) {
> + u32 aql_txq_limit_bc;
...
I think you should be consistent with multicast/broadcast. It really
isn't handling purely broadcast anyway.
johannes
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] wifi: mac80211: add ieee80211_txq_aql_pending()
2026-03-23 10:19 ` [PATCH 4/4] wifi: mac80211: add ieee80211_txq_aql_pending() Felix Fietkau
@ 2026-03-23 10:39 ` Johannes Berg
2026-03-23 10:43 ` Felix Fietkau
2026-03-23 10:55 ` Johannes Berg
1 sibling, 1 reply; 14+ messages in thread
From: Johannes Berg @ 2026-03-23 10:39 UTC (permalink / raw)
To: Felix Fietkau, linux-wireless
On Mon, 2026-03-23 at 10:19 +0000, Felix Fietkau wrote:
> Add a function to allow drivers to query the pending AQL airtime
> for a given txq, for both unicast and broadcast.
> This will be used for mt76 to limit buffering in AP mode for power-save
> stations.
Can you elaborate? Seems strange to make buffering decisions in the HW
queues (presumably) based on what's already in SW queues?
johannes
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] wifi: mac80211: add ieee80211_txq_aql_pending()
2026-03-23 10:39 ` Johannes Berg
@ 2026-03-23 10:43 ` Felix Fietkau
0 siblings, 0 replies; 14+ messages in thread
From: Felix Fietkau @ 2026-03-23 10:43 UTC (permalink / raw)
To: Johannes Berg, linux-wireless
On 23.03.26 11:39, Johannes Berg wrote:
> On Mon, 2026-03-23 at 10:19 +0000, Felix Fietkau wrote:
>> Add a function to allow drivers to query the pending AQL airtime
>> for a given txq, for both unicast and broadcast.
>> This will be used for mt76 to limit buffering in AP mode for power-save
>> stations.
>
> Can you elaborate? Seems strange to make buffering decisions in the HW
> queues (presumably) based on what's already in SW queues?
AQL is tracking what's pushed to HW queues already, not SW queues.
On MT7615 and newer, firmware handles wakeup of PS stations internally
based on queued packets.
I want to make sure that the driver doesn't queue more packets for
sleeping stations than what's needed to wake them up.
- Felix
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] wifi: mac80211: add ieee80211_txq_aql_pending()
2026-03-23 10:19 ` [PATCH 4/4] wifi: mac80211: add ieee80211_txq_aql_pending() Felix Fietkau
2026-03-23 10:39 ` Johannes Berg
@ 2026-03-23 10:55 ` Johannes Berg
1 sibling, 0 replies; 14+ messages in thread
From: Johannes Berg @ 2026-03-23 10:55 UTC (permalink / raw)
To: Felix Fietkau, linux-wireless
On Mon, 2026-03-23 at 10:19 +0000, Felix Fietkau wrote:
>
> +u32 ieee80211_txq_aql_pending(struct ieee80211_hw *hw,
> + struct ieee80211_txq *txq)
> +{
> + struct ieee80211_local *local = hw_to_local(hw);
> + struct sta_info *sta;
> +
> + if (!txq->sta)
> + return atomic_read(&local->aql_bc_pending_airtime);
> +
> + sta = container_of(txq->sta, struct sta_info, sta);
> +
> + if (unlikely(txq->tid == IEEE80211_NUM_TIDS))
> + return 0;
I think better to put that check before the !txq->sta check. I don't
think it's there yet, but I think NAN is adding a per-interface mgmt
queue or so. Better to just cover all the cases here now.
johannes
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] wifi: mac80211: factor out part of ieee80211_calc_expected_tx_airtime
2026-03-23 10:19 [PATCH 1/4] wifi: mac80211: factor out part of ieee80211_calc_expected_tx_airtime Felix Fietkau
` (2 preceding siblings ...)
2026-03-23 10:19 ` [PATCH 4/4] wifi: mac80211: add ieee80211_txq_aql_pending() Felix Fietkau
@ 2026-03-23 16:00 ` Pablo MARTIN-GOMEZ
2026-03-25 3:58 ` Felix Fietkau
3 siblings, 1 reply; 14+ messages in thread
From: Pablo MARTIN-GOMEZ @ 2026-03-23 16:00 UTC (permalink / raw)
To: Felix Fietkau, linux-wireless; +Cc: johannes
Hello,
On 23/03/2026 11:19, Felix Fietkau wrote:
> Create ieee80211_rate_expected_tx_airtime helper function, which returns
> the expected tx airtime for a given rate and packet length in units of
> 1024 usec, for more accuracy.
>
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> ---
> net/mac80211/airtime.c | 87 ++++++++++++++++++++++----------------
> net/mac80211/ieee80211_i.h | 5 +++
> 2 files changed, 56 insertions(+), 36 deletions(-)
>
> diff --git a/net/mac80211/airtime.c b/net/mac80211/airtime.c
> index c61df637232a..0c54cdbd753c 100644
> --- a/net/mac80211/airtime.c
> +++ b/net/mac80211/airtime.c
> @@ -685,7 +685,7 @@ static int ieee80211_fill_rx_status(struct ieee80211_rx_status *stat,
> if (ieee80211_fill_rate_info(hw, stat, band, ri))
> return 0;
>
> - if (!ieee80211_rate_valid(rate))
> + if (!rate || !ieee80211_rate_valid(rate))
> return -1;
>
> if (rate->flags & IEEE80211_TX_RC_160_MHZ_WIDTH)
> @@ -753,6 +753,53 @@ u32 ieee80211_calc_tx_airtime(struct ieee80211_hw *hw,
> }
> EXPORT_SYMBOL_GPL(ieee80211_calc_tx_airtime);
>
> +u32 ieee80211_rate_expected_tx_airtime(struct ieee80211_hw *hw,
> + struct ieee80211_tx_rate *tx_rate,
> + struct rate_info *ri,
> + enum nl80211_band band,
> + bool ampdu, int len)
> +{
> + struct ieee80211_rx_status stat;
> + u32 duration, overhead;
> + u8 agg_shift;
> +
> + if (ieee80211_fill_rx_status(&stat, hw, tx_rate, ri, band, len))
> + return 0;
> +
> + if (stat.encoding == RX_ENC_LEGACY || !ampdu)
> + return ieee80211_calc_rx_airtime(hw, &stat, len) * 1024;
> +
> + duration = ieee80211_get_rate_duration(hw, &stat, &overhead);
> +
> + /*
> + * Assume that HT/VHT transmission on any AC except VO will
> + * use aggregation. Since we don't have reliable reporting
> + * of aggregation length, assume an average size based on the
> + * tx rate.
> + * This will not be very accurate, but much better than simply
> + * assuming un-aggregated tx in all cases.
> + */
> + if (duration > 400 * 1024) /* <= VHT20 MCS2 1S */
> + agg_shift = 1;
> + else if (duration > 250 * 1024) /* <= VHT20 MCS3 1S or MCS1 2S */
> + agg_shift = 2;
> + else if (duration > 150 * 1024) /* <= VHT20 MCS5 1S or MCS2 2S */
> + agg_shift = 3;
> + else if (duration > 70 * 1024) /* <= VHT20 MCS5 2S */
> + agg_shift = 4;
> + else if (stat.encoding != RX_ENC_HE ||
> + duration > 20 * 1024) /* <= HE40 MCS6 2S */
> + agg_shift = 5;
> + else
> + agg_shift = 6;
> +
> + duration *= len;
> + duration /= AVG_PKT_SIZE;
> + duration += (overhead * 1024 >> agg_shift);
I know this patch is just a refactoring, but I think this moved code is
bugged. If (and it's a big if) I understood correctly the chain of
macros and the comments, `ieee80211_get_rate_duration` return the
`duration` in 1024 µs of an average packet (which would imply
1f38b8c564b8 is wrong) and the (PHY) `overhead` in µs for a (average)
packet. So I believe the code should be:
```c
duration = ieee80211_get_rate_duration(hw, &stat, &overhead);
duration *= 1024; /* now duration is in µs */
/* the agg_shift calculation has to be fixed */
duration += (overhead >> agg_shift); /* for one packet, we "assign" a
fraction of the overhead */
duration *= len/AVG_PKT_SIZE; /* we multiply by the number of packets */
duration /= 1024; /* we go back to a duration in 1024 µs*/
return duration;
```
If this is correct, then `ieee80211_calc_rx_airtime` is also bugged
likewise.
> +
> + return duration;
> +}
> +
> u32 ieee80211_calc_expected_tx_airtime(struct ieee80211_hw *hw,
> struct ieee80211_vif *vif,
> struct ieee80211_sta *pubsta,
Pablo MG
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] wifi: mac80211: factor out part of ieee80211_calc_expected_tx_airtime
2026-03-23 16:00 ` [PATCH 1/4] wifi: mac80211: factor out part of ieee80211_calc_expected_tx_airtime Pablo MARTIN-GOMEZ
@ 2026-03-25 3:58 ` Felix Fietkau
2026-03-25 11:21 ` Pablo MARTIN-GOMEZ
0 siblings, 1 reply; 14+ messages in thread
From: Felix Fietkau @ 2026-03-25 3:58 UTC (permalink / raw)
To: Pablo MARTIN-GOMEZ, linux-wireless; +Cc: johannes
On 23.03.26 17:00, Pablo MARTIN-GOMEZ wrote:
> Hello,
>
> On 23/03/2026 11:19, Felix Fietkau wrote:
>> Create ieee80211_rate_expected_tx_airtime helper function, which returns
>> the expected tx airtime for a given rate and packet length in units of
>> 1024 usec, for more accuracy.
>>
>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>> ---
>> net/mac80211/airtime.c | 87 ++++++++++++++++++++++----------------
>> net/mac80211/ieee80211_i.h | 5 +++
>> 2 files changed, 56 insertions(+), 36 deletions(-)
>>
>> diff --git a/net/mac80211/airtime.c b/net/mac80211/airtime.c
>> index c61df637232a..0c54cdbd753c 100644
>> --- a/net/mac80211/airtime.c
>> +++ b/net/mac80211/airtime.c
>> @@ -685,7 +685,7 @@ static int ieee80211_fill_rx_status(struct ieee80211_rx_status *stat,
>> if (ieee80211_fill_rate_info(hw, stat, band, ri))
>> return 0;
>>
>> - if (!ieee80211_rate_valid(rate))
>> + if (!rate || !ieee80211_rate_valid(rate))
>> return -1;
>>
>> if (rate->flags & IEEE80211_TX_RC_160_MHZ_WIDTH)
>> @@ -753,6 +753,53 @@ u32 ieee80211_calc_tx_airtime(struct ieee80211_hw *hw,
>> }
>> EXPORT_SYMBOL_GPL(ieee80211_calc_tx_airtime);
>>
>> +u32 ieee80211_rate_expected_tx_airtime(struct ieee80211_hw *hw,
>> + struct ieee80211_tx_rate *tx_rate,
>> + struct rate_info *ri,
>> + enum nl80211_band band,
>> + bool ampdu, int len)
>> +{
>> + struct ieee80211_rx_status stat;
>> + u32 duration, overhead;
>> + u8 agg_shift;
>> +
>> + if (ieee80211_fill_rx_status(&stat, hw, tx_rate, ri, band, len))
>> + return 0;
>> +
>> + if (stat.encoding == RX_ENC_LEGACY || !ampdu)
>> + return ieee80211_calc_rx_airtime(hw, &stat, len) * 1024;
>> +
>> + duration = ieee80211_get_rate_duration(hw, &stat, &overhead);
>> +
>> + /*
>> + * Assume that HT/VHT transmission on any AC except VO will
>> + * use aggregation. Since we don't have reliable reporting
>> + * of aggregation length, assume an average size based on the
>> + * tx rate.
>> + * This will not be very accurate, but much better than simply
>> + * assuming un-aggregated tx in all cases.
>> + */
>> + if (duration > 400 * 1024) /* <= VHT20 MCS2 1S */
>> + agg_shift = 1;
>> + else if (duration > 250 * 1024) /* <= VHT20 MCS3 1S or MCS1 2S */
>> + agg_shift = 2;
>> + else if (duration > 150 * 1024) /* <= VHT20 MCS5 1S or MCS2 2S */
>> + agg_shift = 3;
>> + else if (duration > 70 * 1024) /* <= VHT20 MCS5 2S */
>> + agg_shift = 4;
>> + else if (stat.encoding != RX_ENC_HE ||
>> + duration > 20 * 1024) /* <= HE40 MCS6 2S */
>> + agg_shift = 5;
>> + else
>> + agg_shift = 6;
>> +
>> + duration *= len;
>> + duration /= AVG_PKT_SIZE;
>> + duration += (overhead * 1024 >> agg_shift);
> I know this patch is just a refactoring, but I think this moved code is
> bugged. If (and it's a big if) I understood correctly the chain of
> macros and the comments, `ieee80211_get_rate_duration` return the
> `duration` in 1024 µs of an average packet (which would imply
> 1f38b8c564b8 is wrong) and the (PHY) `overhead` in µs for a (average)
> packet. So I believe the code should be:
> ```c
> duration = ieee80211_get_rate_duration(hw, &stat, &overhead);
> duration *= 1024; /* now duration is in µs */
> /* the agg_shift calculation has to be fixed */
> duration += (overhead >> agg_shift); /* for one packet, we "assign" a
> fraction of the overhead */
> duration *= len/AVG_PKT_SIZE; /* we multiply by the number of packets */
> duration /= 1024; /* we go back to a duration in 1024 µs*/
>
> return duration;
> ```
The overhead (preamble, signal field, etc.) is a fixed per-frame PHY
cost that doesn't depend on how many data bytes are in the frame. In the
aggregated case, agg_shift amortizes that fixed cost across the
estimated number of subframes in the aggregate. So the correct order is:
scale the data duration to the actual packet size, then add the
amortized overhead once.
This is the same pattern used in ieee80211_calc_rx_airtime:
duration *= len;
duration /= AVG_PKT_SIZE;
duration /= 1024;
return duration + overhead;
Your proposed rewrite would multiply the overhead by len / AVG_PKT_SIZE,
making it proportional to packet size, which is incorrect, because a
512-byte frame and a 1500-byte frame have the same PHY preamble duration.
- Felix
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] wifi: mac80211: factor out part of ieee80211_calc_expected_tx_airtime
2026-03-25 3:58 ` Felix Fietkau
@ 2026-03-25 11:21 ` Pablo MARTIN-GOMEZ
2026-03-25 11:41 ` Felix Fietkau
0 siblings, 1 reply; 14+ messages in thread
From: Pablo MARTIN-GOMEZ @ 2026-03-25 11:21 UTC (permalink / raw)
To: Felix Fietkau, linux-wireless; +Cc: johannes
On 25/03/2026 04:58, Felix Fietkau wrote:
> On 23.03.26 17:00, Pablo MARTIN-GOMEZ wrote:
>> Hello,
>>
[...]
>> I know this patch is just a refactoring, but I think this moved code is
>> bugged. If (and it's a big if) I understood correctly the chain of
>> macros and the comments, `ieee80211_get_rate_duration` return the
>> `duration` in 1024 µs of an average packet (which would imply
>> 1f38b8c564b8 is wrong) and the (PHY) `overhead` in µs for a (average)
>> packet. So I believe the code should be:
>> ```c
>> duration = ieee80211_get_rate_duration(hw, &stat, &overhead);
>> duration *= 1024; /* now duration is in µs */
>> /* the agg_shift calculation has to be fixed */
>> duration += (overhead >> agg_shift); /* for one packet, we
>> "assign" a
>> fraction of the overhead */
>> duration *= len/AVG_PKT_SIZE; /* we multiply by the number of
>> packets */
>> duration /= 1024; /* we go back to a duration in 1024 µs*/
>>
>> return duration;
>> ```
>
> The overhead (preamble, signal field, etc.) is a fixed per-frame PHY
> cost that doesn't depend on how many data bytes are in the frame. In the
> aggregated case, agg_shift amortizes that fixed cost across the
> estimated number of subframes in the aggregate. So the correct order is:
> scale the data duration to the actual packet size, then add the
> amortized overhead once.
My bad, I didn't understand that `len` was the byte size of a MPDU.
So I was wrong on where I put the overhead, but (a priori) not on the
rest of the calculation *if* my understanding of the units is correct.
If 1f38b8c564b8 is correct and so `duration` is in ns and `overhead` is
in µs, then your code is correct, but the commit message is wrong
because `ieee80211_rate_expected_tx_airtime` is returning a value in ns.
My snippet fixed if `duration` is in 1024 µs:
```c
duration = ieee80211_get_rate_duration(hw, &stat, &overhead); /*
duration of an average MPDU in 1024 µs */
duration *= 1024; /* duration in µs */
duration /= AVG_PKT_SIZE; /* duration in µs for a byte */
duration *= len; /* duration in µs for the actual MPDU */
duration += (overhead >> agg_shift); /* duration in µs for an
approximate PPDU aka airtime */
duration /= 1024; /* airtime duration in 1024 µs*/
return duration;
```
[`ieee80211_calc_expected_tx_airtime` has to be fixed too]
The current patch:
```c
duration·=·ieee80211_get_rate_duration(hw,·&stat,·&overhead); /*
duration of an average MPDU in ns */
duration·*=·len;
duration·/=·AVG_PKT_SIZE; /* duration in ns for the actual MPDU */
duration·+=·(overhead·*·1024·>>·agg_shift); /* adding the overhead in
µs to a duration in ns to get PPDU duration: overhead [µs] == overhead *
1024 [ns] */
return·duration; /* airtime duration in ns */
```
> This is the same pattern used in ieee80211_calc_rx_airtime:
>
> duration *= len;
> duration /= AVG_PKT_SIZE;
> duration /= 1024;
> return duration + overhead;
>
> Your proposed rewrite would multiply the overhead by len / AVG_PKT_SIZE,
> making it proportional to packet size, which is incorrect, because a
> 512-byte frame and a 1500-byte frame have the same PHY preamble duration.
>
> - Felix
Pablo MG
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] wifi: mac80211: factor out part of ieee80211_calc_expected_tx_airtime
2026-03-25 11:21 ` Pablo MARTIN-GOMEZ
@ 2026-03-25 11:41 ` Felix Fietkau
2026-03-25 13:05 ` Pablo MARTIN-GOMEZ
0 siblings, 1 reply; 14+ messages in thread
From: Felix Fietkau @ 2026-03-25 11:41 UTC (permalink / raw)
To: Pablo MARTIN-GOMEZ, linux-wireless; +Cc: johannes
On 25.03.26 12:21, Pablo MARTIN-GOMEZ wrote:
> On 25/03/2026 04:58, Felix Fietkau wrote:
>> On 23.03.26 17:00, Pablo MARTIN-GOMEZ wrote:
>>> Hello,
>>>
> [...]
>>> I know this patch is just a refactoring, but I think this moved code is
>>> bugged. If (and it's a big if) I understood correctly the chain of
>>> macros and the comments, `ieee80211_get_rate_duration` return the
>>> `duration` in 1024 µs of an average packet (which would imply
>>> 1f38b8c564b8 is wrong) and the (PHY) `overhead` in µs for a (average)
>>> packet. So I believe the code should be:
>>> ```c
>>> duration = ieee80211_get_rate_duration(hw, &stat, &overhead);
>>> duration *= 1024; /* now duration is in µs */
>>> /* the agg_shift calculation has to be fixed */
>>> duration += (overhead >> agg_shift); /* for one packet, we
>>> "assign" a
>>> fraction of the overhead */
>>> duration *= len/AVG_PKT_SIZE; /* we multiply by the number of
>>> packets */
>>> duration /= 1024; /* we go back to a duration in 1024 µs*/
>>>
>>> return duration;
>>> ```
>>
>> The overhead (preamble, signal field, etc.) is a fixed per-frame PHY
>> cost that doesn't depend on how many data bytes are in the frame. In the
>> aggregated case, agg_shift amortizes that fixed cost across the
>> estimated number of subframes in the aggregate. So the correct order is:
>> scale the data duration to the actual packet size, then add the
>> amortized overhead once.
> My bad, I didn't understand that `len` was the byte size of a MPDU.
>
> So I was wrong on where I put the overhead, but (a priori) not on the
> rest of the calculation *if* my understanding of the units is correct.
> If 1f38b8c564b8 is correct and so `duration` is in ns and `overhead` is
> in µs, then your code is correct, but the commit message is wrong
> because `ieee80211_rate_expected_tx_airtime` is returning a value in ns.
>
> My snippet fixed if `duration` is in 1024 µs:
> ```c
> duration = ieee80211_get_rate_duration(hw, &stat, &overhead); /*
> duration of an average MPDU in 1024 µs */
> duration *= 1024; /* duration in µs */
> duration /= AVG_PKT_SIZE; /* duration in µs for a byte */
> duration *= len; /* duration in µs for the actual MPDU */
> duration += (overhead >> agg_shift); /* duration in µs for an
> approximate PPDU aka airtime */
> duration /= 1024; /* airtime duration in 1024 µs*/
>
> return duration;
> ```
> [`ieee80211_calc_expected_tx_airtime` has to be fixed too]
>
> The current patch:
> ```c
> duration·=·ieee80211_get_rate_duration(hw,·&stat,·&overhead); /*
> duration of an average MPDU in ns */
> duration·*=·len;
> duration·/=·AVG_PKT_SIZE; /* duration in ns for the actual MPDU */
> duration·+=·(overhead·*·1024·>>·agg_shift); /* adding the overhead in
> µs to a duration in ns to get PPDU duration: overhead [µs] == overhead *
> 1024 [ns] */
> return·duration; /* airtime duration in ns */
> ```
The formatting of your snippets is really weird. Are you using some kind
of LLM?
There is definitely an issue in the commit message, which I will fix in
v2. It says the unit is 1024 us, when in fact it should say that the
unit is 1/1024 us (approximately ns). Maybe that's what got you
confused. Either way, your 'fixed' snippet seems wrong to me and the
code should be fine as-is.
- Felix
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] wifi: mac80211: factor out part of ieee80211_calc_expected_tx_airtime
2026-03-25 11:41 ` Felix Fietkau
@ 2026-03-25 13:05 ` Pablo MARTIN-GOMEZ
0 siblings, 0 replies; 14+ messages in thread
From: Pablo MARTIN-GOMEZ @ 2026-03-25 13:05 UTC (permalink / raw)
To: Felix Fietkau, linux-wireless; +Cc: johannes
On 25/03/2026 12:41, Felix Fietkau wrote:
> On 25.03.26 12:21, Pablo MARTIN-GOMEZ wrote:
>> On 25/03/2026 04:58, Felix Fietkau wrote:
>>> On 23.03.26 17:00, Pablo MARTIN-GOMEZ wrote:
>>>> Hello,
>>>>
[...]
>> My bad, I didn't understand that `len` was the byte size of a MPDU.
>>
>> So I was wrong on where I put the overhead, but (a priori) not on the
>> rest of the calculation *if* my understanding of the units is correct.
>> If 1f38b8c564b8 is correct and so `duration` is in ns and `overhead` is
>> in µs, then your code is correct, but the commit message is wrong
>> because `ieee80211_rate_expected_tx_airtime` is returning a value in ns.
>>
>> My snippet fixed if `duration` is in 1024 µs:
>> ```c
>> duration = ieee80211_get_rate_duration(hw, &stat, &overhead); /*
>> duration of an average MPDU in 1024 µs */
>> duration *= 1024; /* duration in µs */
>> duration /= AVG_PKT_SIZE; /* duration in µs for a byte */
>> duration *= len; /* duration in µs for the actual MPDU */
>> duration += (overhead >> agg_shift); /* duration in µs for an
>> approximate PPDU aka airtime */
>> duration /= 1024; /* airtime duration in 1024 µs*/
>>
>> return duration;
>> ```
>> [`ieee80211_calc_expected_tx_airtime` has to be fixed too]
>>
>> The current patch:
>> ```c
>> duration·=·ieee80211_get_rate_duration(hw,·&stat,·&overhead); /*
>> duration of an average MPDU in ns */
>> duration·*=·len;
>> duration·/=·AVG_PKT_SIZE; /* duration in ns for the actual MPDU */
>> duration·+=·(overhead·*·1024·>>·agg_shift); /* adding the
>> overhead in
>> µs to a duration in ns to get PPDU duration: overhead [µs] == overhead *
>> 1024 [ns] */
>> return·duration; /* airtime duration in ns */
>> ```
>
> The formatting of your snippets is really weird. Are you using some kind
> of LLM?
That's handcrafted markdown, but then I have a MUA plugin that line
wraps everything, including code. I need to fix that, sorry about that.
>
> There is definitely an issue in the commit message, which I will fix in
> v2. It says the unit is 1024 us, when in fact it should say that the
> unit is 1/1024 us (approximately ns). Maybe that's what got you
> confused. Either way, your 'fixed' snippet seems wrong to me and the
> code should be fine as-is.
Ok then, with 1/1024 µs, the code looks fine. Now, I need to re-analyze
`airtime.c` to understand why `ieee80211_get_rate_duration` returns 1 /
1024 µs instead of 1024 µs like I though it was returning.
>
> - Felix
>
>
Pablo MG
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-03-25 13:05 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-23 10:19 [PATCH 1/4] wifi: mac80211: factor out part of ieee80211_calc_expected_tx_airtime Felix Fietkau
2026-03-23 10:19 ` [PATCH 2/4] wifi: mac80211: estimate expected throughput if not provided by driver/rc Felix Fietkau
2026-03-23 10:35 ` Johannes Berg
2026-03-23 10:19 ` [PATCH 3/4] wifi: mac80211: add AQL support for broadcast packets Felix Fietkau
2026-03-23 10:38 ` Johannes Berg
2026-03-23 10:19 ` [PATCH 4/4] wifi: mac80211: add ieee80211_txq_aql_pending() Felix Fietkau
2026-03-23 10:39 ` Johannes Berg
2026-03-23 10:43 ` Felix Fietkau
2026-03-23 10:55 ` Johannes Berg
2026-03-23 16:00 ` [PATCH 1/4] wifi: mac80211: factor out part of ieee80211_calc_expected_tx_airtime Pablo MARTIN-GOMEZ
2026-03-25 3:58 ` Felix Fietkau
2026-03-25 11:21 ` Pablo MARTIN-GOMEZ
2026-03-25 11:41 ` Felix Fietkau
2026-03-25 13:05 ` Pablo MARTIN-GOMEZ
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox