Adding support for UDP ephemeral ports (#3691)

* Fixing immplementation of UDP epehmeral support and adding two tests cases to TestUdp

* Updating SDD. Fixing configureSend so it can be passed a port of 0.

* Actually adding SDD this time. Also fixed some incorrect wording in hpp file.

* Removing unnecessary period

* Removing redundant test. All the functionality can be tested in the same test.

* Removing redundant code path.

* Incorporating some fixes for static analysis findings

* Revert "Incorporating some fixes for static analysis findings"

This reverts commit 644fbbe4e9fb797c765a2e13ef8c4f057e838edf.

* Incorporating some fixes to static analyzer findings.

* Fixing typo

* Changing use of char to CHAR

* Adding documentation for ephemeral ports to Drv/Ip SDD. Fixing configureSend so that it maintains the configured timeout_seconds and timeout_microseconds.

* Adding netinet to spelling expect list.

* Changing the use of socklen_t to U32

* Incorporating PR suggestion

* Reverting back to using socklen_t for address length

.github/actions/spelling/expect.txt

@@ -625,6 +625,7 @@ nbits
 ncsl
 ndiffs
 neascout
+netinet
 newloc
 newroot
 newself

Drv/Ip/UdpSocket.cpp

@@ -50,7 +50,7 @@ struct SocketState {
     }
 };
 
