Skip to content

Commit

Permalink
Merge pull request #1588 from private-octopus/ack-ecn-non-validating
Browse files Browse the repository at this point in the history
ACK and Stream are not path probing
  • Loading branch information
huitema authored Dec 2, 2023
2 parents 70aaa41 + 4cb0e3c commit 93af56f
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 12 deletions.
18 changes: 9 additions & 9 deletions picoquic/frames.c
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
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
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

0 comments on commit 93af56f

Please sign in to comment.