Skip to content

Commit

Permalink
dnsdist: Fix warnings from clang-tidy
Browse files Browse the repository at this point in the history
  • Loading branch information
@rgacogne
rgacogne committed Jan 23, 2024
1 parent 97a2ad3 commit 4ed255e
Show file tree
Hide file tree
Showing 10 changed files with 173 additions and 177 deletions.
76 changes: 31 additions & 45 deletions pdns/bpf-filter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,52 +33,38 @@

#include "misc.hh"

int bpf_create_map(enum bpf_map_type map_type, int key_size, int value_size,
int max_entries, int map_flags);
int bpf_update_elem(int fd, void *key, void *value, unsigned long long flags);
int bpf_lookup_elem(int fd, void *key, void *value);
int bpf_delete_elem(int fd, void *key);
int bpf_get_next_key(int fd, void *key, void *next_key);

int bpf_prog_load(enum bpf_prog_type prog_type,
const struct bpf_insn *insns, int insn_len,
const char *license, int kern_version);

int bpf_obj_pin(int fd, const char *pathname);
int bpf_obj_get(const char *pathname);

static __u64 ptr_to_u64(void *ptr)
static __u64 ptr_to_u64(const void *ptr)
{
return (__u64) (unsigned long) ptr;
}

/* these can be static as they are not declared in libbpf.h: */
static int bpf_pin_map(int fd, const std::string& path)
static int bpf_pin_map(int descriptor, const std::string& path)
{
union bpf_attr attr;
memset(&attr, 0, sizeof(attr));
attr.bpf_fd = fd;
attr.pathname = ptr_to_u64(const_cast<char*>(path.c_str()));
attr.bpf_fd = descriptor;
attr.pathname = ptr_to_u64(path.c_str());
return syscall(SYS_bpf, BPF_OBJ_PIN, &attr, sizeof(attr));
}

static int bpf_load_pinned_map(const std::string& path)
{
union bpf_attr attr;
memset(&attr, 0, sizeof(attr));
attr.pathname = ptr_to_u64(const_cast<char*>(path.c_str()));
attr.pathname = ptr_to_u64(path.c_str());
return syscall(SYS_bpf, BPF_OBJ_GET, &attr, sizeof(attr));
}

static void bpf_check_map_sizes(int fd, uint32_t expectedKeySize, uint32_t expectedValueSize)
static void bpf_check_map_sizes(int descriptor, uint32_t expectedKeySize, uint32_t expectedValueSize)
{
struct bpf_map_info info;
uint32_t info_len = sizeof(info);
memset(&info, 0, sizeof(info));

union bpf_attr attr;
memset(&attr, 0, sizeof(attr));
attr.info.bpf_fd = fd;
attr.info.bpf_fd = descriptor;
attr.info.info_len = info_len;
attr.info.info = ptr_to_u64(&info);

Expand All @@ -97,8 +83,8 @@ static void bpf_check_map_sizes(int fd, uint32_t expectedKeySize, uint32_t expec
}
}

int bpf_create_map(enum bpf_map_type map_type, int key_size, int value_size,
int max_entries, int map_flags)
static int bpf_create_map(enum bpf_map_type map_type, int key_size, int value_size,

Check warning on line 86 in pdns/bpf-filter.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, dnsdist) 

4 adjacent parameters of 'bpf_create_map' of similar type ('int') are easily swapped by mistake (bugprone-easily-swappable-parameters - Level=Warning)

Check warning on line 86 in pdns/bpf-filter.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, dnsdist) 

