Fix pings

include/grpc/impl/codegen/atm_gcc_atomic.h

@@ -25,6 +25,7 @@
 
 typedef intptr_t gpr_atm;
 #define GPR_ATM_MAX INTPTR_MAX
+#define GPR_ATM_MIN INTPTR_MIN
 
 #ifdef GPR_LOW_LEVEL_COUNTERS
 extern gpr_atm gpr_counter_atm_cas;

include/grpc/impl/codegen/atm_gcc_sync.h

@@ -25,6 +25,7 @@
 
 typedef intptr_t gpr_atm;
 #define GPR_ATM_MAX INTPTR_MAX
+#define GPR_ATM_MIN INTPTR_MIN
 
 #define GPR_ATM_COMPILE_BARRIER_() __asm__ __volatile__("" : : : "memory")
 

include/grpc/impl/codegen/atm_windows.h

@@ -24,6 +24,7 @@
 
 typedef intptr_t gpr_atm;
 #define GPR_ATM_MAX INTPTR_MAX
+#define GPR_ATM_MIN INTPTR_MIN
 
 #define gpr_atm_full_barrier MemoryBarrier
 

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

@@ -1766,19 +1766,15 @@ static void send_goaway(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
 void grpc_chttp2_add_ping_strike(grpc_exec_ctx *exec_ctx,
                                  grpc_chttp2_transport *t) {
   t->ping_recv_state.ping_strikes++;
-  if (t->ping_policy.max_ping_strikes != 0) {
-    gpr_log(GPR_DEBUG, "%s: PING strike %d/%d", t->peer_string,
-            t->ping_recv_state.ping_strikes, t->ping_policy.max_ping_strikes);
-    if (t->ping_recv_state.ping_strikes > t->ping_policy.max_ping_strikes) {
-      send_goaway(
-          exec_ctx, t,
-          grpc_error_set_int(
-              GRPC_ERROR_CREATE_FROM_STATIC_STRING("too_many_pings"),
-              GRPC_ERROR_INT_HTTP2_ERROR, GRPC_HTTP2_ENHANCE_YOUR_CALM));
-      /*The transport will be closed after the write is done */
-      close_transport_locked(
-          exec_ctx, t, GRPC_ERROR_CREATE_FROM_STATIC_STRING("Too many pings"));
-    }
+  if (++t->ping_recv_state.ping_strikes > t->ping_policy.max_ping_strikes &&
+      t->ping_policy.max_ping_strikes != 0) {
+    send_goaway(exec_ctx, t,
+                grpc_error_set_int(
+                    GRPC_ERROR_CREATE_FROM_STATIC_STRING("too_many_pings"),
+                    GRPC_ERROR_INT_HTTP2_ERROR, GRPC_HTTP2_ENHANCE_YOUR_CALM));
+    /*The transport will be closed after the write is done */
+    close_transport_locked(
+        exec_ctx, t, GRPC_ERROR_CREATE_FROM_STATIC_STRING("Too many pings"));
   }
 }
 

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

@@ -89,6 +89,7 @@ grpc_error *grpc_chttp2_ping_parser_parse(grpc_exec_ctx *exec_ctx, void *parser,
       grpc_chttp2_ack_ping(exec_ctx, t, p->opaque_8bytes);
     } else {
       if (!t->is_client) {
+        grpc_millis now = grpc_exec_ctx_now(exec_ctx);
         grpc_millis next_allowed_ping =
             t->ping_recv_state.last_ping_recv_time +
             t->ping_policy.min_recv_ping_interval_without_data;
@@ -102,11 +103,11 @@ grpc_error *grpc_chttp2_ping_parser_parse(grpc_exec_ctx *exec_ctx, void *parser,
               t->ping_recv_state.last_ping_recv_time + 7200 * GPR_MS_PER_SEC;
         }
 
-        if (next_allowed_ping > grpc_exec_ctx_now(exec_ctx)) {
+        if (next_allowed_ping > now) {
           grpc_chttp2_add_ping_strike(exec_ctx, t);
         }
 
-        t->ping_recv_state.last_ping_recv_time = grpc_exec_ctx_now(exec_ctx);
+        t->ping_recv_state.last_ping_recv_time = now;
       }
       if (!g_disable_ping_ack) {
         if (t->ping_ack_count == t->ping_ack_capacity) {

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

@@ -385,7 +385,7 @@ error_handler:
     t->parser_data = &s->data_parser;
     t->ping_state.pings_before_data_required =
         t->ping_policy.max_pings_without_data;
-    t->ping_state.last_ping_sent_time = 0;
+    t->ping_state.last_ping_sent_time = GRPC_MILLIS_INF_PAST;
     return GRPC_ERROR_NONE;
   } else if (grpc_error_get_int(err, GRPC_ERROR_INT_STREAM_ID, NULL)) {
     /* handle stream errors by closing the stream */
@@ -562,7 +562,7 @@ static grpc_error *init_header_frame_parser(grpc_exec_ctx *exec_ctx,
 
   t->ping_state.pings_before_data_required =
       t->ping_policy.max_pings_without_data;
-  t->ping_state.last_ping_sent_time = 0;
+  t->ping_state.last_ping_sent_time = GRPC_MILLIS_INF_PAST;
 
   /* could be a new grpc_chttp2_stream or an existing grpc_chttp2_stream */
   s = grpc_chttp2_parsing_lookup_stream(t, t->incoming_stream_id);

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

@@ -86,15 +86,12 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx,
     next_allowed_ping =
         t->ping_recv_state.last_ping_recv_time + 7200 * GPR_MS_PER_SEC;
   }
-  /* gpr_log(GPR_DEBUG, "next_allowed_ping:%d.%09d now:%d.%09d",
-          (int)next_allowed_ping.tv_sec, (int)next_allowed_ping.tv_nsec,
-          (int)now.tv_sec, (int)now.tv_nsec); */
   if (next_allowed_ping > now) {
     /* not enough elapsed time between successive pings */
     if (GRPC_TRACER_ON(grpc_http_trace) ||
         GRPC_TRACER_ON(grpc_bdp_estimator_trace)) {
       gpr_log(GPR_DEBUG,
-              "Ping delayed [%p]: not enough time elapsed since last ping",
+              "Ping delayed [%s]: not enough time elapsed since last ping",
               t->peer_string);
     }
     if (!t->ping_state.is_delayed_ping_timer_set) {
@@ -269,7 +266,7 @@ grpc_chttp2_begin_write_result grpc_chttp2_begin_write(
                                   s->send_initial_metadata, &hopt, &t->outbuf);
         now_writing = true;
         if (!t->is_client) {
-          t->ping_recv_state.last_ping_recv_time = 0;
+          t->ping_recv_state.last_ping_recv_time = GRPC_MILLIS_INF_PAST;
           t->ping_recv_state.ping_strikes = 0;
         }
         initial_metadata_writes++;
@@ -307,7 +304,7 @@ grpc_chttp2_begin_write_result grpc_chttp2_begin_write(
           &t->outbuf, grpc_chttp2_window_update_create(s->id, stream_announce,
                                                        &s->stats.outgoing));
       if (!t->is_client) {
-        t->ping_recv_state.last_ping_recv_time = 0;
+        t->ping_recv_state.last_ping_recv_time = GRPC_MILLIS_INF_PAST;
         t->ping_recv_state.ping_strikes = 0;
       }
       flow_control_writes++;
@@ -491,7 +488,7 @@ grpc_chttp2_begin_write_result grpc_chttp2_begin_write(
         &t->outbuf, grpc_chttp2_window_update_create(0, transport_announce,
                                                      &throwaway_stats));
     if (!t->is_client) {
-      t->ping_recv_state.last_ping_recv_time = 0;
+      t->ping_recv_state.last_ping_recv_time = GRPC_MILLIS_INF_PAST;
       t->ping_recv_state.ping_strikes = 0;
     }
   }

src/core/lib/iomgr/exec_ctx.h

@@ -27,6 +27,7 @@
 typedef gpr_atm grpc_millis;
 
 #define GRPC_MILLIS_INF_FUTURE GPR_ATM_MAX
+#define GRPC_MILLIS_INF_PAST GPR_ATM_MIN
 
 /** A workqueue represents a list of work to be executed asynchronously.
     Forward declared here to avoid a circular dependency with workqueue.h. */