Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c66d5d07 authored by Xiao Ma's avatar Xiao Ma
Browse files

Refactor the ReceiveThread with PacketReader in DhcpClient. 

To implement the IP address conflict detection, we need another
ReceiveThread to listen ARP packets as well. Ideally, we should
listen both of DHCP packets and ARP packets in the same thread
with DhcpClient state machine handler.

This CL replaces the ReceiveThread with PacketReader to listen
DHCP packets. After refactoring, DHCP packets are read from the
same handler thread instead of a separate receiving thread. Then
we can also leverage the same handler thread to listen the ARP
packets simultaneously via another raw socket.

Bug: 130775067
Test: atest NetworkStackTests NetworkStackIntegrationTests

Change-Id: I23e91a6c2d99cf8053e62f2ae9d78481ece6384d
parent df4c156e

src/android/net/dhcp/DhcpClient.java

+79 −81
Original line number Original line Diff line number Diff line
@@ -37,6 +37,7 @@ import static android.system.OsConstants.AF_PACKET; 
import static android.system.OsConstants.ETH_P_IP;

import static android.system.OsConstants.ETH_P_IP;

import static android.system.OsConstants.IPPROTO_UDP;

import static android.system.OsConstants.IPPROTO_UDP;

import static android.system.OsConstants.SOCK_DGRAM;

import static android.system.OsConstants.SOCK_DGRAM;

import static android.system.OsConstants.SOCK_NONBLOCK;

import static android.system.OsConstants.SOCK_RAW;

import static android.system.OsConstants.SOCK_RAW;

import static android.system.OsConstants.SOL_SOCKET;

import static android.system.OsConstants.SOL_SOCKET;

import static android.system.OsConstants.SO_BROADCAST;

import static android.system.OsConstants.SO_BROADCAST;
@@ -46,6 +47,7 @@ import static android.system.OsConstants.SO_REUSEADDR; 
import static com.android.server.util.NetworkStackConstants.IPV4_ADDR_ANY;

import static com.android.server.util.NetworkStackConstants.IPV4_ADDR_ANY;





import android.annotation.NonNull;

import android.annotation.NonNull;

import android.annotation.Nullable;

import android.content.Context;

import android.content.Context;

import android.net.DhcpResults;

import android.net.DhcpResults;

import android.net.InetAddresses;

import android.net.InetAddresses;
@@ -60,7 +62,9 @@ import android.net.metrics.DhcpErrorEvent; 
import android.net.metrics.IpConnectivityLog;

import android.net.metrics.IpConnectivityLog;

import android.net.util.InterfaceParams;

import android.net.util.InterfaceParams;

import android.net.util.NetworkStackUtils;

import android.net.util.NetworkStackUtils;

import android.net.util.PacketReader;

import android.net.util.SocketUtils;

import android.net.util.SocketUtils;

import android.os.Handler;

import android.os.Message;

import android.os.Message;

import android.os.SystemClock;

import android.os.SystemClock;

import android.system.ErrnoException;

