Refactor BpfNetMaps and getChainEnabled
Address comments from aosp/2117045 and aosp/2131752
Rename USE_NETD to PRE_T
Rename getChainEnabled to isChainEnabled
Remove unnecessary parentheses
Fix comment
Bug: 217624062
Test: atest BpfNetMapsTest
Change-Id: Iaff8c9fc5f74de3fe41a7fb010355b1742fbce90
diff --git a/framework/src/android/net/ConnectivityManager.java b/framework/src/android/net/ConnectivityManager.java
index 02083ff..eeedfd1 100644
--- a/framework/src/android/net/ConnectivityManager.java
+++ b/framework/src/android/net/ConnectivityManager.java
@@ -5922,7 +5922,7 @@
}
/**
- * Get the specified firewall chain status.
+ * Get the specified firewall chain's status.
*
* @param chain target chain.
* @return {@code true} if chain is enabled, {@code false} if chain is disabled.
diff --git a/service/src/com/android/server/BpfNetMaps.java b/service/src/com/android/server/BpfNetMaps.java
index 3ee3ea1..81e9e3a 100644
--- a/service/src/com/android/server/BpfNetMaps.java
+++ b/service/src/com/android/server/BpfNetMaps.java
@@ -53,7 +53,7 @@
private static final String TAG = "BpfNetMaps";
private final INetd mNetd;
// Use legacy netd for releases before T.
- private static final boolean USE_NETD = !SdkLevel.isAtLeastT();
+ private static final boolean PRE_T = !SdkLevel.isAtLeastT();
private static boolean sInitialized = false;
// Lock for sConfigurationMap entry for UID_RULES_CONFIGURATION_KEY.
@@ -112,7 +112,7 @@
*/
private static synchronized void ensureInitialized() {
if (sInitialized) return;
- if (!USE_NETD) {
+ if (!PRE_T) {
System.loadLibrary("service-connectivity");
native_init();
initialize(new Dependencies());
@@ -143,7 +143,7 @@
public BpfNetMaps() {
this(null);
- if (USE_NETD) throw new IllegalArgumentException("BpfNetMaps need to use netd before T");
+ if (PRE_T) throw new IllegalArgumentException("BpfNetMaps need to use netd before T");
}
public BpfNetMaps(final INetd netd) {
@@ -169,8 +169,8 @@
}
}
- private void throwIfUseNetd(final String msg) {
- if (USE_NETD) {
+ private void throwIfPreT(final String msg) {
+ if (PRE_T) {
throw new UnsupportedOperationException(msg);
}
}
@@ -233,7 +233,7 @@
* cause of the failure.
*/
public void setChildChain(final int childChain, final boolean enable) {
- throwIfUseNetd("setChildChain is not available on pre-T devices");
+ throwIfPreT("setChildChain is not available on pre-T devices");
final long match = getMatchByFirewallChain(childChain);
try {
@@ -244,7 +244,7 @@
"Unable to get firewall chain status: sConfigurationMap does not have"
+ " entry for UID_RULES_CONFIGURATION_KEY");
}
- final long newConfig = enable ? (config.val | match) : (config.val & (~match));
+ final long newConfig = enable ? (config.val | match) : (config.val & ~match);
sConfigurationMap.updateEntry(UID_RULES_CONFIGURATION_KEY, new U32(newConfig));
}
} catch (ErrnoException e) {
@@ -254,7 +254,7 @@
}
/**
- * Get the specified firewall chain status.
+ * Get the specified firewall chain's status.
*
* @param childChain target chain
* @return {@code true} if chain is enabled, {@code false} if chain is not enabled.
@@ -262,8 +262,8 @@
* @throws ServiceSpecificException in case of failure, with an error code indicating the
* cause of the failure.
*/
- public boolean getChainEnabled(final int childChain) {
- throwIfUseNetd("getChainEnabled is not available on pre-T devices");
+ public boolean isChainEnabled(final int childChain) {
+ throwIfPreT("isChainEnabled is not available on pre-T devices");
final long match = getMatchByFirewallChain(childChain);
try {
@@ -334,7 +334,7 @@
* cause of the failure.
*/
public void addUidInterfaceRules(final String ifName, final int[] uids) throws RemoteException {
- if (USE_NETD) {
+ if (PRE_T) {
mNetd.firewallAddUidInterfaceRules(ifName, uids);
return;
}
@@ -354,7 +354,7 @@
* cause of the failure.
*/
public void removeUidInterfaceRules(final int[] uids) throws RemoteException {
- if (USE_NETD) {
+ if (PRE_T) {
mNetd.firewallRemoveUidInterfaceRules(uids);
return;
}
@@ -397,7 +397,7 @@
* @throws RemoteException when netd has crashed.
*/
public void setNetPermForUids(final int permissions, final int[] uids) throws RemoteException {
- if (USE_NETD) {
+ if (PRE_T) {
mNetd.trafficSetNetPermForUids(permissions, uids);
return;
}
@@ -413,7 +413,7 @@
*/
public void dump(final FileDescriptor fd, boolean verbose)
throws IOException, ServiceSpecificException {
- if (USE_NETD) {
+ if (PRE_T) {
throw new ServiceSpecificException(
EOPNOTSUPP, "dumpsys connectivity trafficcontroller dump not available on pre-T"
+ " devices, use dumpsys netd trafficcontroller instead.");
diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
index 6568654..b210bb3 100644
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -11387,7 +11387,7 @@
public boolean getFirewallChainEnabled(final int chain) {
enforceNetworkStackOrSettingsPermission();
- return mBpfNetMaps.getChainEnabled(chain);
+ return mBpfNetMaps.isChainEnabled(chain);
}
@Override
diff --git a/tests/unit/java/com/android/server/BpfNetMapsTest.java b/tests/unit/java/com/android/server/BpfNetMapsTest.java
index 99e7ecc..634ec9c 100644
--- a/tests/unit/java/com/android/server/BpfNetMapsTest.java
+++ b/tests/unit/java/com/android/server/BpfNetMapsTest.java
@@ -116,7 +116,7 @@
verify(mNetd).trafficSetNetPermForUids(PERMISSION_INTERNET, TEST_UIDS);
}
- private void doTestGetChainEnabled(final List<Integer> enableChains) throws Exception {
+ private void doTestIsChainEnabled(final List<Integer> enableChains) throws Exception {
long match = 0;
for (final int chain: enableChains) {
match |= mBpfNetMaps.getMatchByFirewallChain(chain);
@@ -126,67 +126,67 @@
for (final int chain: FIREWALL_CHAINS) {
final String testCase = "EnabledChains: " + enableChains + " CheckedChain: " + chain;
if (enableChains.contains(chain)) {
- assertTrue("Expected getChainEnabled returns True, " + testCase,
- mBpfNetMaps.getChainEnabled(chain));
+ assertTrue("Expected isChainEnabled returns True, " + testCase,
+ mBpfNetMaps.isChainEnabled(chain));
} else {
- assertFalse("Expected getChainEnabled returns False, " + testCase,
- mBpfNetMaps.getChainEnabled(chain));
+ assertFalse("Expected isChainEnabled returns False, " + testCase,
+ mBpfNetMaps.isChainEnabled(chain));
}
}
}
- private void doTestGetChainEnabled(final int enableChain) throws Exception {
- doTestGetChainEnabled(List.of(enableChain));
+ private void doTestIsChainEnabled(final int enableChain) throws Exception {
+ doTestIsChainEnabled(List.of(enableChain));
}
@Test
@IgnoreUpTo(Build.VERSION_CODES.S_V2)
- public void testGetChainEnabled() throws Exception {
- doTestGetChainEnabled(FIREWALL_CHAIN_DOZABLE);
- doTestGetChainEnabled(FIREWALL_CHAIN_STANDBY);
- doTestGetChainEnabled(FIREWALL_CHAIN_POWERSAVE);
- doTestGetChainEnabled(FIREWALL_CHAIN_RESTRICTED);
- doTestGetChainEnabled(FIREWALL_CHAIN_LOW_POWER_STANDBY);
- doTestGetChainEnabled(FIREWALL_CHAIN_OEM_DENY_1);
- doTestGetChainEnabled(FIREWALL_CHAIN_OEM_DENY_2);
- doTestGetChainEnabled(FIREWALL_CHAIN_OEM_DENY_3);
+ public void testIsChainEnabled() throws Exception {
+ doTestIsChainEnabled(FIREWALL_CHAIN_DOZABLE);
+ doTestIsChainEnabled(FIREWALL_CHAIN_STANDBY);
+ doTestIsChainEnabled(FIREWALL_CHAIN_POWERSAVE);
+ doTestIsChainEnabled(FIREWALL_CHAIN_RESTRICTED);
+ doTestIsChainEnabled(FIREWALL_CHAIN_LOW_POWER_STANDBY);
+ doTestIsChainEnabled(FIREWALL_CHAIN_OEM_DENY_1);
+ doTestIsChainEnabled(FIREWALL_CHAIN_OEM_DENY_2);
+ doTestIsChainEnabled(FIREWALL_CHAIN_OEM_DENY_3);
}
@Test
@IgnoreUpTo(Build.VERSION_CODES.S_V2)
- public void testGetChainEnabledMultipleChainEnabled() throws Exception {
- doTestGetChainEnabled(List.of(
+ public void testIsChainEnabledMultipleChainEnabled() throws Exception {
+ doTestIsChainEnabled(List.of(
FIREWALL_CHAIN_DOZABLE,
FIREWALL_CHAIN_STANDBY));
- doTestGetChainEnabled(List.of(
+ doTestIsChainEnabled(List.of(
FIREWALL_CHAIN_DOZABLE,
FIREWALL_CHAIN_STANDBY,
FIREWALL_CHAIN_POWERSAVE,
FIREWALL_CHAIN_RESTRICTED));
- doTestGetChainEnabled(FIREWALL_CHAINS);
+ doTestIsChainEnabled(FIREWALL_CHAINS);
}
@Test
@IgnoreUpTo(Build.VERSION_CODES.S_V2)
- public void testGetChainEnabledInvalidChain() {
+ public void testIsChainEnabledInvalidChain() {
final Class<ServiceSpecificException> expected = ServiceSpecificException.class;
- assertThrows(expected, () -> mBpfNetMaps.getChainEnabled(-1 /* childChain */));
- assertThrows(expected, () -> mBpfNetMaps.getChainEnabled(1000 /* childChain */));
+ assertThrows(expected, () -> mBpfNetMaps.isChainEnabled(-1 /* childChain */));
+ assertThrows(expected, () -> mBpfNetMaps.isChainEnabled(1000 /* childChain */));
}
@Test
@IgnoreUpTo(Build.VERSION_CODES.S_V2)
- public void testGetChainEnabledMissingConfiguration() {
+ public void testIsChainEnabledMissingConfiguration() {
// sConfigurationMap does not have entry for UID_RULES_CONFIGURATION_KEY
assertThrows(ServiceSpecificException.class,
- () -> mBpfNetMaps.getChainEnabled(FIREWALL_CHAIN_DOZABLE));
+ () -> mBpfNetMaps.isChainEnabled(FIREWALL_CHAIN_DOZABLE));
}
@Test
@IgnoreAfter(Build.VERSION_CODES.S_V2)
- public void testGetChainEnabledBeforeT() {
+ public void testIsChainEnabledBeforeT() {
assertThrows(UnsupportedOperationException.class,
- () -> mBpfNetMaps.getChainEnabled(FIREWALL_CHAIN_DOZABLE));
+ () -> mBpfNetMaps.isChainEnabled(FIREWALL_CHAIN_DOZABLE));
}
private void doTestSetChildChain(final List<Integer> testChains) throws Exception {