Merge changes from topic "wrap_fd_to_parcelfiledescriptor"

* changes:
  Catch new exceptions from BpfMap
  Revert "Open and close clat bpf map while clat is starting and stoping"
  Revert "ClatCoordinator: replace BpfMap with IBpfMap"
This commit is contained in:
Nucca Chen 2022-05-12 13:42:23 +00:00 • committed by Gerrit Code Review
commit f889ed7a30
4 changed files with 16 additions and 51 deletions

View file

@ -77,6 +77,7 @@ import com.android.net.module.util.netlink.NetlinkSocket;
import com.android.networkstack.tethering.apishim.common.BpfCoordinatorShim;
import com.android.networkstack.tethering.util.TetheringUtils.ForwardedStats;
import java.io.IOException;
import java.net.Inet4Address;
import java.net.Inet6Address;
import java.net.InetAddress;
@ -1024,7 +1025,7 @@ public class BpfCoordinator {
map.forEach((k, v) -> {
pw.println(String.format("%s: %s", k, v));
});
} catch (ErrnoException e) {
} catch (ErrnoException | IOException e) {
pw.println("Error dumping BPF stats map: " + e);
}
}
@ -1072,7 +1073,7 @@ public class BpfCoordinator {
return;
}
map.forEach((k, v) -> pw.println(ipv6UpstreamRuletoString(k, v)));
} catch (ErrnoException e) {
} catch (ErrnoException | IOException e) {
pw.println("Error dumping IPv6 upstream map: " + e);
}
}
@ -1116,7 +1117,7 @@ public class BpfCoordinator {
if (CollectionUtils.contains(args, DUMPSYS_RAWMAP_ARG_STATS)) {
try (BpfMap<TetherStatsKey, TetherStatsValue> statsMap = mDeps.getBpfStatsMap()) {
dumpRawMap(statsMap, pw);
} catch (ErrnoException e) {
} catch (ErrnoException | IOException e) {
pw.println("Error dumping stats map: " + e);
}
return;
@ -1124,7 +1125,7 @@ public class BpfCoordinator {
if (CollectionUtils.contains(args, DUMPSYS_RAWMAP_ARG_UPSTREAM4)) {
try (BpfMap<Tether4Key, Tether4Value> upstreamMap = mDeps.getBpfUpstream4Map()) {
dumpRawMap(upstreamMap, pw);
} catch (ErrnoException e) {
} catch (ErrnoException | IOException e) {
pw.println("Error dumping IPv4 map: " + e);
}
return;
@ -1195,7 +1196,7 @@ public class BpfCoordinator {
pw.increaseIndent();
dumpIpv4ForwardingRuleMap(now, DOWNSTREAM, downstreamMap, pw);
pw.decreaseIndent();
} catch (ErrnoException e) {
} catch (ErrnoException | IOException e) {
pw.println("Error dumping IPv4 map: " + e);
}
}
@ -1220,7 +1221,7 @@ public class BpfCoordinator {
}
if (v.val > 0) pw.println(String.format("%s: %d", counterName, v.val));
});
} catch (ErrnoException e) {
} catch (ErrnoException | IOException e) {
pw.println("Error dumping counter map: " + e);
}
}
@ -1244,7 +1245,7 @@ public class BpfCoordinator {
pw.println(String.format("%d (%s) -> %d (%s)", k.ifIndex, getIfName(k.ifIndex),
v.ifIndex, getIfName(v.ifIndex)));
});
} catch (ErrnoException e) {
} catch (ErrnoException | IOException e) {
pw.println("Error dumping dev map: " + e);
}
pw.decreaseIndent();

View file

@ -358,8 +358,8 @@ public final class BpfMapTest {
try {
mTestMap.clear();
fail("clearing already-closed map should throw");
} catch (ErrnoException expected) {
assertEquals(OsConstants.EBADF, expected.errno);
} catch (IllegalStateException expected) {
// ParcelFileDescriptor.getFd throws IllegalStateException: Already closed.
}
}

View file

