Don't keep storing NUD failures indefinitely.
The current code that stores NUD failures in the memory store
will stop storing NUD failures in the memory store if it already
contains 10 failures in the past 6 hours. This is intended to
stop the code from constantly storing failures and filling up the
database.
However, if there have not been 10 failures in the past 6 hours,
then the code will never stop writing to the database until
IpClient is restarted and queries the memory store again. Prevent
this by also storing the number of failures seen since IpClient
was last started, and applying the threshold to those failures as
well.
Bug: 329010113
Test: new unit tests
Change-Id: I78c0bb3ad10886bff2d83e8ae4be3b2ce618c8ad
diff --git a/src/android/net/ip/IpClient.java b/src/android/net/ip/IpClient.java
index 0106d59..bed48b6 100644
--- a/src/android/net/ip/IpClient.java
+++ b/src/android/net/ip/IpClient.java
@@ -837,16 +837,25 @@
private ApfCapabilities mCurrentApfCapabilities;
private WakeupMessage mIpv6AutoconfTimeoutAlarm = null;
private boolean mIgnoreNudFailure;
- // An array of NUD failure event count associated with the query database since the timestamps
- // in the past, and is always initialized to null in StoppedState. Currently supported array
- // elements are as follows:
- // element 0: failures in the past week
- // element 1: failures in the past day
- // element 2: failures in the past 6h
+ /**
+ * An array of NUD failure event counts retrieved from the memory store since the timestamps
+ * in the past, and is always initialized to null in StoppedState. Currently supported array
+ * elements are as follows:
+ * element 0: failures in the past week
+ * element 1: failures in the past day
+ * element 2: failures in the past 6h
+ */
@Nullable
private int[] mNudFailureEventCounts = null;
/**
+ * The number of NUD failure events that were stored in the moemory store since this IpClient
+ * was last started. Always set to zero in StoppedState. Used to prevent writing excessive NUD
+ * failure events to the memory store.
+ */
+ private int mNudFailuresStoredSinceStart = 0;
+
+ /**
* Reading the snapshot is an asynchronous operation initiated by invoking
* Callback.startReadPacketFilter() and completed when the WiFi Service responds with an
* EVENT_READ_PACKET_FILTER_COMPLETE message. The mApfDataSnapshotComplete condition variable
@@ -2519,11 +2528,22 @@
// In order to avoid overflowing the database (the maximum is 10MB) in case of a NUD failure
// happens frequently (e.g, every 30s in a broken network), we stop writing the NUD failure
- // event to database if the event count in past 6h has exceeded the daily threshold.
+ // event to database if the total event count in past 6h, plus the number of events written
+ // since IpClient was started, has exceeded the daily threshold.
+ //
+ // The code also counts the number of events written since this IpClient was last started.
+ // Otherwise, if NUD failures are already being ignored due to a (daily or weekly) threshold
+ // being hit by events that happened more than 6 hours ago, but there have been no failures in
+ // the last 6 hours, the code would never stop logging failures (filling up the memory store)
+ // until IpClient is restarted and queries the memory store again.
+ //
+ // TODO: given that the code now looks at the number of events since IpClient was last started,
+ // is the 6-hour query still necessary?
private boolean shouldStopWritingNudFailureEventToDatabase() {
// NUD failure query has not completed yet.
if (mNudFailureEventCounts == null) return true;
- return mNudFailureEventCounts[2] >= mNudFailureCountDailyThreshold;
+ return mNudFailureEventCounts[2] + mNudFailuresStoredSinceStart
+ >= mNudFailureCountDailyThreshold;
}
private void maybeStoreNudFailureToDatabase(final NudEventType type) {
@@ -2542,6 +2562,7 @@
Log.e(TAG, "Failed to store NUD failure event");
}
});
+ mNudFailuresStoredSinceStart++;
if (DBG) {
Log.d(TAG, "store network event " + type
+ " at " + now
@@ -2782,6 +2803,7 @@
mMulticastNsSourceAddresses.clear();
mDelegatedPrefixes.clear();
mNudFailureEventCounts = null;
+ mNudFailuresStoredSinceStart = 0;
resetLinkProperties();
if (mStartTimeMillis > 0) {
diff --git a/tests/integration/common/android/net/ip/IpClientIntegrationTestCommon.java b/tests/integration/common/android/net/ip/IpClientIntegrationTestCommon.java
index 52ee71d..7451a7f 100644
--- a/tests/integration/common/android/net/ip/IpClientIntegrationTestCommon.java
+++ b/tests/integration/common/android/net/ip/IpClientIntegrationTestCommon.java
@@ -128,6 +128,7 @@
import static org.mockito.Mockito.timeout;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
import android.app.AlarmManager;
@@ -6189,6 +6190,40 @@
}
@Test
+ @Flag(name = IP_REACHABILITY_IGNORE_INCOMPLETE_IPV6_DNS_SERVER_VERSION, enabled = false)
+ @Flag(name = IP_REACHABILITY_IGNORE_INCOMPLETE_IPV6_DEFAULT_ROUTER_VERSION, enabled = false)
+ @Flag(name = IP_REACHABILITY_IGNORE_NUD_FAILURE_VERSION, enabled = true)
+ @SignatureRequiredTest(reason = "need to delete cluster from real db in tearDown")
+ public void testIgnoreNudFailuresStopWritingEvents() throws Exception {
+ // Add enough failures that NUD failures are ignored.
+ long when = (long) (System.currentTimeMillis() - SIX_HOURS_IN_MS * 1.1);
+ long expiry = when + ONE_WEEK_IN_MS;
+ storeNudFailureEvents(when, expiry, 10, IIpMemoryStore.NETWORK_EVENT_NUD_FAILURE_ORGANIC);
+
+ // Add enough recent failures to almost, but not quite reach the 6-hour threshold.
+ when = (long) (System.currentTimeMillis() - SIX_HOURS_IN_MS * 0.1);
+ expiry = when + ONE_WEEK_IN_MS;
+ storeNudFailureEvents(when, expiry, 9, IIpMemoryStore.NETWORK_EVENT_NUD_FAILURE_ORGANIC);
+
+ prepareIpReachabilityMonitorAddressResolutionTest(IPV6_ON_LINK_DNS_SERVER,
+ ROUTER_LINK_LOCAL);
+
+ // The first new failure is ignored and written to the database.
+ // The total is 10 failures in the last 6 hours.
+ sendPacketToUnreachableNeighbor(ipv6Addr(IPV6_OFF_LINK_DNS_SERVER));
+ expectAndDropMulticastNses(ROUTER_LINK_LOCAL, false /* expectNeighborLost */);
+ verify(mIpMemoryStore).storeNetworkEvent(any(), anyLong(), anyLong(),
+ eq(IIpMemoryStore.NETWORK_EVENT_NUD_FAILURE_ORGANIC), any());
+
+ // The second new failure is ignored, but not written.
+ reset(mIpMemoryStore);
+ sendPacketToUnreachableNeighbor(ipv6Addr(IPV6_ON_LINK_DNS_SERVER));
+ expectAndDropMulticastNses(ipv6Addr(IPV6_ON_LINK_DNS_SERVER),
+ false /* expectNeighborLost */);
+ verifyNoMoreInteractions(mIpMemoryStore);
+ }
+
+ @Test
@Flag(name = IP_REACHABILITY_IGNORE_NUD_FAILURE_VERSION, enabled = false)
@SignatureRequiredTest(reason = "need to delete cluster from real db in tearDown")
public void testIgnoreNudFailuresIfTooManyInPastWeek_stopWritingEvent_flagOff()
