From e2219127c2ec54059982709f01b342e19370be25 Mon Sep 17 00:00:00 2001 From: Michael Plass Date: Sun, 1 Apr 2018 21:54:38 -0700 Subject: Request ip reachablility checks on low wifi score This is under control of the nud param of wifi_score_params Bug: 73383969 Test: Unit tests Test: Manual test with nud=1, and with nud=10 Change-Id: Ia7f0ec8d0ff1bd658b414bcce389dcb9ce357dc0 --- .../com/android/server/wifi/ScoringParams.java | 2 +- .../com/android/server/wifi/WifiScoreReport.java | 101 ++++++++++++++------- .../com/android/server/wifi/WifiStateMachine.java | 9 +- .../com/android/server/wifi/ScoringParamsTest.java | 2 +- .../android/server/wifi/WifiScoreReportTest.java | 84 ++++++++++++++++- 5 files changed, 160 insertions(+), 38 deletions(-) diff --git a/service/java/com/android/server/wifi/ScoringParams.java b/service/java/com/android/server/wifi/ScoringParams.java index 24966fb3d..5b9951ff8 100644 --- a/service/java/com/android/server/wifi/ScoringParams.java +++ b/service/java/com/android/server/wifi/ScoringParams.java @@ -68,7 +68,7 @@ public class ScoringParams { public static final String KEY_NUD = "nud"; public static final int MIN_NUD = 0; public static final int MAX_NUD = 10; - public int nud = 0; + public int nud = 8; Values() { } diff --git a/service/java/com/android/server/wifi/WifiScoreReport.java b/service/java/com/android/server/wifi/WifiScoreReport.java index 32529f88d..c6a7105be 100644 --- a/service/java/com/android/server/wifi/WifiScoreReport.java +++ b/service/java/com/android/server/wifi/WifiScoreReport.java @@ -39,9 +39,8 @@ public class WifiScoreReport { private boolean mVerboseLoggingEnabled = false; private static final long FIRST_REASONABLE_WALL_CLOCK = 1490000000000L; // mid-December 2016 - // Cache of the last score report. - private String mReport; - private boolean mReportValid = false; + // Cache of the last score + private int mScore = NetworkAgent.WIFI_BASE_SCORE; private final ScoringParams mScoringParams; private final Clock mClock; @@ -57,39 +56,18 @@ public class WifiScoreReport { mVelocityBasedConnectedScore = new VelocityBasedConnectedScore(scoringParams, clock); } - /** - * Method returning the String representation of the last score report. - * - * @return String score report - */ - public String getLastReport() { - return mReport; - } - /** * Reset the last calculated score. */ public void reset() { - mReport = ""; - if (mReportValid) { - mSessionNumber++; - mReportValid = false; - } + mSessionNumber++; + mScore = NetworkAgent.WIFI_BASE_SCORE; + mLastKnownNudCheckScore = ConnectedScore.WIFI_TRANSITION_SCORE; mAggressiveConnectedScore.reset(); mVelocityBasedConnectedScore.reset(); if (mVerboseLoggingEnabled) Log.d(TAG, "reset"); } - /** - * Checks if the last report data is valid or not. This will be cleared when {@link #reset()} is - * invoked. - * - * @return true if valid, false otherwise. - */ - public boolean isLastReportValid() { - return mReportValid; - } - /** * Enable/Disable verbose logging in score report generation. */ @@ -162,7 +140,7 @@ public class WifiScoreReport { //report score if (score != wifiInfo.score) { if (mVerboseLoggingEnabled) { - Log.d(TAG, " report new wifi score " + score); + Log.d(TAG, "report new wifi score " + score); } wifiInfo.score = score; if (networkAgent != null) { @@ -170,9 +148,67 @@ public class WifiScoreReport { } } - mReport = String.format(Locale.US, " score=%d", score); - mReportValid = true; wifiMetrics.incrementWifiScoreCount(score); + mScore = score; + } + + private static final double TIME_CONSTANT_MILLIS = 30.0e+3; + private static final long NUD_THROTTLE_MILLIS = 5000; + private long mLastKnownNudCheckTimeMillis = 0; + private int mLastKnownNudCheckScore = ConnectedScore.WIFI_TRANSITION_SCORE; + private int mNudYes = 0; // Counts when we voted for a NUD + private int mNudCount = 0; // Counts when we were told a NUD was sent + + /** + * Recommends that a layer 3 check be done + * + * The caller can use this to (help) decide that an IP reachability check + * is desirable. The check is not done here; that is the caller's responsibility. + * + * @return true to indicate that an IP reachability check is recommended + */ + public boolean shouldCheckIpLayer() { + int nud = mScoringParams.getNudKnob(); + if (nud == 0) { + return false; + } + long millis = mClock.getWallClockMillis(); + long deltaMillis = millis - mLastKnownNudCheckTimeMillis; + // Don't ever ask back-to-back - allow at least 5 seconds + // for the previous one to finish. + if (deltaMillis < NUD_THROTTLE_MILLIS) { + return false; + } + // nud is between 1 and 10 at this point + double deltaLevel = 11 - nud; + // nextNudBreach is the bar the score needs to cross before we ask for NUD + double nextNudBreach = ConnectedScore.WIFI_TRANSITION_SCORE; + // If we were below threshold the last time we checked, then compute a new bar + // that starts down from there and decays exponentially back up to the steady-state + // bar. If 5 time constants have passed, we are 99% of the way there, so skip the math. + if (mLastKnownNudCheckScore < ConnectedScore.WIFI_TRANSITION_SCORE + && deltaMillis < 5.0 * TIME_CONSTANT_MILLIS) { + double a = Math.exp(-deltaMillis / TIME_CONSTANT_MILLIS); + nextNudBreach = a * (mLastKnownNudCheckScore - deltaLevel) + (1.0 - a) * nextNudBreach; + } + if (mScore >= nextNudBreach) { + return false; + } + mNudYes++; + return true; + } + + /** + * Should be called when a reachability check has been issued + * + * When the caller has requested an IP reachability check, calling this will + * help to rate-limit requests via shouldCheckIpLayer() + */ + public void noteIpCheck() { + long millis = mClock.getWallClockMillis(); + mLastKnownNudCheckTimeMillis = millis; + mLastKnownNudCheckScore = mScore; + mNudCount++; } /** @@ -201,10 +237,11 @@ public class WifiScoreReport { try { String timestamp = new SimpleDateFormat("MM-dd HH:mm:ss.SSS").format(new Date(now)); s = String.format(Locale.US, // Use US to avoid comma/decimal confusion - "%s,%d,%d,%.1f,%.1f,%.1f,%d,%d,%.2f,%.2f,%.2f,%.2f,%d,%d,%d", + "%s,%d,%d,%.1f,%.1f,%.1f,%d,%d,%.2f,%.2f,%.2f,%.2f,%d,%d,%d,%d,%d", timestamp, mSessionNumber, netId, rssi, filteredRssi, rssiThreshold, freq, linkSpeed, txSuccessRate, txRetriesRate, txBadRate, rxSuccessRate, + mNudYes, mNudCount, s1, s2, score); } catch (Exception e) { Log.e(TAG, "format problem", e); @@ -235,7 +272,7 @@ public class WifiScoreReport { history = new LinkedList<>(mLinkMetricsHistory); } pw.println("time,session,netid,rssi,filtered_rssi,rssi_threshold," - + "freq,linkspeed,tx_good,tx_retry,tx_bad,rx_pps,s1,s2,score"); + + "freq,linkspeed,tx_good,tx_retry,tx_bad,rx_pps,nudrq,nuds,s1,s2,score"); for (String line : history) { pw.println(line); } diff --git a/service/java/com/android/server/wifi/WifiStateMachine.java b/service/java/com/android/server/wifi/WifiStateMachine.java index 2d60e3553..100503713 100644 --- a/service/java/com/android/server/wifi/WifiStateMachine.java +++ b/service/java/com/android/server/wifi/WifiStateMachine.java @@ -2210,9 +2210,7 @@ public class WifiStateMachine extends StateMachine { if (report != null) { sb.append(" ").append(report); } - if (mWifiScoreReport.isLastReportValid()) { - sb.append(mWifiScoreReport.getLastReport()); - } + sb.append(String.format(" score=%d", mWifiInfo.score)); break; case CMD_START_CONNECT: case WifiManager.CONNECT_NETWORK: @@ -4352,6 +4350,7 @@ public class WifiStateMachine extends StateMachine { // interest (e.g. routers); harmless if none are configured. if (state == SupplicantState.COMPLETED) { mIpClient.confirmConfiguration(); + mWifiScoreReport.noteIpCheck(); } if (!SupplicantState.isDriverActive(state)) { @@ -5189,6 +5188,10 @@ public class WifiStateMachine extends StateMachine { // Send the update score to network agent. mWifiScoreReport.calculateAndReportScore( mWifiInfo, mNetworkAgent, mWifiMetrics); + if (mWifiScoreReport.shouldCheckIpLayer()) { + mIpClient.confirmConfiguration(); + mWifiScoreReport.noteIpCheck(); + } } sendMessageDelayed(obtainMessage(CMD_RSSI_POLL, mRssiPollToken, 0), mPollRssiIntervalMsecs); diff --git a/tests/wifitests/src/com/android/server/wifi/ScoringParamsTest.java b/tests/wifitests/src/com/android/server/wifi/ScoringParamsTest.java index 42a4c133d..fc9b33ad2 100644 --- a/tests/wifitests/src/com/android/server/wifi/ScoringParamsTest.java +++ b/tests/wifitests/src/com/android/server/wifi/ScoringParamsTest.java @@ -94,7 +94,7 @@ public class ScoringParamsTest { public void testToString() throws Exception { mScoringParams = new ScoringParams(); String expect = - "rssi2=-83:-80:-73:-60,rssi5=-80:-77:-70:-57,pps=0:1:100,horizon=15,nud=0"; + "rssi2=-83:-80:-73:-60,rssi5=-80:-77:-70:-57,pps=0:1:100,horizon=15,nud=8"; String actual = mScoringParams.toString(); assertEquals(expect, actual); } diff --git a/tests/wifitests/src/com/android/server/wifi/WifiScoreReportTest.java b/tests/wifitests/src/com/android/server/wifi/WifiScoreReportTest.java index bf662f01a..38e45d306 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiScoreReportTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiScoreReportTest.java @@ -16,6 +16,8 @@ package com.android.server.wifi; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.AdditionalAnswers.answerVoid; import static org.mockito.ArgumentMatchers.anyString; @@ -272,7 +274,7 @@ public class WifiScoreReportTest { mWifiInfo.rxSuccessRate = mScoringParams.getYippeeSkippyPacketsPerSecond() + 0.1; assertTrue(mWifiInfo.txSuccessRate > 10); mWifiInfo.setFrequency(5220); - for (int r = -30; r >= -200; r -= 2) { + for (int r = -30; r >= -120; r -= 2) { mWifiInfo.setRssi(r); mWifiScoreReport.calculateAndReportScore(mWifiInfo, mNetworkAgent, mWifiMetrics); assertTrue(mWifiInfo.score > ConnectedScore.WIFI_TRANSITION_SCORE); @@ -288,6 +290,86 @@ public class WifiScoreReportTest { assertTrue(mWifiInfo.score < ConnectedScore.WIFI_TRANSITION_SCORE); } + /** + * Never ask for nud check when nud=0 + */ + @Test + public void neverAskForNudCheckWhenNudKnobIsZero() throws Exception { + assertTrue(mScoringParams.update("nud=0")); + assertEquals(0, mScoringParams.getNudKnob()); + mWifiInfo.setFrequency(5220); + for (int rssi = -30; rssi >= -120; rssi -= 1) { + mWifiInfo.setRssi(rssi); + mWifiScoreReport.calculateAndReportScore(mWifiInfo, mNetworkAgent, mWifiMetrics); + assertFalse(mWifiScoreReport.shouldCheckIpLayer()); + } + } + + /** + * Eventually ask for nud check when nud=1 + */ + @Test + public void eventuallyAskForNudCheckWhenNudKnobIsOne() throws Exception { + String oops = "nud=1"; + long lastAskedMillis = 0; // Check that we don't send too soon + int asks = 0; // Keep track of how many time we asked + assertTrue(mScoringParams.update("nud=1")); + assertEquals(1, mScoringParams.getNudKnob()); + mWifiInfo.setFrequency(5220); + for (int rssi = -40; rssi >= -120; rssi -= 1) { + mWifiInfo.setRssi(rssi); + mWifiScoreReport.calculateAndReportScore(mWifiInfo, mNetworkAgent, mWifiMetrics); + boolean ask = mWifiScoreReport.shouldCheckIpLayer(); + if (ask) { + assertTrue(mWifiInfo.score < ConnectedScore.WIFI_TRANSITION_SCORE); + assertTrue(oops, mClock.mWallClockMillis >= lastAskedMillis + 5000); + lastAskedMillis = mClock.mWallClockMillis; + oops += " " + lastAskedMillis + ":" + mWifiInfo.score; + mWifiScoreReport.noteIpCheck(); + asks++; + } + } + assertTrue(oops + " asks:" + asks, asks > 5 && asks < 12); + } + + + /** + * Ask for more nud checks when nud=10 + */ + @Test + public void askForMoreNudChecksWhenNudKnobIsBigger() throws Exception { + String oops = "nud=10"; + long lastAskedMillis = 0; // Check that we don't send too soon + int asks = 0; // Keep track of how many time we asked + assertTrue(mScoringParams.update("nud=10")); + assertEquals(10, mScoringParams.getNudKnob()); + mWifiInfo.setFrequency(5220); + for (int rssi = -40; rssi >= -120; rssi -= 1) { + mWifiInfo.setRssi(rssi); + mWifiScoreReport.calculateAndReportScore(mWifiInfo, mNetworkAgent, mWifiMetrics); + boolean ask = mWifiScoreReport.shouldCheckIpLayer(); + if (ask) { + assertTrue(mWifiInfo.score < ConnectedScore.WIFI_TRANSITION_SCORE); + assertTrue(oops, mClock.mWallClockMillis >= lastAskedMillis + 5000); + lastAskedMillis = mClock.mWallClockMillis; + oops += " " + lastAskedMillis + ":" + mWifiInfo.score; + mWifiScoreReport.noteIpCheck(); + asks++; + } + } + assertTrue(oops + " asks:" + asks, asks > 12 && asks < 80); + } + + /** + * Test initial conditions, and after reset() + */ + @Test + public void exerciseReset() throws Exception { + assertFalse(mWifiScoreReport.shouldCheckIpLayer()); + mWifiScoreReport.reset(); + assertFalse(mWifiScoreReport.shouldCheckIpLayer()); + } + /** * This setup causes some reports to be generated when println * methods are called, to check for "concurrent" modification -- cgit v1.2.3 From 66e108c8bcc57ac1b9eef2a165907b7444c2065a Mon Sep 17 00:00:00 2001 From: Michael Plass Date: Wed, 4 Apr 2018 11:54:34 -0700 Subject: [WifiScoreReport] Use tx retries as well as tx bad for adjustment Devices have implemented the tx-bad counters quite differently; bring in the retry count as well to dilute the difference. (Also more correct in principle, when counters are properly implemented). Also, reset the adjustment on the reset() call, and add a unit test that would have caught this bug. Bug: 74613347 Test: Unit tests Change-Id: Ice6ccdf39c6d222a89733d2e61e1f3c26116ebb7 --- .../java/com/android/server/wifi/VelocityBasedConnectedScore.java | 6 ++++-- .../com/android/server/wifi/VelocityBasedConnectedScoreTest.java | 5 +++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/service/java/com/android/server/wifi/VelocityBasedConnectedScore.java b/service/java/com/android/server/wifi/VelocityBasedConnectedScore.java index cb47978df..e7d61f733 100644 --- a/service/java/com/android/server/wifi/VelocityBasedConnectedScore.java +++ b/service/java/com/android/server/wifi/VelocityBasedConnectedScore.java @@ -62,6 +62,7 @@ public class VelocityBasedConnectedScore extends ConnectedScore { @Override public void reset() { mLastMillis = 0; + mThresholdAdjustment = 0; } /** @@ -153,8 +154,9 @@ public class VelocityBasedConnectedScore extends ConnectedScore { if (txSuccessPps < mMinimumPpsForMeasuringSuccess) return; if (rxSuccessPps < mMinimumPpsForMeasuringSuccess) return; double txBadPps = wifiInfo.txBadRate; - double probabilityOfSuccessfulTx = txSuccessPps / (txSuccessPps + txBadPps); - if (probabilityOfSuccessfulTx >= 0.2) { + double txRetriesPps = wifiInfo.txRetriesRate; + double probabilityOfSuccessfulTx = txSuccessPps / (txSuccessPps + txBadPps + txRetriesPps); + if (probabilityOfSuccessfulTx > 0.2) { // May want this amount to vary with how close to threshold we are mThresholdAdjustment -= 0.5; } diff --git a/tests/wifitests/src/com/android/server/wifi/VelocityBasedConnectedScoreTest.java b/tests/wifitests/src/com/android/server/wifi/VelocityBasedConnectedScoreTest.java index 23ae660f5..0d5f5efa6 100644 --- a/tests/wifitests/src/com/android/server/wifi/VelocityBasedConnectedScoreTest.java +++ b/tests/wifitests/src/com/android/server/wifi/VelocityBasedConnectedScoreTest.java @@ -109,6 +109,11 @@ public class VelocityBasedConnectedScoreTest { } int score = mVelocityBasedConnectedScore.generateScore(); assertTrue(score > ConnectedScore.WIFI_TRANSITION_SCORE); + // If we reset, should be below threshold after the first input + mVelocityBasedConnectedScore.reset(); + mVelocityBasedConnectedScore.updateUsingWifiInfo(mWifiInfo, mClock.getWallClockMillis()); + score = mVelocityBasedConnectedScore.generateScore(); + assertTrue(score < ConnectedScore.WIFI_TRANSITION_SCORE); } /** -- cgit v1.2.3