linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 5.9 1/3] mac80211: use rate provided via status->rate on ieee80211_tx_status_ext for AQL
@ 2020-08-13 15:52 Felix Fietkau
  2020-08-13 15:52 ` [PATCH 5.9 2/3] mac80211: factor out code to look up the average packet length duration for a rate Felix Fietkau
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Felix Fietkau @ 2020-08-13 15:52 UTC (permalink / raw)
  To: linux-wireless; +Cc: johannes

Since ieee80211_tx_info does not have enough room to encode HE rates, HE
drivers use status->rate to provide rate info.
Store it in struct sta_info and use it for AQL.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 net/mac80211/airtime.c  | 53 +++++++++++++++++++++++++++++++++++++++--
 net/mac80211/sta_info.h |  1 +
 net/mac80211/status.c   | 12 ++++++----
 3 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/net/mac80211/airtime.c b/net/mac80211/airtime.c
index 366f76c9003d..656c9a033714 100644
--- a/net/mac80211/airtime.c
+++ b/net/mac80211/airtime.c
@@ -487,14 +487,61 @@ u32 ieee80211_calc_rx_airtime(struct ieee80211_hw *hw,
 }
 EXPORT_SYMBOL_GPL(ieee80211_calc_rx_airtime);
 
+static bool ieee80211_fill_rate_info(struct ieee80211_hw *hw,
+				     struct ieee80211_rx_status *stat, u8 band,
+				     struct rate_info *ri)
+{
+	struct ieee80211_supported_band *sband = hw->wiphy->bands[band];
+	int i;
+
+	if (!ri || !sband)
+	    return false;
+
+	stat->bw = ri->bw;
+	stat->nss = ri->nss;
+	stat->rate_idx = ri->mcs;
+
+	if (ri->flags & RATE_INFO_FLAGS_HE_MCS)
+		stat->encoding = RX_ENC_HE;
+	else if (ri->flags & RATE_INFO_FLAGS_VHT_MCS)
+		stat->encoding = RX_ENC_VHT;
+	else if (ri->flags & RATE_INFO_FLAGS_MCS)
+		stat->encoding = RX_ENC_HT;
+	else
+		stat->encoding = RX_ENC_LEGACY;
+
+	if (ri->flags & RATE_INFO_FLAGS_SHORT_GI)
+		stat->enc_flags |= RX_ENC_FLAG_SHORT_GI;
+
+	stat->he_gi = ri->he_gi;
+
+	if (stat->encoding != RX_ENC_LEGACY)
+		return true;
+
+	stat->rate_idx = 0;
+	for (i = 0; i < sband->n_bitrates; i++) {
+		if (ri->legacy != sband->bitrates[i].bitrate)
+			continue;
+
+		stat->rate_idx = i;
+		return true;
+	}
+
+	return false;
+}
+
 static u32 ieee80211_calc_tx_airtime_rate(struct ieee80211_hw *hw,
 					  struct ieee80211_tx_rate *rate,
+					  struct rate_info *ri,
 					  u8 band, int len)
 {
 	struct ieee80211_rx_status stat = {
 		.band = band,
 	};
 
+	if (ieee80211_fill_rate_info(hw, &stat, band, ri))
+		goto out;
+
 	if (rate->idx < 0 || !rate->count)
 		return 0;
 
@@ -522,6 +569,7 @@ static u32 ieee80211_calc_tx_airtime_rate(struct ieee80211_hw *hw,
 		stat.encoding = RX_ENC_LEGACY;
 	}
 
+out:
 	return ieee80211_calc_rx_airtime(hw, &stat, len);
 }
 
@@ -536,7 +584,7 @@ u32 ieee80211_calc_tx_airtime(struct ieee80211_hw *hw,
 		struct ieee80211_tx_rate *rate = &info->status.rates[i];
 		u32 cur_duration;
 
-		cur_duration = ieee80211_calc_tx_airtime_rate(hw, rate,
+		cur_duration = ieee80211_calc_tx_airtime_rate(hw, rate, NULL,
 							      info->band, len);
 		if (!cur_duration)
 			break;
@@ -573,6 +621,7 @@ u32 ieee80211_calc_expected_tx_airtime(struct ieee80211_hw *hw,
 		struct sta_info *sta = container_of(pubsta, struct sta_info,
 						    sta);
 		struct ieee80211_tx_rate *rate = &sta->tx_stats.last_rate;
+		struct rate_info *ri = &sta->tx_stats.last_rate_info;
 		u32 airtime;
 
 		if (!(rate->flags & (IEEE80211_TX_RC_VHT_MCS |
@@ -586,7 +635,7 @@ u32 ieee80211_calc_expected_tx_airtime(struct ieee80211_hw *hw,
 		 * This will not be very accurate, but much better than simply
 		 * assuming un-aggregated tx.
 		 */
-		airtime = ieee80211_calc_tx_airtime_rate(hw, rate, band,
+		airtime = ieee80211_calc_tx_airtime_rate(hw, rate, ri, band,
 							 ampdu ? len * 16 : len);
 		if (ampdu)
 			airtime /= 16;
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 9d398c9daa4c..8060d142e4f1 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -611,6 +611,7 @@ struct sta_info {
 		u64 packets[IEEE80211_NUM_ACS];
 		u64 bytes[IEEE80211_NUM_ACS];
 		struct ieee80211_tx_rate last_rate;
+		struct rate_info last_rate_info;
 		u64 msdu[IEEE80211_NUM_TIDS + 1];
 	} tx_stats;
 	u16 tid_seq[IEEE80211_QOS_CTL_TID_MASK + 1];
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index adb1d30ce06e..6de63f1d8c7b 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -1137,9 +1137,17 @@ void ieee80211_tx_status_ext(struct ieee80211_hw *hw,
 	struct ieee80211_tx_info *info = status->info;
 	struct ieee80211_sta *pubsta = status->sta;
 	struct ieee80211_supported_band *sband;
+	struct sta_info *sta;
 	int retry_count;
 	bool acked, noack_success;
 
+	if (pubsta) {
+		sta = container_of(pubsta, struct sta_info, sta);
+
+		if (status->rate)
+			sta->tx_stats.last_rate_info = *status->rate;
+	}
+
 	if (status->skb)
 		return __ieee80211_tx_status(hw, status);
 
@@ -1154,10 +1162,6 @@ void ieee80211_tx_status_ext(struct ieee80211_hw *hw,
 	noack_success = !!(info->flags & IEEE80211_TX_STAT_NOACK_TRANSMITTED);
 
 	if (pubsta) {
-		struct sta_info *sta;
-
-		sta = container_of(pubsta, struct sta_info, sta);
-
 		if (!acked && !noack_success)
 			sta->status_stats.retry_failed++;
 		sta->status_stats.retry_count += retry_count;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5.9 2/3] mac80211: factor out code to look up the average packet length duration for a rate
  2020-08-13 15:52 [PATCH 5.9 1/3] mac80211: use rate provided via status->rate on ieee80211_tx_status_ext for AQL Felix Fietkau
@ 2020-08-13 15:52 ` Felix Fietkau
  2020-08-20 12:44   ` Toke Høiland-Jørgensen
  2020-08-13 15:52 ` [PATCH 5.9 3/3] mac80211: improve AQL aggregation estimation for low data rates Felix Fietkau
  2020-08-20 12:43 ` [PATCH 5.9 1/3] mac80211: use rate provided via status->rate on ieee80211_tx_status_ext for AQL Toke Høiland-Jørgensen
  2 siblings, 1 reply; 12+ messages in thread
From: Felix Fietkau @ 2020-08-13 15:52 UTC (permalink / raw)
  To: linux-wireless; +Cc: johannes

This will be used to enhance AQL estimated aggregation length

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 net/mac80211/airtime.c | 120 +++++++++++++++++++++++++----------------
 1 file changed, 73 insertions(+), 47 deletions(-)

diff --git a/net/mac80211/airtime.c b/net/mac80211/airtime.c
index 656c9a033714..2d959b22b141 100644
--- a/net/mac80211/airtime.c
+++ b/net/mac80211/airtime.c
@@ -405,18 +405,14 @@ ieee80211_calc_legacy_rate_duration(u16 bitrate, bool short_pre,
 	return duration;
 }
 
-u32 ieee80211_calc_rx_airtime(struct ieee80211_hw *hw,
-			      struct ieee80211_rx_status *status,
-			      int len)
+static u32 ieee80211_get_rate_duration(struct ieee80211_hw *hw,
+				       struct ieee80211_rx_status *status,
+				       u32 *overhead)
 {
-	struct ieee80211_supported_band *sband;
-	const struct ieee80211_rate *rate;
 	bool sgi = status->enc_flags & RX_ENC_FLAG_SHORT_GI;
-	bool sp = status->enc_flags & RX_ENC_FLAG_SHORTPRE;
 	int bw, streams;
 	int group, idx;
 	u32 duration;
-	bool cck;
 
 	switch (status->bw) {
 	case RATE_INFO_BW_20:
@@ -437,20 +433,6 @@ u32 ieee80211_calc_rx_airtime(struct ieee80211_hw *hw,
 	}
 
 	switch (status->encoding) {
-	case RX_ENC_LEGACY:
-		if (WARN_ON_ONCE(status->band > NL80211_BAND_5GHZ))
-			return 0;
-
-		sband = hw->wiphy->bands[status->band];
-		if (!sband || status->rate_idx >= sband->n_bitrates)
-			return 0;
-
-		rate = &sband->bitrates[status->rate_idx];
-		cck = rate->flags & IEEE80211_RATE_MANDATORY_B;
-
-		return ieee80211_calc_legacy_rate_duration(rate->bitrate, sp,
-							   cck, len);
-
 	case RX_ENC_VHT:
 		streams = status->nss;
 		idx = status->rate_idx;
@@ -477,13 +459,47 @@ u32 ieee80211_calc_rx_airtime(struct ieee80211_hw *hw,
 
 	duration = airtime_mcs_groups[group].duration[idx];
 	duration <<= airtime_mcs_groups[group].shift;
+	*overhead = 36 + (streams << 2);
+
+	return duration;
+}
+
+
+u32 ieee80211_calc_rx_airtime(struct ieee80211_hw *hw,
+			      struct ieee80211_rx_status *status,
+			      int len)
+{
+	struct ieee80211_supported_band *sband;
+	u32 duration, overhead = 0;
+
+	if (status->encoding == RX_ENC_LEGACY) {
+		const struct ieee80211_rate *rate;
+		bool sp = status->enc_flags & RX_ENC_FLAG_SHORTPRE;
+		bool cck;
+
+		if (WARN_ON_ONCE(status->band > NL80211_BAND_5GHZ))
+			return 0;
+
+		sband = hw->wiphy->bands[status->band];
+		if (!sband || status->rate_idx >= sband->n_bitrates)
+			return 0;
+
+		rate = &sband->bitrates[status->rate_idx];
+		cck = rate->flags & IEEE80211_RATE_MANDATORY_B;
+
+		return ieee80211_calc_legacy_rate_duration(rate->bitrate, sp,
+							   cck, len);
+	}
+
+	duration = ieee80211_get_rate_duration(hw, status, &overhead);
+	if (!duration)
+		return 0;
+
 	duration *= len;
 	duration /= AVG_PKT_SIZE;
 	duration /= 1024;
 
-	duration += 36 + (streams << 2);
-
-	return duration;
+	return duration + overhead;
 }
 EXPORT_SYMBOL_GPL(ieee80211_calc_rx_airtime);
 
@@ -530,46 +546,56 @@ static bool ieee80211_fill_rate_info(struct ieee80211_hw *hw,
 	return false;
 }
 
-static u32 ieee80211_calc_tx_airtime_rate(struct ieee80211_hw *hw,
-					  struct ieee80211_tx_rate *rate,
-					  struct rate_info *ri,
-					  u8 band, int len)
+static int ieee80211_fill_rx_status(struct ieee80211_rx_status *stat,
+				    struct ieee80211_hw *hw,
+				    struct ieee80211_tx_rate *rate,
+				    struct rate_info *ri, u8 band, int len)
 {
-	struct ieee80211_rx_status stat = {
-		.band = band,
-	};
+	memset(stat, 0, sizeof(*stat));
+	stat->band = band;
 
-	if (ieee80211_fill_rate_info(hw, &stat, band, ri))
-		goto out;
+	if (ieee80211_fill_rate_info(hw, stat, band, ri))
+		return 0;
 
 	if (rate->idx < 0 || !rate->count)
-		return 0;
+		return -1;
 
 	if (rate->flags & IEEE80211_TX_RC_80_MHZ_WIDTH)
-		stat.bw = RATE_INFO_BW_80;
+		stat->bw = RATE_INFO_BW_80;
 	else if (rate->flags & IEEE80211_TX_RC_40_MHZ_WIDTH)
-		stat.bw = RATE_INFO_BW_40;
+		stat->bw = RATE_INFO_BW_40;
 	else
-		stat.bw = RATE_INFO_BW_20;
+		stat->bw = RATE_INFO_BW_20;
 
-	stat.enc_flags = 0;
+	stat->enc_flags = 0;
 	if (rate->flags & IEEE80211_TX_RC_USE_SHORT_PREAMBLE)
-		stat.enc_flags |= RX_ENC_FLAG_SHORTPRE;
+		stat->enc_flags |= RX_ENC_FLAG_SHORTPRE;
 	if (rate->flags & IEEE80211_TX_RC_SHORT_GI)
-		stat.enc_flags |= RX_ENC_FLAG_SHORT_GI;
+		stat->enc_flags |= RX_ENC_FLAG_SHORT_GI;
 
-	stat.rate_idx = rate->idx;
+	stat->rate_idx = rate->idx;
 	if (rate->flags & IEEE80211_TX_RC_VHT_MCS) {
-		stat.encoding = RX_ENC_VHT;
-		stat.rate_idx = ieee80211_rate_get_vht_mcs(rate);
-		stat.nss = ieee80211_rate_get_vht_nss(rate);
+		stat->encoding = RX_ENC_VHT;
+		stat->rate_idx = ieee80211_rate_get_vht_mcs(rate);
+		stat->nss = ieee80211_rate_get_vht_nss(rate);
 	} else if (rate->flags & IEEE80211_TX_RC_MCS) {
-		stat.encoding = RX_ENC_HT;
+		stat->encoding = RX_ENC_HT;
 	} else {
-		stat.encoding = RX_ENC_LEGACY;
+		stat->encoding = RX_ENC_LEGACY;
 	}
 
-out:
+	return 0;
+}
+
+static u32 ieee80211_calc_tx_airtime_rate(struct ieee80211_hw *hw,
+					  struct ieee80211_tx_rate *rate,
+					  struct rate_info *ri,
+					  u8 band, int len)
+{
+	struct ieee80211_rx_status stat;
+
+	ieee80211_fill_rx_status(&stat, hw, rate, ri, band, len);
+
 	return ieee80211_calc_rx_airtime(hw, &stat, len);
 }
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5.9 3/3] mac80211: improve AQL aggregation estimation for low data rates
  2020-08-13 15:52 [PATCH 5.9 1/3] mac80211: use rate provided via status->rate on ieee80211_tx_status_ext for AQL Felix Fietkau
  2020-08-13 15:52 ` [PATCH 5.9 2/3] mac80211: factor out code to look up the average packet length duration for a rate Felix Fietkau
