Merge changes I2d6f80f0,I9c26852d am: 50522024a1 am: 6151bb7aed
Change-Id: I52c3ba5b46dc6e52307f68c9201fc19b0a368a09
diff --git a/Tethering/res/values/config.xml b/Tethering/res/values/config.xml
index 9dda716..6604db6 100644
--- a/Tethering/res/values/config.xml
+++ b/Tethering/res/values/config.xml
@@ -57,6 +57,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/Tethering/res/values/overlayable.xml b/Tethering/res/values/overlayable.xml
index 4c78a74..288dd5d 100644
--- a/Tethering/res/values/overlayable.xml
+++ b/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/Tethering/src/android/net/ip/IpServer.java b/Tethering/src/android/net/ip/IpServer.java
index 83727bc..ca485f5 100644
--- a/Tethering/src/android/net/ip/IpServer.java
+++ b/Tethering/src/android/net/ip/IpServer.java
@@ -227,6 +227,7 @@
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 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 @@
mInterfaceType = interfaceType;
mLinkProperties = new LinkProperties();
mUsingLegacyDhcp = usingLegacyDhcp;
+ mUsingBpfOffload = usingBpfOffload;
mDeps = deps;
resetLinkProperties();
mLastError = TetheringManager.TETHER_ERROR_NO_ERROR;
@@ -321,8 +324,15 @@
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 @@
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 @@
}
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 @@
}
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/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java
index ae6119f..2a5e620 100644
--- a/Tethering/src/com/android/networkstack/tethering/Tethering.java
+++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java
@@ -2296,7 +2296,7 @@
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/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java b/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java
index 9d4e747..91a6e29 100644
--- a/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java
+++ b/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java
@@ -73,6 +73,12 @@
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 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 @@
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 @@
pw.print("provisioningAppNoUi: ");
pw.println(provisioningAppNoUi);
+ pw.print("enableBpfOffload: ");
+ pw.println(enableBpfOffload);
+
pw.print("enableLegacyDhcpServer: ");
pw.println(enableLegacyDhcpServer);
}
@@ -228,6 +240,7 @@
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 @@
}
}
- 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 @@
}
}
+ 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/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java
index f9be7b9..b9622da 100644
--- a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java
+++ b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java
@@ -106,6 +106,7 @@
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 @@
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 @@
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 @@
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 @@
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 @@
@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 @@
@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 @@
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/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java
index e8ba5b8..fbfa871 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringConfigurationTest.java
@@ -127,6 +127,8 @@
.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 @@
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(