diff options
| -rw-r--r-- | service/java/com/android/server/wifi/WifiScoreReport.java | 170 | ||||
| -rw-r--r-- | tests/wifitests/Android.bp | 6 | ||||
| -rw-r--r-- | tests/wifitests/src/com/android/server/wifi/WifiScoreReportTest.java | 74 |
3 files changed, 161 insertions, 89 deletions
diff --git a/service/java/com/android/server/wifi/WifiScoreReport.java b/service/java/com/android/server/wifi/WifiScoreReport.java index ad3c1ca27..5561a9104 100644 --- a/service/java/com/android/server/wifi/WifiScoreReport.java +++ b/service/java/com/android/server/wifi/WifiScoreReport.java @@ -56,11 +56,10 @@ public class WifiScoreReport { // Cache of the last score private int mScore = ConnectedScore.WIFI_MAX_SCORE; - private int mSessionId = INVALID_SESSION_ID; private final ScoringParams mScoringParams; private final Clock mClock; - private int mSessionNumber = 0; + private int mSessionNumber = 0; // not to be confused with sessionid, this just counts resets private String mInterfaceName; private final BssidBlocklistMonitor mBssidBlocklistMonitor; private long mLastScoreBreachLowTimeMillis = -1; @@ -75,16 +74,19 @@ public class WifiScoreReport { WifiThreadRunner mWifiThreadRunner; /** - * Callback proxy. See {@link WifiManager#ScoreUpdateObserver}. + * Callback proxy. See {@link android.net.wifi.WifiManager.ScoreUpdateObserver}. */ private class ScoreUpdateObserverProxy extends IScoreUpdateObserver.Stub { @Override public void notifyScoreUpdate(int sessionId, int score) { mWifiThreadRunner.post(() -> { - if (mWifiConnectedNetworkScorerHolder == null) { - return; - } - if (!validateSessionId(sessionId)) { + if (mWifiConnectedNetworkScorerHolder == null + || sessionId == INVALID_SESSION_ID + || sessionId != getCurrentSessionId()) { + Log.w(TAG, "Ignoring stale/invalid external score" + + " sessionId=" + sessionId + + " currentSessionId=" + getCurrentSessionId() + + " score=" + score); return; } if (mNetworkAgent != null) { @@ -108,21 +110,20 @@ public class WifiScoreReport { @Override public void triggerUpdateOfWifiUsabilityStats(int sessionId) { mWifiThreadRunner.post(() -> { - if (mWifiConnectedNetworkScorerHolder == null) { - return; - } - if (!validateSessionId(sessionId)) { + if (mWifiConnectedNetworkScorerHolder == null + || sessionId == INVALID_SESSION_ID + || sessionId != getCurrentSessionId() + || mInterfaceName == null) { + Log.w(TAG, "Ignoring triggerUpdateOfWifiUsabilityStats" + + " sessionId=" + sessionId + + " currentSessionId=" + getCurrentSessionId() + + " interfaceName=" + mInterfaceName); return; } - if (mInterfaceName == null) { - Log.e(TAG, "getWifiLinkLayerStats called without an interface"); - return; - } - WifiLinkLayerStats stats = mWifiNative.getWifiLinkLayerStats(mInterfaceName); // update mWifiInfo - // TODO: b/149583741 Better coordinate this class and ClientModeImpl to remove + // TODO(b/153075963): Better coordinate this class and ClientModeImpl to remove // redundant codes below and in ClientModeImpl#fetchRssiLinkSpeedAndFrequencyNative. WifiNl80211Manager.SignalPollResult pollResult = mWifiNative.signalPoll(mInterfaceName); @@ -159,34 +160,19 @@ public class WifiScoreReport { } } + // TODO(b/153075963): This should not be plumbed through WifiMetrics mWifiMetrics.updateWifiUsabilityStatsEntries(mWifiInfo, stats); }); } - - private boolean validateSessionId(int sessionId) { - if (sessionId == INVALID_SESSION_ID) { - Log.e(TAG, "Called with invalid ID"); - return false; - } - if (mSessionId == INVALID_SESSION_ID) { - if (mVerboseLoggingEnabled) Log.d(TAG, "Called with score after session reset"); - return false; - } - if (sessionId != mSessionId) { - // This indicates a more serious timing issue - Log.w(TAG, "Called with old session ID"); - return false; - } - return true; - } } /** - * Container for storing info about external scorer and tracks its death. + * Container for storing info about external scorer and tracking its death. */ private final class WifiConnectedNetworkScorerHolder implements IBinder.DeathRecipient { private final IBinder mBinder; private final IWifiConnectedNetworkScorer mScorer; + private int mSessionId = INVALID_SESSION_ID; WifiConnectedNetworkScorerHolder(IBinder binder, IWifiConnectedNetworkScorer scorer) { mBinder = binder; @@ -209,10 +195,9 @@ public class WifiScoreReport { /** * App hosting the binder has died. */ + @Override public void binderDied() { - mWifiThreadRunner.post(() -> { - revertAospConnectedScorer(); - }); + mWifiThreadRunner.post(() -> revertToDefaultConnectedScorer()); } /** @@ -223,10 +208,39 @@ public class WifiScoreReport { } /** - * Get Wi-Fi connected network scorer + * Starts a new scoring session. */ - public IWifiConnectedNetworkScorer getScorer() { - return mScorer; + public void startSession(int sessionId) { + if (sessionId == INVALID_SESSION_ID) { + throw new IllegalArgumentException(); + } + if (mSessionId != INVALID_SESSION_ID) { + // This is not expected to happen, log if it does + Log.e(TAG, "Stopping session " + mSessionId + " before starting " + sessionId); + stopSession(); + } + // Bail now if the scorer has gone away + if (this != mWifiConnectedNetworkScorerHolder) { + return; + } + mSessionId = sessionId; + try { + mScorer.onStart(sessionId); + } catch (RemoteException e) { + Log.e(TAG, "Unable to start Wifi connected network scorer " + this, e); + revertToDefaultConnectedScorer(); + } + } + public void stopSession() { + final int sessionId = mSessionId; + if (sessionId == INVALID_SESSION_ID) return; + mSessionId = INVALID_SESSION_ID; + try { + mScorer.onStop(sessionId); + } catch (RemoteException e) { + Log.e(TAG, "Unable to stop Wifi connected network scorer " + this, e); + revertToDefaultConnectedScorer(); + } } } @@ -353,7 +367,7 @@ public class WifiScoreReport { mScore = score; } - private void updateWifiMetrics(long now, int s2, int score) { + private int getCurrentNetId() { int netId = 0; if (mNetworkAgent != null) { final Network network = mNetworkAgent.getNetwork(); @@ -361,6 +375,28 @@ public class WifiScoreReport { netId = network.getNetId(); } } + return netId; + } + + private int getCurrentSessionId() { + return sessionIdFromNetId(getCurrentNetId()); + } + + /** + * Encodes a network id into a scoring session id. + * + * We use a different numeric value for session id and the network id + * to make it clear that these are not the same thing. However, for + * easier debugging, the network id can be recovered by dropping the + * last decimal digit (at least until they get very, very, large). + */ + public static int sessionIdFromNetId(final int netId) { + if (netId <= 0) return INVALID_SESSION_ID; + return (int) (((long) netId * 10 + (8 - (netId % 9))) % Integer.MAX_VALUE + 1); + } + + private void updateWifiMetrics(long now, int s2, int score) { + int netId = getCurrentNetId(); mAggressiveConnectedScore.updateUsingWifiInfo(mWifiInfo, now); int s1 = mAggressiveConnectedScore.generateScore(); @@ -516,6 +552,7 @@ public class WifiScoreReport { */ public boolean setWifiConnectedNetworkScorer(IBinder binder, IWifiConnectedNetworkScorer scorer) { + if (binder == null || scorer == null) return false; // Enforce that only a single scorer can be set successfully. if (mWifiConnectedNetworkScorerHolder != null) { Log.e(TAG, "Failed to set current scorer because one scorer is already set"); @@ -532,15 +569,17 @@ public class WifiScoreReport { scorer.onSetScoreUpdateObserver(mScoreUpdateObserver); } catch (RemoteException e) { Log.e(TAG, "Unable to set score update observer " + scorer, e); - revertAospConnectedScorer(); + revertToDefaultConnectedScorer(); return false; } - if (mSessionId != INVALID_SESSION_ID) { - startConnectedNetworkScorer(mSessionId); - } // Disable AOSP scorer mVelocityBasedConnectedScore = null; mWifiMetrics.setIsExternalWifiScorerOn(true); + // If there is already a connection, start a new session + final int netId = getCurrentNetId(); + if (netId > 0) { + startConnectedNetworkScorer(netId); + } return true; } @@ -552,26 +591,25 @@ public class WifiScoreReport { return; } mWifiConnectedNetworkScorerHolder.reset(); - revertAospConnectedScorer(); + revertToDefaultConnectedScorer(); } /** * Start the registered Wi-Fi connected network scorer. - * @param sessionId + * @param netId identifies the current android.net.Network */ - public void startConnectedNetworkScorer(int sessionId) { - mSessionId = sessionId; - if (mWifiConnectedNetworkScorerHolder == null) { + public void startConnectedNetworkScorer(int netId) { + final int sessionId = getCurrentSessionId(); + if (mWifiConnectedNetworkScorerHolder == null + || netId != getCurrentNetId() + || sessionId == INVALID_SESSION_ID) { + Log.w(TAG, "Cannot start external scoring" + + " netId=" + netId + + " currentNetId=" + getCurrentNetId() + + " sessionId=" + sessionId); return; } - try { - IWifiConnectedNetworkScorer scorer = mWifiConnectedNetworkScorerHolder.getScorer(); - scorer.onStart(sessionId); - } catch (RemoteException e) { - Log.e(TAG, "Unable to start Wifi connected network scorer " - + mWifiConnectedNetworkScorerHolder, e); - revertAospConnectedScorer(); - } + mWifiConnectedNetworkScorerHolder.startSession(sessionId); mLastScoreBreachLowTimeMillis = INVALID_WALL_CLOCK_MILLIS; } @@ -582,15 +620,8 @@ public class WifiScoreReport { if (mWifiConnectedNetworkScorerHolder == null) { return; } - try { - IWifiConnectedNetworkScorer scorer = mWifiConnectedNetworkScorerHolder.getScorer(); - scorer.onStop(mSessionId); - } catch (RemoteException e) { - Log.e(TAG, "Unable to stop Wifi connected network scorer " - + mWifiConnectedNetworkScorerHolder, e); - } + mWifiConnectedNetworkScorerHolder.stopSession(); - mSessionId = INVALID_SESSION_ID; long millis = mClock.getWallClockMillis(); // Blocklist the current BSS if ((mLastScoreBreachLowTimeMillis != INVALID_WALL_CLOCK_MILLIS) @@ -605,7 +636,6 @@ public class WifiScoreReport { /** * Set NetworkAgent - * @param agent */ public void setNetworkAgent(NetworkAgent agent) { mNetworkAgent = agent; @@ -626,8 +656,8 @@ public class WifiScoreReport { mInterfaceName = ifaceName; } - private void revertAospConnectedScorer() { - // Create AOSP scorer and null external Wifi connected network scorer + private void revertToDefaultConnectedScorer() { + Log.d(TAG, "Using VelocityBasedConnectedScore"); mVelocityBasedConnectedScore = new VelocityBasedConnectedScore(mScoringParams, mClock); mWifiConnectedNetworkScorerHolder = null; mWifiMetrics.setIsExternalWifiScorerOn(false); diff --git a/tests/wifitests/Android.bp b/tests/wifitests/Android.bp index 7fd2a5f75..2f3cbad1b 100644 --- a/tests/wifitests/Android.bp +++ b/tests/wifitests/Android.bp @@ -900,6 +900,9 @@ android_test { "com.android.server.wifi.util.KeyValueListParser", "com.android.server.wifi.util.KeyValueListParser$*", "com.android.server.wifi.util.KeyValueListParser.**", + "com.android.server.wifi.util.LruConnectionTracker", + "com.android.server.wifi.util.LruConnectionTracker$*", + "com.android.server.wifi.util.LruConnectionTracker.**", "com.android.server.wifi.util.LruList", "com.android.server.wifi.util.LruList$*", "com.android.server.wifi.util.LruList.**", @@ -927,6 +930,9 @@ android_test { "com.android.server.wifi.util.ScanResultUtil", "com.android.server.wifi.util.ScanResultUtil$*", "com.android.server.wifi.util.ScanResultUtil.**", + "com.android.server.wifi.util.SettingsMigrationDataHolder", + "com.android.server.wifi.util.SettingsMigrationDataHolder$*", + "com.android.server.wifi.util.SettingsMigrationDataHolder.**", "com.android.server.wifi.util.StringUtil", "com.android.server.wifi.util.StringUtil$*", "com.android.server.wifi.util.StringUtil.**", diff --git a/tests/wifitests/src/com/android/server/wifi/WifiScoreReportTest.java b/tests/wifitests/src/com/android/server/wifi/WifiScoreReportTest.java index 41775ec25..5fe8eaac7 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiScoreReportTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiScoreReportTest.java @@ -60,7 +60,6 @@ import com.android.wifi.resources.R; import org.junit.After; import org.junit.Before; import org.junit.Test; -import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -82,7 +81,8 @@ public class WifiScoreReportTest extends WifiBaseTest { } } - private static final int TEST_SESSION_ID = 8603706; + private static final int TEST_NETWORK_ID = 860370; + private static final int TEST_SESSION_ID = 8603703; // last digit is a check digit FakeClock mClock; WifiConfiguration mWifiConfiguration; @@ -100,6 +100,7 @@ public class WifiScoreReportTest extends WifiBaseTest { @Mock IWifiConnectedNetworkScorer mWifiConnectedNetworkScorer; @Mock WifiNative mWifiNative; @Mock BssidBlocklistMonitor mBssidBlocklistMonitor; + @Mock Network mNetwork; private TestLooper mLooper; public class WifiConnectedNetworkScorerImpl extends IWifiConnectedNetworkScorer.Stub { @@ -189,10 +190,11 @@ public class WifiScoreReportTest extends WifiBaseTest { final ConnectivityManager cm = mock(ConnectivityManager.class); when(mContext.getSystemService(Context.CONNECTIVITY_SERVICE)).thenReturn(cm); when(cm.registerNetworkAgent(any(), any(), any(), any(), anyInt(), any(), anyInt())) - .thenReturn(mock(Network.class)); + .thenReturn(mNetwork); + when(mNetwork.getNetId()).thenReturn(0); mNetworkAgent = spy(new TestNetworkAgent(mContext)); mClock = new FakeClock(); - mScoringParams = new ScoringParams(mContext); + mScoringParams = new ScoringParams(); mWifiThreadRunner = new WifiThreadRunner(new Handler(mLooper.getLooper())); mWifiScoreReport = new WifiScoreReport(mScoringParams, mClock, mWifiMetrics, mWifiInfo, mWifiNative, mBssidBlocklistMonitor, mWifiThreadRunner); @@ -412,7 +414,7 @@ public class WifiScoreReportTest extends WifiBaseTest { asks++; } } - assertTrue(oops + " asks:" + asks, asks > 5 && asks < 12); + assertTrue(oops + " asks:" + asks, asks > 5 && asks < 15); } @@ -609,16 +611,45 @@ public class WifiScoreReportTest extends WifiBaseTest { } /** + * Verify netId to sessionId conversion. + */ + @Test + public void testSessionId() throws Exception { + assertEquals(-1, WifiScoreReport.sessionIdFromNetId(Integer.MIN_VALUE)); + assertEquals(-1, WifiScoreReport.sessionIdFromNetId(-42)); + assertEquals(-1, WifiScoreReport.sessionIdFromNetId(-1)); + assertEquals(-1, WifiScoreReport.sessionIdFromNetId(0)); + assertEquals(18, WifiScoreReport.sessionIdFromNetId(1)); + assertEquals(3339, WifiScoreReport.sessionIdFromNetId(333)); + assertEquals(TEST_SESSION_ID, WifiScoreReport.sessionIdFromNetId(TEST_NETWORK_ID)); + int dangerOfOverflow = Integer.MAX_VALUE / 10; + assertEquals(214748364, dangerOfOverflow); + assertEquals(2147483646, WifiScoreReport.sessionIdFromNetId(dangerOfOverflow)); + assertEquals(8, WifiScoreReport.sessionIdFromNetId(dangerOfOverflow + 1)); + assertEquals(8, WifiScoreReport.sessionIdFromNetId(Integer.MAX_VALUE)); + } + + /** * Verify that client gets session ID when onStart() method is called. */ @Test public void testClientGetSessionIdOnStart() throws Exception { - ArgumentCaptor<Integer> startId = ArgumentCaptor.forClass(Integer.class); // Register Client for verification. mWifiScoreReport.setWifiConnectedNetworkScorer(mAppBinder, mWifiConnectedNetworkScorer); - mWifiScoreReport.startConnectedNetworkScorer(TEST_SESSION_ID); - verify(mWifiConnectedNetworkScorer).onStart(startId.capture()); - assertEquals((int) startId.getValue(), TEST_SESSION_ID); + when(mNetwork.getNetId()).thenReturn(TEST_NETWORK_ID); + mWifiScoreReport.startConnectedNetworkScorer(TEST_NETWORK_ID); + verify(mWifiConnectedNetworkScorer).onStart(TEST_SESSION_ID); + } + + /** + * Verify that onStart is called if there is already an active network when registered. + */ + @Test + public void testClientStartOnRegWhileActive() throws Exception { + when(mNetwork.getNetId()).thenReturn(TEST_NETWORK_ID); + mWifiScoreReport.startConnectedNetworkScorer(TEST_NETWORK_ID); + mWifiScoreReport.setWifiConnectedNetworkScorer(mAppBinder, mWifiConnectedNetworkScorer); + verify(mWifiConnectedNetworkScorer).onStart(TEST_SESSION_ID); } /** @@ -626,13 +657,12 @@ public class WifiScoreReportTest extends WifiBaseTest { */ @Test public void testClientGetSessionIdOnStop() throws Exception { - ArgumentCaptor<Integer> stopId = ArgumentCaptor.forClass(Integer.class); // Register Client for verification. mWifiScoreReport.setWifiConnectedNetworkScorer(mAppBinder, mWifiConnectedNetworkScorer); - mWifiScoreReport.startConnectedNetworkScorer(TEST_SESSION_ID); + when(mNetwork.getNetId()).thenReturn(TEST_NETWORK_ID); + mWifiScoreReport.startConnectedNetworkScorer(TEST_NETWORK_ID); mWifiScoreReport.stopConnectedNetworkScorer(); - verify(mWifiConnectedNetworkScorer).onStop(stopId.capture()); - assertEquals((int) stopId.getValue(), TEST_SESSION_ID); + verify(mWifiConnectedNetworkScorer).onStop(TEST_SESSION_ID); } /** @@ -655,7 +685,8 @@ public class WifiScoreReportTest extends WifiBaseTest { WifiConnectedNetworkScorerImpl scorerImpl = new WifiConnectedNetworkScorerImpl(); // Register Client for verification. mWifiScoreReport.setWifiConnectedNetworkScorer(mAppBinder, scorerImpl); - mWifiScoreReport.startConnectedNetworkScorer(TEST_SESSION_ID); + when(mNetwork.getNetId()).thenReturn(TEST_NETWORK_ID); + mWifiScoreReport.startConnectedNetworkScorer(TEST_NETWORK_ID); assertEquals(TEST_SESSION_ID, scorerImpl.mSessionId); @@ -687,7 +718,8 @@ public class WifiScoreReportTest extends WifiBaseTest { // Register Client for verification. mWifiScoreReport.setWifiConnectedNetworkScorer(mAppBinder, scorerImpl); mWifiScoreReport.setInterfaceName("wlan0"); - mWifiScoreReport.startConnectedNetworkScorer(TEST_SESSION_ID); + when(mNetwork.getNetId()).thenReturn(TEST_NETWORK_ID); + mWifiScoreReport.startConnectedNetworkScorer(TEST_NETWORK_ID); scorerImpl.mScoreUpdateObserver.triggerUpdateOfWifiUsabilityStats(scorerImpl.mSessionId); mLooper.dispatchAll(); @@ -703,7 +735,8 @@ public class WifiScoreReportTest extends WifiBaseTest { WifiConnectedNetworkScorerImpl scorerImpl = new WifiConnectedNetworkScorerImpl(); // Register Client for verification. mWifiScoreReport.setWifiConnectedNetworkScorer(mAppBinder, scorerImpl); - mWifiScoreReport.startConnectedNetworkScorer(TEST_SESSION_ID); + when(mNetwork.getNetId()).thenReturn(TEST_NETWORK_ID); + mWifiScoreReport.startConnectedNetworkScorer(TEST_NETWORK_ID); mClock.mStepMillis = 0; mClock.mWallClockMillis = 5001; @@ -732,7 +765,8 @@ public class WifiScoreReportTest extends WifiBaseTest { WifiConnectedNetworkScorerImpl scorerImpl = new WifiConnectedNetworkScorerImpl(); // Register Client for verification. mWifiScoreReport.setWifiConnectedNetworkScorer(mAppBinder, scorerImpl); - mWifiScoreReport.startConnectedNetworkScorer(TEST_SESSION_ID); + when(mNetwork.getNetId()).thenReturn(TEST_NETWORK_ID); + mWifiScoreReport.startConnectedNetworkScorer(TEST_NETWORK_ID); mClock.mStepMillis = 0; mClock.mWallClockMillis = 10; @@ -755,7 +789,8 @@ public class WifiScoreReportTest extends WifiBaseTest { WifiConnectedNetworkScorerImpl scorerImpl = new WifiConnectedNetworkScorerImpl(); // Register Client for verification. mWifiScoreReport.setWifiConnectedNetworkScorer(mAppBinder, scorerImpl); - mWifiScoreReport.startConnectedNetworkScorer(TEST_SESSION_ID); + when(mNetwork.getNetId()).thenReturn(TEST_NETWORK_ID); + mWifiScoreReport.startConnectedNetworkScorer(TEST_NETWORK_ID); mClock.mStepMillis = 0; mClock.mWallClockMillis = 10; @@ -777,7 +812,8 @@ public class WifiScoreReportTest extends WifiBaseTest { WifiConnectedNetworkScorerImpl scorerImpl = new WifiConnectedNetworkScorerImpl(); // Register Client for verification. mWifiScoreReport.setWifiConnectedNetworkScorer(mAppBinder, scorerImpl); - mWifiScoreReport.startConnectedNetworkScorer(TEST_SESSION_ID); + when(mNetwork.getNetId()).thenReturn(TEST_NETWORK_ID); + mWifiScoreReport.startConnectedNetworkScorer(TEST_NETWORK_ID); mClock.mStepMillis = 0; mClock.mWallClockMillis = 10; |