@ 2020-08-13 15:52 ` Felix Fietkau
  2020-08-20 12:44   ` Toke Høiland-Jørgensen
  2020-08-21 14:46   ` Sebastian Gottschall
  2020-08-20 12:43 ` [PATCH 5.9 1/3] mac80211: use rate provided via status->rate on ieee80211_tx_status_ext for AQL Toke Høiland-Jørgensen
  2 siblings, 2 replies; 12+ messages in thread
From: Felix Fietkau @ 2020-08-13 15:52 UTC (permalink / raw)
  To: linux-wireless; +Cc: johannes

Links with low data rates use much smaller aggregates and are much more
sensitive to latency added by bufferbloat.
Tune the assumed aggregation length based on the tx rate duration.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 net/mac80211/airtime.c | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/net/mac80211/airtime.c b/net/mac80211/airtime.c
index 2d959b22b141..6a46771f9d54 100644
--- a/net/mac80211/airtime.c
+++ b/net/mac80211/airtime.c
@@ -646,27 +646,40 @@ 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 *rate = &sta->tx_stats.last_rate;
 		struct rate_info *ri = &sta->tx_stats.last_rate_info;
-		u32 airtime;
+		u32 duration, overhead;
+		u8 agg_shift;
 
-		if (!(rate->flags & (IEEE80211_TX_RC_VHT_MCS |
-				     IEEE80211_TX_RC_MCS)))
-			ampdu = false;
+		ieee80211_fill_rx_status(&stat, hw, rate, ri, band, len);
 
