Merge branch 'develop' into bugifx/deprecate-link-config-unused-param

dnp3 · Apr 21, 2022 · 8f72ac8 · 8f72ac8
2 parents c87d6df + 0b2ae58
commit 8f72ac8
Show file tree

Hide file tree

Showing 7 changed files with 54 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,8 @@
 * :beetle: Fix keep-alive timer not properly calculated. See [#407](https://github.com/dnp3/opendnp3/pull/407).
 * :beetle: Fix `LinkContext` and `MContext` possible lifetime issue.
   See [#407](https://github.com/dnp3/opendnp3/pull/407).
+* :beetle: Fix UDP reconnect delay not being honoured. Also, initial UDP read errors
+  (due to ICMP packets) do not close the socket immediately. See [#438](https://github.com/dnp3/opendnp3/pull/438).
 * :coffin: Deprecate the `LinkConfig` constructor with an unused `useConfirms` argument.
   See [#439](https://github.com/dnp3/opendnp3/pull/439).
 

diff --git a/cpp/examples/master-udp/main.cpp b/cpp/examples/master-udp/main.cpp
@@ -39,8 +39,8 @@ int main(int argc, char* argv[])
     DNP3Manager manager(1, ConsoleLogger::Create());
 
     // Connect via a UDP socket to a outstation
-    auto channel = manager.AddUDPChannel("udpclient", logLevels, ChannelRetry::Default(), IPEndpoint("0.0.0.0", 20000),
-                                         IPEndpoint("192.168.0.106", 19999), PrintingChannelListener::Create());
+    auto channel = manager.AddUDPChannel("udpclient", logLevels, ChannelRetry::Default(), IPEndpoint("127.0.0.1", 20000),
+                                         IPEndpoint("127.0.0.1", 20001), PrintingChannelListener::Create());
 
     // The master config object for a master. The default are
     // useable, but understanding the options are important.

diff --git a/cpp/examples/outstation-udp/main.cpp b/cpp/examples/outstation-udp/main.cpp
@@ -68,8 +68,8 @@ int main(int argc, char* argv[])
     DNP3Manager manager(1, ConsoleLogger::Create());
 
     // Create a UDP socket
-    auto channel = manager.AddUDPChannel("server", logLevels, ChannelRetry::Default(), IPEndpoint("192.168.0.106", 19999),
-                                         IPEndpoint("192.168.0.193", 20000), PrintingChannelListener::Create());
+    auto channel = manager.AddUDPChannel("udpserver", logLevels, ChannelRetry::Default(), IPEndpoint("127.0.0.1", 20001),
+                                         IPEndpoint("127.0.0.1", 20000), PrintingChannelListener::Create());
 
     // The main object for a outstation. The defaults are useable,
     // but understanding the options are important.

diff --git a/cpp/lib/src/channel/IOHandler.cpp b/cpp/lib/src/channel/IOHandler.cpp
@@ -52,7 +52,7 @@ void IOHandler::OnReadComplete(const std::error_code& ec, size_t num)
 {
     if (ec)
     {
-        SIMPLE_LOG_BLOCK(this->logger, flags::WARN, ec.message().c_str());
+        FORMAT_LOG_BLOCK(this->logger, flags::WARN, "read error: %s", ec.message().c_str());
 
         this->Reset();
 
@@ -73,7 +73,7 @@ void IOHandler::OnWriteComplete(const std::error_code& ec, size_t num)
 
     if (ec)
     {
-        SIMPLE_LOG_BLOCK(this->logger, flags::WARN, ec.message().c_str());
+        FORMAT_LOG_BLOCK(this->logger, flags::WARN, "write error: %s", ec.message().c_str());
         this->Reset();
 
         this->UpdateListener(ChannelState::OPENING);

diff --git a/cpp/lib/src/channel/UDPClientIOHandler.cpp b/cpp/lib/src/channel/UDPClientIOHandler.cpp
@@ -59,7 +59,9 @@ void UDPClientIOHandler::SuspendChannelAccept()
 
 void UDPClientIOHandler::OnChannelShutdown()
 {
-    this->BeginChannelAccept();
+    this->retrytimer = this->executor->start(this->retry.reconnectDelay.value, [this, self = shared_from_this()]() {
+        this->BeginChannelAccept();
+    });
 }
 
 bool UDPClientIOHandler::TryOpen(const TimeDuration& delay)
@@ -89,19 +91,19 @@ bool UDPClientIOHandler::TryOpen(const TimeDuration& delay)
         else
         {
             FORMAT_LOG_BLOCK(this->logger, flags::INFO, "UDP socket binded to: %s, port %u, sending to %s, port %u",
-                             localEndpoint.address.c_str(), localEndpoint.port, remoteEndpoint.address.c_str(),
-                             remoteEndpoint.port);
+                             socket.local_endpoint().address().to_string().c_str(), socket.local_endpoint().port(),
+                             socket.remote_endpoint().address().to_string().c_str(), socket.remote_endpoint().port());
 
             if (client)
             {
-                this->OnNewChannel(UDPSocketChannel::Create(executor, std::move(socket)));
+                this->OnNewChannel(UDPSocketChannel::Create(executor, this->logger, std::move(socket)));
             }
         }
     };
 
     FORMAT_LOG_BLOCK(this->logger, flags::INFO, "Binding UDP socket to: %s, port %u, resolving address: %s, port %u",
-                     localEndpoint.address.c_str(), localEndpoint.port, remoteEndpoint.address.c_str(),
-                     remoteEndpoint.port);
+                     localEndpoint.address.c_str(), localEndpoint.port,
+                     remoteEndpoint.address.c_str(), remoteEndpoint.port);
 
     this->client->Open(localEndpoint, remoteEndpoint, cb);
 

diff --git a/cpp/lib/src/channel/UDPSocketChannel.cpp b/cpp/lib/src/channel/UDPSocketChannel.cpp
@@ -20,18 +20,46 @@
 
 #include "channel/UDPSocketChannel.h"
 
+#include "logging/LogMacros.h"
+
+static constexpr uint8_t MAX_FIRST_READ_RETRIES = 5;
+
 namespace opendnp3
 {
 
 UDPSocketChannel::UDPSocketChannel(const std::shared_ptr<exe4cpp::StrandExecutor>& executor,
-                                   asio::ip::udp::socket socket)
-    : IAsyncChannel(executor), socket(std::move(socket))
+                                   const Logger& logger, asio::ip::udp::socket socket)
+    : IAsyncChannel(executor), logger(logger), socket(std::move(socket)), first_successful_read(false), num_first_read_retries(0)
 {
 }
 
 void UDPSocketChannel::BeginReadImpl(ser4cpp::wseq_t dest)
 {
-    auto callback = [this](const std::error_code& ec, size_t num) { this->OnReadCallback(ec, num); };
+    auto callback = [this](const std::error_code& ec, size_t num) {
+        if(!this->first_successful_read && this->num_first_read_retries < MAX_FIRST_READ_RETRIES)
+        {
+            if(ec)
+            {
+                FORMAT_LOG_BLOCK(this->logger, flags::WARN, "read error: %s", ec.message().c_str());
+                FORMAT_LOG_BLOCK(this->logger, flags::DBG, "UDP ignoring initial errors (%d of %d)", this->num_first_read_retries + 1, MAX_FIRST_READ_RETRIES);
+
+                // We ignore failed reads until we get a successful one
+                const auto no_ec = std::error_code{};
+                this->OnReadCallback(no_ec, num);
+
+                // Avoid infinite loop
+                ++this->num_first_read_retries;
+
+                return;
+            }
+            else
+            {
+                this->first_successful_read = true;
+            }
+        }
+
+        this->OnReadCallback(ec, num);
+    };
 
     socket.async_receive(asio::buffer(dest, dest.length()), this->executor->wrap(callback));
 }

diff --git a/cpp/lib/src/channel/UDPSocketChannel.h b/cpp/lib/src/channel/UDPSocketChannel.h
@@ -21,6 +21,7 @@
 #define OPENDNP3_UDPSOCKETCHANNEL_H
 
 #include "channel/IAsyncChannel.h"
+#include "opendnp3/logging/Logger.h"
 
 namespace opendnp3
 {
@@ -30,20 +31,23 @@ class UDPSocketChannel final : public IAsyncChannel
 
 public:
     static std::shared_ptr<IAsyncChannel> Create(std::shared_ptr<exe4cpp::StrandExecutor> executor,
-                                                 asio::ip::udp::socket socket)
+                                                 const Logger& logger, asio::ip::udp::socket socket)
     {
-        return std::make_shared<UDPSocketChannel>(executor, std::move(socket));
+        return std::make_shared<UDPSocketChannel>(executor, logger, std::move(socket));
     }
 
-    UDPSocketChannel(const std::shared_ptr<exe4cpp::StrandExecutor>& executor, asio::ip::udp::socket socket);
+    UDPSocketChannel(const std::shared_ptr<exe4cpp::StrandExecutor>& executor, const Logger& logger, asio::ip::udp::socket socket);
 
 protected:
     void BeginReadImpl(ser4cpp::wseq_t dest) final;
     void BeginWriteImpl(const ser4cpp::rseq_t& buffer) final;
     void ShutdownImpl() final;
 
 private:
+    Logger logger;
     asio::ip::udp::socket socket;
+    bool first_successful_read;
+    uint8_t num_first_read_retries;
 };
 
 } // namespace opendnp3