From dce585d52842c738ed6e9a319b4e80f79e289eb9 Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Mon, 3 Oct 2022 08:05:06 +0200 Subject: [PATCH 01/23] added json/yaml output for scion showpaths --- scion/cmd/scion/showpaths.go | 45 +++++++++++++++++++++++++----------- scion/showpaths/BUILD.bazel | 1 + scion/showpaths/showpaths.go | 33 +++++++++++++++----------- 3 files changed, 52 insertions(+), 27 deletions(-) diff --git a/scion/cmd/scion/showpaths.go b/scion/cmd/scion/showpaths.go index 9a0e49164c..04980f2636 100644 --- a/scion/cmd/scion/showpaths.go +++ b/scion/cmd/scion/showpaths.go @@ -41,6 +41,7 @@ func newShowpaths(pather CommandPather) *cobra.Command { logLevel string noColor bool tracer string + format string } var cmd = &cobra.Command{ @@ -61,7 +62,7 @@ By default, the paths are probed. Paths served from the SCION Deamon's might not forward traffic successfully (e.g. if a network link went down, or there is a black hole on the path). To disable path probing, set the appropriate flag. -'showpaths' can be instructed to output the paths as json using the the \--json flag. +'showpaths' can be instructed to output the paths in a specific format using the \--format flag. If no alive path is discovered, json output is not enabled, and probing is not disabled, showpaths will exit with the code 1. @@ -81,6 +82,12 @@ On other errors, showpaths will exit with code 2. return serrors.WrapStr("setting up tracing", err) } defer closer() + switch flags.format { + case "human", "json", "yaml": + break + default: + return serrors.New("format not supported", "format", flags.format) + } cmd.SilenceUsage = true @@ -107,20 +114,27 @@ On other errors, showpaths will exit with code 2. if err != nil { return err } - if flags.json { + + switch flags.format { + case "human": + if res.IsLocal() { + fmt.Fprintf(os.Stdout, "Empty path, destination is local AS %s\n", res.Destination) + return nil + } + fmt.Fprintln(os.Stdout, "Available paths to", res.Destination) + if len(res.Paths) == 0 { + return app.WithExitCode(serrors.New("no path found"), 1) + } + res.Human(os.Stdout, flags.extended, !flags.noColor) + if res.Alive() == 0 && !flags.cfg.NoProbe { + return app.WithExitCode(serrors.New("no path alive"), 1) + } + case "json": return res.JSON(os.Stdout) - } - if res.IsLocal() { - fmt.Fprintf(os.Stdout, "Empty path, destination is local AS %s\n", res.Destination) - return nil - } - fmt.Fprintln(os.Stdout, "Available paths to", res.Destination) - if len(res.Paths) == 0 { - return app.WithExitCode(serrors.New("no path found"), 1) - } - res.Human(os.Stdout, flags.extended, !flags.noColor) - if res.Alive() == 0 && !flags.cfg.NoProbe { - return app.WithExitCode(serrors.New("no path alive"), 1) + case "yaml": + return res.YAML(os.Stdout) + default: + return serrors.New("output format not supported", "format", flags.format) } return nil }, @@ -139,9 +153,12 @@ On other errors, showpaths will exit with code 2. "Do not probe the paths and print the health status") cmd.Flags().BoolVarP(&flags.json, "json", "j", false, "Write the output as machine readable json") + cmd.Flags().StringVar(&flags.format, "format", "human", + "Specify the output format (json|yaml)") cmd.Flags().BoolVar(&flags.noColor, "no-color", false, "disable colored output") cmd.Flags().StringVar(&flags.logLevel, "log.level", "", app.LogLevelUsage) cmd.Flags().StringVar(&flags.tracer, "tracing.agent", "", "Tracing agent address") cmd.Flags().BoolVar(&flags.cfg.Epic, "epic", false, "Enable EPIC.") + cmd.Flags().MarkDeprecated("json", "json flag is deprecated, use format flag") return cmd } diff --git a/scion/showpaths/BUILD.bazel b/scion/showpaths/BUILD.bazel index 3462edcb3e..c3a838b16c 100644 --- a/scion/showpaths/BUILD.bazel +++ b/scion/showpaths/BUILD.bazel @@ -16,5 +16,6 @@ go_library( "//pkg/snet:go_default_library", "//private/app/path:go_default_library", "//private/app/path/pathprobe:go_default_library", + "@in_gopkg_yaml_v2//:go_default_library", ], ) diff --git a/scion/showpaths/showpaths.go b/scion/showpaths/showpaths.go index 0973d2316b..e8983e9438 100644 --- a/scion/showpaths/showpaths.go +++ b/scion/showpaths/showpaths.go @@ -33,27 +33,28 @@ import ( "github.com/scionproto/scion/pkg/snet" "github.com/scionproto/scion/private/app/path" "github.com/scionproto/scion/private/app/path/pathprobe" + "gopkg.in/yaml.v2" ) // Result contains all the discovered paths. type Result struct { - LocalIA addr.IA `json:"local_isd_as"` - Destination addr.IA `json:"destination"` - Paths []Path `json:"paths,omitempty"` + LocalIA addr.IA `json:"local_isd_as" yaml:"local_isd_as"` + Destination addr.IA `json:"destination" yaml:"destination"` + Paths []Path `json:"paths,omitempty" yaml:"paths,omitempty"` } // Path holds information about the discovered path. type Path struct { - FullPath snet.Path `json:"-"` - Fingerprint string `json:"fingerprint"` - Hops []Hop `json:"hops"` - NextHop string `json:"next_hop"` - Expiry time.Time `json:"expiry"` - MTU uint16 `json:"mtu"` - Latency []time.Duration `json:"latency"` - Status string `json:"status,omitempty"` - StatusInfo string `json:"status_info,omitempty"` - Local net.IP `json:"local_ip,omitempty"` + FullPath snet.Path `json:"-" yaml:"-"` + Fingerprint string `json:"fingerprint" yaml:"fingerprint"` + Hops []Hop `json:"hops" yaml:"hops"` + NextHop string `json:"next_hop" yaml:"next_hop"` + Expiry time.Time `json:"expiry" yaml:"expiry"` + MTU uint16 `json:"mtu" yaml:"mtu"` + Latency []time.Duration `json:"latency" yaml:"latency"` + Status string `json:"status,omitempty" yaml:"status,omitempty"` + StatusInfo string `json:"status_info,omitempty" yaml:"status_info,omitempty"` + Local net.IP `json:"local_ip,omitempty" yaml:"local_ip,omitempty"` } // Hop represents an hop on the path. @@ -294,6 +295,12 @@ func (r Result) JSON(w io.Writer) error { return enc.Encode(r) } +// JSON writes the showpaths result as a yaml object to the writer. +func (r Result) YAML(w io.Writer) error { + enc := yaml.NewEncoder(w) + return enc.Encode(r) +} + // IsLocal returns true iff Source and Destination AS are identical func (r Result) IsLocal() bool { return r.LocalIA == r.Destination From 0d072804ed13b8ecd8cc3b1904d57fcfe74c3357 Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Thu, 27 Oct 2022 13:34:52 +0200 Subject: [PATCH 02/23] initial traceroute try --- scion/cmd/scion/showpaths.go | 2 +- scion/cmd/scion/traceroute.go | 29 ++++++++++++++++++++++++++--- scion/traceroute/BUILD.bazel | 1 + scion/traceroute/traceroute.go | 23 +++++++++++++++++++++++ 4 files changed, 51 insertions(+), 4 deletions(-) diff --git a/scion/cmd/scion/showpaths.go b/scion/cmd/scion/showpaths.go index 04980f2636..d40d533273 100644 --- a/scion/cmd/scion/showpaths.go +++ b/scion/cmd/scion/showpaths.go @@ -154,7 +154,7 @@ On other errors, showpaths will exit with code 2. cmd.Flags().BoolVarP(&flags.json, "json", "j", false, "Write the output as machine readable json") cmd.Flags().StringVar(&flags.format, "format", "human", - "Specify the output format (json|yaml)") + "Specify the output format (human|json|yaml)") cmd.Flags().BoolVar(&flags.noColor, "no-color", false, "disable colored output") cmd.Flags().StringVar(&flags.logLevel, "log.level", "", app.LogLevelUsage) cmd.Flags().StringVar(&flags.tracer, "tracing.agent", "", "Tracing agent address") diff --git a/scion/cmd/scion/traceroute.go b/scion/cmd/scion/traceroute.go index 0c5b643db4..907827d1a1 100644 --- a/scion/cmd/scion/traceroute.go +++ b/scion/cmd/scion/traceroute.go @@ -51,10 +51,11 @@ func newTraceroute(pather CommandPather) *cobra.Command { timeout time.Duration tracer string epic bool + format string } var cmd = &cobra.Command{ - Use: "traceroute [flags] ", + Use: "traceroute [flags] TEST", Aliases: []string{"tr"}, Short: "Trace the SCION route to a remote SCION AS using SCMP traceroute packets", Example: fmt.Sprintf(" %[1]s traceroute 1-ff00:0:110,10.0.0.1", pather.CommandPath()), @@ -79,6 +80,12 @@ On other errors, traceroute will exit with code 2. return serrors.WrapStr("setting up tracing", err) } defer closer() + switch flags.format { + case "human", "json", "yaml": + break + default: + return serrors.New("format not supported", "format", flags.format) + } cmd.SilenceUsage = true @@ -146,6 +153,9 @@ On other errors, traceroute will exit with code 2. Host: &net.UDPAddr{IP: localIP}, } ctx = app.WithSignal(traceCtx, os.Interrupt, syscall.SIGTERM) + res := &traceroute.Result{ + Updates: []traceroute.Update{}, + } var stats traceroute.Stats cfg := traceroute.Config{ Dispatcher: reliable.NewDispatcher(dispatcher), @@ -157,8 +167,13 @@ On other errors, traceroute will exit with code 2. ProbesPerHop: 3, ErrHandler: func(err error) { fmt.Fprintf(os.Stderr, "ERROR: %s\n", err) }, UpdateHandler: func(u traceroute.Update) { - fmt.Printf("%d %s %s\n", u.Index, fmtRemote(u.Remote, u.Interface), - fmtRTTs(u.RTTs, flags.timeout)) + fmt.Printf("next hop: %s\n", u.Remote.NextHop) + fmt.Printf("IP: %s\n", u.Remote.Host.IP) + res.Updates = append(res.Updates, u) + if flags.format == "human" { + fmt.Printf("%d %s %s\n", u.Index, fmtRemote(u.Remote, u.Interface), + fmtRTTs(u.RTTs, flags.timeout)) + } }, EPIC: flags.epic, } @@ -169,6 +184,12 @@ On other errors, traceroute will exit with code 2. if stats.Sent != stats.Recv { return app.WithExitCode(serrors.New("packets were lost"), 1) } + switch flags.format { + case "json": + return res.JSON(os.Stdout) + case "yaml": + return res.YAML(os.Stdout) + } return nil }, } @@ -182,6 +203,8 @@ On other errors, traceroute will exit with code 2. cmd.Flags().StringVar(&flags.logLevel, "log.level", "", app.LogLevelUsage) cmd.Flags().StringVar(&flags.tracer, "tracing.agent", "", "Tracing agent address") cmd.Flags().BoolVar(&flags.epic, "epic", false, "Enable EPIC.") + cmd.Flags().StringVar(&flags.format, "format", "human", + "Specify the output format (human|json|yaml)") return cmd } diff --git a/scion/traceroute/BUILD.bazel b/scion/traceroute/BUILD.bazel index b58bd9fe4a..d755dafd62 100644 --- a/scion/traceroute/BUILD.bazel +++ b/scion/traceroute/BUILD.bazel @@ -14,5 +14,6 @@ go_library( "//pkg/snet:go_default_library", "//pkg/snet/path:go_default_library", "//pkg/sock/reliable:go_default_library", + "@in_gopkg_yaml_v2//:go_default_library", ], ) diff --git a/scion/traceroute/traceroute.go b/scion/traceroute/traceroute.go index 31b016c2b4..0cd50d4ddb 100644 --- a/scion/traceroute/traceroute.go +++ b/scion/traceroute/traceroute.go @@ -17,6 +17,8 @@ package traceroute import ( "context" + "encoding/json" + "io" "math/rand" "net" "time" @@ -29,6 +31,7 @@ import ( "github.com/scionproto/scion/pkg/snet" snetpath "github.com/scionproto/scion/pkg/snet/path" "github.com/scionproto/scion/pkg/sock/reliable" + "gopkg.in/yaml.v2" ) // Update contains the information for a single hop. @@ -54,6 +57,12 @@ type Stats struct { Sent, Recv uint } +type Result struct { + // LocalIA addr.IA `json:"local_isd_as" yaml:"local_isd_as"` + // Destination addr.IA `json:"destination" yaml:"destination"` + Updates []Update `json:"updates,omitempty" yaml:"updates,omitempty"` +} + // Config configures the traceroute run. type Config struct { Dispatcher reliable.Dispatcher @@ -330,3 +339,17 @@ func (h scmpHandler) handle(pkt *snet.Packet) (snet.SCMPTracerouteReply, error) } return r, nil } + +// JSON writes the showpaths result as a json object to the writer. +func (r Result) JSON(w io.Writer) error { + enc := json.NewEncoder(w) + enc.SetIndent("", " ") + enc.SetEscapeHTML(false) + return enc.Encode(r) +} + +// JSON writes the showpaths result as a yaml object to the writer. +func (r Result) YAML(w io.Writer) error { + enc := yaml.NewEncoder(w) + return enc.Encode(r) +} From 0b51623ab3c22bed6cf36cbe1f5ada98dcd525b0 Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Thu, 27 Oct 2022 19:16:58 +0200 Subject: [PATCH 03/23] structured output working --- pkg/snet/path/epic.go | 13 ++++++ pkg/snet/path/path.go | 18 +++++++++ scion/cmd/scion/ping.go | 74 +++++++++++++++++++++++++++++----- scion/cmd/scion/showpaths.go | 14 ++++--- scion/cmd/scion/traceroute.go | 50 ++++++++++++++++------- scion/ping/BUILD.bazel | 1 + scion/ping/ping.go | 71 ++++++++++++++++++++++++++++++++ scion/traceroute/traceroute.go | 65 +++++++++++++++++++++++++---- 8 files changed, 269 insertions(+), 37 deletions(-) diff --git a/pkg/snet/path/epic.go b/pkg/snet/path/epic.go index a3e44bb585..1865c03b2a 100644 --- a/pkg/snet/path/epic.go +++ b/pkg/snet/path/epic.go @@ -47,6 +47,19 @@ func NewEPICDataplanePath(p SCION, auths snet.EpicAuths) (*EPIC, error) { return epicPath, nil } +func CloneEICDataplanePath(p *EPIC) *EPIC { + res := &EPIC{} + res.AuthPHVF = make([]byte, len(p.AuthPHVF)) + copy(res.AuthPHVF, p.AuthPHVF) + res.AuthLHVF = make([]byte, len(p.AuthLHVF)) + copy(res.AuthLHVF, p.AuthLHVF) + res.SCION = make([]byte, len(p.SCION)) + copy(res.SCION, p.SCION) + res.counter = p.counter + res.mu = sync.Mutex{} + return res +} + func (e *EPIC) SetPath(s *slayers.SCION) error { e.mu.Lock() defer e.mu.Unlock() diff --git a/pkg/snet/path/path.go b/pkg/snet/path/path.go index 08748964d9..1d41233b23 100644 --- a/pkg/snet/path/path.go +++ b/pkg/snet/path/path.go @@ -38,6 +38,11 @@ type Path struct { Meta snet.PathMetadata } +type Hop struct { + Interface int `json:"interface" yaml:"interface"` + IA string `json:"isd_as" yaml:"isd_as"` +} + func (p Path) UnderlayNextHop() *net.UDPAddr { if p.NextHop == nil { return nil @@ -87,3 +92,16 @@ func fmtInterfaces(ifaces []snet.PathInterface) []string { hops = append(hops, fmt.Sprintf("%d %s", intf.ID, intf.IA)) return hops } + +func GetHops(path snet.Path) []Hop { + ifaces := path.Metadata().Interfaces + var hops []Hop + if len(ifaces) == 0 { + return hops + } + for i := range ifaces { + intf := ifaces[i] + hops = append(hops, Hop{IA: intf.IA.String(), Interface: int(intf.ID)}) + } + return hops +} diff --git a/scion/cmd/scion/ping.go b/scion/cmd/scion/ping.go index 9500fdf9d9..f8e8f1dfa1 100644 --- a/scion/cmd/scion/ping.go +++ b/scion/cmd/scion/ping.go @@ -17,6 +17,7 @@ package main import ( "context" "fmt" + "io" "math" "net" "os" @@ -57,6 +58,7 @@ func newPing(pather CommandPather) *cobra.Command { timeout time.Duration tracer string epic bool + format string } var cmd = &cobra.Command{ @@ -90,6 +92,15 @@ On other errors, ping will exit with code 2. return serrors.WrapStr("setting up tracing", err) } defer closer() + var cmdout io.Writer + switch flags.format { + case "human": + cmdout = os.Stdout + case "json", "yaml": + cmdout = io.Discard + default: + return serrors.New("format not supported", "format", flags.format) + } cmd.SilenceUsage = true @@ -172,9 +183,9 @@ On other errors, ping will exit with code 2. return serrors.WrapStr("resolving local address", err) } - fmt.Printf("Resolved local address:\n %s\n", localIP) + fmt.Fprintf(cmdout, "Resolved local address:\n %s\n", localIP) } - fmt.Printf("Using path:\n %s\n\n", path) + fmt.Fprintf(cmdout, "Using path:\n %s\n\n", path) span.SetTag("src.host", localIP) local := &snet.UDPAddr{ IA: info.IA, @@ -205,7 +216,7 @@ On other errors, ping will exit with code 2. if err != nil { return err } - fmt.Printf("PING %s pld=%dB scion_pkt=%dB\n", remote, pldSize, pktSize) + fmt.Fprintf(cmdout, "PING %s pld=%dB scion_pkt=%dB\n", remote, pldSize, pktSize) start := time.Now() ctx = app.WithSignal(traceCtx, os.Interrupt, syscall.SIGTERM) @@ -213,6 +224,20 @@ On other errors, ping will exit with code 2. if count == 0 { count = math.MaxUint16 } + + res := ping.Result{ + ScionPacketSize: pktSize, + Path: ping.Path{ + Expiry: path.Metadata().Expiry, + Fingerprint: snet.Fingerprint(path).String(), + Hops: snetpath.GetHops(path), + Latency: path.Metadata().Latency, + Mtu: int(path.Metadata().MTU), + NextHop: path.UnderlayNextHop().String(), + }, + PayloadSize: pldSize, + } + stats, err := ping.Run(ctx, ping.Config{ Dispatcher: reliable.NewDispatcher(dispatcher), Attempts: count, @@ -225,24 +250,50 @@ On other errors, ping will exit with code 2. fmt.Fprintf(os.Stderr, "ERROR: %s\n", err) }, UpdateHandler: func(update ping.Update) { + state := "success" var additional string switch update.State { case ping.AfterTimeout: additional = " state=After timeout" + state = "after_timeout" case ping.OutOfOrder: additional = " state=Out of Order" + state = "out_of_order" case ping.Duplicate: additional = " state=Duplicate" + state = "duplicate" } - fmt.Fprintf(os.Stdout, "%d bytes from %s,%s: scmp_seq=%d time=%s%s\n", + res.Replies = append(res.Replies, ping.PingUpdate{ + RoundTripTime: update.RTT.String(), + ScionPacketSize: update.Size, + ScmpSeq: update.Sequence, + SourceHost: update.Source.Host.String(), + SourceIA: update.Source.IA.String(), + State: state, + }) + fmt.Fprintf(cmdout, "%d bytes from %s,%s: scmp_seq=%d time=%s%s\n", update.Size, update.Source.IA, update.Source.Host, update.Sequence, update.RTT, additional) }, }) - pingSummary(stats, remote, time.Since(start)) + pktLoss := pingSummary(stats, remote, time.Since(start), cmdout) if err != nil { return err } + res.Statistics = ping.PingStatistics{ + Received: stats.Received, + Sent: stats.Sent, + Loss: pktLoss, + Time: time.Since(start), + } + + switch flags.format { + case "json": + return res.JSON(os.Stdout) + case "yaml": + return res.YAML(os.Stdout) + } + if stats.Received == 0 { return app.WithExitCode(serrors.New("no reply packet received"), 1) } @@ -266,7 +317,7 @@ the SCION path.`, ) cmd.Flags().UintVar(&flags.pktSize, "packet-size", 0, `number of bytes to be sent including the SCION Header and SCMP echo header, -the desired size must provide enough space for the required headers. This flag +the desired size must provide enough space for the required headers. This flag overrides the 'payload_size' flag.`, ) cmd.Flags().BoolVar(&flags.maxMTU, "max-mtu", false, @@ -276,6 +327,8 @@ SCMP echo header and payload are equal to the MTU of the path. This flag overrid cmd.Flags().StringVar(&flags.logLevel, "log.level", "", app.LogLevelUsage) cmd.Flags().StringVar(&flags.tracer, "tracing.agent", "", "Tracing agent address") cmd.Flags().BoolVar(&flags.epic, "epic", false, "Enable EPIC for path probing.") + cmd.Flags().StringVar(&flags.format, "format", "human", + "Specify the output format (human|json|yaml)") return cmd } @@ -287,12 +340,13 @@ func calcMaxPldSize(local, remote *snet.UDPAddr, mtu int) (int, error) { return mtu - overhead, nil } -func pingSummary(stats ping.Stats, remote *snet.UDPAddr, run time.Duration) { +func pingSummary(stats ping.Stats, remote *snet.UDPAddr, run time.Duration, cmdout io.Writer) int { var pktLoss int if stats.Sent != 0 { pktLoss = 100 - stats.Received*100/stats.Sent } - fmt.Printf("\n--- %s,%s statistics ---\n", remote.IA, remote.Host.IP) - fmt.Printf("%d packets transmitted, %d received, %d%% packet loss, time %v\n", - stats.Sent, stats.Received, pktLoss, run.Round(time.Microsecond)) + fmt.Fprintf(cmdout, "\n--- %s,%s statistics ---\n", remote.IA, remote.Host.IP) + fmt.Fprintf(cmdout, "%d packets transmitted, %d received, %d%% packet loss, time %v\n", + stats.Sent, stats.Received, stats.Loss, run.Round(time.Microsecond)) + return pktLoss } diff --git a/scion/cmd/scion/showpaths.go b/scion/cmd/scion/showpaths.go index d40d533273..1ace83620b 100644 --- a/scion/cmd/scion/showpaths.go +++ b/scion/cmd/scion/showpaths.go @@ -17,6 +17,7 @@ package main import ( "context" "fmt" + "io" "os" "time" @@ -82,9 +83,12 @@ On other errors, showpaths will exit with code 2. return serrors.WrapStr("setting up tracing", err) } defer closer() + var cmdout io.Writer switch flags.format { - case "human", "json", "yaml": - break + case "human": + cmdout = os.Stdout + case "json", "yaml": + cmdout = io.Discard default: return serrors.New("format not supported", "format", flags.format) } @@ -118,14 +122,14 @@ On other errors, showpaths will exit with code 2. switch flags.format { case "human": if res.IsLocal() { - fmt.Fprintf(os.Stdout, "Empty path, destination is local AS %s\n", res.Destination) + fmt.Fprintf(cmdout, "Empty path, destination is local AS %s\n", res.Destination) return nil } - fmt.Fprintln(os.Stdout, "Available paths to", res.Destination) + fmt.Fprintln(cmdout, "Available paths to", res.Destination) if len(res.Paths) == 0 { return app.WithExitCode(serrors.New("no path found"), 1) } - res.Human(os.Stdout, flags.extended, !flags.noColor) + res.Human(cmdout, flags.extended, !flags.noColor) if res.Alive() == 0 && !flags.cfg.NoProbe { return app.WithExitCode(serrors.New("no path alive"), 1) } diff --git a/scion/cmd/scion/traceroute.go b/scion/cmd/scion/traceroute.go index 907827d1a1..187c275e6b 100644 --- a/scion/cmd/scion/traceroute.go +++ b/scion/cmd/scion/traceroute.go @@ -17,6 +17,7 @@ package main import ( "context" "fmt" + "io" "net" "os" "strings" @@ -30,6 +31,7 @@ import ( "github.com/scionproto/scion/pkg/private/serrors" "github.com/scionproto/scion/pkg/snet" "github.com/scionproto/scion/pkg/snet/addrutil" + snetpath "github.com/scionproto/scion/pkg/snet/path" "github.com/scionproto/scion/pkg/sock/reliable" "github.com/scionproto/scion/private/app" "github.com/scionproto/scion/private/app/flag" @@ -55,7 +57,7 @@ func newTraceroute(pather CommandPather) *cobra.Command { } var cmd = &cobra.Command{ - Use: "traceroute [flags] TEST", + Use: "traceroute [flags] ", Aliases: []string{"tr"}, Short: "Trace the SCION route to a remote SCION AS using SCMP traceroute packets", Example: fmt.Sprintf(" %[1]s traceroute 1-ff00:0:110,10.0.0.1", pather.CommandPath()), @@ -80,9 +82,12 @@ On other errors, traceroute will exit with code 2. return serrors.WrapStr("setting up tracing", err) } defer closer() + var cmdout io.Writer switch flags.format { - case "human", "json", "yaml": - break + case "human": + cmdout = os.Stdout + case "json", "yaml": + cmdout = io.Discard default: return serrors.New("format not supported", "format", flags.format) } @@ -144,19 +149,28 @@ On other errors, traceroute will exit with code 2. if localIP, err = addrutil.ResolveLocal(target); err != nil { return serrors.WrapStr("resolving local address", err) } - fmt.Printf("Resolved local address:\n %s\n", localIP) + fmt.Fprintf(cmdout, "Resolved local address:\n %s\n", localIP) } - fmt.Printf("Using path:\n %s\n\n", path) + fmt.Fprintf(cmdout, "Using path:\n %s\n\n", path) + + var res traceroute.Result + res.Path = traceroute.Path{ + Expiry: path.Metadata().Expiry, + Fingerprint: snet.Fingerprint(path).String(), + Hops: snetpath.GetHops(path), + Latency: path.Metadata().Latency, + Mtu: int(path.Metadata().MTU), + NextHop: path.UnderlayNextHop().String(), + } + span.SetTag("src.host", localIP) local := &snet.UDPAddr{ IA: info.IA, Host: &net.UDPAddr{IP: localIP}, } ctx = app.WithSignal(traceCtx, os.Interrupt, syscall.SIGTERM) - res := &traceroute.Result{ - Updates: []traceroute.Update{}, - } var stats traceroute.Stats + var updates []traceroute.Update cfg := traceroute.Config{ Dispatcher: reliable.NewDispatcher(dispatcher), Remote: remote, @@ -167,13 +181,9 @@ On other errors, traceroute will exit with code 2. ProbesPerHop: 3, ErrHandler: func(err error) { fmt.Fprintf(os.Stderr, "ERROR: %s\n", err) }, UpdateHandler: func(u traceroute.Update) { - fmt.Printf("next hop: %s\n", u.Remote.NextHop) - fmt.Printf("IP: %s\n", u.Remote.Host.IP) - res.Updates = append(res.Updates, u) - if flags.format == "human" { - fmt.Printf("%d %s %s\n", u.Index, fmtRemote(u.Remote, u.Interface), - fmtRTTs(u.RTTs, flags.timeout)) - } + updates = append(updates, u) + fmt.Fprintf(cmdout, "%d %s %s\n", u.Index, fmtRemote(u.Remote, u.Interface), + fmtRTTs(u.RTTs, flags.timeout)) }, EPIC: flags.epic, } @@ -184,6 +194,16 @@ On other errors, traceroute will exit with code 2. if stats.Sent != stats.Recv { return app.WithExitCode(serrors.New("packets were lost"), 1) } + res.Hops = make([]traceroute.HopInfo, 0, len(updates)) + for _, update := range updates { + res.Hops = append(res.Hops, traceroute.HopInfo{ + InterfaceID: uint16(update.Interface), + IP: update.Remote.Host.IP.String(), + IA: update.Remote.IA, + RoundTripTimes: update.RTTs, + }) + } + switch flags.format { case "json": return res.JSON(os.Stdout) diff --git a/scion/ping/BUILD.bazel b/scion/ping/BUILD.bazel index 964b11c68e..2e210fed05 100644 --- a/scion/ping/BUILD.bazel +++ b/scion/ping/BUILD.bazel @@ -17,5 +17,6 @@ go_library( "//pkg/snet/path:go_default_library", "//pkg/sock/reliable:go_default_library", "//private/topology/underlay:go_default_library", + "@in_gopkg_yaml_v2//:go_default_library", ], ) diff --git a/scion/ping/ping.go b/scion/ping/ping.go index 7f2ceafe00..2bba6e7e6c 100644 --- a/scion/ping/ping.go +++ b/scion/ping/ping.go @@ -18,6 +18,8 @@ package ping import ( "context" "encoding/binary" + "encoding/json" + "io" "math/rand" "net" "time" @@ -27,14 +29,17 @@ import ( "github.com/scionproto/scion/pkg/private/common" "github.com/scionproto/scion/pkg/private/serrors" "github.com/scionproto/scion/pkg/snet" + "github.com/scionproto/scion/pkg/snet/path" "github.com/scionproto/scion/pkg/sock/reliable" "github.com/scionproto/scion/private/topology/underlay" + "gopkg.in/yaml.v2" ) // Stats contains the statistics of a ping run. type Stats struct { Sent int Received int + Loss int } // Update contains intermediary information about a received echo reply @@ -80,6 +85,58 @@ type Config struct { UpdateHandler func(Update) } +type Result struct { + Path Path `json:"path" yaml:"path"` + PayloadSize int `json:"payload_size" yaml:"payload_size"` + ScionPacketSize int `json:"scion_packet_size" yaml:"scion_packet_size"` + Replies []PingUpdate `json:"replies" yaml:"replies"` + Statistics PingStatistics `json:"statistics" yaml:"statistics"` +} + +type PingStatistics struct { + Sent int `json:"sent" yaml:"sent"` + Received int `json:"received" yaml:"received"` + Loss int `json:"packet_loss" yaml:"packet_loss"` + Time time.Duration `json:"time" yaml:"time"` +} + +// Path defines model for Path. +type Path struct { + // Expiration time of the path. + Expiry time.Time `json:"expiry" yaml:"expiry"` + + // Hex-string representing the paths fingerprint. + Fingerprint string `json:"fingerprint" yaml:"fingerprint"` + Hops []path.Hop `json:"hops" yaml:"hops"` + + // Optional array of latency measurements between any two consecutive interfaces. Entry i describes the latency between interface i and i+1. + Latency []time.Duration `json:"latency,omitempty" yaml:"latency,omitempty"` + + // The maximum transmission unit in bytes for SCION packets. This represents the protocol data unit (PDU) of the SCION layer on this path. + Mtu int `json:"mtu" yaml:"mtu"` + + // The internal UDP/IP underlay address of the SCION router that forwards traffic for this path. + NextHop string `json:"next_hop" yaml:"next_hop"` +} + +// PingRun defines model for PingRun. +type PingUpdate struct { + RoundTripTime string `json:"round_trip_time" yaml:"round_trip_time"` + + // Size of the entire SCION packet in bytes. This includes the SCION common header, and the SCION path header. + ScionPacketSize int `json:"scion_packet_size" yaml:"scion_packet_size"` + + // SCMP sequence number + ScmpSeq int `json:"scmp_seq" yaml:"scmp_seq"` + + // Source host address + SourceHost string `json:"source_host" yaml:"source_host"` + SourceIA string `json:"source_isd_as" yaml:"source_isd_as"` + + // Status describing the outcome of a ping response. + State string `json:"state" yaml:"state"` +} + // Run ping with the configuration. This blocks until the configured number // attempts is sent, or the context is canceled. func Run(ctx context.Context, cfg Config) (Stats, error) { @@ -324,3 +381,17 @@ func (h scmpHandler) handle(pkt *snet.Packet) (snet.SCMPEchoReply, error) { } return r, nil } + +// JSON writes the ping result as a json object to the writer. +func (r Result) JSON(w io.Writer) error { + enc := json.NewEncoder(w) + enc.SetIndent("", " ") + enc.SetEscapeHTML(false) + return enc.Encode(r) +} + +// JSON writes the ping result as a yaml object to the writer. +func (r Result) YAML(w io.Writer) error { + enc := yaml.NewEncoder(w) + return enc.Encode(r) +} diff --git a/scion/traceroute/traceroute.go b/scion/traceroute/traceroute.go index 0cd50d4ddb..b7a93fccde 100644 --- a/scion/traceroute/traceroute.go +++ b/scion/traceroute/traceroute.go @@ -29,6 +29,7 @@ import ( "github.com/scionproto/scion/pkg/private/serrors" "github.com/scionproto/scion/pkg/slayers/path/scion" "github.com/scionproto/scion/pkg/snet" + "github.com/scionproto/scion/pkg/snet/path" snetpath "github.com/scionproto/scion/pkg/snet/path" "github.com/scionproto/scion/pkg/sock/reliable" "gopkg.in/yaml.v2" @@ -58,9 +59,35 @@ type Stats struct { } type Result struct { - // LocalIA addr.IA `json:"local_isd_as" yaml:"local_isd_as"` - // Destination addr.IA `json:"destination" yaml:"destination"` - Updates []Update `json:"updates,omitempty" yaml:"updates,omitempty"` + Path Path `json:"path" yaml:"path"` + Hops []HopInfo `json:"hops" yaml:"hops"` +} + +type HopInfo struct { + InterfaceID uint16 `json:"interface_id" yaml:"interface_id"` + // IP address of the router responding to the traceroute request. + IP string `json:"ip" yaml:"ip"` + IA addr.IA `json:"isd_as" yaml:"isd_as"` + RoundTripTimes []time.Duration `json:"round_trip_times" yaml:"round_trip_times"` +} + +// Path defines model for Path. +type Path struct { + // Expiration time of the path. + Expiry time.Time `json:"expiry" yaml:"expiry"` + + // Hex-string representing the paths fingerprint. + Fingerprint string `json:"fingerprint" yaml:"fingerprint"` + Hops []path.Hop `json:"hops" yaml:"hops"` + + // Optional array of latency measurements between any two consecutive interfaces. Entry i describes the latency between interface i and i+1. + Latency []time.Duration `json:"latency,omitempty" yaml:"latency,omitempty"` + + // The maximum transmission unit in bytes for SCION packets. This represents the protocol data unit (PDU) of the SCION layer on this path. + Mtu int `json:"mtu" yaml:"mtu"` + + // The internal UDP/IP underlay address of the SCION router that forwards traffic for this path. + NextHop string `json:"next_hop" yaml:"next_hop"` } // Config configures the traceroute run. @@ -315,13 +342,37 @@ type scmpHandler struct { func (h scmpHandler) Handle(pkt *snet.Packet) error { r, err := h.handle(pkt) + ip := make(net.IP, len(pkt.Source.Host.IP())) + copy(ip, pkt.Source.Host.IP()) + + var path snet.DataplanePath + switch p := pkt.Path.(type) { + case snetpath.SCION: + raw := make([]byte, len(p.Raw)) + copy(raw, p.Raw) + path = snetpath.SCION{ + Raw: raw, + } + case *snetpath.EPIC: + path = snetpath.CloneEICDataplanePath(p) + case snet.RawPath: + raw := make([]byte, len(p.Raw)) + copy(raw, p.Raw) + path = snet.RawPath{ + PathType: p.PathType, + Raw: raw, + } + default: + return serrors.New("unknown type for packet Path", "type", common.TypeOf(pkt.Path)) + } + h.replies <- reply{ Received: time.Now(), Reply: r, Remote: &snet.UDPAddr{ IA: pkt.Source.IA, - Host: &net.UDPAddr{IP: pkt.Source.Host.IP()}, - Path: pkt.Path, + Host: &net.UDPAddr{IP: ip}, + Path: path, }, Error: err, } @@ -340,7 +391,7 @@ func (h scmpHandler) handle(pkt *snet.Packet) (snet.SCMPTracerouteReply, error) return r, nil } -// JSON writes the showpaths result as a json object to the writer. +// JSON writes the traceroute result as a json object to the writer. func (r Result) JSON(w io.Writer) error { enc := json.NewEncoder(w) enc.SetIndent("", " ") @@ -348,7 +399,7 @@ func (r Result) JSON(w io.Writer) error { return enc.Encode(r) } -// JSON writes the showpaths result as a yaml object to the writer. +// JSON writes the traceroute result as a yaml object to the writer. func (r Result) YAML(w io.Writer) error { enc := yaml.NewEncoder(w) return enc.Encode(r) From 901dfc4cad1131ae563275cb1fcbe6429c7785cf Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Thu, 27 Oct 2022 19:38:46 +0200 Subject: [PATCH 04/23] special exits in case of human format --- scion/cmd/scion/ping.go | 9 ++++++--- scion/cmd/scion/traceroute.go | 9 ++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/scion/cmd/scion/ping.go b/scion/cmd/scion/ping.go index f8e8f1dfa1..b3a8ac003f 100644 --- a/scion/cmd/scion/ping.go +++ b/scion/cmd/scion/ping.go @@ -74,6 +74,8 @@ and reports back the statistics. When the \--healthy-only option is set, ping first determines healthy paths through probing and chooses amongst them. +'ping' can be instructed to output the paths in a specific format using the \--format flag. + If no reply packet is received at all, ping will exit with code 1. On other errors, ping will exit with code 2. @@ -288,15 +290,16 @@ On other errors, ping will exit with code 2. } switch flags.format { + case "human": + if stats.Received == 0 { + return app.WithExitCode(serrors.New("no reply packet received"), 1) + } case "json": return res.JSON(os.Stdout) case "yaml": return res.YAML(os.Stdout) } - if stats.Received == 0 { - return app.WithExitCode(serrors.New("no reply packet received"), 1) - } return nil }, } diff --git a/scion/cmd/scion/traceroute.go b/scion/cmd/scion/traceroute.go index 187c275e6b..2900550979 100644 --- a/scion/cmd/scion/traceroute.go +++ b/scion/cmd/scion/traceroute.go @@ -64,6 +64,8 @@ func newTraceroute(pather CommandPather) *cobra.Command { Long: fmt.Sprintf(`'traceroute' traces the SCION path to a remote AS using SCMP traceroute packets. +'showpaths' can be instructed to output the paths in a specific format using the \--format flag. + If any packet is dropped, traceroute will exit with code 1. On other errors, traceroute will exit with code 2. %s`, app.SequenceHelp), @@ -191,9 +193,6 @@ On other errors, traceroute will exit with code 2. if err != nil { return err } - if stats.Sent != stats.Recv { - return app.WithExitCode(serrors.New("packets were lost"), 1) - } res.Hops = make([]traceroute.HopInfo, 0, len(updates)) for _, update := range updates { res.Hops = append(res.Hops, traceroute.HopInfo{ @@ -205,6 +204,10 @@ On other errors, traceroute will exit with code 2. } switch flags.format { + case "human": + if stats.Sent != stats.Recv { + return app.WithExitCode(serrors.New("packets were lost"), 1) + } case "json": return res.JSON(os.Stdout) case "yaml": From b5c3e051462382d3b855a449bf5322220817a343 Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Fri, 28 Oct 2022 10:20:52 +0200 Subject: [PATCH 05/23] linter errors --- scion/ping/ping.go | 9 ++++++--- scion/traceroute/traceroute.go | 6 ++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/scion/ping/ping.go b/scion/ping/ping.go index 2bba6e7e6c..de47ea75fa 100644 --- a/scion/ping/ping.go +++ b/scion/ping/ping.go @@ -109,10 +109,12 @@ type Path struct { Fingerprint string `json:"fingerprint" yaml:"fingerprint"` Hops []path.Hop `json:"hops" yaml:"hops"` - // Optional array of latency measurements between any two consecutive interfaces. Entry i describes the latency between interface i and i+1. + // Optional array of latency measurements between any two consecutive interfaces. Entry i + // describes the latency between interface i and i+1. Latency []time.Duration `json:"latency,omitempty" yaml:"latency,omitempty"` - // The maximum transmission unit in bytes for SCION packets. This represents the protocol data unit (PDU) of the SCION layer on this path. + // The maximum transmission unit in bytes for SCION packets. This represents the protocol data + // unit (PDU) of the SCION layer on this path. Mtu int `json:"mtu" yaml:"mtu"` // The internal UDP/IP underlay address of the SCION router that forwards traffic for this path. @@ -123,7 +125,8 @@ type Path struct { type PingUpdate struct { RoundTripTime string `json:"round_trip_time" yaml:"round_trip_time"` - // Size of the entire SCION packet in bytes. This includes the SCION common header, and the SCION path header. + // Size of the entire SCION packet in bytes. This includes the SCION common header, and the + // SCION path header. ScionPacketSize int `json:"scion_packet_size" yaml:"scion_packet_size"` // SCMP sequence number diff --git a/scion/traceroute/traceroute.go b/scion/traceroute/traceroute.go index b7a93fccde..b0043fbf76 100644 --- a/scion/traceroute/traceroute.go +++ b/scion/traceroute/traceroute.go @@ -80,10 +80,12 @@ type Path struct { Fingerprint string `json:"fingerprint" yaml:"fingerprint"` Hops []path.Hop `json:"hops" yaml:"hops"` - // Optional array of latency measurements between any two consecutive interfaces. Entry i describes the latency between interface i and i+1. + // Optional array of latency measurements between any two consecutive interfaces. Entry i + // describes the latency between interface i and i+1. Latency []time.Duration `json:"latency,omitempty" yaml:"latency,omitempty"` - // The maximum transmission unit in bytes for SCION packets. This represents the protocol data unit (PDU) of the SCION layer on this path. + // The maximum transmission unit in bytes for SCION packets. This represents the protocol data + // unit (PDU) of the SCION layer on this path. Mtu int `json:"mtu" yaml:"mtu"` // The internal UDP/IP underlay address of the SCION router that forwards traffic for this path. From 7aa2e924ef10cb1a52a9dd2d4e6db540b8486c20 Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Fri, 28 Oct 2022 15:58:38 +0200 Subject: [PATCH 06/23] feedback 1 --- pkg/snet/path.go | 4 +- pkg/snet/path/path.go | 12 +-- private/path/pathpol/sequence.go | 60 ++++++++------ scion/cmd/scion/BUILD.bazel | 2 + scion/cmd/scion/ping.go | 138 +++++++++++++++++++++++++------ scion/cmd/scion/traceroute.go | 7 ++ scion/ping/BUILD.bazel | 1 - scion/ping/ping.go | 99 ++++++---------------- scion/showpaths/BUILD.bazel | 1 + scion/showpaths/showpaths.go | 7 ++ scion/traceroute/traceroute.go | 7 +- 11 files changed, 198 insertions(+), 140 deletions(-) diff --git a/pkg/snet/path.go b/pkg/snet/path.go index 9af8a1ef44..32f191f906 100644 --- a/pkg/snet/path.go +++ b/pkg/snet/path.go @@ -70,9 +70,9 @@ type Path interface { // PathInterface is an interface of the path. type PathInterface struct { // ID is the ID of the interface. - ID common.IFIDType + ID common.IFIDType `json:"interface" yaml:"interface"` // IA is the ISD AS identifier of the interface. - IA addr.IA + IA addr.IA `json:"isd_as" yaml:"isd_as"` } func (iface PathInterface) String() string { diff --git a/pkg/snet/path/path.go b/pkg/snet/path/path.go index 1d41233b23..cb31fab7fa 100644 --- a/pkg/snet/path/path.go +++ b/pkg/snet/path/path.go @@ -38,11 +38,6 @@ type Path struct { Meta snet.PathMetadata } -type Hop struct { - Interface int `json:"interface" yaml:"interface"` - IA string `json:"isd_as" yaml:"isd_as"` -} - func (p Path) UnderlayNextHop() *net.UDPAddr { if p.NextHop == nil { return nil @@ -93,15 +88,16 @@ func fmtInterfaces(ifaces []snet.PathInterface) []string { return hops } -func GetHops(path snet.Path) []Hop { +// GetHops constructs a list of snet path interfaces from an snet path +func GetHops(path snet.Path) []snet.PathInterface { ifaces := path.Metadata().Interfaces - var hops []Hop + var hops []snet.PathInterface if len(ifaces) == 0 { return hops } for i := range ifaces { intf := ifaces[i] - hops = append(hops, Hop{IA: intf.IA.String(), Interface: int(intf.ID)}) + hops = append(hops, snet.PathInterface{IA: intf.IA, ID: intf.ID}) } return hops } diff --git a/private/path/pathpol/sequence.go b/private/path/pathpol/sequence.go index 04c1e73421..42ef2cc391 100644 --- a/private/path/pathpol/sequence.go +++ b/private/path/pathpol/sequence.go @@ -83,33 +83,12 @@ func (s *Sequence) Eval(paths []snet.Path) []snet.Path { } result := []snet.Path{} for _, path := range paths { - var desc string - ifaces := path.Metadata().Interfaces - switch { - // Path should contain even number of interfaces. 1 for source AS, - // 1 for destination AS and 2 per each intermediate AS. Invalid paths should - // not occur but if they do let's ignore them. - case len(ifaces)%2 != 0: - log.Error("Invalid path with even number of hops", "path", path) + desc, err := GetSequence(path) + desc = desc + " " + if err != nil { + log.Error("get sequence from path", "err", err) continue - // Empty paths are special cased. - case len(ifaces) == 0: - desc = "" - default: - // Turn the path into a string. For each AS on the path there will be - // one element in form #,, - // e.g. 64-ff00:0:112#3,5. For the source AS, the inbound interface will be - // zero. For destination AS, outbound interface will be zero. - - hops := make([]string, 0, len(ifaces)/2+1) - hops = append(hops, hop(ifaces[0].IA, 0, ifaces[0].ID)) - for i := 1; i < len(ifaces)-1; i += 2 { - hops = append(hops, hop(ifaces[i].IA, ifaces[i].ID, ifaces[i+1].ID)) - } - hops = append(hops, hop(ifaces[len(ifaces)-1].IA, ifaces[len(ifaces)-1].ID, 0)) - desc = strings.Join(hops, " ") + " " } - // Check whether the string matches the sequence regexp. if s.re.MatchString(desc) { result = append(result, path) @@ -315,3 +294,34 @@ func (l *sequenceListener) ExitIFace(c *sequence.IFaceContext) { func hop(ia addr.IA, ingress, egress common.IFIDType) string { return fmt.Sprintf("%s#%d,%d", ia, ingress, egress) } + +// GetSequence constructs the sequence string from snet path +// example format: "1-ff00:0:133#0 1-ff00:0:120#2,1 0 0 1-ff00:0:110#0" +func GetSequence(path snet.Path) (string, error) { + var desc string + ifaces := path.Metadata().Interfaces + switch { + // Path should contain even number of interfaces. 1 for source AS, + // 1 for destination AS and 2 per each intermediate AS. Invalid paths should + // not occur but if they do let's ignore them. + case len(ifaces)%2 != 0: + return "", serrors.New("Invalid path with odd number of hops", "path", path) + // Empty paths are special cased. + case len(ifaces) == 0: + desc = "" + default: + // Turn the path into a string. For each AS on the path there will be + // one element in form #,, + // e.g. 64-ff00:0:112#3,5. For the source AS, the inbound interface will be + // zero. For destination AS, outbound interface will be zero. + + hops := make([]string, 0, len(ifaces)/2+1) + hops = append(hops, hop(ifaces[0].IA, 0, ifaces[0].ID)) + for i := 1; i < len(ifaces)-1; i += 2 { + hops = append(hops, hop(ifaces[i].IA, ifaces[i].ID, ifaces[i+1].ID)) + } + hops = append(hops, hop(ifaces[len(ifaces)-1].IA, ifaces[len(ifaces)-1].ID, 0)) + desc = strings.Join(hops, " ") + } + return desc, nil +} diff --git a/scion/cmd/scion/BUILD.bazel b/scion/cmd/scion/BUILD.bazel index b60f18bee3..ac40efa87e 100644 --- a/scion/cmd/scion/BUILD.bazel +++ b/scion/cmd/scion/BUILD.bazel @@ -28,6 +28,7 @@ go_library( "//private/app/flag:go_default_library", "//private/app/path:go_default_library", "//private/env:go_default_library", + "//private/path/pathpol:go_default_library", "//private/topology:go_default_library", "//private/tracing:go_default_library", "//scion/ping:go_default_library", @@ -36,6 +37,7 @@ go_library( "@com_github_opentracing_opentracing_go//:go_default_library", "@com_github_spf13_cobra//:go_default_library", "@com_github_spf13_cobra//doc:go_default_library", + "@in_gopkg_yaml_v2//:go_default_library", ], ) diff --git a/scion/cmd/scion/ping.go b/scion/cmd/scion/ping.go index b3a8ac003f..252c19508a 100644 --- a/scion/cmd/scion/ping.go +++ b/scion/cmd/scion/ping.go @@ -16,6 +16,7 @@ package main import ( "context" + "encoding/json" "fmt" "io" "math" @@ -25,6 +26,7 @@ import ( "time" "github.com/spf13/cobra" + "gopkg.in/yaml.v2" "github.com/scionproto/scion/pkg/daemon" "github.com/scionproto/scion/pkg/log" @@ -36,10 +38,50 @@ import ( "github.com/scionproto/scion/private/app" "github.com/scionproto/scion/private/app/flag" "github.com/scionproto/scion/private/app/path" + "github.com/scionproto/scion/private/path/pathpol" "github.com/scionproto/scion/private/tracing" "github.com/scionproto/scion/scion/ping" ) +type Result struct { + Path Path `json:"path" yaml:"path"` + PayloadSize int `json:"payload_size" yaml:"payload_size"` + ScionPacketSize int `json:"scion_packet_size" yaml:"scion_packet_size"` + Replies []PingUpdate `json:"replies" yaml:"replies"` + Statistics ping.Stats `json:"statistics" yaml:"statistics"` +} + +// Path defines model for Path. +type Path struct { + // Expiration time of the path. + Expiry time.Time `json:"expiry" yaml:"expiry"` + + // Hex-string representing the paths fingerprint. + Fingerprint string `json:"fingerprint" yaml:"fingerprint"` + Hops []snet.PathInterface `json:"hops" yaml:"hops"` + Sequence string `json:"hops_sequence" yaml:"hops_sequence"` + + // Optional array of latency measurements between any two consecutive interfaces. Entry i + // describes the latency between interface i and i+1. + Latency []time.Duration `json:"latency,omitempty" yaml:"latency,omitempty"` + LocalIp net.IP `json:"local_ip,omitempty" yaml:"local_ip,omitempty"` + + // The maximum transmission unit in bytes for SCION packets. This represents the protocol data + // unit (PDU) of the SCION layer on this path. + Mtu int `json:"mtu" yaml:"mtu"` + + // The internal UDP/IP underlay address of the SCION router that forwards traffic for this path. + NextHop string `json:"next_hop" yaml:"next_hop"` +} + +type PingUpdate struct { + Size int `json:"scion_packet_size" yaml:"scion_packet_size"` + Source string `json:"source_isd_as" yaml:"source_isd_as"` + Sequence int `json:"scmp_seq" yaml:"scmp_seq"` + RTT time.Duration `json:"round_trip_time" yaml:"round_trip_time"` + State string `json:"state" yaml:"state"` +} + func newPing(pather CommandPather) *cobra.Command { var envFlags flag.SCIONEnvironment var flags struct { @@ -227,13 +269,19 @@ On other errors, ping will exit with code 2. count = math.MaxUint16 } - res := ping.Result{ + seq, err := pathpol.GetSequence(path) + if err != nil { + return serrors.New("get sequence from used path") + } + res := Result{ ScionPacketSize: pktSize, - Path: ping.Path{ + Path: Path{ Expiry: path.Metadata().Expiry, Fingerprint: snet.Fingerprint(path).String(), Hops: snetpath.GetHops(path), + Sequence: seq, Latency: path.Metadata().Latency, + LocalIp: localIP, Mtu: int(path.Metadata().MTU), NextHop: path.UnderlayNextHop().String(), }, @@ -252,42 +300,33 @@ On other errors, ping will exit with code 2. fmt.Fprintf(os.Stderr, "ERROR: %s\n", err) }, UpdateHandler: func(update ping.Update) { - state := "success" var additional string switch update.State { case ping.AfterTimeout: additional = " state=After timeout" - state = "after_timeout" case ping.OutOfOrder: additional = " state=Out of Order" - state = "out_of_order" case ping.Duplicate: additional = " state=Duplicate" - state = "duplicate" } - res.Replies = append(res.Replies, ping.PingUpdate{ - RoundTripTime: update.RTT.String(), - ScionPacketSize: update.Size, - ScmpSeq: update.Sequence, - SourceHost: update.Source.Host.String(), - SourceIA: update.Source.IA.String(), - State: state, + res.Replies = append(res.Replies, PingUpdate{ + Size: update.Size, + Source: update.Source.String(), + Sequence: update.Sequence, + RTT: update.RTT, + State: update.State.String(), }) fmt.Fprintf(cmdout, "%d bytes from %s,%s: scmp_seq=%d time=%s%s\n", update.Size, update.Source.IA, update.Source.Host, update.Sequence, update.RTT, additional) }, }) - pktLoss := pingSummary(stats, remote, time.Since(start), cmdout) + processRTTs(&stats, res.Replies) + pingSummary(&stats, remote, time.Since(start), cmdout) if err != nil { return err } - res.Statistics = ping.PingStatistics{ - Received: stats.Received, - Sent: stats.Sent, - Loss: pktLoss, - Time: time.Since(start), - } + res.Statistics = stats switch flags.format { case "human": @@ -343,13 +382,62 @@ func calcMaxPldSize(local, remote *snet.UDPAddr, mtu int) (int, error) { return mtu - overhead, nil } -func pingSummary(stats ping.Stats, remote *snet.UDPAddr, run time.Duration, cmdout io.Writer) int { - var pktLoss int +func pingSummary(stats *ping.Stats, remote *snet.UDPAddr, run time.Duration, cmdout io.Writer) { if stats.Sent != 0 { - pktLoss = 100 - stats.Received*100/stats.Sent + stats.Loss = 100 - stats.Received*100/stats.Sent } + stats.Time = run fmt.Fprintf(cmdout, "\n--- %s,%s statistics ---\n", remote.IA, remote.Host.IP) fmt.Fprintf(cmdout, "%d packets transmitted, %d received, %d%% packet loss, time %v\n", - stats.Sent, stats.Received, stats.Loss, run.Round(time.Microsecond)) - return pktLoss + stats.Sent, stats.Received, stats.Loss, stats.Time.Round(time.Microsecond)) + if stats.Received != 0 { + fmt.Fprintf(cmdout, "rtt min/avg/max/mdev = %v/%v/%v/%v\n", + stats.MinRTT.Round(time.Microsecond), + stats.AvgRTT.Round(time.Microsecond), + stats.MaxRTT.Round(time.Microsecond), + stats.MdevRTT.Round(time.Microsecond), + ) + } +} + +// processRTTs computes the min, average, max and standard deviation of the update RTTs +func processRTTs(stats *ping.Stats, replies []PingUpdate) { + if len(replies) == 0 { + return + } + stats.MinRTT = replies[0].RTT + stats.MaxRTT = replies[0].RTT + var sum time.Duration + for i := 0; i < len(replies); i++ { + if replies[i].RTT < stats.MinRTT { + stats.MinRTT = replies[i].RTT + } + if replies[i].RTT > stats.MaxRTT { + stats.MaxRTT = replies[i].RTT + } + sum += replies[i].RTT + } + stats.AvgRTT = time.Duration(int(sum.Nanoseconds()) / len(replies)) + + // standard deviation + var sd float64 + for i := 0; i < len(replies); i++ { + sd += math.Pow(float64(replies[i].RTT)-float64(stats.AvgRTT), 2) + } + stats.MdevRTT = time.Duration(math.Sqrt(sd / float64(len(replies)))) + +} + +// JSON writes the ping result as a json object to the writer. +func (r Result) JSON(w io.Writer) error { + enc := json.NewEncoder(w) + enc.SetIndent("", " ") + enc.SetEscapeHTML(false) + return enc.Encode(r) +} + +// JSON writes the ping result as a yaml object to the writer. +func (r Result) YAML(w io.Writer) error { + enc := yaml.NewEncoder(w) + return enc.Encode(r) } diff --git a/scion/cmd/scion/traceroute.go b/scion/cmd/scion/traceroute.go index 2900550979..468a75c0dc 100644 --- a/scion/cmd/scion/traceroute.go +++ b/scion/cmd/scion/traceroute.go @@ -36,6 +36,7 @@ import ( "github.com/scionproto/scion/private/app" "github.com/scionproto/scion/private/app/flag" "github.com/scionproto/scion/private/app/path" + "github.com/scionproto/scion/private/path/pathpol" "github.com/scionproto/scion/private/topology" "github.com/scionproto/scion/private/tracing" "github.com/scionproto/scion/scion/traceroute" @@ -155,12 +156,18 @@ On other errors, traceroute will exit with code 2. } fmt.Fprintf(cmdout, "Using path:\n %s\n\n", path) + seq, err := pathpol.GetSequence(path) + if err != nil { + return serrors.New("get sequence from used path") + } var res traceroute.Result res.Path = traceroute.Path{ Expiry: path.Metadata().Expiry, Fingerprint: snet.Fingerprint(path).String(), Hops: snetpath.GetHops(path), + Sequence: seq, Latency: path.Metadata().Latency, + LocalIp: localIP, Mtu: int(path.Metadata().MTU), NextHop: path.UnderlayNextHop().String(), } diff --git a/scion/ping/BUILD.bazel b/scion/ping/BUILD.bazel index 2e210fed05..964b11c68e 100644 --- a/scion/ping/BUILD.bazel +++ b/scion/ping/BUILD.bazel @@ -17,6 +17,5 @@ go_library( "//pkg/snet/path:go_default_library", "//pkg/sock/reliable:go_default_library", "//private/topology/underlay:go_default_library", - "@in_gopkg_yaml_v2//:go_default_library", ], ) diff --git a/scion/ping/ping.go b/scion/ping/ping.go index de47ea75fa..adf95fddb3 100644 --- a/scion/ping/ping.go +++ b/scion/ping/ping.go @@ -18,8 +18,6 @@ package ping import ( "context" "encoding/binary" - "encoding/json" - "io" "math/rand" "net" "time" @@ -29,17 +27,20 @@ import ( "github.com/scionproto/scion/pkg/private/common" "github.com/scionproto/scion/pkg/private/serrors" "github.com/scionproto/scion/pkg/snet" - "github.com/scionproto/scion/pkg/snet/path" "github.com/scionproto/scion/pkg/sock/reliable" "github.com/scionproto/scion/private/topology/underlay" - "gopkg.in/yaml.v2" ) // Stats contains the statistics of a ping run. type Stats struct { - Sent int - Received int - Loss int + Sent int `json:"sent" yaml:"sent"` + Received int `json:"received" yaml:"received"` + Loss int `json:"packet_loss" yaml:"packet_loss"` + Time time.Duration `json:"time" yaml:"time"` + MinRTT time.Duration `json:"min_RTT" yaml:"min_RTT"` + AvgRTT time.Duration `json:"avg_RTT" yaml:"avg_RTT"` + MaxRTT time.Duration `json:"max_RTT" yaml:"max_RTT"` + MdevRTT time.Duration `json:"mdev_RTT" yaml:"mdev_RTT"` } // Update contains intermediary information about a received echo reply @@ -62,6 +63,21 @@ const ( Duplicate ) +func (s State) String() string { + switch s { + case Success: + return "success" + case AfterTimeout: + return "after_timeout" + case OutOfOrder: + return "out_of_order" + case Duplicate: + return "duplicate" + default: + return "unknown" + } +} + // Config configures the ping run. type Config struct { Dispatcher reliable.Dispatcher @@ -85,61 +101,6 @@ type Config struct { UpdateHandler func(Update) } -type Result struct { - Path Path `json:"path" yaml:"path"` - PayloadSize int `json:"payload_size" yaml:"payload_size"` - ScionPacketSize int `json:"scion_packet_size" yaml:"scion_packet_size"` - Replies []PingUpdate `json:"replies" yaml:"replies"` - Statistics PingStatistics `json:"statistics" yaml:"statistics"` -} - -type PingStatistics struct { - Sent int `json:"sent" yaml:"sent"` - Received int `json:"received" yaml:"received"` - Loss int `json:"packet_loss" yaml:"packet_loss"` - Time time.Duration `json:"time" yaml:"time"` -} - -// Path defines model for Path. -type Path struct { - // Expiration time of the path. - Expiry time.Time `json:"expiry" yaml:"expiry"` - - // Hex-string representing the paths fingerprint. - Fingerprint string `json:"fingerprint" yaml:"fingerprint"` - Hops []path.Hop `json:"hops" yaml:"hops"` - - // Optional array of latency measurements between any two consecutive interfaces. Entry i - // describes the latency between interface i and i+1. - Latency []time.Duration `json:"latency,omitempty" yaml:"latency,omitempty"` - - // The maximum transmission unit in bytes for SCION packets. This represents the protocol data - // unit (PDU) of the SCION layer on this path. - Mtu int `json:"mtu" yaml:"mtu"` - - // The internal UDP/IP underlay address of the SCION router that forwards traffic for this path. - NextHop string `json:"next_hop" yaml:"next_hop"` -} - -// PingRun defines model for PingRun. -type PingUpdate struct { - RoundTripTime string `json:"round_trip_time" yaml:"round_trip_time"` - - // Size of the entire SCION packet in bytes. This includes the SCION common header, and the - // SCION path header. - ScionPacketSize int `json:"scion_packet_size" yaml:"scion_packet_size"` - - // SCMP sequence number - ScmpSeq int `json:"scmp_seq" yaml:"scmp_seq"` - - // Source host address - SourceHost string `json:"source_host" yaml:"source_host"` - SourceIA string `json:"source_isd_as" yaml:"source_isd_as"` - - // Status describing the outcome of a ping response. - State string `json:"state" yaml:"state"` -} - // Run ping with the configuration. This blocks until the configured number // attempts is sent, or the context is canceled. func Run(ctx context.Context, cfg Config) (Stats, error) { @@ -384,17 +345,3 @@ func (h scmpHandler) handle(pkt *snet.Packet) (snet.SCMPEchoReply, error) { } return r, nil } - -// JSON writes the ping result as a json object to the writer. -func (r Result) JSON(w io.Writer) error { - enc := json.NewEncoder(w) - enc.SetIndent("", " ") - enc.SetEscapeHTML(false) - return enc.Encode(r) -} - -// JSON writes the ping result as a yaml object to the writer. -func (r Result) YAML(w io.Writer) error { - enc := yaml.NewEncoder(w) - return enc.Encode(r) -} diff --git a/scion/showpaths/BUILD.bazel b/scion/showpaths/BUILD.bazel index c3a838b16c..7bd10a36ce 100644 --- a/scion/showpaths/BUILD.bazel +++ b/scion/showpaths/BUILD.bazel @@ -16,6 +16,7 @@ go_library( "//pkg/snet:go_default_library", "//private/app/path:go_default_library", "//private/app/path/pathprobe:go_default_library", + "//private/path/pathpol:go_default_library", "@in_gopkg_yaml_v2//:go_default_library", ], ) diff --git a/scion/showpaths/showpaths.go b/scion/showpaths/showpaths.go index e8983e9438..561eb517ea 100644 --- a/scion/showpaths/showpaths.go +++ b/scion/showpaths/showpaths.go @@ -33,6 +33,7 @@ import ( "github.com/scionproto/scion/pkg/snet" "github.com/scionproto/scion/private/app/path" "github.com/scionproto/scion/private/app/path/pathprobe" + "github.com/scionproto/scion/private/path/pathpol" "gopkg.in/yaml.v2" ) @@ -48,6 +49,7 @@ type Path struct { FullPath snet.Path `json:"-" yaml:"-"` Fingerprint string `json:"fingerprint" yaml:"fingerprint"` Hops []Hop `json:"hops" yaml:"hops"` + Sequence string `json:"hops_sequence" yaml:"hops_sequence"` NextHop string `json:"next_hop" yaml:"next_hop"` Expiry time.Time `json:"expiry" yaml:"expiry"` MTU uint16 `json:"mtu" yaml:"mtu"` @@ -411,6 +413,11 @@ func Run(ctx context.Context, dst addr.IA, cfg Config) (*Result, error) { rpath.StatusInfo = status.AdditionalInfo rpath.Local = status.LocalIP } + seq, err := pathpol.GetSequence(path) + if err != nil { + continue + } + rpath.Sequence = seq res.Paths = append(res.Paths, rpath) } return res, nil diff --git a/scion/traceroute/traceroute.go b/scion/traceroute/traceroute.go index b0043fbf76..775ca3cefe 100644 --- a/scion/traceroute/traceroute.go +++ b/scion/traceroute/traceroute.go @@ -29,7 +29,6 @@ import ( "github.com/scionproto/scion/pkg/private/serrors" "github.com/scionproto/scion/pkg/slayers/path/scion" "github.com/scionproto/scion/pkg/snet" - "github.com/scionproto/scion/pkg/snet/path" snetpath "github.com/scionproto/scion/pkg/snet/path" "github.com/scionproto/scion/pkg/sock/reliable" "gopkg.in/yaml.v2" @@ -77,12 +76,14 @@ type Path struct { Expiry time.Time `json:"expiry" yaml:"expiry"` // Hex-string representing the paths fingerprint. - Fingerprint string `json:"fingerprint" yaml:"fingerprint"` - Hops []path.Hop `json:"hops" yaml:"hops"` + Fingerprint string `json:"fingerprint" yaml:"fingerprint"` + Hops []snet.PathInterface `json:"hops" yaml:"hops"` + Sequence string `json:"hops_sequence" yaml:"hops_sequence"` // Optional array of latency measurements between any two consecutive interfaces. Entry i // describes the latency between interface i and i+1. Latency []time.Duration `json:"latency,omitempty" yaml:"latency,omitempty"` + LocalIp net.IP `json:"local_ip,omitempty" yaml:"local_ip,omitempty"` // The maximum transmission unit in bytes for SCION packets. This represents the protocol data // unit (PDU) of the SCION layer on this path. From 828789fbcb01c2b80a89b935cd7ff1550f5537d2 Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Mon, 31 Oct 2022 10:44:02 +0100 Subject: [PATCH 07/23] feedback 2 --- pkg/snet/path.go | 4 +- pkg/snet/path/epic.go | 13 ----- pkg/snet/path/path.go | 14 ----- private/path/pathpol/sequence.go | 3 +- scion/cmd/scion/BUILD.bazel | 2 + scion/cmd/scion/common.go | 62 ++++++++++++++++++++ scion/cmd/scion/ping.go | 82 +++++++------------------- scion/cmd/scion/showpaths.go | 28 ++++----- scion/cmd/scion/traceroute.go | 99 +++++++++++++++++++++----------- scion/showpaths/BUILD.bazel | 1 - scion/showpaths/showpaths.go | 19 +----- scion/traceroute/BUILD.bazel | 1 - scion/traceroute/traceroute.go | 99 ++++---------------------------- 13 files changed, 182 insertions(+), 245 deletions(-) create mode 100644 scion/cmd/scion/common.go diff --git a/pkg/snet/path.go b/pkg/snet/path.go index 32f191f906..9af8a1ef44 100644 --- a/pkg/snet/path.go +++ b/pkg/snet/path.go @@ -70,9 +70,9 @@ type Path interface { // PathInterface is an interface of the path. type PathInterface struct { // ID is the ID of the interface. - ID common.IFIDType `json:"interface" yaml:"interface"` + ID common.IFIDType // IA is the ISD AS identifier of the interface. - IA addr.IA `json:"isd_as" yaml:"isd_as"` + IA addr.IA } func (iface PathInterface) String() string { diff --git a/pkg/snet/path/epic.go b/pkg/snet/path/epic.go index 1865c03b2a..a3e44bb585 100644 --- a/pkg/snet/path/epic.go +++ b/pkg/snet/path/epic.go @@ -47,19 +47,6 @@ func NewEPICDataplanePath(p SCION, auths snet.EpicAuths) (*EPIC, error) { return epicPath, nil } -func CloneEICDataplanePath(p *EPIC) *EPIC { - res := &EPIC{} - res.AuthPHVF = make([]byte, len(p.AuthPHVF)) - copy(res.AuthPHVF, p.AuthPHVF) - res.AuthLHVF = make([]byte, len(p.AuthLHVF)) - copy(res.AuthLHVF, p.AuthLHVF) - res.SCION = make([]byte, len(p.SCION)) - copy(res.SCION, p.SCION) - res.counter = p.counter - res.mu = sync.Mutex{} - return res -} - func (e *EPIC) SetPath(s *slayers.SCION) error { e.mu.Lock() defer e.mu.Unlock() diff --git a/pkg/snet/path/path.go b/pkg/snet/path/path.go index cb31fab7fa..08748964d9 100644 --- a/pkg/snet/path/path.go +++ b/pkg/snet/path/path.go @@ -87,17 +87,3 @@ func fmtInterfaces(ifaces []snet.PathInterface) []string { hops = append(hops, fmt.Sprintf("%d %s", intf.ID, intf.IA)) return hops } - -// GetHops constructs a list of snet path interfaces from an snet path -func GetHops(path snet.Path) []snet.PathInterface { - ifaces := path.Metadata().Interfaces - var hops []snet.PathInterface - if len(ifaces) == 0 { - return hops - } - for i := range ifaces { - intf := ifaces[i] - hops = append(hops, snet.PathInterface{IA: intf.IA, ID: intf.ID}) - } - return hops -} diff --git a/private/path/pathpol/sequence.go b/private/path/pathpol/sequence.go index 42ef2cc391..33378e2be4 100644 --- a/private/path/pathpol/sequence.go +++ b/private/path/pathpol/sequence.go @@ -296,7 +296,8 @@ func hop(ia addr.IA, ingress, egress common.IFIDType) string { } // GetSequence constructs the sequence string from snet path -// example format: "1-ff00:0:133#0 1-ff00:0:120#2,1 0 0 1-ff00:0:110#0" +// example format: +// 1-ff00:0:133#0 1-ff00:0:120#2,1 1-ff00:0:110#0 func GetSequence(path snet.Path) (string, error) { var desc string ifaces := path.Metadata().Interfaces diff --git a/scion/cmd/scion/BUILD.bazel b/scion/cmd/scion/BUILD.bazel index ac40efa87e..4e60f134cb 100644 --- a/scion/cmd/scion/BUILD.bazel +++ b/scion/cmd/scion/BUILD.bazel @@ -5,6 +5,7 @@ go_library( name = "go_default_library", srcs = [ "address.go", + "common.go", "gendocs.go", "main.go", "observability.go", @@ -18,6 +19,7 @@ go_library( "//pkg/addr:go_default_library", "//pkg/daemon:go_default_library", "//pkg/log:go_default_library", + "//pkg/private/common:go_default_library", "//pkg/private/serrors:go_default_library", "//pkg/snet:go_default_library", "//pkg/snet/addrutil:go_default_library", diff --git a/scion/cmd/scion/common.go b/scion/cmd/scion/common.go new file mode 100644 index 0000000000..80a363cf77 --- /dev/null +++ b/scion/cmd/scion/common.go @@ -0,0 +1,62 @@ +package main + +import ( + "fmt" + "io" + "net" + + "github.com/scionproto/scion/pkg/addr" + "github.com/scionproto/scion/pkg/private/common" + "github.com/scionproto/scion/pkg/private/serrors" + "github.com/scionproto/scion/pkg/snet" +) + +// Path defines model for Path. +type Path struct { + // Hex-string representing the paths fingerprint. + Fingerprint string `json:"fingerprint" yaml:"fingerprint"` + Hops []Hop `json:"hops" yaml:"hops"` + Sequence string `json:"hops_sequence" yaml:"hops_sequence"` + + LocalIP net.IP `json:"local_ip,omitempty" yaml:"local_ip,omitempty"` + + // The internal UDP/IP underlay address of the SCION router that forwards traffic for this path. + NextHop string `json:"next_hop" yaml:"next_hop"` +} + +// Hop represents an hop on the path. +type Hop struct { + ID common.IFIDType `json:"interface" yaml:"interface"` + IA addr.IA `json:"isd_as" yaml:"isd_as"` +} + +// GetHops constructs a list of snet path interfaces from an snet path +func getHops(path snet.Path) []Hop { + ifaces := path.Metadata().Interfaces + var hops []Hop + if len(ifaces) == 0 { + return hops + } + for i := range ifaces { + intf := ifaces[i] + hops = append(hops, Hop{IA: intf.IA, ID: intf.ID}) + } + return hops +} + +// getPrintf returns a printf function for the "human" formatting flag and an empty one for machine +// readable format flags +func getPrintf(outputFlag string, writer io.Writer) (func(format string, ctx ...interface{}), error) { + printf := func(format string, ctx ...interface{}) {} + switch outputFlag { + case "human": + printf = func(format string, ctx ...interface{}) { + fmt.Fprintf(writer, format, ctx...) + } + return printf, nil + case "yaml", "json": + return printf, nil + default: + return printf, serrors.New("format not supported", "format", outputFlag) + } +} diff --git a/scion/cmd/scion/ping.go b/scion/cmd/scion/ping.go index 252c19508a..70af921879 100644 --- a/scion/cmd/scion/ping.go +++ b/scion/cmd/scion/ping.go @@ -18,7 +18,6 @@ import ( "context" "encoding/json" "fmt" - "io" "math" "net" "os" @@ -51,29 +50,6 @@ type Result struct { Statistics ping.Stats `json:"statistics" yaml:"statistics"` } -// Path defines model for Path. -type Path struct { - // Expiration time of the path. - Expiry time.Time `json:"expiry" yaml:"expiry"` - - // Hex-string representing the paths fingerprint. - Fingerprint string `json:"fingerprint" yaml:"fingerprint"` - Hops []snet.PathInterface `json:"hops" yaml:"hops"` - Sequence string `json:"hops_sequence" yaml:"hops_sequence"` - - // Optional array of latency measurements between any two consecutive interfaces. Entry i - // describes the latency between interface i and i+1. - Latency []time.Duration `json:"latency,omitempty" yaml:"latency,omitempty"` - LocalIp net.IP `json:"local_ip,omitempty" yaml:"local_ip,omitempty"` - - // The maximum transmission unit in bytes for SCION packets. This represents the protocol data - // unit (PDU) of the SCION layer on this path. - Mtu int `json:"mtu" yaml:"mtu"` - - // The internal UDP/IP underlay address of the SCION router that forwards traffic for this path. - NextHop string `json:"next_hop" yaml:"next_hop"` -} - type PingUpdate struct { Size int `json:"scion_packet_size" yaml:"scion_packet_size"` Source string `json:"source_isd_as" yaml:"source_isd_as"` @@ -136,14 +112,9 @@ On other errors, ping will exit with code 2. return serrors.WrapStr("setting up tracing", err) } defer closer() - var cmdout io.Writer - switch flags.format { - case "human": - cmdout = os.Stdout - case "json", "yaml": - cmdout = io.Discard - default: - return serrors.New("format not supported", "format", flags.format) + printf, err := getPrintf(flags.format, cmd.OutOrStdout()) + if err != nil { + return serrors.WrapStr("get formatting", err) } cmd.SilenceUsage = true @@ -227,9 +198,9 @@ On other errors, ping will exit with code 2. return serrors.WrapStr("resolving local address", err) } - fmt.Fprintf(cmdout, "Resolved local address:\n %s\n", localIP) + printf("Resolved local address:\n %s\n", localIP) } - fmt.Fprintf(cmdout, "Using path:\n %s\n\n", path) + printf("Using path:\n %s\n\n", path) span.SetTag("src.host", localIP) local := &snet.UDPAddr{ IA: info.IA, @@ -260,7 +231,7 @@ On other errors, ping will exit with code 2. if err != nil { return err } - fmt.Fprintf(cmdout, "PING %s pld=%dB scion_pkt=%dB\n", remote, pldSize, pktSize) + printf("PING %s pld=%dB scion_pkt=%dB\n", remote, pldSize, pktSize) start := time.Now() ctx = app.WithSignal(traceCtx, os.Interrupt, syscall.SIGTERM) @@ -276,13 +247,10 @@ On other errors, ping will exit with code 2. res := Result{ ScionPacketSize: pktSize, Path: Path{ - Expiry: path.Metadata().Expiry, Fingerprint: snet.Fingerprint(path).String(), - Hops: snetpath.GetHops(path), + Hops: getHops(path), Sequence: seq, - Latency: path.Metadata().Latency, - LocalIp: localIP, - Mtu: int(path.Metadata().MTU), + LocalIP: localIP, NextHop: path.UnderlayNextHop().String(), }, PayloadSize: pldSize, @@ -316,13 +284,13 @@ On other errors, ping will exit with code 2. RTT: update.RTT, State: update.State.String(), }) - fmt.Fprintf(cmdout, "%d bytes from %s,%s: scmp_seq=%d time=%s%s\n", + printf("%d bytes from %s,%s: scmp_seq=%d time=%s%s\n", update.Size, update.Source.IA, update.Source.Host, update.Sequence, update.RTT, additional) }, }) processRTTs(&stats, res.Replies) - pingSummary(&stats, remote, time.Since(start), cmdout) + pingSummary(&stats, remote, time.Since(start), printf) if err != nil { return err } @@ -334,9 +302,13 @@ On other errors, ping will exit with code 2. return app.WithExitCode(serrors.New("no reply packet received"), 1) } case "json": - return res.JSON(os.Stdout) + enc := json.NewEncoder(os.Stdout) + enc.SetIndent("", " ") + enc.SetEscapeHTML(false) + return enc.Encode(res) case "yaml": - return res.YAML(os.Stdout) + enc := yaml.NewEncoder(os.Stdout) + return enc.Encode(res) } return nil @@ -382,16 +354,16 @@ func calcMaxPldSize(local, remote *snet.UDPAddr, mtu int) (int, error) { return mtu - overhead, nil } -func pingSummary(stats *ping.Stats, remote *snet.UDPAddr, run time.Duration, cmdout io.Writer) { +func pingSummary(stats *ping.Stats, remote *snet.UDPAddr, run time.Duration, printf func(format string, ctx ...interface{})) { if stats.Sent != 0 { stats.Loss = 100 - stats.Received*100/stats.Sent } stats.Time = run - fmt.Fprintf(cmdout, "\n--- %s,%s statistics ---\n", remote.IA, remote.Host.IP) - fmt.Fprintf(cmdout, "%d packets transmitted, %d received, %d%% packet loss, time %v\n", + printf("\n--- %s,%s statistics ---\n", remote.IA, remote.Host.IP) + printf("%d packets transmitted, %d received, %d%% packet loss, time %v\n", stats.Sent, stats.Received, stats.Loss, stats.Time.Round(time.Microsecond)) if stats.Received != 0 { - fmt.Fprintf(cmdout, "rtt min/avg/max/mdev = %v/%v/%v/%v\n", + printf("rtt min/avg/max/mdev = %v/%v/%v/%v\n", stats.MinRTT.Round(time.Microsecond), stats.AvgRTT.Round(time.Microsecond), stats.MaxRTT.Round(time.Microsecond), @@ -427,17 +399,3 @@ func processRTTs(stats *ping.Stats, replies []PingUpdate) { stats.MdevRTT = time.Duration(math.Sqrt(sd / float64(len(replies)))) } - -// JSON writes the ping result as a json object to the writer. -func (r Result) JSON(w io.Writer) error { - enc := json.NewEncoder(w) - enc.SetIndent("", " ") - enc.SetEscapeHTML(false) - return enc.Encode(r) -} - -// JSON writes the ping result as a yaml object to the writer. -func (r Result) YAML(w io.Writer) error { - enc := yaml.NewEncoder(w) - return enc.Encode(r) -} diff --git a/scion/cmd/scion/showpaths.go b/scion/cmd/scion/showpaths.go index 1ace83620b..d95cda0362 100644 --- a/scion/cmd/scion/showpaths.go +++ b/scion/cmd/scion/showpaths.go @@ -16,12 +16,13 @@ package main import ( "context" + "encoding/json" "fmt" - "io" "os" "time" "github.com/spf13/cobra" + "gopkg.in/yaml.v2" "github.com/scionproto/scion/pkg/addr" "github.com/scionproto/scion/pkg/log" @@ -83,14 +84,9 @@ On other errors, showpaths will exit with code 2. return serrors.WrapStr("setting up tracing", err) } defer closer() - var cmdout io.Writer - switch flags.format { - case "human": - cmdout = os.Stdout - case "json", "yaml": - cmdout = io.Discard - default: - return serrors.New("format not supported", "format", flags.format) + printf, err := getPrintf(flags.format, cmd.OutOrStdout()) + if err != nil { + return serrors.WrapStr("get formatting", err) } cmd.SilenceUsage = true @@ -122,21 +118,25 @@ On other errors, showpaths will exit with code 2. switch flags.format { case "human": if res.IsLocal() { - fmt.Fprintf(cmdout, "Empty path, destination is local AS %s\n", res.Destination) + printf("Empty path, destination is local AS %s\n", res.Destination) return nil } - fmt.Fprintln(cmdout, "Available paths to", res.Destination) + printf("Available paths to", res.Destination) if len(res.Paths) == 0 { return app.WithExitCode(serrors.New("no path found"), 1) } - res.Human(cmdout, flags.extended, !flags.noColor) + res.Human(cmd.OutOrStdout(), flags.extended, !flags.noColor) if res.Alive() == 0 && !flags.cfg.NoProbe { return app.WithExitCode(serrors.New("no path alive"), 1) } case "json": - return res.JSON(os.Stdout) + enc := json.NewEncoder(os.Stdout) + enc.SetIndent("", " ") + enc.SetEscapeHTML(false) + return enc.Encode(res) case "yaml": - return res.YAML(os.Stdout) + enc := yaml.NewEncoder(os.Stdout) + return enc.Encode(res) default: return serrors.New("output format not supported", "format", flags.format) } diff --git a/scion/cmd/scion/traceroute.go b/scion/cmd/scion/traceroute.go index 468a75c0dc..fd718f099e 100644 --- a/scion/cmd/scion/traceroute.go +++ b/scion/cmd/scion/traceroute.go @@ -16,8 +16,8 @@ package main import ( "context" + "encoding/json" "fmt" - "io" "net" "os" "strings" @@ -25,13 +25,14 @@ import ( "time" "github.com/spf13/cobra" + "gopkg.in/yaml.v2" + "github.com/scionproto/scion/pkg/addr" "github.com/scionproto/scion/pkg/daemon" "github.com/scionproto/scion/pkg/log" "github.com/scionproto/scion/pkg/private/serrors" "github.com/scionproto/scion/pkg/snet" "github.com/scionproto/scion/pkg/snet/addrutil" - snetpath "github.com/scionproto/scion/pkg/snet/path" "github.com/scionproto/scion/pkg/sock/reliable" "github.com/scionproto/scion/private/app" "github.com/scionproto/scion/private/app/flag" @@ -42,6 +43,34 @@ import ( "github.com/scionproto/scion/scion/traceroute" ) +type ResultTraceroute struct { + Path TraceroutePath `json:"path" yaml:"path"` + Hops []HopInfo `json:"hops" yaml:"hops"` +} + +// Path defines model for Path. +type TraceroutePath struct { + Path + // Expiration time of the path. + Expiry time.Time `json:"expiry" yaml:"expiry"` + + // Optional array of latency measurements between any two consecutive interfaces. Entry i + // describes the latency between interface i and i+1. + Latency []time.Duration `json:"latency,omitempty" yaml:"latency,omitempty"` + + // The maximum transmission unit in bytes for SCION packets. This represents the protocol data + // unit (PDU) of the SCION layer on this path. + MTU int `json:"mtu" yaml:"mtu"` +} + +type HopInfo struct { + InterfaceID uint16 `json:"interface_id" yaml:"interface_id"` + // IP address of the router responding to the traceroute request. + IP string `json:"ip" yaml:"ip"` + IA addr.IA `json:"isd_as" yaml:"isd_as"` + RoundTripTimes []time.Duration `json:"round_trip_times" yaml:"round_trip_times"` +} + func newTraceroute(pather CommandPather) *cobra.Command { var envFlags flag.SCIONEnvironment var flags struct { @@ -85,14 +114,9 @@ On other errors, traceroute will exit with code 2. return serrors.WrapStr("setting up tracing", err) } defer closer() - var cmdout io.Writer - switch flags.format { - case "human": - cmdout = os.Stdout - case "json", "yaml": - cmdout = io.Discard - default: - return serrors.New("format not supported", "format", flags.format) + printf, err := getPrintf(flags.format, cmd.OutOrStdout()) + if err != nil { + return serrors.WrapStr("get formatting", err) } cmd.SilenceUsage = true @@ -152,24 +176,31 @@ On other errors, traceroute will exit with code 2. if localIP, err = addrutil.ResolveLocal(target); err != nil { return serrors.WrapStr("resolving local address", err) } - fmt.Fprintf(cmdout, "Resolved local address:\n %s\n", localIP) + printf("Resolved local address:\n %s\n", localIP) } - fmt.Fprintf(cmdout, "Using path:\n %s\n\n", path) + printf("Using path:\n %s\n\n", path) seq, err := pathpol.GetSequence(path) if err != nil { return serrors.New("get sequence from used path") } - var res traceroute.Result - res.Path = traceroute.Path{ - Expiry: path.Metadata().Expiry, - Fingerprint: snet.Fingerprint(path).String(), - Hops: snetpath.GetHops(path), - Sequence: seq, - Latency: path.Metadata().Latency, - LocalIp: localIP, - Mtu: int(path.Metadata().MTU), - NextHop: path.UnderlayNextHop().String(), + var res ResultTraceroute + res.Path = TraceroutePath{ + Path: Path{ + Fingerprint: snet.Fingerprint(path).String(), + Hops: getHops(path), + Sequence: seq, + LocalIP: localIP, + NextHop: path.UnderlayNextHop().String(), + }, + Expiry: path.Metadata().Expiry, + // Fingerprint: snet.Fingerprint(path).String(), + // Hops: getHops(path), + // Sequence: seq, + Latency: path.Metadata().Latency, + // LocalIP: localIP, + MTU: int(path.Metadata().MTU), + // NextHop: path.UnderlayNextHop().String(), } span.SetTag("src.host", localIP) @@ -191,7 +222,7 @@ On other errors, traceroute will exit with code 2. ErrHandler: func(err error) { fmt.Fprintf(os.Stderr, "ERROR: %s\n", err) }, UpdateHandler: func(u traceroute.Update) { updates = append(updates, u) - fmt.Fprintf(cmdout, "%d %s %s\n", u.Index, fmtRemote(u.Remote, u.Interface), + printf("%d %s %s\n", u.Index, fmtRemote(u.Remote, u.Interface), fmtRTTs(u.RTTs, flags.timeout)) }, EPIC: flags.epic, @@ -200,11 +231,11 @@ On other errors, traceroute will exit with code 2. if err != nil { return err } - res.Hops = make([]traceroute.HopInfo, 0, len(updates)) + res.Hops = make([]HopInfo, 0, len(updates)) for _, update := range updates { - res.Hops = append(res.Hops, traceroute.HopInfo{ + res.Hops = append(res.Hops, HopInfo{ InterfaceID: uint16(update.Interface), - IP: update.Remote.Host.IP.String(), + IP: update.Remote.Host.IP().String(), IA: update.Remote.IA, RoundTripTimes: update.RTTs, }) @@ -216,9 +247,13 @@ On other errors, traceroute will exit with code 2. return app.WithExitCode(serrors.New("packets were lost"), 1) } case "json": - return res.JSON(os.Stdout) + enc := json.NewEncoder(os.Stdout) + enc.SetIndent("", " ") + enc.SetEscapeHTML(false) + return enc.Encode(res) case "yaml": - return res.YAML(os.Stdout) + enc := yaml.NewEncoder(os.Stdout) + return enc.Encode(res) } return nil }, @@ -250,9 +285,9 @@ func fmtRTTs(rtts []time.Duration, timeout time.Duration) string { return strings.Join(parts, " ") } -func fmtRemote(remote *snet.UDPAddr, intf uint64) string { - if remote == nil { - return "??" - } +func fmtRemote(remote snet.SCIONAddress, intf uint64) string { + // if remote == nil { + // return "??" + // } return fmt.Sprintf("%s IfID=%d", remote, intf) } diff --git a/scion/showpaths/BUILD.bazel b/scion/showpaths/BUILD.bazel index 7bd10a36ce..8f3139a275 100644 --- a/scion/showpaths/BUILD.bazel +++ b/scion/showpaths/BUILD.bazel @@ -17,6 +17,5 @@ go_library( "//private/app/path:go_default_library", "//private/app/path/pathprobe:go_default_library", "//private/path/pathpol:go_default_library", - "@in_gopkg_yaml_v2//:go_default_library", ], ) diff --git a/scion/showpaths/showpaths.go b/scion/showpaths/showpaths.go index 561eb517ea..fc2053de03 100644 --- a/scion/showpaths/showpaths.go +++ b/scion/showpaths/showpaths.go @@ -16,7 +16,6 @@ package showpaths import ( "context" - "encoding/json" "fmt" "io" "math" @@ -34,7 +33,6 @@ import ( "github.com/scionproto/scion/private/app/path" "github.com/scionproto/scion/private/app/path/pathprobe" "github.com/scionproto/scion/private/path/pathpol" - "gopkg.in/yaml.v2" ) // Result contains all the discovered paths. @@ -289,20 +287,6 @@ func sanitizeString(str string) string { }, str) } -// JSON writes the showpaths result as a json object to the writer. -func (r Result) JSON(w io.Writer) error { - enc := json.NewEncoder(w) - enc.SetIndent("", " ") - enc.SetEscapeHTML(false) - return enc.Encode(r) -} - -// JSON writes the showpaths result as a yaml object to the writer. -func (r Result) YAML(w io.Writer) error { - enc := yaml.NewEncoder(w) - return enc.Encode(r) -} - // IsLocal returns true iff Source and Destination AS are identical func (r Result) IsLocal() bool { return r.LocalIA == r.Destination @@ -414,10 +398,11 @@ func Run(ctx context.Context, dst addr.IA, cfg Config) (*Result, error) { rpath.Local = status.LocalIP } seq, err := pathpol.GetSequence(path) + rpath.Sequence = seq if err != nil { + // rpath.Sequence = "invalid" continue } - rpath.Sequence = seq res.Paths = append(res.Paths, rpath) } return res, nil diff --git a/scion/traceroute/BUILD.bazel b/scion/traceroute/BUILD.bazel index d755dafd62..b58bd9fe4a 100644 --- a/scion/traceroute/BUILD.bazel +++ b/scion/traceroute/BUILD.bazel @@ -14,6 +14,5 @@ go_library( "//pkg/snet:go_default_library", "//pkg/snet/path:go_default_library", "//pkg/sock/reliable:go_default_library", - "@in_gopkg_yaml_v2//:go_default_library", ], ) diff --git a/scion/traceroute/traceroute.go b/scion/traceroute/traceroute.go index 775ca3cefe..220758d57f 100644 --- a/scion/traceroute/traceroute.go +++ b/scion/traceroute/traceroute.go @@ -17,8 +17,6 @@ package traceroute import ( "context" - "encoding/json" - "io" "math/rand" "net" "time" @@ -29,9 +27,8 @@ import ( "github.com/scionproto/scion/pkg/private/serrors" "github.com/scionproto/scion/pkg/slayers/path/scion" "github.com/scionproto/scion/pkg/snet" - snetpath "github.com/scionproto/scion/pkg/snet/path" + "github.com/scionproto/scion/pkg/snet/path" "github.com/scionproto/scion/pkg/sock/reliable" - "gopkg.in/yaml.v2" ) // Update contains the information for a single hop. @@ -39,7 +36,7 @@ type Update struct { // Index indicates the hop index in the path. Index int // Remote is the remote router. - Remote *snet.UDPAddr + Remote snet.SCIONAddress // Interface is the interface ID of the remote router. Interface uint64 // RTTs are the RTTs for this hop. To detect whether there was a timeout the @@ -49,7 +46,7 @@ type Update struct { } func (u Update) empty() bool { - return u.Index == 0 && u.Remote == nil && u.Interface == 0 && len(u.RTTs) == 0 + return u.Index == 0 && u.Interface == 0 && len(u.RTTs) == 0 } // Stats contains the amount of sent and received packets. @@ -57,42 +54,6 @@ type Stats struct { Sent, Recv uint } -type Result struct { - Path Path `json:"path" yaml:"path"` - Hops []HopInfo `json:"hops" yaml:"hops"` -} - -type HopInfo struct { - InterfaceID uint16 `json:"interface_id" yaml:"interface_id"` - // IP address of the router responding to the traceroute request. - IP string `json:"ip" yaml:"ip"` - IA addr.IA `json:"isd_as" yaml:"isd_as"` - RoundTripTimes []time.Duration `json:"round_trip_times" yaml:"round_trip_times"` -} - -// Path defines model for Path. -type Path struct { - // Expiration time of the path. - Expiry time.Time `json:"expiry" yaml:"expiry"` - - // Hex-string representing the paths fingerprint. - Fingerprint string `json:"fingerprint" yaml:"fingerprint"` - Hops []snet.PathInterface `json:"hops" yaml:"hops"` - Sequence string `json:"hops_sequence" yaml:"hops_sequence"` - - // Optional array of latency measurements between any two consecutive interfaces. Entry i - // describes the latency between interface i and i+1. - Latency []time.Duration `json:"latency,omitempty" yaml:"latency,omitempty"` - LocalIp net.IP `json:"local_ip,omitempty" yaml:"local_ip,omitempty"` - - // The maximum transmission unit in bytes for SCION packets. This represents the protocol data - // unit (PDU) of the SCION layer on this path. - Mtu int `json:"mtu" yaml:"mtu"` - - // The internal UDP/IP underlay address of the SCION router that forwards traffic for this path. - NextHop string `json:"next_hop" yaml:"next_hop"` -} - // Config configures the traceroute run. type Config struct { Dispatcher reliable.Dispatcher @@ -135,7 +96,7 @@ type tracerouter struct { // Run runs the traceroute. func Run(ctx context.Context, cfg Config) (Stats, error) { - if _, isEmpty := cfg.PathEntry.Dataplane().(snetpath.Empty); isEmpty { + if _, isEmpty := cfg.PathEntry.Dataplane().(path.Empty); isEmpty { return Stats{}, serrors.New("empty path is not allowed for traceroute") } id := rand.Uint64() @@ -167,7 +128,7 @@ func Run(ctx context.Context, cfg Config) (Stats, error) { } func (t *tracerouter) Traceroute(ctx context.Context) (Stats, error) { - scionPath, ok := t.path.Dataplane().(snetpath.SCION) + scionPath, ok := t.path.Dataplane().(path.SCION) if !ok { return Stats{}, serrors.New("only SCION path allowed for traceroute", "type", common.TypeOf(t.path.Dataplane())) @@ -227,7 +188,7 @@ func (t *tracerouter) Traceroute(ctx context.Context) (Stats, error) { func (t *tracerouter) probeHop(ctx context.Context, hfIdx uint8, egress bool) (Update, error) { var decoded scion.Decoded - if err := decoded.DecodeFromBytes(t.path.Dataplane().(snetpath.SCION).Raw); err != nil { + if err := decoded.DecodeFromBytes(t.path.Dataplane().(path.SCION).Raw); err != nil { return Update{}, serrors.WrapStr("decoding path", err) } @@ -238,14 +199,14 @@ func (t *tracerouter) probeHop(ctx context.Context, hfIdx uint8, egress bool) (U hf.IngressRouterAlert = true } - scionAlertPath, err := snetpath.NewSCIONFromDecoded(decoded) + scionAlertPath, err := path.NewSCIONFromDecoded(decoded) if err != nil { return Update{}, serrors.WrapStr("setting alert flag", err) } var alertPath snet.DataplanePath if t.epic { - epicAlertPath, err := snetpath.NewEPICDataplanePath( + epicAlertPath, err := path.NewEPICDataplanePath( scionAlertPath, t.path.Metadata().EpicAuths, ) @@ -335,7 +296,7 @@ func (t tracerouter) drain(ctx context.Context) { type reply struct { Received time.Time Reply snet.SCMPTracerouteReply - Remote *snet.UDPAddr + Remote snet.SCIONAddress Error error } @@ -345,37 +306,13 @@ type scmpHandler struct { func (h scmpHandler) Handle(pkt *snet.Packet) error { r, err := h.handle(pkt) - ip := make(net.IP, len(pkt.Source.Host.IP())) - copy(ip, pkt.Source.Host.IP()) - - var path snet.DataplanePath - switch p := pkt.Path.(type) { - case snetpath.SCION: - raw := make([]byte, len(p.Raw)) - copy(raw, p.Raw) - path = snetpath.SCION{ - Raw: raw, - } - case *snetpath.EPIC: - path = snetpath.CloneEICDataplanePath(p) - case snet.RawPath: - raw := make([]byte, len(p.Raw)) - copy(raw, p.Raw) - path = snet.RawPath{ - PathType: p.PathType, - Raw: raw, - } - default: - return serrors.New("unknown type for packet Path", "type", common.TypeOf(pkt.Path)) - } h.replies <- reply{ Received: time.Now(), Reply: r, - Remote: &snet.UDPAddr{ + Remote: snet.SCIONAddress{ IA: pkt.Source.IA, - Host: &net.UDPAddr{IP: ip}, - Path: path, + Host: pkt.Source.Host.Copy(), }, Error: err, } @@ -393,17 +330,3 @@ func (h scmpHandler) handle(pkt *snet.Packet) (snet.SCMPTracerouteReply, error) } return r, nil } - -// JSON writes the traceroute result as a json object to the writer. -func (r Result) JSON(w io.Writer) error { - enc := json.NewEncoder(w) - enc.SetIndent("", " ") - enc.SetEscapeHTML(false) - return enc.Encode(r) -} - -// JSON writes the traceroute result as a yaml object to the writer. -func (r Result) YAML(w io.Writer) error { - enc := yaml.NewEncoder(w) - return enc.Encode(r) -} From 85021dfc822a0f7292103364a14dfc60a1a7983d Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Mon, 31 Oct 2022 10:51:26 +0100 Subject: [PATCH 08/23] small changes (linter etc) --- scion/cmd/scion/common.go | 5 ++++- scion/cmd/scion/ping.go | 7 ++++++- scion/cmd/scion/traceroute.go | 12 ++---------- scion/showpaths/showpaths.go | 3 +-- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/scion/cmd/scion/common.go b/scion/cmd/scion/common.go index 80a363cf77..97b1eb47b8 100644 --- a/scion/cmd/scion/common.go +++ b/scion/cmd/scion/common.go @@ -46,7 +46,10 @@ func getHops(path snet.Path) []Hop { // getPrintf returns a printf function for the "human" formatting flag and an empty one for machine // readable format flags -func getPrintf(outputFlag string, writer io.Writer) (func(format string, ctx ...interface{}), error) { +func getPrintf(outputFlag string, writer io.Writer) ( + func(format string, ctx ...interface{}), + error, +) { printf := func(format string, ctx ...interface{}) {} switch outputFlag { case "human": diff --git a/scion/cmd/scion/ping.go b/scion/cmd/scion/ping.go index 70af921879..eab3d2a933 100644 --- a/scion/cmd/scion/ping.go +++ b/scion/cmd/scion/ping.go @@ -354,7 +354,12 @@ func calcMaxPldSize(local, remote *snet.UDPAddr, mtu int) (int, error) { return mtu - overhead, nil } -func pingSummary(stats *ping.Stats, remote *snet.UDPAddr, run time.Duration, printf func(format string, ctx ...interface{})) { +func pingSummary( + stats *ping.Stats, + remote *snet.UDPAddr, + run time.Duration, + printf func(format string, ctx ...interface{}), +) { if stats.Sent != 0 { stats.Loss = 100 - stats.Received*100/stats.Sent } diff --git a/scion/cmd/scion/traceroute.go b/scion/cmd/scion/traceroute.go index fd718f099e..eb55d9a356 100644 --- a/scion/cmd/scion/traceroute.go +++ b/scion/cmd/scion/traceroute.go @@ -193,14 +193,9 @@ On other errors, traceroute will exit with code 2. LocalIP: localIP, NextHop: path.UnderlayNextHop().String(), }, - Expiry: path.Metadata().Expiry, - // Fingerprint: snet.Fingerprint(path).String(), - // Hops: getHops(path), - // Sequence: seq, + Expiry: path.Metadata().Expiry, Latency: path.Metadata().Latency, - // LocalIP: localIP, - MTU: int(path.Metadata().MTU), - // NextHop: path.UnderlayNextHop().String(), + MTU: int(path.Metadata().MTU), } span.SetTag("src.host", localIP) @@ -286,8 +281,5 @@ func fmtRTTs(rtts []time.Duration, timeout time.Duration) string { } func fmtRemote(remote snet.SCIONAddress, intf uint64) string { - // if remote == nil { - // return "??" - // } return fmt.Sprintf("%s IfID=%d", remote, intf) } diff --git a/scion/showpaths/showpaths.go b/scion/showpaths/showpaths.go index fc2053de03..40356ec74b 100644 --- a/scion/showpaths/showpaths.go +++ b/scion/showpaths/showpaths.go @@ -400,8 +400,7 @@ func Run(ctx context.Context, dst addr.IA, cfg Config) (*Result, error) { seq, err := pathpol.GetSequence(path) rpath.Sequence = seq if err != nil { - // rpath.Sequence = "invalid" - continue + rpath.Sequence = "invalid" } res.Paths = append(res.Paths, rpath) } From ca0ef39c5550afc4c7f62bd374d68b3c73d49f0c Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Mon, 31 Oct 2022 11:20:28 +0100 Subject: [PATCH 09/23] code-block formatting error --- private/path/pathpol/sequence.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/private/path/pathpol/sequence.go b/private/path/pathpol/sequence.go index 33378e2be4..3c0ec123ad 100644 --- a/private/path/pathpol/sequence.go +++ b/private/path/pathpol/sequence.go @@ -296,8 +296,9 @@ func hop(ia addr.IA, ingress, egress common.IFIDType) string { } // GetSequence constructs the sequence string from snet path -// example format: -// 1-ff00:0:133#0 1-ff00:0:120#2,1 1-ff00:0:110#0 +// output format: +// +// 1-ff00:0:133#0 1-ff00:0:120#2,1 1-ff00:0:110#0 func GetSequence(path snet.Path) (string, error) { var desc string ifaces := path.Metadata().Interfaces From 4fee854be16588cee839ea36571c89aceccb053d Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Tue, 1 Nov 2022 09:33:27 +0100 Subject: [PATCH 10/23] feedback 3 --- private/path/pathpol/sequence.go | 2 +- scion/cmd/scion/common.go | 16 +++----- scion/cmd/scion/ping.go | 69 +++++++++++++++++--------------- scion/cmd/scion/showpaths.go | 8 ++-- scion/cmd/scion/traceroute.go | 12 +++--- scion/ping/ping.go | 4 -- scion/traceroute/traceroute.go | 2 +- 7 files changed, 52 insertions(+), 61 deletions(-) diff --git a/private/path/pathpol/sequence.go b/private/path/pathpol/sequence.go index 3c0ec123ad..3c50fdb56f 100644 --- a/private/path/pathpol/sequence.go +++ b/private/path/pathpol/sequence.go @@ -298,7 +298,7 @@ func hop(ia addr.IA, ingress, egress common.IFIDType) string { // GetSequence constructs the sequence string from snet path // output format: // -// 1-ff00:0:133#0 1-ff00:0:120#2,1 1-ff00:0:110#0 +// 1-ff00:0:133#42 1-ff00:0:120#2,1 1-ff00:0:110#21 func GetSequence(path snet.Path) (string, error) { var desc string ifaces := path.Metadata().Interfaces diff --git a/scion/cmd/scion/common.go b/scion/cmd/scion/common.go index 97b1eb47b8..3721e436f8 100644 --- a/scion/cmd/scion/common.go +++ b/scion/cmd/scion/common.go @@ -7,11 +7,10 @@ import ( "github.com/scionproto/scion/pkg/addr" "github.com/scionproto/scion/pkg/private/common" - "github.com/scionproto/scion/pkg/private/serrors" "github.com/scionproto/scion/pkg/snet" ) -// Path defines model for Path. +// Path defines the base model for the `ping` and `traceroute` result path type Path struct { // Hex-string representing the paths fingerprint. Fingerprint string `json:"fingerprint" yaml:"fingerprint"` @@ -46,20 +45,15 @@ func getHops(path snet.Path) []Hop { // getPrintf returns a printf function for the "human" formatting flag and an empty one for machine // readable format flags -func getPrintf(outputFlag string, writer io.Writer) ( - func(format string, ctx ...interface{}), - error, -) { - printf := func(format string, ctx ...interface{}) {} +func getPrintf(outputFlag string, writer io.Writer) func(format string, ctx ...interface{}) { switch outputFlag { case "human": - printf = func(format string, ctx ...interface{}) { + return func(format string, ctx ...interface{}) { fmt.Fprintf(writer, format, ctx...) } - return printf, nil case "yaml", "json": - return printf, nil + return func(format string, ctx ...interface{}) {} default: - return printf, serrors.New("format not supported", "format", outputFlag) + return nil } } diff --git a/scion/cmd/scion/ping.go b/scion/cmd/scion/ping.go index eab3d2a933..19e21e4953 100644 --- a/scion/cmd/scion/ping.go +++ b/scion/cmd/scion/ping.go @@ -47,7 +47,15 @@ type Result struct { PayloadSize int `json:"payload_size" yaml:"payload_size"` ScionPacketSize int `json:"scion_packet_size" yaml:"scion_packet_size"` Replies []PingUpdate `json:"replies" yaml:"replies"` - Statistics ping.Stats `json:"statistics" yaml:"statistics"` + Statistics Stats `json:"statistics" yaml:"statistics"` +} + +type Stats struct { + ping.Stats + MinRTT time.Duration `json:"min_RTT" yaml:"min_RTT"` + AvgRTT time.Duration `json:"avg_RTT" yaml:"avg_RTT"` + MaxRTT time.Duration `json:"max_RTT" yaml:"max_RTT"` + MdevRTT time.Duration `json:"mdev_RTT" yaml:"mdev_RTT"` } type PingUpdate struct { @@ -92,8 +100,6 @@ and reports back the statistics. When the \--healthy-only option is set, ping first determines healthy paths through probing and chooses amongst them. -'ping' can be instructed to output the paths in a specific format using the \--format flag. - If no reply packet is received at all, ping will exit with code 1. On other errors, ping will exit with code 2. @@ -112,9 +118,9 @@ On other errors, ping will exit with code 2. return serrors.WrapStr("setting up tracing", err) } defer closer() - printf, err := getPrintf(flags.format, cmd.OutOrStdout()) - if err != nil { - return serrors.WrapStr("get formatting", err) + printf := getPrintf(flags.format, cmd.OutOrStdout()) + if printf == nil { + return fmt.Errorf("output format not supported: %s", flags.format) } cmd.SilenceUsage = true @@ -286,15 +292,15 @@ On other errors, ping will exit with code 2. }) printf("%d bytes from %s,%s: scmp_seq=%d time=%s%s\n", update.Size, update.Source.IA, update.Source.Host, update.Sequence, - update.RTT, additional) + fmt.Sprintf("%.3fms", float64(update.RTT.Nanoseconds())/1e6), additional) }, }) - processRTTs(&stats, res.Replies) - pingSummary(&stats, remote, time.Since(start), printf) if err != nil { return err } - res.Statistics = stats + res.Statistics.Stats = stats + res.Statistics.processRTTs(res.Replies) + res.Statistics.pingSummary(remote, time.Since(start), printf) switch flags.format { case "human": @@ -354,53 +360,52 @@ func calcMaxPldSize(local, remote *snet.UDPAddr, mtu int) (int, error) { return mtu - overhead, nil } -func pingSummary( - stats *ping.Stats, +func (s *Stats) pingSummary( remote *snet.UDPAddr, run time.Duration, printf func(format string, ctx ...interface{}), ) { - if stats.Sent != 0 { - stats.Loss = 100 - stats.Received*100/stats.Sent + if s.Stats.Sent != 0 { + s.Stats.Loss = 100 - s.Stats.Received*100/s.Stats.Sent } - stats.Time = run + s.Stats.Time = run printf("\n--- %s,%s statistics ---\n", remote.IA, remote.Host.IP) printf("%d packets transmitted, %d received, %d%% packet loss, time %v\n", - stats.Sent, stats.Received, stats.Loss, stats.Time.Round(time.Microsecond)) - if stats.Received != 0 { - printf("rtt min/avg/max/mdev = %v/%v/%v/%v\n", - stats.MinRTT.Round(time.Microsecond), - stats.AvgRTT.Round(time.Microsecond), - stats.MaxRTT.Round(time.Microsecond), - stats.MdevRTT.Round(time.Microsecond), + s.Stats.Sent, s.Stats.Received, s.Stats.Loss, s.Stats.Time.Round(time.Microsecond)) + if s.Stats.Received != 0 { + printf("rtt min/avg/max/mdev = %s/%s/%s/%s ms\n", + fmt.Sprintf("%.3f", float64(s.MinRTT.Nanoseconds())/1e6), + fmt.Sprintf("%.3f", float64(s.AvgRTT.Nanoseconds())/1e6), + fmt.Sprintf("%.3f", float64(s.MaxRTT.Nanoseconds())/1e6), + fmt.Sprintf("%.3f", float64(s.MdevRTT.Nanoseconds())/1e6), ) } } // processRTTs computes the min, average, max and standard deviation of the update RTTs -func processRTTs(stats *ping.Stats, replies []PingUpdate) { +func (s *Stats) processRTTs(replies []PingUpdate) { if len(replies) == 0 { return } - stats.MinRTT = replies[0].RTT - stats.MaxRTT = replies[0].RTT + s.MinRTT = replies[0].RTT + s.MaxRTT = replies[0].RTT var sum time.Duration for i := 0; i < len(replies); i++ { - if replies[i].RTT < stats.MinRTT { - stats.MinRTT = replies[i].RTT + if replies[i].RTT < s.MinRTT { + s.MinRTT = replies[i].RTT } - if replies[i].RTT > stats.MaxRTT { - stats.MaxRTT = replies[i].RTT + if replies[i].RTT > s.MaxRTT { + s.MaxRTT = replies[i].RTT } sum += replies[i].RTT } - stats.AvgRTT = time.Duration(int(sum.Nanoseconds()) / len(replies)) + s.AvgRTT = time.Duration(int(sum.Nanoseconds()) / len(replies)) // standard deviation var sd float64 for i := 0; i < len(replies); i++ { - sd += math.Pow(float64(replies[i].RTT)-float64(stats.AvgRTT), 2) + sd += math.Pow(float64(replies[i].RTT)-float64(s.AvgRTT), 2) } - stats.MdevRTT = time.Duration(math.Sqrt(sd / float64(len(replies)))) + s.MdevRTT = time.Duration(math.Sqrt(sd / float64(len(replies)))) } diff --git a/scion/cmd/scion/showpaths.go b/scion/cmd/scion/showpaths.go index d95cda0362..77421ea5ea 100644 --- a/scion/cmd/scion/showpaths.go +++ b/scion/cmd/scion/showpaths.go @@ -64,8 +64,6 @@ By default, the paths are probed. Paths served from the SCION Deamon's might not forward traffic successfully (e.g. if a network link went down, or there is a black hole on the path). To disable path probing, set the appropriate flag. -'showpaths' can be instructed to output the paths in a specific format using the \--format flag. - If no alive path is discovered, json output is not enabled, and probing is not disabled, showpaths will exit with the code 1. On other errors, showpaths will exit with code 2. @@ -84,9 +82,9 @@ On other errors, showpaths will exit with code 2. return serrors.WrapStr("setting up tracing", err) } defer closer() - printf, err := getPrintf(flags.format, cmd.OutOrStdout()) - if err != nil { - return serrors.WrapStr("get formatting", err) + printf := getPrintf(flags.format, cmd.OutOrStdout()) + if printf == nil { + return fmt.Errorf("output format not supported: %s", flags.format) } cmd.SilenceUsage = true diff --git a/scion/cmd/scion/traceroute.go b/scion/cmd/scion/traceroute.go index eb55d9a356..59bf7b5a6a 100644 --- a/scion/cmd/scion/traceroute.go +++ b/scion/cmd/scion/traceroute.go @@ -48,7 +48,7 @@ type ResultTraceroute struct { Hops []HopInfo `json:"hops" yaml:"hops"` } -// Path defines model for Path. +// TraceroutePath extends the basic Path model with additional information type TraceroutePath struct { Path // Expiration time of the path. @@ -94,8 +94,6 @@ func newTraceroute(pather CommandPather) *cobra.Command { Long: fmt.Sprintf(`'traceroute' traces the SCION path to a remote AS using SCMP traceroute packets. -'showpaths' can be instructed to output the paths in a specific format using the \--format flag. - If any packet is dropped, traceroute will exit with code 1. On other errors, traceroute will exit with code 2. %s`, app.SequenceHelp), @@ -114,9 +112,9 @@ On other errors, traceroute will exit with code 2. return serrors.WrapStr("setting up tracing", err) } defer closer() - printf, err := getPrintf(flags.format, cmd.OutOrStdout()) - if err != nil { - return serrors.WrapStr("get formatting", err) + printf := getPrintf(flags.format, cmd.OutOrStdout()) + if printf == nil { + return fmt.Errorf("output format not supported: %s", flags.format) } cmd.SilenceUsage = true @@ -275,7 +273,7 @@ func fmtRTTs(rtts []time.Duration, timeout time.Duration) string { parts = append(parts, "*") continue } - parts = append(parts, rtt.String()) + parts = append(parts, fmt.Sprintf("%.3fms", float64(rtt.Nanoseconds())/1e6)) } return strings.Join(parts, " ") } diff --git a/scion/ping/ping.go b/scion/ping/ping.go index adf95fddb3..efd2862510 100644 --- a/scion/ping/ping.go +++ b/scion/ping/ping.go @@ -37,10 +37,6 @@ type Stats struct { Received int `json:"received" yaml:"received"` Loss int `json:"packet_loss" yaml:"packet_loss"` Time time.Duration `json:"time" yaml:"time"` - MinRTT time.Duration `json:"min_RTT" yaml:"min_RTT"` - AvgRTT time.Duration `json:"avg_RTT" yaml:"avg_RTT"` - MaxRTT time.Duration `json:"max_RTT" yaml:"max_RTT"` - MdevRTT time.Duration `json:"mdev_RTT" yaml:"mdev_RTT"` } // Update contains intermediary information about a received echo reply diff --git a/scion/traceroute/traceroute.go b/scion/traceroute/traceroute.go index 220758d57f..8c9c75251c 100644 --- a/scion/traceroute/traceroute.go +++ b/scion/traceroute/traceroute.go @@ -46,7 +46,7 @@ type Update struct { } func (u Update) empty() bool { - return u.Index == 0 && u.Interface == 0 && len(u.RTTs) == 0 + return u.Index == 0 && u.Remote == (snet.SCIONAddress{}) && u.Interface == 0 && len(u.RTTs) == 0 } // Stats contains the amount of sent and received packets. From 2f3dbec6c0b2a9dc62cffa11d85bf880a18b6472 Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Tue, 1 Nov 2022 15:30:20 +0100 Subject: [PATCH 11/23] feedback 4 --- scion/cmd/scion/common.go | 2 +- scion/cmd/scion/ping.go | 50 +++++++++++++++++------------------ scion/cmd/scion/showpaths.go | 2 +- scion/cmd/scion/traceroute.go | 36 ++++++------------------- 4 files changed, 34 insertions(+), 56 deletions(-) diff --git a/scion/cmd/scion/common.go b/scion/cmd/scion/common.go index 3721e436f8..e337c77b4c 100644 --- a/scion/cmd/scion/common.go +++ b/scion/cmd/scion/common.go @@ -15,7 +15,7 @@ type Path struct { // Hex-string representing the paths fingerprint. Fingerprint string `json:"fingerprint" yaml:"fingerprint"` Hops []Hop `json:"hops" yaml:"hops"` - Sequence string `json:"hops_sequence" yaml:"hops_sequence"` + Sequence string `json:"sequence" yaml:"sequence"` LocalIP net.IP `json:"local_ip,omitempty" yaml:"local_ip,omitempty"` diff --git a/scion/cmd/scion/ping.go b/scion/cmd/scion/ping.go index 19e21e4953..e0d05c57f5 100644 --- a/scion/cmd/scion/ping.go +++ b/scion/cmd/scion/ping.go @@ -52,10 +52,10 @@ type Result struct { type Stats struct { ping.Stats - MinRTT time.Duration `json:"min_RTT" yaml:"min_RTT"` - AvgRTT time.Duration `json:"avg_RTT" yaml:"avg_RTT"` - MaxRTT time.Duration `json:"max_RTT" yaml:"max_RTT"` - MdevRTT time.Duration `json:"mdev_RTT" yaml:"mdev_RTT"` + MinRTT time.Duration `json:"min_rtt" yaml:"min_rtt"` + AvgRTT time.Duration `json:"avg_rtt" yaml:"avg_rtt"` + MaxRTT time.Duration `json:"max_rtt" yaml:"max_rtt"` + MdevRTT time.Duration `json:"mdev_rtt" yaml:"mdev_rtt"` } type PingUpdate struct { @@ -298,12 +298,24 @@ On other errors, ping will exit with code 2. if err != nil { return err } + calculateStats(&stats, time.Since(start)) res.Statistics.Stats = stats - res.Statistics.processRTTs(res.Replies) - res.Statistics.pingSummary(remote, time.Since(start), printf) + processRTTs(&res.Statistics, res.Replies) switch flags.format { case "human": + s := res.Statistics.Stats + printf("\n--- %s,%s statistics ---\n", remote.IA, remote.Host.IP) + printf("%d packets transmitted, %d received, %d%% packet loss, time %v\n", + s.Sent, s.Received, s.Loss, s.Time.Round(time.Microsecond)) + if s.Received != 0 { + printf("rtt min/avg/max/mdev = %s/%s/%s/%s ms\n", + fmt.Sprintf("%.3f", float64(res.Statistics.MinRTT.Nanoseconds())/1e6), + fmt.Sprintf("%.3f", float64(res.Statistics.AvgRTT.Nanoseconds())/1e6), + fmt.Sprintf("%.3f", float64(res.Statistics.MaxRTT.Nanoseconds())/1e6), + fmt.Sprintf("%.3f", float64(res.Statistics.MdevRTT.Nanoseconds())/1e6), + ) + } if stats.Received == 0 { return app.WithExitCode(serrors.New("no reply packet received"), 1) } @@ -360,30 +372,16 @@ func calcMaxPldSize(local, remote *snet.UDPAddr, mtu int) (int, error) { return mtu - overhead, nil } -func (s *Stats) pingSummary( - remote *snet.UDPAddr, - run time.Duration, - printf func(format string, ctx ...interface{}), -) { - if s.Stats.Sent != 0 { - s.Stats.Loss = 100 - s.Stats.Received*100/s.Stats.Sent - } - s.Stats.Time = run - printf("\n--- %s,%s statistics ---\n", remote.IA, remote.Host.IP) - printf("%d packets transmitted, %d received, %d%% packet loss, time %v\n", - s.Stats.Sent, s.Stats.Received, s.Stats.Loss, s.Stats.Time.Round(time.Microsecond)) - if s.Stats.Received != 0 { - printf("rtt min/avg/max/mdev = %s/%s/%s/%s ms\n", - fmt.Sprintf("%.3f", float64(s.MinRTT.Nanoseconds())/1e6), - fmt.Sprintf("%.3f", float64(s.AvgRTT.Nanoseconds())/1e6), - fmt.Sprintf("%.3f", float64(s.MaxRTT.Nanoseconds())/1e6), - fmt.Sprintf("%.3f", float64(s.MdevRTT.Nanoseconds())/1e6), - ) +// calculateStats computes the loss percentage and sets the required time +func calculateStats(s *ping.Stats, run time.Duration) { + if s.Sent != 0 { + s.Loss = 100 - s.Received*100/s.Sent } + s.Time = run } // processRTTs computes the min, average, max and standard deviation of the update RTTs -func (s *Stats) processRTTs(replies []PingUpdate) { +func processRTTs(s *Stats, replies []PingUpdate) { if len(replies) == 0 { return } diff --git a/scion/cmd/scion/showpaths.go b/scion/cmd/scion/showpaths.go index 77421ea5ea..b9f161879e 100644 --- a/scion/cmd/scion/showpaths.go +++ b/scion/cmd/scion/showpaths.go @@ -119,7 +119,7 @@ On other errors, showpaths will exit with code 2. printf("Empty path, destination is local AS %s\n", res.Destination) return nil } - printf("Available paths to", res.Destination) + printf("Available paths to %s\n", res.Destination) if len(res.Paths) == 0 { return app.WithExitCode(serrors.New("no path found"), 1) } diff --git a/scion/cmd/scion/traceroute.go b/scion/cmd/scion/traceroute.go index 59bf7b5a6a..ff1ae32172 100644 --- a/scion/cmd/scion/traceroute.go +++ b/scion/cmd/scion/traceroute.go @@ -44,23 +44,8 @@ import ( ) type ResultTraceroute struct { - Path TraceroutePath `json:"path" yaml:"path"` - Hops []HopInfo `json:"hops" yaml:"hops"` -} - -// TraceroutePath extends the basic Path model with additional information -type TraceroutePath struct { - Path - // Expiration time of the path. - Expiry time.Time `json:"expiry" yaml:"expiry"` - - // Optional array of latency measurements between any two consecutive interfaces. Entry i - // describes the latency between interface i and i+1. - Latency []time.Duration `json:"latency,omitempty" yaml:"latency,omitempty"` - - // The maximum transmission unit in bytes for SCION packets. This represents the protocol data - // unit (PDU) of the SCION layer on this path. - MTU int `json:"mtu" yaml:"mtu"` + Path Path `json:"path" yaml:"path"` + Hops []HopInfo `json:"hops" yaml:"hops"` } type HopInfo struct { @@ -183,17 +168,12 @@ On other errors, traceroute will exit with code 2. return serrors.New("get sequence from used path") } var res ResultTraceroute - res.Path = TraceroutePath{ - Path: Path{ - Fingerprint: snet.Fingerprint(path).String(), - Hops: getHops(path), - Sequence: seq, - LocalIP: localIP, - NextHop: path.UnderlayNextHop().String(), - }, - Expiry: path.Metadata().Expiry, - Latency: path.Metadata().Latency, - MTU: int(path.Metadata().MTU), + res.Path = Path{ + Fingerprint: snet.Fingerprint(path).String(), + Hops: getHops(path), + Sequence: seq, + LocalIP: localIP, + NextHop: path.UnderlayNextHop().String(), } span.SetTag("src.host", localIP) From f3b6e00579d1e430c5f0244da9a22994d045d09b Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Tue, 1 Nov 2022 16:16:02 +0100 Subject: [PATCH 12/23] ping stats cleanup --- scion/cmd/scion/ping.go | 46 ++++++++++++++++++++++------------------- scion/ping/ping.go | 6 ++---- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/scion/cmd/scion/ping.go b/scion/cmd/scion/ping.go index e0d05c57f5..f410bf5f1f 100644 --- a/scion/cmd/scion/ping.go +++ b/scion/cmd/scion/ping.go @@ -52,6 +52,8 @@ type Result struct { type Stats struct { ping.Stats + Loss int `json:"packet_loss" yaml:"packet_loss"` + Time time.Duration `json:"time" yaml:"time"` MinRTT time.Duration `json:"min_rtt" yaml:"min_rtt"` AvgRTT time.Duration `json:"avg_rtt" yaml:"avg_rtt"` MaxRTT time.Duration `json:"max_rtt" yaml:"max_rtt"` @@ -298,16 +300,14 @@ On other errors, ping will exit with code 2. if err != nil { return err } - calculateStats(&stats, time.Since(start)) - res.Statistics.Stats = stats - processRTTs(&res.Statistics, res.Replies) + res.Statistics = calculateStats(stats, res.Replies, time.Since(start)) switch flags.format { case "human": s := res.Statistics.Stats printf("\n--- %s,%s statistics ---\n", remote.IA, remote.Host.IP) printf("%d packets transmitted, %d received, %d%% packet loss, time %v\n", - s.Sent, s.Received, s.Loss, s.Time.Round(time.Microsecond)) + s.Sent, s.Received, res.Statistics.Loss, res.Statistics.Time.Round(time.Microsecond)) if s.Received != 0 { printf("rtt min/avg/max/mdev = %s/%s/%s/%s ms\n", fmt.Sprintf("%.3f", float64(res.Statistics.MinRTT.Nanoseconds())/1e6), @@ -372,38 +372,42 @@ func calcMaxPldSize(local, remote *snet.UDPAddr, mtu int) (int, error) { return mtu - overhead, nil } -// calculateStats computes the loss percentage and sets the required time -func calculateStats(s *ping.Stats, run time.Duration) { +// calculateStats computes the Stats from the ping stats and updates +func calculateStats(s ping.Stats, replies []PingUpdate, run time.Duration) Stats { + var loss int if s.Sent != 0 { - s.Loss = 100 - s.Received*100/s.Sent + loss = 100 - s.Received*100/s.Sent + } + + stats := Stats{ + Stats: s, + Loss: loss, + Time: run, } - s.Time = run -} -// processRTTs computes the min, average, max and standard deviation of the update RTTs -func processRTTs(s *Stats, replies []PingUpdate) { if len(replies) == 0 { - return + return stats } - s.MinRTT = replies[0].RTT - s.MaxRTT = replies[0].RTT + stats.MinRTT = replies[0].RTT + stats.MaxRTT = replies[0].RTT var sum time.Duration for i := 0; i < len(replies); i++ { - if replies[i].RTT < s.MinRTT { - s.MinRTT = replies[i].RTT + if replies[i].RTT < stats.MinRTT { + stats.MinRTT = replies[i].RTT } - if replies[i].RTT > s.MaxRTT { - s.MaxRTT = replies[i].RTT + if replies[i].RTT > stats.MaxRTT { + stats.MaxRTT = replies[i].RTT } sum += replies[i].RTT } - s.AvgRTT = time.Duration(int(sum.Nanoseconds()) / len(replies)) + stats.AvgRTT = time.Duration(int(sum.Nanoseconds()) / len(replies)) // standard deviation var sd float64 for i := 0; i < len(replies); i++ { - sd += math.Pow(float64(replies[i].RTT)-float64(s.AvgRTT), 2) + sd += math.Pow(float64(replies[i].RTT)-float64(stats.AvgRTT), 2) } - s.MdevRTT = time.Duration(math.Sqrt(sd / float64(len(replies)))) + stats.MdevRTT = time.Duration(math.Sqrt(sd / float64(len(replies)))) + return stats } diff --git a/scion/ping/ping.go b/scion/ping/ping.go index efd2862510..c332bce42c 100644 --- a/scion/ping/ping.go +++ b/scion/ping/ping.go @@ -33,10 +33,8 @@ import ( // Stats contains the statistics of a ping run. type Stats struct { - Sent int `json:"sent" yaml:"sent"` - Received int `json:"received" yaml:"received"` - Loss int `json:"packet_loss" yaml:"packet_loss"` - Time time.Duration `json:"time" yaml:"time"` + Sent int `json:"sent" yaml:"sent"` + Received int `json:"received" yaml:"received"` } // Update contains intermediary information about a received echo reply From c78dc9b0566e2f133ca952b819ad4fa7302ac6e8 Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Tue, 1 Nov 2022 17:32:47 +0100 Subject: [PATCH 13/23] removed unnecessary Sprintf --- scion/cmd/scion/ping.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scion/cmd/scion/ping.go b/scion/cmd/scion/ping.go index f410bf5f1f..9715adf297 100644 --- a/scion/cmd/scion/ping.go +++ b/scion/cmd/scion/ping.go @@ -309,11 +309,11 @@ On other errors, ping will exit with code 2. printf("%d packets transmitted, %d received, %d%% packet loss, time %v\n", s.Sent, s.Received, res.Statistics.Loss, res.Statistics.Time.Round(time.Microsecond)) if s.Received != 0 { - printf("rtt min/avg/max/mdev = %s/%s/%s/%s ms\n", - fmt.Sprintf("%.3f", float64(res.Statistics.MinRTT.Nanoseconds())/1e6), - fmt.Sprintf("%.3f", float64(res.Statistics.AvgRTT.Nanoseconds())/1e6), - fmt.Sprintf("%.3f", float64(res.Statistics.MaxRTT.Nanoseconds())/1e6), - fmt.Sprintf("%.3f", float64(res.Statistics.MdevRTT.Nanoseconds())/1e6), + printf("rtt min/avg/max/mdev = %.3f/%.3f/%.3f/%.3f ms\n", + float64(res.Statistics.MinRTT.Nanoseconds())/1e6, + float64(res.Statistics.AvgRTT.Nanoseconds())/1e6, + float64(res.Statistics.MaxRTT.Nanoseconds())/1e6, + float64(res.Statistics.MdevRTT.Nanoseconds())/1e6, ) } if stats.Received == 0 { From a30eee9abc5dd235a546255ec095db08ba2b0879 Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Wed, 2 Nov 2022 08:58:11 +0100 Subject: [PATCH 14/23] failed sequence_test due to empty desc not beeing extended with whitespace --- private/path/pathpol/sequence.go | 3 +-- scion/cmd/scion/ping.go | 4 +++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/private/path/pathpol/sequence.go b/private/path/pathpol/sequence.go index 3c50fdb56f..876cf73e76 100644 --- a/private/path/pathpol/sequence.go +++ b/private/path/pathpol/sequence.go @@ -84,7 +84,6 @@ func (s *Sequence) Eval(paths []snet.Path) []snet.Path { result := []snet.Path{} for _, path := range paths { desc, err := GetSequence(path) - desc = desc + " " if err != nil { log.Error("get sequence from path", "err", err) continue @@ -323,7 +322,7 @@ func GetSequence(path snet.Path) (string, error) { hops = append(hops, hop(ifaces[i].IA, ifaces[i].ID, ifaces[i+1].ID)) } hops = append(hops, hop(ifaces[len(ifaces)-1].IA, ifaces[len(ifaces)-1].ID, 0)) - desc = strings.Join(hops, " ") + desc = strings.Join(hops, " ") + " " } return desc, nil } diff --git a/scion/cmd/scion/ping.go b/scion/cmd/scion/ping.go index 9715adf297..89afb9268d 100644 --- a/scion/cmd/scion/ping.go +++ b/scion/cmd/scion/ping.go @@ -307,7 +307,9 @@ On other errors, ping will exit with code 2. s := res.Statistics.Stats printf("\n--- %s,%s statistics ---\n", remote.IA, remote.Host.IP) printf("%d packets transmitted, %d received, %d%% packet loss, time %v\n", - s.Sent, s.Received, res.Statistics.Loss, res.Statistics.Time.Round(time.Microsecond)) + s.Sent, s.Received, res.Statistics.Loss, + res.Statistics.Time.Round(time.Microsecond), + ) if s.Received != 0 { printf("rtt min/avg/max/mdev = %.3f/%.3f/%.3f/%.3f ms\n", float64(res.Statistics.MinRTT.Nanoseconds())/1e6, From 5baf4d125efc673ddf5ba6d64b30038699eff328 Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Wed, 2 Nov 2022 09:29:31 +0100 Subject: [PATCH 15/23] unchecked error for mark deprecated flag --- scion/cmd/scion/showpaths.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scion/cmd/scion/showpaths.go b/scion/cmd/scion/showpaths.go index b9f161879e..43101a95ef 100644 --- a/scion/cmd/scion/showpaths.go +++ b/scion/cmd/scion/showpaths.go @@ -161,6 +161,9 @@ On other errors, showpaths will exit with code 2. cmd.Flags().StringVar(&flags.logLevel, "log.level", "", app.LogLevelUsage) cmd.Flags().StringVar(&flags.tracer, "tracing.agent", "", "Tracing agent address") cmd.Flags().BoolVar(&flags.cfg.Epic, "epic", false, "Enable EPIC.") - cmd.Flags().MarkDeprecated("json", "json flag is deprecated, use format flag") + err := cmd.Flags().MarkDeprecated("json", "json flag is deprecated, use format flag") + if err != nil { + panic(err) + } return cmd } From 597e3fba33b055a3df8ace8a66cadd07a7fe0ed2 Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Wed, 2 Nov 2022 14:39:14 +0100 Subject: [PATCH 16/23] getPrintf return error if format not supported --- scion/cmd/scion/common.go | 12 ++++++++---- scion/cmd/scion/ping.go | 6 +++--- scion/cmd/scion/showpaths.go | 6 +++--- scion/cmd/scion/traceroute.go | 7 +++---- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/scion/cmd/scion/common.go b/scion/cmd/scion/common.go index e337c77b4c..1c0df8c790 100644 --- a/scion/cmd/scion/common.go +++ b/scion/cmd/scion/common.go @@ -7,6 +7,7 @@ import ( "github.com/scionproto/scion/pkg/addr" "github.com/scionproto/scion/pkg/private/common" + "github.com/scionproto/scion/pkg/private/serrors" "github.com/scionproto/scion/pkg/snet" ) @@ -45,15 +46,18 @@ func getHops(path snet.Path) []Hop { // getPrintf returns a printf function for the "human" formatting flag and an empty one for machine // readable format flags -func getPrintf(outputFlag string, writer io.Writer) func(format string, ctx ...interface{}) { +func getPrintf(outputFlag string, writer io.Writer) ( + func(format string, ctx ...interface{}), + error, +) { switch outputFlag { case "human": return func(format string, ctx ...interface{}) { fmt.Fprintf(writer, format, ctx...) - } + }, nil case "yaml", "json": - return func(format string, ctx ...interface{}) {} + return func(format string, ctx ...interface{}) {}, nil default: - return nil + return nil, serrors.New("format not supported", "format", outputFlag) } } diff --git a/scion/cmd/scion/ping.go b/scion/cmd/scion/ping.go index 89afb9268d..cc1409529d 100644 --- a/scion/cmd/scion/ping.go +++ b/scion/cmd/scion/ping.go @@ -120,9 +120,9 @@ On other errors, ping will exit with code 2. return serrors.WrapStr("setting up tracing", err) } defer closer() - printf := getPrintf(flags.format, cmd.OutOrStdout()) - if printf == nil { - return fmt.Errorf("output format not supported: %s", flags.format) + printf, err := getPrintf(flags.format, cmd.OutOrStdout()) + if err != nil { + return serrors.WrapStr("get formatting", err) } cmd.SilenceUsage = true diff --git a/scion/cmd/scion/showpaths.go b/scion/cmd/scion/showpaths.go index 43101a95ef..814c9f702b 100644 --- a/scion/cmd/scion/showpaths.go +++ b/scion/cmd/scion/showpaths.go @@ -82,9 +82,9 @@ On other errors, showpaths will exit with code 2. return serrors.WrapStr("setting up tracing", err) } defer closer() - printf := getPrintf(flags.format, cmd.OutOrStdout()) - if printf == nil { - return fmt.Errorf("output format not supported: %s", flags.format) + printf, err := getPrintf(flags.format, cmd.OutOrStdout()) + if err != nil { + return serrors.WrapStr("get formatting", err) } cmd.SilenceUsage = true diff --git a/scion/cmd/scion/traceroute.go b/scion/cmd/scion/traceroute.go index ff1ae32172..0602d15afa 100644 --- a/scion/cmd/scion/traceroute.go +++ b/scion/cmd/scion/traceroute.go @@ -97,11 +97,10 @@ On other errors, traceroute will exit with code 2. return serrors.WrapStr("setting up tracing", err) } defer closer() - printf := getPrintf(flags.format, cmd.OutOrStdout()) - if printf == nil { - return fmt.Errorf("output format not supported: %s", flags.format) + printf, err := getPrintf(flags.format, cmd.OutOrStdout()) + if err != nil { + return serrors.WrapStr("get formatting", err) } - cmd.SilenceUsage = true if err := envFlags.LoadExternalVars(); err != nil { From b7b06b9e1709b9c78b2e1cb797f4fac1701a261c Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Wed, 2 Nov 2022 15:08:35 +0100 Subject: [PATCH 17/23] return values not split --- scion/cmd/scion/common.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/scion/cmd/scion/common.go b/scion/cmd/scion/common.go index 1c0df8c790..3a49454d44 100644 --- a/scion/cmd/scion/common.go +++ b/scion/cmd/scion/common.go @@ -46,11 +46,8 @@ func getHops(path snet.Path) []Hop { // getPrintf returns a printf function for the "human" formatting flag and an empty one for machine // readable format flags -func getPrintf(outputFlag string, writer io.Writer) ( - func(format string, ctx ...interface{}), - error, -) { - switch outputFlag { +func getPrintf(output string, writer io.Writer) (func(format string, ctx ...interface{}), error) { + switch output { case "human": return func(format string, ctx ...interface{}) { fmt.Fprintf(writer, format, ctx...) @@ -58,6 +55,6 @@ func getPrintf(outputFlag string, writer io.Writer) ( case "yaml", "json": return func(format string, ctx ...interface{}) {}, nil default: - return nil, serrors.New("format not supported", "format", outputFlag) + return nil, serrors.New("format not supported", "format", output) } } From 45e697eab5f75ca6a866d09ccaec840e961c6a6a Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Thu, 3 Nov 2022 10:05:17 +0100 Subject: [PATCH 18/23] sequence whitespace, some small fixes --- private/path/pathpol/sequence.go | 5 ++++- scion/cmd/scion/common.go | 16 +++++++++++++++- scion/cmd/scion/showpaths.go | 4 ++++ scion/showpaths/showpaths.go | 2 +- 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/private/path/pathpol/sequence.go b/private/path/pathpol/sequence.go index 876cf73e76..bf9ae6bcfe 100644 --- a/private/path/pathpol/sequence.go +++ b/private/path/pathpol/sequence.go @@ -84,6 +84,9 @@ func (s *Sequence) Eval(paths []snet.Path) []snet.Path { result := []snet.Path{} for _, path := range paths { desc, err := GetSequence(path) + if desc != "" { + desc = desc + " " + } if err != nil { log.Error("get sequence from path", "err", err) continue @@ -322,7 +325,7 @@ func GetSequence(path snet.Path) (string, error) { hops = append(hops, hop(ifaces[i].IA, ifaces[i].ID, ifaces[i+1].ID)) } hops = append(hops, hop(ifaces[len(ifaces)-1].IA, ifaces[len(ifaces)-1].ID, 0)) - desc = strings.Join(hops, " ") + " " + desc = strings.Join(hops, " ") } return desc, nil } diff --git a/scion/cmd/scion/common.go b/scion/cmd/scion/common.go index 3a49454d44..89246babb5 100644 --- a/scion/cmd/scion/common.go +++ b/scion/cmd/scion/common.go @@ -1,3 +1,17 @@ +// Copyright 2022 Anapaya Systems +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package main import ( @@ -30,7 +44,7 @@ type Hop struct { IA addr.IA `json:"isd_as" yaml:"isd_as"` } -// GetHops constructs a list of snet path interfaces from an snet path +// getHops constructs a list of snet path interfaces from an snet path func getHops(path snet.Path) []Hop { ifaces := path.Metadata().Interfaces var hops []Hop diff --git a/scion/cmd/scion/showpaths.go b/scion/cmd/scion/showpaths.go index 814c9f702b..f531ed9e9c 100644 --- a/scion/cmd/scion/showpaths.go +++ b/scion/cmd/scion/showpaths.go @@ -82,6 +82,10 @@ On other errors, showpaths will exit with code 2. return serrors.WrapStr("setting up tracing", err) } defer closer() + + if flags.json && !cmd.Flags().Lookup("format").Changed { + flags.format = "json" + } printf, err := getPrintf(flags.format, cmd.OutOrStdout()) if err != nil { return serrors.WrapStr("get formatting", err) diff --git a/scion/showpaths/showpaths.go b/scion/showpaths/showpaths.go index 40356ec74b..25475696db 100644 --- a/scion/showpaths/showpaths.go +++ b/scion/showpaths/showpaths.go @@ -47,7 +47,7 @@ type Path struct { FullPath snet.Path `json:"-" yaml:"-"` Fingerprint string `json:"fingerprint" yaml:"fingerprint"` Hops []Hop `json:"hops" yaml:"hops"` - Sequence string `json:"hops_sequence" yaml:"hops_sequence"` + Sequence string `json:"sequence" yaml:"sequence"` NextHop string `json:"next_hop" yaml:"next_hop"` Expiry time.Time `json:"expiry" yaml:"expiry"` MTU uint16 `json:"mtu" yaml:"mtu"` From 13e007aef812126042780400e61ae452fd6cf046 Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Mon, 7 Nov 2022 10:30:10 +0100 Subject: [PATCH 19/23] time duration as floats --- scion/cmd/scion/ping.go | 44 +++++++++++++++++------------------ scion/cmd/scion/traceroute.go | 12 ++++++---- 2 files changed, 30 insertions(+), 26 deletions(-) diff --git a/scion/cmd/scion/ping.go b/scion/cmd/scion/ping.go index cc1409529d..63289473f9 100644 --- a/scion/cmd/scion/ping.go +++ b/scion/cmd/scion/ping.go @@ -52,20 +52,20 @@ type Result struct { type Stats struct { ping.Stats - Loss int `json:"packet_loss" yaml:"packet_loss"` - Time time.Duration `json:"time" yaml:"time"` - MinRTT time.Duration `json:"min_rtt" yaml:"min_rtt"` - AvgRTT time.Duration `json:"avg_rtt" yaml:"avg_rtt"` - MaxRTT time.Duration `json:"max_rtt" yaml:"max_rtt"` - MdevRTT time.Duration `json:"mdev_rtt" yaml:"mdev_rtt"` + Loss int `json:"packet_loss" yaml:"packet_loss"` + Time float64 `json:"time" yaml:"time"` + MinRTT float64 `json:"min_rtt" yaml:"min_rtt"` + AvgRTT float64 `json:"avg_rtt" yaml:"avg_rtt"` + MaxRTT float64 `json:"max_rtt" yaml:"max_rtt"` + MdevRTT float64 `json:"mdev_rtt" yaml:"mdev_rtt"` } type PingUpdate struct { - Size int `json:"scion_packet_size" yaml:"scion_packet_size"` - Source string `json:"source_isd_as" yaml:"source_isd_as"` - Sequence int `json:"scmp_seq" yaml:"scmp_seq"` - RTT time.Duration `json:"round_trip_time" yaml:"round_trip_time"` - State string `json:"state" yaml:"state"` + Size int `json:"scion_packet_size" yaml:"scion_packet_size"` + Source string `json:"source_isd_as" yaml:"source_isd_as"` + Sequence int `json:"scmp_seq" yaml:"scmp_seq"` + RTT float64 `json:"round_trip_time" yaml:"round_trip_time"` + State string `json:"state" yaml:"state"` } func newPing(pather CommandPather) *cobra.Command { @@ -289,7 +289,7 @@ On other errors, ping will exit with code 2. Size: update.Size, Source: update.Source.String(), Sequence: update.Sequence, - RTT: update.RTT, + RTT: float64(update.RTT.Nanoseconds()) / 1e6, State: update.State.String(), }) printf("%d bytes from %s,%s: scmp_seq=%d time=%s%s\n", @@ -308,14 +308,14 @@ On other errors, ping will exit with code 2. printf("\n--- %s,%s statistics ---\n", remote.IA, remote.Host.IP) printf("%d packets transmitted, %d received, %d%% packet loss, time %v\n", s.Sent, s.Received, res.Statistics.Loss, - res.Statistics.Time.Round(time.Microsecond), + time.Duration(res.Statistics.Time)*time.Millisecond, ) if s.Received != 0 { printf("rtt min/avg/max/mdev = %.3f/%.3f/%.3f/%.3f ms\n", - float64(res.Statistics.MinRTT.Nanoseconds())/1e6, - float64(res.Statistics.AvgRTT.Nanoseconds())/1e6, - float64(res.Statistics.MaxRTT.Nanoseconds())/1e6, - float64(res.Statistics.MdevRTT.Nanoseconds())/1e6, + res.Statistics.MinRTT, + res.Statistics.AvgRTT, + res.Statistics.MaxRTT, + res.Statistics.MdevRTT, ) } if stats.Received == 0 { @@ -384,7 +384,7 @@ func calculateStats(s ping.Stats, replies []PingUpdate, run time.Duration) Stats stats := Stats{ Stats: s, Loss: loss, - Time: run, + Time: math.Round((float64(run.Nanoseconds())/1e6)*1000) / 1000, } if len(replies) == 0 { @@ -392,7 +392,7 @@ func calculateStats(s ping.Stats, replies []PingUpdate, run time.Duration) Stats } stats.MinRTT = replies[0].RTT stats.MaxRTT = replies[0].RTT - var sum time.Duration + var sum float64 for i := 0; i < len(replies); i++ { if replies[i].RTT < stats.MinRTT { stats.MinRTT = replies[i].RTT @@ -402,14 +402,14 @@ func calculateStats(s ping.Stats, replies []PingUpdate, run time.Duration) Stats } sum += replies[i].RTT } - stats.AvgRTT = time.Duration(int(sum.Nanoseconds()) / len(replies)) + stats.AvgRTT = math.Round(sum/float64(len(replies))*1000) / 1000 // standard deviation var sd float64 for i := 0; i < len(replies); i++ { - sd += math.Pow(float64(replies[i].RTT)-float64(stats.AvgRTT), 2) + sd += math.Pow(replies[i].RTT-stats.AvgRTT, 2) } - stats.MdevRTT = time.Duration(math.Sqrt(sd / float64(len(replies)))) + stats.MdevRTT = math.Round(math.Sqrt(sd/float64(len(replies)))*1000) / 1000 return stats } diff --git a/scion/cmd/scion/traceroute.go b/scion/cmd/scion/traceroute.go index 0602d15afa..9a6de81d3a 100644 --- a/scion/cmd/scion/traceroute.go +++ b/scion/cmd/scion/traceroute.go @@ -51,9 +51,9 @@ type ResultTraceroute struct { type HopInfo struct { InterfaceID uint16 `json:"interface_id" yaml:"interface_id"` // IP address of the router responding to the traceroute request. - IP string `json:"ip" yaml:"ip"` - IA addr.IA `json:"isd_as" yaml:"isd_as"` - RoundTripTimes []time.Duration `json:"round_trip_times" yaml:"round_trip_times"` + IP string `json:"ip" yaml:"ip"` + IA addr.IA `json:"isd_as" yaml:"isd_as"` + RoundTripTimes []float64 `json:"round_trip_times" yaml:"round_trip_times"` } func newTraceroute(pather CommandPather) *cobra.Command { @@ -205,11 +205,15 @@ On other errors, traceroute will exit with code 2. } res.Hops = make([]HopInfo, 0, len(updates)) for _, update := range updates { + RTTs := make([]float64, 0, len(update.RTTs)) + for _, rtt := range update.RTTs { + RTTs = append(RTTs, float64(rtt.Nanoseconds())/1e6) + } res.Hops = append(res.Hops, HopInfo{ InterfaceID: uint16(update.Interface), IP: update.Remote.Host.IP().String(), IA: update.Remote.IA, - RoundTripTimes: update.RTTs, + RoundTripTimes: RTTs, }) } From a501c6ae1dbfca1644e9769ea407920b53f1ad5d Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Thu, 10 Nov 2022 19:06:41 +0100 Subject: [PATCH 20/23] feedback milli handling --- scion/cmd/scion/common.go | 28 +++++++++++++++ scion/cmd/scion/ping.go | 65 ++++++++++++++++++----------------- scion/cmd/scion/traceroute.go | 12 +++---- 3 files changed, 68 insertions(+), 37 deletions(-) diff --git a/scion/cmd/scion/common.go b/scion/cmd/scion/common.go index 89246babb5..1bd4abe356 100644 --- a/scion/cmd/scion/common.go +++ b/scion/cmd/scion/common.go @@ -15,9 +15,12 @@ package main import ( + "encoding/json" "fmt" "io" + "math" "net" + "time" "github.com/scionproto/scion/pkg/addr" "github.com/scionproto/scion/pkg/private/common" @@ -72,3 +75,28 @@ func getPrintf(output string, writer io.Writer) (func(format string, ctx ...inte return nil, serrors.New("format not supported", "format", output) } } + +type durationMillis time.Duration + +func (d durationMillis) String() string { + return fmt.Sprintf("%.3fms", d.Millis()) +} + +func (d durationMillis) MarshalJSON() ([]byte, error) { + return json.Marshal(d.MillisRounded()) +} + +func (d durationMillis) MarshalYAML() (interface{}, error) { + return d.MillisRounded(), nil +} + +// millis returns the duration as a floating point number of milliseconds +func (d durationMillis) Millis() float64 { + return float64(d) / 1e6 +} + +// millisRounded returns the duration as a floating point number of +// milliseconds, rounded to microseconds (3 digits precision). +func (d durationMillis) MillisRounded() float64 { + return math.Round(float64(d)/1000) / 1000 +} diff --git a/scion/cmd/scion/ping.go b/scion/cmd/scion/ping.go index 63289473f9..43bdb7547a 100644 --- a/scion/cmd/scion/ping.go +++ b/scion/cmd/scion/ping.go @@ -51,21 +51,21 @@ type Result struct { } type Stats struct { - ping.Stats - Loss int `json:"packet_loss" yaml:"packet_loss"` - Time float64 `json:"time" yaml:"time"` - MinRTT float64 `json:"min_rtt" yaml:"min_rtt"` - AvgRTT float64 `json:"avg_rtt" yaml:"avg_rtt"` - MaxRTT float64 `json:"max_rtt" yaml:"max_rtt"` - MdevRTT float64 `json:"mdev_rtt" yaml:"mdev_rtt"` + ping.Stats `yaml:",inline"` + Loss int `json:"packet_loss" yaml:"packet_loss"` + Time durationMillis `json:"time" yaml:"time"` + MinRTT durationMillis `json:"min_rtt" yaml:"min_rtt"` + AvgRTT durationMillis `json:"avg_rtt" yaml:"avg_rtt"` + MaxRTT durationMillis `json:"max_rtt" yaml:"max_rtt"` + MdevRTT durationMillis `json:"mdev_rtt" yaml:"mdev_rtt"` } type PingUpdate struct { - Size int `json:"scion_packet_size" yaml:"scion_packet_size"` - Source string `json:"source_isd_as" yaml:"source_isd_as"` - Sequence int `json:"scmp_seq" yaml:"scmp_seq"` - RTT float64 `json:"round_trip_time" yaml:"round_trip_time"` - State string `json:"state" yaml:"state"` + Size int `json:"scion_packet_size" yaml:"scion_packet_size"` + Source string `json:"source" yaml:"source"` + Sequence int `json:"scmp_seq" yaml:"scmp_seq"` + RTT durationMillis `json:"round_trip_time" yaml:"round_trip_time"` + State string `json:"state" yaml:"state"` } func newPing(pather CommandPather) *cobra.Command { @@ -289,12 +289,12 @@ On other errors, ping will exit with code 2. Size: update.Size, Source: update.Source.String(), Sequence: update.Sequence, - RTT: float64(update.RTT.Nanoseconds()) / 1e6, + RTT: durationMillis(update.RTT), State: update.State.String(), }) printf("%d bytes from %s,%s: scmp_seq=%d time=%s%s\n", update.Size, update.Source.IA, update.Source.Host, update.Sequence, - fmt.Sprintf("%.3fms", float64(update.RTT.Nanoseconds())/1e6), additional) + durationMillis(update.RTT), additional) }, }) if err != nil { @@ -308,14 +308,14 @@ On other errors, ping will exit with code 2. printf("\n--- %s,%s statistics ---\n", remote.IA, remote.Host.IP) printf("%d packets transmitted, %d received, %d%% packet loss, time %v\n", s.Sent, s.Received, res.Statistics.Loss, - time.Duration(res.Statistics.Time)*time.Millisecond, + res.Statistics.Time, ) if s.Received != 0 { printf("rtt min/avg/max/mdev = %.3f/%.3f/%.3f/%.3f ms\n", - res.Statistics.MinRTT, - res.Statistics.AvgRTT, - res.Statistics.MaxRTT, - res.Statistics.MdevRTT, + res.Statistics.MinRTT.Millis(), + res.Statistics.AvgRTT.Millis(), + res.Statistics.MaxRTT.Millis(), + res.Statistics.MdevRTT.Millis(), ) } if stats.Received == 0 { @@ -384,32 +384,35 @@ func calculateStats(s ping.Stats, replies []PingUpdate, run time.Duration) Stats stats := Stats{ Stats: s, Loss: loss, - Time: math.Round((float64(run.Nanoseconds())/1e6)*1000) / 1000, + Time: durationMillis(run), } if len(replies) == 0 { return stats } - stats.MinRTT = replies[0].RTT - stats.MaxRTT = replies[0].RTT - var sum float64 + minRTT := replies[0].RTT + maxRTT := replies[0].RTT + var sum durationMillis for i := 0; i < len(replies); i++ { - if replies[i].RTT < stats.MinRTT { - stats.MinRTT = replies[i].RTT + if replies[i].RTT < minRTT { + minRTT = replies[i].RTT } - if replies[i].RTT > stats.MaxRTT { - stats.MaxRTT = replies[i].RTT + if replies[i].RTT > maxRTT { + maxRTT = replies[i].RTT } sum += replies[i].RTT } - stats.AvgRTT = math.Round(sum/float64(len(replies))*1000) / 1000 + avgRTT := durationMillis(int(sum) / len(replies)) // standard deviation var sd float64 for i := 0; i < len(replies); i++ { - sd += math.Pow(replies[i].RTT-stats.AvgRTT, 2) + sd += math.Pow(float64(replies[i].RTT-avgRTT), 2) } - stats.MdevRTT = math.Round(math.Sqrt(sd/float64(len(replies)))*1000) / 1000 - + mdevRTT := math.Sqrt(sd / float64(len(replies))) + stats.MinRTT = durationMillis(minRTT) + stats.MaxRTT = durationMillis(maxRTT) + stats.AvgRTT = durationMillis(avgRTT) + stats.MdevRTT = durationMillis(mdevRTT) return stats } diff --git a/scion/cmd/scion/traceroute.go b/scion/cmd/scion/traceroute.go index 9a6de81d3a..3ebeabea8a 100644 --- a/scion/cmd/scion/traceroute.go +++ b/scion/cmd/scion/traceroute.go @@ -51,9 +51,9 @@ type ResultTraceroute struct { type HopInfo struct { InterfaceID uint16 `json:"interface_id" yaml:"interface_id"` // IP address of the router responding to the traceroute request. - IP string `json:"ip" yaml:"ip"` - IA addr.IA `json:"isd_as" yaml:"isd_as"` - RoundTripTimes []float64 `json:"round_trip_times" yaml:"round_trip_times"` + IP string `json:"ip" yaml:"ip"` + IA addr.IA `json:"isd_as" yaml:"isd_as"` + RoundTripTimes []durationMillis `json:"round_trip_times" yaml:"round_trip_times"` } func newTraceroute(pather CommandPather) *cobra.Command { @@ -205,9 +205,9 @@ On other errors, traceroute will exit with code 2. } res.Hops = make([]HopInfo, 0, len(updates)) for _, update := range updates { - RTTs := make([]float64, 0, len(update.RTTs)) + RTTs := make([]durationMillis, 0, len(update.RTTs)) for _, rtt := range update.RTTs { - RTTs = append(RTTs, float64(rtt.Nanoseconds())/1e6) + RTTs = append(RTTs, durationMillis(rtt)) } res.Hops = append(res.Hops, HopInfo{ InterfaceID: uint16(update.Interface), @@ -256,7 +256,7 @@ func fmtRTTs(rtts []time.Duration, timeout time.Duration) string { parts = append(parts, "*") continue } - parts = append(parts, fmt.Sprintf("%.3fms", float64(rtt.Nanoseconds())/1e6)) + parts = append(parts, durationMillis(rtt).String()) } return strings.Join(parts, " ") } From a239a37ccb43d7851a90ef9c9d3048494925b38e Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Fri, 11 Nov 2022 08:27:41 +0100 Subject: [PATCH 21/23] unnecessary conversion --- scion/cmd/scion/ping.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scion/cmd/scion/ping.go b/scion/cmd/scion/ping.go index 43bdb7547a..065f4fd28a 100644 --- a/scion/cmd/scion/ping.go +++ b/scion/cmd/scion/ping.go @@ -410,9 +410,9 @@ func calculateStats(s ping.Stats, replies []PingUpdate, run time.Duration) Stats sd += math.Pow(float64(replies[i].RTT-avgRTT), 2) } mdevRTT := math.Sqrt(sd / float64(len(replies))) - stats.MinRTT = durationMillis(minRTT) - stats.MaxRTT = durationMillis(maxRTT) - stats.AvgRTT = durationMillis(avgRTT) + stats.MinRTT = minRTT + stats.MaxRTT = maxRTT + stats.AvgRTT = avgRTT stats.MdevRTT = durationMillis(mdevRTT) return stats } From e269b56e8eb29b6d495ef6a1135aca47da5eca7e Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Fri, 11 Nov 2022 14:08:43 +0100 Subject: [PATCH 22/23] traceroute timeout fix --- scion/cmd/scion/traceroute.go | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/scion/cmd/scion/traceroute.go b/scion/cmd/scion/traceroute.go index 3ebeabea8a..228bb37733 100644 --- a/scion/cmd/scion/traceroute.go +++ b/scion/cmd/scion/traceroute.go @@ -204,17 +204,9 @@ On other errors, traceroute will exit with code 2. return err } res.Hops = make([]HopInfo, 0, len(updates)) - for _, update := range updates { - RTTs := make([]durationMillis, 0, len(update.RTTs)) - for _, rtt := range update.RTTs { - RTTs = append(RTTs, durationMillis(rtt)) - } - res.Hops = append(res.Hops, HopInfo{ - InterfaceID: uint16(update.Interface), - IP: update.Remote.Host.IP().String(), - IA: update.Remote.IA, - RoundTripTimes: RTTs, - }) + hops := getHops(path) + for i, update := range updates { + res.Hops = append(res.Hops, getHopInfo(update, hops[i])) } switch flags.format { @@ -262,5 +254,24 @@ func fmtRTTs(rtts []time.Duration, timeout time.Duration) string { } func fmtRemote(remote snet.SCIONAddress, intf uint64) string { + if remote == (snet.SCIONAddress{}) { + return "??" + } return fmt.Sprintf("%s IfID=%d", remote, intf) } + +func getHopInfo(u traceroute.Update, hop Hop) HopInfo { + if u.Remote == (snet.SCIONAddress{}) { + return HopInfo{IA: hop.IA, InterfaceID: uint16(hop.ID)} + } + RTTs := make([]durationMillis, 0, len(u.RTTs)) + for _, rtt := range u.RTTs { + RTTs = append(RTTs, durationMillis(rtt)) + } + return HopInfo{ + InterfaceID: uint16(u.Interface), + IP: u.Remote.Host.IP().String(), + IA: u.Remote.IA, + RoundTripTimes: RTTs, + } +} From 270db5a3ddee23f9edb609b703e259e0b329cdcb Mon Sep 17 00:00:00 2001 From: Tobias Buner Date: Mon, 28 Nov 2022 11:11:42 +0100 Subject: [PATCH 23/23] updated command docs --- doc/command/scion/scion_ping.rst | 3 ++- doc/command/scion/scion_showpaths.rst | 4 +--- doc/command/scion/scion_traceroute.rst | 1 + 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/command/scion/scion_ping.rst b/doc/command/scion/scion_ping.rst index 0241d148d7..f79691cf0c 100644 --- a/doc/command/scion/scion_ping.rst +++ b/doc/command/scion/scion_ping.rst @@ -87,6 +87,7 @@ Options -c, --count uint16 total number of packets to send --dispatcher string Path to the dispatcher socket (default "/run/shm/dispatcher/default.sock") --epic Enable EPIC for path probing. + --format string Specify the output format (human|json|yaml) (default "human") --healthy-only only use healthy paths -h, --help help for ping -i, --interactive interactive mode @@ -99,7 +100,7 @@ Options 'payload_size' and 'packet_size' flags. --no-color disable colored output --packet-size uint number of bytes to be sent including the SCION Header and SCMP echo header, - the desired size must provide enough space for the required headers. This flag + the desired size must provide enough space for the required headers. This flag overrides the 'payload_size' flag. -s, --payload-size uint number of bytes to be sent in addition to the SCION Header and SCMP echo header; the total size of the packet is still variable size due to the variable size of diff --git a/doc/command/scion/scion_showpaths.rst b/doc/command/scion/scion_showpaths.rst index e324d753d1..9801251fe0 100644 --- a/doc/command/scion/scion_showpaths.rst +++ b/doc/command/scion/scion_showpaths.rst @@ -16,8 +16,6 @@ By default, the paths are probed. Paths served from the SCION Deamon's might not forward traffic successfully (e.g. if a network link went down, or there is a black hole on the path). To disable path probing, set the appropriate flag. -'showpaths' can be instructed to output the paths as json using the the \--json flag. - If no alive path is discovered, json output is not enabled, and probing is not disabled, showpaths will exit with the code 1. On other errors, showpaths will exit with code 2. @@ -93,9 +91,9 @@ Options --dispatcher string Path to the dispatcher socket (default "/run/shm/dispatcher/default.sock") --epic Enable EPIC. -e, --extended Show extended path meta data information + --format string Specify the output format (human|json|yaml) (default "human") -h, --help help for showpaths --isd-as isd-as The local ISD-AS to use. (default 0-0) - -j, --json Write the output as machine readable json -l, --local ip Local IP address to listen on. (default zero IP) --log.level string Console logging level verbosity (debug|info|error) -m, --maxpaths int Maximum number of paths that are displayed (default 10) diff --git a/doc/command/scion/scion_traceroute.rst b/doc/command/scion/scion_traceroute.rst index b6f9ca2e6a..67ea2fb5d0 100644 --- a/doc/command/scion/scion_traceroute.rst +++ b/doc/command/scion/scion_traceroute.rst @@ -79,6 +79,7 @@ Options --dispatcher string Path to the dispatcher socket (default "/run/shm/dispatcher/default.sock") --epic Enable EPIC. + --format string Specify the output format (human|json|yaml) (default "human") -h, --help help for traceroute -i, --interactive interactive mode --isd-as isd-as The local ISD-AS to use. (default 0-0)