LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

ClatCoordinator: only dump information when clat is started

Browse Source 
ClatCoordinator should not rely on the caller Nat464Xlat to
avoid dumping when clat is not started. It is safer to dump
information after ClatCoordinator has checked its status.
Only dumping ClatCoordinator with clatd started reduces the null
pointer check of dumped elements as well.

Test: atest ClatCoordinatorTest
Change-Id: Ia0be9a593cd75fd34f647230d642b0dcf84aaf3b
This commit is contained in:
Hungming Chen
2022-09-21 17:47:54 +08:00
committed by Nucca Chen
parent 23196b6ccb
commit 0090e969af
3 changed files with 60 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
service/src/com/android/server/connectivity/ClatCoordinator.java
View File

@@ -595,13 +595,17 @@ public class ClatCoordinator {
Log.i(TAG, "untag socket cookie " + cookie);
}
private boolean isStarted() {
return mClatdTracker != null;
}
/**
* Start clatd for a given interface and NAT64 prefix.
*/
public String clatStart(final String iface, final int netId,
@NonNull final IpPrefix nat64Prefix)
throws IOException {
if (mClatdTracker != null) {
if (isStarted()) {
throw new IOException("Clatd is already running on " + mClatdTracker.iface
+ " (pid " + mClatdTracker.pid + ")");
}
@@ -833,7 +837,7 @@ public class ClatCoordinator {
* Stop clatd
*/
public void clatStop() throws IOException {
if (mClatdTracker == null) {
if (!isStarted()) {
throw new IOException("Clatd has not started");
}
Log.i(TAG, "Stopping clatd pid=" + mClatdTracker.pid + " on " + mClatdTracker.iface);
@@ -902,12 +906,16 @@ public class ClatCoordinator {
public void dump(@NonNull IndentingPrintWriter pw) {
// TODO: move map dump to a global place to avoid duplicate dump while there are two or
// more IPv6 only networks.
pw.println("CLAT tracker: " + mClatdTracker.toString());
pw.println("Forwarding rules:");
pw.increaseIndent();
dumpBpfIngress(pw);
dumpBpfEgress(pw);
pw.decreaseIndent();
if (isStarted()) {
pw.println("CLAT tracker: " + mClatdTracker.toString());
pw.println("Forwarding rules:");
pw.increaseIndent();
dumpBpfIngress(pw);
dumpBpfEgress(pw);
pw.decreaseIndent();
} else {
pw.println("<not started>");
}
pw.println();
}

3
service/src/com/android/server/connectivity/Nat464Xlat.java
View File

@@ -540,6 +540,9 @@ public class Nat464Xlat {
*/
public void dump(IndentingPrintWriter pw) {
if (SdkLevel.isAtLeastT()) {
// Dump ClatCoordinator information while clatd has been started but not running. The
// reason is that it helps to have more information if clatd is started but the
// v4-* interface doesn't bring up. See #isStarted, #isRunning.
if (isStarted()) {
pw.println("ClatCoordinator:");
pw.increaseIndent();

74
tests/unit/java/com/android/server/connectivity/ClatCoordinatorTest.java
View File

@@ -516,28 +516,38 @@ public class ClatCoordinatorTest {
assertEquals(65508, ClatCoordinator.adjustMtu(CLAT_MAX_MTU + 1 /* over maximum mtu */));
}
@Test
public void testDump() throws Exception {
final ClatCoordinator coordinator = makeClatCoordinator();
private void verifyDump(final ClatCoordinator coordinator, boolean clatStarted) {
final StringWriter stringWriter = new StringWriter();
final IndentingPrintWriter ipw = new IndentingPrintWriter(stringWriter, " ");
coordinator.clatStart(BASE_IFACE, NETID, NAT64_IP_PREFIX);
coordinator.dump(ipw);
final String[] dumpStrings = stringWriter.toString().split("\n");
assertEquals(6, dumpStrings.length);
assertEquals("CLAT tracker: iface: test0 (1000), v4iface: v4-test0 (1001), "
+ "v4: /192.0.0.46, v6: /2001:db8:0:b11::464, pfx96: /64:ff9b::, "
+ "pid: 10483, cookie: 27149", dumpStrings[0].trim());
assertEquals("Forwarding rules:", dumpStrings[1].trim());
assertEquals("BPF ingress map: iif nat64Prefix v6Addr -> v4Addr oif",
dumpStrings[2].trim());
assertEquals("1000 /64:ff9b::/96 /2001:db8:0:b11::464 -> /192.0.0.46 1001",
dumpStrings[3].trim());
assertEquals("BPF egress map: iif v4Addr -> v6Addr nat64Prefix oif",
dumpStrings[4].trim());
assertEquals("1001 /192.0.0.46 -> /2001:db8:0:b11::464 /64:ff9b::/96 1000 ether",
dumpStrings[5].trim());
if (clatStarted) {
assertEquals(6, dumpStrings.length);
assertEquals("CLAT tracker: iface: test0 (1000), v4iface: v4-test0 (1001), "
+ "v4: /192.0.0.46, v6: /2001:db8:0:b11::464, pfx96: /64:ff9b::, "
+ "pid: 10483, cookie: 27149", dumpStrings[0].trim());
assertEquals("Forwarding rules:", dumpStrings[1].trim());
assertEquals("BPF ingress map: iif nat64Prefix v6Addr -> v4Addr oif",
dumpStrings[2].trim());
assertEquals("1000 /64:ff9b::/96 /2001:db8:0:b11::464 -> /192.0.0.46 1001",
dumpStrings[3].trim());
assertEquals("BPF egress map: iif v4Addr -> v6Addr nat64Prefix oif",
dumpStrings[4].trim());
assertEquals("1001 /192.0.0.46 -> /2001:db8:0:b11::464 /64:ff9b::/96 1000 ether",
dumpStrings[5].trim());
} else {
assertEquals(1, dumpStrings.length);
assertEquals("<not started>", dumpStrings[0].trim());
}
}
@Test
public void testDump() throws Exception {
final ClatCoordinator coordinator = makeClatCoordinator();
verifyDump(coordinator, false /* clatStarted */);
coordinator.clatStart(BASE_IFACE, NETID, NAT64_IP_PREFIX);
verifyDump(coordinator, true /* clatStarted */);
}
@Test
@@ -548,25 +558,18 @@ public class ClatCoordinatorTest {
() -> coordinator.clatStart(BASE_IFACE, NETID, invalidPrefix));
}
private void assertStartClat(final TestDependencies deps) throws Exception {
final ClatCoordinator coordinator = new ClatCoordinator(deps);
assertNotNull(coordinator.clatStart(BASE_IFACE, NETID, NAT64_IP_PREFIX));
}
private void assertNotStartClat(final TestDependencies deps) {
// Expect that the injection function of TestDependencies causes clatStart() failed.
final ClatCoordinator coordinator = new ClatCoordinator(deps);
assertThrows(IOException.class,
() -> coordinator.clatStart(BASE_IFACE, NETID, NAT64_IP_PREFIX));
}
private void checkNotStartClat(final TestDependencies deps, final boolean needToCloseTunFd,
final boolean needToClosePacketSockFd, final boolean needToCloseRawSockFd)
throws Exception {
// [1] Expect that modified TestDependencies can't start clatd.
// Use precise check to make sure that there is no unexpected file descriptor closing.
clearInvocations(TUN_PFD, RAW_SOCK_PFD, PACKET_SOCK_PFD);
assertNotStartClat(deps);
// [1] Expect that modified TestDependencies can't start clatd.
// Expect that the injection function of TestDependencies causes clatStart() failed.
final ClatCoordinator coordinatorWithBrokenDeps = new ClatCoordinator(deps);
assertThrows(IOException.class,
() -> coordinatorWithBrokenDeps.clatStart(BASE_IFACE, NETID, NAT64_IP_PREFIX));
// Use precise check to make sure that there is no unexpected file descriptor closing.
if (needToCloseTunFd) {
verify(TUN_PFD).close();
} else {
@@ -583,10 +586,15 @@ public class ClatCoordinatorTest {
verify(RAW_SOCK_PFD, never()).close();
}
// Check that dump doesn't crash after any clat starting failure.
verifyDump(coordinatorWithBrokenDeps, false /* clatStarted */);
// [2] Expect that unmodified TestDependencies can start clatd.
// Used to make sure that the above modified TestDependencies has really broken the
// clatd starting.
assertStartClat(new TestDependencies());
final ClatCoordinator coordinatorWithDefaultDeps = new ClatCoordinator(
new TestDependencies());
assertNotNull(coordinatorWithDefaultDeps.clatStart(BASE_IFACE, NETID, NAT64_IP_PREFIX));
}
// The following testNotStartClat* tests verifies bunches of code for unwinding the