Merge "[CTT-5] Stop update TCP conntrack entry timeout"
This commit is contained in:
@@ -123,9 +123,14 @@ public class BpfCoordinator {
|
|||||||
return makeMapPath((downstream ? "downstream" : "upstream") + ipVersion);
|
return makeMapPath((downstream ? "downstream" : "upstream") + ipVersion);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TODO: probably to remember what the timeout updated things to last is. But that requires
|
||||||
|
// either r/w map entries (which seems bad/racy) or a separate map to keep track of all flows
|
||||||
|
// and remember when they were updated and with what timeout.
|
||||||
@VisibleForTesting
|
@VisibleForTesting
|
||||||
static final int CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS = 60_000;
|
static final int CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS = 60_000;
|
||||||
@VisibleForTesting
|
@VisibleForTesting
|
||||||
|
static final int CONNTRACK_TIMEOUT_UPDATE_SLACK_MS = 20_000;
|
||||||
|
@VisibleForTesting
|
||||||
static final int NF_CONNTRACK_TCP_TIMEOUT_ESTABLISHED = 432_000;
|
static final int NF_CONNTRACK_TCP_TIMEOUT_ESTABLISHED = 432_000;
|
||||||
@VisibleForTesting
|
@VisibleForTesting
|
||||||
static final int NF_CONNTRACK_UDP_TIMEOUT_STREAM = 180;
|
static final int NF_CONNTRACK_UDP_TIMEOUT_STREAM = 180;
|
||||||
@@ -1904,6 +1909,23 @@ public class BpfCoordinator {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
boolean requireConntrackTimeoutUpdate(long nowNs, long lastUsedNs, int proto) {
|
||||||
|
// Refreshing tcp timeout without checking tcp state may make the conntrack entry live
|
||||||
|
// 5 days (432000s) even while the session is being closed. Its BPF rule may not be
|
||||||
|
// deleted for 5 days because the tcp state gets stuck and conntrack delete message is
|
||||||
|
// not sent. Note that both the conntrack monitor and refreshing timeout updater are
|
||||||
|
// in the same thread. Beware while the tcp status may be changed running in refreshing
|
||||||
|
// timeout updater and may read out-of-date tcp stats.
|
||||||
|
// See nf_conntrack_tcp_timeout_established in kernel document.
|
||||||
|
// TODO: support refreshing TCP conntrack timeout.
|
||||||
|
if (proto == OsConstants.IPPROTO_TCP) return false;
|
||||||
|
|
||||||
|
// The timeout requirement check needs the slack time because the scheduled timer may
|
||||||
|
// be not precise. The timeout update has a chance to be missed.
|
||||||
|
return (nowNs - lastUsedNs) < (long) (CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS
|
||||||
|
+ CONNTRACK_TIMEOUT_UPDATE_SLACK_MS) * 1_000_000;
|
||||||
|
}
|
||||||
|
|
||||||
private void refreshAllConntrackTimeouts() {
|
private void refreshAllConntrackTimeouts() {
|
||||||
final long now = mDeps.elapsedRealtimeNanos();
|
final long now = mDeps.elapsedRealtimeNanos();
|
||||||
|
|
||||||
@@ -1911,7 +1933,7 @@ public class BpfCoordinator {
|
|||||||
// because TCP is a bidirectional traffic. Probably don't need to extend timeout by
|
// because TCP is a bidirectional traffic. Probably don't need to extend timeout by
|
||||||
// both directions for TCP.
|
// both directions for TCP.
|
||||||
mBpfCoordinatorShim.tetherOffloadRuleForEach(UPSTREAM, (k, v) -> {
|
mBpfCoordinatorShim.tetherOffloadRuleForEach(UPSTREAM, (k, v) -> {
|
||||||
if ((now - v.lastUsed) / 1_000_000 < CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS) {
|
if (requireConntrackTimeoutUpdate(now, v.lastUsed, k.l4proto)) {
|
||||||
updateConntrackTimeout((byte) k.l4proto,
|
updateConntrackTimeout((byte) k.l4proto,
|
||||||
parseIPv4Address(k.src4), (short) k.srcPort,
|
parseIPv4Address(k.src4), (short) k.srcPort,
|
||||||
parseIPv4Address(k.dst4), (short) k.dstPort);
|
parseIPv4Address(k.dst4), (short) k.dstPort);
|
||||||
@@ -1919,10 +1941,10 @@ public class BpfCoordinator {
|
|||||||
});
|
});
|
||||||
|
|
||||||
// Reverse the source and destination {address, port} from downstream value because
|
// Reverse the source and destination {address, port} from downstream value because
|
||||||
// #updateConntrackTimeout refresh the timeout of netlink attribute CTA_TUPLE_ORIG
|
// #updateConntrackTimeout refreshes the timeout of netlink attribute CTA_TUPLE_ORIG
|
||||||
// which is opposite direction for downstream map value.
|
// which is opposite direction for downstream map value.
|
||||||
mBpfCoordinatorShim.tetherOffloadRuleForEach(DOWNSTREAM, (k, v) -> {
|
mBpfCoordinatorShim.tetherOffloadRuleForEach(DOWNSTREAM, (k, v) -> {
|
||||||
if ((now - v.lastUsed) / 1_000_000 < CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS) {
|
if (requireConntrackTimeoutUpdate(now, v.lastUsed, k.l4proto)) {
|
||||||
updateConntrackTimeout((byte) k.l4proto,
|
updateConntrackTimeout((byte) k.l4proto,
|
||||||
parseIPv4Address(v.dst46), (short) v.dstPort,
|
parseIPv4Address(v.dst46), (short) v.dstPort,
|
||||||
parseIPv4Address(v.src46), (short) v.srcPort);
|
parseIPv4Address(v.src46), (short) v.srcPort);
|
||||||
|
|||||||
@@ -41,7 +41,7 @@ import static android.system.OsConstants.NETLINK_NETFILTER;
|
|||||||
import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;
|
import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;
|
||||||
import static com.android.dx.mockito.inline.extended.ExtendedMockito.staticMockMarker;
|
import static com.android.dx.mockito.inline.extended.ExtendedMockito.staticMockMarker;
|
||||||
import static com.android.networkstack.tethering.BpfCoordinator.CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS;
|
import static com.android.networkstack.tethering.BpfCoordinator.CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS;
|
||||||
import static com.android.networkstack.tethering.BpfCoordinator.NF_CONNTRACK_TCP_TIMEOUT_ESTABLISHED;
|
import static com.android.networkstack.tethering.BpfCoordinator.CONNTRACK_TIMEOUT_UPDATE_SLACK_MS;
|
||||||
import static com.android.networkstack.tethering.BpfCoordinator.NF_CONNTRACK_UDP_TIMEOUT_STREAM;
|
import static com.android.networkstack.tethering.BpfCoordinator.NF_CONNTRACK_UDP_TIMEOUT_STREAM;
|
||||||
import static com.android.networkstack.tethering.BpfCoordinator.StatsType;
|
import static com.android.networkstack.tethering.BpfCoordinator.StatsType;
|
||||||
import static com.android.networkstack.tethering.BpfCoordinator.StatsType.STATS_PER_IFACE;
|
import static com.android.networkstack.tethering.BpfCoordinator.StatsType.STATS_PER_IFACE;
|
||||||
@@ -1526,23 +1526,27 @@ public class BpfCoordinatorTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private void checkRefreshConntrackTimeout(final TestBpfMap<Tether4Key, Tether4Value> bpfMap,
|
private void checkRefreshConntrackTimeout(final TestBpfMap<Tether4Key, Tether4Value> bpfMap,
|
||||||
final Tether4Key tcpKey, final Tether4Value tcpValue, final Tether4Key udpKey,
|
int proto, final Tether4Key key, final Tether4Value value) throws Exception {
|
||||||
final Tether4Value udpValue) throws Exception {
|
if (proto != IPPROTO_TCP && proto != IPPROTO_UDP) {
|
||||||
|
fail("Not support protocol " + proto);
|
||||||
|
}
|
||||||
// Both system elapsed time since boot and the rule last used time are used to measure
|
// Both system elapsed time since boot and the rule last used time are used to measure
|
||||||
// the rule expiration. In this test, all test rules are fixed the last used time to 0.
|
// the rule expiration. In this test, all test rules are fixed the last used time to 0.
|
||||||
// Set the different testing elapsed time to make the rule to be valid or expired.
|
// Set the different testing elapsed time to make the rule to be valid or expired.
|
||||||
//
|
//
|
||||||
// Timeline:
|
// Timeline:
|
||||||
// 0 60 (seconds)
|
// CONNTRACK_TIMEOUT_UPDATE_SLACK_MS
|
||||||
// +---+---+---+---+--...--+---+---+---+---+---+- ..
|
// ->| |<-
|
||||||
// | CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS |
|
// +---+---+---+---+---+--...--+---+---+---+---+---+-..-+---+-- ..
|
||||||
// +---+---+---+---+--...--+---+---+---+---+---+- ..
|
// | CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS |
|
||||||
// |<- valid diff ->|
|
// +---+---+---+---+---+--...--+---+---+---+---+---+-..-+---+-- ..
|
||||||
// |<- expired diff ->|
|
// |<- valid diff ->|
|
||||||
// ^ ^ ^
|
// |<- expired diff ->|
|
||||||
// last used time elapsed time (valid) elapsed time (expired)
|
// ^ ^ ^
|
||||||
final long validTime = (CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS - 1) * 1_000_000L;
|
// last used time elapsed time (valid) elapsed time (expired)
|
||||||
final long expiredTime = (CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS + 1) * 1_000_000L;
|
final long validTimeNs = CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS * 1_000_000L;
|
||||||
|
final long expiredTimeNs = (CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS
|
||||||
|
+ CONNTRACK_TIMEOUT_UPDATE_SLACK_MS + 1) * 1_000_000L;
|
||||||
|
|
||||||
// Static mocking for NetlinkSocket.
|
// Static mocking for NetlinkSocket.
|
||||||
MockitoSession mockSession = ExtendedMockito.mockitoSession()
|
MockitoSession mockSession = ExtendedMockito.mockitoSession()
|
||||||
@@ -1551,30 +1555,27 @@ public class BpfCoordinatorTest {
|
|||||||
try {
|
try {
|
||||||
final BpfCoordinator coordinator = makeBpfCoordinator();
|
final BpfCoordinator coordinator = makeBpfCoordinator();
|
||||||
coordinator.startPolling();
|
coordinator.startPolling();
|
||||||
bpfMap.insertEntry(tcpKey, tcpValue);
|
bpfMap.insertEntry(key, value);
|
||||||
bpfMap.insertEntry(udpKey, udpValue);
|
|
||||||
|
|
||||||
// [1] Don't refresh contrack timeout.
|
// [1] Don't refresh contrack timeout.
|
||||||
setElapsedRealtimeNanos(expiredTime);
|
setElapsedRealtimeNanos(expiredTimeNs);
|
||||||
mTestLooper.moveTimeForward(CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS);
|
mTestLooper.moveTimeForward(CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS);
|
||||||
waitForIdle();
|
waitForIdle();
|
||||||
ExtendedMockito.verifyNoMoreInteractions(staticMockMarker(NetlinkSocket.class));
|
ExtendedMockito.verifyNoMoreInteractions(staticMockMarker(NetlinkSocket.class));
|
||||||
ExtendedMockito.clearInvocations(staticMockMarker(NetlinkSocket.class));
|
ExtendedMockito.clearInvocations(staticMockMarker(NetlinkSocket.class));
|
||||||
|
|
||||||
// [2] Refresh contrack timeout.
|
// [2] Refresh contrack timeout. UDP Only.
|
||||||
setElapsedRealtimeNanos(validTime);
|
setElapsedRealtimeNanos(validTimeNs);
|
||||||
mTestLooper.moveTimeForward(CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS);
|
mTestLooper.moveTimeForward(CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS);
|
||||||
waitForIdle();
|
waitForIdle();
|
||||||
final byte[] expectedNetlinkTcp = ConntrackMessage.newIPv4TimeoutUpdateRequest(
|
|
||||||
IPPROTO_TCP, PRIVATE_ADDR, (int) PRIVATE_PORT, REMOTE_ADDR,
|
if (proto == IPPROTO_UDP) {
|
||||||
(int) REMOTE_PORT, NF_CONNTRACK_TCP_TIMEOUT_ESTABLISHED);
|
final byte[] expectedNetlinkUdp = ConntrackMessage.newIPv4TimeoutUpdateRequest(
|
||||||
final byte[] expectedNetlinkUdp = ConntrackMessage.newIPv4TimeoutUpdateRequest(
|
IPPROTO_UDP, PRIVATE_ADDR, (int) PRIVATE_PORT, REMOTE_ADDR,
|
||||||
IPPROTO_UDP, PRIVATE_ADDR, (int) PRIVATE_PORT, REMOTE_ADDR,
|
(int) REMOTE_PORT, NF_CONNTRACK_UDP_TIMEOUT_STREAM);
|
||||||
(int) REMOTE_PORT, NF_CONNTRACK_UDP_TIMEOUT_STREAM);
|
ExtendedMockito.verify(() -> NetlinkSocket.sendOneShotKernelMessage(
|
||||||
ExtendedMockito.verify(() -> NetlinkSocket.sendOneShotKernelMessage(
|
eq(NETLINK_NETFILTER), eq(expectedNetlinkUdp)));
|
||||||
eq(NETLINK_NETFILTER), eq(expectedNetlinkTcp)));
|
}
|
||||||
ExtendedMockito.verify(() -> NetlinkSocket.sendOneShotKernelMessage(
|
|
||||||
eq(NETLINK_NETFILTER), eq(expectedNetlinkUdp)));
|
|
||||||
ExtendedMockito.verifyNoMoreInteractions(staticMockMarker(NetlinkSocket.class));
|
ExtendedMockito.verifyNoMoreInteractions(staticMockMarker(NetlinkSocket.class));
|
||||||
ExtendedMockito.clearInvocations(staticMockMarker(NetlinkSocket.class));
|
ExtendedMockito.clearInvocations(staticMockMarker(NetlinkSocket.class));
|
||||||
|
|
||||||
@@ -1591,33 +1592,57 @@ public class BpfCoordinatorTest {
|
|||||||
|
|
||||||
@Test
|
@Test
|
||||||
@IgnoreUpTo(Build.VERSION_CODES.R)
|
@IgnoreUpTo(Build.VERSION_CODES.R)
|
||||||
public void testRefreshConntrackTimeout_Upstream4Map() throws Exception {
|
public void testRefreshConntrackTimeout_Upstream4MapTcp() throws Exception {
|
||||||
// TODO: Replace the dependencies BPF map with a non-mocked TestBpfMap object.
|
// TODO: Replace the dependencies BPF map with a non-mocked TestBpfMap object.
|
||||||
final TestBpfMap<Tether4Key, Tether4Value> bpfUpstream4Map =
|
final TestBpfMap<Tether4Key, Tether4Value> bpfUpstream4Map =
|
||||||
new TestBpfMap<>(Tether4Key.class, Tether4Value.class);
|
new TestBpfMap<>(Tether4Key.class, Tether4Value.class);
|
||||||
doReturn(bpfUpstream4Map).when(mDeps).getBpfUpstream4Map();
|
doReturn(bpfUpstream4Map).when(mDeps).getBpfUpstream4Map();
|
||||||
|
|
||||||
final Tether4Key tcpKey = makeUpstream4Key(IPPROTO_TCP);
|
final Tether4Key key = makeUpstream4Key(IPPROTO_TCP);
|
||||||
final Tether4Key udpKey = makeUpstream4Key(IPPROTO_UDP);
|
final Tether4Value value = makeUpstream4Value();
|
||||||
final Tether4Value tcpValue = makeUpstream4Value();
|
|
||||||
final Tether4Value udpValue = makeUpstream4Value();
|
|
||||||
|
|
||||||
checkRefreshConntrackTimeout(bpfUpstream4Map, tcpKey, tcpValue, udpKey, udpValue);
|
checkRefreshConntrackTimeout(bpfUpstream4Map, IPPROTO_TCP, key, value);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@IgnoreUpTo(Build.VERSION_CODES.R)
|
@IgnoreUpTo(Build.VERSION_CODES.R)
|
||||||
public void testRefreshConntrackTimeout_Downstream4Map() throws Exception {
|
public void testRefreshConntrackTimeout_Upstream4MapUdp() throws Exception {
|
||||||
|
// TODO: Replace the dependencies BPF map with a non-mocked TestBpfMap object.
|
||||||
|
final TestBpfMap<Tether4Key, Tether4Value> bpfUpstream4Map =
|
||||||
|
new TestBpfMap<>(Tether4Key.class, Tether4Value.class);
|
||||||
|
doReturn(bpfUpstream4Map).when(mDeps).getBpfUpstream4Map();
|
||||||
|
|
||||||
|
final Tether4Key key = makeUpstream4Key(IPPROTO_UDP);
|
||||||
|
final Tether4Value value = makeUpstream4Value();
|
||||||
|
|
||||||
|
checkRefreshConntrackTimeout(bpfUpstream4Map, IPPROTO_UDP, key, value);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@IgnoreUpTo(Build.VERSION_CODES.R)
|
||||||
|
public void testRefreshConntrackTimeout_Downstream4MapTcp() throws Exception {
|
||||||
// TODO: Replace the dependencies BPF map with a non-mocked TestBpfMap object.
|
// TODO: Replace the dependencies BPF map with a non-mocked TestBpfMap object.
|
||||||
final TestBpfMap<Tether4Key, Tether4Value> bpfDownstream4Map =
|
final TestBpfMap<Tether4Key, Tether4Value> bpfDownstream4Map =
|
||||||
new TestBpfMap<>(Tether4Key.class, Tether4Value.class);
|
new TestBpfMap<>(Tether4Key.class, Tether4Value.class);
|
||||||
doReturn(bpfDownstream4Map).when(mDeps).getBpfDownstream4Map();
|
doReturn(bpfDownstream4Map).when(mDeps).getBpfDownstream4Map();
|
||||||
|
|
||||||
final Tether4Key tcpKey = makeDownstream4Key(IPPROTO_TCP);
|
final Tether4Key key = makeDownstream4Key(IPPROTO_TCP);
|
||||||
final Tether4Key udpKey = makeDownstream4Key(IPPROTO_UDP);
|
final Tether4Value value = makeDownstream4Value();
|
||||||
final Tether4Value tcpValue = makeDownstream4Value();
|
|
||||||
final Tether4Value udpValue = makeDownstream4Value();
|
|
||||||
|
|
||||||
checkRefreshConntrackTimeout(bpfDownstream4Map, tcpKey, tcpValue, udpKey, udpValue);
|
checkRefreshConntrackTimeout(bpfDownstream4Map, IPPROTO_TCP, key, value);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@IgnoreUpTo(Build.VERSION_CODES.R)
|
||||||
|
public void testRefreshConntrackTimeout_Downstream4MapUdp() throws Exception {
|
||||||
|
// TODO: Replace the dependencies BPF map with a non-mocked TestBpfMap object.
|
||||||
|
final TestBpfMap<Tether4Key, Tether4Value> bpfDownstream4Map =
|
||||||
|
new TestBpfMap<>(Tether4Key.class, Tether4Value.class);
|
||||||
|
doReturn(bpfDownstream4Map).when(mDeps).getBpfDownstream4Map();
|
||||||
|
|
||||||
|
final Tether4Key key = makeDownstream4Key(IPPROTO_UDP);
|
||||||
|
final Tether4Value value = makeDownstream4Value();
|
||||||
|
|
||||||
|
checkRefreshConntrackTimeout(bpfDownstream4Map, IPPROTO_UDP, key, value);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user