Merge "Remove duplicated bpf offload support check in IpServer" into main

This commit is contained in:
KH Shi
2023-09-07 14:46:34 +00:00
committed by Gerrit Code Review
3 changed files with 22 additions and 49 deletions

View File

@@ -252,7 +252,6 @@ public class IpServer extends StateMachine {
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 @@ public class IpServer extends StateMachine {
mInterfaceType = interfaceType;
mLinkProperties = new LinkProperties();
mUsingLegacyDhcp = config.useLegacyDhcpServer();
mUsingBpfOffload = config.isBpfOffloadEnabled();
mP2pLeasesSubnetPrefixLength = config.getP2pLeasesSubnetPrefixLength();
mPrivateAddressCoordinator = addressCoordinator;
mDeps = deps;
@@ -326,11 +324,9 @@ public class IpServer extends StateMachine {
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 @@ public class IpServer extends StateMachine {
}
}
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 @@ public class IpServer extends StateMachine {
// 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 @@ public class IpServer extends StateMachine {
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 @@ public class IpServer extends StateMachine {
for (String ifname : mUpstreamIfaceSet.ifnames) cleanupUpstreamInterface(ifname);
mUpstreamIfaceSet = null;
clearIpv6ForwardingRules();
mBpfCoordinator.tetherOffloadRuleClear(IpServer.this);
}
private void cleanupUpstreamInterface(String upstreamIface) {

View File

@@ -523,6 +523,18 @@ public class BpfCoordinator {
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();
}

View File

@@ -244,6 +244,8 @@ public class IpServerTest {
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 @@ public class IpServerTest {
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();