LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Nat464Xlat: interface notification handler on ConnectivityService

Browse Source 
This patch adds a layer of asynchonicity to the NetworkBaseObserver
callbacks implemented by Nat464Xlat in order to allow these callbacks
to run on the main ConnectivityService handler.

This allows to run interfaceLinkStateChanged and interfaceRemoved
callbacks in the same thread context as other Nat464Xlat methods and
solves the following issues:
  - NPE risk due to race between fixupLinkProperties called on the
    ConnectivityService thread and interfaceRemoved called as a
    callback by NetworkManagementService.
  - stale LinkProperties reads in both callbacks not called on
    ConnectivityService handler.
  - removes the race between stop() and interfaceRemoved().

This patch also:
  - removes/simplifies comments related to the threading
    model which are no obsolete.
  - extract clatd management logic from ConnectivityService into
    NetworkAgentInfo

Bug: 62997041
Bug: 64571917
Test:  runtest frameworks-net
       manually connected to ipv6 network and went to test-ipv6.com
Merged-In: I889d98e47423ff3d4746d6ed8015b265286e7c52
Merged-In: I2f002cd197e2eeaaadadd747a6b33d264cd34433
Merged-In: Id3ab064cf9f4417c0e8988fff4167b65b3c8c414
Merged-In: Ib224392c9a185f6bd79fd60cd5cb5549f2a7851e
Merged-In: I9116a493ca1cbdf6a25664a1b0017aa6c9b38eb4
Merged-In: I12918d208364eef55067ae9d59fbc38477e1f1c6

(cherry picked from commit 771d5c2f0126ba692897c9716f4098ae6e3a870c)

Change-Id: I34c4a0c32ce7c9b7bd7acf8f87b932e15c573bd8
This commit is contained in:
Hugo Benichi
2017-07-05 11:08:48 +09:00
parent 9cd15c7f2e
commit 39e10e2111
3 changed files with 96 additions and 94 deletions
Download Patch File Download Diff File Expand all files Collapse all files

56
services/core/java/com/android/server/ConnectivityService.java
View File

@@ -2011,16 +2011,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: {
@@ -2269,7 +2260,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.
@@ -4383,7 +4374,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 {
@@ -4398,32 +4389,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.
@@ -4658,6 +4623,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);

90
services/core/java/com/android/server/connectivity/Nat464Xlat.java
View File

@@ -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;
}
@@ -104,6 +96,13 @@ public class Nat464Xlat extends BaseNetworkObserver {
return mState != State.IDLE;
}
/**
* @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.
*/
@@ -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

44
services/core/java/com/android/server/connectivity/NetworkAgentInfo.java
View File

@@ -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() + "} " +