Consider NetworkOffer is unneeded if it cannot satisfy the request

Currently, to prevent from network connect-teardown-loops that
caused by inaccurate reports, e.g. the provider always provides
a better network than the offer, the NetworkOffer is considered
needed if its provider is currently serving the request. This
is because there is no accurate way to know whether the offer is
corresponding to the network that is currently serving the
request.

However, if the offer cannot even satisfies the request, consider
the offer is needed does not make any sense. Since it can
never be the one that currently serving the request, nor be
the one that might beat current satisfier.

Test: android.net.NetworkProviderTest
Bug: 189074532
Original-Change: https://android-review.googlesource.com/1731452
Merged-In: Ie3ea59f980c3767782b8e6b03e401c02f664f9bd
Change-Id: Ie3ea59f980c3767782b8e6b03e401c02f664f9bd
This commit is contained in:
Junyu Lai
2021-07-05 09:09:01 +00:00
parent 8920d32880
commit 135f50679f
3 changed files with 11 additions and 13 deletions

View File

@@ -8347,13 +8347,13 @@ public class ConnectivityService extends IConnectivityManager.Stub
// Second phase : deal with the active request (if any)
if (null != activeRequest && activeRequest.isRequest()) {
final boolean oldNeeded = offer.neededFor(activeRequest);
// An offer is needed if it is currently served by this provider or if this offer
// can beat the current satisfier.
// If an offer can satisfy the request, it is considered needed if it is currently
// served by this provider or if this offer can beat the current satisfier.
final boolean currentlyServing = satisfier != null
&& satisfier.factorySerialNumber == offer.providerId;
final boolean newNeeded = (currentlyServing
|| (activeRequest.canBeSatisfiedBy(offer.caps)
&& networkRanker.mightBeat(activeRequest, satisfier, offer)));
&& satisfier.factorySerialNumber == offer.providerId
&& activeRequest.canBeSatisfiedBy(offer.caps);
final boolean newNeeded = currentlyServing
|| networkRanker.mightBeat(activeRequest, satisfier, offer);
if (newNeeded != oldNeeded) {
if (newNeeded) {
offer.onNetworkNeeded(activeRequest);

View File

@@ -143,6 +143,6 @@ public class NetworkOffer implements NetworkRanker.Scoreable {
@Override
public String toString() {
return "NetworkOffer [ Score " + score + " ]";
return "NetworkOffer [ Score " + score + " Caps " + caps + "]";
}
}

View File

@@ -315,9 +315,7 @@ class NetworkProviderTest {
LinkProperties(), scoreWeaker, config, provider) {}
agent.register()
agent.markConnected()
// TODO: The request is satisying by offer 2 instead of offer 1, thus it should not be
// considered as needed.
offerCallback1.expectOnNetworkNeeded(ncFilter2)
offerCallback1.assertNoCallback() // Still unneeded.
offerCallback2.assertNoCallback() // Still needed.
offerCallback3.assertNoCallback() // Still needed.
offerCallback4.expectOnNetworkUnneeded(ncFilter4)
@@ -326,7 +324,7 @@ class NetworkProviderTest {
// if a request is currently satisfied by the network provided by the same provider.
// TODO: Consider offers with weaker score are unneeded.
agent.sendNetworkScore(scoreStronger)
offerCallback1.assertNoCallback()
offerCallback1.assertNoCallback() // Still unneeded.
offerCallback2.assertNoCallback() // Still needed.
offerCallback3.assertNoCallback() // Still needed.
offerCallback4.assertNoCallback() // Still unneeded.
@@ -334,7 +332,7 @@ class NetworkProviderTest {
// Verify that offer callbacks cannot receive any event if offer is unregistered.
provider2.unregisterNetworkOffer(offerCallback4)
agent.unregister()
offerCallback1.assertNoCallback() // Still needed.
offerCallback1.assertNoCallback() // Still unneeded.
offerCallback2.assertNoCallback() // Still needed.
offerCallback3.assertNoCallback() // Still needed.
// Since the agent is unregistered, and the offer has chance to satisfy the request,
@@ -344,7 +342,7 @@ class NetworkProviderTest {
// Verify that offer callbacks cannot receive any event if provider is unregistered.
mCm.unregisterNetworkProvider(provider)
mCm.unregisterNetworkCallback(cb2)
offerCallback1.assertNoCallback() // Should be unneeded if not unregistered.
offerCallback1.assertNoCallback() // No callback since it is still unneeded.
offerCallback2.assertNoCallback() // Should be unneeded if not unregistered.
offerCallback3.assertNoCallback() // Should be unneeded if not unregistered.
offerCallback4.assertNoCallback() // Already unregistered.