Merge "[BOT.11] BpfCoordinator could be disabled by device config" am: e8c985be78 am: 36e37ff125

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1313955

Change-Id: If0f1df54216517aff5af0ce4ae120b25d7fdbaf6

Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java

@@ -89,6 +89,13 @@ public class BpfCoordinator {
     @Nullable
     private final BpfTetherStatsProvider mStatsProvider;
 
+    // True if BPF offload is supported, false otherwise. The BPF offload could be disabled by
+    // a runtime resource overlay package or device configuration. This flag is only initialized
+    // in the constructor because it is hard to unwind all existing change once device
+    // configuration is changed. Especially the forwarding rules. Keep the same setting
+    // to make it simpler. See also TetheringConfiguration.
+    private final boolean mUsingBpf;
+
     // Tracks whether BPF tethering is started or not. This is set by tethering before it
     // starts the first IpServer and is cleared by tethering shortly before the last IpServer
     // is stopped. Note that rule updates (especially deletions, but sometimes additions as
@@ -146,22 +153,42 @@ public class BpfCoordinator {
     };
 
     @VisibleForTesting
-    public static class Dependencies {
+    public abstract static class Dependencies {
-        int getPerformPollInterval() {
+        /**
+         * Get polling Interval in milliseconds.
+         */
+        public int getPerformPollInterval() {
             // TODO: Consider make this configurable.
             return DEFAULT_PERFORM_POLL_INTERVAL_MS;
         }
+
+        /** Get handler. */
+        @NonNull public abstract Handler getHandler();
+
+        /** Get netd. */
+        @NonNull public abstract INetd getNetd();
+
+        /** Get network stats manager. */
+        @NonNull public abstract NetworkStatsManager getNetworkStatsManager();
+
+        /** Get shared log. */
+        @NonNull public abstract SharedLog getSharedLog();
+
+        /** Get tethering configuration. */
+        @Nullable public abstract TetheringConfiguration getTetherConfig();
     }
 
     @VisibleForTesting
-    public BpfCoordinator(@NonNull Handler handler, @NonNull INetd netd,
+    public BpfCoordinator(@NonNull Dependencies deps) {
-            @NonNull NetworkStatsManager nsm, @NonNull SharedLog log, @NonNull Dependencies deps) {
+        mDeps = deps;
-        mHandler = handler;
+        mHandler = mDeps.getHandler();
-        mNetd = netd;
+        mNetd = mDeps.getNetd();
-        mLog = log.forSubComponent(TAG);
+        mLog = mDeps.getSharedLog().forSubComponent(TAG);
+        mUsingBpf = isOffloadEnabled();
         BpfTetherStatsProvider provider = new BpfTetherStatsProvider();
         try {
-            nsm.registerNetworkStatsProvider(getClass().getSimpleName(), provider);
+            mDeps.getNetworkStatsManager().registerNetworkStatsProvider(
+                    getClass().getSimpleName(), provider);
         } catch (RuntimeException e) {
             // TODO: Perhaps not allow to use BPF offload because the reregistration failure
             // implied that no data limit could be applies on a metered upstream if any.
@@ -169,7 +196,6 @@ public class BpfCoordinator {
             provider = null;
         }
         mStatsProvider = provider;
-        mDeps = deps;
     }
 
     /**
@@ -181,6 +207,11 @@ public class BpfCoordinator {
     public void startPolling() {
         if (mPollingStarted) return;
 
+        if (!mUsingBpf) {
+            mLog.i("Offload disabled");
+            return;
+        }
+
         mPollingStarted = true;
         maybeSchedulePollingStats();
 
@@ -215,6 +246,8 @@ public class BpfCoordinator {
      */
     public void tetherOffloadRuleAdd(
             @NonNull final IpServer ipServer, @NonNull final Ipv6ForwardingRule rule) {
+        if (!mUsingBpf) return;
+
         try {
             // TODO: Perhaps avoid to add a duplicate rule.
             mNetd.tetherOffloadRuleAdd(rule.toTetherOffloadRuleParcel());
@@ -254,6 +287,8 @@ public class BpfCoordinator {
      */
     public void tetherOffloadRuleRemove(
             @NonNull final IpServer ipServer, @NonNull final Ipv6ForwardingRule rule) {
+        if (!mUsingBpf) return;
+
         try {
             // TODO: Perhaps avoid to remove a non-existent rule.
             mNetd.tetherOffloadRuleRemove(rule.toTetherOffloadRuleParcel());
@@ -297,6 +332,8 @@ public class BpfCoordinator {
      * Note that this can be only called on handler thread.
      */
     public void tetherOffloadRuleClear(@NonNull final IpServer ipServer) {
+        if (!mUsingBpf) return;
+
         final LinkedHashMap<Inet6Address, Ipv6ForwardingRule> rules = mIpv6ForwardingRules.get(
                 ipServer);
         if (rules == null) return;
@@ -312,6 +349,8 @@ public class BpfCoordinator {
      * Note that this can be only called on handler thread.
      */
     public void tetherOffloadRuleUpdate(@NonNull final IpServer ipServer, int newUpstreamIfindex) {
+        if (!mUsingBpf) return;
+
         final LinkedHashMap<Inet6Address, Ipv6ForwardingRule> rules = mIpv6ForwardingRules.get(
                 ipServer);
         if (rules == null) return;
@@ -334,6 +373,8 @@ public class BpfCoordinator {
      * Note that this can be only called on handler thread.
      */
     public void addUpstreamNameToLookupTable(int upstreamIfindex, @NonNull String upstreamIface) {
+        if (!mUsingBpf) return;
+
         if (upstreamIfindex == 0 || TextUtils.isEmpty(upstreamIface)) return;
 
         // The same interface index to name mapping may be added by different IpServer objects or
@@ -357,6 +398,7 @@ public class BpfCoordinator {
     public void dump(@NonNull IndentingPrintWriter pw) {
         final ConditionVariable dumpDone = new ConditionVariable();
         mHandler.post(() -> {
+            pw.println("mUsingBpf: " + mUsingBpf);
             pw.println("Polling " + (mPollingStarted ? "started" : "not started"));
             pw.println("Stats provider " + (mStatsProvider != null
                     ? "registered" : "not registered"));
@@ -547,6 +589,11 @@ public class BpfCoordinator {
         }
     }
 
+    private boolean isOffloadEnabled() {
+        final TetheringConfiguration config = mDeps.getTetherConfig();
+        return (config != null) ? config.enableBpfOffload : true /* default value */;
+    }
+
     private int getInterfaceIndexFromRules(@NonNull String ifName) {
         for (LinkedHashMap<Inet6Address, Ipv6ForwardingRule> rules : mIpv6ForwardingRules
                 .values()) {

Tethering/src/com/android/networkstack/tethering/Tethering.java

@@ -63,6 +63,7 @@ import static android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID;
 
 import static com.android.networkstack.tethering.TetheringNotificationUpdater.DOWNSTREAM_NONE;
 
+import android.app.usage.NetworkStatsManager;
 import android.bluetooth.BluetoothAdapter;
 import android.bluetooth.BluetoothPan;
 import android.bluetooth.BluetoothProfile;
@@ -286,8 +287,6 @@ public class Tethering {
         mUpstreamNetworkMonitor = mDeps.getUpstreamNetworkMonitor(mContext, mTetherMasterSM, mLog,
                 TetherMasterSM.EVENT_UPSTREAM_CALLBACK);
         mForwardedDownstreams = new LinkedHashSet<>();
-        mBpfCoordinator = mDeps.getBpfCoordinator(
-                mHandler, mNetd, mLog, new BpfCoordinator.Dependencies());
 
         IntentFilter filter = new IntentFilter();
         filter.addAction(ACTION_CARRIER_CONFIG_CHANGED);
@@ -325,6 +324,37 @@ public class Tethering {
         // Load tethering configuration.
         updateConfiguration();
 
+        // Must be initialized after tethering configuration is loaded because BpfCoordinator
+        // constructor needs to use the configuration.
+        mBpfCoordinator = mDeps.getBpfCoordinator(
+                new BpfCoordinator.Dependencies() {
+                    @NonNull
+                    public Handler getHandler() {
+                        return mHandler;
+                    }
+
+                    @NonNull
+                    public INetd getNetd() {
+                        return mNetd;
+                    }
+
+                    @NonNull
+                    public NetworkStatsManager getNetworkStatsManager() {
+                        return (NetworkStatsManager) mContext.getSystemService(
+                            Context.NETWORK_STATS_SERVICE);
+                    }
+
+                    @NonNull
+                    public SharedLog getSharedLog() {
+                        return mLog;
+                    }
+
+                    @Nullable
+                    public TetheringConfiguration getTetherConfig() {
+                        return mConfig;
+                    }
+                });
+
         startStateMachineUpdaters();
     }
 

Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java

@@ -46,11 +46,8 @@ public abstract class TetheringDependencies {
      * Get a reference to the BpfCoordinator to be used by tethering.
      */
     public @NonNull BpfCoordinator getBpfCoordinator(
-            @NonNull Handler handler, @NonNull INetd netd, @NonNull SharedLog log,
             @NonNull BpfCoordinator.Dependencies deps) {
-        final NetworkStatsManager statsManager =
+        return new BpfCoordinator(deps);
-                (NetworkStatsManager) getContext().getSystemService(Context.NETWORK_STATS_SERVICE);
-        return new BpfCoordinator(handler, netd, statsManager, log, deps);
     }
 
     /**

Tethering/tests/unit/src/android/net/ip/IpServerTest.java

@@ -89,12 +89,14 @@ import android.os.test.TestLooper;
 import android.text.TextUtils;
 
 import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
 import androidx.test.filters.SmallTest;
 import androidx.test.runner.AndroidJUnit4;
 
 import com.android.networkstack.tethering.BpfCoordinator;
 import com.android.networkstack.tethering.BpfCoordinator.Ipv6ForwardingRule;
 import com.android.networkstack.tethering.PrivateAddressCoordinator;
+import com.android.networkstack.tethering.TetheringConfiguration;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -226,9 +228,36 @@ public class IpServerTest {
         when(mSharedLog.forSubComponent(anyString())).thenReturn(mSharedLog);
         when(mAddressCoordinator.requestDownstreamAddress(any())).thenReturn(mTestAddress);
 
-        BpfCoordinator bc = new BpfCoordinator(new Handler(mLooper.getLooper()), mNetd,
+        mBpfCoordinator = spy(new BpfCoordinator(
-                mStatsManager, mSharedLog, new BpfCoordinator.Dependencies());
+                new BpfCoordinator.Dependencies() {
-        mBpfCoordinator = spy(bc);
+                    @NonNull
+                    public Handler getHandler() {
+                        return new Handler(mLooper.getLooper());
+                    }
+
+                    @NonNull
+                    public INetd getNetd() {
+                        return mNetd;
+                    }
+
+                    @NonNull
+                    public NetworkStatsManager getNetworkStatsManager() {
+                        return mStatsManager;
+                    }
+
+                    @NonNull
+                    public SharedLog getSharedLog() {
+                        return mSharedLog;
+                    }
+
+                    @Nullable
+                    public TetheringConfiguration getTetherConfig() {
+                        // Returning null configuration object is a hack to enable BPF offload.
+                        // See BpfCoordinator#isOffloadEnabled.
+                        // TODO: Mock TetheringConfiguration to test.
+                        return null;
+                    }
+                }));
     }
 
     @Test
@@ -671,18 +700,21 @@ public class IpServerTest {
         }
     }
 
-    private TetherOffloadRuleParcel matches(
+    @NonNull
+    private static TetherOffloadRuleParcel matches(
             int upstreamIfindex, InetAddress dst, MacAddress dstMac) {
         return argThat(new TetherOffloadRuleParcelMatcher(upstreamIfindex, dst, dstMac));
     }
 
+    @NonNull
     private static Ipv6ForwardingRule makeForwardingRule(
             int upstreamIfindex, @NonNull InetAddress dst, @NonNull MacAddress dstMac) {
         return new Ipv6ForwardingRule(upstreamIfindex, TEST_IFACE_PARAMS.index,
                 (Inet6Address) dst, TEST_IFACE_PARAMS.macAddr, dstMac);
     }
 
-    private TetherStatsParcel buildEmptyTetherStatsParcel(int ifIndex) {
+    @NonNull
+    private static TetherStatsParcel buildEmptyTetherStatsParcel(int ifIndex) {
         TetherStatsParcel parcel = new TetherStatsParcel();
         parcel.ifIndex = ifIndex;
         return parcel;

Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java

@@ -45,7 +45,6 @@ import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
-import android.annotation.NonNull;
 import android.app.usage.NetworkStatsManager;
 import android.net.INetd;
 import android.net.InetAddresses;
@@ -58,6 +57,8 @@ import android.net.util.SharedLog;
 import android.os.Handler;
 import android.os.test.TestLooper;
 
+import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
 import androidx.test.filters.SmallTest;
 import androidx.test.runner.AndroidJUnit4;
 
@@ -101,9 +102,37 @@ public class BpfCoordinatorTest {
     private BpfCoordinator.Dependencies mDeps =
             new BpfCoordinator.Dependencies() {
             @Override
-            int getPerformPollInterval() {
+            public int getPerformPollInterval() {
                 return DEFAULT_PERFORM_POLL_INTERVAL_MS;
             }
+
+            @NonNull
+            public Handler getHandler() {
+                return new Handler(mTestLooper.getLooper());
+            }
+
+            @NonNull
+            public INetd getNetd() {
+                return mNetd;
+            }
+
+            @NonNull
+            public NetworkStatsManager getNetworkStatsManager() {
+                return mStatsManager;
+            }
+
+            @NonNull
+            public SharedLog getSharedLog() {
+                return new SharedLog("test");
+            }
+
+            @Nullable
+            public TetheringConfiguration getTetherConfig() {
+                // Returning null configuration object is a hack to enable BPF offload.
+                // See BpfCoordinator#isOffloadEnabled.
+                // TODO: Mock TetheringConfiguration to test.
+                return null;
+            }
     };
 
     @Before public void setUp() {
@@ -120,9 +149,7 @@ public class BpfCoordinatorTest {
 
     @NonNull
     private BpfCoordinator makeBpfCoordinator() throws Exception {
-        BpfCoordinator coordinator = new BpfCoordinator(
+        final BpfCoordinator coordinator = new BpfCoordinator(mDeps);
-                new Handler(mTestLooper.getLooper()), mNetd, mStatsManager, new SharedLog("test"),
-                mDeps);
         final ArgumentCaptor<BpfCoordinator.BpfTetherStatsProvider>
                 tetherStatsProviderCaptor =
                 ArgumentCaptor.forClass(BpfCoordinator.BpfTetherStatsProvider.class);
@@ -335,8 +362,8 @@ public class BpfCoordinatorTest {
         inOrder.verifyNoMoreInteractions();
 
         // [2] Specific limit.
-        // Applying the data limit boundary {min, max, infinity} on current upstream.
+        // Applying the data limit boundary {min, 1gb, max, infinity} on current upstream.
-        for (final long quota : new long[] {0, Long.MAX_VALUE, QUOTA_UNLIMITED}) {
+        for (final long quota : new long[] {0, 1048576000, Long.MAX_VALUE, QUOTA_UNLIMITED}) {
             mTetherStatsProvider.onSetLimit(mobileIface, quota);
             waitForIdle();
             inOrder.verify(mNetd).tetherOffloadSetInterfaceQuota(mobileIfIndex, quota);

Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java

@@ -346,8 +346,8 @@ public class TetheringTest {
         }
 
         @Override
-        public BpfCoordinator getBpfCoordinator(Handler handler, INetd netd,
+        public BpfCoordinator getBpfCoordinator(
-                SharedLog log, BpfCoordinator.Dependencies deps) {
+                BpfCoordinator.Dependencies deps) {
             return mBpfCoordinator;
         }