Open Bug 1226838 Opened 9 years ago Updated 2 years ago

Improve CandPairingFilter ice unit tests

Categories

(Core :: WebRTC: Networking, defect, P4)

defect

Tracking

()

Tracking Status
firefox45 --- affected

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
backlog: --- → webrtc/webaudio+
Rank: 35
Priority: -- → P3
Whiteboard: [good first bug]
Hi, would like to work on this bug, please assign it to me, can you also provide more info.Thank you :)
Flags: needinfo?(drno)
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)
Nils pretty much covered it. Let us know if you have any more questions.
Flags: needinfo?(docfaraday)
Mass change P3->P4 to align with new Mozilla triage process.
Priority: P3 → P4
Keywords: good-first-bug
Whiteboard: [good first bug]
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.