Merge "[BOT.12] Add unit test for disabling BpfCoordinator by config"

6 files changed, 112 insertions, 35 deletions

diff --git a/packages/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/packages/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
index aa1d59de89c3..12464e12890d 100644
--- a/packages/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
+++ b/packages/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<Inet6Address, Ipv6ForwardingRule> 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<Inet6Address, Ipv6ForwardingRule> 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<IpServer, LinkedHashMap<Inet6Address, Ipv6ForwardingRule>>
+            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<String> getInterfaceNamesForTesting() {
+        return mInterfaceNames;
+    }
 }
diff --git a/packages/Tethering/src/com/android/networkstack/tethering/Tethering.java b/packages/Tethering/src/com/android/networkstack/tethering/Tethering.java
index 974f799c5054..71ab1767643b 100644
--- a/packages/Tethering/src/com/android/networkstack/tethering/Tethering.java
+++ b/packages/Tethering/src/com/android/networkstack/tethering/Tethering.java
@@ -339,8 +339,7 @@ public class Tethering {
 
                     @NonNull
                     public NetworkStatsManager getNetworkStatsManager() {
-                        return (NetworkStatsManager) mContext.getSystemService(
-                            Context.NETWORK_STATS_SERVICE);
+                        return mContext.getSystemService(NetworkStatsManager.class);
                     }
 
                     @NonNull
@@ -2386,7 +2385,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/packages/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java b/packages/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java
index 48a600dfe6e1..1e94de12ce11 100644
--- a/packages/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java
+++ b/packages/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;
@@ -111,6 +109,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");
@@ -137,7 +137,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);
@@ -218,7 +218,7 @@ public class TetheringConfiguration {
         pw.println(provisioningAppNoUi);
 
         pw.print("enableBpfOffload: ");
-        pw.println(enableBpfOffload);
+        pw.println(mEnableBpfOffload);
 
         pw.print("enableLegacyDhcpServer: ");
         pw.println(enableLegacyDhcpServer);
@@ -240,7 +240,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());
     }
@@ -279,6 +279,10 @@ public class TetheringConfiguration {
         return mOffloadPollInterval;
     }
 
+    public boolean isBpfOffloadEnabled() {
+        return mEnableBpfOffload;
+    }
+
     private static Collection<Integer> getUpstreamIfaceTypes(Resources res, boolean dunRequired) {
         final int[] ifaceTypes = res.getIntArray(R.array.config_tether_upstream_types);
         final ArrayList<Integer> upstreamIfaceTypes = new ArrayList<>(ifaceTypes.length);
diff --git a/packages/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/packages/Tethering/tests/unit/src/android/net/ip/IpServerTest.java
index fdfd92617be2..4f8860539158 100644
--- a/packages/Tethering/tests/unit/src/android/net/ip/IpServerTest.java
+++ b/packages/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<DhcpServingParamsParcel> 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/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java b/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java
index f63a473f1d5c..31d98147c790 100644
--- a/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java
+++ b/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java
@@ -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<Inet6Address, Ipv6ForwardingRule> 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<Inet6Address, Ipv6ForwardingRule>();
+        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/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java b/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java
index 1999ad786ed4..3f4cfe7afb47 100644
--- a/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java
+++ b/packages/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java
@@ -292,7 +292,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
@@ -301,7 +301,7 @@ public class TetheringConfigurationTest {
             initializeBpfOffloadConfiguration(res, "true");
             final TetheringConfiguration enableByDevConOverride =
                     new TetheringConfiguration(mMockContext, mLog, INVALID_SUBSCRIPTION_ID);
-            assertTrue(enableByDevConOverride.enableBpfOffload);
+            assertTrue(enableByDevConOverride.isBpfOffloadEnabled());
         }
     }
 
@@ -310,7 +310,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
@@ -319,7 +319,7 @@ public class TetheringConfigurationTest {
             initializeBpfOffloadConfiguration(res, "false");
             final TetheringConfiguration disableByDevConOverride =
                     new TetheringConfiguration(mMockContext, mLog, INVALID_SUBSCRIPTION_ID);
-            assertFalse(disableByDevConOverride.enableBpfOffload);
+            assertFalse(disableByDevConOverride.isBpfOffloadEnabled());
         }
     }