From a9af37006b20d012ab7dcd329c088cb8d0795289 Mon Sep 17 00:00:00 2001 From: xshu Date: Mon, 11 May 2020 17:24:00 -0700 Subject: BssidBlocklistMonitor history It's been found that often the local log does not cover enough time for Bssid blocklist related debugging. This change will make sure that we will get data on the latest 30 unblocked BSSIDs during bugreport dump. Bug: 156004955 Test: atest com.android.server.wifi Change-Id: I16efa8cae4b2cabdc5727aab214b66ccee465a5c --- .../android/server/wifi/BssidBlocklistMonitor.java | 79 ++++++++++++++++++++-- .../server/wifi/BssidBlocklistMonitorTest.java | 20 ++++++ 2 files changed, 94 insertions(+), 5 deletions(-) diff --git a/service/java/com/android/server/wifi/BssidBlocklistMonitor.java b/service/java/com/android/server/wifi/BssidBlocklistMonitor.java index 2d93e304d..96770443d 100644 --- a/service/java/com/android/server/wifi/BssidBlocklistMonitor.java +++ b/service/java/com/android/server/wifi/BssidBlocklistMonitor.java @@ -24,6 +24,7 @@ import android.util.ArrayMap; import android.util.LocalLog; import android.util.Log; +import com.android.internal.annotations.VisibleForTesting; import com.android.wifi.resources.R; import java.io.FileDescriptor; @@ -32,6 +33,7 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.util.ArrayList; import java.util.Calendar; +import java.util.LinkedList; import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -39,7 +41,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; /** - * This classes manages the addition and removal of BSSIDs to the BSSID blocklist, which is used + * This class manages the addition and removal of BSSIDs to the BSSID blocklist, which is used * for firmware roaming and network selection. */ public class BssidBlocklistMonitor { @@ -107,6 +109,9 @@ public class BssidBlocklistMonitor { // Map of bssid to BssidStatus private Map mBssidStatusMap = new ArrayMap<>(); + // Keeps history of 30 blocked BSSIDs that were most recently removed. + private BssidStatusHistoryLogger mBssidStatusHistoryLogger = new BssidStatusHistoryLogger(30); + /** * Create a new instance of BssidBlocklistMonitor */ @@ -149,9 +154,10 @@ public class BssidBlocklistMonitor { */ public void dump(FileDescriptor fd, PrintWriter pw, String[] args) { pw.println("Dump of BssidBlocklistMonitor"); - pw.println("BssidBlocklistMonitor - Bssid blocklist Begin ----"); + pw.println("BssidBlocklistMonitor - Bssid blocklist begin ----"); mBssidStatusMap.values().stream().forEach(entry -> pw.println(entry)); - pw.println("BssidBlocklistMonitor - Bssid blocklist End ----"); + pw.println("BssidBlocklistMonitor - Bssid blocklist end ----"); + mBssidStatusHistoryLogger.dump(pw); } private void addToBlocklist(@NonNull BssidStatus entry, long durationMs, String reasonString) { @@ -399,7 +405,14 @@ public class BssidBlocklistMonitor { */ public void clearBssidBlocklistForSsid(@NonNull String ssid) { int prevSize = mBssidStatusMap.size(); - mBssidStatusMap.entrySet().removeIf(e -> e.getValue().ssid.equals(ssid)); + mBssidStatusMap.entrySet().removeIf(e -> { + BssidStatus status = e.getValue(); + if (status.ssid.equals(ssid)) { + mBssidStatusHistoryLogger.add(status, "clearBssidBlocklistForSsid"); + return true; + } + return false; + }); int diff = prevSize - mBssidStatusMap.size(); if (diff > 0) { localLog(TAG + " clearBssidBlocklistForSsid: SSID=" + ssid @@ -413,6 +426,9 @@ public class BssidBlocklistMonitor { public void clearBssidBlocklist() { if (mBssidStatusMap.size() > 0) { int prevSize = mBssidStatusMap.size(); + for (BssidStatus status : mBssidStatusMap.values()) { + mBssidStatusHistoryLogger.add(status, "clearBssidBlocklist"); + } mBssidStatusMap.clear(); localLog(TAG + " clearBssidBlocklist: num BSSIDs cleared=" + (prevSize - mBssidStatusMap.size())); @@ -449,6 +465,7 @@ public class BssidBlocklistMonitor { BssidStatus status = e.getValue(); if (status.isInBlocklist) { if (status.blocklistEndTimeMs < curTime) { + mBssidStatusHistoryLogger.add(status, "updateAndGetBssidBlocklistInternal"); return true; } builder.accept(status); @@ -490,6 +507,50 @@ public class BssidBlocklistMonitor { } } + @VisibleForTesting + public int getBssidStatusHistoryLoggerSize() { + return mBssidStatusHistoryLogger.size(); + } + + private class BssidStatusHistoryLogger { + private LinkedList mLogHistory = new LinkedList<>(); + private int mBufferSize; + + BssidStatusHistoryLogger(int bufferSize) { + mBufferSize = bufferSize; + } + + public void add(BssidStatus bssidStatus, String trigger) { + // only log history for Bssids that had been blocked. + if (bssidStatus == null || !bssidStatus.isInBlocklist) { + return; + } + StringBuilder sb = new StringBuilder(); + mCalendar.setTimeInMillis(mClock.getWallClockMillis()); + sb.append(", logTimeMs=" + + String.format("%tm-%td %tH:%tM:%tS.%tL", mCalendar, mCalendar, + mCalendar, mCalendar, mCalendar, mCalendar)); + sb.append(", trigger=" + trigger); + mLogHistory.add(bssidStatus.toString() + sb.toString()); + if (mLogHistory.size() > mBufferSize) { + mLogHistory.removeFirst(); + } + } + + @VisibleForTesting + public int size() { + return mLogHistory.size(); + } + + public void dump(PrintWriter pw) { + pw.println("BssidBlocklistMonitor - Bssid blocklist history begin ----"); + for (String line : mLogHistory) { + pw.println(line); + } + pw.println("BssidBlocklistMonitor - Bssid blocklist history end ----"); + } + } + /** * Helper class that counts the number of failures per BSSID. */ @@ -502,6 +563,7 @@ public class BssidBlocklistMonitor { // The following are used to flag how long this BSSID stays in the blocklist. public boolean isInBlocklist; public long blocklistEndTimeMs; + public long blocklistStartTimeMs; BssidStatus(String bssid, String ssid) { this.bssid = bssid; @@ -522,7 +584,8 @@ public class BssidBlocklistMonitor { */ public void addToBlocklist(long durationMs, String blockReason) { isInBlocklist = true; - blocklistEndTimeMs = mClock.getWallClockMillis() + durationMs; + blocklistStartTimeMs = mClock.getWallClockMillis(); + blocklistEndTimeMs = blocklistStartTimeMs + durationMs; mBlockReason = blockReason; } @@ -530,7 +593,9 @@ public class BssidBlocklistMonitor { * Remove this BSSID from the blocklist. */ public void removeFromBlocklist() { + mBssidStatusHistoryLogger.add(this, "removeFromBlocklist"); isInBlocklist = false; + blocklistStartTimeMs = 0; blocklistEndTimeMs = 0; mBlockReason = ""; localLog(TAG + " removeFromBlocklist BSSID=" + bssid); @@ -544,6 +609,10 @@ public class BssidBlocklistMonitor { sb.append(", isInBlocklist=" + isInBlocklist); if (isInBlocklist) { sb.append(", blockReason=" + mBlockReason); + mCalendar.setTimeInMillis(blocklistStartTimeMs); + sb.append(", blocklistStartTimeMs=" + + String.format("%tm-%td %tH:%tM:%tS.%tL", mCalendar, mCalendar, + mCalendar, mCalendar, mCalendar, mCalendar)); mCalendar.setTimeInMillis(blocklistEndTimeMs); sb.append(", blocklistEndTimeMs=" + String.format("%tm-%td %tH:%tM:%tS.%tL", mCalendar, mCalendar, diff --git a/tests/wifitests/src/com/android/server/wifi/BssidBlocklistMonitorTest.java b/tests/wifitests/src/com/android/server/wifi/BssidBlocklistMonitorTest.java index 2567faa91..4369a831b 100644 --- a/tests/wifitests/src/com/android/server/wifi/BssidBlocklistMonitorTest.java +++ b/tests/wifitests/src/com/android/server/wifi/BssidBlocklistMonitorTest.java @@ -582,6 +582,24 @@ public class BssidBlocklistMonitorTest { verifyAddTestBssidToBlocklist(); mBssidBlocklistMonitor.clearBssidBlocklist(); assertEquals(0, mBssidBlocklistMonitor.updateAndGetBssidBlocklist().size()); + + } + + /** + * Verify that the BssidStatusHistoryLoggerSize is capped. + */ + @Test + public void testBssidStatusHistoryLoggerSize() { + int bssidStatusHistoryLoggerSize = 30; + for (int i = 0; i < bssidStatusHistoryLoggerSize; i++) { + verifyAddTestBssidToBlocklist(); + mBssidBlocklistMonitor.clearBssidBlocklist(); + assertEquals(i + 1, mBssidBlocklistMonitor.getBssidStatusHistoryLoggerSize()); + } + verifyAddTestBssidToBlocklist(); + mBssidBlocklistMonitor.clearBssidBlocklist(); + assertEquals(bssidStatusHistoryLoggerSize, + mBssidBlocklistMonitor.getBssidStatusHistoryLoggerSize()); } /** @@ -633,6 +651,7 @@ public class BssidBlocklistMonitorTest { // Verify that the BSSID is removed from blocklist by clearBssidBlocklistForSsid mBssidBlocklistMonitor.clearBssidBlocklistForSsid(TEST_SSID_1); assertEquals(0, mBssidBlocklistMonitor.updateAndGetBssidBlocklist().size()); + assertEquals(1, mBssidBlocklistMonitor.getBssidStatusHistoryLoggerSize()); // Add the BSSID to blocklist again. mBssidBlocklistMonitor.blockBssidForDurationMs(TEST_BSSID_1, TEST_SSID_1, testDuration); @@ -641,5 +660,6 @@ public class BssidBlocklistMonitorTest { // Verify that the BSSID is removed from blocklist once the specified duration is over. when(mClock.getWallClockMillis()).thenReturn(testDuration + 1); assertEquals(0, mBssidBlocklistMonitor.updateAndGetBssidBlocklist().size()); + assertEquals(2, mBssidBlocklistMonitor.getBssidStatusHistoryLoggerSize()); } } -- cgit v1.2.3