From 4308bfc4f05af28d07576ae21ac57679da3a0953 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Mon, 20 Apr 2020 11:37:18 +0000 Subject: [PATCH 1/4] Support learning the NAT64 prefix from two different sources. The NAT64 prefix from the RA always takes precedence over the NAT64 prefix from DNS discovery, because it is detected faster, and detecting it does not require sending any packets. Bug: 150648313 Test: new unit test Merged-In: Ic7452431d2d9aea1ae59b67a9d8383c6cc5b3902 Change-Id: Ic7452431d2d9aea1ae59b67a9d8383c6cc5b3902 --- .../android/server/ConnectivityService.java | 1 + .../server/connectivity/Nat464Xlat.java | 69 +++++++++----- .../server/ConnectivityServiceTest.java | 89 +++++++++++++++++++ .../server/connectivity/Nat464XlatTest.java | 44 ++++++++- 4 files changed, 179 insertions(+), 24 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index f1ea5d0184..552331ede9 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -5889,6 +5889,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private void processLinkPropertiesFromAgent(NetworkAgentInfo nai, LinkProperties lp) { lp.ensureDirectlyConnectedRoutes(); + nai.clatd.setNat64PrefixFromRa(lp.getNat64Prefix()); } private void updateLinkProperties(NetworkAgentInfo networkAgent, LinkProperties newLp, diff --git a/services/core/java/com/android/server/connectivity/Nat464Xlat.java b/services/core/java/com/android/server/connectivity/Nat464Xlat.java index e6b2d2678f..8ad21966cb 100644 --- a/services/core/java/com/android/server/connectivity/Nat464Xlat.java +++ b/services/core/java/com/android/server/connectivity/Nat464Xlat.java @@ -81,15 +81,23 @@ public class Nat464Xlat extends BaseNetworkObserver { RUNNING, // start() called, and the stacked iface is known to be up. } - /** NAT64 prefix currently in use. Only valid in STARTING or RUNNING states. */ + /** + * NAT64 prefix currently in use. Only valid in STARTING or RUNNING states. + * Used, among other things, to avoid updates when switching from a prefix learned from one + * source (e.g., RA) to the same prefix learned from another source (e.g., RA). + */ private IpPrefix mNat64PrefixInUse; /** NAT64 prefix (if any) discovered from DNS via RFC 7050. */ private IpPrefix mNat64PrefixFromDns; + /** NAT64 prefix (if any) learned from the network via RA. */ + private IpPrefix mNat64PrefixFromRa; private String mBaseIface; private String mIface; private Inet6Address mIPv6Address; private State mState = State.IDLE; + private boolean mPrefixDiscoveryRunning; + public Nat464Xlat(NetworkAgentInfo nai, INetd netd, IDnsResolver dnsResolver, INetworkManagementService nmService) { mDnsResolver = dnsResolver; @@ -138,15 +146,6 @@ public class Nat464Xlat extends BaseNetworkObserver { return requiresClat(nai) && lp != null && lp.getNat64Prefix() != null; } - /** - * @return true if we have started prefix discovery and not yet stopped it (regardless of - * whether it is still running or has succeeded). - * A true result corresponds to internal states DISCOVERING, STARTING and RUNNING. - */ - public boolean isPrefixDiscoveryStarted() { - return mState == State.DISCOVERING || isStarted(); - } - /** * @return true if clatd has been started and has not yet stopped. * A true result corresponds to internal states STARTING and RUNNING. @@ -181,7 +180,7 @@ public class Nat464Xlat extends BaseNetworkObserver { return; } - mNat64PrefixInUse = getNat64Prefix(); + mNat64PrefixInUse = selectNat64Prefix(); String addrStr = null; try { addrStr = mNetd.clatdStart(baseIface, mNat64PrefixInUse.toString()); @@ -218,8 +217,14 @@ public class Nat464Xlat extends BaseNetworkObserver { mNat64PrefixInUse = null; mIface = null; mBaseIface = null; - if (requiresClat(mNetwork)) { - mState = State.DISCOVERING; + + if (isPrefixDiscoveryNeeded()) { + if (!mPrefixDiscoveryRunning) { + startPrefixDiscovery(); + } else { + // Prefix discovery is already running. Nothing to do. + mState = State.DISCOVERING; + } } else { stopPrefixDiscovery(); // Enters IDLE state. } @@ -283,6 +288,7 @@ public class Nat464Xlat extends BaseNetworkObserver { Slog.e(TAG, "Error starting prefix discovery on netId " + getNetId() + ": " + e); } mState = State.DISCOVERING; + mPrefixDiscoveryRunning = true; } private void stopPrefixDiscovery() { @@ -292,10 +298,18 @@ public class Nat464Xlat extends BaseNetworkObserver { Slog.e(TAG, "Error stopping prefix discovery on netId " + getNetId() + ": " + e); } mState = State.IDLE; + mPrefixDiscoveryRunning = false; + } + + private boolean isPrefixDiscoveryNeeded() { + // If there is no NAT64 prefix in the RA, prefix discovery is always needed. It cannot be + // stopped after it succeeds, because stopping it will cause netd to report that the prefix + // has been removed, and that will cause us to stop clatd. + return requiresClat(mNetwork) && mNat64PrefixFromRa == null; } private void maybeHandleNat64PrefixChange() { - final IpPrefix newPrefix = getNat64Prefix(); + final IpPrefix newPrefix = selectNat64Prefix(); if (!Objects.equals(mNat64PrefixInUse, newPrefix)) { Slog.d(TAG, "NAT64 prefix changed from " + mNat64PrefixInUse + " to " + newPrefix); @@ -314,13 +328,10 @@ public class Nat464Xlat extends BaseNetworkObserver { // TODO: turn this class into a proper StateMachine. http://b/126113090 switch (mState) { case IDLE: - if (requiresClat(mNetwork)) { - // Network is detected to be IPv6-only. - // TODO: consider going to STARTING directly if the NAT64 prefix is already - // known. This would however result in clatd running without prefix discovery - // running, which might be a surprising combination. + if (isPrefixDiscoveryNeeded()) { startPrefixDiscovery(); // Enters DISCOVERING state. - return; + } else if (requiresClat(mNetwork)) { + start(); // Enters STARTING state. } break; @@ -351,8 +362,20 @@ public class Nat464Xlat extends BaseNetworkObserver { } } - private IpPrefix getNat64Prefix() { - return mNat64PrefixFromDns; + /** + * Picks a NAT64 prefix to use. Always prefers the prefix from the RA if one is received from + * both RA and DNS, because the prefix in the RA has better security and updatability, and will + * almost always be received first anyway. + * + * Any network that supports legacy hosts will support discovering the DNS64 prefix via DNS as + * well. If the prefix from the RA is withdrawn, fall back to that for reliability purposes. + */ + private IpPrefix selectNat64Prefix() { + return mNat64PrefixFromRa != null ? mNat64PrefixFromRa : mNat64PrefixFromDns; + } + + public void setNat64PrefixFromRa(IpPrefix prefix) { + mNat64PrefixFromRa = prefix; } public void setNat64PrefixFromDns(IpPrefix prefix) { @@ -367,7 +390,7 @@ public class Nat464Xlat extends BaseNetworkObserver { public void fixupLinkProperties(@NonNull LinkProperties oldLp, @NonNull LinkProperties lp) { // This must be done even if clatd is not running, because otherwise shouldStartClat would // never return true. - lp.setNat64Prefix(getNat64Prefix()); + lp.setNat64Prefix(selectNat64Prefix()); if (!isRunning()) { return; diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index dad0363a80..c1e51dd62a 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -6179,6 +6179,95 @@ public class ConnectivityServiceTest { mCm.unregisterNetworkCallback(networkCallback); } + private void expectNat64PrefixChange(TestableNetworkCallback callback, + TestNetworkAgentWrapper agent, IpPrefix prefix) { + callback.expectLinkPropertiesThat(agent, x -> Objects.equals(x.getNat64Prefix(), prefix)); + } + + @Test + public void testNat64PrefixMultipleSources() throws Exception { + final String iface = "wlan0"; + final String pref64FromRaStr = "64:ff9b::"; + final String pref64FromDnsStr = "2001:db8:64::"; + final IpPrefix pref64FromRa = new IpPrefix(InetAddress.getByName(pref64FromRaStr), 96); + final IpPrefix pref64FromDns = new IpPrefix(InetAddress.getByName(pref64FromDnsStr), 96); + final IpPrefix newPref64FromRa = new IpPrefix("2001:db8:64:64:64:64::/96"); + + final NetworkRequest request = new NetworkRequest.Builder() + .addCapability(NET_CAPABILITY_INTERNET) + .build(); + final TestNetworkCallback callback = new TestNetworkCallback(); + mCm.registerNetworkCallback(request, callback); + + final LinkProperties baseLp = new LinkProperties(); + baseLp.setInterfaceName(iface); + baseLp.addLinkAddress(new LinkAddress("2001:db8:1::1/64")); + baseLp.addDnsServer(InetAddress.getByName("2001:4860:4860::6464")); + + reset(mMockNetd, mMockDnsResolver); + InOrder inOrder = inOrder(mMockNetd, mMockDnsResolver); + + // If a network already has a NAT64 prefix on connect, clatd is started immediately and + // prefix discovery is never started. + LinkProperties lp = new LinkProperties(baseLp); + lp.setNat64Prefix(pref64FromRa); + mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI, lp); + mCellNetworkAgent.connect(false); + final Network network = mCellNetworkAgent.getNetwork(); + int netId = network.getNetId(); + callback.expectAvailableCallbacksUnvalidated(mCellNetworkAgent); + inOrder.verify(mMockNetd).clatdStart(iface, pref64FromRa.toString()); + inOrder.verify(mMockDnsResolver, never()).startPrefix64Discovery(netId); + callback.assertNoCallback(); + assertEquals(pref64FromRa, mCm.getLinkProperties(network).getNat64Prefix()); + + // If the RA prefix is withdrawn, clatd is stopped and prefix discovery is started. + lp.setNat64Prefix(null); + mCellNetworkAgent.sendLinkProperties(lp); + expectNat64PrefixChange(callback, mCellNetworkAgent, null); + inOrder.verify(mMockNetd).clatdStop(iface); + inOrder.verify(mMockDnsResolver).startPrefix64Discovery(netId); + + mService.mNetdEventCallback.onNat64PrefixEvent(netId, true /* added */, + pref64FromDnsStr, 96); + expectNat64PrefixChange(callback, mCellNetworkAgent, pref64FromDns); + inOrder.verify(mMockNetd).clatdStart(iface, pref64FromDns.toString()); + + // If the RA prefix reappears, clatd is restarted and prefix discovery is stopped. + lp.setNat64Prefix(pref64FromRa); + mCellNetworkAgent.sendLinkProperties(lp); + expectNat64PrefixChange(callback, mCellNetworkAgent, pref64FromRa); + inOrder.verify(mMockNetd).clatdStop(iface); + inOrder.verify(mMockDnsResolver).stopPrefix64Discovery(netId); + inOrder.verify(mMockNetd).clatdStart(iface, pref64FromRa.toString()); + inOrder.verify(mMockDnsResolver, never()).startPrefix64Discovery(netId); + + // If the RA prefix changes, clatd is restarted and prefix discovery is not started. + lp.setNat64Prefix(newPref64FromRa); + mCellNetworkAgent.sendLinkProperties(lp); + expectNat64PrefixChange(callback, mCellNetworkAgent, newPref64FromRa); + inOrder.verify(mMockNetd).clatdStop(iface); + inOrder.verify(mMockNetd).clatdStart(iface, newPref64FromRa.toString()); + inOrder.verify(mMockDnsResolver, never()).stopPrefix64Discovery(netId); + inOrder.verify(mMockDnsResolver, never()).startPrefix64Discovery(netId); + + // If the RA prefix changes to the same value, nothing happens. + lp.setNat64Prefix(newPref64FromRa); + mCellNetworkAgent.sendLinkProperties(lp); + callback.assertNoCallback(); + assertEquals(newPref64FromRa, mCm.getLinkProperties(network).getNat64Prefix()); + inOrder.verify(mMockNetd, never()).clatdStop(iface); + inOrder.verify(mMockNetd, never()).clatdStart(eq(iface), anyString()); + inOrder.verify(mMockDnsResolver, never()).stopPrefix64Discovery(netId); + inOrder.verify(mMockDnsResolver, never()).startPrefix64Discovery(netId); + + // The transition between no prefix and DNS prefix is tested in testStackedLinkProperties. + + callback.assertNoCallback(); + mCellNetworkAgent.disconnect(); + mCm.unregisterNetworkCallback(callback); + } + @Test public void testDataActivityTracking() throws Exception { final TestNetworkCallback networkCallback = new TestNetworkCallback(); diff --git a/tests/net/java/com/android/server/connectivity/Nat464XlatTest.java b/tests/net/java/com/android/server/connectivity/Nat464XlatTest.java index d0ebb5283f..84dad981e0 100644 --- a/tests/net/java/com/android/server/connectivity/Nat464XlatTest.java +++ b/tests/net/java/com/android/server/connectivity/Nat464XlatTest.java @@ -60,6 +60,7 @@ public class Nat464XlatTest { static final String STACKED_IFACE = "v4-test0"; static final LinkAddress ADDR = new LinkAddress("192.0.2.5/29"); static final String NAT64_PREFIX = "64:ff9b::/96"; + static final String OTHER_NAT64_PREFIX = "2001:db8:0:64::/96"; static final int NETID = 42; @Mock ConnectivityService mConnectivity; @@ -139,7 +140,7 @@ public class Nat464XlatTest { for (NetworkInfo.DetailedState state : supportedDetailedStates) { mNai.networkInfo.setDetailedState(state, "reason", "extraInfo"); - mNai.linkProperties.setNat64Prefix(new IpPrefix("2001:db8:0:64::/96")); + mNai.linkProperties.setNat64Prefix(new IpPrefix(OTHER_NAT64_PREFIX)); assertRequiresClat(false, mNai); assertShouldStartClat(false, mNai); @@ -398,6 +399,47 @@ public class Nat464XlatTest { verifyNoMoreInteractions(mNetd, mNms, mConnectivity); } + @Test + public void testNat64PrefixPreference() throws Exception { + final IpPrefix prefixFromDns = new IpPrefix(NAT64_PREFIX); + final IpPrefix prefixFromRa = new IpPrefix(OTHER_NAT64_PREFIX); + + Nat464Xlat nat = makeNat464Xlat(); + + final LinkProperties emptyLp = new LinkProperties(); + LinkProperties fixedupLp; + + fixedupLp = new LinkProperties(); + nat.setNat64PrefixFromDns(prefixFromDns); + nat.fixupLinkProperties(emptyLp, fixedupLp); + assertEquals(prefixFromDns, fixedupLp.getNat64Prefix()); + + fixedupLp = new LinkProperties(); + nat.setNat64PrefixFromRa(prefixFromRa); + nat.fixupLinkProperties(emptyLp, fixedupLp); + assertEquals(prefixFromRa, fixedupLp.getNat64Prefix()); + + fixedupLp = new LinkProperties(); + nat.setNat64PrefixFromRa(null); + nat.fixupLinkProperties(emptyLp, fixedupLp); + assertEquals(prefixFromDns, fixedupLp.getNat64Prefix()); + + fixedupLp = new LinkProperties(); + nat.setNat64PrefixFromRa(prefixFromRa); + nat.fixupLinkProperties(emptyLp, fixedupLp); + assertEquals(prefixFromRa, fixedupLp.getNat64Prefix()); + + fixedupLp = new LinkProperties(); + nat.setNat64PrefixFromDns(null); + nat.fixupLinkProperties(emptyLp, fixedupLp); + assertEquals(prefixFromRa, fixedupLp.getNat64Prefix()); + + fixedupLp = new LinkProperties(); + nat.setNat64PrefixFromRa(null); + nat.fixupLinkProperties(emptyLp, fixedupLp); + assertEquals(null, fixedupLp.getNat64Prefix()); + } + static void assertIdle(Nat464Xlat nat) { assertTrue("Nat464Xlat was not IDLE", !nat.isStarted()); } From 1c0d43fc1544713146dd13a30c0767f57bf4ae7c Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Mon, 20 Apr 2020 11:37:54 +0000 Subject: [PATCH 2/4] Make the Nat464Xlat tests more realistic. 464xlat will never be started on a network that is not connected, or on a network that has no IPv6 address. This is a no-op test-only change but it is necessary for an upcoming change that violates some of the invalid assumptions currently made by this test and causes it to fail. Bug: 150648313 Test: test-only change Change-Id: I41766e9adaa7c24454648b371e6e3cc647693be5 Merged-In: I41766e9adaa7c24454648b371e6e3cc647693be5 (cherry picked from commit df0c522d18ee73c1d20cff1a1dc955b383e6c355) --- .../server/connectivity/Nat464XlatTest.java | 66 +++++++++++++++++-- 1 file changed, 61 insertions(+), 5 deletions(-) diff --git a/tests/net/java/com/android/server/connectivity/Nat464XlatTest.java b/tests/net/java/com/android/server/connectivity/Nat464XlatTest.java index 84dad981e0..5046b6586f 100644 --- a/tests/net/java/com/android/server/connectivity/Nat464XlatTest.java +++ b/tests/net/java/com/android/server/connectivity/Nat464XlatTest.java @@ -58,6 +58,7 @@ public class Nat464XlatTest { static final String BASE_IFACE = "test0"; static final String STACKED_IFACE = "v4-test0"; + static final LinkAddress V6ADDR = new LinkAddress("2001:db8:1::f00/64"); static final LinkAddress ADDR = new LinkAddress("192.0.2.5/29"); static final String NAT64_PREFIX = "64:ff9b::/96"; static final String OTHER_NAT64_PREFIX = "2001:db8:0:64::/96"; @@ -82,6 +83,14 @@ public class Nat464XlatTest { }; } + private void markNetworkConnected() { + mNai.networkInfo.setDetailedState(NetworkInfo.DetailedState.CONNECTED, "", ""); + } + + private void markNetworkDisconnected() { + mNai.networkInfo.setDetailedState(NetworkInfo.DetailedState.DISCONNECTED, "", ""); + } + @Before public void setUp() throws Exception { mLooper = new TestLooper(); @@ -93,6 +102,7 @@ public class Nat464XlatTest { mNai.linkProperties.setInterfaceName(BASE_IFACE); mNai.networkInfo = new NetworkInfo(null); mNai.networkInfo.setType(ConnectivityManager.TYPE_WIFI); + markNetworkConnected(); when(mNai.connService()).thenReturn(mConnectivity); when(mNai.netAgentConfig()).thenReturn(mAgentConfig); when(mNai.handler()).thenReturn(mHandler); @@ -177,11 +187,20 @@ public class Nat464XlatTest { } } - @Test - public void testNormalStartAndStop() throws Exception { + private void makeClatUnnecessary(boolean dueToDisconnect) { + if (dueToDisconnect) { + markNetworkDisconnected(); + } else { + mNai.linkProperties.addLinkAddress(ADDR); + } + } + + private void checkNormalStartAndStop(boolean dueToDisconnect) throws Exception { Nat464Xlat nat = makeNat464Xlat(); ArgumentCaptor c = ArgumentCaptor.forClass(LinkProperties.class); + mNai.linkProperties.addLinkAddress(V6ADDR); + nat.setNat64PrefixFromDns(new IpPrefix(NAT64_PREFIX)); // Start clat. @@ -201,6 +220,7 @@ public class Nat464XlatTest { assertRunning(nat); // Stop clat (Network disconnects, IPv4 addr appears, ...). + makeClatUnnecessary(dueToDisconnect); nat.stop(); verify(mNetd).clatdStop(eq(BASE_IFACE)); @@ -218,11 +238,23 @@ public class Nat464XlatTest { verifyNoMoreInteractions(mNetd, mNms, mConnectivity); } + @Test + public void testNormalStartAndStopDueToDisconnect() throws Exception { + checkNormalStartAndStop(true); + } + + @Test + public void testNormalStartAndStopDueToIpv4Addr() throws Exception { + checkNormalStartAndStop(false); + } + private void checkStartStopStart(boolean interfaceRemovedFirst) throws Exception { Nat464Xlat nat = makeNat464Xlat(); ArgumentCaptor c = ArgumentCaptor.forClass(LinkProperties.class); InOrder inOrder = inOrder(mNetd, mConnectivity); + mNai.linkProperties.addLinkAddress(V6ADDR); + nat.setNat64PrefixFromDns(new IpPrefix(NAT64_PREFIX)); nat.start(); @@ -345,10 +377,11 @@ public class Nat464XlatTest { verifyNoMoreInteractions(mNetd, mNms, mConnectivity); } - @Test - public void testStopBeforeClatdStarts() throws Exception { + private void checkStopBeforeClatdStarts(boolean dueToDisconnect) throws Exception { Nat464Xlat nat = makeNat464Xlat(); + mNai.linkProperties.addLinkAddress(new LinkAddress("2001:db8::1/64")); + nat.setNat64PrefixFromDns(new IpPrefix(NAT64_PREFIX)); nat.start(); @@ -357,6 +390,7 @@ public class Nat464XlatTest { verify(mNetd).clatdStart(eq(BASE_IFACE), eq(NAT64_PREFIX)); // ConnectivityService immediately stops clat (Network disconnects, IPv4 addr appears, ...) + makeClatUnnecessary(dueToDisconnect); nat.stop(); verify(mNetd).clatdStop(eq(BASE_IFACE)); @@ -378,9 +412,20 @@ public class Nat464XlatTest { } @Test - public void testStopAndClatdNeverStarts() throws Exception { + public void testStopDueToDisconnectBeforeClatdStarts() throws Exception { + checkStopBeforeClatdStarts(true); + } + + @Test + public void testStopDueToIpv4AddrBeforeClatdStarts() throws Exception { + checkStopBeforeClatdStarts(false); + } + + private void checkStopAndClatdNeverStarts(boolean dueToDisconnect) throws Exception { Nat464Xlat nat = makeNat464Xlat(); + mNai.linkProperties.addLinkAddress(new LinkAddress("2001:db8::1/64")); + nat.setNat64PrefixFromDns(new IpPrefix(NAT64_PREFIX)); nat.start(); @@ -389,6 +434,7 @@ public class Nat464XlatTest { verify(mNetd).clatdStart(eq(BASE_IFACE), eq(NAT64_PREFIX)); // ConnectivityService immediately stops clat (Network disconnects, IPv4 addr appears, ...) + makeClatUnnecessary(dueToDisconnect); nat.stop(); verify(mNetd).clatdStop(eq(BASE_IFACE)); @@ -399,6 +445,16 @@ public class Nat464XlatTest { verifyNoMoreInteractions(mNetd, mNms, mConnectivity); } + @Test + public void testStopDueToDisconnectAndClatdNeverStarts() throws Exception { + checkStopAndClatdNeverStarts(true); + } + + @Test + public void testStopDueToIpv4AddressAndClatdNeverStarts() throws Exception { + checkStopAndClatdNeverStarts(false); + } + @Test public void testNat64PrefixPreference() throws Exception { final IpPrefix prefixFromDns = new IpPrefix(NAT64_PREFIX); From c9016cafe3443a34675e1ebad3e1dbbc0de440fd Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Mon, 20 Apr 2020 11:38:11 +0000 Subject: [PATCH 3/4] Stop prefix discovery if an RA prefix arrives in DISCOVERING Currently, if a prefix is learned from an RA while prefix discovery is running, clatd will be correctly started, but prefix discovery will be stopped. In order to fix this, make it possible to call stopPrefixDiscovery without transitioning to IDLE state (which is obviously necessary in this case), by moving the assignment of the next state from that method to its callers. For consistency, do the same for startPrefixDiscovery. Bug: 150648313 Test: new test coverage Change-Id: I3803fa3d9806848b331c35ee8bac256934bd1f21 Merged-In: I3803fa3d9806848b331c35ee8bac256934bd1f21 (cherry picked from commit c7c6f76402a989f91b02c37574b6a9de592cf1af) --- .../android/server/connectivity/Nat464Xlat.java | 14 ++++++++------ .../android/server/ConnectivityServiceTest.java | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/Nat464Xlat.java b/services/core/java/com/android/server/connectivity/Nat464Xlat.java index 8ad21966cb..34d0bed9a8 100644 --- a/services/core/java/com/android/server/connectivity/Nat464Xlat.java +++ b/services/core/java/com/android/server/connectivity/Nat464Xlat.java @@ -195,6 +195,9 @@ public class Nat464Xlat extends BaseNetworkObserver { } catch (ClassCastException | IllegalArgumentException | NullPointerException e) { Slog.e(TAG, "Invalid IPv6 address " + addrStr); } + if (mPrefixDiscoveryRunning && !isPrefixDiscoveryNeeded()) { + stopPrefixDiscovery(); + } } /** @@ -221,12 +224,11 @@ public class Nat464Xlat extends BaseNetworkObserver { if (isPrefixDiscoveryNeeded()) { if (!mPrefixDiscoveryRunning) { startPrefixDiscovery(); - } else { - // Prefix discovery is already running. Nothing to do. - mState = State.DISCOVERING; } + mState = State.DISCOVERING; } else { - stopPrefixDiscovery(); // Enters IDLE state. + stopPrefixDiscovery(); + mState = State.IDLE; } } @@ -287,7 +289,6 @@ public class Nat464Xlat extends BaseNetworkObserver { } catch (RemoteException | ServiceSpecificException e) { Slog.e(TAG, "Error starting prefix discovery on netId " + getNetId() + ": " + e); } - mState = State.DISCOVERING; mPrefixDiscoveryRunning = true; } @@ -297,7 +298,6 @@ public class Nat464Xlat extends BaseNetworkObserver { } catch (RemoteException | ServiceSpecificException e) { Slog.e(TAG, "Error stopping prefix discovery on netId " + getNetId() + ": " + e); } - mState = State.IDLE; mPrefixDiscoveryRunning = false; } @@ -330,6 +330,7 @@ public class Nat464Xlat extends BaseNetworkObserver { case IDLE: if (isPrefixDiscoveryNeeded()) { startPrefixDiscovery(); // Enters DISCOVERING state. + mState = State.DISCOVERING; } else if (requiresClat(mNetwork)) { start(); // Enters STARTING state. } @@ -344,6 +345,7 @@ public class Nat464Xlat extends BaseNetworkObserver { if (!requiresClat(mNetwork)) { // IPv4 address added. Go back to IDLE state. stopPrefixDiscovery(); + mState = State.IDLE; return; } break; diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index c1e51dd62a..a478e68c7d 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -6228,6 +6228,22 @@ public class ConnectivityServiceTest { inOrder.verify(mMockNetd).clatdStop(iface); inOrder.verify(mMockDnsResolver).startPrefix64Discovery(netId); + // If the RA prefix appears while DNS discovery is in progress, discovery is stopped and + // clatd is started with the prefix from the RA. + lp.setNat64Prefix(pref64FromRa); + mCellNetworkAgent.sendLinkProperties(lp); + expectNat64PrefixChange(callback, mCellNetworkAgent, pref64FromRa); + inOrder.verify(mMockNetd).clatdStart(iface, pref64FromRa.toString()); + inOrder.verify(mMockDnsResolver).stopPrefix64Discovery(netId); + + // Withdraw the RA prefix so we can test the case where an RA prefix appears after DNS + // discovery has succeeded. + lp.setNat64Prefix(null); + mCellNetworkAgent.sendLinkProperties(lp); + expectNat64PrefixChange(callback, mCellNetworkAgent, null); + inOrder.verify(mMockNetd).clatdStop(iface); + inOrder.verify(mMockDnsResolver).startPrefix64Discovery(netId); + mService.mNetdEventCallback.onNat64PrefixEvent(netId, true /* added */, pref64FromDnsStr, 96); expectNat64PrefixChange(callback, mCellNetworkAgent, pref64FromDns); From 4f5455fb44313c67e9f115def676485a81b30acf Mon Sep 17 00:00:00 2001 From: Aaron Huang Date: Mon, 20 Apr 2020 09:06:59 +0000 Subject: [PATCH 4/4] Add network agent to enhance NetworkProvider test Add network agent to test more situation that could get the onNetworkRequested callback. Bug: 153614605 Bug: 153613690 Bug: 153612373 Test: atest CtsNetTestCasesLatestSdk:android.net.NetworkProviderTest Change-Id: I7f827710b47546bd4419cc1ff06f03ec4635583d Merged-In: Id494a1697cc1b73e8e56ae585a69faec31c59f52 (cherry picked from commit 9e92e57fd70944cbe8bb61bbb7a5fa728d0e68f5) --- .../java/android/net/NetworkProviderTest.kt | 37 +++++++++++++++++-- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/tests/net/common/java/android/net/NetworkProviderTest.kt b/tests/net/common/java/android/net/NetworkProviderTest.kt index 4601c4bf4a..b7c47c2bc2 100644 --- a/tests/net/common/java/android/net/NetworkProviderTest.kt +++ b/tests/net/common/java/android/net/NetworkProviderTest.kt @@ -105,14 +105,43 @@ class NetworkProviderTest { .build() val cb = ConnectivityManager.NetworkCallback() mCm.requestNetwork(nr, cb) - provider.expectCallback() { - callback -> callback.request.getNetworkSpecifier() == specifier && + provider.expectCallback() { callback -> + callback.request.getNetworkSpecifier() == specifier && callback.request.hasTransport(TRANSPORT_TEST) } + val initialScore = 40 + val updatedScore = 60 + val nc = NetworkCapabilities().apply { + addTransportType(NetworkCapabilities.TRANSPORT_TEST) + removeCapability(NetworkCapabilities.NET_CAPABILITY_TRUSTED) + removeCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) + addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED) + addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING) + addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN) + setNetworkSpecifier(specifier) + } + val lp = LinkProperties() + val config = NetworkAgentConfig.Builder().build() + val agent = object : NetworkAgent(context, mHandlerThread.looper, "TestAgent", nc, lp, + initialScore, config, provider) {} + + provider.expectCallback() { callback -> + callback.request.getNetworkSpecifier() == specifier && + callback.score == initialScore && + callback.id == agent.providerId + } + + agent.sendNetworkScore(updatedScore) + provider.expectCallback() { callback -> + callback.request.getNetworkSpecifier() == specifier && + callback.score == updatedScore && + callback.id == agent.providerId + } + mCm.unregisterNetworkCallback(cb) - provider.expectCallback() { - callback -> callback.request.getNetworkSpecifier() == specifier && + provider.expectCallback() { callback -> + callback.request.getNetworkSpecifier() == specifier && callback.request.hasTransport(TRANSPORT_TEST) } mCm.unregisterNetworkProvider(provider)