@ -116,12 +116,10 @@ public class ClatCoordinator {
private final INetd mNetd;
@NonNull
private final Dependencies mDeps;
// IBpfMap objects {mIngressMap, mEgressMap} are initialized in #maybeStartBpf and closed in
// #maybeStopBpf.
@Nullable
private IBpfMap<ClatIngress6Key, ClatIngress6Value> mIngressMap = null;
private final IBpfMap<ClatIngress6Key, ClatIngress6Value> mIngressMap;
@Nullable
private IBpfMap<ClatEgress4Key, ClatEgress4Value> mEgressMap = null;
private final IBpfMap<ClatEgress4Key, ClatEgress4Value> mEgressMap;
@Nullable
private ClatdTracker mClatdTracker = null;
@ -375,35 +373,12 @@ public class ClatCoordinator {
public ClatCoordinator(@NonNull Dependencies deps) {
mDeps = deps;
mNetd = mDeps.getNetd();
}
private void closeEgressMap() {
try {
mEgressMap.close();
} catch (Exception e) {
Log.e(TAG, "Cannot close egress4 map: " + e);
}
mEgressMap = null;
}
private void closeIngressMap() {
try {
mIngressMap.close();
} catch (Exception e) {
Log.e(TAG, "Cannot close ingress6 map: " + e);
}
mIngressMap = null;
mIngressMap = mDeps.getBpfIngress6Map();
mEgressMap = mDeps.getBpfEgress4Map();
}
private void maybeStartBpf(final ClatdTracker tracker) {
mEgressMap = mDeps.getBpfEgress4Map();
if (mEgressMap == null) return;
mIngressMap = mDeps.getBpfIngress6Map();
if (mIngressMap == null) {
closeEgressMap();
return;
}
if (mIngressMap == null || mEgressMap == null) return;
final boolean isEthernet;
try {
@ -747,13 +722,6 @@ public class ClatCoordinator {
} catch (ErrnoException | IllegalStateException e) {
Log.e(TAG, "Could not delete entry (" + rxKey + "): " + e);
}
// Manual close BPF map file descriptors. Just don't rely on that GC releasing to close
// the file descriptors even if class BpfMap supports close file descriptor in
// finalize(). If the interfaces are added and removed quickly, too many unclosed file
// descriptors may cause unexpected problem.
closeEgressMap();
closeIngressMap();
}
/**
@ -822,7 +790,7 @@ public class ClatCoordinator {
}
/**
* Dump the cordinator information. Only called when clat is started. See Nat464Xlat#dump.
* Dump the cordinator information.
*
* @param pw print writer.
*/

View file

@ -447,8 +447,6 @@ public class ClatCoordinatorTest {
argThat(fd -> Objects.equals(RAW_SOCK_PFD.getFileDescriptor(), fd)),
eq(BASE_IFACE), eq(NAT64_PREFIX_STRING),
eq(XLAT_LOCAL_IPV4ADDR_STRING), eq(XLAT_LOCAL_IPV6ADDR_STRING));
inOrder.verify(mDeps).getBpfEgress4Map();
inOrder.verify(mDeps).getBpfIngress6Map();
inOrder.verify(mEgressMap).insertEntry(eq(EGRESS_KEY), eq(EGRESS_VALUE));
inOrder.verify(mIngressMap).insertEntry(eq(INGRESS_KEY), eq(INGRESS_VALUE));
inOrder.verify(mDeps).tcQdiscAddDevClsact(eq(STACKED_IFINDEX));
@ -471,8 +469,6 @@ public class ClatCoordinatorTest {
eq((short) PRIO_CLAT), eq((short) ETH_P_IP));
inOrder.verify(mEgressMap).deleteEntry(eq(EGRESS_KEY));
inOrder.verify(mIngressMap).deleteEntry(eq(INGRESS_KEY));
inOrder.verify(mEgressMap).close();
inOrder.verify(mIngressMap).close();
inOrder.verify(mDeps).stopClatd(eq(BASE_IFACE), eq(NAT64_PREFIX_STRING),
eq(XLAT_LOCAL_IPV4ADDR_STRING), eq(XLAT_LOCAL_IPV6ADDR_STRING), eq(CLATD_PID));
inOrder.verify(mDeps).untagSocket(eq(RAW_SOCK_COOKIE));