Get current tracked BSSID from Layer2Info on starting provisioning. am: 861fe42cc8 am: a3ae54bcb2

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

Change-Id: Ia7d47ee39379093f7ce508ffbc246bf52c18a980
diff --git a/src/android/net/ip/IpClient.java b/src/android/net/ip/IpClient.java
index efe1054..f7c728b 100644
--- a/src/android/net/ip/IpClient.java
+++ b/src/android/net/ip/IpClient.java
@@ -859,6 +859,28 @@
                 false /* defaultEnabled */);
     }
 
+    private void setInitialBssid(final ProvisioningConfiguration req) {
+        final ScanResultInfo scanResultInfo = req.mScanResultInfo;
+        mCurrentBssid = null;
+        // http://b/185202634
+        // ScanResultInfo is not populated in some situations.
+        // On S and above, prefer getting the BSSID from the Layer2Info.
+        // On R and below, get the BSSID from the ScanResultInfo and fall back to
+        // getting it from the Layer2Info. This ensures no regressions if any R
+        // devices pass in a null or meaningless BSSID in the Layer2Info.
+        if (!ShimUtils.isAtLeastS() && scanResultInfo != null) {
+            try {
+                mCurrentBssid = MacAddress.fromString(scanResultInfo.getBssid());
+            } catch (IllegalArgumentException e) {
+                Log.wtf(mTag, "Invalid BSSID: " + scanResultInfo.getBssid()
+                        + " in provisioning configuration", e);
+            }
+        }
+        if (mCurrentBssid == null && req.mLayer2Info != null) {
+            mCurrentBssid = req.mLayer2Info.mBssid;
+        }
+    }
+
     @Override
     protected void onQuitting() {
         mCallback.onQuit();
@@ -882,17 +904,7 @@
             return;
         }
 
-        final ScanResultInfo scanResultInfo = req.mScanResultInfo;
-        mCurrentBssid = null;
-        if (scanResultInfo != null) {
-            try {
-                mCurrentBssid = MacAddress.fromString(scanResultInfo.getBssid());
-            } catch (IllegalArgumentException e) {
-                Log.wtf(mTag, "Invalid BSSID: " + scanResultInfo.getBssid()
-                        + " in provisioning configuration", e);
-            }
-        }
-
+        setInitialBssid(req);
         if (req.mLayer2Info != null) {
             mL2Key = req.mLayer2Info.mL2Key;
             mCluster = req.mLayer2Info.mCluster;
diff --git a/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java b/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java
index c5a8b89..f3419a3 100644
--- a/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java
+++ b/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java
@@ -782,11 +782,14 @@
     private void startIpClientProvisioning(final boolean isDhcpLeaseCacheEnabled,
             final boolean shouldReplyRapidCommitAck, final boolean isPreconnectionEnabled,
             final boolean isDhcpIpConflictDetectEnabled, final boolean isIPv6OnlyPreferredEnabled,
-            final String displayName, final ScanResultInfo scanResultInfo) throws Exception {
+            final String displayName, final ScanResultInfo scanResultInfo,
+            final Layer2Information layer2Info) throws Exception {
         ProvisioningConfiguration.Builder prov = new ProvisioningConfiguration.Builder()
                 .withoutIpReachabilityMonitor()
-                .withLayer2Information(new Layer2Information(TEST_L2KEY, TEST_CLUSTER,
-                        MacAddress.fromString(TEST_DEFAULT_BSSID)))
+                .withLayer2Information(layer2Info == null
+                        ? new Layer2Information(TEST_L2KEY, TEST_CLUSTER,
+                              MacAddress.fromString(TEST_DEFAULT_BSSID))
+                        : layer2Info)
                 .withoutIPv6();
         if (isPreconnectionEnabled) prov.withPreconnection();
         if (displayName != null) prov.withDisplayName(displayName);
@@ -808,7 +811,7 @@
             throws Exception {
         startIpClientProvisioning(isDhcpLeaseCacheEnabled, isDhcpRapidCommitEnabled,
                 isPreconnectionEnabled, isDhcpIpConflictDetectEnabled, isIPv6OnlyPreferredEnabled,
-                null /* displayName */, null /* ScanResultInfo */);
+                null /* displayName */, null /* ScanResultInfo */, null /* layer2Info */);
     }
 
     private void assertIpMemoryStoreNetworkAttributes(final Integer leaseTimeSec,
@@ -863,10 +866,11 @@
             final boolean isDhcpIpConflictDetectEnabled,
             final boolean isIPv6OnlyPreferredEnabled,
             final String captivePortalApiUrl, final String displayName,
-            final ScanResultInfo scanResultInfo) throws Exception {
+            final ScanResultInfo scanResultInfo, final Layer2Information layer2Info)
+            throws Exception {
         startIpClientProvisioning(isDhcpLeaseCacheEnabled, shouldReplyRapidCommitAck,
                 false /* isPreconnectionEnabled */, isDhcpIpConflictDetectEnabled,
-                isIPv6OnlyPreferredEnabled, displayName, scanResultInfo);
+                isIPv6OnlyPreferredEnabled, displayName, scanResultInfo, layer2Info);
         return handleDhcpPackets(isSuccessLease, leaseTimeSec, shouldReplyRapidCommitAck, mtu,
                 captivePortalApiUrl);
     }
