Hi Michael and Peter, just another beside one sorry for that...
On Mo, 2019-09-30 at 14:35 +0000, peter.mueller@ipfire.org wrote:
Hello Michael, hello Erik,
thank you for your replies on this.
I think we need to document this on the wiki before we merge this patch.
Certainly, yes. I will do so if a patch covering this is merged.
Why the added whitespace? Should have been an extra patch.
All right. shellcheck complained here, and I thought this would be a low-hanging fruit. Shall I change something like this for multiple scripts in a patch?
Quality of our scripts in IPFire 2.x is quite poor at some points, and since we will have do deal with it for some time, I thought improving might be a good idea...
ROOTSERVERIPS could have been an array and could have been local.
Okay.
You do not need to call xargs. It is a rather expensive way to remove line breaks. Does this suit you:
local ROOTSERVERIPS="$( awk '/\s+A\s+/ { print $4 }' ${ROOTHINTS} )" [...] for ip in "${ROOTSERVERIPS[@]}"; do [...]
If yes, I will hand in a second patch. If not, please give me some suggestion. :-)
It is also interesting that ipset does not allow to add more IP addresses in one go. This looks like a very expensive loop for a lot of IP addresses. I think this is fine here for about a dozen addresses, but importing a blacklist of thousands or tens of thousands of IP addresses will take a long time.
there is the possiblity to speed this process significantly up via 'ipset restore' whereby the format from 'ipset save' can be used which looks like this (if no counters has been set) -->
... add ipset_setname 11.22.33.44 -exist add ipset_setname 22.33.44.55 -exist ...
so if there is a vast list you can convert it e.g. via perl and pipe it to 'ipset restore’.
Ah, that is what I was looking for.
Example: IP list called 'vast_list' is formatted one per line
... 11.22.33.44 22.33.44.55 ...
can be formatted and restored with a
perl -pe 'chomp; $_ = "add ipset_setname $_ -exist\n"' vast_list | ipset restore
Just as a little gimmick :-).
You can also do this without perl:
xargs printf — “add ipset_setname %s -exist\n” | ipset restore
It is quite expensive to launch the perl interpreter and this solution should be a little bit more lightweight. It has become a little bit of a sport now to have minimal bash snippets :)
Did a fast test with awk, sed and Perl which looks like this:
+ perl -pe 'chomp; $_ = "add ipset_setname $_ -exist\n"' ip
real 0m0.071s user 0m0.060s sys 0m0.010s + sed -e 's/.*/add ipset_name & -exist/' ip
real 0m0.151s user 0m0.129s sys 0m0.012s + awk '{ print "add ipset_name "$0" -exist" }' ip
real 0m0.073s user 0m0.059s sys 0m0.014s + echo
++ wc -l ip + echo 'Number of IPs: 18985' Number of IPs: 18985 + exit 0
The printf command didn´t work.
Well, I consider processing 13 IP addresses not that problematic. How often does one a reboot, and how much extra time will ipset need here?
This simply is not a problem. Anyway, thank you for the hints - there still is an issue for processing some blacklists via firewall engine... :-)
Yes true, am handling some bigger lists at this time so it was a beneath info in general so to say ;-)
Thanks, and best regards, Peter Müller
Best,
Erik