+		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 of 16.
+		 * 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.
+		 * assuming un-aggregated tx in all cases.
 		 */
-		airtime = ieee80211_calc_tx_airtime_rate(hw, rate, ri, band,
-							 ampdu ? len * 16 : len);
-		if (ampdu)
-			airtime /= 16;
-
-		return airtime;
+		if (duration > 400) /* <= VHT20 MCS2 1S */
+			agg_shift = 1;
+		else if (duration > 250) /* <= VHT20 MCS3 1S or MCS1 2S */
+			agg_shift = 2;
+		else if (duration > 150) /* <= VHT20 MCS5 1S or MCS3 2S */
+			agg_shift = 3;
+		else
+			agg_shift = 4;
+
+		duration *= len;
+		duration /= AVG_PKT_SIZE;
+		duration /= 1024;
+
+		return duration + (overhead >> agg_shift);
 	}
 
 	if (!conf)
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 5.9 1/3] mac80211: use rate provided via status->rate on ieee80211_tx_status_ext for AQL
  2020-08-13 15:52 [PATCH 5.9 1/3] mac80211: use rate provided via status->rate on ieee80211_tx_status_ext for AQL Felix Fietkau
  2020-08-13 15:52 ` [PATCH 5.9 2/3] mac80211: factor out code to look up the average packet length duration for a rate Felix Fietkau
  2020-08-13 15:52 ` [PATCH 5.9 3/3] mac80211: improve AQL aggregation estimation for low data rates Felix Fietkau