4 adjacent parameters of 'bpf_create_map' of similar type ('int') are easily swapped by mistake (bugprone-easily-swappable-parameters - Level=Warning)
int max_entries, int map_flags)
{
union bpf_attr attr;
memset(&attr, 0, sizeof(attr));
Expand All @@ -110,49 +96,49 @@ int bpf_create_map(enum bpf_map_type map_type, int key_size, int value_size,
return syscall(SYS_bpf, BPF_MAP_CREATE, &attr, sizeof(attr));
}

int bpf_update_elem(int fd, void *key, void *value, unsigned long long flags)
static int bpf_update_elem(int descriptor, void *key, void *value, unsigned long long flags)
{
union bpf_attr attr;
memset(&attr, 0, sizeof(attr));
attr.map_fd = fd;
attr.map_fd = descriptor;
attr.key = ptr_to_u64(key);
attr.value = ptr_to_u64(value);
attr.flags = flags;
return syscall(SYS_bpf, BPF_MAP_UPDATE_ELEM, &attr, sizeof(attr));
}

int bpf_lookup_elem(int fd, void *key, void *value)
static int bpf_lookup_elem(int descriptor, void *key, void *value)
{
union bpf_attr attr;
memset(&attr, 0, sizeof(attr));
attr.map_fd = fd;
attr.map_fd = descriptor;
attr.key = ptr_to_u64(key);
attr.value = ptr_to_u64(value);
return syscall(SYS_bpf, BPF_MAP_LOOKUP_ELEM, &attr, sizeof(attr));
}

int bpf_delete_elem(int fd, void *key)
static int bpf_delete_elem(int descriptor, void *key)
{
union bpf_attr attr;
memset(&attr, 0, sizeof(attr));
attr.map_fd = fd;
attr.map_fd = descriptor;
attr.key = ptr_to_u64(key);
return syscall(SYS_bpf, BPF_MAP_DELETE_ELEM, &attr, sizeof(attr));
}

int bpf_get_next_key(int fd, void *key, void *next_key)
static int bpf_get_next_key(int descriptor, void *key, void *next_key)
{
union bpf_attr attr;
memset(&attr, 0, sizeof(attr));
attr.map_fd = fd;
attr.map_fd = descriptor;
attr.key = ptr_to_u64(key);
attr.next_key = ptr_to_u64(next_key);
return syscall(SYS_bpf, BPF_MAP_GET_NEXT_KEY, &attr, sizeof(attr));
}

int bpf_prog_load(enum bpf_prog_type prog_type,
const struct bpf_insn *insns, int prog_len,
const char *license, int kern_version)
static int bpf_prog_load(enum bpf_prog_type prog_type,
const struct bpf_insn *insns, int prog_len,
const char *license, int kern_version)
{
char log_buf[65535];
union bpf_attr attr;
Expand Down Expand Up @@ -351,15 +337,15 @@ BPFFilter::Map::Map(const BPFFilter::MapConfiguration& config, BPFFilter::MapFor

static FDWrapper loadProgram(const struct bpf_insn* filter, size_t filterSize)
{
auto fd = FDWrapper(bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER,
filter,
filterSize,
"GPL",
0));
if (fd.getHandle() == -1) {
auto descriptor = FDWrapper(bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER,
filter,
filterSize,

Check warning on line 342 in pdns/bpf-filter.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, dnsdist) 

narrowing conversion from 'size_t' (aka 'unsigned long') to signed type 'int' is implementation-defined (bugprone-narrowing-conversions - Level=Warning)

Check warning on line 342 in pdns/bpf-filter.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, dnsdist) 

narrowing conversion from 'size_t' (aka 'unsigned long') to signed type 'int' is implementation-defined (bugprone-narrowing-conversions - Level=Warning)
"GPL",
0));
if (descriptor.getHandle() == -1) {
throw std::runtime_error("error loading BPF filter: " + stringerror());
}
return fd;
return descriptor;
}


Expand Down Expand Up @@ -442,8 +428,8 @@ BPFFilter::BPFFilter(std::unordered_map<std::string, MapConfiguration>& configs,

void BPFFilter::addSocket(int sock)
{
int fd = d_mainfilter.getHandle();
int res = setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, &fd, sizeof(fd));
int descriptor = d_mainfilter.getHandle();
int res = setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, &descriptor, sizeof(descriptor));

