diff --git a/service/src/com/android/server/connectivity/ClatCoordinator.java b/service/src/com/android/server/connectivity/ClatCoordinator.java index 33494d44a3..4a7c77adf8 100644 --- a/service/src/com/android/server/connectivity/ClatCoordinator.java +++ b/service/src/com/android/server/connectivity/ClatCoordinator.java @@ -39,6 +39,7 @@ 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; @@ -115,12 +116,10 @@ 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 BpfMap mIngressMap = null; + private final IBpfMap mIngressMap; @Nullable - private BpfMap mEgressMap = null; + private final IBpfMap mEgressMap; @Nullable private ClatdTracker mClatdTracker = null; @@ -248,7 +247,7 @@ public class ClatCoordinator { /** Get ingress6 BPF map. */ @Nullable - public BpfMap getBpfIngress6Map() { + public IBpfMap 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. @@ -265,7 +264,7 @@ public class ClatCoordinator { /** Get egress4 BPF map. */ @Nullable - public BpfMap getBpfEgress4Map() { + public IBpfMap 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. @@ -374,35 +373,12 @@ public class ClatCoordinator { public ClatCoordinator(@NonNull Dependencies deps) { mDeps = deps; mNetd = mDeps.getNetd(); - } - - 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; + mIngressMap = mDeps.getBpfIngress6Map(); + mEgressMap = mDeps.getBpfEgress4Map(); } private void maybeStartBpf(final ClatdTracker tracker) { - mEgressMap = mDeps.getBpfEgress4Map(); - if (mEgressMap == null) return; - - mIngressMap = mDeps.getBpfIngress6Map(); - if (mIngressMap == null) { - closeEgressMap(); - return; - } + if (mIngressMap == null || mEgressMap == null) return; final boolean isEthernet; try { @@ -746,13 +722,6 @@ 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(); } /** @@ -821,7 +790,7 @@ public class ClatCoordinator { } /** - * Dump the cordinator information. Only called when clat is started. See Nat464Xlat#dump. + * Dump the cordinator information. * * @param pw print writer. */ diff --git a/tests/unit/java/com/android/server/connectivity/ClatCoordinatorTest.java b/tests/unit/java/com/android/server/connectivity/ClatCoordinatorTest.java index de5698fc4b..f84d10f395 100644 --- a/tests/unit/java/com/android/server/connectivity/ClatCoordinatorTest.java +++ b/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.BpfMap; +import com.android.net.module.util.IBpfMap; 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 BpfMap mIngressMap; - @Mock private BpfMap mEgressMap; + @Mock private IBpfMap mIngressMap; + @Mock private IBpfMap 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 BpfMap getBpfIngress6Map() { + public IBpfMap getBpfIngress6Map() { return mIngressMap; } /** Get egress4 BPF map. */ @Override - public BpfMap getBpfEgress4Map() { + public IBpfMap getBpfEgress4Map() { return mEgressMap; } @@ -447,8 +447,6 @@ 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)); @@ -471,8 +469,6 @@ 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));