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>

diff --git a/changes/bug24671 b/changes/bug24671
new file mode 100644 (file)
index 0000000..34d09e7
--- /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 9acd89b37f73fa0c9b4bece0683cf2961c7330a5..b7a4ee021e8b4004e0caf0411b5da40a8e3d4821 100644 (file)
--- 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. */