shill: Connection: If IP Address changes, flush everything
If a new IP configuration arrives which changes the address
assigned to the family of this IPConfig, flush addresses
and routes before applying the new configuration. Otherwise,
we end up adding the new address inclusively as a secondary
IP address and userspace programs continue to use the old
address.
BUG=chromium-os:33066
TEST=New unit tests. Manual: Change DHCP server configuration,
unplug and replug to the same network. The old DHCP configuration
is loaded (since the lease is still valid and the gateway is
reachable) but when the new DHCP information arrives, ensure that
the new IP address (and only that address) is configured, and the
routes are sane (i.e., both LAN interface route and default route
exist).
Change-Id: Ic746368d97c503271995ff30b6818d770f4340c5
Reviewed-on: https://gerrit.chromium.org/gerrit/29170
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Commit-Ready: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/connection.cc b/connection.cc
index f3a2e6d..d258dd7 100644
--- a/connection.cc
+++ b/connection.cc
@@ -146,6 +146,14 @@
LOG(WARNING) << "Expect limited network connectivity.";
}
+ if (device_info_->HasOtherAddress(interface_index_, local)) {
+ // The address has changed for this interface. We need to flush
+ // everything and start over.
+ LOG(INFO) << __func__ << ": Flushing old addresses and routes.";
+ routing_table_->FlushRoutes(interface_index_);
+ device_info_->FlushAddresses(interface_index_);
+ }
+
LOG(INFO) << __func__ << ": Installing with parameters:"
<< " local=" << local.ToString()
<< " broadcast=" << broadcast.ToString()
diff --git a/connection.h b/connection.h
index d0fa04e..813893c 100644
--- a/connection.h
+++ b/connection.h
@@ -104,9 +104,6 @@
private:
friend class ConnectionTest;
FRIEND_TEST(ConnectionTest, AddConfig);
- FRIEND_TEST(ConnectionTest, AddConfigReverse);
- FRIEND_TEST(ConnectionTest, AddConfigWithBrokenNetmask);
- FRIEND_TEST(ConnectionTest, AddConfigWithPeer);
FRIEND_TEST(ConnectionTest, Binder);
FRIEND_TEST(ConnectionTest, Binders);
FRIEND_TEST(ConnectionTest, Destructor);
diff --git a/connection_unittest.cc b/connection_unittest.cc
index 2fd5bd0..e99f55a 100644
--- a/connection_unittest.cc
+++ b/connection_unittest.cc
@@ -120,6 +120,14 @@
return connection->has_broadcast_domain_;
}
+ uint32 GetDefaultMetric() {
+ return Connection::kDefaultMetric;
+ }
+
+ uint32 GetNonDefaultMetricBase() {
+ return Connection::kNonDefaultMetricBase;
+ }
+
protected:
class DisconnectCallbackTarget {
public:
@@ -188,6 +196,10 @@
}
TEST_F(ConnectionTest, AddConfig) {
+ EXPECT_CALL(*device_info_,
+ HasOtherAddress(kTestDeviceInterfaceIndex0,
+ IsIPAddress(local_address_, kPrefix0)))
+ .WillOnce(Return(false));
EXPECT_CALL(rtnl_handler_,
AddInterfaceAddress(kTestDeviceInterfaceIndex0,
IsIPAddress(local_address_, kPrefix0),
@@ -196,12 +208,12 @@
EXPECT_CALL(routing_table_,
SetDefaultRoute(kTestDeviceInterfaceIndex0,
IsIPAddress(gateway_address_, 0),
- Connection::kNonDefaultMetricBase +
+ GetNonDefaultMetricBase() +
kTestDeviceInterfaceIndex0));
EXPECT_CALL(routing_table_,
ConfigureRoutes(kTestDeviceInterfaceIndex0,
ipconfig_,
- Connection::kDefaultMetric));
+ GetDefaultMetric()));
connection_->UpdateFromIPConfig(ipconfig_);
IPAddress test_local_address(local_address_);
test_local_address.set_prefix(kPrefix0);
@@ -221,7 +233,7 @@
EXPECT_FALSE(connection_->CreateGatewayRoute());
EXPECT_CALL(routing_table_, SetDefaultMetric(kTestDeviceInterfaceIndex0,
- Connection::kDefaultMetric));
+ GetDefaultMetric()));
EXPECT_CALL(resolver_, SetDNSFromLists(
ipconfig_->properties().dns_servers,
ipconfig_->properties().domain_search));
@@ -246,7 +258,7 @@
EXPECT_CALL(routing_table_,
SetDefaultMetric(kTestDeviceInterfaceIndex0,
- Connection::kNonDefaultMetricBase +
+ GetNonDefaultMetricBase() +
kTestDeviceInterfaceIndex0));
EXPECT_CALL(routing_table_, FlushCache())
.WillOnce(Return(true));
@@ -261,6 +273,10 @@
properties_.peer_address = kPeerAddress;
properties_.gateway = string();
UpdateProperties();
+ EXPECT_CALL(*device_info_,
+ HasOtherAddress(kTestDeviceInterfaceIndex0,
+ IsIPAddress(local_address_, kPrefix0)))
+ .WillOnce(Return(false));
EXPECT_CALL(rtnl_handler_,
AddInterfaceAddress(kTestDeviceInterfaceIndex0,
IsIPAddress(local_address_, kPrefix0),
@@ -270,7 +286,7 @@
EXPECT_CALL(routing_table_,
ConfigureRoutes(kTestDeviceInterfaceIndex0,
ipconfig_,
- Connection::kDefaultMetric));
+ GetDefaultMetric()));
connection_->UpdateFromIPConfig(ipconfig_);
EXPECT_FALSE(GetHasBroadcastDomain(connection_));
}
@@ -282,6 +298,10 @@
// Connection should override with a prefix which will allow the
// gateway to be reachable.
+ EXPECT_CALL(*device_info_,
+ HasOtherAddress(kTestDeviceInterfaceIndex0,
+ IsIPAddress(local_address_, kPrefix0)))
+ .WillOnce(Return(false));
EXPECT_CALL(rtnl_handler_,
AddInterfaceAddress(kTestDeviceInterfaceIndex0,
IsIPAddress(local_address_, kPrefix0),
@@ -290,12 +310,12 @@
EXPECT_CALL(routing_table_,
SetDefaultRoute(kTestDeviceInterfaceIndex0,
IsIPAddress(gateway_address_, 0),
- Connection::kNonDefaultMetricBase +
+ GetNonDefaultMetricBase() +
kTestDeviceInterfaceIndex0));
EXPECT_CALL(routing_table_,
ConfigureRoutes(kTestDeviceInterfaceIndex0,
ipconfig_,
- Connection::kDefaultMetric));
+ GetDefaultMetric()));
connection_->UpdateFromIPConfig(ipconfig_);
// Assign a gateway address that violates the minimum plausible prefix
@@ -308,6 +328,10 @@
// Connection cannot override this prefix, so it will switch to a
// model where the peer address is set to the value of the gateway
// address.
+ EXPECT_CALL(*device_info_,
+ HasOtherAddress(kTestDeviceInterfaceIndex0,
+ IsIPAddress(local_address_, kPrefix1)))
+ .WillOnce(Return(false));
EXPECT_CALL(rtnl_handler_,
AddInterfaceAddress(kTestDeviceInterfaceIndex0,
IsIPAddress(local_address_, kPrefix1),
@@ -322,7 +346,7 @@
TEST_F(ConnectionTest, AddConfigReverse) {
EXPECT_CALL(routing_table_, SetDefaultMetric(kTestDeviceInterfaceIndex0,
- Connection::kDefaultMetric));
+ GetDefaultMetric()));
vector<string> empty_list;
EXPECT_CALL(resolver_, SetDNSFromLists(empty_list, empty_list));
scoped_refptr<MockDevice> device(new StrictMock<MockDevice>(
@@ -342,6 +366,10 @@
connection_->SetIsDefault(true);
Mock::VerifyAndClearExpectations(&routing_table_);
+ EXPECT_CALL(*device_info_,
+ HasOtherAddress(kTestDeviceInterfaceIndex0,
+ IsIPAddress(local_address_, kPrefix0)))
+ .WillOnce(Return(false));
EXPECT_CALL(rtnl_handler_,
AddInterfaceAddress(kTestDeviceInterfaceIndex0,
IsIPAddress(local_address_, kPrefix0),
@@ -349,16 +377,40 @@
IsIPAddress(default_address_, 0)));
EXPECT_CALL(routing_table_, SetDefaultRoute(kTestDeviceInterfaceIndex0,
IsIPAddress(gateway_address_, 0),
- Connection::kDefaultMetric));
+ GetDefaultMetric()));
EXPECT_CALL(routing_table_,
ConfigureRoutes(kTestDeviceInterfaceIndex0,
ipconfig_,
- Connection::kDefaultMetric));
+ GetDefaultMetric()));
EXPECT_CALL(resolver_, SetDNSFromIPConfig(ipconfig_));
connection_->UpdateFromIPConfig(ipconfig_);
}
+TEST_F(ConnectionTest, HasOtherAddress) {
+ EXPECT_CALL(*device_info_,
+ HasOtherAddress(kTestDeviceInterfaceIndex0,
+ IsIPAddress(local_address_, kPrefix0)))
+ .WillOnce(Return(true));
+ EXPECT_CALL(routing_table_, FlushRoutes(kTestDeviceInterfaceIndex0));
+ EXPECT_CALL(*device_info_, FlushAddresses(kTestDeviceInterfaceIndex0));
+ EXPECT_CALL(rtnl_handler_,
+ AddInterfaceAddress(kTestDeviceInterfaceIndex0,
+ IsIPAddress(local_address_, kPrefix0),
+ IsIPAddress(broadcast_address_, 0),
+ IsIPAddress(default_address_, 0)));
+ EXPECT_CALL(routing_table_,
+ SetDefaultRoute(kTestDeviceInterfaceIndex0,
+ IsIPAddress(gateway_address_, 0),
+ GetNonDefaultMetricBase() +
+ kTestDeviceInterfaceIndex0));
+ EXPECT_CALL(routing_table_,
+ ConfigureRoutes(kTestDeviceInterfaceIndex0,
+ ipconfig_,
+ GetDefaultMetric()));
+ connection_->UpdateFromIPConfig(ipconfig_);
+}
+
TEST_F(ConnectionTest, RouteRequest) {
ConnectionRefPtr connection = GetNewConnection();
scoped_refptr<MockDevice> device(new StrictMock<MockDevice>(
diff --git a/device_info.cc b/device_info.cc
index e45e1a0..3838a15 100644
--- a/device_info.cc
+++ b/device_info.cc
@@ -606,6 +606,33 @@
}
}
+bool DeviceInfo::HasOtherAddress(
+ int interface_index, const IPAddress &this_address) const {
+ SLOG(Device, 3) << __func__ << "(" << interface_index << ")";
+ const Info *info = GetInfo(interface_index);
+ if (!info) {
+ return false;
+ }
+ const vector<AddressData> &addresses = info->ip_addresses;
+ vector<AddressData>::const_iterator iter;
+ bool has_other_address = false;
+ bool has_this_address = false;
+ for (iter = addresses.begin(); iter != addresses.end(); ++iter) {
+ if (iter->address.family() != this_address.family()) {
+ continue;
+ }
+ if (iter->address.address().Equals(this_address.address())) {
+ has_this_address = true;
+ } else if (this_address.family() == IPAddress::kFamilyIPv4) {
+ has_other_address = true;
+ } else if ((iter->scope == RT_SCOPE_UNIVERSE &&
+ (iter->flags & IFA_F_TEMPORARY) == 0)) {
+ has_other_address = true;
+ }
+ }
+ return has_other_address && !has_this_address;
+}
+
bool DeviceInfo::GetFlags(int interface_index, unsigned int *flags) const {
const Info *info = GetInfo(interface_index);
if (!info) {
diff --git a/device_info.h b/device_info.h
index dd827eb..22f990d 100644
--- a/device_info.h
+++ b/device_info.h
@@ -81,7 +81,12 @@
uint64 *rx_bytes, uint64 *tx_bytes) const;
virtual bool GetAddresses(int interface_index,
std::vector<AddressData> *addresses) const;
+ // Flush all addresses associated with |interface_index|.
virtual void FlushAddresses(int interface_index) const;
+ // Returns whether this interface does not have |this_address|
+ // but has another non-temporary address of the same family.
+ virtual bool HasOtherAddress(
+ int interface_index, const IPAddress &this_address) const;
virtual bool CreateTunnelInterface(std::string *interface_name) const;
virtual bool DeleteInterface(int interface_index) const;
diff --git a/device_info_unittest.cc b/device_info_unittest.cc
index c50ce5b..78695a4 100644
--- a/device_info_unittest.cc
+++ b/device_info_unittest.cc
@@ -117,6 +117,7 @@
static const char kTestIPAddress2[];
static const char kTestIPAddress3[];
static const char kTestIPAddress4[];
+ static const char kTestIPAddress5[];
static const int kReceiveByteCount;
static const int kTransmitByteCount;
@@ -150,6 +151,7 @@
const char DeviceInfoTest::kTestIPAddress2[] = "fe80::1aa9:5ff:abcd:1235";
const char DeviceInfoTest::kTestIPAddress3[] = "fe80::1aa9:5ff:abcd:1236";
const char DeviceInfoTest::kTestIPAddress4[] = "fe80::1aa9:5ff:abcd:1237";
+const char DeviceInfoTest::kTestIPAddress5[] = "192.168.1.2";
const int DeviceInfoTest::kReceiveByteCount = 1234;
const int DeviceInfoTest::kTransmitByteCount = 5678;
@@ -594,6 +596,93 @@
device_info_.FlushAddresses(kTestDeviceIndex);
}
+TEST_F(DeviceInfoTest, HasOtherAddress) {
+ scoped_ptr<RTNLMessage> message(BuildLinkMessage(RTNLMessage::kModeAdd));
+ SendMessageToDeviceInfo(*message);
+
+ IPAddress address0(IPAddress::kFamilyIPv4);
+ EXPECT_TRUE(address0.SetAddressFromString(kTestIPAddress0));
+
+ // There are no addresses on this interface.
+ EXPECT_FALSE(device_info_.HasOtherAddress(kTestDeviceIndex, address0));
+
+ message.reset(BuildAddressMessage(RTNLMessage::kModeAdd,
+ address0,
+ 0,
+ RT_SCOPE_UNIVERSE));
+ SendMessageToDeviceInfo(*message);
+
+ IPAddress address1(IPAddress::kFamilyIPv6);
+ EXPECT_TRUE(address1.SetAddressFromString(kTestIPAddress1));
+ address1.set_prefix(kTestIPAddressPrefix1);
+ message.reset(BuildAddressMessage(RTNLMessage::kModeAdd,
+ address1,
+ 0,
+ RT_SCOPE_LINK));
+ SendMessageToDeviceInfo(*message);
+
+ IPAddress address2(IPAddress::kFamilyIPv6);
+ EXPECT_TRUE(address2.SetAddressFromString(kTestIPAddress2));
+ message.reset(BuildAddressMessage(RTNLMessage::kModeAdd,
+ address2,
+ IFA_F_TEMPORARY,
+ RT_SCOPE_UNIVERSE));
+ SendMessageToDeviceInfo(*message);
+
+ IPAddress address3(IPAddress::kFamilyIPv6);
+ EXPECT_TRUE(address3.SetAddressFromString(kTestIPAddress3));
+
+ // The only IPv6 addresses on this interface are either flagged as
+ // temporary, or they are not universally scoped.
+ EXPECT_FALSE(device_info_.HasOtherAddress(kTestDeviceIndex, address3));
+
+
+ message.reset(BuildAddressMessage(RTNLMessage::kModeAdd,
+ address3,
+ 0,
+ RT_SCOPE_UNIVERSE));
+ SendMessageToDeviceInfo(*message);
+
+ // address0 is on this interface.
+ EXPECT_FALSE(device_info_.HasOtherAddress(kTestDeviceIndex, address0));
+ // address1 is on this interface.
+ EXPECT_FALSE(device_info_.HasOtherAddress(kTestDeviceIndex, address1));
+ // address2 is on this interface.
+ EXPECT_FALSE(device_info_.HasOtherAddress(kTestDeviceIndex, address2));
+ // address3 is on this interface.
+ EXPECT_FALSE(device_info_.HasOtherAddress(kTestDeviceIndex, address3));
+
+ IPAddress address4(IPAddress::kFamilyIPv6);
+ EXPECT_TRUE(address4.SetAddressFromString(kTestIPAddress4));
+
+ // address4 is not on this interface, but address3 is, and is a qualified
+ // IPv6 address.
+ EXPECT_TRUE(device_info_.HasOtherAddress(kTestDeviceIndex, address4));
+
+ message.reset(BuildAddressMessage(RTNLMessage::kModeAdd,
+ address4,
+ IFA_F_PERMANENT,
+ RT_SCOPE_UNIVERSE));
+ SendMessageToDeviceInfo(*message);
+
+ // address4 is now on this interface.
+ EXPECT_FALSE(device_info_.HasOtherAddress(kTestDeviceIndex, address4));
+
+ IPAddress address5(IPAddress::kFamilyIPv4);
+ EXPECT_TRUE(address5.SetAddressFromString(kTestIPAddress5));
+ // address5 is not on this interface, but address0 is.
+ EXPECT_TRUE(device_info_.HasOtherAddress(kTestDeviceIndex, address5));
+
+ message.reset(BuildAddressMessage(RTNLMessage::kModeAdd,
+ address5,
+ IFA_F_PERMANENT,
+ RT_SCOPE_UNIVERSE));
+ SendMessageToDeviceInfo(*message);
+
+ // address5 is now on this interface.
+ EXPECT_FALSE(device_info_.HasOtherAddress(kTestDeviceIndex, address5));
+}
+
TEST_F(DeviceInfoTest, HasSubdir) {
ScopedTempDir temp_dir;
EXPECT_TRUE(temp_dir.CreateUniqueTempDir());
diff --git a/mock_device_info.h b/mock_device_info.h
index b696cce..71ae32d 100644
--- a/mock_device_info.h
+++ b/mock_device_info.h
@@ -17,6 +17,7 @@
class ByteString;
class ControlInterface;
class EventDispatcher;
+class IPAddress;
class Manager;
class Metrics;
@@ -41,6 +42,9 @@
MOCK_CONST_METHOD2(GetAddresses, bool(int interface_index,
std::vector<AddressData>* addresses));
MOCK_CONST_METHOD1(FlushAddresses, void(int interface_index));
+ MOCK_CONST_METHOD2(HasOtherAddress,
+ bool(int interface_index,
+ const IPAddress &excluded_address));
MOCK_CONST_METHOD1(CreateTunnelInterface, bool(std::string *interface_name));
MOCK_CONST_METHOD1(DeleteInterface, bool(int interface_index));
MOCK_METHOD1(RegisterDevice, void(const DeviceRefPtr &));