changeset 4451:f4dbdb2dcba6

new configuration option "MaximumPduLength" to tune the maximum PDU length
author Sebastien Jodogne <s.jodogne@gmail.com>
date Mon, 18 Jan 2021 14:51:52 +0100
parents 9bf2f9e0af47
children a86fd55c4df0
files NEWS OrthancFramework/Sources/DicomNetworking/DicomAssociation.cpp OrthancFramework/Sources/DicomNetworking/DicomAssociationParameters.cpp OrthancFramework/Sources/DicomNetworking/DicomAssociationParameters.h OrthancFramework/Sources/DicomNetworking/DicomServer.cpp OrthancFramework/Sources/DicomNetworking/DicomServer.h OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.cpp OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.h OrthancFramework/Sources/SerializationToolbox.cpp OrthancFramework/Sources/SerializationToolbox.h OrthancFramework/UnitTestsSources/JobsTests.cpp OrthancServer/Resources/Configuration.json OrthancServer/Sources/main.cpp
diffstat 13 files changed, 164 insertions(+), 28 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Thu Jan 14 09:13:29 2021 +0100
+++ b/NEWS	Mon Jan 18 14:51:52 2021 +0100
@@ -5,12 +5,13 @@
 -------
 
 * Support of DICOM TLS
-* New configuration options related to DICOM TLS:
+* New configuration options related to DICOM networking:
   - "DicomTlsEnabled" to enable DICOM TLS in Orthanc SCP
   - "DicomTlsCertificate" to provide the TLS certificate to be used in both Orthanc SCU and SCP
   - "DicomTlsPrivateKey" to provide the private key of the TLS certificate
   - "DicomTlsTrustedCertificates" to provide the list of TLS certificates to be trusted by Orthanc
   - "UseDicomTls" in "DicomModalities" to enable DICOM TLS in outgoing SCU on a per-modality basis
+  - "MaximumPduLength" to tune the maximum PDU length (Protocol Data Unit)
 * New command-line option: "--openapi" to write the OpenAPI documentation of the REST API to a file
 
 Plugins
--- a/OrthancFramework/Sources/DicomNetworking/DicomAssociation.cpp	Thu Jan 14 09:13:29 2021 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/DicomAssociation.cpp	Mon Jan 18 14:51:52 2021 +0100
@@ -274,7 +274,7 @@
                       << " (manufacturer: " << EnumerationToString(parameters.GetRemoteModality().GetManufacturer()) << ")";
 
     CheckConnecting(parameters, ASC_initializeNetwork(NET_REQUESTOR, 0, /*opt_acse_timeout*/ acseTimeout, &net_));
-    CheckConnecting(parameters, ASC_createAssociationParameters(&params_, /*opt_maxReceivePDULength*/ ASC_DEFAULTMAXPDU));
+    CheckConnecting(parameters, ASC_createAssociationParameters(&params_, parameters.GetMaximumPduLength()));
 
 #if ORTHANC_ENABLE_SSL == 1
     if (parameters.GetRemoteModality().IsDicomTlsEnabled())
--- a/OrthancFramework/Sources/DicomNetworking/DicomAssociationParameters.cpp	Thu Jan 14 09:13:29 2021 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/DicomAssociationParameters.cpp	Mon Jan 18 14:51:52 2021 +0100
@@ -30,6 +30,8 @@
 #include "../SystemToolbox.h"
 #include "NetworkingCompatibility.h"
 
+#include <dcmtk/dcmnet/diutil.h>  // For ASC_DEFAULTMAXPDU
+
 #include <boost/thread/mutex.hpp>
 
 // By default, the default timeout for client DICOM connections is set to 10 seconds
@@ -38,6 +40,7 @@
 static std::string   defaultOwnPrivateKeyPath_;
 static std::string   defaultOwnCertificatePath_;
 static std::string   defaultTrustedCertificatesPath_;
+static unsigned int  defaultMaximumPduLength_ = ASC_DEFAULTMAXPDU;
 
 
 namespace Orthanc
