Fix bytesWritten return

Summary: Turns out these functions have been returning 0 for bytes written for a while. Nothing was using them (yet), so there wasn't any functional breakage.

Reviewed By: kvtsoy

Differential Revision: D46653336

fbshipit-source-id: 765b3363c1fc0729e8239d1293ddcc4ae4bb9c79

quic/api/QuicTransportFunctions.cpp

@@ -129,6 +129,7 @@ WriteQuicDataResult writeQuicDataToSocketImpl(
   WriteQuicDataResult result;
   auto& packetsWritten = result.packetsWritten;
   auto& probesWritten = result.probesWritten;
+  auto& bytesWritten = result.bytesWritten;
   auto& numProbePackets =
       connection.pendingEvents.numProbePackets[PacketNumberSpace::AppData];
   if (numProbePackets) {
@@ -149,7 +150,7 @@ WriteQuicDataResult writeQuicDataToSocketImpl(
       probeSchedulerBuilder.cryptoFrames();
     }
     auto probeScheduler = std::move(probeSchedulerBuilder).build();
-    probesWritten = writeProbingDataToSocket(
+    auto probeResult = writeProbingDataToSocket(
         sock,
         connection,
         srcConnId,
@@ -162,6 +163,8 @@ WriteQuicDataResult writeQuicDataToSocketImpl(
         aead,
         headerCipher,
         version);
+    probesWritten = probeResult.probesWritten;
+    bytesWritten += probeResult.bytesWritten;
     // We only get one chance to write out the probes.
     numProbePackets = 0;
     packetLimit =
@@ -186,21 +189,22 @@ WriteQuicDataResult writeQuicDataToSocketImpl(
     schedulerBuilder.cryptoFrames();
   }
   FrameScheduler scheduler = std::move(schedulerBuilder).build();
-  packetsWritten = writeConnectionDataToSocket(
-                       sock,
-                       connection,
-                       srcConnId,
-                       dstConnId,
-                       std::move(builder),
-                       PacketNumberSpace::AppData,
-                       scheduler,
-                       congestionControlWritableBytes,
-                       packetLimit,
-                       aead,
-                       headerCipher,
-                       version,
-                       writeLoopBeginTime)
-                       .packetsWritten;
+  auto connectionDataResult = writeConnectionDataToSocket(
+      sock,
+      connection,
+      srcConnId,
+      dstConnId,
+      std::move(builder),
+      PacketNumberSpace::AppData,
+      scheduler,
+      congestionControlWritableBytes,
+      packetLimit,
+      aead,
+      headerCipher,
+      version,
+      writeLoopBeginTime);
+  packetsWritten += connectionDataResult.packetsWritten;
+  bytesWritten += connectionDataResult.bytesWritten;
   VLOG_IF(10, packetsWritten || probesWritten)
       << nodeToString(connection.nodeType) << " written data "
       << (exceptCryptoStream ? "without crypto data " : "")
@@ -1022,7 +1026,7 @@ WriteQuicDataResult writeCryptoAndAckDataToSocket(
           .numProbePackets[LongHeader::typeToPacketNumberSpace(packetType)];
   if (numProbePackets &&
       (cryptoStream.retransmissionBuffer.size() || scheduler.hasData())) {
-    probesWritten = writeProbingDataToSocket(
+    auto probeResult = writeProbingDataToSocket(
         sock,
         connection,
         srcConnId,
@@ -1036,6 +1040,8 @@ WriteQuicDataResult writeCryptoAndAckDataToSocket(
         headerCipher,
         version,
         token);
+    probesWritten += probeResult.probesWritten;
+    bytesWritten += probeResult.bytesWritten;
   }
   packetLimit = probesWritten > packetLimit ? 0 : (packetLimit - probesWritten);
   // Only get one chance to write probes.
@@ -1491,7 +1497,7 @@ WriteQuicDataResult writeConnectionDataToSocket(
   return {ioBufBatch.getPktSent(), 0, bytesWritten};
 }
 
-uint64_t writeProbingDataToSocket(
+WriteQuicDataResult writeProbingDataToSocket(
     folly::AsyncUDPSocket& sock,
     QuicConnectionStateBase& connection,
     const ConnectionId& srcConnId,
@@ -1519,24 +1525,25 @@ uint64_t writeProbingDataToSocket(
     probesToSend = std::max<uint8_t>(probesToSend, kPacketToSendForPTO);
     dataProbesToSend = probesToSend - 1;
   }
-  auto probesWritten = writeConnectionDataToSocket(
-                           sock,
-                           connection,
-                           srcConnId,
-                           dstConnId,
-                           builder,
-                           pnSpace,
-                           cloningScheduler,
-                           connection.transportSettings.enableWritableBytesLimit
-                               ? probePacketWritableBytes
-                               : unlimitedWritableBytes,
-                           dataProbesToSend,
-                           aead,
-                           headerCipher,
-                           version,
-                           writeLoopBeginTime,
-                           token)
-                           .packetsWritten;
+  auto cloningResult = writeConnectionDataToSocket(
+      sock,
+      connection,
+      srcConnId,
+      dstConnId,
+      builder,
+      pnSpace,
+      cloningScheduler,
+      connection.transportSettings.enableWritableBytesLimit
+          ? probePacketWritableBytes
+          : unlimitedWritableBytes,
+      dataProbesToSend,
+      aead,
+      headerCipher,
+      version,
+      writeLoopBeginTime,
+      token);
+  auto probesWritten = cloningResult.packetsWritten;
+  auto bytesWritten = cloningResult.bytesWritten;
   if (probesWritten < probesToSend) {
     // If we can use an IMMEDIATE_ACK, that's better than a PING.
     auto probeSchedulerBuilder = FrameScheduler::Builder(
@@ -1552,28 +1559,29 @@ uint64_t writeProbingDataToSocket(
       probeSchedulerBuilder.pingFrames();
     }
     auto probeScheduler = std::move(probeSchedulerBuilder).build();
-    probesWritten += writeConnectionDataToSocket(
-                         sock,
-                         connection,
-                         srcConnId,
-                         dstConnId,
-                         builder,
-                         pnSpace,
-                         probeScheduler,
-                         connection.transportSettings.enableWritableBytesLimit
-                             ? probePacketWritableBytes
-                             : unlimitedWritableBytes,
-                         probesToSend - probesWritten,
-                         aead,
-                         headerCipher,
-                         version,
-                         writeLoopBeginTime)
-                         .packetsWritten;
+    auto probingResult = writeConnectionDataToSocket(
+        sock,
+        connection,
+        srcConnId,
+        dstConnId,
+        builder,
+        pnSpace,
+        probeScheduler,
+        connection.transportSettings.enableWritableBytesLimit
+            ? probePacketWritableBytes
+            : unlimitedWritableBytes,
+        probesToSend - probesWritten,
+        aead,
+        headerCipher,
+        version,
+        writeLoopBeginTime);
+    probesWritten += probingResult.packetsWritten;
+    bytesWritten += probingResult.bytesWritten;
   }
   VLOG_IF(10, probesWritten > 0)
       << nodeToString(connection.nodeType)
       << " writing probes using scheduler=CloningScheduler " << connection;
-  return probesWritten;
+  return {0, probesWritten, bytesWritten};
 }
 
 WriteDataReason shouldWriteData(/*const*/ QuicConnectionStateBase& conn) {

quic/api/QuicTransportFunctions.h

@@ -295,7 +295,7 @@ WriteQuicDataResult writeConnectionDataToSocket(
     TimePoint writeLoopBeginTime,
     const std::string& token = std::string());
 
-uint64_t writeProbingDataToSocket(
+WriteQuicDataResult writeProbingDataToSocket(
     folly::AsyncUDPSocket& sock,
     QuicConnectionStateBase& connection,
     const ConnectionId& srcConnId,

quic/api/test/QuicTransportFunctionsTest.cpp

@@ -43,18 +43,19 @@ uint64_t writeProbingDataToSocketForTest(
                                            .cryptoFrames())
                                  .build();
   return writeProbingDataToSocket(
-      sock,
-      conn,
-      *conn.clientConnectionId,
-      *conn.serverConnectionId,
-      ShortHeaderBuilder(),
-      EncryptionLevel::AppData,
-      PacketNumberSpace::AppData,
-      scheduler,
-      probesToSend,
-      aead,
-      headerCipher,
-      version);
+             sock,
+             conn,
+             *conn.clientConnectionId,
+             *conn.serverConnectionId,
+             ShortHeaderBuilder(),
+             EncryptionLevel::AppData,
+             PacketNumberSpace::AppData,
+             scheduler,
+             probesToSend,
+             aead,
+             headerCipher,
+             version)
+      .probesWritten;
 }
 
 void writeCryptoDataProbesToSocketForTest(
@@ -2727,6 +2728,7 @@ TEST_F(QuicTransportFunctionsTest, WriteQuicDataToSocketLimitTest) {
       conn->transportSettings.writeConnectionDataPacketsLimit);
   EXPECT_EQ(0, res.packetsWritten);
   EXPECT_EQ(0, res.probesWritten);
+  EXPECT_EQ(0, res.bytesWritten);
 
   // Normal limit
   conn->pendingEvents.numProbePackets[PacketNumberSpace::Initial] = 0;
@@ -2734,11 +2736,13 @@ TEST_F(QuicTransportFunctionsTest, WriteQuicDataToSocketLimitTest) {
   conn->pendingEvents.numProbePackets[PacketNumberSpace::AppData] = 0;
   conn->transportSettings.writeConnectionDataPacketsLimit =
       kDefaultWriteConnectionDataPacketLimit;
+  uint64_t actualWritten = 0;
   EXPECT_CALL(*rawSocket, write(_, _))
       .Times(1)
       .WillOnce(Invoke([&](const SocketAddress&,
                            const std::unique_ptr<folly::IOBuf>& iobuf) {
         writableBytes -= iobuf->computeChainDataLength();
+        actualWritten += iobuf->computeChainDataLength();
         return iobuf->computeChainDataLength();
       }));
   EXPECT_CALL(*rawCongestionController, onPacketSent(_)).Times(1);
@@ -2755,9 +2759,11 @@ TEST_F(QuicTransportFunctionsTest, WriteQuicDataToSocketLimitTest) {
 
   EXPECT_EQ(1, res.packetsWritten);
   EXPECT_EQ(0, res.probesWritten);
+  EXPECT_EQ(actualWritten, res.bytesWritten);
 
   // Probing can exceed packet limit. In practice we limit it to
   // kPacketToSendForPTO
+  actualWritten = 0;
   conn->pendingEvents.numProbePackets[PacketNumberSpace::AppData] =
       kDefaultWriteConnectionDataPacketLimit * 2;
   writeDataToQuicStream(*stream1, buf->clone(), true);
@@ -2769,6 +2775,7 @@ TEST_F(QuicTransportFunctionsTest, WriteQuicDataToSocketLimitTest) {
       .Times(kDefaultWriteConnectionDataPacketLimit * 2)
       .WillRepeatedly(Invoke([&](const SocketAddress&,
                                  const std::unique_ptr<folly::IOBuf>& iobuf) {
+        actualWritten += iobuf->computeChainDataLength();
         return iobuf->computeChainDataLength();
       }));
   EXPECT_CALL(*rawCongestionController, onPacketSent(_))
@@ -2787,6 +2794,7 @@ TEST_F(QuicTransportFunctionsTest, WriteQuicDataToSocketLimitTest) {
 
   EXPECT_EQ(0, res.packetsWritten);
   EXPECT_EQ(kDefaultWriteConnectionDataPacketLimit * 2, res.probesWritten);
+  EXPECT_EQ(actualWritten, res.bytesWritten);
 }
 
 TEST_F(
@@ -2934,6 +2942,7 @@ TEST_F(QuicTransportFunctionsTest, NothingWritten) {
       conn->transportSettings.writeConnectionDataPacketsLimit);
   EXPECT_EQ(0, res.packetsWritten);
   EXPECT_EQ(0, res.probesWritten);
+  EXPECT_EQ(0, res.bytesWritten);
 }
 
 const QuicWriteFrame& getFirstFrameInOutstandingPackets(
@@ -3370,6 +3379,7 @@ TEST_F(QuicTransportFunctionsTest, NoCryptoProbeWriteIfNoProbeCredit) {
 
   EXPECT_EQ(1, res.packetsWritten);
   EXPECT_EQ(0, res.probesWritten);
+  EXPECT_EQ(conn->udpSendPacketLen, res.bytesWritten);
   ASSERT_EQ(1, conn->outstandings.packets.size());
   EXPECT_TRUE(getFirstOutstandingPacket(*conn, PacketNumberSpace::Initial)
                   ->metadata.isHandshake);
@@ -3469,10 +3479,12 @@ TEST_F(QuicTransportFunctionsTest, WritePureAckWhenNoWritableBytes) {
   EXPECT_CALL(*rawCongestionController, getWritableBytes())
       .WillRepeatedly(Return(0));
 
+  uint64_t actualWritten = 0;
   EXPECT_CALL(*rawSocket, write(_, _))
       .WillRepeatedly(Invoke([&](const SocketAddress&,
                                  const std::unique_ptr<folly::IOBuf>& iobuf) {
         EXPECT_LE(iobuf->computeChainDataLength(), 30);
+        actualWritten += iobuf->computeChainDataLength();
         return iobuf->computeChainDataLength();
       }));
   EXPECT_CALL(*rawCongestionController, onPacketSent(_)).Times(0);
@@ -3486,6 +3498,7 @@ TEST_F(QuicTransportFunctionsTest, WritePureAckWhenNoWritableBytes) {
       getVersion(*conn),
       conn->transportSettings.writeConnectionDataPacketsLimit);
   EXPECT_GT(res.packetsWritten, 0);
+  EXPECT_EQ(res.bytesWritten, actualWritten);
   EXPECT_EQ(0, conn->outstandings.packets.size());
 }
 
@@ -4044,7 +4057,13 @@ TEST_F(QuicTransportFunctionsTest, WriteLimitBytRttFraction) {
   auto buf = buildRandomInputData(2048 * 2048);
   writeDataToQuicStream(*stream1, buf->clone(), true);
 
-  EXPECT_CALL(*rawSocket, write(_, _)).WillRepeatedly(Return(1));
+  uint64_t actualWritten = 0;
+  EXPECT_CALL(*rawSocket, write(_, _))
+      .WillRepeatedly(Invoke([&](const SocketAddress&,
+                                 const std::unique_ptr<folly::IOBuf>& iobuf) {
+        actualWritten += iobuf->computeChainDataLength();
+        return iobuf->computeChainDataLength();
+      }));
   EXPECT_CALL(*rawCongestionController, getWritableBytes())
       .WillRepeatedly(Return(50));
   auto writeLoopBeginTime = Clock::now();
@@ -4060,6 +4079,7 @@ TEST_F(QuicTransportFunctionsTest, WriteLimitBytRttFraction) {
       writeLoopBeginTime);
 
   EXPECT_GT(1000, res.packetsWritten);
+  EXPECT_EQ(actualWritten, res.bytesWritten);
   EXPECT_EQ(res.probesWritten, 0);
 
   res = writeQuicDataToSocket(