Merge "Decouple Ipv6UpstreamRule with Ipv6DownstreamRule" into main

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<TetherStatsValue> tetherOffloadGetStats() {
-        if (!isInitialized()) return null;
-
         final SparseArray<TetherStatsValue> tetherStatsList = new SparseArray<TetherStatsValue>();
         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<Tether4Key, Tether4Value> 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) {

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(
+        updateIpv6ForwardingRules(mLastIPv6UpstreamIfindex, upstreamIfIndex, upstreamSupportsBpf);
-                mLastIPv6UpstreamIfindex, upstreamIfIndex, upstreamSupportsBpf, null);
         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
+    private int getInterfaceIndexForRule(int ifindex, boolean supportsBpf) {
-    // changes or if a neighbor event is received.
+        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) {
+            boolean upstreamSupportsBpf) {
-        // 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;
-        }
-
         // 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,
+        Ipv6DownstreamRule rule = new Ipv6DownstreamRule(
-                (Inet6Address) e.ip, mInterfaceParams.macAddr, dstMac);
+                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,
+            addOrRemoveIpv6Downstream(e);
-                    mUpstreamSupportsBpf, 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);

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<String> 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<IpServer, LinkedHashMap<Inet6Address, Ipv6DownstreamRule>>
             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<IpServer, ArraySet<Ipv6UpstreamRule>>
+            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
+     * Add IPv6 upstream rule. After adding the first rule on a given upstream, must add the
-     * limit on the given upstream.
+     * data limit on the given upstream.
-     * Note that this can be only called on handler thread.
      */
-    public void addIpv6DownstreamRule(
+    private void addIpv6UpstreamRule(
-            @NonNull final IpServer ipServer, @NonNull final Ipv6DownstreamRule rule) {
+            @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<Inet6Address,
-                    Ipv6DownstreamRule>());
-        }
-        LinkedHashMap<Inet6Address, Ipv6DownstreamRule> 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,
+        if (!mBpfCoordinatorShim.addIpv6UpstreamRule(rule)) {
-                    rule.downstreamIfindex, IPV6_ZERO_PREFIX64, rule.srcMac, NULL_MAC_ADDRESS,
+            return;
-                    NULL_MAC_ADDRESS);
-            if (!mBpfCoordinatorShim.addIpv6UpstreamRule(upstreamRule)) {
-                mLog.e("Failed to add upstream IPv6 forwarding rule: " + upstreamRule);
-            }
         }
 