@@ -66,12 +69,14 @@
     ownPrivateKeyPath_ = defaultOwnPrivateKeyPath_;
     ownCertificatePath_ = defaultOwnCertificatePath_;
     trustedCertificatesPath_ = defaultTrustedCertificatesPath_;
+    maximumPduLength_ = defaultMaximumPduLength_;
   }
 
 
   DicomAssociationParameters::DicomAssociationParameters() :
     localAet_("ORTHANC"),
-    timeout_(0)  // Will be set by SetDefaultParameters()
+    timeout_(0),  // Will be set by SetDefaultParameters()
+    maximumPduLength_(0)  // Will be set by SetDefaultParameters()
   {
     remote_.SetApplicationEntityTitle("ANY-SCP");
     SetDefaultParameters();
@@ -81,7 +86,8 @@
   DicomAssociationParameters::DicomAssociationParameters(const std::string& localAet,
                                                          const RemoteModalityParameters& remote) :
     localAet_(localAet),
-    timeout_(0)  // Will be set by SetDefaultParameters()
+    timeout_(0),  // Will be set by SetDefaultParameters()
+    maximumPduLength_(0)  // Will be set by SetDefaultParameters()
   {
     SetRemoteModality(remote);
     SetDefaultParameters();
@@ -139,7 +145,11 @@
             remote_.GetHost() == other.remote_.GetHost() &&
             remote_.GetPortNumber() == other.remote_.GetPortNumber() &&
             remote_.GetManufacturer() == other.remote_.GetManufacturer() &&
-            timeout_ == other.timeout_);
+            timeout_ == other.timeout_ &&
+            ownPrivateKeyPath_ == other.ownPrivateKeyPath_ &&
+            ownCertificatePath_ == other.ownCertificatePath_ &&
+            trustedCertificatesPath_ == other.trustedCertificatesPath_ &&
+            maximumPduLength_ == other.maximumPduLength_);
   }
 
   void DicomAssociationParameters::SetTimeout(uint32_t seconds)
@@ -211,7 +221,18 @@
     return trustedCertificatesPath_;
   }
 
+  unsigned int DicomAssociationParameters::GetMaximumPduLength() const
+  {
+    return maximumPduLength_;
+  }
 
+  void DicomAssociationParameters::SetMaximumPduLength(unsigned int pdu)
+  {
+    CheckMaximumPduLength(pdu);
+    maximumPduLength_ = pdu;
+  }
+    
+  
 
   static const char* const LOCAL_AET = "LocalAet";
   static const char* const REMOTE = "Remote";
@@ -219,6 +240,7 @@
   static const char* const OWN_PRIVATE_KEY = "OwnPrivateKey";             // New in Orthanc 1.9.0
   static const char* const OWN_CERTIFICATE = "OwnCertificate";            // New in Orthanc 1.9.0
   static const char* const TRUSTED_CERTIFICATES = "TrustedCertificates";  // New in Orthanc 1.9.0
+  static const char* const MAXIMUM_PDU_LENGTH = "MaximumPduLength";       // New in Orthanc 1.9.0
 
   
   void DicomAssociationParameters::SerializeJob(Json::Value& target) const
@@ -232,6 +254,7 @@
       target[LOCAL_AET] = localAet_;
       remote_.Serialize(target[REMOTE], true /* force advanced format */);
       target[TIMEOUT] = timeout_;
+      target[MAXIMUM_PDU_LENGTH] = maximumPduLength_;
 
       // Don't write the DICOM TLS parameters if they are not required
       if (ownPrivateKeyPath_.empty())
@@ -279,6 +302,13 @@
       result.localAet_ = SerializationToolbox::ReadString(serialized, LOCAL_AET);
       result.timeout_ = SerializationToolbox::ReadInteger(serialized, TIMEOUT, GetDefaultTimeout());
 
+      // The calls to "isMember()" below are for compatibility with Orthanc <= 1.8.2 serialization
+      if (serialized.isMember(MAXIMUM_PDU_LENGTH))
+      {
+        result.maximumPduLength_ = SerializationToolbox::ReadUnsignedInteger(
+          serialized, MAXIMUM_PDU_LENGTH, defaultMaximumPduLength_);
+      }
+
       if (serialized.isMember(OWN_PRIVATE_KEY))
       {
         result.ownPrivateKeyPath_ = SerializationToolbox::ReadString(serialized, OWN_PRIVATE_KEY);
@@ -394,4 +424,38 @@
       defaultTrustedCertificatesPath_.clear();
     }
   }
