Merge "Nat464Xlat: correct racefree teardown" into oc-mr1-dev

This commit is contained in:
Hugo Benichi
2017-09-05 22:35:45 +00:00
committed by Android (Google) Code Review
4 changed files with 396 additions and 118 deletions

View File

@@ -2024,16 +2024,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: {
@@ -2282,7 +2273,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.
@@ -4402,7 +4393,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 {
@@ -4417,32 +4408,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.
@@ -4677,6 +4642,26 @@ public class ConnectivityService extends IConnectivityManager.Stub
}
}
public void handleUpdateLinkProperties(NetworkAgentInfo nai, LinkProperties newLp) {
if (mNetworkForNetId.get(nai.network.netId) != nai) {
// Ignore updates for disconnected networks
return;
}
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);

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 responsibility of ConnectivityService to
* call into this class from its own Handler thread.
*
* @hide
*/
@@ -55,28 +54,23 @@ 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;
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.
RUNNING, // start() called, and the stacked iface is known to be up.
STOPPING; // stop() called, this Nat464Xlat is still registered as a network observer for
// the stacked interface.
}
// 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;
}
@@ -89,6 +83,8 @@ public class Nat464Xlat extends BaseNetworkObserver {
// TODO: migrate to NetworkCapabilities.TRANSPORT_*.
final int netType = nai.networkInfo.getType();
final boolean supported = ArrayUtils.contains(NETWORK_TYPES, nai.networkInfo.getType());
// TODO: this should also consider if the network is in SUSPENDED state to avoid stopping
// clatd in SUSPENDED state.
final boolean connected = nai.networkInfo.isConnected();
// We only run clat on networks that don't have a native IPv4 address.
final boolean hasIPv4Address =
@@ -104,6 +100,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.
*/
@@ -112,25 +115,77 @@ public class Nat464Xlat extends BaseNetworkObserver {
}
/**
* Sets internal state.
* @return true if clatd has been stopped.
*/
public boolean isStopping() {
return mState == State.STOPPING;
}
/**
* Start clatd, register this Nat464Xlat as a network observer for the stacked interface,
* and set internal state.
*/
private void enterStartingState(String baseIface) {
try {
mNMService.registerObserver(this);
} catch(RemoteException e) {
Slog.e(TAG,
"startClat: Can't register interface observer for clat on " + mNetwork.name());
return;
}
try {
mNMService.startClatd(baseIface);
} catch(RemoteException|IllegalStateException e) {
Slog.e(TAG, "Error starting clatd on " + baseIface, e);
}
mIface = CLAT_PREFIX + baseIface;
mBaseIface = baseIface;
mState = State.STARTING;
}
/**
* Clears internal state. Must not be called by ConnectivityService.
* Enter running state just after getting confirmation that the stacked interface is up, and
* turn ND offload off if on WiFi.
*/
private void enterRunningState() {
maybeSetIpv6NdOffload(mBaseIface, false);
mState = State.RUNNING;
}
/**
* Stop clatd, and turn ND offload on if it had been turned off.
*/
private void enterStoppingState() {
if (isRunning()) {
maybeSetIpv6NdOffload(mBaseIface, true);
}
try {
mNMService.stopClatd(mBaseIface);
} catch(RemoteException|IllegalStateException e) {
Slog.e(TAG, "Error stopping clatd on " + mBaseIface, e);
}
mState = State.STOPPING;
}
/**
* Unregister as a base observer for the stacked interface, and clear internal state.
*/
private void enterIdleState() {
try {
mNMService.unregisterObserver(this);
} catch(RemoteException|IllegalStateException e) {
Slog.e(TAG, "Error unregistering clatd observer on " + mBaseIface, e);
}
mIface = null;
mBaseIface = null;
mState = State.IDLE;
}
/**
* Starts the clat daemon. Called by ConnectivityService on the handler thread.
* Starts the clat daemon.
*/
public void start() {
if (isStarted()) {
@@ -143,53 +198,30 @@ public class Nat464Xlat extends BaseNetworkObserver {
return;
}
try {
mNMService.registerObserver(this);
} catch(RemoteException e) {
Slog.e(TAG, "startClat: Can't register interface observer for clat on " + mNetwork);
return;
}
String baseIface = mNetwork.linkProperties.getInterfaceName();
if (baseIface == null) {
Slog.e(TAG, "startClat: Can't start clat on null interface");
return;
}
// TODO: should we only do this if mNMService.startClatd() succeeds?
Slog.i(TAG, "Starting clatd on " + baseIface);
enterStartingState(baseIface);
Slog.i(TAG, "Starting clatd on " + mBaseIface);
try {
mNMService.startClatd(mBaseIface);
} catch(RemoteException|IllegalStateException e) {
Slog.e(TAG, "Error starting clatd on " + mBaseIface, e);
}
}
/**
* Stops the clat daemon. Called by ConnectivityService on the handler thread.
* Stops the clat daemon.
*/
public void stop() {
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 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();
boolean wasStarting = isStarting();
enterStoppingState();
if (wasStarting) {
enterIdleState();
}
}
/**
@@ -257,59 +289,58 @@ 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, "clatAddress was null for stacked iface " + iface);
return;
}
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);
enterRunningState();
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)) {
/**
* Removes stacked link on base link and transitions to IDLE state.
*/
private void handleInterfaceRemoved(String iface) {
if (!Objects.equals(mIface, iface)) {
return;
}
if (!isRunning()) {
if (!isRunning() && !isStopping()) {
return;
}
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.
//
// 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);
mNMService.stopClatd(mBaseIface);
} catch (RemoteException|IllegalStateException e) {
// Well, we tried.
if (!isStopping()) {
// Ensure clatd is stopped if stop() has not been called: this likely means that clatd
// has crashed.
enterStoppingState();
}
maybeSetIpv6NdOffload(mBaseIface, true);
LinkProperties lp = new LinkProperties(mNetwork.linkProperties);
lp.removeStackedLink(mIface);
enterIdleState();
updateConnectivityService(lp);
LinkProperties lp = new LinkProperties(mNetwork.linkProperties);
lp.removeStackedLink(iface);
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

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;
@@ -268,6 +270,14 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
networkMisc = misc;
}
public ConnectivityService connService() {
return mConnService;
}
public Handler handler() {
return mHandler;
}
// Functions for manipulating the requests satisfied by this network.
//
// These functions must only called on ConnectivityService's main thread.
@@ -551,6 +561,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() + "} " +

View File

@@ -0,0 +1,226 @@
/*
* Copyright (C) 2017 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.android.server.connectivity;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
import android.net.ConnectivityManager;
import android.net.InterfaceConfiguration;
import android.net.LinkAddress;
import android.net.LinkProperties;
import android.net.NetworkInfo;
import android.os.Handler;
import android.os.INetworkManagementService;
import android.os.test.TestLooper;
import android.support.test.filters.SmallTest;
import android.support.test.runner.AndroidJUnit4;
import com.android.server.ConnectivityService;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
@RunWith(AndroidJUnit4.class)
@SmallTest
public class Nat464XlatTest {
static final String BASE_IFACE = "test0";
static final String STACKED_IFACE = "v4-test0";
static final LinkAddress ADDR = new LinkAddress("192.0.2.5/29");
@Mock ConnectivityService mConnectivity;
@Mock INetworkManagementService mNms;
@Mock InterfaceConfiguration mConfig;
@Mock NetworkAgentInfo mNai;
TestLooper mLooper;
Handler mHandler;
Nat464Xlat makeNat464Xlat() {
return new Nat464Xlat(mNms, mNai);
}
@Before
public void setUp() throws Exception {
mLooper = new TestLooper();
mHandler = new Handler(mLooper.getLooper());
MockitoAnnotations.initMocks(this);
mNai.linkProperties = new LinkProperties();
mNai.linkProperties.setInterfaceName(BASE_IFACE);
mNai.networkInfo = new NetworkInfo(null);
mNai.networkInfo.setType(ConnectivityManager.TYPE_WIFI);
when(mNai.connService()).thenReturn(mConnectivity);
when(mNai.handler()).thenReturn(mHandler);
when(mNms.getInterfaceConfig(eq(STACKED_IFACE))).thenReturn(mConfig);
when(mConfig.getLinkAddress()).thenReturn(ADDR);
}
@Test
public void testNormalStartAndStop() throws Exception {
Nat464Xlat nat = makeNat464Xlat();
ArgumentCaptor<LinkProperties> c = ArgumentCaptor.forClass(LinkProperties.class);
// ConnectivityService starts clat.
nat.start();
verify(mNms).registerObserver(eq(nat));
verify(mNms).startClatd(eq(BASE_IFACE));
// Stacked interface up notification arrives.
nat.interfaceLinkStateChanged(STACKED_IFACE, true);
mLooper.dispatchNext();
verify(mNms).getInterfaceConfig(eq(STACKED_IFACE));
verify(mNms).setInterfaceIpv6NdOffload(eq(BASE_IFACE), eq(false));
verify(mConnectivity).handleUpdateLinkProperties(eq(mNai), c.capture());
assertFalse(c.getValue().getStackedLinks().isEmpty());
assertTrue(c.getValue().getAllInterfaceNames().contains(STACKED_IFACE));
assertRunning(nat);
// ConnectivityService stops clat (Network disconnects, IPv4 addr appears, ...).
nat.stop();
verify(mNms).stopClatd(eq(BASE_IFACE));
verify(mNms).setInterfaceIpv6NdOffload(eq(BASE_IFACE), eq(true));
// Stacked interface removed notification arrives.
nat.interfaceRemoved(STACKED_IFACE);
mLooper.dispatchNext();
verify(mNms).unregisterObserver(eq(nat));
verify(mConnectivity, times(2)).handleUpdateLinkProperties(eq(mNai), c.capture());
assertTrue(c.getValue().getStackedLinks().isEmpty());
assertFalse(c.getValue().getAllInterfaceNames().contains(STACKED_IFACE));
assertIdle(nat);
verifyNoMoreInteractions(mNms, mConnectivity);
}
@Test
public void testClatdCrashWhileRunning() throws Exception {
Nat464Xlat nat = makeNat464Xlat();
ArgumentCaptor<LinkProperties> c = ArgumentCaptor.forClass(LinkProperties.class);
// ConnectivityService starts clat.
nat.start();
verify(mNms).registerObserver(eq(nat));
verify(mNms).startClatd(eq(BASE_IFACE));
// Stacked interface up notification arrives.
nat.interfaceLinkStateChanged(STACKED_IFACE, true);
mLooper.dispatchNext();
verify(mNms).getInterfaceConfig(eq(STACKED_IFACE));
verify(mNms).setInterfaceIpv6NdOffload(eq(BASE_IFACE), eq(false));
verify(mConnectivity, times(1)).handleUpdateLinkProperties(eq(mNai), c.capture());
assertFalse(c.getValue().getStackedLinks().isEmpty());
assertTrue(c.getValue().getAllInterfaceNames().contains(STACKED_IFACE));
assertRunning(nat);
// Stacked interface removed notification arrives (clatd crashed, ...).
nat.interfaceRemoved(STACKED_IFACE);
mLooper.dispatchNext();
verify(mNms).unregisterObserver(eq(nat));
verify(mNms).stopClatd(eq(BASE_IFACE));
verify(mNms).setInterfaceIpv6NdOffload(eq(BASE_IFACE), eq(true));
verify(mConnectivity, times(2)).handleUpdateLinkProperties(eq(mNai), c.capture());
assertTrue(c.getValue().getStackedLinks().isEmpty());
assertFalse(c.getValue().getAllInterfaceNames().contains(STACKED_IFACE));
assertIdle(nat);
// ConnectivityService stops clat: no-op.
nat.stop();
verifyNoMoreInteractions(mNms, mConnectivity);
}
@Test
public void testStopBeforeClatdStarts() throws Exception {
Nat464Xlat nat = makeNat464Xlat();
// ConnectivityService starts clat.
nat.start();
verify(mNms).registerObserver(eq(nat));
verify(mNms).startClatd(eq(BASE_IFACE));
// ConnectivityService immediately stops clat (Network disconnects, IPv4 addr appears, ...)
nat.stop();
verify(mNms).unregisterObserver(eq(nat));
verify(mNms).stopClatd(eq(BASE_IFACE));
assertIdle(nat);
// In-flight interface up notification arrives: no-op
nat.interfaceLinkStateChanged(STACKED_IFACE, true);
mLooper.dispatchNext();
// Interface removed notification arrives after stopClatd() takes effect: no-op.
nat.interfaceRemoved(STACKED_IFACE);
mLooper.dispatchNext();
assertIdle(nat);
verifyNoMoreInteractions(mNms, mConnectivity);
}
@Test
public void testStopAndClatdNeverStarts() throws Exception {
Nat464Xlat nat = makeNat464Xlat();
// ConnectivityService starts clat.
nat.start();
verify(mNms).registerObserver(eq(nat));
verify(mNms).startClatd(eq(BASE_IFACE));
// ConnectivityService immediately stops clat (Network disconnects, IPv4 addr appears, ...)
nat.stop();
verify(mNms).unregisterObserver(eq(nat));
verify(mNms).stopClatd(eq(BASE_IFACE));
assertIdle(nat);
verifyNoMoreInteractions(mNms, mConnectivity);
}
static void assertIdle(Nat464Xlat nat) {
assertTrue("Nat464Xlat was not IDLE", !nat.isStarted());
}
static void assertRunning(Nat464Xlat nat) {
assertTrue("Nat464Xlat was not RUNNING", nat.isRunning());
}
}