1013007 - Evaluate Telemetry on global STUN rate limit, and re-activate with reasonable limits

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect)

Comment 1

[Maire Reavy [:mreavy]]

Hi Lawrence, I believe you're the release driver for Fx 32.  We want to re-enable throttling of ICE connectivity checks (which we removed to collect metrics) before we go to release.  We'd love to gather some more metrics on Beta if we can.  So we'd like to re-enable the throttling part-way through Beta (roughly 3 weeks in) with 3 weeks to bake.  Are you ok with that timing?   Thanks.

Comment 2

[Lawrence Mandel [:lmandel] (use needinfo)]

I'm fine with this plan. Please setup the re-enablement of throttling based on the EARLY_BETA_OR_EARLIER flag, which the relman team will flip at the midpoint of the beta cycle.

EARLY_BETA_OR_EARLIER is defined depending on the corresponding value in build/defines.sh. This file is managed manually by the release management team, with the variable being cleared once we're past the "early beta" point in the release cycle.

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Attachment: re-enable STUN throttling in mid-beta and later

Comment 4

[Byron Campen [:bwc]]

Comment on attachment 8468084 [details] [diff] [review]
re-enable STUN throttling in mid-beta and later

Review of attachment 8468084 [details] [diff] [review]:
-----------------------------------------------------------------

We need to do this for the long-term violations as well: http://dxr.mozilla.org/mozilla-central/source/media/mtransport/nr_socket_prsock.cpp#553

We probably also want to wrap the assert, and we'll need to increase the short-term limit since we do see it being exceeded occasionally (we've never seen the long-term limit broken on telemetry). I'm pretty sure 16K would do the trick.

Comment 5

[Lawrence Mandel [:lmandel] (use needinfo)]

EARLY_BETA_OR_EARLIER is going to be preffed off in beta 6 (today). Do you have enough information to reenable throttling this week?

Comment 6

[Lawrence Mandel [:lmandel] (use needinfo)]

Assigning to Randell as he seems to be working on this.

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

Attachment: re-enable STUN throttling in mid-beta and later

Comment 8

[Byron Campen [:bwc]]

Comment on attachment 8471123 [details] [diff] [review]
re-enable STUN throttling in mid-beta and later

Review of attachment 8471123 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good with one nit.

::: media/mtransport/nr_socket_prsock.cpp
@@ +516,5 @@
>    if (nr_is_stun_request_message((UCHAR*)msg, len)) {
>      // Global rate limiting for stun requests, to mitigate the ice hammer DoS
>      // (see http://tools.ietf.org/html/draft-thomson-mmusic-ice-webrtc)
>  
>      // Tolerate rate of 8k/sec, for one second.

Nit: Update comment.

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/releases/mozilla-beta/rev/b42bbb72b7a8

Comment 10

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8471123 [details] [diff] [review]
re-enable STUN throttling in mid-beta and later

Adding beta approval to the bug for consistency.

Comment 11

[Byron Campen [:bwc]]

Aren't we going to check this into central? We need to raise the rate limit there too so we can monitor how the new limit does.

Comment 12

[Byron Campen [:bwc]]

Attachment: Double the global STUN long-term rate limit.

Comment 13

[Byron Campen [:bwc]]

Comment on attachment 8477029 [details] [diff] [review]
Double the global STUN long-term rate limit.

This landed in the wrong place.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

What about Aurora33? Presumably we need an approval request? :)

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4bd8c4b749e1

Comment 16

[Byron Campen [:bwc]]

Comment on attachment 8471123 [details] [diff] [review]
re-enable STUN throttling in mid-beta and later

Approval Request Comment

Already approved for beta, needs to go to aurora too.

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4bd8c4b749e1

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d3fe7588f7ff