diff options
4 files changed, 119 insertions, 13 deletions
diff --git a/src/com/android/networkstack/netlink/TcpSocketTracker.java b/src/com/android/networkstack/netlink/TcpSocketTracker.java index 8f24aec..8929cdb 100644 --- a/src/com/android/networkstack/netlink/TcpSocketTracker.java +++ b/src/com/android/networkstack/netlink/TcpSocketTracker.java @@ -39,6 +39,9 @@ import static android.system.OsConstants.SOL_SOCKET; import static android.system.OsConstants.SO_SNDTIMEO; import android.content.Context; +import android.net.INetd; +import android.net.MarkMaskParcel; +import android.net.Network; import android.net.netlink.NetlinkConstants; import android.net.netlink.NetlinkSocket; import android.net.netlink.StructInetDiagMsg; @@ -46,6 +49,8 @@ import android.net.netlink.StructNlMsgHdr; import android.net.util.NetworkStackUtils; import android.net.util.SocketUtils; import android.os.AsyncTask; +import android.os.IBinder; +import android.os.RemoteException; import android.os.SystemClock; import android.provider.DeviceConfig; import android.system.ErrnoException; @@ -59,6 +64,8 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import com.android.internal.annotations.VisibleForTesting; +import com.android.networkstack.apishim.NetworkShimImpl; +import com.android.networkstack.apishim.UnsupportedApiLevelException; import java.io.FileDescriptor; import java.io.InterruptedIOException; @@ -84,6 +91,8 @@ public class TcpSocketTracker { private static final long IO_TIMEOUT = 3_000L; /** Cookie offset of an InetMagMessage header. */ private static final int IDIAG_COOKIE_OFFSET = 44; + private static final int UNKNOWN_MARK = 0xffffffff; + private static final int NULL_MASK = 0; /** * Gather the socket info. * @@ -106,6 +115,12 @@ public class TcpSocketTracker { */ private final SparseArray<byte[]> mSockDiagMsg = new SparseArray<>(); private final Dependencies mDependencies; + private final INetd mNetd; + private final Network mNetwork; + // The fwmark value of {@code mNetwork}. + private final int mNetworkMark; + // The network id mask of fwmark. + private final int mNetworkMask; private int mMinPacketsThreshold = DEFAULT_DATA_STALL_MIN_PACKETS_THRESHOLD; private int mTcpPacketsFailRateThreshold = DEFAULT_TCP_PACKETS_FAIL_PERCENTAGE; @VisibleForTesting @@ -124,8 +139,17 @@ public class TcpSocketTracker { } }; - public TcpSocketTracker(@NonNull final Dependencies dps) { + public TcpSocketTracker(@NonNull final Dependencies dps, @NonNull final Network network) { mDependencies = dps; + mNetwork = network; + mNetd = mDependencies.getNetd(); + + // If the parcel is null, nothing should be matched which is achieved by the combination of + // {@code NULL_MASK} and {@code UNKNOWN_MARK}. + final MarkMaskParcel parcel = getNetworkMarkMask(); + mNetworkMark = (parcel != null) ? parcel.mark : UNKNOWN_MARK; + mNetworkMask = (parcel != null) ? parcel.mask : NULL_MASK; + // Request tcp info from NetworkStack directly needs extra SELinux permission added after Q // release. if (!mDependencies.isTcpInfoParsingSupported()) return; @@ -145,12 +169,24 @@ public class TcpSocketTracker { mDependencies.addDeviceConfigChangedListener(mConfigListener); } + @Nullable + private MarkMaskParcel getNetworkMarkMask() { + try { + final int netId = NetworkShimImpl.newInstance(mNetwork).getNetId(); + return mNetd.getFwmarkForNetwork(netId); + } catch (UnsupportedApiLevelException e) { + log("Get netId is not available in this API level."); + } catch (RemoteException e) { + Log.e(TAG, "Error getting fwmark for network, ", e); + } + return null; + } + /** * Request to send a SockDiag Netlink request. Receive and parse the returned message. This * function is not thread-safe and should only be called from only one thread. * * @Return if this polling request executes successfully or not. - * TODO: Need to filter socket info based on the target network. */ public boolean pollSocketsInfo() { if (!mDependencies.isTcpInfoParsingSupported()) return false; @@ -294,6 +330,10 @@ public class TcpSocketTracker { private TcpStat calculateLatestPacketsStat(@NonNull final SocketInfo current, @Nullable final SocketInfo previous) { final TcpStat stat = new TcpStat(); + // Ignore non-target network sockets. + if ((current.fwmark & mNetworkMask) != mNetworkMark) { + return null; + } if (current.tcpInfo == null) { log("Current tcpInfo is null."); @@ -428,7 +468,6 @@ public class TcpSocketTracker { // One of {@code AF_INET6, AF_INET}. public final int ipFamily; // "fwmark" value of the socket queried from native. - // TODO: Used to do bit-wise '&' operation to get netId information. public final int fwmark; // Socket information updated elapsed real time. public final long updateTime; @@ -554,6 +593,14 @@ public class TcpSocketTracker { return mContext; } + /** + * Get an INetd connector. + */ + public INetd getNetd() { + return INetd.Stub.asInterface( + (IBinder) mContext.getSystemService(Context.NETD_SERVICE)); + } + /** Add device config change listener */ public void addDeviceConfigChangedListener( @NonNull final DeviceConfig.OnPropertiesChangedListener listener) { diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java index 9b6915c..f950b47 100644 --- a/src/com/android/server/connectivity/NetworkMonitor.java +++ b/src/com/android/server/connectivity/NetworkMonitor.java @@ -405,7 +405,7 @@ public class NetworkMonitor extends StateMachine { SharedLog validationLog) { this(context, cb, network, new IpConnectivityLog(), validationLog, Dependencies.DEFAULT, new DataStallStatsUtils(), - getTcpSocketTrackerOrNull(context)); + getTcpSocketTrackerOrNull(context, network)); } @VisibleForTesting @@ -2240,7 +2240,6 @@ public class NetworkMonitor extends StateMachine { // Check TCP signal. Suspect it may be a data stall if : // 1. TCP connection fail rate(lost+retrans) is higher than threshold. // 2. Accumulate enough packets count. - // TODO: Need to filter per target network. final TcpSocketTracker tst = getTcpSocketTracker(); if (dataStallEvaluateTypeEnabled(DATA_STALL_EVALUATION_TYPE_TCP) && tst != null) { if (tst.getLatestReceivedCount() > 0) { @@ -2406,14 +2405,14 @@ public class NetworkMonitor extends StateMachine { } @Nullable - private static TcpSocketTracker getTcpSocketTrackerOrNull(Context context) { + private static TcpSocketTracker getTcpSocketTrackerOrNull(Context context, Network network) { return ((Dependencies.DEFAULT.getDeviceConfigPropertyInt( NAMESPACE_CONNECTIVITY, CONFIG_DATA_STALL_EVALUATION_TYPE, DEFAULT_DATA_STALL_EVALUATION_TYPES) & DATA_STALL_EVALUATION_TYPE_TCP) != 0) ? new TcpSocketTracker(new TcpSocketTracker.Dependencies(context, - ShimUtils.isReleaseOrDevelopmentApiAbove(Build.VERSION_CODES.Q))) + ShimUtils.isReleaseOrDevelopmentApiAbove(Build.VERSION_CODES.Q)), network) : null; } } diff --git a/tests/unit/src/com/android/networkstack/netlink/TcpSocketTrackerTest.java b/tests/unit/src/com/android/networkstack/netlink/TcpSocketTrackerTest.java index 25ffc66..341545f 100644 --- a/tests/unit/src/com/android/networkstack/netlink/TcpSocketTrackerTest.java +++ b/tests/unit/src/com/android/networkstack/netlink/TcpSocketTrackerTest.java @@ -22,6 +22,9 @@ import static android.net.util.DataStallUtils.DEFAULT_TCP_PACKETS_FAIL_PERCENTAG import static android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY; import static android.system.OsConstants.AF_INET; +import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn; +import static com.android.dx.mockito.inline.extended.ExtendedMockito.mockitoSession; + import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertTrue; @@ -31,18 +34,27 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.any; import static org.mockito.Mockito.when; +import android.net.INetd; +import android.net.MarkMaskParcel; +import android.net.Network; import android.net.netlink.StructNlMsgHdr; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; +import com.android.networkstack.apishim.NetworkShim; +import com.android.networkstack.apishim.NetworkShimImpl; + import libcore.util.HexEncoding; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.mockito.MockitoSession; +import org.mockito.quality.Strictness; import java.io.FileDescriptor; import java.nio.ByteBuffer; @@ -111,7 +123,7 @@ public class TcpSocketTrackerTest { "00000000" + // data "0800" + // len = 8 "0F00" + // type = 15(INET_DIAG_MARK) - "851A0C00" + // data, socket mark=793221 + "850A0C00" + // data, socket mark=789125 "AC00" + // len = 172 "0200" + // type = 2(INET_DIAG_INFO) // tcp_info @@ -175,18 +187,48 @@ public class TcpSocketTrackerTest { + "00"; // retrans private static final byte[] TEST_RESPONSE_BYTES = HexEncoding.decode(TEST_RESPONSE_HEX.toCharArray(), false); + private static final int TEST_NETID1 = 0xA85; + private static final int TEST_NETID2 = 0x1A85; + private static final int TEST_NETID1_FWMARK = 0x0A85; + private static final int TEST_NETID2_FWMARK = 0x1A85; + private static final int NETID_MASK = 0xffff; @Mock private TcpSocketTracker.Dependencies mDependencies; @Mock private FileDescriptor mMockFd; - + @Mock private Network mNetwork = new Network(TEST_NETID1); + @Mock private INetd mNetd; + private MockitoSession mSession; + @Mock NetworkShim mNetworkShim; @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); + when(mDependencies.getNetd()).thenReturn(mNetd); when(mDependencies.isTcpInfoParsingSupported()).thenReturn(true); when(mDependencies.connectToKernel()).thenReturn(mMockFd); when(mDependencies.getDeviceConfigPropertyInt( eq(NAMESPACE_CONNECTIVITY), eq(CONFIG_TCP_PACKETS_FAIL_PERCENTAGE), anyInt())).thenReturn(DEFAULT_TCP_PACKETS_FAIL_PERCENTAGE); + mSession = mockitoSession() + .spyStatic(NetworkShimImpl.class) + .strictness(Strictness.WARN) + .startMocking(); + + doReturn(mNetworkShim).when(() -> NetworkShimImpl.newInstance(mNetwork)); + when(mNetworkShim.getNetId()).thenReturn(TEST_NETID1); + when(mNetd.getFwmarkForNetwork(eq(TEST_NETID1))) + .thenReturn(makeMarkMaskParcel(NETID_MASK, TEST_NETID1_FWMARK)); + } + + @After + public void tearDown() { + mSession.finishMocking(); + } + + private MarkMaskParcel makeMarkMaskParcel(final int mask, final int mark) { + final MarkMaskParcel parcel = new MarkMaskParcel(); + parcel.mask = mask; + parcel.mark = mark; + return parcel; } private ByteBuffer getByteBuffer(final byte[] bytes) { @@ -198,7 +240,7 @@ public class TcpSocketTrackerTest { @Test public void testParseSockInfo() { final ByteBuffer buffer = getByteBuffer(SOCK_DIAG_TCP_INET_BYTES); - final TcpSocketTracker tst = new TcpSocketTracker(mDependencies); + final TcpSocketTracker tst = new TcpSocketTracker(mDependencies, mNetwork); buffer.position(SOCKDIAG_MSG_HEADER_SIZE); final TcpSocketTracker.SocketInfo parsed = tst.parseSockInfo(buffer, AF_INET, 276, 100L); @@ -248,7 +290,7 @@ public class TcpSocketTrackerTest { expected.put(TcpInfo.Field.DELIVERY_RATE, 0L); assertEquals(parsed.tcpInfo, new TcpInfo(expected)); - assertEquals(parsed.fwmark, 793221); + assertEquals(parsed.fwmark, 789125); assertEquals(parsed.updateTime, 100); assertEquals(parsed.ipFamily, AF_INET); } @@ -270,7 +312,7 @@ public class TcpSocketTrackerTest { @Test public void testPollSocketsInfo() throws Exception { when(mDependencies.isTcpInfoParsingSupported()).thenReturn(false); - final TcpSocketTracker tst = new TcpSocketTracker(mDependencies); + final TcpSocketTracker tst = new TcpSocketTracker(mDependencies, mNetwork); assertFalse(tst.pollSocketsInfo()); when(mDependencies.isTcpInfoParsingSupported()).thenReturn(true); @@ -335,7 +377,7 @@ public class TcpSocketTrackerTest { @Test public void testPollSocketsInfo_BadFormat() throws Exception { - final TcpSocketTracker tst = new TcpSocketTracker(mDependencies); + final TcpSocketTracker tst = new TcpSocketTracker(mDependencies, mNetwork); ByteBuffer tcpBuffer = getByteBuffer(TEST_RESPONSE_BYTES); when(mDependencies.recvMessage(any())).thenReturn(tcpBuffer); @@ -351,4 +393,19 @@ public class TcpSocketTrackerTest { // Expect to reset to 0. assertEquals(0, tst.getLatestPacketFailPercentage()); } + + @Test + public void testUnMatchNetwork() throws Exception { + when(mNetworkShim.getNetId()).thenReturn(TEST_NETID2); + when(mNetd.getFwmarkForNetwork(eq(TEST_NETID2))) + .thenReturn(makeMarkMaskParcel(NETID_MASK, TEST_NETID2_FWMARK)); + final TcpSocketTracker tst = new TcpSocketTracker(mDependencies, mNetwork); + final ByteBuffer tcpBuffer = getByteBuffer(TEST_RESPONSE_BYTES); + when(mDependencies.recvMessage(any())).thenReturn(tcpBuffer); + assertTrue(tst.pollSocketsInfo()); + + assertEquals(0, tst.getSentSinceLastRecv()); + assertEquals(-1, tst.getLatestPacketFailPercentage()); + assertFalse(tst.isDataStallSuspected()); + } } diff --git a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java index 1e512f4..7289ab4 100644 --- a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java +++ b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java @@ -71,6 +71,7 @@ import android.content.res.Configuration; import android.content.res.Resources; import android.net.ConnectivityManager; import android.net.DnsResolver; +import android.net.INetd; import android.net.INetworkMonitorCallbacks; import android.net.LinkProperties; import android.net.Network; @@ -163,6 +164,7 @@ public class NetworkMonitorTest { private @Mock WifiInfo mWifiInfo; private @Captor ArgumentCaptor<String> mNetworkTestedRedirectUrlCaptor; private @Mock TcpSocketTracker.Dependencies mTstDependencies; + private @Mock INetd mNetd; private @Mock TcpSocketTracker mTst; private HashSet<WrappedNetworkMonitor> mCreatedNetworkMonitors; private HashSet<BroadcastReceiver> mRegisteredReceivers; @@ -371,6 +373,7 @@ public class NetworkMonitorTest { setFallbackSpecs(null); // Test with no fallback spec by default when(mRandom.nextInt()).thenReturn(0); + when(mTstDependencies.getNetd()).thenReturn(mNetd); // DNS probe timeout should not be defined more than half of HANDLER_TIMEOUT_MS. Otherwise, // it will fail the test because of timeout expired for querying AAAA and A sequentially. when(mResources.getInteger(eq(R.integer.config_captive_portal_dns_probe_timeout))) |