+
+
+
+  void DicomAssociationParameters::CheckMaximumPduLength(unsigned int pdu)
+  {
+    if (pdu > ASC_MAXIMUMPDUSIZE)
+    {
+      throw OrthancException(ErrorCode_ParameterOutOfRange, "Maximum PDU length must be smaller than " +
+                             boost::lexical_cast<std::string>(ASC_MAXIMUMPDUSIZE));
+    }
+    else if (pdu < ASC_MINIMUMPDUSIZE)
+    {
+      throw OrthancException(ErrorCode_ParameterOutOfRange, "Maximum PDU length must be greater than " +
+                             boost::lexical_cast<std::string>(ASC_MINIMUMPDUSIZE));
+    }
+  }
+
+
+  void DicomAssociationParameters::SetDefaultMaximumPduLength(unsigned int pdu)
+  {
+    CheckMaximumPduLength(pdu);
+
+    {
+      boost::mutex::scoped_lock lock(defaultConfigurationMutex_);
+      defaultMaximumPduLength_ = pdu;
+    }
+  }
+
+
+  unsigned int DicomAssociationParameters::GetDefaultMaximumPduLength()
+  {
+    boost::mutex::scoped_lock lock(defaultConfigurationMutex_);
+    return defaultMaximumPduLength_;
+  }
 }
--- a/OrthancFramework/Sources/DicomNetworking/DicomAssociationParameters.h	Thu Jan 14 09:13:29 2021 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/DicomAssociationParameters.h	Mon Jan 18 14:51:52 2021 +0100
@@ -39,6 +39,7 @@
     std::string               ownPrivateKeyPath_;
     std::string               ownCertificatePath_;
     std::string               trustedCertificatesPath_;
+    unsigned int              maximumPduLength_;
 
     static void CheckHost(const std::string& host);
 
@@ -93,6 +94,10 @@
     const std::string& GetOwnCertificatePath() const;
 
     const std::string& GetTrustedCertificatesPath() const;
+
+    unsigned int GetMaximumPduLength() const;
+
+    void SetMaximumPduLength(unsigned int pdu);
     
     void SerializeJob(Json::Value& target) const;
 
@@ -106,5 +111,11 @@
                                              const std::string& certificatePath);
 
     static void SetDefaultTrustedCertificatesPath(const std::string& path);
+
+    static void CheckMaximumPduLength(unsigned int pdu);
+
+    static void SetDefaultMaximumPduLength(unsigned int pdu);
+
+    static unsigned int GetDefaultMaximumPduLength();
   };
 }
--- a/OrthancFramework/Sources/DicomNetworking/DicomServer.cpp	Thu Jan 14 09:13:29 2021 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/DicomServer.cpp	Mon Jan 18 14:51:52 2021 +0100
@@ -28,6 +28,7 @@
 #include "../OrthancException.h"
 #include "../SystemToolbox.h"
 #include "../Toolbox.h"
+#include "DicomAssociationParameters.h"
 #include "Internals/CommandDispatcher.h"
 
 #include <boost/thread.hpp>
