From dc6715c6f8814d67dd1dc00d630633b70718cbcb Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Tue, 9 Feb 2021 23:12:37 +0900 Subject: [PATCH 1/3] Set the limit whenever any IPv4 or IPv6 rule exists. Currently, BpfCoordinator only sets the data limit on a given upstream whenever the first IPv6 rule is created on that upstream, and clears it whenever the last rule is deleted on that upstream. It never does this when adding or removing IPv4 rules. This makes it impossible to offload traffic on IPv4-only networks. Fix this by setting the limit when IPv4 rules are created or deleted as well. Test: atest TetheringCoverageTests Manual tests as the follows Test {add, clear} limit with IPv6-only network [OK] Test {add} limit with IPv4-only upstream [OK] TODO: Test {clear} limit with IPv4-only network. blocked by aosp/1579873 because the IPv4 rules have never deleted. Change-Id: I5a29bdd18e564318759f617023163e23fb5a3ed0 --- .../apishim/api30/BpfCoordinatorShimImpl.java | 6 ++ .../apishim/api31/BpfCoordinatorShimImpl.java | 60 ++++++++++++-- .../apishim/common/BpfCoordinatorShim.java | 5 ++ Tethering/src/android/net/ip/IpServer.java | 34 ++++++-- .../tethering/BpfCoordinator.java | 83 ++++++++++++------- .../unit/src/android/net/ip/IpServerTest.java | 8 ++ .../networkstack/tethering/TetheringTest.java | 5 +- 7 files changed, 151 insertions(+), 50 deletions(-) diff --git a/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java b/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java index f27c831689..ce71f454de 100644 --- a/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java +++ b/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java @@ -170,6 +170,12 @@ public class BpfCoordinatorShimImpl return true; } + @Override + public boolean isAnyIpv4RuleOnUpstream(int ifIndex) { + /* no op */ + return false; + } + @Override public String toString() { return "Netd used"; diff --git a/Tethering/apishim/31/com/android/networkstack/tethering/apishim/api31/BpfCoordinatorShimImpl.java b/Tethering/apishim/31/com/android/networkstack/tethering/apishim/api31/BpfCoordinatorShimImpl.java index 4f7fe65a6a..2bb9fd7e7c 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 @@ -23,6 +23,7 @@ import android.net.util.SharedLog; import android.system.ErrnoException; import android.system.Os; import android.system.OsConstants; +import android.util.Log; import android.util.SparseArray; import androidx.annotation.NonNull; @@ -84,6 +85,20 @@ public class BpfCoordinatorShimImpl @Nullable private final BpfMap mBpfLimitMap; + // Tracking IPv4 rule count while any rule is using the given upstream interfaces. Used for + // reducing the BPF map iteration query. The count is increased or decreased when the rule is + // added or removed successfully on mBpfDownstream4Map. Counting the rules on downstream4 map + // is because tetherOffloadRuleRemove can't get upstream interface index from upstream key, + // unless pass upstream value which is not required for deleting map entry. The upstream + // interface index is the same in Upstream4Value.oif and Downstream4Key.iif. For now, it is + // okay to count on Downstream4Key. See BpfConntrackEventConsumer#accept. + // Note that except the constructor, any calls to mBpfDownstream4Map.clear() need to clear + // this counter as well. + // TODO: Count the rule on upstream if multi-upstream is supported and the + // packet needs to be sent and responded on different upstream interfaces. + // TODO: Add IPv6 rule count. + private final SparseArray mRule4CountOnUpstream = new SparseArray<>(); + public BpfCoordinatorShimImpl(@NonNull final Dependencies deps) { mLog = deps.getSharedLog().forSubComponent(TAG); @@ -324,18 +339,22 @@ public class BpfCoordinatorShimImpl if (!isInitialized()) return false; try { - // The last used time field of the value is updated by the bpf program. Adding the same - // map pair twice causes the unexpected refresh. Must be fixed before starting the - // conntrack timeout extension implementation. - // TODO: consider using insertEntry. if (downstream) { - mBpfDownstream4Map.updateEntry(key, value); + mBpfDownstream4Map.insertEntry(key, value); + + // Increase the rule count while a adding rule is using a given upstream interface. + final int upstreamIfindex = (int) key.iif; + int count = mRule4CountOnUpstream.get(upstreamIfindex, 0 /* default */); + mRule4CountOnUpstream.put(upstreamIfindex, ++count); } else { - mBpfUpstream4Map.updateEntry(key, value); + mBpfUpstream4Map.insertEntry(key, value); } } catch (ErrnoException e) { - mLog.e("Could not update entry: ", e); + mLog.e("Could not insert entry (" + key + ", " + value + "): " + e); return false; + } catch (IllegalStateException e) { + // Silent if the rule already exists. Note that the errno EEXIST was rethrown as + // IllegalStateException. See BpfMap#insertEntry. } return true; } @@ -346,7 +365,26 @@ public class BpfCoordinatorShimImpl try { if (downstream) { - mBpfDownstream4Map.deleteEntry(key); + if (!mBpfDownstream4Map.deleteEntry(key)) { + mLog.e("Could not delete entry (key: " + key + ")"); + return false; + } + + // Decrease the rule count while a deleting rule is not using a given upstream + // interface anymore. + final int upstreamIfindex = (int) key.iif; + Integer count = mRule4CountOnUpstream.get(upstreamIfindex); + if (count == null) { + Log.wtf(TAG, "Could not delete count for interface " + upstreamIfindex); + return false; + } + + if (--count == 0) { + // Remove the entry if the count decreases to zero. + mRule4CountOnUpstream.remove(upstreamIfindex); + } else { + mRule4CountOnUpstream.put(upstreamIfindex, count); + } } else { mBpfUpstream4Map.deleteEntry(key); } @@ -386,6 +424,12 @@ public class BpfCoordinatorShimImpl return true; } + @Override + public boolean isAnyIpv4RuleOnUpstream(int ifIndex) { + // No entry means no rule for the given interface because 0 has never been stored. + return mRule4CountOnUpstream.get(ifIndex) != null; + } + private String mapStatus(BpfMap m, String name) { return name + "{" + (m != null ? "OK" : "ERROR") + "}"; } diff --git a/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java b/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java index b7b4c47cc3..ccdf78c228 100644 --- a/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java +++ b/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java @@ -144,6 +144,11 @@ public abstract class BpfCoordinatorShim { */ public abstract boolean tetherOffloadRuleRemove(boolean downstream, @NonNull Tether4Key key); + /** + * Whether there is currently any IPv4 rule on the specified upstream. + */ + public abstract boolean isAnyIpv4RuleOnUpstream(int ifIndex); + /** * Attach BPF program. * diff --git a/Tethering/src/android/net/ip/IpServer.java b/Tethering/src/android/net/ip/IpServer.java index e5380e008d..da15fa8862 100644 --- a/Tethering/src/android/net/ip/IpServer.java +++ b/Tethering/src/android/net/ip/IpServer.java @@ -742,16 +742,14 @@ public class IpServer extends StateMachine { params.dnses.add(dnsServer); } } - - // Add upstream index to name mapping for the tether stats usage in the coordinator. - // Although this mapping could be added by both class Tethering and IpServer, adding - // mapping from IpServer guarantees that the mapping is added before the adding - // forwarding rules. That is because there are different state machines in both - // classes. It is hard to guarantee the link property update order between multiple - // state machines. - mBpfCoordinator.addUpstreamNameToLookupTable(upstreamIfIndex, upstreamIface); } + // Add upstream index to name mapping. See the comment of the interface mapping update in + // 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); + // If v6only is null, we pass in null to setRaParams(), which handles // deprecation of any existing RA data. @@ -1335,6 +1333,26 @@ public class IpServer extends StateMachine { mUpstreamIfaceSet = newUpstreamIfaceSet; for (String ifname : added) { + // Add upstream index to name mapping for the tether stats usage in the + // coordinator. Although this mapping could be added by both class + // Tethering and IpServer, adding mapping from IpServer guarantees that + // the mapping is added before adding forwarding rules. That is because + // there are different state machines in both classes. It is hard to + // guarantee the link property update order between multiple state machines. + // Note that both IPv4 and IPv6 interface may be added because + // Tethering::setUpstreamNetwork calls getTetheringInterfaces which merges + // IPv4 and IPv6 interface name (if any) into an InterfaceSet. The IPv6 + // capability may be updated later. In that case, IPv6 interface mapping is + // updated in updateUpstreamIPv6LinkProperties. + if (!ifname.startsWith("v4-")) { // ignore clat interfaces + final InterfaceParams upstreamIfaceParams = + mDeps.getInterfaceParams(ifname); + if (upstreamIfaceParams != null) { + mBpfCoordinator.addUpstreamNameToLookupTable( + upstreamIfaceParams.index, ifname); + } + } + mBpfCoordinator.maybeAttachProgram(mIfaceName, ifname); try { mNetd.tetherAddForward(mIfaceName, ifname); diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index 74eb87b6b1..6f54d106df 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java @@ -478,18 +478,7 @@ public class BpfCoordinator { LinkedHashMap rules = mIpv6ForwardingRules.get(ipServer); // When the first rule is added to an upstream, setup upstream forwarding and data limit. - final int upstreamIfindex = rule.upstreamIfindex; - if (!isAnyRuleOnUpstream(upstreamIfindex)) { - // If failed to set a data limit, probably should not use this upstream, because - // the upstream may not want to blow through the data limit that was told to apply. - // TODO: Perhaps stop the coordinator. - boolean success = updateDataLimit(upstreamIfindex); - if (!success) { - final String iface = mInterfaceNames.get(upstreamIfindex); - mLog.e("Setting data limit for " + iface + " failed."); - } - - } + maybeSetLimit(rule.upstreamIfindex); if (!isAnyRuleFromDownstreamToUpstream(rule.downstreamIfindex, rule.upstreamIfindex)) { final int downstream = rule.downstreamIfindex; @@ -544,22 +533,7 @@ public class BpfCoordinator { } // Do cleanup functionality if there is no more rule on the given upstream. - final int upstreamIfindex = rule.upstreamIfindex; - if (!isAnyRuleOnUpstream(upstreamIfindex)) { - final TetherStatsValue statsValue = - mBpfCoordinatorShim.tetherOffloadGetAndClearStats(upstreamIfindex); - if (statsValue == null) { - Log.wtf(TAG, "Fail to cleanup tether stats for upstream index " + upstreamIfindex); - return; - } - - SparseArray tetherStatsList = new SparseArray(); - tetherStatsList.put(upstreamIfindex, statsValue); - - // Update the last stats delta and delete the local cache for a given upstream. - updateQuotaAndStatsFromSnapshot(tetherStatsList); - mStats.remove(upstreamIfindex); - } + maybeClearLimit(rule.upstreamIfindex); } /** @@ -1113,6 +1087,8 @@ public class BpfCoordinator { // Support raw ip only. // TODO: add ether ip support. + // TODO: parse CTA_PROTOINFO of conntrack event in ConntrackMonitor. For TCP, only add rules + // while TCP status is established. private class BpfConntrackEventConsumer implements ConntrackEventConsumer { @NonNull private Tether4Key makeTetherUpstream4Key( @@ -1178,8 +1154,9 @@ public class BpfCoordinator { if (e.msgType == (NetlinkConstants.NFNL_SUBSYS_CTNETLINK << 8 | NetlinkConstants.IPCTNL_MSG_CT_DELETE)) { - mBpfCoordinatorShim.tetherOffloadRuleRemove(false, upstream4Key); - mBpfCoordinatorShim.tetherOffloadRuleRemove(true, downstream4Key); + mBpfCoordinatorShim.tetherOffloadRuleRemove(UPSTREAM, upstream4Key); + mBpfCoordinatorShim.tetherOffloadRuleRemove(DOWNSTREAM, downstream4Key); + maybeClearLimit(upstreamIndex); return; } @@ -1187,8 +1164,9 @@ public class BpfCoordinator { final Tether4Value downstream4Value = makeTetherDownstream4Value(e, tetherClient, upstreamIndex); - mBpfCoordinatorShim.tetherOffloadRuleAdd(false, upstream4Key, upstream4Value); - mBpfCoordinatorShim.tetherOffloadRuleAdd(true, downstream4Key, downstream4Value); + maybeSetLimit(upstreamIndex); + mBpfCoordinatorShim.tetherOffloadRuleAdd(UPSTREAM, upstream4Key, upstream4Value); + mBpfCoordinatorShim.tetherOffloadRuleAdd(DOWNSTREAM, downstream4Key, downstream4Value); } } @@ -1250,6 +1228,47 @@ public class BpfCoordinator { return sendDataLimitToBpfMap(ifIndex, quotaBytes); } + private void maybeSetLimit(int upstreamIfindex) { + if (isAnyRuleOnUpstream(upstreamIfindex) + || mBpfCoordinatorShim.isAnyIpv4RuleOnUpstream(upstreamIfindex)) { + return; + } + + // If failed to set a data limit, probably should not use this upstream, because + // the upstream may not want to blow through the data limit that was told to apply. + // TODO: Perhaps stop the coordinator. + boolean success = updateDataLimit(upstreamIfindex); + if (!success) { + final String iface = mInterfaceNames.get(upstreamIfindex); + mLog.e("Setting data limit for " + iface + " failed."); + } + } + + // TODO: This should be also called while IpServer wants to clear all IPv4 rules. Relying on + // conntrack event can't cover this case. + private void maybeClearLimit(int upstreamIfindex) { + if (isAnyRuleOnUpstream(upstreamIfindex) + || mBpfCoordinatorShim.isAnyIpv4RuleOnUpstream(upstreamIfindex)) { + return; + } + + final TetherStatsValue statsValue = + mBpfCoordinatorShim.tetherOffloadGetAndClearStats(upstreamIfindex); + if (statsValue == null) { + Log.wtf(TAG, "Fail to cleanup tether stats for upstream index " + upstreamIfindex); + return; + } + + SparseArray tetherStatsList = new SparseArray(); + tetherStatsList.put(upstreamIfindex, statsValue); + + // Update the last stats delta and delete the local cache for a given upstream. + updateQuotaAndStatsFromSnapshot(tetherStatsList); + mStats.remove(upstreamIfindex); + } + + // TODO: Rename to isAnyIpv6RuleOnUpstream and define an isAnyRuleOnUpstream method that called + // both isAnyIpv6RuleOnUpstream and mBpfCoordinatorShim.isAnyIpv4RuleOnUpstream. private boolean isAnyRuleOnUpstream(int upstreamIfindex) { for (LinkedHashMap rules : mIpv6ForwardingRules .values()) { diff --git a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java index adf1f671ca..1380bbf3c0 100644 --- a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java +++ b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java @@ -474,6 +474,8 @@ public class IpServerTest { InOrder inOrder = inOrder(mNetd, mBpfCoordinator); // Add the forwarding pair . + inOrder.verify(mBpfCoordinator).addUpstreamNameToLookupTable(UPSTREAM_IFINDEX, + UPSTREAM_IFACE); inOrder.verify(mBpfCoordinator).maybeAttachProgram(IFACE_NAME, UPSTREAM_IFACE); inOrder.verify(mNetd).tetherAddForward(IFACE_NAME, UPSTREAM_IFACE); inOrder.verify(mNetd).ipfwdAddInterfaceForward(IFACE_NAME, UPSTREAM_IFACE); @@ -494,6 +496,8 @@ public class IpServerTest { inOrder.verify(mNetd).tetherRemoveForward(IFACE_NAME, UPSTREAM_IFACE); // Add the forwarding pair . + inOrder.verify(mBpfCoordinator).addUpstreamNameToLookupTable(UPSTREAM_IFINDEX2, + UPSTREAM_IFACE2); inOrder.verify(mBpfCoordinator).maybeAttachProgram(IFACE_NAME, UPSTREAM_IFACE2); inOrder.verify(mNetd).tetherAddForward(IFACE_NAME, UPSTREAM_IFACE2); inOrder.verify(mNetd).ipfwdAddInterfaceForward(IFACE_NAME, UPSTREAM_IFACE2); @@ -517,6 +521,8 @@ public class IpServerTest { // Add the forwarding pair and expect that failed on // tetherAddForward. + inOrder.verify(mBpfCoordinator).addUpstreamNameToLookupTable(UPSTREAM_IFINDEX2, + UPSTREAM_IFACE2); inOrder.verify(mBpfCoordinator).maybeAttachProgram(IFACE_NAME, UPSTREAM_IFACE2); inOrder.verify(mNetd).tetherAddForward(IFACE_NAME, UPSTREAM_IFACE2); @@ -543,6 +549,8 @@ public class IpServerTest { // Add the forwarding pair and expect that failed on // ipfwdAddInterfaceForward. + inOrder.verify(mBpfCoordinator).addUpstreamNameToLookupTable(UPSTREAM_IFINDEX2, + UPSTREAM_IFACE2); inOrder.verify(mBpfCoordinator).maybeAttachProgram(IFACE_NAME, UPSTREAM_IFACE2); inOrder.verify(mNetd).tetherAddForward(IFACE_NAME, UPSTREAM_IFACE2); inOrder.verify(mNetd).ipfwdAddInterfaceForward(IFACE_NAME, UPSTREAM_IFACE2); diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java index 0611086448..1e23da13c3 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java @@ -332,13 +332,14 @@ public class TetheringTest { assertTrue("Non-mocked interface " + ifName, ifName.equals(TEST_USB_IFNAME) || ifName.equals(TEST_WLAN_IFNAME) + || ifName.equals(TEST_WIFI_IFNAME) || ifName.equals(TEST_MOBILE_IFNAME) || ifName.equals(TEST_P2P_IFNAME) || ifName.equals(TEST_NCM_IFNAME) || ifName.equals(TEST_ETH_IFNAME)); final String[] ifaces = new String[] { - TEST_USB_IFNAME, TEST_WLAN_IFNAME, TEST_MOBILE_IFNAME, TEST_P2P_IFNAME, - TEST_NCM_IFNAME, TEST_ETH_IFNAME}; + TEST_USB_IFNAME, TEST_WLAN_IFNAME, TEST_WIFI_IFNAME, TEST_MOBILE_IFNAME, + TEST_P2P_IFNAME, TEST_NCM_IFNAME, TEST_ETH_IFNAME}; return new InterfaceParams(ifName, ArrayUtils.indexOf(ifaces, ifName) + IFINDEX_OFFSET, MacAddress.ALL_ZEROS_ADDRESS); } From 9b8c60629f014bca6b3324cc806c47d4f42dd3a4 Mon Sep 17 00:00:00 2001 From: Hungming Chen Date: Thu, 18 Mar 2021 18:35:54 +0800 Subject: [PATCH 2/3] Populate the key destination mac address Required because XDP offload needs input interface mac address to be a part of the key. The mac address is used for checking packets which are received from exceped input interface. Test: atest TetheringCoverageTests, TetheringPrivilegedTests Change-Id: Ied159454b516c0d70efe0a85744d1bb606892f2d --- .../apishim/api30/BpfCoordinatorShimImpl.java | 6 +- .../apishim/api31/BpfCoordinatorShimImpl.java | 14 ++-- .../apishim/common/BpfCoordinatorShim.java | 12 ++- Tethering/bpf_progs/bpf_tethering.h | 17 ++-- .../tethering/BpfCoordinator.java | 68 +++++++++++++-- .../tethering/TetherDownstream6Key.java | 35 ++++---- .../tethering/TetherUpstream6Key.java | 14 +++- .../networkstack/tethering/BpfMapTest.java | 18 ++-- .../unit/src/android/net/ip/IpServerTest.java | 83 ++++++++++++------- .../tethering/BpfCoordinatorTest.java | 25 +++--- 10 files changed, 200 insertions(+), 92 deletions(-) diff --git a/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java b/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java index ce71f454de..e310fb62f1 100644 --- a/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java +++ b/Tethering/apishim/30/com/android/networkstack/tethering/apishim/api30/BpfCoordinatorShimImpl.java @@ -80,12 +80,14 @@ public class BpfCoordinatorShimImpl @Override public boolean startUpstreamIpv6Forwarding(int downstreamIfindex, int upstreamIfindex, - MacAddress srcMac, MacAddress dstMac, int mtu) { + @NonNull MacAddress inDstMac, @NonNull MacAddress outSrcMac, + @NonNull MacAddress outDstMac, int mtu) { return true; } @Override - public boolean stopUpstreamIpv6Forwarding(int downstreamIfindex, int upstreamIfindex) { + public boolean stopUpstreamIpv6Forwarding(int downstreamIfindex, + int upstreamIfindex, @NonNull MacAddress inDstMac) { return true; } 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 2bb9fd7e7c..d7ce1392b5 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 @@ -184,12 +184,13 @@ public class BpfCoordinatorShimImpl @Override public boolean startUpstreamIpv6Forwarding(int downstreamIfindex, int upstreamIfindex, - MacAddress srcMac, MacAddress dstMac, int mtu) { + @NonNull MacAddress inDstMac, @NonNull MacAddress outSrcMac, + @NonNull MacAddress outDstMac, int mtu) { if (!isInitialized()) return false; - final TetherUpstream6Key key = new TetherUpstream6Key(downstreamIfindex); - final Tether6Value value = new Tether6Value(upstreamIfindex, srcMac, - dstMac, OsConstants.ETH_P_IPV6, mtu); + final TetherUpstream6Key key = new TetherUpstream6Key(downstreamIfindex, inDstMac); + final Tether6Value value = new Tether6Value(upstreamIfindex, outSrcMac, + outDstMac, OsConstants.ETH_P_IPV6, mtu); try { mBpfUpstream6Map.insertEntry(key, value); } catch (ErrnoException | IllegalStateException e) { @@ -200,10 +201,11 @@ public class BpfCoordinatorShimImpl } @Override - public boolean stopUpstreamIpv6Forwarding(int downstreamIfindex, int upstreamIfindex) { + public boolean stopUpstreamIpv6Forwarding(int downstreamIfindex, int upstreamIfindex, + @NonNull MacAddress inDstMac) { if (!isInitialized()) return false; - final TetherUpstream6Key key = new TetherUpstream6Key(downstreamIfindex); + final TetherUpstream6Key key = new TetherUpstream6Key(downstreamIfindex, inDstMac); try { mBpfUpstream6Map.deleteEntry(key); } catch (ErrnoException e) { diff --git a/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java b/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java index ccdf78c228..79a628b403 100644 --- a/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java +++ b/Tethering/apishim/common/com/android/networkstack/tethering/apishim/common/BpfCoordinatorShim.java @@ -78,21 +78,25 @@ public abstract class BpfCoordinatorShim { * @param downstreamIfindex the downstream interface index * @param upstreamIfindex the upstream interface index - * @param srcMac the source MAC address to use for packets - * @oaram dstMac the destination MAC address to use for packets + * @param inDstMac the destination MAC address to use for XDP + * @param outSrcMac the source MAC address to use for packets + * @param outDstMac the destination MAC address to use for packets * @return true if operation succeeded or was a no-op, false otherwise */ public abstract boolean startUpstreamIpv6Forwarding(int downstreamIfindex, int upstreamIfindex, - MacAddress srcMac, MacAddress dstMac, int mtu); + @NonNull MacAddress inDstMac, @NonNull MacAddress outSrcMac, + @NonNull MacAddress outDstMac, int mtu); /** * Stops IPv6 forwarding between the specified interfaces. * @param downstreamIfindex the downstream interface index * @param upstreamIfindex the upstream interface index + * @param inDstMac the destination MAC address to use for XDP * @return true if operation succeeded or was a no-op, false otherwise */ - public abstract boolean stopUpstreamIpv6Forwarding(int downstreamIfindex, int upstreamIfindex); + public abstract boolean stopUpstreamIpv6Forwarding(int downstreamIfindex, + int upstreamIfindex, @NonNull MacAddress inDstMac); /** * Return BPF tethering offload statistics. diff --git a/Tethering/bpf_progs/bpf_tethering.h b/Tethering/bpf_progs/bpf_tethering.h index efda228479..5fdf8cd88e 100644 --- a/Tethering/bpf_progs/bpf_tethering.h +++ b/Tethering/bpf_progs/bpf_tethering.h @@ -107,11 +107,12 @@ typedef uint64_t TetherLimitValue; // in bytes // Ethernet) have 6-byte MAC addresses. typedef struct { - uint32_t iif; // The input interface index - // TODO: extend this to include dstMac - struct in6_addr neigh6; // The destination IPv6 address + uint32_t iif; // The input interface index + uint8_t dstMac[ETH_ALEN]; // destination ethernet mac address (zeroed iff rawip ingress) + uint8_t zero[2]; // zero pad for 8 byte alignment + struct in6_addr neigh6; // The destination IPv6 address } TetherDownstream6Key; -STRUCT_SIZE(TetherDownstream6Key, 4 + 16); // 20 +STRUCT_SIZE(TetherDownstream6Key, 4 + 6 + 2 + 16); // 28 typedef struct { uint32_t oif; // The output interface to redirect to @@ -154,10 +155,12 @@ STRUCT_SIZE(TetherDownstream64Value, 4 + 14 + 2 + 4 + 4 + 2 + 2 + 8); // 40 #define TETHER_UPSTREAM6_MAP_PATH BPF_PATH_TETHER "map_offload_tether_upstream6_map" typedef struct { - uint32_t iif; // The input interface index - // TODO: extend this to include dstMac and src ip /64 subnet + uint32_t iif; // The input interface index + uint8_t dstMac[ETH_ALEN]; // destination ethernet mac address (zeroed iff rawip ingress) + uint8_t zero[2]; // zero pad for 8 byte alignment + // TODO: extend this to include src ip /64 subnet } TetherUpstream6Key; -STRUCT_SIZE(TetherUpstream6Key, 4); +STRUCT_SIZE(TetherUpstream6Key, 12); #define TETHER_DOWNSTREAM4_TC_PROG_RAWIP_NAME "prog_offload_schedcls_tether_downstream4_rawip" #define TETHER_DOWNSTREAM4_TC_PROG_ETHER_NAME "prog_offload_schedcls_tether_downstream4_ether" diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index 6f54d106df..27fb581569 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java @@ -485,7 +485,7 @@ public class BpfCoordinator { final int upstream = 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. - if (!mBpfCoordinatorShim.startUpstreamIpv6Forwarding(downstream, upstream, + if (!mBpfCoordinatorShim.startUpstreamIpv6Forwarding(downstream, upstream, rule.srcMac, NULL_MAC_ADDRESS, NULL_MAC_ADDRESS, NetworkStackConstants.ETHER_MTU)) { mLog.e("Failed to enable upstream IPv6 forwarding from " + mInterfaceNames.get(downstream) + " to " + mInterfaceNames.get(upstream)); @@ -526,7 +526,8 @@ public class BpfCoordinator { if (!isAnyRuleFromDownstreamToUpstream(rule.downstreamIfindex, rule.upstreamIfindex)) { final int downstream = rule.downstreamIfindex; final int upstream = rule.upstreamIfindex; - if (!mBpfCoordinatorShim.stopUpstreamIpv6Forwarding(downstream, upstream)) { + if (!mBpfCoordinatorShim.stopUpstreamIpv6Forwarding(downstream, upstream, + rule.srcMac)) { mLog.e("Failed to disable upstream IPv6 forwarding from " + mInterfaceNames.get(downstream) + " to " + mInterfaceNames.get(upstream)); } @@ -795,8 +796,8 @@ public class BpfCoordinator { } private String ipv6UpstreamRuletoString(TetherUpstream6Key key, Tether6Value value) { - return String.format("%d(%s) -> %d(%s) %04x %s %s", - key.iif, getIfName(key.iif), value.oif, getIfName(value.oif), + return String.format("%d(%s) %s -> %d(%s) %04x %s %s", + key.iif, getIfName(key.iif), key.dstMac, value.oif, getIfName(value.oif), value.ethProto, value.ethSrcMac, value.ethDstMac); } @@ -885,6 +886,62 @@ public class BpfCoordinator { /** IPv6 forwarding rule class. */ public static class Ipv6ForwardingRule { + // The upstream6 and downstream6 rules are built as the following tables. Only raw ip + // upstream interface is supported. + // TODO: support ether ip upstream interface. + // + // NAT network topology: + // + // public network (rawip) private network + // | UE | + // +------------+ V +------------+------------+ V +------------+ + // | Sever +---------+ Upstream | Downstream +---------+ Client | + // +------------+ +------------+------------+ +------------+ + // + // upstream6 key and value: + // + // +------+-------------+ + // | TetherUpstream6Key | + // +------+------+------+ + // |field |iif |dstMac| + // | | | | + // +------+------+------+ + // |value |downst|downst| + // | |ream |ream | + // +------+------+------+ + // + // +------+----------------------------------+ + // | |Tether6Value | + // +------+------+------+------+------+------+ + // |field |oif |ethDst|ethSrc|ethPro|pmtu | + // | | |mac |mac |to | | + // +------+------+------+------+------+------+ + // |value |upstre|-- |-- |ETH_P_|1500 | + // | |am | | |IP | | + // +------+------+------+------+------+------+ + // + // downstream6 key and value: + // + // +------+--------------------+ + // | |TetherDownstream6Key| + // +------+------+------+------+ + // |field |iif |dstMac|neigh6| + // | | | | | + // +------+------+------+------+ + // |value |upstre|-- |client| + // | |am | | | + // +------+------+------+------+ + // + // +------+----------------------------------+ + // | |Tether6Value | + // +------+------+------+------+------+------+ + // |field |oif |ethDst|ethSrc|ethPro|pmtu | + // | | |mac |mac |to | | + // +------+------+------+------+------+------+ + // |value |downst|client|downst|ETH_P_|1500 | + // | |ream | |ream |IP | | + // +------+------+------+------+------+------+ + // public final int upstreamIfindex; public final int downstreamIfindex; @@ -934,7 +991,8 @@ public class BpfCoordinator { */ @NonNull public TetherDownstream6Key makeTetherDownstream6Key() { - return new TetherDownstream6Key(upstreamIfindex, address.getAddress()); + return new TetherDownstream6Key(upstreamIfindex, NULL_MAC_ADDRESS, + address.getAddress()); } /** diff --git a/Tethering/src/com/android/networkstack/tethering/TetherDownstream6Key.java b/Tethering/src/com/android/networkstack/tethering/TetherDownstream6Key.java index 3860cba745..a08ad4ab03 100644 --- a/Tethering/src/com/android/networkstack/tethering/TetherDownstream6Key.java +++ b/Tethering/src/com/android/networkstack/tethering/TetherDownstream6Key.java @@ -16,6 +16,10 @@ package com.android.networkstack.tethering; +import android.net.MacAddress; + +import androidx.annotation.NonNull; + import com.android.net.module.util.Struct; import com.android.net.module.util.Struct.Field; import com.android.net.module.util.Struct.Type; @@ -24,16 +28,23 @@ import java.net.Inet6Address; import java.net.InetAddress; import java.net.UnknownHostException; import java.util.Arrays; +import java.util.Objects; /** The key of BpfMap which is used for bpf offload. */ public class TetherDownstream6Key extends Struct { @Field(order = 0, type = Type.U32) public final long iif; // The input interface index. - @Field(order = 1, type = Type.ByteArray, arraysize = 16) + @Field(order = 1, type = Type.EUI48, padding = 2) + public final MacAddress dstMac; // Destination ethernet mac address (zeroed iff rawip ingress). + + @Field(order = 2, type = Type.ByteArray, arraysize = 16) public final byte[] neigh6; // The destination IPv6 address. - public TetherDownstream6Key(final long iif, final byte[] neigh6) { + public TetherDownstream6Key(final long iif, @NonNull final MacAddress dstMac, + final byte[] neigh6) { + Objects.requireNonNull(dstMac); + try { final Inet6Address unused = (Inet6Address) InetAddress.getByAddress(neigh6); } catch (ClassCastException | UnknownHostException e) { @@ -41,29 +52,15 @@ public class TetherDownstream6Key extends Struct { + Arrays.toString(neigh6)); } this.iif = iif; + this.dstMac = dstMac; this.neigh6 = neigh6; } - @Override - public boolean equals(Object obj) { - if (this == obj) return true; - - if (!(obj instanceof TetherDownstream6Key)) return false; - - final TetherDownstream6Key that = (TetherDownstream6Key) obj; - - return iif == that.iif && Arrays.equals(neigh6, that.neigh6); - } - - @Override - public int hashCode() { - return Long.hashCode(iif) ^ Arrays.hashCode(neigh6); - } - @Override public String toString() { try { - return String.format("iif: %d, neigh: %s", iif, Inet6Address.getByAddress(neigh6)); + return String.format("iif: %d, dstMac: %s, neigh: %s", iif, dstMac, + Inet6Address.getByAddress(neigh6)); } catch (UnknownHostException e) { // Should not happen because construtor already verify neigh6. throw new IllegalStateException("Invalid TetherDownstream6Key"); diff --git a/Tethering/src/com/android/networkstack/tethering/TetherUpstream6Key.java b/Tethering/src/com/android/networkstack/tethering/TetherUpstream6Key.java index c736f2a883..5893885a76 100644 --- a/Tethering/src/com/android/networkstack/tethering/TetherUpstream6Key.java +++ b/Tethering/src/com/android/networkstack/tethering/TetherUpstream6Key.java @@ -16,14 +16,26 @@ package com.android.networkstack.tethering; +import android.net.MacAddress; + +import androidx.annotation.NonNull; + import com.android.net.module.util.Struct; +import java.util.Objects; + /** Key type for upstream IPv6 forwarding map. */ public class TetherUpstream6Key extends Struct { @Field(order = 0, type = Type.S32) public final int iif; // The input interface index. - public TetherUpstream6Key(int iif) { + @Field(order = 1, type = Type.EUI48, padding = 2) + public final MacAddress dstMac; // Destination ethernet mac address (zeroed iff rawip ingress). + + public TetherUpstream6Key(int iif, @NonNull final MacAddress dstMac) { + Objects.requireNonNull(dstMac); + this.iif = iif; + this.dstMac = dstMac; } } diff --git a/Tethering/tests/privileged/src/com/android/networkstack/tethering/BpfMapTest.java b/Tethering/tests/privileged/src/com/android/networkstack/tethering/BpfMapTest.java index 62302c37c8..2e79739c1f 100644 --- a/Tethering/tests/privileged/src/com/android/networkstack/tethering/BpfMapTest.java +++ b/Tethering/tests/privileged/src/com/android/networkstack/tethering/BpfMapTest.java @@ -65,13 +65,13 @@ public final class BpfMapTest { @Before public void setUp() throws Exception { mTestData = new ArrayMap<>(); - mTestData.put(createTetherDownstream6Key(101, "2001:db8::1"), + mTestData.put(createTetherDownstream6Key(101, "00:00:00:00:00:aa", "2001:db8::1"), createTether6Value(11, "00:00:00:00:00:0a", "11:11:11:00:00:0b", ETH_P_IPV6, 1280)); - mTestData.put(createTetherDownstream6Key(102, "2001:db8::2"), + mTestData.put(createTetherDownstream6Key(102, "00:00:00:00:00:bb", "2001:db8::2"), createTether6Value(22, "00:00:00:00:00:0c", "22:22:22:00:00:0d", ETH_P_IPV6, 1400)); - mTestData.put(createTetherDownstream6Key(103, "2001:db8::3"), + mTestData.put(createTetherDownstream6Key(103, "00:00:00:00:00:cc", "2001:db8::3"), createTether6Value(33, "00:00:00:00:00:0e", "33:33:33:00:00:0f", ETH_P_IPV6, 1500)); @@ -94,11 +94,12 @@ public final class BpfMapTest { assertTrue(mTestMap.isEmpty()); } - private TetherDownstream6Key createTetherDownstream6Key(long iif, String address) - throws Exception { + private TetherDownstream6Key createTetherDownstream6Key(long iif, String mac, + String address) throws Exception { + final MacAddress dstMac = MacAddress.fromString(mac); final InetAddress ipv6Address = InetAddress.getByName(address); - return new TetherDownstream6Key(iif, ipv6Address.getAddress()); + return new TetherDownstream6Key(iif, dstMac, ipv6Address.getAddress()); } private Tether6Value createTether6Value(int oif, String src, String dst, int proto, int pmtu) { @@ -164,7 +165,7 @@ public final class BpfMapTest { public void testGetNextKey() throws Exception { // [1] If the passed-in key is not found on empty map, return null. final TetherDownstream6Key nonexistentKey = - createTetherDownstream6Key(1234, "2001:db8::10"); + createTetherDownstream6Key(1234, "00:00:00:00:00:01", "2001:db8::10"); assertNull(mTestMap.getNextKey(nonexistentKey)); // [2] If the passed-in key is null on empty map, throw NullPointerException. @@ -344,7 +345,8 @@ public final class BpfMapTest { // Build test data for TEST_MAP_SIZE + 1 entries. for (int i = 1; i <= TEST_MAP_SIZE + 1; i++) { - testData.put(createTetherDownstream6Key(i, "2001:db8::1"), + testData.put( + createTetherDownstream6Key(i, "00:00:00:00:00:01", "2001:db8::1"), createTether6Value(100, "de:ad:be:ef:00:01", "de:ad:be:ef:00:02", ETH_P_IPV6, 1500)); } diff --git a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java index 1380bbf3c0..435cab5140 100644 --- a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java +++ b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java @@ -838,8 +838,8 @@ public class IpServerTest { @NonNull private static TetherDownstream6Key makeDownstream6Key(int upstreamIfindex, - @NonNull final InetAddress dst) { - return new TetherDownstream6Key(upstreamIfindex, dst.getAddress()); + @NonNull MacAddress upstreamMac, @NonNull final InetAddress dst) { + return new TetherDownstream6Key(upstreamIfindex, upstreamMac, dst.getAddress()); } @NonNull @@ -857,10 +857,12 @@ public class IpServerTest { } private void verifyTetherOffloadRuleAdd(@Nullable InOrder inOrder, int upstreamIfindex, - @NonNull final InetAddress dst, @NonNull final MacAddress dstMac) throws Exception { + @NonNull MacAddress upstreamMac, @NonNull final InetAddress dst, + @NonNull final MacAddress dstMac) throws Exception { if (mBpfDeps.isAtLeastS()) { verifyWithOrder(inOrder, mBpfDownstream6Map).updateEntry( - makeDownstream6Key(upstreamIfindex, dst), makeDownstream6Value(dstMac)); + makeDownstream6Key(upstreamIfindex, upstreamMac, dst), + makeDownstream6Value(dstMac)); } else { verifyWithOrder(inOrder, mNetd).tetherOffloadRuleAdd(matches(upstreamIfindex, dst, dstMac)); @@ -868,10 +870,11 @@ public class IpServerTest { } private void verifyNeverTetherOffloadRuleAdd(int upstreamIfindex, - @NonNull final InetAddress dst, @NonNull final MacAddress dstMac) throws Exception { + @NonNull MacAddress upstreamMac, @NonNull final InetAddress dst, + @NonNull final MacAddress dstMac) throws Exception { if (mBpfDeps.isAtLeastS()) { verify(mBpfDownstream6Map, never()).updateEntry( - makeDownstream6Key(upstreamIfindex, dst), + makeDownstream6Key(upstreamIfindex, upstreamMac, dst), makeDownstream6Value(dstMac)); } else { verify(mNetd, never()).tetherOffloadRuleAdd(matches(upstreamIfindex, dst, dstMac)); @@ -887,10 +890,11 @@ public class IpServerTest { } private void verifyTetherOffloadRuleRemove(@Nullable InOrder inOrder, int upstreamIfindex, - @NonNull final InetAddress dst, @NonNull final MacAddress dstMac) throws Exception { + @NonNull MacAddress upstreamMac, @NonNull final InetAddress dst, + @NonNull final MacAddress dstMac) throws Exception { if (mBpfDeps.isAtLeastS()) { verifyWithOrder(inOrder, mBpfDownstream6Map).deleteEntry(makeDownstream6Key( - upstreamIfindex, dst)); + upstreamIfindex, upstreamMac, dst)); } else { // |dstMac| is not required for deleting rules. Used bacause tetherOffloadRuleRemove // uses a whole rule to be a argument. @@ -911,7 +915,8 @@ public class IpServerTest { private void verifyStartUpstreamIpv6Forwarding(@Nullable InOrder inOrder, int upstreamIfindex) throws Exception { if (!mBpfDeps.isAtLeastS()) return; - final TetherUpstream6Key key = new TetherUpstream6Key(TEST_IFACE_PARAMS.index); + final TetherUpstream6Key key = new TetherUpstream6Key(TEST_IFACE_PARAMS.index, + TEST_IFACE_PARAMS.macAddr); final Tether6Value value = new Tether6Value(upstreamIfindex, MacAddress.ALL_ZEROS_ADDRESS, MacAddress.ALL_ZEROS_ADDRESS, ETH_P_IPV6, NetworkStackConstants.ETHER_MTU); @@ -921,7 +926,8 @@ public class IpServerTest { private void verifyStopUpstreamIpv6Forwarding(@Nullable InOrder inOrder) throws Exception { if (!mBpfDeps.isAtLeastS()) return; - final TetherUpstream6Key key = new TetherUpstream6Key(TEST_IFACE_PARAMS.index); + final TetherUpstream6Key key = new TetherUpstream6Key(TEST_IFACE_PARAMS.index, + TEST_IFACE_PARAMS.macAddr); verifyWithOrder(inOrder, mBpfUpstream6Map).deleteEntry(key); } @@ -991,14 +997,16 @@ public class IpServerTest { recvNewNeigh(myIfindex, neighA, NUD_REACHABLE, macA); verify(mBpfCoordinator).tetherOffloadRuleAdd( mIpServer, makeForwardingRule(UPSTREAM_IFINDEX, neighA, macA)); - verifyTetherOffloadRuleAdd(null, 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); verify(mBpfCoordinator).tetherOffloadRuleAdd( mIpServer, makeForwardingRule(UPSTREAM_IFINDEX, neighB, macB)); - verifyTetherOffloadRuleAdd(null, UPSTREAM_IFINDEX, neighB, macB); + verifyTetherOffloadRuleAdd(null, + UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighB, macB); verifyNoUpstreamIpv6ForwardingChange(null); resetNetdBpfMapAndCoordinator(); @@ -1013,7 +1021,8 @@ public class IpServerTest { recvNewNeigh(myIfindex, neighA, NUD_FAILED, null); verify(mBpfCoordinator).tetherOffloadRuleRemove( mIpServer, makeForwardingRule(UPSTREAM_IFINDEX, neighA, macNull)); - verifyTetherOffloadRuleRemove(null, UPSTREAM_IFINDEX, neighA, macNull); + verifyTetherOffloadRuleRemove(null, + UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighA, macNull); verifyNoUpstreamIpv6ForwardingChange(null); resetNetdBpfMapAndCoordinator(); @@ -1021,7 +1030,8 @@ public class IpServerTest { recvDelNeigh(myIfindex, neighB, NUD_STALE, macB); verify(mBpfCoordinator).tetherOffloadRuleRemove( mIpServer, makeForwardingRule(UPSTREAM_IFINDEX, neighB, macNull)); - verifyTetherOffloadRuleRemove(null, UPSTREAM_IFINDEX, neighB, macNull); + verifyTetherOffloadRuleRemove(null, + UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighB, macNull); verifyStopUpstreamIpv6Forwarding(null); resetNetdBpfMapAndCoordinator(); @@ -1036,12 +1046,16 @@ public class IpServerTest { lp.setInterfaceName(UPSTREAM_IFACE2); dispatchTetherConnectionChanged(UPSTREAM_IFACE2, lp, -1); verify(mBpfCoordinator).tetherOffloadRuleUpdate(mIpServer, UPSTREAM_IFINDEX2); - verifyTetherOffloadRuleRemove(inOrder, UPSTREAM_IFINDEX, neighA, macA); - verifyTetherOffloadRuleRemove(inOrder, UPSTREAM_IFINDEX, neighB, macB); + 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, neighA, macA); + verifyTetherOffloadRuleAdd(inOrder, + UPSTREAM_IFINDEX2, UPSTREAM_IFACE_PARAMS2.macAddr, neighA, macA); verifyStartUpstreamIpv6Forwarding(inOrder, UPSTREAM_IFINDEX2); - verifyTetherOffloadRuleAdd(inOrder, UPSTREAM_IFINDEX2, neighB, macB); + verifyTetherOffloadRuleAdd(inOrder, + UPSTREAM_IFINDEX2, UPSTREAM_IFACE_PARAMS2.macAddr, neighB, macB); verifyNoUpstreamIpv6ForwardingChange(inOrder); resetNetdBpfMapAndCoordinator(); @@ -1052,8 +1066,10 @@ public class IpServerTest { // - processMessage CMD_IPV6_TETHER_UPDATE for the IPv6 upstream is lost. // See dispatchTetherConnectionChanged. verify(mBpfCoordinator, times(2)).tetherOffloadRuleClear(mIpServer); - verifyTetherOffloadRuleRemove(null, UPSTREAM_IFINDEX2, neighA, macA); - verifyTetherOffloadRuleRemove(null, UPSTREAM_IFINDEX2, neighB, macB); + verifyTetherOffloadRuleRemove(null, + UPSTREAM_IFINDEX2, UPSTREAM_IFACE_PARAMS2.macAddr, neighA, macA); + verifyTetherOffloadRuleRemove(null, + UPSTREAM_IFINDEX2, UPSTREAM_IFACE_PARAMS2.macAddr, neighB, macB); verifyStopUpstreamIpv6Forwarding(inOrder); resetNetdBpfMapAndCoordinator(); @@ -1072,17 +1088,20 @@ public class IpServerTest { recvNewNeigh(myIfindex, neighB, NUD_REACHABLE, macB); verify(mBpfCoordinator).tetherOffloadRuleAdd( mIpServer, makeForwardingRule(UPSTREAM_IFINDEX, neighB, macB)); - verifyTetherOffloadRuleAdd(null, UPSTREAM_IFINDEX, neighB, macB); + verifyTetherOffloadRuleAdd(null, + UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighB, macB); verifyStartUpstreamIpv6Forwarding(null, UPSTREAM_IFINDEX); verify(mBpfCoordinator, never()).tetherOffloadRuleAdd( mIpServer, makeForwardingRule(UPSTREAM_IFINDEX, neighA, macA)); - verifyNeverTetherOffloadRuleAdd(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); - verifyTetherOffloadRuleRemove(null, UPSTREAM_IFINDEX, neighB, macB); + verifyTetherOffloadRuleRemove(null, + UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighB, macB); verifyStopUpstreamIpv6Forwarding(null); // When the interface goes down, rules are removed. @@ -1092,18 +1111,22 @@ public class IpServerTest { recvNewNeigh(myIfindex, neighB, NUD_REACHABLE, macB); verify(mBpfCoordinator).tetherOffloadRuleAdd( mIpServer, makeForwardingRule(UPSTREAM_IFINDEX, neighA, macA)); - verifyTetherOffloadRuleAdd(null, UPSTREAM_IFINDEX, neighA, macA); + verifyTetherOffloadRuleAdd(null, + UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighA, macA); verifyStartUpstreamIpv6Forwarding(null, UPSTREAM_IFINDEX); verify(mBpfCoordinator).tetherOffloadRuleAdd( mIpServer, makeForwardingRule(UPSTREAM_IFINDEX, neighB, macB)); - verifyTetherOffloadRuleAdd(null, UPSTREAM_IFINDEX, neighB, macB); + verifyTetherOffloadRuleAdd(null, + UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neighB, macB); resetNetdBpfMapAndCoordinator(); mIpServer.stop(); mLooper.dispatchAll(); verify(mBpfCoordinator).tetherOffloadRuleClear(mIpServer); - verifyTetherOffloadRuleRemove(null, UPSTREAM_IFINDEX, neighA, macA); - verifyTetherOffloadRuleRemove(null, UPSTREAM_IFINDEX, neighB, macB); + 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(); @@ -1132,14 +1155,16 @@ public class IpServerTest { recvNewNeigh(myIfindex, neigh, NUD_REACHABLE, macA); verify(mBpfCoordinator).tetherOffloadRuleAdd( mIpServer, makeForwardingRule(UPSTREAM_IFINDEX, neigh, macA)); - verifyTetherOffloadRuleAdd(null, 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); verify(mBpfCoordinator).tetherOffloadRuleRemove( mIpServer, makeForwardingRule(UPSTREAM_IFINDEX, neigh, macNull)); - verifyTetherOffloadRuleRemove(null, UPSTREAM_IFINDEX, neigh, macNull); + verifyTetherOffloadRuleRemove(null, + UPSTREAM_IFINDEX, UPSTREAM_IFACE_PARAMS.macAddr, neigh, macNull); verifyStopUpstreamIpv6Forwarding(null); 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 293d0df67e..27fdac5731 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java @@ -109,7 +109,7 @@ public class BpfCoordinatorTest { public final DevSdkIgnoreRule mIgnoreRule = new DevSdkIgnoreRule(); private static final int DOWNSTREAM_IFINDEX = 10; - private static final MacAddress DOWNSTREAM_MAC = MacAddress.ALL_ZEROS_ADDRESS; + private static final MacAddress DOWNSTREAM_MAC = MacAddress.fromString("12:34:56:78:90:ab"); private static final InetAddress NEIGH_A = InetAddresses.parseNumericAddress("2001:db8::1"); private static final InetAddress NEIGH_B = InetAddresses.parseNumericAddress("2001:db8::2"); private static final MacAddress MAC_A = MacAddress.fromString("00:00:00:00:00:0a"); @@ -383,19 +383,20 @@ public class BpfCoordinatorTest { } private void verifyStartUpstreamIpv6Forwarding(@Nullable InOrder inOrder, int downstreamIfIndex, - int upstreamIfindex) throws Exception { + MacAddress downstreamMac, int upstreamIfindex) throws Exception { if (!mDeps.isAtLeastS()) return; - final TetherUpstream6Key key = new TetherUpstream6Key(downstreamIfIndex); + final TetherUpstream6Key key = new TetherUpstream6Key(downstreamIfIndex, downstreamMac); 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) + private void verifyStopUpstreamIpv6Forwarding(@Nullable InOrder inOrder, int downstreamIfIndex, + MacAddress downstreamMac) throws Exception { if (!mDeps.isAtLeastS()) return; - final TetherUpstream6Key key = new TetherUpstream6Key(downstreamIfIndex); + final TetherUpstream6Key key = new TetherUpstream6Key(downstreamIfIndex, downstreamMac); verifyWithOrder(inOrder, mBpfUpstream6Map).deleteEntry(key); } @@ -732,9 +733,10 @@ public class BpfCoordinatorTest { final TetherDownstream6Key key = rule.makeTetherDownstream6Key(); assertEquals(key.iif, (long) mobileIfIndex); + assertEquals(key.dstMac, MacAddress.ALL_ZEROS_ADDRESS); // rawip upstream assertTrue(Arrays.equals(key.neigh6, NEIGH_A.getAddress())); - // iif (4) + neigh6 (16) = 20. - assertEquals(20, key.writeToBytes().length); + // iif (4) + dstMac(6) + padding(2) + neigh6 (16) = 28. + assertEquals(28, key.writeToBytes().length); } @Test @@ -875,7 +877,7 @@ public class BpfCoordinatorTest { verifyTetherOffloadRuleAdd(inOrder, ethernetRuleA); verifyTetherOffloadSetInterfaceQuota(inOrder, ethIfIndex, QUOTA_UNLIMITED, true /* isInit */); - verifyStartUpstreamIpv6Forwarding(inOrder, DOWNSTREAM_IFINDEX, ethIfIndex); + verifyStartUpstreamIpv6Forwarding(inOrder, DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC, ethIfIndex); coordinator.tetherOffloadRuleAdd(mIpServer, ethernetRuleB); verifyTetherOffloadRuleAdd(inOrder, ethernetRuleB); @@ -892,12 +894,13 @@ public class BpfCoordinatorTest { coordinator.tetherOffloadRuleUpdate(mIpServer, mobileIfIndex); verifyTetherOffloadRuleRemove(inOrder, ethernetRuleA); verifyTetherOffloadRuleRemove(inOrder, ethernetRuleB); - verifyStopUpstreamIpv6Forwarding(inOrder, DOWNSTREAM_IFINDEX); + verifyStopUpstreamIpv6Forwarding(inOrder, DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC); verifyTetherOffloadGetAndClearStats(inOrder, ethIfIndex); verifyTetherOffloadRuleAdd(inOrder, mobileRuleA); verifyTetherOffloadSetInterfaceQuota(inOrder, mobileIfIndex, QUOTA_UNLIMITED, true /* isInit */); - verifyStartUpstreamIpv6Forwarding(inOrder, DOWNSTREAM_IFINDEX, mobileIfIndex); + verifyStartUpstreamIpv6Forwarding(inOrder, DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC, + mobileIfIndex); verifyTetherOffloadRuleAdd(inOrder, mobileRuleB); // [3] Clear all rules for a given IpServer. @@ -906,7 +909,7 @@ public class BpfCoordinatorTest { coordinator.tetherOffloadRuleClear(mIpServer); verifyTetherOffloadRuleRemove(inOrder, mobileRuleA); verifyTetherOffloadRuleRemove(inOrder, mobileRuleB); - verifyStopUpstreamIpv6Forwarding(inOrder, DOWNSTREAM_IFINDEX); + verifyStopUpstreamIpv6Forwarding(inOrder, DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC); verifyTetherOffloadGetAndClearStats(inOrder, mobileIfIndex); // [4] Force pushing stats update to verify that the last diff of stats is reported on all From 62733f55242cd23303a52da88f19543bd7009625 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Thu, 1 Apr 2021 21:51:41 -0700 Subject: [PATCH 3/3] bpf_progs - adjust for dstMac addition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test: atest, TreeHugger Signed-off-by: Maciej Żenczykowski Change-Id: I515be275d09dc7e6bae2564f7db2445ea15cc757 --- Tethering/bpf_progs/offload.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tethering/bpf_progs/offload.c b/Tethering/bpf_progs/offload.c index 5f29d4f60a..c32d0514b2 100644 --- a/Tethering/bpf_progs/offload.c +++ b/Tethering/bpf_progs/offload.c @@ -169,6 +169,7 @@ static inline __always_inline int do_forward6(struct __sk_buff* skb, const bool TetherUpstream6Key ku = { .iif = skb->ifindex, }; + if (is_ethernet) __builtin_memcpy(downstream ? kd.dstMac : ku.dstMac, eth->h_dest, ETH_ALEN); Tether6Value* v = downstream ? bpf_tether_downstream6_map_lookup_elem(&kd) : bpf_tether_upstream6_map_lookup_elem(&ku); @@ -474,7 +475,7 @@ static inline __always_inline int do_forward4(struct __sk_buff* skb, const bool .srcPort = is_tcp ? tcph->source : udph->source, .dstPort = is_tcp ? tcph->dest : udph->dest, }; - if (is_ethernet) for (int i = 0; i < ETH_ALEN; ++i) k.dstMac[i] = eth->h_dest[i]; + if (is_ethernet) __builtin_memcpy(k.dstMac, eth->h_dest, ETH_ALEN); Tether4Value* v = downstream ? bpf_tether_downstream4_map_lookup_elem(&k) : bpf_tether_upstream4_map_lookup_elem(&k);