diff options
3 files changed, 96 insertions, 94 deletions
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index aa878efe619a..fb20533837af 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2023,16 +2023,7 @@ public class ConnectivityService extends IConnectivityManager.Stub break; } case NetworkAgent.EVENT_NETWORK_PROPERTIES_CHANGED: { - if (VDBG) { - log("Update of LinkProperties for " + nai.name() + - "; created=" + nai.created + - "; everConnected=" + nai.everConnected); - } - LinkProperties oldLp = nai.linkProperties; - synchronized (nai) { - nai.linkProperties = (LinkProperties)msg.obj; - } - if (nai.everConnected) updateLinkProperties(nai, oldLp); + handleUpdateLinkProperties(nai, (LinkProperties) msg.obj); break; } case NetworkAgent.EVENT_NETWORK_INFO_CHANGED: { @@ -2281,7 +2272,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } nai.networkMonitor.sendMessage(NetworkMonitor.CMD_NETWORK_DISCONNECTED); mNetworkAgentInfos.remove(msg.replyTo); - maybeStopClat(nai); + nai.maybeStopClat(); synchronized (mNetworkForNetId) { // Remove the NetworkAgent, but don't mark the netId as // available until we've told netd to delete it below. @@ -4375,7 +4366,7 @@ public class ConnectivityService extends IConnectivityManager.Stub updateDnses(newLp, oldLp, netId); // Start or stop clat accordingly to network state. - updateClat(networkAgent); + networkAgent.updateClat(mNetd); if (isDefaultNetwork(networkAgent)) { handleApplyDefaultProxy(newLp.getHttpProxy()); } else { @@ -4390,32 +4381,6 @@ public class ConnectivityService extends IConnectivityManager.Stub mKeepaliveTracker.handleCheckKeepalivesStillValid(networkAgent); } - private void updateClat(NetworkAgentInfo nai) { - if (Nat464Xlat.requiresClat(nai)) { - maybeStartClat(nai); - } else { - maybeStopClat(nai); - } - } - - /** Ensure clat has started for this network. */ - private void maybeStartClat(NetworkAgentInfo nai) { - if (nai.clatd != null && nai.clatd.isStarted()) { - return; - } - nai.clatd = new Nat464Xlat(mNetd, mTrackerHandler, nai); - nai.clatd.start(); - } - - /** Ensure clat has stopped for this network. */ - private void maybeStopClat(NetworkAgentInfo nai) { - if (nai.clatd == null) { - return; - } - nai.clatd.stop(); - nai.clatd = null; - } - private void wakeupModifyInterface(String iface, NetworkCapabilities caps, boolean add) { // Marks are only available on WiFi interaces. Checking for // marks on unsupported interfaces is harmless. @@ -4650,6 +4615,21 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + public void handleUpdateLinkProperties(NetworkAgentInfo nai, LinkProperties newLp) { + if (VDBG) { + log("Update of LinkProperties for " + nai.name() + + "; created=" + nai.created + + "; everConnected=" + nai.everConnected); + } + LinkProperties oldLp = nai.linkProperties; + synchronized (nai) { + nai.linkProperties = newLp; + } + if (nai.everConnected) { + updateLinkProperties(nai, oldLp); + } + } + private void sendUpdatedScoreToFactories(NetworkAgentInfo nai) { for (int i = 0; i < nai.numNetworkRequests(); i++) { NetworkRequest nr = nai.requestAt(i); diff --git a/services/core/java/com/android/server/connectivity/Nat464Xlat.java b/services/core/java/com/android/server/connectivity/Nat464Xlat.java index f8d23d4c8d51..10c8b8b1e0aa 100644 --- a/services/core/java/com/android/server/connectivity/Nat464Xlat.java +++ b/services/core/java/com/android/server/connectivity/Nat464Xlat.java @@ -20,22 +20,21 @@ import android.net.InterfaceConfiguration; import android.net.ConnectivityManager; import android.net.LinkAddress; import android.net.LinkProperties; -import android.net.NetworkAgent; import android.net.RouteInfo; -import android.os.Handler; -import android.os.Message; import android.os.INetworkManagementService; import android.os.RemoteException; import android.util.Slog; -import com.android.server.net.BaseNetworkObserver; import com.android.internal.util.ArrayUtils; +import com.android.server.net.BaseNetworkObserver; import java.net.Inet4Address; import java.util.Objects; /** - * Class to manage a 464xlat CLAT daemon. + * Class to manage a 464xlat CLAT daemon. Nat464Xlat is not thread safe and should be manipulated + * from a consistent and unique thread context. It is the responsability of ConnectivityService to + * call into this class from its own Handler thread. * * @hide */ @@ -55,9 +54,6 @@ public class Nat464Xlat extends BaseNetworkObserver { private final INetworkManagementService mNMService; - // ConnectivityService Handler for LinkProperties updates. - private final Handler mHandler; - // The network we're running on, and its type. private final NetworkAgentInfo mNetwork; @@ -67,16 +63,12 @@ public class Nat464Xlat extends BaseNetworkObserver { RUNNING; // start() called, and the stacked iface is known to be up. } - // Once mIface is non-null and isStarted() is true, methods called by ConnectivityService on - // its handler thread must not modify any internal state variables; they are only updated by the - // interface observers, called on the notification threads. private String mBaseIface; private String mIface; - private volatile State mState = State.IDLE; + private State mState = State.IDLE; - public Nat464Xlat(INetworkManagementService nmService, Handler handler, NetworkAgentInfo nai) { + public Nat464Xlat(INetworkManagementService nmService, NetworkAgentInfo nai) { mNMService = nmService; - mHandler = handler; mNetwork = nai; } @@ -105,6 +97,13 @@ public class Nat464Xlat extends BaseNetworkObserver { } /** + * @return true if clatd has been started but the stacked interface is not yet up. + */ + public boolean isStarting() { + return mState == State.STARTING; + } + + /** * @return true if clatd has been started and the stacked interface is up. */ public boolean isRunning() { @@ -121,7 +120,7 @@ public class Nat464Xlat extends BaseNetworkObserver { } /** - * Clears internal state. Must not be called by ConnectivityService. + * Clears internal state. */ private void enterIdleState() { mIface = null; @@ -130,7 +129,7 @@ public class Nat464Xlat extends BaseNetworkObserver { } /** - * Starts the clat daemon. Called by ConnectivityService on the handler thread. + * Starts the clat daemon. */ public void start() { if (isStarted()) { @@ -167,7 +166,7 @@ public class Nat464Xlat extends BaseNetworkObserver { } /** - * Stops the clat daemon. Called by ConnectivityService on the handler thread. + * Stops the clat daemon. */ public void stop() { if (!isStarted()) { @@ -181,15 +180,8 @@ public class Nat464Xlat extends BaseNetworkObserver { } catch(RemoteException|IllegalStateException e) { Slog.e(TAG, "Error stopping clatd on " + mBaseIface, e); } - // When clatd stops and its interface is deleted, interfaceRemoved() will notify - // ConnectivityService and call enterIdleState(). - } - - private void updateConnectivityService(LinkProperties lp) { - Message msg = mHandler.obtainMessage(NetworkAgent.EVENT_NETWORK_PROPERTIES_CHANGED, lp); - msg.replyTo = mNetwork.messenger; - Slog.i(TAG, "sending message to ConnectivityService: " + msg); - msg.sendToTarget(); + // When clatd stops and its interface is deleted, handleInterfaceRemoved() will trigger + // ConnectivityService#handleUpdateLinkProperties and call enterIdleState(). } /** @@ -257,19 +249,15 @@ public class Nat464Xlat extends BaseNetworkObserver { } /** - * Adds stacked link on base link and transitions to Running state - * This is called by the InterfaceObserver on its own thread, so can race with stop(). + * Adds stacked link on base link and transitions to RUNNING state. */ - @Override - public void interfaceLinkStateChanged(String iface, boolean up) { - if (!isStarted() || !up || !Objects.equals(mIface, iface)) { - return; - } - if (isRunning()) { + private void handleInterfaceLinkStateChanged(String iface, boolean up) { + if (!isStarting() || !up || !Objects.equals(mIface, iface)) { return; } LinkAddress clatAddress = getLinkAddress(iface); if (clatAddress == null) { + Slog.e(TAG, "cladAddress was null for stacked iface " + iface); return; } mState = State.RUNNING; @@ -279,15 +267,14 @@ public class Nat464Xlat extends BaseNetworkObserver { maybeSetIpv6NdOffload(mBaseIface, false); LinkProperties lp = new LinkProperties(mNetwork.linkProperties); lp.addStackedLink(makeLinkProperties(clatAddress)); - updateConnectivityService(lp); + mNetwork.connService.handleUpdateLinkProperties(mNetwork, lp); } - @Override - public void interfaceRemoved(String iface) { - if (!isStarted() || !Objects.equals(mIface, iface)) { - return; - } - if (!isRunning()) { + /** + * Removes stacked link on base link and transitions to IDLE state. + */ + private void handleInterfaceRemoved(String iface) { + if (!isRunning() || !Objects.equals(mIface, iface)) { return; } @@ -295,21 +282,28 @@ public class Nat464Xlat extends BaseNetworkObserver { // The interface going away likely means clatd has crashed. Ask netd to stop it, // because otherwise when we try to start it again on the same base interface netd // will complain that it's already started. - // - // Note that this method can be called by the interface observer at the same time - // that ConnectivityService calls stop(). In this case, the second call to - // stopClatd() will just throw IllegalStateException, which we'll ignore. try { mNMService.unregisterObserver(this); + // TODO: add STOPPING state to avoid calling stopClatd twice. mNMService.stopClatd(mBaseIface); - } catch (RemoteException|IllegalStateException e) { - // Well, we tried. + } catch(RemoteException|IllegalStateException e) { + Slog.e(TAG, "Error stopping clatd on " + mBaseIface, e); } maybeSetIpv6NdOffload(mBaseIface, true); LinkProperties lp = new LinkProperties(mNetwork.linkProperties); lp.removeStackedLink(mIface); enterIdleState(); - updateConnectivityService(lp); + mNetwork.connService.handleUpdateLinkProperties(mNetwork, lp); + } + + @Override + public void interfaceLinkStateChanged(String iface, boolean up) { + mNetwork.handler.post(() -> { handleInterfaceLinkStateChanged(iface, up); }); + } + + @Override + public void interfaceRemoved(String iface) { + mNetwork.handler.post(() -> { handleInterfaceRemoved(iface); }); } @Override diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index 872923a03256..7c4ef0f0f3b9 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -27,7 +27,9 @@ import android.net.NetworkMisc; import android.net.NetworkRequest; import android.net.NetworkState; import android.os.Handler; +import android.os.INetworkManagementService; import android.os.Messenger; +import android.os.RemoteException; import android.os.SystemClock; import android.util.Log; import android.util.SparseArray; @@ -247,9 +249,9 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> { private static final String TAG = ConnectivityService.class.getSimpleName(); private static final boolean VDBG = false; - private final ConnectivityService mConnService; + public final ConnectivityService connService; private final Context mContext; - private final Handler mHandler; + final Handler handler; public NetworkAgentInfo(Messenger messenger, AsyncChannel ac, Network net, NetworkInfo info, LinkProperties lp, NetworkCapabilities nc, int score, Context context, Handler handler, @@ -261,10 +263,10 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> { linkProperties = lp; networkCapabilities = nc; currentScore = score; - mConnService = connService; + this.connService = connService; mContext = context; - mHandler = handler; - networkMonitor = mConnService.createNetworkMonitor(context, handler, this, defaultRequest); + this.handler = handler; + networkMonitor = connService.createNetworkMonitor(context, handler, this, defaultRequest); networkMisc = misc; } @@ -430,7 +432,7 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> { private boolean ignoreWifiUnvalidationPenalty() { boolean isWifi = networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) && networkCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET); - boolean avoidBadWifi = mConnService.avoidBadWifi() || avoidUnvalidated; + boolean avoidBadWifi = connService.avoidBadWifi() || avoidUnvalidated; return isWifi && !avoidBadWifi && everValidated; } @@ -514,8 +516,8 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> { } if (newExpiry > 0) { - mLingerMessage = mConnService.makeWakeupMessage( - mContext, mHandler, + mLingerMessage = connService.makeWakeupMessage( + mContext, handler, "NETWORK_LINGER_COMPLETE." + network.netId, EVENT_NETWORK_LINGER_COMPLETE, this); mLingerMessage.schedule(newExpiry); @@ -551,6 +553,32 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> { for (LingerTimer timer : mLingerTimers) { pw.println(timer); } } + public void updateClat(INetworkManagementService netd) { + if (Nat464Xlat.requiresClat(this)) { + maybeStartClat(netd); + } else { + maybeStopClat(); + } + } + + /** Ensure clat has started for this network. */ + public void maybeStartClat(INetworkManagementService netd) { + if (clatd != null && clatd.isStarted()) { + return; + } + clatd = new Nat464Xlat(netd, this); + clatd.start(); + } + + /** Ensure clat has stopped for this network. */ + public void maybeStopClat() { + if (clatd == null) { + return; + } + clatd.stop(); + clatd = null; + } + public String toString() { return "NetworkAgentInfo{ ni{" + networkInfo + "} " + "network{" + network + "} nethandle{" + network.getNetworkHandle() + "} " + |