@@ -56,6 +57,7 @@
 
 
   void DicomServer::ServerThread(DicomServer* server,
+                                 unsigned int maximumPduLength,
                                  bool useDicomTls)
   {
     CLOG(INFO, DICOM) << "DICOM server started";
@@ -65,7 +67,7 @@
       /* receive an association and acknowledge or reject it. If the association was */
       /* acknowledged, offer corresponding services and invoke one or more if required. */
       std::unique_ptr<Internals::CommandDispatcher> dispatcher(
-        Internals::AcceptAssociation(*server, server->pimpl_->network_, useDicomTls));
+        Internals::AcceptAssociation(*server, server->pimpl_->network_, maximumPduLength, useDicomTls));
 
       try
       {
@@ -86,21 +88,22 @@
 
   DicomServer::DicomServer() : 
     pimpl_(new PImpl),
+    checkCalledAet_(true),
     aet_("ANY-SCP"),
-    useDicomTls_(false)
+    port_(104),
+    continue_(false),
+    associationTimeout_(30),
+    modalities_(NULL),
+    findRequestHandlerFactory_(NULL),
+    moveRequestHandlerFactory_(NULL),
+    getRequestHandlerFactory_(NULL),
+    storeRequestHandlerFactory_(NULL),
+    worklistRequestHandlerFactory_(NULL),
+    storageCommitmentFactory_(NULL),
+    applicationEntityFilter_(NULL),
+    useDicomTls_(false),
+    maximumPduLength_(ASC_DEFAULTMAXPDU)
   {
-    port_ = 104;
-    modalities_ = NULL;
-    findRequestHandlerFactory_ = NULL;
-    moveRequestHandlerFactory_ = NULL;
-    getRequestHandlerFactory_ = NULL;
-    storeRequestHandlerFactory_ = NULL;
-    worklistRequestHandlerFactory_ = NULL;
-    storageCommitmentFactory_ = NULL;
-    applicationEntityFilter_ = NULL;
-    checkCalledAet_ = true;
-    associationTimeout_ = 30;
-    continue_ = false;
   }
 
   DicomServer::~DicomServer()
@@ -420,7 +423,7 @@
 
     continue_ = true;
     pimpl_->workers_.reset(new RunnableWorkersPool(4));   // Use 4 workers - TODO as a parameter?
-    pimpl_->thread_ = boost::thread(ServerThread, this, useDicomTls_);
+    pimpl_->thread_ = boost::thread(ServerThread, this, maximumPduLength_, useDicomTls_);
   }
 
 
@@ -559,4 +562,17 @@
   {
     return trustedCertificatesPath_;
   }
+
+  unsigned int DicomServer::GetMaximumPduLength() const
+  {
+    return maximumPduLength_;
+  }
+
+  void DicomServer::SetMaximumPduLength(unsigned int pdu)
+  {
+    DicomAssociationParameters::CheckMaximumPduLength(pdu);
+
+    Stop();
+    maximumPduLength_ = pdu;
+  }
 }
--- a/OrthancFramework/Sources/DicomNetworking/DicomServer.h	Thu Jan 14 09:13:29 2021 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/DicomServer.h	Mon Jan 18 14:51:52 2021 +0100
@@ -86,8 +86,10 @@
     std::string  ownPrivateKeyPath_;
     std::string  ownCertificatePath_;
     std::string  trustedCertificatesPath_;
+    unsigned int maximumPduLength_;
 
     static void ServerThread(DicomServer* server,
+                             unsigned int maximumPduLength,
                              bool useDicomTls);
 
   public:
@@ -154,6 +156,8 @@
     
     void SetTrustedCertificatesPath(const std::string& path);
     const std::string& GetTrustedCertificatesPath() const;
+
+    unsigned int GetMaximumPduLength() const;
+    void SetMaximumPduLength(unsigned int pdu);
   };
-
 }
--- a/OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.cpp	Thu Jan 14 09:13:29 2021 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.cpp	Mon Jan 18 14:51:52 2021 +0100
@@ -255,6 +255,7 @@
 
     CommandDispatcher* AcceptAssociation(const DicomServer& server,
                                          T_ASC_Network *net,
+                                         unsigned int maximumPduLength,
                                          bool useDicomTls)
     {
       DcmAssociationConfiguration asccfg;
@@ -264,9 +265,7 @@
       OFString sprofile;
       OFString temp_str;
 
-      cond = ASC_receiveAssociation(net, &assoc, 
-                                    /*opt_maxPDU*/ ASC_DEFAULTMAXPDU, 
-                                    NULL, NULL,
+      cond = ASC_receiveAssociation(net, &assoc, maximumPduLength, NULL, NULL,
                                     useDicomTls /*opt_secureConnection*/,
                                     DUL_NOBLOCK, 1);
 
@@ -694,7 +693,7 @@
       }
 
       IApplicationEntityFilter* filter = server.HasApplicationEntityFilter() ? &server.GetApplicationEntityFilter() : NULL;
-      return new CommandDispatcher(server, assoc, remoteIp, remoteAet, calledAet, filter);
+      return new CommandDispatcher(server, assoc, remoteIp, remoteAet, calledAet, maximumPduLength, filter);
     }
 
 
