pvxe / nftables-geoip

Python script that generates nft maps of ip address blocks and corresponding geolocation. This data is taken from db-ip.com, so yo don't have to worry about accepting any EULA.
GNU General Public License v2.0
115 stars 17 forks source link

Separate dictionary creation #1

Closed sunshineplan closed 7 months ago

sunshineplan commented 4 years ago

Downloading db-ip.com geoip csv file... Writing country definition files... Writing nftables maps (geoip-ipv{4,6}.nft)... Killed

Machine: GCP f1-micro

pvxe commented 4 years ago

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)

sunshineplan commented 4 years ago

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.

pvxe commented 4 years ago

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!

weregoat commented 4 years ago

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.

pvxe commented 4 years ago

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.

pvxe commented 4 years ago

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.

¹ geoipsets

weregoat commented 4 years ago

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.

TCB13 commented 11 months ago

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.

Yes! Because I guess most people want to use this on a basis of "I want to block traffic from everywhere except for country A and B". Separate files like geoip-def-ipv4-{contry}.nft / geoip-def-ipv6-{contry}.nft would be perfect.

pvxe commented 7 months ago

This PR has been merged into the script: https://github.com/pvxe/nftables-geoip/pull/7

Now there is the possibility to generate nft maps containing only a set of specified countries using the -c/--country-filter parameter.