Merge "Remove duplicated bpf offload support check in IpServer" into main
diff --git a/Tethering/src/android/net/ip/IpServer.java b/Tethering/src/android/net/ip/IpServer.java
index eadba58..a851410 100644
--- a/Tethering/src/android/net/ip/IpServer.java
+++ b/Tethering/src/android/net/ip/IpServer.java
@@ -252,7 +252,6 @@
private final int mInterfaceType;
private final LinkProperties mLinkProperties;
private final boolean mUsingLegacyDhcp;
- private final boolean mUsingBpfOffload;
private final int mP2pLeasesSubnetPrefixLength;
private final Dependencies mDeps;
@@ -314,7 +313,6 @@
mInterfaceType = interfaceType;
mLinkProperties = new LinkProperties();
mUsingLegacyDhcp = config.useLegacyDhcpServer();
- mUsingBpfOffload = config.isBpfOffloadEnabled();
mP2pLeasesSubnetPrefixLength = config.getP2pLeasesSubnetPrefixLength();
mPrivateAddressCoordinator = addressCoordinator;
mDeps = deps;
@@ -326,11 +324,9 @@
mIpNeighborMonitor = mDeps.getIpNeighborMonitor(getHandler(), mLog,
new MyNeighborEventConsumer());
- // IP neighbor monitor monitors the neighbor events for adding/removing offload
- // forwarding rules per client. If BPF offload is not supported, don't start listening
- // for neighbor events. See updateIpv6ForwardingRules, addIpv6DownstreamRule,
- // removeIpv6DownstreamRule.
- if (mUsingBpfOffload && !mIpNeighborMonitor.start()) {
+ // IP neighbor monitor monitors the neighbor events for adding/removing IPv6 downstream rule
+ // per client. If BPF offload is not supported, don't start listening for neighbor events.
+ if (mBpfCoordinator.isUsingBpfOffload() && !mIpNeighborMonitor.start()) {
mLog.e("Failed to create IpNeighborMonitor on " + mIfaceName);
}
@@ -891,37 +887,6 @@
}
}
- private void addIpv6DownstreamRule(Ipv6DownstreamRule rule) {
- // Theoretically, we don't need this check because IP neighbor monitor doesn't start if BPF
- // offload is disabled. Add this check just in case.
- // TODO: Perhaps remove this protection check.
- if (!mUsingBpfOffload) return;
-
- mBpfCoordinator.addIpv6DownstreamRule(this, rule);
- }
-
- private void removeIpv6DownstreamRule(Ipv6DownstreamRule rule) {
- // TODO: Perhaps remove this protection check.
- // See the related comment in #addIpv6DownstreamRule.
- if (!mUsingBpfOffload) return;
-
- mBpfCoordinator.removeIpv6DownstreamRule(this, rule);
- }
-
- private void clearIpv6ForwardingRules() {
- if (!mUsingBpfOffload) return;
-
- mBpfCoordinator.tetherOffloadRuleClear(this);
- }
-
- private void updateIpv6ForwardingRule(int newIfindex) {
- // TODO: Perhaps remove this protection check.
- // See the related comment in #addIpv6DownstreamRule.
- if (!mUsingBpfOffload) return;
-
- mBpfCoordinator.tetherOffloadRuleUpdate(this, newIfindex);
- }
-
// Handles all updates to IPv6 forwarding rules. These can currently change only if the upstream
// changes or if a neighbor event is received.
private void updateIpv6ForwardingRules(int prevUpstreamIfindex, int upstreamIfindex,
@@ -930,14 +895,14 @@
// nothing else.
// TODO: Rather than always clear rules, ensure whether ipv6 ever enable first.
if (upstreamIfindex == 0 || !upstreamSupportsBpf) {
- clearIpv6ForwardingRules();
+ mBpfCoordinator.tetherOffloadRuleClear(this);
return;
}
// If the upstream interface has changed, remove all rules and re-add them with the new
// upstream interface.
if (prevUpstreamIfindex != upstreamIfindex) {
- updateIpv6ForwardingRule(upstreamIfindex);
+ mBpfCoordinator.tetherOffloadRuleUpdate(this, upstreamIfindex);
}
// If we're here to process a NeighborEvent, do so now.
@@ -955,18 +920,14 @@
Ipv6DownstreamRule rule = new Ipv6DownstreamRule(upstreamIfindex, mInterfaceParams.index,
(Inet6Address) e.ip, mInterfaceParams.macAddr, dstMac);
if (e.isValid()) {
- addIpv6DownstreamRule(rule);
+ mBpfCoordinator.addIpv6DownstreamRule(this, rule);
} else {
- removeIpv6DownstreamRule(rule);
+ mBpfCoordinator.removeIpv6DownstreamRule(this, rule);
}
}
// TODO: consider moving into BpfCoordinator.
private void updateClientInfoIpv4(NeighborEvent e) {
- // TODO: Perhaps remove this protection check.
- // See the related comment in #addIpv6DownstreamRule.
- if (!mUsingBpfOffload) return;
-
if (e == null) return;
if (!(e.ip instanceof Inet4Address) || e.ip.isMulticastAddress()
|| e.ip.isLoopbackAddress() || e.ip.isLinkLocalAddress()) {
@@ -1342,7 +1303,7 @@
for (String ifname : mUpstreamIfaceSet.ifnames) cleanupUpstreamInterface(ifname);
mUpstreamIfaceSet = null;
- clearIpv6ForwardingRules();
+ mBpfCoordinator.tetherOffloadRuleClear(IpServer.this);
}
private void cleanupUpstreamInterface(String upstreamIface) {
diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
index 7311125..1b23a6c 100644
--- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
+++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
@@ -523,6 +523,18 @@
mLog.i("Polling stopped");
}
+ /**
+ * Return whether BPF offload is supported
+ */
+ public boolean isUsingBpfOffload() {
+ return isUsingBpf();
+ }
+
+ // This is identical to isUsingBpfOffload above but is only used internally.
+ // The reason for having two separate methods is that the code calls isUsingBpf
+ // very often. But the tests call verifyNoMoreInteractions, which will check all
+ // calls to public methods. If isUsingBpf were public, the test would need to
+ // verify all calls to it, which would clutter the test.
private boolean isUsingBpf() {
return mIsBpfEnabled && mBpfCoordinatorShim.isInitialized();
}
diff --git a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java
index 19d70c6..c0718d1 100644
--- a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java
+++ b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java
@@ -244,6 +244,8 @@
when(mTetherConfig.isBpfOffloadEnabled()).thenReturn(usingBpfOffload);
when(mTetherConfig.useLegacyDhcpServer()).thenReturn(usingLegacyDhcp);
when(mTetherConfig.getP2pLeasesSubnetPrefixLength()).thenReturn(P2P_SUBNET_PREFIX_LENGTH);
+ // Recreate mBpfCoordinator again here because mTetherConfig has changed
+ mBpfCoordinator = spy(new BpfCoordinator(mBpfDeps));
mIpServer = new IpServer(
IFACE_NAME, mLooper.getLooper(), interfaceType, mSharedLog, mNetd, mBpfCoordinator,
mCallback, mTetherConfig, mAddressCoordinator, mTetheringMetrics, mDependencies);
@@ -1244,13 +1246,11 @@
resetNetdBpfMapAndCoordinator();
recvNewNeigh(myIfindex, neigh, NUD_REACHABLE, macA);
- verify(mBpfCoordinator, never()).addIpv6DownstreamRule(any(), any());
verifyNeverTetherOffloadRuleAdd();
verifyNoUpstreamIpv6ForwardingChange(null);
resetNetdBpfMapAndCoordinator();
recvDelNeigh(myIfindex, neigh, NUD_STALE, macA);
- verify(mBpfCoordinator, never()).removeIpv6DownstreamRule(any(), any());
verifyNeverTetherOffloadRuleRemove();
verifyNoUpstreamIpv6ForwardingChange(null);
resetNetdBpfMapAndCoordinator();