From f99a33a1b4599f6297faa316f408b58deebabefb Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Tue, 18 Oct 2016 10:36:33 +0900 Subject: DO NOT MERGE: 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 (cherry picked from commit e1c173d2240a8eedf7685c9371087dc047a6931f) Change-Id: Idb640dec13ba64180985544b9709a586af66eb6e --- core/java/android/net/IIpConnectivityMetrics.aidl | 3 ++- .../src/android/util/TokenBucketTest.java | 1 - .../server/connectivity/IpConnectivityMetrics.java | 24 ++++++++++++++++++++++ .../connectivity/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 8f634bbf0cc9..d36b7661aaa3 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 a053ad33f589..f7ac20c7287f 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 642f2e01987b..be681739d2ee 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; @@ -56,6 +60,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. @@ -73,6 +79,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; @@ -122,6 +130,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; @@ -131,6 +143,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; @@ -256,4 +273,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/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java b/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java index 14b5cbeb9276..aa491bbabd79 100644 --- a/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java +++ b/tests/net/java/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); -- cgit v1.2.3-59-g8ed1b