From 67c14b549f47ec1314366009731e4713a8a8f663 Mon Sep 17 00:00:00 2001 From: Hungming Chen Date: Wed, 30 Dec 2020 20:15:20 +0800 Subject: [PATCH 1/2] [NFCT.TETHER.5] Migrate tetherOffloadSetInterfaceQuota from netd to mainline A preparation for updating BPF map in mainline module. Test: atest TetheringCoverageTests Change-Id: I67dfba750c7303e4aeaf65f5086db1290d176b4d --- .../apishim/api30/BpfCoordinatorShimImpl.java | 11 +++ .../apishim/api31/BpfCoordinatorShimImpl.java | 71 ++++++++++++++- .../apishim/common/BpfCoordinatorShim.java | 10 +++ .../tethering/BpfCoordinator.java | 28 +++--- .../tethering/TetherLimitKey.java | 53 +++++++++++ .../tethering/TetherLimitValue.java | 57 ++++++++++++ .../unit/src/android/net/ip/IpServerTest.java | 8 ++ .../tethering/BpfCoordinatorTest.java | 89 ++++++++++++++++--- 8 files changed, 303 insertions(+), 24 deletions(-) create mode 100644 Tethering/src/com/android/networkstack/tethering/TetherLimitKey.java create mode 100644 Tethering/src/com/android/networkstack/tethering/TetherLimitValue.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 dc5fd6da17..8ef6d6fc1f 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 @@ -92,6 +92,17 @@ public class BpfCoordinatorShimImpl return toTetherStatsValueSparseArray(tetherStatsList); } + @Override + public boolean tetherOffloadSetInterfaceQuota(int ifIndex, long quotaBytes) { + try { + mNetd.tetherOffloadSetInterfaceQuota(ifIndex, quotaBytes); + } catch (RemoteException | ServiceSpecificException e) { + mLog.e("Exception when updating quota " + quotaBytes + ": ", e); + return false; + } + return true; + } + @NonNull private SparseArray toTetherStatsValueSparseArray( @NonNull final TetherStatsParcel[] parcels) { 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 f6630d67f8..f94b5a946c 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 @@ -16,6 +16,8 @@ package com.android.networkstack.tethering.apishim.api31; +import static android.net.netstats.provider.NetworkStatsProvider.QUOTA_UNLIMITED; + import android.net.util.SharedLog; import android.system.ErrnoException; import android.system.OsConstants; @@ -29,6 +31,8 @@ import com.android.networkstack.tethering.BpfCoordinator.Ipv6ForwardingRule; import com.android.networkstack.tethering.BpfMap; import com.android.networkstack.tethering.TetherIngressKey; import com.android.networkstack.tethering.TetherIngressValue; +import com.android.networkstack.tethering.TetherLimitKey; +import com.android.networkstack.tethering.TetherLimitValue; import com.android.networkstack.tethering.TetherStatsKey; import com.android.networkstack.tethering.TetherStatsValue; @@ -51,15 +55,20 @@ public class BpfCoordinatorShimImpl @Nullable private final BpfMap mBpfStatsMap; + // BPF map of per-interface quota for tethering offload. + @Nullable + private final BpfMap mBpfLimitMap; + public BpfCoordinatorShimImpl(@NonNull final Dependencies deps) { mLog = deps.getSharedLog().forSubComponent(TAG); mBpfIngressMap = deps.getBpfIngressMap(); mBpfStatsMap = deps.getBpfStatsMap(); + mBpfLimitMap = deps.getBpfLimitMap(); } @Override public boolean isInitialized() { - return mBpfIngressMap != null && mBpfStatsMap != null; + return mBpfIngressMap != null && mBpfStatsMap != null && mBpfLimitMap != null; } @Override @@ -112,12 +121,70 @@ public class BpfCoordinatorShimImpl return tetherStatsList; } + @Override + public boolean tetherOffloadSetInterfaceQuota(int ifIndex, long quotaBytes) { + if (!isInitialized()) return false; + + // The common case is an update, where the stats already exist, + // hence we read first, even though writing with BPF_NOEXIST + // first would make the code simpler. + long rxBytes, txBytes; + TetherStatsValue statsValue = null; + + try { + statsValue = mBpfStatsMap.getValue(new TetherStatsKey(ifIndex)); + } catch (ErrnoException e) { + // The BpfMap#getValue doesn't throw an errno ENOENT exception. Catch other error + // while trying to get stats entry. + mLog.e("Could not get stats entry of interface index " + ifIndex + ": ", e); + return false; + } + + if (statsValue != null) { + // Ok, there was a stats entry. + rxBytes = statsValue.rxBytes; + txBytes = statsValue.txBytes; + } else { + // No stats entry - create one with zeroes. + try { + // This function is the *only* thing that can create entries. + // BpfMap#insertEntry use BPF_NOEXIST to create the entry. The entry is created + // if and only if it doesn't exist. + mBpfStatsMap.insertEntry(new TetherStatsKey(ifIndex), new TetherStatsValue( + 0 /* rxPackets */, 0 /* rxBytes */, 0 /* rxErrors */, 0 /* txPackets */, + 0 /* txBytes */, 0 /* txErrors */)); + } catch (ErrnoException | IllegalArgumentException e) { + mLog.e("Could not create stats entry: ", e); + return false; + } + rxBytes = 0; + txBytes = 0; + } + + // rxBytes + txBytes won't overflow even at 5gbps for ~936 years. + long newLimit = rxBytes + txBytes + quotaBytes; + + // if adding limit (e.g., if limit is QUOTA_UNLIMITED) caused overflow: clamp to 'infinity' + if (newLimit < rxBytes + txBytes) newLimit = QUOTA_UNLIMITED; + + try { + mBpfLimitMap.updateEntry(new TetherLimitKey(ifIndex), new TetherLimitValue(newLimit)); + } catch (ErrnoException e) { + mLog.e("Fail to set quota " + quotaBytes + " for interface index " + ifIndex + ": ", e); + return false; + } + + return true; + } + @Override public String toString() { return "mBpfIngressMap{" + (mBpfIngressMap != null ? "initialized" : "not initialized") + "}, " + "mBpfStatsMap{" - + (mBpfStatsMap != null ? "initialized" : "not initialized") + "} " + + (mBpfStatsMap != null ? "initialized" : "not initialized") + "}, " + + "mBpfLimitMap{" + + (mBpfLimitMap != null ? "initialized" : "not initialized") + "} " + "}"; } } 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 2ce3252da1..de65ea5799 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 @@ -79,5 +79,15 @@ public abstract class BpfCoordinatorShim { */ @Nullable public abstract SparseArray tetherOffloadGetStats(); + + /** + * Set a per-interface quota for tethering offload. + * + * @param ifIndex Index of upstream interface + * @param quotaBytes The quota defined as the number of bytes, starting from zero and counting + * from *now*. A value of QUOTA_UNLIMITED (-1) indicates there is no limit. + */ + @Nullable + public abstract boolean tetherOffloadSetInterfaceQuota(int ifIndex, long quotaBytes); } diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index 4f918ecea3..d5ab55061c 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java @@ -80,6 +80,8 @@ public class BpfCoordinator { "/sys/fs/bpf/map_offload_tether_ingress_map"; private static final String TETHER_STATS_MAP_PATH = "/sys/fs/bpf/map_offload_tether_stats_map"; + private static final String TETHER_LIMIT_MAP_PATH = + "/sys/fs/bpf/map_offload_tether_limit_map"; @VisibleForTesting enum StatsType { @@ -212,6 +214,17 @@ public class BpfCoordinator { return null; } } + + /** Get limit BPF map. */ + @Nullable public BpfMap getBpfLimitMap() { + try { + return new BpfMap<>(TETHER_LIMIT_MAP_PATH, + BpfMap.BPF_F_RDWR, TetherLimitKey.class, TetherLimitValue.class); + } catch (ErrnoException e) { + Log.e(TAG, "Cannot create limit map: " + e); + return null; + } + } } @VisibleForTesting @@ -673,20 +686,13 @@ public class BpfCoordinator { return quotaBytes; } - private boolean sendDataLimitToNetd(int ifIndex, long quotaBytes) { + private boolean sendDataLimitToBpfMap(int ifIndex, long quotaBytes) { if (ifIndex == 0) { Log.wtf(TAG, "Invalid interface index."); return false; } - try { - mNetd.tetherOffloadSetInterfaceQuota(ifIndex, quotaBytes); - } catch (RemoteException | ServiceSpecificException e) { - mLog.e("Exception when updating quota " + quotaBytes + ": ", e); - return false; - } - - return true; + return mBpfCoordinatorShim.tetherOffloadSetInterfaceQuota(ifIndex, quotaBytes); } // Handle the data limit update from the service which is the stats provider registered for. @@ -699,7 +705,7 @@ public class BpfCoordinator { if (ifIndex == 0) return; final long quotaBytes = getQuotaBytes(iface); - sendDataLimitToNetd(ifIndex, quotaBytes); + sendDataLimitToBpfMap(ifIndex, quotaBytes); } // Handle the data limit update while adding forwarding rules. @@ -710,7 +716,7 @@ public class BpfCoordinator { return false; } final long quotaBytes = getQuotaBytes(iface); - return sendDataLimitToNetd(ifIndex, quotaBytes); + return sendDataLimitToBpfMap(ifIndex, quotaBytes); } private boolean isAnyRuleOnUpstream(int upstreamIfindex) { diff --git a/Tethering/src/com/android/networkstack/tethering/TetherLimitKey.java b/Tethering/src/com/android/networkstack/tethering/TetherLimitKey.java new file mode 100644 index 0000000000..bc9bb474a2 --- /dev/null +++ b/Tethering/src/com/android/networkstack/tethering/TetherLimitKey.java @@ -0,0 +1,53 @@ +/* + * Copyright (C) 2020 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 tethering per-interface limit. */ +public class TetherLimitKey extends Struct { + @Field(order = 0, type = Type.U32) + public final long ifindex; // upstream interface index + + public TetherLimitKey(final long ifindex) { + this.ifindex = ifindex; + } + + // TODO: remove equals, hashCode and toString once aosp/1536721 is merged. + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + + if (!(obj instanceof TetherLimitKey)) return false; + + final TetherLimitKey that = (TetherLimitKey) obj; + + return ifindex == that.ifindex; + } + + @Override + public int hashCode() { + return Long.hashCode(ifindex); + } + + @Override + public String toString() { + return String.format("ifindex: %d", ifindex); + } +} diff --git a/Tethering/src/com/android/networkstack/tethering/TetherLimitValue.java b/Tethering/src/com/android/networkstack/tethering/TetherLimitValue.java new file mode 100644 index 0000000000..ed7e7d4324 --- /dev/null +++ b/Tethering/src/com/android/networkstack/tethering/TetherLimitValue.java @@ -0,0 +1,57 @@ +/* + * Copyright (C) 2020 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 value of BpfMap which is used for tethering per-interface limit. */ +public class TetherLimitValue extends Struct { + // Use the signed long variable to store the int64 limit on limit BPF map. + // S64 is enough for each interface limit even at 5Gbps for ~468 years. + // 2^63 / (5 * 1000 * 1000 * 1000) * 8 / 86400 / 365 = 468. + // Note that QUOTA_UNLIMITED (-1) indicates there is no limit. + @Field(order = 0, type = Type.S64) + public final long limit; + + public TetherLimitValue(final long limit) { + this.limit = limit; + } + + // TODO: remove equals, hashCode and toString once aosp/1536721 is merged. + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + + if (!(obj instanceof TetherLimitValue)) return false; + + final TetherLimitValue that = (TetherLimitValue) obj; + + return limit == that.limit; + } + + @Override + public int hashCode() { + return Long.hashCode(limit); + } + + @Override + public String toString() { + return String.format("limit: %d", limit); + } +} diff --git a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java index 91a518eebc..4763558236 100644 --- a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java +++ b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java @@ -106,6 +106,8 @@ import com.android.networkstack.tethering.BpfMap; import com.android.networkstack.tethering.PrivateAddressCoordinator; import com.android.networkstack.tethering.TetherIngressKey; import com.android.networkstack.tethering.TetherIngressValue; +import com.android.networkstack.tethering.TetherLimitKey; +import com.android.networkstack.tethering.TetherLimitValue; import com.android.networkstack.tethering.TetherStatsKey; import com.android.networkstack.tethering.TetherStatsValue; import com.android.networkstack.tethering.TetheringConfiguration; @@ -172,6 +174,7 @@ public class IpServerTest { @Mock private TetheringConfiguration mTetherConfig; @Mock private BpfMap mBpfIngressMap; @Mock private BpfMap mBpfStatsMap; + @Mock private BpfMap mBpfLimitMap; @Captor private ArgumentCaptor mDhcpParamsCaptor; @@ -301,6 +304,11 @@ public class IpServerTest { public BpfMap getBpfStatsMap() { return mBpfStatsMap; } + + @Nullable + public BpfMap getBpfLimitMap() { + return mBpfLimitMap; + } }; 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 5ca220c788..183571f266 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java @@ -126,6 +126,23 @@ public class BpfCoordinatorTest { public void updateEntry(K key, V value) throws ErrnoException { mMap.put(key, value); } + + @Override + public void insertEntry(K key, V value) throws ErrnoException, + IllegalArgumentException { + // The entry is created if and only if it doesn't exist. See BpfMap#insertEntry. + if (mMap.get(key) != null) { + throw new IllegalArgumentException(key + " already exist"); + } + mMap.put(key, value); + } + + @Override + public V getValue(@NonNull K key) throws ErrnoException { + // Return value for a given key. Otherwise, return null without an error ENOENT. + // BpfMap#getValue treats that the entry is not found as no error. + return mMap.get(key); + } }; @Mock private NetworkStatsManager mStatsManager; @@ -142,6 +159,8 @@ public class BpfCoordinatorTest { private final TestLooper mTestLooper = new TestLooper(); private final TestBpfMap mBpfStatsMap = spy(new TestBpfMap<>(TetherStatsKey.class, TetherStatsValue.class)); + private final TestBpfMap mBpfLimitMap = + spy(new TestBpfMap<>(TetherLimitKey.class, TetherLimitValue.class)); private BpfCoordinator.Dependencies mDeps = spy(new BpfCoordinator.Dependencies() { @NonNull @@ -178,6 +197,11 @@ public class BpfCoordinatorTest { public BpfMap getBpfStatsMap() { return mBpfStatsMap; } + + @Nullable + public BpfMap getBpfLimitMap() { + return mBpfLimitMap; + } }); @Before public void setUp() { @@ -325,6 +349,34 @@ public class BpfCoordinatorTest { } } + private void verifyTetherOffloadSetInterfaceQuota(@Nullable InOrder inOrder, int ifIndex, + long quotaBytes, boolean isInit) throws Exception { + if (mDeps.isAtLeastS()) { + final TetherStatsKey key = new TetherStatsKey(ifIndex); + verifyWithOrder(inOrder, mBpfStatsMap).getValue(key); + if (isInit) { + verifyWithOrder(inOrder, mBpfStatsMap).insertEntry(key, new TetherStatsValue( + 0L /* rxPackets */, 0L /* rxBytes */, 0L /* rxErrors */, + 0L /* txPackets */, 0L /* txBytes */, 0L /* txErrors */)); + } + verifyWithOrder(inOrder, mBpfLimitMap).updateEntry(new TetherLimitKey(ifIndex), + new TetherLimitValue(quotaBytes)); + } else { + verifyWithOrder(inOrder, mNetd).tetherOffloadSetInterfaceQuota(ifIndex, quotaBytes); + } + } + + private void verifyNeverTetherOffloadSetInterfaceQuota(@Nullable InOrder inOrder) + throws Exception { + if (mDeps.isAtLeastS()) { + inOrder.verify(mBpfStatsMap, never()).getValue(any()); + inOrder.verify(mBpfStatsMap, never()).insertEntry(any(), any()); + inOrder.verify(mBpfLimitMap, never()).updateEntry(any(), any()); + } else { + inOrder.verify(mNetd, never()).tetherOffloadSetInterfaceQuota(anyInt(), anyLong()); + } + } + // S+ and R api minimum tests. // The following tests are used to provide minimum checking for the APIs on different flow. // The auto merge is not enabled on mainline prod. The code flow R may be verified at the @@ -347,6 +399,8 @@ public class BpfCoordinatorTest { final Ipv6ForwardingRule rule = buildTestForwardingRule(mobileIfIndex, NEIGH_A, MAC_A); coordinator.tetherOffloadRuleAdd(mIpServer, rule); verifyTetherOffloadRuleAdd(null, rule); + verifyTetherOffloadSetInterfaceQuota(null, mobileIfIndex, QUOTA_UNLIMITED, + true /* isInit */); // Removing the last rule on current upstream immediately sends the cleanup stuff to netd. when(mNetd.tetherOffloadGetAndClearStats(mobileIfIndex)) @@ -596,10 +650,11 @@ public class BpfCoordinatorTest { // Set the unlimited quota as default if the service has never applied a data limit for a // given upstream. Note that the data limit only be applied on an upstream which has rules. final Ipv6ForwardingRule rule = buildTestForwardingRule(mobileIfIndex, NEIGH_A, MAC_A); - final InOrder inOrder = inOrder(mNetd, mBpfIngressMap); + final InOrder inOrder = inOrder(mNetd, mBpfIngressMap, mBpfLimitMap, mBpfStatsMap); coordinator.tetherOffloadRuleAdd(mIpServer, rule); verifyTetherOffloadRuleAdd(inOrder, rule); - inOrder.verify(mNetd).tetherOffloadSetInterfaceQuota(mobileIfIndex, QUOTA_UNLIMITED); + verifyTetherOffloadSetInterfaceQuota(inOrder, mobileIfIndex, QUOTA_UNLIMITED, + true /* isInit */); inOrder.verifyNoMoreInteractions(); // [2] Specific limit. @@ -607,7 +662,8 @@ public class BpfCoordinatorTest { for (final long quota : new long[] {0, 1048576000, Long.MAX_VALUE, QUOTA_UNLIMITED}) { mTetherStatsProvider.onSetLimit(mobileIface, quota); waitForIdle(); - inOrder.verify(mNetd).tetherOffloadSetInterfaceQuota(mobileIfIndex, quota); + verifyTetherOffloadSetInterfaceQuota(inOrder, mobileIfIndex, quota, + false /* isInit */); inOrder.verifyNoMoreInteractions(); } @@ -637,28 +693,28 @@ public class BpfCoordinatorTest { // 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. final long limit = 12345; - final InOrder inOrder = inOrder(mNetd, mBpfIngressMap); + final InOrder inOrder = inOrder(mNetd, mBpfIngressMap, mBpfLimitMap, mBpfStatsMap); mTetherStatsProvider.onSetLimit(mobileIface, limit); waitForIdle(); - inOrder.verify(mNetd, never()).tetherOffloadSetInterfaceQuota(anyInt(), anyLong()); + verifyNeverTetherOffloadSetInterfaceQuota(inOrder); // Adding the first rule on current upstream immediately sends the quota to netd. final Ipv6ForwardingRule ruleA = buildTestForwardingRule(mobileIfIndex, NEIGH_A, MAC_A); coordinator.tetherOffloadRuleAdd(mIpServer, ruleA); verifyTetherOffloadRuleAdd(inOrder, ruleA); - inOrder.verify(mNetd).tetherOffloadSetInterfaceQuota(mobileIfIndex, limit); + verifyTetherOffloadSetInterfaceQuota(inOrder, mobileIfIndex, limit, true /* isInit */); inOrder.verifyNoMoreInteractions(); // Adding the second rule on current upstream does not send the quota to netd. final Ipv6ForwardingRule ruleB = buildTestForwardingRule(mobileIfIndex, NEIGH_B, MAC_B); coordinator.tetherOffloadRuleAdd(mIpServer, ruleB); verifyTetherOffloadRuleAdd(inOrder, ruleB); - inOrder.verify(mNetd, never()).tetherOffloadSetInterfaceQuota(anyInt(), anyLong()); + verifyNeverTetherOffloadSetInterfaceQuota(inOrder); // Removing the second rule on current upstream does not send the quota to netd. coordinator.tetherOffloadRuleRemove(mIpServer, ruleB); verifyTetherOffloadRuleRemove(inOrder, ruleB); - inOrder.verify(mNetd, never()).tetherOffloadSetInterfaceQuota(anyInt(), anyLong()); + verifyNeverTetherOffloadSetInterfaceQuota(inOrder); // Removing the last rule on current upstream immediately sends the cleanup stuff to netd. when(mNetd.tetherOffloadGetAndClearStats(mobileIfIndex)) @@ -682,7 +738,7 @@ public class BpfCoordinatorTest { coordinator.addUpstreamNameToLookupTable(ethIfIndex, ethIface); coordinator.addUpstreamNameToLookupTable(mobileIfIndex, mobileIface); - final InOrder inOrder = inOrder(mNetd, mBpfIngressMap); + final InOrder inOrder = inOrder(mNetd, mBpfIngressMap, mBpfLimitMap, mBpfStatsMap); // Before the rule test, here are the additional actions while the rules are changed. // - After adding the first rule on a given upstream, the coordinator adds a data limit. @@ -700,7 +756,8 @@ public class BpfCoordinatorTest { coordinator.tetherOffloadRuleAdd(mIpServer, ethernetRuleA); verifyTetherOffloadRuleAdd(inOrder, ethernetRuleA); - inOrder.verify(mNetd).tetherOffloadSetInterfaceQuota(ethIfIndex, QUOTA_UNLIMITED); + verifyTetherOffloadSetInterfaceQuota(inOrder, ethIfIndex, QUOTA_UNLIMITED, + true /* isInit */); coordinator.tetherOffloadRuleAdd(mIpServer, ethernetRuleB); verifyTetherOffloadRuleAdd(inOrder, ethernetRuleB); @@ -718,7 +775,8 @@ public class BpfCoordinatorTest { coordinator.tetherOffloadRuleUpdate(mIpServer, mobileIfIndex); verifyTetherOffloadRuleRemove(inOrder, ethernetRuleA); verifyTetherOffloadRuleAdd(inOrder, mobileRuleA); - inOrder.verify(mNetd).tetherOffloadSetInterfaceQuota(mobileIfIndex, QUOTA_UNLIMITED); + verifyTetherOffloadSetInterfaceQuota(inOrder, mobileIfIndex, QUOTA_UNLIMITED, + true /* isInit */); verifyTetherOffloadRuleRemove(inOrder, ethernetRuleB); inOrder.verify(mNetd).tetherOffloadGetAndClearStats(ethIfIndex); verifyTetherOffloadRuleAdd(inOrder, mobileRuleB); @@ -825,6 +883,15 @@ public class BpfCoordinatorTest { checkBpfDisabled(); } + @Test + @IgnoreUpTo(Build.VERSION_CODES.R) + public void testBpfDisabledbyNoBpfLimitMap() throws Exception { + setupFunctioningNetdInterface(); + doReturn(null).when(mDeps).getBpfLimitMap(); + + checkBpfDisabled(); + } + @Test public void testTetheringConfigSetPollingInterval() throws Exception { setupFunctioningNetdInterface(); From 6971e91d6284fbe14682cf88cf296e140b6192a2 Mon Sep 17 00:00:00 2001 From: Hungming Chen Date: Thu, 31 Dec 2020 17:29:43 +0800 Subject: [PATCH 2/2] [NFCT.TETHER.6] Migrate tetherOffloadGetAndClearStats from netd to mainline A preparation for updating BPF map in mainline module. Test: atest TetheringCoverageTests Change-Id: Id87b88f6dfcdfe5765756442ed880933cd1c6baf --- .../apishim/api30/BpfCoordinatorShimImpl.java | 15 ++++ .../apishim/api31/BpfCoordinatorShimImpl.java | 87 +++++++++++++++++++ .../apishim/common/BpfCoordinatorShim.java | 19 ++++ .../tethering/BpfCoordinator.java | 30 +++---- .../tethering/BpfCoordinatorTest.java | 77 ++++++++++++---- 5 files changed, 192 insertions(+), 36 deletions(-) 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 8ef6d6fc1f..eafa3eae4c 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 @@ -116,6 +116,21 @@ public class BpfCoordinatorShimImpl return tetherStatsList; } + @Override + @Nullable + public TetherStatsValue tetherOffloadGetAndClearStats(int ifIndex) { + try { + final TetherStatsParcel stats = + mNetd.tetherOffloadGetAndClearStats(ifIndex); + return new TetherStatsValue(stats.rxPackets, stats.rxBytes, 0 /* rxErrors */, + stats.txPackets, stats.txBytes, 0 /* txErrors */); + } catch (RemoteException | ServiceSpecificException e) { + mLog.e("Exception when cleanup tether stats for upstream index " + + ifIndex + ": ", e); + return null; + } + } + @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 f94b5a946c..4ebf9144f2 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 @@ -20,6 +20,7 @@ import static android.net.netstats.provider.NetworkStatsProvider.QUOTA_UNLIMITED import android.net.util.SharedLog; import android.system.ErrnoException; +import android.system.Os; import android.system.OsConstants; import android.util.SparseArray; @@ -36,6 +37,8 @@ import com.android.networkstack.tethering.TetherLimitValue; import com.android.networkstack.tethering.TetherStatsKey; import com.android.networkstack.tethering.TetherStatsValue; +import java.io.FileDescriptor; + /** * Bpf coordinator class for API shims. */ @@ -43,6 +46,11 @@ public class BpfCoordinatorShimImpl extends com.android.networkstack.tethering.apishim.common.BpfCoordinatorShim { private static final String TAG = "api31.BpfCoordinatorShimImpl"; + // AF_KEY socket type. See include/linux/socket.h. + private static final int AF_KEY = 15; + // PFKEYv2 constants. See include/uapi/linux/pfkeyv2.h. + private static final int PF_KEY_V2 = 2; + @NonNull private final SharedLog mLog; @@ -177,6 +185,53 @@ public class BpfCoordinatorShimImpl return true; } + @Override + @Nullable + public TetherStatsValue tetherOffloadGetAndClearStats(int ifIndex) { + if (!isInitialized()) return null; + + // getAndClearTetherOffloadStats is called after all offload rules have already been + // deleted for the given upstream interface. Before starting to do cleanup stuff in this + // function, use synchronizeKernelRCU to make sure that all the current running eBPF + // programs are finished on all CPUs, especially the unfinished packet processing. After + // synchronizeKernelRCU returned, we can safely read or delete on the stats map or the + // limit map. + final int res = synchronizeKernelRCU(); + if (res != 0) { + // Error log but don't return. Do as much cleanup as possible. + mLog.e("synchronize_rcu() failed: " + res); + } + + TetherStatsValue statsValue = null; + try { + statsValue = mBpfStatsMap.getValue(new TetherStatsKey(ifIndex)); + } catch (ErrnoException e) { + mLog.e("Could not get stats entry for interface index " + ifIndex + ": ", e); + return null; + } + + if (statsValue == null) { + mLog.e("Could not get stats entry for interface index " + ifIndex); + return null; + } + + try { + mBpfStatsMap.deleteEntry(new TetherStatsKey(ifIndex)); + } catch (ErrnoException e) { + mLog.e("Could not delete stats entry for interface index " + ifIndex + ": ", e); + return null; + } + + try { + mBpfLimitMap.deleteEntry(new TetherLimitKey(ifIndex)); + } catch (ErrnoException e) { + mLog.e("Could not delete limit for interface index " + ifIndex + ": ", e); + return null; + } + + return statsValue; + } + @Override public String toString() { return "mBpfIngressMap{" @@ -187,4 +242,36 @@ public class BpfCoordinatorShimImpl + (mBpfLimitMap != null ? "initialized" : "not initialized") + "} " + "}"; } + + /** + * Call synchronize_rcu() to block until all existing RCU read-side critical sections have + * been completed. + * Note that BpfCoordinatorTest have no permissions to create or close pf_key socket. It is + * okay for now because the caller #bpfGetAndClearStats doesn't care the result of this + * function. The tests don't be broken. + * TODO: Wrap this function into Dependencies for mocking in tests. + */ + private int synchronizeKernelRCU() { + // This is a temporary hack for network stats map swap on devices running + // 4.9 kernels. The kernel code of socket release on pf_key socket will + // explicitly call synchronize_rcu() which is exactly what we need. + FileDescriptor pfSocket; + try { + pfSocket = Os.socket(AF_KEY, OsConstants.SOCK_RAW | OsConstants.SOCK_CLOEXEC, + PF_KEY_V2); + } catch (ErrnoException e) { + mLog.e("create PF_KEY socket failed: ", e); + return e.errno; + } + + // When closing socket, synchronize_rcu() gets called in sock_release(). + try { + Os.close(pfSocket); + } catch (ErrnoException e) { + mLog.e("failed to close the PF_KEY socket: ", e); + return e.errno; + } + + return 0; + } } 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 de65ea5799..61abfa3aaa 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 @@ -89,5 +89,24 @@ public abstract class BpfCoordinatorShim { */ @Nullable public abstract boolean tetherOffloadSetInterfaceQuota(int ifIndex, long quotaBytes); + + /** + * Return BPF tethering offload statistics and clear the stats for a given upstream. + * + * Must only be called once all offload rules have already been deleted for the given upstream + * interface. The existing stats will be fetched and returned. The stats and the limit for the + * given upstream interface will be deleted as well. + * + * The stats and limit for a given upstream interface must be initialized (using + * tetherOffloadSetInterfaceQuota) before any offload will occur on that interface. + * + * Note that this can be only called while the BPF maps were initialized. + * + * @param ifIndex Index of upstream interface. + * @return TetherStatsValue, which contains the given upstream interface's tethering statistics + * since tethering was first started on that upstream interface. + */ + @Nullable + public abstract TetherStatsValue tetherOffloadGetAndClearStats(int ifIndex); } diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index d5ab55061c..64ac37ce03 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java @@ -34,15 +34,12 @@ import android.net.MacAddress; import android.net.NetworkStats; import android.net.NetworkStats.Entry; import android.net.TetherOffloadRuleParcel; -import android.net.TetherStatsParcel; import android.net.ip.IpServer; import android.net.netstats.provider.NetworkStatsProvider; import android.net.util.SharedLog; import android.net.util.TetheringUtils.ForwardedStats; import android.os.ConditionVariable; import android.os.Handler; -import android.os.RemoteException; -import android.os.ServiceSpecificException; import android.system.ErrnoException; import android.text.TextUtils; import android.util.Log; @@ -361,22 +358,19 @@ public class BpfCoordinator { // Do cleanup functionality if there is no more rule on the given upstream. final int upstreamIfindex = rule.upstreamIfindex; if (!isAnyRuleOnUpstream(upstreamIfindex)) { - try { - final TetherStatsParcel stats = - mNetd.tetherOffloadGetAndClearStats(upstreamIfindex); - SparseArray tetherStatsList = - new SparseArray(); - tetherStatsList.put(stats.ifIndex, new TetherStatsValue(stats.rxPackets, - stats.rxBytes, 0 /* rxErrors */, stats.txPackets, stats.txBytes, - 0 /* txErrors */)); - - // Update the last stats delta and delete the local cache for a given upstream. - updateQuotaAndStatsFromSnapshot(tetherStatsList); - mStats.remove(upstreamIfindex); - } catch (RemoteException | ServiceSpecificException e) { - Log.wtf(TAG, "Exception when cleanup tether stats for upstream index " - + upstreamIfindex + ": ", e); + final TetherStatsValue statsValue = + mBpfCoordinatorShim.tetherOffloadGetAndClearStats(upstreamIfindex); + if (statsValue == null) { + Log.wtf(TAG, "Fail to cleanup tether stats for upstream index " + upstreamIfindex); + return; } + + SparseArray tetherStatsList = new SparseArray(); + tetherStatsList.put(upstreamIfindex, statsValue); + + // Update the last stats delta and delete the local cache for a given upstream. + updateQuotaAndStatsFromSnapshot(tetherStatsList); + mStats.remove(upstreamIfindex); } } 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 183571f266..4abaf03f25 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java @@ -137,6 +137,11 @@ public class BpfCoordinatorTest { mMap.put(key, value); } + @Override + public boolean deleteEntry(Struct key) throws ErrnoException { + return mMap.remove(key) != null; + } + @Override public V getValue(@NonNull K key) throws ErrnoException { // Return value for a given key. Otherwise, return null without an error ENOENT. @@ -253,12 +258,16 @@ public class BpfCoordinatorTest { } // Update a stats entry or create if not exists. + private void updateStatsEntryToStatsMap(@NonNull TetherStatsParcel stats) throws Exception { + final TetherStatsKey key = new TetherStatsKey(stats.ifIndex); + final TetherStatsValue value = new TetherStatsValue(stats.rxPackets, stats.rxBytes, + 0L /* rxErrors */, stats.txPackets, stats.txBytes, 0L /* txErrors */); + mBpfStatsMap.updateEntry(key, value); + } + private void updateStatsEntry(@NonNull TetherStatsParcel stats) throws Exception { if (mDeps.isAtLeastS()) { - final TetherStatsKey key = new TetherStatsKey(stats.ifIndex); - final TetherStatsValue value = new TetherStatsValue(stats.rxPackets, stats.rxBytes, - 0L /* rxErrors */, stats.txPackets, stats.txBytes, 0L /* txErrors */); - mBpfStatsMap.updateEntry(key, value); + updateStatsEntryToStatsMap(stats); } else { when(mNetd.tetherOffloadGetStats()).thenReturn(new TetherStatsParcel[] {stats}); } @@ -282,6 +291,21 @@ public class BpfCoordinatorTest { waitForIdle(); } + // In tests, the stats need to be set before deleting the last rule. + // The reason is that BpfCoordinator#tetherOffloadRuleRemove reads the stats + // of the deleting interface after the last rule deleted. #tetherOffloadRuleRemove + // does the interface cleanup failed if there is no stats for the deleting interface. + // Note that the mocked tetherOffloadGetAndClearStats of netd replaces all stats entries + // because it doesn't store the previous entries. + private void updateStatsEntryForTetherOffloadGetAndClearStats(TetherStatsParcel stats) + throws Exception { + if (mDeps.isAtLeastS()) { + updateStatsEntryToStatsMap(stats); + } else { + when(mNetd.tetherOffloadGetAndClearStats(stats.ifIndex)).thenReturn(stats); + } + } + private void clearStatsInvocations() { if (mDeps.isAtLeastS()) { clearInvocations(mBpfStatsMap); @@ -377,6 +401,17 @@ public class BpfCoordinatorTest { } } + private void verifyTetherOffloadGetAndClearStats(@Nullable InOrder inOrder, int ifIndex) + throws Exception { + if (mDeps.isAtLeastS()) { + inOrder.verify(mBpfStatsMap).getValue(new TetherStatsKey(ifIndex)); + inOrder.verify(mBpfStatsMap).deleteEntry(new TetherStatsKey(ifIndex)); + inOrder.verify(mBpfLimitMap).deleteEntry(new TetherLimitKey(ifIndex)); + } else { + inOrder.verify(mNetd).tetherOffloadGetAndClearStats(ifIndex); + } + } + // S+ and R api minimum tests. // The following tests are used to provide minimum checking for the APIs on different flow. // The auto merge is not enabled on mainline prod. The code flow R may be verified at the @@ -396,17 +431,23 @@ public class BpfCoordinatorTest { final Integer mobileIfIndex = 100; coordinator.addUpstreamNameToLookupTable(mobileIfIndex, mobileIface); + // InOrder is required because mBpfStatsMap may be accessed by both + // BpfCoordinator#tetherOffloadRuleAdd and BpfCoordinator#tetherOffloadGetAndClearStats. + // The #verifyTetherOffloadGetAndClearStats can't distinguish who has ever called + // mBpfStatsMap#getValue and get a wrong calling count which counts all. + final InOrder inOrder = inOrder(mNetd, mBpfIngressMap, mBpfLimitMap, mBpfStatsMap); final Ipv6ForwardingRule rule = buildTestForwardingRule(mobileIfIndex, NEIGH_A, MAC_A); coordinator.tetherOffloadRuleAdd(mIpServer, rule); - verifyTetherOffloadRuleAdd(null, rule); - verifyTetherOffloadSetInterfaceQuota(null, mobileIfIndex, QUOTA_UNLIMITED, + verifyTetherOffloadRuleAdd(inOrder, rule); + verifyTetherOffloadSetInterfaceQuota(inOrder, mobileIfIndex, QUOTA_UNLIMITED, true /* isInit */); // Removing the last rule on current upstream immediately sends the cleanup stuff to netd. - when(mNetd.tetherOffloadGetAndClearStats(mobileIfIndex)) - .thenReturn(buildTestTetherStatsParcel(mobileIfIndex, 0, 0, 0, 0)); + updateStatsEntryForTetherOffloadGetAndClearStats( + buildTestTetherStatsParcel(mobileIfIndex, 0, 0, 0, 0)); coordinator.tetherOffloadRuleRemove(mIpServer, rule); - verifyTetherOffloadRuleRemove(null, rule); + verifyTetherOffloadRuleRemove(inOrder, rule); + verifyTetherOffloadGetAndClearStats(inOrder, mobileIfIndex); } // TODO: remove once presubmit tests on R even the code is submitted on S. @@ -717,11 +758,11 @@ public class BpfCoordinatorTest { verifyNeverTetherOffloadSetInterfaceQuota(inOrder); // Removing the last rule on current upstream immediately sends the cleanup stuff to netd. - when(mNetd.tetherOffloadGetAndClearStats(mobileIfIndex)) - .thenReturn(buildTestTetherStatsParcel(mobileIfIndex, 0, 0, 0, 0)); + updateStatsEntryForTetherOffloadGetAndClearStats( + buildTestTetherStatsParcel(mobileIfIndex, 0, 0, 0, 0)); coordinator.tetherOffloadRuleRemove(mIpServer, ruleA); verifyTetherOffloadRuleRemove(inOrder, ruleA); - inOrder.verify(mNetd).tetherOffloadGetAndClearStats(mobileIfIndex); + verifyTetherOffloadGetAndClearStats(inOrder, mobileIfIndex); inOrder.verifyNoMoreInteractions(); } @@ -767,8 +808,8 @@ public class BpfCoordinatorTest { mobileIfIndex, NEIGH_A, MAC_A); final Ipv6ForwardingRule mobileRuleB = buildTestForwardingRule( mobileIfIndex, NEIGH_B, MAC_B); - when(mNetd.tetherOffloadGetAndClearStats(ethIfIndex)) - .thenReturn(buildTestTetherStatsParcel(ethIfIndex, 10, 20, 30, 40)); + updateStatsEntryForTetherOffloadGetAndClearStats( + buildTestTetherStatsParcel(ethIfIndex, 10, 20, 30, 40)); // Update the existing rules for upstream changes. The rules are removed and re-added one // by one for updating upstream interface index by #tetherOffloadRuleUpdate. @@ -778,16 +819,16 @@ public class BpfCoordinatorTest { verifyTetherOffloadSetInterfaceQuota(inOrder, mobileIfIndex, QUOTA_UNLIMITED, true /* isInit */); verifyTetherOffloadRuleRemove(inOrder, ethernetRuleB); - inOrder.verify(mNetd).tetherOffloadGetAndClearStats(ethIfIndex); + verifyTetherOffloadGetAndClearStats(inOrder, ethIfIndex); verifyTetherOffloadRuleAdd(inOrder, mobileRuleB); // [3] Clear all rules for a given IpServer. - when(mNetd.tetherOffloadGetAndClearStats(mobileIfIndex)) - .thenReturn(buildTestTetherStatsParcel(mobileIfIndex, 50, 60, 70, 80)); + updateStatsEntryForTetherOffloadGetAndClearStats( + buildTestTetherStatsParcel(mobileIfIndex, 50, 60, 70, 80)); coordinator.tetherOffloadRuleClear(mIpServer); verifyTetherOffloadRuleRemove(inOrder, mobileRuleA); verifyTetherOffloadRuleRemove(inOrder, mobileRuleB); - inOrder.verify(mNetd).tetherOffloadGetAndClearStats(mobileIfIndex); + verifyTetherOffloadGetAndClearStats(inOrder, mobileIfIndex); // [4] Force pushing stats update to verify that the last diff of stats is reported on all // upstreams.