Merge "Stop TCP keepalive from CS for fd initiated stop events"

This commit is contained in:
Chiachang Wang
2023-06-09 01:05:46 +00:00
committed by Gerrit Code Review
3 changed files with 104 additions and 29 deletions

View File

@@ -106,7 +106,12 @@ public class KeepaliveTracker {
private final int mAllowedUnprivilegedSlotsForUid;
public KeepaliveTracker(Context context, Handler handler) {
mTcpController = new TcpKeepaliveController(handler);
this(context, handler, new TcpKeepaliveController(handler));
}
@VisibleForTesting
KeepaliveTracker(Context context, Handler handler, TcpKeepaliveController tcpController) {
mTcpController = tcpController;
mContext = context;
mSupportedKeepalives = KeepaliveResourceUtil.getSupportedKeepalives(context);
@@ -375,7 +380,7 @@ public class KeepaliveTracker {
break;
case TYPE_TCP:
try {
mTcpController.startSocketMonitor(mFd, this, mSlot);
mTcpController.startSocketMonitor(mFd, mCallback, mSlot);
} catch (InvalidSocketException e) {
handleStopKeepalive(mNai, mSlot, ERROR_INVALID_SOCKET);
return ERROR_INVALID_SOCKET;
@@ -455,12 +460,6 @@ public class KeepaliveTracker {
}
}
// TODO: This does not clean up the autoKi in AutomaticOnOffKeepaliveTracker and it is not
// possible without a big refactor.
void onFileDescriptorInitiatedStop(final int socketKeepaliveReason) {
handleStopKeepalive(mNai, mSlot, socketKeepaliveReason);
}
/**
* Construct a new KeepaliveInfo from existing KeepaliveInfo with a new fd.
*/

View File

@@ -15,6 +15,7 @@
*/
package com.android.server.connectivity;
import static android.net.NetworkAgent.CMD_STOP_SOCKET_KEEPALIVE;
import static android.net.SocketKeepalive.DATA_RECEIVED;
import static android.net.SocketKeepalive.ERROR_INVALID_IP_ADDRESS;
import static android.net.SocketKeepalive.ERROR_INVALID_SOCKET;
@@ -33,6 +34,7 @@ import static android.system.OsConstants.TIOCOUTQ;
import static com.android.net.module.util.NetworkStackConstants.IPV4_HEADER_MIN_LEN;
import android.annotation.NonNull;
import android.net.ISocketKeepaliveCallback;
import android.net.InvalidPacketException;
import android.net.NetworkUtils;
import android.net.SocketKeepalive.InvalidSocketException;
@@ -50,7 +52,6 @@ import android.util.SparseArray;
import com.android.internal.annotations.GuardedBy;
import com.android.internal.annotations.VisibleForTesting;
import com.android.net.module.util.IpUtils;
import com.android.server.connectivity.KeepaliveTracker.KeepaliveInfo;
import java.io.FileDescriptor;
import java.net.InetAddress;
@@ -88,6 +89,8 @@ public class TcpKeepaliveController {
private final MessageQueue mFdHandlerQueue;
private final Handler mConnectivityServiceHandler;
private static final int FD_EVENTS = EVENT_INPUT | EVENT_ERROR;
private static final int TCP_HEADER_LENGTH = 20;
@@ -115,6 +118,7 @@ public class TcpKeepaliveController {
public TcpKeepaliveController(final Handler connectivityServiceHandler) {
mFdHandlerQueue = connectivityServiceHandler.getLooper().getQueue();
mConnectivityServiceHandler = connectivityServiceHandler;
}
/** Build tcp keepalive packet. */
@@ -324,12 +328,13 @@ public class TcpKeepaliveController {
* Start monitoring incoming packets.
*
* @param fd socket fd to monitor.
* @param ki a {@link KeepaliveInfo} that tracks information about a socket keepalive.
* @param callback a {@link ISocketKeepaliveCallback} that tracks information about a socket
* keepalive.
* @param slot keepalive slot.
*/
public void startSocketMonitor(@NonNull final FileDescriptor fd,
@NonNull final KeepaliveInfo ki, final int slot)
throws IllegalArgumentException, InvalidSocketException {
public void startSocketMonitor(
@NonNull final FileDescriptor fd, @NonNull final ISocketKeepaliveCallback callback,
final int slot) throws IllegalArgumentException, InvalidSocketException {
synchronized (mListeners) {
if (null != mListeners.get(slot)) {
throw new IllegalArgumentException("This slot is already taken");
@@ -350,7 +355,8 @@ public class TcpKeepaliveController {
} else {
reason = DATA_RECEIVED;
}
ki.onFileDescriptorInitiatedStop(reason);
mConnectivityServiceHandler.obtainMessage(CMD_STOP_SOCKET_KEEPALIVE,
0 /* unused */, reason, callback.asBinder()).sendToTarget();
// The listener returns the new set of events to listen to. Because 0 means no
// event, the listener gets unregistered.
return 0;

View File

@@ -18,6 +18,7 @@ package com.android.server.connectivity;
import static android.content.pm.PackageManager.PERMISSION_GRANTED;
import static android.net.ConnectivityManager.TYPE_MOBILE;
import static android.net.NetworkAgent.CMD_STOP_SOCKET_KEEPALIVE;
import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR;
import static com.android.testutils.HandlerUtils.visibleOnHandlerThread;
@@ -28,6 +29,7 @@ import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyInt;
@@ -47,6 +49,7 @@ import android.content.Context;
import android.content.res.Resources;
import android.net.INetd;
import android.net.ISocketKeepaliveCallback;
import android.net.KeepalivePacketData;
import android.net.LinkAddress;
import android.net.LinkProperties;
import android.net.MarkMaskParcel;
@@ -55,6 +58,7 @@ import android.net.Network;
import android.net.NetworkCapabilities;
import android.net.NetworkInfo;
import android.net.SocketKeepalive;
import android.net.TcpKeepalivePacketData;
import android.os.Binder;
import android.os.Build;
import android.os.Handler;
@@ -120,6 +124,7 @@ public class AutomaticOnOffKeepaliveTrackerTest {
TestKeepaliveTracker mKeepaliveTracker;
AOOTestHandler mTestHandler;
TestTcpKeepaliveController mTcpController;
// Hexadecimal representation of a SOCK_DIAG response with tcp info.
private static final String SOCK_DIAG_TCP_INET_HEX =
@@ -233,9 +238,9 @@ public class AutomaticOnOffKeepaliveTrackerTest {
public final FileDescriptor fd;
public final ISocketKeepaliveCallback socketKeepaliveCallback;
public final Network underpinnedNetwork;
public final NattKeepalivePacketData kpd;
public final KeepalivePacketData kpd;
TestKeepaliveInfo(NattKeepalivePacketData kpd) throws Exception {
TestKeepaliveInfo(KeepalivePacketData kpd) throws Exception {
this.kpd = kpd;
socket = new Socket();
socket.bind(null);
@@ -252,8 +257,9 @@ public class AutomaticOnOffKeepaliveTrackerTest {
private class TestKeepaliveTracker extends KeepaliveTracker {
private KeepaliveInfo mKi;
TestKeepaliveTracker(@NonNull final Context context, @NonNull final Handler handler) {
super(context, handler);
TestKeepaliveTracker(@NonNull final Context context, @NonNull final Handler handler,
@NonNull final TcpKeepaliveController tcpController) {
super(context, handler, tcpController);
}
public void setReturnedKeepaliveInfo(@NonNull final KeepaliveInfo ki) {
@@ -272,6 +278,24 @@ public class AutomaticOnOffKeepaliveTrackerTest {
}
return mKi;
}
@NonNull
@Override
public KeepaliveInfo makeTcpKeepaliveInfo(@Nullable final NetworkAgentInfo nai,
@Nullable final FileDescriptor fd, final int intervalSeconds,
@NonNull final ISocketKeepaliveCallback cb) {
if (null == mKi) {
throw new IllegalStateException("Please call `setReturnedKeepaliveInfo`"
+ " before makeTcpKeepaliveInfo is called");
}
return mKi;
}
}
private static class TestTcpKeepaliveController extends TcpKeepaliveController {
TestTcpKeepaliveController(final Handler connectivityServiceHandler) {
super(connectivityServiceHandler);
}
}
@Before
@@ -303,7 +327,8 @@ public class AutomaticOnOffKeepaliveTrackerTest {
mHandlerThread = new HandlerThread("KeepaliveTrackerTest");
mHandlerThread.start();
mTestHandler = new AOOTestHandler(mHandlerThread.getLooper());
mKeepaliveTracker = new TestKeepaliveTracker(mCtx, mTestHandler);
mTcpController = new TestTcpKeepaliveController(mTestHandler);
mKeepaliveTracker = new TestKeepaliveTracker(mCtx, mTestHandler, mTcpController);
doReturn(mKeepaliveTracker).when(mDependencies).newKeepaliveTracker(mCtx, mTestHandler);
doReturn(true).when(mDependencies).isFeatureEnabled(any(), anyBoolean());
mAOOKeepaliveTracker =
@@ -333,6 +358,14 @@ public class AutomaticOnOffKeepaliveTrackerTest {
Log.d(TAG, "Test handler received CMD_MONITOR_AUTOMATIC_KEEPALIVE : " + msg);
mLastAutoKi = mAOOKeepaliveTracker.getKeepaliveForBinder((IBinder) msg.obj);
break;
case CMD_STOP_SOCKET_KEEPALIVE:
Log.d(TAG, "Test handler received CMD_STOP_SOCKET_KEEPALIVE : " + msg);
mLastAutoKi = mAOOKeepaliveTracker.getKeepaliveForBinder((IBinder) msg.obj);
if (mLastAutoKi == null) {
fail("Attempt to stop an already stopped keepalive");
}
mAOOKeepaliveTracker.handleStopKeepalive(mLastAutoKi, msg.arg2);
break;
}
}
}
@@ -481,14 +514,15 @@ public class AutomaticOnOffKeepaliveTrackerTest {
mTestHandler, () -> mAOOKeepaliveTracker.getKeepaliveForBinder(binder));
}
private void checkAndProcessKeepaliveStart(final NattKeepalivePacketData kpd) throws Exception {
private void checkAndProcessKeepaliveStart(final KeepalivePacketData kpd) throws Exception {
checkAndProcessKeepaliveStart(TEST_SLOT, kpd);
}
private void checkAndProcessKeepaliveStart(
int slot, final NattKeepalivePacketData kpd) throws Exception {
verify(mNai).onStartNattSocketKeepalive(slot, TEST_KEEPALIVE_INTERVAL_SEC, kpd);
verify(mNai).onAddNattKeepalivePacketFilter(slot, kpd);
int slot, final KeepalivePacketData kpd) throws Exception {
verify(mNai).onStartNattSocketKeepalive(
slot, TEST_KEEPALIVE_INTERVAL_SEC, (NattKeepalivePacketData) kpd);
verify(mNai).onAddNattKeepalivePacketFilter(slot, (NattKeepalivePacketData) kpd);
triggerEventKeepalive(slot, SocketKeepalive.SUCCESS);
}
@@ -531,9 +565,10 @@ public class AutomaticOnOffKeepaliveTrackerTest {
public void testHandleEventSocketKeepalive_startingFailureHardwareError() throws Exception {
final TestKeepaliveInfo testInfo = doStartNattKeepalive();
verify(mNai)
.onStartNattSocketKeepalive(TEST_SLOT, TEST_KEEPALIVE_INTERVAL_SEC, testInfo.kpd);
verify(mNai).onAddNattKeepalivePacketFilter(TEST_SLOT, testInfo.kpd);
verify(mNai).onStartNattSocketKeepalive(
TEST_SLOT, TEST_KEEPALIVE_INTERVAL_SEC, (NattKeepalivePacketData) testInfo.kpd);
verify(mNai).onAddNattKeepalivePacketFilter(
TEST_SLOT, (NattKeepalivePacketData) testInfo.kpd);
// Network agent returns an error, fails to start the keepalive.
triggerEventKeepalive(TEST_SLOT, SocketKeepalive.ERROR_HARDWARE_ERROR);
@@ -674,9 +709,10 @@ public class AutomaticOnOffKeepaliveTrackerTest {
clearInvocations(mNai);
doResumeKeepalive(getAutoKiForBinder(testInfo.binder));
verify(mNai)
.onStartNattSocketKeepalive(TEST_SLOT, TEST_KEEPALIVE_INTERVAL_SEC, testInfo.kpd);
verify(mNai).onAddNattKeepalivePacketFilter(TEST_SLOT, testInfo.kpd);
verify(mNai).onStartNattSocketKeepalive(
TEST_SLOT, TEST_KEEPALIVE_INTERVAL_SEC, (NattKeepalivePacketData) testInfo.kpd);
verify(mNai).onAddNattKeepalivePacketFilter(
TEST_SLOT, (NattKeepalivePacketData) testInfo.kpd);
// Network agent returns error on starting the keepalive.
triggerEventKeepalive(TEST_SLOT, SocketKeepalive.ERROR_HARDWARE_ERROR);
@@ -772,4 +808,38 @@ public class AutomaticOnOffKeepaliveTrackerTest {
verifyNoMoreInteractions(ignoreStubs(testInfo1.socketKeepaliveCallback));
verifyNoMoreInteractions(ignoreStubs(testInfo2.socketKeepaliveCallback));
}
@Test
public void testStartTcpKeepalive_fdInitiatedStop() throws Exception {
final InetAddress srcAddress = InetAddress.getByAddress(
new byte[] { (byte) 192, 0, 0, (byte) 129 });
mNai.linkProperties.addLinkAddress(new LinkAddress(srcAddress, 24));
final KeepalivePacketData kpd = new TcpKeepalivePacketData(
InetAddress.getByAddress(new byte[] { (byte) 192, 0, 0, (byte) 129 }) /* srcAddr */,
12345 /* srcPort */,
InetAddress.getByAddress(new byte[] { 8, 8, 8, 8}) /* dstAddr */,
12345 /* dstPort */, new byte[] {1}, 111 /* tcpSeq */,
222 /* tcpAck */, 800 /* tcpWindow */, 2 /* tcpWindowScale */,
4 /* ipTos */, 64 /* ipTtl */);
final TestKeepaliveInfo testInfo = new TestKeepaliveInfo(kpd);
final KeepaliveInfo ki = mKeepaliveTracker.new KeepaliveInfo(
testInfo.socketKeepaliveCallback, mNai, kpd,
TEST_KEEPALIVE_INTERVAL_SEC, KeepaliveInfo.TYPE_TCP, testInfo.fd);
mKeepaliveTracker.setReturnedKeepaliveInfo(ki);
// Setup TCP keepalive.
mAOOKeepaliveTracker.startTcpKeepalive(mNai, testInfo.fd, TEST_KEEPALIVE_INTERVAL_SEC,
testInfo.socketKeepaliveCallback);
HandlerUtils.waitForIdle(mTestHandler, TIMEOUT_MS);
// A closed socket will result in EVENT_HANGUP and trigger error to
// FileDescriptorEventListener.
testInfo.socket.close();
HandlerUtils.waitForIdle(mTestHandler, TIMEOUT_MS);
// The keepalive should be removed in AutomaticOnOffKeepaliveTracker.
getAutoKiForBinder(testInfo.binder);
}
}