import android.system.ErrnoException;
@@ -210,14 +214,9 @@ public class DhcpClient extends StateMachine { 
    private final Random mRandom;

    private final Random mRandom;

    private final IpConnectivityLog mMetricsLog = new IpConnectivityLog();

    private final IpConnectivityLog mMetricsLog = new IpConnectivityLog();





    // Sockets.

    // We use a UDP socket to send, so the kernel handles ARP and routing for us (DHCP servers can

    // - We use a packet socket to receive, because servers send us packets bound for IP addresses

    //   which we have not yet configured, and the kernel protocol stack drops these.

    // - We use a UDP socket to send, so the kernel handles ARP and routing for us (DHCP servers can

    // be off-link as well as on-link).

    // be off-link as well as on-link).

    private FileDescriptor mPacketSock;

    private FileDescriptor mUdpSock;

    private FileDescriptor mUdpSock;

    private ReceiveThread mReceiveThread;





    // State variables.

    // State variables.

    private final StateMachine mController;

    private final StateMachine mController;
@@ -244,6 +243,8 @@ public class DhcpClient extends StateMachine { 
    private Dependencies mDependencies;

    private Dependencies mDependencies;

    @NonNull

    @NonNull

    private final NetworkStackIpMemoryStore mIpMemoryStore;

    private final NetworkStackIpMemoryStore mIpMemoryStore;

    @Nullable

    private DhcpPacketHandler mDhcpPacketHandler;





    // Milliseconds SystemClock timestamps used to record transition times to DhcpBoundState.

    // Milliseconds SystemClock timestamps used to record transition times to DhcpBoundState.

    private long mLastInitEnterTime;

    private long mLastInitEnterTime;
@@ -396,23 +397,6 @@ public class DhcpClient extends StateMachine { 
        mTransactionStartMillis = SystemClock.elapsedRealtime();

        mTransactionStartMillis = SystemClock.elapsedRealtime();

    }

    }





    private boolean initSockets() {

        return initPacketSocket() && initUdpSocket();

    }



    private boolean initPacketSocket() {

        try {

            mPacketSock = Os.socket(AF_PACKET, SOCK_RAW, ETH_P_IP);

            SocketAddress addr = makePacketSocketAddress(ETH_P_IP, mIface.index);

            Os.bind(mPacketSock, addr);

            NetworkStackUtils.attachDhcpFilter(mPacketSock);

        } catch(SocketException|ErrnoException e) {

            Log.e(TAG, "Error creating packet socket", e);

            return false;

        }

        return true;

    }



    private boolean initUdpSocket() {

    private boolean initUdpSocket() {

        final int oldTag = TrafficStats.getAndSetThreadStatsTag(

        final int oldTag = TrafficStats.getAndSetThreadStatsTag(

                TrafficStatsConstants.TAG_SYSTEM_DHCP);

                TrafficStatsConstants.TAG_SYSTEM_DHCP);
@@ -442,53 +426,70 @@ public class DhcpClient extends StateMachine { 
        }

        }

    }

    }





    private void closeSockets() {

    private class DhcpPacketHandler extends PacketReader {

        closeSocketQuietly(mUdpSock);

        private FileDescriptor mPacketSock;

        closeSocketQuietly(mPacketSock);

    }



    class ReceiveThread extends Thread {



        private final byte[] mPacket = new byte[DhcpPacket.MAX_LENGTH];

        private volatile boolean mStopped = false;





        public void halt() {

        DhcpPacketHandler(Handler handler) {

            mStopped = true;

            super(handler);

            closeSockets();  // Interrupts the read() call the thread is blocked in.

        }

        }





        @Override

        @Override

        public void run() {

        protected void handlePacket(byte[] recvbuf, int length) {

            if (DBG) Log.d(TAG, "Receive thread started");

            while (!mStopped) {

                int length = 0;  // Or compiler can't tell it's initialized if a parse error occurs.

            try {

            try {

                    length = Os.read(mPacketSock, mPacket, 0, mPacket.length);

                final DhcpPacket packet = DhcpPacket.decodeFullPacket(recvbuf, length,

                    DhcpPacket packet = null;

                        DhcpPacket.ENCAP_L2);

                    packet = DhcpPacket.decodeFullPacket(mPacket, length, DhcpPacket.ENCAP_L2);

                if (DBG) Log.d(TAG, "Received packet: " + packet);

                if (DBG) Log.d(TAG, "Received packet: " + packet);

                sendMessage(CMD_RECEIVED_PACKET, packet);

                sendMessage(CMD_RECEIVED_PACKET, packet);

                } catch (IOException|ErrnoException e) {

                    if (!mStopped) {

                        Log.e(TAG, "Read error", e);

                        logError(DhcpErrorEvent.RECEIVE_ERROR);

                    }

            } catch (DhcpPacket.ParseException e) {

            } catch (DhcpPacket.ParseException e) {

                Log.e(TAG, "Can't parse packet: " + e.getMessage());

                Log.e(TAG, "Can't parse packet: " + e.getMessage());

                if (PACKET_DBG) {

                if (PACKET_DBG) {

                        Log.d(TAG, HexDump.dumpHexString(mPacket, 0, length));

                    Log.d(TAG, HexDump.dumpHexString(recvbuf, 0, length));

                }

                }

                if (e.errorCode == DhcpErrorEvent.DHCP_NO_COOKIE) {

                if (e.errorCode == DhcpErrorEvent.DHCP_NO_COOKIE) {

                        int snetTagId = 0x534e4554;

                    final int snetTagId = 0x534e4554;

                        String bugId = "31850211";

                    final String bugId = "31850211";

                        int uid = -1;

                    final int uid = -1;

                        String data = DhcpPacket.ParseException.class.getName();

                    final String data = DhcpPacket.ParseException.class.getName();

                    EventLog.writeEvent(snetTagId, bugId, uid, data);

                    EventLog.writeEvent(snetTagId, bugId, uid, data);

                }

                }

                    logError(e.errorCode);

                mMetricsLog.log(mIfaceName, new DhcpErrorEvent(e.errorCode));

            }

            }

        }

        }

            if (DBG) Log.d(TAG, "Receive thread stopped");



        @Override

        protected FileDescriptor createFd() {

            try {

                mPacketSock = Os.socket(AF_PACKET, SOCK_RAW | SOCK_NONBLOCK, 0 /* protocol */);

                NetworkStackUtils.attachDhcpFilter(mPacketSock);

                final SocketAddress addr = makePacketSocketAddress(ETH_P_IP, mIface.index);

                Os.bind(mPacketSock, addr);

            } catch (SocketException | ErrnoException e) {

                logError("Error creating packet socket", e);

                closeFd(mPacketSock);

                mPacketSock = null;

                return null;

            }

            return mPacketSock;

        }



        @Override

        protected int readPacket(FileDescriptor fd, byte[] packetBuffer) throws Exception {

            try {

                return Os.read(fd, packetBuffer, 0, packetBuffer.length);

            } catch (IOException | ErrnoException e) {

                mMetricsLog.log(mIfaceName, new DhcpErrorEvent(DhcpErrorEvent.RECEIVE_ERROR));

                throw e;

            }

        }



        @Override

        protected void logError(@NonNull String msg, @Nullable Exception e) {

            Log.e(TAG, msg, e);

        }



        public int transmitPacket(final ByteBuffer buf, final SocketAddress socketAddress)

                throws ErrnoException, SocketException {

            return Os.sendto(mPacketSock, buf.array(), 0, buf.limit(), 0, socketAddress);

        }

        }

    }

    }
