Do not enable ingress rate limit until clsact qdisc exists am: f1fe8ee928

Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/2003978

Change-Id: I81ef8189ca04738b16a584e079e73ffa92e31a9f
diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
index c90fcd5..eabcd10 100644
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -4242,7 +4242,7 @@
             mDnsManager.removeNetwork(nai.network);
 
             // clean up tc police filters on interface.
-            if (canNetworkBeRateLimited(nai) && mIngressRateLimit >= 0) {
+            if (nai.everConnected && canNetworkBeRateLimited(nai) && mIngressRateLimit >= 0) {
                 mDeps.disableIngressRateLimit(nai.linkProperties.getInterfaceName());
             }
         }
@@ -9011,19 +9011,6 @@
             // A network that has just connected has zero requests and is thus a foreground network.
             networkAgent.networkCapabilities.addCapability(NET_CAPABILITY_FOREGROUND);
 
-            // If a rate limit has been configured and is applicable to this network (network
-            // provides internet connectivity), apply it.
-            // Note: in case of a system server crash, there is a very small chance that this
-            // leaves some interfaces rate limited (i.e. if the rate limit had been changed just
-            // before the crash and was never applied). One solution would be to delete all
-            // potential tc police filters every time this is called. Since this is an unlikely
-            // scenario in the first place (and worst case, the interface stays rate limited until
-            // the device is rebooted), this seems a little overkill.
-            if (canNetworkBeRateLimited(networkAgent) && mIngressRateLimit >= 0) {
-                mDeps.enableIngressRateLimit(networkAgent.linkProperties.getInterfaceName(),
-                        mIngressRateLimit);
-            }
-
             if (!createNativeNetwork(networkAgent)) return;
             if (networkAgent.propagateUnderlyingCapabilities()) {
                 // Initialize the network's capabilities to their starting values according to the
@@ -9047,6 +9034,17 @@
             updateLinkProperties(networkAgent, new LinkProperties(networkAgent.linkProperties),
                     null);
 
+            // If a rate limit has been configured and is applicable to this network (network
+            // provides internet connectivity), apply it. The tc police filter cannot be attached
+            // before the clsact qdisc is added which happens as part of updateLinkProperties ->
+            // updateInterfaces -> INetd#networkAddInterface.
+            // Note: in case of a system server crash, the NetworkController constructor in netd
+            // (called when netd starts up) deletes the clsact qdisc of all interfaces.
+            if (canNetworkBeRateLimited(networkAgent) && mIngressRateLimit >= 0) {
+                mDeps.enableIngressRateLimit(networkAgent.linkProperties.getInterfaceName(),
+                        mIngressRateLimit);
+            }
+
             // Until parceled LinkProperties are sent directly to NetworkMonitor, the connect
             // command must be sent after updating LinkProperties to maximize chances of
             // NetworkMonitor seeing the correct LinkProperties when starting.
diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
index 098d48a..777da17 100644
--- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
@@ -1998,6 +1998,13 @@
             // updated. Check that this happened.
             assertEquals(-1L, (long) mActiveRateLimit.getOrDefault(iface, -1L));
             mActiveRateLimit.put(iface, rateInBytesPerSecond);
+            // verify that clsact qdisc has already been created, otherwise attaching a tc police
+            // filter will fail.
+            try {
+                verify(mMockNetd).networkAddInterface(anyInt(), eq(iface));
+            } catch (RemoteException e) {
+                fail(e.getMessage());
+            }
         }
 
         @Override