Add min_ping_interval_without_data

3 files changed, 27 insertions, 12 deletions

diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
index 195b34a377..c4deb24e41 100644
--- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c
+++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
@@ -154,6 +154,7 @@ static void retry_initiate_ping_locked(grpc_exec_ctx *exec_ctx, void *tp,
 #define DEFAULT_MIN_TIME_BETWEEN_PINGS_MS 0
 #define DEFAULT_MAX_PINGS_BETWEEN_DATA 3
 #define DEFAULT_MAX_PING_STRIKES 2
+#define DEFAULT_MIN_PING_INTERVAL_WITHOUT_DATA_S 300
 
 /** keepalive-relevant functions */
 static void init_keepalive_ping_locked(grpc_exec_ctx *exec_ctx, void *arg,
@@ -353,6 +354,8 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
       .min_time_between_pings =
           gpr_time_from_millis(DEFAULT_MIN_TIME_BETWEEN_PINGS_MS, GPR_TIMESPAN),
       .max_ping_strikes = DEFAULT_MAX_PING_STRIKES,
+      .min_ping_interval_without_data = gpr_time_from_seconds(
+          DEFAULT_MIN_PING_INTERVAL_WITHOUT_DATA_S, GPR_TIMESPAN),
   };
 
   /* client-side keepalive setting */
@@ -402,7 +405,7 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
                              GRPC_ARG_HTTP2_MAX_PING_STRIKES)) {
         t->ping_policy.max_ping_strikes = grpc_channel_arg_get_integer(
             &channel_args->args[i],
-            (grpc_integer_options){DEFAULT_MAX_PINGS_BETWEEN_DATA, 0, INT_MAX});
+            (grpc_integer_options){DEFAULT_MAX_PING_STRIKES, 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(
@@ -412,6 +415,14 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
                                        INT_MAX}),
             GPR_TIMESPAN);
       } else if (0 == strcmp(channel_args->args[i].key,
+                             GRPC_ARG_HTTP2_MIN_PING_INTERVAL_WITHOUT_DATA_S)) {
+        t->ping_policy.min_ping_interval_without_data = gpr_time_from_seconds(
+            grpc_channel_arg_get_integer(
+                &channel_args->args[i],
+                (grpc_integer_options){DEFAULT_MIN_PING_INTERVAL_WITHOUT_DATA_S,
+                                       0, INT_MAX}),
+            GPR_TIMESPAN);
+      } else if (0 == strcmp(channel_args->args[i].key,
                              GRPC_ARG_HTTP2_WRITE_BUFFER_SIZE)) {
         t->write_buffer_size = (uint32_t)grpc_channel_arg_get_integer(
             &channel_args->args[i],
diff --git a/src/core/ext/transport/chttp2/transport/frame_ping.c b/src/core/ext/transport/chttp2/transport/frame_ping.c
index 1398e55953..98f53bf361 100644
--- a/src/core/ext/transport/chttp2/transport/frame_ping.c
+++ b/src/core/ext/transport/chttp2/transport/frame_ping.c
@@ -105,19 +105,22 @@ grpc_error *grpc_chttp2_ping_parser_parse(grpc_exec_ctx *exec_ctx, void *parser,
     } else {
       if (!t->is_client) {
         gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC);
-        gpr_timespec elapsed =
-            gpr_time_sub(now, t->ping_recv_state.last_ping_recv_time);
+        gpr_timespec next_allowed_ping =
+            gpr_time_add(t->ping_recv_state.last_ping_recv_time,
+                         t->ping_policy.min_ping_interval_without_data);
+
         if (t->keepalive_permit_without_calls == 0 &&
             grpc_chttp2_stream_map_size(&t->stream_map) == 0) {
-          if (gpr_time_cmp(elapsed, gpr_time_from_seconds(7200, GPR_TIMESPAN)) <
-              0) {
-            grpc_chttp2_ping_strike(exec_ctx, t);
-          }
-        } else {
-          if (gpr_time_cmp(elapsed, t->ping_policy.min_time_between_pings) <
-              0) {
-            grpc_chttp2_ping_strike(exec_ctx, t);
-          }
+          /* The “2 hours” restricts the number of PINGS to an implementation
+             equivalent to TCP Keep-Alive, whose interval is specified to
+             default to no less than two hours in RFC1122. */
+          next_allowed_ping =
+              gpr_time_add(t->ping_recv_state.last_ping_recv_time,
+                           gpr_time_from_seconds(7200, GPR_TIMESPAN));
+        }
+
+        if (gpr_time_cmp(next_allowed_ping, now) > 0) {
+          grpc_chttp2_ping_strike(exec_ctx, t);
         }
       }
       if (!g_disable_ping_ack) {
diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h
index 889d8ce754..c302d4ac34 100644
--- a/src/core/ext/transport/chttp2/transport/internal.h
+++ b/src/core/ext/transport/chttp2/transport/internal.h
@@ -98,6 +98,7 @@ typedef struct {
   gpr_timespec min_time_between_pings;
   int max_pings_without_data;
   int max_ping_strikes;
+  gpr_timespec min_ping_interval_without_data;
 } grpc_chttp2_repeated_ping_policy;
 
 typedef struct {