From 8217b53fa659289abb1f50b7e228303b1011232b Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 6 Oct 2025 15:22:24 +0000 Subject: [PATCH 1/7] Update module github.com/pion/transport/v3 to v3.0.8 Generated by renovateBot --- go.mod | 4 ++-- go.sum | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 8c11515..2312bbc 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/pion/mdns/v2 v2.0.7 github.com/pion/randutil v0.1.0 github.com/pion/stun/v3 v3.0.0 - github.com/pion/transport/v3 v3.0.7 + github.com/pion/transport/v3 v3.0.8 github.com/pion/turn/v4 v4.1.1 github.com/stretchr/testify v1.11.1 golang.org/x/net v0.34.0 @@ -19,7 +19,7 @@ require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/kr/pretty v0.1.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/wlynxg/anet v0.0.3 // indirect + github.com/wlynxg/anet v0.0.5 // indirect golang.org/x/crypto v0.32.0 // indirect golang.org/x/sys v0.30.0 // indirect gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect diff --git a/go.sum b/go.sum index dea8c91..1c66a07 100644 --- a/go.sum +++ b/go.sum @@ -17,16 +17,16 @@ github.com/pion/randutil v0.1.0 h1:CFG1UdESneORglEsnimhUjf33Rwjubwj6xfiOXBa3mA= github.com/pion/randutil v0.1.0/go.mod h1:XcJrSMMbbMRhASFVOlj/5hQial/Y8oH/HVo7TBZq+j8= github.com/pion/stun/v3 v3.0.0 h1:4h1gwhWLWuZWOJIJR9s2ferRO+W3zA/b6ijOI6mKzUw= github.com/pion/stun/v3 v3.0.0/go.mod h1:HvCN8txt8mwi4FBvS3EmDghW6aQJ24T+y+1TKjB5jyU= -github.com/pion/transport/v3 v3.0.7 h1:iRbMH05BzSNwhILHoBoAPxoB9xQgOaJk+591KC9P1o0= -github.com/pion/transport/v3 v3.0.7/go.mod h1:YleKiTZ4vqNxVwh77Z0zytYi7rXHl7j6uPLGhhz9rwo= +github.com/pion/transport/v3 v3.0.8 h1:oI3myyYnTKUSTthu/NZZ8eu2I5sHbxbUNNFW62olaYc= +github.com/pion/transport/v3 v3.0.8/go.mod h1:+c2eewC5WJQHiAA46fkMMzoYZSuGzA/7E2FPrOYHctQ= github.com/pion/turn/v4 v4.1.1 h1:9UnY2HB99tpDyz3cVVZguSxcqkJ1DsTSZ+8TGruh4fc= github.com/pion/turn/v4 v4.1.1/go.mod h1:2123tHk1O++vmjI5VSD0awT50NywDAq5A2NNNU4Jjs8= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= -github.com/wlynxg/anet v0.0.3 h1:PvR53psxFXstc12jelG6f1Lv4MWqE0tI76/hHGjh9rg= -github.com/wlynxg/anet v0.0.3/go.mod h1:eay5PRQr7fIVAMbTbchTnO9gG65Hg/uYGdc7mguHxoA= +github.com/wlynxg/anet v0.0.5 h1:J3VJGi1gvo0JwZ/P1/Yc/8p63SoW98B5dHkYDmpgvvU= +github.com/wlynxg/anet v0.0.5/go.mod h1:eay5PRQr7fIVAMbTbchTnO9gG65Hg/uYGdc7mguHxoA= golang.org/x/crypto v0.32.0 h1:euUpcYgM8WcP71gNpTqQCn6rC2t6ULUPiOzfWaXVVfc= golang.org/x/crypto v0.32.0/go.mod h1:ZnnJkOaASj8g0AjIduWNlq2NRxL0PlBrbKVyZ6V/Ugc= golang.org/x/net v0.34.0 h1:Mb7Mrk043xzHgnRM88suvJFwzVrRfHEHJEl5/71CKw0= From 43dce03277024686905c0bf27a2b5e9dec074811 Mon Sep 17 00:00:00 2001 From: Pion <59523206+pionbot@users.noreply.github.com> Date: Sat, 11 Oct 2025 19:42:14 +0000 Subject: [PATCH 2/7] Update CI configs to v0.11.31 Update lint scripts and CI configs. --- .github/workflows/fuzz.yaml | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 .github/workflows/fuzz.yaml diff --git a/.github/workflows/fuzz.yaml b/.github/workflows/fuzz.yaml new file mode 100644 index 0000000..2f888ad --- /dev/null +++ b/.github/workflows/fuzz.yaml @@ -0,0 +1,27 @@ +# +# DO NOT EDIT THIS FILE +# +# It is automatically copied from https://github.com/pion/.goassets repository. +# If this repository should have package specific CI config, +# remove the repository name from .goassets/.github/workflows/assets-sync.yml. +# +# If you want to update the shared CI config, send a PR to +# https://github.com/pion/.goassets instead of this repository. +# +# SPDX-FileCopyrightText: 2023 The Pion community +# SPDX-License-Identifier: MIT + +name: Fuzz +on: + push: + branches: + - master + schedule: + - cron: "0 */8 * * *" + +jobs: + fuzz: + uses: pion/.goassets/.github/workflows/fuzz.reusable.yml@master + with: + go-version: "1.25" # auto-update/latest-go-version + fuzz-time: "60s" From b5142e4d36009aabcc193a60e23063c9f240fb4a Mon Sep 17 00:00:00 2001 From: Nils Ohlmeier Date: Mon, 20 Oct 2025 16:01:44 -0600 Subject: [PATCH 3/7] Deduplicated error messages for easier debugging --- agent.go | 6 +++--- selection.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/agent.go b/agent.go index 8b6e745..a300f98 100644 --- a/agent.go +++ b/agent.go @@ -1157,7 +1157,7 @@ func (a *Agent) handleInbound(msg *stun.Message, local Candidate, remote net.Add if msg.Type.Class == stun.ClassSuccessResponse { //nolint:nestif if err := stun.MessageIntegrity([]byte(a.remotePwd)).Check(msg); err != nil { - a.log.Warnf("Discard message from (%s), %v", remote, err) + a.log.Warnf("Discard success response with broken integrity from (%s), %v", remote, err) return } @@ -1178,11 +1178,11 @@ func (a *Agent) handleInbound(msg *stun.Message, local Candidate, remote net.Add ) if err := stunx.AssertUsername(msg, a.localUfrag+":"+a.remoteUfrag); err != nil { - a.log.Warnf("Discard message from (%s), %v", remote, err) + a.log.Warnf("Discard request with wrong username from (%s), %v", remote, err) return } else if err := stun.MessageIntegrity([]byte(a.localPwd)).Check(msg); err != nil { - a.log.Warnf("Discard message from (%s), %v", remote, err) + a.log.Warnf("Discard request with broken integrity from (%s), %v", remote, err) return } diff --git a/selection.go b/selection.go index bce5bc9..d82a04c 100644 --- a/selection.go +++ b/selection.go @@ -132,7 +132,7 @@ func (s *controllingSelector) HandleBindingRequest(message *stun.Message, local, func (s *controllingSelector) HandleSuccessResponse(m *stun.Message, local, remote Candidate, remoteAddr net.Addr) { ok, pendingRequest, rtt := s.agent.handleInboundBindingSuccess(m.TransactionID) if !ok { - s.log.Warnf("Discard message from (%s), unknown TransactionID 0x%x", remote, m.TransactionID) + s.log.Warnf("Discard success response from (%s), unknown TransactionID 0x%x", remote, m.TransactionID) return } From 163dca56e47588ec7223bbb3d581afdd946f52c8 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Sat, 25 Oct 2025 18:33:44 +0000 Subject: [PATCH 4/7] Update module github.com/pion/stun/v3 to v3.0.1 Generated by renovateBot --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 2312bbc..866920f 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/pion/logging v0.2.4 github.com/pion/mdns/v2 v2.0.7 github.com/pion/randutil v0.1.0 - github.com/pion/stun/v3 v3.0.0 + github.com/pion/stun/v3 v3.0.1 github.com/pion/transport/v3 v3.0.8 github.com/pion/turn/v4 v4.1.1 github.com/stretchr/testify v1.11.1 diff --git a/go.sum b/go.sum index 1c66a07..78a3d0e 100644 --- a/go.sum +++ b/go.sum @@ -15,8 +15,8 @@ github.com/pion/mdns/v2 v2.0.7 h1:c9kM8ewCgjslaAmicYMFQIde2H9/lrZpjBkN8VwoVtM= github.com/pion/mdns/v2 v2.0.7/go.mod h1:vAdSYNAT0Jy3Ru0zl2YiW3Rm/fJCwIeM0nToenfOJKA= github.com/pion/randutil v0.1.0 h1:CFG1UdESneORglEsnimhUjf33Rwjubwj6xfiOXBa3mA= github.com/pion/randutil v0.1.0/go.mod h1:XcJrSMMbbMRhASFVOlj/5hQial/Y8oH/HVo7TBZq+j8= -github.com/pion/stun/v3 v3.0.0 h1:4h1gwhWLWuZWOJIJR9s2ferRO+W3zA/b6ijOI6mKzUw= -github.com/pion/stun/v3 v3.0.0/go.mod h1:HvCN8txt8mwi4FBvS3EmDghW6aQJ24T+y+1TKjB5jyU= +github.com/pion/stun/v3 v3.0.1 h1:jx1uUq6BdPihF0yF33Jj2mh+C9p0atY94IkdnW174kA= +github.com/pion/stun/v3 v3.0.1/go.mod h1:RHnvlKFg+qHgoKIqtQWMOJF52wsImCAf/Jh5GjX+4Tw= github.com/pion/transport/v3 v3.0.8 h1:oI3myyYnTKUSTthu/NZZ8eu2I5sHbxbUNNFW62olaYc= github.com/pion/transport/v3 v3.0.8/go.mod h1:+c2eewC5WJQHiAA46fkMMzoYZSuGzA/7E2FPrOYHctQ= github.com/pion/turn/v4 v4.1.1 h1:9UnY2HB99tpDyz3cVVZguSxcqkJ1DsTSZ+8TGruh4fc= From d1491ba5ca6b08efeca229710f70ea1c3f322804 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Sat, 25 Oct 2025 23:18:36 -0400 Subject: [PATCH 5/7] Fix race condition in setSelectedPair --- agent.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/agent.go b/agent.go index a300f98..8731395 100644 --- a/agent.go +++ b/agent.go @@ -504,13 +504,14 @@ func (a *Agent) setSelectedPair(pair *CandidatePair) { a.selectedPair.Store(pair) a.log.Tracef("Set selected candidate pair: %s", pair) - a.updateConnectionState(ConnectionStateConnected) + // Signal connected: notify any Connect() calls waiting on onConnected + a.onConnectedOnce.Do(func() { close(a.onConnected) }) - // Notify when the selected pair changes - a.selectedCandidatePairNotifier.EnqueueSelectedCandidatePair(pair) + // Update connection state to Connected and notify state change handlers + a.updateConnectionState(ConnectionStateConnected) - // Signal connected - a.onConnectedOnce.Do(func() { close(a.onConnected) }) + // Notify when the selected candidate pair changes + a.selectedCandidatePairNotifier.EnqueueSelectedCandidatePair(pair) } func (a *Agent) pingAllCandidates() { From 649e13aab501977b576e01c3a6af7ccebdc0d634 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Sat, 25 Oct 2025 20:24:44 -0700 Subject: [PATCH 6/7] Remove unnecessary comments in agent.go --- agent.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/agent.go b/agent.go index 8731395..fb5ea5c 100644 --- a/agent.go +++ b/agent.go @@ -504,14 +504,14 @@ func (a *Agent) setSelectedPair(pair *CandidatePair) { a.selectedPair.Store(pair) a.log.Tracef("Set selected candidate pair: %s", pair) - // Signal connected: notify any Connect() calls waiting on onConnected - a.onConnectedOnce.Do(func() { close(a.onConnected) }) + // Signal connected: notify any Connect() calls waiting on onConnected + a.onConnectedOnce.Do(func() { close(a.onConnected) }) - // Update connection state to Connected and notify state change handlers - a.updateConnectionState(ConnectionStateConnected) + // Update connection state to Connected and notify state change handlers + a.updateConnectionState(ConnectionStateConnected) - // Notify when the selected candidate pair changes - a.selectedCandidatePairNotifier.EnqueueSelectedCandidatePair(pair) + // Notify when the selected candidate pair changes + a.selectedCandidatePairNotifier.EnqueueSelectedCandidatePair(pair) } func (a *Agent) pingAllCandidates() { From f56f52ca84162eea4c80c603ef811c6e0d423655 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Tue, 21 Oct 2025 07:55:08 -0400 Subject: [PATCH 7/7] Add automatic renomination --- agent.go | 247 +++++- agent_options.go | 68 ++ agent_options_test.go | 79 ++ agent_test.go | 194 ++++- examples/automatic-renomination/README.md | 555 ++++++++++++ examples/automatic-renomination/main.go | 311 +++++++ selection.go | 118 ++- selection_test.go | 985 +++++++++++++++++++++- 8 files changed, 2525 insertions(+), 32 deletions(-) create mode 100644 examples/automatic-renomination/README.md create mode 100644 examples/automatic-renomination/main.go diff --git a/agent.go b/agent.go index fb5ea5c..391fde8 100644 --- a/agent.go +++ b/agent.go @@ -29,10 +29,11 @@ import ( ) type bindingRequest struct { - timestamp time.Time - transactionID [stun.TransactionIDSize]byte - destination net.Addr - isUseCandidate bool + timestamp time.Time + transactionID [stun.TransactionIDSize]byte + destination net.Addr + isUseCandidate bool + nominationValue *uint32 // Tracks nomination value for renomination requests } // Agent represents the ICE agent. @@ -159,6 +160,11 @@ type Agent struct { continualGatheringPolicy ContinualGatheringPolicy networkMonitorInterval time.Duration lastKnownInterfaces map[string]netip.Addr // map[iface+ip] for deduplication + + // Automatic renomination + automaticRenomination bool + renominationInterval time.Duration + lastRenominationTime time.Time } // NewAgent creates a new Agent. @@ -171,27 +177,40 @@ func NewAgentWithOptions(opts ...AgentOption) (*Agent, error) { return newAgentWithConfig(&AgentConfig{}, opts...) } +// setupMDNSConfig validates and returns mDNS configuration. +func setupMDNSConfig(config *AgentConfig) (string, MulticastDNSMode, error) { + mDNSName := config.MulticastDNSHostName + if mDNSName == "" { + var err error + if mDNSName, err = generateMulticastDNSName(); err != nil { + return "", 0, err + } + } + + if !strings.HasSuffix(mDNSName, ".local") || len(strings.Split(mDNSName, ".")) != 2 { + return "", 0, ErrInvalidMulticastDNSHostName + } + + mDNSMode := config.MulticastDNSMode + if mDNSMode == 0 { + mDNSMode = MulticastDNSModeQueryOnly + } + + return mDNSName, mDNSMode, nil +} + // newAgentWithConfig is the internal function that creates an agent with config and options. +// +//nolint:gocognit func newAgentWithConfig(config *AgentConfig, opts ...AgentOption) (*Agent, error) { //nolint:cyclop var err error if config.PortMax < config.PortMin { return nil, ErrPort } - mDNSName := config.MulticastDNSHostName - if mDNSName == "" { - if mDNSName, err = generateMulticastDNSName(); err != nil { - return nil, err - } - } - - if !strings.HasSuffix(mDNSName, ".local") || len(strings.Split(mDNSName, ".")) != 2 { - return nil, ErrInvalidMulticastDNSHostName - } - - mDNSMode := config.MulticastDNSMode - if mDNSMode == 0 { - mDNSMode = MulticastDNSModeQueryOnly + mDNSName, mDNSMode, err := setupMDNSConfig(config) + if err != nil { + return nil, err } loggerFactory := config.LoggerFactory @@ -253,6 +272,9 @@ func newAgentWithConfig(config *AgentConfig, opts ...AgentOption) (*Agent, error continualGatheringPolicy: GatherOnce, // Default to GatherOnce networkMonitorInterval: 2 * time.Second, lastKnownInterfaces: make(map[string]netip.Addr), + + automaticRenomination: false, + renominationInterval: 3 * time.Second, // Default matching libwebrtc } agent.connectionStateNotifier = &handlerNotifier{ @@ -538,6 +560,36 @@ func (a *Agent) pingAllCandidates() { } } +// keepAliveCandidatesForRenomination pings all candidate pairs to keep them tested +// and ready for automatic renomination. Unlike pingAllCandidates, this: +// - Pings pairs in succeeded state to keep RTT measurements fresh +// - Ignores maxBindingRequests limit (we want to keep testing alternate paths) +// - Only pings pairs that are not failed. +func (a *Agent) keepAliveCandidatesForRenomination() { + a.log.Trace("Keep alive candidates for automatic renomination") + + if len(a.checklist) == 0 { + return + } + + for _, pair := range a.checklist { + switch pair.state { + case CandidatePairStateFailed: + // Skip failed pairs + continue + case CandidatePairStateWaiting: + // Transition waiting pairs to in-progress + pair.state = CandidatePairStateInProgress + case CandidatePairStateInProgress, CandidatePairStateSucceeded: + // Continue pinging in-progress and succeeded pairs + } + + // Ping all non-failed pairs (including succeeded ones) + // to keep RTT measurements fresh for renomination decisions + a.getSelector().PingCandidate(pair.Local, pair.Remote) + } +} + func (a *Agent) getBestAvailableCandidatePair() *CandidatePair { var best *CandidatePair for _, p := range a.checklist { @@ -1026,12 +1078,20 @@ func (a *Agent) findRemoteCandidate(networkType NetworkType, addr net.Addr) Cand func (a *Agent) sendBindingRequest(msg *stun.Message, local, remote Candidate) { a.log.Tracef("Ping STUN from %s to %s", local, remote) + // Extract nomination value if present + var nominationValue *uint32 + var nomination NominationAttribute + if err := nomination.GetFromWithType(msg, a.nominationAttribute); err == nil { + nominationValue = &nomination.Value + } + a.invalidatePendingBindingRequests(time.Now()) a.pendingBindingRequests = append(a.pendingBindingRequests, bindingRequest{ - timestamp: time.Now(), - transactionID: msg.TransactionID, - destination: remote.addr(), - isUseCandidate: msg.Contains(stun.AttrUseCandidate), + timestamp: time.Now(), + transactionID: msg.TransactionID, + destination: remote.addr(), + isUseCandidate: msg.Contains(stun.AttrUseCandidate), + nominationValue: nominationValue, }) if pair := a.findPair(local, remote); pair != nil { @@ -1478,3 +1538,146 @@ func (a *Agent) sendNominationRequest(pair *CandidatePair, nominationValue uint3 return nil } + +// evaluateCandidatePairQuality calculates a quality score for a candidate pair. +// Higher scores indicate better quality. The score considers: +// - Candidate types (host > srflx > relay) +// - RTT (lower is better) +// - Connection stability. +func (a *Agent) evaluateCandidatePairQuality(pair *CandidatePair) float64 { //nolint:cyclop + if pair == nil || pair.state != CandidatePairStateSucceeded { + return 0 + } + + score := float64(0) + + // Type preference scoring (host=100, srflx=50, prflx=30, relay=10) + localTypeScore := float64(0) + switch pair.Local.Type() { + case CandidateTypeHost: + localTypeScore = 100 + case CandidateTypeServerReflexive: + localTypeScore = 50 + case CandidateTypePeerReflexive: + localTypeScore = 30 + case CandidateTypeRelay: + localTypeScore = 10 + case CandidateTypeUnspecified: + localTypeScore = 0 + } + + remoteTypeScore := float64(0) + switch pair.Remote.Type() { + case CandidateTypeHost: + remoteTypeScore = 100 + case CandidateTypeServerReflexive: + remoteTypeScore = 50 + case CandidateTypePeerReflexive: + remoteTypeScore = 30 + case CandidateTypeRelay: + remoteTypeScore = 10 + case CandidateTypeUnspecified: + remoteTypeScore = 0 + } + + // Combined type score (average of local and remote) + score += (localTypeScore + remoteTypeScore) / 2 + + // RTT scoring (convert to penalty, lower RTT = higher score) + // Use current RTT if available, otherwise assume high latency + rtt := pair.CurrentRoundTripTime() + if rtt > 0 { + // Convert RTT to Duration for cleaner calculation + rttDuration := time.Duration(rtt * float64(time.Second)) + rttMs := float64(rttDuration / time.Millisecond) + if rttMs < 1 { + rttMs = 1 // Minimum 1ms to avoid log(0) + } + // Subtract RTT penalty (logarithmic to reduce impact of very high RTTs) + score -= math.Log10(rttMs) * 10 + } else { + // No RTT data available, apply moderate penalty + score -= 30 + } + + // Boost score if pair has been stable (received responses recently) + if pair.ResponsesReceived() > 0 { + lastResponse := pair.LastResponseReceivedAt() + if !lastResponse.IsZero() && time.Since(lastResponse) < 5*time.Second { + score += 20 // Stability bonus + } + } + + return score +} + +// shouldRenominate determines if automatic renomination should occur. +// It compares the current selected pair with a candidate pair and decides +// if switching would provide significant benefit. +func (a *Agent) shouldRenominate(current, candidate *CandidatePair) bool { //nolint:cyclop + if current == nil || candidate == nil || current.equal(candidate) || candidate.state != CandidatePairStateSucceeded { + return false + } + + // Type-based switching (always prefer direct over relay) + currentIsRelay := current.Local.Type() == CandidateTypeRelay || + current.Remote.Type() == CandidateTypeRelay + candidateIsDirect := candidate.Local.Type() == CandidateTypeHost && + candidate.Remote.Type() == CandidateTypeHost + + if currentIsRelay && candidateIsDirect { + a.log.Debugf("Should renominate: relay -> direct connection available") + + return true + } + + // RTT-based switching (must improve by at least 10ms) + currentRTT := current.CurrentRoundTripTime() + candidateRTT := candidate.CurrentRoundTripTime() + + // Only compare RTT if both values are valid + if currentRTT > 0 && candidateRTT > 0 { + currentRTTDuration := time.Duration(currentRTT * float64(time.Second)) + candidateRTTDuration := time.Duration(candidateRTT * float64(time.Second)) + rttImprovement := currentRTTDuration - candidateRTTDuration + + if rttImprovement > 10*time.Millisecond { + a.log.Debugf("Should renominate: RTT improvement of %v", rttImprovement) + + return true + } + } + + // Quality score comparison (must improve by at least 15%) + currentScore := a.evaluateCandidatePairQuality(current) + candidateScore := a.evaluateCandidatePairQuality(candidate) + + if candidateScore > currentScore*1.15 { + a.log.Debugf("Should renominate: quality score improved from %.2f to %.2f", + currentScore, candidateScore) + + return true + } + + return false +} + +// findBestCandidatePair finds the best available candidate pair based on quality assessment. +func (a *Agent) findBestCandidatePair() *CandidatePair { + var best *CandidatePair + bestScore := float64(-math.MaxFloat64) + + for _, pair := range a.checklist { + if pair.state != CandidatePairStateSucceeded { + continue + } + + score := a.evaluateCandidatePairQuality(pair) + if score > bestScore { + bestScore = score + best = pair + } + } + + return best +} diff --git a/agent_options.go b/agent_options.go index edf9332..37e4ec3 100644 --- a/agent_options.go +++ b/agent_options.go @@ -7,6 +7,7 @@ import ( "sync/atomic" "time" + "github.com/pion/logging" "github.com/pion/stun/v3" ) @@ -223,3 +224,70 @@ func WithCandidateTypes(candidateTypes []CandidateType) AgentOption { return nil } } + +// WithAutomaticRenomination enables automatic renomination of candidate pairs +// when better pairs become available after initial connection establishment. +// This feature requires renomination to be enabled and both agents to support it. +// +// When enabled, the controlling agent will periodically evaluate candidate pairs +// and renominate if a significantly better pair is found (e.g., switching from +// relay to direct connection, or when RTT improves significantly). +// +// The interval parameter specifies the minimum time to wait after connection +// before considering automatic renomination. If set to 0, it defaults to 3 seconds. +// +// Example: +// +// agent, err := NewAgentWithOptions( +// WithRenomination(DefaultNominationValueGenerator()), +// WithAutomaticRenomination(3*time.Second), +// ) +func WithAutomaticRenomination(interval time.Duration) AgentOption { + return func(a *Agent) error { + a.automaticRenomination = true + if interval > 0 { + a.renominationInterval = interval + } + // Note: renomination must be enabled separately via WithRenomination + return nil + } +} + +// WithInterfaceFilter sets a filter function to whitelist or blacklist network interfaces +// for ICE candidate gathering. +// +// The filter function receives the interface name and should return true to keep the interface, +// or false to exclude it. +// +// Example: +// +// // Only use interfaces starting with "eth" +// agent, err := NewAgentWithOptions( +// WithInterfaceFilter(func(interfaceName string) bool { +// return len(interfaceName) >= 3 && interfaceName[:3] == "eth" +// }), +// ) +func WithInterfaceFilter(filter func(string) bool) AgentOption { + return func(a *Agent) error { + a.interfaceFilter = filter + + return nil + } +} + +// WithLoggerFactory sets the logger factory for the agent. +// +// Example: +// +// import "github.com/pion/logging" +// +// loggerFactory := logging.NewDefaultLoggerFactory() +// loggerFactory.DefaultLogLevel = logging.LogLevelDebug +// agent, err := NewAgentWithOptions(WithLoggerFactory(loggerFactory)) +func WithLoggerFactory(loggerFactory logging.LoggerFactory) AgentOption { + return func(a *Agent) error { + a.log = loggerFactory.NewLogger("ice") + + return nil + } +} diff --git a/agent_options_test.go b/agent_options_test.go index 27cbaad..017a56c 100644 --- a/agent_options_test.go +++ b/agent_options_test.go @@ -6,6 +6,7 @@ package ice import ( "testing" + "github.com/pion/logging" "github.com/pion/stun/v3" "github.com/stretchr/testify/assert" ) @@ -296,3 +297,81 @@ func TestMultipleConfigOptions(t *testing.T) { } }) } + +func TestWithInterfaceFilter(t *testing.T) { + t.Run("sets interface filter", func(t *testing.T) { + filter := func(interfaceName string) bool { + return interfaceName == "eth0" + } + + agent, err := NewAgentWithOptions(WithInterfaceFilter(filter)) + assert.NoError(t, err) + defer agent.Close() //nolint:errcheck + + assert.NotNil(t, agent.interfaceFilter) + assert.True(t, agent.interfaceFilter("eth0")) + assert.False(t, agent.interfaceFilter("wlan0")) + }) + + t.Run("default is nil", func(t *testing.T) { + agent, err := NewAgentWithOptions() + assert.NoError(t, err) + defer agent.Close() //nolint:errcheck + + assert.Nil(t, agent.interfaceFilter) + }) + + t.Run("works with config", func(t *testing.T) { + filter := func(interfaceName string) bool { + return interfaceName == "lo" + } + + config := &AgentConfig{ + NetworkTypes: []NetworkType{NetworkTypeUDP4}, + InterfaceFilter: filter, + } + + agent, err := NewAgent(config) + assert.NoError(t, err) + defer agent.Close() //nolint:errcheck + + assert.NotNil(t, agent.interfaceFilter) + assert.True(t, agent.interfaceFilter("lo")) + assert.False(t, agent.interfaceFilter("eth0")) + }) +} + +func TestWithLoggerFactory(t *testing.T) { + t.Run("sets logger factory", func(t *testing.T) { + loggerFactory := logging.NewDefaultLoggerFactory() + loggerFactory.DefaultLogLevel = logging.LogLevelDebug + + agent, err := NewAgentWithOptions(WithLoggerFactory(loggerFactory)) + assert.NoError(t, err) + defer agent.Close() //nolint:errcheck + + assert.NotNil(t, agent.log) + }) + + t.Run("default uses default logger", func(t *testing.T) { + agent, err := NewAgentWithOptions() + assert.NoError(t, err) + defer agent.Close() //nolint:errcheck + + assert.NotNil(t, agent.log) + }) + + t.Run("works with config", func(t *testing.T) { + loggerFactory := logging.NewDefaultLoggerFactory() + config := &AgentConfig{ + NetworkTypes: []NetworkType{NetworkTypeUDP4}, + LoggerFactory: loggerFactory, + } + + agent, err := NewAgent(config) + assert.NoError(t, err) + defer agent.Close() //nolint:errcheck + + assert.NotNil(t, agent.log) + }) +} diff --git a/agent_test.go b/agent_test.go index c85f76a..c258a49 100644 --- a/agent_test.go +++ b/agent_test.go @@ -127,7 +127,7 @@ func TestHandlePeerReflexive(t *testing.T) { //nolint:cyclop tID := [stun.TransactionIDSize]byte{} copy(tID[:], "ABC") agent.pendingBindingRequests = []bindingRequest{ - {time.Now(), tID, &net.UDPAddr{}, false}, + {time.Now(), tID, &net.UDPAddr{}, false, nil}, } hostConfig := CandidateHostConfig{ @@ -2078,3 +2078,195 @@ func TestAgentConfig_initWithDefaults_UsesProvidedValues(t *testing.T) { require.Equal(t, valRelayWait, a.relayAcceptanceMinWait, "expected override for RelayAcceptanceMinWait") require.Equal(t, valStunTimeout, a.stunGatherTimeout, "expected override for STUNGatherTimeout") } + +// TestAutomaticRenominationWithVNet tests automatic renomination with simple vnet setup. +// This is a simplified test that verifies the renomination mechanism triggers correctly. +func TestAutomaticRenominationWithVNet(t *testing.T) { + defer test.CheckRoutines(t)() + + loggerFactory := logging.NewDefaultLoggerFactory() + + // Create simple vnet with two agents on same network (no NAT) + wan, err := vnet.NewRouter(&vnet.RouterConfig{ + CIDR: "0.0.0.0/0", + LoggerFactory: loggerFactory, + }) + require.NoError(t, err) + + net0, err := vnet.NewNet(&vnet.NetConfig{ + StaticIPs: []string{"192.168.0.1"}, + }) + require.NoError(t, err) + require.NoError(t, wan.AddNet(net0)) + + net1, err := vnet.NewNet(&vnet.NetConfig{ + StaticIPs: []string{"192.168.0.2"}, + }) + require.NoError(t, err) + require.NoError(t, wan.AddNet(net1)) + + require.NoError(t, wan.Start()) + defer wan.Stop() //nolint:errcheck + + // Create agents with automatic renomination + keepaliveInterval := 100 * time.Millisecond + checkInterval := 50 * time.Millisecond + renominationInterval := 200 * time.Millisecond + + agent1, err := newAgentWithConfig(&AgentConfig{ + NetworkTypes: []NetworkType{NetworkTypeUDP4}, + MulticastDNSMode: MulticastDNSModeDisabled, + Net: net0, + KeepaliveInterval: &keepaliveInterval, + CheckInterval: &checkInterval, + }, + WithRenomination(DefaultNominationValueGenerator()), + WithAutomaticRenomination(renominationInterval), + ) + require.NoError(t, err) + defer agent1.Close() //nolint:errcheck + + agent2, err := NewAgent(&AgentConfig{ + NetworkTypes: []NetworkType{NetworkTypeUDP4}, + MulticastDNSMode: MulticastDNSModeDisabled, + Net: net1, + KeepaliveInterval: &keepaliveInterval, + CheckInterval: &checkInterval, + }) + require.NoError(t, err) + defer agent2.Close() //nolint:errcheck + + agent2.enableRenomination = true + agent2.nominationValueGenerator = DefaultNominationValueGenerator() + + // Connect the agents using the existing helper + conn1, conn2 := connectWithVNet(t, agent1, agent2) + + // Verify connection works + testData := []byte("test data") + _, err = conn1.Write(testData) + require.NoError(t, err) + + buf := make([]byte, len(testData)) + _, err = conn2.Read(buf) + require.NoError(t, err) + require.Equal(t, testData, buf) +} + +// TestAutomaticRenominationRTTImprovement tests that automatic renomination +// triggers when RTT significantly improves. +func TestAutomaticRenominationRTTImprovement(t *testing.T) { + defer test.CheckRoutines(t)() + + // This test verifies the RTT-based renomination logic + agent, err := NewAgent(&AgentConfig{}) + require.NoError(t, err) + defer agent.Close() //nolint:errcheck + + // Create two pairs with different RTTs + localHost1, err := NewCandidateHost(&CandidateHostConfig{ + Network: "udp", + Address: "192.168.1.1", + Port: 10000, + Component: 1, + }) + require.NoError(t, err) + + localHost2, err := NewCandidateHost(&CandidateHostConfig{ + Network: "udp", + Address: "192.168.1.3", // Different address + Port: 10001, + Component: 1, + }) + require.NoError(t, err) + + remoteHost, err := NewCandidateHost(&CandidateHostConfig{ + Network: "udp", + Address: "192.168.1.2", + Port: 20000, + Component: 1, + }) + require.NoError(t, err) + + // Current pair with high RTT + currentPair := newCandidatePair(localHost1, remoteHost, true) + currentPair.state = CandidatePairStateSucceeded + currentPair.UpdateRoundTripTime(100 * time.Millisecond) + + // Candidate pair with significantly better RTT (>10ms improvement) + betterPair := newCandidatePair(localHost2, remoteHost, true) + betterPair.state = CandidatePairStateSucceeded + betterPair.UpdateRoundTripTime(50 * time.Millisecond) // 50ms improvement + + // Should trigger renomination due to RTT improvement + shouldRenominate := agent.shouldRenominate(currentPair, betterPair) + require.True(t, shouldRenominate, "Should renominate for >10ms RTT improvement") + + // Test with small RTT improvement (<10ms) + slightlyBetterPair := newCandidatePair(localHost2, remoteHost, true) + slightlyBetterPair.state = CandidatePairStateSucceeded + slightlyBetterPair.UpdateRoundTripTime(95 * time.Millisecond) // Only 5ms improvement + + shouldRenominate = agent.shouldRenominate(currentPair, slightlyBetterPair) + require.False(t, shouldRenominate, "Should not renominate for <10ms RTT improvement") +} + +// TestAutomaticRenominationRelayToDirect tests that automatic renomination +// always prefers direct connections over relay connections. +func TestAutomaticRenominationRelayToDirect(t *testing.T) { + defer test.CheckRoutines(t)() + + agent, err := NewAgent(&AgentConfig{}) + require.NoError(t, err) + defer agent.Close() //nolint:errcheck + + // Create relay pair + localRelay, err := NewCandidateRelay(&CandidateRelayConfig{ + Network: "udp", + Address: "10.0.0.1", + Port: 30000, + Component: 1, + RelAddr: "192.168.1.1", + RelPort: 10000, + }) + require.NoError(t, err) + + remoteRelay, err := NewCandidateRelay(&CandidateRelayConfig{ + Network: "udp", + Address: "10.0.0.2", + Port: 40000, + Component: 1, + RelAddr: "192.168.1.2", + RelPort: 20000, + }) + require.NoError(t, err) + + relayPair := newCandidatePair(localRelay, remoteRelay, true) + relayPair.state = CandidatePairStateSucceeded + relayPair.UpdateRoundTripTime(50 * time.Millisecond) + + // Create host pair with similar RTT + localHost, err := NewCandidateHost(&CandidateHostConfig{ + Network: "udp", + Address: "192.168.1.1", + Port: 10000, + Component: 1, + }) + require.NoError(t, err) + + remoteHost, err := NewCandidateHost(&CandidateHostConfig{ + Network: "udp", + Address: "192.168.1.2", + Port: 20000, + Component: 1, + }) + require.NoError(t, err) + + hostPair := newCandidatePair(localHost, remoteHost, true) + hostPair.state = CandidatePairStateSucceeded + hostPair.UpdateRoundTripTime(45 * time.Millisecond) // Similar RTT + + // Should always prefer direct over relay + shouldRenominate := agent.shouldRenominate(relayPair, hostPair) + require.True(t, shouldRenominate, "Should always renominate from relay to direct connection") +} diff --git a/examples/automatic-renomination/README.md b/examples/automatic-renomination/README.md new file mode 100644 index 0000000..fc75c6d --- /dev/null +++ b/examples/automatic-renomination/README.md @@ -0,0 +1,555 @@ +# Automatic Renomination Example + +This example demonstrates the ICE automatic renomination feature using real network interfaces. Automatic renomination allows the controlling ICE agent to automatically switch between candidate pairs when a better connection path becomes available. + +## What is Automatic Renomination? + +Automatic renomination is a feature where the controlling ICE agent continuously monitors candidate pairs and automatically switches to better pairs when they become available. This is particularly useful for: + +- **Adapting to network changes**: When network conditions improve or degrade +- **Optimizing for latency**: Automatically switching to lower-latency paths +- **Quality of service**: Maintaining the best possible connection quality +- **Interface failover**: Switching to alternate interfaces when primary path fails + +## How It Works + +The automatic renomination feature evaluates candidate pairs based on: + +1. **Candidate types**: Direct connections (host-to-host) are preferred over relay connections +2. **Round-trip time (RTT)**: Lower latency paths are preferred +3. **Connection stability**: Pairs that have recently received responses are favored + +When a candidate pair is found that is significantly better than the current selection (>10ms RTT improvement or direct vs relay), the agent automatically renominates to use the better pair. + +## Quick Start Tutorial + +This step-by-step tutorial walks you through setting up virtual network interfaces and testing automatic renomination. + +**Important:** This example uses **network namespaces with two veth pairs** to properly isolate network traffic so that `tc` (traffic control) rules can affect latency. The controlled agent runs in the default namespace, and the controlling agent runs in a separate namespace (ns1). They communicate via two veth pairs, giving us multiple candidate pairs for automatic renomination. + +### Step 1: Create Network Namespace with Two veth Pairs + +Create a network namespace and two veth pairs to connect them: + +```bash +# Create namespace +sudo ip netns add ns1 + +# Create FIRST veth pair (veth0 <-> veth1) +sudo ip link add veth0 type veth peer name veth1 +sudo ip link set veth1 netns ns1 +sudo ip addr add 192.168.100.1/24 dev veth0 +sudo ip link set veth0 up +sudo ip netns exec ns1 ip addr add 192.168.100.2/24 dev veth1 +sudo ip netns exec ns1 ip link set veth1 up + +# Create SECOND veth pair (veth2 <-> veth3) +sudo ip link add veth2 type veth peer name veth3 +sudo ip link set veth3 netns ns1 +sudo ip addr add 192.168.101.1/24 dev veth2 +sudo ip link set veth2 up +sudo ip netns exec ns1 ip addr add 192.168.101.2/24 dev veth3 +sudo ip netns exec ns1 ip link set veth3 up + +# Bring up loopback in ns1 +sudo ip netns exec ns1 ip link set lo up +``` + +**Verify connectivity on both pairs:** +```bash +# Ping via first veth pair +ping -c 2 192.168.100.2 + +# Ping via second veth pair +ping -c 2 192.168.101.2 +``` + +You should see successful pings on both with low latency (~0.05ms). + +**Verify that tc rules work:** +```bash +# Add 100ms latency to veth0 (first pair) +sudo tc qdisc add dev veth0 root netem delay 100ms + +# Test ping via first pair - should now show ~100ms latency +ping -c 2 192.168.100.2 + +# Test ping via second pair - should still be fast +ping -c 2 192.168.101.2 + +# Remove tc rule for now +sudo tc qdisc del dev veth0 root +``` + +After adding the tc rule to veth0, pings to 192.168.100.2 should show ~100ms latency, while pings to 192.168.101.2 remain fast. This proves tc is working and we have independent paths! + +### Step 2: Start the Controlled Agent (Default Namespace) + +Open a terminal and start the controlled (non-controlling) agent in the **default namespace**: + +```bash +cd examples/automatic-renomination +go run main.go +``` + +**Expected output:** +``` +=== Automatic Renomination Example === +Local Agent is CONTROLLED + +Press 'Enter' when both processes have started +``` + +Don't press Enter yet - wait for Step 3. + +### Step 3: Start the Controlling Agent (ns1 Namespace) + +Open a second terminal and start the controlling agent in the **ns1 namespace**: + +```bash +cd examples/automatic-renomination +sudo ip netns exec ns1 go run main.go -controlling +``` + +**Expected output:** +``` +=== Automatic Renomination Example === +Local Agent is CONTROLLING (with automatic renomination enabled) + +Press 'Enter' when both processes have started +``` + +### Step 4: Connect the Agents + +Press Enter in **both terminals**. You should see candidate gathering and connection establishment: + +**Expected output (both terminals):** +``` +Gathering candidates... +Local candidate: candidate:... 192.168.100.x ... typ host +Local candidate: candidate:... 192.168.101.x ... typ host +Added remote candidate: candidate:... 192.168.100.x ... typ host +Added remote candidate: candidate:... 192.168.101.x ... typ host +Starting ICE connection... +>>> ICE Connection State: Checking + +>>> SELECTED CANDIDATE PAIR CHANGED <<< + Local: candidate:... 192.168.10x.x ... typ host (type: host) + Remote: candidate:... 192.168.10x.x ... typ host (type: host) + +>>> ICE Connection State: Connected + +=== CONNECTED === +``` + +You should see **2 local candidates** and **2 remote candidates**, giving you **4 candidate pairs** total. + +**Controlling agent will also show:** +``` +Automatic renomination is enabled on the controlling agent. +To test it, you can use traffic control (tc) to change network conditions: + + # Add 100ms latency to eth0: + sudo tc qdisc add dev eth0 root netem delay 100ms + + # Remove the latency: + sudo tc qdisc del dev eth0 root + +Watch for "SELECTED CANDIDATE PAIR CHANGED" messages above. +The agent will automatically renominate to better paths when detected. +``` + +You should also see periodic messages being exchanged with RTT information: +``` +Sent: Message #1 from controlling agent [RTT: 0.35ms] +Received: Message #1 from controlled agent [RTT: 0.35ms] +``` + +### Step 5: Add Latency to Trigger Renomination + +In a third terminal, add latency to veth0 (the first veth pair): + +```bash +# Add 100ms latency to veth0 +sudo tc qdisc add dev veth0 root netem delay 100ms + +# Check that the rule was applied +sudo tc qdisc show dev veth0 +``` + +**Expected output:** +``` +qdisc netem 8001: root refcnt 2 limit 1000 delay 100ms +``` + +### Step 6: Observe Automatic Renomination + +Watch the **controlling agent's terminal**. Look at the debug output showing candidate pair states: + +``` +=== DEBUG: Candidate Pair States === + candidate:... <-> candidate:... + State: succeeded, Nominated: true + RTT: 100.xx ms <-- This pair now has high latency! + candidate:... <-> candidate:... + State: succeeded, Nominated: false + RTT: 0.3x ms <-- This pair is still fast! +=================================== +``` + +Within a few seconds (based on the renomination interval of 3 seconds), once the RTT difference exceeds 10ms, you should see: + +**Expected output:** +``` +>>> SELECTED CANDIDATE PAIR CHANGED <<< + Local: candidate:... 192.168.101.x ... typ host (type: host) + Remote: candidate:... 192.168.101.x ... typ host (type: host) +``` + +This shows the agent automatically switched from the slow path (192.168.100.x) to the fast path (192.168.101.x)! + +**What to look for:** +- RTT on the nominated pair increases from ~0.3ms to ~100ms after adding tc rule +- The RTT displayed in sent/received messages will also increase to ~100ms +- After 3-6 seconds, renomination triggers +- New selected pair uses the 192.168.101.x addresses (veth2/veth3) +- RTT in both the debug output and sent/received messages drops back to ~0.3ms + +### Step 7: Remove Latency and Observe Switch Back + +Remove the latency from veth0: + +```bash +sudo tc qdisc del dev veth0 root +``` + +Wait a few seconds and watch for another renomination event as the agent switches back to the now-improved path. + +### Step 8: Clean Up + +When you're done testing, clean up the namespace and both veth pairs: + +```bash +# Stop both agents with Ctrl+C first + +# Remove any traffic control rules +sudo tc qdisc del dev veth0 root 2>/dev/null || true +sudo tc qdisc del dev veth2 root 2>/dev/null || true + +# Remove namespace (this automatically removes veth1 and veth3) +sudo ip netns del ns1 2>/dev/null || true + +# Remove veth0 and veth2 from default namespace +sudo ip link delete veth0 2>/dev/null || true +sudo ip link delete veth2 2>/dev/null || true + +# Verify cleanup +ip link show | grep veth # Should show no output +ip netns list | grep ns1 # Should show no output +``` + +## Running the Example + +This example requires running two processes - one controlling and one controlled. + +### Terminal 1 (Controlling Agent) + +```bash +go run main.go -controlling +``` + +### Terminal 2 (Controlled Agent) + +```bash +go run main.go +``` + +Press Enter in both terminals once both processes are running to start the ICE connection. + +## Testing Automatic Renomination + +Once connected, you'll see messages like: + +``` +=== CONNECTED === + +Automatic renomination is enabled on the controlling agent. +To test it, you can use traffic control (tc) to change network conditions: + + # Add 100ms latency to eth0: + sudo tc qdisc add dev eth0 root netem delay 100ms + + # Remove the latency: + sudo tc qdisc del dev eth0 root + +Watch for "SELECTED CANDIDATE PAIR CHANGED" messages above. +``` + +### Using Traffic Control (tc) to Trigger Renomination + +Traffic control (`tc`) is a Linux tool for simulating network conditions. Here are some useful commands: + +#### Add latency to an interface + +```bash +# Add 100ms delay +sudo tc qdisc add dev eth0 root netem delay 100ms + +# Add variable latency (50ms ± 10ms) +sudo tc qdisc add dev eth0 root netem delay 50ms 10ms +``` + +#### Simulate packet loss + +```bash +# Add 5% packet loss +sudo tc qdisc add dev eth0 root netem loss 5% +``` + +#### Limit bandwidth + +```bash +# Limit to 1mbit +sudo tc qdisc add dev eth0 root tbf rate 1mbit burst 32kbit latency 400ms +``` + +#### Remove all rules + +```bash +# Remove all tc rules from interface +sudo tc qdisc del dev eth0 root +``` + +#### Check current rules + +```bash +# View current tc configuration +sudo tc qdisc show dev eth0 +``` + +### Expected Behavior + +When you change network conditions with `tc`, watch the controlling agent's output: + +1. Initial connection will select the best available path +2. When you add latency/loss, the RTT increases +3. If the RTT difference exceeds 10ms, automatic renomination may trigger +4. You'll see a "SELECTED CANDIDATE PAIR CHANGED" message +5. The connection continues using the new pair + +**Note**: Renomination only occurs if a significantly better pair is available. Simply degrading one path won't trigger renomination unless there's an alternate path that's measurably better. + +## Understanding the Output + +### Connection State Changes + +``` +>>> ICE Connection State: Checking +>>> ICE Connection State: Connected +``` + +These show the overall ICE connection state progression. + +### Candidate Discovery + +``` +Local candidate: candidate:1 1 udp 2130706431 192.168.1.100 54321 typ host +Added remote candidate: candidate:2 1 udp 2130706431 192.168.1.101 54322 typ host +``` + +These show discovered local and remote ICE candidates. + +### Pair Selection + +``` +>>> SELECTED CANDIDATE PAIR CHANGED <<< + Local: candidate:1 1 udp 2130706431 192.168.1.100 54321 typ host (type: host) + Remote: candidate:2 1 udp 2130706431 192.168.1.101 54322 typ host (type: host) +``` + +This indicates automatic renomination occurred and shows the new selected pair. + +### Message RTT Display + +``` +Sent: Message #1 from controlling agent [RTT: 0.35ms] +Received: Message #1 from controlled agent [RTT: 0.35ms] +``` + +Each sent and received message displays the current Round-Trip Time (RTT) of the selected candidate pair. This RTT value: +- Shows the current latency of the connection path +- Updates in real-time as network conditions change +- Helps verify that automatic renomination is working (RTT should improve after switching to a better path) +- May show "N/A" briefly during initial connection or if RTT hasn't been measured yet + +## Testing Scenarios + +### Scenario 1: Interface Latency Change + +1. Start both agents +2. Wait for connection +3. Add latency to one interface: `sudo tc qdisc add dev eth0 root netem delay 100ms` +4. If multiple interfaces exist with different RTTs, automatic renomination should occur +5. Remove latency: `sudo tc qdisc del dev eth0 root` + +### Scenario 2: Multiple Network Interfaces + +If your machine has multiple network interfaces (e.g., WiFi and Ethernet): + +1. Start agents connected via one interface +2. Degrade that interface with `tc` +3. The agent should automatically switch to the other interface if it provides better quality + +### Scenario 3: Connection Recovery + +1. Start with one interface having high latency +2. Once connected, remove the latency +3. The agent should detect the improved path and switch back + +## Configuration Options + +You can modify the example to customize automatic renomination: + +### Renomination Interval + +```go +renominationInterval := 5 * time.Second // How often to check (default: 3s) +iceAgent, err = ice.NewAgentWithOptions( + ice.WithNetworkTypes([]ice.NetworkType{ice.NetworkTypeUDP4, ice.NetworkTypeUDP6}), + ice.WithInterfaceFilter(interfaceFilter), + ice.WithRenomination(ice.DefaultNominationValueGenerator()), + ice.WithAutomaticRenomination(renominationInterval), +) +``` + +### Interface Filter + +The example uses an `InterfaceFilter` to constrain the ICE agent to only use veth interfaces: + +```go +interfaceFilter := func(interfaceName string) bool { + // Allow all veth interfaces (veth0, veth1, veth2, veth3) + // This gives us multiple candidate pairs for automatic renomination + return len(interfaceName) >= 4 && interfaceName[:4] == "veth" +} +``` + +To use your real network interfaces instead: + +```go +// Option 1: Use all interfaces (no InterfaceFilter) +iceAgent, err = ice.NewAgentWithOptions( + ice.WithNetworkTypes([]ice.NetworkType{ice.NetworkTypeUDP4, ice.NetworkTypeUDP6}), + ice.WithRenomination(ice.DefaultNominationValueGenerator()), + ice.WithAutomaticRenomination(renominationInterval), +) + +// Option 2: Filter to specific real interfaces (e.g., eth0 and wlan0) +interfaceFilter := func(interfaceName string) bool { + return interfaceName == "eth0" || interfaceName == "wlan0" +} + +// Option 3: Use only IPv4 interfaces starting with "eth" +interfaceFilter := func(interfaceName string) bool { + return strings.HasPrefix(interfaceName, "eth") +} +``` + +**Note:** When using real network interfaces without network namespaces, you'll need to run the two processes on different machines to properly test tc rules, as local traffic on the same machine may bypass tc. + +## Troubleshooting + +### ICE agent not using veth interfaces + +If you see candidates on your real interfaces (like eth0, eth2, wlan0) instead of veth0/veth1: + +- **Check the InterfaceFilter**: Make sure the code has the `InterfaceFilter` configured to only allow veth0 and veth1 +- **Verify veth interfaces exist**: Run `ip link show veth0` (and `sudo ip netns exec ns1 ip link show veth1`) to confirm they're created +- **Verify interfaces are UP**: Run `ip link show veth0` and check for `UP` in the output +- **Check IP addresses**: Run `ip addr show veth0` and `sudo ip netns exec ns1 ip addr show veth1` to confirm the 192.168.100.x addresses are assigned + +### No candidates found / Connection fails + +If the agents fail to connect after adding the InterfaceFilter: + +- **Create dummy interfaces first**: The dummy interfaces must be created before starting the agents +- **Both agents need the filter**: Both controlling and controlled agents must have the same InterfaceFilter +- **Check for errors**: Look for errors during candidate gathering that might indicate interface issues + +### No renomination occurring + +- **Only one candidate pair available**: Automatic renomination needs alternate paths to switch between. If only one candidate pair exists, renomination won't occur. +- **Insufficient quality difference**: The new path must be significantly better (>10ms RTT improvement or better candidate type) to trigger renomination. +- **Not enough time elapsed**: The renomination interval (default 3s) must pass before evaluation occurs. +- **Wrong interface**: Make sure you're adding latency to the interface that's actually being used. Check the "SELECTED CANDIDATE PAIR CHANGED" message to see which interface/IP is in use. + +### Permission denied with tc commands + +All `tc` commands require root privileges. Use `sudo` before each command. + +### Interface name not found + +Use `ip link` to list available network interfaces on your system. Replace `eth0` with your actual interface name (e.g., `enp0s3`, `wlan0`, `wlp3s0`). + +## Cleanup + +After testing, it's important to clean up any virtual interfaces and traffic control rules you created. + +### Remove Traffic Control Rules + +If you added any tc rules to interfaces, remove them: + +```bash +# Remove tc rules from veth interfaces +sudo tc qdisc del dev veth0 root 2>/dev/null || true +sudo tc qdisc del dev veth2 root 2>/dev/null || true + +# List all interfaces with tc rules +tc qdisc show + +# Remove tc rules from any interface shown above +# sudo tc qdisc del dev root +``` + +### Remove Network Namespace and veth Pairs + +If you created the namespace and veth pairs for testing, remove them: + +```bash +# Remove namespace (automatically removes veth1 and veth3) +sudo ip netns del ns1 2>/dev/null || true + +# Remove veth0 and veth2 from default namespace +sudo ip link delete veth0 2>/dev/null || true +sudo ip link delete veth2 2>/dev/null || true + +# Verify removal +ip link show | grep veth # Should show no output +ip netns list | grep ns1 # Should show no output +``` + +### Verify Clean State + +Check that everything is cleaned up: + +```bash +# Check for any remaining tc rules +tc qdisc show + +# Check for veth interfaces +ip link show | grep veth + +# Check for namespaces +ip netns list + +# All commands should show no veth interfaces or ns1 namespace if cleanup was successful +``` + +## Additional Resources + +- [ICE RFC 8445](https://datatracker.ietf.org/doc/html/rfc8445) - Interactive Connectivity Establishment (ICE) Protocol +- [draft-thatcher-ice-renomination](https://datatracker.ietf.org/doc/html/draft-thatcher-ice-renomination-01) - ICE Renomination Specification +- [Linux tc man page](https://man7.org/linux/man-pages/man8/tc.8.html) - Traffic Control documentation +- [netem documentation](https://wiki.linuxfoundation.org/networking/netem) - Network Emulation guide +- [Linux network namespaces](https://man7.org/linux/man-pages/man8/ip-netns.8.html) - Network namespace documentation +- [veth pairs](https://man7.org/linux/man-pages/man4/veth.4.html) - Virtual ethernet pair documentation diff --git a/examples/automatic-renomination/main.go b/examples/automatic-renomination/main.go new file mode 100644 index 0000000..3ba8c88 --- /dev/null +++ b/examples/automatic-renomination/main.go @@ -0,0 +1,311 @@ +// SPDX-FileCopyrightText: 2025 The Pion community +// SPDX-License-Identifier: MIT + +// Package main demonstrates automatic renomination with real network interfaces. +// Run two instances of this program - one controlling and one controlled - and use +// traffic control (tc) commands to simulate network changes and trigger automatic renomination. +package main + +import ( + "bufio" + "context" + "flag" + "fmt" + "net/http" + "net/url" + "os" + "time" + + "github.com/pion/ice/v4" + "github.com/pion/logging" +) + +const ( + rttNotAvailable = "N/A" +) + +//nolint:gochecknoglobals +var ( + isControlling bool + iceAgent *ice.Agent + remoteAuthChannel chan string + localHTTPPort, remoteHTTPPort int + localHTTPAddr, remoteHTTPAddr string + selectedLocalCandidateID string + selectedRemoteCandidateID string +) + +// getRTT returns the current RTT for the selected candidate pair. +func getRTT() string { + if selectedLocalCandidateID == "" || selectedRemoteCandidateID == "" { + return rttNotAvailable + } + + stats := iceAgent.GetCandidatePairsStats() + for _, stat := range stats { + if stat.LocalCandidateID == selectedLocalCandidateID && stat.RemoteCandidateID == selectedRemoteCandidateID { + if stat.CurrentRoundTripTime > 0 { + return fmt.Sprintf("%.2fms", stat.CurrentRoundTripTime*1000) + } + + return rttNotAvailable + } + } + + return rttNotAvailable +} + +// HTTP Listener to get ICE Credentials from remote Peer. +func remoteAuth(_ http.ResponseWriter, r *http.Request) { + if err := r.ParseForm(); err != nil { + panic(err) + } + + remoteAuthChannel <- r.PostForm["ufrag"][0] + remoteAuthChannel <- r.PostForm["pwd"][0] +} + +// HTTP Listener to get ICE Candidate from remote Peer. +func remoteCandidate(_ http.ResponseWriter, r *http.Request) { + if err := r.ParseForm(); err != nil { + panic(err) + } + + c, err := ice.UnmarshalCandidate(r.PostForm["candidate"][0]) + if err != nil { + panic(err) + } + + if err := iceAgent.AddRemoteCandidate(c); err != nil { //nolint:contextcheck + panic(err) + } + + fmt.Printf("Added remote candidate: %s\n", c) +} + +func main() { //nolint:cyclop,maintidx + var ( + err error + conn *ice.Conn + ) + remoteAuthChannel = make(chan string, 3) + + flag.BoolVar(&isControlling, "controlling", false, "is ICE Agent controlling") + flag.Parse() + + if isControlling { + // Controlling agent runs in ns1 namespace + localHTTPPort = 9000 + remoteHTTPPort = 9001 + localHTTPAddr = "192.168.100.2" // veth1 in ns1 + remoteHTTPAddr = "192.168.100.1" // veth0 in default namespace + } else { + // Controlled agent runs in default namespace + localHTTPPort = 9001 + remoteHTTPPort = 9000 + localHTTPAddr = "192.168.100.1" // veth0 in default namespace + remoteHTTPAddr = "192.168.100.2" // veth1 in ns1 + } + + http.HandleFunc("/remoteAuth", remoteAuth) + http.HandleFunc("/remoteCandidate", remoteCandidate) + go func() { + if err = http.ListenAndServe(fmt.Sprintf("%s:%d", localHTTPAddr, localHTTPPort), nil); err != nil { //nolint:gosec + panic(err) + } + }() + + fmt.Println("=== Automatic Renomination Example ===") + if isControlling { + fmt.Println("Local Agent is CONTROLLING (with automatic renomination enabled)") + } else { + fmt.Println("Local Agent is CONTROLLED") + } + fmt.Println() + fmt.Print("Press 'Enter' when both processes have started") + if _, err = bufio.NewReader(os.Stdin).ReadBytes('\n'); err != nil { + panic(err) + } + + // Create the ICE agent with automatic renomination enabled on the controlling side + // Use InterfaceFilter to constrain to veth interfaces for testing + interfaceFilter := func(interfaceName string) bool { + // Allow all veth interfaces (veth0, veth1, veth2, veth3) + // This gives us multiple candidate pairs for automatic renomination + return len(interfaceName) >= 4 && interfaceName[:4] == "veth" + } + + // Create a logger factory with Debug level enabled + loggerFactory := logging.NewDefaultLoggerFactory() + loggerFactory.DefaultLogLevel = logging.LogLevelDebug + + if isControlling { + renominationInterval := 3 * time.Second + iceAgent, err = ice.NewAgentWithOptions( + ice.WithNetworkTypes([]ice.NetworkType{ice.NetworkTypeUDP4, ice.NetworkTypeUDP6}), + ice.WithInterfaceFilter(interfaceFilter), + ice.WithLoggerFactory(loggerFactory), + ice.WithRenomination(ice.DefaultNominationValueGenerator()), + ice.WithAutomaticRenomination(renominationInterval), + ) + } else { + iceAgent, err = ice.NewAgentWithOptions( + ice.WithNetworkTypes([]ice.NetworkType{ice.NetworkTypeUDP4, ice.NetworkTypeUDP6}), + ice.WithInterfaceFilter(interfaceFilter), + ice.WithLoggerFactory(loggerFactory), + ) + } + if err != nil { + panic(err) + } + + // When we have gathered a new ICE Candidate send it to the remote peer + if err = iceAgent.OnCandidate(func(c ice.Candidate) { + if c == nil { + return + } + + fmt.Printf("Local candidate: %s\n", c) + + _, err = http.PostForm(fmt.Sprintf("http://%s:%d/remoteCandidate", remoteHTTPAddr, remoteHTTPPort), //nolint + url.Values{ + "candidate": {c.Marshal()}, + }) + if err != nil { + panic(err) + } + }); err != nil { + panic(err) + } + + // When ICE Connection state has changed print to stdout + if err = iceAgent.OnConnectionStateChange(func(c ice.ConnectionState) { + fmt.Printf(">>> ICE Connection State: %s\n", c.String()) + }); err != nil { + panic(err) + } + + // When selected candidate pair changes, print it + if err = iceAgent.OnSelectedCandidatePairChange(func(local, remote ice.Candidate) { + // Track the selected candidate IDs for RTT lookup + selectedLocalCandidateID = local.ID() + selectedRemoteCandidateID = remote.ID() + + fmt.Println() + fmt.Println(">>> SELECTED CANDIDATE PAIR CHANGED <<<") + fmt.Printf(" Local: %s (type: %s)\n", local, local.Type()) + fmt.Printf(" Remote: %s (type: %s)\n", remote, remote.Type()) + fmt.Println() + }); err != nil { + panic(err) + } + + // Get the local auth details and send to remote peer + localUfrag, localPwd, err := iceAgent.GetLocalUserCredentials() + if err != nil { + panic(err) + } + + _, err = http.PostForm(fmt.Sprintf("http://%s:%d/remoteAuth", remoteHTTPAddr, remoteHTTPPort), //nolint + url.Values{ + "ufrag": {localUfrag}, + "pwd": {localPwd}, + }) + if err != nil { + panic(err) + } + + remoteUfrag := <-remoteAuthChannel + remotePwd := <-remoteAuthChannel + + if err = iceAgent.GatherCandidates(); err != nil { + panic(err) + } + + fmt.Println("Gathering candidates...") + time.Sleep(2 * time.Second) // Give time for candidate gathering + + // Start the ICE Agent. One side must be controlled, and the other must be controlling + fmt.Println("Starting ICE connection...") + if isControlling { + conn, err = iceAgent.Dial(context.Background(), remoteUfrag, remotePwd) + } else { + conn, err = iceAgent.Accept(context.Background(), remoteUfrag, remotePwd) + } + if err != nil { + panic(err) + } + + fmt.Println() + fmt.Println("=== CONNECTED ===") + fmt.Println() + + if isControlling { + fmt.Println("Automatic renomination is enabled on the controlling agent.") + fmt.Println("To test it, you can use traffic control (tc) to change network conditions:") + fmt.Println() + fmt.Println(" # Add 100ms latency to veth0:") + fmt.Println(" sudo tc qdisc add dev veth0 root netem delay 100ms") + fmt.Println() + fmt.Println(" # Remove the latency:") + fmt.Println(" sudo tc qdisc del dev veth0 root") + fmt.Println() + fmt.Println("Watch for \"SELECTED CANDIDATE PAIR CHANGED\" messages above.") + fmt.Println("The agent will automatically renominate to better paths when detected.") + fmt.Println() + + // Print debug info about candidate pairs every 5 seconds + go func() { + for { + time.Sleep(5 * time.Second) + fmt.Println() + fmt.Println("=== DEBUG: Candidate Pair States ===") + stats := iceAgent.GetCandidatePairsStats() + for _, stat := range stats { + fmt.Printf(" %s <-> %s\n", stat.LocalCandidateID, stat.RemoteCandidateID) + fmt.Printf(" State: %s, Nominated: %v\n", stat.State, stat.Nominated) + if stat.CurrentRoundTripTime > 0 { + fmt.Printf(" RTT: %.2fms\n", stat.CurrentRoundTripTime*1000) + } + } + fmt.Println("===================================") + fmt.Println() + } + }() + } + + // Send a message every 5 seconds + go func() { + counter := 0 + for { + time.Sleep(5 * time.Second) + counter++ + + role := "controlling" + if !isControlling { + role = "controlled" + } + msg := fmt.Sprintf("Message #%d from %s agent", counter, role) + if _, err = conn.Write([]byte(msg)); err != nil { + fmt.Printf("Write error: %v\n", err) + + return + } + + fmt.Printf("Sent: %s [RTT: %s]\n", msg, getRTT()) + } + }() + + // Receive messages in a loop from the remote peer + buf := make([]byte, 1500) + for { + n, err := conn.Read(buf) + if err != nil { + fmt.Printf("Read error: %v\n", err) + + return + } + + fmt.Printf("Received: %s [RTT: %s]\n", string(buf[:n]), getRTT()) + } +} diff --git a/selection.go b/selection.go index d82a04c..533d566 100644 --- a/selection.go +++ b/selection.go @@ -54,6 +54,14 @@ func (s *controllingSelector) ContactCandidates() { if s.agent.validateSelectedPair() { s.log.Trace("Checking keepalive") s.agent.checkKeepalive() + + // If automatic renomination is enabled, continuously ping all candidate pairs + // to keep them tested with fresh RTT measurements for switching decisions + if s.agent.automaticRenomination && s.agent.enableRenomination { + s.agent.keepAliveCandidatesForRenomination() + } + + s.checkForAutomaticRenomination() } case s.nominatedPair != nil: s.nominatePair(s.nominatedPair) @@ -163,8 +171,20 @@ func (s *controllingSelector) HandleSuccessResponse(m *stun.Message, local, remo pair.state = CandidatePairStateSucceeded s.log.Tracef("Found valid candidate pair: %s", pair) - if pendingRequest.isUseCandidate && s.agent.getSelectedPair() == nil { - s.agent.setSelectedPair(pair) + + // Handle nomination/renomination + if pendingRequest.isUseCandidate { + selectedPair := s.agent.getSelectedPair() + + // If this is a renomination request (has nomination value), always update the selected pair + // If it's a standard nomination (no value), only set if no pair is selected yet + if pendingRequest.nominationValue != nil { + s.log.Infof("Renomination success response received for pair %s (nomination value: %d), switching to this pair", + pair, *pendingRequest.nominationValue) + s.agent.setSelectedPair(pair) + } else if selectedPair == nil { + s.agent.setSelectedPair(pair) + } } pair.UpdateRoundTripTime(rtt) @@ -187,6 +207,68 @@ func (s *controllingSelector) PingCandidate(local, remote Candidate) { s.agent.sendBindingRequest(msg, local, remote) } +// checkForAutomaticRenomination evaluates if automatic renomination should occur. +// This is called periodically when the agent is in connected state and automatic +// renomination is enabled. +func (s *controllingSelector) checkForAutomaticRenomination() { + if !s.agent.automaticRenomination || !s.agent.enableRenomination { + s.log.Tracef("Automatic renomination check skipped: automaticRenomination=%v, enableRenomination=%v", + s.agent.automaticRenomination, s.agent.enableRenomination) + + return + } + + timeSinceStart := time.Since(s.startTime) + if timeSinceStart < s.agent.renominationInterval { + s.log.Tracef("Automatic renomination check skipped: not enough time since start (%v < %v)", + timeSinceStart, s.agent.renominationInterval) + + return + } + + if !s.agent.lastRenominationTime.IsZero() { + timeSinceLastRenomination := time.Since(s.agent.lastRenominationTime) + if timeSinceLastRenomination < s.agent.renominationInterval { + s.log.Tracef("Automatic renomination check skipped: too soon since last renomination (%v < %v)", + timeSinceLastRenomination, s.agent.renominationInterval) + + return + } + } + + currentPair := s.agent.getSelectedPair() + if currentPair == nil { + s.log.Tracef("Automatic renomination check skipped: no current selected pair") + + return + } + + bestPair := s.agent.findBestCandidatePair() + if bestPair == nil { + s.log.Tracef("Automatic renomination check skipped: no best pair found") + + return + } + + s.log.Debugf("Evaluating automatic renomination: current=%s (RTT=%.2fms), best=%s (RTT=%.2fms)", + currentPair, currentPair.CurrentRoundTripTime()*1000, + bestPair, bestPair.CurrentRoundTripTime()*1000) + + if s.agent.shouldRenominate(currentPair, bestPair) { + s.log.Infof("Automatic renomination triggered: switching from %s to %s", + currentPair, bestPair) + + // Update last renomination time to prevent rapid renominations + s.agent.lastRenominationTime = time.Now() + + if err := s.agent.RenominateCandidate(bestPair.Local, bestPair.Remote); err != nil { + s.log.Errorf("Failed to trigger automatic renomination: %v", err) + } + } else { + s.log.Debugf("Automatic renomination not warranted") + } +} + type controlledSelector struct { agent *Agent log logging.LeveledLogger @@ -219,6 +301,31 @@ func (s *controlledSelector) shouldAcceptNomination(nominationValue *uint32) boo return false } +// shouldSwitchSelectedPair determines if we should switch to a new nominated pair. +// Returns true if the switch should occur, false otherwise. +func (s *controlledSelector) shouldSwitchSelectedPair(pair, selectedPair *CandidatePair, nominationValue *uint32) bool { + switch { + case selectedPair == nil: + // No current selection, accept the nomination + return true + case selectedPair == pair: + // Same pair, no change needed + return false + case nominationValue != nil: + // Renomination is in use (nomination value present) + // Accept the switch based on nomination value alone, not priority + // The shouldAcceptNomination check already validated this is a valid renomination + s.log.Debugf("Accepting renomination to pair %s (nomination value: %d)", pair, *nominationValue) + + return true + } + + // Standard ICE nomination without renomination - apply priority rules + // Only switch if we don't check priority, OR new pair has strictly higher priority + return !s.agent.needsToCheckPriorityOnNominated() || + selectedPair.priority() < pair.priority() +} + func (s *controlledSelector) ContactCandidates() { if s.agent.getSelectedPair() != nil { if s.agent.validateSelectedPair() { @@ -334,13 +441,10 @@ func (s *controlledSelector) HandleBindingRequest(message *stun.Message, local, // generated a valid pair (Section 7.2.5.3.2). The agent sets the // nominated flag value of the valid pair to true. selectedPair := s.agent.getSelectedPair() - if selectedPair == nil || - (selectedPair != pair && - (!s.agent.needsToCheckPriorityOnNominated() || - selectedPair.priority() <= pair.priority())) { + if s.shouldSwitchSelectedPair(pair, selectedPair, nominationValue) { s.log.Tracef("Accepting nomination for pair %s", pair) s.agent.setSelectedPair(pair) - } else if selectedPair != pair { + } else { s.log.Tracef("Ignore nominate new pair %s, already nominated pair %s", pair, selectedPair) } } else { diff --git a/selection_test.go b/selection_test.go index ffd0b35..0795a5b 100644 --- a/selection_test.go +++ b/selection_test.go @@ -25,6 +25,12 @@ import ( "github.com/stretchr/testify/require" ) +const ( + selectionTestPassword = "pwd" + selectionTestRemoteUfrag = "remote" + selectionTestLocalUfrag = "local" +) + func sendUntilDone(t *testing.T, writingConn, readingConn net.Conn, maxAttempts int) bool { t.Helper() @@ -278,7 +284,7 @@ func TestControllingSelector_PingCandidate_BuildError(t *testing.T) { // make Username really big so stun.Build returns an error. a.remoteUfrag = bigStr() a.localUfrag = bigStr() - a.remotePwd = "pwd" + a.remotePwd = selectionTestPassword a.tieBreaker = 1 testLogger := &testICELogger{} @@ -297,7 +303,7 @@ func TestControlledSelector_PingCandidate_BuildError(t *testing.T) { a := bareAgentForPing() a.remoteUfrag = bigStr() a.localUfrag = bigStr() - a.remotePwd = "pwd" + a.remotePwd = selectionTestPassword a.tieBreaker = 1 testLogger := &testICELogger{} @@ -357,3 +363,978 @@ func TestControlledSelector_HandleSuccessResponse_UnknownTxID(t *testing.T) { require.True(t, logger.warned, "expected Warnf to be called for unknown TransactionID (hitting !ok branch)") } + +func TestAutomaticRenomination(t *testing.T) { //nolint:maintidx + report := test.CheckRoutines(t) + defer report() + + t.Run("Configuration", func(t *testing.T) { + t.Run("WithAutomaticRenomination enables feature", func(t *testing.T) { + agent, err := NewAgentWithOptions( + WithRenomination(DefaultNominationValueGenerator()), + WithAutomaticRenomination(5*time.Second), + ) + require.NoError(t, err) + defer func() { + require.NoError(t, agent.Close()) + }() + + assert.True(t, agent.automaticRenomination) + assert.Equal(t, 5*time.Second, agent.renominationInterval) + assert.True(t, agent.enableRenomination) + }) + + t.Run("Default interval when zero", func(t *testing.T) { + agent, err := NewAgentWithOptions( + WithRenomination(DefaultNominationValueGenerator()), + WithAutomaticRenomination(0), + ) + require.NoError(t, err) + defer func() { + require.NoError(t, agent.Close()) + }() + + assert.True(t, agent.automaticRenomination) + assert.Equal(t, 3*time.Second, agent.renominationInterval) + }) + }) + + t.Run("Quality Assessment", func(t *testing.T) { + agent, err := NewAgent(&AgentConfig{}) + require.NoError(t, err) + defer func() { + require.NoError(t, agent.Close()) + }() + + localHost, err := NewCandidateHost(&CandidateHostConfig{ + Network: "udp", + Address: "192.168.1.1", + Port: 10000, + Component: 1, + }) + require.NoError(t, err) + + remoteHost, err := NewCandidateHost(&CandidateHostConfig{ + Network: "udp", + Address: "192.168.1.2", + Port: 20000, + Component: 1, + }) + require.NoError(t, err) + + localRelay, err := NewCandidateRelay(&CandidateRelayConfig{ + Network: "udp", + Address: "10.0.0.1", + Port: 30000, + Component: 1, + RelAddr: "192.168.1.1", + RelPort: 10000, + }) + require.NoError(t, err) + + remoteRelay, err := NewCandidateRelay(&CandidateRelayConfig{ + Network: "udp", + Address: "10.0.0.2", + Port: 40000, + Component: 1, + RelAddr: "192.168.1.2", + RelPort: 20000, + }) + require.NoError(t, err) + + t.Run("Host pair scores higher than relay pair", func(t *testing.T) { + hostPair := newCandidatePair(localHost, remoteHost, true) + hostPair.state = CandidatePairStateSucceeded + hostPair.UpdateRoundTripTime(10 * time.Millisecond) + + relayPair := newCandidatePair(localRelay, remoteRelay, true) + relayPair.state = CandidatePairStateSucceeded + relayPair.UpdateRoundTripTime(10 * time.Millisecond) + + hostScore := agent.evaluateCandidatePairQuality(hostPair) + relayScore := agent.evaluateCandidatePairQuality(relayPair) + + assert.Greater(t, hostScore, relayScore, + "Host pair should score higher than relay pair with same RTT") + }) + + t.Run("Lower RTT scores higher", func(t *testing.T) { + pair1 := newCandidatePair(localHost, remoteHost, true) + pair1.state = CandidatePairStateSucceeded + pair1.UpdateRoundTripTime(5 * time.Millisecond) + + pair2 := newCandidatePair(localHost, remoteHost, true) + pair2.state = CandidatePairStateSucceeded + pair2.UpdateRoundTripTime(50 * time.Millisecond) + + score1 := agent.evaluateCandidatePairQuality(pair1) + score2 := agent.evaluateCandidatePairQuality(pair2) + + assert.Greater(t, score1, score2, + "Pair with lower RTT should score higher") + }) + }) + + t.Run("Should Renominate Logic", func(t *testing.T) { + agent, err := NewAgent(&AgentConfig{}) + require.NoError(t, err) + defer func() { + require.NoError(t, agent.Close()) + }() + + localHost, err := NewCandidateHost(&CandidateHostConfig{ + Network: "udp", + Address: "192.168.1.1", + Port: 10000, + Component: 1, + }) + require.NoError(t, err) + + remoteHost, err := NewCandidateHost(&CandidateHostConfig{ + Network: "udp", + Address: "192.168.1.2", + Port: 20000, + Component: 1, + }) + require.NoError(t, err) + + localRelay, err := NewCandidateRelay(&CandidateRelayConfig{ + Network: "udp", + Address: "10.0.0.1", + Port: 30000, + Component: 1, + RelAddr: "192.168.1.1", + RelPort: 10000, + }) + require.NoError(t, err) + + remoteRelay, err := NewCandidateRelay(&CandidateRelayConfig{ + Network: "udp", + Address: "10.0.0.2", + Port: 40000, + Component: 1, + RelAddr: "192.168.1.2", + RelPort: 20000, + }) + require.NoError(t, err) + + t.Run("Should renominate relay to host", func(t *testing.T) { + relayPair := newCandidatePair(localRelay, remoteRelay, true) + relayPair.state = CandidatePairStateSucceeded + relayPair.UpdateRoundTripTime(50 * time.Millisecond) + + hostPair := newCandidatePair(localHost, remoteHost, true) + hostPair.state = CandidatePairStateSucceeded + hostPair.UpdateRoundTripTime(45 * time.Millisecond) // Similar RTT + + shouldSwitch := agent.shouldRenominate(relayPair, hostPair) + assert.True(t, shouldSwitch, + "Should renominate from relay to host even with similar RTT") + }) + + t.Run("Should renominate for RTT improvement > 10ms", func(t *testing.T) { + // Create different host candidates for pair2 to avoid same-pair check + localHost2, err := NewCandidateHost(&CandidateHostConfig{ + Network: "udp", + Address: "192.168.1.3", + Port: 10001, + Component: 1, + }) + require.NoError(t, err) + + pair1 := newCandidatePair(localHost, remoteHost, true) + pair1.state = CandidatePairStateSucceeded + pair1.UpdateRoundTripTime(50 * time.Millisecond) + + pair2 := newCandidatePair(localHost2, remoteHost, true) + pair2.state = CandidatePairStateSucceeded + pair2.UpdateRoundTripTime(30 * time.Millisecond) // 20ms improvement + + shouldSwitch := agent.shouldRenominate(pair1, pair2) + assert.True(t, shouldSwitch, + "Should renominate for RTT improvement > 10ms") + }) + + t.Run("Should not renominate for small RTT improvement", func(t *testing.T) { + // Create different host candidates for pair2 to avoid same-pair check + localHost2, err := NewCandidateHost(&CandidateHostConfig{ + Network: "udp", + Address: "192.168.1.3", + Port: 10001, + Component: 1, + }) + require.NoError(t, err) + + pair1 := newCandidatePair(localHost, remoteHost, true) + pair1.state = CandidatePairStateSucceeded + pair1.UpdateRoundTripTime(50 * time.Millisecond) + + pair2 := newCandidatePair(localHost2, remoteHost, true) + pair2.state = CandidatePairStateSucceeded + pair2.UpdateRoundTripTime(45 * time.Millisecond) // Only 5ms improvement + + shouldSwitch := agent.shouldRenominate(pair1, pair2) + assert.False(t, shouldSwitch, + "Should not renominate for RTT improvement < 10ms") + }) + + t.Run("Should not renominate to same pair", func(t *testing.T) { + pair := newCandidatePair(localHost, remoteHost, true) + pair.state = CandidatePairStateSucceeded + + shouldSwitch := agent.shouldRenominate(pair, pair) + assert.False(t, shouldSwitch, + "Should not renominate to the same pair") + }) + + t.Run("Should not renominate to non-succeeded pair", func(t *testing.T) { + currentPair := newCandidatePair(localHost, remoteHost, true) + currentPair.state = CandidatePairStateSucceeded + + candidatePair := newCandidatePair(localHost, remoteHost, true) + candidatePair.state = CandidatePairStateInProgress + + shouldSwitch := agent.shouldRenominate(currentPair, candidatePair) + assert.False(t, shouldSwitch, + "Should not renominate to non-succeeded pair") + }) + }) + + t.Run("Find Best Candidate Pair", func(t *testing.T) { + agent, err := NewAgent(&AgentConfig{}) + require.NoError(t, err) + defer func() { + require.NoError(t, agent.Close()) + }() + + // Create candidates + localHost, err := NewCandidateHost(&CandidateHostConfig{ + Network: "udp", + Address: "192.168.1.1", + Port: 10000, + Component: 1, + }) + require.NoError(t, err) + + remoteHost, err := NewCandidateHost(&CandidateHostConfig{ + Network: "udp", + Address: "192.168.1.2", + Port: 20000, + Component: 1, + }) + require.NoError(t, err) + + localRelay, err := NewCandidateRelay(&CandidateRelayConfig{ + Network: "udp", + Address: "10.0.0.1", + Port: 30000, + Component: 1, + RelAddr: "192.168.1.1", + RelPort: 10000, + }) + require.NoError(t, err) + + remoteRelay, err := NewCandidateRelay(&CandidateRelayConfig{ + Network: "udp", + Address: "10.0.0.2", + Port: 40000, + Component: 1, + RelAddr: "192.168.1.2", + RelPort: 20000, + }) + require.NoError(t, err) + + ctx := context.Background() + err = agent.loop.Run(ctx, func(context.Context) { + // Add pairs to checklist + hostPair := agent.addPair(localHost, remoteHost) + hostPair.state = CandidatePairStateSucceeded + hostPair.UpdateRoundTripTime(10 * time.Millisecond) + + relayPair := agent.addPair(localRelay, remoteRelay) + relayPair.state = CandidatePairStateSucceeded + relayPair.UpdateRoundTripTime(50 * time.Millisecond) + + // Find best should return host pair + best := agent.findBestCandidatePair() + assert.NotNil(t, best) + assert.Equal(t, hostPair, best, + "Best pair should be the host pair with lower latency") + }) + require.NoError(t, err) + }) +} + +func TestAutomaticRenominationIntegration(t *testing.T) { //nolint:cyclop + report := test.CheckRoutines(t) + defer report() + + t.Run("Automatic renomination triggers after interval", func(t *testing.T) { + // Create agents with automatic renomination enabled + aAgent, err := NewAgentWithOptions( + WithRenomination(DefaultNominationValueGenerator()), + WithAutomaticRenomination(100*time.Millisecond), // Short interval for testing + ) + require.NoError(t, err) + defer func() { + require.NoError(t, aAgent.Close()) + }() + + bAgent, err := NewAgentWithOptions( + WithRenomination(DefaultNominationValueGenerator()), + ) + require.NoError(t, err) + defer func() { + require.NoError(t, bAgent.Close()) + }() + + // Start gathering candidates + err = aAgent.OnCandidate(func(c Candidate) { + if c != nil { + t.Logf("Agent A gathered candidate: %s", c) + } + }) + require.NoError(t, err) + + err = bAgent.OnCandidate(func(c Candidate) { + if c != nil { + t.Logf("Agent B gathered candidate: %s", c) + } + }) + require.NoError(t, err) + + require.NoError(t, aAgent.GatherCandidates()) + require.NoError(t, bAgent.GatherCandidates()) + + // Wait for gathering to complete + time.Sleep(100 * time.Millisecond) + + // Exchange credentials + aUfrag, aPwd, err := aAgent.GetLocalUserCredentials() + require.NoError(t, err) + bUfrag, bPwd, err := bAgent.GetLocalUserCredentials() + require.NoError(t, err) + + // Get candidates + aCandidates, err := aAgent.GetLocalCandidates() + require.NoError(t, err) + bCandidates, err := bAgent.GetLocalCandidates() + require.NoError(t, err) + + // Verify we have candidates + if len(aCandidates) == 0 || len(bCandidates) == 0 { + t.Skip("No candidates gathered, skipping integration test") + } + + require.NoError(t, aAgent.startConnectivityChecks(true, bUfrag, bPwd)) + require.NoError(t, bAgent.startConnectivityChecks(false, aUfrag, aPwd)) + + // Exchange candidates + for _, c := range aCandidates { + cpCand, copyErr := c.copy() + require.NoError(t, copyErr) + require.NoError(t, bAgent.AddRemoteCandidate(cpCand)) + } + for _, c := range bCandidates { + cpCand, copyErr := c.copy() + require.NoError(t, copyErr) + require.NoError(t, aAgent.AddRemoteCandidate(cpCand)) + } + + // Wait for initial connection + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + // Wait for connection on both agents + select { + case <-aAgent.onConnected: + case <-ctx.Done(): + require.Fail(t, "Agent A failed to connect") + } + + select { + case <-bAgent.onConnected: + case <-ctx.Done(): + require.Fail(t, "Agent B failed to connect") + } + + // Record initial selected pairs + initialAPair, err := aAgent.GetSelectedCandidatePair() + require.NoError(t, err) + require.NotNil(t, initialAPair) + + // Note: In a real scenario, automatic renomination would trigger + // when a better path becomes available (e.g., relay -> direct). + // For this test, we're just verifying the mechanism is in place. + + // Wait to see if automatic renomination check runs + // (it should run but may not renominate if no better pair exists) + time.Sleep(200 * time.Millisecond) + + // The automatic renomination check should have run at least once + // We can't easily verify renomination occurred without simulating + // network changes, but we can verify the feature is enabled + assert.True(t, aAgent.automaticRenomination) + assert.True(t, aAgent.enableRenomination) + }) +} + +func TestKeepAliveCandidatesForRenomination(t *testing.T) { + report := test.CheckRoutines(t) + defer report() + + // Create test candidates that don't require real network I/O + createTestCandidates := func() (Candidate, Candidate, Candidate) { + local1 := newPingNoIOCand() + local1.candidateBase.networkType = NetworkTypeUDP4 + local1.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.1"), Port: 10000} + + local2 := newPingNoIOCand() + local2.candidateBase.networkType = NetworkTypeUDP4 + local2.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.3"), Port: 10001} + + remote := newPingNoIOCand() + remote.candidateBase.networkType = NetworkTypeUDP4 + remote.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.2"), Port: 20000} + + return local1, local2, remote + } + + t.Run("Only pings all candidates when automatic renomination enabled", func(t *testing.T) { + localHost1, localHost2, remoteHost := createTestCandidates() + + // Test with automatic renomination DISABLED + agentWithoutAutoRenom := bareAgentForPing() + agentWithoutAutoRenom.log = logging.NewDefaultLoggerFactory().NewLogger("test") + agentWithoutAutoRenom.remoteUfrag = selectionTestRemoteUfrag + agentWithoutAutoRenom.localUfrag = selectionTestLocalUfrag + agentWithoutAutoRenom.remotePwd = selectionTestPassword + agentWithoutAutoRenom.tieBreaker = 1 + agentWithoutAutoRenom.isControlling.Store(true) + agentWithoutAutoRenom.setSelector() + + // Add pairs - one selected (succeeded) and one alternate (succeeded) + pair1 := agentWithoutAutoRenom.addPair(localHost1, remoteHost) + pair1.state = CandidatePairStateSucceeded + pair1.UpdateRoundTripTime(10 * time.Millisecond) + // Don't set selected pair for the "without renomination" agent + + pair2 := agentWithoutAutoRenom.addPair(localHost2, remoteHost) + pair2.state = CandidatePairStateSucceeded + pair2.UpdateRoundTripTime(50 * time.Millisecond) + + // keepAliveCandidatesForRenomination should do nothing when automatic renomination is disabled + agentWithoutAutoRenom.keepAliveCandidatesForRenomination() + + // Since automatic renomination is off, the function should not ping anything + // We can't easily verify no pings were sent, but we verify the function completes + + // Test with automatic renomination ENABLED + agentWithAutoRenom := bareAgentForPing() + agentWithAutoRenom.log = logging.NewDefaultLoggerFactory().NewLogger("test") + agentWithAutoRenom.automaticRenomination = true + agentWithAutoRenom.enableRenomination = true + agentWithAutoRenom.renominationInterval = 100 * time.Millisecond + agentWithAutoRenom.remoteUfrag = selectionTestRemoteUfrag + agentWithAutoRenom.localUfrag = selectionTestLocalUfrag + agentWithAutoRenom.remotePwd = selectionTestPassword + agentWithAutoRenom.tieBreaker = 1 + agentWithAutoRenom.isControlling.Store(true) + agentWithAutoRenom.setSelector() + + // Add pairs with different states + pair1 = agentWithAutoRenom.addPair(localHost1, remoteHost) + pair1.state = CandidatePairStateSucceeded + pair1.UpdateRoundTripTime(10 * time.Millisecond) + // Don't set selected pair for this test + + pair2 = agentWithAutoRenom.addPair(localHost2, remoteHost) + pair2.state = CandidatePairStateSucceeded + pair2.UpdateRoundTripTime(50 * time.Millisecond) + + // Call keepAliveCandidatesForRenomination - should ping all pairs + agentWithAutoRenom.keepAliveCandidatesForRenomination() + + // Verify both pairs remain in succeeded state (not changed by the function) + assert.Equal(t, CandidatePairStateSucceeded, pair1.state) + assert.Equal(t, CandidatePairStateSucceeded, pair2.state) + }) + + t.Run("Pings succeeded pairs unlike pingAllCandidates", func(t *testing.T) { + localHost1, localHost2, remoteHost := createTestCandidates() + + agent := bareAgentForPing() + agent.log = logging.NewDefaultLoggerFactory().NewLogger("test") + agent.automaticRenomination = true + agent.enableRenomination = true + agent.renominationInterval = 100 * time.Millisecond + agent.remoteUfrag = selectionTestRemoteUfrag + agent.localUfrag = selectionTestLocalUfrag + agent.remotePwd = selectionTestPassword + agent.tieBreaker = 1 + agent.isControlling.Store(true) + agent.setSelector() + + // Create a pair in succeeded state + pair := agent.addPair(localHost1, remoteHost) + pair.state = CandidatePairStateSucceeded + pair.UpdateRoundTripTime(10 * time.Millisecond) + + // Create another pair in succeeded state + pair2 := agent.addPair(localHost2, remoteHost) + pair2.state = CandidatePairStateSucceeded + pair2.UpdateRoundTripTime(50 * time.Millisecond) + + // keepAliveCandidatesForRenomination should ping succeeded pairs + // (pingAllCandidates would skip them) + agent.keepAliveCandidatesForRenomination() + + // Pairs should still be in succeeded state + assert.Equal(t, CandidatePairStateSucceeded, pair.state) + assert.Equal(t, CandidatePairStateSucceeded, pair2.state) + }) + + t.Run("Transitions waiting pairs to in-progress", func(t *testing.T) { + localHost1, _, remoteHost := createTestCandidates() + + agent := bareAgentForPing() + agent.log = logging.NewDefaultLoggerFactory().NewLogger("test") + agent.automaticRenomination = true + agent.enableRenomination = true + agent.renominationInterval = 100 * time.Millisecond + agent.remoteUfrag = selectionTestRemoteUfrag + agent.localUfrag = selectionTestLocalUfrag + agent.remotePwd = selectionTestPassword + agent.tieBreaker = 1 + agent.isControlling.Store(true) + agent.setSelector() + + // Create a pair in waiting state + pair := agent.addPair(localHost1, remoteHost) + pair.state = CandidatePairStateWaiting + + // Call keepAliveCandidatesForRenomination + agent.keepAliveCandidatesForRenomination() + + // Pair should transition to in-progress + assert.Equal(t, CandidatePairStateInProgress, pair.state) + }) + + t.Run("Skips failed pairs", func(t *testing.T) { + localHost1, localHost2, remoteHost := createTestCandidates() + + agent := bareAgentForPing() + agent.log = logging.NewDefaultLoggerFactory().NewLogger("test") + agent.automaticRenomination = true + agent.enableRenomination = true + agent.renominationInterval = 100 * time.Millisecond + agent.remoteUfrag = selectionTestRemoteUfrag + agent.localUfrag = selectionTestLocalUfrag + agent.remotePwd = selectionTestPassword + agent.tieBreaker = 1 + agent.isControlling.Store(true) + agent.setSelector() + + // Create a succeeded pair + pair1 := agent.addPair(localHost1, remoteHost) + pair1.state = CandidatePairStateSucceeded + + // Create a failed pair + pair2 := agent.addPair(localHost2, remoteHost) + pair2.state = CandidatePairStateFailed + + // Call keepAliveCandidatesForRenomination + agent.keepAliveCandidatesForRenomination() + + // Failed pair should remain failed (not transitioned to in-progress) + assert.Equal(t, CandidatePairStateFailed, pair2.state) + // Succeeded pair should remain succeeded + assert.Equal(t, CandidatePairStateSucceeded, pair1.state) + }) +} + +// TestRenominationAcceptance verifies that the controlled agent correctly +// accepts renomination based on nomination values, not just priority. +func TestRenominationAcceptance(t *testing.T) { + t.Run("Accepts renomination with nomination value regardless of priority", func(t *testing.T) { + // Create a controlled agent + agent := bareAgentForPing() + agent.log = logging.NewDefaultLoggerFactory().NewLogger("test") + agent.remoteUfrag = selectionTestRemoteUfrag + agent.localUfrag = selectionTestLocalUfrag + agent.remotePwd = selectionTestPassword + agent.tieBreaker = 1 + agent.isControlling.Store(false) // Controlled agent + agent.nominationAttribute = stun.AttrType(0x0030) + agent.onConnected = make(chan struct{}) // Initialize the channel + agent.setSelector() + + selector, ok := agent.getSelector().(*controlledSelector) + require.True(t, ok, "expected controlledSelector") + + // Create two host candidates with same priority + local1 := newPingNoIOCand() + local1.candidateBase.networkType = NetworkTypeUDP4 + local1.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.1"), Port: 10000} + + local2 := newPingNoIOCand() + local2.candidateBase.networkType = NetworkTypeUDP4 + local2.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.3"), Port: 10001} + + remote := newPingNoIOCand() + remote.candidateBase.networkType = NetworkTypeUDP4 + remote.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.2"), Port: 20000} + + // Create two pairs with the same priority (both host candidates) + pair1 := agent.addPair(local1, remote) + pair1.state = CandidatePairStateSucceeded + + pair2 := agent.addPair(local2, remote) + pair2.state = CandidatePairStateSucceeded + + // Select the first pair initially + agent.setSelectedPair(pair1) + assert.Equal(t, pair1, agent.getSelectedPair()) + assert.True(t, pair1.nominated) + + // Build a nomination request for the second pair with a nomination value + nominationValue := uint32(100) + msg, err := stun.Build(stun.BindingRequest, + stun.TransactionID, + stun.NewUsername(agent.localUfrag+":"+agent.remoteUfrag), + UseCandidate(), + NominationSetter{ + Value: nominationValue, + AttrType: agent.nominationAttribute, + }, + stun.NewShortTermIntegrity(agent.localPwd), + stun.Fingerprint, + ) + require.NoError(t, err) + + // Handle the binding request with nomination value for pair2 + selector.HandleBindingRequest(msg, local2, remote) + + // The controlled agent should accept the renomination even though + // pair2 has the same priority as pair1, because a nomination value is present + selectedPair := agent.getSelectedPair() + assert.Equal(t, pair2, selectedPair, + "Should switch to pair2 when renomination with nomination value is received") + assert.True(t, pair2.nominated) + }) + + t.Run("Standard nomination still requires higher priority without nomination value", func(t *testing.T) { + // Create a controlled agent + agent := bareAgentForPing() + agent.log = logging.NewDefaultLoggerFactory().NewLogger("test") + agent.remoteUfrag = selectionTestRemoteUfrag + agent.localUfrag = selectionTestLocalUfrag + agent.remotePwd = selectionTestPassword + agent.tieBreaker = 1 + agent.isControlling.Store(false) + agent.onConnected = make(chan struct{}) + agent.setSelector() + + selector, ok := agent.getSelector().(*controlledSelector) + require.True(t, ok, "expected controlledSelector") + + // Create candidates - we'll simulate lower priority by using different types + local1 := newPingNoIOCand() + local1.candidateBase.networkType = NetworkTypeUDP4 + local1.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.1"), Port: 10000} + local1.candidateBase.candidateType = CandidateTypeHost // Higher priority + + local2 := newPingNoIOCand() + local2.candidateBase.networkType = NetworkTypeUDP4 + local2.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.3"), Port: 10001} + local2.candidateBase.candidateType = CandidateTypeHost // Same priority + + remote := newPingNoIOCand() + remote.candidateBase.networkType = NetworkTypeUDP4 + remote.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.2"), Port: 20000} + remote.candidateBase.candidateType = CandidateTypeHost + + pair1 := agent.addPair(local1, remote) + pair1.state = CandidatePairStateSucceeded + + pair2 := agent.addPair(local2, remote) + pair2.state = CandidatePairStateSucceeded + + // Select the first pair + agent.setSelectedPair(pair1) + + // Build a standard nomination request WITHOUT nomination value for pair2 + msg, err := stun.Build(stun.BindingRequest, + stun.TransactionID, + stun.NewUsername(agent.localUfrag+":"+agent.remoteUfrag), + UseCandidate(), + stun.NewShortTermIntegrity(agent.localPwd), + stun.Fingerprint, + ) + require.NoError(t, err) + + // Handle the binding request for pair2 (same priority) + selector.HandleBindingRequest(msg, local2, remote) + + // Without renomination, standard ICE rules apply + // Since pair2 has equal priority to pair1, it should NOT be accepted + // (only higher priority pairs are accepted in standard ICE) + selectedPair := agent.getSelectedPair() + assert.Equal(t, pair1, selectedPair, + "Should NOT switch to pair2 with standard nomination when priority is equal") + }) + + t.Run("Higher nomination values override lower ones", func(t *testing.T) { + agent := bareAgentForPing() + agent.log = logging.NewDefaultLoggerFactory().NewLogger("test") + agent.remoteUfrag = selectionTestRemoteUfrag + agent.localUfrag = selectionTestLocalUfrag + agent.remotePwd = selectionTestPassword + agent.tieBreaker = 1 + agent.isControlling.Store(false) + agent.nominationAttribute = stun.AttrType(0x0030) + agent.onConnected = make(chan struct{}) + agent.setSelector() + + selector, ok := agent.getSelector().(*controlledSelector) + require.True(t, ok, "expected controlledSelector") + + local1 := newPingNoIOCand() + local1.candidateBase.networkType = NetworkTypeUDP4 + local1.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.1"), Port: 10000} + + local2 := newPingNoIOCand() + local2.candidateBase.networkType = NetworkTypeUDP4 + local2.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.3"), Port: 10001} + + local3 := newPingNoIOCand() + local3.candidateBase.networkType = NetworkTypeUDP4 + local3.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.4"), Port: 10002} + + remote := newPingNoIOCand() + remote.candidateBase.networkType = NetworkTypeUDP4 + remote.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.2"), Port: 20000} + + pair1 := agent.addPair(local1, remote) + pair1.state = CandidatePairStateSucceeded + + pair2 := agent.addPair(local2, remote) + pair2.state = CandidatePairStateSucceeded + + pair3 := agent.addPair(local3, remote) + pair3.state = CandidatePairStateSucceeded + + // Nominate pair1 with value 100 + msg1, err := stun.Build(stun.BindingRequest, + stun.TransactionID, + stun.NewUsername(agent.localUfrag+":"+agent.remoteUfrag), + UseCandidate(), + NominationSetter{Value: 100, AttrType: agent.nominationAttribute}, + stun.NewShortTermIntegrity(agent.localPwd), + stun.Fingerprint, + ) + require.NoError(t, err) + selector.HandleBindingRequest(msg1, local1, remote) + assert.Equal(t, pair1, agent.getSelectedPair()) + + // Try to nominate pair2 with a LOWER value (50) - should be rejected + msg2, err := stun.Build(stun.BindingRequest, + stun.TransactionID, + stun.NewUsername(agent.localUfrag+":"+agent.remoteUfrag), + UseCandidate(), + NominationSetter{Value: 50, AttrType: agent.nominationAttribute}, + stun.NewShortTermIntegrity(agent.localPwd), + stun.Fingerprint, + ) + require.NoError(t, err) + selector.HandleBindingRequest(msg2, local2, remote) + assert.Equal(t, pair1, agent.getSelectedPair(), + "Should reject nomination with lower value") + + // Nominate pair3 with a HIGHER value (200) - should be accepted + msg3, err := stun.Build(stun.BindingRequest, + stun.TransactionID, + stun.NewUsername(agent.localUfrag+":"+agent.remoteUfrag), + UseCandidate(), + NominationSetter{Value: 200, AttrType: agent.nominationAttribute}, + stun.NewShortTermIntegrity(agent.localPwd), + stun.Fingerprint, + ) + require.NoError(t, err) + selector.HandleBindingRequest(msg3, local3, remote) + assert.Equal(t, pair3, agent.getSelectedPair(), + "Should accept nomination with higher value") + }) +} + +// TestControllingSideRenomination verifies that the controlling agent correctly +// updates its selected pair when receiving a success response for a renomination request. +func TestControllingSideRenomination(t *testing.T) { + t.Run("Switches selected pair on renomination success response", func(t *testing.T) { + // Create a controlling agent + agent := bareAgentForPing() + agent.log = logging.NewDefaultLoggerFactory().NewLogger("test") + agent.remoteUfrag = selectionTestRemoteUfrag + agent.localUfrag = selectionTestLocalUfrag + agent.remotePwd = selectionTestPassword + agent.tieBreaker = 1 + agent.isControlling.Store(true) // Controlling agent + agent.nominationAttribute = stun.AttrType(0x0030) + agent.onConnected = make(chan struct{}) // Initialize the channel + agent.setSelector() + + selector, ok := agent.getSelector().(*controllingSelector) + require.True(t, ok, "expected controllingSelector") + + // Create two host candidates + local1 := newPingNoIOCand() + local1.candidateBase.networkType = NetworkTypeUDP4 + local1.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.1"), Port: 10000} + + local2 := newPingNoIOCand() + local2.candidateBase.networkType = NetworkTypeUDP4 + local2.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.3"), Port: 10001} + + remote := newPingNoIOCand() + remote.candidateBase.networkType = NetworkTypeUDP4 + remote.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.2"), Port: 20000} + + // Create two pairs + pair1 := agent.addPair(local1, remote) + pair1.state = CandidatePairStateSucceeded + pair1.UpdateRoundTripTime(10 * time.Millisecond) + + pair2 := agent.addPair(local2, remote) + pair2.state = CandidatePairStateSucceeded + pair2.UpdateRoundTripTime(5 * time.Millisecond) + + // Select the first pair initially + agent.setSelectedPair(pair1) + assert.Equal(t, pair1, agent.getSelectedPair()) + assert.True(t, pair1.nominated) + + // Build a renomination request with nomination value for pair2 + nominationValue := uint32(100) + msg, err := stun.Build(stun.BindingRequest, + stun.TransactionID, + stun.NewUsername(agent.remoteUfrag+":"+agent.localUfrag), + UseCandidate(), + AttrControlling(agent.tieBreaker), + PriorityAttr(local2.Priority()), + NominationSetter{ + Value: nominationValue, + AttrType: agent.nominationAttribute, + }, + stun.NewShortTermIntegrity(agent.remotePwd), + stun.Fingerprint, + ) + require.NoError(t, err) + + // Simulate sending the binding request (adds to pendingBindingRequests) + agent.sendBindingRequest(msg, local2, remote) + + // Verify the nomination value was stored in the pending request + require.Len(t, agent.pendingBindingRequests, 1) + require.NotNil(t, agent.pendingBindingRequests[0].nominationValue) + require.Equal(t, nominationValue, *agent.pendingBindingRequests[0].nominationValue) + + // Build a success response + successMsg, err := stun.Build(msg, stun.BindingSuccess, + &stun.XORMappedAddress{ + IP: net.ParseIP("192.168.1.2").To4(), + Port: 20000, + }, + stun.NewShortTermIntegrity(agent.remotePwd), + stun.Fingerprint, + ) + require.NoError(t, err) + + // Handle the success response - this should switch to pair2 + selector.HandleSuccessResponse(successMsg, local2, remote, remote.addr()) + + // The controlling agent should have switched to pair2 + selectedPair := agent.getSelectedPair() + assert.Equal(t, pair2, selectedPair, + "Controlling agent should switch to pair2 after renomination success response") + assert.True(t, pair2.nominated) + }) + + t.Run("Does not switch on standard nomination success if pair already selected", func(t *testing.T) { + // Create a controlling agent + agent := bareAgentForPing() + agent.log = logging.NewDefaultLoggerFactory().NewLogger("test") + agent.remoteUfrag = selectionTestRemoteUfrag + agent.localUfrag = selectionTestLocalUfrag + agent.remotePwd = selectionTestPassword + agent.tieBreaker = 1 + agent.isControlling.Store(true) + agent.onConnected = make(chan struct{}) + agent.setSelector() + + selector, ok := agent.getSelector().(*controllingSelector) + require.True(t, ok, "expected controllingSelector") + + // Create two host candidates + local1 := newPingNoIOCand() + local1.candidateBase.networkType = NetworkTypeUDP4 + local1.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.1"), Port: 10000} + + local2 := newPingNoIOCand() + local2.candidateBase.networkType = NetworkTypeUDP4 + local2.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.3"), Port: 10001} + + remote := newPingNoIOCand() + remote.candidateBase.networkType = NetworkTypeUDP4 + remote.candidateBase.resolvedAddr = &net.UDPAddr{IP: net.ParseIP("192.168.1.2"), Port: 20000} + + // Create two pairs + pair1 := agent.addPair(local1, remote) + pair1.state = CandidatePairStateSucceeded + + pair2 := agent.addPair(local2, remote) + pair2.state = CandidatePairStateSucceeded + + // Select the first pair initially + agent.setSelectedPair(pair1) + assert.Equal(t, pair1, agent.getSelectedPair()) + + // Build a standard nomination request WITHOUT nomination value for pair2 + msg, err := stun.Build(stun.BindingRequest, + stun.TransactionID, + stun.NewUsername(agent.remoteUfrag+":"+agent.localUfrag), + UseCandidate(), + AttrControlling(agent.tieBreaker), + PriorityAttr(local2.Priority()), + stun.NewShortTermIntegrity(agent.remotePwd), + stun.Fingerprint, + ) + require.NoError(t, err) + + // Simulate sending the binding request + agent.sendBindingRequest(msg, local2, remote) + + // Verify no nomination value was stored + require.Len(t, agent.pendingBindingRequests, 1) + require.Nil(t, agent.pendingBindingRequests[0].nominationValue) + + // Build a success response + successMsg, err := stun.Build(msg, stun.BindingSuccess, + &stun.XORMappedAddress{ + IP: net.ParseIP("192.168.1.2").To4(), + Port: 20000, + }, + stun.NewShortTermIntegrity(agent.remotePwd), + stun.Fingerprint, + ) + require.NoError(t, err) + + // Handle the success response - this should NOT switch since it's standard nomination + // and a pair is already selected + selector.HandleSuccessResponse(successMsg, local2, remote, remote.addr()) + + // The controlling agent should remain with pair1 + selectedPair := agent.getSelectedPair() + assert.Equal(t, pair1, selectedPair, + "Controlling agent should NOT switch with standard nomination when pair already selected") + }) +}