diff options
7 files changed, 108 insertions, 88 deletions
diff --git a/common/device/com/android/net/module/util/BpfMap.java b/common/device/com/android/net/module/util/BpfMap.java index 9df2b035..d45caceb 100644 --- a/common/device/com/android/net/module/util/BpfMap.java +++ b/common/device/com/android/net/module/util/BpfMap.java @@ -194,9 +194,11 @@ public class BpfMap<K extends Struct, V extends Struct> implements IBpfMap<K, V> } private K getNextKeyInternal(@Nullable K key) throws ErrnoException { - final byte[] rawKey = getNextRawKey( - key == null ? null : key.writeToBytes()); - if (rawKey == null) return null; + byte[] rawKey = new byte[mKeySize]; + + if (!nativeGetNextMapKey(mMapFd.getFd(), + key == null ? null : key.writeToBytes(), + rawKey)) return null; final ByteBuffer buffer = ByteBuffer.wrap(rawKey); buffer.order(ByteOrder.nativeOrder()); @@ -215,13 +217,6 @@ public class BpfMap<K extends Struct, V extends Struct> implements IBpfMap<K, V> return getNextKeyInternal(key); } - private byte[] getNextRawKey(@Nullable final byte[] key) throws ErrnoException { - byte[] nextKey = new byte[mKeySize]; - if (nativeGetNextMapKey(mMapFd.getFd(), key, nextKey)) return nextKey; - - return null; - } - /** Get the first key of eBpf map. */ @Override public K getFirstKey() throws ErrnoException { @@ -233,30 +228,23 @@ public class BpfMap<K extends Struct, V extends Struct> implements IBpfMap<K, V> public boolean containsKey(@NonNull K key) throws ErrnoException { Objects.requireNonNull(key); - final byte[] rawValue = getRawValue(key.writeToBytes()); - return rawValue != null; + byte[] rawValue = new byte[mValueSize]; + return nativeFindMapEntry(mMapFd.getFd(), key.writeToBytes(), rawValue); } /** Retrieve a value from the map. Return null if there is no such key. */ @Override public V getValue(@NonNull K key) throws ErrnoException { Objects.requireNonNull(key); - final byte[] rawValue = getRawValue(key.writeToBytes()); - if (rawValue == null) return null; + byte[] rawValue = new byte[mValueSize]; + if (!nativeFindMapEntry(mMapFd.getFd(), key.writeToBytes(), rawValue)) return null; final ByteBuffer buffer = ByteBuffer.wrap(rawValue); buffer.order(ByteOrder.nativeOrder()); return Struct.parse(mValueClass, buffer); } - private byte[] getRawValue(final byte[] key) throws ErrnoException { - byte[] value = new byte[mValueSize]; - if (nativeFindMapEntry(mMapFd.getFd(), key, value)) return value; - - return null; - } - /** * Iterate through the map and handle each key -> value retrieved base on the given BiConsumer. * The given BiConsumer may to delete the passed-in entry, but is not allowed to perform any diff --git a/common/native/bpf_headers/include/bpf/BpfMap.h b/common/native/bpf_headers/include/bpf/BpfMap.h index 51e6d162..847083e5 100644 --- a/common/native/bpf_headers/include/bpf/BpfMap.h +++ b/common/native/bpf_headers/include/bpf/BpfMap.h @@ -29,6 +29,10 @@ namespace android { namespace bpf { +using base::Result; +using base::unique_fd; +using std::function; + // This is a class wrapper for eBPF maps. The eBPF map is a special in-kernel // data structure that stores data in <Key, Value> pairs. It can be read/write // from userspace by passing syscalls with the map file descriptor. This class @@ -80,7 +84,7 @@ class BpfMap { } #endif - base::Result<Key> getFirstKey() const { + Result<Key> getFirstKey() const { Key firstKey; if (getFirstMapKey(mMapFd, &firstKey)) { return ErrnoErrorf("Get firstKey map {} failed", mMapFd.get()); @@ -88,7 +92,7 @@ class BpfMap { return firstKey; } - base::Result<Key> getNextKey(const Key& key) const { + Result<Key> getNextKey(const Key& key) const { Key nextKey; if (getNextMapKey(mMapFd, &key, &nextKey)) { return ErrnoErrorf("Get next key of map {} failed", mMapFd.get()); @@ -96,14 +100,14 @@ class BpfMap { return nextKey; } - base::Result<void> writeValue(const Key& key, const Value& value, uint64_t flags) { + Result<void> writeValue(const Key& key, const Value& value, uint64_t flags) { if (writeToMapEntry(mMapFd, &key, &value, flags)) { return ErrnoErrorf("Write to map {} failed", mMapFd.get()); } return {}; } - base::Result<Value> readValue(const Key key) const { + Result<Value> readValue(const Key key) const { Value value; if (findMapEntry(mMapFd, &key, &value)) { return ErrnoErrorf("Read value of map {} failed", mMapFd.get()); @@ -111,7 +115,7 @@ class BpfMap { return value; } - base::Result<void> deleteValue(const Key& key) { + Result<void> deleteValue(const Key& key) { if (deleteMapEntry(mMapFd, &key)) { return ErrnoErrorf("Delete entry from map {} failed", mMapFd.get()); } @@ -119,7 +123,7 @@ class BpfMap { } protected: - [[clang::reinitializes]] base::Result<void> init(const char* path, int fd) { + [[clang::reinitializes]] Result<void> init(const char* path, int fd) { mMapFd.reset(fd); if (!mMapFd.ok()) { return ErrnoErrorf("Pinned map not accessible or does not exist: ({})", path); @@ -134,7 +138,7 @@ class BpfMap { public: // Function that tries to get map from a pinned path. - [[clang::reinitializes]] base::Result<void> init(const char* path) { + [[clang::reinitializes]] Result<void> init(const char* path) { return init(path, mapRetrieveRW(path)); } @@ -144,7 +148,7 @@ class BpfMap { // this should only ever be used by test code, it is equivalent to: // .reset(createMap(type, keysize, valuesize, max_entries, map_flags) // TODO: derive map_flags from BpfMap vs BpfMapRO - [[clang::reinitializes]] base::Result<void> resetMap(bpf_map_type map_type, + [[clang::reinitializes]] Result<void> resetMap(bpf_map_type map_type, uint32_t max_entries, uint32_t map_flags = 0) { mMapFd.reset(createMap(map_type, sizeof(Key), sizeof(Value), max_entries, map_flags)); @@ -155,30 +159,30 @@ class BpfMap { // Iterate through the map and handle each key retrieved based on the filter // without modification of map content. - base::Result<void> iterate( - const std::function<base::Result<void>(const Key& key, const BpfMap<Key, Value>& map)>& - filter) const; + Result<void> iterate( + const function<Result<void>(const Key& key, + const BpfMap<Key, Value>& map)>& filter) const; // Iterate through the map and get each <key, value> pair, handle each <key, // value> pair based on the filter without modification of map content. - base::Result<void> iterateWithValue( - const std::function<base::Result<void>(const Key& key, const Value& value, - const BpfMap<Key, Value>& map)>& filter) const; + Result<void> iterateWithValue( + const function<Result<void>(const Key& key, const Value& value, + const BpfMap<Key, Value>& map)>& filter) const; // Iterate through the map and handle each key retrieved based on the filter - base::Result<void> iterate( - const std::function<base::Result<void>(const Key& key, BpfMap<Key, Value>& map)>& - filter); + Result<void> iterate( + const function<Result<void>(const Key& key, + BpfMap<Key, Value>& map)>& filter); // Iterate through the map and get each <key, value> pair, handle each <key, // value> pair based on the filter. - base::Result<void> iterateWithValue( - const std::function<base::Result<void>(const Key& key, const Value& value, - BpfMap<Key, Value>& map)>& filter); - - const base::unique_fd& getMap() const { return mMapFd; }; + Result<void> iterateWithValue( + const function<Result<void>(const Key& key, const Value& value, + BpfMap<Key, Value>& map)>& filter); #ifdef BPF_MAP_MAKE_VISIBLE_FOR_TESTING + const unique_fd& getMap() const { return mMapFd; }; + // Copy assignment operator - due to need for fd duping, should not be used in non-test code. BpfMap<Key, Value>& operator=(const BpfMap<Key, Value>& other) { if (this != &other) mMapFd.reset(fcntl(other.mMapFd.get(), F_DUPFD_CLOEXEC, 0)); @@ -197,7 +201,7 @@ class BpfMap { return *this; } - void reset(base::unique_fd fd) = delete; + void reset(unique_fd fd) = delete; #ifdef BPF_MAP_MAKE_VISIBLE_FOR_TESTING // Note that unique_fd.reset() carefully saves and restores the errno, @@ -216,7 +220,7 @@ class BpfMap { bool isValid() const { return mMapFd.ok(); } - base::Result<void> clear() { + Result<void> clear() { while (true) { auto key = getFirstKey(); if (!key.ok()) { @@ -233,28 +237,25 @@ class BpfMap { } } - base::Result<bool> isEmpty() const { + Result<bool> isEmpty() const { auto key = getFirstKey(); - if (!key.ok()) { - // Return error code ENOENT means the map is empty - if (key.error().code() == ENOENT) return true; - return key.error(); - } - return false; + if (key.ok()) return false; + if (key.error().code() == ENOENT) return true; + return key.error(); } private: - base::unique_fd mMapFd; + unique_fd mMapFd; }; template <class Key, class Value> -base::Result<void> BpfMap<Key, Value>::iterate( - const std::function<base::Result<void>(const Key& key, const BpfMap<Key, Value>& map)>& - filter) const { - base::Result<Key> curKey = getFirstKey(); +Result<void> BpfMap<Key, Value>::iterate( + const function<Result<void>(const Key& key, + const BpfMap<Key, Value>& map)>& filter) const { + Result<Key> curKey = getFirstKey(); while (curKey.ok()) { - const base::Result<Key>& nextKey = getNextKey(curKey.value()); - base::Result<void> status = filter(curKey.value(), *this); + const Result<Key>& nextKey = getNextKey(curKey.value()); + Result<void> status = filter(curKey.value(), *this); if (!status.ok()) return status; curKey = nextKey; } @@ -263,15 +264,15 @@ base::Result<void> BpfMap<Key, Value>::iterate( } template <class Key, class Value> -base::Result<void> BpfMap<Key, Value>::iterateWithValue( - const std::function<base::Result<void>(const Key& key, const Value& value, - const BpfMap<Key, Value>& map)>& filter) const { - base::Result<Key> curKey = getFirstKey(); +Result<void> BpfMap<Key, Value>::iterateWithValue( + const function<Result<void>(const Key& key, const Value& value, + const BpfMap<Key, Value>& map)>& filter) const { + Result<Key> curKey = getFirstKey(); while (curKey.ok()) { - const base::Result<Key>& nextKey = getNextKey(curKey.value()); - base::Result<Value> curValue = readValue(curKey.value()); + const Result<Key>& nextKey = getNextKey(curKey.value()); + Result<Value> curValue = readValue(curKey.value()); if (!curValue.ok()) return curValue.error(); - base::Result<void> status = filter(curKey.value(), curValue.value(), *this); + Result<void> status = filter(curKey.value(), curValue.value(), *this); if (!status.ok()) return status; curKey = nextKey; } @@ -280,12 +281,13 @@ base::Result<void> BpfMap<Key, Value>::iterateWithValue( } template <class Key, class Value> -base::Result<void> BpfMap<Key, Value>::iterate( - const std::function<base::Result<void>(const Key& key, BpfMap<Key, Value>& map)>& filter) { - base::Result<Key> curKey = getFirstKey(); +Result<void> BpfMap<Key, Value>::iterate( + const function<Result<void>(const Key& key, + BpfMap<Key, Value>& map)>& filter) { + Result<Key> curKey = getFirstKey(); while (curKey.ok()) { - const base::Result<Key>& nextKey = getNextKey(curKey.value()); - base::Result<void> status = filter(curKey.value(), *this); + const Result<Key>& nextKey = getNextKey(curKey.value()); + Result<void> status = filter(curKey.value(), *this); if (!status.ok()) return status; curKey = nextKey; } @@ -294,15 +296,15 @@ base::Result<void> BpfMap<Key, Value>::iterate( } template <class Key, class Value> -base::Result<void> BpfMap<Key, Value>::iterateWithValue( - const std::function<base::Result<void>(const Key& key, const Value& value, - BpfMap<Key, Value>& map)>& filter) { - base::Result<Key> curKey = getFirstKey(); +Result<void> BpfMap<Key, Value>::iterateWithValue( + const function<Result<void>(const Key& key, const Value& value, + BpfMap<Key, Value>& map)>& filter) { + Result<Key> curKey = getFirstKey(); while (curKey.ok()) { - const base::Result<Key>& nextKey = getNextKey(curKey.value()); - base::Result<Value> curValue = readValue(curKey.value()); + const Result<Key>& nextKey = getNextKey(curKey.value()); + Result<Value> curValue = readValue(curKey.value()); if (!curValue.ok()) return curValue.error(); - base::Result<void> status = filter(curKey.value(), curValue.value(), *this); + Result<void> status = filter(curKey.value(), curValue.value(), *this); if (!status.ok()) return status; curKey = nextKey; } @@ -319,7 +321,7 @@ class BpfMapRO : public BpfMap<Key, Value> { : BpfMap<Key, Value>(pathname, BPF_F_RDONLY) {} // Function that tries to get map from a pinned path. - [[clang::reinitializes]] base::Result<void> init(const char* path) { + [[clang::reinitializes]] Result<void> init(const char* path) { return BpfMap<Key,Value>::init(path, mapRetrieveRO(path)); } }; diff --git a/common/tests/unit/src/com/android/net/module/util/CollectionUtilsTest.kt b/common/tests/unit/src/com/android/net/module/util/CollectionUtilsTest.kt index 9fb025b3..e23f9991 100644 --- a/common/tests/unit/src/com/android/net/module/util/CollectionUtilsTest.kt +++ b/common/tests/unit/src/com/android/net/module/util/CollectionUtilsTest.kt @@ -63,9 +63,6 @@ class CollectionUtilsTest { assertEquals(0, CollectionUtils.indexOfSubArray(haystack, byteArrayOf())) assertEquals(-1, CollectionUtils.indexOfSubArray(byteArrayOf(), byteArrayOf(3, 2))) assertEquals(0, CollectionUtils.indexOfSubArray(byteArrayOf(), byteArrayOf())) - assertThrows(NullPointerException::class.java) { - CollectionUtils.indexOfSubArray(haystack, null) - } } @Test diff --git a/common/testutils/devicetests/com/android/testutils/FakeDns.kt b/common/testutils/devicetests/com/android/testutils/FakeDns.kt index 825d7487..1f82a35d 100644 --- a/common/testutils/devicetests/com/android/testutils/FakeDns.kt +++ b/common/testutils/devicetests/com/android/testutils/FakeDns.kt @@ -81,9 +81,9 @@ class FakeDns(val mockResolver: DnsResolver) { var type = if (posType != -1) it.arguments[posType] as Int else TYPE_UNSPECIFIED val answer = getAnswer(hostname, type) - if (!answer?.addresses.isNullOrEmpty()) { + if (answer != null && !answer.addresses.isNullOrEmpty()) { Handler(Looper.getMainLooper()).post({ executor.execute({ - callback.onAnswer(answer?.addresses, 0); }) }) + callback.onAnswer(answer.addresses, 0); }) }) } } diff --git a/common/testutils/devicetests/com/android/testutils/NonNullTestUtils.java b/common/testutils/devicetests/com/android/testutils/NonNullTestUtils.java new file mode 100644 index 00000000..463c4706 --- /dev/null +++ b/common/testutils/devicetests/com/android/testutils/NonNullTestUtils.java @@ -0,0 +1,33 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.testutils; + +import android.annotation.NonNull; + +/** + * Utilities to help Kotlin test to verify java @NonNull code + */ +public class NonNullTestUtils { + /** + * This method allows Kotlin to pass nullable to @NonNull java code for testing. + * For Foo(@NonNull arg) java method, Kotlin can pass nullable variable by + * Foo(NonNullTestUtils.nullUnsafe(nullableVar)). + */ + @NonNull public static <T> T nullUnsafe(T v) { + return v; + } +} diff --git a/common/testutils/devicetests/com/android/testutils/PacketBridge.kt b/common/testutils/devicetests/com/android/testutils/PacketBridge.kt index da3508de..d50f78a1 100644 --- a/common/testutils/devicetests/com/android/testutils/PacketBridge.kt +++ b/common/testutils/devicetests/com/android/testutils/PacketBridge.kt @@ -48,8 +48,8 @@ class PacketBridge( private val natMap = NatMap() private val binder = Binder() - private val cm = context.getSystemService(ConnectivityManager::class.java) - private val tnm = context.getSystemService(TestNetworkManager::class.java) + private val cm = context.getSystemService(ConnectivityManager::class.java)!! + private val tnm = context.getSystemService(TestNetworkManager::class.java)!! // Create test networks. private val internalIface = tnm.createTunInterface(listOf(internalAddr)) diff --git a/common/testutils/devicetests/com/android/testutils/TestNetworkTracker.kt b/common/testutils/devicetests/com/android/testutils/TestNetworkTracker.kt index b743b6c9..84fb47bc 100644 --- a/common/testutils/devicetests/com/android/testutils/TestNetworkTracker.kt +++ b/common/testutils/devicetests/com/android/testutils/TestNetworkTracker.kt @@ -91,7 +91,7 @@ private fun initTestNetwork( lp: LinkProperties?, setupTimeoutMs: Long = 10_000L ): TestNetworkTracker { - val tnm = context.getSystemService(TestNetworkManager::class.java) + val tnm = context.getSystemService(TestNetworkManager::class.java)!! val iface = if (isAtLeastS()) tnm.createTunInterface(linkAddrs) else tnm.createTunInterface(linkAddrs.toTypedArray()) val lpWithIface = if (lp == null) null else LinkProperties(lp).apply { @@ -112,7 +112,7 @@ class TestNetworkTracker internal constructor( val lp: LinkProperties?, setupTimeoutMs: Long ) : TestableNetworkCallback.HasNetwork { - private val cm = context.getSystemService(ConnectivityManager::class.java) + private val cm = context.getSystemService(ConnectivityManager::class.java)!! private val binder = Binder() private val networkCallback: NetworkCallback |