Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ACK and Stream are not path probing #1588

Merged
merged 3 commits into from
Dec 2, 2023
Merged

ACK and Stream are not path probing #1588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
acc5805
ACK and Stream are not path probing
huitema Dec 2, 2023
090b816
Fix comment probing vs validating
huitema Dec 2, 2023
4cb0e3c
Fix validating/probing in another comment
huitema Dec 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions picoquic/frames.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5263,15 +5263,15 @@ 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 probing frame received */
picoquic_packet_context_enum pc = picoquic_context_from_epoch(epoch);
picoquic_packet_data_t packet_data;

memset(&packet_data, 0, sizeof(packet_data));

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) {
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
}

Expand Down
2 changes: 1 addition & 1 deletion picoquic/picoquic_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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*/
Expand Down
4 changes: 2 additions & 2 deletions picoquic/sender.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4125,13 +4125,13 @@ 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
* 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 */
Expand Down
Loading