From 6f219d4fef44e31317cf09326bf6df56e03d513f Mon Sep 17 00:00:00 2001 From: drbob Date: Sun, 4 Jul 2010 13:19:09 +0000 Subject: [PATCH] Bugfixes: * Corrected mLocal -> mExt in ipset.cc * Added pqiipset_test * added both Ext and Local address to GUI display. * p3connmgr: Ip Addresses only updated if we connected (otherwise port is wrong). * p3connmgr: update external address when we get it. git-svn-id: http://svn.code.sf.net/p/retroshare/code/trunk@3251 b45a01b8-16f6-495d-af2f-9b41ad6348cc --- libretroshare/src/pqi/p3connmgr.cc | 98 +++++++++---- libretroshare/src/pqi/pqiipset.cc | 2 +- libretroshare/src/rsserver/p3peers.cc | 9 +- libretroshare/src/tests/pqi/Makefile | 9 +- libretroshare/src/tests/pqi/pqiipset_test.cc | 142 +++++++++++++++++++ 5 files changed, 228 insertions(+), 32 deletions(-) create mode 100644 libretroshare/src/tests/pqi/pqiipset_test.cc diff --git a/libretroshare/src/pqi/p3connmgr.cc b/libretroshare/src/pqi/p3connmgr.cc index 04bfdda76..f39202bb8 100644 --- a/libretroshare/src/pqi/p3connmgr.cc +++ b/libretroshare/src/pqi/p3connmgr.cc @@ -642,6 +642,7 @@ void p3ConnectMgr::netTick() connMtx.lock(); /* LOCK MUTEX */ uint32_t netStatus = mNetStatus; + time_t age = time(NULL) - mNetInitTS; connMtx.unlock(); /* UNLOCK MUTEX */ /* start tcp network - if necessary */ @@ -663,7 +664,22 @@ void p3ConnectMgr::netTick() #ifdef CONN_DEBUG_TICK std::cerr << "p3ConnectMgr::netTick() STATUS: UNKNOWN" << std::endl; #endif - netStartup(); + + /* add a small delay to stop restarting straight after a RESET + * This is so can we shutdown cleanly + */ +#define STARTUP_DELAY 5 + if (age < STARTUP_DELAY) + { +#ifdef CONN_DEBUG_TICK + std::cerr << "p3ConnectMgr::netTick() Delaying Startup" << std::endl; +#endif + } + else + { + netStartup(); + } + break; case RS_NET_UPNP_INIT: @@ -916,14 +932,24 @@ void p3ConnectMgr::netExtCheck() /* finalise address */ if (mNetFlags.mExtAddrOk) { + +#ifdef CONN_DEBUG_TICK + std::cerr << "p3ConnectMgr::netExtCheck() "; + std::cerr << "ExtAddr: " << inet_ntoa(mNetFlags.mExtAddr.sin_addr); + std::cerr << ":" << ntohs(mNetFlags.mExtAddr.sin_port); + std::cerr << std::endl; +#endif + //update ip address list + mOwnState.currentserveraddr = mNetFlags.mExtAddr; + + pqiIpAddress addrInfo; + addrInfo.mAddr = mNetFlags.mExtAddr; + addrInfo.mSeenTime = time(NULL); + addrInfo.mSrc = 0; + mOwnState.ipAddrs.mExt.updateIpAddressList(addrInfo); + mNetStatus = RS_NET_DONE; std::cerr << "p3ConnectMgr::netExtCheck() Ext Ok: RS_NET_DONE" << std::endl; -#ifdef CONN_DEBUG_TICK - std::cerr << "p3ConnectMgr::netExtCheck() "; - std::cerr << "ExtAddr: " << inet_ntoa(mNetFlags.mExtAddr.sin_addr); - std::cerr << ":" << ntohs(mNetFlags.mExtAddr.sin_port); - std::cerr << std::endl; -#endif if (!mNetFlags.mExtAddrStableOk) { @@ -1637,7 +1663,7 @@ bool p3ConnectMgr::connectResult(std::string id, bool success, uint32_t flags, s if (success) { - /* update address (will come although through from DISC) */ + /* update address (should also come through from DISC) */ #ifdef CONN_DEBUG std::cerr << "p3ConnectMgr::connectResult() Connect!: id: " << id << std::endl; @@ -1651,27 +1677,43 @@ bool p3ConnectMgr::connectResult(std::string id, bool success, uint32_t flags, s it->second.actions |= RS_PEER_CONNECTED; it->second.lastcontact = time(NULL); /* time of connect */ it->second.connecttype = flags; + + + /* only update the peer's address if we were in a connect attempt. + * Otherwise, they connected to us, and the address will be a + * random port of their outgoing TCP socket + * + * NB even if we received the connection, the IP address is likely to okay. + */ + //used to send back to the peer it's own ext address - it->second.currentserveraddr = remote_peer_address; + //it->second.currentserveraddr = remote_peer_address; - //add the ip address in the address list if we were in a connect attempt and the attempt address is the same as the connect result - pqiIpAddress raddr; - raddr.mAddr = remote_peer_address; - raddr.mSeenTime = time(NULL); - raddr.mSrc = 0; - if (isPrivateNet(&(remote_peer_address.sin_addr))) + if ((it->second.inConnAttempt) && + (it->second.currentConnAddrAttempt.addr.sin_addr.s_addr + == remote_peer_address.sin_addr.s_addr) && + (it->second.currentConnAddrAttempt.addr.sin_port + == remote_peer_address.sin_port)) { - it->second.ipAddrs.updateLocalAddrs(raddr); + pqiIpAddress raddr; + raddr.mAddr = remote_peer_address; + raddr.mSeenTime = time(NULL); + raddr.mSrc = 0; + if (isPrivateNet(&(remote_peer_address.sin_addr))) + { + it->second.ipAddrs.updateLocalAddrs(raddr); + it->second.currentlocaladdr = remote_peer_address; + } + else + { + it->second.ipAddrs.updateExtAddrs(raddr); + it->second.currentserveraddr = remote_peer_address; + } +#ifdef CONN_DEBUG + std::cerr << "p3ConnectMgr::connectResult() adding current peer address in list." << std::endl; + it->second.ipAddrs.printAddrs(std::cerr); +#endif } - else - { - it->second.ipAddrs.updateExtAddrs(raddr); - } - - #ifdef CONN_DEBUG - std::cerr << "p3ConnectMgr::connectResult() adding current peer adress in list." << std::endl; - it->second.ipAddrs.printAddrs(std::cerr); - #endif /* remove other attempts */ it->second.inConnAttempt = false; @@ -2771,7 +2813,6 @@ bool p3ConnectMgr::checkNetAddress() addrChanged = true; } -#if 0 /* if localaddr = serveraddr, then ensure that the ports * are the same (modify server)... this mismatch can * occur when the local port is changed.... @@ -2780,7 +2821,6 @@ bool p3ConnectMgr::checkNetAddress() { mOwnState.currentserveraddr.sin_port = mOwnState.currentlocaladdr.sin_port; } -#endif // ensure that address family is set, otherwise windows Barfs. mOwnState.currentlocaladdr.sin_family = AF_INET; @@ -2796,6 +2836,10 @@ bool p3ConnectMgr::checkNetAddress() #ifdef CONN_DEBUG_TICK std::cerr << "p3ConnectMgr::checkNetAddress() Final Local Address: " << inet_ntoa(mOwnState.currentlocaladdr.sin_addr); std::cerr << ":" << ntohs(mOwnState.currentlocaladdr.sin_port) << std::endl; + std::cerr << "p3ConnectMgr::checkNetAddress() Addres History: "; + std::cerr << std::endl; + mOwnState.ipAddrs.printAddrs(std::cerr); + std::cerr << std::endl; #endif } diff --git a/libretroshare/src/pqi/pqiipset.cc b/libretroshare/src/pqi/pqiipset.cc index 3118601c9..94743810a 100644 --- a/libretroshare/src/pqi/pqiipset.cc +++ b/libretroshare/src/pqi/pqiipset.cc @@ -213,7 +213,7 @@ bool pqiIpAddrSet::updateLocalAddrs(const pqiIpAddress &addr) bool pqiIpAddrSet::updateExtAddrs(const pqiIpAddress &addr) { - return mLocal.updateIpAddressList(addr); + return mExt.updateIpAddressList(addr); } bool pqiIpAddrSet::updateAddrs(const pqiIpAddrSet &addrs) diff --git a/libretroshare/src/rsserver/p3peers.cc b/libretroshare/src/rsserver/p3peers.cc index f5f3b643f..3caa095e1 100644 --- a/libretroshare/src/rsserver/p3peers.cc +++ b/libretroshare/src/rsserver/p3peers.cc @@ -319,12 +319,19 @@ bool p3Peers::getPeerDetails(std::string id, RsPeerDetails &d) std::list::iterator it; + for(it = pcs.ipAddrs.mLocal.mAddrs.begin(); + it != pcs.ipAddrs.mLocal.mAddrs.end(); it++) + { + std::ostringstream toto; + toto << ntohs(it->mAddr.sin_port) << " " << (time(NULL) - it->mSeenTime) << " sec"; + d.ipAddressList.push_back("L:" + std::string(inet_ntoa(it->mAddr.sin_addr)) + ":" + toto.str()); + } for(it = pcs.ipAddrs.mExt.mAddrs.begin(); it != pcs.ipAddrs.mExt.mAddrs.end(); it++) { std::ostringstream toto; toto << ntohs(it->mAddr.sin_port) << " " << (time(NULL) - it->mSeenTime) << " sec"; - d.ipAddressList.push_back(std::string(inet_ntoa(it->mAddr.sin_addr)) + ":" + toto.str()); + d.ipAddressList.push_back("E:" + std::string(inet_ntoa(it->mAddr.sin_addr)) + ":" + toto.str()); } diff --git a/libretroshare/src/tests/pqi/Makefile b/libretroshare/src/tests/pqi/Makefile index 1e466251f..05e315e56 100644 --- a/libretroshare/src/tests/pqi/Makefile +++ b/libretroshare/src/tests/pqi/Makefile @@ -12,12 +12,13 @@ TESTOBJ = conn_harness.o ppg_harness.o TESTOBJ += net_test.o dht_test.o net_test1.o netiface_test.o dht_test.o TESTOBJ += pkt_test.o pqiarchive_test.o pqiperson_test.o -TESTOBJ += extaddrfinder_test.o +TESTOBJ += extaddrfinder_test.o pqiipset_test.o TESTOBJ += p3connmgr_reset_test.o p3connmgr_connect_test.o #conn_test.o TESTS = net_test net_test1 netiface_test pqiarchive_test pqiperson_test extaddrfinder_test -TESTS = p3connmgr_reset_test p3connmgr_connect_test +TESTS += pqiipset_test +TESTS += p3connmgr_reset_test p3connmgr_connect_test #TESTS = p3connmgr_test1 MANUAL_TESTS = dht_test @@ -55,10 +56,12 @@ pqiperson_test: pqiperson_test.o testconnect.o extaddrfinder_test: extaddrfinder_test.o $(CC) $(CFLAGS) -o extaddrfinder_test extaddrfinder_test.o $(LIBS) +pqiipset_test: pqiipset_test.o + $(CC) $(CFLAGS) -o pqiipset_test pqiipset_test.o $(LIBS) + p3connmgr_reset_test: p3connmgr_reset_test.o $(CC) $(CFLAGS) -o p3connmgr_reset_test p3connmgr_reset_test.o $(LIBS) - p3connmgr_connect_test: p3connmgr_connect_test.o conn_harness.o ppg_harness.o $(CC) $(CFLAGS) -o p3connmgr_connect_test p3connmgr_connect_test.o conn_harness.o ppg_harness.o $(LIBS) diff --git a/libretroshare/src/tests/pqi/pqiipset_test.cc b/libretroshare/src/tests/pqi/pqiipset_test.cc new file mode 100644 index 000000000..be75be764 --- /dev/null +++ b/libretroshare/src/tests/pqi/pqiipset_test.cc @@ -0,0 +1,142 @@ +/* + * libretroshare/src/test/pqi pqiipset_test.cc + * + * 3P/PQI network interface for RetroShare. + * + * Copyright 2007-2010 by Robert Fernie. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Library General Public + * License Version 2 as published by the Free Software Foundation. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Library General Public License for more details. + * + * You should have received a copy of the GNU Library General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 + * USA. + * + * Please report all bugs and problems to "retroshare@lunamutt.com". + * + */ + +/****** + * NETWORKING Test to check Big/Little Endian behaviour + * as well as socket behaviour + * + */ + +#include "util/utest.h" + +#include "pqi/pqiipset.h" +#include "pqi/pqinetwork.h" +#include "util/rsnet.h" +#include +#include + + +bool test_addr_list(); + + +INITTEST(); + +int main(int argc, char **argv) +{ + std::cerr << "libretroshare:pqi " << argv[0] << std::endl; + + test_addr_list(); + + FINALREPORT("libretroshare pqiipset Tests"); + return TESTRESULT(); +} + + /* test 1: byte manipulation */ +bool test_addr_list() +{ + pqiIpAddress addr; + pqiIpAddrList list; + + for(int i = 100; i < 150; i++) + { + std::ostringstream out; + out << "192.168.2." << i; + inet_aton(out.str().c_str(), &(addr.mAddr.sin_addr)); + addr.mAddr.sin_port = htons(7812); + addr.mSeenTime = time(NULL) - i; + + list.updateIpAddressList(addr); + } + + /* print out the list */ + std::cerr << "IpAddressList (expect variation: 192.168.2.[100-103]:7812)"; + std::cerr << std::endl; + list.printIpAddressList(std::cerr); + std::cerr << std::endl; + + const uint32_t expectedListSize = 4; + CHECK(list.mAddrs.size() == expectedListSize); + + time_t min_time = time(NULL) - expectedListSize + 100; + + /* expect the most recent ones to appear */ + std::list::iterator it; + for(it = list.mAddrs.begin(); it != list.mAddrs.end(); it++) + { + CHECK(it->mSeenTime < min_time); + } + + /* now add some with same address + port */ + { + std::ostringstream out; + out << "192.168.2.200"; + inet_aton(out.str().c_str(), &(addr.mAddr.sin_addr)); + addr.mAddr.sin_port = htons(8812); + } + + /* make sure it more recent than the previous ones */ + for(int i = 100; i > 89; i--) + { + addr.mSeenTime = time(NULL) - i; + list.updateIpAddressList(addr); + + /* check that was added to the back */ + CHECK(list.mAddrs.back().mSeenTime == addr.mSeenTime); + CHECK(list.mAddrs.back().mAddr.sin_addr.s_addr == addr.mAddr.sin_addr.s_addr); + CHECK(list.mAddrs.back().mAddr.sin_port == addr.mAddr.sin_port); + } + + /* print out the list */ + std::cerr << "IpAddressList (last item to be 192.168.2.200:8812)"; + std::cerr << std::endl; + list.printIpAddressList(std::cerr); + std::cerr << std::endl; + + /* now add with the different ports */ + + for(int i = 70; i > 50; i--) + { + addr.mAddr.sin_port = htons(8000 + i); + addr.mSeenTime = time(NULL) - i; + + list.updateIpAddressList(addr); + + /* check that was added to the back */ + CHECK(list.mAddrs.back().mSeenTime == addr.mSeenTime); + CHECK(list.mAddrs.back().mAddr.sin_addr.s_addr == addr.mAddr.sin_addr.s_addr); + CHECK(list.mAddrs.back().mAddr.sin_port == addr.mAddr.sin_port); + + } + + std::cerr << "IpAddressList (expect same Ip, but variations in port)"; + std::cerr << std::endl; + list.printIpAddressList(std::cerr); + std::cerr << std::endl; + + REPORT("pqiIpAddrList Test"); + + return 1; +} +