shill: Ignore certan DNS search paths
Add a manager property to provide a list of DNS Search paths
that should be ignored when applying DHCP parameters. This
covers a list of DNS search suffixes that are popularly
provided, but are generally unlikely to provide any useful
results, therefore significantly reducing DNS lookup performance.
BUG=chromium-os:34495
TEST=Unit-tests + Manual: Add a domain to the default search list and make sure it is filtered from the "search" line in resolv.conf.
CQ-DEPENDS: I54bdd33a05bb704d8c3ff05f71e034fe42635e89
Change-Id: Id92b39f1ad0ae64b3ff50c7671cdf388d92a07af
Reviewed-on: https://gerrit.chromium.org/gerrit/33460
Reviewed-by: Christopher Wiley <wiley@chromium.org>
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Commit-Ready: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/default_profile.cc b/default_profile.cc
index 0f89638..51246e6 100644
--- a/default_profile.cc
+++ b/default_profile.cc
@@ -34,6 +34,9 @@
// static
const char DefaultProfile::kStorageHostName[] = "HostName";
// static
+const char DefaultProfile::kStorageIgnoredDNSSearchPaths[] =
+ "IgnoredDNSSearchPaths";
+// static
const char DefaultProfile::kStorageLinkMonitorTechnologies[] =
"LinkMonitorTechnologies";
// static
@@ -65,6 +68,8 @@
&manager_props.check_portal_list);
store->RegisterConstString(flimflam::kCountryProperty,
&manager_props.country);
+ store->RegisterConstString(shill::kIgnoredDNSSearchPathsProperty,
+ &manager_props.ignored_dns_search_paths);
store->RegisterConstString(shill::kLinkMonitorTechnologiesProperty,
&manager_props.link_monitor_technologies);
store->RegisterConstBool(flimflam::kOfflineModeProperty,
@@ -91,6 +96,12 @@
manager_props->check_portal_list = PortalDetector::kDefaultCheckPortalList;
}
if (!storage()->GetString(kStorageId,
+ kStorageIgnoredDNSSearchPaths,
+ &manager_props->ignored_dns_search_paths)) {
+ manager_props->ignored_dns_search_paths =
+ Resolver::kDefaultIgnoredSearchList;
+ }
+ if (!storage()->GetString(kStorageId,
kStorageLinkMonitorTechnologies,
&manager_props->link_monitor_technologies)) {
manager_props->link_monitor_technologies =
@@ -141,6 +152,9 @@
kStorageCheckPortalList,
props_.check_portal_list);
storage()->SetString(kStorageId,
+ kStorageIgnoredDNSSearchPaths,
+ props_.ignored_dns_search_paths);
+ storage()->SetString(kStorageId,
kStorageLinkMonitorTechnologies,
props_.link_monitor_technologies);
storage()->SetString(kStorageId,
diff --git a/default_profile.h b/default_profile.h
index 4ed7fc2..50c82bb 100644
--- a/default_profile.h
+++ b/default_profile.h
@@ -33,7 +33,8 @@
const Manager::Properties &manager_props);
virtual ~DefaultProfile();
- // Loads global configuration into manager properties.
+ // Loads global configuration into manager properties. This should
+ // only be called by the Manager.
virtual bool LoadManagerProperties(Manager::Properties *manager_props);
// Override the Profile superclass implementation to accept all Ethernet
@@ -67,6 +68,7 @@
static const char kStorageArpGateway[];
static const char kStorageCheckPortalList[];
static const char kStorageHostName[];
+ static const char kStorageIgnoredDNSSearchPaths[];
static const char kStorageLinkMonitorTechnologies[];
static const char kStorageName[];
static const char kStorageOfflineMode[];
diff --git a/default_profile_unittest.cc b/default_profile_unittest.cc
index ed47db3..bfa6cf4 100644
--- a/default_profile_unittest.cc
+++ b/default_profile_unittest.cc
@@ -126,6 +126,11 @@
.WillOnce(Return(true));
EXPECT_CALL(*storage.get(),
SetString(DefaultProfile::kStorageId,
+ DefaultProfile::kStorageIgnoredDNSSearchPaths,
+ ""))
+ .WillOnce(Return(true));
+ EXPECT_CALL(*storage.get(),
+ SetString(DefaultProfile::kStorageId,
DefaultProfile::kStorageLinkMonitorTechnologies,
""))
.WillOnce(Return(true));
@@ -174,6 +179,11 @@
.WillOnce(Return(false));
EXPECT_CALL(*storage.get(),
GetString(DefaultProfile::kStorageId,
+ DefaultProfile::kStorageIgnoredDNSSearchPaths,
+ &manager_props.ignored_dns_search_paths))
+ .WillOnce(Return(false));
+ EXPECT_CALL(*storage.get(),
+ GetString(DefaultProfile::kStorageId,
DefaultProfile::kStorageLinkMonitorTechnologies,
_))
.WillOnce(Return(false));
@@ -200,6 +210,8 @@
EXPECT_FALSE(manager_props.offline_mode);
EXPECT_EQ(PortalDetector::kDefaultCheckPortalList,
manager_props.check_portal_list);
+ EXPECT_EQ(Resolver::kDefaultIgnoredSearchList,
+ manager_props.ignored_dns_search_paths);
EXPECT_EQ(LinkMonitor::kDefaultLinkMonitorTechnologies,
manager_props.link_monitor_technologies);
EXPECT_EQ(PortalDetector::kDefaultURL, manager_props.portal_url);
@@ -229,6 +241,12 @@
DefaultProfile::kStorageCheckPortalList,
_))
.WillOnce(DoAll(SetArgumentPointee<2>(portal_list), Return(true)));
+ const string ignored_paths("chromium.org,google.com");
+ EXPECT_CALL(*storage.get(),
+ GetString(DefaultProfile::kStorageId,
+ DefaultProfile::kStorageIgnoredDNSSearchPaths,
+ _))
+ .WillOnce(DoAll(SetArgumentPointee<2>(ignored_paths), Return(true)));
const string link_monitor_technologies("ethernet,wimax");
EXPECT_CALL(*storage.get(),
GetString(DefaultProfile::kStorageId,
@@ -264,6 +282,7 @@
EXPECT_EQ(host_name, manager_props.host_name);
EXPECT_TRUE(manager_props.offline_mode);
EXPECT_EQ(portal_list, manager_props.check_portal_list);
+ EXPECT_EQ(ignored_paths, manager_props.ignored_dns_search_paths);
EXPECT_EQ(link_monitor_technologies,
manager_props.link_monitor_technologies);
EXPECT_EQ(portal_url, manager_props.portal_url);
diff --git a/doc/manager-api.txt b/doc/manager-api.txt
index 7691c62..4351ee1 100644
--- a/doc/manager-api.txt
+++ b/doc/manager-api.txt
@@ -329,6 +329,15 @@
The default for this name is empty, which
means that the system will not report a hostname.
+ string IgnoredDNSSearchPaths [readwrite]
+
+ A comma-separated list of DNS domain suffixes
+ which should be ignored when creating a DNS
+ search list from DHCP-provided parameters.
+ This list will be consulted every time DHCP
+ information is updated (while connecting to
+ a network, or when a DHCP lease is renewed).
+
string LinkMonitorTechnologies [readwrite]
The list of technologies for which thie Link
diff --git a/manager.cc b/manager.cc
index cac91aa..cebf74c 100644
--- a/manager.cc
+++ b/manager.cc
@@ -16,6 +16,7 @@
#include <base/file_util.h>
#include <base/memory/ref_counted.h>
#include <base/stringprintf.h>
+#include <base/string_split.h>
#include <base/string_util.h>
#include <chromeos/dbus/service_constants.h>
@@ -77,6 +78,7 @@
modem_info_(control_interface, dispatcher, metrics, this, glib),
vpn_provider_(control_interface, dispatcher, metrics, this),
wimax_provider_(control_interface, dispatcher, metrics, this),
+ resolver_(Resolver::GetInstance()),
running_(false),
connect_profiles_to_rpc_(true),
ephemeral_profile_(new EphemeralProfile(control_interface, this)),
@@ -111,6 +113,9 @@
HelpRegisterDerivedStrings(flimflam::kEnabledTechnologiesProperty,
&Manager::EnabledTechnologies,
NULL);
+ HelpRegisterDerivedString(shill::kIgnoredDNSSearchPathsProperty,
+ &Manager::GetIgnoredDNSSearchPaths,
+ &Manager::SetIgnoredDNSSearchPaths);
store_.RegisterString(shill::kLinkMonitorTechnologiesProperty,
&props_.link_monitor_technologies);
store_.RegisterBool(flimflam::kOfflineModeProperty, &props_.offline_mode);
@@ -168,7 +173,7 @@
power_manager_->AddStateChangeCallback(Metrics::kMetricPowerManagerKey, cb);
CHECK(file_util::CreateDirectory(run_path_)) << run_path_.value();
- Resolver::GetInstance()->set_path(run_path_.Append("resolv.conf"));
+ resolver_->set_path(run_path_.Append("resolv.conf"));
InitializeProfiles();
running_ = true;
@@ -227,7 +232,7 @@
props_));
CHECK(default_profile->InitStorage(glib_, Profile::kCreateOrOpenExisting,
NULL));
- CHECK(default_profile->LoadManagerProperties(&props_));
+ CHECK(LoadProperties(default_profile));
profiles_.push_back(default_profile.release());
Error error;
string path;
@@ -328,7 +333,7 @@
return;
}
- if (!default_profile->LoadManagerProperties(&props_)) {
+ if (!LoadProperties(default_profile)) {
Error::PopulateAndLog(error, Error::kInvalidArguments,
"Could not load Manager properties from profile " +
name);
@@ -850,6 +855,14 @@
}
}
+bool Manager::LoadProperties(const scoped_refptr<DefaultProfile> &profile) {
+ if (!profile->LoadManagerProperties(&props_)) {
+ return false;
+ }
+ SetIgnoredDNSSearchPaths(props_.ignored_dns_search_paths, NULL);
+ return true;
+}
+
void Manager::AddTerminationAction(const std::string &name,
const base::Closure &start) {
termination_actions_.Add(name, start);
@@ -1180,6 +1193,20 @@
use_startup_portal_list_ = false;
}
+string Manager::GetIgnoredDNSSearchPaths(Error */*error*/) {
+ return props_.ignored_dns_search_paths;
+}
+
+void Manager::SetIgnoredDNSSearchPaths(const string &ignored_paths,
+ Error */*error*/) {
+ props_.ignored_dns_search_paths = ignored_paths;
+ vector<string> ignored_path_list;
+ if (!ignored_paths.empty()) {
+ base::SplitString(ignored_paths, ',', &ignored_path_list);
+ }
+ resolver_->set_ignored_search_list(ignored_path_list);
+}
+
// called via RPC (e.g., from ManagerDBusAdaptor)
ServiceRefPtr Manager::GetService(const KeyValueStore &args, Error *error) {
if (args.ContainsString(flimflam::kTypeProperty) &&
diff --git a/manager.h b/manager.h
index 5077637..759fefc 100644
--- a/manager.h
+++ b/manager.h
@@ -33,10 +33,12 @@
class ControlInterface;
class DBusManager;
+class DefaultProfile;
class Error;
class EventDispatcher;
class ManagerAdaptorInterface;
class Metrics;
+class Resolver;
class Manager : public base::SupportsWeakPtr<Manager> {
public:
@@ -63,6 +65,8 @@
// Comma-separated list of technologies for which link-monitoring is
// enabled.
std::string link_monitor_technologies;
+ // Comma-separated list of DNS search paths to be ignored.
+ std::string ignored_dns_search_paths;
};
Manager(ControlInterface *control_interface,
@@ -286,8 +290,10 @@
std::string GetActiveProfileRpcIdentifier(Error *error);
std::string GetCheckPortalList(Error *error);
RpcIdentifier GetDefaultServiceRpcIdentifier(Error *error);
+ std::string GetIgnoredDNSSearchPaths(Error *error);
ServiceRefPtr GetServiceInner(const KeyValueStore &args, Error *error);
void SetCheckPortalList(const std::string &portal_list, Error *error);
+ void SetIgnoredDNSSearchPaths(const std::string &ignored_paths, Error *error);
void EmitDefaultService();
bool IsTechnologyInList(const std::string &technology_list,
Technology::Identifier tech) const;
@@ -299,6 +305,9 @@
// increment |service_iterator|).
bool UnloadService(std::vector<ServiceRefPtr>::iterator *service_iterator);
+ // Load Manager default properties from |profile|.
+ bool LoadProperties(const scoped_refptr<DefaultProfile> &profile);
+
void HelpRegisterConstDerivedRpcIdentifier(
const std::string &name,
RpcIdentifier(Manager::*get)(Error *));
@@ -342,6 +351,8 @@
ModemInfo modem_info_;
VPNProvider vpn_provider_;
WiMaxProvider wimax_provider_;
+ // Hold pointer to singleton Resolver instance for testing purposes.
+ Resolver *resolver_;
bool running_;
// Used to facilitate unit tests which can't use RPC.
bool connect_profiles_to_rpc_;
diff --git a/manager_unittest.cc b/manager_unittest.cc
index 441cfe3..2e73f5a 100644
--- a/manager_unittest.cc
+++ b/manager_unittest.cc
@@ -33,6 +33,7 @@
#include "shill/mock_metrics.h"
#include "shill/mock_power_manager.h"
#include "shill/mock_profile.h"
+#include "shill/mock_resolver.h"
#include "shill/mock_service.h"
#include "shill/mock_store.h"
#include "shill/mock_wifi.h"
@@ -205,6 +206,18 @@
return manager()->GetDefaultServiceRpcIdentifier(NULL);
}
+ void SetResolver(Resolver *resolver) {
+ manager()->resolver_ = resolver;
+ }
+
+ void SetIgnoredDNSSearchPaths(const string &search_paths) {
+ manager()->SetIgnoredDNSSearchPaths(search_paths, NULL);
+ }
+
+ const string &GetIgnoredDNSSearchPaths() {
+ return manager()->props_.ignored_dns_search_paths;
+ }
+
protected:
typedef scoped_refptr<MockService> MockServiceRefPtr;
@@ -2491,4 +2504,28 @@
EXPECT_TRUE(error.IsOngoing());
}
+TEST_F(ManagerTest, IgnoredSearchList) {
+ scoped_ptr<MockResolver> resolver(new StrictMock<MockResolver>());
+ SetResolver(resolver.get());
+ vector<string> ignored_paths;
+ EXPECT_CALL(*resolver.get(), set_ignored_search_list(ignored_paths));
+ SetIgnoredDNSSearchPaths("");
+ EXPECT_EQ("", GetIgnoredDNSSearchPaths());
+
+ const string kIgnored0 = "chromium.org";
+ ignored_paths.push_back(kIgnored0);
+ EXPECT_CALL(*resolver.get(), set_ignored_search_list(ignored_paths));
+ SetIgnoredDNSSearchPaths(kIgnored0);
+ EXPECT_EQ(kIgnored0, GetIgnoredDNSSearchPaths());
+
+ const string kIgnored1 = "google.com";
+ const string kIgnoredSum = kIgnored0 + "," + kIgnored1;
+ ignored_paths.push_back(kIgnored1);
+ EXPECT_CALL(*resolver.get(), set_ignored_search_list(ignored_paths));
+ SetIgnoredDNSSearchPaths(kIgnoredSum);
+ EXPECT_EQ(kIgnoredSum, GetIgnoredDNSSearchPaths());
+
+ SetResolver(Resolver::GetInstance());
+}
+
} // namespace shill
diff --git a/mock_resolver.h b/mock_resolver.h
index fe2a5a2..ac404e5 100644
--- a/mock_resolver.h
+++ b/mock_resolver.h
@@ -25,6 +25,8 @@
const std::vector<std::string> &domain_search,
TimeoutParameters timeout));
MOCK_METHOD0(ClearDNS, bool());
+ MOCK_METHOD1(set_ignored_search_list,
+ void(const std::vector<std::string> &ignored_list));
private:
DISALLOW_COPY_AND_ASSIGN(MockResolver);
diff --git a/resolver.cc b/resolver.cc
index e10c587..6826ece 100644
--- a/resolver.cc
+++ b/resolver.cc
@@ -4,6 +4,7 @@
#include "shill/resolver.h"
+#include <algorithm>
#include <string>
#include <vector>
@@ -24,6 +25,7 @@
base::LazyInstance<Resolver> g_resolver = LAZY_INSTANCE_INITIALIZER;
} // namespace
+const char Resolver::kDefaultIgnoredSearchList[] = "gateway.2wire.net";
const char Resolver::kDefaultShortTimeoutTechnologies[] = "ethernet,wifi";
const char Resolver::kDefaultTimeoutOptions[] =
"options single-request timeout:1 attempts:3";
@@ -38,8 +40,8 @@
return g_resolver.Pointer();
}
-bool Resolver::SetDNSFromLists(const std::vector<std::string> &dns_servers,
- const std::vector<std::string> &domain_search,
+bool Resolver::SetDNSFromLists(const vector<string> &dns_servers,
+ const vector<string> &domain_search,
TimeoutParameters timeout) {
SLOG(Resolver, 2) << __func__;
@@ -57,8 +59,18 @@
lines.push_back("nameserver " + *iter);
}
- if (!domain_search.empty()) {
- lines.push_back("search " + JoinString(domain_search, ' '));
+ vector<string> filtered_domain_search;
+ for (iter = domain_search.begin();
+ iter != domain_search.end(); ++iter) {
+ if (std::find(ignored_search_list_.begin(),
+ ignored_search_list_.end(),
+ *iter) == ignored_search_list_.end()) {
+ filtered_domain_search.push_back(*iter);
+ }
+ }
+
+ if (!filtered_domain_search.empty()) {
+ lines.push_back("search " + JoinString(filtered_domain_search, ' '));
}
// Send queries one-at-a-time, rather than parallelizing IPv4
diff --git a/resolver.h b/resolver.h
index 2aeb4a7..6a2ccb1 100644
--- a/resolver.h
+++ b/resolver.h
@@ -25,6 +25,14 @@
kShortTimeout
};
+ // The default comma-separated list of search-list prefixes that
+ // should be ignored when writing out a DNS configuration. These
+ // are usually preconfigured by a DHCP server and are not of real
+ // value to the user. This will release DNS bandwidth for searches
+ // we expect will have a better chance of getting what the user is
+ // looking for.
+ static const char kDefaultIgnoredSearchList[];
+
// The default comma-separated list of technologies for which short
// DNS timeouts should be enabled.
static const char kDefaultShortTimeoutTechnologies[];
@@ -46,6 +54,13 @@
// Remove any created domain name service file.
virtual bool ClearDNS();
+ // Sets the list of ignored DNS search suffixes. This list will be used
+ // to filter the domain_search parameter of later SetDNSFromLists() calls.
+ virtual void set_ignored_search_list(
+ const std::vector<std::string> &ignored_list) {
+ ignored_search_list_ = ignored_list;
+ }
+
protected:
Resolver();
@@ -57,6 +72,7 @@
static const char kShortTimeoutOptions[];
FilePath path_;
+ std::vector<std::string> ignored_search_list_;
DISALLOW_COPY_AND_ASSIGN(Resolver);
};
diff --git a/resolver_unittest.cc b/resolver_unittest.cc
index 2ede654..5988c12 100644
--- a/resolver_unittest.cc
+++ b/resolver_unittest.cc
@@ -24,6 +24,7 @@
const char kNameServer1[] = "8.8.9.9";
const char kSearchDomain0[] = "chromium.org";
const char kSearchDomain1[] = "google.com";
+const char kSearchDomain2[] = "crosbug.com";
const char kExpectedOutput[] =
"nameserver 8.8.8.8\n"
"nameserver 8.8.9.9\n"
@@ -34,6 +35,11 @@
"nameserver 8.8.9.9\n"
"search chromium.org google.com\n"
"options single-request timeout-ms:300 attempts:15\n";
+const char kExpectedIgnoredSearchOutput[] =
+ "nameserver 8.8.8.8\n"
+ "nameserver 8.8.9.9\n"
+ "search google.com\n"
+ "options single-request timeout:1 attempts:3\n";
} // namespace {}
class ResolverTest : public Test {
@@ -117,4 +123,27 @@
EXPECT_FALSE(file_util::PathExists(path_));
}
+TEST_F(ResolverTest, IgnoredSearchList) {
+ EXPECT_FALSE(file_util::PathExists(path_));
+ EXPECT_TRUE(resolver_->ClearDNS());
+
+ MockControl control;
+ vector<string> dns_servers;
+ vector<string> domain_search;
+ dns_servers.push_back(kNameServer0);
+ dns_servers.push_back(kNameServer1);
+ domain_search.push_back(kSearchDomain0);
+ domain_search.push_back(kSearchDomain1);
+ vector<string> ignored_search;
+ ignored_search.push_back(kSearchDomain0);
+ ignored_search.push_back(kSearchDomain2);
+ resolver_->set_ignored_search_list(ignored_search);
+ EXPECT_TRUE(resolver_->SetDNSFromLists(
+ dns_servers, domain_search, Resolver::kDefaultTimeout));
+ EXPECT_TRUE(file_util::PathExists(path_));
+ EXPECT_EQ(kExpectedIgnoredSearchOutput, ReadFile());
+
+ EXPECT_TRUE(resolver_->ClearDNS());
+}
+
} // namespace shill