@@ -912,7 +916,8 @@
         return performDhcpHandshake(isSuccessLease, leaseTimeSec, isDhcpLeaseCacheEnabled,
                 isDhcpRapidCommitEnabled, mtu, isDhcpIpConflictDetectEnabled,
                 false /* isIPv6OnlyPreferredEnabled */,
-                null /* captivePortalApiUrl */, null /* displayName */, null /* scanResultInfo */);
+                null /* captivePortalApiUrl */, null /* displayName */, null /* scanResultInfo */,
+                null /* layer2Info */);
     }
 
     private List<DhcpPacket> performDhcpHandshake() throws Exception {
@@ -921,10 +926,21 @@
                 TEST_DEFAULT_MTU, false /* isDhcpIpConflictDetectEnabled */);
     }
 
-    private DhcpPacket getNextDhcpPacket() throws ParseException {
-        byte[] packet = mDhcpPacketReadHead.getValue().poll(PACKET_TIMEOUT_MS, this::isDhcpPacket);
+    private DhcpPacket getNextDhcpPacket(final long timeout) throws Exception {
+        byte[] packet;
+        while ((packet = mDhcpPacketReadHead.getValue()
+                .poll(timeout, this::isDhcpPacket)) != null) {
+            final DhcpPacket dhcpPacket = DhcpPacket.decodeFullPacket(packet, packet.length,
+                    ENCAP_L2);
+            if (dhcpPacket != null) return dhcpPacket;
+        }
+        return null;
+    }
+
+    private DhcpPacket getNextDhcpPacket() throws Exception {
+        final DhcpPacket packet = getNextDhcpPacket(PACKET_TIMEOUT_MS);
         assertNotNull("No expected DHCP packet received on interface within timeout", packet);
-        return DhcpPacket.decodeFullPacket(packet, packet.length, ENCAP_L2);
+        return packet;
     }
 
     private DhcpPacket getReplyFromDhcpLease(final NetworkAttributes na, boolean timeout)
@@ -2048,9 +2064,8 @@
         final List<DhcpPacket> sentPackets = performDhcpHandshake(true /* isSuccessLease */,
                 TEST_LEASE_DURATION_S, true /* isDhcpLeaseCacheEnabled */,
                 false /* isDhcpRapidCommitEnabled */, TEST_DEFAULT_MTU,
-                false /* isDhcpIpConflictDetectEnabled */,
-                false /* isIPv6OnlyPreferredEnabled */,
-                null /* captivePortalApiUrl */, null /* displayName */, null /* scanResultInfo */);
+                false /* isDhcpIpConflictDetectEnabled */);
+
         assertEquals(2, sentPackets.size());
         verifyIPv4OnlyProvisioningSuccess(Collections.singletonList(CLIENT_ADDR));
         assertHostname(true, TEST_HOST_NAME, TEST_HOST_NAME_TRANSLITERATION, sentPackets);
@@ -2066,9 +2081,8 @@
         final List<DhcpPacket> sentPackets = performDhcpHandshake(true /* isSuccessLease */,
                 TEST_LEASE_DURATION_S, true /* isDhcpLeaseCacheEnabled */,
                 false /* isDhcpRapidCommitEnabled */, TEST_DEFAULT_MTU,
-                false /* isDhcpIpConflictDetectEnabled */,
-                false /* isIPv6OnlyPreferredEnabled */,
-                null /* captivePortalApiUrl */, null /* displayName */, null /* scanResultInfo */);
+                false /* isDhcpIpConflictDetectEnabled */);
+
         assertEquals(2, sentPackets.size());
         verifyIPv4OnlyProvisioningSuccess(Collections.singletonList(CLIENT_ADDR));
         assertHostname(false, TEST_HOST_NAME, TEST_HOST_NAME_TRANSLITERATION, sentPackets);
