Getting interface params in ClearingIpAddressState#enter.

In case of wifi might stop IpClient and then restart prov immediately
to avoid wifi disconnection especially when roaming happens, this
might cause getting interface params with incorrect interface mtu when
starting provisioning again before interface mtu hasn't been restored.

Moving getting interface params to ClearingIpAddressState#enter ensures
that interface parameters are fetched on the handler thread so they are
properly ordered with other events, such as restoring the interface MTU
on teardown.

Bug: 152169857
Test: atest NetworkStackIntegrationTests NetworkStackTests

Merged-In: Ifd06b8d18ce570b1aa424ad215111c80f094ca97
(cherry picked from commit a6aba532103a3258d09e6931106d6fb05368c130)

Change-Id: I0c8c1ed1646ab12bc30da2b7f8e5819d2d9495ba

3 files changed, 48 insertions, 14 deletions

diff --git a/src/android/net/ip/IpClient.java b/src/android/net/ip/IpClient.java
index ebb31c6..74a4070 100644
--- a/src/android/net/ip/IpClient.java
+++ b/src/android/net/ip/IpClient.java
@@ -765,14 +765,6 @@ public class IpClient extends StateMachine {
             return;
         }
 
-        mInterfaceParams = mDependencies.getInterfaceParams(mInterfaceName);
-        if (mInterfaceParams == null) {
-            logError("Failed to find InterfaceParams for " + mInterfaceName);
-            doImmediateProvisioningFailure(IpManagerEvent.ERROR_INTERFACE_NOT_FOUND);
-            return;
-        }
-
-        mCallback.setNeighborDiscoveryOffload(true);
         sendMessage(CMD_START, new android.net.shared.ProvisioningConfiguration(req));
     }
 
@@ -1636,6 +1628,17 @@ public class IpClient extends StateMachine {
                 // tethering or during an IpClient restart.
                 stopAllIP();
             }
+
+            // Ensure that interface parameters are fetched on the handler thread so they are
+            // properly ordered with other events, such as restoring the interface MTU on teardown.
+            mInterfaceParams = mDependencies.getInterfaceParams(mInterfaceName);
+            if (mInterfaceParams == null) {
+                logError("Failed to find InterfaceParams for " + mInterfaceName);
+                doImmediateProvisioningFailure(IpManagerEvent.ERROR_INTERFACE_NOT_FOUND);
+                transitionTo(mStoppedState);
+                return;
+            }
+            mCallback.setNeighborDiscoveryOffload(true);
         }
 
         @Override
diff --git a/tests/integration/src/android/net/ip/IpClientIntegrationTest.java b/tests/integration/src/android/net/ip/IpClientIntegrationTest.java
index 0fa6266..2ee288d 100644
--- a/tests/integration/src/android/net/ip/IpClientIntegrationTest.java
+++ b/tests/integration/src/android/net/ip/IpClientIntegrationTest.java
@@ -524,7 +524,6 @@ public class IpClientIntegrationTest {
         mDependencies.setHostnameConfiguration(isHostnameConfigurationEnabled, hostname);
         mIpc.setL2KeyAndGroupHint(TEST_L2KEY, TEST_GROUPHINT);
         mIpc.startProvisioning(builder.build());
-        verify(mCb).setNeighborDiscoveryOffload(true);
         if (!isPreconnectionEnabled) {
             verify(mCb, timeout(TEST_TIMEOUT_MS)).setFallbackMulticastFilter(false);
         }
@@ -642,7 +641,6 @@ public class IpClientIntegrationTest {
                             ArgumentCaptor.forClass(LinkProperties.class);
                     verifyProvisioningSuccess(captor, Collections.singletonList(CLIENT_ADDR));
                 }
-
                 return packetList;
             }
         }
@@ -725,6 +723,12 @@ public class IpClientIntegrationTest {
             assertEquals(NetworkInterface.getByName(mIfaceName).getMTU(), mtu);
         }
 
