From 2fbc671d4a9ae759cb6495f4f01c50e994efc89e Mon Sep 17 00:00:00 2001 From: Nucca Chen Date: Wed, 17 Jun 2020 06:31:45 +0000 Subject: [PATCH 1/6] [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/6] [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/6] [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; } From 0a0d4e156d4dbdd11aaedebc904d6d403a433cfb Mon Sep 17 00:00:00 2001 From: Nucca Chen Date: Wed, 17 Jun 2020 07:03:57 +0000 Subject: [PATCH 4/6] [BOT.12] Add unit test for disabling BpfCoordinator by config Bug: 150736748 Test: BpfCoordinatorTest Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1313802 Merged-In: Iedb936b7592b6be773d1b84a2498bfc5a440a198 Change-Id: I298ae39a1fa61b2cf97752aa908aa2d7d0f9783d (cherry picked from commit 3aba923ce2d52803a32143ef25ecaf9d43a6993c) --- .../tethering/BpfCoordinator.java | 39 +++++++--- .../networkstack/tethering/Tethering.java | 5 +- .../tethering/TetheringConfiguration.java | 14 ++-- .../unit/src/android/net/ip/IpServerTest.java | 7 +- .../tethering/BpfCoordinatorTest.java | 74 +++++++++++++++++-- .../tethering/TetheringConfigurationTest.java | 8 +- 6 files changed, 112 insertions(+), 35 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index aa1d59de89..12464e1289 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java @@ -94,7 +94,7 @@ public class BpfCoordinator { // 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; + private final boolean mIsBpfEnabled; // 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 @@ -184,7 +184,7 @@ public class BpfCoordinator { mHandler = mDeps.getHandler(); mNetd = mDeps.getNetd(); mLog = mDeps.getSharedLog().forSubComponent(TAG); - mUsingBpf = isOffloadEnabled(); + mIsBpfEnabled = isBpfEnabled(); BpfTetherStatsProvider provider = new BpfTetherStatsProvider(); try { mDeps.getNetworkStatsManager().registerNetworkStatsProvider( @@ -207,7 +207,7 @@ public class BpfCoordinator { public void startPolling() { if (mPollingStarted) return; - if (!mUsingBpf) { + if (!mIsBpfEnabled) { mLog.i("Offload disabled"); return; } @@ -246,7 +246,7 @@ public class BpfCoordinator { */ public void tetherOffloadRuleAdd( @NonNull final IpServer ipServer, @NonNull final Ipv6ForwardingRule rule) { - if (!mUsingBpf) return; + if (!mIsBpfEnabled) return; try { // TODO: Perhaps avoid to add a duplicate rule. @@ -287,7 +287,7 @@ public class BpfCoordinator { */ public void tetherOffloadRuleRemove( @NonNull final IpServer ipServer, @NonNull final Ipv6ForwardingRule rule) { - if (!mUsingBpf) return; + if (!mIsBpfEnabled) return; try { // TODO: Perhaps avoid to remove a non-existent rule. @@ -332,7 +332,7 @@ public class BpfCoordinator { * Note that this can be only called on handler thread. */ public void tetherOffloadRuleClear(@NonNull final IpServer ipServer) { - if (!mUsingBpf) return; + if (!mIsBpfEnabled) return; final LinkedHashMap rules = mIpv6ForwardingRules.get( ipServer); @@ -349,7 +349,7 @@ 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; + if (!mIsBpfEnabled) return; final LinkedHashMap rules = mIpv6ForwardingRules.get( ipServer); @@ -373,7 +373,7 @@ 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 (!mIsBpfEnabled) return; if (upstreamIfindex == 0 || TextUtils.isEmpty(upstreamIface)) return; @@ -398,7 +398,7 @@ public class BpfCoordinator { public void dump(@NonNull IndentingPrintWriter pw) { final ConditionVariable dumpDone = new ConditionVariable(); mHandler.post(() -> { - pw.println("mUsingBpf: " + mUsingBpf); + pw.println("mIsBpfEnabled: " + mIsBpfEnabled); pw.println("Polling " + (mPollingStarted ? "started" : "not started")); pw.println("Stats provider " + (mStatsProvider != null ? "registered" : "not registered")); @@ -589,9 +589,9 @@ public class BpfCoordinator { } } - private boolean isOffloadEnabled() { + private boolean isBpfEnabled() { final TetheringConfiguration config = mDeps.getTetherConfig(); - return (config != null) ? config.enableBpfOffload : true /* default value */; + return (config != null) ? config.isBpfOffloadEnabled() : true /* default value */; } private int getInterfaceIndexFromRules(@NonNull String ifName) { @@ -754,4 +754,21 @@ public class BpfCoordinator { mHandler.postDelayed(mScheduledPollingTask, mDeps.getPerformPollInterval()); } + + // Return forwarding rule map. This is used for testing only. + // Note that this can be only called on handler thread. + @NonNull + @VisibleForTesting + final HashMap> + getForwardingRulesForTesting() { + return mIpv6ForwardingRules; + } + + // Return upstream interface name map. This is used for testing only. + // Note that this can be only called on handler thread. + @NonNull + @VisibleForTesting + final SparseArray getInterfaceNamesForTesting() { + return mInterfaceNames; + } } diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java index 99ffa774b5..c72ac52740 100644 --- a/Tethering/src/com/android/networkstack/tethering/Tethering.java +++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java @@ -340,8 +340,7 @@ public class Tethering { @NonNull public NetworkStatsManager getNetworkStatsManager() { - return (NetworkStatsManager) mContext.getSystemService( - Context.NETWORK_STATS_SERVICE); + return mContext.getSystemService(NetworkStatsManager.class); } @NonNull @@ -2405,7 +2404,7 @@ public class Tethering { final TetherState tetherState = new TetherState( new IpServer(iface, mLooper, interfaceType, mLog, mNetd, mBpfCoordinator, makeControlCallback(), mConfig.enableLegacyDhcpServer, - mConfig.enableBpfOffload, mPrivateAddressCoordinator, + mConfig.isBpfOffloadEnabled(), mPrivateAddressCoordinator, mDeps.getIpServerDependencies())); mTetherStates.put(iface, tetherState); tetherState.ipServer.start(); diff --git a/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java b/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java index 1d45f129b5..18b2b7804f 100644 --- a/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java +++ b/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java @@ -101,8 +101,6 @@ public class TetheringConfiguration { public final String[] legacyDhcpRanges; public final String[] defaultIPv4DNS; public final boolean enableLegacyDhcpServer; - // TODO: Add to TetheringConfigurationParcel if required. - public final boolean enableBpfOffload; public final String[] provisioningApp; public final String provisioningAppNoUi; @@ -112,6 +110,8 @@ public class TetheringConfiguration { public final int activeDataSubId; private final int mOffloadPollInterval; + // TODO: Add to TetheringConfigurationParcel if required. + private final boolean mEnableBpfOffload; public TetheringConfiguration(Context ctx, SharedLog log, int id) { final SharedLog configLog = log.forSubComponent("config"); @@ -138,7 +138,7 @@ public class TetheringConfiguration { legacyDhcpRanges = getLegacyDhcpRanges(res); defaultIPv4DNS = copy(DEFAULT_IPV4_DNS); - enableBpfOffload = getEnableBpfOffload(res); + mEnableBpfOffload = getEnableBpfOffload(res); enableLegacyDhcpServer = getEnableLegacyDhcpServer(res); provisioningApp = getResourceStringArray(res, R.array.config_mobile_hotspot_provision_app); @@ -222,7 +222,7 @@ public class TetheringConfiguration { pw.println(provisioningAppNoUi); pw.print("enableBpfOffload: "); - pw.println(enableBpfOffload); + pw.println(mEnableBpfOffload); pw.print("enableLegacyDhcpServer: "); pw.println(enableLegacyDhcpServer); @@ -244,7 +244,7 @@ public class TetheringConfiguration { toIntArray(preferredUpstreamIfaceTypes))); sj.add(String.format("provisioningApp:%s", makeString(provisioningApp))); sj.add(String.format("provisioningAppNoUi:%s", provisioningAppNoUi)); - sj.add(String.format("enableBpfOffload:%s", enableBpfOffload)); + sj.add(String.format("enableBpfOffload:%s", mEnableBpfOffload)); sj.add(String.format("enableLegacyDhcpServer:%s", enableLegacyDhcpServer)); return String.format("TetheringConfiguration{%s}", sj.toString()); } @@ -283,6 +283,10 @@ public class TetheringConfiguration { return mOffloadPollInterval; } + public boolean isBpfOffloadEnabled() { + return mEnableBpfOffload; + } + private static Collection getUpstreamIfaceTypes(Resources res, boolean dunRequired) { final int[] ifaceTypes = res.getIntArray(R.array.config_tether_upstream_types); final ArrayList upstreamIfaceTypes = new ArrayList<>(ifaceTypes.length); diff --git a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java index fdfd92617b..4f88605391 100644 --- a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java +++ b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java @@ -144,6 +144,7 @@ public class IpServerTest { @Mock private IpServer.Dependencies mDependencies; @Mock private PrivateAddressCoordinator mAddressCoordinator; @Mock private NetworkStatsManager mStatsManager; + @Mock private TetheringConfiguration mTetherConfig; @Captor private ArgumentCaptor mDhcpParamsCaptor; @@ -227,6 +228,7 @@ public class IpServerTest { MockitoAnnotations.initMocks(this); when(mSharedLog.forSubComponent(anyString())).thenReturn(mSharedLog); when(mAddressCoordinator.requestDownstreamAddress(any())).thenReturn(mTestAddress); + when(mTetherConfig.isBpfOffloadEnabled()).thenReturn(true /* default value */); mBpfCoordinator = spy(new BpfCoordinator( new BpfCoordinator.Dependencies() { @@ -252,10 +254,7 @@ public class IpServerTest { @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; + return mTetherConfig; } })); } 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 f63a473f1d..31d98147c7 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,11 @@ 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.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyLong; import static org.mockito.Matchers.anyString; @@ -78,6 +79,7 @@ import java.net.Inet6Address; import java.net.InetAddress; import java.util.ArrayList; import java.util.Arrays; +import java.util.LinkedHashMap; @RunWith(AndroidJUnit4.class) @SmallTest @@ -92,6 +94,7 @@ public class BpfCoordinatorTest { @Mock private NetworkStatsManager mStatsManager; @Mock private INetd mNetd; @Mock private IpServer mIpServer; + @Mock private TetheringConfiguration mTetherConfig; // Late init since methods must be called by the thread that created this object. private TestableNetworkStatsProviderCbBinder mTetherStatsProviderCb; @@ -128,15 +131,13 @@ public class BpfCoordinatorTest { @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; + return mTetherConfig; } }; @Before public void setUp() { MockitoAnnotations.initMocks(this); + when(mTetherConfig.isBpfOffloadEnabled()).thenReturn(true /* default value */); } private void waitForIdle() { @@ -501,4 +502,61 @@ public class BpfCoordinatorTest { .addEntry(buildTestEntry(STATS_PER_UID, ethIface, 10, 20, 30, 40)) .addEntry(buildTestEntry(STATS_PER_UID, mobileIface, 50, 60, 70, 80))); } + + @Test + public void testTetheringConfigDisable() throws Exception { + setupFunctioningNetdInterface(); + when(mTetherConfig.isBpfOffloadEnabled()).thenReturn(false); + + final BpfCoordinator coordinator = makeBpfCoordinator(); + coordinator.startPolling(); + + // The tether stats polling task should not be scheduled. + mTestLooper.moveTimeForward(DEFAULT_PERFORM_POLL_INTERVAL_MS); + waitForIdle(); + verify(mNetd, never()).tetherOffloadGetStats(); + + // The interface name lookup table can't be added. + final String iface = "rmnet_data0"; + final Integer ifIndex = 100; + coordinator.addUpstreamNameToLookupTable(ifIndex, iface); + assertEquals(0, coordinator.getInterfaceNamesForTesting().size()); + + // The rule can't be added. + final InetAddress neigh = InetAddresses.parseNumericAddress("2001:db8::1"); + final MacAddress mac = MacAddress.fromString("00:00:00:00:00:0a"); + final Ipv6ForwardingRule rule = buildTestForwardingRule(ifIndex, neigh, mac); + coordinator.tetherOffloadRuleAdd(mIpServer, rule); + verify(mNetd, never()).tetherOffloadRuleAdd(any()); + LinkedHashMap rules = + coordinator.getForwardingRulesForTesting().get(mIpServer); + assertNull(rules); + + // The rule can't be removed. This is not a realistic case because adding rule is not + // allowed. That implies no rule could be removed, cleared or updated. Verify these + // cases just in case. + rules = new LinkedHashMap(); + rules.put(rule.address, rule); + coordinator.getForwardingRulesForTesting().put(mIpServer, rules); + coordinator.tetherOffloadRuleRemove(mIpServer, rule); + verify(mNetd, never()).tetherOffloadRuleRemove(any()); + rules = coordinator.getForwardingRulesForTesting().get(mIpServer); + assertNotNull(rules); + assertEquals(1, rules.size()); + + // The rule can't be cleared. + coordinator.tetherOffloadRuleClear(mIpServer); + verify(mNetd, never()).tetherOffloadRuleRemove(any()); + rules = coordinator.getForwardingRulesForTesting().get(mIpServer); + assertNotNull(rules); + assertEquals(1, rules.size()); + + // The rule can't be updated. + coordinator.tetherOffloadRuleUpdate(mIpServer, rule.upstreamIfindex + 1 /* new */); + verify(mNetd, never()).tetherOffloadRuleRemove(any()); + verify(mNetd, never()).tetherOffloadRuleAdd(any()); + rules = coordinator.getForwardingRulesForTesting().get(mIpServer); + assertNotNull(rules); + assertEquals(1, rules.size()); + } } diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java index 312186391d..a9ac4e2851 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java @@ -294,7 +294,7 @@ public class TetheringConfigurationTest { initializeBpfOffloadConfiguration(true, null /* unset */); final TetheringConfiguration enableByRes = new TetheringConfiguration(mMockContext, mLog, INVALID_SUBSCRIPTION_ID); - assertTrue(enableByRes.enableBpfOffload); + assertTrue(enableByRes.isBpfOffloadEnabled()); } @Test @@ -303,7 +303,7 @@ public class TetheringConfigurationTest { initializeBpfOffloadConfiguration(res, "true"); final TetheringConfiguration enableByDevConOverride = new TetheringConfiguration(mMockContext, mLog, INVALID_SUBSCRIPTION_ID); - assertTrue(enableByDevConOverride.enableBpfOffload); + assertTrue(enableByDevConOverride.isBpfOffloadEnabled()); } } @@ -312,7 +312,7 @@ public class TetheringConfigurationTest { initializeBpfOffloadConfiguration(false, null /* unset */); final TetheringConfiguration disableByRes = new TetheringConfiguration(mMockContext, mLog, INVALID_SUBSCRIPTION_ID); - assertFalse(disableByRes.enableBpfOffload); + assertFalse(disableByRes.isBpfOffloadEnabled()); } @Test @@ -321,7 +321,7 @@ public class TetheringConfigurationTest { initializeBpfOffloadConfiguration(res, "false"); final TetheringConfiguration disableByDevConOverride = new TetheringConfiguration(mMockContext, mLog, INVALID_SUBSCRIPTION_ID); - assertFalse(disableByDevConOverride.enableBpfOffload); + assertFalse(disableByDevConOverride.isBpfOffloadEnabled()); } } From 1c0d8487b48115eba8eb87ed84c1673b5188f028 Mon Sep 17 00:00:00 2001 From: Nucca Chen Date: Wed, 17 Jun 2020 07:04:26 +0000 Subject: [PATCH 5/6] [BOT.13] Make offload coordinator poll interval configurable Bug: 150736748 Test: BpfCoordinatorTest Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1315576 Merged-In: I7f8dde3b57ee14eb33edbe2fd383df33cccc231c Change-Id: I70a6e5c8e765daf40cb738feb7fd70cf3c8f052d (cherry picked from commit bb61893406993556386fa2b8894bad4e3b14bd4e) --- .../tethering/BpfCoordinator.java | 26 ++++--- .../tethering/BpfCoordinatorTest.java | 71 +++++++++++++++---- 2 files changed, 73 insertions(+), 24 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index 12464e1289..20f30ea7a4 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java @@ -25,6 +25,8 @@ import static android.net.NetworkStats.UID_ALL; import static android.net.NetworkStats.UID_TETHERING; import static android.net.netstats.provider.NetworkStatsProvider.QUOTA_UNLIMITED; +import static com.android.networkstack.tethering.TetheringConfiguration.DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS; + import android.app.usage.NetworkStatsManager; import android.net.INetd; import android.net.MacAddress; @@ -69,8 +71,6 @@ 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. @VisibleForTesting enum StatsType { @@ -154,14 +154,6 @@ public class BpfCoordinator { @VisibleForTesting 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(); @@ -403,6 +395,7 @@ public class BpfCoordinator { pw.println("Stats provider " + (mStatsProvider != null ? "registered" : "not registered")); pw.println("Upstream quota: " + mInterfaceQuotas.toString()); + pw.println("Polling interval: " + getPollingInterval() + " ms"); pw.println("Forwarding stats:"); pw.increaseIndent(); @@ -745,6 +738,17 @@ public class BpfCoordinator { updateQuotaAndStatsFromSnapshot(tetherStatsList); } + @VisibleForTesting + int getPollingInterval() { + // The valid range of interval is DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS..max_long. + // Ignore the config value is less than the minimum polling interval. Note that the + // minimum interval definition is invoked as OffloadController#isPollingStatsNeeded does. + // TODO: Perhaps define a minimum polling interval constant. + final TetheringConfiguration config = mDeps.getTetherConfig(); + final int configInterval = (config != null) ? config.getOffloadPollInterval() : 0; + return Math.max(DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS, configInterval); + } + private void maybeSchedulePollingStats() { if (!mPollingStarted) return; @@ -752,7 +756,7 @@ public class BpfCoordinator { mHandler.removeCallbacks(mScheduledPollingTask); } - mHandler.postDelayed(mScheduledPollingTask, mDeps.getPerformPollInterval()); + mHandler.postDelayed(mScheduledPollingTask, getPollingInterval()); } // Return forwarding rule map. This is used for testing only. 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 31d98147c7..64242ae825 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java @@ -25,11 +25,10 @@ import static android.net.NetworkStats.UID_ALL; import static android.net.NetworkStats.UID_TETHERING; import static android.net.netstats.provider.NetworkStatsProvider.QUOTA_UNLIMITED; -import static com.android.networkstack.tethering.BpfCoordinator - .DEFAULT_PERFORM_POLL_INTERVAL_MS; 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 com.android.networkstack.tethering.TetheringConfiguration.DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -104,11 +103,6 @@ public class BpfCoordinatorTest { private final TestLooper mTestLooper = new TestLooper(); private BpfCoordinator.Dependencies mDeps = new BpfCoordinator.Dependencies() { - @Override - public int getPerformPollInterval() { - return DEFAULT_PERFORM_POLL_INTERVAL_MS; - } - @NonNull public Handler getHandler() { return new Handler(mTestLooper.getLooper()); @@ -183,9 +177,11 @@ public class BpfCoordinatorTest { return parcel; } + // Set up specific tether stats list and wait for the stats cache is updated by polling thread + // in the coordinator. Beware of that it is only used for the default polling interval. private void setTetherOffloadStatsList(TetherStatsParcel[] tetherStatsList) throws Exception { when(mNetd.tetherOffloadGetStats()).thenReturn(tetherStatsList); - mTestLooper.moveTimeForward(DEFAULT_PERFORM_POLL_INTERVAL_MS); + mTestLooper.moveTimeForward(DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS); waitForIdle(); } @@ -254,7 +250,7 @@ public class BpfCoordinatorTest { clearInvocations(mNetd); // Verify the polling update thread stopped. - mTestLooper.moveTimeForward(DEFAULT_PERFORM_POLL_INTERVAL_MS); + mTestLooper.moveTimeForward(DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS); waitForIdle(); verify(mNetd, never()).tetherOffloadGetStats(); } @@ -279,20 +275,20 @@ public class BpfCoordinatorTest { when(mNetd.tetherOffloadGetStats()).thenReturn( new TetherStatsParcel[] {buildTestTetherStatsParcel(mobileIfIndex, 0, 0, 0, 0)}); mTetherStatsProvider.onSetAlert(100); - mTestLooper.moveTimeForward(DEFAULT_PERFORM_POLL_INTERVAL_MS); + mTestLooper.moveTimeForward(DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS); waitForIdle(); mTetherStatsProviderCb.assertNoCallback(); // Verify that notifyAlertReached fired when quota is reached. when(mNetd.tetherOffloadGetStats()).thenReturn( new TetherStatsParcel[] {buildTestTetherStatsParcel(mobileIfIndex, 50, 0, 50, 0)}); - mTestLooper.moveTimeForward(DEFAULT_PERFORM_POLL_INTERVAL_MS); + mTestLooper.moveTimeForward(DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS); waitForIdle(); mTetherStatsProviderCb.expectNotifyAlertReached(); // Verify that set quota with UNLIMITED won't trigger any callback. mTetherStatsProvider.onSetAlert(QUOTA_UNLIMITED); - mTestLooper.moveTimeForward(DEFAULT_PERFORM_POLL_INTERVAL_MS); + mTestLooper.moveTimeForward(DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS); waitForIdle(); mTetherStatsProviderCb.assertNoCallback(); } @@ -512,7 +508,7 @@ public class BpfCoordinatorTest { coordinator.startPolling(); // The tether stats polling task should not be scheduled. - mTestLooper.moveTimeForward(DEFAULT_PERFORM_POLL_INTERVAL_MS); + mTestLooper.moveTimeForward(DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS); waitForIdle(); verify(mNetd, never()).tetherOffloadGetStats(); @@ -559,4 +555,53 @@ public class BpfCoordinatorTest { assertNotNull(rules); assertEquals(1, rules.size()); } + + @Test + public void testTetheringConfigSetPollingInterval() throws Exception { + setupFunctioningNetdInterface(); + + final BpfCoordinator coordinator = makeBpfCoordinator(); + + // [1] The default polling interval. + coordinator.startPolling(); + assertEquals(DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS, coordinator.getPollingInterval()); + coordinator.stopPolling(); + + // [2] Expect the invalid polling interval isn't applied. The valid range of interval is + // DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS..max_long. + for (final int interval + : new int[] {0, 100, DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS - 1}) { + when(mTetherConfig.getOffloadPollInterval()).thenReturn(interval); + coordinator.startPolling(); + assertEquals(DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS, coordinator.getPollingInterval()); + coordinator.stopPolling(); + } + + // [3] Set a specific polling interval which is larger than default value. + // Use a large polling interval to avoid flaky test because the time forwarding + // approximation is used to verify the scheduled time of the polling thread. + final int pollingInterval = 100_000; + when(mTetherConfig.getOffloadPollInterval()).thenReturn(pollingInterval); + coordinator.startPolling(); + + // Expect the specific polling interval to be applied. + assertEquals(pollingInterval, coordinator.getPollingInterval()); + + // Start on a new polling time slot. + mTestLooper.moveTimeForward(pollingInterval); + waitForIdle(); + clearInvocations(mNetd); + + // Move time forward to 90% polling interval time. Expect that the polling thread has not + // scheduled yet. + mTestLooper.moveTimeForward((long) (pollingInterval * 0.9)); + waitForIdle(); + verify(mNetd, never()).tetherOffloadGetStats(); + + // Move time forward to the remaining 10% polling interval time. Expect that the polling + // thread has scheduled. + mTestLooper.moveTimeForward((long) (pollingInterval * 0.1)); + waitForIdle(); + verify(mNetd).tetherOffloadGetStats(); + } } From 8dd7e0a936c8e499159cc2b0ac67a3785f36f4ec Mon Sep 17 00:00:00 2001 From: Luke Huang Date: Tue, 16 Jun 2020 08:13:28 +0800 Subject: [PATCH 6/6] Move DnsPacket to libs net This class might be used by some mainline modules. Bug: 151052811 Test: atest DnsPacketTest Test: atest DnsResolverTest Change-Id: I8841d91456952ded5efbf8ea221289aecc7746ad --- Tethering/jarjar-rules.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Tethering/jarjar-rules.txt b/Tethering/jarjar-rules.txt index e90a2ccaa2..8f072e4cd2 100644 --- a/Tethering/jarjar-rules.txt +++ b/Tethering/jarjar-rules.txt @@ -15,3 +15,6 @@ rule com.android.internal.util.TrafficStatsConstants* com.android.networkstack.t rule android.net.LocalLog* com.android.networkstack.tethering.LocalLog@1 rule android.net.shared.Inet4AddressUtils* com.android.networkstack.tethering.shared.Inet4AddressUtils@1 + +# Classes from net-utils-framework-common +rule com.android.net.module.util.** com.android.networkstack.tethering.util.@1 \ No newline at end of file