Repository: pion/ice (go)

Issue:
TCP ICE candidates should have lower preference than UDP
Currently their priority is higher thank UDP srflx / prflx, which results with TCP prflx -> TCP passive host candidates being used before UDP prflx / UDP srflx pairs.

Related to #245.

Pull request description:
Make TCP ICE candidates have lower preference
Closes #262.

Adds the changes in determining local preference for ICE TCP candidates according to RFC 6544, section 4.2.

https://tools.ietf.org/html/rfc6544#section-4.2

The only thing I'm not sure about is the value of `otherPref`, which is currently always set to `8191`, but the RFC says the following:

> If any two candidates have the same type-preference and direction-pref, they MUST have a unique other-pref.  With this specification, this usually only happens with multi-homed hosts, in which case other-pref is the preference for the particular IP address from which the candidate was obtained.  When there is only a single IP address, this value SHOULD be set to the maximum allowed value (8191).

Patch:
diff --git a/candidate_base.go b/candidate_base.go
index 12313cd..59377f0 100644
--- a/candidate_base.go
+++ b/candidate_base.go
@@ -90,6 +90,72 @@ func (c *candidateBase) Component() uint16 {
 
 // LocalPreference returns the local preference for this candidate
 func (c *candidateBase) LocalPreference() uint16 {
+	if c.NetworkType().IsTCP() {
+		// RFC 6544, section 4.2
+		//
+		// In Section 4.1.2.1 of [RFC5245], a recommended formula for UDP ICE
+		// candidate prioritization is defined.  For TCP candidates, the same
+		// formula and candidate type preferences SHOULD be used, and the
+		// RECOMMENDED type preferences for the new candidate types defined in
+		// this document (see Section 5) are 105 for NAT-assisted candidates and
+		// 75 for UDP-tunneled candidates.
+		//
+		// (...)
+		//
+		// With TCP candidates, the local preference part of the recommended
+		// priority formula is updated to also include the directionality
+		// (active, passive, or simultaneous-open) of the TCP connection.  The
+		// RECOMMENDED local preference is then defined as:
+		//
+		//     local preference = (2^13) * direction-pref + other-pref
+		//
+		// The direction-pref MUST be between 0 and 7 (both inclusive), with 7
+		// being the most preferred.  The other-pref MUST be between 0 and 8191
+		// (both inclusive), with 8191 being the most preferred.  It is
+		// RECOMMENDED that the host, UDP-tunneled, and relayed TCP candidates
+		// have the direction-pref assigned as follows: 6 for active, 4 for
+		// passive, and 2 for S-O.  For the NAT-assisted and server reflexive
+		// candidates, the RECOMMENDED values are: 6 for S-O, 4 for active, and
+		// 2 for passive.
+		//
+		// (...)
+		//
+		// If any two candidates have the same type-preference and direction-
+		// pref, they MUST have a unique other-pref.  With this specification,
+		// this usually only happens with multi-homed hosts, in which case
+		// other-pref is the preference for the particular IP address from which
+		// the candidate was obtained.  When there is only a single IP address,
+		// this value SHOULD be set to the maximum allowed value (8191).
+		var otherPref uint16 = 8191
+
+		directionPref := func() uint16 {
+			switch c.Type() {
+			case CandidateTypeHost, CandidateTypeRelay:
+				switch c.tcpType {
+				case TCPTypeActive:
+					return 6
+				case TCPTypePassive:
+					return 4
+				case TCPTypeSimultaneousOpen:
+					return 2
+				}
+			case CandidateTypePeerReflexive, CandidateTypeServerReflexive:
+				switch c.tcpType {
+				case TCPTypeSimultaneousOpen:
+					return 6
+				case TCPTypeActive:
+					return 4
+				case TCPTypePassive:
+					return 2
+				}
+			}
+
+			return 0
+		}()
+
+		return (1<<13)*directionPref + otherPref
+	}
+
 	return defaultLocalPreference
 }
 


Test patch:
diff --git a/candidate_test.go b/candidate_test.go
index e3d41f6..fea3492 100644
--- a/candidate_test.go
+++ b/candidate_test.go
@@ -21,6 +21,39 @@ func TestCandidatePriority(t *testing.T) {
 			},
 			WantPriority: 2130706431,
 		},
+		{
+			Candidate: &CandidateHost{
+				candidateBase: candidateBase{
+					candidateType: CandidateTypeHost,
+					component:     ComponentRTP,
+					networkType:   NetworkTypeTCP4,
+					tcpType:       TCPTypeActive,
+				},
+			},
+			WantPriority: 2128609279,
+		},
+		{
+			Candidate: &CandidateHost{
+				candidateBase: candidateBase{
+					candidateType: CandidateTypeHost,
+					component:     ComponentRTP,
+					networkType:   NetworkTypeTCP4,
+					tcpType:       TCPTypePassive,
+				},
+			},
+			WantPriority: 2124414975,
+		},
+		{
+			Candidate: &CandidateHost{
+				candidateBase: candidateBase{
+					candidateType: CandidateTypeHost,
+					component:     ComponentRTP,
+					networkType:   NetworkTypeTCP4,
+					tcpType:       TCPTypeSimultaneousOpen,
+				},
+			},
+			WantPriority: 2120220671,
+		},
 		{
 			Candidate: &CandidatePeerReflexive{
 				candidateBase: candidateBase{
@@ -30,6 +63,39 @@ func TestCandidatePriority(t *testing.T) {
 			},
 			WantPriority: 1862270975,
 		},
+		{
+			Candidate: &CandidatePeerReflexive{
+				candidateBase: candidateBase{
+					candidateType: CandidateTypePeerReflexive,
+					component:     ComponentRTP,
+					networkType:   NetworkTypeTCP6,
+					tcpType:       TCPTypeSimultaneousOpen,
+				},
+			},
+			WantPriority: 1860173823,
+		},
+		{
+			Candidate: &CandidatePeerReflexive{
+				candidateBase: candidateBase{
+					candidateType: CandidateTypePeerReflexive,
+					component:     ComponentRTP,
+					networkType:   NetworkTypeTCP6,
+					tcpType:       TCPTypeActive,
+				},
+			},
+			WantPriority: 1855979519,
+		},
+		{
+			Candidate: &CandidatePeerReflexive{
+				candidateBase: candidateBase{
+					candidateType: CandidateTypePeerReflexive,
+					component:     ComponentRTP,
+					networkType:   NetworkTypeTCP6,
+					tcpType:       TCPTypePassive,
+				},
+			},
+			WantPriority: 1851785215,
+		},
 		{
 			Candidate: &CandidateServerReflexive{
 				candidateBase: candidateBase{