From 5c9d6cbf163325693e6ff4151472f39cbcca3a25 Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Tue, 23 Jan 2024 17:14:55 +0900 Subject: [PATCH] Implement offload priority Implement the TODO to assign priority to services based on an overlayable configuration. This changes the default priority from 0 to MAX_VALUE, as higher priority values are less prioritized, so services not in the priority list should have a high value. There is currently no user of the API depending on the priority, so the change should not affect clients. Test: atest Change-Id: I3aae9b5fa7756de64c63974c192e4c96b632ce4e --- .../src/com/android/server/NsdService.java | 7 +- .../connectivity/mdns/MdnsAdvertiser.java | 46 +++++++-- .../res/values/config.xml | 7 ++ .../res/values/overlayable.xml | 1 + .../net/src/android/net/cts/NsdManagerTest.kt | 3 +- .../com/android/server/NsdServiceTest.java | 9 +- .../connectivity/mdns/MdnsAdvertiserTest.kt | 97 +++++++++++++++++-- 7 files changed, 147 insertions(+), 23 deletions(-) diff --git a/service-t/src/com/android/server/NsdService.java b/service-t/src/com/android/server/NsdService.java index 9b2f80b3ac..62a2ae5230 100644 --- a/service-t/src/com/android/server/NsdService.java +++ b/service-t/src/com/android/server/NsdService.java @@ -1751,7 +1751,8 @@ public class NsdService extends INsdManager.Stub { mMdnsSocketClient, LOGGER.forSubComponent("MdnsDiscoveryManager"), flags); handler.post(() -> mMdnsSocketClient.setCallback(mMdnsDiscoveryManager)); mAdvertiser = deps.makeMdnsAdvertiser(handler.getLooper(), mMdnsSocketProvider, - new AdvertiserCallback(), LOGGER.forSubComponent("MdnsAdvertiser"), flags); + new AdvertiserCallback(), LOGGER.forSubComponent("MdnsAdvertiser"), flags, + mContext); mClock = deps.makeClock(); } @@ -1830,8 +1831,8 @@ public class NsdService extends INsdManager.Stub { public MdnsAdvertiser makeMdnsAdvertiser( @NonNull Looper looper, @NonNull MdnsSocketProvider socketProvider, @NonNull MdnsAdvertiser.AdvertiserCallback cb, @NonNull SharedLog sharedLog, - MdnsFeatureFlags featureFlags) { - return new MdnsAdvertiser(looper, socketProvider, cb, sharedLog, featureFlags); + MdnsFeatureFlags featureFlags, Context context) { + return new MdnsAdvertiser(looper, socketProvider, cb, sharedLog, featureFlags, context); } /** 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 135d957948..b2af93cdb4 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsAdvertiser.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsAdvertiser.java @@ -22,6 +22,7 @@ import static com.android.server.connectivity.mdns.MdnsRecord.MAX_LABEL_LENGTH; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.RequiresApi; +import android.content.Context; import android.net.LinkAddress; import android.net.Network; import android.net.nsd.NsdManager; @@ -34,9 +35,11 @@ import android.util.ArrayMap; import android.util.Log; import android.util.SparseArray; +import com.android.connectivity.resources.R; import com.android.internal.annotations.VisibleForTesting; import com.android.net.module.util.CollectionUtils; import com.android.net.module.util.SharedLog; +import com.android.server.connectivity.ConnectivityResources; import com.android.server.connectivity.mdns.util.MdnsUtils; import java.util.ArrayList; @@ -84,6 +87,7 @@ public class MdnsAdvertiser { private final Map> mInterfaceOffloadServices = new ArrayMap<>(); private final MdnsFeatureFlags mMdnsFeatureFlags; + private final Map mServiceTypeToOffloadPriority; /** * Dependencies for {@link MdnsAdvertiser}, useful for testing. @@ -621,14 +625,16 @@ public class MdnsAdvertiser { public MdnsAdvertiser(@NonNull Looper looper, @NonNull MdnsSocketProvider socketProvider, @NonNull AdvertiserCallback cb, @NonNull SharedLog sharedLog, - @NonNull MdnsFeatureFlags mDnsFeatureFlags) { - this(looper, socketProvider, cb, new Dependencies(), sharedLog, mDnsFeatureFlags); + @NonNull MdnsFeatureFlags mDnsFeatureFlags, @NonNull Context context) { + this(looper, socketProvider, cb, new Dependencies(), sharedLog, mDnsFeatureFlags, + context); } @VisibleForTesting MdnsAdvertiser(@NonNull Looper looper, @NonNull MdnsSocketProvider socketProvider, @NonNull AdvertiserCallback cb, @NonNull Dependencies deps, - @NonNull SharedLog sharedLog, @NonNull MdnsFeatureFlags mDnsFeatureFlags) { + @NonNull SharedLog sharedLog, @NonNull MdnsFeatureFlags mDnsFeatureFlags, + @NonNull Context context) { mLooper = looper; mCb = cb; mSocketProvider = socketProvider; @@ -636,6 +642,31 @@ public class MdnsAdvertiser { mDeviceHostName = deps.generateHostname(); mSharedLog = sharedLog; mMdnsFeatureFlags = mDnsFeatureFlags; + final ConnectivityResources res = new ConnectivityResources(context); + mServiceTypeToOffloadPriority = parseOffloadPriorityList( + res.get().getStringArray(R.array.config_nsdOffloadServicesPriority), sharedLog); + } + + private static Map parseOffloadPriorityList( + @NonNull String[] resValues, SharedLog sharedLog) { + final Map priorities = new ArrayMap<>(resValues.length); + for (String entry : resValues) { + final String[] priorityAndType = entry.split(":", 2); + if (priorityAndType.length != 2) { + sharedLog.wtf("Invalid config_nsdOffloadServicesPriority ignored: " + entry); + continue; + } + + final int priority; + try { + priority = Integer.parseInt(priorityAndType[0]); + } catch (NumberFormatException e) { + sharedLog.wtf("Invalid priority in config_nsdOffloadServicesPriority: " + entry); + continue; + } + priorities.put(MdnsUtils.toDnsLowerCase(priorityAndType[1]), priority); + } + return priorities; } private void checkThread() { @@ -777,16 +808,17 @@ public class MdnsAdvertiser { private OffloadServiceInfoWrapper createOffloadService(int serviceId, @NonNull Registration registration, byte[] rawOffloadPacket) { final NsdServiceInfo nsdServiceInfo = registration.getServiceInfo(); + final Integer mapPriority = mServiceTypeToOffloadPriority.get( + MdnsUtils.toDnsLowerCase(nsdServiceInfo.getServiceType())); + // Higher values of priority are less prioritized + final int priority = mapPriority == null ? Integer.MAX_VALUE : mapPriority; final OffloadServiceInfo offloadServiceInfo = new OffloadServiceInfo( new OffloadServiceInfo.Key(nsdServiceInfo.getServiceName(), nsdServiceInfo.getServiceType()), new ArrayList<>(nsdServiceInfo.getSubtypes()), String.join(".", mDeviceHostName), rawOffloadPacket, - // TODO: define overlayable resources in - // ServiceConnectivityResources that set the priority based on - // service type. - 0 /* priority */, + priority, // TODO: set the offloadType based on the callback timing. OffloadEngine.OFFLOAD_TYPE_REPLY); return new OffloadServiceInfoWrapper(serviceId, offloadServiceInfo); diff --git a/service/ServiceConnectivityResources/res/values/config.xml b/service/ServiceConnectivityResources/res/values/config.xml index 6f9d46f36c..2d3647a67f 100644 --- a/service/ServiceConnectivityResources/res/values/config.xml +++ b/service/ServiceConnectivityResources/res/values/config.xml @@ -128,6 +128,13 @@ + + + + false diff --git a/service/ServiceConnectivityResources/res/values/overlayable.xml b/service/ServiceConnectivityResources/res/values/overlayable.xml index 1c0759999c..f2c4d91534 100644 --- a/service/ServiceConnectivityResources/res/values/overlayable.xml +++ b/service/ServiceConnectivityResources/res/values/overlayable.xml @@ -31,6 +31,7 @@ + diff --git a/tests/cts/net/src/android/net/cts/NsdManagerTest.kt b/tests/cts/net/src/android/net/cts/NsdManagerTest.kt index 1309e79872..e433442994 100644 --- a/tests/cts/net/src/android/net/cts/NsdManagerTest.kt +++ b/tests/cts/net/src/android/net/cts/NsdManagerTest.kt @@ -686,7 +686,8 @@ class NsdManagerTest { assertEquals(listOf("_subtype"), serviceInfo.subtypes) assertTrue(serviceInfo.hostname.startsWith("Android_")) assertTrue(serviceInfo.hostname.endsWith("local")) - assertEquals(0, serviceInfo.priority) + // Test service types should not be in the priority list + assertEquals(Integer.MAX_VALUE, serviceInfo.priority) assertEquals(OffloadEngine.OFFLOAD_TYPE_REPLY.toLong(), serviceInfo.offloadType) val offloadPayload = serviceInfo.offloadPayload assertNotNull(offloadPayload) diff --git a/tests/unit/java/com/android/server/NsdServiceTest.java b/tests/unit/java/com/android/server/NsdServiceTest.java index 87e7967bcd..a17197e4bb 100644 --- a/tests/unit/java/com/android/server/NsdServiceTest.java +++ b/tests/unit/java/com/android/server/NsdServiceTest.java @@ -234,7 +234,8 @@ public class NsdServiceTest { doReturn(mSocketProvider).when(mDeps).makeMdnsSocketProvider(any(), any(), any(), any()); doReturn(DEFAULT_RUNNING_APP_ACTIVE_IMPORTANCE_CUTOFF).when(mDeps).getDeviceConfigInt( eq(NsdService.MDNS_CONFIG_RUNNING_APP_ACTIVE_IMPORTANCE_CUTOFF), anyInt()); - doReturn(mAdvertiser).when(mDeps).makeMdnsAdvertiser(any(), any(), any(), any(), any()); + doReturn(mAdvertiser).when(mDeps).makeMdnsAdvertiser(any(), any(), any(), any(), any(), + any()); doReturn(mMetrics).when(mDeps).makeNetworkNsdReportedMetrics(anyInt()); doReturn(mClock).when(mDeps).makeClock(); doReturn(TEST_TIME_MS).when(mClock).elapsedRealtime(); @@ -1320,7 +1321,7 @@ public class NsdServiceTest { // final String serviceTypeWithLocalDomain = SERVICE_TYPE + ".local"; final ArgumentCaptor cbCaptor = ArgumentCaptor.forClass(MdnsAdvertiser.AdvertiserCallback.class); - verify(mDeps).makeMdnsAdvertiser(any(), any(), cbCaptor.capture(), any(), any()); + verify(mDeps).makeMdnsAdvertiser(any(), any(), cbCaptor.capture(), any(), any(), any()); final NsdServiceInfo regInfo = new NsdServiceInfo(SERVICE_NAME, SERVICE_TYPE); regInfo.setHost(parseNumericAddress("192.0.2.123")); @@ -1371,7 +1372,7 @@ public class NsdServiceTest { // final String serviceTypeWithLocalDomain = SERVICE_TYPE + ".local"; final ArgumentCaptor cbCaptor = ArgumentCaptor.forClass(MdnsAdvertiser.AdvertiserCallback.class); - verify(mDeps).makeMdnsAdvertiser(any(), any(), cbCaptor.capture(), any(), any()); + verify(mDeps).makeMdnsAdvertiser(any(), any(), cbCaptor.capture(), any(), any(), any()); final NsdServiceInfo regInfo = new NsdServiceInfo(SERVICE_NAME, "invalid_type"); regInfo.setHost(parseNumericAddress("192.0.2.123")); @@ -1398,7 +1399,7 @@ public class NsdServiceTest { // final String serviceTypeWithLocalDomain = SERVICE_TYPE + ".local"; final ArgumentCaptor cbCaptor = ArgumentCaptor.forClass(MdnsAdvertiser.AdvertiserCallback.class); - verify(mDeps).makeMdnsAdvertiser(any(), any(), cbCaptor.capture(), any(), any()); + verify(mDeps).makeMdnsAdvertiser(any(), any(), cbCaptor.capture(), any(), any(), any()); final NsdServiceInfo regInfo = new NsdServiceInfo("a".repeat(70), SERVICE_TYPE); regInfo.setHost(parseNumericAddress("192.0.2.123")); diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsAdvertiserTest.kt b/tests/unit/java/com/android/server/connectivity/mdns/MdnsAdvertiserTest.kt index 121f844df9..5c04362168 100644 --- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsAdvertiserTest.kt +++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsAdvertiserTest.kt @@ -16,6 +16,8 @@ package com.android.server.connectivity.mdns +import android.content.Context +import android.content.res.Resources import android.net.InetAddresses.parseNumericAddress import android.net.LinkAddress import android.net.Network @@ -26,7 +28,9 @@ import android.net.nsd.OffloadServiceInfo import android.os.Build import android.os.Handler import android.os.HandlerThread +import com.android.connectivity.resources.R import com.android.net.module.util.SharedLog +import com.android.server.connectivity.ConnectivityResources import com.android.server.connectivity.mdns.MdnsAdvertiser.AdvertiserCallback import com.android.server.connectivity.mdns.MdnsSocketProvider.SocketCallback import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo @@ -55,9 +59,10 @@ import org.mockito.Mockito.verify private const val SERVICE_ID_1 = 1 private const val SERVICE_ID_2 = 2 -private const val LONG_SERVICE_ID_1 = 3 -private const val LONG_SERVICE_ID_2 = 4 -private const val CASE_INSENSITIVE_TEST_SERVICE_ID = 5 +private const val SERVICE_ID_3 = 3 +private const val LONG_SERVICE_ID_1 = 4 +private const val LONG_SERVICE_ID_2 = 5 +private const val CASE_INSENSITIVE_TEST_SERVICE_ID = 6 private const val TIMEOUT_MS = 10_000L private val TEST_ADDR = parseNumericAddress("2001:db8::123") private val TEST_ADDR2 = parseNumericAddress("2001:db8::124") @@ -150,6 +155,12 @@ private val OFFLOAD_SERVICEINFO_NO_SUBTYPE2 = OffloadServiceInfo( OffloadEngine.OFFLOAD_TYPE_REPLY.toLong() ) +private val SERVICES_PRIORITY_LIST = arrayOf( + "0:_advertisertest._tcp", + "5:_prioritytest._udp", + "5:_otherprioritytest._tcp" +) + @RunWith(DevSdkIgnoreRunner::class) @IgnoreUpTo(Build.VERSION_CODES.S_V2) class MdnsAdvertiserTest { @@ -164,6 +175,8 @@ class MdnsAdvertiserTest { private val mockInterfaceAdvertiser1 = mock(MdnsInterfaceAdvertiser::class.java) private val mockInterfaceAdvertiser2 = mock(MdnsInterfaceAdvertiser::class.java) private val mockDeps = mock(MdnsAdvertiser.Dependencies::class.java) + private val context = mock(Context::class.java) + private val resources = mock(Resources::class.java) private val flags = MdnsFeatureFlags.newBuilder().setIsMdnsOffloadFeatureEnabled(true).build() @Before @@ -184,12 +197,21 @@ class MdnsAdvertiserTest { doReturn(TEST_INTERFACE2).`when`(mockInterfaceAdvertiser2).socketInterfaceName doReturn(TEST_OFFLOAD_PACKET1).`when`(mockInterfaceAdvertiser1).getRawOffloadPayload( SERVICE_ID_1) + doReturn(TEST_OFFLOAD_PACKET1).`when`(mockInterfaceAdvertiser1).getRawOffloadPayload( + SERVICE_ID_2) + doReturn(TEST_OFFLOAD_PACKET1).`when`(mockInterfaceAdvertiser1).getRawOffloadPayload( + SERVICE_ID_3) doReturn(TEST_OFFLOAD_PACKET1).`when`(mockInterfaceAdvertiser2).getRawOffloadPayload( SERVICE_ID_1) + doReturn(resources).`when`(context).getResources() + doReturn(SERVICES_PRIORITY_LIST).`when`(resources).getStringArray( + R.array.config_nsdOffloadServicesPriority) + ConnectivityResources.setResourcesContextForTest(context) } @After fun tearDown() { + ConnectivityResources.setResourcesContextForTest(null) thread.quitSafely() thread.join() } @@ -203,7 +225,7 @@ class MdnsAdvertiserTest { @Test fun testAddService_OneNetwork() { val advertiser = - MdnsAdvertiser(thread.looper, socketProvider, cb, mockDeps, sharedlog, flags) + MdnsAdvertiser(thread.looper, socketProvider, cb, mockDeps, sharedlog, flags, context) postSync { advertiser.addOrUpdateService(SERVICE_ID_1, SERVICE_1, DEFAULT_ADVERTISING_OPTION) } @@ -265,7 +287,7 @@ class MdnsAdvertiserTest { @Test fun testAddService_AllNetworksWithSubType() { val advertiser = - MdnsAdvertiser(thread.looper, socketProvider, cb, mockDeps, sharedlog, flags) + MdnsAdvertiser(thread.looper, socketProvider, cb, mockDeps, sharedlog, flags, context) postSync { advertiser.addOrUpdateService(SERVICE_ID_1, ALL_NETWORKS_SERVICE_SUBTYPE, DEFAULT_ADVERTISING_OPTION) } @@ -334,10 +356,68 @@ class MdnsAdvertiserTest { verify(socketProvider).unrequestSocket(socketCb) } + @Test + fun testAddService_OffloadPriority() { + val advertiser = + MdnsAdvertiser(thread.looper, socketProvider, cb, mockDeps, sharedlog, flags, context) + postSync { + advertiser.addOrUpdateService(SERVICE_ID_1, SERVICE_1, DEFAULT_ADVERTISING_OPTION) + advertiser.addOrUpdateService(SERVICE_ID_2, + NsdServiceInfo("TestService2", "_PRIORITYTEST._udp").apply { + port = 12345 + hostAddresses = listOf(TEST_ADDR) + }, DEFAULT_ADVERTISING_OPTION) + advertiser.addOrUpdateService( + SERVICE_ID_3, + NsdServiceInfo("TestService3", "_notprioritized._tcp").apply { + port = 12345 + hostAddresses = listOf(TEST_ADDR) + }, DEFAULT_ADVERTISING_OPTION) + } + + val socketCbCaptor = ArgumentCaptor.forClass(SocketCallback::class.java) + verify(socketProvider).requestSocket(eq(SERVICE_1.network), socketCbCaptor.capture()) + + val socketCb = socketCbCaptor.value + postSync { socketCb.onSocketCreated(TEST_SOCKETKEY_1, mockSocket1, listOf(TEST_LINKADDR)) } + + val intAdvCbCaptor1 = ArgumentCaptor.forClass(MdnsInterfaceAdvertiser.Callback::class.java) + verify(mockDeps).makeAdvertiser(eq(mockSocket1), eq(listOf(TEST_LINKADDR)), + eq(thread.looper), any(), intAdvCbCaptor1.capture(), eq(TEST_HOSTNAME), any(), any() + ) + + doReturn(false).`when`(mockInterfaceAdvertiser1).isProbing(SERVICE_ID_1) + doReturn(false).`when`(mockInterfaceAdvertiser1).isProbing(SERVICE_ID_2) + doReturn(false).`when`(mockInterfaceAdvertiser1).isProbing(SERVICE_ID_3) + postSync { + intAdvCbCaptor1.value.onServiceProbingSucceeded(mockInterfaceAdvertiser1, SERVICE_ID_1) + intAdvCbCaptor1.value.onServiceProbingSucceeded(mockInterfaceAdvertiser1, SERVICE_ID_2) + intAdvCbCaptor1.value.onServiceProbingSucceeded(mockInterfaceAdvertiser1, SERVICE_ID_3) + } + + verify(cb).onOffloadStartOrUpdate(eq(TEST_INTERFACE1), eq(OFFLOAD_SERVICEINFO_NO_SUBTYPE)) + verify(cb).onOffloadStartOrUpdate(eq(TEST_INTERFACE1), eq(OffloadServiceInfo( + OffloadServiceInfo.Key("TestService2", "_PRIORITYTEST._udp"), + emptyList() /* subtypes */, + "Android_test.local", + TEST_OFFLOAD_PACKET1, + 5, /* priority */ + OffloadEngine.OFFLOAD_TYPE_REPLY.toLong() + ))) + verify(cb).onOffloadStartOrUpdate(eq(TEST_INTERFACE1), eq(OffloadServiceInfo( + OffloadServiceInfo.Key("TestService3", "_notprioritized._tcp"), + emptyList() /* subtypes */, + "Android_test.local", + TEST_OFFLOAD_PACKET1, + Integer.MAX_VALUE, /* priority */ + OffloadEngine.OFFLOAD_TYPE_REPLY.toLong() + ))) + } + @Test fun testAddService_Conflicts() { val advertiser = - MdnsAdvertiser(thread.looper, socketProvider, cb, mockDeps, sharedlog, flags) + MdnsAdvertiser(thread.looper, socketProvider, cb, mockDeps, sharedlog, flags, context) postSync { advertiser.addOrUpdateService(SERVICE_ID_1, SERVICE_1, DEFAULT_ADVERTISING_OPTION) } @@ -425,7 +505,8 @@ class MdnsAdvertiserTest { @Test fun testAddOrUpdateService_Updates() { val advertiser = - MdnsAdvertiser(thread.looper, socketProvider, cb, mockDeps, sharedlog, flags) + MdnsAdvertiser(thread.looper, socketProvider, cb, mockDeps, sharedlog, flags, + context) postSync { advertiser.addOrUpdateService(SERVICE_ID_1, ALL_NETWORKS_SERVICE, DEFAULT_ADVERTISING_OPTION) } @@ -463,7 +544,7 @@ class MdnsAdvertiserTest { @Test fun testRemoveService_whenAllServiceRemoved_thenUpdateHostName() { val advertiser = - MdnsAdvertiser(thread.looper, socketProvider, cb, mockDeps, sharedlog, flags) + MdnsAdvertiser(thread.looper, socketProvider, cb, mockDeps, sharedlog, flags, context) verify(mockDeps, times(1)).generateHostname() postSync { advertiser.addOrUpdateService(SERVICE_ID_1, SERVICE_1, DEFAULT_ADVERTISING_OPTION) }