Skip to content

Commit e735bc0

Browse files
zpostfactozpostfacto
committed
ICE client refactor: ICECandidatePair stores local interface
Not the local candidate! A "local candidate" is a thing that we need to tell our peer about, but when we need to send or receive packets, we use a socket. When a socket has a host candidate, a server reflexive candidate, etc, we don't really know or care which of those candidates the peer used to talk to us. Also, deleted IsCandidatePermitted. We need this, but the place we were calling it was wrong.
1 parent fd500ef commit e735bc0

2 files changed

Lines changed: 37 additions & 67 deletions

File tree

‎src/steamnetworkingsockets/clientlib/steamnetworkingsockets_ice_client.cpp‎

Lines changed: 33 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1104,11 +1104,11 @@ void CSteamNetworkingICESession::InvalidateInterfaceList()
11041104

11051105
void CSteamNetworkingICESession::SetSelectedCandidatePair( ICECandidatePair *pPair )
11061106
{
1107-
SpewMsg( "\n\nSelected candidate %s -> %s.\n\n", SteamNetworkingIPAddrRender( pPair->m_localCandidate.m_pInterface->m_pSocket->m_boundAddr ).c_str(), SteamNetworkingIPAddrRender( pPair->m_remoteCandidate.m_addr ).c_str() );
1107+
SpewMsg( "\n\nSelected candidate %s -> %s.\n\n", SteamNetworkingIPAddrRender( pPair->m_pInterface->m_pSocket->m_boundAddr ).c_str(), SteamNetworkingIPAddrRender( pPair->m_remoteCandidate.m_addr ).c_str() );
11081108
m_pSelectedCandidatePair = pPair;
1109-
m_pSelectedSocket = pPair->m_localCandidate.m_pInterface->m_pSocket;
1109+
m_pSelectedSocket = pPair->m_pInterface->m_pSocket;
11101110
if ( m_pCallbacks )
1111-
m_pCallbacks->OnConnectionSelected( pPair->m_localCandidate, pPair->m_remoteCandidate );
1111+
m_pCallbacks->OnConnectionSelected( *pPair->m_pInterface, pPair->m_remoteCandidate );
11121112
}
11131113

