Open and close clat bpf map while clat is starting and stoping

BpfMap class supports AutoCloseable interface which closes
file descriptor only in try-exit. BpfMap class doesn't close
fds while the object is released.

Change the timing of opening and closing bpf map file descriptors
to clat is starting and stoping.

Moreover, the reason that manual close BPF map file descriptors is
as follows. Just don't rely on that GC releasing to close the file
descriptors even if class BpfMap supports close file descriptor in
finalize(). If the interfaces are added and removed quickly, too
many unclosed file descriptors may cause unexpected problems.

Bug: 230880517
Test: manual test
Steps:
1. Connect to IPv6 only wifi (GoogleGuest) and mobile data
2. Check that map fds are appeared:
   /proc/$(system_server_pid)/fd/$(bpf_map_fd)

$ adb shell ps | grep system_server
system        1929   825 20311224 730060 do_epoll_wait      0 S system_server

$ adb shell ls -all proc/1929/fd | grep bpf-map
.. system system 64 2022-05-05 13:36:42 .. 331 -> anon_inode:bpf-map
.. system system 64 2022-05-05 13:36:42 .. 348 -> anon_inode:bpf-map

3. Check the clat maps are added.
$ adb shell dumpsys connectivity
NetworkAgentInfo{network{105}  handle{454377263117}  ni{WIFI ..
Nat464Xlat:
..
  Forwarding rules:
    BPF ingress map: iif nat64Prefix v6Addr -> v4Addr oif
      47 /64:ff9b::/96 /2a00:79e1:abc:6f02:f182:6c29:ab56:9961 -> /192.0.0.4 62
    BPF egress map: iif v4Addr -> v6Addr nat64Prefix oif
      62 /192.0.0.4 -> /2a00:79e1:abc:6f02:f182:6c29:ab56:9961 /64:ff9b::/96 47 ether
NetworkAgentInfo{network{106}  handle{458672230413}  ni{MOBILE[LTE] ..
    Nat464Xlat:
      <not start>

4. Disconnect from wifi

5. Check that map fds are disappeared:
   /proc/$(system_server_pid)/fd/$(bpf_map_fd)

$ adb shell ls -all proc/1929/fd | grep bpf-map
(fd 331 and 348 were not found)

Change-Id: I60c0301bf00beae5cf5ab3535c6a3da68a2a4a9b

service/src/com/android/server/connectivity/ClatCoordinator.java

@@ -39,7 +39,6 @@ import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.IndentingPrintWriter;
 import com.android.modules.utils.build.SdkLevel;
 import com.android.net.module.util.BpfMap;
-import com.android.net.module.util.IBpfMap;
 import com.android.net.module.util.InterfaceParams;
 import com.android.net.module.util.TcUtils;
 import com.android.net.module.util.bpf.ClatEgress4Key;
@@ -116,10 +115,12 @@ public class ClatCoordinator {
     private final INetd mNetd;
     @NonNull
     private final Dependencies mDeps;
+    // BpfMap objects {mIngressMap, mEgressMap} are initialized in #maybeStartBpf and closed in
+    // #maybeStopBpf.
     @Nullable
-    private final IBpfMap<ClatIngress6Key, ClatIngress6Value> mIngressMap;
+    private BpfMap<ClatIngress6Key, ClatIngress6Value> mIngressMap = null;
     @Nullable
-    private final IBpfMap<ClatEgress4Key, ClatEgress4Value> mEgressMap;
+    private BpfMap<ClatEgress4Key, ClatEgress4Value> mEgressMap = null;
     @Nullable
     private ClatdTracker mClatdTracker = null;
 
@@ -247,7 +248,7 @@ public class ClatCoordinator {
 
         /** Get ingress6 BPF map. */
         @Nullable
-        public IBpfMap<ClatIngress6Key, ClatIngress6Value> getBpfIngress6Map() {
+        public BpfMap<ClatIngress6Key, ClatIngress6Value> getBpfIngress6Map() {
             // Pre-T devices don't use ClatCoordinator to access clat map. Since Nat464Xlat
             // initializes a ClatCoordinator object to avoid redundant null pointer check
             // while using, ignore the BPF map initialization on pre-T devices.
@@ -264,7 +265,7 @@ public class ClatCoordinator {
 
         /** Get egress4 BPF map. */
         @Nullable
-        public IBpfMap<ClatEgress4Key, ClatEgress4Value> getBpfEgress4Map() {
+        public BpfMap<ClatEgress4Key, ClatEgress4Value> getBpfEgress4Map() {
             // Pre-T devices don't use ClatCoordinator to access clat map. Since Nat464Xlat
             // initializes a ClatCoordinator object to avoid redundant null pointer check
             // while using, ignore the BPF map initialization on pre-T devices.
@@ -373,12 +374,35 @@ public class ClatCoordinator {
     public ClatCoordinator(@NonNull Dependencies deps) {
         mDeps = deps;
         mNetd = mDeps.getNetd();
-        mIngressMap = mDeps.getBpfIngress6Map();
+    }
-        mEgressMap = mDeps.getBpfEgress4Map();
+
+    private void closeEgressMap() {
+        try {
+            mEgressMap.close();
+        } catch (ErrnoException e) {
+            Log.e(TAG, "Cannot close egress4 map: " + e);
+        }
+        mEgressMap = null;
+    }
+
+    private void closeIngressMap() {
+        try {
+            mIngressMap.close();
+        } catch (ErrnoException e) {
+            Log.e(TAG, "Cannot close ingress6 map: " + e);
+        }
+        mIngressMap = null;
     }
 
     private void maybeStartBpf(final ClatdTracker tracker) {
-        if (mIngressMap == null || mEgressMap == null) return;
+        mEgressMap = mDeps.getBpfEgress4Map();
+        if (mEgressMap == null) return;
+
+        mIngressMap = mDeps.getBpfIngress6Map();
+        if (mIngressMap == null) {
+            closeEgressMap();
+            return;
+        }
 
         final boolean isEthernet;
         try {
@@ -722,6 +746,13 @@ public class ClatCoordinator {
         } catch (ErrnoException | IllegalStateException e) {
             Log.e(TAG, "Could not delete entry (" + rxKey + "): " + e);
         }
+
+        // Manual close BPF map file descriptors. Just don't rely on that GC releasing to close
+        // the file descriptors even if class BpfMap supports close file descriptor in
+        // finalize(). If the interfaces are added and removed quickly, too many unclosed file
+        // descriptors may cause unexpected problem.
+        closeEgressMap();
+        closeIngressMap();
     }
 
     /**
@@ -790,7 +821,7 @@ public class ClatCoordinator {
     }
 
     /**
-     * Dump the cordinator information.
+     * Dump the cordinator information. Only called when clat is started. See Nat464Xlat#dump.
      *
      * @param pw print writer.
      */

tests/unit/java/com/android/server/connectivity/ClatCoordinatorTest.java

@@ -46,7 +46,7 @@ import android.os.ParcelFileDescriptor;
 
 import androidx.test.filters.SmallTest;
 
-import com.android.net.module.util.IBpfMap;
+import com.android.net.module.util.BpfMap;
 import com.android.net.module.util.bpf.ClatEgress4Key;
 import com.android.net.module.util.bpf.ClatEgress4Value;
 import com.android.net.module.util.bpf.ClatIngress6Key;
@@ -123,8 +123,8 @@ public class ClatCoordinatorTest {
 
     @Mock private INetd mNetd;
     @Spy private TestDependencies mDeps = new TestDependencies();
-    @Mock private IBpfMap<ClatIngress6Key, ClatIngress6Value> mIngressMap;
+    @Mock private BpfMap<ClatIngress6Key, ClatIngress6Value> mIngressMap;
-    @Mock private IBpfMap<ClatEgress4Key, ClatEgress4Value> mEgressMap;
+    @Mock private BpfMap<ClatEgress4Key, ClatEgress4Value> mEgressMap;
 
     /**
       * The dependency injection class is used to mock the JNI functions and system functions
@@ -326,13 +326,13 @@ public class ClatCoordinatorTest {
 
         /** Get ingress6 BPF map. */
         @Override
-        public IBpfMap<ClatIngress6Key, ClatIngress6Value> getBpfIngress6Map() {
+        public BpfMap<ClatIngress6Key, ClatIngress6Value> getBpfIngress6Map() {
             return mIngressMap;
         }
 
         /** Get egress4 BPF map. */
         @Override
-        public IBpfMap<ClatEgress4Key, ClatEgress4Value> getBpfEgress4Map() {
+        public BpfMap<ClatEgress4Key, ClatEgress4Value> getBpfEgress4Map() {
             return mEgressMap;
         }
 
@@ -447,6 +447,8 @@ public class ClatCoordinatorTest {
                 argThat(fd -> Objects.equals(RAW_SOCK_PFD.getFileDescriptor(), fd)),
                 eq(BASE_IFACE), eq(NAT64_PREFIX_STRING),
                 eq(XLAT_LOCAL_IPV4ADDR_STRING), eq(XLAT_LOCAL_IPV6ADDR_STRING));
+        inOrder.verify(mDeps).getBpfEgress4Map();
+        inOrder.verify(mDeps).getBpfIngress6Map();
         inOrder.verify(mEgressMap).insertEntry(eq(EGRESS_KEY), eq(EGRESS_VALUE));
         inOrder.verify(mIngressMap).insertEntry(eq(INGRESS_KEY), eq(INGRESS_VALUE));
         inOrder.verify(mDeps).tcQdiscAddDevClsact(eq(STACKED_IFINDEX));
@@ -469,6 +471,8 @@ public class ClatCoordinatorTest {
                 eq((short) PRIO_CLAT), eq((short) ETH_P_IP));
         inOrder.verify(mEgressMap).deleteEntry(eq(EGRESS_KEY));
         inOrder.verify(mIngressMap).deleteEntry(eq(INGRESS_KEY));
+        inOrder.verify(mEgressMap).close();
+        inOrder.verify(mIngressMap).close();
         inOrder.verify(mDeps).stopClatd(eq(BASE_IFACE), eq(NAT64_PREFIX_STRING),
                 eq(XLAT_LOCAL_IPV4ADDR_STRING), eq(XLAT_LOCAL_IPV6ADDR_STRING), eq(CLATD_PID));
         inOrder.verify(mDeps).untagSocket(eq(RAW_SOCK_COOKIE));