Merge "Refactor interface IP code into InterfaceController"

services/core/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachine.java

@@ -27,6 +27,7 @@ import android.net.LinkAddress;
 import android.net.LinkProperties;
 import android.net.NetworkUtils;
 import android.net.RouteInfo;
+import android.net.ip.InterfaceController;
 import android.net.ip.RouterAdvertisementDaemon;
 import android.net.ip.RouterAdvertisementDaemon.RaParams;
 import android.net.util.NetdService;
@@ -107,8 +108,10 @@ public class TetherInterfaceStateMachine extends StateMachine {
 
     private final SharedLog mLog;
     private final INetworkManagementService mNMService;
+    private final INetd mNetd;
     private final INetworkStatsService mStatsService;
     private final IControlsTethering mTetherController;
+    private final InterfaceController mInterfaceCtrl;
 
     private final String mIfaceName;
     private final int mInterfaceType;
@@ -136,8 +139,11 @@ public class TetherInterfaceStateMachine extends StateMachine {
         super(ifaceName, looper);
         mLog = log.forSubComponent(ifaceName);
         mNMService = nMService;
+        // TODO: This should be passed in for testability.
+        mNetd = NetdService.getInstance();
         mStatsService = statsService;
         mTetherController = tetherController;
+        mInterfaceCtrl = new InterfaceController(ifaceName, nMService, mNetd, mLog);
         mIfaceName = ifaceName;
         mInterfaceType = interfaceType;
         mLinkProperties = new LinkProperties();
@@ -179,6 +185,7 @@ public class TetherInterfaceStateMachine extends StateMachine {
 
     private void stopIPv4() { configureIPv4(false); }
 
+    // TODO: Refactor this in terms of calls to InterfaceController.
     private boolean configureIPv4(boolean enabled) {
         if (VDBG) Log.d(TAG, "configureIPv4(" + enabled + ")");
 
@@ -381,8 +388,8 @@ public class TetherInterfaceStateMachine extends StateMachine {
 
     private void configureLocalIPv6Dns(
             HashSet<Inet6Address> deprecatedDnses, HashSet<Inet6Address> newDnses) {
-        final INetd netd = NetdService.getInstance();
+        // TODO: Is this really necessary? Can we not fail earlier if INetd cannot be located?
-        if (netd == null) {
+        if (mNetd == null) {
             if (newDnses != null) newDnses.clear();
             mLog.e("No netd service instance available; not setting local IPv6 addresses");
             return;
@@ -391,11 +398,8 @@ public class TetherInterfaceStateMachine extends StateMachine {
         // [1] Remove deprecated local DNS IP addresses.
         if (!deprecatedDnses.isEmpty()) {
             for (Inet6Address dns : deprecatedDnses) {
-                final String dnsString = dns.getHostAddress();
+                if (!mInterfaceCtrl.removeAddress(dns, RFC7421_PREFIX_LENGTH)) {
-                try {
+                    mLog.e("Failed to remove local dns IP " + dns);
-                    netd.interfaceDelAddress(mIfaceName, dnsString, RFC7421_PREFIX_LENGTH);
-                } catch (ServiceSpecificException | RemoteException e) {
-                    mLog.e("Failed to remove local dns IP " + dnsString + ": " + e);
                 }
 
                 mLinkProperties.removeLinkAddress(new LinkAddress(dns, RFC7421_PREFIX_LENGTH));
@@ -410,11 +414,8 @@ public class TetherInterfaceStateMachine extends StateMachine {
             }
 
             for (Inet6Address dns : addedDnses) {
-                final String dnsString = dns.getHostAddress();
+                if (!mInterfaceCtrl.addAddress(dns, RFC7421_PREFIX_LENGTH)) {
-                try {
+                    mLog.e("Failed to add local dns IP " + dns);
-                    netd.interfaceAddAddress(mIfaceName, dnsString, RFC7421_PREFIX_LENGTH);
-                } catch (ServiceSpecificException | RemoteException e) {
-                    mLog.e("Failed to add local dns IP " + dnsString + ": " + e);
                     newDnses.remove(dns);
                 }
 
@@ -423,7 +424,7 @@ public class TetherInterfaceStateMachine extends StateMachine {
         }
 
         try {
-            netd.tetherApplyDnsInterfaces();
+            mNetd.tetherApplyDnsInterfaces();
         } catch (ServiceSpecificException | RemoteException e) {
             mLog.e("Failed to update local DNS caching server");
             if (newDnses != null) newDnses.clear();

services/net/java/android/net/ip/InterfaceController.java

@@ -0,0 +1,162 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.net.ip;
+
+import android.net.INetd;
+import android.net.InterfaceConfiguration;
+import android.net.LinkAddress;
+import android.net.util.NetdService;
+import android.net.util.SharedLog;
+import android.os.INetworkManagementService;
+import android.os.RemoteException;
+import android.os.ServiceSpecificException;
+import android.system.OsConstants;
+
+import java.net.InetAddress;
+
+
+/**
+ * Encapsulates the multiple IP configuration operations performed on an interface.
+ *
+ * TODO: refactor/eliminate the redundant ways to set and clear addresses.
+ *
+ * @hide
+ */
+public class InterfaceController {
+    private final static boolean DBG = false;
+
+    private final String mIfName;
+    private final INetworkManagementService mNMS;
+    private final INetd mNetd;
+    private final SharedLog mLog;
+
+    public InterfaceController(String ifname, INetworkManagementService nms, INetd netd,
+            SharedLog log) {
+        mIfName = ifname;
+        mNMS = nms;
+        mNetd = netd;
+        mLog = log;
+    }
+
+    public boolean setIPv4Address(LinkAddress address) {
+        final InterfaceConfiguration ifcg = new InterfaceConfiguration();
+        ifcg.setLinkAddress(address);
+        try {
+            mNMS.setInterfaceConfig(mIfName, ifcg);
+            if (DBG) mLog.log("IPv4 configuration succeeded");
+        } catch (IllegalStateException | RemoteException e) {
+            logError("IPv4 configuration failed: %s", e);
+            return false;
+        }
+        return true;
+    }
+
+    public boolean clearIPv4Address() {
+        try {
+            final InterfaceConfiguration ifcg = new InterfaceConfiguration();
+            ifcg.setLinkAddress(new LinkAddress("0.0.0.0/0"));
+            mNMS.setInterfaceConfig(mIfName, ifcg);
+        } catch (IllegalStateException | RemoteException e) {
+            logError("Failed to clear IPv4 address on interface %s: %s", mIfName, e);
+            return false;
+        }
+        return true;
+    }
+
+    public boolean enableIPv6() {
+        try {
+            mNMS.enableIpv6(mIfName);
+        } catch (IllegalStateException | RemoteException e) {
+            logError("enabling IPv6 failed: %s", e);
+            return false;
+        }
+        return true;
+    }
+
+    public boolean disableIPv6() {
+        try {
+            mNMS.disableIpv6(mIfName);
+        } catch (IllegalStateException | RemoteException e) {
+            logError("disabling IPv6 failed: %s", e);
+            return false;
+        }
+        return true;
+    }
+
+    public boolean setIPv6PrivacyExtensions(boolean enabled) {
+        try {
+            mNMS.setInterfaceIpv6PrivacyExtensions(mIfName, enabled);
+        } catch (IllegalStateException | RemoteException e) {
+            logError("error setting IPv6 privacy extensions: %s", e);
+            return false;
+        }
+        return true;
+    }
+
+    public boolean setIPv6AddrGenModeIfSupported(int mode) {
+        try {
+            mNMS.setIPv6AddrGenMode(mIfName, mode);
+        } catch (RemoteException e) {
+            logError("Unable to set IPv6 addrgen mode: %s", e);
+            return false;
+        } catch (ServiceSpecificException e) {
+            if (e.errorCode != OsConstants.EOPNOTSUPP) {
+                logError("Unable to set IPv6 addrgen mode: %s", e);
+                return false;
+            }
+        }
+        return true;
+    }
+
+    public boolean addAddress(LinkAddress addr) {
+        return addAddress(addr.getAddress(), addr.getPrefixLength());
+    }
+
+    public boolean addAddress(InetAddress ip, int prefixLen) {
+        try {
+            mNetd.interfaceAddAddress(mIfName, ip.getHostAddress(), prefixLen);
+        } catch (ServiceSpecificException | RemoteException e) {
+            logError("failed to add %s/%d: %s", ip, prefixLen, e);
+            return false;
+        }
+        return true;
+    }
+
+    public boolean removeAddress(InetAddress ip, int prefixLen) {
+        try {
+            mNetd.interfaceDelAddress(mIfName, ip.getHostAddress(), prefixLen);
+        } catch (ServiceSpecificException | RemoteException e) {
+            logError("failed to remove %s/%d: %s", ip, prefixLen, e);
+            return false;
+        }
+        return true;
+    }
+
+    public boolean clearAllAddresses() {
+        try {
+            mNMS.clearInterfaceAddresses(mIfName);
+        } catch (Exception e) {
+            logError("Failed to clear addresses: %s", e);
+            return false;
+        }
+        return true;
+    }
+
+    private void logError(String fmt, Object... args) {
+        mLog.e(String.format(fmt, args));
+    }
+}

services/net/java/android/net/ip/IpManager.java

@@ -22,7 +22,6 @@ import com.android.internal.util.WakeupMessage;
 import android.content.Context;
 import android.net.DhcpResults;
 import android.net.INetd;
-import android.net.InterfaceConfiguration;
 import android.net.IpPrefix;
 import android.net.LinkAddress;
 import android.net.LinkProperties.ProvisioningChange;
@@ -43,9 +42,7 @@ import android.os.INetworkManagementService;
 import android.os.Message;
 import android.os.RemoteException;
 import android.os.ServiceManager;
-import android.os.ServiceSpecificException;
 import android.os.SystemClock;
-import android.system.OsConstants;
 import android.text.TextUtils;
 import android.util.LocalLog;
 import android.util.Log;
@@ -568,7 +565,7 @@ public class IpManager extends StateMachine {
     private final LocalLog mConnectivityPacketLog;
     private final MessageHandlingLogger mMsgStateLogger;
     private final IpConnectivityLog mMetricsLog = new IpConnectivityLog();
-    private final INetd mNetd;
+    private final InterfaceController mInterfaceCtrl;
 
     private NetworkInterface mNetworkInterface;
 
@@ -612,12 +609,13 @@ public class IpManager extends StateMachine {
         mClatInterfaceName = CLAT_PREFIX + ifName;
         mCallback = new LoggingCallbackWrapper(callback);
         mNwService = nwService;
-        mNetd = netd;
 
         mLog = new SharedLog(MAX_LOG_RECORDS, mTag);
         mConnectivityPacketLog = new LocalLog(MAX_PACKET_RECORDS);
         mMsgStateLogger = new MessageHandlingLogger();
 
+        mInterfaceCtrl = new InterfaceController(mInterfaceName, mNwService, netd, mLog);
+
         mNetlinkTracker = new NetlinkTracker(
                 mInterfaceName,
                 new NetlinkTracker.Callback() {
@@ -1157,29 +1155,6 @@ public class IpManager extends StateMachine {
         return (delta != ProvisioningChange.LOST_PROVISIONING);
     }
 
-    private boolean setIPv4Address(LinkAddress address) {
-        final InterfaceConfiguration ifcg = new InterfaceConfiguration();
-        ifcg.setLinkAddress(address);
-        try {
-            mNwService.setInterfaceConfig(mInterfaceName, ifcg);
-            if (DBG) Log.d(mTag, "IPv4 configuration succeeded");
-        } catch (IllegalStateException | RemoteException e) {
-            logError("IPv4 configuration failed: %s", e);
-            return false;
-        }
-        return true;
-    }
-
-    private void clearIPv4Address() {
-        try {
-            final InterfaceConfiguration ifcg = new InterfaceConfiguration();
-            ifcg.setLinkAddress(new LinkAddress("0.0.0.0/0"));
-            mNwService.setInterfaceConfig(mInterfaceName, ifcg);
-        } catch (IllegalStateException | RemoteException e) {
-            logError("Failed to clear IPv4 address on interface %s: %s", mInterfaceName, e);
-        }
-    }
-
     private void handleIPv4Success(DhcpResults dhcpResults) {
         mDhcpResults = new DhcpResults(dhcpResults);
         final LinkProperties newLp = assembleLinkProperties();
@@ -1199,7 +1174,7 @@ public class IpManager extends StateMachine {
         // that could trigger a call to this function. If we missed handling
         // that message in StartedState for some reason we would still clear
         // any addresses upon entry to StoppedState.
-        clearIPv4Address();
+        mInterfaceCtrl.clearIPv4Address();
         mDhcpResults = null;
         if (DBG) { Log.d(mTag, "onNewDhcpResults(null)"); }
         mCallback.onNewDhcpResults(null);
@@ -1238,7 +1213,7 @@ public class IpManager extends StateMachine {
         // If we have a StaticIpConfiguration attempt to apply it and
         // handle the result accordingly.
         if (mConfiguration.mStaticIpConfig != null) {
-            if (setIPv4Address(mConfiguration.mStaticIpConfig.ipAddress)) {
+            if (mInterfaceCtrl.setIPv4Address(mConfiguration.mStaticIpConfig.ipAddress)) {
                 handleIPv4Success(new DhcpResults(mConfiguration.mStaticIpConfig));
             } else {
                 return false;
@@ -1253,46 +1228,16 @@ public class IpManager extends StateMachine {
         return true;
     }
 
-    private void setIPv6AddrGenModeIfSupported() throws RemoteException {
-        try {
-            mNwService.setIPv6AddrGenMode(mInterfaceName, mConfiguration.mIPv6AddrGenMode);
-        } catch (ServiceSpecificException e) {
-            if (e.errorCode != OsConstants.EOPNOTSUPP) {
-                logError("Unable to set IPv6 addrgen mode: %s", e);
-            }
-        }
-    }
-
     private boolean startIPv6() {
-        // Set privacy extensions.
+        return mInterfaceCtrl.setIPv6PrivacyExtensions(true) &&
-        try {
+               mInterfaceCtrl.setIPv6AddrGenModeIfSupported(mConfiguration.mIPv6AddrGenMode) &&
-            mNwService.setInterfaceIpv6PrivacyExtensions(mInterfaceName, true);
+               mInterfaceCtrl.enableIPv6();
-
-            setIPv6AddrGenModeIfSupported();
-            mNwService.enableIpv6(mInterfaceName);
-        } catch (IllegalStateException | RemoteException | ServiceSpecificException e) {
-            logError("Unable to change interface settings: %s", e);
-            return false;
-        }
-
-        return true;
     }
 
     private boolean applyInitialConfig(InitialConfiguration config) {
-        if (mNetd == null) {
-            logError("tried to add %s to %s but INetd was null", config, mInterfaceName);
-            return false;
-        }
-
         // TODO: also support specifying a static IPv4 configuration in InitialConfiguration.
         for (LinkAddress addr : findAll(config.ipAddresses, LinkAddress::isIPv6)) {
-            try {
+            if (!mInterfaceCtrl.addAddress(addr)) return false;
-                mNetd.interfaceAddAddress(
-                        mInterfaceName, addr.getAddress().getHostAddress(), addr.getPrefixLength());
-            } catch (ServiceSpecificException | RemoteException e) {
-                logError("failed to add %s to %s: %s", addr, mInterfaceName, e);
-                return false;
-            }
         }
 
         return true;
@@ -1329,17 +1274,8 @@ public class IpManager extends StateMachine {
         //     - we don't get IPv4 routes from netlink
         // so we neither react to nor need to wait for changes in either.
 
-        try {
+        mInterfaceCtrl.disableIPv6();
-            mNwService.disableIpv6(mInterfaceName);
+        mInterfaceCtrl.clearAllAddresses();
-        } catch (Exception e) {
-            logError("Failed to disable IPv6: %s", e);
-        }
-
-        try {
-            mNwService.clearInterfaceAddresses(mInterfaceName);
-        } catch (Exception e) {
-            logError("Failed to clear addresses: %s", e);
-        }
     }
 
     class StoppedState extends State {
@@ -1418,7 +1354,7 @@ public class IpManager extends StateMachine {
                     break;
 
                 case DhcpClient.CMD_CLEAR_LINKADDRESS:
-                    clearIPv4Address();
+                    mInterfaceCtrl.clearIPv4Address();
                     break;
 
                 case DhcpClient.CMD_ON_QUIT:
@@ -1674,12 +1610,12 @@ public class IpManager extends StateMachine {
                     break;
 
                 case DhcpClient.CMD_CLEAR_LINKADDRESS:
-                    clearIPv4Address();
+                    mInterfaceCtrl.clearIPv4Address();
                     break;
 
                 case DhcpClient.CMD_CONFIGURE_LINKADDRESS: {
                     final LinkAddress ipAddress = (LinkAddress) msg.obj;
-                    if (setIPv4Address(ipAddress)) {
+                    if (mInterfaceCtrl.setIPv4Address(ipAddress)) {
                         mDhcpClient.sendMessage(DhcpClient.EVENT_LINKADDRESS_CONFIGURED);
                     } else {
                         logError("Failed to set IPv4 address.");