From 67c5e03b54e322a4dab897cbdd36d51ecc007f1e Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Thu, 14 Sep 2017 16:31:38 +0900 Subject: Extract RingBuffer class from NetdEventListenerService This patch takes out the ring buffer array added for NFLOG wakeup packet events logging and extract it into its own class for reuse. This new RingBuffer class has the two minimal useful functions append() and toArray(). Bug: 65164242 Bug: 65700460 Test: runtest frameworks-net, with new unit test Change-Id: Ib94d79a93f4e99661b7d0fac67117b91d57af980 --- .../java/com/android/internal/util/RingBuffer.java | 67 ++++++++++ .../server/connectivity/IpConnectivityMetrics.java | 6 +- .../connectivity/NetdEventListenerService.java | 29 +---- .../com/android/internal/util/RingBufferTest.java | 145 +++++++++++++++++++++ 4 files changed, 222 insertions(+), 25 deletions(-) create mode 100644 core/java/com/android/internal/util/RingBuffer.java create mode 100644 tests/net/java/com/android/internal/util/RingBufferTest.java diff --git a/core/java/com/android/internal/util/RingBuffer.java b/core/java/com/android/internal/util/RingBuffer.java new file mode 100644 index 000000000000..ad84353f23a9 --- /dev/null +++ b/core/java/com/android/internal/util/RingBuffer.java @@ -0,0 +1,67 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.internal.util; + +import static com.android.internal.util.Preconditions.checkArgumentPositive; + +import java.lang.reflect.Array; +import java.util.Arrays; + +/** + * A simple ring buffer structure with bounded capacity backed by an array. + * Events can always be added at the logical end of the buffer. If the buffer is + * full, oldest events are dropped when new events are added. + * {@hide} + */ +public class RingBuffer { + + // Array for storing events. + private final T[] mBuffer; + // Cursor keeping track of the logical end of the array. This cursor never + // wraps and instead keeps track of the total number of append() operations. + private long mCursor = 0; + + public RingBuffer(Class c, int capacity) { + checkArgumentPositive(capacity, "A RingBuffer cannot have 0 capacity"); + // Java cannot create generic arrays without a runtime hint. + mBuffer = (T[]) Array.newInstance(c, capacity); + } + + public int size() { + return (int) Math.min(mBuffer.length, (long) mCursor); + } + + public void append(T t) { + mBuffer[indexOf(mCursor++)] = t; + } + + public T[] toArray() { + // Only generic way to create a T[] from another T[] + T[] out = Arrays.copyOf(mBuffer, size(), (Class) mBuffer.getClass()); + // Reverse iteration from youngest event to oldest event. + long inCursor = mCursor - 1; + int outIdx = out.length - 1; + while (outIdx >= 0) { + out[outIdx--] = (T) mBuffer[indexOf(inCursor--)]; + } + return out; + } + + private int indexOf(long cursor) { + return (int) Math.abs(cursor % mBuffer.length); + } +} diff --git a/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java b/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java index 475d786aedd1..bfe960c339ae 100644 --- a/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java +++ b/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java @@ -44,7 +44,11 @@ import java.util.ArrayList; import java.util.List; import java.util.function.ToIntFunction; -/** {@hide} */ +/** + * Event buffering service for core networking and connectivity metrics. + * + * {@hide} + */ final public class IpConnectivityMetrics extends SystemService { private static final String TAG = IpConnectivityMetrics.class.getSimpleName(); private static final boolean DBG = false; diff --git a/services/core/java/com/android/server/connectivity/NetdEventListenerService.java b/services/core/java/com/android/server/connectivity/NetdEventListenerService.java index 25dba3570e20..6206dfcd6622 100644 --- a/services/core/java/com/android/server/connectivity/NetdEventListenerService.java +++ b/services/core/java/com/android/server/connectivity/NetdEventListenerService.java @@ -38,6 +38,7 @@ import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.BitUtils; import com.android.internal.util.IndentingPrintWriter; +import com.android.internal.util.RingBuffer; import com.android.internal.util.TokenBucket; import com.android.server.connectivity.metrics.nano.IpConnectivityLogClass.IpConnectivityEvent; import java.io.PrintWriter; @@ -82,9 +83,8 @@ public class NetdEventListenerService extends INetdEventListener.Stub { private final ArrayMap mWakeupStats = new ArrayMap<>(); // Ring buffer array for storing packet wake up events sent by Netd. @GuardedBy("this") - private final WakeupEvent[] mWakeupEvents = new WakeupEvent[WAKEUP_EVENT_BUFFER_LENGTH]; - @GuardedBy("this") - private long mWakeupEventCursor = 0; + private final RingBuffer mWakeupEvents = + new RingBuffer(WakeupEvent.class, WAKEUP_EVENT_BUFFER_LENGTH); private final ConnectivityManager mCm; @@ -175,13 +175,11 @@ public class NetdEventListenerService extends INetdEventListener.Stub { @GuardedBy("this") private void addWakeupEvent(String iface, long timestampMs, int uid) { - int index = wakeupEventIndex(mWakeupEventCursor); - mWakeupEventCursor++; WakeupEvent event = new WakeupEvent(); event.iface = iface; event.timestampMs = timestampMs; event.uid = uid; - mWakeupEvents[index] = event; + mWakeupEvents.append(event); WakeupStats stats = mWakeupStats.get(iface); if (stats == null) { stats = new WakeupStats(iface); @@ -190,23 +188,6 @@ public class NetdEventListenerService extends INetdEventListener.Stub { stats.countEvent(event); } - @GuardedBy("this") - private WakeupEvent[] getWakeupEvents() { - int length = (int) Math.min(mWakeupEventCursor, (long) mWakeupEvents.length); - WakeupEvent[] out = new WakeupEvent[length]; - // Reverse iteration from youngest event to oldest event. - long inCursor = mWakeupEventCursor - 1; - int outIdx = out.length - 1; - while (outIdx >= 0) { - out[outIdx--] = mWakeupEvents[wakeupEventIndex(inCursor--)]; - } - return out; - } - - private static int wakeupEventIndex(long cursor) { - return (int) Math.abs(cursor % WAKEUP_EVENT_BUFFER_LENGTH); - } - public synchronized void flushStatistics(List events) { flushProtos(events, mConnectEvents, IpConnectivityEventBuilder::toProto); flushProtos(events, mDnsEvents, IpConnectivityEventBuilder::toProto); @@ -230,7 +211,7 @@ public class NetdEventListenerService extends INetdEventListener.Stub { for (int i = 0; i < mWakeupStats.size(); i++) { pw.println(mWakeupStats.valueAt(i)); } - for (WakeupEvent wakeup : getWakeupEvents()) { + for (WakeupEvent wakeup : mWakeupEvents.toArray()) { pw.println(wakeup); } } diff --git a/tests/net/java/com/android/internal/util/RingBufferTest.java b/tests/net/java/com/android/internal/util/RingBufferTest.java new file mode 100644 index 000000000000..7a2344317223 --- /dev/null +++ b/tests/net/java/com/android/internal/util/RingBufferTest.java @@ -0,0 +1,145 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.internal.util; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +import android.support.test.filters.SmallTest; +import android.support.test.runner.AndroidJUnit4; + +import org.junit.Test; +import org.junit.runner.RunWith; +import java.util.Arrays; +import java.util.Objects; + + +@SmallTest +@RunWith(AndroidJUnit4.class) +public class RingBufferTest { + + @Test + public void testEmptyRingBuffer() { + RingBuffer buffer = new RingBuffer<>(String.class, 100); + + assertArraysEqual(new String[0], buffer.toArray()); + } + + @Test + public void testIncorrectConstructorArguments() { + try { + RingBuffer buffer = new RingBuffer<>(String.class, -10); + fail("Should not be able to create a negative capacity RingBuffer"); + } catch (IllegalArgumentException expected) { + } + + try { + RingBuffer buffer = new RingBuffer<>(String.class, 0); + fail("Should not be able to create a 0 capacity RingBuffer"); + } catch (IllegalArgumentException expected) { + } + } + + @Test + public void testRingBufferWithNoWrapping() { + RingBuffer buffer = new RingBuffer<>(String.class, 100); + + buffer.append("a"); + buffer.append("b"); + buffer.append("c"); + buffer.append("d"); + buffer.append("e"); + + String[] expected = {"a", "b", "c", "d", "e"}; + assertArraysEqual(expected, buffer.toArray()); + } + + @Test + public void testRingBufferWithCapacity1() { + RingBuffer buffer = new RingBuffer<>(String.class, 1); + + buffer.append("a"); + assertArraysEqual(new String[]{"a"}, buffer.toArray()); + + buffer.append("b"); + assertArraysEqual(new String[]{"b"}, buffer.toArray()); + + buffer.append("c"); + assertArraysEqual(new String[]{"c"}, buffer.toArray()); + + buffer.append("d"); + assertArraysEqual(new String[]{"d"}, buffer.toArray()); + + buffer.append("e"); + assertArraysEqual(new String[]{"e"}, buffer.toArray()); + } + + @Test + public void testRingBufferWithWrapping() { + int capacity = 100; + RingBuffer buffer = new RingBuffer<>(String.class, capacity); + + buffer.append("a"); + buffer.append("b"); + buffer.append("c"); + buffer.append("d"); + buffer.append("e"); + + String[] expected1 = {"a", "b", "c", "d", "e"}; + assertArraysEqual(expected1, buffer.toArray()); + + String[] expected2 = new String[capacity]; + int firstIndex = 0; + int lastIndex = capacity - 1; + + expected2[firstIndex] = "e"; + for (int i = 1; i < capacity; i++) { + buffer.append("x"); + expected2[i] = "x"; + } + assertArraysEqual(expected2, buffer.toArray()); + + buffer.append("x"); + expected2[firstIndex] = "x"; + assertArraysEqual(expected2, buffer.toArray()); + + for (int i = 0; i < 10; i++) { + for (String s : expected2) { + buffer.append(s); + } + } + assertArraysEqual(expected2, buffer.toArray()); + + buffer.append("a"); + expected2[lastIndex] = "a"; + assertArraysEqual(expected2, buffer.toArray()); + } + + static void assertArraysEqual(T[] expected, T[] got) { + if (expected.length != got.length) { + fail(Arrays.toString(expected) + " and " + Arrays.toString(got) + + " did not have the same length"); + } + + for (int i = 0; i < expected.length; i++) { + if (!Objects.equals(expected[i], got[i])) { + fail(Arrays.toString(expected) + " and " + Arrays.toString(got) + + " were not equal"); + } + } + } +} -- cgit v1.2.3-59-g8ed1b From 1198ba1533cd2818dd69c443c3ae8cec3284b515 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Fri, 15 Sep 2017 14:18:57 +0900 Subject: Separate connectivity event buffer for bug reports This patch uses the RingBuffer class previously extracted out of NetdEventListenerService for buffering connectivity events in two independent buffers: - the current existing buffer used for metrics reporting - a new rolling buffer, used for bug report dumpsys. This improves the suefulness of connectivity metrics for bug reports by solving these three issues tied to the usage of the existing metrics reporting buffer: - the buffer is always cleared when metrics reporting happens. If a bug report is taken shortly after, there is no past connectivity event added to that bug report. - the buffer has a max capacity and starts dropping new events when it saturates, until metrics reporting happens. When this happens, a bug report will not contain recent connectivity events. - some types of event are rate limited to avoid flooding the metrics buffer. events dropped due to rate limits never appears in the bug report, but the new bug report buffer ignores rate limiting. Bug: 65164242 Bug: 65700460 Test: runtest frameworks-net, manually inspecting ouput of $ adb shell dumpsys connmetrics -a Change-Id: Ia47e566b0c9a6629a26afb7067d5a8efadc25aef --- .../server/connectivity/IpConnectivityMetrics.java | 43 ++++++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java b/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java index bfe960c339ae..f2445fa36006 100644 --- a/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java +++ b/services/core/java/com/android/server/connectivity/IpConnectivityMetrics.java @@ -34,6 +34,7 @@ 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.RingBuffer; import com.android.internal.util.TokenBucket; import com.android.server.SystemService; import com.android.server.connectivity.metrics.nano.IpConnectivityLogClass.IpConnectivityEvent; @@ -62,7 +63,10 @@ final public class IpConnectivityMetrics extends SystemService { private static final String SERVICE_NAME = IpConnectivityLog.SERVICE_NAME; - // Default size of the event buffer. Once the buffer is full, incoming events are dropped. + // Default size of the event rolling log for bug report dumps. + private static final int DEFAULT_LOG_SIZE = 500; + // Default size of the event buffer for metrics reporting. + // 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; @@ -71,24 +75,38 @@ final public class IpConnectivityMetrics extends SystemService { private static final int ERROR_RATE_LIMITED = -1; - // Lock ensuring that concurrent manipulations of the event buffer are correct. + // Lock ensuring that concurrent manipulations of the event buffers are correct. // There are three concurrent operations to synchronize: // - appending events to the buffer. // - iterating throught the buffer. // - flushing the buffer content and replacing it by a new buffer. private final Object mLock = new Object(); + // Implementation instance of IIpConnectivityMetrics.aidl. @VisibleForTesting public final Impl impl = new Impl(); + // Subservice listening to Netd events via INetdEventListener.aidl. @VisibleForTesting NetdEventListenerService mNetdListener; + // Rolling log of the most recent events. This log is used for dumping + // connectivity events in bug reports. + @GuardedBy("mLock") + private final RingBuffer mEventLog = + new RingBuffer(ConnectivityMetricsEvent.class, DEFAULT_LOG_SIZE); + // Buffer of connectivity events used for metrics reporting. This buffer + // does not rotate automatically and instead saturates when it becomes full. + // It is flushed at metrics reporting. @GuardedBy("mLock") private ArrayList mBuffer; + // Total number of events dropped from mBuffer since last metrics reporting. @GuardedBy("mLock") private int mDropped; + // Capacity of mBuffer @GuardedBy("mLock") private int mCapacity; + // A list of rate limiting counters keyed by connectivity event types for + // metrics reporting mBuffer. @GuardedBy("mLock") private final ArrayMap, TokenBucket> mBuckets = makeRateLimitingBuckets(); @@ -136,6 +154,7 @@ final public class IpConnectivityMetrics extends SystemService { private int append(ConnectivityMetricsEvent event) { if (DBG) Log.d(TAG, "logEvent: " + event); synchronized (mLock) { + mEventLog.append(event); final int left = mCapacity - mBuffer.size(); if (event == null) { return left; @@ -220,6 +239,23 @@ final public class IpConnectivityMetrics extends SystemService { } } + /** + * Prints for bug reports the content of the rolling event log and the + * content of Netd event listener. + */ + private void cmdDumpsys(FileDescriptor fd, PrintWriter pw, String[] args) { + final ConnectivityMetricsEvent[] events; + synchronized (mLock) { + events = mEventLog.toArray(); + } + for (ConnectivityMetricsEvent ev : events) { + pw.println(ev.toString()); + } + if (mNetdListener != null) { + mNetdListener.list(pw); + } + } + private void cmdStats(FileDescriptor fd, PrintWriter pw, String[] args) { synchronized (mLock) { pw.println("Buffered events: " + mBuffer.size()); @@ -262,7 +298,8 @@ final public class IpConnectivityMetrics extends SystemService { cmdFlush(fd, pw, args); return; case CMD_DUMPSYS: - // Fallthrough to CMD_LIST when dumpsys.cpp dumps services states (bug reports) + cmdDumpsys(fd, pw, args); + return; case CMD_LIST: cmdList(fd, pw, args); return; -- cgit v1.2.3-59-g8ed1b