Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b152cd0a authored by Erik Kline's avatar Erik Kline
Browse files

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
Change-Id: Ic83ad2a65637277dcb273feb27b2d1bb7a11eb2b
parent 1451124f

core/java/android/net/metrics/IpManagerEvent.java

+2 −1
Original line number Diff line number Diff line
@@ -40,11 +40,12 @@ public final class IpManagerEvent implements Parcelable { 
    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 {}

services/net/java/android/net/ip/IpClient.java

+40 −13
Original line number Diff line number Diff line
@@ -540,6 +540,8 @@ public class IpClient extends StateMachine { 
    // 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 @@ public class IpClient extends StateMachine { 
    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 @@ public class IpClient extends StateMachine { 
    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 class IpClient extends StateMachine { 
     */

    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 @@ public class IpClient extends StateMachine { 
            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 @@ public class IpClient extends StateMachine { 
    }



    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));

    }

services/net/java/android/net/ip/IpManager.java

+1 −14
Original line number Diff line number Diff line
@@ -144,20 +144,7 @@ public class IpManager extends IpClient { 
    }



    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) {

tests/net/java/android/net/ip/IpManagerTest.java→tests/net/java/android/net/ip/IpClientTest.java

+88 −35
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ import static org.junit.Assert.assertFalse; 
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.INetd; 
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.HashSet; 
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 @@ public class IpManagerTest { 
    @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 @@ public class IpManagerTest { 
        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 void setTestInterfaceParams(String ifname) {

        mIfParams = (ifname != null)

                ? new InterfaceParams(ifname, TEST_IFINDEX, TEST_MAC)

                : null;

        when(mDependecies.getInterfaceParams(anyString())).thenReturn(mIfParams);

    }



    private IpManager makeIpManager(String ifname) throws Exception {

        final IpManager ipm = new IpManager(mContext, ifname, mCb, mNMService, mNetd);

    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 @@ public class IpManagerTest { 
        verify(mNMService, times(1)).registerObserver(arg.capture());

        mObserver = arg.getValue();

        reset(mNMService);

        return ipm;

        return ipc;

    }



    @Test

    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 testNullCallbackDoesNotThrow() throws Exception {

        final IpManager ipm = new IpManager(mContext, "lo", null, mNMService);

    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 @@ public class IpManagerTest { 
                .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 @@ public class IpManagerTest { 
                .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 @@ public class IpManagerTest { 
        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 @@ public class IpManagerTest { 
        };



        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 @@ public class IpManagerTest { 
        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 @@ public class IpManagerTest { 
        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 @@ public class IpManagerTest { 
        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'));

    }

}