From a0ac6bca2e58de9505eff0abc287896bc6f436c3 Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Wed, 29 Apr 2020 12:25:33 +0000 Subject: [PATCH 1/2] [SP18] Poll network stats in OffloadController to support data warning The OEM implemented tether offload does not support data warning since the HAL only tells the hardware about data limit but not warning. However, to add such interface in HAL needs OEM to comply and implement in hardware. Thus, as a short-term solution, polls network statistics from HAL and notify upper layer when it reaches the alert quota set by NetworkStatsService. Note that when CPU is sleeping, the data warning of tethering offload will not work since the polling is also suspended. Test: manual Test: atest OffloadControllerTest Bug: 149467454 Change-Id: I2467b64779b74cd5fec73b42fb303584f52cb1cb Merged-In: I2467b64779b74cd5fec73b42fb303584f52cb1cb (cherry picked from commit 43f3b153fb0dcc5a5f81845f3df6a44a4e619123) --- .../tethering/OffloadController.java | 74 +++++++++++++++++++ .../tethering/OffloadHardwareInterface.java | 1 - 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/packages/Tethering/src/com/android/networkstack/tethering/OffloadController.java b/packages/Tethering/src/com/android/networkstack/tethering/OffloadController.java index c007c174fe4f5..445a09d7618de 100644 --- a/packages/Tethering/src/com/android/networkstack/tethering/OffloadController.java +++ b/packages/Tethering/src/com/android/networkstack/tethering/OffloadController.java @@ -23,6 +23,7 @@ import static android.net.NetworkStats.SET_DEFAULT; import static android.net.NetworkStats.TAG_NONE; import static android.net.NetworkStats.UID_ALL; import static android.net.NetworkStats.UID_TETHERING; +import static android.net.netstats.provider.NetworkStatsProvider.QUOTA_UNLIMITED; import static android.provider.Settings.Global.TETHER_OFFLOAD_DISABLED; import android.annotation.NonNull; @@ -76,6 +77,7 @@ public class OffloadController { private static final boolean DBG = false; private static final String ANYIP = "0.0.0.0"; private static final ForwardedStats EMPTY_STATS = new ForwardedStats(); + private static final int DEFAULT_PERFORM_POLL_INTERVAL_MS = 5000; @VisibleForTesting enum StatsType { @@ -115,6 +117,16 @@ public class OffloadController { // includes upstream interfaces that have a quota set. private HashMap mInterfaceQuotas = new HashMap<>(); + // Tracking remaining alert quota. Unlike limit quota is subject to interface, the alert + // quota is interface independent and global for tether offload. Note that this is only + // accessed on the handler thread and in the constructor. + private long mRemainingAlertQuota = QUOTA_UNLIMITED; + // Runnable that used to schedule the next stats poll. + private final Runnable mScheduledPollingTask = () -> { + updateStatsForCurrentUpstream(); + maybeSchedulePollingStats(); + }; + private int mNatUpdateCallbacksReceived; private int mNatUpdateNetlinkErrors; @@ -240,6 +252,7 @@ public class OffloadController { mLog.log("tethering offload started"); mNatUpdateCallbacksReceived = 0; mNatUpdateNetlinkErrors = 0; + maybeSchedulePollingStats(); } return isStarted; } @@ -255,6 +268,9 @@ public class OffloadController { mHwInterface.stopOffloadControl(); mControlInitialized = false; mConfigInitialized = false; + if (mHandler.hasCallbacks(mScheduledPollingTask)) { + mHandler.removeCallbacks(mScheduledPollingTask); + } if (wasStarted) mLog.log("tethering offload stopped"); } @@ -345,6 +361,11 @@ public class OffloadController { @Override public void onSetAlert(long quotaBytes) { // TODO: Ask offload HAL to notify alert without stopping traffic. + // Post it to handler thread since it access remaining quota bytes. + mHandler.post(() -> { + updateAlertQuota(quotaBytes); + maybeSchedulePollingStats(); + }); } } @@ -366,15 +387,66 @@ public class OffloadController { // the stats for each interface, and does not observe partial writes where rxBytes is // updated and txBytes is not. ForwardedStats diff = mHwInterface.getForwardedStats(iface); + final long usedAlertQuota = diff.rxBytes + diff.txBytes; ForwardedStats base = mForwardedStats.get(iface); if (base != null) { diff.add(base); } + + // Update remaining alert quota if it is still positive. + if (mRemainingAlertQuota > 0 && usedAlertQuota > 0) { + // Trim to zero if overshoot. + final long newQuota = Math.max(mRemainingAlertQuota - usedAlertQuota, 0); + updateAlertQuota(newQuota); + } + mForwardedStats.put(iface, diff); // diff is a new object, just created by getForwardedStats(). Therefore, anyone reading from // mForwardedStats (i.e., any caller of getTetherStats) will see the new stats immediately. } + /** + * Update remaining alert quota, fire the {@link NetworkStatsProvider#notifyAlertReached()} + * callback when it reaches zero. This can be invoked either from service setting the alert, or + * {@code maybeUpdateStats} when updating stats. Note that this can be only called on + * handler thread. + * + * @param newQuota non-negative value to indicate the new quota, or + * {@link NetworkStatsProvider#QUOTA_UNLIMITED} to indicate there is no + * quota. + */ + private void updateAlertQuota(long newQuota) { + if (newQuota < QUOTA_UNLIMITED) { + throw new IllegalArgumentException("invalid quota value " + newQuota); + } + if (mRemainingAlertQuota == newQuota) return; + + mRemainingAlertQuota = newQuota; + if (mRemainingAlertQuota == 0) { + mLog.i("notifyAlertReached"); + if (mStatsProvider != null) mStatsProvider.notifyAlertReached(); + } + } + + /** + * Schedule polling if needed, this will be stopped if offload has been + * stopped or remaining quota reaches zero or upstream is empty. + * Note that this can be only called on handler thread. + */ + private void maybeSchedulePollingStats() { + if (!isPollingStatsNeeded()) return; + + if (mHandler.hasCallbacks(mScheduledPollingTask)) { + mHandler.removeCallbacks(mScheduledPollingTask); + } + mHandler.postDelayed(mScheduledPollingTask, DEFAULT_PERFORM_POLL_INTERVAL_MS); + } + + private boolean isPollingStatsNeeded() { + return started() && mRemainingAlertQuota > 0 + && !TextUtils.isEmpty(currentUpstreamInterface()); + } + private boolean maybeUpdateDataLimit(String iface) { // setDataLimit may only be called while offload is occurring on this upstream. if (!started() || !TextUtils.equals(iface, currentUpstreamInterface())) { @@ -414,6 +486,8 @@ public class OffloadController { final String iface = currentUpstreamInterface(); if (!TextUtils.isEmpty(iface)) mForwardedStats.putIfAbsent(iface, EMPTY_STATS); + maybeSchedulePollingStats(); + // TODO: examine return code and decide what to do if programming // upstream parameters fails (probably just wait for a subsequent // onOffloadEvent() callback to tell us offload is available again and diff --git a/packages/Tethering/src/com/android/networkstack/tethering/OffloadHardwareInterface.java b/packages/Tethering/src/com/android/networkstack/tethering/OffloadHardwareInterface.java index c4a1078d0be05..293f8eae32d5e 100644 --- a/packages/Tethering/src/com/android/networkstack/tethering/OffloadHardwareInterface.java +++ b/packages/Tethering/src/com/android/networkstack/tethering/OffloadHardwareInterface.java @@ -308,7 +308,6 @@ public class OffloadHardwareInterface { return stats; } - mLog.log(logmsg + YIELDS + stats); return stats; } From 87cfa0d8e6dd2161d84863ba360b088039d606ed Mon Sep 17 00:00:00 2001 From: Treehugger Robot Date: Wed, 29 Apr 2020 13:28:12 +0000 Subject: [PATCH 2/2] [SP18.1] add dependency object to OffloadController In order to mock constant in unit test, a dependency object is introduced with minimum code change to achieve this. Test: atest TetheringTests Bug: 149467454 Change-Id: I38628daddcb7be7c74846e78d36dc88f065b97d9 Merged-In: I38628daddcb7be7c74846e78d36dc88f065b97d9 (cherry picked from commit 7eff4acc9e051e5360526cbc4d80964198b6d73e) --- .../tethering/OffloadController.java | 17 +++++++++++++++-- .../networkstack/tethering/Tethering.java | 2 +- .../tethering/OffloadControllerTest.java | 8 +++++++- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/packages/Tethering/src/com/android/networkstack/tethering/OffloadController.java b/packages/Tethering/src/com/android/networkstack/tethering/OffloadController.java index 445a09d7618de..1817f35f1dcde 100644 --- a/packages/Tethering/src/com/android/networkstack/tethering/OffloadController.java +++ b/packages/Tethering/src/com/android/networkstack/tethering/OffloadController.java @@ -130,8 +130,20 @@ public class OffloadController { private int mNatUpdateCallbacksReceived; private int mNatUpdateNetlinkErrors; + @NonNull + private final Dependencies mDeps; + + // TODO: Put more parameters in constructor into dependency object. + static class Dependencies { + int getPerformPollInterval() { + // TODO: Consider make this configurable. + return DEFAULT_PERFORM_POLL_INTERVAL_MS; + } + } + public OffloadController(Handler h, OffloadHardwareInterface hwi, - ContentResolver contentResolver, NetworkStatsManager nsm, SharedLog log) { + ContentResolver contentResolver, NetworkStatsManager nsm, SharedLog log, + @NonNull Dependencies deps) { mHandler = h; mHwInterface = hwi; mContentResolver = contentResolver; @@ -147,6 +159,7 @@ public class OffloadController { provider = null; } mStatsProvider = provider; + mDeps = deps; } /** Start hardware offload. */ @@ -439,7 +452,7 @@ public class OffloadController { if (mHandler.hasCallbacks(mScheduledPollingTask)) { mHandler.removeCallbacks(mScheduledPollingTask); } - mHandler.postDelayed(mScheduledPollingTask, DEFAULT_PERFORM_POLL_INTERVAL_MS); + mHandler.postDelayed(mScheduledPollingTask, mDeps.getPerformPollInterval()); } private boolean isPollingStatsNeeded() { diff --git a/packages/Tethering/src/com/android/networkstack/tethering/Tethering.java b/packages/Tethering/src/com/android/networkstack/tethering/Tethering.java index 4e16c49caa0e8..0a95a5e0073b8 100644 --- a/packages/Tethering/src/com/android/networkstack/tethering/Tethering.java +++ b/packages/Tethering/src/com/android/networkstack/tethering/Tethering.java @@ -273,7 +273,7 @@ public class Tethering { mHandler = mTetherMasterSM.getHandler(); mOffloadController = new OffloadController(mHandler, mDeps.getOffloadHardwareInterface(mHandler, mLog), mContext.getContentResolver(), - statsManager, mLog); + statsManager, mLog, new OffloadController.Dependencies()); mUpstreamNetworkMonitor = mDeps.getUpstreamNetworkMonitor(mContext, mTetherMasterSM, mLog, TetherMasterSM.EVENT_UPSTREAM_CALLBACK); mForwardedDownstreams = new LinkedHashSet<>(); diff --git a/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/OffloadControllerTest.java b/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/OffloadControllerTest.java index 65797200fa92f..088a663190b81 100644 --- a/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/OffloadControllerTest.java +++ b/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/OffloadControllerTest.java @@ -116,6 +116,12 @@ public class OffloadControllerTest { private final ArgumentCaptor mControlCallbackCaptor = ArgumentCaptor.forClass(OffloadHardwareInterface.ControlCallback.class); private MockContentResolver mContentResolver; + private OffloadController.Dependencies mDeps = new OffloadController.Dependencies() { + @Override + int getPerformPollInterval() { + return 0; + } + }; @Before public void setUp() { MockitoAnnotations.initMocks(this); @@ -150,7 +156,7 @@ public class OffloadControllerTest { private OffloadController makeOffloadController() throws Exception { OffloadController offload = new OffloadController(new Handler(Looper.getMainLooper()), - mHardware, mContentResolver, mStatsManager, new SharedLog("test")); + mHardware, mContentResolver, mStatsManager, new SharedLog("test"), mDeps); final ArgumentCaptor tetherStatsProviderCaptor = ArgumentCaptor.forClass(OffloadController.OffloadTetheringStatsProvider.class);