net: Optimize TCP/UDP port selection

Optimize TCP/UDP port selection, and fix possibly dead loop.

Finish discussion in https://github.com/apache/nuttx/pull/12116#discussion_r1560851977

Note:
Linux also uses EADDRINUSE for failing in finding a portno, according to https://man7.org/linux/man-pages/man2/bind.2.html

Signed-off-by: Zhe Weng <wengzhe@xiaomi.com>

net/nat/nat.c

@@ -32,25 +32,10 @@
 #include "nat/nat.h"
 #include "tcp/tcp.h"
 #include "udp/udp.h"
+#include "utils/utils.h"
 
 #ifdef CONFIG_NET_NAT
 
-/****************************************************************************
- * Pre-processor Definitions
- ****************************************************************************/
-
-#define NEXT_PORT(nport, hport) \
-  do \
-    { \
-      ++(hport); \
-      if ((hport) >= CONFIG_NET_DEFAULT_MAX_PORT || \
-          (hport) < CONFIG_NET_DEFAULT_MIN_PORT) \
-        { \
-          (hport) = CONFIG_NET_DEFAULT_MIN_PORT; \
-        } \
-      (nport) = HTONS(hport); \
-    } while (0)
-
 /****************************************************************************
  * Private Functions
  ****************************************************************************/
