Nat464Xlat: internal state guards cleanup + state enum

This patch does some cleanup of Nat464Xlat internal state guards
against the Nat464Xlat state Idle | Started | Running, which reduces
code nesting.

It also replaces introspection of internal state for distinguishing
between different stages in 464xlat lifecycle with an enum explicitly
introducing these three Idle | Started | Running states.

Bug: 62997041
Bug: 64571917
Test:  runtest frameworks-net
       manually connected to ipv6 network and went to test-ipv6.com
Merged-In: I6efc9fed2420ca488731a2b9b9c3c025b16eca10
Merged-In: I188ac4c367db11cb33b67fe92df3a120e3c6fbce
Merged-In: I7e2c5db8d537fb0ab47cde37158b7f04d7786942
Merged-In: Ic2246a97618c596dbdbf48cda39c2f5b66e3bfb6
Merged-In: Ib04b9a3d56e9daf61b299a30e24a3c8839819a00
Merged-In: Icc1558a0f0e7c299270f550897347458e2bd3188

(cherry pick from commit 4f6f139869ddadf6f9ed50967c106a10a2e8ce3f)

Change-Id: Iebc7d153d8cd0b90d074d8d6eed821fbc3f1370d
This commit is contained in:
Hugo Benichi
2017-06-29 14:04:13 +09:00
parent 5a5c1bb116
commit 9cd15c7f2e

View File

