From fe8196387357de214365873db29b2a2eb8b8a1a6 Mon Sep 17 00:00:00 2001 From: Dhanya Thattil <33750417+thattil@users.noreply.github.com> Date: Fri, 25 Sep 2020 10:15:39 +0200 Subject: [PATCH] rxr: udp socket size max of INT_MAX/2 (#191) --- python/slsdet/detector.py | 2 +- slsDetectorSoftware/include/Detector.h | 8 +++---- slsDetectorSoftware/src/CmdProxy.h | 4 ++-- slsDetectorSoftware/src/Detector.cpp | 6 +++--- slsDetectorSoftware/src/Module.cpp | 14 ++++++------ slsDetectorSoftware/src/Module.h | 6 +++--- slsReceiverSoftware/src/ClientInterface.cpp | 24 ++++++++++++++------- slsReceiverSoftware/src/Implementation.cpp | 19 ++++++++++------ slsReceiverSoftware/src/Implementation.h | 10 ++++----- slsReceiverSoftware/src/Listener.cpp | 16 ++++++-------- slsReceiverSoftware/src/Listener.h | 12 +++++------ slsSupportLib/include/UdpRxSocket.h | 6 +++--- slsSupportLib/src/UdpRxSocket.cpp | 8 +++---- 13 files changed, 74 insertions(+), 61 deletions(-) diff --git a/python/slsdet/detector.py b/python/slsdet/detector.py index ba3368989..f254e5ed5 100755 --- a/python/slsdet/detector.py +++ b/python/slsdet/detector.py @@ -1298,7 +1298,7 @@ class Detector(CppDetectorApi): @property @element def rx_udpsocksize(self): - """UDP socket buffer size in receiver. Tune rmem_default and rmem_max accordingly.""" + """UDP socket buffer size in receiver. Tune rmem_default and rmem_max accordingly. Max size: INT_MAX/2.""" return self.getRxUDPSocketBufferSize() @rx_udpsocksize.setter diff --git a/slsDetectorSoftware/include/Detector.h b/slsDetectorSoftware/include/Detector.h index 100e3e706..6ad7416c1 100644 --- a/slsDetectorSoftware/include/Detector.h +++ b/slsDetectorSoftware/include/Detector.h @@ -717,17 +717,17 @@ class Detector { /** Default: padding enabled. Disabling padding is the fastest */ void setPartialFramesPadding(bool value, Positions pos = {}); - Result getRxUDPSocketBufferSize(Positions pos = {}) const; + Result getRxUDPSocketBufferSize(Positions pos = {}) const; /** UDP socket buffer size in receiver. Tune rmem_default and rmem_max - * accordingly */ - void setRxUDPSocketBufferSize(int64_t udpsockbufsize, Positions pos = {}); + * accordingly. Max value is INT_MAX/2. */ + void setRxUDPSocketBufferSize(int udpsockbufsize, Positions pos = {}); /** TODO: * Gets actual udp socket buffer size. Double the size of rx_udpsocksize due * to kernel bookkeeping. */ - Result getRxRealUDPSocketBufferSize(Positions pos = {}) const; + Result getRxRealUDPSocketBufferSize(Positions pos = {}) const; Result getRxLock(Positions pos = {}); diff --git a/slsDetectorSoftware/src/CmdProxy.h b/slsDetectorSoftware/src/CmdProxy.h index 89929f5b7..a1b70d293 100644 --- a/slsDetectorSoftware/src/CmdProxy.h +++ b/slsDetectorSoftware/src/CmdProxy.h @@ -1568,9 +1568,9 @@ class CmdProxy { INTEGER_COMMAND_VEC_ID( rx_udpsocksize, getRxUDPSocketBufferSize, setRxUDPSocketBufferSize, - StringTo, + StringTo, "[n_size]\n\tUDP socket buffer size in receiver. Tune rmem_default and " - "rmem_max accordingly."); + "rmem_max accordingly. Max value is INT_MAX/2."); GET_COMMAND(rx_realudpsocksize, getRxRealUDPSocketBufferSize, "\n\tActual udp socket buffer size. Double the size of " diff --git a/slsDetectorSoftware/src/Detector.cpp b/slsDetectorSoftware/src/Detector.cpp index ee47cf05e..58dae7d45 100644 --- a/slsDetectorSoftware/src/Detector.cpp +++ b/slsDetectorSoftware/src/Detector.cpp @@ -907,16 +907,16 @@ void Detector::setPartialFramesPadding(bool value, Positions pos) { pimpl->Parallel(&Module::setPartialFramesPadding, pos, value); } -Result Detector::getRxUDPSocketBufferSize(Positions pos) const { +Result Detector::getRxUDPSocketBufferSize(Positions pos) const { return pimpl->Parallel(&Module::getReceiverUDPSocketBufferSize, pos); } -void Detector::setRxUDPSocketBufferSize(int64_t udpsockbufsize, Positions pos) { +void Detector::setRxUDPSocketBufferSize(int udpsockbufsize, Positions pos) { pimpl->Parallel(&Module::setReceiverUDPSocketBufferSize, pos, udpsockbufsize); } -Result Detector::getRxRealUDPSocketBufferSize(Positions pos) const { +Result Detector::getRxRealUDPSocketBufferSize(Positions pos) const { return pimpl->Parallel(&Module::getReceiverRealUDPSocketBufferSize, pos); } diff --git a/slsDetectorSoftware/src/Module.cpp b/slsDetectorSoftware/src/Module.cpp index 9d54457cd..b952546dd 100644 --- a/slsDetectorSoftware/src/Module.cpp +++ b/slsDetectorSoftware/src/Module.cpp @@ -844,17 +844,17 @@ void Module::setPartialFramesPadding(bool padding) { sendToReceiver(F_SET_RECEIVER_PADDING, static_cast(padding), nullptr); } -int64_t Module::getReceiverUDPSocketBufferSize() const { - int64_t arg = GET_FLAG; - return sendToReceiver(F_RECEIVER_UDP_SOCK_BUF_SIZE, arg); +int Module::getReceiverUDPSocketBufferSize() const { + int arg = GET_FLAG; + return sendToReceiver(F_RECEIVER_UDP_SOCK_BUF_SIZE, arg); } -int64_t Module::getReceiverRealUDPSocketBufferSize() const { - return sendToReceiver(F_RECEIVER_REAL_UDP_SOCK_BUF_SIZE); +int Module::getReceiverRealUDPSocketBufferSize() const { + return sendToReceiver(F_RECEIVER_REAL_UDP_SOCK_BUF_SIZE); } -void Module::setReceiverUDPSocketBufferSize(int64_t udpsockbufsize) { - sendToReceiver(F_RECEIVER_UDP_SOCK_BUF_SIZE, udpsockbufsize); +void Module::setReceiverUDPSocketBufferSize(int udpsockbufsize) { + sendToReceiver(F_RECEIVER_UDP_SOCK_BUF_SIZE, udpsockbufsize); } bool Module::getReceiverLock() const { diff --git a/slsDetectorSoftware/src/Module.h b/slsDetectorSoftware/src/Module.h index 4ce63ec1a..c045f2b45 100644 --- a/slsDetectorSoftware/src/Module.h +++ b/slsDetectorSoftware/src/Module.h @@ -236,9 +236,9 @@ class Module : public virtual slsDetectorDefs { void setReceiverFramesDiscardPolicy(frameDiscardPolicy f); bool getPartialFramesPadding() const; void setPartialFramesPadding(bool padding); - int64_t getReceiverUDPSocketBufferSize() const; - int64_t getReceiverRealUDPSocketBufferSize() const; - void setReceiverUDPSocketBufferSize(int64_t udpsockbufsize); + int getReceiverUDPSocketBufferSize() const; + int getReceiverRealUDPSocketBufferSize() const; + void setReceiverUDPSocketBufferSize(int udpsockbufsize); bool getReceiverLock() const; void setReceiverLock(bool lock); sls::IpAddr getReceiverLastClientIP() const; diff --git a/slsReceiverSoftware/src/ClientInterface.cpp b/slsReceiverSoftware/src/ClientInterface.cpp index 09ee538bc..756dd3968 100644 --- a/slsReceiverSoftware/src/ClientInterface.cpp +++ b/slsReceiverSoftware/src/ClientInterface.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -1138,15 +1139,22 @@ int ClientInterface::get_additional_json_header(Interface &socket) { } int ClientInterface::set_udp_socket_buffer_size(Interface &socket) { - auto index = socket.Receive(); - if (index >= 0) { - verifyIdle(socket); - LOG(logDEBUG1) << "Setting UDP Socket Buffer size: " << index; - impl()->setUDPSocketBufferSize(index); + auto size = socket.Receive(); + if (size == 0) { + throw RuntimeError("Receiver socket buffer size must be > 0."); } - int64_t retval = impl()->getUDPSocketBufferSize(); - if (index != 0) - validate(index, retval, + if (size > 0) { + verifyIdle(socket); + if (size > INT_MAX / 2) { + throw RuntimeError( + "Receiver socket buffer size exceeded max (INT_MAX/2)"); + } + LOG(logDEBUG1) << "Setting UDP Socket Buffer size: " << size; + impl()->setUDPSocketBufferSize(size); + } + int retval = impl()->getUDPSocketBufferSize(); + if (size != 0) + validate(size, retval, "set udp socket buffer size (No CAP_NET_ADMIN privileges?)", DEC); LOG(logDEBUG1) << "UDP Socket Buffer Size:" << retval; diff --git a/slsReceiverSoftware/src/Implementation.cpp b/slsReceiverSoftware/src/Implementation.cpp index 8b4fa65b3..0a9b672c2 100644 --- a/slsReceiverSoftware/src/Implementation.cpp +++ b/slsReceiverSoftware/src/Implementation.cpp @@ -946,12 +946,14 @@ void Implementation::setUDPPortNumber2(const uint32_t i) { LOG(logINFO) << "UDP Port Number[1]: " << udpPortNum[1]; } -int64_t Implementation::getUDPSocketBufferSize() const { +int Implementation::getUDPSocketBufferSize() const { return udpSocketBufferSize; } -void Implementation::setUDPSocketBufferSize(const int64_t s) { - int64_t size = (s == 0) ? udpSocketBufferSize : s; +void Implementation::setUDPSocketBufferSize(const int s) { + // custom setup is not 0 (must complain if set up didnt work) + // testing default setup at startup, argument is 0 to use default values + int size = (s == 0) ? udpSocketBufferSize : s; size_t listSize = listener.size(); if (myDetectorType == JUNGFRAU && (int)listSize != numUDPInterfaces) { throw sls::RuntimeError( @@ -959,12 +961,17 @@ void Implementation::setUDPSocketBufferSize(const int64_t s) { " do not match listener size " + std::to_string(listSize)); } - for (unsigned int i = 0; i < listSize; ++i) { - listener[i]->CreateDummySocketForUDPSocketBufferSize(size); + for (auto &l : listener) { + l->CreateDummySocketForUDPSocketBufferSize(size); + } + // custom and didnt set, throw error + if (s != 0 && udpSocketBufferSize != s) { + throw sls::RuntimeError("Could not set udp socket buffer size. (No " + "CAP_NET_ADMIN privileges?)"); } } -int64_t Implementation::getActualUDPSocketBufferSize() const { +int Implementation::getActualUDPSocketBufferSize() const { return actualUDPSocketBufferSize; } diff --git a/slsReceiverSoftware/src/Implementation.h b/slsReceiverSoftware/src/Implementation.h index a15d24695..5563afa1e 100644 --- a/slsReceiverSoftware/src/Implementation.h +++ b/slsReceiverSoftware/src/Implementation.h @@ -108,9 +108,9 @@ class Implementation : private virtual slsDetectorDefs { uint32_t getUDPPortNumber2() const; /* [Eiger][Jungfrau] */ void setUDPPortNumber2(const uint32_t i); - int64_t getUDPSocketBufferSize() const; - void setUDPSocketBufferSize(const int64_t s); - int64_t getActualUDPSocketBufferSize() const; + int getUDPSocketBufferSize() const; + void setUDPSocketBufferSize(const int s); + int getActualUDPSocketBufferSize() const; /************************************************** * * @@ -302,8 +302,8 @@ class Implementation : private virtual slsDetectorDefs { std::array eth; std::array udpPortNum{ {DEFAULT_UDP_PORTNO, DEFAULT_UDP_PORTNO + 1}}; - int64_t udpSocketBufferSize{0}; - int64_t actualUDPSocketBufferSize{0}; + int udpSocketBufferSize{0}; + int actualUDPSocketBufferSize{0}; // zmq parameters bool dataStreamEnable{false}; diff --git a/slsReceiverSoftware/src/Listener.cpp b/slsReceiverSoftware/src/Listener.cpp index b8d32edf5..82d38a85f 100644 --- a/slsReceiverSoftware/src/Listener.cpp +++ b/slsReceiverSoftware/src/Listener.cpp @@ -21,14 +21,12 @@ const std::string Listener::TypeName = "Listener"; Listener::Listener(int ind, detectorType dtype, Fifo *f, std::atomic *s, uint32_t *portno, std::string *e, - uint64_t *nf, int64_t *us, int64_t *as, - uint32_t *fpf, frameDiscardPolicy *fdp, bool *act, - bool *depaden, bool *sm) + uint64_t *nf, int *us, int *as, uint32_t *fpf, + frameDiscardPolicy *fdp, bool *act, bool *depaden, bool *sm) : ThreadObject(ind, TypeName), fifo(f), myDetectorType(dtype), status(s), - udpPortNumber(portno), eth(e), numImages(nf), - udpSocketBufferSize(us), actualUDPSocketBufferSize(as), - framesPerFile(fpf), frameDiscardMode(fdp), activated(act), - deactivatedPaddingEnable(depaden), silentMode(sm) { + udpPortNumber(portno), eth(e), numImages(nf), udpSocketBufferSize(us), + actualUDPSocketBufferSize(as), framesPerFile(fpf), frameDiscardMode(fdp), + activated(act), deactivatedPaddingEnable(depaden), silentMode(sm) { LOG(logDEBUG) << "Listener " << ind << " created"; } @@ -142,7 +140,7 @@ void Listener::ShutDownUDPSocket() { } } -void Listener::CreateDummySocketForUDPSocketBufferSize(int64_t s) { +void Listener::CreateDummySocketForUDPSocketBufferSize(int s) { LOG(logINFO) << "Testing UDP Socket Buffer size " << s << " with test port " << *udpPortNumber; @@ -151,7 +149,7 @@ void Listener::CreateDummySocketForUDPSocketBufferSize(int64_t s) { return; } - int64_t temp = *udpSocketBufferSize; + int temp = *udpSocketBufferSize; *udpSocketBufferSize = s; // if eth is mistaken with ip address diff --git a/slsReceiverSoftware/src/Listener.h b/slsReceiverSoftware/src/Listener.h index 80d72d1f3..f1dbbbe69 100644 --- a/slsReceiverSoftware/src/Listener.h +++ b/slsReceiverSoftware/src/Listener.h @@ -41,9 +41,9 @@ class Listener : private virtual slsDetectorDefs, public ThreadObject { * @param sm pointer to silent mode */ Listener(int ind, detectorType dtype, Fifo *f, std::atomic *s, - uint32_t *portno, std::string *e, uint64_t *nf, - int64_t *us, int64_t *as, uint32_t *fpf, frameDiscardPolicy *fdp, - bool *act, bool *depaden, bool *sm); + uint32_t *portno, std::string *e, uint64_t *nf, int *us, int *as, + uint32_t *fpf, frameDiscardPolicy *fdp, bool *act, bool *depaden, + bool *sm); /** * Destructor @@ -98,7 +98,7 @@ class Listener : private virtual slsDetectorDefs, public ThreadObject { * to set & get actual buffer size * @param s UDP socket buffer size to be set */ - void CreateDummySocketForUDPSocketBufferSize(int64_t s); + void CreateDummySocketForUDPSocketBufferSize(int s); /** * Set hard coded (calculated but not from detector) row and column @@ -173,10 +173,10 @@ class Listener : private virtual slsDetectorDefs, public ThreadObject { uint64_t *numImages; /** UDP Socket Buffer Size */ - int64_t *udpSocketBufferSize; + int *udpSocketBufferSize; /** actual UDP Socket Buffer Size (double due to kernel bookkeeping) */ - int64_t *actualUDPSocketBufferSize; + int *actualUDPSocketBufferSize; /** frames per file */ uint32_t *framesPerFile; diff --git a/slsSupportLib/include/UdpRxSocket.h b/slsSupportLib/include/UdpRxSocket.h index 8f3c451dd..55acf51e9 100644 --- a/slsSupportLib/include/UdpRxSocket.h +++ b/slsSupportLib/include/UdpRxSocket.h @@ -14,11 +14,11 @@ class UdpRxSocket { public: UdpRxSocket(int port, ssize_t packet_size, const char *hostname = nullptr, - size_t kernel_buffer_size = 0); + int kernel_buffer_size = 0); ~UdpRxSocket(); bool ReceivePacket(char *dst) noexcept; - size_t getBufferSize() const; - void setBufferSize(ssize_t size); + int getBufferSize() const; + void setBufferSize(int size); ssize_t getPacketSize() const noexcept; void Shutdown(); diff --git a/slsSupportLib/src/UdpRxSocket.cpp b/slsSupportLib/src/UdpRxSocket.cpp index 280a370e9..aadb3efac 100644 --- a/slsSupportLib/src/UdpRxSocket.cpp +++ b/slsSupportLib/src/UdpRxSocket.cpp @@ -14,7 +14,7 @@ namespace sls { UdpRxSocket::UdpRxSocket(int port, ssize_t packet_size, const char *hostname, - size_t kernel_buffer_size) + int kernel_buffer_size) : packet_size_(packet_size) { struct addrinfo hints; memset(&hints, 0, sizeof(hints)); @@ -73,15 +73,15 @@ ssize_t UdpRxSocket::ReceiveDataOnly(char *dst) noexcept { return r; } -size_t UdpRxSocket::getBufferSize() const { - size_t ret = 0; +int UdpRxSocket::getBufferSize() const { + int ret = 0; socklen_t optlen = sizeof(ret); if (getsockopt(sockfd_, SOL_SOCKET, SO_RCVBUF, &ret, &optlen) == -1) throw RuntimeError("Could not get socket buffer size"); return ret; } -void UdpRxSocket::setBufferSize(ssize_t size) { +void UdpRxSocket::setBufferSize(int size) { if (setsockopt(sockfd_, SOL_SOCKET, SO_RCVBUF, &size, sizeof(size))) throw RuntimeError("Could not set socket buffer size"); }