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

Merge "Nat464Xlat: rely on netd events being called on handler thread." into main

Browse Source
This commit is contained in:
Remi NGUYEN VAN
2023-09-27 02:01:58 +00:00
committed by Gerrit Code Review
parent f0afc53836 3b817cbad3
commit d196fab898
4 changed files with 36 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
service/src/com/android/server/ConnectivityService.java
View File

@@ -11418,7 +11418,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
public void onInterfaceLinkStateChanged(@NonNull String iface, boolean up) {
mHandler.post(() -> {
for (NetworkAgentInfo nai : mNetworkAgentInfos) {
nai.clatd.interfaceLinkStateChanged(iface, up);
nai.clatd.handleInterfaceLinkStateChanged(iface, up);
}
});
}
@@ -11427,7 +11427,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
public void onInterfaceRemoved(@NonNull String iface) {
mHandler.post(() -> {
for (NetworkAgentInfo nai : mNetworkAgentInfos) {
nai.clatd.interfaceRemoved(iface);
nai.clatd.handleInterfaceRemoved(iface);
}
});
}

16
service/src/com/android/server/connectivity/Nat464Xlat.java
View File

@@ -483,8 +483,9 @@ public class Nat464Xlat {
/**
* Adds stacked link on base link and transitions to RUNNING state.
* Must be called on the handler thread.
*/
private void handleInterfaceLinkStateChanged(String iface, boolean up) {
public void handleInterfaceLinkStateChanged(String iface, boolean up) {
// TODO: if we call start(), then stop(), then start() again, and the
// interfaceLinkStateChanged notification for the first start is delayed past the first
// stop, then the code becomes out of sync with system state and will behave incorrectly.
@@ -499,6 +500,7 @@ public class Nat464Xlat {
// Once this code is converted to StateMachine, it will be possible to use deferMessage to
// ensure it stays in STARTING state until the interfaceLinkStateChanged notification fires,
// and possibly use a timeout (or provide some guarantees at the lower layer) to address #1.
ensureRunningOnHandlerThread();
if (!isStarting() || !up || !Objects.equals(mIface, iface)) {
return;
}
@@ -519,8 +521,10 @@ public class Nat464Xlat {
/**
* Removes stacked link on base link and transitions to IDLE state.
* Must be called on the handler thread.
*/
private void handleInterfaceRemoved(String iface) {
public void handleInterfaceRemoved(String iface) {
ensureRunningOnHandlerThread();
if (!Objects.equals(mIface, iface)) {
return;
}
@@ -536,14 +540,6 @@ public class Nat464Xlat {
stop();
}
public void interfaceLinkStateChanged(String iface, boolean up) {
mNetwork.handler().post(() -> { handleInterfaceLinkStateChanged(iface, up); });
}
public void interfaceRemoved(String iface) {
mNetwork.handler().post(() -> handleInterfaceRemoved(iface));
}
/**
* Translate the input v4 address to v6 clat address.
*/

15
tests/unit/java/com/android/server/ConnectivityServiceTest.java
View File

@@ -10772,6 +10772,8 @@ public class ConnectivityServiceTest {
final RouteInfo ipv4Subnet = new RouteInfo(myIpv4, null, MOBILE_IFNAME);
final RouteInfo stackedDefault =
new RouteInfo((IpPrefix) null, myIpv4.getAddress(), CLAT_MOBILE_IFNAME);
final BaseNetdUnsolicitedEventListener netdUnsolicitedListener =
getRegisteredNetdUnsolicitedEventListener();
final NetworkRequest networkRequest = new NetworkRequest.Builder()
.addTransportType(TRANSPORT_CELLULAR)
@@ -10839,7 +10841,6 @@ public class ConnectivityServiceTest {
assertRoutesRemoved(cellNetId, ipv4Subnet);
// When NAT64 prefix discovery succeeds, LinkProperties are updated and clatd is started.
Nat464Xlat clat = getNat464Xlat(mCellAgent);
assertNull(mCm.getLinkProperties(mCellAgent.getNetwork()).getNat64Prefix());
mService.mResolverUnsolEventCallback.onNat64PrefixEvent(
makeNat64PrefixEvent(cellNetId, PREFIX_OPERATION_ADDED, kNat64PrefixString, 96));
@@ -10850,7 +10851,8 @@ public class ConnectivityServiceTest {
verifyClatdStart(null /* inOrder */, MOBILE_IFNAME, cellNetId, kNat64Prefix.toString());
// Clat iface comes up. Expect stacked link to be added.
clat.interfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true);
netdUnsolicitedListener.onInterfaceLinkStateChanged(
CLAT_MOBILE_IFNAME, true);
networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent);
List<LinkProperties> stackedLps = mCm.getLinkProperties(mCellAgent.getNetwork())
.getStackedLinks();
@@ -10896,7 +10898,7 @@ public class ConnectivityServiceTest {
kOtherNat64Prefix.toString());
networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent,
cb -> cb.getLp().getNat64Prefix().equals(kOtherNat64Prefix));
clat.interfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true);
netdUnsolicitedListener.onInterfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true);
networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent,
cb -> cb.getLp().getStackedLinks().size() == 1);
assertRoutesAdded(cellNetId, stackedDefault);
@@ -10924,7 +10926,7 @@ public class ConnectivityServiceTest {
assertRoutesRemoved(cellNetId, stackedDefault);
// The interface removed callback happens but has no effect after stop is called.
clat.interfaceRemoved(CLAT_MOBILE_IFNAME);
netdUnsolicitedListener.onInterfaceRemoved(CLAT_MOBILE_IFNAME);
networkCallback.assertNoCallback();
verify(mMockNetd, times(1)).networkRemoveInterface(cellNetId, CLAT_MOBILE_IFNAME);
@@ -10961,7 +10963,7 @@ public class ConnectivityServiceTest {
verifyClatdStart(null /* inOrder */, MOBILE_IFNAME, cellNetId, kNat64Prefix.toString());
// Clat iface comes up. Expect stacked link to be added.
clat.interfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true);
netdUnsolicitedListener.onInterfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true);
networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent,
cb -> cb.getLp().getStackedLinks().size() == 1
&& cb.getLp().getNat64Prefix() != null);
@@ -11029,8 +11031,7 @@ public class ConnectivityServiceTest {
// Clatd is started and clat iface comes up. Expect stacked link to be added.
verifyClatdStart(null /* inOrder */, MOBILE_IFNAME, cellNetId, kNat64Prefix.toString());
clat = getNat464Xlat(mCellAgent);
clat.interfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true /* up */);
netdUnsolicitedListener.onInterfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true /* up */);
networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent,
cb -> cb.getLp().getStackedLinks().size() == 1
&& cb.getLp().getNat64Prefix().equals(kNat64Prefix));

49
tests/unit/java/com/android/server/connectivity/Nat464XlatTest.java
View File

@@ -86,7 +86,6 @@ public class Nat464XlatTest {
@Mock ClatCoordinator mClatCoordinator;
TestLooper mLooper;
Handler mHandler;
NetworkAgentConfig mAgentConfig = new NetworkAgentConfig();
Nat464Xlat makeNat464Xlat(boolean isCellular464XlatEnabled) {
@@ -96,6 +95,14 @@ public class Nat464XlatTest {
}
};
// The test looper needs to be created here on the test case thread and not in setUp,
// because setUp and test cases are run in different threads. Creating the test looper in
// setUp would make Looper.getThread() return the setUp thread, which does not match the
// test case thread that is actually used to process the messages.
mLooper = new TestLooper();
final Handler handler = new Handler(mLooper.getLooper());
doReturn(handler).when(mNai).handler();
return new Nat464Xlat(mNai, mNetd, mDnsResolver, deps) {
@Override protected int getNetId() {
return NETID;
@@ -117,9 +124,6 @@ public class Nat464XlatTest {
@Before
public void setUp() throws Exception {
mLooper = new TestLooper();
mHandler = new Handler(mLooper.getLooper());
MockitoAnnotations.initMocks(this);
mNai.linkProperties = new LinkProperties();
@@ -130,7 +134,6 @@ public class Nat464XlatTest {
markNetworkConnected();
when(mNai.connService()).thenReturn(mConnectivity);
when(mNai.netAgentConfig()).thenReturn(mAgentConfig);
when(mNai.handler()).thenReturn(mHandler);
final InterfaceConfigurationParcel mConfig = new InterfaceConfigurationParcel();
when(mNetd.interfaceGetCfg(eq(STACKED_IFACE))).thenReturn(mConfig);
mConfig.ipv4Addr = ADDR.getAddress().getHostAddress();
@@ -272,8 +275,7 @@ public class Nat464XlatTest {
verifyClatdStart(null /* inOrder */);
// Stacked interface up notification arrives.
nat.interfaceLinkStateChanged(STACKED_IFACE, true);
mLooper.dispatchNext();
nat.handleInterfaceLinkStateChanged(STACKED_IFACE, true);
verify(mNetd).interfaceGetCfg(eq(STACKED_IFACE));
verify(mConnectivity).handleUpdateLinkProperties(eq(mNai), c.capture());
@@ -294,8 +296,7 @@ public class Nat464XlatTest {
// Verify the generated v6 is reset when clat is stopped.
assertNull(nat.mIPv6Address);
// Stacked interface removed notification arrives and is ignored.
nat.interfaceRemoved(STACKED_IFACE);
mLooper.dispatchNext();
nat.handleInterfaceRemoved(STACKED_IFACE);
verifyNoMoreInteractions(mNetd, mConnectivity);
}
@@ -324,8 +325,7 @@ public class Nat464XlatTest {
verifyClatdStart(inOrder);
// Stacked interface up notification arrives.
nat.interfaceLinkStateChanged(STACKED_IFACE, true);
mLooper.dispatchNext();
nat.handleInterfaceLinkStateChanged(STACKED_IFACE, true);
inOrder.verify(mConnectivity).handleUpdateLinkProperties(eq(mNai), c.capture());
assertFalse(c.getValue().getStackedLinks().isEmpty());
@@ -344,10 +344,8 @@ public class Nat464XlatTest {
if (interfaceRemovedFirst) {
// Stacked interface removed notification arrives and is ignored.
nat.interfaceRemoved(STACKED_IFACE);
mLooper.dispatchNext();
nat.interfaceLinkStateChanged(STACKED_IFACE, false);
mLooper.dispatchNext();
nat.handleInterfaceRemoved(STACKED_IFACE);
nat.handleInterfaceLinkStateChanged(STACKED_IFACE, false);
}
assertTrue(c.getValue().getStackedLinks().isEmpty());
@@ -361,15 +359,12 @@ public class Nat464XlatTest {
if (!interfaceRemovedFirst) {
// Stacked interface removed notification arrives and is ignored.
nat.interfaceRemoved(STACKED_IFACE);
mLooper.dispatchNext();
nat.interfaceLinkStateChanged(STACKED_IFACE, false);
mLooper.dispatchNext();
nat.handleInterfaceRemoved(STACKED_IFACE);
nat.handleInterfaceLinkStateChanged(STACKED_IFACE, false);
}
// Stacked interface up notification arrives.
nat.interfaceLinkStateChanged(STACKED_IFACE, true);
mLooper.dispatchNext();
nat.handleInterfaceLinkStateChanged(STACKED_IFACE, true);
inOrder.verify(mConnectivity).handleUpdateLinkProperties(eq(mNai), c.capture());
assertFalse(c.getValue().getStackedLinks().isEmpty());
@@ -411,8 +406,7 @@ public class Nat464XlatTest {
verifyClatdStart(null /* inOrder */);
// Stacked interface up notification arrives.
nat.interfaceLinkStateChanged(STACKED_IFACE, true);
mLooper.dispatchNext();
nat.handleInterfaceLinkStateChanged(STACKED_IFACE, true);
verify(mNetd).interfaceGetCfg(eq(STACKED_IFACE));
verify(mConnectivity, times(1)).handleUpdateLinkProperties(eq(mNai), c.capture());
@@ -421,8 +415,7 @@ public class Nat464XlatTest {
assertRunning(nat);
// Stacked interface removed notification arrives (clatd crashed, ...).
nat.interfaceRemoved(STACKED_IFACE);
mLooper.dispatchNext();
nat.handleInterfaceRemoved(STACKED_IFACE);
verifyClatdStop(null /* inOrder */);
verify(mConnectivity, times(2)).handleUpdateLinkProperties(eq(mNai), c.capture());
@@ -457,12 +450,10 @@ public class Nat464XlatTest {
assertIdle(nat);
// In-flight interface up notification arrives: no-op
nat.interfaceLinkStateChanged(STACKED_IFACE, true);
mLooper.dispatchNext();
nat.handleInterfaceLinkStateChanged(STACKED_IFACE, true);
// Interface removed notification arrives after stopClatd() takes effect: no-op.
nat.interfaceRemoved(STACKED_IFACE);
mLooper.dispatchNext();
nat.handleInterfaceRemoved(STACKED_IFACE);
assertIdle(nat);