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

Commit d1d734f1 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
Change-Id: I0e4fe4da5b56375589ef4b617819bfaae8abcf33
parent a893556b

getaddrinfo.cpp

+13 −7
Original line number Diff line number Diff line
@@ -53,6 +53,7 @@ 
#include <sys/un.h>

#include <unistd.h>



#include <chrono>

#include <future>



#include <android-base/logging.h>
@@ -1607,7 +1608,8 @@ struct QueryResult { 
    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();



    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);



    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 the query choked with EDNS0, retry without EDNS0

        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) {

    std::vector<std::future<QueryResult>> results;

    results.reserve(2);

    std::chrono::milliseconds sleepTimeMs{};

    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

        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);

        if (sleepTime > 1000) sleepTime = 1000;

        if (t->next) usleep(sleepTime * 1000);

            if (sleepFlag > 1000) sleepFlag = 1000;

            sleepTimeMs = std::chrono::milliseconds(sleepFlag);

        }

    }



    int ancount = 0;

res_send.cpp

+9 −1
Original line number Diff line number Diff line
@@ -76,6 +76,8 @@ 


#define LOG_TAG "resolv"



#include <chrono>



#include <sys/param.h>

#include <sys/socket.h>

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

#include "util.h"



using namespace std::chrono_literals;

// TODO: use the namespace something like android::netd_resolv for libnetd_resolv

using android::base::ErrnoError;

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,

              uint32_t flags) {

              uint32_t flags, std::chrono::milliseconds sleepTimeMs) {

    LOG(DEBUG) << __func__;



    // 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;

    }



    // 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

    if (!(statp->netcontext_flags & NET_CONTEXT_FLAG_USE_LOCAL_NAMESERVERS)) {

        bool fallback = false;

resolv_private.h

+2 −1
Original line number 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_nmkquery(int op, const char* qname, int cl, int type, const uint8_t* data, int datalen,

                 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 getaddrinfo_numeric(const char* hostname, const char* servname, addrinfo hints,

tests/resolv_integration_test.cpp

+47 −0
Original line number Diff line number Diff line
@@ -4779,6 +4779,8 @@ TEST_F(ResolverTest, GetAddrInfoParallelLookupTimeout) { 
    StartDns(neverRespondDns, records);

    ScopedSystemProperties scopedSystemProperties(

            "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.

    resetNetwork();
@@ -4797,3 +4799,48 @@ TEST_F(ResolverTest, GetAddrInfoParallelLookupTimeout) { 
            << "took time should approximate equal timeout";

    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));

}