From 74dfb30a0403c8c7e28e432285364b1df1f6cd89 Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Sat, 10 Aug 2019 17:22:28 +0900
Subject: [PATCH] Simplify expiry handler
---
doc/programmers-guide.rst | 15 +++-------
examples/client.cc | 38 ++++++++++++------------
examples/client.h | 3 +-
examples/server.cc | 57 +++++++++++++++++++++++-------------
examples/server.h | 1 +
lib/includes/ngtcp2/ngtcp2.h | 16 ++++++----
lib/ngtcp2_conn.c | 15 ++++++++++
7 files changed, 89 insertions(+), 56 deletions(-)
diff --git a/doc/programmers-guide.rst b/doc/programmers-guide.rst
index 4ba4f69a..dae712f7 100644
--- a/doc/programmers-guide.rst
+++ b/doc/programmers-guide.rst
@@ -264,18 +264,11 @@ monotonic clock should work better. It should be same clock passed to
``ngtcp2_setting``.
`ngtcp2_conn_get_expiry()` tells an application when timer fires.
-When timer fires, it has to call some API functions. If the current
-timestamp is equal to or larger than the value returned from
-`ngtcp2_conn_loss_detection_expiry()`, it has to call
-`ngtcp2_conn_on_loss_detection_timer()` and `ngtcp2_conn_write_pkt()`
-(or `ngtcp2_conn_writev_stream()`). If the current timestamp is equal
-to or larger than the value returned from
-`ngtcp2_conn_ack_delay_expiry()`, it has to call
-`ngtcp2_conn_cancel_expired_ack_delay_timer()` and
-`ngtcp2_conn_write_pkt()` (or `ngtcp2_conn_writev_stream()`). After
-calling these functions, new expiry will be set. The application
-should call `ngtcp2_conn_get_expiry()` to restart timer.
+When timer fires, call `ngtcp2_conn_handle_expiry()` and
+`ngtcp2_conn_write_pkt()` (or `ngtcp2_conn_writev_stream()`).
+After calling these functions, new expiry will be set. The
+application should call `ngtcp2_conn_get_expiry()` to restart timer.
After QUIC handshake
--------------------
diff --git a/examples/client.cc b/examples/client.cc
index b34f1828..de184c25 100644
--- a/examples/client.cc
+++ b/examples/client.cc
@@ -350,15 +350,13 @@ namespace {
void retransmitcb(struct ev_loop *loop, ev_timer *w, int revents) {
int rv;
auto c = static_cast<Client *>(w->data);
- auto conn = c->conn();
- auto now = util::timestamp(loop);
- auto retransmit = ngtcp2_conn_loss_detection_expiry(conn) <= now;
- if (ngtcp2_conn_ack_delay_expiry(conn) <= now) {
- ngtcp2_conn_cancel_expired_ack_delay_timer(conn, now);
+ rv = c->handle_expiry();
+ if (rv != 0) {
+ goto fail;
}
- rv = c->on_write(retransmit);
+ rv = c->on_write();
if (rv != 0) {
goto fail;
}
@@ -1328,7 +1326,21 @@ int Client::on_read() {
void Client::reset_idle_timer() { ev_timer_again(loop_, &timer_); }
-int Client::on_write(bool retransmit) {
+int Client::handle_expiry() {
+ auto now = util::timestamp(loop_);
+ auto rv = ngtcp2_conn_handle_expiry(conn_, now);
+ if (rv != 0) {
+ std::cerr << "ngtcp2_conn_handle_expiry: " << ngtcp2_strerror(rv)
+ << std::endl;
+ last_error_ = quic_err_transport(NGTCP2_ERR_INTERNAL);
+ disconnect();
+ return -1;
+ }
+
+ return 0;
+}
+
+int Client::on_write() {
if (sendbuf_.size() > 0) {
auto rv = send_packet();
if (rv != NETWORK_ERR_OK) {
@@ -1342,18 +1354,6 @@ int Client::on_write(bool retransmit) {
assert(sendbuf_.left() >= max_pktlen_);
- if (retransmit) {
- auto rv =
- ngtcp2_conn_on_loss_detection_timer(conn_, util::timestamp(loop_));
- if (rv != 0) {
- std::cerr << "ngtcp2_conn_on_loss_detection_timer: "
- << ngtcp2_strerror(rv) << std::endl;
- last_error_ = quic_err_transport(NGTCP2_ERR_INTERNAL);
- disconnect();
- return -1;
- }
- }
-
auto rv = write_streams();
if (rv != 0) {
if (rv == NETWORK_ERR_SEND_BLOCKED) {
diff --git a/examples/client.h b/examples/client.h
index 2271f977..db6690a7 100644
--- a/examples/client.h
+++ b/examples/client.h
@@ -162,10 +162,11 @@ public:
int tls_handshake(bool initial = false);
int read_tls();
int on_read();
- int on_write(bool retransmit = false);
+ int on_write();
int write_streams();
int feed_data(const sockaddr *sa, socklen_t salen, uint8_t *data,
size_t datalen);
+ int handle_expiry();
void schedule_retransmit();
int handshake_completed();
diff --git a/examples/server.cc b/examples/server.cc
index 49fcb00f..e24f766d 100644
--- a/examples/server.cc
+++ b/examples/server.cc
@@ -803,35 +803,27 @@ void retransmitcb(struct ev_loop *loop, ev_timer *w, int revents) {
auto h = static_cast<Handler *>(w->data);
auto s = h->server();
- auto conn = h->conn();
- auto now = util::timestamp(loop);
if (!config.quiet) {
std::cerr << "Timer expired" << std::endl;
}
- if (ngtcp2_conn_loss_detection_expiry(conn) <= now) {
- if (!config.quiet) {
- std::cerr << "Loss detection timer expired" << std::endl;
- }
- rv = ngtcp2_conn_on_loss_detection_timer(conn, util::timestamp(loop));
- if (rv != 0) {
- std::cerr << "ngtcp2_conn_on_loss_detection_timer: "
- << ngtcp2_strerror(rv) << std::endl;
- s->remove(h);
- return;
- }
- }
- if (ngtcp2_conn_ack_delay_expiry(conn) <= now) {
- if (!config.quiet) {
- std::cerr << "Delayed ACK timer expired" << std::endl;
- }
- ngtcp2_conn_cancel_expired_ack_delay_timer(conn, now);
+ rv = h->handle_expiry();
+ if (rv != 0) {
+ goto fail;
}
rv = h->on_write();
+ if (rv != 0) {
+ goto fail;
+ }
+
+ ev_timer_stop(loop, w);
+
+ return;
+
+fail:
switch (rv) {
- case 0:
case NETWORK_ERR_CLOSE_WAIT:
case NETWORK_ERR_SEND_BLOCKED:
ev_timer_stop(loop, w);
@@ -2057,6 +2049,31 @@ int Handler::on_read(const Endpoint &ep, const sockaddr *sa, socklen_t salen,
void Handler::reset_idle_timer() { ev_timer_again(loop_, &timer_); }
+int Handler::handle_expiry() {
+ auto now = util::timestamp(loop_);
+ if (ngtcp2_conn_loss_detection_expiry(conn_) <= now) {
+ if (!config.quiet) {
+ std::cerr << "Loss detection timer expired" << std::endl;
+ }
+ }
+
+ if (ngtcp2_conn_ack_delay_expiry(conn_) <= now) {
+ if (!config.quiet) {
+ std::cerr << "Delayed ACK timer expired" << std::endl;
+ }
+ }
+
+ auto rv = ngtcp2_conn_handle_expiry(conn_, now);
+ if (rv != 0) {
+ std::cerr << "ngtcp2_conn_handle_expiry: " << ngtcp2_strerror(rv)
+ << std::endl;
+ last_error_ = quic_err_transport(rv);
+ return handle_error();
+ }
+
+ return 0;
+}
+
int Handler::on_write() {
int rv;
diff --git a/examples/server.h b/examples/server.h
index e5b9e5ff..37db99bf 100644
--- a/examples/server.h
+++ b/examples/server.h
@@ -200,6 +200,7 @@ public:
int feed_data(const Endpoint &ep, const sockaddr *sa, socklen_t salen,
uint8_t *data, size_t datalen);
void schedule_retransmit();
+ int handle_expiry();
void signal_write();
int handshake_completed();
diff --git a/lib/includes/ngtcp2/ngtcp2.h b/lib/includes/ngtcp2/ngtcp2.h
index f6551922..cc78387f 100644
--- a/lib/includes/ngtcp2/ngtcp2.h
+++ b/lib/includes/ngtcp2/ngtcp2.h
@@ -1780,14 +1780,20 @@ NGTCP2_EXTERN ngtcp2_tstamp ngtcp2_conn_ack_delay_expiry(ngtcp2_conn *conn);
* min(ngtcp2_conn_loss_detection_expiry(conn),
* ngtcp2_conn_ack_delay_expiry(conn), other timers in |conn|).
*
- * Besides any special requirements for
- * `ngtcp2_conn_loss_detection_expiry()` and
- * `ngtcp2_conn_ack_delay_expiry()`, when the timer is expired,
- * `ngtcp2_conn_write_pkt` (or `ngtcp2_conn_writev_stream`) should be
- * called.
+ * Call `ngtcp2_conn_handle_expiry()` and `ngtcp2_conn_write_pkt` (or
+ * `ngtcp2_conn_writev_stream`) if expiry time is passed.
*/
NGTCP2_EXTERN ngtcp2_tstamp ngtcp2_conn_get_expiry(ngtcp2_conn *conn);
+/**
+ * @function
+ *
+ * `ngtcp2_conn_handle_expiry` handles expired timer. It do nothing
+ * if timer is not expired.
+ */
+NGTCP2_EXTERN int ngtcp2_conn_handle_expiry(ngtcp2_conn *conn,
+ ngtcp2_tstamp ts);
+
/**
* @function
*
diff --git a/lib/ngtcp2_conn.c b/lib/ngtcp2_conn.c
index 591c2f8b..280e3dc9 100644
--- a/lib/ngtcp2_conn.c
+++ b/lib/ngtcp2_conn.c
@@ -7430,6 +7430,21 @@ ngtcp2_tstamp ngtcp2_conn_get_expiry(ngtcp2_conn *conn) {
return ngtcp2_min(res, t3);
}
+int ngtcp2_conn_handle_expiry(ngtcp2_conn *conn, ngtcp2_tstamp ts) {
+ int rv;
+
+ ngtcp2_conn_cancel_expired_ack_delay_timer(conn, ts);
+
+ if (ngtcp2_conn_loss_detection_expiry(conn) <= ts) {
+ rv = ngtcp2_conn_on_loss_detection_timer(conn, ts);
+ if (rv != 0) {
+ return rv;
+ }
+ }
+
+ return 0;
+}
+
static void acktr_cancel_expired_ack_delay_timer(ngtcp2_acktr *acktr,
ngtcp2_tstamp ts) {
if (!(acktr->flags & NGTCP2_ACKTR_FLAG_CANCEL_TIMER) &&
--
GitLab