Fail if the interface is not available when starting
Addresses a long-standing TODO. Now, when calling IpClient's
startProvisioning(), the interface has to be available (i.e.
InterfaceParams#getByName() must return non-null).
Also:
- add a test
- refactor for testability
- delete some constructors no longer used
- properly handle passed-in null IpClient.Callback
- some more IpManager -> IpClient renaming
- permit recording metrics before starting a provisioning
attempt (logging immediate errors) without Log.wtf().
Test: as follows
- built
- flashed
- booted
- runtest frameworks/opt/net/wifi/tests/wifitests/runtests.sh passes
- runtest frameworks-net passes
- basic WiFi IpClient connections works fine
Bug: 62476366
Bug: 73487570
Merged-In: I68e5e24122dc31e730cdbe8d75e33847e6332da4
Merged-In: Ifd27f5d908947cd7b4e1b8d54f9fa87e43ebb11b
Merged-In: Ief3c8e1652f69af0276fe35946ae1bf6e6b1b57e
Change-Id: Ic83ad2a65637277dcb273feb27b2d1bb7a11eb2b
(cherry picked from commit b152cd0aa4f333b721615bb17773b35a989245fb)
diff --git a/core/java/android/net/metrics/IpManagerEvent.java b/core/java/android/net/metrics/IpManagerEvent.java
index a94b928..f8a63ce 100644
--- a/core/java/android/net/metrics/IpManagerEvent.java
+++ b/core/java/android/net/metrics/IpManagerEvent.java
@@ -40,11 +40,12 @@
public static final int ERROR_STARTING_IPV6 = 5;
public static final int ERROR_STARTING_IPREACHABILITYMONITOR = 6;
public static final int ERROR_INVALID_PROVISIONING = 7;
+ public static final int ERROR_INTERFACE_NOT_FOUND = 8;
@IntDef(value = {
PROVISIONING_OK, PROVISIONING_FAIL, COMPLETE_LIFECYCLE,
ERROR_STARTING_IPV4, ERROR_STARTING_IPV6, ERROR_STARTING_IPREACHABILITYMONITOR,
- ERROR_INVALID_PROVISIONING,
+ ERROR_INVALID_PROVISIONING, ERROR_INTERFACE_NOT_FOUND,
})
@Retention(RetentionPolicy.SOURCE)
public @interface EventType {}
diff --git a/services/net/java/android/net/ip/IpClient.java b/services/net/java/android/net/ip/IpClient.java
index 1f370a5..9863370 100644
--- a/services/net/java/android/net/ip/IpClient.java
+++ b/services/net/java/android/net/ip/IpClient.java
@@ -540,6 +540,8 @@
// TODO: Revert this hack once IpClient and Nat464Xlat work in concert.
private static final String CLAT_PREFIX = "v4-";
+ private static final int IMMEDIATE_FAILURE_DURATION = 0;
+
private final State mStoppedState = new StoppedState();
private final State mStoppingState = new StoppingState();
private final State mStartedState = new StartedState();
@@ -551,6 +553,7 @@
private final String mClatInterfaceName;
@VisibleForTesting
protected final Callback mCallback;
+ private final Dependencies mDependencies;
private final CountDownLatch mShutdownLatch;
private final INetworkManagementService mNwService;
private final NetlinkTracker mNetlinkTracker;
@@ -579,10 +582,23 @@
private boolean mMulticastFiltering;
private long mStartTimeMillis;
+ public static class Dependencies {
+ public INetworkManagementService getNMS() {
+ return INetworkManagementService.Stub.asInterface(
+ ServiceManager.getService(Context.NETWORKMANAGEMENT_SERVICE));
+ }
+
+ public INetd getNetd() {
+ return NetdService.getInstance();
+ }
+
+ public InterfaceParams getInterfaceParams(String ifname) {
+ return InterfaceParams.getByName(ifname);
+ }
+ }
+
public IpClient(Context context, String ifName, Callback callback) {
- this(context, ifName, callback, INetworkManagementService.Stub.asInterface(
- ServiceManager.getService(Context.NETWORKMANAGEMENT_SERVICE)),
- NetdService.getInstance());
+ this(context, ifName, callback, new Dependencies());
}
/**
@@ -591,27 +607,35 @@
*/
public IpClient(Context context, String ifName, Callback callback,
INetworkManagementService nwService) {
- this(context, ifName, callback, nwService, NetdService.getInstance());
+ this(context, ifName, callback, new Dependencies() {
+ @Override
+ public INetworkManagementService getNMS() { return nwService; }
+ });
}
@VisibleForTesting
- IpClient(Context context, String ifName, Callback callback,
- INetworkManagementService nwService, INetd netd) {
+ IpClient(Context context, String ifName, Callback callback, Dependencies deps) {
super(IpClient.class.getSimpleName() + "." + ifName);
+ Preconditions.checkNotNull(ifName);
+ Preconditions.checkNotNull(callback);
+
mTag = getName();
mContext = context;
mInterfaceName = ifName;
mClatInterfaceName = CLAT_PREFIX + ifName;
mCallback = new LoggingCallbackWrapper(callback);
+ mDependencies = deps;
mShutdownLatch = new CountDownLatch(1);
- mNwService = nwService;
+ mNwService = deps.getNMS();
mLog = new SharedLog(MAX_LOG_RECORDS, mTag);
mConnectivityPacketLog = new LocalLog(MAX_PACKET_RECORDS);
mMsgStateLogger = new MessageHandlingLogger();
- mInterfaceCtrl = new InterfaceController(mInterfaceName, mNwService, netd, mLog);
+ // TODO: Consider creating, constructing, and passing in some kind of
+ // InterfaceController.Dependencies class.
+ mInterfaceCtrl = new InterfaceController(mInterfaceName, mNwService, deps.getNetd(), mLog);
mNetlinkTracker = new NetlinkTracker(
mInterfaceName,
@@ -742,11 +766,11 @@
return;
}
- mInterfaceParams = InterfaceParams.getByName(mInterfaceName);
+ mInterfaceParams = mDependencies.getInterfaceParams(mInterfaceName);
if (mInterfaceParams == null) {
logError("Failed to find InterfaceParams for " + mInterfaceName);
- // TODO: call doImmediateProvisioningFailure() with an error code
- // indicating something like "interface not ready".
+ doImmediateProvisioningFailure(IpManagerEvent.ERROR_INTERFACE_NOT_FOUND);
+ return;
}
mCallback.setNeighborDiscoveryOffload(true);
@@ -930,8 +954,11 @@
}
private void recordMetric(final int type) {
- if (mStartTimeMillis <= 0) { Log.wtf(mTag, "Start time undefined!"); }
- final long duration = SystemClock.elapsedRealtime() - mStartTimeMillis;
+ // We may record error metrics prior to starting.
+ // Map this to IMMEDIATE_FAILURE_DURATION.
+ final long duration = (mStartTimeMillis > 0)
+ ? (SystemClock.elapsedRealtime() - mStartTimeMillis)
+ : IMMEDIATE_FAILURE_DURATION;
mMetricsLog.log(mInterfaceName, new IpManagerEvent(type, duration));
}
diff --git a/services/net/java/android/net/ip/IpManager.java b/services/net/java/android/net/ip/IpManager.java
index 3898145..508a43d 100644
--- a/services/net/java/android/net/ip/IpManager.java
+++ b/services/net/java/android/net/ip/IpManager.java
@@ -144,20 +144,7 @@
}
public IpManager(Context context, String ifName, Callback callback) {
- this(context, ifName, callback, INetworkManagementService.Stub.asInterface(
- ServiceManager.getService(Context.NETWORKMANAGEMENT_SERVICE)),
- NetdService.getInstance());
- }
-
- public IpManager(Context context, String ifName, Callback callback,
- INetworkManagementService nwService) {
- this(context, ifName, callback, nwService, NetdService.getInstance());
- }
-
- @VisibleForTesting
- public IpManager(Context context, String ifName, Callback callback,
- INetworkManagementService nwService, INetd netd) {
- super(context, ifName, callback, nwService, netd);
+ super(context, ifName, callback);
}
public void startProvisioning(ProvisioningConfiguration req) {
diff --git a/tests/net/java/android/net/ip/IpManagerTest.java b/tests/net/java/android/net/ip/IpClientTest.java
similarity index 80%
rename from tests/net/java/android/net/ip/IpManagerTest.java
rename to tests/net/java/android/net/ip/IpClientTest.java
index 22d88fb..e9e880d 100644
--- a/tests/net/java/android/net/ip/IpManagerTest.java
+++ b/tests/net/java/android/net/ip/IpClientTest.java
@@ -21,6 +21,7 @@
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
@@ -38,10 +39,12 @@
import android.net.IpPrefix;
import android.net.LinkAddress;
import android.net.LinkProperties;
+import android.net.MacAddress;
import android.net.RouteInfo;
-import android.net.ip.IpManager.Callback;
-import android.net.ip.IpManager.InitialConfiguration;
-import android.net.ip.IpManager.ProvisioningConfiguration;
+import android.net.ip.IpClient.Callback;
+import android.net.ip.IpClient.InitialConfiguration;
+import android.net.ip.IpClient.ProvisioningConfiguration;
+import android.net.util.InterfaceParams;
import android.os.INetworkManagementService;
import android.provider.Settings;
import android.support.test.filters.SmallTest;
@@ -68,15 +71,19 @@
import java.util.Set;
/**
- * Tests for IpManager.
+ * Tests for IpClient.
*/
@RunWith(AndroidJUnit4.class)
@SmallTest
-public class IpManagerTest {
+public class IpClientTest {
private static final int DEFAULT_AVOIDBADWIFI_CONFIG_VALUE = 1;
private static final String VALID = "VALID";
private static final String INVALID = "INVALID";
+ private static final String TEST_IFNAME = "test_wlan0";
+ private static final int TEST_IFINDEX = 1001;
+ // See RFC 7042#section-2.1.2 for EUI-48 documentation values.
+ private static final MacAddress TEST_MAC = MacAddress.fromString("00:00:5E:00:53:01");
@Mock private Context mContext;
@Mock private INetworkManagementService mNMService;
@@ -84,9 +91,11 @@
@Mock private Resources mResources;
@Mock private Callback mCb;
@Mock private AlarmManager mAlarm;
+ @Mock private IpClient.Dependencies mDependecies;
private MockContentResolver mContentResolver;
- BaseNetworkObserver mObserver;
+ private BaseNetworkObserver mObserver;
+ private InterfaceParams mIfParams;
@Before
public void setUp() throws Exception {
@@ -100,10 +109,23 @@
mContentResolver = new MockContentResolver();
mContentResolver.addProvider(Settings.AUTHORITY, new FakeSettingsProvider());
when(mContext.getContentResolver()).thenReturn(mContentResolver);
+
+ mIfParams = null;
+
+ when(mDependecies.getNMS()).thenReturn(mNMService);
+ when(mDependecies.getNetd()).thenReturn(mNetd);
}
- private IpManager makeIpManager(String ifname) throws Exception {
- final IpManager ipm = new IpManager(mContext, ifname, mCb, mNMService, mNetd);
+ private void setTestInterfaceParams(String ifname) {
+ mIfParams = (ifname != null)
+ ? new InterfaceParams(ifname, TEST_IFINDEX, TEST_MAC)
+ : null;
+ when(mDependecies.getInterfaceParams(anyString())).thenReturn(mIfParams);
+ }
+
+ private IpClient makeIpClient(String ifname) throws Exception {
+ setTestInterfaceParams(ifname);
+ final IpClient ipc = new IpClient(mContext, ifname, mCb, mDependecies);
verify(mNMService, timeout(100).times(1)).disableIpv6(ifname);
verify(mNMService, timeout(100).times(1)).clearInterfaceAddresses(ifname);
ArgumentCaptor<BaseNetworkObserver> arg =
@@ -111,23 +133,54 @@
verify(mNMService, times(1)).registerObserver(arg.capture());
mObserver = arg.getValue();
reset(mNMService);
- return ipm;
+ return ipc;
}
@Test
- public void testNullCallbackDoesNotThrow() throws Exception {
- final IpManager ipm = new IpManager(mContext, "lo", null, mNMService);
+ public void testNullInterfaceNameMostDefinitelyThrows() throws Exception {
+ setTestInterfaceParams(null);
+ try {
+ final IpClient ipc = new IpClient(mContext, null, mCb, mDependecies);
+ ipc.shutdown();
+ fail();
+ } catch (NullPointerException npe) {
+ // Phew; null interface names not allowed.
+ }
+ }
+
+ @Test
+ public void testNullCallbackMostDefinitelyThrows() throws Exception {
+ final String ifname = "lo";
+ setTestInterfaceParams(ifname);
+ try {
+ final IpClient ipc = new IpClient(mContext, ifname, null, mDependecies);
+ ipc.shutdown();
+ fail();
+ } catch (NullPointerException npe) {
+ // Phew; null callbacks not allowed.
+ }
}
@Test
public void testInvalidInterfaceDoesNotThrow() throws Exception {
- final IpManager ipm = new IpManager(mContext, "test_wlan0", mCb, mNMService);
+ setTestInterfaceParams(TEST_IFNAME);
+ final IpClient ipc = new IpClient(mContext, TEST_IFNAME, mCb, mDependecies);
+ ipc.shutdown();
+ }
+
+ @Test
+ public void testInterfaceNotFoundFailsImmediately() throws Exception {
+ setTestInterfaceParams(null);
+ final IpClient ipc = new IpClient(mContext, TEST_IFNAME, mCb, mDependecies);
+ ipc.startProvisioning(new IpClient.ProvisioningConfiguration());
+ verify(mCb, times(1)).onProvisioningFailure(any());
+ ipc.shutdown();
}
@Test
public void testDefaultProvisioningConfiguration() throws Exception {
- final String iface = "test_wlan0";
- final IpManager ipm = makeIpManager(iface);
+ final String iface = TEST_IFNAME;
+ final IpClient ipc = makeIpClient(iface);
ProvisioningConfiguration config = new ProvisioningConfiguration.Builder()
.withoutIPv4()
@@ -136,20 +189,20 @@
.withoutIpReachabilityMonitor()
.build();
- ipm.startProvisioning(config);
+ ipc.startProvisioning(config);
verify(mCb, times(1)).setNeighborDiscoveryOffload(true);
verify(mCb, timeout(100).times(1)).setFallbackMulticastFilter(false);
verify(mCb, never()).onProvisioningFailure(any());
- ipm.stop();
+ ipc.shutdown();
verify(mNMService, timeout(100).times(1)).disableIpv6(iface);
verify(mNMService, timeout(100).times(1)).clearInterfaceAddresses(iface);
}
@Test
public void testProvisioningWithInitialConfiguration() throws Exception {
- final String iface = "test_wlan0";
- final IpManager ipm = makeIpManager(iface);
+ final String iface = TEST_IFNAME;
+ final IpClient ipc = makeIpClient(iface);
String[] addresses = {
"fe80::a4be:f92:e1f7:22d1/64",
@@ -164,7 +217,7 @@
.withInitialConfiguration(conf(links(addresses), prefixes(prefixes), ips()))
.build();
- ipm.startProvisioning(config);
+ ipc.startProvisioning(config);
verify(mCb, times(1)).setNeighborDiscoveryOffload(true);
verify(mCb, timeout(100).times(1)).setFallbackMulticastFilter(false);
verify(mCb, never()).onProvisioningFailure(any());
@@ -190,7 +243,7 @@
want.setInterfaceName(iface);
verify(mCb, timeout(100).times(1)).onProvisioningSuccess(eq(want));
- ipm.stop();
+ ipc.shutdown();
verify(mNMService, timeout(100).times(1)).disableIpv6(iface);
verify(mNMService, timeout(100).times(1)).clearInterfaceAddresses(iface);
}
@@ -228,7 +281,7 @@
};
for (IsProvisionedTestCase testcase : testcases) {
- if (IpManager.isProvisioned(testcase.lp, testcase.config) != testcase.isProvisioned) {
+ if (IpClient.isProvisioned(testcase.lp, testcase.config) != testcase.isProvisioned) {
fail(testcase.errorMessage());
}
}
@@ -424,11 +477,11 @@
List<String> list3 = Arrays.asList("bar", "baz");
List<String> list4 = Arrays.asList("foo", "bar", "baz");
- assertTrue(IpManager.all(list1, (x) -> false));
- assertFalse(IpManager.all(list2, (x) -> false));
- assertTrue(IpManager.all(list3, (x) -> true));
- assertTrue(IpManager.all(list2, (x) -> x.charAt(0) == 'f'));
- assertFalse(IpManager.all(list4, (x) -> x.charAt(0) == 'f'));
+ assertTrue(IpClient.all(list1, (x) -> false));
+ assertFalse(IpClient.all(list2, (x) -> false));
+ assertTrue(IpClient.all(list3, (x) -> true));
+ assertTrue(IpClient.all(list2, (x) -> x.charAt(0) == 'f'));
+ assertFalse(IpClient.all(list4, (x) -> x.charAt(0) == 'f'));
}
@Test
@@ -438,11 +491,11 @@
List<String> list3 = Arrays.asList("bar", "baz");
List<String> list4 = Arrays.asList("foo", "bar", "baz");
- assertFalse(IpManager.any(list1, (x) -> true));
- assertTrue(IpManager.any(list2, (x) -> true));
- assertTrue(IpManager.any(list2, (x) -> x.charAt(0) == 'f'));
- assertFalse(IpManager.any(list3, (x) -> x.charAt(0) == 'f'));
- assertTrue(IpManager.any(list4, (x) -> x.charAt(0) == 'f'));
+ assertFalse(IpClient.any(list1, (x) -> true));
+ assertTrue(IpClient.any(list2, (x) -> true));
+ assertTrue(IpClient.any(list2, (x) -> x.charAt(0) == 'f'));
+ assertFalse(IpClient.any(list3, (x) -> x.charAt(0) == 'f'));
+ assertTrue(IpClient.any(list4, (x) -> x.charAt(0) == 'f'));
}
@Test
@@ -451,9 +504,9 @@
List<String> list2 = Arrays.asList("foo");
List<String> list3 = Arrays.asList("foo", "bar", "baz");
- assertEquals(list1, IpManager.findAll(list1, (x) -> true));
- assertEquals(list1, IpManager.findAll(list3, (x) -> false));
- assertEquals(list3, IpManager.findAll(list3, (x) -> true));
- assertEquals(list2, IpManager.findAll(list3, (x) -> x.charAt(0) == 'f'));
+ assertEquals(list1, IpClient.findAll(list1, (x) -> true));
+ assertEquals(list1, IpClient.findAll(list3, (x) -> false));
+ assertEquals(list3, IpClient.findAll(list3, (x) -> true));
+ assertEquals(list2, IpClient.findAll(list3, (x) -> x.charAt(0) == 'f'));
}
}