Minor style fixes for active TCP candidate support

This commit is contained in:
Steffen Vogel
2023-05-16 20:07:39 +02:00
parent 1d502ca6ec
commit 00bbd2954c
7 changed files with 116 additions and 96 deletions
+1 -1
View File
@@ -623,7 +623,7 @@ func (a *Agent) addPair(local, remote Candidate) *CandidatePair {
a.log.Errorf("Failed to cast local candidate to CandidateHost")
return nil
}
localCandidateHost.port = localAddress.Port // this causes a data race with candidateBase.Port()
localCandidateHost.port = localAddress.Port // This causes a data race with candidateBase.Port()
local.start(a, packetConn, a.startedCh)
}
p := newCandidatePair(local, remote, a.isControlling)
+7 -7
View File
@@ -48,6 +48,7 @@ func TestAgentActiveTCP(t *testing.T) {
listenIPAddress net.IP
selectedPairNetworkType string
}
testCases := []testCase{
{
name: "TCP4 connection",
@@ -56,7 +57,7 @@ func TestAgentActiveTCP(t *testing.T) {
selectedPairNetworkType: tcp,
},
{
name: "UDP is preferred over TCP4", // fails some time
name: "UDP is preferred over TCP4", // This fails some time
networkTypes: supportedNetworkTypes(),
listenIPAddress: getLocalIPAddress(t, NetworkTypeTCP4),
selectedPairNetworkType: udp,
@@ -64,21 +65,20 @@ func TestAgentActiveTCP(t *testing.T) {
}
if ipv6Available(t) {
tcpv6Cases := []testCase{
{
testCases = append(testCases,
testCase{
name: "TCP6 connection",
networkTypes: []NetworkType{NetworkTypeTCP6},
listenIPAddress: getLocalIPAddress(t, NetworkTypeTCP6),
selectedPairNetworkType: tcp,
},
{
name: "UDP is preferred over TCP6", // fails some time
testCase{
name: "UDP is preferred over TCP6", // This fails some time
networkTypes: supportedNetworkTypes(),
listenIPAddress: getLocalIPAddress(t, NetworkTypeTCP6),
selectedPairNetworkType: udp,
},
}
testCases = append(testCases, tcpv6Cases...)
)
}
for _, testCase := range testCases {
+38 -23
View File
@@ -188,6 +188,44 @@ func (c *candidateBase) LocalPreference() uint16 {
return defaultLocalPreference
}
// TypePreference returns the type preference for this candidate
func (c *candidateBase) TypePreference() uint16 {
pref := c.Type().Preference()
if pref == 0 {
return 0
}
if c.NetworkType().IsTCP() {
var tcpPriorityOffset uint16 = defaultTCPPriorityOffset
if c.agent() != nil {
tcpPriorityOffset = c.agent().tcpPriorityOffset
}
pref -= tcpPriorityOffset
}
return pref
}
// Priority computes the priority for this ICE Candidate
// See: https://www.rfc-editor.org/rfc/rfc8445#section-5.1.2.1
func (c *candidateBase) Priority() uint32 {
if c.priorityOverride != 0 {
return c.priorityOverride
}
// The local preference MUST be an integer from 0 (lowest preference) to
// 65535 (highest preference) inclusive. When there is only a single IP
// address, this value SHOULD be set to 65535. If there are multiple
// candidates for a particular component for a particular data stream
// that have the same type, the local preference MUST be unique for each
// one.
return (1<<24)*uint32(c.TypePreference()) +
(1<<8)*uint32(c.LocalPreference()) +
(1<<0)*uint32(256-c.Component())
}
// RelatedAddress returns *CandidateRelatedAddress
func (c *candidateBase) RelatedAddress() *CandidateRelatedAddress {
return c.relatedAddress
@@ -343,29 +381,6 @@ func (c *candidateBase) writeTo(raw []byte, dst Candidate) (int, error) {
return n, nil
}
// Priority computes the priority for this ICE Candidate
func (c *candidateBase) Priority() uint32 {
if c.priorityOverride != 0 {
return c.priorityOverride
}
// The local preference MUST be an integer from 0 (lowest preference) to
// 65535 (highest preference) inclusive. When there is only a single IP
// address, this value SHOULD be set to 65535. If there are multiple
// candidates for a particular component for a particular data stream
// that have the same type, the local preference MUST be unique for each
// one.
var tcpPriorityOffset uint16 = defaultTCPPriorityOffset
if c.agent() != nil {
tcpPriorityOffset = c.agent().tcpPriorityOffset
}
return (1<<24)*uint32(c.Type().Preference(c.networkType, tcpPriorityOffset)) +
(1<<8)*uint32(c.LocalPreference()) +
uint32(256-c.Component())
}
// Equal is used to compare two candidateBases
func (c *candidateBase) Equal(other Candidate) bool {
return c.NetworkType() == other.NetworkType() &&
+52
View File
@@ -13,6 +13,58 @@ import (
"github.com/stretchr/testify/require"
)
func TestCandidateTypePreference(t *testing.T) {
r := require.New(t)
hostDefaultPreference := uint16(126)
prflxDefaultPreference := uint16(110)
srflxDefaultPreference := uint16(100)
relayDefaultPreference := uint16(0)
tcpOffsets := []uint16{0, 10}
for _, tcpOffset := range tcpOffsets {
agent := &Agent{
tcpPriorityOffset: tcpOffset,
}
for _, networkType := range supportedNetworkTypes() {
hostCandidate := candidateBase{
candidateType: CandidateTypeHost,
networkType: networkType,
currAgent: agent,
}
prflxCandidate := candidateBase{
candidateType: CandidateTypePeerReflexive,
networkType: networkType,
currAgent: agent,
}
srflxCandidate := candidateBase{
candidateType: CandidateTypeServerReflexive,
networkType: networkType,
currAgent: agent,
}
relayCandidate := candidateBase{
candidateType: CandidateTypeRelay,
networkType: networkType,
currAgent: agent,
}
if networkType.IsTCP() {
r.Equal(hostDefaultPreference-tcpOffset, hostCandidate.TypePreference())
r.Equal(prflxDefaultPreference-tcpOffset, prflxCandidate.TypePreference())
r.Equal(srflxDefaultPreference-tcpOffset, srflxCandidate.TypePreference())
} else {
r.Equal(hostDefaultPreference, hostCandidate.TypePreference())
r.Equal(prflxDefaultPreference, prflxCandidate.TypePreference())
r.Equal(srflxDefaultPreference, srflxCandidate.TypePreference())
}
r.Equal(relayDefaultPreference, relayCandidate.TypePreference())
}
}
}
func TestCandidatePriority(t *testing.T) {
for _, test := range []struct {
Candidate Candidate
+4 -9
View File
@@ -38,24 +38,19 @@ func (c CandidateType) String() string {
// The RECOMMENDED values are 126 for host candidates, 100
// for server reflexive candidates, 110 for peer reflexive candidates,
// and 0 for relayed candidates.
func (c CandidateType) Preference(networkType NetworkType, tcpOffset uint16) uint16 {
var result uint16
func (c CandidateType) Preference() uint16 {
switch c {
case CandidateTypeHost:
result = 126
return 126
case CandidateTypePeerReflexive:
result = 110
return 110
case CandidateTypeServerReflexive:
result = 100
return 100
case CandidateTypeRelay, CandidateTypeUnspecified:
return 0
default:
return 0
}
if networkType.IsTCP() {
return result - tcpOffset
}
return result
}
func containsCandidateType(candidateType CandidateType, candidateTypeList []CandidateType) bool {
-42
View File
@@ -1,42 +0,0 @@
// SPDX-FileCopyrightText: 2023 The Pion community <https://pion.ly>
// SPDX-License-Identifier: MIT
package ice
import (
"testing"
"github.com/stretchr/testify/require"
)
func TestCandidateTypePreference(t *testing.T) {
r := require.New(t)
hostDefaultPreference := uint16(126)
prflxDefaultPreference := uint16(110)
srflxDefaultPreference := uint16(100)
relayDefaultPreference := uint16(0)
unspecifiedDefaultPreference := uint16(0)
tcpOffsets := []uint16{0, 10}
for _, tcpOffset := range tcpOffsets {
for _, networkType := range supportedNetworkTypes() {
if networkType.IsTCP() {
r.Equal(hostDefaultPreference-tcpOffset, CandidateTypeHost.Preference(networkType, tcpOffset))
r.Equal(prflxDefaultPreference-tcpOffset, CandidateTypePeerReflexive.Preference(networkType, tcpOffset))
r.Equal(srflxDefaultPreference-tcpOffset, CandidateTypeServerReflexive.Preference(networkType, tcpOffset))
} else {
r.Equal(hostDefaultPreference, CandidateTypeHost.Preference(networkType, tcpOffset))
r.Equal(prflxDefaultPreference, CandidateTypePeerReflexive.Preference(networkType, tcpOffset))
r.Equal(srflxDefaultPreference, CandidateTypeServerReflexive.Preference(networkType, tcpOffset))
}
}
}
for _, tcpOffset := range tcpOffsets {
for _, networkType := range supportedNetworkTypes() {
r.Equal(relayDefaultPreference, CandidateTypeRelay.Preference(networkType, tcpOffset))
r.Equal(unspecifiedDefaultPreference, CandidateTypeUnspecified.Preference(networkType, tcpOffset))
}
}
}
+14 -14
View File
@@ -25,7 +25,7 @@ const (
stunGatherTimeout = time.Second * 5
)
type connAndPort struct {
type connConfig struct {
conn net.PacketConn
port int
tcpType TCPType
@@ -161,18 +161,18 @@ func (a *Agent) gatherCandidatesLocal(ctx context.Context, networkTypes []Networ
}
for network := range networks {
var connectionConfigurations []connAndPort
var connConfigs []connConfig
switch network {
case tcp:
// Handle ICE TCP active mode
connectionConfigurations = append(connectionConfigurations, connAndPort{nil, 0, TCPTypeActive})
connConfigs = append(connConfigs, connConfig{nil, 0, TCPTypeActive})
// Handle ICE TCP passive mode
if a.tcpMux != nil {
connectionConfigurations = a.getTCPMuxConnections(mappedIP, ip, network, connectionConfigurations)
connConfigs = a.getTCPMuxConns(mappedIP, ip, network, connConfigs)
}
if len(connectionConfigurations) == 0 {
if len(connConfigs) == 0 {
// Didn't succeed with any, try the next network.
continue
}
@@ -186,36 +186,36 @@ func (a *Agent) gatherCandidatesLocal(ctx context.Context, networkTypes []Networ
}
if udpConn, ok := conn.LocalAddr().(*net.UDPAddr); ok {
connectionConfigurations = append(connectionConfigurations, connAndPort{conn, udpConn.Port, TCPTypeUnspecified})
connConfigs = append(connConfigs, connConfig{conn, udpConn.Port, TCPTypeUnspecified})
} else {
a.log.Warnf("Failed to get port of UDPAddr from ListenUDPInPortRange: %s %s %s", network, ip, a.localUfrag)
continue
}
}
for _, connectionConfiguration := range connectionConfigurations {
for _, connConfig := range connConfigs {
hostConfig := CandidateHostConfig{
Network: network,
Address: address,
Port: connectionConfiguration.port,
Port: connConfig.port,
Component: ComponentRTP,
TCPType: connectionConfiguration.tcpType,
TCPType: connConfig.tcpType,
}
c, err := NewCandidateHost(&hostConfig)
if err != nil {
closeConnAndLog(connectionConfiguration.conn, a.log, "failed to create host candidate: %s %s %d: %v", network, mappedIP, connectionConfiguration.port, err)
closeConnAndLog(connConfig.conn, a.log, "failed to create host candidate: %s %s %d: %v", network, mappedIP, connConfig.port, err)
continue
}
if a.mDNSMode == MulticastDNSModeQueryAndGather {
if err = c.setIP(ip); err != nil {
closeConnAndLog(connectionConfiguration.conn, a.log, "failed to create host candidate: %s %s %d: %v", network, mappedIP, connectionConfiguration.port, err)
closeConnAndLog(connConfig.conn, a.log, "failed to create host candidate: %s %s %d: %v", network, mappedIP, connConfig.port, err)
continue
}
}
if err := a.addCandidate(ctx, c, connectionConfiguration.conn); err != nil {
if err := a.addCandidate(ctx, c, connConfig.conn); err != nil {
if closeErr := c.close(); closeErr != nil {
a.log.Warnf("Failed to close candidate: %v", closeErr)
}
@@ -226,7 +226,7 @@ func (a *Agent) gatherCandidatesLocal(ctx context.Context, networkTypes []Networ
}
}
func (a *Agent) getTCPMuxConnections(mappedIP net.IP, ip net.IP, network string, conns []connAndPort) []connAndPort {
func (a *Agent) getTCPMuxConns(mappedIP net.IP, ip net.IP, network string, conns []connConfig) []connConfig {
var muxConns []net.PacketConn
if multi, ok := a.tcpMux.(AllConnsGetter); ok {
a.log.Debugf("GetAllConns by ufrag: %s", a.localUfrag)
@@ -249,7 +249,7 @@ func (a *Agent) getTCPMuxConnections(mappedIP net.IP, ip net.IP, network string,
// Extract the port for each PacketConn we got.
for _, conn := range muxConns {
if tcpConn, ok := conn.LocalAddr().(*net.TCPAddr); ok {
conns = append(conns, connAndPort{conn, tcpConn.Port, TCPTypePassive})
conns = append(conns, connConfig{conn, tcpConn.Port, TCPTypePassive})
} else {
a.log.Warnf("Failed to get port of connection from TCPMux: %s %s %s", network, ip, a.localUfrag)
}