From 73599a5f5d67c28afb00816d50c21fed33108760 Mon Sep 17 00:00:00 2001 From: Motomu Utsumi Date: Wed, 24 Aug 2022 11:59:21 +0900 Subject: [PATCH] Use IBpfMap type instead of BpfMap type Currently, production code uses BpfMap type and TestBpfMap extends BpfMap but this makes it diffcult to test because TestBpfMap loads the JNI. This CL updates to use IBpfMap type in the production code. Upcoming CL updates TestBpfMap to implement IBpfMap instead of extending BpfMap so that test can still use TestBpfMap but tests do not need to load JNI. Bug: 217624062 Test: atest BpfCoordinatorTest BpfNetMapsTest Change-Id: Ie67e14bf5519fb4427474ecc0fda441877a9555f --- .../apishim/api31/BpfCoordinatorShimImpl.java | 18 ++++----- .../tethering/BpfCoordinator.java | 37 ++++++++++--------- .../tethering/BpfCoordinatorTest.java | 30 +++++++-------- .../src/com/android/server/BpfNetMaps.java | 19 +++++----- .../com/android/server/BpfNetMapsTest.java | 8 ++-- 5 files changed, 57 insertions(+), 55 deletions(-) diff --git a/Tethering/apishim/31/com/android/networkstack/tethering/apishim/api31/BpfCoordinatorShimImpl.java b/Tethering/apishim/31/com/android/networkstack/tethering/apishim/api31/BpfCoordinatorShimImpl.java index 776832fcf2..3cad1c665a 100644 --- a/Tethering/apishim/31/com/android/networkstack/tethering/apishim/api31/BpfCoordinatorShimImpl.java +++ b/Tethering/apishim/31/com/android/networkstack/tethering/apishim/api31/BpfCoordinatorShimImpl.java @@ -28,7 +28,7 @@ import android.util.SparseArray; import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import com.android.net.module.util.BpfMap; +import com.android.net.module.util.IBpfMap; import com.android.net.module.util.IBpfMap.ThrowingBiConsumer; import com.android.net.module.util.SharedLog; import com.android.net.module.util.bpf.Tether4Key; @@ -66,31 +66,31 @@ public class BpfCoordinatorShimImpl // BPF map for downstream IPv4 forwarding. @Nullable - private final BpfMap mBpfDownstream4Map; + private final IBpfMap mBpfDownstream4Map; // BPF map for upstream IPv4 forwarding. @Nullable - private final BpfMap mBpfUpstream4Map; + private final IBpfMap mBpfUpstream4Map; // BPF map for downstream IPv6 forwarding. @Nullable - private final BpfMap mBpfDownstream6Map; + private final IBpfMap mBpfDownstream6Map; // BPF map for upstream IPv6 forwarding. @Nullable - private final BpfMap mBpfUpstream6Map; + private final IBpfMap mBpfUpstream6Map; // BPF map of tethering statistics of the upstream interface since tethering startup. @Nullable - private final BpfMap mBpfStatsMap; + private final IBpfMap mBpfStatsMap; // BPF map of per-interface quota for tethering offload. @Nullable - private final BpfMap mBpfLimitMap; + private final IBpfMap mBpfLimitMap; // BPF map of interface index mapping for XDP. @Nullable - private final BpfMap mBpfDevMap; + private final IBpfMap mBpfDevMap; // Tracking IPv4 rule count while any rule is using the given upstream interfaces. Used for // reducing the BPF map iteration query. The count is increased or decreased when the rule is @@ -482,7 +482,7 @@ public class BpfCoordinatorShimImpl return true; } - private String mapStatus(BpfMap m, String name) { + private String mapStatus(IBpfMap m, String name) { return name + "{" + (m != null ? "OK" : "ERROR") + "}"; } diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index 05a28841b3..142a0b9150 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java @@ -60,6 +60,7 @@ import com.android.modules.utils.build.SdkLevel; import com.android.net.module.util.BpfDump; import com.android.net.module.util.BpfMap; import com.android.net.module.util.CollectionUtils; +import com.android.net.module.util.IBpfMap; import com.android.net.module.util.InterfaceParams; import com.android.net.module.util.NetworkStackConstants; import com.android.net.module.util.SharedLog; @@ -320,7 +321,7 @@ public class BpfCoordinator { } /** Get downstream4 BPF map. */ - @Nullable public BpfMap getBpfDownstream4Map() { + @Nullable public IBpfMap getBpfDownstream4Map() { if (!isAtLeastS()) return null; try { return new BpfMap<>(TETHER_DOWNSTREAM4_MAP_PATH, @@ -332,7 +333,7 @@ public class BpfCoordinator { } /** Get upstream4 BPF map. */ - @Nullable public BpfMap getBpfUpstream4Map() { + @Nullable public IBpfMap getBpfUpstream4Map() { if (!isAtLeastS()) return null; try { return new BpfMap<>(TETHER_UPSTREAM4_MAP_PATH, @@ -344,7 +345,7 @@ public class BpfCoordinator { } /** Get downstream6 BPF map. */ - @Nullable public BpfMap getBpfDownstream6Map() { + @Nullable public IBpfMap getBpfDownstream6Map() { if (!isAtLeastS()) return null; try { return new BpfMap<>(TETHER_DOWNSTREAM6_FS_PATH, @@ -356,7 +357,7 @@ public class BpfCoordinator { } /** Get upstream6 BPF map. */ - @Nullable public BpfMap getBpfUpstream6Map() { + @Nullable public IBpfMap getBpfUpstream6Map() { if (!isAtLeastS()) return null; try { return new BpfMap<>(TETHER_UPSTREAM6_FS_PATH, BpfMap.BPF_F_RDWR, @@ -368,7 +369,7 @@ public class BpfCoordinator { } /** Get stats BPF map. */ - @Nullable public BpfMap getBpfStatsMap() { + @Nullable public IBpfMap getBpfStatsMap() { if (!isAtLeastS()) return null; try { return new BpfMap<>(TETHER_STATS_MAP_PATH, @@ -380,7 +381,7 @@ public class BpfCoordinator { } /** Get limit BPF map. */ - @Nullable public BpfMap getBpfLimitMap() { + @Nullable public IBpfMap getBpfLimitMap() { if (!isAtLeastS()) return null; try { return new BpfMap<>(TETHER_LIMIT_MAP_PATH, @@ -392,7 +393,7 @@ public class BpfCoordinator { } /** Get dev BPF map. */ - @Nullable public BpfMap getBpfDevMap() { + @Nullable public IBpfMap getBpfDevMap() { if (!isAtLeastS()) return null; try { return new BpfMap<>(TETHER_DEV_MAP_PATH, @@ -1047,7 +1048,7 @@ public class BpfCoordinator { } } private void dumpBpfStats(@NonNull IndentingPrintWriter pw) { - try (BpfMap map = mDeps.getBpfStatsMap()) { + try (IBpfMap map = mDeps.getBpfStatsMap()) { if (map == null) { pw.println("No BPF stats map"); return; @@ -1102,7 +1103,7 @@ public class BpfCoordinator { } private void dumpIpv6UpstreamRules(IndentingPrintWriter pw) { - try (BpfMap map = mDeps.getBpfUpstream6Map()) { + try (IBpfMap map = mDeps.getBpfUpstream6Map()) { if (map == null) { pw.println("No IPv6 upstream"); return; @@ -1130,7 +1131,7 @@ public class BpfCoordinator { } private void dumpIpv6DownstreamRules(IndentingPrintWriter pw) { - try (BpfMap map = mDeps.getBpfDownstream6Map()) { + try (IBpfMap map = mDeps.getBpfDownstream6Map()) { if (map == null) { pw.println("No IPv6 downstream"); return; @@ -1161,7 +1162,7 @@ public class BpfCoordinator { pw.decreaseIndent(); } - private void dumpRawMap(BpfMap map, + private void dumpRawMap(IBpfMap map, IndentingPrintWriter pw) throws ErrnoException { if (map == null) { pw.println("No BPF support"); @@ -1192,7 +1193,7 @@ public class BpfCoordinator { // expected argument order. // TODO: dump downstream4 map. if (CollectionUtils.contains(args, DUMPSYS_RAWMAP_ARG_STATS)) { - try (BpfMap statsMap = mDeps.getBpfStatsMap()) { + try (IBpfMap statsMap = mDeps.getBpfStatsMap()) { dumpRawMap(statsMap, pw); } catch (ErrnoException | IOException e) { pw.println("Error dumping stats map: " + e); @@ -1200,7 +1201,7 @@ public class BpfCoordinator { return; } if (CollectionUtils.contains(args, DUMPSYS_RAWMAP_ARG_UPSTREAM4)) { - try (BpfMap upstreamMap = mDeps.getBpfUpstream4Map()) { + try (IBpfMap upstreamMap = mDeps.getBpfUpstream4Map()) { dumpRawMap(upstreamMap, pw); } catch (ErrnoException | IOException e) { pw.println("Error dumping IPv4 map: " + e); @@ -1245,7 +1246,7 @@ public class BpfCoordinator { } private void dumpIpv4ForwardingRuleMap(long now, boolean downstream, - BpfMap map, IndentingPrintWriter pw) throws ErrnoException { + IBpfMap map, IndentingPrintWriter pw) throws ErrnoException { if (map == null) { pw.println("No IPv4 support"); return; @@ -1260,8 +1261,8 @@ public class BpfCoordinator { private void dumpBpfForwardingRulesIpv4(IndentingPrintWriter pw) { final long now = SystemClock.elapsedRealtimeNanos(); - try (BpfMap upstreamMap = mDeps.getBpfUpstream4Map(); - BpfMap downstreamMap = mDeps.getBpfDownstream4Map()) { + try (IBpfMap upstreamMap = mDeps.getBpfUpstream4Map(); + IBpfMap downstreamMap = mDeps.getBpfDownstream4Map()) { pw.println("IPv4 Upstream: proto [inDstMac] iif(iface) src -> nat -> " + "dst [outDstMac] age"); pw.increaseIndent(); @@ -1283,7 +1284,7 @@ public class BpfCoordinator { pw.println("No counter support"); return; } - try (BpfMap map = new BpfMap<>(TETHER_ERROR_MAP_PATH, BpfMap.BPF_F_RDONLY, + try (IBpfMap map = new BpfMap<>(TETHER_ERROR_MAP_PATH, BpfMap.BPF_F_RDONLY, S32.class, S32.class)) { map.forEach((k, v) -> { @@ -1304,7 +1305,7 @@ public class BpfCoordinator { } private void dumpDevmap(@NonNull IndentingPrintWriter pw) { - try (BpfMap map = mDeps.getBpfDevMap()) { + try (IBpfMap map = mDeps.getBpfDevMap()) { if (map == null) { pw.println("No devmap support"); return; diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java index b100f58562..758b53344b 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java @@ -94,8 +94,8 @@ import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; import com.android.dx.mockito.inline.extended.ExtendedMockito; -import com.android.net.module.util.BpfMap; import com.android.net.module.util.CollectionUtils; +import com.android.net.module.util.IBpfMap; import com.android.net.module.util.InterfaceParams; import com.android.net.module.util.NetworkStackConstants; import com.android.net.module.util.SharedLog; @@ -362,9 +362,9 @@ public class BpfCoordinatorTest { @Mock private IpServer mIpServer2; @Mock private TetheringConfiguration mTetherConfig; @Mock private ConntrackMonitor mConntrackMonitor; - @Mock private BpfMap mBpfDownstream6Map; - @Mock private BpfMap mBpfUpstream6Map; - @Mock private BpfMap mBpfDevMap; + @Mock private IBpfMap mBpfDownstream6Map; + @Mock private IBpfMap mBpfUpstream6Map; + @Mock private IBpfMap mBpfDevMap; // Late init since methods must be called by the thread that created this object. private TestableNetworkStatsProviderCbBinder mTetherStatsProviderCb; @@ -379,13 +379,13 @@ public class BpfCoordinatorTest { private final ArgumentCaptor mStringArrayCaptor = ArgumentCaptor.forClass(ArrayList.class); private final TestLooper mTestLooper = new TestLooper(); - private final BpfMap mBpfDownstream4Map = + private final IBpfMap mBpfDownstream4Map = spy(new TestBpfMap<>(Tether4Key.class, Tether4Value.class)); - private final BpfMap mBpfUpstream4Map = + private final IBpfMap mBpfUpstream4Map = spy(new TestBpfMap<>(Tether4Key.class, Tether4Value.class)); - private final TestBpfMap mBpfStatsMap = + private final IBpfMap mBpfStatsMap = spy(new TestBpfMap<>(TetherStatsKey.class, TetherStatsValue.class)); - private final TestBpfMap mBpfLimitMap = + private final IBpfMap mBpfLimitMap = spy(new TestBpfMap<>(TetherLimitKey.class, TetherLimitValue.class)); private BpfCoordinator.Dependencies mDeps = spy(new BpfCoordinator.Dependencies() { @@ -424,37 +424,37 @@ public class BpfCoordinatorTest { } @Nullable - public BpfMap getBpfDownstream4Map() { + public IBpfMap getBpfDownstream4Map() { return mBpfDownstream4Map; } @Nullable - public BpfMap getBpfUpstream4Map() { + public IBpfMap getBpfUpstream4Map() { return mBpfUpstream4Map; } @Nullable - public BpfMap getBpfDownstream6Map() { + public IBpfMap getBpfDownstream6Map() { return mBpfDownstream6Map; } @Nullable - public BpfMap getBpfUpstream6Map() { + public IBpfMap getBpfUpstream6Map() { return mBpfUpstream6Map; } @Nullable - public BpfMap getBpfStatsMap() { + public IBpfMap getBpfStatsMap() { return mBpfStatsMap; } @Nullable - public BpfMap getBpfLimitMap() { + public IBpfMap getBpfLimitMap() { return mBpfLimitMap; } @Nullable - public BpfMap getBpfDevMap() { + public IBpfMap getBpfDevMap() { return mBpfDevMap; } }); diff --git a/service/src/com/android/server/BpfNetMaps.java b/service/src/com/android/server/BpfNetMaps.java index 7387483470..2dde0db857 100644 --- a/service/src/com/android/server/BpfNetMaps.java +++ b/service/src/com/android/server/BpfNetMaps.java @@ -46,6 +46,7 @@ import com.android.internal.annotations.VisibleForTesting; import com.android.modules.utils.build.SdkLevel; import com.android.net.module.util.BpfMap; import com.android.net.module.util.DeviceConfigUtils; +import com.android.net.module.util.IBpfMap; import com.android.net.module.util.Struct.U32; import com.android.net.module.util.Struct.U8; @@ -101,10 +102,10 @@ public class BpfNetMaps { private static final long STATS_SELECT_MAP_A = 0; private static final long STATS_SELECT_MAP_B = 1; - private static BpfMap sConfigurationMap = null; + private static IBpfMap sConfigurationMap = null; // BpfMap for UID_OWNER_MAP_PATH. This map is not accessed by others. - private static BpfMap sUidOwnerMap = null; - private static BpfMap sUidPermissionMap = null; + private static IBpfMap sUidOwnerMap = null; + private static IBpfMap sUidPermissionMap = null; // LINT.IfChange(match_type) @VisibleForTesting public static final long NO_MATCH = 0; @@ -134,7 +135,7 @@ public class BpfNetMaps { * Set configurationMap for test. */ @VisibleForTesting - public static void setConfigurationMapForTest(BpfMap configurationMap) { + public static void setConfigurationMapForTest(IBpfMap configurationMap) { sConfigurationMap = configurationMap; } @@ -142,7 +143,7 @@ public class BpfNetMaps { * Set uidOwnerMap for test. */ @VisibleForTesting - public static void setUidOwnerMapForTest(BpfMap uidOwnerMap) { + public static void setUidOwnerMapForTest(IBpfMap uidOwnerMap) { sUidOwnerMap = uidOwnerMap; } @@ -150,11 +151,11 @@ public class BpfNetMaps { * Set uidPermissionMap for test. */ @VisibleForTesting - public static void setUidPermissionMapForTest(BpfMap uidPermissionMap) { + public static void setUidPermissionMapForTest(IBpfMap uidPermissionMap) { sUidPermissionMap = uidPermissionMap; } - private static BpfMap getConfigurationMap() { + private static IBpfMap getConfigurationMap() { try { return new BpfMap<>( CONFIGURATION_MAP_PATH, BpfMap.BPF_F_RDWR, U32.class, U32.class); @@ -163,7 +164,7 @@ public class BpfNetMaps { } } - private static BpfMap getUidOwnerMap() { + private static IBpfMap getUidOwnerMap() { try { return new BpfMap<>( UID_OWNER_MAP_PATH, BpfMap.BPF_F_RDWR, U32.class, UidOwnerValue.class); @@ -172,7 +173,7 @@ public class BpfNetMaps { } } - private static BpfMap getUidPermissionMap() { + private static IBpfMap getUidPermissionMap() { try { return new BpfMap<>( UID_PERMISSION_MAP_PATH, BpfMap.BPF_F_RDWR, U32.class, U8.class); diff --git a/tests/unit/java/com/android/server/BpfNetMapsTest.java b/tests/unit/java/com/android/server/BpfNetMapsTest.java index be286eca2f..15a2e56fcf 100644 --- a/tests/unit/java/com/android/server/BpfNetMapsTest.java +++ b/tests/unit/java/com/android/server/BpfNetMapsTest.java @@ -58,7 +58,7 @@ import android.os.ServiceSpecificException; import androidx.test.filters.SmallTest; 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.Struct.U32; import com.android.net.module.util.Struct.U8; import com.android.testutils.DevSdkIgnoreRule; @@ -113,10 +113,10 @@ public final class BpfNetMapsTest { @Mock INetd mNetd; @Mock BpfNetMaps.Dependencies mDeps; @Mock Context mContext; - private final BpfMap mConfigurationMap = new TestBpfMap<>(U32.class, U32.class); - private final BpfMap mUidOwnerMap = + private final IBpfMap mConfigurationMap = new TestBpfMap<>(U32.class, U32.class); + private final IBpfMap mUidOwnerMap = new TestBpfMap<>(U32.class, UidOwnerValue.class); - private final BpfMap mUidPermissionMap = new TestBpfMap<>(U32.class, U8.class); + private final IBpfMap mUidPermissionMap = new TestBpfMap<>(U32.class, U8.class); @Before public void setUp() throws Exception {