diff options
| -rw-r--r-- | services/core/java/com/android/server/connectivity/tethering/OffloadController.java | 65 | ||||
| -rw-r--r-- | tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java | 75 |
2 files changed, 121 insertions, 19 deletions
diff --git a/services/core/java/com/android/server/connectivity/tethering/OffloadController.java b/services/core/java/com/android/server/connectivity/tethering/OffloadController.java index 6d5c428e58f8..788867f9137e 100644 --- a/services/core/java/com/android/server/connectivity/tethering/OffloadController.java +++ b/services/core/java/com/android/server/connectivity/tethering/OffloadController.java @@ -63,6 +63,8 @@ import java.util.concurrent.TimeUnit; */ public class OffloadController { private static final String TAG = OffloadController.class.getSimpleName(); + private static final String ANYIP = "0.0.0.0"; + private static final ForwardedStats EMPTY_STATS = new ForwardedStats(); private final Handler mHandler; private final OffloadHardwareInterface mHwInterface; @@ -148,6 +150,14 @@ public class OffloadController { public void onStoppedUnsupported() { if (!started()) return; mLog.log("onStoppedUnsupported"); + // Poll for statistics and trigger a sweep of tethering + // stats by observers. This might not succeed, but it's + // worth trying anyway. We need to do this because from + // this point on we continue with software forwarding, + // and we need to synchronize stats and limits between + // software and hardware forwarding. + updateStatsForAllUpstreams(); + forceTetherStatsPoll(); } @Override @@ -155,11 +165,15 @@ public class OffloadController { if (!started()) return; mLog.log("onSupportAvailable"); - // [1] Poll for statistics and notify NetworkStats - // [2] (Re)Push all state: - // [a] push local prefixes - // [b] push downstreams - // [c] push upstream parameters + // [1] Poll for statistics and trigger a sweep of stats + // by observers. We need to do this to ensure that any + // limits set take into account any software tethering + // traffic that has been happening in the meantime. + updateStatsForAllUpstreams(); + forceTetherStatsPoll(); + // [2] (Re)Push all state. + // TODO: computeAndPushLocalPrefixes() + // TODO: push all downstream state. pushUpstreamParameters(null); } @@ -181,12 +195,7 @@ public class OffloadController { // The stats for the previous upstream were already updated on this thread // just after the upstream was changed, so they are also up-to-date. updateStatsForCurrentUpstream(); - - try { - mNms.tetherLimitReached(mStatsProvider); - } catch (RemoteException e) { - mLog.e("Cannot report data limit reached: " + e); - } + forceTetherStatsPoll(); } @Override @@ -305,13 +314,33 @@ public class OffloadController { maybeUpdateStats(currentUpstreamInterface()); } + private void updateStatsForAllUpstreams() { + // In practice, there should only ever be a single digit number of + // upstream interfaces over the lifetime of an active tethering session. + // Roughly speaking, imagine a very ambitious one or two of each of the + // following interface types: [ "rmnet_data", "wlan", "eth", "rndis" ]. + for (Map.Entry<String, ForwardedStats> kv : mForwardedStats.entrySet()) { + maybeUpdateStats(kv.getKey()); + } + } + + private void forceTetherStatsPoll() { + try { + mNms.tetherLimitReached(mStatsProvider); + } catch (RemoteException e) { + mLog.e("Cannot report data limit reached: " + e); + } + } + public void setUpstreamLinkProperties(LinkProperties lp) { if (!started() || Objects.equals(mUpstreamLinkProperties, lp)) return; - String prevUpstream = (mUpstreamLinkProperties != null) ? - mUpstreamLinkProperties.getInterfaceName() : null; + final String prevUpstream = currentUpstreamInterface(); mUpstreamLinkProperties = (lp != null) ? new LinkProperties(lp) : null; + // Make sure we record this interface in the ForwardedStats map. + final String iface = currentUpstreamInterface(); + if (!TextUtils.isEmpty(iface)) mForwardedStats.putIfAbsent(iface, EMPTY_STATS); // TODO: examine return code and decide what to do if programming // upstream parameters fails (probably just wait for a subsequent @@ -378,16 +407,20 @@ public class OffloadController { } private boolean pushUpstreamParameters(String prevUpstream) { - if (mUpstreamLinkProperties == null) { + final String iface = currentUpstreamInterface(); + + if (TextUtils.isEmpty(iface)) { + final boolean rval = mHwInterface.setUpstreamParameters("", ANYIP, ANYIP, null); + // Update stats after we've told the hardware to stop forwarding so + // we don't miss packets. maybeUpdateStats(prevUpstream); - return mHwInterface.setUpstreamParameters(null, null, null, null); + return rval; } // A stacked interface cannot be an upstream for hardware offload. // Consequently, we examine only the primary interface name, look at // getAddresses() rather than getAllAddresses(), and check getRoutes() // rather than getAllRoutes(). - final String iface = mUpstreamLinkProperties.getInterfaceName(); final ArrayList<String> v6gateways = new ArrayList<>(); String v4addr = null; String v4gateway = null; diff --git a/tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java b/tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java index dfe3f982e5bf..7a2ff8995458 100644 --- a/tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java +++ b/tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java @@ -32,12 +32,13 @@ import static org.mockito.Matchers.anyLong; import static org.mockito.Matchers.anyObject; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import android.content.Context; @@ -436,6 +437,9 @@ public class OffloadControllerTest { ethernetStats.txBytes = 100000; when(mHardware.getForwardedStats(eq(ethernetIface))).thenReturn(ethernetStats); offload.setUpstreamLinkProperties(null); + // Expect that we first clear the HAL's upstream parameters. + inOrder.verify(mHardware, times(1)).setUpstreamParameters( + eq(""), eq("0.0.0.0"), eq("0.0.0.0"), eq(null)); // Expect that we fetch stats from the previous upstream. inOrder.verify(mHardware, times(1)).getForwardedStats(eq(ethernetIface)); @@ -445,8 +449,6 @@ public class OffloadControllerTest { waitForIdle(); // There is no current upstream, so no stats are fetched. inOrder.verify(mHardware, never()).getForwardedStats(any()); - inOrder.verify(mHardware, times(1)).setUpstreamParameters( - eq(null), eq(null), eq(null), eq(null)); inOrder.verifyNoMoreInteractions(); assertEquals(2, stats.size()); @@ -621,6 +623,73 @@ public class OffloadControllerTest { inOrder.verifyNoMoreInteractions(); } + @Test + public void testControlCallbackOnStoppedUnsupportedFetchesAllStats() throws Exception { + setupFunctioningHardwareInterface(); + enableOffload(); + + final OffloadController offload = makeOffloadController(); + offload.start(); + + // Pretend to set a few different upstreams (only the interface name + // matters for this test; we're ignoring IP and route information). + final LinkProperties upstreamLp = new LinkProperties(); + for (String ifname : new String[]{RMNET0, WLAN0, RMNET0}) { + upstreamLp.setInterfaceName(ifname); + offload.setUpstreamLinkProperties(upstreamLp); + } + + // Clear invocation history, especially the getForwardedStats() calls + // that happen with setUpstreamParameters(). + clearInvocations(mHardware); + + OffloadHardwareInterface.ControlCallback callback = mControlCallbackCaptor.getValue(); + callback.onStoppedUnsupported(); + + // Verify forwarded stats behaviour. + verify(mHardware, times(1)).getForwardedStats(eq(RMNET0)); + verify(mHardware, times(1)).getForwardedStats(eq(WLAN0)); + verifyNoMoreInteractions(mHardware); + verify(mNMService, times(1)).tetherLimitReached(mTetherStatsProviderCaptor.getValue()); + verifyNoMoreInteractions(mNMService); + } + + @Test + public void testControlCallbackOnSupportAvailableFetchesAllStatsAndPushesAllParameters() + throws Exception { + setupFunctioningHardwareInterface(); + enableOffload(); + + final OffloadController offload = makeOffloadController(); + offload.start(); + + // Pretend to set a few different upstreams (only the interface name + // matters for this test; we're ignoring IP and route information). + final LinkProperties upstreamLp = new LinkProperties(); + for (String ifname : new String[]{RMNET0, WLAN0, RMNET0}) { + upstreamLp.setInterfaceName(ifname); + offload.setUpstreamLinkProperties(upstreamLp); + } + + // Clear invocation history, especially the getForwardedStats() calls + // that happen with setUpstreamParameters(). + clearInvocations(mHardware); + + OffloadHardwareInterface.ControlCallback callback = mControlCallbackCaptor.getValue(); + callback.onSupportAvailable(); + + // Verify forwarded stats behaviour. + verify(mHardware, times(1)).getForwardedStats(eq(RMNET0)); + verify(mHardware, times(1)).getForwardedStats(eq(WLAN0)); + verify(mNMService, times(1)).tetherLimitReached(mTetherStatsProviderCaptor.getValue()); + verifyNoMoreInteractions(mNMService); + + // TODO: verify local prefixes and downstreams are also pushed to the HAL. + verify(mHardware, times(1)).setUpstreamParameters(eq(RMNET0), any(), any(), any()); + verify(mHardware, times(1)).setDataLimit(eq(RMNET0), anyLong()); + verifyNoMoreInteractions(mHardware); + } + private static void assertArrayListContains(ArrayList<String> list, String... elems) { for (String element : elems) { assertTrue(list.contains(element)); |