Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 020ba599 authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Automerger Merge Worker
Browse files

Merge "[BOT.12] Add unit test for disabling BpfCoordinator by config" into...

Merge "[BOT.12] Add unit test for disabling BpfCoordinator by config" into rvc-dev am: 3358560b am: c2cb5067

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

Change-Id: I62f44adbed23d553931dc454658602c4601d3d68
parents 950e9a1f c2cb5067
Loading
Loading
Loading
Loading
+28 −11
Original line number Diff line number Diff line
@@ -94,7 +94,7 @@ public class BpfCoordinator {
    // 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;
    private final boolean mIsBpfEnabled;

    // 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
@@ -184,7 +184,7 @@ public class BpfCoordinator {
        mHandler = mDeps.getHandler();
        mNetd = mDeps.getNetd();
        mLog = mDeps.getSharedLog().forSubComponent(TAG);
        mUsingBpf = isOffloadEnabled();
        mIsBpfEnabled = isBpfEnabled();
        BpfTetherStatsProvider provider = new BpfTetherStatsProvider();
        try {
            mDeps.getNetworkStatsManager().registerNetworkStatsProvider(
@@ -207,7 +207,7 @@ public class BpfCoordinator {
    public void startPolling() {
        if (mPollingStarted) return;

        if (!mUsingBpf) {
        if (!mIsBpfEnabled) {
            mLog.i("Offload disabled");
            return;
        }
@@ -246,7 +246,7 @@ public class BpfCoordinator {
     */
    public void tetherOffloadRuleAdd(
            @NonNull final IpServer ipServer, @NonNull final Ipv6ForwardingRule rule) {
        if (!mUsingBpf) return;
        if (!mIsBpfEnabled) return;

        try {
            // TODO: Perhaps avoid to add a duplicate rule.
@@ -287,7 +287,7 @@ public class BpfCoordinator {
     */
    public void tetherOffloadRuleRemove(
            @NonNull final IpServer ipServer, @NonNull final Ipv6ForwardingRule rule) {
        if (!mUsingBpf) return;
        if (!mIsBpfEnabled) return;

        try {
            // TODO: Perhaps avoid to remove a non-existent rule.
@@ -332,7 +332,7 @@ public class BpfCoordinator {
     * Note that this can be only called on handler thread.
     */
    public void tetherOffloadRuleClear(@NonNull final IpServer ipServer) {
        if (!mUsingBpf) return;
        if (!mIsBpfEnabled) return;

        final LinkedHashMap<Inet6Address, Ipv6ForwardingRule> rules = mIpv6ForwardingRules.get(
                ipServer);
@@ -349,7 +349,7 @@ 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;
        if (!mIsBpfEnabled) return;

        final LinkedHashMap<Inet6Address, Ipv6ForwardingRule> rules = mIpv6ForwardingRules.get(
                ipServer);
@@ -373,7 +373,7 @@ 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 (!mIsBpfEnabled) return;

        if (upstreamIfindex == 0 || TextUtils.isEmpty(upstreamIface)) return;

@@ -398,7 +398,7 @@ public class BpfCoordinator {
    public void dump(@NonNull IndentingPrintWriter pw) {
        final ConditionVariable dumpDone = new ConditionVariable();
        mHandler.post(() -> {
            pw.println("mUsingBpf: " + mUsingBpf);
            pw.println("mIsBpfEnabled: " + mIsBpfEnabled);
            pw.println("Polling " + (mPollingStarted ? "started" : "not started"));
            pw.println("Stats provider " + (mStatsProvider != null
                    ? "registered" : "not registered"));
@@ -589,9 +589,9 @@ public class BpfCoordinator {
        }
    }

    private boolean isOffloadEnabled() {
    private boolean isBpfEnabled() {
        final TetheringConfiguration config = mDeps.getTetherConfig();
        return (config != null) ? config.enableBpfOffload : true /* default value */;
        return (config != null) ? config.isBpfOffloadEnabled() : true /* default value */;
    }

    private int getInterfaceIndexFromRules(@NonNull String ifName) {
@@ -754,4 +754,21 @@ public class BpfCoordinator {

        mHandler.postDelayed(mScheduledPollingTask, mDeps.getPerformPollInterval());
    }

    // Return forwarding rule map. This is used for testing only.
    // Note that this can be only called on handler thread.
    @NonNull
    @VisibleForTesting
    final HashMap<IpServer, LinkedHashMap<Inet6Address, Ipv6ForwardingRule>>
            getForwardingRulesForTesting() {
        return mIpv6ForwardingRules;
    }

    // Return upstream interface name map. This is used for testing only.
    // Note that this can be only called on handler thread.
    @NonNull
    @VisibleForTesting
    final SparseArray<String> getInterfaceNamesForTesting() {
        return mInterfaceNames;
    }
}
+2 −3
Original line number Diff line number Diff line
@@ -340,8 +340,7 @@ public class Tethering {

                    @NonNull
                    public NetworkStatsManager getNetworkStatsManager() {
                        return (NetworkStatsManager) mContext.getSystemService(
                            Context.NETWORK_STATS_SERVICE);
                        return mContext.getSystemService(NetworkStatsManager.class);
                    }

                    @NonNull
@@ -2405,7 +2404,7 @@ public class Tethering {
        final TetherState tetherState = new TetherState(
                new IpServer(iface, mLooper, interfaceType, mLog, mNetd, mBpfCoordinator,
                             makeControlCallback(), mConfig.enableLegacyDhcpServer,
                             mConfig.enableBpfOffload, mPrivateAddressCoordinator,
                             mConfig.isBpfOffloadEnabled(), mPrivateAddressCoordinator,
                             mDeps.getIpServerDependencies()));
        mTetherStates.put(iface, tetherState);
        tetherState.ipServer.start();
+9 −5
Original line number Diff line number Diff line
@@ -101,8 +101,6 @@ public class TetheringConfiguration {
    public final String[] legacyDhcpRanges;
    public final String[] defaultIPv4DNS;
    public final boolean enableLegacyDhcpServer;
    // TODO: Add to TetheringConfigurationParcel if required.
    public final boolean enableBpfOffload;

    public final String[] provisioningApp;
    public final String provisioningAppNoUi;
@@ -112,6 +110,8 @@ public class TetheringConfiguration {
    public final int activeDataSubId;

    private final int mOffloadPollInterval;
    // TODO: Add to TetheringConfigurationParcel if required.
    private final boolean mEnableBpfOffload;

    public TetheringConfiguration(Context ctx, SharedLog log, int id) {
        final SharedLog configLog = log.forSubComponent("config");
@@ -138,7 +138,7 @@ public class TetheringConfiguration {

        legacyDhcpRanges = getLegacyDhcpRanges(res);
        defaultIPv4DNS = copy(DEFAULT_IPV4_DNS);
        enableBpfOffload = getEnableBpfOffload(res);
        mEnableBpfOffload = getEnableBpfOffload(res);
        enableLegacyDhcpServer = getEnableLegacyDhcpServer(res);

        provisioningApp = getResourceStringArray(res, R.array.config_mobile_hotspot_provision_app);
@@ -222,7 +222,7 @@ public class TetheringConfiguration {
        pw.println(provisioningAppNoUi);

        pw.print("enableBpfOffload: ");
        pw.println(enableBpfOffload);
        pw.println(mEnableBpfOffload);

        pw.print("enableLegacyDhcpServer: ");
        pw.println(enableLegacyDhcpServer);
@@ -244,7 +244,7 @@ public class TetheringConfiguration {
                toIntArray(preferredUpstreamIfaceTypes)));
        sj.add(String.format("provisioningApp:%s", makeString(provisioningApp)));
        sj.add(String.format("provisioningAppNoUi:%s", provisioningAppNoUi));
        sj.add(String.format("enableBpfOffload:%s", enableBpfOffload));
        sj.add(String.format("enableBpfOffload:%s", mEnableBpfOffload));
        sj.add(String.format("enableLegacyDhcpServer:%s", enableLegacyDhcpServer));
        return String.format("TetheringConfiguration{%s}", sj.toString());
    }
@@ -283,6 +283,10 @@ public class TetheringConfiguration {
        return mOffloadPollInterval;
    }

    public boolean isBpfOffloadEnabled() {
        return mEnableBpfOffload;
    }

    private static Collection<Integer> getUpstreamIfaceTypes(Resources res, boolean dunRequired) {
        final int[] ifaceTypes = res.getIntArray(R.array.config_tether_upstream_types);
        final ArrayList<Integer> upstreamIfaceTypes = new ArrayList<>(ifaceTypes.length);
+3 −4
Original line number Diff line number Diff line
@@ -144,6 +144,7 @@ public class IpServerTest {
    @Mock private IpServer.Dependencies mDependencies;
    @Mock private PrivateAddressCoordinator mAddressCoordinator;
    @Mock private NetworkStatsManager mStatsManager;
    @Mock private TetheringConfiguration mTetherConfig;

    @Captor private ArgumentCaptor<DhcpServingParamsParcel> mDhcpParamsCaptor;

@@ -227,6 +228,7 @@ public class IpServerTest {
        MockitoAnnotations.initMocks(this);
        when(mSharedLog.forSubComponent(anyString())).thenReturn(mSharedLog);
        when(mAddressCoordinator.requestDownstreamAddress(any())).thenReturn(mTestAddress);
        when(mTetherConfig.isBpfOffloadEnabled()).thenReturn(true /* default value */);

        mBpfCoordinator = spy(new BpfCoordinator(
                new BpfCoordinator.Dependencies() {
@@ -252,10 +254,7 @@ public class IpServerTest {

                    @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;
                        return mTetherConfig;
                    }
                }));
    }
+66 −8
Original line number Diff line number Diff line
@@ -31,10 +31,11 @@ import static com.android.networkstack.tethering.BpfCoordinator.StatsType;
import static com.android.networkstack.tethering.BpfCoordinator.StatsType.STATS_PER_IFACE;
import static com.android.networkstack.tethering.BpfCoordinator.StatsType.STATS_PER_UID;

import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertNotNull;
import static junit.framework.Assert.fail;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.fail;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.anyLong;
import static org.mockito.Matchers.anyString;
@@ -78,6 +79,7 @@ import java.net.Inet6Address;
import java.net.InetAddress;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.LinkedHashMap;

@RunWith(AndroidJUnit4.class)
@SmallTest
@@ -92,6 +94,7 @@ public class BpfCoordinatorTest {
    @Mock private NetworkStatsManager mStatsManager;
    @Mock private INetd mNetd;
    @Mock private IpServer mIpServer;
    @Mock private TetheringConfiguration mTetherConfig;

    // Late init since methods must be called by the thread that created this object.
    private TestableNetworkStatsProviderCbBinder mTetherStatsProviderCb;
@@ -128,15 +131,13 @@ public class BpfCoordinatorTest {

            @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;
                return mTetherConfig;
            }
    };

    @Before public void setUp() {
        MockitoAnnotations.initMocks(this);
        when(mTetherConfig.isBpfOffloadEnabled()).thenReturn(true /* default value */);
    }

    private void waitForIdle() {
@@ -501,4 +502,61 @@ public class BpfCoordinatorTest {
                .addEntry(buildTestEntry(STATS_PER_UID, ethIface, 10, 20, 30, 40))
                .addEntry(buildTestEntry(STATS_PER_UID, mobileIface, 50, 60, 70, 80)));
    }

    @Test
    public void testTetheringConfigDisable() throws Exception {
        setupFunctioningNetdInterface();
        when(mTetherConfig.isBpfOffloadEnabled()).thenReturn(false);

        final BpfCoordinator coordinator = makeBpfCoordinator();
        coordinator.startPolling();

        // The tether stats polling task should not be scheduled.
        mTestLooper.moveTimeForward(DEFAULT_PERFORM_POLL_INTERVAL_MS);
        waitForIdle();
        verify(mNetd, never()).tetherOffloadGetStats();

        // The interface name lookup table can't be added.
        final String iface = "rmnet_data0";
        final Integer ifIndex = 100;
        coordinator.addUpstreamNameToLookupTable(ifIndex, iface);
        assertEquals(0, coordinator.getInterfaceNamesForTesting().size());

        // The rule can't be added.
        final InetAddress neigh = InetAddresses.parseNumericAddress("2001:db8::1");
        final MacAddress mac = MacAddress.fromString("00:00:00:00:00:0a");
        final Ipv6ForwardingRule rule = buildTestForwardingRule(ifIndex, neigh, mac);
        coordinator.tetherOffloadRuleAdd(mIpServer, rule);
        verify(mNetd, never()).tetherOffloadRuleAdd(any());
        LinkedHashMap<Inet6Address, Ipv6ForwardingRule> rules =
                coordinator.getForwardingRulesForTesting().get(mIpServer);
        assertNull(rules);

        // The rule can't be removed. This is not a realistic case because adding rule is not
        // allowed. That implies no rule could be removed, cleared or updated. Verify these
        // cases just in case.
        rules = new LinkedHashMap<Inet6Address, Ipv6ForwardingRule>();
        rules.put(rule.address, rule);
        coordinator.getForwardingRulesForTesting().put(mIpServer, rules);
        coordinator.tetherOffloadRuleRemove(mIpServer, rule);
        verify(mNetd, never()).tetherOffloadRuleRemove(any());
        rules = coordinator.getForwardingRulesForTesting().get(mIpServer);
        assertNotNull(rules);
        assertEquals(1, rules.size());

        // The rule can't be cleared.
        coordinator.tetherOffloadRuleClear(mIpServer);
        verify(mNetd, never()).tetherOffloadRuleRemove(any());
        rules = coordinator.getForwardingRulesForTesting().get(mIpServer);
        assertNotNull(rules);
        assertEquals(1, rules.size());

        // The rule can't be updated.
        coordinator.tetherOffloadRuleUpdate(mIpServer, rule.upstreamIfindex + 1 /* new */);
        verify(mNetd, never()).tetherOffloadRuleRemove(any());
        verify(mNetd, never()).tetherOffloadRuleAdd(any());
        rules = coordinator.getForwardingRulesForTesting().get(mIpServer);
        assertNotNull(rules);
        assertEquals(1, rules.size());
    }
}
Loading