From c7a1d23cdf87024ae799b3aecd4be338adb0e163 Mon Sep 17 00:00:00 2001 From: Paul Jensen Date: Mon, 6 Apr 2015 11:54:53 -0400 Subject: [PATCH 1/3] Non-functional code cleanup of ConnectivityService. 1. Remove ConnectivityService.findConnectionTypeForIface() as this can be done just as easily with supported APIs now. 2. Avoid making copies of Network objects as this precludes reuse of Network internals (e.g. socket factory, connection pool). Change-Id: I52f92e35d769d8350471f485e408169608630082 --- .../java/android/net/ConnectivityManager.java | 42 ------------------- .../android/net/IConnectivityManager.aidl | 2 - .../android/server/ConnectivityService.java | 33 +++++---------- 3 files changed, 10 insertions(+), 67 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 317e2361f6..826786b0bf 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -821,48 +821,6 @@ public class ConnectivityManager { } } - /** - * Tells each network type to set its radio power state as directed. - * - * @param turnOn a boolean, {@code true} to turn the radios on, - * {@code false} to turn them off. - * @return a boolean, {@code true} indicating success. All network types - * will be tried, even if some fail. - * - *

This method requires the caller to hold the permission - * {@link android.Manifest.permission#CHANGE_NETWORK_STATE}. - * {@hide} - */ -// TODO - check for any callers and remove -// public boolean setRadios(boolean turnOn) { -// try { -// return mService.setRadios(turnOn); -// } catch (RemoteException e) { -// return false; -// } -// } - - /** - * Tells a given networkType to set its radio power state as directed. - * - * @param networkType the int networkType of interest. - * @param turnOn a boolean, {@code true} to turn the radio on, - * {@code} false to turn it off. - * @return a boolean, {@code true} indicating success. - * - *

