Merge changes Ic6a1aa92,I685f924a,I50fab91e
* changes:
Use packBitList to prevent long-related mistakes
Introduce ensureListenableCapabilities
Accept accessUids from telephony when it's the carrier config app
diff --git a/service/jni/com_android_server_BpfNetMaps.cpp b/service/jni/com_android_server_BpfNetMaps.cpp
index bde52a5..2aaa4c3 100644
--- a/service/jni/com_android_server_BpfNetMaps.cpp
+++ b/service/jni/com_android_server_BpfNetMaps.cpp
@@ -39,10 +39,9 @@
namespace android {
static void native_init(JNIEnv* env, jobject clazz) {
- // start is still being called by netd
- Status status = mTc.initMaps();
+ Status status = mTc.start();
if (!isOk(status)) {
- ALOGE("%s failed", __func__);
+ ALOGE("%s failed, error code = %d", __func__, status.code());
}
}
@@ -51,7 +50,7 @@
Status status = mTc.updateUidOwnerMap(appUids, PENALTY_BOX_MATCH,
TrafficController::IptOp::IptOpInsert);
if (!isOk(status)) {
- ALOGE("%s failed, errer code = %d", __func__, status.code());
+ ALOGE("%s failed, error code = %d", __func__, status.code());
}
return (jint)status.code();
}
@@ -61,7 +60,7 @@
Status status = mTc.updateUidOwnerMap(appUids, PENALTY_BOX_MATCH,
TrafficController::IptOp::IptOpDelete);
if (!isOk(status)) {
- ALOGE("%s failed, errer code = %d", __func__, status.code());
+ ALOGE("%s failed, error code = %d", __func__, status.code());
}
return (jint)status.code();
}
@@ -71,7 +70,7 @@
Status status = mTc.updateUidOwnerMap(appUids, HAPPY_BOX_MATCH,
TrafficController::IptOp::IptOpInsert);
if (!isOk(status)) {
- ALOGE("%s failed, errer code = %d", __func__, status.code());
+ ALOGE("%s failed, error code = %d", __func__, status.code());
}
return (jint)status.code();
}
@@ -81,7 +80,7 @@
Status status = mTc.updateUidOwnerMap(appUids, HAPPY_BOX_MATCH,
TrafficController::IptOp::IptOpDelete);
if (!isOk(status)) {
- ALOGD("%s failed, errer code = %d", __func__, status.code());
+ ALOGD("%s failed, error code = %d", __func__, status.code());
}
return (jint)status.code();
}
diff --git a/service/native/Android.bp b/service/native/Android.bp
index 0e805e2..a20c54f 100644
--- a/service/native/Android.bp
+++ b/service/native/Android.bp
@@ -70,6 +70,7 @@
"libnetdutils",
"libtraffic_controller",
"libutils",
+ "libnetd_updatable",
"netd_aidl_interface-lateststable-ndk",
],
}
diff --git a/service/native/TrafficControllerTest.cpp b/service/native/TrafficControllerTest.cpp
index f401636..39f3365 100644
--- a/service/native/TrafficControllerTest.cpp
+++ b/service/native/TrafficControllerTest.cpp
@@ -38,6 +38,7 @@
#include "TrafficController.h"
#include "bpf/BpfUtils.h"
+#include "NetdUpdatablePublic.h"
using namespace android::bpf; // NOLINT(google-build-using-namespace): grandfathered
@@ -746,5 +747,133 @@
expectPrivilegedUserSetEmpty();
}
+constexpr uint32_t SOCK_CLOSE_WAIT_US = 30 * 1000;
+constexpr uint32_t ENOBUFS_POLL_WAIT_US = 10 * 1000;
+
+using android::base::Error;
+using android::base::Result;
+using android::bpf::BpfMap;
+
+// This test set up a SkDestroyListener that is running parallel with the production
+// SkDestroyListener. The test will create thousands of sockets and tag them on the
+// production cookieUidTagMap and close them in a short time. When the number of
+// sockets get closed exceeds the buffer size, it will start to return ENOBUFF
+// error. The error will be ignored by the production SkDestroyListener and the
+// test will clean up the tags in tearDown if there is any remains.
+
+// TODO: Instead of test the ENOBUFF error, we can test the production
+// SkDestroyListener to see if it failed to delete a tagged socket when ENOBUFF
+// triggered.
+class NetlinkListenerTest : public testing::Test {
+ protected:
+ NetlinkListenerTest() {}
+ BpfMap<uint64_t, UidTagValue> mCookieTagMap;
+
+ void SetUp() {
+ mCookieTagMap.reset(android::bpf::mapRetrieveRW(COOKIE_TAG_MAP_PATH));
+ ASSERT_TRUE(mCookieTagMap.isValid());
+ }
+
+ void TearDown() {
+ const auto deleteTestCookieEntries = [](const uint64_t& key, const UidTagValue& value,
+ BpfMap<uint64_t, UidTagValue>& map) {
+ if ((value.uid == TEST_UID) && (value.tag == TEST_TAG)) {
+ Result<void> res = map.deleteValue(key);
+ if (res.ok() || (res.error().code() == ENOENT)) {
+ return Result<void>();
+ }
+ ALOGE("Failed to delete data(cookie = %" PRIu64 "): %s\n", key,
+ strerror(res.error().code()));
+ }
+ // Move forward to next cookie in the map.
+ return Result<void>();
+ };
+ EXPECT_RESULT_OK(mCookieTagMap.iterateWithValue(deleteTestCookieEntries));
+ }
+
+ Result<void> checkNoGarbageTagsExist() {
+ const auto checkGarbageTags = [](const uint64_t&, const UidTagValue& value,
+ const BpfMap<uint64_t, UidTagValue>&) -> Result<void> {
+ if ((TEST_UID == value.uid) && (TEST_TAG == value.tag)) {
+ return Error(EUCLEAN) << "Closed socket is not untagged";
+ }
+ return {};
+ };
+ return mCookieTagMap.iterateWithValue(checkGarbageTags);
+ }
+
+ bool checkMassiveSocketDestroy(int totalNumber, bool expectError) {
+ std::unique_ptr<android::netdutils::NetlinkListenerInterface> skDestroyListener;
+ auto result = android::net::TrafficController::makeSkDestroyListener();
+ if (!isOk(result)) {
+ ALOGE("Unable to create SkDestroyListener: %s", toString(result).c_str());
+ } else {
+ skDestroyListener = std::move(result.value());
+ }
+ int rxErrorCount = 0;
+ // Rx handler extracts nfgenmsg looks up and invokes registered dispatch function.
+ const auto rxErrorHandler = [&rxErrorCount](const int, const int) { rxErrorCount++; };
+ skDestroyListener->registerSkErrorHandler(rxErrorHandler);
+ int fds[totalNumber];
+ for (int i = 0; i < totalNumber; i++) {
+ fds[i] = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
+ // The likely reason for a failure is running out of available file descriptors.
+ EXPECT_LE(0, fds[i]) << i << " of " << totalNumber;
+ if (fds[i] < 0) {
+ // EXPECT_LE already failed above, so test case is a failure, but we don't
+ // want potentially tens of thousands of extra failures creating and then
+ // closing all these fds cluttering up the logs.
+ totalNumber = i;
+ break;
+ };
+ libnetd_updatable_tagSocket(fds[i], TEST_TAG, TEST_UID, 1000);
+ }
+
+ // TODO: Use a separate thread that has its own fd table so we can
+ // close sockets even faster simply by terminating that thread.
+ for (int i = 0; i < totalNumber; i++) {
+ EXPECT_EQ(0, close(fds[i]));
+ }
+ // wait a bit for netlink listener to handle all the messages.
+ usleep(SOCK_CLOSE_WAIT_US);
+ if (expectError) {
+ // If ENOBUFS triggered, check it only called into the handler once, ie.
+ // that the netlink handler is not spinning.
+ int currentErrorCount = rxErrorCount;
+ // 0 error count is acceptable because the system has chances to close all sockets
+ // normally.
+ EXPECT_LE(0, rxErrorCount);
+ if (!rxErrorCount) return true;
+
+ usleep(ENOBUFS_POLL_WAIT_US);
+ EXPECT_EQ(currentErrorCount, rxErrorCount);
+ } else {
+ EXPECT_RESULT_OK(checkNoGarbageTagsExist());
+ EXPECT_EQ(0, rxErrorCount);
+ }
+ return false;
+ }
+};
+
+TEST_F(NetlinkListenerTest, TestAllSocketUntagged) {
+ checkMassiveSocketDestroy(10, false);
+ checkMassiveSocketDestroy(100, false);
+}
+
+// Disabled because flaky on blueline-userdebug; this test relies on the main thread
+// winning a race against the NetlinkListener::run() thread. There's no way to ensure
+// things will be scheduled the same way across all architectures and test environments.
+TEST_F(NetlinkListenerTest, DISABLED_TestSkDestroyError) {
+ bool needRetry = false;
+ int retryCount = 0;
+ do {
+ needRetry = checkMassiveSocketDestroy(32500, true);
+ if (needRetry) retryCount++;
+ } while (needRetry && retryCount < 3);
+ // Should review test if it can always close all sockets correctly.
+ EXPECT_GT(3, retryCount);
+}
+
+
} // namespace net
} // namespace android
diff --git a/service/native/include/TrafficController.h b/service/native/include/TrafficController.h
index 2d89344..ddcf445 100644
--- a/service/native/include/TrafficController.h
+++ b/service/native/include/TrafficController.h
@@ -32,22 +32,14 @@
using netdutils::StatusOr;
class TrafficController {
- // TODO: marking this private for right now, as start is already called by
- // netd. start() calls initMaps(), initPrograms(), and sets up the socket
- // destroy listener. Both initPrograms() and setting up the socket destroy
- // listener should only be done once.
+ public:
+ static constexpr char DUMP_KEYWORD[] = "trafficcontroller";
+
/*
* Initialize the whole controller
*/
netdutils::Status start();
- public:
- static constexpr char DUMP_KEYWORD[] = "trafficcontroller";
-
- // TODO: marking this public for right now, as start() is already called by
- // netd.
- netdutils::Status initMaps() EXCLUDES(mMutex);
-
int setCounterSet(int counterSetNum, uid_t uid, uid_t callingUid) EXCLUDES(mMutex);
/*
@@ -195,6 +187,8 @@
std::mutex mMutex;
+ netdutils::Status initMaps() EXCLUDES(mMutex);
+
// Keep track of uids that have permission UPDATE_DEVICE_STATS so we don't
// need to call back to system server for permission check.
std::set<uid_t> mPrivilegedUser GUARDED_BY(mMutex);
diff --git a/service/src/com/android/server/BpfNetMaps.java b/service/src/com/android/server/BpfNetMaps.java
index 6c6a19d..7a3bab3 100644
--- a/service/src/com/android/server/BpfNetMaps.java
+++ b/service/src/com/android/server/BpfNetMaps.java
@@ -33,8 +33,7 @@
private static final String TAG = "BpfNetMaps";
private final INetd mNetd;
// Use legacy netd for releases before T.
- // TODO: change to !SdkLevel.isAtLeastT()
- private static final boolean USE_NETD = true;
+ private static final boolean USE_NETD = !SdkLevel.isAtLeastT();
private static boolean sInitialized = false;
/**
diff --git a/tests/integration/Android.bp b/tests/integration/Android.bp
index eb7c2d1..530fa91 100644
--- a/tests/integration/Android.bp
+++ b/tests/integration/Android.bp
@@ -54,6 +54,7 @@
"libnativehelper_compat_libc++",
"libnetworkstackutilsjni",
"libandroid_net_connectivity_com_android_net_module_util_jni",
+ "libservice-connectivity",
],
jarjar_rules: ":connectivity-jarjar-rules",
}