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

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

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