From 632092bb287da982bb897ee0545a5ee35218d3c0 Mon Sep 17 00:00:00 2001
From: Lorenzo Colitti <lorenzo@google.com>
Date: Mon, 20 Apr 2020 16:04:46 +0900
Subject: [PATCH] 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
---
 .../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 8ad21966cb9f1..34d0bed9a8023 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 c1e51dd62ad4f..a478e68c7dba6 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);