Skip to content

Commit e3ac0ed

Browse files
committed
Imrove p2p tests
Make sure we test a super common and important scenario: two hosts on the same LAN that *also* have the same gateway, and thus could talk thorugh a hairpinned public route, but should use the LAN route instead based purely on priority numbers, not latency
1 parent 4aefbf6 commit e3ac0ed

1 file changed

Lines changed: 49 additions & 32 deletions

File tree

tests/test_p2p.py

Lines changed: 49 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ def StartClientInThread( role, local, remote, extra_args=[] ):
162162
_SRV_INT2 = '127.0.3.2' # second server internal address
163163
_CLI_INT2 = '127.0.4.2' # second client internal address
164164
_DEAD_INT = '127.0.9.2' # address used for disabled adapters
165+
_CLI_SAME_LAN = '127.0.1.3' # client on the same /24 private LAN as _SRV_INT
165166

166167
def _nat( internal, gateway, nat_type ):
167168
# Gateway must be declared before the adapter that uses it
@@ -173,59 +174,81 @@ def _disabled_adapter( ip ):
173174
def _slow_nat( internal, gateway, nat_type, latency_ms ):
174175
return [ '--mock-gateway', gateway, '--mock-nat', nat_type, '--mock-adapter', internal, '--mock-latency', str(latency_ms) ]
175176

176-
# Each entry: ( description, server_extra_args, client_extra_args, expected_server_route, expected_client_route )
177-
# Route types: 'local' = host-to-host (no NAT traversal needed)
178-
# 'udp' = srflx or peer-reflexive (NAT traversal used)
177+
# Each entry: ( description, server_extra_args, client_extra_args, expected_route )
178+
# Both sides must report the same route type — ICE nominates one candidate pair
179+
# and both ends classify the same path, so agreement is guaranteed by the protocol.
180+
# Route types: 'local' = Fast flag set: both host candidates on the same private /24 LAN subnet
181+
# 'udp' = direct UDP but not same-LAN (NAT traversal, or public IPs)
179182
# 'relay' = TURN relay
180183
CLIENT_SERVER_TEST_CASES = [
181184
( 'no-mock',
182185
[], [],
183-
'local', 'local' ),
186+
'local' ),
187+
188+
# Both on the same private /24 LAN: the core case for 'local' classification.
189+
( 'same LAN (private subnet, no NAT)',
190+
[ '--mock-adapter', _SRV_INT ],
191+
[ '--mock-adapter', _CLI_SAME_LAN ],
192+
'local' ),
193+
194+
# Both on the same LAN but each also has a NAT to the public internet via the
195+
# same shared gateway — the typical home/office scenario. Both a direct host path
196+
# and a hairpin path through the gateway exist; ICE must select the direct host
197+
# path (higher priority) and classify it as 'local'.
198+
( 'same LAN, shared gateway (hairpin)',
199+
_nat( _SRV_INT, _SRV_GW, 'full-cone' ),
200+
_nat( _CLI_SAME_LAN, _SRV_GW, 'full-cone' ),
201+
'local' ),
202+
203+
# Both on the public network: direct host-to-host but NOT 'local' — public IPs
204+
# are not classified as fast even when they share a subnet.
184205
( 'no-nat (both public)',
185206
[ '--mock-adapter', _SRV_GW ],
186207
[ '--mock-adapter', _CLI_GW ],
187-
'local', 'local' ),
208+
'udp' ),
209+
188210
( 'full-cone NAT',
189211
_nat( _SRV_INT, _SRV_GW, 'full-cone' ),
190212
_nat( _CLI_INT, _CLI_GW, 'full-cone' ),
191-
'udp', 'udp' ),
213+
'udp' ),
192214
( 'restricted-cone NAT',
193215
_nat( _SRV_INT, _SRV_GW, 'restricted-cone' ),
194216
_nat( _CLI_INT, _CLI_GW, 'restricted-cone' ),
195-
'udp', 'udp' ),
217+
'udp' ),
196218
( 'port-restricted-cone NAT',
197219
_nat( _SRV_INT, _SRV_GW, 'port-restricted-cone' ),
198220
_nat( _CLI_INT, _CLI_GW, 'port-restricted-cone' ),
199-
'udp', 'udp' ),
221+
'udp' ),
200222
# symmetric-vs-symmetric requires TURN relay; ICE alone cannot traverse it
201223
( 'asymmetric: server public, client full-cone',
202224
[ '--mock-adapter', _SRV_GW ],
203225
_nat( _CLI_INT, _CLI_GW, 'full-cone' ),
204-
'udp', 'local' ), # server sees client's srflx; client's host-host pair wins via NAT passthrough
226+
'udp' ), # client host (127.0.2.x) to server host (127.0.100.x): different subnets
205227
( 'asymmetric: server full-cone, client symmetric',
206228
_nat( _SRV_INT, _SRV_GW, 'full-cone' ),
207229
_nat( _CLI_INT, _CLI_GW, 'symmetric' ),
208-
'udp', 'udp' ),
230+
'udp' ),
209231

