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

Commit 9100c294 authored by Mike Yu's avatar Mike Yu
Browse files

Improve prioritizing DNS servers to handle no permission failure 

The quality of a DNS server should not dramatically drop if
the failure is due to no permission.

Bug: 137169582
Test: ran resolv_unit_test
Test: ran resolv_integration_test with the sorting enabled
Change-Id: Idad2ef2c34e708bd4bd64c50669de21f1abe3ac0
parent eca28ba4

DnsStats.cpp

+11 −1
Original line number Diff line number Diff line
@@ -110,8 +110,17 @@ void StatsRecords::push(const Record& record) { 


    // Update the quality factors.

    mSkippedCount = 0;



    // Because failures due to no permission can't prove that the quality of DNS server is bad,

    // skip the penalty update. The average latency, however, has been updated. For short-latency

    // servers, it will be fine. For long-latency servers, their average latency will be

    // decreased but the latency-based algorithm will adjust their average latency back to the

    // right range after few attempts when network is not restricted.

    // The check is synced from isNetworkRestricted() in res_send.cpp.

    if (record.linux_errno != EPERM) {

        updatePenalty(record);

    }

}



void StatsRecords::updateStatsData(const Record& record, const bool add) {

    const int rcode = record.rcode;
@@ -194,6 +203,7 @@ bool DnsStats::addStats(const IPSockAddr& ipSockAddr, const DnsQueryEvent& recor 
        if (serverSockAddr == ipSockAddr) {

            const StatsRecords::Record rec = {

                    .rcode = record.rcode(),

                    .linux_errno = record.linux_errno(),

                    .latencyUs = microseconds(record.latency_micros()),

            };

            statsRecords.push(rec);

DnsStats.h

+2 −1
Original line number Diff line number Diff line
@@ -68,7 +68,8 @@ struct StatsData { 
class StatsRecords {

  public:

    struct Record {

        int rcode;

        int rcode = 0;        // NS_R_NO_ERROR

        int linux_errno = 0;  // SYS_NO_ERROR

        std::chrono::microseconds latencyUs;

    };

DnsStatsTest.cpp

+21 −2
Original line number Diff line number Diff line
@@ -59,8 +59,16 @@ class StatsRecordsTest : public ::testing::Test {}; 
TEST_F(StatsRecordsTest, PushRecord) {

    const IPSockAddr server = IPSockAddr::toIPSockAddr("127.0.0.2", 53);

    constexpr size_t size = 3;

    const StatsRecords::Record recordNoError = {NS_R_NO_ERROR, 10ms};

    const StatsRecords::Record recordTimeout = {NS_R_TIMEOUT, 250ms};

    const StatsRecords::Record recordNoError = {

            .rcode = NS_R_NO_ERROR,

            .linux_errno = 0,

            .latencyUs{10ms},

    };

    const StatsRecords::Record recordTimeout = {

            .rcode = NS_R_TIMEOUT,

            .linux_errno = 0,

            .latencyUs{250ms},

    };



    StatsRecords sr(server, size);

    EXPECT_EQ(sr.getStatsData(), makeStatsData(server, 0, 0ms, {}));
@@ -424,6 +432,17 @@ TEST_F(DnsStatsTest, GetServers_SortingByLatency) { 
    EXPECT_THAT(mDnsStats.getSortedServers(PROTO_UDP),

                testing::ElementsAreArray({server2, server3, server1, server4}));



    // Add some internal_error records with permission error to server2.

    // The internal_error won't cause the priority of server2 drop. (but some of the other

    // quality factors will still be counted, such as skipped_count and latency)

    auto recordFromNetworkRestricted = makeDnsQueryEvent(PROTO_UDP, NS_R_INTERNAL_ERROR, 1ms);

    recordFromNetworkRestricted.set_linux_errno(static_cast<LinuxErrno>(EPERM));

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

        EXPECT_TRUE(mDnsStats.addStats(server2, recordFromNetworkRestricted));

    }

    EXPECT_THAT(mDnsStats.getSortedServers(PROTO_UDP),

                testing::ElementsAreArray({server2, server3, server1, server4}));



    // The list of the DNS servers changed.

    EXPECT_TRUE(mDnsStats.setServers({server2, server4}, PROTO_UDP));

    EXPECT_THAT(mDnsStats.getSortedServers(PROTO_UDP),