Improve coverage and quality of socket loop function.

CMakeLists.txt

@@ -163,6 +163,7 @@ set(PICOQUIC_TEST_LIBRARY_FILES
     picoquictest/satellite_test.c
     picoquictest/skip_frame_test.c
     picoquictest/socket_test.c
+    picoquictest/sockloop_test.c
     picoquictest/splay_test.c
     picoquictest/stream0_frame_test.c
     picoquictest/stresstest.c

UnitTest1/unittest1.cpp

@@ -111,6 +111,48 @@ namespace UnitTest1
             Assert::AreEqual(ret, 0);
         }
 
+        TEST_METHOD(sockloop_basic)
+        {
+            int ret = sockloop_basic_test();
+
+            Assert::AreEqual(ret, 0);
+        }
+
+        TEST_METHOD(sockloop_eio)
+        {
+            int ret = sockloop_eio_test();
+
+            Assert::AreEqual(ret, 0);
+        }
+
+        TEST_METHOD(sockloop_errsock)
+        {
+            int ret = sockloop_errsock_test();
+
+            Assert::AreEqual(ret, 0);
+        }
+
+        TEST_METHOD(sockloop_ipv4)
+        {
+            int ret = sockloop_ipv4_test();
+
+            Assert::AreEqual(ret, 0);
+        }
+
+        TEST_METHOD(sockloop_migration)
+        {
+            int ret = sockloop_migration_test();
+
+            Assert::AreEqual(ret, 0);
+        }
+
+        TEST_METHOD(sockloop_nat)
+        {
+            int ret = sockloop_nat_test();
+
+            Assert::AreEqual(ret, 0);
+        }
+
         TEST_METHOD(splay)
         {
             int ret = splay_test();

picoquic/picoquic.h

@@ -849,10 +849,10 @@ void picoquic_set_rejected_version(picoquic_cnx_t* cnx, uint32_t rejected_versio
  * connection data harder in case of NAT traversal.
  */
 
-int picoquic_probe_new_path(picoquic_cnx_t* cnx, const struct sockaddr* addr_from,
-    const struct sockaddr* addr_to, uint64_t current_time);
-int picoquic_probe_new_path_ex(picoquic_cnx_t* cnx, const struct sockaddr* addr_from,
-    const struct sockaddr* addr_to, int if_index, uint64_t current_time, int to_preferred_address);
+int picoquic_probe_new_path(picoquic_cnx_t* cnx, const struct sockaddr* addr_peer,
+    const struct sockaddr* addr_local, uint64_t current_time);
+int picoquic_probe_new_path_ex(picoquic_cnx_t* cnx, const struct sockaddr* addr_peer,
+    const struct sockaddr* addr_local, int if_index, uint64_t current_time, int to_preferred_address);
 void picoquic_enable_path_callbacks(picoquic_cnx_t* cnx, int are_enabled);
 void picoquic_enable_path_callbacks_default(picoquic_quic_t* quic, int are_enabled);
 int picoquic_set_app_path_ctx(picoquic_cnx_t* cnx, uint64_t unique_path_id, void * app_path_ctx);

picoquic/picoquic.vcxproj

@@ -207,6 +207,7 @@
     <ClInclude Include="picosocks.h" />
     <ClInclude Include="picosplay.h" />
     <ClInclude Include="picoquic.h" />
+    <ClInclude Include="sockloop.h" />
     <ClInclude Include="tls_api.h" />
     <ClInclude Include="picoquic_utils.h" />
     <ClInclude Include="wincompat.h" />

picoquic/picoquic.vcxproj.filters

@@ -194,5 +194,8 @@
     <ClInclude Include="picoquic_crypto_provider_api.h">
       <Filter>Header Files</Filter>
     </ClInclude>
+    <ClInclude Include="sockloop.h">
+      <Filter>Header Files</Filter>
+    </ClInclude>
   </ItemGroup>
 </Project>

picoquic/picoquic_packet_loop.h

@@ -29,10 +29,45 @@
 extern "C" {
 #endif
 
-#define PICOQUIC_PACKET_LOOP_SOCKETS_MAX 2
+#define PICOQUIC_PACKET_LOOP_SOCKETS_MAX 4
 #define PICOQUIC_PACKET_LOOP_SEND_MAX 10
 #define PICOQUIC_PACKET_LOOP_SEND_DELAY_MAX 2500
 
+typedef struct st_picoquic_socket_ctx_t {
+    SOCKET_TYPE fd;
+    int af;
+    uint16_t port;
+
+    /* Flags */
+    unsigned int is_started : 1;
+    unsigned int supports_udp_send_coalesced : 1;
+    unsigned int supports_udp_recv_coalesced : 1;
+    /* Receive data buffer and fields */
+    size_t recv_buffer_size;
+    uint8_t* recv_buffer;
+    struct sockaddr_storage addr_from;
+    struct sockaddr_storage addr_dest;
+    socklen_t from_length;
+    socklen_t dest_length;
+    int dest_if;
+    unsigned char received_ecn;
+    int bytes_recv;
+    /* Management of sendmsg */
+    char cmsg_buffer[1024];
+    size_t udp_coalesced_size;
+#ifdef _WINDOWS
+    /* Windows specific */
+    WSAOVERLAPPED overlap;
+    LPFN_WSARECVMSG WSARecvMsg;
+    LPFN_WSASENDMSG WSASendMsg;
+    WSABUF dataBuf;
+    WSAMSG msg;
+    int nb_immediate_receive;
+    int so_sndbuf;
+    int so_rcvbuf;
+#endif
+} picoquic_socket_ctx_t;
+
 /* The packet loop will call the application back after specific events.
  */
 typedef enum {
@@ -62,6 +97,26 @@ typedef struct st_packet_loop_time_check_arg_t {
     int64_t delta_t;
 } packet_loop_time_check_arg_t;
 
+/* Version 2 of packet loop, works in progress.
+* Parameters are set in a struct, for future
+* extensibility.
+ */
+typedef struct st_picoquic_packet_loop_param_t {
+    uint16_t local_port;
+    int local_af;
+    int dest_if;
+    int socket_buffer_size;
+    int do_not_use_gso;
+    int extra_socket_required;
+    int simulate_eio;
+    size_t send_length_max;
+} picoquic_packet_loop_param_t;
+
+int picoquic_packet_loop_v2(picoquic_quic_t* quic,
+    picoquic_packet_loop_param_t * param,
+    picoquic_packet_loop_cb_fn loop_callback,
+    void * loop_callback_ctx);
+
 /* Two versions of the packet loop, one portable and one speciailezed
  * for winsock.
  */
@@ -84,6 +139,11 @@ int picoquic_packet_loop_win(picoquic_quic_t* quic,
     void* loop_callback_ctx);
 #endif
 
+/* Following declarations are used for unit tests. */
+void picoquic_packet_loop_close_socket(picoquic_socket_ctx_t* s_ctx);
+int picoquic_packet_loop_open_sockets(uint16_t local_port, int local_af, int socket_buffer_size, int extra_socket_required,
+    int do_not_use_gso, picoquic_socket_ctx_t* s_ctx);
+
 #ifdef __cplusplus
 }
 #endif

picoquic/picoquic_utils.h

@@ -223,6 +223,10 @@ void picoquic_delete_event(picoquic_event_t* event);
 int picoquic_signal_event(picoquic_event_t* event);
 int picoquic_wait_for_event(picoquic_event_t* event, uint64_t microsec_wait);
 
+/* Simple potable random number generation
+ */
+uint64_t picoquic_uniform_random(uint64_t rnd_max);
+
 /* Set of random number generation functions, designed for tests.
  * The random numbers are defined by a 64 bit context, initialized to a seed.
  * The same seed will always generate the same sequence.

picoquic/picosocks.h

@@ -206,6 +206,13 @@ int picoquic_select_ex(SOCKET_TYPE* sockets,
     int* socket_rank,
     uint64_t* current_time);
 
+int picoquic_recvmsg(SOCKET_TYPE fd,
+    struct sockaddr_storage* addr_from,
+    struct sockaddr_storage* addr_dest,
+    int* dest_if,
+    unsigned char* received_ecn,
+    uint8_t* buffer, int buffer_max);
+
 int picoquic_sendmsg(SOCKET_TYPE fd,
     struct sockaddr* addr_dest,
     struct sockaddr* addr_from,

picoquic/quicctx.c

@@ -1995,8 +1995,8 @@ int picoquic_assign_peer_cnxid_to_path(picoquic_cnx_t* cnx, int path_id)
 }
 
 /* Create a new path in order to trigger a migration */
-int picoquic_probe_new_path_ex(picoquic_cnx_t* cnx, const struct sockaddr* addr_from,
-    const struct sockaddr* addr_to, int if_index, uint64_t current_time, int to_preferred_address)
+int picoquic_probe_new_path_ex(picoquic_cnx_t* cnx, const struct sockaddr* addr_peer,
+    const struct sockaddr* addr_local, int if_index, uint64_t current_time, int to_preferred_address)
 {
     int ret = 0;
     int partial_match_path = -1;
@@ -2008,11 +2008,11 @@ int picoquic_probe_new_path_ex(picoquic_cnx_t* cnx, const struct sockaddr* addr_
         ret = PICOQUIC_ERROR_MIGRATION_DISABLED;
         DBG_PRINTF("Tried to create probe with migration disabled = %d", cnx->remote_parameters.migration_disabled);
     }
-    else if ((path_id = picoquic_find_path_by_address(cnx, addr_to, addr_from, &partial_match_path)) >= 0) {
+    else if ((path_id = picoquic_find_path_by_address(cnx, addr_local, addr_peer, &partial_match_path)) >= 0) {
         /* This path already exists. Will not create it, but will restore it in working order if disabled. */
         ret = -1;
     }
-    else if (partial_match_path >= 0 && addr_from->sa_family == 0) {
+    else if (partial_match_path >= 0 && addr_peer->sa_family == 0) {
         /* This path already exists. Will not create it, but will restore it in working order if disabled. */
         ret = -1;
     }
@@ -2024,7 +2024,7 @@ int picoquic_probe_new_path_ex(picoquic_cnx_t* cnx, const struct sockaddr* addr_
         /* Too many paths created already */
         ret = -1;
     }
-    else if (picoquic_create_path(cnx, current_time, addr_to, addr_from) > 0) {
+    else if (picoquic_create_path(cnx, current_time, addr_local, addr_peer) > 0) {
         path_id = cnx->nb_paths - 1;
         ret = picoquic_assign_peer_cnxid_to_path(cnx, path_id);
 
@@ -2082,10 +2082,10 @@ int picoquic_set_app_path_ctx(picoquic_cnx_t* cnx, uint64_t unique_path_id, void
     return ret;
 }
 
-int picoquic_probe_new_path(picoquic_cnx_t* cnx, const struct sockaddr* addr_from,
-    const struct sockaddr* addr_to, uint64_t current_time)
+int picoquic_probe_new_path(picoquic_cnx_t* cnx, const struct sockaddr* addr_peer,
+    const struct sockaddr* addr_local, uint64_t current_time)
 {
-    return picoquic_probe_new_path_ex(cnx, addr_from, addr_to, 0, current_time, 0);
+    return picoquic_probe_new_path_ex(cnx, addr_peer, addr_local, 0, current_time, 0);
 }
 
 /* TODO: the "unique_path_id" should really be a unique ID, managed by the stack.
@@ -4688,3 +4688,10 @@ int picoquic_process_version_upgrade(picoquic_cnx_t* cnx, int old_version_index,
     }
     return ret;
 }
+
+/* Simple portable number generation. */
+uint64_t picoquic_uniform_random(uint64_t rnd_max)
+{
+    return picoquic_public_uniform_random(rnd_max);
+}
+

picoquic/sender.c

@@ -3184,34 +3184,36 @@ int picoquic_prepare_packet_almost_ready(picoquic_cnx_t* cnx, picoquic_path_t* p
 
     /* Verify first that there is no need for retransmit or ack
      * on initial or handshake context. */
-    if (cnx->crypto_context[picoquic_epoch_initial].aead_encrypt != NULL) {
-        length = picoquic_prepare_packet_old_context(cnx, picoquic_packet_context_initial,
-            path_x, packet, send_buffer_min_max, current_time, next_wake_time, &header_length);
-    }
-    else {
-        length = 0;
-    }
+    if (path_x->p_local_cnxid != NULL) {
+        if (cnx->crypto_context[picoquic_epoch_initial].aead_encrypt != NULL) {
+            length = picoquic_prepare_packet_old_context(cnx, picoquic_packet_context_initial,
+                path_x, packet, send_buffer_min_max, current_time, next_wake_time, &header_length);
+        }
+        else {
+            length = 0;
+        }
 
-    if (length == 0) {
-        length = picoquic_prepare_packet_old_context(cnx, picoquic_packet_context_handshake,
-            path_x, packet, send_buffer_min_max, current_time, next_wake_time, &header_length);
-        if (length > 0) {
-            checksum_overhead = picoquic_get_checksum_length(cnx, picoquic_epoch_handshake);
-            bytes_max = bytes + send_buffer_min_max - checksum_overhead;
+        if (length == 0) {
+            length = picoquic_prepare_packet_old_context(cnx, picoquic_packet_context_handshake,
+                path_x, packet, send_buffer_min_max, current_time, next_wake_time, &header_length);
+            if (length > 0) {
+                checksum_overhead = picoquic_get_checksum_length(cnx, picoquic_epoch_handshake);
+                bytes_max = bytes + send_buffer_min_max - checksum_overhead;
+            }
         }
-    }
-    else {
-        checksum_overhead = picoquic_get_checksum_length(cnx, picoquic_epoch_initial);
-        bytes_max = bytes + send_buffer_min_max - checksum_overhead;
+        else {
+            checksum_overhead = picoquic_get_checksum_length(cnx, picoquic_epoch_initial);
+            bytes_max = bytes + send_buffer_min_max - checksum_overhead;
 
-        *is_initial_sent = 1;
-    }
+            *is_initial_sent = 1;
+        }
 
-    if (length > 0) {
-        cnx->initial_repeat_needed = 0;
+        if (length > 0) {
+            cnx->initial_repeat_needed = 0;
 
-        if (cnx->client_mode && *is_initial_sent && send_buffer_min_max < length + checksum_overhead + PICOQUIC_MIN_SEGMENT_SIZE) {
-            length = picoquic_pad_to_target_length(packet->bytes, length, send_buffer_min_max - checksum_overhead);
+            if (cnx->client_mode && *is_initial_sent && send_buffer_min_max < length + checksum_overhead + PICOQUIC_MIN_SEGMENT_SIZE) {
+                length = picoquic_pad_to_target_length(packet->bytes, length, send_buffer_min_max - checksum_overhead);
+            }
         }
     }
 
@@ -4227,7 +4229,7 @@ static int picoquic_select_next_path(picoquic_cnx_t * cnx, uint64_t current_time
             picoquic_demote_path(cnx, i, current_time);
             continue;
         }
-        else if (cnx->path[i]->challenge_verified) {
+        else if (cnx->path[i]->challenge_verified && cnx->cnx_state == picoquic_state_ready) {
             /* logic to synchronize path selection between server and client:
              * On the client side, this is driven by the "probe/validate" sequence; the
              * assumption is that if the client probes a new path, it want to use it