[mdns] separate the multicast reply quota for IPv4 and IPv6
This CL separates the multicast reply quota for IPv4 and IPv6. Before
this change, when an IPv4 mDNS query and an IPv6 mdns query arrives at
the same time for the same record (which is a common behavior for DNS-SD
resolvers), MdnsRecordRepository will only respond to one of them. The
other response will be throttled silently.
It's especially problematic when the IPv4 query and IPv6 query are sent
by different hosts. One of the hosts will get no reply.
Bug: 335096961
Test: Unit test and verified on a local device.
Change-Id: I0d7a9fad2b43bbfb8b042828295cdf08bf4ae5c8
diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java b/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java
index 073e465..5ff19ca 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java
@@ -140,17 +140,25 @@
public final boolean isSharedName;
/**
- * Last time (as per SystemClock.elapsedRealtime) when advertised via multicast, 0 if never
+ * Last time (as per SystemClock.elapsedRealtime) when advertised via multicast on IPv4, 0
+ * if never
*/
- public long lastAdvertisedTimeMs;
+ public long lastAdvertisedOnIpv4TimeMs;
/**
- * Last time (as per SystemClock.elapsedRealtime) when sent via unicast or multicast,
- * 0 if never
+ * Last time (as per SystemClock.elapsedRealtime) when advertised via multicast on IPv6, 0
+ * if never
*/
- // FIXME: the `lastSentTimeMs` and `lastAdvertisedTimeMs` should be maintained separately
- // for IPv4 and IPv6, because neither IPv4 nor and IPv6 clients can receive replies in
- // different address space.
+ public long lastAdvertisedOnIpv6TimeMs;
+
+ /**
+ * Last time (as per SystemClock.elapsedRealtime) when sent via unicast or multicast, 0 if
+ * never.
+ *
+ * <p>Different from lastAdvertisedOnIpv(4|6)TimeMs, lastSentTimeMs is mainly used for
+ * tracking is a record is ever sent out, no matter unicast/multicast or IPv4/IPv6. It's
+ * unnecessary to maintain two versions (IPv4/IPv6) for it.
+ */
public long lastSentTimeMs;
RecordInfo(NsdServiceInfo serviceInfo, T record, boolean sharedName) {
@@ -578,6 +586,7 @@
@Nullable
public MdnsReplyInfo getReply(MdnsPacket packet, InetSocketAddress src) {
final long now = SystemClock.elapsedRealtime();
+ final boolean isQuestionOnIpv4 = src.getAddress() instanceof Inet4Address;
// TODO: b/322142420 - Set<RecordInfo<?>> may contain duplicate records wrapped in different
// RecordInfo<?>s when custom host is enabled.
@@ -595,7 +604,7 @@
null /* serviceSrvRecord */, null /* serviceTxtRecord */,
null /* hostname */,
replyUnicastEnabled, now, answerInfo, additionalAnswerInfo,
- Collections.emptyList())) {
+ Collections.emptyList(), isQuestionOnIpv4)) {
replyUnicast &= question.isUnicastReplyRequested();
}
@@ -607,7 +616,7 @@
registration.srvRecord, registration.txtRecord,
registration.serviceInfo.getHostname(),
replyUnicastEnabled, now,
- answerInfo, additionalAnswerInfo, packet.answers)) {
+ answerInfo, additionalAnswerInfo, packet.answers, isQuestionOnIpv4)) {
replyUnicast &= question.isUnicastReplyRequested();
registration.repliedServiceCount++;
registration.sentPacketCount++;
@@ -685,7 +694,7 @@
// multicast responses. Unicast replies are faster as they do not need to wait for the
// beacon interval on Wi-Fi.
dest = src;
- } else if (src.getAddress() instanceof Inet4Address) {
+ } else if (isQuestionOnIpv4) {
dest = IPV4_SOCKET_ADDR;
} else {
dest = IPV6_SOCKET_ADDR;
@@ -697,7 +706,11 @@
// TODO: consider actual packet send delay after response aggregation
info.lastSentTimeMs = now + delayMs;
if (!replyUnicast) {
- info.lastAdvertisedTimeMs = info.lastSentTimeMs;
+ if (isQuestionOnIpv4) {
+ info.lastAdvertisedOnIpv4TimeMs = info.lastSentTimeMs;
+ } else {
+ info.lastAdvertisedOnIpv6TimeMs = info.lastSentTimeMs;
+ }
}
// Different RecordInfos may the contain the same record
if (!answerRecords.contains(info.record)) {
@@ -729,7 +742,8 @@
@Nullable String hostname,
boolean replyUnicastEnabled, long now, @NonNull Set<RecordInfo<?>> answerInfo,
@NonNull Set<RecordInfo<?>> additionalAnswerInfo,
- @NonNull List<MdnsRecord> knownAnswerRecords) {
+ @NonNull List<MdnsRecord> knownAnswerRecords,
+ boolean isQuestionOnIpv4) {
boolean hasDnsSdPtrRecordAnswer = false;
boolean hasDnsSdSrvRecordAnswer = false;
boolean hasFullyOwnedNameMatch = false;
@@ -778,10 +792,20 @@
// TODO: responses to probe queries should bypass this check and only ensure the
// reply is sent 250ms after the last sent time (RFC 6762 p.15)
- if (!(replyUnicastEnabled && question.isUnicastReplyRequested())
- && info.lastAdvertisedTimeMs > 0L
- && now - info.lastAdvertisedTimeMs < MIN_MULTICAST_REPLY_INTERVAL_MS) {
- continue;
+ if (!(replyUnicastEnabled && question.isUnicastReplyRequested())) {
+ if (isQuestionOnIpv4) { // IPv4
+ if (info.lastAdvertisedOnIpv4TimeMs > 0L
+ && now - info.lastAdvertisedOnIpv4TimeMs
+ < MIN_MULTICAST_REPLY_INTERVAL_MS) {
+ continue;
+ }
+ } else { // IPv6
+ if (info.lastAdvertisedOnIpv6TimeMs > 0L
+ && now - info.lastAdvertisedOnIpv6TimeMs
+ < MIN_MULTICAST_REPLY_INTERVAL_MS) {
+ continue;
+ }
+ }
}
answerInfo.add(info);
@@ -1305,7 +1329,8 @@
final long now = SystemClock.elapsedRealtime();
for (RecordInfo<?> record : registration.allRecords) {
record.lastSentTimeMs = now;
- record.lastAdvertisedTimeMs = now;
+ record.lastAdvertisedOnIpv4TimeMs = now;
+ record.lastAdvertisedOnIpv6TimeMs = now;
}
registration.sentPacketCount += sentPacketCount;
}
diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt b/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt
index 271cc65..bdefbf5 100644
--- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt
+++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt
@@ -1003,6 +1003,58 @@
}
@Test
+ fun testGetReply_ipv4AndIpv6Queries_ipv4AndIpv6Replies() {
+ val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME, makeFlags())
+ repository.initWithService(TEST_SERVICE_ID_1, TEST_SERVICE_1, setOf(TEST_SUBTYPE))
+ val query = makeQuery(TYPE_PTR to arrayOf("_testservice", "_tcp", "local"))
+
+ val srcIpv4 = InetSocketAddress(parseNumericAddress("192.0.2.123"), 5353)
+ val replyIpv4 = repository.getReply(query, srcIpv4)
+ val srcIpv6 = InetSocketAddress(parseNumericAddress("2001:db8::123"), 5353)
+ val replyIpv6 = repository.getReply(query, srcIpv6)
+
+ assertNotNull(replyIpv4)
+ assertEquals(MdnsConstants.getMdnsIPv4Address(), replyIpv4.destination.address)
+ assertEquals(MdnsConstants.MDNS_PORT, replyIpv4.destination.port)
+ assertNotNull(replyIpv6)
+ assertEquals(MdnsConstants.getMdnsIPv6Address(), replyIpv6.destination.address)
+ assertEquals(MdnsConstants.MDNS_PORT, replyIpv6.destination.port)
+ }
+
+ @Test
+ fun testGetReply_twoIpv4Queries_theSecondReplyIsThrottled() {
+ val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME, makeFlags())
+ repository.initWithService(TEST_SERVICE_ID_1, TEST_SERVICE_1, setOf(TEST_SUBTYPE))
+ val query = makeQuery(TYPE_PTR to arrayOf("_testservice", "_tcp", "local"))
+
+ val srcIpv4 = InetSocketAddress(parseNumericAddress("192.0.2.123"), 5353)
+ val firstReplyIpv4 = repository.getReply(query, srcIpv4)
+ val secondReply = repository.getReply(query, srcIpv4)
+
+ assertNotNull(firstReplyIpv4)
+ assertEquals(MdnsConstants.getMdnsIPv4Address(), firstReplyIpv4.destination.address)
+ assertEquals(MdnsConstants.MDNS_PORT, firstReplyIpv4.destination.port)
+ assertNull(secondReply)
+ }
+
+
+ @Test
+ fun testGetReply_twoIpv6Queries_theSecondReplyIsThrottled() {
+ val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME, makeFlags())
+ repository.initWithService(TEST_SERVICE_ID_1, TEST_SERVICE_1, setOf(TEST_SUBTYPE))
+ val query = makeQuery(TYPE_PTR to arrayOf("_testservice", "_tcp", "local"))
+
+ val srcIpv6 = InetSocketAddress(parseNumericAddress("2001:db8::123"), 5353)
+ val firstReplyIpv6 = repository.getReply(query, srcIpv6)
+ val secondReply = repository.getReply(query, srcIpv6)
+
+ assertNotNull(firstReplyIpv6)
+ assertEquals(MdnsConstants.getMdnsIPv6Address(), firstReplyIpv6.destination.address)
+ assertEquals(MdnsConstants.MDNS_PORT, firstReplyIpv6.destination.port)
+ assertNull(secondReply)
+ }
+
+ @Test
fun testGetConflictingServices() {
val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME, makeFlags())
repository.addService(TEST_SERVICE_ID_1, TEST_SERVICE_1, null /* ttl */)