From 571a59cba90110af9eebb4c7f08ea0daee9d3739 Mon Sep 17 00:00:00 2001
From: huitema <huitema@huitema.net>
Date: Fri, 23 Nov 2018 13:29:55 -0800
Subject: [PATCH] Revise the RACK logic
---
picoquic/picoquic.h | 1 +
picoquic/sender.c | 78 ++++++++++++++++++++++-----------------------
2 files changed, 40 insertions(+), 39 deletions(-)
diff --git a/picoquic/picoquic.h b/picoquic/picoquic.h
index 00ad2c27..1cc891ea 100644
--- a/picoquic/picoquic.h
+++ b/picoquic/picoquic.h
@@ -381,6 +381,7 @@ picoquic_cnx_t* picoquic_get_earliest_cnx_to_wake(picoquic_quic_t* quic, uint64_
picoquic_state_enum picoquic_get_cnx_state(picoquic_cnx_t* cnx);
int picoquic_tls_is_psk_handshake(picoquic_cnx_t* cnx);
+int picoquic_tls_is_early_data_skipped(picoquic_cnx_t* cnx);
void picoquic_get_peer_addr(picoquic_cnx_t* cnx, struct sockaddr** addr, int* addr_len);
void picoquic_get_local_addr(picoquic_cnx_t* cnx, struct sockaddr** addr, int* addr_len);
diff --git a/picoquic/sender.c b/picoquic/sender.c
index e763fa38..76f72070 100644
--- a/picoquic/sender.c
+++ b/picoquic/sender.c
@@ -690,51 +690,51 @@ static int picoquic_retransmit_needed_by_packet(picoquic_cnx_t* cnx,
picoquic_packet_t* p, uint64_t current_time, uint64_t * next_retransmit_time, int* timer_based)
{
picoquic_packet_context_enum pc = p->pc;
+ uint64_t retransmit_time;
int64_t delta_seq = cnx->pkt_ctx[pc].highest_acknowledged - p->sequence_number;
int should_retransmit = 0;
-
- if (delta_seq > 3) {
- /*
- * SACK Logic.
- * more than N packets were seen at the receiver after this one.
- */
- should_retransmit = 1;
- }
- else {
- if (p->ptype != picoquic_packet_0rtt_protected &&
- delta_seq > 0 &&
- p->send_time <= cnx->pkt_ctx[pc].latest_time_acknowledged) {
- uint64_t delta_t = (cnx->pkt_ctx[pc].latest_time_acknowledged - p->send_time) +
- (current_time - cnx->pkt_ctx[pc].highest_acknowledged_time);
-
- /* TODO: out of order delivery time ought to be dynamic */
- if (delta_t >= PICOQUIC_RACK_DELAY) {
- should_retransmit = 1;
- } else {
- uint64_t next_rack_time = current_time + (PICOQUIC_RACK_DELAY - delta_t);
- if (next_rack_time < *next_retransmit_time) {
- *next_retransmit_time = next_rack_time;
- }
+ int is_timer_based = 0;
+
+ if (delta_seq > 0) {
+ /* By default, we use timer based RACK logic to absorb out of order deliveries */
+ retransmit_time = p->send_time + cnx->path[0]->smoothed_rtt + (cnx->path[0]->smoothed_rtt >> 3);
+
+ /* RACK logic fails when the smoothed RTT is too small, in which case we
+ * rely on dupack logic possible, or on a safe estimate of the RACK delay if it
+ * is not */
+ if (delta_seq < 3) {
+ uint64_t rack_timer_min = cnx->pkt_ctx[pc].latest_time_acknowledged + PICOQUIC_RACK_DELAY;
+ if (retransmit_time < rack_timer_min) {
+ retransmit_time = rack_timer_min;
}
}
+ }
+ else
+ {
+ /* There has not been any higher packet acknowledged, thus we fall back on timer logic. */
+ uint64_t rto = (cnx->pkt_ctx[pc].nb_retransmit == 0) ?
+ cnx->path[0]->retransmit_timer : (1000000ull << (cnx->pkt_ctx[pc].nb_retransmit - 1));
+ retransmit_time = p->send_time + rto;
+ is_timer_based = 1;
+ }
- if (should_retransmit == 0) {
- /* Don't fire yet, because of possible out of order delivery */
- int64_t time_out = current_time - p->send_time;
- uint64_t retransmit_timer = (cnx->pkt_ctx[pc].nb_retransmit == 0) ?
- cnx->path[0]->retransmit_timer : (1000000ull << (cnx->pkt_ctx[pc].nb_retransmit - 1));
-
- if ((uint64_t)time_out < retransmit_timer) {
- /* Do not retransmit if the timer has not yet elapsed */
- should_retransmit = 0;
- if (current_time + retransmit_timer < *next_retransmit_time) {
- *next_retransmit_time = current_time + retransmit_timer;
- }
- } else {
- should_retransmit = 1;
- *timer_based = 1;
- }
+ if (p->ptype == picoquic_packet_0rtt_protected) {
+ /* Special case for 0RTT packets */
+ if (cnx->cnx_state != picoquic_state_ready &&
+ cnx->cnx_state != picoquic_state_client_ready_start) {
+ /* Set the retransmit time ahead of current time since the connection is not ready */
+ retransmit_time = current_time + cnx->path[0]->smoothed_rtt + PICOQUIC_RACK_DELAY;
}
+ /* TODO: if early data was skipped by the server, we should retransmit
+ * immediately. However, there is not good API to do that */
+ }
+
+ if (current_time >= retransmit_time) {
+ should_retransmit = 1;
+ *timer_based = is_timer_based;
+ } else {
+ *next_retransmit_time = retransmit_time;
+ *timer_based = 0;
}
return should_retransmit;
--
GitLab