@ 2020-08-20 12:43 ` Toke Høiland-Jørgensen
  2 siblings, 0 replies; 12+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-08-20 12:43 UTC (permalink / raw)
  To: Felix Fietkau, linux-wireless; +Cc: johannes

Felix Fietkau <nbd@nbd.name> writes:

> Since ieee80211_tx_info does not have enough room to encode HE rates, HE
> drivers use status->rate to provide rate info.
> Store it in struct sta_info and use it for AQL.
>
> Signed-off-by: Felix Fietkau <nbd@nbd.name>

Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5.9 2/3] mac80211: factor out code to look up the average packet length duration for a rate
  2020-08-13 15:52 ` [PATCH 5.9 2/3] mac80211: factor out code to look up the average packet length duration for a rate Felix Fietkau
@ 2020-08-20 12:44   ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 12+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-08-20 12:44 UTC (permalink / raw)
  To: Felix Fietkau, linux-wireless; +Cc: johannes

Felix Fietkau <nbd@nbd.name> writes:

> This will be used to enhance AQL estimated aggregation length
>
> Signed-off-by: Felix Fietkau <nbd@nbd.name>

Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5.9 3/3] mac80211: improve AQL aggregation estimation for low data rates
  2020-08-13 15:52 ` [PATCH 5.9 3/3] mac80211: improve AQL aggregation estimation for low data rates Felix Fietkau
@ 2020-08-20 12:44   ` Toke Høiland-Jørgensen
  2020-08-21 14:46   ` Sebastian Gottschall
  1 sibling, 0 replies; 12+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-08-20 12:44 UTC (permalink / raw)
  To: Felix Fietkau, linux-wireless; +Cc: johannes

Felix Fietkau <nbd@nbd.name> writes:

> Links with low data rates use much smaller aggregates and are much more
> sensitive to latency added by bufferbloat.
> Tune the assumed aggregation length based on the tx rate duration.
>
> Signed-off-by: Felix Fietkau <nbd@nbd.name>

Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5.9 3/3] mac80211: improve AQL aggregation estimation for low data rates
  2020-08-13 15:52 ` [PATCH 5.9 3/3] mac80211: improve AQL aggregation estimation for low data rates Felix Fietkau
  2020-08-20 12:44   ` Toke Høiland-Jørgensen
@ 2020-08-21 14:46   ` Sebastian Gottschall
  2020-08-21 14:58     ` Toke Høiland-Jørgensen
  1 sibling, 1 reply; 12+ messages in thread
From: Sebastian Gottschall @ 2020-08-21 14:46 UTC (permalink / raw)
  To: Felix Fietkau, linux-wireless; +Cc: johannes

in my tests the patch series decreases the performance of ath10k 
chipsets by 50%, even if feature is not enabled.
so sorry. no ack from me.
so far i was able to reproduce it on linksys ea8500 and asrock-G10 both 
with qca99xx chipset

Am 13.08.2020 um 17:52 schrieb Felix Fietkau:
> Links with low data rates use much smaller aggregates and are much more
> sensitive to latency added by bufferbloat.
> Tune the assumed aggregation length based on the tx rate duration.
>
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> ---
>   net/mac80211/airtime.c | 37 +++++++++++++++++++++++++------------
>   1 file changed, 25 insertions(+), 12 deletions(-)
>
> diff --git a/net/mac80211/airtime.c b/net/mac80211/airtime.c
> index 2d959b22b141..6a46771f9d54 100644
> --- a/net/mac80211/airtime.c
> +++ b/net/mac80211/airtime.c
> @@ -646,27 +646,40 @@ 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 *rate = &sta->tx_stats.last_rate;
>   		struct rate_info *ri = &sta->tx_stats.last_rate_info;
> -		u32 airtime;
> +		u32 duration, overhead;
> +		u8 agg_shift;
>   
> -		if (!(rate->flags & (IEEE80211_TX_RC_VHT_MCS |
> -				     IEEE80211_TX_RC_MCS)))
> -			ampdu = false;
> +		ieee80211_fill_rx_status(&stat, hw, rate, ri, band, len);
>   
> +		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 of 16.
> +		 * 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.
> +		 * assuming un-aggregated tx in all cases.
>   		 */
> -		airtime = ieee80211_calc_tx_airtime_rate(hw, rate, ri, band,
> -							 ampdu ? len * 16 : len);
> -		if (ampdu)
> -			airtime /= 16;
> -
> -		return airtime;
> +		if (duration > 400) /* <= VHT20 MCS2 1S */
> +			agg_shift = 1;
> +		else if (duration > 250) /* <= VHT20 MCS3 1S or MCS1 2S */
> +			agg_shift = 2;
> +		else if (duration > 150) /* <= VHT20 MCS5 1S or MCS3 2S */
> +			agg_shift = 3;
> +		else
> +			agg_shift = 4;
> +
> +		duration *= len;
> +		duration /= AVG_PKT_SIZE;
> +		duration /= 1024;
> +
> +		return duration + (overhead >> agg_shift);
>   	}
>   
>   	if (!conf)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5.9 3/3] mac80211: improve AQL aggregation estimation for low data rates
  2020-08-21 14:46   ` Sebastian Gottschall
