From 9a67e65e1741e4cdf216ece64bdc7c9c09ba5849 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89mile=20Gr=C3=A9goire?= Date: Tue, 21 Apr 2020 14:40:21 -0400 Subject: [PATCH] Remove confirmed link-layer support. Fix #364 and possibly #138. --- CMakeLists.txt | 2 +- cpp/lib/include/opendnp3/link/LinkConfig.h | 12 - cpp/lib/src/link/LinkContext.cpp | 42 +-- cpp/lib/src/link/LinkContext.h | 15 - cpp/lib/src/link/PriLinkLayerStates.cpp | 150 -------- cpp/lib/src/link/PriLinkLayerStates.h | 82 +--- cpp/tests/unit/TestLinkLayer.cpp | 354 ------------------ dotnet/CLRAdapter/src/Conversions.cpp | 5 +- dotnet/CLRInterface/src/config/LinkConfig.cs | 26 -- .../com/automatak/dnp3/LinkLayerConfig.java | 15 - java/cpp/adapters/ConfigReader.cpp | 2 - java/cpp/jni/JNILinkLayerConfig.cpp | 16 - java/cpp/jni/JNILinkLayerConfig.h | 4 - 13 files changed, 5 insertions(+), 720 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e2d48497b7..7ded5dfd0b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -78,7 +78,7 @@ if(DNP3_TLS) find_package(OpenSSL REQUIRED) endif() -if(DNP3_TESTS) +if(DNP3_TESTS OR DNP3_FUZZING) include(./deps/catch.cmake) endif() diff --git a/cpp/lib/include/opendnp3/link/LinkConfig.h b/cpp/lib/include/opendnp3/link/LinkConfig.h index bd4c761a3d..4f451bcc86 100644 --- a/cpp/lib/include/opendnp3/link/LinkConfig.h +++ b/cpp/lib/include/opendnp3/link/LinkConfig.h @@ -34,8 +34,6 @@ struct LinkConfig LinkConfig() = delete; LinkConfig(bool isMaster, - bool useConfirms, - uint32_t numRetry, uint16_t localAddr, uint16_t remoteAddr, TimeDuration timeout, @@ -43,8 +41,6 @@ struct LinkConfig : IsMaster(isMaster), - UseConfirms(useConfirms), - NumRetry(numRetry), LocalAddr(localAddr), RemoteAddr(remoteAddr), Timeout(timeout), @@ -56,8 +52,6 @@ struct LinkConfig : IsMaster(isMaster), - UseConfirms(useConfirms), - NumRetry(0), LocalAddr(isMaster ? 1 : 1024), RemoteAddr(isMaster ? 1024 : 1), Timeout(TimeDuration::Seconds(1)), @@ -73,12 +67,6 @@ struct LinkConfig /// The master/outstation bit set on all messages bool IsMaster; - /// If true, the link layer will send data requesting confirmation - bool UseConfirms; - - /// The number of retry attempts the link will attempt after the initial try - uint32_t NumRetry; - /// dnp3 address of the local device uint16_t LocalAddr; diff --git a/cpp/lib/src/link/LinkContext.cpp b/cpp/lib/src/link/LinkContext.cpp index fa3dd8c514..d1bb210fa4 100644 --- a/cpp/lib/src/link/LinkContext.cpp +++ b/cpp/lib/src/link/LinkContext.cpp @@ -37,12 +37,9 @@ LinkContext::LinkContext(const Logger& logger, config(config), pSegments(nullptr), txMode(LinkTransmitMode::Idle), - numRetryRemaining(0), executor(executor), nextReadFCB(false), - nextWriteFCB(false), isOnline(false), - isRemoteReset(false), keepAliveTimeout(false), lastMessageTimestamp(executor->get_time()), pPriState(&PLLS_Idle::Instance()), @@ -85,7 +82,6 @@ bool LinkContext::OnLowerLayerDown() isOnline = false; keepAliveTimeout = false; - isRemoteReset = false; pSegments = nullptr; txMode = LinkTransmitMode::Idle; pendingPriTx.clear(); @@ -149,17 +145,6 @@ bool LinkContext::OnTxReady() return true; } -ser4cpp::rseq_t LinkContext::FormatPrimaryBufferWithConfirmed(const Addresses& addr, - const ser4cpp::rseq_t& tpdu, - bool FCB) -{ - auto dest = this->priTxBuffer.as_wseq(); - auto output - = LinkFrame::FormatConfirmedUserData(dest, config.IsMaster, FCB, addr.destination, addr.source, tpdu, &logger); - FORMAT_HEX_BLOCK(logger, flags::LINK_TX_HEX, output, 10, 18); - return output; -} - ser4cpp::rseq_t LinkContext::FormatPrimaryBufferWithUnconfirmed(const Addresses& addr, const ser4cpp::rseq_t& tpdu) { auto buffer = this->priTxBuffer.as_wseq(); @@ -206,14 +191,6 @@ void LinkContext::QueueLinkStatus(uint16_t destination) this->QueueTransmit(buffer, false); } -void LinkContext::QueueResetLinks(uint16_t destination) -{ - auto dest = priTxBuffer.as_wseq(); - auto buffer = LinkFrame::FormatResetLinkStates(dest, config.IsMaster, destination, this->config.LocalAddr, &logger); - FORMAT_HEX_BLOCK(logger, flags::LINK_TX_HEX, buffer, 10, 18); - this->QueueTransmit(buffer, true); -} - void LinkContext::QueueRequestLinkStatus(uint16_t destination) { auto dest = priTxBuffer.as_wseq(); @@ -223,22 +200,6 @@ void LinkContext::QueueRequestLinkStatus(uint16_t destination) this->QueueTransmit(buffer, true); } -void LinkContext::ResetRetry() -{ - this->numRetryRemaining = config.NumRetry; -} - -bool LinkContext::Retry() -{ - if (numRetryRemaining > 0) - { - --numRetryRemaining; - return true; - } - - return false; -} - void LinkContext::PushDataUp(const Message& message) { upper->OnReceive(message); @@ -262,8 +223,7 @@ void LinkContext::TryStartTransmission() if (this->pSegments) { - this->pPriState = (this->config.UseConfirms) ? &pPriState->TrySendConfirmed(*this, *pSegments) - : &pPriState->TrySendUnconfirmed(*this, *pSegments); + this->pPriState = &pPriState->TrySendUnconfirmed(*this, *pSegments); } } diff --git a/cpp/lib/src/link/LinkContext.h b/cpp/lib/src/link/LinkContext.h index 61b6d0c331..26ada6371c 100644 --- a/cpp/lib/src/link/LinkContext.h +++ b/cpp/lib/src/link/LinkContext.h @@ -68,18 +68,10 @@ class LinkContext { nextReadFCB = true; } - void ResetWriteFCB() - { - nextWriteFCB = true; - } void ToggleReadFCB() { nextReadFCB = !nextReadFCB; } - void ToggleWriteFCB() - { - nextWriteFCB = !nextWriteFCB; - } // --- helpers for dealing with layer state transitations --- bool OnLowerLayerUp(); @@ -89,20 +81,16 @@ class LinkContext // --- helpers for formatting user data messages --- ser4cpp::rseq_t FormatPrimaryBufferWithUnconfirmed(const Addresses& addr, const ser4cpp::rseq_t& tpdu); - ser4cpp::rseq_t FormatPrimaryBufferWithConfirmed(const Addresses& addr, const ser4cpp::rseq_t& tpdu, bool FCB); // --- Helpers for queueing frames --- void QueueAck(uint16_t destination); void QueueLinkStatus(uint16_t destination); - void QueueResetLinks(uint16_t destination); void QueueRequestLinkStatus(uint16_t destination); void QueueTransmit(const ser4cpp::rseq_t& buffer, bool primary); // --- public members ---- - void ResetRetry(); - bool Retry(); void PushDataUp(const Message& message); void CompleteSendOperation(); void TryStartTransmission(); @@ -127,16 +115,13 @@ class LinkContext const LinkLayerConfig config; ITransportSegment* pSegments; LinkTransmitMode txMode; - uint32_t numRetryRemaining; const std::shared_ptr executor; exe4cpp::Timer rspTimeoutTimer; exe4cpp::Timer keepAliveTimer; bool nextReadFCB; - bool nextWriteFCB; bool isOnline; - bool isRemoteReset; bool keepAliveTimeout; Timestamp lastMessageTimestamp; StackStatistics::Link statistics; diff --git a/cpp/lib/src/link/PriLinkLayerStates.cpp b/cpp/lib/src/link/PriLinkLayerStates.cpp index 959ba6a362..21c33ae9a4 100644 --- a/cpp/lib/src/link/PriLinkLayerStates.cpp +++ b/cpp/lib/src/link/PriLinkLayerStates.cpp @@ -71,11 +71,6 @@ PriStateBase& PriStateBase::OnTimeout(LinkContext& ctx) return *this; } -PriStateBase& PriStateBase::TrySendConfirmed(LinkContext& /*ctx*/, ITransportSegment& /*unused*/) -{ - return *this; -} - PriStateBase& PriStateBase::TrySendUnconfirmed(LinkContext& /*ctx*/, ITransportSegment& /*unused*/) { return *this; @@ -100,22 +95,6 @@ PriStateBase& PLLS_Idle::TrySendUnconfirmed(LinkContext& ctx, ITransportSegment& return PLLS_SendUnconfirmedTransmitWait::Instance(); } -PriStateBase& PLLS_Idle::TrySendConfirmed(LinkContext& ctx, ITransportSegment& segments) -{ - if (ctx.isRemoteReset) - { - ctx.ResetRetry(); - auto buffer - = ctx.FormatPrimaryBufferWithConfirmed(segments.GetAddresses(), segments.GetSegment(), ctx.nextWriteFCB); - ctx.QueueTransmit(buffer, true); - return PLLS_ConfUserDataTransmitWait::Instance(); - } - - ctx.ResetRetry(); - ctx.QueueResetLinks(segments.GetAddresses().destination); - return PLLS_LinkResetTransmitWait::Instance(); -} - PriStateBase& PLLS_Idle::TrySendRequestLinkStatus(LinkContext& ctx) { ctx.keepAliveTimeout = false; @@ -145,32 +124,6 @@ PriStateBase& PLLS_SendUnconfirmedTransmitWait::OnTxReady(LinkContext& ctx) return PLLS_Idle::Instance(); } -///////////////////////////////////////////////////////////////////////////// -// Wait for the link layer to transmit the reset links -///////////////////////////////////////////////////////////////////////////// - -PLLS_LinkResetTransmitWait PLLS_LinkResetTransmitWait::instance; - -PriStateBase& PLLS_LinkResetTransmitWait::OnTxReady(LinkContext& ctx) -{ - // now we're waiting for an ACK - ctx.StartResponseTimer(); - return PLLS_ResetLinkWait::Instance(); -} - -///////////////////////////////////////////////////////////////////////////// -// Wait for the link layer to transmit confirmed user data -///////////////////////////////////////////////////////////////////////////// - -PLLS_ConfUserDataTransmitWait PLLS_ConfUserDataTransmitWait::instance; - -PriStateBase& PLLS_ConfUserDataTransmitWait::OnTxReady(LinkContext& ctx) -{ - // now we're waiting on an ACK - ctx.StartResponseTimer(); - return PLLS_ConfDataWait::Instance(); -} - ///////////////////////////////////////////////////////////////////////////// // Wait for the link layer to transmit the request link status ///////////////////////////////////////////////////////////////////////////// @@ -184,109 +137,6 @@ PriStateBase& PLLS_RequestLinkStatusTransmitWait::OnTxReady(LinkContext& ctx) return PLLS_RequestLinkStatusWait::Instance(); } -//////////////////////////////////////////////////////// -// Class PLLS_ResetLinkWait -//////////////////////////////////////////////////////// - -PLLS_ResetLinkWait PLLS_ResetLinkWait::instance; - -PriStateBase& PLLS_ResetLinkWait::OnAck(LinkContext& ctx, bool /*rxBuffFull*/) -{ - ctx.isRemoteReset = true; - ctx.ResetWriteFCB(); - ctx.CancelTimer(); - auto buffer = ctx.FormatPrimaryBufferWithConfirmed(ctx.pSegments->GetAddresses(), ctx.pSegments->GetSegment(), - ctx.nextWriteFCB); - ctx.QueueTransmit(buffer, true); - ctx.listener->OnStateChange(LinkStatus::RESET); - return PLLS_ConfUserDataTransmitWait::Instance(); -} - -PriStateBase& PLLS_ResetLinkWait::OnTimeout(LinkContext& ctx) -{ - if (ctx.Retry()) - { - FORMAT_LOG_BLOCK(ctx.logger, flags::WARN, "Link reset timeout, retrying %i remaining", ctx.numRetryRemaining); - ctx.QueueResetLinks(ctx.config.RemoteAddr); - return PLLS_LinkResetTransmitWait::Instance(); - } - - SIMPLE_LOG_BLOCK(ctx.logger, flags::WARN, "Link reset final timeout, no retries remain"); - ctx.CompleteSendOperation(); - return PLLS_Idle::Instance(); -} - -PriStateBase& PLLS_ResetLinkWait::Failure(LinkContext& ctx) -{ - ctx.CancelTimer(); - ctx.CompleteSendOperation(); - return PLLS_Idle::Instance(); -} - -//////////////////////////////////////////////////////// -// Class PLLS_ConfDataWait -//////////////////////////////////////////////////////// - -PLLS_ConfDataWait PLLS_ConfDataWait::instance; - -PriStateBase& PLLS_ConfDataWait::OnAck(LinkContext& ctx, bool /*rxBuffFull*/) -{ - ctx.ToggleWriteFCB(); - ctx.CancelTimer(); - - if (ctx.pSegments->Advance()) - { - auto buffer = ctx.FormatPrimaryBufferWithConfirmed(ctx.pSegments->GetAddresses(), ctx.pSegments->GetSegment(), - ctx.nextWriteFCB); - ctx.QueueTransmit(buffer, true); - return PLLS_ConfUserDataTransmitWait::Instance(); - } - // we're done! - - ctx.CompleteSendOperation(); - return PLLS_Idle::Instance(); -} - -PriStateBase& PLLS_ConfDataWait::OnNack(LinkContext& ctx, bool rxBuffFull) -{ - ctx.listener->OnStateChange(LinkStatus::UNRESET); - - if (rxBuffFull) - { - return Failure(ctx); - } - - ctx.ResetRetry(); - ctx.CancelTimer(); - ctx.QueueResetLinks(ctx.pSegments->GetAddresses().destination); - return PLLS_LinkResetTransmitWait::Instance(); -} - -PriStateBase& PLLS_ConfDataWait::Failure(LinkContext& ctx) -{ - ctx.isRemoteReset = false; - ctx.CancelTimer(); - ctx.CompleteSendOperation(); - return PLLS_Idle::Instance(); -} - -PriStateBase& PLLS_ConfDataWait::OnTimeout(LinkContext& ctx) -{ - if (ctx.Retry()) - { - FORMAT_LOG_BLOCK(ctx.logger, flags::WARN, "confirmed data timeout, retrying %u remaining", - ctx.numRetryRemaining); - auto buffer = ctx.FormatPrimaryBufferWithConfirmed(ctx.pSegments->GetAddresses(), ctx.pSegments->GetSegment(), - ctx.nextWriteFCB); - ctx.QueueTransmit(buffer, true); - return PLLS_ConfUserDataTransmitWait::Instance(); - } - - SIMPLE_LOG_BLOCK(ctx.logger, flags::WARN, "Confirmed data final timeout, no retries remain"); - ctx.listener->OnStateChange(LinkStatus::UNRESET); - return Failure(ctx); -} - //////////////////////////////////////////////////////// // Class PLLS_RequestLinkStatusWait //////////////////////////////////////////////////////// diff --git a/cpp/lib/src/link/PriLinkLayerStates.h b/cpp/lib/src/link/PriLinkLayerStates.h index ffd7eb021a..453b7b3991 100644 --- a/cpp/lib/src/link/PriLinkLayerStates.h +++ b/cpp/lib/src/link/PriLinkLayerStates.h @@ -42,7 +42,6 @@ class PriStateBase virtual PriStateBase& OnTimeout(LinkContext&); // transmission events to handle - virtual PriStateBase& TrySendConfirmed(LinkContext&, ITransportSegment& segments); virtual PriStateBase& TrySendUnconfirmed(LinkContext&, ITransportSegment& segments); virtual PriStateBase& TrySendRequestLinkStatus(LinkContext&); @@ -56,7 +55,6 @@ class PLLS_Idle final : public PriStateBase MACRO_STATE_SINGLETON_INSTANCE(PLLS_Idle); virtual PriStateBase& TrySendUnconfirmed(LinkContext&, ITransportSegment& segments) override; - virtual PriStateBase& TrySendConfirmed(LinkContext&, ITransportSegment& segments) override; virtual PriStateBase& TrySendRequestLinkStatus(LinkContext&) override; }; @@ -72,29 +70,7 @@ class PLLS_SendUnconfirmedTransmitWait final : public PriStateBase }; ///////////////////////////////////////////////////////////////////////////// -// Wait for the link layer to transmit the reset links -///////////////////////////////////////////////////////////////////////////// - -class PLLS_LinkResetTransmitWait : public PriStateBase -{ - MACRO_STATE_SINGLETON_INSTANCE(PLLS_LinkResetTransmitWait); - - virtual PriStateBase& OnTxReady(LinkContext& ctx) override; -}; - -///////////////////////////////////////////////////////////////////////////// -// Wait for the link layer to transmit confirmed user data -///////////////////////////////////////////////////////////////////////////// - -class PLLS_ConfUserDataTransmitWait : public PriStateBase -{ - MACRO_STATE_SINGLETON_INSTANCE(PLLS_ConfUserDataTransmitWait); - - virtual PriStateBase& OnTxReady(LinkContext& ctx) override; -}; - -///////////////////////////////////////////////////////////////////////////// -// Wait for the link layer to transmit confirmed user data +// Waiting for a link status transmission ///////////////////////////////////////////////////////////////////////////// class PLLS_RequestLinkStatusTransmitWait : public PriStateBase @@ -104,62 +80,6 @@ class PLLS_RequestLinkStatusTransmitWait : public PriStateBase virtual PriStateBase& OnTxReady(LinkContext& ctx) override; }; -///////////////////////////////////////////////////////////////////////////// -// Waiting for a ACK to reset the link -///////////////////////////////////////////////////////////////////////////// - -// @section desc As soon as we get an ACK, send the delayed pri frame -class PLLS_ResetLinkWait final : public PriStateBase -{ - MACRO_STATE_SINGLETON_INSTANCE(PLLS_ResetLinkWait); - - PriStateBase& OnAck(LinkContext&, bool aIsRcvBuffFull) override; - - PriStateBase& OnNack(LinkContext& link, bool) override - { - return Failure(link); - } - PriStateBase& OnLinkStatus(LinkContext& link, bool) override - { - return Failure(link); - } - PriStateBase& OnNotSupported(LinkContext& link, bool) override - { - return Failure(link); - } - - PriStateBase& OnTimeout(LinkContext&) override; - -private: - PriStateBase& Failure(LinkContext&); -}; - -///////////////////////////////////////////////////////////////////////////// -// Waiting for a ACK to user data -///////////////////////////////////////////////////////////////////////////// - -// @section desc As soon as we get an ACK, send the delayed pri frame -class PLLS_ConfDataWait final : public PriStateBase -{ - MACRO_STATE_SINGLETON_INSTANCE(PLLS_ConfDataWait); - - PriStateBase& OnAck(LinkContext&, bool aIsRcvBuffFull) override; - PriStateBase& OnNack(LinkContext& ctx, bool) override; - PriStateBase& OnLinkStatus(LinkContext& link, bool) override - { - return Failure(link); - } - PriStateBase& OnNotSupported(LinkContext& link, bool) override - { - return Failure(link); - } - - PriStateBase& OnTimeout(LinkContext&) override; - -private: - PriStateBase& Failure(LinkContext&); -}; - ///////////////////////////////////////////////////////////////////////////// // Waiting for a link status response ///////////////////////////////////////////////////////////////////////////// diff --git a/cpp/tests/unit/TestLinkLayer.cpp b/cpp/tests/unit/TestLinkLayer.cpp index 0677bbfc3a..a4f793845c 100644 --- a/cpp/tests/unit/TestLinkLayer.cpp +++ b/cpp/tests/unit/TestLinkLayer.cpp @@ -147,71 +147,6 @@ TEST_CASE(SUITE("BroadcastSecondaryResetLink")) REQUIRE(t.link.GetStatistics().numUnexpectedFrame == 1); } -TEST_CASE(SUITE("SecAckWrongFCB")) -{ - LinkConfig cfg = LinkLayerTest::DefaultConfig(); - cfg.UseConfirms = true; - - LinkLayerTest t(cfg); - t.link.OnLowerLayerUp(); - - t.OnFrame(LinkFunction::PRI_RESET_LINK_STATES, false, false, false, 1, 1024); - REQUIRE(t.NumTotalWrites() == 1); - t.link.OnTxReady(); - - ByteStr b(250, 0); - t.OnFrame(LinkFunction::PRI_CONFIRMED_USER_DATA, false, false, false, 1, 1024, b.ToRSeq()); - t.link.OnTxReady(); - REQUIRE(t.NumTotalWrites() == 2); - - REQUIRE(t.PopLastWriteAsHex() == LinkHex::Ack(true, false, 1024, 1)); - REQUIRE(t.upper->receivedQueue.empty()); // data should not be passed up! -} - -TEST_CASE(SUITE("BroadcastConfirmedDataWithoutResetDoesntForward")) -{ - LinkConfig cfg = LinkLayerTest::DefaultConfig(); - cfg.UseConfirms = true; - - LinkLayerTest t(cfg); - t.link.OnLowerLayerUp(); - - ByteStr b(250, 0); - t.OnFrame(LinkFunction::PRI_CONFIRMED_USER_DATA, false, false, false, LinkBroadcastAddress::ShallConfirm, 1024, - b.ToRSeq()); - t.link.OnTxReady(); - REQUIRE(t.upper->receivedQueue.empty()); -} - -TEST_CASE(SUITE("BroadcastConfirmedDataFlipNFCBAndDoesntRespond")) -{ - LinkConfig cfg = LinkLayerTest::DefaultConfig(); - cfg.UseConfirms = true; - - LinkLayerTest t(cfg); - t.link.OnLowerLayerUp(); - - t.OnFrame(LinkFunction::PRI_RESET_LINK_STATES, false, false, false, 1, 1024); - REQUIRE(t.NumTotalWrites() == 1); - t.link.OnTxReady(); - - ByteStr b(250, 0); - t.OnFrame(LinkFunction::PRI_CONFIRMED_USER_DATA, false, true, false, LinkBroadcastAddress::ShallConfirm, 1024, - b.ToRSeq()); - t.link.OnTxReady(); - REQUIRE(t.upper->receivedQueue.size() == 1); - REQUIRE(t.upper->receivedQueue.front() == b.ToHex()); - REQUIRE(t.link.GetStatistics().numUnexpectedFrame == 0); - REQUIRE(t.NumTotalWrites() == 1); - - // Wrong FCB shouldn't be forwarded - t.OnFrame(LinkFunction::PRI_CONFIRMED_USER_DATA, false, true, false, LinkBroadcastAddress::ShallConfirm, 1024, - b.ToRSeq()); - t.link.OnTxReady(); - REQUIRE(t.upper->receivedQueue.size() == 1); - REQUIRE(t.NumTotalWrites() == 1); -} - // When we get another reset links when we're already reset, // ACK it and reset the link state TEST_CASE(SUITE("SecondaryResetResetLinkStates")) @@ -344,292 +279,3 @@ TEST_CASE(SUITE("CloseBehavior")) t.link.Send(segments); REQUIRE(t.NumTotalWrites() == 2); } - -TEST_CASE(SUITE("ResetLinkTimerExpiration")) -{ - LinkConfig cfg = LinkLayerTest::DefaultConfig(); - cfg.UseConfirms = true; - - LinkLayerTest t(cfg); - t.link.OnLowerLayerUp(); - - MockTransportSegment segments(250, HexConversions::increment_hex(0, 250), cfg.GetAddresses()); - t.link.Send(segments); - REQUIRE(t.NumTotalWrites() == 1); - t.link.OnTxReady(); // reset link - - REQUIRE(t.PopLastWriteAsHex() == LinkHex::ResetLinkStates(true, 1024, 1)); - REQUIRE(t.upper->GetCounters().numTxReady == 0); - - t.exe->advance_time(cfg.Timeout.value); - REQUIRE(t.exe->run_many() > 0); - REQUIRE(t.upper->GetCounters().numTxReady == 1); -} - -TEST_CASE(SUITE("ResetLinkTimerExpirationWithRetry")) -{ - LinkConfig cfg = LinkLayerTest::DefaultConfig(); - cfg.NumRetry = 1; - cfg.UseConfirms = true; - - LinkLayerTest t(cfg); - t.link.OnLowerLayerUp(); - - MockTransportSegment segments(250, HexConversions::increment_hex(0, 250), cfg.GetAddresses()); - t.link.Send(segments); - REQUIRE(t.NumTotalWrites() == 1); - t.link.OnTxReady(); - t.exe->advance_time(cfg.Timeout.value); - REQUIRE(t.exe->run_many() > 0); // timeout the wait for Ack - - REQUIRE(t.upper->GetCounters().numTxReady == 0); - REQUIRE(t.NumTotalWrites() == 2); - - REQUIRE(t.PopLastWriteAsHex() == LinkHex::ResetLinkStates(true, 1024, 1)); // check that reset links got sent again - t.link.OnTxReady(); - - t.OnFrame(LinkFunction::SEC_ACK, false, false, false, 1, 1024); // this time Ack it - REQUIRE(t.NumTotalWrites() == 3); - REQUIRE(t.PopLastWriteAsHex() - == LinkHex::ConfirmedUserData(true, true, 1024, 1, - HexConversions::increment_hex(0x00, 250))); // check that the data got sent - - t.link.OnTxReady(); - - t.exe->advance_time(cfg.Timeout.value); - REQUIRE(t.exe->run_many() > 0); // timeout the ACK - REQUIRE(t.upper->GetCounters().numTxReady == 1); - - // Test retry reset - segments.Reset(); - t.link.Send(segments); - REQUIRE(t.NumTotalWrites() == 4); - t.link.OnTxReady(); - - t.exe->advance_time(cfg.Timeout.value); - REQUIRE(t.exe->run_many() > 0); - REQUIRE(t.upper->GetCounters().numTxReady == 1); -} - -TEST_CASE(SUITE("ResetLinkTimerExpirationWithRetryResetState")) -{ - LinkConfig cfg = LinkLayerTest::DefaultConfig(); - cfg.NumRetry = 1; - cfg.UseConfirms = true; - - LinkLayerTest t(cfg); - t.link.OnLowerLayerUp(); - - MockTransportSegment segments(250, HexConversions::increment_hex(0, 250), cfg.GetAddresses()); - t.link.Send(segments); - REQUIRE(t.NumTotalWrites() == 1); - t.link.OnTxReady(); - t.OnFrame(LinkFunction::SEC_ACK, false, false, false, 1, 1024); - REQUIRE(t.NumTotalWrites() == 2); - t.link.OnTxReady(); - t.OnFrame(LinkFunction::SEC_ACK, false, false, false, 1, 1024); - - REQUIRE(t.exe->run_many() > 0); - REQUIRE(t.upper->GetCounters().numTxReady == 1); - - segments.Reset(); - t.link.Send(segments); - REQUIRE(t.NumTotalWrites() == 3); - t.link.OnTxReady(); - - t.exe->advance_time(cfg.Timeout.value); - REQUIRE(t.exe->run_one()); // timeout - REQUIRE(t.upper->GetCounters().numTxReady == 1); - REQUIRE(t.NumTotalWrites() == 4); - t.link.OnTxReady(); - - t.OnFrame(LinkFunction::SEC_ACK, false, false, false, 1, 1024); - REQUIRE(t.exe->run_many() > 0); - REQUIRE(t.upper->GetCounters().numTxReady == 2); - - // Test retry reset - segments.Reset(); - t.link.Send(segments); - REQUIRE(t.NumTotalWrites() == 5); // Should now be waiting for an ACK with active timer - t.link.OnTxReady(); - - t.exe->advance_time(cfg.Timeout.value); - REQUIRE(t.exe->run_one()); - REQUIRE(t.upper->GetCounters().numTxReady == 2); -} - -TEST_CASE(SUITE("ConfirmedDataRetry")) -{ - LinkConfig cfg = LinkLayerTest::DefaultConfig(); - cfg.NumRetry = 1; - cfg.UseConfirms = true; - - LinkLayerTest t(cfg); - t.link.OnLowerLayerUp(); - - MockTransportSegment segments(250, HexConversions::increment_hex(0, 250), cfg.GetAddresses()); - t.link.Send(segments); - t.link.OnTxReady(); - REQUIRE(t.NumTotalWrites() == 1); // Should now be waiting for an ACK with active timer - - t.OnFrame(LinkFunction::SEC_ACK, false, false, false, 1, 1024); - REQUIRE(t.NumTotalWrites() == 2); - t.link.OnTxReady(); - - t.exe->advance_time(cfg.Timeout.value); - REQUIRE(t.exe->run_many() > 0); // timeout the ConfData, check that it retransmits - REQUIRE(t.NumTotalWrites() == 3); - - REQUIRE(t.PopLastWriteAsHex() - == LinkHex::ConfirmedUserData(true, true, 1024, 1, HexConversions::increment_hex(0x00, 250))); - t.link.OnTxReady(); - - t.OnFrame(LinkFunction::SEC_ACK, false, false, false, 1, 1024); - REQUIRE(t.exe->run_many() > 0); - REQUIRE(t.NumTotalWrites() == 3); - REQUIRE(t.upper->GetCounters().numTxReady == 1); -} - -TEST_CASE(SUITE("ConfirmedDataFailureResetsLink")) -{ - LinkConfig cfg = LinkLayerTest::DefaultConfig(); - cfg.NumRetry = 1; - cfg.UseConfirms = true; - - LinkLayerTest t(cfg); - t.link.OnLowerLayerUp(); - - MockTransportSegment segments(250, HexConversions::increment_hex(0, 250), cfg.GetAddresses()); - t.link.Send(segments); - t.link.OnTxReady(); - REQUIRE(t.NumTotalWrites() == 1); // Should now be waiting for an ACK with active timer - - t.OnFrame(LinkFunction::SEC_ACK, false, false, false, 1, 1024); - REQUIRE(t.NumTotalWrites() == 2); - - REQUIRE(t.PopLastWriteAsHex() - == LinkHex::ConfirmedUserData(true, true, 1024, 1, HexConversions::increment_hex(0x00, 250))); - t.link.OnTxReady(); - - // Timeout original transmission - t.exe->advance_time(cfg.Timeout.value); - REQUIRE(t.exe->run_many() > 0); - REQUIRE(t.NumTotalWrites() == 3); - - REQUIRE(t.PopLastWriteAsHex() - == LinkHex::ConfirmedUserData(true, true, 1024, 1, HexConversions::increment_hex(0x00, 250))); - t.link.OnTxReady(); - - // Timeout retransmission, no more retransmission - t.exe->advance_time(cfg.Timeout.value); - REQUIRE(t.exe->run_many() > 0); - REQUIRE(t.NumTotalWrites() == 3); - - // When sending something else, then reset link state - t.link.Send(segments); - t.link.OnTxReady(); - REQUIRE(t.NumTotalWrites() == 4); // Should now be waiting for an ACK with active timer - REQUIRE(t.PopLastWriteAsHex() == LinkHex::ResetLinkStates(true, 1024, 1)); -} - -TEST_CASE(SUITE("ResetLinkRetries")) -{ - LinkConfig cfg = LinkLayerTest::DefaultConfig(); - cfg.NumRetry = 3; - cfg.UseConfirms = true; - - LinkLayerTest t(cfg); - t.link.OnLowerLayerUp(); - - MockTransportSegment segments(250, HexConversions::increment_hex(0, 250), cfg.GetAddresses()); - t.link.Send(segments); - - for (int i = 1; i < 5; ++i) - { - REQUIRE(t.NumTotalWrites() == i); // sends link retry - REQUIRE(t.PopLastWriteAsHex() == LinkHex::ResetLinkStates(true, 1024, 1)); - t.link.OnTxReady(); - t.exe->advance_time(cfg.Timeout.value); - REQUIRE(t.exe->run_many() > 0); // timeout - } - - REQUIRE(t.NumTotalWrites() == 4); -} - -TEST_CASE(SUITE("ConfirmedDataNackDFCClear")) -{ - LinkConfig cfg = LinkLayerTest::DefaultConfig(); - cfg.NumRetry = 1; - cfg.UseConfirms = true; - - LinkLayerTest t(cfg); - t.link.OnLowerLayerUp(); - - MockTransportSegment segments(250, HexConversions::increment_hex(0, 250), cfg.GetAddresses()); - t.link.Send(segments); - t.link.OnTxReady(); - REQUIRE(t.NumTotalWrites() == 1); // Should now be waiting for an ACK with active timer - - t.OnFrame(LinkFunction::SEC_ACK, false, false, false, 1, 1024); - t.link.OnTxReady(); - REQUIRE(t.NumTotalWrites() == 2); // num transmitting confirmed data - - t.OnFrame(LinkFunction::SEC_NACK, false, false, false, 1, 1024); // test that we try to reset the link again - t.link.OnTxReady(); - REQUIRE(t.NumTotalWrites() == 3); - - t.OnFrame(LinkFunction::SEC_ACK, false, false, false, 1, 1024); // ACK the link reset - REQUIRE(t.NumTotalWrites() == 4); -} - -TEST_CASE(SUITE("SendDataTimerExpiration")) -{ - LinkConfig cfg = LinkLayerTest::DefaultConfig(); - cfg.UseConfirms = true; - - LinkLayerTest t(cfg); - t.link.OnLowerLayerUp(); - - MockTransportSegment segments(250, HexConversions::increment_hex(0, 250), cfg.GetAddresses()); - t.link.Send(segments); - REQUIRE(t.NumTotalWrites() == 1); - t.link.OnTxReady(); - - t.OnFrame(LinkFunction::SEC_ACK, false, false, false, 1, 1024); // ACK the reset links - REQUIRE(t.NumTotalWrites() == 2); - - REQUIRE(t.NumTotalWrites() == 2); - REQUIRE(t.PopLastWriteAsHex() - == LinkHex::ConfirmedUserData(true, true, 1024, 1, - HexConversions::increment_hex(0x00, 250))); // check that data was sent - t.link.OnTxReady(); - - t.exe->advance_time(cfg.Timeout.value); - REQUIRE(t.exe->run_many() > 0); // trigger the timeout callback - REQUIRE(t.upper->GetCounters().numTxReady == 1); -} - -TEST_CASE(SUITE("SendDataSuccess")) -{ - LinkConfig cfg = LinkLayerTest::DefaultConfig(); - cfg.UseConfirms = true; - - LinkLayerTest t(cfg); - t.link.OnLowerLayerUp(); - - MockTransportSegment segments(250, HexConversions::increment_hex(0, 250), cfg.GetAddresses()); - t.link.Send(segments); - t.link.OnTxReady(); - t.OnFrame(LinkFunction::SEC_ACK, false, false, false, 1, 1024); - t.link.OnTxReady(); - t.OnFrame(LinkFunction::SEC_ACK, false, false, false, 1, 1024); - REQUIRE(t.exe->run_many() > 0); - REQUIRE(t.upper->GetCounters().numTxReady == 1); - - segments.Reset(); - t.link.Send(segments); // now we should be directly sending w/o having to reset, and the FCB should flip - - REQUIRE(t.NumTotalWrites() == 3); - REQUIRE(t.PopLastWriteAsHex() - == LinkHex::ConfirmedUserData(true, false, 1024, 1, HexConversions::increment_hex(0x00, 250))); -} diff --git a/dotnet/CLRAdapter/src/Conversions.cpp b/dotnet/CLRAdapter/src/Conversions.cpp index b828d1dc4f..612087d5c4 100644 --- a/dotnet/CLRAdapter/src/Conversions.cpp +++ b/dotnet/CLRAdapter/src/Conversions.cpp @@ -362,9 +362,8 @@ namespace DNP3 opendnp3::LinkConfig Conversions::ConvertConfig(LinkConfig ^ config) { - return opendnp3::LinkConfig(config->isMaster, config->useConfirms, config->numRetry, config->localAddr, - config->remoteAddr, ConvertTimespan(config->responseTimeout), - ConvertTimespan(config->keepAliveTimeout)); + return opendnp3::LinkConfig(config->isMaster, config->localAddr, config->remoteAddr, + ConvertTimespan(config->responseTimeout), ConvertTimespan(config->keepAliveTimeout)); } opendnp3::EventBufferConfig Conversions::ConvertConfig(EventBufferConfig ^ cm) diff --git a/dotnet/CLRInterface/src/config/LinkConfig.cs b/dotnet/CLRInterface/src/config/LinkConfig.cs index 72bb6f8bfc..d2669e9188 100644 --- a/dotnet/CLRInterface/src/config/LinkConfig.cs +++ b/dotnet/CLRInterface/src/config/LinkConfig.cs @@ -33,23 +33,17 @@ public class LinkConfig /// Full constructor /// /// true if this layer will be used with a master, false otherwise - /// true to use link layer confirmations for all data, false otherwise - /// The number of retry attempts the link will attempt after the initial try /// dnp3 address of the local device /// dnp3 address of the remote device /// the response timeout for confirmed requests /// the keep-alive timeout interval public LinkConfig( bool isMaster, - bool useConfirms, - System.UInt32 numRetry, System.UInt16 localAddr, System.UInt16 remoteAddr, TimeSpan responseTimeout, TimeSpan keepAliveTimeout) { this.isMaster = isMaster; - this.useConfirms = useConfirms; - this.numRetry = numRetry; this.localAddr = localAddr; this.remoteAddr = remoteAddr; this.responseTimeout = responseTimeout; @@ -64,8 +58,6 @@ public LinkConfig( bool isMaster, public LinkConfig(bool isMaster, bool useConfirms) : this( isMaster, - useConfirms, - DefaultNumRetries, GetDefaultSourceAddress(isMaster), GetDefaultDestinationAddress(isMaster), DefaultResponseTimeout, @@ -100,14 +92,6 @@ public static System.UInt16 DefaultMasterAddress } } - public static System.UInt32 DefaultNumRetries - { - get - { - return 0; - } - } - public static System.UInt16 GetDefaultSourceAddress(bool isMaster) { return isMaster ? DefaultMasterAddress : DefaultOutstationAddress; @@ -144,16 +128,6 @@ public static TimeSpan DefaultKeepAliveTimeout /// public bool isMaster; - /// - /// If true, the link layer will send data requesting confirmation - /// - public bool useConfirms; - - /// - /// The number of retry attempts the link will attempt after the initial try - /// - public System.UInt32 numRetry; - /// /// dnp3 address of the local device /// diff --git a/java/bindings/src/main/java/com/automatak/dnp3/LinkLayerConfig.java b/java/bindings/src/main/java/com/automatak/dnp3/LinkLayerConfig.java index c87928021a..206dd81048 100644 --- a/java/bindings/src/main/java/com/automatak/dnp3/LinkLayerConfig.java +++ b/java/bindings/src/main/java/com/automatak/dnp3/LinkLayerConfig.java @@ -31,21 +31,6 @@ public class LinkLayerConfig { */ public boolean isMaster; - /** - * If true, the link layer will send data requesting confirmation. This is generally NEVER - * set for a TCP connection and only sometimes set for serial. - * - * defaults to false - */ - public boolean useConfirms = false; - - /** - * The number of retry attempts the link will attempt after the initial try if using confirms - * - * defaults to 0 - */ - public int numRetry = 0; - /** * dnp3 address of the local device as a 16-bit unsigned integer */ diff --git a/java/cpp/adapters/ConfigReader.cpp b/java/cpp/adapters/ConfigReader.cpp index 309be3aacd..1cd770060d 100644 --- a/java/cpp/adapters/ConfigReader.cpp +++ b/java/cpp/adapters/ConfigReader.cpp @@ -68,8 +68,6 @@ LinkConfig ConfigReader::Convert(JNIEnv* env, jni::JLinkLayerConfig jlinkcfg) auto& ref = jni::JCache::LinkLayerConfig; cfg.IsMaster = !(ref.getisMaster(env, jlinkcfg) == 0u); - cfg.UseConfirms = !(ref.getuseConfirms(env, jlinkcfg) == 0u); - cfg.NumRetry = ref.getnumRetry(env, jlinkcfg); cfg.LocalAddr = static_cast(ref.getlocalAddr(env, jlinkcfg)); cfg.RemoteAddr = static_cast(ref.getremoteAddr(env, jlinkcfg)); cfg.Timeout diff --git a/java/cpp/jni/JNILinkLayerConfig.cpp b/java/cpp/jni/JNILinkLayerConfig.cpp index c68f9374aa..bc5d0a5efd 100644 --- a/java/cpp/jni/JNILinkLayerConfig.cpp +++ b/java/cpp/jni/JNILinkLayerConfig.cpp @@ -45,12 +45,6 @@ namespace jni this->isMasterField = env->GetFieldID(this->clazz, "isMaster", "Z"); if(!this->isMasterField) return false; - this->useConfirmsField = env->GetFieldID(this->clazz, "useConfirms", "Z"); - if(!this->useConfirmsField) return false; - - this->numRetryField = env->GetFieldID(this->clazz, "numRetry", "I"); - if(!this->numRetryField) return false; - this->localAddrField = env->GetFieldID(this->clazz, "localAddr", "I"); if(!this->localAddrField) return false; @@ -86,11 +80,6 @@ namespace jni return env->GetIntField(instance, this->localAddrField); } - jint LinkLayerConfig::getnumRetry(JNIEnv* env, JLinkLayerConfig instance) - { - return env->GetIntField(instance, this->numRetryField); - } - jint LinkLayerConfig::getremoteAddr(JNIEnv* env, JLinkLayerConfig instance) { return env->GetIntField(instance, this->remoteAddrField); @@ -100,10 +89,5 @@ namespace jni { return LocalRef(env, env->GetObjectField(instance, this->responseTimeoutField)); } - - jboolean LinkLayerConfig::getuseConfirms(JNIEnv* env, JLinkLayerConfig instance) - { - return env->GetBooleanField(instance, this->useConfirmsField); - } } } diff --git a/java/cpp/jni/JNILinkLayerConfig.h b/java/cpp/jni/JNILinkLayerConfig.h index eb41330678..da44fb4f93 100644 --- a/java/cpp/jni/JNILinkLayerConfig.h +++ b/java/cpp/jni/JNILinkLayerConfig.h @@ -55,10 +55,8 @@ namespace jni jboolean getisMaster(JNIEnv* env, JLinkLayerConfig instance); LocalRef getkeepAliveTimeout(JNIEnv* env, JLinkLayerConfig instance); jint getlocalAddr(JNIEnv* env, JLinkLayerConfig instance); - jint getnumRetry(JNIEnv* env, JLinkLayerConfig instance); jint getremoteAddr(JNIEnv* env, JLinkLayerConfig instance); LocalRef getresponseTimeout(JNIEnv* env, JLinkLayerConfig instance); - jboolean getuseConfirms(JNIEnv* env, JLinkLayerConfig instance); private: @@ -66,8 +64,6 @@ namespace jni // field ids jfieldID isMasterField = nullptr; - jfieldID useConfirmsField = nullptr; - jfieldID numRetryField = nullptr; jfieldID localAddrField = nullptr; jfieldID remoteAddrField = nullptr; jfieldID responseTimeoutField = nullptr;