Convert IControlsTethering from interface into callback

Additionally:
    - add updateLinkProperties() method to new callback
    - skeletally connect LinkProperties updates through
      to OffloadController

TODOs liberally sprinkled through out.

Test: as follows
    - built
    - flashed
    - booted
    - runtest frameworks-net passes
Bug: 29337859
Bug: 32163131
Change-Id: I631d17b26be153534551a1615931fc98b598b953

diff --git a/services/core/java/com/android/server/connectivity/Tethering.java b/services/core/java/com/android/server/connectivity/Tethering.java
index 08df271..606f68b 100644 (file)
--- a/services/core/java/com/android/server/connectivity/Tethering.java
+++ b/services/core/java/com/android/server/connectivity/Tethering.java
@@ -114,7 +114,7 @@ import java.util.concurrent.atomic.AtomicInteger;
  * This class holds much of the business logic to allow Android devices
  * to act as IP gateways via USB, BT, and WiFi interfaces.
  */
-public class Tethering extends BaseNetworkObserver implements IControlsTethering {
+public class Tethering extends BaseNetworkObserver {
 
     private final static String TAG = Tethering.class.getSimpleName();
     private final static boolean DBG = false;
@@ -170,6 +170,8 @@ public class Tethering extends BaseNetworkObserver implements IControlsTethering
     private final StateMachine mTetherMasterSM;
     private final OffloadController mOffloadController;
     private final UpstreamNetworkMonitor mUpstreamNetworkMonitor;
+    // TODO: Figure out how to merge this and other downstream-tracking objects
+    // into a single coherent structure.
     private final HashSet<TetherInterfaceStateMachine> mForwardedDownstreams;
     private final SimChangeListener mSimChange;
 
@@ -181,7 +183,7 @@ public class Tethering extends BaseNetworkObserver implements IControlsTethering
     private boolean mRndisEnabled;       // track the RNDIS function enabled state
     private boolean mUsbTetherRequested; // true if USB tethering should be started
                                          // when RNDIS is enabled
-    // True iff WiFi tethering should be started when soft AP is ready.
+    // True iff. WiFi tethering should be started when soft AP is ready.
     private boolean mWifiTetherRequested;
 
     public Tethering(Context context, INetworkManagementService nmService,
@@ -1102,6 +1104,7 @@ public class Tethering extends BaseNetworkObserver implements IControlsTethering
         static final int EVENT_UPSTREAM_CALLBACK                = BASE_MASTER + 5;
         // we treated the error and want now to clear it
         static final int CMD_CLEAR_ERROR                        = BASE_MASTER + 6;
+        static final int EVENT_IFACE_UPDATE_LINKPROPERTIES      = BASE_MASTER + 7;
 
         private State mInitialState;
         private State mTetherModeAliveState;
@@ -1169,6 +1172,9 @@ public class Tethering extends BaseNetworkObserver implements IControlsTethering
                         if (VDBG) Log.d(TAG, "Tether Mode unrequested by " + who);
                         handleInterfaceServingStateInactive(who);
                         break;
+                    case EVENT_IFACE_UPDATE_LINKPROPERTIES:
+                        // Silently ignore these for now.
+                        break;
                     default:
                         return NOT_HANDLED;
                 }
@@ -1335,6 +1341,7 @@ public class Tethering extends BaseNetworkObserver implements IControlsTethering
             if (mode == IControlsTethering.STATE_TETHERED) {
                 mForwardedDownstreams.add(who);
             } else {
+                mOffloadController.removeDownstreamInterface(who.interfaceName());
                 mForwardedDownstreams.remove(who);
             }
 
@@ -1359,6 +1366,7 @@ public class Tethering extends BaseNetworkObserver implements IControlsTethering
         private void handleInterfaceServingStateInactive(TetherInterfaceStateMachine who) {
             mNotifyList.remove(who);
             mIPv6TetheringCoordinator.removeActiveDownstream(who);
+            mOffloadController.removeDownstreamInterface(who.interfaceName());
             mForwardedDownstreams.remove(who);
 
             // If this is a Wi-Fi interface, tell WifiManager of any errors.
@@ -1462,6 +1470,15 @@ public class Tethering extends BaseNetworkObserver implements IControlsTethering
                         }
                         break;
                     }
+                    case EVENT_IFACE_UPDATE_LINKPROPERTIES: {
+                        final LinkProperties newLp = (LinkProperties) message.obj;
+                        if (message.arg1 == IControlsTethering.STATE_TETHERED) {
+                            mOffloadController.notifyDownstreamLinkProperties(newLp);
+                        } else {
+                            mOffloadController.removeDownstreamInterface(newLp.getInterfaceName());
+                        }
+                        break;
+                    }
                     case CMD_UPSTREAM_CHANGED:
                         updateUpstreamWanted();
                         if (!mUpstreamWanted) break;
@@ -1692,9 +1709,25 @@ public class Tethering extends BaseNetworkObserver implements IControlsTethering
         return false;
     }
 
