shill: Service: Remove DHCP minimal config code
This code does not end up being very useful (as evidenced by
UMA stats) and can reduce functionality in infrastructures
that require the use of some DHCP options.
This reverts commit 26a997b04bd65a6252987ea6fdcbbb70785d1d8d,
except for the cleanup of "*service" in unit tests and the
setting of SelectedService in ethernet.cc.
BUG=chromium:360452
TEST=Unit tests
Change-Id: Ib2c49fc5639452bfffd5cd03c1bb69e32568cc60
Reviewed-on: https://chromium-review.googlesource.com/205588
Commit-Queue: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
diff --git a/device.cc b/device.cc
index 9111f9d..76df528 100644
--- a/device.cc
+++ b/device.cc
@@ -437,7 +437,7 @@
}
bool Device::ShouldUseMinimalDHCPConfig() const {
- return selected_service_ && selected_service_->ShouldUseMinimalDHCPConfig();
+ return false;
}
bool Device::IsUsingStaticIP() const {
@@ -556,7 +556,6 @@
// SetConnection must occur after the UpdateFromIPConfig so the
// service can use the values derived from the connection.
if (selected_service_) {
- selected_service_->OnDHCPSuccess();
selected_service_->SetConnection(connection_);
}
// The service state change needs to happen last, so that at the
@@ -581,8 +580,6 @@
// TODO(pstew): This logic gets yet more complex when multiple
// IPConfig types are run in parallel (e.g. DHCP and DHCP6)
if (selected_service_) {
- selected_service_->OnDHCPFailure();
-
if (IsUsingStaticIP()) {
// Consider three cases:
//
diff --git a/device_unittest.cc b/device_unittest.cc
index 6910b48..05d006f 100644
--- a/device_unittest.cc
+++ b/device_unittest.cc
@@ -394,7 +394,6 @@
metrics(),
manager()));
SelectService(service);
- EXPECT_CALL(*service, OnDHCPFailure());
EXPECT_CALL(*service, DisconnectWithFailure(Service::kFailureDHCP, _));
EXPECT_CALL(*service, SetConnection(IsNullRefPtr()));
EXPECT_CALL(*ipconfig, ResetProperties());
@@ -414,7 +413,6 @@
service->static_ip_parameters_.args_.SetInt(kPrefixlenProperty, 16);
// Even though we won't call DisconnectWithFailure, we should still have
// the service learn from the failed DHCP attempt.
- EXPECT_CALL(*service, OnDHCPFailure());
EXPECT_CALL(*service, DisconnectWithFailure(_, _)).Times(0);
EXPECT_CALL(*service, SetConnection(_)).Times(0);
// The IPConfig should retain the previous values.
@@ -438,7 +436,6 @@
EXPECT_CALL(*service, IsPortalDetectionDisabled())
.WillRepeatedly(Return(true));
EXPECT_CALL(*service, SetState(Service::kStateOnline));
- EXPECT_CALL(*service, OnDHCPSuccess());
EXPECT_CALL(*service, SetConnection(NotNullRefPtr()));
EXPECT_CALL(*GetDeviceMockAdaptor(),
EmitRpcIdentifierArrayChanged(
@@ -791,23 +788,6 @@
}
TEST_F(DeviceTest, ShouldUseMinimalDHCPConfig) {
- // With no selected service, we should not choose a minimal config.
- EXPECT_FALSE(device_->ShouldUseMinimalDHCPConfig());
-
- MockManager manager(control_interface(),
- dispatcher(),
- metrics(),
- glib());
- scoped_refptr<MockService> service(
- new StrictMock<MockService>(control_interface(),
- dispatcher(),
- metrics(),
- &manager));
- SelectService(service);
- EXPECT_CALL(*service, ShouldUseMinimalDHCPConfig())
- .WillOnce(Return(true))
- .WillOnce(Return(false));
- EXPECT_TRUE(device_->ShouldUseMinimalDHCPConfig());
EXPECT_FALSE(device_->ShouldUseMinimalDHCPConfig());
}
diff --git a/metrics.cc b/metrics.cc
index d9e183e..277da88 100644
--- a/metrics.cc
+++ b/metrics.cc
@@ -252,10 +252,6 @@
const int Metrics::kMetricVpnUserAuthenticationTypeMax =
Metrics::kVpnUserAuthenticationTypeMax;
-// static
-const char Metrics::kMetricDHCPOptionFailureDetected[] =
- "Network.Shill.DHCPOptionFailureDetected";
-
const char Metrics::kMetricExpiredLeaseLengthSecondsSuffix[] =
"ExpiredLeaseLengthSeconds";
const int Metrics::kMetricExpiredLeaseLengthSecondsMax =
@@ -1057,36 +1053,6 @@
kCorruptedProfileMax);
}
-void Metrics::NotifyDHCPOptionFailure(const Service &service) {
- int failure_technology = kDHCPOptionFailureTechnologyUnknown;
- switch (service.technology()) {
- case Technology::kCellular:
- failure_technology = kDHCPOptionFailureTechnologyCellular;
- break;
- case Technology::kEthernet:
- failure_technology = kDHCPOptionFailureTechnologyEthernet;
- break;
- case Technology::kEthernetEap:
- failure_technology = kDHCPOptionFailureTechnologyEthernetEap;
- break;
- case Technology::kWifi:
- failure_technology = kDHCPOptionFailureTechnologyWifi;
- break;
- case Technology::kWiMax:
- failure_technology = kDHCPOptionFailureTechnologyWiMax;
- break;
- case Technology::kVPN:
- failure_technology = kDHCPOptionFailureTechnologyVPN;
- break;
- default:
- break;
- }
-
- SendEnumToUMA(kMetricDHCPOptionFailureDetected,
- failure_technology,
- kDHCPOptionFailureTechnologyMax);
-}
-
void Metrics::NotifyWifiAutoConnectableServices(int num_services) {
SendToUMA(kMetricWifiAutoConnectableServices,
num_services,
diff --git a/metrics.h b/metrics.h
index d86f064..f1f85c0 100644
--- a/metrics.h
+++ b/metrics.h
@@ -264,17 +264,6 @@
kVpnUserAuthenticationTypeMax
};
- enum DHCPOptionFailureTechnology {
- kDHCPOptionFailureTechnologyCellular = 0,
- kDHCPOptionFailureTechnologyEthernet,
- kDHCPOptionFailureTechnologyEthernetEap,
- kDHCPOptionFailureTechnologyWifi,
- kDHCPOptionFailureTechnologyWiMax,
- kDHCPOptionFailureTechnologyVPN,
- kDHCPOptionFailureTechnologyUnknown,
- kDHCPOptionFailureTechnologyMax
- };
-
enum UserInitiatedEvent {
kUserInitiatedEventWifiScan = 0,
kUserInitiatedEventMax
@@ -457,11 +446,6 @@
static const char kMetricVpnUserAuthenticationType[];
static const int kMetricVpnUserAuthenticationTypeMax;
- // We have detected that a DHCP server can only deliver leases if
- // we reduce the number of options that we request of it. This
- // implies an infrastructure issue.
- static const char kMetricDHCPOptionFailureDetected[];
-
// The length in seconds of a lease that has expired while the DHCP
// client was attempting to renew the lease..
static const char kMetricExpiredLeaseLengthSecondsSuffix[];
@@ -686,9 +670,6 @@
// Notifies this object about a corrupted profile.
virtual void NotifyCorruptedProfile();
- // Notifies this object about a service with DHCP infrastructure problems.
- virtual void NotifyDHCPOptionFailure(const Service &service);
-
// Notifies this object about user-initiated event.
virtual void NotifyUserInitiatedEvent(int event);
diff --git a/mock_metrics.h b/mock_metrics.h
index 84177ec..02fdc9e 100644
--- a/mock_metrics.h
+++ b/mock_metrics.h
@@ -39,7 +39,6 @@
MOCK_METHOD0(Notify3GPPRegistrationDelayedDropPosted, void());
MOCK_METHOD0(Notify3GPPRegistrationDelayedDropCanceled, void());
MOCK_METHOD0(NotifyCorruptedProfile, void());
- MOCK_METHOD1(NotifyDHCPOptionFailure, void(const Service &service));
MOCK_METHOD3(SendEnumToUMA, bool(const std::string &name, int sample,
int max));
MOCK_METHOD5(SendToUMA, bool(const std::string &name, int sample, int min,
diff --git a/mock_service.h b/mock_service.h
index 6e37187..a4a681f 100644
--- a/mock_service.h
+++ b/mock_service.h
@@ -64,9 +64,6 @@
MOCK_CONST_METHOD0(eap, const EapCredentials *());
MOCK_CONST_METHOD0(technology, Technology::Identifier());
MOCK_METHOD1(OnPropertyChanged, void(const std::string &property));
- MOCK_METHOD0(OnDHCPFailure, void());
- MOCK_METHOD0(OnDHCPSuccess, void());
- MOCK_METHOD0(ShouldUseMinimalDHCPConfig, bool());
MOCK_METHOD0(ClearExplicitlyDisconnected, void());
// Set a string for this Service via |store|. Can be wired to Save() for
diff --git a/service.cc b/service.cc
index 0f9928f..afa1d1c 100644
--- a/service.cc
+++ b/service.cc
@@ -79,7 +79,6 @@
const char Service::kStorageFavorite[] = "Favorite";
const char Service::kStorageGUID[] = "GUID";
const char Service::kStorageHasEverConnected[] = "HasEverConnected";
-const char Service::kStorageLastDHCPOptionFailure[] = "LastDHCPOptionFailure";
const char Service::kStorageName[] = "Name";
const char Service::kStoragePriority[] = "Priority";
const char Service::kStorageProxyConfig[] = "ProxyConfig";
@@ -100,10 +99,6 @@
const int Service::kReportDisconnectsThreshold = 2;
const int Service::kReportMisconnectsThreshold = 3;
const int Service::kMaxDisconnectEventHistory = 20;
-const int Service::kMaxDHCPOptionFailures = 2;
-
-// If we encounter MTU issues, hold off sending a full DHCP request for 30 days.
-const int Service::kDHCPOptionHoldOffPeriodSeconds = 30 * 24 * 60 * 60;
// static
unsigned int Service::next_serial_number_ = 0;
@@ -151,8 +146,6 @@
sockets_(new Sockets()),
time_(Time::GetInstance()),
diagnostics_reporter_(DiagnosticsReporter::GetInstance()),
- consecutive_dhcp_failures_(0),
- dhcp_option_failure_state_(kDHCPOptionFailureNotDetected),
connection_id_(0) {
HelpRegisterDerivedBool(kAutoConnectProperty,
&Service::GetAutoConnect,
@@ -472,17 +465,6 @@
storage->GetBool(id, kStorageSaveCredentials, &save_credentials_);
LoadString(storage, id, kStorageUIData, "", &ui_data_);
- uint64 last_dhcp_option_failure;
- if (storage->GetUint64(id, kStorageLastDHCPOptionFailure,
- &last_dhcp_option_failure)) {
- struct timeval tv = { 0, 0 };
- tv.tv_sec = last_dhcp_option_failure;
- last_dhcp_option_failure_ = Timestamp(tv, "");
- dhcp_option_failure_state_ = kDHCPOptionFailureConfirmed;
- } else {
- last_dhcp_option_failure_ = Timestamp();
- dhcp_option_failure_state_ = kDHCPOptionFailureNotDetected;
- }
storage->GetInt(id, kStorageConnectionId, &connection_id_);
static_ip_parameters_.Load(storage, id);
@@ -508,9 +490,6 @@
proxy_config_ = "";
save_credentials_ = true;
ui_data_ = "";
- consecutive_dhcp_failures_ = 0;
- last_dhcp_option_failure_ = Timestamp();
- dhcp_option_failure_state_ = kDHCPOptionFailureNotDetected;
if (mutable_eap()) {
mutable_eap()->Reset();
}
@@ -563,25 +542,12 @@
storage->SetBool(id, kStorageSaveCredentials, save_credentials_);
SaveString(storage, id, kStorageUIData, ui_data_, false, true);
- if (dhcp_option_failure_state_ == kDHCPOptionFailureConfirmed ||
- dhcp_option_failure_state_ == kDHCPOptionFailureRetestMinimalRequest ||
- dhcp_option_failure_state_ == kDHCPOptionFailureRetestFullRequest ||
- dhcp_option_failure_state_ == kDHCPOptionFailureRetestGotNoReply) {
- // In any of the states where we maintain a history of DHCP option
- // failures and we haven't confirmed that this is no longer an issue,
- // we should save the last option failure time even if it is very long ago.
- storage->SetUint64(id, kStorageLastDHCPOptionFailure,
- last_dhcp_option_failure_.monotonic.tv_sec);
- } else {
- storage->DeleteKey(id, kStorageLastDHCPOptionFailure);
- }
storage->SetInt(id, kStorageConnectionId, connection_id_);
static_ip_parameters_.Save(storage, id);
if (eap()) {
eap()->Save(storage, id, save_credentials_);
}
-
return true;
}
@@ -881,7 +847,7 @@
struct timeval period = (const struct timeval){ seconds_ago };
while (!events->empty()) {
if (events->size() < static_cast<size_t>(kMaxDisconnectEventHistory)) {
- struct timeval elapsed { 0 };
+ struct timeval elapsed = {0, 0};
timersub(&now.monotonic, &events->front().monotonic, &elapsed);
if (timercmp(&elapsed, &period, <)) {
break;
@@ -1556,133 +1522,6 @@
adaptor_->EmitStringChanged(kErrorProperty, error);
}
-void Service::OnDHCPFailure() {
- consecutive_dhcp_failures_++;
- switch (dhcp_option_failure_state_) {
- case kDHCPOptionFailureNotDetected:
- // If we run into too many consecutive DHCP failures, we must suspect
- // that this may be due in part to the number of options we are requesting
- // from the server. Next time, we should try asking for less options.
- if (consecutive_dhcp_failures_ >= kMaxDHCPOptionFailures) {
- SLOG(Service, 2) << "Service " << unique_name_ << " is failing to "
- << "receive DHCP responses. The next attempt will "
- << "be with minimal DHCP options.";
- dhcp_option_failure_state_ = kDHCPOptionFailureSuspected;
- }
- break;
-
- case kDHCPOptionFailureSuspected:
- // Requesting a shorter DHCP reply does not seem to have helped. As
- // such, we should exonerate DHCP options as the source of the failures.
- SLOG(Service, 2) << "Service " << unique_name_ << " did not get a DHCP "
- << "response even with minimal options set.";
- dhcp_option_failure_state_ = kDHCPOptionFailureNotDetected;
-
- // Unless we set our failure count back to 0, we'll end up toggling back
- // and forth endlessly between the Suspected and NotDetected states.
- consecutive_dhcp_failures_ = 0;
- break;
-
- case kDHCPOptionFailureConfirmed:
- // Nothing to do here. We've previously confirmed that this network
- // has problems with requests for a large number of DHCP options, so
- // we'll continue to send small requests until our timeout expires.
- break;
-
- case kDHCPOptionFailureRetestFullRequest:
- // This means that we are still running into issues with DHCP responses
- // when we send a full request. It's likely that the problem with this
- // network still exists. Let's confirm this by switching back to minimal
- // requests.
- SLOG(Service, 2) << "Service " << unique_name_ << " tried a full DHCP "
- << "request and it still appears to be failing.";
- dhcp_option_failure_state_ = kDHCPOptionFailureRetestMinimalRequest;
- break;
-
- case kDHCPOptionFailureRetestMinimalRequest:
- // Requesting a shorter DHCP reply does not seem to have helped.
- // However we still have a memory of this once being a problem, so
- // it is probably best to continue sending short requests and postpone
- // a re-test until we get a reply.
- SLOG(Service, 2) << "Service " << unique_name_ << " seems not to be "
- << "getting any DHCP responses at all.";
- dhcp_option_failure_state_ = kDHCPOptionFailureRetestGotNoReply;
- break;
-
- case kDHCPOptionFailureRetestGotNoReply:
- // Nothing to do here. We are still failing to receive a reply.
- break;
- }
-}
-
-void Service::OnDHCPSuccess() {
- consecutive_dhcp_failures_ = 0;
- switch (dhcp_option_failure_state_) {
- case kDHCPOptionFailureNotDetected:
- // Nothing to do. All is well with the world.
- break;
-
- case kDHCPOptionFailureSuspected: // FALLTHROUGH
- case kDHCPOptionFailureRetestMinimalRequest:
- LOG(WARNING) << "Service " << unique_name_ << " can get a DHCP "
- << "response only with minimal options requested.";
- // We just performed an experiment to prove that the DHCP failures we
- // have been having might be related to the number of options we have
- // been requesting. It appears that as soon as we reduced the option
- // count, our request succeeded.
- dhcp_option_failure_state_ = kDHCPOptionFailureConfirmed;
- last_dhcp_option_failure_ = time_->GetNow();
- metrics_->NotifyDHCPOptionFailure(*this);
- break;
-
- case kDHCPOptionFailureConfirmed:
- // Nothing to do here. We've previously confirmed that this network
- // has problems with requests for a large number of DHCP options, so
- // we'll continue to send small requests until our timeout expires.
- break;
-
- case kDHCPOptionFailureRetestFullRequest:
- // We expected this request to fail, since we attempted a full request
- // on a network we previously believed dropped DHCP replies for such
- // requests. Let's exonerate this network.
- SLOG(Service, 2) << "Service " << unique_name_ << " was able to receive "
- << "a response to a full DHCP request. Switching back "
- << "to full requests by default.";
- dhcp_option_failure_state_ = kDHCPOptionFailureNotDetected;
- break;
-
- case kDHCPOptionFailureRetestGotNoReply:
- // We are finally receiving DHCP replies again. Resume the test
- // starting with the next time we perform a DHCP request.
- SLOG(Service, 2) << "Service " << unique_name_ << " was finally able to "
- << "receive a DHCP response.";
- dhcp_option_failure_state_ = kDHCPOptionFailureRetestFullRequest;
- break;
- }
-}
-
-bool Service::ShouldUseMinimalDHCPConfig() {
- // If it's been a while since we have tested for options failures, we should
- // re-confirm if this issue still exists.
- if (dhcp_option_failure_state_ == kDHCPOptionFailureConfirmed) {
- Timestamp now = time_->GetNow();
- struct timeval elapsed { 0 };
- timersub(&now.monotonic, &last_dhcp_option_failure_.monotonic, &elapsed);
- struct timeval period { kDHCPOptionHoldOffPeriodSeconds };
- if (timercmp(&elapsed, &period, >=)) {
- SLOG(Service, 2) << "Service " << unique_name_ << " will be re-tested to "
- << "see if the server responds to a full DHCP request.";
- dhcp_option_failure_state_ = kDHCPOptionFailureRetestFullRequest;
- }
- }
-
- return
- dhcp_option_failure_state_ == kDHCPOptionFailureSuspected ||
- dhcp_option_failure_state_ == kDHCPOptionFailureConfirmed ||
- dhcp_option_failure_state_ == kDHCPOptionFailureRetestMinimalRequest ||
- dhcp_option_failure_state_ == kDHCPOptionFailureRetestGotNoReply;
-}
-
void Service::ClearExplicitlyDisconnected() {
if (explicitly_disconnected_) {
explicitly_disconnected_ = false;
diff --git a/service.h b/service.h
index 23d82c8..fd02b7d 100644
--- a/service.h
+++ b/service.h
@@ -73,7 +73,6 @@
static const char kStorageFavorite[];
static const char kStorageGUID[];
static const char kStorageHasEverConnected[];
- static const char kStorageLastDHCPOptionFailure[];
static const char kStorageName[];
static const char kStoragePriority[];
static const char kStorageProxyConfig[];
@@ -124,14 +123,7 @@
kCryptoRc4,
kCryptoAes
};
- enum DHCPOptionFailureState {
- kDHCPOptionFailureNotDetected,
- kDHCPOptionFailureSuspected,
- kDHCPOptionFailureConfirmed,
- kDHCPOptionFailureRetestFullRequest,
- kDHCPOptionFailureRetestMinimalRequest,
- kDHCPOptionFailureRetestGotNoReply
- };
+
static const int kPriorityNone;
// A constructor for the Service object
@@ -430,15 +422,6 @@
// Called by the manager once after a resume.
virtual void OnAfterResume();
- // Called by the device if a DHCP attempt fails.
- virtual void OnDHCPFailure();
-
- // Called by the device if a DHCP attempt succeeds.
- virtual void OnDHCPSuccess();
-
- // Called by the device to test for historical DHCP issues.
- virtual bool ShouldUseMinimalDHCPConfig();
-
// Called by the manager to clear remembered state of being explicitly
// disconnected.
virtual void ClearExplicitlyDisconnected();
@@ -681,15 +664,6 @@
static const int kReportMisconnectsThreshold;
static const int kMaxDisconnectEventHistory;
- // The number of DHCP failures to allow before we start suspecting that
- // this may be indirectly due to the number of options we are requesting.
- static const int kMaxDHCPOptionFailures;
-
- // The number of DHCP failures to allow before we start suspecting that
- // this may be indirectly due to the number of options we are requesting.
- static const int kDHCPOptionHoldOffPeriodSeconds;
-
-
bool GetAutoConnect(Error *error);
std::string GetCheckPortal(Error *error);
@@ -829,17 +803,6 @@
Time *time_;
DiagnosticsReporter *diagnostics_reporter_;
- // Tracks the number of consecutive failed DHCP attempts.
- int consecutive_dhcp_failures_;
-
- // Tracks the last time we had a failure that appered correlated
- // to the DHCP client asking for too many options.
- Timestamp last_dhcp_option_failure_;
-
- // Tracks the current progress in deciding if DHCP failures appear
- // correlated to requests for a large number of DHCP options.
- DHCPOptionFailureState dhcp_option_failure_state_;
-
// The |serial_number_| for the next Service.
static unsigned int next_serial_number_;
diff --git a/service_unittest.cc b/service_unittest.cc
index 7e934ab..96998f2 100644
--- a/service_unittest.cc
+++ b/service_unittest.cc
@@ -55,7 +55,6 @@
using testing::ReturnNull;
using testing::ReturnRef;
using testing::StrictMock;
-using testing::StrNe;
using testing::SetArgumentPointee;
using testing::Test;
using testing::Values;
@@ -179,39 +178,6 @@
return service_->SetAutoConnectFull(connect, error);
}
- int GetConsecutiveDHCPFailures() {
- return service_->consecutive_dhcp_failures_;
- }
-
- void SetConsecutiveDHCPFailures(int failures) {
- service_->consecutive_dhcp_failures_ = failures;
- }
-
- int GetLastDHCPOptionFailure() {
- return service_->last_dhcp_option_failure_.monotonic.tv_sec;
- }
-
- void SetLastDHCPOptionFailure(int monotonic_seconds) {
- service_->last_dhcp_option_failure_ = GetTimestamp(monotonic_seconds, "");
- }
-
- Service::DHCPOptionFailureState GetDHCPOptionFailureState() {
- return service_->dhcp_option_failure_state_;
- }
-
- void SetDHCPOptionFailureState(
- Service::DHCPOptionFailureState failure_state) {
- service_->dhcp_option_failure_state_ = failure_state;
- }
-
- int GetMaxDHCPOptionFailures() {
- return Service::kMaxDHCPOptionFailures;
- }
-
- int GetDHCPOptionHoldOffPeriodSeconds() {
- return Service::kDHCPOptionHoldOffPeriodSeconds;
- }
-
MockManager mock_manager_;
MockDiagnosticsReporter diagnostics_reporter_;
MockTime time_;
@@ -669,19 +635,12 @@
EXPECT_FALSE(service_->explicitly_disconnected_);
EXPECT_TRUE(service_->has_ever_connected_);
service_->explicitly_disconnected_ = true;
- SetConsecutiveDHCPFailures(100);
- SetLastDHCPOptionFailure(200);
- SetDHCPOptionFailureState(Service::kDHCPOptionFailureConfirmed);
EXPECT_CALL(*eap_, Reset());
service_->Unload();
EXPECT_EQ(string(""), service_->ui_data_);
EXPECT_EQ(string(""), service_->guid_);
EXPECT_FALSE(service_->explicitly_disconnected_);
EXPECT_FALSE(service_->has_ever_connected_);
- EXPECT_EQ(0, GetConsecutiveDHCPFailures());
- EXPECT_EQ(0, GetLastDHCPOptionFailure());
- EXPECT_EQ(Service::kDHCPOptionFailureNotDetected,
- GetDHCPOptionFailureState());
}
TEST_F(ServiceTest, State) {
@@ -1925,310 +1884,6 @@
TestCustomSetterNoopChange(service_, &mock_manager_);
}
-TEST_F(ServiceTest, DHCPOptionFailureState) {
- // We are testing the transitions out of each node in this
- // state diagram:
- //
- // [ Not Detected (send full request) ] <------------
- // | ^ |
- // | | |
- // n * failure failure |
- // | | |
- // V | |
- // [ Suspected (send minimal request) ] |
- // | |
- // success |
- // | |
- // V |
- // [ Confirmed (send minimal request) ] |
- // ^ | |
- // | hold timer elapsed |
- // | | |
- // success V |
- // | [ Retest Full Request ] ----success--/
- // | | ^
- // | failure |
- // | | |
- // | V |
- // [ Retest Minimal Request ] |
- // | success
- // failure |
- // | |
- // V |
- // [ Retest With No Reply (send minimal requests) ]
-
- // Check initial state.
- EXPECT_EQ(0, GetConsecutiveDHCPFailures());
- EXPECT_EQ(0, GetLastDHCPOptionFailure());
- EXPECT_EQ(Service::kDHCPOptionFailureNotDetected,
- GetDHCPOptionFailureState());
-
- // Let's make up a constant to represent enough iterations that we
- // safely expect that the state machine won't transition as a result
- // of more iterations.
- const int kManyTimes = GetMaxDHCPOptionFailures() * 10;
-
- // NotDetected -> NotDetected.
- for (int i = 0; i < kManyTimes; ++i) {
- service_->OnDHCPSuccess();
- EXPECT_EQ(0, GetConsecutiveDHCPFailures());
- EXPECT_EQ(0, GetLastDHCPOptionFailure());
- EXPECT_EQ(Service::kDHCPOptionFailureNotDetected,
- GetDHCPOptionFailureState());
- EXPECT_FALSE(service_->ShouldUseMinimalDHCPConfig());
- }
-
- for (int i = 0; i < GetMaxDHCPOptionFailures() - 1; ++i) {
- service_->OnDHCPFailure();
- EXPECT_EQ(i + 1, GetConsecutiveDHCPFailures());
- EXPECT_EQ(0, GetLastDHCPOptionFailure());
- EXPECT_EQ(Service::kDHCPOptionFailureNotDetected,
- GetDHCPOptionFailureState());
- EXPECT_FALSE(service_->ShouldUseMinimalDHCPConfig());
- }
-
- NiceMock<MockStore> storage;
- EXPECT_CALL(storage,
- DeleteKey(_, StrNe(Service::kStorageLastDHCPOptionFailure)))
- .WillRepeatedly(Return(true));
- EXPECT_CALL(storage,
- DeleteKey(storage_id_, Service::kStorageLastDHCPOptionFailure));
- EXPECT_CALL(*eap_, Save(_, _, _)).Times(AnyNumber());
- EXPECT_TRUE(service_->Save(&storage));
-
- // NotDetected -> Suspected.
- service_->OnDHCPFailure();
- EXPECT_EQ(GetMaxDHCPOptionFailures(), GetConsecutiveDHCPFailures());
- // This value only updates at the time the failure is confirmed.
- EXPECT_EQ(0, GetLastDHCPOptionFailure());
- EXPECT_EQ(Service::kDHCPOptionFailureSuspected, GetDHCPOptionFailureState());
- EXPECT_TRUE(service_->ShouldUseMinimalDHCPConfig());
-
- EXPECT_CALL(storage,
- DeleteKey(storage_id_, Service::kStorageLastDHCPOptionFailure));
- EXPECT_TRUE(service_->Save(&storage));
-
- // Suspected -> NotDetected.
- EXPECT_EQ(GetMaxDHCPOptionFailures(), GetConsecutiveDHCPFailures());
- service_->OnDHCPFailure();
- EXPECT_EQ(0, GetConsecutiveDHCPFailures());
- EXPECT_EQ(0, GetLastDHCPOptionFailure());
- EXPECT_EQ(Service::kDHCPOptionFailureNotDetected,
- GetDHCPOptionFailureState());
- EXPECT_FALSE(service_->ShouldUseMinimalDHCPConfig());
-
- EXPECT_CALL(storage,
- DeleteKey(storage_id_, Service::kStorageLastDHCPOptionFailure));
- EXPECT_TRUE(service_->Save(&storage));
-
- // Suspected -> Confirmed.
- SetConsecutiveDHCPFailures(GetMaxDHCPOptionFailures());
- SetLastDHCPOptionFailure(0);
- SetDHCPOptionFailureState(Service::kDHCPOptionFailureSuspected);
-
- int kFirstFailureTime = 1234;
- EXPECT_CALL(time_, GetNow())
- .WillRepeatedly(Return(GetTimestamp(kFirstFailureTime, "")));
- EXPECT_CALL(*metrics(), NotifyDHCPOptionFailure(_));
- service_->OnDHCPSuccess();
- Mock::VerifyAndClearExpectations(metrics());
-
- EXPECT_EQ(kFirstFailureTime, GetLastDHCPOptionFailure());
- EXPECT_EQ(Service::kDHCPOptionFailureConfirmed, GetDHCPOptionFailureState());
- EXPECT_TRUE(service_->ShouldUseMinimalDHCPConfig());
-
- EXPECT_CALL(storage,
- SetUint64(storage_id_,
- Service::kStorageLastDHCPOptionFailure,
- kFirstFailureTime));
- EXPECT_TRUE(service_->Save(&storage));
-
- // Confirmed -> Confirmed.
- Mock::VerifyAndClearExpectations(&time_);
- int kAlmostAtFirstFailureExpiry =
- kFirstFailureTime + GetDHCPOptionHoldOffPeriodSeconds() - 1;
- EXPECT_CALL(time_, GetNow())
- .WillRepeatedly(Return(GetTimestamp(kAlmostAtFirstFailureExpiry, "")));
-
- for (int i = 0; i < kManyTimes; ++i) {
- service_->OnDHCPSuccess();
- EXPECT_EQ(0, GetConsecutiveDHCPFailures());
- EXPECT_EQ(kFirstFailureTime, GetLastDHCPOptionFailure());
- EXPECT_EQ(Service::kDHCPOptionFailureConfirmed,
- GetDHCPOptionFailureState());
- EXPECT_TRUE(service_->ShouldUseMinimalDHCPConfig());
- }
-
- for (int i = 0; i < kManyTimes; ++i) {
- service_->OnDHCPFailure();
- EXPECT_EQ(i + 1, GetConsecutiveDHCPFailures());
- EXPECT_EQ(kFirstFailureTime, GetLastDHCPOptionFailure());
- EXPECT_EQ(Service::kDHCPOptionFailureConfirmed,
- GetDHCPOptionFailureState());
- EXPECT_TRUE(service_->ShouldUseMinimalDHCPConfig());
- }
-
- EXPECT_CALL(storage,
- SetUint64(storage_id_,
- Service::kStorageLastDHCPOptionFailure,
- kFirstFailureTime));
- EXPECT_TRUE(service_->Save(&storage));
-
- // Confirmed -> RetestFullRequest.
- Mock::VerifyAndClearExpectations(&time_);
- int kFirstFailureExpiry =
- kFirstFailureTime + GetDHCPOptionHoldOffPeriodSeconds();
- EXPECT_CALL(time_, GetNow())
- .WillRepeatedly(Return(GetTimestamp(kFirstFailureExpiry, "")));
- EXPECT_FALSE(service_->ShouldUseMinimalDHCPConfig());
- EXPECT_EQ(kFirstFailureTime, GetLastDHCPOptionFailure());
- EXPECT_EQ(Service::kDHCPOptionFailureRetestFullRequest,
- GetDHCPOptionFailureState());
-
- EXPECT_CALL(storage,
- SetUint64(storage_id_,
- Service::kStorageLastDHCPOptionFailure,
- kFirstFailureTime));
- EXPECT_TRUE(service_->Save(&storage));
-
- // RetestFullRequest -> NotDetected.
- service_->OnDHCPSuccess();
- EXPECT_EQ(0, GetConsecutiveDHCPFailures());
- EXPECT_EQ(kFirstFailureTime, GetLastDHCPOptionFailure());
- EXPECT_EQ(Service::kDHCPOptionFailureNotDetected,
- GetDHCPOptionFailureState());
- EXPECT_FALSE(service_->ShouldUseMinimalDHCPConfig());
-
- EXPECT_CALL(storage,
- DeleteKey(storage_id_, Service::kStorageLastDHCPOptionFailure));
- EXPECT_TRUE(service_->Save(&storage));
-
- // RetestFullRequest -> RetestMinimalRequest.
- SetDHCPOptionFailureState(Service::kDHCPOptionFailureRetestFullRequest);
-
- service_->OnDHCPFailure();
- EXPECT_EQ(1, GetConsecutiveDHCPFailures());
- // This value only updates at the time the failure is confirmed.
- EXPECT_EQ(kFirstFailureTime, GetLastDHCPOptionFailure());
- EXPECT_EQ(Service::kDHCPOptionFailureRetestMinimalRequest,
- GetDHCPOptionFailureState());
- EXPECT_TRUE(service_->ShouldUseMinimalDHCPConfig());
-
- EXPECT_CALL(storage,
- SetUint64(storage_id_,
- Service::kStorageLastDHCPOptionFailure,
- kFirstFailureTime));
- EXPECT_TRUE(service_->Save(&storage));
-
- // RetestMinimalRequest -> Confirmed.
- Mock::VerifyAndClearExpectations(&time_);
- int kSecondFailureTime = kFirstFailureExpiry + 1;
- EXPECT_CALL(time_, GetNow())
- .WillRepeatedly(Return(GetTimestamp(kSecondFailureTime, "")));
- EXPECT_CALL(*metrics(), NotifyDHCPOptionFailure(_));
- service_->OnDHCPSuccess();
- Mock::VerifyAndClearExpectations(metrics());
-
- EXPECT_EQ(0, GetConsecutiveDHCPFailures());
- EXPECT_EQ(kSecondFailureTime, GetLastDHCPOptionFailure());
- EXPECT_EQ(Service::kDHCPOptionFailureConfirmed, GetDHCPOptionFailureState());
- EXPECT_TRUE(service_->ShouldUseMinimalDHCPConfig());
-
- EXPECT_CALL(storage,
- SetUint64(storage_id_,
- Service::kStorageLastDHCPOptionFailure,
- kSecondFailureTime));
- EXPECT_TRUE(service_->Save(&storage));
-
- // RetestMinimalRequest -> RetestGotNoReply.
- SetDHCPOptionFailureState(Service::kDHCPOptionFailureRetestMinimalRequest);
- SetConsecutiveDHCPFailures(1);
- SetLastDHCPOptionFailure(kFirstFailureTime);
-
- service_->OnDHCPFailure();
- EXPECT_EQ(2, GetConsecutiveDHCPFailures());
- EXPECT_EQ(kFirstFailureTime, GetLastDHCPOptionFailure());
- EXPECT_EQ(Service::kDHCPOptionFailureRetestGotNoReply,
- GetDHCPOptionFailureState());
- EXPECT_TRUE(service_->ShouldUseMinimalDHCPConfig());
-
- EXPECT_CALL(storage,
- SetUint64(storage_id_,
- Service::kStorageLastDHCPOptionFailure,
- kFirstFailureTime));
- EXPECT_TRUE(service_->Save(&storage));
-
- // RetestGotNoReply -> RetestGotNoReply.
- for (int i = 0; i < kManyTimes; ++i) {
- service_->OnDHCPFailure();
- EXPECT_EQ(i + 3, GetConsecutiveDHCPFailures());
- EXPECT_EQ(kFirstFailureTime, GetLastDHCPOptionFailure());
- EXPECT_EQ(Service::kDHCPOptionFailureRetestGotNoReply,
- GetDHCPOptionFailureState());
- EXPECT_TRUE(service_->ShouldUseMinimalDHCPConfig());
- }
-
- EXPECT_CALL(storage,
- SetUint64(storage_id_,
- Service::kStorageLastDHCPOptionFailure,
- kFirstFailureTime));
- EXPECT_TRUE(service_->Save(&storage));
-
- // RetestGotNoReply -> RetestFullRequest.
- service_->OnDHCPSuccess();
- EXPECT_EQ(0, GetConsecutiveDHCPFailures());
- EXPECT_EQ(kFirstFailureTime, GetLastDHCPOptionFailure());
- EXPECT_EQ(Service::kDHCPOptionFailureRetestFullRequest,
- GetDHCPOptionFailureState());
- EXPECT_FALSE(service_->ShouldUseMinimalDHCPConfig());
-
- EXPECT_CALL(storage,
- SetUint64(storage_id_,
- Service::kStorageLastDHCPOptionFailure,
- kFirstFailureTime));
- EXPECT_TRUE(service_->Save(&storage));
-
- // Load into NotDetected.
- EXPECT_CALL(storage, ContainsGroup(storage_id_)).WillRepeatedly(Return(true));
- EXPECT_CALL(*eap_, Load(&storage, storage_id_)).Times(AnyNumber());
- EXPECT_CALL(storage,
- GetUint64(storage_id_, Service::kStorageLastDHCPOptionFailure, _))
- .WillOnce(Return(false));
- EXPECT_TRUE(service_->Load(&storage));
- EXPECT_EQ(0, GetLastDHCPOptionFailure());
- EXPECT_EQ(Service::kDHCPOptionFailureNotDetected,
- GetDHCPOptionFailureState());
- EXPECT_FALSE(service_->ShouldUseMinimalDHCPConfig());
- // Note that ShouldUseMinimalDHCPConfig() doesn't change this state.
- EXPECT_EQ(Service::kDHCPOptionFailureNotDetected,
- GetDHCPOptionFailureState());
-
- // Load into Confirmed.
- EXPECT_CALL(storage,
- GetUint64(storage_id_, Service::kStorageLastDHCPOptionFailure, _))
- .WillOnce(DoAll(SetArgumentPointee<2>(kSecondFailureTime), Return(true)));
- EXPECT_TRUE(service_->Load(&storage));
- EXPECT_EQ(kSecondFailureTime, GetLastDHCPOptionFailure());
- EXPECT_EQ(Service::kDHCPOptionFailureConfirmed, GetDHCPOptionFailureState());
- EXPECT_TRUE(service_->ShouldUseMinimalDHCPConfig());
- // Note that ShouldUseMinimalDHCPConfig() doesn't change this state.
- EXPECT_EQ(Service::kDHCPOptionFailureConfirmed, GetDHCPOptionFailureState());
-
- // Load into RetestFullRequest.
- EXPECT_CALL(storage,
- GetUint64(storage_id_, Service::kStorageLastDHCPOptionFailure, _))
- .WillOnce(DoAll(SetArgumentPointee<2>(kFirstFailureTime), Return(true)));
- EXPECT_TRUE(service_->Load(&storage));
- EXPECT_EQ(kFirstFailureTime, GetLastDHCPOptionFailure());
- // At load time we believe we're confirmed...
- EXPECT_EQ(Service::kDHCPOptionFailureConfirmed, GetDHCPOptionFailureState());
- // But as soon as we query ShouldUseMinimalConfig, we'll switch, since
- // kFirstFailureTime is too far in the past.
- EXPECT_FALSE(service_->ShouldUseMinimalDHCPConfig());
- EXPECT_EQ(Service::kDHCPOptionFailureRetestFullRequest,
- GetDHCPOptionFailureState());
-}
-
TEST_F(ServiceTest, GetTethering) {
Error error;
EXPECT_EQ("", service_->GetTethering(&error));