QBONE Bonnet: Prepare for ability to split Read/Write fds in `TunDevice`. This modifies the `TunDeviceInterface` API such that it has separate accessors for a read and a write file descriptor. For now, the concrete implementation is unmodified and returns its single file descriptor from both, matching current behavior. A future implementation of the interface could in theory open and return distinct fds. Protected by n/a, not GFE. PiperOrigin-RevId: 868335506
diff --git a/quiche/quic/qbone/bonnet/mock_tun_device.h b/quiche/quic/qbone/bonnet/mock_tun_device.h index b712db5..bc5b462 100644 --- a/quiche/quic/qbone/bonnet/mock_tun_device.h +++ b/quiche/quic/qbone/bonnet/mock_tun_device.h
@@ -20,7 +20,8 @@ MOCK_METHOD(void, CloseDevice, (), (override)); - MOCK_METHOD(int, GetFileDescriptor, (), (const, override)); + MOCK_METHOD(int, GetReadFileDescriptor, (), (const, override)); + MOCK_METHOD(int, GetWriteFileDescriptor, (), (const, override)); }; } // namespace quic
diff --git a/quiche/quic/qbone/bonnet/tun_device.cc b/quiche/quic/qbone/bonnet/tun_device.cc index a39aabf..b9c6d1b 100644 --- a/quiche/quic/qbone/bonnet/tun_device.cc +++ b/quiche/quic/qbone/bonnet/tun_device.cc
@@ -5,6 +5,7 @@ #include "quiche/quic/qbone/bonnet/tun_device.h" #include <fcntl.h> +#include <linux/filter.h> #include <linux/if_tun.h> #include <net/if.h> #include <sys/ioctl.h> @@ -14,6 +15,7 @@ #include <string> #include "absl/cleanup/cleanup.h" +#include "absl/flags/flag.h" #include "quiche/quic/platform/api/quic_bug_tracker.h" #include "quiche/quic/platform/api/quic_logging.h" #include "quiche/quic/qbone/platform/kernel_interface.h" @@ -93,7 +95,50 @@ return NetdeviceIoctl(SIOCSIFFLAGS, reinterpret_cast<void*>(&if_request)); } -int TunTapDevice::GetFileDescriptor() const { return file_descriptor_; } +bool TunTapDevice::CheckFeatures(KernelInterface& kernel, int tun_device_fd) { + unsigned int actual_features; + if (kernel.ioctl(tun_device_fd, TUNGETFEATURES, &actual_features) != 0) { + QUIC_PLOG(WARNING) << "Failed to TUNGETFEATURES"; + return false; + } + unsigned int required_features = IFF_TUN | IFF_NO_PI; + if ((required_features & actual_features) != required_features) { + QUIC_LOG(WARNING) + << "Required feature does not exist. required_features: 0x" << std::hex + << required_features << " vs actual_features: 0x" << std::hex + << actual_features; + return false; + } + return true; +} + +bool TunTapDevice::OpenFileDescriptor(KernelInterface& kernel, + const std::string& path, ifreq if_request, + int flags, bool persist, int* return_fd) { + *return_fd = kernel.open(path.c_str(), flags); + if (*return_fd < 0) { + QUIC_PLOG(WARNING) << "Failed to open " << path; + *return_fd = kInvalidFd; + return false; + } + if (!CheckFeatures(kernel, *return_fd)) { + return false; + } + + if (kernel.ioctl(*return_fd, TUNSETIFF, + reinterpret_cast<void*>(&if_request)) != 0) { + QUIC_PLOG(WARNING) << "Failed to TUNSETIFF on fd(" << *return_fd << ")"; + return false; + } + + if (kernel.ioctl(*return_fd, TUNSETPERSIST, + persist ? reinterpret_cast<void*>(&if_request) : nullptr) != + 0) { + QUIC_PLOG(WARNING) << "Failed to TUNSETPERSIST on fd(" << *return_fd << ")"; + return false; + } + return true; +} bool TunTapDevice::OpenDevice() { if (file_descriptor_ != kInvalidFd) { @@ -127,28 +172,10 @@ } }); - const std::string tun_device_path = - absl::GetFlag(FLAGS_qbone_client_tun_device_path); - int fd = kernel_.open(tun_device_path.c_str(), O_RDWR); - if (fd < 0) { - QUIC_PLOG(WARNING) << "Failed to open " << tun_device_path; - return successfully_opened; - } - file_descriptor_ = fd; - if (!CheckFeatures(fd)) { - return successfully_opened; - } - - if (kernel_.ioctl(fd, TUNSETIFF, reinterpret_cast<void*>(&if_request)) != 0) { - QUIC_PLOG(WARNING) << "Failed to TUNSETIFF on fd(" << fd << ")"; - return successfully_opened; - } - - if (kernel_.ioctl( - fd, TUNSETPERSIST, - persist_ ? reinterpret_cast<void*>(&if_request) : nullptr) != 0) { - QUIC_PLOG(WARNING) << "Failed to TUNSETPERSIST on fd(" << fd << ")"; - return successfully_opened; + if (!OpenFileDescriptor(kernel_, + absl::GetFlag(FLAGS_qbone_client_tun_device_path), + if_request, O_RDWR, persist_, &file_descriptor_)) { + return false; } successfully_opened = true; @@ -177,23 +204,6 @@ return true; } -bool TunTapDevice::CheckFeatures(int tun_device_fd) { - unsigned int actual_features; - if (kernel_.ioctl(tun_device_fd, TUNGETFEATURES, &actual_features) != 0) { - QUIC_PLOG(WARNING) << "Failed to TUNGETFEATURES"; - return false; - } - unsigned int required_features = IFF_TUN | IFF_NO_PI; - if ((required_features & actual_features) != required_features) { - QUIC_LOG(WARNING) - << "Required feature does not exist. required_features: 0x" << std::hex - << required_features << " vs actual_features: 0x" << std::hex - << actual_features; - return false; - } - return true; -} - bool TunTapDevice::NetdeviceIoctl(int request, void* argp) { int fd = kernel_.socket(AF_INET6, SOCK_DGRAM, 0); if (fd < 0) {
diff --git a/quiche/quic/qbone/bonnet/tun_device.h b/quiche/quic/qbone/bonnet/tun_device.h index ccc22cb..8eb1615 100644 --- a/quiche/quic/qbone/bonnet/tun_device.h +++ b/quiche/quic/qbone/bonnet/tun_device.h
@@ -51,9 +51,15 @@ // CloseDevice. void CloseDevice() override; - // Gets the file descriptor that can be used to send/receive packets. - // This returns -1 when the TUN device is in an invalid state. - int GetFileDescriptor() const override; + // Get the file descriptors that can be used to send/receive packets. + // These return -1 when the TUN device is in an invalid state. + int GetReadFileDescriptor() const override { return file_descriptor_; } + int GetWriteFileDescriptor() const override { return file_descriptor_; } + + static bool CheckFeatures(KernelInterface& kernel, int tun_device_fd); + static bool OpenFileDescriptor(KernelInterface& kernel, + const std::string& path, ifreq if_request, + int flags, bool persist, int* return_fd); private: // Creates or reopens the tun device. @@ -62,9 +68,6 @@ // Configure the interface. bool ConfigureInterface(); - // Checks if the required kernel features exists. - bool CheckFeatures(int tun_device_fd); - // Opens a socket and makes netdevice ioctl call bool NetdeviceIoctl(int request, void* argp);
diff --git a/quiche/quic/qbone/bonnet/tun_device_interface.h b/quiche/quic/qbone/bonnet/tun_device_interface.h index e88efa9..1bec7e9 100644 --- a/quiche/quic/qbone/bonnet/tun_device_interface.h +++ b/quiche/quic/qbone/bonnet/tun_device_interface.h
@@ -28,9 +28,10 @@ // CloseDevice. virtual void CloseDevice() = 0; - // Gets the file descriptor that can be used to send/receive packets. - // This returns -1 when the TUN device is in an invalid state. - virtual int GetFileDescriptor() const = 0; + // Get the file descriptors that can be used to send/receive packets. + // These return -1 when the TUN device is in an invalid state. + virtual int GetReadFileDescriptor() const = 0; + virtual int GetWriteFileDescriptor() const = 0; }; } // namespace quic
diff --git a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc index 946cdcd..3d975d6 100644 --- a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc +++ b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.cc
@@ -36,8 +36,9 @@ bool TunDevicePacketExchanger::WritePacket(const char* packet, size_t size, std::string* error) { - if (fd_ < 0) { - *error = absl::StrCat("Invalid file descriptor of the TUN device: ", fd_); + if (write_fd_ < 0) { + *error = + absl::StrCat("Invalid file descriptor of the TUN device: ", write_fd_); stats_->OnWriteError(error); return false; } @@ -46,12 +47,13 @@ if (is_tap_) { buffer = ApplyL2Headers(*buffer); } - int result = kernel_->write(fd_, buffer->data(), buffer->length()); + int result = kernel_->write(write_fd_, buffer->data(), buffer->length()); 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. + // 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(); @@ -66,15 +68,16 @@ std::unique_ptr<QuicData> TunDevicePacketExchanger::ReadPacket( std::string* error) { - if (fd_ < 0) { - *error = absl::StrCat("Invalid file descriptor of the TUN device: ", fd_); + if (read_fd_ < 0) { + *error = + absl::StrCat("Invalid file descriptor of the TUN device: ", read_fd_); stats_->OnReadError(error); return nullptr; } // 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 = kernel_->read(fd_, read_buffer.get(), mtu_); + int result = kernel_->read(read_fd_, read_buffer.get(), mtu_); // 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) { @@ -97,7 +100,12 @@ return buffer; } -void TunDevicePacketExchanger::set_file_descriptor(int fd) { fd_ = fd; } +void TunDevicePacketExchanger::set_read_file_descriptor(int fd) { + read_fd_ = fd; +} +void TunDevicePacketExchanger::set_write_file_descriptor(int fd) { + write_fd_ = fd; +} const TunDevicePacketExchanger::StatsInterface* TunDevicePacketExchanger::stats_interface() const {
diff --git a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h index 57f0bf6..1f4d0fc 100644 --- a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h +++ b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger.h
@@ -47,7 +47,8 @@ QbonePacketExchanger::Visitor* visitor, bool is_tap, StatsInterface* stats, absl::string_view ifname); - void set_file_descriptor(int fd); + void set_read_file_descriptor(int fd); + void set_write_file_descriptor(int fd); ABSL_MUST_USE_RESULT const StatsInterface* stats_interface() const; @@ -63,7 +64,8 @@ std::unique_ptr<QuicData> ConsumeL2Headers(const QuicData& l2_packet); - int fd_ = -1; + int read_fd_ = -1; + int write_fd_ = -1; size_t mtu_; KernelInterface* kernel_; NetlinkInterface* netlink_;
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 cd9f668..b5a4de9 100644 --- a/quiche/quic/qbone/bonnet/tun_device_packet_exchanger_test.cc +++ b/quiche/quic/qbone/bonnet/tun_device_packet_exchanger_test.cc
@@ -17,7 +17,8 @@ namespace { const size_t kMtu = 1000; -const int kFd = 15; +const int kReadFd = 15; +const int kWriteFd = 16; using ::testing::_; using ::testing::StrEq; @@ -35,7 +36,8 @@ TunDevicePacketExchangerTest() : exchanger_(kMtu, &mock_kernel_, nullptr, &mock_visitor_, false, &mock_stats_, absl::string_view()) { - exchanger_.set_file_descriptor(kFd); + exchanger_.set_read_file_descriptor(kReadFd); + exchanger_.set_write_file_descriptor(kWriteFd); } ~TunDevicePacketExchangerTest() override = default; @@ -49,7 +51,7 @@ TEST_F(TunDevicePacketExchangerTest, WritePacketReturnsFalseOnError) { std::string packet = "fake packet"; - EXPECT_CALL(mock_kernel_, write(kFd, _, packet.size())) + EXPECT_CALL(mock_kernel_, write(kWriteFd, _, packet.size())) .WillOnce([](int fd, const void* buf, size_t count) { errno = ECOMM; return -1; @@ -63,7 +65,7 @@ TEST_F(TunDevicePacketExchangerTest, WritePacketReturnFalseAndBlockedOnBlockedTunnel) { std::string packet = "fake packet"; - EXPECT_CALL(mock_kernel_, write(kFd, _, packet.size())) + EXPECT_CALL(mock_kernel_, write(kWriteFd, _, packet.size())) .WillOnce([](int fd, const void* buf, size_t count) { errno = EAGAIN; return -1; @@ -77,7 +79,7 @@ TEST_F(TunDevicePacketExchangerTest, WritePacketReturnsTrueOnSuccessfulWrite) { std::string packet = "fake packet"; - EXPECT_CALL(mock_kernel_, write(kFd, _, packet.size())) + 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; @@ -89,7 +91,7 @@ } TEST_F(TunDevicePacketExchangerTest, ReadPacketReturnsNullOnError) { - EXPECT_CALL(mock_kernel_, read(kFd, _, kMtu)) + EXPECT_CALL(mock_kernel_, read(kReadFd, _, kMtu)) .WillOnce([](int fd, void* buf, size_t count) { errno = ECOMM; return -1; @@ -99,7 +101,7 @@ } TEST_F(TunDevicePacketExchangerTest, ReadPacketReturnsNullOnBlockedRead) { - EXPECT_CALL(mock_kernel_, read(kFd, _, kMtu)) + EXPECT_CALL(mock_kernel_, read(kReadFd, _, kMtu)) .WillOnce([](int fd, void* buf, size_t count) { errno = EAGAIN; return -1; @@ -112,7 +114,7 @@ TEST_F(TunDevicePacketExchangerTest, ReadPacketReturnsThePacketOnSuccessfulRead) { std::string packet = "fake_packet"; - EXPECT_CALL(mock_kernel_, read(kFd, _, kMtu)) + EXPECT_CALL(mock_kernel_, read(kReadFd, _, kMtu)) .WillOnce([packet](int fd, void* buf, size_t count) { memcpy(buf, packet.data(), packet.size()); return packet.size();
diff --git a/quiche/quic/qbone/bonnet/tun_device_test.cc b/quiche/quic/qbone/bonnet/tun_device_test.cc index 5bb50e1..1e3b9de 100644 --- a/quiche/quic/qbone/bonnet/tun_device_test.cc +++ b/quiche/quic/qbone/bonnet/tun_device_test.cc
@@ -27,7 +27,7 @@ // Quite a bit of EXPECT_CALL().Times(AnyNumber()).WillRepeatedly() are used to // make sure we can correctly set common expectations and override the // expectation with later call to EXPECT_CALL(). ON_CALL cannot be used here -// since when EPXECT_CALL overrides ON_CALL, it ignores the parameter matcher +// since when EXPECT_CALL overrides ON_CALL, it ignores the parameter matcher // which results in unexpected call even if ON_CALL exists. class TunDeviceTest : public QuicTest { protected: @@ -124,7 +124,8 @@ SetInitExpectations(/* mtu = */ 1500, /* persist = */ false); TunTapDevice tun_device(kDeviceName, 1500, false, true, false, &mock_kernel_); EXPECT_TRUE(tun_device.Init()); - EXPECT_GT(tun_device.GetFileDescriptor(), -1); + EXPECT_GT(tun_device.GetReadFileDescriptor(), -1); + EXPECT_GT(tun_device.GetWriteFileDescriptor(), -1); ExpectUp(/* fail = */ false); EXPECT_TRUE(tun_device.Up()); @@ -137,7 +138,8 @@ .WillOnce(Return(-1)); TunTapDevice tun_device(kDeviceName, 1500, false, true, false, &mock_kernel_); EXPECT_FALSE(tun_device.Init()); - EXPECT_EQ(tun_device.GetFileDescriptor(), -1); + EXPECT_EQ(tun_device.GetReadFileDescriptor(), -1); + EXPECT_EQ(tun_device.GetWriteFileDescriptor(), -1); ExpectDown(false); } @@ -146,7 +148,8 @@ EXPECT_CALL(mock_kernel_, ioctl(_, TUNGETFEATURES, _)).WillOnce(Return(-1)); TunTapDevice tun_device(kDeviceName, 1500, false, true, false, &mock_kernel_); EXPECT_FALSE(tun_device.Init()); - EXPECT_EQ(tun_device.GetFileDescriptor(), -1); + EXPECT_EQ(tun_device.GetReadFileDescriptor(), -1); + EXPECT_EQ(tun_device.GetWriteFileDescriptor(), -1); ExpectDown(false); } @@ -160,7 +163,8 @@ }); TunTapDevice tun_device(kDeviceName, 1500, false, true, false, &mock_kernel_); EXPECT_FALSE(tun_device.Init()); - EXPECT_EQ(tun_device.GetFileDescriptor(), -1); + EXPECT_EQ(tun_device.GetReadFileDescriptor(), -1); + EXPECT_EQ(tun_device.GetWriteFileDescriptor(), -1); ExpectDown(false); } @@ -169,7 +173,8 @@ EXPECT_CALL(mock_kernel_, ioctl(_, TUNSETIFF, _)).WillOnce(Return(-1)); TunTapDevice tun_device(kDeviceName, 1500, true, true, false, &mock_kernel_); EXPECT_FALSE(tun_device.Init()); - EXPECT_EQ(tun_device.GetFileDescriptor(), -1); + EXPECT_EQ(tun_device.GetReadFileDescriptor(), -1); + EXPECT_EQ(tun_device.GetWriteFileDescriptor(), -1); } TEST_F(TunDeviceTest, FailToPersistDevice) { @@ -177,7 +182,8 @@ EXPECT_CALL(mock_kernel_, ioctl(_, TUNSETPERSIST, _)).WillOnce(Return(-1)); TunTapDevice tun_device(kDeviceName, 1500, true, true, false, &mock_kernel_); EXPECT_FALSE(tun_device.Init()); - EXPECT_EQ(tun_device.GetFileDescriptor(), -1); + EXPECT_EQ(tun_device.GetReadFileDescriptor(), -1); + EXPECT_EQ(tun_device.GetWriteFileDescriptor(), -1); } TEST_F(TunDeviceTest, FailToOpenSocket) { @@ -185,7 +191,8 @@ EXPECT_CALL(mock_kernel_, socket(AF_INET6, _, _)).WillOnce(Return(-1)); TunTapDevice tun_device(kDeviceName, 1500, true, true, false, &mock_kernel_); EXPECT_FALSE(tun_device.Init()); - EXPECT_EQ(tun_device.GetFileDescriptor(), -1); + EXPECT_EQ(tun_device.GetReadFileDescriptor(), -1); + EXPECT_EQ(tun_device.GetWriteFileDescriptor(), -1); } TEST_F(TunDeviceTest, FailToSetMtu) { @@ -193,14 +200,16 @@ EXPECT_CALL(mock_kernel_, ioctl(_, SIOCSIFMTU, _)).WillOnce(Return(-1)); TunTapDevice tun_device(kDeviceName, 1500, true, true, false, &mock_kernel_); EXPECT_FALSE(tun_device.Init()); - EXPECT_EQ(tun_device.GetFileDescriptor(), -1); + EXPECT_EQ(tun_device.GetReadFileDescriptor(), -1); + EXPECT_EQ(tun_device.GetWriteFileDescriptor(), -1); } TEST_F(TunDeviceTest, FailToUp) { SetInitExpectations(/* mtu = */ 1500, /* persist = */ true); TunTapDevice tun_device(kDeviceName, 1500, true, true, false, &mock_kernel_); EXPECT_TRUE(tun_device.Init()); - EXPECT_GT(tun_device.GetFileDescriptor(), -1); + EXPECT_GT(tun_device.GetReadFileDescriptor(), -1); + EXPECT_GT(tun_device.GetWriteFileDescriptor(), -1); ExpectUp(/* fail = */ true); EXPECT_FALSE(tun_device.Up());