Merge "Reorder clatStart() to setup 464xlat sockets before tun interface" into main
diff --git a/service/src/com/android/server/connectivity/ClatCoordinator.java b/service/src/com/android/server/connectivity/ClatCoordinator.java
index aec4f24..630dba8 100644
--- a/service/src/com/android/server/connectivity/ClatCoordinator.java
+++ b/service/src/com/android/server/connectivity/ClatCoordinator.java
@@ -637,8 +637,9 @@
             throw new IOException("Invalid IPv6 address " + v6Str);
         }
 
-        // [3] Open, configure and bring up the tun interface.
-        // Create the v4-... tun interface.
+
+        // [3] Open and configure local 464xlat read/write sockets.
+        // Opens a packet socket to receive IPv6 packets in clatd.
 
         // Initialize all required file descriptors with null pointer. This makes the following
         // error handling easier. Simply always call #maybeCleanUp for closing file descriptors,
@@ -647,61 +648,6 @@
         ParcelFileDescriptor readSock6 = null;
         ParcelFileDescriptor writeSock6 = null;
 
-        final String tunIface = CLAT_PREFIX + iface;
-        try {
-            tunFd = mDeps.adoptFd(mDeps.createTunInterface(tunIface));
-        } catch (IOException e) {
-            throw new IOException("Create tun interface " + tunIface + " failed: " + e);
-        }
-
-        final int tunIfIndex = mDeps.getInterfaceIndex(tunIface);
-        if (tunIfIndex == INVALID_IFINDEX) {
-            maybeCleanUp(tunFd, readSock6, writeSock6);
-            throw new IOException("Fail to get interface index for interface " + tunIface);
-        }
-
-        // disable IPv6 on it - failing to do so is not a critical error
-        try {
-            mNetd.interfaceSetEnableIPv6(tunIface, false /* enabled */);
-        } catch (RemoteException | ServiceSpecificException e) {
-            Log.e(TAG, "Disable IPv6 on " + tunIface + " failed: " + e);
-        }
-
-        // Detect ipv4 mtu.
-        final int detectedMtu;
-        try {
-            detectedMtu = mDeps.detectMtu(pfx96Str,
-                ByteBuffer.wrap(GOOGLE_DNS_4.getAddress()).getInt(), fwmark);
-        } catch (IOException e) {
-            maybeCleanUp(tunFd, readSock6, writeSock6);
-            throw new IOException("Detect MTU on " + tunIface + " failed: " + e);
-        }
-        final int mtu = adjustMtu(detectedMtu);
-        Log.i(TAG, "detected ipv4 mtu of " + detectedMtu + " adjusted to " + mtu);
-
-        // Config tun interface mtu, address and bring up.
-        try {
-            mNetd.interfaceSetMtu(tunIface, mtu);
-        } catch (RemoteException | ServiceSpecificException e) {
-            maybeCleanUp(tunFd, readSock6, writeSock6);
-            throw new IOException("Set MTU " + mtu + " on " + tunIface + " failed: " + e);
-        }
-        final InterfaceConfigurationParcel ifConfig = new InterfaceConfigurationParcel();
-        ifConfig.ifName = tunIface;
-        ifConfig.ipv4Addr = v4Str;
-        ifConfig.prefixLength = 32;
-        ifConfig.hwAddr = "";
-        ifConfig.flags = new String[] {IF_STATE_UP};
-        try {
-            mNetd.interfaceSetCfg(ifConfig);
-        } catch (RemoteException | ServiceSpecificException e) {
-            maybeCleanUp(tunFd, readSock6, writeSock6);
-            throw new IOException("Setting IPv4 address to " + ifConfig.ipv4Addr + "/"
-                    + ifConfig.prefixLength + " failed on " + ifConfig.ifName + ": " + e);
-        }
-
-        // [4] Open and configure local 464xlat read/write sockets.
-        // Opens a packet socket to receive IPv6 packets in clatd.
         try {
             // Use a JNI call to get native file descriptor instead of Os.socket() because we would
             // like to use ParcelFileDescriptor to manage file descriptor. But ctor
@@ -760,6 +706,63 @@
             throw new IOException("configure packet socket failed: " + e);
         }
 
