LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge "Fix an infinite loop with network offers"

Browse Source
This commit is contained in:
Chalard Jean
2021-08-20 06:17:31 +00:00
committed by Gerrit Code Review
parent ad8fb7fce4 bb902a5fee
commit 77992bbfbb
5 changed files with 221 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
service/src/com/android/server/ConnectivityService.java
View File

@@ -4622,9 +4622,16 @@ public class ConnectivityService extends IConnectivityManager.Stub
}
private void updateAvoidBadWifi() {
ensureRunningOnConnectivityServiceThread();
// Agent info scores and offer scores depend on whether cells yields to bad wifi.
for (final NetworkAgentInfo nai : mNetworkAgentInfos) {
nai.updateScoreForNetworkAgentUpdate();
}
// UpdateOfferScore will update mNetworkOffers inline, so make a copy first.
final ArrayList<NetworkOfferInfo> offersToUpdate = new ArrayList<>(mNetworkOffers);
for (final NetworkOfferInfo noi : offersToUpdate) {
updateOfferScore(noi.offer);
}
rematchAllNetworksAndRequests();
}
@@ -6413,11 +6420,23 @@ public class ConnectivityService extends IConnectivityManager.Stub
Objects.requireNonNull(score);
Objects.requireNonNull(caps);
Objects.requireNonNull(callback);
final boolean yieldToBadWiFi = caps.hasTransport(TRANSPORT_CELLULAR) && !avoidBadWifi();
final NetworkOffer offer = new NetworkOffer(
FullScore.makeProspectiveScore(score, caps), caps, callback, providerId);
FullScore.makeProspectiveScore(score, caps, yieldToBadWiFi),
caps, callback, providerId);
mHandler.sendMessage(mHandler.obtainMessage(EVENT_REGISTER_NETWORK_OFFER, offer));
}
private void updateOfferScore(final NetworkOffer offer) {
final boolean yieldToBadWiFi =
offer.caps.hasTransport(TRANSPORT_CELLULAR) && !avoidBadWifi();
final NetworkOffer newOffer = new NetworkOffer(
offer.score.withYieldToBadWiFi(yieldToBadWiFi),
offer.caps, offer.callback, offer.providerId);
if (offer.equals(newOffer)) return;
handleRegisterNetworkOffer(newOffer);
}
@Override
public void unofferNetwork(@NonNull final INetworkOfferCallback callback) {
mHandler.sendMessage(mHandler.obtainMessage(EVENT_UNREGISTER_NETWORK_OFFER, callback));
@@ -6838,6 +6857,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
* @param newOffer The new offer. If the callback member is the same as an existing
* offer, it is an update of that offer.
*/
// TODO : rename this to handleRegisterOrUpdateNetworkOffer
private void handleRegisterNetworkOffer(@NonNull final NetworkOffer newOffer) {
ensureRunningOnConnectivityServiceThread();
if (!isNetworkProviderWithIdRegistered(newOffer.providerId)) {
@@ -6852,6 +6872,14 @@ public class ConnectivityService extends IConnectivityManager.Stub
if (null != existingOffer) {
handleUnregisterNetworkOffer(existingOffer);
newOffer.migrateFrom(existingOffer.offer);
if (DBG) {
// handleUnregisterNetworkOffer has already logged the old offer
log("update offer from providerId " + newOffer.providerId + " new : " + newOffer);
}
} else {
if (DBG) {
log("register offer from providerId " + newOffer.providerId + " : " + newOffer);
}
}
final NetworkOfferInfo noi = new NetworkOfferInfo(newOffer);
try {
@@ -6866,6 +6894,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
private void handleUnregisterNetworkOffer(@NonNull final NetworkOfferInfo noi) {
ensureRunningOnConnectivityServiceThread();
if (DBG) {
log("unregister offer from providerId " + noi.offer.providerId + " : " + noi.offer);
}
mNetworkOffers.remove(noi);
noi.offer.callback.asBinder().unlinkToDeath(noi, 0 /* flags */);
}

14
service/src/com/android/server/connectivity/FullScore.java
View File

@@ -183,7 +183,7 @@ public class FullScore {
* @return a FullScore appropriate for comparing to actual network's scores.
*/
public static FullScore makeProspectiveScore(@NonNull final NetworkScore score,
@NonNull final NetworkCapabilities caps) {
@NonNull final NetworkCapabilities caps, final boolean yieldToBadWiFi) {
// If the network offers Internet access, it may validate.
final boolean mayValidate = caps.hasCapability(NET_CAPABILITY_INTERNET);
// VPN transports are known in advance.
@@ -197,8 +197,6 @@ public class FullScore {
final boolean everUserSelected = false;
// Don't assume the user will accept unvalidated connectivity.
final boolean acceptUnvalidated = false;
// Don't assume clinging to bad wifi
final boolean yieldToBadWiFi = false;
// A prospective score is invincible if the legacy int in the filter is over the maximum
// score.
final boolean invincible = score.getLegacyInt() > NetworkRanker.LEGACY_INT_MAX;
@@ -258,6 +256,16 @@ public class FullScore {
keepConnectedReason);
}
/**
* Returns this score but with the specified yield to bad wifi policy.
*/
public FullScore withYieldToBadWiFi(final boolean newYield) {
return new FullScore(mLegacyInt,
newYield ? mPolicies | (1L << POLICY_YIELD_TO_BAD_WIFI)
: mPolicies & ~(1L << POLICY_YIELD_TO_BAD_WIFI),
mKeepConnectedReason);
}
/**
* Returns this score but validated.
*/

1
service/src/com/android/server/connectivity/NetworkAgentInfo.java
View File

@@ -1187,6 +1187,7 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo>, NetworkRa
? " underlying{" + Arrays.toString(declaredUnderlyingNetworks) + "}" : "")
+ " lp{" + linkProperties + "}"
+ " nc{" + networkCapabilities + "}"
+ " factorySerialNumber=" + factorySerialNumber
+ "}";
}

21
tests/integration/util/com/android/server/NetworkAgentWrapper.java
View File

@@ -83,6 +83,12 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork {
public NetworkAgentWrapper(int transport, LinkProperties linkProperties,
NetworkCapabilities ncTemplate, Context context) throws Exception {
this(transport, linkProperties, ncTemplate, null /* provider */, context);
}
public NetworkAgentWrapper(int transport, LinkProperties linkProperties,
NetworkCapabilities ncTemplate, NetworkProvider provider,
Context context) throws Exception {
final int type = transportToLegacyType(transport);
final String typeName = ConnectivityManager.getNetworkTypeName(type);
mNetworkCapabilities = (ncTemplate != null) ? ncTemplate : new NetworkCapabilities();
@@ -124,12 +130,12 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork {
.setLegacyTypeName(typeName)
.setLegacyExtraInfo(extraInfo)
.build();
mNetworkAgent = makeNetworkAgent(linkProperties, mNetworkAgentConfig);
mNetworkAgent = makeNetworkAgent(linkProperties, mNetworkAgentConfig, provider);
}
protected InstrumentedNetworkAgent makeNetworkAgent(LinkProperties linkProperties,
final NetworkAgentConfig nac) throws Exception {
return new InstrumentedNetworkAgent(this, linkProperties, nac);
final NetworkAgentConfig nac, NetworkProvider provider) throws Exception {
return new InstrumentedNetworkAgent(this, linkProperties, nac, provider);
}
public static class InstrumentedNetworkAgent extends NetworkAgent {
@@ -138,10 +144,15 @@ public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork {
public InstrumentedNetworkAgent(NetworkAgentWrapper wrapper, LinkProperties lp,
NetworkAgentConfig nac) {
this(wrapper, lp, nac, null /* provider */);
}
public InstrumentedNetworkAgent(NetworkAgentWrapper wrapper, LinkProperties lp,
NetworkAgentConfig nac, NetworkProvider provider) {
super(wrapper.mContext, wrapper.mHandlerThread.getLooper(), wrapper.mLogTag,
wrapper.mNetworkCapabilities, lp, wrapper.mScore, nac,
new NetworkProvider(wrapper.mContext, wrapper.mHandlerThread.getLooper(),
PROVIDER_NAME));
null != provider ? provider : new NetworkProvider(wrapper.mContext,
wrapper.mHandlerThread.getLooper(), PROVIDER_NAME));
mWrapper = wrapper;
register();
}

182
tests/unit/java/com/android/server/ConnectivityServiceTest.java
View File

@@ -241,6 +241,7 @@ import android.net.NetworkInfo;
import android.net.NetworkInfo.DetailedState;
import android.net.NetworkPolicyManager;
import android.net.NetworkPolicyManager.NetworkPolicyCallback;
import android.net.NetworkProvider;
import android.net.NetworkRequest;
import android.net.NetworkScore;
import android.net.NetworkSpecifier;
@@ -338,6 +339,7 @@ import com.android.testutils.ExceptionUtils;
import com.android.testutils.HandlerUtils;
import com.android.testutils.RecorderCallback.CallbackEntry;
import com.android.testutils.TestableNetworkCallback;
import com.android.testutils.TestableNetworkOfferCallback;
import org.junit.After;
import org.junit.Before;
@@ -816,17 +818,22 @@ public class ConnectivityServiceTest {
private String mRedirectUrl;
TestNetworkAgentWrapper(int transport) throws Exception {
this(transport, new LinkProperties(), null);
this(transport, new LinkProperties(), null /* ncTemplate */, null /* provider */);
}
TestNetworkAgentWrapper(int transport, LinkProperties linkProperties)
throws Exception {
this(transport, linkProperties, null);
this(transport, linkProperties, null /* ncTemplate */, null /* provider */);
}
private TestNetworkAgentWrapper(int transport, LinkProperties linkProperties,
NetworkCapabilities ncTemplate) throws Exception {
super(transport, linkProperties, ncTemplate, mServiceContext);
this(transport, linkProperties, ncTemplate, null /* provider */);
}
private TestNetworkAgentWrapper(int transport, LinkProperties linkProperties,
NetworkCapabilities ncTemplate, NetworkProvider provider) throws Exception {
super(transport, linkProperties, ncTemplate, provider, mServiceContext);
// Waits for the NetworkAgent to be registered, which includes the creation of the
// NetworkMonitor.
@@ -835,29 +842,12 @@ public class ConnectivityServiceTest {
HandlerUtils.waitForIdle(ConnectivityThread.get(), TIMEOUT_MS);
}
@Override
protected InstrumentedNetworkAgent makeNetworkAgent(LinkProperties linkProperties,
NetworkAgentConfig nac) throws Exception {
mNetworkMonitor = mock(INetworkMonitor.class);
class TestInstrumentedNetworkAgent extends InstrumentedNetworkAgent {
TestInstrumentedNetworkAgent(NetworkAgentWrapper wrapper, LinkProperties lp,
NetworkAgentConfig nac, NetworkProvider provider) {
super(wrapper, lp, nac, provider);
}
final Answer validateAnswer = inv -> {
new Thread(ignoreExceptions(this::onValidationRequested)).start();
return null;
};
doAnswer(validateAnswer).when(mNetworkMonitor).notifyNetworkConnected(any(), any());
doAnswer(validateAnswer).when(mNetworkMonitor).forceReevaluation(anyInt());
final ArgumentCaptor<Network> nmNetworkCaptor = ArgumentCaptor.forClass(Network.class);
final ArgumentCaptor<INetworkMonitorCallbacks> nmCbCaptor =
ArgumentCaptor.forClass(INetworkMonitorCallbacks.class);
doNothing().when(mNetworkStack).makeNetworkMonitor(
nmNetworkCaptor.capture(),
any() /* name */,
nmCbCaptor.capture());
final InstrumentedNetworkAgent na =
new InstrumentedNetworkAgent(this, linkProperties, nac) {
@Override
public void networkStatus(int status, String redirectUrl) {
mRedirectUrl = redirectUrl;
@@ -881,8 +871,32 @@ public class ConnectivityServiceTest {
super.onNetworkDestroyed();
if (mDisconnectedCallback != null) mDisconnectedCallback.run();
}
}
@Override
protected InstrumentedNetworkAgent makeNetworkAgent(LinkProperties linkProperties,
NetworkAgentConfig nac, NetworkProvider provider) throws Exception {
mNetworkMonitor = mock(INetworkMonitor.class);
final Answer validateAnswer = inv -> {
new Thread(ignoreExceptions(this::onValidationRequested)).start();
return null;
};
doAnswer(validateAnswer).when(mNetworkMonitor).notifyNetworkConnected(any(), any());
doAnswer(validateAnswer).when(mNetworkMonitor).forceReevaluation(anyInt());
final ArgumentCaptor<Network> nmNetworkCaptor = ArgumentCaptor.forClass(Network.class);
final ArgumentCaptor<INetworkMonitorCallbacks> nmCbCaptor =
ArgumentCaptor.forClass(INetworkMonitorCallbacks.class);
doNothing().when(mNetworkStack).makeNetworkMonitor(
nmNetworkCaptor.capture(),
any() /* name */,
nmCbCaptor.capture());
final InstrumentedNetworkAgent na =
new TestInstrumentedNetworkAgent(this, linkProperties, nac, provider);
assertEquals(na.getNetwork().netId, nmNetworkCaptor.getValue().netId);
mNmCallbacks = nmCbCaptor.getValue();
@@ -4797,6 +4811,124 @@ public class ConnectivityServiceTest {
testAvoidBadWifiConfig_controlledBySettings();
}
@Test
public void testOffersAvoidsBadWifi() throws Exception {
// Normal mode : the carrier doesn't restrict moving away from bad wifi.
// This has getAvoidBadWifi return true.
doReturn(1).when(mResources).getInteger(R.integer.config_networkAvoidBadWifi);
// Don't request cell separately for the purposes of this test.
setAlwaysOnNetworks(false);
final NetworkProvider cellProvider = new NetworkProvider(mServiceContext,
mCsHandlerThread.getLooper(), "Cell provider");
final NetworkProvider wifiProvider = new NetworkProvider(mServiceContext,
mCsHandlerThread.getLooper(), "Wifi provider");
mCm.registerNetworkProvider(cellProvider);
mCm.registerNetworkProvider(wifiProvider);
final NetworkScore cellScore = new NetworkScore.Builder().build();
final NetworkScore wifiScore = new NetworkScore.Builder().build();
final NetworkCapabilities defaultCaps = new NetworkCapabilities.Builder()
.addCapability(NET_CAPABILITY_INTERNET)
.addCapability(NET_CAPABILITY_NOT_VCN_MANAGED)
.build();
final NetworkCapabilities cellCaps = new NetworkCapabilities.Builder()
.addTransportType(TRANSPORT_CELLULAR)
.addCapability(NET_CAPABILITY_INTERNET)
.addCapability(NET_CAPABILITY_NOT_VCN_MANAGED)
.build();
final NetworkCapabilities wifiCaps = new NetworkCapabilities.Builder()
.addTransportType(TRANSPORT_WIFI)
.addCapability(NET_CAPABILITY_INTERNET)
.addCapability(NET_CAPABILITY_NOT_VCN_MANAGED)
.build();
final TestableNetworkOfferCallback cellCallback = new TestableNetworkOfferCallback(
TIMEOUT_MS /* timeout */, TEST_CALLBACK_TIMEOUT_MS /* noCallbackTimeout */);
final TestableNetworkOfferCallback wifiCallback = new TestableNetworkOfferCallback(
TIMEOUT_MS /* timeout */, TEST_CALLBACK_TIMEOUT_MS /* noCallbackTimeout */);
Log.e("ConnectivityService", "test registering " + cellProvider);
// Offer callbacks will run on the CS handler thread in this test.
cellProvider.registerNetworkOffer(cellScore, cellCaps, r -> r.run(), cellCallback);
wifiProvider.registerNetworkOffer(wifiScore, wifiCaps, r -> r.run(), wifiCallback);
// Both providers see the default request.
cellCallback.expectOnNetworkNeeded(defaultCaps);
wifiCallback.expectOnNetworkNeeded(defaultCaps);
// Listen to cell and wifi to know when agents are finished processing
final TestNetworkCallback cellNetworkCallback = new TestNetworkCallback();
final NetworkRequest cellRequest = new NetworkRequest.Builder()
.addTransportType(TRANSPORT_CELLULAR).build();
mCm.registerNetworkCallback(cellRequest, cellNetworkCallback);
final TestNetworkCallback wifiNetworkCallback = new TestNetworkCallback();
final NetworkRequest wifiRequest = new NetworkRequest.Builder()
.addTransportType(TRANSPORT_WIFI).build();
mCm.registerNetworkCallback(wifiRequest, wifiNetworkCallback);
// Cell connects and validates.
mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR,
new LinkProperties(), null /* ncTemplate */, cellProvider);
mCellNetworkAgent.connect(true);
cellNetworkCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent);
cellCallback.assertNoCallback();
wifiCallback.assertNoCallback();
// Bring up wifi. At first it's invalidated, so cell is still needed.
mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI,
new LinkProperties(), null /* ncTemplate */, wifiProvider);
mWiFiNetworkAgent.connect(false);
wifiNetworkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
cellCallback.assertNoCallback();
wifiCallback.assertNoCallback();
// Wifi validates. Cell is no longer needed, because it's outscored.
mWiFiNetworkAgent.setNetworkValid(true /* isStrictMode */);
// Have CS reconsider the network (see testPartialConnectivity)
mCm.reportNetworkConnectivity(mWiFiNetworkAgent.getNetwork(), true);
wifiNetworkCallback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent);
cellCallback.expectOnNetworkUnneeded(defaultCaps);
wifiCallback.assertNoCallback();
// Wifi is no longer validated. Cell is needed again.
mWiFiNetworkAgent.setNetworkInvalid(true /* isStrictMode */);
mCm.reportNetworkConnectivity(mWiFiNetworkAgent.getNetwork(), false);
wifiNetworkCallback.expectCapabilitiesWithout(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent);
cellCallback.expectOnNetworkNeeded(defaultCaps);
wifiCallback.assertNoCallback();
// Disconnect wifi and pretend the carrier restricts moving away from bad wifi.
mWiFiNetworkAgent.disconnect();
wifiNetworkCallback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent);
// This has getAvoidBadWifi return false. This test doesn't change the value of the
// associated setting.
doReturn(0).when(mResources).getInteger(R.integer.config_networkAvoidBadWifi);
mPolicyTracker.reevaluate();
waitForIdle();
// Connect wifi again, cell is needed until wifi validates.
mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI,
new LinkProperties(), null /* ncTemplate */, wifiProvider);
mWiFiNetworkAgent.connect(false);
wifiNetworkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent);
cellCallback.assertNoCallback();
wifiCallback.assertNoCallback();
mWiFiNetworkAgent.setNetworkValid(true /* isStrictMode */);
mCm.reportNetworkConnectivity(mWiFiNetworkAgent.getNetwork(), true);
wifiNetworkCallback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent);
cellCallback.expectOnNetworkUnneeded(defaultCaps);
wifiCallback.assertNoCallback();
// Wifi loses validation. Because the device doesn't avoid bad wifis, cell is
// not needed.
mWiFiNetworkAgent.setNetworkInvalid(true /* isStrictMode */);
mCm.reportNetworkConnectivity(mWiFiNetworkAgent.getNetwork(), false);
wifiNetworkCallback.expectCapabilitiesWithout(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent);
cellCallback.assertNoCallback();
wifiCallback.assertNoCallback();
}
@Test
public void testAvoidBadWifi() throws Exception {
final ContentResolver cr = mServiceContext.getContentResolver();