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.

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

dnsdist: Report Lua(Response)Action failures #6283

Merged
merged 2 commits into from
Feb 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion pdns/dnsdist-lua-vars.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,15 @@ void setupLuaVars()
{"Pool", (int)DNSAction::Action::Pool},
{"None",(int)DNSAction::Action::None},
{"Delay", (int)DNSAction::Action::Delay},
{"Truncate", (int)DNSAction::Action::Truncate}
{"Truncate", (int)DNSAction::Action::Truncate},
{"ServFail", (int)DNSAction::Action::ServFail}
});

g_lua.writeVariable("DNSResponseAction", std::unordered_map<string,int>{
{"Allow", (int)DNSResponseAction::Action::Allow },
{"Delay", (int)DNSResponseAction::Action::Delay },
{"HeaderModify", (int)DNSResponseAction::Action::HeaderModify },
{"ServFail", (int)DNSResponseAction::Action::ServFail },
{"None", (int)DNSResponseAction::Action::None }
});

Expand Down
32 changes: 24 additions & 8 deletions pdns/dnsdist-lua.hh
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
*/
#pragma once

#include "dolog.hh"

class LuaAction : public DNSAction
{
public:
Expand All @@ -31,10 +33,17 @@ public:
Action operator()(DNSQuestion* dq, string* ruleresult) const override
{
std::lock_guard<std::mutex> lock(g_luamutex);
auto ret = d_func(dq);
if(ruleresult)
*ruleresult=std::get<1>(ret);
return (Action)std::get<0>(ret);
try {
auto ret = d_func(dq);
if(ruleresult)
*ruleresult=std::get<1>(ret);
return (Action)std::get<0>(ret);
} catch (std::exception &e) {
warnlog("LuaAction failed inside lua, returning ServFail: %s", e.what());
} catch (...) {
warnlog("LuaAction failed inside lua, returning ServFail: [unknown exception]");
}
return DNSAction::Action::ServFail;
}

string toString() const override
Expand All @@ -56,10 +65,17 @@ public:
Action operator()(DNSResponse* dr, string* ruleresult) const override
{
std::lock_guard<std::mutex> lock(g_luamutex);
auto ret = d_func(dr);
if(ruleresult)
*ruleresult=std::get<1>(ret);
return (Action)std::get<0>(ret);
try {
auto ret = d_func(dr);
if(ruleresult)
*ruleresult=std::get<1>(ret);
return (Action)std::get<0>(ret);
} catch (std::exception &e) {
warnlog("LuaResponseAction failed inside lua, returning ServFail: %s", e.what());
} catch (...) {
warnlog("LuaResponseAction failed inside lua, returning ServFail: [unknown exception]");
}
return DNSResponseAction::Action::ServFail;
}

string toString() const override
Expand Down
2 changes: 2 additions & 0 deletions pdns/dnsdist-snmp.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ static const oid cpuSysMSecOID[] = { DNSDIST_STATS_OID, 33 };
static const oid fdUsageOID[] = { DNSDIST_STATS_OID, 34 };
static const oid dynBlockedOID[] = { DNSDIST_STATS_OID, 35 };
static const oid dynBlockedNMGSizeOID[] = { DNSDIST_STATS_OID, 36 };
static const oid ruleServFailOID[] = { DNSDIST_STATS_OID, 37 };

static std::unordered_map<oid, DNSDistStats::entry_t> s_statsMap;

Expand Down Expand Up @@ -547,6 +548,7 @@ DNSDistSNMPAgent::DNSDistSNMPAgent(const std::string& name, const std::string& m
registerCounter64Stat("ruleDrop", ruleDropOID, OID_LENGTH(ruleDropOID), &g_stats.ruleDrop);
registerCounter64Stat("ruleNXDomain", ruleNXDomainOID, OID_LENGTH(ruleNXDomainOID), &g_stats.ruleNXDomain);
registerCounter64Stat("ruleRefused", ruleRefusedOID, OID_LENGTH(ruleRefusedOID), &g_stats.ruleRefused);
registerCounter64Stat("ruleServFail", ruleServFailOID, OID_LENGTH(ruleServFailOID), &g_stats.ruleServFail);
registerCounter64Stat("selfAnswered", selfAnsweredOID, OID_LENGTH(selfAnsweredOID), &g_stats.selfAnswered);
registerCounter64Stat("downstreamTimeouts", downstreamTimeoutsOID, OID_LENGTH(downstreamTimeoutsOID), &g_stats.downstreamTimeouts);
registerCounter64Stat("downstreamSendErrors", downstreamSendErrorsOID, OID_LENGTH(downstreamSendErrorsOID), &g_stats.downstreamSendErrors);
Expand Down
10 changes: 10 additions & 0 deletions pdns/dnsdist.cc
Original file line number Diff line number Diff line change
Expand Up @@ -992,6 +992,12 @@ bool processQuery(LocalHolders& holders, DNSQuestion& dq, string& poolname, int*
g_stats.ruleRefused++;
return true;
break;
case DNSAction::Action::ServFail:
dq.dh->rcode = RCode::ServFail;
dq.dh->qr=true;
g_stats.ruleServFail++;
return true;
break;
case DNSAction::Action::Spoof:
spoofResponseFromString(dq, ruleresult);
return true;
Expand Down Expand Up @@ -1039,6 +1045,10 @@ bool processResponse(LocalStateHolder<vector<DNSDistResponseRuleAction> >& local
case DNSResponseAction::Action::HeaderModify:
return true;
break;
case DNSResponseAction::Action::ServFail:
dr.dh->rcode = RCode::ServFail;
return true;
break;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should increment g_stats.ruleServFail here too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't increment any of the ruleXXX counters for ResponseActions right now, and I think that would be confusing because the sums will not match up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, that makes sense :)

/* non-terminal actions follow */
case DNSResponseAction::Action::Delay:
*delayMsec = static_cast<int>(pdns_stou(ruleresult)); // sorry
Expand Down
6 changes: 4 additions & 2 deletions pdns/dnsdist.hh
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ struct DNSResponse : DNSQuestion
class DNSAction
{
public:
enum class Action { Drop, Nxdomain, Refused, Spoof, Allow, HeaderModify, Pool, Delay, Truncate, None};
enum class Action { Drop, Nxdomain, Refused, Spoof, Allow, HeaderModify, Pool, Delay, Truncate, ServFail, None};
virtual Action operator()(DNSQuestion*, string* ruleresult) const =0;
virtual ~DNSAction()
{
Expand All @@ -184,7 +184,7 @@ public:
class DNSResponseAction
{
public:
enum class Action { Allow, Delay, Drop, HeaderModify, None };
enum class Action { Allow, Delay, Drop, HeaderModify, ServFail, None };
virtual Action operator()(DNSResponse*, string* ruleresult) const =0;
virtual ~DNSResponseAction()
{
Expand Down Expand Up @@ -230,6 +230,7 @@ struct DNSDistStats
stat_t ruleDrop{0};
stat_t ruleNXDomain{0};
stat_t ruleRefused{0};
stat_t ruleServFail{0};
stat_t selfAnswered{0};
stat_t downstreamTimeouts{0};
stat_t downstreamSendErrors{0};
Expand All @@ -250,6 +251,7 @@ struct DNSDistStats
{"rule-drop", &ruleDrop},
{"rule-nxdomain", &ruleNXDomain},
{"rule-refused", &ruleRefused},
{"rule-servfail", &ruleServFail},
{"self-answered", &selfAnswered},
{"downstream-timeouts", &downstreamTimeouts},
{"downstream-send-errors", &downstreamSendErrors},
Expand Down
8 changes: 8 additions & 0 deletions pdns/dnsdistdist/DNSDIST-MIB.txt
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,14 @@ dynBlockNMGSize OBJECT-TYPE
"Dynamic blocks (NMG) size"
::= { stats 36 }

ruleServFail OBJECT-TYPE
SYNTAX Counter64
MAX-ACCESS read-only
STATUS current
DESCRIPTION
"Number of ServFail responses returned because of a rule"
::= { stats 37 }

backendStatTable OBJECT-TYPE
SYNTAX SEQUENCE OF BackendStatEntry
MAX-ACCESS not-accessible
Expand Down
13 changes: 6 additions & 7 deletions pdns/dnsdistdist/docs/rules-actions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ Rule Generators

Invoke a Lua function that accepts a :class:`DNSQuestion`.
This function works similar to using :func:`LuaAction`.
The ``function`` should return a :ref:`DNSAction`.
The ``function`` should return a :ref:`DNSAction`. If the Lua code fails, ServFail is returned.

:param DNSRule: match queries based on this rule
:param string function: the name of a Lua function
Expand All @@ -165,10 +165,9 @@ Rule Generators
.. versionchanged:: 1.3.0
Added the optional parameter ``options``.

Invoke a Lua function that accepts a :class:`DNSQuestion` on the response.
This function works similar to using :func:`LuaAction`.

The ``function`` should return a :ref:`DNSAction`.
Invoke a Lua function that accepts a :class:`DNSResponse`.
This function works similar to using :func:`LuaResponseAction`.
The ``function`` should return a :ref:`DNSResponseAction`. If the Lua code fails, ServFail is returned.

:param DNSRule: match queries based on this rule
:param string function: the name of a Lua function
Expand Down Expand Up @@ -785,15 +784,15 @@ The following actions exist.

Invoke a Lua function that accepts a :class:`DNSQuestion`.

The ``function`` should return a :ref:`DNSAction`.
The ``function`` should return a :ref:`DNSAction`. If the Lua code fails, ServFail is returned.

:param string function: the name of a Lua function

.. function:: LuaResponseAction(function)

Invoke a Lua function that accepts a :class:`DNSResponse`.

The ``function`` should return a :ref:`DNSResponseAction`.
The ``function`` should return a :ref:`DNSResponseAction`. If the Lua code fails, ServFail is returned.

:param string function: the name of a Lua function

Expand Down
4 changes: 4 additions & 0 deletions pdns/dnsdistdist/docs/statistics.rst
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,10 @@ rule-refused
------------
Number of Refused answers returned because of a rule.

rule-servfail
-------------
Number of ServFail answers returned because of a rule.

self-answered
-------------
Number of self-answered responses.
Expand Down
2 changes: 1 addition & 1 deletion regression-tests.dnsdist/test_API.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ def testServersLocalhostStatistics(self):
'latency-avg1000000', 'uptime', 'real-memory-usage', 'noncompliant-queries',
'noncompliant-responses', 'rdqueries', 'empty-queries', 'cache-hits',
'cache-misses', 'cpu-user-msec', 'cpu-sys-msec', 'fd-usage', 'dyn-blocked',
'dyn-block-nmg-size']
'dyn-block-nmg-size', 'rule-servfail']

for key in expected:
self.assertIn(key, values)
Expand Down