refactor: use golang.org/x/net/icmp to send/receive ICMP Echo packets

clambin · Dec 26, 2022 · 970a149 · 970a149
1 parent 73b769c
commit 970a149
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 25 deletions.
diff --git a/collector/pinger/icmp.go b/collector/pinger/icmp.go
@@ -1,7 +1,6 @@
 package pinger
 
 import (
-	"fmt"
 	log "github.com/sirupsen/logrus"
 	"golang.org/x/net/icmp"
 	"golang.org/x/net/ipv4"
@@ -22,7 +21,6 @@ type packet struct {
 
 func newConnection() (*icmpConnection, error) {
 	c := icmpConnection{id: os.Getpid() & 0xffff}
-	log.Debugf("icmpConnection id: %d", c.id)
 
 	var err error
 	if nettest.SupportsRawSocket() {
@@ -35,7 +33,7 @@ func newConnection() (*icmpConnection, error) {
 }
 
 func (c *icmpConnection) send(target net.Addr, seqno int) error {
-	msg := &icmp.Message{
+	msg := icmp.Message{
 		Type: ipv4.ICMPTypeEcho,
 		Code: 0,
 		Body: &icmp.Echo{
@@ -49,9 +47,6 @@ func (c *icmpConnection) send(target net.Addr, seqno int) error {
 	if err == nil {
 		_, err = c.conn.WriteTo(wb, target)
 	}
-	if err != nil {
-		err = fmt.Errorf("%s: %w", target, err)
-	}
 	return err
 }
 
@@ -72,11 +67,9 @@ func (c *icmpConnection) listen(ch chan<- packet) error {
 		// FIXME: when running in a k8s container, received ID is not pid&0xffff???
 		// use reply data instead
 		//if reply.ID != c.id {
-		//	if reply.ID != 1 {
 		if string(reply.Data) != "hello" {
-			log.Infof("dropping unexpected packet. id=%d, seq=%d, data=%s", reply.ID, reply.Seq, string(reply.Data))
+			log.Debugf("dropping unexpected packet: %v", reply)
 			continue
-			//	}
 		}
 
 		ch <- packet{peer: peer, seqno: reply.Seq}

diff --git a/collector/pinger/pinger.go b/collector/pinger/pinger.go
@@ -79,15 +79,15 @@ func (p *Pinger) Run(ctx context.Context, interval time.Duration) {
 	}()
 
 	var wg sync.WaitGroup
+	wg.Add(len(p.targets))
 	for _, t := range p.targets {
-		wg.Add(1)
 		go func(t *target) {
+			defer wg.Done()
 			p.runPing(ctx, t, interval)
-			wg.Done()
 		}(t)
 	}
-
 	defer wg.Wait()
+
 	for {
 		select {
 		case <-ctx.Done():
@@ -106,7 +106,9 @@ func (p *Pinger) runPing(ctx context.Context, t *target, interval time.Duration)
 		case <-ctx.Done():
 			return
 		case <-ticker.C:
-			_ = p.ping(t)
+			if err := p.ping(t); err != nil {
+				log.WithError(err).WithField("target", t.host).Error("failed to send icmp echo request")
+			}
 		}
 	}
 }
@@ -115,12 +117,12 @@ func (p *Pinger) ping(t *target) error {
 	t.lock.Lock()
 	defer t.lock.Unlock()
 
-	if err := p.conn.send(t.addr, t.seqno); err != nil {
-		return fmt.Errorf("send: %w", err)
+	err := p.conn.send(t.addr, t.seqno)
+	if err == nil {
+		t.packets[t.seqno] = time.Now()
+		t.seqno++
 	}
-	t.packets[t.seqno] = time.Now()
-	t.seqno++
-	return nil
+	return err
 }
 
 func (p *Pinger) pong(response packet) {

diff --git a/collector/tracker/tracker.go b/collector/tracker/tracker.go
@@ -1,6 +1,7 @@
 package tracker
 
 import (
+	"github.com/clambin/go-common/set"
 	"sort"
 	"sync"
 	"time"
@@ -88,12 +89,9 @@ func (t *Tracker) calculateLoss() (gap int) {
 }
 
 func unique(seqNrs []int) (result []int) {
-	uniqueSeqNrs := make(map[int]struct{})
-	for _, seqNr := range seqNrs {
-		if _, ok := uniqueSeqNrs[seqNr]; !ok {
-			uniqueSeqNrs[seqNr] = struct{}{}
-			result = append(result, seqNr)
-		}
+	s := set.Create(seqNrs)
+	for key := range s {
+		result = append(result, key)
 	}
 	sort.Ints(result)
 	return

diff --git a/go.mod b/go.mod
@@ -1,6 +1,6 @@
 module github.com/clambin/pinger
 
-go 1.17
+go 1.19
 
 require (
 	github.com/prometheus/client_golang v1.14.0
@@ -17,6 +17,7 @@ require (
 	github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cespare/xxhash/v2 v2.1.2 // indirect
+	github.com/clambin/go-common/set v0.1.2 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
 	github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect

diff --git a/go.sum b/go.sum
@@ -52,6 +52,8 @@ github.com/cespare/xxhash/v2 v2.1.2/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XL
 github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI=
 github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI=
 github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU=
+github.com/clambin/go-common/set v0.1.2 h1:Pr0FXPVJsH4dc/OelhjXYo0xVmOLIUurg3saMh8KTz8=
+github.com/clambin/go-common/set v0.1.2/go.mod h1:GFjAZynNo4BNjlx2hzDMHIU4ays7QjpJUSHXB/3b1ZE=
 github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
 github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=