Almost everything debugged, except for multipath

Author: huitema

picoquic/frames.c

@@ -705,9 +705,9 @@ const uint8_t* picoquic_decode_retire_connection_id_frame(picoquic_cnx_t* cnx, c
         picoquic_connection_error(cnx, PICOQUIC_TRANSPORT_FRAME_FORMAT_ERROR,
             (is_mp)?picoquic_frame_type_path_retire_connection_id:picoquic_frame_type_retire_connection_id);
     }
-    else if (path_x->p_local_cnxid != NULL &&
+    else if (path_x->first_tuple->p_local_cnxid != NULL &&
         (!is_mp || path_x->unique_path_id == unique_path_id) &&
-        sequence == path_x->p_local_cnxid->sequence) {
+        sequence == path_x->first_tuple->p_local_cnxid->sequence) {
         /* Cannot delete the path through which it arrives */
         picoquic_connection_error(cnx, PICOQUIC_TRANSPORT_PROTOCOL_VIOLATION,
             (is_mp) ? picoquic_frame_type_path_retire_connection_id : picoquic_frame_type_retire_connection_id);
@@ -3822,17 +3822,13 @@ uint8_t* picoquic_format_ack_frame_in_context(picoquic_cnx_t* cnx, uint8_t* byte
         (((is_ecn) ? picoquic_frame_type_path_ack_ecn : picoquic_frame_type_path_ack));
 
     /* Check that there something to acknowledge */
-#if 1
     int not_needed = picoquic_sack_list_is_empty(&ack_ctx->sack_list);
     if (!not_needed && !ack_ctx->act[is_opportunistic].ack_needed &&
         ack_ctx->sack_list.ack_tree.size == 1) {
         picoquic_sack_item_t* last_sack = picoquic_sack_last_item(&ack_ctx->sack_list);
         not_needed = (last_sack->nb_times_sent[is_opportunistic] >= PICOQUIC_MAX_ACK_RANGE_REPEAT);
     }
     if (!not_needed){
-#else
-    if (!picoquic_sack_list_is_empty(&ack_ctx->sack_list)) {
-#endif
         uint8_t* num_block_byte = NULL;
         picoquic_sack_item_t* last_sack = picoquic_sack_last_item(&ack_ctx->sack_list);
 
@@ -4031,9 +4027,9 @@ uint64_t picoquic_ack_gap_override_if_needed(picoquic_cnx_t* cnx, int path_index
     uint64_t ack_gap = cnx->ack_gap_remote;
     if (cnx->is_multipath_enabled) {
         if (!cnx->path[path_index]->path_is_demoted &&
-            !cnx->path[path_index]->challenge_failed &&
-            !cnx->path[path_index]->response_required &&
-            cnx->path[path_index]->challenge_verified &&
+            !cnx->path[path_index]->first_tuple->challenge_failed &&
+            !cnx->path[path_index]->first_tuple->response_required &&
+            cnx->path[path_index]->first_tuple->challenge_verified &&
             cnx->path[path_index]->received < 100 * PICOQUIC_MAX_PACKET_SIZE) {
             ack_gap = 2;
         }
@@ -4652,26 +4648,30 @@ const uint8_t* picoquic_decode_path_challenge_frame(picoquic_cnx_t* cnx, const u
             if (!is_valid) {
                 /* If multipath is not enabled, we must verify that the addresses
                  * source (addr_from) matches the peer address if known. */
-                if (addr_from == NULL ||
-                    picoquic_compare_addr(addr_from, (struct sockaddr*)&path_x->first_tuple->peer_addr) == 0) {
-                    /* If the source address matches, we must verify that the destination
-                    * address also matches. Given how the socket code works there will be cases
-                    * when the local port is now yet known. In that case, we only compare
-                    * the IP address component . Otherwise, we compare the whole address.
-                    */
-                    if (addr_to == NULL ||
-                        (picoquic_get_addr_port((struct sockaddr*)&path_x->first_tuple->local_addr) == 0 &&
-                            picoquic_compare_ip_addr(addr_to, (struct sockaddr*)&path_x->first_tuple->local_addr) == 0) ||
-                        picoquic_compare_addr(addr_to, (struct sockaddr*)&path_x->first_tuple->local_addr) == 0) {
-                        is_valid = 1;
+                picoquic_tuple_t* tuple = path_x->first_tuple;
+                while (tuple != NULL) {
+
+                    if (addr_from == NULL ||
+                        picoquic_compare_addr(addr_from, (struct sockaddr*)&tuple->peer_addr) == 0) {
+                        /* If the source address matches, we must verify that the destination
+                        * address also matches. Given how the socket code works there will be cases
+                        * when the local port is now yet known. In that case, we only compare
+                        * the IP address component . Otherwise, we compare the whole address.
+                        */
+                        if (addr_to == NULL ||
+                            (picoquic_get_addr_port((struct sockaddr*)&tuple->local_addr) == 0 &&
+                                picoquic_compare_ip_addr(addr_to, (struct sockaddr*)&tuple->local_addr) == 0) ||
+                            picoquic_compare_addr(addr_to, (struct sockaddr*)&tuple->local_addr) == 0) {
+                            is_valid = 1;
+                            tuple->challenge_response = challenge_response;
+                            tuple->response_required = 1;
+                            break;
+                        }
                     }
+                    tuple = tuple->next_tuple;
                 }
             }
-            if (is_valid) {
-                path_x->first_tuple->challenge_response = challenge_response;
-                path_x->response_required = 1;
-            }
-            else {
+            if (!is_valid) {
                 char buf1[128], buf2[128], buf3[128], buf4[128];
                 picoquic_log_app_message(cnx,
                     "Path challenge[%" PRIu64 "] from %s to %s ignored, wrong addresses, expected %s - %s.\n",
@@ -4714,54 +4714,45 @@ const uint8_t* picoquic_decode_path_response_frame(picoquic_cnx_t* cnx, const ui
         /* Per QUIC V1, path responses must come on the same path. Ignore them if this cannot be verified. */
         if (path_x != NULL) {
             int found_challenge = 0;
-            int found_nat_challenge = 0;
+            picoquic_tuple_t* tuple = path_x->first_tuple;
+            picoquic_tuple_t* previous_tuple = NULL;
 
-            for (int ichal = 0; ichal < PICOQUIC_CHALLENGE_REPEAT_MAX; ichal++) {
-                if (response == path_x->first_tuple->challenge[ichal]) {
-                    found_challenge = 1;
-                    break;
-                }
-            }
-            if (!found_challenge && path_x->nat_peer_addr.ss_family != AF_UNSPEC) {
+            while (tuple != NULL) {
                 for (int ichal = 0; ichal < PICOQUIC_CHALLENGE_REPEAT_MAX; ichal++) {
-                    if (response == path_x->nat_challenge[ichal]) {
-                        found_nat_challenge = 1;
+                    if (response == tuple->challenge[ichal]) {
+                        found_challenge = 1;
                         break;
                     }
                 }
-            }
-            if (found_nat_challenge && !path_x->challenge_verified) {
-                /* while probing NAT, the NAT response arrived before the normal path response */
-                /* Update the addresses */
-                picoquic_store_addr(&path_x->first_tuple->local_addr, (struct sockaddr*)&path_x->nat_local_addr);
-                picoquic_update_peer_addr(path_x, (struct sockaddr*)&path_x->nat_peer_addr);
-                path_x->first_tuple->if_index_dest = path_x->if_index_nat_dest;
-                /* if useful, update the CID */
-                if (path_x->p_remote_nat_cnxid != NULL) {
-                    picoquic_dereference_stashed_cnxid(cnx, path_x, 0);
-                    path_x->p_remote_cnxid = path_x->p_remote_nat_cnxid;
-                    path_x->p_remote_nat_cnxid = NULL;
+                if (found_challenge) {
+                    break;
                 }
-                /* Consider this a successful challenge */
-                found_challenge = 1;
+                previous_tuple = tuple;
+                tuple = tuple->next_tuple;
             }
 
-            if (found_challenge && !path_x->challenge_verified){
-                /* TODO: update the RTT if using initial value */
-                path_x->challenge_verified = 1;
-
+            if (found_challenge && !tuple->challenge_verified) {
+                tuple->challenge_verified = 1;
                 /* Provide a qualified time estimate from challenge time */
-                picoquic_update_path_rtt(cnx, path_x, path_x, -1, path_x->first_tuple->challenge_time_first, current_time, 0, 0);
-
-                if (cnx->are_path_callbacks_enabled &&
-                    cnx->callback_fn(cnx, path_x->unique_path_id, NULL, 0, picoquic_callback_path_available,
-                    cnx->callback_ctx, path_x->app_path_ctx) != 0) {
-                    picoquic_connection_error_ex(cnx, PICOQUIC_TRANSPORT_INTERNAL_ERROR,
-                        picoquic_frame_type_path_response, "path available callback");
-                    bytes = NULL;
+                picoquic_update_path_rtt(cnx, path_x, path_x, -1, tuple->challenge_time_first, current_time, 0, 0);
+                /* This challenge is verified. 
+                * If this is a client, the path should replace the current path.
+                * If this is a server, the replacement should happen if we have a NAT, maybe,
+                * or if qualifying data is received, which will happen later.
+                */
+                if (previous_tuple != NULL /* && cnx->client_mode */) {
+                    previous_tuple->next_tuple = tuple->next_tuple;
+                    tuple->next_tuple = path_x->first_tuple;
+                    path_x->first_tuple = tuple;
+
+                    if (cnx->are_path_callbacks_enabled &&
+                        cnx->callback_fn(cnx, path_x->unique_path_id, NULL, 0, picoquic_callback_path_available,
+                            cnx->callback_ctx, path_x->app_path_ctx) != 0) {
+                        picoquic_connection_error_ex(cnx, PICOQUIC_TRANSPORT_INTERNAL_ERROR,
+                            picoquic_frame_type_path_response, "path available callback");
+                        bytes = NULL;
+                    }
                 }
-                /* Erase the NAT address, to avoid continuing the NAT challenge */
-                path_x->nat_peer_addr.ss_family = AF_UNSPEC;
             }
         }
     }
@@ -4795,8 +4786,8 @@ int picoquic_should_repeat_path_response_frame(picoquic_cnx_t* cnx, const uint8_
         }
 
         if (path_index >= 0 &&
-            (cnx->path[path_index]->challenge_verified ||
-                (cnx->client_mode && !cnx->path[path_index]->challenge_failed))) {
+            (cnx->path[path_index]->first_tuple->challenge_verified ||
+                (cnx->client_mode && !cnx->path[path_index]->first_tuple->challenge_failed))) {
             should_repeat = 1;
         }
         else {
@@ -5549,7 +5540,7 @@ int picoquic_queue_path_available_or_backup_frame(
 {
     int ret = 0;
 
-    if (path_x->p_remote_cnxid == NULL) {
+    if (path_x->first_tuple->p_remote_cnxid == NULL) {
         ret = -1;
     }
     else {
@@ -5560,7 +5551,7 @@ int picoquic_queue_path_available_or_backup_frame(
         uint64_t sequence = cnx->status_sequence_to_send_next++;
         uint64_t path_id = (cnx->is_multipath_enabled)?
             path_x->unique_path_id :
-            path_x->p_remote_cnxid->sequence;
+            path_x->first_tuple->p_remote_cnxid->sequence;
         int is_pure_ack = 0;
         int more_data = 0;
         uint8_t* bytes_next = picoquic_format_path_available_or_backup_frame(
@@ -6071,21 +6062,24 @@ uint8_t* picoquic_format_observed_address_frame(
     return bytes;
 }
 
+
+
 uint8_t* picoquic_prepare_observed_address_frame(uint8_t* bytes, const uint8_t* bytes_max,
-    picoquic_path_t* path_x, uint64_t current_time, uint64_t * next_wake_time,
+    picoquic_path_t* path_x, picoquic_tuple_t * tuple,
+    uint64_t current_time, uint64_t * next_wake_time,
     int * more_data, int* is_pure_ack)
 {
-    if (!path_x->observed_addr_acked && 
-        path_x->nb_observed_repeat < 4 &&
-        path_x->first_tuple->peer_addr.ss_family != AF_UNSPEC) {
+    if (!path_x->observed_addr_acked &&
+        tuple->nb_observed_repeat < 4 &&
+        tuple->peer_addr.ss_family != AF_UNSPEC) {
         int is_needed = 0;
 
-        if (path_x->nb_observed_repeat == 0) {
+        if (tuple->nb_observed_repeat == 0) {
             is_needed = 1;
             path_x->observed_sequence_sent = path_x->cnx->observed_number++;
         }
         else {
-            uint64_t repeat_time = path_x->observed_time + path_x->retransmit_timer;
+            uint64_t repeat_time = tuple->observed_time + path_x->retransmit_timer;
 
             if (repeat_time <= current_time) {
                 is_needed = 1;
@@ -6100,14 +6094,14 @@ uint8_t* picoquic_prepare_observed_address_frame(uint8_t* bytes, const uint8_t*
             uint8_t* ip_addr = NULL;
             uint16_t port = 0;
 
-            if (path_x->first_tuple->peer_addr.ss_family == AF_INET6) {
-                struct sockaddr_in6* addr = (struct sockaddr_in6*)&path_x->first_tuple->peer_addr;
+            if (tuple->peer_addr.ss_family == AF_INET6) {
+                struct sockaddr_in6* addr = (struct sockaddr_in6*)&tuple->peer_addr;
                 ftype = picoquic_frame_type_observed_address_v6;
                 ip_addr = (uint8_t*)&addr->sin6_addr;
                 port = ntohs(addr->sin6_port);
             }
             else {
-                struct sockaddr_in* addr = (struct sockaddr_in*)&path_x->first_tuple->peer_addr;
+                struct sockaddr_in* addr = (struct sockaddr_in*)&tuple->peer_addr;
                 ftype = picoquic_frame_type_observed_address_v4;
                 ip_addr = (uint8_t*)&addr->sin_addr;
                 port = ntohs(addr->sin_port);
@@ -6119,8 +6113,8 @@ uint8_t* picoquic_prepare_observed_address_frame(uint8_t* bytes, const uint8_t*
             if (bytes_next > bytes) {
                 *is_pure_ack = 0;
                 bytes = bytes_next;
-                path_x->nb_observed_repeat += 1;
-                path_x->observed_time = current_time;
+                tuple->nb_observed_repeat += 1;
+                tuple->observed_time = current_time;
             }
         }
     }
@@ -6208,6 +6202,7 @@ int picoquic_process_ack_of_observed_address_frame(picoquic_cnx_t* cnx, picoquic
         ret = -1;
     }
     else {
+        /* TODO: tie this to a specific address and port */
         path_x->observed_addr_acked = 1;
         *consumed = bytes_next - bytes;
     }

picoquic/logwriter.c

@@ -1059,7 +1059,7 @@ void binlog_new_connection(picoquic_cnx_t * cnx)
 
         bytewrite_int8(msg, cnx->client_mode != 0);
         bytewrite_int32(msg, cnx->proposed_version);
-        bytewrite_cid(msg, &cnx->path[0]->p_remote_cnxid->cnx_id);
+        bytewrite_cid(msg, &cnx->path[0]->first_tuple->p_remote_cnxid->cnx_id);
 
         /* Algorithms used */
         bytewrite_cstr(msg, cnx->congestion_alg->congestion_algorithm_id);

picoquic/loss_recovery.c

@@ -468,12 +468,12 @@ static size_t picoquic_retransmit_needed_packet(picoquic_cnx_t* cnx, picoquic_pa
                                 }
                             }
                             if (!all_paths_dubious) {
-                                old_path->challenge_failed = 1;
+                                old_path->first_tuple->challenge_failed = 1;
                                 cnx->path_demotion_needed = 1;
                             }
                         }
                         else {
-                            old_path->challenge_failed = 1;
+                            old_path->first_tuple->challenge_failed = 1;
                             cnx->path_demotion_needed = 1;
                         }
                     }
@@ -899,7 +899,7 @@ static void picoquic_count_and_notify_loss(
     if (timer_based_retransmit < 2) {
         picoquic_log_packet_lost(cnx, old_p->send_path, old_p->ptype, old_p->sequence_number,
             (timer_based_retransmit) ? "timer" : "repeat",
-            (old_p->send_path == NULL || old_p->send_path->p_remote_cnxid == NULL) ? NULL : &old_p->send_path->p_remote_cnxid->cnx_id,
+            (old_p->send_path == NULL || old_p->send_path->first_tuple->p_remote_cnxid == NULL) ? NULL : &old_p->send_path->first_tuple->p_remote_cnxid->cnx_id,
             old_p->length, current_time);
 
         if (!old_p->is_preemptive_repeat) {