Any chance to support aggressive nomination?

zyxar commented 3 years ago

Summary

I believe pion/ice is using regular nomination at present, right? With aggressive nomination, if one of the networks goes down (or the highest priority pair disconnects for some reason), re-nomination, i.e. switching to another different candidate pair without ICE-Restart would be more simple.

Motivation

Say if a mobile client changes network, (e.g., switching from WiFi to Cellular), re-nomination is faster than ICE-Restart. Any changes we can enable this mode? Add settings, flags?

Describe alternatives you've considered

ICE-Restart

Trickle-ICE is needed.

Additional context

Current HandleBindingRequest:

func (s *controlledSelector) HandleBindingRequest(m *stun.Message, local, remote Candidate) {
    useCandidate := m.Contains(stun.AttrUseCandidate)

    p := s.agent.findPair(local, remote)

    if p == nil {
        p = s.agent.addPair(local, remote)
    }

    if useCandidate {
        // https://tools.ietf.org/html/rfc8445#section-7.3.1.5

        if p.state == CandidatePairStateSucceeded {
            // If the state of this pair is Succeeded, it means that the check
            // previously sent by this pair produced a successful response and
            // generated a valid pair (Section 7.2.5.3.2).  The agent sets the
            // nominated flag value of the valid pair to true.
            if selectedPair := s.agent.getSelectedPair(); selectedPair == nil {
                s.agent.setSelectedPair(p)
            }
            s.agent.sendBindingSuccess(m, local, remote)

to aggressive:

func (s *controlledSelector) HandleBindingRequest(m *stun.Message, local, remote Candidate) {
    useCandidate := m.Contains(stun.AttrUseCandidate)

    p := s.agent.findPair(local, remote)

    if p == nil {
        p = s.agent.addPair(local, remote)
    }

    if useCandidate {
        // https://tools.ietf.org/html/rfc8445#section-7.3.1.5

        if p.state == CandidatePairStateSucceeded {
            // If the state of this pair is Succeeded, it means that the check
            // previously sent by this pair produced a successful response and
            // generated a valid pair (Section 7.2.5.3.2).  The agent sets the
            // nominated flag value of the valid pair to true.
            if !p.equal(s.agent.getSelectedPair()) {
                s.agent.setSelectedPair(p)
            }
            s.agent.sendBindingSuccess(m, local, remote)

Related (maybe) tickets: #359 #82 #271 #305

Sean-Der commented 3 years ago

Hey @zyxar

I am not against it technically! It would be nice if we could have some sort of standardization conversation.

I don't want to negatively impact other implementations if we add something that only works against one client (Pion <-> Chrome or Pion <-> Pion). It could add brittleness to the protocol, and the lack of documentation could make it hard for others.

stv0g commented 1 year ago

Aggressive nomination has been deprecated in RFC 8445:

The nomination process that was referred to as "aggressive nomination" in RFC 5245 has been deprecated in this specification.

But I also think that pion/ice currently already supports aggressive nomination. See:

stv0g commented 1 year ago

It actually says, that aggressive nomination is always activated:

https://github.com/pion/ice/blob/19ffbe7e61754a77b7e1924933db2d54b20b25e3/agent_config.go#L168

Hence, I will close this issue.

nils-ohlmeier commented 1 year ago

FYI Firefox uses aggressive nomination from the old RFC. It causes a problem with endpoints which use ice lite. That is one of the reasons aggressive nomination got deprecated in the new RFC.

Unfortunately I'm not aware of any deployment which actually implements the new ICE algorithm from the newer RFC 8445.

stv0g commented 1 year ago

Thanks @nils-ohlmeier for the insight.

I am also wondering why Pion enables aggressive nomination always without a functioning option to disable it...

mickel8 commented 1 year ago

Do you know if chrome also uses aggressive-nomination? When running this example from webrtc-samples, it looks like it does or at least the first conn check sent by the controlling side contains USE-CANDIDATE flag.

The behavior of Google Meet is slightly different

pion / ice