From 73ea66d8ee54076bca12af7f6145d9ff4bccf21a Mon Sep 17 00:00:00 2001
From: Vignesh Babu <vigneshbabu@google.com>
Date: Thu, 10 Nov 2022 14:35:27 -0800
Subject: [PATCH] A http2 setting to ensure safe rollout of tcp receive buffer
 auto-sizing and peer-state based framing experiments (#31404)

* A http2 setting to ensure safe rollout of tcp receive buffer auto-sizing and peer-state based framing experiments

* fix comments + sanity + iwyu

* comments

* update per comments

* comments

* iwyu

* address comments

* remove if check
---
 include/grpc/impl/codegen/grpc_types.h        |  7 ++
 .../chttp2/transport/chttp2_transport.cc      | 46 +++++++--
 .../chttp2/transport/flow_control.cc          | 16 +++
 .../transport/chttp2/transport/flow_control.h | 27 ++++-
 .../chttp2/transport/http2_settings.cc        |  4 +-
 .../chttp2/transport/http2_settings.h         | 14 +--
 .../ext/transport/chttp2/transport/internal.h |  4 +
 .../posix_engine/posix_endpoint.cc            |  2 +-
 src/core/lib/iomgr/tcp_posix.cc               |  5 +-
 .../transport/chttp2/flow_control_test.cc     | 98 +++++++++++++++++++
 tools/codegen/core/gen_settings_ids.py        | 11 ++-
 11 files changed, 209 insertions(+), 25 deletions(-)

diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h
index 699c08d9c08..dcaf807d0b4 100644
--- a/include/grpc/impl/codegen/grpc_types.h
+++ b/include/grpc/impl/codegen/grpc_types.h
@@ -235,6 +235,13 @@ typedef struct {
 /** Should we allow receipt of true-binary data on http2 connections?
     Defaults to on (1) */
 #define GRPC_ARG_HTTP2_ENABLE_TRUE_BINARY "grpc.http2.true_binary"
+/** An experimental channel arg which determines whether the perferred crypto
+ * frame size http2 setting sent to the peer at startup. If set to 0 (false
+ * - default), the preferred frame size is not sent to the peer. Otherwise it
+ * sends a default preferred crypto frame size value of 4GB to the peer at
+ * the startup of each connection. */
+#define GRPC_ARG_EXPERIMENTAL_HTTP2_PREFERRED_CRYPTO_FRAME_SIZE \
+  "grpc.experimental.http2.enable_preferred_frame_size"
 /** 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"
diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc
index b945fb00431..93d47da7399 100644
--- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc
+++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc
@@ -61,7 +61,6 @@
 #include "src/core/lib/channel/channel_args.h"
 #include "src/core/lib/debug/stats.h"
 #include "src/core/lib/debug/stats_data.h"
-#include "src/core/lib/experiments/experiments.h"
 #include "src/core/lib/gpr/useful.h"
 #include "src/core/lib/gprpp/bitset.h"
 #include "src/core/lib/gprpp/debug_location.h"
@@ -323,6 +322,13 @@ static void read_channel_args(grpc_chttp2_transport* t,
   t->keepalive_permit_without_calls =
       channel_args.GetBool(GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS)
           .value_or(false);
+  // Only send the prefered rx frame size http2 setting if we are instructed
+  // to auto size the buffers allocated at tcp level and we also can adjust
+  // sending frame size.
+  t->enable_preferred_rx_crypto_frame_advertisement =
+      channel_args
+          .GetBool(GRPC_ARG_EXPERIMENTAL_HTTP2_PREFERRED_CRYPTO_FRAME_SIZE)
+          .value_or(false);
 
   if (channel_args.GetBool(GRPC_ARG_ENABLE_CHANNELZ)
           .value_or(GRPC_ENABLE_CHANNELZ_DEFAULT)) {
@@ -393,6 +399,16 @@ static void read_channel_args(grpc_chttp2_transport* t,
               is_client ? "clients" : "servers");
     }
   }
+
+  if (t->enable_preferred_rx_crypto_frame_advertisement) {
+    const grpc_chttp2_setting_parameters* sp =
+        &grpc_chttp2_settings_parameters
+            [GRPC_CHTTP2_SETTINGS_GRPC_PREFERRED_RECEIVE_CRYPTO_FRAME_SIZE];
+    queue_setting_update(
+        t, GRPC_CHTTP2_SETTINGS_GRPC_PREFERRED_RECEIVE_CRYPTO_FRAME_SIZE,
+        grpc_core::Clamp(INT_MAX, static_cast<int>(sp->min_value),
+                         static_cast<int>(sp->max_value)));
+  }
 }
 
 static void init_transport_keepalive_settings(grpc_chttp2_transport* t) {
@@ -864,16 +880,17 @@ static void write_action(void* gt, grpc_error_handle /*error*/) {
   grpc_chttp2_transport* t = static_cast<grpc_chttp2_transport*>(gt);
   void* cl = t->cl;
   t->cl = nullptr;
-  // If the peer_state_based_framing experiment is set to true,
-  // choose max_frame_size as 2 * max http2 frame size of peer. If peer is under
-  // high memory pressure, then it would advertise a smaller max http2 frame
-  // size. With this logic, the sender would automatically reduce the sending
-  // frame size as well.
+  // Choose max_frame_size as the prefered rx crypto frame size indicated by the
+  // peer.
   int max_frame_size =
-      grpc_core::IsPeerStateBasedFramingEnabled()
-          ? 2 * t->settings[GRPC_PEER_SETTINGS]
-                           [GRPC_CHTTP2_SETTINGS_MAX_FRAME_SIZE]
-          : INT_MAX;
+      t->settings
+          [GRPC_PEER_SETTINGS]
+          [GRPC_CHTTP2_SETTINGS_GRPC_PREFERRED_RECEIVE_CRYPTO_FRAME_SIZE];
+  // Note: max frame size is 0 if the remote peer does not support adjusting the
+  // sending frame size.
+  if (max_frame_size == 0) {
+    max_frame_size = INT_MAX;
+  }
   grpc_endpoint_write(
       t->ep, &t->outbuf,
       GRPC_CLOSURE_INIT(&t->write_action_end_locked, write_action_end, t,
@@ -2301,6 +2318,15 @@ void grpc_chttp2_act_on_flowctl_action(
                 queue_setting_update(t, GRPC_CHTTP2_SETTINGS_MAX_FRAME_SIZE,
                                      action.max_frame_size());
               });
+  if (t->enable_preferred_rx_crypto_frame_advertisement) {
+    WithUrgency(
+        t, action.preferred_rx_crypto_frame_size_update(),
+        GRPC_CHTTP2_INITIATE_WRITE_SEND_SETTINGS, [t, &action]() {
+          queue_setting_update(
+              t, GRPC_CHTTP2_SETTINGS_GRPC_PREFERRED_RECEIVE_CRYPTO_FRAME_SIZE,
+              action.preferred_rx_crypto_frame_size());
+        });
+  }
 }
 
 static grpc_error_handle try_http_parsing(grpc_chttp2_transport* t) {
diff --git a/src/core/ext/transport/chttp2/transport/flow_control.cc b/src/core/ext/transport/chttp2/transport/flow_control.cc
index a9ff0e4a556..226c7019278 100644
--- a/src/core/ext/transport/chttp2/transport/flow_control.cc
+++ b/src/core/ext/transport/chttp2/transport/flow_control.cc
@@ -366,6 +366,22 @@ FlowControlAction TransportFlowControl::PeriodicUpdate() {
               16384, 16777215)),
           &action, &FlowControlAction::set_send_max_frame_size_update);
     }
+
+    if (IsTcpFrameSizeTuningEnabled()) {
+      // Advertise PREFERRED_RECEIVE_CRYPTO_FRAME_SIZE to peer. By advertising
+      // PREFERRED_RECEIVE_CRYPTO_FRAME_SIZE to the peer, we are informing the
+      // peer that we have tcp frame size tuning enabled and we inform it of our
+      // prefered rx frame sizes. The prefered rx frame size is determined as:
+      // Clamp(target_frame_size_ * 2, 16384, 0x7fffffff). In the future, this
+      // maybe updated to a different function of the memory pressure.
+      UpdateSetting(
+          GRPC_CHTTP2_SETTINGS_GRPC_PREFERRED_RECEIVE_CRYPTO_FRAME_SIZE,
+          &target_preferred_rx_crypto_frame_size_,
+          Clamp(static_cast<unsigned int>(target_frame_size_ * 2), 16384u,
+                0x7ffffffu),
+          &action,
+          &FlowControlAction::set_preferred_rx_crypto_frame_size_update);
+    }
   }
   return UpdateAction(action);
 }
diff --git a/src/core/ext/transport/chttp2/transport/flow_control.h b/src/core/ext/transport/chttp2/transport/flow_control.h
index 8f11971d471..48c531eb371 100644
--- a/src/core/ext/transport/chttp2/transport/flow_control.h
+++ b/src/core/ext/transport/chttp2/transport/flow_control.h
@@ -21,6 +21,7 @@
 
 #include <grpc/support/port_platform.h>
 