-        // Must update the adding rule after calling #isAnyRuleOnUpstream because it needs to
+        ArraySet<Ipv6UpstreamRule> rules = mIpv6UpstreamRules.computeIfAbsent(
-        // check if it is about adding a first rule for a given upstream.
+                ipServer, k -> new ArraySet<Ipv6UpstreamRule>());
+        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<Ipv6UpstreamRule> 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<Inet6Address, Ipv6DownstreamRule> rules =
+                mIpv6DownstreamRules.computeIfAbsent(ipServer,
+                        k -> new LinkedHashMap<Inet6Address, Ipv6DownstreamRule>());
         rules.put(rule.address, rule);
     }
 
     /**
-     * Remove IPv6 downstream rule. After removing the last rule on a given upstream, must clear
+     * Remove IPv6 downstream rule.
-     * data limit, update the last tether stats and remove the tether stats in the BPF maps.
      * 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<Inet6Address, Ipv6DownstreamRule> rules = mIpv6DownstreamRules.get(ipServer);
         if (rules == null) return;
 
-        // Must remove rules before calling #isAnyRuleOnUpstream because it needs to check if
+        // If no rule is removed, return early. Avoid unnecessary work on a non-existent rule which
-        // the last rule is removed for a given upstream. If no rule is removed, return early.
+        // may have never been added or removed already.
-        // 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);
-            }
     }
 
-        // Do cleanup functionality if there is no more rule on the given upstream.
+    /**
-        maybeClearLimit(rule.upstreamIfindex);
+      * Clear all downstream rules for a given IpServer and return a copy of all removed rules.
+      */
+    @Nullable
+    private Collection<Ipv6DownstreamRule> clearIpv6DownstreamRules(
+            @NonNull final IpServer ipServer) {
+        final LinkedHashMap<Inet6Address, Ipv6DownstreamRule> downstreamRules =
+                mIpv6DownstreamRules.remove(ipServer);
+        if (downstreamRules == null) return null;
+
+        final Collection<Ipv6DownstreamRule> removedRules = downstreamRules.values();
+        for (final Ipv6DownstreamRule rule : removedRules) {
+            if (rule.upstreamIfindex == NO_UPSTREAM) continue;
+            mBpfCoordinatorShim.removeIpv6DownstreamRule(rule);
+        }
+        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<Inet6Address, Ipv6DownstreamRule> rules =
+        // Clear downstream rules first, because clearing upstream rules fetches the stats, and
-                mIpv6DownstreamRules.get(ipServer);
+        // fetching the stats requires that no rules be forwarding traffic to or from the upstream.
-        if (rules == null) return;
+        clearIpv6DownstreamRules(ipServer);
-
+        clearIpv6UpstreamRules(ipServer);
-        // Need to build a rule list because the rule map may be changed in the iteration.
-        for (final Ipv6DownstreamRule rule : new ArrayList<Ipv6DownstreamRule>(rules.values())) {
-            removeIpv6DownstreamRule(ipServer, rule);
-        }
     }
 
     /**
-     * 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<Inet6Address, Ipv6DownstreamRule> rules =
+        // Remove IPv6 downstream rules. Remove the old ones before adding the new rules, otherwise
-                mIpv6DownstreamRules.get(ipServer);
+        // we need to keep a copy of the old rules.
-        if (rules == null) return;
+        // 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<Ipv6DownstreamRule> deletedDownstreamRules =
+                clearIpv6DownstreamRules(ipServer);
 
-        // Need to build a rule list because the rule map may be changed in the iteration.
+        // Remove IPv6 upstream rules. Downstream rules must be removed first because
-        // First remove all the old rules, then add all the new rules. This is because the upstream
+        // BpfCoordinatorShimImpl#tetherOffloadGetAndClearStats will be called after the removal of
-        // forwarding code in addIpv6DownstreamRule cannot support rules on two upstreams at the
+        // the last upstream rule and it requires that no rules be forwarding traffic to or from
-        // same time. Deleting the rules first ensures that upstream forwarding is disabled on the
+        // that upstream.
-        // old upstream when the last rule is removed from it, and re-enabled on the new upstream
+        clearIpv6UpstreamRules(ipServer);
-        // when the first rule is added to it.
+
-        // TODO: Once the IPv6 client processing code has moved from IpServer to BpfCoordinator, do
+        // Add new upstream rules.
-        // something smarter.
+        if (newUpstreamIfindex != 0 && interfaceParams != null && interfaceParams.macAddr != null) {
-        final ArrayList<Ipv6DownstreamRule> rulesCopy = new ArrayList<>(rules.values());
+            addIpv6UpstreamRule(ipServer, new Ipv6UpstreamRule(
-        for (final Ipv6DownstreamRule rule : rulesCopy) {
+                    newUpstreamIfindex, interfaceParams.index, IPV6_ZERO_PREFIX64,
-            // Remove the old rule before adding the new one because the map uses the same key for
+                    interfaceParams.macAddr, NULL_MAC_ADDRESS, NULL_MAC_ADDRESS));
-            // 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);
         }
-        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<Inet6Address, Ipv6DownstreamRule> rules :
+        for (ArraySet<Ipv6UpstreamRule> rules : mIpv6UpstreamRules.values()) {
-                mIpv6DownstreamRules.values()) {
+            for (Ipv6UpstreamRule rule : rules) {
-            for (Ipv6DownstreamRule rule : rules.values()) {
                 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<Inet6Address, Ipv6DownstreamRule> rules :
+        for (ArraySet<Ipv6UpstreamRule> rules : mIpv6UpstreamRules.values()) {
-                mIpv6DownstreamRules.values()) {
+            for (Ipv6UpstreamRule rule : rules) {
-            for (Ipv6DownstreamRule rule : rules.values()) {
                 if (upstreamIfindex == rule.upstreamIfindex) return true;
             }
         }
         return false;
     }
 
-    private boolean isAnyRuleFromDownstreamToUpstream(int downstreamIfindex, int upstreamIfindex) {
-        for (LinkedHashMap<Inet6Address, Ipv6DownstreamRule> 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) {

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 <IFACE_NAME, UPSTREAM_IFACE>.
-        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 <IFACE_NAME, UPSTREAM_IFACE2>.
-        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 <IFACE_NAME, UPSTREAM_IFACE2> 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 <IFACE_NAME, UPSTREAM_IFACE2> 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);
+        verifyStartUpstreamIpv6Forwarding(null, UPSTREAM_IFINDEX);
-        recvNewNeigh(myIfindex, neighB, NUD_REACHABLE, macB);
         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();
     }
 

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<Integer, UpstreamInformation> 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 InOrder inOrder = inOrder(mNetd, mBpfUpstream6Map, mBpfDownstream6Map, mBpfLimitMap,
-        final Ipv6DownstreamRule rule = buildTestDownstreamRule(mobileIfIndex, NEIGH_A, MAC_A);
+                mBpfStatsMap);
-        coordinator.addIpv6DownstreamRule(mIpServer, rule);
+        final Ipv6UpstreamRule upstreamRule = buildTestUpstreamRule(
-        verifyAddDownstreamRule(inOrder, rule);
+                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);
+        coordinator.updateAllIpv6Rules(mIpServer, DOWNSTREAM_IFACE_PARAMS, 0);
-        verifyRemoveDownstreamRule(inOrder, rule);
+        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.maybeAddUpstreamToLookupTable(wlanIfIndex, wlanIface);
-        coordinator.addUpstreamNameToLookupTable(mobileIfIndex, mobileIface);
+        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) {
+    private static Ipv6UpstreamRule buildTestUpstreamRule(
-        return new Ipv6UpstreamRule(upstreamIfindex, DOWNSTREAM_IFINDEX,
+            int upstreamIfindex, int downstreamIfindex, @NonNull MacAddress inDstMac) {
-                IPV6_ZERO_PREFIX, DOWNSTREAM_MAC, MacAddress.ALL_ZEROS_ADDRESS,
+        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 Ipv6UpstreamRule rule = buildTestUpstreamRule(
-        final InOrder inOrder = inOrder(mNetd, mBpfDownstream6Map, mBpfLimitMap, mBpfStatsMap);
+                mobileIfIndex, DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC);
-        coordinator.addIpv6DownstreamRule(mIpServer, rule);
+        final InOrder inOrder = inOrder(mNetd, mBpfUpstream6Map, mBpfLimitMap, mBpfStatsMap);
-        verifyAddDownstreamRule(inOrder, rule);
+        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.
+        // Adding the first rule on current upstream immediately sends the quota to BPF.
-        final Ipv6DownstreamRule ruleA = buildTestDownstreamRule(mobileIfIndex, NEIGH_A, MAC_A);
+        final Ipv6UpstreamRule ruleA = buildTestUpstreamRule(
-        coordinator.addIpv6DownstreamRule(mIpServer, ruleA);
+                mobileIfIndex, DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC);
-        verifyAddDownstreamRule(inOrder, ruleA);
+        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.
+        // Adding the second rule on current upstream does not send the quota to BPF.
-        final Ipv6DownstreamRule ruleB = buildTestDownstreamRule(mobileIfIndex, NEIGH_B, MAC_B);
+        final Ipv6UpstreamRule ruleB = buildTestUpstreamRule(
-        coordinator.addIpv6DownstreamRule(mIpServer, ruleB);
+                mobileIfIndex, DOWNSTREAM_IFINDEX2, DOWNSTREAM_MAC2);
-        verifyAddDownstreamRule(inOrder, ruleB);
+        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.
+        // Removing the second rule on current upstream does not send the quota to BPF.
-        coordinator.removeIpv6DownstreamRule(mIpServer, ruleB);
+        coordinator.updateAllIpv6Rules(mIpServer2, DOWNSTREAM_IFACE_PARAMS2, 0);
-        verifyRemoveDownstreamRule(inOrder, ruleB);
+        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);
+        coordinator.updateAllIpv6Rules(mIpServer, DOWNSTREAM_IFACE_PARAMS, 0);
-        verifyRemoveDownstreamRule(inOrder, ruleA);
+        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.maybeAddUpstreamToLookupTable(ethIfIndex, ethIface);
-        coordinator.addUpstreamNameToLookupTable(mobileIfIndex, mobileIface);
+        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);
+        coordinator.updateAllIpv6Rules(mIpServer, DOWNSTREAM_IFACE_PARAMS, ethIfIndex);
-        verifyAddDownstreamRule(inOrder, ethernetRuleA);
         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,
+        verifyAddUpstreamRule(inOrder, mobileUpstreamRule);
-                mobileIfIndex);
+        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);
+        coordinator.maybeAddUpstreamToLookupTable(UPSTREAM_IFINDEX, UPSTREAM_IFACE);
-        final Ipv6DownstreamRule ruleA = buildTestDownstreamRule(UPSTREAM_IFINDEX, NEIGH_A, MAC_A);
+        coordinator.updateAllIpv6Rules(mIpServer, DOWNSTREAM_IFACE_PARAMS, UPSTREAM_IFINDEX);
-        final Ipv6DownstreamRule ruleB = buildTestDownstreamRule(UPSTREAM_IFINDEX, NEIGH_B, MAC_B);
-
-        coordinator.addIpv6DownstreamRule(mIpServer, ruleA);
         verify(mBpfDevMap).updateEntry(eq(new TetherDevKey(UPSTREAM_IFINDEX)),
                 eq(new TetherDevValue(UPSTREAM_IFINDEX)));
         verify(mBpfDevMap).updateEntry(eq(new TetherDevKey(DOWNSTREAM_IFINDEX)),
                 eq(new TetherDevValue(DOWNSTREAM_IFINDEX)));
         clearInvocations(mBpfDevMap);
 
-        coordinator.addIpv6DownstreamRule(mIpServer, ruleB);
+        // Adding the second downstream, only the second downstream ifindex is added to DevMap,
-        verify(mBpfDevMap, never()).updateEntry(any(), any());
+        // 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,