+        // Sometimes, IpClient receives an update with an empty LinkProperties during startup,
+        // when the link-local address is deleted after interface bringup. Reset expectations
+        // here to ensure that verifyAfterIpClientShutdown does not fail because it sees two
+        // empty LinkProperties changes instead of one.
+        reset(mCb);
+
         if (shouldRemoveTapInterface) removeTapInterface(mTapFd);
         try {
             mIpc.shutdown();
@@ -1081,10 +1085,37 @@ public class IpClientIntegrationTest {
                 .build();
 
         mIpc.startProvisioning(config);
-        verify(mCb).onProvisioningFailure(any());
+        verify(mCb, timeout(TEST_TIMEOUT_MS)).onProvisioningFailure(any());
         verify(mCb, never()).setNeighborDiscoveryOffload(true);
     }
 
+    @Test
+    public void testRestoreInitialInterfaceMtu_stopIpClientAndRestart() throws Exception {
+        long currentTime = System.currentTimeMillis();
+
+        performDhcpHandshake(true /* isSuccessLease */, TEST_LEASE_DURATION_S,
+                true /* isDhcpLeaseCacheEnabled */, false /* shouldReplyRapidCommitAck */,
+                TEST_MIN_MTU, false /* isDhcpIpConflictDetectEnabled */);
+        assertIpMemoryStoreNetworkAttributes(TEST_LEASE_DURATION_S, currentTime, TEST_MIN_MTU);
+
+        // Pretend that ConnectivityService set the MTU.
+        mNetd.interfaceSetMtu(mIfaceName, TEST_MIN_MTU);
+        assertEquals(NetworkInterface.getByName(mIfaceName).getMTU(), TEST_MIN_MTU);
+
+        reset(mCb);
+        reset(mIpMemoryStore);
+
+        // Stop IpClient and then restart provisioning immediately.
+        mIpc.stop();
+        currentTime = System.currentTimeMillis();
+        // Intend to set mtu option to 0, then verify that won't influence interface mtu restore.
+        performDhcpHandshake(true /* isSuccessLease */, TEST_LEASE_DURATION_S,
+                true /* isDhcpLeaseCacheEnabled */, false /* shouldReplyRapidCommitAck */,
+                0 /* mtu */, false /* isDhcpIpConflictDetectEnabled */);
+        assertIpMemoryStoreNetworkAttributes(TEST_LEASE_DURATION_S, currentTime, 0 /* mtu */);
+        assertEquals(NetworkInterface.getByName(mIfaceName).getMTU(), TEST_DEFAULT_MTU);
+    }
+
     private boolean isRouterSolicitation(final byte[] packetBytes) {
         ByteBuffer packet = ByteBuffer.wrap(packetBytes);
         return packet.getShort(ETHER_TYPE_OFFSET) == (short) ETH_P_IPV6
diff --git a/tests/unit/src/android/net/ip/IpClientTest.java b/tests/unit/src/android/net/ip/IpClientTest.java
index 4be5442..5fa5a6c 100644
--- a/tests/unit/src/android/net/ip/IpClientTest.java
+++ b/tests/unit/src/android/net/ip/IpClientTest.java
@@ -221,7 +221,7 @@ public class IpClientTest {
         final IpClient ipc = new IpClient(mContext, TEST_IFNAME, mCb, mObserverRegistry,
                 mNetworkStackServiceManager, mDependencies);
         ipc.startProvisioning(new ProvisioningConfiguration());
-        verify(mCb, times(1)).onProvisioningFailure(any());
+        verify(mCb, timeout(TEST_TIMEOUT_MS).times(1)).onProvisioningFailure(any());
         verify(mIpMemoryStore, never()).storeNetworkAttributes(any(), any(), any());
         ipc.shutdown();
     }
@@ -249,7 +249,7 @@ public class IpClientTest {
                 .build();
 
         ipc.startProvisioning(config);
-        verify(mCb, times(1)).setNeighborDiscoveryOffload(true);
+        verify(mCb, timeout(TEST_TIMEOUT_MS).times(1)).setNeighborDiscoveryOffload(true);
         verify(mCb, timeout(TEST_TIMEOUT_MS).times(1)).setFallbackMulticastFilter(false);
 
         final LinkProperties lp = makeIPv6ProvisionedLinkProperties();
@@ -364,7 +364,7 @@ public class IpClientTest {
                 .build();
 
         ipc.startProvisioning(config);
-        verify(mCb, times(1)).setNeighborDiscoveryOffload(true);
+        verify(mCb, timeout(TEST_TIMEOUT_MS).times(1)).setNeighborDiscoveryOffload(true);
         verify(mCb, timeout(TEST_TIMEOUT_MS).times(1)).setFallbackMulticastFilter(false);
         verify(mCb, never()).onProvisioningFailure(any());
         ipc.setL2KeyAndGroupHint(l2Key, groupHint);