From 4456f33a958a7f09e608399da83c4d12b2e7d191 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Mon, 19 Dec 2016 14:50:52 +0900 Subject: [PATCH] ApfTest: fix flaky testApfFilterRa. testApfFilterRa is failing with probabiliy 1/10 ~ 1/15 on the following assert: assertDrop(program, packet, lifetime/6), for lifetime values that are multiple of 6, where 6 is the hardcoded fraction of RA lifetime to filter in ApfFilter.java. When the lifetime is not a multiple of 6, the remainder of 1 to 5 seconds gives enough margin so that when the APF program is simulated the faked lifetime of the program is less than lifetime/6 away and the packet is dropped. However for lifetimes which are exact multiples of 6, this margin is always 0s and that result in nondeterminism in the result. This is consistent with the obervation that the only failed assert was for a lifetime of 300s, the only multiple of 6. This can be observed by detecting the age limit at which the filter stops dropping packet oscillating between lifetime/6 and lifetime/6 + 1 for lifetimes which are multiple of 6. This patch fixes the flakyness by freezing the flow of time in tests so that the expected filter age threshold is consistent and stable. Test: no failure observed in 1000 runs. Bug: 32561414 Change-Id: I5251d047039f34b82ce8a5d20ae46563e1e0cce8 --- .../net/java/android/net/apf/ApfFilter.java | 17 ++-- tests/net/java/android/net/apf/ApfTest.java | 87 ++++++++++++------- 2 files changed, 63 insertions(+), 41 deletions(-) diff --git a/services/net/java/android/net/apf/ApfFilter.java b/services/net/java/android/net/apf/ApfFilter.java index 83001df580233..0a907496ebddf 100644 --- a/services/net/java/android/net/apf/ApfFilter.java +++ b/services/net/java/android/net/apf/ApfFilter.java @@ -286,7 +286,8 @@ public class ApfFilter { } // Returns seconds since device boot. - private static long curTime() { + @VisibleForTesting + protected long currentTimeSeconds() { return SystemClock.elapsedRealtime() / DateUtils.SECOND_IN_MILLIS; } @@ -450,7 +451,7 @@ public class ApfFilter { } mPacket = ByteBuffer.wrap(Arrays.copyOf(packet, length)); - mLastSeen = curTime(); + mLastSeen = currentTimeSeconds(); // Sanity check packet in case a packet arrives before we attach RA filter // to our packet socket. b/29586253 @@ -580,7 +581,7 @@ public class ApfFilter { // How many seconds does this RA's have to live, taking into account the fact // that we might have seen it a while ago. long currentLifetime() { - return mMinLifetime - (curTime() - mLastSeen); + return mMinLifetime - (currentTimeSeconds() - mLastSeen); } boolean isExpired() { @@ -946,7 +947,7 @@ public class ApfFilter { Log.e(TAG, "Failed to generate APF program.", e); return; } - mLastTimeInstalledProgram = curTime(); + mLastTimeInstalledProgram = currentTimeSeconds(); mLastInstalledProgramMinLifetime = programMinLifetime; mLastInstalledProgram = program; mNumProgramUpdates++; @@ -965,7 +966,7 @@ public class ApfFilter { */ private boolean shouldInstallnewProgram() { long expiry = mLastTimeInstalledProgram + mLastInstalledProgramMinLifetime; - return expiry < curTime() + MAX_PROGRAM_LIFETIME_WORTH_REFRESHING; + return expiry < currentTimeSeconds() + MAX_PROGRAM_LIFETIME_WORTH_REFRESHING; } private void hexDump(String msg, byte[] packet, int length) { @@ -999,7 +1000,7 @@ public class ApfFilter { if (ra.matches(packet, length)) { if (VDBG) log("matched RA " + ra); // Update lifetimes. - ra.mLastSeen = curTime(); + ra.mLastSeen = currentTimeSeconds(); ra.mMinLifetime = ra.minLifetime(packet, length); ra.seenCount++; @@ -1128,7 +1129,7 @@ public class ApfFilter { pw.println("Program updates: " + mNumProgramUpdates); pw.println(String.format( "Last program length %d, installed %ds ago, lifetime %ds", - mLastInstalledProgram.length, curTime() - mLastTimeInstalledProgram, + mLastInstalledProgram.length, currentTimeSeconds() - mLastTimeInstalledProgram, mLastInstalledProgramMinLifetime)); pw.println("RA filters:"); @@ -1137,7 +1138,7 @@ public class ApfFilter { pw.println(ra); pw.increaseIndent(); pw.println(String.format( - "Seen: %d, last %ds ago", ra.seenCount, curTime() - ra.mLastSeen)); + "Seen: %d, last %ds ago", ra.seenCount, currentTimeSeconds() - ra.mLastSeen)); if (DBG) { pw.println("Last match:"); pw.increaseIndent(); diff --git a/tests/net/java/android/net/apf/ApfTest.java b/tests/net/java/android/net/apf/ApfTest.java index ff6175427ce71..91d6c6840f47c 100644 --- a/tests/net/java/android/net/apf/ApfTest.java +++ b/tests/net/java/android/net/apf/ApfTest.java @@ -29,9 +29,11 @@ import android.net.metrics.IpConnectivityLog; import android.net.metrics.RaEvent; import android.os.ConditionVariable; import android.os.Parcelable; +import android.os.SystemClock; import android.system.ErrnoException; import android.system.Os; import android.test.AndroidTestCase; +import android.text.format.DateUtils; import android.test.suitebuilder.annotation.SmallTest; import static android.system.OsConstants.*; @@ -604,6 +606,8 @@ public class ApfTest extends AndroidTestCase { public final static byte[] MOCK_MAC_ADDR = {1,2,3,4,5,6}; private FileDescriptor mWriteSocket; + private final long mFixedTimeMs = SystemClock.elapsedRealtime(); + public TestApfFilter(IpManager.Callback ipManagerCallback, boolean multicastFilter, IpConnectivityLog log) throws Exception { super(new ApfCapabilities(2, 1700, ARPHRD_ETHER), NetworkInterface.getByName("lo"), @@ -616,6 +620,11 @@ public class ApfTest extends AndroidTestCase { Os.write(mWriteSocket, packet, 0, packet.length); } + @Override + protected long currentTimeSeconds() { + return mFixedTimeMs / DateUtils.SECOND_IN_MILLIS; + } + @Override void maybeStartFilter() { mHardwareAddress = MOCK_MAC_ADDR; @@ -969,27 +978,30 @@ public class ApfTest extends AndroidTestCase { // Verify that the last program pushed to the IpManager.Callback properly filters the // given packet for the given lifetime. - private void verifyRaLifetime(MockIpManagerCallback ipManagerCallback, ByteBuffer packet, - int lifetime) { - byte[] program = ipManagerCallback.getApfProgram(); + private void verifyRaLifetime(byte[] program, ByteBuffer packet, int lifetime) { + final int FRACTION_OF_LIFETIME = 6; + final int ageLimit = lifetime / FRACTION_OF_LIFETIME; - // Verify new program should drop RA for 1/6th its lifetime + // Verify new program should drop RA for 1/6th its lifetime and pass afterwards. assertDrop(program, packet.array()); - assertDrop(program, packet.array(), lifetime/6); - assertPass(program, packet.array(), lifetime/6 + 1); + assertDrop(program, packet.array(), ageLimit); + assertPass(program, packet.array(), ageLimit + 1); assertPass(program, packet.array(), lifetime); - // Verify RA checksum is ignored + final short originalChecksum = packet.getShort(ICMP6_RA_CHECKSUM_OFFSET); packet.putShort(ICMP6_RA_CHECKSUM_OFFSET, (short)12345); assertDrop(program, packet.array()); packet.putShort(ICMP6_RA_CHECKSUM_OFFSET, (short)-12345); assertDrop(program, packet.array()); + packet.putShort(ICMP6_RA_CHECKSUM_OFFSET, originalChecksum); // Verify other changes to RA make it not match filter + final byte originalFirstByte = packet.get(0); packet.put(0, (byte)-1); assertPass(program, packet.array()); packet.put(0, (byte)0); assertDrop(program, packet.array()); + packet.put(0, originalFirstByte); } // Test that when ApfFilter is shown the given packet, it generates a program to filter it @@ -999,9 +1011,8 @@ public class ApfTest extends AndroidTestCase { // Verify new program generated if ApfFilter witnesses RA ipManagerCallback.resetApfProgramWait(); apfFilter.pretendPacketReceived(packet.array()); - ipManagerCallback.getApfProgram(); - - verifyRaLifetime(ipManagerCallback, packet, lifetime); + byte[] program = ipManagerCallback.getApfProgram(); + verifyRaLifetime(program, packet, lifetime); } private void verifyRaEvent(RaEvent expected) { @@ -1046,18 +1057,26 @@ public class ApfTest extends AndroidTestCase { TestApfFilter apfFilter = new TestApfFilter(ipManagerCallback, DROP_MULTICAST, mLog); byte[] program = ipManagerCallback.getApfProgram(); + final int ROUTER_LIFETIME = 1000; + final int PREFIX_VALID_LIFETIME = 200; + final int PREFIX_PREFERRED_LIFETIME = 100; + final int RDNSS_LIFETIME = 300; + final int ROUTE_LIFETIME = 400; + // Note that lifetime of 2000 will be ignored in favor of shorter route lifetime of 1000. + final int DNSSL_LIFETIME = 2000; + // Verify RA is passed the first time ByteBuffer basePacket = ByteBuffer.wrap(new byte[ICMP6_RA_OPTION_OFFSET]); basePacket.putShort(ETH_ETHERTYPE_OFFSET, (short)ETH_P_IPV6); basePacket.put(IPV6_NEXT_HEADER_OFFSET, (byte)IPPROTO_ICMPV6); basePacket.put(ICMP6_TYPE_OFFSET, (byte)ICMP6_ROUTER_ADVERTISEMENT); - basePacket.putShort(ICMP6_RA_ROUTER_LIFETIME_OFFSET, (short)1000); + basePacket.putShort(ICMP6_RA_ROUTER_LIFETIME_OFFSET, (short)ROUTER_LIFETIME); basePacket.position(IPV6_DEST_ADDR_OFFSET); basePacket.put(IPV6_ALL_NODES_ADDRESS); assertPass(program, basePacket.array()); - testRaLifetime(apfFilter, ipManagerCallback, basePacket, 1000); - verifyRaEvent(new RaEvent(1000, -1, -1, -1, -1, -1)); + testRaLifetime(apfFilter, ipManagerCallback, basePacket, ROUTER_LIFETIME); + verifyRaEvent(new RaEvent(ROUTER_LIFETIME, -1, -1, -1, -1, -1)); // Ensure zero-length options cause the packet to be silently skipped. // Do this before we test other packets. http://b/29586253 @@ -1079,11 +1098,14 @@ public class ApfTest extends AndroidTestCase { prefixOptionPacket.put((byte)ICMP6_PREFIX_OPTION_TYPE); prefixOptionPacket.put((byte)(ICMP6_PREFIX_OPTION_LEN / 8)); prefixOptionPacket.putInt( - ICMP6_RA_OPTION_OFFSET + ICMP6_PREFIX_OPTION_PREFERRED_LIFETIME_OFFSET, 100); + ICMP6_RA_OPTION_OFFSET + ICMP6_PREFIX_OPTION_PREFERRED_LIFETIME_OFFSET, + PREFIX_PREFERRED_LIFETIME); prefixOptionPacket.putInt( - ICMP6_RA_OPTION_OFFSET + ICMP6_PREFIX_OPTION_VALID_LIFETIME_OFFSET, 200); - testRaLifetime(apfFilter, ipManagerCallback, prefixOptionPacket, 100); - verifyRaEvent(new RaEvent(1000, 200, 100, -1, -1, -1)); + ICMP6_RA_OPTION_OFFSET + ICMP6_PREFIX_OPTION_VALID_LIFETIME_OFFSET, + PREFIX_VALID_LIFETIME); + testRaLifetime(apfFilter, ipManagerCallback, prefixOptionPacket, PREFIX_PREFERRED_LIFETIME); + verifyRaEvent(new RaEvent( + ROUTER_LIFETIME, PREFIX_VALID_LIFETIME, PREFIX_PREFERRED_LIFETIME, -1, -1, -1)); ByteBuffer rdnssOptionPacket = ByteBuffer.wrap( new byte[ICMP6_RA_OPTION_OFFSET + ICMP6_4_BYTE_OPTION_LEN]); @@ -1092,9 +1114,9 @@ public class ApfTest extends AndroidTestCase { rdnssOptionPacket.put((byte)ICMP6_RDNSS_OPTION_TYPE); rdnssOptionPacket.put((byte)(ICMP6_4_BYTE_OPTION_LEN / 8)); rdnssOptionPacket.putInt( - ICMP6_RA_OPTION_OFFSET + ICMP6_4_BYTE_LIFETIME_OFFSET, 300); - testRaLifetime(apfFilter, ipManagerCallback, rdnssOptionPacket, 300); - verifyRaEvent(new RaEvent(1000, -1, -1, -1, 300, -1)); + ICMP6_RA_OPTION_OFFSET + ICMP6_4_BYTE_LIFETIME_OFFSET, RDNSS_LIFETIME); + testRaLifetime(apfFilter, ipManagerCallback, rdnssOptionPacket, RDNSS_LIFETIME); + verifyRaEvent(new RaEvent(ROUTER_LIFETIME, -1, -1, -1, RDNSS_LIFETIME, -1)); ByteBuffer routeInfoOptionPacket = ByteBuffer.wrap( new byte[ICMP6_RA_OPTION_OFFSET + ICMP6_4_BYTE_OPTION_LEN]); @@ -1103,9 +1125,9 @@ public class ApfTest extends AndroidTestCase { routeInfoOptionPacket.put((byte)ICMP6_ROUTE_INFO_OPTION_TYPE); routeInfoOptionPacket.put((byte)(ICMP6_4_BYTE_OPTION_LEN / 8)); routeInfoOptionPacket.putInt( - ICMP6_RA_OPTION_OFFSET + ICMP6_4_BYTE_LIFETIME_OFFSET, 400); - testRaLifetime(apfFilter, ipManagerCallback, routeInfoOptionPacket, 400); - verifyRaEvent(new RaEvent(1000, -1, -1, 400, -1, -1)); + ICMP6_RA_OPTION_OFFSET + ICMP6_4_BYTE_LIFETIME_OFFSET, ROUTE_LIFETIME); + testRaLifetime(apfFilter, ipManagerCallback, routeInfoOptionPacket, ROUTE_LIFETIME); + verifyRaEvent(new RaEvent(ROUTER_LIFETIME, -1, -1, ROUTE_LIFETIME, -1, -1)); ByteBuffer dnsslOptionPacket = ByteBuffer.wrap( new byte[ICMP6_RA_OPTION_OFFSET + ICMP6_4_BYTE_OPTION_LEN]); @@ -1114,18 +1136,17 @@ public class ApfTest extends AndroidTestCase { dnsslOptionPacket.put((byte)ICMP6_DNSSL_OPTION_TYPE); dnsslOptionPacket.put((byte)(ICMP6_4_BYTE_OPTION_LEN / 8)); dnsslOptionPacket.putInt( - ICMP6_RA_OPTION_OFFSET + ICMP6_4_BYTE_LIFETIME_OFFSET, 2000); - // Note that lifetime of 2000 will be ignored in favor of shorter - // route lifetime of 1000. - testRaLifetime(apfFilter, ipManagerCallback, dnsslOptionPacket, 1000); - verifyRaEvent(new RaEvent(1000, -1, -1, -1, -1, 2000)); + ICMP6_RA_OPTION_OFFSET + ICMP6_4_BYTE_LIFETIME_OFFSET, DNSSL_LIFETIME); + testRaLifetime(apfFilter, ipManagerCallback, dnsslOptionPacket, ROUTER_LIFETIME); + verifyRaEvent(new RaEvent(ROUTER_LIFETIME, -1, -1, -1, -1, DNSSL_LIFETIME)); // Verify that current program filters all five RAs: - verifyRaLifetime(ipManagerCallback, basePacket, 1000); - verifyRaLifetime(ipManagerCallback, prefixOptionPacket, 100); - verifyRaLifetime(ipManagerCallback, rdnssOptionPacket, 300); - verifyRaLifetime(ipManagerCallback, routeInfoOptionPacket, 400); - verifyRaLifetime(ipManagerCallback, dnsslOptionPacket, 1000); + program = ipManagerCallback.getApfProgram(); + verifyRaLifetime(program, basePacket, ROUTER_LIFETIME); + verifyRaLifetime(program, prefixOptionPacket, PREFIX_PREFERRED_LIFETIME); + verifyRaLifetime(program, rdnssOptionPacket, RDNSS_LIFETIME); + verifyRaLifetime(program, routeInfoOptionPacket, ROUTE_LIFETIME); + verifyRaLifetime(program, dnsslOptionPacket, ROUTER_LIFETIME); apfFilter.shutdown(); }