shill: openvpn: Reselect service and reuse IP config on restart.
This got broken when we fixed the crash on disconnect or failed
connects.
BUG=chromium-os:35031
TEST=Unit tests. Also, connected to corp VPN, waited on the new tab
page ~10 minutes to trigger idle ping restart, observed service going
through Idle and Associating. Before the patch the connection got
stuck (broken ip routes, broken service state), after the patch
service got connected successfully, its state was updated properly and
VPN worked.
Change-Id: Ib86fab1db04f20810f1ada82b614cdb9ccdf1241
Reviewed-on: https://gerrit.chromium.org/gerrit/34737
Tested-by: Darin Petkov <petkov@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Commit-Ready: Darin Petkov <petkov@chromium.org>
diff --git a/device.h b/device.h
index 0273106..9cd823e 100644
--- a/device.h
+++ b/device.h
@@ -342,12 +342,13 @@
void set_link_monitor(LinkMonitor *link_monitor);
private:
+ friend class CellularCapabilityTest;
+ friend class CellularTest;
friend class DeviceAdaptorInterface;
friend class DeviceByteCountTest;
friend class DevicePortalDetectionTest;
friend class DeviceTest;
- friend class CellularTest;
- friend class CellularCapabilityTest;
+ friend class OpenVPNDriverTest;
friend class WiFiObjectTest;
static const char kIPFlagTemplate[];
diff --git a/openvpn_driver.cc b/openvpn_driver.cc
index aa86f8d..2e228ae 100644
--- a/openvpn_driver.cc
+++ b/openvpn_driver.cc
@@ -192,6 +192,7 @@
service_->SetState(state);
service_ = NULL;
}
+ ip_properties_ = IPConfig::Properties();
}
bool OpenVPNDriver::SpawnOpenVPN() {
@@ -278,7 +279,6 @@
device_ = new VPN(control_, dispatcher(), metrics_, manager(),
link_name, interface_index);
device_->SetEnabled(true);
- device_->SelectService(service_);
rpc_task_.reset(new RPCTask(control_, this));
if (!SpawnOpenVPN()) {
@@ -301,13 +301,10 @@
device_->OnDisconnected();
return;
}
- IPConfig::Properties properties;
- if (device_->ipconfig()) {
- // On restart/reconnect, update the existing IP configuration.
- properties = device_->ipconfig()->properties();
- }
- ParseIPConfiguration(dict, &properties);
- device_->UpdateIPConfig(properties);
+ // On restart/reconnect, update the existing IP configuration.
+ ParseIPConfiguration(dict, &ip_properties_);
+ device_->SelectService(service_);
+ device_->UpdateIPConfig(ip_properties_);
StopConnectTimeout();
}
diff --git a/openvpn_driver.h b/openvpn_driver.h
index a84f76a..b074a11 100644
--- a/openvpn_driver.h
+++ b/openvpn_driver.h
@@ -197,6 +197,7 @@
std::string tunnel_interface_;
VPNRefPtr device_;
FilePath tls_auth_file_;
+ IPConfig::Properties ip_properties_;
// The PID of the spawned openvpn process. May be 0 if no process has been
// spawned yet or the process has died.
diff --git a/openvpn_driver_unittest.cc b/openvpn_driver_unittest.cc
index 5067979..e1a0050 100644
--- a/openvpn_driver_unittest.cc
+++ b/openvpn_driver_unittest.cc
@@ -111,6 +111,10 @@
driver_->args()->RemoveString(arg);
}
+ const ServiceRefPtr &GetSelectedService() {
+ return device_->selected_service();
+ }
+
// Used to assert that a flag appears in the options.
void ExpectInFlags(const vector<string> &options, const string &flag,
const string &value);
@@ -241,19 +245,17 @@
TEST_F(OpenVPNDriverTest, Notify) {
map<string, string> config;
+ driver_->service_ = service_;
driver_->device_ = device_;
driver_->StartConnectTimeout();
EXPECT_CALL(*device_,
UpdateIPConfig(Field(&IPConfig::Properties::address, "")));
driver_->Notify("up", config);
EXPECT_FALSE(driver_->IsConnectTimeoutStarted());
+ EXPECT_TRUE(GetSelectedService().get() == service_.get());
// Tests that existing properties are reused if no new ones provided.
- IPConfigRefPtr ipconfig(new IPConfig(&control_, device_->link_name()));
- IPConfig::Properties props;
- props.address = "1.2.3.4";
- ipconfig->set_properties(props);
- device_->set_ipconfig(ipconfig);
+ driver_->ip_properties_.address = "1.2.3.4";
EXPECT_CALL(*device_,
UpdateIPConfig(Field(&IPConfig::Properties::address, "1.2.3.4")));
driver_->Notify("up", config);
@@ -764,6 +766,7 @@
driver_->tunnel_interface_ = kInterfaceName;
driver_->device_ = device_;
driver_->service_ = service_;
+ driver_->ip_properties_.address = "1.2.3.4";
driver_->StartConnectTimeout();
FilePath tls_auth_file;
EXPECT_TRUE(file_util::CreateTemporaryFile(&tls_auth_file));
@@ -789,6 +792,7 @@
EXPECT_FALSE(driver_->service_);
EXPECT_FALSE(file_util::PathExists(tls_auth_file));
EXPECT_TRUE(driver_->tls_auth_file_.empty());
+ EXPECT_TRUE(driver_->ip_properties_.address.empty());
EXPECT_FALSE(driver_->IsConnectTimeoutStarted());
}