From 2fbc671d4a9ae759cb6495f4f01c50e994efc89e Mon Sep 17 00:00:00 2001 From: Nucca Chen Date: Wed, 17 Jun 2020 06:31:45 +0000 Subject: [PATCH 1/3] [BOT.10] Add unit test for data limit and rule change in BpfCoordinator The applying data limit is based on the forwarding rule changes. Add the tests for verifying their interactions with netd. Bug: 150736748 Test: BpfCoordinatorTest Original-Change: https://android-review.googlesource.com/1311659 Merged-In: I5a98c4cd74e2de6005ee05defa761f6af3fd4e75 Change-Id: I5a98c4cd74e2de6005ee05defa761f6af3fd4e75 --- .../tethering/BpfCoordinatorTest.java | 231 ++++++++++++++++++ 1 file changed, 231 insertions(+) diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java index e2d7aab4e3..ba0f41c4fa 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java @@ -31,10 +31,16 @@ import static com.android.networkstack.tethering.BpfCoordinator.StatsType; import static com.android.networkstack.tethering.BpfCoordinator.StatsType.STATS_PER_IFACE; import static com.android.networkstack.tethering.BpfCoordinator.StatsType.STATS_PER_UID; +import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertNotNull; +import static junit.framework.Assert.fail; +import static org.mockito.Matchers.anyInt; +import static org.mockito.Matchers.anyLong; import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.argThat; import static org.mockito.Mockito.clearInvocations; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -42,8 +48,12 @@ import static org.mockito.Mockito.when; import android.annotation.NonNull; import android.app.usage.NetworkStatsManager; import android.net.INetd; +import android.net.InetAddresses; +import android.net.MacAddress; import android.net.NetworkStats; +import android.net.TetherOffloadRuleParcel; import android.net.TetherStatsParcel; +import android.net.ip.IpServer; import android.net.util.SharedLog; import android.os.Handler; import android.os.test.TestLooper; @@ -51,22 +61,37 @@ import android.os.test.TestLooper; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; +import com.android.networkstack.tethering.BpfCoordinator.Ipv6ForwardingRule; import com.android.testutils.TestableNetworkStatsProviderCbBinder; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; +import org.mockito.ArgumentMatcher; +import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.net.Inet6Address; +import java.net.InetAddress; import java.util.ArrayList; +import java.util.Arrays; @RunWith(AndroidJUnit4.class) @SmallTest public class BpfCoordinatorTest { + private static final int DOWNSTREAM_IFINDEX = 10; + private static final MacAddress DOWNSTREAM_MAC = MacAddress.ALL_ZEROS_ADDRESS; + private static final InetAddress NEIGH_A = InetAddresses.parseNumericAddress("2001:db8::1"); + private static final InetAddress NEIGH_B = InetAddresses.parseNumericAddress("2001:db8::2"); + private static final MacAddress MAC_A = MacAddress.fromString("00:00:00:00:00:0a"); + private static final MacAddress MAC_B = MacAddress.fromString("11:22:33:00:00:0b"); + @Mock private NetworkStatsManager mStatsManager; @Mock private INetd mNetd; + @Mock private IpServer mIpServer; + // Late init since methods must be called by the thread that created this object. private TestableNetworkStatsProviderCbBinder mTetherStatsProviderCb; private BpfCoordinator.BpfTetherStatsProvider mTetherStatsProvider; @@ -243,4 +268,210 @@ public class BpfCoordinatorTest { waitForIdle(); mTetherStatsProviderCb.assertNoCallback(); } + + // The custom ArgumentMatcher simply comes from IpServerTest. + // TODO: move both of them into a common utility class for reusing the code. + private static class TetherOffloadRuleParcelMatcher implements + ArgumentMatcher { + public final int upstreamIfindex; + public final int downstreamIfindex; + public final Inet6Address address; + public final MacAddress srcMac; + public final MacAddress dstMac; + + TetherOffloadRuleParcelMatcher(@NonNull Ipv6ForwardingRule rule) { + upstreamIfindex = rule.upstreamIfindex; + downstreamIfindex = rule.downstreamIfindex; + address = rule.address; + srcMac = rule.srcMac; + dstMac = rule.dstMac; + } + + public boolean matches(@NonNull TetherOffloadRuleParcel parcel) { + return upstreamIfindex == parcel.inputInterfaceIndex + && (downstreamIfindex == parcel.outputInterfaceIndex) + && Arrays.equals(address.getAddress(), parcel.destination) + && (128 == parcel.prefixLength) + && Arrays.equals(srcMac.toByteArray(), parcel.srcL2Address) + && Arrays.equals(dstMac.toByteArray(), parcel.dstL2Address); + } + + public String toString() { + return String.format("TetherOffloadRuleParcelMatcher(%d, %d, %s, %s, %s", + upstreamIfindex, downstreamIfindex, address.getHostAddress(), srcMac, dstMac); + } + } + + @NonNull + private TetherOffloadRuleParcel matches(@NonNull Ipv6ForwardingRule rule) { + return argThat(new TetherOffloadRuleParcelMatcher(rule)); + } + + @NonNull + private static Ipv6ForwardingRule buildTestForwardingRule( + int upstreamIfindex, @NonNull InetAddress address, @NonNull MacAddress dstMac) { + return new Ipv6ForwardingRule(upstreamIfindex, DOWNSTREAM_IFINDEX, (Inet6Address) address, + DOWNSTREAM_MAC, dstMac); + } + + @Test + public void testSetDataLimit() throws Exception { + setupFunctioningNetdInterface(); + + final BpfCoordinator coordinator = makeBpfCoordinator(); + + final String mobileIface = "rmnet_data0"; + final Integer mobileIfIndex = 100; + coordinator.addUpstreamNameToLookupTable(mobileIfIndex, mobileIface); + + // [1] Default limit. + // Set the unlimited quota as default if the service has never applied a data limit for a + // given upstream. Note that the data limit only be applied on an upstream which has rules. + final Ipv6ForwardingRule rule = buildTestForwardingRule(mobileIfIndex, NEIGH_A, MAC_A); + final InOrder inOrder = inOrder(mNetd); + coordinator.tetherOffloadRuleAdd(mIpServer, rule); + inOrder.verify(mNetd).tetherOffloadRuleAdd(matches(rule)); + inOrder.verify(mNetd).tetherOffloadSetInterfaceQuota(mobileIfIndex, QUOTA_UNLIMITED); + 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}) { + mTetherStatsProvider.onSetLimit(mobileIface, quota); + waitForIdle(); + inOrder.verify(mNetd).tetherOffloadSetInterfaceQuota(mobileIfIndex, quota); + inOrder.verifyNoMoreInteractions(); + } + + // [3] Invalid limit. + // The valid range of quota is 0..max_int64 or -1 (unlimited). + final long invalidLimit = Long.MIN_VALUE; + try { + mTetherStatsProvider.onSetLimit(mobileIface, invalidLimit); + waitForIdle(); + fail("No exception thrown for invalid limit " + invalidLimit + "."); + } catch (IllegalArgumentException expected) { + assertEquals(expected.getMessage(), "invalid quota value " + invalidLimit); + } + } + + // TODO: Test the case in which the rules are changed from different IpServer objects. + @Test + public void testSetDataLimitOnRuleChange() throws Exception { + setupFunctioningNetdInterface(); + + final BpfCoordinator coordinator = makeBpfCoordinator(); + + final String mobileIface = "rmnet_data0"; + final Integer mobileIfIndex = 100; + coordinator.addUpstreamNameToLookupTable(mobileIfIndex, mobileIface); + + // Applying a data limit to the current upstream does not take any immediate action. + // The data limit could be only set on an upstream which has rules. + final long limit = 12345; + final InOrder inOrder = inOrder(mNetd); + mTetherStatsProvider.onSetLimit(mobileIface, limit); + waitForIdle(); + inOrder.verify(mNetd, never()).tetherOffloadSetInterfaceQuota(anyInt(), anyLong()); + + // Adding the first rule on current upstream immediately sends the quota to netd. + final Ipv6ForwardingRule ruleA = buildTestForwardingRule(mobileIfIndex, NEIGH_A, MAC_A); + coordinator.tetherOffloadRuleAdd(mIpServer, ruleA); + inOrder.verify(mNetd).tetherOffloadRuleAdd(matches(ruleA)); + inOrder.verify(mNetd).tetherOffloadSetInterfaceQuota(mobileIfIndex, limit); + inOrder.verifyNoMoreInteractions(); + + // Adding the second rule on current upstream does not send the quota to netd. + final Ipv6ForwardingRule ruleB = buildTestForwardingRule(mobileIfIndex, NEIGH_B, MAC_B); + coordinator.tetherOffloadRuleAdd(mIpServer, ruleB); + inOrder.verify(mNetd).tetherOffloadRuleAdd(matches(ruleB)); + inOrder.verify(mNetd, never()).tetherOffloadSetInterfaceQuota(anyInt(), anyLong()); + + // Removing the second rule on current upstream does not send the quota to netd. + coordinator.tetherOffloadRuleRemove(mIpServer, ruleB); + inOrder.verify(mNetd).tetherOffloadRuleRemove(matches(ruleB)); + inOrder.verify(mNetd, never()).tetherOffloadSetInterfaceQuota(anyInt(), anyLong()); + + // Removing the last rule on current upstream immediately sends the cleanup stuff to netd. + when(mNetd.tetherOffloadGetAndClearStats(mobileIfIndex)) + .thenReturn(buildTestTetherStatsParcel(mobileIfIndex, 0, 0, 0, 0)); + coordinator.tetherOffloadRuleRemove(mIpServer, ruleA); + inOrder.verify(mNetd).tetherOffloadRuleRemove(matches(ruleA)); + inOrder.verify(mNetd).tetherOffloadGetAndClearStats(mobileIfIndex); + inOrder.verifyNoMoreInteractions(); + } + + @Test + public void testTetherOffloadRuleUpdateAndClear() throws Exception { + setupFunctioningNetdInterface(); + + final BpfCoordinator coordinator = makeBpfCoordinator(); + + final String ethIface = "eth1"; + final String mobileIface = "rmnet_data0"; + final Integer ethIfIndex = 100; + final Integer mobileIfIndex = 101; + coordinator.addUpstreamNameToLookupTable(ethIfIndex, ethIface); + coordinator.addUpstreamNameToLookupTable(mobileIfIndex, mobileIface); + + final InOrder inOrder = inOrder(mNetd); + + // Before the rule test, here are the additional actions while the rules are changed. + // - After adding the first rule on a given upstream, the coordinator adds a data limit. + // If the service has never applied the data limit, set an unlimited quota as default. + // - After removing the last rule on a given upstream, the coordinator gets the last stats. + // Then, it clears the stats and the limit entry from BPF maps. + // See tetherOffloadRule{Add, Remove, Clear, Clean}. + + // [1] Adding rules on the upstream Ethernet. + // Note that the default data limit is applied after the first rule is added. + final Ipv6ForwardingRule ethernetRuleA = buildTestForwardingRule( + ethIfIndex, NEIGH_A, MAC_A); + final Ipv6ForwardingRule ethernetRuleB = buildTestForwardingRule( + ethIfIndex, NEIGH_B, MAC_B); + + coordinator.tetherOffloadRuleAdd(mIpServer, ethernetRuleA); + inOrder.verify(mNetd).tetherOffloadRuleAdd(matches(ethernetRuleA)); + inOrder.verify(mNetd).tetherOffloadSetInterfaceQuota(ethIfIndex, QUOTA_UNLIMITED); + + coordinator.tetherOffloadRuleAdd(mIpServer, ethernetRuleB); + inOrder.verify(mNetd).tetherOffloadRuleAdd(matches(ethernetRuleB)); + + // [2] Update the existing rules from Ethernet to cellular. + final Ipv6ForwardingRule mobileRuleA = buildTestForwardingRule( + mobileIfIndex, NEIGH_A, MAC_A); + final Ipv6ForwardingRule mobileRuleB = buildTestForwardingRule( + mobileIfIndex, NEIGH_B, MAC_B); + when(mNetd.tetherOffloadGetAndClearStats(ethIfIndex)) + .thenReturn(buildTestTetherStatsParcel(ethIfIndex, 10, 20, 30, 40)); + + // Update the existing rules for upstream changes. The rules are removed and re-added one + // by one for updating upstream interface index by #tetherOffloadRuleUpdate. + coordinator.tetherOffloadRuleUpdate(mIpServer, mobileIfIndex); + inOrder.verify(mNetd).tetherOffloadRuleRemove(matches(ethernetRuleA)); + inOrder.verify(mNetd).tetherOffloadRuleAdd(matches(mobileRuleA)); + inOrder.verify(mNetd).tetherOffloadSetInterfaceQuota(mobileIfIndex, QUOTA_UNLIMITED); + inOrder.verify(mNetd).tetherOffloadRuleRemove(matches(ethernetRuleB)); + inOrder.verify(mNetd).tetherOffloadGetAndClearStats(ethIfIndex); + inOrder.verify(mNetd).tetherOffloadRuleAdd(matches(mobileRuleB)); + + // [3] Clear all rules for a given IpServer. + when(mNetd.tetherOffloadGetAndClearStats(mobileIfIndex)) + .thenReturn(buildTestTetherStatsParcel(mobileIfIndex, 50, 60, 70, 80)); + coordinator.tetherOffloadRuleClear(mIpServer); + inOrder.verify(mNetd).tetherOffloadRuleRemove(matches(mobileRuleA)); + inOrder.verify(mNetd).tetherOffloadRuleRemove(matches(mobileRuleB)); + inOrder.verify(mNetd).tetherOffloadGetAndClearStats(mobileIfIndex); + + // [4] Force pushing stats update to verify that the last diff of stats is reported on all + // upstreams. + mTetherStatsProvider.pushTetherStats(); + mTetherStatsProviderCb.expectNotifyStatsUpdated( + new NetworkStats(0L, 2) + .addEntry(buildTestEntry(STATS_PER_IFACE, ethIface, 10, 20, 30, 40)) + .addEntry(buildTestEntry(STATS_PER_IFACE, mobileIface, 50, 60, 70, 80)), + new NetworkStats(0L, 2) + .addEntry(buildTestEntry(STATS_PER_UID, ethIface, 10, 20, 30, 40)) + .addEntry(buildTestEntry(STATS_PER_UID, mobileIface, 50, 60, 70, 80))); + } } From a340f25d3ee2dcdac66ee9b3ffd39ee5c8a9a84d Mon Sep 17 00:00:00 2001 From: Nucca Chen Date: Wed, 17 Jun 2020 06:32:35 +0000 Subject: [PATCH 2/3] [BOT.8] Dump BPF offload information in dumpsys $ adb shell dumpsys tethering BPF offload: Polling started Stats provider registered Upstream quota: {rmnet_data2=9223372036854775807} Forwarding stats: 12(rmnet_data2) - ForwardedStats(rxb: 1065, rxp: 5, txb: 0, txp: 0) Forwarding rules: [wlan1]: iif(iface) oif(iface) v6addr srcmac dstmac 12(rmnet_data2) 31(wlan1) /2401:e180:8831:77ae:a900:a03b:41fb.. Bug: 150736748 Test: Enable tethering on mobile data and check dumpsys tethering Original-Change: https://android-review.googlesource.com/1302438 Merged-In: I95ea3050d92f3ba8136a63cd399d3450d183c8dc Change-Id: I95ea3050d92f3ba8136a63cd399d3450d183c8dc --- .../tethering/BpfCoordinator.java | 73 +++++++++++++++++++ .../networkstack/tethering/Tethering.java | 5 ++ 2 files changed, 78 insertions(+) diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index fc27b6add0..4315485f06 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java @@ -36,6 +36,7 @@ import android.net.ip.IpServer; import android.net.netstats.provider.NetworkStatsProvider; import android.net.util.SharedLog; import android.net.util.TetheringUtils.ForwardedStats; +import android.os.ConditionVariable; import android.os.Handler; import android.os.RemoteException; import android.os.ServiceSpecificException; @@ -47,11 +48,13 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import com.android.internal.annotations.VisibleForTesting; +import com.android.internal.util.IndentingPrintWriter; import java.net.Inet6Address; import java.util.ArrayList; import java.util.HashMap; import java.util.LinkedHashMap; +import java.util.Map; import java.util.Objects; /** @@ -65,6 +68,7 @@ import java.util.Objects; */ public class BpfCoordinator { private static final String TAG = BpfCoordinator.class.getSimpleName(); + private static final int DUMP_TIMEOUT_MS = 10_000; @VisibleForTesting static final int DEFAULT_PERFORM_POLL_INTERVAL_MS = 5000; // TODO: Make it customizable. @@ -344,6 +348,75 @@ public class BpfCoordinator { } } + /** + * Dump information. + * Block the function until all the data are dumped on the handler thread or timed-out. The + * reason is that dumpsys invokes this function on the thread of caller and the data may only + * be allowed to be accessed on the handler thread. + */ + public void dump(@NonNull IndentingPrintWriter pw) { + final ConditionVariable dumpDone = new ConditionVariable(); + mHandler.post(() -> { + pw.println("Polling " + (mPollingStarted ? "started" : "not started")); + pw.println("Stats provider " + (mStatsProvider != null + ? "registered" : "not registered")); + pw.println("Upstream quota: " + mInterfaceQuotas.toString()); + + pw.println("Forwarding stats:"); + pw.increaseIndent(); + if (mStats.size() == 0) { + pw.println(""); + } else { + dumpStats(pw); + } + pw.decreaseIndent(); + + pw.println("Forwarding rules:"); + pw.increaseIndent(); + if (mIpv6ForwardingRules.size() == 0) { + pw.println(""); + } else { + dumpIpv6ForwardingRules(pw); + } + pw.decreaseIndent(); + + dumpDone.open(); + }); + if (!dumpDone.block(DUMP_TIMEOUT_MS)) { + pw.println("... dump timed-out after " + DUMP_TIMEOUT_MS + "ms"); + } + } + + private void dumpStats(@NonNull IndentingPrintWriter pw) { + for (int i = 0; i < mStats.size(); i++) { + final int upstreamIfindex = mStats.keyAt(i); + final ForwardedStats stats = mStats.get(upstreamIfindex); + pw.println(String.format("%d(%s) - %s", upstreamIfindex, mInterfaceNames.get( + upstreamIfindex), stats.toString())); + } + } + + private void dumpIpv6ForwardingRules(@NonNull IndentingPrintWriter pw) { + for (Map.Entry> entry : + mIpv6ForwardingRules.entrySet()) { + IpServer ipServer = entry.getKey(); + // The rule downstream interface index is paired with the interface name from + // IpServer#interfaceName. See #startIPv6, #updateIpv6ForwardingRules in IpServer. + final String downstreamIface = ipServer.interfaceName(); + pw.println("[" + downstreamIface + "]: iif(iface) oif(iface) v6addr srcmac dstmac"); + + pw.increaseIndent(); + LinkedHashMap rules = entry.getValue(); + for (Ipv6ForwardingRule rule : rules.values()) { + final int upstreamIfindex = rule.upstreamIfindex; + pw.println(String.format("%d(%s) %d(%s) %s %s %s", upstreamIfindex, + mInterfaceNames.get(upstreamIfindex), rule.downstreamIfindex, + downstreamIface, rule.address, rule.srcMac, rule.dstMac)); + } + pw.decreaseIndent(); + } + } + /** IPv6 forwarding rule class. */ public static class Ipv6ForwardingRule { public final int upstreamIfindex; diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java index 2f01186b11..3184b1ffed 100644 --- a/Tethering/src/com/android/networkstack/tethering/Tethering.java +++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java @@ -2236,6 +2236,11 @@ public class Tethering { mOffloadController.dump(pw); pw.decreaseIndent(); + pw.println("BPF offload:"); + pw.increaseIndent(); + mBpfCoordinator.dump(pw); + pw.decreaseIndent(); + pw.println("Private address coordinator:"); pw.increaseIndent(); mPrivateAddressCoordinator.dump(pw); From d105961e583aca5ff84a84c4451619297591e7fc Mon Sep 17 00:00:00 2001 From: Nucca Chen Date: Wed, 17 Jun 2020 07:03:39 +0000 Subject: [PATCH 3/3] [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 fe737bc2b26b91558bb1aa09f42f35c5af07ee66) --- .../tethering/BpfCoordinator.java | 65 ++++++++++++++++--- .../networkstack/tethering/Tethering.java | 34 +++++++++- .../tethering/TetheringDependencies.java | 5 +- .../unit/src/android/net/ip/IpServerTest.java | 42 ++++++++++-- .../tethering/BpfCoordinatorTest.java | 41 ++++++++++-- .../networkstack/tethering/TetheringTest.java | 4 +- 6 files changed, 162 insertions(+), 29 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index 4315485f06..aa1d59de89 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/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 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 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 rules : mIpv6ForwardingRules .values()) { diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java index 3184b1ffed..99ffa774b5 100644 --- a/Tethering/src/com/android/networkstack/tethering/Tethering.java +++ b/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/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java b/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java index 31f747d3c4..131a5fbf2a 100644 --- a/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java +++ b/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/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java index c3bc915a23..fdfd92617b 100644 --- a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java +++ b/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/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java index ba0f41c4fa..f63a473f1d 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java +++ b/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 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/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java index f53c42b7e7..526199226a 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java +++ b/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; }