1208176 - ice_unittests is broken, again

Status: RESOLVED FIXED

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

Description

[Byron Campen [:bwc]]

Tested on my linux box. Seems to be related to prflx showing up where we did not expect.

[  FAILED  ] 9 tests, listed below:
[  FAILED  ] IceConnectTest.TestConnect
[  FAILED  ] IceConnectTest.TestConnectBothControllingP1Wins
[  FAILED  ] IceConnectTest.TestConnectBothControllingP2Wins
[  FAILED  ] IceConnectTest.TestConnectIceLiteOfferer
[  FAILED  ] IceConnectTest.TestConnectTwoComponents
[  FAILED  ] IceConnectTest.TestConnectTwoComponentsDisableSecond
[  FAILED  ] IceConnectTest.TestConnectAutoPrioritize
[  FAILED  ] IceConnectTest.TestSendReceive
[  FAILED  ] IceConnectTest.TestPollCandPairsAfterConnect

Comment 1

[Nils Ohlmeier [:drno]]

Similar problems occasionally show up for TCP ice unit tests, which I though so far were limited to TCP.

Comment 2

[Byron Campen [:bwc]]

This seems to only be a problem on my linux box, due to some extra virbr interfaces, and linux's tendency to play it fast and loose when sending traffic to yourself. The patch from bug 1186590 perturbed things just enough to expose the problem, since it caused the virbr interface to be prioritized over the real one.

One thing that I could do is make the test-cases accept prflx as a general rule. However, this might mask problems with trickle, since one side could fail to trickle anything and stuff would still work. I would also have to add a one-sided trickle test (which I suppose we should have anyway).

Another option is to add my weird "real" interface name to the priority list, which probably makes sense to do anyway, although it is only a band-aid.

Comment 3

[Byron Campen [:bwc]]

Attachment: MozReview Request: Bug 1208176 - Part 1: Add a couple of interface names. r=drno

Bug 1208176 - Part 1: Add a couple of interface names.

Comment 4

[Byron Campen [:bwc]]

Attachment: MozReview Request: Bug 1208176 - Part 2: Add a one-sided trickle test case to ice_unittest. r=drno

Bug 1208176 - Part 2: Add a one-sided trickle test case to ice_unittest.

Comment 5

[Byron Campen [:bwc]]

Attachment: MozReview Request: Bug 1208176 - Part 3: Be forgiving when we see prflx instead of host candidates in ice_unittest. r=drno

Bug 1208176 - Part 3: Be forgiving when we see prflx instead of host candidates in ice_unittest.

Comment 6

[Nils Ohlmeier [:drno]]

Comment on attachment 8670456 [details]
MozReview Request: Bug 1208176 - Part 1: Add a couple of interface names. r=drno

https://reviewboard.mozilla.org/r/21375/#review19279

Comment 7

[Nils Ohlmeier [:drno]]

Comment on attachment 8670457 [details]
MozReview Request: Bug 1208176 - Part 2: Add a one-sided trickle test case to ice_unittest. r=drno

https://reviewboard.mozilla.org/r/21377/#review19283

::: media/mtransport/test/ice_unittest.cpp:2513
(Diff revision 1)
> +TEST_F(IceConnectTest, OneSidedTrickle) {
> +  AddStream("first", 1);
> +  ASSERT_TRUE(Gather());
> +  ConnectTrickle();
> +  RealisticTrickleDelay(p1_->ControlTrickle(0));
> +  DropTrickleCandidates(p2_->ControlTrickle(0));
> +  ASSERT_TRUE_WAIT(p1_->ice_complete(), 1000);
> +  ASSERT_TRUE_WAIT(p2_->ice_complete(), 1000);
> +}

Just to be safe: how about the reverse p1 dropping everything?

Comment 8

[Nils Ohlmeier [:drno]]

Comment on attachment 8670458 [details]
MozReview Request: Bug 1208176 - Part 3: Be forgiving when we see prflx instead of host candidates in ice_unittest. r=drno

https://reviewboard.mozilla.org/r/21379/#review19285

Comment 9

[Byron Campen [:bwc]]

Comment on attachment 8670456 [details]
MozReview Request: Bug 1208176 - Part 1: Add a couple of interface names. r=drno

Bug 1208176 - Part 1: Add a couple of interface names. r=drno

Comment 10

[Byron Campen [:bwc]]

Comment on attachment 8670457 [details]
MozReview Request: Bug 1208176 - Part 2: Add a one-sided trickle test case to ice_unittest. r=drno

Bug 1208176 - Part 2: Add a one-sided trickle test case to ice_unittest. r=drno

Comment 11

[Byron Campen [:bwc]]

Comment on attachment 8670458 [details]
MozReview Request: Bug 1208176 - Part 3: Be forgiving when we see prflx instead of host candidates in ice_unittest. r=drno

Bug 1208176 - Part 3: Be forgiving when we see prflx instead of host candidates in ice_unittest. r=drno

Comment 12

[Byron Campen [:bwc]]

Check back on try.

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ae9a75f27ba4
https://hg.mozilla.org/integration/mozilla-inbound/rev/a2aa1b936dcb
https://hg.mozilla.org/integration/mozilla-inbound/rev/9effb6eb0254

Comment 14

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

https://hg.mozilla.org/mozilla-central/rev/ae9a75f27ba4
https://hg.mozilla.org/mozilla-central/rev/a2aa1b936dcb
https://hg.mozilla.org/mozilla-central/rev/9effb6eb0254