QBONE Bonnet: use `writev` to eliminate copies when adding eth headers. In TAP mode, Bonnet needs to take L3 packets decapped from the QUIC connection and deliver them as L2 packets to the kernel. It does this by prepending an ethernet header to the packet write. For bonnet, this header is a fixed dummy header with the MAC of the device as src/dest and proto IPv6 and so can be generated once. Additionally, instead of copying the payload into a new buffer, we can use `writev` to prepend the fixed header when present. Most bonnet clients do not use TAP mode, but Borneo is using TAP mode to make passing the device into their TIVM easier. Removal of copies from the read path occurred in cl/934614328. PiperOrigin-RevId: 940562139
diff --git a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc index 5367127..d15c38c 100644 --- a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc +++ b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc
@@ -43,13 +43,17 @@ return false; } - auto buffer = std::make_unique<QuicData>(packet, size); - if (is_tap_) { - buffer = ApplyL2Headers(*buffer); + 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_->write(write_fd_, buffer->data(), buffer->length()); + int result = kernel_->writev(write_fd_, iov, 2); absl::Duration latency = std::max(absl::Now() - start, absl::ZeroDuration()); if (result == -1) { @@ -137,35 +141,22 @@ return stats_; } -std::unique_ptr<QuicData> TunDevicePacketExchanger::ApplyL2Headers( - const QuicData& l3_packet) { - if (is_tap_ && !mac_initialized_) { +void TunDevicePacketExchanger::InitializeEthHdr() { + if (!eth_hdr_initialized_) { NetlinkInterface::LinkInfo link_info{}; if (netlink_->GetLinkInfo(ifname_, &link_info)) { - memcpy(tap_mac_, link_info.hardware_address, ETH_ALEN); - mac_initialized_ = true; + // Set src & dst to my own address + memcpy(ð_hdr_.h_dest, link_info.hardware_address, ETH_ALEN); + 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_initialized_ = true; } else { QUIC_LOG_EVERY_N_SEC(ERROR, 30) << "Unable to get link info for: " << ifname_; } } - - const auto l2_packet_size = l3_packet.length() + ETH_HLEN; - auto l2_buffer = std::make_unique<char[]>(l2_packet_size); - - // Populate the Ethernet header - auto* hdr = reinterpret_cast<ethhdr*>(l2_buffer.get()); - // Set src & dst to my own address - memcpy(hdr->h_dest, tap_mac_, ETH_ALEN); - memcpy(hdr->h_source, tap_mac_, ETH_ALEN); - // Assume ipv6 for now - // TODO(b/195113643): Support additional protocols. - hdr->h_proto = absl::ghtons(ETH_P_IPV6); - - // Copy the l3 packet into buffer, just after the ethernet header. - memcpy(l2_buffer.get() + ETH_HLEN, l3_packet.data(), l3_packet.length()); - - return std::make_unique<QuicData>(l2_buffer.release(), l2_packet_size, true); } bool TunDevicePacketExchanger::ValidateL2Headers(const ethhdr& eth_header, @@ -224,7 +215,8 @@ int pos = sizeof(in6_addr); payload[pos++] = ND_OPT_TARGET_LINKADDR; // Type payload[pos++] = 1; // Length in units of 8 octets - memcpy(&payload[pos], tap_mac_, ETH_ALEN); // This interfaces' MAC address + memcpy(&payload[pos], eth_hdr_.h_source, + ETH_ALEN); // This interfaces' MAC address // Populate the ICMPv6 header icmp6_hdr response_hdr{};
diff --git a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h index 6467226..c953444 100644 --- a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h +++ b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h
@@ -63,7 +63,7 @@ bool WritePacket(const char* packet, size_t size, std::string* error) override; - std::unique_ptr<QuicData> ApplyL2Headers(const QuicData& l3_packet); + void InitializeEthHdr(); bool ValidateL2Headers(const ethhdr& eth_header, const QuicData& packet); @@ -75,8 +75,8 @@ const std::string ifname_; const bool is_tap_; - uint8_t tap_mac_[ETH_ALEN]{}; - bool mac_initialized_ = false; + 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 b323f8e..fd22342 100644 --- a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger_test.cc +++ b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger_test.cc
@@ -58,8 +58,13 @@ TEST_F(TunDevicePacketExchangerTest, WritePacketReturnsFalseOnError) { std::string packet = "fake packet"; - EXPECT_CALL(mock_kernel_, write(kWriteFd, _, packet.size())) - .WillOnce([](int fd, const void* buf, size_t count) { + 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; }); @@ -72,8 +77,13 @@ TEST_F(TunDevicePacketExchangerTest, WritePacketReturnFalseAndBlockedOnBlockedTunnel) { std::string packet = "fake packet"; - EXPECT_CALL(mock_kernel_, write(kWriteFd, _, packet.size())) - .WillOnce([](int fd, const void* buf, size_t count) { + 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 = EAGAIN; return -1; }); @@ -86,17 +96,74 @@ TEST_F(TunDevicePacketExchangerTest, WritePacketReturnsTrueOnSuccessfulWrite) { std::string packet = "fake packet"; - EXPECT_CALL(mock_kernel_, write(kWriteFd, _, packet.size())) - .WillOnce([packet](int fd, const void* buf, size_t count) { - EXPECT_THAT(reinterpret_cast<const char*>(buf), StrEq(packet)); - return count; - }); + 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(_, _)).Times(1); + EXPECT_CALL(mock_stats_, OnPacketWritten(packet.size(), _)).Times(1); EXPECT_CALL(mock_visitor_, OnWrite(StrEq(packet))).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<MockPacketExchangerStatsInterface> mock_stats; + TunDevicePacketExchanger tap_exchanger(kMtu, &mock_kernel, &mock_netlink, + &mock_visitor, /*is_tap=*/true, + &mock_stats, "tap0"); + tap_exchanger.set_write_file_descriptor(kWriteFd); + + std::string packet = "fake packet"; + + // Expectations on Netlink to get the hardware address when writing the + // first packet + EXPECT_CALL(mock_netlink, GetLinkInfo("tap0", _)) + .WillOnce([](const std::string& ifname, + NetlinkInterface::LinkInfo* link_info) -> bool { + uint8_t mac[ETH_ALEN] = {0x00, 0x11, 0x22, 0x33, 0x44, 0x55}; + memcpy(link_info->hardware_address, mac, ETH_ALEN); + return true; + }); + + // iov[0] should contain the Ethernet header populated by InitializeEthHdr + EXPECT_CALL(mock_kernel, writev(kWriteFd, _, 2)) + .WillOnce([&packet](int fd, const struct iovec* iov, + int iovcnt) -> ssize_t { + EXPECT_NE(iov[0].iov_base, nullptr); + EXPECT_EQ(iov[0].iov_len, ETH_HLEN); + + const char* first_buffer = static_cast<const char*>(iov[0].iov_base); + EXPECT_EQ(absl::string_view(first_buffer, ETH_ALEN), + absl::string_view("\x00\x11\x22\x33\x44\x55", ETH_ALEN)); + EXPECT_EQ(absl::string_view(first_buffer + ETH_ALEN, ETH_ALEN), + absl::string_view("\x00\x11\x22\x33\x44\x55", ETH_ALEN)); + + uint16_t proto; + memcpy(&proto, first_buffer + 2 * ETH_ALEN, 2); + EXPECT_EQ(proto, absl::ghtons(ETH_P_IPV6)); + + EXPECT_EQ(absl::string_view(static_cast<const char*>(iov[1].iov_base), + iov[1].iov_len), + "fake packet"); + 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); + + tap_exchanger.WritePacketToNetwork(packet.data(), packet.size()); +} + TEST_F(TunDevicePacketExchangerTest, ReadPacketReturnsNullOnError) { EXPECT_CALL(mock_kernel_, readv(kReadFd, _, 2)) .WillOnce([](int fd, const struct iovec* iov, int iovcnt) { @@ -232,8 +299,10 @@ }); // 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_kernel_, writev(kWriteFd, _, 2)) + .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); // ReadAndDeliverPacket should return false because packet was handled
diff --git a/quiche/quic/qbone/platform/kernel_interface.h b/quiche/quic/qbone/platform/kernel_interface.h index c46d22f..9fa2a6f 100644 --- a/quiche/quic/qbone/platform/kernel_interface.h +++ b/quiche/quic/qbone/platform/kernel_interface.h
@@ -44,6 +44,7 @@ virtual int setsockopt(int fd, int level, int optname, const void* optval, socklen_t optlen) = 0; virtual ssize_t write(int fd, const void* buf, size_t count) = 0; + virtual ssize_t writev(int fd, const struct iovec* iov, int iovcnt) = 0; virtual int getsockname(int sockfd, struct sockaddr* addr, socklen_t* addrlen) = 0; virtual unsigned int if_nametoindex(const char* ifname) = 0; @@ -136,6 +137,10 @@ static Runner syscall("write"); return syscall.Run(&::write, fd, buf, count); } + ssize_t writev(int fd, const struct iovec* iov, int iovcnt) override { + static Runner syscall("writev"); + return syscall.Run(&::writev, fd, iov, iovcnt); + } int getsockname(int sockfd, struct sockaddr* addr, socklen_t* addrlen) override { static Runner syscall("getsockname");
diff --git a/quiche/quic/qbone/platform/mock_kernel.h b/quiche/quic/qbone/platform/mock_kernel.h index 612341b..370fd19 100644 --- a/quiche/quic/qbone/platform/mock_kernel.h +++ b/quiche/quic/qbone/platform/mock_kernel.h
@@ -45,6 +45,8 @@ MOCK_METHOD(int, setsockopt, (int, int, int, const void*, socklen_t), (override)); MOCK_METHOD(ssize_t, write, (int fd, const void*, size_t count), (override)); + MOCK_METHOD(ssize_t, writev, (int fd, const struct iovec* iov, int iovcnt), + (override)); MOCK_METHOD(int, getsockname, (int sockfd, struct sockaddr* addr, socklen_t* addrlen), (override));