Merge "Rollback to DHCPDISCOVER w/o Rapid Commit if handshake doesn't succeeds." am: 835efd629d

Original change: https://android-review.googlesource.com/c/platform/packages/modules/NetworkStack/+/1839636

Change-Id: I7790eefe75237e695a644365ef146ecde00d7577

src/android/net/dhcp/DhcpClient.java

diff --git a/src/android/net/dhcp/DhcpClient.java b/src/android/net/dhcp/DhcpClient.java
index 6433e04..d20e769 100644
--- a/src/android/net/dhcp/DhcpClient.java
+++ b/src/android/net/dhcp/DhcpClient.java
@@ -747,9 +747,14 @@
     }
 
     private boolean sendDiscoverPacket() {
+        // When Rapid Commit option is enabled, limit only the first 3 DHCPDISCOVER packets
+        // taking Rapid Commit option, in order to prevent the potential interoperability issue
+        // and be able to rollback later. See {@link DHCP_TIMEOUT_MS} for the (re)transmission
+        // schedule with 10% jitter.
+        final boolean requestRapidCommit = isDhcpRapidCommitEnabled() && (getSecs() <= 4);
         final ByteBuffer packet = DhcpPacket.buildDiscoverPacket(
                 DhcpPacket.ENCAP_L2, mTransactionId, getSecs(), mHwAddr,
-                DO_UNICAST, getRequestedParams(), isDhcpRapidCommitEnabled(), mHostname,
+                DO_UNICAST, getRequestedParams(), requestRapidCommit, mHostname,
                 mConfiguration.options);
         mMetrics.incrementCountForDiscover();
         return transmitPacket(packet, "DHCPDISCOVER", DhcpPacket.ENCAP_L2, INADDR_BROADCAST);

src/android/net/dhcp/DhcpPacket.java

diff --git a/src/android/net/dhcp/DhcpPacket.java b/src/android/net/dhcp/DhcpPacket.java
index 76dc807..63c5b0c 100644
--- a/src/android/net/dhcp/DhcpPacket.java
+++ b/src/android/net/dhcp/DhcpPacket.java
@@ -321,7 +321,8 @@
      * packet may include this option.
      */
     public static final byte DHCP_RAPID_COMMIT = 80;
-    protected boolean mRapidCommit;
+    @VisibleForTesting(otherwise = VisibleForTesting.PROTECTED)
+    public boolean mRapidCommit;
 
     /**
      * DHCP IPv6-Only Preferred Option(RFC 8925).

tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java

diff --git a/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java b/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java
index 378c733..afe0990 100644
--- a/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java
+++ b/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java
@@ -71,6 +71,7 @@
 import static org.junit.Assume.assumeFalse;
 import static org.junit.Assume.assumeTrue;
 import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.ArgumentMatchers.contains;
 import static org.mockito.ArgumentMatchers.longThat;
 import static org.mockito.Mockito.any;
@@ -592,6 +593,8 @@
         if (useNetworkStackSignature()) {
             setUpMocks();
             setUpIpClient();
+            // Enable packet retransmit alarm in DhcpClient.
+            enableRealAlarm("DhcpClient." + mIfaceName + ".KICK");
         }
 
         mIIpClient = makeIIpClient(mIfaceName, mCb);
@@ -688,6 +691,17 @@
         }
     }
 
+    private void enableRealAlarm(String cmdName) {
+        doAnswer((inv) -> {
+            final Context context = InstrumentationRegistry.getTargetContext();
+            final AlarmManager alarmManager = context.getSystemService(AlarmManager.class);
+            alarmManager.setExact(inv.getArgument(0), inv.getArgument(1), inv.getArgument(2),
+                    inv.getArgument(3), inv.getArgument(4));
+            return null;
+        }).when(mAlarm).setExact(anyInt(), anyLong(), eq(cmdName), any(OnAlarmListener.class),
+                any(Handler.class));
+    }
+
     private IpClient makeIpClient() throws Exception {
         IpClient ipc = new IpClient(mContext, mIfaceName, mCb, mNetworkObserverRegistry,
                 mNetworkStackServiceManager, mDependencies);
@@ -1388,6 +1402,27 @@
         assertIpMemoryStoreNetworkAttributes(TEST_LEASE_DURATION_S, currentTime, TEST_DEFAULT_MTU);
     }
 
+    @Test
+    public void testRollbackFromRapidCommitOption() throws Exception {
+        startIpClientProvisioning(false /* isDhcpLeaseCacheEnabled */,
+                true /* isDhcpRapidCommitEnabled */, false /* isPreConnectionEnabled */,
+                false /* isDhcpIpConflictDetectEnabled */, false /* isIPv6OnlyPreferredEnabled */);
+
+        final List<DhcpPacket> discoverList = new ArrayList<DhcpPacket>();
+        DhcpPacket packet;
+        do {
+            packet = getNextDhcpPacket();
+            assertTrue(packet instanceof DhcpDiscoverPacket);
+            discoverList.add(packet);
+        } while (discoverList.size() < 4);
+
+        // Check the only first 3 DHCPDISCOVERs take rapid commit option.
+        assertTrue(discoverList.get(0).mRapidCommit);
+        assertTrue(discoverList.get(1).mRapidCommit);
+        assertTrue(discoverList.get(2).mRapidCommit);
+        assertFalse(discoverList.get(3).mRapidCommit);
+    }
+
     @Test @SignatureRequiredTest(reason = "TODO: evaluate whether signature perms are required")
     public void testDhcpClientStartWithCachedInfiniteLease() throws Exception {
         final DhcpPacket packet = getReplyFromDhcpLease(