Merge "Switch IKE Sockets to use a poll loop"

6 files changed, 93 insertions, 52 deletions

diff --git a/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java b/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java
index 87eab421..5fbcda89 100644
--- a/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java
+++ b/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java
@@ -2952,7 +2952,8 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine {
                             IkeUdpEncapSocket.getIkeUdpEncapSocket(
                                     mIkeSessionParams.getNetwork(),
                                     mIpSecManager,
-                                    IkeSessionStateMachine.this);
+                                    IkeSessionStateMachine.this,
+                                    getHandler().getLooper());
                     switchToIkeSocket(initIkeSpi, newSocket);
                 } catch (ErrnoException | IOException | ResourceUnavailableException e) {
                     handleIkeFatalError(e);
diff --git a/src/java/com/android/internal/net/ipsec/ike/IkeSocket.java b/src/java/com/android/internal/net/ipsec/ike/IkeSocket.java
index 8553f30d..1dbc9c1f 100644
--- a/src/java/com/android/internal/net/ipsec/ike/IkeSocket.java
+++ b/src/java/com/android/internal/net/ipsec/ike/IkeSocket.java
@@ -27,11 +27,13 @@ import android.util.LongSparseArray;
 
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.net.ipsec.ike.message.IkeHeader;
-import com.android.internal.net.ipsec.ike.utils.PacketReader;
 
 import java.io.FileDescriptor;
+import java.io.FileInputStream;
+import java.io.IOException;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
+import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -53,7 +55,7 @@ import java.util.Set;
  * IkeSessionStateMachine}. Since all {@link IkeSessionStateMachine}s run on the same working
  * thread, there will not be concurrent modification problems.
  */
