Separate dictionary creation
sunshineplan opened this issue · 9 comments
Downloading db-ip.com geoip csv file...
Writing country definition files...
Writing nftables maps (geoip-ipv{4,6}.nft)...
Killed
Machine: GCP f1-micro
Hi sunshine, I think this is a good enhacement idea.
I just need to take some time organizing the project before, like adding some contribution guide and dealing with formatting issues (using autopep8 and pylint)
Hi JMGuisadoG
Thank you for reopening this issue.
In fact, even if this script after modified can be run successfully, it will also trigger oom when nft -f output file. This is an nft memory issue. The only way is add more memory or modify the script to fetch smaller list(In my case, I only need one country IP list). If choose add more memory, this issue will not happen. So I think this issue is not strongly needed.
It is kind if warning people use geoip nftables function required more than 300M free memory space.
It's indeed interesting and a good opportunity to take into account use cases I did not foresee.
In fact, even if this script after modified can be run successfully, it will also trigger oom when nft -f output file. This is an nft memory issue. The only way is add more memory or modify the script to fetch smaller list(In my case, I only need one country IP list). If choose add more memory, this issue will not happen. So I think this issue is not strongly needed.
Output by continent should also be implemented and should alleviate memory usage when using nft -f. Probably I'll open a separate issue for that.
As you can see geoip-def-{continent}.nft are created but only geoip-def-all.nft can be used as of now. A filter is to be added to generate continent-specific eg. geoip-ipv4-{continent}
It is kind if warning people use geoip nftables function required more than 300M free memory space.
Maybe I should add this to the caveats section.
Thanks for your feedback!
Another possibility, if I may suggest one, would be to limit the map to a limited set of countries (specified on the command line) instead than by continents.
Another possibility, if I may suggest one, would be to limit the map to a limited set of countries (specified on the command line) instead than by continents.
This is a good idea, all this may fall into enhancements to the script functionality, adding parameters and modularizing the execution, so probably I'd open separate issues or create a GitHub project to track this properly.
For the record, new changes have reduced memory usage of the script execution by 150MB more or less.
command time -v ./nft_geoip.py --download --file-location location.csv -o output/
Yields Maximum resident set size (kbytes): 168848
As of now, there exists other tools¹ that focus on generation of geoip sets for nftables. So, I think it reasonable to point to those tools if a set is preferred over a map.
I'd have no problem adding a country filter as arguments but I don't know how preferable is a few countries map over an address set for each country. I most probably will add optional flags to only write ipv4 or ipv6, writing both if none of the flags are present.
As of now, there exists other tools¹ that focus on generation of geoip sets for nftables. So, I think it reasonable to point to those tools if a set is preferred over a map.
It's a fair point, I agree.