Hi,

I would like to stop the pinger after a call to OnRecv, and not waste time until OnIdle is it possible?

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?

I see that callback functionality can be provided to handle a successful ping event, but there is no option to provide a custom handler for errors (ping timeouts). How much effort would it be to add this?

How can i know that server is unreachable?

        p := fastping.NewPinger()
        ra, err := net.ResolveIPAddr("ip4:icmp", "111.111.111.1")
        if err != nil {
                panic(err)
        }
        p.AddIPAddr(ra)
        p.OnRecv = func(addr *net.IPAddr, rtt time.Duration) {
                fmt.Printf("IP Addr: %s receive, RTT: %v\n", addr.String(), rtt)
        }
        p.OnIdle = func() {
                fmt.Println("finish")
        }
        err = p.Run()
        if err != nil {
                fmt.Println(err)
        }

output

finish

in cmd

Isn't ICMP ping not enough? In what scenario does the UDP ping works better than ICMP ping?

I did see that rtt is 0.0 in OnRecv callback.
Please look at this line, it is possiblie that the rtt is 0,and it call OnRecv.
https://github.com/tatsushid/go-fastping/blob/master/fastping.go#L653

I think this is bug, as my ping logger will get a 0 rtt for some real network routes.
It will lower the average rtt to an impossible value. (lower than the light rtt).

Ping always failed when payload is more then 512 on Windows,
test on Mac is fine.
I changed buffer size larger on recvICMP(), it's works fine now.

ICMP can discover MTU by trying different send package size. Thanks.

I get this error when I run the example code from the README in my own main or when I build the ping command from this project and invoke it thus:

$ ~/go/bin/ping www.example.com
Ping failed: listen ip4:icmp <nil>: operation not permitted

Running it under sudo succeeds however. Maybe it is worth adding a sentence to the README pointing out the need for root access.

netstat -a | grep icmp | wc -l

This library we have implemented two different ways and have always found it keeping ICMP requests and not closing them somehow for some reason.

We rewrote our own pinger and it has no leaks. I suggest the author run all their tests using a long-running process like a web server calling fastping and ensuring that there are no ICMP requests growing with this command. Using a unit test which exits out quickly will not show how these ICMP requests possibly are still open.

There is definitely a possibility it could have been our code causing the leak, but we gave it a second chance and used channels and 100% knew it was not our library keeping these open, yet it still leaked ICMP counts which overtime will completely hose a server if there are too many being held open.

Hopefully, this command is helpful for anyone using this library (or the authors) to be wary of using this library and keeping watch on your ICMP counts bloating and taking down webservers.

I am not going to provide any code samples because we are not using this library anymore and I just wanted to leave that command with people just in case they read this and want to see if there are any leaked pings that can grow overtime and cause IRQ issues taking down a server and causing high load.

p.AddIPAddr(ra). I added lots of ra on p perhaps thousands of it.
OnRecv I record the recieved IP and turn on a bool.
p.Run, could return with no error, yet, the recieved is not stable.
While I know Ping is not stable on network, but it should not be too unreliable( I devided the thousands of IPs to 250 a group, and ping them at a time, the result could be more stable).
For my situation, I pinged 10000 IPs, if I ping them using AddIPAddr, I could only get perhaps 1000 IPs alive, But if I change to 250 IPs a group and ping all the 10000 IPs for 40 groups, I could get perhaps 8000 IPs alive.
Does anyone have an idea on this situation?

Got random panic errors. Any way to fix this?

`time="2018-06-18T22:55:32+02:00" level=debug msg="Pinger iteration finished"
panic: close of closed channel

goroutine 10 [running]:
github.com/tatsushid/go-fastping.(*Pinger).run(0xc4202842d0, 0xc42010ce01)
/usr/local/src/go/src/github.com/tatsushid/go-fastping/fastping.go:479 +0x7c4
github.com/tatsushid/go-fastping.(*Pinger).Run(0xc4202842d0, 0x0, 0x0)
/usr/local/src/go/src/github.com/tatsushid/go-fastping/fastping.go:329 +0xee
main.ping_hosts_single(0xc4202c2dc0)
/usr/local/src/go/src/nlbhealthmonitor/main.go:261 +0xca
main.is_icmp_status_ok(0xc420210081)
/usr/local/src/go/src/nlbhealthmonitor/main.go:217 +0x37
`

