From 4a3835ad3dd1c04521c4f3215d56db607ebc4ad7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adrien=20B=C3=A9raud?= <adrien.beraud@savoirfairelinux.com>
Date: Thu, 30 Jul 2015 12:28:52 -0400
Subject: [PATCH] upnp: handle late IGD findings
This commit bumps OpenDHT to make use of the related
connectivityChanged() method of the dht.
Issue: #78262
Change-Id: I64ea7b745cfc0653a09e8f6fb2d23b2d3fc6db4e
---
contrib/src/opendht/rules.mak | 2 +-
src/ringdht/ringaccount.cpp | 44 ++++++++++++++++++++++++++++-------
src/ringdht/ringaccount.h | 2 ++
src/sip/sipaccount.cpp | 22 +++++++++++-------
src/upnp/upnp_context.cpp | 20 ++++++++++++++++
src/upnp/upnp_context.h | 16 +++++++++++--
src/upnp/upnp_control.cpp | 21 ++++++++++++++++-
src/upnp/upnp_control.h | 13 +++++++++--
src/upnp/upnp_igd.h | 3 +++
9 files changed, 121 insertions(+), 22 deletions(-)
diff --git a/contrib/src/opendht/rules.mak b/contrib/src/opendht/rules.mak
index df9807ec52..7f864cdfda 100644
--- a/contrib/src/opendht/rules.mak
+++ b/contrib/src/opendht/rules.mak
@@ -1,5 +1,5 @@
# OPENDHT
-OPENDHT_VERSION := 930e3e890bae83356ab04959c37aab970717dd05
+OPENDHT_VERSION := 3dc1966dcc6f9d407193832b030890132159fae6
OPENDHT_URL := https://github.com/savoirfairelinux/opendht/archive/$(OPENDHT_VERSION).tar.gz
PKGS += opendht
diff --git a/src/ringdht/ringaccount.cpp b/src/ringdht/ringaccount.cpp
index 42d7c342fe..606281998a 100644
--- a/src/ringdht/ringaccount.cpp
+++ b/src/ringdht/ringaccount.cpp
@@ -601,6 +601,9 @@ RingAccount::handleEvents()
bool RingAccount::mapPortUPnP()
{
+ // return true if not using UPnP
+ bool added = true;
+
if (getUPnPActive()) {
/* create port mapping from published port to local port to the local IP
* note that since different RING accounts can use the same port,
@@ -611,19 +614,20 @@ bool RingAccount::mapPortUPnP()
*/
uint16_t port_used;
std::lock_guard<std::mutex> lock(upnp_mtx);
- if (upnp_->addAnyMapping(dhtPort_, ring::upnp::PortType::UDP, false, &port_used)) {
+ added = upnp_->addAnyMapping(dhtPort_, ring::upnp::PortType::UDP, false, &port_used);
+ if (added) {
if (port_used != dhtPort_)
RING_DBG("UPnP could not map port %u for DHT, using %u instead", dhtPort_, port_used);
dhtPortUsed_ = port_used;
- return true;
- } else {
- /* failed to map any port */
- return false;
}
- } else {
- /* not using UPnP, so return true */
- return true;
}
+
+ std::weak_ptr<RingAccount> w = std::static_pointer_cast<RingAccount>(shared_from_this());
+ upnp_->setIGDListener([w] {
+ if (auto shared = w.lock())
+ shared->connectivityChanged();
+ });
+ return added;
}
void RingAccount::doRegister()
@@ -884,6 +888,7 @@ void RingAccount::doUnregister(std::function<void(bool)> released_cb)
}
/* RING_DBG("UPnP: removing port mapping for DHT account."); */
+ upnp_->setIGDListener();
upnp_->removeMappings();
Manager::instance().unregisterEventHandler((uintptr_t)this);
@@ -1190,4 +1195,27 @@ RingAccount::sendTrustRequest(const std::string& to, const std::vector<uint8_t>&
dht::TrustRequest(DHT_TYPE_NS, payload));
}
+void
+RingAccount::connectivityChanged()
+{
+ if (not dht_.isRunning())
+ return;
+ if ( upnpEnabled_ ) {
+ auto shared = std::static_pointer_cast<RingAccount>(shared_from_this());
+ std::thread{[shared] {
+ auto& this_ = *shared.get();
+ auto oldPort = static_cast<in_port_t>(this_.dhtPortUsed_);
+ if (not this_.mapPortUPnP())
+ RING_WARN("UPnP: Could not map DHT port");
+ auto newPort = static_cast<in_port_t>(this_.dhtPortUsed_);
+ if (oldPort != newPort) {
+ RING_WARN("DHT port changed: restarting network");
+ this_.doRegister_();
+ } else
+ this_.dht_.connectivityChanged();
+ }}.detach();
+ } else
+ dht_.connectivityChanged();
+}
+
} // namespace ring
diff --git a/src/ringdht/ringaccount.h b/src/ringdht/ringaccount.h
index 4f54921199..88d23e5804 100644
--- a/src/ringdht/ringaccount.h
+++ b/src/ringdht/ringaccount.h
@@ -261,6 +261,8 @@ class RingAccount : public SIPAccountBase {
void sendTrustRequest(const std::string& to, const std::vector<uint8_t>& payload);
+ void connectivityChanged();
+
private:
void doRegister_();
diff --git a/src/sip/sipaccount.cpp b/src/sip/sipaccount.cpp
index 4dafb8adf2..134c677541 100644
--- a/src/sip/sipaccount.cpp
+++ b/src/sip/sipaccount.cpp
@@ -695,6 +695,9 @@ SIPAccount::getVolatileAccountDetails() const
bool SIPAccount::mapPortUPnP()
{
+ // return true if not using UPnP
+ bool added = true;
+
if (getUPnPActive()) {
/* create port mapping from published port to local port to the local IP
* note that since different accounts can use the same port,
@@ -704,19 +707,21 @@ bool SIPAccount::mapPortUPnP()
* a different port, if succesfull, then we have to use that port for SIP
*/
uint16_t port_used;
- if (upnp_->addAnyMapping(publishedPort_, localPort_, ring::upnp::PortType::UDP, false, false, &port_used)) {
+ bool added = upnp_->addAnyMapping(publishedPort_, localPort_, ring::upnp::PortType::UDP, false, false, &port_used);
+ if (added) {
if (port_used != publishedPort_)
RING_DBG("UPnP could not map published port %u for SIP, using %u instead", publishedPort_, port_used);
publishedPortUsed_ = port_used;
- return true;
- } else {
- /* failed to map any port */
- return false;
}
- } else {
- /* not using UPnP, so return true */
- return true;
}
+
+ std::weak_ptr<SIPAccount> w = std::static_pointer_cast<SIPAccount>(shared_from_this());
+ upnp_->setIGDListener([w] {
+ if (auto shared = w.lock())
+ shared->doRegister();
+ });
+
+ return added;
}
void SIPAccount::doRegister()
@@ -895,6 +900,7 @@ void SIPAccount::doUnregister(std::function<void(bool)> released_cb)
released_cb(true);
/* RING_DBG("UPnP: removing port mapping for SIP account."); */
+ upnp_->setIGDListener();
upnp_->removeMappings();
}
diff --git a/src/upnp/upnp_context.cpp b/src/upnp/upnp_context.cpp
index 64ebdabdfe..e6da0d249f 100644
--- a/src/upnp/upnp_context.cpp
+++ b/src/upnp/upnp_context.cpp
@@ -224,6 +224,24 @@ UPnPContext::hasValidIGD(std::chrono::seconds timeout)
return not validIGDs_.empty();
}
+size_t
+UPnPContext::addIGDListener(IGDFoundCallback&& cb)
+{
+ std::lock_guard<std::mutex> lock(validIGDMutex_);
+ auto token = ++listenerToken_;
+ igdListeners_.emplace(token, std::move(cb));
+ return token;
+}
+
+void
+UPnPContext::removeIGDListener(size_t token)
+{
+ std::lock_guard<std::mutex> lock(validIGDMutex_);
+ auto it = igdListeners_.find(token);
+ if (it != igdListeners_.end())
+ igdListeners_.erase(it);
+}
+
/**
* chooses the IGD to use,
* assumes you already have a lock on validIGDMutex_
@@ -643,6 +661,8 @@ UPnPContext::parseIGD(IXML_Document* doc, const Upnp_Discovery* d_event)
removeMappingsByLocalIPAndDescription(new_igd.get(), Mapping::UPNP_DEFAULT_MAPPING_DESCRIPTION);
validIGDs_.emplace(UDN, std::move(new_igd));
validIGDCondVar_.notify_all();
+ for (const auto& l : igdListeners_)
+ l.second();
}
}
}
diff --git a/src/upnp/upnp_context.h b/src/upnp/upnp_context.h
index a4d090c22b..c9c869ea08 100644
--- a/src/upnp/upnp_context.h
+++ b/src/upnp/upnp_context.h
@@ -63,11 +63,9 @@ class IpAddr;
namespace ring { namespace upnp {
class UPnPContext {
-
public:
constexpr static unsigned SEARCH_TIMEOUT {30};
-
#if HAVE_LIBUPNP
UPnPContext();
~UPnPContext();
@@ -79,6 +77,9 @@ public:
*/
bool hasValidIGD(std::chrono::seconds timeout = {});
+ size_t addIGDListener(IGDFoundCallback&& cb);
+ void removeIGDListener(size_t token);
+
/**
* tries to add mapping from and to the port_desired
* if unique == true, makes sure the client is not using this port already
@@ -177,6 +178,17 @@ private:
mutable std::mutex validIGDMutex_;
std::condition_variable validIGDCondVar_;
+ /**
+ * Map of valid IGD listeners.
+ */
+ std::map<size_t, IGDFoundCallback> igdListeners_;
+
+ /**
+ * Last provided token for valid IGD listeners.
+ * 0 is the invalid token.
+ */
+ size_t listenerToken_ {0};
+
/**
* chooses the IGD to use (currently selects the first one in the map)
* assumes you already have a lock on igd_mutex_
diff --git a/src/upnp/upnp_control.cpp b/src/upnp/upnp_control.cpp
index ffcf933fa5..9ec9375274 100644
--- a/src/upnp/upnp_control.cpp
+++ b/src/upnp/upnp_control.cpp
@@ -56,6 +56,8 @@ Controller::~Controller()
{
/* remove all mappings */
removeMappings();
+ if (listToken_ and upnpContext_)
+ upnpContext_->removeIGDListener(listToken_);
}
bool
@@ -67,6 +69,16 @@ Controller::hasValidIGD(std::chrono::seconds timeout)
return false;
}
+void
+Controller::setIGDListener(IGDFoundCallback&& cb)
+{
+ if (not upnpContext_)
+ return;
+ if (listToken_)
+ upnpContext_->removeIGDListener(listToken_);
+ listToken_ = cb ? upnpContext_->addIGDListener(std::move(cb)) : 0;
+}
+
bool
Controller::addAnyMapping(uint16_t port_desired,
uint16_t port_local,
@@ -79,6 +91,14 @@ Controller::addAnyMapping(uint16_t port_desired,
if (not upnpContext_)
return false;
+ auto& instanceMappings = type == PortType::UDP ? udpMappings_ : tcpMappings_;
+ Mapping target(port_desired, port_local, type);
+ for (const auto& m : instanceMappings)
+ if (m.second == target) {
+ RING_DBG("UPnP maping already existed: %s", m.second.toString().c_str());
+ return true;
+ }
+
Mapping mapping = upnpContext_->addAnyMapping(port_desired, port_local, type,
use_same_port, unique);
if (mapping) {
@@ -87,7 +107,6 @@ Controller::addAnyMapping(uint16_t port_desired,
*port_used = usedPort;
/* add to map */
- auto& instanceMappings = type == PortType::UDP ? udpMappings_ : tcpMappings_;
instanceMappings.emplace(usedPort, std::move(mapping));
return true;
}
diff --git a/src/upnp/upnp_control.h b/src/upnp/upnp_control.h
index 612456e689..924a95b044 100644
--- a/src/upnp/upnp_control.h
+++ b/src/upnp/upnp_control.h
@@ -51,8 +51,6 @@ class UPnPContext;
class Controller {
public:
- typedef std::function<void(bool)> IGDFoundCallback;
-
/* constructor */
Controller();
/* destructor */
@@ -65,6 +63,12 @@ public:
*/
bool hasValidIGD(std::chrono::seconds timeout = {});
+ /**
+ * Set or clear a listener for valid IGDs.
+ * For simplicity there is one listener per controller.
+ */
+ void setIGDListener(IGDFoundCallback&& cb = {});
+
/**
* tries to add mapping from and to the port_desired
* if unique == true, makes sure the client is not using this port already
@@ -120,6 +124,11 @@ private:
PortMapLocal udpMappings_;
PortMapLocal tcpMappings_;
+ /**
+ * IGD listener token
+ */
+ size_t listToken_ {0};
+
/**
* Try to remove all mappings of the given type
*/
diff --git a/src/upnp/upnp_igd.h b/src/upnp/upnp_igd.h
index 90ead0df47..b0d472f4d7 100644
--- a/src/upnp/upnp_igd.h
+++ b/src/upnp/upnp_igd.h
@@ -33,6 +33,7 @@
#include <string>
#include <map>
+#include <functional>
#include "noncopyable.h"
#include "ip_utils.h"
@@ -127,6 +128,8 @@ public:
class PortMapLocal : public std::map<uint16_t, Mapping> {};
class PortMapGlobal : public std::map<uint16_t, GlobalMapping> {};
+using IGDFoundCallback = std::function<void()>;
+
/* defines a UPnP capable Internet Gateway Device (a router) */
class IGD {
public:
--
GitLab