From 9b281a51a2cc538a35d5c627625e9aed1aaeded4 Mon Sep 17 00:00:00 2001 From: Russ <8377044+rdubrock@users.noreply.github.com> Date: Wed, 28 Jul 2021 13:10:29 -0800 Subject: [PATCH 1/9] close the mtr channel from the sending location --- cli/cli.go | 7 +++++-- pkg/mtr/mtr.go | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/cli/cli.go b/cli/cli.go index 8ddb324..daa141d 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -62,13 +62,16 @@ var RootCmd = &cobra.Command{ go func(ch chan struct{}) { for { mu.Lock() - <-ch + _, isOpen := <-ch + if !isOpen { + mu.Unlock() + return + } render(m) mu.Unlock() } }(ch) m.Run(ch, COUNT) - close(ch) mu.Lock() render(m) mu.Unlock() diff --git a/pkg/mtr/mtr.go b/pkg/mtr/mtr.go index b675cef..52138d5 100644 --- a/pkg/mtr/mtr.go +++ b/pkg/mtr/mtr.go @@ -190,4 +190,5 @@ func (m *MTR) discover(ch chan struct{}, count int) { unknownHopsCount = 0 } } + close(ch) } From 28a4abfb8b048525bfb8d04dc404230cd59b9da4 Mon Sep 17 00:00:00 2001 From: Russ <8377044+rdubrock@users.noreply.github.com> Date: Thu, 29 Jul 2021 13:09:34 -0800 Subject: [PATCH 2/9] keep channel internal to the package, make read only --- README.md | 3 +-- cli/cli.go | 15 +++++++++------ go.mod | 2 +- install.sh | 2 +- main.go | 2 +- pkg/hop/hop.go | 2 +- pkg/mtr/mtr.go | 21 ++++++++++++--------- 7 files changed, 26 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 0d1b912..e294ef0 100644 --- a/README.md +++ b/README.md @@ -1,11 +1,10 @@ # MTR -[![Build Status](https://travis-ci.org/tonobo/mtr.svg?branch=master)](https://travis-ci.org/tonobo/mtr) A MTR implementation written in golang, completely without shell-execs. Just install with the following command: ``` -go get -u github.com/tonobo/mtr +go get -u github.com/grafana/mtr sudo setcap cap_net_raw+ep PATH-TO-GOMTR ``` or a precompiled version diff --git a/cli/cli.go b/cli/cli.go index daa141d..7dcf860 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -6,9 +6,9 @@ import ( "time" tm "github.com/buger/goterm" + "github.com/grafana/mtr/pkg/mtr" pj "github.com/hokaccha/go-prettyjson" "github.com/spf13/cobra" - "github.com/tonobo/mtr/pkg/mtr" ) var ( @@ -43,12 +43,15 @@ var RootCmd = &cobra.Command{ return err } if jsonFmt { - go func(ch chan struct{}) { + go func(ch <-chan struct{}) { for { - <-ch + _, isOpen := <-ch + if !isOpen { + return + } } }(ch) - m.Run(ch, COUNT) + m.Run(COUNT) s, err := pj.Marshal(m) if err != nil { return err @@ -59,7 +62,7 @@ var RootCmd = &cobra.Command{ fmt.Println("Start:", time.Now()) tm.Clear() mu := &sync.Mutex{} - go func(ch chan struct{}) { + go func(ch <-chan struct{}) { for { mu.Lock() _, isOpen := <-ch @@ -71,7 +74,7 @@ var RootCmd = &cobra.Command{ mu.Unlock() } }(ch) - m.Run(ch, COUNT) + m.Run(COUNT) mu.Lock() render(m) mu.Unlock() diff --git a/go.mod b/go.mod index 8ec5439..4fb1e68 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module github.com/tonobo/mtr +module github.com/grafana/mtr require ( github.com/buger/goterm v0.0.0-20181115115552-c206103e1f37 diff --git a/install.sh b/install.sh index 1375530..0f03ef1 100644 --- a/install.sh +++ b/install.sh @@ -339,7 +339,7 @@ End of functions from https://github.com/client9/shlib EOF PROJECT_NAME="mtr" -OWNER=tonobo +OWNER="grafana" REPO="mtr" BINARY=mtr FORMAT=tar.gz diff --git a/main.go b/main.go index cc529fb..004eb25 100644 --- a/main.go +++ b/main.go @@ -3,7 +3,7 @@ package main import ( "fmt" - "github.com/tonobo/mtr/cli" + "github.com/grafana/mtr/cli" ) func main() { diff --git a/pkg/hop/hop.go b/pkg/hop/hop.go index d8d60ee..968b725 100644 --- a/pkg/hop/hop.go +++ b/pkg/hop/hop.go @@ -9,7 +9,7 @@ import ( "time" gm "github.com/buger/goterm" - "github.com/tonobo/mtr/pkg/icmp" + "github.com/grafana/mtr/pkg/icmp" ) type HopStatistic struct { diff --git a/pkg/mtr/mtr.go b/pkg/mtr/mtr.go index 52138d5..e240327 100644 --- a/pkg/mtr/mtr.go +++ b/pkg/mtr/mtr.go @@ -10,8 +10,8 @@ import ( "time" gm "github.com/buger/goterm" - "github.com/tonobo/mtr/pkg/hop" - "github.com/tonobo/mtr/pkg/icmp" + "github.com/grafana/mtr/pkg/hop" + "github.com/grafana/mtr/pkg/icmp" ) type MTR struct { @@ -26,10 +26,11 @@ type MTR struct { maxHops int maxUnknownHops int ptrLookup bool + channel chan struct{} } func NewMTR(addr, srcAddr string, timeout time.Duration, interval time.Duration, - hopsleep time.Duration, maxHops, maxUnknownHops, ringBufferSize int, ptr bool) (*MTR, chan struct{}, error) { + hopsleep time.Duration, maxHops, maxUnknownHops, ringBufferSize int, ptr bool) (*MTR, <-chan struct{}, error) { if net.ParseIP(addr) == nil { addrs, err := net.LookupHost(addr) if err != nil || len(addrs) == 0 { @@ -44,6 +45,7 @@ func NewMTR(addr, srcAddr string, timeout time.Duration, interval time.Duration, srcAddr = "::" } } + ch := make(chan struct{}) return &MTR{ SrcAddress: srcAddr, interval: interval, @@ -56,7 +58,8 @@ func NewMTR(addr, srcAddr string, timeout time.Duration, interval time.Duration, ringBufferSize: ringBufferSize, maxUnknownHops: maxUnknownHops, ptrLookup: ptr, - }, make(chan struct{}), nil + channel: ch, + }, ch, nil } func (m *MTR) registerStatistic(ttl int, r icmp.ICMPReturn) *hop.HopStatistic { @@ -142,12 +145,13 @@ func (m *MTR) Render(offset int) { } } -func (m *MTR) Run(ch chan struct{}, count int) { - m.discover(ch, count) +func (m *MTR) Run(count int) { + m.discover(count) + close(m.channel) } // discover discovers all hops on the route -func (m *MTR) discover(ch chan struct{}, count int) { +func (m *MTR) discover(count int) { // Sequences are incrementing as we don't won't to get old replys which might be from a previous run (where we timed out and continued). // We can't use the process id as unique identifier as there might be multiple runs within a single binary, thus we use a fixed pseudo random number. rand.Seed(time.Now().UnixNano()) @@ -176,7 +180,7 @@ func (m *MTR) discover(ch chan struct{}, count int) { s.Dest = &ipAddr s.PID = id m.mutex.Unlock() - ch <- struct{}{} + m.channel <- struct{}{} if hopReturn.Addr == m.Address { break } @@ -190,5 +194,4 @@ func (m *MTR) discover(ch chan struct{}, count int) { unknownHopsCount = 0 } } - close(ch) } From ca1655d856830e47d59568a380c2f11d6a259fbc Mon Sep 17 00:00:00 2001 From: Russ <8377044+rdubrock@users.noreply.github.com> Date: Thu, 29 Jul 2021 14:04:05 -0800 Subject: [PATCH 3/9] revert module renaming --- README.md | 2 +- cli/cli.go | 2 +- go.mod | 2 +- install.sh | 2 +- main.go | 2 +- pkg/hop/hop.go | 2 +- pkg/mtr/mtr.go | 4 ++-- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index e294ef0..04e60bd 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ A MTR implementation written in golang, completely without shell-execs. Just install with the following command: ``` -go get -u github.com/grafana/mtr +go get -u github.com/tonobo/mtr sudo setcap cap_net_raw+ep PATH-TO-GOMTR ``` or a precompiled version diff --git a/cli/cli.go b/cli/cli.go index 7dcf860..ba4d734 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -6,9 +6,9 @@ import ( "time" tm "github.com/buger/goterm" - "github.com/grafana/mtr/pkg/mtr" pj "github.com/hokaccha/go-prettyjson" "github.com/spf13/cobra" + "github.com/tonobo/mtr/pkg/mtr" ) var ( diff --git a/go.mod b/go.mod index 4fb1e68..8ec5439 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module github.com/grafana/mtr +module github.com/tonobo/mtr require ( github.com/buger/goterm v0.0.0-20181115115552-c206103e1f37 diff --git a/install.sh b/install.sh index 0f03ef1..d1ab27f 100644 --- a/install.sh +++ b/install.sh @@ -339,7 +339,7 @@ End of functions from https://github.com/client9/shlib EOF PROJECT_NAME="mtr" -OWNER="grafana" +OWNER="tonobo" REPO="mtr" BINARY=mtr FORMAT=tar.gz diff --git a/main.go b/main.go index 004eb25..cc529fb 100644 --- a/main.go +++ b/main.go @@ -3,7 +3,7 @@ package main import ( "fmt" - "github.com/grafana/mtr/cli" + "github.com/tonobo/mtr/cli" ) func main() { diff --git a/pkg/hop/hop.go b/pkg/hop/hop.go index 968b725..d8d60ee 100644 --- a/pkg/hop/hop.go +++ b/pkg/hop/hop.go @@ -9,7 +9,7 @@ import ( "time" gm "github.com/buger/goterm" - "github.com/grafana/mtr/pkg/icmp" + "github.com/tonobo/mtr/pkg/icmp" ) type HopStatistic struct { diff --git a/pkg/mtr/mtr.go b/pkg/mtr/mtr.go index e240327..03f74cf 100644 --- a/pkg/mtr/mtr.go +++ b/pkg/mtr/mtr.go @@ -10,8 +10,8 @@ import ( "time" gm "github.com/buger/goterm" - "github.com/grafana/mtr/pkg/hop" - "github.com/grafana/mtr/pkg/icmp" + "github.com/tonobo/mtr/pkg/hop" + "github.com/tonobo/mtr/pkg/icmp" ) type MTR struct { From 0e49e040a0a589e42f58c2115ed3f55f5627663e Mon Sep 17 00:00:00 2001 From: Russ <8377044+rdubrock@users.noreply.github.com> Date: Tue, 3 Aug 2021 12:01:07 -0800 Subject: [PATCH 4/9] add the option to run with a context to control timeouts --- pkg/mtr/mtr.go | 77 +++++++++++++++++++++++++++++--------------------- 1 file changed, 45 insertions(+), 32 deletions(-) diff --git a/pkg/mtr/mtr.go b/pkg/mtr/mtr.go index 03f74cf..c448bf6 100644 --- a/pkg/mtr/mtr.go +++ b/pkg/mtr/mtr.go @@ -2,6 +2,7 @@ package mtr import ( "container/ring" + "context" "fmt" "math" "math/rand" @@ -145,13 +146,18 @@ func (m *MTR) Render(offset int) { } } -func (m *MTR) Run(count int) { - m.discover(count) +func (m *MTR) RunWithContext(ctx context.Context, count int) { + m.discover(ctx, count) close(m.channel) } +func (m *MTR) Run(count int) { + ctx := context.Background() + m.RunWithContext(ctx, count) +} + // discover discovers all hops on the route -func (m *MTR) discover(count int) { +func (m *MTR) discover(ctx context.Context, count int) { // Sequences are incrementing as we don't won't to get old replys which might be from a previous run (where we timed out and continued). // We can't use the process id as unique identifier as there might be multiple runs within a single binary, thus we use a fixed pseudo random number. rand.Seed(time.Now().UnixNano()) @@ -161,37 +167,44 @@ func (m *MTR) discover(count int) { ipAddr := net.IPAddr{IP: net.ParseIP(m.Address)} for i := 1; i <= count; i++ { - time.Sleep(m.interval) - - unknownHopsCount := 0 - for ttl := 1; ttl < m.maxHops; ttl++ { - seq++ - time.Sleep(m.hopsleep) - var hopReturn icmp.ICMPReturn - var err error - if ipAddr.IP.To4() != nil { - hopReturn, err = icmp.SendDiscoverICMP(m.SrcAddress, &ipAddr, ttl, id, m.timeout, seq) - } else { - hopReturn, err = icmp.SendDiscoverICMPv6(m.SrcAddress, &ipAddr, ttl, id, m.timeout, seq) - } - - m.mutex.Lock() - s := m.registerStatistic(ttl, hopReturn) - s.Dest = &ipAddr - s.PID = id - m.mutex.Unlock() - m.channel <- struct{}{} - if hopReturn.Addr == m.Address { - break - } - if err != nil || !hopReturn.Success { - unknownHopsCount++ - if unknownHopsCount >= m.maxUnknownHops { - break + select { + case <-ctx.Done(): + return + case <-time.After(m.interval): + unknownHopsCount := 0 + for ttl := 1; ttl < m.maxHops; ttl++ { + seq++ + select { + case <-ctx.Done(): + return + case <-time.After(m.hopsleep): + var hopReturn icmp.ICMPReturn + var err error + if ipAddr.IP.To4() != nil { + hopReturn, err = icmp.SendDiscoverICMP(m.SrcAddress, &ipAddr, ttl, id, m.timeout, seq) + } else { + hopReturn, err = icmp.SendDiscoverICMPv6(m.SrcAddress, &ipAddr, ttl, id, m.timeout, seq) + } + + m.mutex.Lock() + s := m.registerStatistic(ttl, hopReturn) + s.Dest = &ipAddr + s.PID = id + m.mutex.Unlock() + m.channel <- struct{}{} + if hopReturn.Addr == m.Address { + break + } + if err != nil || !hopReturn.Success { + unknownHopsCount++ + if unknownHopsCount >= m.maxUnknownHops { + break + } + continue + } + unknownHopsCount = 0 } - continue } - unknownHopsCount = 0 } } } From d766f9a1da4b8fa7d95b791aa9ac8e567b042fef Mon Sep 17 00:00:00 2001 From: Russ <8377044+rdubrock@users.noreply.github.com> Date: Tue, 3 Aug 2021 14:02:09 -0800 Subject: [PATCH 5/9] return success boolean --- pkg/mtr/mtr.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/pkg/mtr/mtr.go b/pkg/mtr/mtr.go index c448bf6..69484eb 100644 --- a/pkg/mtr/mtr.go +++ b/pkg/mtr/mtr.go @@ -146,18 +146,20 @@ func (m *MTR) Render(offset int) { } } -func (m *MTR) RunWithContext(ctx context.Context, count int) { - m.discover(ctx, count) +func (m *MTR) RunWithContext(ctx context.Context, count int) bool { + success := m.discover(ctx, count) close(m.channel) + return success } -func (m *MTR) Run(count int) { +func (m *MTR) Run(count int) bool { ctx := context.Background() - m.RunWithContext(ctx, count) + success := m.RunWithContext(ctx, count) + return success } // discover discovers all hops on the route -func (m *MTR) discover(ctx context.Context, count int) { +func (m *MTR) discover(ctx context.Context, count int) bool { // Sequences are incrementing as we don't won't to get old replys which might be from a previous run (where we timed out and continued). // We can't use the process id as unique identifier as there might be multiple runs within a single binary, thus we use a fixed pseudo random number. rand.Seed(time.Now().UnixNano()) @@ -169,14 +171,14 @@ func (m *MTR) discover(ctx context.Context, count int) { for i := 1; i <= count; i++ { select { case <-ctx.Done(): - return + return false case <-time.After(m.interval): unknownHopsCount := 0 for ttl := 1; ttl < m.maxHops; ttl++ { seq++ select { case <-ctx.Done(): - return + return false case <-time.After(m.hopsleep): var hopReturn icmp.ICMPReturn var err error @@ -207,4 +209,5 @@ func (m *MTR) discover(ctx context.Context, count int) { } } } + return true } From a6a10c270be1ece35a7be22cd19b062a04abbdd7 Mon Sep 17 00:00:00 2001 From: Russ <8377044+rdubrock@users.noreply.github.com> Date: Tue, 3 Aug 2021 14:15:15 -0800 Subject: [PATCH 6/9] return an error instead of a boolean --- pkg/mtr/mtr.go | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/pkg/mtr/mtr.go b/pkg/mtr/mtr.go index 69484eb..30a5403 100644 --- a/pkg/mtr/mtr.go +++ b/pkg/mtr/mtr.go @@ -15,6 +15,8 @@ import ( "github.com/tonobo/mtr/pkg/icmp" ) +var TimeoutErr = fmt.Errorf("Timed out before trace could complete") + type MTR struct { SrcAddress string `json:"source"` mutex *sync.RWMutex @@ -146,20 +148,20 @@ func (m *MTR) Render(offset int) { } } -func (m *MTR) RunWithContext(ctx context.Context, count int) bool { - success := m.discover(ctx, count) +func (m *MTR) RunWithContext(ctx context.Context, count int) error { + err := m.discover(ctx, count) close(m.channel) - return success + return err } -func (m *MTR) Run(count int) bool { +func (m *MTR) Run(count int) error { ctx := context.Background() - success := m.RunWithContext(ctx, count) - return success + err := m.RunWithContext(ctx, count) + return err } // discover discovers all hops on the route -func (m *MTR) discover(ctx context.Context, count int) bool { +func (m *MTR) discover(ctx context.Context, count int) error { // Sequences are incrementing as we don't won't to get old replys which might be from a previous run (where we timed out and continued). // We can't use the process id as unique identifier as there might be multiple runs within a single binary, thus we use a fixed pseudo random number. rand.Seed(time.Now().UnixNano()) @@ -171,14 +173,14 @@ func (m *MTR) discover(ctx context.Context, count int) bool { for i := 1; i <= count; i++ { select { case <-ctx.Done(): - return false + return TimeoutErr case <-time.After(m.interval): unknownHopsCount := 0 for ttl := 1; ttl < m.maxHops; ttl++ { seq++ select { case <-ctx.Done(): - return false + return TimeoutErr case <-time.After(m.hopsleep): var hopReturn icmp.ICMPReturn var err error @@ -209,5 +211,5 @@ func (m *MTR) discover(ctx context.Context, count int) bool { } } } - return true + return nil } From ee05b0e4388ef3aece29369aca1fc321b071ea48 Mon Sep 17 00:00:00 2001 From: Russ <8377044+rdubrock@users.noreply.github.com> Date: Wed, 4 Aug 2021 14:15:11 -0800 Subject: [PATCH 7/9] return an error on failure --- go.mod | 7 +++++-- go.sum | 19 +++++++++++++++---- pkg/mtr/mtr.go | 15 +++++++++------ 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/go.mod b/go.mod index 8ec5439..7478759 100644 --- a/go.mod +++ b/go.mod @@ -1,13 +1,16 @@ module github.com/tonobo/mtr +go 1.16 + require ( github.com/buger/goterm v0.0.0-20181115115552-c206103e1f37 github.com/fatih/color v1.7.0 // indirect github.com/hokaccha/go-prettyjson v0.0.0-20180920040306-f579f869bbfe + github.com/inconshreveable/mousetrap v1.0.0 // indirect github.com/mattn/go-colorable v0.1.0 // indirect github.com/mattn/go-isatty v0.0.4 // indirect github.com/spf13/cobra v0.0.3 github.com/spf13/pflag v1.0.3 // indirect - golang.org/x/net v0.0.0-20190213061140-3a22650c66bd - golang.org/x/sys v0.0.0-20190220154126-629670e5acc5 // indirect + golang.org/x/net v0.0.0-20201021035429-f5854403a974 + golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 // indirect ) diff --git a/go.sum b/go.sum index f14dd48..07cb455 100644 --- a/go.sum +++ b/go.sum @@ -4,6 +4,8 @@ github.com/fatih/color v1.7.0 h1:DkWD4oS2D8LGGgTQ6IvwJJXSL5Vp2ffcQg58nFV38Ys= github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/hokaccha/go-prettyjson v0.0.0-20180920040306-f579f869bbfe h1:MCgzztuoH5LZNr9AkIaicIDvCfACu11KUCCZQnRHDC0= github.com/hokaccha/go-prettyjson v0.0.0-20180920040306-f579f869bbfe/go.mod h1:pFlLw2CfqZiIBOx6BuCeRLCrfxBJipTY0nIOF/VbGcI= +github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM= +github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= github.com/mattn/go-colorable v0.1.0 h1:v2XXALHHh6zHfYTJ+cSkwtyffnaOyR1MXaA91mTrb8o= github.com/mattn/go-colorable v0.1.0/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= github.com/mattn/go-isatty v0.0.4 h1:bnP0vzxcAdeI1zdubAl5PjU6zsERjGZb7raWodagDYs= @@ -12,7 +14,16 @@ github.com/spf13/cobra v0.0.3 h1:ZlrZ4XsMRm04Fr5pSFxBgfND2EBVa1nLpiy1stUsX/8= github.com/spf13/cobra v0.0.3/go.mod h1:1l0Ry5zgKvJasoi3XT1TypsSe7PqH0Sj9dhYf7v3XqQ= github.com/spf13/pflag v1.0.3 h1:zPAT6CGy6wXeQ7NtTnaTerfKOsV6V6F8agHXFiazDkg= github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= -golang.org/x/net v0.0.0-20190213061140-3a22650c66bd h1:HuTn7WObtcDo9uEEU7rEqL0jYthdXAmZ6PP+meazmaU= -golang.org/x/net v0.0.0-20190213061140-3a22650c66bd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= -golang.org/x/sys v0.0.0-20190220154126-629670e5acc5 h1:3Nsfe5Xa1wTt01QxlAFIY5j9ycDtS+d7mhvI8ZY5bn0= -golang.org/x/sys v0.0.0-20190220154126-629670e5acc5/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/net v0.0.0-20201021035429-f5854403a974 h1:IX6qOQeG5uLjB/hjjwjedwfjND0hgjPMMyO1RoIXQNI= +golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k= +golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= diff --git a/pkg/mtr/mtr.go b/pkg/mtr/mtr.go index 30a5403..8b37f9f 100644 --- a/pkg/mtr/mtr.go +++ b/pkg/mtr/mtr.go @@ -15,7 +15,8 @@ import ( "github.com/tonobo/mtr/pkg/icmp" ) -var TimeoutErr = fmt.Errorf("Timed out before trace could complete") +var ErrTimeout = fmt.Errorf("timed out before trace could complete") +var ErrMaxUnknownHops = fmt.Errorf("max unknown hops exceeded") type MTR struct { SrcAddress string `json:"source"` @@ -173,14 +174,16 @@ func (m *MTR) discover(ctx context.Context, count int) error { for i := 1; i <= count; i++ { select { case <-ctx.Done(): - return TimeoutErr + return ErrTimeout case <-time.After(m.interval): unknownHopsCount := 0 + + ttlLoop: for ttl := 1; ttl < m.maxHops; ttl++ { seq++ select { case <-ctx.Done(): - return TimeoutErr + return ErrTimeout case <-time.After(m.hopsleep): var hopReturn icmp.ICMPReturn var err error @@ -197,14 +200,14 @@ func (m *MTR) discover(ctx context.Context, count int) error { m.mutex.Unlock() m.channel <- struct{}{} if hopReturn.Addr == m.Address { - break + break ttlLoop } if err != nil || !hopReturn.Success { unknownHopsCount++ if unknownHopsCount >= m.maxUnknownHops { - break + return ErrMaxUnknownHops } - continue + continue ttlLoop } unknownHopsCount = 0 } From 89b054ac21be3659e7f9484ad83792db50866f63 Mon Sep 17 00:00:00 2001 From: Russ <8377044+rdubrock@users.noreply.github.com> Date: Wed, 3 Nov 2021 13:10:02 -0800 Subject: [PATCH 8/9] expose ptr lookup function externally --- pkg/hop/hop.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/hop/hop.go b/pkg/hop/hop.go index d8d60ee..4f80986 100644 --- a/pkg/hop/hop.go +++ b/pkg/hop/hop.go @@ -145,7 +145,7 @@ func (h *HopStatistic) Render(ptrLookup bool) { l := fmt.Sprintf("%d", h.RingBufferSize) gm.Printf("%3d:|-- %-20s %5.1f%% %4d %6.1f %6.1f %6.1f %6.1f %"+l+"s\n", h.TTL, - fmt.Sprintf("%.20s", h.lookupAddr(ptrLookup, 0)), + fmt.Sprintf("%.20s", h.LookupAddr(ptrLookup, 0)), h.Loss(), h.Sent, h.Last.Elapsed.Seconds()*1000, @@ -156,8 +156,11 @@ func (h *HopStatistic) Render(ptrLookup bool) { ) } -func (h *HopStatistic) lookupAddr(ptrLookup bool, index int) string { - addr := "???" +func (h *HopStatistic) LookupAddr(ptrLookup bool, index int) string { + if h.dnsCache == nil { + h.dnsCache = map[string]string{} + } + addr := "" if h.Targets[index] != "" { addr = h.Targets[index] if ptrLookup { From d4e77dda8ebebff4e4f66f255e2677a1c207bcee Mon Sep 17 00:00:00 2001 From: "Marcelo E. Magallon" Date: Fri, 21 Oct 2022 16:33:47 -0600 Subject: [PATCH 9/9] Rework the logic that handles "replies" Modify the logic so that it's more robust against errors and malformed packets. Also try to make sure that we are matching against the correct bytes in the response. Signed-off-by: Marcelo E. Magallon --- pkg/icmp/icmp.go | 102 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 82 insertions(+), 20 deletions(-) diff --git a/pkg/icmp/icmp.go b/pkg/icmp/icmp.go index 3e61b76..ab7a41f 100644 --- a/pkg/icmp/icmp.go +++ b/pkg/icmp/icmp.go @@ -155,15 +155,41 @@ func listenForSpecific6(conn *icmp.PacketConn, deadline time.Time, neededPeer st } if x.Type.(ipv6.ICMPType) == ipv6.ICMPTypeTimeExceeded { - body := x.Body.(*icmp.TimeExceeded).Data - x, _ := icmp.ParseMessage(ProtocolIPv6ICMP, body[40:]) - switch x.Body.(type) { + msg, ok := x.Body.(*icmp.TimeExceeded) + if !ok { // this should never happen + continue + } + + // Be defensive regarding the junk we might end up + // getting here. Make sure the ICMP packet contains. an + // IPv6 packet. + _, err := ipv6.ParseHeader(msg.Data) + if err != nil { + continue + } + + echoPkt := msg.Data[ipv6.HeaderLen:] + + icmpMsg, err := icmp.ParseMessage(ProtocolIPv6ICMP, echoPkt) + if err != nil { + // We cannot error out here because we + // don't know if this packet was the + // one we were expecting or the one we + // were expecting is still in flight. + // + // To make that decision we need to be + // able to parse the packet. + continue + } + + switch body := icmpMsg.Body.(type) { case *icmp.Echo: - echoBody := x.Body.(*icmp.Echo) - if echoBody.Seq == needSeq && echoBody.ID == id { + if body.Seq == needSeq && body.ID == id { return peer.String(), []byte{}, nil } + continue + default: // ignore } @@ -206,22 +232,58 @@ func listenForSpecific4(conn *icmp.PacketConn, deadline time.Time, neededPeer st continue } - if typ, ok := x.Type.(ipv4.ICMPType); ok && typ.String() == "time exceeded" { - body := x.Body.(*icmp.TimeExceeded).Data - - index := bytes.Index(body, sent[:4]) - if index > 0 { - x, _ := icmp.ParseMessage(ProtocolICMP, body[index:]) - switch x.Body.(type) { - case *icmp.Echo: - echoBody := x.Body.(*icmp.Echo) - if echoBody.Seq == needSeq && echoBody.ID == pid { - return peer.String(), []byte{}, nil - } - continue - default: - // ignore + if typ, ok := x.Type.(ipv4.ICMPType); ok && typ == ipv4.ICMPTypeTimeExceeded { + msg, ok := x.Body.(*icmp.TimeExceeded) + if !ok { // this should never happen + continue + } + + // Be defensive regarding the junk we might end up + // getting here. Make sure the ICMP packet contains: + // 1. an IPv4 packet + hdr, err := ipv4.ParseHeader(msg.Data) + if err != nil { + continue + } + + // 2. containing an ICMP packet + if hdr.Protocol != ProtocolICMP { + continue + } + + // 3. with a length that makes sense + echoPkt := msg.Data[hdr.Len:] + if len(sent) < 4 || len(echoPkt) < 4 { + continue + } + + // The echo packet should match what we sent. + if !bytes.Equal(sent[:4], echoPkt[:4]) { + continue + } + + icmpMsg, err := icmp.ParseMessage(ProtocolICMP, echoPkt) + if err != nil { + // We cannot error out here because we + // don't know if this packet was the + // one we were expecting or the one we + // were expecting is still in flight. + // + // To make that decision we need to be + // able to parse the packet. + continue + } + + switch body := icmpMsg.Body.(type) { + case *icmp.Echo: + if body.Seq == needSeq && body.ID == pid { + return peer.String(), []byte{}, nil } + + continue + + default: + // ignore } }