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

Internal name resolving functionality #253

Merged
merged 3 commits into from Apr 13, 2018
Merged

Internal name resolving functionality #253

merged 3 commits into from Apr 13, 2018

Conversation

DL6ER
Copy link
Member

@DL6ER DL6ER commented Apr 11, 2018

By submitting this pull request, I confirm the following (please check boxes, eg [X]) Failure to fill the template will close your PR:

Please submit all pull requests against the development branch. Failure to do so will delay or deny your request

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

How familiar are you with the codebase?:

10


This re-implements lots of already extensively tested code that was present in pre-FTLDNS for over a year. This implementation is only slightly different by ensuring that name resolution is done in a specific non-blocking way.

Before saving to the database (every minute by default), FTLDNS tries to obtain host names for clients and forward destinations that first appeared within this minute.

After garbage collection (once per hour by default), FTLDNS tries to re-obtain the host names for all clients that had at least one query in the selected time interval (24 hours by default). Host names of forward destinations are expected to not change.

This removes the necessity for doing the name lookups on the client side (or on an intermediate level like PHP as is currently implemented).

…ts of already extensively tested code that was present in pre-FTLDNS for over a year. This implementation is only slightly different by ensuring that name resolution is done in a specific non-blocking way.

Signed-off-by: DL6ER <dl6er@dl6er.de>
@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/lot-of-ptr-queries/8380/7

@DL6ER DL6ER mentioned this pull request Apr 11, 2018
9 tasks
… resolve 0.0.0.0

Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER
Copy link
Member Author

DL6ER commented Apr 12, 2018

Immediately after start up of FTLDNS (all forward destinations are still "new"):
screenshot at 2018-04-12 21-19-48

Latest one minute later:
screenshot at 2018-04-12 21-17-31

@PromoFaux PromoFaux self-requested a review April 12, 2018 23:02
Copy link
Member

@PromoFaux PromoFaux left a comment

Choose a reason for hiding this comment

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

Approved

@DL6ER DL6ER merged commit eb64f7b into FTLDNS Apr 13, 2018
@DL6ER DL6ER deleted the FTLDNS-resolve branch April 13, 2018 16:00
free(ip);
//free(name); // This is just the same as ip
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't look like you even need to allocate ip anymore?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's true. I will change it directly on the FTLDNS branch.

// This has to be run outside of the thread locks
// to prevent locking the resolver
resolveNewClients();

Copy link
Contributor

Choose a reason for hiding this comment

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

If the user disables the database, then the clients will never be resolved. Maybe move this to gc.c?

* Network-wide ad blocking via your own hardware.
*
* FTL Engine
* DNS Client Implementation
Copy link
Contributor

Choose a reason for hiding this comment

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

Specifically, DNS Client Hostname Resolution

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, it is also a DNS client implementation in the sense of that it queries the DNS server

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

4 participants