diff --git a/Vcn/service-b/src/com/android/server/vcn/VcnGatewayConnection.java b/Vcn/service-b/src/com/android/server/vcn/VcnGatewayConnection.java index b5f043aa7e..57ef5da9a9 100644 --- a/Vcn/service-b/src/com/android/server/vcn/VcnGatewayConnection.java +++ b/Vcn/service-b/src/com/android/server/vcn/VcnGatewayConnection.java @@ -432,6 +432,8 @@ public class VcnGatewayConnection extends StateMachine { public final boolean shouldQuit; + // TODO(b/440645797): Consider not taking in shouldQuit as a param and relying on the reason + // deterministically. EventDisconnectRequestedInfo(@NonNull String reason, boolean shouldQuit) { this.reason = Objects.requireNonNull(reason); this.shouldQuit = shouldQuit; @@ -451,6 +453,26 @@ public class VcnGatewayConnection extends StateMachine { final EventDisconnectRequestedInfo rhs = (EventDisconnectRequestedInfo) other; return reason.equals(rhs.reason) && shouldQuit == rhs.shouldQuit; } + + @VcnMetrics.GatewayTeardownReason + int getGatewayTeardownReason() { + // TODO(b/440645797): Assuming DISCONNECT_REASON_UNDERLYING_NETWORK_LOST means + // shouldQuit == false. Consider removing shouldQuit variable and rely on the reason + // deterministically. + if (!shouldQuit) { + return VcnMetrics.GATEWAY_TEARDOWN_REASON_NONE; + } + switch (reason) { + case DISCONNECT_REASON_INTERNAL_ERROR: + return VcnMetrics.GATEWAY_TEARDOWN_REASON_INTERNAL_ERROR; + case DISCONNECT_REASON_NETWORK_AGENT_UNWANTED: + return VcnMetrics.GATEWAY_TEARDOWN_REASON_NETWORK_AGENT_UNWANTED; + case DISCONNECT_REASON_TEARDOWN: + return VcnMetrics.GATEWAY_TEARDOWN_REASON_REQUESTED; + default: + return VcnMetrics.GATEWAY_TEARDOWN_REASON_UNSPECIFIED; + } + } } /** @@ -673,6 +695,15 @@ public class VcnGatewayConnection extends StateMachine { */ private OneWayBoolean mIsQuitting = new OneWayBoolean(); + /** + * The reason we are about to quit/teardown VcnGatewayConnection. + * + *

When mIsQuitting is set true, we also update the reason for logging purposes. If multiple + * reasons/triggers end up sending teardown command, we will log with the last reason received. + */ + private @VcnMetrics.GatewayTeardownReason int mTeardownReason = + VcnMetrics.GATEWAY_TEARDOWN_REASON_NONE; + /** * Whether the VcnGatewayConnection is in safe mode. * @@ -886,6 +917,7 @@ public class VcnGatewayConnection extends StateMachine { mGatewayStatusCallback.onQuit(); + mVcnMetrics.logVcnGatewayTeardown(mId, mTeardownReason); mConnectivityDiagnosticsManager.unregisterConnectivityDiagnosticsCallback( mConnectivityDiagnosticsCallback); } @@ -1110,6 +1142,11 @@ public class VcnGatewayConnection extends StateMachine { maybeReleaseWakeLock(); } + private void markQuitting(@VcnMetrics.GatewayTeardownReason int teardownReason) { + mIsQuitting.setTrue(); + mTeardownReason = teardownReason; + } + private WakeupMessage createScheduledAlarm( @NonNull String cmdName, Message delayedMessage, long delay) { final Handler handler = getHandler(); @@ -1508,7 +1545,7 @@ public class VcnGatewayConnection extends StateMachine { logInfo("Tearing down. Cause: " + info.reason + "; quitting = " + info.shouldQuit); if (info.shouldQuit) { - mIsQuitting.setTrue(); + markQuitting(info.getGatewayTeardownReason()); } teardownNetwork(); @@ -1579,8 +1616,9 @@ public class VcnGatewayConnection extends StateMachine { } break; case EVENT_DISCONNECT_REQUESTED: - if (((EventDisconnectRequestedInfo) msg.obj).shouldQuit) { - mIsQuitting.setTrue(); + EventDisconnectRequestedInfo info = (EventDisconnectRequestedInfo) msg.obj; + if (info.shouldQuit) { + markQuitting(info.getGatewayTeardownReason()); quitNow(); } @@ -1667,7 +1705,7 @@ public class VcnGatewayConnection extends StateMachine { case EVENT_DISCONNECT_REQUESTED: EventDisconnectRequestedInfo info = ((EventDisconnectRequestedInfo) msg.obj); if (info.shouldQuit) { - mIsQuitting.setTrue(); + markQuitting(info.getGatewayTeardownReason()); } teardownNetwork(); @@ -1856,6 +1894,8 @@ public class VcnGatewayConnection extends StateMachine { } logInfo("NetworkAgent was unwanted"); + // TODO(b/440645797): This tears down with _TEARDOWN reason rather + // than _NETWORK_AGENT_UNWANTED. teardownAsynchronously(); } /* networkUnwantedCallback */, (status) -> { @@ -2675,6 +2715,12 @@ public class VcnGatewayConnection extends StateMachine { return mIsQuitting.getValue(); } + @VisibleForTesting(visibility = Visibility.PRIVATE) + @VcnMetrics.GatewayTeardownReason + int getTeardownReason() { + return mTeardownReason; + } + @VisibleForTesting(visibility = Visibility.PRIVATE) void setQuitting() { mIsQuitting.setTrue(); diff --git a/Vcn/service-b/src/com/android/server/vcn/metrics/VcnMetrics.java b/Vcn/service-b/src/com/android/server/vcn/metrics/VcnMetrics.java index b21481ee59..0ccd4ecd4d 100644 --- a/Vcn/service-b/src/com/android/server/vcn/metrics/VcnMetrics.java +++ b/Vcn/service-b/src/com/android/server/vcn/metrics/VcnMetrics.java @@ -16,18 +16,47 @@ package com.android.server.vcn.metrics; +import static com.android.server.vcn.metrics.VcnStatsLog.VCN_GATEWAY_CONNECTION_STATE_CHANGED; +import static com.android.server.vcn.metrics.VcnStatsLog.VCN_GATEWAY_CONNECTION_STATE_CHANGED__GW_TEARDOWN_REASON__GATEWAY_TEARDOWN_REASON_INTERNAL_ERROR; +import static com.android.server.vcn.metrics.VcnStatsLog.VCN_GATEWAY_CONNECTION_STATE_CHANGED__GW_TEARDOWN_REASON__GATEWAY_TEARDOWN_REASON_NETWORK_AGENT_UNWANTED; import static com.android.server.vcn.metrics.VcnStatsLog.VCN_GATEWAY_CONNECTION_STATE_CHANGED__GW_TEARDOWN_REASON__GATEWAY_TEARDOWN_REASON_NONE; +import static com.android.server.vcn.metrics.VcnStatsLog.VCN_GATEWAY_CONNECTION_STATE_CHANGED__GW_TEARDOWN_REASON__GATEWAY_TEARDOWN_REASON_REQUESTED; +import static com.android.server.vcn.metrics.VcnStatsLog.VCN_GATEWAY_CONNECTION_STATE_CHANGED__GW_TEARDOWN_REASON__GATEWAY_TEARDOWN_REASON_UNSPECIFIED; + +import android.annotation.IntDef; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; /** Utility class for logging VCN metrics. */ public class VcnMetrics { + /** @hide */ + @Retention(RetentionPolicy.SOURCE) + @IntDef( + prefix = {"GATEWAY_TEARDOWN_REASON_"}, + value = { + GATEWAY_TEARDOWN_REASON_NONE, + GATEWAY_TEARDOWN_REASON_INTERNAL_ERROR, + GATEWAY_TEARDOWN_REASON_NETWORK_AGENT_UNWANTED, + GATEWAY_TEARDOWN_REASON_REQUESTED + }) + public @interface GatewayTeardownReason {} - private static final int GATEWAY_TEARDOWN_REASON_NONE = + public static final int GATEWAY_TEARDOWN_REASON_NONE = VCN_GATEWAY_CONNECTION_STATE_CHANGED__GW_TEARDOWN_REASON__GATEWAY_TEARDOWN_REASON_NONE; + public static final int GATEWAY_TEARDOWN_REASON_INTERNAL_ERROR = + VCN_GATEWAY_CONNECTION_STATE_CHANGED__GW_TEARDOWN_REASON__GATEWAY_TEARDOWN_REASON_INTERNAL_ERROR; + public static final int GATEWAY_TEARDOWN_REASON_NETWORK_AGENT_UNWANTED = + VCN_GATEWAY_CONNECTION_STATE_CHANGED__GW_TEARDOWN_REASON__GATEWAY_TEARDOWN_REASON_NETWORK_AGENT_UNWANTED; + public static final int GATEWAY_TEARDOWN_REASON_REQUESTED = + VCN_GATEWAY_CONNECTION_STATE_CHANGED__GW_TEARDOWN_REASON__GATEWAY_TEARDOWN_REASON_REQUESTED; + public static final int GATEWAY_TEARDOWN_REASON_UNSPECIFIED = + VCN_GATEWAY_CONNECTION_STATE_CHANGED__GW_TEARDOWN_REASON__GATEWAY_TEARDOWN_REASON_UNSPECIFIED; /** Log an atom when a VcnGatewayConnection has entered safe mode. */ public void logEnterSafeMode(int gatewayConnectionId) { VcnStatsLog.write( - VcnStatsLog.VCN_GATEWAY_CONNECTION_STATE_CHANGED, + VCN_GATEWAY_CONNECTION_STATE_CHANGED, gatewayConnectionId, GATEWAY_TEARDOWN_REASON_NONE, true /* isInSafeMode */); @@ -42,6 +71,18 @@ public class VcnMetrics { false /* isInSafeMode */); } + /** + * Log an atom when a VcnGatewayConnection has been torn down with reason. It will also reset + * other VcnGatewayConnection related states i.e. safemode. + */ + public void logVcnGatewayTeardown(int gatewayConnectionId, @GatewayTeardownReason int reason) { + VcnStatsLog.write( + VcnStatsLog.VCN_GATEWAY_CONNECTION_STATE_CHANGED, + gatewayConnectionId, + reason, + false /* isInSafeMode */); + } + /** Log an atom when VCN network is connected. */ public void logVcnNetworkConnected(int gatewayConnectionId, int networkId) { VcnStatsLog.write( @@ -52,7 +93,10 @@ public class VcnMetrics { false /* isValidated */); } - /** Log an atom when VCN network is being torn down. */ + /** + * Log an atom when VCN network is being torn down. It will also reset other state related to + * the network i.e. validated. + */ public void logVcnNetworkNotConnected(int gatewayConnectionId, int networkId) { VcnStatsLog.write( VcnStatsLog.VCN_NETWORK_STATE_CHANGED, diff --git a/Vcn/tests/unit/java/com/android/server/vcn/VcnGatewayConnectionDisconnectedStateTest.java b/Vcn/tests/unit/java/com/android/server/vcn/VcnGatewayConnectionDisconnectedStateTest.java index 0b470b9a35..7e9440a7f7 100644 --- a/Vcn/tests/unit/java/com/android/server/vcn/VcnGatewayConnectionDisconnectedStateTest.java +++ b/Vcn/tests/unit/java/com/android/server/vcn/VcnGatewayConnectionDisconnectedStateTest.java @@ -25,6 +25,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -34,6 +35,8 @@ import android.net.IpSecManager; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; +import com.android.server.vcn.metrics.VcnMetrics; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -96,22 +99,6 @@ public class VcnGatewayConnectionDisconnectedStateTest extends VcnGatewayConnect verifyDisconnectRequestAlarmAndGetCallback(false /* expectCanceled */); } - @Test - public void testTeardown() throws Exception { - mGatewayConnection.teardownAsynchronously(); - mTestLooper.dispatchAll(); - - // Verify that sending a non-quitting disconnect request does not unset the isQuitting flag - mGatewayConnection.sendDisconnectRequestedAndAcquireWakelock("TEST", false); - mTestLooper.dispatchAll(); - - assertNull(mGatewayConnection.getCurrentState()); - verify(mIpSecSvc).deleteTunnelInterface(eq(TEST_IPSEC_TUNNEL_RESOURCE_ID), any()); - verifySafeModeTimeoutAlarmAndGetCallback(true /* expectCanceled */); - assertTrue(mGatewayConnection.isQuitting()); - verify(mGatewayStatusCallback).onQuit(); - } - @Test public void testNonTeardownDisconnectRequest() throws Exception { mGatewayConnection.sendDisconnectRequestedAndAcquireWakelock("TEST", false); @@ -120,6 +107,39 @@ public class VcnGatewayConnectionDisconnectedStateTest extends VcnGatewayConnect assertEquals(mGatewayConnection.mDisconnectedState, mGatewayConnection.getCurrentState()); assertFalse(mGatewayConnection.isQuitting()); verify(mGatewayStatusCallback, never()).onQuit(); + verify(mVcnMetrics, never()).logVcnGatewayTeardown(anyInt(), anyInt()); // No safe mode timer changes expected. } + + @Test + public void testTeardownDisconnectRequest() throws Exception { + mGatewayConnection.sendDisconnectRequestedAndAcquireWakelock("TEST", true); + mTestLooper.dispatchAll(); + + assertNull(mGatewayConnection.getCurrentState()); + verify(mIpSecSvc).deleteTunnelInterface(eq(TEST_IPSEC_TUNNEL_RESOURCE_ID), any()); + verifySafeModeTimeoutAlarmAndGetCallback(true /* expectCanceled */); + assertTrue(mGatewayConnection.isQuitting()); + verify(mGatewayStatusCallback).onQuit(); + verify(mVcnMetrics).logVcnGatewayTeardown(anyInt(), anyInt()); + } + + @Test + public void testTeardown_subsequentNonTeardown_stillIsQuitting() throws Exception { + mGatewayConnection.teardownAsynchronously(); + mTestLooper.dispatchAll(); + assertEquals( + mGatewayConnection.getTeardownReason(), + VcnMetrics.GATEWAY_TEARDOWN_REASON_REQUESTED); + + // Verify that sending a non-quitting disconnect request does not unset the isQuitting flag + mGatewayConnection.sendDisconnectRequestedAndAcquireWakelock("TEST", false); + mTestLooper.dispatchAll(); + + assertNull(mGatewayConnection.getCurrentState()); + assertTrue(mGatewayConnection.isQuitting()); + assertEquals( + mGatewayConnection.getTeardownReason(), + VcnMetrics.GATEWAY_TEARDOWN_REASON_REQUESTED); + } }