if (res != 0) {
throw std::runtime_error("Error attaching BPF filter to this socket: " + stringerror());
Expand All @@ -452,8 +438,8 @@ void BPFFilter::addSocket(int sock)

void BPFFilter::removeSocket(int sock)
{
int fd = d_mainfilter.getHandle();
int res = setsockopt(sock, SOL_SOCKET, SO_DETACH_BPF, &fd, sizeof(fd));
int descriptor = d_mainfilter.getHandle();
int res = setsockopt(sock, SOL_SOCKET, SO_DETACH_BPF, &descriptor, sizeof(descriptor));

if (res != 0) {
throw std::runtime_error("Error detaching BPF filter from this socket: " + stringerror());
Expand Down
177 changes: 93 additions & 84 deletions pdns/dnsdist-lua.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,10 +307,101 @@ static bool checkConfigurationTime(const std::string& name)
return false;
}

using newserver_t = LuaAssociativeTable<boost::variant<bool, std::string, LuaArray<std::string>, LuaArray<std::shared_ptr<XskSocket>>, DownstreamState::checkfunc_t>>;

static void handleNewServerHealthCheckParameters(boost::optional<newserver_t>& vars, DownstreamState::Config& config)
{
std::string valueStr;

if (getOptionalValue<std::string>(vars, "checkInterval", valueStr) > 0) {
config.checkInterval = static_cast<unsigned int>(std::stoul(valueStr));
}

if (getOptionalValue<std::string>(vars, "healthCheckMode", valueStr) > 0) {
const auto& mode = valueStr;
if (pdns_iequals(mode, "auto")) {
config.availability = DownstreamState::Availability::Auto;
}
else if (pdns_iequals(mode, "lazy")) {
config.availability = DownstreamState::Availability::Lazy;
}
else if (pdns_iequals(mode, "up")) {
config.availability = DownstreamState::Availability::Up;
}
else if (pdns_iequals(mode, "down")) {
config.availability = DownstreamState::Availability::Down;
}
else {
warnlog("Ignoring unknown value '%s' for 'healthCheckMode' on 'newServer'", mode);
}
}

if (getOptionalValue<std::string>(vars, "checkName", valueStr) > 0) {
config.checkName = DNSName(valueStr);
}

getOptionalValue<std::string>(vars, "checkType", config.checkType);
getOptionalIntegerValue("newServer", vars, "checkClass", config.checkClass);
getOptionalValue<DownstreamState::checkfunc_t>(vars, "checkFunction", config.checkFunction);
getOptionalIntegerValue("newServer", vars, "checkTimeout", config.checkTimeout);
getOptionalValue<bool>(vars, "checkTCP", config.d_tcpCheck);
getOptionalValue<bool>(vars, "setCD", config.setCD);
getOptionalValue<bool>(vars, "mustResolve", config.mustResolve);

if (getOptionalValue<std::string>(vars, "lazyHealthCheckSampleSize", valueStr) > 0) {
const auto& value = std::stoi(valueStr);
checkParameterBound("lazyHealthCheckSampleSize", value);
config.d_lazyHealthCheckSampleSize = value;
}

if (getOptionalValue<std::string>(vars, "lazyHealthCheckMinSampleCount", valueStr) > 0) {
const auto& value = std::stoi(valueStr);
checkParameterBound("lazyHealthCheckMinSampleCount", value);
config.d_lazyHealthCheckMinSampleCount = value;
}

if (getOptionalValue<std::string>(vars, "lazyHealthCheckThreshold", valueStr) > 0) {
const auto& value = std::stoi(valueStr);
checkParameterBound("lazyHealthCheckThreshold", value, std::numeric_limits<uint8_t>::max());
config.d_lazyHealthCheckThreshold = value;
}

if (getOptionalValue<std::string>(vars, "lazyHealthCheckFailedInterval", valueStr) > 0) {
const auto& value = std::stoi(valueStr);
checkParameterBound("lazyHealthCheckFailedInterval", value);
config.d_lazyHealthCheckFailedInterval = value;
}

getOptionalValue<bool>(vars, "lazyHealthCheckUseExponentialBackOff", config.d_lazyHealthCheckUseExponentialBackOff);

if (getOptionalValue<std::string>(vars, "lazyHealthCheckMaxBackOff", valueStr) > 0) {
const auto& value = std::stoi(valueStr);
checkParameterBound("lazyHealthCheckMaxBackOff", value);
config.d_lazyHealthCheckMaxBackOff = value;
}

if (getOptionalValue<std::string>(vars, "lazyHealthCheckMode", valueStr) > 0) {
const auto& mode = valueStr;
if (pdns_iequals(mode, "TimeoutOnly")) {
config.d_lazyHealthCheckMode = DownstreamState::LazyHealthCheckMode::TimeoutOnly;
}
else if (pdns_iequals(mode, "TimeoutOrServFail")) {
config.d_lazyHealthCheckMode = DownstreamState::LazyHealthCheckMode::TimeoutOrServFail;
}
else {
warnlog("Ignoring unknown value '%s' for 'lazyHealthCheckMode' on 'newServer'", mode);
}
}

getOptionalValue<bool>(vars, "lazyHealthCheckWhenUpgraded", config.d_upgradeToLazyHealthChecks);

getOptionalIntegerValue("newServer", vars, "maxCheckFailures", config.maxCheckFailures);
getOptionalIntegerValue("newServer", vars, "rise", config.minRiseSuccesses);
}

// NOLINTNEXTLINE(readability-function-cognitive-complexity): this function declares Lua bindings, even with a good refactoring it will likely blow up the threshold
static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck)
{
using newserver_t = LuaAssociativeTable<boost::variant<bool, std::string, LuaArray<std::string>, LuaArray<std::shared_ptr<XskSocket>>, DownstreamState::checkfunc_t>>;
luaCtx.writeFunction("inClientStartup", [client]() {
return client && !g_configurationDone;
});
Expand Down Expand Up @@ -406,9 +497,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck)
getOptionalIntegerValue("newServer", vars, "tcpSendTimeout", config.tcpSendTimeout);
getOptionalIntegerValue("newServer", vars, "tcpRecvTimeout", config.tcpRecvTimeout);

if (getOptionalValue<std::string>(vars, "checkInterval", valueStr) > 0) {
config.checkInterval = static_cast<unsigned int>(std::stoul(valueStr));
}
handleNewServerHealthCheckParameters(vars, config);

bool fastOpen{false};
if (getOptionalValue<bool>(vars, "tcpFastOpen", fastOpen) > 0) {
Expand All @@ -430,93 +519,13 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck)
config.id = boost::uuids::string_generator()(valueStr);
}

