Merge "[Thread] refactor setInfraLinkState" into main am: adc778d038
Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/3273892
Change-Id: Ie5bf40b7b90c65fb9357ea4a1883150b3b29f1eb
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/thread/service/java/com/android/server/thread/NsdPublisher.java b/thread/service/java/com/android/server/thread/NsdPublisher.java
index 38c77bf..9697c02 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.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 @@
// 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 @@
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 @@
+ 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) {
diff --git a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java
index 1f9a10d..f7da644 100644
--- a/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java
+++ b/thread/service/java/com/android/server/thread/ThreadNetworkControllerService.java
@@ -212,7 +212,7 @@
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 @@
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 @@
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 @@
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 @@
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 @@
if (mUpstreamNetworkCallback == null) {
throw new AssertionError("The upstream network request null.");
}
- mNetworkToInterface.clear();
+ mNetworkToLinkProperties.clear();
mConnectivityManager.unregisterNetworkCallback(mUpstreamNetworkCallback);
mUpstreamNetworkCallback = null;
}
@@ -721,20 +721,19 @@
@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 @@
public void onLost(@NonNull Network network) {
checkOnHandlerThread();
LOG.i("Thread network is lost: " + network);
- disableBorderRouting();
+ setInfraLinkState(newInfraLinkStateBuilder().build());
}
@Override
@@ -764,13 +763,15 @@
+ 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 @@
}
}
- 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 + " -> " + infraLinkState);
- mInfraLinkState = infraLinkState;
+ 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;
+ }
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 @@
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 1959198..d52191a 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 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 @@
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);
}
}
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 a723d07..d8cdbc4 100644
--- a/thread/tests/unit/src/com/android/server/thread/ThreadNetworkControllerServiceTest.java
+++ b/thread/tests/unit/src/com/android/server/thread/ThreadNetworkControllerServiceTest.java
@@ -64,6 +64,7 @@
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 @@
@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 @@
mMockUserManager,
mConnectivityResources,
() -> DEFAULT_COUNTRY_CODE,
- mMockNetworkToInterface);
+ mMockNetworkToLinkProperties);
mService.setTestNetworkAgent(mMockNetworkAgent);
}