Retry networkAddInterface on EBUSY errorCode am: 9163b2a5ce

Original change: https://googleplex-android-review.googlesource.com/c/platform/packages/modules/NetworkStack/+/11910087

Change-Id: If45954c86175de659d76a861daff77d3ead51020

2 files changed, 201 insertions, 2 deletions

diff --git a/common/moduleutils/src/android/net/shared/NetdUtils.java b/common/moduleutils/src/android/net/shared/NetdUtils.java
index 0137bb7..5fa29c9 100644
--- a/common/moduleutils/src/android/net/shared/NetdUtils.java
+++ b/common/moduleutils/src/android/net/shared/NetdUtils.java
@@ -17,6 +17,7 @@
 package android.net.shared;
 
 import static android.net.RouteInfo.RTN_UNICAST;
+import static android.system.OsConstants.EBUSY;
 
 import android.net.INetd;
 import android.net.IpPrefix;
@@ -24,6 +25,8 @@ import android.net.RouteInfo;
 import android.net.TetherConfigParcel;
 import android.os.RemoteException;
 import android.os.ServiceSpecificException;
+import android.os.SystemClock;
+import android.util.Log;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -33,6 +36,8 @@ import java.util.List;
  * @hide
  */
 public class NetdUtils {
+    private static final String TAG = NetdUtils.class.getSimpleName();
+
     /** Start tethering. */
     public static void tetherStart(final INetd netd, final boolean usingLegacyDnsProxy,
             final String[] dhcpRange) throws RemoteException, ServiceSpecificException {
@@ -45,14 +50,46 @@ public class NetdUtils {
     /** Setup interface for tethering. */
     public static void tetherInterface(final INetd netd, final String iface, final IpPrefix dest)
             throws RemoteException, ServiceSpecificException {
-        netd.tetherInterfaceAdd(iface);
+        tetherInterface(netd, iface, dest, 20 /* maxAttempts */, 50 /* pollingIntervalMs */);
+    }
 
-        netd.networkAddInterface(INetd.LOCAL_NET_ID, iface);
+    /** Setup interface with configurable retries for tethering. */
+    public static void tetherInterface(final INetd netd, final String iface, final IpPrefix dest,
+            int maxAttempts, int pollingIntervalMs)
+            throws RemoteException, ServiceSpecificException {
+        netd.tetherInterfaceAdd(iface);
+        networkAddInterface(netd, iface, maxAttempts, pollingIntervalMs);
         List<RouteInfo> routes = new ArrayList<>();
         routes.add(new RouteInfo(dest, null, iface, RTN_UNICAST));
         RouteUtils.addRoutesToLocalNetwork(netd, iface, routes);
     }
 
+    /**
+     * Retry Netd#networkAddInterface for EBUSY error code.
+     * If the same interface (e.g., wlan0) is in client mode and then switches to tethered mode.
+     * There can be a race where puts the interface into the local network but interface is still
+     * in use in netd because the ConnectivityService thread hasn't processed the disconnect yet.
+     * See b/158269544 for detail.
+     */
+    private static void networkAddInterface(final INetd netd, final String iface,
+            int maxAttempts, int pollingIntervalMs)
+            throws ServiceSpecificException, RemoteException {
+        for (int i = 1; i <= maxAttempts; i++) {
+            try {
+                netd.networkAddInterface(INetd.LOCAL_NET_ID, iface);
+                return;
+            } catch (ServiceSpecificException e) {
+                if (e.errorCode == EBUSY && i < maxAttempts) {
+                    SystemClock.sleep(pollingIntervalMs);
+                    continue;
+                }
+
+                Log.e(TAG, "Retry Netd#networkAddInterface failure: " + e);
+                throw e;
+            }
+        }
+    }
+
     /** Reset interface for tethering. */
     public static void untetherInterface(final INetd netd, String iface)
             throws RemoteException, ServiceSpecificException {
diff --git a/tests/unit/src/android/net/shared/NetdUtilsTest.java b/tests/unit/src/android/net/shared/NetdUtilsTest.java
new file mode 100644
index 0000000..f3ef53a
--- /dev/null
+++ b/tests/unit/src/android/net/shared/NetdUtilsTest.java
@@ -0,0 +1,162 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.net.shared;
+
+import static android.net.INetd.LOCAL_NET_ID;
+import static android.system.OsConstants.EBUSY;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyInt;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+
+import android.net.INetd;
+import android.net.IpPrefix;
+import android.os.RemoteException;
+import android.os.ServiceSpecificException;
+
+import androidx.test.filters.SmallTest;
+import androidx.test.runner.AndroidJUnit4;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+@RunWith(AndroidJUnit4.class)
+@SmallTest
+public final class NetdUtilsTest {
+    private static final String IFACE_NAME = "testnet1";
+    private static final IpPrefix TEST_IPPREFIX = new IpPrefix("192.168.42.1/24");
+
+    @Mock private INetd mNetd;
+
+    @Before public void setUp() throws Exception {
+        MockitoAnnotations.initMocks(this);
+    }
+
+    private void setNetworkAddInterfaceOutcome(final Exception cause, int numLoops)
+            throws Exception {
+        // This cannot be an int because local variables referenced from a lambda expression must
+        // be final or effectively final.
+        final Counter myCounter = new Counter();
+        doAnswer((invocation) -> {
+            myCounter.count();
+            if (myCounter.isCounterReached(numLoops)) {
+                if (cause == null) return null;
+
+                throw cause;
+            }
+
+            throw new ServiceSpecificException(EBUSY);
+        }).when(mNetd).networkAddInterface(LOCAL_NET_ID, IFACE_NAME);
+    }
+
+    class Counter {
+        private int mValue = 0;
+
+        private void count() {
+            mValue++;
+        }
+
+        private boolean isCounterReached(int target) {
+            return mValue >= target;
+        }
+    }
+
+    private void verifyTetherInterfaceSucceeds(int expectedTries) throws Exception {
+        setNetworkAddInterfaceOutcome(null, expectedTries);
+
+        NetdUtils.tetherInterface(mNetd, IFACE_NAME, TEST_IPPREFIX);
+        verify(mNetd).tetherInterfaceAdd(IFACE_NAME);
+        verify(mNetd, times(expectedTries)).networkAddInterface(LOCAL_NET_ID, IFACE_NAME);
+        verify(mNetd, times(2)).networkAddRoute(eq(LOCAL_NET_ID), eq(IFACE_NAME), any(), any());
+        verifyNoMoreInteractions(mNetd);
+        reset(mNetd);
+    }
+
+    @Test
+    public void testTetherInterfaceSuccessful() throws Exception {
+        // Expect #networkAddInterface successful at first tries.
+        verifyTetherInterfaceSucceeds(1);
+
+        // Expect #networkAddInterface successful after 10 tries.
+        verifyTetherInterfaceSucceeds(10);
+    }
+
+    private void runTetherInterfaceWithServiceSpecificException(int expectedTries,
+            int expectedCode) throws Exception {
+        setNetworkAddInterfaceOutcome(new ServiceSpecificException(expectedCode), expectedTries);
+
+        try {
+            NetdUtils.tetherInterface(mNetd, IFACE_NAME, TEST_IPPREFIX, 20, 0);
+            fail("Expect throw ServiceSpecificException");
+        } catch (ServiceSpecificException e) {
+            assertEquals(e.errorCode, expectedCode);
+        }
+
+        verifyNetworkAddInterfaceFails(expectedTries);
+        reset(mNetd);
+    }
+
+    private void runTetherInterfaceWithRemoteException(int expectedTries) throws Exception {
+        setNetworkAddInterfaceOutcome(new RemoteException(), expectedTries);
+
+        try {
+            NetdUtils.tetherInterface(mNetd, IFACE_NAME, TEST_IPPREFIX, 20, 0);
+            fail("Expect throw RemoteException");
+        } catch (RemoteException e) { }
+
+        verifyNetworkAddInterfaceFails(expectedTries);
+        reset(mNetd);
+    }
+
+    private void verifyNetworkAddInterfaceFails(int expectedTries) throws Exception {
+        verify(mNetd).tetherInterfaceAdd(IFACE_NAME);
+        verify(mNetd, times(expectedTries)).networkAddInterface(LOCAL_NET_ID, IFACE_NAME);
+        verify(mNetd, never()).networkAddRoute(anyInt(), anyString(), any(), any());
+        verifyNoMoreInteractions(mNetd);
+    }
+
+    @Test
+    public void testTetherInterfaceFailOnNetworkAddInterface() throws Exception {
+        // Test throwing ServiceSpecificException with EBUSY failure.
+        runTetherInterfaceWithServiceSpecificException(20, EBUSY);
+
+        // Test throwing ServiceSpecificException with unexpectedError.
+        final int unexpectedError = 999;
+        runTetherInterfaceWithServiceSpecificException(1, unexpectedError);
+
+        // Test throwing ServiceSpecificException with unexpectedError after 7 tries.
+        runTetherInterfaceWithServiceSpecificException(7, unexpectedError);
+
+        // Test throwing RemoteException.
+        runTetherInterfaceWithRemoteException(1);
+
+        // Test throwing RemoteException after 3 tries.
+        runTetherInterfaceWithRemoteException(3);
+    }
+}