@@ -2084,9 +2098,8 @@
         final List<DhcpPacket> sentPackets = performDhcpHandshake(true /* isSuccessLease */,
                 TEST_LEASE_DURATION_S, true /* isDhcpLeaseCacheEnabled */,
                 false /* isDhcpRapidCommitEnabled */, TEST_DEFAULT_MTU,
-                false /* isDhcpIpConflictDetectEnabled */,
-                false /* isIPv6OnlyPreferredEnabled */,
-                null /* captivePortalApiUrl */, null /* displayName */, null /* scanResultInfo */);
+                false /* isDhcpIpConflictDetectEnabled */);
+
         assertEquals(2, sentPackets.size());
         verifyIPv4OnlyProvisioningSuccess(Collections.singletonList(CLIENT_ADDR));
         assertHostname(true, null /* hostname */, null /* hostnameAfterTransliteration */,
@@ -2183,7 +2196,8 @@
                 false /* isDhcpRapidCommitEnabled */, TEST_DEFAULT_MTU,
                 false /* isDhcpIpConflictDetectEnabled */,
                 false /* isIPv6OnlyPreferredEnabled */,
-                null /* captivePortalApiUrl */, displayName, info /* scanResultInfo */);
+                null /* captivePortalApiUrl */, displayName, info /* scanResultInfo */,
+                null /* layer2Info */);
         assertEquals(2, sentPackets.size());
         verifyIPv4OnlyProvisioningSuccess(Collections.singletonList(CLIENT_ADDR));
 
@@ -2270,10 +2284,9 @@
     }
 
     private void doDhcpRoamingTest(final boolean hasMismatchedIpAddress, final String displayName,
-            final String ssid, final String bssid, final boolean expectRoaming) throws Exception {
+            final MacAddress bssid, final boolean expectRoaming) throws Exception {
         long currentTime = System.currentTimeMillis();
-        final ScanResultInfo scanResultInfo = (ssid == null || bssid == null)
-                ? null : makeScanResultInfo(ssid, bssid);
+        final Layer2Information layer2Info = new Layer2Information(TEST_L2KEY, TEST_CLUSTER, bssid);
 
         doAnswer(invocation -> {
             // we don't rely on the Init-Reboot state to renew previous cached IP lease.
@@ -2290,7 +2303,8 @@
                 true /* isDhcpLeaseCacheEnabled */, false /* isDhcpRapidCommitEnabled */,
                 TEST_DEFAULT_MTU, false /* isDhcpIpConflictDetectEnabled */,
                 false /* isIPv6OnlyPreferredEnabled */,
-                null /* captivePortalApiUrl */, displayName, scanResultInfo);
+                null /* captivePortalApiUrl */, displayName, null /* scanResultInfo */,
+                layer2Info);
         verifyIPv4OnlyProvisioningSuccess(Collections.singletonList(CLIENT_ADDR));
         assertIpMemoryStoreNetworkAttributes(TEST_LEASE_DURATION_S, currentTime, TEST_DEFAULT_MTU);
 
@@ -2336,37 +2350,31 @@
     @Test @SignatureRequiredTest(reason = "TODO: evaluate whether signature perms are required")
     public void testDhcpRoaming() throws Exception {
         doDhcpRoamingTest(false /* hasMismatchedIpAddress */, "\"0001docomo\"" /* display name */,
-                TEST_DHCP_ROAM_SSID, TEST_DEFAULT_BSSID, true /* expectRoaming */);
+                MacAddress.fromString(TEST_DEFAULT_BSSID), true /* expectRoaming */);
     }
 
     @Test @SignatureRequiredTest(reason = "TODO: evaluate whether signature perms are required")
     public void testDhcpRoaming_invalidBssid() throws Exception {
         doDhcpRoamingTest(false /* hasMismatchedIpAddress */, "\"0001docomo\"" /* display name */,
-                TEST_DHCP_ROAM_SSID, TEST_DHCP_ROAM_BSSID, false /* expectRoaming */);
+                MacAddress.fromString(TEST_DHCP_ROAM_BSSID), false /* expectRoaming */);
     }
 
     @Test @SignatureRequiredTest(reason = "TODO: evaluate whether signature perms are required")
