QBONE Bonnet: Convert `TunDevicePacketExchanger::ReadPacket` to use kernel `readv` Refactor `TunDevicePacketExchanger::ReadPacket` to use `kernel_->readv` instead of read. This allows reading the L2 header (when is_tap_ is true) and L3 payload in a single syscall using iovec, avoiding the need to shift the buffer afterwards. PiperOrigin-RevId: 934614328
diff --git a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc index 667dc0d..5367127 100644 --- a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc +++ b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc
@@ -6,6 +6,7 @@ #include <netinet/icmp6.h> #include <netinet/ip6.h> +#include <sys/uio.h> #include <algorithm> #include <memory> @@ -31,11 +32,7 @@ netlink_(netlink), ifname_(ifname), is_tap_(is_tap), - stats_(stats) { - if (is_tap_) { - mtu_ += ETH_HLEN; - } -} + stats_(stats) {} bool TunDevicePacketExchanger::WritePacket(const char* packet, size_t size, std::string* error) { @@ -85,8 +82,18 @@ // 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(); - int result = kernel_->read(read_fd_, read_buffer.get(), mtu_); + result = kernel_->readv(read_fd_, iov, 2); + 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 @@ -101,13 +108,20 @@ return nullptr; } - auto buffer = std::make_unique<QuicData>(read_buffer.release(), result, true); - if (is_tap_) { - buffer = ConsumeL2Headers(*buffer); + if (is_tap_ && result < ETH_HLEN) { + *error = "Read packet too short for ethernet header."; + stats_->OnReadError(*error); + return nullptr; } - if (buffer) { - stats_->OnPacketRead(buffer->length(), latency); + + 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; } + + stats_->OnPacketRead(buffer->length(), latency); return buffer; } @@ -154,46 +168,41 @@ return std::make_unique<QuicData>(l2_buffer.release(), l2_packet_size, true); } -std::unique_ptr<QuicData> TunDevicePacketExchanger::ConsumeL2Headers( - const QuicData& l2_packet) { - if (l2_packet.length() < ETH_HLEN) { - // Packet is too short for ethernet headers. Drop it. - return nullptr; +bool TunDevicePacketExchanger::ValidateL2Headers(const ethhdr& eth_header, + const QuicData& packet) { + if (eth_header.h_proto != absl::ghtons(ETH_P_IPV6)) { + return false; } - auto* hdr = reinterpret_cast<const ethhdr*>(l2_packet.data()); - if (hdr->h_proto != absl::ghtons(ETH_P_IPV6)) { - return nullptr; - } - constexpr auto kIp6PrefixLen = ETH_HLEN + sizeof(ip6_hdr); + constexpr auto kIp6PrefixLen = sizeof(ip6_hdr); constexpr auto kIcmp6PrefixLen = kIp6PrefixLen + sizeof(icmp6_hdr); - if (l2_packet.length() < kIp6PrefixLen) { + if (packet.length() < kIp6PrefixLen) { // Packet is too short to be ipv6. Drop it. - return nullptr; + return false; } - auto* ip_hdr = reinterpret_cast<const ip6_hdr*>(l2_packet.data() + ETH_HLEN); + 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; bool is_neighbor_solicit = false; if (is_icmp) { - if (l2_packet.length() < kIcmp6PrefixLen) { + if (packet.length() < kIcmp6PrefixLen) { // Packet is too short to be icmp6. Drop it. - return nullptr; + return false; } is_neighbor_solicit = - reinterpret_cast<const icmp6_hdr*>(l2_packet.data() + kIp6PrefixLen) + reinterpret_cast<const icmp6_hdr*>(packet.data() + kIp6PrefixLen) ->icmp6_type == ND_NEIGHBOR_SOLICIT; } if (is_neighbor_solicit) { // If we've received a neighbor solicitation, craft an advertisement to // respond with and write it back to the local interface. - auto* icmp6_payload = l2_packet.data() + kIcmp6PrefixLen; + auto* icmp6_payload = packet.data() + kIcmp6PrefixLen; QuicIpAddress target_address( *reinterpret_cast<const in6_addr*>(icmp6_payload)); if (target_address != *QboneConstants::GatewayAddress()) { // Only respond to solicitations for our gateway address - return nullptr; + return false; } // Neighbor Advertisement crafted per: @@ -233,17 +242,10 @@ }); // Do not forward the neighbor solicitation through the tunnel since it's // link-local. - return nullptr; + return false; } - // If this isn't a Neighbor Solicitation, remove the L2 headers and forward - // it as though it were an L3 packet. - const auto l3_packet_size = l2_packet.length() - ETH_HLEN; - auto shift_buffer = std::make_unique<char[]>(l3_packet_size); - memcpy(shift_buffer.get(), l2_packet.data() + ETH_HLEN, l3_packet_size); - - return std::make_unique<QuicData>(shift_buffer.release(), l3_packet_size, - true); + return true; } } // 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 a8248da..6467226 100644 --- a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h +++ b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h
@@ -65,7 +65,7 @@ std::unique_ptr<QuicData> ApplyL2Headers(const QuicData& l3_packet); - std::unique_ptr<QuicData> ConsumeL2Headers(const QuicData& l2_packet); + bool ValidateL2Headers(const ethhdr& eth_header, const QuicData& packet); int read_fd_ = -1; int write_fd_ = -1;
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 4febbe2..b323f8e 100644 --- a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger_test.cc +++ b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger_test.cc
@@ -4,6 +4,11 @@ #include "quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h" +#include <arpa/inet.h> +#include <netinet/icmp6.h> +#include <netinet/ip6.h> +#include <sys/uio.h> + #include <string> #include "absl/status/status.h" @@ -12,6 +17,8 @@ #include "quiche/quic/qbone/bonnet/mock_packet_exchanger_stats_interface.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/qbone_constants.h" namespace quic::test { namespace { @@ -91,8 +98,8 @@ } TEST_F(TunDevicePacketExchangerTest, ReadPacketReturnsNullOnError) { - EXPECT_CALL(mock_kernel_, read(kReadFd, _, kMtu)) - .WillOnce([](int fd, void* buf, size_t count) { + EXPECT_CALL(mock_kernel_, readv(kReadFd, _, 2)) + .WillOnce([](int fd, const struct iovec* iov, int iovcnt) { errno = ECOMM; return -1; }); @@ -101,8 +108,8 @@ } TEST_F(TunDevicePacketExchangerTest, ReadPacketReturnsNullOnBlockedRead) { - EXPECT_CALL(mock_kernel_, read(kReadFd, _, kMtu)) - .WillOnce([](int fd, void* buf, size_t count) { + EXPECT_CALL(mock_kernel_, readv(kReadFd, _, 2)) + .WillOnce([](int fd, const struct iovec* iov, int iovcnt) { errno = EAGAIN; return -1; }); @@ -114,9 +121,11 @@ TEST_F(TunDevicePacketExchangerTest, ReadPacketReturnsThePacketOnSuccessfulRead) { std::string packet = "fake_packet"; - EXPECT_CALL(mock_kernel_, read(kReadFd, _, kMtu)) - .WillOnce([packet](int fd, void* buf, size_t count) { - memcpy(buf, packet.data(), packet.size()); + 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))); @@ -124,5 +133,114 @@ EXPECT_TRUE(exchanger_.ReadAndDeliverPacket(&mock_client_)); } +class TunDevicePacketExchangerTapTest : public QuicTest { + protected: + TunDevicePacketExchangerTapTest() + : exchanger_(kMtu, &mock_kernel_, &mock_netlink_, &mock_visitor_, true, + &mock_stats_, "tap0") { + exchanger_.set_read_file_descriptor(kReadFd); + exchanger_.set_write_file_descriptor(kWriteFd); + } + + ~TunDevicePacketExchangerTapTest() override = default; + + MockKernel mock_kernel_; + StrictMock<MockNetlink> mock_netlink_; + StrictMock<MockVisitor> mock_visitor_; + StrictMock<MockQboneClient> mock_client_; + StrictMock<MockPacketExchangerStatsInterface> mock_stats_; + TunDevicePacketExchanger exchanger_; +}; + +TEST_F(TunDevicePacketExchangerTapTest, ReadPacketTapSuccess) { + ip6_hdr ip_hdr{}; + ip_hdr.ip6_vfc = 0x60; // Version 6 + ip_hdr.ip6_nxt = 59; // No next header + + std::string l3_payload = "hello"; + std::string l3_packet = + std::string(reinterpret_cast<char*>(&ip_hdr), sizeof(ip_hdr)) + + l3_payload; + + ethhdr eth_hdr{}; + eth_hdr.h_proto = absl::ghtons(ETH_P_IPV6); + + EXPECT_CALL(mock_kernel_, readv(kReadFd, _, 2)) + .WillOnce( + [eth_hdr, l3_packet](int fd, const struct iovec* iov, int iovcnt) { + EXPECT_EQ(iov[0].iov_len, ETH_HLEN); + EXPECT_EQ(iov[1].iov_len, kMtu); + memcpy(iov[0].iov_base, ð_hdr, ETH_HLEN); + memcpy(iov[1].iov_base, l3_packet.data(), l3_packet.size()); + return ETH_HLEN + l3_packet.size(); + }); + + EXPECT_CALL(mock_client_, ProcessPacketFromNetwork(StrEq(l3_packet))); + EXPECT_CALL(mock_stats_, OnPacketRead(l3_packet.size(), _)).Times(1); + EXPECT_TRUE(exchanger_.ReadAndDeliverPacket(&mock_client_)); +} + +TEST_F(TunDevicePacketExchangerTapTest, ReadPacketTapInvalidL2) { + ethhdr eth_hdr{}; + eth_hdr.h_proto = absl::ghtons(ETH_P_ARP); // Non-IPv6 + + EXPECT_CALL(mock_kernel_, readv(kReadFd, _, 2)) + .WillOnce([eth_hdr](int fd, const struct iovec* iov, int iovcnt) { + memcpy(iov[0].iov_base, ð_hdr, ETH_HLEN); + return ETH_HLEN + 10; // Read some bytes + }); + + EXPECT_CALL(mock_visitor_, OnReadError("")); + EXPECT_FALSE(exchanger_.ReadAndDeliverPacket(&mock_client_)); +} + +TEST_F(TunDevicePacketExchangerTapTest, ReadPacketTapNeighborSolicitation) { + ip6_hdr ip_hdr{}; + ip_hdr.ip6_vfc = 0x60; // Version 6 + ip_hdr.ip6_nxt = IPPROTO_ICMPV6; + inet_pton(AF_INET6, "fe80::2", &ip_hdr.ip6_src); + inet_pton(AF_INET6, "fe80::1", &ip_hdr.ip6_dst); + + icmp6_hdr icmp_hdr{}; + icmp_hdr.icmp6_type = ND_NEIGHBOR_SOLICIT; + + in6_addr target_address = QboneConstants::GatewayAddress()->GetIPv6(); + + std::string l3_packet = + std::string(reinterpret_cast<char*>(&ip_hdr), sizeof(ip_hdr)) + + std::string(reinterpret_cast<char*>(&icmp_hdr), sizeof(icmp_hdr)) + + std::string(reinterpret_cast<char*>(&target_address), + sizeof(target_address)); + + ethhdr eth_hdr{}; + eth_hdr.h_proto = absl::ghtons(ETH_P_IPV6); + + EXPECT_CALL(mock_kernel_, readv(kReadFd, _, 2)) + .WillOnce( + [eth_hdr, l3_packet](int fd, const struct iovec* iov, int iovcnt) { + memcpy(iov[0].iov_base, ð_hdr, ETH_HLEN); + memcpy(iov[1].iov_base, l3_packet.data(), l3_packet.size()); + return ETH_HLEN + l3_packet.size(); + }); + + // Expect GetLinkInfo to populate ethhdr on writing neighbor solicit response. + EXPECT_CALL(mock_netlink_, GetLinkInfo("tap0", _)) + .WillOnce( + [](const std::string& ifname, NetlinkInterface::LinkInfo* link_info) { + memset(link_info->hardware_address, 0x12, ETH_ALEN); + return true; + }); + + // Expect neighbor solicitation response to be written out. + EXPECT_CALL(mock_kernel_, write(kWriteFd, _, _)) + .WillOnce([](int fd, const void* buf, size_t count) { return count; }); + EXPECT_CALL(mock_stats_, OnPacketWritten(_, _)).Times(1); + + // ReadAndDeliverPacket should return false because packet was handled + // internally (Neighbor Discovery). + EXPECT_CALL(mock_visitor_, OnReadError("")); + EXPECT_FALSE(exchanger_.ReadAndDeliverPacket(&mock_client_)); +} + } // namespace } // namespace quic::test
diff --git a/quiche/quic/qbone/platform/kernel_interface.h b/quiche/quic/qbone/platform/kernel_interface.h index f1f97a1..c46d22f 100644 --- a/quiche/quic/qbone/platform/kernel_interface.h +++ b/quiche/quic/qbone/platform/kernel_interface.h
@@ -11,6 +11,7 @@ #include <sys/ioctl.h> #include <sys/socket.h> #include <sys/types.h> +#include <sys/uio.h> #include <unistd.h> #include <cstddef> @@ -30,6 +31,7 @@ virtual int ioctl(int fd, int request, void* argp) = 0; virtual int open(const char* pathname, int flags) = 0; virtual ssize_t read(int fd, void* buf, size_t count) = 0; + virtual ssize_t readv(int fd, const struct iovec* iov, int iovcnt) = 0; virtual ssize_t recvfrom(int sockfd, void* buf, size_t len, int flags, struct sockaddr* src_addr, socklen_t* addrlen) = 0; virtual int recvmmsg(int sockfd, struct mmsghdr* msgvec, unsigned int vlen, @@ -95,6 +97,10 @@ static Runner syscall("read"); return syscall.Run(&::read, fd, buf, count); } + ssize_t readv(int fd, const struct iovec* iov, int iovcnt) override { + static Runner syscall("readv"); + return syscall.Run(&::readv, fd, iov, iovcnt); + } ssize_t recvfrom(int sockfd, void* buf, size_t len, int flags, struct sockaddr* src_addr, socklen_t* addrlen) override { static Runner syscall("recvfrom");
diff --git a/quiche/quic/qbone/platform/mock_kernel.h b/quiche/quic/qbone/platform/mock_kernel.h index d86238d..612341b 100644 --- a/quiche/quic/qbone/platform/mock_kernel.h +++ b/quiche/quic/qbone/platform/mock_kernel.h
@@ -5,6 +5,8 @@ #ifndef QUICHE_QUIC_QBONE_PLATFORM_MOCK_KERNEL_H_ #define QUICHE_QUIC_QBONE_PLATFORM_MOCK_KERNEL_H_ +#include <sys/uio.h> + #include <cstddef> #include <ctime> @@ -23,6 +25,8 @@ MOCK_METHOD(int, ioctl, (int fd, int request, void*), (override)); MOCK_METHOD(int, open, (const char*, int flags), (override)); MOCK_METHOD(ssize_t, read, (int fd, void*, size_t count), (override)); + MOCK_METHOD(ssize_t, readv, (int fd, const struct iovec* iov, int iovcnt), + (override)); MOCK_METHOD(ssize_t, recvfrom, (int sockfd, void*, size_t len, int flags, struct sockaddr*, socklen_t*),