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 a280046c52..4d1e7ef482 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 @@ -167,7 +167,6 @@ public class BpfCoordinatorShimImpl @Override public boolean addIpv6UpstreamRule(@NonNull final Ipv6UpstreamRule rule) { - if (!isInitialized()) return false; // RFC7421_PREFIX_LENGTH = 64 which is the most commonly used IPv6 subnet prefix length. if (rule.sourcePrefix.getPrefixLength() != RFC7421_PREFIX_LENGTH) return false; @@ -185,7 +184,6 @@ public class BpfCoordinatorShimImpl @Override public boolean removeIpv6UpstreamRule(@NonNull final Ipv6UpstreamRule rule) { - if (!isInitialized()) return false; // RFC7421_PREFIX_LENGTH = 64 which is the most commonly used IPv6 subnet prefix length. if (rule.sourcePrefix.getPrefixLength() != RFC7421_PREFIX_LENGTH) return false; @@ -200,8 +198,6 @@ public class BpfCoordinatorShimImpl @Override public boolean addIpv6DownstreamRule(@NonNull final Ipv6DownstreamRule rule) { - if (!isInitialized()) return false; - final TetherDownstream6Key key = rule.makeTetherDownstream6Key(); final Tether6Value value = rule.makeTether6Value(); @@ -217,8 +213,6 @@ public class BpfCoordinatorShimImpl @Override public boolean removeIpv6DownstreamRule(@NonNull final Ipv6DownstreamRule rule) { - if (!isInitialized()) return false; - try { mBpfDownstream6Map.deleteEntry(rule.makeTetherDownstream6Key()); } catch (ErrnoException e) { @@ -234,8 +228,6 @@ public class BpfCoordinatorShimImpl @Override @Nullable public SparseArray tetherOffloadGetStats() { - if (!isInitialized()) return null; - final SparseArray tetherStatsList = new SparseArray(); try { // The reported tether stats are total data usage for all currently-active upstream @@ -250,8 +242,6 @@ public class BpfCoordinatorShimImpl @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. @@ -307,8 +297,6 @@ public class BpfCoordinatorShimImpl @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 @@ -354,8 +342,6 @@ public class BpfCoordinatorShimImpl @Override public boolean tetherOffloadRuleAdd(boolean downstream, @NonNull Tether4Key key, @NonNull Tether4Value value) { - if (!isInitialized()) return false; - try { if (downstream) { mBpfDownstream4Map.insertEntry(key, value); @@ -379,8 +365,6 @@ public class BpfCoordinatorShimImpl @Override public boolean tetherOffloadRuleRemove(boolean downstream, @NonNull Tether4Key key) { - if (!isInitialized()) return false; - try { if (downstream) { if (!mBpfDownstream4Map.deleteEntry(key)) return false; // Rule did not exist @@ -413,8 +397,6 @@ public class BpfCoordinatorShimImpl @Override public void tetherOffloadRuleForEach(boolean downstream, @NonNull ThrowingBiConsumer action) { - if (!isInitialized()) return; - try { if (downstream) { mBpfDownstream4Map.forEach(action); @@ -428,8 +410,6 @@ public class BpfCoordinatorShimImpl @Override public boolean attachProgram(String iface, boolean downstream, boolean ipv4) { - if (!isInitialized()) return false; - try { BpfUtils.attachProgram(iface, downstream, ipv4); } catch (IOException e) { @@ -441,8 +421,6 @@ public class BpfCoordinatorShimImpl @Override public boolean detachProgram(String iface, boolean ipv4) { - if (!isInitialized()) return false; - try { BpfUtils.detachProgram(iface, ipv4); } catch (IOException e) { @@ -460,8 +438,6 @@ public class BpfCoordinatorShimImpl @Override public boolean addDevMap(int ifIndex) { - if (!isInitialized()) return false; - try { mBpfDevMap.updateEntry(new TetherDevKey(ifIndex), new TetherDevValue(ifIndex)); } catch (ErrnoException e) { @@ -473,8 +449,6 @@ public class BpfCoordinatorShimImpl @Override public boolean removeDevMap(int ifIndex) { - if (!isInitialized()) return false; - try { mBpfDevMap.deleteEntry(new TetherDevKey(ifIndex)); } catch (ErrnoException e) { diff --git a/Tethering/src/android/net/ip/IpServer.java b/Tethering/src/android/net/ip/IpServer.java index a85141060c..e030902637 100644 --- a/Tethering/src/android/net/ip/IpServer.java +++ b/Tethering/src/android/net/ip/IpServer.java @@ -127,6 +127,7 @@ public class IpServer extends StateMachine { // TODO: have this configurable private static final int DHCP_LEASE_TIME_SECS = 3600; + private static final int NO_UPSTREAM = 0; private static final MacAddress NULL_MAC_ADDRESS = MacAddress.fromString("00:00:00:00:00:00"); private static final String TAG = "IpServer"; @@ -259,6 +260,12 @@ public class IpServer extends StateMachine { private int mLastError; private int mServingMode; private InterfaceSet mUpstreamIfaceSet; // may change over time + // mInterfaceParams can't be final now because IpServer will be created when receives + // WIFI_AP_STATE_CHANGED broadcasts or when it detects that the wifi interface has come up. + // In the latter case, the interface is not fully initialized and the MAC address might not + // be correct (it will be set with a randomized MAC address later). + // TODO: Consider create the IpServer only when tethering want to enable it, then we can + // make mInterfaceParams final. private InterfaceParams mInterfaceParams; // TODO: De-duplicate this with mLinkProperties above. Currently, these link // properties are those selected by the IPv6TetheringCoordinator and relayed @@ -740,7 +747,7 @@ public class IpServer extends StateMachine { RaParams params = null; String upstreamIface = null; InterfaceParams upstreamIfaceParams = null; - int upstreamIfIndex = 0; + int upstreamIfIndex = NO_UPSTREAM; if (v6only != null) { upstreamIface = v6only.getInterfaceName(); @@ -772,7 +779,7 @@ public class IpServer extends StateMachine { // CMD_TETHER_CONNECTION_CHANGED. Adding the mapping update here to the avoid potential // timing issue. It prevents that the IPv6 capability is updated later than // CMD_TETHER_CONNECTION_CHANGED. - mBpfCoordinator.addUpstreamNameToLookupTable(upstreamIfIndex, upstreamIface); + mBpfCoordinator.maybeAddUpstreamToLookupTable(upstreamIfIndex, upstreamIface); // If v6only is null, we pass in null to setRaParams(), which handles // deprecation of any existing RA data. @@ -780,8 +787,7 @@ public class IpServer extends StateMachine { // Not support BPF on virtual upstream interface final boolean upstreamSupportsBpf = upstreamIface != null && !isVcnInterface(upstreamIface); - updateIpv6ForwardingRules( - mLastIPv6UpstreamIfindex, upstreamIfIndex, upstreamSupportsBpf, null); + updateIpv6ForwardingRules(mLastIPv6UpstreamIfindex, upstreamIfIndex, upstreamSupportsBpf); mLastIPv6LinkProperties = v6only; mLastIPv6UpstreamIfindex = upstreamIfIndex; mUpstreamSupportsBpf = upstreamSupportsBpf; @@ -887,25 +893,23 @@ public class IpServer extends StateMachine { } } - // Handles all updates to IPv6 forwarding rules. These can currently change only if the upstream - // changes or if a neighbor event is received. + private int getInterfaceIndexForRule(int ifindex, boolean supportsBpf) { + return supportsBpf ? ifindex : NO_UPSTREAM; + } + + // Handles updates to IPv6 forwarding rules if the upstream changes. private void updateIpv6ForwardingRules(int prevUpstreamIfindex, int upstreamIfindex, - boolean upstreamSupportsBpf, NeighborEvent e) { - // If no longer have an upstream or upstream not supports BPF, clear forwarding rules and do - // nothing else. - // TODO: Rather than always clear rules, ensure whether ipv6 ever enable first. - if (upstreamIfindex == 0 || !upstreamSupportsBpf) { - mBpfCoordinator.tetherOffloadRuleClear(this); - return; - } - + boolean upstreamSupportsBpf) { // If the upstream interface has changed, remove all rules and re-add them with the new - // upstream interface. + // upstream interface. If upstream is a virtual network, treated as no upstream. if (prevUpstreamIfindex != upstreamIfindex) { - mBpfCoordinator.tetherOffloadRuleUpdate(this, upstreamIfindex); + mBpfCoordinator.updateAllIpv6Rules(this, this.mInterfaceParams, + getInterfaceIndexForRule(upstreamIfindex, upstreamSupportsBpf)); } + } - // If we're here to process a NeighborEvent, do so now. + // Handles updates to IPv6 downstream rules if a neighbor event is received. + private void addOrRemoveIpv6Downstream(NeighborEvent e) { // mInterfaceParams must be non-null or the event would not have arrived. if (e == null) return; if (!(e.ip instanceof Inet6Address) || e.ip.isMulticastAddress() @@ -917,8 +921,9 @@ public class IpServer extends StateMachine { // Do this here instead of in the Ipv6DownstreamRule constructor to ensure that we // never add rules with a null MAC, only delete them. MacAddress dstMac = e.isValid() ? e.macAddr : NULL_MAC_ADDRESS; - Ipv6DownstreamRule rule = new Ipv6DownstreamRule(upstreamIfindex, mInterfaceParams.index, - (Inet6Address) e.ip, mInterfaceParams.macAddr, dstMac); + Ipv6DownstreamRule rule = new Ipv6DownstreamRule( + getInterfaceIndexForRule(mLastIPv6UpstreamIfindex, mUpstreamSupportsBpf), + mInterfaceParams.index, (Inet6Address) e.ip, mInterfaceParams.macAddr, dstMac); if (e.isValid()) { mBpfCoordinator.addIpv6DownstreamRule(this, rule); } else { @@ -951,8 +956,7 @@ public class IpServer extends StateMachine { if (mInterfaceParams != null && mInterfaceParams.index == e.ifindex && mInterfaceParams.hasMacAddress) { - updateIpv6ForwardingRules(mLastIPv6UpstreamIfindex, mLastIPv6UpstreamIfindex, - mUpstreamSupportsBpf, e); + addOrRemoveIpv6Downstream(e); updateClientInfoIpv4(e); } } @@ -1285,6 +1289,7 @@ public class IpServer extends StateMachine { @Override public void exit() { cleanupUpstream(); + mBpfCoordinator.clearAllIpv6Rules(IpServer.this); super.exit(); } @@ -1303,7 +1308,8 @@ public class IpServer extends StateMachine { for (String ifname : mUpstreamIfaceSet.ifnames) cleanupUpstreamInterface(ifname); mUpstreamIfaceSet = null; - mBpfCoordinator.tetherOffloadRuleClear(IpServer.this); + mBpfCoordinator.updateAllIpv6Rules( + IpServer.this, IpServer.this.mInterfaceParams, NO_UPSTREAM); } private void cleanupUpstreamInterface(String upstreamIface) { @@ -1370,7 +1376,7 @@ public class IpServer extends StateMachine { final InterfaceParams upstreamIfaceParams = mDeps.getInterfaceParams(ifname); if (upstreamIfaceParams != null) { - mBpfCoordinator.addUpstreamNameToLookupTable( + mBpfCoordinator.maybeAddUpstreamToLookupTable( upstreamIfaceParams.index, ifname); } } @@ -1430,6 +1436,8 @@ public class IpServer extends StateMachine { class UnavailableState extends State { @Override public void enter() { + // TODO: move mIpNeighborMonitor.stop() to TetheredState#exit, and trigger a neighbours + // dump after starting mIpNeighborMonitor. mIpNeighborMonitor.stop(); mLastError = TETHER_ERROR_NO_ERROR; sendInterfaceState(STATE_UNAVAILABLE); diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index 1b23a6c7f0..46c815f2f8 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java @@ -50,6 +50,7 @@ import android.os.SystemClock; import android.system.ErrnoException; import android.system.OsConstants; import android.text.TextUtils; +import android.util.ArrayMap; import android.util.ArraySet; import android.util.Log; import android.util.SparseArray; @@ -153,6 +154,7 @@ public class BpfCoordinator { static final int NF_CONNTRACK_UDP_TIMEOUT_STREAM = 180; @VisibleForTesting static final int INVALID_MTU = 0; + static final int NO_UPSTREAM = 0; // List of TCP port numbers which aren't offloaded because the packets require the netfilter // conntrack helper. See also TetherController::setForwardRules in netd. @@ -221,6 +223,23 @@ public class BpfCoordinator { // TODO: Remove the unused interface name. private final SparseArray mInterfaceNames = new SparseArray<>(); + // How IPv6 upstream rules and downstream rules are managed in BpfCoordinator: + // 1. Each upstream rule represents a downstream interface to an upstream interface forwarding. + // No upstream rule will be exist if there is no upstream interface. + // Note that there is at most one upstream interface for a given downstream interface. + // 2. Each downstream rule represents an IPv6 neighbor, regardless of the existence of the + // upstream interface. If the upstream is not present, the downstream rules have an upstream + // interface index of NO_UPSTREAM, only exist in BpfCoordinator and won't be written to the + // BPF map. When the upstream comes back, those downstream rules will be updated by calling + // Ipv6DownstreamRule#onNewUpstream and written to the BPF map again. We don't remove the + // downstream rules when upstream is lost is because the upstream may come back with the + // same prefix and we won't receive any neighbor update event in this case. + // TODO: Remove downstream rules when upstream is lost and dump neighbors table when upstream + // interface comes back in order to reconstruct the downstream rules. + // 3. It is the same thing for BpfCoordinator if there is no upstream interface or the upstream + // interface is a virtual interface (which currently not supports BPF). In this case, + // IpServer will update its upstream ifindex to NO_UPSTREAM to the BpfCoordinator. + // Map of downstream rule maps. Each of these maps represents the IPv6 forwarding rules for a // given downstream. Each map: // - Is owned by the IpServer that is responsible for that downstream. @@ -240,6 +259,16 @@ public class BpfCoordinator { private final HashMap> mIpv6DownstreamRules = new LinkedHashMap<>(); + // Map of IPv6 upstream rules maps. Each of these maps represents the IPv6 upstream rules for a + // given downstream. Each map: + // - Is owned by the IpServer that is responsible for that downstream. + // - Must only be modified by that IpServer. + // - Is created when the IpServer adds its first upstream rule, and deleted when the IpServer + // deletes its last upstream rule (or clears its upstream rules) + // - Each upstream rule in the ArraySet is corresponding to an upstream interface. + private final ArrayMap> + mIpv6UpstreamRules = new ArrayMap<>(); + // Map of downstream client maps. Each of these maps represents the IPv4 clients for a given // downstream. Needed to build IPv4 forwarding rules when conntrack events are received. // Each map: @@ -603,130 +632,173 @@ public class BpfCoordinator { } /** - * Add IPv6 downstream rule. After adding the first rule on a given upstream, must add the data - * limit on the given upstream. - * Note that this can be only called on handler thread. + * Add IPv6 upstream rule. After adding the first rule on a given upstream, must add the + * data limit on the given upstream. */ - public void addIpv6DownstreamRule( - @NonNull final IpServer ipServer, @NonNull final Ipv6DownstreamRule rule) { + private void addIpv6UpstreamRule( + @NonNull final IpServer ipServer, @NonNull final Ipv6UpstreamRule rule) { if (!isUsingBpf()) return; - // TODO: Perhaps avoid to add a duplicate rule. - if (!mBpfCoordinatorShim.addIpv6DownstreamRule(rule)) return; - - if (!mIpv6DownstreamRules.containsKey(ipServer)) { - mIpv6DownstreamRules.put(ipServer, new LinkedHashMap()); - } - LinkedHashMap rules = mIpv6DownstreamRules.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); - if (!isAnyRuleFromDownstreamToUpstream(rule.downstreamIfindex, rule.upstreamIfindex)) { - // TODO: support upstream forwarding on non-point-to-point interfaces. - // TODO: get the MTU from LinkProperties and update the rules when it changes. - Ipv6UpstreamRule upstreamRule = new Ipv6UpstreamRule(rule.upstreamIfindex, - rule.downstreamIfindex, IPV6_ZERO_PREFIX64, rule.srcMac, NULL_MAC_ADDRESS, - NULL_MAC_ADDRESS); - if (!mBpfCoordinatorShim.addIpv6UpstreamRule(upstreamRule)) { - mLog.e("Failed to add upstream IPv6 forwarding rule: " + upstreamRule); - } + // TODO: support upstream forwarding on non-point-to-point interfaces. + // TODO: get the MTU from LinkProperties and update the rules when it changes. + if (!mBpfCoordinatorShim.addIpv6UpstreamRule(rule)) { + return; } - // Must update the adding rule after calling #isAnyRuleOnUpstream because it needs to - // check if it is about adding a first rule for a given upstream. + ArraySet rules = mIpv6UpstreamRules.computeIfAbsent( + ipServer, k -> new ArraySet()); + rules.add(rule); + } + + /** + * Clear all IPv6 upstream rules for a given downstream. After removing the last rule on a given + * upstream, must clear data limit, update the last tether stats and remove the tether stats in + * the BPF maps. + */ + private void clearIpv6UpstreamRules(@NonNull final IpServer ipServer) { + if (!isUsingBpf()) return; + + final ArraySet upstreamRules = mIpv6UpstreamRules.remove(ipServer); + if (upstreamRules == null) return; + + int upstreamIfindex = 0; + for (Ipv6UpstreamRule rule: upstreamRules) { + if (upstreamIfindex != 0 && rule.upstreamIfindex != upstreamIfindex) { + Log.wtf(TAG, "BUG: upstream rules point to more than one interface"); + } + upstreamIfindex = rule.upstreamIfindex; + mBpfCoordinatorShim.removeIpv6UpstreamRule(rule); + } + // Clear the limit if there are no more rules on the given upstream. + // Using upstreamIfindex outside the loop is fine because all the rules for a given IpServer + // will always have the same upstream index (since they are always added all together by + // updateAllIpv6Rules). + // The upstreamIfindex can't be 0 because we won't add an Ipv6UpstreamRule with + // upstreamIfindex == 0 and if there is no Ipv6UpstreamRule for an IpServer, it will be + // removed from mIpv6UpstreamRules. + if (upstreamIfindex == 0) { + Log.wtf(TAG, "BUG: upstream rules have empty Set or rule.upstreamIfindex == 0"); + return; + } + maybeClearLimit(upstreamIfindex); + } + + /** + * Add IPv6 downstream rule. + * Note that this can be only called on handler thread. + */ + public void addIpv6DownstreamRule( + @NonNull final IpServer ipServer, @NonNull final Ipv6DownstreamRule rule) { + if (!isUsingBpf()) return; + + // TODO: Perhaps avoid to add a duplicate rule. + if (rule.upstreamIfindex != NO_UPSTREAM + && !mBpfCoordinatorShim.addIpv6DownstreamRule(rule)) return; + + LinkedHashMap rules = + mIpv6DownstreamRules.computeIfAbsent(ipServer, + k -> new LinkedHashMap()); rules.put(rule.address, rule); } /** - * Remove IPv6 downstream rule. After removing the last rule on a given upstream, must clear - * data limit, update the last tether stats and remove the tether stats in the BPF maps. + * Remove IPv6 downstream rule. * Note that this can be only called on handler thread. */ public void removeIpv6DownstreamRule( @NonNull final IpServer ipServer, @NonNull final Ipv6DownstreamRule rule) { if (!isUsingBpf()) return; - if (!mBpfCoordinatorShim.removeIpv6DownstreamRule(rule)) return; + if (rule.upstreamIfindex != NO_UPSTREAM + && !mBpfCoordinatorShim.removeIpv6DownstreamRule(rule)) return; LinkedHashMap rules = mIpv6DownstreamRules.get(ipServer); if (rules == null) return; - // Must remove rules before calling #isAnyRuleOnUpstream because it needs to check if - // the last rule is removed for a given upstream. If no rule is removed, return early. - // Avoid unnecessary work on a non-existent rule which may have never been added or - // removed already. + // If no rule is removed, return early. Avoid unnecessary work on a non-existent rule which + // may have never been added or removed already. if (rules.remove(rule.address) == null) return; // Remove the downstream entry if it has no more rule. if (rules.isEmpty()) { mIpv6DownstreamRules.remove(ipServer); } + } - // If no more rules between this upstream and downstream, stop upstream forwarding. - if (!isAnyRuleFromDownstreamToUpstream(rule.downstreamIfindex, rule.upstreamIfindex)) { - Ipv6UpstreamRule upstreamRule = new Ipv6UpstreamRule(rule.upstreamIfindex, - rule.downstreamIfindex, IPV6_ZERO_PREFIX64, rule.srcMac, NULL_MAC_ADDRESS, - NULL_MAC_ADDRESS); - if (!mBpfCoordinatorShim.removeIpv6UpstreamRule(upstreamRule)) { - mLog.e("Failed to remove upstream IPv6 forwarding rule: " + upstreamRule); - } + /** + * Clear all downstream rules for a given IpServer and return a copy of all removed rules. + */ + @Nullable + private Collection clearIpv6DownstreamRules( + @NonNull final IpServer ipServer) { + final LinkedHashMap downstreamRules = + mIpv6DownstreamRules.remove(ipServer); + if (downstreamRules == null) return null; + + final Collection removedRules = downstreamRules.values(); + for (final Ipv6DownstreamRule rule : removedRules) { + if (rule.upstreamIfindex == NO_UPSTREAM) continue; + mBpfCoordinatorShim.removeIpv6DownstreamRule(rule); } - - // Do cleanup functionality if there is no more rule on the given upstream. - maybeClearLimit(rule.upstreamIfindex); + return removedRules; } /** * Clear all forwarding rules for a given downstream. * Note that this can be only called on handler thread. - * TODO: rename to tetherOffloadRuleClear6 because of IPv6 only. */ - public void tetherOffloadRuleClear(@NonNull final IpServer ipServer) { + public void clearAllIpv6Rules(@NonNull final IpServer ipServer) { if (!isUsingBpf()) return; - final LinkedHashMap rules = - mIpv6DownstreamRules.get(ipServer); - if (rules == null) return; - - // Need to build a rule list because the rule map may be changed in the iteration. - for (final Ipv6DownstreamRule rule : new ArrayList(rules.values())) { - removeIpv6DownstreamRule(ipServer, rule); - } + // Clear downstream rules first, because clearing upstream rules fetches the stats, and + // fetching the stats requires that no rules be forwarding traffic to or from the upstream. + clearIpv6DownstreamRules(ipServer); + clearIpv6UpstreamRules(ipServer); } /** - * Update existing forwarding rules to new upstream for a given downstream. + * Delete all upstream and downstream rules for the passed-in IpServer, and if the new upstream + * is nonzero, reapply them to the new upstream. * Note that this can be only called on handler thread. */ - public void tetherOffloadRuleUpdate(@NonNull final IpServer ipServer, int newUpstreamIfindex) { + public void updateAllIpv6Rules(@NonNull final IpServer ipServer, + final InterfaceParams interfaceParams, int newUpstreamIfindex) { if (!isUsingBpf()) return; - final LinkedHashMap rules = - mIpv6DownstreamRules.get(ipServer); - if (rules == null) return; + // Remove IPv6 downstream rules. Remove the old ones before adding the new rules, otherwise + // we need to keep a copy of the old rules. + // We still need to keep the downstream rules even when the upstream goes away because it + // may come back with the same prefixes (unlikely, but possible). Neighbor entries won't be + // deleted and we're not expected to receive new Neighbor events in this case. + // TODO: Add new rule first to reduce the latency which has no rule. But this is okay + // because if this is a new upstream, it will probably have different prefixes than + // the one these downstream rules are in. If so, they will never see any downstream + // traffic before new neighbor entries are created. + final Collection deletedDownstreamRules = + clearIpv6DownstreamRules(ipServer); - // Need to build a rule list because the rule map may be changed in the iteration. - // First remove all the old rules, then add all the new rules. This is because the upstream - // forwarding code in addIpv6DownstreamRule cannot support rules on two upstreams at the - // same time. Deleting the rules first ensures that upstream forwarding is disabled on the - // old upstream when the last rule is removed from it, and re-enabled on the new upstream - // when the first rule is added to it. - // TODO: Once the IPv6 client processing code has moved from IpServer to BpfCoordinator, do - // something smarter. - final ArrayList rulesCopy = new ArrayList<>(rules.values()); - for (final Ipv6DownstreamRule rule : rulesCopy) { - // Remove the old rule before adding the new one because the map uses the same key for - // both rules. Reversing the processing order causes that the new rule is removed as - // unexpected. - // TODO: Add new rule first to reduce the latency which has no rule. - removeIpv6DownstreamRule(ipServer, rule); + // Remove IPv6 upstream rules. Downstream rules must be removed first because + // BpfCoordinatorShimImpl#tetherOffloadGetAndClearStats will be called after the removal of + // the last upstream rule and it requires that no rules be forwarding traffic to or from + // that upstream. + clearIpv6UpstreamRules(ipServer); + + // Add new upstream rules. + if (newUpstreamIfindex != 0 && interfaceParams != null && interfaceParams.macAddr != null) { + addIpv6UpstreamRule(ipServer, new Ipv6UpstreamRule( + newUpstreamIfindex, interfaceParams.index, IPV6_ZERO_PREFIX64, + interfaceParams.macAddr, NULL_MAC_ADDRESS, NULL_MAC_ADDRESS)); } - for (final Ipv6DownstreamRule rule : rulesCopy) { + + // Add updated downstream rules. + if (deletedDownstreamRules == null) return; + for (final Ipv6DownstreamRule rule : deletedDownstreamRules) { addIpv6DownstreamRule(ipServer, rule.onNewUpstream(newUpstreamIfindex)); } } @@ -737,7 +809,7 @@ public class BpfCoordinator { * expects the interface name in NetworkStats object. * Note that this can be only called on handler thread. */ - public void addUpstreamNameToLookupTable(int upstreamIfindex, @NonNull String upstreamIface) { + public void maybeAddUpstreamToLookupTable(int upstreamIfindex, @Nullable String upstreamIface) { if (!isUsingBpf()) return; if (upstreamIfindex == 0 || TextUtils.isEmpty(upstreamIface)) return; @@ -1007,7 +1079,7 @@ public class BpfCoordinator { * TODO: consider error handling if the attach program failed. */ public void maybeAttachProgram(@NonNull String intIface, @NonNull String extIface) { - if (isVcnInterface(extIface)) return; + if (!isUsingBpf() || isVcnInterface(extIface)) return; if (forwardingPairExists(intIface, extIface)) return; @@ -1031,6 +1103,8 @@ public class BpfCoordinator { * Detach BPF program */ public void maybeDetachProgram(@NonNull String intIface, @NonNull String extIface) { + if (!isUsingBpf()) return; + forwardingPairRemove(intIface, extIface); // Detaching program may fail because the interface has been removed already. @@ -1967,9 +2041,8 @@ public class BpfCoordinator { } private int getInterfaceIndexFromRules(@NonNull String ifName) { - for (LinkedHashMap rules : - mIpv6DownstreamRules.values()) { - for (Ipv6DownstreamRule rule : rules.values()) { + for (ArraySet rules : mIpv6UpstreamRules.values()) { + for (Ipv6UpstreamRule rule : rules) { final int upstreamIfindex = rule.upstreamIfindex; if (TextUtils.equals(ifName, mInterfaceNames.get(upstreamIfindex))) { return upstreamIfindex; @@ -1987,6 +2060,7 @@ public class BpfCoordinator { } private boolean sendDataLimitToBpfMap(int ifIndex, long quotaBytes) { + if (!isUsingBpf()) return false; if (ifIndex == 0) { Log.wtf(TAG, "Invalid interface index."); return false; @@ -2060,28 +2134,14 @@ public class BpfCoordinator { // TODO: Rename to isAnyIpv6RuleOnUpstream and define an isAnyRuleOnUpstream method that called // both isAnyIpv6RuleOnUpstream and mBpfCoordinatorShim.isAnyIpv4RuleOnUpstream. private boolean isAnyRuleOnUpstream(int upstreamIfindex) { - for (LinkedHashMap rules : - mIpv6DownstreamRules.values()) { - for (Ipv6DownstreamRule rule : rules.values()) { + for (ArraySet rules : mIpv6UpstreamRules.values()) { + for (Ipv6UpstreamRule rule : rules) { if (upstreamIfindex == rule.upstreamIfindex) return true; } } return false; } - private boolean isAnyRuleFromDownstreamToUpstream(int downstreamIfindex, int upstreamIfindex) { - for (LinkedHashMap rules : - mIpv6DownstreamRules.values()) { - for (Ipv6DownstreamRule rule : rules.values()) { - if (downstreamIfindex == rule.downstreamIfindex - && upstreamIfindex == rule.upstreamIfindex) { - return true; - } - } - } - 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) { diff --git a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java index c0718d1b3e..d497a4dd6b 100644 --- a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java +++ b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java @@ -158,6 +158,7 @@ public class IpServerTest { private static final String UPSTREAM_IFACE = "upstream0"; private static final String UPSTREAM_IFACE2 = "upstream1"; private static final String IPSEC_IFACE = "ipsec0"; + private static final int NO_UPSTREAM = 0; private static final int UPSTREAM_IFINDEX = 101; private static final int UPSTREAM_IFINDEX2 = 102; private static final int IPSEC_IFINDEX = 103; @@ -274,8 +275,18 @@ public class IpServerTest { LinkProperties lp = new LinkProperties(); lp.setInterfaceName(upstreamIface); dispatchTetherConnectionChanged(upstreamIface, lp, 0); + if (usingBpfOffload) { + InterfaceParams interfaceParams = mDependencies.getInterfaceParams(upstreamIface); + assertNotNull("missing upstream interface: " + upstreamIface, interfaceParams); + verify(mBpfCoordinator).updateAllIpv6Rules( + mIpServer, TEST_IFACE_PARAMS, interfaceParams.index); + verifyStartUpstreamIpv6Forwarding(null, interfaceParams.index); + } else { + verifyNoUpstreamIpv6ForwardingChange(null); + } } - reset(mNetd, mCallback, mAddressCoordinator, mBpfCoordinator); + reset(mCallback, mAddressCoordinator); + resetNetdBpfMapAndCoordinator(); when(mAddressCoordinator.requestDownstreamAddress(any(), anyInt(), anyBoolean())).thenReturn(mTestAddress); } @@ -531,7 +542,7 @@ public class IpServerTest { InOrder inOrder = inOrder(mNetd, mBpfCoordinator); // Add the forwarding pair . - inOrder.verify(mBpfCoordinator).addUpstreamNameToLookupTable(UPSTREAM_IFINDEX, + inOrder.verify(mBpfCoordinator).maybeAddUpstreamToLookupTable(UPSTREAM_IFINDEX, UPSTREAM_IFACE); inOrder.verify(mBpfCoordinator).maybeAttachProgram(IFACE_NAME, UPSTREAM_IFACE); inOrder.verify(mNetd).tetherAddForward(IFACE_NAME, UPSTREAM_IFACE); @@ -553,7 +564,7 @@ public class IpServerTest { inOrder.verify(mNetd).tetherRemoveForward(IFACE_NAME, UPSTREAM_IFACE); // Add the forwarding pair . - inOrder.verify(mBpfCoordinator).addUpstreamNameToLookupTable(UPSTREAM_IFINDEX2, + inOrder.verify(mBpfCoordinator).maybeAddUpstreamToLookupTable(UPSTREAM_IFINDEX2, UPSTREAM_IFACE2); inOrder.verify(mBpfCoordinator).maybeAttachProgram(IFACE_NAME, UPSTREAM_IFACE2); inOrder.verify(mNetd).tetherAddForward(IFACE_NAME, UPSTREAM_IFACE2); @@ -578,7 +589,7 @@ public class IpServerTest { // Add the forwarding pair and expect that failed on // tetherAddForward. - inOrder.verify(mBpfCoordinator).addUpstreamNameToLookupTable(UPSTREAM_IFINDEX2, + inOrder.verify(mBpfCoordinator).maybeAddUpstreamToLookupTable(UPSTREAM_IFINDEX2, UPSTREAM_IFACE2); inOrder.verify(mBpfCoordinator).maybeAttachProgram(IFACE_NAME, UPSTREAM_IFACE2); inOrder.verify(mNetd).tetherAddForward(IFACE_NAME, UPSTREAM_IFACE2); @@ -606,7 +617,7 @@ public class IpServerTest { // Add the forwarding pair and expect that failed on // ipfwdAddInterfaceForward. - inOrder.verify(mBpfCoordinator).addUpstreamNameToLookupTable(UPSTREAM_IFINDEX2, + inOrder.verify(mBpfCoordinator).maybeAddUpstreamToLookupTable(UPSTREAM_IFINDEX2, UPSTREAM_IFACE2); inOrder.verify(mBpfCoordinator).maybeAttachProgram(IFACE_NAME, UPSTREAM_IFACE2); inOrder.verify(mNetd).tetherAddForward(IFACE_NAME, UPSTREAM_IFACE2); @@ -627,7 +638,15 @@ public class IpServerTest { inOrder.verify(mBpfCoordinator).maybeDetachProgram(IFACE_NAME, UPSTREAM_IFACE); inOrder.verify(mNetd).ipfwdRemoveInterfaceForward(IFACE_NAME, UPSTREAM_IFACE); inOrder.verify(mNetd).tetherRemoveForward(IFACE_NAME, UPSTREAM_IFACE); - inOrder.verify(mBpfCoordinator).tetherOffloadRuleClear(mIpServer); + inOrder.verify(mBpfCoordinator).updateAllIpv6Rules( + mIpServer, TEST_IFACE_PARAMS, NO_UPSTREAM); + if (!mBpfDeps.isAtLeastS()) { + inOrder.verify(mNetd).tetherOffloadGetAndClearStats(UPSTREAM_IFINDEX); + } + // When tethering stops, upstream interface is set to zero and thus clearing all upstream + // rules. Downstream rules are needed to be cleared explicitly by calling + // BpfCoordinator#clearAllIpv6Rules in TetheredState#exit. + inOrder.verify(mBpfCoordinator).clearAllIpv6Rules(mIpServer); inOrder.verify(mNetd).tetherApplyDnsInterfaces(); inOrder.verify(mNetd).tetherInterfaceRemove(IFACE_NAME); inOrder.verify(mNetd).networkRemoveInterface(INetd.LOCAL_NET_ID, IFACE_NAME); @@ -1064,13 +1083,12 @@ public class IpServerTest { recvNewNeigh(notMyIfindex, neighA, NUD_REACHABLE, macA); verifyNoMoreInteractions(mBpfCoordinator, mNetd, mBpfDownstream6Map, mBpfUpstream6Map); - // Events on this interface are received and sent to netd. + // Events on this interface are received and sent to BpfCoordinator. recvNewNeigh(myIfindex, neighA, NUD_REACHABLE, macA); verify(mBpfCoordinator).addIpv6DownstreamRule( mIpServer, makeDownstreamRule(UPSTREAM_IFINDEX, neighA, macA)); verifyTetherOffloadRuleAdd(null, UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighA, macA); - verifyStartUpstreamIpv6Forwarding(null, UPSTREAM_IFINDEX); resetNetdBpfMapAndCoordinator(); recvNewNeigh(myIfindex, neighB, NUD_REACHABLE, macB); @@ -1078,7 +1096,6 @@ public class IpServerTest { mIpServer, makeDownstreamRule(UPSTREAM_IFINDEX, neighB, macB)); verifyTetherOffloadRuleAdd(null, UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighB, macB); - verifyNoUpstreamIpv6ForwardingChange(null); resetNetdBpfMapAndCoordinator(); // Link-local and multicast neighbors are ignored. @@ -1094,7 +1111,6 @@ public class IpServerTest { mIpServer, makeDownstreamRule(UPSTREAM_IFINDEX, neighA, macNull)); verifyTetherOffloadRuleRemove(null, UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighA, macNull); - verifyNoUpstreamIpv6ForwardingChange(null); resetNetdBpfMapAndCoordinator(); // A neighbor that is deleted causes the rule to be removed. @@ -1103,12 +1119,10 @@ public class IpServerTest { mIpServer, makeDownstreamRule(UPSTREAM_IFINDEX, neighB, macNull)); verifyTetherOffloadRuleRemove(null, UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighB, macNull); - verifyStopUpstreamIpv6Forwarding(null); resetNetdBpfMapAndCoordinator(); // Upstream changes result in updating the rules. recvNewNeigh(myIfindex, neighA, NUD_REACHABLE, macA); - verifyStartUpstreamIpv6Forwarding(null, UPSTREAM_IFINDEX); recvNewNeigh(myIfindex, neighB, NUD_REACHABLE, macB); resetNetdBpfMapAndCoordinator(); @@ -1116,89 +1130,100 @@ public class IpServerTest { LinkProperties lp = new LinkProperties(); lp.setInterfaceName(UPSTREAM_IFACE2); dispatchTetherConnectionChanged(UPSTREAM_IFACE2, lp, -1); - verify(mBpfCoordinator).tetherOffloadRuleUpdate(mIpServer, UPSTREAM_IFINDEX2); + verify(mBpfCoordinator).updateAllIpv6Rules(mIpServer, TEST_IFACE_PARAMS, UPSTREAM_IFINDEX2); verifyTetherOffloadRuleRemove(inOrder, UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighA, macA); verifyTetherOffloadRuleRemove(inOrder, UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighB, macB); verifyStopUpstreamIpv6Forwarding(inOrder); - verifyTetherOffloadRuleAdd(inOrder, - UPSTREAM_IFINDEX2, UPSTREAM_IFACE_PARAMS2.macAddr, neighA, macA); verifyStartUpstreamIpv6Forwarding(inOrder, UPSTREAM_IFINDEX2); + verifyTetherOffloadRuleAdd(inOrder, + UPSTREAM_IFINDEX2, UPSTREAM_IFACE_PARAMS2.macAddr, neighA, macA); verifyTetherOffloadRuleAdd(inOrder, UPSTREAM_IFINDEX2, UPSTREAM_IFACE_PARAMS2.macAddr, neighB, macB); - verifyNoUpstreamIpv6ForwardingChange(inOrder); resetNetdBpfMapAndCoordinator(); // When the upstream is lost, rules are removed. dispatchTetherConnectionChanged(null, null, 0); - // Clear function is called two times by: + // Upstream clear function is called two times by: // - processMessage CMD_TETHER_CONNECTION_CHANGED for the upstream is lost. // - processMessage CMD_IPV6_TETHER_UPDATE for the IPv6 upstream is lost. // See dispatchTetherConnectionChanged. - verify(mBpfCoordinator, times(2)).tetherOffloadRuleClear(mIpServer); + verify(mBpfCoordinator, times(2)).updateAllIpv6Rules( + mIpServer, TEST_IFACE_PARAMS, NO_UPSTREAM); + verifyStopUpstreamIpv6Forwarding(inOrder); verifyTetherOffloadRuleRemove(null, UPSTREAM_IFINDEX2, UPSTREAM_IFACE_PARAMS2.macAddr, neighA, macA); verifyTetherOffloadRuleRemove(null, UPSTREAM_IFINDEX2, UPSTREAM_IFACE_PARAMS2.macAddr, neighB, macB); - verifyStopUpstreamIpv6Forwarding(inOrder); + // Upstream lost doesn't clear the downstream rules from BpfCoordinator. + // Do that here. + recvDelNeigh(myIfindex, neighA, NUD_STALE, macA); + recvDelNeigh(myIfindex, neighB, NUD_STALE, macB); + verify(mBpfCoordinator).removeIpv6DownstreamRule( + mIpServer, makeDownstreamRule(NO_UPSTREAM, neighA, macNull)); + verify(mBpfCoordinator).removeIpv6DownstreamRule( + mIpServer, makeDownstreamRule(NO_UPSTREAM, neighB, macNull)); resetNetdBpfMapAndCoordinator(); - // If the upstream is IPv4-only, no rules are added. + // If the upstream is IPv4-only, no IPv6 rules are added to BPF map. dispatchTetherConnectionChanged(UPSTREAM_IFACE); resetNetdBpfMapAndCoordinator(); recvNewNeigh(myIfindex, neighA, NUD_REACHABLE, macA); - // Clear function is called by #updateIpv6ForwardingRules for the IPv6 upstream is lost. - verify(mBpfCoordinator).tetherOffloadRuleClear(mIpServer); verifyNoUpstreamIpv6ForwardingChange(null); + // Downstream rules are only added to BpfCoordinator but not BPF map. + verify(mBpfCoordinator).addIpv6DownstreamRule( + mIpServer, makeDownstreamRule(NO_UPSTREAM, neighA, macA)); + verifyNeverTetherOffloadRuleAdd(); verifyNoMoreInteractions(mBpfCoordinator, mNetd, mBpfDownstream6Map, mBpfUpstream6Map); - // Rules can be added again once upstream IPv6 connectivity is available. + // Rules can be added again once upstream IPv6 connectivity is available. The existing rules + // with an upstream of NO_UPSTREAM are reapplied. lp.setInterfaceName(UPSTREAM_IFACE); dispatchTetherConnectionChanged(UPSTREAM_IFACE, lp, -1); + verifyStartUpstreamIpv6Forwarding(null, UPSTREAM_IFINDEX); + verify(mBpfCoordinator).addIpv6DownstreamRule( + mIpServer, makeDownstreamRule(UPSTREAM_IFINDEX, neighA, macA)); + verifyTetherOffloadRuleAdd(null, + UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighA, macA); recvNewNeigh(myIfindex, neighB, NUD_REACHABLE, macB); verify(mBpfCoordinator).addIpv6DownstreamRule( mIpServer, makeDownstreamRule(UPSTREAM_IFINDEX, neighB, macB)); verifyTetherOffloadRuleAdd(null, UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighB, macB); - verifyStartUpstreamIpv6Forwarding(null, UPSTREAM_IFINDEX); - verify(mBpfCoordinator, never()).addIpv6DownstreamRule( - mIpServer, makeDownstreamRule(UPSTREAM_IFINDEX, neighA, macA)); - verifyNeverTetherOffloadRuleAdd( - UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighA, macA); // If upstream IPv6 connectivity is lost, rules are removed. resetNetdBpfMapAndCoordinator(); dispatchTetherConnectionChanged(UPSTREAM_IFACE, null, 0); - verify(mBpfCoordinator).tetherOffloadRuleClear(mIpServer); + verify(mBpfCoordinator).updateAllIpv6Rules(mIpServer, TEST_IFACE_PARAMS, NO_UPSTREAM); verifyTetherOffloadRuleRemove(null, UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighB, macB); verifyStopUpstreamIpv6Forwarding(null); - // When the interface goes down, rules are removed. + // When upstream IPv6 connectivity comes back, upstream rules are added and downstream rules + // are reapplied. lp.setInterfaceName(UPSTREAM_IFACE); dispatchTetherConnectionChanged(UPSTREAM_IFACE, lp, -1); - recvNewNeigh(myIfindex, neighA, NUD_REACHABLE, macA); - recvNewNeigh(myIfindex, neighB, NUD_REACHABLE, macB); + verifyStartUpstreamIpv6Forwarding(null, UPSTREAM_IFINDEX); verify(mBpfCoordinator).addIpv6DownstreamRule( mIpServer, makeDownstreamRule(UPSTREAM_IFINDEX, neighA, macA)); verifyTetherOffloadRuleAdd(null, UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighA, macA); - verifyStartUpstreamIpv6Forwarding(null, UPSTREAM_IFINDEX); verify(mBpfCoordinator).addIpv6DownstreamRule( mIpServer, makeDownstreamRule(UPSTREAM_IFINDEX, neighB, macB)); verifyTetherOffloadRuleAdd(null, UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighB, macB); resetNetdBpfMapAndCoordinator(); + // When the downstream interface goes down, rules are removed. mIpServer.stop(); mLooper.dispatchAll(); - verify(mBpfCoordinator).tetherOffloadRuleClear(mIpServer); + verify(mBpfCoordinator).clearAllIpv6Rules(mIpServer); + verifyStopUpstreamIpv6Forwarding(null); verifyTetherOffloadRuleRemove(null, UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighA, macA); verifyTetherOffloadRuleRemove(null, UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighB, macB); - verifyStopUpstreamIpv6Forwarding(null); verify(mIpNeighborMonitor).stop(); resetNetdBpfMapAndCoordinator(); } @@ -1228,7 +1253,6 @@ public class IpServerTest { mIpServer, makeDownstreamRule(UPSTREAM_IFINDEX, neigh, macA)); verifyTetherOffloadRuleAdd(null, UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neigh, macA); - verifyStartUpstreamIpv6Forwarding(null, UPSTREAM_IFINDEX); resetNetdBpfMapAndCoordinator(); recvDelNeigh(myIfindex, neigh, NUD_STALE, macA); @@ -1236,7 +1260,14 @@ public class IpServerTest { mIpServer, makeDownstreamRule(UPSTREAM_IFINDEX, neigh, macNull)); verifyTetherOffloadRuleRemove(null, UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neigh, macNull); - verifyStopUpstreamIpv6Forwarding(null); + resetNetdBpfMapAndCoordinator(); + + // Upstream IPv6 connectivity change causes upstream rules change. + LinkProperties lp2 = new LinkProperties(); + lp2.setInterfaceName(UPSTREAM_IFACE2); + dispatchTetherConnectionChanged(UPSTREAM_IFACE2, lp2, 0); + verify(mBpfCoordinator).updateAllIpv6Rules(mIpServer, TEST_IFACE_PARAMS, UPSTREAM_IFINDEX2); + verifyStartUpstreamIpv6Forwarding(null, UPSTREAM_IFINDEX2); resetNetdBpfMapAndCoordinator(); // [2] Disable BPF offload. @@ -1247,12 +1278,16 @@ public class IpServerTest { recvNewNeigh(myIfindex, neigh, NUD_REACHABLE, macA); verifyNeverTetherOffloadRuleAdd(); - verifyNoUpstreamIpv6ForwardingChange(null); resetNetdBpfMapAndCoordinator(); recvDelNeigh(myIfindex, neigh, NUD_STALE, macA); verifyNeverTetherOffloadRuleRemove(); + resetNetdBpfMapAndCoordinator(); + + // Upstream IPv6 connectivity change doesn't cause the rule to be added or removed. + dispatchTetherConnectionChanged(UPSTREAM_IFACE2, lp2, 0); verifyNoUpstreamIpv6ForwardingChange(null); + verifyNeverTetherOffloadRuleRemove(); resetNetdBpfMapAndCoordinator(); } 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 04eb4304cb..601f587f76 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java @@ -169,6 +169,8 @@ public class BpfCoordinatorTest { private static final String UPSTREAM_IFACE = "rmnet0"; private static final String UPSTREAM_XLAT_IFACE = "v4-rmnet0"; private static final String UPSTREAM_IFACE2 = "wlan0"; + private static final String DOWNSTREAM_IFACE = "downstream1"; + private static final String DOWNSTREAM_IFACE2 = "downstream2"; private static final MacAddress DOWNSTREAM_MAC = MacAddress.fromString("12:34:56:78:90:ab"); private static final MacAddress DOWNSTREAM_MAC2 = MacAddress.fromString("ab:90:78:56:34:12"); @@ -213,6 +215,11 @@ public class BpfCoordinatorTest { private static final InterfaceParams UPSTREAM_IFACE_PARAMS2 = new InterfaceParams( UPSTREAM_IFACE2, UPSTREAM_IFINDEX2, MacAddress.fromString("44:55:66:00:00:0c"), NetworkStackConstants.ETHER_MTU); + private static final InterfaceParams DOWNSTREAM_IFACE_PARAMS = new InterfaceParams( + DOWNSTREAM_IFACE, DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC, NetworkStackConstants.ETHER_MTU); + private static final InterfaceParams DOWNSTREAM_IFACE_PARAMS2 = new InterfaceParams( + DOWNSTREAM_IFACE2, DOWNSTREAM_IFINDEX2, DOWNSTREAM_MAC2, + NetworkStackConstants.ETHER_MTU); private static final Map UPSTREAM_INFORMATIONS = Map.of( UPSTREAM_IFINDEX, new UpstreamInformation(UPSTREAM_IFACE_PARAMS, @@ -640,24 +647,6 @@ public class BpfCoordinatorTest { } } - private void verifyStartUpstreamIpv6Forwarding(@Nullable InOrder inOrder, int downstreamIfIndex, - MacAddress downstreamMac, int upstreamIfindex) throws Exception { - if (!mDeps.isAtLeastS()) return; - final TetherUpstream6Key key = new TetherUpstream6Key(downstreamIfIndex, downstreamMac, 0); - final Tether6Value value = new Tether6Value(upstreamIfindex, - MacAddress.ALL_ZEROS_ADDRESS, MacAddress.ALL_ZEROS_ADDRESS, - ETH_P_IPV6, NetworkStackConstants.ETHER_MTU); - verifyWithOrder(inOrder, mBpfUpstream6Map).insertEntry(key, value); - } - - private void verifyStopUpstreamIpv6Forwarding(@Nullable InOrder inOrder, int downstreamIfIndex, - MacAddress downstreamMac) - throws Exception { - if (!mDeps.isAtLeastS()) return; - final TetherUpstream6Key key = new TetherUpstream6Key(downstreamIfIndex, downstreamMac, 0); - verifyWithOrder(inOrder, mBpfUpstream6Map).deleteEntry(key); - } - private void verifyNoUpstreamIpv6ForwardingChange(@Nullable InOrder inOrder) throws Exception { if (!mDeps.isAtLeastS()) return; if (inOrder != null) { @@ -671,6 +660,13 @@ public class BpfCoordinatorTest { } } + private void verifyAddUpstreamRule(@Nullable InOrder inOrder, + @NonNull Ipv6UpstreamRule rule) throws Exception { + if (!mDeps.isAtLeastS()) return; + verifyWithOrder(inOrder, mBpfUpstream6Map).insertEntry( + rule.makeTetherUpstream6Key(), rule.makeTether6Value()); + } + private void verifyAddDownstreamRule(@Nullable InOrder inOrder, @NonNull Ipv6DownstreamRule rule) throws Exception { if (mDeps.isAtLeastS()) { @@ -681,6 +677,11 @@ public class BpfCoordinatorTest { } } + private void verifyNeverAddUpstreamRule() throws Exception { + if (!mDeps.isAtLeastS()) return; + verify(mBpfUpstream6Map, never()).insertEntry(any(), any()); + } + private void verifyNeverAddDownstreamRule() throws Exception { if (mDeps.isAtLeastS()) { verify(mBpfDownstream6Map, never()).updateEntry(any(), any()); @@ -689,6 +690,13 @@ public class BpfCoordinatorTest { } } + private void verifyRemoveUpstreamRule(@Nullable InOrder inOrder, + @NonNull final Ipv6UpstreamRule rule) throws Exception { + if (!mDeps.isAtLeastS()) return; + verifyWithOrder(inOrder, mBpfUpstream6Map).deleteEntry( + rule.makeTetherUpstream6Key()); + } + private void verifyRemoveDownstreamRule(@Nullable InOrder inOrder, @NonNull final Ipv6DownstreamRule rule) throws Exception { if (mDeps.isAtLeastS()) { @@ -699,6 +707,11 @@ public class BpfCoordinatorTest { } } + private void verifyNeverRemoveUpstreamRule() throws Exception { + if (!mDeps.isAtLeastS()) return; + verify(mBpfUpstream6Map, never()).deleteEntry(any()); + } + private void verifyNeverRemoveDownstreamRule() throws Exception { if (mDeps.isAtLeastS()) { verify(mBpfDownstream6Map, never()).deleteEntry(any()); @@ -763,24 +776,31 @@ public class BpfCoordinatorTest { final String mobileIface = "rmnet_data0"; final Integer mobileIfIndex = 100; - coordinator.addUpstreamNameToLookupTable(mobileIfIndex, mobileIface); + coordinator.maybeAddUpstreamToLookupTable(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, mBpfDownstream6Map, mBpfLimitMap, mBpfStatsMap); - final Ipv6DownstreamRule rule = buildTestDownstreamRule(mobileIfIndex, NEIGH_A, MAC_A); - coordinator.addIpv6DownstreamRule(mIpServer, rule); - verifyAddDownstreamRule(inOrder, rule); + final InOrder inOrder = inOrder(mNetd, mBpfUpstream6Map, mBpfDownstream6Map, mBpfLimitMap, + mBpfStatsMap); + final Ipv6UpstreamRule upstreamRule = buildTestUpstreamRule( + mobileIfIndex, DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC); + final Ipv6DownstreamRule downstreamRule = buildTestDownstreamRule( + mobileIfIndex, NEIGH_A, MAC_A); + coordinator.updateAllIpv6Rules(mIpServer, DOWNSTREAM_IFACE_PARAMS, mobileIfIndex); verifyTetherOffloadSetInterfaceQuota(inOrder, mobileIfIndex, QUOTA_UNLIMITED, true /* isInit */); + verifyAddUpstreamRule(inOrder, upstreamRule); + coordinator.addIpv6DownstreamRule(mIpServer, downstreamRule); + verifyAddDownstreamRule(inOrder, downstreamRule); - // Removing the last rule on current upstream immediately sends the cleanup stuff to netd. + // Removing the last rule on current upstream immediately sends the cleanup stuff to BPF. updateStatsEntryForTetherOffloadGetAndClearStats( buildTestTetherStatsParcel(mobileIfIndex, 0, 0, 0, 0)); - coordinator.removeIpv6DownstreamRule(mIpServer, rule); - verifyRemoveDownstreamRule(inOrder, rule); + coordinator.updateAllIpv6Rules(mIpServer, DOWNSTREAM_IFACE_PARAMS, 0); + verifyRemoveDownstreamRule(inOrder, downstreamRule); + verifyRemoveUpstreamRule(inOrder, upstreamRule); verifyTetherOffloadGetAndClearStats(inOrder, mobileIfIndex); } @@ -806,7 +826,7 @@ public class BpfCoordinatorTest { final String mobileIface = "rmnet_data0"; final Integer mobileIfIndex = 100; - coordinator.addUpstreamNameToLookupTable(mobileIfIndex, mobileIface); + coordinator.maybeAddUpstreamToLookupTable(mobileIfIndex, mobileIface); updateStatsEntriesAndWaitForUpdate(new TetherStatsParcel[] { buildTestTetherStatsParcel(mobileIfIndex, 1000, 100, 2000, 200)}); @@ -847,8 +867,8 @@ public class BpfCoordinatorTest { // Add interface name to lookup table. In realistic case, the upstream interface name will // be added by IpServer when IpServer has received with a new IPv6 upstream update event. - coordinator.addUpstreamNameToLookupTable(wlanIfIndex, wlanIface); - coordinator.addUpstreamNameToLookupTable(mobileIfIndex, mobileIface); + coordinator.maybeAddUpstreamToLookupTable(wlanIfIndex, wlanIface); + coordinator.maybeAddUpstreamToLookupTable(mobileIfIndex, mobileIface); // [1] Both interface stats are changed. // Setup the tether stats of wlan and mobile interface. Note that move forward the time of @@ -912,7 +932,7 @@ public class BpfCoordinatorTest { final String mobileIface = "rmnet_data0"; final Integer mobileIfIndex = 100; - coordinator.addUpstreamNameToLookupTable(mobileIfIndex, mobileIface); + coordinator.maybeAddUpstreamToLookupTable(mobileIfIndex, mobileIface); // Verify that set quota to 0 will immediately triggers a callback. mTetherStatsProvider.onSetAlert(0); @@ -978,9 +998,10 @@ public class BpfCoordinatorTest { } @NonNull - private static Ipv6UpstreamRule buildTestUpstreamRule(int upstreamIfindex) { - return new Ipv6UpstreamRule(upstreamIfindex, DOWNSTREAM_IFINDEX, - IPV6_ZERO_PREFIX, DOWNSTREAM_MAC, MacAddress.ALL_ZEROS_ADDRESS, + private static Ipv6UpstreamRule buildTestUpstreamRule( + int upstreamIfindex, int downstreamIfindex, @NonNull MacAddress inDstMac) { + return new Ipv6UpstreamRule(upstreamIfindex, downstreamIfindex, + IPV6_ZERO_PREFIX, inDstMac, MacAddress.ALL_ZEROS_ADDRESS, MacAddress.ALL_ZEROS_ADDRESS); } @@ -1027,17 +1048,18 @@ public class BpfCoordinatorTest { final String mobileIface = "rmnet_data0"; final int mobileIfIndex = 100; - coordinator.addUpstreamNameToLookupTable(mobileIfIndex, mobileIface); + coordinator.maybeAddUpstreamToLookupTable(mobileIfIndex, mobileIface); // [1] Default limit. // 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 Ipv6DownstreamRule rule = buildTestDownstreamRule(mobileIfIndex, NEIGH_A, MAC_A); - final InOrder inOrder = inOrder(mNetd, mBpfDownstream6Map, mBpfLimitMap, mBpfStatsMap); - coordinator.addIpv6DownstreamRule(mIpServer, rule); - verifyAddDownstreamRule(inOrder, rule); + final Ipv6UpstreamRule rule = buildTestUpstreamRule( + mobileIfIndex, DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC); + final InOrder inOrder = inOrder(mNetd, mBpfUpstream6Map, mBpfLimitMap, mBpfStatsMap); + coordinator.updateAllIpv6Rules(mIpServer, DOWNSTREAM_IFACE_PARAMS, mobileIfIndex); verifyTetherOffloadSetInterfaceQuota(inOrder, mobileIfIndex, QUOTA_UNLIMITED, true /* isInit */); + verifyAddUpstreamRule(inOrder, rule); inOrder.verifyNoMoreInteractions(); // [2] Specific limit. @@ -1062,7 +1084,6 @@ public class BpfCoordinatorTest { } } - // TODO: Test the case in which the rules are changed from different IpServer objects. @Test public void testSetDataLimitOnRule6Change() throws Exception { setupFunctioningNetdInterface(); @@ -1071,39 +1092,41 @@ public class BpfCoordinatorTest { final String mobileIface = "rmnet_data0"; final int mobileIfIndex = 100; - coordinator.addUpstreamNameToLookupTable(mobileIfIndex, mobileIface); + coordinator.maybeAddUpstreamToLookupTable(mobileIfIndex, mobileIface); // 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, mBpfDownstream6Map, mBpfLimitMap, mBpfStatsMap); + final InOrder inOrder = inOrder(mNetd, mBpfUpstream6Map, mBpfLimitMap, mBpfStatsMap); mTetherStatsProvider.onSetLimit(mobileIface, limit); waitForIdle(); verifyNeverTetherOffloadSetInterfaceQuota(inOrder); - // Adding the first rule on current upstream immediately sends the quota to netd. - final Ipv6DownstreamRule ruleA = buildTestDownstreamRule(mobileIfIndex, NEIGH_A, MAC_A); - coordinator.addIpv6DownstreamRule(mIpServer, ruleA); - verifyAddDownstreamRule(inOrder, ruleA); + // Adding the first rule on current upstream immediately sends the quota to BPF. + final Ipv6UpstreamRule ruleA = buildTestUpstreamRule( + mobileIfIndex, DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC); + coordinator.updateAllIpv6Rules(mIpServer, DOWNSTREAM_IFACE_PARAMS, mobileIfIndex); verifyTetherOffloadSetInterfaceQuota(inOrder, mobileIfIndex, limit, true /* isInit */); + verifyAddUpstreamRule(inOrder, ruleA); inOrder.verifyNoMoreInteractions(); - // Adding the second rule on current upstream does not send the quota to netd. - final Ipv6DownstreamRule ruleB = buildTestDownstreamRule(mobileIfIndex, NEIGH_B, MAC_B); - coordinator.addIpv6DownstreamRule(mIpServer, ruleB); - verifyAddDownstreamRule(inOrder, ruleB); + // Adding the second rule on current upstream does not send the quota to BPF. + final Ipv6UpstreamRule ruleB = buildTestUpstreamRule( + mobileIfIndex, DOWNSTREAM_IFINDEX2, DOWNSTREAM_MAC2); + coordinator.updateAllIpv6Rules(mIpServer2, DOWNSTREAM_IFACE_PARAMS2, mobileIfIndex); + verifyAddUpstreamRule(inOrder, ruleB); verifyNeverTetherOffloadSetInterfaceQuota(inOrder); - // Removing the second rule on current upstream does not send the quota to netd. - coordinator.removeIpv6DownstreamRule(mIpServer, ruleB); - verifyRemoveDownstreamRule(inOrder, ruleB); + // Removing the second rule on current upstream does not send the quota to BPF. + coordinator.updateAllIpv6Rules(mIpServer2, DOWNSTREAM_IFACE_PARAMS2, 0); + verifyRemoveUpstreamRule(inOrder, ruleB); verifyNeverTetherOffloadSetInterfaceQuota(inOrder); - // Removing the last rule on current upstream immediately sends the cleanup stuff to netd. + // Removing the last rule on current upstream immediately sends the cleanup stuff to BPF. updateStatsEntryForTetherOffloadGetAndClearStats( buildTestTetherStatsParcel(mobileIfIndex, 0, 0, 0, 0)); - coordinator.removeIpv6DownstreamRule(mIpServer, ruleA); - verifyRemoveDownstreamRule(inOrder, ruleA); + coordinator.updateAllIpv6Rules(mIpServer, DOWNSTREAM_IFACE_PARAMS, 0); + verifyRemoveUpstreamRule(inOrder, ruleA); verifyTetherOffloadGetAndClearStats(inOrder, mobileIfIndex); inOrder.verifyNoMoreInteractions(); } @@ -1118,8 +1141,8 @@ public class BpfCoordinatorTest { final String mobileIface = "rmnet_data0"; final Integer ethIfIndex = 100; final Integer mobileIfIndex = 101; - coordinator.addUpstreamNameToLookupTable(ethIfIndex, ethIface); - coordinator.addUpstreamNameToLookupTable(mobileIfIndex, mobileIface); + coordinator.maybeAddUpstreamToLookupTable(ethIfIndex, ethIface); + coordinator.maybeAddUpstreamToLookupTable(mobileIfIndex, mobileIface); final InOrder inOrder = inOrder(mNetd, mBpfDownstream6Map, mBpfUpstream6Map, mBpfLimitMap, mBpfStatsMap); @@ -1133,20 +1156,25 @@ public class BpfCoordinatorTest { // [1] Adding rules on the upstream Ethernet. // Note that the default data limit is applied after the first rule is added. + final Ipv6UpstreamRule ethernetUpstreamRule = buildTestUpstreamRule( + ethIfIndex, DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC); final Ipv6DownstreamRule ethernetRuleA = buildTestDownstreamRule( ethIfIndex, NEIGH_A, MAC_A); final Ipv6DownstreamRule ethernetRuleB = buildTestDownstreamRule( ethIfIndex, NEIGH_B, MAC_B); - coordinator.addIpv6DownstreamRule(mIpServer, ethernetRuleA); - verifyAddDownstreamRule(inOrder, ethernetRuleA); + coordinator.updateAllIpv6Rules(mIpServer, DOWNSTREAM_IFACE_PARAMS, ethIfIndex); verifyTetherOffloadSetInterfaceQuota(inOrder, ethIfIndex, QUOTA_UNLIMITED, true /* isInit */); - verifyStartUpstreamIpv6Forwarding(inOrder, DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC, ethIfIndex); + verifyAddUpstreamRule(inOrder, ethernetUpstreamRule); + coordinator.addIpv6DownstreamRule(mIpServer, ethernetRuleA); + verifyAddDownstreamRule(inOrder, ethernetRuleA); coordinator.addIpv6DownstreamRule(mIpServer, ethernetRuleB); verifyAddDownstreamRule(inOrder, ethernetRuleB); // [2] Update the existing rules from Ethernet to cellular. + final Ipv6UpstreamRule mobileUpstreamRule = buildTestUpstreamRule( + mobileIfIndex, DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC); final Ipv6DownstreamRule mobileRuleA = buildTestDownstreamRule( mobileIfIndex, NEIGH_A, MAC_A); final Ipv6DownstreamRule mobileRuleB = buildTestDownstreamRule( @@ -1156,25 +1184,24 @@ public class BpfCoordinatorTest { // Update the existing rules for upstream changes. The rules are removed and re-added one // by one for updating upstream interface index by #tetherOffloadRuleUpdate. - coordinator.tetherOffloadRuleUpdate(mIpServer, mobileIfIndex); + coordinator.updateAllIpv6Rules(mIpServer, DOWNSTREAM_IFACE_PARAMS, mobileIfIndex); verifyRemoveDownstreamRule(inOrder, ethernetRuleA); verifyRemoveDownstreamRule(inOrder, ethernetRuleB); - verifyStopUpstreamIpv6Forwarding(inOrder, DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC); + verifyRemoveUpstreamRule(inOrder, ethernetUpstreamRule); verifyTetherOffloadGetAndClearStats(inOrder, ethIfIndex); - verifyAddDownstreamRule(inOrder, mobileRuleA); verifyTetherOffloadSetInterfaceQuota(inOrder, mobileIfIndex, QUOTA_UNLIMITED, true /* isInit */); - verifyStartUpstreamIpv6Forwarding(inOrder, DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC, - mobileIfIndex); + verifyAddUpstreamRule(inOrder, mobileUpstreamRule); + verifyAddDownstreamRule(inOrder, mobileRuleA); verifyAddDownstreamRule(inOrder, mobileRuleB); // [3] Clear all rules for a given IpServer. updateStatsEntryForTetherOffloadGetAndClearStats( buildTestTetherStatsParcel(mobileIfIndex, 50, 60, 70, 80)); - coordinator.tetherOffloadRuleClear(mIpServer); + coordinator.clearAllIpv6Rules(mIpServer); verifyRemoveDownstreamRule(inOrder, mobileRuleA); verifyRemoveDownstreamRule(inOrder, mobileRuleB); - verifyStopUpstreamIpv6Forwarding(inOrder, DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC); + verifyRemoveUpstreamRule(inOrder, mobileUpstreamRule); verifyTetherOffloadGetAndClearStats(inOrder, mobileIfIndex); // [4] Force pushing stats update to verify that the last diff of stats is reported on all @@ -1204,7 +1231,7 @@ public class BpfCoordinatorTest { // The interface name lookup table can't be added. final String iface = "rmnet_data0"; final Integer ifIndex = 100; - coordinator.addUpstreamNameToLookupTable(ifIndex, iface); + coordinator.maybeAddUpstreamToLookupTable(ifIndex, iface); assertEquals(0, coordinator.getInterfaceNamesForTesting().size()); // The rule can't be added. @@ -1230,14 +1257,15 @@ public class BpfCoordinatorTest { assertEquals(1, rules.size()); // The rule can't be cleared. - coordinator.tetherOffloadRuleClear(mIpServer); + coordinator.clearAllIpv6Rules(mIpServer); verifyNeverRemoveDownstreamRule(); rules = coordinator.getIpv6DownstreamRulesForTesting().get(mIpServer); assertNotNull(rules); assertEquals(1, rules.size()); // The rule can't be updated. - coordinator.tetherOffloadRuleUpdate(mIpServer, rule.upstreamIfindex + 1 /* new */); + coordinator.updateAllIpv6Rules( + mIpServer, DOWNSTREAM_IFACE_PARAMS, rule.upstreamIfindex + 1 /* new */); verifyNeverRemoveDownstreamRule(); verifyNeverAddDownstreamRule(); rules = coordinator.getIpv6DownstreamRulesForTesting().get(mIpServer); @@ -1552,7 +1580,7 @@ public class BpfCoordinatorTest { // interface index. doReturn(upstreamInfo.interfaceParams).when(mDeps).getInterfaceParams( upstreamInfo.interfaceParams.name); - coordinator.addUpstreamNameToLookupTable(upstreamInfo.interfaceParams.index, + coordinator.maybeAddUpstreamToLookupTable(upstreamInfo.interfaceParams.index, upstreamInfo.interfaceParams.name); final LinkProperties lp = new LinkProperties(); @@ -1677,19 +1705,21 @@ public class BpfCoordinatorTest { public void testAddDevMapRule6() throws Exception { final BpfCoordinator coordinator = makeBpfCoordinator(); - coordinator.addUpstreamNameToLookupTable(UPSTREAM_IFINDEX, UPSTREAM_IFACE); - final Ipv6DownstreamRule ruleA = buildTestDownstreamRule(UPSTREAM_IFINDEX, NEIGH_A, MAC_A); - final Ipv6DownstreamRule ruleB = buildTestDownstreamRule(UPSTREAM_IFINDEX, NEIGH_B, MAC_B); - - coordinator.addIpv6DownstreamRule(mIpServer, ruleA); + coordinator.maybeAddUpstreamToLookupTable(UPSTREAM_IFINDEX, UPSTREAM_IFACE); + coordinator.updateAllIpv6Rules(mIpServer, DOWNSTREAM_IFACE_PARAMS, UPSTREAM_IFINDEX); 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.addIpv6DownstreamRule(mIpServer, ruleB); - verify(mBpfDevMap, never()).updateEntry(any(), any()); + // Adding the second downstream, only the second downstream ifindex is added to DevMap, + // the existing upstream ifindex won't be added again. + coordinator.updateAllIpv6Rules(mIpServer2, DOWNSTREAM_IFACE_PARAMS2, UPSTREAM_IFINDEX); + verify(mBpfDevMap).updateEntry(eq(new TetherDevKey(DOWNSTREAM_IFINDEX2)), + eq(new TetherDevValue(DOWNSTREAM_IFINDEX2))); + verify(mBpfDevMap, never()).updateEntry(eq(new TetherDevKey(UPSTREAM_IFINDEX)), + eq(new TetherDevValue(UPSTREAM_IFINDEX))); } @Test @@ -2153,7 +2183,8 @@ public class BpfCoordinatorTest { assertEquals("upstreamIfindex: 1001, downstreamIfindex: 2001, address: 2001:db8::1, " + "srcMac: 12:34:56:78:90:ab, dstMac: 00:00:00:00:00:0a", downstreamRule.toString()); - final Ipv6UpstreamRule upstreamRule = buildTestUpstreamRule(UPSTREAM_IFINDEX); + final Ipv6UpstreamRule upstreamRule = buildTestUpstreamRule( + UPSTREAM_IFINDEX, DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC); assertEquals("upstreamIfindex: 1001, downstreamIfindex: 2001, sourcePrefix: ::/64, " + "inDstMac: 12:34:56:78:90:ab, outSrcMac: 00:00:00:00:00:00, " + "outDstMac: 00:00:00:00:00:00", upstreamRule.toString()); @@ -2221,7 +2252,7 @@ public class BpfCoordinatorTest { 0L /* txPackets */, 0L /* txBytes */, 0L /* txErrors */)); // dumpDevmap - coordinator.addUpstreamNameToLookupTable(UPSTREAM_IFINDEX, UPSTREAM_IFACE); + coordinator.maybeAddUpstreamToLookupTable(UPSTREAM_IFINDEX, UPSTREAM_IFACE); mBpfDevMap.insertEntry( new TetherDevKey(UPSTREAM_IFINDEX), new TetherDevValue(UPSTREAM_IFINDEX)); @@ -2390,7 +2421,7 @@ public class BpfCoordinatorTest { // +-------+-------+-------+-------+-------+ // [1] Mobile IPv4 only - coordinator.addUpstreamNameToLookupTable(UPSTREAM_IFINDEX, UPSTREAM_IFACE); + coordinator.maybeAddUpstreamToLookupTable(UPSTREAM_IFINDEX, UPSTREAM_IFACE); doReturn(UPSTREAM_IFACE_PARAMS).when(mDeps).getInterfaceParams(UPSTREAM_IFACE); final UpstreamNetworkState mobileIPv4UpstreamState = new UpstreamNetworkState( buildUpstreamLinkProperties(UPSTREAM_IFACE, @@ -2442,7 +2473,7 @@ public class BpfCoordinatorTest { verifyIpv4Upstream(ipv4UpstreamIndices, interfaceNames); // Mobile IPv6 and xlat - // IpServer doesn't add xlat interface mapping via #addUpstreamNameToLookupTable on + // IpServer doesn't add xlat interface mapping via #maybeAddUpstreamToLookupTable on // S and T devices. coordinator.updateUpstreamNetworkState(mobile464xlatUpstreamState); // Upstream IPv4 address mapping is removed because xlat interface is not supported. @@ -2457,7 +2488,7 @@ public class BpfCoordinatorTest { // [6] Wifi IPv4 and IPv6 // Expect that upstream index map is cleared because ether ip is not supported. - coordinator.addUpstreamNameToLookupTable(UPSTREAM_IFINDEX2, UPSTREAM_IFACE2); + coordinator.maybeAddUpstreamToLookupTable(UPSTREAM_IFINDEX2, UPSTREAM_IFACE2); doReturn(UPSTREAM_IFACE_PARAMS2).when(mDeps).getInterfaceParams(UPSTREAM_IFACE2); final UpstreamNetworkState wifiDualStackUpstreamState = new UpstreamNetworkState( buildUpstreamLinkProperties(UPSTREAM_IFACE2,