From 9548c8580c0bedad6ae78f6fec96cab924dffcf2 Mon Sep 17 00:00:00 2001 From: markchien Date: Tue, 5 May 2020 17:42:44 +0800 Subject: [PATCH 1/4] Override tethering module APK-in-APEX for Go variant Bug: 155604224 Test: build Change-Id: I4147173b5f3668491ff9cb7f1f86715b036d6d4b --- Tethering/Android.bp | 1 + Tethering/apex/Android.bp | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/Tethering/Android.bp b/Tethering/Android.bp index bfb65241ec..83f761f812 100644 --- a/Tethering/Android.bp +++ b/Tethering/Android.bp @@ -109,6 +109,7 @@ android_app { manifest: "AndroidManifest_InProcess.xml", // InProcessTethering is a replacement for Tethering overrides: ["Tethering"], + apex_available: ["com.android.tethering"], } // Updatable tethering packaged as an application diff --git a/Tethering/apex/Android.bp b/Tethering/apex/Android.bp index 24df5f6960..20ccd2ad64 100644 --- a/Tethering/apex/Android.bp +++ b/Tethering/apex/Android.bp @@ -36,3 +36,12 @@ android_app_certificate { name: "com.android.tethering.certificate", certificate: "com.android.tethering", } + +override_apex { + name: "com.android.tethering.inprocess", + base: "com.android.tethering", + package_name: "com.android.tethering.inprocess", + apps: [ + "InProcessTethering", + ], +} From c96fbe0abe5baab541cc827eb7a69701ce964dce Mon Sep 17 00:00:00 2001 From: Mark Chien Date: Thu, 7 May 2020 03:49:42 +0000 Subject: [PATCH 2/4] Test tethering log dump Bug: 145490751 Test: atest TetheringTests Merged-In: I01fc6969041711f7a15880144ee5eac591086ecd Change-Id: I01fc6969041711f7a15880144ee5eac591086ecd --- .../networkstack/tethering/Tethering.java | 8 +--- .../tethering/TetheringDependencies.java | 14 +++++++ .../networkstack/tethering/TetheringTest.java | 40 +++++++++++++++++-- 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java index 952325cc43..b2a43c47d1 100644 --- a/Tethering/src/com/android/networkstack/tethering/Tethering.java +++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java @@ -62,7 +62,6 @@ import static android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID; import static com.android.networkstack.tethering.TetheringNotificationUpdater.DOWNSTREAM_NONE; -import android.app.usage.NetworkStatsManager; import android.bluetooth.BluetoothAdapter; import android.bluetooth.BluetoothPan; import android.bluetooth.BluetoothProfile; @@ -268,12 +267,9 @@ public class Tethering { mTetherMasterSM = new TetherMasterSM("TetherMaster", mLooper, deps); mTetherMasterSM.start(); - final NetworkStatsManager statsManager = - (NetworkStatsManager) mContext.getSystemService(Context.NETWORK_STATS_SERVICE); mHandler = mTetherMasterSM.getHandler(); - mOffloadController = new OffloadController(mHandler, - mDeps.getOffloadHardwareInterface(mHandler, mLog), mContext.getContentResolver(), - statsManager, mLog, new OffloadController.Dependencies() { + mOffloadController = mDeps.getOffloadController(mHandler, mLog, + new OffloadController.Dependencies() { @Override public TetheringConfiguration getTetherConfig() { diff --git a/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java b/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java index 9b54b5ff24..802f2acb33 100644 --- a/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java +++ b/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java @@ -16,6 +16,7 @@ package com.android.networkstack.tethering; +import android.app.usage.NetworkStatsManager; import android.bluetooth.BluetoothAdapter; import android.content.Context; import android.net.INetd; @@ -46,6 +47,19 @@ public abstract class TetheringDependencies { return new OffloadHardwareInterface(h, log); } + /** + * Get a reference to the offload controller to be used by tethering. + */ + @NonNull + public OffloadController getOffloadController(@NonNull Handler h, + @NonNull SharedLog log, @NonNull OffloadController.Dependencies deps) { + final NetworkStatsManager statsManager = + (NetworkStatsManager) getContext().getSystemService(Context.NETWORK_STATS_SERVICE); + return new OffloadController(h, getOffloadHardwareInterface(h, log), + getContext().getContentResolver(), statsManager, log, deps); + } + + /** * Get a reference to the UpstreamNetworkMonitor to be used by tethering. */ diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java index 0363f5f998..fff7a70f54 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java @@ -150,6 +150,8 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.io.FileDescriptor; +import java.io.PrintWriter; import java.net.Inet4Address; import java.net.Inet6Address; import java.util.ArrayList; @@ -212,6 +214,9 @@ public class TetheringTest { private Tethering mTethering; private PhoneStateListener mPhoneStateListener; private InterfaceConfigurationParcel mInterfaceConfiguration; + private TetheringConfiguration mConfig; + private EntitlementManager mEntitleMgr; + private OffloadController mOffloadCtrl; private class TestContext extends BroadcastInterceptingContext { TestContext(Context base) { @@ -297,8 +302,9 @@ public class TetheringTest { } } - private class MockTetheringConfiguration extends TetheringConfiguration { - MockTetheringConfiguration(Context ctx, SharedLog log, int id) { + // MyTetheringConfiguration is used to override static method for testing. + private class MyTetheringConfiguration extends TetheringConfiguration { + MyTetheringConfiguration(Context ctx, SharedLog log, int id) { super(ctx, log, id); } @@ -327,6 +333,15 @@ public class TetheringTest { return mOffloadHardwareInterface; } + @Override + public OffloadController getOffloadController(Handler h, SharedLog log, + OffloadController.Dependencies deps) { + mOffloadCtrl = spy(super.getOffloadController(h, log, deps)); + // Return real object here instead of mock because + // testReportFailCallbackIfOffloadNotSupported depend on real OffloadController object. + return mOffloadCtrl; + } + @Override public UpstreamNetworkMonitor getUpstreamNetworkMonitor(Context ctx, StateMachine target, SharedLog log, int what) { @@ -351,6 +366,13 @@ public class TetheringTest { return mNetworkRequest; } + @Override + public EntitlementManager getEntitlementManager(Context ctx, StateMachine target, + SharedLog log, int what) { + mEntitleMgr = spy(super.getEntitlementManager(ctx, target, log, what)); + return mEntitleMgr; + } + @Override public boolean isTetheringSupported() { return true; @@ -359,7 +381,8 @@ public class TetheringTest { @Override public TetheringConfiguration generateTetheringConfiguration(Context ctx, SharedLog log, int subId) { - return new MockTetheringConfiguration(ctx, log, subId); + mConfig = spy(new MyTetheringConfiguration(ctx, log, subId)); + return mConfig; } @Override @@ -1726,6 +1749,17 @@ public class TetheringTest { verify(mNotificationUpdater, never()).onUpstreamCapabilitiesChanged(any()); } + @Test + public void testDumpTetheringLog() throws Exception { + final FileDescriptor mockFd = mock(FileDescriptor.class); + final PrintWriter mockPw = mock(PrintWriter.class); + runUsbTethering(null); + mTethering.dump(mockFd, mockPw, new String[0]); + verify(mConfig).dump(any()); + verify(mEntitleMgr).dump(any()); + verify(mOffloadCtrl).dump(any()); + } + // TODO: Test that a request for hotspot mode doesn't interfere with an // already operating tethering mode interface. } From 1aba7987de269fed94073e7694f604c2e17750cd Mon Sep 17 00:00:00 2001 From: Jeongik Cha Date: Thu, 7 May 2020 11:26:19 +0000 Subject: [PATCH 3/4] Use stable networkstack-aidl-interfaces Test: m nothing Bug: 133526962 Original-Change: https://android-review.googlesource.com/1301313 Merged-In: I507f40866d04db5ed3361831e01eaa4dfaf20bed Change-Id: I507f40866d04db5ed3361831e01eaa4dfaf20bed --- Tethering/Android.bp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tethering/Android.bp b/Tethering/Android.bp index bfb65241ec..07a4f71117 100644 --- a/Tethering/Android.bp +++ b/Tethering/Android.bp @@ -27,7 +27,7 @@ java_defaults { "androidx.annotation_annotation", "netd_aidl_interface-V3-java", "netlink-client", - "networkstack-aidl-interfaces-unstable-java", + "networkstack-aidl-interfaces-java", "android.hardware.tetheroffload.config-V1.0-java", "android.hardware.tetheroffload.control-V1.0-java", "net-utils-framework-common", From 92afd387fde5d46b7d113be9084c61516c4446e1 Mon Sep 17 00:00:00 2001 From: Mark Chien Date: Thu, 7 May 2020 11:26:35 +0000 Subject: [PATCH 4/4] Refactor the EntitlementManager 1. Change ArraySet usage to BitSet 2. Change mCellularUpstreamPermitted to mLastCellularUpstreamPermitted. Before this change: a member variable(mCellularUpstreamPermitted) is used to check whether cellular upstream is permitted, the code must ensure to update this variable once entitlement result is changed or the entitlement check is triggered but does not have a result yet. In this change: Instead of storing the information about whether cellular is permitted in a member variable. The information is recalculated every time when user call isCellularUpstreamPermitted(). Now isCellularUpstreamPermitted() is always be used to check whether cellular upstream is permitted no matter inside or outside EntitlementManager. This make the code be easier to maintain that we do not need to care when mCellularUpstreamPermitted need to be updated because the information would be recalculated every time. And the recalculation is lock free because this is only used inside tethering while running in the same thread. Bug: 141256482 Test: atest TetheringTests Merged-In: Ic83f42ff4eec38adf039d55d80fcb9b0f16373cc Change-Id: Ic83f42ff4eec38adf039d55d80fcb9b0f16373cc --- .../tethering/EntitlementManager.java | 113 +++++++++--------- 1 file changed, 56 insertions(+), 57 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/EntitlementManager.java b/Tethering/src/com/android/networkstack/tethering/EntitlementManager.java index 049a9f68bb..d9785415c8 100644 --- a/Tethering/src/com/android/networkstack/tethering/EntitlementManager.java +++ b/Tethering/src/com/android/networkstack/tethering/EntitlementManager.java @@ -45,13 +45,13 @@ import android.os.SystemClock; import android.os.SystemProperties; import android.provider.Settings; import android.telephony.CarrierConfigManager; -import android.util.ArraySet; import android.util.SparseIntArray; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.StateMachine; import java.io.PrintWriter; +import java.util.BitSet; /** * Re-check tethering provisioning for enabled downstream tether types. @@ -74,11 +74,11 @@ public class EntitlementManager { private final ComponentName mSilentProvisioningService; private static final int MS_PER_HOUR = 60 * 60 * 1000; - // The ArraySet contains enabled downstream types, ex: + // The BitSet is the bit map of each enabled downstream types, ex: // {@link TetheringManager.TETHERING_WIFI} // {@link TetheringManager.TETHERING_USB} // {@link TetheringManager.TETHERING_BLUETOOTH} - private final ArraySet mCurrentTethers; + private final BitSet mCurrentDownstreams; private final Context mContext; private final int mPermissionChangeMessageCode; private final SharedLog mLog; @@ -87,9 +87,9 @@ public class EntitlementManager { private final StateMachine mTetherMasterSM; // Key: TetheringManager.TETHERING_*(downstream). // Value: TetheringManager.TETHER_ERROR_{NO_ERROR or PROVISION_FAILED}(provisioning result). - private final SparseIntArray mCellularPermitted; + private final SparseIntArray mCurrentEntitlementResults; private PendingIntent mProvisioningRecheckAlarm; - private boolean mCellularUpstreamPermitted = true; + private boolean mLastCellularUpstreamPermitted = true; private boolean mUsingCellularAsUpstream = false; private boolean mNeedReRunProvisioningUi = false; private OnUiEntitlementFailedListener mListener; @@ -97,11 +97,10 @@ public class EntitlementManager { public EntitlementManager(Context ctx, StateMachine tetherMasterSM, SharedLog log, int permissionChangeMessageCode) { - mContext = ctx; mLog = log.forSubComponent(TAG); - mCurrentTethers = new ArraySet(); - mCellularPermitted = new SparseIntArray(); + mCurrentDownstreams = new BitSet(); + mCurrentEntitlementResults = new SparseIntArray(); mEntitlementCacheValue = new SparseIntArray(); mTetherMasterSM = tetherMasterSM; mPermissionChangeMessageCode = permissionChangeMessageCode; @@ -144,13 +143,19 @@ public class EntitlementManager { * Check if cellular upstream is permitted. */ public boolean isCellularUpstreamPermitted() { - // If provisioning is required and EntitlementManager don't know any downstream, - // cellular upstream should not be allowed. final TetheringConfiguration config = mFetcher.fetchTetheringConfiguration(); - if (mCurrentTethers.size() == 0 && isTetherProvisioningRequired(config)) { - return false; - } - return mCellularUpstreamPermitted; + + return isCellularUpstreamPermitted(config); + } + + private boolean isCellularUpstreamPermitted(final TetheringConfiguration config) { + if (!isTetherProvisioningRequired(config)) return true; + + // If provisioning is required and EntitlementManager doesn't know any downstreams, + // cellular upstream should not be allowed. + if (mCurrentDownstreams.isEmpty()) return false; + + return mCurrentEntitlementResults.indexOfValue(TETHER_ERROR_NO_ERROR) > -1; } /** @@ -164,29 +169,22 @@ public class EntitlementManager { public void startProvisioningIfNeeded(int downstreamType, boolean showProvisioningUi) { if (!isValidDownstreamType(downstreamType)) return; - if (!mCurrentTethers.contains(downstreamType)) mCurrentTethers.add(downstreamType); + mCurrentDownstreams.set(downstreamType, true); final TetheringConfiguration config = mFetcher.fetchTetheringConfiguration(); - if (isTetherProvisioningRequired(config)) { - // If provisioning is required and the result is not available yet, - // cellular upstream should not be allowed. - if (mCellularPermitted.size() == 0) { - mCellularUpstreamPermitted = false; - } - // If upstream is not cellular, provisioning app would not be launched - // till upstream change to cellular. - if (mUsingCellularAsUpstream) { - if (showProvisioningUi) { - runUiTetherProvisioning(downstreamType, config.activeDataSubId); - } else { - runSilentTetherProvisioning(downstreamType, config.activeDataSubId); - } - mNeedReRunProvisioningUi = false; + if (!isTetherProvisioningRequired(config)) return; + + // If upstream is not cellular, provisioning app would not be launched + // till upstream change to cellular. + if (mUsingCellularAsUpstream) { + if (showProvisioningUi) { + runUiTetherProvisioning(downstreamType, config.activeDataSubId); } else { - mNeedReRunProvisioningUi |= showProvisioningUi; + runSilentTetherProvisioning(downstreamType, config.activeDataSubId); } + mNeedReRunProvisioningUi = false; } else { - mCellularUpstreamPermitted = true; + mNeedReRunProvisioningUi |= showProvisioningUi; } } @@ -195,14 +193,14 @@ public class EntitlementManager { * * @param type tethering type from TetheringManager.TETHERING_{@code *} */ - public void stopProvisioningIfNeeded(int type) { - if (!isValidDownstreamType(type)) return; + public void stopProvisioningIfNeeded(int downstreamType) { + if (!isValidDownstreamType(downstreamType)) return; - mCurrentTethers.remove(type); + mCurrentDownstreams.set(downstreamType, false); // There are lurking bugs where the notion of "provisioning required" or // "tethering supported" may change without without tethering being notified properly. // Remove the mapping all the time no matter provisioning is required or not. - removeDownstreamMapping(type); + removeDownstreamMapping(downstreamType); } /** @@ -213,7 +211,7 @@ public class EntitlementManager { public void notifyUpstream(boolean isCellular) { if (DBG) { mLog.i("notifyUpstream: " + isCellular - + ", mCellularUpstreamPermitted: " + mCellularUpstreamPermitted + + ", mLastCellularUpstreamPermitted: " + mLastCellularUpstreamPermitted + ", mNeedReRunProvisioningUi: " + mNeedReRunProvisioningUi); } mUsingCellularAsUpstream = isCellular; @@ -231,7 +229,7 @@ public class EntitlementManager { } private void maybeRunProvisioning(final TetheringConfiguration config) { - if (mCurrentTethers.size() == 0 || !isTetherProvisioningRequired(config)) { + if (mCurrentDownstreams.isEmpty() || !isTetherProvisioningRequired(config)) { return; } @@ -239,8 +237,9 @@ public class EntitlementManager { // are allowed. Therefore even if the silent check here ends in a failure and the UI later // yields success, then the downstream that got a failure will re-evaluate as a result of // the change and get the new correct value. - for (Integer downstream : mCurrentTethers) { - if (mCellularPermitted.indexOfKey(downstream) < 0) { + for (int downstream = mCurrentDownstreams.nextSetBit(0); downstream >= 0; + downstream = mCurrentDownstreams.nextSetBit(downstream + 1)) { + if (mCurrentEntitlementResults.indexOfKey(downstream) < 0) { if (mNeedReRunProvisioningUi) { mNeedReRunProvisioningUi = false; runUiTetherProvisioning(downstream, config.activeDataSubId); @@ -286,7 +285,7 @@ public class EntitlementManager { mLog.log("reevaluateSimCardProvisioning() don't run in TetherMaster thread"); } mEntitlementCacheValue.clear(); - mCellularPermitted.clear(); + mCurrentEntitlementResults.clear(); // TODO: refine provisioning check to isTetherProvisioningRequired() ?? if (!config.hasMobileHotspotProvisionApp() @@ -410,22 +409,21 @@ public class EntitlementManager { } private void evaluateCellularPermission(final TetheringConfiguration config) { - final boolean oldPermitted = mCellularUpstreamPermitted; - mCellularUpstreamPermitted = (!isTetherProvisioningRequired(config) - || mCellularPermitted.indexOfValue(TETHER_ERROR_NO_ERROR) > -1); + final boolean oldPermitted = mLastCellularUpstreamPermitted; + mLastCellularUpstreamPermitted = isCellularUpstreamPermitted(config); if (DBG) { mLog.i("Cellular permission change from " + oldPermitted - + " to " + mCellularUpstreamPermitted); + + " to " + mLastCellularUpstreamPermitted); } - if (mCellularUpstreamPermitted != oldPermitted) { - mLog.log("Cellular permission change: " + mCellularUpstreamPermitted); + if (mLastCellularUpstreamPermitted != oldPermitted) { + mLog.log("Cellular permission change: " + mLastCellularUpstreamPermitted); mTetherMasterSM.sendMessage(mPermissionChangeMessageCode); } // Only schedule periodic re-check when tether is provisioned // and the result is ok. - if (mCellularUpstreamPermitted && mCellularPermitted.size() > 0) { + if (mLastCellularUpstreamPermitted && mCurrentEntitlementResults.size() > 0) { scheduleProvisioningRechecks(config); } else { cancelTetherProvisioningRechecks(); @@ -441,10 +439,10 @@ public class EntitlementManager { */ protected void addDownstreamMapping(int type, int resultCode) { mLog.i("addDownstreamMapping: " + type + ", result: " + resultCode - + " ,TetherTypeRequested: " + mCurrentTethers.contains(type)); - if (!mCurrentTethers.contains(type)) return; + + " ,TetherTypeRequested: " + mCurrentDownstreams.get(type)); + if (!mCurrentDownstreams.get(type)) return; - mCellularPermitted.put(type, resultCode); + mCurrentEntitlementResults.put(type, resultCode); final TetheringConfiguration config = mFetcher.fetchTetheringConfiguration(); evaluateCellularPermission(config); } @@ -455,7 +453,7 @@ public class EntitlementManager { */ protected void removeDownstreamMapping(int type) { mLog.i("removeDownstreamMapping: " + type); - mCellularPermitted.delete(type); + mCurrentEntitlementResults.delete(type); final TetheringConfiguration config = mFetcher.fetchTetheringConfiguration(); evaluateCellularPermission(config); } @@ -488,14 +486,15 @@ public class EntitlementManager { * @param pw {@link PrintWriter} is used to print formatted */ public void dump(PrintWriter pw) { - pw.print("mCellularUpstreamPermitted: "); - pw.println(mCellularUpstreamPermitted); - for (Integer type : mCurrentTethers) { + pw.print("isCellularUpstreamPermitted: "); + pw.println(isCellularUpstreamPermitted()); + for (int type = mCurrentDownstreams.nextSetBit(0); type >= 0; + type = mCurrentDownstreams.nextSetBit(type + 1)) { pw.print("Type: "); pw.print(typeString(type)); - if (mCellularPermitted.indexOfKey(type) > -1) { + if (mCurrentEntitlementResults.indexOfKey(type) > -1) { pw.print(", Value: "); - pw.println(errorString(mCellularPermitted.get(type))); + pw.println(errorString(mCurrentEntitlementResults.get(type))); } else { pw.println(", Value: empty"); }