1268961 - Triggered check logic can inappropriately prevent nomination of a pair

Status: NEW

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

Description

[Byron Campen [:bwc]]

This is a problem in the ICE spec, but we may want to change our behavior here.

Scenario 1: Let's say the controller has two pairs, called "srflx" and "relay". It is sending checks for both, but neither has a response yet. Due to timing (eg; a NAT pinhole opens at the right time), relay gets a STUN response first. Because srflx is in progress, and higher priority, it is not cancelled, and is allowed to continue on to nomination. All is right with the world.

Scenario 2: Identical to scenario 1, except just before the response for relay comes in, srflx gets a STUN request. This causes srflx's STUN transaction to be cancelled, transitions srflx to waiting, and places it in the triggered check queue. Then, when the response for relay comes in, srflx is removed because it is in state waiting. srflx never makes it to nomination, because a STUN request in the other direction made it transition from in progress to waiting.

It does not seem appropriate to me that receiving a STUN request for a pair can cause that pair to not work when it would have otherwise. I see a few things that might make more sense:

1) Do not transition pairs from in progress -> waiting when putting them in the triggered check queue. Continue to cancel the STUN transaction and schedule a new one though. The pair will not be removed/cancelled if nomination happens as long as its priority is higher than the nominated pair.

2) When nomination happens, don't remove waiting pairs with a higher priority (this is something that we'd be doing for continuous nomination anyhow).

3) When nomination happens, don't remove stuff from the triggered check queue.

Comment 1

[Nils Ohlmeier [:drno]]

I guess the answer actually depends on if you are running in aggressive mode or not. 

Aggressive mode:
I would argue that in aggressive mode there is actually no point in trying to empty/cancel the trigger check queue. Everything in the triggered check queue has proven to at least let traffic pass through in on direction. So each of the pairs in the triggered check queue still has the potential to turn out to be better then what has been nominated so far. Thus in aggressive mode you should not touch the triggered checked queue when nominating and just continuously nominate if you find a better pair.

Normal mode:
As you chosen criteria has been reached when you nominate you want to put and end to more viable pairs coming up. Thus it makes sense to try to cancel as many pairs as possible. I guess that is what the spec says/describes.

Comment 2

[Bulk Bug Changes for mreavy's org]

Mass change P2->P3 to align with new Mozilla triage process.