Export the Physical Technology of a VPN service.
Exposes the Type of the lowest service used for a VPN service
through the "PhysicalTechnology" property of a VPN service.
TEST=cros_workon_make shill --test and tested manually.
BUG=chromium:213086
Manual Test procedure.
1. Connect to a wifi network.
2. Connect to a VPN service on that wifi network.
3. Run /usr/local/lib/flimflam/test/list-services on a terminal
and verify the first service on its output contains a service with
a Type "vpn" and a PhysicalTechnology "wifi":
[ /service/6 ]
...
Type = vpn
...
PhysicalTechnology = wifi
Change-Id: Ic843429de3f4d5a4208e271bb779c3e3160f036b
Reviewed-on: https://gerrit.chromium.org/gerrit/61510
Reviewed-by: Paul Stewart <pstew@chromium.org>
Commit-Queue: Alex Deymo <deymo@chromium.org>
Reviewed-by: Alex Deymo <deymo@chromium.org>
Tested-by: Alex Deymo <deymo@chromium.org>
diff --git a/connection.cc b/connection.cc
index 9bce98b..b581752 100644
--- a/connection.cc
+++ b/connection.cc
@@ -17,6 +17,7 @@
using base::Closure;
using base::Unretained;
using std::deque;
+using std::set;
using std::string;
using std::vector;
@@ -508,4 +509,24 @@
}
}
+ConnectionRefPtr Connection::GetCarrierConnection() {
+ SLOG(Connection, 2) << __func__ << " @ " << interface_name_;
+ set<Connection *> visited;
+ ConnectionRefPtr carrier = this;
+ while (carrier->GetLowerConnection()) {
+ if (ContainsKey(visited, carrier.get())) {
+ LOG(ERROR) << "Circular connection chain starting at: "
+ << carrier->interface_name();
+ // If a loop is detected return a NULL value to signal that the carrier
+ // connection is unknown.
+ return NULL;
+ }
+ visited.insert(carrier.get());
+ carrier = carrier->GetLowerConnection();
+ }
+ SLOG(Connection, 2) << "Carrier connection: " << carrier->interface_name()
+ << " @ " << interface_name_;
+ return carrier;
+}
+
} // namespace shill
diff --git a/connection.h b/connection.h
index cc8bff8..2ad529e 100644
--- a/connection.h
+++ b/connection.h
@@ -102,7 +102,11 @@
virtual const IPAddress &local() const { return local_; }
virtual const IPAddress &gateway() const { return gateway_; }
- Technology::Identifier technology() const { return technology_; }
+ virtual Technology::Identifier technology() const { return technology_; }
+
+ // Return the lowest connection on which this connection depends. In case of
+ // error, a NULL is returned.
+ virtual ConnectionRefPtr GetCarrierConnection();
protected:
friend class base::RefCounted<Connection>;
@@ -117,6 +121,7 @@
FRIEND_TEST(ConnectionTest, Binders);
FRIEND_TEST(ConnectionTest, Destructor);
FRIEND_TEST(ConnectionTest, FixGatewayReachability);
+ FRIEND_TEST(ConnectionTest, GetCarrierConnection);
FRIEND_TEST(ConnectionTest, InitState);
FRIEND_TEST(ConnectionTest, OnRouteQueryResponse);
FRIEND_TEST(ConnectionTest, RequestHostRoute);
diff --git a/connection_unittest.cc b/connection_unittest.cc
index 6faa8f8..6015c18 100644
--- a/connection_unittest.cc
+++ b/connection_unittest.cc
@@ -870,4 +870,34 @@
connection = NULL;
}
+TEST_F(ConnectionTest, GetCarrierConnection) {
+ EXPECT_EQ(connection_.get(), connection_->GetCarrierConnection().get());
+
+ ConnectionRefPtr connection1 = GetNewConnection();
+ ConnectionRefPtr connection2 = GetNewConnection();
+ ConnectionRefPtr connection3 = GetNewConnection();
+
+ connection_->lower_binder_.Attach(connection1);
+ EXPECT_EQ(connection1.get(), connection_->GetCarrierConnection().get());
+
+ connection1->lower_binder_.Attach(connection2);
+ EXPECT_EQ(connection2.get(), connection_->GetCarrierConnection().get());
+
+ connection2->lower_binder_.Attach(connection3);
+ EXPECT_EQ(connection3.get(), connection_->GetCarrierConnection().get());
+
+ // Create a cycle back to |connection1|.
+ connection3->lower_binder_.Attach(connection1);
+ EXPECT_EQ(NULL, connection_->GetCarrierConnection().get());
+
+ AddDestructorExpectations();
+ connection3 = NULL;
+
+ AddDestructorExpectations();
+ connection2 = NULL;
+
+ AddDestructorExpectations();
+ connection1 = NULL;
+}
+
} // namespace shill
diff --git a/doc/service-api.txt b/doc/service-api.txt
index 45d10fb..9ea2b55 100644
--- a/doc/service-api.txt
+++ b/doc/service-api.txt
@@ -1003,6 +1003,13 @@
[We will be supporting this soon for VPN]
+ string PhysicalTechnology [readonly, optional]
+
+ If the service type is "vpn" and the service is
+ connected, this property is present and exposes the
+ Type property of the underliying physical service used.
+ Otherwise the property is not present.
+
int32 Priority [readwrite]
An optional value used to calculate the priority order
diff --git a/link_monitor_unittest.cc b/link_monitor_unittest.cc
index 8c5f249..0bfae85 100644
--- a/link_monitor_unittest.cc
+++ b/link_monitor_unittest.cc
@@ -111,6 +111,8 @@
EXPECT_CALL(*connection_, local()).WillRepeatedly(ReturnRef(local_ip_));
EXPECT_TRUE(gateway_ip_.SetAddressFromString(kRemoteIPAddress));
EXPECT_CALL(*connection_, gateway()).WillRepeatedly(ReturnRef(gateway_ip_));
+ EXPECT_CALL(*connection_, technology())
+ .WillRepeatedly(Return(Technology::kEthernet));
}
virtual void TearDown() {
diff --git a/mock_connection.h b/mock_connection.h
index 7f13fad..ffbd0e0 100644
--- a/mock_connection.h
+++ b/mock_connection.h
@@ -29,7 +29,9 @@
MOCK_METHOD1(RequestHostRoute, bool(const IPAddress &destination));
MOCK_CONST_METHOD0(local, const IPAddress &());
MOCK_CONST_METHOD0(gateway, const IPAddress &());
+ MOCK_CONST_METHOD0(technology, Technology::Identifier());
MOCK_METHOD0(CreateGatewayRoute, bool());
+ MOCK_METHOD0(GetCarrierConnection, ConnectionRefPtr());
private:
DISALLOW_COPY_AND_ASSIGN(MockConnection);
diff --git a/vpn_service.cc b/vpn_service.cc
index fcf3aa0..15dde68 100644
--- a/vpn_service.cc
+++ b/vpn_service.cc
@@ -13,6 +13,7 @@
#include "shill/logging.h"
#include "shill/manager.h"
#include "shill/profile.h"
+#include "shill/property_accessor.h"
#include "shill/technology.h"
#include "shill/vpn_driver.h"
#include "shill/vpn_provider.h"
@@ -38,6 +39,13 @@
SetConnectable(true);
set_save_credentials(false);
mutable_store()->RegisterString(flimflam::kVPNDomainProperty, &vpn_domain_);
+ mutable_store()->RegisterDerivedString(
+ kPhysicalTechnologyProperty,
+ StringAccessor(
+ new CustomAccessor<VPNService, string>(
+ this,
+ &VPNService::GetPhysicalTechologyProperty,
+ NULL)));
}
VPNService::~VPNService() {}
@@ -192,4 +200,19 @@
return true;
}
+string VPNService::GetPhysicalTechologyProperty(Error *error) {
+ ConnectionRefPtr conn = connection();
+ if (conn)
+ conn = conn->GetCarrierConnection();
+
+ if (!conn) {
+ Error::PopulateAndLog(error,
+ Error::kOperationFailed,
+ Error::GetDefaultMessage(Error::kOperationFailed));
+ return "";
+ }
+
+ return Technology::NameFromIdentifier(conn->technology());
+}
+
} // namespace shill
diff --git a/vpn_service.h b/vpn_service.h
index 9b37cb4..76af372 100644
--- a/vpn_service.h
+++ b/vpn_service.h
@@ -52,12 +52,18 @@
friend class VPNServiceTest;
FRIEND_TEST(VPNServiceTest, GetDeviceRpcId);
FRIEND_TEST(VPNServiceTest, SetConnection);
+ FRIEND_TEST(VPNServiceTest, GetPhysicalTechologyPropertyFailsIfNoCarrier);
+ FRIEND_TEST(VPNServiceTest, GetPhysicalTechologyPropertyOverWifi);
static const char kAutoConnNeverConnected[];
static const char kAutoConnVPNAlreadyActive[];
virtual std::string GetDeviceRpcId(Error *error);
+ // Returns the Type name of the lowest connection (presumably the "physical"
+ // connection) that this service depends on.
+ std::string GetPhysicalTechologyProperty(Error *error);
+
std::string storage_id_;
scoped_ptr<VPNDriver> driver_;
scoped_ptr<Connection::Binder> connection_binder_;
diff --git a/vpn_service_unittest.cc b/vpn_service_unittest.cc
index 0517d51..7d15437 100644
--- a/vpn_service_unittest.cc
+++ b/vpn_service_unittest.cc
@@ -24,6 +24,7 @@
using std::string;
using testing::_;
+using testing::Mock;
using testing::NiceMock;
using testing::Return;
using testing::ReturnRef;
@@ -343,4 +344,51 @@
TestCustomSetterNoopChange(service_, &manager_);
}
+TEST_F(VPNServiceTest, GetPhysicalTechologyPropertyFailsIfNoCarrier) {
+ scoped_refptr<Connection> null_connection;
+
+ EXPECT_CALL(*sockets_, Socket(_, _, _)).WillOnce(Return(-1));
+ service_->SetConnection(connection_);
+ EXPECT_EQ(connection_.get(), service_->connection().get());
+
+ // Simulate an error in the GetCarrierConnection() returning a NULL reference.
+ EXPECT_CALL(*connection_, GetCarrierConnection())
+ .WillOnce(Return(null_connection));
+
+ Error error;
+ EXPECT_EQ("", service_->GetPhysicalTechologyProperty(&error));
+ EXPECT_EQ(Error::kOperationFailed, error.type());
+}
+
+TEST_F(VPNServiceTest, GetPhysicalTechologyPropertyOverWifi) {
+ scoped_refptr<NiceMock<MockConnection>> lower_connection_ =
+ new NiceMock<MockConnection>(&device_info_);
+
+ EXPECT_CALL(*connection_, technology())
+ .Times(0);
+ EXPECT_CALL(*connection_, GetCarrierConnection())
+ .WillOnce(Return(lower_connection_));
+
+ EXPECT_CALL(*sockets_, Socket(_, _, _)).WillOnce(Return(-1));
+ service_->SetConnection(connection_);
+ EXPECT_EQ(connection_.get(), service_->connection().get());
+
+ // Set the type of the lower connection to "wifi" and expect that type to be
+ // returned by GetPhysical TechnologyProperty().
+ EXPECT_CALL(*lower_connection_, technology())
+ .WillOnce(Return(Technology::kWifi));
+
+ Error error;
+ EXPECT_EQ(flimflam::kTypeWifi,
+ service_->GetPhysicalTechologyProperty(&error));
+ EXPECT_TRUE(error.IsSuccess());
+
+ // Clear expectations now, so the Return(lower_connection_) action releases
+ // the reference to |lower_connection_| allowing it to be destroyed now.
+ Mock::VerifyAndClearExpectations(connection_);
+ // Destroying the |lower_connection_| at function exit will also call an extra
+ // FlushAddresses on the |device_info_| object.
+ EXPECT_CALL(device_info_, FlushAddresses(0));
+}
+
} // namespace shill