From 6d8da9f72d485f6f38537540dc4518cef07c08c2 Mon Sep 17 00:00:00 2001 From: junyulai Date: Wed, 10 Mar 2021 19:36:43 +0800 Subject: [PATCH 1/2] [SP34] Adapt onSetWarningAndLimit This is a no-op change that just adapt new API from NetworkStatsProvider to get warning and limit bytes at the same time. This change also stores them locally for subsequent patches to set warning bytes to hardware. Test: Will be included in the subsequent patch. Bug: 149467454 Ignore-AOSP-First: avoid long automerger delay Change-Id: Iec01cb01fd1ce481ce0bd736762baddde1e38084 --- .../tethering/OffloadController.java | 72 ++++++++++++++----- .../tethering/OffloadControllerTest.java | 11 +-- 2 files changed, 63 insertions(+), 20 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/OffloadController.java b/Tethering/src/com/android/networkstack/tethering/OffloadController.java index 44e3916bdf..c612271ee7 100644 --- a/Tethering/src/com/android/networkstack/tethering/OffloadController.java +++ b/Tethering/src/com/android/networkstack/tethering/OffloadController.java @@ -114,11 +114,42 @@ public class OffloadController { private ConcurrentHashMap mForwardedStats = new ConcurrentHashMap<>(16, 0.75F, 1); + private static class InterfaceQuota { + public final long warningBytes; + public final long limitBytes; + + public static InterfaceQuota MAX_VALUE = new InterfaceQuota(Long.MAX_VALUE, Long.MAX_VALUE); + + InterfaceQuota(long warningBytes, long limitBytes) { + this.warningBytes = warningBytes; + this.limitBytes = limitBytes; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof InterfaceQuota)) return false; + InterfaceQuota that = (InterfaceQuota) o; + return warningBytes == that.warningBytes + && limitBytes == that.limitBytes; + } + + @Override + public int hashCode() { + return (int) (warningBytes * 3 + limitBytes * 5); + } + + @Override + public String toString() { + return "InterfaceQuota{" + "warning=" + warningBytes + ", limit=" + limitBytes + '}'; + } + } + // Maps upstream interface names to interface quotas. // Always contains the latest value received from the framework for each interface, regardless // of whether offload is currently running (or is even supported) on that interface. Only // includes upstream interfaces that have a quota set. - private HashMap mInterfaceQuotas = new HashMap<>(); + 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 @@ -263,7 +294,8 @@ public class OffloadController { mLog.i("tethering offload control not supported"); stop(); } else { - mLog.log("tethering offload started"); + mLog.log("tethering offload started, version: " + + OffloadHardwareInterface.halVerToString(mControlHalVersion)); mNatUpdateCallbacksReceived = 0; mNatUpdateNetlinkErrors = 0; maybeSchedulePollingStats(); @@ -322,24 +354,35 @@ public class OffloadController { @Override public void onSetLimit(String iface, long quotaBytes) { + onSetWarningAndLimit(iface, QUOTA_UNLIMITED, quotaBytes); + } + + @Override + public void onSetWarningAndLimit(@NonNull String iface, + long warningBytes, long limitBytes) { // Listen for all iface is necessary since upstream might be changed after limit // is set. mHandler.post(() -> { - final Long curIfaceQuota = mInterfaceQuotas.get(iface); + final InterfaceQuota curIfaceQuota = mInterfaceQuotas.get(iface); + final InterfaceQuota newIfaceQuota = new InterfaceQuota( + warningBytes == QUOTA_UNLIMITED ? Long.MAX_VALUE : warningBytes, + limitBytes == QUOTA_UNLIMITED ? Long.MAX_VALUE : limitBytes); // If the quota is set to unlimited, the value set to HAL is Long.MAX_VALUE, // which is ~8.4 x 10^6 TiB, no one can actually reach it. Thus, it is not // useful to set it multiple times. // Otherwise, the quota needs to be updated to tell HAL to re-count from now even // if the quota is the same as the existing one. - if (null == curIfaceQuota && QUOTA_UNLIMITED == quotaBytes) return; + if (null == curIfaceQuota && InterfaceQuota.MAX_VALUE.equals(newIfaceQuota)) { + return; + } - if (quotaBytes == QUOTA_UNLIMITED) { + if (InterfaceQuota.MAX_VALUE.equals(newIfaceQuota)) { mInterfaceQuotas.remove(iface); } else { - mInterfaceQuotas.put(iface, quotaBytes); + mInterfaceQuotas.put(iface, newIfaceQuota); } - maybeUpdateDataLimit(iface); + maybeUpdateDataWarningAndLimit(iface); }); } @@ -465,18 +508,15 @@ public class OffloadController { >= DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS; } - private boolean maybeUpdateDataLimit(String iface) { - // setDataLimit may only be called while offload is occurring on this upstream. + private boolean maybeUpdateDataWarningAndLimit(String iface) { + // setDataLimit or setDataWarningAndLimit may only be called while offload is occurring + // on this upstream. if (!started() || !TextUtils.equals(iface, currentUpstreamInterface())) { return true; } - Long limit = mInterfaceQuotas.get(iface); - if (limit == null) { - limit = Long.MAX_VALUE; - } - - return mHwInterface.setDataLimit(iface, limit); + final InterfaceQuota quota = mInterfaceQuotas.getOrDefault(iface, InterfaceQuota.MAX_VALUE); + return mHwInterface.setDataLimit(iface, quota.limitBytes); } private void updateStatsForCurrentUpstream() { @@ -630,7 +670,7 @@ public class OffloadController { maybeUpdateStats(prevUpstream); // Data limits can only be set once offload is running on the upstream. - success = maybeUpdateDataLimit(iface); + success = maybeUpdateDataWarningAndLimit(iface); if (!success) { // If we failed to set a data limit, don't use this upstream, because we don't want to // blow through the data limit that we were told to apply. diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/OffloadControllerTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/OffloadControllerTest.java index 88f205445e..14c34f02b4 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/OffloadControllerTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/OffloadControllerTest.java @@ -58,7 +58,6 @@ import android.annotation.NonNull; import android.app.usage.NetworkStatsManager; import android.content.Context; import android.content.pm.ApplicationInfo; -import android.net.ITetheringStatsProvider; import android.net.IpPrefix; import android.net.LinkAddress; import android.net.LinkProperties; @@ -503,8 +502,12 @@ public class OffloadControllerTest { expectedUidStatsDiff); } + /** + * Test OffloadController that uses V1.0 HAL setDataLimit with NetworkStatsProvider#onSetLimit + * which is called by R framework. + */ @Test - public void testSetInterfaceQuota() throws Exception { + public void testSetDataLimit() throws Exception { enableOffload(); final OffloadController offload = startOffloadController(OFFLOAD_HAL_VERSION_1_0, true /*expectStart*/); @@ -540,9 +543,9 @@ public class OffloadControllerTest { waitForIdle(); inOrder.verify(mHardware).setDataLimit(mobileIface, mobileLimit); - // Setting a limit of ITetheringStatsProvider.QUOTA_UNLIMITED causes the limit to be set + // Setting a limit of NetworkStatsProvider.QUOTA_UNLIMITED causes the limit to be set // to Long.MAX_VALUE. - mTetherStatsProvider.onSetLimit(mobileIface, ITetheringStatsProvider.QUOTA_UNLIMITED); + mTetherStatsProvider.onSetLimit(mobileIface, NetworkStatsProvider.QUOTA_UNLIMITED); waitForIdle(); inOrder.verify(mHardware).setDataLimit(mobileIface, Long.MAX_VALUE); From a36f33ef9093cd9d5f5f17caed8421e2ddd2d2f9 Mon Sep 17 00:00:00 2001 From: junyulai Date: Wed, 10 Mar 2021 20:22:30 +0800 Subject: [PATCH 2/2] [SP35] Pass data warning to tethering offload This is supported by: 1. Utilize the new API from both NetworkStatsProvider and IOffloadControl to send data warning quota to hardware. And pass the warning reached notification back to NPMS. 2. Disable software solution introduced in R release for V1.1+ hardware, since now we can fully offload data warning and limit notification to hardware. Test: atest TetheringTests Fix: 149467454 Ignore-AOSP-First: avoid long automerger delay Change-Id: Ie49461694d77ab7f25a549433b01b5b0167bd489 --- .../tethering/OffloadController.java | 33 +++- .../tethering/OffloadControllerTest.java | 160 +++++++++++++++--- 2 files changed, 168 insertions(+), 25 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/OffloadController.java b/Tethering/src/com/android/networkstack/tethering/OffloadController.java index c612271ee7..beb18219b7 100644 --- a/Tethering/src/com/android/networkstack/tethering/OffloadController.java +++ b/Tethering/src/com/android/networkstack/tethering/OffloadController.java @@ -26,6 +26,8 @@ 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 static com.android.networkstack.tethering.OffloadHardwareInterface.OFFLOAD_HAL_VERSION_1_0; +import static com.android.networkstack.tethering.OffloadHardwareInterface.OFFLOAD_HAL_VERSION_1_1; import static com.android.networkstack.tethering.OffloadHardwareInterface.OFFLOAD_HAL_VERSION_NONE; import static com.android.networkstack.tethering.TetheringConfiguration.DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS; @@ -280,6 +282,18 @@ public class OffloadController { } } + @Override + public void onWarningReached() { + if (!started()) return; + mLog.log("onWarningReached"); + + updateStatsForCurrentUpstream(); + if (mStatsProvider != null) { + mStatsProvider.pushTetherStats(); + mStatsProvider.notifyWarningReached(); + } + } + @Override public void onNatTimeoutUpdate(int proto, String srcAddr, int srcPort, @@ -417,7 +431,11 @@ public class OffloadController { @Override public void onSetAlert(long quotaBytes) { - // TODO: Ask offload HAL to notify alert without stopping traffic. + // Ignore set alert calls from HAL V1.1 since the hardware supports set warning now. + // Thus, the software polling mechanism is not needed. + if (!useStatsPolling()) { + return; + } // Post it to handler thread since it access remaining quota bytes. mHandler.post(() -> { updateAlertQuota(quotaBytes); @@ -502,12 +520,17 @@ public class OffloadController { private boolean isPollingStatsNeeded() { return started() && mRemainingAlertQuota > 0 + && useStatsPolling() && !TextUtils.isEmpty(currentUpstreamInterface()) && mDeps.getTetherConfig() != null && mDeps.getTetherConfig().getOffloadPollInterval() >= DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS; } + private boolean useStatsPolling() { + return mControlHalVersion == OFFLOAD_HAL_VERSION_1_0; + } + private boolean maybeUpdateDataWarningAndLimit(String iface) { // setDataLimit or setDataWarningAndLimit may only be called while offload is occurring // on this upstream. @@ -516,7 +539,13 @@ public class OffloadController { } final InterfaceQuota quota = mInterfaceQuotas.getOrDefault(iface, InterfaceQuota.MAX_VALUE); - return mHwInterface.setDataLimit(iface, quota.limitBytes); + final boolean ret; + if (mControlHalVersion >= OFFLOAD_HAL_VERSION_1_1) { + ret = mHwInterface.setDataWarningAndLimit(iface, quota.warningBytes, quota.limitBytes); + } else { + ret = mHwInterface.setDataLimit(iface, quota.limitBytes); + } + return ret; } private void updateStatsForCurrentUpstream() { diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/OffloadControllerTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/OffloadControllerTest.java index 14c34f02b4..d800816055 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/OffloadControllerTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/OffloadControllerTest.java @@ -149,6 +149,7 @@ public class OffloadControllerTest { when(mHardware.setUpstreamParameters(anyString(), any(), any(), any())).thenReturn(true); when(mHardware.getForwardedStats(any())).thenReturn(new ForwardedStats()); when(mHardware.setDataLimit(anyString(), anyLong())).thenReturn(true); + when(mHardware.setDataWarningAndLimit(anyString(), anyLong(), anyLong())).thenReturn(true); } private void enableOffload() { @@ -503,80 +504,166 @@ public class OffloadControllerTest { } /** - * Test OffloadController that uses V1.0 HAL setDataLimit with NetworkStatsProvider#onSetLimit - * which is called by R framework. + * Test OffloadController with different combinations of HAL and framework versions can set + * data warning and/or limit correctly. */ @Test - public void testSetDataLimit() throws Exception { + public void testSetDataWarningAndLimit() throws Exception { + // Verify the OffloadController is called by R framework, where the framework doesn't send + // warning. + checkSetDataWarningAndLimit(false, OFFLOAD_HAL_VERSION_1_0); + checkSetDataWarningAndLimit(false, OFFLOAD_HAL_VERSION_1_1); + // Verify the OffloadController is called by S+ framework, where the framework sends + // warning along with limit. + checkSetDataWarningAndLimit(true, OFFLOAD_HAL_VERSION_1_0); + checkSetDataWarningAndLimit(true, OFFLOAD_HAL_VERSION_1_1); + } + + private void checkSetDataWarningAndLimit(boolean isProviderSetWarning, int controlVersion) + throws Exception { enableOffload(); final OffloadController offload = - startOffloadController(OFFLOAD_HAL_VERSION_1_0, true /*expectStart*/); + startOffloadController(controlVersion, true /*expectStart*/); final String ethernetIface = "eth1"; final String mobileIface = "rmnet_data0"; final long ethernetLimit = 12345; + final long mobileWarning = 123456; final long mobileLimit = 12345678; final LinkProperties lp = new LinkProperties(); lp.setInterfaceName(ethernetIface); - offload.setUpstreamLinkProperties(lp); final InOrder inOrder = inOrder(mHardware); - when(mHardware.setUpstreamParameters(any(), any(), any(), any())).thenReturn(true); + when(mHardware.setUpstreamParameters( + any(), any(), any(), any())).thenReturn(true); when(mHardware.setDataLimit(anyString(), anyLong())).thenReturn(true); + when(mHardware.setDataWarningAndLimit(anyString(), anyLong(), anyLong())).thenReturn(true); + offload.setUpstreamLinkProperties(lp); + // Applying an interface sends the initial quota to the hardware. + if (controlVersion >= OFFLOAD_HAL_VERSION_1_1) { + inOrder.verify(mHardware).setDataWarningAndLimit(ethernetIface, Long.MAX_VALUE, + Long.MAX_VALUE); + } else { + inOrder.verify(mHardware).setDataLimit(ethernetIface, Long.MAX_VALUE); + } + inOrder.verifyNoMoreInteractions(); + + // Verify that set to unlimited again won't cause duplicated calls to the hardware. + if (isProviderSetWarning) { + mTetherStatsProvider.onSetWarningAndLimit(ethernetIface, + NetworkStatsProvider.QUOTA_UNLIMITED, NetworkStatsProvider.QUOTA_UNLIMITED); + } else { + mTetherStatsProvider.onSetLimit(ethernetIface, NetworkStatsProvider.QUOTA_UNLIMITED); + } + waitForIdle(); + inOrder.verifyNoMoreInteractions(); // Applying an interface quota to the current upstream immediately sends it to the hardware. - mTetherStatsProvider.onSetLimit(ethernetIface, ethernetLimit); + if (isProviderSetWarning) { + mTetherStatsProvider.onSetWarningAndLimit(ethernetIface, + NetworkStatsProvider.QUOTA_UNLIMITED, ethernetLimit); + } else { + mTetherStatsProvider.onSetLimit(ethernetIface, ethernetLimit); + } waitForIdle(); - inOrder.verify(mHardware).setDataLimit(ethernetIface, ethernetLimit); + if (controlVersion >= OFFLOAD_HAL_VERSION_1_1) { + inOrder.verify(mHardware).setDataWarningAndLimit(ethernetIface, Long.MAX_VALUE, + ethernetLimit); + } else { + inOrder.verify(mHardware).setDataLimit(ethernetIface, ethernetLimit); + } inOrder.verifyNoMoreInteractions(); // Applying an interface quota to another upstream does not take any immediate action. - mTetherStatsProvider.onSetLimit(mobileIface, mobileLimit); + if (isProviderSetWarning) { + mTetherStatsProvider.onSetWarningAndLimit(mobileIface, mobileWarning, mobileLimit); + } else { + mTetherStatsProvider.onSetLimit(mobileIface, mobileLimit); + } waitForIdle(); - inOrder.verify(mHardware, never()).setDataLimit(anyString(), anyLong()); + if (controlVersion >= OFFLOAD_HAL_VERSION_1_1) { + inOrder.verify(mHardware, never()).setDataWarningAndLimit(anyString(), anyLong(), + anyLong()); + } else { + inOrder.verify(mHardware, never()).setDataLimit(anyString(), anyLong()); + } // Switching to that upstream causes the quota to be applied if the parameters were applied // correctly. lp.setInterfaceName(mobileIface); offload.setUpstreamLinkProperties(lp); waitForIdle(); - inOrder.verify(mHardware).setDataLimit(mobileIface, mobileLimit); + if (controlVersion >= OFFLOAD_HAL_VERSION_1_1) { + inOrder.verify(mHardware).setDataWarningAndLimit(mobileIface, + isProviderSetWarning ? mobileWarning : Long.MAX_VALUE, + mobileLimit); + } else { + inOrder.verify(mHardware).setDataLimit(mobileIface, mobileLimit); + } // Setting a limit of NetworkStatsProvider.QUOTA_UNLIMITED causes the limit to be set // to Long.MAX_VALUE. - mTetherStatsProvider.onSetLimit(mobileIface, NetworkStatsProvider.QUOTA_UNLIMITED); + if (isProviderSetWarning) { + mTetherStatsProvider.onSetWarningAndLimit(mobileIface, + NetworkStatsProvider.QUOTA_UNLIMITED, NetworkStatsProvider.QUOTA_UNLIMITED); + } else { + mTetherStatsProvider.onSetLimit(mobileIface, NetworkStatsProvider.QUOTA_UNLIMITED); + } waitForIdle(); - inOrder.verify(mHardware).setDataLimit(mobileIface, Long.MAX_VALUE); + if (controlVersion >= OFFLOAD_HAL_VERSION_1_1) { + inOrder.verify(mHardware).setDataWarningAndLimit(mobileIface, Long.MAX_VALUE, + Long.MAX_VALUE); + } else { + inOrder.verify(mHardware).setDataLimit(mobileIface, Long.MAX_VALUE); + } - // If setting upstream parameters fails, then the data limit is not set. + // If setting upstream parameters fails, then the data warning and limit is not set. when(mHardware.setUpstreamParameters(any(), any(), any(), any())).thenReturn(false); lp.setInterfaceName(ethernetIface); offload.setUpstreamLinkProperties(lp); - mTetherStatsProvider.onSetLimit(mobileIface, mobileLimit); + if (isProviderSetWarning) { + mTetherStatsProvider.onSetWarningAndLimit(mobileIface, mobileWarning, mobileLimit); + } else { + mTetherStatsProvider.onSetLimit(mobileIface, mobileLimit); + } waitForIdle(); inOrder.verify(mHardware, never()).setDataLimit(anyString(), anyLong()); + inOrder.verify(mHardware, never()).setDataWarningAndLimit(anyString(), anyLong(), + anyLong()); - // If setting the data limit fails while changing upstreams, offload is stopped. + // If setting the data warning and/or limit fails while changing upstreams, offload is + // stopped. when(mHardware.setUpstreamParameters(any(), any(), any(), any())).thenReturn(true); when(mHardware.setDataLimit(anyString(), anyLong())).thenReturn(false); + when(mHardware.setDataWarningAndLimit(anyString(), anyLong(), anyLong())).thenReturn(false); lp.setInterfaceName(mobileIface); offload.setUpstreamLinkProperties(lp); - mTetherStatsProvider.onSetLimit(mobileIface, mobileLimit); + if (isProviderSetWarning) { + mTetherStatsProvider.onSetWarningAndLimit(mobileIface, mobileWarning, mobileLimit); + } else { + mTetherStatsProvider.onSetLimit(mobileIface, mobileLimit); + } waitForIdle(); inOrder.verify(mHardware).getForwardedStats(ethernetIface); inOrder.verify(mHardware).stopOffloadControl(); } @Test - public void testDataLimitCallback() throws Exception { + public void testDataWarningAndLimitCallback() throws Exception { enableOffload(); - final OffloadController offload = - startOffloadController(OFFLOAD_HAL_VERSION_1_0, true /*expectStart*/); + startOffloadController(OFFLOAD_HAL_VERSION_1_0, true /*expectStart*/); OffloadHardwareInterface.ControlCallback callback = mControlCallbackCaptor.getValue(); callback.onStoppedLimitReached(); mTetherStatsProviderCb.expectNotifyStatsUpdated(); + mTetherStatsProviderCb.expectNotifyWarningOrLimitReached(); + + startOffloadController(OFFLOAD_HAL_VERSION_1_1, true /*expectStart*/); + callback = mControlCallbackCaptor.getValue(); + callback.onWarningReached(); + mTetherStatsProviderCb.expectNotifyStatsUpdated(); + mTetherStatsProviderCb.expectNotifyWarningOrLimitReached(); } @Test @@ -764,9 +851,7 @@ public class OffloadControllerTest { // Initialize with fake eth upstream. final String ethernetIface = "eth1"; InOrder inOrder = inOrder(mHardware); - final LinkProperties lp = new LinkProperties(); - lp.setInterfaceName(ethernetIface); - offload.setUpstreamLinkProperties(lp); + offload.setUpstreamLinkProperties(makeEthernetLinkProperties()); // Previous upstream was null, so no stats are fetched. inOrder.verify(mHardware, never()).getForwardedStats(any()); @@ -799,4 +884,33 @@ public class OffloadControllerTest { mTetherStatsProviderCb.assertNoCallback(); verify(mHardware, never()).getForwardedStats(any()); } + + private static LinkProperties makeEthernetLinkProperties() { + final String ethernetIface = "eth1"; + final LinkProperties lp = new LinkProperties(); + lp.setInterfaceName(ethernetIface); + return lp; + } + + private void checkSoftwarePollingUsed(int controlVersion) throws Exception { + enableOffload(); + setOffloadPollInterval(DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS); + OffloadController offload = + startOffloadController(controlVersion, true /*expectStart*/); + offload.setUpstreamLinkProperties(makeEthernetLinkProperties()); + mTetherStatsProvider.onSetAlert(0); + waitForIdle(); + if (controlVersion >= OFFLOAD_HAL_VERSION_1_1) { + mTetherStatsProviderCb.assertNoCallback(); + } else { + mTetherStatsProviderCb.expectNotifyAlertReached(); + } + verify(mHardware, never()).getForwardedStats(any()); + } + + @Test + public void testSoftwarePollingUsed() throws Exception { + checkSoftwarePollingUsed(OFFLOAD_HAL_VERSION_1_0); + checkSoftwarePollingUsed(OFFLOAD_HAL_VERSION_1_1); + } }