From 70dbe4609b2ff25a024bba95093835fa09a17316 Mon Sep 17 00:00:00 2001 From: Sergio Gonzalez Monroy Date: Tue, 24 Sep 2019 09:23:10 +0200 Subject: [PATCH] comments r3 --- go/border/ifstate/ifstate.go | 2 +- go/border/internal/metrics/ctrl.go | 31 ++++++++++++++++++++------- go/border/internal/metrics/input.go | 6 +++--- go/border/internal/metrics/metrics.go | 14 ++++++------ go/border/internal/metrics/output.go | 18 ++++++++-------- go/border/internal/metrics/process.go | 16 ++++++-------- go/border/io.go | 9 ++++---- go/border/io_test.go | 2 +- go/border/rctrl/ctrl.go | 6 +++--- go/border/rctrl/ifstate.go | 15 +++++++------ go/border/rctrl/revinfo.go | 12 +++++------ go/border/rctx/io.go | 23 +++++++++++--------- go/border/router.go | 3 ++- go/border/setup-posix.go | 12 +++++------ 14 files changed, 96 insertions(+), 73 deletions(-) diff --git a/go/border/ifstate/ifstate.go b/go/border/ifstate/ifstate.go index f63f2a1c70..d3f46fe6d3 100644 --- a/go/border/ifstate/ifstate.go +++ b/go/border/ifstate/ifstate.go @@ -56,7 +56,7 @@ func (s *ifStates) Store(key common.IFIDType, val *state) { var states ifStates type state struct { - // info is a pointer to an Info object. Processing gorouting can update this value. + // info is a pointer to an Info object. Processing goroutine can update this value. info unsafe.Pointer } diff --git a/go/border/internal/metrics/ctrl.go b/go/border/internal/metrics/ctrl.go index a82edc56f6..c9ce49c732 100644 --- a/go/border/internal/metrics/ctrl.go +++ b/go/border/internal/metrics/ctrl.go @@ -22,7 +22,6 @@ import ( // Control type values const ( - // IFStateInfo = "ifstate_info" IFStateReq = "ifstate_request" Revocation = "revocation" @@ -46,26 +45,42 @@ func (l ControlLabels) Values() []string { } type control struct { - pkts *prometheus.CounterVec - ifstate *prometheus.GaugeVec + sentMsgs *prometheus.CounterVec + receivedMsgs *prometheus.CounterVec + ifstate *prometheus.GaugeVec + ifstateTick prometheus.Counter } func newControl() control { sub := "control" return control{ - pkts: prom.NewCounterVec(Namespace, sub, - "pkts_total", "Total number of processed packets.", ControlLabels{}.Labels()), + sentMsgs: prom.NewCounterVec(Namespace, sub, + "sent_msgs_total", "Total number of sent messages.", ControlLabels{}.Labels()), + receivedMsgs: prom.NewCounterVec(Namespace, sub, + "received_msgs_total", "Total number of recevied messages.", ControlLabels{}.Labels()), ifstate: prom.NewGaugeVec(Namespace, sub, "interface_active", "Interface is active.", IntfLabels{}.Labels()), + ifstateTick: prom.NewCounter(Namespace, sub, + "ifstate_ticks_total", "Total number of IFState requests ticks."), } } -// Pkts returns the counter for the given label set. -func (c *control) Pkts(l ControlLabels) prometheus.Counter { - return c.pkts.WithLabelValues(l.Values()...) +// SentMsgs returns the counter for the given label set. +func (c *control) SentMsgs(l ControlLabels) prometheus.Counter { + return c.sentMsgs.WithLabelValues(l.Values()...) +} + +// ReceivedMsgs returns the counter for the given label set. +func (c *control) ReceivedMsgs(l ControlLabels) prometheus.Counter { + return c.receivedMsgs.WithLabelValues(l.Values()...) } // IFState returns the gauge for the given label set. func (c *control) IFState(l IntfLabels) prometheus.Gauge { return c.ifstate.WithLabelValues(l.Values()...) } + +// IFStateTick returns the counter for the given label set. +func (c *control) IFStateTick() prometheus.Counter { + return c.ifstateTick +} diff --git a/go/border/internal/metrics/input.go b/go/border/internal/metrics/input.go index 103b5717f4..27d60ee607 100644 --- a/go/border/internal/metrics/input.go +++ b/go/border/internal/metrics/input.go @@ -38,9 +38,9 @@ func newInput() input { l := IntfLabels{}.Labels() return input{ pkts: prom.NewCounterVec(Namespace, sub, - "pkts_total", "Total number of input packets received.", l), + "pkts_total", "Total number of packets received.", l), bytes: prom.NewCounterVec(Namespace, sub, - "bytes_total", "Total number of input bytes received.", l), + "bytes_total", "Total number of bytes received.", l), pktSize: prom.NewHistogramVec(Namespace, sub, "pkt_size_bytes", "Size of input packets in bytes", l, []float64{64, 256, 512, 1024, 1280, 1500, 3000, 6000, 9000}), @@ -53,7 +53,7 @@ func newInput() input { "overflow_packets_total", "Total number of packets dropped by kernel due to receive buffer overflow.", l), latency: prom.NewCounterVec(Namespace, sub, - "latency_seconds_total", + "read_latency_seconds_total", "Total time packets wait in the kernel to be read, in seconds", l), } } diff --git a/go/border/internal/metrics/metrics.go b/go/border/internal/metrics/metrics.go index aa10ae4b92..d5f627b640 100644 --- a/go/border/internal/metrics/metrics.go +++ b/go/border/internal/metrics/metrics.go @@ -17,8 +17,6 @@ package metrics import ( - "fmt" - "github.com/scionproto/scion/go/lib/common" "github.com/scionproto/scion/go/lib/prom" ) @@ -43,6 +41,8 @@ const ( ErrProcessLocal = "err_process_local" // ErrParsePayload is an error parsing the packet payload. ErrParsePayload = "err_parse_payload" + // ErrResolveSVC is an error resolving a SVC address + ErrResolveSVC = "err_resolve_svc" ) // Metrics initialization. @@ -54,23 +54,25 @@ var ( ) type IntfLabels struct { - // Itnf in the interface ID + // Itnf is the interface ID Intf string + // NeighIA is the remote IA of a given interface. + NeighIA string } // Labels returns the list of labels. func (l IntfLabels) Labels() []string { - return []string{"intf"} + return []string{"intf", "neigh_ia"} } // Values returns the label values in the order defined by Labels. func (l IntfLabels) Values() []string { - return []string{l.Intf} + return []string{l.Intf, l.NeighIA} } func IntfToLabel(ifid common.IFIDType) string { if ifid == 0 { return "loc" } - return fmt.Sprintf("%d", ifid) + return ifid.String() } diff --git a/go/border/internal/metrics/output.go b/go/border/internal/metrics/output.go index 2e6a6ff3fd..0b66eaae5e 100644 --- a/go/border/internal/metrics/output.go +++ b/go/border/internal/metrics/output.go @@ -29,7 +29,7 @@ type output struct { // Low-level output stats writes *prometheus.CounterVec writeErrors *prometheus.CounterVec - latency *prometheus.CounterVec + duration *prometheus.CounterVec } func newOutput() output { @@ -37,9 +37,9 @@ func newOutput() output { l := IntfLabels{}.Labels() return output{ pkts: prom.NewCounterVec(Namespace, sub, - "pkts_total", "Total number of output packets sent.", l), + "pkts_total", "Total number of packets sent.", l), bytes: prom.NewCounterVec(Namespace, sub, - "bytes_total", "Total number of output bytes sent.", l), + "bytes_total", "Total number of bytes sent.", l), pktSize: prom.NewHistogramVec(Namespace, sub, "pkt_size_bytes", "Size of output packets in bytes", l, []float64{64, 256, 512, 1024, 1280, 1500, 3000, 6000, 9000}), @@ -48,9 +48,9 @@ func newOutput() output { "writes_total", "Total number of output socket writes.", l), writeErrors: prom.NewCounterVec(Namespace, sub, "write_errors_total", "Total number of output socket write errors.", l), - latency: prom.NewCounterVec(Namespace, sub, - "latency_seconds_total", - "Total time packets wait in the kernel to be write, in seconds", l), + duration: prom.NewCounterVec(Namespace, sub, + "write_duration_seconds_total", + "Total time spent writing packets to the kernel, in seconds.", l), } } @@ -79,7 +79,7 @@ func (o *output) WriteErrors(l IntfLabels) prometheus.Counter { return o.writeErrors.WithLabelValues(l.Values()...) } -// Latency returns the counter for the given label set. -func (o *output) Latency(l IntfLabels) prometheus.Counter { - return o.latency.WithLabelValues(l.Values()...) +// Duration returns the counter for the given label set. +func (o *output) Duration(l IntfLabels) prometheus.Counter { + return o.duration.WithLabelValues(l.Values()...) } diff --git a/go/border/internal/metrics/process.go b/go/border/internal/metrics/process.go index c732d07e9c..d091c4f5ae 100644 --- a/go/border/internal/metrics/process.go +++ b/go/border/internal/metrics/process.go @@ -23,8 +23,6 @@ import ( const ( // Drop is the output interface for packets with errors that are dropped. Drop = "drop" - // Ctrl is the output interface for packets sent to control. - Ctrl = "control" ) type ProcessLabels struct { @@ -47,8 +45,8 @@ func (l ProcessLabels) Values() []string { } type process struct { - pkts *prometheus.CounterVec - time *prometheus.CounterVec + pkts *prometheus.CounterVec + duration *prometheus.CounterVec } func newProcess() process { @@ -56,8 +54,8 @@ func newProcess() process { return process{ pkts: prom.NewCounterVec(Namespace, sub, "pkts_total", "Total number of processed packets.", ProcessLabels{}.Labels()), - time: prom.NewCounterVec(Namespace, sub, - "time_seconds_total", "Total packet processing time.", IntfLabels{}.Labels()), + duration: prom.NewCounterVec(Namespace, sub, + "duration_seconds_total", "Total packet processing duration.", IntfLabels{}.Labels()), } } @@ -66,7 +64,7 @@ func (p *process) Pkts(l ProcessLabels) prometheus.Counter { return p.pkts.WithLabelValues(l.Values()...) } -// Time returns the counter for the given label set. -func (p *process) Time(l IntfLabels) prometheus.Counter { - return p.time.WithLabelValues(l.Values()...) +// Duration returns the counter for the given label set. +func (p *process) Duration(l IntfLabels) prometheus.Counter { + return p.duration.WithLabelValues(l.Values()...) } diff --git a/go/border/io.go b/go/border/io.go index b1851af5dd..2e84b6cb61 100644 --- a/go/border/io.go +++ b/go/border/io.go @@ -59,7 +59,7 @@ func (r *Router) posixInput(s *rctx.Sock, stop, stopped chan struct{}) { readMetas := make([]conn.ReadMeta, inputBatchCnt) var err error - l := metrics.IntfLabels{Intf: s.Label} + l := metrics.IntfLabels{Intf: s.Label, NeighIA: s.NeighIA} // Pre-calculate metrics inputPkts := metrics.Input.Pkts(l) inputBytes := metrics.Input.Bytes(l) @@ -68,10 +68,11 @@ func (r *Router) posixInput(s *rctx.Sock, stop, stopped chan struct{}) { inputReadErrs := metrics.Input.ReadErrors(l) inputRcvOvfl := metrics.Input.RcvOvfl(l) inputLatency := metrics.Input.Latency(l) + procPktTime := metrics.Process.Duration(l) // Called when the packet's reference count hits 0. free := func(rp *rpkt.RtrPkt) { - metrics.Process.Time(l).Add(time.Since(rp.TimeIn).Seconds()) + procPktTime.Add(time.Since(rp.TimeIn).Seconds()) rp.Reset() r.freePkts.Write(ringbuf.EntryList{rp}, true) } @@ -201,13 +202,13 @@ func (r *Router) posixOutput(s *rctx.Sock, _, stopped chan struct{}) { msgs := conn.NewWriteMessages(outputBatchCnt) // Pre-calculate metrics - l := metrics.IntfLabels{Intf: s.Label} + l := metrics.IntfLabels{Intf: s.Label, NeighIA: s.NeighIA} outputPkts := metrics.Output.Pkts(l) outputBytes := metrics.Output.Bytes(l) outputPktSize := metrics.Output.PktSize(l) outputWrites := metrics.Output.Writes(l) outputWriteErrs := metrics.Output.WriteErrors(l) - outputWriteLatency := metrics.Output.Latency(l) + outputWriteLatency := metrics.Output.Duration(l) // This loop is exited in two cases: // 1. When the the ring is closed and fully drained. diff --git a/go/border/io_test.go b/go/border/io_test.go index 27c6f8c1ec..3775acf0ff 100644 --- a/go/border/io_test.go +++ b/go/border/io_test.go @@ -157,7 +157,7 @@ func newTestDst(t *testing.T) *overlay.OverlayAddr { } func newTestSock(r *Router, ringSize int, mconn conn.Conn) *rctx.Sock { - return rctx.NewSock(ringbuf.New(ringSize, nil, "loc_out"), mconn, 0, 12, + return rctx.NewSock(ringbuf.New(ringSize, nil, "loc_out"), mconn, 0, 12, "neigh_ia", nil, r.posixOutput, PosixSock) } diff --git a/go/border/rctrl/ctrl.go b/go/border/rctrl/ctrl.go index 03712015d0..729d09818e 100644 --- a/go/border/rctrl/ctrl.go +++ b/go/border/rctrl/ctrl.go @@ -82,18 +82,18 @@ func processCtrl() { b := make(common.RawBytes, maxBufSize) cl := metrics.ControlLabels{Type: metrics.IFStateInfo} for { + cl.Result = metrics.Success pktLen, src, err := snetConn.ReadFromSCION(b) if err != nil { cl.Result = metrics.ErrRead - metrics.Control.Pkts(cl).Inc() + metrics.Control.ReceivedMsgs(cl).Inc() fatal.Fatal(common.NewBasicError("Reading packet", err)) } - cl.Result = metrics.Success if err = processCtrlFromRaw(b[:pktLen]); err != nil { logger.Error("Processing ctrl pld", "src", src, "err", err) cl.Result = metrics.ErrParse } - metrics.Control.Pkts(cl).Inc() + metrics.Control.ReceivedMsgs(cl).Inc() } } diff --git a/go/border/rctrl/ifstate.go b/go/border/rctrl/ifstate.go index 8337b5093a..001e97e2f9 100644 --- a/go/border/rctrl/ifstate.go +++ b/go/border/rctrl/ifstate.go @@ -44,7 +44,9 @@ func ifStateUpdate() { if err := genIFStateReq(); err != nil { logger.Error(err.Error(), nil) } + metrics.Control.IFStateTick().Inc() for range time.Tick(ifStateFreq) { + metrics.Control.IFStateTick().Inc() if err := genIFStateReq(); err != nil { logger.Error(err.Error(), nil) } @@ -59,17 +61,17 @@ func genIFStateReq() error { } cpld, err := ctrl.NewPathMgmtPld(&path_mgmt.IFStateReq{}, nil, nil) if err != nil { - metrics.Control.Pkts(cl).Inc() + metrics.Control.SentMsgs(cl).Inc() return common.NewBasicError("Generating IFStateReq Ctrl payload", err) } scpld, err := cpld.SignedPld(infra.NullSigner) if err != nil { - metrics.Control.Pkts(cl).Inc() + metrics.Control.SentMsgs(cl).Inc() return common.NewBasicError("Generating IFStateReq signed Ctrl payload", err) } pld, err := scpld.PackPld() if err != nil { - metrics.Control.Pkts(cl).Inc() + metrics.Control.SentMsgs(cl).Inc() return common.NewBasicError("Writing IFStateReq signed Ctrl payload", err) } dst := &snet.Addr{ @@ -78,7 +80,8 @@ func genIFStateReq() error { } bsAddrs, err := rctx.Get().ResolveSVCMulti(addr.SvcBS) if err != nil { - metrics.Control.Pkts(cl).Inc() + cl.Result = metrics.ErrResolveSVC + metrics.Control.SentMsgs(cl).Inc() return common.NewBasicError("Resolving SVC BS multicast", err) } @@ -87,13 +90,13 @@ func genIFStateReq() error { dst.NextHop = addr if _, err := snetConn.WriteToSCION(pld, dst); err != nil { cl.Result = metrics.ErrWrite - metrics.Control.Pkts(cl).Inc() + metrics.Control.SentMsgs(cl).Inc() errors = append(errors, common.NewBasicError("Writing IFStateReq", err, "dst", dst)) continue } logger.Debug("Sent IFStateReq", "dst", dst, "overlayDst", addr) cl.Result = metrics.Success - metrics.Control.Pkts(cl).Inc() + metrics.Control.SentMsgs(cl).Inc() } return errors.ToError() } diff --git a/go/border/rctrl/revinfo.go b/go/border/rctrl/revinfo.go index f9f1b10d28..7cfcd513a1 100644 --- a/go/border/rctrl/revinfo.go +++ b/go/border/rctrl/revinfo.go @@ -51,19 +51,19 @@ func fwdRevInfo(sRevInfo *path_mgmt.SignedRevInfo, dstHost addr.HostSVC) { ctx := rctx.Get() cpld, err := ctrl.NewPathMgmtPld(sRevInfo, nil, nil) if err != nil { - metrics.Control.Pkts(cl).Inc() + metrics.Control.SentMsgs(cl).Inc() log.Error("Error generating RevInfo Ctrl payload", "err", err) return } scpld, err := cpld.SignedPld(infra.NullSigner) if err != nil { - metrics.Control.Pkts(cl).Inc() + metrics.Control.SentMsgs(cl).Inc() log.Error("Error generating RevInfo signed Ctrl payload", "err", err) return } pld, err := scpld.PackPld() if err != nil { - metrics.Control.Pkts(cl).Inc() + metrics.Control.SentMsgs(cl).Inc() logger.Error("Writing RevInfo signed Ctrl payload", "err", err) return } @@ -73,17 +73,17 @@ func fwdRevInfo(sRevInfo *path_mgmt.SignedRevInfo, dstHost addr.HostSVC) { } dst.NextHop, err = ctx.ResolveSVCAny(dstHost) if err != nil { - metrics.Control.Pkts(cl).Inc() + metrics.Control.SentMsgs(cl).Inc() logger.Error("Resolving SVC anycast", "err", err, "addr", dst) return } if _, err := snetConn.WriteToSCION(pld, dst); err != nil { cl.Result = metrics.ErrWrite - metrics.Control.Pkts(cl).Inc() + metrics.Control.SentMsgs(cl).Inc() logger.Error("Writing RevInfo", "dst", dst, "err", err) return } cl.Result = metrics.Success - metrics.Control.Pkts(cl).Inc() + metrics.Control.SentMsgs(cl).Inc() logger.Debug("Sent RevInfo", "dst", dst, "overlayDst", dst.NextHop) } diff --git a/go/border/rctx/io.go b/go/border/rctx/io.go index d51994fb1f..a10471ee53 100644 --- a/go/border/rctx/io.go +++ b/go/border/rctx/io.go @@ -45,6 +45,8 @@ type Sock struct { Ifid common.IFIDType // Label is the interface label Label string + // NeighIA is the interface remote IA + NeighIA string // Reader is an optional function that reads from Sock.Ring. It is spawned // in a go routine when Sock.Start() is called. Reader SockFunc @@ -61,18 +63,19 @@ type Sock struct { } func NewSock(ring *ringbuf.Ring, conn conn.Conn, dir rcmn.Dir, ifid common.IFIDType, - reader, writer SockFunc, sockType brconf.SockType) *Sock { + neighIA string, reader, writer SockFunc, sockType brconf.SockType) *Sock { s := &Sock{ - Ring: ring, - Conn: conn, - Dir: dir, - Ifid: ifid, - Label: metrics.IntfToLabel(ifid), - Reader: reader, - Writer: writer, - stop: make(chan struct{}), - Type: sockType, + Ring: ring, + Conn: conn, + Dir: dir, + Ifid: ifid, + Label: metrics.IntfToLabel(ifid), + NeighIA: neighIA, + Reader: reader, + Writer: writer, + stop: make(chan struct{}), + Type: sockType, } if s.Reader != nil { s.readerStopped = make(chan struct{}, 1) diff --git a/go/border/router.go b/go/border/router.go index f14ea5e9d2..42b47dfa4d 100644 --- a/go/border/router.go +++ b/go/border/router.go @@ -127,7 +127,8 @@ func (r *Router) processPacket(rp *rpkt.RtrPkt) { } } l := metrics.ProcessLabels{ - IntfIn: metrics.IntfToLabel(rp.Ingress.IfID), + IntfIn: metrics.IntfToLabel(rp.Ingress.IfID), + IntfOut: metrics.Drop, } // Assign a pseudorandom ID to the packet, for correlating log entries. rp.Id = log.RandId(4) diff --git a/go/border/setup-posix.go b/go/border/setup-posix.go index 8c4d8b2dd6..27f26318b0 100644 --- a/go/border/setup-posix.go +++ b/go/border/setup-posix.go @@ -91,9 +91,9 @@ func (p posixLoc) addSock(r *Router, ctx *rctx.Ctx) error { } // Setup input goroutine. ctx.LocSockIn = rctx.NewSock(ringbuf.New(64, nil, "loc_in"), - over, rcmn.DirLocal, 0, r.posixInput, r.handleSock, PosixSock) + over, rcmn.DirLocal, 0, "", r.posixInput, r.handleSock, PosixSock) ctx.LocSockOut = rctx.NewSock(ringbuf.New(64, nil, "loc_out"), - over, rcmn.DirLocal, 0, nil, r.posixOutput, PosixSock) + over, rcmn.DirLocal, 0, "", nil, r.posixOutput, PosixSock) log.Debug("Done setting up new local socket.", "conn", over.LocalAddr()) return nil } @@ -160,11 +160,11 @@ func (p posixExt) addIntf(r *Router, ctx *rctx.Ctx, intf *topology.IFInfo) error } // Setup input goroutine. ctx.ExtSockIn[intf.Id] = rctx.NewSock( - ringbuf.New(64, nil, fmt.Sprintf("ext_in_%s", intf)), - c, rcmn.DirExternal, intf.Id, r.posixInput, r.handleSock, PosixSock) + ringbuf.New(64, nil, fmt.Sprintf("ext_in_%s", intf.Id)), + c, rcmn.DirExternal, intf.Id, intf.ISD_AS.String(), r.posixInput, r.handleSock, PosixSock) ctx.ExtSockOut[intf.Id] = rctx.NewSock( - ringbuf.New(64, nil, fmt.Sprintf("ext_out_%s", intf)), - c, rcmn.DirExternal, intf.Id, nil, r.posixOutput, PosixSock) + ringbuf.New(64, nil, fmt.Sprintf("ext_out_%s", intf.Id)), + c, rcmn.DirExternal, intf.Id, intf.ISD_AS.String(), nil, r.posixOutput, PosixSock) log.Debug("Done setting up new external socket.", "intf", intf) return nil }