Do not enable ingress rate limit until clsact qdisc exists
The tc police filter attaches to the clsact qdisc, so the rate limit
cannot be enabled before the qdisc is added to the interface.
The clsact qdisc is added as part of INetd#networkAddInterface, which is
called from inside updateLinkProperties.
Test: atest FrameworksNetTests:ConnectivityServiceTest
Change-Id: I0713605ff3684f8271eb3f0e29ab7116561963f1
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