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: Added XDP middleware for dropped/redirected queries logging #11020

Merged
merged 2 commits into from Dec 1, 2022

Conversation

MiniPierre
Copy link

Short description

According to this issue #11009, this pull request add the possibility to load user-defined XDP function when performing TC or DROP action. This allow the user to log the queries even if they do not reach libpcap.
I added a custom XDP filter and a Python script to load it and output information about dropped/tc packets

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@MiniPierre
Copy link
Author

MiniPierre commented Nov 22, 2021

I saw that @rgacogne added a type to dns_action, actually it made the test script fail so I removed it, no problem with having it back

@rgacogne
Copy link
Member

I saw that @rgacogne added a type to dns_action, actually it made the test script fail so I removed it, no problem with having it back

I briefly tested adding it back and the program loaded fine, but perhaps it fails after the startup?

Copy link
Member

@rgacogne rgacogne left a comment

Choose a reason for hiding this comment

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

Looks great, thanks! The indentation in the ebpf.src files could be improved (we have a .clang-format at the root of the repo that might help) but that's not a blocker for me.

@MiniPierre
Copy link
Author

Just adapted the indentation, I was in a hurry and did not took the time to fix this before.

I saw that @rgacogne added a type to dns_action, actually it made the test script fail so I removed it, no problem with having it back

I briefly tested adding it back and the program loaded fine, but perhaps it fails after the startup?

It was failing at the python script startup, maybe it is a problem on our side

@rgacogne
Copy link
Member

Thanks for fixing the indentation!

It was failing at the python script startup, maybe it is a problem on our side

Actually that's my fault, you can't specify the underlying type of an enum in C, I guess I'm too used to C++11 these days, and my compiler somehow allows that..
The problem with the current code is that the size of the map_value depends on the compiler, which might be a problem. But it's not related to your changes, so don't worry about it, I'll fix in a different PR.

@MiniPierre
Copy link
Author

Could you tell me how you converted your .ebpf.src files to .ebpf ? I am trying to switch from Python to C++ to load the logging middleware as Python is not fast enough to pull the event perf buffer under high load, so events get dropped.

@rgacogne
Copy link
Member

The easiest way for you, since you are already using bcc, is to dump the eBPF code of a given function to a file like this:

xdp = BPF(src_file="xdp-filter.ebpf.src")
newFile = open ("/tmp/ebpf.bin", "wb")
dump = xdp.dump_func("xdp_dns_filter")
newFile.write(dump)
newFile.close()

In dnsdist I did not want to commit a huge binary blob, especially since we have to update the file descriptors in that blob to match the one linked to eBPF maps, so I used this script (which doesn't cover all functions but would be easy to update) to get the eBPF code in a more easy to understand form: https://coredump.fr/static/powerdns/reverse.py

@rgacogne
Copy link
Member

rgacogne commented Nov 24, 2021

If you try to use that script, 113 is the code for bpf_probe_read_kernel (sorry!) :)

Copy link
Member

@rgacogne rgacogne left a comment

Choose a reason for hiding this comment

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

Rebased on master to fix conflicts, looks good to me!

@rgacogne rgacogne merged commit 9d86b2b into PowerDNS:master Dec 1, 2022
@Y7n05h Y7n05h mentioned this pull request Feb 8, 2023
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants