From 3feb78228230a60ba515fa681ce76b4ea27cc36f Mon Sep 17 00:00:00 2001 From: Hungming Chen Date: Tue, 11 May 2021 21:07:18 +0800 Subject: [PATCH] bpf: Add interface index to BPF devmap Add upstream and downstream interface index to BPF map and rename the BPF map name from xdp_devmap to dev. $ adb shell dumpsys tethering Device map: ifindex (iface) -> ifindex (iface) 21 (21) -> 21 (21) 25 (25) -> 25 (25) 12 (rmnet_data2) -> 12 (rmnet_data2) $ adb shell ip addr 12: rmnet_data2 .. 21: wlan1 .. 25: rndis0 .. $ adb shell ls /sys/fs/bpf/tethering map_offload_tether_dev_map Test: atest TetheringCoverageTests Change-Id: Ic49965f3374d9e196ee672ec2f0e9e08f3847deb --- .../apishim/api30/BpfCoordinatorShimImpl.java | 12 ++++ .../apishim/api31/BpfCoordinatorShimImpl.java | 44 ++++++++++++- .../apishim/common/BpfCoordinatorShim.java | 10 +++ Tethering/bpf_progs/offload.c | 3 +- .../tethering/BpfCoordinator.java | 62 +++++++++++++++++++ .../networkstack/tethering/TetherDevKey.java | 31 ++++++++++ .../tethering/TetherDevValue.java | 31 ++++++++++ .../unit/src/android/net/ip/IpServerTest.java | 8 +++ .../tethering/BpfCoordinatorTest.java | 61 ++++++++++++++++-- 9 files changed, 252 insertions(+), 10 deletions(-) create mode 100644 Tethering/src/com/android/networkstack/tethering/TetherDevKey.java create mode 100644 Tethering/src/com/android/networkstack/tethering/TetherDevValue.java diff --git a/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java b/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java index e310fb62f1..33f1c29ea8 100644 --- a/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java +++ b/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java @@ -178,6 +178,18 @@ public class BpfCoordinatorShimImpl return false; } + @Override + public boolean addDevMap(int ifIndex) { + /* no op */ + return false; + } + + @Override + public boolean removeDevMap(int ifIndex) { + /* no op */ + return false; + } + @Override public String toString() { return "Netd used"; diff --git a/Tethering/apishim/31/com/android/networkstack/tethering/apishim/api31/BpfCoordinatorShimImpl.java b/Tethering/apishim/31/com/android/networkstack/tethering/apishim/api31/BpfCoordinatorShimImpl.java index d7ce1392b5..74ddcbce18 100644 --- a/Tethering/apishim/31/com/android/networkstack/tethering/apishim/api31/BpfCoordinatorShimImpl.java +++ b/Tethering/apishim/31/com/android/networkstack/tethering/apishim/api31/BpfCoordinatorShimImpl.java @@ -36,6 +36,8 @@ import com.android.networkstack.tethering.BpfUtils; import com.android.networkstack.tethering.Tether4Key; import com.android.networkstack.tethering.Tether4Value; import com.android.networkstack.tethering.Tether6Value; +import com.android.networkstack.tethering.TetherDevKey; +import com.android.networkstack.tethering.TetherDevValue; import com.android.networkstack.tethering.TetherDownstream6Key; import com.android.networkstack.tethering.TetherLimitKey; import com.android.networkstack.tethering.TetherLimitValue; @@ -85,6 +87,10 @@ public class BpfCoordinatorShimImpl @Nullable private final BpfMap mBpfLimitMap; + // BPF map of interface index mapping for XDP. + @Nullable + private final BpfMap mBpfDevMap; + // Tracking IPv4 rule count while any rule is using the given upstream interfaces. Used for // reducing the BPF map iteration query. The count is increased or decreased when the rule is // added or removed successfully on mBpfDownstream4Map. Counting the rules on downstream4 map @@ -108,6 +114,7 @@ public class BpfCoordinatorShimImpl mBpfUpstream6Map = deps.getBpfUpstream6Map(); mBpfStatsMap = deps.getBpfStatsMap(); mBpfLimitMap = deps.getBpfLimitMap(); + mBpfDevMap = deps.getBpfDevMap(); // Clear the stubs of the maps for handling the system service crash if any. // Doesn't throw the exception and clear the stubs as many as possible. @@ -141,12 +148,18 @@ public class BpfCoordinatorShimImpl } catch (ErrnoException e) { mLog.e("Could not clear mBpfLimitMap: " + e); } + try { + if (mBpfDevMap != null) mBpfDevMap.clear(); + } catch (ErrnoException e) { + mLog.e("Could not clear mBpfDevMap: " + e); + } } @Override public boolean isInitialized() { return mBpfDownstream4Map != null && mBpfUpstream4Map != null && mBpfDownstream6Map != null - && mBpfUpstream6Map != null && mBpfStatsMap != null && mBpfLimitMap != null; + && mBpfUpstream6Map != null && mBpfStatsMap != null && mBpfLimitMap != null + && mBpfDevMap != null; } @Override @@ -432,6 +445,32 @@ public class BpfCoordinatorShimImpl return mRule4CountOnUpstream.get(ifIndex) != null; } + @Override + public boolean addDevMap(int ifIndex) { + if (!isInitialized()) return false; + + try { + mBpfDevMap.updateEntry(new TetherDevKey(ifIndex), new TetherDevValue(ifIndex)); + } catch (ErrnoException e) { + mLog.e("Could not add interface " + ifIndex + ": " + e); + return false; + } + return true; + } + + @Override + public boolean removeDevMap(int ifIndex) { + if (!isInitialized()) return false; + + try { + mBpfDevMap.deleteEntry(new TetherDevKey(ifIndex)); + } catch (ErrnoException e) { + mLog.e("Could not delete interface " + ifIndex + ": " + e); + return false; + } + return true; + } + private String mapStatus(BpfMap m, String name) { return name + "{" + (m != null ? "OK" : "ERROR") + "}"; } @@ -444,7 +483,8 @@ public class BpfCoordinatorShimImpl mapStatus(mBpfDownstream4Map, "mBpfDownstream4Map"), mapStatus(mBpfUpstream4Map, "mBpfUpstream4Map"), mapStatus(mBpfStatsMap, "mBpfStatsMap"), - mapStatus(mBpfLimitMap, "mBpfLimitMap") + mapStatus(mBpfLimitMap, "mBpfLimitMap"), + mapStatus(mBpfDevMap, "mBpfDevMap") }); } diff --git a/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java b/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java index 79a628b403..8a7a49c961 100644 --- a/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java +++ b/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java @@ -166,5 +166,15 @@ public abstract class BpfCoordinatorShim { * TODO: consider using InterfaceParams to replace interface name. */ public abstract boolean detachProgram(@NonNull String iface); + + /** + * Add interface index mapping. + */ + public abstract boolean addDevMap(int ifIndex); + + /** + * Remove interface index mapping. + */ + public abstract boolean removeDevMap(int ifIndex); } diff --git a/Tethering/bpf_progs/offload.c b/Tethering/bpf_progs/offload.c index 36f6783dc2..6ff370c23e 100644 --- a/Tethering/bpf_progs/offload.c +++ b/Tethering/bpf_progs/offload.c @@ -767,8 +767,7 @@ DEFINE_BPF_PROG_KVER_RANGE("schedcls/tether_upstream4_ether$stub", AID_ROOT, AID // ----- XDP Support ----- -DEFINE_BPF_MAP_GRW(tether_xdp_devmap, DEVMAP_HASH, uint32_t, uint32_t, 64, - AID_NETWORK_STACK) +DEFINE_BPF_MAP_GRW(tether_dev_map, DEVMAP_HASH, uint32_t, uint32_t, 64, AID_NETWORK_STACK) static inline __always_inline int do_xdp_forward6(struct xdp_md *ctx, const bool is_ethernet, const bool downstream) { diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index 2eed968d51..8adcbd9ff9 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java @@ -72,6 +72,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -104,6 +105,7 @@ public class BpfCoordinator { private static final String TETHER_STATS_MAP_PATH = makeMapPath("stats"); private static final String TETHER_LIMIT_MAP_PATH = makeMapPath("limit"); private static final String TETHER_ERROR_MAP_PATH = makeMapPath("error"); + private static final String TETHER_DEV_MAP_PATH = makeMapPath("dev"); /** The names of all the BPF counters defined in bpf_tethering.h. */ public static final String[] sBpfCounterNames = getBpfCounterNames(); @@ -220,6 +222,11 @@ public class BpfCoordinator { // Map for upstream and downstream pair. private final HashMap> mForwardingPairs = new HashMap<>(); + // Set for upstream and downstream device map. Used for caching BPF dev map status and + // reduce duplicate adding or removing map operations. Use LinkedHashSet because the test + // BpfCoordinatorTest needs predictable iteration order. + private final Set mDeviceMapSet = new LinkedHashSet<>(); + // Runnable that used by scheduling next polling of stats. private final Runnable mScheduledPollingTask = () -> { updateForwardedStats(); @@ -336,6 +343,18 @@ public class BpfCoordinator { return null; } } + + /** Get dev BPF map. */ + @Nullable public BpfMap getBpfDevMap() { + if (!isAtLeastS()) return null; + try { + return new BpfMap<>(TETHER_DEV_MAP_PATH, + BpfMap.BPF_F_RDWR, TetherDevKey.class, TetherDevValue.class); + } catch (ErrnoException e) { + Log.e(TAG, "Cannot create dev map: " + e); + return null; + } + } } @VisibleForTesting @@ -490,6 +509,9 @@ public class BpfCoordinator { } LinkedHashMap rules = mIpv6ForwardingRules.get(ipServer); + // Add upstream and downstream interface index to dev map. + maybeAddDevMap(rule.upstreamIfindex, rule.downstreamIfindex); + // When the first rule is added to an upstream, setup upstream forwarding and data limit. maybeSetLimit(rule.upstreamIfindex); @@ -758,6 +780,11 @@ public class BpfCoordinator { dumpIpv4ForwardingRules(pw); pw.decreaseIndent(); + pw.println("Device map:"); + pw.increaseIndent(); + dumpDevmap(pw); + pw.decreaseIndent(); + pw.println(); pw.println("Forwarding counters:"); pw.increaseIndent(); @@ -890,6 +917,31 @@ public class BpfCoordinator { } } + private void dumpDevmap(@NonNull IndentingPrintWriter pw) { + try (BpfMap map = mDeps.getBpfDevMap()) { + if (map == null) { + pw.println("No devmap support"); + return; + } + if (map.isEmpty()) { + pw.println("No interface index"); + return; + } + pw.println("ifindex (iface) -> ifindex (iface)"); + pw.increaseIndent(); + map.forEach((k, v) -> { + // Only get upstream interface name. Just do the best to make the index readable. + // TODO: get downstream interface name because the index is either upstrema or + // downstream interface in dev map. + pw.println(String.format("%d (%s) -> %d (%s)", k.ifIndex, getIfName(k.ifIndex), + v.ifIndex, getIfName(v.ifIndex))); + }); + } catch (ErrnoException e) { + pw.println("Error dumping dev map: " + e); + } + pw.decreaseIndent(); + } + /** IPv6 forwarding rule class. */ public static class Ipv6ForwardingRule { // The upstream6 and downstream6 rules are built as the following tables. Only raw ip @@ -1229,6 +1281,7 @@ public class BpfCoordinator { final Tether4Value downstream4Value = makeTetherDownstream4Value(e, tetherClient, upstreamIndex); + maybeAddDevMap(upstreamIndex, tetherClient.downstreamIfindex); maybeSetLimit(upstreamIndex); mBpfCoordinatorShim.tetherOffloadRuleAdd(UPSTREAM, upstream4Key, upstream4Value); mBpfCoordinatorShim.tetherOffloadRuleAdd(DOWNSTREAM, downstream4Key, downstream4Value); @@ -1357,6 +1410,15 @@ public class BpfCoordinator { return false; } + // TODO: remove the index from map while the interface has been removed because the map size + // is 64 entries. See packages\modules\Connectivity\Tethering\bpf_progs\offload.c. + private void maybeAddDevMap(int upstreamIfindex, int downstreamIfindex) { + for (Integer index : new Integer[] {upstreamIfindex, downstreamIfindex}) { + if (mDeviceMapSet.contains(index)) continue; + if (mBpfCoordinatorShim.addDevMap(index)) mDeviceMapSet.add(index); + } + } + private void forwardingPairAdd(@NonNull String intIface, @NonNull String extIface) { if (!mForwardingPairs.containsKey(extIface)) { mForwardingPairs.put(extIface, new HashSet()); diff --git a/Tethering/src/com/android/networkstack/tethering/TetherDevKey.java b/Tethering/src/com/android/networkstack/tethering/TetherDevKey.java new file mode 100644 index 0000000000..4283c1b131 --- /dev/null +++ b/Tethering/src/com/android/networkstack/tethering/TetherDevKey.java @@ -0,0 +1,31 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.networkstack.tethering; + +import com.android.net.module.util.Struct; +import com.android.net.module.util.Struct.Field; +import com.android.net.module.util.Struct.Type; + +/** The key of BpfMap which is used for mapping interface index. */ +public class TetherDevKey extends Struct { + @Field(order = 0, type = Type.U32) + public final long ifIndex; // interface index + + public TetherDevKey(final long ifIndex) { + this.ifIndex = ifIndex; + } +} diff --git a/Tethering/src/com/android/networkstack/tethering/TetherDevValue.java b/Tethering/src/com/android/networkstack/tethering/TetherDevValue.java new file mode 100644 index 0000000000..1cd99b5d8e --- /dev/null +++ b/Tethering/src/com/android/networkstack/tethering/TetherDevValue.java @@ -0,0 +1,31 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.networkstack.tethering; + +import com.android.net.module.util.Struct; +import com.android.net.module.util.Struct.Field; +import com.android.net.module.util.Struct.Type; + +/** The key of BpfMap which is used for mapping interface index. */ +public class TetherDevValue extends Struct { + @Field(order = 0, type = Type.U32) + public final long ifIndex; // interface index + + public TetherDevValue(final long ifIndex) { + this.ifIndex = ifIndex; + } +} diff --git a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java index 435cab5140..ce69cb30ee 100644 --- a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java +++ b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java @@ -107,6 +107,8 @@ import com.android.networkstack.tethering.PrivateAddressCoordinator; import com.android.networkstack.tethering.Tether4Key; import com.android.networkstack.tethering.Tether4Value; import com.android.networkstack.tethering.Tether6Value; +import com.android.networkstack.tethering.TetherDevKey; +import com.android.networkstack.tethering.TetherDevValue; import com.android.networkstack.tethering.TetherDownstream6Key; import com.android.networkstack.tethering.TetherLimitKey; import com.android.networkstack.tethering.TetherLimitValue; @@ -182,6 +184,7 @@ public class IpServerTest { @Mock private BpfMap mBpfUpstream6Map; @Mock private BpfMap mBpfStatsMap; @Mock private BpfMap mBpfLimitMap; + @Mock private BpfMap mBpfDevMap; @Captor private ArgumentCaptor mDhcpParamsCaptor; @@ -334,6 +337,11 @@ public class IpServerTest { public BpfMap getBpfLimitMap() { return mBpfLimitMap; } + + @Nullable + public BpfMap getBpfDevMap() { + return mBpfDevMap; + } }; mBpfCoordinator = spy(new BpfCoordinator(mBpfDeps)); diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java index 233f6dbfa7..cc912f4dba 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java @@ -207,6 +207,7 @@ public class BpfCoordinatorTest { @Mock private BpfMap mBpfUpstream4Map; @Mock private BpfMap mBpfDownstream6Map; @Mock private BpfMap mBpfUpstream6Map; + @Mock private BpfMap mBpfDevMap; // Late init since methods must be called by the thread that created this object. private TestableNetworkStatsProviderCbBinder mTetherStatsProviderCb; @@ -284,6 +285,11 @@ public class BpfCoordinatorTest { public BpfMap getBpfLimitMap() { return mBpfLimitMap; } + + @Nullable + public BpfMap getBpfDevMap() { + return mBpfDevMap; + } }); @Before public void setUp() { @@ -1368,12 +1374,9 @@ public class BpfCoordinatorTest { coordinator.tetherOffloadClientAdd(mIpServer, clientInfo); } - // TODO: Test the IPv4 and IPv6 exist concurrently. - // TODO: Test the IPv4 rule delete failed. - @Test - @IgnoreUpTo(Build.VERSION_CODES.R) - public void testSetDataLimitOnRule4Change() throws Exception { - final BpfCoordinator coordinator = makeBpfCoordinator(); + private void initBpfCoordinatorForRule4(final BpfCoordinator coordinator) throws Exception { + // Needed because addUpstreamIfindexToMap only updates upstream information when polling + // was started. coordinator.startPolling(); // Needed because tetherOffloadRuleRemove of api31.BpfCoordinatorShimImpl only decreases @@ -1387,6 +1390,15 @@ public class BpfCoordinatorTest { coordinator.addUpstreamNameToLookupTable(UPSTREAM_IFINDEX, UPSTREAM_IFACE); setUpstreamInformationTo(coordinator); setDownstreamAndClientInformationTo(coordinator); + } + + // TODO: Test the IPv4 and IPv6 exist concurrently. + // TODO: Test the IPv4 rule delete failed. + @Test + @IgnoreUpTo(Build.VERSION_CODES.R) + public void testSetDataLimitOnRule4Change() throws Exception { + final BpfCoordinator coordinator = makeBpfCoordinator(); + initBpfCoordinatorForRule4(coordinator); // Applying a data limit to the current upstream does not take any immediate action. // The data limit could be only set on an upstream which has rules. @@ -1445,4 +1457,41 @@ public class BpfCoordinatorTest { verifyTetherOffloadGetAndClearStats(inOrder, UPSTREAM_IFINDEX); inOrder.verifyNoMoreInteractions(); } + + @Test + @IgnoreUpTo(Build.VERSION_CODES.R) + public void testAddDevMapRule6() throws Exception { + final BpfCoordinator coordinator = makeBpfCoordinator(); + + coordinator.addUpstreamNameToLookupTable(UPSTREAM_IFINDEX, UPSTREAM_IFACE); + final Ipv6ForwardingRule ruleA = buildTestForwardingRule(UPSTREAM_IFINDEX, NEIGH_A, MAC_A); + final Ipv6ForwardingRule ruleB = buildTestForwardingRule(UPSTREAM_IFINDEX, NEIGH_B, MAC_B); + + coordinator.tetherOffloadRuleAdd(mIpServer, ruleA); + verify(mBpfDevMap).updateEntry(eq(new TetherDevKey(UPSTREAM_IFINDEX)), + eq(new TetherDevValue(UPSTREAM_IFINDEX))); + verify(mBpfDevMap).updateEntry(eq(new TetherDevKey(DOWNSTREAM_IFINDEX)), + eq(new TetherDevValue(DOWNSTREAM_IFINDEX))); + clearInvocations(mBpfDevMap); + + coordinator.tetherOffloadRuleAdd(mIpServer, ruleB); + verify(mBpfDevMap, never()).updateEntry(any(), any()); + } + + @Test + @IgnoreUpTo(Build.VERSION_CODES.R) + public void testAddDevMapRule4() throws Exception { + final BpfCoordinator coordinator = makeBpfCoordinator(); + initBpfCoordinatorForRule4(coordinator); + + mConsumer.accept(makeTestConntrackEvent(IPCTNL_MSG_CT_NEW, IPPROTO_TCP)); + verify(mBpfDevMap).updateEntry(eq(new TetherDevKey(UPSTREAM_IFINDEX)), + eq(new TetherDevValue(UPSTREAM_IFINDEX))); + verify(mBpfDevMap).updateEntry(eq(new TetherDevKey(DOWNSTREAM_IFINDEX)), + eq(new TetherDevValue(DOWNSTREAM_IFINDEX))); + clearInvocations(mBpfDevMap); + + mConsumer.accept(makeTestConntrackEvent(IPCTNL_MSG_CT_NEW, IPPROTO_UDP)); + verify(mBpfDevMap, never()).updateEntry(any(), any()); + } }