From 06b81b6af3c5db81ecd18f72917ea23d403bae3f Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Mon, 2 Sep 2024 16:18:24 +0800 Subject: [PATCH] Track tethering active sessions counts This commit implements the necessary counters to bookkeep max connection count, which will be used in follow-up changes to monitor and upload active tethering session metrics. The feature is disabled by default and will be gradually rolled out. Test: atest TetheringTests:com.android.networkstack.tethering.BpfCoordinatorTest Bug: 354619988 Change-Id: Iaf251b0ec08bde95c1b12df022db2e03179309de --- Tethering/Android.bp | 1 + .../tethering/BpfCoordinator.java | 106 ++++++++++++++++- .../networkstack/tethering/Tethering.java | 5 + .../tethering/TetheringConfiguration.java | 6 + .../tethering/BpfCoordinatorTest.java | 109 ++++++++++++++++++ 5 files changed, 225 insertions(+), 2 deletions(-) diff --git a/Tethering/Android.bp b/Tethering/Android.bp index d04660de01..498a1fe758 100644 --- a/Tethering/Android.bp +++ b/Tethering/Android.bp @@ -54,6 +54,7 @@ java_defaults { "src/**/*.java", ":framework-connectivity-shared-srcs", ":services-tethering-shared-srcs", + ":statslog-connectivity-java-gen", ":statslog-tethering-java-gen", ], static_libs: [ diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index 5c853f427b..80eb042795 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java @@ -33,10 +33,12 @@ import static com.android.net.module.util.ip.ConntrackMonitor.ConntrackEvent; import static com.android.networkstack.tethering.BpfUtils.DOWNSTREAM; import static com.android.networkstack.tethering.BpfUtils.UPSTREAM; import static com.android.networkstack.tethering.TetheringConfiguration.DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS; +import static com.android.networkstack.tethering.TetheringConfiguration.TETHER_ACTIVE_SESSIONS_METRICS; import static com.android.networkstack.tethering.UpstreamNetworkState.isVcnInterface; import static com.android.networkstack.tethering.util.TetheringUtils.getTetheringJniLibraryName; import android.app.usage.NetworkStatsManager; +import android.content.Context; import android.net.INetd; import android.net.IpPrefix; import android.net.LinkProperties; @@ -65,6 +67,7 @@ import com.android.modules.utils.build.SdkLevel; import com.android.net.module.util.BpfDump; import com.android.net.module.util.BpfMap; import com.android.net.module.util.CollectionUtils; +import com.android.net.module.util.DeviceConfigUtils; import com.android.net.module.util.IBpfMap; import com.android.net.module.util.InterfaceParams; import com.android.net.module.util.NetworkStackConstants; @@ -314,12 +317,17 @@ public class BpfCoordinator { scheduleConntrackTimeoutUpdate(); }; + private final boolean mSupportActiveSessionsMetrics; + // TODO: add BpfMap retrieving function. @VisibleForTesting public abstract static class Dependencies { /** Get handler. */ @NonNull public abstract Handler getHandler(); + /** Get context. */ + @NonNull public abstract Context getContext(); + /** Get netd. */ @NonNull public abstract INetd getNetd(); @@ -472,6 +480,13 @@ public class BpfCoordinator { return null; } } + + /** + * @see DeviceConfigUtils#isTetheringFeatureEnabled + */ + public boolean isFeatureEnabled(Context context, String name) { + return DeviceConfigUtils.isTetheringFeatureEnabled(context, name); + } } @VisibleForTesting @@ -508,6 +523,10 @@ public class BpfCoordinator { if (!mBpfCoordinatorShim.isInitialized()) { mLog.e("Bpf shim not initialized"); } + + // BPF IPv4 forwarding only supports on S+. + mSupportActiveSessionsMetrics = mDeps.isAtLeastS() + && mDeps.isFeatureEnabled(mDeps.getContext(), TETHER_ACTIVE_SESSIONS_METRICS); } /** @@ -534,6 +553,17 @@ public class BpfCoordinator { if (mHandler.hasCallbacks(mScheduledConntrackTimeoutUpdate)) { mHandler.removeCallbacks(mScheduledConntrackTimeoutUpdate); } + // Clear counters in case there is any counter unsync problem + // previously due to possible bpf failures. + // Normally this won't happen because all clients are cleared before + // reaching here. See IpServer.BaseServingState#exit(). + if (mSupportActiveSessionsMetrics) { + final int currentCount = mBpfConntrackEventConsumer.getCurrentConnectionCount(); + if (currentCount != 0) { + Log.wtf(TAG, "Unexpected CurrentConnectionCount: " + currentCount); + } + mBpfConntrackEventConsumer.clearConnectionCounters(); + } // Stop scheduled polling stats and poll the latest stats from BPF maps. if (mHandler.hasCallbacks(mScheduledPollingStats)) { mHandler.removeCallbacks(mScheduledPollingStats); @@ -1031,6 +1061,10 @@ public class BpfCoordinator { for (final Tether4Key k : deleteDownstreamRuleKeys) { mBpfCoordinatorShim.tetherOffloadRuleRemove(DOWNSTREAM, k); } + if (mSupportActiveSessionsMetrics) { + mBpfConntrackEventConsumer.decreaseCurrentConnectionCount( + deleteUpstreamRuleKeys.size()); + } // Cleanup each upstream interface by a set which avoids duplicated work on the same // upstream interface. Cleaning up the same interface twice (or more) here may raise @@ -1300,6 +1334,13 @@ public class BpfCoordinator { pw.increaseIndent(); dumpCounters(pw); pw.decreaseIndent(); + + pw.println(); + pw.println("mSupportActiveSessionsMetrics: " + mSupportActiveSessionsMetrics); + pw.println("getLastMaxConnectionCount: " + + mBpfConntrackEventConsumer.getLastMaxConnectionCount()); + pw.println("getCurrentConnectionCount: " + + mBpfConntrackEventConsumer.getCurrentConnectionCount()); } private void dumpStats(@NonNull IndentingPrintWriter pw) { @@ -1991,6 +2032,21 @@ public class BpfCoordinator { // while TCP status is established. @VisibleForTesting class BpfConntrackEventConsumer implements ConntrackEventConsumer { + /** + * Tracks the current number of tethering connections and the maximum + * observed since the last metrics collection. Used to provide insights + * into the distribution of active tethering sessions for metrics reporting. + + * These variables are accessed on the handler thread, which includes: + * 1. ConntrackEvents signaling the addition or removal of an IPv4 rule. + * 2. ConntrackEvents indicating the removal of a tethering client, + * triggering the removal of associated rules. + * 3. Removal of the last IpServer, which resets counters to handle + * potential synchronization issues. + */ + private int mLastMaxConnectionCount = 0; + private int mCurrentConnectionCount = 0; + // The upstream4 and downstream4 rules are built as the following tables. Only raw ip // upstream interface is supported. Note that the field "lastUsed" is only updated by // BPF program which records the last used time for a given rule. @@ -2124,6 +2180,10 @@ public class BpfCoordinator { return; } + if (mSupportActiveSessionsMetrics) { + decreaseCurrentConnectionCount(1); + } + maybeClearLimit(upstreamIndex); return; } @@ -2136,8 +2196,50 @@ public class BpfCoordinator { maybeAddDevMap(upstreamIndex, tetherClient.downstreamIfindex); maybeSetLimit(upstreamIndex); - mBpfCoordinatorShim.tetherOffloadRuleAdd(UPSTREAM, upstream4Key, upstream4Value); - mBpfCoordinatorShim.tetherOffloadRuleAdd(DOWNSTREAM, downstream4Key, downstream4Value); + + final boolean addedUpstream = mBpfCoordinatorShim.tetherOffloadRuleAdd( + UPSTREAM, upstream4Key, upstream4Value); + final boolean addedDownstream = mBpfCoordinatorShim.tetherOffloadRuleAdd( + DOWNSTREAM, downstream4Key, downstream4Value); + if (addedUpstream != addedDownstream) { + Log.wtf(TAG, "The bidirectional rules should be added concurrently (" + + "upstream: " + addedUpstream + + ", downstream: " + addedDownstream + ")"); + return; + } + if (mSupportActiveSessionsMetrics && addedUpstream && addedDownstream) { + mCurrentConnectionCount++; + mLastMaxConnectionCount = Math.max(mCurrentConnectionCount, + mLastMaxConnectionCount); + } + } + + public int getLastMaxConnectionAndResetToCurrent() { + final int ret = mLastMaxConnectionCount; + mLastMaxConnectionCount = mCurrentConnectionCount; + return ret; + } + + /** For dumping current state only. */ + public int getLastMaxConnectionCount() { + return mLastMaxConnectionCount; + } + + public int getCurrentConnectionCount() { + return mCurrentConnectionCount; + } + + public void decreaseCurrentConnectionCount(int count) { + mCurrentConnectionCount -= count; + if (mCurrentConnectionCount < 0) { + Log.wtf(TAG, "Unexpected mCurrentConnectionCount: " + + mCurrentConnectionCount); + } + } + + public void clearConnectionCounters() { + mCurrentConnectionCount = 0; + mLastMaxConnectionCount = 0; } } diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java index d62f18f87b..94ce4edcb2 100644 --- a/Tethering/src/com/android/networkstack/tethering/Tethering.java +++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java @@ -373,6 +373,11 @@ public class Tethering { return mHandler; } + @NonNull + public Context getContext() { + return mContext; + } + @NonNull public INetd getNetd() { return mNetd; diff --git a/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java b/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java index 298940e4fc..c9817c958d 100644 --- a/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java +++ b/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java @@ -144,6 +144,12 @@ public class TetheringConfiguration { /** A flag for using synchronous or asynchronous state machine. */ public static boolean USE_SYNC_SM = false; + /** + * A feature flag to control whether the active sessions metrics should be enabled. + * Disabled by default. + */ + public static final String TETHER_ACTIVE_SESSIONS_METRICS = "tether_active_sessions_metrics"; + public final String[] tetherableUsbRegexs; public final String[] tetherableWifiRegexs; public final String[] tetherableWigigRegexs; diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java index e54a7e0ddb..3c01f5d9ce 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java @@ -60,6 +60,7 @@ import static com.android.networkstack.tethering.BpfCoordinator.toIpv4MappedAddr import static com.android.networkstack.tethering.BpfUtils.DOWNSTREAM; import static com.android.networkstack.tethering.BpfUtils.UPSTREAM; import static com.android.networkstack.tethering.TetheringConfiguration.DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS; +import static com.android.networkstack.tethering.TetheringConfiguration.TETHER_ACTIVE_SESSIONS_METRICS; import static com.android.testutils.MiscAsserts.assertSameElements; import static org.junit.Assert.assertArrayEquals; @@ -87,6 +88,7 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import android.app.usage.NetworkStatsManager; +import android.content.Context; import android.net.INetd; import android.net.InetAddresses; import android.net.IpPrefix; @@ -140,6 +142,8 @@ import com.android.testutils.DevSdkIgnoreRule.IgnoreAfter; import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo; import com.android.testutils.TestBpfMap; import com.android.testutils.TestableNetworkStatsProviderCbBinder; +import com.android.testutils.com.android.testutils.SetFeatureFlagsRule; +import com.android.testutils.com.android.testutils.SetFeatureFlagsRule.FeatureFlag; import org.junit.Before; import org.junit.Rule; @@ -171,6 +175,16 @@ public class BpfCoordinatorTest { @Rule public final DevSdkIgnoreRule mIgnoreRule = new DevSdkIgnoreRule(); + final HashMap mFeatureFlags = new HashMap<>(); + // This will set feature flags from @FeatureFlag annotations + // into the map before setUp() runs. + @Rule + public final SetFeatureFlagsRule mSetFeatureFlagsRule = + new SetFeatureFlagsRule((name, enabled) -> { + mFeatureFlags.put(name, enabled); + return null; + }, (name) -> mFeatureFlags.getOrDefault(name, false)); + private static final boolean IPV4 = true; private static final boolean IPV6 = false; @@ -406,6 +420,11 @@ public class BpfCoordinatorTest { return this; } + public Builder setPrivateAddress(Inet4Address privateAddr) { + mPrivateAddr = privateAddr; + return this; + } + public Builder setRemotePort(int remotePort) { mRemotePort = (short) remotePort; return this; @@ -429,6 +448,7 @@ public class BpfCoordinatorTest { @Mock private NetworkStatsManager mStatsManager; @Mock private INetd mNetd; + @Mock private Context mMockContext; @Mock private IpServer mIpServer; @Mock private IpServer mIpServer2; @Mock private TetheringConfiguration mTetherConfig; @@ -474,6 +494,11 @@ public class BpfCoordinatorTest { return mHandler; } + @NonNull + public Context getContext() { + return mMockContext; + } + @NonNull public INetd getNetd() { return mNetd; @@ -546,6 +571,11 @@ public class BpfCoordinatorTest { public IBpfMap getBpfErrorMap() { return mBpfErrorMap; } + + @Override + public boolean isFeatureEnabled(Context context, String name) { + return mFeatureFlags.getOrDefault(name, false); + } }); @Before public void setUp() { @@ -1977,6 +2007,85 @@ public class BpfCoordinatorTest { verify(mBpfDevMap, never()).updateEntry(any(), any()); } + @FeatureFlag(name = TETHER_ACTIVE_SESSIONS_METRICS) + // BPF IPv4 forwarding only supports on S+. + @IgnoreUpTo(Build.VERSION_CODES.R) + @Test + public void testMaxConnectionCount_metricsEnabled() throws Exception { + doTestMaxConnectionCount(true); + } + + @FeatureFlag(name = TETHER_ACTIVE_SESSIONS_METRICS, enabled = false) + @Test + public void testMaxConnectionCount_metricsDisabled() throws Exception { + doTestMaxConnectionCount(false); + } + + private void doTestMaxConnectionCount(final boolean supportActiveSessionsMetrics) + throws Exception { + final BpfCoordinator coordinator = makeBpfCoordinator(); + initBpfCoordinatorForRule4(coordinator); + resetNetdAndBpfMaps(); + assertEquals(0, mConsumer.getLastMaxConnectionAndResetToCurrent()); + + // Prepare add/delete rule events. + final ArrayList addRuleEvents = new ArrayList<>(); + final ArrayList delRuleEvents = new ArrayList<>(); + for (int i = 0; i < 5; i++) { + final ConntrackEvent addEvent = new TestConntrackEvent.Builder().setMsgType( + IPCTNL_MSG_CT_NEW).setProto(IPPROTO_TCP).setRemotePort(i).build(); + addRuleEvents.add(addEvent); + final ConntrackEvent delEvent = new TestConntrackEvent.Builder().setMsgType( + IPCTNL_MSG_CT_DELETE).setProto(IPPROTO_TCP).setRemotePort(i).build(); + delRuleEvents.add(delEvent); + } + + // Add rules, verify counter increases. + for (int i = 0; i < 5; i++) { + mConsumer.accept(addRuleEvents.get(i)); + assertConsumerCountersEquals(supportActiveSessionsMetrics ? i + 1 : 0); + } + + // Add the same events again should not increase the counter because + // all events are already exist. + for (final ConntrackEvent event : addRuleEvents) { + mConsumer.accept(event); + assertConsumerCountersEquals(supportActiveSessionsMetrics ? 5 : 0); + } + + // Verify removing non-existent items won't change the counters. + for (int i = 5; i < 8; i++) { + mConsumer.accept(new TestConntrackEvent.Builder().setMsgType( + IPCTNL_MSG_CT_DELETE).setProto(IPPROTO_TCP).setRemotePort(i).build()); + assertConsumerCountersEquals(supportActiveSessionsMetrics ? 5 : 0); + } + + // Verify remove the rules decrease the counter. + // Note the max counter returns the max, so it returns the count before deleting. + for (int i = 0; i < 5; i++) { + mConsumer.accept(delRuleEvents.get(i)); + assertEquals(supportActiveSessionsMetrics ? 4 - i : 0, + mConsumer.getCurrentConnectionCount()); + assertEquals(supportActiveSessionsMetrics ? 5 - i : 0, + mConsumer.getLastMaxConnectionCount()); + assertEquals(supportActiveSessionsMetrics ? 5 - i : 0, + mConsumer.getLastMaxConnectionAndResetToCurrent()); + } + + // Verify remove these rules again doesn't decrease the counter. + for (int i = 0; i < 5; i++) { + mConsumer.accept(delRuleEvents.get(i)); + assertConsumerCountersEquals(0); + } + } + + // Helper method to assert all counter values inside consumer. + private void assertConsumerCountersEquals(int expectedCount) { + assertEquals(expectedCount, mConsumer.getCurrentConnectionCount()); + assertEquals(expectedCount, mConsumer.getLastMaxConnectionCount()); + assertEquals(expectedCount, mConsumer.getLastMaxConnectionAndResetToCurrent()); + } + private void setElapsedRealtimeNanos(long nanoSec) { mElapsedRealtimeNanos = nanoSec; }