joy/android_packages_modules_Connectivity
Watch
1
Fork
You've already forked android_packages_modules_Connectivity
 0
Code Issues Pull requests Projects Releases Wiki Activity

Merge changes from topic "log-placeholder-tethering-request" into main

Browse source 
* changes:
  Remove redundant requestedState from IpServer#enable
  Log tethering started with placeholder request
  no-op: Make TetheringRequest @NonNull in enableIpServing
This commit is contained in:
Quang Luong 2025-01-30 00:44:35 -08:00 • committed by Gerrit Code Review
commit 3e0f6b37a2
3 changed files with 101 additions and 91 deletions
Download patch file Download diff file Expand all files Collapse all files

22
Tethering/src/android/net/ip/IpServer.java
View file

@ -429,9 +429,11 @@ public class IpServer extends StateMachineShim {
return Collections.unmodifiableList(mDhcpLeases);
}
/** Enable this IpServer. IpServer state machine will be tethered or localHotspot state. */
public void enable(final int requestedState, final TetheringRequest request) {
sendMessage(CMD_TETHER_REQUESTED, requestedState, 0, request);
/**
* Enable this IpServer. IpServer state machine will be tethered or localHotspot state based on
* the connectivity scope of the TetheringRequest. */
public void enable(@NonNull final TetheringRequest request) {
sendMessage(CMD_TETHER_REQUESTED, 0, 0, request);
}
/** Stop this IpServer. After this is called this IpServer should not be used any more. */
@ -1026,11 +1028,11 @@ public class IpServer extends StateMachineShim {
mLinkProperties.setInterfaceName(mIfaceName);
}
private void maybeConfigureStaticIp(final TetheringRequest request) {
private void maybeConfigureStaticIp(@NonNull final TetheringRequest request) {
// Ignore static address configuration if they are invalid or null. In theory, static
// addresses should not be invalid here because TetheringManager do not allow caller to
// specify invalid static address configuration.
if (request == null || request.getLocalIpv4Address() == null
if (request.getLocalIpv4Address() == null
|| request.getClientStaticIpv4Address() == null || !checkStaticAddressConfiguration(
request.getLocalIpv4Address(), request.getClientStaticIpv4Address())) {
return;
@ -1053,13 +1055,13 @@ public class IpServer extends StateMachineShim {
case CMD_TETHER_REQUESTED:
mLastError = TETHER_ERROR_NO_ERROR;
mTetheringRequest = (TetheringRequest) message.obj;
switch (message.arg1) {
case STATE_LOCAL_ONLY:
maybeConfigureStaticIp((TetheringRequest) message.obj);
switch (mTetheringRequest.getConnectivityScope()) {
case CONNECTIVITY_SCOPE_LOCAL:
maybeConfigureStaticIp(mTetheringRequest);
transitionTo(mLocalHotspotState);
break;
case STATE_TETHERED:
maybeConfigureStaticIp((TetheringRequest) message.obj);
case CONNECTIVITY_SCOPE_GLOBAL:
maybeConfigureStaticIp(mTetheringRequest);
transitionTo(mTetheredState);
break;
default:

127
Tethering/src/com/android/networkstack/tethering/Tethering.java
View file

@ -28,6 +28,7 @@ import static android.net.ConnectivityManager.CONNECTIVITY_ACTION;
import static android.net.ConnectivityManager.EXTRA_NETWORK_INFO;
import static android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK;
import static android.net.TetheringManager.ACTION_TETHER_STATE_CHANGED;
import static android.net.TetheringManager.CONNECTIVITY_SCOPE_GLOBAL;
import static android.net.TetheringManager.CONNECTIVITY_SCOPE_LOCAL;
import static android.net.TetheringManager.EXTRA_ACTIVE_LOCAL_ONLY;
import static android.net.TetheringManager.EXTRA_ACTIVE_TETHER;
@ -73,6 +74,7 @@ import static com.android.networkstack.tethering.metrics.TetheringStatsLog.CORE_
import static com.android.networkstack.tethering.metrics.TetheringStatsLog.CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_P2P;
import static com.android.networkstack.tethering.metrics.TetheringStatsLog.CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_P2P_SUCCESS;
import static com.android.networkstack.tethering.metrics.TetheringStatsLog.CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_SUCCESS;
import static com.android.networkstack.tethering.metrics.TetheringStatsLog.CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_TETHER_WITH_PLACEHOLDER_REQUEST;
import static com.android.networkstack.tethering.util.TetheringMessageBase.BASE_MAIN_SM;
import android.app.usage.NetworkStatsManager;
@ -944,9 +946,9 @@ public class Tethering {
public void onAvailable(String iface) {
if (this != mBluetoothCallback) return;
final TetheringRequest request = getPendingTetheringRequest(TETHERING_BLUETOOTH);
enableIpServing(request, TETHERING_BLUETOOTH, iface,
getRequestedState(TETHERING_BLUETOOTH));
final TetheringRequest request =
getOrCreatePendingTetheringRequest(TETHERING_BLUETOOTH);
enableIpServing(request, iface);
mConfiguredBluetoothIface = iface;
}
@ -1002,9 +1004,8 @@ public class Tethering {
return;
}
final TetheringRequest request = getPendingTetheringRequest(TETHERING_ETHERNET);
enableIpServing(request, TETHERING_ETHERNET, iface,
getRequestedState(TETHERING_ETHERNET));
final TetheringRequest request = getOrCreatePendingTetheringRequest(TETHERING_ETHERNET);
enableIpServing(request, iface);
mConfiguredEthernetIface = iface;
}
@ -1025,12 +1026,8 @@ public class Tethering {
} else {
mConfiguredVirtualIface = iface;
}
final TetheringRequest request = getPendingTetheringRequest(TETHERING_VIRTUAL);
enableIpServing(
request,
TETHERING_VIRTUAL,
mConfiguredVirtualIface,
getRequestedState(TETHERING_VIRTUAL));
final TetheringRequest request = getOrCreatePendingTetheringRequest(TETHERING_VIRTUAL);
enableIpServing(request, mConfiguredVirtualIface);
} else if (mConfiguredVirtualIface != null) {
ensureIpServerStopped(mConfiguredVirtualIface);
mConfiguredVirtualIface = null;
@ -1040,12 +1037,12 @@ public class Tethering {
/**
* Create a legacy tethering request for calls to the legacy tether() API, which doesn't take an
* explicit request.
* explicit request. These are always CONNECTIVITY_SCOPE_GLOBAL, per historical behavior.
*/
private TetheringRequest createLegacyTetheringRequest(int type, int connectivityScope) {
private TetheringRequest createLegacyGlobalScopeTetheringRequest(int type) {
final TetheringRequest request = new TetheringRequest.Builder(type).build();
request.getParcel().requestType = TetheringRequest.REQUEST_TYPE_LEGACY;
request.getParcel().connectivityScope = connectivityScope;
request.getParcel().connectivityScope = CONNECTIVITY_SCOPE_GLOBAL;
return request;
}
@ -1067,7 +1064,8 @@ public class Tethering {
* Note: There are edge cases where the pending request is absent and we must temporarily
* synthesize a placeholder request, such as if stopTethering was called before link layer
* went up, or if the link layer goes up without us poking it (e.g. adb shell cmd wifi
* start-softap).
* start-softap). These placeholder requests only specify the tethering type and the
* default connectivity scope.
*/
@NonNull
private TetheringRequest getOrCreatePendingTetheringRequest(int type) {
@ -1098,7 +1096,8 @@ public class Tethering {
} catch (RemoteException e) { }
}
int result = tetherInternal(null, iface, IpServer.STATE_TETHERED);
final TetheringRequest request = createLegacyGlobalScopeTetheringRequest(type);
int result = tetherInternal(request, iface);
switch (type) {
case TETHERING_WIFI:
TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
@ -1150,11 +1149,7 @@ public class Tethering {
mHandler.post(() -> handleLegacyTether(iface, listener));
}
// TODO: is it possible to make the request @NonNull here?
// Because the IpServer doesn't actually look at (almost anything in) the request, we can just
// make callers synthesize a reasonable-looking request and make this @NonNull.
private int tetherInternal(@Nullable TetheringRequest request, String iface,
int requestedState) {
private int tetherInternal(@NonNull TetheringRequest request, String iface) {
if (DBG) Log.d(TAG, "Tethering " + iface);
TetherState tetherState = mTetherStates.get(iface);
if (tetherState == null) {
@ -1171,16 +1166,14 @@ public class Tethering {
// processed, this will be a no-op and it will not return an error.
//
// This code cannot race with untether() because they both run on the handler thread.
//
// TODO: once the request is @NonNull, it will be correct to fetch the type from it, because
// the type used to fetch (or synthesize) the request is the same as the type passed in at
// IpServer creation time.
// BUG: the connectivity scope in the request is ignored. This matches current behaviour.
// TODO: see if the connectivity scope can be made (or is already) correct, and fetch the
// requested state from there.
final int type = tetherState.ipServer.interfaceType();
mPendingTetheringRequests.remove(type);
tetherState.ipServer.enable(requestedState, request);
mPendingTetheringRequests.remove(request.getTetheringType());
tetherState.ipServer.enable(request);
if (request.getRequestType() == REQUEST_TYPE_PLACEHOLDER) {
TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
"Started tethering with placeholder request: " + request,
CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_TETHER_WITH_PLACEHOLDER_REQUEST);
}
return TETHER_ERROR_NO_ERROR;
}
@ -1239,23 +1232,6 @@ public class Tethering {
return mEntitlementMgr.isTetherProvisioningRequired(cfg);
}
// TODO: make this take a TetheringRequest instead.
private int getRequestedState(int type) {
final TetheringRequest request = mPendingTetheringRequests.get(type);
// The request could have been deleted before we had a chance to complete it.
// If so, assume that the scope is the default scope for this tethering type.
// This likely doesn't matter - if the request has been deleted, then tethering is
// likely going to be stopped soon anyway.
final int connectivityScope = (request != null)
? request.getConnectivityScope()
: TetheringRequest.getDefaultConnectivityScope(type);
return connectivityScope == CONNECTIVITY_SCOPE_LOCAL
? IpServer.STATE_LOCAL_ONLY
: IpServer.STATE_TETHERED;
}
private int getServedUsbType(boolean forNcmFunction) {
// TETHERING_NCM is only used if the device does not use NCM for regular USB tethering.
if (forNcmFunction && !mConfig.isUsingNcm()) return TETHERING_NCM;
@ -1621,17 +1597,13 @@ public class Tethering {
return mPendingTetheringRequests.get(type, null);
}
// TODO: make the request @NonNull and move the tetheringType and ipServingMode into it.
private void enableIpServing(@Nullable TetheringRequest request, int tetheringType,
String ifname, int ipServingMode) {
enableIpServing(request, tetheringType, ifname, ipServingMode, false /* isNcm */);
private void enableIpServing(@NonNull TetheringRequest request, String ifname) {
enableIpServing(request, ifname, false /* isNcm */);
}
// TODO: make the request @NonNull and move the tetheringType and ipServingMode into it.
private void enableIpServing(@Nullable TetheringRequest request, int tetheringType,
String ifname, int ipServingMode, boolean isNcm) {
ensureIpServerStartedForType(ifname, tetheringType, isNcm);
if (tetherInternal(request, ifname, ipServingMode) != TETHER_ERROR_NO_ERROR) {
private void enableIpServing(@NonNull TetheringRequest request, String ifname, boolean isNcm) {
ensureIpServerStartedForType(ifname, request.getTetheringType(), isNcm);
if (tetherInternal(request, ifname) != TETHER_ERROR_NO_ERROR) {
Log.e(TAG, "unable start tethering on iface " + ifname);
}
}
@ -1683,9 +1655,10 @@ public class Tethering {
mLog.e(ifname + " is not a tetherable iface, ignoring");
return;
}
// No need to look for active requests. There can never be explicit requests for
// No need to call getOrCreatePendingRequest. There can never be explicit requests for
// TETHERING_WIFI_P2P because enableTetheringInternal ignores that type.
enableIpServing(null, type, ifname, IpServer.STATE_LOCAL_ONLY);
final TetheringRequest request = createImplicitLocalOnlyTetheringRequest(type);
enableIpServing(request, ifname);
}
private void disableWifiP2pIpServingIfNeeded(String ifname) {
@ -1707,16 +1680,20 @@ public class Tethering {
// Map wifiIpMode values to IpServer.Callback serving states.
TetheringRequest request;
final int ipServingMode;
final int type;
switch (wifiIpMode) {
case IFACE_IP_MODE_TETHERED:
ipServingMode = IpServer.STATE_TETHERED;
type = maybeInferWifiTetheringType(ifname);
request = getPendingTetheringRequest(type);
request = getOrCreatePendingTetheringRequest(type);
// Wifi requests will always have CONNECTIVITY_SCOPE_GLOBAL, because
// TetheringRequest.Builder will not allow callers to set CONNECTIVITY_SCOPE_LOCAL
// for TETHERING_WIFI. However, if maybeInferWifiTetheringType returns a non-Wifi
// type (which could happen on a pre-T implementation of Wi-Fi if the regexps are
// misconfigured), then force the connectivity scope to global in order to match the
// historical behavior.
request.getParcel().connectivityScope = CONNECTIVITY_SCOPE_GLOBAL;
break;
case IFACE_IP_MODE_LOCAL_ONLY:
ipServingMode = IpServer.STATE_LOCAL_ONLY;
type = maybeInferWifiTetheringType(ifname);
// BUG: this request is incorrect - instead of LOHS, it will reflect whatever
// request (if any) is being processed for TETHERING_WIFI. However, this is the
@ -1724,6 +1701,21 @@ public class Tethering {
// such request b) tetherinternal doesn't look at the connectivity scope of the
// request, it takes the scope from requestedState.
request = getPendingTetheringRequest(type);
if (request == null) {
request = createImplicitLocalOnlyTetheringRequest(TETHERING_WIFI);
} else {
// If we've taken this request from the pending requests, then force the
// connectivity scope to local so we start IpServer in local-only mode (this
// matches historical behavior). This should be OK since the connectivity scope
// is only used to start IpServer in the correct mode.
// TODO: This will break fuzzy-matching logic for start/stop tethering in the
// future. Figure out how to reconcile that with this forced scope.
// Possibly ignore the connectivity scope for wifi if both requests are
// explicit, since explicit Wifi requests may only have
// CONNECTIVITY_SCOPE_GLOBAL. Or possibly, don't add any edge case and
// treat it as a different request entirely.
request.getParcel().connectivityScope = CONNECTIVITY_SCOPE_LOCAL;
}
break;
default:
mLog.e("Cannot enable IP serving in unknown WiFi mode: " + wifiIpMode);
@ -1738,7 +1730,7 @@ public class Tethering {
}
if (!TextUtils.isEmpty(ifname)) {
enableIpServing(request, type, ifname, ipServingMode);
enableIpServing(request, ifname);
} else {
mLog.e("Cannot enable IP serving on missing interface name");
}
@ -1759,7 +1751,6 @@ public class Tethering {
// for both TETHERING_USB and TETHERING_NCM, so the local-only NCM interface will be
// stopped immediately.
final int tetheringType = getServedUsbType(forNcmFunction);
final int requestedState = getRequestedState(tetheringType);
String[] ifaces = null;
try {
ifaces = mNetd.interfaceGetList();
@ -1768,11 +1759,11 @@ public class Tethering {
return;
}
final TetheringRequest request = getPendingTetheringRequest(tetheringType);
final TetheringRequest request = getOrCreatePendingTetheringRequest(tetheringType);
if (ifaces != null) {
for (String iface : ifaces) {
if (ifaceNameToType(iface) == tetheringType) {
enableIpServing(request, tetheringType, iface, requestedState, forNcmFunction);
enableIpServing(request, iface, forNcmFunction);
return;
}
}

43
Tethering/tests/unit/src/android/net/ip/IpServerTest.java
View file

@ -48,7 +48,6 @@ import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.clearInvocations;
@ -74,6 +73,7 @@ import android.net.LinkAddress;
import android.net.LinkProperties;
import android.net.MacAddress;
import android.net.RouteInfo;
import android.net.TetheringManager.TetheringRequest;
import android.net.dhcp.DhcpServerCallbacks;
import android.net.dhcp.DhcpServingParamsParcel;
import android.net.dhcp.IDhcpEventCallbacks;
@ -240,7 +240,8 @@ public class IpServerTest {
Set<LinkAddress> upstreamAddresses, boolean usingLegacyDhcp, boolean usingBpfOffload)
throws Exception {
initStateMachine(interfaceType, usingLegacyDhcp, usingBpfOffload);
dispatchCommand(IpServer.CMD_TETHER_REQUESTED, STATE_TETHERED);
dispatchCommand(IpServer.CMD_TETHER_REQUESTED, 0, 0,
createMockTetheringRequest(CONNECTIVITY_SCOPE_GLOBAL));
verify(mBpfCoordinator).addIpServer(mIpServer);
if (upstreamIface != null) {
InterfaceParams interfaceParams = mDependencies.getInterfaceParams(upstreamIface);
@ -345,7 +346,8 @@ public class IpServerTest {
public void canBeTetheredAsBluetooth() throws Exception {
initStateMachine(TETHERING_BLUETOOTH);
dispatchCommand(IpServer.CMD_TETHER_REQUESTED, STATE_TETHERED);
dispatchCommand(IpServer.CMD_TETHER_REQUESTED, 0, 0,
createMockTetheringRequest(CONNECTIVITY_SCOPE_GLOBAL));
InOrder inOrder = inOrder(mCallback, mNetd, mRoutingCoordinatorManager);
if (isAtLeastT()) {
inOrder.verify(mRoutingCoordinatorManager)
@ -400,7 +402,8 @@ public class IpServerTest {
public void canBeTetheredAsUsb() throws Exception {
initStateMachine(TETHERING_USB);
dispatchCommand(IpServer.CMD_TETHER_REQUESTED, STATE_TETHERED);
dispatchCommand(IpServer.CMD_TETHER_REQUESTED, 0, 0,
createMockTetheringRequest(CONNECTIVITY_SCOPE_GLOBAL));
InOrder inOrder = inOrder(mCallback, mNetd, mRoutingCoordinatorManager);
inOrder.verify(mRoutingCoordinatorManager).requestStickyDownstreamAddress(anyInt(),
eq(CONNECTIVITY_SCOPE_GLOBAL), any());
@ -423,7 +426,8 @@ public class IpServerTest {
public void canBeTetheredAsWifiP2p_NotUsingDedicatedIp() throws Exception {
initStateMachine(TETHERING_WIFI_P2P);
dispatchCommand(IpServer.CMD_TETHER_REQUESTED, STATE_LOCAL_ONLY);
dispatchCommand(IpServer.CMD_TETHER_REQUESTED, 0, 0,
createMockTetheringRequest(CONNECTIVITY_SCOPE_LOCAL));
InOrder inOrder = inOrder(mCallback, mNetd, mRoutingCoordinatorManager);
inOrder.verify(mRoutingCoordinatorManager).requestStickyDownstreamAddress(anyInt(),
eq(CONNECTIVITY_SCOPE_LOCAL), any());
@ -447,7 +451,8 @@ public class IpServerTest {
initStateMachine(TETHERING_WIFI_P2P, false /* usingLegacyDhcp */, DEFAULT_USING_BPF_OFFLOAD,
true /* shouldEnableWifiP2pDedicatedIp */);
dispatchCommand(IpServer.CMD_TETHER_REQUESTED, STATE_LOCAL_ONLY);
dispatchCommand(IpServer.CMD_TETHER_REQUESTED, 0, 0,
createMockTetheringRequest(CONNECTIVITY_SCOPE_LOCAL));
InOrder inOrder = inOrder(mCallback, mNetd, mRoutingCoordinatorManager);
// When using WiFi P2p dedicated IP, the IpServer just picks the IP address without
// requesting for it at RoutingCoordinatorManager.
@ -627,7 +632,8 @@ public class IpServerTest {
initStateMachine(TETHERING_USB);
doThrow(RemoteException.class).when(mNetd).tetherInterfaceAdd(IFACE_NAME);
dispatchCommand(IpServer.CMD_TETHER_REQUESTED, STATE_TETHERED);
dispatchCommand(IpServer.CMD_TETHER_REQUESTED, 0, 0,
createMockTetheringRequest(CONNECTIVITY_SCOPE_GLOBAL));
InOrder usbTeardownOrder = inOrder(mNetd, mCallback);
usbTeardownOrder.verify(mNetd).interfaceSetCfg(
argThat(cfg -> IFACE_NAME.equals(cfg.ifName)));
@ -713,7 +719,8 @@ public class IpServerTest {
@Test
public void startsDhcpServerOnNcm() throws Exception {
initStateMachine(TETHERING_NCM);
dispatchCommand(IpServer.CMD_TETHER_REQUESTED, STATE_LOCAL_ONLY);
dispatchCommand(IpServer.CMD_TETHER_REQUESTED, 0, 0,
createMockTetheringRequest(CONNECTIVITY_SCOPE_LOCAL));
dispatchTetherConnectionChanged(UPSTREAM_IFACE);
assertDhcpStarted(new IpPrefix("192.168.42.0/24"));
@ -722,7 +729,8 @@ public class IpServerTest {
@Test
public void testOnNewPrefixRequest() throws Exception {
initStateMachine(TETHERING_NCM);
dispatchCommand(IpServer.CMD_TETHER_REQUESTED, STATE_LOCAL_ONLY);
dispatchCommand(IpServer.CMD_TETHER_REQUESTED, 0, 0,
createMockTetheringRequest(CONNECTIVITY_SCOPE_LOCAL));
final IDhcpEventCallbacks eventCallbacks;
final ArgumentCaptor<IDhcpEventCallbacks> dhcpEventCbsCaptor =
@ -911,7 +919,8 @@ public class IpServerTest {
doNothing().when(mDependencies).makeDhcpServer(any(), mDhcpParamsCaptor.capture(),
cbCaptor.capture());
initStateMachine(TETHERING_WIFI);
dispatchCommand(IpServer.CMD_TETHER_REQUESTED, STATE_TETHERED);
dispatchCommand(IpServer.CMD_TETHER_REQUESTED, 0, 0,
createMockTetheringRequest(CONNECTIVITY_SCOPE_GLOBAL));
verify(mDhcpServer, never()).startWithCallbacks(any(), any());
// No stop dhcp server because dhcp server is not created yet.
@ -957,14 +966,22 @@ public class IpServerTest {
assertDhcpServingParams(mDhcpParamsCaptor.getValue(), expectedPrefix);
}
private TetheringRequest createMockTetheringRequest(int connectivityScope) {
TetheringRequest request = mock(TetheringRequest.class);
when(request.getConnectivityScope()).thenReturn(connectivityScope);
return request;
}
/**
* Send a command to the state machine under test, and run the event loop to idle.
*
* @param command One of the IpServer.CMD_* constants.
* @param arg1 An additional argument to pass.
* @param arg1 An additional argument to pass.
* @param arg2 An additional argument to pass.
* @param obj An additional object to pass.
*/
private void dispatchCommand(int command, int arg1) {
mIpServer.sendMessage(command, arg1);
private void dispatchCommand(int command, int arg1, int arg2, Object obj) {
mIpServer.sendMessage(command, arg1, arg2, obj);
mLooper.dispatchAll();
}