Move NetworkCapabilities parsing to EthernetConfigParser entirely
Previously, this code path was hard to follow. EthernetConfigParser
parsed the capabilities specified in the xml, while
createDefaultNetworkCapabilities and / or createNetworkCapabilities
added additional capabilities to those specified in the xml.
This change moves the creation of the final NetworkCapabilities for
configured interfaces inside EthernetConfigParser to make it easier to
follow.
Test: TH
Change-Id: I367f53774eeee5aeca0fa7071db0403a6ce8bac5
diff --git a/service-t/src/com/android/server/ethernet/EthernetTracker.java b/service-t/src/com/android/server/ethernet/EthernetTracker.java
index 1c453cb..e41f1d6 100644
--- a/service-t/src/com/android/server/ethernet/EthernetTracker.java
+++ b/service-t/src/com/android/server/ethernet/EthernetTracker.java
@@ -69,7 +69,6 @@
import java.net.NetworkInterface;
import java.net.SocketException;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.Enumeration;
import java.util.Iterator;
import java.util.List;
@@ -104,6 +103,22 @@
// TODO: consider using SharedLog consistently across ethernet service.
private static final SharedLog sLog = new SharedLog(TAG);
+ @VisibleForTesting
+ public static final NetworkCapabilities DEFAULT_CAPABILITIES = new NetworkCapabilities.Builder()
+ .addTransportType(TRANSPORT_ETHERNET)
+ .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
+ .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_CONGESTED)
+ .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED)
+ .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED)
+ .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING)
+ .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED)
+ .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED)
+ // TODO: do not hardcode link bandwidth.
+ .setLinkUpstreamBandwidthKbps(100 * 1000 /* 100 Mbps */)
+ .setLinkDownstreamBandwidthKbps(100 * 1000 /* 100 Mbps */)
+ .build();
+
+
/**
* Interface names we track. This is a product-dependent regular expression.
* Use isValidEthernetInterface to check if a interface name is a valid ethernet interface (this
@@ -648,7 +663,7 @@
nc = mNetworkCapabilities.get(hwAddress);
if (nc == null) {
final boolean isTestIface = iface.matches(TEST_IFACE_REGEXP);
- nc = createDefaultNetworkCapabilities(isTestIface, TRANSPORT_ETHERNET);
+ nc = createDefaultNetworkCapabilities(isTestIface);
}
}
@@ -762,24 +777,9 @@
* <interface name|mac address>;[Network Capabilities];[IP config];[Override Transport]}
*/
private void parseEthernetConfig(String configString) {
- final EthernetConfigParser config = new EthernetConfigParser(configString);
- NetworkCapabilities nc;
- // Starting with Android B (API level 36), we provide default NetworkCapabilities
- // for Ethernet interfaces when no explicit capabilities are specified in the
- // configuration string. This change is made to ensure consistent and expected
- // network behavior for Ethernet devices.
- //
- // It's possible that OEMs or device manufacturers may have relied on the previous
- // behavior (where interfaces without specified capabilities would have minimal
- // capabilities) to prevent certain Ethernet interfaces from becoming
- // the default network. To avoid breaking existing device configurations, this
- // change is gated by the SDK level.
- if (mDeps.isAtLeastB() && config.mCaps.isEmpty()) {
- nc = createDefaultNetworkCapabilities(false /* isTestIface */, config.mTransport);
- } else {
- nc = createNetworkCapabilities(config.mCaps, config.mTransport).build();
- }
- mNetworkCapabilities.put(config.mIface, nc);
+ final EthernetConfigParser config =
+ new EthernetConfigParser(configString, mDeps.isAtLeastB());
+ mNetworkCapabilities.put(config.mIface, config.mCaps);
if (null != config.mIpConfig) {
IpConfiguration ipConfig = parseStaticIpConfiguration(config.mIpConfig);
@@ -787,66 +787,19 @@
}
}
- private static NetworkCapabilities createDefaultNetworkCapabilities(
- boolean isTestIface, int transportType) {
- NetworkCapabilities.Builder builder =
- createNetworkCapabilities(Collections.emptyList(), transportType)
- .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED)
- .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED)
- .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING)
- .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_CONGESTED)
- .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED)
- .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED);
-
+ private static NetworkCapabilities createDefaultNetworkCapabilities(boolean isTestIface) {
+ final NetworkCapabilities.Builder builder =
+ new NetworkCapabilities.Builder(DEFAULT_CAPABILITIES);
if (isTestIface) {
builder.addTransportType(NetworkCapabilities.TRANSPORT_TEST);
- } else {
- builder.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
+ // TODO: do not remove INTERNET capability for test networks.
+ builder.removeCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
}
return builder.build();
}
/**
- * Parses a static list of network capabilities
- *
- * @param capabilities A List of NetworkCapabilities.
- * @param overrideTransport A string representing a single integer encoded override transport
- * type. Must be one of the NetworkCapability.TRANSPORT_*
- * values. TRANSPORT_VPN is not supported. Errors with input
- * will cause the override to be ignored.
- */
- @VisibleForTesting
- static NetworkCapabilities.Builder createNetworkCapabilities(List<Integer> capabilities,
- int transportType) {
-
- final NetworkCapabilities.Builder builder =
- capabilities.isEmpty()
- ? new NetworkCapabilities.Builder()
- : NetworkCapabilities.Builder.withoutDefaultCapabilities();
-
- // Determine the transport type. If someone has tried to define an override transport then
- // attempt to add it. Since we can only have one override, all errors with it will
- // gracefully default back to TRANSPORT_ETHERNET and warn the user. VPN is not allowed as an
- // override type. Wifi Aware and LoWPAN are currently unsupported as well.
- builder.addTransportType(transportType);
-
- builder.setLinkUpstreamBandwidthKbps(100 * 1000);
- builder.setLinkDownstreamBandwidthKbps(100 * 1000);
-
- for (int capability : capabilities) {
- builder.addCapability(capability);
- }
- // Ethernet networks have no way to update the following capabilities, so they always
- // have them.
- builder.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING);
- builder.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_CONGESTED);
- builder.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED);
-
- return builder;
- }
-
- /**
* Parses static IP configuration.
*
* @param staticIpConfig represents static IP configuration in the following format: {@code
@@ -1017,30 +970,43 @@
@VisibleForTesting
static class EthernetConfigParser {
final String mIface;
- final List<Integer> mCaps;
+ final NetworkCapabilities mCaps;
final String mIpConfig;
- final int mTransport;
- private static List<Integer> parseCapabilities(@Nullable String capabilitiesString) {
+ private static NetworkCapabilities parseCapabilities(@Nullable String capabilitiesString,
+ boolean isAtLeastB) {
+ final NetworkCapabilities.Builder builder =
+ NetworkCapabilities.Builder.withoutDefaultCapabilities();
+ builder.setLinkUpstreamBandwidthKbps(100 * 1000 /* 100 Mbps */);
+ builder.setLinkDownstreamBandwidthKbps(100 * 1000 /* 100 Mbps */);
+ // Ethernet networks have no way to update the following capabilities, so they always
+ // have them.
+ builder.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING);
+ builder.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_CONGESTED);
+ builder.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED);
+
if (TextUtils.isEmpty(capabilitiesString)) {
- return Collections.emptyList();
+ if (!isAtLeastB) {
+ return builder.build();
+ }
+ // On Android B+, a null or empty string defaults to the same set of default
+ // capabilities assigned to unconfigured interfaces.
+ return new NetworkCapabilities(DEFAULT_CAPABILITIES);
}
- final ArrayList<Integer> capabilities = new ArrayList<>();
for (String strNetworkCapability : capabilitiesString.split(",")) {
if (TextUtils.isEmpty(strNetworkCapability)) {
continue;
}
final Integer capability;
try {
- capability = Integer.valueOf(strNetworkCapability);
+ builder.addCapability(Integer.valueOf(strNetworkCapability));
} catch (NumberFormatException e) {
Log.e(TAG, "Failed to parse capability: " + strNetworkCapability, e);
continue;
}
- capabilities.add(capability);
}
- return Collections.unmodifiableList(capabilities);
+ return builder.build();
}
private static int parseTransportType(@Nullable String transportString) {
@@ -1071,13 +1037,18 @@
}
}
- EthernetConfigParser(String configString) {
+ EthernetConfigParser(String configString, boolean isAtLeastB) {
Objects.requireNonNull(configString, "EthernetConfigParser requires non-null config");
final String[] tokens = configString.split(";", /* limit of tokens */ 4);
mIface = tokens[0];
- mCaps = parseCapabilities(tokens.length > 1 ? tokens[1] : null);
+
+ final NetworkCapabilities nc =
+ parseCapabilities(tokens.length > 1 ? tokens[1] : null, isAtLeastB);
+ final int transportType = parseTransportType(tokens.length > 3 ? tokens[3] : null);
+ nc.addTransportType(transportType);
+ mCaps = nc;
+
mIpConfig = tokens.length > 2 && !TextUtils.isEmpty(tokens[2]) ? tokens[2] : null;
- mTransport = parseTransportType(tokens.length > 3 ? tokens[3] : null);
}
}
}
diff --git a/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java b/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java
index 03fb505..dc469a1 100644
--- a/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java
+++ b/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java
@@ -180,163 +180,70 @@
.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED);
}
- /**
- * Test: Attempt to create a capabilities with various valid sets of capabilities/transports
- */
+
@Test
- public void createNetworkCapabilities() {
- // Particularly common expected results
- NetworkCapabilities defaultCapabilities =
- makeEthernetCapabilitiesBuilder(false /* clearDefaults */)
- .setLinkUpstreamBandwidthKbps(100000)
- .setLinkDownstreamBandwidthKbps(100000)
- .addTransportType(NetworkCapabilities.TRANSPORT_ETHERNET)
- .build();
+ public void testNetworkCapabilityParsing() {
+ final NetworkCapabilities baseNc = NetworkCapabilities.Builder.withoutDefaultCapabilities()
+ .addTransportType(NetworkCapabilities.TRANSPORT_ETHERNET)
+ .setLinkUpstreamBandwidthKbps(100 * 1000 /* 100 Mbps */)
+ .setLinkDownstreamBandwidthKbps(100 * 1000 /* 100 Mbps */)
+ .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING)
+ .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_CONGESTED)
+ .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED)
+ .build();
- NetworkCapabilities ethernetClearedWithCommonCaps =
- makeEthernetCapabilitiesBuilder(true /* clearDefaults */)
- .setLinkUpstreamBandwidthKbps(100000)
- .setLinkDownstreamBandwidthKbps(100000)
- .addTransportType(NetworkCapabilities.TRANSPORT_ETHERNET)
- .addCapability(12)
- .addCapability(13)
- .addCapability(14)
- .addCapability(15)
- .build();
+ EthernetConfigParser parser = new EthernetConfigParser("eth0;", false /*isAtLeastB*/);
+ assertThat(parser.mCaps).isEqualTo(baseNc);
- // Empty capabilities and transports should return the default capabilities set
- // with TRANSPORT_ETHERNET
- assertParsedNetworkCapabilities(defaultCapabilities, "", "");
+ // On Android B+, empty capabilities default to using DEFAULT_CAPABILITIES.
+ parser = new EthernetConfigParser("eth0;;;;;;;", true /*isAtLeastB*/);
+ assertThat(parser.mCaps).isEqualTo(EthernetTracker.DEFAULT_CAPABILITIES);
- // Adding a list of capabilities will leave exactly those capabilities with a default
- // TRANSPORT_ETHERNET since no overrides are specified
- assertParsedNetworkCapabilities(ethernetClearedWithCommonCaps, "12,13,14,15", "");
+ parser = new EthernetConfigParser("eth0;12,13,14,15;", false /*isAtLeastB*/);
+ assertThat(parser.mCaps.getCapabilities()).asList().containsAtLeast(12, 13, 14, 15);
- // Adding any invalid capabilities to the list will cause them to be ignored
- assertParsedNetworkCapabilities(ethernetClearedWithCommonCaps, "12,13,14,15,65,73", "");
- assertParsedNetworkCapabilities(ethernetClearedWithCommonCaps, "12,13,14,15,abcdefg", "");
+ parser = new EthernetConfigParser("eth0;12,13,500,abc", false /*isAtLeastB*/);
+ // 18, 20, 21 are added by EthernetConfigParser.
+ assertThat(parser.mCaps.getCapabilities()).asList().containsExactly(12, 13, 18, 20, 21);
- // Adding a valid override transport will remove the default TRANSPORT_ETHERNET transport
- // and apply only the override to the capabilities object
- assertParsedNetworkCapabilities(
- makeEthernetCapabilitiesBuilder(false /* clearDefaults */)
- .setLinkUpstreamBandwidthKbps(100000)
- .setLinkDownstreamBandwidthKbps(100000)
- .addTransportType(0)
- .build(),
- "",
- "0");
- assertParsedNetworkCapabilities(
- makeEthernetCapabilitiesBuilder(false /* clearDefaults */)
- .setLinkUpstreamBandwidthKbps(100000)
- .setLinkDownstreamBandwidthKbps(100000)
- .addTransportType(1)
- .build(),
- "",
- "1");
- assertParsedNetworkCapabilities(
- makeEthernetCapabilitiesBuilder(false /* clearDefaults */)
- .setLinkUpstreamBandwidthKbps(100000)
- .setLinkDownstreamBandwidthKbps(100000)
- .addTransportType(2)
- .build(),
- "",
- "2");
- assertParsedNetworkCapabilities(
- makeEthernetCapabilitiesBuilder(false /* clearDefaults */)
- .setLinkUpstreamBandwidthKbps(100000)
- .setLinkDownstreamBandwidthKbps(100000)
- .addTransportType(3)
- .build(),
- "",
- "3");
+ parser = new EthernetConfigParser("eth0;1,2,3;;0", false /*isAtLeastB*/);
+ assertThat(parser.mCaps.getCapabilities()).asList().containsAtLeast(1, 2, 3);
+ assertThat(parser.mCaps.hasSingleTransport(NetworkCapabilities.TRANSPORT_CELLULAR)).isTrue();
- // "4" is TRANSPORT_VPN, which is unsupported. Should default back to TRANSPORT_ETHERNET
- assertParsedNetworkCapabilities(defaultCapabilities, "", "4");
+ // TRANSPORT_VPN (4) is not allowed.
+ parser = new EthernetConfigParser("eth0;;;4", false /*isAtLeastB*/);
+ assertThat(parser.mCaps.hasSingleTransport(NetworkCapabilities.TRANSPORT_ETHERNET)).isTrue();
- // "5" is TRANSPORT_WIFI_AWARE, which is currently supported due to no legacy TYPE_NONE
- // conversion. When that becomes available, this test must be updated
- assertParsedNetworkCapabilities(defaultCapabilities, "", "5");
-
- // "6" is TRANSPORT_LOWPAN, which is currently supported due to no legacy TYPE_NONE
- // conversion. When that becomes available, this test must be updated
- assertParsedNetworkCapabilities(defaultCapabilities, "", "6");
-
- // Adding an invalid override transport will leave the transport as TRANSPORT_ETHERNET
- assertParsedNetworkCapabilities(defaultCapabilities, "", "100");
- assertParsedNetworkCapabilities(defaultCapabilities, "", "abcdefg");
-
- // Ensure the adding of both capabilities and transports work
- assertParsedNetworkCapabilities(
- makeEthernetCapabilitiesBuilder(true /* clearDefaults */)
- .setLinkUpstreamBandwidthKbps(100000)
- .setLinkDownstreamBandwidthKbps(100000)
- .addCapability(12)
- .addCapability(13)
- .addCapability(14)
- .addCapability(15)
- .addTransportType(3)
- .build(),
- "12,13,14,15",
- "3");
-
- // Ensure order does not matter for capability list
- assertParsedNetworkCapabilities(ethernetClearedWithCommonCaps, "13,12,15,14", "");
- }
-
- private void assertParsedNetworkCapabilities(
- NetworkCapabilities expectedNetworkCapabilities,
- String configCapabilities,
- String configTransports) {
- final String ipConfig = "";
- final String configString =
- String.join(";", TEST_IFACE, configCapabilities, ipConfig, configTransports);
- final EthernetConfigParser config = new EthernetConfigParser(configString);
- assertEquals(
- expectedNetworkCapabilities,
- EthernetTracker.createNetworkCapabilities(config.mCaps, config.mTransport).build());
+ // invalid capability and transport type
+ parser = new EthernetConfigParser("eth0;-1,a,1000,,;;-1", false /*isAtLeastB*/);
+ assertThat(parser.mCaps).isEqualTo(baseNc);
}
@Test
- public void testCreateEthernetConfigParserReturnsCorrectValue() {
- final String capabilities = "2,4,6,8";
- final String ipConfig = "3";
- final String transport = "1";
- final String configString = String.join(";", TEST_IFACE, capabilities, ipConfig, transport);
+ public void testInterfaceNameParsing() {
+ EthernetConfigParser parser = new EthernetConfigParser("eth12", false /*isAtLeastB*/);
+ assertThat(parser.mIface).isEqualTo("eth12");
- final EthernetConfigParser config = new EthernetConfigParser(configString);
+ parser = new EthernetConfigParser("", true /*isAtLeastB*/);
+ assertThat(parser.mIface).isEqualTo("");
- assertEquals(TEST_IFACE, config.mIface);
- assertThat(config.mCaps).containsExactly(2, 4, 6, 8);
- assertEquals(ipConfig, config.mIpConfig);
- assertEquals(NetworkCapabilities.TRANSPORT_WIFI, config.mTransport);
+ parser = new EthernetConfigParser("eth0;12;", true /*isAtLeastB*/);
+ assertThat(parser.mIface).isEqualTo("eth0");
}
@Test
- public void testCreateEthernetConfigParser_withInvalidTransport() {
- final String capabilities = "2";
- final String ipConfig = "3";
- final String transport = "100"; // Invalid transport type
- final String configString = String.join(";", TEST_IFACE, capabilities, ipConfig, transport);
-
- final EthernetConfigParser config = new EthernetConfigParser(configString);
- assertEquals(NetworkCapabilities.TRANSPORT_ETHERNET, config.mTransport);
- }
-
- @Test
- public void testCreateEthernetConfigParser_withDisallowedTransport() {
- final String capabilities = "2";
- final String ipConfig = "3";
- final String transport = "4"; // TRANSPORT_VPN is not allowed
- final String configString = String.join(";", TEST_IFACE, capabilities, ipConfig, transport);
-
- final EthernetConfigParser config = new EthernetConfigParser(configString);
- assertEquals(NetworkCapabilities.TRANSPORT_ETHERNET, config.mTransport);
+ public void testIpConfigParsing() {
+ // Note that EthernetConfigParser doesn't actually parse the IpConfig (yet).
+ final EthernetConfigParser parser = new EthernetConfigParser(
+ "eth0;1,2,3;ip=192.168.0.10/24 gateway=192.168.0.1 dns=4.4.4.4,8.8.8.8;1",
+ false /*isAtLeastB*/);
+ assertThat(parser.mIpConfig)
+ .isEqualTo("ip=192.168.0.10/24 gateway=192.168.0.1 dns=4.4.4.4,8.8.8.8");
}
@Test
public void testCreateEthernetConfigParserThrowsNpeWithNullInput() {
- assertThrows(NullPointerException.class, () -> new EthernetConfigParser(null));
+ assertThrows(NullPointerException.class, () -> new EthernetConfigParser(null, false));
}
@Test