limit number of ice servers to 32 #2679
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
limits the number of ICE server entries to 64. This value is arbitrarily chosen, more than a few servers do not make sense in practice. As candidates from each server need a unique priority, the number space is limited. See https://bugs.chromium.org/p/webrtc/issues/detail?id=13195 and https://mailarchive.ietf.org/arch/msg/mmusic/isyEwdluHuTl5mMnVUDrmawBnIU/ for background information.
|
Firefox starts emitting console warnings at 5. Should we go with a lower value? |
|
I guess the question is first which value it will be. If it's reasonably high, throwing might be acceptable. 🙂 |
|
I expect quite a few attempts to use a list of "free stun and turn servers" (the list with the credentials from https://www.html5rocks.com/en/tutorials/webrtc/infrastructure/ which expired eight years ago). 64 is reasonably high that it should not be an issue, we could probably do an UMA histogram in chrome to get an impression on what people actually use. |
|
I think we can get histograms of the current usage from Chrome but that will take several release cycles. |
|
Preliminary results show that there are some people using 14, but very very few above that. |
|
32 and ignoring seems more web compatible |
alvestrand
reviewed
Nov 15, 2021
aarongable
pushed a commit
to chromium/chromium
that referenced
this pull request
Oct 10, 2022
after they have been informing the decision to limit to 32 servers in w3c/webrtc-pc#2679 WebRTC removal: https://webrtc-review.googlesource.com/c/src/+/277804 BUG=chromium:1360224 Change-Id: I00466d5ae483abbc353605e90f542c4f465f565b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3939467 Reviewed-by: Johannes Kron <[email protected]> Commit-Queue: Philipp Hancke <[email protected]> Cr-Commit-Position: refs/heads/main@{#1056861}
ibaoger
pushed a commit
to ibaoger/webrtc
that referenced
this pull request
Oct 10, 2022
which have shown that 32 is a reasonably safe limit and informed w3c/webrtc-pc#2679 BUG=webrtc:13265,chromium:1360224 Change-Id: I63155653862e4c12720b8655c51ed5f3bdc742f0 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/277804 Reviewed-by: Johannes Kron <[email protected]> Reviewed-by: Harald Alvestrand <[email protected]> Commit-Queue: Philipp Hancke <[email protected]> Cr-Commit-Position: refs/heads/main@{#38339}
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Feb 15, 2023
Upstream commit: https://webrtc.googlesource.com/src/+/28fb04de62233d285d4b36a2f3fc2cbf0d16733b metrics: remove WebRTC.PeerConnection.IceServers.* which have shown that 32 is a reasonably safe limit and informed w3c/webrtc-pc#2679 BUG=webrtc:13265,chromium:1360224 Change-Id: I63155653862e4c12720b8655c51ed5f3bdc742f0 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/277804 Reviewed-by: Johannes Kron <[email protected]> Reviewed-by: Harald Alvestrand <[email protected]> Commit-Queue: Philipp Hancke <[email protected]> Cr-Commit-Position: refs/heads/main@{#38339}
jamienicol
pushed a commit
to jamienicol/gecko
that referenced
this pull request
Feb 20, 2023
Upstream commit: https://webrtc.googlesource.com/src/+/28fb04de62233d285d4b36a2f3fc2cbf0d16733b metrics: remove WebRTC.PeerConnection.IceServers.* which have shown that 32 is a reasonably safe limit and informed w3c/webrtc-pc#2679 BUG=webrtc:13265,chromium:1360224 Change-Id: I63155653862e4c12720b8655c51ed5f3bdc742f0 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/277804 Reviewed-by: Johannes Kron <[email protected]> Reviewed-by: Harald Alvestrand <[email protected]> Commit-Queue: Philipp Hancke <[email protected]> Cr-Commit-Position: refs/heads/main@{#38339}
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Apr 11, 2023
Upstream commit: https://webrtc.googlesource.com/src/+/41a83571707a574bbe3da32f564b963b8092d59b Limit number of TURN servers to 32 Limit the number of TURN servers to 32 in order to allow the prioritization to assume a fixed offset for (de)prioritizing candidates. See w3c/webrtc-pc#2679 for discussion including some data on current usage. Guarded by WebRTC-LimitTurnServers which is used as a killswitch. BUG=webrtc:13195 Change-Id: Ib12726af426ae4238aa7eb6aa062c71af52d495f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/285340 Reviewed-by: Harald Alvestrand <[email protected]> Commit-Queue: Philipp Hancke <[email protected]> Reviewed-by: Florent Castelli <[email protected]> Cr-Commit-Position: refs/heads/main@{#38767}
|
Updated title to match limit in the PR which is 32. |
23 tasks
daemory
pushed a commit
to dangxiwang/webrtc
that referenced
this pull request
Apr 19, 2024
which have shown that 32 is a reasonably safe limit and informed w3c/webrtc-pc#2679 BUG=webrtc:13265,chromium:1360224 Change-Id: I63155653862e4c12720b8655c51ed5f3bdc742f0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
limits the number of ICE server entries to 64.
This value is arbitrarily chosen, more than a few servers
do not make sense in practice. As candidates from each
server need a unique priority, the number space is limited.
See
https://bugs.chromium.org/p/webrtc/issues/detail?id=13195
and
https://mailarchive.ietf.org/arch/msg/mmusic/isyEwdluHuTl5mMnVUDrmawBnIU/
for background information.
We need to decide whether to throw an error (and if yes, which one) or silently ignore as it is done for certificates
Preview | Diff