diff --git a/service-t/native/libs/libnetworkstats/NetworkTraceHandler.cpp b/service-t/native/libs/libnetworkstats/NetworkTraceHandler.cpp index aeadb4a256..be4ffe3a2f 100644 --- a/service-t/native/libs/libnetworkstats/NetworkTraceHandler.cpp +++ b/service-t/native/libs/libnetworkstats/NetworkTraceHandler.cpp @@ -55,13 +55,12 @@ void NetworkTraceHandler::InitPerfettoTracing() { NetworkTraceHandler::RegisterDataSource(); } -NetworkTraceHandler::NetworkTraceHandler() - : NetworkTraceHandler([this](const PacketTrace& pkt) { - NetworkTraceHandler::Trace( - [this, pkt](NetworkTraceHandler::TraceContext ctx) { - Fill(pkt, *ctx.NewTracePacket()); - }); - }) {} +// static +NetworkTracePoller NetworkTraceHandler::sPoller([](const PacketTrace& pkt) { + NetworkTraceHandler::Trace([pkt](NetworkTraceHandler::TraceContext ctx) { + NetworkTraceHandler::Fill(pkt, *ctx.NewTracePacket()); + }); +}); void NetworkTraceHandler::OnSetup(const SetupArgs& args) { const std::string& raw = args.config->network_packet_trace_config_raw(); @@ -75,21 +74,27 @@ void NetworkTraceHandler::OnSetup(const SetupArgs& args) { } void NetworkTraceHandler::OnStart(const StartArgs&) { - if (!Start()) return; - mTaskRunner = perfetto::Platform::GetDefaultPlatform()->CreateTaskRunner({}); - Loop(); + mStarted = sPoller.Start(mPollMs); } void NetworkTraceHandler::OnStop(const StopArgs&) { - Stop(); - mTaskRunner.reset(); + if (mStarted) sPoller.Stop(); + mStarted = false; } -void NetworkTraceHandler::Loop() { - mTaskRunner->PostDelayedTask([this]() { Loop(); }, mPollMs); - ConsumeAll(); +void NetworkTracePoller::SchedulePolling() { + // Schedules another run of ourselves to recursively poll periodically. + mTaskRunner->PostDelayedTask( + [this]() { + mMutex.lock(); + SchedulePolling(); + ConsumeAllLocked(); + mMutex.unlock(); + }, + mPollMs); } +// static class method void NetworkTraceHandler::Fill(const PacketTrace& src, TracePacket& dst) { dst.set_timestamp(src.timestampNs); auto* event = dst.set_network_packet(); @@ -113,9 +118,23 @@ void NetworkTraceHandler::Fill(const PacketTrace& src, TracePacket& dst) { } } -bool NetworkTraceHandler::Start() { +bool NetworkTracePoller::Start(uint32_t pollMs) { ALOGD("Starting datasource"); + std::scoped_lock lock(mMutex); + if (mSessionCount > 0) { + if (mPollMs != pollMs) { + // Nothing technical prevents mPollMs from changing, it's just unclear + // what the right behavior is. Taking the min of active values could poll + // too frequently giving some sessions too much data. Taking the max could + // be too infrequent. For now, do nothing. + ALOGI("poll_ms can't be changed while running, ignoring poll_ms=%d", + pollMs); + } + mSessionCount++; + return true; + } + auto status = mConfigurationMap.init(PACKET_TRACE_ENABLED_MAP_PATH); if (!status.ok()) { ALOGW("Failed to bind config map: %s", status.error().message().c_str()); @@ -136,24 +155,41 @@ bool NetworkTraceHandler::Start() { return false; } + // Start a task runner to run ConsumeAll every mPollMs milliseconds. + mTaskRunner = perfetto::Platform::GetDefaultPlatform()->CreateTaskRunner({}); + mPollMs = pollMs; + SchedulePolling(); + + mSessionCount++; return true; } -bool NetworkTraceHandler::Stop() { +bool NetworkTracePoller::Stop() { ALOGD("Stopping datasource"); + std::scoped_lock lock(mMutex); + if (mSessionCount == 0) return false; // This should never happen + + // If this isn't the last session, don't clean up yet. + if (--mSessionCount > 0) return true; + auto res = mConfigurationMap.writeValue(0, false, BPF_ANY); if (!res.ok()) { ALOGW("Failed to disable tracing: %s", res.error().message().c_str()); - return false; } + mTaskRunner.reset(); mRingBuffer.reset(); - return true; + return res.ok(); } -bool NetworkTraceHandler::ConsumeAll() { +bool NetworkTracePoller::ConsumeAll() { + std::scoped_lock lock(mMutex); + return ConsumeAllLocked(); +} + +bool NetworkTracePoller::ConsumeAllLocked() { if (mRingBuffer == nullptr) { ALOGW("Tracing is not active"); return false; diff --git a/service-t/native/libs/libnetworkstats/NetworkTraceHandlerTest.cpp b/service-t/native/libs/libnetworkstats/NetworkTraceHandlerTest.cpp index 560194f6df..543be214fc 100644 --- a/service-t/native/libs/libnetworkstats/NetworkTraceHandlerTest.cpp +++ b/service-t/native/libs/libnetworkstats/NetworkTraceHandlerTest.cpp @@ -39,6 +39,9 @@ using ::testing::Test; namespace android { namespace bpf { +// Use uint32 max to cause the handler to never Loop. Instead, the tests will +// manually drive things by calling ConsumeAll explicitly. +constexpr uint32_t kNeverPoll = std::numeric_limits::max(); __be16 bindAndListen(int s) { sockaddr_in sin = {.sin_family = AF_INET}; @@ -83,7 +86,7 @@ struct PacketPrinter { } }; -class NetworkTraceHandlerTest : public testing::Test { +class NetworkTracePollerTest : public testing::Test { protected: void SetUp() { if (access(PACKET_TRACE_RINGBUF_PATH, R_OK)) { @@ -95,31 +98,49 @@ class NetworkTraceHandlerTest : public testing::Test { } }; -TEST_F(NetworkTraceHandlerTest, PollWhileInactive) { - NetworkTraceHandler handler([&](const PacketTrace& pkt) {}); +TEST_F(NetworkTracePollerTest, PollWhileInactive) { + NetworkTracePoller handler([&](const PacketTrace& pkt) {}); // One succeed after start and before stop. EXPECT_FALSE(handler.ConsumeAll()); - ASSERT_TRUE(handler.Start()); + ASSERT_TRUE(handler.Start(kNeverPoll)); EXPECT_TRUE(handler.ConsumeAll()); ASSERT_TRUE(handler.Stop()); EXPECT_FALSE(handler.ConsumeAll()); } -TEST_F(NetworkTraceHandlerTest, TraceTcpSession) { +TEST_F(NetworkTracePollerTest, ConcurrentSessions) { + // Simulate two concurrent sessions (two starts followed by two stops). Check + // that tracing is stopped only after both sessions finish. + NetworkTracePoller handler([&](const PacketTrace& pkt) {}); + + ASSERT_TRUE(handler.Start(kNeverPoll)); + EXPECT_TRUE(handler.ConsumeAll()); + + ASSERT_TRUE(handler.Start(kNeverPoll)); + EXPECT_TRUE(handler.ConsumeAll()); + + ASSERT_TRUE(handler.Stop()); + EXPECT_TRUE(handler.ConsumeAll()); + + ASSERT_TRUE(handler.Stop()); + EXPECT_FALSE(handler.ConsumeAll()); +} + +TEST_F(NetworkTracePollerTest, TraceTcpSession) { __be16 server_port = 0; std::vector packets; // Record all packets with the bound address and current uid. This callback is // involked only within ConsumeAll, at which point the port should have // already been filled in and all packets have been processed. - NetworkTraceHandler handler([&](const PacketTrace& pkt) { + NetworkTracePoller handler([&](const PacketTrace& pkt) { if (pkt.sport != server_port && pkt.dport != server_port) return; if (pkt.uid != getuid()) return; packets.push_back(pkt); }); - ASSERT_TRUE(handler.Start()); + ASSERT_TRUE(handler.Start(kNeverPoll)); const uint32_t kClientTag = 2468; const uint32_t kServerTag = 1357; diff --git a/service-t/native/libs/libnetworkstats/include/netdbpf/NetworkTraceHandler.h b/service-t/native/libs/libnetworkstats/include/netdbpf/NetworkTraceHandler.h index c257aa090b..3f244b338f 100644 --- a/service-t/native/libs/libnetworkstats/include/netdbpf/NetworkTraceHandler.h +++ b/service-t/native/libs/libnetworkstats/include/netdbpf/NetworkTraceHandler.h @@ -22,6 +22,7 @@ #include #include +#include "android-base/thread_annotations.h" #include "bpf/BpfMap.h" #include "bpf/BpfRingbuf.h" @@ -31,6 +32,56 @@ namespace android { namespace bpf { +// NetworkTracePoller is responsible for interactions with the BPF ring buffer +// including polling. This class is an internal helper for NetworkTraceHandler, +// it is not meant to be used elsewhere. +class NetworkTracePoller { + public: + // Testonly: initialize with a callback capable of intercepting data. + NetworkTracePoller(std::function callback) + : mCallback(std::move(callback)) {} + + // Starts tracing with the given poll interval. + bool Start(uint32_t pollMs) EXCLUDES(mMutex); + + // Stops tracing and release any held state. + bool Stop() EXCLUDES(mMutex); + + // Consumes all available events from the ringbuffer. + bool ConsumeAll() EXCLUDES(mMutex); + + private: + void SchedulePolling() REQUIRES(mMutex); + bool ConsumeAllLocked() REQUIRES(mMutex); + + std::mutex mMutex; + + // Records the number of successfully started active sessions so that only the + // first active session attempts setup and only the last cleans up. Note that + // the session count will remain zero if Start fails. It is expected that Stop + // will not be called for any trace session where Start fails. + int mSessionCount GUARDED_BY(mMutex); + + // How often to poll the ring buffer, defined by the trace config. + uint32_t mPollMs GUARDED_BY(mMutex); + + // The function to process PacketTrace, typically a Perfetto sink. + std::function mCallback GUARDED_BY(mMutex); + + // The BPF ring buffer handle. + std::unique_ptr> mRingBuffer GUARDED_BY(mMutex); + + // The packet tracing config map (really a 1-element array). + BpfMap mConfigurationMap GUARDED_BY(mMutex); + + // This must be the last member, causing it to be the first deleted. If it is + // not, members required for callbacks can be deleted before it's stopped. + std::unique_ptr mTaskRunner GUARDED_BY(mMutex); +}; + +// NetworkTraceHandler implements the android.network_packets data source. This +// class is registered with Perfetto and is instantiated when tracing starts and +// destroyed when tracing ends. There is one instance per trace session. class NetworkTraceHandler : public perfetto::DataSource { public: // Registers this DataSource. @@ -39,45 +90,19 @@ class NetworkTraceHandler : public perfetto::DataSource { // Connects to the system Perfetto daemon and registers the trace handler. static void InitPerfettoTracing(); - // Initialize with the default Perfetto callback. - NetworkTraceHandler(); - - // Testonly: initialize with a callback capable of intercepting data. - NetworkTraceHandler(std::function callback) - : mCallback(std::move(callback)) {} - - // Testonly: standalone functions without perfetto dependency. - bool Start(); - bool Stop(); - bool ConsumeAll(); - // perfetto::DataSource overrides: - void OnSetup(const SetupArgs&) override; + void OnSetup(const SetupArgs& args) override; void OnStart(const StartArgs&) override; void OnStop(const StopArgs&) override; - // Convert a PacketTrace into a Perfetto trace packet. - void Fill(const PacketTrace& src, - ::perfetto::protos::pbzero::TracePacket& dst); - private: - void Loop(); + // Convert a PacketTrace into a Perfetto trace packet. + static void Fill(const PacketTrace& src, + ::perfetto::protos::pbzero::TracePacket& dst); - // How often to poll the ring buffer, defined by the trace config. + static NetworkTracePoller sPoller; uint32_t mPollMs; - - // The function to process PacketTrace, typically a Perfetto sink. - std::function mCallback; - - // The BPF ring buffer handle. - std::unique_ptr> mRingBuffer; - - // The packet tracing config map (really a 1-element array). - BpfMap mConfigurationMap; - - // This must be the last member, causing it to be the first deleted. If it is - // not, members required for callbacks can be deleted before it's stopped. - std::unique_ptr mTaskRunner; + bool mStarted; }; } // namespace bpf