Merge "Add a Log.wtf() on user builds to test Pitot reporting pipeline" into main
diff --git a/Tethering/tests/privileged/src/com/android/networkstack/tethering/BpfMapTest.java b/Tethering/tests/privileged/src/com/android/networkstack/tethering/BpfMapTest.java
index f8aa69f..47aebe8 100644
--- a/Tethering/tests/privileged/src/com/android/networkstack/tethering/BpfMapTest.java
+++ b/Tethering/tests/privileged/src/com/android/networkstack/tethering/BpfMapTest.java
@@ -69,6 +69,10 @@
private static final int TEST_MAP_SIZE = 16;
private static final String TETHER_DOWNSTREAM6_FS_PATH =
"/sys/fs/bpf/tethering/map_test_tether_downstream6_map";
+ private static final String TETHER2_DOWNSTREAM6_FS_PATH =
+ "/sys/fs/bpf/tethering/map_test_tether2_downstream6_map";
+ private static final String TETHER3_DOWNSTREAM6_FS_PATH =
+ "/sys/fs/bpf/tethering/map_test_tether3_downstream6_map";
private ArrayMap<TetherDownstream6Key, Tether6Value> mTestData;
@@ -108,8 +112,8 @@
private BpfMap<TetherDownstream6Key, Tether6Value> openTestMap() throws Exception {
return mShouldTestSingleWriterMap
- ? new SingleWriterBpfMap<>(TETHER_DOWNSTREAM6_FS_PATH, TetherDownstream6Key.class,
- Tether6Value.class)
+ ? SingleWriterBpfMap.getSingleton(TETHER2_DOWNSTREAM6_FS_PATH,
+ TetherDownstream6Key.class, Tether6Value.class)
: new BpfMap<>(TETHER_DOWNSTREAM6_FS_PATH, TetherDownstream6Key.class,
Tether6Value.class);
}
@@ -154,7 +158,7 @@
assertEquals(OsConstants.EPERM, expected.errno);
}
}
- try (BpfMap writeOnlyMap = new BpfMap<>(TETHER_DOWNSTREAM6_FS_PATH, BpfMap.BPF_F_WRONLY,
+ try (BpfMap writeOnlyMap = new BpfMap<>(TETHER3_DOWNSTREAM6_FS_PATH, BpfMap.BPF_F_WRONLY,
TetherDownstream6Key.class, Tether6Value.class)) {
assertNotNull(writeOnlyMap);
try {
@@ -506,12 +510,6 @@
@Test
public void testSingleWriterCacheEffectiveness() throws Exception {
assumeTrue(mShouldTestSingleWriterMap);
-
- // Ensure the map is not empty.
- for (int i = 0; i < mTestData.size(); i++) {
- mTestMap.insertEntry(mTestData.keyAt(i), mTestData.valueAt(i));
- }
-
// Benchmark parameters.
final int timeoutMs = 5_000; // Only hit if threads don't complete.
final int benchmarkTimeMs = 2_000;
@@ -520,11 +518,17 @@
// Only require 3x to reduce test flakiness.
final int expectedSpeedup = 3;
- final BpfMap cachedMap = new SingleWriterBpfMap(TETHER_DOWNSTREAM6_FS_PATH,
+ final BpfMap cachedMap = SingleWriterBpfMap.getSingleton(TETHER2_DOWNSTREAM6_FS_PATH,
TetherDownstream6Key.class, Tether6Value.class);
final BpfMap uncachedMap = new BpfMap(TETHER_DOWNSTREAM6_FS_PATH,
TetherDownstream6Key.class, Tether6Value.class);
+ // Ensure the maps are not empty.
+ for (int i = 0; i < mTestData.size(); i++) {
+ cachedMap.insertEntry(mTestData.keyAt(i), mTestData.valueAt(i));
+ uncachedMap.insertEntry(mTestData.keyAt(i), mTestData.valueAt(i));
+ }
+
final CompletableFuture<Integer> cachedResult = new CompletableFuture<>();
final CompletableFuture<Integer> uncachedResult = new CompletableFuture<>();
diff --git a/bpf_progs/test.c b/bpf_progs/test.c
index fff3512..6a4471c 100644
--- a/bpf_progs/test.c
+++ b/bpf_progs/test.c
@@ -45,6 +45,10 @@
// Used only by TetheringPrivilegedTests, not by production code.
DEFINE_BPF_MAP_GRW(tether_downstream6_map, HASH, TetherDownstream6Key, Tether6Value, 16,
TETHERING_GID)
+DEFINE_BPF_MAP_GRW(tether2_downstream6_map, HASH, TetherDownstream6Key, Tether6Value, 16,
+ TETHERING_GID)
+DEFINE_BPF_MAP_GRW(tether3_downstream6_map, HASH, TetherDownstream6Key, Tether6Value, 16,
+ TETHERING_GID)
// Used only by BpfBitmapTest, not by production code.
DEFINE_BPF_MAP_GRW(bitmap, ARRAY, int, uint64_t, 2, TETHERING_GID)
diff --git a/service/src/com/android/server/BpfNetMaps.java b/service/src/com/android/server/BpfNetMaps.java
index a30735a..b3e7d8c 100644
--- a/service/src/com/android/server/BpfNetMaps.java
+++ b/service/src/com/android/server/BpfNetMaps.java
@@ -189,7 +189,7 @@
@RequiresApi(Build.VERSION_CODES.TIRAMISU)
private static IBpfMap<S32, U32> getConfigurationMap() {
try {
- return new SingleWriterBpfMap<>(
+ return SingleWriterBpfMap.getSingleton(
CONFIGURATION_MAP_PATH, S32.class, U32.class);
} catch (ErrnoException e) {
throw new IllegalStateException("Cannot open netd configuration map", e);
@@ -199,7 +199,7 @@
@RequiresApi(Build.VERSION_CODES.TIRAMISU)
private static IBpfMap<S32, UidOwnerValue> getUidOwnerMap() {
try {
- return new SingleWriterBpfMap<>(
+ return SingleWriterBpfMap.getSingleton(
UID_OWNER_MAP_PATH, S32.class, UidOwnerValue.class);
} catch (ErrnoException e) {
throw new IllegalStateException("Cannot open uid owner map", e);
@@ -209,7 +209,7 @@
@RequiresApi(Build.VERSION_CODES.TIRAMISU)
private static IBpfMap<S32, U8> getUidPermissionMap() {
try {
- return new SingleWriterBpfMap<>(
+ return SingleWriterBpfMap.getSingleton(
UID_PERMISSION_MAP_PATH, S32.class, U8.class);
} catch (ErrnoException e) {
throw new IllegalStateException("Cannot open uid permission map", e);
@@ -230,7 +230,7 @@
@RequiresApi(Build.VERSION_CODES.TIRAMISU)
private static IBpfMap<S32, U8> getDataSaverEnabledMap() {
try {
- return new SingleWriterBpfMap<>(
+ return SingleWriterBpfMap.getSingleton(
DATA_SAVER_ENABLED_MAP_PATH, S32.class, U8.class);
} catch (ErrnoException e) {
throw new IllegalStateException("Cannot open data saver enabled map", e);
@@ -240,7 +240,7 @@
@RequiresApi(Build.VERSION_CODES.TIRAMISU)
private static IBpfMap<IngressDiscardKey, IngressDiscardValue> getIngressDiscardMap() {
try {
- return new SingleWriterBpfMap<>(INGRESS_DISCARD_MAP_PATH,
+ return SingleWriterBpfMap.getSingleton(INGRESS_DISCARD_MAP_PATH,
IngressDiscardKey.class, IngressDiscardValue.class);
} catch (ErrnoException e) {
throw new IllegalStateException("Cannot open ingress discard map", e);
diff --git a/service/src/com/android/server/connectivity/ClatCoordinator.java b/service/src/com/android/server/connectivity/ClatCoordinator.java
index f333dae..b1c770b 100644
--- a/service/src/com/android/server/connectivity/ClatCoordinator.java
+++ b/service/src/com/android/server/connectivity/ClatCoordinator.java
@@ -45,6 +45,7 @@
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.SingleWriterBpfMap;
import com.android.net.module.util.TcUtils;
import com.android.net.module.util.bpf.ClatEgress4Key;
import com.android.net.module.util.bpf.ClatEgress4Value;
@@ -256,7 +257,7 @@
@Nullable
public IBpfMap<ClatIngress6Key, ClatIngress6Value> getBpfIngress6Map() {
try {
- return new BpfMap<>(CLAT_INGRESS6_MAP_PATH,
+ return SingleWriterBpfMap.getSingleton(CLAT_INGRESS6_MAP_PATH,
ClatIngress6Key.class, ClatIngress6Value.class);
} catch (ErrnoException e) {
Log.e(TAG, "Cannot create ingress6 map: " + e);
@@ -268,7 +269,7 @@
@Nullable
public IBpfMap<ClatEgress4Key, ClatEgress4Value> getBpfEgress4Map() {
try {
- return new BpfMap<>(CLAT_EGRESS4_MAP_PATH,
+ return SingleWriterBpfMap.getSingleton(CLAT_EGRESS4_MAP_PATH,
ClatEgress4Key.class, ClatEgress4Value.class);
} catch (ErrnoException e) {
Log.e(TAG, "Cannot create egress4 map: " + e);
@@ -280,6 +281,7 @@
@Nullable
public IBpfMap<CookieTagMapKey, CookieTagMapValue> getBpfCookieTagMap() {
try {
+ // also read and written from other locations
return new BpfMap<>(COOKIE_TAG_MAP_PATH,
CookieTagMapKey.class, CookieTagMapValue.class);
} catch (ErrnoException e) {
diff --git a/staticlibs/device/com/android/net/module/util/BpfMap.java b/staticlibs/device/com/android/net/module/util/BpfMap.java
index dafff20..0fbc25d 100644
--- a/staticlibs/device/com/android/net/module/util/BpfMap.java
+++ b/staticlibs/device/com/android/net/module/util/BpfMap.java
@@ -15,6 +15,7 @@
*/
package com.android.net.module.util;
+import static android.system.OsConstants.EBUSY;
import static android.system.OsConstants.EEXIST;
import static android.system.OsConstants.ENOENT;
@@ -72,6 +73,12 @@
private static ConcurrentHashMap<Pair<String, Integer>, ParcelFileDescriptor> sFdCache =
new ConcurrentHashMap<>();
+ private static ParcelFileDescriptor checkModeExclusivity(ParcelFileDescriptor fd, int mode)
+ throws ErrnoException {
+ if (mode == BPF_F_RDWR_EXCLUSIVE) throw new ErrnoException("cachedBpfFdGet", EBUSY);
+ return fd;
+ }
+
private static ParcelFileDescriptor cachedBpfFdGet(String path, int mode,
int keySize, int valueSize)
throws ErrnoException, NullPointerException {
@@ -82,12 +89,12 @@
var key = Pair.create(path, (mode << 26) ^ (keySize << 16) ^ valueSize);
// unlocked fetch is safe: map is concurrent read capable, and only inserted into
ParcelFileDescriptor fd = sFdCache.get(key);
- if (fd != null) return fd;
+ if (fd != null) return checkModeExclusivity(fd, mode);
// ok, no cached fd present, need to grab a lock
synchronized (BpfMap.class) {
// need to redo the check
fd = sFdCache.get(key);
- if (fd != null) return fd;
+ if (fd != null) return checkModeExclusivity(fd, mode);
// okay, we really haven't opened this before...
fd = ParcelFileDescriptor.adoptFd(nativeBpfFdGet(path, mode, keySize, valueSize));
sFdCache.put(key, fd);
diff --git a/staticlibs/device/com/android/net/module/util/SingleWriterBpfMap.java b/staticlibs/device/com/android/net/module/util/SingleWriterBpfMap.java
index bbd921d..cd6bfec 100644
--- a/staticlibs/device/com/android/net/module/util/SingleWriterBpfMap.java
+++ b/staticlibs/device/com/android/net/module/util/SingleWriterBpfMap.java
@@ -17,6 +17,7 @@
import android.os.Build;
import android.system.ErrnoException;
+import android.util.Pair;
import androidx.annotation.NonNull;
import androidx.annotation.RequiresApi;
@@ -61,7 +62,12 @@
// our code can contain hundreds of items.
private final HashMap<K, V> mCache = new HashMap<>();
- public SingleWriterBpfMap(@NonNull final String path, final Class<K> key,
+ // This should only ever be called (hence private) once for a given 'path'.
+ // Java-wise what matters is the entire {path, key, value} triplet,
+ // but of course the kernel exclusive lock is just on the path (fd),
+ // and any BpfMap has (or should have...) well defined key/value types
+ // (or at least their sizes) so in practice it doesn't really matter.
+ private SingleWriterBpfMap(@NonNull final String path, final Class<K> key,
final Class<V> value) throws ErrnoException, NullPointerException {
super(path, BPF_F_RDWR_EXCLUSIVE, key, value);
@@ -73,6 +79,24 @@
}
}
+ // This allows reuse of SingleWriterBpfMap objects for the same {path, keyClass, valueClass}.
+ // These are never destroyed, so once created the lock is (effectively) held till process death
+ // (even if fixed, there would still be a write-only fd cache in underlying BpfMap base class).
+ private static final HashMap<Pair<String, Pair<Class, Class>>, SingleWriterBpfMap>
+ singletonCache = new HashMap<>();
+
+ // This is the public 'factory method' that (creates if needed and) returns a singleton instance
+ // for a given map. This holds an exclusive lock and has a permanent write-through cache.
+ // It will not be released until process death (or at least unload of the relevant class loader)
+ public synchronized static <KK extends Struct, VV extends Struct> SingleWriterBpfMap<KK,VV>
+ getSingleton(@NonNull final String path, final Class<KK> key, final Class<VV> value)
+ throws ErrnoException, NullPointerException {
+ var cacheKey = new Pair<>(path, new Pair<Class,Class>(key, value));
+ if (!singletonCache.containsKey(cacheKey))
+ singletonCache.put(cacheKey, new SingleWriterBpfMap(path, key, value));
+ return singletonCache.get(cacheKey);
+ }
+
@Override
public synchronized void updateEntry(K key, V value) throws ErrnoException {
super.updateEntry(key, value);
diff --git a/tests/mts/bpf_existence_test.cpp b/tests/mts/bpf_existence_test.cpp
index dca9dd7..e212e7d 100644
--- a/tests/mts/bpf_existence_test.cpp
+++ b/tests/mts/bpf_existence_test.cpp
@@ -68,6 +68,8 @@
TETHERING "map_offload_tether_upstream6_map",
TETHERING "map_test_bitmap",
TETHERING "map_test_tether_downstream6_map",
+ TETHERING "map_test_tether2_downstream6_map",
+ TETHERING "map_test_tether3_downstream6_map",
TETHERING "prog_offload_schedcls_tether_downstream4_ether",
TETHERING "prog_offload_schedcls_tether_downstream4_rawip",
TETHERING "prog_offload_schedcls_tether_downstream6_ether",