On Wed, Dec 13, 2017 at 07:42:03AM +0100, Claudio Jeker wrote:
> On Wed, Dec 13, 2017 at 12:25:39AM +0000, Rivo Nurges wrote:
> > If you http PUT a "big" file through relayd, server<>relay read side
> > will eventually get a EVBUFFER_TIMEOUT. Nothing comes back from the
> > server until the PUT is done. I disabled server read timeouts for PUT
> > requests.
> 
> I have seen something similar and came to the conclusion that the timeout
> handling of relayd is not correct. As long as traffic is flowing the
> timeout should be reset (at least that is what every other implementation
> does). This is not really happening in relayd. I have seen this on GET
> requests that are huge (timeout hits in the middle of the transimit and
> kills the session).

I have commited more regression tests that check the timeout with
unidirectional traffic flow.  I could not find an error.  In theory
when we have an idle timeout in one direction, relayd checks wheter
there is trafic flowing in the other direction.  The tests set the
timeout to 2 seconds and send 5 bytes while sleeping one second
between each byte.  The timeout does not trigger.

So it seems that you encounter some corner case.  I need more
information.

- Do you use http or https?
- Do you use persistent connections?
- Do you use chunked encoding?
- Does it only occur with http or also with plain tcp?
- Does disabling socket splicing help?
- Does it happen when the connect to the server is slow?

While testing I saw that with socket splicing the timeout is handled
twice.  We get an wakeup from the idle splicing and from libevent
timeout.  I think it is sufficient to only use the idle splicing
if it is available.

Does this diff help?

bluhm

Index: relay.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.237
diff -u -p -r1.237 relay.c
--- relay.c     27 Dec 2017 15:53:30 -0000      1.237
+++ relay.c     4 Jan 2018 22:44:20 -0000
@@ -733,16 +733,21 @@ relay_connected(int fd, short sig, void 
        if ((rlay->rl_conf.flags & F_TLSCLIENT) && (out->tls != NULL))
                relay_tls_connected(out);
 
-       bufferevent_settimeout(bev,
-           rlay->rl_conf.timeout.tv_sec, rlay->rl_conf.timeout.tv_sec);
        bufferevent_setwatermark(bev, EV_WRITE,
                RELAY_MIN_PREFETCHED * proto->tcpbufsiz, 0);
        bufferevent_enable(bev, EV_READ|EV_WRITE);
        if (con->se_in.bev)
                bufferevent_enable(con->se_in.bev, EV_READ);
 
-       if (relay_splice(&con->se_out) == -1)
+       switch (relay_splice(&con->se_out)) {
+       case 0:
+               bufferevent_settimeout(bev,
+                   rlay->rl_conf.timeout.tv_sec, rlay->rl_conf.timeout.tv_sec);
+               break;
+       case -1:
                relay_close(con, strerror(errno));
+               break;
+       }
 }
 
 void
@@ -784,14 +789,19 @@ relay_input(struct rsession *con)
        if ((rlay->rl_conf.flags & F_TLS) && con->se_in.tls != NULL)
                relay_tls_connected(&con->se_in);
 
-       bufferevent_settimeout(con->se_in.bev,
-           rlay->rl_conf.timeout.tv_sec, rlay->rl_conf.timeout.tv_sec);
        bufferevent_setwatermark(con->se_in.bev, EV_WRITE,
                RELAY_MIN_PREFETCHED * proto->tcpbufsiz, 0);
        bufferevent_enable(con->se_in.bev, EV_READ|EV_WRITE);
 
-       if (relay_splice(&con->se_in) == -1)
+       switch (relay_splice(&con->se_in)) {
+       case 0:
+               bufferevent_settimeout(con->se_in.bev,
+                   rlay->rl_conf.timeout.tv_sec, rlay->rl_conf.timeout.tv_sec);
+               break;
+       case -1:
                relay_close(con, strerror(errno));
+               break;
+       }
 }
 
 void

Reply via email to