@@ -86,7 +71,7 @@ static uint16_t nat_port_select_without_stack(
   uint16_t hport = NTOHS(portno);
   while (nat_port_inuse(domain, protocol, ip, portno))
     {
-      NEXT_PORT(portno, hport);
+      NET_PORT_NEXT_NH(portno, hport);
       if (portno == local_port)
         {
           /* We have looped back, failed. */
@@ -308,7 +293,7 @@ uint16_t nat_port_select(FAR struct net_driver_s *dev,
           while (icmp_findconn(dev, id) ||
                  nat_port_inuse(domain, IP_PROTO_ICMP, external_ip, id))
             {
-              NEXT_PORT(id, hid);
+              NET_PORT_NEXT_NH(id, hid);
               if (id == local_port)
                 {
                   /* We have looped back, failed. */
@@ -334,7 +319,7 @@ uint16_t nat_port_select(FAR struct net_driver_s *dev,
           while (icmpv6_active(id) ||
                  nat_port_inuse(domain, IP_PROTO_ICMP6, external_ip, id))
             {
-              NEXT_PORT(id, hid);
+              NET_PORT_NEXT_NH(id, hid);
               if (id == local_port)
                 {
                   /* We have looped back, failed. */

net/tcp/tcp_conn.c

@@ -583,38 +583,30 @@ int tcp_selectport(uint8_t domain,
 
   if (g_last_tcp_port == 0)
     {
-      net_getrandom(&g_last_tcp_port, sizeof(uint16_t));
+      NET_PORT_RANDOM_INIT(g_last_tcp_port);
-
-      g_last_tcp_port = g_last_tcp_port %
-                        (CONFIG_NET_DEFAULT_MAX_PORT -
-                         CONFIG_NET_DEFAULT_MIN_PORT + 1);
-      g_last_tcp_port += CONFIG_NET_DEFAULT_MIN_PORT;
     }
 
   if (portno == 0)
     {
+      uint16_t loop_start = g_last_tcp_port;
+
       /* No local port assigned. Loop until we find a valid listen port
-       * number that is not being used by any other connection. NOTE the
+       * number that is not being used by any other connection.
-       * following loop is assumed to terminate but could not if all
-       * 32000-4096+1 ports are in used (unlikely).
        */
 
       do
         {
           /* Guess that the next available port number will be the one after
-           * the last port number assigned. Make sure that the port number
+           * the last port number assigned.
-           * is within range.
            */
 
-          ++g_last_tcp_port;
+          NET_PORT_NEXT_NH(portno, g_last_tcp_port);
-
+          if (g_last_tcp_port == loop_start)
-          if (g_last_tcp_port > CONFIG_NET_DEFAULT_MAX_PORT ||
-              g_last_tcp_port < CONFIG_NET_DEFAULT_MIN_PORT)
             {
-              g_last_tcp_port = CONFIG_NET_DEFAULT_MIN_PORT;
+              /* We have looped back, failed. */
-            }
 
-          portno = HTONS(g_last_tcp_port);
+              return -EADDRINUSE;
+            }
         }
       while (tcp_listener(domain, ipaddr, portno)
 #ifdef CONFIG_NET_NAT

net/udp/udp.h

@@ -265,6 +265,9 @@ FAR struct udp_conn_s *udp_nextconn(FAR struct udp_conn_s *conn);
  *   implementation, it is reasonable to assume that that error cannot happen
  *   and that a port number will always be available.
  *
+ * Returned Value:
+ *   Next available port number in host byte order, 0 for failure.
+ *
  ****************************************************************************/
 
 uint16_t udp_select_port(uint8_t domain, FAR union ip_binding_u *u);

net/udp/udp_conn.c

@@ -70,6 +70,7 @@
 #include "socket/socket.h"
 #include "igmp/igmp.h"
 #include "udp/udp.h"
+#include "utils/utils.h"
 
 /****************************************************************************
  * Private Data
@@ -525,7 +526,7 @@ static FAR struct udp_conn_s *udp_alloc_conn(void)
  *   None
  *
  * Returned Value:
- *   Next available port number
+ *   Next available port number in host byte order, 0 for failure.
  *
  ****************************************************************************/
 
@@ -540,30 +541,24 @@ uint16_t udp_select_port(uint8_t domain, FAR union ip_binding_u *u)
 
   if (g_last_udp_port == 0)
     {
-      g_last_udp_port = clock_systime_ticks() %
+      NET_PORT_RANDOM_INIT(g_last_udp_port);
-                        (CONFIG_NET_DEFAULT_MAX_PORT -
-                         CONFIG_NET_DEFAULT_MIN_PORT + 1);
-      g_last_udp_port += CONFIG_NET_DEFAULT_MIN_PORT;
     }
 
   /* Find an unused local port number.  Loop until we find a valid
    * listen port number that is not being used by any other connection.
    */
 
+  portno = g_last_udp_port; /* Record a starting port number */
+
   do
     {
-      /* Guess that the next available port number will be the one after
+      NET_PORT_NEXT_H(g_last_udp_port);
-       * the last port number assigned.
+      if (g_last_udp_port == portno)
-       */
-
-      ++g_last_udp_port;
-
-      /* Make sure that the port number is within range */
-
-      if (g_last_udp_port > CONFIG_NET_DEFAULT_MAX_PORT ||
-          g_last_udp_port < CONFIG_NET_DEFAULT_MIN_PORT)
         {
-          g_last_udp_port = CONFIG_NET_DEFAULT_MIN_PORT;
+          /* We have looped back, failed. */
+
+          portno = 0;
+          goto errout;
         }
     }
   while (udp_find_conn(domain, u, HTONS(g_last_udp_port), 0) != NULL
@@ -578,6 +573,8 @@ uint16_t udp_select_port(uint8_t domain, FAR union ip_binding_u *u)
    */
 
   portno = g_last_udp_port;
+
+errout:
   net_unlock();
 
   return portno;
@@ -918,8 +915,16 @@ int udp_bind(FAR struct udp_conn_s *conn, FAR const struct sockaddr *addr)
     {
       /* Yes.. Select any unused local port number */
 
-      conn->lport = HTONS(udp_select_port(conn->domain, &conn->u));
+      portno = HTONS(udp_select_port(conn->domain, &conn->u));
-      ret         = OK;
+      if (portno == 0)
+        {
+          ret         = -EADDRINUSE;
+        }
+      else
+        {
+          conn->lport = portno;
+          ret         = OK;
+        }
     }
   else
     {
@@ -1000,6 +1005,11 @@ int udp_connect(FAR struct udp_conn_s *conn, FAR const struct sockaddr *addr)
        */
 
       conn->lport = HTONS(udp_select_port(conn->domain, &conn->u));
+      if (!conn->lport)
+        {
+          nerr("ERROR: Failed to get a local port!\n");
+          return -EADDRINUSE;
+        }
     }
 
   /* Is there a remote port (rport)? */

net/udp/udp_sendto_buffered.c

@@ -263,6 +263,11 @@ static int sendto_next_transfer(FAR struct udp_conn_s *conn)
        */
 
       conn->lport = HTONS(udp_select_port(conn->domain, &conn->u));
+      if (!conn->lport)
+        {
+          nerr("ERROR: Failed to get a local port!\n");
+          return -EADDRINUSE;
+        }
     }
 
   /* Get the device that will handle the remote packet transfers.  This

net/utils/utils.h

@@ -30,6 +30,47 @@
 #include <nuttx/net/ip.h>
 #include <nuttx/net/netdev.h>
 
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Some utils for port selection */
+
+#define NET_PORT_RANDOM_INIT(port) \
+  do \
+    { \
+      net_getrandom(&(port), sizeof(port)); \
+      (port) = (port) % (CONFIG_NET_DEFAULT_MAX_PORT - \
+                         CONFIG_NET_DEFAULT_MIN_PORT + 1); \
+      (port) += CONFIG_NET_DEFAULT_MIN_PORT; \
+    } while (0)
+
+/* Get next net port number, and make sure that the port number is within
+ * range.  In host byte order.
+ */
+
+#define NET_PORT_NEXT_H(hport) \
+  do \
+    { \
+      ++(hport); \
+      if ((hport) > CONFIG_NET_DEFAULT_MAX_PORT || \
+          (hport) < CONFIG_NET_DEFAULT_MIN_PORT) \
+        { \
+          (hport) = CONFIG_NET_DEFAULT_MIN_PORT; \
+        } \
+    } while (0)
+
+/* Get next net port number, and make sure that the port number is within
+ * range.  In both network & host byte order.
+ */
+
+#define NET_PORT_NEXT_NH(nport, hport) \
+  do \
+    { \
+      NET_PORT_NEXT_H(hport); \
+      (nport) = HTONS(hport); \
+    } while (0)
+
 /****************************************************************************
  * Public Types
  ****************************************************************************/