From addebccc56c3c21385a219c9d4e7450cfdb3c3bb Mon Sep 17 00:00:00 2001
From: Sudheer Shanka <sudheersai@google.com>
Date: Tue, 3 Oct 2017 09:43:20 -0700
Subject: [PATCH] Update NPMS to inform NMS of the changes in fw rules
 synchronously.

Currently, NPMS informs NMS synchronously at some events and
asynchronously at some others. If these sync and async calls get
interleaved, then NMS will end up in an inconsistent state.

In N-MR1, NPMS was updated to inform NMS asynchronously to avoid some
lock contentions during screen unlock. This shouldn't be a problem any more
since NPMS no longer updates its internal state on ui thread and also
netd calls have been optimized from ~50ms to ~7ms since then.

Bug: 66015813
Test: cts-tradefed run singleCommand cts-dev -m CtsHostsideNetworkTests -t \
      com.android.cts.net.HostsideRestrictBackgroundNetworkTests

Change-Id: I24ccb5819430f19014630e7b90cba1f0b993e430
---
 .../net/NetworkPolicyManagerService.java      | 32 +++++++------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
index 9fd54ec4f7891..5159c70e991c8 100644
--- a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
+++ b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java
@@ -332,7 +332,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
     private static final int MSG_UPDATE_INTERFACE_QUOTA = 10;
     private static final int MSG_REMOVE_INTERFACE_QUOTA = 11;
     private static final int MSG_POLICIES_CHANGED = 13;
-    private static final int MSG_SET_FIREWALL_RULES = 14;
     private static final int MSG_RESET_FIREWALL_RULES_BY_UID = 15;
 
     private static final int UID_MSG_STATE_CHANGED = 100;
@@ -3184,9 +3183,9 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
                     uidRules.put(mUidState.keyAt(i), FIREWALL_RULE_ALLOW);
                 }
             }
-            setUidFirewallRulesAsync(chain, uidRules, CHAIN_TOGGLE_ENABLE);
+            setUidFirewallRulesUL(chain, uidRules, CHAIN_TOGGLE_ENABLE);
         } else {
-            setUidFirewallRulesAsync(chain, null, CHAIN_TOGGLE_DISABLE);
+            setUidFirewallRulesUL(chain, null, CHAIN_TOGGLE_DISABLE);
         }
     }
 
@@ -3253,7 +3252,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
                 }
             }
 
-            setUidFirewallRulesAsync(FIREWALL_CHAIN_STANDBY, uidRules, CHAIN_TOGGLE_NONE);
+            setUidFirewallRulesUL(FIREWALL_CHAIN_STANDBY, uidRules, CHAIN_TOGGLE_NONE);
         } finally {
             Trace.traceEnd(Trace.TRACE_TAG_NETWORK);
         }
@@ -3954,18 +3953,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
                     removeInterfaceQuota((String) msg.obj);
                     return true;
                 }
-                case MSG_SET_FIREWALL_RULES: {
-                    final int chain = msg.arg1;
-                    final int toggle = msg.arg2;
-                    final SparseIntArray uidRules = (SparseIntArray) msg.obj;
-                    if (uidRules != null) {
-                        setUidFirewallRules(chain, uidRules);
-                    }
-                    if (toggle != CHAIN_TOGGLE_NONE) {
-                        enableFirewallChainUL(chain, toggle == CHAIN_TOGGLE_ENABLE);
-                    }
-                    return true;
-                }
                 case MSG_RESET_FIREWALL_RULES_BY_UID: {
                     resetUidFirewallRules(msg.arg1);
                     return true;
@@ -4111,15 +4098,20 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
 
     /**
      * Calls {@link #setUidFirewallRules(int, SparseIntArray)} and
-     * {@link #enableFirewallChainUL(int, boolean)} asynchronously.
+     * {@link #enableFirewallChainUL(int, boolean)} synchronously.
      *
      * @param chain firewall chain.
      * @param uidRules new UID rules; if {@code null}, only toggles chain state.
      * @param toggle whether the chain should be enabled, disabled, or not changed.
      */
-    private void setUidFirewallRulesAsync(int chain, @Nullable SparseIntArray uidRules,
+    private void setUidFirewallRulesUL(int chain, @Nullable SparseIntArray uidRules,
             @ChainToggleType int toggle) {
-        mHandler.obtainMessage(MSG_SET_FIREWALL_RULES, chain, toggle, uidRules).sendToTarget();
+        if (uidRules != null) {
+            setUidFirewallRulesUL(chain, uidRules);
+        }
+        if (toggle != CHAIN_TOGGLE_NONE) {
+            enableFirewallChainUL(chain, toggle == CHAIN_TOGGLE_ENABLE);
+        }
     }
 
     /**
@@ -4127,7 +4119,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
      * here to netd.  It will clean up dead rules and make sure the target chain only contains rules
      * specified here.
      */
-    private void setUidFirewallRules(int chain, SparseIntArray uidRules) {
+    private void setUidFirewallRulesUL(int chain, SparseIntArray uidRules) {
         try {
             int size = uidRules.size();
             int[] uids = new int[size];