[SP21] Address comments for API council review about aosp/1172143
Test: atest FrameworksNetTests ImsPhoneCallTrackerTest Test: atest TetheringTests NetworkStackTests Test: m doc-comment-check-docs Fix: 148552904 Change-Id: I141393f229e772d2eb9f7c156849e379bd71b845 Merged-In: I141393f229e772d2eb9f7c156849e379bd71b845 (cherry picked from aosp/1253717)
This commit is contained in:
@@ -39,8 +39,7 @@ import android.net.RouteInfo;
|
||||
import android.net.netlink.ConntrackMessage;
|
||||
import android.net.netlink.NetlinkConstants;
|
||||
import android.net.netlink.NetlinkSocket;
|
||||
import android.net.netstats.provider.AbstractNetworkStatsProvider;
|
||||
import android.net.netstats.provider.NetworkStatsProviderCallback;
|
||||
import android.net.netstats.provider.NetworkStatsProvider;
|
||||
import android.net.util.SharedLog;
|
||||
import android.os.Handler;
|
||||
import android.provider.Settings;
|
||||
@@ -89,8 +88,8 @@ public class OffloadController {
|
||||
private final Handler mHandler;
|
||||
private final OffloadHardwareInterface mHwInterface;
|
||||
private final ContentResolver mContentResolver;
|
||||
private final @NonNull OffloadTetheringStatsProvider mStatsProvider;
|
||||
private final @Nullable NetworkStatsProviderCallback mStatsProviderCb;
|
||||
@Nullable
|
||||
private final OffloadTetheringStatsProvider mStatsProvider;
|
||||
private final SharedLog mLog;
|
||||
private final HashMap<String, LinkProperties> mDownstreams;
|
||||
private boolean mConfigInitialized;
|
||||
@@ -124,19 +123,18 @@ public class OffloadController {
|
||||
mHandler = h;
|
||||
mHwInterface = hwi;
|
||||
mContentResolver = contentResolver;
|
||||
mStatsProvider = new OffloadTetheringStatsProvider();
|
||||
mLog = log.forSubComponent(TAG);
|
||||
mDownstreams = new HashMap<>();
|
||||
mExemptPrefixes = new HashSet<>();
|
||||
mLastLocalPrefixStrs = new HashSet<>();
|
||||
NetworkStatsProviderCallback providerCallback = null;
|
||||
OffloadTetheringStatsProvider provider = new OffloadTetheringStatsProvider();
|
||||
try {
|
||||
providerCallback = nsm.registerNetworkStatsProvider(
|
||||
getClass().getSimpleName(), mStatsProvider);
|
||||
nsm.registerNetworkStatsProvider(getClass().getSimpleName(), provider);
|
||||
} catch (RuntimeException e) {
|
||||
Log.wtf(TAG, "Cannot register offload stats provider: " + e);
|
||||
provider = null;
|
||||
}
|
||||
mStatsProviderCb = providerCallback;
|
||||
mStatsProvider = provider;
|
||||
}
|
||||
|
||||
/** Start hardware offload. */
|
||||
@@ -185,7 +183,7 @@ public class OffloadController {
|
||||
// and we need to synchronize stats and limits between
|
||||
// software and hardware forwarding.
|
||||
updateStatsForAllUpstreams();
|
||||
mStatsProvider.pushTetherStats();
|
||||
if (mStatsProvider != null) mStatsProvider.pushTetherStats();
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -198,7 +196,7 @@ public class OffloadController {
|
||||
// limits set take into account any software tethering
|
||||
// traffic that has been happening in the meantime.
|
||||
updateStatsForAllUpstreams();
|
||||
mStatsProvider.pushTetherStats();
|
||||
if (mStatsProvider != null) mStatsProvider.pushTetherStats();
|
||||
// [2] (Re)Push all state.
|
||||
computeAndPushLocalPrefixes(UpdateType.FORCE);
|
||||
pushAllDownstreamState();
|
||||
@@ -217,10 +215,12 @@ public class OffloadController {
|
||||
// TODO: rev the HAL so that it provides an interface name.
|
||||
|
||||
updateStatsForCurrentUpstream();
|
||||
mStatsProvider.pushTetherStats();
|
||||
// Push stats to service does not cause the service react to it immediately.
|
||||
// Inform the service about limit reached.
|
||||
if (mStatsProviderCb != null) mStatsProviderCb.onLimitReached();
|
||||
if (mStatsProvider != null) {
|
||||
mStatsProvider.pushTetherStats();
|
||||
// Push stats to service does not cause the service react to it
|
||||
// immediately. Inform the service about limit reached.
|
||||
mStatsProvider.notifyLimitReached();
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -263,13 +263,17 @@ public class OffloadController {
|
||||
}
|
||||
|
||||
@VisibleForTesting
|
||||
class OffloadTetheringStatsProvider extends AbstractNetworkStatsProvider {
|
||||
class OffloadTetheringStatsProvider extends NetworkStatsProvider {
|
||||
// These stats must only ever be touched on the handler thread.
|
||||
@NonNull
|
||||
private NetworkStats mIfaceStats = new NetworkStats(0L, 0);
|
||||
@NonNull
|
||||
private NetworkStats mUidStats = new NetworkStats(0L, 0);
|
||||
|
||||
/**
|
||||
* A helper function that collect tether stats from local hashmap. Note that this does not
|
||||
* invoke binder call.
|
||||
*/
|
||||
@VisibleForTesting
|
||||
@NonNull
|
||||
NetworkStats getTetherStats(@NonNull StatsType how) {
|
||||
@@ -287,7 +291,7 @@ public class OffloadController {
|
||||
}
|
||||
|
||||
@Override
|
||||
public void setLimit(String iface, long quotaBytes) {
|
||||
public void onSetLimit(String iface, long quotaBytes) {
|
||||
// Listen for all iface is necessary since upstream might be changed after limit
|
||||
// is set.
|
||||
mHandler.post(() -> {
|
||||
@@ -315,13 +319,12 @@ public class OffloadController {
|
||||
*/
|
||||
public void pushTetherStats() {
|
||||
// TODO: remove the accumulated stats and report the diff from HAL directly.
|
||||
if (null == mStatsProviderCb) return;
|
||||
final NetworkStats ifaceDiff =
|
||||
getTetherStats(StatsType.STATS_PER_IFACE).subtract(mIfaceStats);
|
||||
final NetworkStats uidDiff =
|
||||
getTetherStats(StatsType.STATS_PER_UID).subtract(mUidStats);
|
||||
try {
|
||||
mStatsProviderCb.onStatsUpdated(0 /* token */, ifaceDiff, uidDiff);
|
||||
notifyStatsUpdated(0 /* token */, ifaceDiff, uidDiff);
|
||||
mIfaceStats = mIfaceStats.add(ifaceDiff);
|
||||
mUidStats = mUidStats.add(uidDiff);
|
||||
} catch (RuntimeException e) {
|
||||
@@ -330,7 +333,7 @@ public class OffloadController {
|
||||
}
|
||||
|
||||
@Override
|
||||
public void requestStatsUpdate(int token) {
|
||||
public void onRequestStatsUpdate(int token) {
|
||||
// Do not attempt to update stats by querying the offload HAL
|
||||
// synchronously from a different thread than the Handler thread. http://b/64771555.
|
||||
mHandler.post(() -> {
|
||||
@@ -340,7 +343,7 @@ public class OffloadController {
|
||||
}
|
||||
|
||||
@Override
|
||||
public void setAlert(long quotaBytes) {
|
||||
public void onSetAlert(long quotaBytes) {
|
||||
// TODO: Ask offload HAL to notify alert without stopping traffic.
|
||||
}
|
||||
}
|
||||
|
||||
@@ -33,6 +33,8 @@ import static com.android.testutils.MiscAssertsKt.assertContainsAll;
|
||||
import static com.android.testutils.MiscAssertsKt.assertThrows;
|
||||
import static com.android.testutils.NetworkStatsUtilsKt.orderInsensitiveEquals;
|
||||
|
||||
import static junit.framework.Assert.assertNotNull;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.mockito.ArgumentMatchers.anyInt;
|
||||
@@ -61,8 +63,7 @@ import android.net.LinkProperties;
|
||||
import android.net.NetworkStats;
|
||||
import android.net.NetworkStats.Entry;
|
||||
import android.net.RouteInfo;
|
||||
import android.net.netstats.provider.AbstractNetworkStatsProvider;
|
||||
import android.net.netstats.provider.NetworkStatsProviderCallback;
|
||||
import android.net.netstats.provider.INetworkStatsProviderCallback;
|
||||
import android.net.util.SharedLog;
|
||||
import android.os.Handler;
|
||||
import android.os.Looper;
|
||||
@@ -108,12 +109,10 @@ public class OffloadControllerTest {
|
||||
@Mock private ApplicationInfo mApplicationInfo;
|
||||
@Mock private Context mContext;
|
||||
@Mock private NetworkStatsManager mStatsManager;
|
||||
@Mock private NetworkStatsProviderCallback mTetherStatsProviderCb;
|
||||
@Mock private INetworkStatsProviderCallback mTetherStatsProviderCb;
|
||||
private OffloadController.OffloadTetheringStatsProvider mTetherStatsProvider;
|
||||
private final ArgumentCaptor<ArrayList> mStringArrayCaptor =
|
||||
ArgumentCaptor.forClass(ArrayList.class);
|
||||
private final ArgumentCaptor<OffloadController.OffloadTetheringStatsProvider>
|
||||
mTetherStatsProviderCaptor =
|
||||
ArgumentCaptor.forClass(OffloadController.OffloadTetheringStatsProvider.class);
|
||||
private final ArgumentCaptor<OffloadHardwareInterface.ControlCallback> mControlCallbackCaptor =
|
||||
ArgumentCaptor.forClass(OffloadHardwareInterface.ControlCallback.class);
|
||||
private MockContentResolver mContentResolver;
|
||||
@@ -126,8 +125,6 @@ public class OffloadControllerTest {
|
||||
mContentResolver.addProvider(Settings.AUTHORITY, new FakeSettingsProvider());
|
||||
when(mContext.getContentResolver()).thenReturn(mContentResolver);
|
||||
FakeSettingsProvider.clearSettingsProvider();
|
||||
when(mStatsManager.registerNetworkStatsProvider(anyString(), any()))
|
||||
.thenReturn(mTetherStatsProviderCb);
|
||||
}
|
||||
|
||||
@After public void tearDown() throws Exception {
|
||||
@@ -154,8 +151,14 @@ public class OffloadControllerTest {
|
||||
private OffloadController makeOffloadController() throws Exception {
|
||||
OffloadController offload = new OffloadController(new Handler(Looper.getMainLooper()),
|
||||
mHardware, mContentResolver, mStatsManager, new SharedLog("test"));
|
||||
final ArgumentCaptor<OffloadController.OffloadTetheringStatsProvider>
|
||||
tetherStatsProviderCaptor =
|
||||
ArgumentCaptor.forClass(OffloadController.OffloadTetheringStatsProvider.class);
|
||||
verify(mStatsManager).registerNetworkStatsProvider(anyString(),
|
||||
mTetherStatsProviderCaptor.capture());
|
||||
tetherStatsProviderCaptor.capture());
|
||||
mTetherStatsProvider = tetherStatsProviderCaptor.getValue();
|
||||
assertNotNull(mTetherStatsProvider);
|
||||
mTetherStatsProvider.setProviderCallbackBinder(mTetherStatsProviderCb);
|
||||
return offload;
|
||||
}
|
||||
|
||||
@@ -413,9 +416,6 @@ public class OffloadControllerTest {
|
||||
final OffloadController offload = makeOffloadController();
|
||||
offload.start();
|
||||
|
||||
final OffloadController.OffloadTetheringStatsProvider provider =
|
||||
mTetherStatsProviderCaptor.getValue();
|
||||
|
||||
final String ethernetIface = "eth1";
|
||||
final String mobileIface = "rmnet_data0";
|
||||
|
||||
@@ -443,8 +443,8 @@ public class OffloadControllerTest {
|
||||
inOrder.verify(mHardware, times(1)).getForwardedStats(eq(mobileIface));
|
||||
|
||||
// Verify that the fetched stats are stored.
|
||||
final NetworkStats ifaceStats = provider.getTetherStats(STATS_PER_IFACE);
|
||||
final NetworkStats uidStats = provider.getTetherStats(STATS_PER_UID);
|
||||
final NetworkStats ifaceStats = mTetherStatsProvider.getTetherStats(STATS_PER_IFACE);
|
||||
final NetworkStats uidStats = mTetherStatsProvider.getTetherStats(STATS_PER_UID);
|
||||
final NetworkStats expectedIfaceStats = new NetworkStats(0L, 2)
|
||||
.addValues(buildTestEntry(STATS_PER_IFACE, mobileIface, 999, 99999))
|
||||
.addValues(buildTestEntry(STATS_PER_IFACE, ethernetIface, 12345, 54321));
|
||||
@@ -462,13 +462,12 @@ public class OffloadControllerTest {
|
||||
NetworkStats.class);
|
||||
|
||||
// Force pushing stats update to verify the stats reported.
|
||||
provider.pushTetherStats();
|
||||
verify(mTetherStatsProviderCb, times(1)).onStatsUpdated(anyInt(),
|
||||
ifaceStatsCaptor.capture(), uidStatsCaptor.capture());
|
||||
mTetherStatsProvider.pushTetherStats();
|
||||
verify(mTetherStatsProviderCb, times(1))
|
||||
.notifyStatsUpdated(anyInt(), ifaceStatsCaptor.capture(), uidStatsCaptor.capture());
|
||||
assertTrue(orderInsensitiveEquals(expectedIfaceStats, ifaceStatsCaptor.getValue()));
|
||||
assertTrue(orderInsensitiveEquals(expectedUidStats, uidStatsCaptor.getValue()));
|
||||
|
||||
|
||||
when(mHardware.getForwardedStats(eq(ethernetIface))).thenReturn(
|
||||
new ForwardedStats(100000, 100000));
|
||||
offload.setUpstreamLinkProperties(null);
|
||||
@@ -483,8 +482,8 @@ public class OffloadControllerTest {
|
||||
inOrder.verifyNoMoreInteractions();
|
||||
|
||||
// Verify that the stored stats is accumulated.
|
||||
final NetworkStats ifaceStatsAccu = provider.getTetherStats(STATS_PER_IFACE);
|
||||
final NetworkStats uidStatsAccu = provider.getTetherStats(STATS_PER_UID);
|
||||
final NetworkStats ifaceStatsAccu = mTetherStatsProvider.getTetherStats(STATS_PER_IFACE);
|
||||
final NetworkStats uidStatsAccu = mTetherStatsProvider.getTetherStats(STATS_PER_UID);
|
||||
final NetworkStats expectedIfaceStatsAccu = new NetworkStats(0L, 2)
|
||||
.addValues(buildTestEntry(STATS_PER_IFACE, mobileIface, 999, 99999))
|
||||
.addValues(buildTestEntry(STATS_PER_IFACE, ethernetIface, 112345, 154321));
|
||||
@@ -498,7 +497,7 @@ public class OffloadControllerTest {
|
||||
|
||||
// Verify that only diff of stats is reported.
|
||||
reset(mTetherStatsProviderCb);
|
||||
provider.pushTetherStats();
|
||||
mTetherStatsProvider.pushTetherStats();
|
||||
final NetworkStats expectedIfaceStatsDiff = new NetworkStats(0L, 2)
|
||||
.addValues(buildTestEntry(STATS_PER_IFACE, mobileIface, 0, 0))
|
||||
.addValues(buildTestEntry(STATS_PER_IFACE, ethernetIface, 100000, 100000));
|
||||
@@ -506,8 +505,8 @@ public class OffloadControllerTest {
|
||||
final NetworkStats expectedUidStatsDiff = new NetworkStats(0L, 2)
|
||||
.addValues(buildTestEntry(STATS_PER_UID, mobileIface, 0, 0))
|
||||
.addValues(buildTestEntry(STATS_PER_UID, ethernetIface, 100000, 100000));
|
||||
verify(mTetherStatsProviderCb, times(1)).onStatsUpdated(anyInt(),
|
||||
ifaceStatsCaptor.capture(), uidStatsCaptor.capture());
|
||||
verify(mTetherStatsProviderCb, times(1))
|
||||
.notifyStatsUpdated(anyInt(), ifaceStatsCaptor.capture(), uidStatsCaptor.capture());
|
||||
assertTrue(orderInsensitiveEquals(expectedIfaceStatsDiff, ifaceStatsCaptor.getValue()));
|
||||
assertTrue(orderInsensitiveEquals(expectedUidStatsDiff, uidStatsCaptor.getValue()));
|
||||
}
|
||||
@@ -529,19 +528,18 @@ public class OffloadControllerTest {
|
||||
lp.setInterfaceName(ethernetIface);
|
||||
offload.setUpstreamLinkProperties(lp);
|
||||
|
||||
AbstractNetworkStatsProvider provider = mTetherStatsProviderCaptor.getValue();
|
||||
final InOrder inOrder = inOrder(mHardware);
|
||||
when(mHardware.setUpstreamParameters(any(), any(), any(), any())).thenReturn(true);
|
||||
when(mHardware.setDataLimit(anyString(), anyLong())).thenReturn(true);
|
||||
|
||||
// Applying an interface quota to the current upstream immediately sends it to the hardware.
|
||||
provider.setLimit(ethernetIface, ethernetLimit);
|
||||
mTetherStatsProvider.onSetLimit(ethernetIface, ethernetLimit);
|
||||
waitForIdle();
|
||||
inOrder.verify(mHardware).setDataLimit(ethernetIface, ethernetLimit);
|
||||
inOrder.verifyNoMoreInteractions();
|
||||
|
||||
// Applying an interface quota to another upstream does not take any immediate action.
|
||||
provider.setLimit(mobileIface, mobileLimit);
|
||||
mTetherStatsProvider.onSetLimit(mobileIface, mobileLimit);
|
||||
waitForIdle();
|
||||
inOrder.verify(mHardware, never()).setDataLimit(anyString(), anyLong());
|
||||
|
||||
@@ -554,7 +552,7 @@ public class OffloadControllerTest {
|
||||
|
||||
// Setting a limit of ITetheringStatsProvider.QUOTA_UNLIMITED causes the limit to be set
|
||||
// to Long.MAX_VALUE.
|
||||
provider.setLimit(mobileIface, ITetheringStatsProvider.QUOTA_UNLIMITED);
|
||||
mTetherStatsProvider.onSetLimit(mobileIface, ITetheringStatsProvider.QUOTA_UNLIMITED);
|
||||
waitForIdle();
|
||||
inOrder.verify(mHardware).setDataLimit(mobileIface, Long.MAX_VALUE);
|
||||
|
||||
@@ -562,7 +560,7 @@ public class OffloadControllerTest {
|
||||
when(mHardware.setUpstreamParameters(any(), any(), any(), any())).thenReturn(false);
|
||||
lp.setInterfaceName(ethernetIface);
|
||||
offload.setUpstreamLinkProperties(lp);
|
||||
provider.setLimit(mobileIface, mobileLimit);
|
||||
mTetherStatsProvider.onSetLimit(mobileIface, mobileLimit);
|
||||
waitForIdle();
|
||||
inOrder.verify(mHardware, never()).setDataLimit(anyString(), anyLong());
|
||||
|
||||
@@ -571,7 +569,7 @@ public class OffloadControllerTest {
|
||||
when(mHardware.setDataLimit(anyString(), anyLong())).thenReturn(false);
|
||||
lp.setInterfaceName(mobileIface);
|
||||
offload.setUpstreamLinkProperties(lp);
|
||||
provider.setLimit(mobileIface, mobileLimit);
|
||||
mTetherStatsProvider.onSetLimit(mobileIface, mobileLimit);
|
||||
waitForIdle();
|
||||
inOrder.verify(mHardware).getForwardedStats(ethernetIface);
|
||||
inOrder.verify(mHardware).stopOffloadControl();
|
||||
@@ -587,7 +585,7 @@ public class OffloadControllerTest {
|
||||
|
||||
OffloadHardwareInterface.ControlCallback callback = mControlCallbackCaptor.getValue();
|
||||
callback.onStoppedLimitReached();
|
||||
verify(mTetherStatsProviderCb, times(1)).onStatsUpdated(anyInt(), any(), any());
|
||||
verify(mTetherStatsProviderCb, times(1)).notifyStatsUpdated(anyInt(), any(), any());
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -691,7 +689,7 @@ public class OffloadControllerTest {
|
||||
verify(mHardware, times(1)).getForwardedStats(eq(RMNET0));
|
||||
verify(mHardware, times(1)).getForwardedStats(eq(WLAN0));
|
||||
// TODO: verify the exact stats reported.
|
||||
verify(mTetherStatsProviderCb, times(1)).onStatsUpdated(anyInt(), any(), any());
|
||||
verify(mTetherStatsProviderCb, times(1)).notifyStatsUpdated(anyInt(), any(), any());
|
||||
verifyNoMoreInteractions(mTetherStatsProviderCb);
|
||||
verifyNoMoreInteractions(mHardware);
|
||||
}
|
||||
@@ -756,7 +754,7 @@ public class OffloadControllerTest {
|
||||
// Verify forwarded stats behaviour.
|
||||
verify(mHardware, times(1)).getForwardedStats(eq(RMNET0));
|
||||
verify(mHardware, times(1)).getForwardedStats(eq(WLAN0));
|
||||
verify(mTetherStatsProviderCb, times(1)).onStatsUpdated(anyInt(), any(), any());
|
||||
verify(mTetherStatsProviderCb, times(1)).notifyStatsUpdated(anyInt(), any(), any());
|
||||
verifyNoMoreInteractions(mTetherStatsProviderCb);
|
||||
|
||||
// TODO: verify local prefixes and downstreams are also pushed to the HAL.
|
||||
|
||||
Reference in New Issue
Block a user