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

 # OPENDHT
-OPENDHT_VERSION := 930e3e890bae83356ab04959c37aab970717dd05
+OPENDHT_VERSION := 3dc1966dcc6f9d407193832b030890132159fae6
 OPENDHT_URL := https://github.com/savoirfairelinux/opendht/archive/$(OPENDHT_VERSION).tar.gz
 
 PKGS += opendht

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

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_();

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();
 }
 

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();
         }
     }
 }

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_

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;
     }

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
      */

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: