Merge changes I414dce58,I165716a0,I101d8949

* changes:
  Implement RetryTimeout state
  Test Network startup in ConnectedState
  VDBG guard common verbose logging in Vcn

services/core/java/com/android/server/vcn/Vcn.java

@@ -16,6 +16,7 @@
 
 package com.android.server.vcn;
 
+import static com.android.server.VcnManagementService.VDBG;
 
 import android.annotation.NonNull;
 import android.net.NetworkCapabilities;
@@ -225,7 +226,7 @@ public class Vcn extends Handler {
 
     private void handleConfigUpdated(@NonNull VcnConfig config) {
         // TODO: Add a dump function in VcnConfig that omits PII. Until then, use hashCode()
-        Slog.v(getLogTag(), String.format("Config updated: config = %s", config.hashCode()));
+        Slog.v(getLogTag(), "Config updated: config = " + config.hashCode());
 
         mConfig = config;
 
@@ -251,17 +252,29 @@ public class Vcn extends Handler {
     private void handleNetworkRequested(
             @NonNull NetworkRequest request, int score, int providerId) {
         if (score > getNetworkScore()) {
-            Slog.v(getLogTag(),
+            if (VDBG) {
-                    "Request already satisfied by higher-scoring (" + score + ") network from "
+                Slog.v(
-                            + "provider " + providerId + ": " + request);
+                        getLogTag(),
+                        "Request already satisfied by higher-scoring ("
+                                + score
+                                + ") network from "
+                                + "provider "
+                                + providerId
+                                + ": "
+                                + request);
+            }
             return;
         }
 
         // If preexisting VcnGatewayConnection(s) satisfy request, return
         for (VcnGatewayConnectionConfig gatewayConnectionConfig : mVcnGatewayConnections.keySet()) {
             if (requestSatisfiedByGatewayConnectionConfig(request, gatewayConnectionConfig)) {
-                Slog.v(getLogTag(),
+                if (VDBG) {
-                        "Request already satisfied by existing VcnGatewayConnection: " + request);
+                    Slog.v(
+                            getLogTag(),
+                            "Request already satisfied by existing VcnGatewayConnection: "
+                                    + request);
+                }
                 return;
             }
         }
@@ -308,7 +321,7 @@ public class Vcn extends Handler {
     }
 
     private String getLogTag() {
-        return String.format("%s [%d]", TAG, mSubscriptionGroup.hashCode());
+        return TAG + " [" + mSubscriptionGroup.hashCode() + "]";
     }
 
     /** Retrieves the network score for a VCN Network */

services/core/java/com/android/server/vcn/VcnGatewayConnection.java

@@ -75,6 +75,7 @@ import java.net.Inet4Address;
 import java.net.Inet6Address;
 import java.net.InetAddress;
 import java.util.Arrays;
+import java.util.List;
 import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
@@ -297,9 +298,9 @@ public class VcnGatewayConnection extends StateMachine {
     private static final int EVENT_SETUP_COMPLETED = 6;
 
     private static class EventSetupCompletedInfo implements EventInfo {
-        @NonNull public final ChildSessionConfiguration childSessionConfig;
+        @NonNull public final VcnChildSessionConfiguration childSessionConfig;
 
-        EventSetupCompletedInfo(@NonNull ChildSessionConfiguration childSessionConfig) {
+        EventSetupCompletedInfo(@NonNull VcnChildSessionConfiguration childSessionConfig) {
             this.childSessionConfig = Objects.requireNonNull(childSessionConfig);
         }
 
@@ -471,7 +472,7 @@ public class VcnGatewayConnection extends StateMachine {
      * <p>Set in Connected and Migrating states, always @NonNull in Connected, Migrating
      * states, @Nullable otherwise.
      */
-    private ChildSessionConfiguration mChildConfig;
+    private VcnChildSessionConfiguration mChildConfig;
 
     /**
      * The active network agent.
@@ -659,7 +660,7 @@ public class VcnGatewayConnection extends StateMachine {
                 new EventTransformCreatedInfo(direction, transform));
     }
 
-    private void childOpened(int token, @NonNull ChildSessionConfiguration childConfig) {
+    private void childOpened(int token, @NonNull VcnChildSessionConfiguration childConfig) {
         sendMessage(EVENT_SETUP_COMPLETED, token, new EventSetupCompletedInfo(childConfig));
     }
 
@@ -1008,7 +1009,7 @@ public class VcnGatewayConnection extends StateMachine {
         protected void updateNetworkAgent(
                 @NonNull IpSecTunnelInterface tunnelIface,
                 @NonNull NetworkAgent agent,
-                @NonNull ChildSessionConfiguration childConfig) {
+                @NonNull VcnChildSessionConfiguration childConfig) {
             final NetworkCapabilities caps =
                     buildNetworkCapabilities(mConnectionConfig, mUnderlying);
             final LinkProperties lp =
@@ -1020,7 +1021,7 @@ public class VcnGatewayConnection extends StateMachine {
 
         protected NetworkAgent buildNetworkAgent(
                 @NonNull IpSecTunnelInterface tunnelIface,
-                @NonNull ChildSessionConfiguration childConfig) {
+                @NonNull VcnChildSessionConfiguration childConfig) {
             final NetworkCapabilities caps =
                     buildNetworkCapabilities(mConnectionConfig, mUnderlying);
             final LinkProperties lp =
@@ -1068,15 +1069,15 @@ public class VcnGatewayConnection extends StateMachine {
         protected void setupInterface(
                 int token,
                 @NonNull IpSecTunnelInterface tunnelIface,
-                @NonNull ChildSessionConfiguration childConfig) {
+                @NonNull VcnChildSessionConfiguration childConfig) {
             setupInterface(token, tunnelIface, childConfig, null);
         }
 
         protected void setupInterface(
                 int token,
                 @NonNull IpSecTunnelInterface tunnelIface,
-                @NonNull ChildSessionConfiguration childConfig,
+                @NonNull VcnChildSessionConfiguration childConfig,
-                @Nullable ChildSessionConfiguration oldChildConfig) {
+                @Nullable VcnChildSessionConfiguration oldChildConfig) {
             try {
                 final Set<LinkAddress> newAddrs =
                         new ArraySet<>(childConfig.getInternalAddresses());
@@ -1189,7 +1190,7 @@ public class VcnGatewayConnection extends StateMachine {
         protected void setupInterfaceAndNetworkAgent(
                 int token,
                 @NonNull IpSecTunnelInterface tunnelIface,
-                @NonNull ChildSessionConfiguration childConfig) {
+                @NonNull VcnChildSessionConfiguration childConfig) {
             setupInterface(token, tunnelIface, childConfig);
 
             if (mNetworkAgent == null) {
@@ -1207,7 +1208,64 @@ public class VcnGatewayConnection extends StateMachine {
      */
     class RetryTimeoutState extends ActiveBaseState {
         @Override
-        protected void processStateMsg(Message msg) {}
+        protected void enterState() throws Exception {
+            // Reset upon entry to ConnectedState
+            mFailedAttempts++;
+
+            if (mUnderlying == null) {
+                Slog.wtf(TAG, "Underlying network was null in retry state");
+                transitionTo(mDisconnectedState);
+            } else {
+                sendMessageDelayed(
+                        EVENT_RETRY_TIMEOUT_EXPIRED, mCurrentToken, getNextRetryIntervalsMs());
+            }
+        }
+
+        @Override
+        protected void processStateMsg(Message msg) {
+            switch (msg.what) {
+                case EVENT_UNDERLYING_NETWORK_CHANGED:
+                    final UnderlyingNetworkRecord oldUnderlying = mUnderlying;
+                    mUnderlying = ((EventUnderlyingNetworkChangedInfo) msg.obj).newUnderlying;
+
+                    // If new underlying is null, all networks were lost; go back to disconnected.
+                    if (mUnderlying == null) {
+                        removeMessages(EVENT_RETRY_TIMEOUT_EXPIRED);
+
+                        transitionTo(mDisconnectedState);
+                        return;
+                    } else if (oldUnderlying != null
+                            && mUnderlying.network.equals(oldUnderlying.network)) {
+                        // If the network has not changed, do nothing.
+                        return;
+                    }
+
+                    // Fallthrough
+                case EVENT_RETRY_TIMEOUT_EXPIRED:
+                    removeMessages(EVENT_RETRY_TIMEOUT_EXPIRED);
+
+                    transitionTo(mConnectingState);
+                    break;
+                case EVENT_DISCONNECT_REQUESTED:
+                    handleDisconnectRequested(((EventDisconnectRequestedInfo) msg.obj).reason);
+                    break;
+                default:
+                    logUnhandledMessage(msg);
+                    break;
+            }
+        }
+
+        private long getNextRetryIntervalsMs() {
+            final int retryDelayIndex = mFailedAttempts - 1;
+            final long[] retryIntervalsMs = mConnectionConfig.getRetryIntervalsMs();
+
+            // Repeatedly use last item in retry timeout list.
+            if (retryDelayIndex >= retryIntervalsMs.length) {
+                return retryIntervalsMs[retryIntervalsMs.length - 1];
+            }
+
+            return retryIntervalsMs[retryDelayIndex];
+        }
     }
 
     @VisibleForTesting(visibility = Visibility.PRIVATE)
@@ -1277,7 +1335,7 @@ public class VcnGatewayConnection extends StateMachine {
     private static LinkProperties buildConnectedLinkProperties(
             @NonNull VcnGatewayConnectionConfig gatewayConnectionConfig,
             @NonNull IpSecTunnelInterface tunnelIface,
-            @NonNull ChildSessionConfiguration childConfig) {
+            @NonNull VcnChildSessionConfiguration childConfig) {
         final LinkProperties lp = new LinkProperties();
 
         lp.setInterfaceName(tunnelIface.getInterfaceName());
@@ -1328,17 +1386,25 @@ public class VcnGatewayConnection extends StateMachine {
         }
     }
 
-    private class ChildSessionCallbackImpl implements ChildSessionCallback {
+    /** Implementation of ChildSessionCallback, exposed for testing. */
+    @VisibleForTesting(visibility = Visibility.PRIVATE)
+    public class VcnChildSessionCallback implements ChildSessionCallback {
         private final int mToken;
 
-        ChildSessionCallbackImpl(int token) {
+        VcnChildSessionCallback(int token) {
             mToken = token;
         }
 
+        /** Internal proxy method for injecting of mocked ChildSessionConfiguration */
+        @VisibleForTesting(visibility = Visibility.PRIVATE)
+        void onOpened(@NonNull VcnChildSessionConfiguration childConfig) {
+            Slog.v(TAG, "ChildOpened for token " + mToken);
+            childOpened(mToken, childConfig);
+        }
+
         @Override
         public void onOpened(@NonNull ChildSessionConfiguration childConfig) {
-            Slog.v(TAG, "ChildOpened for token " + mToken);
+            onOpened(new VcnChildSessionConfiguration(childConfig));
-            childOpened(mToken, childConfig);
         }
 
         @Override
@@ -1421,7 +1487,7 @@ public class VcnGatewayConnection extends StateMachine {
                 buildIkeParams(),
                 buildChildParams(),
                 new IkeSessionCallbackImpl(token),
-                new ChildSessionCallbackImpl(token));
+                new VcnChildSessionCallback(token));
     }
 
     /** External dependencies used by VcnGatewayConnection, for injection in tests */
@@ -1458,6 +1524,35 @@ public class VcnGatewayConnection extends StateMachine {
         }
     }
 
+    /**
+     * Proxy implementation of Child Session Configuration, used for testing.
+     *
+     * <p>This wrapper allows mocking of the final, parcelable ChildSessionConfiguration object for
+     * testing purposes. This is the unfortunate result of mockito-inline (for mocking final
+     * classes) not working properly with system services & associated classes.
+     *
+     * <p>This class MUST EXCLUSIVELY be a passthrough, proxying calls directly to the actual
+     * ChildSessionConfiguration.
+     */
+    @VisibleForTesting(visibility = Visibility.PRIVATE)
+    public static class VcnChildSessionConfiguration {
+        private final ChildSessionConfiguration mChildConfig;
+
+        public VcnChildSessionConfiguration(ChildSessionConfiguration childConfig) {
+            mChildConfig = childConfig;
+        }
+
+        /** Retrieves the addresses to be used inside the tunnel. */
+        public List<LinkAddress> getInternalAddresses() {
+            return mChildConfig.getInternalAddresses();
+        }
+
+        /** Retrieves the DNS servers to be used inside the tunnel. */
+        public List<InetAddress> getInternalDnsServers() {
+            return mChildConfig.getInternalDnsServers();
+        }
+    }
+
     /** Proxy implementation of IKE session, used for testing. */
     @VisibleForTesting(visibility = Visibility.PRIVATE)
     public static class VcnIkeSession {

services/core/java/com/android/server/vcn/VcnNetworkProvider.java

@@ -16,6 +16,8 @@
 
 package com.android.server.vcn;
 
+import static com.android.server.VcnManagementService.VDBG;
+
 import android.annotation.NonNull;
 import android.content.Context;
 import android.net.NetworkProvider;
@@ -83,11 +85,16 @@ public class VcnNetworkProvider extends NetworkProvider {
 
     @Override
     public void onNetworkRequested(@NonNull NetworkRequest request, int score, int providerId) {
+        if (VDBG) {
             Slog.v(
                     TAG,
-                String.format(
+                    "Network requested: Request = "
-                        "Network requested: Request = %s, score = %d, providerId = %d",
+                            + request
-                        request, score, providerId));
+                            + ", score = "
+                            + score
+                            + ", providerId = "
+                            + providerId);
+        }
 
         final NetworkRequestEntry entry = new NetworkRequestEntry(request, score, providerId);
 

tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java

@@ -18,22 +18,35 @@ package com.android.server.vcn;
 
 import static android.net.IpSecManager.DIRECTION_IN;
 import static android.net.IpSecManager.DIRECTION_OUT;
+import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR;
+import static android.net.NetworkCapabilities.TRANSPORT_WIFI;
 
+import static com.android.server.vcn.VcnGatewayConnection.VcnChildSessionConfiguration;
 import static com.android.server.vcn.VcnGatewayConnection.VcnIkeSession;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyInt;
 import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 
+import android.net.LinkProperties;
+import android.net.NetworkCapabilities;
+
 import androidx.test.filters.SmallTest;
 import androidx.test.runner.AndroidJUnit4;
 
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+
+import java.util.Collections;
 
 /** Tests for VcnGatewayConnection.ConnectedState */
 @RunWith(AndroidJUnit4.class)
@@ -106,6 +119,51 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection
         assertEquals(mGatewayConnection.mConnectedState, mGatewayConnection.getCurrentState());
     }
 
+    @Test
+    public void testChildOpenedRegistersNetwork() throws Exception {
+        final VcnChildSessionConfiguration mMockChildSessionConfig =
+                mock(VcnChildSessionConfiguration.class);
+        doReturn(Collections.singletonList(TEST_INTERNAL_ADDR))
+                .when(mMockChildSessionConfig)
+                .getInternalAddresses();
+        doReturn(Collections.singletonList(TEST_DNS_ADDR))
+                .when(mMockChildSessionConfig)
+                .getInternalDnsServers();
+
+        getChildSessionCallback().onOpened(mMockChildSessionConfig);
+        mTestLooper.dispatchAll();
+
+        assertEquals(mGatewayConnection.mConnectedState, mGatewayConnection.getCurrentState());
+
+        final ArgumentCaptor<LinkProperties> lpCaptor =
+                ArgumentCaptor.forClass(LinkProperties.class);
+        final ArgumentCaptor<NetworkCapabilities> ncCaptor =
+                ArgumentCaptor.forClass(NetworkCapabilities.class);
+        verify(mConnMgr)
+                .registerNetworkAgent(
+                        any(),
+                        any(),
+                        lpCaptor.capture(),
+                        ncCaptor.capture(),
+                        anyInt(),
+                        any(),
+                        anyInt());
+        verify(mIpSecSvc)
+                .addAddressToTunnelInterface(
+                        eq(TEST_IPSEC_TUNNEL_RESOURCE_ID), eq(TEST_INTERNAL_ADDR), any());
+
+        final LinkProperties lp = lpCaptor.getValue();
+        assertEquals(Collections.singletonList(TEST_INTERNAL_ADDR), lp.getLinkAddresses());
+        assertEquals(Collections.singletonList(TEST_DNS_ADDR), lp.getDnsServers());
+
+        final NetworkCapabilities nc = ncCaptor.getValue();
+        assertTrue(nc.hasTransport(TRANSPORT_CELLULAR));
+        assertFalse(nc.hasTransport(TRANSPORT_WIFI));
+        for (int cap : mConfig.getAllExposedCapabilities()) {
+            assertTrue(nc.hasCapability(cap));
+        }
+    }
+
     @Test
     public void testChildSessionClosedTriggersDisconnect() throws Exception {
         getChildSessionCallback().onClosed();
@@ -122,6 +180,4 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection
         assertEquals(mGatewayConnection.mRetryTimeoutState, mGatewayConnection.getCurrentState());
         verify(mIkeSession).close();
     }
-
-    // TODO: Add tests for childOpened() when ChildSessionConfiguration can be mocked or created
 }

tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionRetryTimeoutStateTest.java

@@ -0,0 +1,78 @@
+/*
+ * Copyright (C) 2020 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 com.android.server.vcn;
+
+import static org.junit.Assert.assertEquals;
+
+import androidx.test.filters.SmallTest;
+import androidx.test.runner.AndroidJUnit4;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+/** Tests for VcnGatewayConnection.RetryTimeoutState */
+@RunWith(AndroidJUnit4.class)
+@SmallTest
+public class VcnGatewayConnectionRetryTimeoutStateTest extends VcnGatewayConnectionTestBase {
+    @Before
+    public void setUp() throws Exception {
+        super.setUp();
+
+        mGatewayConnection.setUnderlyingNetwork(TEST_UNDERLYING_NETWORK_RECORD_1);
+        mGatewayConnection.transitionTo(mGatewayConnection.mRetryTimeoutState);
+        mTestLooper.dispatchAll();
+    }
+
+    @Test
+    public void testNewNetworkTriggerRetry() throws Exception {
+        mGatewayConnection
+                .getUnderlyingNetworkTrackerCallback()
+                .onSelectedUnderlyingNetworkChanged(TEST_UNDERLYING_NETWORK_RECORD_2);
+        mTestLooper.dispatchAll();
+
+        assertEquals(mGatewayConnection.mConnectingState, mGatewayConnection.getCurrentState());
+    }
+
+    @Test
+    public void testSameNetworkDoesNotTriggerRetry() throws Exception {
+        mGatewayConnection
+                .getUnderlyingNetworkTrackerCallback()
+                .onSelectedUnderlyingNetworkChanged(TEST_UNDERLYING_NETWORK_RECORD_1);
+        mTestLooper.dispatchAll();
+
+        assertEquals(mGatewayConnection.mRetryTimeoutState, mGatewayConnection.getCurrentState());
+    }
+
+    @Test
+    public void testNullNetworkTriggersDisconnect() throws Exception {
+        mGatewayConnection
+                .getUnderlyingNetworkTrackerCallback()
+                .onSelectedUnderlyingNetworkChanged(null);
+        mTestLooper.dispatchAll();
+
+        assertEquals(mGatewayConnection.mDisconnectedState, mGatewayConnection.getCurrentState());
+    }
+
+    @Test
+    public void testTimeoutElapsingTriggersRetry() throws Exception {
+        mTestLooper.moveTimeForward(mConfig.getRetryIntervalsMs()[0]);
+        mTestLooper.dispatchAll();
+
+        assertEquals(mGatewayConnection.mConnectingState, mGatewayConnection.getCurrentState());
+    }
+}

tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java

@@ -27,10 +27,13 @@ import static org.mockito.Mockito.verify;
 
 import android.annotation.NonNull;
 import android.content.Context;
+import android.net.ConnectivityManager;
+import android.net.InetAddresses;
 import android.net.IpSecConfig;
 import android.net.IpSecManager;
 import android.net.IpSecTransform;
 import android.net.IpSecTunnelInterfaceResponse;
+import android.net.LinkAddress;
 import android.net.LinkProperties;
 import android.net.Network;
 import android.net.NetworkCapabilities;
@@ -44,15 +47,22 @@ import android.os.test.TestLooper;
 import com.android.server.IpSecService;
 import com.android.server.vcn.TelephonySubscriptionTracker.TelephonySubscriptionSnapshot;
 import com.android.server.vcn.Vcn.VcnGatewayStatusCallback;
+import com.android.server.vcn.VcnGatewayConnection.VcnChildSessionCallback;
 
 import org.junit.Before;
 import org.mockito.ArgumentCaptor;
 
+import java.net.InetAddress;
 import java.util.Collections;
 import java.util.UUID;
 
 public class VcnGatewayConnectionTestBase {
     protected static final ParcelUuid TEST_SUB_GRP = new ParcelUuid(UUID.randomUUID());
+    protected static final InetAddress TEST_DNS_ADDR =
+            InetAddresses.parseNumericAddress("2001:DB8:0:1::");
+    protected static final LinkAddress TEST_INTERNAL_ADDR =
+            new LinkAddress(InetAddresses.parseNumericAddress("2001:DB8:0:2::"), 64);
+
     protected static final int TEST_IPSEC_SPI_VALUE = 0x1234;
     protected static final int TEST_IPSEC_SPI_RESOURCE_ID = 1;
     protected static final int TEST_IPSEC_TRANSFORM_RESOURCE_ID = 2;
@@ -86,6 +96,7 @@ public class VcnGatewayConnectionTestBase {
     @NonNull protected final UnderlyingNetworkTracker mUnderlyingNetworkTracker;
 
     @NonNull protected final IpSecService mIpSecSvc;
+    @NonNull protected final ConnectivityManager mConnMgr;
 
     protected VcnIkeSession mMockIkeSession;
     protected VcnGatewayConnection mGatewayConnection;
@@ -103,6 +114,10 @@ public class VcnGatewayConnectionTestBase {
         mIpSecSvc = mock(IpSecService.class);
         setupIpSecManager(mContext, mIpSecSvc);
 
+        mConnMgr = mock(ConnectivityManager.class);
+        VcnTestUtils.setupSystemService(
+                mContext, mConnMgr, Context.CONNECTIVITY_SERVICE, ConnectivityManager.class);
+
         doReturn(mContext).when(mVcnContext).getContext();
         doReturn(mTestLooper.getLooper()).when(mVcnContext).getLooper();
         doReturn(mVcnNetworkProvider).when(mVcnContext).getVcnNetworkProvider();
@@ -145,10 +160,10 @@ public class VcnGatewayConnectionTestBase {
         return captor.getValue();
     }
 
-    protected ChildSessionCallback getChildSessionCallback() {
+    protected VcnChildSessionCallback getChildSessionCallback() {
         ArgumentCaptor<ChildSessionCallback> captor =
                 ArgumentCaptor.forClass(ChildSessionCallback.class);
         verify(mDeps).newIkeSession(any(), any(), any(), any(), captor.capture());
-        return captor.getValue();
+        return (VcnChildSessionCallback) captor.getValue();
     }
 }