Automated g4 rollback of changelist 373010349.
*** Reason for rollback ***
This change no longer downs the interface when taking down the qbone tunnel. Instead, we close the fd we have for interacting with the TUN -- this has the effect of removing the interface when nopersist is set on the device.
*** Original change description ***
Automated g4 rollback of changelist 372620094.
*** Reason for rollback ***
Breaking Endor because we bring down the TUN Device when tearing down the bonnet in the init container.
*** Original change description ***
Defer TUN device creation until after the quic connection has been established.
IOS XR's XR container takes a few seconds for networking state to be sync'd from the host -- during which time any manipulations to the linux contaier's host networking stack breaks the synchronizatio...
***
PiperOrigin-RevId: 373210829
diff --git a/quic/qbone/bonnet/mock_tun_device.h b/quic/qbone/bonnet/mock_tun_device.h
index d593913..056346c 100644
--- a/quic/qbone/bonnet/mock_tun_device.h
+++ b/quic/qbone/bonnet/mock_tun_device.h
@@ -18,6 +18,8 @@
MOCK_METHOD(bool, Down, (), (override));
+ MOCK_METHOD(void, CloseDevice, (), (override));
+
MOCK_METHOD(int, GetFileDescriptor, (), (const, override));
};
diff --git a/quic/qbone/bonnet/tun_device.cc b/quic/qbone/bonnet/tun_device.cc
index 3ca52e8..86e68d7 100644
--- a/quic/qbone/bonnet/tun_device.cc
+++ b/quic/qbone/bonnet/tun_device.cc
@@ -39,7 +39,7 @@
if (!persist_) {
Down();
}
- CleanUpFileDescriptor();
+ CloseDevice();
}
bool TunDevice::Init() {
@@ -63,39 +63,33 @@
// TODO(pengg): might be better to use netlink socket, once we have a library to
// use
bool TunDevice::Up() {
- if (setup_tun_ && !is_interface_up_) {
- struct ifreq if_request;
- memset(&if_request, 0, sizeof(if_request));
- // copy does not zero-terminate the result string, but we've memset the
- // entire struct.
- interface_name_.copy(if_request.ifr_name, IFNAMSIZ);
- if_request.ifr_flags = IFF_UP;
-
- is_interface_up_ =
- NetdeviceIoctl(SIOCSIFFLAGS, reinterpret_cast<void*>(&if_request));
- return is_interface_up_;
- } else {
+ if (!setup_tun_) {
return true;
}
+ struct ifreq if_request;
+ memset(&if_request, 0, sizeof(if_request));
+ // copy does not zero-terminate the result string, but we've memset the
+ // entire struct.
+ interface_name_.copy(if_request.ifr_name, IFNAMSIZ);
+ if_request.ifr_flags = IFF_UP;
+
+ return NetdeviceIoctl(SIOCSIFFLAGS, reinterpret_cast<void*>(&if_request));
}
// TODO(pengg): might be better to use netlink socket, once we have a library to
// use
bool TunDevice::Down() {
- if (setup_tun_ && is_interface_up_) {
- struct ifreq if_request;
- memset(&if_request, 0, sizeof(if_request));
- // copy does not zero-terminate the result string, but we've memset the
- // entire struct.
- interface_name_.copy(if_request.ifr_name, IFNAMSIZ);
- if_request.ifr_flags = 0;
-
- is_interface_up_ =
- !NetdeviceIoctl(SIOCSIFFLAGS, reinterpret_cast<void*>(&if_request));
- return !is_interface_up_;
- } else {
+ if (!setup_tun_) {
return true;
}
+ struct ifreq if_request;
+ memset(&if_request, 0, sizeof(if_request));
+ // copy does not zero-terminate the result string, but we've memset the
+ // entire struct.
+ interface_name_.copy(if_request.ifr_name, IFNAMSIZ);
+ if_request.ifr_flags = 0;
+
+ return NetdeviceIoctl(SIOCSIFFLAGS, reinterpret_cast<void*>(&if_request));
}
int TunDevice::GetFileDescriptor() const {
@@ -103,6 +97,10 @@
}
bool TunDevice::OpenDevice() {
+ if (file_descriptor_ != kInvalidFd) {
+ CloseDevice();
+ }
+
struct ifreq if_request;
memset(&if_request, 0, sizeof(if_request));
// copy does not zero-terminate the result string, but we've memset the entire
@@ -117,7 +115,7 @@
if_request.ifr_flags = IFF_TUN | IFF_MULTI_QUEUE | IFF_NO_PI;
// TODO(pengg): port MakeCleanup to quic/platform? This makes the call to
- // CleanUpFileDescriptor nicer and less error-prone.
+ // CloseDevice nicer and less error-prone.
// When the device is running with IFF_MULTI_QUEUE set, each call to open will
// create a queue which can be used to read/write packets from/to the device.
const std::string tun_device_path =
@@ -125,18 +123,18 @@
int fd = kernel_.open(tun_device_path.c_str(), O_RDWR);
if (fd < 0) {
QUIC_PLOG(WARNING) << "Failed to open " << tun_device_path;
- CleanUpFileDescriptor();
+ CloseDevice();
return false;
}
file_descriptor_ = fd;
if (!CheckFeatures(fd)) {
- CleanUpFileDescriptor();
+ CloseDevice();
return false;
}
if (kernel_.ioctl(fd, TUNSETIFF, reinterpret_cast<void*>(&if_request)) != 0) {
QUIC_PLOG(WARNING) << "Failed to TUNSETIFF on fd(" << fd << ")";
- CleanUpFileDescriptor();
+ CloseDevice();
return false;
}
@@ -144,7 +142,7 @@
fd, TUNSETPERSIST,
persist_ ? reinterpret_cast<void*>(&if_request) : nullptr) != 0) {
QUIC_PLOG(WARNING) << "Failed to TUNSETPERSIST on fd(" << fd << ")";
- CleanUpFileDescriptor();
+ CloseDevice();
return false;
}
@@ -166,7 +164,7 @@
if_request.ifr_mtu = mtu_;
if (!NetdeviceIoctl(SIOCSIFMTU, reinterpret_cast<void*>(&if_request))) {
- CleanUpFileDescriptor();
+ CloseDevice();
return false;
}
@@ -206,7 +204,7 @@
return true;
}
-void TunDevice::CleanUpFileDescriptor() {
+void TunDevice::CloseDevice() {
if (file_descriptor_ != kInvalidFd) {
kernel_.close(file_descriptor_);
file_descriptor_ = kInvalidFd;
diff --git a/quic/qbone/bonnet/tun_device.h b/quic/qbone/bonnet/tun_device.h
index bff85f5..129e1d1 100644
--- a/quic/qbone/bonnet/tun_device.h
+++ b/quic/qbone/bonnet/tun_device.h
@@ -49,6 +49,11 @@
// Marks the interface down to stop receiving packets.
bool Down() override;
+ // Closes the open file descriptor for the TUN device (if one exists).
+ // It is safe to reinitialize and reuse this TunDevice after calling
+ // 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;
@@ -63,10 +68,6 @@
// Checks if the required kernel features exists.
bool CheckFeatures(int tun_device_fd);
- // Closes the opened file descriptor and makes sure the file descriptor
- // is no longer available from GetFileDescriptor;
- void CleanUpFileDescriptor();
-
// Opens a socket and makes netdevice ioctl call
bool NetdeviceIoctl(int request, void* argp);
@@ -76,7 +77,6 @@
const bool setup_tun_;
int file_descriptor_;
KernelInterface& kernel_;
- bool is_interface_up_ = false;
};
} // namespace quic
diff --git a/quic/qbone/bonnet/tun_device_interface.h b/quic/qbone/bonnet/tun_device_interface.h
index e99c547..3c6f962 100644
--- a/quic/qbone/bonnet/tun_device_interface.h
+++ b/quic/qbone/bonnet/tun_device_interface.h
@@ -12,7 +12,7 @@
// An interface with methods for interacting with a TUN device.
class TunDeviceInterface {
public:
- virtual ~TunDeviceInterface() {}
+ virtual ~TunDeviceInterface() = default;
// Actually creates/reopens and configures the device.
virtual bool Init() = 0;
@@ -23,6 +23,11 @@
// Marks the interface down to stop receiving packets.
virtual bool Down() = 0;
+ // Closes the open file descriptor for the TUN device (if one exists).
+ // It is safe to reinitialize and reuse this TunDevice after calling
+ // 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;
diff --git a/quic/qbone/bonnet/tun_device_packet_exchanger.cc b/quic/qbone/bonnet/tun_device_packet_exchanger.cc
index 2119671..4783e9c 100644
--- a/quic/qbone/bonnet/tun_device_packet_exchanger.cc
+++ b/quic/qbone/bonnet/tun_device_packet_exchanger.cc
@@ -11,14 +11,12 @@
namespace quic {
TunDevicePacketExchanger::TunDevicePacketExchanger(
- int fd,
size_t mtu,
KernelInterface* kernel,
QbonePacketExchanger::Visitor* visitor,
size_t max_pending_packets,
StatsInterface* stats)
: QbonePacketExchanger(visitor, max_pending_packets),
- fd_(fd),
mtu_(mtu),
kernel_(kernel),
stats_(stats) {}
@@ -78,8 +76,8 @@
return std::make_unique<QuicData>(read_buffer.release(), result, true);
}
-int TunDevicePacketExchanger::file_descriptor() const {
- return fd_;
+void TunDevicePacketExchanger::set_file_descriptor(int fd) {
+ fd_ = fd;
}
const TunDevicePacketExchanger::StatsInterface*
diff --git a/quic/qbone/bonnet/tun_device_packet_exchanger.h b/quic/qbone/bonnet/tun_device_packet_exchanger.h
index 1d28fee..115f5b5 100644
--- a/quic/qbone/bonnet/tun_device_packet_exchanger.h
+++ b/quic/qbone/bonnet/tun_device_packet_exchanger.h
@@ -35,8 +35,6 @@
ABSL_MUST_USE_RESULT virtual int64_t PacketsWritten() const = 0;
};
- // |fd| is a open file descriptor on a TUN device that's opened for both read
- // and write.
// |mtu| is the mtu of the TUN device.
// |kernel| is not owned but should out live objects of this class.
// |visitor| is not owned but should out live objects of this class.
@@ -44,14 +42,13 @@
// the TUN device become blocked.
// |stats| is notified about packet read/write statistics. It is not owned,
// but should outlive objects of this class.
- TunDevicePacketExchanger(int fd,
- size_t mtu,
+ TunDevicePacketExchanger(size_t mtu,
KernelInterface* kernel,
QbonePacketExchanger::Visitor* visitor,
size_t max_pending_packets,
StatsInterface* stats);
- ABSL_MUST_USE_RESULT int file_descriptor() const;
+ void set_file_descriptor(int fd);
ABSL_MUST_USE_RESULT const StatsInterface* stats_interface() const;
diff --git a/quic/qbone/bonnet/tun_device_packet_exchanger_test.cc b/quic/qbone/bonnet/tun_device_packet_exchanger_test.cc
index 4a00c60..c8f3ff0 100644
--- a/quic/qbone/bonnet/tun_device_packet_exchanger_test.cc
+++ b/quic/qbone/bonnet/tun_device_packet_exchanger_test.cc
@@ -30,14 +30,15 @@
class TunDevicePacketExchangerTest : public QuicTest {
protected:
TunDevicePacketExchangerTest()
- : exchanger_(kFd,
- kMtu,
+ : exchanger_(kMtu,
&mock_kernel_,
&mock_visitor_,
kMaxPendingPackets,
- &mock_stats_) {}
+ &mock_stats_) {
+ exchanger_.set_file_descriptor(kFd);
+ }
- ~TunDevicePacketExchangerTest() override {}
+ ~TunDevicePacketExchangerTest() override = default;
MockKernel mock_kernel_;
StrictMock<MockVisitor> mock_visitor_;
diff --git a/quic/qbone/bonnet/tun_device_test.cc b/quic/qbone/bonnet/tun_device_test.cc
index 9c216af..6a02f07 100644
--- a/quic/qbone/bonnet/tun_device_test.cc
+++ b/quic/qbone/bonnet/tun_device_test.cc
@@ -139,6 +139,7 @@
TunDevice tun_device(kDeviceName, 1500, false, true, &mock_kernel_);
EXPECT_FALSE(tun_device.Init());
EXPECT_EQ(tun_device.GetFileDescriptor(), -1);
+ ExpectDown(false);
}
TEST_F(TunDeviceTest, FailToCheckFeature) {
@@ -147,6 +148,7 @@
TunDevice tun_device(kDeviceName, 1500, false, true, &mock_kernel_);
EXPECT_FALSE(tun_device.Init());
EXPECT_EQ(tun_device.GetFileDescriptor(), -1);
+ ExpectDown(false);
}
TEST_F(TunDeviceTest, TooFewFeature) {
@@ -160,6 +162,7 @@
TunDevice tun_device(kDeviceName, 1500, false, true, &mock_kernel_);
EXPECT_FALSE(tun_device.Init());
EXPECT_EQ(tun_device.GetFileDescriptor(), -1);
+ ExpectDown(false);
}
TEST_F(TunDeviceTest, FailToSetFlag) {
