Iterate QuicVersion::MVFST

Summary:
This iterates the mvfst version to be semantically equivalent to draft-27, and leaves support for the old mvfst version.

The client will not yet be moved to draft-27 by default.

Reviewed By: lnicco

Differential Revision: D20182452

fbshipit-source-id: 1e11ad7296a6cd8d15ca5ed359d9ed82af79bb17

quic/QuicConstants.cpp

@@ -67,6 +67,7 @@ std::vector<QuicVersion> filterSupportedVersions(
       std::back_inserter(filteredVersions),
       [](auto version) {
         return version == QuicVersion::MVFST ||
+            version == QuicVersion::MVFST_D24 ||
             version == QuicVersion::QUIC_DRAFT ||
             version == QuicVersion::MVFST_INVALID;
       });

quic/QuicConstants.h

@@ -183,7 +183,8 @@ enum class QuicNodeType : bool {
 
 enum class QuicVersion : uint32_t {
   VERSION_NEGOTIATION = 0x00000000,
-  MVFST = 0xfaceb001,
+  MVFST_D24 = 0xfaceb001,
+  MVFST = 0xfaceb002,
   QUIC_DRAFT = 0xFF00001b, // Draft-27
   MVFST_INVALID = 0xfaceb00f,
 };

quic/client/test/QuicClientTransportTest.cpp

@@ -1072,6 +1072,7 @@ INSTANTIATE_TEST_CASE_P(
     QuicClientTransportIntegrationTest,
     ::testing::Values(
         TestingParams(QuicVersion::MVFST),
+        TestingParams(QuicVersion::MVFST_D24),
         TestingParams(QuicVersion::QUIC_DRAFT),
         TestingParams(QuicVersion::QUIC_DRAFT, 0)));
 

quic/codec/Types.cpp

@@ -407,6 +407,8 @@ std::string toString(QuicVersion version) {
   switch (version) {
     case QuicVersion::VERSION_NEGOTIATION:
       return "VERSION_NEGOTIATION";
+    case QuicVersion::MVFST_D24:
+      return "MVFST_D24";
     case QuicVersion::MVFST:
       return "MVFST";
     case QuicVersion::QUIC_DRAFT:

quic/common/test/TestUtils.h

@@ -38,8 +38,8 @@ class MockClock {
   }
 };
 
-constexpr QuicVersion MVFST1 = static_cast<QuicVersion>(0xfaceb002);
-constexpr QuicVersion MVFST2 = static_cast<QuicVersion>(0xfaceb003);
+constexpr QuicVersion MVFST1 = static_cast<QuicVersion>(0xfaceb00d);
+constexpr QuicVersion MVFST2 = static_cast<QuicVersion>(0xfaceb00e);
 
 constexpr folly::StringPiece kTestHost = "host";
 

quic/fizz/handshake/FizzCryptoFactory.cpp

@@ -23,10 +23,14 @@ Buf FizzCryptoFactory::makeInitialTrafficSecret(
   auto connIdRange = folly::range(clientDestinationConnId);
   folly::StringPiece salt;
   switch (version) {
-    case QuicVersion::MVFST:
+    // Our transport version is equivalent to d-24 mostly, but we never
+    // updated the salt to avoid a version transition.
+    case QuicVersion::MVFST_D24:
       salt = kQuicDraft22Salt;
       break;
+    // The salt has not changed since d-23.
     case QuicVersion::QUIC_DRAFT:
+    case QuicVersion::MVFST:
       salt = kQuicDraft23Salt;
       break;
     default:

quic/fizz/handshake/FizzTransportParameters.h

@@ -53,7 +53,7 @@ inline fizz::Extension encodeExtension(
   fizz::Extension ext;
   ext.extension_type = fizz::ExtensionType::quic_transport_parameters;
   ext.extension_data = folly::IOBuf::create(0);
-  if (encodingVersion != QuicVersion::QUIC_DRAFT) {
+  if (encodingVersion == QuicVersion::MVFST_D24) {
     folly::io::Appender appender(ext.extension_data.get(), 40);
     fizz::detail::writeVector<uint16_t>(params.parameters, appender);
   } else {
@@ -69,7 +69,7 @@ inline fizz::Extension encodeExtension(
   fizz::Extension ext;
   ext.extension_type = fizz::ExtensionType::quic_transport_parameters;
   ext.extension_data = folly::IOBuf::create(0);
-  if (encodingVersion != QuicVersion::QUIC_DRAFT) {
+  if (encodingVersion == QuicVersion::MVFST_D24) {
     folly::io::Appender appender(ext.extension_data.get(), 40);
     fizz::detail::writeVector<uint16_t>(params.parameters, appender);
   } else {
@@ -85,7 +85,7 @@ inline fizz::Extension encodeExtension(
   fizz::Extension ext;
   ext.extension_type = fizz::ExtensionType::quic_transport_parameters;
   ext.extension_data = folly::IOBuf::create(0);
-  if (encodingVersion != QuicVersion::QUIC_DRAFT) {
+  if (encodingVersion == QuicVersion::MVFST_D24) {
     folly::io::Appender appender(ext.extension_data.get(), 40);
     fizz::detail::writeVector<uint16_t>(params.parameters, appender);
   } else {
@@ -108,7 +108,7 @@ inline folly::Optional<quic::ClientTransportParameters> getClientExtension(
   }
   quic::ClientTransportParameters parameters;
   folly::io::Cursor cursor(it->extension_data.get());
-  if (encodingVersion != quic::QuicVersion::QUIC_DRAFT) {
+  if (encodingVersion == quic::QuicVersion::MVFST_D24) {
     detail::readVector<uint16_t>(parameters.parameters, cursor);
   } else {
     decodeVarintParams(parameters.parameters, cursor);
@@ -125,7 +125,7 @@ inline folly::Optional<quic::ServerTransportParameters> getServerExtension(
   }
   quic::ServerTransportParameters parameters;
   folly::io::Cursor cursor(it->extension_data.get());
-  if (encodingVersion != quic::QuicVersion::QUIC_DRAFT) {
+  if (encodingVersion == quic::QuicVersion::MVFST_D24) {
     detail::readVector<uint16_t>(parameters.parameters, cursor);
   } else {
     decodeVarintParams(parameters.parameters, cursor);
@@ -142,7 +142,7 @@ inline folly::Optional<quic::TicketTransportParameters> getTicketExtension(
   }
   quic::TicketTransportParameters parameters;
   folly::io::Cursor cursor(it->extension_data.get());
-  if (encodingVersion != quic::QuicVersion::QUIC_DRAFT) {
+  if (encodingVersion == quic::QuicVersion::MVFST_D24) {
     detail::readVector<uint16_t>(parameters.parameters, cursor);
   } else {
     decodeVarintParams(parameters.parameters, cursor);

quic/fizz/handshake/test/FizzTransportParametersTest.cpp

@@ -60,15 +60,15 @@ StringPiece ticketParamsD27{"ffa5000604049d7f3e7d"};
 
 TEST_F(QuicExtensionsTest, TestClientParamsD24) {
   auto exts = getExtensions(clientParamsD24);
-  auto ext = getClientExtension(exts, QuicVersion::MVFST);
+  auto ext = getClientExtension(exts, QuicVersion::MVFST_D24);
   EXPECT_EQ(ext->parameters.size(), 1);
   EXPECT_EQ(
       ext->parameters[0].parameter, TransportParameterId::initial_max_data);
   EXPECT_EQ(
       *getIntegerParameter(
           TransportParameterId::initial_max_data, ext->parameters),
       494878333ULL);
-  checkEncode(std::move(*ext), clientParamsD24, QuicVersion::MVFST);
+  checkEncode(std::move(*ext), clientParamsD24, QuicVersion::MVFST_D24);
 }
 
 TEST_F(QuicExtensionsTest, TestClientParamsD27) {
@@ -86,15 +86,15 @@ TEST_F(QuicExtensionsTest, TestClientParamsD27) {
 
 TEST_F(QuicExtensionsTest, TestServerParamsD24) {
   auto exts = getExtensions(serverParamsD24);
-  auto ext = getServerExtension(exts, QuicVersion::MVFST);
+  auto ext = getServerExtension(exts, QuicVersion::MVFST_D24);
 
   EXPECT_EQ(
       ext->parameters[0].parameter, TransportParameterId::initial_max_data);
   EXPECT_EQ(
       *getIntegerParameter(
           TransportParameterId::initial_max_data, ext->parameters),
       494878333ULL);
-  checkEncode(std::move(*ext), serverParamsD24, QuicVersion::MVFST);
+  checkEncode(std::move(*ext), serverParamsD24, QuicVersion::MVFST_D24);
 }
 
 TEST_F(QuicExtensionsTest, TestServerParamsD27) {
@@ -112,7 +112,7 @@ TEST_F(QuicExtensionsTest, TestServerParamsD27) {
 
 TEST_F(QuicExtensionsTest, TestTicketParamsD24) {
   auto exts = getExtensions(ticketParamsD24);
-  auto ext = getTicketExtension(exts, QuicVersion::MVFST);
+  auto ext = getTicketExtension(exts, QuicVersion::MVFST_D24);
 
   EXPECT_EQ(ext->parameters.size(), 1);
   EXPECT_EQ(
@@ -121,7 +121,7 @@ TEST_F(QuicExtensionsTest, TestTicketParamsD24) {
       *getIntegerParameter(
           TransportParameterId::initial_max_data, ext->parameters),
       494878333ULL);
-  checkEncode(std::move(*ext), ticketParamsD24, QuicVersion::MVFST);
+  checkEncode(std::move(*ext), ticketParamsD24, QuicVersion::MVFST_D24);
 }
 
 TEST_F(QuicExtensionsTest, TestTicketParamsD27) {

quic/server/QuicServer.h

@@ -335,7 +335,7 @@ class QuicServer : public QuicServerWorker::WorkerCallback,
       const std::vector<folly::EventBase*>& evbs);
 
   std::vector<QuicVersion> supportedVersions_{
-      {QuicVersion::MVFST, QuicVersion::QUIC_DRAFT}};
+      {QuicVersion::MVFST, QuicVersion::MVFST_D24, QuicVersion::QUIC_DRAFT}};
   std::atomic<bool> shutdown_{true};
   std::shared_ptr<const fizz::server::FizzServerContext> ctx_;
   TransportSettings transportSettings_;

quic/server/state/ServerStateMachine.cpp

@@ -263,7 +263,7 @@ void updateHandshakeState(QuicServerConnectionState& conn) {
     auto doneTime = conn.readCodec->getHandshakeDoneTime();
     if (!doneTime) {
       conn.readCodec->onHandshakeDone(Clock::now());
-      if (conn.version == QuicVersion::QUIC_DRAFT) {
+      if (conn.version != QuicVersion::MVFST_D24) {
         sendSimpleFrame(conn, HandshakeDoneFrame());
       }
     }

quic/server/state/ServerStateMachine.h

@@ -128,8 +128,8 @@ struct QuicServerConnectionState : public QuicConnectionStateBase {
     // TODO: this is wrong, it should be the handshake finish time. But i need
     // a relatively sane time now to make the timestamps all sane.
     connectionTime = Clock::now();
-    supportedVersions =
-        std::vector<QuicVersion>{{QuicVersion::MVFST, QuicVersion::QUIC_DRAFT}};
+    supportedVersions = std::vector<QuicVersion>{
+        {QuicVersion::MVFST, QuicVersion::MVFST_D24, QuicVersion::QUIC_DRAFT}};
     originalVersion = QuicVersion::MVFST;
     serverHandshakeLayer = new ServerHandshake(this);
     handshakeLayer.reset(serverHandshakeLayer);

quic/server/test/QuicServerTransportTest.cpp

@@ -833,6 +833,7 @@ TEST_F(QuicServerTransportTest, IdleTimerNotResetWhenDataOutstanding) {
   // Clear the receivedNewPacketBeforeWrite flag, since we may reveice from
   // client during the SetUp of the test case.
   server->getNonConstConn().receivedNewPacketBeforeWrite = false;
+  server->getNonConstConn().outstandingPackets.clear();
   StreamId streamId = server->createBidirectionalStream().value();
 
   server->idleTimeout().cancelTimeout();