Re: [PATCH 1/4] wifi: mac80211: factor out part of ieee80211_calc_expected_tx_airtime

[Pablo MARTIN-GOMEZ <pmartin-gomez@freebox.fr>]

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