diff --git a/gateway/pathhealth/remotewatcher.go b/gateway/pathhealth/remotewatcher.go index c503aae7eb..7ee9baeb51 100644 --- a/gateway/pathhealth/remotewatcher.go +++ b/gateway/pathhealth/remotewatcher.go @@ -17,7 +17,6 @@ package pathhealth import ( "context" "fmt" - "math/rand" "sync" "time" @@ -257,7 +256,7 @@ func (w *remoteWatcher) updatePaths(ctx context.Context) { func (w *remoteWatcher) selectID() (uint16, bool) { for i := 0; i < 100; i++ { - id := uint16(rand.Uint32()) + id := snet.RandomSCMPIdentifer() if _, ok := w.pathWatchersByID[id]; !ok { return id, true } diff --git a/pkg/snet/packet.go b/pkg/snet/packet.go index 6444d34d03..31f7592398 100644 --- a/pkg/snet/packet.go +++ b/pkg/snet/packet.go @@ -15,6 +15,8 @@ package snet import ( + "math/rand" + "github.com/google/gopacket" "github.com/scionproto/scion/pkg/addr" @@ -192,6 +194,30 @@ func (m SCMPInternalConnectivityDown) length() int { return 28 + len(m.Payload) } +const ( + // SCMPIdentifierStart and SCMPIdentiferEnd define the range for Identifiers + // that should be used for SCMPEchoRequest and SCMPTracerouteRequest, + // in preparation for a dispatcher-less snet. + // This range corresponds to the port range used for SCION/UDP by the + // dispatcher. Using the same range for Identifiers in SCMP requests will + // allow a router to dispatch SCMP requests based on the Identifier, + // without risk of interfering with unaware endpoints. + // + // WARNING: transitional, this will be removed in the dispatcher-less snet. + SCMPIdentifierStart = 32768 + SCMPIdentifierEnd = 65535 +) + +// RandomSCMPIdentifier returns a random SCMP identifier in the range +// [SCMPIdentifierStart, SCMPIdentifierEnd]. +// +// WARNING: This is a transitional helper function, which will be removed +// in the dispatcher-less snet; then, the underlay port must be used as identifier. +func RandomSCMPIdentifer() uint16 { + id := SCMPIdentifierStart + rand.Int31n(SCMPIdentifierEnd-SCMPIdentifierStart+1) + return uint16(id) +} + // SCMPEchoRequest is the SCMP echo request payload. type SCMPEchoRequest struct { Identifier uint16 diff --git a/private/app/path/path.go b/private/app/path/path.go index 6b8bffacbc..b89e53a09a 100644 --- a/private/app/path/path.go +++ b/private/app/path/path.go @@ -143,7 +143,7 @@ func filterUnhealthy( DstIA: remote, LocalIA: cfg.LocalIA, LocalIP: cfg.LocalIP, - ID: uint16(rand.Uint32()), + ID: snet.RandomSCMPIdentifer(), SCIONPacketConnMetrics: cfg.SCIONPacketConnMetrics, Dispatcher: cfg.Dispatcher, }.GetStatuses(subCtx, nonEmptyPaths, pathprobe.WithEPIC(epic)) diff --git a/scion/ping/ping.go b/scion/ping/ping.go index 4efea528d5..8fc46e9b3c 100644 --- a/scion/ping/ping.go +++ b/scion/ping/ping.go @@ -18,7 +18,6 @@ package ping import ( "context" "encoding/binary" - "math/rand" "net" "sync" "time" @@ -103,13 +102,13 @@ func Run(ctx context.Context, cfg Config) (Stats, error) { return Stats{}, serrors.New("interval below millisecond") } - id := rand.Uint64() replies := make(chan reply, 10) + id := snet.RandomSCMPIdentifer() svc := snet.DefaultPacketDispatcherService{ Dispatcher: cfg.Dispatcher, SCMPHandler: scmpHandler{ - id: uint16(id), + id: id, replies: replies, }, } @@ -148,7 +147,7 @@ type pinger struct { timeout time.Duration pldSize int - id uint64 + id uint16 conn snet.PacketConn local *snet.UDPAddr replies <-chan reply @@ -224,7 +223,7 @@ func (p *pinger) send(remote *snet.UDPAddr) error { binary.BigEndian.PutUint64(p.pld, uint64(time.Now().UnixNano())) pkt, err := pack(p.local, remote, snet.SCMPEchoRequest{ - Identifier: uint16(p.id), + Identifier: p.id, SeqNumber: uint16(sequence), Payload: p.pld, }) diff --git a/scion/showpaths/showpaths.go b/scion/showpaths/showpaths.go index 37e328fd31..2e950e0b69 100644 --- a/scion/showpaths/showpaths.go +++ b/scion/showpaths/showpaths.go @@ -19,7 +19,6 @@ import ( "fmt" "io" "math" - "math/rand" "net/netip" "strconv" "strings" @@ -356,7 +355,7 @@ func Run(ctx context.Context, dst addr.IA, cfg Config) (*Result, error) { DstIA: dst, LocalIA: localIA, LocalIP: cfg.Local, - ID: uint16(rand.Uint32()), + ID: snet.RandomSCMPIdentifer(), Dispatcher: cfg.Dispatcher, }.GetStatuses(ctx, p, pathprobe.WithEPIC(cfg.Epic)) if err != nil { diff --git a/scion/traceroute/traceroute.go b/scion/traceroute/traceroute.go index 7ac6a6cb1a..2893ab983d 100644 --- a/scion/traceroute/traceroute.go +++ b/scion/traceroute/traceroute.go @@ -17,7 +17,6 @@ package traceroute import ( "context" - "math/rand" "net" "net/netip" "time" @@ -100,7 +99,7 @@ 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") } - id := rand.Uint64() + id := snet.RandomSCMPIdentifer() replies := make(chan reply, 10) dispatcher := snet.DefaultPacketDispatcherService{ Dispatcher: cfg.Dispatcher, @@ -121,7 +120,7 @@ func Run(ctx context.Context, cfg Config) (Stats, error) { replies: replies, errHandler: cfg.ErrHandler, updateHandler: cfg.UpdateHandler, - id: uint16(id), + id: id, path: cfg.PathEntry, epic: cfg.EPIC, }