shill: add IPv6Disabled device property
Add a readwrite Device property IPv6Disabled to disable and
reenable IPv6 on a device without restarting shill or disrupting
IPv4. The force disabling of IPv6 on cellular devices is also
migrated into using this property.
BUG=b:160374783
TEST=unit, manual on atlas through dbus-send
dbus-send --system --dest=org.chromium.flimflam --print-reply
/device/eth0 org.chromium.flimflam.Device.SetProperty
string:IPv6Disabled boolean:false
Change-Id: Ia30854d7637aab84d0cbbb3da2b0a79281421a71
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2279395
Tested-by: Taoyu Li <taoyl@chromium.org>
Commit-Queue: Alex Khouderchah <akhouderchah@chromium.org>
Reviewed-by: Hugo Benichi <hugobenichi@google.com>
Reviewed-by: Alex Khouderchah <akhouderchah@chromium.org>
diff --git a/shill/cellular/cellular.cc b/shill/cellular/cellular.cc
index 2c403a6..306e046 100644
--- a/shill/cellular/cellular.cc
+++ b/shill/cellular/cellular.cc
@@ -165,6 +165,9 @@
weak_ptr_factory_(this) {
RegisterProperties();
+ Error error;
+ SetIPv6Disabled(IsIPv6DisabledByDefault(), &error);
+
// TODO(pprabhu) Split MobileOperatorInfo into a context that stores the
// costly database, and lighter objects that |Cellular| can own.
// crbug.com/363874
@@ -363,7 +366,7 @@
// this device using the old IP, we need to make sure we prevent further
// packets from going out.
if (manager() && manager()->device_info() && socket_destroyer_) {
- DisableIPv6();
+ StopIPv6();
for (const auto& address :
manager()->device_info()->GetAddresses(interface_index())) {
@@ -474,7 +477,7 @@
capability_->Reset(error, callback);
}
-bool Cellular::IsIPv6Allowed() const {
+bool Cellular::IsIPv6DisabledByDefault() const {
// A cellular device is disabled before the system goes into suspend mode.
// However, outstanding TCP sockets may not be nuked when the associated
// network interface goes down. When the system resumes from suspend, the
@@ -485,7 +488,7 @@
// the same modem within a connection session and may drop the connection.
// Here we disable IPv6 support on cellular devices to work around the issue.
//
- return false;
+ return true;
}
void Cellular::DropConnection() {
@@ -577,7 +580,7 @@
}
// Re-enable IPv6 so we can renegotiate an IP address.
- EnableIPv6();
+ StartIPv6();
// TODO(quiche): Consider if this should be conditional. If, e.g.,
// the device was still disabling when we suspended, will trying to
diff --git a/shill/cellular/cellular.h b/shill/cellular/cellular.h
index 15f40c2..d44763e 100644
--- a/shill/cellular/cellular.h
+++ b/shill/cellular/cellular.h
@@ -196,7 +196,7 @@
Error* error,
const ResultCallback& callback) override;
void Reset(Error* error, const ResultCallback& callback) override;
- bool IsIPv6Allowed() const override;
+ bool IsIPv6DisabledByDefault() const override;
void DropConnection() override;
void SetServiceState(Service::ConnectState state) override;
void SetServiceFailure(Service::ConnectFailure failure_state) override;
diff --git a/shill/device.cc b/shill/device.cc
index 1779bf2..50e1414 100644
--- a/shill/device.cc
+++ b/shill/device.cc
@@ -23,9 +23,9 @@
#include <base/files/file_util.h>
#include <base/memory/ref_counted.h>
#include <base/stl_util.h>
-#include <base/strings/stringprintf.h>
#include <base/strings/string_number_conversions.h>
#include <base/strings/string_util.h>
+#include <base/strings/stringprintf.h>
#include <chromeos/dbus/service_constants.h>
#include "shill/connection.h"
@@ -152,6 +152,7 @@
rtnl_handler_(RTNLHandler::GetInstance()),
time_(Time::GetInstance()),
last_link_monitor_failed_time_(0),
+ ipv6_disabled_(false),
is_loose_routing_(false),
is_multi_homed_(false),
fixed_ip_params_(false),
@@ -182,6 +183,8 @@
// kDBusObjectProperty: Register in Cellular
store_.RegisterConstString(kInterfaceProperty, &link_name_);
+ HelpRegisterDerivedBool(kIPv6DisabledProperty, &Device::GetIPv6Disabled,
+ &Device::SetIPv6Disabled, &Device::ClearIPv6Disabled);
HelpRegisterConstDerivedRpcIdentifier(
kSelectedServiceProperty, &Device::GetSelectedServiceRpcIdentifier);
HelpRegisterConstDerivedRpcIdentifiers(kIPConfigsProperty,
@@ -287,20 +290,40 @@
}
}
-bool Device::IsIPv6Allowed() const {
+bool Device::GetIPv6Disabled(Error* error) {
+ return ipv6_disabled_;
+}
+
+bool Device::SetIPv6Disabled(const bool& disabling, Error* /*error*/) {
+ if (disabling == ipv6_disabled_)
+ return false;
+ ipv6_disabled_ = disabling;
+ if (ipv6_disabled_) {
+ StopIPv6();
+ } else {
+ StartIPv6();
+ }
return true;
}
-void Device::DisableIPv6() {
+void Device::ClearIPv6Disabled(Error* error) {
+ SetIPv6Disabled(IsIPv6DisabledByDefault(), error);
+}
+
+bool Device::IsIPv6DisabledByDefault() const {
+ return false;
+}
+
+void Device::StopIPv6() {
SLOG(this, 2) << __func__;
SetIPFlag(IPAddress::kFamilyIPv6, kIPFlagDisableIPv6, "1");
}
-void Device::EnableIPv6() {
+void Device::StartIPv6() {
SLOG(this, 2) << __func__;
- if (!IsIPv6Allowed()) {
+ if (ipv6_disabled_) {
LOG(INFO) << "Skip enabling IPv6 on " << link_name_
- << " as it is not allowed.";
+ << " as it is disabled.";
return;
}
SetIPFlag(IPAddress::kFamilyIPv6, kIPFlagDisableIPv6, "0");
@@ -512,7 +535,7 @@
}
void Device::DestroyIPConfig() {
- DisableIPv6();
+ StopIPv6();
bool ipconfig_changed = false;
if (ipconfig_) {
ipconfig_->ReleaseIP(IPConfig::kReleaseReasonDisconnect);
@@ -772,7 +795,7 @@
bool Device::AcquireIPConfigWithLeaseName(const string& lease_name) {
DestroyIPConfig();
- EnableIPv6();
+ StartIPv6();
bool arp_gateway = manager_->GetArpGateway() && ShouldUseArpGateway();
DHCPConfigRefPtr dhcp_config;
if (selected_service_) {
@@ -849,7 +872,7 @@
void Device::AssignIPConfig(const IPConfig::Properties& properties) {
DestroyIPConfig();
- EnableIPv6();
+ StartIPv6();
ipconfig_ = new IPConfig(control_interface(), link_name_);
ipconfig_->set_properties(properties);
dispatcher()->PostTask(FROM_HERE, Bind(&Device::OnIPConfigUpdated,
@@ -860,6 +883,15 @@
dhcp_provider_->DestroyLease(name);
}
+void Device::HelpRegisterDerivedBool(const string& name,
+ bool (Device::*get)(Error* error),
+ bool (Device::*set)(const bool&, Error*),
+ void (Device::*clear)(Error*)) {
+ store_.RegisterDerivedBool(
+ name,
+ BoolAccessor(new CustomAccessor<Device, bool>(this, get, set, clear)));
+}
+
void Device::HelpRegisterConstDerivedString(
const string& name, string (Device::*get)(Error* error)) {
store_.RegisterDerivedString(
diff --git a/shill/device.h b/shill/device.h
index 57a7f1a..aa7f086 100644
--- a/shill/device.h
+++ b/shill/device.h
@@ -124,13 +124,13 @@
virtual void Reset(Error* error, const ResultCallback& callback);
virtual void RefreshIPConfig(Error* error);
- // Returns true if IPv6 is allowed and should be enabled when the device
+ // Returns false if IPv6 is allowed and should be enabled when the device
// tries to acquire an IP configuration. The default implementation allows
// IPv6, which can be overridden by a derived class.
- virtual bool IsIPv6Allowed() const;
+ virtual bool IsIPv6DisabledByDefault() const;
- mockable void DisableIPv6();
- mockable void EnableIPv6();
+ void StopIPv6();
+ void StartIPv6();
mockable void EnableIPv6Privacy();
// Returns true if the selected service on the device (if any) is connected.
@@ -301,7 +301,8 @@
// Programs the NIC to wake on every packet of IP protocol type belonging to
// |packet_types|. |error| indicates the result of the operation.
virtual void AddWakeOnPacketOfTypes(
- const std::vector<std::string>& packet_types, Error* error);
+ const std::vector<std::string>& packet_types,
+ Error* error);
// Removes a rule previously programmed into the NIC to wake the system from
// suspend upon receiving packets from |ip_endpoint|. |error| indicates the
// result of the operation.
@@ -311,7 +312,8 @@
// protocol type belonging to |packet_types|.
// |error| indicates the result of the operation.
virtual void RemoveWakeOnPacketOfTypes(
- const std::vector<std::string>& packet_types, Error* error);
+ const std::vector<std::string>& packet_types,
+ Error* error);
// Removes all wake-on-packet rules programmed into the NIC. |error| indicates
// the result of the operation.
virtual void RemoveAllWakeOnPacketConnections(Error* error);
@@ -371,7 +373,6 @@
FRIEND_TEST(DeviceTest, DestroyIPConfig);
FRIEND_TEST(DeviceTest, DestroyIPConfigNULL);
FRIEND_TEST(DeviceTest, ConfigWithMinimumMTU);
- FRIEND_TEST(DeviceTest, EnableIPv6);
FRIEND_TEST(DeviceTest, GetProperties);
FRIEND_TEST(DeviceTest, IPConfigUpdatedFailureWithIPv6Config);
FRIEND_TEST(DeviceTest, IPConfigUpdatedFailureWithIPv6Connection);
@@ -393,6 +394,8 @@
FRIEND_TEST(DeviceTest, SetEnabledPersistent);
FRIEND_TEST(DeviceTest, ShouldUseArpGateway);
FRIEND_TEST(DeviceTest, Start);
+ FRIEND_TEST(DeviceTest, StartIPv6);
+ FRIEND_TEST(DeviceTest, StartIPv6Disabled);
FRIEND_TEST(DeviceTest, Stop);
FRIEND_TEST(DeviceTest, StopWithFixedIpParams);
FRIEND_TEST(DeviceTest, StopWithNetworkInterfaceDisabledAfterward);
@@ -522,6 +525,16 @@
// Indicates if the selected service is configured with a static IP address.
bool IsUsingStaticIP() const;
+ // RPC getter, setter, and clear method for the "IPv6Disabled" property.
+ bool GetIPv6Disabled(Error* error);
+ bool SetIPv6Disabled(const bool& connect, Error* error);
+ void ClearIPv6Disabled(Error* error);
+
+ void HelpRegisterDerivedBool(const std::string& name,
+ bool (Device::*get)(Error* error),
+ bool (Device::*set)(const bool& value,
+ Error* error),
+ void (Device::*clear)(Error* error));
void HelpRegisterConstDerivedString(const std::string& name,
std::string (Device::*get)(Error*));
@@ -861,6 +874,8 @@
std::unique_ptr<PortalDetector> connection_tester_;
+ bool ipv6_disabled_;
+
// Track whether packets from non-optimal routes will be accepted by this
// device. This is referred to as "loose mode" (see RFC3704).
bool is_loose_routing_;
diff --git a/shill/device_test.cc b/shill/device_test.cc
index e4085d0..f461d61 100644
--- a/shill/device_test.cc
+++ b/shill/device_test.cc
@@ -80,8 +80,6 @@
int interface_index,
Technology technology)
: Device(manager, link_name, address, interface_index, technology) {
- ON_CALL(*this, IsIPv6Allowed())
- .WillByDefault(Invoke(this, &TestDevice::DeviceIsIPv6Allowed));
ON_CALL(*this, SetIPFlag(_, _, _))
.WillByDefault(Invoke(this, &TestDevice::DeviceSetIPFlag));
ON_CALL(*this, IsTrafficMonitorEnabled())
@@ -107,7 +105,6 @@
DCHECK(error);
}
- MOCK_METHOD(bool, IsIPv6Allowed, (), (const, override));
MOCK_METHOD(bool, IsTrafficMonitorEnabled, (), (const, override));
MOCK_METHOD(bool,
ShouldBringNetworkInterfaceDownAfterDisabled,
@@ -128,8 +125,6 @@
(const PortalDetector::Result&, const PortalDetector::Result&),
(override));
- virtual bool DeviceIsIPv6Allowed() const { return Device::IsIPv6Allowed(); }
-
virtual bool DeviceIsTrafficMonitorEnabled() const {
return Device::IsTrafficMonitorEnabled();
}
@@ -465,7 +460,7 @@
device_->AcquireIPConfig();
}
-TEST_F(DeviceTest, EnableIPv6) {
+TEST_F(DeviceTest, StartIPv6) {
EXPECT_CALL(*device_,
SetIPFlag(IPAddress::kFamilyIPv6,
StrEq(Device::kIPFlagDisableIPv6), StrEq("0")))
@@ -475,13 +470,21 @@
SetIPFlag(IPAddress::kFamilyIPv6,
StrEq(Device::kIPFlagAcceptRouterAdvertisements), StrEq("2")))
.WillOnce(Return(true));
- device_->EnableIPv6();
+ device_->StartIPv6();
}
-TEST_F(DeviceTest, EnableIPv6NotAllowed) {
- EXPECT_CALL(*device_, IsIPv6Allowed()).WillOnce(Return(false));
+TEST_F(DeviceTest, StartIPv6Disabled) {
+ Error error;
+ EXPECT_CALL(*device_,
+ SetIPFlag(IPAddress::kFamilyIPv6,
+ StrEq(Device::kIPFlagDisableIPv6), StrEq("1")))
+ .WillOnce(Return(true));
+ device_->SetIPv6Disabled(true, &error);
+ Mock::VerifyAndClearExpectations(device_.get());
EXPECT_CALL(*device_, SetIPFlag(_, _, _)).Times(0);
- device_->EnableIPv6();
+ device_->StartIPv6();
+ Mock::VerifyAndClearExpectations(device_.get());
+ device_->SetIPv6Disabled(false, &error);
}
TEST_F(DeviceTest, MultiHomed) {
diff --git a/shill/doc/device-api.txt b/shill/doc/device-api.txt
index d4a1946..a7c1a61 100644
--- a/shill/doc/device-api.txt
+++ b/shill/doc/device-api.txt
@@ -542,6 +542,12 @@
these IPConfigs, a PropertyChanged signal will be
emitted for the Device's IPConfig property.
+ boolean IPv6Disabled [readwrite]
+
+ A switch to force disabling IPv6 on the device.
+ Disabling and enabling this switch has no impact on
+ IPv4 connectivity.
+
int32 LinkMonitorResponseTime [readonly]
Latest running average of the link monitor response
diff --git a/shill/mock_device.h b/shill/mock_device.h
index a766131..3ed68dd 100644
--- a/shill/mock_device.h
+++ b/shill/mock_device.h
@@ -45,8 +45,6 @@
MOCK_METHOD(void, Scan, (Error*, const std::string&), (override));
MOCK_METHOD(bool, Load, (const StoreInterface*), (override));
MOCK_METHOD(bool, Save, (StoreInterface*), (override));
- MOCK_METHOD(void, DisableIPv6, (), (override));
- MOCK_METHOD(void, EnableIPv6, (), (override));
MOCK_METHOD(void, EnableIPv6Privacy, (), (override));
MOCK_METHOD(void, SetLooseRouting, (bool), (override));
MOCK_METHOD(void, SetIsMultiHomed, (bool), (override));
diff --git a/system_api/dbus/shill/dbus-constants.h b/system_api/dbus/shill/dbus-constants.h
index c893dc2..9fe7a69 100644
--- a/system_api/dbus/shill/dbus-constants.h
+++ b/system_api/dbus/shill/dbus-constants.h
@@ -321,6 +321,7 @@
// Base Device property names.
const char kAddressProperty[] = "Address"; // Also used for IPConfig.
const char kIPConfigsProperty[] = "IPConfigs";
+const char kIPv6DisabledProperty[] = "IPv6Disabled";
const char kInterfaceProperty[] = "Interface"; // Network interface name.
const char kLinkMonitorResponseTimeProperty[] = "LinkMonitorResponseTime";
// kNameProperty: Defined above for Service. DEPRECATED (crbug.com/1011136).
