From 5f89996e69414397a522a149b477139d9d3aadf5 Mon Sep 17 00:00:00 2001
From: Yuchen Zeng <zyc@google.com>
Date: Wed, 29 Mar 2017 17:55:28 -0700
Subject: [PATCH] Enable server-side keepalive pings

---
 include/grpc/impl/codegen/grpc_types.h        | 13 +--
 .../chttp2/transport/chttp2_transport.c       | 86 +++++++++++++------
 .../ext/transport/chttp2/transport/internal.h |  3 +-
 test/core/end2end/tests/keepalive_timeout.c   |  4 +-
 4 files changed, 71 insertions(+), 35 deletions(-)

diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h
index 07d883e3e2..204763cd53 100644
--- a/include/grpc/impl/codegen/grpc_types.h
+++ b/include/grpc/impl/codegen/grpc_types.h
@@ -214,12 +214,13 @@ typedef struct {
 /** How much data are we willing to queue up per stream if
     GRPC_WRITE_BUFFER_HINT is set? This is an upper bound */
 #define GRPC_ARG_HTTP2_WRITE_BUFFER_SIZE "grpc.http2.write_buffer_size"
-/** After a duration of this time the client pings the server to see if the
-    transport is still alive. Int valued, seconds. */
-#define GRPC_ARG_CLIENT_KEEPALIVE_TIME_MS "grpc.client_keepalive_time_ms"
-/** After waiting for a duration of this time, if the client does not receive
-    the ping ack, it will close the transport. Int valued, seconds. */
-#define GRPC_ARG_CLIENT_KEEPALIVE_TIMEOUT_MS "grpc.client_keepalive_timeout_ms"
+/** After a duration of this time the client/server pings its peer to see if the
+    transport is still alive. Int valued, milliseconds. */
+#define GRPC_ARG_KEEPALIVE_TIME_MS "grpc.keepalive_time_ms"
+/** After waiting for a duration of this time, if the keepalive ping sender does
+    not receive the ping ack, it will close the transport. Int valued,
+    milliseconds. */
+#define GRPC_ARG_KEEPALIVE_TIMEOUT_MS "grpc.keepalive_timeout_ms"
 /** Is it permissible to send keepalive pings without any outstanding streams.
     Int valued, 0(false)/1(true). */
 #define GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS \
diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
index 366cf7f2ca..8cdcc29f4e 100644
--- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c
+++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
@@ -70,13 +70,19 @@
 #define DEFAULT_MAX_HEADER_LIST_SIZE (16 * 1024)
 
 #define DEFAULT_CLIENT_KEEPALIVE_TIME_MS INT_MAX
-#define DEFAULT_CLIENT_KEEPALIVE_TIMEOUT_MS 20000
+#define DEFAULT_CLIENT_KEEPALIVE_TIMEOUT_MS 20000 /* 20 seconds */
+#define DEFAULT_SERVER_KEEPALIVE_TIME_MS 7200000  /* 2 hours */
+#define DEFAULT_SERVER_KEEPALIVE_TIMEOUT_MS 20000 /* 20 seconds */
 #define DEFAULT_KEEPALIVE_PERMIT_WITHOUT_CALLS false
 
 static int g_default_client_keepalive_time_ms =
     DEFAULT_CLIENT_KEEPALIVE_TIME_MS;
 static int g_default_client_keepalive_timeout_ms =
     DEFAULT_CLIENT_KEEPALIVE_TIMEOUT_MS;
+static int g_default_server_keepalive_time_ms =
+    DEFAULT_SERVER_KEEPALIVE_TIME_MS;
+static int g_default_server_keepalive_timeout_ms =
+    DEFAULT_SERVER_KEEPALIVE_TIMEOUT_MS;
 static bool g_default_keepalive_permit_without_calls =
     DEFAULT_KEEPALIVE_PERMIT_WITHOUT_CALLS;
 
@@ -359,17 +365,30 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
           DEFAULT_MIN_PING_INTERVAL_WITHOUT_DATA_MS, GPR_TIMESPAN),
   };
 
