diff --git a/service-t/src/com/android/server/NsdService.java b/service-t/src/com/android/server/NsdService.java index 468d7bd76f..93ccb85295 100644 --- a/service-t/src/com/android/server/NsdService.java +++ b/service-t/src/com/android/server/NsdService.java @@ -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); } /** diff --git a/service/src/com/android/server/BpfNetMaps.java b/service/src/com/android/server/BpfNetMaps.java index 4b24aaf850..6a34a242ad 100644 --- a/service/src/com/android/server/BpfNetMaps.java +++ b/service/src/com/android/server/BpfNetMaps.java @@ -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); diff --git a/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java b/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java index 3befcfa43b..11345d3aa8 100644 --- a/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java +++ b/service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java @@ -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); } /** diff --git a/service/src/com/android/server/connectivity/KeepaliveTracker.java b/service/src/com/android/server/connectivity/KeepaliveTracker.java index feba82198d..a51f09f968 100644 --- a/service/src/com/android/server/connectivity/KeepaliveTracker.java +++ b/service/src/com/android/server/connectivity/KeepaliveTracker.java @@ -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); } } diff --git a/staticlibs/device/com/android/net/module/util/DeviceConfigUtils.java b/staticlibs/device/com/android/net/module/util/DeviceConfigUtils.java index e646f37a6e..5b75699774 100644 --- a/staticlibs/device/com/android/net/module/util/DeviceConfigUtils.java +++ b/staticlibs/device/com/android/net/module/util/DeviceConfigUtils.java @@ -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 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)); } /** diff --git a/staticlibs/tests/unit/src/com/android/net/module/util/DeviceConfigUtilsTest.java b/staticlibs/tests/unit/src/com/android/net/module/util/DeviceConfigUtilsTest.java index 5a96bcbb04..9e046765e2 100644 --- a/staticlibs/tests/unit/src/com/android/net/module/util/DeviceConfigUtilsTest.java +++ b/staticlibs/tests/unit/src/com/android/net/module/util/DeviceConfigUtilsTest.java @@ -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()); } }