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

Commit 581636ab authored by Mike Yu's avatar Mike Yu
Browse files

Clean up the flag parallel_lookup_release 

The flag has been enabled by default for more than 2 years since
aosp/1517564. This change removes not only the flag but also
the dead code res_queryN().

Bug: 277699672
Test: atest
Change-Id: I3445f9ccdbb9746c0f78ad8b50e3193c6a1868a3
parent c7f1d4ef

Experiments.h

+0 −1
Original line number Diff line number Diff line
@@ -66,7 +66,6 @@ class Experiments { 
            "max_cache_entries",

            "max_queries_global",

            "mdns_resolution",

            "parallel_lookup_release",

            "parallel_lookup_sleep_time",

            "retransmission_time_interval",

            "retry_count",

getaddrinfo.cpp

+5 −88
Original line number Diff line number Diff line
@@ -151,7 +151,6 @@ static bool files_getaddrinfo(const size_t netid, const char* name, const addrin 
static int _find_src_addr(const struct sockaddr*, struct sockaddr*, unsigned, uid_t,

                          bool allow_v6_linklocal);



static int res_queryN(const char* name, res_target* target, ResState* res, int* herrno);

static int res_searchN(const char* name, res_target* target, ResState* res, int* herrno);

static int res_querydomainN(const char* name, const char* domain, res_target* target, ResState* res,

                            int* herrno);
@@ -1603,6 +1602,8 @@ struct QueryResult { 
    NetworkDnsEventReported event;

};



// Formulate a normal query, send, and await answer.

// Caller must parse answer and determine whether it answers the question.

QueryResult doQuery(const char* name, res_target* t, ResState* res,

                    std::chrono::milliseconds sleepTimeMs) {

    HEADER* hp = (HEADER*)(void*)t->answer.data();
@@ -1640,7 +1641,6 @@ QueryResult doQuery(const char* name, res_target* t, ResState* res, 
    int rcode = NOERROR;

    n = res_nsend(&res_temp, {buf, n}, {t->answer.data(), anslen}, &rcode, 0, sleepTimeMs);

    if (n < 0 || hp->rcode != NOERROR || ntohs(hp->ancount) == 0) {

        // To ensure that the rcode handling is identical to res_queryN().

        if (rcode != RCODE_TIMEOUT) rcode = hp->rcode;

        // if the query choked with EDNS0, retry without EDNS0

        if ((res_temp.netcontext_flags &
@@ -1666,6 +1666,8 @@ QueryResult doQuery(const char* name, res_target* t, ResState* res, 


}  // namespace



// This function runs doQuery() for each res_target in parallel.

// The `target`, which is set in dns_getaddrinfo(), contains at most two res_target.

static int res_queryN_parallel(const char* name, res_target* target, ResState* res, int* herrno) {

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

    results.reserve(2);
@@ -1705,91 +1707,6 @@ static int res_queryN_parallel(const char* name, res_target* target, ResState* r 
    return ancount;

}



static int res_queryN_wrapper(const char* name, res_target* target, ResState* res, int* herrno) {

    const bool parallel_lookup = Experiments::getInstance()->getFlag("parallel_lookup_release", 1);

    if (parallel_lookup) return res_queryN_parallel(name, target, res, herrno);



    return res_queryN(name, target, res, herrno);

}



/*

 * Formulate a normal query, send, and await answer.

 * Returned answer is placed in supplied buffer "answer".

 * Perform preliminary check of answer, returning success only

 * if no error is indicated and the answer count is nonzero.

 * Return the size of the response on success, -1 on error.

 * Error number is left in *herrno.

 *

 * Caller must parse answer and determine whether it answers the question.

 */

static int res_queryN(const char* name, res_target* target, ResState* res, int* herrno) {

    uint8_t buf[MAXPACKET];

    int n;

    struct res_target* t;

    int rcode;

    int ancount;



    assert(name != NULL);

    /* XXX: target may be NULL??? */



    rcode = NOERROR;

    ancount = 0;



    for (t = target; t; t = t->next) {

        HEADER* hp = (HEADER*)(void*)t->answer.data();

        bool retried = false;

    again:

        hp->rcode = NOERROR; /* default */



        /* make it easier... */

        int cl = t->qclass;

        int type = t->qtype;

        const int anslen = t->answer.size();



        LOG(DEBUG) << __func__ << ": (" << cl << ", " << type << ")";

        n = res_nmkquery(QUERY, name, cl, type, {}, buf, res->netcontext_flags);

        if (n > 0 &&

            (res->netcontext_flags &

             (NET_CONTEXT_FLAG_USE_DNS_OVER_TLS | NET_CONTEXT_FLAG_USE_EDNS)) &&

            !retried)  // TODO:  remove the retry flag and provide a sufficient test coverage.

            n = res_nopt(res, n, buf, anslen);

        if (n <= 0) {

            LOG(ERROR) << __func__ << ": res_nmkquery failed";

            *herrno = NO_RECOVERY;

            return n;

        }



        n = res_nsend(res, {buf, n}, {t->answer.data(), anslen}, &rcode, 0);

        if (n < 0 || hp->rcode != NOERROR || ntohs(hp->ancount) == 0) {

            // Record rcode from DNS response header only if no timeout.

            // Keep rcode timeout for reporting later if any.

            if (rcode != RCODE_TIMEOUT) rcode = hp->rcode;  // record most recent error

            // if the query choked with EDNS0, retry without EDNS0 that when the server

            // has no response, resovler won't retry and do nothing. Even fallback to UDP,

            // we also has the same symptom if EDNS is enabled.

            if ((res->netcontext_flags &

                 (NET_CONTEXT_FLAG_USE_DNS_OVER_TLS | NET_CONTEXT_FLAG_USE_EDNS)) &&

                (res->flags & RES_F_EDNS0ERR) && !retried) {

                LOG(DEBUG) << __func__ << ": retry without EDNS0";

                retried = true;

                goto again;

            }

            LOG(INFO) << __func__ << ": rcode=" << rcode << ", ancount=" << ntohs(hp->ancount);

            continue;

        }



        ancount += ntohs(hp->ancount);



        t->n = n;

    }



    if (ancount == 0) {

        *herrno = getHerrnoFromRcode(rcode);

        return -1;

    }

    return ancount;

}



/*

 * Formulate a normal query, send, and retrieve answer in supplied buffer.

 * Return the size of the response on success, -1 on error.
@@ -1936,5 +1853,5 @@ static int res_querydomainN(const char* name, const char* domain, res_target* ta 
        }

        snprintf(nbuf, sizeof(nbuf), "%s.%s", name, domain);

    }

    return res_queryN_wrapper(longname, target, res, herrno);

    return res_queryN_parallel(longname, target, res, herrno);

}

res_query.cpp

+2 −2
Original line number Diff line number Diff line
@@ -141,8 +141,8 @@ again: 
        LOG(INFO) << __func__ << ": send error";



        // Note that rcodes SERVFAIL, NOTIMP, REFUSED may cause res_nquery() to return a general

        // error code EAI_AGAIN, but mapping the error code from rcode as res_queryN() does for

        // getaddrinfo(). Different rcodes trigger different behaviors:

        // error code EAI_AGAIN, but mapping the error code from rcode as res_queryN_parallel()

        // does for getaddrinfo(). Different rcodes trigger different behaviors:

        //

        // - SERVFAIL, NOTIMP, REFUSED

        //   These result in send_dg() returning 0, causing res_nsend() to try the next

resolv_private.h

+1 −1
Original line number Diff line number Diff line
@@ -160,7 +160,7 @@ struct ResState { 
 * Error code extending h_errno codes defined in bionic/libc/include/netdb.h.

 *

 * This error code, including legacy h_errno, is returned from res_nquery(), res_nsearch(),

 * res_nquerydomain(), res_queryN(), res_searchN() and res_querydomainN() for DNS metrics.

 * res_nquerydomain(), res_queryN_parallel(), res_searchN() and res_querydomainN() for DNS metrics.

 *

 * TODO: Consider mapping legacy and extended h_errno into a unified resolver error code mapping.

 */

tests/resolv_integration_test.cpp

+0 −6
Original line number Diff line number Diff line
@@ -6199,11 +6199,6 @@ TEST_F(ResolverTest, GetAddrInfoParallelLookupTimeout) { 
    test::DNSResponder neverRespondDns(kDefaultServer, "53", static_cast<ns_rcode>(-1));

    neverRespondDns.setResponseProbability(0.0);

    StartDns(neverRespondDns, records);

    ScopedSystemProperties sp(kParallelLookupReleaseFlag, "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();



    ASSERT_TRUE(mDnsClient.SetResolversFromParcel(

            ResolverParams::Builder().setDotServers({}).setParams(params).build()));
@@ -6233,7 +6228,6 @@ TEST_F(ResolverTest, GetAddrInfoParallelLookupSleepTime) { 
            300, 25, 8, 8, 1000 /* BASE_TIMEOUT_MSEC */, 1 /* retry count */};

    test::DNSResponder dns(kDefaultServer);

    StartDns(dns, records);

    ScopedSystemProperties sp1(kParallelLookupReleaseFlag, "1");

    constexpr int PARALLEL_LOOKUP_SLEEP_TIME_MS = 500;

    ScopedSystemProperties sp2(kParallelLookupSleepTimeFlag,

                               std::to_string(PARALLEL_LOOKUP_SLEEP_TIME_MS));