Merge remote-tracking branch 'tor-github/pr/329'

changes/ticket27678

@@ -0,0 +1,5 @@
+  o Minor features (controller):
+    - Emit CIRC_BW events as soon as we detect that we processed an invalid
+      or otherwise dropped cell on a circuit. This allows vanguards and
+      other controllers to react more quickly to dropped cells. Closes
+      ticket 27678.

src/core/or/command.c

@@ -480,6 +480,8 @@ command_process_relay_cell(cell_t *cell, channel_t *chan)
   const or_options_t *options = get_options();
   circuit_t *circ;
   int reason, direction;
+  uint32_t orig_delivered_bw = 0;
+  uint32_t orig_overhead_bw = 0;
 
   circ = circuit_get_by_circid_channel(cell->circ_id, chan);
 
@@ -512,6 +514,15 @@ command_process_relay_cell(cell_t *cell, channel_t *chan)
     /* Count the payload bytes only. We don't care about cell headers */
     ocirc->n_read_circ_bw = tor_add_u32_nowrap(ocirc->n_read_circ_bw,
                                                CELL_PAYLOAD_SIZE);
+
+    /* Stash the original delivered and overhead values. These values are
+     * updated by circuit_read_valid_data() during cell processing by
+     * connection_edge_process_relay_cell(), called from
+     * circuit_receive_relay_cell() below. If they do not change, we inform
+     * the control port about dropped cells immediately after the call
+     * to circuit_receive_relay_cell() below. */
+    orig_delivered_bw = ocirc->n_delivered_read_circ_bw;
+    orig_overhead_bw = ocirc->n_overhead_read_circ_bw;
   }
 
   if (!CIRCUIT_IS_ORIGIN(circ) &&
@@ -535,6 +546,8 @@ command_process_relay_cell(cell_t *cell, channel_t *chan)
                (unsigned)cell->circ_id);
       if (CIRCUIT_IS_ORIGIN(circ)) {
         circuit_log_path(LOG_WARN, LD_OR, TO_ORIGIN_CIRCUIT(circ));
+        /* Always emit a bandwidth event for closed circs */
+        control_event_circ_bandwidth_used_for_circ(TO_ORIGIN_CIRCUIT(circ));
       } else if (circ->n_chan) {
         log_warn(LD_OR, " upstream=%s",
                  channel_get_actual_remote_descr(circ->n_chan));
@@ -560,9 +573,33 @@ command_process_relay_cell(cell_t *cell, channel_t *chan)
     log_fn(LOG_PROTOCOL_WARN,LD_PROTOCOL,"circuit_receive_relay_cell "
            "(%s) failed. Closing.",
            direction==CELL_DIRECTION_OUT?"forward":"backward");
+    /* Always emit a bandwidth event for closed circs */
+    if (CIRCUIT_IS_ORIGIN(circ)) {
+      control_event_circ_bandwidth_used_for_circ(TO_ORIGIN_CIRCUIT(circ));
+    }
     circuit_mark_for_close(circ, -reason);
   }
 
