gfe-relnote: (n/a) Only enable QUIC MTU discovery in the server->client direction. No behavior change in GFE, not protected. Currently, MTUH/MTUL enabled MTU discovery on both directions, the chrome->server direction does not work well(b/144832632), so we are disabling it. PiperOrigin-RevId: 281750464 Change-Id: I973397b1178473826ceafb20add86b40602a3466
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index 479cfaf..908fc4a 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -426,12 +426,13 @@ } max_undecryptable_packets_ = config.max_undecryptable_packets(); - if (config.HasClientSentConnectionOption(kMTUH, perspective_)) { + if (config.HasClientRequestedIndependentOption(kMTUH, perspective_)) { SetMtuDiscoveryTarget(kMtuDiscoveryTargetPacketSizeHigh); } - if (config.HasClientSentConnectionOption(kMTUL, perspective_)) { + if (config.HasClientRequestedIndependentOption(kMTUL, perspective_)) { SetMtuDiscoveryTarget(kMtuDiscoveryTargetPacketSizeLow); } + if (debug_visitor_ != nullptr) { debug_visitor_->OnSetFromConfig(config); } @@ -952,7 +953,11 @@ largest_acked > GetLargestSentPacket()) { QUIC_DLOG(WARNING) << ENDPOINT << "Peer's observed unsent packet:" << largest_acked - << " vs " << GetLargestSentPacket(); + << " vs " << GetLargestSentPacket() + << ". SupportsMultiplePacketNumberSpaces():" + << SupportsMultiplePacketNumberSpaces() + << ", last_decrypted_packet_level_:" + << last_decrypted_packet_level_; // We got an ack for data we have not sent. CloseConnection(QUIC_INVALID_ACK_DATA, "Largest observed too high.", ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); @@ -3416,6 +3421,7 @@ } void QuicConnection::SetMtuDiscoveryTarget(QuicByteCount target) { + QUIC_DVLOG(2) << ENDPOINT << "SetMtuDiscoveryTarget: " << target; if (mtu_discovery_v2_) { mtu_discoverer_.Disable(); mtu_discoverer_.Enable(max_packet_length(),
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index f5f1287..c23e2b8 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -773,14 +773,14 @@ } // Enable path MTU discovery. Assumes that the test is performed from the - // client perspective and the higher value of MTU target is used. + // server perspective and the higher value of MTU target is used. void EnablePathMtuDiscovery(MockSendAlgorithm* send_algorithm) { - ASSERT_EQ(Perspective::IS_CLIENT, perspective()); + ASSERT_EQ(Perspective::IS_SERVER, perspective()); QuicConfig config; QuicTagVector connection_options; connection_options.push_back(kMTUH); - config.SetConnectionOptionsToSend(connection_options); + config.SetInitialReceivedConnectionOptions(connection_options); EXPECT_CALL(*send_algorithm, SetFromConfig(_, _)); SetFromConfig(config); @@ -1116,7 +1116,10 @@ QuicFrames frames; frames.push_back(QuicFrame(frame)); QuicPacketCreatorPeer::SetSendVersionInPacket( - &peer_creator_, connection_.perspective() == Perspective::IS_SERVER); + &peer_creator_, + QuicPacketCreatorPeer::GetEncryptionLevel(&peer_creator_) < + ENCRYPTION_FORWARD_SECURE && + connection_.perspective() == Perspective::IS_SERVER); char buffer[kMaxOutgoingPacketSize]; SerializedPacket serialized_packet = @@ -1608,6 +1611,25 @@ } } + void MtuDiscoveryTestInit() { + set_perspective(Perspective::IS_SERVER); + QuicConnectionPeer::SetNegotiatedVersion(&connection_); + QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false); + connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); + peer_creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE); + // QuicFramer::GetMaxPlaintextSize uses the smallest max plaintext size + // across all encrypters. The initial encrypter used with IETF QUIC has a + // 16-byte overhead, while the NullEncrypter used throughout this test has a + // 12-byte overhead. This test tests behavior that relies on computing the + // packet size correctly, so by unsetting the initial encrypter, we avoid + // having a mismatch between the overheads for the encrypters used. In + // non-test scenarios all encrypters used for a given connection have the + // same overhead, either 12 bytes for ones using Google QUIC crypto, or 16 + // bytes for ones using TLS. + connection_.SetEncrypter(ENCRYPTION_INITIAL, nullptr); + EXPECT_TRUE(connection_.connected()); + } + QuicConnectionId connection_id_; QuicFramer framer_; @@ -4805,10 +4827,7 @@ // Tests whether sending an MTU discovery packet to peer successfully causes the // maximum packet size to increase. TEST_P(QuicConnectionTest, SendMtuDiscoveryPacket) { - if (connection_.SupportsMultiplePacketNumberSpaces()) { - return; - } - EXPECT_TRUE(connection_.connected()); + MtuDiscoveryTestInit(); // Send an MTU probe. const size_t new_mtu = kDefaultMaxPacketSize + 100; @@ -4819,17 +4838,6 @@ EXPECT_EQ(new_mtu, mtu_probe_size); EXPECT_EQ(QuicPacketNumber(1u), creator_->packet_number()); - // QuicFramer::GetMaxPlaintextSize uses the smallest max plaintext size across - // all encrypters. The initial encrypter used with IETF QUIC has a 16-byte - // overhead, while the NullEncrypter used throughout this test has a 12-byte - // overhead. This test tests behavior that relies on computing the packet size - // correctly, so by unsetting the initial encrypter, we avoid having a - // mismatch between the overheads for the encrypters used. In non-test - // scenarios all encrypters used for a given connection have the same - // overhead, either 12 bytes for ones using Google QUIC crypto, or 16 bytes - // for ones using TLS. - connection_.SetEncrypter(ENCRYPTION_INITIAL, nullptr); - // Send more than MTU worth of data. No acknowledgement was received so far, // so the MTU should be at its old value. const std::string data(kDefaultMaxPacketSize + 1, '.'); @@ -4844,7 +4852,6 @@ // Acknowledge all packets so far. QuicAckFrame probe_ack = InitAckFrame(3); - EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _)); ProcessAckPacket(&probe_ack); EXPECT_EQ(new_mtu, connection_.max_packet_length()); @@ -4858,7 +4865,7 @@ // Tests whether MTU discovery does not happen when it is not explicitly enabled // by the connection options. TEST_P(QuicConnectionTest, MtuDiscoveryDisabled) { - EXPECT_TRUE(connection_.connected()); + MtuDiscoveryTestInit(); const QuicPacketCount packets_between_probes_base = 10; set_packets_between_probes_base(packets_between_probes_base); @@ -4874,18 +4881,7 @@ // Tests whether MTU discovery works when all probes are acknowledged on the // first try. TEST_P(QuicConnectionTest, MtuDiscoveryEnabled) { - EXPECT_TRUE(connection_.connected()); - - // QuicFramer::GetMaxPlaintextSize uses the smallest max plaintext size across - // all encrypters. The initial encrypter used with IETF QUIC has a 16-byte - // overhead, while the NullEncrypter used throughout this test has a 12-byte - // overhead. This test tests behavior that relies on computing the packet size - // correctly, so by unsetting the initial encrypter, we avoid having a - // mismatch between the overheads for the encrypters used. In non-test - // scenarios all encrypters used for a given connection have the same - // overhead, either 12 bytes for ones using Google QUIC crypto, or 16 bytes - // for ones using TLS. - connection_.SetEncrypter(ENCRYPTION_INITIAL, nullptr); + MtuDiscoveryTestInit(); const QuicPacketCount packets_between_probes_base = 5; set_packets_between_probes_base(packets_between_probes_base); @@ -4919,7 +4915,6 @@ // Acknowledge all packets sent so far. QuicAckFrame probe_ack = InitAckFrame(probe_packet_number); - EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _)) .Times(AnyNumber()); ProcessAckPacket(&probe_ack); @@ -4976,9 +4971,7 @@ // Simulate the case where the first attempt to send a probe is write blocked, // and after unblock, the second attempt returns a MSG_TOO_BIG error. TEST_P(QuicConnectionTest, MtuDiscoveryWriteBlocked) { - EXPECT_TRUE(connection_.connected()); - - connection_.SetEncrypter(ENCRYPTION_INITIAL, nullptr); + MtuDiscoveryTestInit(); const QuicPacketCount packets_between_probes_base = 5; set_packets_between_probes_base(packets_between_probes_base); @@ -5018,7 +5011,7 @@ // Tests whether MTU discovery works correctly when the probes never get // acknowledged. TEST_P(QuicConnectionTest, MtuDiscoveryFailed) { - EXPECT_TRUE(connection_.connected()); + MtuDiscoveryTestInit(); // Lower the number of probes between packets in order to make the test go // much faster. @@ -5038,8 +5031,6 @@ const QuicPacketCount number_of_packets = packets_between_probes_base * (1 << (kMtuDiscoveryAttempts + 1)); std::vector<QuicPacketNumber> mtu_discovery_packets; - // Called by the first ack. - EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); // Called on many acks. EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _)) .Times(AnyNumber()); @@ -5103,18 +5094,7 @@ if (!GetQuicReloadableFlag(quic_mtu_discovery_v2)) { return; } - EXPECT_TRUE(connection_.connected()); - - // QuicFramer::GetMaxPlaintextSize uses the smallest max plaintext size across - // all encrypters. The initial encrypter used with IETF QUIC has a 16-byte - // overhead, while the NullEncrypter used throughout this test has a 12-byte - // overhead. This test tests behavior that relies on computing the packet size - // correctly, so by unsetting the initial encrypter, we avoid having a - // mismatch between the overheads for the encrypters used. In non-test - // scenarios all encrypters used for a given connection have the same - // overhead, either 12 bytes for ones using Google QUIC crypto, or 16 bytes - // for ones using TLS. - connection_.SetEncrypter(ENCRYPTION_INITIAL, nullptr); + MtuDiscoveryTestInit(); const QuicPacketCount packets_between_probes_base = 5; set_packets_between_probes_base(packets_between_probes_base); @@ -5145,7 +5125,6 @@ // Acknowledge all packets sent so far. QuicAckFrame first_ack = InitAckFrame(probe_packet_number); - EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _)) .Times(AnyNumber()); ProcessAckPacket(&first_ack); @@ -5203,18 +5182,7 @@ // Tests whether MTU discovery works when the writer has a limit on how large a // packet can be. TEST_P(QuicConnectionTest, MtuDiscoveryWriterLimited) { - EXPECT_TRUE(connection_.connected()); - - // QuicFramer::GetMaxPlaintextSize uses the smallest max plaintext size across - // all encrypters. The initial encrypter used with IETF QUIC has a 16-byte - // overhead, while the NullEncrypter used throughout this test has a 12-byte - // overhead. This test tests behavior that relies on computing the packet size - // correctly, so by unsetting the initial encrypter, we avoid having a - // mismatch between the overheads for the encrypters used. In non-test - // scenarios all encrypters used for a given connection have the same - // overhead, either 12 bytes for ones using Google QUIC crypto, or 16 bytes - // for ones using TLS. - connection_.SetEncrypter(ENCRYPTION_INITIAL, nullptr); + MtuDiscoveryTestInit(); const QuicByteCount mtu_limit = kMtuDiscoveryTargetPacketSizeHigh - 1; writer_->set_max_packet_size(mtu_limit); @@ -5251,7 +5219,6 @@ // Acknowledge all packets sent so far. QuicAckFrame probe_ack = InitAckFrame(probe_sequence_number); - EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _)) .Times(AnyNumber()); ProcessAckPacket(&probe_ack); @@ -5307,7 +5274,7 @@ // Tests whether MTU discovery works when the writer returns an error despite // advertising higher packet length. TEST_P(QuicConnectionTest, MtuDiscoveryWriterFailed) { - EXPECT_TRUE(connection_.connected()); + MtuDiscoveryTestInit(); const QuicByteCount mtu_limit = kMtuDiscoveryTargetPacketSizeHigh - 1; const QuicByteCount initial_mtu = connection_.max_packet_length(); @@ -5344,7 +5311,6 @@ // Acknowledge all packets sent so far, except for the lost probe. QuicAckFrame probe_ack = ConstructAckFrame(creator_->packet_number(), probe_number); - EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _)); ProcessAckPacket(&probe_ack); EXPECT_EQ(initial_mtu, connection_.max_packet_length()); @@ -5360,7 +5326,7 @@ } TEST_P(QuicConnectionTest, NoMtuDiscoveryAfterConnectionClosed) { - EXPECT_TRUE(connection_.connected()); + MtuDiscoveryTestInit(); const QuicPacketCount packets_between_probes_base = 10; set_packets_between_probes_base(packets_between_probes_base);
diff --git a/quic/core/quic_packet_creator.cc b/quic/core/quic_packet_creator.cc index 1a957f8..bb726e0 100644 --- a/quic/core/quic_packet_creator.cc +++ b/quic/core/quic_packet_creator.cc
@@ -697,7 +697,8 @@ MaybeAddPadding(); QUIC_DVLOG(2) << ENDPOINT << "Serializing packet " << header - << QuicFramesToString(queued_frames_); + << QuicFramesToString(queued_frames_) << " at encryption_level " + << EncryptionLevelToString(packet_.encryption_level); DCHECK_GE(max_plaintext_size_, packet_size_); // Use the packet_size_ instead of the buffer size to ensure smaller