210232
# Disabled adapter: verify connection still succeeds when one adapter is down
211233
( 'server has disabled second adapter',
212234
[ '--mock-adapter', _SRV_GW ] + _disabled_adapter( _DEAD_INT ),
213235
_nat( _CLI_INT, _CLI_GW, 'full-cone' ),
214-
'udp', 'local' ),
236+
'udp' ),
215237
( 'client has disabled second adapter',
216238
[ '--mock-adapter', _SRV_GW ],
217239
_nat( _CLI_INT, _CLI_GW, 'full-cone' ) + _disabled_adapter( _DEAD_INT ),
218-
'udp', 'local' ),
240+
'udp' ),
219241

220242
# Multi-adapter with latency: fast public adapter + slow NATd adapter.
221-
# ICE should prefer the low-latency host-to-host path.
243+
# ICE should prefer the low-latency host-to-host path. Both public adapters
244+
# are on 127.0.100.x (not a private subnet) so the route is 'udp', not 'local'.
222245
( 'both multi-adapter: fast public + slow NATd',
223246
[ '--mock-adapter', _SRV_GW ] + _slow_nat( _SRV_INT2, _SRV_GW2, 'full-cone', 50 ),
224247
[ '--mock-adapter', _CLI_GW ] + _slow_nat( _CLI_INT2, _CLI_GW2, 'full-cone', 50 ),
225-
'local', 'local' ),
248+
'udp' ),
226249
]
227250

228-
def ClientServerTest( server_extra_args=[], client_extra_args=[], expected_server_route=None, expected_client_route=None ):
251+
def ClientServerTest( server_extra_args=[], client_extra_args=[], expected_route=None ):
229252
global g_failed
230253
server = StartClientInThread( "server", "peer_server", "peer_client", server_extra_args )
231254
client = StartClientInThread( "client", "peer_client", "peer_server", client_extra_args )
@@ -234,21 +257,15 @@ def ClientServerTest( server_extra_args=[], client_extra_args=[], expected_serve
234257
server.join( timeout=20 )
235258
client.join( timeout=20 )
236259

237-
# Verify route types if expected values were provided
238-
if expected_server_route is not None:
239-
if server.route_type is None:
240-
print( "ERROR: server did not report a route type" )
241-
g_failed = True
242-
elif server.route_type != expected_server_route:
243-
print( "ERROR: server route type '%s', expected '%s'" % ( server.route_type, expected_server_route ) )
244-
g_failed = True
245-
if expected_client_route is not None:
246-
if client.route_type is None:
247-
print( "ERROR: client did not report a route type" )
248-
g_failed = True
249-
elif client.route_type != expected_client_route:
250-
print( "ERROR: client route type '%s', expected '%s'" % ( client.route_type, expected_client_route ) )
251-
g_failed = True
260+
# Verify route types if an expected value was provided
261+
if expected_route is not None:
262+
for peer, thread in [ ( 'server', server ), ( 'client', client ) ]:
263+
if thread.route_type is None:
264+
print( "ERROR: %s did not report a route type" % peer )
265+
g_failed = True
266+
elif thread.route_type != expected_route:
267+
print( "ERROR: %s route type '%s', expected '%s'" % ( peer, thread.route_type, expected_route ) )
268+
g_failed = True
252269

253270
#
254271
# Main
@@ -298,11 +315,11 @@ def ClientServerTest( server_extra_args=[], client_extra_args=[], expected_serve
298315
print( "Signaling server is ready, starting test clients" )
299316

300317
# Run the tests
301-
for desc, srv_args, cli_args, exp_srv_route, exp_cli_route in CLIENT_SERVER_TEST_CASES:
318+
for desc, srv_args, cli_args, exp_route in CLIENT_SERVER_TEST_CASES:
302319
print( "=================================================================" )
303320
print( "Test: " + desc )
304321
print( "=================================================================" )
305-
ClientServerTest( srv_args, cli_args, exp_srv_route, exp_cli_route )
322+
ClientServerTest( srv_args, cli_args, exp_route )
306323
if g_failed:
307324
break
308325

0 commit comments

Comments
 (0)