From 676c84ef98092e39b158dfad86f1c6f5fc5a1e47 Mon Sep 17 00:00:00 2001 From: chiachangwang Date: Tue, 14 Feb 2023 09:22:05 +0000 Subject: [PATCH] Add underpinned Network parameter in SocketKeepalive.start() Take a Network parameter to have an one-to-one mapping between keepalive and its underpinned network on the automatic keepalive. Existing design could not really tell which network should the automatic keepalive check for the TCP socket status if there are multiple automatic keepalives enabled, e.g. Bothe IWLAN and VPN on WiFi enable the automatic keepalive. The keepalive for IWLAN should check if there are any TCP sockets on the IWLAN network instead of VPN network. Bug: 259000745 Test: atest FrameworksNetTests Test: Cts in the follow up commit Change-Id: I7353f4ef43e8fdad02c4d4a0bb5f6efa7d94c1b4 --- framework/api/system-current.txt | 2 +- .../src/android/net/IConnectivityManager.aidl | 2 +- .../src/android/net/NattSocketKeepalive.java | 7 +++-- .../src/android/net/SocketKeepalive.java | 10 ++++--- .../src/android/net/TcpSocketKeepalive.java | 2 +- .../android/server/ConnectivityService.java | 27 ++++++++++--------- .../AutomaticOnOffKeepaliveTracker.java | 24 ++++++++++++----- 7 files changed, 46 insertions(+), 28 deletions(-) diff --git a/framework/api/system-current.txt b/framework/api/system-current.txt index 0b039837b6..196e023d2a 100644 --- a/framework/api/system-current.txt +++ b/framework/api/system-current.txt @@ -470,7 +470,7 @@ package android.net { } public abstract class SocketKeepalive implements java.lang.AutoCloseable { - method public final void start(@IntRange(from=0xa, to=0xe10) int, int); + method public final void start(@IntRange(from=0xa, to=0xe10) int, int, @NonNull android.net.Network); field public static final int ERROR_NO_SUCH_SLOT = -33; // 0xffffffdf field public static final int FLAG_AUTOMATIC_ON_OFF = 1; // 0x1 field public static final int SUCCESS = 0; // 0x0 diff --git a/framework/src/android/net/IConnectivityManager.aidl b/framework/src/android/net/IConnectivityManager.aidl index c8f588d1ad..db001f921b 100644 --- a/framework/src/android/net/IConnectivityManager.aidl +++ b/framework/src/android/net/IConnectivityManager.aidl @@ -188,7 +188,7 @@ interface IConnectivityManager void startNattKeepaliveWithFd(in Network network, in ParcelFileDescriptor pfd, int resourceId, int intervalSeconds, in ISocketKeepaliveCallback cb, String srcAddr, - String dstAddr, boolean automaticOnOffKeepalives); + String dstAddr, boolean automaticOnOffKeepalives, in Network underpinnedNetwork); void startTcpKeepalive(in Network network, in ParcelFileDescriptor pfd, int intervalSeconds, in ISocketKeepaliveCallback cb); diff --git a/framework/src/android/net/NattSocketKeepalive.java b/framework/src/android/net/NattSocketKeepalive.java index 77137f444a..a83b5b4ac6 100644 --- a/framework/src/android/net/NattSocketKeepalive.java +++ b/framework/src/android/net/NattSocketKeepalive.java @@ -66,10 +66,12 @@ public final class NattSocketKeepalive extends SocketKeepalive { * the supplied {@link Callback} will see a call to * {@link Callback#onError(int)} with {@link #ERROR_INVALID_INTERVAL}. * @param flags Flags to enable/disable available options on this keepalive. + * @param underpinnedNetwork The underpinned network of this keepalive. + * * @hide */ @Override - protected void startImpl(int intervalSec, int flags) { + protected void startImpl(int intervalSec, int flags, Network underpinnedNetwork) { if (0 != (flags & ~FLAG_AUTOMATIC_ON_OFF)) { throw new IllegalArgumentException("Illegal flag value for " + this.getClass().getSimpleName() + " : " + flags); @@ -79,7 +81,8 @@ public final class NattSocketKeepalive extends SocketKeepalive { try { mService.startNattKeepaliveWithFd(mNetwork, mPfd, mResourceId, intervalSec, mCallback, mSource.getHostAddress(), - mDestination.getHostAddress(), automaticOnOffKeepalives); + mDestination.getHostAddress(), automaticOnOffKeepalives, + underpinnedNetwork); } catch (RemoteException e) { Log.e(TAG, "Error starting socket keepalive: ", e); throw e.rethrowFromSystemServer(); diff --git a/framework/src/android/net/SocketKeepalive.java b/framework/src/android/net/SocketKeepalive.java index 00104f6432..311126ec0a 100644 --- a/framework/src/android/net/SocketKeepalive.java +++ b/framework/src/android/net/SocketKeepalive.java @@ -355,7 +355,7 @@ public abstract class SocketKeepalive implements AutoCloseable { */ public final void start(@IntRange(from = MIN_INTERVAL_SEC, to = MAX_INTERVAL_SEC) int intervalSec) { - startImpl(intervalSec, 0 /* flags */); + startImpl(intervalSec, 0 /* flags */, null /* underpinnedNetwork */); } /** @@ -374,16 +374,18 @@ public abstract class SocketKeepalive implements AutoCloseable { * the supplied {@link Callback} will see a call to * {@link Callback#onError(int)} with {@link #ERROR_INVALID_INTERVAL}. * @param flags Flags to enable/disable available options on this keepalive. + * @param underpinnedNetwork The underpinned network of this keepalive. * @hide */ @SystemApi(client = PRIVILEGED_APPS) public final void start(@IntRange(from = MIN_INTERVAL_SEC, to = MAX_INTERVAL_SEC) - int intervalSec, @StartFlags int flags) { - startImpl(intervalSec, flags); + int intervalSec, @StartFlags int flags, @NonNull Network underpinnedNetwork) { + startImpl(intervalSec, flags, underpinnedNetwork); } /** @hide */ - protected abstract void startImpl(int intervalSec, @StartFlags int flags); + protected abstract void startImpl(int intervalSec, @StartFlags int flags, + Network underpinnedNetwork); /** * Requests that keepalive be stopped. The application must wait for {@link Callback#onStopped} diff --git a/framework/src/android/net/TcpSocketKeepalive.java b/framework/src/android/net/TcpSocketKeepalive.java index cda5830485..b548f6d2f1 100644 --- a/framework/src/android/net/TcpSocketKeepalive.java +++ b/framework/src/android/net/TcpSocketKeepalive.java @@ -50,7 +50,7 @@ public final class TcpSocketKeepalive extends SocketKeepalive { * acknowledgement. */ @Override - protected void startImpl(int intervalSec, int flags) { + protected void startImpl(int intervalSec, int flags, Network underpinnedNetwork) { if (0 != flags) { throw new IllegalArgumentException("Illegal flag value for " + this.getClass().getSimpleName() + " : " + flags); diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 1332f32b2d..3dcc448972 100755 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -5589,13 +5589,14 @@ public class ConnectivityService extends IConnectivityManager.Stub mKeepaliveTracker.getKeepaliveForBinder((IBinder) msg.obj); if (null == ki) return; // The callback was unregistered before the alarm fired + final Network underpinnedNetwork = ki.getUnderpinnedNetwork(); final Network network = ki.getNetwork(); boolean networkFound = false; - final ArrayList vpnsRunningOnThisNetwork = new ArrayList<>(); + boolean underpinnedNetworkFound = false; for (NetworkAgentInfo n : mNetworkAgentInfos) { if (n.network.equals(network)) networkFound = true; - if (n.isVPN() && n.everConnected() && hasUnderlyingNetwork(n, network)) { - vpnsRunningOnThisNetwork.add(n); + if (n.everConnected() && n.network.equals(underpinnedNetwork)) { + underpinnedNetworkFound = true; } } @@ -5603,12 +5604,11 @@ public class ConnectivityService extends IConnectivityManager.Stub // cleaned up already. There is no point trying to resume keepalives. if (!networkFound) return; - if (!vpnsRunningOnThisNetwork.isEmpty()) { + if (underpinnedNetworkFound) { mKeepaliveTracker.handleMonitorAutomaticKeepalive(ki, - // TODO: check all the VPNs running on top of this network - vpnsRunningOnThisNetwork.get(0).network.netId); + underpinnedNetwork.netId); } else { - // If no VPN, then make sure the keepalive is running. + // If no underpinned network, then make sure the keepalive is running. mKeepaliveTracker.handleMaybeResumeKeepalive(ki); } break; @@ -9868,21 +9868,22 @@ public class ConnectivityService extends IConnectivityManager.Stub getNetworkAgentInfoForNetwork(network), null /* fd */, intervalSeconds, cb, srcAddr, srcPort, dstAddr, NattSocketKeepalive.NATT_PORT, // Keep behavior of the deprecated method as it is. Set automaticOnOffKeepalives to - // false because there is no way and no plan to configure automaticOnOffKeepalives - // in this deprecated method. - false /* automaticOnOffKeepalives */); + // false and set the underpinned network to null because there is no way and no + // plan to configure automaticOnOffKeepalives or underpinnedNetwork in this + // deprecated method. + false /* automaticOnOffKeepalives */, null /* underpinnedNetwork */); } @Override public void startNattKeepaliveWithFd(Network network, ParcelFileDescriptor pfd, int resourceId, int intervalSeconds, ISocketKeepaliveCallback cb, String srcAddr, - String dstAddr, boolean automaticOnOffKeepalives) { + String dstAddr, boolean automaticOnOffKeepalives, Network underpinnedNetwork) { try { final FileDescriptor fd = pfd.getFileDescriptor(); mKeepaliveTracker.startNattKeepalive( getNetworkAgentInfoForNetwork(network), fd, resourceId, - intervalSeconds, cb, - srcAddr, dstAddr, NattSocketKeepalive.NATT_PORT, automaticOnOffKeepalives); + intervalSeconds, cb, srcAddr, dstAddr, NattSocketKeepalive.NATT_PORT, + automaticOnOffKeepalives, underpinnedNetwork); } finally { // FileDescriptors coming from AIDL calls must be manually closed to prevent leaks. // startNattKeepalive calls Os.dup(fd) before returning, so we can close immediately. diff --git a/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java b/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java index daabfd8564..760a5f3917 100644 --- a/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java +++ b/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java @@ -196,11 +196,16 @@ public class AutomaticOnOffKeepaliveTracker { private final PendingIntent mTcpPollingAlarm; @AutomaticOnOffState private int mAutomaticOnOffState; + @Nullable + private final Network mUnderpinnedNetwork; AutomaticOnOffKeepalive(@NonNull final KeepaliveTracker.KeepaliveInfo ki, - final boolean autoOnOff, @NonNull Context context) throws InvalidSocketException { + final boolean autoOnOff, @NonNull Context context, + @Nullable Network underpinnedNetwork) + throws InvalidSocketException { this.mKi = Objects.requireNonNull(ki); mCallback = ki.mCallback; + mUnderpinnedNetwork = underpinnedNetwork; if (autoOnOff && mDependencies.isFeatureEnabled(AUTOMATIC_ON_OFF_KEEPALIVE_VERSION)) { mAutomaticOnOffState = STATE_ENABLED; if (null == ki.mFd) { @@ -228,6 +233,11 @@ public class AutomaticOnOffKeepaliveTracker { return mKi.getNai().network; } + @Nullable + public Network getUnderpinnedNetwork() { + return mUnderpinnedNetwork; + } + public boolean match(Network network, int slot) { return mKi.getNai().network().equals(network) && mKi.getSlot() == slot; } @@ -475,13 +485,13 @@ public class AutomaticOnOffKeepaliveTracker { @NonNull String srcAddrString, int srcPort, @NonNull String dstAddrString, - int dstPort, boolean automaticOnOffKeepalives) { + int dstPort, boolean automaticOnOffKeepalives, @Nullable Network underpinnedNetwork) { final KeepaliveTracker.KeepaliveInfo ki = mKeepaliveTracker.makeNattKeepaliveInfo(nai, fd, intervalSeconds, cb, srcAddrString, srcPort, dstAddrString, dstPort); if (null == ki) return; try { final AutomaticOnOffKeepalive autoKi = new AutomaticOnOffKeepalive(ki, - automaticOnOffKeepalives, mContext); + automaticOnOffKeepalives, mContext, underpinnedNetwork); mConnectivityServiceHandler.obtainMessage(NetworkAgent.CMD_START_SOCKET_KEEPALIVE, // TODO : move ConnectivityService#encodeBool to a static lib. automaticOnOffKeepalives ? 1 : 0, 0, autoKi).sendToTarget(); @@ -504,13 +514,14 @@ public class AutomaticOnOffKeepaliveTracker { @NonNull String srcAddrString, @NonNull String dstAddrString, int dstPort, - boolean automaticOnOffKeepalives) { + boolean automaticOnOffKeepalives, + @Nullable Network underpinnedNetwork) { final KeepaliveTracker.KeepaliveInfo ki = mKeepaliveTracker.makeNattKeepaliveInfo(nai, fd, resourceId, intervalSeconds, cb, srcAddrString, dstAddrString, dstPort); if (null == ki) return; try { final AutomaticOnOffKeepalive autoKi = new AutomaticOnOffKeepalive(ki, - automaticOnOffKeepalives, mContext); + automaticOnOffKeepalives, mContext, underpinnedNetwork); mConnectivityServiceHandler.obtainMessage(NetworkAgent.CMD_START_SOCKET_KEEPALIVE, // TODO : move ConnectivityService#encodeBool to a static lib. automaticOnOffKeepalives ? 1 : 0, 0, autoKi).sendToTarget(); @@ -539,7 +550,8 @@ public class AutomaticOnOffKeepaliveTracker { if (null == ki) return; try { final AutomaticOnOffKeepalive autoKi = new AutomaticOnOffKeepalive(ki, - false /* autoOnOff, tcp keepalives are never auto on/off */, mContext); + false /* autoOnOff, tcp keepalives are never auto on/off */, + mContext, null /* underpinnedNetwork, tcp keepalives do not refer to this */); mConnectivityServiceHandler.obtainMessage(CMD_START_SOCKET_KEEPALIVE, autoKi) .sendToTarget(); } catch (InvalidSocketException e) {