[http] Abort connections after a long period of inactivity

Once an HTTP download has started (i.e. once all request headers have
been sent), we generally have no more data to transmit.  If an HTTP
connection dies silently (e.g. due to a network failure, a NIC driver
bug, or a server crash) then there is no mechanism that will currently
detect this situation by default.

We do send TCP keep-alives (to maintain state in intermediate routers
and firewalls), but we do not attempt to elicit a response from the
server.  RFC 9293 explicitly states that the absence of a response to
a TCP keep-alive probe must not be interpreted as indicating a dead
connection, since TCP cannot guarantee reliable delivery of packets
that do not advance the sequence number.

Scripts may use the "--timeout" option to impose an overall time limit
on downloads, but this mechanism is off by default and requires
additional thought and configuration by the user (which goes against
iPXE's general philosophy of being as automatic as possible).

Add an idle connection watchdog timer which will cause the HTTP
download to abort after 120 seconds of inactivity.  Activity is
defined as an I/O buffer being delivered to the HTTP transaction's
upstream data transfer interface.

Downloads over HTTPS may experience a substantial delay until the
first recorded activity, since all TLS negotiation (including
cross-chained certificate downloads and OCSP checks) must complete
before any application data can be sent.  We choose to not reset the
watchdog timer during TLS negotiation, on the basis that 120 seconds
is already an unreasonably long time for a TLS negotiation to take to
complete.  If necessary, resetting the watchdog timer could be
accomplished by having the TLS layer deliver zero-length I/O buffers
(via xfer_seek()) to indicate forward progress being made.

When using PeerDist content encoding, the downloaded content
information is not passed through to the content-decoded interface and
so will not be classed as activity.  Any activity in the individual
PeerDist block downloads (either from peers or as range requests from
the origin server) will be classed as activity in the overall
download, since individual block downloads do not buffer data but
instead pass it through directly via the PeerDist download
multiplexer.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/include/ipxe/http.h b/src/include/ipxe/http.h
index f5416042a55c6793bc82c21fa0d08e916fc852e7..fc3e7b7a1093eab590024cb9d77a7d2ec8a787b3 100644 (file)
--- a/src/include/ipxe/http.h
+++ b/src/include/ipxe/http.h
@@ -427,6 +427,8 @@ struct http_transaction {
        struct process process;
        /** Reconnection timer */
        struct retry_timer retry;
+       /** Idle connection watchdog timer */
+       struct retry_timer watchdog;
 
        /** Request URI */
        struct uri *uri;

diff --git a/src/net/tcp/httpcore.c b/src/net/tcp/httpcore.c
index 42512cda495242063e9a9ced506f5f3dff101f9c..8fee0421ca30dfb765ce1def8323cbfac6ec22fa 100644 (file)
--- a/src/net/tcp/httpcore.c
+++ b/src/net/tcp/httpcore.c
@@ -106,6 +106,9 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 /** Retry delay used when we cannot understand the Retry-After header */
 #define HTTP_RETRY_SECONDS 5
 
+/** Idle connection watchdog timeout */
+#define HTTP_WATCHDOG_SECONDS 120
+
 /** Receive profiler */
 static struct profiler http_rx_profiler __profiler = { .name = "http.rx" };
 
@@ -281,8 +284,9 @@ static void http_close ( struct http_transaction *http, int rc ) {
        /* Stop process */
        process_del ( &http->process );
 
-       /* Stop timer */
+       /* Stop timers */
        stop_timer ( &http->retry );
+       stop_timer ( &http->watchdog );
 
        /* Close all interfaces */
        intfs_shutdown ( rc, &http->conn, &http->transfer, &http->content,
@@ -301,6 +305,18 @@ static void http_close_error ( struct http_transaction *http, int rc ) {
        http_close ( http, ( rc ? rc : -EPIPE ) );
 }
 
+/**
+ * Hold off HTTP idle connection watchdog timer
+ *
+ * @v http             HTTP transaction
+ */
+static inline void http_watchdog ( struct http_transaction *http ) {
+
+       /* (Re)start watchdog timer */
+       start_timer_fixed ( &http->watchdog,
+                           ( HTTP_WATCHDOG_SECONDS * TICKS_PER_SEC ) );
+}
+
 /**
  * Reopen stale HTTP connection
  *
@@ -322,6 +338,9 @@ static void http_reopen ( struct http_transaction *http ) {
        /* Reset state */
        http->state = &http_request;
 
+       /* Restart idle connection watchdog timer */
+       http_watchdog ( http );
+
        /* Reschedule transmission process */
        process_add ( &http->process );
 
@@ -346,6 +365,22 @@ static void http_retry_expired ( struct retry_timer *retry,
        http_reopen ( http );
 }
 
+/**
+ * Handle idle connection watchdog timer expiry
+ *
+ * @v watchdog         Idle connection watchdog timer
+ * @v over             Failure indicator
+ */
+static void http_watchdog_expired ( struct retry_timer *watchdog,
+                                   int over __unused ) {
+       struct http_transaction *http =
+               container_of ( watchdog, struct http_transaction, watchdog );
+
+       /* Abort connection */
+       DBGC ( http, "HTTP %p aborting idle connection\n", http );
+       http_close ( http, -ETIMEDOUT );
+}
+
 /**
  * HTTP transmit process
  *
@@ -461,6 +496,9 @@ static int http_content_deliver ( struct http_transaction *http,
                return 0;
        }
 
+       /* Hold off idle connection watchdog timer */
+       http_watchdog ( http );
+
        /* Deliver to data transfer interface */
        profile_start ( &http_xfer_profiler );
        if ( ( rc = xfer_deliver ( &http->xfer, iob_disown ( iobuf ),
@@ -651,6 +689,7 @@ int http_open ( struct interface *xfer, struct http_method *method,
        intf_plug_plug ( &http->transfer, &http->content );
        process_init ( &http->process, &http_process_desc, &http->refcnt );
        timer_init ( &http->retry, http_retry_expired, &http->refcnt );
+       timer_init ( &http->watchdog, http_watchdog_expired, &http->refcnt );
        http->uri = uri_get ( uri );
        http->request.method = method;
        http->request.uri = request_uri_string;
@@ -676,6 +715,9 @@ int http_open ( struct interface *xfer, struct http_method *method,
                goto err_connect;
        }
 
+       /* Start watchdog timer */
+       http_watchdog ( http );
+
        /* Attach to parent interface, mortalise self, and return */
        intf_plug_plug ( &http->xfer, xfer );
        ref_put ( &http->refcnt );
@@ -812,6 +854,7 @@ static int http_transfer_complete ( struct http_transaction *http ) {
                http, http->response.retry_after );
        start_timer_fixed ( &http->retry,
                            ( http->response.retry_after * TICKS_PER_SEC ) );
+       stop_timer ( &http->watchdog );
        return 0;
 }