commit f794e7d4f903d2fe9cda0a06ebfcca9d1ced440a
author Motomu Utsumi <motomuman@google.com> Wed Aug 03 06:38:43 2022 +0000
committer Motomu Utsumi <motomuman@google.com> Tue Aug 09 04:00:02 2022 +0000
tree b4fce3cb71f8a07551d63fce2d8f636ec2b861f6
parent b2d32b74710f3f523fc1b008e4a18d6c24260044

Switch previous and new code path in addUidInterfaceRules

addUidInterfaceRules was updated to use Java BpfMap class instead of native JNI
functions in aosp/2141538.
But it is better to verify this refactoring by the experiment.
So this commit update addUidInterfaceRules to switch previous code path(JNI)
and new code path(Java) based on the flag.

Also this commit removes @GuardedBy("sUidOwnerMap") from
native_addUidInterfaceRules.
This was needed to avoid JNI and Java code modify the UidOwnerMap
concurrently when some API uses previous code path (JNI) and other APIs
use new code path (Java BpfMap class).
But, after this topic, it will not be needed because all the APIs will use JNI
"or" all the APIs will use Java BpfMap class to update UidOwnerMap.

Bug: 217624062
Test: atest BpfNetMapsTest HostsideVpnTests#testBlockIncomingPacket
Change-Id: Idb6400ed5808f8c5f4b0a962a356ac3850fbcff3

service/src/com/android/server/BpfNetMaps.java

diff --git a/service/src/com/android/server/BpfNetMaps.java b/service/src/com/android/server/BpfNetMaps.java
index 6d1d3cd..d77fb73 100644
--- a/service/src/com/android/server/BpfNetMaps.java
+++ b/service/src/com/android/server/BpfNetMaps.java
@@ -566,24 +566,30 @@
             mNetd.firewallAddUidInterfaceRules(ifName, uids);
             return;
         }
-        // Null ifName is a wildcard to allow apps to receive packets on all interfaces and ifIndex
-        // is set to 0.
-        final int ifIndex;
-        if (ifName == null) {
-            ifIndex = 0;
+
+        if (sEnableJavaBpfMap) {
+            // Null ifName is a wildcard to allow apps to receive packets on all interfaces and
+            // ifIndex is set to 0.
+            final int ifIndex;
+            if (ifName == null) {
+                ifIndex = 0;
+            } else {
+                ifIndex = mDeps.getIfIndex(ifName);
+                if (ifIndex == 0) {
+                    throw new ServiceSpecificException(ENODEV,
+                            "Failed to get index of interface " + ifName);
+                }
+            }
+            for (final int uid : uids) {
+                try {
+                    addRule(uid, IIF_MATCH, ifIndex, "addUidInterfaceRules");
+                } catch (ServiceSpecificException e) {
+                    Log.e(TAG, "addRule failed uid=" + uid + " ifName=" + ifName + ", " + e);
+                }
+            }
         } else {
-            ifIndex = mDeps.getIfIndex(ifName);
-            if (ifIndex == 0) {
-                throw new ServiceSpecificException(ENODEV,
-                        "Failed to get index of interface " + ifName);
-            }
-        }
-        for (final int uid: uids) {
-            try {
-                addRule(uid, IIF_MATCH, ifIndex, "addUidInterfaceRules");
-            } catch (ServiceSpecificException e) {
-                Log.e(TAG, "addRule failed uid=" + uid + " ifName=" + ifName + ", " + e);
-            }
+            final int err = native_addUidInterfaceRules(ifName, uids);
+            maybeThrow(err, "Unable to add uid interface rules");
         }
     }
 
@@ -691,7 +697,6 @@
     private native int native_replaceUidChain(String name, boolean isAllowlist, int[] uids);
     @GuardedBy("sUidOwnerMap")
     private native int native_setUidRule(int childChain, int uid, int firewallRule);
-    @GuardedBy("sUidOwnerMap")
     private native int native_addUidInterfaceRules(String ifName, int[] uids);
     @GuardedBy("sUidOwnerMap")
     private native int native_removeUidInterfaceRules(int[] uids);