Merge "Implement slow restransmission schedule in DHCP Renew/Rebind state." am: bc7aed8fbb Original change: https://android-review.googlesource.com/c/platform/packages/modules/NetworkStack/+/2136113 Change-Id: I09db7e0a8999d5eecfbcf3db534768cbbf26afef Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/src/android/net/dhcp/DhcpClient.java b/src/android/net/dhcp/DhcpClient.java index c88b653..b41484c 100644 --- a/src/android/net/dhcp/DhcpClient.java +++ b/src/android/net/dhcp/DhcpClient.java
@@ -35,6 +35,7 @@ import static android.net.util.NetworkStackUtils.DHCP_IPV6_ONLY_PREFERRED_VERSION; import static android.net.util.NetworkStackUtils.DHCP_IP_CONFLICT_DETECT_VERSION; import static android.net.util.NetworkStackUtils.DHCP_RAPID_COMMIT_VERSION; +import static android.net.util.NetworkStackUtils.DHCP_SLOW_RETRANSMISSION_VERSION; import static android.net.util.NetworkStackUtils.closeSocketQuietly; import static android.net.util.SocketUtils.makePacketSocketAddress; import static android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY; @@ -349,6 +350,7 @@ private long mTransactionStartMillis; private DhcpResults mDhcpLease; private long mDhcpLeaseExpiry; + private long mT2; private DhcpResults mOffer; private Configuration mConfiguration; private Inet4Address mLastAssignedIpv4Address; @@ -580,6 +582,15 @@ true /* defaultEnabled */); } + /** + * Check whether to adopt slow DHCPREQUEST retransmission approach in Renewing/Rebinding state + * suggested in RFC2131 section 4.4.5. + */ + public boolean isSlowRetransmissionEnabled() { + return mDependencies.isFeatureEnabled(mContext, DHCP_SLOW_RETRANSMISSION_VERSION, + false /* defaultEnabled */); + } + private void recordMetricEnabledFeatures() { if (isDhcpLeaseCacheEnabled()) mMetrics.setDhcpEnabledFeature(DhcpFeature.DF_INITREBOOT); if (isDhcpRapidCommitEnabled()) mMetrics.setDhcpEnabledFeature(DhcpFeature.DF_RAPIDCOMMIT); @@ -806,6 +817,7 @@ final long remainingDelay = mDhcpLeaseExpiry - now; final long renewDelay = remainingDelay / 2; final long rebindDelay = remainingDelay * 7 / 8; + mT2 = now + rebindDelay; mRenewAlarm.schedule(now + renewDelay); mRebindAlarm.schedule(now + rebindDelay); mExpiryAlarm.schedule(now + remainingDelay); @@ -880,6 +892,7 @@ private void clearDhcpState() { mDhcpLease = null; mDhcpLeaseExpiry = 0; + mT2 = 0; mOffer = null; } @@ -1199,7 +1212,7 @@ return baseTimer + jitter; } - protected void scheduleKick() { + protected void scheduleFastKick() { long now = SystemClock.elapsedRealtime(); long timeout = jitterTimer(mTimer); long alarmTime = now + timeout; @@ -1209,6 +1222,12 @@ mTimer = MAX_TIMEOUT_MS; } } + + protected void scheduleKick() { + // Always adopt the fast kick schedule by default unless this method is overrided + // by subclasses. + scheduleFastKick(); + } } class ObtainingConfigurationState extends LoggingState { @@ -1823,6 +1842,21 @@ // in renew/rebind state or just restart reconfiguration from StoppedState. protected abstract boolean shouldRestartOnNak(); + // Schedule alarm for the next DHCPREQUEST tranmission. Per RFC2131 if the client + // receives no response to its DHCPREQUEST message, the client should wait one-half + // of the remaining time until T2 in RENEWING state, and one-half of the remaining + // lease time in REBINDING state, down to a minimum of 60 seconds before transmitting + // DHCPREQUEST. + private static final long MIN_DELAY_BEFORE_NEXT_REQUEST = 60_000L; + protected void scheduleSlowKick(final long target) { + final long now = SystemClock.elapsedRealtime(); + long remainingDelay = (target - now) / 2; + if (remainingDelay < MIN_DELAY_BEFORE_NEXT_REQUEST) { + remainingDelay = MIN_DELAY_BEFORE_NEXT_REQUEST; + } + mKickAlarm.schedule(now + remainingDelay); + } + protected boolean sendPacket() { return sendRequestPacket( (Inet4Address) mDhcpLease.ipAddress.getAddress(), // ciaddr @@ -1895,13 +1929,18 @@ protected boolean shouldRestartOnNak() { return false; } + + @Override + protected void scheduleKick() { + if (isSlowRetransmissionEnabled()) { + scheduleSlowKick(mT2); + } else { + scheduleFastKick(); + } + } } - class DhcpRebindingState extends DhcpReacquiringState { - public DhcpRebindingState() { - mLeaseMsg = "Rebound"; - } - + class DhcpRebindingBaseState extends DhcpReacquiringState { @Override public void enter() { super.enter(); @@ -1926,7 +1965,29 @@ } } - class DhcpRefreshingAddressState extends DhcpRebindingState { + class DhcpRebindingState extends DhcpRebindingBaseState { + DhcpRebindingState() { + mLeaseMsg = "Rebound"; + } + + @Override + protected void scheduleKick() { + if (isSlowRetransmissionEnabled()) { + scheduleSlowKick(mDhcpLeaseExpiry); + } else { + scheduleFastKick(); + } + } + } + + // The slow retransmission approach complied with RFC2131 should only be applied + // for Renewing and Rebinding state. For this state it's expected to refresh IPv4 + // link address after roam as soon as possible, obviously it should not adopt the + // slow retransmission algorithm. Create a base DhcpRebindingBaseState state and + // have both of DhcpRebindingState and DhcpRefreshingAddressState extend from it, + // then override the scheduleKick method in DhcpRebindingState to comply with slow + // schedule and keep DhcpRefreshingAddressState as-is to use the fast schedule. + class DhcpRefreshingAddressState extends DhcpRebindingBaseState { DhcpRefreshingAddressState() { mLeaseMsg = "Refreshing address"; }
diff --git a/src/android/net/util/NetworkStackUtils.java b/src/android/net/util/NetworkStackUtils.java index 6dc2a5b..a6ea343 100755 --- a/src/android/net/util/NetworkStackUtils.java +++ b/src/android/net/util/NetworkStackUtils.java
@@ -211,6 +211,13 @@ "dhcp_ipv6_only_preferred_version"; /** + * Minimum module version at which to enable slow DHCP retransmission approach in renew/rebind + * state suggested in RFC2131 section 4.4.5. + */ + public static final String DHCP_SLOW_RETRANSMISSION_VERSION = + "dhcp_slow_retransmission_version"; + + /** * Minimum module version at which to enable dismissal CaptivePortalLogin app in validated * network feature. CaptivePortalLogin app will also use validation facilities in * {@link NetworkMonitor} to perform portal validation if feature is enabled.
diff --git a/tests/integration/common/android/net/ip/IpClientIntegrationTestCommon.java b/tests/integration/common/android/net/ip/IpClientIntegrationTestCommon.java index 5364a00..ff99bc8 100644 --- a/tests/integration/common/android/net/ip/IpClientIntegrationTestCommon.java +++ b/tests/integration/common/android/net/ip/IpClientIntegrationTestCommon.java
@@ -1143,6 +1143,8 @@ assertNotEquals(0, lp.getDnsServers().size()); assertEquals(addresses.size(), lp.getAddresses().size()); assertTrue(lp.getAddresses().containsAll(addresses)); + assertTrue(hasRouteTo(lp, IPV4_TEST_SUBNET_PREFIX)); // IPv4 directly-connected route + assertTrue(hasRouteTo(lp, IPV4_ANY_ADDRESS_PREFIX)); // IPv4 default route return lp; } @@ -1588,7 +1590,7 @@ assertDhcpRequestForReacquire(packet); packetList.add(packet); } - assertTrue(packetList.size() > 1); + assertEquals(1, packetList.size()); } private LinkProperties prepareDhcpReacquireTest() throws Exception { @@ -1597,6 +1599,7 @@ mNetworkAgentThread.start(); final long currentTime = System.currentTimeMillis(); + setFeatureEnabled(NetworkStackUtils.DHCP_SLOW_RETRANSMISSION_VERSION, true); performDhcpHandshake(true /* isSuccessLease */, TEST_LEASE_DURATION_S, true /* isDhcpLeaseCacheEnabled */, false /* isDhcpRapidCommitEnabled */, TEST_DEFAULT_MTU, @@ -1632,21 +1635,20 @@ request.senderIp /* target IP */, SERVER_ADDR /* sender IP */); HandlerUtils.waitForIdle(handler, TEST_TIMEOUT_MS); - // Verify the multiple unicast DHCPREQUESTs to be received in the renew state per current - // packet retransmission schedule. + // Verify there should be only one unicast DHCPREQUESTs to be received per RFC2131. assertReceivedDhcpRequestPacketCount(); return rebindAlarm; } - @Test @SignatureRequiredTest(reason = "Need to mock the DHCP renew/rebind/kick alarms") + @Test @SignatureRequiredTest(reason = "Need to mock the DHCP renew/rebind alarms") public void testDhcpRenew() throws Exception { final LinkProperties lp = prepareDhcpReacquireTest(); final InOrder inOrder = inOrder(mAlarm); runDhcpRenewTest(mDependencies.mDhcpClient.getHandler(), lp, inOrder); } - @Test @SignatureRequiredTest(reason = "Need to mock the DHCP renew/rebind/kick alarms") + @Test @SignatureRequiredTest(reason = "Need to mock the DHCP renew/rebind alarms") public void testDhcpRebind() throws Exception { final LinkProperties lp = prepareDhcpReacquireTest(); final Handler handler = mDependencies.mDhcpClient.getHandler(); @@ -1655,9 +1657,8 @@ // Trigger rebind alarm and forece DHCP client enter RebindingState. DHCP client sends // broadcast DHCPREQUEST to nearby servers, then check how many DHCPREQUEST packets are - // retransmitted within PACKET_TIMEOUT_MS(5s), there should be more than one DHCPREQUEST - // captured per current retransmission schedule(t=0, t=1, t=3, t=7, t=16 and allowing for - // 10% jitter). + // retransmitted within PACKET_TIMEOUT_MS(5s), there should be only one DHCPREQUEST + // captured per RFC2131. handler.post(() -> rebindAlarm.onAlarm()); assertReceivedDhcpRequestPacketCount(); }