Make interface IP serving code set LinkProperties

Additionally, clean up awkward IPv6TetheringInterfaceServices
instantiation mechanics.  In future, this class will be absorbed
by TetherInterfaceStateMachine (prior to its renaming to IpServer).

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

Change-Id: Ib620e3df182f9f8e2c019aae1cd8981eb0b29376

diff --git a/services/core/java/com/android/server/connectivity/Tethering.java b/services/core/java/com/android/server/connectivity/Tethering.java
index 4a5129d..0f9d881 100644 (file)
--- a/services/core/java/com/android/server/connectivity/Tethering.java
+++ b/services/core/java/com/android/server/connectivity/Tethering.java
@@ -1258,10 +1258,10 @@ public class Tethering extends BaseNetworkObserver {
                         sendMessageDelayed(CMD_RETRY_UPSTREAM, UPSTREAM_SETTLE_TIME_MS);
                     }
                 }
-                setUpstreamByType(ns);
+                setUpstreamNetwork(ns);
             }
 
-            protected void setUpstreamByType(NetworkState ns) {
+            protected void setUpstreamNetwork(NetworkState ns) {
                 String iface = null;
                 if (ns != null && ns.linkProperties != null) {
                     // Find the interface with the default IPv4 route. It may be the
@@ -1786,7 +1786,9 @@ public class Tethering extends BaseNetworkObserver {
             }
         }
 
-        mLog.log(String.format("OBSERVED LinkProperties update iface=%s state=%s", iface, state));
+        mLog.log(String.format(
+                "OBSERVED LinkProperties update iface=%s state=%s lp=%s",
+                iface, IControlsTethering.getStateString(state), newLp));
         final int which = TetherMasterSM.EVENT_IFACE_UPDATE_LINKPROPERTIES;
         mTetherMasterSM.sendMessage(which, state, 0, newLp);
     }

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 aaa63b1..2b81347 100644 (file)
--- a/services/core/java/com/android/server/connectivity/tethering/IControlsTethering.java
+++ b/services/core/java/com/android/server/connectivity/tethering/IControlsTethering.java
@@ -33,6 +33,16 @@ public class IControlsTethering {
     public static final int STATE_TETHERED    = 2;
     public static final int STATE_LOCAL_ONLY  = 3;
 
+    public static String getStateString(int state) {
+        switch (state) {
+            case STATE_UNAVAILABLE: return "UNAVAILABLE";
+            case STATE_AVAILABLE:   return "AVAILABLE";
+            case STATE_TETHERED:    return "TETHERED";
+            case STATE_LOCAL_ONLY:  return "LOCAL_ONLY";
+        }
+        return "UNKNOWN: " + state;
+    }
+
     /**
      * Notify that |who| has changed its tethering state.
      *

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 ce6b8be..08deef8 100644 (file)
--- a/services/core/java/com/android/server/connectivity/tethering/OffloadController.java
+++ b/services/core/java/com/android/server/connectivity/tethering/OffloadController.java
@@ -85,13 +85,16 @@ public class OffloadController {
             mLog.i("tethering offload control not supported");
             stop();
         }
+        mLog.log("tethering offload started");
     }
 
     public void stop() {
+        final boolean wasStarted = started();
         mUpstreamLinkProperties = null;
         mHwInterface.stopOffloadControl();
         mControlInitialized = false;
         mConfigInitialized = false;
+        if (wasStarted) mLog.log("tethering offload stopped");
     }
 
     public void setUpstreamLinkProperties(LinkProperties lp) {

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 bff13d4..86b2551 100644 (file)
--- a/services/core/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachine.java
+++ b/services/core/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachine.java
@@ -56,9 +56,10 @@ import java.util.Objects;
 import java.util.Random;
 
 /**
- * @hide
+ * Provides the interface to IP-layer serving functionality for a given network
+ * interface, e.g. for tethering or "local-only hotspot" mode.
  *
- * Tracks the eligibility of a given network interface for tethering.
+ * @hide
  */
 public class TetherInterfaceStateMachine extends StateMachine {
     private static final IpPrefix LINK_LOCAL_PREFIX = new IpPrefix("fe80::/64");
@@ -117,6 +118,12 @@ public class TetherInterfaceStateMachine extends StateMachine {
     private String mMyUpstreamIfaceName;  // may change over time
     private NetworkInterface mNetworkInterface;
     private byte[] mHwAddr;
+    // TODO: De-duplicate this with mLinkProperties above. Currently, these link
+    // properties are those selected by the IPv6TetheringCoordinator and relayed
+    // to us. By comparison, mLinkProperties contains the addresses and directly
+    // connected routes that have been formed from these properties iff. we have
+    // succeeded in configuring them and are able to announce them within Router
+    // Advertisements (otherwise, we do not add them to mLinkProperties at all).
     private LinkProperties mLastIPv6LinkProperties;
     private RouterAdvertisementDaemon mRaDaemon;
     private RaParams mLastRaParams;
@@ -133,7 +140,7 @@ public class TetherInterfaceStateMachine extends StateMachine {
         mIfaceName = ifaceName;
         mInterfaceType = interfaceType;
         mLinkProperties = new LinkProperties();
-        mLinkProperties.setInterfaceName(mIfaceName);
+        resetLinkProperties();
         mLastError = ConnectivityManager.TETHER_ERROR_NO_ERROR;
 
         mInitialState = new InitialState();
@@ -162,10 +169,15 @@ public class TetherInterfaceStateMachine extends StateMachine {
      * Internals.
      */
 
-    // configured when we start tethering and unconfig'd on error or conclusion
-    private boolean configureIfaceIp(boolean enabled) {
-        if (VDBG) Log.d(TAG, "configureIfaceIp(" + enabled + ")");
+    private boolean startIPv4() { return configureIPv4(true); }
 
+    private void stopIPv4() { configureIPv4(false); }
+
+    private boolean configureIPv4(boolean enabled) {
+        if (VDBG) Log.d(TAG, "configureIPv4(" + enabled + ")");
+
+        // TODO: Replace this hard-coded information with dynamically selected
+        // config passed down to us by a higher layer IP-coordinating element.
         String ipAsString = null;
         int prefixLen = 0;
         if (mInterfaceType == ConnectivityManager.TETHERING_USB) {
@@ -179,32 +191,45 @@ public class TetherInterfaceStateMachine extends StateMachine {
             return true;
         }
 
-        InterfaceConfiguration ifcg = null;
+        final LinkAddress linkAddr;
         try {
-            ifcg = mNMService.getInterfaceConfig(mIfaceName);
-            if (ifcg != null) {
-                InetAddress addr = NetworkUtils.numericToInetAddress(ipAsString);
-                ifcg.setLinkAddress(new LinkAddress(addr, prefixLen));
-                if (mInterfaceType == ConnectivityManager.TETHERING_WIFI) {
-                    // The WiFi stack has ownership of the interface up/down state.
-                    // It is unclear whether the bluetooth or USB stacks will manage their own
-                    // state.
-                    ifcg.ignoreInterfaceUpDownStatus();
+            final InterfaceConfiguration ifcg = mNMService.getInterfaceConfig(mIfaceName);
+            if (ifcg == null) {
+                mLog.e("Received null interface config");
+                return false;
+            }
+
+            InetAddress addr = NetworkUtils.numericToInetAddress(ipAsString);
+            linkAddr = new LinkAddress(addr, prefixLen);
+            ifcg.setLinkAddress(linkAddr);
+            if (mInterfaceType == ConnectivityManager.TETHERING_WIFI) {
+                // The WiFi stack has ownership of the interface up/down state.
+                // It is unclear whether the Bluetooth or USB stacks will manage their own
+                // state.
+                ifcg.ignoreInterfaceUpDownStatus();
+            } else {
+                if (enabled) {
+                    ifcg.setInterfaceUp();
                 } else {
-                    if (enabled) {
-                        ifcg.setInterfaceUp();
-                    } else {
-                        ifcg.setInterfaceDown();
-                    }
+                    ifcg.setInterfaceDown();
                 }
-                ifcg.clearFlag("running");
-                mNMService.setInterfaceConfig(mIfaceName, ifcg);
             }
+            ifcg.clearFlag("running");
+            mNMService.setInterfaceConfig(mIfaceName, ifcg);
         } catch (Exception e) {
             mLog.e("Error configuring interface " + e);
             return false;
         }
 
+        // Directly-connected route.
+        final RouteInfo route = new RouteInfo(linkAddr);
+        if (enabled) {
+            mLinkProperties.addLinkAddress(linkAddr);
+            mLinkProperties.addRoute(route);
+        } else {
+            mLinkProperties.removeLinkAddress(linkAddr);
+            mLinkProperties.removeRoute(route);
+        }
         return true;
     }
 
@@ -294,7 +319,7 @@ public class TetherInterfaceStateMachine extends StateMachine {
         mLastIPv6LinkProperties = v6only;
     }
 
-    private void configureLocalRoutes(
+    private void configureLocalIPv6Routes(
             HashSet<IpPrefix> deprecatedPrefixes, HashSet<IpPrefix> newPrefixes) {
         // [1] Remove the routes that are deprecated.
         if (!deprecatedPrefixes.isEmpty()) {
@@ -309,6 +334,8 @@ public class TetherInterfaceStateMachine extends StateMachine {
             } catch (RemoteException e) {
                 mLog.e("Failed to remove IPv6 routes from local table: " + e);
             }
+
+            for (RouteInfo route : toBeRemoved) mLinkProperties.removeRoute(route);
         }
 
         // [2] Add only the routes that have not previously been added.
@@ -340,11 +367,13 @@ public class TetherInterfaceStateMachine extends StateMachine {
                 } catch (RemoteException e) {
                     mLog.e("Failed to add IPv6 routes to local table: " + e);
                 }
+
+                for (RouteInfo route : toBeAdded) mLinkProperties.addRoute(route);
             }
         }
     }
 
-    private void configureLocalDns(
+    private void configureLocalIPv6Dns(
             HashSet<Inet6Address> deprecatedDnses, HashSet<Inet6Address> newDnses) {
         final INetd netd = NetdService.getInstance();
         if (netd == null) {
@@ -362,6 +391,8 @@ public class TetherInterfaceStateMachine extends StateMachine {
                 } catch (ServiceSpecificException | RemoteException e) {
                     mLog.e("Failed to remove local dns IP " + dnsString + ": " + e);
                 }
+
+                mLinkProperties.removeLinkAddress(new LinkAddress(dns, RFC7421_PREFIX_LENGTH));
             }
         }
 
@@ -380,6 +411,8 @@ public class TetherInterfaceStateMachine extends StateMachine {
                     mLog.e("Failed to add local dns IP " + dnsString + ": " + e);
                     newDnses.remove(dns);
                 }
+
+                mLinkProperties.addLinkAddress(new LinkAddress(dns, RFC7421_PREFIX_LENGTH));
             }
         }
 
@@ -396,10 +429,10 @@ public class TetherInterfaceStateMachine extends StateMachine {
             final RaParams deprecatedParams =
                     RaParams.getDeprecatedRaParams(mLastRaParams, newParams);
 
-            configureLocalRoutes(deprecatedParams.prefixes,
+            configureLocalIPv6Routes(deprecatedParams.prefixes,
                     (newParams != null) ? newParams.prefixes : null);
 
-            configureLocalDns(deprecatedParams.dnses,
+            configureLocalIPv6Dns(deprecatedParams.dnses,
                     (newParams != null) ? newParams.dnses : null);
 
             mRaDaemon.buildNewRa(deprecatedParams, newParams);
@@ -419,12 +452,19 @@ public class TetherInterfaceStateMachine extends StateMachine {
     private void sendInterfaceState(int newInterfaceState) {
         mTetherController.updateInterfaceState(
                 TetherInterfaceStateMachine.this, newInterfaceState, mLastError);
-        // TODO: Populate mLinkProperties correctly, and send more sensible
-        // updates more frequently (not just here).
+        sendLinkProperties();
+    }
+
+    private void sendLinkProperties() {
         mTetherController.updateLinkProperties(
                 TetherInterfaceStateMachine.this, new LinkProperties(mLinkProperties));
     }
 
+    private void resetLinkProperties() {
+        mLinkProperties.clear();
+        mLinkProperties.setInterfaceName(mIfaceName);
+    }
+
     class InitialState extends State {
         @Override
         public void enter() {
@@ -464,7 +504,7 @@ public class TetherInterfaceStateMachine extends StateMachine {
     class BaseServingState extends State {
         @Override
         public void enter() {
-            if (!configureIfaceIp(true)) {
+            if (!startIPv4()) {
                 mLastError = ConnectivityManager.TETHER_ERROR_IFACE_CFG_ERROR;
                 return;
             }
@@ -498,7 +538,9 @@ public class TetherInterfaceStateMachine extends StateMachine {
                 mLog.e("Failed to untether interface: " + e);
             }
 
-            configureIfaceIp(false);
+            stopIPv4();
+
+            resetLinkProperties();
         }
 
         @Override
@@ -515,6 +557,7 @@ public class TetherInterfaceStateMachine extends StateMachine {
                     break;
                 case CMD_IPV6_TETHER_UPDATE:
                     updateUpstreamIPv6LinkProperties((LinkProperties) message.obj);
+                    sendLinkProperties();
                     break;
                 case CMD_IP_FORWARDING_ENABLE_ERROR:
                 case CMD_IP_FORWARDING_DISABLE_ERROR:
@@ -625,7 +668,6 @@ public class TetherInterfaceStateMachine extends StateMachine {
             if (super.processMessage(message)) return true;
 
             maybeLogMessage(this, message.what);
-            boolean retValue = true;
             switch (message.what) {
                 case CMD_TETHER_REQUESTED:
                     mLog.e("CMD_TETHER_REQUESTED while already tethering.");
@@ -655,10 +697,9 @@ public class TetherInterfaceStateMachine extends StateMachine {
                     mMyUpstreamIfaceName = newUpstreamIfaceName;
                     break;
                 default:
-                    retValue = false;
-                    break;
+                    return false;
             }
-            return retValue;
+            return true;
         }
     }
 

diff --git a/tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java b/tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java
index 14284d6..1ddaf66 100644 (file)
--- a/tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java
+++ b/tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java
@@ -62,7 +62,8 @@ public class OffloadControllerTest {
     @Mock private OffloadHardwareInterface mHardware;
     @Mock private ApplicationInfo mApplicationInfo;
     @Mock private Context mContext;
-    final ArgumentCaptor<ArrayList> mStringArrayCaptor = ArgumentCaptor.forClass(ArrayList.class);
+    private final ArgumentCaptor<ArrayList> mStringArrayCaptor =
+            ArgumentCaptor.forClass(ArrayList.class);
     private MockContentResolver mContentResolver;
 
     @Before public void setUp() throws Exception {

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 ce419a5..db5373a 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,6 +16,8 @@
 
 package com.android.server.connectivity.tethering;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.eq;
@@ -40,17 +42,23 @@ import static com.android.server.connectivity.tethering.IControlsTethering.STATE
 import android.net.ConnectivityManager;
 import android.net.INetworkStatsService;
 import android.net.InterfaceConfiguration;
+import android.net.LinkAddress;
 import android.net.LinkProperties;
+import android.net.RouteInfo;
 import android.net.util.SharedLog;
 import android.os.INetworkManagementService;
 import android.os.RemoteException;
 import android.os.test.TestLooper;
 import android.support.test.filters.SmallTest;
 import android.support.test.runner.AndroidJUnit4;
+import android.text.TextUtils;
+
+import java.net.Inet4Address;
 
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
 import org.mockito.InOrder;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
@@ -69,6 +77,8 @@ public class TetherInterfaceStateMachineTest {
     @Mock private SharedLog mSharedLog;
 
     private final TestLooper mLooper = new TestLooper();
+    private final ArgumentCaptor<LinkProperties> mLinkPropertiesCaptor =
+            ArgumentCaptor.forClass(LinkProperties.class);
     private TetherInterfaceStateMachine mTestedSm;
 
     private void initStateMachine(int interfaceType) throws Exception {
@@ -77,7 +87,7 @@ public class TetherInterfaceStateMachineTest {
                 mNMService, mStatsService, mTetherHelper);
         mTestedSm.start();
         // Starting the state machine always puts us in a consistent state and notifies
-        // the test of the world that we've changed from an unknown to available state.
+        // the rest of the world that we've changed from an unknown to available state.
         mLooper.dispatchAll();
         reset(mNMService, mStatsService, mTetherHelper);
         when(mNMService.getInterfaceConfig(IFACE_NAME)).thenReturn(mInterfaceConfiguration);
@@ -181,7 +191,8 @@ public class TetherInterfaceStateMachineTest {
         inOrder.verify(mTetherHelper).updateInterfaceState(
                 mTestedSm, STATE_TETHERED, TETHER_ERROR_NO_ERROR);
         inOrder.verify(mTetherHelper).updateLinkProperties(
-                eq(mTestedSm), any(LinkProperties.class));
+                eq(mTestedSm), mLinkPropertiesCaptor.capture());
+        assertIPv4AddressAndDirectlyConnectedRoute(mLinkPropertiesCaptor.getValue());
         verifyNoMoreInteractions(mNMService, mStatsService, mTetherHelper);
     }
 
@@ -281,7 +292,8 @@ public class TetherInterfaceStateMachineTest {
             usbTeardownOrder.verify(mTetherHelper).updateInterfaceState(
                     mTestedSm, STATE_UNAVAILABLE, TETHER_ERROR_NO_ERROR);
             usbTeardownOrder.verify(mTetherHelper).updateLinkProperties(
-                    eq(mTestedSm), any(LinkProperties.class));
+                    eq(mTestedSm), mLinkPropertiesCaptor.capture());
+            assertNoAddressesNorRoutes(mLinkPropertiesCaptor.getValue());
         }
     }
 
@@ -298,7 +310,8 @@ public class TetherInterfaceStateMachineTest {
         usbTeardownOrder.verify(mTetherHelper).updateInterfaceState(
                 mTestedSm, STATE_AVAILABLE, TETHER_ERROR_TETHER_IFACE_ERROR);
         usbTeardownOrder.verify(mTetherHelper).updateLinkProperties(
-                eq(mTestedSm), any(LinkProperties.class));
+                eq(mTestedSm), mLinkPropertiesCaptor.capture());
+        assertNoAddressesNorRoutes(mLinkPropertiesCaptor.getValue());
     }
 
     @Test
@@ -313,7 +326,8 @@ public class TetherInterfaceStateMachineTest {
         usbTeardownOrder.verify(mTetherHelper).updateInterfaceState(
                 mTestedSm, STATE_AVAILABLE, TETHER_ERROR_ENABLE_NAT_ERROR);
         usbTeardownOrder.verify(mTetherHelper).updateLinkProperties(
-                eq(mTestedSm), any(LinkProperties.class));
+                eq(mTestedSm), mLinkPropertiesCaptor.capture());
+        assertNoAddressesNorRoutes(mLinkPropertiesCaptor.getValue());
     }
 
     @Test
@@ -360,4 +374,28 @@ public class TetherInterfaceStateMachineTest {
                 upstreamIface);
         mLooper.dispatchAll();
     }
+
+    private void assertIPv4AddressAndDirectlyConnectedRoute(LinkProperties lp) {
+        // Find the first IPv4 LinkAddress.
+        LinkAddress addr4 = null;
+        for (LinkAddress addr : lp.getLinkAddresses()) {
+            if (!(addr.getAddress() instanceof Inet4Address)) continue;
+            addr4 = addr;
+            break;
+        }
+        assertTrue("missing IPv4 address", addr4 != null);
+
+        // Assert the presence of the associated directly connected route.
+        final RouteInfo directlyConnected = new RouteInfo(addr4, null, lp.getInterfaceName());
+        assertTrue("missing directly connected route: '" + directlyConnected.toString() + "'",
+                   lp.getRoutes().contains(directlyConnected));
+    }
+
+    private void assertNoAddressesNorRoutes(LinkProperties lp) {
+        assertTrue(lp.getLinkAddresses().isEmpty());
+        assertTrue(lp.getRoutes().isEmpty());
+        // We also check that interface name is non-empty, because we should
+        // never see an empty interface name in any LinkProperties update.
+        assertFalse(TextUtils.isEmpty(lp.getInterfaceName()));
+    }
 }