On Fri, 2018-01-05 at 00:12 +0100, Alexander Bluhm wrote:
> 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
