Merge branch 'ticket40816_049_01' into 'main'

metrics: Add new relay metrics to MetricsPort

Closes #40816

See merge request tpo/core/tor!793

7 files changed, 98 insertions, 0 deletions

diff --git a/changes/ticket40816 b/changes/ticket40816
new file mode 100644
index 0000000000..509b11ad7e
--- /dev/null
+++ b/changes/ticket40816
@@ -0,0 +1,4 @@
+  o Minor feature (metrics port, relay):
+    - Add new metrics for relays on the MetricsPort namely the count of drop
+      cell, destroy cell and the number of circuit protocol violation seen that
+      lead to a circuit close. Closes ticket 40816.
diff --git a/src/core/or/circuitlist.c b/src/core/or/circuitlist.c
index 643d97b064..8f8ed915fb 100644
--- a/src/core/or/circuitlist.c
+++ b/src/core/or/circuitlist.c
@@ -160,6 +160,10 @@ double cc_stats_circ_close_ss_cwnd_ma = 0;
 
 uint64_t cc_stats_circs_closed = 0;
 
+/** Total number of circuit protocol violation. This is incremented when the
+ * END_CIRC_REASON_TORPROTOCOL is used to close a circuit. */
+uint64_t circ_n_proto_violation = 0;
+
 /********* END VARIABLES ************/
 
 /* Implement circuit handle helpers. */
@@ -2197,6 +2201,10 @@ circuit_mark_for_close_, (circuit_t *circ, int reason, int line,
   tor_assert(line);
   tor_assert(file);
 
+  if (reason == END_CIRC_REASON_TORPROTOCOL) {
+    circ_n_proto_violation++;
+  }
+
   /* Check whether the circuitpadding subsystem wants to block this close */
   if (circpad_marked_circuit_for_padding(circ, reason)) {
     return;
diff --git a/src/core/or/circuitlist.h b/src/core/or/circuitlist.h
index ca3c5bd0ee..0c8f958d2a 100644
--- a/src/core/or/circuitlist.h
+++ b/src/core/or/circuitlist.h
@@ -172,6 +172,7 @@
 extern double cc_stats_circ_close_cwnd_ma;
 extern double cc_stats_circ_close_ss_cwnd_ma;
 extern uint64_t cc_stats_circs_closed;
+extern uint64_t circ_n_proto_violation;
 
 /** Convert a circuit_t* to a pointer to the enclosing or_circuit_t.  Assert
  * if the cast is impossible. */
diff --git a/src/feature/relay/relay_metrics.c b/src/feature/relay/relay_metrics.c
index 8b8c07f580..492a5945b8 100644
--- a/src/feature/relay/relay_metrics.c
+++ b/src/feature/relay/relay_metrics.c
@@ -13,6 +13,7 @@
 #include "core/or/or.h"
 #include "core/mainloop/connection.h"
 #include "core/mainloop/mainloop.h"
+#include "core/or/command.h"
 #include "core/or/congestion_control_common.h"
 #include "core/or/congestion_control_vegas.h"
 #include "core/or/congestion_control_flow.h"
@@ -54,6 +55,9 @@ static void fill_socket_values(void);
 static void fill_onionskins_values(void);
 static void fill_oom_values(void);
 static void fill_streams_values(void);
+static void fill_relay_circ_proto_violation(void);
+static void fill_relay_destroy_cell(void);
+static void fill_relay_drop_cell(void);
 static void fill_relay_flags(void);
 static void fill_tcp_exhaustion_values(void);
 static void fill_traffic_values(void);
@@ -217,6 +221,27 @@ static const relay_metrics_entry_t base_metrics[] =
     .help = "Total number of REND1 cells we received",
     .fill_fn = fill_rend1_cells,
   },
+  {
+    .key = RELAY_METRICS_CIRC_DESTROY_CELL,
+    .type = METRICS_TYPE_COUNTER,
+    .name = METRICS_NAME(relay_destroy_cell_total),
+    .help = "Total number of DESTROY cell we received",
+    .fill_fn = fill_relay_destroy_cell,
+  },
+  {
+    .key = RELAY_METRICS_CIRC_PROTO_VIOLATION,
+    .type = METRICS_TYPE_COUNTER,
+    .name = METRICS_NAME(relay_circ_proto_violation_total),
+    .help = "Total number of circuit protocol violation",
+    .fill_fn = fill_relay_circ_proto_violation,
+  },
+  {
+    .key = RELAY_METRICS_CIRC_DROP_CELL,
+    .type = METRICS_TYPE_COUNTER,
+    .name = METRICS_NAME(relay_drop_cell_total),
+    .help = "Total number of DROP cell we received",
+    .fill_fn = fill_relay_drop_cell,
+  },
 };
 static const size_t num_base_metrics = ARRAY_LENGTH(base_metrics);
 
@@ -1206,6 +1231,46 @@ fill_rend1_cells(void)
   }
 }
 
