Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Any chance to support aggressive nomination? #362

Closed
zyxar opened this issue Apr 23, 2021 · 6 comments
Closed

Any chance to support aggressive nomination? #362

zyxar opened this issue Apr 23, 2021 · 6 comments
Labels
enhancement New feature or request

Comments

@zyxar
Copy link
Member

zyxar commented Apr 23, 2021

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
Copy link
Member

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 stv0g added the enhancement New feature or request label Apr 12, 2023
@stv0g
Copy link
Member

stv0g commented Apr 12, 2023

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
Copy link
Member

stv0g commented Apr 12, 2023

It actually says, that aggressive nomination is always activated:

// Deprecated: AcceptAggressiveNomination always enabled.

Hence, I will close this issue.

@stv0g stv0g closed this as completed Apr 12, 2023
@nils-ohlmeier
Copy link

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
Copy link
Member

stv0g commented Apr 13, 2023

Thanks @nils-ohlmeier for the insight.

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

@mickel8
Copy link

mickel8 commented Jul 17, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

5 participants