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: DynBlockGroupRules : eBPF block doesn't use eBPF if DNSAction.Drop is not set exactly in each rule #11504

Closed
rumato163 opened this issue Apr 5, 2022 · 10 comments · Fixed by #11544

Comments

@rumato163
Copy link

rumato163 commented Apr 5, 2022

  • Program: dnsdist
  • Issue type: Bug report

Short description

Since 1.6 ,when dnsdist creates a DynBlock, It should use eBPF as a filter to block IP's if setDefaultBPFFilter() is set properly.
But if the action is not set to 'DNSAction.Drop' for each rule it does not use eBPF.

Environment

  • Operating system: gentoo
  • Software version: dnsdist-1.7.0
  • Software source: gentoo .ebuild

Steps to reproduce

bpf = newBPFFilter(1000, 1000, 32)
setDefaultBPFFilter(bpf)
setDynBlocksAction(DNSAction.Drop)

local dbr = dynBlockRulesGroup()
  dbr:setRCodeRate(DNSRCode.NXDOMAIN, 5, 5, "Exceeded NXD rate", 60)

function maintenance()
  dbr:apply()
end

Expected behaviour

If I'll emulate NXD rate from loopback dynblock have to block it and also bpf should include it in it's list because of eBPF have to be used.

> showDynBlocks()
What                      Seconds   Blocks Warning    Action               Reason
127.0.0.1/32                   43        1 false      Drop                 Exceeded NXD rate
> bpf:getStats()
127.0.0.1: 1

Actual behaviour

It does not sow up in bf list at all

> showDynBlocks()
What                      Seconds   Blocks Warning    Action               Reason
127.0.0.1/32                   43        1 false      Drop                 Exceeded NXD rate
> bpf:getStats()

Other information

I can get it work properly only if I set 'DNSAction.Drop' for each rule in group like that:

local dbr = dynBlockRulesGroup()
-- #dbr:excludeRange(recursive_ips)
dbr:setRCodeRate(DNSRCode.NXDOMAIN, 5, 5, "Exceeded NXD rate", 60, DNSAction.Drop)
dbr:setRCodeRate(DNSRCode.SERVFAIL, 5, 5, "Exceeded ServFail rate", 60, DNSAction.Drop)
dbr:setQTypeRate(DNSQType.ANY, 5, 5, "Exceeded ANY rate", 60, DNSAction.Drop)

So it looks like 'setDynBlocksAction(DNSAction.Drop)' is ignored and nothing goes to eBPF

@DasSkelett
Copy link

DasSkelett commented Apr 14, 2022

We can confirm this issue, also on dnsdist 1.7. Specifying the drop action explicitly as mentioned in the issue works.

@rgacogne
Copy link
Member

That code indeed ignores the value of setDynBlocksAction which is a bug. I'll fix it in 1.7.1 and 1.8.0, and in the meantime I think the work-around mentioned above works well enough, even if it's a bit cumbersome to have to specify the action on every line :-/

@rgacogne
Copy link
Member

I just submitted a fix for this issue in #11544. If one of you can test it and confirm that it fixes the issue, that would be great!
I'm planning on releasing 1.7.1 in a few weeks, with that fix included.

@rumato163
Copy link
Author

rumato163 commented Apr 15, 2022

Sure, I can test it.
But I need some time to get your commit and make it from source.
Or you have a better way to test it before 1.7.1 announcement?

@rgacogne
Copy link
Member

I could build custom packages from my backport branch (#11550) but I don't think it would help you as a gentoo user :) I can generate a patch that applies cleanly on top of the 1.7.0 tarball, though, just a minute.

@rgacogne
Copy link
Member

@rumato163
Copy link
Author

rumato163 commented Apr 15, 2022

Nice! That's gonna work for me )))
Will test it on Monday, most likely

@rgacogne
Copy link
Member

This version is a bit better and does not break the unit tests: https://gist.github.com/rgacogne/4bf1d39e5a1ee29b337988e21ab7de2f (I went a bit too far down the "minimal patch" road with the first version).

@rumato163
Copy link
Author

rumato163 commented Apr 18, 2022

Hi @rgacogne
Looks like your patch did the trick.
I can confirm that now it works without pointing DNSAction for each rule in DynBlockRuleGroup.

But also it works by default. Even if "setDynBlocksAction(DNSAction.Drop)" is not set. I guess it's correct because Drop action is pre-defined for all the DynBlockRules by default.

Also I can confirm that eBPF will not be used if "setDefaultBPFFilter(bpf)" is not set. And it also looks correct according to the manual.

@rgacogne
Copy link
Member

Thanks a lot for testing and confirming!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants