From 93d38a9d8deb47c046a8e878cd259175cab0258b Mon Sep 17 00:00:00 2001
From: Joseph Beshay <jbeshay@meta.com>
Date: Wed, 12 Jun 2024 12:08:04 -0700
Subject: [PATCH] Schedule an ack immediately when a packet marked with ECN CE
 is received

Summary:
Timely reaction to congestion requires relaying any CE marks to the sender as soon as possible.

This change schedules an ack to be sent whenever incoming packets are received with CE marks. This will only happen when the readEcnOnIngress option is enabled.

Reviewed By: mjoras

Differential Revision: D58423959

fbshipit-source-id: 30f8cf8b11d0446985c2d87d7df67c24c0d5afdf
---
 quic/api/QuicTransportFunctions.cpp      | 12 ++++++++++++
 quic/api/QuicTransportFunctions.h        |  4 +++-
 quic/client/QuicClientTransport.cpp      |  1 +
 quic/server/state/ServerStateMachine.cpp |  1 +
 4 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/quic/api/QuicTransportFunctions.cpp b/quic/api/QuicTransportFunctions.cpp
index a35f3134b..2e2775e7a 100644
--- a/quic/api/QuicTransportFunctions.cpp
+++ b/quic/api/QuicTransportFunctions.cpp
@@ -2063,4 +2063,16 @@ void maybeAddPacketMark(
   }
 }
 
+void maybeScheduleAckForCongestionFeedback(
+    const ReceivedUdpPacket& receivedPacket,
+    AckState& ackState) {
+  // If the packet was marked as having encountered congestion, send an ACK
+  // immediately to ensure timely response from the peer.
+  // Note that the tosValue will be populated only if the enableEcnOnEgress
+  // transport setting is enabled.
+  if ((receivedPacket.tosValue & 0b11) == 0b11) {
+    ackState.needsToSendAckImmediately = true;
+  }
+}
+
 } // namespace quic
diff --git a/quic/api/QuicTransportFunctions.h b/quic/api/QuicTransportFunctions.h
index 2d177065e..921871759 100644
--- a/quic/api/QuicTransportFunctions.h
+++ b/quic/api/QuicTransportFunctions.h
@@ -343,5 +343,7 @@ void maybeVerifyPendingKeyUpdate(
 void maybeAddPacketMark(
     QuicConnectionStateBase& conn,
     OutstandingPacketWrapper& op);
-
+void maybeScheduleAckForCongestionFeedback(
+    const ReceivedUdpPacket& receivedPacket,
+    AckState& ackState);
 } // namespace quic
diff --git a/quic/client/QuicClientTransport.cpp b/quic/client/QuicClientTransport.cpp
index 21fbe1750..a0f8e8c79 100644
--- a/quic/client/QuicClientTransport.cpp
+++ b/quic/client/QuicClientTransport.cpp
@@ -655,6 +655,7 @@ void QuicClientTransport::processUdpPacketData(
     handshakeConfirmed(*conn_);
   }
 
+  maybeScheduleAckForCongestionFeedback(udpPacket, ackState);
   maybeHandleIncomingKeyUpdate(*conn_);
 
   // Try reading bytes off of crypto, and performing a handshake.
diff --git a/quic/server/state/ServerStateMachine.cpp b/quic/server/state/ServerStateMachine.cpp
index c918b00bc..ac485089a 100644
--- a/quic/server/state/ServerStateMachine.cpp
+++ b/quic/server/state/ServerStateMachine.cpp
@@ -1297,6 +1297,7 @@ void onServerReadDataFromOpen(
       handshakeConfirmed(conn);
     }
 
+    maybeScheduleAckForCongestionFeedback(readData.udpPacket, ackState);
     maybeHandleIncomingKeyUpdate(conn);
 
     // Update writable limit before processing the handshake data. This is so