@@ -16,8 +16,6 @@
package com.android.server.connectivity;
import java.net.Inet4Address;
import android.net.InterfaceConfiguration;
import android.net.ConnectivityManager;
import android.net.LinkAddress;
@@ -33,6 +31,9 @@ import android.util.Slog;
import com.android.server.net.BaseNetworkObserver;
import com.android.internal.util.ArrayUtils;
import java.net.Inet4Address;
import java.util.Objects;
/**
* Class to manage a 464xlat CLAT daemon.
*
@@ -60,21 +61,18 @@ public class Nat464Xlat extends BaseNetworkObserver {
// The network we're running on, and its type.
private final NetworkAgentInfo mNetwork;
// Internal state variables.
//
// The possible states are:
// - Idle: start() not called. Everything is null.
// - Starting: start() called. Interfaces are non-null. isStarted() returns true.
// mIsRunning is false.
// - Running: start() called, and interfaceLinkStateChanged() told us that mIface is up.
// mIsRunning is true.
//
private enum State {
IDLE, // start() not called. Base iface and stacked iface names are null.
STARTING, // start() called. Base iface and stacked iface names are known.
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 boolean mIsRunning;
private volatile State mState = State.IDLE;
public Nat464Xlat(INetworkManagementService nmService, Handler handler, NetworkAgentInfo nai) {
mNMService = nmService;
@@ -99,20 +97,36 @@ public class Nat464Xlat extends BaseNetworkObserver {
}
/**
* Determines whether clatd is started. Always true, except a) if start has not yet been called,
* or b) if our interface was removed.
* @return true if clatd has been started and has not yet stopped.
* A true result corresponds to internal states STARTING and RUNNING.
*/
public boolean isStarted() {
return mIface != null;
return mState != State.IDLE;
}
/**
* @return true if clatd has been started and the stacked interface is up.
*/
public boolean isRunning() {
return mState == State.RUNNING;
}
/**
* Sets internal state.
*/
private void enterStartingState(String baseIface) {
mIface = CLAT_PREFIX + baseIface;
mBaseIface = baseIface;
mState = State.STARTING;
}
/**
* Clears internal state. Must not be called by ConnectivityService.
*/
private void clear() {
private void enterIdleState() {
mIface = null;
mBaseIface = null;
mIsRunning = false;
mState = State.IDLE;
}
/**
@@ -136,19 +150,19 @@ public class Nat464Xlat extends BaseNetworkObserver {
return;
}
mBaseIface = mNetwork.linkProperties.getInterfaceName();
if (mBaseIface == null) {
String baseIface = mNetwork.linkProperties.getInterfaceName();
if (baseIface == null) {
Slog.e(TAG, "startClat: Can't start clat on null interface");
return;
}
mIface = CLAT_PREFIX + mBaseIface;
// From now on, isStarted() will return true.
// TODO: should we only do this if mNMService.startClatd() succeeds?
enterStartingState(baseIface);
Slog.i(TAG, "Starting clatd on " + mBaseIface);
try {
mNMService.startClatd(mBaseIface);
} catch(RemoteException|IllegalStateException e) {
Slog.e(TAG, "Error starting clatd: " + e);
Slog.e(TAG, "Error starting clatd on " + mBaseIface, e);
}
}
@@ -156,18 +170,19 @@ public class Nat464Xlat extends BaseNetworkObserver {
* Stops the clat daemon. Called by ConnectivityService on the handler thread.
*/
public void stop() {
if (isStarted()) {
Slog.i(TAG, "Stopping clatd");
if (!isStarted()) {
Slog.e(TAG, "stopClat: already stopped or not started");
return;
}
Slog.i(TAG, "Stopping clatd on " + mBaseIface);
try {
mNMService.stopClatd(mBaseIface);
} catch(RemoteException|IllegalStateException e) {
Slog.e(TAG, "Error stopping clatd: " + e);
Slog.e(TAG, "Error stopping clatd on " + mBaseIface, e);
}
// When clatd stops and its interface is deleted, interfaceRemoved() will notify
// ConnectivityService and call clear().
} else {
Slog.e(TAG, "clatd: already stopped");
}
// ConnectivityService and call enterIdleState().
}
private void updateConnectivityService(LinkProperties lp) {
@@ -183,16 +198,19 @@ public class Nat464Xlat extends BaseNetworkObserver {
* has no idea that 464xlat is running on top of it.
*/
public void fixupLinkProperties(LinkProperties oldLp) {
if (mNetwork.clatd != null &&
mIsRunning &&
mNetwork.linkProperties != null &&
!mNetwork.linkProperties.getAllInterfaceNames().contains(mIface)) {
if (!isRunning()) {
return;
}
LinkProperties lp = mNetwork.linkProperties;
if (lp == null || lp.getAllInterfaceNames().contains(mIface)) {
return;
}
Slog.d(TAG, "clatd running, updating NAI for " + mIface);
for (LinkProperties stacked: oldLp.getStackedLinks()) {
if (mIface.equals(stacked.getInterfaceName())) {
mNetwork.linkProperties.addStackedLink(stacked);
break;
}
if (Objects.equals(mIface, stacked.getInterfaceName())) {
lp.addStackedLink(stacked);
return;
}
}
}
@@ -238,33 +256,42 @@ 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().
*/
@Override
public void interfaceLinkStateChanged(String iface, boolean up) {
// Called by the InterfaceObserver on its own thread, so can race with stop().
if (isStarted() && up && mIface.equals(iface)) {
Slog.i(TAG, "interface " + iface + " is up, mIsRunning " + mIsRunning + "->true");
if (!mIsRunning) {
if (!isStarted() || !up || !Objects.equals(mIface, iface)) {
return;
}
if (isRunning()) {
return;
}
LinkAddress clatAddress = getLinkAddress(iface);
if (clatAddress == null) {
return;
}
mIsRunning = true;
mState = State.RUNNING;
Slog.i(TAG, String.format("interface %s is up, adding stacked link %s on top of %s",
mIface, mIface, mBaseIface));
maybeSetIpv6NdOffload(mBaseIface, false);
LinkProperties lp = new LinkProperties(mNetwork.linkProperties);
lp.addStackedLink(makeLinkProperties(clatAddress));
Slog.i(TAG, "Adding stacked link " + mIface + " on top of " + mBaseIface);
updateConnectivityService(lp);
}
}
}
@Override
public void interfaceRemoved(String iface) {
if (isStarted() && mIface.equals(iface)) {
Slog.i(TAG, "interface " + iface + " removed, mIsRunning " + mIsRunning + "->false");
if (!isStarted() || !Objects.equals(mIface, iface)) {
return;
}
if (!isRunning()) {
return;
}
if (mIsRunning) {
Slog.i(TAG, "interface " + iface + " removed");
// 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.
@@ -281,14 +308,12 @@ public class Nat464Xlat extends BaseNetworkObserver {
maybeSetIpv6NdOffload(mBaseIface, true);
LinkProperties lp = new LinkProperties(mNetwork.linkProperties);
lp.removeStackedLink(mIface);
clear();
enterIdleState();
updateConnectivityService(lp);
}
}
}
@Override
public String toString() {
return "mBaseIface: " + mBaseIface + ", mIface: " + mIface + ", mIsRunning: " + mIsRunning;
return "mBaseIface: " + mBaseIface + ", mIface: " + mIface + ", mState: " + mState;
}
}