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

Commit 8ac63405 authored by Mike Yu's avatar Mike Yu
Browse files

Avoid keeping sending queries to an invalid nameserver 

When a nameserver is known to be not workable due to wrong setup,
e.g. invalid address or network unreachable, it should be considered
unusable, just like what the code does for a nameserver which always
times out.

This change is beneficial to:
  [1] Fix the bug which the code can never mark all of the
      nameservers as unusable in some edge cases.
  [2] Decrease the noise of internal_error in the metrics.

Bug: 144828038
Test: cd packages/modules/DnsResolver && atest
Change-Id: Id84c49690a7ae1dbd209f9f7751120052efd6c13
parent de6bd960

res_stats.cpp

+2 −2
Original line number Diff line number Diff line
@@ -118,11 +118,11 @@ static bool res_stats_usable_server(const res_params* params, res_stats* stats) 
    android_net_res_stats_aggregate(stats, &successes, &errors, &timeouts, &internal_errors,

                                    &rtt_avg, &last_sample_time);

    if (successes >= 0 && errors >= 0 && timeouts >= 0) {

        int total = successes + errors + timeouts;

        int total = successes + errors + timeouts + internal_errors;

        LOG(INFO) << __func__ << ": NS stats: S " << successes << " + E " << errors << " + T "

                  << timeouts << " + I " << internal_errors << " = " << total

                  << ", rtt = " << rtt_avg << ", min_samples = " << unsigned(params->min_samples);

        if (total >= params->min_samples && (errors > 0 || timeouts > 0)) {

        if (total >= params->min_samples) {

            int success_rate = successes * 100 / total;

            LOG(INFO) << __func__ << ": success rate " << success_rate;

            if (success_rate < params->success_threshold) {

tests/dns_responder/dns_responder_client_ndk.h

+3 −0
Original line number Diff line number Diff line
@@ -63,10 +63,12 @@ class DnsResponderClient { 
    static void SetupMappings(unsigned num_hosts, const std::vector<std::string>& domains,

                              std::vector<Mapping>* mappings);



    // This function is deprecated. Please use SetResolversFromParcel() instead.

    bool SetResolversForNetwork(const std::vector<std::string>& servers = kDefaultServers,

                                const std::vector<std::string>& domains = kDefaultSearchDomains,

                                const std::vector<int>& params = kDefaultParams);



    // This function is deprecated. Please use SetResolversFromParcel() instead.

    bool SetResolversWithTls(const std::vector<std::string>& servers,

                             const std::vector<std::string>& searchDomains,

                             const std::vector<int>& params, const std::string& name) {
@@ -75,6 +77,7 @@ class DnsResponderClient { 
        return SetResolversWithTls(servers, searchDomains, params, servers, name);

    }



    // This function is deprecated. Please use SetResolversFromParcel() instead.

    bool SetResolversWithTls(const std::vector<std::string>& servers,

                             const std::vector<std::string>& searchDomains,

                             const std::vector<int>& params,

tests/resolv_integration_test.cpp

+90 −2
Original line number Diff line number Diff line
@@ -84,6 +84,7 @@ extern "C" int android_getaddrinfofornet(const char* hostname, const char* servn 


using aidl::android::net::IDnsResolver;

using aidl::android::net::INetd;

using aidl::android::net::ResolverParamsParcel;

using android::base::ParseInt;

using android::base::StringPrintf;

using android::base::unique_fd;
@@ -912,6 +913,94 @@ TEST_F(ResolverTest, GetAddrInfoV6_concurrent) { 
    EXPECT_EQ(0, wait_for_pending_req_timeout_count);

}



TEST_F(ResolverTest, SkipBadServersDueToInternalError) {

    constexpr char listen_addr1[] = "fe80::1";

    constexpr char listen_addr2[] = "255.255.255.255";

    constexpr char listen_addr3[] = "127.0.0.3";



    test::DNSResponder dns(listen_addr3);

    ASSERT_TRUE(dns.startServer());



    ResolverParamsParcel parcel = DnsResponderClient::GetDefaultResolverParamsParcel();

    parcel.servers = {listen_addr1, listen_addr2, listen_addr3};



    // Bad servers can be distinguished after two attempts.

    parcel.minSamples = 2;

    ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel));



    // Start querying five times.

    for (int i = 0; i < 5; i++) {

        std::string hostName = StringPrintf("hello%d.com.", i);

        dns.addMapping(hostName, ns_type::ns_t_a, "1.2.3.4");

        const addrinfo hints = {.ai_family = AF_INET, .ai_socktype = SOCK_DGRAM};

        EXPECT_TRUE(safe_getaddrinfo(hostName.c_str(), nullptr, &hints) != nullptr);

    }



    std::vector<std::string> res_servers;

    std::vector<std::string> res_domains;

    std::vector<std::string> res_tls_servers;

    res_params res_params;

    std::vector<ResolverStats> res_stats;

    int wait_for_pending_req_timeout_count;

    ASSERT_TRUE(DnsResponderClient::GetResolverInfo(

            mDnsClient.resolvService(), TEST_NETID, &res_servers, &res_domains, &res_tls_servers,

            &res_params, &res_stats, &wait_for_pending_req_timeout_count));



    // Verify the result by means of the statistics.

    EXPECT_EQ(res_stats[0].successes, 0);

    EXPECT_EQ(res_stats[1].successes, 0);

    EXPECT_EQ(res_stats[2].successes, 5);

    EXPECT_EQ(res_stats[0].internal_errors, 2);

    EXPECT_EQ(res_stats[1].internal_errors, 2);

    EXPECT_EQ(res_stats[2].internal_errors, 0);

}



TEST_F(ResolverTest, SkipBadServersDueToTimeout) {

    constexpr char listen_addr1[] = "127.0.0.3";

    constexpr char listen_addr2[] = "127.0.0.4";



    // Set dns1 non-responsive and dns2 workable.

    test::DNSResponder dns1(listen_addr1, test::kDefaultListenService, static_cast<ns_rcode>(-1));

    test::DNSResponder dns2(listen_addr2);

    dns1.setResponseProbability(0.0);

    ASSERT_TRUE(dns1.startServer());

    ASSERT_TRUE(dns2.startServer());



    ResolverParamsParcel parcel = DnsResponderClient::GetDefaultResolverParamsParcel();

    parcel.servers = {listen_addr1, listen_addr2};



    // Bad servers can be distinguished after two attempts.

    parcel.minSamples = 2;

    ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel));



    // Start querying five times.

    for (int i = 0; i < 5; i++) {

        std::string hostName = StringPrintf("hello%d.com.", i);

        dns1.addMapping(hostName, ns_type::ns_t_a, "1.2.3.4");

        dns2.addMapping(hostName, ns_type::ns_t_a, "1.2.3.5");

        const addrinfo hints = {.ai_family = AF_INET, .ai_socktype = SOCK_DGRAM};

        EXPECT_TRUE(safe_getaddrinfo(hostName.c_str(), nullptr, &hints) != nullptr);

    }



    std::vector<std::string> res_servers;

    std::vector<std::string> res_domains;

    std::vector<std::string> res_tls_servers;

    res_params res_params;

    std::vector<ResolverStats> res_stats;

    int wait_for_pending_req_timeout_count;

    ASSERT_TRUE(DnsResponderClient::GetResolverInfo(

            mDnsClient.resolvService(), TEST_NETID, &res_servers, &res_domains, &res_tls_servers,

            &res_params, &res_stats, &wait_for_pending_req_timeout_count));



    // Verify the result by means of the statistics as well as the query counts.

    EXPECT_EQ(res_stats[0].successes, 0);

    EXPECT_EQ(res_stats[1].successes, 5);

    EXPECT_EQ(res_stats[0].timeouts, 2);

    EXPECT_EQ(res_stats[1].timeouts, 0);

    EXPECT_EQ(dns1.queries().size(), 2U);

    EXPECT_EQ(dns2.queries().size(), 5U);

}



TEST_F(ResolverTest, EmptySetup) {

    std::vector<std::string> servers;

    std::vector<std::string> domains;
@@ -3494,8 +3583,7 @@ TEST_F(ResolverTest, ConnectTlsServerTimeout) { 
    ScopedSystemProperties scopedSystemProperties(kDotConnectTimeoutMsFlag, "100");



    // Set up resolver to opportunistic mode with the default configuration.

    const aidl::android::net::ResolverParamsParcel parcel =

            DnsResponderClient::GetDefaultResolverParamsParcel();

    const ResolverParamsParcel parcel = DnsResponderClient::GetDefaultResolverParamsParcel();

    ASSERT_TRUE(mDnsClient.SetResolversFromParcel(parcel));

    EXPECT_TRUE(WaitForPrivateDnsValidation(tls.listen_address(), true));

    dns.clearQueries();