diff options
| author | 2020-06-17 07:03:39 +0000 | |
|---|---|---|
| committer | 2020-06-17 07:40:40 +0000 | |
| commit | 3f856b48f94c1286679fc1bfabe87f4f434813c4 (patch) | |
| tree | ea33b35eceb902395ddfaa9414ea0b16a8398e4e | |
| parent | 1f9efaff8d2ce77f7cefce1f56659234ad459a8c (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)
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; } |