Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix query counts #1322

Merged
merged 2 commits into from
Apr 2, 2022
Merged

Fix query counts #1322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a24e187
Add individual reply counters as well as their sum to the API + ensur…
DL6ER Mar 26, 2022
8734b60
Ensure we subtract from the old reply counter when reply type changes
DL6ER Mar 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions src/api/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,13 @@ void getStats(const int *sock)
ssend(*sock, "dns_queries_all_types %i\n", sumalltypes);

// Send individual reply type counters
ssend(*sock, "reply_NODATA %i\nreply_NXDOMAIN %i\nreply_CNAME %i\nreply_IP %i\n",
counters->reply[REPLY_NODATA], counters->reply[REPLY_NXDOMAIN],
counters->reply[REPLY_CNAME], counters->reply[REPLY_IP]);
int sumallreplies = 0;
for(enum reply_type reply = REPLY_UNKNOWN; reply < QUERY_REPLY_MAX; reply++)
{
ssend(*sock, "reply_%s %i\n", get_query_reply_str(reply), counters->reply[reply]);
sumallreplies += counters->reply[reply];
}
ssend(*sock, "dns_queries_all_replies %i\n", sumallreplies);
ssend(*sock, "privacy_level %i\n", config.privacylevel);
}
else
Expand Down
12 changes: 9 additions & 3 deletions src/database/query-table.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,14 @@ void DB_read_queries(void)
if(type < 100)
{
// Mapped query type
query->type = type;
if(type >= TYPE_A && type < TYPE_MAX)
query->type = type;
else
{
// Invalid query type
logg("DB warn: Query type %d is invalid.", type);
continue;
}
}
else
{
Expand Down Expand Up @@ -900,8 +907,7 @@ void DB_read_queries(void)
client->lastQuery = queryTimeStamp;

// Handle type counters
if(type >= TYPE_A && type < TYPE_MAX)
counters->querytype[type-1]++;
counters->querytype[query->type-1]++;

// Update overTime data
overTime[timeidx].total++;
Expand Down
71 changes: 31 additions & 40 deletions src/dnsmasq_interface.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
#include "database/message-table.h"

// Private prototypes
static const char *reply_status_str[QUERY_REPLY_MAX+1];
static void print_flags(const unsigned int flags);
#define query_set_reply(flags, type, addr, query, response) _query_set_reply(flags, type, addr, query, response, __FILE__, __LINE__)
static void _query_set_reply(const unsigned int flags, const enum reply_type reply, const union all_addr *addr, queriesData* query,
Expand Down Expand Up @@ -664,9 +663,6 @@ bool _FTL_new_query(const unsigned int flags, const char *name,
id, queryID, short_path(file), line);
}

// Update counters
counters->querytype[querytype-1]++;

// Update overTime
const unsigned int timeidx = getOverTimeID(querytimestamp);

Expand Down Expand Up @@ -720,6 +716,7 @@ bool _FTL_new_query(const unsigned int flags, const char *name,
query->flags.response_calculated = false;
// Initialize reply type
query->reply = REPLY_UNKNOWN;
counters->reply[REPLY_UNKNOWN]++;
// Store DNSSEC result for this domain
query->dnssec = DNSSEC_UNSPECIFIED;
// Every domain is insecure in the beginning. It can get secure or bogus
Expand Down Expand Up @@ -755,6 +752,9 @@ bool _FTL_new_query(const unsigned int flags, const char *name,
client->lastQuery = querytimestamp;
client->numQueriesARP++;

// Update counters
counters->querytype[querytype-1]++;

// Process interface information of client (if available)
// Skip interface name length 1 to skip "-". No real interface should
// have a name with a length of 1...
Expand Down Expand Up @@ -1449,7 +1449,7 @@ static bool _FTL_check_blocking(int queryID, int domainID, int clientID, const c
logg("Blocking %s as %s is %s", domainstr, blockedDomain, blockingreason);

if(force_next_DNS_reply != 0)
logg("Forcing next reply to %s", reply_status_str[force_next_DNS_reply]);
logg("Forcing next reply to %s", get_query_reply_str(force_next_DNS_reply));
}
else if(db_okay)
{
Expand Down Expand Up @@ -2570,95 +2570,86 @@ void print_flags(const unsigned int flags)
free(flagstr);
}

static const char *reply_status_str[QUERY_REPLY_MAX+1] = {
"UNKNOWN",
"NODATA",
"NXDOMAIN",
"CNAME",
"IP",
"DOMAIN",
"RRNAME",
"SERVFAIL",
"REFUSED",
"NOTIMP",
"OTHER",
"DNSSEC",
"NONE",
"BLOB",
"MAX"
};

static void _query_set_reply(const unsigned int flags, const enum reply_type reply,
const union all_addr *addr,
queriesData *query, const struct timeval response,
const char *file, const int line)
{
enum reply_type new_reply = REPLY_UNKNOWN;
// If reply is set, we use it directly instead of interpreting the flags
if(reply != 0)
{
query->reply = reply;
new_reply = reply;
}
// else: Iterate through possible values by analyzing both the flags and the addr bits
yubiuser marked this conversation as resolved.
Show resolved Hide resolved
else if(flags & F_NEG ||
(flags & F_NOERR && !(flags & (F_IPV4 | F_IPV6))) || // <-- FTL_make_answer() when no A or AAAA is added
force_next_DNS_reply == REPLY_NXDOMAIN ||
force_next_DNS_reply == REPLY_NODATA)
(flags & F_NOERR && !(flags & (F_IPV4 | F_IPV6))) || // <-- FTL_make_answer() when no A or AAAA is added
force_next_DNS_reply == REPLY_NXDOMAIN ||
force_next_DNS_reply == REPLY_NODATA)
{
if(flags & F_NXDOMAIN || force_next_DNS_reply == REPLY_NXDOMAIN)
// NXDOMAIN
query->reply = REPLY_NXDOMAIN;
new_reply = REPLY_NXDOMAIN;
else
// NODATA(-IPv6)
query->reply = REPLY_NODATA;
new_reply = REPLY_NODATA;
}
else if(flags & F_CNAME)
// <CNAME>
query->reply = REPLY_CNAME;
new_reply = REPLY_CNAME;
else if(flags & F_REVERSE)
// reserve lookup
query->reply = REPLY_DOMAIN;
new_reply = REPLY_DOMAIN;
else if(flags & F_RRNAME)
// TXT query
query->reply = REPLY_RRNAME;
new_reply = REPLY_RRNAME;
else if((flags & F_RCODE && addr != NULL) || force_next_DNS_reply == REPLY_REFUSED)
{
if((addr != NULL && addr->log.rcode == REFUSED)
|| force_next_DNS_reply == REPLY_REFUSED )
{
// REFUSED query
query->reply = REPLY_REFUSED;
new_reply = REPLY_REFUSED;
}
else if(addr != NULL && addr->log.rcode == SERVFAIL)
{
// SERVFAIL query
query->reply = REPLY_SERVFAIL;
new_reply = REPLY_SERVFAIL;
}
yubiuser marked this conversation as resolved.
Show resolved Hide resolved
}
else if(flags & F_KEYTAG)
query->reply = REPLY_DNSSEC;
new_reply = REPLY_DNSSEC;
else if(force_next_DNS_reply == REPLY_NONE)
{
query->reply = REPLY_NONE;
new_reply = REPLY_NONE;
}
else if(flags & (F_IPV4 | F_IPV6))
{
// IP address
query->reply = REPLY_IP;
new_reply = REPLY_IP;
}
else
{
// Other binary, possibly proprietry, data
query->reply = REPLY_BLOB;
new_reply = REPLY_BLOB;
}

if(config.debug & DEBUG_QUERIES)
{
const char *path = short_path(file);
logg("Set reply to %s (%d) in %s:%d", reply_status_str[query->reply], query->reply,
logg("Set reply to %s (%d) in %s:%d", get_query_reply_str(query->reply), query->reply,
path, line);
if(query->reply != REPLY_UNKNOWN && query->reply != new_reply)
logg("Reply of query %i was %s now changing to %s", query->id,
get_query_reply_str(query->reply), get_query_reply_str(new_reply));
}

counters->reply[query->reply]++;
// Subtract from old reply counter
counters->reply[query->reply]--;
// Add to new reply counter
counters->reply[new_reply]++;
// Store reply type
query->reply = new_reply;

// Save response time
// Skipped internally if already computed
Expand Down
25 changes: 18 additions & 7 deletions test/test_suite.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,13 +412,24 @@
#[[ ${lines[8]} == "clients_ever_seen 8" ]]
#[[ ${lines[9]} == "unique_clients 8" ]]
[[ ${lines[10]} == "dns_queries_all_types 47" ]]
[[ ${lines[11]} == "reply_NODATA 0" ]]
[[ ${lines[12]} == "reply_NXDOMAIN 1" ]]
[[ ${lines[13]} == "reply_CNAME 5" ]]
[[ ${lines[14]} == "reply_IP 23" ]]
[[ ${lines[15]} == "privacy_level 0" ]]
[[ ${lines[16]} == "status enabled" ]]
[[ ${lines[17]} == "" ]]
[[ ${lines[11]} == "reply_UNKNOWN 0" ]]
[[ ${lines[12]} == "reply_NODATA 0" ]]
[[ ${lines[13]} == "reply_NXDOMAIN 1" ]]
[[ ${lines[14]} == "reply_CNAME 3" ]]
[[ ${lines[15]} == "reply_IP 23" ]]
[[ ${lines[16]} == "reply_DOMAIN 0" ]]
[[ ${lines[17]} == "reply_RRNAME 5" ]]
[[ ${lines[18]} == "reply_SERVFAIL 0" ]]
[[ ${lines[19]} == "reply_REFUSED 0" ]]
[[ ${lines[20]} == "reply_NOTIMP 0" ]]
[[ ${lines[21]} == "reply_OTHER 0" ]]
[[ ${lines[22]} == "reply_DNSSEC 5" ]]
[[ ${lines[23]} == "reply_NONE 0" ]]
[[ ${lines[24]} == "reply_BLOB 10" ]]
[[ ${lines[25]} == "dns_queries_all_replies 47" ]]
[[ ${lines[26]} == "privacy_level 0" ]]
[[ ${lines[27]} == "status enabled" ]]
[[ ${lines[28]} == "" ]]
}

# Here and below: It is not meaningful to assume a particular order
Expand Down