Opened 7 years ago

Closed 5 years ago

#708 closed defect (fixed)

transport selection, uPNP interaction

Reported by: sponge Owned by: zzz
Priority: minor Milestone: 0.9.8
Component: router/transport Version: 0.9.1
Keywords: NTCP UDP UPnP transport Cc:
Parent Tickets: Sensitive: no

Description

There are several mixed scenarios, all are related to the same logic.

Settings:
uPNP on, NTCP on, UDP off, UDP has a port number, NTCP has no port number

Result:
uPNP opens the UDP port, does not open the TCP port, and enables both transports for outbound only.

Expected Result:
uPNP should only open NTCP port and use it for in and outbound connections. UDP should be disabled.

Settings:
uPNP on, UDP on, NTCP on, UDP has a port number, NTCP has no port number

Result:
uPNP opens the UDP port, does not open the TCP port, and enables both transports for outbound only.

Expected Result:
uPNP should open both NTCP and UDP ports using the UDP port setting, and use them for in and outbound connections.

I have not had time to check more scenarios, but I am sure there are many more.

Subtickets

Change History (6)

comment:1 Changed 7 years ago by Meeh

I've tested it.

Router software: MikroTik? RouterOS 5.9
I2P Version: 0.9.1-5 OSX

and the results are:

Test 1 (uPNP on, NTCP on, UDP off, UDP has a port number, NTCP has no port number):

I can confirm that ntcp settings shows "currently unknown" when udp is disabled. runned for 4min and is still "testing" network. After about 5min I get this error from the routerconsole:

Network: "ERR - UDP Disabled and Inbound TCP host/port not set"

Whole error message:
"ERR - UDP Disabled and Inbound TCP host/port not set - You have not configured inbound TCP with a hostname and port above, however you have disabled UDP. Therefore your router cannot accept inbound connections. Please configure a TCP host and port above or enable UDP."

On a working UPnP setup I would expect something more like:
"TCP port XXX was successfully forwarded by UPnP" and
"UDP port XXX was successfully forwarded by UPnP"

this was not present.
No TCP config was set in router.config

Test 2 (uPNP on, UDP on, NTCP on, UDP has a port number, NTCP has no port number):

I've tried to set a blank port in TCP setting, the routerconsole decided to set it back to "use same port as UDP"
This creates "i2np.ntcp.autoport=false" in router.config, but does not set the port.

Network: "OK".

comment:2 Changed 7 years ago by sponge

Priority: blockerminor

Setting to minor as per zzz's request.

Android code has a router config properties fixer within it that takes care of some of this issue.

NTCP apparently needs some work to be able to function on it's own for it to do any sort of self-discovery, and that's also part of the problem.

comment:3 Changed 7 years ago by zzz

This is an Android-only issue. No there are not "many more" scenarios. The only apparent problem is that NTCP must have a port set manually if SSU is disabled. In standard I2P, the router console blares a huge warning if this is the case. Of course there is no router console in Android so it wasn't apparent.

So the problem is just that NTCP should pick its own port if SSU is disabled. It does now, it just doesn't use SSU's port selection logic or report it to UPnP (I don't think - haven't verified).

OP's (sponge) additional code in Android RouterService? to work around this is OK I guess, but it also trumps standard SSU port selection when SSU is enabled.

As OP implied on IRC, perhaps the best fix is to move SSU's port selection code up to the generic transport layer, where NTCP can get at it too.

Most of the code OP added to RouterService? can then go away again.

comment:4 Changed 7 years ago by zzz

<sponge> zzz: also, before we dive into reorganization of NTCP/UDP/UPnP code area (collectively, transports) we really need to flow chart the process so that we know for sure that we are catching all the different points.
<zzz> you're proposing a reorganization? why?
<sponge> I'm proposing reorganization of the who-calls-what-when-and-why bits
<sponge> As far as the race, I'm betting it happens because transports are not ready yet to get the UPnP notification sometimes. UPnP needs to be told transports are indeed running, or to reschedule the notification to try later. Or something along those lines.

That's just speculation, with no evidence I have seen.

But anyway, to humor you, here's a picture as you requested: http://pastethis.i2p/show/1946/

comment:5 Changed 7 years ago by str4d

Milestone: 0.9.3

comment:6 Changed 5 years ago by zzz

Milestone: 0.9.8
Resolution: fixed
Status: newclosed

I believe this was all accomplished as a part of the IPv6 work merged in 0.9.8. Anything remaining is too low-priority to worry about as it relates to non-default configurations. If there's anything remaining that's android-specific, please open a ticket against that component.

Note: See TracTickets for help on using tickets.