From acc5805132f2e27f90f600d1162f2f0da192611d Mon Sep 17 00:00:00 2001 From: Christian Huitema Date: Sat, 2 Dec 2023 12:56:18 -0800 Subject: [PATCH 1/3] ACK and Stream are not path probing --- picoquic/frames.c | 18 +++++++++--------- picoquic/picoquic_internal.h | 2 +- picoquic/sender.c | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/picoquic/frames.c b/picoquic/frames.c index 9c46adff0..3447d9372 100644 --- a/picoquic/frames.c +++ b/picoquic/frames.c @@ -5263,7 +5263,7 @@ int picoquic_decode_frames(picoquic_cnx_t* cnx, picoquic_path_t * path_x, const { const uint8_t *bytes_max = bytes + bytes_maxsize; int ack_needed = 0; - int is_path_validating_packet = 1; /* Will be set to zero if non validating frame received */ + int is_path_probing_packet = 1; /* Will be set to zero if non validating frame received */ picoquic_packet_context_enum pc = picoquic_context_from_epoch(epoch); picoquic_packet_data_t packet_data; @@ -5271,7 +5271,7 @@ int picoquic_decode_frames(picoquic_cnx_t* cnx, picoquic_path_t * path_x, const while (bytes != NULL && bytes < bytes_max) { uint8_t first_byte = bytes[0]; - int is_path_validating_frame = 0; + int is_path_probing_frame = 0; if (PICOQUIC_IN_RANGE(first_byte, picoquic_frame_type_stream_range_min, picoquic_frame_type_stream_range_max)) { if (epoch != picoquic_epoch_0rtt && epoch != picoquic_epoch_1rtt) { @@ -5330,7 +5330,7 @@ int picoquic_decode_frames(picoquic_cnx_t* cnx, picoquic_path_t * path_x, const else { switch (first_byte) { case picoquic_frame_type_padding: - is_path_validating_frame = 1; + is_path_probing_frame = 1; bytes = picoquic_skip_0len_frame(bytes, bytes_max); break; case picoquic_frame_type_reset_stream: @@ -5376,7 +5376,7 @@ int picoquic_decode_frames(picoquic_cnx_t* cnx, picoquic_path_t * path_x, const ack_needed = 1; break; case picoquic_frame_type_new_connection_id: - is_path_validating_frame = 1; + is_path_probing_frame = 1; bytes = picoquic_decode_new_connection_id_frame(cnx, bytes, bytes_max, current_time); ack_needed = 1; break; @@ -5385,12 +5385,12 @@ int picoquic_decode_frames(picoquic_cnx_t* cnx, picoquic_path_t * path_x, const ack_needed = 1; break; case picoquic_frame_type_path_challenge: - is_path_validating_frame = 1; + is_path_probing_frame = 1; bytes = picoquic_decode_path_challenge_frame(cnx, bytes, bytes_max, (path_is_not_allocated)?NULL:path_x, addr_from, addr_to); break; case picoquic_frame_type_path_response: - is_path_validating_frame = 1; + is_path_probing_frame = 1; bytes = picoquic_decode_path_response_frame(cnx, bytes, bytes_max, (path_is_not_allocated) ? NULL : path_x, current_time); break; @@ -5496,8 +5496,8 @@ int picoquic_decode_frames(picoquic_cnx_t* cnx, picoquic_path_t * path_x, const break; } } - is_path_validating_packet &= is_path_validating_frame; } + is_path_probing_packet &= is_path_probing_frame; } if (bytes != NULL) { @@ -5508,8 +5508,8 @@ int picoquic_decode_frames(picoquic_cnx_t* cnx, picoquic_path_t * path_x, const picoquic_set_ack_needed(cnx, current_time, pc, path_x->p_local_cnxid, 0); } - if (epoch == picoquic_epoch_1rtt && !is_path_validating_packet && pn64 > path_x->last_non_validating_pn) { - path_x->last_non_validating_pn = pn64; + if (epoch == picoquic_epoch_1rtt && !is_path_probing_packet && pn64 > path_x->last_non_path_probing_pn) { + path_x->last_non_path_probing_pn = pn64; } } diff --git a/picoquic/picoquic_internal.h b/picoquic/picoquic_internal.h index c82e6bfc7..2d4b0af0a 100644 --- a/picoquic/picoquic_internal.h +++ b/picoquic/picoquic_internal.h @@ -1001,7 +1001,7 @@ typedef struct st_picoquic_path_t { uint64_t demotion_time; uint64_t challenge_time_first; uint8_t challenge_repeat_count; - uint64_t last_non_validating_pn; + uint64_t last_non_path_probing_pn; /* Last time a packet was sent on this path. */ uint64_t last_sent_time; /* Number of packets sent on this path*/ diff --git a/picoquic/sender.c b/picoquic/sender.c index de39e9b83..3a490db6c 100644 --- a/picoquic/sender.c +++ b/picoquic/sender.c @@ -4131,7 +4131,7 @@ static int picoquic_select_next_path(picoquic_cnx_t * cnx, uint64_t current_time * verify that only the "most recent" packets trigger the validation * logic. */ - if (cnx->client_mode || cnx->path[i]->last_non_validating_pn >= + if (cnx->client_mode || cnx->path[i]->last_non_path_probing_pn >= picoquic_sack_list_last(&cnx->ack_ctx[picoquic_packet_context_application].sack_list) || cnx->path[i]->is_nat_challenge) { /* This path becomes the new default */ From 090b8160b0c0d15eff2011576ee9ccff16dca882 Mon Sep 17 00:00:00 2001 From: Christian Huitema Date: Sat, 2 Dec 2023 13:09:37 -0800 Subject: [PATCH 2/3] Fix comment probing vs validating --- picoquic/sender.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/picoquic/sender.c b/picoquic/sender.c index 3a490db6c..a3141f740 100644 --- a/picoquic/sender.c +++ b/picoquic/sender.c @@ -4125,7 +4125,7 @@ static int picoquic_select_next_path(picoquic_cnx_t * cnx, uint64_t current_time * 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 * as soon as confirmed. On the server side, this is enforced by observing - * incoming traffic: if a path is validated and "non path validating" + * incoming traffic: if a path is validated and "non path probing" * frames were received, then the path should be promoted. However, on * the server side, we have to be careful with packet reordering, and * verify that only the "most recent" packets trigger the validation From 4cb0e3ccfeffb7b21f94821959e1adbf46cd2599 Mon Sep 17 00:00:00 2001 From: Christian Huitema Date: Sat, 2 Dec 2023 14:02:22 -0800 Subject: [PATCH 3/3] Fix validating/probing in another comment --- picoquic/frames.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/picoquic/frames.c b/picoquic/frames.c index 3447d9372..254325279 100644 --- a/picoquic/frames.c +++ b/picoquic/frames.c @@ -5263,7 +5263,7 @@ int picoquic_decode_frames(picoquic_cnx_t* cnx, picoquic_path_t * path_x, const { const uint8_t *bytes_max = bytes + bytes_maxsize; int ack_needed = 0; - int is_path_probing_packet = 1; /* Will be set to zero if non validating frame received */ + int is_path_probing_packet = 1; /* Will be set to zero if non probing frame received */ picoquic_packet_context_enum pc = picoquic_context_from_epoch(epoch); picoquic_packet_data_t packet_data;