Ping rate limiting for HTTP2

include/grpc/impl/codegen/grpc_types.h

@@ -179,6 +179,15 @@ typedef struct {
     Larger values give lower CPU usage for large messages, but more head of line
     blocking for small messages. */
 #define GRPC_ARG_HTTP2_MAX_FRAME_SIZE "grpc.http2.max_frame_size"
+/** Minimum time (in milliseconds) between successive ping frames being sent */
+#define GRPC_ARG_HTTP2_MIN_TIME_BETWEEN_PINGS_MS \
+  "grpc.http2.min_time_between_pings_ms"
+/** How many pings can we send before needing to send a data frame or header
+    frame?
+    (0 indicates that an infinite number of pings can be sent without sending
+     a data frame or header frame) */
+#define GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA \
+  "grpc.http2.max_pings_without_data"
 /** Default authority to pass if none specified on call construction. A string.
  * */
 #define GRPC_ARG_DEFAULT_AUTHORITY "grpc.default_authority"

src/core/ext/transport/chttp2/transport/chttp2_transport.c

@@ -133,6 +133,9 @@ static void send_ping_locked(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
                              grpc_closure *on_initiate,
                              grpc_closure *on_complete);
 
+#define DEFAULT_MIN_TIME_BETWEEN_PINGS_MS 100
+#define DEFAULT_MAX_PINGS_BETWEEN_DATA 1
+
 /*******************************************************************************
  * CONSTRUCTION/DESTRUCTION/REFCOUNTING
  */
@@ -302,6 +305,12 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
   push_setting(exec_ctx, t, GRPC_CHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE,
                DEFAULT_MAX_HEADER_LIST_SIZE);
 
+  t->ping_policy = (grpc_chttp2_repeated_ping_policy){
+      .max_pings_without_data = DEFAULT_MAX_PINGS_BETWEEN_DATA,
+      .min_time_between_pings =
+          gpr_time_from_millis(DEFAULT_MIN_TIME_BETWEEN_PINGS_MS, GPR_TIMESPAN),
+  };
+
   if (channel_args) {
     for (i = 0; i < channel_args->num_args; i++) {
       if (0 == strcmp(channel_args->args[i].key,
@@ -327,6 +336,19 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
           grpc_chttp2_hpack_compressor_set_max_usable_size(&t->hpack_compressor,
                                                            (uint32_t)value);
         }
+      } else if (0 == strcmp(channel_args->args[i].key,
+                             GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA)) {
+        t->ping_policy.max_pings_without_data = grpc_channel_arg_get_integer(
+            &channel_args->args[i],
+            (grpc_integer_options){DEFAULT_MAX_PINGS_BETWEEN_DATA, 0, INT_MAX});
+      } else if (0 == strcmp(channel_args->args[i].key,
+                             GRPC_ARG_HTTP2_MIN_TIME_BETWEEN_PINGS_MS)) {
+        t->ping_policy.min_time_between_pings = gpr_time_from_millis(
+            grpc_channel_arg_get_integer(
+                &channel_args->args[i],
+                (grpc_integer_options){DEFAULT_MIN_TIME_BETWEEN_PINGS_MS, 0,
+                                       INT_MAX}),
+            GPR_TIMESPAN);
       } else {
         static const struct {
           const char *channel_arg_name;
@@ -1913,8 +1935,8 @@ static void read_action_locked(grpc_exec_ctx *exec_ctx, void *tp,
       gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC);
       gpr_timespec dt_timespec = gpr_time_sub(now, t->last_pid_update);
       double dt = (double)dt_timespec.tv_sec + dt_timespec.tv_nsec * 1e-9;
-      if (dt > 3) {
-        grpc_pid_controller_reset(&t->pid_controller);
+      if (dt > 0.1) {
+        dt = 0.1;
       }
       double log2_bdp_guess =
           grpc_pid_controller_update(&t->pid_controller, bdp_error, dt);

src/core/ext/transport/chttp2/transport/internal.h

@@ -93,6 +93,16 @@ typedef struct {
   uint64_t inflight_id;
 } grpc_chttp2_ping_queue;
 
+typedef struct {
+  gpr_timespec min_time_between_pings;
+  int max_pings_without_data;
+} grpc_chttp2_repeated_ping_policy;
+
+typedef struct {
+  gpr_timespec last_ping_sent_time;
+  int pings_before_data_required;
+} grpc_chttp2_repeated_ping_state;
+
 /* deframer state for the overall http2 stream of bytes */
 typedef enum {
   /* prefix: one entry per http2 connection prefix byte */
@@ -281,6 +291,8 @@ struct grpc_chttp2_transport {
 
   /** ping queues for various ping insertion points */
   grpc_chttp2_ping_queue ping_queues[GRPC_CHTTP2_PING_TYPE_COUNT];
+  grpc_chttp2_repeated_ping_policy ping_policy;
+  grpc_chttp2_repeated_ping_state ping_state;
   uint64_t ping_ctr; /* unique id for pings */
 
   /** parser for headers */

src/core/ext/transport/chttp2/transport/writing.c

@@ -75,6 +75,17 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx,
     /* ping already in-flight: wait */
     return;
   }
+  if (t->ping_state.pings_before_data_required > 0 &&
+      t->ping_policy.max_pings_without_data != 0) {
+    /* need to send something of substance before sending a ping again */
+    return;
+  }
+  gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC);
+  if (gpr_time_cmp(gpr_time_sub(now, t->ping_state.last_ping_sent_time),
+                   t->ping_policy.min_time_between_pings) < 0) {
+    /* not enough elapsed time between successive pings */
+    return;
+  }
   /* coalesce equivalent pings into this one */
   switch (ping_type) {
     case GRPC_CHTTP2_PING_BEFORE_TRANSPORT_WINDOW_UPDATE:
@@ -92,6 +103,9 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx,
                          &pq->lists[GRPC_CHTTP2_PCL_INFLIGHT]);
   grpc_slice_buffer_add(&t->outbuf,
                         grpc_chttp2_ping_create(false, pq->inflight_id));
+  t->ping_state.last_ping_sent_time = now;
+  t->ping_state.pings_before_data_required -=
+      (t->ping_state.pings_before_data_required != 0);
 }
 
 static void update_list(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
@@ -165,6 +179,8 @@ bool grpc_chttp2_begin_write(grpc_exec_ctx *exec_ctx,
       s->sent_initial_metadata = true;
       sent_initial_metadata = true;
       now_writing = true;
+      t->ping_state.pings_before_data_required =
+          t->ping_policy.max_pings_without_data;
     }
     /* send any window updates */
     if (s->announce_window > 0) {
@@ -202,6 +218,8 @@ bool grpc_chttp2_begin_write(grpc_exec_ctx *exec_ctx,
                                         send_bytes);
           GRPC_CHTTP2_FLOW_DEBIT_TRANSPORT("write", t, outgoing_window,
                                            send_bytes);
+          t->ping_state.pings_before_data_required =
+              t->ping_policy.max_pings_without_data;
           if (is_last_frame) {
             s->send_trailing_metadata = NULL;
             s->sent_trailing_metadata = true;