Open
Bug 1226838
Opened 9 years ago
Updated 2 years ago
Improve CandPairingFilter ice unit tests
Categories
(Core :: WebRTC: Networking, defect, P4)
Core
WebRTC: Networking
Tracking
()
NEW
Tracking | Status | |
---|---|---|
firefox45 | --- | affected |
backlog | webrtc/webaudio+ |
People
(Reporter: drno, Unassigned)
References
Details
(Keywords: good-first-bug)
The two *CandPairingFilter tests in ice_unittest from bug 1219557 don't support: - running on hosts with multiple private IPv4 addresses - running on hosts with only public IPv4 addresses - running on hosts with IPv6 addresses only
Reporter | ||
Updated•9 years ago
|
backlog: --- → webrtc/webaudio+
Rank: 35
Priority: -- → P3
Whiteboard: [good first bug]
Comment 1•8 years ago
|
||
Hi, would like to work on this bug, please assign it to me, can you also provide more info.Thank you :)
Flags: needinfo?(drno)
Reporter | ||
Comment 2•8 years ago
|
||
So the problem is this: in bug 1219557 I landed code which prevents IP addresses from different private IP address ranges to get paired, because it is highly unlikely that a pair with e.g. 10.0.0.1 and 192.168.0.1 will ever succeed. The new tests in ice_unittest basically gather the host candidates and then try to find out the private IP address range in use on that machine, e.g. 192.168.x.x. Then the test adds a remote ICE candidate from a different IP address range. Now these new tests are suppose to work if these tests get executed on a machine with exactly one private IPv4 address available. So if a developer executes the ice_unittest on a machine with: A) multiple interfaces with IP from different IP address ranges B) the network interface(s) only have public IPv4 addresses C) the network interface(s) have only IPv6 addresses these tests will fail with an error message. For A) we would need to check if at least one private IP subnet is NOT in use on the current machine and add a fake ICE candidate from that unused private subnet. For that you would need to first determine the list of IP nets in use. Then determine the list of unused IP nets. And then let the tests add an IP from the list of unused IP nets. If the list of unused IP nets is empty/zero I'm fine with still letting the test fail. For B) and C) I guess we would need to find a way to inject locally host candidates from an private IP subnet. Not sure if that is actually feasible. Byron, any idea if that is somehow possible? Now :mjf just pointed out to me that the TestSrflxCandPairingFilter test is actually broken. On machine with a single IPv4 private IP address and with access to the public Internet and DNS this test should pass. But because the test first filters the local candidates to Server Reflexive like this: SetCandidateFilter(IsSrflxCandidate); but then the function GetCandidatesPrivateIpv4Range() later searches only for host candidates like this: if (c.find("typ host") != std::string::npos) { the TestSrflxCandPairingFilter always fails, because it believes there is no single private IPv4 host address. This should be fixed as well. Let me know if you have further questions :-)
Flags: needinfo?(drno) → needinfo?(docfaraday)
Comment 3•8 years ago
|
||
Nils pretty much covered it. Let us know if you have any more questions.
Flags: needinfo?(docfaraday)
Comment 4•7 years ago
|
||
Mass change P3->P4 to align with new Mozilla triage process.
Priority: P3 → P4
Updated•4 years ago
|
Keywords: good-first-bug
Whiteboard: [good first bug]
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•