developer | 42b6328 | 2022-06-16 13:33:13 +0800 | [diff] [blame^] | 1 | From: Felix Fietkau <nbd@nbd.name> |
| 2 | Date: Sat, 28 May 2022 16:44:53 +0200 |
| 3 | Subject: [PATCH] mac80211: fix overflow issues in airtime fairness code |
| 4 | |
| 5 | The airtime weight calculation overflows with a default weight value of 256 |
| 6 | whenever more than 8ms worth of airtime is reported. |
| 7 | Bigger weight values impose even smaller limits on maximum airtime values. |
| 8 | This can mess up airtime based calculations for drivers that don't report |
| 9 | per-PPDU airtime values, but batch up values instead. |
| 10 | |
| 11 | Fix this by reordering multiplications/shifts and by reducing unnecessary |
| 12 | intermediate precision (which was lost in a later stage anyway). |
| 13 | |
| 14 | The new shift value limits the maximum weight to 4096, which should be more |
| 15 | than enough. Any values bigger than that will be rejected. |
| 16 | |
| 17 | Signed-off-by: Felix Fietkau <nbd@nbd.name> |
| 18 | --- |
| 19 | |
| 20 | --- a/net/mac80211/cfg.c |
| 21 | +++ b/net/mac80211/cfg.c |
| 22 | @@ -1602,6 +1602,9 @@ static int sta_apply_parameters(struct i |
| 23 | mask = params->sta_flags_mask; |
| 24 | set = params->sta_flags_set; |
| 25 | |
| 26 | + if (params->airtime_weight > BIT(IEEE80211_RECIPROCAL_SHIFT_STA)) |
| 27 | + return -EINVAL; |
| 28 | + |
| 29 | if (ieee80211_vif_is_mesh(&sdata->vif)) { |
| 30 | /* |
| 31 | * In mesh mode, ASSOCIATED isn't part of the nl80211 |
| 32 | --- a/net/mac80211/ieee80211_i.h |
| 33 | +++ b/net/mac80211/ieee80211_i.h |
| 34 | @@ -1666,50 +1666,33 @@ static inline struct airtime_info *to_ai |
| 35 | /* To avoid divisions in the fast path, we keep pre-computed reciprocals for |
| 36 | * airtime weight calculations. There are two different weights to keep track |
| 37 | * of: The per-station weight and the sum of weights per phy. |
| 38 | - * |
| 39 | - * For the per-station weights (kept in airtime_info below), we use 32-bit |
| 40 | - * reciprocals with a devisor of 2^19. This lets us keep the multiplications and |
| 41 | - * divisions for the station weights as 32-bit operations at the cost of a bit |
| 42 | - * of rounding error for high weights; but the choice of divisor keeps rounding |
| 43 | - * errors <10% for weights <2^15, assuming no more than 8ms of airtime is |
| 44 | - * reported at a time. |
| 45 | - * |
| 46 | - * For the per-phy sum of weights the values can get higher, so we use 64-bit |
| 47 | - * operations for those with a 32-bit divisor, which should avoid any |
| 48 | - * significant rounding errors. |
| 49 | + * The per-sta shift value supports weight values of 1-4096 |
| 50 | */ |
| 51 | -#define IEEE80211_RECIPROCAL_DIVISOR_64 0x100000000ULL |
| 52 | -#define IEEE80211_RECIPROCAL_SHIFT_64 32 |
| 53 | -#define IEEE80211_RECIPROCAL_DIVISOR_32 0x80000U |
| 54 | -#define IEEE80211_RECIPROCAL_SHIFT_32 19 |
| 55 | +#define IEEE80211_RECIPROCAL_SHIFT_SUM 24 |
| 56 | +#define IEEE80211_RECIPROCAL_SHIFT_STA 12 |
| 57 | +#define IEEE80211_WEIGHT_SHIFT 8 |
| 58 | |
| 59 | -static inline void airtime_weight_set(struct airtime_info *air_info, u16 weight) |
| 60 | +static inline void airtime_weight_set(struct airtime_info *air_info, u32 weight) |
| 61 | { |
| 62 | if (air_info->weight == weight) |
| 63 | return; |
| 64 | |
| 65 | air_info->weight = weight; |
| 66 | - if (weight) { |
| 67 | - air_info->weight_reciprocal = |
| 68 | - IEEE80211_RECIPROCAL_DIVISOR_32 / weight; |
| 69 | - } else { |
| 70 | - air_info->weight_reciprocal = 0; |
| 71 | - } |
| 72 | + if (weight) |
| 73 | + weight = BIT(IEEE80211_RECIPROCAL_SHIFT_STA) / weight; |
| 74 | + air_info->weight_reciprocal = weight; |
| 75 | } |
| 76 | |
| 77 | static inline void airtime_weight_sum_set(struct airtime_sched_info *air_sched, |
| 78 | - int weight_sum) |
| 79 | + u32 weight_sum) |
| 80 | { |
| 81 | if (air_sched->weight_sum == weight_sum) |
| 82 | return; |
| 83 | |
| 84 | air_sched->weight_sum = weight_sum; |
| 85 | - if (air_sched->weight_sum) { |
| 86 | - air_sched->weight_sum_reciprocal = IEEE80211_RECIPROCAL_DIVISOR_64; |
| 87 | - do_div(air_sched->weight_sum_reciprocal, air_sched->weight_sum); |
| 88 | - } else { |
| 89 | - air_sched->weight_sum_reciprocal = 0; |
| 90 | - } |
| 91 | + if (weight_sum) |
| 92 | + weight_sum = BIT(IEEE80211_RECIPROCAL_SHIFT_SUM) / weight_sum; |
| 93 | + air_sched->weight_sum_reciprocal = weight_sum; |
| 94 | } |
| 95 | |
| 96 | /* A problem when trying to enforce airtime fairness is that we want to divide |
| 97 | --- a/net/mac80211/sta_info.c |
| 98 | +++ b/net/mac80211/sta_info.c |
| 99 | @@ -1894,9 +1894,9 @@ void ieee80211_register_airtime(struct i |
| 100 | { |
| 101 | struct ieee80211_sub_if_data *sdata = vif_to_sdata(txq->vif); |
| 102 | struct ieee80211_local *local = sdata->local; |
| 103 | - u64 weight_sum, weight_sum_reciprocal; |
| 104 | struct airtime_sched_info *air_sched; |
| 105 | struct airtime_info *air_info; |
| 106 | + u64 weight_sum_reciprocal; |
| 107 | u32 airtime = 0; |
| 108 | |
| 109 | air_sched = &local->airtime[txq->ac]; |
| 110 | @@ -1907,27 +1907,21 @@ void ieee80211_register_airtime(struct i |
| 111 | if (local->airtime_flags & AIRTIME_USE_RX) |
| 112 | airtime += rx_airtime; |
| 113 | |
| 114 | - /* Weights scale so the unit weight is 256 */ |
| 115 | - airtime <<= 8; |
| 116 | - |
| 117 | spin_lock_bh(&air_sched->lock); |
| 118 | |
| 119 | air_info->tx_airtime += tx_airtime; |
| 120 | air_info->rx_airtime += rx_airtime; |
| 121 | |
| 122 | - if (air_sched->weight_sum) { |
| 123 | - weight_sum = air_sched->weight_sum; |
| 124 | + if (air_sched->weight_sum) |
| 125 | weight_sum_reciprocal = air_sched->weight_sum_reciprocal; |
| 126 | - } else { |
| 127 | - weight_sum = air_info->weight; |
| 128 | + else |
| 129 | weight_sum_reciprocal = air_info->weight_reciprocal; |
| 130 | - } |
| 131 | |
| 132 | /* Round the calculation of global vt */ |
| 133 | - air_sched->v_t += (u64)((airtime + (weight_sum >> 1)) * |
| 134 | - weight_sum_reciprocal) >> IEEE80211_RECIPROCAL_SHIFT_64; |
| 135 | - air_info->v_t += (u32)((airtime + (air_info->weight >> 1)) * |
| 136 | - air_info->weight_reciprocal) >> IEEE80211_RECIPROCAL_SHIFT_32; |
| 137 | + air_sched->v_t += ((u64)airtime * weight_sum_reciprocal) >> |
| 138 | + (IEEE80211_RECIPROCAL_SHIFT_SUM - IEEE80211_WEIGHT_SHIFT); |
| 139 | + air_info->v_t += (airtime * air_info->weight_reciprocal) >> |
| 140 | + (IEEE80211_RECIPROCAL_SHIFT_STA - IEEE80211_WEIGHT_SHIFT); |
| 141 | ieee80211_resort_txq(&local->hw, txq); |
| 142 | |
| 143 | spin_unlock_bh(&air_sched->lock); |