Address comments on ag/11204387.
Add comments and slightly increase test coverage.
Bug: 153694684
Test: new test coverage in IpClientIntegrationTest
Change-Id: I160a0801449cbe9e66976eaacdd3a914adc3d341
diff --git a/src/android/net/ip/IpClientLinkObserver.java b/src/android/net/ip/IpClientLinkObserver.java
index 66a4038..dc58e7b 100644
--- a/src/android/net/ip/IpClientLinkObserver.java
+++ b/src/android/net/ip/IpClientLinkObserver.java
@@ -282,6 +282,12 @@
}
private final AlarmManager.OnAlarmListener mExpirePref64Alarm = () -> {
+ // TODO: in the rare case where the alarm fires and posts the lambda to the handler
+ // thread while we are processing an RA that changes the lifetime of the same prefix,
+ // this code will run anyway even if the alarm is rescheduled or cancelled. If the
+ // lifetime in the RA is zero this doesn't matter (we just harmlessly cancel the alarm
+ // one extra time) but if the lifetime is nonzero then the prefix will be added and
+ // immediately removed by this code.
updatePref64(mShim.getNat64Prefix(mLinkProperties),
mNat64PrefixExpiry, mNat64PrefixExpiry);
};
@@ -321,9 +327,11 @@
// If we already have a prefix, continue using it and ignore the new one. Stopping and
// restarting clatd is disruptive because it will break existing IPv4 connections.
- // TODO: this means that if we receive an RA that adds a new prefix and deletes the old
+ // Note: this means that if we receive an RA that adds a new prefix and deletes the old
// prefix, we might receive and ignore the new prefix, then delete the old prefix, and
- // have no prefix until the next RA is received.
+ // have no prefix until the next RA is received. This is because the kernel returns ND
+ // user options one at a time even if they are in the same RA.
+ // TODO: keep track of the last few prefixes seen, like DnsServerRepository does.
if (mNat64PrefixExpiry > now) return;
// The current prefix has expired. Either replace it with the new one or delete it.
diff --git a/tests/integration/src/android/net/ip/IpClientIntegrationTest.java b/tests/integration/src/android/net/ip/IpClientIntegrationTest.java
index 6a5aff3..2db553e 100644
--- a/tests/integration/src/android/net/ip/IpClientIntegrationTest.java
+++ b/tests/integration/src/android/net/ip/IpClientIntegrationTest.java
@@ -1431,6 +1431,14 @@
expectNoNat64PrefixUpdate(inOrder, prefix);
reset(mCb, mAlarm);
+ // Reduce the lifetime and expect to reschedule expiry.
+ pref64 = new StructNdOptPref64(prefix, 1500).toByteBuffer();
+ ra = buildRaPacket(pio, rdnss, pref64);
+ mPacketReader.sendResponse(ra);
+ pref64Alarm = expectAlarmSet(inOrder, "PREF64", 1496);
+ expectNoNat64PrefixUpdate(inOrder, prefix);
+ reset(mCb, mAlarm);
+
// Withdraw the prefix and expect it to be set to null.
pref64 = new StructNdOptPref64(prefix, 0).toByteBuffer();
ra = buildRaPacket(pio, rdnss, pref64);
@@ -1456,7 +1464,7 @@
expectNoNat64PrefixUpdate(inOrder, prefix);
reset(mCb, mAlarm);
- // Withdraw the prefix and expect to switch to the new prefix.
+ // Withdraw the old prefix and continue to announce the new one. Expect a prefix change.
pref64 = new StructNdOptPref64(prefix, 0).toByteBuffer();
ra = buildRaPacket(pio, rdnss, pref64, otherPref64);
mPacketReader.sendResponse(ra);
