SingleWriterBpfMap - introduce getSingleton with caching
Test: TreeHugger, atest BpfMapTest finally happy!
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Ia4ddcbb4994bb0fd5df7d36e0d306ca5061d8963
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 a0928c5..47aebe8 100644
--- a/Tethering/tests/privileged/src/com/android/networkstack/tethering/BpfMapTest.java
+++ b/Tethering/tests/privileged/src/com/android/networkstack/tethering/BpfMapTest.java
@@ -112,8 +112,8 @@
private BpfMap<TetherDownstream6Key, Tether6Value> openTestMap() throws Exception {
return mShouldTestSingleWriterMap
- ? new SingleWriterBpfMap<>(TETHER2_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);
}
@@ -510,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;
@@ -524,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/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/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);