This method requires the caller to hold the permission - * {@link android.Manifest.permission#CHANGE_NETWORK_STATE}. - * {@hide} - */ -// TODO - check for any callers and remove -// public boolean setRadio(int networkType, boolean turnOn) { -// try { -// return mService.setRadio(networkType, turnOn); -// } catch (RemoteException e) { -// return false; -// } -// } - /** * Tells the underlying networking system that the caller wants to * begin using the named feature. The interpretation of {@code feature} diff --git a/core/java/android/net/IConnectivityManager.aidl b/core/java/android/net/IConnectivityManager.aidl index d8852f882c..31e60e7ce5 100644 --- a/core/java/android/net/IConnectivityManager.aidl +++ b/core/java/android/net/IConnectivityManager.aidl @@ -118,8 +118,6 @@ interface IConnectivityManager void captivePortalCheckCompleted(in NetworkInfo info, boolean isCaptivePortal); - int findConnectionTypeForIface(in String iface); - int checkMobileProvisioning(int suggestedTimeOutMs); String getMobileProvisioningUrl(); diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index d9ef766f2b..3103592c2d 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -721,7 +721,9 @@ public class ConnectivityService extends IConnectivityManager.Stub info = new NetworkInfo(nai.networkInfo); lp = new LinkProperties(nai.linkProperties); nc = new NetworkCapabilities(nai.networkCapabilities); - network = new Network(nai.network); + // Network objects are outwardly immutable so there is no point to duplicating. + // Duplicating also precludes sharing socket factories and connection pools. + network = nai.network; subscriberId = (nai.networkMisc != null) ? nai.networkMisc.subscriberId : null; } info.setType(networkType); @@ -789,7 +791,9 @@ public class ConnectivityService extends IConnectivityManager.Stub info = new NetworkInfo(nai.networkInfo); lp = new LinkProperties(nai.linkProperties); nc = new NetworkCapabilities(nai.networkCapabilities); - network = new Network(nai.network); + // Network objects are outwardly immutable so there is no point to duplicating. + // Duplicating also precludes sharing socket factories and connection pools. + network = nai.network; subscriberId = (nai.networkMisc != null) ? nai.networkMisc.subscriberId : null; } } @@ -967,13 +971,13 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public Network[] getAllNetworks() { enforceAccessPermission(); - final ArrayList result = new ArrayList(); synchronized (mNetworkForNetId) { + final Network[] result = new Network[mNetworkForNetId.size()]; for (int i = 0; i < mNetworkForNetId.size(); i++) { - result.add(new Network(mNetworkForNetId.valueAt(i).network)); + result[i] = mNetworkForNetId.valueAt(i).network; } + return result; } - return result.toArray(new Network[result.size()]); } private NetworkCapabilities getNetworkCapabilitiesAndValidation(NetworkAgentInfo nai) { @@ -2860,23 +2864,6 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - public int findConnectionTypeForIface(String iface) { - enforceConnectivityInternalPermission(); - - if (TextUtils.isEmpty(iface)) return ConnectivityManager.TYPE_NONE; - - synchronized(mNetworkForNetId) { - for (int i = 0; i < mNetworkForNetId.size(); i++) { - NetworkAgentInfo nai = mNetworkForNetId.valueAt(i); - LinkProperties lp = nai.linkProperties; - if (lp != null && iface.equals(lp.getInterfaceName()) && nai.networkInfo != null) { - return nai.networkInfo.getType(); - } - } - } - return ConnectivityManager.TYPE_NONE; - } - @Override public int checkMobileProvisioning(int suggestedTimeOutMs) { // TODO: Remove? Any reason to trigger a provisioning check? @@ -3130,7 +3117,7 @@ public class ConnectivityService extends IConnectivityManager.Stub loge("Starting user already has a VPN"); return; } - userVpn = new Vpn(mHandler.getLooper(), mContext, mNetd, this, userId); + userVpn = new Vpn(mHandler.getLooper(), mContext, mNetd, userId); mVpns.put(userId, userVpn); } } From 1f567385ff6f6b565559e94f5928629bf02b7129 Mon Sep 17 00:00:00 2001 From: Paul Jensen Date: Fri, 13 Feb 2015 14:18:39 -0500 Subject: [PATCH 2/3] Add ConnectivityManager.getActiveNetwork(). Rework NetID allocation in ConnectivityService so registerNetworkAgent() can return the allocated NetID. Bug: 19416463 Change-Id: I68e395552cf27422c80b4dfae5db5d56a0d68f5d --- .../java/android/net/ConnectivityManager.java | 35 ++++++++++-- .../android/net/IConnectivityManager.aidl | 3 +- core/java/android/net/NetworkAgent.java | 6 +- .../android/server/ConnectivityService.java | 57 +++++++++++++++---- .../server/connectivity/NetworkAgentInfo.java | 9 ++- 5 files changed, 91 insertions(+), 19 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 826786b0bf..99e368d5e4 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -601,6 +601,27 @@ public class ConnectivityManager { } } + /** + * Returns a {@link Network} object corresponding to the currently active + * default data network. In the event that the current active default data + * network disconnects, the returned {@code Network} object will no longer + * be usable. This will return {@code null} when there is no default + * network. + * + * @return a {@link Network} object for the current default network or + * {@code null} if no default network is currently active + * + *

This method requires the caller to hold the permission + * {@link android.Manifest.permission#ACCESS_NETWORK_STATE}. + */ + public Network getActiveNetwork() { + try { + return mService.getActiveNetwork(); + } catch (RemoteException e) { + return null; + } + } + /** * Returns details about the currently active default data network * for a given uid. This is for internal use only to avoid spying @@ -1927,12 +1948,18 @@ public class ConnectivityManager { } catch (RemoteException e) { } } - /** {@hide} */ - public void registerNetworkAgent(Messenger messenger, NetworkInfo ni, LinkProperties lp, + /** + * @hide + * Register a NetworkAgent with ConnectivityService. + * @return NetID corresponding to NetworkAgent. + */ + public int registerNetworkAgent(Messenger messenger, NetworkInfo ni, LinkProperties lp, NetworkCapabilities nc, int score, NetworkMisc misc) { try { - mService.registerNetworkAgent(messenger, ni, lp, nc, score, misc); - } catch (RemoteException e) { } + return mService.registerNetworkAgent(messenger, ni, lp, nc, score, misc); + } catch (RemoteException e) { + return NETID_UNSET; + } } /** diff --git a/core/java/android/net/IConnectivityManager.aidl b/core/java/android/net/IConnectivityManager.aidl index 31e60e7ce5..171c7d414d 100644 --- a/core/java/android/net/IConnectivityManager.aidl +++ b/core/java/android/net/IConnectivityManager.aidl @@ -42,6 +42,7 @@ import com.android.internal.net.VpnProfile; /** {@hide} */ interface IConnectivityManager { + Network getActiveNetwork(); NetworkInfo getActiveNetworkInfo(); NetworkInfo getActiveNetworkInfoForUid(int uid); NetworkInfo getNetworkInfo(int networkType); @@ -132,7 +133,7 @@ interface IConnectivityManager void unregisterNetworkFactory(in Messenger messenger); - void registerNetworkAgent(in Messenger messenger, in NetworkInfo ni, in LinkProperties lp, + int registerNetworkAgent(in Messenger messenger, in NetworkInfo ni, in LinkProperties lp, in NetworkCapabilities nc, int score, in NetworkMisc misc); NetworkRequest requestNetwork(in NetworkCapabilities networkCapabilities, diff --git a/core/java/android/net/NetworkAgent.java b/core/java/android/net/NetworkAgent.java index 74d4ac23e0..ddaa808d9b 100644 --- a/core/java/android/net/NetworkAgent.java +++ b/core/java/android/net/NetworkAgent.java @@ -42,6 +42,10 @@ import java.util.concurrent.atomic.AtomicBoolean; * @hide */ public abstract class NetworkAgent extends Handler { + // Guaranteed to be valid (not NETID_UNSET), otherwise registerNetworkAgent() would have thrown + // an exception. + public final int netId; + private volatile AsyncChannel mAsyncChannel; private final String LOG_TAG; private static final boolean DBG = true; @@ -142,7 +146,7 @@ public abstract class NetworkAgent extends Handler { if (VDBG) log("Registering NetworkAgent"); ConnectivityManager cm = (ConnectivityManager)mContext.getSystemService( Context.CONNECTIVITY_SERVICE); - cm.registerNetworkAgent(new Messenger(this), new NetworkInfo(ni), + netId = cm.registerNetworkAgent(new Messenger(this), new NetworkInfo(ni), new LinkProperties(lp), new NetworkCapabilities(nc), score, misc); } diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 3103592c2d..ee44b0990c 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -20,6 +20,7 @@ import static android.Manifest.permission.MANAGE_NETWORK_POLICY; import static android.Manifest.permission.RECEIVE_DATA_ACTIVITY_CHANGE; import static android.net.ConnectivityManager.CONNECTIVITY_ACTION; import static android.net.ConnectivityManager.CONNECTIVITY_ACTION_IMMEDIATE; +import static android.net.ConnectivityManager.NETID_UNSET; import static android.net.ConnectivityManager.TYPE_NONE; import static android.net.ConnectivityManager.TYPE_VPN; import static android.net.ConnectivityManager.getNetworkTypeName; @@ -89,6 +90,7 @@ import android.telephony.TelephonyManager; import android.text.TextUtils; import android.util.Slog; import android.util.SparseArray; +import android.util.SparseBooleanArray; import android.util.SparseIntArray; import android.util.Xml; @@ -691,16 +693,15 @@ public class ConnectivityService extends IConnectivityManager.Stub return mNextNetworkRequestId++; } - private void assignNextNetId(NetworkAgentInfo nai) { + private int reserveNetId() { synchronized (mNetworkForNetId) { for (int i = MIN_NET_ID; i <= MAX_NET_ID; i++) { int netId = mNextNetId; if (++mNextNetId > MAX_NET_ID) mNextNetId = MIN_NET_ID; // Make sure NetID unused. http://b/16815182 - if (mNetworkForNetId.get(netId) == null) { - nai.network = new Network(netId); - mNetworkForNetId.put(netId, nai); - return; + if (!mNetIdInUse.get(netId)) { + mNetIdInUse.put(netId, true); + return netId; } } } @@ -859,6 +860,28 @@ public class ConnectivityService extends IConnectivityManager.Stub return getFilteredNetworkInfo(state.networkInfo, state.linkProperties, uid); } + @Override + public Network getActiveNetwork() { + enforceAccessPermission(); + final int uid = Binder.getCallingUid(); + final int user = UserHandle.getUserId(uid); + int vpnNetId = NETID_UNSET; + synchronized (mVpns) { + final Vpn vpn = mVpns.get(user); + if (vpn != null && vpn.appliesToUid(uid)) vpnNetId = vpn.getNetId(); + } + NetworkAgentInfo nai; + if (vpnNetId != NETID_UNSET) { + synchronized (mNetworkForNetId) { + nai = mNetworkForNetId.get(vpnNetId); + } + if (nai != null) return nai.network; + } + nai = getDefaultNetwork(); + if (nai != null && isNetworkWithLinkPropertiesBlocked(nai.linkProperties, uid)) nai = null; + return nai != null ? nai.network : null; + } + /** * Find the first Provisioning network. * @@ -1942,6 +1965,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (nai != null) { synchronized (mNetworkForNetId) { mNetworkForNetId.remove(nai.network.netId); + mNetIdInUse.delete(nai.network.netId); } // Just in case. mLegacyTypeTracker.remove(nai); @@ -1985,6 +2009,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mLegacyTypeTracker.remove(nai); synchronized (mNetworkForNetId) { mNetworkForNetId.remove(nai.network.netId); + mNetIdInUse.delete(nai.network.netId); } // Since we've lost the network, go through all the requests that // it was satisfying and see if any other factory can satisfy them. @@ -3369,14 +3394,23 @@ public class ConnectivityService extends IConnectivityManager.Stub * and the are the highest scored network available. * the are keyed off the Requests requestId. */ + // TODO: Yikes, this is accessed on multiple threads: add synchronization. private final SparseArray mNetworkForRequestId = new SparseArray(); + // NOTE: Accessed on multiple threads, must be synchronized on itself. + @GuardedBy("mNetworkForNetId") private final SparseArray mNetworkForNetId = new SparseArray(); + // NOTE: Accessed on multiple threads, synchronized with mNetworkForNetId. + // An entry is first added to mNetIdInUse, prior to mNetworkForNetId, so + // there may not be a strict 1:1 correlation between the two. + @GuardedBy("mNetworkForNetId") + private final SparseBooleanArray mNetIdInUse = new SparseBooleanArray(); // NetworkAgentInfo keyed off its connecting messenger // TODO - eval if we can reduce the number of lists/hashmaps/sparsearrays + // NOTE: Only should be accessed on ConnectivityServiceThread, except dump(). private final HashMap mNetworkAgentInfos = new HashMap(); @@ -3391,7 +3425,7 @@ public class ConnectivityService extends IConnectivityManager.Stub return nai == getDefaultNetwork(); } - public void registerNetworkAgent(Messenger messenger, NetworkInfo networkInfo, + public int registerNetworkAgent(Messenger messenger, NetworkInfo networkInfo, LinkProperties linkProperties, NetworkCapabilities networkCapabilities, int currentScore, NetworkMisc networkMisc) { enforceConnectivityInternalPermission(); @@ -3399,20 +3433,23 @@ public class ConnectivityService extends IConnectivityManager.Stub // TODO: Instead of passing mDefaultRequest, provide an API to determine whether a Network // satisfies mDefaultRequest. NetworkAgentInfo nai = new NetworkAgentInfo(messenger, new AsyncChannel(), - new NetworkInfo(networkInfo), new LinkProperties(linkProperties), - new NetworkCapabilities(networkCapabilities), currentScore, mContext, mTrackerHandler, - new NetworkMisc(networkMisc), mDefaultRequest); + new Network(reserveNetId()), new NetworkInfo(networkInfo), new LinkProperties( + linkProperties), new NetworkCapabilities(networkCapabilities), currentScore, + mContext, mTrackerHandler, new NetworkMisc(networkMisc), mDefaultRequest); synchronized (this) { nai.networkMonitor.systemReady = mSystemReady; } if (DBG) log("registerNetworkAgent " + nai); mHandler.sendMessage(mHandler.obtainMessage(EVENT_REGISTER_NETWORK_AGENT, nai)); + return nai.network.netId; } private void handleRegisterNetworkAgent(NetworkAgentInfo na) { if (VDBG) log("Got NetworkAgent Messenger"); mNetworkAgentInfos.put(na.messenger, na); - assignNextNetId(na); + synchronized (mNetworkForNetId) { + mNetworkForNetId.put(na.network.netId, na); + } na.asyncChannel.connect(mContext, mTrackerHandler, na.messenger); NetworkInfo networkInfo = na.networkInfo; na.networkInfo = null; diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index f3e0bbc573..2d1f939eb8 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -40,7 +40,10 @@ import java.util.ArrayList; */ public class NetworkAgentInfo { public NetworkInfo networkInfo; - public Network network; + // This Network object should always be used if possible, so as to encourage reuse of the + // enclosed socket factory and connection pool. Avoid creating other Network objects. + // This Network object is always valid. + public final Network network; public LinkProperties linkProperties; public NetworkCapabilities networkCapabilities; public final NetworkMonitor networkMonitor; @@ -83,12 +86,12 @@ public class NetworkAgentInfo { // Used by ConnectivityService to keep track of 464xlat. public Nat464Xlat clatd; - public NetworkAgentInfo(Messenger messenger, AsyncChannel ac, NetworkInfo info, + public NetworkAgentInfo(Messenger messenger, AsyncChannel ac, Network net, NetworkInfo info, LinkProperties lp, NetworkCapabilities nc, int score, Context context, Handler handler, NetworkMisc misc, NetworkRequest defaultRequest) { this.messenger = messenger; asyncChannel = ac; - network = null; + network = net; networkInfo = info; linkProperties = lp; networkCapabilities = nc; From b95d7950f553583d3f4f749049b0115304d29966 Mon Sep 17 00:00:00 2001 From: Paul Jensen Date: Tue, 7 Apr 2015 12:43:13 -0400 Subject: [PATCH 3/3] Add ConnectivityManager.reportNetworkConnectivity() API This new API allows reporting networks that are perceived to provide Internet connectivity and networks that are not. This allows the framework to avoid needlessly reevaluating networks where the apps perception matches the framework's perception. This was not possible with the prior API, reportBadNetwork. Bug: 16214361 Change-Id: Id4409bd7538854bd837231fb50e693c10a62b4f2 --- .../java/android/net/ConnectivityManager.java | 25 ++++++++++++++++- .../android/net/IConnectivityManager.aidl | 2 +- .../android/server/ConnectivityService.java | 28 ++++++++++--------- 3 files changed, 40 insertions(+), 15 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 99e368d5e4..050cafc562 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -1708,10 +1708,33 @@ public class ConnectivityManager { * * @param network The {@link Network} the application was attempting to use * or {@code null} to indicate the current default network. + * @deprecated Use {@link #reportNetworkConnectivity} which allows reporting both + * working and non-working connectivity. */ public void reportBadNetwork(Network network) { try { - mService.reportBadNetwork(network); + // One of these will be ignored because it matches system's current state. + // The other will trigger the necessary reevaluation. + mService.reportNetworkConnectivity(network, true); + mService.reportNetworkConnectivity(network, false); + } catch (RemoteException e) { + } + } + + /** + * Report to the framework whether a network has working connectivity. + * This provides a hint to the system that a particular network is providing + * working connectivity or not. In response the framework may re-evaluate + * the network's connectivity and might take further action thereafter. + * + * @param network The {@link Network} the application was attempting to use + * or {@code null} to indicate the current default network. + * @param hasConnectivity {@code true} if the application was able to successfully access the + * Internet using {@code network} or {@code false} if not. + */ + public void reportNetworkConnectivity(Network network, boolean hasConnectivity) { + try { + mService.reportNetworkConnectivity(network, hasConnectivity); } catch (RemoteException e) { } } diff --git a/core/java/android/net/IConnectivityManager.aidl b/core/java/android/net/IConnectivityManager.aidl index 171c7d414d..05ef2e60fa 100644 --- a/core/java/android/net/IConnectivityManager.aidl +++ b/core/java/android/net/IConnectivityManager.aidl @@ -95,7 +95,7 @@ interface IConnectivityManager void reportInetCondition(int networkType, int percentage); - void reportBadNetwork(in Network network); + void reportNetworkConnectivity(in Network network, boolean hasConnectivity); ProxyInfo getGlobalProxy(); diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index ee44b0990c..ff6dc38298 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2461,25 +2461,27 @@ public class ConnectivityService extends IConnectivityManager.Stub public void reportInetCondition(int networkType, int percentage) { NetworkAgentInfo nai = mLegacyTypeTracker.getNetworkForType(networkType); if (nai == null) return; - boolean isGood = percentage > 50; - // Revalidate if the app report does not match our current validated state. - if (isGood != nai.lastValidated) { - // Make the message logged by reportBadNetwork below less confusing. - if (DBG && isGood) log("reportInetCondition: type=" + networkType + " ok, revalidate"); - reportBadNetwork(nai.network); - } + reportNetworkConnectivity(nai.network, percentage > 50); } - public void reportBadNetwork(Network network) { + public void reportNetworkConnectivity(Network network, boolean hasConnectivity) { enforceAccessPermission(); enforceInternetPermission(); - if (network == null) return; - - final int uid = Binder.getCallingUid(); - NetworkAgentInfo nai = getNetworkAgentInfoForNetwork(network); + NetworkAgentInfo nai; + if (network == null) { + nai = getDefaultNetwork(); + } else { + nai = getNetworkAgentInfoForNetwork(network); + } if (nai == null) return; - if (DBG) log("reportBadNetwork(" + nai.name() + ") by " + uid); + // Revalidate if the app report does not match our current validated state. + if (hasConnectivity == nai.lastValidated) return; + final int uid = Binder.getCallingUid(); + if (DBG) { + log("reportNetworkConnectivity(" + nai.network.netId + ", " + hasConnectivity + + ") by " + uid); + } synchronized (nai) { // Validating an uncreated network could result in a call to rematchNetworkAndRequests() // which isn't meant to work on uncreated networks.