+#include <limits.h>
 #include <stdint.h>
 
 #include <iosfwd>
@@ -59,6 +60,7 @@ static constexpr uint32_t kMinPositiveInitialWindowSize = 1024;
 static constexpr const uint32_t kMaxInitialWindowSize = (1u << 30);
 // The maximum per-stream flow control window delta to advertise.
 static constexpr const int64_t kMaxWindowDelta = (1u << 20);
+static constexpr const int kDefaultPreferredRxCryptoFrameSize = INT_MAX;
 
 // TODO(ctiller): clean up when flow_control_fixes is enabled by default
 static constexpr uint32_t kFrameSize = 1024 * 1024;
@@ -92,8 +94,14 @@ class FlowControlAction {
   Urgency send_max_frame_size_update() const {
     return send_max_frame_size_update_;
   }
+  Urgency preferred_rx_crypto_frame_size_update() const {
+    return preferred_rx_crypto_frame_size_update_;
+  }
   uint32_t initial_window_size() const { return initial_window_size_; }
   uint32_t max_frame_size() const { return max_frame_size_; }
+  uint32_t preferred_rx_crypto_frame_size() const {
+    return preferred_rx_crypto_frame_size_;
+  }
 
   FlowControlAction& set_send_stream_update(Urgency u) {
     send_stream_update_ = u;
@@ -115,6 +123,12 @@ class FlowControlAction {
     max_frame_size_ = update;
     return *this;
   }
+  FlowControlAction& set_preferred_rx_crypto_frame_size_update(
+      Urgency u, uint32_t update) {
+    preferred_rx_crypto_frame_size_update_ = u;
+    preferred_rx_crypto_frame_size_ = update;
+    return *this;
+  }
 
   static const char* UrgencyString(Urgency u);
   std::string DebugString() const;
@@ -129,7 +143,11 @@ class FlowControlAction {
            (send_initial_window_update_ == Urgency::NO_ACTION_NEEDED ||
             initial_window_size_ == other.initial_window_size_) &&
            (send_max_frame_size_update_ == Urgency::NO_ACTION_NEEDED ||
-            max_frame_size_ == other.max_frame_size_);
+            max_frame_size_ == other.max_frame_size_) &&
+           (preferred_rx_crypto_frame_size_update_ ==
+                Urgency::NO_ACTION_NEEDED ||
+            preferred_rx_crypto_frame_size_ ==
+                other.preferred_rx_crypto_frame_size_);
   }
 
  private:
@@ -137,8 +155,10 @@ class FlowControlAction {
   Urgency send_transport_update_ = Urgency::NO_ACTION_NEEDED;
   Urgency send_initial_window_update_ = Urgency::NO_ACTION_NEEDED;
   Urgency send_max_frame_size_update_ = Urgency::NO_ACTION_NEEDED;
+  Urgency preferred_rx_crypto_frame_size_update_ = Urgency::NO_ACTION_NEEDED;
   uint32_t initial_window_size_ = 0;
   uint32_t max_frame_size_ = 0;
+  uint32_t preferred_rx_crypto_frame_size_ = 0;
 };
 
 std::ostream& operator<<(std::ostream& out, FlowControlAction::Urgency urgency);
@@ -232,6 +252,9 @@ class TransportFlowControl final {
 
   int64_t target_window() const;
   int64_t target_frame_size() const { return target_frame_size_; }
+  int64_t target_preferred_rx_crypto_frame_size() const {
+    return target_preferred_rx_crypto_frame_size_;
+  }
 
   BdpEstimator* bdp_estimator() { return &bdp_estimator_; }
 
@@ -291,6 +314,8 @@ class TransportFlowControl final {
   int64_t remote_window_ = kDefaultWindow;
   int64_t target_initial_window_size_ = kDefaultWindow;
   int64_t target_frame_size_ = kDefaultFrameSize;
+  int64_t target_preferred_rx_crypto_frame_size_ =
+      kDefaultPreferredRxCryptoFrameSize;
   int64_t announced_window_ = kDefaultWindow;
   uint32_t acked_init_window_ = kDefaultWindow;
 };
diff --git a/src/core/ext/transport/chttp2/transport/http2_settings.cc b/src/core/ext/transport/chttp2/transport/http2_settings.cc
index 294ee8e4aa8..4fdd8519629 100644
--- a/src/core/ext/transport/chttp2/transport/http2_settings.cc
+++ b/src/core/ext/transport/chttp2/transport/http2_settings.cc
@@ -25,7 +25,7 @@
 #include "src/core/lib/gpr/useful.h"
 #include "src/core/lib/transport/http2_errors.h"
 
-const uint16_t grpc_setting_id_to_wire_id[] = {1, 2, 3, 4, 5, 6, 65027};
+const uint16_t grpc_setting_id_to_wire_id[] = {1, 2, 3, 4, 5, 6, 65027, 65028};
 
 bool grpc_wire_id_to_setting_id(uint32_t wire_id, grpc_chttp2_setting_id* out) {
   uint32_t i = wire_id - 1;
@@ -59,4 +59,6 @@ const grpc_chttp2_setting_parameters
          GRPC_CHTTP2_CLAMP_INVALID_VALUE, GRPC_HTTP2_PROTOCOL_ERROR},
         {"GRPC_ALLOW_TRUE_BINARY_METADATA", 0u, 0u, 1u,
          GRPC_CHTTP2_CLAMP_INVALID_VALUE, GRPC_HTTP2_PROTOCOL_ERROR},
+        {"GRPC_PREFERRED_RECEIVE_CRYPTO_FRAME_SIZE", 0u, 16384u, 2147483647u,
+         GRPC_CHTTP2_CLAMP_INVALID_VALUE, GRPC_HTTP2_PROTOCOL_ERROR},
 };
diff --git a/src/core/ext/transport/chttp2/transport/http2_settings.h b/src/core/ext/transport/chttp2/transport/http2_settings.h
index c96391b44ad..9e5329341fa 100644
--- a/src/core/ext/transport/chttp2/transport/http2_settings.h
+++ b/src/core/ext/transport/chttp2/transport/http2_settings.h
@@ -25,7 +25,7 @@
 
 #include <stdint.h>
 
-enum grpc_chttp2_setting_id {
+typedef enum {
   GRPC_CHTTP2_SETTINGS_HEADER_TABLE_SIZE = 0,               /* wire id 1 */
   GRPC_CHTTP2_SETTINGS_ENABLE_PUSH = 1,                     /* wire id 2 */
   GRPC_CHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS = 2,          /* wire id 3 */
@@ -33,10 +33,11 @@ enum grpc_chttp2_setting_id {
   GRPC_CHTTP2_SETTINGS_MAX_FRAME_SIZE = 4,                  /* wire id 5 */
   GRPC_CHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE = 5,            /* wire id 6 */
   GRPC_CHTTP2_SETTINGS_GRPC_ALLOW_TRUE_BINARY_METADATA = 6, /* wire id 65027 */
-};
-
-#define GRPC_CHTTP2_NUM_SETTINGS 7
+  GRPC_CHTTP2_SETTINGS_GRPC_PREFERRED_RECEIVE_CRYPTO_FRAME_SIZE =
+      7, /* wire id 65028 */
+} grpc_chttp2_setting_id;
 
+#define GRPC_CHTTP2_NUM_SETTINGS 8
 extern const uint16_t grpc_setting_id_to_wire_id[];
 
 bool grpc_wire_id_to_setting_id(uint32_t wire_id, grpc_chttp2_setting_id* out);
@@ -46,14 +47,15 @@ typedef enum {
   GRPC_CHTTP2_DISCONNECT_ON_INVALID_VALUE
 } grpc_chttp2_invalid_value_behavior;
 
-struct grpc_chttp2_setting_parameters {
+typedef struct {
   const char* name;
   uint32_t default_value;
   uint32_t min_value;
   uint32_t max_value;
   grpc_chttp2_invalid_value_behavior invalid_value_behavior;
   uint32_t error_value;
-};
+} grpc_chttp2_setting_parameters;
+
 extern const grpc_chttp2_setting_parameters
     grpc_chttp2_settings_parameters[GRPC_CHTTP2_NUM_SETTINGS];
 
diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h
index 44cf678c0c6..69aa4f23fa9 100644
--- a/src/core/ext/transport/chttp2/transport/internal.h
+++ b/src/core/ext/transport/chttp2/transport/internal.h
@@ -460,6 +460,10 @@ struct grpc_chttp2_transport
    * thereby reducing the number of induced frames. */
   uint32_t num_pending_induced_frames = 0;
   bool reading_paused_on_pending_induced_frames = false;
+  /** Based on channel args, preferred_rx_crypto_frame_sizes are advertised to
+   * the peer
+   */
+  bool enable_preferred_rx_crypto_frame_advertisement = false;
 };
 
 typedef enum {
diff --git a/src/core/lib/event_engine/posix_engine/posix_endpoint.cc b/src/core/lib/event_engine/posix_engine/posix_endpoint.cc
index fee264bbd37..b2257f6ffec 100644
--- a/src/core/lib/event_engine/posix_engine/posix_endpoint.cc
+++ b/src/core/lib/event_engine/posix_engine/posix_endpoint.cc
@@ -579,7 +579,7 @@ void PosixEndpointImpl::Read(absl::AnyInvocable<void(absl::Status)> on_read,
   incoming_buffer_->Swap(last_read_buffer_);
   read_mu_.Unlock();
   if (args != nullptr && grpc_core::IsTcpFrameSizeTuningEnabled()) {
-    min_progress_size_ = args->read_hint_bytes;
+    min_progress_size_ = std::max(static_cast<int>(args->read_hint_bytes), 1);
   } else {
     min_progress_size_ = 1;
   }
diff --git a/src/core/lib/iomgr/tcp_posix.cc b/src/core/lib/iomgr/tcp_posix.cc
index 72e1b6609ea..a2f362d1ba1 100644
--- a/src/core/lib/iomgr/tcp_posix.cc
+++ b/src/core/lib/iomgr/tcp_posix.cc
@@ -1144,8 +1144,9 @@ static void tcp_read(grpc_endpoint* ep, grpc_slice_buffer* incoming_buffer,
   tcp->read_cb = cb;
   tcp->read_mu.Lock();
   tcp->incoming_buffer = incoming_buffer;
-  tcp->min_progress_size =
-      grpc_core::IsTcpFrameSizeTuningEnabled() ? min_progress_size : 1;
+  tcp->min_progress_size = grpc_core::IsTcpFrameSizeTuningEnabled()
+                               ? std::max(min_progress_size, 1)
+                               : 1;
   grpc_slice_buffer_reset_and_unref(incoming_buffer);
   grpc_slice_buffer_swap(incoming_buffer, &tcp->last_read_buffer);
   TCP_REF(tcp, "read");
diff --git a/test/core/transport/chttp2/flow_control_test.cc b/test/core/transport/chttp2/flow_control_test.cc
index b1319488d62..cabf9bb400d 100644
--- a/test/core/transport/chttp2/flow_control_test.cc
+++ b/test/core/transport/chttp2/flow_control_test.cc
@@ -18,18 +18,71 @@
 
 #include "gtest/gtest.h"
 
+#include <grpc/grpc.h>
+#include <grpc/support/time.h>
+
+#include "src/core/lib/experiments/experiments.h"
+#include "src/core/lib/gpr/useful.h"
 #include "src/core/lib/gprpp/ref_counted_ptr.h"
+#include "src/core/lib/gprpp/time.h"
 #include "src/core/lib/iomgr/exec_ctx.h"
 #include "src/core/lib/resource_quota/resource_quota.h"
+#include "src/core/lib/transport/bdp_estimator.h"
+
+extern gpr_timespec (*gpr_now_impl)(gpr_clock_type clock_type);
 
 namespace grpc_core {
 namespace chttp2 {
 
 namespace {
+
+constexpr uint64_t kMaxAdvanceTimeMillis = 24ull * 365 * 3600 * 1000;
+
 auto* g_memory_owner = new MemoryOwner(
     ResourceQuota::Default()->memory_quota()->CreateMemoryOwner("test"));
+
+gpr_timespec g_now;
+gpr_timespec now_impl(gpr_clock_type clock_type) {
+  GPR_ASSERT(clock_type != GPR_TIMESPAN);
+  gpr_timespec ts = g_now;
+  ts.clock_type = clock_type;
+  return ts;
 }
 
+void InitGlobals() {
+  g_now = {1, 0, GPR_CLOCK_MONOTONIC};
+  TestOnlySetProcessEpoch(g_now);
+  gpr_now_impl = now_impl;
+}
+
+void AdvanceClockMillis(uint64_t millis) {
+  ExecCtx exec_ctx;
+  g_now = gpr_time_add(g_now, gpr_time_from_millis(Clamp(millis, uint64_t(1),
+                                                         kMaxAdvanceTimeMillis),
+                                                   GPR_TIMESPAN));
+  exec_ctx.InvalidateNow();
+}
+
+class TransportTargetWindowEstimatesMocker
+    : public chttp2::TestOnlyTransportTargetWindowEstimatesMocker {
+ public:
+  explicit TransportTargetWindowEstimatesMocker() {}
+
+  double ComputeNextTargetInitialWindowSizeFromPeriodicUpdate(
+      double current_target) override {
+    const double kSmallWindow = 16384;
+    const double kBigWindow = 1024 * 1024;
+    // Bounce back and forth between small and big initial windows.
+    if (current_target > kSmallWindow) {
+      return kSmallWindow;
+    } else {
+      return kBigWindow;
+    }
+  }
+};
+
+}  // namespace
+
 TEST(FlowControl, NoOp) {
   ExecCtx exec_ctx;
   TransportFlowControl tfc("test", true, g_memory_owner);
@@ -38,6 +91,7 @@ TEST(FlowControl, NoOp) {
   EXPECT_EQ(tfc.acked_init_window(), 65535);
   EXPECT_EQ(tfc.remote_window(), 65535);
   EXPECT_EQ(tfc.target_frame_size(), 16384);
+  EXPECT_EQ(tfc.target_preferred_rx_crypto_frame_size(), INT_MAX);
   EXPECT_EQ(sfc.remote_window_delta(), 0);
   EXPECT_EQ(sfc.min_progress_size(), 0);
   EXPECT_EQ(sfc.announced_window_delta(), 0);
@@ -47,12 +101,16 @@ TEST(FlowControl, SendData) {
   ExecCtx exec_ctx;
   TransportFlowControl tfc("test", true, g_memory_owner);
   StreamFlowControl sfc(&tfc);
+  int64_t prev_preferred_rx_frame_size =
+      tfc.target_preferred_rx_crypto_frame_size();
   {
     StreamFlowControl::OutgoingUpdateContext sfc_upd(&sfc);
     sfc_upd.SentData(1024);
   }
   EXPECT_EQ(sfc.remote_window_delta(), -1024);
   EXPECT_EQ(tfc.remote_window(), 65535 - 1024);
+  EXPECT_EQ(tfc.target_preferred_rx_crypto_frame_size(),
+            prev_preferred_rx_frame_size);
 }
 
 TEST(FlowControl, InitialTransportUpdate) {
@@ -70,15 +128,52 @@ TEST(FlowControl, InitialStreamUpdate) {
             FlowControlAction());
 }
 
+TEST(FlowControl, PeriodicUpdate) {
+  ExecCtx exec_ctx;
+  TransportFlowControl tfc("test", true, g_memory_owner);
+  constexpr int kNumPeriodicUpdates = 100;
+  Timestamp next_ping = Timestamp::Now() + Duration::Milliseconds(1000);
+  uint32_t prev_max_frame_size = tfc.target_frame_size();
+  for (int i = 0; i < kNumPeriodicUpdates; i++) {
+    BdpEstimator* bdp = tfc.bdp_estimator();
+    bdp->AddIncomingBytes(1024 + (i * 100));
+    // Advance clock till the timestamp of the next ping.
+    AdvanceClockMillis((next_ping - Timestamp::Now()).millis());
+    bdp->SchedulePing();
+    bdp->StartPing();
+    AdvanceClockMillis(10);
+    next_ping = bdp->CompletePing();
+    FlowControlAction action = tfc.PeriodicUpdate();
+    if (IsTcpFrameSizeTuningEnabled()) {
+      if (action.send_max_frame_size_update() !=
+          FlowControlAction::Urgency::NO_ACTION_NEEDED) {
+        prev_max_frame_size = action.max_frame_size();
+      }
+      EXPECT_EQ(action.preferred_rx_crypto_frame_size(),
+                Clamp(2 * prev_max_frame_size, 16384u, 0x7fffffffu));
+      EXPECT_TRUE(action.preferred_rx_crypto_frame_size_update() !=
+                  FlowControlAction::Urgency::NO_ACTION_NEEDED);
+    } else {
+      EXPECT_EQ(action.preferred_rx_crypto_frame_size(), 0);
+      EXPECT_TRUE(action.preferred_rx_crypto_frame_size_update() ==
+                  FlowControlAction::Urgency::NO_ACTION_NEEDED);
+    }
+  }
+}
+
 TEST(FlowControl, RecvData) {
   ExecCtx exec_ctx;
   TransportFlowControl tfc("test", true, g_memory_owner);
   StreamFlowControl sfc(&tfc);
   StreamFlowControl::IncomingUpdateContext sfc_upd(&sfc);
+  int64_t prev_preferred_rx_frame_size =
+      tfc.target_preferred_rx_crypto_frame_size();
   EXPECT_EQ(absl::OkStatus(), sfc_upd.RecvData(1024));
   sfc_upd.MakeAction();
   EXPECT_EQ(tfc.announced_window(), 65535 - 1024);
   EXPECT_EQ(sfc.announced_window_delta(), -1024);
+  EXPECT_EQ(tfc.target_preferred_rx_crypto_frame_size(),
+            prev_preferred_rx_frame_size);
 }
 
 TEST(FlowControl, TrackMinProgressSize) {
@@ -170,5 +265,8 @@ TEST(FlowControl, GradualReadsUpdate) {
 
 int main(int argc, char** argv) {
   ::testing::InitGoogleTest(&argc, argv);
+  grpc_core::chttp2::g_test_only_transport_target_window_estimates_mocker =
+      new grpc_core::chttp2::TransportTargetWindowEstimatesMocker();
+  grpc_core::chttp2::InitGlobals();
   return RUN_ALL_TESTS();
 }
diff --git a/tools/codegen/core/gen_settings_ids.py b/tools/codegen/core/gen_settings_ids.py
index b860d46375d..ff532a4f091 100755
--- a/tools/codegen/core/gen_settings_ids.py
+++ b/tools/codegen/core/gen_settings_ids.py
@@ -50,10 +50,12 @@ _SETTINGS = {
                 clamp_invalid_value),
     'GRPC_ALLOW_TRUE_BINARY_METADATA':
         Setting(0xfe03, 0, 0, 1, clamp_invalid_value),
+    'GRPC_PREFERRED_RECEIVE_CRYPTO_FRAME_SIZE':
+        Setting(0xfe04, 0, 16384, 0x7fffffff, clamp_invalid_value),
 }
 
 H = open('src/core/ext/transport/chttp2/transport/http2_settings.h', 'w')
-C = open('src/core/ext/transport/chttp2/transport/http2_settings.c', 'w')
+C = open('src/core/ext/transport/chttp2/transport/http2_settings.cc', 'w')
 
 
 # utility: print a big comment block into a set of files
@@ -91,14 +93,15 @@ print("#ifndef GRPC_CORE_EXT_TRANSPORT_CHTTP2_TRANSPORT_HTTP2_SETTINGS_H",
 print("#define GRPC_CORE_EXT_TRANSPORT_CHTTP2_TRANSPORT_HTTP2_SETTINGS_H",
       file=H)
 print(file=H)
+print("#include <grpc/support/port_platform.h>", file=H)
 print("#include <stdint.h>", file=H)
-print("#include <stdbool.h>", file=H)
 print(file=H)
 
+print("#include <grpc/support/port_platform.h>", file=C)
 print("#include \"src/core/ext/transport/chttp2/transport/http2_settings.h\"",
       file=C)
 print(file=C)
-print("#include <grpc/support/useful.h>", file=C)
+print("#include \"src/core/lib/gpr/useful.h\"", file=C)
 print("#include \"src/core/lib/transport/http2_errors.h\"", file=C)
 print(file=C)
 
@@ -162,7 +165,7 @@ for i, r in enumerate(p.r):
         print('case %d: h += %d; break;' % (i, r), file=C)
 print("""
   }
-  *out = (grpc_chttp2_setting_id)h;
+  *out = static_cast<grpc_chttp2_setting_id>(h);
   return h < GPR_ARRAY_SIZE(grpc_setting_id_to_wire_id) && grpc_setting_id_to_wire_id[h] == wire_id;
 }
 """ % cgargs,