@@ -500,7 +501,7 @@ public class DhcpClient extends StateMachine { 
        try {

        try {

            if (encap == DhcpPacket.ENCAP_L2) {

            if (encap == DhcpPacket.ENCAP_L2) {

                if (DBG) Log.d(TAG, "Broadcasting " + description);

                if (DBG) Log.d(TAG, "Broadcasting " + description);

                Os.sendto(mPacketSock, buf.array(), 0, buf.limit(), 0, mInterfaceBroadcastAddr);

                mDhcpPacketHandler.transmitPacket(buf, mInterfaceBroadcastAddr);

            } else if (encap == DhcpPacket.ENCAP_BOOTP && to.equals(INADDR_BROADCAST)) {

            } else if (encap == DhcpPacket.ENCAP_BOOTP && to.equals(INADDR_BROADCAST)) {

                if (DBG) Log.d(TAG, "Broadcasting " + description);

                if (DBG) Log.d(TAG, "Broadcasting " + description);

                // We only send L3-encapped broadcasts in DhcpRebindingState,

                // We only send L3-encapped broadcasts in DhcpRebindingState,
@@ -774,21 +775,22 @@ public class DhcpClient extends StateMachine { 
        @Override

        @Override

        public void enter() {

        public void enter() {

            clearDhcpState();

            clearDhcpState();

            if (initInterface() && initSockets()) {

            if (initInterface() && initUdpSocket()) {

                mReceiveThread = new ReceiveThread();

                mDhcpPacketHandler = new DhcpPacketHandler(getHandler());

                mReceiveThread.start();

                if (mDhcpPacketHandler.start()) return;

            } else {

                Log.e(TAG, "Fail to start DHCP Packet Handler");

            }

            notifyFailure();

            notifyFailure();

            transitionTo(mStoppedState);

            transitionTo(mStoppedState);

        }

        }

        }





        @Override

        @Override

        public void exit() {

        public void exit() {

            if (mReceiveThread != null) {

            if (mDhcpPacketHandler != null) {

                mReceiveThread.halt();  // Also closes sockets.

                mDhcpPacketHandler.stop();

                mReceiveThread = null;

                if (DBG) Log.d(TAG, "DHCP Packet Handler stopped");

            }

            }

            closeSocketQuietly(mUdpSock);

            clearDhcpState();

            clearDhcpState();

        }

        }
@@ -1289,10 +1291,6 @@ public class DhcpClient extends StateMachine { 
    class DhcpRebootingState extends LoggingState {

    class DhcpRebootingState extends LoggingState {

    }

    }





    private void logError(int errorCode) {

        mMetricsLog.log(mIfaceName, new DhcpErrorEvent(errorCode));

    }



    private void logState(String name, int durationMs) {

    private void logState(String name, int durationMs) {

        final DhcpClientEvent event = new DhcpClientEvent.Builder()

        final DhcpClientEvent event = new DhcpClientEvent.Builder()

                .setMsg(name)

                .setMsg(name)

tests/integration/src/android/net/ip/IpClientIntegrationTest.java

+5 −3
Original line number Original line Diff line number Diff line
@@ -130,6 +130,7 @@ public class IpClientIntegrationTest { 
    private String mIfaceName;

    private String mIfaceName;

    private INetd mNetd;

    private INetd mNetd;

    private HandlerThread mPacketReaderThread;

    private HandlerThread mPacketReaderThread;

    private Handler mHandler;

    private TapPacketReader mPacketReader;

    private TapPacketReader mPacketReader;

    private IpClient mIpc;

    private IpClient mIpc;

    private Dependencies mDependencies;

    private Dependencies mDependencies;
@@ -271,7 +272,7 @@ public class IpClientIntegrationTest { 
    @After

    @After

    public void tearDown() throws Exception {

    public void tearDown() throws Exception {

        if (mPacketReader != null) {

        if (mPacketReader != null) {

            mPacketReader.stop(); // Also closes the socket

            mHandler.post(() -> mPacketReader.stop()); // Also closes the socket

        }

        }

        if (mPacketReaderThread != null) {

        if (mPacketReaderThread != null) {

            mPacketReaderThread.quitSafely();

            mPacketReaderThread.quitSafely();
@@ -297,10 +298,11 @@ public class IpClientIntegrationTest { 
        mIfaceName = iface.getInterfaceName();

        mIfaceName = iface.getInterfaceName();

        mPacketReaderThread = new HandlerThread(IpClientIntegrationTest.class.getSimpleName());

        mPacketReaderThread = new HandlerThread(IpClientIntegrationTest.class.getSimpleName());

        mPacketReaderThread.start();

        mPacketReaderThread.start();

        mHandler = mPacketReaderThread.getThreadHandler();





        final ParcelFileDescriptor tapFd = iface.getFileDescriptor();

        final ParcelFileDescriptor tapFd = iface.getFileDescriptor();

        mPacketReader = new TapPacketReader(mPacketReaderThread.getThreadHandler(), tapFd);

        mPacketReader = new TapPacketReader(mHandler, tapFd);

        mPacketReader.start();

        mHandler.post(() -> mPacketReader.start());

    }

    }





    private void setUpIpClient() throws Exception {

    private void setUpIpClient() throws Exception {