Bonnet Tun Exchanger Refactor: Drop the base class that barely did anything TunDevicePacketExchanger had a base class that didn't do anything significant. Overcomplicates my efforts to refactor the exchanger for worker-thread writes. Merge the little bit of behavior directly into TunDevicePacketExchanger and replace the base class with a pure interface. Also, while at it, move the now-interface (and its mock) into the bonnet/ subdir and rename from QbonePacketExchanger to QBONE client packet exchanger to better describe its purpose. Through some of the moved/modified logic, there may be some very minor changes to how the packets get logged/monitored, e.g. neighbor solicit packets (which are directly responded to in the exchanger) now get treated as a write packet rather than a read error. PiperOrigin-RevId: 952953687
diff --git a/build/source_list.bzl b/build/source_list.bzl index 7823208..38cbf2c 100644 --- a/build/source_list.bzl +++ b/build/source_list.bzl
@@ -1735,9 +1735,11 @@ "quic/qbone/bonnet/icmp_reachable_interface.h", "quic/qbone/bonnet/mock_icmp_reachable.h", "quic/qbone/bonnet/mock_packet_exchanger_stats_interface.h", + "quic/qbone/bonnet/mock_qbone_client_packet_exchanger.h", "quic/qbone/bonnet/mock_qbone_tunnel.h", "quic/qbone/bonnet/mock_tun_device.h", "quic/qbone/bonnet/mock_tun_device_controller.h", + "quic/qbone/bonnet/qbone_client_packet_exchanger.h", "quic/qbone/bonnet/qbone_tunnel_info.h", "quic/qbone/bonnet/qbone_tunnel_interface.h", "quic/qbone/bonnet/tun_device.h", @@ -1745,7 +1747,6 @@ "quic/qbone/bonnet/tun_device_interface.h", "quic/qbone/bonnet/tun_device_packet_exchanger.h", "quic/qbone/mock_qbone_client.h", - "quic/qbone/mock_qbone_packet_exchanger.h", "quic/qbone/mock_qbone_server_session.h", "quic/qbone/platform/icmp_packet.h", "quic/qbone/platform/ip_range.h", @@ -1761,7 +1762,6 @@ "quic/qbone/qbone_client_session.h", "quic/qbone/qbone_constants.h", "quic/qbone/qbone_control_stream.h", - "quic/qbone/qbone_packet_exchanger.h", "quic/qbone/qbone_packet_processor.h", "quic/qbone/qbone_packet_processor_test_tools.h", "quic/qbone/qbone_packet_writer.h", @@ -1797,8 +1797,6 @@ "quic/qbone/qbone_client_test.cc", "quic/qbone/qbone_constants.cc", "quic/qbone/qbone_control_stream.cc", - "quic/qbone/qbone_packet_exchanger.cc", - "quic/qbone/qbone_packet_exchanger_test.cc", "quic/qbone/qbone_packet_processor.cc", "quic/qbone/qbone_packet_processor_test.cc", "quic/qbone/qbone_packet_processor_test_tools.cc",
diff --git a/build/source_list.gni b/build/source_list.gni index cb57ef7..f23c1b9 100644 --- a/build/source_list.gni +++ b/build/source_list.gni
@@ -1740,9 +1740,11 @@ "src/quiche/quic/qbone/bonnet/icmp_reachable_interface.h", "src/quiche/quic/qbone/bonnet/mock_icmp_reachable.h", "src/quiche/quic/qbone/bonnet/mock_packet_exchanger_stats_interface.h", + "src/quiche/quic/qbone/bonnet/mock_qbone_client_packet_exchanger.h", "src/quiche/quic/qbone/bonnet/mock_qbone_tunnel.h", "src/quiche/quic/qbone/bonnet/mock_tun_device.h", "src/quiche/quic/qbone/bonnet/mock_tun_device_controller.h", + "src/quiche/quic/qbone/bonnet/qbone_client_packet_exchanger.h", "src/quiche/quic/qbone/bonnet/qbone_tunnel_info.h", "src/quiche/quic/qbone/bonnet/qbone_tunnel_interface.h", "src/quiche/quic/qbone/bonnet/tun_device.h", @@ -1750,7 +1752,6 @@ "src/quiche/quic/qbone/bonnet/tun_device_interface.h", "src/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h", "src/quiche/quic/qbone/mock_qbone_client.h", - "src/quiche/quic/qbone/mock_qbone_packet_exchanger.h", "src/quiche/quic/qbone/mock_qbone_server_session.h", "src/quiche/quic/qbone/platform/icmp_packet.h", "src/quiche/quic/qbone/platform/ip_range.h", @@ -1766,7 +1767,6 @@ "src/quiche/quic/qbone/qbone_client_session.h", "src/quiche/quic/qbone/qbone_constants.h", "src/quiche/quic/qbone/qbone_control_stream.h", - "src/quiche/quic/qbone/qbone_packet_exchanger.h", "src/quiche/quic/qbone/qbone_packet_processor.h", "src/quiche/quic/qbone/qbone_packet_processor_test_tools.h", "src/quiche/quic/qbone/qbone_packet_writer.h", @@ -1802,8 +1802,6 @@ "src/quiche/quic/qbone/qbone_client_test.cc", "src/quiche/quic/qbone/qbone_constants.cc", "src/quiche/quic/qbone/qbone_control_stream.cc", - "src/quiche/quic/qbone/qbone_packet_exchanger.cc", - "src/quiche/quic/qbone/qbone_packet_exchanger_test.cc", "src/quiche/quic/qbone/qbone_packet_processor.cc", "src/quiche/quic/qbone/qbone_packet_processor_test.cc", "src/quiche/quic/qbone/qbone_packet_processor_test_tools.cc",
diff --git a/build/source_list.json b/build/source_list.json index 8aef483..dcbd9bf 100644 --- a/build/source_list.json +++ b/build/source_list.json
@@ -1739,9 +1739,11 @@ "quiche/quic/qbone/bonnet/icmp_reachable_interface.h", "quiche/quic/qbone/bonnet/mock_icmp_reachable.h", "quiche/quic/qbone/bonnet/mock_packet_exchanger_stats_interface.h", + "quiche/quic/qbone/bonnet/mock_qbone_client_packet_exchanger.h", "quiche/quic/qbone/bonnet/mock_qbone_tunnel.h", "quiche/quic/qbone/bonnet/mock_tun_device.h", "quiche/quic/qbone/bonnet/mock_tun_device_controller.h", + "quiche/quic/qbone/bonnet/qbone_client_packet_exchanger.h", "quiche/quic/qbone/bonnet/qbone_tunnel_info.h", "quiche/quic/qbone/bonnet/qbone_tunnel_interface.h", "quiche/quic/qbone/bonnet/tun_device.h", @@ -1749,7 +1751,6 @@ "quiche/quic/qbone/bonnet/tun_device_interface.h", "quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h", "quiche/quic/qbone/mock_qbone_client.h", - "quiche/quic/qbone/mock_qbone_packet_exchanger.h", "quiche/quic/qbone/mock_qbone_server_session.h", "quiche/quic/qbone/platform/icmp_packet.h", "quiche/quic/qbone/platform/ip_range.h", @@ -1765,7 +1766,6 @@ "quiche/quic/qbone/qbone_client_session.h", "quiche/quic/qbone/qbone_constants.h", "quiche/quic/qbone/qbone_control_stream.h", - "quiche/quic/qbone/qbone_packet_exchanger.h", "quiche/quic/qbone/qbone_packet_processor.h", "quiche/quic/qbone/qbone_packet_processor_test_tools.h", "quiche/quic/qbone/qbone_packet_writer.h", @@ -1801,8 +1801,6 @@ "quiche/quic/qbone/qbone_client_test.cc", "quiche/quic/qbone/qbone_constants.cc", "quiche/quic/qbone/qbone_control_stream.cc", - "quiche/quic/qbone/qbone_packet_exchanger.cc", - "quiche/quic/qbone/qbone_packet_exchanger_test.cc", "quiche/quic/qbone/qbone_packet_processor.cc", "quiche/quic/qbone/qbone_packet_processor_test.cc", "quiche/quic/qbone/qbone_packet_processor_test_tools.cc",
diff --git a/quiche/quic/qbone/bonnet/mock_qbone_client_packet_exchanger.h b/quiche/quic/qbone/bonnet/mock_qbone_client_packet_exchanger.h new file mode 100644 index 0000000..3c5167c --- /dev/null +++ b/quiche/quic/qbone/bonnet/mock_qbone_client_packet_exchanger.h
@@ -0,0 +1,26 @@ +// Copyright 2026 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef QUICHE_QUIC_QBONE_MOCK_QBONE_PACKET_EXCHANGER_H_ +#define QUICHE_QUIC_QBONE_MOCK_QBONE_PACKET_EXCHANGER_H_ + +#include <cstddef> + +#include "quiche/quic/platform/api/quic_test.h" +#include "quiche/quic/qbone/bonnet/qbone_client_packet_exchanger.h" +#include "quiche/quic/qbone/qbone_client_interface.h" + +namespace quic::test { + +class MockQboneClientPacketExchanger : public QboneClientPacketExchanger { + public: + MOCK_METHOD(bool, ReadAndDeliverPacket, (QboneClientInterface* qbone_client), + (override)); + MOCK_METHOD(void, WritePacketToNetwork, (const char* packet, size_t size), + (override)); +}; + +} // namespace quic::test + +#endif // QUICHE_QUIC_QBONE_MOCK_QBONE_PACKET_EXCHANGER_H_
diff --git a/quiche/quic/qbone/bonnet/qbone_client_packet_exchanger.h b/quiche/quic/qbone/bonnet/qbone_client_packet_exchanger.h new file mode 100644 index 0000000..1a07a00 --- /dev/null +++ b/quiche/quic/qbone/bonnet/qbone_client_packet_exchanger.h
@@ -0,0 +1,48 @@ +// Copyright (c) 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef QUICHE_QUIC_QBONE_QBONE_PACKET_EXCHANGER_H_ +#define QUICHE_QUIC_QBONE_QBONE_PACKET_EXCHANGER_H_ + +#include <cstddef> +#include <memory> +#include <string> + +#include "absl/status/status.h" +#include "absl/strings/string_view.h" +#include "quiche/quic/qbone/qbone_client_interface.h" + +namespace quic { + +// Handles reading and writing on the local network and exchange packets between +// the local network with a QBONE connection. +class QboneClientPacketExchanger { + public: + // The owner might want to receive notifications when read or write fails. + // TODO(b/535980431): Simplify and make more generally useful, so that this + // can serve as the primary mechanism for passing out async results. + class Visitor { + public: + virtual ~Visitor() {} + virtual void OnReadError(const std::string& error) {} + virtual void OnWriteError(const std::string& error) {} + virtual absl::Status OnWrite(absl::string_view packet) { + return absl::OkStatus(); + } + }; + + virtual ~QboneClientPacketExchanger() = default; + + // Reads a packet from the local network and delivers the packet to + // qbone_client. Returns true if there may be more packets to read. + virtual bool ReadAndDeliverPacket(QboneClientInterface* qbone_client) = 0; + + // Writes a packet to the local network. If the write would be blocked, the + // packet is dropped. + virtual void WritePacketToNetwork(const char* packet, size_t size) = 0; +}; + +} // namespace quic + +#endif // QUICHE_QUIC_QBONE_QBONE_PACKET_EXCHANGER_H_
diff --git a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc index 7575359..a132b16 100644 --- a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc +++ b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc
@@ -9,78 +9,49 @@ #include <sys/uio.h> #include <algorithm> +#include <cerrno> +#include <cstddef> #include <memory> #include <string> -#include <utility> +#include "absl/base/nullability.h" +#include "absl/status/status.h" #include "absl/strings/str_cat.h" +#include "absl/strings/string_view.h" #include "absl/time/clock.h" #include "absl/time/time.h" +#include "quiche/quic/core/quic_packets.h" +#include "quiche/quic/platform/api/quic_logging.h" #include "quiche/quic/qbone/platform/icmp_packet.h" +#include "quiche/quic/qbone/platform/kernel_interface.h" #include "quiche/quic/qbone/platform/netlink_interface.h" +#include "quiche/quic/qbone/qbone_client_interface.h" #include "quiche/quic/qbone/qbone_constants.h" namespace quic { TunDevicePacketExchanger::TunDevicePacketExchanger( size_t mtu, KernelInterface* kernel, NetlinkInterface* netlink, - QbonePacketExchanger::Visitor* visitor, bool is_tap, StatsInterface* stats, + Visitor* absl_nullable visitor, bool is_tap, StatsInterface* stats, absl::string_view ifname) - : QbonePacketExchanger(visitor), - mtu_(mtu), + : mtu_(mtu), kernel_(kernel), netlink_(netlink), + visitor_(visitor), ifname_(ifname), is_tap_(is_tap), stats_(stats) {} -bool TunDevicePacketExchanger::WritePacket(const char* packet, size_t size, - std::string* error) { - if (write_fd_ < 0) { - *error = - absl::StrCat("Invalid file descriptor of the TUN device: ", write_fd_); - stats_->OnWriteError(*error); - return false; - } - - if (is_tap_ && !eth_hdr_initialized_) { - InitializeEthHdr(); - } - struct iovec iov[2]; - iov[0].iov_base = is_tap_ ? ð_hdr_ : nullptr; - iov[0].iov_len = is_tap_ ? ETH_HLEN : 0; - iov[1].iov_base = const_cast<char*>(packet); - iov[1].iov_len = size; - - absl::Time start = absl::Now(); - int result = kernel_->writev(write_fd_, iov, 2); - absl::Duration latency = std::max(absl::Now() - start, absl::ZeroDuration()); - - if (result == -1) { - if (errno == EWOULDBLOCK || errno == EAGAIN) { - // The tunnel is blocked. Note that this does not mean the receive - // buffer of a TCP connection is filled. This simply means the TUN - // device itself is blocked on handing packets to the rest of the - // kernel. - *error = - absl::ErrnoToStatus(errno, "Write to the TUN device was blocked.") - .message(); - stats_->OnWriteError(*error); - } - return false; - } - stats_->OnPacketWritten(result, latency); - - return true; -} - -std::unique_ptr<QuicData> TunDevicePacketExchanger::ReadPacket( - std::string* error) { +bool TunDevicePacketExchanger::ReadAndDeliverPacket( + QboneClientInterface* qbone_client) { if (read_fd_ < 0) { - *error = + std::string error = absl::StrCat("Invalid file descriptor of the TUN device: ", read_fd_); - stats_->OnReadError(*error); - return nullptr; + if (visitor_) { + visitor_->OnReadError(error); + } + stats_->OnReadError(error); + return false; } // Reading on a TUN device returns a packet at a time. If the packet is longer @@ -103,30 +74,120 @@ // Note that 0 means end of file, but we're talking about a TUN device - there // is no end of file. Therefore 0 also indicates error. if (result <= 0) { + std::string error; if (errno == EAGAIN || errno == EWOULDBLOCK) { - *error = + error = absl::ErrnoToStatus(errno, "Read from the TUN device was blocked.") .message(); - stats_->OnReadError(*error); + stats_->OnReadError(error); } - return nullptr; + // TODO(b/535980431): This passes an empty-string error for error codes + // other than EAGAIN/EWOULDBLOCK, matching the behavior of a previous + // implementation. Consider changing this to at least have a generic error + // for any other cases. + if (visitor_) { + visitor_->OnReadError(error); + } + return false; } if (is_tap_ && result < ETH_HLEN) { - *error = "Read packet too short for ethernet header."; - stats_->OnReadError(*error); - return nullptr; + std::string error = "Read packet too short for ethernet header."; + if (visitor_) { + visitor_->OnReadError(error); + } + stats_->OnReadError(error); + return false; } size_t l3_packet_size = is_tap_ ? result - ETH_HLEN : result; auto buffer = std::make_unique<QuicData>(read_buffer.release(), l3_packet_size, true); - if (is_tap_ && !ValidateL2Headers(eth_header, *buffer)) { - return nullptr; + if (is_tap_) { + switch (ValidateL2Headers(eth_header, *buffer)) { + case L2ValidationResult::kInvalid: { + std::string error = "Invalid L2 headers."; + if (visitor_) { + visitor_->OnReadError(error); + } + stats_->OnReadError(error); + return false; + } + case L2ValidationResult::kValidLinkLocal: + // TODO(b/535980431): This returns false to match the behavior of a + // previous implementation because no packet is forwarded to the tunnel, + // but consider changing this to true. A link-local packet does not mean + // there are no more packets to read from the TUN device. + return false; + case L2ValidationResult::kValidNormal: + // Packet is valid and should be forwarded to the tunnel. Fall through + // to normal processing. + break; + } } stats_->OnPacketRead(buffer->length(), latency); - return buffer; + qbone_client->ProcessPacketFromNetwork(buffer->AsStringPiece()); + return true; +} + +void TunDevicePacketExchanger::WritePacketToNetwork(const char* packet, + size_t size) { + if (visitor_) { + absl::Status status = visitor_->OnWrite(absl::string_view(packet, size)); + if (!status.ok()) { + QUIC_LOG_EVERY_N_SEC(ERROR, 60) << status; + } + } + + if (write_fd_ < 0) { + std::string error = + absl::StrCat("Invalid file descriptor of the TUN device: ", write_fd_); + QUIC_LOG_EVERY_N_SEC(ERROR, 60) << "Packet write failed: " << error; + if (visitor_) { + visitor_->OnWriteError(error); + } + stats_->OnWriteError(error); + return; + } + + if (is_tap_ && !eth_hdr_initialized_) { + InitializeEthHdr(); + } + struct iovec iov[2]; + iov[0].iov_base = is_tap_ ? ð_hdr_ : nullptr; + iov[0].iov_len = is_tap_ ? ETH_HLEN : 0; + iov[1].iov_base = const_cast<char*>(packet); + iov[1].iov_len = size; + + absl::Time start = absl::Now(); + int result = kernel_->writev(write_fd_, iov, 2); + absl::Duration latency = std::max(absl::Now() - start, absl::ZeroDuration()); + + if (result == -1) { + std::string error; + if (errno == EWOULDBLOCK || errno == EAGAIN) { + // The tunnel is blocked. Note that this does not mean the receive + // buffer of a TCP connection is filled. This simply means the TUN + // device itself is blocked on handing packets to the rest of the + // kernel. + error = absl::ErrnoToStatus(errno, "Write to the TUN device was blocked.") + .message(); + stats_->OnWriteError(error); + } + + // TODO(b/535980431): This logs/returns an empty-string error for error + // codes other than EAGAIN/EWOULDBLOCK, matching the behavior of a previous + // implementation. Consider changing this to at least have a generic error + // for any other cases. + QUIC_LOG_EVERY_N_SEC(ERROR, 60) << "Packet write failed: " << error; + if (visitor_) { + visitor_->OnWriteError(error); + } + return; + } + + stats_->OnPacketWritten(result, latency); } void TunDevicePacketExchanger::set_read_file_descriptor(int fd) { @@ -159,16 +220,17 @@ } } -bool TunDevicePacketExchanger::ValidateL2Headers(const ethhdr& eth_header, - const QuicData& packet) { +TunDevicePacketExchanger::L2ValidationResult +TunDevicePacketExchanger::ValidateL2Headers(const ethhdr& eth_header, + const QuicData& packet) { if (eth_header.h_proto != absl::ghtons(ETH_P_IPV6)) { - return false; + return L2ValidationResult::kInvalid; } constexpr auto kIp6PrefixLen = sizeof(ip6_hdr); constexpr auto kIcmp6PrefixLen = kIp6PrefixLen + sizeof(icmp6_hdr); if (packet.length() < kIp6PrefixLen) { // Packet is too short to be ipv6. Drop it. - return false; + return L2ValidationResult::kInvalid; } auto* ip_hdr = reinterpret_cast<const ip6_hdr*>(packet.data()); const bool is_icmp = ip_hdr->ip6_ctlun.ip6_un1.ip6_un1_nxt == IPPROTO_ICMPV6; @@ -177,7 +239,7 @@ if (is_icmp) { if (packet.length() < kIcmp6PrefixLen) { // Packet is too short to be icmp6. Drop it. - return false; + return L2ValidationResult::kInvalid; } is_neighbor_solicit = reinterpret_cast<const icmp6_hdr*>(packet.data() + kIp6PrefixLen) @@ -197,7 +259,7 @@ *reinterpret_cast<const in6_addr*>(icmp6_payload)); if (target_address != *QboneConstants::GatewayAddress()) { // Only respond to solicitations for our gateway address - return false; + return L2ValidationResult::kValidLinkLocal; } // Neighbor Advertisement crafted per: @@ -217,8 +279,8 @@ // | Type | Length | Link-Layer Address ... // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ int pos = sizeof(in6_addr); - payload[pos++] = ND_OPT_TARGET_LINKADDR; // Type - payload[pos++] = 1; // Length in units of 8 octets + payload[pos++] = ND_OPT_TARGET_LINKADDR; // Type + payload[pos++] = 1; // Length in units of 8 octets memcpy(&payload[pos], eth_hdr_.h_source, ETH_ALEN); // This interfaces' MAC address @@ -233,15 +295,12 @@ CreateIcmpPacket(ip_hdr->ip6_src, ip_hdr->ip6_src, response_hdr, absl::string_view(payload.get(), payload_size), [this](absl::string_view packet) { - std::string error; - WritePacket(packet.data(), packet.size(), &error); + WritePacketToNetwork(packet.data(), packet.size()); }); - // Do not forward the neighbor solicitation through the tunnel since it's - // link-local. - return false; + return L2ValidationResult::kValidLinkLocal; } - return true; + return L2ValidationResult::kValidNormal; } } // namespace quic
diff --git a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h index c953444..b1743ec 100644 --- a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h +++ b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h
@@ -8,16 +8,22 @@ #include <linux/if_ether.h> #include <cstddef> +#include <cstdint> +#include <string> +#include "absl/base/attributes.h" +#include "absl/base/nullability.h" +#include "absl/strings/string_view.h" #include "absl/time/time.h" #include "quiche/quic/core/quic_packets.h" +#include "quiche/quic/qbone/bonnet/qbone_client_packet_exchanger.h" #include "quiche/quic/qbone/platform/kernel_interface.h" #include "quiche/quic/qbone/platform/netlink_interface.h" -#include "quiche/quic/qbone/qbone_packet_exchanger.h" +#include "quiche/quic/qbone/qbone_client_interface.h" namespace quic { -class TunDevicePacketExchanger : public QbonePacketExchanger { +class TunDevicePacketExchanger : public QboneClientPacketExchanger { public: class StatsInterface { public: @@ -45,33 +51,46 @@ // |visitor| is not owned but should out live objects of this class. // |stats| is notified about packet read/write statistics. It is not owned, // but should outlive objects of this class. - TunDevicePacketExchanger(size_t mtu, KernelInterface* kernel, - NetlinkInterface* netlink, - QbonePacketExchanger::Visitor* visitor, bool is_tap, - StatsInterface* stats, absl::string_view ifname); + TunDevicePacketExchanger( + size_t mtu, KernelInterface* kernel, NetlinkInterface* netlink, + QboneClientPacketExchanger::Visitor* absl_nullable visitor + ABSL_ATTRIBUTE_LIFETIME_BOUND, + bool is_tap, StatsInterface* stats, absl::string_view ifname); void set_read_file_descriptor(int fd); void set_write_file_descriptor(int fd); ABSL_MUST_USE_RESULT const StatsInterface* stats_interface() const; - private: - // From QbonePacketExchanger. - std::unique_ptr<QuicData> ReadPacket(std::string* error) override; + // QboneClientPacketExchanger: + bool ReadAndDeliverPacket(QboneClientInterface* qbone_client) override; + void WritePacketToNetwork(const char* packet, size_t size) override; - // From QbonePacketExchanger. - bool WritePacket(const char* packet, size_t size, - std::string* error) override; + private: + enum class L2ValidationResult { + // Headers are invalid. Packet should be dropped. + kInvalid, + + // Headers are valid, and the packet should be forwarded to the tunnel. + kValidNormal, + + // Headers are valid, and the packet is a recognized link-local packet. The + // packet should not be forwarded to the tunnel. An appropriate response has + // already been sent back to the network. + kValidLinkLocal + }; void InitializeEthHdr(); - bool ValidateL2Headers(const ethhdr& eth_header, const QuicData& packet); + L2ValidationResult ValidateL2Headers(const ethhdr& eth_header, + const QuicData& packet); int read_fd_ = -1; int write_fd_ = -1; size_t mtu_; KernelInterface* kernel_; NetlinkInterface* netlink_; + QboneClientPacketExchanger::Visitor* const absl_nullable visitor_; const std::string ifname_; const bool is_tap_;
diff --git a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger_test.cc b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger_test.cc index fd22342..094b2c7 100644 --- a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger_test.cc +++ b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger_test.cc
@@ -9,12 +9,16 @@ #include <netinet/ip6.h> #include <sys/uio.h> +#include <cerrno> +#include <cstddef> +#include <cstring> #include <string> #include "absl/status/status.h" #include "absl/strings/string_view.h" #include "quiche/quic/platform/api/quic_test.h" #include "quiche/quic/qbone/bonnet/mock_packet_exchanger_stats_interface.h" +#include "quiche/quic/qbone/bonnet/qbone_client_packet_exchanger.h" #include "quiche/quic/qbone/mock_qbone_client.h" #include "quiche/quic/qbone/platform/mock_kernel.h" #include "quiche/quic/qbone/platform/mock_netlink.h" @@ -31,7 +35,7 @@ using ::testing::StrEq; using ::testing::StrictMock; -class MockVisitor : public QbonePacketExchanger::Visitor { +class MockVisitor : public QboneClientPacketExchanger::Visitor { public: MOCK_METHOD(void, OnReadError, (const std::string&), (override)); MOCK_METHOD(void, OnWriteError, (const std::string&), (override)); @@ -56,7 +60,7 @@ TunDevicePacketExchanger exchanger_; }; -TEST_F(TunDevicePacketExchangerTest, WritePacketReturnsFalseOnError) { +TEST_F(TunDevicePacketExchangerTest, WritePacketError) { std::string packet = "fake packet"; EXPECT_CALL(mock_kernel_, writev(kWriteFd, _, 2)) .WillOnce([](int fd, const struct iovec* iov, int iovcnt) -> ssize_t { @@ -74,8 +78,7 @@ exchanger_.WritePacketToNetwork(packet.data(), packet.size()); } -TEST_F(TunDevicePacketExchangerTest, - WritePacketReturnFalseAndBlockedOnBlockedTunnel) { +TEST_F(TunDevicePacketExchangerTest, WritePacketBlocked) { std::string packet = "fake packet"; EXPECT_CALL(mock_kernel_, writev(kWriteFd, _, 2)) .WillOnce([](int fd, const struct iovec* iov, int iovcnt) -> ssize_t { @@ -94,7 +97,7 @@ exchanger_.WritePacketToNetwork(packet.data(), packet.size()); } -TEST_F(TunDevicePacketExchangerTest, WritePacketReturnsTrueOnSuccessfulWrite) { +TEST_F(TunDevicePacketExchangerTest, WritePacketSuccessfulWrite) { std::string packet = "fake packet"; EXPECT_CALL(mock_kernel_, writev(kWriteFd, _, 2)) .WillOnce( @@ -164,17 +167,17 @@ tap_exchanger.WritePacketToNetwork(packet.data(), packet.size()); } -TEST_F(TunDevicePacketExchangerTest, ReadPacketReturnsNullOnError) { +TEST_F(TunDevicePacketExchangerTest, ReadPacketError) { EXPECT_CALL(mock_kernel_, readv(kReadFd, _, 2)) .WillOnce([](int fd, const struct iovec* iov, int iovcnt) { errno = ECOMM; return -1; }); EXPECT_CALL(mock_visitor_, OnReadError(_)); - exchanger_.ReadAndDeliverPacket(&mock_client_); + EXPECT_FALSE(exchanger_.ReadAndDeliverPacket(&mock_client_)); } -TEST_F(TunDevicePacketExchangerTest, ReadPacketReturnsNullOnBlockedRead) { +TEST_F(TunDevicePacketExchangerTest, ReadPacketBlocked) { EXPECT_CALL(mock_kernel_, readv(kReadFd, _, 2)) .WillOnce([](int fd, const struct iovec* iov, int iovcnt) { errno = EAGAIN; @@ -185,8 +188,7 @@ EXPECT_FALSE(exchanger_.ReadAndDeliverPacket(&mock_client_)); } -TEST_F(TunDevicePacketExchangerTest, - ReadPacketReturnsThePacketOnSuccessfulRead) { +TEST_F(TunDevicePacketExchangerTest, ReadPacketSuccessfulRead) { std::string packet = "fake_packet"; EXPECT_CALL(mock_kernel_, readv(kReadFd, _, 2)) .WillOnce([packet](int fd, const struct iovec* iov, int iovcnt) { @@ -200,6 +202,82 @@ EXPECT_TRUE(exchanger_.ReadAndDeliverPacket(&mock_client_)); } +TEST_F(TunDevicePacketExchangerTest, WriteWithNullVisitor) { + TunDevicePacketExchanger exchanger(kMtu, &mock_kernel_, nullptr, + /*visitor=*/nullptr, false, &mock_stats_, + absl::string_view()); + exchanger.set_write_file_descriptor(kWriteFd); + + std::string packet = "fake packet"; + EXPECT_CALL(mock_kernel_, writev(kWriteFd, _, 2)) + .WillOnce( + [&packet](int fd, const struct iovec* iov, int iovcnt) -> ssize_t { + EXPECT_EQ(iov[0].iov_base, nullptr); + EXPECT_EQ(iov[0].iov_len, 0); + EXPECT_THAT(reinterpret_cast<const char*>(iov[1].iov_base), + StrEq(packet)); + EXPECT_EQ(iov[1].iov_len, packet.size()); + return packet.size(); + }); + + EXPECT_CALL(mock_stats_, OnPacketWritten(packet.size(), _)).Times(1); + exchanger.WritePacketToNetwork(packet.data(), packet.size()); +} + +TEST_F(TunDevicePacketExchangerTest, WritePacketErrorWithNullVisitor) { + TunDevicePacketExchanger exchanger(kMtu, &mock_kernel_, nullptr, + /*visitor=*/nullptr, false, &mock_stats_, + absl::string_view()); + exchanger.set_write_file_descriptor(kWriteFd); + + std::string packet = "fake packet"; + EXPECT_CALL(mock_kernel_, writev(kWriteFd, _, 2)) + .WillOnce([](int fd, const struct iovec* iov, int iovcnt) -> ssize_t { + EXPECT_EQ(iov[0].iov_base, nullptr); + EXPECT_EQ(iov[0].iov_len, 0); + EXPECT_THAT(reinterpret_cast<const char*>(iov[1].iov_base), + testing::StrEq("fake packet")); + EXPECT_EQ(iov[1].iov_len, 11); + errno = ECOMM; + return -1; + }); + + exchanger.WritePacketToNetwork(packet.data(), packet.size()); +} + +TEST_F(TunDevicePacketExchangerTest, ReadPacketWithNullVisitor) { + TunDevicePacketExchanger exchanger(kMtu, &mock_kernel_, nullptr, + /*visitor=*/nullptr, false, &mock_stats_, + absl::string_view()); + exchanger.set_read_file_descriptor(kReadFd); + + std::string packet = "fake_packet"; + EXPECT_CALL(mock_kernel_, readv(kReadFd, _, 2)) + .WillOnce([packet](int fd, const struct iovec* iov, int iovcnt) { + EXPECT_EQ(iov[0].iov_len, 0); + EXPECT_EQ(iov[1].iov_len, kMtu); + memcpy(iov[1].iov_base, packet.data(), packet.size()); + return packet.size(); + }); + EXPECT_CALL(mock_client_, ProcessPacketFromNetwork(StrEq(packet))); + EXPECT_CALL(mock_stats_, OnPacketRead(_, _)).Times(1); + EXPECT_TRUE(exchanger.ReadAndDeliverPacket(&mock_client_)); +} + +TEST_F(TunDevicePacketExchangerTest, ReadPacketErrorWithNullVisitor) { + TunDevicePacketExchanger exchanger(kMtu, &mock_kernel_, nullptr, + /*visitor=*/nullptr, false, &mock_stats_, + absl::string_view()); + exchanger.set_read_file_descriptor(kReadFd); + + EXPECT_CALL(mock_kernel_, readv(kReadFd, _, 2)) + .WillOnce([](int fd, const struct iovec* iov, int iovcnt) { + errno = ECOMM; + return -1; + }); + EXPECT_FALSE(exchanger.ReadAndDeliverPacket(&mock_client_)); +} + class TunDevicePacketExchangerTapTest : public QuicTest { protected: TunDevicePacketExchangerTapTest() @@ -257,7 +335,8 @@ return ETH_HLEN + 10; // Read some bytes }); - EXPECT_CALL(mock_visitor_, OnReadError("")); + EXPECT_CALL(mock_visitor_, OnReadError(_)); + EXPECT_CALL(mock_stats_, OnReadError(_)); EXPECT_FALSE(exchanger_.ReadAndDeliverPacket(&mock_client_)); } @@ -304,10 +383,10 @@ return iov[0].iov_len + iov[1].iov_len; }); EXPECT_CALL(mock_stats_, OnPacketWritten(_, _)).Times(1); + EXPECT_CALL(mock_visitor_, OnWrite(_)); // ReadAndDeliverPacket should return false because packet was handled // internally (Neighbor Discovery). - EXPECT_CALL(mock_visitor_, OnReadError("")); EXPECT_FALSE(exchanger_.ReadAndDeliverPacket(&mock_client_)); }
diff --git a/quiche/quic/qbone/mock_qbone_packet_exchanger.h b/quiche/quic/qbone/mock_qbone_packet_exchanger.h deleted file mode 100644 index 42a6691..0000000 --- a/quiche/quic/qbone/mock_qbone_packet_exchanger.h +++ /dev/null
@@ -1,31 +0,0 @@ -// Copyright 2026 The Chromium Authors -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef QUICHE_QUIC_QBONE_MOCK_QBONE_PACKET_EXCHANGER_H_ -#define QUICHE_QUIC_QBONE_MOCK_QBONE_PACKET_EXCHANGER_H_ - -#include <cstddef> -#include <memory> -#include <string> - -#include "quiche/quic/core/quic_packets.h" -#include "quiche/quic/platform/api/quic_test.h" -#include "quiche/quic/qbone/qbone_packet_exchanger.h" - -namespace quic::test { - -class MockQbonePacketExchanger : public QbonePacketExchanger { - public: - MockQbonePacketExchanger() : QbonePacketExchanger(/*visitor=*/nullptr) {} - - MOCK_METHOD(std::unique_ptr<QuicData>, ReadPacket, (std::string * error), - (override)); - MOCK_METHOD(bool, WritePacket, - (const char* packet, size_t size, std::string* error), - (override)); -}; - -} // namespace quic::test - -#endif // QUICHE_QUIC_QBONE_MOCK_QBONE_PACKET_EXCHANGER_H_
diff --git a/quiche/quic/qbone/qbone_client.cc b/quiche/quic/qbone/qbone_client.cc index b86e6de..5a01199 100644 --- a/quiche/quic/qbone/qbone_client.cc +++ b/quiche/quic/qbone/qbone_client.cc
@@ -15,7 +15,7 @@ #include "quiche/quic/core/quic_bandwidth.h" #include "quiche/quic/core/quic_default_connection_helper.h" #include "quiche/quic/platform/api/quic_testvalue.h" -#include "quiche/quic/qbone/qbone_packet_exchanger.h" +#include "quiche/quic/qbone/bonnet/qbone_client_packet_exchanger.h" #include "quiche/quic/tools/quic_client_base.h" #include "quiche/quic/tools/quic_client_default_network_helper.h" #include "quiche/common/platform/api/quiche_command_line_flags.h" @@ -47,7 +47,7 @@ const ParsedQuicVersionVector& supported_versions, QuicSession::Visitor* session_owner, const QuicConfig& config, QuicEventLoop* event_loop, std::unique_ptr<ProofVerifier> proof_verifier, - QbonePacketExchanger* absl_nonnull local_network_packet_exchanger, + QboneClientPacketExchanger* absl_nonnull local_network_packet_exchanger, QboneClientControlStream::Handler* qbone_handler) : QuicClientBase(server_id, supported_versions, config, new QuicDefaultConnectionHelper(),
diff --git a/quiche/quic/qbone/qbone_client.h b/quiche/quic/qbone/qbone_client.h index 1838c87..34b2b45 100644 --- a/quiche/quic/qbone/qbone_client.h +++ b/quiche/quic/qbone/qbone_client.h
@@ -12,9 +12,9 @@ #include "absl/strings/string_view.h" #include "quiche/quic/core/io/quic_event_loop.h" #include "quiche/quic/core/quic_bandwidth.h" +#include "quiche/quic/qbone/bonnet/qbone_client_packet_exchanger.h" #include "quiche/quic/qbone/qbone_client_interface.h" #include "quiche/quic/qbone/qbone_client_session.h" -#include "quiche/quic/qbone/qbone_packet_exchanger.h" #include "quiche/quic/tools/quic_client_base.h" namespace quic { @@ -30,8 +30,8 @@ QuicSession::Visitor* session_owner, const QuicConfig& config, QuicEventLoop* event_loop, std::unique_ptr<ProofVerifier> proof_verifier, - QbonePacketExchanger* absl_nonnull local_network_packet_exchanger - ABSL_ATTRIBUTE_LIFETIME_BOUND, + QboneClientPacketExchanger* absl_nonnull + local_network_packet_exchanger ABSL_ATTRIBUTE_LIFETIME_BOUND, QboneClientControlStream::Handler* qbone_handler); ~QboneClient() override; QboneClientSession* qbone_session(); @@ -61,7 +61,7 @@ const ParsedQuicVersionVector& supported_versions, QuicConnection* connection) override; - QbonePacketExchanger& local_network_packet_exchanger() { + QboneClientPacketExchanger& local_network_packet_exchanger() { return local_network_packet_exchanger_; } @@ -74,7 +74,7 @@ bool HasActiveRequests() override; private: - QbonePacketExchanger& local_network_packet_exchanger_; + QboneClientPacketExchanger& local_network_packet_exchanger_; QboneClientControlStream::Handler* qbone_handler_; QuicSession::Visitor* session_owner_;
diff --git a/quiche/quic/qbone/qbone_client_session.cc b/quiche/quic/qbone/qbone_client_session.cc index 8d2a111..6e31e61 100644 --- a/quiche/quic/qbone/qbone_client_session.cc +++ b/quiche/quic/qbone/qbone_client_session.cc
@@ -5,13 +5,17 @@ #include "quiche/quic/qbone/qbone_client_session.h" #include <memory> +#include <string> #include <utility> +#include <vector> +#include "absl/base/nullability.h" #include "absl/strings/string_view.h" #include "quiche/quic/core/quic_types.h" +#include "quiche/quic/qbone/bonnet/qbone_client_packet_exchanger.h" #include "quiche/quic/qbone/qbone_constants.h" -#include "quiche/quic/qbone/qbone_packet_exchanger.h" -#include "quiche/common/platform/api/quiche_command_line_flags.h" +#include "quiche/quic/qbone/qbone_session_base.h" +#include "quiche/common/platform/api/quiche_logging.h" namespace quic { @@ -21,7 +25,7 @@ QuicSession::Visitor* owner, const QuicConfig& config, const ParsedQuicVersionVector& supported_versions, const QuicServerId& server_id, - QbonePacketExchanger* absl_nonnull local_network_packet_exchanger, + QboneClientPacketExchanger* absl_nonnull local_network_packet_exchanger, QboneClientControlStream::Handler* handler) : QboneSessionBase(connection, owner, config, supported_versions), local_network_packet_exchanger_(*local_network_packet_exchanger),
diff --git a/quiche/quic/qbone/qbone_client_session.h b/quiche/quic/qbone/qbone_client_session.h index c1e1cea..ef7310c 100644 --- a/quiche/quic/qbone/qbone_client_session.h +++ b/quiche/quic/qbone/qbone_client_session.h
@@ -5,15 +5,18 @@ #ifndef QUICHE_QUIC_QBONE_QBONE_CLIENT_SESSION_H_ #define QUICHE_QUIC_QBONE_QBONE_CLIENT_SESSION_H_ +#include <memory> +#include <string> +#include <vector> + #include "absl/base/attributes.h" #include "absl/base/nullability.h" #include "absl/strings/string_view.h" #include "quiche/quic/core/quic_crypto_client_stream.h" #include "quiche/quic/platform/api/quic_export.h" +#include "quiche/quic/qbone/bonnet/qbone_client_packet_exchanger.h" #include "quiche/quic/qbone/qbone_control.pb.h" #include "quiche/quic/qbone/qbone_control_stream.h" -#include "quiche/quic/qbone/qbone_packet_exchanger.h" -#include "quiche/quic/qbone/qbone_packet_writer.h" #include "quiche/quic/qbone/qbone_session_base.h" namespace quic { @@ -28,7 +31,7 @@ QuicSession::Visitor* owner, const QuicConfig& config, const ParsedQuicVersionVector& supported_versions, const QuicServerId& server_id, - QbonePacketExchanger* absl_nonnull local_network_packet_exchanger + QboneClientPacketExchanger* absl_nonnull local_network_packet_exchanger ABSL_ATTRIBUTE_LIFETIME_BOUND, QboneClientControlStream::Handler* handler); QboneClientSession(const QboneClientSession&) = delete; @@ -88,7 +91,7 @@ } private: - QbonePacketExchanger& local_network_packet_exchanger_; + QboneClientPacketExchanger& local_network_packet_exchanger_; QuicServerId server_id_; // Config for QUIC crypto client stream, used by the client. QuicCryptoClientConfig* quic_crypto_client_config_;
diff --git a/quiche/quic/qbone/qbone_client_test.cc b/quiche/quic/qbone/qbone_client_test.cc index f875a3f..830628a 100644 --- a/quiche/quic/qbone/qbone_client_test.cc +++ b/quiche/quic/qbone/qbone_client_test.cc
@@ -26,7 +26,7 @@ #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/mock_qbone_packet_exchanger.h" +#include "quiche/quic/qbone/bonnet/mock_qbone_client_packet_exchanger.h" #include "quiche/quic/qbone/qbone_packet_processor_test_tools.h" #include "quiche/quic/qbone/qbone_server_session.h" #include "quiche/quic/test_tools/crypto_test_utils.h" @@ -165,12 +165,10 @@ : QboneClient(server_address, server_id, supported_versions, /*session_owner=*/nullptr, QuicConfig(), event_loop, std::move(proof_verifier), &packet_exchanger_, nullptr) { - ON_CALL(packet_exchanger_, WritePacket(_, _, _)) - .WillByDefault( - [this](const char* packet, size_t size, std::string* error) { - data_.push_back(std::string(packet, size)); - return true; - }); + ON_CALL(packet_exchanger_, WritePacketToNetwork(_, _)) + .WillByDefault([this](const char* packet, size_t size) { + data_.push_back(std::string(packet, size)); + }); } ~QboneTestClient() override {} @@ -203,7 +201,7 @@ const std::vector<std::string>& data() { return data_; } private: - MockQbonePacketExchanger packet_exchanger_; + MockQboneClientPacketExchanger packet_exchanger_; std::vector<std::string> data_; };
diff --git a/quiche/quic/qbone/qbone_packet_exchanger.cc b/quiche/quic/qbone/qbone_packet_exchanger.cc deleted file mode 100644 index 13eee33..0000000 --- a/quiche/quic/qbone/qbone_packet_exchanger.cc +++ /dev/null
@@ -1,47 +0,0 @@ -// Copyright (c) 2019 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "quiche/quic/qbone/qbone_packet_exchanger.h" - -#include <memory> -#include <string> - -#include "absl/status/status.h" -#include "absl/strings/string_view.h" - -namespace quic { - -bool QbonePacketExchanger::ReadAndDeliverPacket( - QboneClientInterface* qbone_client) { - std::string error; - std::unique_ptr<QuicData> packet = ReadPacket(&error); - if (packet == nullptr) { - if (visitor_) { - visitor_->OnReadError(error); - } - return false; - } - qbone_client->ProcessPacketFromNetwork(packet->AsStringPiece()); - return true; -} - -void QbonePacketExchanger::WritePacketToNetwork(const char* packet, - size_t size) { - if (visitor_) { - absl::Status status = visitor_->OnWrite(absl::string_view(packet, size)); - if (!status.ok()) { - QUIC_LOG_EVERY_N_SEC(ERROR, 60) << status; - } - } - - std::string error; - if (WritePacket(packet, size, &error)) { - return; - } - QUIC_LOG_EVERY_N_SEC(ERROR, 60) << "Packet write failed: " << error; - if (visitor_) { - visitor_->OnWriteError(error); - } -} -} // namespace quic
diff --git a/quiche/quic/qbone/qbone_packet_exchanger.h b/quiche/quic/qbone/qbone_packet_exchanger.h deleted file mode 100644 index 5616426..0000000 --- a/quiche/quic/qbone/qbone_packet_exchanger.h +++ /dev/null
@@ -1,71 +0,0 @@ -// Copyright (c) 2019 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef QUICHE_QUIC_QBONE_QBONE_PACKET_EXCHANGER_H_ -#define QUICHE_QUIC_QBONE_QBONE_PACKET_EXCHANGER_H_ - -#include <cstddef> -#include <memory> -#include <string> - -#include "absl/status/status.h" -#include "absl/strings/string_view.h" -#include "quiche/quic/core/quic_packets.h" -#include "quiche/quic/qbone/qbone_client_interface.h" - -namespace quic { - -// Handles reading and writing on the local network and exchange packets between -// the local network with a QBONE connection. -class QbonePacketExchanger { - public: - // The owner might want to receive notifications when read or write fails. - class Visitor { - public: - virtual ~Visitor() {} - virtual void OnReadError(const std::string& error) {} - virtual void OnWriteError(const std::string& error) {} - virtual absl::Status OnWrite(absl::string_view packet) { - return absl::OkStatus(); - } - }; - // Does not take ownership of visitor. - QbonePacketExchanger(Visitor* visitor) : visitor_(visitor) {} - - QbonePacketExchanger(const QbonePacketExchanger&) = delete; - QbonePacketExchanger& operator=(const QbonePacketExchanger&) = delete; - - QbonePacketExchanger(QbonePacketExchanger&&) = delete; - QbonePacketExchanger& operator=(QbonePacketExchanger&&) = delete; - - virtual ~QbonePacketExchanger() = default; - - // Returns true if there may be more packets to read. - // Implementations handles the actual raw read and delivers the packet to - // qbone_client. - bool ReadAndDeliverPacket(QboneClientInterface* qbone_client); - - // Writes a packet to the local network. If the write would be blocked, the - // packet is dropped. - void WritePacketToNetwork(const char* packet, size_t size); - - private: - // The actual implementation that reads a packet from the local network. - // Returns the packet if one is successfully read. This might nullptr when a) - // there is no packet to read, b) the read failed. error contains the error - // message. - virtual std::unique_ptr<QuicData> ReadPacket(std::string* error) = 0; - - // The actual implementation that writes a packet to the local network. - // Returns true if the write succeeds. If write is unsuccessful, error - // contains the error message. - virtual bool WritePacket(const char* packet, size_t size, - std::string* error) = 0; - - Visitor* visitor_; -}; - -} // namespace quic - -#endif // QUICHE_QUIC_QBONE_QBONE_PACKET_EXCHANGER_H_
diff --git a/quiche/quic/qbone/qbone_packet_exchanger_test.cc b/quiche/quic/qbone/qbone_packet_exchanger_test.cc deleted file mode 100644 index 13fffe9..0000000 --- a/quiche/quic/qbone/qbone_packet_exchanger_test.cc +++ /dev/null
@@ -1,184 +0,0 @@ -// Copyright (c) 2019 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "quiche/quic/qbone/qbone_packet_exchanger.h" - -#include <list> -#include <memory> -#include <string> -#include <utility> -#include <vector> - -#include "absl/status/status.h" -#include "absl/strings/string_view.h" -#include "quiche/quic/platform/api/quic_test.h" -#include "quiche/quic/qbone/mock_qbone_client.h" - -namespace quic { -namespace { - -using ::testing::StrEq; -using ::testing::StrictMock; - -class MockVisitor : public QbonePacketExchanger::Visitor { - public: - MOCK_METHOD(void, OnReadError, (const std::string&), (override)); - MOCK_METHOD(void, OnWriteError, (const std::string&), (override)); - MOCK_METHOD(absl::Status, OnWrite, (absl::string_view), (override)); -}; - -class FakeQbonePacketExchanger : public QbonePacketExchanger { - public: - using QbonePacketExchanger::QbonePacketExchanger; - - // Adds a packet to the end of list of packets to be returned by ReadPacket. - // When the list is empty, ReadPacket returns nullptr to signify error as - // defined by QbonePacketExchanger. If SetReadError is not called or called - // with empty error string, ReadPacket sets blocked to true. - void AddPacketToBeRead(std::unique_ptr<QuicData> packet) { - packets_to_be_read_.push_back(std::move(packet)); - } - - // Sets the error to be returned by ReadPacket when the list of packets is - // empty. If error is empty string, blocked is set by ReadPacket. - void SetReadError(const std::string& error) { read_error_ = error; } - - // Force WritePacket to fail with the given status. - void ForceWriteFailure(const std::string& error) { write_error_ = error; } - - // Packets that have been successfully written by WritePacket. - const std::vector<std::string>& packets_written() const { - return packets_written_; - } - - private: - // Implements QbonePacketExchanger::ReadPacket. - std::unique_ptr<QuicData> ReadPacket(std::string* error) override { - if (packets_to_be_read_.empty()) { - *error = read_error_; - return nullptr; - } - - std::unique_ptr<QuicData> packet = std::move(packets_to_be_read_.front()); - packets_to_be_read_.pop_front(); - return packet; - } - - // Implements QbonePacketExchanger::WritePacket. - bool WritePacket(const char* packet, size_t size, - std::string* error) override { - if (!write_error_.empty()) { - *error = write_error_; - return false; - } - - packets_written_.push_back(std::string(packet, size)); - return true; - } - - std::string read_error_; - std::list<std::unique_ptr<QuicData>> packets_to_be_read_; - - std::string write_error_; - std::vector<std::string> packets_written_; -}; - -TEST(QbonePacketExchangerTest, - ReadAndDeliverPacketDeliversPacketToQboneClient) { - StrictMock<MockVisitor> visitor; - FakeQbonePacketExchanger exchanger(&visitor); - StrictMock<MockQboneClient> client; - - std::string packet = "data"; - exchanger.AddPacketToBeRead( - std::make_unique<QuicData>(packet.data(), packet.length())); - EXPECT_CALL(client, ProcessPacketFromNetwork(StrEq("data"))); - - EXPECT_TRUE(exchanger.ReadAndDeliverPacket(&client)); -} - -TEST(QbonePacketExchangerTest, - ReadAndDeliverPacketNotifiesVisitorOnReadFailure) { - MockVisitor visitor; - FakeQbonePacketExchanger exchanger(&visitor); - MockQboneClient client; - - // Force read error. - std::string io_error = "I/O error"; - exchanger.SetReadError(io_error); - EXPECT_CALL(visitor, OnReadError(StrEq(io_error))).Times(1); - - EXPECT_FALSE(exchanger.ReadAndDeliverPacket(&client)); -} - -TEST(QbonePacketExchangerTest, - ReadAndDeliverPacketDoesNotNotifyVisitorOnBlockedIO) { - MockVisitor visitor; - FakeQbonePacketExchanger exchanger(&visitor); - MockQboneClient client; - - // No more packets to read. - EXPECT_FALSE(exchanger.ReadAndDeliverPacket(&client)); -} - -TEST(QbonePacketExchangerTest, - WritePacketToNetworkWritesDirectlyToNetworkWhenNotBlocked) { - MockVisitor visitor; - FakeQbonePacketExchanger exchanger(&visitor); - MockQboneClient client; - - std::string packet = "data"; - exchanger.WritePacketToNetwork(packet.data(), packet.length()); - - ASSERT_EQ(exchanger.packets_written().size(), 1); - EXPECT_THAT(exchanger.packets_written()[0], StrEq(packet)); -} - -TEST(QbonePacketExchangerTest, WritePacketToNetworkDropsPacketIfBlocked) { - std::vector<std::string> packets = {"packet0", "packet1", "packet2"}; - MockVisitor visitor; - FakeQbonePacketExchanger exchanger(&visitor); - MockQboneClient client; - - exchanger.ForceWriteFailure("blocked"); - for (int i = 0; i < packets.size(); i++) { - exchanger.WritePacketToNetwork(packets[i].data(), packets[i].length()); - } - - // Blocked writes cause packets to be dropped. - ASSERT_TRUE(exchanger.packets_written().empty()); -} - -TEST(QbonePacketExchangerTest, WriteErrorsGetNotified) { - MockVisitor visitor; - FakeQbonePacketExchanger exchanger(&visitor); - MockQboneClient client; - std::string packet = "data"; - - // Write error is delivered to visitor during WritePacketToNetwork. - std::string io_error = "I/O error"; - exchanger.ForceWriteFailure(io_error); - EXPECT_CALL(visitor, OnWriteError(StrEq(io_error))).Times(1); - exchanger.WritePacketToNetwork(packet.data(), packet.length()); - ASSERT_TRUE(exchanger.packets_written().empty()); -} - -TEST(QbonePacketExchangerTest, NullVisitorDoesntCrash) { - FakeQbonePacketExchanger exchanger(nullptr); - MockQboneClient client; - std::string packet = "data"; - - // Force read error. - std::string io_error = "I/O error"; - exchanger.SetReadError(io_error); - EXPECT_FALSE(exchanger.ReadAndDeliverPacket(&client)); - - // Force write error - exchanger.ForceWriteFailure(io_error); - exchanger.WritePacketToNetwork(packet.data(), packet.length()); - EXPECT_TRUE(exchanger.packets_written().empty()); -} - -} // namespace -} // namespace quic
diff --git a/quiche/quic/qbone/qbone_session_test.cc b/quiche/quic/qbone/qbone_session_test.cc index 6211a4c..81ab855 100644 --- a/quiche/quic/qbone/qbone_session_test.cc +++ b/quiche/quic/qbone/qbone_session_test.cc
@@ -20,7 +20,7 @@ #include "quiche/quic/platform/api/quic_expect_bug.h" #include "quiche/quic/platform/api/quic_test.h" #include "quiche/quic/platform/api/quic_test_loopback.h" -#include "quiche/quic/qbone/mock_qbone_packet_exchanger.h" +#include "quiche/quic/qbone/bonnet/mock_qbone_client_packet_exchanger.h" #include "quiche/quic/qbone/platform/icmp_packet.h" #include "quiche/quic/qbone/qbone_client_session.h" #include "quiche/quic/qbone/qbone_constants.h" @@ -313,12 +313,10 @@ } { - EXPECT_CALL(client_packet_exchanger_, WritePacket(_, _, _)) - .WillRepeatedly( - [this](const char* packet, size_t size, std::string* error) { - client_packets_to_network_.push_back(std::string(packet, size)); - return true; - }); + EXPECT_CALL(client_packet_exchanger_, WritePacketToNetwork(_, _)) + .WillRepeatedly([this](const char* packet, size_t size) { + client_packets_to_network_.push_back(std::string(packet, size)); + }); client_connection_ = new QuicConnection( TestConnectionId(), client_address, server_address, &helper_, @@ -543,7 +541,7 @@ std::unique_ptr<QuicCryptoClientConfig> client_crypto_config_; std::unique_ptr<QuicCryptoServerConfig> server_crypto_config_; - StrictMock<MockQbonePacketExchanger> client_packet_exchanger_; + StrictMock<MockQboneClientPacketExchanger> client_packet_exchanger_; std::vector<std::string> client_packets_to_network_; std::unique_ptr<DataSavingQbonePacketWriter> server_writer_; std::unique_ptr<DataSavingQboneControlHandler<QboneClientRequest>>