Merge changes I3250bdb9,I20f6e5e1,I70c96a09,I88f77bd4,If6cefa51, ...

* changes:
  ethernet: rename methods in EthernetNetworkFactory
  ethernet: move registerNetworkOffer into its own function
  ethernet: provider to keep track of requestId
  ethernet: return result when removing interface
  ethernet: enable testCallbacks_forServerModeInterfaces on CF
  ethernet: expand server mode callbacks test
  ethernet: add test for removing interface while in server mode
This commit is contained in:
Patrick Rohr
2022-07-01 23:03:24 +00:00
committed by Gerrit Code Review
2 changed files with 85 additions and 41 deletions

View File

@@ -60,7 +60,9 @@ import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
/**
* {@link NetworkProvider} that manages NetworkOffers for Ethernet networks.
* Class that manages NetworkOffers for Ethernet networks.
*
* TODO: this class should be merged into EthernetTracker.
*/
public class EthernetNetworkFactory {
private final static String TAG = EthernetNetworkFactory.class.getSimpleName();
@@ -221,11 +223,17 @@ public class EthernetNetworkFactory {
}
@VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
protected void removeInterface(String interfaceName) {
protected boolean removeInterface(String interfaceName) {
NetworkInterfaceState iface = mTrackingInterfaces.remove(interfaceName);
if (iface != null) {
iface.destroy();
iface.unregisterNetworkOfferAndStop();
return true;
}
// TODO(b/236892130): if an interface is currently in server mode, it may not be properly
// removed.
// TODO: when false is returned, do not send a STATE_ABSENT callback.
Log.w(TAG, interfaceName + " is not tracked and cannot be removed");
return false;
}
/** Returns true if state has been modified */
@@ -292,7 +300,7 @@ public class EthernetNetworkFactory {
private boolean mLinkUp;
private int mLegacyType;
private LinkProperties mLinkProperties = new LinkProperties();
private Set<NetworkRequest> mRequests = new ArraySet<>();
private final Set<Integer> mRequestIds = new ArraySet<>();
private volatile @Nullable IpClientManager mIpClient;
private @NonNull NetworkCapabilities mCapabilities;
@@ -401,7 +409,7 @@ public class EthernetNetworkFactory {
// existing requests.
// ConnectivityService filters requests for us based on the NetworkCapabilities
// passed in the registerNetworkOffer() call.
mRequests.add(request);
mRequestIds.add(request.requestId);
// if the network is already started, this is a no-op.
start();
}
@@ -412,8 +420,12 @@ public class EthernetNetworkFactory {
Log.d(TAG,
String.format("%s: onNetworkUnneeded for request: %s", name, request));
}
mRequests.remove(request);
if (mRequests.isEmpty()) {
if (!mRequestIds.remove(request.requestId)) {
// This can only happen if onNetworkNeeded was not called for a request or if
// the requestId changed. Both should *never* happen.
Log.wtf(TAG, "onNetworkUnneeded called for unknown request");
}
if (mRequestIds.isEmpty()) {
// not currently serving any requests, stop the network.
stop();
}
@@ -454,7 +466,7 @@ public class EthernetNetworkFactory {
+ "transport type.");
}
private static NetworkScore getBestNetworkScore() {
private static NetworkScore getNetworkScore() {
return new NetworkScore.Builder().build();
}
@@ -465,9 +477,7 @@ public class EthernetNetworkFactory {
if (mLinkUp) {
// registering a new network offer will update the existing one, not install a
// new one.
mNetworkProvider.registerNetworkOffer(getBestNetworkScore(),
new NetworkCapabilities(capabilities), cmd -> mHandler.post(cmd),
mNetworkOfferCallback);
registerNetworkOffer();
}
}
@@ -629,14 +639,12 @@ public class EthernetNetworkFactory {
if (!up) { // was up, goes down
// retract network offer and stop IpClient.
destroy();
unregisterNetworkOfferAndStop();
// If only setting the interface down, send a callback to signal completion.
EthernetNetworkFactory.maybeSendNetworkManagementCallback(listener, name, null);
} else { // was down, goes up
// register network offer
mNetworkProvider.registerNetworkOffer(getBestNetworkScore(),
new NetworkCapabilities(mCapabilities), (cmd) -> mHandler.post(cmd),
mNetworkOfferCallback);
registerNetworkOffer();
}
return true;
@@ -660,10 +668,16 @@ public class EthernetNetworkFactory {
mLinkProperties.clear();
}
public void destroy() {
private void registerNetworkOffer() {
mNetworkProvider.registerNetworkOffer(getNetworkScore(),
new NetworkCapabilities(mCapabilities), cmd -> mHandler.post(cmd),
mNetworkOfferCallback);
}
public void unregisterNetworkOfferAndStop() {
mNetworkProvider.unregisterNetworkOffer(mNetworkOfferCallback);
stop();
mRequests.clear();
mRequestIds.clear();
}
private static void provisionIpClient(@NonNull final IpClientManager ipClient,

View File

@@ -49,7 +49,6 @@ import android.os.Build
import android.os.Handler
import android.os.Looper
import android.os.OutcomeReceiver
import android.os.SystemProperties
import android.platform.test.annotations.AppModeFull
import android.util.ArraySet
import androidx.test.platform.app.InstrumentationRegistry
@@ -69,13 +68,13 @@ import com.android.testutils.runAsShell
import com.android.testutils.waitForIdle
import org.junit.After
import org.junit.Assume.assumeTrue
import org.junit.Assume.assumeFalse
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import java.net.Inet6Address
import java.util.concurrent.CompletableFuture
import java.util.concurrent.ExecutionException
import java.util.concurrent.TimeoutException
import java.util.concurrent.TimeUnit
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
@@ -206,12 +205,8 @@ class EthernetManagerTest {
fun eventuallyExpect(expected: CallbackEntry) = events.poll(TIMEOUT_MS) { it == expected }
fun eventuallyExpect(interfaceName: String, state: Int, role: Int) {
assertNotNull(eventuallyExpect(createChangeEvent(interfaceName, state, role)))
}
fun eventuallyExpect(iface: EthernetTestInterface, state: Int, role: Int) {
eventuallyExpect(iface.name, state, role)
assertNotNull(eventuallyExpect(createChangeEvent(iface.name, state, role)))
}
fun assertNoCallback() {
@@ -458,32 +453,45 @@ class EthernetManagerTest {
}
}
// TODO: this function is now used in two places (EthernetManagerTest and
// EthernetTetheringTest), so it should be moved to testutils.
private fun isAdbOverNetwork(): Boolean {
// If adb TCP port opened, this test may running by adb over network.
return (SystemProperties.getInt("persist.adb.tcp.port", -1) > -1 ||
SystemProperties.getInt("service.adb.tcp.port", -1) > -1)
private fun assumeNoInterfaceForTetheringAvailable() {
// Interfaces that have configured NetworkCapabilities will never be used for tethering,
// see aosp/2123900.
try {
// assumeException does not exist.
requestTetheredInterface().expectOnAvailable()
// interface used for tethering is available, throw an assumption error.
assumeTrue(false)
} catch (e: TimeoutException) {
// do nothing -- the TimeoutException indicates that no interface is available for
// tethering.
releaseTetheredInterface()
}
}
@Test
fun testCallbacks_forServerModeInterfaces() {
// do not run this test when adb might be connected over ethernet.
assumeFalse(isAdbOverNetwork())
// do not run this test if an interface that can be used for tethering already exists.
assumeNoInterfaceForTetheringAvailable()
val iface = createInterface()
requestTetheredInterface().expectOnAvailable()
val listener = EthernetStateListener()
addInterfaceStateListener(listener)
// it is possible that a physical interface is present, so it is not guaranteed that iface
// will be put into server mode. This should not matter for the test though. Calling
// createInterface() makes sure we have at least one interface available.
val iface = createInterface()
val cb = requestTetheredInterface()
val ifaceName = cb.expectOnAvailable()
listener.eventuallyExpect(ifaceName, STATE_LINK_UP, ROLE_SERVER)
// TODO(b/236895792): THIS IS A BUG! Existing server mode interfaces are not reported when
// an InterfaceStateListener is registered.
// Note: using eventuallyExpect as there may be other interfaces present.
// listener.eventuallyExpect(iface, STATE_LINK_UP, ROLE_SERVER)
releaseTetheredInterface()
listener.eventuallyExpect(ifaceName, STATE_LINK_UP, ROLE_CLIENT)
listener.eventuallyExpect(iface, STATE_LINK_UP, ROLE_CLIENT)
requestTetheredInterface().expectOnAvailable()
// This should be changed to expectCallback, once b/236895792 is fixed.
listener.eventuallyExpect(iface, STATE_LINK_UP, ROLE_SERVER)
releaseTetheredInterface()
listener.expectCallback(iface, STATE_LINK_UP, ROLE_CLIENT)
}
/**
@@ -651,4 +659,26 @@ class EthernetManagerTest {
iface.setCarrierEnabled(false)
cb.eventuallyExpectLost()
}
@Test
fun testRemoveInterface_whileInServerMode() {
assumeNoInterfaceForTetheringAvailable()
val listener = EthernetStateListener()
addInterfaceStateListener(listener)
val iface = createInterface()
val ifaceName = requestTetheredInterface().expectOnAvailable()
assertEquals(iface.name, ifaceName)
listener.eventuallyExpect(iface, STATE_LINK_UP, ROLE_SERVER)
removeInterface(iface)
// Note: removeInterface already verifies that a STATE_ABSENT, ROLE_NONE callback is
// received, but it can't hurt to explicitly check for it.
listener.expectCallback(iface, STATE_ABSENT, ROLE_NONE)
releaseTetheredInterface()
listener.assertNoCallback()
}
}