Merge "[Thread] refactor setInfraLinkState" into main

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<Network, String> mNetworkToInterface;
+    private final Map<Network, LinkProperties> 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<Network, String> networkToInterface) {
+            Map<Network, LinkProperties> 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<Network, String> networkToInterface) {
+            Context context,
+            Handler handler,
+            Map<Network, LinkProperties> 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<String> 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) {

thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java

@@ -212,7 +212,7 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub
     private NetworkRequest mUpstreamNetworkRequest;
     private UpstreamNetworkCallback mUpstreamNetworkCallback;
     private TestNetworkSpecifier mUpstreamTestNetworkSpecifier;
-    private final Map<Network, String> mNetworkToInterface;
+    private final Map<Network, LinkProperties> mNetworkToLinkProperties;
     private final ThreadPersistentSettings mPersistentSettings;
     private final UserManager mUserManager;
     private boolean mUserRestricted;
@@ -235,7 +235,7 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub
             UserManager userManager,
             ConnectivityResources resources,
             Supplier<String> countryCodeSupplier,
-            Map<Network, String> networkToInterface) {
+            Map<Network, LinkProperties> networkToLinkProperties) {
         mContext = context;
         mHandler = handler;
         mNetworkProvider = networkProvider;
@@ -244,9 +244,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;
@@ -265,7 +265,7 @@ final class ThreadNetworkControllerService extends IThreadNetworkController.Stub
         Handler handler = new Handler(handlerThread.getLooper());
         NetworkProvider networkProvider =
                 new NetworkProvider(context, handlerThread.getLooper(), "ThreadNetworkProvider");
-        Map<Network, String> networkToInterface = new HashMap<Network, String>();
+        Map<Network, LinkProperties> networkToLinkProperties = new HashMap<>();
 
         return new ThreadNetworkControllerService(
                 context,
@@ -276,11 +276,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() {
@@ -699,7 +699,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;
     }
@@ -721,20 +721,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());
             }
         }
     }
@@ -750,7 +749,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
@@ -764,13 +763,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);
             }
@@ -1256,47 +1257,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);
@@ -1425,8 +1418,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 {

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<Network, String> networkToInterface = new HashMap<>();
+        HashMap<Network, LinkProperties> networkToLinkProperties = new HashMap<>();
         mNsdPublisher =
-                new NsdPublisher(mMockNsdManager, mMockDnsResolver, handler, networkToInterface);
+                new NsdPublisher(
+                        mMockNsdManager, mMockDnsResolver, handler, networkToLinkProperties);
         mNsdPublisher.setNetworkForHostResolution(mNetwork);
     }
 }

thread/tests/unit/src/com/android/server/thread/ThreadNetworkControllerServiceTest.java

@@ -64,6 +64,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;
@@ -174,7 +175,7 @@ public final class ThreadNetworkControllerServiceTest {
     @Mock private IBinder mIBinder;
     @Mock Resources mResources;
     @Mock ConnectivityResources mConnectivityResources;
-    @Mock Map<Network, String> mMockNetworkToInterface;
+    @Mock Map<Network, LinkProperties> mMockNetworkToLinkProperties;
 
     private Context mContext;
     private TestLooper mTestLooper;
@@ -241,7 +242,7 @@ public final class ThreadNetworkControllerServiceTest {
                         mMockUserManager,
                         mConnectivityResources,
                         () -> DEFAULT_COUNTRY_CODE,
-                        mMockNetworkToInterface);
+                        mMockNetworkToLinkProperties);
         mService.setTestNetworkAgent(mMockNetworkAgent);
     }