+        // [4] Open, configure and bring up the tun interface.
+        // Create the v4-... tun interface.
+
+        final String tunIface = CLAT_PREFIX + iface;
+        try {
+            tunFd = mDeps.adoptFd(mDeps.createTunInterface(tunIface));
+        } catch (IOException e) {
+            maybeCleanUp(tunFd, readSock6, writeSock6);
+            throw new IOException("Create tun interface " + tunIface + " failed: " + e);
+        }
+
+        final int tunIfIndex = mDeps.getInterfaceIndex(tunIface);
+        if (tunIfIndex == INVALID_IFINDEX) {
+            maybeCleanUp(tunFd, readSock6, writeSock6);
+            throw new IOException("Fail to get interface index for interface " + tunIface);
+        }
+
+        // disable IPv6 on it - failing to do so is not a critical error
+        try {
+            mNetd.interfaceSetEnableIPv6(tunIface, false /* enabled */);
+        } catch (RemoteException | ServiceSpecificException e) {
+            Log.e(TAG, "Disable IPv6 on " + tunIface + " failed: " + e);
+        }
+
+        // Detect ipv4 mtu.
+        final int detectedMtu;
+        try {
+            detectedMtu = mDeps.detectMtu(pfx96Str,
+                    ByteBuffer.wrap(GOOGLE_DNS_4.getAddress()).getInt(), fwmark);
+        } catch (IOException e) {
+            maybeCleanUp(tunFd, readSock6, writeSock6);
+            throw new IOException("Detect MTU on " + tunIface + " failed: " + e);
+        }
+        final int mtu = adjustMtu(detectedMtu);
+        Log.i(TAG, "detected ipv4 mtu of " + detectedMtu + " adjusted to " + mtu);
+
+        // Config tun interface mtu, address and bring up.
+        try {
+            mNetd.interfaceSetMtu(tunIface, mtu);
+        } catch (RemoteException | ServiceSpecificException e) {
+            maybeCleanUp(tunFd, readSock6, writeSock6);
+            throw new IOException("Set MTU " + mtu + " on " + tunIface + " failed: " + e);
+        }
+        final InterfaceConfigurationParcel ifConfig = new InterfaceConfigurationParcel();
+        ifConfig.ifName = tunIface;
+        ifConfig.ipv4Addr = v4Str;
+        ifConfig.prefixLength = 32;
+        ifConfig.hwAddr = "";
+        ifConfig.flags = new String[] {IF_STATE_UP};
+        try {
+            mNetd.interfaceSetCfg(ifConfig);
+        } catch (RemoteException | ServiceSpecificException e) {
+            maybeCleanUp(tunFd, readSock6, writeSock6);
+            throw new IOException("Setting IPv4 address to " + ifConfig.ipv4Addr + "/"
+                    + ifConfig.prefixLength + " failed on " + ifConfig.ifName + ": " + e);
+        }
+
         // [5] Start clatd.
         final int pid;
         try {
diff --git a/tests/unit/java/com/android/server/connectivity/ClatCoordinatorTest.java b/tests/unit/java/com/android/server/connectivity/ClatCoordinatorTest.java
index da7fda3..72dde7f 100644
--- a/tests/unit/java/com/android/server/connectivity/ClatCoordinatorTest.java
+++ b/tests/unit/java/com/android/server/connectivity/ClatCoordinatorTest.java
@@ -38,6 +38,7 @@
 import static org.mockito.Mockito.clearInvocations;
 import static org.mockito.Mockito.eq;
 import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
@@ -418,21 +419,6 @@
         inOrder.verify(mDeps).generateIpv6Address(eq(BASE_IFACE),
                 eq(XLAT_LOCAL_IPV4ADDR_STRING), eq(NAT64_PREFIX_STRING), eq(MARK));
 
-        // Open, configure and bring up the tun interface.
-        inOrder.verify(mDeps).createTunInterface(eq(STACKED_IFACE));
-        inOrder.verify(mDeps).adoptFd(eq(TUN_FD));
-        inOrder.verify(mDeps).getInterfaceIndex(eq(STACKED_IFACE));
-        inOrder.verify(mNetd).interfaceSetEnableIPv6(eq(STACKED_IFACE), eq(false /* enable */));
-        inOrder.verify(mDeps).detectMtu(eq(NAT64_PREFIX_STRING), eq(GOOGLE_DNS_4), eq(MARK));
-        inOrder.verify(mNetd).interfaceSetMtu(eq(STACKED_IFACE),
-                eq(1472 /* ETHER_MTU(1500) - MTU_DELTA(28) */));
-        inOrder.verify(mNetd).interfaceSetCfg(argThat(cfg ->
-                STACKED_IFACE.equals(cfg.ifName)
-                && XLAT_LOCAL_IPV4ADDR_STRING.equals(cfg.ipv4Addr)
-                && (32 == cfg.prefixLength)
-                && "".equals(cfg.hwAddr)
-                && assertContainsFlag(cfg.flags, IF_STATE_UP)));
-
         // Open and configure 464xlat read/write sockets.
         inOrder.verify(mDeps).openPacketSocket();
         inOrder.verify(mDeps).adoptFd(eq(PACKET_SOCK_FD));
@@ -449,6 +435,21 @@
                 argThat(fd -> Objects.equals(PACKET_SOCK_PFD.getFileDescriptor(), fd)),
                 eq(XLAT_LOCAL_IPV6ADDR_STRING), eq(BASE_IFINDEX));
 
