From afe26600fb0c6a5e3bc1cfcbd0428784761b7a59 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Mon, 2 Nov 2020 17:41:17 +0900 Subject: [PATCH] Migrate away from AsyncChannel in NetworkAgent Use two oneway binder interfaces instead. The interfaces post messages to handlers as was implemented before, but provide a more strictly defined interface, with less hops between NetworkAgent, AsyncChannel, and ConnectivityService. Exempt-From-Owner-Approval: Owners OOO, change approved by team members Ignore-AOSP-First: merge conflicts in dependent changes Test: atest FrameworksNetTests CtsNetTestCasesLatestSdk Change-Id: Ica51d0179bcb3b4e314d2c3e85709aead6ca5657 --- framework/Android.bp | 29 +++++ .../connectivity/aidl/INetworkAgent.aidl | 46 +++++++ .../aidl/INetworkAgentRegistry.aidl | 36 ++++++ .../src/android/net/cts/NetworkAgentTest.kt | 117 +++++++----------- 4 files changed, 156 insertions(+), 72 deletions(-) create mode 100644 framework/Android.bp create mode 100644 framework/src/com/android/connectivity/aidl/INetworkAgent.aidl create mode 100644 framework/src/com/android/connectivity/aidl/INetworkAgentRegistry.aidl diff --git a/framework/Android.bp b/framework/Android.bp new file mode 100644 index 0000000000..8db8d7699a --- /dev/null +++ b/framework/Android.bp @@ -0,0 +1,29 @@ +// +// 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. +// + +// TODO: use a java_library in the bootclasspath instead +filegroup { + name: "framework-connectivity-sources", + srcs: [ + "src/**/*.java", + "src/**/*.aidl", + ], + path: "src", + visibility: [ + "//frameworks/base", + "//packages/modules/Connectivity:__subpackages__", + ], +} \ No newline at end of file diff --git a/framework/src/com/android/connectivity/aidl/INetworkAgent.aidl b/framework/src/com/android/connectivity/aidl/INetworkAgent.aidl new file mode 100644 index 0000000000..1af9e769b7 --- /dev/null +++ b/framework/src/com/android/connectivity/aidl/INetworkAgent.aidl @@ -0,0 +1,46 @@ +/** + * 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 perNmissions and + * limitations under the License. + */ +package com.android.connectivity.aidl; + +import android.net.NattKeepalivePacketData; +import android.net.TcpKeepalivePacketData; + +import com.android.connectivity.aidl.INetworkAgentRegistry; + +/** + * Interface to notify NetworkAgent of connectivity events. + * @hide + */ +oneway interface INetworkAgent { + void onRegistered(in INetworkAgentRegistry registry); + void onDisconnected(); + void onBandwidthUpdateRequested(); + void onValidationStatusChanged(int validationStatus, + in @nullable String captivePortalUrl); + void onSaveAcceptUnvalidated(boolean acceptUnvalidated); + void onStartNattSocketKeepalive(int slot, int intervalDurationMs, + in NattKeepalivePacketData packetData); + void onStartTcpSocketKeepalive(int slot, int intervalDurationMs, + in TcpKeepalivePacketData packetData); + void onStopSocketKeepalive(int slot); + void onSignalStrengthThresholdsUpdated(in int[] thresholds); + void onPreventAutomaticReconnect(); + void onAddNattKeepalivePacketFilter(int slot, + in NattKeepalivePacketData packetData); + void onAddTcpKeepalivePacketFilter(int slot, + in TcpKeepalivePacketData packetData); + void onRemoveKeepalivePacketFilter(int slot); +} diff --git a/framework/src/com/android/connectivity/aidl/INetworkAgentRegistry.aidl b/framework/src/com/android/connectivity/aidl/INetworkAgentRegistry.aidl new file mode 100644 index 0000000000..d42a34055c --- /dev/null +++ b/framework/src/com/android/connectivity/aidl/INetworkAgentRegistry.aidl @@ -0,0 +1,36 @@ +/** + * 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 perNmissions and + * limitations under the License. + */ +package com.android.connectivity.aidl; + +import android.net.LinkProperties; +import android.net.Network; +import android.net.NetworkCapabilities; +import android.net.NetworkInfo; + +/** + * Interface for NetworkAgents to send network network properties. + * @hide + */ +oneway interface INetworkAgentRegistry { + void sendNetworkCapabilities(in NetworkCapabilities nc); + void sendLinkProperties(in LinkProperties lp); + // TODO: consider replacing this by "markConnected()" and removing + void sendNetworkInfo(in NetworkInfo info); + void sendScore(int score); + void sendExplicitlySelected(boolean explicitlySelected, boolean acceptPartial); + void sendSocketKeepaliveEvent(int slot, int reason); + void sendUnderlyingNetworks(in @nullable List networks); +} diff --git a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt index 87aa2a3110..803c9d804f 100644 --- a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt +++ b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt @@ -23,15 +23,9 @@ import android.net.IpPrefix import android.net.KeepalivePacketData import android.net.LinkAddress import android.net.LinkProperties +import android.net.NattKeepalivePacketData import android.net.Network import android.net.NetworkAgent -import android.net.NetworkAgent.CMD_ADD_KEEPALIVE_PACKET_FILTER -import android.net.NetworkAgent.CMD_PREVENT_AUTOMATIC_RECONNECT -import android.net.NetworkAgent.CMD_REMOVE_KEEPALIVE_PACKET_FILTER -import android.net.NetworkAgent.CMD_REPORT_NETWORK_STATUS -import android.net.NetworkAgent.CMD_SAVE_ACCEPT_UNVALIDATED -import android.net.NetworkAgent.CMD_START_SOCKET_KEEPALIVE -import android.net.NetworkAgent.CMD_STOP_SOCKET_KEEPALIVE import android.net.NetworkAgent.INVALID_NETWORK import android.net.NetworkAgent.VALID_NETWORK import android.net.NetworkAgentConfig @@ -64,16 +58,14 @@ import android.net.cts.NetworkAgentTest.TestableNetworkAgent.CallbackEntry.OnSta import android.net.cts.NetworkAgentTest.TestableNetworkAgent.CallbackEntry.OnStopSocketKeepalive import android.net.cts.NetworkAgentTest.TestableNetworkAgent.CallbackEntry.OnValidationStatus import android.os.Build -import android.os.Bundle -import android.os.Handler import android.os.HandlerThread import android.os.Looper import android.os.Message -import android.os.Messenger import android.util.DebugUtils.valueToString import androidx.test.InstrumentationRegistry import androidx.test.runner.AndroidJUnit4 -import com.android.internal.util.AsyncChannel +import com.android.connectivity.aidl.INetworkAgent +import com.android.connectivity.aidl.INetworkAgentRegistry import com.android.net.module.util.ArrayTrackRecord import com.android.testutils.DevSdkIgnoreRule import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo @@ -82,7 +74,6 @@ import com.android.testutils.RecorderCallback.CallbackEntry.Lost import com.android.testutils.TestableNetworkCallback import org.junit.After import org.junit.Assert.assertArrayEquals -import org.junit.Assert.fail import org.junit.Before import org.junit.Rule import org.junit.Test @@ -93,6 +84,7 @@ import org.mockito.ArgumentMatchers.argThat import org.mockito.ArgumentMatchers.eq import org.mockito.Mockito.doReturn import org.mockito.Mockito.mock +import org.mockito.Mockito.timeout import org.mockito.Mockito.verify import java.time.Duration import java.util.Arrays @@ -103,6 +95,7 @@ import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue +import kotlin.test.fail // This test doesn't really have a constraint on how fast the methods should return. If it's // going to fail, it will simply wait forever, so setting a high timeout lowers the flake ratio @@ -140,7 +133,7 @@ class NetworkAgentTest { private val mCM = realContext.getSystemService(ConnectivityManager::class.java) private val mHandlerThread = HandlerThread("${javaClass.simpleName} handler thread") - private val mFakeConnectivityService by lazy { FakeConnectivityService(mHandlerThread.looper) } + private val mFakeConnectivityService = FakeConnectivityService() private class Provider(context: Context, looper: Looper) : NetworkProvider(context, looper, "NetworkAgentTest NetworkProvider") @@ -167,39 +160,30 @@ class NetworkAgentTest { * This fake only supports speaking to one harnessed agent at a time because it * only keeps track of one async channel. */ - private class FakeConnectivityService(looper: Looper) { - private val CMD_EXPECT_DISCONNECT = 1 - private var disconnectExpected = false - private val msgHistory = ArrayTrackRecord().newReadHead() - private val asyncChannel = AsyncChannel() - private val handler = object : Handler(looper) { - override fun handleMessage(msg: Message) { - msgHistory.add(Message.obtain(msg)) // make a copy as the original will be recycled - when (msg.what) { - CMD_EXPECT_DISCONNECT -> disconnectExpected = true - AsyncChannel.CMD_CHANNEL_HALF_CONNECTED -> - asyncChannel.sendMessage(AsyncChannel.CMD_CHANNEL_FULL_CONNECTION) - AsyncChannel.CMD_CHANNEL_DISCONNECTED -> - if (!disconnectExpected) { - fail("Agent unexpectedly disconnected") - } else { - disconnectExpected = false - } - } - } + private class FakeConnectivityService { + val mockRegistry = mock(INetworkAgentRegistry::class.java) + private var agentField: INetworkAgent? = null + private val registry = object : INetworkAgentRegistry.Stub(), + INetworkAgentRegistry by mockRegistry { + // asBinder has implementations in both INetworkAgentRegistry.Stub and mockRegistry, so + // it needs to be disambiguated. Just fail the test as it should be unused here. + // asBinder is used when sending the registry in binder transactions, so not in this + // test (the test just uses in-process direct calls). If it were used across processes, + // using the Stub super.asBinder() implementation would allow sending the registry in + // binder transactions, while recording incoming calls on the other mockito-generated + // methods. + override fun asBinder() = fail("asBinder should be unused in this test") } - fun connect(agentMsngr: Messenger) = asyncChannel.connect(realContext, handler, agentMsngr) + val agent: INetworkAgent + get() = agentField ?: fail("No INetworkAgent") - fun disconnect() = asyncChannel.disconnect() + fun connect(agent: INetworkAgent) { + this.agentField = agent + agent.onRegistered(registry) + } - fun sendMessage(what: Int, arg1: Int = 0, arg2: Int = 0, obj: Any? = null) = - asyncChannel.sendMessage(Message(what, arg1, arg2, obj)) - - fun expectMessage(what: Int) = - assertNotNull(msgHistory.poll(DEFAULT_TIMEOUT_MS) { it.what == what }) - - fun willExpectDisconnectOnce() = handler.sendEmptyMessage(CMD_EXPECT_DISCONNECT) + fun disconnect() = agent.onDisconnected() } private open class TestableNetworkAgent( @@ -445,17 +429,15 @@ class NetworkAgentTest { @Test fun testSocketKeepalive(): Unit = createNetworkAgentWithFakeCS().let { agent -> - val packet = object : KeepalivePacketData( + val packet = NattKeepalivePacketData( LOCAL_IPV4_ADDRESS /* srcAddress */, 1234 /* srcPort */, REMOTE_IPV4_ADDRESS /* dstAddress */, 4567 /* dstPort */, - ByteArray(100 /* size */) { it.toByte() /* init */ }) {} + ByteArray(100 /* size */)) val slot = 4 val interval = 37 - mFakeConnectivityService.sendMessage(CMD_ADD_KEEPALIVE_PACKET_FILTER, - arg1 = slot, obj = packet) - mFakeConnectivityService.sendMessage(CMD_START_SOCKET_KEEPALIVE, - arg1 = slot, arg2 = interval, obj = packet) + mFakeConnectivityService.agent.onAddNattKeepalivePacketFilter(slot, packet) + mFakeConnectivityService.agent.onStartNattSocketKeepalive(slot, interval, packet) agent.expectCallback().let { assertEquals(it.slot, slot) @@ -472,13 +454,11 @@ class NetworkAgentTest { // Check that when the agent sends a keepalive event, ConnectivityService receives the // expected message. agent.sendSocketKeepaliveEvent(slot, SocketKeepalive.ERROR_UNSUPPORTED) - mFakeConnectivityService.expectMessage(NetworkAgent.EVENT_SOCKET_KEEPALIVE).let() { - assertEquals(slot, it.arg1) - assertEquals(SocketKeepalive.ERROR_UNSUPPORTED, it.arg2) - } + verify(mFakeConnectivityService.mockRegistry, timeout(DEFAULT_TIMEOUT_MS)) + .sendSocketKeepaliveEvent(slot, SocketKeepalive.ERROR_UNSUPPORTED) - mFakeConnectivityService.sendMessage(CMD_STOP_SOCKET_KEEPALIVE, arg1 = slot) - mFakeConnectivityService.sendMessage(CMD_REMOVE_KEEPALIVE_PACKET_FILTER, arg1 = slot) + mFakeConnectivityService.agent.onStopSocketKeepalive(slot) + mFakeConnectivityService.agent.onRemoveKeepalivePacketFilter(slot) agent.expectCallback().let { assertEquals(it.slot, slot) } @@ -639,7 +619,7 @@ class NetworkAgentTest { val mockCm = mock(ConnectivityManager::class.java) doReturn(mockCm).`when`(mockContext).getSystemService(Context.CONNECTIVITY_SERVICE) createConnectedNetworkAgent(mockContext) - verify(mockCm).registerNetworkAgent(any(Messenger::class.java), + verify(mockCm).registerNetworkAgent(any(), argThat { it.detailedState == NetworkInfo.DetailedState.CONNECTING }, any(LinkProperties::class.java), any(NetworkCapabilities::class.java), @@ -651,7 +631,7 @@ class NetworkAgentTest { @Test fun testSetAcceptUnvalidated() { createNetworkAgentWithFakeCS().let { agent -> - mFakeConnectivityService.sendMessage(CMD_SAVE_ACCEPT_UNVALIDATED, 1) + mFakeConnectivityService.agent.onSaveAcceptUnvalidated(true) agent.expectCallback().let { assertTrue(it.accept) } @@ -662,19 +642,18 @@ class NetworkAgentTest { @Test fun testSetAcceptUnvalidatedPreventAutomaticReconnect() { createNetworkAgentWithFakeCS().let { agent -> - mFakeConnectivityService.sendMessage(CMD_SAVE_ACCEPT_UNVALIDATED, 0) - mFakeConnectivityService.sendMessage(CMD_PREVENT_AUTOMATIC_RECONNECT) + mFakeConnectivityService.agent.onSaveAcceptUnvalidated(false) + mFakeConnectivityService.agent.onPreventAutomaticReconnect() agent.expectCallback().let { assertFalse(it.accept) } agent.expectCallback() agent.assertNoCallback() // When automatic reconnect is turned off, the network is torn down and - // ConnectivityService sends a disconnect. This in turn causes the agent - // to send a DISCONNECTED message to CS. - mFakeConnectivityService.willExpectDisconnectOnce() + // ConnectivityService disconnects. As part of the disconnect, ConnectivityService will + // also send itself a message to unregister the NetworkAgent from its internal + // structure. mFakeConnectivityService.disconnect() - mFakeConnectivityService.expectMessage(AsyncChannel.CMD_CHANNEL_DISCONNECTED) agent.expectCallback() } } @@ -682,12 +661,10 @@ class NetworkAgentTest { @Test fun testPreventAutomaticReconnect() { createNetworkAgentWithFakeCS().let { agent -> - mFakeConnectivityService.sendMessage(CMD_PREVENT_AUTOMATIC_RECONNECT) + mFakeConnectivityService.agent.onPreventAutomaticReconnect() agent.expectCallback() agent.assertNoCallback() - mFakeConnectivityService.willExpectDisconnectOnce() mFakeConnectivityService.disconnect() - mFakeConnectivityService.expectMessage(AsyncChannel.CMD_CHANNEL_DISCONNECTED) agent.expectCallback() } } @@ -695,18 +672,14 @@ class NetworkAgentTest { @Test fun testValidationStatus() = createNetworkAgentWithFakeCS().let { agent -> val uri = Uri.parse("http://www.google.com") - val bundle = Bundle().apply { - putString(NetworkAgent.REDIRECT_URL_KEY, uri.toString()) - } - mFakeConnectivityService.sendMessage(CMD_REPORT_NETWORK_STATUS, - arg1 = VALID_NETWORK, obj = bundle) + mFakeConnectivityService.agent.onValidationStatusChanged(VALID_NETWORK, + uri.toString()) agent.expectCallback().let { assertEquals(it.status, VALID_NETWORK) assertEquals(it.uri, uri) } - mFakeConnectivityService.sendMessage(CMD_REPORT_NETWORK_STATUS, - arg1 = INVALID_NETWORK, obj = Bundle()) + mFakeConnectivityService.agent.onValidationStatusChanged(INVALID_NETWORK, null) agent.expectCallback().let { assertEquals(it.status, INVALID_NETWORK) assertNull(it.uri)