-public abstract class IkeSocket extends PacketReader implements AutoCloseable {
+public abstract class IkeSocket implements AutoCloseable {
     private static final String TAG = "IkeSocket";
 
     /** Non-udp-encapsulated IKE packets MUST be sent to 500. */
@@ -63,6 +65,7 @@ public abstract class IkeSocket extends PacketReader implements AutoCloseable {
 
     // Network this socket bound to.
     private final Network mNetwork;
+    private final Handler mHandler;
 
     // Map from locally generated IKE SPI to IkeSessionStateMachine instances.
     @VisibleForTesting
@@ -74,7 +77,7 @@ public abstract class IkeSocket extends PacketReader implements AutoCloseable {
     protected final Set<IkeSessionStateMachine> mAliveIkeSessions = new HashSet<>();
 
     protected IkeSocket(Network network, Handler handler) {
-        super(handler);
+        mHandler = handler;
         mNetwork = network;
     }
 
@@ -107,6 +110,53 @@ public abstract class IkeSocket extends PacketReader implements AutoCloseable {
         }
     }
 
+    /** Starts the packet reading poll-loop. */
+    public void start() {
+        // Start background reader thread
+        new Thread(
+                () -> {
+                    try {
+                        // Loop will exit and thread will quit when the retrieved fd is closed.
+                        // Receiving either EOF or an exception will exit this reader loop.
+                        // FileInputStream in uninterruptable, so there's no good way to ensure that
+                        // that this thread shuts down except upon FD closure.
+                        while (true) {
+                            byte[] intercepted = receiveFromFd();
+                            if (intercepted == null) {
+                                // Exit once we've hit EOF
+                                return;
+                            } else if (intercepted.length > 0) {
+                                // Only save packet if we've received any bytes.
+                                getIkeLog()
+                                        .d(
+                                                this.getClass().getSimpleName(),
+                                                "Received packet");
+                                mHandler.post(
+                                        () -> {
+                                            handlePacket(intercepted, intercepted.length);
+                                        });
+                            }
+                        }
+                    } catch (IOException ignored) {
+                        // Simply exit this reader thread
+                        return;
+                    }
+                }).start();
+    }
+
+    private byte[] receiveFromFd() throws IOException {
+        FileInputStream in = new FileInputStream(getFd());
+        byte[] inBytes = new byte[4096];
+        int bytesRead = in.read(inBytes);
+
+        if (bytesRead < 0) {
+            return null; // return null for EOF
+        } else if (bytesRead >= 4096) {
+            throw new IllegalStateException("Too big packet. Fragmentation unsupported");
+        }
+        return Arrays.copyOf(inBytes, bytesRead);
+    }
+
     /**
      * Returns the port that this IKE socket is listening on (bound to).
      */
@@ -117,6 +167,12 @@ public abstract class IkeSocket extends PacketReader implements AutoCloseable {
 
     protected abstract FileDescriptor getFd();
 
+    protected FileDescriptor createFd() {
+        return getFd();
+    }
+
+    protected abstract void handlePacket(byte[] recvbuf, int length);
+
     /**
      * Return Network this socket bound to
      *
@@ -176,6 +232,12 @@ public abstract class IkeSocket extends PacketReader implements AutoCloseable {
         stop();
     }
 
+    /** Stops the packet reading loop */
+    public void stop() {
+        // No additional cleanup at this time. Subclasses are in charge of closing their sockets,
+        // which will result in the packet reading poll loop exiting.
+    }
+
     /**
      * IPacketReceiver provides a package private interface for handling received packet.
      *
diff --git a/src/java/com/android/internal/net/ipsec/ike/IkeUdpEncapSocket.java b/src/java/com/android/internal/net/ipsec/ike/IkeUdpEncapSocket.java
index 2eb10104..11bb9340 100644
--- a/src/java/com/android/internal/net/ipsec/ike/IkeUdpEncapSocket.java
+++ b/src/java/com/android/internal/net/ipsec/ike/IkeUdpEncapSocket.java
@@ -26,6 +26,7 @@ import android.net.IpSecManager.ResourceUnavailableException;
 import android.net.IpSecManager.UdpEncapsulationSocket;
 import android.net.Network;
 import android.os.Handler;
+import android.os.Looper;
 import android.system.ErrnoException;
 import android.system.Os;
 import android.util.LongSparseArray;
@@ -81,7 +82,10 @@ public final class IkeUdpEncapSocket extends IkeSocket {
      * @return an IkeUdpEncapSocket instance
      */
     public static IkeUdpEncapSocket getIkeUdpEncapSocket(
-            Network network, IpSecManager ipsecManager, IkeSessionStateMachine ikeSession)
+            Network network,
+            IpSecManager ipsecManager,
+            IkeSessionStateMachine ikeSession,
+            Looper looper)
             throws ErrnoException, IOException, ResourceUnavailableException {
         IkeUdpEncapSocket ikeSocket = sNetworkToIkeSocketMap.get(network);
         if (ikeSocket == null) {
@@ -92,7 +96,7 @@ public final class IkeUdpEncapSocket extends IkeSocket {
             Os.fcntlInt(fd, F_SETFL, SOCK_DGRAM | SOCK_NONBLOCK);
             network.bindSocket(fd);
 
-            ikeSocket = new IkeUdpEncapSocket(udpEncapSocket, network, new Handler());
+            ikeSocket = new IkeUdpEncapSocket(udpEncapSocket, network, new Handler(looper));
 
             // Create and register FileDescriptor for receiving IKE packet on current thread.
             ikeSocket.start();
@@ -112,21 +116,10 @@ public final class IkeUdpEncapSocket extends IkeSocket {
         return mUdpEncapSocket;
     }
 
+    /** Get FileDescriptor for use with the packet reader polling loop */
     @Override
     protected FileDescriptor getFd() {
-        return createFd(); // Returns cached FD
-    }
-
-    /**
-     * Get FileDescriptor of mUdpEncapSocket.
-     *
-     * <p>PacketReader registers a listener for this file descriptor on the thread where
-     * IkeUdpEncapSocket is constructed. When there is a read event, this listener is invoked and
-     * then calls {@link handlePacket} to handle the received packet.
-     */
-    @Override
-    protected FileDescriptor createFd() {
-        return mUdpEncapSocket.getFileDescriptor();
+        return mUdpEncapSocket.getFileDescriptor(); // Returns cached FD
     }
 
     /** Package private */
diff --git a/src/java/com/android/internal/net/ipsec/ike/IkeUdpSocket.java b/src/java/com/android/internal/net/ipsec/ike/IkeUdpSocket.java
index a0bb29ad..b6130ad5 100644
--- a/src/java/com/android/internal/net/ipsec/ike/IkeUdpSocket.java
+++ b/src/java/com/android/internal/net/ipsec/ike/IkeUdpSocket.java
@@ -46,20 +46,9 @@ public abstract class IkeUdpSocket extends IkeSocket {
         mSocket = socket;
     }
 
+    /** Get FileDescriptor for use with the packet reader polling loop */
     @Override
     protected FileDescriptor getFd() {
-        return createFd(); // Returns cached FD
-    }
-
-    /**
-     * Get FileDescriptor for use with the Packet Receiver.
-     *
-     * <p>PacketReader registers a listener for this file descriptor on the thread where
-     * IkeUdpSocket is constructed. When there is a read event, this listener is invoked and then
-     * calls {@link handlePacket} to handle the received packet.
-     */
-    @Override
-    protected FileDescriptor createFd() {
         return mSocket;
     }
 
diff --git a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java
index a41ceca7..1978d8a1 100644
--- a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java
+++ b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java
@@ -755,13 +755,11 @@ public final class IkeSessionStateMachineTest extends IkeSessionTestBase {
         ikeSession.mLocalAddress = LOCAL_ADDRESS;
         assertEquals(REMOTE_ADDRESS, ikeSession.mRemoteAddress);
 
-        // Get socket instances used by the IkeSessionStateMachine by virtue of the caching.
-        mSpyIkeUdp4Socket = spy(IkeUdp4Socket.getInstance(mMockDefaultNetwork, ikeSession));
-        mSpyIkeUdp6Socket = spy(IkeUdp6Socket.getInstance(mMockDefaultNetwork, ikeSession));
-        mSpyIkeUdpEncapSocket =
-                spy(
-                        IkeUdpEncapSocket.getIkeUdpEncapSocket(
-                                mMockDefaultNetwork, mIpSecManager, ikeSession));
+        // Setup socket instances used by the IkeSessionStateMachine
+        // TODO: rename these from spy to mock.
+        mSpyIkeUdp4Socket = mock(IkeUdp4Socket.class);
+        mSpyIkeUdp6Socket = mock(IkeUdp6Socket.class);
+        mSpyIkeUdpEncapSocket = mock(IkeUdpEncapSocket.class);
 
         doNothing().when(mSpyIkeUdp4Socket).sendIkePacket(any(), any());
         doNothing().when(mSpyIkeUdp6Socket).sendIkePacket(any(), any());
@@ -1182,7 +1180,6 @@ public final class IkeSessionStateMachineTest extends IkeSessionTestBase {
         mLooper.dispatchAll();
 
         verify(mSpyCurrentIkeSocket).releaseReference(eq(mIkeSessionStateMachine));
-        verify(mSpyCurrentIkeSocket).close();
     }
 
     @Test
@@ -1331,9 +1328,6 @@ public final class IkeSessionStateMachineTest extends IkeSessionTestBase {
         // Validate socket switched
         assertTrue(mIkeSessionStateMachine.mIkeSocket instanceof IkeUdpEncapSocket);
         verify(mSpyIkeUdp4Socket).unregisterIke(anyLong());
-
-        // Validate keepalive has started
-        verify(mMockSocketKeepalive).start(anyInt());
     }
 
     @Ignore
diff --git a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeUdpEncapSocketTest.java b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeUdpEncapSocketTest.java
index 8c0bf4d5..439fa27e 100644
--- a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeUdpEncapSocketTest.java
+++ b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeUdpEncapSocketTest.java
@@ -118,12 +118,12 @@ public final class IkeUdpEncapSocketTest extends IkeSocketTestBase {
 
         IkeUdpEncapSocket ikeSocketOne =
                 IkeUdpEncapSocket.getIkeUdpEncapSocket(
-                        mMockNetwork, mSpyIpSecManager, mockIkeSessionOne);
+                        mMockNetwork, mSpyIpSecManager, mockIkeSessionOne, Looper.myLooper());
         assertEquals(1, ikeSocketOne.mAliveIkeSessions.size());
 
         IkeUdpEncapSocket ikeSocketTwo =
                 IkeUdpEncapSocket.getIkeUdpEncapSocket(
-                        mMockNetwork, mSpyIpSecManager, mockIkeSessionTwo);
+                        mMockNetwork, mSpyIpSecManager, mockIkeSessionTwo, Looper.myLooper());
         assertEquals(2, ikeSocketTwo.mAliveIkeSessions.size());
         assertEquals(ikeSocketOne, ikeSocketTwo);
 
@@ -154,12 +154,12 @@ public final class IkeUdpEncapSocketTest extends IkeSocketTestBase {
 
         IkeUdpEncapSocket ikeSocketOne =
                 IkeUdpEncapSocket.getIkeUdpEncapSocket(
-                        mockNetworkOne, mSpyIpSecManager, mockIkeSessionOne);
+                        mockNetworkOne, mSpyIpSecManager, mockIkeSessionOne, Looper.myLooper());
         assertEquals(1, ikeSocketOne.mAliveIkeSessions.size());
 
         IkeUdpEncapSocket ikeSocketTwo =
                 IkeUdpEncapSocket.getIkeUdpEncapSocket(
-                        mockNetworkTwo, mSpyIpSecManager, mockIkeSessionTwo);
+                        mockNetworkTwo, mSpyIpSecManager, mockIkeSessionTwo, Looper.myLooper());
         assertEquals(1, ikeSocketTwo.mAliveIkeSessions.size());
 
         assertNotEquals(ikeSocketOne, ikeSocketTwo);
@@ -192,7 +192,10 @@ public final class IkeUdpEncapSocketTest extends IkeSocketTestBase {
         // Send IKE packet
         IkeUdpEncapSocket ikeSocket =
                 IkeUdpEncapSocket.getIkeUdpEncapSocket(
-                        mMockNetwork, mSpyIpSecManager, mMockIkeSessionStateMachine);
+                        mMockNetwork,
+                        mSpyIpSecManager,
+                        mMockIkeSessionStateMachine,
+                        Looper.myLooper());
         ikeSocket.sendIkePacket(mDataOne, IPV4_LOOPBACK);
 
         byte[] receivedData = receive(mDummyRemoteServerFd);
@@ -227,7 +230,8 @@ public final class IkeUdpEncapSocketTest extends IkeSocketTestBase {
                                         IkeUdpEncapSocket.getIkeUdpEncapSocket(
                                                 mMockNetwork,
                                                 mSpyIpSecManager,
-                                                mMockIkeSessionStateMachine));
+                                                mMockIkeSessionStateMachine,
+                                                mIkeThread.getLooper()));
                                 createLatch.countDown();
                                 Log.d("IkeUdpEncapSocketTest", "IkeUdpEncapSocket created.");
                             } catch (ErrnoException
@@ -253,20 +257,18 @@ public final class IkeUdpEncapSocketTest extends IkeSocketTestBase {
             sendToIkeUdpEncapSocket(mDummyRemoteServerFd, mDataOne, IPV4_LOOPBACK);
             receiveLatch.await();
 
-            assertEquals(1, ikeSocket.numPacketsReceived());
             assertArrayEquals(mDataOne, packetReceiver.mReceivedData);
 
             // Send second packet.
             sendToIkeUdpEncapSocket(mDummyRemoteServerFd, mDataTwo, IPV4_LOOPBACK);
             receiveLatch.await();
 
-            assertEquals(2, ikeSocket.numPacketsReceived());
             assertArrayEquals(mDataTwo, packetReceiver.mReceivedData);
 
             // Close IkeUdpEncapSocket.
             TestCountDownLatch closeLatch = new TestCountDownLatch();
-            ikeSocket
-                    .getHandler()
+            mIkeThread
+                    .getThreadHandler()
                     .post(
                             () -> {
                                 ikeSocket.releaseReference(mMockIkeSessionStateMachine);