-    public void testDhcpRoaming_nullScanResultInfo() throws Exception {
+    public void testDhcpRoaming_nullBssid() throws Exception {
         doDhcpRoamingTest(false /* hasMismatchedIpAddress */, "\"0001docomo\"" /* display name */,
-                null /* SSID */, null /* BSSID */, false /* expectRoaming */);
-    }
-
-    @Test @SignatureRequiredTest(reason = "TODO: evaluate whether signature perms are required")
-    public void testDhcpRoaming_invalidSsid() throws Exception {
-        doDhcpRoamingTest(false /* hasMismatchedIpAddress */, "\"0001docomo\"" /* display name */,
-                TEST_DEFAULT_SSID, TEST_DEFAULT_BSSID, false /* expectRoaming */);
+                null /* BSSID */, false /* expectRoaming */);
     }
 
     @Test @SignatureRequiredTest(reason = "TODO: evaluate whether signature perms are required")
     public void testDhcpRoaming_invalidDisplayName() throws Exception {
         doDhcpRoamingTest(false /* hasMismatchedIpAddress */, "\"test-ssid\"" /* display name */,
-                TEST_DHCP_ROAM_SSID, TEST_DEFAULT_BSSID, false /* expectRoaming */);
+                MacAddress.fromString(TEST_DEFAULT_BSSID), false /* expectRoaming */);
     }
 
     @Test @SignatureRequiredTest(reason = "TODO: evaluate whether signature perms are required")
     public void testDhcpRoaming_mismatchedLeasedIpAddress() throws Exception {
         doDhcpRoamingTest(true /* hasMismatchedIpAddress */, "\"0001docomo\"" /* display name */,
-                TEST_DHCP_ROAM_SSID, TEST_DEFAULT_BSSID, true /* expectRoaming */);
+                MacAddress.fromString(TEST_DEFAULT_BSSID), true /* expectRoaming */);
     }
 
     private void performDualStackProvisioning() throws Exception {
@@ -2860,10 +2868,13 @@
 
     private void startGratuitousArpAndNaAfterRoamingTest(boolean isGratuitousArpNaRoamingEnabled,
             boolean hasIpv4, boolean hasIpv6) throws Exception {
+        final Layer2Information layer2Info = new Layer2Information(TEST_L2KEY, TEST_CLUSTER,
+                MacAddress.fromString(TEST_DEFAULT_BSSID));
         final ScanResultInfo scanResultInfo =
                 makeScanResultInfo(TEST_DEFAULT_SSID, TEST_DEFAULT_BSSID);
         final ProvisioningConfiguration.Builder prov = new ProvisioningConfiguration.Builder()
                 .withoutIpReachabilityMonitor()
+                .withLayer2Information(layer2Info)
                 .withScanResultInfo(scanResultInfo)
                 .withDisplayName("ssid");
         if (!hasIpv4) prov.withoutIPv4();
@@ -2957,4 +2968,119 @@
         assertEquals(0, naList.size());
         assertEquals(1, arpList.size());
     }
+
+    private void doInitialBssidSetupTest(final Layer2Information layer2Info,
+            final ScanResultInfo scanResultInfo) throws Exception {
+        ProvisioningConfiguration.Builder prov = new ProvisioningConfiguration.Builder()
+                .withoutIpReachabilityMonitor()
+                .withLayer2Information(layer2Info)
+                .withScanResultInfo(scanResultInfo)
+                .withDisplayName("\"0001docomo\"")
+                .withoutIPv6();
+
+        setDhcpFeatures(false /* isDhcpLeaseCacheEnabled */, true /* shouldReplyRapidCommitAck */,
+                false /* isDhcpIpConflictDetectEnabled */, false /* isIPv6OnlyPreferredEnabled */);
+        startIpClientProvisioning(prov.build());
+
+        handleDhcpPackets(true /* isSuccessLease */, TEST_LEASE_DURATION_S,
+                true /* shouldReplyRapidCommitAck */, TEST_DEFAULT_MTU, null /* serverSentUrl */);
+        verifyIPv4OnlyProvisioningSuccess(Collections.singletonList(CLIENT_ADDR));
+        forceLayer2Roaming();
+    }
+
+    @Test @IgnoreUpTo(Build.VERSION_CODES.R)
+    public void testSetInitialBssidFromLayer2Info() throws Exception {
+        final Layer2Information layer2Info = new Layer2Information(TEST_L2KEY, TEST_CLUSTER,
+                MacAddress.fromString(TEST_DEFAULT_BSSID));
+
+        doInitialBssidSetupTest(layer2Info, null /* scanResultInfo */);
+
+        // Initial BSSID comes from layer2Info, it's different with target roaming bssid,
+        // then verify that DHCPREQUEST packet is sent after roaming.
+        final DhcpPacket packet = getNextDhcpPacket();
+        assertTrue(packet instanceof DhcpRequestPacket);
+    }
+
+    @Test @IgnoreUpTo(Build.VERSION_CODES.R)
+    public void testSetInitialBssidFromLayer2Info_NullBssid() throws Exception {
+        final Layer2Information layer2Info = new Layer2Information(TEST_L2KEY, TEST_CLUSTER,
+                null /* bssid */);
+        final ScanResultInfo scanResultInfo =
+                makeScanResultInfo(TEST_DEFAULT_SSID, TEST_DHCP_ROAM_BSSID);
+
+        doInitialBssidSetupTest(layer2Info, scanResultInfo);
+
+        // Initial BSSID comes from layer2Info, it's null, no DHCPREQUEST packet
+        // will be sent after roaming.
+        final DhcpPacket packet = getNextDhcpPacket(TEST_TIMEOUT_MS);
+        assertNull(packet);
+    }
+
+    @Test @IgnoreUpTo(Build.VERSION_CODES.R)
+    public void testSetInitialBssidFromLayer2Info_SameRoamingBssid() throws Exception {
+        final Layer2Information layer2Info = new Layer2Information(TEST_L2KEY, TEST_CLUSTER,
+                MacAddress.fromString(TEST_DHCP_ROAM_BSSID));
+
+        doInitialBssidSetupTest(layer2Info, null /* scanResultInfo */);
+
+        // Initial BSSID comes from layer2Info, it's same with target roaming bssid,
+        // no DHCPREQUEST packet will be sent after roaming.
+        final DhcpPacket packet = getNextDhcpPacket(TEST_TIMEOUT_MS);
+        assertNull(packet);
+    }
+
+    @Test @IgnoreAfter(Build.VERSION_CODES.R)
+    public void testSetInitialBssidFromScanResultInfo() throws Exception {
+        final ScanResultInfo scanResultInfo =
+                makeScanResultInfo(TEST_DEFAULT_SSID, TEST_DEFAULT_BSSID);
+
+        doInitialBssidSetupTest(null /* layer2Info */, scanResultInfo);
+
+        // Initial BSSID comes from ScanResultInfo, it's different with target roaming bssid,
+        // then verify that DHCPREQUEST packet is sent after roaming.
+        final DhcpPacket packet = getNextDhcpPacket();
+        assertTrue(packet instanceof DhcpRequestPacket);
+    }
+
+    @Test @IgnoreAfter(Build.VERSION_CODES.R)
+    public void testSetInitialBssidFromScanResultInfo_SameRoamingBssid() throws Exception {
+        final ScanResultInfo scanResultInfo =
+                makeScanResultInfo(TEST_DEFAULT_SSID, TEST_DHCP_ROAM_BSSID);
+
+        doInitialBssidSetupTest(null /* layer2Info */, scanResultInfo);
+
+        // Initial BSSID comes from ScanResultInfo, it's same with target roaming bssid,
+        // no DHCPREQUEST packet will be sent after roaming.
+        final DhcpPacket packet = getNextDhcpPacket(TEST_TIMEOUT_MS);
+        assertNull(packet);
+    }
+
+    @Test @IgnoreAfter(Build.VERSION_CODES.R)
+    public void testSetInitialBssidFromScanResultInfo_BrokenInitialBssid() throws Exception {
+        final ScanResultInfo scanResultInfo =
+                makeScanResultInfo(TEST_DEFAULT_SSID, "00:11:22:33:44:");
+
+        doInitialBssidSetupTest(null /* layer2Info */, scanResultInfo);
+
+        // Initial BSSID comes from ScanResultInfo, it's broken MAC address format and fallback
+        // to null layer2Info, no DHCPREQUEST packet will be sent after roaming.
+        final DhcpPacket packet = getNextDhcpPacket(TEST_TIMEOUT_MS);
+        assertNull(packet);
+    }
+
+    @Test @IgnoreAfter(Build.VERSION_CODES.R)
+    public void testSetInitialBssidFromScanResultInfo_BrokenInitialBssidFallback()
+            throws Exception {
+        final Layer2Information layer2Info = new Layer2Information(TEST_L2KEY, TEST_CLUSTER,
+                MacAddress.fromString(TEST_DEFAULT_BSSID));
+        final ScanResultInfo scanResultInfo =
+                makeScanResultInfo(TEST_DEFAULT_SSID, "00:11:22:33:44:");
+
+        doInitialBssidSetupTest(layer2Info, scanResultInfo);
+
+        // Initial BSSID comes from ScanResultInfo, it's broken MAC address format and fallback
+        // to check layer2Info, then verify DHCPREQUEST packet will be sent after roaming.
+        final DhcpPacket packet = getNextDhcpPacket();
+        assertTrue(packet instanceof DhcpRequestPacket);
+    }
 }