Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5b30e710 authored by Luke Huang's avatar Luke Huang
Browse files

Improve query sleep time for getaddrinfo parallel lookup

Move the sleep function behind cache checking to avoid unnecessary cost.

Bug: 155046588
Test: atest
Merged-In: I0e4fe4da5b56375589ef4b617819bfaae8abcf33
Change-Id: Ic62383d52d76ba9dec79d9745a6e7b7a79cd35de
(cherry picked from commit 5e97b8ad)
parent b8012f78
Loading
Loading
Loading
Loading
+13 −7
Original line number Original line Diff line number Diff line
@@ -53,6 +53,7 @@
#include <sys/un.h>
#include <sys/un.h>
#include <unistd.h>
#include <unistd.h>


#include <chrono>
#include <future>
#include <future>


#include <android-base/logging.h>
#include <android-base/logging.h>
@@ -1607,7 +1608,8 @@ struct QueryResult {
    NetworkDnsEventReported event;
    NetworkDnsEventReported event;
};
};


QueryResult doQuery(const char* name, res_target* t, res_state res) {
QueryResult doQuery(const char* name, res_target* t, res_state res,
                    std::chrono::milliseconds sleepTimeMs) {
    HEADER* hp = (HEADER*)(void*)t->answer.data();
    HEADER* hp = (HEADER*)(void*)t->answer.data();


    hp->rcode = NOERROR;  // default
    hp->rcode = NOERROR;  // default
@@ -1643,7 +1645,7 @@ QueryResult doQuery(const char* name, res_target* t, res_state res) {
    ResState res_temp = fromResState(*res, &event);
    ResState res_temp = fromResState(*res, &event);


    int rcode = NOERROR;
    int rcode = NOERROR;
    n = res_nsend(&res_temp, buf, n, t->answer.data(), anslen, &rcode, 0);
    n = res_nsend(&res_temp, buf, n, t->answer.data(), anslen, &rcode, 0, sleepTimeMs);
    if (n < 0 || hp->rcode != NOERROR || ntohs(hp->ancount) == 0) {
    if (n < 0 || hp->rcode != NOERROR || ntohs(hp->ancount) == 0) {
        // if the query choked with EDNS0, retry without EDNS0
        // if the query choked with EDNS0, retry without EDNS0
        if ((res_temp.netcontext_flags &
        if ((res_temp.netcontext_flags &
@@ -1671,13 +1673,17 @@ QueryResult doQuery(const char* name, res_target* t, res_state res) {
static int res_queryN_parallel(const char* name, res_target* target, res_state res, int* herrno) {
static int res_queryN_parallel(const char* name, res_target* target, res_state res, int* herrno) {
    std::vector<std::future<QueryResult>> results;
    std::vector<std::future<QueryResult>> results;
    results.reserve(2);
    results.reserve(2);
    std::chrono::milliseconds sleepTimeMs{};
    for (res_target* t = target; t; t = t->next) {
    for (res_target* t = target; t; t = t->next) {
        results.emplace_back(std::async(std::launch::async, doQuery, name, t, res));
        results.emplace_back(std::async(std::launch::async, doQuery, name, t, res, sleepTimeMs));
        // Avoiding gateways drop packets if queries are sent too close together
        // Avoiding gateways drop packets if queries are sent too close together
        int sleepTime = android::net::Experiments::getInstance()->getFlag(
        // Only needed if we have multiple queries in a row.
        if (t->next) {
            int sleepFlag = android::net::Experiments::getInstance()->getFlag(
                    "parallel_lookup_sleep_time", SLEEP_TIME_MS);
                    "parallel_lookup_sleep_time", SLEEP_TIME_MS);
        if (sleepTime > 1000) sleepTime = 1000;
            if (sleepFlag > 1000) sleepFlag = 1000;
        if (t->next) usleep(sleepTime * 1000);
            sleepTimeMs = std::chrono::milliseconds(sleepFlag);
        }
    }
    }


    int ancount = 0;
    int ancount = 0;
+9 −1
Original line number Original line Diff line number Diff line
@@ -76,6 +76,8 @@


#define LOG_TAG "resolv"
#define LOG_TAG "resolv"


#include <chrono>

#include <sys/param.h>
#include <sys/param.h>
#include <sys/socket.h>
#include <sys/socket.h>
#include <sys/time.h>
#include <sys/time.h>
@@ -115,6 +117,7 @@
#include "stats.pb.h"
#include "stats.pb.h"
#include "util.h"
#include "util.h"


using namespace std::chrono_literals;
// TODO: use the namespace something like android::netd_resolv for libnetd_resolv
// TODO: use the namespace something like android::netd_resolv for libnetd_resolv
using android::base::ErrnoError;
using android::base::ErrnoError;
using android::base::Result;
using android::base::Result;
@@ -408,7 +411,7 @@ static DnsQueryEvent* addDnsQueryEvent(NetworkDnsEventReported* event) {
}
}


int res_nsend(res_state statp, const uint8_t* buf, int buflen, uint8_t* ans, int anssiz, int* rcode,
int res_nsend(res_state statp, const uint8_t* buf, int buflen, uint8_t* ans, int anssiz, int* rcode,
              uint32_t flags) {
              uint32_t flags, std::chrono::milliseconds sleepTimeMs) {
    LOG(DEBUG) << __func__;
    LOG(DEBUG) << __func__;


    // Should not happen
    // Should not happen
@@ -448,6 +451,11 @@ int res_nsend(res_state statp, const uint8_t* buf, int buflen, uint8_t* ans, int
        return -ESRCH;
        return -ESRCH;
    }
    }


    // If parallel_lookup is enabled, it might be required to wait some time to avoid
    // gateways drop packets if queries are sent too close together
    if (sleepTimeMs != 0ms) {
        std::this_thread::sleep_for(sleepTimeMs);
    }
    // DoT
    // DoT
    if (!(statp->netcontext_flags & NET_CONTEXT_FLAG_USE_LOCAL_NAMESERVERS)) {
    if (!(statp->netcontext_flags & NET_CONTEXT_FLAG_USE_LOCAL_NAMESERVERS)) {
        bool fallback = false;
        bool fallback = false;
+2 −1
Original line number Original line Diff line number Diff line
@@ -140,7 +140,8 @@ int res_nsearch(res_state, const char*, int, int, uint8_t*, int, int*);
int res_nquerydomain(res_state, const char*, const char*, int, int, uint8_t*, int, int*);
int res_nquerydomain(res_state, const char*, const char*, int, int, uint8_t*, int, int*);
int res_nmkquery(int op, const char* qname, int cl, int type, const uint8_t* data, int datalen,
int res_nmkquery(int op, const char* qname, int cl, int type, const uint8_t* data, int datalen,
                 uint8_t* buf, int buflen, int netcontext_flags);
                 uint8_t* buf, int buflen, int netcontext_flags);
int res_nsend(res_state, const uint8_t*, int, uint8_t*, int, int*, uint32_t);
int res_nsend(res_state statp, const uint8_t* buf, int buflen, uint8_t* ans, int anssiz, int* rcode,
              uint32_t flags, std::chrono::milliseconds sleepTimeMs = {});
int res_nopt(res_state, int, uint8_t*, int, int);
int res_nopt(res_state, int, uint8_t*, int, int);


int getaddrinfo_numeric(const char* hostname, const char* servname, addrinfo hints,
int getaddrinfo_numeric(const char* hostname, const char* servname, addrinfo hints,
+47 −0
Original line number Original line Diff line number Diff line
@@ -4779,6 +4779,8 @@ TEST_F(ResolverTest, GetAddrInfoParallelLookupTimeout) {
    StartDns(neverRespondDns, records);
    StartDns(neverRespondDns, records);
    ScopedSystemProperties scopedSystemProperties(
    ScopedSystemProperties scopedSystemProperties(
            "persist.device_config.netd_native.parallel_lookup", "1");
            "persist.device_config.netd_native.parallel_lookup", "1");
    // The default value of parallel_lookup_sleep_time should be very small
    // that we can ignore in this test case.
    // Re-setup test network to make experiment flag take effect.
    // Re-setup test network to make experiment flag take effect.
    resetNetwork();
    resetNetwork();


@@ -4797,3 +4799,48 @@ TEST_F(ResolverTest, GetAddrInfoParallelLookupTimeout) {
            << "took time should approximate equal timeout";
            << "took time should approximate equal timeout";
    EXPECT_EQ(2U, GetNumQueries(neverRespondDns, host_name));
    EXPECT_EQ(2U, GetNumQueries(neverRespondDns, host_name));
}
}

TEST_F(ResolverTest, GetAddrInfoParallelLookupSleepTime) {
    constexpr char listen_addr[] = "127.0.0.4";
    constexpr int TIMING_TOLERANCE_MS = 200;
    const std::vector<DnsRecord> records = {
            {kHelloExampleCom, ns_type::ns_t_a, kHelloExampleComAddrV4},
            {kHelloExampleCom, ns_type::ns_t_aaaa, kHelloExampleComAddrV6},
    };
    const std::vector<int> params = {300, 25, 8, 8, 1000 /* BASE_TIMEOUT_MSEC */,
                                     1 /* retry count */};
    test::DNSResponder dns(listen_addr);
    StartDns(dns, records);
    ScopedSystemProperties scopedSystemProperties1(
            "persist.device_config.netd_native.parallel_lookup", "1");
    constexpr int PARALLEL_LOOKUP_SLEEP_TIME_MS = 500;
    ScopedSystemProperties scopedSystemProperties2(
            "persist.device_config.netd_native.parallel_lookup_sleep_time",
            std::to_string(PARALLEL_LOOKUP_SLEEP_TIME_MS));
    // Re-setup test network to make experiment flag take effect.
    resetNetwork();

    ASSERT_TRUE(mDnsClient.SetResolversForNetwork({listen_addr}, kDefaultSearchDomains, params));
    dns.clearQueries();

    // Expect the safe_getaddrinfo_time_taken() might take ~500ms to return because we set
    // parallel_lookup_sleep_time to 500ms.
    const addrinfo hints = {.ai_family = AF_UNSPEC, .ai_socktype = SOCK_DGRAM};
    auto [result, timeTakenMs] = safe_getaddrinfo_time_taken(kHelloExampleCom, nullptr, hints);

    EXPECT_NE(nullptr, result);
    EXPECT_THAT(ToStrings(result), testing::UnorderedElementsAreArray(
                                           {kHelloExampleComAddrV4, kHelloExampleComAddrV6}));
    EXPECT_NEAR(PARALLEL_LOOKUP_SLEEP_TIME_MS, timeTakenMs, TIMING_TOLERANCE_MS)
            << "took time should approximate equal timeout";
    EXPECT_EQ(2U, GetNumQueries(dns, kHelloExampleCom));

    // Expect the PARALLEL_LOOKUP_SLEEP_TIME_MS won't affect the query under cache hit case.
    dns.clearQueries();
    std::tie(result, timeTakenMs) = safe_getaddrinfo_time_taken(kHelloExampleCom, nullptr, hints);
    EXPECT_NE(nullptr, result);
    EXPECT_THAT(ToStrings(result), testing::UnorderedElementsAreArray(
                                           {kHelloExampleComAddrV4, kHelloExampleComAddrV6}));
    EXPECT_GT(PARALLEL_LOOKUP_SLEEP_TIME_MS, timeTakenMs);
    EXPECT_EQ(0U, GetNumQueries(dns, kHelloExampleCom));
}