ClatdController - add mutex annotations
This effectively makes ClatdController single threaded.
Which makes things nice and simple.
Test: atest libbpf_android_test libnetdbpf_test netd_integration_test netd_unit_test netdutils_test resolv_integration_test resolv_unit_test
Bug: 65674744
Change-Id: I352761b6c44c17f9ea0897ea821a826f642659d5
diff --git a/server/ClatdController.cpp b/server/ClatdController.cpp
index 3ef2fe1..7c16404 100644
--- a/server/ClatdController.cpp
+++ b/server/ClatdController.cpp
@@ -71,7 +71,9 @@
ClatdController::~ClatdController() {
}
-void ClatdController::Init(void) {
+void ClatdController::init(void) {
+ std::lock_guard guard(mutex);
+
// TODO: should refactor into separate function for testability
if (bpf::getBpfSupportLevel() == bpf::BpfLevel::NONE) {
ALOGI("Pre-4.9 kernel or pre-P api shipping level - disabling clat ebpf.");
@@ -403,6 +405,7 @@
int ClatdController::startClatd(const std::string& interface, const std::string& nat64Prefix,
std::string* v6Str) {
+ std::lock_guard guard(mutex);
ClatdTracker* existing = getClatdTracker(interface);
if (existing != nullptr) {
ALOGE("clatd pid=%d already started on %s", existing->pid, interface.c_str());
@@ -447,6 +450,7 @@
}
int ClatdController::stopClatd(const std::string& interface) {
+ std::lock_guard guard(mutex);
ClatdTracker* tracker = getClatdTracker(interface);
if (tracker == nullptr) {
diff --git a/server/ClatdController.h b/server/ClatdController.h
index ceebde6..6bfc0f1 100644
--- a/server/ClatdController.h
+++ b/server/ClatdController.h
@@ -39,19 +39,18 @@
class ClatdController {
public:
- explicit ClatdController(NetworkController* controller);
- virtual ~ClatdController();
+ explicit ClatdController(NetworkController* controller) EXCLUDES(mutex);
+ virtual ~ClatdController() EXCLUDES(mutex);
- void Init(void);
+ /* First thing init/startClatd/stopClatd/dump do is grab the mutex. */
+ void init(void) EXCLUDES(mutex);
int startClatd(const std::string& interface, const std::string& nat64Prefix,
- std::string* v6Addr);
- int stopClatd(const std::string& interface);
+ std::string* v6Addr) EXCLUDES(mutex);
+ int stopClatd(const std::string& interface) EXCLUDES(mutex);
void dump(netdutils::DumpWriter& dw) EXCLUDES(mutex);
- std::mutex mutex;
-
private:
struct ClatdTracker {
const NetworkController* netCtrl = nullptr;
@@ -75,9 +74,11 @@
int init(const std::string& interface, const std::string& nat64Prefix);
};
- const NetworkController* mNetCtrl;
- std::map<std::string, ClatdTracker> mClatdTrackers;
- ClatdTracker* getClatdTracker(const std::string& interface);
+ std::mutex mutex;
+
+ const NetworkController* mNetCtrl GUARDED_BY(mutex);
+ std::map<std::string, ClatdTracker> mClatdTrackers GUARDED_BY(mutex);
+ ClatdTracker* getClatdTracker(const std::string& interface) REQUIRES(mutex);
static in_addr_t selectIpv4Address(const in_addr ip, int16_t prefixlen);
static int generateIpv6Address(const char* iface, const in_addr v4, const in6_addr& nat64Prefix,
@@ -89,12 +90,12 @@
ClatEbpfMaybe, // >=4.9 kernel && P api shipping level -- might work
ClatEbpfEnabled, // >=4.9 kernel && >=Q api shipping level -- must work
};
- eClatEbpfMode mClatEbpfMode;
- base::unique_fd mNetlinkFd;
- bpf::BpfMap<ClatIngressKey, ClatIngressValue> mClatIngressMap;
+ eClatEbpfMode mClatEbpfMode GUARDED_BY(mutex);
+ base::unique_fd mNetlinkFd GUARDED_BY(mutex);
+ bpf::BpfMap<ClatIngressKey, ClatIngressValue> mClatIngressMap GUARDED_BY(mutex);
- void maybeStartBpf(const ClatdTracker& tracker);
- void maybeStopBpf(const ClatdTracker& tracker);
+ void maybeStartBpf(const ClatdTracker& tracker) REQUIRES(mutex);
+ void maybeStopBpf(const ClatdTracker& tracker) REQUIRES(mutex);
// For testing.
friend class ClatdControllerTest;
diff --git a/server/CommandListener.cpp b/server/CommandListener.cpp
index 5819a93..7a980e7 100644
--- a/server/CommandListener.cpp
+++ b/server/CommandListener.cpp
@@ -119,7 +119,7 @@
registerLockingCmd(new IdletimerControlCmd(), gCtls->idletimerCtrl.lock);
registerLockingCmd(new ResolverCmd());
registerLockingCmd(new FirewallCmd(), gCtls->firewallCtrl.lock);
- registerLockingCmd(new ClatdCmd(), gCtls->clatdCtrl.mutex);
+ registerCmd(new ClatdCmd());
registerLockingCmd(new NetworkCommand());
registerLockingCmd(new StrictCmd(), gCtls->strictCtrl.lock);
}
diff --git a/server/Controllers.cpp b/server/Controllers.cpp
index c3b549f..01eae77 100644
--- a/server/Controllers.cpp
+++ b/server/Controllers.cpp
@@ -277,7 +277,7 @@
initIptablesRules();
Stopwatch s;
- clatdCtrl.Init();
+ clatdCtrl.init();
gLog.info("Initializing ClatdController: %.1fms", s.getTimeAndReset());
netdutils::Status tcStatus = trafficCtrl.start();
diff --git a/server/NetdNativeService.cpp b/server/NetdNativeService.cpp
index b1b5e0b..c2598ce 100644
--- a/server/NetdNativeService.cpp
+++ b/server/NetdNativeService.cpp
@@ -852,13 +852,13 @@
binder::Status NetdNativeService::clatdStart(const std::string& ifName,
const std::string& nat64Prefix, std::string* v6Addr) {
- NETD_LOCKING_RPC(gCtls->clatdCtrl.mutex, PERM_NETWORK_STACK, PERM_MAINLINE_NETWORK_STACK);
+ ENFORCE_ANY_PERMISSION(PERM_NETWORK_STACK, PERM_MAINLINE_NETWORK_STACK);
int res = gCtls->clatdCtrl.startClatd(ifName.c_str(), nat64Prefix, v6Addr);
return statusFromErrcode(res);
}
binder::Status NetdNativeService::clatdStop(const std::string& ifName) {
- NETD_LOCKING_RPC(gCtls->clatdCtrl.mutex, PERM_NETWORK_STACK, PERM_MAINLINE_NETWORK_STACK);
+ ENFORCE_ANY_PERMISSION(PERM_NETWORK_STACK, PERM_MAINLINE_NETWORK_STACK);
int res = gCtls->clatdCtrl.stopClatd(ifName.c_str());
return statusFromErrcode(res);
}