+        // Open, configure and bring up the tun interface.
+        inOrder.verify(mDeps).createTunInterface(eq(STACKED_IFACE));
+        inOrder.verify(mDeps).adoptFd(eq(TUN_FD));
+        inOrder.verify(mDeps).getInterfaceIndex(eq(STACKED_IFACE));
+        inOrder.verify(mNetd).interfaceSetEnableIPv6(eq(STACKED_IFACE), eq(false /* enable */));
+        inOrder.verify(mDeps).detectMtu(eq(NAT64_PREFIX_STRING), eq(GOOGLE_DNS_4), eq(MARK));
+        inOrder.verify(mNetd).interfaceSetMtu(eq(STACKED_IFACE),
+                eq(1472 /* ETHER_MTU(1500) - MTU_DELTA(28) */));
+        inOrder.verify(mNetd).interfaceSetCfg(argThat(cfg ->
+                STACKED_IFACE.equals(cfg.ifName)
+                        && XLAT_LOCAL_IPV4ADDR_STRING.equals(cfg.ipv4Addr)
+                        && (32 == cfg.prefixLength)
+                        && "".equals(cfg.hwAddr)
+                        && assertContainsFlag(cfg.flags, IF_STATE_UP)));
+
         // Start clatd.
         inOrder.verify(mDeps).startClatd(
                 argThat(fd -> Objects.equals(TUN_PFD.getFileDescriptor(), fd)),
@@ -630,9 +631,13 @@
             public int createTunInterface(@NonNull String tuniface) throws IOException {
                 throw new IOException();
             }
+            @Override
+            public IBpfMap<CookieTagMapKey, CookieTagMapValue> getBpfCookieTagMap() {
+                return  mock(IBpfMap.class);
+            }
         }
         checkNotStartClat(new FailureDependencies(), false /* needToCloseTunFd */,
-                false /* needToClosePacketSockFd */, false /* needToCloseRawSockFd */);
+                true /* needToClosePacketSockFd */, true /* needToCloseRawSockFd */);
     }
 
     @Test
@@ -643,9 +648,14 @@
                     throws IOException {
                 throw new IOException();
             }
+
+            @Override
+            public IBpfMap<CookieTagMapKey, CookieTagMapValue> getBpfCookieTagMap() {
+                return  mock(IBpfMap.class);
+            }
         }
         checkNotStartClat(new FailureDependencies(), true /* needToCloseTunFd */,
-                false /* needToClosePacketSockFd */, false /* needToCloseRawSockFd */);
+                true /* needToClosePacketSockFd */, true /* needToCloseRawSockFd */);
     }
 
     @Test
@@ -656,7 +666,7 @@
                 throw new IOException();
             }
         }
-        checkNotStartClat(new FailureDependencies(), true /* needToCloseTunFd */,
+        checkNotStartClat(new FailureDependencies(), false /* needToCloseTunFd */,
                 false /* needToClosePacketSockFd */, false /* needToCloseRawSockFd */);
     }
 
@@ -668,7 +678,7 @@
                 throw new IOException();
             }
         }
-        checkNotStartClat(new FailureDependencies(), true /* needToCloseTunFd */,
+        checkNotStartClat(new FailureDependencies(), false /* needToCloseTunFd */,
                 true /* needToClosePacketSockFd */, false /* needToCloseRawSockFd */);
     }
 
@@ -681,7 +691,7 @@
                 throw new IOException();
             }
         }
-        checkNotStartClat(new FailureDependencies(), true /* needToCloseTunFd */,
+        checkNotStartClat(new FailureDependencies(), false /* needToCloseTunFd */,
                 true /* needToClosePacketSockFd */, true /* needToCloseRawSockFd */);
     }
 
@@ -694,7 +704,7 @@
                 throw new IOException();
             }
         }
-        checkNotStartClat(new FailureDependencies(), true /* needToCloseTunFd */,
+        checkNotStartClat(new FailureDependencies(), false /* needToCloseTunFd */,
                 true /* needToClosePacketSockFd */, true /* needToCloseRawSockFd */);
     }
 
@@ -721,7 +731,7 @@
                 throw new IOException();
             }
         }
-        checkNotStartClat(new FailureDependencies(), true /* needToCloseTunFd */,
+        checkNotStartClat(new FailureDependencies(), false /* needToCloseTunFd */,
                 true /* needToClosePacketSockFd */, true /* needToCloseRawSockFd */);
     }
 
@@ -733,7 +743,7 @@
                 return null;
             }
         }
-        checkNotStartClat(new FailureDependencies(), true /* needToCloseTunFd */,
+        checkNotStartClat(new FailureDependencies(), false /* needToCloseTunFd */,
                 true /* needToClosePacketSockFd */, true /* needToCloseRawSockFd */);
     }
 }