-  /* client-side keepalive setting */
-  t->keepalive_time =
-      g_default_client_keepalive_time_ms == INT_MAX
-          ? gpr_inf_future(GPR_TIMESPAN)
-          : gpr_time_from_millis(g_default_client_keepalive_time_ms,
-                                 GPR_TIMESPAN);
-  t->keepalive_timeout =
-      g_default_client_keepalive_timeout_ms == INT_MAX
-          ? gpr_inf_future(GPR_TIMESPAN)
-          : gpr_time_from_millis(g_default_client_keepalive_timeout_ms,
-                                 GPR_TIMESPAN);
+  /* Keepalive setting */
+  if (t->is_client) {
+    t->keepalive_time =
+        g_default_client_keepalive_time_ms == INT_MAX
+            ? gpr_inf_future(GPR_TIMESPAN)
+            : gpr_time_from_millis(g_default_client_keepalive_time_ms,
+                                   GPR_TIMESPAN);
+    t->keepalive_timeout =
+        g_default_client_keepalive_timeout_ms == INT_MAX
+            ? gpr_inf_future(GPR_TIMESPAN)
+            : gpr_time_from_millis(g_default_client_keepalive_timeout_ms,
+                                   GPR_TIMESPAN);
+  } else {
+    t->keepalive_time =
+        g_default_server_keepalive_time_ms == INT_MAX
+            ? gpr_inf_future(GPR_TIMESPAN)
+            : gpr_time_from_millis(g_default_server_keepalive_time_ms,
+                                   GPR_TIMESPAN);
+    t->keepalive_timeout =
+        g_default_server_keepalive_timeout_ms == INT_MAX
+            ? gpr_inf_future(GPR_TIMESPAN)
+            : gpr_time_from_millis(g_default_server_keepalive_timeout_ms,
+                                   GPR_TIMESPAN);
+  }
   t->keepalive_permit_without_calls = g_default_keepalive_permit_without_calls;
 
   if (channel_args) {
@@ -434,20 +453,24 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
         t->enable_bdp_probe = grpc_channel_arg_get_integer(
             &channel_args->args[i], (grpc_integer_options){1, 0, 1});
       } else if (0 == strcmp(channel_args->args[i].key,
-                             GRPC_ARG_CLIENT_KEEPALIVE_TIME_MS)) {
+                             GRPC_ARG_KEEPALIVE_TIME_MS)) {
         const int value = grpc_channel_arg_get_integer(
             &channel_args->args[i],
-            (grpc_integer_options){g_default_client_keepalive_time_ms, 1,
-                                   INT_MAX});
+            (grpc_integer_options){t->is_client
+                                       ? g_default_client_keepalive_time_ms
+                                       : g_default_server_keepalive_time_ms,
+                                   1, INT_MAX});
         t->keepalive_time = value == INT_MAX
                                 ? gpr_inf_future(GPR_TIMESPAN)
                                 : gpr_time_from_millis(value, GPR_TIMESPAN);
       } else if (0 == strcmp(channel_args->args[i].key,
-                             GRPC_ARG_CLIENT_KEEPALIVE_TIMEOUT_MS)) {
+                             GRPC_ARG_KEEPALIVE_TIMEOUT_MS)) {
         const int value = grpc_channel_arg_get_integer(
             &channel_args->args[i],
-            (grpc_integer_options){g_default_client_keepalive_timeout_ms, 0,
-                                   INT_MAX});
+            (grpc_integer_options){t->is_client
+                                       ? g_default_client_keepalive_timeout_ms
+                                       : g_default_server_keepalive_timeout_ms,
+                                   0, INT_MAX});
         t->keepalive_timeout = value == INT_MAX
                                    ? gpr_inf_future(GPR_TIMESPAN)
                                    : gpr_time_from_millis(value, GPR_TIMESPAN);
@@ -511,8 +534,8 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
   t->ping_recv_state.last_ping_recv_time = gpr_inf_past(GPR_CLOCK_MONOTONIC);
   t->ping_recv_state.ping_strikes = 0;
 
