Merge "Fixing multithreading issues in Ethernet Factory"
diff --git a/java/com/android/server/ethernet/EthernetNetworkFactory.java b/java/com/android/server/ethernet/EthernetNetworkFactory.java
index 9c3148b..d910629 100644
--- a/java/com/android/server/ethernet/EthernetNetworkFactory.java
+++ b/java/com/android/server/ethernet/EthernetNetworkFactory.java
@@ -183,7 +183,8 @@
* Returns an array of available interface names. The array is sorted: unrestricted interfaces
* goes first, then sorted by name.
*/
- String[] getAvailableInterfaces(boolean includeRestricted) {
+ @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
+ protected String[] getAvailableInterfaces(boolean includeRestricted) {
return mTrackingInterfaces.values()
.stream()
.filter(iface -> !iface.isRestricted() || includeRestricted)
@@ -195,7 +196,8 @@
.toArray(String[]::new);
}
- void addInterface(@NonNull final String ifaceName, @NonNull final String hwAddress,
+ @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
+ protected void addInterface(@NonNull final String ifaceName, @NonNull final String hwAddress,
@NonNull final IpConfiguration ipConfig,
@NonNull final NetworkCapabilities capabilities) {
if (mTrackingInterfaces.containsKey(ifaceName)) {
@@ -282,7 +284,8 @@
.addTransportType(NetworkCapabilities.TRANSPORT_ETHERNET).build();
}
- void removeInterface(String interfaceName) {
+ @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
+ protected void removeInterface(String interfaceName) {
NetworkInterfaceState iface = mTrackingInterfaces.remove(interfaceName);
if (iface != null) {
iface.maybeSendNetworkManagementCallbackForAbort();
diff --git a/java/com/android/server/ethernet/EthernetServiceImpl.java b/java/com/android/server/ethernet/EthernetServiceImpl.java
index edda321..5e830ad 100644
--- a/java/com/android/server/ethernet/EthernetServiceImpl.java
+++ b/java/com/android/server/ethernet/EthernetServiceImpl.java
@@ -41,6 +41,7 @@
import java.io.FileDescriptor;
import java.io.PrintWriter;
+import java.util.List;
import java.util.Objects;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -281,4 +282,18 @@
mTracker.disconnectNetwork(iface, listener);
}
+
+ @Override
+ public void setEthernetEnabled(boolean enabled) {
+ PermissionUtils.enforceNetworkStackPermissionOr(mContext,
+ android.Manifest.permission.NETWORK_SETTINGS);
+
+ mTracker.setEthernetEnabled(enabled);
+ }
+
+ @Override
+ public List<String> getInterfaceList() {
+ PermissionUtils.enforceAccessNetworkStatePermission(mContext, TAG);
+ return mTracker.getInterfaceList();
+ }
}
diff --git a/java/com/android/server/ethernet/EthernetTracker.java b/java/com/android/server/ethernet/EthernetTracker.java
index 0a0e327..c291b3f 100644
--- a/java/com/android/server/ethernet/EthernetTracker.java
+++ b/java/com/android/server/ethernet/EthernetTracker.java
@@ -16,6 +16,8 @@
package com.android.server.ethernet;
+import static android.net.EthernetManager.ETHERNET_STATE_DISABLED;
+import static android.net.EthernetManager.ETHERNET_STATE_ENABLED;
import static android.net.TestNetworkManager.TEST_TAP_PREFIX;
import static com.android.internal.annotations.VisibleForTesting.Visibility.PACKAGE;
@@ -55,6 +57,7 @@
import java.io.FileDescriptor;
import java.net.InetAddress;
import java.util.ArrayList;
+import java.util.List;
import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
@@ -88,7 +91,7 @@
* Interface names we track. This is a product-dependent regular expression, plus,
* if setIncludeTestInterfaces is true, any test interfaces.
*/
- private String mIfaceMatch;
+ private volatile String mIfaceMatch;
/**
* Track test interfaces if true, don't track otherwise.
*/
@@ -119,6 +122,8 @@
private boolean mTetheredInterfaceWasAvailable = false;
private volatile IpConfiguration mIpConfigForDefaultInterface;
+ private int mEthernetState = ETHERNET_STATE_ENABLED;
+
private class TetheredInterfaceRequestList extends
RemoteCallbackList<ITetheredInterfaceCallback> {
@Override
@@ -337,6 +342,22 @@
return mFactory.getAvailableInterfaces(includeRestricted);
}
+ List<String> getInterfaceList() {
+ final List<String> interfaceList = new ArrayList<String>();
+ final String[] ifaces;
+ try {
+ ifaces = mNetd.interfaceGetList();
+ } catch (RemoteException e) {
+ Log.e(TAG, "Could not get list of interfaces " + e);
+ return interfaceList;
+ }
+ final String ifaceMatch = mIfaceMatch;
+ for (String iface : ifaces) {
+ if (iface.matches(ifaceMatch)) interfaceList.add(iface);
+ }
+ return interfaceList;
+ }
+
/**
* Returns true if given interface was configured as restricted (doesn't have
* NET_CAPABILITY_NOT_RESTRICTED) capability. Otherwise, returns false.
@@ -355,6 +376,8 @@
for (String iface : getInterfaces(canUseRestrictedNetworks)) {
unicastInterfaceStateChange(listener, iface);
}
+
+ unicastEthernetStateChange(listener, mEthernetState);
});
}
@@ -825,6 +848,53 @@
}
}
+ @VisibleForTesting(visibility = PACKAGE)
+ protected void setEthernetEnabled(boolean enabled) {
+ mHandler.post(() -> {
+ int newState = enabled ? ETHERNET_STATE_ENABLED : ETHERNET_STATE_DISABLED;
+ if (mEthernetState == newState) return;
+
+ mEthernetState = newState;
+
+ if (enabled) {
+ trackAvailableInterfaces();
+ } else {
+ // TODO: maybe also disable server mode interface as well.
+ untrackFactoryInterfaces();
+ }
+ broadcastEthernetStateChange(mEthernetState);
+ });
+ }
+
+ private void untrackFactoryInterfaces() {
+ for (String iface : mFactory.getAvailableInterfaces(true /* includeRestricted */)) {
+ stopTrackingInterface(iface);
+ }
+ }
+
+ private void unicastEthernetStateChange(@NonNull IEthernetServiceListener listener,
+ int state) {
+ ensureRunningOnEthernetServiceThread();
+ try {
+ listener.onEthernetStateChanged(state);
+ } catch (RemoteException e) {
+ // Do nothing here.
+ }
+ }
+
+ private void broadcastEthernetStateChange(int state) {
+ ensureRunningOnEthernetServiceThread();
+ final int n = mListeners.beginBroadcast();
+ for (int i = 0; i < n; i++) {
+ try {
+ mListeners.getBroadcastItem(i).onEthernetStateChanged(state);
+ } catch (RemoteException e) {
+ // Do nothing here.
+ }
+ }
+ mListeners.finishBroadcast();
+ }
+
void dump(FileDescriptor fd, IndentingPrintWriter pw, String[] args) {
postAndWaitForRunnable(() -> {
pw.println(getClass().getSimpleName());
diff --git a/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java b/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java
index e67c4c8..dd1f1ed 100644
--- a/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java
+++ b/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java
@@ -19,12 +19,15 @@
import static android.net.NetworkCapabilities.TRANSPORT_TEST;
import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -339,4 +342,31 @@
mEthernetServiceImpl.disconnectNetwork(TEST_IFACE, NULL_LISTENER);
verify(mEthernetTracker).disconnectNetwork(eq(TEST_IFACE), eq(NULL_LISTENER));
}
+
+ private void denyPermissions(String... permissions) {
+ for (String permission: permissions) {
+ doReturn(PackageManager.PERMISSION_DENIED).when(mContext)
+ .checkCallingOrSelfPermission(eq(permission));
+ }
+ }
+
+ @Test
+ public void testSetEthernetEnabled() {
+ denyPermissions(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK);
+ mEthernetServiceImpl.setEthernetEnabled(true);
+ verify(mEthernetTracker).setEthernetEnabled(true);
+ reset(mEthernetTracker);
+
+ denyPermissions(Manifest.permission.NETWORK_STACK);
+ mEthernetServiceImpl.setEthernetEnabled(false);
+ verify(mEthernetTracker).setEthernetEnabled(false);
+ reset(mEthernetTracker);
+
+ denyPermissions(Manifest.permission.NETWORK_SETTINGS);
+ try {
+ mEthernetServiceImpl.setEthernetEnabled(true);
+ fail("Should get SecurityException");
+ } catch (SecurityException e) { }
+ verify(mEthernetTracker, never()).setEthernetEnabled(false);
+ }
}
diff --git a/tests/java/com/android/server/ethernet/EthernetTrackerTest.java b/tests/java/com/android/server/ethernet/EthernetTrackerTest.java
index bab9643..b1831c4 100644
--- a/tests/java/com/android/server/ethernet/EthernetTrackerTest.java
+++ b/tests/java/com/android/server/ethernet/EthernetTrackerTest.java
@@ -25,19 +25,26 @@
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import android.content.Context;
import android.content.res.Resources;
+import android.net.EthernetManager;
import android.net.InetAddresses;
import android.net.INetworkInterfaceOutcomeReceiver;
+import android.net.IEthernetServiceListener;
import android.net.INetd;
import android.net.IpConfiguration;
import android.net.IpConfiguration.IpAssignment;
import android.net.IpConfiguration.ProxySettings;
+import android.net.InterfaceConfigurationParcel;
import android.net.LinkAddress;
import android.net.NetworkCapabilities;
import android.net.StaticIpConfiguration;
@@ -393,4 +400,57 @@
assertTrue(isValidTestInterface);
}
+
+ public static class EthernetStateListener extends IEthernetServiceListener.Stub {
+ @Override
+ public void onEthernetStateChanged(int state) { }
+
+ @Override
+ public void onInterfaceStateChanged(String iface, int state, int role,
+ IpConfiguration configuration) { }
+ }
+
+ @Test
+ public void testListenEthernetStateChange() throws Exception {
+ final String testIface = "testtap123";
+ final String testHwAddr = "11:22:33:44:55:66";
+ final InterfaceConfigurationParcel ifaceParcel = new InterfaceConfigurationParcel();
+ ifaceParcel.ifName = testIface;
+ ifaceParcel.hwAddr = testHwAddr;
+ ifaceParcel.flags = new String[] {INetd.IF_STATE_UP};
+
+ tracker.setIncludeTestInterfaces(true);
+ waitForIdle();
+
+ when(mNetd.interfaceGetList()).thenReturn(new String[] {testIface});
+ when(mNetd.interfaceGetCfg(eq(testIface))).thenReturn(ifaceParcel);
+ doReturn(new String[] {testIface}).when(mFactory).getAvailableInterfaces(anyBoolean());
+ doReturn(EthernetManager.STATE_LINK_UP).when(mFactory).getInterfaceState(eq(testIface));
+
+ final EthernetStateListener listener = spy(new EthernetStateListener());
+ tracker.addListener(listener, true /* canUseRestrictedNetworks */);
+ // Check default state.
+ waitForIdle();
+ verify(listener).onInterfaceStateChanged(eq(testIface), eq(EthernetManager.STATE_LINK_UP),
+ anyInt(), any());
+ verify(listener).onEthernetStateChanged(eq(EthernetManager.ETHERNET_STATE_ENABLED));
+ reset(listener);
+
+ doReturn(EthernetManager.STATE_ABSENT).when(mFactory).getInterfaceState(eq(testIface));
+ tracker.setEthernetEnabled(false);
+ waitForIdle();
+ verify(mFactory).removeInterface(eq(testIface));
+ verify(listener).onEthernetStateChanged(eq(EthernetManager.ETHERNET_STATE_DISABLED));
+ verify(listener).onInterfaceStateChanged(eq(testIface), eq(EthernetManager.STATE_ABSENT),
+ anyInt(), any());
+ reset(listener);
+
+ doReturn(EthernetManager.STATE_LINK_UP).when(mFactory).getInterfaceState(eq(testIface));
+ tracker.setEthernetEnabled(true);
+ waitForIdle();
+ verify(mFactory).addInterface(eq(testIface), eq(testHwAddr), any(), any());
+ verify(listener).onEthernetStateChanged(eq(EthernetManager.ETHERNET_STATE_ENABLED));
+ verify(listener).onInterfaceStateChanged(eq(testIface), eq(EthernetManager.STATE_LINK_UP),
+ anyInt(), any());
+ }
}