Hi
I found that fastping could panic when call RemoveIP() in other goroutine:
fastping fatal error: concurrent map iteration and map write
The panic point in sendICMP():

	...
	for key, addr := range p.addrs {
	...

Which an unprotected p.addrs in concurrent enviroment.
So when I was running p.RunLoop() without p.Stop(), and call p.RemoveIP() in another goroutine.
A panic will happen:
fastping fatal error: concurrent map iteration and map write

I've changed the network to udp mode with p.Network("udp") to avoid the need for sudo privileges, but I get socket: permission denied as an error.

When run with debug on, it prints:
2016/06/16 14:52:34 Run(): Start
2016/06/16 14:52:34 Run(): close(p.ctx.done)

Running without p.Network("udp") works fine (but requires sudo).
The error is identical in 32 bit and 64 bit Ubuntu 14.04.
The same code works as intended on mac.

Regardless of the latency of the ICMP packets, Pinger.Run() currently only returns after MaxRTT.

I understanding using MaxRTT to space pings when running with RunLoop(), but when i explicitly call Pinger.Run(), I want it to return as soon as the response ICMP packet is received.

use golang official package manager: dep

dep ensure -add github.com/tatsushid/go-fastping
ensure Solve(): No versions of github.com/tatsushid/go-fastping met constraints:
        master: unable to deduce repository and source type for "golang.org/x/net/icmp": unable to read metadata: un
able to fetch raw metadata: failed HTTP request to URL "http://golang.org/x/net/icmp?go-get=1": Get http://golang.or
g/x/net/icmp?go-get=1: dial tcp 216.239.37.1:80: connectex: A connection attempt failed because the connected party
did not properly respond after a period of time, or established connection failed because connected host has failed
to respond.
        fix/refactor-sendicmp: unable to deduce repository and source type for "golang.org/x/net/icmp": unable to re
ad metadata: unable to fetch raw metadata: failed HTTP request to URL "http://golang.org/x/net/icmp?go-get=1": Get h
ttp://golang.org/x/net/icmp?go-get=1: dial tcp 216.239.37.1:80: connectex: A connection attempt failed because the c
onnected party did not properly respond after a period of time, or established connection failed because connected h
ost has failed to respond.

Hi, I noticed, that Stop() function does not write bool to done channel, but instead reads from it.
In my code I am using the following:

p.RunLoop()
<-p.Done()
if err = p.Err(); err != nil {
	return result, err
}

which deadlocks, so I suggest Stop() function should be changed to:

func (p *Pinger) Stop() {
p.debugln("Stop(): close(p.ctx.stop)")
close(p.ctx.stop)
p.debugln("Stop(): p.ctx.done <- true")
p.ctx.done <- true
}

Regards,
Uros

Thanks for the package.

Not sure which came first, but is there any reason why you wouldn't use golang.org/x/net/icmp instead of your own icmp.go?

Id generate use rand.Intn(0xffff) will lost return ip in high concurrent env.

func main() {
rand.Seed(time.Now().UnixNano())
for i:=0; i<10000; i++ {
wg.Add(1)
go do()
}
wg.Wait()
}

func do() {
defer wg.Done()
fmt.Println(rand.Intn(0xffff))
}

This will generate repeat id.

As of the latest build of go, I cannot build this library because of the dependency on "golang.org/x/net/internal/iana"

imports golang.org/x/net/internal/iana: use of internal package not allowed

I see only round trip time reported on ICMP echo requests. But does ping.Run() send only 1 request to calculate rtt? Is there any plan to add jitter and packet drops?

I have a use case where I have to get the network interface which was used to make the network call.

Is it possible?

package main

import (
	"fmt"
	"net"
	"os"
	"time"

	fastping "github.com/tatsushid/go-fastping"
)

func main() {

	go func() {
		var rt time.Duration
		p := fastping.NewPinger()
		ra, err := net.ResolveIPAddr("ip4:icmp", "127.0.0.1")
		if err != nil {
			fmt.Println(err)
			os.Exit(1)
		}
		p.AddIPAddr(ra)
		p.Network("udp")
		p.OnRecv = func(addr *net.IPAddr, rtt time.Duration) {
			rt = rtt
		}

		p.OnIdle = func() {
		}

		err = p.Run()
		if err != nil {
			fmt.Println(err)
		}

		fmt.Println("time1: " + rt.String())
	}()

	go func() {
		var rt time.Duration
		p := fastping.NewPinger()
		ra, err := net.ResolveIPAddr("ip4:icmp", "127.0.0.1")
		if err != nil {
			fmt.Println(err)
			os.Exit(1)
		}
		p.AddIPAddr(ra)
		p.Network("udp")
		p.OnRecv = func(addr *net.IPAddr, rtt time.Duration) {
			rt = rtt
		}

		p.OnIdle = func() {
		}

		err = p.Run()
		if err != nil {
			fmt.Println(err)
		}

		fmt.Println("time2: " + rt.String())

	}()

	select {}
}

go run ping.go

time2: 161µs
time1: 0s

go version: go1.7.1 linux/amd64

when you compile " go build fastping.go " , you may attack below errors.

the solution: you need to "go get -u golang.org/x/net/icmp" .

golang.org/x/net/ipv6

../../../../golang.org/x/net/ipv6/sockopt_asmreq_unix.go:15: setsockoptIPMreq redeclared in this block
previous declaration at ../../../../golang.org/x/net/ipv6/sockopt_asmreq_posix.go:15
../../../../golang.org/x/net/ipv6/sockopt_unix.go:15: getInt redeclared in this block
previous declaration at ../../../../golang.org/x/net/ipv6/sockopt_posix.go:15
../../../../golang.org/x/net/ipv6/sockopt_unix.go:27: setInt redeclared in this block
previous declaration at ../../../../golang.org/x/net/ipv6/sockopt_posix.go:27
../../../../golang.org/x/net/ipv6/sockopt_unix.go:35: getInterface redeclared in this block
previous declaration at ../../../../golang.org/x/net/ipv6/sockopt_posix.go:35
../../../../golang.org/x/net/ipv6/sockopt_unix.go:54: setInterface redeclared in this block
previous declaration at ../../../../golang.org/x/net/ipv6/sockopt_posix.go:54
../../../../golang.org/x/net/ipv6/sockopt_unix.go:65: getICMPFilter redeclared in this block
previous declaration at ../../../../golang.org/x/net/ipv6/sockopt_posix.go:65
../../../../golang.org/x/net/ipv6/sockopt_unix.go:77: setICMPFilter redeclared in this block
previous declaration at ../../../../golang.org/x/net/ipv6/sockopt_posix.go:77
../../../../golang.org/x/net/ipv6/sockopt_unix.go:84: getMTUInfo redeclared in this block
previous declaration at ../../../../golang.org/x/net/ipv6/sockopt_posix.go:84
../../../../golang.org/x/net/ipv6/sockopt_unix.go:103: setGroup redeclared in this block
previous declaration at ../../../../golang.org/x/net/ipv6/sockopt_posix.go:103
../../../../golang.org/x/net/ipv6/sockopt_unix.go:117: setSourceGroup redeclared in this block
previous declaration at ../../../../golang.org/x/net/ipv6/sockopt_posix.go:117
../../../../golang.org/x/net/ipv6/sockopt_unix.go:117: too many errors

golang.org/x/net/ipv4

../../../../golang.org/x/net/ipv4/sockopt_posix.go:29: cannot use s (type uintptr) as type int in argument to getsockopt
../../../../golang.org/x/net/ipv4/sockopt_posix.go:51: cannot use s (type uintptr) as type int in argument to setsockopt
../../../../golang.org/x/net/ipv4/sockopt_unix.go:17: getInt redeclared in this block
previous declaration at ../../../../golang.org/x/net/ipv4/sockopt_posix.go:17
../../../../golang.org/x/net/ipv4/sockopt_unix.go:38: setInt redeclared in this block
previous declaration at ../../../../golang.org/x/net/ipv4/sockopt_posix.go:38
../../../../golang.org/x/net/ipv4/sockopt_unix.go:54: getInterface redeclared in this block
previous declaration at ../../../../golang.org/x/net/ipv4/sockopt_posix.go:54
../../../../golang.org/x/net/ipv4/sockopt_unix.go:68: setInterface redeclared in this block
previous declaration at ../../../../golang.org/x/net/ipv4/sockopt_posix.go:68
../../../../golang.org/x/net/ipv4/sockopt_unix.go:82: getICMPFilter redeclared in this block