Support more than one clatd at a time.

1. Make Nat464Xlat a per-network object, one for every network
   requiring clat, instead of a ConnectivityService singleton.
2. Make the NetworkManagementService clatd commands take an
   interface.
3. When we attempt to start clatd on a network, store its
   Nat464Xlat object in the NetworkAgentInfo, so we have an
   authoritative way of knowing whether clat is running on a
   given network.
4. Rework Nat464Xlat, hopefully simplifying it.

Bug: 12111730
Change-Id: I1fa5508ef020cd1c3d1c7a1f7b06370ac5fc2ae2

5 files changed, 170 insertions, 134 deletions

diff --git a/core/java/android/os/INetworkManagementService.aidl b/core/java/android/os/INetworkManagementService.aidl
index 16250c7..d0ba4b8 100644
--- a/core/java/android/os/INetworkManagementService.aidl
+++ b/core/java/android/os/INetworkManagementService.aidl
@@ -336,19 +336,19 @@ interface INetworkManagementService
     void removeVpnUidRanges(int netId, in UidRange[] ranges);
 
     /**
-     * Start the clatd (464xlat) service
+     * Start the clatd (464xlat) service on the given interface.
      */
     void startClatd(String interfaceName);
 
     /**
-     * Stop the clatd (464xlat) service
+     * Stop the clatd (464xlat) service on the given interface.
      */
-    void stopClatd();
+    void stopClatd(String interfaceName);
 
     /**
-     * Determine whether the clatd (464xlat) service has been started
+     * Determine whether the clatd (464xlat) service has been started on the given interface.
      */
-    boolean isClatdStarted();
+    boolean isClatdStarted(String interfaceName);
 
     /**
      * Start listening for mobile activity state changes.
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 6a6dcaf..bf67461 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -238,8 +238,6 @@ public class ConnectivityService extends IConnectivityManager.Stub {
     private boolean mLockdownEnabled;
     private LockdownVpnTracker mLockdownTracker;
 
-    private Nat464Xlat mClat;
-
     /** Lock around {@link #mUidRules} and {@link #mMeteredIfaces}. */
     private Object mRulesLock = new Object();
     /** Currently active network rules by UID. */
