From cedfab91c6688797a1f635d7a33b8cdf30aa7c62 Mon Sep 17 00:00:00 2001 From: Motomu Utsumi Date: Wed, 27 Jul 2022 08:14:09 +0000 Subject: [PATCH] Move cookie tag bpf map dump to NetworkStatsService Map status dump will do access check if map is null. This could show different message from the current dump output. Information in map content dump does not change $ dumpsys connectivity trafficcontroller .... mCookieTagMap: cookie=1398 tag=0x0 uid=1029 cookie=1433 tag=0xffffff82 uid=1051 cookie=1166 tag=0xfffffe01 uid=1073 $ dumpsys netstats .... mCookieTagMap: cookie=1144 tag=0xfffffe01 uid=1073 cookie=1376 tag=0x0 uid=1029 cookie=1408 tag=0xffffff82 uid=1051 Bug: 217624062 Test: dumpsys netstats, dumpstate, atest NetworkStatsServiceTest Change-Id: I14dd6f969a0b5eb24ace62361ce2484cf18b7470 --- .../server/net/NetworkStatsService.java | 60 +++++++++++++++++++ service/native/TrafficController.cpp | 16 +---- service/native/TrafficControllerTest.cpp | 3 - .../server/net/NetworkStatsServiceTest.java | 26 ++++++++ 4 files changed, 87 insertions(+), 18 deletions(-) diff --git a/service-t/src/com/android/server/net/NetworkStatsService.java b/service-t/src/com/android/server/net/NetworkStatsService.java index 77931b1cf9..f4e02b3bcb 100644 --- a/service-t/src/com/android/server/net/NetworkStatsService.java +++ b/service-t/src/com/android/server/net/NetworkStatsService.java @@ -53,6 +53,7 @@ import static android.net.netstats.NetworkStatsDataMigrationUtils.PREFIX_UID_TAG import static android.net.netstats.NetworkStatsDataMigrationUtils.PREFIX_XT; import static android.os.Trace.TRACE_TAG_NETWORK; import static android.system.OsConstants.ENOENT; +import static android.system.OsConstants.R_OK; import static android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID; import static android.text.format.DateUtils.DAY_IN_MILLIS; import static android.text.format.DateUtils.HOUR_IN_MILLIS; @@ -130,6 +131,7 @@ import android.provider.Settings.Global; import android.service.NetworkInterfaceProto; import android.service.NetworkStatsServiceDumpProto; import android.system.ErrnoException; +import android.system.Os; import android.telephony.PhoneStateListener; import android.telephony.SubscriptionPlan; import android.text.TextUtils; @@ -156,6 +158,7 @@ import com.android.net.module.util.IBpfMap; import com.android.net.module.util.LocationPermissionChecker; import com.android.net.module.util.NetworkStatsUtils; import com.android.net.module.util.PermissionUtils; +import com.android.net.module.util.Struct; import com.android.net.module.util.Struct.U32; import com.android.net.module.util.Struct.U8; import com.android.net.module.util.bpf.CookieTagMapKey; @@ -2698,6 +2701,21 @@ public class NetworkStatsService extends INetworkStatsService.Stub { mUidTagRecorder.dumpLocked(pw, fullHistory); pw.decreaseIndent(); } + + pw.println(); + pw.println("BPF map status:"); + pw.increaseIndent(); + dumpMapStatus(pw); + pw.decreaseIndent(); + pw.println(); + + // Following BPF map content dump contains uid and tag regardless of the flags because + // following dumps are moved from TrafficController and bug report already contains this + // information. + pw.println("BPF map content:"); + pw.increaseIndent(); + dumpCookieTagMapLocked(pw); + pw.decreaseIndent(); } } @@ -2732,6 +2750,48 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } } + private String getMapStatus( + final IBpfMap map, final String path) { + if (map != null) { + return "OK"; + } + try { + Os.access(path, R_OK); + return "NULL(map is pinned to " + path + ")"; + } catch (ErrnoException e) { + return "NULL(map is not pinned to " + path + ": " + Os.strerror(e.errno) + ")"; + } + } + + private void dumpMapStatus(final IndentingPrintWriter pw) { + pw.println("mCookieTagMap: " + getMapStatus(mCookieTagMap, COOKIE_TAG_MAP_PATH)); + } + + @GuardedBy("mStatsLock") + private void dumpCookieTagMapLocked(final IndentingPrintWriter pw) { + if (mCookieTagMap == null) { + return; + } + pw.println("mCookieTagMap:"); + pw.increaseIndent(); + try { + mCookieTagMap.forEach((key, value) -> { + // value could be null if there is a concurrent entry deletion. + // http://b/220084230. + if (value != null) { + pw.println("cookie=" + key.socketCookie + + " tag=0x" + Long.toHexString(value.tag) + + " uid=" + value.uid); + } else { + pw.println("Entry is deleted while dumping, iterating from first entry"); + } + }); + } catch (ErrnoException e) { + pw.println("mCookieTagMap dump end with error: " + Os.strerror(e.errno)); + } + pw.decreaseIndent(); + } + private NetworkStats readNetworkStatsSummaryDev() { try { return mStatsFactory.readNetworkStatsSummaryDev(); diff --git a/service/native/TrafficController.cpp b/service/native/TrafficController.cpp index 9331548e3f..e9c028fea3 100644 --- a/service/native/TrafficController.cpp +++ b/service/native/TrafficController.cpp @@ -570,8 +570,6 @@ void TrafficController::dump(int fd, bool verbose) { ScopedIndent indentPreBpfModule(dw); dw.blankline(); - dw.println("mCookieTagMap status: %s", - getMapStatus(mCookieTagMap.getMap(), COOKIE_TAG_MAP_PATH).c_str()); dw.println("mUidCounterSetMap status: %s", getMapStatus(mUidCounterSetMap.getMap(), UID_COUNTERSET_MAP_PATH).c_str()); dw.println("mAppUidStatsMap status: %s", @@ -611,18 +609,6 @@ void TrafficController::dump(int fd, bool verbose) { ScopedIndent indentForMapContent(dw); - // Print CookieTagMap content. - dumpBpfMap("mCookieTagMap", dw, ""); - const auto printCookieTagInfo = [&dw](const uint64_t& key, const UidTagValue& value, - const BpfMap&) { - dw.println("cookie=%" PRIu64 " tag=0x%x uid=%u", key, value.tag, value.uid); - return base::Result(); - }; - base::Result res = mCookieTagMap.iterateWithValue(printCookieTagInfo); - if (!res.ok()) { - dw.println("mCookieTagMap print end with error: %s", res.error().message().c_str()); - } - // Print UidCounterSetMap content. dumpBpfMap("mUidCounterSetMap", dw, ""); const auto printUidInfo = [&dw](const uint32_t& key, const uint8_t& value, @@ -630,7 +616,7 @@ void TrafficController::dump(int fd, bool verbose) { dw.println("%u %u", key, value); return base::Result(); }; - res = mUidCounterSetMap.iterateWithValue(printUidInfo); + base::Result res = mUidCounterSetMap.iterateWithValue(printUidInfo); if (!res.ok()) { dw.println("mUidCounterSetMap print end with error: %s", res.error().message().c_str()); } diff --git a/service/native/TrafficControllerTest.cpp b/service/native/TrafficControllerTest.cpp index 7730c13e97..d6ed721d29 100644 --- a/service/native/TrafficControllerTest.cpp +++ b/service/native/TrafficControllerTest.cpp @@ -791,8 +791,6 @@ TEST_F(TrafficControllerTest, TestDumpsys) { // ifaceIndex ifaceName tag_hex uid_int cnt_set rxBytes rxPackets txBytes txPackets // 999 test0 0x2a 10086 1 100 1 0 0 std::vector expectedLines = { - "mCookieTagMap:", - fmt::format("cookie={} tag={:#x} uid={}", TEST_COOKIE, TEST_TAG, TEST_UID), "mUidCounterSetMap:", fmt::format("{} {}", TEST_UID3, TEST_COUNTERSET), "mAppUidStatsMap::", // TODO@: fix double colon @@ -833,7 +831,6 @@ TEST_F(TrafficControllerTest, dumpsysInvalidMaps) { "Read value of map -1 failed: Bad file descriptor"; std::vector expectedLines = { - fmt::format("mCookieTagMap {}", kErrIterate), fmt::format("mUidCounterSetMap {}", kErrIterate), fmt::format("mAppUidStatsMap {}", kErrIterate), fmt::format("mStatsMapA {}", kErrIterate), diff --git a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java index 484d71771b..168aeefe92 100644 --- a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java @@ -16,6 +16,7 @@ package com.android.server.net; +import static android.Manifest.permission.DUMP; import static android.Manifest.permission.READ_NETWORK_USAGE_HISTORY; import static android.Manifest.permission.UPDATE_DEVICE_STATS; import static android.app.usage.NetworkStatsManager.PREFIX_DEV; @@ -156,7 +157,10 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import java.io.File; +import java.io.FileDescriptor; import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; import java.nio.file.Files; import java.nio.file.Path; import java.time.Clock; @@ -283,6 +287,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { case PERMISSION_MAINLINE_NETWORK_STACK: case READ_NETWORK_USAGE_HISTORY: case UPDATE_DEVICE_STATS: + case DUMP: return PERMISSION_GRANTED; default: return PERMISSION_DENIED; @@ -2317,4 +2322,25 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { assertTrue(mAppUidStatsMap.containsKey(new UidStatsMapKey(UID_RED))); assertTrue(mUidCounterSetMap.containsKey(new U32(UID_RED))); } + + private void assertDumpContains(final String dump, final String message) { + assertTrue(String.format("dump(%s) does not contain '%s'", dump, message), + dump.contains(message)); + } + + private String getDump() { + final StringWriter sw = new StringWriter(); + mService.dump(new FileDescriptor(), new PrintWriter(sw), new String[]{}); + return sw.toString(); + } + + @Test + public void testDumpCookieTagMap() throws ErrnoException { + initBpfMapsWithTagData(UID_BLUE); + + final String dump = getDump(); + assertDumpContains(dump, "mCookieTagMap: OK"); + assertDumpContains(dump, "cookie=2002 tag=0x1 uid=1002"); + assertDumpContains(dump, "cookie=3002 tag=0x2 uid=1002"); + } }