Skip to content

Only reload pihole.toml if changed#1778

Merged
DL6ER merged 4 commits into
development-v6from
new/config_checksum
Nov 26, 2023
Merged

Only reload pihole.toml if changed#1778
DL6ER merged 4 commits into
development-v6from
new/config_checksum

Conversation

@DL6ER

@DL6ER DL6ER commented Nov 22, 2023

Copy link
Copy Markdown
Member

What does this implement/fix?

Currently, we always reload pihole.toml if it was touched by anyone, even if nothing got changed. This PR changes this to only reload pihole.toml if the content really changed. Internally, we use sha256sum to memorize a checksum of the file.

Due to a cache-optimal design, we can even outperform the system-provided sha256sum:

$ time sha256sum pihole-FTL
4f72f290b8be5498b31c32e27ee9c6d7dafedcd454770d9a1f36efc8fe403edf  pihole-FTL

real    0m0.113s
user    0m0.104s
sys     0m0.009s

$ time pihole-FTL sha256sum pihole-FTL
4f72f290b8be5498b31c32e27ee9c6d7dafedcd454770d9a1f36efc8fe403edf  pihole-FTL

real    0m0.044s
user    0m0.036s
sys     0m0.008s
$ time sha256sum oci.tar
6a885fe60b78d1abd6a73c239ed465e0a10681942d66b3e6317b40fdbc9a01b9  oci.tar

real	0m2.090s
user	0m2.030s
sys	0m0.060s

$ time ./pihole-FTL sha256sum oci.tar
6a885fe60b78d1abd6a73c239ed465e0a10681942d66b3e6317b40fdbc9a01b9  oci.tar

real	0m0.456s
user	0m0.344s
sys	0m0.112s

(oci.tar is a 550 MB file)

I ran the commands several times to ensure the file was cached in memory to rule out possible disk influences.


Related issue or feature (if applicable): N/A

Pull request in docs with documentation (if applicable): N/A


By submitting this pull request, I confirm the following:

  1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented my proposed changes within the code.
  3. I am willing to help maintain this change if there are issues with it later.
  4. It is compatible with the EUPL 1.2 license
  5. I have squashed any insignificant commits. (git rebase)

Checklist:

  • The code change is tested and works locally.
  • I based my code and PRs against the repositories developmental branch.
  • I signed off all commits. Pi-hole enforces the DCO for all contributions
  • I signed all my commits. Pi-hole requires signatures to verify authorship
  • I have read the above and my PR is ready for review.

Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER DL6ER requested a review from a team November 22, 2023 22:15
@yubiuser

Copy link
Copy Markdown
Member

Can we use hashing also when comparing pihole.toml and pihole.toml.temp (and the other files). This might be faster than comparing line by line.

@DL6ER

DL6ER commented Nov 23, 2023

Copy link
Copy Markdown
Member Author

We'd anyway have to read line-by-line to be able to skip the first N lines to skip the timestamp in the header (or all files will always be different!). If reading in bulk, we have to iterate until we have found enough \n and then offset the SHA computation. Either or, comparing line-by-line is much faster because iterating in C comes at no costs (unless we are talking about billions of lines) and a direct memory comparison is an instruction the CPU can do at once, the entire SHA computation is, on the other hand, a pretty massive operation:

Proof-of-work-SHA256
(source)

The core of the SHA algorithm is called the compression function and consists of 4 cycles of 20 steps each (=80 steps). The loops are very similar in structure except that they use a different primitive logic function.

Each block is taken as an input parameter by all 4 loops along with a constant K and the values of the eight registers. At the end of the computation new values will be obtained for A, B, C, D, E, F, G, and H which will be used for the computation of the next block until the final block H is obtained.

edit FIPS PUB 180-4 has all the details, look at Scn. 6.2 in particular.

@yubiuser yubiuser left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Something is not right. Changing settings from the web interface is fine, but manually editing pihole.toml does not log the writing and does not backup the log

The first line of the log is a setting changed from the web interface, the second via manual editing.

2023-11-23 21:14:39.657 [43543/T43800] INFO: Config file written to /etc/pihole/pihole.toml
2023-11-23 21:15:34.556 [43543/T43793] INFO: Reloading config due to pihole.toml change

@DL6ER

DL6ER commented Nov 23, 2023

Copy link
Copy Markdown
Member Author

but manually editing pihole.toml does not log the writing and does not backup the log

I think this is because the recent bugfix #1775 was not included here. I just merged development-v6

@DL6ER DL6ER merged commit 30c3e9c into development-v6 Nov 26, 2023
@DL6ER DL6ER deleted the new/config_checksum branch November 26, 2023 20:39
@PromoFaux PromoFaux mentioned this pull request Feb 18, 2025
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.

2 participants