Various improvements#209
Various improvements#209friendly-bits wants to merge 71 commits intolynxthecat:various_improvementsfrom
Conversation
- Implement support for permanent blocklist - Rework message printing/logging - Lots and lots of logic changes to support the above
|
Oh wow, nice. Would this support permanent blocklist on USB stick? Could you outline roughly how you've implemented this? |
Yes
The comments in the generated config file expalin how the permanent blocklist feature works. As to nitty-gritty details, there is a lot going on, 2.5k lines of code changed. I've been working on this for a few weeks. So I think it's easier to see the details from the code than to explain them. If you have specific questions, I can answer them. (normally, I would split this into many small commits, but this implementation required a huge refactor and subsequent bug fixing, so incremental commits didn't make sense to me) I believe there are some TODO's left in the code yet, so it's not ready to merge quite yet, but at least in my testing everything is already working as intended. |
|
Update: there were just 2 TODO's. One is done with above commit. The other one is to parallelize domain lookups - I think better to postpone this to some later time. So as far as I'm concerned, this should be good for merging into the |
friendly-bits
force-pushed
the
various_improvements
branch
from
ac602de to
a38b5aa
Compare
- process_list_part(): check pipeline exit code and act on errors - process_list_part(): simplify size-exceeded check - gen_list_parts(): improve status message
|
I discovered and fixed some minor issues, and made a few additional improvements. Now all seems to be good. Waiting for your input @lynxthecat . |
|
^ Some work on improving console messages (mainly spacing) which became a little loose. |
|
So the 'managed' option would enable users to free up router memory by storing blocklist entirely on USB? Is it setup such that a restart of DNSMASQ will result in loading in from USB? |
Multiple dnsmasq instances are already supported in current adblock-lean. We do currently have a limitation, as any configured dnsmasq instances will use the same blocklist. The multiple blocklist instances idea intends to solve that. Each such blocklist instance could be configured to be loaded by any (or all) dnsmasq instances.
I'm not sure I understand. How would dnsmasq know which traffic is child-safe? If you would want to have separate content blocking for children, multiple blocklist instances would allow for that. E.g. you set up a separate network segments for grownups and for children (perhaps VLANs), you configure 2 dnsmasq instances. For one dnsmasq instance, you specify one network interface (corresponding to one VLAN), for the other dnsmasq instance you specify another network interface (corresponding to the other VLAN). Then you configure 2 blocklist instances in adblock-lean, and you specify a different dnsmasq instance index for each. Then you have separate content blocking.
I believe you are referring to our discussion concerning the other idea you mentioned: zero downtime. This I don't think is technically possible because only one dnsmasq instance can bind to a particular network interface and port at a time, and because AFAIK there is no way to reconfigure a dnsmasq instance on-the-fly to bind to a different network interface and port. Essentially, you tell dnsmasq which network interface and port to bind to when you call dnsmasq. Which makes it impossible to have one instance serving clients and another one loading blocklist and later on replacing the first one. The second instance will simply refuse to run. |
|
For reference, here is the config system I implemented for QosMate. The format is UCI but I wrote a custom parser in awk which validates the config (like our config management system does in adblock-lean) and generates shell code which is then The following function calls |
Yes this is the sort of thing I had in mind.
I'm certain there's a way round this one way or another. This is perhaps an extreme example to give, but when setting up cake instances it is routine to create a dedicated IFB interface on which to set up a cake interface (e.g. to deal with ingress traffic) - see e.g. here, and so analogously I imagine it would be possible to set up an IFB interface and set up the further dnsmasq instance on that. I'm just opining that I think there is a way to implement this zero-downtime switchover concept. Whether the complexity and processing cost is worth it I'm not sure - especially for the typical home network use case when everyone is asleep in the middle of the night. On the other hand, surely there are situations where you want zero downtime in respect of filtered DNS queries (if only because there's plenty of memory and processing present and one wants to maximise filtered DNS performance)? |
- start implementing support for multiple blocklists - add command 'create_addnmounts' - reorder commands printed in the help message - start hotplug script implementation - store metadata in $META_FILE - implement is_hex_lc() - replace get_curr_blocklist_paths() with get_curr_bl_path() - implement find_persist_files() - implement split_path_spec() - rm_pause_bl(): support specifying blocklist instance - detect_util(): simplify implementation - detect_main_utils(): check for additional essential utils - start(), status(), pause(), resume(): adjust logic for multiple blocklists - improve console output spacing
- try_mv(): support '-q' option
- implement get_md5()
- implement mk_hotplug_script(), disable_hotplug_script()
- implement check_persist_dir()
- create_addnmounts(): logic changes
- print_def_config(): use better spec keywords: integer -> uint; integer_list -> uint_list
- print_def_config(): add option PERSIST_HOTPLUG_SCRIPT
- detect_pkg_manager(): set $PKG_FILES_LIST_CMD
- check_addnmounts(): change API, implementation to support multiple blocklists
- check_addnmounts(): check for specified dnsmasq indexes
- check_addnmounts(): smarter match for subdirs of included dirs
- get_dnsmasq_instances(): set PID_${index} rather than MAC_${index}
- get_dnsmasq_instances(): remove code for MAC-address fetching
- get_dnsmasq_instances(): simplify ifaces fetching
- check_active_blocklist(): require blocklist instance index, support per-instance checks - get_abl_run_state(): support multiple blocklists - set_abl_env(): support multiple blocklists, factor out code for setting specific bl-instance env into set_abl_inst_env() - implement gen_blocklists() - gen_blocklist(): get initial uptime from args - gen_blocklist(): do not add blocklist test entry: now added by install_blocklist() - implement get_dnsmasq_ips() to fetch and set per-dnsmasq-index instance IP addresses - implement mk_metadata() - implement read_all_metadata() - implement get_metadata() - implement install_blocklists() - export_blocklist(): rewrite with simple logic - restore_saved_blocklist(): rewrite with simple logic
- set_abl_inst_env(): remove logic for setting INSTALL_LOCATION, INSTALL_LOCATION_FALLBACK, INSTALL_PATH_FALLBACK, INSTALL_DESC - set_abl_inst_env(): set INSTALL_PATH_RAM
- gen_blocklists: infer location from install_path - install_blocklists: simnplify logic; infer location, install_desc from install_path
- rename read_all_metadata() -> read_blocklist_metadata() - metadata implementation and API: only use one metadata file stored in known path, store the MD5 hash next to persistent blocklists - metadata API: store separate entries for: PATH_RAM, MD5_RAM, CNT_RAM, PATH_PERSIST, MD5_PERSIST, CNT_PERSIST - metadata API: add IS_PAUSED entry - mk_metadata(), read_blocklist_metadata(): change implementation to conform with updated metadata API - merge read_blocklist_metadata(), get_metadata(). Now read_blocklist_metadata() is intended to be run only once at script init and populate global vars
- Finalize metadata setting/getting/storage/retrieval implementation - Move more functions into abl-manage - Add a workaround to allow testing before config implementation changes - Too many smaller changes to list This is still a work-in-progress.
friendly-bits
force-pushed
the
various_improvements
branch
from
f1c2bc0 to
accfc01
Compare
|
I pushed my current work-in-progress with the commits above ^. This already has most commands working correctly (start, stop, pause, resume, setup and others) but |
- set_bl_env: improve logic - move some functions around
- get_blocklist_timestamp(): support multiple blocklists - do_stop(): avoid stopping all blocklist instances when invalid blocklist id is specified - do_stop(): do not call get_bl_run_state() when set_global_env() fails - status(): support multiple blocklists - pause_resume(): allow empty value for $new_single_instance - uninstall() set $ASSERT_NOEXIT
- get_dnsmasq_ips(): support specifying blocklist ID's - set_global_env(): improve logic - set_global_env(): sanitize blocklist ID's from input - set_global_env(): bail on critical errors - rename assert_valid_bl_ids() -> assert_known_bl_ids() - assert_known_bl_ids(): do not exit when $ASSERT_NOEXIT is set - get_bl_params(): fix '-f' option not enforced - try_commit_metadata(): do not error out on $curr_location being empty
- status(): fix incorrect state printed when paused or stopped - pause_resume(): only load bl params when needed, avoid printing irrelevant errors
2 participants
This PR implements support for persistent blocklist, reworks messaging/logging system, and includes a lot of refactoring and logic changes to support this.