Bonnet Tun Exchanger Refactor: Repurpose/cleanup Visitor for async exchanger Need a way to pass out async results from the exchanger. There's already an old optional Visitor class, only used for PktTracer stuff, so I'm going to adapt and reuse that for my needs. Some trivial behavior changes in what specifically gets logged/monitored/reported-to-pkttracer during errors. E.g., all the error strings are slightly changing, and we no longer send the write packet to the PktTracer after a write error. PiperOrigin-RevId: 956116769
diff --git a/quiche/quic/qbone/bonnet/mock_qbone_client_packet_exchanger.h b/quiche/quic/qbone/bonnet/mock_qbone_client_packet_exchanger.h index 3c5167c..11db1c1 100644 --- a/quiche/quic/qbone/bonnet/mock_qbone_client_packet_exchanger.h +++ b/quiche/quic/qbone/bonnet/mock_qbone_client_packet_exchanger.h
@@ -6,7 +6,9 @@ #define QUICHE_QUIC_QBONE_MOCK_QBONE_PACKET_EXCHANGER_H_ #include <cstddef> +#include <vector> +#include "absl/status/statusor.h" #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" @@ -15,7 +17,19 @@ class MockQboneClientPacketExchanger : public QboneClientPacketExchanger { public: - MOCK_METHOD(bool, ReadAndDeliverPacket, (QboneClientInterface* qbone_client), + class MockVisitor : public QboneClientPacketExchanger::Visitor { + public: + MOCK_METHOD( + void, OnRead, + (absl::StatusOr<std::vector<QboneClientPacketExchanger::ReadResult>>), + (override)); + MOCK_METHOD( + void, OnWrite, + (absl::StatusOr<std::vector<QboneClientPacketExchanger::WriteResult>>), + (override)); + }; + + MOCK_METHOD(bool, ReadAndDeliverPacket, (QboneClientInterface * qbone_client), (override)); MOCK_METHOD(void, WritePacketToNetwork, (const char* packet, size_t size), (override));
diff --git a/quiche/quic/qbone/bonnet/qbone_client_packet_exchanger.h b/quiche/quic/qbone/bonnet/qbone_client_packet_exchanger.h index 1a07a00..27862b1 100644 --- a/quiche/quic/qbone/bonnet/qbone_client_packet_exchanger.h +++ b/quiche/quic/qbone/bonnet/qbone_client_packet_exchanger.h
@@ -6,11 +6,11 @@ #define QUICHE_QUIC_QBONE_QBONE_PACKET_EXCHANGER_H_ #include <cstddef> -#include <memory> -#include <string> +#include <vector> -#include "absl/status/status.h" -#include "absl/strings/string_view.h" +#include "absl/status/statusor.h" +#include "absl/time/time.h" +#include "absl/types/span.h" #include "quiche/quic/qbone/qbone_client_interface.h" namespace quic { @@ -19,17 +19,22 @@ // 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. + struct ReadResult { + absl::Span<const std::byte> packet; + absl::Duration latency; + }; + + struct WriteResult { + absl::Span<const std::byte> packet; + absl::Duration latency; + }; + 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 ~Visitor() = default; + + virtual void OnRead(absl::StatusOr<std::vector<ReadResult>> results) = 0; + virtual void OnWrite(absl::StatusOr<std::vector<WriteResult>> results) = 0; }; virtual ~QboneClientPacketExchanger() = default;
diff --git a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc index a132b16..f6f0c2d 100644 --- a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc +++ b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc
@@ -11,106 +11,118 @@ #include <algorithm> #include <cerrno> #include <cstddef> +#include <cstring> #include <memory> #include <string> +#include <utility> +#include <vector> +#include "absl/base/macros.h" #include "absl/base/nullability.h" #include "absl/status/status.h" +#include "absl/status/statusor.h" #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" #include "absl/time/clock.h" #include "absl/time/time.h" +#include "absl/types/span.h" #include "quiche/quic/core/quic_packets.h" +#include "quiche/quic/platform/api/quic_ip_address.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" +#include "quiche/common/quiche_endian.h" namespace quic { +using ::quiche::QuicheEndian; + TunDevicePacketExchanger::TunDevicePacketExchanger( size_t mtu, KernelInterface* kernel, NetlinkInterface* netlink, Visitor* absl_nullable visitor, bool is_tap, StatsInterface* stats, absl::string_view ifname) - : mtu_(mtu), - kernel_(kernel), + : kernel_(kernel), netlink_(netlink), visitor_(visitor), ifname_(ifname), + // Reading on a TUN device returns a packet at a time. If the packet is + // longer than the buffer, it's truncated. + read_buffer_(mtu), is_tap_(is_tap), stats_(stats) {} bool TunDevicePacketExchanger::ReadAndDeliverPacket( QboneClientInterface* qbone_client) { if (read_fd_ < 0) { - std::string error = - absl::StrCat("Invalid file descriptor of the TUN device: ", read_fd_); + absl::Status error = absl::InternalError( + absl::StrCat("Invalid file descriptor of the TUN device: ", read_fd_)); + QUIC_LOG_EVERY_N_SEC(ERROR, 60) << "Packet read failed: " << error; + stats_->OnReadError(absl::StrCat(absl::StatusCodeToString(error.code()), + ": ", error.message())); if (visitor_) { - visitor_->OnReadError(error); + visitor_->OnRead(std::move(error)); } - stats_->OnReadError(error); return false; } - // Reading on a TUN device returns a packet at a time. If the packet is longer - // than the buffer, it's truncated. - auto read_buffer = std::make_unique<char[]>(mtu_); - - int result = 0; ethhdr eth_header; struct iovec iov[2]; iov[0].iov_base = is_tap_ ? ð_header : nullptr; iov[0].iov_len = is_tap_ ? ETH_HLEN : 0; - iov[1].iov_base = read_buffer.get(); - iov[1].iov_len = mtu_; - absl::Time start = absl::Now(); - result = kernel_->readv(read_fd_, iov, 2); + iov[1].iov_base = read_buffer_.data(); + iov[1].iov_len = read_buffer_.size(); + absl::Status status = absl::OkStatus(); + absl::Time start = absl::Now(); + int result = kernel_->readv(read_fd_, iov, ABSL_ARRAYSIZE(iov)); + if (result < 0) { + status = absl::ErrnoToStatus(errno, "Read from the TUN device failed."); + } else if (result == 0) { + // 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. + status = absl::InternalError( + "Read from the TUN device returned unexpected 0 (EOF)."); + } absl::Duration latency = std::max(absl::Now() - start, absl::ZeroDuration()); - // 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 = - absl::ErrnoToStatus(errno, "Read from the TUN device was blocked.") - .message(); - stats_->OnReadError(error); - } - // 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 (!status.ok()) { + QUIC_LOG_EVERY_N_SEC(ERROR, 60) << "Packet read failed: " << status; + stats_->OnReadError(absl::StrCat(absl::StatusCodeToString(status.code()), + ": ", status.message())); if (visitor_) { - visitor_->OnReadError(error); + visitor_->OnRead(std::move(status)); } return false; } - if (is_tap_ && result < ETH_HLEN) { - std::string error = "Read packet too short for ethernet header."; + int l3_packet_size = is_tap_ ? result - ETH_HLEN : result; + if (l3_packet_size <= 0 || l3_packet_size > read_buffer_.size()) { + absl::Status error = + absl::InternalError(absl::StrCat("Invalid packet size.")); + QUIC_LOG_EVERY_N_SEC(ERROR, 60) << "Packet read failed: " << error; + stats_->OnReadError(absl::StrCat(absl::StatusCodeToString(error.code()), + ": ", error.message())); if (visitor_) { - visitor_->OnReadError(error); + visitor_->OnRead(std::move(error)); } - stats_->OnReadError(error); return false; } + absl::Span<const std::byte> l3_packet = + absl::MakeSpan(read_buffer_.data(), l3_packet_size); - 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_) { - switch (ValidateL2Headers(eth_header, *buffer)) { + switch (ValidateL2Headers(eth_header, l3_packet)) { case L2ValidationResult::kInvalid: { - std::string error = "Invalid L2 headers."; + absl::Status error = absl::InvalidArgumentError("Invalid L2 headers."); + stats_->OnReadError(absl::StrCat(absl::StatusCodeToString(error.code()), + ": ", error.message())); if (visitor_) { - visitor_->OnReadError(error); + visitor_->OnRead(std::move(error)); } - stats_->OnReadError(error); return false; } case L2ValidationResult::kValidLinkLocal: @@ -126,28 +138,27 @@ } } - stats_->OnPacketRead(buffer->length(), latency); - qbone_client->ProcessPacketFromNetwork(buffer->AsStringPiece()); + if (visitor_) { + visitor_->OnRead(std::vector<ReadResult>{ + ReadResult{.packet = l3_packet, .latency = latency}}); + } + stats_->OnPacketRead(l3_packet.size(), latency); + qbone_client->ProcessPacketFromNetwork(absl::string_view( + reinterpret_cast<const char*>(l3_packet.data()), l3_packet.size())); 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_); + absl::Status error = absl::InternalError( + absl::StrCat("Invalid file descriptor of the TUN device: ", write_fd_)); QUIC_LOG_EVERY_N_SEC(ERROR, 60) << "Packet write failed: " << error; + stats_->OnWriteError(absl::StrCat(absl::StatusCodeToString(error.code()), + ": ", error.message())); if (visitor_) { - visitor_->OnWriteError(error); + visitor_->OnWrite(std::move(error)); } - stats_->OnWriteError(error); return; } @@ -160,33 +171,30 @@ iov[1].iov_base = const_cast<char*>(packet); iov[1].iov_len = size; + absl::Status status = absl::OkStatus(); absl::Time start = absl::Now(); - int result = kernel_->writev(write_fd_, iov, 2); + int result = kernel_->writev(write_fd_, iov, ABSL_ARRAYSIZE(iov)); + if (result < 0) { + status = absl::ErrnoToStatus(errno, "Write to the TUN device failed."); + } 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 (!status.ok()) { + QUIC_LOG_EVERY_N_SEC(ERROR, 60) << "Packet write failed: " << status; + stats_->OnWriteError(absl::StrCat(absl::StatusCodeToString(status.code()), + ": ", status.message())); if (visitor_) { - visitor_->OnWriteError(error); + visitor_->OnWrite(std::move(status)); } return; } + if (visitor_) { + visitor_->OnWrite(std::vector<WriteResult>{WriteResult{ + .packet = + absl::MakeSpan(reinterpret_cast<const std::byte*>(packet), size), + .latency = latency}}); + } stats_->OnPacketWritten(result, latency); } @@ -211,7 +219,7 @@ memcpy(ð_hdr_.h_source, link_info.hardware_address, ETH_ALEN); // Assume ipv6 for now // TODO(b/195113643): Support additional protocols. - eth_hdr_.h_proto = absl::ghtons(ETH_P_IPV6); + eth_hdr_.h_proto = QuicheEndian::HostToNet16(ETH_P_IPV6); eth_hdr_initialized_ = true; } else { QUIC_LOG_EVERY_N_SEC(ERROR, 30) @@ -221,9 +229,9 @@ } TunDevicePacketExchanger::L2ValidationResult -TunDevicePacketExchanger::ValidateL2Headers(const ethhdr& eth_header, - const QuicData& packet) { - if (eth_header.h_proto != absl::ghtons(ETH_P_IPV6)) { +TunDevicePacketExchanger::ValidateL2Headers( + const ethhdr& eth_header, absl::Span<const std::byte> packet) { + if (eth_header.h_proto != QuicheEndian::HostToNet16(ETH_P_IPV6)) { return L2ValidationResult::kInvalid; } constexpr auto kIp6PrefixLen = sizeof(ip6_hdr); @@ -242,7 +250,7 @@ return L2ValidationResult::kInvalid; } is_neighbor_solicit = - reinterpret_cast<const icmp6_hdr*>(packet.data() + kIp6PrefixLen) + reinterpret_cast<const icmp6_hdr*>(packet.subspan(kIp6PrefixLen).data()) ->icmp6_type == ND_NEIGHBOR_SOLICIT; } @@ -253,10 +261,15 @@ } // If we've received a neighbor solicitation, craft an advertisement to // respond with and write it back to the local interface. - auto* icmp6_payload = packet.data() + kIcmp6PrefixLen; + absl::Span<const std::byte> icmp6_payload = packet.subspan(kIcmp6PrefixLen); + + if (icmp6_payload.size() < sizeof(in6_addr)) { + // Packet is too short to contain a valid ICMPv6 payload. Drop it. + return L2ValidationResult::kInvalid; + } QuicIpAddress target_address( - *reinterpret_cast<const in6_addr*>(icmp6_payload)); + *reinterpret_cast<const in6_addr*>(icmp6_payload.data())); if (target_address != *QboneConstants::GatewayAddress()) { // Only respond to solicitations for our gateway address return L2ValidationResult::kValidLinkLocal; @@ -271,7 +284,7 @@ const int payload_size = sizeof(in6_addr) + kIcmpv6OptionSize; auto payload = std::make_unique<char[]>(payload_size); // Place the solicited IPv6 address at the beginning of the response payload - memcpy(payload.get(), icmp6_payload, sizeof(in6_addr)); + memcpy(payload.get(), icmp6_payload.data(), sizeof(in6_addr)); // Setup the Target link-layer address option: // 0 1 2 3 // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
diff --git a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h index b1743ec..09a392f 100644 --- a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h +++ b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h
@@ -10,11 +10,13 @@ #include <cstddef> #include <cstdint> #include <string> +#include <vector> #include "absl/base/attributes.h" #include "absl/base/nullability.h" #include "absl/strings/string_view.h" #include "absl/time/time.h" +#include "absl/types/span.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" @@ -83,18 +85,19 @@ void InitializeEthHdr(); L2ValidationResult ValidateL2Headers(const ethhdr& eth_header, - const QuicData& packet); + absl::Span<const std::byte> 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_; + std::vector<std::byte> read_buffer_; + const bool is_tap_; - ethhdr eth_hdr_; + ethhdr eth_hdr_ = {}; bool eth_hdr_initialized_ = false; StatsInterface* stats_;
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 094b2c7..d390851 100644 --- a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger_test.cc +++ b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger_test.cc
@@ -11,6 +11,7 @@ #include <cerrno> #include <cstddef> +#include <cstdint> #include <cstring> #include <string> @@ -18,11 +19,14 @@ #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/mock_qbone_client_packet_exchanger.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" +#include "quiche/quic/qbone/platform/netlink_interface.h" #include "quiche/quic/qbone/qbone_constants.h" +#include "quiche/common/quiche_endian.h" namespace quic::test { namespace { @@ -31,17 +35,18 @@ const int kReadFd = 15; const int kWriteFd = 16; +using ::absl_testing::IsOkAndHolds; +using ::absl_testing::StatusIs; +using ::quiche::QuicheEndian; using ::testing::_; +using ::testing::ElementsAre; +using ::testing::ElementsAreArray; +using ::testing::Field; +using ::testing::Ne; +using ::testing::SizeIs; using ::testing::StrEq; using ::testing::StrictMock; -class MockVisitor : public QboneClientPacketExchanger::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 TunDevicePacketExchangerTest : public QuicTest { protected: TunDevicePacketExchangerTest() @@ -54,7 +59,7 @@ ~TunDevicePacketExchangerTest() override = default; MockKernel mock_kernel_; - StrictMock<MockVisitor> mock_visitor_; + StrictMock<MockQboneClientPacketExchanger::MockVisitor> mock_visitor_; StrictMock<MockQboneClient> mock_client_; StrictMock<MockPacketExchangerStatsInterface> mock_stats_; TunDevicePacketExchanger exchanger_; @@ -73,8 +78,8 @@ return -1; }); - EXPECT_CALL(mock_visitor_, OnWriteError(_)); - EXPECT_CALL(mock_visitor_, OnWrite(StrEq(packet))).Times(1); + EXPECT_CALL(mock_visitor_, OnWrite(StatusIs(Ne(absl::StatusCode::kOk)))); + EXPECT_CALL(mock_stats_, OnWriteError(_)); exchanger_.WritePacketToNetwork(packet.data(), packet.size()); } @@ -91,9 +96,8 @@ return -1; }); - EXPECT_CALL(mock_stats_, OnWriteError(_)).Times(1); - EXPECT_CALL(mock_visitor_, OnWrite(StrEq(packet))).Times(1); - EXPECT_CALL(mock_visitor_, OnWriteError(_)).Times(1); + EXPECT_CALL(mock_stats_, OnWriteError(_)); + EXPECT_CALL(mock_visitor_, OnWrite(StatusIs(Ne(absl::StatusCode::kOk)))); exchanger_.WritePacketToNetwork(packet.data(), packet.size()); } @@ -110,15 +114,21 @@ return packet.size(); }); - EXPECT_CALL(mock_stats_, OnPacketWritten(packet.size(), _)).Times(1); - EXPECT_CALL(mock_visitor_, OnWrite(StrEq(packet))).Times(1); + EXPECT_CALL(mock_stats_, OnPacketWritten(packet.size(), _)); + EXPECT_CALL( + mock_visitor_, + OnWrite(IsOkAndHolds(ElementsAre(Field( + &QboneClientPacketExchanger::WriteResult::packet, + ElementsAreArray(reinterpret_cast<const std::byte*>(packet.data()), + packet.size())))))) + .Times(1); exchanger_.WritePacketToNetwork(packet.data(), packet.size()); } TEST_F(TunDevicePacketExchangerTest, TapWritePacketSuccessful) { StrictMock<MockKernel> mock_kernel; StrictMock<MockNetlink> mock_netlink; - StrictMock<MockVisitor> mock_visitor; + StrictMock<MockQboneClientPacketExchanger::MockVisitor> mock_visitor; StrictMock<MockPacketExchangerStatsInterface> mock_stats; TunDevicePacketExchanger tap_exchanger(kMtu, &mock_kernel, &mock_netlink, &mock_visitor, /*is_tap=*/true, @@ -152,7 +162,7 @@ uint16_t proto; memcpy(&proto, first_buffer + 2 * ETH_ALEN, 2); - EXPECT_EQ(proto, absl::ghtons(ETH_P_IPV6)); + EXPECT_EQ(proto, QuicheEndian::HostToNet16(ETH_P_IPV6)); EXPECT_EQ(absl::string_view(static_cast<const char*>(iov[1].iov_base), iov[1].iov_len), @@ -160,9 +170,13 @@ return ETH_HLEN + packet.length(); }); - EXPECT_CALL(mock_stats, OnPacketWritten(ETH_HLEN + packet.size(), _)) - .Times(1); - EXPECT_CALL(mock_visitor, OnWrite(StrEq(packet))).Times(1); + EXPECT_CALL(mock_stats, OnPacketWritten(ETH_HLEN + packet.size(), _)); + EXPECT_CALL( + mock_visitor, + OnWrite(IsOkAndHolds(ElementsAre(Field( + &QboneClientPacketExchanger::WriteResult::packet, + ElementsAreArray(reinterpret_cast<const std::byte*>(packet.data()), + packet.size())))))); tap_exchanger.WritePacketToNetwork(packet.data(), packet.size()); } @@ -173,7 +187,8 @@ errno = ECOMM; return -1; }); - EXPECT_CALL(mock_visitor_, OnReadError(_)); + EXPECT_CALL(mock_visitor_, OnRead(StatusIs(Ne(absl::StatusCode::kOk)))); + EXPECT_CALL(mock_stats_, OnReadError(_)); EXPECT_FALSE(exchanger_.ReadAndDeliverPacket(&mock_client_)); } @@ -183,8 +198,8 @@ errno = EAGAIN; return -1; }); - EXPECT_CALL(mock_stats_, OnReadError(_)).Times(1); - EXPECT_CALL(mock_visitor_, OnReadError(_)).Times(1); + EXPECT_CALL(mock_stats_, OnReadError(_)); + EXPECT_CALL(mock_visitor_, OnRead(StatusIs(Ne(absl::StatusCode::kOk)))); EXPECT_FALSE(exchanger_.ReadAndDeliverPacket(&mock_client_)); } @@ -198,7 +213,13 @@ return packet.size(); }); EXPECT_CALL(mock_client_, ProcessPacketFromNetwork(StrEq(packet))); - EXPECT_CALL(mock_stats_, OnPacketRead(_, _)).Times(1); + EXPECT_CALL(mock_stats_, OnPacketRead(_, _)); + EXPECT_CALL( + mock_visitor_, + OnRead(IsOkAndHolds(ElementsAre(Field( + &QboneClientPacketExchanger::ReadResult::packet, + ElementsAreArray(reinterpret_cast<const std::byte*>(packet.data()), + packet.size())))))); EXPECT_TRUE(exchanger_.ReadAndDeliverPacket(&mock_client_)); } @@ -242,6 +263,8 @@ return -1; }); + EXPECT_CALL(mock_stats_, OnWriteError(_)); + exchanger.WritePacketToNetwork(packet.data(), packet.size()); } @@ -275,6 +298,7 @@ errno = ECOMM; return -1; }); + EXPECT_CALL(mock_stats_, OnReadError(_)); EXPECT_FALSE(exchanger.ReadAndDeliverPacket(&mock_client_)); } @@ -291,7 +315,7 @@ MockKernel mock_kernel_; StrictMock<MockNetlink> mock_netlink_; - StrictMock<MockVisitor> mock_visitor_; + StrictMock<MockQboneClientPacketExchanger::MockVisitor> mock_visitor_; StrictMock<MockQboneClient> mock_client_; StrictMock<MockPacketExchangerStatsInterface> mock_stats_; TunDevicePacketExchanger exchanger_; @@ -308,7 +332,7 @@ l3_payload; ethhdr eth_hdr{}; - eth_hdr.h_proto = absl::ghtons(ETH_P_IPV6); + eth_hdr.h_proto = QuicheEndian::HostToNet16(ETH_P_IPV6); EXPECT_CALL(mock_kernel_, readv(kReadFd, _, 2)) .WillOnce( @@ -321,13 +345,19 @@ }); EXPECT_CALL(mock_client_, ProcessPacketFromNetwork(StrEq(l3_packet))); - EXPECT_CALL(mock_stats_, OnPacketRead(l3_packet.size(), _)).Times(1); + EXPECT_CALL(mock_stats_, OnPacketRead(l3_packet.size(), _)); + EXPECT_CALL( + mock_visitor_, + OnRead(IsOkAndHolds(ElementsAre(Field( + &QboneClientPacketExchanger::ReadResult::packet, + ElementsAreArray(reinterpret_cast<const std::byte*>(l3_packet.data()), + l3_packet.size())))))); EXPECT_TRUE(exchanger_.ReadAndDeliverPacket(&mock_client_)); } TEST_F(TunDevicePacketExchangerTapTest, ReadPacketTapInvalidL2) { ethhdr eth_hdr{}; - eth_hdr.h_proto = absl::ghtons(ETH_P_ARP); // Non-IPv6 + eth_hdr.h_proto = QuicheEndian::HostToNet16(ETH_P_ARP); // Non-IPv6 EXPECT_CALL(mock_kernel_, readv(kReadFd, _, 2)) .WillOnce([eth_hdr](int fd, const struct iovec* iov, int iovcnt) { @@ -335,7 +365,7 @@ return ETH_HLEN + 10; // Read some bytes }); - EXPECT_CALL(mock_visitor_, OnReadError(_)); + EXPECT_CALL(mock_visitor_, OnRead(StatusIs(Ne(absl::StatusCode::kOk)))); EXPECT_CALL(mock_stats_, OnReadError(_)); EXPECT_FALSE(exchanger_.ReadAndDeliverPacket(&mock_client_)); } @@ -359,7 +389,7 @@ sizeof(target_address)); ethhdr eth_hdr{}; - eth_hdr.h_proto = absl::ghtons(ETH_P_IPV6); + eth_hdr.h_proto = QuicheEndian::HostToNet16(ETH_P_IPV6); EXPECT_CALL(mock_kernel_, readv(kReadFd, _, 2)) .WillOnce( @@ -382,8 +412,8 @@ .WillOnce([](int fd, const struct iovec* iov, int iovcnt) -> ssize_t { return iov[0].iov_len + iov[1].iov_len; }); - EXPECT_CALL(mock_stats_, OnPacketWritten(_, _)).Times(1); - EXPECT_CALL(mock_visitor_, OnWrite(_)); + EXPECT_CALL(mock_stats_, OnPacketWritten(_, _)); + EXPECT_CALL(mock_visitor_, OnWrite(IsOkAndHolds(SizeIs(1)))); // ReadAndDeliverPacket should return false because packet was handled // internally (Neighbor Discovery).