Merge changes I2d6f80f0,I9c26852d am: c26ad188b7

Change-Id: I584138925bf090c1c4b600fc2327220d7e5e39c9

7 files changed, 185 insertions, 21 deletions

diff --git a/packages/Tethering/res/values/config.xml b/packages/Tethering/res/values/config.xml
index c99922196346..0ea459a6cfe5 100644
--- a/packages/Tethering/res/values/config.xml
+++ b/packages/Tethering/res/values/config.xml
@@ -55,6 +55,12 @@
         <item>"bt-pan"</item>
     </string-array>
 
+    <!-- Use the BPF offload for tethering when the kernel has support. True by default.
+         If the device doesn't want to support tether BPF offload, this should be false.
+         Note that this setting could be override by device config.
+    -->
+    <bool translatable="false" name="config_tether_enable_bpf_offload">true</bool>
+
     <!-- Use the old dnsmasq DHCP server for tethering instead of the framework implementation. -->
     <bool translatable="false" name="config_tether_enable_legacy_dhcp_server">false</bool>
 
diff --git a/packages/Tethering/res/values/overlayable.xml b/packages/Tethering/res/values/overlayable.xml
index 4c78a74d5358..288dd5ddf3f8 100644
--- a/packages/Tethering/res/values/overlayable.xml
+++ b/packages/Tethering/res/values/overlayable.xml
@@ -23,6 +23,11 @@
             <item type="array" name="config_tether_wifi_p2p_regexs"/>
             <item type="array" name="config_tether_bluetooth_regexs"/>
             <item type="array" name="config_tether_dhcp_range"/>
+            <!-- Use the BPF offload for tethering when the kernel has support. True by default.
+                 If the device doesn't want to support tether BPF offload, this should be false.
+                 Note that this setting could be override by device config.
+            -->
+            <item type="bool" name="config_tether_enable_bpf_offload"/>
             <item type="bool" name="config_tether_enable_legacy_dhcp_server"/>
             <item type="integer" name="config_tether_offload_poll_interval"/>
             <item type="array" name="config_tether_upstream_types"/>