if (getOptionalValue<std::string>(vars, "healthCheckMode", valueStr) > 0) {
const auto& mode = valueStr;
if (pdns_iequals(mode, "auto")) {
config.availability = DownstreamState::Availability::Auto;
}
else if (pdns_iequals(mode, "lazy")) {
config.availability = DownstreamState::Availability::Lazy;
}
else if (pdns_iequals(mode, "up")) {
config.availability = DownstreamState::Availability::Up;
}
else if (pdns_iequals(mode, "down")) {
config.availability = DownstreamState::Availability::Down;
}
else {
warnlog("Ignoring unknown value '%s' for 'healthCheckMode' on 'newServer'", mode);
}
}

if (getOptionalValue<std::string>(vars, "checkName", valueStr) > 0) {
config.checkName = DNSName(valueStr);
}

getOptionalValue<std::string>(vars, "checkType", config.checkType);
getOptionalIntegerValue("newServer", vars, "checkClass", config.checkClass);
getOptionalValue<DownstreamState::checkfunc_t>(vars, "checkFunction", config.checkFunction);
getOptionalIntegerValue("newServer", vars, "checkTimeout", config.checkTimeout);
getOptionalValue<bool>(vars, "checkTCP", config.d_tcpCheck);
getOptionalValue<bool>(vars, "setCD", config.setCD);
getOptionalValue<bool>(vars, "mustResolve", config.mustResolve);

if (getOptionalValue<std::string>(vars, "lazyHealthCheckSampleSize", valueStr) > 0) {
const auto& value = std::stoi(valueStr);
checkParameterBound("lazyHealthCheckSampleSize", value);
config.d_lazyHealthCheckSampleSize = value;
}

