From 3e214b1e8700e354c1e35609d0eea579d09abdda Mon Sep 17 00:00:00 2001 From: Matthias Frei Date: Tue, 2 Jul 2024 13:57:42 +0200 Subject: [PATCH 1/8] WIP: remove SCMPHandler for PacketConn --- gateway/gateway.go | 67 ++++++++++++----------------- gateway/pathhealth/pathwatcher.go | 27 ++++++------ pkg/snet/conn.go | 41 ++++++++++-------- pkg/snet/interface.go | 4 +- pkg/snet/packet_conn.go | 65 +++------------------------- pkg/snet/reader.go | 43 ++++++++++++++---- pkg/snet/scmp.go | 55 ++++++++--------------- pkg/snet/snet.go | 35 +++++++-------- private/app/appnet/infraenv.go | 27 ++---------- private/app/path/pathprobe/paths.go | 46 ++++++++++---------- scion-pki/certs/renew.go | 12 +++--- scion/ping/ping.go | 67 ++++++++++------------------- scion/traceroute/traceroute.go | 59 +++++++++++++------------ tools/end2end/main.go | 41 ++++++------------ 14 files changed, 238 insertions(+), 351 deletions(-) diff --git a/gateway/gateway.go b/gateway/gateway.go index 5ad5b84530..26da091fc5 100644 --- a/gateway/gateway.go +++ b/gateway/gateway.go @@ -94,12 +94,13 @@ func (dpf DataplaneSessionFactory) New(id uint8, policyID int, type PacketConnFactory struct { Network *snet.SCIONNetwork Addr *net.UDPAddr + Options []snet.ConnOption } func (pcf PacketConnFactory) New() (net.PacketConn, error) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() - conn, err := pcf.Network.Listen(ctx, "udp", pcf.Addr) + conn, err := pcf.Network.Listen(ctx, "udp", pcf.Addr, pcf.Options...) if err != nil { return nil, serrors.WrapStr("creating packet conn", err) } @@ -406,29 +407,24 @@ func (g *Gateway) Run(ctx context.Context) error { return serrors.WrapStr("unable to generate TLS config", err) } - // scionNetworkNoSCMP is the network for the QUIC server connection. Because SCMP errors - // will cause the server's accepts to fail, we ignore SCMP. - scionNetworkNoSCMP := &snet.SCIONNetwork{ - Topology: g.Daemon, - // Discard all SCMP propagation, to avoid accept/read errors on the - // QUIC server/client. - SCMPHandler: snet.SCMPPropagationStopper{ - Handler: snet.DefaultSCMPHandler{ - RevocationHandler: revocationHandler, - SCMPErrors: g.Metrics.SCMPErrors, - }, - Log: log.FromCtx(ctx).Debug, - }, + scionNetwork := &snet.SCIONNetwork{ + Topology: g.Daemon, PacketConnMetrics: g.Metrics.SCIONPacketConnMetrics, Metrics: g.Metrics.SCIONNetworkMetrics, } + scmpHandler := snet.DefaultSCMPHandler{ + RevocationHandler: revocationHandler, + SCMPErrors: g.Metrics.SCMPErrors, + Log: log.FromCtx(ctx).Debug, + } // Initialize the UDP/SCION QUIC conn for outgoing Gateway Discovery RPCs and outgoing Prefix // Fetching. Open up a random high port for this. - clientConn, err := scionNetworkNoSCMP.Listen( + clientConn, err := scionNetwork.Listen( context.TODO(), "udp", &net.UDPAddr{IP: g.ControlClientIP}, + snet.WithSCMPHandler(scmpHandler), ) if err != nil { return serrors.WrapStr("unable to initialize client QUIC connection", err) @@ -469,17 +465,6 @@ func (g *Gateway) Run(ctx context.Context) error { } } - // scionNetwork is the network for all SCION connections, with the exception of the QUIC server - // and client connection. - scionNetwork := &snet.SCIONNetwork{ - Topology: g.Daemon, - SCMPHandler: snet.DefaultSCMPHandler{ - RevocationHandler: revocationHandler, - SCMPErrors: g.Metrics.SCMPErrors, - }, - PacketConnMetrics: g.Metrics.SCIONPacketConnMetrics, - Metrics: g.Metrics.SCIONNetworkMetrics, - } remoteMonitor := &control.RemoteMonitor{ IAs: remoteIAsChannel, RemotesMonitored: rmMetric, @@ -518,10 +503,11 @@ func (g *Gateway) Run(ctx context.Context) error { }() logger.Debug("Remote monitor started.") - serverConn, err := scionNetworkNoSCMP.Listen( + serverConn, err := scionNetwork.Listen( context.TODO(), "udp", g.ControlServerAddr, + snet.WithSCMPHandler(scmpHandler), ) if err != nil { return serrors.WrapStr("unable to initialize server QUIC connection", err) @@ -569,7 +555,8 @@ func (g *Gateway) Run(ctx context.Context) error { // received from the session monitors of the remote gateway. // ********************************************************************************* - probeConn, err := scionNetwork.Listen(context.TODO(), "udp", g.ProbeServerAddr) + probeConn, err := scionNetwork.Listen(context.TODO(), "udp", g.ProbeServerAddr, + snet.WithSCMPHandler(scmpHandler)) if err != nil { return serrors.WrapStr("creating server probe conn", err) } @@ -584,8 +571,16 @@ func (g *Gateway) Run(ctx context.Context) error { }() // Start dataplane ingress - if err := StartIngress(ctx, scionNetwork, g.DataServerAddr, deviceManager, - g.Metrics); err != nil { + dataplaneServerConn, err := scionNetwork.Listen( + context.TODO(), + "udp", + g.DataServerAddr, + snet.WithSCMPHandler(scmpHandler), + ) + if err != nil { + return serrors.WrapStr("creating ingress conn", err) + } + if err := StartIngress(ctx, dataplaneServerConn, deviceManager, g.Metrics); err != nil { return err } @@ -628,12 +623,14 @@ func (g *Gateway) Run(ctx context.Context) error { ProbeConnFactory: PacketConnFactory{ Network: scionNetwork, Addr: &net.UDPAddr{IP: g.ProbeClientIP}, + Options: []snet.ConnOption{snet.WithSCMPHandler(scmpHandler)}, }, DeviceManager: deviceManager, DataplaneSessionFactory: DataplaneSessionFactory{ PacketConnFactory: PacketConnFactory{ Network: scionNetwork, Addr: &net.UDPAddr{IP: g.DataClientIP}, + Options: []snet.ConnOption{snet.WithSCMPHandler(scmpHandler)}, }, Metrics: CreateSessionMetrics(g.Metrics), }, @@ -759,18 +756,10 @@ func CreateIngressMetrics(m *Metrics) dataplane.IngressMetrics { } } -func StartIngress(ctx context.Context, scionNetwork *snet.SCIONNetwork, dataAddr *net.UDPAddr, +func StartIngress(ctx context.Context, dataplaneServerConn *snet.Conn, deviceManager control.DeviceManager, metrics *Metrics) error { logger := log.FromCtx(ctx) - dataplaneServerConn, err := scionNetwork.Listen( - context.TODO(), - "udp", - dataAddr, - ) - if err != nil { - return serrors.WrapStr("creating ingress conn", err) - } ingressMetrics := CreateIngressMetrics(metrics) ingressServer := &dataplane.IngressServer{ Conn: dataplaneServerConn, diff --git a/gateway/pathhealth/pathwatcher.go b/gateway/pathhealth/pathwatcher.go index a61ae80ca4..68529bbccd 100644 --- a/gateway/pathhealth/pathwatcher.go +++ b/gateway/pathhealth/pathwatcher.go @@ -83,13 +83,6 @@ func (f *DefaultPathWatcherFactory) New( return create(remote) } conn, err := (&snet.SCIONNetwork{ - SCMPHandler: scmpHandler{ - wrappedHandler: snet.DefaultSCMPHandler{ - RevocationHandler: f.RevocationHandler, - SCMPErrors: f.SCMPErrors, - }, - pkts: pktChan, - }, PacketConnMetrics: f.SCIONPacketConnMetrics, Topology: f.Topology, }).OpenRaw(ctx, &net.UDPAddr{IP: f.LocalIP.AsSlice()}) @@ -105,6 +98,13 @@ func (f *DefaultPathWatcherFactory) New( IA: f.LocalIA, Host: addr.HostIP(f.LocalIP), }, + scmpHandler: scmpHandler{ + wrappedHandler: snet.DefaultSCMPHandler{ + RevocationHandler: f.RevocationHandler, + SCMPErrors: f.SCMPErrors, + }, + pkts: pktChan, + }, pktChan: pktChan, probesSent: createCounter(f.ProbesSent, remote), probesReceived: createCounter(f.ProbesReceived, remote), @@ -122,6 +122,8 @@ type pathWatcher struct { // conn is the packet conn used to send probes on. The pathwatcher takes // ownership and will close it on termination. conn snet.PacketConn + // scmpHandler is handles SCMPs received on conn and inserts traceroute packets into pktChan + scmpHandler snet.SCMPHandler // id is used as SCMP traceroute ID. Since each pathwatcher should have it's // own high port this value can be random. id uint16 @@ -239,13 +241,14 @@ func (w *pathWatcher) drainConn(ctx context.Context) { return } if err != nil { - if _, ok := err.(*snet.OpError); ok { - // ignore SCMP errors they are already dealt with in the SCMP - // handler. - continue - } logger.Info("Unexpected error when reading probe reply", "err", err) } + if _, ok := pkt.Payload.(snet.SCMPPayload); !ok { + continue + } + if err := w.scmpHandler.Handle(&pkt); err == nil { + logger.Info("Unexpected error when handling SCMP packet", "err", err) + } } } diff --git a/pkg/snet/conn.go b/pkg/snet/conn.go index 3ec657f299..5506ac522d 100644 --- a/pkg/snet/conn.go +++ b/pkg/snet/conn.go @@ -18,29 +18,16 @@ package snet import ( "context" "net" + "syscall" "time" "github.com/scionproto/scion/pkg/private/common" - "github.com/scionproto/scion/pkg/private/ctrl/path_mgmt" "github.com/scionproto/scion/pkg/private/serrors" - "github.com/scionproto/scion/pkg/slayers" ) -type OpError struct { - typeCode slayers.SCMPTypeCode - revInfo *path_mgmt.RevInfo -} - -func (e *OpError) RevInfo() *path_mgmt.RevInfo { - return e.revInfo -} - -func (e *OpError) Error() string { - return e.typeCode.String() -} - var _ net.Conn = (*Conn)(nil) var _ net.PacketConn = (*Conn)(nil) +var _ syscall.Conn = (*Conn)(nil) type Conn struct { conn PacketConn @@ -96,6 +83,7 @@ func NewCookedConn( conn: pconn, buffer: make([]byte, common.SupportedMTU), replyPather: o.replyPather, + scmpHandler: o.scmpHandler, local: local, }, }, nil @@ -109,6 +97,10 @@ func (c *Conn) RemoteAddr() net.Addr { return c.remote } +func (c *Conn) SyscallConn() (syscall.RawConn, error) { + return c.conn.SyscallConn() +} + func (c *Conn) SetDeadline(t time.Time) error { if err := c.scionConnReader.SetReadDeadline(t); err != nil { return err @@ -128,16 +120,25 @@ type ConnOption func(o *options) // WithReplyPather sets the reply pather for the connection. // The reply pather is responsible for determining the path to send replies to. -// If the provided replyPather is not nil, it will be set as the reply pather for the connection. +// If this option is not provided, DefaultReplyPather is used. func WithReplyPather(replyPather ReplyPather) ConnOption { return func(o *options) { - if replyPather != nil { - o.replyPather = replyPather - } + o.replyPather = replyPather + } +} + +// WithSCMPHandler sets the SCMP handler for the connection. +// The SCMP handler is a callback to react to SCMP messages, specifically to error messages. +// If this option is not provided, no SCMP handler is used (equivalent to +// DefaultSCMPHandler with an empty RevocationHandler). +func WithSCMPHandler(scmpHandler SCMPHandler) ConnOption { + return func(o *options) { + o.scmpHandler = scmpHandler } } // WithRemote sets the remote address for the connection. +// This only applies to NewCookedConn, but not Dial/Listen. func WithRemote(addr *UDPAddr) ConnOption { return func(o *options) { o.remote = addr @@ -146,12 +147,14 @@ func WithRemote(addr *UDPAddr) ConnOption { type options struct { replyPather ReplyPather + scmpHandler SCMPHandler remote *UDPAddr } func apply(opts []ConnOption) options { o := options{ replyPather: DefaultReplyPather{}, + scmpHandler: nil, } for _, option := range opts { option(&o) diff --git a/pkg/snet/interface.go b/pkg/snet/interface.go index 1e333b56dd..9ae994db3c 100644 --- a/pkg/snet/interface.go +++ b/pkg/snet/interface.go @@ -22,6 +22,6 @@ import ( type Network interface { OpenRaw(ctx context.Context, addr *net.UDPAddr) (PacketConn, error) - Listen(ctx context.Context, network string, listen *net.UDPAddr) (*Conn, error) - Dial(ctx context.Context, network string, listen *net.UDPAddr, remote *UDPAddr) (*Conn, error) + Listen(ctx context.Context, network string, listen *net.UDPAddr, options ...ConnOption) (*Conn, error) + Dial(ctx context.Context, network string, listen *net.UDPAddr, remote *UDPAddr, options ...ConnOption) (*Conn, error) } diff --git a/pkg/snet/packet_conn.go b/pkg/snet/packet_conn.go index 8e545f73ce..1d5100d474 100644 --- a/pkg/snet/packet_conn.go +++ b/pkg/snet/packet_conn.go @@ -24,7 +24,6 @@ import ( "github.com/scionproto/scion/pkg/metrics/v2" "github.com/scionproto/scion/pkg/private/common" "github.com/scionproto/scion/pkg/private/serrors" - "github.com/scionproto/scion/pkg/slayers" "github.com/scionproto/scion/pkg/slayers/path/empty" "github.com/scionproto/scion/pkg/slayers/path/epic" "github.com/scionproto/scion/pkg/slayers/path/onehop" @@ -73,22 +72,6 @@ func (b *Bytes) Prepare() { *b = (*b)[:cap(*b)] } -type L4Header interface { - closed() -} - -type UDPL4 struct { - slayers.UDP -} - -func (UDPL4) closed() {} - -type SCMPExternalInterfaceDownL4 struct { - slayers.SCMPExternalInterfaceDown -} - -func (SCMPExternalInterfaceDownL4) closed() {} - // SCIONAddress is the fully-specified address of a host. type SCIONAddress = addr.Addr @@ -116,10 +99,6 @@ type SCIONPacketConnMetrics struct { type SCIONPacketConn struct { // Conn is the connection to send/receive serialized packets on. Conn *net.UDPConn - // SCMPHandler is invoked for packets that contain an SCMP L4. If the - // handler is nil, errors are returned back to applications every time an - // SCMP message is received. - SCMPHandler SCMPHandler // Metrics are the metrics exported by the conn. Metrics SCIONPacketConnMetrics interfaceMap interfaceMap @@ -162,31 +141,12 @@ func (c *SCIONPacketConn) SetWriteDeadline(d time.Time) error { } func (c *SCIONPacketConn) ReadFrom(pkt *Packet, ov *net.UDPAddr) error { - for { - // Read until we get an error or a data packet - remoteAddr, err := c.readFrom(pkt) - if err != nil { - return err - } - *ov = *remoteAddr - if scmp, ok := pkt.Payload.(SCMPPayload); ok { - if c.SCMPHandler == nil { - metrics.CounterInc(c.Metrics.SCMPErrors) - return serrors.New("scmp packet received, but no handler found", - "type_code", slayers.CreateSCMPTypeCode(scmp.Type(), scmp.Code()), - "src", pkt.Source) - } - if err := c.SCMPHandler.Handle(pkt); err != nil { - // Return error intact s.t. applications can handle custom - // error types returned by SCMP handlers. - return err - } - continue - } - // non-SCMP L4s are assumed to be data and get passed back to the - // app. - return nil + remoteAddr, err := c.readFrom(pkt) + if err != nil { + return err } + *ov = *remoteAddr + return nil } func (c *SCIONPacketConn) SyscallConn() (syscall.RawConn, error) { @@ -328,21 +288,6 @@ func (c *SCIONPacketConn) lastHop(p *Packet) (*net.UDPAddr, error) { } } -type SerializationOptions struct { - // If ComputeChecksums is true, the checksums in sent Packets are - // recomputed. Otherwise, the checksum value is left intact. - ComputeChecksums bool - // If FixLengths is true, any lengths in sent Packets are recomputed - // to match the data contained in payloads/inner layers. This currently - // concerns extension headers and the L4 header. - FixLengths bool - // If InitializePaths is set to true, then forwarding paths are reset to - // their starting InfoField/HopField during serialization, irrespective of - // previous offsets. If it is set to false, then the fields are left - // unchanged. - InitializePaths bool -} - type interfaceMap map[uint16]netip.AddrPort func (m interfaceMap) get(id uint16) (*net.UDPAddr, error) { diff --git a/pkg/snet/reader.go b/pkg/snet/reader.go index 06de4f5a38..d11b088f99 100644 --- a/pkg/snet/reader.go +++ b/pkg/snet/reader.go @@ -32,9 +32,11 @@ type ReplyPather interface { } type scionConnReader struct { + conn PacketConn + local *UDPAddr + replyPather ReplyPather - conn PacketConn - local *UDPAddr + scmpHandler SCMPHandler mtx sync.Mutex buffer []byte @@ -69,11 +71,12 @@ func (c *scionConnReader) read(b []byte) (int, *UDPAddr, error) { Bytes: Bytes(c.buffer), } var lastHop net.UDPAddr - err := c.conn.ReadFrom(&pkt, &lastHop) + err := c.readPacketUDP(&pkt, &lastHop) if err != nil { return 0, nil, err } + udp := pkt.Payload.(UDPPayload) rpath, ok := pkt.Path.(RawPath) if !ok { return 0, nil, serrors.New("unexpected path", "type", common.TypeOf(pkt.Path)) @@ -83,11 +86,6 @@ func (c *scionConnReader) read(b []byte) (int, *UDPAddr, error) { return 0, nil, serrors.WrapStr("creating reply path", err) } - udp, ok := pkt.Payload.(UDPPayload) - if !ok { - return 0, nil, serrors.New("unexpected payload", "type", common.TypeOf(pkt.Payload)) - } - // XXX(JordiSubira): We explicitly forbid nil or unspecified address in the current constructor // for Conn. // If this were ever to change, we would always fall into the following if statement, then @@ -119,6 +117,35 @@ func (c *scionConnReader) read(b []byte) (int, *UDPAddr, error) { return n, remote, nil } +// readPacketUDP repeatedly reads a packet until a UDP datagram is found. +// If an SCMP Handler is configured, it will be called on SCMP messages. +func (c *scionConnReader) readPacketUDP(pkt *Packet, lastHop *net.UDPAddr) error { + for { + err := c.conn.ReadFrom(pkt, lastHop) + if err != nil { + return err + } + + switch pkt.Payload.(type) { + case UDPPayload: + return nil + case SCMPPayload: + if c.scmpHandler == nil { + // TODO metrics + // metrics.CounterInc(c.Metrics.SCMPErrors) + continue + } + err := c.scmpHandler.Handle(pkt) + if err != nil { + // metrics.CounterInc(c.Metrics.SCMPErrors) + } + continue + default: + continue + } + } +} + func (c *scionConnReader) SetReadDeadline(t time.Time) error { return c.conn.SetReadDeadline(t) } diff --git a/pkg/snet/scmp.go b/pkg/snet/scmp.go index ddbbf38268..3b7d18183c 100644 --- a/pkg/snet/scmp.go +++ b/pkg/snet/scmp.go @@ -18,7 +18,6 @@ import ( "context" "time" - "github.com/scionproto/scion/pkg/log" "github.com/scionproto/scion/pkg/metrics/v2" "github.com/scionproto/scion/pkg/private/common" "github.com/scionproto/scion/pkg/private/ctrl/path_mgmt" @@ -35,14 +34,9 @@ type RevocationHandler interface { // SCMPHandler customizes the way snet connections deal with SCMP. type SCMPHandler interface { - // Handle processes the packet as an SCMP packet. If packet is not SCMP, it - // returns an error. - // - // If the handler returns an error value, snet will propagate the error - // back to the caller. If the return value is nil, snet will reattempt to - // read a data packet from the underlying connection. - // - // Handlers that wish to ignore SCMP can just return nil. + // Handle processes the packet as an SCMP packet. If packet is not SCMP or otherwise + // malformed, it returns an error. + // Errors may be counted in metrics, but otherwise ignored. // // If the handler mutates the packet, the changes are seen by snet // connection method callers. @@ -58,6 +52,9 @@ type DefaultSCMPHandler struct { RevocationHandler RevocationHandler // SCMPErrors reports the total number of SCMP Errors encountered. SCMPErrors metrics.Counter + // Log is an optional function that is used to log SCMP messages + // TODO log... + Log func(msg string, ctx ...any) } func (h DefaultSCMPHandler) Handle(pkt *Packet) error { @@ -72,7 +69,7 @@ func (h DefaultSCMPHandler) Handle(pkt *Packet) error { switch scmp.Type() { case slayers.SCMPTypeExternalInterfaceDown: msg := pkt.Payload.(SCMPExternalInterfaceDown) - return h.handleSCMPRev(typeCode, &path_mgmt.RevInfo{ + return h.handleSCMPRev(&path_mgmt.RevInfo{ IfID: common.IFIDType(msg.Interface), RawIsdas: msg.IA, RawTimestamp: util.TimeToSecs(time.Now()), @@ -80,47 +77,31 @@ func (h DefaultSCMPHandler) Handle(pkt *Packet) error { }) case slayers.SCMPTypeInternalConnectivityDown: msg := pkt.Payload.(SCMPInternalConnectivityDown) - return h.handleSCMPRev(typeCode, &path_mgmt.RevInfo{ + return h.handleSCMPRev(&path_mgmt.RevInfo{ IfID: common.IFIDType(msg.Egress), RawIsdas: msg.IA, RawTimestamp: util.TimeToSecs(time.Now()), RawTTL: 10, }) + default: - // Only handle connectivity down for now - log.Debug("Ignoring scmp packet", "scmp", typeCode, "src", pkt.Source) + h.log("Ignoring SCMP packet", "scmp", typeCode, "src", pkt.Source) return nil } } -func (h *DefaultSCMPHandler) handleSCMPRev(typeCode slayers.SCMPTypeCode, - revInfo *path_mgmt.RevInfo) error { - if h.RevocationHandler != nil { - err := h.RevocationHandler.Revoke(context.TODO(), revInfo) - if err != nil { - log.Info("Notifying revocation handler failed", "err", err) - } +func (h DefaultSCMPHandler) log(msg string, ctx ...any) { + if h.Log == nil { + return } - return &OpError{typeCode: typeCode, revInfo: revInfo} + h.Log(msg, ctx) } -// SCMPPropagationStopper wraps an SCMP handler and stops propagation of the -// SCMP errors up the stack. This can be necessary if the client code aborts on -// unexpected errors. This is a temporary solution until we address -// https://github.com/scionproto/scion/issues/4389. -// -// EXPERIMENTAL: This handler is experimental and may be removed in the future. -type SCMPPropagationStopper struct { - // Handler is the wrapped handler. - Handler SCMPHandler - // Log is an optional function that is called when the wrapped handler - // returns an error and propagation is stopped. - Log func(msg string, ctx ...any) -} +// TODO: matzf replace RevocationHandler interface with something that does not rely on ancient RevInfo struct +func (h *DefaultSCMPHandler) handleSCMPRev(revInfo *path_mgmt.RevInfo) error { -func (h SCMPPropagationStopper) Handle(pkt *Packet) error { - if err := h.Handler.Handle(pkt); err != nil && h.Log != nil { - h.Log("Stopped SCMP error propagation", "err", err) + if h.RevocationHandler != nil { + return h.RevocationHandler.Revoke(context.TODO(), revInfo) } return nil } diff --git a/pkg/snet/snet.go b/pkg/snet/snet.go index 2abe8d61f2..cffa1f5029 100644 --- a/pkg/snet/snet.go +++ b/pkg/snet/snet.go @@ -28,12 +28,6 @@ // used to send a message to a chosen destination. // // Multiple networking contexts can share the same SCIOND. -// -// Write calls never return SCMP errors directly. If a write call caused an -// SCMP message to be received by the Conn, it can be inspected by calling -// Read. In this case, the error value is non-nil and can be type asserted to -// *OpError. Method SCMP() can be called on the error to extract the SCMP -// header. package snet import ( @@ -41,6 +35,7 @@ import ( "errors" "net" "net/netip" + "slices" "syscall" "github.com/scionproto/scion/pkg/addr" @@ -70,14 +65,10 @@ type SCIONNetwork struct { // Topology provides local AS information, needed to handle sockets and // traffic. Topology Topology - // ReplyPather is used to create reply paths when reading packets on Conn - // (that implements net.Conn). If unset, the default reply pather is used, - // which parses the incoming path as a path.Path and reverses it. - ReplyPather ReplyPather - // Metrics holds the metrics emitted by the network. - Metrics SCIONNetworkMetrics - // SCMPHandler describes the network behaviour upon receiving SCMP traffic. - SCMPHandler SCMPHandler + + // XXX: SCMPHandler / Reverser + + Metrics SCIONNetworkMetrics PacketConnMetrics SCIONPacketConnMetrics } @@ -119,7 +110,6 @@ func (n *SCIONNetwork) OpenRaw(ctx context.Context, addr *net.UDPAddr) (PacketCo } return &SCIONPacketConn{ Conn: pconn, - SCMPHandler: n.SCMPHandler, Metrics: n.PacketConnMetrics, interfaceMap: ifAddrs, }, nil @@ -133,8 +123,13 @@ func (n *SCIONNetwork) OpenRaw(ctx context.Context, addr *net.UDPAddr) (PacketCo // // The context is used for connection setup, it doesn't affect the returned // connection. -func (n *SCIONNetwork) Dial(ctx context.Context, network string, listen *net.UDPAddr, - remote *UDPAddr) (*Conn, error) { +func (n *SCIONNetwork) Dial( + ctx context.Context, + network string, + listen *net.UDPAddr, + remote *UDPAddr, + options ...ConnOption, +) (*Conn, error) { // XXX(JordiSubira): Currently Dial does not check that received packets are // originated from the expected remote address. This should be adapted to // check that the remote packets are originated from the expected remote address. @@ -151,7 +146,8 @@ func (n *SCIONNetwork) Dial(ctx context.Context, network string, listen *net.UDP return nil, err } log.FromCtx(ctx).Debug("UDP socket opened on", "addr", packetConn.LocalAddr(), "to", remote) - return NewCookedConn(packetConn, n.Topology, WithReplyPather(n.ReplyPather), WithRemote(remote)) + options = append(slices.Clone(options), WithRemote(remote)) + return NewCookedConn(packetConn, n.Topology, options...) } // Listen opens a Conn. The returned connection's ReadFrom and WriteTo methods @@ -165,6 +161,7 @@ func (n *SCIONNetwork) Listen( ctx context.Context, network string, listen *net.UDPAddr, + options ...ConnOption, ) (*Conn, error) { metrics.CounterInc(n.Metrics.Listens) @@ -176,7 +173,7 @@ func (n *SCIONNetwork) Listen( return nil, err } log.FromCtx(ctx).Debug("UDP socket openned on", "addr", packetConn.LocalAddr()) - return NewCookedConn(packetConn, n.Topology, WithReplyPather(n.ReplyPather)) + return NewCookedConn(packetConn, n.Topology, options...) } func listenUDPRange(addr *net.UDPAddr, start, end uint16) (*net.UDPConn, error) { diff --git a/private/app/appnet/infraenv.go b/private/app/appnet/infraenv.go index 7d86d39937..f93b6e0e56 100644 --- a/private/app/appnet/infraenv.go +++ b/private/app/appnet/infraenv.go @@ -199,7 +199,6 @@ func (nc *NetworkConfig) AddressRewriter() *AddressRewriter { LocalIA: nc.IA, Network: &snet.SCIONNetwork{ Topology: nc.Topology, - SCMPHandler: nc.SCMPHandler, Metrics: nc.SCIONNetworkMetrics, PacketConnMetrics: nc.SCIONPacketConnMetrics, }, @@ -221,11 +220,8 @@ func (nc *NetworkConfig) initQUICSockets() (net.PacketConn, net.PacketConn, erro } serverNet := &snet.SCIONNetwork{ - Topology: nc.Topology, - // XXX(roosd): This is essential, the server must not read SCMP - // errors. Otherwise, the accept loop will always return that error - // on every subsequent call to accept. - SCMPHandler: ignoreSCMP{}, + Topology: nc.Topology, + Metrics: nc.SCIONNetworkMetrics, PacketConnMetrics: nc.SCIONPacketConnMetrics, } pconn, err := serverNet.OpenRaw(context.Background(), nc.Public) @@ -248,13 +244,7 @@ func (nc *NetworkConfig) initQUICSockets() (net.PacketConn, net.PacketConn, erro } clientNet := &snet.SCIONNetwork{ - Topology: nc.Topology, - // Discard all SCMP propagation, to avoid read errors on the QUIC - // client. - SCMPHandler: snet.SCMPPropagationStopper{ - Handler: nc.SCMPHandler, - Log: log.Debug, - }, + Topology: nc.Topology, Metrics: nc.SCIONNetworkMetrics, PacketConnMetrics: nc.SCIONPacketConnMetrics, } @@ -300,14 +290,3 @@ func NewRouter(localIA addr.IA, sd env.Daemon) (snet.Router, error) { } return router, nil } - -// ignoreSCMP ignores all received SCMP packets. -// -// XXX(roosd): This is needed such that the QUIC server does not shut down when -// receiving a SCMP error. DO NOT REMOVE! -type ignoreSCMP struct{} - -func (ignoreSCMP) Handle(pkt *snet.Packet) error { - // Always reattempt reads from the socket. - return nil -} diff --git a/private/app/path/pathprobe/paths.go b/private/app/path/pathprobe/paths.go index 62df6c737a..4957fa14d7 100644 --- a/private/app/path/pathprobe/paths.go +++ b/private/app/path/pathprobe/paths.go @@ -163,7 +163,6 @@ func (p Prober) GetStatuses(ctx context.Context, paths []snet.Path, // Instantiate network sn := &snet.SCIONNetwork{ - SCMPHandler: &scmpHandler{}, PacketConnMetrics: p.SCIONPacketConnMetrics, Topology: p.Topology, } @@ -255,19 +254,28 @@ func (p Prober) GetStatuses(ctx context.Context, paths []snet.Path, // Wait for the replies. var pkt snet.Packet var ov net.UDPAddr - for range paths { + statusKnown := make(map[string]struct{}) + for len(statusKnown) < len(paths) { if err := conn.ReadFrom(&pkt, &ov); err != nil { - var r reply - if errors.As(err, &r) { - addStatus(r.PathKey, r.Status) - continue - } // If the deadline is exceeded, all remaining paths have timed out. if errors.Is(err, os.ErrDeadlineExceeded) { break } return serrors.WrapStr("waiting for probe reply", err, "local", localIP) } + + r, err := handleSCMP(&pkt) + if err != nil { + continue + } + // Ignore any further update after status is set successfully + if _, done := statusKnown[r.PathKey]; done { + continue + } + addStatus(r.PathKey, r.Status) + if r.Status.Status != StatusUnknown { + statusKnown[r.PathKey] = struct{}{} + } } return nil }) @@ -298,42 +306,36 @@ type reply struct { PathKey string } -func (r reply) Error() string { - return fmt.Sprint(r.Status) -} - -type scmpHandler struct{} - -func (h *scmpHandler) Handle(pkt *snet.Packet) error { +func handleSCMP(pkt *snet.Packet) (reply, error) { path, ok := pkt.Path.(snet.RawPath) if !ok { - return serrors.New("not an snet.RawPath") + return reply{}, serrors.New("not an snet.RawPath") } replyPath, err := snet.DefaultReplyPather{}.ReplyPath(path) if err != nil { - return serrors.WrapStr("creating reply path", err) + return reply{}, serrors.WrapStr("creating reply path", err) } rawReplyPath, ok := replyPath.(snet.RawReplyPath) if !ok { - return serrors.New("not an snet.RawReplyPath", "type", common.TypeOf(replyPath)) + return reply{}, serrors.New("not an snet.RawReplyPath", "type", common.TypeOf(replyPath)) } scionReplyPath := snetpath.SCION{ Raw: make([]byte, rawReplyPath.Path.Len()), } if err := rawReplyPath.Path.SerializeTo(scionReplyPath.Raw); err != nil { - return serrors.WrapStr("serialization failed", err) + return reply{}, serrors.WrapStr("serialization failed", err) } - status, err := h.toStatus(pkt) + status, err := scmpToStatus(pkt) if err != nil { - return err + return reply{}, err } return reply{ Status: status, PathKey: PathKey(snetpath.Path{DataplanePath: scionReplyPath}), - } + }, nil } -func (h *scmpHandler) toStatus(pkt *snet.Packet) (Status, error) { +func scmpToStatus(pkt *snet.Packet) (Status, error) { if pkt.Payload == nil { return Status{}, serrors.New("no payload found") } diff --git a/scion-pki/certs/renew.go b/scion-pki/certs/renew.go index cd13c6bfef..c89970055d 100644 --- a/scion-pki/certs/renew.go +++ b/scion-pki/certs/renew.go @@ -738,15 +738,12 @@ func (r *renewer) requestRemote( sn := &snet.SCIONNetwork{ Topology: r.Daemon, - SCMPHandler: snet.SCMPPropagationStopper{ - Handler: snet.DefaultSCMPHandler{ - RevocationHandler: daemon.RevHandler{Connector: r.Daemon}, - }, - Log: log.FromCtx(ctx).Debug, - }, } - conn, err := sn.Listen(ctx, "udp", local.Host) + scmpHandler := snet.DefaultSCMPHandler{ + RevocationHandler: daemon.RevHandler{Connector: r.Daemon}, + } + conn, err := sn.Listen(ctx, "udp", local.Host, snet.WithSCMPHandler(scmpHandler)) if err != nil { return nil, serrors.WrapStr("dialing", err) } @@ -761,6 +758,7 @@ func (r *renewer) requestRemote( Resolver: &svc.Resolver{ LocalIA: local.IA, Network: sn, + // XXX scmp handler lost here LocalIP: local.Host.IP, }, }, diff --git a/scion/ping/ping.go b/scion/ping/ping.go index af0a983d1a..1b0e69c962 100644 --- a/scion/ping/ping.go +++ b/scion/ping/ping.go @@ -103,13 +103,8 @@ func Run(ctx context.Context, cfg Config) (Stats, error) { return Stats{}, serrors.New("interval below millisecond") } - replies := make(chan reply, 10) - scmpHandler := &scmpHandler{ - replies: replies, - } sn := &snet.SCIONNetwork{ - SCMPHandler: scmpHandler, - Topology: cfg.Topology, + Topology: cfg.Topology, } conn, err := sn.OpenRaw(ctx, cfg.Local.Host) if err != nil { @@ -122,7 +117,6 @@ func Run(ctx context.Context, cfg Config) (Stats, error) { // we set the identifier on the handler to the same value as // the udp port id := local.Host.Port - scmpHandler.SetId(id) // we need to have at least 8 bytes to store the request time in the // payload. @@ -138,7 +132,7 @@ func Run(ctx context.Context, cfg Config) (Stats, error) { id: uint16(id), conn: conn, local: local, - replies: replies, + replies: make(chan reply, 10), errHandler: cfg.ErrHandler, updateHandler: cfg.UpdateHandler, } @@ -154,7 +148,7 @@ type pinger struct { id uint16 conn snet.PacketConn local *snet.UDPAddr - replies <-chan reply + replies chan reply // Handlers errHandler func(error) @@ -281,13 +275,13 @@ func (p *pinger) receive(reply reply) { func (p *pinger) drain(ctx context.Context) { var last time.Time + var pkt snet.Packet + var ov net.UDPAddr for { select { case <-ctx.Done(): return default: - var pkt snet.Packet - var ov net.UDPAddr if err := p.conn.ReadFrom(&pkt, &ov); err != nil && p.errHandler != nil { // Rate limit the error reports. if now := time.Now(); now.Sub(last) > 500*time.Millisecond { @@ -295,6 +289,14 @@ func (p *pinger) drain(ctx context.Context) { last = now } } + echo, err := toSCMPEchoReply(p.id, &pkt) + p.replies <- reply{ + Received: time.Now(), + Source: pkt.Source, + Size: len(pkt.Bytes), + Reply: echo, + Error: err, + } } } } @@ -307,48 +309,23 @@ type reply struct { Error error } -type scmpHandler struct { - id uint16 - replies chan<- reply -} - -func (h scmpHandler) Handle(pkt *snet.Packet) error { - echo, err := h.handle(pkt) - h.replies <- reply{ - Received: time.Now(), - Source: pkt.Source, - Size: len(pkt.Bytes), - Reply: echo, - Error: err, - } - return nil -} - -func (h *scmpHandler) SetId(id int) { - h.id = uint16(id) -} - -func (h scmpHandler) handle(pkt *snet.Packet) (snet.SCMPEchoReply, error) { - if pkt.Payload == nil { - return snet.SCMPEchoReply{}, serrors.New("no v2 payload found") - } - switch s := pkt.Payload.(type) { +func toSCMPEchoReply(expectedId uint16, pkt *snet.Packet) (snet.SCMPEchoReply, error) { + switch r := pkt.Payload.(type) { case snet.SCMPEchoReply: + if r.Identifier != expectedId { + return snet.SCMPEchoReply{}, serrors.New("wrong SCMP ID", + "expected", expectedId, "actual", r.Identifier) + } + return r, nil case snet.SCMPExternalInterfaceDown: return snet.SCMPEchoReply{}, serrors.New("external interface is down", - "isd_as", s.IA, "interface", s.Interface) + "isd_as", r.IA, "interface", r.Interface) case snet.SCMPInternalConnectivityDown: return snet.SCMPEchoReply{}, serrors.New("internal connectivity is down", - "isd_as", s.IA, "ingress", s.Ingress, "egress", s.Egress) + "isd_as", r.IA, "ingress", r.Ingress, "egress", r.Egress) default: return snet.SCMPEchoReply{}, serrors.New("not SCMPEchoReply", "type", common.TypeOf(pkt.Payload), ) } - r := pkt.Payload.(snet.SCMPEchoReply) - if r.Identifier != h.id { - return snet.SCMPEchoReply{}, serrors.New("wrong SCMP ID", - "expected", h.id, "actual", r.Identifier) - } - return r, nil } diff --git a/scion/traceroute/traceroute.go b/scion/traceroute/traceroute.go index bcecbb9e04..eba34d0067 100644 --- a/scion/traceroute/traceroute.go +++ b/scion/traceroute/traceroute.go @@ -83,7 +83,7 @@ type tracerouter struct { errHandler func(error) updateHandler func(Update) - replies <-chan reply + replies chan reply path snet.Path epic bool @@ -98,10 +98,8 @@ func Run(ctx context.Context, cfg Config) (Stats, error) { if _, isEmpty := cfg.PathEntry.Dataplane().(path.Empty); isEmpty { return Stats{}, serrors.New("empty path is not allowed for traceroute") } - replies := make(chan reply, 10) sn := &snet.SCIONNetwork{ - SCMPHandler: scmpHandler{replies: replies}, - Topology: cfg.Topology, + Topology: cfg.Topology, } conn, err := sn.OpenRaw(ctx, cfg.Local.Host) if err != nil { @@ -115,7 +113,7 @@ func Run(ctx context.Context, cfg Config) (Stats, error) { conn: conn, local: local, remote: cfg.Remote, - replies: replies, + replies: make(chan reply, 10), errHandler: cfg.ErrHandler, updateHandler: cfg.UpdateHandler, id: uint16(conn.LocalAddr().(*net.UDPAddr).Port), @@ -299,6 +297,14 @@ func (t tracerouter) drain(ctx context.Context) { last = now } } + tr, err := toSCMPTracerouteReply(t.id, &pkt) + t.replies <- reply{ + Received: time.Now(), + Reply: tr, + Remote: pkt.Source, + Error: err, + } + } } } @@ -310,30 +316,23 @@ type reply struct { Error error } -type scmpHandler struct { - replies chan<- reply -} - -func (h scmpHandler) Handle(pkt *snet.Packet) error { - r, err := h.handle(pkt) - - h.replies <- reply{ - Received: time.Now(), - Reply: r, - Remote: pkt.Source, - Error: err, - } - return nil -} - -func (h scmpHandler) handle(pkt *snet.Packet) (snet.SCMPTracerouteReply, error) { - if pkt.Payload == nil { - return snet.SCMPTracerouteReply{}, serrors.New("no payload found") - } - r, ok := pkt.Payload.(snet.SCMPTracerouteReply) - if !ok { - return snet.SCMPTracerouteReply{}, serrors.New("not SCMPTracerouteReply", - "type", common.TypeOf(pkt.Payload)) +func toSCMPTracerouteReply(expectedId uint16, pkt *snet.Packet) (snet.SCMPTracerouteReply, error) { + switch r := pkt.Payload.(type) { + case snet.SCMPTracerouteReply: + if r.Identifier != expectedId { + return snet.SCMPTracerouteReply{}, serrors.New("wrong SCMP ID", + "expected", expectedId, "actual", r.Identifier) + } + return r, nil + case snet.SCMPExternalInterfaceDown: + return snet.SCMPTracerouteReply{}, serrors.New("external interface is down", + "isd_as", r.IA, "interface", r.Interface) + case snet.SCMPInternalConnectivityDown: + return snet.SCMPTracerouteReply{}, serrors.New("internal connectivity is down", + "isd_as", r.IA, "ingress", r.Ingress, "egress", r.Egress) + default: + return snet.SCMPTracerouteReply{}, serrors.New("not SCMPEchoReply", + "type", common.TypeOf(pkt.Payload), + ) } - return r, nil } diff --git a/tools/end2end/main.go b/tools/end2end/main.go index d16578140d..aec40fb4b2 100644 --- a/tools/end2end/main.go +++ b/tools/end2end/main.go @@ -25,10 +25,8 @@ import ( "bytes" "context" "encoding/json" - "errors" "flag" "fmt" - "net" "os" "time" @@ -135,14 +133,15 @@ func (s server) run() { sdConn := integration.SDConn() defer sdConn.Close() sn := &snet.SCIONNetwork{ - SCMPHandler: snet.DefaultSCMPHandler{ - RevocationHandler: daemon.RevHandler{Connector: sdConn}, - SCMPErrors: scmpErrorsCounter, - }, PacketConnMetrics: scionPacketConnMetrics, Topology: sdConn, } - conn, err := sn.Listen(context.Background(), "udp", integration.Local.Host) + scmpHandler := snet.DefaultSCMPHandler{ + RevocationHandler: daemon.RevHandler{Connector: sdConn}, + SCMPErrors: scmpErrorsCounter, + } + conn, err := sn.Listen(context.Background(), "udp", integration.Local.Host, + snet.WithSCMPHandler(scmpHandler)) if err != nil { integration.LogFatal("Error listening", "err", err) } @@ -164,7 +163,7 @@ func (s server) run() { func (s server) handlePing(conn *snet.Conn) error { rawPld := make([]byte, common.MaxMTU) - n, clientAddr, err := readFrom(conn, rawPld) + n, clientAddr, err := conn.ReadFrom(rawPld) if err != nil { return serrors.WrapStr("reading packet", err) } @@ -234,10 +233,6 @@ func (c *client) run() int { c.sdConn = integration.SDConn() defer c.sdConn.Close() c.network = &snet.SCIONNetwork{ - SCMPHandler: snet.DefaultSCMPHandler{ - RevocationHandler: daemon.RevHandler{Connector: c.sdConn}, - SCMPErrors: scmpErrorsCounter, - }, PacketConnMetrics: scionPacketConnMetrics, Topology: c.sdConn, } @@ -301,7 +296,12 @@ func (c *client) ping(ctx context.Context, n int, path snet.Path) (func(), error return nil, serrors.WrapStr("packing ping", err) } log.FromCtx(ctx).Info("Dialing", "remote", remote) - c.conn, err = c.network.Dial(ctx, "udp", integration.Local.Host, &remote) + scmpHandler := snet.DefaultSCMPHandler{ + RevocationHandler: daemon.RevHandler{Connector: c.sdConn}, + SCMPErrors: scmpErrorsCounter, + } + c.conn, err = c.network.Dial(ctx, "udp", integration.Local.Host, &remote, + snet.WithSCMPHandler(scmpHandler)) if err != nil { return nil, serrors.WrapStr("dialing conn", err) } @@ -380,7 +380,7 @@ func (c *client) pong(ctx context.Context) error { return serrors.WrapStr("setting read deadline", err) } rawPld := make([]byte, common.MaxMTU) - n, serverAddr, err := readFrom(c.conn, rawPld) + n, serverAddr, err := c.conn.ReadFrom(rawPld) if err != nil { return serrors.WrapStr("reading packet", err) } @@ -409,16 +409,3 @@ func getDeadline(ctx context.Context) time.Time { } return dl } - -func readFrom(conn *snet.Conn, pld []byte) (int, net.Addr, error) { - n, remoteAddr, err := conn.ReadFrom(pld) - // Attach more context to error - var opErr *snet.OpError - if !(errors.As(err, &opErr) && opErr.RevInfo() != nil) { - return n, remoteAddr, err - } - return n, remoteAddr, serrors.WithCtx(err, - "isd_as", opErr.RevInfo().IA(), - "interface", opErr.RevInfo().IfID, - ) -} From e532af2c43105b061808156ce0caf7da13a6992f Mon Sep 17 00:00:00 2001 From: Matthias Frei Date: Tue, 2 Jul 2024 14:33:12 +0200 Subject: [PATCH 2/8] WIP: return error from handler, but default handler never errors --- gateway/pathhealth/scmp.go | 3 +-- pkg/snet/reader.go | 4 +--- pkg/snet/scmp.go | 26 +++++++++++++------------- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/gateway/pathhealth/scmp.go b/gateway/pathhealth/scmp.go index 2b310c393d..e44bf4edc9 100644 --- a/gateway/pathhealth/scmp.go +++ b/gateway/pathhealth/scmp.go @@ -16,7 +16,6 @@ package pathhealth import ( "github.com/scionproto/scion/pkg/addr" - "github.com/scionproto/scion/pkg/private/serrors" "github.com/scionproto/scion/pkg/snet" ) @@ -33,7 +32,7 @@ type scmpHandler struct { func (h scmpHandler) Handle(pkt *snet.Packet) error { if pkt.Payload == nil { - return serrors.New("no payload found") + return nil } tr, ok := pkt.Payload.(snet.SCMPTracerouteReply) if !ok { diff --git a/pkg/snet/reader.go b/pkg/snet/reader.go index d11b088f99..50a083b8d3 100644 --- a/pkg/snet/reader.go +++ b/pkg/snet/reader.go @@ -131,13 +131,11 @@ func (c *scionConnReader) readPacketUDP(pkt *Packet, lastHop *net.UDPAddr) error return nil case SCMPPayload: if c.scmpHandler == nil { - // TODO metrics - // metrics.CounterInc(c.Metrics.SCMPErrors) continue } err := c.scmpHandler.Handle(pkt) if err != nil { - // metrics.CounterInc(c.Metrics.SCMPErrors) + return err } continue default: diff --git a/pkg/snet/scmp.go b/pkg/snet/scmp.go index 3b7d18183c..b9e15fc230 100644 --- a/pkg/snet/scmp.go +++ b/pkg/snet/scmp.go @@ -21,7 +21,6 @@ import ( "github.com/scionproto/scion/pkg/metrics/v2" "github.com/scionproto/scion/pkg/private/common" "github.com/scionproto/scion/pkg/private/ctrl/path_mgmt" - "github.com/scionproto/scion/pkg/private/serrors" "github.com/scionproto/scion/pkg/private/util" "github.com/scionproto/scion/pkg/slayers" ) @@ -32,25 +31,27 @@ type RevocationHandler interface { Revoke(ctx context.Context, revInfo *path_mgmt.RevInfo) error } -// SCMPHandler customizes the way snet connections deal with SCMP. +// SCMPHandler customizes the way snet.Conn deals with SCMP messages during Read/ReadFrom. type SCMPHandler interface { - // Handle processes the packet as an SCMP packet. If packet is not SCMP or otherwise - // malformed, it returns an error. - // Errors may be counted in metrics, but otherwise ignored. + // Handle processes the packet as an SCMP packet. // - // If the handler mutates the packet, the changes are seen by snet - // connection method callers. + // If the handler returns an error, snet.Conn.Read/ReadFrom will + // abort and propagate the error back to the caller. + // A packet that is not an SCMP or that is otherwise invalid or unexpected + // should be ignored without error. Handle(pkt *Packet) error } // DefaultSCMPHandler handles SCMP messages received from the network. If a // revocation handler is configured, it is informed of any received interface // down messages. +// It never returns an error "in line", so snet.Conn.Read/ReadFrom will not be +// aware of SCMP errors. Use the RevocationHandler to react to path failures. type DefaultSCMPHandler struct { // RevocationHandler manages revocations received via SCMP. If nil, the // handler is not called. RevocationHandler RevocationHandler - // SCMPErrors reports the total number of SCMP Errors encountered. + // SCMPErrors reports the total number of SCMP Error messages encountered. SCMPErrors metrics.Counter // Log is an optional function that is used to log SCMP messages // TODO log... @@ -60,7 +61,7 @@ type DefaultSCMPHandler struct { func (h DefaultSCMPHandler) Handle(pkt *Packet) error { scmp, ok := pkt.Payload.(SCMPPayload) if !ok { - return serrors.New("scmp handler invoked with non-scmp packet", "pkt", pkt) + return nil } typeCode := slayers.CreateSCMPTypeCode(scmp.Type(), scmp.Code()) if !typeCode.InfoMsg() { @@ -69,7 +70,7 @@ func (h DefaultSCMPHandler) Handle(pkt *Packet) error { switch scmp.Type() { case slayers.SCMPTypeExternalInterfaceDown: msg := pkt.Payload.(SCMPExternalInterfaceDown) - return h.handleSCMPRev(&path_mgmt.RevInfo{ + h.handleSCMPRev(&path_mgmt.RevInfo{ IfID: common.IFIDType(msg.Interface), RawIsdas: msg.IA, RawTimestamp: util.TimeToSecs(time.Now()), @@ -77,17 +78,16 @@ func (h DefaultSCMPHandler) Handle(pkt *Packet) error { }) case slayers.SCMPTypeInternalConnectivityDown: msg := pkt.Payload.(SCMPInternalConnectivityDown) - return h.handleSCMPRev(&path_mgmt.RevInfo{ + h.handleSCMPRev(&path_mgmt.RevInfo{ IfID: common.IFIDType(msg.Egress), RawIsdas: msg.IA, RawTimestamp: util.TimeToSecs(time.Now()), RawTTL: 10, }) - default: h.log("Ignoring SCMP packet", "scmp", typeCode, "src", pkt.Source) - return nil } + return nil } func (h DefaultSCMPHandler) log(msg string, ctx ...any) { From 02eaf699c2a3c55c1cf1f011461b64ece49f2cf5 Mon Sep 17 00:00:00 2001 From: Matthias Frei Date: Tue, 2 Jul 2024 14:49:59 +0200 Subject: [PATCH 3/8] WIP gateway simplify not a handler --- gateway/pathhealth/pathwatcher.go | 29 ++++++++++++++++------------- gateway/pathhealth/scmp.go | 22 ---------------------- 2 files changed, 16 insertions(+), 35 deletions(-) diff --git a/gateway/pathhealth/pathwatcher.go b/gateway/pathhealth/pathwatcher.go index 68529bbccd..7804ef37aa 100644 --- a/gateway/pathhealth/pathwatcher.go +++ b/gateway/pathhealth/pathwatcher.go @@ -98,12 +98,9 @@ func (f *DefaultPathWatcherFactory) New( IA: f.LocalIA, Host: addr.HostIP(f.LocalIP), }, - scmpHandler: scmpHandler{ - wrappedHandler: snet.DefaultSCMPHandler{ - RevocationHandler: f.RevocationHandler, - SCMPErrors: f.SCMPErrors, - }, - pkts: pktChan, + scmpHandler: snet.DefaultSCMPHandler{ + RevocationHandler: f.RevocationHandler, + SCMPErrors: f.SCMPErrors, }, pktChan: pktChan, probesSent: createCounter(f.ProbesSent, remote), @@ -122,7 +119,7 @@ type pathWatcher struct { // conn is the packet conn used to send probes on. The pathwatcher takes // ownership and will close it on termination. conn snet.PacketConn - // scmpHandler is handles SCMPs received on conn and inserts traceroute packets into pktChan + // Handler for non-traceroute SCMP packets received on conn scmpHandler snet.SCMPHandler // id is used as SCMP traceroute ID. Since each pathwatcher should have it's // own high port this value can be random. @@ -131,7 +128,7 @@ type pathWatcher struct { localAddr snet.SCIONAddress // pktChan is the channel which provides the incoming packets on the // connection. - pktChan <-chan traceroutePkt + pktChan chan traceroutePkt probesSent metrics.Counter probesReceived metrics.Counter @@ -243,11 +240,17 @@ func (w *pathWatcher) drainConn(ctx context.Context) { if err != nil { logger.Info("Unexpected error when reading probe reply", "err", err) } - if _, ok := pkt.Payload.(snet.SCMPPayload); !ok { - continue - } - if err := w.scmpHandler.Handle(&pkt); err == nil { - logger.Info("Unexpected error when handling SCMP packet", "err", err) + switch pld := pkt.Payload.(type) { + case snet.SCMPTracerouteReply: + w.pktChan <- traceroutePkt{ + Remote: pkt.Source.IA, + Identifier: pld.Identifier, + Sequence: pld.Sequence, + } + case snet.SCMPPayload: + if w.scmpHandler != nil { + w.scmpHandler.Handle(&pkt) + } } } } diff --git a/gateway/pathhealth/scmp.go b/gateway/pathhealth/scmp.go index e44bf4edc9..021c6b522c 100644 --- a/gateway/pathhealth/scmp.go +++ b/gateway/pathhealth/scmp.go @@ -16,7 +16,6 @@ package pathhealth import ( "github.com/scionproto/scion/pkg/addr" - "github.com/scionproto/scion/pkg/snet" ) type traceroutePkt struct { @@ -24,24 +23,3 @@ type traceroutePkt struct { Identifier uint16 Sequence uint16 } - -type scmpHandler struct { - wrappedHandler snet.SCMPHandler - pkts chan<- traceroutePkt -} - -func (h scmpHandler) Handle(pkt *snet.Packet) error { - if pkt.Payload == nil { - return nil - } - tr, ok := pkt.Payload.(snet.SCMPTracerouteReply) - if !ok { - return h.wrappedHandler.Handle(pkt) - } - h.pkts <- traceroutePkt{ - Remote: pkt.Source.IA, - Identifier: tr.Identifier, - Sequence: tr.Sequence, - } - return nil -} From 86de517a6f6025b786dd87e7163a8ee9551b7d55 Mon Sep 17 00:00:00 2001 From: Matthias Frei Date: Wed, 3 Jul 2024 09:48:42 +0200 Subject: [PATCH 4/8] WIP: reintroduce default SCMP Handler in SCIONNetwork --- gateway/gateway.go | 44 +++++++++++++------------------ gateway/pathhealth/pathwatcher.go | 2 +- pkg/snet/conn.go | 22 +++++++--------- pkg/snet/snet.go | 26 ++++++++++++++---- scion-pki/certs/renew.go | 9 +++---- tools/end2end/main.go | 22 +++++++--------- 6 files changed, 65 insertions(+), 60 deletions(-) diff --git a/gateway/gateway.go b/gateway/gateway.go index 26da091fc5..92fb14c99c 100644 --- a/gateway/gateway.go +++ b/gateway/gateway.go @@ -94,13 +94,12 @@ func (dpf DataplaneSessionFactory) New(id uint8, policyID int, type PacketConnFactory struct { Network *snet.SCIONNetwork Addr *net.UDPAddr - Options []snet.ConnOption } func (pcf PacketConnFactory) New() (net.PacketConn, error) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() - conn, err := pcf.Network.Listen(ctx, "udp", pcf.Addr, pcf.Options...) + conn, err := pcf.Network.Listen(ctx, "udp", pcf.Addr) if err != nil { return nil, serrors.WrapStr("creating packet conn", err) } @@ -408,15 +407,15 @@ func (g *Gateway) Run(ctx context.Context) error { } scionNetwork := &snet.SCIONNetwork{ - Topology: g.Daemon, + Topology: g.Daemon, + SCMPHandler: snet.DefaultSCMPHandler{ + RevocationHandler: revocationHandler, + SCMPErrors: g.Metrics.SCMPErrors, + Log: log.FromCtx(ctx).Debug, + }, PacketConnMetrics: g.Metrics.SCIONPacketConnMetrics, Metrics: g.Metrics.SCIONNetworkMetrics, } - scmpHandler := snet.DefaultSCMPHandler{ - RevocationHandler: revocationHandler, - SCMPErrors: g.Metrics.SCMPErrors, - Log: log.FromCtx(ctx).Debug, - } // Initialize the UDP/SCION QUIC conn for outgoing Gateway Discovery RPCs and outgoing Prefix // Fetching. Open up a random high port for this. @@ -424,7 +423,6 @@ func (g *Gateway) Run(ctx context.Context) error { context.TODO(), "udp", &net.UDPAddr{IP: g.ControlClientIP}, - snet.WithSCMPHandler(scmpHandler), ) if err != nil { return serrors.WrapStr("unable to initialize client QUIC connection", err) @@ -507,7 +505,6 @@ func (g *Gateway) Run(ctx context.Context) error { context.TODO(), "udp", g.ControlServerAddr, - snet.WithSCMPHandler(scmpHandler), ) if err != nil { return serrors.WrapStr("unable to initialize server QUIC connection", err) @@ -555,8 +552,7 @@ func (g *Gateway) Run(ctx context.Context) error { // received from the session monitors of the remote gateway. // ********************************************************************************* - probeConn, err := scionNetwork.Listen(context.TODO(), "udp", g.ProbeServerAddr, - snet.WithSCMPHandler(scmpHandler)) + probeConn, err := scionNetwork.Listen(context.TODO(), "udp", g.ProbeServerAddr) if err != nil { return serrors.WrapStr("creating server probe conn", err) } @@ -571,16 +567,8 @@ func (g *Gateway) Run(ctx context.Context) error { }() // Start dataplane ingress - dataplaneServerConn, err := scionNetwork.Listen( - context.TODO(), - "udp", - g.DataServerAddr, - snet.WithSCMPHandler(scmpHandler), - ) - if err != nil { - return serrors.WrapStr("creating ingress conn", err) - } - if err := StartIngress(ctx, dataplaneServerConn, deviceManager, g.Metrics); err != nil { + if err := StartIngress(ctx, scionNetwork, g.DataServerAddr, deviceManager, + g.Metrics); err != nil { return err } @@ -623,14 +611,12 @@ func (g *Gateway) Run(ctx context.Context) error { ProbeConnFactory: PacketConnFactory{ Network: scionNetwork, Addr: &net.UDPAddr{IP: g.ProbeClientIP}, - Options: []snet.ConnOption{snet.WithSCMPHandler(scmpHandler)}, }, DeviceManager: deviceManager, DataplaneSessionFactory: DataplaneSessionFactory{ PacketConnFactory: PacketConnFactory{ Network: scionNetwork, Addr: &net.UDPAddr{IP: g.DataClientIP}, - Options: []snet.ConnOption{snet.WithSCMPHandler(scmpHandler)}, }, Metrics: CreateSessionMetrics(g.Metrics), }, @@ -756,10 +742,18 @@ func CreateIngressMetrics(m *Metrics) dataplane.IngressMetrics { } } -func StartIngress(ctx context.Context, dataplaneServerConn *snet.Conn, +func StartIngress(ctx context.Context, scionNetwork *snet.SCIONNetwork, dataAddr *net.UDPAddr, deviceManager control.DeviceManager, metrics *Metrics) error { logger := log.FromCtx(ctx) + dataplaneServerConn, err := scionNetwork.Listen( + context.TODO(), + "udp", + dataAddr, + ) + if err != nil { + return serrors.WrapStr("creating ingress conn", err) + } ingressMetrics := CreateIngressMetrics(metrics) ingressServer := &dataplane.IngressServer{ Conn: dataplaneServerConn, diff --git a/gateway/pathhealth/pathwatcher.go b/gateway/pathhealth/pathwatcher.go index 7804ef37aa..f74ac371dc 100644 --- a/gateway/pathhealth/pathwatcher.go +++ b/gateway/pathhealth/pathwatcher.go @@ -119,7 +119,7 @@ type pathWatcher struct { // conn is the packet conn used to send probes on. The pathwatcher takes // ownership and will close it on termination. conn snet.PacketConn - // Handler for non-traceroute SCMP packets received on conn + // scmpHandler handles non-traceroute SCMP packets received on conn scmpHandler snet.SCMPHandler // id is used as SCMP traceroute ID. Since each pathwatcher should have it's // own high port this value can be random. diff --git a/pkg/snet/conn.go b/pkg/snet/conn.go index 5506ac522d..658687544e 100644 --- a/pkg/snet/conn.go +++ b/pkg/snet/conn.go @@ -116,23 +116,21 @@ func (c *Conn) Close() error { } // ConnOption is a functional option type for configuring a Conn. -type ConnOption func(o *options) +type ConnOption func(o *connOptions) // WithReplyPather sets the reply pather for the connection. // The reply pather is responsible for determining the path to send replies to. // If this option is not provided, DefaultReplyPather is used. func WithReplyPather(replyPather ReplyPather) ConnOption { - return func(o *options) { + return func(o *connOptions) { o.replyPather = replyPather } } // WithSCMPHandler sets the SCMP handler for the connection. // The SCMP handler is a callback to react to SCMP messages, specifically to error messages. -// If this option is not provided, no SCMP handler is used (equivalent to -// DefaultSCMPHandler with an empty RevocationHandler). func WithSCMPHandler(scmpHandler SCMPHandler) ConnOption { - return func(o *options) { + return func(o *connOptions) { o.scmpHandler = scmpHandler } } @@ -140,24 +138,24 @@ func WithSCMPHandler(scmpHandler SCMPHandler) ConnOption { // WithRemote sets the remote address for the connection. // This only applies to NewCookedConn, but not Dial/Listen. func WithRemote(addr *UDPAddr) ConnOption { - return func(o *options) { + return func(o *connOptions) { o.remote = addr } } -type options struct { +type connOptions struct { replyPather ReplyPather scmpHandler SCMPHandler remote *UDPAddr } -func apply(opts []ConnOption) options { - o := options{ - replyPather: DefaultReplyPather{}, - scmpHandler: nil, - } +func apply(opts []ConnOption) connOptions { + o := connOptions{} for _, option := range opts { option(&o) } + if o.replyPather == nil { + o.replyPather = DefaultReplyPather{} + } return o } diff --git a/pkg/snet/snet.go b/pkg/snet/snet.go index cffa1f5029..260072bb3a 100644 --- a/pkg/snet/snet.go +++ b/pkg/snet/snet.go @@ -35,7 +35,6 @@ import ( "errors" "net" "net/netip" - "slices" "syscall" "github.com/scionproto/scion/pkg/addr" @@ -66,7 +65,10 @@ type SCIONNetwork struct { // traffic. Topology Topology - // XXX: SCMPHandler / Reverser + // SCMPHandler is the SCMPHandler used by default for Conns opened with Dial/Listen. + SCMPHandler SCMPHandler + // ReplyPather is the ReplyPather used by default for Conns opened with Dial/Listen. + ReplyPather ReplyPather Metrics SCIONNetworkMetrics PacketConnMetrics SCIONPacketConnMetrics @@ -146,8 +148,15 @@ func (n *SCIONNetwork) Dial( return nil, err } log.FromCtx(ctx).Debug("UDP socket opened on", "addr", packetConn.LocalAddr(), "to", remote) - options = append(slices.Clone(options), WithRemote(remote)) - return NewCookedConn(packetConn, n.Topology, options...) + return NewCookedConn( + packetConn, + n.Topology, + append([]ConnOption{ + WithSCMPHandler(n.SCMPHandler), + WithReplyPather(n.ReplyPather), + WithRemote(remote), + }, options...)..., + ) } // Listen opens a Conn. The returned connection's ReadFrom and WriteTo methods @@ -173,7 +182,14 @@ func (n *SCIONNetwork) Listen( return nil, err } log.FromCtx(ctx).Debug("UDP socket openned on", "addr", packetConn.LocalAddr()) - return NewCookedConn(packetConn, n.Topology, options...) + return NewCookedConn( + packetConn, + n.Topology, + append([]ConnOption{ + WithSCMPHandler(n.SCMPHandler), + WithReplyPather(n.ReplyPather), + }, options...)..., + ) } func listenUDPRange(addr *net.UDPAddr, start, end uint16) (*net.UDPConn, error) { diff --git a/scion-pki/certs/renew.go b/scion-pki/certs/renew.go index c89970055d..310f7ad619 100644 --- a/scion-pki/certs/renew.go +++ b/scion-pki/certs/renew.go @@ -738,12 +738,12 @@ func (r *renewer) requestRemote( sn := &snet.SCIONNetwork{ Topology: r.Daemon, + SCMPHandler: snet.DefaultSCMPHandler{ + RevocationHandler: daemon.RevHandler{Connector: r.Daemon}, + }, } - scmpHandler := snet.DefaultSCMPHandler{ - RevocationHandler: daemon.RevHandler{Connector: r.Daemon}, - } - conn, err := sn.Listen(ctx, "udp", local.Host, snet.WithSCMPHandler(scmpHandler)) + conn, err := sn.Listen(ctx, "udp", local.Host) if err != nil { return nil, serrors.WrapStr("dialing", err) } @@ -758,7 +758,6 @@ func (r *renewer) requestRemote( Resolver: &svc.Resolver{ LocalIA: local.IA, Network: sn, - // XXX scmp handler lost here LocalIP: local.Host.IP, }, }, diff --git a/tools/end2end/main.go b/tools/end2end/main.go index aec40fb4b2..6645589525 100644 --- a/tools/end2end/main.go +++ b/tools/end2end/main.go @@ -133,15 +133,14 @@ func (s server) run() { sdConn := integration.SDConn() defer sdConn.Close() sn := &snet.SCIONNetwork{ + SCMPHandler: snet.DefaultSCMPHandler{ + RevocationHandler: daemon.RevHandler{Connector: sdConn}, + SCMPErrors: scmpErrorsCounter, + }, PacketConnMetrics: scionPacketConnMetrics, Topology: sdConn, } - scmpHandler := snet.DefaultSCMPHandler{ - RevocationHandler: daemon.RevHandler{Connector: sdConn}, - SCMPErrors: scmpErrorsCounter, - } - conn, err := sn.Listen(context.Background(), "udp", integration.Local.Host, - snet.WithSCMPHandler(scmpHandler)) + conn, err := sn.Listen(context.Background(), "udp", integration.Local.Host) if err != nil { integration.LogFatal("Error listening", "err", err) } @@ -233,6 +232,10 @@ func (c *client) run() int { c.sdConn = integration.SDConn() defer c.sdConn.Close() c.network = &snet.SCIONNetwork{ + SCMPHandler: snet.DefaultSCMPHandler{ + RevocationHandler: daemon.RevHandler{Connector: c.sdConn}, + SCMPErrors: scmpErrorsCounter, + }, PacketConnMetrics: scionPacketConnMetrics, Topology: c.sdConn, } @@ -296,12 +299,7 @@ func (c *client) ping(ctx context.Context, n int, path snet.Path) (func(), error return nil, serrors.WrapStr("packing ping", err) } log.FromCtx(ctx).Info("Dialing", "remote", remote) - scmpHandler := snet.DefaultSCMPHandler{ - RevocationHandler: daemon.RevHandler{Connector: c.sdConn}, - SCMPErrors: scmpErrorsCounter, - } - c.conn, err = c.network.Dial(ctx, "udp", integration.Local.Host, &remote, - snet.WithSCMPHandler(scmpHandler)) + c.conn, err = c.network.Dial(ctx, "udp", integration.Local.Host, &remote) if err != nil { return nil, serrors.WrapStr("dialing conn", err) } From 947235468d876404588b4fbc49c7ffdd2291d8bc Mon Sep 17 00:00:00 2001 From: Matthias Frei Date: Wed, 3 Jul 2024 10:01:20 +0200 Subject: [PATCH 5/8] WIP cleanup --- pkg/snet/reader.go | 12 ++++-------- scion-pki/certs/renew.go | 1 + 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/pkg/snet/reader.go b/pkg/snet/reader.go index 50a083b8d3..1b3e09f9cf 100644 --- a/pkg/snet/reader.go +++ b/pkg/snet/reader.go @@ -71,8 +71,7 @@ func (c *scionConnReader) read(b []byte) (int, *UDPAddr, error) { Bytes: Bytes(c.buffer), } var lastHop net.UDPAddr - err := c.readPacketUDP(&pkt, &lastHop) - if err != nil { + if err := c.readPacketUDP(&pkt, &lastHop); err != nil { return 0, nil, err } @@ -117,12 +116,11 @@ func (c *scionConnReader) read(b []byte) (int, *UDPAddr, error) { return n, remote, nil } -// readPacketUDP repeatedly reads a packet until a UDP datagram is found. +// readPacketUDP repeatedly reads a packet until a UDP datagram is found or an error occurs. // If an SCMP Handler is configured, it will be called on SCMP messages. func (c *scionConnReader) readPacketUDP(pkt *Packet, lastHop *net.UDPAddr) error { for { - err := c.conn.ReadFrom(pkt, lastHop) - if err != nil { + if err := c.conn.ReadFrom(pkt, lastHop); err != nil { return err } @@ -133,11 +131,9 @@ func (c *scionConnReader) readPacketUDP(pkt *Packet, lastHop *net.UDPAddr) error if c.scmpHandler == nil { continue } - err := c.scmpHandler.Handle(pkt) - if err != nil { + if err := c.scmpHandler.Handle(pkt); err != nil { return err } - continue default: continue } diff --git a/scion-pki/certs/renew.go b/scion-pki/certs/renew.go index 310f7ad619..b6d59068d8 100644 --- a/scion-pki/certs/renew.go +++ b/scion-pki/certs/renew.go @@ -740,6 +740,7 @@ func (r *renewer) requestRemote( Topology: r.Daemon, SCMPHandler: snet.DefaultSCMPHandler{ RevocationHandler: daemon.RevHandler{Connector: r.Daemon}, + Log: log.FromCtx(ctx).Debug, }, } From 7441cde5a9e1336c4d1cb2aaca073246029405b1 Mon Sep 17 00:00:00 2001 From: Matthias Frei Date: Wed, 3 Jul 2024 11:33:20 +0200 Subject: [PATCH 6/8] snet: expose setting buffer sizes for benefit of quic --- pkg/snet/conn.go | 10 ++++++++++ pkg/snet/packet_conn.go | 10 ++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/pkg/snet/conn.go b/pkg/snet/conn.go index 658687544e..2cd3ac12dc 100644 --- a/pkg/snet/conn.go +++ b/pkg/snet/conn.go @@ -101,6 +101,16 @@ func (c *Conn) SyscallConn() (syscall.RawConn, error) { return c.conn.SyscallConn() } +func (c *Conn) SetReadBuffer(n int) error { + c.conn.SetReadBuffer(n) + return nil +} + +func (c *Conn) SetWriteBuffer(n int) error { + c.conn.SetWriteBuffer(n) + return nil +} + func (c *Conn) SetDeadline(t time.Time) error { if err := c.scionConnReader.SetReadDeadline(t); err != nil { return err diff --git a/pkg/snet/packet_conn.go b/pkg/snet/packet_conn.go index 1d5100d474..0569f01736 100644 --- a/pkg/snet/packet_conn.go +++ b/pkg/snet/packet_conn.go @@ -40,6 +40,8 @@ type PacketConn interface { SetWriteDeadline(t time.Time) error SetDeadline(t time.Time) error SyscallConn() (syscall.RawConn, error) + SetReadBuffer(int) error + SetWriteBuffer(int) error LocalAddr() net.Addr Close() error } @@ -104,10 +106,6 @@ type SCIONPacketConn struct { interfaceMap interfaceMap } -func (c *SCIONPacketConn) SetReadBuffer(bytes int) error { - return c.Conn.SetReadBuffer(bytes) -} - func (c *SCIONPacketConn) SetDeadline(d time.Time) error { return c.Conn.SetDeadline(d) } @@ -153,6 +151,10 @@ func (c *SCIONPacketConn) SyscallConn() (syscall.RawConn, error) { return c.Conn.SyscallConn() } +func (c *SCIONPacketConn) SetReadBuffer(n int) error { + return c.Conn.SetReadBuffer(n) +} + func (c *SCIONPacketConn) readFrom(pkt *Packet) (*net.UDPAddr, error) { pkt.Prepare() n, remoteAddr, err := c.Conn.ReadFrom(pkt.Bytes) From eb310ce88a45d3a23d811c80f301564c0e3cbe14 Mon Sep 17 00:00:00 2001 From: Matthias Frei Date: Wed, 3 Jul 2024 15:54:39 +0200 Subject: [PATCH 7/8] fixup! snet: expose setting buffer sizes for benefit of quic --- pkg/snet/conn.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/snet/conn.go b/pkg/snet/conn.go index 2cd3ac12dc..eb697ca3af 100644 --- a/pkg/snet/conn.go +++ b/pkg/snet/conn.go @@ -102,13 +102,11 @@ func (c *Conn) SyscallConn() (syscall.RawConn, error) { } func (c *Conn) SetReadBuffer(n int) error { - c.conn.SetReadBuffer(n) - return nil + return c.conn.SetReadBuffer(n) } func (c *Conn) SetWriteBuffer(n int) error { - c.conn.SetWriteBuffer(n) - return nil + return c.conn.SetWriteBuffer(n) } func (c *Conn) SetDeadline(t time.Time) error { From 5dadff6ff6cb923825b70d904453aa5788d488f5 Mon Sep 17 00:00:00 2001 From: Matthias Frei Date: Tue, 9 Jul 2024 09:18:59 +0200 Subject: [PATCH 8/8] snet: remove ancient revinfo struct, notify daemon async --- control/BUILD.bazel | 2 + control/revhandler.go | 19 +++++---- gateway/pathhealth/pathwatcher.go | 2 +- pkg/daemon/BUILD.bazel | 2 +- pkg/daemon/apitypes.go | 19 +++++++-- pkg/daemon/daemon.go | 3 +- pkg/daemon/grpc.go | 7 ++-- pkg/daemon/mock_daemon/BUILD.bazel | 1 - pkg/daemon/mock_daemon/mock.go | 9 ++--- pkg/snet/BUILD.bazel | 2 - pkg/snet/interface.go | 6 ++- pkg/snet/mock_snet/BUILD.bazel | 1 - pkg/snet/mock_snet/mock.go | 63 ++++++++++++++++++++++++------ pkg/snet/scmp.go | 58 +++++++++++++-------------- 14 files changed, 120 insertions(+), 74 deletions(-) diff --git a/control/BUILD.bazel b/control/BUILD.bazel index 57b4489eb5..4b651d242b 100644 --- a/control/BUILD.bazel +++ b/control/BUILD.bazel @@ -29,9 +29,11 @@ go_library( "//pkg/log:go_default_library", "//pkg/metrics:go_default_library", "//pkg/metrics/v2:go_default_library", + "//pkg/private/common:go_default_library", "//pkg/private/ctrl/path_mgmt:go_default_library", "//pkg/private/prom:go_default_library", "//pkg/private/serrors:go_default_library", + "//pkg/private/util:go_default_library", "//pkg/proto/hidden_segment:go_default_library", "//pkg/scrypto:go_default_library", "//pkg/scrypto/cppki:go_default_library", diff --git a/control/revhandler.go b/control/revhandler.go index 141eb06aa8..7cf97ff55d 100644 --- a/control/revhandler.go +++ b/control/revhandler.go @@ -16,9 +16,12 @@ package control import ( "context" + "time" + "github.com/scionproto/scion/pkg/addr" + "github.com/scionproto/scion/pkg/private/common" "github.com/scionproto/scion/pkg/private/ctrl/path_mgmt" - "github.com/scionproto/scion/pkg/private/serrors" + "github.com/scionproto/scion/pkg/private/util" "github.com/scionproto/scion/private/revcache" ) @@ -28,13 +31,13 @@ type RevocationHandler struct { RevCache revcache.RevCache } -func (h RevocationHandler) Revoke(ctx context.Context, revInfo *path_mgmt.RevInfo) error { - if _, err := h.RevCache.Insert(ctx, revInfo); err != nil { - return serrors.WrapStr("inserting revocation", err, - "isd_as", revInfo.IA(), - "interface_id", revInfo.IfID, - "expiration", revInfo.Expiration(), - ) +func (h RevocationHandler) Revoke(ia addr.IA, ingress, egress uint64) error { + revInfo := &path_mgmt.RevInfo{ + IfID: common.IFIDType(egress), + RawIsdas: ia, + RawTimestamp: util.TimeToSecs(time.Now()), + RawTTL: 10, } + _, _ = h.RevCache.Insert(context.TODO(), revInfo) return nil } diff --git a/gateway/pathhealth/pathwatcher.go b/gateway/pathhealth/pathwatcher.go index f74ac371dc..674d4cde55 100644 --- a/gateway/pathhealth/pathwatcher.go +++ b/gateway/pathhealth/pathwatcher.go @@ -249,7 +249,7 @@ func (w *pathWatcher) drainConn(ctx context.Context) { } case snet.SCMPPayload: if w.scmpHandler != nil { - w.scmpHandler.Handle(&pkt) + _ = w.scmpHandler.Handle(&pkt) } } } diff --git a/pkg/daemon/BUILD.bazel b/pkg/daemon/BUILD.bazel index 8a8ca23be8..b426f8bee1 100644 --- a/pkg/daemon/BUILD.bazel +++ b/pkg/daemon/BUILD.bazel @@ -15,9 +15,9 @@ go_library( "//pkg/daemon/internal/metrics:go_default_library", "//pkg/drkey:go_default_library", "//pkg/grpc:go_default_library", + "//pkg/log:go_default_library", "//pkg/metrics:go_default_library", "//pkg/private/common:go_default_library", - "//pkg/private/ctrl/path_mgmt:go_default_library", "//pkg/private/prom:go_default_library", "//pkg/private/serrors:go_default_library", "//pkg/proto/daemon:go_default_library", diff --git a/pkg/daemon/apitypes.go b/pkg/daemon/apitypes.go index 4b4564bef8..71364034aa 100644 --- a/pkg/daemon/apitypes.go +++ b/pkg/daemon/apitypes.go @@ -18,9 +18,10 @@ import ( "context" "math/rand" "net" + "time" "github.com/scionproto/scion/pkg/addr" - "github.com/scionproto/scion/pkg/private/ctrl/path_mgmt" + "github.com/scionproto/scion/pkg/log" "github.com/scionproto/scion/pkg/private/serrors" "github.com/scionproto/scion/pkg/snet" "github.com/scionproto/scion/private/topology" @@ -55,8 +56,20 @@ type RevHandler struct { Connector Connector } -func (h RevHandler) Revoke(ctx context.Context, revInfo *path_mgmt.RevInfo) error { - return h.Connector.RevNotification(ctx, revInfo) +func (h RevHandler) Revoke(ia addr.IA, ingress, egress uint64) error { + go func() { + defer log.HandlePanic() + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + // Note that this ignores the ingress interface. + // internal/external connectivity down messages are thus treated the same. + // Need to extend the beacon protocol to distinguish this. + err := h.Connector.RevNotification(ctx, ia, egress) + if err != nil { + log.Debug("Error asynchronously notifying daemon of revocation", "err", err) + } + }() + return nil } // TopoQuerier can be used to get topology information from the SCION Daemon. diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index a9ba3397ee..2e9440dbfe 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -24,7 +24,6 @@ import ( "github.com/scionproto/scion/pkg/daemon/internal/metrics" "github.com/scionproto/scion/pkg/drkey" libmetrics "github.com/scionproto/scion/pkg/metrics" - "github.com/scionproto/scion/pkg/private/ctrl/path_mgmt" "github.com/scionproto/scion/pkg/private/serrors" "github.com/scionproto/scion/pkg/snet" ) @@ -81,7 +80,7 @@ type Connector interface { // of URIs of the service in the local AS. SVCInfo(ctx context.Context, svcTypes []addr.SVC) (map[addr.SVC][]string, error) // RevNotification sends a RevocationInfo message to the daemon. - RevNotification(ctx context.Context, revInfo *path_mgmt.RevInfo) error + RevNotification(ctx context.Context, ia addr.IA, ifID uint64) error // DRKeyGetASHostKey requests a AS-Host Key from the daemon. DRKeyGetASHostKey(ctx context.Context, meta drkey.ASHostMeta) (drkey.ASHostKey, error) // DRKeyGetHostASKey requests a Host-AS Key from the daemon. diff --git a/pkg/daemon/grpc.go b/pkg/daemon/grpc.go index 4cbec6036c..cddb2b837e 100644 --- a/pkg/daemon/grpc.go +++ b/pkg/daemon/grpc.go @@ -28,7 +28,6 @@ import ( "github.com/scionproto/scion/pkg/drkey" libgrpc "github.com/scionproto/scion/pkg/grpc" "github.com/scionproto/scion/pkg/private/common" - "github.com/scionproto/scion/pkg/private/ctrl/path_mgmt" "github.com/scionproto/scion/pkg/private/serrors" sdpb "github.com/scionproto/scion/pkg/proto/daemon" dkpb "github.com/scionproto/scion/pkg/proto/drkey" @@ -165,11 +164,11 @@ func (c grpcConn) SVCInfo( return result, nil } -func (c grpcConn) RevNotification(ctx context.Context, revInfo *path_mgmt.RevInfo) error { +func (c grpcConn) RevNotification(ctx context.Context, ia addr.IA, ifID uint64) error { client := sdpb.NewDaemonServiceClient(c.conn) _, err := client.NotifyInterfaceDown(ctx, &sdpb.NotifyInterfaceDownRequest{ - Id: uint64(revInfo.IfID), - IsdAs: uint64(revInfo.RawIsdas), + Id: ifID, + IsdAs: uint64(ia), }) c.metrics.incIfDown(err) return err diff --git a/pkg/daemon/mock_daemon/BUILD.bazel b/pkg/daemon/mock_daemon/BUILD.bazel index 3423d8d8b0..ba17e1cfc7 100644 --- a/pkg/daemon/mock_daemon/BUILD.bazel +++ b/pkg/daemon/mock_daemon/BUILD.bazel @@ -18,7 +18,6 @@ go_library( "//pkg/addr:go_default_library", "//pkg/daemon:go_default_library", "//pkg/drkey:go_default_library", - "//pkg/private/ctrl/path_mgmt:go_default_library", "//pkg/snet:go_default_library", "@com_github_golang_mock//gomock:go_default_library", ], diff --git a/pkg/daemon/mock_daemon/mock.go b/pkg/daemon/mock_daemon/mock.go index 05a868518c..9d359b6f8c 100644 --- a/pkg/daemon/mock_daemon/mock.go +++ b/pkg/daemon/mock_daemon/mock.go @@ -13,7 +13,6 @@ import ( addr "github.com/scionproto/scion/pkg/addr" daemon "github.com/scionproto/scion/pkg/daemon" drkey "github.com/scionproto/scion/pkg/drkey" - path_mgmt "github.com/scionproto/scion/pkg/private/ctrl/path_mgmt" snet "github.com/scionproto/scion/pkg/snet" ) @@ -176,17 +175,17 @@ func (mr *MockConnectorMockRecorder) PortRange(arg0 interface{}) *gomock.Call { } // RevNotification mocks base method. -func (m *MockConnector) RevNotification(arg0 context.Context, arg1 *path_mgmt.RevInfo) error { +func (m *MockConnector) RevNotification(arg0 context.Context, arg1 addr.IA, arg2 uint64) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "RevNotification", arg0, arg1) + ret := m.ctrl.Call(m, "RevNotification", arg0, arg1, arg2) ret0, _ := ret[0].(error) return ret0 } // RevNotification indicates an expected call of RevNotification. -func (mr *MockConnectorMockRecorder) RevNotification(arg0, arg1 interface{}) *gomock.Call { +func (mr *MockConnectorMockRecorder) RevNotification(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RevNotification", reflect.TypeOf((*MockConnector)(nil).RevNotification), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RevNotification", reflect.TypeOf((*MockConnector)(nil).RevNotification), arg0, arg1, arg2) } // SVCInfo mocks base method. diff --git a/pkg/snet/BUILD.bazel b/pkg/snet/BUILD.bazel index 568cc98d7c..3bdb247013 100644 --- a/pkg/snet/BUILD.bazel +++ b/pkg/snet/BUILD.bazel @@ -24,9 +24,7 @@ go_library( "//pkg/log:go_default_library", "//pkg/metrics/v2:go_default_library", "//pkg/private/common:go_default_library", - "//pkg/private/ctrl/path_mgmt:go_default_library", "//pkg/private/serrors:go_default_library", - "//pkg/private/util:go_default_library", "//pkg/slayers:go_default_library", "//pkg/slayers/path:go_default_library", "//pkg/slayers/path/empty:go_default_library", diff --git a/pkg/snet/interface.go b/pkg/snet/interface.go index 9ae994db3c..34ffcbff4d 100644 --- a/pkg/snet/interface.go +++ b/pkg/snet/interface.go @@ -22,6 +22,8 @@ import ( type Network interface { OpenRaw(ctx context.Context, addr *net.UDPAddr) (PacketConn, error) - Listen(ctx context.Context, network string, listen *net.UDPAddr, options ...ConnOption) (*Conn, error) - Dial(ctx context.Context, network string, listen *net.UDPAddr, remote *UDPAddr, options ...ConnOption) (*Conn, error) + Listen(ctx context.Context, network string, listen *net.UDPAddr, + options ...ConnOption) (*Conn, error) + Dial(ctx context.Context, network string, listen *net.UDPAddr, remote *UDPAddr, + options ...ConnOption) (*Conn, error) } diff --git a/pkg/snet/mock_snet/BUILD.bazel b/pkg/snet/mock_snet/BUILD.bazel index 9ca0f74569..683adb65ed 100644 --- a/pkg/snet/mock_snet/BUILD.bazel +++ b/pkg/snet/mock_snet/BUILD.bazel @@ -23,7 +23,6 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/addr:go_default_library", - "//pkg/private/ctrl/path_mgmt:go_default_library", "//pkg/snet:go_default_library", "@com_github_golang_mock//gomock:go_default_library", ], diff --git a/pkg/snet/mock_snet/mock.go b/pkg/snet/mock_snet/mock.go index c4a1d854b1..4f2ca5355a 100644 --- a/pkg/snet/mock_snet/mock.go +++ b/pkg/snet/mock_snet/mock.go @@ -13,7 +13,6 @@ import ( gomock "github.com/golang/mock/gomock" addr "github.com/scionproto/scion/pkg/addr" - path_mgmt "github.com/scionproto/scion/pkg/private/ctrl/path_mgmt" snet "github.com/scionproto/scion/pkg/snet" ) @@ -41,33 +40,43 @@ func (m *MockNetwork) EXPECT() *MockNetworkMockRecorder { } // Dial mocks base method. -func (m *MockNetwork) Dial(arg0 context.Context, arg1 string, arg2 *net.UDPAddr, arg3 *snet.UDPAddr) (*snet.Conn, error) { +func (m *MockNetwork) Dial(arg0 context.Context, arg1 string, arg2 *net.UDPAddr, arg3 *snet.UDPAddr, arg4 ...snet.ConnOption) (*snet.Conn, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Dial", arg0, arg1, arg2, arg3) + varargs := []interface{}{arg0, arg1, arg2, arg3} + for _, a := range arg4 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "Dial", varargs...) ret0, _ := ret[0].(*snet.Conn) ret1, _ := ret[1].(error) return ret0, ret1 } // Dial indicates an expected call of Dial. -func (mr *MockNetworkMockRecorder) Dial(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { +func (mr *MockNetworkMockRecorder) Dial(arg0, arg1, arg2, arg3 interface{}, arg4 ...interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Dial", reflect.TypeOf((*MockNetwork)(nil).Dial), arg0, arg1, arg2, arg3) + varargs := append([]interface{}{arg0, arg1, arg2, arg3}, arg4...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Dial", reflect.TypeOf((*MockNetwork)(nil).Dial), varargs...) } // Listen mocks base method. -func (m *MockNetwork) Listen(arg0 context.Context, arg1 string, arg2 *net.UDPAddr) (*snet.Conn, error) { +func (m *MockNetwork) Listen(arg0 context.Context, arg1 string, arg2 *net.UDPAddr, arg3 ...snet.ConnOption) (*snet.Conn, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Listen", arg0, arg1, arg2) + varargs := []interface{}{arg0, arg1, arg2} + for _, a := range arg3 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "Listen", varargs...) ret0, _ := ret[0].(*snet.Conn) ret1, _ := ret[1].(error) return ret0, ret1 } // Listen indicates an expected call of Listen. -func (mr *MockNetworkMockRecorder) Listen(arg0, arg1, arg2 interface{}) *gomock.Call { +func (mr *MockNetworkMockRecorder) Listen(arg0, arg1, arg2 interface{}, arg3 ...interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Listen", reflect.TypeOf((*MockNetwork)(nil).Listen), arg0, arg1, arg2) + varargs := append([]interface{}{arg0, arg1, arg2}, arg3...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Listen", reflect.TypeOf((*MockNetwork)(nil).Listen), varargs...) } // OpenRaw mocks base method. @@ -164,6 +173,20 @@ func (mr *MockPacketConnMockRecorder) SetDeadline(arg0 interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetDeadline", reflect.TypeOf((*MockPacketConn)(nil).SetDeadline), arg0) } +// SetReadBuffer mocks base method. +func (m *MockPacketConn) SetReadBuffer(arg0 int) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetReadBuffer", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// SetReadBuffer indicates an expected call of SetReadBuffer. +func (mr *MockPacketConnMockRecorder) SetReadBuffer(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetReadBuffer", reflect.TypeOf((*MockPacketConn)(nil).SetReadBuffer), arg0) +} + // SetReadDeadline mocks base method. func (m *MockPacketConn) SetReadDeadline(arg0 time.Time) error { m.ctrl.T.Helper() @@ -178,6 +201,20 @@ func (mr *MockPacketConnMockRecorder) SetReadDeadline(arg0 interface{}) *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetReadDeadline", reflect.TypeOf((*MockPacketConn)(nil).SetReadDeadline), arg0) } +// SetWriteBuffer mocks base method. +func (m *MockPacketConn) SetWriteBuffer(arg0 int) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetWriteBuffer", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// SetWriteBuffer indicates an expected call of SetWriteBuffer. +func (mr *MockPacketConnMockRecorder) SetWriteBuffer(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetWriteBuffer", reflect.TypeOf((*MockPacketConn)(nil).SetWriteBuffer), arg0) +} + // SetWriteDeadline mocks base method. func (m *MockPacketConn) SetWriteDeadline(arg0 time.Time) error { m.ctrl.T.Helper() @@ -429,15 +466,15 @@ func (m *MockRevocationHandler) EXPECT() *MockRevocationHandlerMockRecorder { } // Revoke mocks base method. -func (m *MockRevocationHandler) Revoke(arg0 context.Context, arg1 *path_mgmt.RevInfo) error { +func (m *MockRevocationHandler) Revoke(arg0 addr.IA, arg1, arg2 uint64) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Revoke", arg0, arg1) + ret := m.ctrl.Call(m, "Revoke", arg0, arg1, arg2) ret0, _ := ret[0].(error) return ret0 } // Revoke indicates an expected call of Revoke. -func (mr *MockRevocationHandlerMockRecorder) Revoke(arg0, arg1 interface{}) *gomock.Call { +func (mr *MockRevocationHandlerMockRecorder) Revoke(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Revoke", reflect.TypeOf((*MockRevocationHandler)(nil).Revoke), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Revoke", reflect.TypeOf((*MockRevocationHandler)(nil).Revoke), arg0, arg1, arg2) } diff --git a/pkg/snet/scmp.go b/pkg/snet/scmp.go index b9e15fc230..5d70a73c10 100644 --- a/pkg/snet/scmp.go +++ b/pkg/snet/scmp.go @@ -15,20 +15,26 @@ package snet import ( - "context" - "time" - + "github.com/scionproto/scion/pkg/addr" "github.com/scionproto/scion/pkg/metrics/v2" - "github.com/scionproto/scion/pkg/private/common" - "github.com/scionproto/scion/pkg/private/ctrl/path_mgmt" - "github.com/scionproto/scion/pkg/private/util" "github.com/scionproto/scion/pkg/slayers" ) -// RevocationHandler is called by the default SCMP Handler whenever revocations are encountered. +// RevocationHandler is called by the default SCMP Handler whenever path +// revocations are encountered. type RevocationHandler interface { - // RevokeRaw handles a revocation received as raw bytes. - Revoke(ctx context.Context, revInfo *path_mgmt.RevInfo) error + // Revoke is called by the default SCMP handler whenever revocations (SCMP + // error messages "external interface down" and "internal connectivity down") are + // encountered. + // For "internal connectivity down", both ingress and egress interface values are defined. + // For "external interface down", ingress is set to 0 and egress is the affected interface. + // + // This is called in the packet receive loop and thus expensive blocking + // operations should be avoided or be triggered asynchronously. + // + // If the handler returns an error, snet.Conn.Read/ReadFrom will abort and + // propagate the error back to the caller. + Revoke(ia addr.IA, ingress, egress uint64) error } // SCMPHandler customizes the way snet.Conn deals with SCMP messages during Read/ReadFrom. @@ -54,7 +60,6 @@ type DefaultSCMPHandler struct { // SCMPErrors reports the total number of SCMP Error messages encountered. SCMPErrors metrics.Counter // Log is an optional function that is used to log SCMP messages - // TODO log... Log func(msg string, ctx ...any) } @@ -67,23 +72,23 @@ func (h DefaultSCMPHandler) Handle(pkt *Packet) error { if !typeCode.InfoMsg() { metrics.CounterInc(h.SCMPErrors) } + if h.RevocationHandler == nil { + h.log("Ignoring SCMP packet", "scmp", typeCode, "src", pkt.Source) + return nil + } + + // Handle revocation switch scmp.Type() { case slayers.SCMPTypeExternalInterfaceDown: msg := pkt.Payload.(SCMPExternalInterfaceDown) - h.handleSCMPRev(&path_mgmt.RevInfo{ - IfID: common.IFIDType(msg.Interface), - RawIsdas: msg.IA, - RawTimestamp: util.TimeToSecs(time.Now()), - RawTTL: 10, - }) + h.log("Handling SCMP ExternalInteraceDown", + "isd_as", msg.IA, "interface", msg.Interface) + return h.RevocationHandler.Revoke(msg.IA, 0, msg.Interface) case slayers.SCMPTypeInternalConnectivityDown: msg := pkt.Payload.(SCMPInternalConnectivityDown) - h.handleSCMPRev(&path_mgmt.RevInfo{ - IfID: common.IFIDType(msg.Egress), - RawIsdas: msg.IA, - RawTimestamp: util.TimeToSecs(time.Now()), - RawTTL: 10, - }) + h.log("Handling SCMP InternalConnectivityDown", + "isd_as", msg.IA, "ingress", msg.Ingress, "egress", msg.Egress) + return h.RevocationHandler.Revoke(msg.IA, msg.Ingress, msg.Egress) default: h.log("Ignoring SCMP packet", "scmp", typeCode, "src", pkt.Source) } @@ -96,12 +101,3 @@ func (h DefaultSCMPHandler) log(msg string, ctx ...any) { } h.Log(msg, ctx) } - -// TODO: matzf replace RevocationHandler interface with something that does not rely on ancient RevInfo struct -func (h *DefaultSCMPHandler) handleSCMPRev(revInfo *path_mgmt.RevInfo) error { - - if h.RevocationHandler != nil { - return h.RevocationHandler.Revoke(context.TODO(), revInfo) - } - return nil -}