From 389bc7f13ecdfcb80525b994efb5f69d2b16728d Mon Sep 17 00:00:00 2001 From: Handa Wang Date: Fri, 18 Oct 2024 01:29:53 +0000 Subject: [PATCH] Split requestDownstreamAddress into two methods This CL splits requestDownstreamAddress into two methods: requestStickyDownstreamAddress and requestDownstreamAddress. - The requestStickyDownstreamAddress is equivalent to the former requestDownstreamAddress when useLastAddress=true. - The new requestDownstreamAddress method allows allocating an address without providing the interface type and scope, which is Thread NAT64's use case. Bug: 369282577 Change-Id: Ib839d121c7b46c492f2c4c1c61b52b5d017fa055 --- Tethering/src/android/net/ip/IpServer.java | 8 ++- .../unit/src/android/net/ip/IpServerTest.java | 39 ++++++----- .../PrivateAddressCoordinatorTest.java | 16 +++-- .../net/module/util/IRoutingCoordinator.aidl | 15 +++-- .../util/PrivateAddressCoordinator.java | 66 +++++++++---------- .../util/RoutingCoordinatorManager.java | 24 +++++-- .../util/RoutingCoordinatorService.java | 29 ++++++-- 7 files changed, 121 insertions(+), 76 deletions(-) diff --git a/Tethering/src/android/net/ip/IpServer.java b/Tethering/src/android/net/ip/IpServer.java index f60d7a1698..a0604f20d9 100644 --- a/Tethering/src/android/net/ip/IpServer.java +++ b/Tethering/src/android/net/ip/IpServer.java @@ -725,8 +725,12 @@ public class IpServer extends StateMachineShim { if (shouldUseWifiP2pDedicatedIp()) return new LinkAddress(LEGACY_WIFI_P2P_IFACE_ADDRESS); - return mRoutingCoordinator.requestDownstreamAddress(mInterfaceType, scope, useLastAddress, - mIpv4PrefixRequest); + if (useLastAddress) { + return mRoutingCoordinator.requestStickyDownstreamAddress(mInterfaceType, scope, + mIpv4PrefixRequest); + } + + return mRoutingCoordinator.requestDownstreamAddress(mIpv4PrefixRequest); } private boolean startIPv6() { diff --git a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java index 8f5e6c4d1c..680e81db05 100644 --- a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java +++ b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java @@ -258,8 +258,8 @@ public class IpServerTest { mIpServer, interfaceParams.index, upstreamPrefixes); } reset(mNetd, mBpfCoordinator, mCallback, mRoutingCoordinatorManager); - when(mRoutingCoordinatorManager.requestDownstreamAddress(anyInt(), anyInt(), - anyBoolean(), any())).thenReturn(mTestAddress); + when(mRoutingCoordinatorManager.requestStickyDownstreamAddress(anyInt(), anyInt(), + any())).thenReturn(mTestAddress); } @SuppressWarnings("DoNotCall") // Ignore warning for synchronous to call to Thread.run() @@ -280,8 +280,9 @@ public class IpServerTest { @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); when(mSharedLog.forSubComponent(anyString())).thenReturn(mSharedLog); - when(mRoutingCoordinatorManager.requestDownstreamAddress(anyInt(), anyInt(), - anyBoolean(), any())).thenReturn(mTestAddress); + when(mRoutingCoordinatorManager.requestStickyDownstreamAddress(anyInt(), anyInt(), + any())).thenReturn(mTestAddress); + when(mRoutingCoordinatorManager.requestDownstreamAddress(any())).thenReturn(mTestAddress); when(mTetherConfig.isBpfOffloadEnabled()).thenReturn(DEFAULT_USING_BPF_OFFLOAD); when(mTetherConfig.useLegacyDhcpServer()).thenReturn(false /* default value */); @@ -348,11 +349,11 @@ public class IpServerTest { InOrder inOrder = inOrder(mCallback, mNetd, mRoutingCoordinatorManager); if (isAtLeastT()) { inOrder.verify(mRoutingCoordinatorManager) - .requestDownstreamAddress( + .requestStickyDownstreamAddress( eq(TETHERING_BLUETOOTH), eq(CONNECTIVITY_SCOPE_GLOBAL), - eq(true), any()); + inOrder.verify(mRoutingCoordinatorManager, never()).requestDownstreamAddress(any()); inOrder.verify(mNetd).interfaceSetCfg(argThat(cfg -> IFACE_NAME.equals(cfg.ifName) && assertContainsFlag(cfg.flags, IF_STATE_UP))); } @@ -401,8 +402,9 @@ public class IpServerTest { dispatchCommand(IpServer.CMD_TETHER_REQUESTED, STATE_TETHERED); InOrder inOrder = inOrder(mCallback, mNetd, mRoutingCoordinatorManager); - inOrder.verify(mRoutingCoordinatorManager).requestDownstreamAddress(anyInt(), - eq(CONNECTIVITY_SCOPE_GLOBAL), eq(true), any()); + inOrder.verify(mRoutingCoordinatorManager).requestStickyDownstreamAddress(anyInt(), + eq(CONNECTIVITY_SCOPE_GLOBAL), any()); + inOrder.verify(mRoutingCoordinatorManager, never()).requestDownstreamAddress(any()); inOrder.verify(mNetd).interfaceSetCfg(argThat(cfg -> IFACE_NAME.equals(cfg.ifName) && assertContainsFlag(cfg.flags, IF_STATE_UP))); inOrder.verify(mNetd).tetherInterfaceAdd(IFACE_NAME); @@ -423,8 +425,9 @@ public class IpServerTest { dispatchCommand(IpServer.CMD_TETHER_REQUESTED, STATE_LOCAL_ONLY); InOrder inOrder = inOrder(mCallback, mNetd, mRoutingCoordinatorManager); - inOrder.verify(mRoutingCoordinatorManager).requestDownstreamAddress(anyInt(), - eq(CONNECTIVITY_SCOPE_LOCAL), eq(true), any()); + inOrder.verify(mRoutingCoordinatorManager).requestStickyDownstreamAddress(anyInt(), + eq(CONNECTIVITY_SCOPE_LOCAL), any()); + inOrder.verify(mRoutingCoordinatorManager, never()).requestDownstreamAddress(any()); inOrder.verify(mNetd).interfaceSetCfg(argThat(cfg -> IFACE_NAME.equals(cfg.ifName) && assertNotContainsFlag(cfg.flags, IF_STATE_UP))); inOrder.verify(mNetd).tetherInterfaceAdd(IFACE_NAME); @@ -449,7 +452,8 @@ public class IpServerTest { // When using WiFi P2p dedicated IP, the IpServer just picks the IP address without // requesting for it at RoutingCoordinatorManager. inOrder.verify(mRoutingCoordinatorManager, never()) - .requestDownstreamAddress(anyInt(), anyInt(), anyBoolean(), any()); + .requestStickyDownstreamAddress(anyInt(), anyInt(), any()); + inOrder.verify(mRoutingCoordinatorManager, never()).requestDownstreamAddress(any()); inOrder.verify(mNetd).interfaceSetCfg(argThat(cfg -> IFACE_NAME.equals(cfg.ifName) && assertNotContainsFlag(cfg.flags, IF_STATE_UP))); inOrder.verify(mNetd).tetherInterfaceAdd(IFACE_NAME); @@ -731,8 +735,9 @@ public class IpServerTest { final ArgumentCaptor lpCaptor = ArgumentCaptor.forClass(LinkProperties.class); InOrder inOrder = inOrder(mNetd, mCallback, mRoutingCoordinatorManager); - inOrder.verify(mRoutingCoordinatorManager).requestDownstreamAddress(anyInt(), - eq(CONNECTIVITY_SCOPE_LOCAL), eq(true), any()); + inOrder.verify(mRoutingCoordinatorManager).requestStickyDownstreamAddress(anyInt(), + eq(CONNECTIVITY_SCOPE_LOCAL), any()); + inOrder.verify(mRoutingCoordinatorManager, never()).requestDownstreamAddress(any()); inOrder.verify(mNetd).networkAddInterface(INetd.LOCAL_NET_ID, IFACE_NAME); // One for ipv4 route, one for ipv6 link local route. inOrder.verify(mNetd, times(2)).networkAddRoute(eq(INetd.LOCAL_NET_ID), eq(IFACE_NAME), @@ -745,13 +750,13 @@ public class IpServerTest { // Simulate the DHCP server receives DHCPDECLINE on MirrorLink and then signals // onNewPrefixRequest callback. final LinkAddress newAddress = new LinkAddress("192.168.100.125/24"); - when(mRoutingCoordinatorManager.requestDownstreamAddress(anyInt(), anyInt(), - anyBoolean(), any())).thenReturn(newAddress); + when(mRoutingCoordinatorManager.requestDownstreamAddress(any())).thenReturn(newAddress); eventCallbacks.onNewPrefixRequest(new IpPrefix("192.168.42.0/24")); mLooper.dispatchAll(); - inOrder.verify(mRoutingCoordinatorManager).requestDownstreamAddress(anyInt(), - eq(CONNECTIVITY_SCOPE_LOCAL), eq(false), any()); + inOrder.verify(mRoutingCoordinatorManager, never()) + .requestStickyDownstreamAddress(anyInt(), anyInt(), any()); + inOrder.verify(mRoutingCoordinatorManager).requestDownstreamAddress(any()); inOrder.verify(mNetd).tetherApplyDnsInterfaces(); inOrder.verify(mCallback).updateLinkProperties(eq(mIpServer), lpCaptor.capture()); verifyNoMoreInteractions(mCallback); diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/PrivateAddressCoordinatorTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/PrivateAddressCoordinatorTest.java index 1ab5766113..08e4ac2329 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/PrivateAddressCoordinatorTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/PrivateAddressCoordinatorTest.java @@ -136,12 +136,16 @@ public final class PrivateAddressCoordinatorTest { private LinkAddress requestDownstreamAddress( final IpServer ipServer, int scope, boolean useLastAddress) throws Exception { - final LinkAddress address = - mPrivateAddressCoordinator.requestDownstreamAddress( - ipServer.interfaceType(), - scope, - useLastAddress, - ipServer.getIpv4PrefixRequest()); + LinkAddress address; + if (useLastAddress) { + address = + mPrivateAddressCoordinator.requestStickyDownstreamAddress( + ipServer.interfaceType(), scope, ipServer.getIpv4PrefixRequest()); + } else { + address = + mPrivateAddressCoordinator.requestDownstreamAddress( + ipServer.getIpv4PrefixRequest()); + } when(ipServer.getAddress()).thenReturn(address); return address; } diff --git a/staticlibs/device/com/android/net/module/util/IRoutingCoordinator.aidl b/staticlibs/device/com/android/net/module/util/IRoutingCoordinator.aidl index 097824f331..7688e6a513 100644 --- a/staticlibs/device/com/android/net/module/util/IRoutingCoordinator.aidl +++ b/staticlibs/device/com/android/net/module/util/IRoutingCoordinator.aidl @@ -115,20 +115,27 @@ interface IRoutingCoordinator { void maybeRemoveDeprecatedUpstreams(); /** - * Request an IPv4 address for the downstream. + * Request an IPv4 address for the downstream. Return the last time used address for the + * provided (interfaceType, scope) pair if possible. * * @param interfaceType the Tethering type (see TetheringManager#TETHERING_*). * @param scope CONNECTIVITY_SCOPE_GLOBAL or CONNECTIVITY_SCOPE_LOCAL - * @param useLastAddress whether to use the last address * @param request a {@link IIpv4PrefixRequest} to report conflicts * @return an IPv4 address allocated for the downstream, could be null */ @nullable - LinkAddress requestDownstreamAddress( + LinkAddress requestStickyDownstreamAddress( in int interfaceType, in int scope, - in boolean useLastAddress, in IIpv4PrefixRequest request); + /** + * Request an IPv4 address for the downstream. + * + * @param request a {@link IIpv4PrefixRequest} to report conflicts + * @return an IPv4 address allocated for the downstream, could be null + */ + @nullable + LinkAddress requestDownstreamAddress(in IIpv4PrefixRequest request); /** Release the IPv4 address allocated for the downstream. */ void releaseDownstream(in IIpv4PrefixRequest request); diff --git a/staticlibs/device/com/android/net/module/util/PrivateAddressCoordinator.java b/staticlibs/device/com/android/net/module/util/PrivateAddressCoordinator.java index 990358d39e..7fcbd4e382 100644 --- a/staticlibs/device/com/android/net/module/util/PrivateAddressCoordinator.java +++ b/staticlibs/device/com/android/net/module/util/PrivateAddressCoordinator.java @@ -78,7 +78,7 @@ public class PrivateAddressCoordinator { private final ArrayMap> mUpstreamPrefixMap; // The downstreams are indexed by Ipv4PrefixRequest, which is a wrapper of the Binder object of // IIpv4PrefixRequest. - private final ArrayMap mDownstreams; + private final ArrayMap mDownstreams; private static final String LEGACY_WIFI_P2P_IFACE_ADDRESS = "192.168.49.1/24"; private static final String LEGACY_BLUETOOTH_IFACE_ADDRESS = "192.168.44.1/24"; private final List mTetheringPrefixes; @@ -168,10 +168,10 @@ public class PrivateAddressCoordinator { } private void handleMaybePrefixConflict(final List prefixes) { - for (Map.Entry entry : mDownstreams.entrySet()) { + for (Map.Entry entry : mDownstreams.entrySet()) { final Ipv4PrefixRequest request = entry.getKey(); - final Downstream downstream = entry.getValue(); - final IpPrefix target = asIpPrefix(downstream.getAddress()); + final LinkAddress downstream = entry.getValue(); + final IpPrefix target = asIpPrefix(downstream); for (IpPrefix source : prefixes) { if (isConflictPrefix(source, target)) { @@ -209,12 +209,14 @@ public class PrivateAddressCoordinator { // TetheringRequest has been set a static IPv4 address. /** - * Pick a random available address and mark its prefix as in use for the provided IpServer, - * returns null if there is no available address. + * Request a downstream address for the provided IIpv4PrefixRequest. + * + * This method will first try to return the last time used address for the provided + * (interfaceType, scope) pair if possible. If not, it will pick a random available address and + * mark its prefix as in use for the provided IIpv4PrefixRequest. */ @Nullable - public LinkAddress requestDownstreamAddress(int interfaceType, final int scope, - boolean useLastAddress, + public LinkAddress requestStickyDownstreamAddress(int interfaceType, final int scope, IIpv4PrefixRequest request) { final Ipv4PrefixRequest wrappedRequest = new Ipv4PrefixRequest(request); final AddressKey addrKey = new AddressKey(interfaceType, scope); @@ -222,20 +224,32 @@ public class PrivateAddressCoordinator { // Once tethering could support multiple interface with the same type, // TetheringSoftApCallback would need to handle it among others. final LinkAddress cachedAddress = mCachedAddresses.get(addrKey); - if (useLastAddress && cachedAddress != null - && !isConflictWithUpstream(asIpPrefix(cachedAddress))) { - mDownstreams.put(wrappedRequest, new Downstream(interfaceType, cachedAddress)); + if (cachedAddress != null && !isConflictWithUpstream(asIpPrefix(cachedAddress))) { + mDownstreams.put(wrappedRequest, cachedAddress); return cachedAddress; } + final LinkAddress newAddress = requestDownstreamAddress(request); + if (newAddress != null) { + mCachedAddresses.put(addrKey, newAddress); + } + return newAddress; + } + + /** + * Pick a random available address and mark its prefix as in use for the provided + * IIpv4PrefixRequest. Return null if there is no available address. + */ + @Nullable + public LinkAddress requestDownstreamAddress(IIpv4PrefixRequest request) { + final Ipv4PrefixRequest wrappedRequest = new Ipv4PrefixRequest(request); final int prefixIndex = getRandomPrefixIndex(); for (int i = 0; i < mTetheringPrefixes.size(); i++) { final IpPrefix prefixRange = mTetheringPrefixes.get( (prefixIndex + i) % mTetheringPrefixes.size()); final LinkAddress newAddress = chooseDownstreamAddress(prefixRange); if (newAddress != null) { - mDownstreams.put(wrappedRequest, new Downstream(interfaceType, newAddress)); - mCachedAddresses.put(addrKey, newAddress); + mDownstreams.put(wrappedRequest, newAddress); return newAddress; } } @@ -379,8 +393,8 @@ public class PrivateAddressCoordinator { // IpServer may use manually-defined address (mStaticIpv4ServerAddr) which does not include // in mCachedAddresses. - for (Downstream downstream : mDownstreams.values()) { - final IpPrefix target = asIpPrefix(downstream.getAddress()); + for (LinkAddress downstream : mDownstreams.values()) { + final IpPrefix target = asIpPrefix(downstream); if (isConflictPrefix(prefix, target)) return target; } @@ -417,24 +431,6 @@ public class PrivateAddressCoordinator { } } - private static final class Downstream { - private final int mInterfaceType; - private final LinkAddress mAddress; - - private Downstream(int interfaceType, LinkAddress address) { - mInterfaceType = interfaceType; - mAddress = address; - } - - public int getInterfaceType() { - return mInterfaceType; - } - - public LinkAddress getAddress() { - return mAddress; - } - } - private static class AddressKey { private final int mTetheringType; private final int mScope; @@ -481,8 +477,8 @@ public class PrivateAddressCoordinator { pw.println("mDownstreams:"); pw.increaseIndent(); - for (Downstream downstream : mDownstreams.values()) { - pw.println(downstream.getInterfaceType() + " - " + downstream.getAddress()); + for (LinkAddress downstream : mDownstreams.values()) { + pw.println(downstream); } pw.decreaseIndent(); diff --git a/staticlibs/device/com/android/net/module/util/RoutingCoordinatorManager.java b/staticlibs/device/com/android/net/module/util/RoutingCoordinatorManager.java index 9ea0947efb..f5af30c6c5 100644 --- a/staticlibs/device/com/android/net/module/util/RoutingCoordinatorManager.java +++ b/staticlibs/device/com/android/net/module/util/RoutingCoordinatorManager.java @@ -195,23 +195,35 @@ public class RoutingCoordinatorManager { } /** - * Request an IPv4 address for the downstream. + * Request an IPv4 address for the downstream. Return the last time used address for the + * provided (interfaceType, scope) pair if possible. * * @param interfaceType the Tethering type (see TetheringManager#TETHERING_*). * @param scope CONNECTIVITY_SCOPE_GLOBAL or CONNECTIVITY_SCOPE_LOCAL - * @param useLastAddress whether to use the last address * @param request a {@link IIpv4PrefixRequest} to report conflicts * @return an IPv4 address allocated for the downstream, could be null */ @Nullable - public LinkAddress requestDownstreamAddress( + public LinkAddress requestStickyDownstreamAddress( int interfaceType, int scope, - boolean useLastAddress, IIpv4PrefixRequest request) { try { - return mService.requestDownstreamAddress( - interfaceType, scope, useLastAddress, request); + return mService.requestStickyDownstreamAddress(interfaceType, scope, request); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + + /** + * Request an IPv4 address for the downstream. + * + * @param request a {@link IIpv4PrefixRequest} to report conflicts + * @return an IPv4 address allocated for the downstream, could be null + */ + public LinkAddress requestDownstreamAddress(IIpv4PrefixRequest request) { + try { + return mService.requestDownstreamAddress(request); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } diff --git a/staticlibs/device/com/android/net/module/util/RoutingCoordinatorService.java b/staticlibs/device/com/android/net/module/util/RoutingCoordinatorService.java index d16c234859..51eb47c121 100644 --- a/staticlibs/device/com/android/net/module/util/RoutingCoordinatorService.java +++ b/staticlibs/device/com/android/net/module/util/RoutingCoordinatorService.java @@ -284,23 +284,40 @@ public class RoutingCoordinatorService extends IRoutingCoordinator.Stub { } /** - * Request an IPv4 address for the downstream. + * Request an IPv4 address for the downstream. Return the last time used address for the + * provided (interfaceType, scope) pair if possible. * * @param interfaceType the Tethering type (see TetheringManager#TETHERING_*). * @param scope CONNECTIVITY_SCOPE_GLOBAL or CONNECTIVITY_SCOPE_LOCAL - * @param useLastAddress whether to use the last address * @param request a {@link IIpv4PrefixRequest} to report conflicts * @return an IPv4 address allocated for the downstream, could be null */ @Override - public LinkAddress requestDownstreamAddress(int interfaceType, int scope, - boolean useLastAddress, IIpv4PrefixRequest request) { + public LinkAddress requestStickyDownstreamAddress(int interfaceType, int scope, + IIpv4PrefixRequest request) { Objects.requireNonNull(request); return BinderUtils.withCleanCallingIdentity( () -> { synchronized (mPrivateAddressCoordinatorLock) { - return mPrivateAddressCoordinator.requestDownstreamAddress( - interfaceType, scope, useLastAddress, request); + return mPrivateAddressCoordinator.requestStickyDownstreamAddress( + interfaceType, scope, request); + } + }); + } + + /** + * Request an IPv4 address for the downstream. + * + * @param request a {@link IIpv4PrefixRequest} to report conflicts + * @return an IPv4 address allocated for the downstream, could be null + */ + @Override + public LinkAddress requestDownstreamAddress(IIpv4PrefixRequest request) { + Objects.requireNonNull(request); + return BinderUtils.withCleanCallingIdentity( + () -> { + synchronized (mPrivateAddressCoordinatorLock) { + return mPrivateAddressCoordinator.requestDownstreamAddress(request); } }); }