From e24b215bf2bc670815d3628862a02899faec7070 Mon Sep 17 00:00:00 2001 From: Handa Wang Date: Wed, 4 Sep 2024 12:22:23 +0000 Subject: [PATCH] [Thread] refactor setInfraLinkState This CL integrates the refactoring of aosp/3264051. Bug: 365892457 Change-Id: I77788c06db02abbf2275fda3015b0f3fa63920e0 --- .../android/server/thread/NsdPublisher.java | 20 ++-- .../ThreadNetworkControllerService.java | 95 ++++++++++--------- .../server/thread/NsdPublisherTest.java | 6 +- .../ThreadNetworkControllerServiceTest.java | 5 +- 4 files changed, 68 insertions(+), 58 deletions(-) diff --git a/thread/service/java/com/android/server/thread/NsdPublisher.java b/thread/service/java/com/android/server/thread/NsdPublisher.java index 38c77bf50e..9697c02138 100644 --- a/thread/service/java/com/android/server/thread/NsdPublisher.java +++ b/thread/service/java/com/android/server/thread/NsdPublisher.java @@ -23,6 +23,7 @@ import android.annotation.Nullable; import android.content.Context; import android.net.DnsResolver; import android.net.InetAddresses; +import android.net.LinkProperties; import android.net.Network; import android.net.nsd.DiscoveryRequest; import android.net.nsd.NsdManager; @@ -67,7 +68,7 @@ public final class NsdPublisher extends INsdPublisher.Stub { // TODO: b/321883491 - specify network for mDNS operations @Nullable private Network mNetwork; - private final Map mNetworkToInterface; + private final Map mNetworkToLinkProperties; private final NsdManager mNsdManager; private final DnsResolver mDnsResolver; private final Handler mHandler; @@ -82,22 +83,24 @@ public final class NsdPublisher extends INsdPublisher.Stub { NsdManager nsdManager, DnsResolver dnsResolver, Handler handler, - Map networkToInterface) { + Map networkToLinkProperties) { mNetwork = null; mNsdManager = nsdManager; mDnsResolver = dnsResolver; mHandler = handler; mExecutor = runnable -> mHandler.post(runnable); - mNetworkToInterface = networkToInterface; + mNetworkToLinkProperties = networkToLinkProperties; } public static NsdPublisher newInstance( - Context context, Handler handler, Map networkToInterface) { + Context context, + Handler handler, + Map networkToLinkProperties) { return new NsdPublisher( context.getSystemService(NsdManager.class), DnsResolver.getInstance(), handler, - networkToInterface); + networkToLinkProperties); } // TODO: b/321883491 - NsdPublisher should be disabled when mNetwork is null @@ -598,9 +601,12 @@ public final class NsdPublisher extends INsdPublisher.Stub { + serviceInfo); List addresses = new ArrayList<>(); int interfaceIndex = 0; - if (mNetworkToInterface.containsKey(serviceInfo.getNetwork())) { + if (mNetworkToLinkProperties.containsKey(serviceInfo.getNetwork())) { interfaceIndex = - Os.if_nametoindex(mNetworkToInterface.get(serviceInfo.getNetwork())); + Os.if_nametoindex( + mNetworkToLinkProperties + .get(serviceInfo.getNetwork()) + .getInterfaceName()); } for (InetAddress address : serviceInfo.getHostAddresses()) { if (address instanceof Inet6Address) { diff --git a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java index 901d4949ea..855a1e8983 100644 --- a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java +++ b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java @@ -209,7 +209,7 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub private NetworkRequest mUpstreamNetworkRequest; private UpstreamNetworkCallback mUpstreamNetworkCallback; private TestNetworkSpecifier mUpstreamTestNetworkSpecifier; - private final Map mNetworkToInterface; + private final Map mNetworkToLinkProperties; private final ThreadPersistentSettings mPersistentSettings; private final UserManager mUserManager; private boolean mUserRestricted; @@ -232,7 +232,7 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub UserManager userManager, ConnectivityResources resources, Supplier countryCodeSupplier, - Map networkToInterface) { + Map networkToLinkProperties) { mContext = context; mHandler = handler; mNetworkProvider = networkProvider; @@ -241,9 +241,9 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub mTunIfController = tunIfController; mInfraIfController = infraIfController; mUpstreamNetworkRequest = newUpstreamNetworkRequest(); - // TODO: mNetworkToInterface should be shared with NsdPublisher, add a test/assert to + // TODO: networkToLinkProperties should be shared with NsdPublisher, add a test/assert to // verify they are the same. - mNetworkToInterface = networkToInterface; + mNetworkToLinkProperties = networkToLinkProperties; mOtDaemonConfig = new OtDaemonConfiguration.Builder().build(); mInfraLinkState = new InfraLinkState.Builder().build(); mPersistentSettings = persistentSettings; @@ -262,7 +262,7 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub Handler handler = new Handler(handlerThread.getLooper()); NetworkProvider networkProvider = new NetworkProvider(context, handlerThread.getLooper(), "ThreadNetworkProvider"); - Map networkToInterface = new HashMap(); + Map networkToLinkProperties = new HashMap<>(); return new ThreadNetworkControllerService( context, @@ -273,11 +273,11 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub new TunInterfaceController(TUN_IF_NAME), new InfraInterfaceController(), persistentSettings, - NsdPublisher.newInstance(context, handler, networkToInterface), + NsdPublisher.newInstance(context, handler, networkToLinkProperties), context.getSystemService(UserManager.class), new ConnectivityResources(context), countryCodeSupplier, - networkToInterface); + networkToLinkProperties); } private NetworkRequest newUpstreamNetworkRequest() { @@ -695,7 +695,7 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub if (mUpstreamNetworkCallback == null) { throw new AssertionError("The upstream network request null."); } - mNetworkToInterface.clear(); + mNetworkToLinkProperties.clear(); mConnectivityManager.unregisterNetworkCallback(mUpstreamNetworkCallback); mUpstreamNetworkCallback = null; } @@ -717,20 +717,19 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub @Override public void onLinkPropertiesChanged( - @NonNull Network network, @NonNull LinkProperties linkProperties) { + @NonNull Network network, @NonNull LinkProperties newLinkProperties) { checkOnHandlerThread(); - String existingIfName = mNetworkToInterface.get(network); - String newIfName = linkProperties.getInterfaceName(); - if (Objects.equals(existingIfName, newIfName)) { + LinkProperties oldLinkProperties = mNetworkToLinkProperties.get(network); + if (Objects.equals(oldLinkProperties, newLinkProperties)) { return; } - LOG.i("Upstream network changed: " + existingIfName + " -> " + newIfName); - mNetworkToInterface.put(network, newIfName); + LOG.i("Upstream network changed: " + oldLinkProperties + " -> " + newLinkProperties); + mNetworkToLinkProperties.put(network, newLinkProperties); // TODO: disable border routing if netIfName is null if (network.equals(mUpstreamNetwork)) { - enableBorderRouting(mNetworkToInterface.get(mUpstreamNetwork)); + setInfraLinkState(newInfraLinkStateBuilder(newLinkProperties).build()); } } } @@ -746,7 +745,7 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub public void onLost(@NonNull Network network) { checkOnHandlerThread(); LOG.i("Thread network is lost: " + network); - disableBorderRouting(); + setInfraLinkState(newInfraLinkStateBuilder().build()); } @Override @@ -760,13 +759,15 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub + localNetworkInfo + "}"); if (localNetworkInfo.getUpstreamNetwork() == null) { - disableBorderRouting(); + setInfraLinkState(newInfraLinkStateBuilder().build()); return; } if (!localNetworkInfo.getUpstreamNetwork().equals(mUpstreamNetwork)) { mUpstreamNetwork = localNetworkInfo.getUpstreamNetwork(); - if (mNetworkToInterface.containsKey(mUpstreamNetwork)) { - enableBorderRouting(mNetworkToInterface.get(mUpstreamNetwork)); + if (mNetworkToLinkProperties.containsKey(mUpstreamNetwork)) { + setInfraLinkState( + newInfraLinkStateBuilder(mNetworkToLinkProperties.get(mUpstreamNetwork)) + .build()); } mNsdPublisher.setNetworkForHostResolution(mUpstreamNetwork); } @@ -1233,47 +1234,39 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub } } - private void setInfraLinkState(InfraLinkState infraLinkState) { - if (mInfraLinkState.equals(infraLinkState)) { + private void setInfraLinkState(InfraLinkState newInfraLinkState) { + if (mInfraLinkState.equals(newInfraLinkState)) { + return; + } + LOG.i("Infra link state changed: " + mInfraLinkState + " -> " + newInfraLinkState); + + setInfraLinkInterfaceName(newInfraLinkState.interfaceName); + mInfraLinkState = newInfraLinkState; + } + + private void setInfraLinkInterfaceName(String newInfraLinkInterfaceName) { + if (Objects.equals(mInfraLinkState.interfaceName, newInfraLinkInterfaceName)) { return; } - LOG.i("Infra link state changed: " + mInfraLinkState + " -> " + infraLinkState); - mInfraLinkState = infraLinkState; ParcelFileDescriptor infraIcmp6Socket = null; - if (mInfraLinkState.interfaceName != null) { + if (newInfraLinkInterfaceName != null) { try { - infraIcmp6Socket = - mInfraIfController.createIcmp6Socket(mInfraLinkState.interfaceName); + infraIcmp6Socket = mInfraIfController.createIcmp6Socket(newInfraLinkInterfaceName); } catch (IOException e) { LOG.e("Failed to create ICMPv6 socket on infra network interface", e); } } try { getOtDaemon() - .setInfraLinkState( - mInfraLinkState, + .setInfraLinkInterfaceName( + newInfraLinkInterfaceName, infraIcmp6Socket, - new LoggingOtStatusReceiver("setInfraLinkState")); + new LoggingOtStatusReceiver("setInfraLinkInterfaceName")); } catch (RemoteException | ThreadNetworkException e) { - LOG.e("Failed to configure border router " + mOtDaemonConfig, e); + LOG.e("Failed to set infra link interface name " + newInfraLinkInterfaceName, e); } } - private void enableBorderRouting(String infraIfName) { - InfraLinkState infraLinkState = - newInfraLinkStateBuilder(mInfraLinkState).setInterfaceName(infraIfName).build(); - LOG.i("Enable border routing on AIL: " + infraIfName); - setInfraLinkState(infraLinkState); - } - - private void disableBorderRouting() { - mUpstreamNetwork = null; - InfraLinkState infraLinkState = - newInfraLinkStateBuilder(mInfraLinkState).setInterfaceName(null).build(); - LOG.i("Disabling border routing"); - setInfraLinkState(infraLinkState); - } - private void handleThreadInterfaceStateChanged(boolean isUp) { try { mTunIfController.setInterfaceUp(isUp); @@ -1377,8 +1370,16 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub return new OtDaemonConfiguration.Builder(); } - private static InfraLinkState.Builder newInfraLinkStateBuilder(InfraLinkState infraLinkState) { - return new InfraLinkState.Builder().setInterfaceName(infraLinkState.interfaceName); + private static InfraLinkState.Builder newInfraLinkStateBuilder() { + return new InfraLinkState.Builder().setInterfaceName(""); + } + + private static InfraLinkState.Builder newInfraLinkStateBuilder( + @Nullable LinkProperties linkProperties) { + if (linkProperties == null) { + return newInfraLinkStateBuilder(); + } + return new InfraLinkState.Builder().setInterfaceName(linkProperties.getInterfaceName()); } private static final class CallbackMetadata { diff --git a/thread/tests/unit/src/com/android/server/thread/NsdPublisherTest.java b/thread/tests/unit/src/com/android/server/thread/NsdPublisherTest.java index 1959198288..d52191ad30 100644 --- a/thread/tests/unit/src/com/android/server/thread/NsdPublisherTest.java +++ b/thread/tests/unit/src/com/android/server/thread/NsdPublisherTest.java @@ -34,6 +34,7 @@ import static org.mockito.hamcrest.MockitoHamcrest.argThat; import android.net.DnsResolver; import android.net.InetAddresses; +import android.net.LinkProperties; import android.net.Network; import android.net.nsd.DiscoveryRequest; import android.net.nsd.NsdManager; @@ -813,9 +814,10 @@ public final class NsdPublisherTest { private void prepareTest() { mTestLooper = new TestLooper(); Handler handler = new Handler(mTestLooper.getLooper()); - HashMap networkToInterface = new HashMap<>(); + HashMap networkToLinkProperties = new HashMap<>(); mNsdPublisher = - new NsdPublisher(mMockNsdManager, mMockDnsResolver, handler, networkToInterface); + new NsdPublisher( + mMockNsdManager, mMockDnsResolver, handler, networkToLinkProperties); mNsdPublisher.setNetworkForHostResolution(mNetwork); } } diff --git a/thread/tests/unit/src/com/android/server/thread/ThreadNetworkControllerServiceTest.java b/thread/tests/unit/src/com/android/server/thread/ThreadNetworkControllerServiceTest.java index f986463916..0736895ba2 100644 --- a/thread/tests/unit/src/com/android/server/thread/ThreadNetworkControllerServiceTest.java +++ b/thread/tests/unit/src/com/android/server/thread/ThreadNetworkControllerServiceTest.java @@ -63,6 +63,7 @@ import android.content.Context; import android.content.Intent; import android.content.res.Resources; import android.net.ConnectivityManager; +import android.net.LinkProperties; import android.net.Network; import android.net.NetworkAgent; import android.net.NetworkProvider; @@ -172,7 +173,7 @@ public final class ThreadNetworkControllerServiceTest { @Mock private IBinder mIBinder; @Mock Resources mResources; @Mock ConnectivityResources mConnectivityResources; - @Mock Map mMockNetworkToInterface; + @Mock Map mMockNetworkToLinkProperties; private Context mContext; private TestLooper mTestLooper; @@ -236,7 +237,7 @@ public final class ThreadNetworkControllerServiceTest { mMockUserManager, mConnectivityResources, () -> DEFAULT_COUNTRY_CODE, - mMockNetworkToInterface); + mMockNetworkToLinkProperties); mService.setTestNetworkAgent(mMockNetworkAgent); }