+/** Fill the metrics store for the RELAY_METRICS_CIRC_DESTROY_CELL counter. */
+static void
+fill_relay_destroy_cell(void)
+{
+  metrics_store_entry_t *sentry;
+  const relay_metrics_entry_t *rentry =
+    &base_metrics[RELAY_METRICS_CIRC_DESTROY_CELL];
+
+  sentry = metrics_store_add(the_store, rentry->type, rentry->name,
+                             rentry->help, 0, NULL);
+  metrics_store_entry_update(sentry,
+                             (int64_t) stats_n_destroy_cells_processed);
+}
+
+/** Fill the metrics store for the RELAY_METRICS_CIRC_DROP_CELL counter. */
+static void
+fill_relay_drop_cell(void)
+{
+  metrics_store_entry_t *sentry;
+  const relay_metrics_entry_t *rentry =
+    &base_metrics[RELAY_METRICS_CIRC_DROP_CELL];
+
+  sentry = metrics_store_add(the_store, rentry->type, rentry->name,
+                             rentry->help, 0, NULL);
+  metrics_store_entry_update(sentry, rep_hist_get_drop_cell_received_count());
+}
+
+/** Fill the metrics store for the RELAY_METRICS_CIRC_PROTO_VIOLATION. */
+static void
+fill_relay_circ_proto_violation(void)
+{
+  metrics_store_entry_t *sentry;
+  const relay_metrics_entry_t *rentry =
+    &base_metrics[RELAY_METRICS_CIRC_PROTO_VIOLATION];
+
+  sentry = metrics_store_add(the_store, rentry->type, rentry->name,
+                             rentry->help, 0, NULL);
+  metrics_store_entry_update(sentry, circ_n_proto_violation);
+}
+
 /** Reset the global store and fill it with all the metrics from base_metrics
  * and their associated values.
  *
diff --git a/src/feature/relay/relay_metrics.h b/src/feature/relay/relay_metrics.h
index cf9dddf955..e7b5b660fa 100644
--- a/src/feature/relay/relay_metrics.h
+++ b/src/feature/relay/relay_metrics.h
@@ -57,6 +57,12 @@ typedef enum {
   RELAY_METRICS_NUM_INTRO1_CELLS,
   /** Number of times we received a REND1 cell */
   RELAY_METRICS_NUM_REND1_CELLS,
+  /** Number of circuit closed by receiving a DESTROY cell. */
+  RELAY_METRICS_CIRC_DESTROY_CELL,
+  /** Number of circuits closed due to protocol violation. */
+  RELAY_METRICS_CIRC_PROTO_VIOLATION,
+  /** Number of drop cell seen. */
+  RELAY_METRICS_CIRC_DROP_CELL,
 } relay_metrics_key_t;
 
 /** The metadata of a relay metric. */
diff --git a/src/feature/stats/rephist.c b/src/feature/stats/rephist.c
index 20610b6011..055081fc7c 100644
--- a/src/feature/stats/rephist.c
+++ b/src/feature/stats/rephist.c
@@ -280,6 +280,9 @@ static dns_stats_t dns_AAAA_stats;
 /** DNS query statistics store. It covers all type of queries. */
 static dns_stats_t dns_all_stats;
 
+/** Counter of the total number of DROP cell received. */
+static uint64_t relay_circ_n_drop_cell_received = 0;
+
 /** Return the point to the DNS statistics store. Ignore the type for now
  * because of a libevent problem. */
 static inline dns_stats_t *
@@ -2815,6 +2818,8 @@ rep_hist_padding_count_write(padding_type_t type)
   switch (type) {
     case PADDING_TYPE_DROP:
       padding_current.write_drop_cell_count++;
+      /* Padding stats get reset thus why we have two counters. */
+      relay_circ_n_drop_cell_received++;
       break;
     case PADDING_TYPE_CELL:
       padding_current.write_pad_cell_count++;
@@ -3022,6 +3027,13 @@ rep_hist_consensus_has_changed(const networkstatus_t *ns)
                             OVERLOAD_ONIONSKIN_NTOR_PERIOD_SECS_MAX);
 }
 
+/** Relay Only: return the total number of DROP cell received. */
+uint64_t
+rep_hist_get_drop_cell_received_count(void)
+{
+  return relay_circ_n_drop_cell_received;
+}
+
 #ifdef TOR_UNIT_TESTS
 /* only exists for unit tests: get HSv2 stats object */
 const hs_v2_stats_t *
diff --git a/src/feature/stats/rephist.h b/src/feature/stats/rephist.h
index a51d81beb9..f595459580 100644
--- a/src/feature/stats/rephist.h
+++ b/src/feature/stats/rephist.h
@@ -192,6 +192,8 @@ uint64_t rep_hist_get_n_tcp_exhaustion(void);
 uint64_t rep_hist_get_n_read_limit_reached(void);
 uint64_t rep_hist_get_n_write_limit_reached(void);
 
+uint64_t rep_hist_get_drop_cell_received_count(void);
+
 #ifdef TOR_UNIT_TESTS
 struct hs_v2_stats_t;
 const struct hs_v2_stats_t *rep_hist_get_hs_v2_stats(void);