diff --git a/lib/ngtcp2_conn.c b/lib/ngtcp2_conn.c
index db9031cdde2a6a0de0b2968c198e1f6082508d6b..741995016f867a924c441721d60d93305930ae10 100644
--- a/lib/ngtcp2_conn.c
+++ b/lib/ngtcp2_conn.c
@@ -1280,11 +1280,10 @@ static int conn_on_pkt_sent(ngtcp2_conn *conn, ngtcp2_rtb *rtb,
if (ent->flags & NGTCP2_RTB_FLAG_ACK_ELICITING) {
conn->cstat.last_tx_pkt_ts[rtb->pktns_id] = ent->ts;
- /* I think pseudo code is wrong; timer should be set when
- ack-eliciting packet is sent. */
- ngtcp2_conn_set_loss_detection_timer(conn, ent->ts);
}
+ ngtcp2_conn_set_loss_detection_timer(conn, ent->ts);
+
return 0;
}
@@ -1934,11 +1933,12 @@ build_pkt:
pfrc = &(*pfrc)->next;
pkt_empty = 0;
- rtb_entry_flags |= NGTCP2_RTB_FLAG_ACK_ELICITING;
+ rtb_entry_flags |=
+ NGTCP2_RTB_FLAG_ACK_ELICITING | NGTCP2_RTB_FLAG_RETRANSMITTABLE;
}
if (!(rtb_entry_flags & NGTCP2_RTB_FLAG_ACK_ELICITING) &&
- pktns->rtb.probe_pkt_left) {
+ pktns->rtb.num_retransmittable && pktns->rtb.probe_pkt_left) {
num_reclaimed = ngtcp2_rtb_reclaim_on_pto(&pktns->rtb, conn, pktns,
pktns->rtb.probe_pkt_left + 1);
if (num_reclaimed < 0) {
@@ -1948,6 +1948,19 @@ build_pkt:
if (num_reclaimed) {
goto build_pkt;
}
+ /* We had pktns->rtb.num_retransmittable > 0 but the contents of
+ those packets have been acknowledged (i.e., retransmission in
+ another packet). For server, in this case, we don't have to
+ send any probe packet. Client needs to send probe packets
+ until it knows that server has completed address validation or
+ handshake has been confirmed. */
+ if (pktns->rtb.num_retransmittable == 0 &&
+ (conn->server ||
+ (conn->flags & (NGTCP2_CONN_FLAG_SERVER_ADDR_VERIFIED |
+ NGTCP2_CONN_FLAG_HANDSHAKE_CONFIRMED)))) {
+ pktns->rtb.probe_pkt_left = 0;
+ ngtcp2_conn_set_loss_detection_timer(conn, ts);
+ }
}
/* Don't send any PING frame if client Initial has not been
@@ -2807,7 +2820,8 @@ static ngtcp2_ssize conn_write_pkt(ngtcp2_conn *conn, uint8_t *dest,
}
pkt_empty = 0;
- rtb_entry_flags |= NGTCP2_RTB_FLAG_ACK_ELICITING;
+ rtb_entry_flags |=
+ NGTCP2_RTB_FLAG_ACK_ELICITING | NGTCP2_RTB_FLAG_RETRANSMITTABLE;
pfrc = &(*pfrc)->next;
}
@@ -2846,7 +2860,8 @@ static ngtcp2_ssize conn_write_pkt(ngtcp2_conn *conn, uint8_t *dest,
pfrc = &(*pfrc)->next;
pkt_empty = 0;
- rtb_entry_flags |= NGTCP2_RTB_FLAG_ACK_ELICITING;
+ rtb_entry_flags |=
+ NGTCP2_RTB_FLAG_ACK_ELICITING | NGTCP2_RTB_FLAG_RETRANSMITTABLE;
}
}
@@ -2877,7 +2892,8 @@ static ngtcp2_ssize conn_write_pkt(ngtcp2_conn *conn, uint8_t *dest,
assert(NGTCP2_ERR_NOBUF == rv);
} else {
pkt_empty = 0;
- rtb_entry_flags |= NGTCP2_RTB_FLAG_ACK_ELICITING;
+ rtb_entry_flags |=
+ NGTCP2_RTB_FLAG_ACK_ELICITING | NGTCP2_RTB_FLAG_RETRANSMITTABLE;
pfrc = &(*pfrc)->next;
}
}
@@ -2908,7 +2924,8 @@ static ngtcp2_ssize conn_write_pkt(ngtcp2_conn *conn, uint8_t *dest,
assert(NGTCP2_ERR_NOBUF == rv);
} else {
pkt_empty = 0;
- rtb_entry_flags |= NGTCP2_RTB_FLAG_ACK_ELICITING;
+ rtb_entry_flags |=
+ NGTCP2_RTB_FLAG_ACK_ELICITING | NGTCP2_RTB_FLAG_RETRANSMITTABLE;
pfrc = &(*pfrc)->next;
}
}
@@ -2939,7 +2956,8 @@ static ngtcp2_ssize conn_write_pkt(ngtcp2_conn *conn, uint8_t *dest,
pkt_empty = 0;
credit_expanded = 1;
- rtb_entry_flags |= NGTCP2_RTB_FLAG_ACK_ELICITING;
+ rtb_entry_flags |=
+ NGTCP2_RTB_FLAG_ACK_ELICITING | NGTCP2_RTB_FLAG_RETRANSMITTABLE;
pfrc = &(*pfrc)->next;
strm->rx.max_offset = strm->rx.unsent_max_offset;
}
@@ -2985,7 +3003,8 @@ static ngtcp2_ssize conn_write_pkt(ngtcp2_conn *conn, uint8_t *dest,
pfrc = &(*pfrc)->next;
pkt_empty = 0;
- rtb_entry_flags |= NGTCP2_RTB_FLAG_ACK_ELICITING;
+ rtb_entry_flags |=
+ NGTCP2_RTB_FLAG_ACK_ELICITING | NGTCP2_RTB_FLAG_RETRANSMITTABLE;
if (ngtcp2_strm_streamfrq_empty(strm)) {
ngtcp2_conn_tx_strmq_pop(conn);
@@ -3004,15 +3023,25 @@ static ngtcp2_ssize conn_write_pkt(ngtcp2_conn *conn, uint8_t *dest,
if (rv != NGTCP2_ERR_NOBUF && !send_stream &&
!(rtb_entry_flags & NGTCP2_RTB_FLAG_ACK_ELICITING) &&
- pktns->tx.frq == NULL && pktns->rtb.probe_pkt_left) {
+ pktns->rtb.num_retransmittable && pktns->tx.frq == NULL &&
+ pktns->rtb.probe_pkt_left) {
num_reclaimed = ngtcp2_rtb_reclaim_on_pto(&pktns->rtb, conn, pktns,
- pktns->rtb.probe_pkt_left);
+ pktns->rtb.probe_pkt_left + 1);
if (num_reclaimed < 0) {
return rv;
}
if (num_reclaimed) {
goto build_pkt;
}
+
+ /* We had pktns->rtb.num_retransmittable > 0 but the contents of
+ those packets have been acknowledged (i.e., retransmission in
+ another packet). In this case, we don't have to send any
+ probe packet. */
+ if (pktns->rtb.num_retransmittable == 0) {
+ pktns->rtb.probe_pkt_left = 0;
+ ngtcp2_conn_set_loss_detection_timer(conn, ts);
+ }
}
/* Add ACK if MAX_DATA or MAX_STREAM_DATA frame is encoded to
@@ -3079,7 +3108,8 @@ static ngtcp2_ssize conn_write_pkt(ngtcp2_conn *conn, uint8_t *dest,
pfrc = &(*pfrc)->next;
pkt_empty = 0;
- rtb_entry_flags |= NGTCP2_RTB_FLAG_ACK_ELICITING;
+ rtb_entry_flags |=
+ NGTCP2_RTB_FLAG_ACK_ELICITING | NGTCP2_RTB_FLAG_RETRANSMITTABLE;
data_strm->tx.offset += ndatalen;
conn->tx.offset += ndatalen;
@@ -3121,8 +3151,7 @@ static ngtcp2_ssize conn_write_pkt(ngtcp2_conn *conn, uint8_t *dest,
}
if (!(rtb_entry_flags & NGTCP2_RTB_FLAG_ACK_ELICITING)) {
- if (pktns->rtb.probe_pkt_left ||
- pktns->tx.num_non_ack_pkt >= NGTCP2_MAX_NON_ACK_TX_PKT) {
+ if (pktns->tx.num_non_ack_pkt >= NGTCP2_MAX_NON_ACK_TX_PKT) {
lfr.type = NGTCP2_FRAME_PING;
rv = conn_ppe_write_frame_hd_log(conn, ppe, &hd_logged, hd, &lfr);
@@ -3132,9 +3161,6 @@ static ngtcp2_ssize conn_write_pkt(ngtcp2_conn *conn, uint8_t *dest,
packet is still empty. */
} else {
rtb_entry_flags |= NGTCP2_RTB_FLAG_ACK_ELICITING;
- if (pktns->rtb.probe_pkt_left) {
- rtb_entry_flags |= NGTCP2_RTB_FLAG_PROBE;
- }
pktns->tx.num_non_ack_pkt = 0;
}
} else {
@@ -3173,6 +3199,8 @@ static ngtcp2_ssize conn_write_pkt(ngtcp2_conn *conn, uint8_t *dest,
ngtcp2_qlog_pkt_sent_end(&conn->qlog, hd, (size_t)nwrite);
+ /* TODO ack-eliciting vs needs-tracking */
+ /* probe packet needs tracking but it does not need ACK, could be lost. */
if ((rtb_entry_flags & NGTCP2_RTB_FLAG_ACK_ELICITING) || padded) {
rv = ngtcp2_rtb_entry_new(&ent, hd, NULL, ts, (size_t)nwrite,
rtb_entry_flags, conn->mem);
@@ -3390,9 +3418,9 @@ static int conn_handshake_remnants_left(ngtcp2_conn *conn) {
ngtcp2_pktns *hs_pktns = conn->hs_pktns;
return !(conn->flags & NGTCP2_CONN_FLAG_HANDSHAKE_COMPLETED) ||
- (in_pktns && (in_pktns->rtb.num_ack_eliciting ||
+ (in_pktns && (in_pktns->rtb.num_retransmittable ||
ngtcp2_ksl_len(&in_pktns->crypto.tx.frq))) ||
- (hs_pktns && (hs_pktns->rtb.num_ack_eliciting ||
+ (hs_pktns && (hs_pktns->rtb.num_retransmittable ||
ngtcp2_ksl_len(&hs_pktns->crypto.tx.frq)));
}
@@ -9367,7 +9395,7 @@ static ngtcp2_pktns *conn_get_earliest_pktns(ngtcp2_conn *conn,
ngtcp2_tstamp earliest_ts = times[NGTCP2_PKTNS_ID_INITIAL];
for (i = NGTCP2_PKTNS_ID_HANDSHAKE; i < NGTCP2_PKTNS_ID_MAX; ++i) {
- if (ns[i] == NULL || ns[i]->rtb.num_ack_eliciting == 0 ||
+ if (ns[i] == NULL || ns[i]->rtb.num_retransmittable == 0 ||
(times[i] == UINT64_MAX ||
(earliest_ts != UINT64_MAX && times[i] >= earliest_ts) ||
(i == NGTCP2_PKTNS_ID_APP &&
@@ -9416,9 +9444,9 @@ void ngtcp2_conn_set_loss_detection_timer(ngtcp2_conn *conn, ngtcp2_tstamp ts) {
return;
}
- if ((!in_pktns || in_pktns->rtb.num_ack_eliciting == 0) &&
- (!hs_pktns || hs_pktns->rtb.num_ack_eliciting == 0) &&
- (pktns->rtb.num_ack_eliciting == 0 ||
+ if ((!in_pktns || in_pktns->rtb.num_retransmittable == 0) &&
+ (!hs_pktns || hs_pktns->rtb.num_retransmittable == 0) &&
+ (pktns->rtb.num_retransmittable == 0 ||
!(conn->flags & NGTCP2_CONN_FLAG_HANDSHAKE_COMPLETED)) &&
(conn->server ||
(conn->flags & (NGTCP2_CONN_FLAG_SERVER_ADDR_VERIFIED |
diff --git a/lib/ngtcp2_conn.h b/lib/ngtcp2_conn.h
index 498604969a472262a57ef36fd1c7f8d986691745..53f9c4d80ce86b7ac795081748ec1d4f210247df 100644
--- a/lib/ngtcp2_conn.h
+++ b/lib/ngtcp2_conn.h
@@ -106,7 +106,7 @@ typedef enum {
/* NGTCP2_MAX_NON_ACK_TX_PKT is the maximum number of continuous non
ACK-eliciting packets. */
-#define NGTCP2_MAX_NON_ACK_TX_PKT 10
+#define NGTCP2_MAX_NON_ACK_TX_PKT 3
/*
* ngtcp2_max_frame is defined so that it covers the largest ACK
diff --git a/lib/ngtcp2_rtb.c b/lib/ngtcp2_rtb.c
index d22570974514a4b8f962debf6694a4c1f9b211db..83308e8e88c4dbf9b19972e271f00cb00ab73b55 100644
--- a/lib/ngtcp2_rtb.c
+++ b/lib/ngtcp2_rtb.c
@@ -236,6 +236,7 @@ void ngtcp2_rtb_init(ngtcp2_rtb *rtb, ngtcp2_pktns_id pktns_id,
rtb->mem = mem;
rtb->largest_acked_tx_pkt_num = -1;
rtb->num_ack_eliciting = 0;
+ rtb->num_retransmittable = 0;
rtb->probe_pkt_left = 0;
rtb->pktns_id = pktns_id;
rtb->cc_pkt_num = 0;
@@ -274,6 +275,9 @@ static void rtb_on_add(ngtcp2_rtb *rtb, ngtcp2_rtb_entry *ent,
if (ent->flags & NGTCP2_RTB_FLAG_ACK_ELICITING) {
++rtb->num_ack_eliciting;
}
+ if (ent->flags & NGTCP2_RTB_FLAG_RETRANSMITTABLE) {
+ ++rtb->num_retransmittable;
+ }
}
static void rtb_on_remove(ngtcp2_rtb *rtb, ngtcp2_rtb_entry *ent,
@@ -282,12 +286,17 @@ static void rtb_on_remove(ngtcp2_rtb *rtb, ngtcp2_rtb_entry *ent,
return;
}
- if ((ent->flags & NGTCP2_RTB_FLAG_ACK_ELICITING) &&
- !(ent->flags & NGTCP2_RTB_FLAG_PTO_RECLAIMED)) {
+ if (ent->flags & NGTCP2_RTB_FLAG_ACK_ELICITING) {
assert(rtb->num_ack_eliciting);
--rtb->num_ack_eliciting;
}
+ if ((ent->flags & NGTCP2_RTB_FLAG_RETRANSMITTABLE) &&
+ !(ent->flags & NGTCP2_RTB_FLAG_PTO_RECLAIMED)) {
+ assert(rtb->num_retransmittable);
+ --rtb->num_retransmittable;
+ }
+
if (rtb->cc_pkt_num <= ent->hd.pkt_num) {
assert(cstat->bytes_in_flight >= ent->pktlen);
cstat->bytes_in_flight -= ent->pktlen;
@@ -1118,10 +1127,12 @@ ngtcp2_ssize ngtcp2_rtb_reclaim_on_pto(ngtcp2_rtb *rtb, ngtcp2_conn *conn,
if ((ent->flags & (NGTCP2_RTB_FLAG_LOST_RETRANSMITTED |
NGTCP2_RTB_FLAG_PTO_RECLAIMED)) ||
- ent->frc == NULL) {
+ !(ent->flags & NGTCP2_RTB_FLAG_RETRANSMITTABLE)) {
continue;
}
+ assert(ent->frc);
+
rv = rtb_reclaim_frame(rtb, &reclaimed, conn, pktns, ent);
if (rv != 0) {
return rv;
@@ -1131,8 +1142,8 @@ ngtcp2_ssize ngtcp2_rtb_reclaim_on_pto(ngtcp2_rtb *rtb, ngtcp2_conn *conn,
the next run. */
ent->flags |= NGTCP2_RTB_FLAG_PTO_RECLAIMED;
- assert(rtb->num_ack_eliciting);
- --rtb->num_ack_eliciting;
+ assert(rtb->num_retransmittable);
+ --rtb->num_retransmittable;
if (reclaimed) {
--num_pkts;
diff --git a/lib/ngtcp2_rtb.h b/lib/ngtcp2_rtb.h
index bfd45fe6a1e6473aa2a5e7015fdc5c6e6aade24e..0d0b738f396a2393511bc21b56813058002829df 100644
--- a/lib/ngtcp2_rtb.h
+++ b/lib/ngtcp2_rtb.h
@@ -176,6 +176,13 @@ typedef enum {
/* NGTCP2_RTB_FLAG_PROBE indicates that the entry includes a probe
packet. */
NGTCP2_RTB_FLAG_PROBE = 0x01,
+ /* NGTCP2_RTB_FLAG_RETRANSMITTABLE indicates that the entry includes
+ a frame which must be retransmitted until it is acknowledged. In
+ most cases, this flag is used along with
+ NGTCP2_RTB_FLAG_ACK_ELICITING. We have these 2 flags because
+ NGTCP2_RTB_FLAG_RETRANSMITTABLE triggers PTO, but just
+ NGTCP2_RTB_FLAG_ACK_ELICITING does not. */
+ NGTCP2_RTB_FLAG_RETRANSMITTABLE = 0x02,
/* NGTCP2_RTB_FLAG_ACK_ELICITING indicates that the entry elicits
acknowledgement. */
NGTCP2_RTB_FLAG_ACK_ELICITING = 0x04,
@@ -262,6 +269,9 @@ typedef struct {
int64_t largest_acked_tx_pkt_num;
/* num_ack_eliciting is the number of ACK eliciting entries. */
size_t num_ack_eliciting;
+ /* num_retransmittable is the number of packets which contain frames
+ that must be retransmitted on loss. */
+ size_t num_retransmittable;
/* probe_pkt_left is the number of probe packet to send */
size_t probe_pkt_left;
/* pktns_id is the identifier of packet number space. */
diff --git a/tests/ngtcp2_conn_test.c b/tests/ngtcp2_conn_test.c
index 2d6ee372f1e202ac273a28ce101023c515ef5ae0..bf347f9d24f91e784caa01bfe943b305bc93ab1a 100644
--- a/tests/ngtcp2_conn_test.c
+++ b/tests/ngtcp2_conn_test.c
@@ -4796,7 +4796,8 @@ void test_ngtcp2_conn_handshake_probe(void) {
spktlen = ngtcp2_conn_write_pkt(conn, NULL, buf, sizeof(buf), ++t);
CU_ASSERT(spktlen > 0);
- CU_ASSERT(1 == conn->in_pktns->rtb.num_ack_eliciting);
+ CU_ASSERT(1 == conn->in_pktns->rtb.num_retransmittable);
+ CU_ASSERT(2 == conn->in_pktns->rtb.num_ack_eliciting);
CU_ASSERT(0 == conn->in_pktns->rtb.probe_pkt_left);
fr.type = NGTCP2_FRAME_ACK;
@@ -4816,6 +4817,7 @@ void test_ngtcp2_conn_handshake_probe(void) {
rv = ngtcp2_conn_on_loss_detection_timer(conn, ++t);
CU_ASSERT(0 == rv);
+ CU_ASSERT(1 == conn->in_pktns->rtb.num_retransmittable);
CU_ASSERT(1 == conn->in_pktns->rtb.num_ack_eliciting);
CU_ASSERT(1 == conn->in_pktns->rtb.probe_pkt_left);
@@ -4824,7 +4826,8 @@ void test_ngtcp2_conn_handshake_probe(void) {
spktlen = ngtcp2_conn_write_pkt(conn, NULL, buf, sizeof(buf), ++t);
CU_ASSERT(spktlen > 0);
- CU_ASSERT(1 == conn->in_pktns->rtb.num_ack_eliciting);
+ CU_ASSERT(0 == conn->in_pktns->rtb.num_retransmittable);
+ CU_ASSERT(2 == conn->in_pktns->rtb.num_ack_eliciting);
CU_ASSERT(0 == conn->in_pktns->rtb.probe_pkt_left);
it = ngtcp2_rtb_head(&conn->in_pktns->rtb);
@@ -4842,7 +4845,7 @@ void test_ngtcp2_conn_handshake_probe(void) {
rv = ngtcp2_conn_on_loss_detection_timer(conn, ++t);
CU_ASSERT(0 == rv);
- CU_ASSERT(1 == conn->in_pktns->rtb.num_ack_eliciting);
+ CU_ASSERT(2 == conn->in_pktns->rtb.num_ack_eliciting);
CU_ASSERT(1 == conn->hs_pktns->rtb.probe_pkt_left);
/* This sends anti-deadlock Handshake packet even if we have nothing
@@ -4850,6 +4853,7 @@ void test_ngtcp2_conn_handshake_probe(void) {
spktlen = ngtcp2_conn_write_pkt(conn, NULL, buf, sizeof(buf), ++t);
CU_ASSERT(spktlen > 0);
+ CU_ASSERT(0 == conn->hs_pktns->rtb.num_retransmittable);
CU_ASSERT(1 == conn->hs_pktns->rtb.num_ack_eliciting);
CU_ASSERT(0 == conn->hs_pktns->rtb.probe_pkt_left);
@@ -5674,7 +5678,7 @@ void test_ngtcp2_conn_rtb_reclaim_on_pto(void) {
}
}
- CU_ASSERT(2 == num_reclaim_pkt);
+ CU_ASSERT(3 == num_reclaim_pkt);
ngtcp2_conn_del(conn);
}