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

Commit 44fd92c3 authored by Lorenzo Colitti's avatar Lorenzo Colitti
Browse files

Make resolv_set_nameservers take the AIDL parcel directly. 

Currently, the code unparcels the parcel and passes its elements
to resolv_set_nameservers as function parameters. These are then
copied to other data structures.

Instead of doing this, just pass the parcel around directly. It
simplifies the code, and it makes it much easier to extend the
parcel in the future.

Test: m
Change-Id: I924b05980dae5c8a343f612f09dbd83d6f01e287
parent 5cfb3f44

ResolverController.cpp

+1 −12
Original line number Diff line number Diff line
@@ -227,18 +227,7 @@ int ResolverController::setResolverConfiguration(const ResolverParamsParcel& res 
        }

    }



    res_params res_params = {};

    res_params.sample_validity = resolverParams.sampleValiditySeconds;

    res_params.success_threshold = resolverParams.successThreshold;

    res_params.min_samples = resolverParams.minSamples;

    res_params.max_samples = resolverParams.maxSamples;

    res_params.base_timeout_msec = resolverParams.baseTimeoutMsec;

    res_params.retry_count = resolverParams.retryCount;



    return resolv_set_nameservers(resolverParams.netId, resolverParams.servers,

                                  resolverParams.domains, res_params,

                                  resolverParams.resolverOptions, resolverParams.transportTypes,

                                  resolverParams.meteredNetwork);

    return resolv_set_nameservers(resolverParams);

}