-    @Override
-    public void notifyInterfaceStateChange(String iface, TetherInterfaceStateMachine who,
-                                           int state, int error) {
+    private IControlsTethering makeControlCallback(String ifname) {
+        return new IControlsTethering() {
+            @Override
+            public void updateInterfaceState(
+                    TetherInterfaceStateMachine who, int state, int lastError) {
+                notifyInterfaceStateChange(ifname, who, state, lastError);
+            }
+
+            @Override
+            public void updateLinkProperties(
+                    TetherInterfaceStateMachine who, LinkProperties newLp) {
+                notifyLinkPropertiesChanged(ifname, who, newLp);
+            }
+        };
+    }
+
+    // TODO: Move into TetherMasterSM.
+    private void notifyInterfaceStateChange(
+            String iface, TetherInterfaceStateMachine who, int state, int error) {
         synchronized (mPublicSync) {
             final TetherState tetherState = mTetherStates.get(iface);
             if (tetherState != null && tetherState.stateMachine.equals(who)) {
@@ -1740,6 +1773,24 @@ public class Tethering extends BaseNetworkObserver implements IControlsTethering
         sendTetherStateChangedBroadcast();
     }
 
+    private void notifyLinkPropertiesChanged(String iface, TetherInterfaceStateMachine who,
+                                             LinkProperties newLp) {
+        final int state;
+        synchronized (mPublicSync) {
+            final TetherState tetherState = mTetherStates.get(iface);
+            if (tetherState != null && tetherState.stateMachine.equals(who)) {
+                state = tetherState.lastState;
+            } else {
+                mLog.log("got notification from stale iface " + iface);
+                return;
+            }
+        }
+
+        mLog.log(String.format("OBSERVED LinkProperties update iface=%s state=%s", iface, state));
+        final int which = TetherMasterSM.EVENT_IFACE_UPDATE_LINKPROPERTIES;
+        mTetherMasterSM.sendMessage(which, state, 0, newLp);
+    }
+
     private void maybeTrackNewInterfaceLocked(final String iface) {
         // If we don't care about this type of interface, ignore.
         final int interfaceType = ifaceNameToType(iface);
@@ -1757,7 +1808,8 @@ public class Tethering extends BaseNetworkObserver implements IControlsTethering
         mLog.log("adding TetheringInterfaceStateMachine for: " + iface);
         final TetherState tetherState = new TetherState(
                 new TetherInterfaceStateMachine(
-                    iface, mLooper, interfaceType, mLog, mNMService, mStatsService, this,
+                    iface, mLooper, interfaceType, mLog, mNMService, mStatsService,
+                    makeControlCallback(iface),
                     new IPv6TetheringInterfaceServices(iface, mNMService, mLog)));
         mTetherStates.put(iface, tetherState);
         tetherState.stateMachine.start();
@@ -1769,7 +1821,7 @@ public class Tethering extends BaseNetworkObserver implements IControlsTethering
             mLog.log("attempting to remove unknown iface (" + iface + "), ignoring");
             return;
         }
-        tetherState.stateMachine.sendMessage(TetherInterfaceStateMachine.CMD_INTERFACE_DOWN);
+        tetherState.stateMachine.stop();
         mLog.log("removing TetheringInterfaceStateMachine for: " + iface);
         mTetherStates.remove(iface);
     }

diff --git a/services/core/java/com/android/server/connectivity/tethering/IControlsTethering.java b/services/core/java/com/android/server/connectivity/tethering/IControlsTethering.java
index c5c86bd..aaa63b1 100644 (file)
--- a/services/core/java/com/android/server/connectivity/tethering/IControlsTethering.java
+++ b/services/core/java/com/android/server/connectivity/tethering/IControlsTethering.java
@@ -16,25 +16,41 @@
 
 package com.android.server.connectivity.tethering;
 
+import android.net.LinkProperties;
+
 /**
  * @hide
  *
  * Interface with methods necessary to notify that a given interface is ready for tethering.
+ *
+ * Rename to something more representative, e.g. IpServingControlCallback.
+ *
+ * All methods MUST be called on the TetherMasterSM main Looper's thread.
  */
-public interface IControlsTethering {
-    public final int STATE_UNAVAILABLE = 0;
-    public final int STATE_AVAILABLE   = 1;
-    public final int STATE_TETHERED    = 2;
-    public final int STATE_LOCAL_ONLY  = 3;
+public class IControlsTethering {
+    public static final int STATE_UNAVAILABLE = 0;
+    public static final int STATE_AVAILABLE   = 1;
+    public static final int STATE_TETHERED    = 2;
+    public static final int STATE_LOCAL_ONLY  = 3;
 
     /**
-     * Notify that |who| has changed its tethering state.  This may be called from any thread.
+     * Notify that |who| has changed its tethering state.
+     *
+     * TODO: Remove the need for the |who| argument.
      *
-     * @param iface a network interface (e.g. "wlan0")
      * @param who corresponding instance of a TetherInterfaceStateMachine
      * @param state one of IControlsTethering.STATE_*
      * @param lastError one of ConnectivityManager.TETHER_ERROR_*
      */
-    void notifyInterfaceStateChange(String iface, TetherInterfaceStateMachine who,
-                                    int state, int lastError);
+    public void updateInterfaceState(TetherInterfaceStateMachine who, int state, int lastError) {}
+
+    /**
+     * Notify that |who| has new LinkProperties.
+     *
+     * TODO: Remove the need for the |who| argument.
+     *
+     * @param who corresponding instance of a TetherInterfaceStateMachine
+     * @param newLp the new LinkProperties to report
+     */
+    public void updateLinkProperties(TetherInterfaceStateMachine who, LinkProperties newLp) {}
 }

diff --git a/services/core/java/com/android/server/connectivity/tethering/OffloadController.java b/services/core/java/com/android/server/connectivity/tethering/OffloadController.java
index cb50e9f..3aca45f 100644 (file)
--- a/services/core/java/com/android/server/connectivity/tethering/OffloadController.java
+++ b/services/core/java/com/android/server/connectivity/tethering/OffloadController.java
@@ -105,7 +105,19 @@ public class OffloadController {
         pushUpstreamParameters();
     }
 
-    // TODO: public void addDownStream(...)
+    public void notifyDownstreamLinkProperties(LinkProperties lp) {
+        if (!started()) return;
+
+        // TODO: Cache LinkProperties on a per-ifname basis and compute the
+        // deltas, calling addDownstream()/removeDownstream() accordingly.
+    }
+
+    public void removeDownstreamInterface(String ifname) {
+        if (!started()) return;
+
+        // TODO: Check cache for LinkProperties of ifname and, if present,
+        // call removeDownstream() accordingly.
+    }
 
     private boolean isOffloadDisabled() {
         // Defaults to |false| if not present.

diff --git a/services/core/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachine.java b/services/core/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachine.java
index 4a1d405..82b9ca0 100644 (file)
--- a/services/core/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachine.java
+++ b/services/core/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachine.java
@@ -90,6 +90,7 @@ public class TetherInterfaceStateMachine extends StateMachine {
 
     private final String mIfaceName;
     private final int mInterfaceType;
+    private final LinkProperties mLinkProperties;
     private final IPv6TetheringInterfaceServices mIPv6TetherSvc;
 
     private int mLastError;
@@ -106,6 +107,8 @@ public class TetherInterfaceStateMachine extends StateMachine {
         mTetherController = tetherController;
         mIfaceName = ifaceName;
         mInterfaceType = interfaceType;
+        mLinkProperties = new LinkProperties();
+        mLinkProperties.setInterfaceName(mIfaceName);
         mIPv6TetherSvc = ipv6Svc;
         mLastError = ConnectivityManager.TETHER_ERROR_NO_ERROR;
 
@@ -127,6 +130,8 @@ public class TetherInterfaceStateMachine extends StateMachine {
 
     public int lastError() { return mLastError; }
 
+    public void stop() { sendMessage(CMD_INTERFACE_DOWN); }
+
     // configured when we start tethering and unconfig'd on error or conclusion
     private boolean configureIfaceIp(boolean enabled) {
         if (VDBG) Log.d(TAG, "configureIfaceIp(" + enabled + ")");
@@ -182,8 +187,12 @@ public class TetherInterfaceStateMachine extends StateMachine {
     }
 
     private void sendInterfaceState(int newInterfaceState) {
-        mTetherController.notifyInterfaceStateChange(
-                mIfaceName, TetherInterfaceStateMachine.this, newInterfaceState, mLastError);
+        mTetherController.updateInterfaceState(
+                TetherInterfaceStateMachine.this, newInterfaceState, mLastError);
+        // TODO: Populate mLinkProperties correctly, and send more sensible
+        // updates more frequently (not just here).
+        mTetherController.updateLinkProperties(
+                TetherInterfaceStateMachine.this, new LinkProperties(mLinkProperties));
     }
 
     class InitialState extends State {
@@ -195,7 +204,6 @@ public class TetherInterfaceStateMachine extends StateMachine {
         @Override
         public boolean processMessage(Message message) {
             maybeLogMessage(this, message.what);
-            boolean retValue = true;
             switch (message.what) {
                 case CMD_TETHER_REQUESTED:
                     mLastError = ConnectivityManager.TETHER_ERROR_NO_ERROR;
@@ -218,10 +226,9 @@ public class TetherInterfaceStateMachine extends StateMachine {
                             (LinkProperties) message.obj);
                     break;
                 default:
-                    retValue = false;
-                    break;
+                    return NOT_HANDLED;
             }
-            return retValue;
+            return HANDLED;
         }
     }
 

diff --git a/tests/net/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachineTest.java b/tests/net/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachineTest.java
index 27e683c..57c258f 100644 (file)
--- a/tests/net/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachineTest.java
+++ b/tests/net/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachineTest.java
@@ -16,7 +16,9 @@
 
 package com.android.server.connectivity.tethering;
 
+import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
+import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.reset;
@@ -38,6 +40,7 @@ import static com.android.server.connectivity.tethering.IControlsTethering.STATE
 import android.net.ConnectivityManager;
 import android.net.INetworkStatsService;
 import android.net.InterfaceConfiguration;
+import android.net.LinkProperties;
 import android.net.util.SharedLog;
 import android.os.INetworkManagementService;
 import android.os.RemoteException;
@@ -103,8 +106,9 @@ public class TetherInterfaceStateMachineTest {
                 mIPv6TetheringInterfaceServices);
         mTestedSm.start();
         mLooper.dispatchAll();
-        verify(mTetherHelper).notifyInterfaceStateChange(
-                IFACE_NAME, mTestedSm, STATE_AVAILABLE, TETHER_ERROR_NO_ERROR);
+        verify(mTetherHelper).updateInterfaceState(
+                mTestedSm, STATE_AVAILABLE, TETHER_ERROR_NO_ERROR);
+        verify(mTetherHelper).updateLinkProperties(eq(mTestedSm), any(LinkProperties.class));
         verifyNoMoreInteractions(mTetherHelper, mNMService, mStatsService);
     }
 
@@ -133,8 +137,9 @@ public class TetherInterfaceStateMachineTest {
         initStateMachine(TETHERING_BLUETOOTH);
 
         dispatchCommand(TetherInterfaceStateMachine.CMD_INTERFACE_DOWN);
-        verify(mTetherHelper).notifyInterfaceStateChange(
-                IFACE_NAME, mTestedSm, STATE_UNAVAILABLE, TETHER_ERROR_NO_ERROR);
+        verify(mTetherHelper).updateInterfaceState(
+                mTestedSm, STATE_UNAVAILABLE, TETHER_ERROR_NO_ERROR);
+        verify(mTetherHelper).updateLinkProperties(eq(mTestedSm), any(LinkProperties.class));
         verifyNoMoreInteractions(mNMService, mStatsService, mTetherHelper);
     }
 
@@ -145,8 +150,10 @@ public class TetherInterfaceStateMachineTest {
         dispatchCommand(TetherInterfaceStateMachine.CMD_TETHER_REQUESTED, STATE_TETHERED);
         InOrder inOrder = inOrder(mTetherHelper, mNMService);
         inOrder.verify(mNMService).tetherInterface(IFACE_NAME);
-        inOrder.verify(mTetherHelper).notifyInterfaceStateChange(
-                IFACE_NAME, mTestedSm, STATE_TETHERED, TETHER_ERROR_NO_ERROR);
+        inOrder.verify(mTetherHelper).updateInterfaceState(
+                mTestedSm, STATE_TETHERED, TETHER_ERROR_NO_ERROR);
+        inOrder.verify(mTetherHelper).updateLinkProperties(
+                eq(mTestedSm), any(LinkProperties.class));
         verifyNoMoreInteractions(mNMService, mStatsService, mTetherHelper);
     }
 
@@ -157,8 +164,10 @@ public class TetherInterfaceStateMachineTest {
         dispatchCommand(TetherInterfaceStateMachine.CMD_TETHER_UNREQUESTED);
         InOrder inOrder = inOrder(mNMService, mStatsService, mTetherHelper);
         inOrder.verify(mNMService).untetherInterface(IFACE_NAME);
-        inOrder.verify(mTetherHelper).notifyInterfaceStateChange(
-                IFACE_NAME, mTestedSm, STATE_AVAILABLE, TETHER_ERROR_NO_ERROR);
+        inOrder.verify(mTetherHelper).updateInterfaceState(
+                mTestedSm, STATE_AVAILABLE, TETHER_ERROR_NO_ERROR);
+        inOrder.verify(mTetherHelper).updateLinkProperties(
+                eq(mTestedSm), any(LinkProperties.class));
         verifyNoMoreInteractions(mNMService, mStatsService, mTetherHelper);
     }
 
@@ -171,8 +180,10 @@ public class TetherInterfaceStateMachineTest {
         inOrder.verify(mNMService).getInterfaceConfig(IFACE_NAME);
         inOrder.verify(mNMService).setInterfaceConfig(IFACE_NAME, mInterfaceConfiguration);
         inOrder.verify(mNMService).tetherInterface(IFACE_NAME);
-        inOrder.verify(mTetherHelper).notifyInterfaceStateChange(
-                IFACE_NAME, mTestedSm, STATE_TETHERED, TETHER_ERROR_NO_ERROR);
+        inOrder.verify(mTetherHelper).updateInterfaceState(
+                mTestedSm, STATE_TETHERED, TETHER_ERROR_NO_ERROR);
+        inOrder.verify(mTetherHelper).updateLinkProperties(
+                eq(mTestedSm), any(LinkProperties.class));
         verifyNoMoreInteractions(mNMService, mStatsService, mTetherHelper);
     }
 
@@ -180,7 +191,8 @@ public class TetherInterfaceStateMachineTest {
     public void handlesFirstUpstreamChange() throws Exception {
         initTetheredStateMachine(TETHERING_BLUETOOTH, null);
 
-        // Telling the state machine about its upstream interface triggers a little more configuration.
+        // Telling the state machine about its upstream interface triggers
+        // a little more configuration.
         dispatchTetherConnectionChanged(UPSTREAM_IFACE);
         InOrder inOrder = inOrder(mNMService);
         inOrder.verify(mNMService).enableNat(IFACE_NAME, UPSTREAM_IFACE);
@@ -248,8 +260,10 @@ public class TetherInterfaceStateMachineTest {
         inOrder.verify(mNMService).stopInterfaceForwarding(IFACE_NAME, UPSTREAM_IFACE);
         inOrder.verify(mNMService).disableNat(IFACE_NAME, UPSTREAM_IFACE);
         inOrder.verify(mNMService).untetherInterface(IFACE_NAME);
-        inOrder.verify(mTetherHelper).notifyInterfaceStateChange(
-                IFACE_NAME, mTestedSm, STATE_AVAILABLE, TETHER_ERROR_NO_ERROR);
+        inOrder.verify(mTetherHelper).updateInterfaceState(
+                mTestedSm, STATE_AVAILABLE, TETHER_ERROR_NO_ERROR);
+        inOrder.verify(mTetherHelper).updateLinkProperties(
+                eq(mTestedSm), any(LinkProperties.class));
         verifyNoMoreInteractions(mNMService, mStatsService, mTetherHelper);
     }
 
@@ -266,8 +280,10 @@ public class TetherInterfaceStateMachineTest {
             usbTeardownOrder.verify(mInterfaceConfiguration).setInterfaceDown();
             usbTeardownOrder.verify(mNMService).setInterfaceConfig(
                     IFACE_NAME, mInterfaceConfiguration);
-            usbTeardownOrder.verify(mTetherHelper).notifyInterfaceStateChange(
-                    IFACE_NAME, mTestedSm, STATE_UNAVAILABLE, TETHER_ERROR_NO_ERROR);
+            usbTeardownOrder.verify(mTetherHelper).updateInterfaceState(
+                    mTestedSm, STATE_UNAVAILABLE, TETHER_ERROR_NO_ERROR);
+            usbTeardownOrder.verify(mTetherHelper).updateLinkProperties(
+                    eq(mTestedSm), any(LinkProperties.class));
         }
     }
 
@@ -281,8 +297,10 @@ public class TetherInterfaceStateMachineTest {
         usbTeardownOrder.verify(mInterfaceConfiguration).setInterfaceDown();
         usbTeardownOrder.verify(mNMService).setInterfaceConfig(
                 IFACE_NAME, mInterfaceConfiguration);
-        usbTeardownOrder.verify(mTetherHelper).notifyInterfaceStateChange(
-                IFACE_NAME, mTestedSm, STATE_AVAILABLE, TETHER_ERROR_TETHER_IFACE_ERROR);
+        usbTeardownOrder.verify(mTetherHelper).updateInterfaceState(
+                mTestedSm, STATE_AVAILABLE, TETHER_ERROR_TETHER_IFACE_ERROR);
+        usbTeardownOrder.verify(mTetherHelper).updateLinkProperties(
+                eq(mTestedSm), any(LinkProperties.class));
     }
 
     @Test
@@ -294,8 +312,10 @@ public class TetherInterfaceStateMachineTest {
         InOrder usbTeardownOrder = inOrder(mNMService, mInterfaceConfiguration, mTetherHelper);
         usbTeardownOrder.verify(mInterfaceConfiguration).setInterfaceDown();
         usbTeardownOrder.verify(mNMService).setInterfaceConfig(IFACE_NAME, mInterfaceConfiguration);
-        usbTeardownOrder.verify(mTetherHelper).notifyInterfaceStateChange(
-                IFACE_NAME, mTestedSm, STATE_AVAILABLE, TETHER_ERROR_ENABLE_NAT_ERROR);
+        usbTeardownOrder.verify(mTetherHelper).updateInterfaceState(
+                mTestedSm, STATE_AVAILABLE, TETHER_ERROR_ENABLE_NAT_ERROR);
+        usbTeardownOrder.verify(mTetherHelper).updateLinkProperties(
+                eq(mTestedSm), any(LinkProperties.class));
     }
 
     @Test