Merge "IpManager: add a provisioning timeout option" into nyc-dev

services/net/java/android/net/dhcp/DhcpClient.java

@@ -105,26 +105,25 @@ public class DhcpClient extends StateMachine {
     /* Commands from controller to start/stop DHCP */
     public static final int CMD_START_DHCP                  = PUBLIC_BASE + 1;
     public static final int CMD_STOP_DHCP                   = PUBLIC_BASE + 2;
-    public static final int CMD_RENEW_DHCP                  = PUBLIC_BASE + 3;
 
     /* Notification from DHCP state machine prior to DHCP discovery/renewal */
-    public static final int CMD_PRE_DHCP_ACTION             = PUBLIC_BASE + 4;
+    public static final int CMD_PRE_DHCP_ACTION             = PUBLIC_BASE + 3;
     /* Notification from DHCP state machine post DHCP discovery/renewal. Indicates
      * success/failure */
-    public static final int CMD_POST_DHCP_ACTION            = PUBLIC_BASE + 5;
+    public static final int CMD_POST_DHCP_ACTION            = PUBLIC_BASE + 4;
     /* Notification from DHCP state machine before quitting */
-    public static final int CMD_ON_QUIT                     = PUBLIC_BASE + 6;
+    public static final int CMD_ON_QUIT                     = PUBLIC_BASE + 5;
 
     /* Command from controller to indicate DHCP discovery/renewal can continue
      * after pre DHCP action is complete */
-    public static final int CMD_PRE_DHCP_ACTION_COMPLETE    = PUBLIC_BASE + 7;
+    public static final int CMD_PRE_DHCP_ACTION_COMPLETE    = PUBLIC_BASE + 6;
 
     /* Command and event notification to/from IpManager requesting the setting
      * (or clearing) of an IPv4 LinkAddress.
      */
-    public static final int CMD_CLEAR_LINKADDRESS           = PUBLIC_BASE + 8;
+    public static final int CMD_CLEAR_LINKADDRESS           = PUBLIC_BASE + 7;
-    public static final int CMD_CONFIGURE_LINKADDRESS       = PUBLIC_BASE + 9;
+    public static final int CMD_CONFIGURE_LINKADDRESS       = PUBLIC_BASE + 8;
-    public static final int EVENT_LINKADDRESS_CONFIGURED    = PUBLIC_BASE + 10;
+    public static final int EVENT_LINKADDRESS_CONFIGURED    = PUBLIC_BASE + 9;
 
     /* Message.arg1 arguments to CMD_POST_DHCP notification */
     public static final int DHCP_SUCCESS = 1;
@@ -135,7 +134,7 @@ public class DhcpClient extends StateMachine {
     private static final int CMD_KICK             = PRIVATE_BASE + 1;
     private static final int CMD_RECEIVED_PACKET  = PRIVATE_BASE + 2;
     private static final int CMD_TIMEOUT          = PRIVATE_BASE + 3;
-    private static final int CMD_ONESHOT_TIMEOUT  = PRIVATE_BASE + 4;
+    private static final int CMD_RENEW_DHCP       = PRIVATE_BASE + 4;
 
     // For message logging.
     private static final Class[] sMessageClasses = { DhcpClient.class };
@@ -177,7 +176,6 @@ public class DhcpClient extends StateMachine {
     private final WakeupMessage mKickAlarm;
     private final WakeupMessage mTimeoutAlarm;
     private final WakeupMessage mRenewAlarm;
-    private final WakeupMessage mOneshotTimeoutAlarm;
     private final String mIfaceName;
 
     private boolean mRegisteredForPreDhcpNotification;
@@ -243,10 +241,6 @@ public class DhcpClient extends StateMachine {
         mTimeoutAlarm = makeWakeupMessage("TIMEOUT", CMD_TIMEOUT);
         // Used to schedule DHCP renews.
         mRenewAlarm = makeWakeupMessage("RENEW", CMD_RENEW_DHCP);
-        // Used to tell the caller when its request (CMD_START_DHCP or CMD_RENEW_DHCP) timed out.
-        // TODO: when the legacy DHCP client is gone, make the client fully asynchronous and
-        // remove this.
-        mOneshotTimeoutAlarm = makeWakeupMessage("ONESHOT_TIMEOUT", CMD_ONESHOT_TIMEOUT);
     }
 
     public void registerForPreDhcpNotification() {
@@ -506,29 +500,12 @@ public class DhcpClient extends StateMachine {
         }
     }
 
-    // The one-shot timeout is used to implement the timeout for CMD_START_DHCP. We can't use a
-    // state timeout to do this because obtaining an IP address involves passing through more than
-    // one state (specifically, it passes at least once through DhcpInitState and once through
-    // DhcpRequestingState). The one-shot timeout is created when CMD_START_DHCP is received, and is
-    // cancelled when exiting DhcpState (either due to a CMD_STOP_DHCP, or because of an error), or
-    // when we get an IP address (when entering DhcpBoundState). If it fires, we send ourselves
-    // CMD_ONESHOT_TIMEOUT and notify the caller that DHCP failed, but we take no other action. For
-    // example, if we're in DhcpInitState and sending DISCOVERs, we continue to do so.
-    //
-    // The one-shot timeout is not used for CMD_RENEW_DHCP because that is implemented using only
-    // one state, so we can just use the state timeout.
-    private void scheduleOneshotTimeout() {
-        final long alarmTime = SystemClock.elapsedRealtime() + DHCP_TIMEOUT_MS;
-        mOneshotTimeoutAlarm.schedule(alarmTime);
-    }
-
     class StoppedState extends LoggingState {
         @Override
         public boolean processMessage(Message message) {
             super.processMessage(message);
             switch (message.what) {
                 case CMD_START_DHCP:
-                    scheduleOneshotTimeout();
                     if (mRegisteredForPreDhcpNotification) {
                         transitionTo(mWaitBeforeStartState);
                     } else {
@@ -571,7 +548,6 @@ public class DhcpClient extends StateMachine {
 
         @Override
         public void exit() {
-            mOneshotTimeoutAlarm.cancel();
             if (mReceiveThread != null) {
                 mReceiveThread.halt();  // Also closes sockets.
                 mReceiveThread = null;
@@ -586,10 +562,6 @@ public class DhcpClient extends StateMachine {
                 case CMD_STOP_DHCP:
                     transitionTo(mStoppedState);
                     return HANDLED;
-                case CMD_ONESHOT_TIMEOUT:
-                    if (DBG) Log.d(TAG, "Timed out");
-                    notifyFailure();
-                    return HANDLED;
                 default:
                     return NOT_HANDLED;
             }
@@ -822,7 +794,6 @@ public class DhcpClient extends StateMachine {
         @Override
         public void enter() {
             super.enter();
-            mOneshotTimeoutAlarm.cancel();
             notifySuccess();
             // TODO: DhcpStateMachine only supported renewing at 50% of the lease time,
             // and did not support rebinding. Now that the legacy DHCP client is gone, fix this.
@@ -888,7 +859,7 @@ public class DhcpClient extends StateMachine {
         @Override
         protected void timeout() {
             transitionTo(mDhcpInitState);
-            sendMessage(CMD_ONESHOT_TIMEOUT);
+            notifyFailure();
         }
     }
 

services/net/java/android/net/ip/IpManager.java

@@ -17,6 +17,7 @@
 package android.net.ip;
 
 import com.android.internal.util.MessageUtils;
+import com.android.internal.util.WakeupMessage;
 
 import android.content.Context;
 import android.net.apf.ApfCapabilities;
@@ -53,6 +54,7 @@ import java.net.InetAddress;
 import java.net.NetworkInterface;
 import java.net.SocketException;
 import java.util.Objects;
+import java.util.StringJoiner;
 
 
 /**
@@ -250,6 +252,7 @@ public class IpManager extends StateMachine {
      *     final ProvisioningConfiguration config =
      *             mIpManager.buildProvisioningConfiguration()
      *                     .withPreDhcpAction()
+     *                     .withProvisioningTimeoutMs(36 * 1000)
      *                     .build();
      *     mIpManager.startProvisioning(config);
      *     ...
@@ -260,6 +263,15 @@ public class IpManager extends StateMachine {
      * must specify the configuration again.
      */
     public static class ProvisioningConfiguration {
+        // TODO: Delete this default timeout once those callers that care are
+        // fixed to pass in their preferred timeout.
+        //
+        // We pick 36 seconds so we can send DHCP requests at
+        //
+        //     t=0, t=2, t=6, t=14, t=30
+        //
+        // allowing for 10% jitter.
+        private static final int DEFAULT_TIMEOUT_MS = 36 * 1000;
 
         public static class Builder {
             private ProvisioningConfiguration mConfig = new ProvisioningConfiguration();
@@ -284,6 +296,11 @@ public class IpManager extends StateMachine {
                 return this;
             }
 
+            public Builder withProvisioningTimeoutMs(int timeoutMs) {
+                mConfig.mProvisioningTimeoutMs = timeoutMs;
+                return this;
+            }
+
             public ProvisioningConfiguration build() {
                 return new ProvisioningConfiguration(mConfig);
             }
@@ -293,6 +310,7 @@ public class IpManager extends StateMachine {
         /* package */ boolean mRequestedPreDhcpAction;
         /* package */ StaticIpConfiguration mStaticIpConfig;
         /* package */ ApfCapabilities mApfCapabilities;
+        /* package */ int mProvisioningTimeoutMs = DEFAULT_TIMEOUT_MS;
 
         public ProvisioningConfiguration() {}
 
@@ -301,6 +319,18 @@ public class IpManager extends StateMachine {
             mRequestedPreDhcpAction = other.mRequestedPreDhcpAction;
             mStaticIpConfig = other.mStaticIpConfig;
             mApfCapabilities = other.mApfCapabilities;
+            mProvisioningTimeoutMs = other.mProvisioningTimeoutMs;
+        }
+
+        @Override
+        public String toString() {
+            return new StringJoiner(", ", getClass().getSimpleName() + "{", "}")
+                    .add("mUsingIpReachabilityMonitor: " + mUsingIpReachabilityMonitor)
+                    .add("mRequestedPreDhcpAction: " + mRequestedPreDhcpAction)
+                    .add("mStaticIpConfig: " + mStaticIpConfig)
+                    .add("mApfCapabilities: " + mApfCapabilities)
+                    .add("mProvisioningTimeoutMs: " + mProvisioningTimeoutMs)
+                    .toString();
         }
     }
 
@@ -315,6 +345,7 @@ public class IpManager extends StateMachine {
     private static final int CMD_UPDATE_TCP_BUFFER_SIZES = 6;
     private static final int CMD_UPDATE_HTTP_PROXY = 7;
     private static final int CMD_SET_MULTICAST_FILTER = 8;
+    private static final int EVENT_PROVISIONING_TIMEOUT = 9;
 
     private static final int MAX_LOG_RECORDS = 500;
 
@@ -337,6 +368,7 @@ public class IpManager extends StateMachine {
     protected final Callback mCallback;
     private final INetworkManagementService mNwService;
     private final NetlinkTracker mNetlinkTracker;
+    private final WakeupMessage mProvisioningTimeoutAlarm;
     private final LocalLog mLocalLog;
 
     private NetworkInterface mNetworkInterface;
@@ -411,6 +443,9 @@ public class IpManager extends StateMachine {
 
         resetLinkProperties();
 
+        mProvisioningTimeoutAlarm = new WakeupMessage(mContext, getHandler(),
+                mTag + ".EVENT_PROVISIONING_TIMEOUT", EVENT_PROVISIONING_TIMEOUT);
+
         // Super simple StateMachine.
         addState(mStoppedState);
         addState(mStartedState);
@@ -649,7 +684,6 @@ public class IpManager extends StateMachine {
     }
 
     private void dispatchCallback(ProvisioningChange delta, LinkProperties newLp) {
-        if (mApfFilter != null) mApfFilter.setLinkProperties(newLp);
         switch (delta) {
             case GAINED_PROVISIONING:
                 if (VDBG) { Log.d(mTag, "onProvisioningSuccess()"); }
@@ -670,7 +704,13 @@ public class IpManager extends StateMachine {
         }
     }
 
+    // Updates all IpManager-related state concerned with LinkProperties.
+    // Returns a ProvisioningChange for possibly notifying other interested
+    // parties that are not fronted by IpManager.
     private ProvisioningChange setLinkProperties(LinkProperties newLp) {
+        if (mApfFilter != null) {
+            mApfFilter.setLinkProperties(newLp);
+        }
         if (mIpReachabilityMonitor != null) {
             mIpReachabilityMonitor.updateLinkProperties(newLp);
         }
@@ -678,13 +718,10 @@ public class IpManager extends StateMachine {
         ProvisioningChange delta = compareProvisioning(mLinkProperties, newLp);
         mLinkProperties = new LinkProperties(newLp);
 
-        if (DBG) {
+        if (delta == ProvisioningChange.GAINED_PROVISIONING) {
-            switch (delta) {
+            // TODO: Add a proper ProvisionedState and cancel the alarm in
-                case GAINED_PROVISIONING:
+            // its enter() method.
-                case LOST_PROVISIONING:
+            mProvisioningTimeoutAlarm.cancel();
-                    Log.d(mTag, "provisioning: " + delta);
-                    break;
-            }
         }
 
         return delta;
@@ -798,33 +835,39 @@ public class IpManager extends StateMachine {
             Log.d(mTag, "onNewDhcpResults(" + Objects.toString(dhcpResults) + ")");
         }
         mCallback.onNewDhcpResults(dhcpResults);
-
         dispatchCallback(delta, newLp);
     }
 
     private void handleIPv4Failure() {
+        // TODO: Investigate deleting this clearIPv4Address() call.
+        //
+        // DhcpClient will send us CMD_CLEAR_LINKADDRESS in all circumstances
+        // that could trigger a call to this function. If we missed handling
+        // that message in StartedState for some reason we would still clear
+        // any addresses upon entry to StoppedState.
         clearIPv4Address();
         mDhcpResults = null;
+        if (VDBG) { Log.d(mTag, "onNewDhcpResults(null)"); }
+        mCallback.onNewDhcpResults(null);
+
+        handleProvisioningFailure();
+    }
+
+    private void handleProvisioningFailure() {
         final LinkProperties newLp = assembleLinkProperties();
         ProvisioningChange delta = setLinkProperties(newLp);
         // If we've gotten here and we're still not provisioned treat that as
         // a total loss of provisioning.
         //
         // Either (a) static IP configuration failed or (b) DHCPv4 failed AND
-        // there was no usable IPv6 obtained before the DHCPv4 timeout.
+        // there was no usable IPv6 obtained before a non-zero provisioning
+        // timeout expired.
         //
         // Regardless: GAME OVER.
-        //
-        // TODO: Make the DHCP client not time out and just continue in
-        // exponential backoff. Callers such as Wi-Fi which need a timeout
-        // should implement it themselves.
         if (delta == ProvisioningChange.STILL_NOT_PROVISIONED) {
             delta = ProvisioningChange.LOST_PROVISIONING;
         }
 
-        if (VDBG) { Log.d(mTag, "onNewDhcpResults(null)"); }
-        mCallback.onNewDhcpResults(null);
-
         dispatchCallback(delta, newLp);
         if (delta == ProvisioningChange.LOST_PROVISIONING) {
             transitionTo(mStoppingState);
@@ -968,11 +1011,19 @@ public class IpManager extends StateMachine {
                         mInterfaceName);
                 mDhcpClient.registerForPreDhcpNotification();
                 mDhcpClient.sendMessage(DhcpClient.CMD_START_DHCP);
+
+                if (mConfiguration.mProvisioningTimeoutMs > 0) {
+                    final long alarmTime = SystemClock.elapsedRealtime() +
+                            mConfiguration.mProvisioningTimeoutMs;
+                    mProvisioningTimeoutAlarm.schedule(alarmTime);
+                }
             }
         }
 
         @Override
         public void exit() {
+            mProvisioningTimeoutAlarm.cancel();
+
             if (mIpReachabilityMonitor != null) {
                 mIpReachabilityMonitor.stop();
                 mIpReachabilityMonitor = null;
@@ -995,7 +1046,7 @@ public class IpManager extends StateMachine {
         public boolean processMessage(Message msg) {
             switch (msg.what) {
                 case CMD_STOP:
-                    transitionTo(mStoppedState);
+                    transitionTo(mStoppingState);
                     break;
 
                 case CMD_START:
@@ -1023,7 +1074,7 @@ public class IpManager extends StateMachine {
 
                 case EVENT_NETLINK_LINKPROPERTIES_CHANGED:
                     if (!handleLinkPropertiesUpdate(SEND_CALLBACKS)) {
-                        transitionTo(mStoppedState);
+                        transitionTo(mStoppingState);
                     }
                     break;
 
@@ -1049,6 +1100,10 @@ public class IpManager extends StateMachine {
                     break;
                 }
 
+                case EVENT_PROVISIONING_TIMEOUT:
+                    handleProvisioningFailure();
+                    break;
+
                 case DhcpClient.CMD_PRE_DHCP_ACTION:
                     if (VDBG) { Log.d(mTag, "onPreDhcpAction()"); }
                     if (mConfiguration.mRequestedPreDhcpAction) {