joy/android_packages_modules_Connectivity
Watch
1
Fork
You've already forked android_packages_modules_Connectivity
 0
Code Issues Pull requests Projects Releases Wiki Activity

Update the logic of isFeatureEnabled and isFeatureNotChickenedOut

Browse source 
If the flag value is unset or 0, isFeatureEnabled return false and
isFeatureNotChickenedOut return true.
If the flag value is -1 (force disable), both return false.
If the flag value is other values, both query the package version and
return `flagValue <= packageVersion`

Now the only difference is a default behavior when the flag is not set
or 0.
So isFeatureEnabled and isFeatureNotChickenedOut can use the same flag.

This CL also fixes the issue in rollback.
Before this CL, isFeatureNotChickenedOut did not check the module
version and could have a issue if there is a rollback.

Test: NetworkStaticLibsTests
Bug: 279108992
Change-Id: I12d6ebadff3aee7b7c614aca4eb0a34ef0db9857
This commit is contained in:
Motomu Utsumi 2023-09-13 14:58:32 +09:00
commit ed4e7ec954
6 changed files with 138 additions and 61 deletions
Download patch file Download diff file Expand all files Collapse all files

7
service-t/src/com/android/server/NsdService.java
View file

@ -26,6 +26,7 @@ import static android.net.nsd.NsdManager.MDNS_DISCOVERY_MANAGER_EVENT;
import static android.net.nsd.NsdManager.MDNS_SERVICE_EVENT;
import static android.net.nsd.NsdManager.RESOLVE_SERVICE_SUCCEEDED;
import static android.provider.DeviceConfig.NAMESPACE_TETHERING;
import static com.android.modules.utils.build.SdkLevel.isAtLeastU;
import static com.android.networkstack.apishim.ConstantsShim.REGISTER_NSD_OFFLOAD_ENGINE;
import static com.android.server.connectivity.mdns.MdnsAdvertiser.AdvertiserMetrics;
@ -1709,7 +1710,7 @@ public class NsdService extends INsdManager.Stub {
mMdnsSocketClient, LOGGER.forSubComponent("MdnsDiscoveryManager"));
handler.post(() -> mMdnsSocketClient.setCallback(mMdnsDiscoveryManager));
MdnsFeatureFlags flags = new MdnsFeatureFlags.Builder().setIsMdnsOffloadFeatureEnabled(
mDeps.isTetheringFeatureNotChickenedOut(
mDeps.isTetheringFeatureNotChickenedOut(mContext,
MdnsFeatureFlags.NSD_FORCE_DISABLE_MDNS_OFFLOAD)).build();
mAdvertiser = deps.makeMdnsAdvertiser(handler.getLooper(), mMdnsSocketProvider,
new AdvertiserCallback(), LOGGER.forSubComponent("MdnsAdvertiser"), flags);
@ -1763,8 +1764,8 @@ public class NsdService extends INsdManager.Stub {
/**
* @see DeviceConfigUtils#isTetheringFeatureNotChickenedOut
*/
public boolean isTetheringFeatureNotChickenedOut(String feature) {
return DeviceConfigUtils.isTetheringFeatureNotChickenedOut(feature);
public boolean isTetheringFeatureNotChickenedOut(Context context, String feature) {
return DeviceConfigUtils.isTetheringFeatureNotChickenedOut(context, feature);
}
/**

2
service/src/com/android/server/BpfNetMaps.java
View file

@ -254,7 +254,7 @@ public class BpfNetMaps {
if (sInitialized) return;
if (sEnableJavaBpfMap == null) {
sEnableJavaBpfMap = SdkLevel.isAtLeastU() ||
DeviceConfigUtils.isTetheringFeatureNotChickenedOut(
DeviceConfigUtils.isTetheringFeatureNotChickenedOut(context,
BPF_NET_MAPS_FORCE_DISABLE_JAVA_BPF_MAP);
}
Log.d(TAG, "BpfNetMaps is initialized with sEnableJavaBpfMap=" + sEnableJavaBpfMap);

2
service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java
View file

@ -974,7 +974,7 @@ public class AutomaticOnOffKeepaliveTracker {
* @return whether the feature is enabled
*/
public boolean isTetheringFeatureNotChickenedOut(@NonNull final String name) {
return DeviceConfigUtils.isTetheringFeatureNotChickenedOut(name);
return DeviceConfigUtils.isTetheringFeatureNotChickenedOut(mContext, name);
}
/**

2
service/src/com/android/server/connectivity/KeepaliveTracker.java
View file

@ -993,7 +993,7 @@ public class KeepaliveTracker {
*/
public boolean isAddressTranslationEnabled(@NonNull Context context) {
return DeviceConfigUtils.isFeatureSupported(context, FEATURE_CLAT_ADDRESS_TRANSLATE)
&& DeviceConfigUtils.isTetheringFeatureNotChickenedOut(
&& DeviceConfigUtils.isTetheringFeatureNotChickenedOut(context,
CONFIG_DISABLE_CLAT_ADDRESS_TRANSLATE);
}
}

74
staticlibs/device/com/android/net/module/util/DeviceConfigUtils.java
View file

@ -40,6 +40,7 @@ import androidx.annotation.VisibleForTesting;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Supplier;
/**
* Utilities for modules to query {@link DeviceConfig} and flags.
@ -70,6 +71,9 @@ public final class DeviceConfigUtils {
sNetworkStackModuleVersion = -1;
}
private static final int FORCE_ENABLE_FEATURE_FLAG_VALUE = 1;
private static final int FORCE_DISABLE_FEATURE_FLAG_VALUE = -1;
private static volatile long sPackageVersion = -1;
private static long getPackageVersion(@NonNull final Context context) {
// sPackageVersion may be set by another thread just after this check, but querying the
@ -186,9 +190,8 @@ public final class DeviceConfigUtils {
*/
public static boolean isNetworkStackFeatureEnabled(@NonNull Context context,
@NonNull String name, boolean defaultEnabled) {
final long packageVersion = getPackageVersion(context);
return isFeatureEnabled(context, packageVersion, NAMESPACE_CONNECTIVITY, name,
defaultEnabled);
return isFeatureEnabled(NAMESPACE_CONNECTIVITY, name, defaultEnabled,
() -> getPackageVersion(context));
}
/**
@ -202,7 +205,7 @@ public final class DeviceConfigUtils {
*
* If the feature is disabled by default and enabled by flag push, this method should be used.
* If the feature is enabled by default and disabled by flag push (kill switch),
* {@link #isTetheringFeatureNotChickenedOut(String)} should be used.
* {@link #isTetheringFeatureNotChickenedOut(Context, String)} should be used.
*
* @param context The global context information about an app environment.
* @param name The name of the property to look up.
@ -210,17 +213,24 @@ public final class DeviceConfigUtils {
*/
public static boolean isTetheringFeatureEnabled(@NonNull Context context,
@NonNull String name) {
final long packageVersion = getTetheringModuleVersion(context);
return isFeatureEnabled(context, packageVersion, NAMESPACE_TETHERING, name,
false /* defaultEnabled */);
return isFeatureEnabled(NAMESPACE_TETHERING, name, false /* defaultEnabled */,
() -> getTetheringModuleVersion(context));
}
private static boolean isFeatureEnabled(@NonNull Context context, long packageVersion,
@NonNull String namespace, String name, boolean defaultEnabled) {
final int propertyVersion = getDeviceConfigPropertyInt(namespace, name,
0 /* default value */);
return (propertyVersion == 0 && defaultEnabled)
|| (propertyVersion != 0 && packageVersion >= (long) propertyVersion);
private static boolean isFeatureEnabled(@NonNull String namespace,
String name, boolean defaultEnabled, Supplier<Long> packageVersionSupplier) {
final int flagValue = getDeviceConfigPropertyInt(namespace, name, 0 /* default value */);
switch (flagValue) {
case 0:
return defaultEnabled;
case FORCE_DISABLE_FEATURE_FLAG_VALUE:
return false;
case FORCE_ENABLE_FEATURE_FLAG_VALUE:
return true;
default:
final long packageVersion = packageVersionSupplier.get();
return packageVersion >= (long) flagValue;
}
}
// Guess the tethering module name based on the package prefix of the connectivity resources
@ -330,43 +340,39 @@ public final class DeviceConfigUtils {
|| moduleVersion >= (featureId & VERSION_MASK);
}
/**
* Check whether one specific experimental feature in specific namespace from
* {@link DeviceConfig} is not disabled. Feature can be disabled by setting a non-zero
* value in the property. If the feature is enabled by default and disabled by flag push
* (kill switch), this method should be used. If the feature is disabled by default and
* enabled by flag push, {@link #isFeatureEnabled} should be used.
*
* @param namespace The namespace containing the property to look up.
* @param name The name of the property to look up.
* @return true if this feature is enabled, or false if disabled.
*/
private static boolean isFeatureNotChickenedOut(String namespace, String name) {
final int propertyVersion = getDeviceConfigPropertyInt(namespace, name,
0 /* default value */);
return propertyVersion == 0;
}
/**
* Check whether one specific experimental feature in Tethering module from {@link DeviceConfig}
* is not disabled.
* If the feature is enabled by default and disabled by flag push (kill switch), this method
* should be used.
* If the feature is disabled by default and enabled by flag push,
* {@link #isTetheringFeatureEnabled(Context, String)} should be used.
*
* @param context The global context information about an app environment.
* @param name The name of the property in tethering module to look up.
* @return true if this feature is enabled, or false if disabled.
*/
public static boolean isTetheringFeatureNotChickenedOut(String name) {
return isFeatureNotChickenedOut(NAMESPACE_TETHERING, name);
public static boolean isTetheringFeatureNotChickenedOut(@NonNull Context context, String name) {
return isFeatureEnabled(NAMESPACE_TETHERING, name, true /* defaultEnabled */,
() -> getTetheringModuleVersion(context));
}
/**
* Check whether one specific experimental feature in NetworkStack module from
* {@link DeviceConfig} is not disabled.
* If the feature is enabled by default and disabled by flag push (kill switch), this method
* should be used.
* If the feature is disabled by default and enabled by flag push,
* {@link #isNetworkStackFeatureEnabled(Context, String)} should be used.
*
* @param context The global context information about an app environment.
* @param name The name of the property in NetworkStack module to look up.
* @return true if this feature is enabled, or false if disabled.
*/
public static boolean isNetworkStackFeatureNotChickenedOut(String name) {
return isFeatureNotChickenedOut(NAMESPACE_CONNECTIVITY, name);
public static boolean isNetworkStackFeatureNotChickenedOut(
@NonNull Context context, String name) {
return isFeatureEnabled(NAMESPACE_CONNECTIVITY, name, true /* defaultEnabled */,
() -> getPackageVersion(context));
}
/**

118
staticlibs/tests/unit/src/com/android/net/module/util/DeviceConfigUtilsTest.java
View file

@ -228,27 +228,57 @@ public class DeviceConfigUtilsTest {
}
@Test
public void testIsNetworkStackFeatureEnabled() {
public void testIsFeatureEnabled() {
doReturn(TEST_FLAG_VALUE_STRING).when(() -> DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY,
TEST_EXPERIMENT_FLAG));
assertTrue(DeviceConfigUtils.isNetworkStackFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
}
@Test
public void testIsTetheringFeatureEnabled() {
doReturn(TEST_FLAG_VALUE_STRING).when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
TEST_EXPERIMENT_FLAG));
assertTrue(DeviceConfigUtils.isNetworkStackFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
assertTrue(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
}
@Test
public void testFeatureDefaultEnabled() {
public void testIsFeatureEnabledFeatureDefaultDisabled() throws Exception {
doReturn(null).when(() -> DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY,
TEST_EXPERIMENT_FLAG));
doReturn(null).when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
TEST_EXPERIMENT_FLAG));
assertFalse(DeviceConfigUtils.isNetworkStackFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
assertFalse(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
// If the flag is unset, package info is not queried
verify(mContext, never()).getPackageManager();
verify(mContext, never()).getPackageName();
verify(mPm, never()).getPackageInfo(anyString(), anyInt());
}
@Test
public void testIsFeatureEnabledFeatureForceEnabled() throws Exception {
doReturn("1").when(() -> DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY,
TEST_EXPERIMENT_FLAG));
doReturn("1").when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
TEST_EXPERIMENT_FLAG));
assertTrue(DeviceConfigUtils.isNetworkStackFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
assertTrue(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
// If the feature is force enabled, package info is not queried
verify(mContext, never()).getPackageManager();
verify(mContext, never()).getPackageName();
verify(mPm, never()).getPackageInfo(anyString(), anyInt());
}
@Test
public void testIsFeatureEnabledFeatureForceDisabled() throws Exception {
doReturn("-1").when(() -> DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY,
TEST_EXPERIMENT_FLAG));
doReturn("-1").when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
TEST_EXPERIMENT_FLAG));
assertFalse(DeviceConfigUtils.isNetworkStackFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
assertFalse(DeviceConfigUtils.isTetheringFeatureEnabled(mContext, TEST_EXPERIMENT_FLAG));
// If the feature is force disabled, package info is not queried
verify(mContext, never()).getPackageManager();
verify(mContext, never()).getPackageName();
verify(mPm, never()).getPackageInfo(anyString(), anyInt());
}
@Test
@ -415,25 +445,65 @@ public class DeviceConfigUtilsTest {
}
@Test
public void testIsTetheringFeatureNotChickenedOut() throws Exception {
doReturn("0").when(() -> DeviceConfig.getProperty(
eq(NAMESPACE_TETHERING), eq(TEST_EXPERIMENT_FLAG)));
assertTrue(DeviceConfigUtils.isTetheringFeatureNotChickenedOut(TEST_EXPERIMENT_FLAG));
doReturn(TEST_FLAG_VALUE_STRING).when(
() -> DeviceConfig.getProperty(eq(NAMESPACE_TETHERING), eq(TEST_EXPERIMENT_FLAG)));
assertFalse(DeviceConfigUtils.isTetheringFeatureNotChickenedOut(TEST_EXPERIMENT_FLAG));
public void testIsFeatureNotChickenedOut() {
doReturn(TEST_FLAG_VALUE_STRING).when(() -> DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY,
TEST_EXPERIMENT_FLAG));
doReturn(TEST_FLAG_VALUE_STRING).when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
TEST_EXPERIMENT_FLAG));
assertTrue(DeviceConfigUtils.isTetheringFeatureNotChickenedOut(
mContext, TEST_EXPERIMENT_FLAG));
assertTrue(DeviceConfigUtils.isNetworkStackFeatureNotChickenedOut(
mContext, TEST_EXPERIMENT_FLAG));
}
@Test
public void testIsNetworkStackFeatureNotChickenedOut() throws Exception {
doReturn("0").when(() -> DeviceConfig.getProperty(
eq(NAMESPACE_CONNECTIVITY), eq(TEST_EXPERIMENT_FLAG)));
assertTrue(DeviceConfigUtils.isNetworkStackFeatureNotChickenedOut(TEST_EXPERIMENT_FLAG));
public void testIsFeatureNotChickenedOutFeatureDefaultEnabled() throws Exception {
doReturn(null).when(() -> DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY,
TEST_EXPERIMENT_FLAG));
doReturn(null).when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
TEST_EXPERIMENT_FLAG));
assertTrue(DeviceConfigUtils.isTetheringFeatureNotChickenedOut(
mContext, TEST_EXPERIMENT_FLAG));
assertTrue(DeviceConfigUtils.isNetworkStackFeatureNotChickenedOut(
mContext, TEST_EXPERIMENT_FLAG));
doReturn(TEST_FLAG_VALUE_STRING).when(
() -> DeviceConfig.getProperty(eq(NAMESPACE_CONNECTIVITY),
eq(TEST_EXPERIMENT_FLAG)));
assertFalse(DeviceConfigUtils.isNetworkStackFeatureNotChickenedOut(TEST_EXPERIMENT_FLAG));
// If the flag is unset, package info is not queried
verify(mContext, never()).getPackageManager();
verify(mContext, never()).getPackageName();
verify(mPm, never()).getPackageInfo(anyString(), anyInt());
}
@Test
public void testIsFeatureNotChickenedOutFeatureForceEnabled() throws Exception {
doReturn("1").when(() -> DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY,
TEST_EXPERIMENT_FLAG));
doReturn("1").when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
TEST_EXPERIMENT_FLAG));
assertTrue(DeviceConfigUtils.isNetworkStackFeatureNotChickenedOut(
mContext, TEST_EXPERIMENT_FLAG));
assertTrue(DeviceConfigUtils.isTetheringFeatureNotChickenedOut(
mContext, TEST_EXPERIMENT_FLAG));
// If the feature is force enabled, package info is not queried
verify(mContext, never()).getPackageManager();
verify(mContext, never()).getPackageName();
verify(mPm, never()).getPackageInfo(anyString(), anyInt());
}
@Test
public void testIsFeatureNotChickenedOutFeatureForceDisabled() throws Exception {
doReturn("-1").when(() -> DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY,
TEST_EXPERIMENT_FLAG));
doReturn("-1").when(() -> DeviceConfig.getProperty(NAMESPACE_TETHERING,
TEST_EXPERIMENT_FLAG));
assertFalse(DeviceConfigUtils.isNetworkStackFeatureNotChickenedOut(
mContext, TEST_EXPERIMENT_FLAG));
assertFalse(DeviceConfigUtils.isTetheringFeatureNotChickenedOut(
mContext, TEST_EXPERIMENT_FLAG));
// If the feature is force disabled, package info is not queried
verify(mContext, never()).getPackageManager();
verify(mContext, never()).getPackageName();
verify(mPm, never()).getPackageInfo(anyString(), anyInt());
}
}