Only stop/start clatd if necessary.

Previously we would restart clatd on every LinkProperties
change, which now happens every time we switch radio technology
(e.g., LTE to HSPA). We also would not stop it if the link got
an IPv4 address.

Bug: 15024258
Bug: 17186694
Bug: 17569702
Change-Id: I65cfcd5e7acec8ea1a12392a59dabd668c58490f

2 files changed, 46 insertions, 40 deletions

diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index f579d6f62d7e..6e67cd8028ca 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -4243,13 +4243,17 @@ public class ConnectivityService extends IConnectivityManager.Stub {
         LinkProperties newLp = networkAgent.linkProperties;
         int netId = networkAgent.network.netId;
 
+        // The NetworkAgentInfo does not know whether clatd is running on its network or not. Before
+        // we do anything else, make sure its LinkProperties are accurate.
+        mClat.fixupLinkProperties(networkAgent, oldLp);
+
         updateInterfaces(newLp, oldLp, netId);
         updateMtu(newLp, oldLp);
-        updateTcpBufferSizes(networkAgent);
         // TODO - figure out what to do for clat
 //        for (LinkProperties lp : newLp.getStackedLinks()) {
 //            updateMtu(lp, null);
 //        }
+        updateTcpBufferSizes(networkAgent);
         final boolean flushDns = updateRoutes(newLp, oldLp, netId);
         updateDnses(newLp, oldLp, netId, flushDns);
         updateClat(newLp, oldLp, networkAgent);
@@ -4257,23 +4261,14 @@ public class ConnectivityService extends IConnectivityManager.Stub {
     }
 
     private void updateClat(LinkProperties newLp, LinkProperties oldLp, NetworkAgentInfo na) {
-        // Update 464xlat state.
-        if (mClat.requiresClat(na)) {
-
-            // If the connection was previously using clat, but is not using it now, stop the clat
-            // daemon. Normally, this happens automatically when the connection disconnects, but if
-            // the disconnect is not reported, or if the connection's LinkProperties changed for
-            // some other reason (e.g., handoff changes the IP addresses on the link), it would
-            // still be running. If it's not running, then stopping it is a no-op.
-            if (Nat464Xlat.isRunningClat(oldLp) && !Nat464Xlat.isRunningClat(newLp)) {
-                mClat.stopClat();
-            }
-            // If the link requires clat to be running, then start the daemon now.
-            if (na.networkInfo.isConnected()) {
-                mClat.startClat(na);
-            } else {
-                mClat.stopClat();
-            }
+        final boolean wasRunningClat = mClat.isRunningClat(na);
+        final boolean shouldRunClat = Nat464Xlat.requiresClat(na);
+
+        if (!wasRunningClat && shouldRunClat) {
+            // Start clatd. If it's already been started but is not running yet, this is a no-op.
+            mClat.startClat(na);
+        } else if (wasRunningClat && !shouldRunClat) {
+            mClat.stopClat();
         }
     }
 
diff --git a/services/core/java/com/android/server/connectivity/Nat464Xlat.java b/services/core/java/com/android/server/connectivity/Nat464Xlat.java
index 096ab66b42de..c382be07f6f1 100644
--- a/services/core/java/com/android/server/connectivity/Nat464Xlat.java
+++ b/services/core/java/com/android/server/connectivity/Nat464Xlat.java
@@ -89,17 +89,20 @@ public class Nat464Xlat extends BaseNetworkObserver {
      * @param network the NetworkAgentInfo corresponding to the network.
      * @return true if the network requires clat, false otherwise.
      */
-    public boolean requiresClat(NetworkAgentInfo network) {
-        int netType = network.networkInfo.getType();
-        LinkProperties lp = network.linkProperties;
+    public static boolean requiresClat(NetworkAgentInfo nai) {
+        final int netType = nai.networkInfo.getType();
+        final boolean connected = nai.networkInfo.isConnected();
+        final boolean hasIPv4Address =
+                (nai.linkProperties != null) ? nai.linkProperties.hasIPv4Address() : false;
+        Slog.d(TAG, "requiresClat: netType=" + netType +
+                    ", connected=" + connected +
+                    ", hasIPv4Address=" + hasIPv4Address);
         // Only support clat on mobile for now.
-        Slog.d(TAG, "requiresClat: netType=" + netType + ", hasIPv4Address=" +
-               lp.hasIPv4Address());
-        return netType == TYPE_MOBILE && !lp.hasIPv4Address();
+        return netType == TYPE_MOBILE && connected && !hasIPv4Address;
     }
 
-    public static boolean isRunningClat(LinkProperties lp) {
-      return lp != null && lp.getAllInterfaceNames().contains(CLAT_INTERFACE_NAME);
+    public boolean isRunningClat(NetworkAgentInfo network) {
+        return mNetworkMessenger == network.messenger;
     }
 
     /**
@@ -149,14 +152,6 @@ public class Nat464Xlat extends BaseNetworkObserver {
         }
     }
 
-    public boolean isStarted() {
-        return mIsStarted;
-    }
-
-    public boolean isRunning() {
-        return mIsRunning;
-    }
-
     private void updateConnectivityService() {
         Message msg = mHandler.obtainMessage(
             NetworkAgent.EVENT_NETWORK_PROPERTIES_CHANGED, mBaseLP);
@@ -165,6 +160,21 @@ public class Nat464Xlat extends BaseNetworkObserver {
         msg.sendToTarget();
     }
 
+    // Copies the stacked clat link in oldLp, if any, to the LinkProperties in nai.
+    public void fixupLinkProperties(NetworkAgentInfo nai, LinkProperties oldLp) {
+        if (isRunningClat(nai) &&
+                nai.linkProperties != null &&
+                !nai.linkProperties.getAllInterfaceNames().contains(CLAT_INTERFACE_NAME)) {
+            Slog.d(TAG, "clatd running, updating NAI for " + nai.linkProperties.getInterfaceName());
+            for (LinkProperties stacked: oldLp.getStackedLinks()) {
+                if (CLAT_INTERFACE_NAME.equals(stacked.getInterfaceName())) {
+                    nai.linkProperties.addStackedLink(stacked);
+                    break;
+                }
+            }
+        }
+    }
+
     @Override
     public void interfaceAdded(String iface) {
         if (iface.equals(CLAT_INTERFACE_NAME)) {
@@ -175,17 +185,18 @@ public class Nat464Xlat extends BaseNetworkObserver {
             // Create the LinkProperties for the clat interface by fetching the
             // IPv4 address for the interface and adding an IPv4 default route,
             // then stack the LinkProperties on top of the link it's running on.
-            // Although the clat interface is a point-to-point tunnel, we don't
-            // point the route directly at the interface because some apps don't
-            // understand routes without gateways (see, e.g., http://b/9597256
-            // http://b/9597516). Instead, set the next hop of the route to the
-            // clat IPv4 address itself (for those apps, it doesn't matter what
-            // the IP of the gateway is, only that there is one).
             try {
                 InterfaceConfiguration config = mNMService.getInterfaceConfig(iface);
                 LinkAddress clatAddress = config.getLinkAddress();
                 mLP.clear();
                 mLP.setInterfaceName(iface);
+
+                // Although the clat interface is a point-to-point tunnel, we don't
+                // point the route directly at the interface because some apps don't
+                // understand routes without gateways (see, e.g., http://b/9597256
+                // http://b/9597516). Instead, set the next hop of the route to the
+                // clat IPv4 address itself (for those apps, it doesn't matter what
+                // the IP of the gateway is, only that there is one).
                 RouteInfo ipv4Default = new RouteInfo(new LinkAddress(Inet4Address.ANY, 0),
                                                       clatAddress.getAddress(), iface);
                 mLP.addRoute(ipv4Default);