Merge pull request #1919 from private-octopus/path-0-re-add

Fix definition of available path

Author: huitema

CMakeLists.txt

@@ -8,7 +8,7 @@ else()
 endif()
 
 project(picoquic
-        VERSION 1.1.38.0
+        VERSION 1.1.38.1
         DESCRIPTION "picoquic library"
         LANGUAGES C CXX)
 

UnitTest1/unittest1.cpp

@@ -2846,12 +2846,24 @@ namespace UnitTest1
             Assert::AreEqual(ret, 0);
         }
 
+        TEST_METHOD(multipath_socket0_error) {
+            int ret = multipath_socket0_error_test();
+
+            Assert::AreEqual(ret, 0);
+        }
+
         TEST_METHOD(multipath_abandon) {
             int ret = multipath_abandon_test();
 
             Assert::AreEqual(ret, 0);
         }
 
+        TEST_METHOD(multipath_back0) {
+            int ret = multipath_back0_test();
+
+            Assert::AreEqual(ret, 0);
+        }
+
         TEST_METHOD(multipath_back1) {
             int ret = multipath_back1_test();
 

picoquic/picoquic.h

@@ -40,7 +40,7 @@
 extern "C" {
 #endif
 
-#define PICOQUIC_VERSION "1.1.38.0"
+#define PICOQUIC_VERSION "1.1.38.1"
 #define PICOQUIC_ERROR_CLASS 0x400
 #define PICOQUIC_ERROR_DUPLICATE (PICOQUIC_ERROR_CLASS + 1)
 #define PICOQUIC_ERROR_AEAD_CHECK (PICOQUIC_ERROR_CLASS + 3)

picoquic/picoquic_internal.h

@@ -1532,6 +1532,7 @@ typedef struct st_picoquic_cnx_t {
     * */
     uint64_t nb_local_cnxid_lists;
     uint64_t next_path_id_in_lists;
+    uint64_t max_path_id_in_cnxid_lists;
     picoquic_local_cnxid_list_t * first_local_cnxid_list;
 
     /* Management of ACK frequency */

picoquic/quicctx.c

@@ -1531,23 +1531,32 @@ uint64_t picoquic_find_avalaible_unique_path_id(picoquic_cnx_t* cnx, uint64_t re
 {
     uint64_t unique_path_id = requested_id;
 
-    if (requested_id == UINT64_MAX) {
-        if (!cnx->is_multipath_enabled) {
-            unique_path_id = cnx->unique_path_id_next;
-            cnx->unique_path_id_next++;
+    if (!cnx->is_multipath_enabled) {
+        if (requested_id != 0 && requested_id != UINT64_MAX) {
+            unique_path_id = UINT64_MAX;
         }
         else {
-            /* Look at available stashes. exlcude stash if id=0, as this is the
-             * always used.
-             */
-            picoquic_remote_cnxid_stash_t* stash = cnx->first_remote_cnxid_stash;
-
-            while (stash != NULL && (stash->is_in_use || stash->unique_path_id == 0)){
-                stash = stash->next_stash;
-            }
-            if (stash != NULL) {
-                unique_path_id = stash->unique_path_id;
-            }
+            unique_path_id = 0;
+        }
+    }
+    else {
+        /* Unique path ID are allocated in sequence on the client. The server should
+         * always use the number proposed by the client in incoming packets */
+        if (requested_id == UINT64_MAX && (cnx->client_mode || cnx->nb_paths == 0)) {
+            while (cnx->unique_path_id_next < cnx->max_path_id_remote &&
+                cnx->unique_path_id_next < cnx->max_path_id_local &&
+                cnx->unique_path_id_next < cnx->max_path_id_in_cnxid_lists) {
+                /* Find next non used CID */
+                unique_path_id = cnx->unique_path_id_next++;
+                /* There should be an available of CNX_ID for this path_id, 
+                * and that path_id should not be already created.
+                */
+                if (picoquic_find_or_create_local_cnxid_list(cnx, unique_path_id, 0) != NULL &&
+                    picoquic_find_path_by_unique_id(cnx, unique_path_id) < 0) {
+                    /* this CID was not already deleted */
+                    break;
+                }
+             }
         }
     }
     return unique_path_id;
@@ -3630,8 +3639,6 @@ picoquic_local_cnxid_list_t* picoquic_find_or_create_local_cnxid_list(picoquic_c
     return local_cnxid_list;
 }
 
-
-
 picoquic_local_cnxid_t* picoquic_create_local_cnxid(picoquic_cnx_t* cnx, 
     uint64_t unique_path_id, picoquic_connection_id_t* suggested_value, uint64_t current_time)
 {
@@ -3690,6 +3697,9 @@ picoquic_local_cnxid_t* picoquic_create_local_cnxid(picoquic_cnx_t* cnx,
                 }
                 if (l_cid->sequence == 0) {
                     local_cnxid_list->local_cnxid_oldest_created = current_time;
+                    if (local_cnxid_list->unique_path_id > cnx->max_path_id_in_cnxid_lists) {
+                        cnx->max_path_id_in_cnxid_lists = local_cnxid_list->unique_path_id;
+                    }
                 }
             }
             else {

picoquic_t/picoquic_t.c

@@ -485,7 +485,9 @@ static const picoquic_test_def_t test_table[] = {
     { "multipath_rotation", multipath_rotation_test },
     { "multipath_break1", multipath_break1_test },
     { "multipath_socket_error", multipath_socket_error_test },
+    { "multipath_socket0_error", multipath_socket0_error_test },
     { "multipath_abandon", multipath_abandon_test },
+    { "multipath_back0", multipath_back0_test },
     { "multipath_back1", multipath_back1_test },
     { "multipath_nat", multipath_nat_test },
     { "multipath_nat_challenge", multipath_nat_challenge_test },

picoquictest/multipath_test.c

@@ -96,6 +96,18 @@ static void multipath_test_set_unreachable(picoquic_test_tls_api_ctx_t* test_ctx
     }
 }
 
+static void multipath_test_set_reachable(picoquic_test_tls_api_ctx_t* test_ctx, int link_id)
+{
+    if (link_id == 0) {
+        test_ctx->c_to_s_link->is_unreachable = 0;
+        test_ctx->s_to_c_link->is_unreachable = 0;
+    }
+    else {
+        test_ctx->c_to_s_link_2->is_unreachable = 0;
+        test_ctx->s_to_c_link_2->is_unreachable = 0;
+    }
+}
+
 static void multipath_test_unkill_links(picoquic_test_tls_api_ctx_t* test_ctx, int link_id, uint64_t current_time)
 {
     /* Make sure that nothing gets sent on the old links */
@@ -438,6 +450,8 @@ typedef enum {
     multipath_test_nat_challenge,
     multipath_test_break1,
     multipath_test_break2,
+    multipath_test_break3,
+    multipath_test_back0,
     multipath_test_back1,
     multipath_test_perf,
     multipath_test_callback,
@@ -911,8 +925,8 @@ int multipath_test_one(uint64_t max_completion_microsec, multipath_test_enum_t t
 
     if (ret == 0 && (test_id == multipath_test_drop_first || test_id == multipath_test_drop_second ||
         test_id == multipath_test_renew || test_id == multipath_test_nat || test_id == multipath_test_nat_challenge ||
-        test_id == multipath_test_break1 || test_id == multipath_test_break2 ||
-        test_id == multipath_test_back1 || test_id == multipath_test_standup ||
+        test_id == multipath_test_break1 || test_id == multipath_test_break2 || test_id == multipath_test_break3 ||
+        test_id == multipath_test_back0 || test_id == multipath_test_back1 || test_id == multipath_test_standup ||
         test_id == multipath_test_abandon || test_id == multipath_test_tunnel)) {
         /* If testing a final link drop before completion, perform a 
          * partial sending loop and then kill the initial link.
@@ -950,6 +964,10 @@ int multipath_test_one(uint64_t max_completion_microsec, multipath_test_enum_t t
                 /* Trigger "destination unreachable" error on next socket call to link 1 */
                 multipath_test_set_unreachable(test_ctx, 1);
             }
+            else if (test_id == multipath_test_back0 || test_id == multipath_test_break3) {
+                /* Trigger "destination unreachable" error on next socket call to link 0 */
+                multipath_test_set_unreachable(test_ctx, 0);
+            }
             else if (test_id == multipath_test_tunnel) {
                 /* Break both links */
                 multipath_test_kill_links(test_ctx, 0);
@@ -964,7 +982,7 @@ int multipath_test_one(uint64_t max_completion_microsec, multipath_test_enum_t t
     /* For the "backup scenario", wait a small interval, then bring the path # 1 back up
      * For the "tunnel" scenario, do the same but wait 5 seconds and then restore both links.
      */
-    if (ret == 0 && (test_id == multipath_test_back1 || test_id == multipath_test_tunnel)) {
+    if (ret == 0 && (test_id == multipath_test_back0 || test_id == multipath_test_back1 || test_id == multipath_test_tunnel)) {
         uint64_t timeout = (test_id == multipath_test_tunnel)?5000000:1000000;
 
         ret = tls_api_wait_for_timeout(test_ctx, &simulated_time, timeout);
@@ -973,6 +991,15 @@ int multipath_test_one(uint64_t max_completion_microsec, multipath_test_enum_t t
         {
             DBG_PRINTF("Wait for %" PRIu64 "us returns %d\n", timeout, ret);
         }
+        else if (test_id == multipath_test_back0) {
+            multipath_test_set_reachable(test_ctx, 0);
+            
+            ret = picoquic_probe_new_path(test_ctx->cnx_client, (struct sockaddr*)&test_ctx->server_addr,
+                    (struct sockaddr*)&test_ctx->client_addr, simulated_time);
+            if (ret != 0) {
+                DBG_PRINTF("Create path on default address returns %d\n", ret);
+            }
+        }
         else {
             if (test_id == multipath_test_tunnel) {
                 multipath_test_unkill_links(test_ctx, 0, simulated_time);
@@ -1140,7 +1167,7 @@ int multipath_test_one(uint64_t max_completion_microsec, multipath_test_enum_t t
         }
     }
 
-    if (ret == 0 && (test_id == multipath_test_break1 || test_id == multipath_test_break2 || test_id == multipath_test_abandon)) {
+    if (ret == 0 && (test_id == multipath_test_break1 || test_id == multipath_test_break2 || test_id == multipath_test_break3 || test_id == multipath_test_abandon)) {
         if (test_ctx->cnx_server->nb_paths != 1) {
             DBG_PRINTF("After break, %d paths on server connection.\n", test_ctx->cnx_server->nb_paths);
             ret = -1;
@@ -1150,7 +1177,7 @@ int multipath_test_one(uint64_t max_completion_microsec, multipath_test_enum_t t
         }
     }
 
-    if (ret == 0 && test_id == multipath_test_back1) {
+    if (ret == 0 && (test_id == multipath_test_back0 || test_id == multipath_test_back1)) {
         if (test_ctx->cnx_server->nb_paths != 2) {
             DBG_PRINTF("After break and back, %d paths on server connection.\n", test_ctx->cnx_server->nb_paths);
             ret = -1;
@@ -1347,6 +1374,15 @@ int multipath_socket_error_test()
     return  multipath_test_one(max_completion_microsec, multipath_test_break2);
 }
 
+/* Test reaction to socket error on first path
+ */
+int multipath_socket0_error_test()
+{
+    uint64_t max_completion_microsec = 10900000;
+
+    return  multipath_test_one(max_completion_microsec, multipath_test_break3);
+}
+
 /* Test that abandoned paths are removed after some time
  */
 int multipath_abandon_test()
@@ -1356,6 +1392,16 @@ int multipath_abandon_test()
     return  multipath_test_one(max_completion_microsec, multipath_test_abandon);
 }
 
+/* Test that after breaking path 0 we can establish a new path on the
+* same link when it comes back up.
+ */
+int multipath_back0_test()
+{
+    uint64_t max_completion_microsec = 3300000;
+
+    return  multipath_test_one(max_completion_microsec, multipath_test_back0);
+}
+
 /* Test that breaking paths can come back up after some time
  */
 int multipath_back1_test()

picoquictest/picoquictest.h

@@ -501,7 +501,9 @@ int multipath_nat_test();
 int multipath_nat_challenge_test();
 int multipath_break1_test();
 int multipath_socket_error_test();
+int multipath_socket0_error_test();
 int multipath_abandon_test();
+int multipath_back0_test();
 int multipath_back1_test();
 int multipath_perf_test();
 int multipath_callback_test();