From 6c9393149307369c78b3ce705423cac1f58082ad Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Wed, 3 Jan 2024 11:24:21 +0800 Subject: [PATCH 1/2] Return Entry from TrafficStats JNI This is no-op refactoring which returns [tx|rx][Bytes|Packets] to the service instead of passing type to JNI and return only 1 value. This is needed for the follow-up changes to caching values in a row in the service side. This change also remove unused JNI methods. Test: atest NetworkStackIntegrationTests:android.net.NetworkStatsIntegrationTest Test: atest ConnectivityCoverageTests:android.net.connectivity.com.android.server.net.NetworkStatsServiceTest Bug: N/A Change-Id: I3b6ac97e6a1bff49cffe6153ffc2618f37a9deb4 --- ...android_server_net_NetworkStatsService.cpp | 96 ++++++++++--------- .../server/net/NetworkStatsService.java | 37 +++++-- 2 files changed, 77 insertions(+), 56 deletions(-) diff --git a/service-t/jni/com_android_server_net_NetworkStatsService.cpp b/service-t/jni/com_android_server_net_NetworkStatsService.cpp index bdbb655e5c..81912ae2cd 100644 --- a/service-t/jni/com_android_server_net_NetworkStatsService.cpp +++ b/service-t/jni/com_android_server_net_NetworkStatsService.cpp @@ -34,77 +34,64 @@ using android::bpf::bpfGetUidStats; using android::bpf::bpfGetIfaceStats; -using android::bpf::bpfGetIfIndexStats; using android::bpf::NetworkTraceHandler; namespace android { -// NOTE: keep these in sync with TrafficStats.java -static const uint64_t UNKNOWN = -1; - -enum StatsType { - RX_BYTES = 0, - RX_PACKETS = 1, - TX_BYTES = 2, - TX_PACKETS = 3, -}; - -static uint64_t getStatsType(StatsValue* stats, StatsType type) { - switch (type) { - case RX_BYTES: - return stats->rxBytes; - case RX_PACKETS: - return stats->rxPackets; - case TX_BYTES: - return stats->txBytes; - case TX_PACKETS: - return stats->txPackets; - default: - return UNKNOWN; +static jobject statsValueToEntry(JNIEnv* env, StatsValue* stats) { + // Find the Java class that represents the structure + jclass gEntryClass = env->FindClass("android/net/NetworkStats$Entry"); + if (gEntryClass == nullptr) { + return nullptr; } + + // Create a new instance of the Java class + jobject result = env->AllocObject(gEntryClass); + if (result == nullptr) { + return nullptr; + } + + // Set the values of the structure fields in the Java object + env->SetLongField(result, env->GetFieldID(gEntryClass, "rxBytes", "J"), stats->rxBytes); + env->SetLongField(result, env->GetFieldID(gEntryClass, "txBytes", "J"), stats->txBytes); + env->SetLongField(result, env->GetFieldID(gEntryClass, "rxPackets", "J"), stats->rxPackets); + env->SetLongField(result, env->GetFieldID(gEntryClass, "txPackets", "J"), stats->txPackets); + + return result; } -static jlong nativeGetTotalStat(JNIEnv* env, jclass clazz, jint type) { +static jobject nativeGetTotalStat(JNIEnv* env, jclass clazz) { StatsValue stats = {}; if (bpfGetIfaceStats(NULL, &stats) == 0) { - return getStatsType(&stats, (StatsType) type); + return statsValueToEntry(env, &stats); } else { - return UNKNOWN; + return nullptr; } } -static jlong nativeGetIfaceStat(JNIEnv* env, jclass clazz, jstring iface, jint type) { +static jobject nativeGetIfaceStat(JNIEnv* env, jclass clazz, jstring iface) { ScopedUtfChars iface8(env, iface); if (iface8.c_str() == NULL) { - return UNKNOWN; + return nullptr; } StatsValue stats = {}; if (bpfGetIfaceStats(iface8.c_str(), &stats) == 0) { - return getStatsType(&stats, (StatsType) type); + return statsValueToEntry(env, &stats); } else { - return UNKNOWN; + return nullptr; } } -static jlong nativeGetIfIndexStat(JNIEnv* env, jclass clazz, jint ifindex, jint type) { - StatsValue stats = {}; - if (bpfGetIfIndexStats(ifindex, &stats) == 0) { - return getStatsType(&stats, (StatsType) type); - } else { - return UNKNOWN; - } -} - -static jlong nativeGetUidStat(JNIEnv* env, jclass clazz, jint uid, jint type) { +static jobject nativeGetUidStat(JNIEnv* env, jclass clazz, jint uid) { StatsValue stats = {}; if (bpfGetUidStats(uid, &stats) == 0) { - return getStatsType(&stats, (StatsType) type); + return statsValueToEntry(env, &stats); } else { - return UNKNOWN; + return nullptr; } } @@ -113,11 +100,26 @@ static void nativeInitNetworkTracing(JNIEnv* env, jclass clazz) { } static const JNINativeMethod gMethods[] = { - {"nativeGetTotalStat", "(I)J", (void*)nativeGetTotalStat}, - {"nativeGetIfaceStat", "(Ljava/lang/String;I)J", (void*)nativeGetIfaceStat}, - {"nativeGetIfIndexStat", "(II)J", (void*)nativeGetIfIndexStat}, - {"nativeGetUidStat", "(II)J", (void*)nativeGetUidStat}, - {"nativeInitNetworkTracing", "()V", (void*)nativeInitNetworkTracing}, + { + "nativeGetTotalStat", + "()Landroid/net/NetworkStats$Entry;", + (void*)nativeGetTotalStat + }, + { + "nativeGetIfaceStat", + "(Ljava/lang/String;)Landroid/net/NetworkStats$Entry;", + (void*)nativeGetIfaceStat + }, + { + "nativeGetUidStat", + "(I)Landroid/net/NetworkStats$Entry;", + (void*)nativeGetUidStat + }, + { + "nativeInitNetworkTracing", + "()V", + (void*)nativeInitNetworkTracing + }, }; int register_android_server_net_NetworkStatsService(JNIEnv* env) { diff --git a/service-t/src/com/android/server/net/NetworkStatsService.java b/service-t/src/com/android/server/net/NetworkStatsService.java index 3ac5e29620..fa95ce6564 100644 --- a/service-t/src/com/android/server/net/NetworkStatsService.java +++ b/service-t/src/com/android/server/net/NetworkStatsService.java @@ -1981,14 +1981,14 @@ public class NetworkStatsService extends INetworkStatsService.Stub { if (callingUid != android.os.Process.SYSTEM_UID && callingUid != uid) { return UNSUPPORTED; } - return nativeGetUidStat(uid, type); + return getEntryValueForType(nativeGetUidStat(uid), type); } @Override public long getIfaceStats(@NonNull String iface, int type) { Objects.requireNonNull(iface); - long nativeIfaceStats = nativeGetIfaceStat(iface, type); - if (nativeIfaceStats == -1) { + long nativeIfaceStats = getEntryValueForType(nativeGetIfaceStat(iface), type); + if (nativeIfaceStats == UNSUPPORTED) { return nativeIfaceStats; } else { // When tethering offload is in use, nativeIfaceStats does not contain usage from @@ -1999,10 +1999,26 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } } + private long getEntryValueForType(@Nullable NetworkStats.Entry entry, int type) { + if (entry == null) return UNSUPPORTED; + switch (type) { + case TrafficStats.TYPE_RX_BYTES: + return entry.rxBytes; + case TrafficStats.TYPE_TX_BYTES: + return entry.txBytes; + case TrafficStats.TYPE_RX_PACKETS: + return entry.rxPackets; + case TrafficStats.TYPE_TX_PACKETS: + return entry.txPackets; + default: + return UNSUPPORTED; + } + } + @Override public long getTotalStats(int type) { - long nativeTotalStats = nativeGetTotalStat(type); - if (nativeTotalStats == -1) { + final long nativeTotalStats = getEntryValueForType(nativeGetTotalStat(), type); + if (nativeTotalStats == UNSUPPORTED) { return nativeTotalStats; } else { // Refer to comment in getIfaceStats @@ -3398,10 +3414,13 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } } - private static native long nativeGetTotalStat(int type); - private static native long nativeGetIfaceStat(String iface, int type); - private static native long nativeGetIfIndexStat(int ifindex, int type); - private static native long nativeGetUidStat(int uid, int type); + // TODO: Read stats by using BpfNetMapsReader. + @Nullable + private static native NetworkStats.Entry nativeGetTotalStat(); + @Nullable + private static native NetworkStats.Entry nativeGetIfaceStat(String iface); + @Nullable + private static native NetworkStats.Entry nativeGetUidStat(int uid); /** Initializes and registers the Perfetto Network Trace data source */ public static native void nativeInitNetworkTracing(); From d2e3b8d69947fb7463ca5cd860c598de46d9dadc Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Wed, 3 Jan 2024 14:53:08 +0800 Subject: [PATCH 2/2] Make getProviderIfaceStats return Entry This refactoring makes the method return Entry for follow-up changes to perform arithmetic operations on them easily. This change also removes error handling when the type is unsupported when calling getProviderIfaceStats because it cannot happen in practice. Test: atest ConnectivityCoverageTests:android.net.connectivity.com.android.server.net.NetworkStatsServiceTest Bug: N/A Change-Id: Id26a24c80bb87c399972ef8c3abf16e0f4719cb1 --- .../server/net/NetworkStatsService.java | 36 ++++++++----------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/service-t/src/com/android/server/net/NetworkStatsService.java b/service-t/src/com/android/server/net/NetworkStatsService.java index fa95ce6564..eb754616a6 100644 --- a/service-t/src/com/android/server/net/NetworkStatsService.java +++ b/service-t/src/com/android/server/net/NetworkStatsService.java @@ -1987,15 +1987,17 @@ public class NetworkStatsService extends INetworkStatsService.Stub { @Override public long getIfaceStats(@NonNull String iface, int type) { Objects.requireNonNull(iface); - long nativeIfaceStats = getEntryValueForType(nativeGetIfaceStat(iface), type); - if (nativeIfaceStats == UNSUPPORTED) { - return nativeIfaceStats; + final NetworkStats.Entry entry = nativeGetIfaceStat(iface); + final long value = getEntryValueForType(entry, type); + if (value == UNSUPPORTED) { + return UNSUPPORTED; } else { // When tethering offload is in use, nativeIfaceStats does not contain usage from // offload, add it back here. Note that the included statistics might be stale // since polling newest stats from hardware might impact system health and not // suitable for TrafficStats API use cases. - return nativeIfaceStats + getProviderIfaceStats(iface, type); + entry.add(getProviderIfaceStats(iface)); + return getEntryValueForType(entry, type); } } @@ -2017,16 +2019,18 @@ public class NetworkStatsService extends INetworkStatsService.Stub { @Override public long getTotalStats(int type) { - final long nativeTotalStats = getEntryValueForType(nativeGetTotalStat(), type); - if (nativeTotalStats == UNSUPPORTED) { - return nativeTotalStats; + final NetworkStats.Entry entry = nativeGetTotalStat(); + final long value = getEntryValueForType(entry, type); + if (value == UNSUPPORTED) { + return UNSUPPORTED; } else { // Refer to comment in getIfaceStats - return nativeTotalStats + getProviderIfaceStats(IFACE_ALL, type); + entry.add(getProviderIfaceStats(IFACE_ALL)); + return getEntryValueForType(entry, type); } } - private long getProviderIfaceStats(@Nullable String iface, int type) { + private NetworkStats.Entry getProviderIfaceStats(@Nullable String iface) { final NetworkStats providerSnapshot = getNetworkStatsFromProviders(STATS_PER_IFACE); final HashSet limitIfaces; if (iface == IFACE_ALL) { @@ -2035,19 +2039,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { limitIfaces = new HashSet<>(); limitIfaces.add(iface); } - final NetworkStats.Entry entry = providerSnapshot.getTotal(null, limitIfaces); - switch (type) { - case TrafficStats.TYPE_RX_BYTES: - return entry.rxBytes; - case TrafficStats.TYPE_RX_PACKETS: - return entry.rxPackets; - case TrafficStats.TYPE_TX_BYTES: - return entry.txBytes; - case TrafficStats.TYPE_TX_PACKETS: - return entry.txPackets; - default: - return 0; - } + return providerSnapshot.getTotal(null, limitIfaces); } /**