diff --git a/packages/Tethering/src/android/net/ip/IpServer.java b/packages/Tethering/src/android/net/ip/IpServer.java
index 83727bcdc67e..ca485f5da55b 100644
--- a/packages/Tethering/src/android/net/ip/IpServer.java
+++ b/packages/Tethering/src/android/net/ip/IpServer.java
@@ -227,6 +227,7 @@ public class IpServer extends StateMachine {
     private final int mInterfaceType;
     private final LinkProperties mLinkProperties;
     private final boolean mUsingLegacyDhcp;
+    private final boolean mUsingBpfOffload;
 
     private final Dependencies mDeps;
 
@@ -304,7 +305,8 @@ public class IpServer extends StateMachine {
 
     public IpServer(
             String ifaceName, Looper looper, int interfaceType, SharedLog log,
-            INetd netd, Callback callback, boolean usingLegacyDhcp, Dependencies deps) {
+            INetd netd, Callback callback, boolean usingLegacyDhcp, boolean usingBpfOffload,
+            Dependencies deps) {
         super(ifaceName, looper);
         mLog = log.forSubComponent(ifaceName);
         mNetd = netd;
@@ -314,6 +316,7 @@ public class IpServer extends StateMachine {
         mInterfaceType = interfaceType;
         mLinkProperties = new LinkProperties();
         mUsingLegacyDhcp = usingLegacyDhcp;
+        mUsingBpfOffload = usingBpfOffload;
         mDeps = deps;
         resetLinkProperties();
         mLastError = TetheringManager.TETHER_ERROR_NO_ERROR;
@@ -321,8 +324,15 @@ public class IpServer extends StateMachine {
 
         mIpNeighborMonitor = mDeps.getIpNeighborMonitor(getHandler(), mLog,
                 new MyNeighborEventConsumer());
-        if (!mIpNeighborMonitor.start()) {
-            mLog.e("Failed to create IpNeighborMonitor on " + mIfaceName);
+
+        // IP neighbor monitor monitors the neighbor event for adding/removing offload
+        // forwarding rules per client. If BPF offload is not supported, don't start listening
+        // neighbor events. See updateIpv6ForwardingRules, addIpv6ForwardingRule,
+        // removeIpv6ForwardingRule.
+        if (mUsingBpfOffload) {
+            if (!mIpNeighborMonitor.start()) {
+                mLog.e("Failed to create IpNeighborMonitor on " + mIfaceName);
+            }
         }
 
         mInitialState = new InitialState();
@@ -715,12 +725,12 @@ public class IpServer extends StateMachine {
             final String upstreamIface = v6only.getInterfaceName();
 
             params = new RaParams();
-            // We advertise an mtu lower by 16, which is the closest multiple of 8 >= 14,
-            // the ethernet header size.  This makes kernel ebpf tethering offload happy.
-            // This hack should be reverted once we have the kernel fixed up.
+            // When BPF offload is enabled, we advertise an mtu lower by 16, which is the closest
+            // multiple of 8 >= 14, the ethernet header size. This makes kernel ebpf tethering
+            // offload happy. This hack should be reverted once we have the kernel fixed up.
             // Note: this will automatically clamp to at least 1280 (ipv6 minimum mtu)
             // see RouterAdvertisementDaemon.java putMtu()
-            params.mtu = v6only.getMtu() - 16;
+            params.mtu = mUsingBpfOffload ? v6only.getMtu() - 16 : v6only.getMtu();
             params.hasDefaultRoute = v6only.hasIpv6DefaultRoute();
 
             if (params.hasDefaultRoute) params.hopLimit = getHopLimit(upstreamIface);
@@ -844,6 +854,11 @@ public class IpServer extends StateMachine {
     }
 
     private void addIpv6ForwardingRule(Ipv6ForwardingRule rule) {
+        // Theoretically, we don't need this check because IP neighbor monitor doesn't start if BPF
+        // offload is disabled. Add this check just in case.
+        // TODO: Perhaps remove this protection check.
+        if (!mUsingBpfOffload) return;
+
         try {
             mNetd.tetherOffloadRuleAdd(rule.toTetherOffloadRuleParcel());
             mIpv6ForwardingRules.put(rule.address, rule);
@@ -853,6 +868,11 @@ public class IpServer extends StateMachine {
     }
 
     private void removeIpv6ForwardingRule(Ipv6ForwardingRule rule, boolean removeFromMap) {
+        // Theoretically, we don't need this check because IP neighbor monitor doesn't start if BPF
+        // offload is disabled. Add this check just in case.
+        // TODO: Perhaps remove this protection check.
+        if (!mUsingBpfOffload) return;
+
         try {
             mNetd.tetherOffloadRuleRemove(rule.toTetherOffloadRuleParcel());
             if (removeFromMap) {
diff --git a/packages/Tethering/src/com/android/networkstack/tethering/Tethering.java b/packages/Tethering/src/com/android/networkstack/tethering/Tethering.java
index d1440a7c7be9..bca86cad3e53 100644
--- a/packages/Tethering/src/com/android/networkstack/tethering/Tethering.java
+++ b/packages/Tethering/src/com/android/networkstack/tethering/Tethering.java
@@ -2296,7 +2296,7 @@ public class Tethering {
         final TetherState tetherState = new TetherState(
                 new IpServer(iface, mLooper, interfaceType, mLog, mNetd,
                              makeControlCallback(), mConfig.enableLegacyDhcpServer,
-                             mDeps.getIpServerDependencies()));
+                             mConfig.enableBpfOffload, 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 9d4e74732729..91a6e29a05da 100644
--- a/packages/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java
+++ b/packages/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java
@@ -73,6 +73,12 @@ public class TetheringConfiguration {
     private static final String[] DEFAULT_IPV4_DNS = {"8.8.4.4", "8.8.8.8"};
 
     /**
+     * Override enabling BPF offload configuration for tethering.
+     */
+    public static final String OVERRIDE_TETHER_ENABLE_BPF_OFFLOAD =
+            "override_tether_enable_bpf_offload";
+
+    /**
      * Use the old dnsmasq DHCP server for tethering instead of the framework implementation.
      */
     public static final String TETHER_ENABLE_LEGACY_DHCP_SERVER =
@@ -95,6 +101,8 @@ 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;
@@ -124,11 +132,12 @@ public class TetheringConfiguration {
         isDunRequired = checkDunRequired(ctx);
 
         chooseUpstreamAutomatically = getResourceBoolean(
-                res, R.bool.config_tether_upstream_automatic);
+                res, R.bool.config_tether_upstream_automatic, false /** default value */);
         preferredUpstreamIfaceTypes = getUpstreamIfaceTypes(res, isDunRequired);
 
         legacyDhcpRanges = getLegacyDhcpRanges(res);
         defaultIPv4DNS = copy(DEFAULT_IPV4_DNS);
+        enableBpfOffload = getEnableBpfOffload(res);
         enableLegacyDhcpServer = getEnableLegacyDhcpServer(res);
 
         provisioningApp = getResourceStringArray(res, R.array.config_mobile_hotspot_provision_app);
@@ -208,6 +217,9 @@ public class TetheringConfiguration {
         pw.print("provisioningAppNoUi: ");
         pw.println(provisioningAppNoUi);
 
+        pw.print("enableBpfOffload: ");
+        pw.println(enableBpfOffload);
+
         pw.print("enableLegacyDhcpServer: ");
         pw.println(enableLegacyDhcpServer);
     }
@@ -228,6 +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("enableLegacyDhcpServer:%s", enableLegacyDhcpServer));
         return String.format("TetheringConfiguration{%s}", sj.toString());
     }
@@ -332,11 +345,11 @@ public class TetheringConfiguration {
         }
     }
 
-    private static boolean getResourceBoolean(Resources res, int resId) {
+    private static boolean getResourceBoolean(Resources res, int resId, boolean defaultValue) {
         try {
             return res.getBoolean(resId);
         } catch (Resources.NotFoundException e404) {
-            return false;
+            return defaultValue;
         }
     }
 
@@ -357,8 +370,29 @@ public class TetheringConfiguration {
         }
     }
 
+    private boolean getEnableBpfOffload(final Resources res) {
+        // Get BPF offload config
+        // Priority 1: Device config
+        // Priority 2: Resource config
+        // Priority 3: Default value
+        final boolean resourceValue = getResourceBoolean(
+                res, R.bool.config_tether_enable_bpf_offload, true /** default value */);
+
+        // Due to the limitation of static mock for testing, using #getProperty directly instead
+        // of getDeviceConfigBoolean. getDeviceConfigBoolean is not invoked because it uses
+        // #getBoolean to get the boolean device config. The test can't know that the returned
+        // boolean value comes from device config or default value (because of null property
+        // string). Because the test would like to verify null property boolean string case,
+        // use DeviceConfig.getProperty here. See also the test case testBpfOffload{*} in
+        // TetheringConfigurationTest.java.
+        final String value = DeviceConfig.getProperty(
+                NAMESPACE_CONNECTIVITY, OVERRIDE_TETHER_ENABLE_BPF_OFFLOAD);
+        return (value != null) ? Boolean.parseBoolean(value) : resourceValue;
+    }
+
     private boolean getEnableLegacyDhcpServer(final Resources res) {
-        return getResourceBoolean(res, R.bool.config_tether_enable_legacy_dhcp_server)
+        return getResourceBoolean(
+                res, R.bool.config_tether_enable_legacy_dhcp_server, false /** default value */)
                 || getDeviceConfigBoolean(TETHER_ENABLE_LEGACY_DHCP_SERVER);
     }
 
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 f9be7b9d3664..b9622da9d230 100644
--- a/packages/Tethering/tests/unit/src/android/net/ip/IpServerTest.java
+++ b/packages/Tethering/tests/unit/src/android/net/ip/IpServerTest.java
@@ -106,6 +106,7 @@ public class IpServerTest {
     private static final String BLUETOOTH_IFACE_ADDR = "192.168.42.1";
     private static final int BLUETOOTH_DHCP_PREFIX_LENGTH = 24;
     private static final int DHCP_LEASE_TIME_SECS = 3600;
+    private static final boolean DEFAULT_USING_BPF_OFFLOAD = true;
 
     private static final InterfaceParams TEST_IFACE_PARAMS = new InterfaceParams(
             IFACE_NAME, 42 /* index */, MacAddress.ALL_ZEROS_ADDRESS, 1500 /* defaultMtu */);
@@ -130,10 +131,11 @@ public class IpServerTest {
     private NeighborEventConsumer mNeighborEventConsumer;
 
     private void initStateMachine(int interfaceType) throws Exception {
-        initStateMachine(interfaceType, false /* usingLegacyDhcp */);
+        initStateMachine(interfaceType, false /* usingLegacyDhcp */, DEFAULT_USING_BPF_OFFLOAD);
     }
 
-    private void initStateMachine(int interfaceType, boolean usingLegacyDhcp) throws Exception {
+    private void initStateMachine(int interfaceType, boolean usingLegacyDhcp,
+            boolean usingBpfOffload) throws Exception {
         doAnswer(inv -> {
             final IDhcpServerCallbacks cb = inv.getArgument(2);
             new Thread(() -> {
@@ -165,7 +167,7 @@ public class IpServerTest {
 
         mIpServer = new IpServer(
                 IFACE_NAME, mLooper.getLooper(), interfaceType, mSharedLog, mNetd,
-                mCallback, usingLegacyDhcp, mDependencies);
+                mCallback, usingLegacyDhcp, usingBpfOffload, mDependencies);
         mIpServer.start();
         mNeighborEventConsumer = neighborCaptor.getValue();
 
@@ -179,12 +181,13 @@ public class IpServerTest {
 
     private void initTetheredStateMachine(int interfaceType, String upstreamIface)
             throws Exception {
-        initTetheredStateMachine(interfaceType, upstreamIface, false);
+        initTetheredStateMachine(interfaceType, upstreamIface, false,
+                DEFAULT_USING_BPF_OFFLOAD);
     }
 
     private void initTetheredStateMachine(int interfaceType, String upstreamIface,
-            boolean usingLegacyDhcp) throws Exception {
-        initStateMachine(interfaceType, usingLegacyDhcp);
+            boolean usingLegacyDhcp, boolean usingBpfOffload) throws Exception {
+        initStateMachine(interfaceType, usingLegacyDhcp, usingBpfOffload);
         dispatchCommand(IpServer.CMD_TETHER_REQUESTED, STATE_TETHERED);
         if (upstreamIface != null) {
             LinkProperties lp = new LinkProperties();
@@ -204,7 +207,8 @@ public class IpServerTest {
         when(mDependencies.getIpNeighborMonitor(any(), any(), any()))
                 .thenReturn(mIpNeighborMonitor);
         mIpServer = new IpServer(IFACE_NAME, mLooper.getLooper(), TETHERING_BLUETOOTH, mSharedLog,
-                mNetd, mCallback, false /* usingLegacyDhcp */, mDependencies);
+                mNetd, mCallback, false /* usingLegacyDhcp */, DEFAULT_USING_BPF_OFFLOAD,
+                mDependencies);
         mIpServer.start();
         mLooper.dispatchAll();
         verify(mCallback).updateInterfaceState(
@@ -494,7 +498,8 @@ public class IpServerTest {
 
     @Test
     public void doesNotStartDhcpServerIfDisabled() throws Exception {
-        initTetheredStateMachine(TETHERING_WIFI, UPSTREAM_IFACE, true /* usingLegacyDhcp */);
+        initTetheredStateMachine(TETHERING_WIFI, UPSTREAM_IFACE, true /* usingLegacyDhcp */,
+                DEFAULT_USING_BPF_OFFLOAD);
         dispatchTetherConnectionChanged(UPSTREAM_IFACE);
 
         verify(mDependencies, never()).makeDhcpServer(any(), any(), any());
@@ -577,7 +582,8 @@ public class IpServerTest {
 
     @Test
     public void addRemoveipv6ForwardingRules() throws Exception {
-        initTetheredStateMachine(TETHERING_WIFI, UPSTREAM_IFACE, false /* usingLegacyDhcp */);
+        initTetheredStateMachine(TETHERING_WIFI, UPSTREAM_IFACE, false /* usingLegacyDhcp */,
+                DEFAULT_USING_BPF_OFFLOAD);
 
         final int myIfindex = TEST_IFACE_PARAMS.index;
         final int notMyIfindex = myIfindex - 1;
@@ -678,6 +684,53 @@ public class IpServerTest {
         reset(mNetd);
     }
 
+    @Test
+    public void enableDisableUsingBpfOffload() throws Exception {
+        final int myIfindex = TEST_IFACE_PARAMS.index;
+        final InetAddress neigh = InetAddresses.parseNumericAddress("2001:db8::1");
+        final MacAddress macA = MacAddress.fromString("00:00:00:00:00:0a");
+        final MacAddress macNull = MacAddress.fromString("00:00:00:00:00:00");
+
+        reset(mNetd);
+
+        // Expect that rules can be only added/removed when the BPF offload config is enabled.
+        // Note that the usingBpfOffload false case is not a realistic test case. Because IP
+        // neighbor monitor doesn't start if BPF offload is disabled, there should have no
+        // neighbor event listening. This is used for testing the protection check just in case.
+        // TODO: Perhaps remove this test once we don't need this check anymore.
+        for (boolean usingBpfOffload : new boolean[]{true, false}) {
+            initTetheredStateMachine(TETHERING_WIFI, UPSTREAM_IFACE, false /* usingLegacyDhcp */,
+                    usingBpfOffload);
+
+            // A neighbor is added.
+            recvNewNeigh(myIfindex, neigh, NUD_REACHABLE, macA);
+            if (usingBpfOffload) {
+                verify(mNetd).tetherOffloadRuleAdd(matches(UPSTREAM_IFINDEX, neigh, macA));
+            } else {
+                verify(mNetd, never()).tetherOffloadRuleAdd(any());
+            }
+            reset(mNetd);
+
+            // A neighbor is deleted.
+            recvDelNeigh(myIfindex, neigh, NUD_STALE, macA);
+            if (usingBpfOffload) {
+                verify(mNetd).tetherOffloadRuleRemove(matches(UPSTREAM_IFINDEX, neigh, macNull));
+            } else {
+                verify(mNetd, never()).tetherOffloadRuleRemove(any());
+            }
+            reset(mNetd);
+        }
+    }
+
+    @Test
+    public void doesNotStartIpNeighborMonitorIfBpfOffloadDisabled() throws Exception {
+        initTetheredStateMachine(TETHERING_WIFI, UPSTREAM_IFACE, false /* usingLegacyDhcp */,
+                false /* usingBpfOffload */);
+
+        // IP neighbor monitor doesn't start if BPF offload is disabled.
+        verify(mIpNeighborMonitor, never()).start();
+    }
+
     private void assertDhcpStarted(IpPrefix expectedPrefix) throws Exception {
         verify(mDependencies, times(1)).makeDhcpServer(eq(IFACE_NAME), any(), any());
         verify(mDhcpServer, timeout(MAKE_DHCPSERVER_TIMEOUT_MS).times(1)).startWithCallbacks(
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 e8ba5b8168d7..fbfa871f76c6 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
@@ -127,6 +127,8 @@ public class TetheringConfigurationTest {
                 .thenReturn(new String[0]);
         when(mResources.getBoolean(R.bool.config_tether_enable_legacy_dhcp_server)).thenReturn(
                 false);
+        initializeBpfOffloadConfiguration(true, null /* unset */);
+
         mHasTelephonyManager = true;
         mMockContext = new MockContext(mContext);
         mEnableLegacyDhcpServer = false;
@@ -278,6 +280,50 @@ public class TetheringConfigurationTest {
         assertFalse(upstreamIterator.hasNext());
     }
 
+    private void initializeBpfOffloadConfiguration(
+            final boolean fromRes, final String fromDevConfig) {
+        when(mResources.getBoolean(R.bool.config_tether_enable_bpf_offload)).thenReturn(fromRes);
+        doReturn(fromDevConfig).when(
+                () -> DeviceConfig.getProperty(eq(NAMESPACE_CONNECTIVITY),
+                eq(TetheringConfiguration.OVERRIDE_TETHER_ENABLE_BPF_OFFLOAD)));
+    }
+
+    @Test
+    public void testBpfOffloadEnabledByResource() {
+        initializeBpfOffloadConfiguration(true, null /* unset */);
+        final TetheringConfiguration enableByRes =
+                new TetheringConfiguration(mMockContext, mLog, INVALID_SUBSCRIPTION_ID);
+        assertTrue(enableByRes.enableBpfOffload);
+    }
+
+    @Test
+    public void testBpfOffloadEnabledByDeviceConfigOverride() {
+        for (boolean res : new boolean[]{true, false}) {
+            initializeBpfOffloadConfiguration(res, "true");
+            final TetheringConfiguration enableByDevConOverride =
+                    new TetheringConfiguration(mMockContext, mLog, INVALID_SUBSCRIPTION_ID);
+            assertTrue(enableByDevConOverride.enableBpfOffload);
+        }
+    }
+
+    @Test
+    public void testBpfOffloadDisabledByResource() {
+        initializeBpfOffloadConfiguration(false, null /* unset */);
+        final TetheringConfiguration disableByRes =
+                new TetheringConfiguration(mMockContext, mLog, INVALID_SUBSCRIPTION_ID);
+        assertFalse(disableByRes.enableBpfOffload);
+    }
+
+    @Test
+    public void testBpfOffloadDisabledByDeviceConfigOverride() {
+        for (boolean res : new boolean[]{true, false}) {
+            initializeBpfOffloadConfiguration(res, "false");
+            final TetheringConfiguration disableByDevConOverride =
+                    new TetheringConfiguration(mMockContext, mLog, INVALID_SUBSCRIPTION_ID);
+            assertFalse(disableByDevConOverride.enableBpfOffload);
+        }
+    }
+
     @Test
     public void testNewDhcpServerDisabled() {
         when(mResources.getBoolean(R.bool.config_tether_enable_legacy_dhcp_server)).thenReturn(