QBONE Server session: remove set_send_packets_as_datagrams and stream fallback QBONE always uses datagrams. PiperOrigin-RevId: 1015864008
diff --git a/quiche/quic/qbone/qbone_session_base.cc b/quiche/quic/qbone/qbone_session_base.cc index b26831f..12c72cd 100644 --- a/quiche/quic/qbone/qbone_session_base.cc +++ b/quiche/quic/qbone/qbone_session_base.cc
@@ -7,20 +7,26 @@ #include <netinet/icmp6.h> #include <netinet/ip6.h> +#include <cstdint> #include <limits> #include <memory> #include <utility> #include "absl/strings/string_view.h" -#include "quiche/quic/core/quic_data_reader.h" +#include "absl/types/span.h" +#include "quiche/quic/core/frames/quic_stream_frame.h" +#include "quiche/quic/core/quic_config.h" +#include "quiche/quic/core/quic_connection.h" +#include "quiche/quic/core/quic_constants.h" +#include "quiche/quic/core/quic_crypto_stream.h" +#include "quiche/quic/core/quic_session.h" +#include "quiche/quic/core/quic_stream.h" #include "quiche/quic/core/quic_types.h" -#include "quiche/quic/platform/api/quic_exported_stats.h" -#include "quiche/quic/platform/api/quic_logging.h" -#include "quiche/quic/platform/api/quic_testvalue.h" +#include "quiche/quic/core/quic_versions.h" +#include "quiche/quic/platform/api/quic_bug_tracker.h" #include "quiche/quic/qbone/platform/icmp_packet.h" -#include "quiche/quic/qbone/qbone_constants.h" +#include "quiche/quic/qbone/qbone_stream.h" #include "quiche/common/platform/api/quiche_command_line_flags.h" -#include "quiche/common/platform/api/quiche_logging.h" #include "quiche/common/quiche_buffer_allocator.h" #include "quiche/common/quiche_mem_slice.h" @@ -130,60 +136,46 @@ return; } - if (send_packets_as_datagrams_) { - quiche::QuicheMemSlice slice(quiche::QuicheBuffer::Copy( - connection()->helper()->GetStreamSendBufferAllocator(), packet)); - switch (SendDatagram(absl::MakeSpan(&slice, 1), /*flush=*/true).status) { - case DATAGRAM_STATUS_SUCCESS: - break; - case DATAGRAM_STATUS_TOO_LARGE: { - if (packet.size() < sizeof(ip6_hdr)) { - QUIC_BUG(quic_bug_10987_2) - << "Dropped malformed packet: IPv6 header too short"; - break; - } - auto* header = reinterpret_cast<const ip6_hdr*>(packet.begin()); - icmp6_hdr icmp_header{}; - icmp_header.icmp6_type = ICMP6_PACKET_TOO_BIG; - icmp_header.icmp6_mtu = - connection()->GetGuaranteedLargestDatagramPayload(); - - CreateIcmpPacket(header->ip6_dst, header->ip6_src, icmp_header, packet, - [this](absl::string_view icmp_packet) { - SendErrorPacketToNetwork(icmp_packet); - }); + quiche::QuicheMemSlice slice(quiche::QuicheBuffer::Copy( + connection()->helper()->GetStreamSendBufferAllocator(), packet)); + switch (SendDatagram(absl::MakeSpan(&slice, 1), /*flush=*/true).status) { + case DATAGRAM_STATUS_SUCCESS: + break; + case DATAGRAM_STATUS_TOO_LARGE: { + if (packet.size() < sizeof(ip6_hdr)) { + QUIC_BUG(quic_bug_10987_2) + << "Dropped malformed packet: IPv6 header too short"; break; } - case DATAGRAM_STATUS_ENCRYPTION_NOT_ESTABLISHED: - QUIC_BUG(quic_bug_10987_3) - << "DATAGRAM_STATUS_ENCRYPTION_NOT_ESTABLISHED"; - break; - case DATAGRAM_STATUS_UNSUPPORTED: - QUIC_BUG(quic_bug_10987_4) << "DATAGRAM_STATUS_UNSUPPORTED"; - break; - case DATAGRAM_STATUS_BLOCKED: - QUIC_BUG(quic_bug_10987_5) << "DATAGRAM_STATUS_BLOCKED"; - break; - case DATAGRAM_STATUS_SETTINGS_NOT_RECEIVED: - QUIC_BUG(quic_bug_10987_8) << "DATAGRAM_STATUS_SETTINGS_NOT_RECEIVED"; - break; - case DATAGRAM_STATUS_INTERNAL_ERROR: - QUIC_BUG(quic_bug_10987_6) << "DATAGRAM_STATUS_INTERNAL_ERROR"; - break; + auto* header = reinterpret_cast<const ip6_hdr*>(packet.begin()); + icmp6_hdr icmp_header{}; + icmp_header.icmp6_type = ICMP6_PACKET_TOO_BIG; + icmp_header.icmp6_mtu = + connection()->GetGuaranteedLargestDatagramPayload(); + + CreateIcmpPacket(header->ip6_dst, header->ip6_src, icmp_header, packet, + [this](absl::string_view icmp_packet) { + SendErrorPacketToNetwork(icmp_packet); + }); + break; } - return; + case DATAGRAM_STATUS_ENCRYPTION_NOT_ESTABLISHED: + QUIC_BUG(quic_bug_10987_3) + << "DATAGRAM_STATUS_ENCRYPTION_NOT_ESTABLISHED"; + break; + case DATAGRAM_STATUS_UNSUPPORTED: + QUIC_BUG(quic_bug_10987_4) << "DATAGRAM_STATUS_UNSUPPORTED"; + break; + case DATAGRAM_STATUS_BLOCKED: + QUIC_BUG(quic_bug_10987_5) << "DATAGRAM_STATUS_BLOCKED"; + break; + case DATAGRAM_STATUS_SETTINGS_NOT_RECEIVED: + QUIC_BUG(quic_bug_10987_8) << "DATAGRAM_STATUS_SETTINGS_NOT_RECEIVED"; + break; + case DATAGRAM_STATUS_INTERNAL_ERROR: + QUIC_BUG(quic_bug_10987_6) << "DATAGRAM_STATUS_INTERNAL_ERROR"; + break; } - - // QBONE streams are ephemeral. - QuicStream* stream = CreateOutgoingStream(); - if (!stream) { - QUIC_BUG(quic_bug_10987_7) << "Failed to create an outgoing QBONE stream."; - return; - } - - QboneWriteOnlyStream* qbone_stream = - static_cast<QboneWriteOnlyStream*>(stream); - qbone_stream->WritePacketToQuicStream(packet); } uint64_t QboneSessionBase::GetNumEphemeralPackets() const {
diff --git a/quiche/quic/qbone/qbone_session_base.h b/quiche/quic/qbone/qbone_session_base.h index bd17dc5..1a8abe0 100644 --- a/quiche/quic/qbone/qbone_session_base.h +++ b/quiche/quic/qbone/qbone_session_base.h
@@ -5,15 +5,18 @@ #ifndef QUICHE_QUIC_QBONE_QBONE_SESSION_BASE_H_ #define QUICHE_QUIC_QBONE_QBONE_SESSION_BASE_H_ +#include <cstdint> #include <memory> #include "absl/strings/string_view.h" -#include "quiche/quic/core/quic_crypto_server_stream_base.h" +#include "quiche/quic/core/frames/quic_stream_frame.h" +#include "quiche/quic/core/quic_config.h" +#include "quiche/quic/core/quic_connection.h" #include "quiche/quic/core/quic_crypto_stream.h" -#include "quiche/quic/core/quic_error_codes.h" #include "quiche/quic/core/quic_session.h" +#include "quiche/quic/core/quic_types.h" +#include "quiche/quic/core/quic_versions.h" #include "quiche/quic/platform/api/quic_export.h" -#include "quiche/quic/qbone/qbone_packet_writer.h" #include "quiche/quic/qbone/qbone_stream.h" namespace quic { @@ -55,10 +58,6 @@ // session used an ephemeral stream instead. uint64_t GetNumFallbackToStream() const; - void set_send_packets_as_datagrams(bool send_packets_as_datagrams) { - send_packets_as_datagrams_ = send_packets_as_datagrams; - } - protected: virtual std::unique_ptr<QuicCryptoStream> CreateCryptoStream() = 0; virtual void SendErrorPacketToNetwork(absl::string_view packet) = 0; @@ -79,15 +78,10 @@ // returns an unowned pointer to the stream for convenience. QuicStream* ActivateDataStream(std::unique_ptr<QuicStream> stream); - // Accepts a given packet from the network and writes it out to the QUIC - // stream. This will send as a QUIC Datagram or create an ephemeral stream per - // packet. + // Accepts a given packet from the network and writes it out as a QUIC + // Datagram. void SendPacketToPeer(absl::string_view packet); - // If true, send QUIC DATAGRAM (aka DATAGRAM) frames instead of ephemeral - // streams. Note that receiving DATAGRAM frames is always supported. - bool send_packets_as_datagrams_ = true; - private: // Used for the crypto handshake. std::unique_ptr<QuicCryptoStream> crypto_stream_;
diff --git a/quiche/quic/qbone/qbone_session_test.cc b/quiche/quic/qbone/qbone_session_test.cc index 14e814f..1bffa4a 100644 --- a/quiche/quic/qbone/qbone_session_test.cc +++ b/quiche/quic/qbone/qbone_session_test.cc
@@ -3,6 +3,8 @@ // found in the LICENSE file. #include <cstddef> +#include <cstdint> +#include <cstring> #include <functional> #include <memory> #include <queue> @@ -12,13 +14,28 @@ #include "absl/strings/string_view.h" #include "absl/types/span.h" +#include "quiche/quic/core/crypto/proof_verifier.h" +#include "quiche/quic/core/crypto/quic_compressed_certs_cache.h" +#include "quiche/quic/core/crypto/quic_crypto_client_config.h" +#include "quiche/quic/core/crypto/quic_crypto_proof.h" #include "quiche/quic/core/crypto/quic_crypto_server_config.h" +#include "quiche/quic/core/crypto/quic_random.h" #include "quiche/quic/core/io/quic_default_event_loop.h" #include "quiche/quic/core/io/quic_event_loop.h" #include "quiche/quic/core/proto/crypto_server_config_proto.h" #include "quiche/quic/core/quic_alarm_factory.h" +#include "quiche/quic/core/quic_connection.h" #include "quiche/quic/core/quic_default_clock.h" +#include "quiche/quic/core/quic_packet_writer.h" +#include "quiche/quic/core/quic_packets.h" +#include "quiche/quic/core/quic_time.h" +#include "quiche/quic/core/quic_types.h" +#include "quiche/quic/core/quic_versions.h" #include "quiche/quic/platform/api/quic_expect_bug.h" +#include "quiche/quic/platform/api/quic_flags.h" +#include "quiche/quic/platform/api/quic_ip_address.h" +#include "quiche/quic/platform/api/quic_ip_address_family.h" +#include "quiche/quic/platform/api/quic_socket_address.h" #include "quiche/quic/platform/api/quic_test.h" #include "quiche/quic/platform/api/quic_test_loopback.h" #include "quiche/quic/qbone/bonnet/mock_qbone_client_packet_exchanger.h" @@ -26,13 +43,16 @@ #include "quiche/quic/qbone/qbone_client_session.h" #include "quiche/quic/qbone/qbone_constants.h" #include "quiche/quic/qbone/qbone_control_placeholder.pb.h" +#include "quiche/quic/qbone/qbone_control_stream.h" #include "quiche/quic/qbone/qbone_packet_processor_test_tools.h" +#include "quiche/quic/qbone/qbone_packet_writer.h" #include "quiche/quic/qbone/qbone_server_session.h" #include "quiche/quic/test_tools/crypto_test_utils.h" #include "quiche/quic/test_tools/mock_clock.h" #include "quiche/quic/test_tools/mock_connection_id_generator.h" #include "quiche/quic/test_tools/quic_connection_peer.h" #include "quiche/quic/test_tools/quic_test_utils.h" +#include "quiche/common/platform/api/quiche_reference_counted.h" #include "quiche/common/quiche_callbacks.h" namespace quic { @@ -429,99 +449,6 @@ EXPECT_THAT(written_packets, Contains(expected)); } - // Test handshake establishment and sending/receiving of data for two - // directions. - void TestStreamConnection(bool use_datagrams) { - ASSERT_TRUE(server_peer_->OneRttKeysAvailable()); - ASSERT_TRUE(client_peer_->OneRttKeysAvailable()); - ASSERT_TRUE(server_peer_->IsEncryptionEstablished()); - ASSERT_TRUE(client_peer_->IsEncryptionEstablished()); - - // Create an outgoing stream from the client and say hello. - QUIC_LOG(INFO) << "Sending client -> server"; - client_peer_->ProcessPacketFromNetwork(TestPacketIn("hello")); - client_peer_->ProcessPacketFromNetwork(TestPacketIn("world")); - runner_.Run(); - // The server should see the data, the client hasn't received - // anything yet. - EXPECT_THAT(server_writer_->data(), - ElementsAre(TestPacketOut("hello"), TestPacketOut("world"))); - EXPECT_TRUE(client_packets_to_network_.empty()); - EXPECT_EQ(0u, server_peer_->GetNumActiveStreams()); - EXPECT_EQ(0u, client_peer_->GetNumActiveStreams()); - - // Let's pretend some service responds. - QUIC_LOG(INFO) << "Sending server -> client"; - server_peer_->ProcessPacketFromNetwork(TestPacketIn("Hello Again")); - server_peer_->ProcessPacketFromNetwork(TestPacketIn("Again")); - runner_.Run(); - EXPECT_THAT(server_writer_->data(), - ElementsAre(TestPacketOut("hello"), TestPacketOut("world"))); - EXPECT_THAT( - client_packets_to_network_, - ElementsAre(TestPacketOut("Hello Again"), TestPacketOut("Again"))); - EXPECT_EQ(0u, server_peer_->GetNumActiveStreams()); - EXPECT_EQ(0u, client_peer_->GetNumActiveStreams()); - - // Try to send long payloads that are larger than the QUIC MTU but - // smaller than the QBONE max size. - // This should trigger the non-ephemeral stream code path. - std::string long_data( - QboneConstants::kMaxQbonePacketBytes - sizeof(ip6_hdr) - 1, 'A'); - QUIC_LOG(INFO) << "Sending server -> client long data"; - server_peer_->ProcessPacketFromNetwork(TestPacketIn(long_data)); - runner_.Run(); - if (use_datagrams) { - ExpectICMPTooBigResponse( - server_writer_->data(), - server_peer_->connection()->GetGuaranteedLargestDatagramPayload(), - TestPacketOut(long_data)); - } else { - EXPECT_THAT(client_packets_to_network_, - Contains(TestPacketOut(long_data))); - } - EXPECT_THAT(server_writer_->data(), - Not(Contains(TestPacketOut(long_data)))); - EXPECT_EQ(0u, server_peer_->GetNumActiveStreams()); - EXPECT_EQ(0u, client_peer_->GetNumActiveStreams()); - - QUIC_LOG(INFO) << "Sending client -> server long data"; - client_peer_->ProcessPacketFromNetwork(TestPacketIn(long_data)); - runner_.Run(); - if (use_datagrams) { - ExpectICMPTooBigResponse( - client_packets_to_network_, - client_peer_->connection()->GetGuaranteedLargestDatagramPayload(), - TestPacketIn(long_data)); - } else { - EXPECT_THAT(server_writer_->data(), Contains(TestPacketOut(long_data))); - } - EXPECT_FALSE(client_peer_->EarlyDataAccepted()); - EXPECT_FALSE(client_peer_->ReceivedInchoateReject()); - EXPECT_THAT(client_peer_->GetNumReceivedServerConfigUpdates(), Eq(0)); - - if (!use_datagrams) { - EXPECT_THAT(client_peer_->GetNumStreamedPackets(), Eq(1)); - EXPECT_THAT(server_peer_->GetNumStreamedPackets(), Eq(1)); - } - - if (use_datagrams) { - EXPECT_THAT(client_peer_->GetNumEphemeralPackets(), Eq(0)); - EXPECT_THAT(server_peer_->GetNumEphemeralPackets(), Eq(0)); - EXPECT_THAT(client_peer_->GetNumDatagramPackets(), Eq(2)); - EXPECT_THAT(server_peer_->GetNumDatagramPackets(), Eq(2)); - } else { - EXPECT_THAT(client_peer_->GetNumEphemeralPackets(), Eq(2)); - EXPECT_THAT(server_peer_->GetNumEphemeralPackets(), Eq(2)); - EXPECT_THAT(client_peer_->GetNumDatagramPackets(), Eq(0)); - EXPECT_THAT(server_peer_->GetNumDatagramPackets(), Eq(0)); - } - - // All streams are ephemeral and should be gone. - EXPECT_EQ(0u, server_peer_->GetNumActiveStreams()); - EXPECT_EQ(0u, client_peer_->GetNumActiveStreams()); - } - // Test that client and server are not connected after handshake failure. void TestDisconnectAfterFailedHandshake() { EXPECT_FALSE(client_peer_->IsEncryptionEstablished()); @@ -560,20 +487,76 @@ ::testing::ValuesIn(GetTestParams()), ::testing::PrintToStringParamName()); -TEST_P(QboneSessionTest, StreamConnection) { - CreateClientAndServerSessions(); - client_peer_->set_send_packets_as_datagrams(false); - server_peer_->set_send_packets_as_datagrams(false); - StartHandshake(); - TestStreamConnection(false); -} - TEST_P(QboneSessionTest, Datagrams) { CreateClientAndServerSessions(); - client_peer_->set_send_packets_as_datagrams(true); - server_peer_->set_send_packets_as_datagrams(true); StartHandshake(); - TestStreamConnection(true); + // Test handshake establishment and sending/receiving of data for two + // directions. + ASSERT_TRUE(server_peer_->OneRttKeysAvailable()); + ASSERT_TRUE(client_peer_->OneRttKeysAvailable()); + ASSERT_TRUE(server_peer_->IsEncryptionEstablished()); + ASSERT_TRUE(client_peer_->IsEncryptionEstablished()); + + // Create an outgoing stream from the client and say hello. + QUIC_LOG(INFO) << "Sending client -> server"; + client_peer_->ProcessPacketFromNetwork(TestPacketIn("hello")); + client_peer_->ProcessPacketFromNetwork(TestPacketIn("world")); + runner_.Run(); + // The server should see the data, the client hasn't received + // anything yet. + EXPECT_THAT(server_writer_->data(), + ElementsAre(TestPacketOut("hello"), TestPacketOut("world"))); + EXPECT_TRUE(client_packets_to_network_.empty()); + EXPECT_EQ(0u, server_peer_->GetNumActiveStreams()); + EXPECT_EQ(0u, client_peer_->GetNumActiveStreams()); + + // Let's pretend some service responds. + QUIC_LOG(INFO) << "Sending server -> client"; + server_peer_->ProcessPacketFromNetwork(TestPacketIn("Hello Again")); + server_peer_->ProcessPacketFromNetwork(TestPacketIn("Again")); + runner_.Run(); + EXPECT_THAT(server_writer_->data(), + ElementsAre(TestPacketOut("hello"), TestPacketOut("world"))); + EXPECT_THAT( + client_packets_to_network_, + ElementsAre(TestPacketOut("Hello Again"), TestPacketOut("Again"))); + EXPECT_EQ(0u, server_peer_->GetNumActiveStreams()); + EXPECT_EQ(0u, client_peer_->GetNumActiveStreams()); + + // Try to send long payloads that are larger than the QUIC MTU but + // smaller than the QBONE max size. + std::string long_data( + QboneConstants::kMaxQbonePacketBytes - sizeof(ip6_hdr) - 1, 'A'); + QUIC_LOG(INFO) << "Sending server -> client long data"; + server_peer_->ProcessPacketFromNetwork(TestPacketIn(long_data)); + runner_.Run(); + ExpectICMPTooBigResponse( + server_writer_->data(), + server_peer_->connection()->GetGuaranteedLargestDatagramPayload(), + TestPacketOut(long_data)); + EXPECT_THAT(server_writer_->data(), Not(Contains(TestPacketOut(long_data)))); + EXPECT_EQ(0u, server_peer_->GetNumActiveStreams()); + EXPECT_EQ(0u, client_peer_->GetNumActiveStreams()); + + QUIC_LOG(INFO) << "Sending client -> server long data"; + client_peer_->ProcessPacketFromNetwork(TestPacketIn(long_data)); + runner_.Run(); + ExpectICMPTooBigResponse( + client_packets_to_network_, + client_peer_->connection()->GetGuaranteedLargestDatagramPayload(), + TestPacketIn(long_data)); + EXPECT_FALSE(client_peer_->EarlyDataAccepted()); + EXPECT_FALSE(client_peer_->ReceivedInchoateReject()); + EXPECT_THAT(client_peer_->GetNumReceivedServerConfigUpdates(), Eq(0)); + + EXPECT_THAT(client_peer_->GetNumEphemeralPackets(), Eq(0)); + EXPECT_THAT(server_peer_->GetNumEphemeralPackets(), Eq(0)); + EXPECT_THAT(client_peer_->GetNumDatagramPackets(), Eq(2)); + EXPECT_THAT(server_peer_->GetNumDatagramPackets(), Eq(2)); + + // All streams are ephemeral and should be gone. + EXPECT_EQ(0u, server_peer_->GetNumActiveStreams()); + EXPECT_EQ(0u, client_peer_->GetNumActiveStreams()); } TEST_P(QboneSessionTest, ClientRejection) {