shill: when updating the metric for a route, make sure to
update RoutingTable's internal state (in addition to updating
kernel state).
It seems the kernel doesn't echo our routing changes back to
us. So we need to explicitly update this ourselves.
BUG=chromium-os:26529
TEST=updated unit test, manual
manual testing:
- set up device with WiFi config, and plug in Ethernet
- "restart flimflam" five times, observe that every time,
metric for route on Ethernet interface is 1.
Change-Id: Ib82f8be58b81e316e893e7952345f86f5489cfca
Reviewed-on: https://gerrit.chromium.org/gerrit/16405
Commit-Ready: mukesh agrawal <quiche@chromium.org>
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Tested-by: mukesh agrawal <quiche@chromium.org>
diff --git a/routing_table.cc b/routing_table.cc
index 1c7ce76..9a68052 100644
--- a/routing_table.cc
+++ b/routing_table.cc
@@ -94,6 +94,17 @@
bool RoutingTable::GetDefaultRoute(int interface_index,
IPAddress::Family family,
RoutingTableEntry *entry) {
+ RoutingTableEntry *found_entry;
+ bool ret = GetDefaultRouteInternal(interface_index, family, &found_entry);
+ if (ret) {
+ *entry = *found_entry;
+ }
+ return ret;
+}
+
+bool RoutingTable::GetDefaultRouteInternal(int interface_index,
+ IPAddress::Family family,
+ RoutingTableEntry **entry) {
VLOG(2) << __func__ << " index " << interface_index << " family " << family;
base::hash_map<int, vector<RoutingTableEntry> >::iterator table =
@@ -108,7 +119,7 @@
for (nent = table->second.begin(); nent != table->second.end(); ++nent) {
if (nent->dst.IsDefault() && nent->dst.family() == family) {
- *entry = *nent;
+ *entry = &(*nent);
VLOG(2) << __func__ << " found "
<< "gateway " << nent->gateway.ToString() << " "
<< "metric " << nent->metric;
@@ -126,23 +137,24 @@
VLOG(2) << __func__ << " index " << interface_index << " metric " << metric;
const IPConfig::Properties &ipconfig_props = ipconfig->properties();
- RoutingTableEntry old_entry;
+ RoutingTableEntry *old_entry;
IPAddress gateway_address(ipconfig_props.address_family);
if (!gateway_address.SetAddressFromString(ipconfig_props.gateway)) {
return false;
}
- if (GetDefaultRoute(interface_index,
- ipconfig_props.address_family,
- &old_entry)) {
- if (old_entry.gateway.Equals(gateway_address)) {
- if (old_entry.metric != metric) {
+ if (GetDefaultRouteInternal(interface_index,
+ ipconfig_props.address_family,
+ &old_entry)) {
+ if (old_entry->gateway.Equals(gateway_address)) {
+ if (old_entry->metric != metric) {
ReplaceMetric(interface_index, old_entry, metric);
}
return true;
} else {
+ // TODO(quiche): Update internal state as well?
ApplyRoute(interface_index,
- old_entry,
+ *old_entry,
RTNLMessage::kModeDelete,
0);
}
@@ -185,15 +197,16 @@
VLOG(2) << __func__ << " "
<< "index " << interface_index << " metric " << metric;
- RoutingTableEntry entry;
-
- if (GetDefaultRoute(interface_index, IPAddress::kFamilyIPv4, &entry) &&
- entry.metric != metric) {
+ RoutingTableEntry *entry;
+ if (GetDefaultRouteInternal(
+ interface_index, IPAddress::kFamilyIPv4, &entry) &&
+ entry->metric != metric) {
ReplaceMetric(interface_index, entry, metric);
}
- if (GetDefaultRoute(interface_index, IPAddress::kFamilyIPv6, &entry) &&
- entry.metric != metric) {
+ if (GetDefaultRouteInternal(
+ interface_index, IPAddress::kFamilyIPv6, &entry) &&
+ entry->metric != metric) {
ReplaceMetric(interface_index, entry, metric);
}
}
@@ -319,17 +332,19 @@
// We do so after creating the route at a new metric so there is no traffic
// disruption to existing network streams.
void RoutingTable::ReplaceMetric(uint32 interface_index,
- const RoutingTableEntry &entry,
+ RoutingTableEntry *entry,
uint32 metric) {
VLOG(2) << __func__ << " "
<< "index " << interface_index << " metric " << metric;
- RoutingTableEntry new_entry = entry;
+ RoutingTableEntry new_entry = *entry;
new_entry.metric = metric;
// First create the route at the new metric.
ApplyRoute(interface_index, new_entry, RTNLMessage::kModeAdd,
NLM_F_CREATE | NLM_F_REPLACE);
// Then delete the route at the old metric.
- ApplyRoute(interface_index, entry, RTNLMessage::kModeDelete, 0);
+ ApplyRoute(interface_index, *entry, RTNLMessage::kModeDelete, 0);
+ // Now, update our routing table (via |*entry|) from |new_entry|.
+ *entry = new_entry;
}
bool RoutingTable::FlushCache() {
diff --git a/routing_table.h b/routing_table.h
index cf31f1c..90eccbe 100644
--- a/routing_table.h
+++ b/routing_table.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -40,6 +40,7 @@
virtual bool AddRoute(int interface_index, const RoutingTableEntry &entry);
// Get the default route associated with an interface of a given addr family.
+ // The route is copied into |*entry|.
virtual bool GetDefaultRoute(int interface_index,
IPAddress::Family family,
RoutingTableEntry *entry);
@@ -73,8 +74,14 @@
const RoutingTableEntry &entry,
RTNLMessage::Mode mode,
unsigned int flags);
+ // Get the default route associated with an interface of a given addr family.
+ // A pointer to the route is placed in |*entry|.
+ virtual bool GetDefaultRouteInternal(int interface_index,
+ IPAddress::Family family,
+ RoutingTableEntry **entry);
+
void ReplaceMetric(uint32 interface_index,
- const RoutingTableEntry &entry,
+ RoutingTableEntry *entry,
uint32 metric);
static const char kRouteFlushPath4[];
diff --git a/routing_table_unittest.cc b/routing_table_unittest.cc
index 1ca7a09..e5b2960 100644
--- a/routing_table_unittest.cc
+++ b/routing_table_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -369,13 +369,20 @@
_,
0));
routing_table_->SetDefaultMetric(kTestDeviceIndex0, entry5.metric);
+ // Furthermore, the routing table should reflect the change in the metric
+ // for the default route for the interface.
+ RoutingTableEntry default_route;
+ EXPECT_TRUE(routing_table_->GetDefaultRoute(kTestDeviceIndex0,
+ IPAddress::kFamilyIPv4,
+ &default_route));
+ EXPECT_EQ(entry5.metric, default_route.metric);
// Ask to flush table0. We should see a delete message sent.
EXPECT_CALL(sockets_,
Send(kTestSocket,
IsRoutingPacket(RTNLMessage::kModeDelete,
kTestDeviceIndex0,
- entry0,
+ entry5,
0),
_,
0));