Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ulimit check is incorrect #327

Merged
merged 4 commits into from
Apr 2, 2023
Merged

Ulimit check is incorrect #327

merged 4 commits into from
Apr 2, 2023

Conversation

bayerhonza
Copy link
Contributor

Hello,

in previous versions, I could run ZDNS with tens of thousands of threads without problem on a server with 64 logical cores. However, now it is impossible because of the added ulimit check that is too strict.

The default ulimit -n value for a newly created bash is 1024. When running zdns with 1000 threads, it now shows a warning:

WARN[0000] Current nofile limit (1024) lower than maximum connection count (64000), try to update. 

with the calculated maximum connection count at 64,000 (supposedly 64 cores x 1000 threads)

However, the number of open sockets is 1001:

printf "google.com\nexample.com"> test.txt; strace -f ./zdns A --threads 1000 --recycle-sockets=true  --input-file test.txt 2>&1 | grep 'socket(AF_INET, SOCK_DGRAM' | wc -l; rm test.txt >/dev/null

one socket open to get the local address:

zdns/pkg/zdns/zdns.go

Lines 200 to 206 in c8e1d8e

if !gc.LocalAddrSpecified {
// Find local address for use in unbound UDP sockets
if conn, err := net.Dial("udp", "8.8.8.8:53"); err != nil {
log.Fatal("Unable to find default IP address: ", err)
} else {
gc.LocalAddrs = append(gc.LocalAddrs, conn.LocalAddr().(*net.UDPAddr).IP)
}

and 1000 sockets correspond to one socket open per goroutine. The total number of open files is less than 1024 and zdns should run without problem.

The problem IMO is coming from the maximum ulimit value:

zdns/pkg/zdns/zdns.go

Lines 217 to 222 in c8e1d8e

if gc.GoMaxProcs != 0 {
runtime.GOMAXPROCS(gc.GoMaxProcs)
ulimit_check(uint64(gc.GoMaxProcs * gc.Threads))
} else {
ulimit_check(uint64(runtime.NumCPU() * gc.Threads))
}

If I understand well GOMAXPROCS, it determines how many routines can run in parallel but it does not multiply the number of open sockets.

I propose to set the ulimit value to the number of threads as each thread is supposed to open one socket (reusable or not).

@bayerhonza bayerhonza requested a review from a team as a code owner April 2, 2023 10:32
@bayerhonza bayerhonza changed the title Ulimit Ulimit check is incorrect Apr 2, 2023
@zakird zakird merged commit 898b3c9 into zmap:master Apr 2, 2023
@zakird
Copy link
Member

zakird commented Apr 2, 2023

Thanks for the fix. Looking back, I'm not sure how we arrived at that original logic, but it's certainly not right.

@bayerhonza bayerhonza deleted the ulimit branch May 11, 2023 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants