diff options
Diffstat (limited to 'service-t')
15 files changed, 881 insertions, 289 deletions
diff --git a/service-t/Android.bp b/service-t/Android.bp index 7e588cd412..bd2f916c32 100644 --- a/service-t/Android.bp +++ b/service-t/Android.bp @@ -31,6 +31,7 @@ filegroup { ], visibility: ["//visibility:private"], } + // The above filegroup can be used to specify different sources depending // on the branch, while minimizing merge conflicts in the rest of the // build rules. @@ -78,6 +79,9 @@ java_library { "//packages/modules/Connectivity/tests:__subpackages__", "//packages/modules/IPsec/tests/iketests", ], + lint: { + baseline_filename: "lint-baseline.xml", + }, } // Test building mDNS as a standalone, so that it can be imported into other repositories as-is. @@ -94,11 +98,12 @@ java_library { min_sdk_version: "21", lint: { error_checks: ["NewApi"], + baseline_filename: "lint-baseline.xml", }, srcs: [ "src/com/android/server/connectivity/mdns/**/*.java", ":framework-connectivity-t-mdns-standalone-build-sources", - ":service-mdns-droidstubs" + ":service-mdns-droidstubs", ], exclude_srcs: [ "src/com/android/server/connectivity/mdns/internal/SocketNetlinkMonitor.java", @@ -127,7 +132,7 @@ droidstubs { srcs: ["src/com/android/server/connectivity/mdns/SocketNetLinkMonitorFactory.java"], libs: [ "net-utils-device-common-mdns-standalone-build-test", - "service-connectivity-tiramisu-pre-jarjar" + "service-connectivity-tiramisu-pre-jarjar", ], visibility: [ "//visibility:private", 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 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; + } -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; + // 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); - } else { - return UNKNOWN; - } -} - -static jlong nativeGetIfIndexStat(JNIEnv* env, jclass clazz, jint ifindex, jint type) { - StatsValue stats = {}; - if (bpfGetIfIndexStats(ifindex, &stats) == 0) { - return getStatsType(&stats, (StatsType) type); + return statsValueToEntry(env, &stats); } else { - return UNKNOWN; + return nullptr; } } -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/native/libs/libnetworkstats/Android.bp b/service-t/native/libs/libnetworkstats/Android.bp index 0dfd0afa92..b9f3adb91c 100644 --- a/service-t/native/libs/libnetworkstats/Android.bp +++ b/service-t/native/libs/libnetworkstats/Android.bp @@ -73,6 +73,7 @@ cc_test { "-Wthread-safety", ], static_libs: [ + "libbase", "libgmock", "libnetworkstats", "libperfetto_client_experimental", @@ -80,7 +81,6 @@ cc_test { "perfetto_trace_protos", ], shared_libs: [ - "libbase", "liblog", "libcutils", "libandroid_net", diff --git a/service-t/src/com/android/metrics/NetworkStatsMetricsLogger.java b/service-t/src/com/android/metrics/NetworkStatsMetricsLogger.java new file mode 100644 index 0000000000..3ed21a2b61 --- /dev/null +++ b/service-t/src/com/android/metrics/NetworkStatsMetricsLogger.java @@ -0,0 +1,171 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.metrics; + +import static android.net.netstats.NetworkStatsDataMigrationUtils.PREFIX_UID; +import static android.net.netstats.NetworkStatsDataMigrationUtils.PREFIX_UID_TAG; +import static android.net.netstats.NetworkStatsDataMigrationUtils.PREFIX_XT; + +import static com.android.server.ConnectivityStatsLog.NETWORK_STATS_RECORDER_FILE_OPERATED; +import static com.android.server.ConnectivityStatsLog.NETWORK_STATS_RECORDER_FILE_OPERATED__FAST_DATA_INPUT_STATE__FDIS_DISABLED; +import static com.android.server.ConnectivityStatsLog.NETWORK_STATS_RECORDER_FILE_OPERATED__FAST_DATA_INPUT_STATE__FDIS_ENABLED; +import static com.android.server.ConnectivityStatsLog.NETWORK_STATS_RECORDER_FILE_OPERATED__OPERATION_TYPE__ROT_READ; +import static com.android.server.ConnectivityStatsLog.NETWORK_STATS_RECORDER_FILE_OPERATED__RECORDER_PREFIX__PREFIX_UID; +import static com.android.server.ConnectivityStatsLog.NETWORK_STATS_RECORDER_FILE_OPERATED__RECORDER_PREFIX__PREFIX_UIDTAG; +import static com.android.server.ConnectivityStatsLog.NETWORK_STATS_RECORDER_FILE_OPERATED__RECORDER_PREFIX__PREFIX_UNKNOWN; +import static com.android.server.ConnectivityStatsLog.NETWORK_STATS_RECORDER_FILE_OPERATED__RECORDER_PREFIX__PREFIX_XT; + +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.net.NetworkStatsCollection; +import android.net.NetworkStatsHistory; +import android.util.Pair; + +import com.android.internal.annotations.VisibleForTesting; +import com.android.server.ConnectivityStatsLog; + +import java.io.File; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.regex.Pattern; + +/** + * Helper class to log NetworkStats related metrics. + * + * This class does not provide thread-safe. + */ +public class NetworkStatsMetricsLogger { + final Dependencies mDeps; + int mReadIndex = 1; + + /** Dependency class */ + @VisibleForTesting + public static class Dependencies { + /** + * Writes a NETWORK_STATS_RECORDER_FILE_OPERATION_REPORTED event to ConnectivityStatsLog. + */ + public void writeRecorderFileReadingStats(int recorderType, int readIndex, + int readLatencyMillis, + int fileCount, int totalFileSize, + int keys, int uids, int totalHistorySize, + boolean useFastDataInput) { + ConnectivityStatsLog.write(NETWORK_STATS_RECORDER_FILE_OPERATED, + NETWORK_STATS_RECORDER_FILE_OPERATED__OPERATION_TYPE__ROT_READ, + recorderType, + readIndex, + readLatencyMillis, + fileCount, + totalFileSize, + keys, + uids, + totalHistorySize, + useFastDataInput + ? NETWORK_STATS_RECORDER_FILE_OPERATED__FAST_DATA_INPUT_STATE__FDIS_ENABLED + : NETWORK_STATS_RECORDER_FILE_OPERATED__FAST_DATA_INPUT_STATE__FDIS_DISABLED); + } + } + + public NetworkStatsMetricsLogger() { + mDeps = new Dependencies(); + } + + @VisibleForTesting + public NetworkStatsMetricsLogger(Dependencies deps) { + mDeps = deps; + } + + private static int prefixToRecorderType(@NonNull String prefix) { + switch (prefix) { + case PREFIX_XT: + return NETWORK_STATS_RECORDER_FILE_OPERATED__RECORDER_PREFIX__PREFIX_XT; + case PREFIX_UID: + return NETWORK_STATS_RECORDER_FILE_OPERATED__RECORDER_PREFIX__PREFIX_UID; + case PREFIX_UID_TAG: + return NETWORK_STATS_RECORDER_FILE_OPERATED__RECORDER_PREFIX__PREFIX_UIDTAG; + default: + return NETWORK_STATS_RECORDER_FILE_OPERATED__RECORDER_PREFIX__PREFIX_UNKNOWN; + } + } + + /** + * Get file count and total byte count for the given directory and prefix. + * + * @return File count and total byte count as a pair, or 0s if met errors. + */ + private static Pair<Integer, Integer> getStatsFilesAttributes( + @Nullable File statsDir, @NonNull String prefix) { + if (statsDir == null) return new Pair<>(0, 0); + + // Only counts the matching files. + // The files are named in the following format: + // <prefix>.<startTimestamp>-[<endTimestamp>] + // e.g. uid_tag.12345- + // See FileRotator#FileInfo for more detail. + final Pattern pattern = Pattern.compile("^" + prefix + "\\.[0-9]+-[0-9]*$"); + + // Ensure that base path exists. + statsDir.mkdirs(); + + int totalFiles = 0; + int totalBytes = 0; + for (String name : emptyIfNull(statsDir.list())) { + if (!pattern.matcher(name).matches()) continue; + + totalFiles++; + // Cast to int is safe since stats persistent files are several MBs in total. + totalBytes += (int) (new File(statsDir, name).length()); + + } + return new Pair<>(totalFiles, totalBytes); + } + + private static String [] emptyIfNull(@Nullable String [] array) { + return (array == null) ? new String[0] : array; + } + + /** + * Log statistics from the NetworkStatsRecorder file reading process into statsd. + */ + public void logRecorderFileReading(@NonNull String prefix, int readLatencyMillis, + @Nullable File statsDir, @NonNull NetworkStatsCollection collection, + boolean useFastDataInput) { + final Set<Integer> uids = new HashSet<>(); + final Map<NetworkStatsCollection.Key, NetworkStatsHistory> entries = + collection.getEntries(); + + for (final NetworkStatsCollection.Key key : entries.keySet()) { + uids.add(key.uid); + } + + int totalHistorySize = 0; + for (final NetworkStatsHistory history : entries.values()) { + totalHistorySize += history.size(); + } + + final Pair<Integer, Integer> fileAttributes = getStatsFilesAttributes(statsDir, prefix); + mDeps.writeRecorderFileReadingStats(prefixToRecorderType(prefix), + mReadIndex++, + readLatencyMillis, + fileAttributes.first /* fileCount */, + fileAttributes.second /* totalFileSize */, + entries.size(), + uids.size(), + totalHistorySize, + useFastDataInput); + } +} diff --git a/service-t/src/com/android/server/NsdService.java b/service-t/src/com/android/server/NsdService.java index 2640332b33..b7fd9a8892 100644 --- a/service-t/src/com/android/server/NsdService.java +++ b/service-t/src/com/android/server/NsdService.java @@ -26,6 +26,8 @@ import static android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK; import static android.net.nsd.NsdManager.MDNS_DISCOVERY_MANAGER_EVENT; import static android.net.nsd.NsdManager.MDNS_SERVICE_EVENT; import static android.net.nsd.NsdManager.RESOLVE_SERVICE_SUCCEEDED; +import static android.net.nsd.NsdManager.TYPE_REGEX; +import static android.net.nsd.NsdManager.TYPE_SUBTYPE_LABEL_REGEX; import static android.provider.DeviceConfig.NAMESPACE_TETHERING; import static com.android.modules.utils.build.SdkLevel.isAtLeastU; @@ -51,6 +53,7 @@ import android.net.mdns.aidl.GetAddressInfo; import android.net.mdns.aidl.IMDnsEventListener; import android.net.mdns.aidl.RegistrationInfo; import android.net.mdns.aidl.ResolutionInfo; +import android.net.nsd.AdvertisingRequest; import android.net.nsd.INsdManager; import android.net.nsd.INsdManagerCallback; import android.net.nsd.INsdServiceConnector; @@ -111,7 +114,10 @@ import java.net.SocketException; import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -170,6 +176,8 @@ public class NsdService extends INsdManager.Stub { "mdns_advertiser_allowlist_"; private static final String MDNS_ALLOWLIST_FLAG_SUFFIX = "_version"; + + @VisibleForTesting static final String MDNS_CONFIG_RUNNING_APP_ACTIVE_IMPORTANCE_CUTOFF = "mdns_config_running_app_active_importance_cutoff"; @@ -186,11 +194,13 @@ public class NsdService extends INsdManager.Stub { static final int NO_TRANSACTION = -1; private static final int NO_SENT_QUERY_COUNT = 0; private static final int DISCOVERY_QUERY_SENT_CALLBACK = 1000; + private static final int MAX_SUBTYPE_COUNT = 100; private static final SharedLog LOGGER = new SharedLog("serviceDiscovery"); private final Context mContext; private final NsdStateMachine mNsdStateMachine; - private final MDnsManager mMDnsManager; + // It can be null on V+ device since mdns native service provided by netd is removed. + private final @Nullable MDnsManager mMDnsManager; private final MDnsEventCallback mMDnsEventCallback; @NonNull private final Dependencies mDeps; @@ -536,6 +546,11 @@ public class NsdService extends INsdManager.Stub { if (DBG) Log.d(TAG, "Daemon is already started."); return; } + + if (mMDnsManager == null) { + Log.wtf(TAG, "maybeStartDaemon: mMDnsManager is null"); + return; + } mMDnsManager.registerEventListener(mMDnsEventCallback); mMDnsManager.startDaemon(); mIsDaemonStarted = true; @@ -548,6 +563,11 @@ public class NsdService extends INsdManager.Stub { if (DBG) Log.d(TAG, "Daemon has not been started."); return; } + + if (mMDnsManager == null) { + Log.wtf(TAG, "maybeStopDaemon: mMDnsManager is null"); + return; + } mMDnsManager.unregisterEventListener(mMDnsEventCallback); mMDnsManager.stopDaemon(); mIsDaemonStarted = false; @@ -688,17 +708,45 @@ public class NsdService extends INsdManager.Stub { return mClients.get(args.connector); } + /** + * Returns {@code false} if {@code subtypes} exceeds the maximum number limit or + * contains invalid subtype label. + */ + private boolean checkSubtypeLabels(Set<String> subtypes) { + if (subtypes.size() > MAX_SUBTYPE_COUNT) { + mServiceLogs.e( + "Too many subtypes: " + subtypes.size() + " (max = " + + MAX_SUBTYPE_COUNT + ")"); + return false; + } + + for (String subtype : subtypes) { + if (!checkSubtypeLabel(subtype)) { + mServiceLogs.e("Subtype " + subtype + " is invalid"); + return false; + } + } + return true; + } + + private Set<String> dedupSubtypeLabels(Collection<String> subtypes) { + final Map<String, String> subtypeMap = new LinkedHashMap<>(subtypes.size()); + for (String subtype : subtypes) { + subtypeMap.put(MdnsUtils.toDnsLowerCase(subtype), subtype); + } + return new ArraySet<>(subtypeMap.values()); + } + @Override public boolean processMessage(Message msg) { final ClientInfo clientInfo; final int transactionId; final int clientRequestId = msg.arg2; - final ListenerArgs args; final OffloadEngineInfo offloadEngineInfo; switch (msg.what) { case NsdManager.DISCOVER_SERVICES: { if (DBG) Log.d(TAG, "Discover services"); - args = (ListenerArgs) msg.obj; + final ListenerArgs args = (ListenerArgs) msg.obj; clientInfo = mClients.get(args.connector); // If the binder death notification for a INsdManagerCallback was received // before any calls are received by NsdService, the clientInfo would be @@ -716,14 +764,14 @@ public class NsdService extends INsdManager.Stub { final NsdServiceInfo info = args.serviceInfo; transactionId = getUniqueId(); - final Pair<String, String> typeAndSubtype = + final Pair<String, List<String>> typeAndSubtype = parseTypeAndSubtype(info.getServiceType()); final String serviceType = typeAndSubtype == null ? null : typeAndSubtype.first; if (clientInfo.mUseJavaBackend || mDeps.isMdnsDiscoveryManagerEnabled(mContext) || useDiscoveryManagerForType(serviceType)) { - if (serviceType == null) { + if (serviceType == null || typeAndSubtype.second.size() > 1) { clientInfo.onDiscoverServicesFailedImmediately(clientRequestId, NsdManager.FAILURE_INTERNAL_ERROR, false /* isLegacy */); break; @@ -738,10 +786,11 @@ public class NsdService extends INsdManager.Stub { .setNetwork(info.getNetwork()) .setRemoveExpiredService(true) .setIsPassiveMode(true); - if (typeAndSubtype.second != null) { + if (!typeAndSubtype.second.isEmpty()) { // The parsing ensures subtype starts with an underscore. // MdnsSearchOptions expects the underscore to not be present. - optionsBuilder.addSubtype(typeAndSubtype.second.substring(1)); + optionsBuilder.addSubtype( + typeAndSubtype.second.get(0).substring(1)); } mMdnsDiscoveryManager.registerListener( listenServiceType, listener, optionsBuilder.build()); @@ -773,7 +822,7 @@ public class NsdService extends INsdManager.Stub { } case NsdManager.STOP_DISCOVERY: { if (DBG) Log.d(TAG, "Stop service discovery"); - args = (ListenerArgs) msg.obj; + final ListenerArgs args = (ListenerArgs) msg.obj; clientInfo = mClients.get(args.connector); // If the binder death notification for a INsdManagerCallback was received // before any calls are received by NsdService, the clientInfo would be @@ -811,7 +860,7 @@ public class NsdService extends INsdManager.Stub { } case NsdManager.REGISTER_SERVICE: { if (DBG) Log.d(TAG, "Register service"); - args = (ListenerArgs) msg.obj; + final AdvertisingArgs args = (AdvertisingArgs) msg.obj; clientInfo = mClients.get(args.connector); // If the binder death notification for a INsdManagerCallback was received // before any calls are received by NsdService, the clientInfo would be @@ -826,11 +875,15 @@ public class NsdService extends INsdManager.Stub { NsdManager.FAILURE_MAX_LIMIT, true /* isLegacy */); break; } - - transactionId = getUniqueId(); - final NsdServiceInfo serviceInfo = args.serviceInfo; + final AdvertisingRequest advertisingRequest = args.advertisingRequest; + if (advertisingRequest == null) { + Log.e(TAG, "Unknown advertisingRequest in registration"); + break; + } + final NsdServiceInfo serviceInfo = advertisingRequest.getServiceInfo(); final String serviceType = serviceInfo.getServiceType(); - final Pair<String, String> typeSubtype = parseTypeAndSubtype(serviceType); + final Pair<String, List<String>> typeSubtype = parseTypeAndSubtype( + serviceType); final String registerServiceType = typeSubtype == null ? null : typeSubtype.first; if (clientInfo.mUseJavaBackend @@ -842,22 +895,53 @@ public class NsdService extends INsdManager.Stub { NsdManager.FAILURE_INTERNAL_ERROR, false /* isLegacy */); break; } + boolean isUpdateOnly = (advertisingRequest.getAdvertisingConfig() + & AdvertisingRequest.NSD_ADVERTISING_UPDATE_ONLY) > 0; + // If it is an update request, then reuse the old transactionId + if (isUpdateOnly) { + final ClientRequest existingClientRequest = + clientInfo.mClientRequests.get(clientRequestId); + if (existingClientRequest == null) { + Log.e(TAG, "Invalid update on requestId: " + clientRequestId); + clientInfo.onRegisterServiceFailedImmediately(clientRequestId, + NsdManager.FAILURE_INTERNAL_ERROR, + false /* isLegacy */); + break; + } + transactionId = existingClientRequest.mTransactionId; + } else { + transactionId = getUniqueId(); + } serviceInfo.setServiceType(registerServiceType); serviceInfo.setServiceName(truncateServiceName( serviceInfo.getServiceName())); + Set<String> subtypes = new ArraySet<>(serviceInfo.getSubtypes()); + for (String subType: typeSubtype.second) { + if (!TextUtils.isEmpty(subType)) { + subtypes.add(subType); + } + } + subtypes = dedupSubtypeLabels(subtypes); + + if (!checkSubtypeLabels(subtypes)) { + clientInfo.onRegisterServiceFailedImmediately(clientRequestId, + NsdManager.FAILURE_BAD_PARAMETERS, false /* isLegacy */); + break; + } + + serviceInfo.setSubtypes(subtypes); maybeStartMonitoringSockets(); - // TODO: pass in the subtype as well. Including the subtype in the - // service type would generate service instance names like - // Name._subtype._sub._type._tcp, which is incorrect - // (it should be Name._type._tcp). + final MdnsAdvertisingOptions mdnsAdvertisingOptions = + MdnsAdvertisingOptions.newBuilder().setIsOnlyUpdate( + isUpdateOnly).build(); mAdvertiser.addOrUpdateService(transactionId, serviceInfo, - typeSubtype.second, - MdnsAdvertisingOptions.newBuilder().build()); + mdnsAdvertisingOptions); storeAdvertiserRequestMap(clientRequestId, transactionId, clientInfo, serviceInfo.getNetwork()); } else { maybeStartDaemon(); + transactionId = getUniqueId(); if (registerService(transactionId, serviceInfo)) { if (DBG) { Log.d(TAG, "Register " + clientRequestId @@ -877,7 +961,7 @@ public class NsdService extends INsdManager.Stub { } case NsdManager.UNREGISTER_SERVICE: { if (DBG) Log.d(TAG, "unregister service"); - args = (ListenerArgs) msg.obj; + final ListenerArgs args = (ListenerArgs) msg.obj; clientInfo = mClients.get(args.connector); // If the binder death notification for a INsdManagerCallback was received // before any calls are received by NsdService, the clientInfo would be @@ -920,7 +1004,7 @@ public class NsdService extends INsdManager.Stub { } case NsdManager.RESOLVE_SERVICE: { if (DBG) Log.d(TAG, "Resolve service"); - args = (ListenerArgs) msg.obj; + final ListenerArgs args = (ListenerArgs) msg.obj; clientInfo = mClients.get(args.connector); // If the binder death notification for a INsdManagerCallback was received // before any calls are received by NsdService, the clientInfo would be @@ -932,7 +1016,7 @@ public class NsdService extends INsdManager.Stub { final NsdServiceInfo info = args.serviceInfo; transactionId = getUniqueId(); - final Pair<String, String> typeSubtype = + final Pair<String, List<String>> typeSubtype = parseTypeAndSubtype(info.getServiceType()); final String serviceType = typeSubtype == null ? null : typeSubtype.first; @@ -982,7 +1066,7 @@ public class NsdService extends INsdManager.Stub { } case NsdManager.STOP_RESOLUTION: { if (DBG) Log.d(TAG, "Stop service resolution"); - args = (ListenerArgs) msg.obj; + final ListenerArgs args = (ListenerArgs) msg.obj; clientInfo = mClients.get(args.connector); // If the binder death notification for a INsdManagerCallback was received // before any calls are received by NsdService, the clientInfo would be @@ -1021,7 +1105,7 @@ public class NsdService extends INsdManager.Stub { } case NsdManager.REGISTER_SERVICE_CALLBACK: { if (DBG) Log.d(TAG, "Register a service callback"); - args = (ListenerArgs) msg.obj; + final ListenerArgs args = (ListenerArgs) msg.obj; clientInfo = mClients.get(args.connector); // If the binder death notification for a INsdManagerCallback was received // before any calls are received by NsdService, the clientInfo would be @@ -1033,7 +1117,7 @@ public class NsdService extends INsdManager.Stub { final NsdServiceInfo info = args.serviceInfo; transactionId = getUniqueId(); - final Pair<String, String> typeAndSubtype = + final Pair<String, List<String>> typeAndSubtype = parseTypeAndSubtype(info.getServiceType()); final String serviceType = typeAndSubtype == null ? null : typeAndSubtype.first; @@ -1064,7 +1148,7 @@ public class NsdService extends INsdManager.Stub { } case NsdManager.UNREGISTER_SERVICE_CALLBACK: { if (DBG) Log.d(TAG, "Unregister a service callback"); - args = (ListenerArgs) msg.obj; + final ListenerArgs args = (ListenerArgs) msg.obj; clientInfo = mClients.get(args.connector); // If the binder death notification for a INsdManagerCallback was received // before any calls are received by NsdService, the clientInfo would be @@ -1388,6 +1472,7 @@ public class NsdService extends INsdManager.Stub { servInfo, network == null ? INetd.LOCAL_NET_ID : network.netId, serviceInfo.getInterfaceIndex()); + servInfo.setSubtypes(dedupSubtypeLabels(serviceInfo.getSubtypes())); return servInfo; } @@ -1581,34 +1666,36 @@ public class NsdService extends INsdManager.Stub { * underscore; they are alphanumerical characters or dashes or underscore, except the * last one that is just alphanumerical. The last label must be _tcp or _udp. * - * <p>The subtype may also be specified with a comma after the service type, for example - * _type._tcp,_subtype. + * <p>The subtypes may also be specified with a comma after the service type, for example + * _type._tcp,_subtype1,_subtype2 * * @param serviceType the request service type for discovery / resolution service * @return constructed service type or null if the given service type is invalid. */ @Nullable - public static Pair<String, String> parseTypeAndSubtype(String serviceType) { + public static Pair<String, List<String>> parseTypeAndSubtype(String serviceType) { if (TextUtils.isEmpty(serviceType)) return null; - - final String typeOrSubtypePattern = "_[a-zA-Z0-9-_]{1,61}[a-zA-Z0-9]"; - final Pattern serviceTypePattern = Pattern.compile( - // Optional leading subtype (_subtype._type._tcp) - // (?: xxx) is a non-capturing parenthesis, don't capture the dot - "^(?:(" + typeOrSubtypePattern + ")\\.)?" - // Actual type (_type._tcp.local) - + "(" + typeOrSubtypePattern + "\\._(?:tcp|udp))" - // Drop '.' at the end of service type that is compatible with old backend. - // e.g. allow "_type._tcp.local." - + "\\.?" - // Optional subtype after comma, for "_type._tcp,_subtype" format - + "(?:,(" + typeOrSubtypePattern + "))?" - + "$"); + final Pattern serviceTypePattern = Pattern.compile(TYPE_REGEX); final Matcher matcher = serviceTypePattern.matcher(serviceType); if (!matcher.matches()) return null; - // Use the subtype either at the beginning or after the comma - final String subtype = matcher.group(1) != null ? matcher.group(1) : matcher.group(3); - return new Pair<>(matcher.group(2), subtype); + final String queryType = matcher.group(2); + // Use the subtype at the beginning + if (matcher.group(1) != null) { + return new Pair<>(queryType, List.of(matcher.group(1))); + } + // Use the subtypes at the end + final String subTypesStr = matcher.group(3); + if (subTypesStr != null && !subTypesStr.isEmpty()) { + final String[] subTypes = subTypesStr.substring(1).split(","); + return new Pair<>(queryType, List.of(subTypes)); + } + + return new Pair<>(queryType, Collections.emptyList()); + } + + /** Returns {@code true} if {@code subtype} is a valid DNS-SD subtype label. */ + private static boolean checkSubtypeLabel(String subtype) { + return Pattern.compile("^" + TYPE_SUBTYPE_LABEL_REGEX + "$").matcher(subtype).matches(); } @VisibleForTesting @@ -1622,7 +1709,8 @@ public class NsdService extends INsdManager.Stub { mContext = ctx; mNsdStateMachine = new NsdStateMachine(TAG, handler); mNsdStateMachine.start(); - mMDnsManager = ctx.getSystemService(MDnsManager.class); + // It can fail on V+ device since mdns native service provided by netd is removed. + mMDnsManager = SdkLevel.isAtLeastV() ? null : ctx.getSystemService(MDnsManager.class); mMDnsEventCallback = new MDnsEventCallback(mNsdStateMachine); mDeps = deps; @@ -1653,6 +1741,8 @@ public class NsdService extends INsdManager.Stub { mContext, MdnsFeatureFlags.NSD_EXPIRED_SERVICES_REMOVAL)) .setIsLabelCountLimitEnabled(mDeps.isTetheringFeatureNotChickenedOut( mContext, MdnsFeatureFlags.NSD_LIMIT_LABEL_COUNT)) + .setIsKnownAnswerSuppressionEnabled(mDeps.isFeatureEnabled( + mContext, MdnsFeatureFlags.NSD_KNOWN_ANSWER_SUPPRESSION)) .build(); mMdnsSocketClient = new MdnsMultinetworkSocketClient(handler.getLooper(), mMdnsSocketProvider, @@ -2014,20 +2104,33 @@ public class NsdService extends INsdManager.Stub { } } + private static class AdvertisingArgs { + public final NsdServiceConnector connector; + public final AdvertisingRequest advertisingRequest; + + AdvertisingArgs(NsdServiceConnector connector, AdvertisingRequest advertisingRequest) { + this.connector = connector; + this.advertisingRequest = advertisingRequest; + } + } + private class NsdServiceConnector extends INsdServiceConnector.Stub implements IBinder.DeathRecipient { + @Override - public void registerService(int listenerKey, NsdServiceInfo serviceInfo) { + public void registerService(int listenerKey, AdvertisingRequest advertisingRequest) + throws RemoteException { mNsdStateMachine.sendMessage(mNsdStateMachine.obtainMessage( NsdManager.REGISTER_SERVICE, 0, listenerKey, - new ListenerArgs(this, serviceInfo))); + new AdvertisingArgs(this, advertisingRequest) + )); } @Override public void unregisterService(int listenerKey) { mNsdStateMachine.sendMessage(mNsdStateMachine.obtainMessage( NsdManager.UNREGISTER_SERVICE, 0, listenerKey, - new ListenerArgs(this, null))); + new ListenerArgs(this, (NsdServiceInfo) null))); } @Override @@ -2039,8 +2142,8 @@ public class NsdService extends INsdManager.Stub { @Override public void stopDiscovery(int listenerKey) { - mNsdStateMachine.sendMessage(mNsdStateMachine.obtainMessage( - NsdManager.STOP_DISCOVERY, 0, listenerKey, new ListenerArgs(this, null))); + mNsdStateMachine.sendMessage(mNsdStateMachine.obtainMessage(NsdManager.STOP_DISCOVERY, + 0, listenerKey, new ListenerArgs(this, (NsdServiceInfo) null))); } @Override @@ -2052,8 +2155,8 @@ public class NsdService extends INsdManager.Stub { @Override public void stopResolution(int listenerKey) { - mNsdStateMachine.sendMessage(mNsdStateMachine.obtainMessage( - NsdManager.STOP_RESOLUTION, 0, listenerKey, new ListenerArgs(this, null))); + mNsdStateMachine.sendMessage(mNsdStateMachine.obtainMessage(NsdManager.STOP_RESOLUTION, + 0, listenerKey, new ListenerArgs(this, (NsdServiceInfo) null))); } @Override @@ -2067,13 +2170,13 @@ public class NsdService extends INsdManager.Stub { public void unregisterServiceInfoCallback(int listenerKey) { mNsdStateMachine.sendMessage(mNsdStateMachine.obtainMessage( NsdManager.UNREGISTER_SERVICE_CALLBACK, 0, listenerKey, - new ListenerArgs(this, null))); + new ListenerArgs(this, (NsdServiceInfo) null))); } @Override public void startDaemon() { - mNsdStateMachine.sendMessage(mNsdStateMachine.obtainMessage( - NsdManager.DAEMON_STARTUP, new ListenerArgs(this, null))); + mNsdStateMachine.sendMessage(mNsdStateMachine.obtainMessage(NsdManager.DAEMON_STARTUP, + new ListenerArgs(this, (NsdServiceInfo) null))); } @Override @@ -2109,25 +2212,24 @@ public class NsdService extends INsdManager.Stub { throw new SecurityException("API is not available in before API level 33"); } - // REGISTER_NSD_OFFLOAD_ENGINE was only added to the SDK in V. - if (SdkLevel.isAtLeastV() && PermissionUtils.checkAnyPermissionOf(context, - REGISTER_NSD_OFFLOAD_ENGINE)) { - return; - } + final ArrayList<String> permissionsList = new ArrayList<>(Arrays.asList(NETWORK_STACK, + PERMISSION_MAINLINE_NETWORK_STACK, NETWORK_SETTINGS)); - // REGISTER_NSD_OFFLOAD_ENGINE cannot be backport to U. In U, check the DEVICE_POWER - // permission instead. - if (!SdkLevel.isAtLeastV() && SdkLevel.isAtLeastU() - && PermissionUtils.checkAnyPermissionOf(context, DEVICE_POWER)) { - return; + if (SdkLevel.isAtLeastV()) { + // REGISTER_NSD_OFFLOAD_ENGINE was only added to the SDK in V. + permissionsList.add(REGISTER_NSD_OFFLOAD_ENGINE); + } else if (SdkLevel.isAtLeastU()) { + // REGISTER_NSD_OFFLOAD_ENGINE cannot be backport to U. In U, check the DEVICE_POWER + // permission instead. + permissionsList.add(DEVICE_POWER); } - if (PermissionUtils.checkAnyPermissionOf(context, NETWORK_STACK, - PERMISSION_MAINLINE_NETWORK_STACK, NETWORK_SETTINGS)) { + + if (PermissionUtils.checkAnyPermissionOf(context, + permissionsList.toArray(new String[0]))) { return; } throw new SecurityException("Requires one of the following permissions: " - + String.join(", ", List.of(REGISTER_NSD_OFFLOAD_ENGINE, NETWORK_STACK, - PERMISSION_MAINLINE_NETWORK_STACK, NETWORK_SETTINGS)) + "."); + + String.join(", ", permissionsList) + "."); } } @@ -2145,6 +2247,11 @@ public class NsdService extends INsdManager.Stub { } private boolean registerService(int transactionId, NsdServiceInfo service) { + if (mMDnsManager == null) { + Log.wtf(TAG, "registerService: mMDnsManager is null"); + return false; + } + if (DBG) { Log.d(TAG, "registerService: " + transactionId + " " + service); } @@ -2162,10 +2269,19 @@ public class NsdService extends INsdManager.Stub { } private boolean unregisterService(int transactionId) { + if (mMDnsManager == null) { + Log.wtf(TAG, "unregisterService: mMDnsManager is null"); + return false; + } return mMDnsManager.stopOperation(transactionId); } private boolean discoverServices(int transactionId, NsdServiceInfo serviceInfo) { + if (mMDnsManager == null) { + Log.wtf(TAG, "discoverServices: mMDnsManager is null"); + return false; + } + final String type = serviceInfo.getServiceType(); final int discoverInterface = getNetworkInterfaceIndex(serviceInfo); if (serviceInfo.getNetwork() != null && discoverInterface == IFACE_IDX_ANY) { @@ -2176,10 +2292,18 @@ public class NsdService extends INsdManager.Stub { } private boolean stopServiceDiscovery(int transactionId) { + if (mMDnsManager == null) { + Log.wtf(TAG, "stopServiceDiscovery: mMDnsManager is null"); + return false; + } return mMDnsManager.stopOperation(transactionId); } private boolean resolveService(int transactionId, NsdServiceInfo service) { + if (mMDnsManager == null) { + Log.wtf(TAG, "resolveService: mMDnsManager is null"); + return false; + } final String name = service.getServiceName(); final String type = service.getServiceType(); final int resolveInterface = getNetworkInterfaceIndex(service); @@ -2253,14 +2377,26 @@ public class NsdService extends INsdManager.Stub { } private boolean stopResolveService(int transactionId) { + if (mMDnsManager == null) { + Log.wtf(TAG, "stopResolveService: mMDnsManager is null"); + return false; + } return mMDnsManager.stopOperation(transactionId); } private boolean getAddrInfo(int transactionId, String hostname, int interfaceIdx) { + if (mMDnsManager == null) { + Log.wtf(TAG, "getAddrInfo: mMDnsManager is null"); + return false; + } return mMDnsManager.getServiceAddress(transactionId, hostname, interfaceIdx); } private boolean stopGetAddrInfo(int transactionId) { + if (mMDnsManager == null) { + Log.wtf(TAG, "stopGetAddrInfo: mMDnsManager is null"); + return false; + } return mMDnsManager.stopOperation(transactionId); } diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsAdvertiser.java b/service-t/src/com/android/server/connectivity/mdns/MdnsAdvertiser.java index fc0e11b025..135d957948 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsAdvertiser.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsAdvertiser.java @@ -44,6 +44,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.UUID; import java.util.function.BiPredicate; import java.util.function.Consumer; @@ -351,8 +352,7 @@ public class MdnsAdvertiser { mPendingRegistrations.put(id, registration); for (int i = 0; i < mAdvertisers.size(); i++) { try { - mAdvertisers.valueAt(i).addService(id, registration.getServiceInfo(), - registration.getSubtype()); + mAdvertisers.valueAt(i).addService(id, registration.getServiceInfo()); } catch (NameConflictException e) { mSharedLog.wtf("Name conflict adding services that should have unique names", e); @@ -367,7 +367,8 @@ public class MdnsAdvertiser { void updateService(int id, @NonNull Registration registration) { mPendingRegistrations.put(id, registration); for (int i = 0; i < mAdvertisers.size(); i++) { - mAdvertisers.valueAt(i).updateService(id, registration.getSubtype()); + mAdvertisers.valueAt(i).updateService( + id, registration.getServiceInfo().getSubtypes()); } } @@ -417,7 +418,7 @@ public class MdnsAdvertiser { final Registration registration = mPendingRegistrations.valueAt(i); try { advertiser.addService(mPendingRegistrations.keyAt(i), - registration.getServiceInfo(), registration.getSubtype()); + registration.getServiceInfo()); } catch (NameConflictException e) { mSharedLog.wtf("Name conflict adding services that should have unique names", e); @@ -485,16 +486,12 @@ public class MdnsAdvertiser { private int mConflictCount; @NonNull private NsdServiceInfo mServiceInfo; - @Nullable - private String mSubtype; - int mConflictDuringProbingCount; int mConflictAfterProbingCount; - private Registration(@NonNull NsdServiceInfo serviceInfo, @Nullable String subtype) { + private Registration(@NonNull NsdServiceInfo serviceInfo) { this.mOriginalName = serviceInfo.getServiceName(); this.mServiceInfo = serviceInfo; - this.mSubtype = subtype; } /** @@ -507,10 +504,11 @@ public class MdnsAdvertiser { } /** - * Update subType for the registration. + * Update subTypes for the registration. */ - public void updateSubtype(@Nullable String subtype) { - this.mSubtype = subtype; + public void updateSubtypes(@NonNull Set<String> subtypes) { + mServiceInfo = new NsdServiceInfo(mServiceInfo); + mServiceInfo.setSubtypes(subtypes); } /** @@ -540,17 +538,8 @@ public class MdnsAdvertiser { // In case of conflict choose a different service name. After the first conflict use // "Name (2)", then "Name (3)" etc. // TODO: use a hidden method in NsdServiceInfo once MdnsAdvertiser is moved to service-t - final NsdServiceInfo newInfo = new NsdServiceInfo(); + final NsdServiceInfo newInfo = new NsdServiceInfo(mServiceInfo); newInfo.setServiceName(getUpdatedServiceName(renameCount)); - newInfo.setServiceType(mServiceInfo.getServiceType()); - for (Map.Entry<String, byte[]> attr : mServiceInfo.getAttributes().entrySet()) { - newInfo.setAttribute(attr.getKey(), - attr.getValue() == null ? null : new String(attr.getValue())); - } - newInfo.setHost(mServiceInfo.getHost()); - newInfo.setPort(mServiceInfo.getPort()); - newInfo.setNetwork(mServiceInfo.getNetwork()); - // interfaceIndex is not set when registering return newInfo; } @@ -565,11 +554,6 @@ public class MdnsAdvertiser { public NsdServiceInfo getServiceInfo() { return mServiceInfo; } - - @Nullable - public String getSubtype() { - return mSubtype; - } } /** @@ -665,14 +649,14 @@ public class MdnsAdvertiser { * * @param id A unique ID for the service. * @param service The service info to advertise. - * @param subtype An optional subtype to advertise the service with. * @param advertisingOptions The advertising options. */ - public void addOrUpdateService(int id, NsdServiceInfo service, @Nullable String subtype, + public void addOrUpdateService(int id, NsdServiceInfo service, MdnsAdvertisingOptions advertisingOptions) { checkThread(); final Registration existingRegistration = mRegistrations.get(id); final Network network = service.getNetwork(); + final Set<String> subtypes = service.getSubtypes(); Registration registration; if (advertisingOptions.isOnlyUpdate()) { if (existingRegistration == null) { @@ -687,10 +671,10 @@ public class MdnsAdvertiser { return; } - mSharedLog.i("Update service " + service + " with ID " + id + " and subtype " + subtype - + " advertisingOptions " + advertisingOptions); + mSharedLog.i("Update service " + service + " with ID " + id + " and subtypes " + + subtypes + " advertisingOptions " + advertisingOptions); registration = existingRegistration; - registration.updateSubtype(subtype); + registration.updateSubtypes(subtypes); } else { if (existingRegistration != null) { mSharedLog.e("Adding duplicate registration for " + service); @@ -698,9 +682,9 @@ public class MdnsAdvertiser { mCb.onRegisterServiceFailed(id, NsdManager.FAILURE_INTERNAL_ERROR); return; } - mSharedLog.i("Adding service " + service + " with ID " + id + " and subtype " + subtype - + " advertisingOptions " + advertisingOptions); - registration = new Registration(service, subtype); + mSharedLog.i("Adding service " + service + " with ID " + id + " and subtypes " + + subtypes + " advertisingOptions " + advertisingOptions); + registration = new Registration(service); final BiPredicate<Network, InterfaceAdvertiserRequest> checkConflictFilter; if (network == null) { // If registering on all networks, no advertiser must have conflicts @@ -793,15 +777,10 @@ public class MdnsAdvertiser { private OffloadServiceInfoWrapper createOffloadService(int serviceId, @NonNull Registration registration, byte[] rawOffloadPacket) { final NsdServiceInfo nsdServiceInfo = registration.getServiceInfo(); - final List<String> subTypes = new ArrayList<>(); - String subType = registration.getSubtype(); - if (subType != null) { - subTypes.add(subType); - } final OffloadServiceInfo offloadServiceInfo = new OffloadServiceInfo( new OffloadServiceInfo.Key(nsdServiceInfo.getServiceName(), nsdServiceInfo.getServiceType()), - subTypes, + new ArrayList<>(nsdServiceInfo.getSubtypes()), String.join(".", mDeviceHostName), rawOffloadPacket, // TODO: define overlayable resources in diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsFeatureFlags.java b/service-t/src/com/android/server/connectivity/mdns/MdnsFeatureFlags.java index 0a6d8c130b..1ad47a30b7 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsFeatureFlags.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsFeatureFlags.java @@ -41,6 +41,11 @@ public class MdnsFeatureFlags { */ public static final String NSD_LIMIT_LABEL_COUNT = "nsd_limit_label_count"; + /** + * A feature flag to control whether the known-answer suppression should be enabled. + */ + public static final String NSD_KNOWN_ANSWER_SUPPRESSION = "nsd_known_answer_suppression"; + // Flag for offload feature public final boolean mIsMdnsOffloadFeatureEnabled; @@ -53,17 +58,22 @@ public class MdnsFeatureFlags { // Flag for label count limit public final boolean mIsLabelCountLimitEnabled; + // Flag for known-answer suppression + public final boolean mIsKnownAnswerSuppressionEnabled; + /** * The constructor for {@link MdnsFeatureFlags}. */ public MdnsFeatureFlags(boolean isOffloadFeatureEnabled, boolean includeInetAddressRecordsInProbing, boolean isExpiredServicesRemovalEnabled, - boolean isLabelCountLimitEnabled) { + boolean isLabelCountLimitEnabled, + boolean isKnownAnswerSuppressionEnabled) { mIsMdnsOffloadFeatureEnabled = isOffloadFeatureEnabled; mIncludeInetAddressRecordsInProbing = includeInetAddressRecordsInProbing; mIsExpiredServicesRemovalEnabled = isExpiredServicesRemovalEnabled; mIsLabelCountLimitEnabled = isLabelCountLimitEnabled; + mIsKnownAnswerSuppressionEnabled = isKnownAnswerSuppressionEnabled; } @@ -79,6 +89,7 @@ public class MdnsFeatureFlags { private boolean mIncludeInetAddressRecordsInProbing; private boolean mIsExpiredServicesRemovalEnabled; private boolean mIsLabelCountLimitEnabled; + private boolean mIsKnownAnswerSuppressionEnabled; /** * The constructor for {@link Builder}. @@ -88,6 +99,7 @@ public class MdnsFeatureFlags { mIncludeInetAddressRecordsInProbing = false; mIsExpiredServicesRemovalEnabled = false; mIsLabelCountLimitEnabled = true; // Default enabled. + mIsKnownAnswerSuppressionEnabled = false; } /** @@ -132,13 +144,24 @@ public class MdnsFeatureFlags { } /** + * Set whether the known-answer suppression is enabled. + * + * @see #NSD_KNOWN_ANSWER_SUPPRESSION + */ + public Builder setIsKnownAnswerSuppressionEnabled(boolean isKnownAnswerSuppressionEnabled) { + mIsKnownAnswerSuppressionEnabled = isKnownAnswerSuppressionEnabled; + return this; + } + + /** * Builds a {@link MdnsFeatureFlags} with the arguments supplied to this builder. */ public MdnsFeatureFlags build() { return new MdnsFeatureFlags(mIsMdnsOffloadFeatureEnabled, mIncludeInetAddressRecordsInProbing, mIsExpiredServicesRemovalEnabled, - mIsLabelCountLimitEnabled); + mIsLabelCountLimitEnabled, + mIsKnownAnswerSuppressionEnabled); } } } diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiser.java b/service-t/src/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiser.java index 463df63914..aa40c9227d 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiser.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiser.java @@ -37,6 +37,7 @@ import com.android.server.connectivity.mdns.util.MdnsUtils; import java.io.IOException; import java.net.InetSocketAddress; import java.util.List; +import java.util.Set; /** * A class that handles advertising services on a {@link MdnsInterfaceSocket} tied to an interface. @@ -232,12 +233,12 @@ public class MdnsInterfaceAdvertiser implements MulticastPacketReader.PacketHand * Update an already registered service without sending exit/re-announcement packet. * * @param id An exiting service id - * @param subtype A new subtype + * @param subtypes New subtypes */ - public void updateService(int id, @Nullable String subtype) { + public void updateService(int id, @NonNull Set<String> subtypes) { // The current implementation is intended to be used in cases where subtypes don't get // announced. - mRecordRepository.updateService(id, subtype); + mRecordRepository.updateService(id, subtypes); } /** @@ -245,9 +246,8 @@ public class MdnsInterfaceAdvertiser implements MulticastPacketReader.PacketHand * * @throws NameConflictException There is already a service being advertised with that name. */ - public void addService(int id, NsdServiceInfo service, @Nullable String subtype) - throws NameConflictException { - final int replacedExitingService = mRecordRepository.addService(id, service, subtype); + public void addService(int id, NsdServiceInfo service) throws NameConflictException { + final int replacedExitingService = mRecordRepository.addService(id, service); // Cancel announcements for the existing service. This only happens for exiting services // (so cancelling exiting announcements), as per RecordRepository.addService. if (replacedExitingService >= 0) { diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java b/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java index 48ece68311..6b6632cd6e 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java @@ -46,6 +46,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.Enumeration; import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -74,8 +75,6 @@ public class MdnsRecordRepository { // Top-level domain for link-local queries, as per RFC6762 3. private static final String LOCAL_TLD = "local"; - // Subtype separator as per RFC6763 7.1 (_printer._sub._http._tcp.local) - private static final String SUBTYPE_SEPARATOR = "_sub"; // Service type for service enumeration (RFC6763 9.) private static final String[] DNS_SD_SERVICE_TYPE = @@ -92,6 +91,7 @@ public class MdnsRecordRepository { private final Looper mLooper; @NonNull private final String[] mDeviceHostname; + @NonNull private final MdnsFeatureFlags mMdnsFeatureFlags; public MdnsRecordRepository(@NonNull Looper looper, @NonNull String[] deviceHostname, @@ -141,6 +141,9 @@ public class MdnsRecordRepository { * Last time (as per SystemClock.elapsedRealtime) when sent via unicast or multicast, * 0 if never */ + // FIXME: the `lastSentTimeMs` and `lastAdvertisedTimeMs` should be maintained separately + // for IPv4 and IPv6, because neither IPv4 nor and IPv6 clients can receive replies in + // different address space. public long lastSentTimeMs; RecordInfo(NsdServiceInfo serviceInfo, T record, boolean sharedName) { @@ -161,8 +164,6 @@ public class MdnsRecordRepository { public final RecordInfo<MdnsTextRecord> txtRecord; @NonNull public final NsdServiceInfo serviceInfo; - @Nullable - public final String subtype; /** * Whether the service is sending exit announcements and will be destroyed soon. @@ -185,28 +186,28 @@ public class MdnsRecordRepository { private boolean isProbing; /** - * Create a ServiceRegistration with only update the subType + * Create a ServiceRegistration with only update the subType. */ - ServiceRegistration withSubtype(String newSubType) { - return new ServiceRegistration(srvRecord.record.getServiceHost(), serviceInfo, - newSubType, repliedServiceCount, sentPacketCount, exiting, isProbing); + ServiceRegistration withSubtypes(@NonNull Set<String> newSubtypes) { + NsdServiceInfo newServiceInfo = new NsdServiceInfo(serviceInfo); + newServiceInfo.setSubtypes(newSubtypes); + return new ServiceRegistration(srvRecord.record.getServiceHost(), newServiceInfo, + repliedServiceCount, sentPacketCount, exiting, isProbing); } - /** * Create a ServiceRegistration for dns-sd service registration (RFC6763). */ ServiceRegistration(@NonNull String[] deviceHostname, @NonNull NsdServiceInfo serviceInfo, - @Nullable String subtype, int repliedServiceCount, int sentPacketCount, - boolean exiting, boolean isProbing) { + int repliedServiceCount, int sentPacketCount, boolean exiting, boolean isProbing) { this.serviceInfo = serviceInfo; - this.subtype = subtype; final String[] serviceType = splitServiceType(serviceInfo); final String[] serviceName = splitFullyQualifiedName(serviceInfo, serviceType); - // Service PTR record - final RecordInfo<MdnsPointerRecord> ptrRecord = new RecordInfo<>( + // Service PTR records + ptrRecords = new ArrayList<>(serviceInfo.getSubtypes().size() + 1); + ptrRecords.add(new RecordInfo<>( serviceInfo, new MdnsPointerRecord( serviceType, @@ -214,26 +215,17 @@ public class MdnsRecordRepository { false /* cacheFlush */, NON_NAME_RECORDS_TTL_MILLIS, serviceName), - true /* sharedName */); - - if (subtype == null) { - this.ptrRecords = Collections.singletonList(ptrRecord); - } else { - final String[] subtypeName = new String[serviceType.length + 2]; - System.arraycopy(serviceType, 0, subtypeName, 2, serviceType.length); - subtypeName[0] = subtype; - subtypeName[1] = SUBTYPE_SEPARATOR; - final RecordInfo<MdnsPointerRecord> subtypeRecord = new RecordInfo<>( - serviceInfo, - new MdnsPointerRecord( - subtypeName, - 0L /* receiptTimeMillis */, - false /* cacheFlush */, - NON_NAME_RECORDS_TTL_MILLIS, - serviceName), - true /* sharedName */); - - this.ptrRecords = List.of(ptrRecord, subtypeRecord); + true /* sharedName */)); + for (String subtype : serviceInfo.getSubtypes()) { + ptrRecords.add(new RecordInfo<>( + serviceInfo, + new MdnsPointerRecord( + MdnsUtils.constructFullSubtype(serviceType, subtype), + 0L /* receiptTimeMillis */, + false /* cacheFlush */, + NON_NAME_RECORDS_TTL_MILLIS, + serviceName), + true /* sharedName */)); } srvRecord = new RecordInfo<>( @@ -284,8 +276,8 @@ public class MdnsRecordRepository { * @param serviceInfo Service to advertise */ ServiceRegistration(@NonNull String[] deviceHostname, @NonNull NsdServiceInfo serviceInfo, - @Nullable String subtype, int repliedServiceCount, int sentPacketCount) { - this(deviceHostname, serviceInfo, subtype, repliedServiceCount, sentPacketCount, + int repliedServiceCount, int sentPacketCount) { + this(deviceHostname, serviceInfo,repliedServiceCount, sentPacketCount, false /* exiting */, true /* isProbing */); } @@ -328,17 +320,16 @@ public class MdnsRecordRepository { * Update a service that already registered in the repository. * * @param serviceId An existing service ID. - * @param subtype A new subtype - * @return + * @param subtypes New subtypes */ - public void updateService(int serviceId, @Nullable String subtype) { + public void updateService(int serviceId, @NonNull Set<String> subtypes) { final ServiceRegistration existingRegistration = mServices.get(serviceId); if (existingRegistration == null) { throw new IllegalArgumentException( "Service ID must already exist for an update request: " + serviceId); } - final ServiceRegistration updatedRegistration = existingRegistration.withSubtype( - subtype); + final ServiceRegistration updatedRegistration = existingRegistration.withSubtypes( + subtypes); mServices.put(serviceId, updatedRegistration); } @@ -352,8 +343,7 @@ public class MdnsRecordRepository { * ID of the replaced service. * @throws NameConflictException There is already a (non-exiting) service using the name. */ - public int addService(int serviceId, NsdServiceInfo serviceInfo, @Nullable String subtype) - throws NameConflictException { + public int addService(int serviceId, NsdServiceInfo serviceInfo) throws NameConflictException { if (mServices.contains(serviceId)) { throw new IllegalArgumentException( "Service ID must not be reused across registrations: " + serviceId); @@ -366,7 +356,7 @@ public class MdnsRecordRepository { } final ServiceRegistration registration = new ServiceRegistration( - mDeviceHostname, serviceInfo, subtype, NO_PACKET /* repliedServiceCount */, + mDeviceHostname, serviceInfo, NO_PACKET /* repliedServiceCount */, NO_PACKET /* sentPacketCount */); mServices.put(serviceId, registration); @@ -510,13 +500,17 @@ public class MdnsRecordRepository { public MdnsReplyInfo getReply(MdnsPacket packet, InetSocketAddress src) { final long now = SystemClock.elapsedRealtime(); final boolean replyUnicast = (packet.flags & MdnsConstants.QCLASS_UNICAST) != 0; - final ArrayList<MdnsRecord> additionalAnswerRecords = new ArrayList<>(); - final ArrayList<RecordInfo<?>> answerInfo = new ArrayList<>(); + + // Use LinkedHashSet for preserving the insert order of the RRs, so that RRs of the same + // service or host are grouped together (which is more developer-friendly). + final Set<RecordInfo<?>> answerInfo = new LinkedHashSet<>(); + final Set<RecordInfo<?>> additionalAnswerInfo = new LinkedHashSet<>(); + for (MdnsRecord question : packet.questions) { // Add answers from general records addReplyFromService(question, mGeneralRecords, null /* servicePtrRecord */, null /* serviceSrvRecord */, null /* serviceTxtRecord */, replyUnicast, now, - answerInfo, additionalAnswerRecords); + answerInfo, additionalAnswerInfo, Collections.emptyList()); // Add answers from each service for (int i = 0; i < mServices.size(); i++) { @@ -524,13 +518,33 @@ public class MdnsRecordRepository { if (registration.exiting || registration.isProbing) continue; if (addReplyFromService(question, registration.allRecords, registration.ptrRecords, registration.srvRecord, registration.txtRecord, replyUnicast, now, - answerInfo, additionalAnswerRecords)) { + answerInfo, additionalAnswerInfo, packet.answers)) { registration.repliedServiceCount++; registration.sentPacketCount++; } } } + // If any record was already in the answer section, remove it from the additional answer + // section. This can typically happen when there are both queries for + // SRV / TXT / A / AAAA and PTR (which can cause SRV / TXT / A / AAAA records being added + // to the additional answer section). + additionalAnswerInfo.removeAll(answerInfo); + + final List<MdnsRecord> additionalAnswerRecords = + new ArrayList<>(additionalAnswerInfo.size()); + for (RecordInfo<?> info : additionalAnswerInfo) { + additionalAnswerRecords.add(info.record); + } + + // RFC6762 6.1: negative responses + // "On receipt of a question for a particular name, rrtype, and rrclass, for which a + // responder does have one or more unique answers, the responder MAY also include an NSEC + // record in the Additional Record Section indicating the nonexistence of other rrtypes + // for that name and rrclass." + addNsecRecordsForUniqueNames(additionalAnswerRecords, + answerInfo.iterator(), additionalAnswerInfo.iterator()); + if (answerInfo.size() == 0 && additionalAnswerRecords.size() == 0) { return null; } @@ -577,6 +591,15 @@ public class MdnsRecordRepository { return new MdnsReplyInfo(answerRecords, additionalAnswerRecords, delayMs, dest); } + private boolean isKnownAnswer(MdnsRecord answer, @NonNull List<MdnsRecord> knownAnswerRecords) { + for (MdnsRecord knownAnswer : knownAnswerRecords) { + if (answer.equals(knownAnswer) && knownAnswer.getTtl() > (answer.getTtl() / 2)) { + return true; + } + } + return false; + } + /** * Add answers and additional answers for a question, from a ServiceRegistration. */ @@ -585,14 +608,15 @@ public class MdnsRecordRepository { @Nullable List<RecordInfo<MdnsPointerRecord>> servicePtrRecords, @Nullable RecordInfo<MdnsServiceRecord> serviceSrvRecord, @Nullable RecordInfo<MdnsTextRecord> serviceTxtRecord, - boolean replyUnicast, long now, @NonNull List<RecordInfo<?>> answerInfo, - @NonNull List<MdnsRecord> additionalAnswerRecords) { + boolean replyUnicast, long now, @NonNull Set<RecordInfo<?>> answerInfo, + @NonNull Set<RecordInfo<?>> additionalAnswerInfo, + @NonNull List<MdnsRecord> knownAnswerRecords) { boolean hasDnsSdPtrRecordAnswer = false; boolean hasDnsSdSrvRecordAnswer = false; boolean hasFullyOwnedNameMatch = false; boolean hasKnownAnswer = false; - final int answersStartIndex = answerInfo.size(); + final int answersStartSize = answerInfo.size(); for (RecordInfo<?> info : serviceRecords) { /* RFC6762 6.: the record name must match the question name, the record rrtype @@ -615,6 +639,20 @@ public class MdnsRecordRepository { } hasKnownAnswer = true; + + // RFC6762 7.1. Known-Answer Suppression: + // A Multicast DNS responder MUST NOT answer a Multicast DNS query if + // the answer it would give is already included in the Answer Section + // with an RR TTL at least half the correct value. If the RR TTL of the + // answer as given in the Answer Section is less than half of the true + // RR TTL as known by the Multicast DNS responder, the responder MUST + // send an answer so as to update the querier's cache before the record + // becomes in danger of expiration. + if (mMdnsFeatureFlags.mIsKnownAnswerSuppressionEnabled + && isKnownAnswer(info.record, knownAnswerRecords)) { + continue; + } + hasDnsSdPtrRecordAnswer |= (servicePtrRecords != null && CollectionUtils.any(servicePtrRecords, r -> info == r)); hasDnsSdSrvRecordAnswer |= (info == serviceSrvRecord); @@ -626,8 +664,6 @@ public class MdnsRecordRepository { continue; } - // TODO: Don't reply if in known answers of the querier (7.1) if TTL is > half - answerInfo.add(info); } @@ -636,7 +672,7 @@ public class MdnsRecordRepository { // ownership, for a type for which that name has no records, the responder MUST [...] // respond asserting the nonexistence of that record" if (hasFullyOwnedNameMatch && !hasKnownAnswer) { - additionalAnswerRecords.add(new MdnsNsecRecord( + MdnsNsecRecord nsecRecord = new MdnsNsecRecord( question.getName(), 0L /* receiptTimeMillis */, true /* cacheFlush */, @@ -644,13 +680,14 @@ public class MdnsRecordRepository { // be the same as the TTL that the record would have had, had it existed." NAME_RECORDS_TTL_MILLIS, question.getName(), - new int[] { question.getType() })); + new int[] { question.getType() }); + additionalAnswerInfo.add( + new RecordInfo<>(null /* serviceInfo */, nsecRecord, false /* isSharedName */)); } // No more records to add if no answer - if (answerInfo.size() == answersStartIndex) return false; + if (answerInfo.size() == answersStartSize) return false; - final List<RecordInfo<?>> additionalAnswerInfo = new ArrayList<>(); // RFC6763 12.1: if including PTR record, include the SRV and TXT records it names if (hasDnsSdPtrRecordAnswer) { if (serviceTxtRecord != null) { @@ -669,15 +706,6 @@ public class MdnsRecordRepository { } } } - - for (RecordInfo<?> info : additionalAnswerInfo) { - additionalAnswerRecords.add(info.record); - } - - // RFC6762 6.1: negative responses - addNsecRecordsForUniqueNames(additionalAnswerRecords, - answerInfo.listIterator(answersStartIndex), - additionalAnswerInfo.listIterator()); return true; } @@ -694,7 +722,7 @@ public class MdnsRecordRepository { * answer and additionalAnswer sections) */ @SafeVarargs - private static void addNsecRecordsForUniqueNames( + private void addNsecRecordsForUniqueNames( List<MdnsRecord> destinationList, Iterator<RecordInfo<?>>... answerRecords) { // Group unique records by name. Use a TreeMap with comparator as arrays don't implement @@ -710,6 +738,12 @@ public class MdnsRecordRepository { for (String[] nsecName : namesInAddedOrder) { final List<MdnsRecord> entryRecords = nsecByName.get(nsecName); + + // Add NSEC records only when the answers include all unique records of this name + if (entryRecords.size() != countUniqueRecords(nsecName)) { + continue; + } + long minTtl = Long.MAX_VALUE; final Set<Integer> types = new ArraySet<>(entryRecords.size()); for (MdnsRecord record : entryRecords) { @@ -727,6 +761,27 @@ public class MdnsRecordRepository { } } + /** Returns the number of unique records on this device for a given {@code name}. */ + private int countUniqueRecords(String[] name) { + int cnt = countUniqueRecords(mGeneralRecords, name); + + for (int i = 0; i < mServices.size(); i++) { + final ServiceRegistration registration = mServices.valueAt(i); + cnt += countUniqueRecords(registration.allRecords, name); + } + return cnt; + } + + private static int countUniqueRecords(List<RecordInfo<?>> records, String[] name) { + int cnt = 0; + for (RecordInfo<?> record : records) { + if (!record.isSharedName && Arrays.equals(name, record.record.getName())) { + cnt++; + } + } + return cnt; + } + /** * Add non-shared records to a map listing them by record name, and to a list of names that * remembers the adding order. @@ -741,10 +796,10 @@ public class MdnsRecordRepository { private static void addNonSharedRecordsToMap( Iterator<RecordInfo<?>> records, Map<String[], List<MdnsRecord>> dest, - List<String[]> namesInAddedOrder) { + @Nullable List<String[]> namesInAddedOrder) { while (records.hasNext()) { final RecordInfo<?> record = records.next(); - if (record.isSharedName) continue; + if (record.isSharedName || record.record instanceof MdnsNsecRecord) continue; final List<MdnsRecord> recordsForName = dest.computeIfAbsent(record.record.name, key -> { namesInAddedOrder.add(key); @@ -929,7 +984,7 @@ public class MdnsRecordRepository { if (existing == null) return null; final ServiceRegistration newService = new ServiceRegistration(mDeviceHostname, newInfo, - existing.subtype, existing.repliedServiceCount, existing.sentPacketCount); + existing.repliedServiceCount, existing.sentPacketCount); mServices.put(serviceId, newService); return makeProbingInfo( serviceId, newService.srvRecord.record, makeProbingInetAddressRecords()); diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsReplySender.java b/service-t/src/com/android/server/connectivity/mdns/MdnsReplySender.java index ea3af5e83b..651b643a3c 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsReplySender.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsReplySender.java @@ -25,6 +25,7 @@ import android.os.Handler; import android.os.Looper; import android.os.Message; +import com.android.internal.annotations.VisibleForTesting; import com.android.net.module.util.SharedLog; import com.android.server.connectivity.mdns.util.MdnsUtils; @@ -57,15 +58,46 @@ public class MdnsReplySender { @NonNull private final SharedLog mSharedLog; private final boolean mEnableDebugLog; + @NonNull + private final Dependencies mDependencies; + + /** + * Dependencies of MdnsReplySender, for injection in tests. + */ + @VisibleForTesting + public static class Dependencies { + /** + * @see Handler#sendMessageDelayed(Message, long) + */ + public void sendMessageDelayed(@NonNull Handler handler, @NonNull Message message, + long delayMillis) { + handler.sendMessageDelayed(message, delayMillis); + } + + /** + * @see Handler#removeMessages(int) + */ + public void removeMessages(@NonNull Handler handler, int what) { + handler.removeMessages(what); + } + } public MdnsReplySender(@NonNull Looper looper, @NonNull MdnsInterfaceSocket socket, @NonNull byte[] packetCreationBuffer, @NonNull SharedLog sharedLog, boolean enableDebugLog) { + this(looper, socket, packetCreationBuffer, sharedLog, enableDebugLog, new Dependencies()); + } + + @VisibleForTesting + public MdnsReplySender(@NonNull Looper looper, @NonNull MdnsInterfaceSocket socket, + @NonNull byte[] packetCreationBuffer, @NonNull SharedLog sharedLog, + boolean enableDebugLog, @NonNull Dependencies dependencies) { mHandler = new SendHandler(looper); mSocket = socket; mPacketCreationBuffer = packetCreationBuffer; mSharedLog = sharedLog; mEnableDebugLog = enableDebugLog; + mDependencies = dependencies; } /** @@ -74,7 +106,8 @@ public class MdnsReplySender { public void queueReply(@NonNull MdnsReplyInfo reply) { ensureRunningOnHandlerThread(mHandler); // TODO: implement response aggregation (RFC 6762 6.4) - mHandler.sendMessageDelayed(mHandler.obtainMessage(MSG_SEND, reply), reply.sendDelayMs); + mDependencies.sendMessageDelayed( + mHandler, mHandler.obtainMessage(MSG_SEND, reply), reply.sendDelayMs); if (mEnableDebugLog) { mSharedLog.v("Scheduling " + reply); @@ -104,7 +137,7 @@ public class MdnsReplySender { */ public void cancelAll() { ensureRunningOnHandlerThread(mHandler); - mHandler.removeMessages(MSG_SEND); + mDependencies.removeMessages(mHandler, MSG_SEND); } private class SendHandler extends Handler { diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java b/service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java index 32f604e8d5..df0a04042e 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java @@ -541,6 +541,9 @@ public class MdnsServiceTypeClient { } if (response.isComplete()) { + // There is a bug here: the newServiceFound is global right now. The state needs + // to be per listener because of the responseMatchesOptions() filter. + // Otherwise, it won't handle the subType update properly. if (newServiceFound || serviceBecomesComplete) { sharedLog.log("onServiceFound: " + serviceInfo); listener.onServiceFound(serviceInfo, false /* isServiceFromCache */); diff --git a/service-t/src/com/android/server/connectivity/mdns/util/MdnsUtils.java b/service-t/src/com/android/server/connectivity/mdns/util/MdnsUtils.java index 1482ebb7a0..8fc81148a4 100644 --- a/service-t/src/com/android/server/connectivity/mdns/util/MdnsUtils.java +++ b/service-t/src/com/android/server/connectivity/mdns/util/MdnsUtils.java @@ -233,6 +233,20 @@ public class MdnsUtils { && mdnsRecord.getRemainingTTL(now) <= mdnsRecord.getTtl() / 2; } + /** + * Creates a new full subtype name with given service type and subtype labels. + * + * For example, given ["_http", "_tcp"] and "_printer", this method returns a new String array + * of ["_printer", "_sub", "_http", "_tcp"]. + */ + public static String[] constructFullSubtype(String[] serviceType, String subtype) { + String[] fullSubtype = new String[serviceType.length + 2]; + fullSubtype[0] = subtype; + fullSubtype[1] = MdnsConstants.SUBTYPE_LABEL; + System.arraycopy(serviceType, 0, fullSubtype, 2, serviceType.length); + return fullSubtype; + } + /** A wrapper class of {@link SystemClock} to be mocked in unit tests. */ public static class Clock { /** diff --git a/service-t/src/com/android/server/ethernet/EthernetTracker.java b/service-t/src/com/android/server/ethernet/EthernetTracker.java index 48e86d8e7e..458d64f36f 100644 --- a/service-t/src/com/android/server/ethernet/EthernetTracker.java +++ b/service-t/src/com/android/server/ethernet/EthernetTracker.java @@ -48,6 +48,7 @@ import android.util.Log; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.IndentingPrintWriter; +import com.android.modules.utils.build.SdkLevel; import com.android.net.module.util.NetdUtils; import com.android.net.module.util.PermissionUtils; import com.android.net.module.util.SharedLog; @@ -237,7 +238,18 @@ public class EthernetTracker { mDeps = deps; // Interface match regex. - mIfaceMatch = mDeps.getInterfaceRegexFromResource(mContext); + String ifaceMatchRegex = mDeps.getInterfaceRegexFromResource(mContext); + // "*" is a magic string to indicate "pick the default". + if (ifaceMatchRegex.equals("*")) { + if (SdkLevel.isAtLeastV()) { + // On V+, include both usb%d and eth%d interfaces. + ifaceMatchRegex = "(usb|eth)\\d+"; + } else { + // On T and U, include only eth%d interfaces. + ifaceMatchRegex = "eth\\d+"; + } + } + mIfaceMatch = ifaceMatchRegex; // Read default Ethernet interface configuration from resources final String[] interfaceConfigs = mDeps.getInterfaceConfigFromResource(context); diff --git a/service-t/src/com/android/server/net/NetworkStatsRecorder.java b/service-t/src/com/android/server/net/NetworkStatsRecorder.java index 3da15856b7..8ee8591a4f 100644 --- a/service-t/src/com/android/server/net/NetworkStatsRecorder.java +++ b/service-t/src/com/android/server/net/NetworkStatsRecorder.java @@ -22,6 +22,7 @@ import static android.net.TrafficStats.MB_IN_BYTES; import static android.text.format.DateUtils.YEAR_IN_MILLIS; import android.annotation.NonNull; +import android.annotation.Nullable; import android.net.NetworkIdentitySet; import android.net.NetworkStats; import android.net.NetworkStats.NonMonotonicObserver; @@ -32,17 +33,20 @@ import android.net.NetworkTemplate; import android.net.TrafficStats; import android.os.Binder; import android.os.DropBoxManager; +import android.os.SystemClock; import android.service.NetworkStatsRecorderProto; import android.util.IndentingPrintWriter; import android.util.Log; import android.util.proto.ProtoOutputStream; import com.android.internal.util.FileRotator; +import com.android.metrics.NetworkStatsMetricsLogger; import com.android.net.module.util.NetworkStatsUtils; import libcore.io.IoUtils; import java.io.ByteArrayOutputStream; +import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -79,6 +83,7 @@ public class NetworkStatsRecorder { private final long mBucketDuration; private final boolean mOnlyTags; private final boolean mWipeOnError; + private final boolean mUseFastDataInput; private long mPersistThresholdBytes = 2 * MB_IN_BYTES; private NetworkStats mLastSnapshot; @@ -89,6 +94,9 @@ public class NetworkStatsRecorder { private final CombiningRewriter mPendingRewriter; private WeakReference<NetworkStatsCollection> mComplete; + private final NetworkStatsMetricsLogger mMetricsLogger = new NetworkStatsMetricsLogger(); + @Nullable + private final File mStatsDir; /** * Non-persisted recorder, with only one bucket. Used by {@link NetworkStatsObservers}. @@ -104,11 +112,13 @@ public class NetworkStatsRecorder { mBucketDuration = YEAR_IN_MILLIS; mOnlyTags = false; mWipeOnError = true; + mUseFastDataInput = false; mPending = null; mSinceBoot = new NetworkStatsCollection(mBucketDuration); mPendingRewriter = null; + mStatsDir = null; } /** @@ -116,7 +126,7 @@ public class NetworkStatsRecorder { */ public NetworkStatsRecorder(FileRotator rotator, NonMonotonicObserver<String> observer, DropBoxManager dropBox, String cookie, long bucketDuration, boolean onlyTags, - boolean wipeOnError) { + boolean wipeOnError, boolean useFastDataInput, @Nullable File statsDir) { mRotator = Objects.requireNonNull(rotator, "missing FileRotator"); mObserver = Objects.requireNonNull(observer, "missing NonMonotonicObserver"); mDropBox = Objects.requireNonNull(dropBox, "missing DropBoxManager"); @@ -125,11 +135,13 @@ public class NetworkStatsRecorder { mBucketDuration = bucketDuration; mOnlyTags = onlyTags; mWipeOnError = wipeOnError; + mUseFastDataInput = useFastDataInput; mPending = new NetworkStatsCollection(bucketDuration); mSinceBoot = new NetworkStatsCollection(bucketDuration); mPendingRewriter = new CombiningRewriter(mPending); + mStatsDir = statsDir; } public void setPersistThreshold(long thresholdBytes) { @@ -179,8 +191,16 @@ public class NetworkStatsRecorder { Objects.requireNonNull(mRotator, "missing FileRotator"); NetworkStatsCollection res = mComplete != null ? mComplete.get() : null; if (res == null) { + final long readStart = SystemClock.elapsedRealtime(); res = loadLocked(Long.MIN_VALUE, Long.MAX_VALUE); mComplete = new WeakReference<NetworkStatsCollection>(res); + final long readEnd = SystemClock.elapsedRealtime(); + // For legacy recorders which are used for data integrity check, which + // have wipeOnError flag unset, skip reporting metrics. + if (mWipeOnError) { + mMetricsLogger.logRecorderFileReading(mCookie, (int) (readEnd - readStart), + mStatsDir, res, mUseFastDataInput); + } } return res; } @@ -195,8 +215,12 @@ public class NetworkStatsRecorder { } private NetworkStatsCollection loadLocked(long start, long end) { - if (LOGD) Log.d(TAG, "loadLocked() reading from disk for " + mCookie); - final NetworkStatsCollection res = new NetworkStatsCollection(mBucketDuration); + if (LOGD) { + Log.d(TAG, "loadLocked() reading from disk for " + mCookie + + " useFastDataInput: " + mUseFastDataInput); + } + final NetworkStatsCollection res = + new NetworkStatsCollection(mBucketDuration, mUseFastDataInput); try { mRotator.readMatching(res, start, end); res.recordCollection(mPending); diff --git a/service-t/src/com/android/server/net/NetworkStatsService.java b/service-t/src/com/android/server/net/NetworkStatsService.java index 2c9f30c8f7..eb754616a6 100644 --- a/service-t/src/com/android/server/net/NetworkStatsService.java +++ b/service-t/src/com/android/server/net/NetworkStatsService.java @@ -44,7 +44,6 @@ import static android.net.NetworkStats.STATS_PER_UID; import static android.net.NetworkStats.TAG_ALL; import static android.net.NetworkStats.TAG_NONE; import static android.net.NetworkStats.UID_ALL; -import static android.net.NetworkStatsCollection.compareStats; import static android.net.NetworkStatsHistory.FIELD_ALL; import static android.net.NetworkTemplate.MATCH_MOBILE; import static android.net.NetworkTemplate.MATCH_TEST; @@ -295,6 +294,11 @@ public class NetworkStatsService extends INetworkStatsService.Stub { static final String CONFIG_ENABLE_NETWORK_STATS_EVENT_LOGGER = "enable_network_stats_event_logger"; + static final String NETSTATS_FASTDATAINPUT_TARGET_ATTEMPTS = + "netstats_fastdatainput_target_attempts"; + static final String NETSTATS_FASTDATAINPUT_SUCCESSES_COUNTER_NAME = "fastdatainput.successes"; + static final String NETSTATS_FASTDATAINPUT_FALLBACKS_COUNTER_NAME = "fastdatainput.fallbacks"; + private final Context mContext; private final NetworkStatsFactory mStatsFactory; private final AlarmManager mAlarmManager; @@ -318,6 +322,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private PersistentInt mImportLegacyAttemptsCounter = null; private PersistentInt mImportLegacySuccessesCounter = null; private PersistentInt mImportLegacyFallbacksCounter = null; + private PersistentInt mFastDataInputSuccessesCounter = null; + private PersistentInt mFastDataInputFallbacksCounter = null; @VisibleForTesting public static final String ACTION_NETWORK_STATS_POLL = @@ -695,6 +701,24 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } /** + * Get the count of using FastDataInput target attempts. + */ + public int getUseFastDataInputTargetAttempts() { + return DeviceConfigUtils.getDeviceConfigPropertyInt( + DeviceConfig.NAMESPACE_TETHERING, + NETSTATS_FASTDATAINPUT_TARGET_ATTEMPTS, 0); + } + + /** + * Compare two {@link NetworkStatsCollection} instances and returning a human-readable + * string description of difference for debugging purpose. + */ + public String compareStats(@NonNull NetworkStatsCollection a, + @NonNull NetworkStatsCollection b, boolean allowKeyChange) { + return NetworkStatsCollection.compareStats(a, b, allowKeyChange); + } + + /** * Create a persistent counter for given directory and name. */ public PersistentInt createPersistentCounter(@NonNull Path dir, @NonNull String name) @@ -892,13 +916,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { synchronized (mStatsLock) { mSystemReady = true; - // create data recorders along with historical rotators - mXtRecorder = buildRecorder(PREFIX_XT, mSettings.getXtConfig(), false, mStatsDir, - true /* wipeOnError */); - mUidRecorder = buildRecorder(PREFIX_UID, mSettings.getUidConfig(), false, mStatsDir, - true /* wipeOnError */); - mUidTagRecorder = buildRecorder(PREFIX_UID_TAG, mSettings.getUidTagConfig(), true, - mStatsDir, true /* wipeOnError */); + makeRecordersLocked(); updatePersistThresholdsLocked(); @@ -963,13 +981,106 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private NetworkStatsRecorder buildRecorder( String prefix, NetworkStatsSettings.Config config, boolean includeTags, - File baseDir, boolean wipeOnError) { + File baseDir, boolean wipeOnError, boolean useFastDataInput) { final DropBoxManager dropBox = (DropBoxManager) mContext.getSystemService( Context.DROPBOX_SERVICE); return new NetworkStatsRecorder(new FileRotator( baseDir, prefix, config.rotateAgeMillis, config.deleteAgeMillis), mNonMonotonicObserver, dropBox, prefix, config.bucketDuration, includeTags, - wipeOnError); + wipeOnError, useFastDataInput, baseDir); + } + + @GuardedBy("mStatsLock") + private void makeRecordersLocked() { + boolean useFastDataInput = true; + try { + mFastDataInputSuccessesCounter = mDeps.createPersistentCounter(mStatsDir.toPath(), + NETSTATS_FASTDATAINPUT_SUCCESSES_COUNTER_NAME); + mFastDataInputFallbacksCounter = mDeps.createPersistentCounter(mStatsDir.toPath(), + NETSTATS_FASTDATAINPUT_FALLBACKS_COUNTER_NAME); + } catch (IOException e) { + Log.wtf(TAG, "Failed to create persistent counters, skip.", e); + useFastDataInput = false; + } + + final int targetAttempts = mDeps.getUseFastDataInputTargetAttempts(); + int successes = 0; + int fallbacks = 0; + try { + successes = mFastDataInputSuccessesCounter.get(); + // Fallbacks counter would be set to non-zero value to indicate the reading was + // not successful. + fallbacks = mFastDataInputFallbacksCounter.get(); + } catch (IOException e) { + Log.wtf(TAG, "Failed to read counters, skip.", e); + useFastDataInput = false; + } + + final boolean doComparison; + if (useFastDataInput) { + // Use FastDataInput if it needs to be evaluated or at least one success. + doComparison = targetAttempts > successes + fallbacks; + // Set target attempt to -1 as the kill switch to disable the feature. + useFastDataInput = targetAttempts >= 0 && (doComparison || successes > 0); + } else { + // useFastDataInput is false due to previous failures. + doComparison = false; + } + + // create data recorders along with historical rotators. + // Don't wipe on error if comparison is needed. + mXtRecorder = buildRecorder(PREFIX_XT, mSettings.getXtConfig(), false, mStatsDir, + !doComparison /* wipeOnError */, useFastDataInput); + mUidRecorder = buildRecorder(PREFIX_UID, mSettings.getUidConfig(), false, mStatsDir, + !doComparison /* wipeOnError */, useFastDataInput); + mUidTagRecorder = buildRecorder(PREFIX_UID_TAG, mSettings.getUidTagConfig(), true, + mStatsDir, !doComparison /* wipeOnError */, useFastDataInput); + + if (!doComparison) return; + + final MigrationInfo[] migrations = new MigrationInfo[]{ + new MigrationInfo(mXtRecorder), + new MigrationInfo(mUidRecorder), + new MigrationInfo(mUidTagRecorder) + }; + // Set wipeOnError flag false so the recorder won't damage persistent data if reads + // failed and calling deleteAll. + final NetworkStatsRecorder[] legacyRecorders = new NetworkStatsRecorder[]{ + buildRecorder(PREFIX_XT, mSettings.getXtConfig(), false, mStatsDir, + false /* wipeOnError */, false /* useFastDataInput */), + buildRecorder(PREFIX_UID, mSettings.getUidConfig(), false, mStatsDir, + false /* wipeOnError */, false /* useFastDataInput */), + buildRecorder(PREFIX_UID_TAG, mSettings.getUidTagConfig(), true, mStatsDir, + false /* wipeOnError */, false /* useFastDataInput */)}; + boolean success = true; + for (int i = 0; i < migrations.length; i++) { + try { + migrations[i].collection = migrations[i].recorder.getOrLoadCompleteLocked(); + } catch (Throwable t) { + Log.wtf(TAG, "Failed to load collection, skip.", t); + success = false; + break; + } + if (!compareImportedToLegacyStats(migrations[i], legacyRecorders[i], + false /* allowKeyChange */)) { + success = false; + break; + } + } + + try { + if (success) { + mFastDataInputSuccessesCounter.set(successes + 1); + } else { + // Fallback. + mXtRecorder = legacyRecorders[0]; + mUidRecorder = legacyRecorders[1]; + mUidTagRecorder = legacyRecorders[2]; + mFastDataInputFallbacksCounter.set(fallbacks + 1); + } + } catch (IOException e) { + Log.wtf(TAG, "Failed to update counters. success = " + success, e); + } } @GuardedBy("mStatsLock") @@ -1068,7 +1179,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { new NetworkStatsSettings.Config(HOUR_IN_MILLIS, 15 * DAY_IN_MILLIS, 90 * DAY_IN_MILLIS); final NetworkStatsRecorder devRecorder = buildRecorder(PREFIX_DEV, devConfig, - false, mStatsDir, true /* wipeOnError */); + false, mStatsDir, true /* wipeOnError */, false /* useFastDataInput */); final MigrationInfo[] migrations = new MigrationInfo[]{ new MigrationInfo(devRecorder), new MigrationInfo(mXtRecorder), new MigrationInfo(mUidRecorder), new MigrationInfo(mUidTagRecorder) @@ -1085,11 +1196,11 @@ public class NetworkStatsService extends INetworkStatsService.Stub { legacyRecorders = new NetworkStatsRecorder[]{ null /* dev Recorder */, buildRecorder(PREFIX_XT, mSettings.getXtConfig(), false, legacyBaseDir, - false /* wipeOnError */), + false /* wipeOnError */, false /* useFastDataInput */), buildRecorder(PREFIX_UID, mSettings.getUidConfig(), false, legacyBaseDir, - false /* wipeOnError */), + false /* wipeOnError */, false /* useFastDataInput */), buildRecorder(PREFIX_UID_TAG, mSettings.getUidTagConfig(), true, legacyBaseDir, - false /* wipeOnError */)}; + false /* wipeOnError */, false /* useFastDataInput */)}; } else { legacyRecorders = null; } @@ -1120,7 +1231,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { if (runComparison) { final boolean success = - compareImportedToLegacyStats(migration, legacyRecorders[i]); + compareImportedToLegacyStats(migration, legacyRecorders[i], + true /* allowKeyChange */); if (!success && !dryRunImportOnly) { tryIncrementLegacyFallbacksCounter(); } @@ -1243,7 +1355,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { * does not match or throw with exceptions. */ private boolean compareImportedToLegacyStats(@NonNull MigrationInfo migration, - @Nullable NetworkStatsRecorder legacyRecorder) { + @Nullable NetworkStatsRecorder legacyRecorder, boolean allowKeyChange) { final NetworkStatsCollection legacyStats; // Skip the recorder that doesn't need to be compared. if (legacyRecorder == null) return true; @@ -1258,7 +1370,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { // The result of comparison is only for logging. try { - final String error = compareStats(migration.collection, legacyStats); + final String error = mDeps.compareStats(migration.collection, legacyStats, + allowKeyChange); if (error != null) { Log.wtf(TAG, "Unexpected comparison result for recorder " + legacyRecorder.getCookie() + ": " + error); @@ -1868,36 +1981,56 @@ 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) { - 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); + } + } + + 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) { - 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<String> limitIfaces; if (iface == IFACE_ALL) { @@ -1906,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); } /** @@ -2639,6 +2760,17 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } } pw.println(CONFIG_ENABLE_NETWORK_STATS_EVENT_LOGGER + ": " + mSupportEventLogger); + pw.print(NETSTATS_FASTDATAINPUT_TARGET_ATTEMPTS, + mDeps.getUseFastDataInputTargetAttempts()); + pw.println(); + try { + pw.print("FastDataInput successes", mFastDataInputSuccessesCounter.get()); + pw.println(); + pw.print("FastDataInput fallbacks", mFastDataInputFallbacksCounter.get()); + pw.println(); + } catch (IOException e) { + pw.println("(failed to dump FastDataInput counters)"); + } pw.decreaseIndent(); @@ -3274,10 +3406,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(); |