UPnP does not work

Bug #230973 reported by QQ
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
ApexDC++
New
Undecided
Unassigned
DC++
Fix Released
Low
Unassigned
LinuxDC++
Confirmed
Wishlist
Unassigned

Bug Description

Version 0.706.

I have UPnP configured correctly in my OS (Windows XP) and my router (Linksys WRT54GL). eMule, Miranda, Skype, uTorrent all work fine and correctly setup ports. However, dc++ does not. It used to work few times, but now it just complains that failed to setup ports. I've run upnp sniffer, and it seems that it doesn't even send out UPnP packets.

If you need additional information, please tell what kind and how to get it :)

Revision history for this message
eMTee (realprogger) wrote :

Try with upnp test (http://www.junegillespie.plus.com/UPnPTest.exe) and also the microsoft test page mentioned in this faq : http://www.dslreports.com/faq/11043
As many upnp problems are because of a broken upnp code in routers I would also try to upgrade the firmware.

Revision history for this message
QQ (algkep+dcppbugs) wrote :

As i've said, UPnP is working fine in my computer and network. All other applications, including, but not limited to eMule, Skype, Miranda, uTorrent manage to make use of it without any problems. DC is the only one which fails to setup ports.

Revision history for this message
eMTee (realprogger) wrote :

I read your words but if you won't rule out that the problem is on your side then your problem can't be solved. The upnp code is in DC++ for several years and works with many 100's or routers (including Linksys models) without problems. On the other hand it is reported that (just as in many models) some Linksys ones also shipped with broken upnp code in its firmware. So make sure you pass all the tests, have the latest firmware in your device. If the problem still exists then let us know...

Revision history for this message
QQ (algkep+dcppbugs) wrote :

Well it's somewhat difficult to blame router or PC if it works for all the applications except DC.. and even for DC it used to work, till about a week ago, when it just stopped working. My router is running latest Tomato, which definitely has working upnp.

I've used Intel's UPnP debugging tools. One of them has upnp packet sniffer. Whenever a upnp application tries to forward ports, it lists all the packets being sent. For DC it doesnt show anything, as if not a single packed would be going out..

Revision history for this message
poy (poy) wrote :

sorry, i haven't read everything closely, but have you tried with other port numbers?
can you see DC++ UPnP ports in the connection properties in Windows?

Revision history for this message
Jacek Sieka (arnetheduck) wrote :

I don't have a upnp router so unless someone is able to reproduce this and provide a patch..

Changed in dcplusplus:
status: New → Incomplete
Revision history for this message
Adrian Moș (adimosh) wrote :

My UPnP works like charm, but I have no reasons to actually use it.

QQ - Firewall software on the PC (should you be running that) can trigger such "unfortunate events". For some reason, the Windows XP SP2 firewall did the same for me back on my old computer. Something I installed (not sure what) caused it to start blocking applications, seemingly at random. Some would be blocked, most wouldn't.

Also, ESET Smart Security in automatic mode is a killer on DC software.

If you can isolate your computer and the router for the rest of the network for a few minutes, please disable all security systems on your PC and try again.

Revision history for this message
MikeJJ (mrmikejj) wrote :

UPnP also fails for me (under wine). uTorrent's UPnP works perfectly though, so it not a PC setup / router / wine issue.

Revision history for this message
axet (ak-axet) wrote :

upnp does not work for me

dcpp 0.708
dlink wdl700ap

Revision history for this message
Pada (apexdc) wrote :

I also have a Linksys WRT54GL, BUT I'm running 'DD-WRT v24-sp1 (07/27/08) vpn' firmware on it and ApexDC 1.2.2 is port forwarding fine with UPnP via VirtualBox that I'm running in Ubuntu 9.10 x64.DD-WRT.
It sux that UPnP isn't supported when running Apex via Wine.

However, I have a strange situation with it. ApexDC++ was unable to port forward at first, with the UPnP test results being:
'TEST 1 - Operating System Support - PASSED
TEST 2 - SSDP Service Running Check - PASSED
TEST 3 - SSDP Service Automatic Check - FAILED
TEST 4 - UPnPHost Service Running Check - FAILED
TEST 5 - UPnPHost Service Automatic Check - FAILED
TEST 6 - UPnP Framework Firewall Exception Check - PASSED
TEST 7 - Adapter #0 - 10.0.0.4 - FAILED
TEST 8 - Get External IP Address - FAILED
UPnP Test Program v1.15 Copyright Mark Gillespie 2005'

I then run the Demo Port Forward app found at http://www.codeproject.com/KB/IP/PortForward.aspx
Once it detected my Linksys router, I was able to port forward with ApexDC++ AND the test results were now:
'TEST 1 - Operating System Support - PASSED
TEST 2 - SSDP Service Running Check - PASSED
TEST 3 - SSDP Service Automatic Check - FAILED
TEST 4 - UPnPHost Service Running Check - FAILED
TEST 5 - UPnPHost Service Automatic Check - FAILED
TEST 6 - UPnP Framework Firewall Exception Check - PASSED
TEST 7 - Adapter #0 - 10.0.0.4 - PASSED
TEST 8 - Get External IP Address (Result: 10.20.60.157) - PASSED
UPnP Test Program v1.15 Copyright Mark Gillespie 2005'

After a while the UPnP tester would return the results I've posted at first again.

I haven't taken the time to look at the Demo Port Forward Utility from CodeProject to see why it enables UPnP.

I could perhaps run Wireshark & see if the Demo Port Forward Utility's code is fully functional and then report back here. I'm just fairly busy with other work/jobs at this moment.

Revision history for this message
poy (poy) wrote :

just as a note, i have found that restarting the "SSDP Discovery" service generally solves the problems with DC++ and that test program (which both use the same ways of setting up UPnP mappings).
also, the "UPnP Device Host" has never seemed necessary to me to get these mappings to work.

it would surely be preferable to use a better lib in the future to do these, and possibly in a separate thread because currently it freezes the UI for a few seconds everytime it tries to re-create mappings...

poy (poy)
Changed in dcplusplus:
importance: Undecided → Low
status: Incomplete → In Progress
Revision history for this message
poy (poy) wrote :

using the MiniUPNP client library from <http://miniupnp.free.fr/> solves all the problems for me; also, implementing it is far simpler than the COM interface that Windows provides. moreover it is cross-platform so LinuxDC++ might also be able to use it.
besides, as an incentive, eMule also uses it...

i am attaching a simple file which implements it in DC++. if it is ok then i can then add the include & lib files to the repo. note that it there is a little DLL, miniupnpc.dll (86 KB), to redistribute. here are the files which would go into a miniupnpc/ folder:

miniupnpc:
dll include lib readme.txt

miniupnpc/dll:
miniupnpc.dll miniupnpc.dll.def

miniupnpc/include:
miniupnpc

miniupnpc/include/miniupnpc:
bsdqueue.h minisoap.h miniwget.h upnpreplyparse.h
codelength.h minissdpc.h minixml.h
declspec.h miniupnpc.h upnpcommands.h
igd_desc_parse.h miniupnpcstrings.h upnperrors.h

miniupnpc/lib:
miniupnpc.lib

Revision history for this message
Jacek Sieka (arnetheduck) wrote :

can't it be statically linked?

Revision history for this message
poy (poy) wrote :

yes, i was just using the wrong lib!

also, including the source files too in the repo and having them compile is easier; here is what the new miniupnpc/ dir would be like:

miniupnpc:
total 154K
4.0K LICENCE 4.0K minissdpc.c 1.0K readme.txt
1.0K SConscript 4.0K minissdpc.h 20K upnpcommands.c
 20K bsdqueue.h 20K miniupnpc.c 8.0K upnpcommands.h
4.0K codelength.h 4.0K miniupnpc.h 4.0K upnperrors.c
4.0K declspec.h 4.0K miniupnpcstrings.h 4.0K upnperrors.h
4.0K igd_desc_parse.c 8.0K miniwget.c 4.0K upnpreplyparse.c
4.0K igd_desc_parse.h 4.0K miniwget.h 4.0K upnpreplyparse.h
4.0K minisoap.c 8.0K minixml.c
4.0K minisoap.h 4.0K minixml.h

Revision history for this message
Jacek Sieka (arnetheduck) wrote :

linux, you want this in the core?

Revision history for this message
Razzloss (razzloss) wrote :

Depends on what you mean by in the core. As a feature UPnP support in the core is preferable over GUI implementation, but for linux the library shouldn't be statically linked. So if it's added so that the support can be disabled at compile time and we can use the miniupnp library provided with (some) distros, then I have no objections (if you think the code is good enough. Last time I evaluated it, maybe a year ago, it looked pretty horrible. Though I guess it's a bit better now.).

--RZ

Revision history for this message
Steven Sheehy (steven-sheehy) wrote :

The abstract UPnP class looks okay to be added to core, but not the UPnP_COM.

Revision history for this message
poy (poy) wrote :

UPnP_COM is the COM (so Win-specific) interface which is the "horrible" code Razzloss has seen before. it can definitely not be ported; moreover, it is still partial, it implements the bare minimum interfaces and that is why it fails in cases where routers take a bit longer than usual to respond, or when the Windows service is inexplicably "busy".

regardless if we use the COM interface or MiniUPnP, the GUI freezes for a couple of seconds at startup, which is annoying. since moving to the dcpp lib has been mentioned, would it be acceptable to create an UPnPManager class that would run in its own thread? it would also handle fallbacks to a different UPnP lib when one fails; at the moment, it is the code responsible for the main window GUI that handles this, which feels odd.
in that case, UPnP_COM can remain in win32/, and the win32 code can call dcpp::UPnPManager and tell it that the UPnP_COM interface is also available should it be needed (in most cases, MiniUPnP should work so UPnP_COM shouldn't even be needed).

about compiling MiniUPnPc for dynamic linking, it is quite easy, just one flag ("STATICLIB") to toggle. then again i haven't tried it on Linux but it can't be too hard...

Revision history for this message
Razzloss (razzloss) wrote :

The horrible code I was talking about was the miniupnp library itself. Not the COM implementation (though I guess COM interface is quite ugly, haven't looked at it, just simply because it's COM.).

I mentioned dynamic linking because we prefer to keep our external dependencies external so those will be updated with the normal OS updating mechanisms without the need for linuxdcpp rebuild. And from that comes the need for the miniupnp related things be easily enabled/disabled with compile time options, as some distributions don't include the library.

--RZ

Revision history for this message
Jacek Sieka (arnetheduck) wrote :

a separate thread seems reasonable, but...
isn't the upnp stuff needed when sending the inf? or we could treat a client as passive until upnp has done its magic and then switch any already logged in hubs to active when upnp is done...

as to native vs included, it's a matter of settings in the build script, and linux isn't using our build script anyway so it's really up to them...

what was/is wrong with miniupnp? ugly code? bad api? unstable?

Revision history for this message
Razzloss (razzloss) wrote :

It was only a matter of ugly code. I didn't really continue the evalution any further when the code looked like it was written by a 1st year cs student (no indentation logic that I could follow, some comments in french some english.) Plus rather large stack allocations (I remember some bug with BSD compiler dying with internal error with those?)

But the code seems to be cleaner now and if it works go ahead and use it.

--RZ

Revision history for this message
poy (poy) wrote :

here is a patch that adds an UPnPManager, which creates a thread to do UPnP mappings. it silently fallbacks to other implementations if it has multiple impls.
i have kept UPnP_MiniUPnPc in win32/ for now - LinuxDC++ people can then adjust it to their needs and move it to dcpp/.

by default, windows builds will now be able to use both UPnP_MiniUPnPc and UPnP_COM, with a preference over UPnP_MiniUPnPc (so if it works, UPnP_COM will never be called).

regarding switching to passive for the very few seconds during which UPnP initialization lasts, i feel this would introduce more problems and i'd prefer to keep it the way it is: consider that the user is in active mode if she has selected UPnP in settings.
besides, when UPnP fails, there's an error message inviting the user to rectify her settings; this is better than silently selecting passive mode on her behalf.

Revision history for this message
Jacek Sieka (arnetheduck) wrote :

do we *really* need support for more than one upnp implementation?
regarding passive, isn't the external ip fetched from upnp? without that, active mode is useless...in the long run, a browser style infobar appearing would probably be the nicest solution...falling back to passive is maybe not desirable but still better than not working at all (and it's closer to the truth - when there's no upnp setup, we're effectively in passive mode)

Revision history for this message
poy (poy) wrote :

if UPnP fails, ports may still be correctly forwarded, and if they are not, then the state of the client would be the same as in "i want active mode but i'll configure my router myself" settings option. there are chances it may still work so it would be unwise to switch to passive mode if the user doesn't want to. (and furthermore, in this case it would be like going to passive mode for 2 seconds at startup to get back to active right afterwards.)

getting the external IP via UPnP is just an added bonus but not at all a goal of the feature; actually the (previous and current) behavior is to not consider it an error if we can't get the IP via UPnP, as long as the port redirection worked.
if no external IP could be fetched, it's no problem, the client will simply send 0.0.0.0 and let the hub set it.

as for having multiple implementations, i just followed eMule as an example and this is how they do it. if MiniUPnP can be found to be reliable enough with all routers out there, sure, we could stick to it, but i don't think it is and having a fallback to the standard Windows COM interface is better for now to not upset people who had it working before.

Revision history for this message
Steven Sheehy (steven-sheehy) wrote :

I'm OK with adding UPnP_MiniUPnPc to dcpp (although I don't particularly like the class name). However, I'm with Razzloss that I would like to see a HAVE_MINIUPNPC or HAVE_UPNP compile time flag be added. I can add it myself once it's committed, but I wouldn't know how to integrate it with the windows build system. We're concerned with adding another library dependency for a feature that is not widely used, unless there is a way to make it optional. The flag is a bit of a hack, but without some sort of plug-in system to allow for dynamic loading of libraries at run-time it is the best option we have.

Revision history for this message
RFJUU (mika-hyttinen) wrote :

Is it possible that the new library would be in next version of DC++? It's somewhat annoying to open the client, then wait for a moment, then see that the port mappings failed and then close the client and restart n+1 times just to see that maybe it finally opens the ports (yeah, it sometimes actually manages to open the ports :P)

Also I noticed that when I have my internet connection shared to my XBOX360 (XBOX360 with ethernet cable to my pc, pc wirelessly to Buffalo WHR-G125) it always fails to map the ports, uTorrent on the other hand, never fails.

Revision history for this message
eMTee (realprogger) wrote :

The new library is planned to be released in the next experimental release of DC++.

Revision history for this message
RFJUU (mika-hyttinen) wrote :

Thanks for the information eMTee. Will test after it's released :)

Revision history for this message
poy (poy) wrote :

i have added it.

it is for the moment only for the Windows part, but once LinuxDC++ has this ready for their usage, win32/MiniUPnPc can be moved to dcpp/ as well as its caller which is in win32/main.cpp right now.

Changed in dcplusplus:
status: In Progress → Fix Committed
Revision history for this message
poy (poy) wrote :

Fixed in DC++ 0.770.

Changed in dcplusplus:
status: Fix Committed → Fix Released
Revision history for this message
Carlos (gewan) wrote :

Oh my god. I'm so happy. If any developers read this, just consider it a big rose. Situation's like this. For as long as I can remember (since initial DC++ release probably) the UPnP has not been working properly in Windoze XP. It looks like some people has had it working, so I guess there could be some issue with my particular router. I've always had D-Link routers (mainly the DIR series). Anyways, the UPnP has never worked properly.

A couple of years ago, I found out that DC++ had has spin-offs (i.e. StrongDC/ApexDC). I was delighted, and hopeful, perhaps these "new" clients had taken care of this issue. Unfortunately, the not working UPnP remained a fact, although I still chose to stick with ApexDC++ ever since I discovered it, since it's simply the best in terms of functionability and user interface. However, it still struck me as weird, this thing with UPnP. Other software worked fine with UPnP, eg. Utorrent. I still ran the known Windows app "upnptest.exe" (and its sequel "upnptest2.exe" aswell), just to make sure everything was fine. It was, but DC clients still wouldn't catch UPnP port redirection.

Now, I At one time, I seemed to manage to pin-point the issue. I went into "Network Connections" (ncpa.cpl) / "Internet Connection" / Properties / Settings, to see how the redirection list looked like after having launched DC/StrongDC/ApexDC. The pattern was consistent for all these DC++ clients. Whereas Utorrent (which worked correctly) had forwarded its desired port to LAN IP of my actual machine (eg. 192.168.0.123), the DC forwardings simply pointed at "127.0.0.1". I then tried manually changing 127.0.0.1 (loopback IP) for my actual LAN IP, and voila, suddenly it worked like a charm. However, every time I restarted ApexDC/StrongDC/DC, it would "reattach" to stupid loopback IP, so the procedure was more or less useless.

Now, I havn't been using DC for a while. So, the other day I downloaded and installed ApexDC v1.3.5, and just by curiosity I chose "UPnP" for connection mode. It worked! And now when I look in the ncpa.cpl it lists my actual LAN IP, not the loopback IP. I'm so delighted, looks like it finally works. As I see it, there are two possibilities here. Either the failure has lied within Windows XP operating system, and thus it has been fixed by any of the Windows hotfixes that I automatically install whenever possible, _or_ the issue has actually lied within the DC core, and has thus been (quite recently) been patched.

Either ways, I'm really happy!

Cheers!
..and thanks for supplying the best DC++ client ever!

Changed in linuxdcpp:
importance: Undecided → Wishlist
status: New → Confirmed
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Patches

Remote bug watches

Bug watches keep track of this bug in other bug trackers.