From 6322741cf62ecac7901ce3f2c66b2fc470a98f84 Mon Sep 17 00:00:00 2001
From: huitema <huitema@huitema.net>
Date: Sun, 9 Dec 2018 14:27:02 -0800
Subject: [PATCH] crashes and hot loops on migration
---
picoquic/frames.c | 3 ++-
picoquic/quicctx.c | 7 +++++++
picoquic/sender.c | 39 +++++++++++++++++++++---------------
picoquicfirst/picoquicdemo.c | 6 +++++-
4 files changed, 37 insertions(+), 18 deletions(-)
diff --git a/picoquic/frames.c b/picoquic/frames.c
index 232193e9..11a22732 100644
--- a/picoquic/frames.c
+++ b/picoquic/frames.c
@@ -1646,7 +1646,8 @@ static int picoquic_process_ack_range(
picoquic_packet_t* next = p->next_packet;
picoquic_path_t * old_path = p->send_path;
- if (old_path != NULL) {
+ /* TODO: understand why we need this check, instead of just a check for NULL */
+ if (old_path == cnx->path[0]) {
if (cnx->congestion_alg != NULL) {
cnx->congestion_alg->alg_notify(old_path,
picoquic_congestion_notification_acknowledgement,
diff --git a/picoquic/quicctx.c b/picoquic/quicctx.c
index 323a1380..0ba74a5e 100644
--- a/picoquic/quicctx.c
+++ b/picoquic/quicctx.c
@@ -819,6 +819,11 @@ void picoquic_delete_path(picoquic_cnx_t* cnx, int path_index)
{
picoquic_path_t * path_x = cnx->path[path_index];
picoquic_packet_t* p = NULL;
+
+ DBG_PRINTF("delete path[%d] (%x)\n", path_index, path_x);
+ if (cnx->quic->F_log != NULL) {
+ fflush(cnx->quic->F_log);
+ }
/* Remove old path data from retransmit queue */
for (picoquic_packet_context_enum pc = 0; pc < picoquic_nb_packet_context; pc++)
@@ -827,6 +832,7 @@ void picoquic_delete_path(picoquic_cnx_t* cnx, int path_index)
while (p != NULL) {
if (p->send_path == path_x) {
+ DBG_PRINTF("Erase path for packet pc: %d, seq:%d\n", pc, p->sequence_number);
p->send_path = NULL;
}
p = p->next_packet;
@@ -835,6 +841,7 @@ void picoquic_delete_path(picoquic_cnx_t* cnx, int path_index)
p = cnx->pkt_ctx[pc].retransmitted_newest;
while (p != NULL) {
if (p->send_path == path_x) {
+ DBG_PRINTF("Erase path for old packet pc: %d, seq:%d\n", pc, p->sequence_number);
p->send_path = NULL;
}
p = p->next_packet;
diff --git a/picoquic/sender.c b/picoquic/sender.c
index 5a68149f..366c8d9b 100644
--- a/picoquic/sender.c
+++ b/picoquic/sender.c
@@ -1017,7 +1017,8 @@ int picoquic_retransmit_needed(picoquic_cnx_t* cnx,
packet->length = length;
cnx->nb_retransmission_total++;
- if (cnx->congestion_alg != NULL && old_path != NULL) {
+ /* TODO: understand why we need to check for exact path value, instead of NULL */
+ if (cnx->congestion_alg != NULL && old_path == cnx->path[0]) {
cnx->congestion_alg->alg_notify(old_path,
(timer_based_retransmit == 0) ? picoquic_congestion_notification_repeat : picoquic_congestion_notification_timeout,
0, 0, lost_packet_number, current_time);
@@ -1151,21 +1152,23 @@ static uint64_t picoquic_get_challenge_wake_time(picoquic_cnx_t* cnx, uint64_t c
/* Consider demotions */
for (int i = 0; next_wake_time > current_time && i < cnx->nb_paths; i++) {
- if (cnx->path[i]->response_required) {
- next_wake_time = current_time;
- break;
- }
-
- if (cnx->path[i]->path_is_demoted &&
- cnx->path[i]->demotion_time < next_wake_time) {
- next_wake_time = cnx->path[i]->demotion_time;
+ if (cnx->path[i]->path_is_demoted) {
+ if (cnx->path[i]->demotion_time < next_wake_time) {
+ next_wake_time = cnx->path[i]->demotion_time;
+ }
}
+ else {
+ if (cnx->path[i]->response_required) {
+ next_wake_time = current_time;
+ break;
+ }
- if (cnx->path[i]->challenge_verified == 0 && cnx->path[i]->path_is_activated) {
- uint64_t next_challenge_time = cnx->path[i]->challenge_time + cnx->path[i]->retransmit_timer;
+ if (cnx->path[i]->challenge_verified == 0 && cnx->path[i]->path_is_activated) {
+ uint64_t next_challenge_time = cnx->path[i]->challenge_time + cnx->path[i]->retransmit_timer;
- if (next_challenge_time < next_wake_time) {
- next_wake_time = next_challenge_time;
+ if (next_challenge_time < next_wake_time) {
+ next_wake_time = next_challenge_time;
+ }
}
}
}
@@ -2353,6 +2356,7 @@ int picoquic_prepare_packet_ready(picoquic_cnx_t* cnx, picoquic_path_t * path_x,
&& path_x->cwin > path_x->bytes_in_transit && picoquic_is_mtu_probe_needed(cnx, path_x)) {
length = picoquic_prepare_mtu_probe(cnx, path_x, header_length, checksum_overhead, bytes);
packet->length = length;
+ packet->send_path = path_x;
path_x->mtu_probe_sent = 1;
is_pure_ack = 0;
}
@@ -2484,7 +2488,7 @@ int picoquic_prepare_segment(picoquic_cnx_t* cnx, picoquic_path_t * path_x, pico
/* Prepare next probe if one is needed, returns send_length == 0 if none necessary */
int picoquic_prepare_probe(picoquic_cnx_t* cnx,
uint64_t current_time, uint8_t* send_buffer, size_t send_buffer_max, size_t* send_length,
- struct sockaddr ** p_addr_to, int * to_len, struct sockaddr ** p_addr_from, int * from_len)
+ struct sockaddr ** p_addr_to, int * to_len, struct sockaddr ** p_addr_from, int * from_len, struct sockaddr ** addr_to_log)
{
int ret = 0;
@@ -2544,6 +2548,7 @@ int picoquic_prepare_probe(picoquic_cnx_t* cnx,
{
if (!cnx->path[i]->path_is_activated) {
inactive_path_index = i;
+ break;
}
}
@@ -2586,7 +2591,9 @@ int picoquic_prepare_probe(picoquic_cnx_t* cnx,
*p_addr_to = (struct sockaddr*)&probe->peer_addr;
*to_len = probe->peer_addr_len;
}
-
+ /* Remember the log address */
+ *addr_to_log = (struct sockaddr*)&probe->peer_addr;
+ /* Set the source address */
if (p_addr_from != NULL) {
*p_addr_from = (struct sockaddr*)&probe->local_addr;
*from_len = probe->local_addr_len;
@@ -2628,7 +2635,7 @@ int picoquic_prepare_packet(picoquic_cnx_t* cnx,
/* If probes are in waiting, send the first one */
ret = picoquic_prepare_probe(cnx, current_time, send_buffer, send_buffer_max, send_length,
- p_addr_to, to_len, p_addr_from, from_len);
+ p_addr_to, to_len, p_addr_from, from_len, &addr_to_log);
if (ret == 0 && *send_length == 0) {
/* Select the path */
diff --git a/picoquicfirst/picoquicdemo.c b/picoquicfirst/picoquicdemo.c
index eec66aa5..c44037f2 100644
--- a/picoquicfirst/picoquicdemo.c
+++ b/picoquicfirst/picoquicdemo.c
@@ -148,6 +148,10 @@ static void picoquic_set_key_log_file_from_env(picoquic_quic_t* quic)
size_t len;
errno_t err = _dupenv_s(&keylog_filename, &len, "SSLKEYLOGFILE");
+ if (keylog_filename == NULL) {
+ return;
+ }
+
if (err == 0) {
err = fopen_s(&F, keylog_filename, "a");
@@ -1505,7 +1509,7 @@ int main(int argc, char** argv)
if (server_key_file == NULL &&
picoquic_get_input_path(default_server_key_file, sizeof(default_server_key_file), solution_dir, SERVER_KEY_FILE) == 0) {
- server_key_file = default_server_cert_file;
+ server_key_file = default_server_key_file;
}
/* Run as server */
--
GitLab