-  /* Start client-side keepalive pings */
-  if (t->is_client) {
+  /* Start keepalive pings */
+  if (gpr_time_cmp(t->keepalive_time, gpr_inf_future(GPR_TIMESPAN)) != 0) {
     t->keepalive_state = GRPC_CHTTP2_KEEPALIVE_STATE_WAITING;
     GRPC_CHTTP2_REF_TRANSPORT(t, "init keepalive ping");
     grpc_timer_init(
@@ -2170,21 +2193,32 @@ static void finish_bdp_ping_locked(grpc_exec_ctx *exec_ctx, void *tp,
   GRPC_CHTTP2_UNREF_TRANSPORT(exec_ctx, t, "bdp_ping");
 }
 
-void grpc_chttp2_config_default_keepalive_args(grpc_channel_args *args) {
+void grpc_chttp2_config_default_keepalive_args(grpc_channel_args *args,
+                                               bool is_client) {
   size_t i;
   if (args) {
     for (i = 0; i < args->num_args; i++) {
-      if (0 == strcmp(args->args[i].key, GRPC_ARG_CLIENT_KEEPALIVE_TIME_MS)) {
-        g_default_client_keepalive_time_ms = grpc_channel_arg_get_integer(
+      if (0 == strcmp(args->args[i].key, GRPC_ARG_KEEPALIVE_TIME_MS)) {
+        const int value = grpc_channel_arg_get_integer(
             &args->args[i],
             (grpc_integer_options){g_default_client_keepalive_time_ms, 1,
                                    INT_MAX});
-      } else if (0 == strcmp(args->args[i].key,
-                             GRPC_ARG_CLIENT_KEEPALIVE_TIMEOUT_MS)) {
-        g_default_client_keepalive_timeout_ms = grpc_channel_arg_get_integer(
+        if (is_client) {
+          g_default_client_keepalive_time_ms = value;
+        } else {
+          g_default_server_keepalive_time_ms = value;
+        }
+      } else if (0 ==
+                 strcmp(args->args[i].key, GRPC_ARG_KEEPALIVE_TIMEOUT_MS)) {
+        const int value = grpc_channel_arg_get_integer(
             &args->args[i],
             (grpc_integer_options){g_default_client_keepalive_timeout_ms, 0,
                                    INT_MAX});
+        if (is_client) {
+          g_default_client_keepalive_timeout_ms = value;
+        } else {
+          g_default_server_keepalive_timeout_ms = value;
+        }
       } else if (0 == strcmp(args->args[i].key,
                              GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS)) {
         g_default_keepalive_permit_without_calls =
diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h
index d38edb5bfc..6eb848b8d7 100644
--- a/src/core/ext/transport/chttp2/transport/internal.h
+++ b/src/core/ext/transport/chttp2/transport/internal.h
@@ -846,6 +846,7 @@ uint32_t grpc_chttp2_target_incoming_window(grpc_chttp2_transport *t);
 
 /** Set the default keepalive configurations, must only be called at
     initialization */
-void grpc_chttp2_config_default_keepalive_args(grpc_channel_args *args);
+void grpc_chttp2_config_default_keepalive_args(grpc_channel_args *args,
+                                               bool is_client);
 
 #endif /* GRPC_CORE_EXT_TRANSPORT_CHTTP2_TRANSPORT_INTERNAL_H */
diff --git a/test/core/end2end/tests/keepalive_timeout.c b/test/core/end2end/tests/keepalive_timeout.c
index d492585b68..4b37755959 100644
--- a/test/core/end2end/tests/keepalive_timeout.c
+++ b/test/core/end2end/tests/keepalive_timeout.c
@@ -111,10 +111,10 @@ static void test_keepalive_timeout(grpc_end2end_test_config config) {
   gpr_timespec deadline = five_seconds_time();
 
   grpc_arg keepalive_args[] = {{.type = GRPC_ARG_INTEGER,
-                                .key = GRPC_ARG_CLIENT_KEEPALIVE_TIME_MS,
+                                .key = GRPC_ARG_KEEPALIVE_TIME_MS,
                                 .value.integer = 1500},
                                {.type = GRPC_ARG_INTEGER,
-                                .key = GRPC_ARG_CLIENT_KEEPALIVE_TIMEOUT_MS,
+                                .key = GRPC_ARG_KEEPALIVE_TIMEOUT_MS,
                                 .value.integer = 0},
                                {.type = GRPC_ARG_INTEGER,
                                 .key = GRPC_ARG_HTTP2_BDP_PROBE,
-- 
GitLab