Commit f9c2ff22bb2df7b8f153afd2a4bea07176bad144
Committed by
David S. Miller
1 parent
1edaa7e8a7
net: tcp: dctcp_update_alpha() fixes.
dctcp_alpha can be read by from dctcp_get_info() without synchro, so use WRITE_ONCE() to prevent compiler from using dctcp_alpha as a temporary variable. Also, playing with small dctcp_shift_g (like 1), can expose an overflow with 32bit values shifted 9 times before divide. Use an u64 field to avoid this problem, and perform the divide only if acked_bytes_ecn is not zero. Signed-off-by: Eric Dumazet <edumazet@google.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Showing 1 changed file with 16 additions and 10 deletions Side-by-side Diff
net/ipv4/tcp_dctcp.c
... | ... | @@ -204,20 +204,26 @@ |
204 | 204 | |
205 | 205 | /* Expired RTT */ |
206 | 206 | if (!before(tp->snd_una, ca->next_seq)) { |
207 | - /* For avoiding denominator == 1. */ | |
208 | - if (ca->acked_bytes_total == 0) | |
209 | - ca->acked_bytes_total = 1; | |
207 | + u64 bytes_ecn = ca->acked_bytes_ecn; | |
208 | + u32 alpha = ca->dctcp_alpha; | |
210 | 209 | |
211 | 210 | /* alpha = (1 - g) * alpha + g * F */ |
212 | - ca->dctcp_alpha = ca->dctcp_alpha - | |
213 | - (ca->dctcp_alpha >> dctcp_shift_g) + | |
214 | - (ca->acked_bytes_ecn << (10U - dctcp_shift_g)) / | |
215 | - ca->acked_bytes_total; | |
216 | 211 | |
217 | - if (ca->dctcp_alpha > DCTCP_MAX_ALPHA) | |
218 | - /* Clamp dctcp_alpha to max. */ | |
219 | - ca->dctcp_alpha = DCTCP_MAX_ALPHA; | |
212 | + alpha -= alpha >> dctcp_shift_g; | |
213 | + if (bytes_ecn) { | |
214 | + /* If dctcp_shift_g == 1, a 32bit value would overflow | |
215 | + * after 8 Mbytes. | |
216 | + */ | |
217 | + bytes_ecn <<= (10 - dctcp_shift_g); | |
218 | + do_div(bytes_ecn, max(1U, ca->acked_bytes_total)); | |
220 | 219 | |
220 | + alpha = min(alpha + (u32)bytes_ecn, DCTCP_MAX_ALPHA); | |
221 | + } | |
222 | + /* dctcp_alpha can be read from dctcp_get_info() without | |
223 | + * synchro, so we ask compiler to not use dctcp_alpha | |
224 | + * as a temporary variable in prior operations. | |
225 | + */ | |
226 | + WRITE_ONCE(ca->dctcp_alpha, alpha); | |
221 | 227 | dctcp_reset(tp, ca); |
222 | 228 | } |
223 | 229 | } |