From 05686dbb6b965eb1a54591d2ed2334337886c6ee Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Wed, 19 Oct 2016 11:17:28 +0900 Subject: [PATCH 1/2] IpConnectivityMetrics reads buffer size in settings Test: IpConnectivityMetricsTest passes. Also manually changed the new setting and verified the buffer size is as expected after flushing the buffer. Bug: 32198637 Change-Id: Ie78d3fb03cea7883585549dac4f2063f0a8d9f4b --- core/java/android/provider/Settings.java | 7 ++++++ .../connectivity/IpConnectivityMetrics.java | 24 +++++++++++++++++-- .../IpConnectivityMetricsTest.java | 2 +- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index d7d8d5088657a..d3511827551ea 100755 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -7220,6 +7220,13 @@ public final class Settings { */ public static final String MOBILE_DATA_ALWAYS_ON = "mobile_data_always_on"; + /** + * Size of the event buffer for IP connectivity metrics. + * @hide + */ + public static final String CONNECTIVITY_METRICS_BUFFER_SIZE = + "connectivity_metrics_buffer_size"; + /** {@hide} */ public static final String NETSTATS_ENABLED = "netstats_enabled"; /** {@hide} */ diff --git a/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java b/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java index 28e724c2a7a1d..346ba2012de6c 100644 --- a/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java +++ b/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java @@ -22,6 +22,7 @@ import android.net.IIpConnectivityMetrics; import android.net.metrics.IpConnectivityLog; import android.os.IBinder; import android.os.Parcelable; +import android.provider.Settings; import android.text.TextUtils; import android.util.Base64; import android.util.Log; @@ -32,6 +33,7 @@ import java.io.FileDescriptor; import java.io.IOException; import java.io.PrintWriter; import java.util.ArrayList; +import java.util.function.ToIntFunction; import static com.android.server.connectivity.metrics.IpConnectivityLogClass.IpConnectivityEvent; @@ -44,6 +46,8 @@ final public class IpConnectivityMetrics extends SystemService { // Default size of the event buffer. Once the buffer is full, incoming events are dropped. private static final int DEFAULT_BUFFER_SIZE = 2000; + // Maximum size of the event buffer. + private static final int MAXIMUM_BUFFER_SIZE = DEFAULT_BUFFER_SIZE * 10; // Lock ensuring that concurrent manipulations of the event buffer are correct. // There are three concurrent operations to synchronize: @@ -63,11 +67,18 @@ final public class IpConnectivityMetrics extends SystemService { @GuardedBy("mLock") private int mCapacity; - public IpConnectivityMetrics(Context ctx) { + private final ToIntFunction mCapacityGetter; + + public IpConnectivityMetrics(Context ctx, ToIntFunction capacityGetter) { super(ctx); + mCapacityGetter = capacityGetter; initBuffer(); } + public IpConnectivityMetrics(Context ctx) { + this(ctx, READ_BUFFER_SIZE); + } + @Override public void onStart() { if (DBG) Log.d(TAG, "onStart"); @@ -86,7 +97,7 @@ final public class IpConnectivityMetrics extends SystemService { @VisibleForTesting public int bufferCapacity() { - return DEFAULT_BUFFER_SIZE; // TODO: read from config + return mCapacityGetter.applyAsInt(getContext()); } private void initBuffer() { @@ -226,4 +237,13 @@ final public class IpConnectivityMetrics extends SystemService { getContext().enforceCallingOrSelfPermission(what, "IpConnectivityMetrics"); } }; + + private static final ToIntFunction READ_BUFFER_SIZE = (ctx) -> { + int size = Settings.Global.getInt(ctx.getContentResolver(), + Settings.Global.CONNECTIVITY_METRICS_BUFFER_SIZE, DEFAULT_BUFFER_SIZE); + if (size <= 0) { + return DEFAULT_BUFFER_SIZE; + } + return Math.min(size, MAXIMUM_BUFFER_SIZE); + }; } diff --git a/services/tests/servicestests/src/com/android/server/connectivity/IpConnectivityMetricsTest.java b/services/tests/servicestests/src/com/android/server/connectivity/IpConnectivityMetricsTest.java index 3fc89b9ff12d3..2936e39b39ec6 100644 --- a/services/tests/servicestests/src/com/android/server/connectivity/IpConnectivityMetricsTest.java +++ b/services/tests/servicestests/src/com/android/server/connectivity/IpConnectivityMetricsTest.java @@ -57,7 +57,7 @@ public class IpConnectivityMetricsTest extends TestCase { public void setUp() { MockitoAnnotations.initMocks(this); - mService = new IpConnectivityMetrics(mCtx); + mService = new IpConnectivityMetrics(mCtx, (ctx) -> 2000); } public void testLoggingEvents() throws Exception { From e1c173d2240a8eedf7685c9371087dc047a6931f Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Tue, 18 Oct 2016 10:36:33 +0900 Subject: [PATCH 2/2] IpConnectivityMetrics: rate limit ApfProgramEvents This patch uses the previously introduced TokenBucket to rate limit ApfProgramEvents, still allowing for burst of ApfProgramEvents when a new interface is set up (due to ipv4 provisioning, multicast lock, ipv6 RAs triggering new APF program events in short amounts of time). Test: new test in IpConnectivityMetricsTest Bug: 1550402 Change-Id: I1a544a8d62e6b02f0ed0088ab76c4bad338d2f44 --- .../android/net/IIpConnectivityMetrics.aidl | 3 ++- .../src/android/util/TokenBucketTest.java | 1 - .../connectivity/IpConnectivityMetrics.java | 24 +++++++++++++++++++ .../IpConnectivityMetricsTest.java | 22 +++++++++++++++++ 4 files changed, 48 insertions(+), 2 deletions(-) diff --git a/core/java/android/net/IIpConnectivityMetrics.aidl b/core/java/android/net/IIpConnectivityMetrics.aidl index 8f634bbf0cc9a..d36b7661aaa3d 100644 --- a/core/java/android/net/IIpConnectivityMetrics.aidl +++ b/core/java/android/net/IIpConnectivityMetrics.aidl @@ -23,7 +23,8 @@ import android.net.ConnectivityMetricsEvent; interface IIpConnectivityMetrics { /** - * @return number of remaining available slots in buffer. + * @return the number of remaining available slots in buffer, + * or -1 if the event was dropped due to rate limiting. */ int logEvent(in ConnectivityMetricsEvent event); } diff --git a/core/tests/coretests/src/android/util/TokenBucketTest.java b/core/tests/coretests/src/android/util/TokenBucketTest.java index a053ad33f589f..f7ac20c7287fa 100644 --- a/core/tests/coretests/src/android/util/TokenBucketTest.java +++ b/core/tests/coretests/src/android/util/TokenBucketTest.java @@ -177,4 +177,3 @@ public class TokenBucketTest extends TestCase { interface Fn { void call(); } } - diff --git a/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java b/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java index 346ba2012de6c..a6522a309d36e 100644 --- a/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java +++ b/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java @@ -19,15 +19,19 @@ package com.android.server.connectivity; import android.content.Context; import android.net.ConnectivityMetricsEvent; import android.net.IIpConnectivityMetrics; +import android.net.metrics.ApfProgramEvent; import android.net.metrics.IpConnectivityLog; import android.os.IBinder; import android.os.Parcelable; import android.provider.Settings; import android.text.TextUtils; +import android.text.format.DateUtils; +import android.util.ArrayMap; import android.util.Base64; import android.util.Log; import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; +import com.android.internal.util.TokenBucket; import com.android.server.SystemService; import java.io.FileDescriptor; import java.io.IOException; @@ -49,6 +53,8 @@ final public class IpConnectivityMetrics extends SystemService { // Maximum size of the event buffer. private static final int MAXIMUM_BUFFER_SIZE = DEFAULT_BUFFER_SIZE * 10; + private static final int ERROR_RATE_LIMITED = -1; + // Lock ensuring that concurrent manipulations of the event buffer are correct. // There are three concurrent operations to synchronize: // - appending events to the buffer. @@ -66,6 +72,8 @@ final public class IpConnectivityMetrics extends SystemService { private int mDropped; @GuardedBy("mLock") private int mCapacity; + @GuardedBy("mLock") + private final ArrayMap, TokenBucket> mBuckets = makeRateLimitingBuckets(); private final ToIntFunction mCapacityGetter; @@ -115,6 +123,10 @@ final public class IpConnectivityMetrics extends SystemService { if (event == null) { return left; } + if (isRateLimited(event)) { + // Do not count as a dropped event. TODO: consider adding separate counter + return ERROR_RATE_LIMITED; + } if (left == 0) { mDropped++; return 0; @@ -124,6 +136,11 @@ final public class IpConnectivityMetrics extends SystemService { } } + private boolean isRateLimited(ConnectivityMetricsEvent event) { + TokenBucket tb = mBuckets.get(event.data.getClass()); + return (tb != null) && !tb.get(); + } + private String flushEncodedOutput() { final ArrayList events; final int dropped; @@ -246,4 +263,11 @@ final public class IpConnectivityMetrics extends SystemService { } return Math.min(size, MAXIMUM_BUFFER_SIZE); }; + + private static ArrayMap, TokenBucket> makeRateLimitingBuckets() { + ArrayMap, TokenBucket> map = new ArrayMap<>(); + // one token every minute, 50 tokens max: burst of ~50 events every hour. + map.put(ApfProgramEvent.class, new TokenBucket((int)DateUtils.MINUTE_IN_MILLIS, 50)); + return map; + } } diff --git a/services/tests/servicestests/src/com/android/server/connectivity/IpConnectivityMetricsTest.java b/services/tests/servicestests/src/com/android/server/connectivity/IpConnectivityMetricsTest.java index 2936e39b39ec6..da2c0d9f89799 100644 --- a/services/tests/servicestests/src/com/android/server/connectivity/IpConnectivityMetricsTest.java +++ b/services/tests/servicestests/src/com/android/server/connectivity/IpConnectivityMetricsTest.java @@ -19,6 +19,7 @@ package com.android.server.connectivity; import android.content.Context; import android.net.ConnectivityMetricsEvent; import android.net.IIpConnectivityMetrics; +import android.net.metrics.ApfProgramEvent; import android.net.metrics.ApfStats; import android.net.metrics.DefaultNetworkEvent; import android.net.metrics.DhcpClientEvent; @@ -112,6 +113,27 @@ public class IpConnectivityMetricsTest extends TestCase { assertEquals("", output3); } + public void testRateLimiting() { + final IpConnectivityLog logger = new IpConnectivityLog(mService.impl); + final ApfProgramEvent ev = new ApfProgramEvent(0, 0, 0, 0, 0); + final long fakeTimestamp = 1; + + int attempt = 100; // More than burst quota, but less than buffer size. + for (int i = 0; i < attempt; i++) { + logger.log(ev); + } + + String output1 = getdump("flush"); + assertFalse("".equals(output1)); + + for (int i = 0; i < attempt; i++) { + assertFalse("expected event to be dropped", logger.log(fakeTimestamp, ev)); + } + + String output2 = getdump("flush"); + assertEquals("", output2); + } + public void testEndToEndLogging() { IpConnectivityLog logger = new IpConnectivityLog(mService.impl);