From 06d34efa749dab1e3b76b47a0fa28d7f52cfd0fb Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 12 Apr 2021 12:33:45 +0200 Subject: [PATCH 1/2] UDP and TCPIP smaller tweaks Using AI_PASSIVE now so the UDP server listens to all addresses (0.0.0.0) instead of just localhost. Also implemented address print function properly --- osal/common/TcpTmTcServer.cpp | 7 ++++-- osal/common/UdpTcPollingTask.cpp | 9 +++++--- osal/common/UdpTmTcBridge.cpp | 22 ++++++++++-------- osal/common/tcpipCommon.cpp | 39 ++++++++++++++++++++++++++++++++ osal/common/tcpipCommon.h | 11 +++++++-- 5 files changed, 71 insertions(+), 17 deletions(-) diff --git a/osal/common/TcpTmTcServer.cpp b/osal/common/TcpTmTcServer.cpp index 296afad8..08a62ffb 100644 --- a/osal/common/TcpTmTcServer.cpp +++ b/osal/common/TcpTmTcServer.cpp @@ -70,6 +70,7 @@ ReturnValue_t TcpTmTcServer::initialize() { #endif freeaddrinfo(addrResult); handleError(Protocol::TCP, ErrorSources::BIND_CALL); + return HasReturnvaluesIF::RETURN_FAILED; } freeaddrinfo(addrResult); @@ -84,8 +85,8 @@ TcpTmTcServer::~TcpTmTcServer() { ReturnValue_t TcpTmTcServer::performOperation(uint8_t opCode) { using namespace tcpip; /* If a connection is accepted, the corresponding socket will be assigned to the new socket */ - socket_t clientSocket; - sockaddr clientSockAddr; + socket_t clientSocket = 0; + sockaddr clientSockAddr = {}; socklen_t connectorSockAddrLen = 0; int retval = 0; @@ -101,6 +102,7 @@ ReturnValue_t TcpTmTcServer::performOperation(uint8_t opCode) { if(clientSocket == INVALID_SOCKET) { handleError(Protocol::TCP, ErrorSources::ACCEPT_CALL, 500); + closeSocket(clientSocket); continue; }; @@ -122,6 +124,7 @@ ReturnValue_t TcpTmTcServer::performOperation(uint8_t opCode) { /* Done, shut down connection */ retval = shutdown(clientSocket, SHUT_SEND); + closeSocket(clientSocket); } return HasReturnvaluesIF::RETURN_OK; } diff --git a/osal/common/UdpTcPollingTask.cpp b/osal/common/UdpTcPollingTask.cpp index 759cee05..47f67b29 100644 --- a/osal/common/UdpTcPollingTask.cpp +++ b/osal/common/UdpTcPollingTask.cpp @@ -15,7 +15,7 @@ #endif //! Debugging preprocessor define. -#define FSFW_UDP_RCV_WIRETAPPING_ENABLED 0 +#define FSFW_UDP_RECV_WIRETAPPING_ENABLED 0 UdpTcPollingTask::UdpTcPollingTask(object_id_t objectId, object_id_t tmtcUnixUdpBridge, size_t maxRecvSize, @@ -66,10 +66,13 @@ ReturnValue_t UdpTcPollingTask::performOperation(uint8_t opCode) { tcpip::handleError(tcpip::Protocol::UDP, tcpip::ErrorSources::RECVFROM_CALL, 1000); continue; } -#if FSFW_CPP_OSTREAM_ENABLED == 1 && FSFW_UDP_RCV_WIRETAPPING_ENABLED == 1 +#if FSFW_UDP_RECV_WIRETAPPING_ENABLED == 1 +#if FSFW_CPP_OSTREAM_ENABLED == 1 sif::debug << "UdpTcPollingTask::performOperation: " << bytesReceived << " bytes received" << std::endl; +#else #endif +#endif /* FSFW_UDP_RCV_WIRETAPPING_ENABLED == 1 */ ReturnValue_t result = handleSuccessfullTcRead(bytesReceived); if(result != HasReturnvaluesIF::RETURN_FAILED) { @@ -84,7 +87,7 @@ ReturnValue_t UdpTcPollingTask::performOperation(uint8_t opCode) { ReturnValue_t UdpTcPollingTask::handleSuccessfullTcRead(size_t bytesRead) { store_address_t storeId; -#if FSFW_UDP_RCV_WIRETAPPING_ENABLED == 1 +#if FSFW_UDP_RECV_WIRETAPPING_ENABLED == 1 arrayprinter::print(receptionBuffer.data(), bytesRead); #endif diff --git a/osal/common/UdpTmTcBridge.cpp b/osal/common/UdpTmTcBridge.cpp index 7f3dc929..ba23f521 100644 --- a/osal/common/UdpTmTcBridge.cpp +++ b/osal/common/UdpTmTcBridge.cpp @@ -70,6 +70,7 @@ ReturnValue_t UdpTmTcBridge::initialize() { hints.ai_family = AF_INET; hints.ai_socktype = SOCK_DGRAM; hints.ai_protocol = IPPROTO_UDP; + hints.ai_flags = AI_PASSIVE; /* Set up UDP socket: https://en.wikipedia.org/wiki/Getaddrinfo @@ -95,6 +96,10 @@ ReturnValue_t UdpTmTcBridge::initialize() { return HasReturnvaluesIF::RETURN_FAILED; } +#if FSFW_UDP_SEND_WIRETAPPING_ENABLED == 1 + tcpip::printAddress(addrResult->ai_addr); +#endif + retval = bind(serverSocket, addrResult->ai_addr, static_cast(addrResult->ai_addrlen)); if(retval != 0) { #if FSFW_CPP_OSTREAM_ENABLED == 1 @@ -103,6 +108,7 @@ ReturnValue_t UdpTmTcBridge::initialize() { #endif freeaddrinfo(addrResult); tcpip::handleError(tcpip::Protocol::UDP, tcpip::ErrorSources::BIND_CALL); + return HasReturnvaluesIF::RETURN_FAILED; } freeaddrinfo(addrResult); return HasReturnvaluesIF::RETURN_OK; @@ -120,10 +126,8 @@ ReturnValue_t UdpTmTcBridge::sendTm(const uint8_t *data, size_t dataLen) { /* The target address can be set by different threads so this lock ensures thread-safety */ MutexGuard lock(mutex, timeoutType, mutexTimeoutMs); -#if FSFW_CPP_OSTREAM_ENABLED == 1 && FSFW_UDP_SEND_WIRETAPPING_ENABLED == 1 - char ipAddress [15]; - sif::debug << "IP Address Sender: "<< inet_ntop(AF_INET, - &clientAddress.sin_addr.s_addr, ipAddress, 15) << std::endl; +#if FSFW_UDP_SEND_WIRETAPPING_ENABLED == 1 + tcpip::printAddress(&clientAddress); #endif int bytesSent = sendto( @@ -151,13 +155,11 @@ void UdpTmTcBridge::checkAndSetClientAddress(sockaddr& newAddress) { /* The target address can be set by different threads so this lock ensures thread-safety */ MutexGuard lock(mutex, timeoutType, mutexTimeoutMs); -#if FSFW_CPP_OSTREAM_ENABLED == 1 && FSFW_UDP_SEND_WIRETAPPING_ENABLED == 1 - char ipAddress [15]; - sif::debug << "IP Address Sender: "<< inet_ntop(AF_INET, - &newAddress.sin_addr.s_addr, ipAddress, 15) << std::endl; - sif::debug << "IP Address Old: " << inet_ntop(AF_INET, - &clientAddress.sin_addr.s_addr, ipAddress, 15) << std::endl; +#if FSFW_UDP_SEND_WIRETAPPING_ENABLED == 1 + tcpip::printAddress(&newAddress); + tcpip::printAddress(&clientAddress); #endif + registerCommConnect(); /* Set new IP address to reply to */ diff --git a/osal/common/tcpipCommon.cpp b/osal/common/tcpipCommon.cpp index 9a5e4647..551e2a42 100644 --- a/osal/common/tcpipCommon.cpp +++ b/osal/common/tcpipCommon.cpp @@ -1,4 +1,9 @@ #include "tcpipCommon.h" +#include + +#ifdef _WIN32 +#include +#endif void tcpip::determineErrorStrings(Protocol protocol, ErrorSources errorSrc, std::string &protStr, std::string &srcString) { @@ -34,3 +39,37 @@ void tcpip::determineErrorStrings(Protocol protocol, ErrorSources errorSrc, std: srcString = "unknown call"; } } + +void tcpip::printAddress(struct sockaddr* addr) { + char ipAddress[INET6_ADDRSTRLEN] = {}; + const char* stringPtr = NULL; + switch(addr->sa_family) { + case AF_INET: { + struct sockaddr_in *addrIn = reinterpret_cast(addr); + stringPtr = inet_ntop(AF_INET, &(addrIn->sin_addr), ipAddress, INET_ADDRSTRLEN); + break; + } + case AF_INET6: { + struct sockaddr_in6 *addrIn = reinterpret_cast(addr); + stringPtr = inet_ntop(AF_INET6, &(addrIn->sin6_addr), ipAddress, INET6_ADDRSTRLEN); + break; + } + } +#if FSFW_CPP_OSTREAM_ENABLED == 1 + if(stringPtr == NULL) { + sif::debug << "Could not convert IP address to text representation, error code " + << errno << std::endl; + } + else { + sif::debug << "IP Address Sender: " << ipAddress << std::endl; + } +#else + if(stringPtr == NULL) { + sif::printDebug("Could not convert IP address to text representation, error code %d\n", + errno); + } + else { + sif::printDebug("IP Address Sender: %s\n", ipAddress); + } +#endif +} diff --git a/osal/common/tcpipCommon.h b/osal/common/tcpipCommon.h index dc5ada52..22b914dc 100644 --- a/osal/common/tcpipCommon.h +++ b/osal/common/tcpipCommon.h @@ -4,6 +4,13 @@ #include "../../timemanager/clockDefinitions.h" #include +#ifdef _WIN32 +#include +#else +#include +#include +#endif + namespace tcpip { const char* const DEFAULT_SERVER_PORT = "7301"; @@ -28,8 +35,8 @@ enum class ErrorSources { void determineErrorStrings(Protocol protocol, ErrorSources errorSrc, std::string& protStr, std::string& srcString); +void printAddress(struct sockaddr* addr); + } - - #endif /* FSFW_OSAL_COMMON_TCPIPCOMMON_H_ */ From 547538fbc5aa9b23d1128f01c100ad5a73b9a520 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 12 Apr 2021 12:38:56 +0200 Subject: [PATCH 2/2] proper MQ implementation --- osal/host/MessageQueue.cpp | 29 ++++++++++------------------- osal/host/MessageQueue.h | 2 +- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/osal/host/MessageQueue.cpp b/osal/host/MessageQueue.cpp index 18272a68..41c55a3d 100644 --- a/osal/host/MessageQueue.cpp +++ b/osal/host/MessageQueue.cpp @@ -64,9 +64,8 @@ ReturnValue_t MessageQueue::receiveMessage(MessageQueueMessageIF* message) { return MessageQueueIF::EMPTY; } MutexGuard mutexLock(queueLock, MutexIF::TimeoutType::WAITING, 20); - MessageQueueMessage* currentMessage = &messageQueue.front(); - std::copy(currentMessage->getBuffer(), - currentMessage->getBuffer() + messageSize, message->getBuffer()); + std::copy(messageQueue.front().data(), messageQueue.front().data() + messageSize, + message->getBuffer()); messageQueue.pop(); // The last partner is the first uint32_t field in the message this->lastPartner = message->getSender(); @@ -80,7 +79,7 @@ MessageQueueId_t MessageQueue::getLastPartner() const { ReturnValue_t MessageQueue::flush(uint32_t* count) { *count = messageQueue.size(); // Clears the queue. - messageQueue = std::queue(); + messageQueue = std::queue>(); return HasReturnvaluesIF::RETURN_OK; } @@ -106,6 +105,9 @@ bool MessageQueue::isDefaultDestinationSet() const { ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, MessageQueueMessageIF* message, MessageQueueId_t sentFrom, bool ignoreFault) { + if(message == nullptr) { + return HasReturnvaluesIF::RETURN_FAILED; + } message->setSender(sentFrom); if(message->getMessageSize() > message->getMaximumMessageSize()) { // Actually, this should never happen or an error will be emitted @@ -128,21 +130,10 @@ ReturnValue_t MessageQueue::sendMessageFromMessageQueue(MessageQueueId_t sendTo, return HasReturnvaluesIF::RETURN_FAILED; } if(targetQueue->messageQueue.size() < targetQueue->messageDepth) { - MutexGuard mutexLock(targetQueue->queueLock, - MutexIF::TimeoutType::WAITING, 20); - // not ideal, works for now though. - MessageQueueMessage* mqmMessage = - dynamic_cast(message); - if(message != nullptr) { - targetQueue->messageQueue.push(*mqmMessage); - } - else { -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "MessageQueue::sendMessageFromMessageQueue: Message" - "is not MessageQueueMessage!" << std::endl; -#endif - } - + MutexGuard mutexLock(targetQueue->queueLock, MutexIF::TimeoutType::WAITING, 20); + targetQueue->messageQueue.push(std::vector(message->getMaximumMessageSize())); + memcpy(targetQueue->messageQueue.back().data(), message->getBuffer(), + message->getMaximumMessageSize()); } else { return MessageQueueIF::FULL; diff --git a/osal/host/MessageQueue.h b/osal/host/MessageQueue.h index 97a9e491..e965123d 100644 --- a/osal/host/MessageQueue.h +++ b/osal/host/MessageQueue.h @@ -212,7 +212,7 @@ protected: //static ReturnValue_t handleSendResult(BaseType_t result, bool ignoreFault); private: - std::queue messageQueue; + std::queue> messageQueue; /** * @brief The class stores the queue id it got assigned. * If initialization fails, the queue id is set to zero.