@ 2020-08-21 14:58     ` Toke Høiland-Jørgensen
  2020-08-21 15:09       ` Sebastian Gottschall
  0 siblings, 1 reply; 12+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-08-21 14:58 UTC (permalink / raw)
  To: Sebastian Gottschall, Felix Fietkau, linux-wireless; +Cc: johannes

Sebastian Gottschall <s.gottschall@dd-wrt.com> writes:

> in my tests the patch series decreases the performance of ath10k 
> chipsets by 50%, even if feature is not enabled.

"even if feature is not enabled" - you mean without the AQL feature bit
set in the driver? That makes no sense...

-Toke


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5.9 3/3] mac80211: improve AQL aggregation estimation for low data rates
  2020-08-21 14:58     ` Toke Høiland-Jørgensen
@ 2020-08-21 15:09       ` Sebastian Gottschall
  2020-08-21 15:33         ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 12+ messages in thread
From: Sebastian Gottschall @ 2020-08-21 15:09 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, Felix Fietkau, linux-wireless; +Cc: johannes

i mean airtime fairness policy was not configured

Am 21.08.2020 um 16:58 schrieb Toke Høiland-Jørgensen:
> Sebastian Gottschall <s.gottschall@dd-wrt.com> writes:
>
>> in my tests the patch series decreases the performance of ath10k
>> chipsets by 50%, even if feature is not enabled.
> "even if feature is not enabled" - you mean without the AQL feature bit
> set in the driver? That makes no sense...
>
> -Toke
>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5.9 3/3] mac80211: improve AQL aggregation estimation for low data rates
  2020-08-21 15:09       ` Sebastian Gottschall
@ 2020-08-21 15:33         ` Toke Høiland-Jørgensen
  2020-08-21 16:22           ` Felix Fietkau
  0 siblings, 1 reply; 12+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-08-21 15:33 UTC (permalink / raw)
  To: Sebastian Gottschall, Felix Fietkau, linux-wireless; +Cc: johannes

Sebastian Gottschall <s.gottschall@dd-wrt.com> writes:

> i mean airtime fairness policy was not configured

Ah, right, that makes a lot more sense :)

Still not sure why this would affect throughput, though; maybe the
change in calculation makes AQL overestimate the airtime and throttle
too much? Or does the saving of last_rate_info on tx_status_ext() work
badly for ath10k?

-Toke


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5.9 3/3] mac80211: improve AQL aggregation estimation for low data rates
  2020-08-21 15:33         ` Toke Høiland-Jørgensen
