From f8ab26379ee206545b35b89b5cddcc9cea86abe5 Mon Sep 17 00:00:00 2001 From: KH Shi Date: Wed, 20 Dec 2023 06:25:54 +0000 Subject: [PATCH 1/2] Revert "Refactor RouterAdvertisementDaemon to build new RA in HandlerThread" This reverts commit 23a2158c386679d60ffe00a1d457a0a252547052. Reason for revert: Design changes, don't use extra Handler in RouterAdvertisementDaemon. Change-Id: Iba829ec028e887b6625f4f972e936ca5757a7614 --- .../net/ip/RouterAdvertisementDaemon.java | 53 +++++++------------ 1 file changed, 20 insertions(+), 33 deletions(-) diff --git a/Tethering/src/android/net/ip/RouterAdvertisementDaemon.java b/Tethering/src/android/net/ip/RouterAdvertisementDaemon.java index 5e9bbcb5d1..d8779b83c0 100644 --- a/Tethering/src/android/net/ip/RouterAdvertisementDaemon.java +++ b/Tethering/src/android/net/ip/RouterAdvertisementDaemon.java @@ -47,10 +47,8 @@ import android.system.ErrnoException; import android.system.Os; import android.system.StructTimeval; import android.util.Log; -import android.util.Pair; import androidx.annotation.NonNull; -import androidx.annotation.Nullable; import com.android.internal.annotations.GuardedBy; import com.android.net.module.util.FdEventsReader; @@ -114,9 +112,8 @@ public class RouterAdvertisementDaemon { private static final int DAY_IN_SECONDS = 86_400; // Commands for IpServer to control RouterAdvertisementDaemon - private static final int CMD_START = 1; - private static final int CMD_STOP = 2; - private static final int CMD_BUILD_NEW_RA = 3; + private static final int CMD_START = 1; + private static final int CMD_STOP = 2; private final InterfaceParams mInterface; private final InetSocketAddress mAllNodes; @@ -281,26 +278,6 @@ public class RouterAdvertisementDaemon { mRsPacketListener = null; } break; - case CMD_BUILD_NEW_RA: - synchronized (mLock) { - // raInfo.first is deprecatedParams and raInfo.second is newParams. - final Pair raInfo = (Pair) msg.obj; - if (raInfo.first != null) { - mDeprecatedInfoTracker.putPrefixes(raInfo.first.prefixes); - mDeprecatedInfoTracker.putDnses(raInfo.first.dnses); - } - - if (raInfo.second != null) { - // Process information that is no longer deprecated. - mDeprecatedInfoTracker.removePrefixes(raInfo.second.prefixes); - mDeprecatedInfoTracker.removeDnses(raInfo.second.dnses); - } - mRaParams = raInfo.second; - assembleRaLocked(); - } - - maybeNotifyMulticastTransmitter(); - break; default: Log.e(TAG, "Unknown message, cmd = " + String.valueOf(msg.what)); break; @@ -359,8 +336,23 @@ public class RouterAdvertisementDaemon { /** Build new RA.*/ public void buildNewRa(RaParams deprecatedParams, RaParams newParams) { - final Pair raInfo = new Pair<>(deprecatedParams, newParams); - sendMessage(CMD_BUILD_NEW_RA, raInfo); + synchronized (mLock) { + if (deprecatedParams != null) { + mDeprecatedInfoTracker.putPrefixes(deprecatedParams.prefixes); + mDeprecatedInfoTracker.putDnses(deprecatedParams.dnses); + } + + if (newParams != null) { + // Process information that is no longer deprecated. + mDeprecatedInfoTracker.removePrefixes(newParams.prefixes); + mDeprecatedInfoTracker.removeDnses(newParams.dnses); + } + + mRaParams = newParams; + assembleRaLocked(); + } + + maybeNotifyMulticastTransmitter(); } /** Start router advertisement daemon. */ @@ -668,16 +660,11 @@ public class RouterAdvertisementDaemon { } private boolean sendMessage(int cmd) { - return sendMessage(cmd, null); - } - - private boolean sendMessage(int cmd, @Nullable Object obj) { if (mRaMessageHandler == null) { return false; } - return mRaMessageHandler.sendMessage( - Message.obtain(mRaMessageHandler, cmd, obj)); + return mRaMessageHandler.sendMessage(Message.obtain(mRaMessageHandler, cmd)); } // TODO: Consider moving this to run on a provided Looper as a Handler, From ec801b887f5772731858253367a3421fb80e76c1 Mon Sep 17 00:00:00 2001 From: KH Shi Date: Wed, 20 Dec 2023 06:35:15 +0000 Subject: [PATCH 2/2] Revert "Refactor RouterAdvertisementDaemon with FdEventsReader" This reverts commit bdcd7db9d851b2951d63dd1ad3768a8d667f5c7e. Reason for revert: Design changes, don't use extra Handler in RouterAdvertisementDaemon. Change-Id: I4888f8f22aea2cadbb6064e63f872c605025930f --- .../net/ip/RouterAdvertisementDaemon.java | 138 +++++------------- 1 file changed, 33 insertions(+), 105 deletions(-) diff --git a/Tethering/src/android/net/ip/RouterAdvertisementDaemon.java b/Tethering/src/android/net/ip/RouterAdvertisementDaemon.java index d8779b83c0..50d6c4b18d 100644 --- a/Tethering/src/android/net/ip/RouterAdvertisementDaemon.java +++ b/Tethering/src/android/net/ip/RouterAdvertisementDaemon.java @@ -18,7 +18,6 @@ package android.net.ip; import static android.system.OsConstants.AF_INET6; import static android.system.OsConstants.IPPROTO_ICMPV6; -import static android.system.OsConstants.SOCK_NONBLOCK; import static android.system.OsConstants.SOCK_RAW; import static android.system.OsConstants.SOL_SOCKET; import static android.system.OsConstants.SO_SNDTIMEO; @@ -39,19 +38,12 @@ import android.net.LinkAddress; import android.net.MacAddress; import android.net.TrafficStats; import android.net.util.SocketUtils; -import android.os.Handler; -import android.os.HandlerThread; -import android.os.Looper; -import android.os.Message; import android.system.ErrnoException; import android.system.Os; import android.system.StructTimeval; import android.util.Log; -import androidx.annotation.NonNull; - import com.android.internal.annotations.GuardedBy; -import com.android.net.module.util.FdEventsReader; import com.android.net.module.util.InterfaceParams; import com.android.net.module.util.structs.Icmpv6Header; import com.android.net.module.util.structs.LlaOption; @@ -111,10 +103,6 @@ public class RouterAdvertisementDaemon { private static final int DAY_IN_SECONDS = 86_400; - // Commands for IpServer to control RouterAdvertisementDaemon - private static final int CMD_START = 1; - private static final int CMD_STOP = 2; - private final InterfaceParams mInterface; private final InetSocketAddress mAllNodes; @@ -132,13 +120,9 @@ public class RouterAdvertisementDaemon { @GuardedBy("mLock") private RaParams mRaParams; - // To be accessed only from RaMessageHandler - private RsPacketListener mRsPacketListener; - private volatile FileDescriptor mSocket; private volatile MulticastTransmitter mMulticastTransmitter; - private volatile RaMessageHandler mRaMessageHandler; - private volatile HandlerThread mRaHandlerThread; + private volatile UnicastResponder mUnicastResponder; /** Encapsulate the RA parameters for RouterAdvertisementDaemon.*/ public static class RaParams { @@ -260,74 +244,6 @@ public class RouterAdvertisementDaemon { } } - private class RaMessageHandler extends Handler { - RaMessageHandler(Looper looper) { - super(looper); - } - - @Override - public void handleMessage(Message msg) { - switch (msg.what) { - case CMD_START: - mRsPacketListener = new RsPacketListener(this); - mRsPacketListener.start(); - break; - case CMD_STOP: - if (mRsPacketListener != null) { - mRsPacketListener.stop(); - mRsPacketListener = null; - } - break; - default: - Log.e(TAG, "Unknown message, cmd = " + String.valueOf(msg.what)); - break; - } - } - } - - private class RsPacketListener extends FdEventsReader { - private static final class RecvBuffer { - // The recycled buffer for receiving Router Solicitations from clients. - // If the RS is larger than IPV6_MIN_MTU the packets are truncated. - // This is fine since currently only byte 0 is examined anyway. - final byte[] mBytes = new byte[IPV6_MIN_MTU]; - final InetSocketAddress mSrcAddr = new InetSocketAddress(0); - } - - RsPacketListener(@NonNull Handler handler) { - super(handler, new RecvBuffer()); - } - - @Override - protected int recvBufSize(@NonNull RecvBuffer buffer) { - return buffer.mBytes.length; - } - - @Override - protected FileDescriptor createFd() { - return mSocket; - } - - @Override - protected int readPacket(@NonNull FileDescriptor fd, @NonNull RecvBuffer buffer) - throws Exception { - return Os.recvfrom( - fd, buffer.mBytes, 0, buffer.mBytes.length, 0 /* flags */, buffer.mSrcAddr); - } - - @Override - protected final void handlePacket(@NonNull RecvBuffer buffer, int length) { - // Do the least possible amount of validations. - if (buffer.mSrcAddr == null - || length <= 0 - || buffer.mBytes[0] != asByte(ICMPV6_ROUTER_SOLICITATION)) { - return; - } - - maybeSendRA(buffer.mSrcAddr); - } - } - public RouterAdvertisementDaemon(InterfaceParams ifParams) { mInterface = ifParams; mAllNodes = new InetSocketAddress(getAllNodesForScopeId(mInterface.index), 0); @@ -358,36 +274,26 @@ public class RouterAdvertisementDaemon { /** Start router advertisement daemon. */ public boolean start() { if (!createSocket()) { - Log.e(TAG, "Failed to start RouterAdvertisementDaemon."); return false; } mMulticastTransmitter = new MulticastTransmitter(); mMulticastTransmitter.start(); - mRaHandlerThread = new HandlerThread(TAG); - mRaHandlerThread.start(); - mRaMessageHandler = new RaMessageHandler(mRaHandlerThread.getLooper()); + mUnicastResponder = new UnicastResponder(); + mUnicastResponder.start(); - return sendMessage(CMD_START); + return true; } /** Stop router advertisement daemon. */ public void stop() { - if (!sendMessage(CMD_STOP)) { - Log.e(TAG, "RouterAdvertisementDaemon has been stopped or was never started."); - return; - } - - mRaHandlerThread.quitSafely(); - mRaHandlerThread = null; - mRaMessageHandler = null; - closeSocket(); // Wake up mMulticastTransmitter thread to interrupt a potential 1 day sleep before // the thread's termination. maybeNotifyMulticastTransmitter(); mMulticastTransmitter = null; + mUnicastResponder = null; } @GuardedBy("mLock") @@ -597,7 +503,7 @@ public class RouterAdvertisementDaemon { final int oldTag = TrafficStats.getAndSetThreadStatsTag(TAG_SYSTEM_NEIGHBOR); try { - mSocket = Os.socket(AF_INET6, SOCK_RAW | SOCK_NONBLOCK, IPPROTO_ICMPV6); + mSocket = Os.socket(AF_INET6, SOCK_RAW, IPPROTO_ICMPV6); // Setting SNDTIMEO is purely for defensive purposes. Os.setsockoptTimeval( mSocket, SOL_SOCKET, SO_SNDTIMEO, StructTimeval.fromMillis(send_timout_ms)); @@ -659,12 +565,34 @@ public class RouterAdvertisementDaemon { } } - private boolean sendMessage(int cmd) { - if (mRaMessageHandler == null) { - return false; - } + private final class UnicastResponder extends Thread { + private final InetSocketAddress mSolicitor = new InetSocketAddress(0); + // The recycled buffer for receiving Router Solicitations from clients. + // If the RS is larger than IPV6_MIN_MTU the packets are truncated. + // This is fine since currently only byte 0 is examined anyway. + private final byte[] mSolicitation = new byte[IPV6_MIN_MTU]; - return mRaMessageHandler.sendMessage(Message.obtain(mRaMessageHandler, cmd)); + @Override + public void run() { + while (isSocketValid()) { + try { + // Blocking receive. + final int rval = Os.recvfrom( + mSocket, mSolicitation, 0, mSolicitation.length, 0, mSolicitor); + // Do the least possible amount of validation. + if (rval < 1 || mSolicitation[0] != asByte(ICMPV6_ROUTER_SOLICITATION)) { + continue; + } + } catch (ErrnoException | SocketException e) { + if (isSocketValid()) { + Log.e(TAG, "recvfrom error: " + e); + } + continue; + } + + maybeSendRA(mSolicitor); + } + } } // TODO: Consider moving this to run on a provided Looper as a Handler,