commit fdfa4a5a140a2d82ebd92e980a577cbcecd2180b Author: David Goulet <dgoulet@torproject.org> Date: Wed Dec 20 14:14:02 2017 -0500 sched: Use lower layer cell limit with KISTLite Instead of using INT_MAX as a write limit for KISTLite, use the lower layer limit which is using the specialized num_cells_writeable() of the channel that will down the line check the connection's outbuf and limit it to 32KB (OR_CONN_HIGHWATER). That way we don't take the chance of bloating the connection's outbuf and we keep the cells in the circuit queue which our OOM handler can take care of, not the outbuf. Finally, this commit adds a log_debug() in the update socket information function of KIST so we can get the socket information in debug. Fixes #24671 Signed-off-by: David Goulet <dgoulet@torproject.org> --- changes/bug24671 | 6 ++++++ src/or/scheduler_kist.c | 22 ++++++++++++++++------ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/changes/bug24671 b/changes/bug24671 new file mode 100644 index 000000000..34d09e704 --- /dev/null +++ b/changes/bug24671 @@ -0,0 +1,6 @@ + o Minor bugfixes (scheduler, KIST): + - Use a sane write limit for KISTLite when writing onto a connection + buffer instead of using INT_MAX and shoving as much as it can. Because + the OOM handler cleans up circuit queues, we are better off at keeping + them in that queue instead of the connection's buffer. Fixes bug 24671; + bugfix on 0.3.2.1-alpha. diff --git a/src/or/scheduler_kist.c b/src/or/scheduler_kist.c index 9acd89b37..b7a4ee021 100644 --- a/src/or/scheduler_kist.c +++ b/src/or/scheduler_kist.c @@ -298,13 +298,18 @@ update_socket_info_impl, (socket_table_ent_t *ent)) fallback: /* If all of a sudden we don't have kist support, we just zero out all the - * variables for this socket since we don't know what they should be. - * We also effectively allow the socket write as much as it wants to the - * kernel, effectively returning it to vanilla scheduler behavior. Writes - * are still limited by the lower layers of Tor: socket blocking, full - * outbuf, etc. */ + * variables for this socket since we don't know what they should be. We + * also allow the socket to write as much as it can from the estimated + * number of cells the lower layer can accept, effectively returning it to + * Vanilla scheduler behavior. */ ent->cwnd = ent->unacked = ent->mss = ent->notsent = 0; - ent->limit = INT_MAX; + /* This function calls the specialized channel object (currently channeltls) + * and ask how many cells it can write on the outbuf which we then multiply + * by the size of the cells for this channel. The cast is because this + * function requires a non-const channel object, meh. */ + ent->limit = channel_num_cells_writeable((channel_t *) ent->chan) * + (get_cell_network_size(ent->chan->wide_circ_ids) + + TLS_PER_CELL_OVERHEAD); } /* Given a socket that isn't in the table, add it. @@ -398,6 +403,11 @@ update_socket_info(socket_table_t *table, const channel_t *chan) return; // Whelp. Entry didn't exist for some reason so nothing to do. } update_socket_info_impl(ent); + log_debug(LD_SCHED, "chan=%" PRIu64 " updated socket info, limit: %" PRIu64 + ", cwnd: %" PRIu32 ", unacked: %" PRIu32 + ", notsent: %" PRIu32 ", mss: %" PRIu32, + ent->chan->global_identifier, ent->limit, ent->cwnd, ent->unacked, + ent->notsent, ent->mss); } /* Increment the channel's socket written value by the number of bytes. */