11141114
void CSteamNetworkingICESession::InternalDeleteCandidatePair( ICECandidatePair *pPair )
@@ -1187,7 +1187,7 @@ void CSteamNetworkingICESession::GatherInterfaces()
11871187
for ( int j = len( m_vecCandidatePairs ) - 1; j >= 0; --j )
11881188
{
11891189
ICECandidatePair *pPair = m_vecCandidatePairs[j];
1190-
if ( pPair->m_localCandidate.m_pInterface == intf )
1190+
if ( pPair->m_pInterface == intf )
11911191
{
11921192
InternalDeleteCandidatePair( pPair );
11931193
erase_at( m_vecCandidatePairs, j );
@@ -1368,7 +1368,7 @@ void CSteamNetworkingICESession::OnPacketReceived( const RecvPktInfo_t &info, IC
13681368
for ( ICECandidatePair *pPair : m_vecCandidatePairs )
13691369
{
13701370
if ( pPair->m_remoteCandidate.m_addr == fromAddr
1371-
&& pPair->m_localCandidate.m_pInterface == pInterface )
1371+
&& pPair->m_pInterface == pInterface )
13721372
{
13731373
pThisPair = pPair;
13741374
break;
@@ -1387,24 +1387,6 @@ void CSteamNetworkingICESession::OnPacketReceived( const RecvPktInfo_t &info, IC
13871387
if ( pThisPair == nullptr )
13881388
{
13891389

1390-
// Find the local candidate
1391-
ICELocalCandidate *pLocalCandidate = nullptr;
1392-
for ( ICELocalCandidate &c : m_vecCandidates )
1393-
{
1394-
if ( c.m_pInterface == pInterface )
1395-
{
1396-
pLocalCandidate = &c;
1397-
break;
1398-
}
1399-
}
1400-
if ( !pLocalCandidate )
1401-
{
1402-
// We should at least have a host candidate!
1403-
AssertMsg( false, "We have an interface, but no candidates?" );
1404-
return;
1405-
}
1406-
1407-
// Find / create local candidate
14081390
ICEPeerCandidate *pRemoteCandidate = nullptr;
14091391
for ( ICEPeerCandidate &c : m_vecPeerCandidates )
14101392
{
@@ -1424,7 +1406,7 @@ void CSteamNetworkingICESession::OnPacketReceived( const RecvPktInfo_t &info, IC
14241406
}
14251407
pRemoteCandidate = push_back_get_ptr( m_vecPeerCandidates, ICEPeerCandidate( newRemoteCandidate, SteamNetworkingIPAddrRender( fromAddr ).c_str() ) );
14261408
}
1427-
pThisPair = new ICECandidatePair( *pLocalCandidate, *pRemoteCandidate, m_role );
1409+
pThisPair = new ICECandidatePair( pInterface, *pRemoteCandidate, m_role );
14281410
m_vecCandidatePairs.push_back( pThisPair );
14291411
}
14301412

@@ -1597,11 +1579,6 @@ void CSteamNetworkingICESession::UpdateHostCandidates()
15971579

15981580
}
15991581

1600-
bool CSteamNetworkingICESession::IsCandidatePermitted( const ICELocalCandidate& localCandidate)
1601-
{
1602-
int nCandidateType = localCandidate.CalcType();
1603-
return ( m_nPermittedCandidateTypes & nCandidateType ) == nCandidateType;
1604-
}
16051582

16061583

16071584
void CSteamNetworkingICESession::STUNRequestCallback_ServerReflexiveCandidate( const RecvSTUNPktInfo_t &info )
@@ -1738,13 +1715,17 @@ void CSteamNetworkingICESession::Think_KeepAliveOnCandidates( SteamNetworkingMic
17381715

17391716
if ( m_pSelectedCandidatePair != nullptr )
17401717
{
1741-
UpdateKeepalive( m_pSelectedCandidatePair->m_localCandidate );
1718+
for ( const ICELocalCandidate& c : m_vecCandidates )
1719+
{
1720+
if ( c.m_pInterface == m_pSelectedCandidatePair->m_pInterface )
1721+
UpdateKeepalive( c );
1722+
}
17421723
}
17431724
else
17441725
{
17451726
for ( const ICELocalCandidate& c : m_vecCandidates )
17461727
{
1747-
UpdateKeepalive( c ) ;
1728+
UpdateKeepalive( c );
17481729
}
17491730
}
17501731
}
@@ -1757,31 +1738,25 @@ void CSteamNetworkingICESession::Think_TestPeerConnectivity()
17571738
{
17581739
m_bCandidatePairsNeedUpdate = false;
17591740

1760-
// For every peer, for every local candidate, make sure the pair is present in the pairs list...
1761-
for ( ICELocalCandidate &localCandidate : m_vecCandidates )
1741+
// For every interface, for every peer candidate, make sure the pair is present.
1742+
for ( const std::unique_ptr<ICESessionInterface> &pIntf : m_vecInterfaces )
17621743
{
1763-
if ( !IsCandidatePermitted( localCandidate ) )
1764-
continue;
1765-
17661744
for ( ICEPeerCandidate &remoteCandidate : m_vecPeerCandidates )
17671745
{
1746+
if ( pIntf->m_pSocket->m_boundAddr.IsIPv4() != remoteCandidate.m_addr.IsIPv4() )
1747+
continue;
17681748
bool bFound = false;
17691749
for ( ICECandidatePair *pPair : m_vecCandidatePairs )
17701750
{
1771-
if ( pPair->m_localCandidate.m_addr == localCandidate.m_addr && pPair->m_remoteCandidate.m_addr == remoteCandidate.m_addr )
1751+
if ( pPair->m_pInterface == pIntf.get() && pPair->m_remoteCandidate.m_addr == remoteCandidate.m_addr )
17721752
{
17731753
bFound = true;
17741754
break;
17751755
}
17761756
}
1777-
if ( bFound )
1778-
continue;
1779-
if ( localCandidate.m_pInterface->m_pSocket->m_boundAddr.IsIPv4() != remoteCandidate.m_addr.IsIPv4() )
1780-
continue;
1781-
17821757
if ( !bFound )
17831758
{
1784-
ICECandidatePair *pNewCandidatePair = new ICECandidatePair( localCandidate, remoteCandidate, m_role );
1759+
ICECandidatePair *pNewCandidatePair = new ICECandidatePair( pIntf.get(), remoteCandidate, m_role );
17851760
m_vecCandidatePairs.push_back( pNewCandidatePair );
17861761
}
17871762
}
@@ -1847,7 +1822,7 @@ void CSteamNetworkingICESession::Think_TestPeerConnectivity()
18471822
if ( pPairToCheck != nullptr )
18481823
{
18491824
// Trigger the connectivity check here...
1850-
ICESessionInterface * const pIntf = pPairToCheck->m_localCandidate.m_pInterface;
1825+
ICESessionInterface * const pIntf = pPairToCheck->m_pInterface;
18511826
pPairToCheck->m_nState = kICECandidatePairState_InProgress;
18521827
pPairToCheck->m_pPeerRequest = CSteamNetworkingSocketsSTUNRequest::CreatePeerConnectivityCheckRequest( pIntf, pPairToCheck->m_remoteCandidate.m_addr, CRecvSTUNPktCallback( StaticSTUNRequestCallback_PeerConnectivityCheck, this ), m_nEncoding );
18531828
if ( pPairToCheck->m_pPeerRequest == nullptr )
@@ -1872,9 +1847,8 @@ void CSteamNetworkingICESession::Think_TestPeerConnectivity()
18721847
attrPriority.m_nType = k_nSTUN_Attr_Priority;
18731848
attrPriority.m_nLength = 4;
18741849
attrPriority.m_pData = new uint32[1];
1875-
uint32 uPriority = pPairToCheck->m_localCandidate.m_nPriority;
1876-
// Adjust priority to be peer-reflexity type preference.
1877-
uPriority = ( uPriority & 0xFFFFFF ) | ( 110 << 24ul );
1850+
// RFC 8445 section 7.2.2: priority attr uses peer-reflexive type preference (110).
1851+
uint32 uPriority = ( 110u << 24 ) | ( ( pPairToCheck->m_pInterface->m_nPriority & 0xFFFF ) << 8 ) | 255u;
18781852
*const_cast<uint32*>(attrPriority.m_pData) = htonl( uPriority );
18791853
pPairToCheck->m_pPeerRequest->m_vecExtraAttrs.AddToTail( attrPriority );
18801854
}
@@ -2131,14 +2105,16 @@ EICECandidateType CSteamNetworkingICESession::ICECandidateBase::CalcType() const
21312105
// CSteamNetworkingICESession::ICECandidatePair
21322106
//
21332107
/////////////////////////////////////////////////////////////////////////////
2134-
CSteamNetworkingICESession::ICECandidatePair::ICECandidatePair( const ICELocalCandidate& localCandidate, const ICEPeerCandidate& remoteCandidate, EICERole role )
2135-
: m_localCandidate( localCandidate ),
2108+
CSteamNetworkingICESession::ICECandidatePair::ICECandidatePair( ICESessionInterface *pInterface, const ICEPeerCandidate& remoteCandidate, EICERole role )
2109+
: m_pInterface( pInterface ),
21362110
m_remoteCandidate( remoteCandidate ),
21372111
m_nState( kICECandidatePairState_Frozen ),
21382112
m_bNominated( false )
21392113
{
2140-
const uint64 D = ( role == k_EICERole_Controlling ) ? localCandidate.m_nPriority : remoteCandidate.m_nPriority;
2141-
const uint64 G = ( role == k_EICERole_Controlling ) ? remoteCandidate.m_nPriority : localCandidate.m_nPriority;
2114+
// Use the highest possible local priority: host-type preference (126) on this interface.
2115+
const uint32 nLocalPriority = ( 126u << 24 ) + ( ( pInterface->m_nPriority & 0xFFFF ) << 8 ) + 255u;
2116+
const uint64 D = ( role == k_EICERole_Controlling ) ? nLocalPriority : remoteCandidate.m_nPriority;
2117+
const uint64 G = ( role == k_EICERole_Controlling ) ? remoteCandidate.m_nPriority : nLocalPriority;
21422118
m_nPriority = ( 1ull << 32 ) * MIN( G, D ) + 2 * MAX( G, D ) + ( G > D ? 1 : 0 );
21432119
m_pPeerRequest = nullptr;
21442120
m_nLastRecordedPing = -1;
@@ -2240,22 +2216,17 @@ void CConnectionTransportP2PICE_Valve::OnLocalCandidateDiscovered( const CSteamN
22402216
LocalCandidateGathered( candidate.CalcType(), std::move( c ) );
22412217
}
22422218

2243-
void CConnectionTransportP2PICE_Valve::OnConnectionSelected( const CSteamNetworkingICESession::ICELocalCandidate& localCandidate, const CSteamNetworkingICESession::ICECandidateBase& remoteCandidate )
2219+
void CConnectionTransportP2PICE_Valve::OnConnectionSelected( const ICESessionInterface& localInterface, const CSteamNetworkingICESession::ICECandidateBase& remoteCandidate )
22442220
{
22452221
ConnectionScopeLock lock( Connection(), "CConnectionTransportP2PICE_Valve::OnConnectionSelected");
22462222

22472223
m_currentRouteRemoteAddress = remoteCandidate.m_addr;
22482224
m_eCurrentRouteKind = k_ESteamNetTransport_UDP;
2249-
if ( localCandidate.m_type == ICECandidateKind::Host
2250-
&& remoteCandidate.m_type == ICECandidateKind::Host )
2251-
{
2252-
int nPrefixLen = localCandidate.m_pInterface->m_nPrefixLen;
2253-
if ( IsRemoteAddressOnLocalSubnet( localCandidate.m_pInterface->m_pSocket->m_boundAddr, nPrefixLen, remoteCandidate.m_addr ) )
2254-
m_eCurrentRouteKind = k_ESteamNetTransport_UDPProbablyLocal;
2255-
}
2256-
m_pingEndToEnd.Reset();
2257-
m_pingEndToEnd.ReceivedPing( m_pICESession->GetPing(), SteamNetworkingSockets_GetLocalTimestamp() );
2258-
Connection().TransportEndToEndConnectivityChanged( this, SteamNetworkingSockets_GetLocalTimestamp() );
2225+
if ( IsRemoteAddressOnLocalSubnet( localInterface.m_pSocket->m_boundAddr, localInterface.m_nPrefixLen, remoteCandidate.m_addr ) )
2226+
m_eCurrentRouteKind = k_ESteamNetTransport_UDPProbablyLocal;
2227+
m_pingEndToEnd.Reset();
2228+
m_pingEndToEnd.ReceivedPing( m_pICESession->GetPing(), SteamNetworkingSockets_GetLocalTimestamp() );
2229+
Connection().TransportEndToEndConnectivityChanged( this, SteamNetworkingSockets_GetLocalTimestamp() );
22592230
}
22602231

22612232
void CConnectionTransportP2PICE_Valve::OnPacketReceived( const RecvPktInfo_t &info )

‎src/steamnetworkingsockets/clientlib/steamnetworkingsockets_ice_client.h‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -263,11 +263,11 @@ namespace SteamNetworkingSocketsLib {
263263
ICECandidatePairState m_nState;
264264
bool m_bNominated;
265265
uint64 m_nPriority;
266-
ICELocalCandidate m_localCandidate;
266+
ICESessionInterface *m_pInterface; // local socket; identifies which local address to send from
267267
ICEPeerCandidate m_remoteCandidate;
268268
CSteamNetworkingSocketsSTUNRequest *m_pPeerRequest;
269269
int m_nLastRecordedPing;
270-
ICECandidatePair( const ICELocalCandidate& localCandidate, const ICEPeerCandidate& remoteCandidate, EICERole role );
270+
ICECandidatePair( ICESessionInterface *pInterface, const ICEPeerCandidate& remoteCandidate, EICERole role );
271271
};
272272

273273
CSteamNetworkingICESessionCallbacks *m_pCallbacks;
@@ -379,7 +379,6 @@ namespace SteamNetworkingSocketsLib {
379379
void UpdateHostCandidates();
380380
void UpdateKeepalive( const ICELocalCandidate& c );
381381
uint32 GetInterfaceLocalPreference( const SteamNetworkingIPAddr& addr );
382-
bool IsCandidatePermitted( const ICELocalCandidate& localCandidate );
383382

384383
void Think_KeepAliveOnCandidates( SteamNetworkingMicroseconds usecNow );
385384
void Think_DiscoverServerReflexiveCandidates();
@@ -410,7 +409,7 @@ namespace SteamNetworkingSocketsLib {
410409
public:
411410
virtual void OnLocalCandidateDiscovered( const CSteamNetworkingICESession::ICELocalCandidate& candidate ) {}
412411
virtual void OnPacketReceived( const RecvPktInfo_t &info ) {}
413-
virtual void OnConnectionSelected( const CSteamNetworkingICESession::ICELocalCandidate& localCandidate, const CSteamNetworkingICESession::ICECandidateBase& remoteCandidate ) {}
412+
virtual void OnConnectionSelected( const ICESessionInterface& localInterface, const CSteamNetworkingICESession::ICECandidateBase& remoteCandidate ) {}
414413
};
415414

416415

@@ -437,7 +436,7 @@ namespace SteamNetworkingSocketsLib {
437436
protected:
438437
virtual void OnLocalCandidateDiscovered( const CSteamNetworkingICESession::ICELocalCandidate& candidate ) override;
439438
virtual void OnPacketReceived( const RecvPktInfo_t &info ) override;
440-
virtual void OnConnectionSelected( const CSteamNetworkingICESession::ICELocalCandidate& localCandidate, const CSteamNetworkingICESession::ICECandidateBase& remoteCandidate ) override;
439+
virtual void OnConnectionSelected( const ICESessionInterface& localInterface, const CSteamNetworkingICESession::ICECandidateBase& remoteCandidate ) override;
441440
};
442441

443442
} // namespace SteamNetworkingSocketsLib

0 commit comments

Comments
 (0)