if (getOptionalValue<std::string>(vars, "lazyHealthCheckMinSampleCount", valueStr) > 0) {
const auto& value = std::stoi(valueStr);
checkParameterBound("lazyHealthCheckMinSampleCount", value);
config.d_lazyHealthCheckMinSampleCount = value;
}

if (getOptionalValue<std::string>(vars, "lazyHealthCheckThreshold", valueStr) > 0) {
const auto& value = std::stoi(valueStr);
checkParameterBound("lazyHealthCheckThreshold", value, std::numeric_limits<uint8_t>::max());
config.d_lazyHealthCheckThreshold = value;
}

if (getOptionalValue<std::string>(vars, "lazyHealthCheckFailedInterval", valueStr) > 0) {
const auto& value = std::stoi(valueStr);
checkParameterBound("lazyHealthCheckFailedInterval", value);
config.d_lazyHealthCheckFailedInterval = value;
}

getOptionalValue<bool>(vars, "lazyHealthCheckUseExponentialBackOff", config.d_lazyHealthCheckUseExponentialBackOff);

if (getOptionalValue<std::string>(vars, "lazyHealthCheckMaxBackOff", valueStr) > 0) {
const auto& value = std::stoi(valueStr);
checkParameterBound("lazyHealthCheckMaxBackOff", value);
config.d_lazyHealthCheckMaxBackOff = value;
}

if (getOptionalValue<std::string>(vars, "lazyHealthCheckMode", valueStr) > 0) {
const auto& mode = valueStr;
if (pdns_iequals(mode, "TimeoutOnly")) {
config.d_lazyHealthCheckMode = DownstreamState::LazyHealthCheckMode::TimeoutOnly;
}
else if (pdns_iequals(mode, "TimeoutOrServFail")) {
config.d_lazyHealthCheckMode = DownstreamState::LazyHealthCheckMode::TimeoutOrServFail;
}
else {
warnlog("Ignoring unknown value '%s' for 'lazyHealthCheckMode' on 'newServer'", mode);
}
}

getOptionalValue<bool>(vars, "lazyHealthCheckWhenUpgraded", config.d_upgradeToLazyHealthChecks);

getOptionalValue<bool>(vars, "useClientSubnet", config.useECS);
getOptionalValue<bool>(vars, "useProxyProtocol", config.useProxyProtocol);
getOptionalValue<bool>(vars, "proxyProtocolAdvertiseTLS", config.d_proxyProtocolAdvertiseTLS);
getOptionalValue<bool>(vars, "disableZeroScope", config.disableZeroScope);
getOptionalValue<bool>(vars, "ipBindAddrNoPort", config.ipBindAddrNoPort);

getOptionalIntegerValue("newServer", vars, "addXPF", config.xpfRRCode);
getOptionalIntegerValue("newServer", vars, "maxCheckFailures", config.maxCheckFailures);
getOptionalIntegerValue("newServer", vars, "rise", config.minRiseSuccesses);

getOptionalValue<bool>(vars, "reconnectOnUp", config.reconnectOnUp);

Expand Down
2 changes: 1 addition & 1 deletion pdns/dnsdist-tcp.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ IncomingTCPConnectionState::QueryProcessingResult IncomingTCPConnectionState::ha
{
/* this pointer will be invalidated the second the buffer is resized, don't hold onto it! */
const dnsheader_aligned dnsHeader(query.data());
if (!checkQueryHeaders(dnsHeader.get(), *d_ci.cs)) {
if (!checkQueryHeaders(*dnsHeader.get(), *d_ci.cs)) {

Check warning on line 677 in pdns/dnsdist-tcp.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, dnsdist) 

redundant get() call on smart pointer (readability-redundant-smartptr-get - Level=Warning)

Check warning on line 677 in pdns/dnsdist-tcp.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, dnsdist) 

redundant get() call on smart pointer (readability-redundant-smartptr-get - Level=Warning)
return QueryProcessingResult::InvalidHeaders;
}

Expand Down
Loading

0 comments on commit 4ed255e

Please sign in to comment.