banip: reduce fork overhead in monitor and feeds#29010
banip: reduce fork overhead in monitor and feeds#29010Coralesoft wants to merge 1 commit intoopenwrt:masterfrom
Conversation
2235d3d to
68c24b3
Compare
There was a problem hiding this comment.
Pull request overview
This PR optimizes banIP’s shell-based log monitor and feed download processing to reduce fork/exec overhead under sustained brute-force traffic, targeting better throughput on low-core OpenWrt routers.
Changes:
- Replace repeated
printf | grepmembership checks with shellcasematching in multiple code paths. - Rework the monitor pipeline to use a persistent
awkfilter for IP extraction and maintain in-process hit counters/caches. - Parallelize country/ASN feed downloads in
banip-service.shusing the existingban_coresthrottle approach.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| net/banip/files/banip-service.sh | Adds case-based allowlist-only skipping and backgrounds country/ASN downloads with a ban_cores throttle. |
| net/banip/files/banip-functions.sh | Replaces grep-based membership checks, refactors monitor IP extraction/counting, and adjusts feed/set handling logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Swap 24 printf|grep membership checks for shell case pattern matching across f_conf, f_getdl, f_getdev, f_getup, f_nftinit, f_down, f_rmset, and banip-service.sh. Use space-delimited exact-token matching so matches stay correct. In f_monitor, move awk-based IP extraction into the pipeline so it runs as a persistent filter instead of forking on every log line. Also replace the per-hit logread counter with in-memory shell variables, and avoid re-dumping the nft set after each block by appending to a local string instead. Also parallelise country and ASN feed downloads in banip-service.sh using the existing ban_cores throttle already used for regular feeds. Tested on GL-MT6000 (4-core ARM), OpenWrt 25.12.1, banIP 1.8.1-r3. Signed-off-by: Colin Brown <devs@coralesoft.nz>
68c24b3 to
1b07f65
Compare
|
@Coralesoft on a first sight this PR looks heavily AI generated. |
|
@dibdot, Here is my test script, I've combined then into a single script to make it easier to run on the router the results ================= [1] basic checks service status [2] IP counter test Detected IPs | OLD | NEW | Speedup [3] IP extraction test processing 1000 lines... [4] set dump cost measuring blocklist dump cost (20 iterations)... [5] case vs grep printf|grep: 1050 ms [6] reload stress test capturing baseline... running 3 reloads... reload 1/3 reload 2/3 reload 3/3 summary results: 8 passed, 0 failed |
|
@dibdot Here is a breakdown for why it works: For the printf | grep, the old code checks if a value is in a list by piping it through two external programs, every time it does this the shell has to start 2 new processes and hand data between them and then wait for them to finish. A case statement does the same check inside entirely inside the shell, no new processes needed. The in memory IP hit counter For the persistent awk in the monitor pipeline, the old monitor extracted the IP address from each log line by forking a new awk process for every single line. The new code runs one awk process that stays alive and filters all lines as they stream through. The old code was like restarting a car engine for every metre travelled versus just keeping it running for the whole trip. Removing the post-block set re-dump, After blocking an IP the old code re-reads the entire nftables blocklist set to update its local cache, the new code just appends the IP to a local string instead, it allready knows what its just added so there is no need to ask the nftables to repeat it back Parallel country/ASN feed downloads, the main gain is that those feeds no longer have to be fetched one after another when multiple split feeds are enabled. |
|
@Coralesoft Thanks for your post. It’ll be a while before I can give you my feedback, as I’m on holiday at the moment. |
|
@dibdot, all good mate, I'm about to do the same :) |
* removed needless fork/exec calls (#29010) * removed needless eval calls * added parallel country and ASN feed downloads (#29010) * rework the IP monitor: * IP extraction, counting, and threshold detection now run entirely inside a single gawk process * added a dynamic cache management and a three-tier IP deduplication * added asynchronous/non-blocking RDAP requests * hardend the cgi script and mail template * fixed #28998 * LuCI: added more status information * LuCI: more fixes & optimizations (e.g. #8486) * readme update Co-authored-by: Colin Brown <devs@coralesoft.nz> Signed-off-by: Dirk Brenken <dev@brenken.org>
|
@Coralesoft I have completely overhauled the monitor, and the new version should work significantly better than the version you proposed. (and my original version). Thank you for your contribution. |
* removed needless fork/exec calls (#29010) * removed needless eval calls * added parallel country and ASN feed downloads (#29010) * rework the IP monitor: * IP extraction, counting, and threshold detection now run entirely inside a single gawk process * added a dynamic cache management and a three-tier IP deduplication * added asynchronous/non-blocking RDAP requests * hardend the cgi script and mail template * fixed #28998 * LuCI: added more status information * LuCI: more fixes & optimizations (e.g. #8486) * readme update Co-authored-by: Colin Brown <devs@coralesoft.nz> Signed-off-by: Dirk Brenken <dev@brenken.org> (cherry picked from commit 9c3470a)
Under brute-force traffic, the banIP log monitor can fall behind the
log stream on a 4-core ARM router due to multiple fork/exec calls for
IP extraction, hit counting, logging, and nft set refresh.
This patch reduces that overhead by:
printf | grepmembership checks with shellcasematching in
f_conf,f_getdl,f_getdev,f_getup,f_nftinit,f_down,f_rmset, andbanip-service.shf_monitorinto the pipeline asa persistent filter with
fflush()logread | grepcounter with shell variables,resetting after 10,000 unique IPs
nft list setre-dump with a string appendf_logcalls into onebanip-service.shusing the existing
ban_coresthrottleNo new dependencies. All changes are POSIX
case/ busybox ashcompatible.
Semantic changes
Two monitor-path changes are not strictly equivalent to the previous
behaviour:
after startup are not reflected until the next restart.
These trade-offs remove repeated per-event process and set refresh
overhead. Full state is still rebuilt on restart.
Benchmarks (GL-MT6000, automated test suite)
casevsprintf|grep(500 iter)📦 Package Details
Maintainer: @dibdot
Description:
Shell-level optimisations to banIP's log monitor and feed processing to reduce fork/exec overhead under sustained load.
🧪 Run Testing Details
✅ Formalities