@ 2020-08-21 16:22           ` Felix Fietkau
  2020-08-21 16:37             ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 12+ messages in thread
From: Felix Fietkau @ 2020-08-21 16:22 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, Sebastian Gottschall, linux-wireless
  Cc: johannes

On 2020-08-21 17:33, Toke Høiland-Jørgensen wrote:
> Sebastian Gottschall <s.gottschall@dd-wrt.com> writes:
> 
>> i mean airtime fairness policy was not configured
> 
> Ah, right, that makes a lot more sense :)
> 
> Still not sure why this would affect throughput, though; maybe the
> change in calculation makes AQL overestimate the airtime and throttle
> too much? Or does the saving of last_rate_info on tx_status_ext() work
> badly for ath10k?
The issue turned out to be missing return code checks on converting the
tx rate to struct ieee80211_rx_status.
I'll send v2 of this series with fixes.

- Felix

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5.9 3/3] mac80211: improve AQL aggregation estimation for low data rates
  2020-08-21 16:22           ` Felix Fietkau
@ 2020-08-21 16:37             ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 12+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-08-21 16:37 UTC (permalink / raw)
  To: Felix Fietkau, Sebastian Gottschall, linux-wireless; +Cc: johannes

Felix Fietkau <nbd@nbd.name> writes:

> On 2020-08-21 17:33, Toke Høiland-Jørgensen wrote:
>> Sebastian Gottschall <s.gottschall@dd-wrt.com> writes:
>> 
>>> i mean airtime fairness policy was not configured
>> 
>> Ah, right, that makes a lot more sense :)
>> 
>> Still not sure why this would affect throughput, though; maybe the
>> change in calculation makes AQL overestimate the airtime and throttle
>> too much? Or does the saving of last_rate_info on tx_status_ext() work
>> badly for ath10k?
> The issue turned out to be missing return code checks on converting the
> tx rate to struct ieee80211_rx_status.
> I'll send v2 of this series with fixes.

Ah, great! :)

-Toke


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-08-21 16:38 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-13 15:52 [PATCH 5.9 1/3] mac80211: use rate provided via status->rate on ieee80211_tx_status_ext for AQL Felix Fietkau
2020-08-13 15:52 ` [PATCH 5.9 2/3] mac80211: factor out code to look up the average packet length duration for a rate Felix Fietkau
2020-08-20 12:44   ` Toke Høiland-Jørgensen
2020-08-13 15:52 ` [PATCH 5.9 3/3] mac80211: improve AQL aggregation estimation for low data rates Felix Fietkau
2020-08-20 12:44   ` Toke Høiland-Jørgensen
2020-08-21 14:46   ` Sebastian Gottschall
2020-08-21 14:58     ` Toke Høiland-Jørgensen
2020-08-21 15:09       ` Sebastian Gottschall
2020-08-21 15:33         ` Toke Høiland-Jørgensen
2020-08-21 16:22           ` Felix Fietkau
2020-08-21 16:37             ` Toke Høiland-Jørgensen
2020-08-20 12:43 ` [PATCH 5.9 1/3] mac80211: use rate provided via status->rate on ieee80211_tx_status_ext for AQL Toke Høiland-Jørgensen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).