From eb15a2d816a6591648f8c3433bbeb71dcd1f733c Mon Sep 17 00:00:00 2001 From: Hungming Chen Date: Sun, 16 Jan 2022 15:15:57 +0800 Subject: [PATCH] [CLATJ#27] Use ClatdCoordinator since T+ devices - For clatd start and stop, use ClatdCoordinator on T+ and Netd on S- - Fix the unit test for T+ and S- devices Note that mokito.verify(.., times(1)) is replaced by verify(..) because times(1) is the default and can be omitted. See verify in mockito/src/main/java/org/mockito/Mockito.java Note that this commit needs to be merged with aosp/1956072. Bug: 212345928 Test: atest FrameworksNetTests manual test 1. Connect to ipv6-only wifi. 2. Try IPv4 traffic. $ ping 8.8.8.8 3. Check bpf entries are added 4. Disconnect from ipv6-only wifi. 5. Check bpf entries are removed 6. testipv4.com shows 10/10 Change-Id: I7dfda6eec19de94e4258971effcd8a1210542473 --- .../android/server/ConnectivityService.java | 14 ++ .../server/connectivity/ClatCoordinator.java | 6 +- .../server/connectivity/Nat464Xlat.java | 36 +++-- .../server/ConnectivityServiceTest.java | 132 ++++++++++++++---- .../server/connectivity/Nat464XlatTest.java | 77 +++++++--- 5 files changed, 210 insertions(+), 55 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 48354382ec..f330dbf30e 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -258,6 +258,7 @@ import com.android.net.module.util.TcUtils; import com.android.net.module.util.netlink.InetDiagMessage; import com.android.server.connectivity.AutodestructReference; import com.android.server.connectivity.CarrierPrivilegeAuthenticator; +import com.android.server.connectivity.ClatCoordinator; import com.android.server.connectivity.ConnectivityFlags; import com.android.server.connectivity.DnsManager; import com.android.server.connectivity.DnsManager.PrivateDnsValidationUpdate; @@ -1404,6 +1405,19 @@ public class ConnectivityService extends IConnectivityManager.Stub return new BpfNetMaps(netd); } + /** + * @see ClatCoordinator + */ + public ClatCoordinator getClatCoordinator(INetd netd) { + return new ClatCoordinator( + new ClatCoordinator.Dependencies() { + @NonNull + public INetd getNetd() { + return netd; + } + }); + } + /** * Wraps {@link TcUtils#tcFilterAddDevIngressPolice} */ diff --git a/service/src/com/android/server/connectivity/ClatCoordinator.java b/service/src/com/android/server/connectivity/ClatCoordinator.java index 8aa5990584..5ca888cd6b 100644 --- a/service/src/com/android/server/connectivity/ClatCoordinator.java +++ b/service/src/com/android/server/connectivity/ClatCoordinator.java @@ -122,8 +122,12 @@ public class ClatCoordinator { @Nullable private ClatdTracker mClatdTracker = null; + /** + * Dependencies of ClatCoordinator which makes ConnectivityService injection + * in tests. + */ @VisibleForTesting - abstract static class Dependencies { + public abstract static class Dependencies { /** * Get netd. */ diff --git a/service/src/com/android/server/connectivity/Nat464Xlat.java b/service/src/com/android/server/connectivity/Nat464Xlat.java index 7b06682d11..35e02ca036 100644 --- a/service/src/com/android/server/connectivity/Nat464Xlat.java +++ b/service/src/com/android/server/connectivity/Nat464Xlat.java @@ -36,9 +36,11 @@ import android.os.ServiceSpecificException; import android.util.Log; import com.android.internal.annotations.VisibleForTesting; +import com.android.modules.utils.build.SdkLevel; import com.android.net.module.util.NetworkStackConstants; import com.android.server.ConnectivityService; +import java.io.IOException; import java.net.Inet6Address; import java.util.Objects; @@ -96,6 +98,7 @@ public class Nat464Xlat { private String mIface; private Inet6Address mIPv6Address; private State mState = State.IDLE; + private ClatCoordinator mClatCoordinator; private boolean mEnableClatOnCellular; private boolean mPrefixDiscoveryRunning; @@ -106,6 +109,7 @@ public class Nat464Xlat { mNetd = netd; mNetwork = nai; mEnableClatOnCellular = deps.getCellular464XlatEnabled(); + mClatCoordinator = deps.getClatCoordinator(mNetd); } /** @@ -179,10 +183,18 @@ public class Nat464Xlat { private void enterStartingState(String baseIface) { mNat64PrefixInUse = selectNat64Prefix(); String addrStr = null; - try { - addrStr = mNetd.clatdStart(baseIface, mNat64PrefixInUse.toString()); - } catch (RemoteException | ServiceSpecificException e) { - Log.e(TAG, "Error starting clatd on " + baseIface + ": " + e); + if (SdkLevel.isAtLeastT()) { + try { + addrStr = mClatCoordinator.clatStart(baseIface, getNetId(), mNat64PrefixInUse); + } catch (IOException e) { + Log.e(TAG, "Error starting clatd on " + baseIface + ": " + e); + } + } else { + try { + addrStr = mNetd.clatdStart(baseIface, mNat64PrefixInUse.toString()); + } catch (RemoteException | ServiceSpecificException e) { + Log.e(TAG, "Error starting clatd on " + baseIface + ": " + e); + } } mIface = CLAT_PREFIX + baseIface; mBaseIface = baseIface; @@ -256,10 +268,18 @@ public class Nat464Xlat { } Log.i(TAG, "Stopping clatd on " + mBaseIface); - try { - mNetd.clatdStop(mBaseIface); - } catch (RemoteException | ServiceSpecificException e) { - Log.e(TAG, "Error stopping clatd on " + mBaseIface + ": " + e); + if (SdkLevel.isAtLeastT()) { + try { + mClatCoordinator.clatStop(); + } catch (IOException e) { + Log.e(TAG, "Error stopping clatd on " + mBaseIface + ": " + e); + } + } else { + try { + mNetd.clatdStop(mBaseIface); + } catch (RemoteException | ServiceSpecificException e) { + Log.e(TAG, "Error stopping clatd on " + mBaseIface + ": " + e); + } } String iface = mIface; diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 4c768030f8..37df4eb1ab 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -159,8 +159,8 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import static org.junit.Assume.assumeTrue; import static org.junit.Assume.assumeFalse; +import static org.junit.Assume.assumeTrue; import static org.mockito.AdditionalMatchers.aryEq; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyLong; @@ -342,6 +342,7 @@ import com.android.server.ConnectivityService.ConnectivityDiagnosticsCallbackInf import com.android.server.ConnectivityService.NetworkRequestInfo; import com.android.server.ConnectivityServiceTest.ConnectivityServiceDependencies.ReportedInterfaces; import com.android.server.connectivity.CarrierPrivilegeAuthenticator; +import com.android.server.connectivity.ClatCoordinator; import com.android.server.connectivity.ConnectivityFlags; import com.android.server.connectivity.MockableSystemProperties; import com.android.server.connectivity.Nat464Xlat; @@ -540,6 +541,7 @@ public class ConnectivityServiceTest { @Mock VpnProfileStore mVpnProfileStore; @Mock SystemConfigManager mSystemConfigManager; @Mock Resources mResources; + @Mock ClatCoordinator mClatCoordinator; @Mock PacProxyManager mPacProxyManager; @Mock BpfNetMaps mBpfNetMaps; @Mock CarrierPrivilegeAuthenticator mCarrierPrivilegeAuthenticator; @@ -2004,6 +2006,11 @@ public class ConnectivityServiceTest { return mBpfNetMaps; } + @Override + public ClatCoordinator getClatCoordinator(INetd netd) { + return mClatCoordinator; + } + final ArrayTrackRecord> mRateLimitHistory = new ArrayTrackRecord<>(); final Map mActiveRateLimit = new HashMap<>(); @@ -9569,6 +9576,59 @@ public class ConnectivityServiceTest { return event; } + private T verifyWithOrder(@Nullable InOrder inOrder, @NonNull T t) { + if (inOrder != null) { + return inOrder.verify(t); + } else { + return verify(t); + } + } + + private T verifyNeverWithOrder(@Nullable InOrder inOrder, @NonNull T t) { + if (inOrder != null) { + return inOrder.verify(t, never()); + } else { + return verify(t, never()); + } + } + + private void verifyClatdStart(@Nullable InOrder inOrder, @NonNull String iface, int netId, + @NonNull String nat64Prefix) throws Exception { + if (SdkLevel.isAtLeastT()) { + verifyWithOrder(inOrder, mClatCoordinator) + .clatStart(eq(iface), eq(netId), eq(new IpPrefix(nat64Prefix))); + } else { + verifyWithOrder(inOrder, mMockNetd).clatdStart(eq(iface), eq(nat64Prefix)); + } + } + + private void verifyNeverClatdStart(@Nullable InOrder inOrder, @NonNull String iface) + throws Exception { + if (SdkLevel.isAtLeastT()) { + verifyNeverWithOrder(inOrder, mClatCoordinator).clatStart(eq(iface), anyInt(), any()); + } else { + verifyNeverWithOrder(inOrder, mMockNetd).clatdStart(eq(iface), anyString()); + } + } + + private void verifyClatdStop(@Nullable InOrder inOrder, @NonNull String iface) + throws Exception { + if (SdkLevel.isAtLeastT()) { + verifyWithOrder(inOrder, mClatCoordinator).clatStop(); + } else { + verifyWithOrder(inOrder, mMockNetd).clatdStop(eq(iface)); + } + } + + private void verifyNeverClatdStop(@Nullable InOrder inOrder, @NonNull String iface) + throws Exception { + if (SdkLevel.isAtLeastT()) { + verifyNeverWithOrder(inOrder, mClatCoordinator).clatStop(); + } else { + verifyNeverWithOrder(inOrder, mMockNetd).clatdStop(eq(iface)); + } + } + @Test public void testStackedLinkProperties() throws Exception { final LinkAddress myIpv4 = new LinkAddress("1.2.3.4/24"); @@ -9601,6 +9661,7 @@ public class ConnectivityServiceTest { mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR, cellLp); reset(mMockDnsResolver); reset(mMockNetd); + reset(mClatCoordinator); // Connect with ipv6 link properties. Expect prefix discovery to be started. mCellNetworkAgent.connect(true); @@ -9639,8 +9700,10 @@ public class ConnectivityServiceTest { && ri.iface != null && ri.iface.startsWith("v4-"))); verifyNoMoreInteractions(mMockNetd); + verifyNoMoreInteractions(mClatCoordinator); verifyNoMoreInteractions(mMockDnsResolver); reset(mMockNetd); + reset(mClatCoordinator); reset(mMockDnsResolver); doReturn(getClatInterfaceConfigParcel(myIpv4)).when(mMockNetd) .interfaceGetCfg(CLAT_MOBILE_IFNAME); @@ -9661,7 +9724,7 @@ public class ConnectivityServiceTest { CallbackEntry.LINK_PROPERTIES_CHANGED, mCellNetworkAgent).getLp(); assertEquals(0, lpBeforeClat.getStackedLinks().size()); assertEquals(kNat64Prefix, lpBeforeClat.getNat64Prefix()); - verify(mMockNetd, times(1)).clatdStart(MOBILE_IFNAME, kNat64Prefix.toString()); + verifyClatdStart(null /* inOrder */, MOBILE_IFNAME, cellNetId, kNat64Prefix.toString()); // Clat iface comes up. Expect stacked link to be added. clat.interfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true); @@ -9693,6 +9756,7 @@ public class ConnectivityServiceTest { new int[] { TRANSPORT_CELLULAR }))); } reset(mMockNetd); + reset(mClatCoordinator); doReturn(getClatInterfaceConfigParcel(myIpv4)).when(mMockNetd) .interfaceGetCfg(CLAT_MOBILE_IFNAME); // Change the NAT64 prefix without first removing it. @@ -9701,11 +9765,12 @@ public class ConnectivityServiceTest { cellNetId, PREFIX_OPERATION_ADDED, kOtherNat64PrefixString, 96)); networkCallback.expectLinkPropertiesThat(mCellNetworkAgent, (lp) -> lp.getStackedLinks().size() == 0); - verify(mMockNetd, times(1)).clatdStop(MOBILE_IFNAME); + verifyClatdStop(null /* inOrder */, MOBILE_IFNAME); assertRoutesRemoved(cellNetId, stackedDefault); verify(mMockNetd, times(1)).networkRemoveInterface(cellNetId, CLAT_MOBILE_IFNAME); - verify(mMockNetd, times(1)).clatdStart(MOBILE_IFNAME, kOtherNat64Prefix.toString()); + verifyClatdStart(null /* inOrder */, MOBILE_IFNAME, cellNetId, + kOtherNat64Prefix.toString()); networkCallback.expectLinkPropertiesThat(mCellNetworkAgent, (lp) -> lp.getNat64Prefix().equals(kOtherNat64Prefix)); clat.interfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true); @@ -9714,6 +9779,7 @@ public class ConnectivityServiceTest { assertRoutesAdded(cellNetId, stackedDefault); verify(mMockNetd, times(1)).networkAddInterface(cellNetId, CLAT_MOBILE_IFNAME); reset(mMockNetd); + reset(mClatCoordinator); // Add ipv4 address, expect that clatd and prefix discovery are stopped and stacked // linkproperties are cleaned up. @@ -9722,7 +9788,7 @@ public class ConnectivityServiceTest { mCellNetworkAgent.sendLinkProperties(cellLp); networkCallback.expectCallback(CallbackEntry.LINK_PROPERTIES_CHANGED, mCellNetworkAgent); assertRoutesAdded(cellNetId, ipv4Subnet); - verify(mMockNetd, times(1)).clatdStop(MOBILE_IFNAME); + verifyClatdStop(null /* inOrder */, MOBILE_IFNAME); verify(mMockDnsResolver, times(1)).stopPrefix64Discovery(cellNetId); // As soon as stop is called, the linkproperties lose the stacked interface. @@ -9739,8 +9805,10 @@ public class ConnectivityServiceTest { networkCallback.assertNoCallback(); verify(mMockNetd, times(1)).networkRemoveInterface(cellNetId, CLAT_MOBILE_IFNAME); verifyNoMoreInteractions(mMockNetd); + verifyNoMoreInteractions(mClatCoordinator); verifyNoMoreInteractions(mMockDnsResolver); reset(mMockNetd); + reset(mClatCoordinator); reset(mMockDnsResolver); doReturn(getClatInterfaceConfigParcel(myIpv4)).when(mMockNetd) .interfaceGetCfg(CLAT_MOBILE_IFNAME); @@ -9762,7 +9830,7 @@ public class ConnectivityServiceTest { mService.mResolverUnsolEventCallback.onNat64PrefixEvent(makeNat64PrefixEvent( cellNetId, PREFIX_OPERATION_ADDED, kNat64PrefixString, 96)); networkCallback.expectCallback(CallbackEntry.LINK_PROPERTIES_CHANGED, mCellNetworkAgent); - verify(mMockNetd, times(1)).clatdStart(MOBILE_IFNAME, kNat64Prefix.toString()); + verifyClatdStart(null /* inOrder */, MOBILE_IFNAME, cellNetId, kNat64Prefix.toString()); // Clat iface comes up. Expect stacked link to be added. clat.interfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true); @@ -9779,7 +9847,7 @@ public class ConnectivityServiceTest { assertRoutesRemoved(cellNetId, ipv4Subnet, stackedDefault); // Stop has no effect because clat is already stopped. - verify(mMockNetd, times(1)).clatdStop(MOBILE_IFNAME); + verifyClatdStop(null /* inOrder */, MOBILE_IFNAME); networkCallback.expectLinkPropertiesThat(mCellNetworkAgent, (lp) -> lp.getStackedLinks().size() == 0); verify(mMockNetd, times(1)).networkRemoveInterface(cellNetId, CLAT_MOBILE_IFNAME); @@ -9792,7 +9860,9 @@ public class ConnectivityServiceTest { eq(Integer.toString(TRANSPORT_CELLULAR))); verify(mMockNetd).networkDestroy(cellNetId); verifyNoMoreInteractions(mMockNetd); + verifyNoMoreInteractions(mClatCoordinator); reset(mMockNetd); + reset(mClatCoordinator); // Test disconnecting a network that is running 464xlat. @@ -9809,7 +9879,7 @@ public class ConnectivityServiceTest { assertRoutesAdded(cellNetId, ipv6Subnet, ipv6Default); // Clatd is started and clat iface comes up. Expect stacked link to be added. - verify(mMockNetd).clatdStart(MOBILE_IFNAME, kNat64Prefix.toString()); + verifyClatdStart(null /* inOrder */, MOBILE_IFNAME, cellNetId, kNat64Prefix.toString()); clat = getNat464Xlat(mCellNetworkAgent); clat.interfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true /* up */); networkCallback.expectLinkPropertiesThat(mCellNetworkAgent, @@ -9819,16 +9889,18 @@ public class ConnectivityServiceTest { // assertRoutesAdded sees all calls since last mMockNetd reset, so expect IPv6 routes again. assertRoutesAdded(cellNetId, ipv6Subnet, ipv6Default, stackedDefault); reset(mMockNetd); + reset(mClatCoordinator); // Disconnect the network. clat is stopped and the network is destroyed. mCellNetworkAgent.disconnect(); networkCallback.expectCallback(CallbackEntry.LOST, mCellNetworkAgent); networkCallback.assertNoCallback(); - verify(mMockNetd).clatdStop(MOBILE_IFNAME); + verifyClatdStop(null /* inOrder */, MOBILE_IFNAME); verify(mMockNetd).idletimerRemoveInterface(eq(MOBILE_IFNAME), anyInt(), eq(Integer.toString(TRANSPORT_CELLULAR))); verify(mMockNetd).networkDestroy(cellNetId); verifyNoMoreInteractions(mMockNetd); + verifyNoMoreInteractions(mClatCoordinator); mCm.unregisterNetworkCallback(networkCallback); } @@ -9859,7 +9931,7 @@ public class ConnectivityServiceTest { baseLp.addDnsServer(InetAddress.getByName("2001:4860:4860::6464")); reset(mMockNetd, mMockDnsResolver); - InOrder inOrder = inOrder(mMockNetd, mMockDnsResolver); + InOrder inOrder = inOrder(mMockNetd, mMockDnsResolver, mClatCoordinator); // If a network already has a NAT64 prefix on connect, clatd is started immediately and // prefix discovery is never started. @@ -9870,7 +9942,7 @@ public class ConnectivityServiceTest { final Network network = mWiFiNetworkAgent.getNetwork(); int netId = network.getNetId(); callback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); - inOrder.verify(mMockNetd).clatdStart(iface, pref64FromRa.toString()); + verifyClatdStart(inOrder, iface, netId, pref64FromRa.toString()); inOrder.verify(mMockDnsResolver).setPrefix64(netId, pref64FromRa.toString()); inOrder.verify(mMockDnsResolver, never()).startPrefix64Discovery(netId); callback.assertNoCallback(); @@ -9880,7 +9952,7 @@ public class ConnectivityServiceTest { lp.setNat64Prefix(null); mWiFiNetworkAgent.sendLinkProperties(lp); expectNat64PrefixChange(callback, mWiFiNetworkAgent, null); - inOrder.verify(mMockNetd).clatdStop(iface); + verifyClatdStop(inOrder, iface); inOrder.verify(mMockDnsResolver).setPrefix64(netId, ""); inOrder.verify(mMockDnsResolver).startPrefix64Discovery(netId); @@ -9889,7 +9961,7 @@ public class ConnectivityServiceTest { lp.setNat64Prefix(pref64FromRa); mWiFiNetworkAgent.sendLinkProperties(lp); expectNat64PrefixChange(callback, mWiFiNetworkAgent, pref64FromRa); - inOrder.verify(mMockNetd).clatdStart(iface, pref64FromRa.toString()); + verifyClatdStart(inOrder, iface, netId, pref64FromRa.toString()); inOrder.verify(mMockDnsResolver).stopPrefix64Discovery(netId); inOrder.verify(mMockDnsResolver).setPrefix64(netId, pref64FromRa.toString()); @@ -9898,22 +9970,22 @@ public class ConnectivityServiceTest { lp.setNat64Prefix(null); mWiFiNetworkAgent.sendLinkProperties(lp); expectNat64PrefixChange(callback, mWiFiNetworkAgent, null); - inOrder.verify(mMockNetd).clatdStop(iface); + verifyClatdStop(inOrder, iface); inOrder.verify(mMockDnsResolver).setPrefix64(netId, ""); inOrder.verify(mMockDnsResolver).startPrefix64Discovery(netId); mService.mResolverUnsolEventCallback.onNat64PrefixEvent( makeNat64PrefixEvent(netId, PREFIX_OPERATION_ADDED, pref64FromDnsStr, 96)); expectNat64PrefixChange(callback, mWiFiNetworkAgent, pref64FromDns); - inOrder.verify(mMockNetd).clatdStart(iface, pref64FromDns.toString()); + verifyClatdStart(inOrder, iface, netId, pref64FromDns.toString()); // If an RA advertises the same prefix that was discovered by DNS, nothing happens: prefix // discovery is not stopped, and there are no callbacks. lp.setNat64Prefix(pref64FromDns); mWiFiNetworkAgent.sendLinkProperties(lp); callback.assertNoCallback(); - inOrder.verify(mMockNetd, never()).clatdStop(iface); - inOrder.verify(mMockNetd, never()).clatdStart(eq(iface), anyString()); + verifyNeverClatdStop(inOrder, iface); + verifyNeverClatdStart(inOrder, iface); inOrder.verify(mMockDnsResolver, never()).stopPrefix64Discovery(netId); inOrder.verify(mMockDnsResolver, never()).startPrefix64Discovery(netId); inOrder.verify(mMockDnsResolver, never()).setPrefix64(eq(netId), anyString()); @@ -9922,8 +9994,8 @@ public class ConnectivityServiceTest { lp.setNat64Prefix(null); mWiFiNetworkAgent.sendLinkProperties(lp); callback.assertNoCallback(); - inOrder.verify(mMockNetd, never()).clatdStop(iface); - inOrder.verify(mMockNetd, never()).clatdStart(eq(iface), anyString()); + verifyNeverClatdStop(inOrder, iface); + verifyNeverClatdStart(inOrder, iface); inOrder.verify(mMockDnsResolver, never()).stopPrefix64Discovery(netId); inOrder.verify(mMockDnsResolver, never()).startPrefix64Discovery(netId); inOrder.verify(mMockDnsResolver, never()).setPrefix64(eq(netId), anyString()); @@ -9932,14 +10004,14 @@ public class ConnectivityServiceTest { lp.setNat64Prefix(pref64FromRa); mWiFiNetworkAgent.sendLinkProperties(lp); expectNat64PrefixChange(callback, mWiFiNetworkAgent, pref64FromRa); - inOrder.verify(mMockNetd).clatdStop(iface); + verifyClatdStop(inOrder, iface); inOrder.verify(mMockDnsResolver).stopPrefix64Discovery(netId); // Stopping prefix discovery results in a prefix removed notification. mService.mResolverUnsolEventCallback.onNat64PrefixEvent( makeNat64PrefixEvent(netId, PREFIX_OPERATION_REMOVED, pref64FromDnsStr, 96)); - inOrder.verify(mMockNetd).clatdStart(iface, pref64FromRa.toString()); + verifyClatdStart(inOrder, iface, netId, pref64FromRa.toString()); inOrder.verify(mMockDnsResolver).setPrefix64(netId, pref64FromRa.toString()); inOrder.verify(mMockDnsResolver, never()).startPrefix64Discovery(netId); @@ -9947,9 +10019,9 @@ public class ConnectivityServiceTest { lp.setNat64Prefix(newPref64FromRa); mWiFiNetworkAgent.sendLinkProperties(lp); expectNat64PrefixChange(callback, mWiFiNetworkAgent, newPref64FromRa); - inOrder.verify(mMockNetd).clatdStop(iface); + verifyClatdStop(inOrder, iface); inOrder.verify(mMockDnsResolver).setPrefix64(netId, ""); - inOrder.verify(mMockNetd).clatdStart(iface, newPref64FromRa.toString()); + verifyClatdStart(inOrder, iface, netId, newPref64FromRa.toString()); inOrder.verify(mMockDnsResolver).setPrefix64(netId, newPref64FromRa.toString()); inOrder.verify(mMockDnsResolver, never()).stopPrefix64Discovery(netId); inOrder.verify(mMockDnsResolver, never()).startPrefix64Discovery(netId); @@ -9959,8 +10031,8 @@ public class ConnectivityServiceTest { mWiFiNetworkAgent.sendLinkProperties(lp); callback.assertNoCallback(); assertEquals(newPref64FromRa, mCm.getLinkProperties(network).getNat64Prefix()); - inOrder.verify(mMockNetd, never()).clatdStop(iface); - inOrder.verify(mMockNetd, never()).clatdStart(eq(iface), anyString()); + verifyNeverClatdStop(inOrder, iface); + verifyNeverClatdStart(inOrder, iface); inOrder.verify(mMockDnsResolver, never()).stopPrefix64Discovery(netId); inOrder.verify(mMockDnsResolver, never()).startPrefix64Discovery(netId); inOrder.verify(mMockDnsResolver, never()).setPrefix64(eq(netId), anyString()); @@ -9972,20 +10044,20 @@ public class ConnectivityServiceTest { lp.setNat64Prefix(null); mWiFiNetworkAgent.sendLinkProperties(lp); expectNat64PrefixChange(callback, mWiFiNetworkAgent, null); - inOrder.verify(mMockNetd).clatdStop(iface); + verifyClatdStop(inOrder, iface); inOrder.verify(mMockDnsResolver).setPrefix64(netId, ""); inOrder.verify(mMockDnsResolver).startPrefix64Discovery(netId); mService.mResolverUnsolEventCallback.onNat64PrefixEvent( makeNat64PrefixEvent(netId, PREFIX_OPERATION_ADDED, pref64FromDnsStr, 96)); expectNat64PrefixChange(callback, mWiFiNetworkAgent, pref64FromDns); - inOrder.verify(mMockNetd).clatdStart(iface, pref64FromDns.toString()); + verifyClatdStart(inOrder, iface, netId, pref64FromDns.toString()); inOrder.verify(mMockDnsResolver, never()).setPrefix64(eq(netId), any()); lp.setNat64Prefix(pref64FromDns); mWiFiNetworkAgent.sendLinkProperties(lp); callback.assertNoCallback(); - inOrder.verify(mMockNetd, never()).clatdStop(iface); - inOrder.verify(mMockNetd, never()).clatdStart(eq(iface), anyString()); + verifyNeverClatdStop(inOrder, iface); + verifyNeverClatdStart(inOrder, iface); inOrder.verify(mMockDnsResolver, never()).stopPrefix64Discovery(netId); inOrder.verify(mMockDnsResolver, never()).startPrefix64Discovery(netId); inOrder.verify(mMockDnsResolver, never()).setPrefix64(eq(netId), anyString()); @@ -9998,7 +10070,7 @@ public class ConnectivityServiceTest { callback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent); b.expectBroadcast(); - inOrder.verify(mMockNetd).clatdStop(iface); + verifyClatdStop(inOrder, iface); inOrder.verify(mMockDnsResolver).stopPrefix64Discovery(netId); inOrder.verify(mMockDnsResolver, never()).setPrefix64(eq(netId), anyString()); diff --git a/tests/unit/java/com/android/server/connectivity/Nat464XlatTest.java b/tests/unit/java/com/android/server/connectivity/Nat464XlatTest.java index aa4c4e37f5..06e0d6df3c 100644 --- a/tests/unit/java/com/android/server/connectivity/Nat464XlatTest.java +++ b/tests/unit/java/com/android/server/connectivity/Nat464XlatTest.java @@ -21,6 +21,8 @@ import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyInt; import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.inOrder; @@ -44,8 +46,11 @@ import android.os.Build; import android.os.Handler; import android.os.test.TestLooper; +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import androidx.test.filters.SmallTest; +import com.android.modules.utils.build.SdkLevel; import com.android.server.ConnectivityService; import com.android.testutils.DevSdkIgnoreRule; import com.android.testutils.DevSdkIgnoreRunner; @@ -75,13 +80,20 @@ public class Nat464XlatTest { @Mock IDnsResolver mDnsResolver; @Mock INetd mNetd; @Mock NetworkAgentInfo mNai; + @Mock ClatCoordinator mClatCoordinator; TestLooper mLooper; Handler mHandler; NetworkAgentConfig mAgentConfig = new NetworkAgentConfig(); Nat464Xlat makeNat464Xlat(boolean isCellular464XlatEnabled) { - return new Nat464Xlat(mNai, mNetd, mDnsResolver, new ConnectivityService.Dependencies()) { + final ConnectivityService.Dependencies deps = new ConnectivityService.Dependencies() { + @Override public ClatCoordinator getClatCoordinator(INetd netd) { + return mClatCoordinator; + } + }; + + return new Nat464Xlat(mNai, mNetd, mDnsResolver, deps) { @Override protected int getNetId() { return NETID; } @@ -208,6 +220,39 @@ public class Nat464XlatTest { } } + private T verifyWithOrder(@Nullable InOrder inOrder, @NonNull T t) { + if (inOrder != null) { + return inOrder.verify(t); + } else { + return verify(t); + } + } + + private void verifyClatdStart(@Nullable InOrder inOrder) throws Exception { + if (SdkLevel.isAtLeastT()) { + verifyWithOrder(inOrder, mClatCoordinator) + .clatStart(eq(BASE_IFACE), eq(NETID), eq(new IpPrefix(NAT64_PREFIX))); + } else { + verifyWithOrder(inOrder, mNetd).clatdStart(eq(BASE_IFACE), eq(NAT64_PREFIX)); + } + } + + private void verifyNeverClatdStart() throws Exception { + if (SdkLevel.isAtLeastT()) { + verify(mClatCoordinator, never()).clatStart(anyString(), anyInt(), any()); + } else { + verify(mNetd, never()).clatdStart(anyString(), anyString()); + } + } + + private void verifyClatdStop(@Nullable InOrder inOrder) throws Exception { + if (SdkLevel.isAtLeastT()) { + verifyWithOrder(inOrder, mClatCoordinator).clatStop(); + } else { + verifyWithOrder(inOrder, mNetd).clatdStop(eq(BASE_IFACE)); + } + } + private void checkNormalStartAndStop(boolean dueToDisconnect) throws Exception { Nat464Xlat nat = makeNat464Xlat(true); ArgumentCaptor c = ArgumentCaptor.forClass(LinkProperties.class); @@ -219,7 +264,7 @@ public class Nat464XlatTest { // Start clat. nat.start(); - verify(mNetd).clatdStart(eq(BASE_IFACE), eq(NAT64_PREFIX)); + verifyClatdStart(null /* inOrder */); // Stacked interface up notification arrives. nat.interfaceLinkStateChanged(STACKED_IFACE, true); @@ -235,7 +280,7 @@ public class Nat464XlatTest { makeClatUnnecessary(dueToDisconnect); nat.stop(); - verify(mNetd).clatdStop(eq(BASE_IFACE)); + verifyClatdStop(null /* inOrder */); verify(mConnectivity, times(2)).handleUpdateLinkProperties(eq(mNai), c.capture()); assertTrue(c.getValue().getStackedLinks().isEmpty()); assertFalse(c.getValue().getAllInterfaceNames().contains(STACKED_IFACE)); @@ -262,7 +307,7 @@ public class Nat464XlatTest { private void checkStartStopStart(boolean interfaceRemovedFirst) throws Exception { Nat464Xlat nat = makeNat464Xlat(true); ArgumentCaptor c = ArgumentCaptor.forClass(LinkProperties.class); - InOrder inOrder = inOrder(mNetd, mConnectivity); + InOrder inOrder = inOrder(mNetd, mConnectivity, mClatCoordinator); mNai.linkProperties.addLinkAddress(V6ADDR); @@ -270,7 +315,7 @@ public class Nat464XlatTest { nat.start(); - inOrder.verify(mNetd).clatdStart(eq(BASE_IFACE), eq(NAT64_PREFIX)); + verifyClatdStart(inOrder); // Stacked interface up notification arrives. nat.interfaceLinkStateChanged(STACKED_IFACE, true); @@ -284,7 +329,7 @@ public class Nat464XlatTest { // ConnectivityService stops clat (Network disconnects, IPv4 addr appears, ...). nat.stop(); - inOrder.verify(mNetd).clatdStop(eq(BASE_IFACE)); + verifyClatdStop(inOrder); inOrder.verify(mConnectivity, times(1)).handleUpdateLinkProperties(eq(mNai), c.capture()); assertTrue(c.getValue().getStackedLinks().isEmpty()); @@ -306,7 +351,7 @@ public class Nat464XlatTest { nat.start(); - inOrder.verify(mNetd).clatdStart(eq(BASE_IFACE), eq(NAT64_PREFIX)); + verifyClatdStart(inOrder); if (!interfaceRemovedFirst) { // Stacked interface removed notification arrives and is ignored. @@ -328,7 +373,7 @@ public class Nat464XlatTest { // ConnectivityService stops clat again. nat.stop(); - inOrder.verify(mNetd).clatdStop(eq(BASE_IFACE)); + verifyClatdStop(inOrder); inOrder.verify(mConnectivity, times(1)).handleUpdateLinkProperties(eq(mNai), c.capture()); assertTrue(c.getValue().getStackedLinks().isEmpty()); @@ -357,7 +402,7 @@ public class Nat464XlatTest { nat.start(); - verify(mNetd).clatdStart(eq(BASE_IFACE), eq(NAT64_PREFIX)); + verifyClatdStart(null /* inOrder */); // Stacked interface up notification arrives. nat.interfaceLinkStateChanged(STACKED_IFACE, true); @@ -373,7 +418,7 @@ public class Nat464XlatTest { nat.interfaceRemoved(STACKED_IFACE); mLooper.dispatchNext(); - verify(mNetd).clatdStop(eq(BASE_IFACE)); + verifyClatdStop(null /* inOrder */); verify(mConnectivity, times(2)).handleUpdateLinkProperties(eq(mNai), c.capture()); verify(mDnsResolver).stopPrefix64Discovery(eq(NETID)); assertTrue(c.getValue().getStackedLinks().isEmpty()); @@ -395,13 +440,13 @@ public class Nat464XlatTest { nat.start(); - verify(mNetd).clatdStart(eq(BASE_IFACE), eq(NAT64_PREFIX)); + verifyClatdStart(null /* inOrder */); // ConnectivityService immediately stops clat (Network disconnects, IPv4 addr appears, ...) makeClatUnnecessary(dueToDisconnect); nat.stop(); - verify(mNetd).clatdStop(eq(BASE_IFACE)); + verifyClatdStop(null /* inOrder */); verify(mDnsResolver).stopPrefix64Discovery(eq(NETID)); assertIdle(nat); @@ -437,13 +482,13 @@ public class Nat464XlatTest { nat.start(); - verify(mNetd).clatdStart(eq(BASE_IFACE), eq(NAT64_PREFIX)); + verifyClatdStart(null /* inOrder */); // ConnectivityService immediately stops clat (Network disconnects, IPv4 addr appears, ...) makeClatUnnecessary(dueToDisconnect); nat.stop(); - verify(mNetd).clatdStop(eq(BASE_IFACE)); + verifyClatdStop(null /* inOrder */); verify(mDnsResolver).stopPrefix64Discovery(eq(NETID)); assertIdle(nat); @@ -518,7 +563,7 @@ public class Nat464XlatTest { mNai.linkProperties.setNat64Prefix(nat64Prefix); nat.setNat64PrefixFromRa(nat64Prefix); nat.update(); - verify(mNetd, never()).clatdStart(anyString(), anyString()); + verifyNeverClatdStart(); assertIdle(nat); } else { // Prefix discovery is started. @@ -529,7 +574,7 @@ public class Nat464XlatTest { mNai.linkProperties.setNat64Prefix(nat64Prefix); nat.setNat64PrefixFromRa(nat64Prefix); nat.update(); - verify(mNetd).clatdStart(BASE_IFACE, NAT64_PREFIX); + verifyClatdStart(null /* inOrder */); assertStarting(nat); } }