Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 5.9 3/3] mac80211: improve AQL aggregation estimation for
 low data rates
To:     Felix Fietkau <nbd@nbd.name>, linux-wireless@vger.kernel.org
Cc:     johannes@sipsolutions.net
References: <20200813155212.97884-1-nbd@nbd.name>
 <20200813155212.97884-3-nbd@nbd.name>
From:   Sebastian Gottschall <s.gottschall@dd-wrt.com>
Message-ID: <c3c5197f-17f7-4f6b-712f-d7c68f39ae38@dd-wrt.com>
Date:   Fri, 21 Aug 2020 16:46:55 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:80.0) Gecko/20100101
 Thunderbird/80.0
MIME-Version: 1.0
In-Reply-To: <20200813155212.97884-3-nbd@nbd.name>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

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)