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

Handling lots of ICMP echo replies improvement #14

Open
FZambia opened this issue Sep 11, 2015 · 6 comments
Open

Handling lots of ICMP echo replies improvement #14

FZambia opened this issue Sep 11, 2015 · 6 comments

Comments

@FZambia
Copy link

FZambia commented Sep 11, 2015

Hello! I was playing with go-fastping yesterday and came across a situation that not all ICMP replies reached my application code.

I sent ICMP requests to several thousands of IP addresses. But I got only about 300 successful replies. tcpdump showed that all ICMP ECHO REPLY packets were delivered to my machine (tcpdump -e icmp[icmptype] == 0 -B 40096 -n).

After some investigation I think I found a possible problem, this is a code in recvICMP:

        select {
        case recv <- &packet{bytes: bytes, addr: ra}:
        case <-ctx.stop:
            p.debugln("recvICMP(): <-ctx.stop")
            wg.Done()
            p.debugln("recvICMP(): wg.Done()")
            return
        }

recv is a channel with buffer size 1 and it seems that application spends a lot of time sending into it. Making it buffered:

recv := make(chan *packet, len(p.addrs))

helped to get all ICMP replies.

So I think that read buffer in underlying ip connection is not big enough to handle lots of incoming data. And when application is busy and can't read packets - some of them just dropped.

What are your thoughts about this?

@kanocz
Copy link
Contributor

kanocz commented Sep 13, 2015

Sounds very interesting, i'll test this on some /8 networks today

@kanocz
Copy link
Contributor

kanocz commented Sep 13, 2015

No, it looks like this change make possible situation when "ping is done" but many information left in channel... So we need also check if something left and process after "done"

@tatsushid
Copy link
Owner

We should have refactor, redesign recvICMP and procRecv to process more packets as it is discussed at #13.

I will try it but please give me a little more time.

@kkirsche
Copy link

+1 would love to see this fixed. I experience the same behavior when pinging 2 hosts.

@LeoLiuYan
Copy link

+1 I experience the same behavior when pinging 254 hosts.

@irom77
Copy link

irom77 commented Dec 11, 2016

Same here, not receiving all replies, playing with 1k hosts

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

No branches or pull requests

6 participants