@@ -703,6 +702,7 @@
                                          const std::string& remoteIp,
                                          const std::string& remoteAet,
                                          const std::string& calledAet,
+                                         unsigned int maximumPduLength,
                                          IApplicationEntityFilter* filter) :
       server_(server),
       assoc_(assoc),
--- a/OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.h	Thu Jan 14 09:13:29 2021 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.h	Mon Jan 18 14:51:52 2021 +0100
@@ -57,6 +57,7 @@
                         const std::string& remoteIp,
                         const std::string& remoteAet,
                         const std::string& calledAet,
+                        unsigned int maximumPduLength,
                         IApplicationEntityFilter* filter);
 
       virtual ~CommandDispatcher();
@@ -66,6 +67,7 @@
 
     CommandDispatcher* AcceptAssociation(const DicomServer& server, 
                                          T_ASC_Network *net,
+                                         unsigned int maximumPduLength,
                                          bool useDicomTls);
 
     OFCondition EchoScp(T_ASC_Association* assoc, 
--- a/OrthancFramework/Sources/SerializationToolbox.cpp	Thu Jan 14 09:13:29 2021 +0100
+++ b/OrthancFramework/Sources/SerializationToolbox.cpp	Mon Jan 18 14:51:52 2021 +0100
@@ -117,6 +117,21 @@
   }
 
 
+  unsigned int SerializationToolbox::ReadUnsignedInteger(const Json::Value& value,
+                                                         const std::string& field,
+                                                         unsigned int defaultValue)
+  {
+    if (value.isMember(field.c_str()))
+    {
+      return ReadUnsignedInteger(value, field);
+    }
+    else
+    {
+      return defaultValue;
+    }
+  }
+
+
   bool SerializationToolbox::ReadBoolean(const Json::Value& value,
                                          const std::string& field)
   {
--- a/OrthancFramework/Sources/SerializationToolbox.h	Thu Jan 14 09:13:29 2021 +0100
+++ b/OrthancFramework/Sources/SerializationToolbox.h	Mon Jan 18 14:51:52 2021 +0100
@@ -47,6 +47,10 @@
     static unsigned int ReadUnsignedInteger(const Json::Value& value,
                                             const std::string& field);
 
+    static unsigned int ReadUnsignedInteger(const Json::Value& value,
+                                            const std::string& field,
+                                            unsigned int defaultValue);
+
     static bool ReadBoolean(const Json::Value& value,
                             const std::string& field);
 
--- a/OrthancFramework/UnitTestsSources/JobsTests.cpp	Thu Jan 14 09:13:29 2021 +0100
+++ b/OrthancFramework/UnitTestsSources/JobsTests.cpp	Mon Jan 18 14:51:52 2021 +0100
@@ -1444,8 +1444,9 @@
     ASSERT_EQ("ORTHANC", v["LocalAet"].asString());
     ASSERT_EQ(DicomAssociationParameters::GetDefaultTimeout(), v["Timeout"].asInt());
     ASSERT_TRUE(v.isMember("Remote"));
+    ASSERT_TRUE(v.isMember("MaximumPduLength"));
 
-    ASSERT_EQ(3u, v.getMemberNames().size());
+    ASSERT_EQ(4u, v.getMemberNames().size());
   
     DicomAssociationParameters b;
     b.UnserializeJob(v);
@@ -1453,6 +1454,7 @@
     ASSERT_EQ("127.0.0.1", b.GetRemoteModality().GetHost());
     ASSERT_EQ(104u, b.GetRemoteModality().GetPortNumber());
     ASSERT_EQ("ORTHANC", b.GetLocalApplicationEntityTitle());
+    ASSERT_EQ(DicomAssociationParameters::GetDefaultMaximumPduLength(), b.GetMaximumPduLength());
     ASSERT_FALSE(b.GetRemoteModality().IsDicomTlsEnabled());
   }
 
