From 85877dfb6dcd4798b2dd5f952b7f2537c3e33a6d Mon Sep 17 00:00:00 2001 From: lucaslin Date: Thu, 21 Jan 2021 02:03:17 +0800 Subject: [PATCH 1/5] [IT4.5] Update radio power from CS directly There were two ways to update the network activity from NetworkManagementService to BatteryStatsService. 1. The Netd unsolicited event onInterfaceClassActivityChanged 2. The idle timer setup and removal The first path was replaced by previous patch to listen netd event from BSS directly. BSS does not rely on NMS to notify event from netd now. This patch is going to replace the second path. In order to clear the dependency between NMS and CS, the idle timer setup and removal will be sent from CS to INetd directly without going via NMS in the follow up patches. NMS will no longer receive the idle timer update. Thus, update the radio power status from CS to BSS directly to separate the network activity logic from NMS. Bug: 170598012 Test: atest FrameworksNetTests Change-Id: I716bd77168896b29a6e04f592adcf27b82edebca --- .../android/server/ConnectivityService.java | 47 +++++++++++++++---- .../server/NetworkManagementService.java | 37 --------------- 2 files changed, 37 insertions(+), 47 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 154e1831ceee1..460fa6d26ba70 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -147,6 +147,7 @@ import android.net.netlink.InetDiagMessage; import android.net.shared.PrivateDnsConfig; import android.net.util.MultinetworkPolicyTracker; import android.net.util.NetdService; +import android.os.BatteryStatsManager; import android.os.Binder; import android.os.Build; import android.os.Bundle; @@ -8634,6 +8635,7 @@ public class ConnectivityService extends IConnectivityManager.Stub * changes. */ private static final class LegacyNetworkActivityTracker { + private static final int NO_UID = -1; private final Context mContext; private final INetworkManagementService mNMS; @@ -8723,6 +8725,8 @@ public class ConnectivityService extends IConnectivityManager.Stub return; // do not track any other networks } + updateRadioPowerState(true /* isActive */, type); + if (timeout > 0 && iface != null) { try { // TODO: Access INetd directly instead of NMS @@ -8741,16 +8745,25 @@ public class ConnectivityService extends IConnectivityManager.Stub final String iface = networkAgent.linkProperties.getInterfaceName(); final NetworkCapabilities caps = networkAgent.networkCapabilities; - if (iface != null && (caps.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) - || caps.hasTransport(NetworkCapabilities.TRANSPORT_WIFI))) { - try { - // the call fails silently if no idle timer setup for this interface - // TODO: Access INetd directly instead of NMS - mNMS.removeIdleTimer(iface); - } catch (Exception e) { - // You shall not crash! - loge("Exception in removeDataActivityTracking " + e); - } + if (iface == null) return; + + final int type; + if (caps.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR)) { + type = NetworkCapabilities.TRANSPORT_CELLULAR; + } else if (caps.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)) { + type = NetworkCapabilities.TRANSPORT_WIFI; + } else { + return; // do not track any other networks + } + + try { + updateRadioPowerState(false /* isActive */, type); + // The call fails silently if no idle timer setup for this interface. + // TODO: Access INetd directly instead of NMS + mNMS.removeIdleTimer(iface); + } catch (Exception e) { + // You shall not crash! + loge("Exception in removeDataActivityTracking " + e); } } @@ -8766,6 +8779,20 @@ public class ConnectivityService extends IConnectivityManager.Stub removeDataActivityTracking(oldNetwork); } } + + private void updateRadioPowerState(boolean isActive, int transportType) { + final BatteryStatsManager bs = mContext.getSystemService(BatteryStatsManager.class); + switch (transportType) { + case NetworkCapabilities.TRANSPORT_CELLULAR: + bs.reportMobileRadioPowerState(isActive, NO_UID); + break; + case NetworkCapabilities.TRANSPORT_WIFI: + bs.reportWifiRadioPowerState(isActive, NO_UID); + break; + default: + logw("Untracked transport type:" + transportType); + } + } } /** diff --git a/services/core/java/com/android/server/NetworkManagementService.java b/services/core/java/com/android/server/NetworkManagementService.java index d30a6405e95dd..4765e94df1db2 100644 --- a/services/core/java/com/android/server/NetworkManagementService.java +++ b/services/core/java/com/android/server/NetworkManagementService.java @@ -80,7 +80,6 @@ import android.os.StrictMode; import android.os.SystemClock; import android.os.SystemProperties; import android.os.Trace; -import android.telephony.DataConnectionRealTimeInfo; import android.text.TextUtils; import android.util.Log; import android.util.Slog; @@ -247,10 +246,6 @@ public class NetworkManagementService extends INetworkManagementService.Stub { private volatile boolean mFirewallEnabled; private volatile boolean mStrictEnabled; - private boolean mMobileActivityFromRadio = false; - private int mLastPowerStateFromRadio = DataConnectionRealTimeInfo.DC_POWER_STATE_LOW; - private int mLastPowerStateFromWifi = DataConnectionRealTimeInfo.DC_POWER_STATE_LOW; - private final RemoteCallbackList mNetworkActivityListeners = new RemoteCallbackList<>(); private boolean mNetworkActive; @@ -397,36 +392,6 @@ public class NetworkManagementService extends INetworkManagementService.Stub { */ private void notifyInterfaceClassActivity(int type, boolean isActive, long tsNanos, int uid) { - final boolean isMobile = ConnectivityManager.isNetworkTypeMobile(type); - int powerState = isActive - ? DataConnectionRealTimeInfo.DC_POWER_STATE_HIGH - : DataConnectionRealTimeInfo.DC_POWER_STATE_LOW; - if (isMobile) { - if (mLastPowerStateFromRadio != powerState) { - mLastPowerStateFromRadio = powerState; - try { - // TODO: The interface changes that comes from netd are handled by BSS itself. - // There are still events caused by setting or removing idle timer, so keep - // reporting from here until setting idler timer moved to CS. - getBatteryStats().noteMobileRadioPowerState(powerState, tsNanos, uid); - } catch (RemoteException e) { - } - } - } - - if (ConnectivityManager.isNetworkTypeWifi(type)) { - if (mLastPowerStateFromWifi != powerState) { - mLastPowerStateFromWifi = powerState; - try { - // TODO: The interface changes that comes from netd are handled by BSS itself. - // There are still events caused by setting or removing idle timer, so keep - // reporting from here until setting idler timer moved to CS. - getBatteryStats().noteWifiRadioPowerState(powerState, tsNanos, uid); - } catch (RemoteException e) { - } - } - } - final boolean active = isActive; invokeForAllObservers(o -> o.interfaceClassDataActivityChanged( type, active, tsNanos, uid)); @@ -1847,8 +1812,6 @@ public class NetworkManagementService extends INetworkManagementService.Stub { protected void dump(FileDescriptor fd, PrintWriter pw, String[] args) { if (!DumpUtils.checkDumpPermission(mContext, TAG, pw)) return; - pw.print("mMobileActivityFromRadio="); pw.print(mMobileActivityFromRadio); - pw.print(" mLastPowerStateFromRadio="); pw.println(mLastPowerStateFromRadio); pw.print("mNetworkActive="); pw.println(mNetworkActive); synchronized (mQuotaLock) { From bc76bd407d6963532bea655defcc425be9034438 Mon Sep 17 00:00:00 2001 From: lucaslin Date: Thu, 21 Jan 2021 02:04:15 +0800 Subject: [PATCH 2/5] [IT4.6] Unbundle NMS out from ConnectivityManager ConnectivityService is no longer to update idle timer to NMS but send to INetd directly after this change. Replace the API implementation in ConnectivityManager to refer into ConnectivityService instead of NetworkManagementService to remove the dependency between CM and NMS for ConnectivityService mainline. Bug: 170598012 Test: atest FrameworksNetTests Change-Id: If0ac9a6427dba5a732a15b5d7ca1351b71b07b7b --- .../src/android/net/ConnectivityManager.java | 19 +-- .../android/server/ConnectivityService.java | 123 ++++++++++++++++-- .../server/NetworkManagementService.java | 5 - .../server/ConnectivityServiceTest.java | 34 +++-- 4 files changed, 134 insertions(+), 47 deletions(-) diff --git a/packages/Connectivity/framework/src/android/net/ConnectivityManager.java b/packages/Connectivity/framework/src/android/net/ConnectivityManager.java index c7bb2a75dba0a..adab6e3fbc8f3 100644 --- a/packages/Connectivity/framework/src/android/net/ConnectivityManager.java +++ b/packages/Connectivity/framework/src/android/net/ConnectivityManager.java @@ -50,7 +50,6 @@ import android.os.Bundle; import android.os.Handler; import android.os.IBinder; import android.os.INetworkActivityListener; -import android.os.INetworkManagementService; import android.os.Looper; import android.os.Message; import android.os.Messenger; @@ -835,7 +834,6 @@ public class ConnectivityManager { private final Context mContext; - private INetworkManagementService mNMService; private INetworkPolicyManager mNPManager; private final TetheringManager mTetheringManager; @@ -2211,17 +2209,6 @@ public class ConnectivityManager { void onNetworkActive(); } - private INetworkManagementService getNetworkManagementService() { - synchronized (this) { - if (mNMService != null) { - return mNMService; - } - IBinder b = ServiceManager.getService(Context.NETWORKMANAGEMENT_SERVICE); - mNMService = INetworkManagementService.Stub.asInterface(b); - return mNMService; - } - } - private final ArrayMap mNetworkActivityListeners = new ArrayMap<>(); @@ -2246,7 +2233,7 @@ public class ConnectivityManager { }; try { - getNetworkManagementService().registerNetworkActivityListener(rl); + mService.registerNetworkActivityListener(rl); mNetworkActivityListeners.put(l, rl); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); @@ -2263,7 +2250,7 @@ public class ConnectivityManager { INetworkActivityListener rl = mNetworkActivityListeners.get(l); Preconditions.checkArgument(rl != null, "Listener was not registered."); try { - getNetworkManagementService().unregisterNetworkActivityListener(rl); + mService.registerNetworkActivityListener(rl); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } @@ -2279,7 +2266,7 @@ public class ConnectivityManager { */ public boolean isDefaultNetworkActive() { try { - return getNetworkManagementService().isNetworkActive(); + return mService.isDefaultNetworkActive(); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 460fa6d26ba70..389b579541958 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -164,6 +164,7 @@ import android.os.Parcelable; import android.os.PersistableBundle; import android.os.PowerManager; import android.os.Process; +import android.os.RemoteCallbackList; import android.os.RemoteException; import android.os.ServiceSpecificException; import android.os.SystemClock; @@ -173,6 +174,7 @@ import android.os.UserManager; import android.provider.Settings; import android.telephony.TelephonyManager; import android.text.TextUtils; +import android.util.ArrayMap; import android.util.ArraySet; import android.util.LocalLog; import android.util.Log; @@ -559,6 +561,11 @@ public class ConnectivityService extends IConnectivityManager.Stub */ private static final int EVENT_SET_OEM_NETWORK_PREFERENCE = 48; + /** + * Used to indicate the system default network becomes active. + */ + private static final int EVENT_REPORT_NETWORK_ACTIVITY = 49; + /** * Argument for {@link #EVENT_PROVISIONING_NOTIFICATION} to indicate that the notification * should be shown. @@ -1193,7 +1200,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mUserAllContext.registerReceiver(mIntentReceiver, intentFilter, null /* broadcastPermission */, mHandler); - mNetworkActivityTracker = new LegacyNetworkActivityTracker(mContext, mNMS); + mNetworkActivityTracker = new LegacyNetworkActivityTracker(mContext, mHandler, mNMS, mNetd); mSettingsObserver = new SettingsObserver(mContext, mHandler); registerSettingsCallbacks(); @@ -2405,7 +2412,7 @@ public class ConnectivityService extends IConnectivityManager.Stub */ @Override public void registerNetworkActivityListener(@NonNull INetworkActivityListener l) { - // TODO: Replace network activity listener registry in ConnectivityManager from NMS to here + mNetworkActivityTracker.registerNetworkActivityListener(l); } /** @@ -2413,7 +2420,7 @@ public class ConnectivityService extends IConnectivityManager.Stub */ @Override public void unregisterNetworkActivityListener(@NonNull INetworkActivityListener l) { - // TODO: Replace network activity listener registry in ConnectivityManager from NMS to here + mNetworkActivityTracker.unregisterNetworkActivityListener(l); } /** @@ -2421,8 +2428,7 @@ public class ConnectivityService extends IConnectivityManager.Stub */ @Override public boolean isDefaultNetworkActive() { - // TODO: Replace isNetworkActive() in NMS. - return false; + return mNetworkActivityTracker.isDefaultNetworkActive(); } /** @@ -4448,6 +4454,9 @@ public class ConnectivityService extends IConnectivityManager.Stub loge("handleMessage.EVENT_SET_OEM_NETWORK_PREFERENCE failed", e); } break; + case EVENT_REPORT_NETWORK_ACTIVITY: + mNetworkActivityTracker.handleReportNetworkActivity(); + break; } } } @@ -8637,12 +8646,33 @@ public class ConnectivityService extends IConnectivityManager.Stub private static final class LegacyNetworkActivityTracker { private static final int NO_UID = -1; private final Context mContext; + private final INetd mNetd; private final INetworkManagementService mNMS; + private final RemoteCallbackList mNetworkActivityListeners = + new RemoteCallbackList<>(); + // Indicate the current system default network activity is active or not. + @GuardedBy("mActiveIdleTimers") + private boolean mNetworkActive; + @GuardedBy("mActiveIdleTimers") + private final ArrayMap mActiveIdleTimers = new ArrayMap(); + private final Handler mHandler; - LegacyNetworkActivityTracker(@NonNull Context context, - @NonNull INetworkManagementService nms) { + private class IdleTimerParams { + public final int timeout; + public final int transportType; + + IdleTimerParams(int timeout, int transport) { + this.timeout = timeout; + this.transportType = transport; + } + } + + LegacyNetworkActivityTracker(@NonNull Context context, @NonNull Handler handler, + @NonNull INetworkManagementService nms, @NonNull INetd netd) { mContext = context; mNMS = nms; + mNetd = netd; + mHandler = handler; try { mNMS.registerObserver(mDataActivityObserver); } catch (RemoteException e) { @@ -8658,9 +8688,50 @@ public class ConnectivityService extends IConnectivityManager.Stub long tsNanos, int uid) { sendDataActivityBroadcast(transportTypeToLegacyType(transportType), active, tsNanos); + synchronized (mActiveIdleTimers) { + mNetworkActive = active; + // If there are no idle timers, it means that system is not monitoring + // activity, so the system default network for those default network + // unspecified apps is always considered active. + // + // TODO: If the mActiveIdleTimers is empty, netd will actually not send + // any network activity change event. Whenever this event is received, + // the mActiveIdleTimers should be always not empty. The legacy behavior + // is no-op. Remove to refer to mNetworkActive only. + if (mNetworkActive || mActiveIdleTimers.isEmpty()) { + mHandler.sendMessage( + mHandler.obtainMessage(EVENT_REPORT_NETWORK_ACTIVITY)); + } + } } }; + // The network activity should only be updated from ConnectivityService handler thread + // when mActiveIdleTimers lock is held. + @GuardedBy("mActiveIdleTimers") + private void reportNetworkActive() { + final int length = mNetworkActivityListeners.beginBroadcast(); + if (DDBG) log("reportNetworkActive, notify " + length + " listeners"); + try { + for (int i = 0; i < length; i++) { + try { + mNetworkActivityListeners.getBroadcastItem(i).onNetworkActive(); + } catch (RemoteException | RuntimeException e) { + loge("Fail to send network activie to listener " + e); + } + } + } finally { + mNetworkActivityListeners.finishBroadcast(); + } + } + + @GuardedBy("mActiveIdleTimers") + public void handleReportNetworkActivity() { + synchronized (mActiveIdleTimers) { + reportNetworkActive(); + } + } + // This is deprecated and only to support legacy use cases. private int transportTypeToLegacyType(int type) { switch (type) { @@ -8729,8 +8800,13 @@ public class ConnectivityService extends IConnectivityManager.Stub if (timeout > 0 && iface != null) { try { - // TODO: Access INetd directly instead of NMS - mNMS.addIdleTimer(iface, timeout, type); + synchronized (mActiveIdleTimers) { + // Networks start up. + mNetworkActive = true; + mActiveIdleTimers.put(iface, new IdleTimerParams(timeout, type)); + mNetd.idletimerAddInterface(iface, timeout, Integer.toString(type)); + reportNetworkActive(); + } } catch (Exception e) { // You shall not crash! loge("Exception in setupDataActivityTracking " + e); @@ -8758,9 +8834,12 @@ public class ConnectivityService extends IConnectivityManager.Stub try { updateRadioPowerState(false /* isActive */, type); - // The call fails silently if no idle timer setup for this interface. - // TODO: Access INetd directly instead of NMS - mNMS.removeIdleTimer(iface); + synchronized (mActiveIdleTimers) { + final IdleTimerParams params = mActiveIdleTimers.remove(iface); + // The call fails silently if no idle timer setup for this interface + mNetd.idletimerRemoveInterface(iface, params.timeout, + Integer.toString(params.transportType)); + } } catch (Exception e) { // You shall not crash! loge("Exception in removeDataActivityTracking " + e); @@ -8793,6 +8872,26 @@ public class ConnectivityService extends IConnectivityManager.Stub logw("Untracked transport type:" + transportType); } } + + public boolean isDefaultNetworkActive() { + synchronized (mActiveIdleTimers) { + // If there are no idle timers, it means that system is not monitoring activity, + // so the default network is always considered active. + // + // TODO : Distinguish between the cases where mActiveIdleTimers is empty because + // tracking is disabled (negative idle timer value configured), or no active default + // network. In the latter case, this reports active but it should report inactive. + return mNetworkActive || mActiveIdleTimers.isEmpty(); + } + } + + public void registerNetworkActivityListener(@NonNull INetworkActivityListener l) { + mNetworkActivityListeners.register(l); + } + + public void unregisterNetworkActivityListener(@NonNull INetworkActivityListener l) { + mNetworkActivityListeners.unregister(l); + } } /** diff --git a/services/core/java/com/android/server/NetworkManagementService.java b/services/core/java/com/android/server/NetworkManagementService.java index 4765e94df1db2..f3d1885134c11 100644 --- a/services/core/java/com/android/server/NetworkManagementService.java +++ b/services/core/java/com/android/server/NetworkManagementService.java @@ -398,11 +398,6 @@ public class NetworkManagementService extends INetworkManagementService.Stub { boolean report = false; synchronized (mIdleTimerLock) { - if (mActiveIdleTimers.isEmpty()) { - // If there are no idle timers, we are not monitoring activity, so we - // are always considered active. - isActive = true; - } if (mNetworkActive != isActive) { mNetworkActive = isActive; report = isActive; diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 182897e37e0ae..e9c3577623aeb 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -8205,8 +8205,8 @@ public class ConnectivityServiceTest { reset(mNetworkManagementService); mCellNetworkAgent.connect(true); networkCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); - verify(mNetworkManagementService, times(1)).addIdleTimer(eq(MOBILE_IFNAME), anyInt(), - eq(NetworkCapabilities.TRANSPORT_CELLULAR)); + verify(mMockNetd, times(1)).idletimerAddInterface(eq(MOBILE_IFNAME), anyInt(), + eq(Integer.toString(TRANSPORT_CELLULAR))); mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); final LinkProperties wifiLp = new LinkProperties(); @@ -8214,25 +8214,27 @@ public class ConnectivityServiceTest { mWiFiNetworkAgent.sendLinkProperties(wifiLp); // Network switch - reset(mNetworkManagementService); mWiFiNetworkAgent.connect(true); networkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); networkCallback.expectCallback(CallbackEntry.LOSING, mCellNetworkAgent); networkCallback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent); - verify(mNetworkManagementService, times(1)).addIdleTimer(eq(WIFI_IFNAME), anyInt(), - eq(NetworkCapabilities.TRANSPORT_WIFI)); - verify(mNetworkManagementService, times(1)).removeIdleTimer(eq(MOBILE_IFNAME)); + verify(mMockNetd, times(1)).idletimerAddInterface(eq(WIFI_IFNAME), anyInt(), + eq(Integer.toString(TRANSPORT_WIFI))); + verify(mMockNetd, times(1)).idletimerRemoveInterface(eq(MOBILE_IFNAME), anyInt(), + eq(Integer.toString(TRANSPORT_CELLULAR))); // Disconnect wifi and switch back to cell - reset(mNetworkManagementService); + reset(mMockNetd); mWiFiNetworkAgent.disconnect(); networkCallback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent); assertNoCallbacks(networkCallback); - verify(mNetworkManagementService, times(1)).removeIdleTimer(eq(WIFI_IFNAME)); - verify(mNetworkManagementService, times(1)).addIdleTimer(eq(MOBILE_IFNAME), anyInt(), - eq(NetworkCapabilities.TRANSPORT_CELLULAR)); + verify(mMockNetd, times(1)).idletimerRemoveInterface(eq(WIFI_IFNAME), anyInt(), + eq(Integer.toString(TRANSPORT_WIFI))); + verify(mMockNetd, times(1)).idletimerAddInterface(eq(MOBILE_IFNAME), anyInt(), + eq(Integer.toString(TRANSPORT_CELLULAR))); // reconnect wifi + reset(mMockNetd); mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); wifiLp.setInterfaceName(WIFI_IFNAME); mWiFiNetworkAgent.sendLinkProperties(wifiLp); @@ -8240,9 +8242,12 @@ public class ConnectivityServiceTest { networkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); networkCallback.expectCallback(CallbackEntry.LOSING, mCellNetworkAgent); networkCallback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent); + verify(mMockNetd, times(1)).idletimerAddInterface(eq(WIFI_IFNAME), anyInt(), + eq(Integer.toString(TRANSPORT_WIFI))); + verify(mMockNetd, times(1)).idletimerRemoveInterface(eq(MOBILE_IFNAME), anyInt(), + eq(Integer.toString(TRANSPORT_CELLULAR))); // Disconnect cell - reset(mNetworkManagementService); reset(mMockNetd); mCellNetworkAgent.disconnect(); networkCallback.expectCallback(CallbackEntry.LOST, mCellNetworkAgent); @@ -8250,17 +8255,18 @@ public class ConnectivityServiceTest { // sent as network being switched. Ensure rule removal for cell will not be triggered // unexpectedly before network being removed. waitForIdle(); - verify(mNetworkManagementService, times(0)).removeIdleTimer(eq(MOBILE_IFNAME)); + verify(mMockNetd, times(0)).idletimerRemoveInterface(eq(MOBILE_IFNAME), anyInt(), + eq(Integer.toString(TRANSPORT_CELLULAR))); verify(mMockNetd, times(1)).networkDestroy(eq(mCellNetworkAgent.getNetwork().netId)); verify(mMockDnsResolver, times(1)) .destroyNetworkCache(eq(mCellNetworkAgent.getNetwork().netId)); // Disconnect wifi ExpectedBroadcast b = expectConnectivityAction(TYPE_WIFI, DetailedState.DISCONNECTED); - reset(mNetworkManagementService); mWiFiNetworkAgent.disconnect(); b.expectBroadcast(); - verify(mNetworkManagementService, times(1)).removeIdleTimer(eq(WIFI_IFNAME)); + verify(mMockNetd, times(1)).idletimerRemoveInterface(eq(WIFI_IFNAME), anyInt(), + eq(Integer.toString(TRANSPORT_WIFI))); // Clean up mCm.unregisterNetworkCallback(networkCallback); From 7dea3c5c3d99bb98ed798632487d4affddeab431 Mon Sep 17 00:00:00 2001 From: lucaslin Date: Thu, 21 Jan 2021 02:04:35 +0800 Subject: [PATCH 3/5] [IT4.7] Add network activity info into dumpsys for debugging Add network activity info into part of ConnectivityService dump for debugging. Sample dump: Legacy network activity: mNetworkActive=true Idle timers: wlan0: timeout=15 type=1 Bug: 170598012 Test: adb shell dumpsys connectivity Change-Id: Ie03e82767564543d8d143873d7ddc0ea07102cb2 --- .../android/server/ConnectivityService.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 389b579541958..60a467db36752 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2693,6 +2693,12 @@ public class ConnectivityService extends IConnectivityManager.Stub pw.increaseIndent(); mPermissionMonitor.dump(pw); pw.decreaseIndent(); + + pw.println(); + pw.println("Legacy network activity:"); + pw.increaseIndent(); + mNetworkActivityTracker.dump(pw); + pw.decreaseIndent(); } private void dumpNetworks(IndentingPrintWriter pw) { @@ -8892,6 +8898,19 @@ public class ConnectivityService extends IConnectivityManager.Stub public void unregisterNetworkActivityListener(@NonNull INetworkActivityListener l) { mNetworkActivityListeners.unregister(l); } + + public void dump(IndentingPrintWriter pw) { + synchronized (mActiveIdleTimers) { + pw.print("mNetworkActive="); pw.println(mNetworkActive); + pw.println("Idle timers:"); + for (HashMap.Entry ent : mActiveIdleTimers.entrySet()) { + pw.print(" "); pw.print(ent.getKey()); pw.println(":"); + final IdleTimerParams params = ent.getValue(); + pw.print(" timeout="); pw.print(params.timeout); + pw.print(" type="); pw.println(params.transportType); + } + } + } } /** From be5d09359c18b6063069ef3d114fbca65c2bc347 Mon Sep 17 00:00:00 2001 From: lucaslin Date: Thu, 21 Jan 2021 02:05:08 +0800 Subject: [PATCH 4/5] [IT05] Remove the unused network activity logic out from NMS The network activity logic is moved to ConnectivityService. CS is no longer setting the idle timer via NMS to netd. The idle timer should only set from CS, so remove the network activity logic out from NMS. Bug: 170598012 Test: atest FrameworksNetTests Change-Id: Ief9e892586aaee8e9542b9313088fa50d1d411e5 --- .../android/os/INetworkManagementService.aidl | 35 ----- .../server/NetworkManagementService.java | 132 +----------------- 2 files changed, 1 insertion(+), 166 deletions(-) diff --git a/core/java/android/os/INetworkManagementService.aidl b/core/java/android/os/INetworkManagementService.aidl index 33deddae01b69..874add5cdbd8d 100644 --- a/core/java/android/os/INetworkManagementService.aidl +++ b/core/java/android/os/INetworkManagementService.aidl @@ -24,7 +24,6 @@ import android.net.Network; import android.net.NetworkStats; import android.net.RouteInfo; import android.net.UidRange; -import android.os.INetworkActivityListener; /** * @hide @@ -294,25 +293,6 @@ interface INetworkManagementService @UnsupportedAppUsage(maxTargetSdk = 30, trackingBug = 170729553) boolean isBandwidthControlEnabled(); - /** - * Sets idletimer for an interface. - * - * This either initializes a new idletimer or increases its - * reference-counting if an idletimer already exists for given - * {@code iface}. - * - * {@code type} is the type of the interface, such as TYPE_MOBILE. - * - * Every {@code addIdleTimer} should be paired with a - * {@link removeIdleTimer} to cleanup when the network disconnects. - */ - void addIdleTimer(String iface, int timeout, int type); - - /** - * Removes idletimer for an interface. - */ - void removeIdleTimer(String iface); - void setFirewallEnabled(boolean enabled); boolean isFirewallEnabled(); void setFirewallInterfaceRule(String iface, boolean allow); @@ -320,21 +300,6 @@ interface INetworkManagementService void setFirewallUidRules(int chain, in int[] uids, in int[] rules); void setFirewallChainEnabled(int chain, boolean enable); - /** - * Start listening for mobile activity state changes. - */ - void registerNetworkActivityListener(INetworkActivityListener listener); - - /** - * Stop listening for mobile activity state changes. - */ - void unregisterNetworkActivityListener(INetworkActivityListener listener); - - /** - * Check whether the mobile radio is currently active. - */ - boolean isNetworkActive(); - void addLegacyRouteForNetId(int netId, in RouteInfo routeInfo, int uid); /** diff --git a/services/core/java/com/android/server/NetworkManagementService.java b/services/core/java/com/android/server/NetworkManagementService.java index f3d1885134c11..44054088d9374 100644 --- a/services/core/java/com/android/server/NetworkManagementService.java +++ b/services/core/java/com/android/server/NetworkManagementService.java @@ -69,7 +69,6 @@ import android.os.BatteryStats; import android.os.Binder; import android.os.Handler; import android.os.IBinder; -import android.os.INetworkActivityListener; import android.os.INetworkManagementService; import android.os.Process; import android.os.RemoteCallbackList; @@ -228,28 +227,9 @@ public class NetworkManagementService extends INetworkManagementService.Stub { @GuardedBy("mQuotaLock") private volatile boolean mDataSaverMode; - private final Object mIdleTimerLock = new Object(); - /** Set of interfaces with active idle timers. */ - private static class IdleTimerParams { - public final int timeout; - public final int type; - public int networkCount; - - IdleTimerParams(int timeout, int type) { - this.timeout = timeout; - this.type = type; - this.networkCount = 1; - } - } - private HashMap mActiveIdleTimers = Maps.newHashMap(); - private volatile boolean mFirewallEnabled; private volatile boolean mStrictEnabled; - private final RemoteCallbackList mNetworkActivityListeners = - new RemoteCallbackList<>(); - private boolean mNetworkActive; - /** * Constructs a new NetworkManagementService instance * @@ -392,20 +372,8 @@ public class NetworkManagementService extends INetworkManagementService.Stub { */ private void notifyInterfaceClassActivity(int type, boolean isActive, long tsNanos, int uid) { - final boolean active = isActive; invokeForAllObservers(o -> o.interfaceClassDataActivityChanged( - type, active, tsNanos, uid)); - - boolean report = false; - synchronized (mIdleTimerLock) { - if (mNetworkActive != isActive) { - mNetworkActive = isActive; - report = isActive; - } - } - if (report) { - reportNetworkActive(); - } + type, isActive, tsNanos, uid)); } @Override @@ -1081,60 +1049,6 @@ public class NetworkManagementService extends INetworkManagementService.Stub { } } - @Override - public void addIdleTimer(String iface, int timeout, final int type) { - NetworkStack.checkNetworkStackPermission(mContext); - - if (DBG) Slog.d(TAG, "Adding idletimer"); - - synchronized (mIdleTimerLock) { - IdleTimerParams params = mActiveIdleTimers.get(iface); - if (params != null) { - // the interface already has idletimer, update network count - params.networkCount++; - return; - } - - try { - mNetdService.idletimerAddInterface(iface, timeout, Integer.toString(type)); - } catch (RemoteException | ServiceSpecificException e) { - throw new IllegalStateException(e); - } - mActiveIdleTimers.put(iface, new IdleTimerParams(timeout, type)); - - // Networks start up. - if (ConnectivityManager.isNetworkTypeMobile(type)) { - mNetworkActive = false; - } - mDaemonHandler.post(() -> notifyInterfaceClassActivity(type, true, - SystemClock.elapsedRealtimeNanos(), -1)); - } - } - - @Override - public void removeIdleTimer(String iface) { - NetworkStack.checkNetworkStackPermission(mContext); - - if (DBG) Slog.d(TAG, "Removing idletimer"); - - synchronized (mIdleTimerLock) { - final IdleTimerParams params = mActiveIdleTimers.get(iface); - if (params == null || --(params.networkCount) > 0) { - return; - } - - try { - mNetdService.idletimerRemoveInterface(iface, - params.timeout, Integer.toString(params.type)); - } catch (RemoteException | ServiceSpecificException e) { - throw new IllegalStateException(e); - } - mActiveIdleTimers.remove(iface); - mDaemonHandler.post(() -> notifyInterfaceClassActivity(params.type, false, - SystemClock.elapsedRealtimeNanos(), -1)); - } - } - @Override public void setInterfaceQuota(String iface, long quotaBytes) { NetworkStack.checkNetworkStackPermission(mContext); @@ -1772,43 +1686,10 @@ public class NetworkManagementService extends INetworkManagementService.Stub { } } - @Override - public void registerNetworkActivityListener(INetworkActivityListener listener) { - mNetworkActivityListeners.register(listener); - } - - @Override - public void unregisterNetworkActivityListener(INetworkActivityListener listener) { - mNetworkActivityListeners.unregister(listener); - } - - @Override - public boolean isNetworkActive() { - synchronized (mNetworkActivityListeners) { - return mNetworkActive || mActiveIdleTimers.isEmpty(); - } - } - - private void reportNetworkActive() { - final int length = mNetworkActivityListeners.beginBroadcast(); - try { - for (int i = 0; i < length; i++) { - try { - mNetworkActivityListeners.getBroadcastItem(i).onNetworkActive(); - } catch (RemoteException | RuntimeException e) { - } - } - } finally { - mNetworkActivityListeners.finishBroadcast(); - } - } - @Override protected void dump(FileDescriptor fd, PrintWriter pw, String[] args) { if (!DumpUtils.checkDumpPermission(mContext, TAG, pw)) return; - pw.print("mNetworkActive="); pw.println(mNetworkActive); - synchronized (mQuotaLock) { pw.print("Active quota ifaces: "); pw.println(mActiveQuotas.toString()); pw.print("Active alert ifaces: "); pw.println(mActiveAlerts.toString()); @@ -1840,17 +1721,6 @@ public class NetworkManagementService extends INetworkManagementService.Stub { mUidFirewallRestrictedRules); } - synchronized (mIdleTimerLock) { - pw.println("Idle timers:"); - for (HashMap.Entry ent : mActiveIdleTimers.entrySet()) { - pw.print(" "); pw.print(ent.getKey()); pw.println(":"); - IdleTimerParams params = ent.getValue(); - pw.print(" timeout="); pw.print(params.timeout); - pw.print(" type="); pw.print(params.type); - pw.print(" networkCount="); pw.println(params.networkCount); - } - } - pw.print("Firewall enabled: "); pw.println(mFirewallEnabled); pw.print("Netd service status: " ); if (mNetdService == null) { From 6080999ebee8aac351b7b3e324069880c43642fd Mon Sep 17 00:00:00 2001 From: Chiachang Wang Date: Thu, 4 Feb 2021 17:29:59 +0800 Subject: [PATCH 5/5] [IT06]Move INetworkActivityListener into connectivity module INetworkActivityListener is hidden and the only usage is inside the connectivity module. Thus, move this into module scope. Bug: 170598012 Test: atest FrameworksNetTests Change-Id: I0a75c440c1daa773217bbd362b212fda4d07ec64 --- .../framework/src/android/net/ConnectivityManager.java | 1 - .../framework/src/android/net/IConnectivityManager.aidl | 2 +- .../framework/src/android/net}/INetworkActivityListener.aidl | 2 +- services/core/java/com/android/server/ConnectivityService.java | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-) rename {core/java/android/os => packages/Connectivity/framework/src/android/net}/INetworkActivityListener.aidl (97%) diff --git a/packages/Connectivity/framework/src/android/net/ConnectivityManager.java b/packages/Connectivity/framework/src/android/net/ConnectivityManager.java index adab6e3fbc8f3..f41a8cb14da86 100644 --- a/packages/Connectivity/framework/src/android/net/ConnectivityManager.java +++ b/packages/Connectivity/framework/src/android/net/ConnectivityManager.java @@ -49,7 +49,6 @@ import android.os.Build.VERSION_CODES; import android.os.Bundle; import android.os.Handler; import android.os.IBinder; -import android.os.INetworkActivityListener; import android.os.Looper; import android.os.Message; import android.os.Messenger; diff --git a/packages/Connectivity/framework/src/android/net/IConnectivityManager.aidl b/packages/Connectivity/framework/src/android/net/IConnectivityManager.aidl index 6391802f33307..160338d396afd 100644 --- a/packages/Connectivity/framework/src/android/net/IConnectivityManager.aidl +++ b/packages/Connectivity/framework/src/android/net/IConnectivityManager.aidl @@ -21,6 +21,7 @@ import android.net.ConnectionInfo; import android.net.ConnectivityDiagnosticsManager; import android.net.IConnectivityDiagnosticsCallback; import android.net.IOnSetOemNetworkPreferenceListener; +import android.net.INetworkActivityListener; import android.net.IQosCallback; import android.net.ISocketKeepaliveCallback; import android.net.LinkProperties; @@ -36,7 +37,6 @@ import android.net.UidRange; import android.net.QosSocketInfo; import android.os.Bundle; import android.os.IBinder; -import android.os.INetworkActivityListener; import android.os.Messenger; import android.os.ParcelFileDescriptor; import android.os.PersistableBundle; diff --git a/core/java/android/os/INetworkActivityListener.aidl b/packages/Connectivity/framework/src/android/net/INetworkActivityListener.aidl similarity index 97% rename from core/java/android/os/INetworkActivityListener.aidl rename to packages/Connectivity/framework/src/android/net/INetworkActivityListener.aidl index 24e6e55f1e578..79687dd3bf06e 100644 --- a/core/java/android/os/INetworkActivityListener.aidl +++ b/packages/Connectivity/framework/src/android/net/INetworkActivityListener.aidl @@ -13,7 +13,7 @@ ** limitations under the License. */ -package android.os; +package android.net; /** * @hide diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 60a467db36752..bd83804b4a797 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -90,6 +90,7 @@ import android.net.IConnectivityDiagnosticsCallback; import android.net.IConnectivityManager; import android.net.IDnsResolver; import android.net.INetd; +import android.net.INetworkActivityListener; import android.net.INetworkManagementEventObserver; import android.net.INetworkMonitor; import android.net.INetworkMonitorCallbacks; @@ -154,7 +155,6 @@ import android.os.Bundle; import android.os.Handler; import android.os.HandlerThread; import android.os.IBinder; -import android.os.INetworkActivityListener; import android.os.INetworkManagementService; import android.os.Looper; import android.os.Message;