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

Nat464Xlat: correct racefree teardown

Browse Source 
This patch relays the NetworkBaseObserver notifications about nat
464xlat stacked interfaces onto the ConnectivityService handler.

This allows to process interface up and down notifications in the
same thread context and eliminates several races:

  - NPE risk due to race between fixupLinkProperties called on
    ConnectivityService thread and interfaceRemoved called on
    NetworkManagementService thread.
  - stale LinkProperties pointer reads in both NetworkBaseObserver
    callbacks not called on ConnectivityService handler.
  - removes the race between stop() and interfaceRemoved().
  - removes superfluous LinkProperties notifications when stop() is
    called before the stacked interface goes up.

The teardown procedure logic common to stop() and interfaceRemoved() is
put into enterStoppedState() and enterIdleState().

This allows to distinguish and correctly handle the following teardown
scenarios:
 - an IPv4 appears -> ConnectivityService calls Nat464Xlat#stop()
                   -> Nat464Xlat calls stopClatd
                   -> clatd stops
                   -> if the stacked interface was up, it is removed
                   -> Nat464Xlat#interfaceRemoved() is triggered and
                      a LinkProperties update is sent.

 - network disconnects -> ConnectivityService calls Nat464Xlat#stop()
                       -> Nat464Xlat calls stopClatd
                       -> clatd stops
                       -> if the stacked interface was up, it is removed
                       -> Nat464Xlat#interfaceRemoved() is triggered and
                          a LinkProperties update is sent.

 - clatd crashes or exit -> Nat464Xlat#interfaceRemoved() is triggered
                         -> Nat464Xlat unregisters itself as a network
                            observer
                         -> ConnectivityService is updated about the
                            stacked interface missing, and restarts
                            Nat464Xlat if needed.

Note that the first two scenarios have two cases: stop() can be called
before the notification for the stacked interface going up (STARTED), or
after (RUNNING). In the first case, Nat464Xlat must unregister
immediately as a network observer to avoid leaks.

This patch also:
  - removes/simplifies comments related to the threading model which
    are no obsolete.
  - extract clatd management logic from ConnectivityService into
    NetworkAgentInfo
  - add new unit tests where there was none before.

Bug: 62918393
Bug: 62997041
Bug: 64571917
Bug: 65225023
Test: runtest frameworks-net
Change-Id: I27221a8a60fd9760b567ed322cc79228df877e56
This commit is contained in:
Hugo Benichi
2017-09-01 01:23:32 +00:00
parent eda8324beb
commit 9d35b759e5
4 changed files with 396 additions and 118 deletions
Download Patch File Download Diff File Expand all files Collapse all files

226
tests/net/java/com/android/server/connectivity/Nat464XlatTest.java Normal file
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());
}
}