int ResolverController::getResolverInfo(int32_t netId, std::vector<std::string>* servers,

res_cache.cpp

+18 −13
Original line number Diff line number Diff line
@@ -67,6 +67,7 @@ 


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

using aidl::android::net::ResolverOptionsParcel;

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

using android::net::DnsQueryEvent;

using android::net::DnsStats;

using android::net::Experiments;
@@ -1640,11 +1641,9 @@ std::vector<std::string> getCustomizedTableByName(const size_t netid, const char 
    return result;

}



int resolv_set_nameservers(unsigned netid, const std::vector<std::string>& servers,

                           const std::vector<std::string>& domains, const res_params& params,

                           const std::optional<ResolverOptionsParcel> optionalResolverOptions,

                           const std::vector<int32_t>& transportTypes, bool metered) {

    std::vector<std::string> nameservers = filter_nameservers(servers);

int resolv_set_nameservers(const ResolverParamsParcel& params) {

    const unsigned netid = params.netId;

    std::vector<std::string> nameservers = filter_nameservers(params.servers);

    const int numservers = static_cast<int>(nameservers.size());



    LOG(DEBUG) << __func__ << ": netId = " << netid << ", numservers = " << numservers;
@@ -1664,7 +1663,14 @@ int resolv_set_nameservers(unsigned netid, const std::vector<std::string>& serve 
    if (netconfig == nullptr) return -ENONET;



    uint8_t old_max_samples = netconfig->params.max_samples;

    netconfig->params = params;



    memset(&netconfig->params, 0, sizeof(netconfig->params));

    netconfig->params.sample_validity = params.sampleValiditySeconds;

    netconfig->params.success_threshold = params.successThreshold;

    netconfig->params.min_samples = params.minSamples;

    netconfig->params.max_samples = params.maxSamples;

    netconfig->params.base_timeout_msec = params.baseTimeoutMsec;

    netconfig->params.retry_count = params.retryCount;



    // This check must always be true, but add a protection against OEMs configure negative values

    // for retry_count and base_timeout_msec.
@@ -1679,7 +1685,7 @@ int resolv_set_nameservers(unsigned netid, const std::vector<std::string>& serve 
                (retransmissionInterval <= 0) ? RES_TIMEOUT : retransmissionInterval;

    }



    if (!resolv_is_nameservers_equal(netconfig->nameservers, nameservers)) {

    if (!resolv_is_nameservers_equal(netconfig->nameservers, params.servers)) {

        // free current before adding new

        free_nameservers_locked(netconfig);

        netconfig->nameservers = std::move(nameservers);
@@ -1701,7 +1707,7 @@ int resolv_set_nameservers(unsigned netid, const std::vector<std::string>& serve 


    // Always update the search paths. Cache-flushing however is not necessary,

    // since the stored cache entries do contain the domain, not just the host name.

    netconfig->search_domains = filter_domains(domains);

    netconfig->search_domains = filter_domains(params.domains);



    // Setup stats for cleartext dns servers.

    if (!netconfig->dnsStats.setAddrs(netconfig->nameserverSockAddrs, PROTO_TCP) ||
@@ -1709,11 +1715,10 @@ int resolv_set_nameservers(unsigned netid, const std::vector<std::string>& serve 
        LOG(WARNING) << __func__ << ": netid = " << netid << ", failed to set dns stats";

        return -EINVAL;

    }

    netconfig->transportTypes = transportTypes;

    netconfig->metered = metered;

    if (optionalResolverOptions.has_value()) {

        const ResolverOptionsParcel& resolverOptions = optionalResolverOptions.value();

        return netconfig->setOptions(resolverOptions);

    netconfig->transportTypes = std::move(params.transportTypes);

    netconfig->metered = params.meteredNetwork;

    if (params.resolverOptions.has_value()) {

        return netconfig->setOptions(params.resolverOptions.value());

    }

    return 0;

}

resolv_cache.h

+1 −5
Original line number Diff line number Diff line
@@ -77,11 +77,7 @@ void _resolv_cache_query_failed(unsigned netid, std::span<const uint8_t> query, 
std::vector<std::string> getCustomizedTableByName(const size_t netid, const char* hostname);



// Sets name servers for a given network.

// TODO: Pass all of ResolverParamsParcel and remove the res_params argument.

int resolv_set_nameservers(unsigned netid, const std::vector<std::string>& servers,

                           const std::vector<std::string>& domains, const res_params& params,

                           std::optional<aidl::android::net::ResolverOptionsParcel> resolverOptions,

                           const std::vector<int32_t>& transportTypes = {}, bool metered = false);

int resolv_set_nameservers(const aidl::android::net::ResolverParamsParcel& params);



// Sets options for a given network.

int resolv_set_options(unsigned netid, const aidl::android::net::ResolverOptionsParcel& options);

tests/Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ cc_test_library { 
    ],

    static_libs: [

        "netd_aidl_interface-lateststable-ndk",

        "dnsresolver_aidl_interface-lateststable-ndk",

        "libconnectivity_native_test_utils",

        "libmodules-utils-build",

        "libnetd_test_dnsresponder_ndk",

tests/resolv_test_utils.h

+29 −0
Original line number Diff line number Diff line
@@ -26,6 +26,7 @@ 
#include <vector>



#include <aidl/android/net/INetd.h>

#include <aidl/android/net/ResolverParamsParcel.h>

#include <android-base/properties.h>

#include <android-modules-utils/sdk_level.h>

#include <firewall.h>
@@ -33,6 +34,7 @@ 
#include <netdutils/InternetAddresses.h>



#include "dns_responder/dns_responder.h"

#include "params.h"

#include "util.h"



class ScopeBlockedUIDRule {
@@ -433,6 +435,33 @@ void SetMdnsRoute(); 
void RemoveMdnsRoute();

void AllowNetworkInBackground(int uid, bool allow);



// Local definition to avoid including resolv_cache.h.

int resolv_set_nameservers(const aidl::android::net::ResolverParamsParcel& params);



// For testing only. Production code passes the parcel down directly.

inline int resolv_set_nameservers(

        unsigned netid, const std::vector<std::string>& servers,

        const std::vector<std::string>& domains, const res_params& res_params,

        std::optional<aidl::android::net::ResolverOptionsParcel> resolverOptions,

        const std::vector<int32_t>& transportTypes = {}, bool metered = false) {

    aidl::android::net::ResolverParamsParcel params;

    params.netId = netid;

    params.servers = servers;

    params.domains = domains;

    params.resolverOptions = resolverOptions;

    params.transportTypes = transportTypes;

    params.meteredNetwork = metered;



    params.sampleValiditySeconds = res_params.sample_validity;

    params.successThreshold = res_params.success_threshold;

    params.minSamples = res_params.min_samples;

    params.maxSamples = res_params.max_samples;

    params.baseTimeoutMsec = res_params.base_timeout_msec;

    params.retryCount = res_params.retry_count;



    return resolv_set_nameservers(params);

}



#define SKIP_IF_BEFORE_T                                                         \

    do {                                                                         \

        if (!isAtLeastT()) {                                                     \