@@ -1467,10 +1469,15 @@
     a.SetOwnCertificatePath("key", "crt");
     a.SetTrustedCertificatesPath("trusted");
 
+    ASSERT_THROW(a.SetMaximumPduLength(4095), OrthancException);
+    ASSERT_THROW(a.SetMaximumPduLength(131073), OrthancException);
+    a.SetMaximumPduLength(4096);
+    a.SetMaximumPduLength(131072);
+
     Json::Value v = Json::objectValue;
     a.SerializeJob(v);
 
-    ASSERT_EQ(6u, v.getMemberNames().size());
+    ASSERT_EQ(7u, v.getMemberNames().size());
   
     DicomAssociationParameters b = DicomAssociationParameters::UnserializeJob(v);
 
@@ -1482,5 +1489,6 @@
     ASSERT_EQ("key", b.GetOwnPrivateKeyPath());
     ASSERT_EQ("crt", b.GetOwnCertificatePath());
     ASSERT_EQ("trusted", b.GetTrustedCertificatesPath());
+    ASSERT_EQ(131072, b.GetMaximumPduLength());
   }  
 }
--- a/OrthancServer/Resources/Configuration.json	Thu Jan 14 09:13:29 2021 +0100
+++ b/OrthancServer/Resources/Configuration.json	Mon Jan 18 14:51:52 2021 +0100
@@ -714,5 +714,11 @@
   // follow for the deidentification/anonymization of the query
   // contents. Possible values are "2008" and "2017c" (new in Orthanc
   // 1.8.2)
-  "DeidentifyLogsDicomVersion" : "2017c"
+  "DeidentifyLogsDicomVersion" : "2017c",
+
+  // Maximum length of the PDU (Protocol Data Unit) in the DICOM
+  // network protocol, expressed in bytes. This value affects both
+  // Orthanc SCU and Orthanc SCP. It defaults to 16KB. The allowed
+  // range is [4096,131072]. (new in Orthanc 1.9.0)
+  "MaximumPduLength" : 16384
 }
--- a/OrthancServer/Sources/main.cpp	Thu Jan 14 09:13:29 2021 +0100
+++ b/OrthancServer/Sources/main.cpp	Mon Jan 18 14:51:52 2021 +0100
@@ -68,6 +68,7 @@
 static const char* const KEY_DICOM_TLS_ENABLED = "DicomTlsEnabled";
 static const char* const KEY_DICOM_TLS_CERTIFICATE = "DicomTlsCertificate";
 static const char* const KEY_DICOM_TLS_TRUSTED_CERTIFICATES = "DicomTlsTrustedCertificates";
+static const char* const KEY_MAXIMUM_PDU_LENGTH = "MaximumPduLength";
 
 
 class OrthancStoreRequestHandler : public IStoreRequestHandler
@@ -1207,6 +1208,8 @@
         dicomServer.SetTrustedCertificatesPath(
           lock.GetConfiguration().GetStringParameter(KEY_DICOM_TLS_TRUSTED_CERTIFICATES, ""));
       }
+
+      dicomServer.SetMaximumPduLength(lock.GetConfiguration().GetUnsignedIntegerParameter(KEY_MAXIMUM_PDU_LENGTH, 16384));
     }
 
 #if ORTHANC_ENABLE_PLUGINS == 1
@@ -1462,6 +1465,8 @@
       lock.GetConfiguration().GetStringParameter(KEY_DICOM_TLS_CERTIFICATE, ""));
     DicomAssociationParameters::SetDefaultTrustedCertificatesPath(
       lock.GetConfiguration().GetStringParameter(KEY_DICOM_TLS_TRUSTED_CERTIFICATES, ""));
+    DicomAssociationParameters::SetDefaultMaximumPduLength(
+      lock.GetConfiguration().GetUnsignedIntegerParameter(KEY_MAXIMUM_PDU_LENGTH, 16384));
   }
   
   ServerContext context(database, storageArea, false /* not running unit tests */, maxCompletedJobs);