-UdpSocket::UdpSocket() : IpSocket(), m_state(new(std::nothrow) SocketState), m_recv_port(0) {
+UdpSocket::UdpSocket() : IpSocket(), m_state(new(std::nothrow) SocketState), m_recv_port(0), m_recv_configured(false) {
     FW_ASSERT(m_state != nullptr);
 }
 
@@ -67,7 +67,6 @@ SocketIpStatus UdpSocket::configure(const char* const hostname, const U16 port,
 
 SocketIpStatus UdpSocket::configureSend(const char* const hostname, const U16 port, const U32 timeout_seconds, const U32 timeout_microseconds) {
     //Timeout is for the send, so configure send will work with the base class
-    FW_ASSERT(port != 0, static_cast<FwAssertArgType>(port)); // Send cannot be on port 0
     FW_ASSERT(hostname != nullptr);
     return this->IpSocket::configure(hostname, port, timeout_seconds, timeout_microseconds);
 }
@@ -77,6 +76,7 @@ SocketIpStatus UdpSocket::configureRecv(const char* hostname, const U16 port) {
     FW_ASSERT(hostname != nullptr);
     this->m_recv_port = port;
     (void) Fw::StringUtils::string_copy(this->m_recv_hostname, hostname, static_cast<FwSizeType>(SOCKET_MAX_HOSTNAME_SIZE));
+    this->m_recv_configured = true;
     return SOCK_SUCCESS;
 }
 
@@ -112,6 +112,9 @@ SocketIpStatus UdpSocket::bind(const PlatformIntType fd) {
         return SOCK_FAILED_TO_READ_BACK_PORT;
     }
 
+    // Update m_recv_port with the actual port assigned (for ephemeral port support)
+    this->m_recv_port = ntohs(address.sin_port);
+
     FW_ASSERT(sizeof(this->m_state->m_addr_recv) == sizeof(address), static_cast<FwAssertArgType>(sizeof(this->m_state->m_addr_recv)), static_cast<FwAssertArgType>(sizeof(address)));
     memcpy(&this->m_state->m_addr_recv, &address, sizeof(this->m_state->m_addr_recv));
 
@@ -124,6 +127,7 @@ SocketIpStatus UdpSocket::openProtocol(SocketDescriptor& socketDescriptor) {
     struct sockaddr_in address;
 
     U16 port = this->m_port;
+    U16 recv_port = this->m_recv_port;
 
     // Acquire a socket, or return error
     if ((socketFd = ::socket(AF_INET, SOCK_DGRAM, 0)) == -1) {
@@ -157,11 +161,10 @@ SocketIpStatus UdpSocket::openProtocol(SocketDescriptor& socketDescriptor) {
         memcpy(&this->m_state->m_addr_send, &address, sizeof(this->m_state->m_addr_send));
     }
 
-    // Receive port set up only done when configure receive was called
-    U16 recv_port = this->m_recv_port;
-    if (recv_port != 0) {
+    // Only bind if configureRecv was called (including ephemeral)
+    if (this->m_recv_configured) {
         status = this->bind(socketFd);
-        // When we are setting up for receiving as well, then we must bind to a port
+
         if (status != SOCK_SUCCESS) {
             (void) ::close(socketFd); // Closing FD as a retry will reopen send side
             return status;
@@ -170,22 +173,13 @@ SocketIpStatus UdpSocket::openProtocol(SocketDescriptor& socketDescriptor) {
 
     // Log message for UDP
     if ((port == 0) && (recv_port > 0)) {
-        Fw::Logger::log("Setup to only receive udp at %s:%hu\n", m_recv_hostname,
-                           recv_port);
-    } else if ((port > 0) && (recv_port == 0))  {
-        Fw::Logger::log("Setup to only send udp at %s:%hu\n", m_hostname,
-                           port);
-    } else if ((port > 0) && (recv_port > 0))  {
-        Fw::Logger::log("Setup to receive udp at %s:%hu and send to %s:%hu\n",
-                           m_recv_hostname,
-                           recv_port,
-                           m_hostname,
-                           port);
-    }
-    // Neither configuration method was called
-    else {
-        FW_ASSERT(port > 0 || recv_port > 0, static_cast<FwAssertArgType>(port), static_cast<FwAssertArgType>(recv_port));
+        Fw::Logger::log("Setup to only receive udp at %s:%hu\n", m_recv_hostname, recv_port);
+    } else if ((port > 0) && (recv_port == 0)) {
+        Fw::Logger::log("Setup to only send udp at %s:%hu\n", m_hostname, port);
+    } else if ((port > 0) && (recv_port > 0)) {
+        Fw::Logger::log("Setup to receive udp at %s:%hu and send to %s:%hu\n", m_recv_hostname, recv_port, m_hostname, port);
     }
+    
     FW_ASSERT(status == SOCK_SUCCESS, static_cast<FwAssertArgType>(status));
     socketDescriptor.fd = socketFd;
     return status;
@@ -199,7 +193,18 @@ I32 UdpSocket::sendProtocol(const SocketDescriptor& socketDescriptor, const U8*
 
 I32 UdpSocket::recvProtocol(const SocketDescriptor& socketDescriptor, U8* const data, const U32 size) {
     FW_ASSERT(this->m_state->m_addr_recv.sin_family != 0); // Make sure the address was previously setup
-    return static_cast<I32>(::recvfrom(socketDescriptor.fd, data, size, SOCKET_IP_RECV_FLAGS, nullptr, nullptr));
+
+    struct sockaddr_in sender_addr;
+    socklen_t sender_addr_len = sizeof(sender_addr);
+    I32 received = static_cast<I32>(::recvfrom(socketDescriptor.fd, data, size, SOCKET_IP_RECV_FLAGS,
+                                              reinterpret_cast<struct sockaddr*>(&sender_addr), &sender_addr_len));
+    // If we have not configured a send port, set it to the source of the last received packet
+    if (received > 0 && this->m_state->m_addr_send.sin_port == 0) {
+        this->m_state->m_addr_send = sender_addr;
+        this->m_port = ntohs(sender_addr.sin_port);
+        Fw::Logger::log("Configured send port to %hu as specified by the last received packet.\n", this->m_port);
+    }
+    return received;
 }
 
 }  // namespace Drv

Drv/Ip/UdpSocket.hpp

@@ -51,13 +51,14 @@ class UdpSocket : public IpSocket {
      * Configures the UDP handler to use the given hostname and port for outgoing transmissions. Incoming hostname
      * and port are configured using the `configureRecv` function call for UDP as it requires separate host/port pairs
      * for outgoing and incoming transmissions. Hostname DNS translation is left up to the caller and thus hostname must
-     * be an IP address in dot-notation of the form "x.x.x.x". Port cannot be set to 0 as dynamic port assignment is not
-     * supported on remote ports.  It is possible to configure the UDP port as a single-direction send port only.
+     * be an IP address in dot-notation of the form "x.x.x.x". If port is set to 0, the socket will be configured for
+     * ephemeral send (dynamic reply-to) and will use the sender's address from the first received datagram for replies.
+     * It is possible to configure the UDP port as a single-direction send port only.
      *
      * Note: delegates to `IpSocket::configure`
      *
      * \param hostname: socket uses for outgoing transmissions. Must be of form x.x.x.x
-     * \param port: port socket uses for outgoing transmissions. Must NOT be 0.
+     * \param port: port socket uses for outgoing transmissions. Can be 0 for ephemeral reply-to mode.
      * \param send_timeout_seconds: send timeout seconds portion
      * \param send_timeout_microseconds: send timeout microseconds portion. Must be less than 1000000
      * \return status of configure
@@ -75,7 +76,7 @@ class UdpSocket : public IpSocket {
      * single-direction receive port only.
      *
      * \param hostname: socket uses for incoming transmissions. Must be of form x.x.x.x
-     * \param port: port socket uses for incoming transmissions.
+     * \param port: port socket uses for incoming transmissions. Can be 0 for ephemeral port assignment.
      * \return status of configure
      */
     SocketIpStatus configureRecv(const char* hostname, const U16 port);
@@ -122,8 +123,9 @@ class UdpSocket : public IpSocket {
     I32 recvProtocol(const SocketDescriptor& socketDescriptor, U8* const data, const U32 size) override;
   private:
     SocketState* m_state; //!< State storage
-    U16 m_recv_port;  //!< IP address port used
-    char m_recv_hostname[SOCKET_MAX_HOSTNAME_SIZE];  //!< Hostname to supply
+    U16 m_recv_port;  //!< Port to receive on
+    CHAR m_recv_hostname[SOCKET_MAX_HOSTNAME_SIZE]; //!< Hostname to receive on
+    bool m_recv_configured; //!< True if configureRecv was called
 };
 }  // namespace Drv
 

Drv/Ip/docs/sdd.md

@@ -152,6 +152,17 @@ socketBoth.configureSend(127.0.0.1, 60211, 0, 100);
 socketBoth.configureRecv(127.0.0.1, 60212);
 ...
 ```
+### Support for Ephemeral Ports
+
+Drv::UdpSocket supports ephemeral ports through passing a 0 as the port argument for either `Drv::UdpSocket::configureSend` 
+or `Drv::UdpSocket::configureRecv`.
+
+For `Drv::UdpSocket::configureSend` this means that you would like to set up the UdpSocket to be able to respond to the source 
+port that is indicated in the UDP datagrams you receive. Note that this configuration will set up the send port to the port 
+specified only in the first message received.
+
+For `Drv::UdpSocket::configureRecv` this means that you would like to be assigned an ephemeral port. This would generally be used
+for setting up a sender that would like to receive responses to messages on an ephemeral port.
 
 ## Drv::SocketComponentHelper Virtual Baseclass
 

Drv/Ip/test/ut/TestUdp.cpp

@@ -2,6 +2,11 @@
 // Created by mstarch on 12/7/20.
 //
 #include <gtest/gtest.h>
+#include <cstring>
+#include <string>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
 #include <Drv/Ip/UdpSocket.hpp>
 #include <Drv/Ip/IpSocket.hpp>
 #include <Os/Console.hpp>
@@ -99,6 +104,48 @@ TEST(SingleSide, TestSingleSideMultipleSendUdp) {
     test_with_loop(100, SEND);
 }
 
+TEST(Ephemeral, TestEphemeralPorts) {
+    Drv::UdpSocket receiver;
+    Drv::SocketDescriptor recv_fd;
+    const U16 recv_port = 50001;
+    // Configure receiver as receiver-only with no send port.
+    receiver.configureRecv("127.0.0.1", recv_port);
+    receiver.configureSend("127.0.0.1", 0, 0, 100);
+    ASSERT_EQ(receiver.open(recv_fd), Drv::SOCK_SUCCESS);
+
+    Drv::UdpSocket sender;
+    Drv::SocketDescriptor send_fd;
+    // Configure sender for both send and receive (duplex) with ephemeral receive port
+    sender.configureSend("127.0.0.1", recv_port, 0, 100);
+    sender.configureRecv("127.0.0.1", 0);
+    ASSERT_EQ(sender.open(send_fd), Drv::SOCK_SUCCESS);
+
+    // Send a test message
+    const char* msg = "hello from ephemeral sender";
+    U32 msg_len = static_cast<U32>(strlen(msg) + 1);
+    ASSERT_EQ(sender.send(send_fd, reinterpret_cast<const U8*>(msg), msg_len), Drv::SOCK_SUCCESS);
+
+    // Receive the message and capture sender's port
+    char recv_buf[64] = {0};
+    U32 recv_buf_len = sizeof(recv_buf);
+    ASSERT_EQ(receiver.recv(recv_fd, reinterpret_cast<U8*>(recv_buf), recv_buf_len), Drv::SOCK_SUCCESS);
+    ASSERT_STREQ(msg, recv_buf);
+
+    // Receiver sends a response back to sender
+    const char* reply = "reply from receiver";
+    U32 reply_len = static_cast<U32>(strlen(reply) + 1);
+    ASSERT_EQ(receiver.send(recv_fd, reinterpret_cast<const U8*>(reply), reply_len), Drv::SOCK_SUCCESS);
+
+    // Sender receives the response
+    char reply_buf[64] = {0};
+    U32 reply_buf_len = sizeof(reply_buf);
+    ASSERT_EQ(sender.recv(send_fd, reinterpret_cast<U8*>(reply_buf), reply_buf_len), Drv::SOCK_SUCCESS);
+    ASSERT_STREQ(reply, reply_buf);
+
+    sender.close(send_fd);
+    receiver.close(recv_fd);
+}
+
 int main(int argc, char** argv) {
     ::testing::InitGoogleTest(&argc, argv);
     return RUN_ALL_TESTS();

Drv/Udp/docs/sdd.md

@@ -27,6 +27,15 @@ wait for the thread to exit using `join`.
 Since UDP support single or bidirectional communication, configuring each direction is done separately using the two
 methods `configureSend` and `configureRecv`. The user must call at least one of the configure methods and may call both.
 
+### Ephemeral Port Support
+
+The Drv::UdpComponentImpl supports ephemeral ports for receiving data. This is done by setting the port number to 0
+when calling `configureRecv`. The port number will be returned when the socket is opened.
+
+When configured as a receiver-only the Drv::UdpComponentImpl can also be set up to send a response back to the sender and use the
+response port that the sender has indicated in the UDP datagram. This is done by setting the port number to 0 when calling
+`configureSend`.
+
 ```c++
 Drv::UdpComponentImpl comm = Drv::UdpComponentImpl("UDP Client");