summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Nucca Chen <nuccachen@google.com> 2020-06-17 07:03:39 +0000
committer Nucca Chen <nuccachen@google.com> 2020-06-17 07:40:40 +0000
commit3f856b48f94c1286679fc1bfabe87f4f434813c4 (patch)
treeea33b35eceb902395ddfaa9414ea0b16a8398e4e
parent1f9efaff8d2ce77f7cefce1f56659234ad459a8c (diff)
[BOT.11] BpfCoordinator could be disabled by device config
Bug: 150736748 Test: BpfCoordinatorTest Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1313955 Merged-In: Id413b7f2f7edb2e5c3e02d5677fe536ed52fbbcb Change-Id: I48a8de478c9200b8c9f88e785340fc7973e4a13f (cherry picked from commit 41d61b6702b80389e77b67fe7762a02b96558d76)
-rw-r--r--packages/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java65
-rw-r--r--packages/Tethering/src/com/android/networkstack/tethering/Tethering.java34
-rw-r--r--packages/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java5
-rw-r--r--packages/Tethering/tests/unit/src/android/net/ip/IpServerTest.java42
-rw-r--r--packages/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java41
-rw-r--r--packages/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java4
6 files changed, 162 insertions, 29 deletions
diff --git a/packages/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/packages/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
index 4315485f060a..aa1d59de89c3 100644
--- a/packages/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
+++ b/packages/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
@@ -89,6 +89,13 @@ public class BpfCoordinator {
@Nullable
private final BpfTetherStatsProvider mStatsProvider;
+ // True if BPF offload is supported, false otherwise. The BPF offload could be disabled by
+ // a runtime resource overlay package or device configuration. This flag is only initialized
+ // in the constructor because it is hard to unwind all existing change once device
+ // configuration is changed. Especially the forwarding rules. Keep the same setting
+ // to make it simpler. See also TetheringConfiguration.
+ private final boolean mUsingBpf;
+
// Tracks whether BPF tethering is started or not. This is set by tethering before it
// starts the first IpServer and is cleared by tethering shortly before the last IpServer
// is stopped. Note that rule updates (especially deletions, but sometimes additions as
@@ -146,22 +153,42 @@ public class BpfCoordinator {
};
@VisibleForTesting
- public static class Dependencies {
- int getPerformPollInterval() {
+ public abstract static class Dependencies {
+ /**
+ * Get polling Interval in milliseconds.
+ */
+ public int getPerformPollInterval() {
// TODO: Consider make this configurable.
return DEFAULT_PERFORM_POLL_INTERVAL_MS;
}
+
+ /** Get handler. */
+ @NonNull public abstract Handler getHandler();
+
+ /** Get netd. */
+ @NonNull public abstract INetd getNetd();
+
+ /** Get network stats manager. */
+ @NonNull public abstract NetworkStatsManager getNetworkStatsManager();
+
+ /** Get shared log. */
+ @NonNull public abstract SharedLog getSharedLog();
+
+ /** Get tethering configuration. */
+ @Nullable public abstract TetheringConfiguration getTetherConfig();
}
@VisibleForTesting
- public BpfCoordinator(@NonNull Handler handler, @NonNull INetd netd,
- @NonNull NetworkStatsManager nsm, @NonNull SharedLog log, @NonNull Dependencies deps) {
- mHandler = handler;
- mNetd = netd;
- mLog = log.forSubComponent(TAG);
+ public BpfCoordinator(@NonNull Dependencies deps) {
+ mDeps = deps;
+ mHandler = mDeps.getHandler();
+ mNetd = mDeps.getNetd();
+ mLog = mDeps.getSharedLog().forSubComponent(TAG);
+ mUsingBpf = isOffloadEnabled();
BpfTetherStatsProvider provider = new BpfTetherStatsProvider();
try {
- nsm.registerNetworkStatsProvider(getClass().getSimpleName(), provider);
+ mDeps.getNetworkStatsManager().registerNetworkStatsProvider(
+ getClass().getSimpleName(), provider);
} catch (RuntimeException e) {
// TODO: Perhaps not allow to use BPF offload because the reregistration failure
// implied that no data limit could be applies on a metered upstream if any.
@@ -169,7 +196,6 @@ public class BpfCoordinator {
provider = null;
}
mStatsProvider = provider;
- mDeps = deps;
}
/**
@@ -181,6 +207,11 @@ public class BpfCoordinator {
public void startPolling() {
if (mPollingStarted) return;
+ if (!mUsingBpf) {
+ mLog.i("Offload disabled");
+ return;
+ }
+
mPollingStarted = true;
maybeSchedulePollingStats();
@@ -215,6 +246,8 @@ public class BpfCoordinator {
*/
public void tetherOffloadRuleAdd(
@NonNull final IpServer ipServer, @NonNull final Ipv6ForwardingRule rule) {
+ if (!mUsingBpf) return;
+
try {
// TODO: Perhaps avoid to add a duplicate rule.
mNetd.tetherOffloadRuleAdd(rule.toTetherOffloadRuleParcel());
@@ -254,6 +287,8 @@ public class BpfCoordinator {
*/
public void tetherOffloadRuleRemove(
@NonNull final IpServer ipServer, @NonNull final Ipv6ForwardingRule rule) {
+ if (!mUsingBpf) return;
+
try {
// TODO: Perhaps avoid to remove a non-existent rule.
mNetd.tetherOffloadRuleRemove(rule.toTetherOffloadRuleParcel());
@@ -297,6 +332,8 @@ public class BpfCoordinator {
* Note that this can be only called on handler thread.
*/
public void tetherOffloadRuleClear(@NonNull final IpServer ipServer) {
+ if (!mUsingBpf) return;
+
final LinkedHashMap<Inet6Address, Ipv6ForwardingRule> rules = mIpv6ForwardingRules.get(
ipServer);
if (rules == null) return;
@@ -312,6 +349,8 @@ public class BpfCoordinator {
* Note that this can be only called on handler thread.
*/
public void tetherOffloadRuleUpdate(@NonNull final IpServer ipServer, int newUpstreamIfindex) {
+ if (!mUsingBpf) return;
+
final LinkedHashMap<Inet6Address, Ipv6ForwardingRule> rules = mIpv6ForwardingRules.get(
ipServer);
if (rules == null) return;
@@ -334,6 +373,8 @@ public class BpfCoordinator {
* Note that this can be only called on handler thread.
*/
public void addUpstreamNameToLookupTable(int upstreamIfindex, @NonNull String upstreamIface) {
+ if (!mUsingBpf) return;
+
if (upstreamIfindex == 0 || TextUtils.isEmpty(upstreamIface)) return;
// The same interface index to name mapping may be added by different IpServer objects or
@@ -357,6 +398,7 @@ public class BpfCoordinator {
public void dump(@NonNull IndentingPrintWriter pw) {
final ConditionVariable dumpDone = new ConditionVariable();
mHandler.post(() -> {
+ pw.println("mUsingBpf: " + mUsingBpf);
pw.println("Polling " + (mPollingStarted ? "started" : "not started"));
pw.println("Stats provider " + (mStatsProvider != null
? "registered" : "not registered"));
@@ -547,6 +589,11 @@ public class BpfCoordinator {
}
}
+ private boolean isOffloadEnabled() {
+ final TetheringConfiguration config = mDeps.getTetherConfig();
+ return (config != null) ? config.enableBpfOffload : true /* default value */;
+ }
+
private int getInterfaceIndexFromRules(@NonNull String ifName) {
for (LinkedHashMap<Inet6Address, Ipv6ForwardingRule> rules : mIpv6ForwardingRules
.values()) {
diff --git a/packages/Tethering/src/com/android/networkstack/tethering/Tethering.java b/packages/Tethering/src/com/android/networkstack/tethering/Tethering.java
index 3184b1ffedff..99ffa774b587 100644
--- a/packages/Tethering/src/com/android/networkstack/tethering/Tethering.java
+++ b/packages/Tethering/src/com/android/networkstack/tethering/Tethering.java
@@ -63,6 +63,7 @@ import static android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID;
import static com.android.networkstack.tethering.TetheringNotificationUpdater.DOWNSTREAM_NONE;
+import android.app.usage.NetworkStatsManager;
import android.bluetooth.BluetoothAdapter;
import android.bluetooth.BluetoothPan;
import android.bluetooth.BluetoothProfile;
@@ -286,8 +287,6 @@ public class Tethering {
mUpstreamNetworkMonitor = mDeps.getUpstreamNetworkMonitor(mContext, mTetherMasterSM, mLog,
TetherMasterSM.EVENT_UPSTREAM_CALLBACK);
mForwardedDownstreams = new LinkedHashSet<>();
- mBpfCoordinator = mDeps.getBpfCoordinator(
- mHandler, mNetd, mLog, new BpfCoordinator.Dependencies());
IntentFilter filter = new IntentFilter();
filter.addAction(ACTION_CARRIER_CONFIG_CHANGED);
@@ -325,6 +324,37 @@ public class Tethering {
// Load tethering configuration.
updateConfiguration();
+ // Must be initialized after tethering configuration is loaded because BpfCoordinator
+ // constructor needs to use the configuration.
+ mBpfCoordinator = mDeps.getBpfCoordinator(
+ new BpfCoordinator.Dependencies() {
+ @NonNull
+ public Handler getHandler() {
+ return mHandler;
+ }
+
+ @NonNull
+ public INetd getNetd() {
+ return mNetd;
+ }
+
+ @NonNull
+ public NetworkStatsManager getNetworkStatsManager() {
+ return (NetworkStatsManager) mContext.getSystemService(
+ Context.NETWORK_STATS_SERVICE);
+ }
+
+ @NonNull
+ public SharedLog getSharedLog() {
+ return mLog;
+ }
+
+ @Nullable
+ public TetheringConfiguration getTetherConfig() {
+ return mConfig;
+ }
+ });
+
startStateMachineUpdaters();
}
diff --git a/packages/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java b/packages/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java
index 31f747d3c4b5..131a5fbf2abe 100644
--- a/packages/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java
+++ b/packages/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java
@@ -46,11 +46,8 @@ public abstract class TetheringDependencies {
* Get a reference to the BpfCoordinator to be used by tethering.
*/
public @NonNull BpfCoordinator getBpfCoordinator(
- @NonNull Handler handler, @NonNull INetd netd, @NonNull SharedLog log,
@NonNull BpfCoordinator.Dependencies deps) {
- final NetworkStatsManager statsManager =
- (NetworkStatsManager) getContext().getSystemService(Context.NETWORK_STATS_SERVICE);
- return new BpfCoordinator(handler, netd, statsManager, log, deps);
+ return new BpfCoordinator(deps);
}
/**
diff --git a/packages/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/packages/Tethering/tests/unit/src/android/net/ip/IpServerTest.java
index c3bc915a232d..fdfd92617be2 100644
--- a/packages/Tethering/tests/unit/src/android/net/ip/IpServerTest.java
+++ b/packages/Tethering/tests/unit/src/android/net/ip/IpServerTest.java
@@ -89,12 +89,14 @@ import android.os.test.TestLooper;
import android.text.TextUtils;
import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
import androidx.test.filters.SmallTest;
import androidx.test.runner.AndroidJUnit4;
import com.android.networkstack.tethering.BpfCoordinator;
import com.android.networkstack.tethering.BpfCoordinator.Ipv6ForwardingRule;
import com.android.networkstack.tethering.PrivateAddressCoordinator;
+import com.android.networkstack.tethering.TetheringConfiguration;
import org.junit.Before;
import org.junit.Test;
@@ -226,9 +228,36 @@ public class IpServerTest {
when(mSharedLog.forSubComponent(anyString())).thenReturn(mSharedLog);
when(mAddressCoordinator.requestDownstreamAddress(any())).thenReturn(mTestAddress);
- BpfCoordinator bc = new BpfCoordinator(new Handler(mLooper.getLooper()), mNetd,
- mStatsManager, mSharedLog, new BpfCoordinator.Dependencies());
- mBpfCoordinator = spy(bc);
+ mBpfCoordinator = spy(new BpfCoordinator(
+ new BpfCoordinator.Dependencies() {
+ @NonNull
+ public Handler getHandler() {
+ return new Handler(mLooper.getLooper());
+ }
+
+ @NonNull
+ public INetd getNetd() {
+ return mNetd;
+ }
+
+ @NonNull
+ public NetworkStatsManager getNetworkStatsManager() {
+ return mStatsManager;
+ }
+
+ @NonNull
+ public SharedLog getSharedLog() {
+ return mSharedLog;
+ }
+
+ @Nullable
+ public TetheringConfiguration getTetherConfig() {
+ // Returning null configuration object is a hack to enable BPF offload.
+ // See BpfCoordinator#isOffloadEnabled.
+ // TODO: Mock TetheringConfiguration to test.
+ return null;
+ }
+ }));
}
@Test
@@ -671,18 +700,21 @@ public class IpServerTest {
}
}
- private TetherOffloadRuleParcel matches(
+ @NonNull
+ private static TetherOffloadRuleParcel matches(
int upstreamIfindex, InetAddress dst, MacAddress dstMac) {
return argThat(new TetherOffloadRuleParcelMatcher(upstreamIfindex, dst, dstMac));
}
+ @NonNull
private static Ipv6ForwardingRule makeForwardingRule(
int upstreamIfindex, @NonNull InetAddress dst, @NonNull MacAddress dstMac) {
return new Ipv6ForwardingRule(upstreamIfindex, TEST_IFACE_PARAMS.index,
(Inet6Address) dst, TEST_IFACE_PARAMS.macAddr, dstMac);
}
- private TetherStatsParcel buildEmptyTetherStatsParcel(int ifIndex) {
+ @NonNull
+ private static TetherStatsParcel buildEmptyTetherStatsParcel(int ifIndex) {
TetherStatsParcel parcel = new TetherStatsParcel();
parcel.ifIndex = ifIndex;
return parcel;
diff --git a/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java b/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java
index ba0f41c4fa8d..f63a473f1d5c 100644
--- a/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java
+++ b/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java
@@ -45,7 +45,6 @@ import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
-import android.annotation.NonNull;
import android.app.usage.NetworkStatsManager;
import android.net.INetd;
import android.net.InetAddresses;
@@ -58,6 +57,8 @@ import android.net.util.SharedLog;
import android.os.Handler;
import android.os.test.TestLooper;
+import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
import androidx.test.filters.SmallTest;
import androidx.test.runner.AndroidJUnit4;
@@ -101,9 +102,37 @@ public class BpfCoordinatorTest {
private BpfCoordinator.Dependencies mDeps =
new BpfCoordinator.Dependencies() {
@Override
- int getPerformPollInterval() {
+ public int getPerformPollInterval() {
return DEFAULT_PERFORM_POLL_INTERVAL_MS;
}
+
+ @NonNull
+ public Handler getHandler() {
+ return new Handler(mTestLooper.getLooper());
+ }
+
+ @NonNull
+ public INetd getNetd() {
+ return mNetd;
+ }
+
+ @NonNull
+ public NetworkStatsManager getNetworkStatsManager() {
+ return mStatsManager;
+ }
+
+ @NonNull
+ public SharedLog getSharedLog() {
+ return new SharedLog("test");
+ }
+
+ @Nullable
+ public TetheringConfiguration getTetherConfig() {
+ // Returning null configuration object is a hack to enable BPF offload.
+ // See BpfCoordinator#isOffloadEnabled.
+ // TODO: Mock TetheringConfiguration to test.
+ return null;
+ }
};
@Before public void setUp() {
@@ -120,9 +149,7 @@ public class BpfCoordinatorTest {
@NonNull
private BpfCoordinator makeBpfCoordinator() throws Exception {
- BpfCoordinator coordinator = new BpfCoordinator(
- new Handler(mTestLooper.getLooper()), mNetd, mStatsManager, new SharedLog("test"),
- mDeps);
+ final BpfCoordinator coordinator = new BpfCoordinator(mDeps);
final ArgumentCaptor<BpfCoordinator.BpfTetherStatsProvider>
tetherStatsProviderCaptor =
ArgumentCaptor.forClass(BpfCoordinator.BpfTetherStatsProvider.class);
@@ -335,8 +362,8 @@ public class BpfCoordinatorTest {
inOrder.verifyNoMoreInteractions();
// [2] Specific limit.
- // Applying the data limit boundary {min, max, infinity} on current upstream.
- for (final long quota : new long[] {0, Long.MAX_VALUE, QUOTA_UNLIMITED}) {
+ // Applying the data limit boundary {min, 1gb, max, infinity} on current upstream.
+ for (final long quota : new long[] {0, 1048576000, Long.MAX_VALUE, QUOTA_UNLIMITED}) {
mTetherStatsProvider.onSetLimit(mobileIface, quota);
waitForIdle();
inOrder.verify(mNetd).tetherOffloadSetInterfaceQuota(mobileIfIndex, quota);
diff --git a/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java b/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
index f53c42b7e760..526199226a6e 100644
--- a/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
+++ b/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
@@ -346,8 +346,8 @@ public class TetheringTest {
}
@Override
- public BpfCoordinator getBpfCoordinator(Handler handler, INetd netd,
- SharedLog log, BpfCoordinator.Dependencies deps) {
+ public BpfCoordinator getBpfCoordinator(
+ BpfCoordinator.Dependencies deps) {
return mBpfCoordinator;
}