+  if (CIRCUIT_IS_ORIGIN(circ)) {
+    origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ);
+
+    /* If neither the delivered nor overhead values changed, this cell
+     * was dropped due to being invalid by one of the error codepaths in
+     * connection_edge_process_relay_cell(), called by
+     * circuit_receive_relay_cell().
+     *
+     * Valid cells, on the other hand, call circuit_read_valid_data()
+     * to update these values upon processing them.
+     *
+     * So, if the values are the same as those stored above,
+     * emit a control port event for CIRC_BW, so the controller can
+     * react quickly to invalid cells. */
+    if (orig_delivered_bw == ocirc->n_delivered_read_circ_bw &&
+        orig_overhead_bw == ocirc->n_overhead_read_circ_bw) {
+      control_event_circ_bandwidth_used_for_circ(ocirc);
+    }
+  }
+
   /* If this is a cell in an RP circuit, count it as part of the
      hidden service stats */
   if (options->HiddenServiceStatistics &&

src/feature/control/control.c

@@ -6047,41 +6047,69 @@ control_event_stream_bandwidth_used(void)
 int
 control_event_circ_bandwidth_used(void)
 {
-  origin_circuit_t *ocirc;
-  struct timeval now;
-  char tbuf[ISO_TIME_USEC_LEN+1];
   if (!EVENT_IS_INTERESTING(EVENT_CIRC_BANDWIDTH_USED))
     return 0;
 
   SMARTLIST_FOREACH_BEGIN(circuit_get_global_list(), circuit_t *, circ) {
     if (!CIRCUIT_IS_ORIGIN(circ))
       continue;
-    ocirc = TO_ORIGIN_CIRCUIT(circ);
+
-    if (!ocirc->n_read_circ_bw && !ocirc->n_written_circ_bw)
+    control_event_circ_bandwidth_used_for_circ(TO_ORIGIN_CIRCUIT(circ));
-      continue;
-    tor_gettimeofday(&now);
-    format_iso_time_nospace_usec(tbuf, &now);
-    send_control_event(EVENT_CIRC_BANDWIDTH_USED,
-                       "650 CIRC_BW ID=%d READ=%lu WRITTEN=%lu TIME=%s "
-                       "DELIVERED_READ=%lu OVERHEAD_READ=%lu "
-                       "DELIVERED_WRITTEN=%lu OVERHEAD_WRITTEN=%lu\r\n",
-                       ocirc->global_identifier,
-                       (unsigned long)ocirc->n_read_circ_bw,
-                       (unsigned long)ocirc->n_written_circ_bw,
-                       tbuf,
-                       (unsigned long)ocirc->n_delivered_read_circ_bw,
-                       (unsigned long)ocirc->n_overhead_read_circ_bw,
-                       (unsigned long)ocirc->n_delivered_written_circ_bw,
-                       (unsigned long)ocirc->n_overhead_written_circ_bw);
-    ocirc->n_written_circ_bw = ocirc->n_read_circ_bw = 0;
-    ocirc->n_overhead_written_circ_bw = ocirc->n_overhead_read_circ_bw = 0;
-    ocirc->n_delivered_written_circ_bw = ocirc->n_delivered_read_circ_bw = 0;
   }
   SMARTLIST_FOREACH_END(circ);
 
   return 0;
 }
 
+/**
+ * Emit a CIRC_BW event line for a specific circuit.
+ *
+ * This function sets the values it emits to 0, and does not emit
+ * an event if there is no new data to report since the last call.
+ *
+ * Therefore, it may be called at any frequency.
+ */
+int
+control_event_circ_bandwidth_used_for_circ(origin_circuit_t *ocirc)
+{
+  struct timeval now;
+  char tbuf[ISO_TIME_USEC_LEN+1];
+
+  tor_assert(ocirc);
+
+  if (!EVENT_IS_INTERESTING(EVENT_CIRC_BANDWIDTH_USED))
+    return 0;
+
+  /* n_read_circ_bw and n_written_circ_bw are always updated
+   * when there is any new cell on a circuit, and set to 0 after
+   * the event, below.
+   *
+   * Therefore, checking them is sufficient to determine if there
+   * is new data to report. */
+  if (!ocirc->n_read_circ_bw && !ocirc->n_written_circ_bw)
+    return 0;
+
+  tor_gettimeofday(&now);
+  format_iso_time_nospace_usec(tbuf, &now);
+  send_control_event(EVENT_CIRC_BANDWIDTH_USED,
+                     "650 CIRC_BW ID=%d READ=%lu WRITTEN=%lu TIME=%s "
+                     "DELIVERED_READ=%lu OVERHEAD_READ=%lu "
+                     "DELIVERED_WRITTEN=%lu OVERHEAD_WRITTEN=%lu\r\n",
+                     ocirc->global_identifier,
+                     (unsigned long)ocirc->n_read_circ_bw,
+                     (unsigned long)ocirc->n_written_circ_bw,
+                     tbuf,
+                     (unsigned long)ocirc->n_delivered_read_circ_bw,
+                     (unsigned long)ocirc->n_overhead_read_circ_bw,
+                     (unsigned long)ocirc->n_delivered_written_circ_bw,
+                     (unsigned long)ocirc->n_overhead_written_circ_bw);
+  ocirc->n_written_circ_bw = ocirc->n_read_circ_bw = 0;
+  ocirc->n_overhead_written_circ_bw = ocirc->n_overhead_read_circ_bw = 0;
+  ocirc->n_delivered_written_circ_bw = ocirc->n_delivered_read_circ_bw = 0;
+
+  return 0;
+}
+
 /** Print out CONN_BW event for a single OR/DIR/EXIT <b>conn</b> and reset
   * bandwidth counters. */
 int

src/feature/control/control.h

@@ -146,6 +146,7 @@ int control_event_bandwidth_used(uint32_t n_read, uint32_t n_written);
 int control_event_stream_bandwidth(edge_connection_t *edge_conn);
 int control_event_stream_bandwidth_used(void);
 int control_event_circ_bandwidth_used(void);
+int control_event_circ_bandwidth_used_for_circ(origin_circuit_t *ocirc);
 int control_event_conn_bandwidth(connection_t *conn);
 int control_event_conn_bandwidth_used(void);
 int control_event_circuit_cell_stats(void);
@@ -246,7 +247,6 @@ void control_event_hs_descriptor_content(const char *onion_address,
                                          const char *desc_id,
                                          const char *hsdir_fp,
                                          const char *content);
-
 void control_free_all(void);
 
 #ifdef CONTROL_PRIVATE