@@ -715,12 +713,10 @@ public class ConnectivityService extends IConnectivityManager.Stub {
         intentFilter.addAction(Intent.ACTION_USER_STOPPING);
         mContext.registerReceiverAsUser(
                 mUserIntentReceiver, UserHandle.ALL, intentFilter, null, null);
-        mClat = new Nat464Xlat(mContext, mNetd, this, mTrackerHandler);
 
         try {
             mNetd.registerObserver(mTethering);
             mNetd.registerObserver(mDataActivityObserver);
-            mNetd.registerObserver(mClat);
         } catch (RemoteException e) {
             loge("Error registering observer :" + e);
         }
@@ -3549,7 +3545,9 @@ public class ConnectivityService extends IConnectivityManager.Stub {
 
         // 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);
+        if (networkAgent.clatd != null) {
+            networkAgent.clatd.fixupLinkProperties(oldLp);
+        }
 
         updateInterfaces(newLp, oldLp, netId);
         updateMtu(newLp, oldLp);
@@ -3568,15 +3566,15 @@ public class ConnectivityService extends IConnectivityManager.Stub {
         }
     }
 
-    private void updateClat(LinkProperties newLp, LinkProperties oldLp, NetworkAgentInfo na) {
-        final boolean wasRunningClat = mClat.isRunningClat(na);
-        final boolean shouldRunClat = Nat464Xlat.requiresClat(na);
+    private void updateClat(LinkProperties newLp, LinkProperties oldLp, NetworkAgentInfo nai) {
+        final boolean wasRunningClat = nai.clatd != null && nai.clatd.isStarted();
+        final boolean shouldRunClat = Nat464Xlat.requiresClat(nai);
 
         if (!wasRunningClat && shouldRunClat) {
-            // Start clatd. If it's already been started but is not running yet, this is a no-op.
-            mClat.startClat(na);
+            nai.clatd = new Nat464Xlat(mContext, mNetd, mTrackerHandler, nai);
+            nai.clatd.start();
         } else if (wasRunningClat && !shouldRunClat) {
-            mClat.stopClat();
+            nai.clatd.stop();
         }
     }
 
diff --git a/services/core/java/com/android/server/NetworkManagementService.java b/services/core/java/com/android/server/NetworkManagementService.java
index 020c951..d03a154 100644
--- a/services/core/java/com/android/server/NetworkManagementService.java
+++ b/services/core/java/com/android/server/NetworkManagementService.java
@@ -1854,23 +1854,23 @@ public class NetworkManagementService extends INetworkManagementService.Stub
     }
 
     @Override
-    public void stopClatd() throws IllegalStateException {
+    public void stopClatd(String interfaceName) throws IllegalStateException {
         mContext.enforceCallingOrSelfPermission(CONNECTIVITY_INTERNAL, TAG);
 
         try {
-            mConnector.execute("clatd", "stop");
+            mConnector.execute("clatd", "stop", interfaceName);
         } catch (NativeDaemonConnectorException e) {
             throw e.rethrowAsParcelableException();
         }
     }
 
     @Override
-    public boolean isClatdStarted() {
+    public boolean isClatdStarted(String interfaceName) {
         mContext.enforceCallingOrSelfPermission(CONNECTIVITY_INTERNAL, TAG);
 
         final NativeDaemonEvent event;
         try {
-            event = mConnector.execute("clatd", "status");
+            event = mConnector.execute("clatd", "status", interfaceName);
         } catch (NativeDaemonConnectorException e) {
             throw e.rethrowAsParcelableException();
         }
diff --git a/services/core/java/com/android/server/connectivity/Nat464Xlat.java b/services/core/java/com/android/server/connectivity/Nat464Xlat.java
index 3b0d8c1..c7a2ce1 100644
--- a/services/core/java/com/android/server/connectivity/Nat464Xlat.java
+++ b/services/core/java/com/android/server/connectivity/Nat464Xlat.java
@@ -43,45 +43,41 @@ import com.android.server.net.BaseNetworkObserver;
  * Class to manage a 464xlat CLAT daemon.
  */
 public class Nat464Xlat extends BaseNetworkObserver {
-    private Context mContext;
-    private INetworkManagementService mNMService;
-    private IConnectivityManager mConnService;
-    // Whether we started clatd and expect it to be running.
-    private boolean mIsStarted;
-    // Whether the clatd interface exists (i.e., clatd is running).
-    private boolean mIsRunning;
-    // The LinkProperties of the clat interface.
-    private LinkProperties mLP;
-    // Current LinkProperties of the network.  Includes mLP as a stacked link when clat is active.
-    private LinkProperties mBaseLP;
+    private static final String TAG = "Nat464Xlat";
+
+    // This must match the interface prefix in clatd.c.
+    private static final String CLAT_PREFIX = "v4-";
+
+    private final INetworkManagementService mNMService;
+
     // ConnectivityService Handler for LinkProperties updates.
-    private Handler mHandler;
-    // Marker to connote which network we're augmenting.
-    private Messenger mNetworkMessenger;
+    private final Handler mHandler;
 
-    // This must match the interface name in clatd.conf.
-    private static final String CLAT_INTERFACE_NAME = "clat4";
+    // The network we're running on.
+    private final NetworkAgentInfo mNetwork;
 
-    private static final String TAG = "Nat464Xlat";
+    // Internal state variables.
+    //
+    // The possible states are:
+    //  - Idle: start() not called. Everything is null.
+    //  - Starting: start() called. Interfaces are non-null. isStarted() returns true.
+    //    mIsRunning is false.
+    //  - Running: start() called, and interfaceAdded() told us that mIface is up. Clat IP address
+    //    is non-null. mIsRunning is true.
+    //
+    // Once mIface is non-null and isStarted() is true, methods called by ConnectivityService on
+    // its handler thread must not modify any internal state variables; they are only updated by the
+    // interface observers, called on the notification threads.
+    private String mBaseIface;
+    private String mIface;
+    private boolean mIsRunning;
 
-    public Nat464Xlat(Context context, INetworkManagementService nmService,
-                      IConnectivityManager connService, Handler handler) {
-        mContext = context;
+    public Nat464Xlat(
+            Context context, INetworkManagementService nmService,
+            Handler handler, NetworkAgentInfo nai) {
         mNMService = nmService;
-        mConnService = connService;
         mHandler = handler;
-
-        mIsStarted = false;
-        mIsRunning = false;
-        mLP = new LinkProperties();
-
-        // If this is a runtime restart, it's possible that clatd is already
-        // running, but we don't know about it. If so, stop it.
-        try {
-            if (mNMService.isClatdStarted()) {
-                mNMService.stopClatd();
-            }
-        } catch(RemoteException e) {}  // Well, we tried.
+        mNetwork = nai;
     }
 
     /**
@@ -94,137 +90,176 @@ public class Nat464Xlat extends BaseNetworkObserver {
         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.
         return netType == TYPE_MOBILE && connected && !hasIPv4Address;
     }
 
-    public boolean isRunningClat(NetworkAgentInfo network) {
-        return mNetworkMessenger == network.messenger;
+    /**
+     * Determines whether clatd is started. Always true, except a) if start has not yet been called,
+     * or b) if our interface was removed.
+     */
+    public boolean isStarted() {
+        return mIface != null;
+    }
+
+    /**
+     * Clears internal state. Must not be called by ConnectivityService.
+     */
+    private void clear() {
+        mIface = null;
+        mBaseIface = null;
+        mIsRunning = false;
     }
 
     /**
-     * Starts the clat daemon.
-     * @param lp The link properties of the interface to start clatd on.
+     * Starts the clat daemon. Called by ConnectivityService on the handler thread.
      */
-    public void startClat(NetworkAgentInfo network) {
-        if (mNetworkMessenger != null && mNetworkMessenger != network.messenger) {
-            Slog.e(TAG, "startClat: too many networks requesting clat");
+    public void start() {
+        if (isStarted()) {
+            Slog.e(TAG, "startClat: already started");
             return;
         }
-        mNetworkMessenger = network.messenger;
-        LinkProperties lp = network.linkProperties;
-        mBaseLP = new LinkProperties(lp);
-        if (mIsStarted) {
-            Slog.e(TAG, "startClat: already started");
+
+        if (mNetwork.linkProperties == null) {
+            Slog.e(TAG, "startClat: Can't start clat with null LinkProperties");
             return;
         }
-        String iface = lp.getInterfaceName();
-        Slog.i(TAG, "Starting clatd on " + iface + ", lp=" + lp);
+
         try {
-            mNMService.startClatd(iface);
+            mNMService.registerObserver(this);
         } catch(RemoteException e) {
-            Slog.e(TAG, "Error starting clat daemon: " + e);
+            Slog.e(TAG, "startClat: Can't register interface observer for clat on " + mNetwork);
+            return;
+        }
+
+        mBaseIface = mNetwork.linkProperties.getInterfaceName();
+        if (mBaseIface == null) {
+            Slog.e(TAG, "startClat: Can't start clat on null interface");
+            return;
+        }
+        mIface = CLAT_PREFIX + mBaseIface;
+        // From now on, isStarted() will return true.
+
+        Slog.i(TAG, "Starting clatd on " + mBaseIface);
+        try {
+            mNMService.startClatd(mBaseIface);
+        } catch(RemoteException|IllegalStateException e) {
+            Slog.e(TAG, "Error starting clatd: " + e);
         }
-        mIsStarted = true;
     }
 
     /**
-     * Stops the clat daemon.
+     * Stops the clat daemon. Called by ConnectivityService on the handler thread.
      */
-    public void stopClat() {
-        if (mIsStarted) {
+    public void stop() {
+        if (isStarted()) {
             Slog.i(TAG, "Stopping clatd");
             try {
-                mNMService.stopClatd();
-            } catch(RemoteException e) {
-                Slog.e(TAG, "Error stopping clat daemon: " + e);
+                mNMService.stopClatd(mBaseIface);
+            } catch(RemoteException|IllegalStateException e) {
+                Slog.e(TAG, "Error stopping clatd: " + e);
             }
-            mIsStarted = false;
-            mIsRunning = false;
-            mNetworkMessenger = null;
-            mBaseLP = null;
-            mLP.clear();
+            // When clatd stops and its interface is deleted, interfaceRemoved() will notify
+            // ConnectivityService and call clear().
         } else {
-            Slog.e(TAG, "stopClat: already stopped");
+            Slog.e(TAG, "clatd: already stopped");
         }
     }
 
-    private void updateConnectivityService() {
-        Message msg = mHandler.obtainMessage(
-            NetworkAgent.EVENT_NETWORK_PROPERTIES_CHANGED, mBaseLP);
-        msg.replyTo = mNetworkMessenger;
+    private void updateConnectivityService(LinkProperties lp) {
+        Message msg = mHandler.obtainMessage(NetworkAgent.EVENT_NETWORK_PROPERTIES_CHANGED, lp);
+        msg.replyTo = mNetwork.messenger;
         Slog.i(TAG, "sending message to ConnectivityService: " + msg);
         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());
+    /**
+     * Copies the stacked clat link in oldLp, if any, to the LinkProperties in mNetwork.
+     * This is necessary because the LinkProperties in mNetwork come from the transport layer, which
+     * has no idea that 464xlat is running on top of it.
+     */
+    public void fixupLinkProperties(LinkProperties oldLp) {
+        if (mNetwork.clatd != null &&
+                mIsRunning &&
+                mNetwork.linkProperties != null &&
+                !mNetwork.linkProperties.getAllInterfaceNames().contains(mIface)) {
+            Slog.d(TAG, "clatd running, updating NAI for " + mIface);
             for (LinkProperties stacked: oldLp.getStackedLinks()) {
-                if (CLAT_INTERFACE_NAME.equals(stacked.getInterfaceName())) {
-                    nai.linkProperties.addStackedLink(stacked);
+                if (mIface.equals(stacked.getInterfaceName())) {
+                    mNetwork.linkProperties.addStackedLink(stacked);
                     break;
                 }
             }
         }
     }
 
+    private LinkProperties makeLinkProperties(LinkAddress clatAddress) {
+        LinkProperties stacked = new LinkProperties();
+        stacked.setInterfaceName(mIface);
+
+        // 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(), mIface);
+        stacked.addRoute(ipv4Default);
+        stacked.addLinkAddress(clatAddress);
+        return stacked;
+    }
+
     @Override
     public void interfaceAdded(String iface) {
-        if (iface.equals(CLAT_INTERFACE_NAME)) {
-            Slog.i(TAG, "interface " + CLAT_INTERFACE_NAME +
-                   " added, mIsRunning = " + mIsRunning + " -> true");
-            mIsRunning = true;
-
-            // 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.
+        // Called by the InterfaceObserver on its own thread, so can race with stop().
+        if (isStarted() && mIface.equals(iface)) {
+            Slog.i(TAG, "interface " + iface + " added, mIsRunning " + mIsRunning + "->true");
+
+            LinkAddress clatAddress;
             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);
-                mLP.addLinkAddress(clatAddress);
-                mBaseLP.addStackedLink(mLP);
-                Slog.i(TAG, "Adding stacked link. tracker LP: " + mBaseLP);
-                updateConnectivityService();
+                clatAddress = config.getLinkAddress();
             } catch(RemoteException e) {
                 Slog.e(TAG, "Error getting link properties: " + e);
+                return;
+            }
+
+            if (!mIsRunning) {
+                mIsRunning = true;
+                LinkProperties lp = new LinkProperties(mNetwork.linkProperties);
+                lp.addStackedLink(makeLinkProperties(clatAddress));
+                Slog.i(TAG, "Adding stacked link " + mIface + " on top of " + mBaseIface);
+                updateConnectivityService(lp);
             }
         }
     }
 
     @Override
     public void interfaceRemoved(String iface) {
-        if (iface == CLAT_INTERFACE_NAME) {
+        if (isStarted() && mIface.equals(iface)) {
+            Slog.i(TAG, "interface " + iface + " removed, mIsRunning " + mIsRunning + "->false");
+
             if (mIsRunning) {
-                NetworkUtils.resetConnections(
-                    CLAT_INTERFACE_NAME,
-                    NetworkUtils.RESET_IPV4_ADDRESSES);
-                mBaseLP.removeStackedLink(CLAT_INTERFACE_NAME);
-                updateConnectivityService();
+                // The interface going away likely means clatd has crashed. Ask netd to stop it,
+                // because otherwise when we try to start it again on the same base interface netd
+                // will complain that it's already started.
+                //
+                // Note that this method can be called by the interface observer at the same time
+                // that ConnectivityService calls stop(). In this case, the second call to
+                // stopClatd() will just throw IllegalStateException, which we'll ignore.
+                try {
+                    mNMService.unregisterObserver(this);
+                    mNMService.stopClatd(mBaseIface);
+                } catch (RemoteException|IllegalStateException e) {
+                    // Well, we tried.
+                }
+                LinkProperties lp = new LinkProperties(mNetwork.linkProperties);
+                lp.removeStackedLink(mIface);
+                clear();
+                updateConnectivityService(lp);
             }
-            Slog.i(TAG, "interface " + CLAT_INTERFACE_NAME +
-                   " removed, mIsRunning = " + mIsRunning + " -> false");
-            mIsRunning = false;
-            mLP.clear();
-            Slog.i(TAG, "mLP = " + mLP);
         }
     }
-};
+}
diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java
index 15ffc0d..4cf2a4a 100644
--- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java
+++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java
@@ -63,6 +63,9 @@ public class NetworkAgentInfo {
     public final Messenger messenger;
     public final AsyncChannel asyncChannel;
 
+    // Used by ConnectivityService to keep track of 464xlat.
+    public Nat464Xlat clatd;
+
     public NetworkAgentInfo(Messenger messenger, AsyncChannel ac, NetworkInfo info,
             LinkProperties lp, NetworkCapabilities nc, int score, Context context, Handler handler,
             NetworkMisc misc) {