From 1443c955feec90ded77cbddddbf60cb18533adbe Mon Sep 17 00:00:00 2001
From: Jean-Christophe Hugly <jice@scion.org>
Date: Fri, 29 Nov 2024 19:56:52 +0100
Subject: [PATCH 01/11] Just a checkpoint. All of this still WIP.

---
 router/cmd/router/main.go         |  12 +-
 router/dataplane.go               | 434 ++++++++++++++++++++++--------
 router/dataplane_internal_test.go |   2 +-
 router/dataplane_test.go          |  58 ++--
 router/export_test.go             |   2 +-
 5 files changed, 354 insertions(+), 154 deletions(-)

diff --git a/router/cmd/router/main.go b/router/cmd/router/main.go
index af8b2fd0fe..178f81bd06 100644
--- a/router/cmd/router/main.go
+++ b/router/cmd/router/main.go
@@ -61,6 +61,11 @@ func realMain(ctx context.Context) error {
 		DataPlane: router.DataPlane{
 			Metrics:                        metrics,
 			ExperimentalSCMPAuthentication: globalCfg.Features.ExperimentalSCMPAuthentication,
+			RunConfig: router.RunConfig{
+				NumProcessors:         globalCfg.Router.NumProcessors,
+				NumSlowPathProcessors: globalCfg.Router.NumSlowPathProcessors,
+				BatchSize:             globalCfg.Router.BatchSize,
+			},
 		},
 		ReceiveBufferSize:   globalCfg.Router.ReceiveBufferSize,
 		SendBufferSize:      globalCfg.Router.SendBufferSize,
@@ -128,12 +133,7 @@ func realMain(ctx context.Context) error {
 	})
 	g.Go(func() error {
 		defer log.HandlePanic()
-		runConfig := &router.RunConfig{
-			NumProcessors:         globalCfg.Router.NumProcessors,
-			NumSlowPathProcessors: globalCfg.Router.NumSlowPathProcessors,
-			BatchSize:             globalCfg.Router.BatchSize,
-		}
-		if err := dp.DataPlane.Run(errCtx, runConfig); err != nil {
+		if err := dp.DataPlane.Run(errCtx); err != nil {
 			return serrors.Wrap("running dataplane", err)
 		}
 		return nil
diff --git a/router/dataplane.go b/router/dataplane.go
index 6c407ab1c6..f690c3920f 100644
--- a/router/dataplane.go
+++ b/router/dataplane.go
@@ -91,6 +91,29 @@ type BatchConn interface {
 	Close() error
 }
 
+// LinkScope describes the kind (or scope) of a link: internal, sibling, or external.
+type linkScope int
+
+const (
+	internal linkScope = iota // to/from end-hosts in the local AS
+	sibling                   // to/from (external interfaces owned by) a sibling router
+	external                  // to/from routers in another AS
+)
+
+// link embodies the router's idea of an underlay connection. It associates the underlay connection,
+// with a bfdSession, a destination address, (and more in the future). It also allows the send
+// operation to be underlay-dependant. It can be (in the future) registered with the underlay
+// implementation so incoming packets can be easily associated to a link.
+// TODO(jiceatscion): use inheritence between implementations.
+type link interface {
+	getScope() linkScope
+	getBfdSession() bfdSession
+	isUp() bool
+	getRemote() netip.AddrPort
+	getIfID() uint16
+	send(p *packet) bool
+}
+
 type disposition int
 
 const (
@@ -169,12 +192,188 @@ func (p *packet) reset() {
 	// Everything else is reset to zero value.
 }
 
+// An external link is connection oriented. And is not shared. As a result, it has a defined
+// interfaceID and a fixed destination address.
+type externalLink struct {
+	// Each link has a queue, consumed by a forwarding task and fed by bfd senders
+	// and packet processors.
+	queue      chan *packet
+	bfdSession bfdSession
+	remote     netip.AddrPort
+	ifID       uint16
+}
+
+func newExternalLink(
+	queue chan *packet,
+	bfdSession bfdSession,
+	remote netip.AddrPort,
+	ifID uint16,
+) *externalLink {
+
+	return &externalLink{
+		queue:      queue,
+		bfdSession: bfdSession,
+		remote:     remote,
+		ifID:       ifID,
+	}
+}
+
+func (l *externalLink) getScope() linkScope {
+	return external
+}
+
+func (l *externalLink) getBfdSession() bfdSession {
+	return l.bfdSession
+}
+
+func (l *externalLink) isUp() bool {
+	return l.bfdSession == nil || l.bfdSession.IsUp()
+}
+
+func (l *externalLink) getRemote() netip.AddrPort {
+	return l.remote
+}
+
+func (l *externalLink) getIfID() uint16 {
+	return l.ifID
+}
+
+// The goal is to have send do those things that are dependent on the underlay connection type
+// such as updating the destination address of the pkt with the known bound one.
+// TODO(jiceatscion): indeed, do it.
+func (l *externalLink) send(p *packet) bool {
+	select {
+	case l.queue <- p:
+	default:
+		return false
+	}
+	return true
+}
+
+// A siblingLink is connection oriented. However, it may be shared between multiple interface IDs.
+// So, it has no defined interface ID but a fixed destination address.
+type siblingLink struct {
+	// Each link has a queue, consumed by a forwarding task and fed by bfd senders
+	// and packet processors.
+	queue      chan *packet
+	bfdSession bfdSession
+	remote     netip.AddrPort
+	ifID       uint16
+}
+
+func newSiblingLink(
+	queue chan *packet,
+	bfdSession bfdSession,
+	remote netip.AddrPort,
+) *siblingLink {
+
+	return &siblingLink{
+		queue:      queue,
+		bfdSession: bfdSession,
+		remote:     remote,
+	}
+}
+
+func (l *siblingLink) getScope() linkScope {
+	return sibling
+}
+func (l *siblingLink) getBfdSession() bfdSession {
+	return l.bfdSession
+}
+
+func (l *siblingLink) isUp() bool {
+	return l.bfdSession == nil || l.bfdSession.IsUp()
+}
+
+func (l *siblingLink) getRemote() netip.AddrPort {
+	return l.remote
+}
+
+func (l *siblingLink) getIfID() uint16 {
+	return 0
+}
+
+func (l *siblingLink) send(p *packet) bool {
+	select {
+	case l.queue <- p:
+	default:
+		return false
+	}
+	return true
+}
+
+// An internalLink is not connection oriented and it is not associated with an AS border interface.
+// So, it has no defined interface ID and no fixed destination address. It can't have a BFD session
+// either. So, it's a link only in name. May be, in the future, we find it more convenient to not
+// treat such connections as links. For now, it helps with code migration. Incoming packets are
+// associated with the link they came in through by way of the underlay source address.
+type internalLink struct {
+	// Each link has a queue, consumed by a forwarding task and fed by bfd senders
+	// and packet processors.
+	queue chan *packet
+}
+
+func newInternalLink(queue chan *packet) *internalLink {
+	return &internalLink{
+		queue: queue,
+	}
+}
+
+func (l *internalLink) getScope() linkScope {
+	return internal
+}
+
+func (l *internalLink) isUp() bool {
+	return true
+}
+
+func (l *internalLink) getBfdSession() bfdSession {
+	return nil
+}
+
+func (l *internalLink) getRemote() netip.AddrPort {
+	return netip.AddrPort{ /* fixme; we have a special value */ }
+}
+
+func (l *internalLink) getIfID() uint16 {
+	return 0
+}
+
+func (l *internalLink) send(p *packet) bool {
+	select {
+	case l.queue <- p:
+	default:
+		return false
+	}
+	return true
+}
+
+// Connections can be shared between links. So we keep a cannonical map.
+// Also, there must be one sending loop and one channel per connection. So that goes here.
+// This business should be moved to underlay implementations, but... one problem at a time.
+// So, this is a minimal implementation to support the single underlay we have today.
+type forwarder struct {
+	conn BatchConn
+	fwQ  chan *packet
+	ifID uint16 // non-zero only when not shared bw links.
+}
+
+func newForwarder(conn BatchConn, queueSize int, ifID uint16) *forwarder {
+	return &forwarder{
+		conn: conn,
+		fwQ:  make(chan *packet, queueSize),
+		ifID: ifID,
+	}
+}
+
 // DataPlane contains a SCION Border Router's forwarding logic. It reads packets
 // from multiple sockets, performs routing, and sends them to their destinations
 // (after updating the path, if that is needed).
 type DataPlane struct {
-	interfaces          map[uint16]BatchConn
-	external            map[uint16]BatchConn
+	links               map[netip.AddrPort]link
+	forwarders          map[netip.AddrPort]*forwarder // We do this here as a refactoring step.
+	interfaces          map[uint16]link
+	external            map[uint16]struct{} // TODO: to be replaced w/ e.g. interfaces[x].getScope()
 	linkTypes           map[uint16]topology.LinkType
 	neighborIAs         map[uint16]addr.IA
 	peerInterfaces      map[uint16]uint16
@@ -192,10 +391,7 @@ type DataPlane struct {
 	dispatchedPortEnd   uint16
 
 	ExperimentalSCMPAuthentication bool
-
-	// The forwarding queues. Each is consumed by a forwarder process and fed by
-	// one bfd sender and the packet processors.
-	fwQs map[uint16]chan *packet
+	RunConfig                      RunConfig
 
 	// The pool that stores all the packet buffers as described in the design document. See
 	// https://github.com/scionproto/scion/blob/master/doc/dev/design/BorderRouter.rst
@@ -234,6 +430,7 @@ var (
 	ingressInterfaceInvalid       = errors.New("ingress interface invalid")
 	macVerificationFailed         = errors.New("MAC verification failed")
 	badPacketSize                 = errors.New("bad packet size")
+	notImplemented                = errors.New("Not Yet Implemented")
 
 	// zeroBuffer will be used to reset the Authenticator option in the
 	// scionPacketProcessor.OptAuth
@@ -339,12 +536,19 @@ func (d *DataPlane) AddInternalInterface(conn BatchConn, ip netip.Addr) error {
 		return emptyValue
 	}
 	if d.interfaces == nil {
-		d.interfaces = make(map[uint16]BatchConn)
+		d.interfaces = make(map[uint16]link)
+		d.forwarders = make(map[netip.AddrPort]*forwarder)
 	} else if d.interfaces[0] != nil {
 		return alreadySet
 	}
-	d.interfaces[0] = conn
+
+	f := newForwarder(conn, d.RunConfig.BatchSize, 0)
+	d.forwarders[netip.AddrPort{}] = f
+	d.interfaces[0] = newInternalLink(f.fwQ)
+	// That is not a real link => not in the links map; should be?
+
 	d.internalIP = ip
+
 	return nil
 }
 
@@ -363,21 +567,31 @@ func (d *DataPlane) AddExternalInterface(ifID uint16, conn BatchConn,
 	if conn == nil || !src.Addr.IsValid() || !dst.Addr.IsValid() {
 		return emptyValue
 	}
-	err := d.addExternalInterfaceBFD(ifID, src, dst, cfg)
+	bfd, err := d.addExternalInterfaceBFD(ifID, src, dst, cfg)
 	if err != nil {
 		return serrors.Wrap("adding external BFD", err, "if_id", ifID)
 	}
 	if d.external == nil {
-		d.external = make(map[uint16]BatchConn)
+		d.external = make(map[uint16]struct{})
 	}
 	if d.interfaces == nil {
-		d.interfaces = make(map[uint16]BatchConn)
+		d.interfaces = make(map[uint16]link)
+	}
+	if d.forwarders == nil {
+		d.forwarders = make(map[netip.AddrPort]*forwarder)
 	}
 	if _, exists := d.external[ifID]; exists {
 		return serrors.JoinNoStack(alreadySet, nil, "ifID", ifID)
 	}
-	d.interfaces[ifID] = conn
-	d.external[ifID] = conn
+	if d.forwarders[dst.Addr] != nil {
+		return serrors.JoinNoStack(alreadySet, nil, "dst Addr", dst.Addr)
+	}
+	fw := newForwarder(conn, d.RunConfig.BatchSize, ifID)
+	lk := newExternalLink(fw.fwQ, bfd, dst.Addr, ifID)
+	d.forwarders[dst.Addr] = fw
+	d.links[dst.Addr] = lk
+	d.interfaces[ifID] = lk
+	d.external[ifID] = struct{}{}
 	return nil
 }
 
@@ -442,10 +656,10 @@ func (d *DataPlane) AddRemotePeer(local, remote uint16) error {
 
 // AddExternalInterfaceBFD adds the inter AS connection BFD session.
 func (d *DataPlane) addExternalInterfaceBFD(ifID uint16,
-	src, dst control.LinkEnd, cfg control.BFD) error {
+	src, dst control.LinkEnd, cfg control.BFD) (bfdSession, error) {
 
 	if *cfg.Disable {
-		return nil
+		return nil, nil
 	}
 	var m bfd.Metrics
 	if d.Metrics != nil {
@@ -463,7 +677,7 @@ func (d *DataPlane) addExternalInterfaceBFD(ifID uint16,
 	}
 	s, err := newBFDSend(d, src.IA, dst.IA, src.Addr, dst.Addr, ifID, d.macFactory())
 	if err != nil {
-		return err
+		return nil, err
 	}
 	return d.addBFDController(ifID, s, cfg, m)
 }
@@ -472,27 +686,22 @@ func (d *DataPlane) addExternalInterfaceBFD(ifID uint16,
 // returns InterfaceUp if the relevant bfdsession state is up, or if there is no BFD
 // session. Otherwise, it returns InterfaceDown.
 func (d *DataPlane) getInterfaceState(ifID uint16) control.InterfaceState {
-	bfdSessions := d.bfdSessions
-	if bfdSession, ok := bfdSessions[ifID]; ok && !bfdSession.IsUp() {
+	if l := d.interfaces[ifID]; l != nil && !l.isUp() {
 		return control.InterfaceDown
 	}
 	return control.InterfaceUp
 }
 
 func (d *DataPlane) addBFDController(ifID uint16, s bfd.Sender, cfg control.BFD,
-	metrics bfd.Metrics) error {
-
-	if d.bfdSessions == nil {
-		d.bfdSessions = make(map[uint16]bfdSession)
-	}
+	metrics bfd.Metrics) (bfdSession, error) {
 
 	// Generate random discriminator. It can't be zero.
 	discInt, err := rand.Int(rand.Reader, big.NewInt(0xfffffffe))
 	if err != nil {
-		return err
+		return nil, err
 	}
 	disc := layers.BFDDiscriminator(uint32(discInt.Uint64()) + 1)
-	d.bfdSessions[ifID] = &bfd.Session{
+	return &bfd.Session{
 		Sender:                s,
 		DetectMult:            layers.BFDDetectMultiplier(cfg.DetectMult),
 		DesiredMinTxInterval:  cfg.DesiredMinTxInterval,
@@ -500,8 +709,7 @@ func (d *DataPlane) addBFDController(ifID uint16, s bfd.Sender, cfg control.BFD,
 		LocalDiscriminator:    disc,
 		ReceiveQueueSize:      10,
 		Metrics:               metrics,
-	}
-	return nil
+	}, nil
 }
 
 // AddSvc adds the address for the given service. This can be called multiple
@@ -559,7 +767,7 @@ func (d *DataPlane) AddNextHop(ifID uint16, src, dst netip.AddrPort, cfg control
 	if !dst.IsValid() || !src.IsValid() {
 		return emptyValue
 	}
-	err := d.addNextHopBFD(ifID, src, dst, cfg, sibling)
+	bfd, err := d.addNextHopBFD(ifID, src, dst, cfg, sibling)
 	if err != nil {
 		return serrors.Wrap("adding next hop BFD", err, "if_id", ifID)
 	}
@@ -569,6 +777,16 @@ func (d *DataPlane) AddNextHop(ifID uint16, src, dst netip.AddrPort, cfg control
 	if _, exists := d.internalNextHops[ifID]; exists {
 		return serrors.JoinNoStack(alreadySet, nil, "ifID", ifID)
 	}
+
+	// We have one link per unique dst address.
+	sib := d.links[dst]
+	if sib == nil {
+		// As a refactoring step, re-use the internal connection's forwarder, which is what we've
+		// been effectively doing so far.
+		sib := newSiblingLink(d.forwarders[netip.AddrPort{}].fwQ, bfd, dst)
+		d.links[dst] = sib
+	}
+	d.interfaces[ifID] = sib
 	d.internalNextHops[ifID] = dst
 	return nil
 }
@@ -577,18 +795,10 @@ func (d *DataPlane) AddNextHop(ifID uint16, src, dst netip.AddrPort, cfg control
 // If the remote ifID belongs to an existing address, the existing
 // BFD session will be re-used.
 func (d *DataPlane) addNextHopBFD(ifID uint16, src, dst netip.AddrPort, cfg control.BFD,
-	sibling string) error {
+	sibling string) (bfdSession, error) {
 
 	if *cfg.Disable {
-		return nil
-	}
-	for k, v := range d.internalNextHops {
-		if v.String() == dst.String() {
-			if c, ok := d.bfdSessions[k]; ok {
-				d.bfdSessions[ifID] = c
-				return nil
-			}
-		}
+		return nil, nil
 	}
 	var m bfd.Metrics
 	if d.Metrics != nil {
@@ -603,7 +813,7 @@ func (d *DataPlane) addNextHopBFD(ifID uint16, src, dst netip.AddrPort, cfg cont
 
 	s, err := newBFDSend(d, d.localIA, d.localIA, src, dst, 0, d.macFactory())
 	if err != nil {
-		return err
+		return nil, err
 	}
 	return d.addBFDController(ifID, s, cfg, m)
 }
@@ -621,49 +831,48 @@ type RunConfig struct {
 	BatchSize             int
 }
 
-func (d *DataPlane) Run(ctx context.Context, cfg *RunConfig) error {
+func (d *DataPlane) Run(ctx context.Context) error {
 	d.mtx.Lock()
 	d.initMetrics()
 
 	processorQueueSize := max(
-		len(d.interfaces)*cfg.BatchSize/cfg.NumProcessors,
-		cfg.BatchSize)
+		len(d.forwarders)*d.RunConfig.BatchSize/d.RunConfig.NumProcessors,
+		d.RunConfig.BatchSize)
 
-	d.initPacketPool(cfg, processorQueueSize)
-	procQs, fwQs, slowQs := initQueues(cfg, d.interfaces, processorQueueSize)
-	d.fwQs = fwQs // Shared with BFD senders
+	d.initPacketPool(processorQueueSize)
+	procQs, slowQs := d.initQueues(processorQueueSize)
 
 	d.setRunning()
-	for ifID, conn := range d.interfaces {
-		go func(ifID uint16, conn BatchConn) {
+	for _, f := range d.forwarders {
+		go func(f *forwarder) {
 			defer log.HandlePanic()
-			d.runReceiver(ifID, conn, cfg, procQs)
-		}(ifID, conn)
-		go func(ifID uint16, conn BatchConn) {
+			d.runReceiver(f, procQs)
+		}(f)
+		go func(f *forwarder) {
 			defer log.HandlePanic()
-			d.runForwarder(ifID, conn, cfg, fwQs[ifID])
-		}(ifID, conn)
+			d.runForwarder(f)
+		}(f)
 	}
-	for i := 0; i < cfg.NumProcessors; i++ {
+	for i := 0; i < d.RunConfig.NumProcessors; i++ {
 		go func(i int) {
 			defer log.HandlePanic()
-			d.runProcessor(i, procQs[i], fwQs, slowQs[i%cfg.NumSlowPathProcessors])
+			d.runProcessor(i, procQs[i], slowQs[i%d.RunConfig.NumSlowPathProcessors])
 		}(i)
 	}
-	for i := 0; i < cfg.NumSlowPathProcessors; i++ {
+	for i := 0; i < d.RunConfig.NumSlowPathProcessors; i++ {
 		go func(i int) {
 			defer log.HandlePanic()
-			d.runSlowPathProcessor(i, slowQs[i], fwQs)
+			d.runSlowPathProcessor(i, slowQs[i])
 		}(i)
 	}
 
-	for k, v := range d.bfdSessions {
-		go func(ifID uint16, c bfdSession) {
+	for a, l := range d.links {
+		go func(a netip.AddrPort, s bfdSession) {
 			defer log.HandlePanic()
-			if err := c.Run(ctx); err != nil && err != bfd.AlreadyRunning {
-				log.Error("BFD session failed to start", "ifID", ifID, "err", err)
+			if err := s.Run(ctx); err != nil && err != bfd.AlreadyRunning {
+				log.Error("BFD session failed to start", "remote address", a, "err", err)
 			}
-		}(k, v)
+		}(a, l.getBfdSession())
 	}
 
 	d.mtx.Unlock()
@@ -673,10 +882,10 @@ func (d *DataPlane) Run(ctx context.Context, cfg *RunConfig) error {
 
 // initializePacketPool calculates the size of the packet pool based on the
 // current dataplane settings and allocates all the buffers
-func (d *DataPlane) initPacketPool(cfg *RunConfig, processorQueueSize int) {
-	poolSize := len(d.interfaces)*cfg.BatchSize +
-		(cfg.NumProcessors+cfg.NumSlowPathProcessors)*(processorQueueSize+1) +
-		len(d.interfaces)*(2*cfg.BatchSize)
+func (d *DataPlane) initPacketPool(processorQueueSize int) {
+	poolSize := len(d.interfaces)*d.RunConfig.BatchSize +
+		(d.RunConfig.NumProcessors+d.RunConfig.NumSlowPathProcessors)*(processorQueueSize+1) +
+		len(d.interfaces)*(2*d.RunConfig.BatchSize)
 
 	log.Debug("Initialize packet pool of size", "poolSize", poolSize)
 	d.packetPool = make(chan *packet, poolSize)
@@ -687,29 +896,24 @@ func (d *DataPlane) initPacketPool(cfg *RunConfig, processorQueueSize int) {
 	}
 }
 
-// initializes the processing routines and forwarders queues
-func initQueues(cfg *RunConfig, interfaces map[uint16]BatchConn,
-	processorQueueSize int) ([]chan *packet, map[uint16]chan *packet, []chan *packet) {
+// initializes the processing routines and queues
+func (d *DataPlane) initQueues(processorQueueSize int) ([]chan *packet, []chan *packet) {
 
-	procQs := make([]chan *packet, cfg.NumProcessors)
-	for i := 0; i < cfg.NumProcessors; i++ {
+	procQs := make([]chan *packet, d.RunConfig.NumProcessors)
+	for i := 0; i < d.RunConfig.NumProcessors; i++ {
 		procQs[i] = make(chan *packet, processorQueueSize)
 	}
-	slowQs := make([]chan *packet, cfg.NumSlowPathProcessors)
-	for i := 0; i < cfg.NumSlowPathProcessors; i++ {
+	slowQs := make([]chan *packet, d.RunConfig.NumSlowPathProcessors)
+	for i := 0; i < d.RunConfig.NumSlowPathProcessors; i++ {
 		slowQs[i] = make(chan *packet, processorQueueSize)
 	}
-	fwQs := make(map[uint16]chan *packet)
-	for ifID := range interfaces {
-		fwQs[ifID] = make(chan *packet, cfg.BatchSize)
-	}
-	return procQs, fwQs, slowQs
+	return procQs, slowQs
 }
 
-func (d *DataPlane) runReceiver(ifID uint16, conn BatchConn, cfg *RunConfig,
-	procQs []chan *packet) {
+func (d *DataPlane) runReceiver(f *forwarder, procQs []chan *packet) {
 
-	log.Debug("Run receiver for", "interface", ifID)
+	// TODO(jiceatscion): ifID isn't veyr unique. It is zero for internal and all sibling links.
+	log.Debug("Run receiver for", "interface", f.ifID)
 
 	// Each receiver (therefore each input interface) has a unique random seed for the procID hash
 	// function.
@@ -724,21 +928,21 @@ func (d *DataPlane) runReceiver(ifID uint16, conn BatchConn, cfg *RunConfig,
 
 	// A collection of socket messages, as the readBatch API expects them. We keep using the same
 	// collection, call after call; only replacing the buffer.
-	msgs := underlayconn.NewReadMessages(cfg.BatchSize)
+	msgs := underlayconn.NewReadMessages(d.RunConfig.BatchSize)
 
 	// An array of corresponding packet references. Each corresponds to one msg.
 	// The packet owns the buffer that we set in the matching msg, plus the metadata that we'll add.
-	packets := make([]*packet, cfg.BatchSize)
+	packets := make([]*packet, d.RunConfig.BatchSize)
 
-	numReusable := 0                     // unused buffers from previous loop
-	metrics := d.forwardingMetrics[ifID] // If receiver exists, fw metrics exist too.
+	numReusable := 0                       // unused buffers from previous loop
+	metrics := d.forwardingMetrics[f.ifID] // If receiver exists, fw metrics exist too.
 
 	enqueueForProcessing := func(size int, srcAddr *net.UDPAddr, pkt *packet) {
 		sc := classOfSize(size)
 		metrics[sc].InputPacketsTotal.Inc()
 		metrics[sc].InputBytesTotal.Add(float64(size))
 
-		procID, err := computeProcID(pkt.rawPacket, cfg.NumProcessors, hashSeed)
+		procID, err := computeProcID(pkt.rawPacket, d.RunConfig.NumProcessors, hashSeed)
 		if err != nil {
 			log.Debug("Error while computing procID", "err", err)
 			d.returnPacketToPool(pkt)
@@ -747,7 +951,7 @@ func (d *DataPlane) runReceiver(ifID uint16, conn BatchConn, cfg *RunConfig,
 		}
 
 		pkt.rawPacket = pkt.rawPacket[:size] // Update size; readBatch does not.
-		pkt.ingress = ifID
+		pkt.ingress = f.ifID
 		pkt.srcAddr = srcAddr
 		select {
 		case procQs[procID] <- pkt:
@@ -761,7 +965,7 @@ func (d *DataPlane) runReceiver(ifID uint16, conn BatchConn, cfg *RunConfig,
 		// collect packets.
 
 		// Give a new buffer to the msgs elements that have been used in the previous loop.
-		for i := 0; i < cfg.BatchSize-numReusable; i++ {
+		for i := 0; i < d.RunConfig.BatchSize-numReusable; i++ {
 			p := d.getPacketFromPool()
 			p.reset()
 			packets[i] = p
@@ -769,10 +973,10 @@ func (d *DataPlane) runReceiver(ifID uint16, conn BatchConn, cfg *RunConfig,
 		}
 
 		// Fill the packets
-		numPkts, err := conn.ReadBatch(msgs)
+		numPkts, err := f.conn.ReadBatch(msgs)
 		numReusable = len(msgs) - numPkts
 		if err != nil {
-			log.Debug("Error while reading batch", "interfaceID", ifID, "err", err)
+			log.Debug("Error while reading batch", "interfaceID", f.ifID, "err", err)
 			continue
 		}
 		for i, msg := range msgs[:numPkts] {
@@ -816,8 +1020,7 @@ func (d *DataPlane) returnPacketToPool(pkt *packet) {
 	d.packetPool <- pkt
 }
 
-func (d *DataPlane) runProcessor(id int, q <-chan *packet,
-	fwQs map[uint16]chan *packet, slowQ chan<- *packet) {
+func (d *DataPlane) runProcessor(id int, q <-chan *packet, slowQ chan<- *packet) {
 
 	log.Debug("Initialize processor with", "id", id)
 	processor := newPacketProcessor(d)
@@ -856,25 +1059,21 @@ func (d *DataPlane) runProcessor(id int, q <-chan *packet,
 			d.returnPacketToPool(p)
 			continue
 		}
-		fwCh, ok := fwQs[p.egress]
+		fwLink, ok := d.interfaces[p.egress]
 		if !ok {
 			log.Debug("Error determining forwarder. Egress is invalid", "egress", p.egress)
 			metrics.DroppedPacketsInvalid.Inc()
 			d.returnPacketToPool(p)
 			continue
 		}
-
-		select {
-		case fwCh <- p:
-		default:
+		if !fwLink.send(p) {
 			d.returnPacketToPool(p)
 			metrics.DroppedPacketsBusyForwarder.Inc()
 		}
 	}
 }
 
-func (d *DataPlane) runSlowPathProcessor(id int, q <-chan *packet,
-	fwQs map[uint16]chan *packet) {
+func (d *DataPlane) runSlowPathProcessor(id int, q <-chan *packet) {
 
 	log.Debug("Initialize slow-path processor with", "id", id)
 	processor := newSlowPathProcessor(d)
@@ -892,15 +1091,13 @@ func (d *DataPlane) runSlowPathProcessor(id int, q <-chan *packet,
 			d.returnPacketToPool(p)
 			continue
 		}
-		fwCh, ok := fwQs[p.egress]
+		fwLink, ok := d.interfaces[p.egress]
 		if !ok {
 			log.Debug("Error determining forwarder. Egress is invalid", "egress", p.egress)
 			d.returnPacketToPool(p)
 			continue
 		}
-		select {
-		case fwCh <- p:
-		default:
+		if !fwLink.send(p) {
 			d.returnPacketToPool(p)
 		}
 	}
@@ -1031,25 +1228,27 @@ func updateOutputMetrics(metrics interfaceMetrics, packets []*packet) {
 	}
 }
 
-func (d *DataPlane) runForwarder(ifID uint16, conn BatchConn, cfg *RunConfig, c <-chan *packet) {
+// It would make more sense for this to be part of Forwarder
+func (d *DataPlane) runForwarder(f *forwarder) {
 
-	log.Debug("Initialize forwarder for", "interface", ifID)
+	// TODO(jiceatscion): ifID isn't so informative; it is zero for internal and all sibling links.
+	log.Debug("Initialize forwarder for", "interface", f.ifID)
 
 	// We use this somewhat like a ring buffer.
-	pkts := make([]*packet, cfg.BatchSize)
+	pkts := make([]*packet, d.RunConfig.BatchSize)
 
 	// We use this as a temporary buffer, but allocate it just once
 	// to save on garbage handling.
-	msgs := make(underlayconn.Messages, cfg.BatchSize)
+	msgs := make(underlayconn.Messages, d.RunConfig.BatchSize)
 	for i := range msgs {
 		msgs[i].Buffers = make([][]byte, 1)
 	}
 
-	metrics := d.forwardingMetrics[ifID]
+	metrics := d.forwardingMetrics[f.ifID]
 
 	toWrite := 0
 	for d.IsRunning() {
-		toWrite += readUpTo(c, cfg.BatchSize-toWrite, toWrite == 0, pkts[toWrite:])
+		toWrite += readUpTo(f.fwQ, d.RunConfig.BatchSize-toWrite, toWrite == 0, pkts[toWrite:])
 
 		// Turn the packets into underlay messages that WriteBatch can send.
 		for i, p := range pkts[:toWrite] {
@@ -1059,7 +1258,7 @@ func (d *DataPlane) runForwarder(ifID uint16, conn BatchConn, cfg *RunConfig, c
 				msgs[i].Addr = p.dstAddr
 			}
 		}
-		written, _ := conn.WriteBatch(msgs[:toWrite], 0)
+		written, _ := f.conn.WriteBatch(msgs[:toWrite], 0)
 		if written < 0 {
 			// WriteBatch returns -1 on error, we just consider this as
 			// 0 packets written
@@ -1543,7 +1742,13 @@ func (p *scionPacketProcessor) validateTransitUnderlaySrc() disposition {
 	return pForward
 }
 
-// Validates the egress interface referenced by the current hop.
+// Validates the egress interface referenced by the current hop. This  is not called for
+// packets to be delivered to the local AS, so pkt.egress is never 0.
+// If pkt.ingress is zero, the packet can be coming from either a local end-host or a
+// sibling router. In either of these cases, it must be leaving via a locally owned external
+// interface (i.e. it can be going to a sibling router or to a local end-host). On the other
+// hand, a packet coming directly from another AS can be going anywhere: local delivery,
+// to another AS directly, or via a sibling router.
 func (p *scionPacketProcessor) validateEgressID() disposition {
 	egressID := p.pkt.egress
 	_, ih := p.d.internalNextHops[egressID]
@@ -2408,16 +2613,14 @@ func (b *bfdSend) Send(bfd *layers.BFD) error {
 
 	// BfdControllers and fwQs are initialized from the same set of ifIDs. So not finding
 	// the forwarding queue is an serious internal error. Let that panic.
-	fwChan := b.dataPlane.fwQs[b.ifID]
+	fwLink := b.dataPlane.interfaces[b.ifID]
 
 	if b.ifID == 0 {
 		// Using the internal interface: must specify the destination address
 		updateNetAddrFromAddrPort(p.dstAddr, b.dstAddr)
 	}
 	// No need to specify pkt.egress. It isn't used downstream from here.
-	select {
-	case fwChan <- p:
-	default:
+	if !fwLink.send(p) {
 		// We do not care if some BFD packets get bounced under high load. If it becomes a problem,
 		// the solution is do use BFD's demand-mode. To be considered in a future refactoring.
 		b.dataPlane.returnPacketToPool(p)
@@ -2739,9 +2942,6 @@ func (d *DataPlane) initMetrics() {
 	d.forwardingMetrics = make(map[uint16]interfaceMetrics)
 	d.forwardingMetrics[0] = newInterfaceMetrics(d.Metrics, 0, d.localIA, d.neighborIAs)
 	for ifID := range d.external {
-		if _, notOwned := d.internalNextHops[ifID]; notOwned {
-			continue
-		}
 		d.forwardingMetrics[ifID] = newInterfaceMetrics(d.Metrics, ifID, d.localIA, d.neighborIAs)
 	}
 
diff --git a/router/dataplane_internal_test.go b/router/dataplane_internal_test.go
index f6274f5f3c..1132f7518e 100644
--- a/router/dataplane_internal_test.go
+++ b/router/dataplane_internal_test.go
@@ -432,7 +432,7 @@ func TestSlowPathProcessing(t *testing.T) {
 	// ProcessPacket assumes some pre-conditions:
 	// * The ingress interface has to exist. This fake map is good for the test cases we have.
 	// * InternalNextHops may not be nil. Empty is ok for all the test cases we have.
-	fakeExternalInterfaces := map[uint16]BatchConn{1: nil}
+	fakeExternalInterfaces := map[uint16]struct{}{1: struct{}{}}
 	fakeInternalNextHops := map[uint16]netip.AddrPort{}
 	fakeServices := map[addr.SVC][]netip.AddrPort{}
 
diff --git a/router/dataplane_test.go b/router/dataplane_test.go
index 1c812f9816..9298aa23c6 100644
--- a/router/dataplane_test.go
+++ b/router/dataplane_test.go
@@ -643,7 +643,7 @@ func TestProcessPkt(t *testing.T) {
 	// * The ingress interface has to exist. This fake map is good for most test cases.
 	//   Others need a custom one.
 	// * InternalNextHops may not be nil. Empty is ok (sufficient unless testing AS transit).
-	fakeExternalInterfaces := map[uint16]router.BatchConn{1: nil, 2: nil, 3: nil}
+	fakeExternalInterfaces := map[uint16]struct{}{1: struct{}{}, 2: struct{}{}, 3: struct{}{}}
 	fakeInternalNextHops := map[uint16]netip.AddrPort{}
 
 	testCases := map[string]struct {
@@ -725,8 +725,8 @@ func TestProcessPkt(t *testing.T) {
 		"outbound": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]router.BatchConn{
-						uint16(1): mock_router.NewMockBatchConn(ctrl),
+					map[uint16]struct{}{
+						uint16(1): struct{}{},
 					},
 					map[uint16]topology.LinkType{
 						1: topology.Child,
@@ -759,9 +759,9 @@ func TestProcessPkt(t *testing.T) {
 		"brtransit": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]router.BatchConn{
-						uint16(1): mock_router.NewMockBatchConn(ctrl),
-						uint16(2): mock_router.NewMockBatchConn(ctrl),
+					map[uint16]struct{}{
+						uint16(1): struct{}{},
+						uint16(2): struct{}{},
 					},
 					map[uint16]topology.LinkType{
 						1: topology.Parent,
@@ -794,9 +794,9 @@ func TestProcessPkt(t *testing.T) {
 		"brtransit non consdir": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]router.BatchConn{
-						uint16(1): mock_router.NewMockBatchConn(ctrl),
-						uint16(2): mock_router.NewMockBatchConn(ctrl),
+					map[uint16]struct{}{
+						uint16(1): struct{}{},
+						uint16(2): struct{}{},
 					},
 					map[uint16]topology.LinkType{
 						2: topology.Parent,
@@ -830,9 +830,9 @@ func TestProcessPkt(t *testing.T) {
 		"brtransit peering consdir": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]router.BatchConn{
-						uint16(1): mock_router.NewMockBatchConn(ctrl),
-						uint16(2): mock_router.NewMockBatchConn(ctrl),
+					map[uint16]struct{}{
+						uint16(1): struct{}{},
+						uint16(2): struct{}{},
 					},
 					map[uint16]topology.LinkType{
 						1: topology.Peer,
@@ -900,9 +900,9 @@ func TestProcessPkt(t *testing.T) {
 		"brtransit peering non consdir": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]router.BatchConn{
-						uint16(1): mock_router.NewMockBatchConn(ctrl),
-						uint16(2): mock_router.NewMockBatchConn(ctrl),
+					map[uint16]struct{}{
+						uint16(1): struct{}{},
+						uint16(2): struct{}{},
 					},
 					map[uint16]topology.LinkType{
 						1: topology.Peer,
@@ -977,9 +977,9 @@ func TestProcessPkt(t *testing.T) {
 			// happens on the next hop.
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]router.BatchConn{
-						uint16(1): mock_router.NewMockBatchConn(ctrl),
-						uint16(2): mock_router.NewMockBatchConn(ctrl),
+					map[uint16]struct{}{
+						uint16(1): struct{}{},
+						uint16(2): struct{}{},
 					},
 					map[uint16]topology.LinkType{
 						1: topology.Peer,
@@ -1051,9 +1051,9 @@ func TestProcessPkt(t *testing.T) {
 		"peering non consdir upstream": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]router.BatchConn{
-						uint16(1): mock_router.NewMockBatchConn(ctrl),
-						uint16(2): mock_router.NewMockBatchConn(ctrl),
+					map[uint16]struct{}{
+						uint16(1): struct{}{},
+						uint16(2): struct{}{},
 					},
 					map[uint16]topology.LinkType{
 						1: topology.Peer,
@@ -1133,8 +1133,8 @@ func TestProcessPkt(t *testing.T) {
 		"astransit direct": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]router.BatchConn{
-						uint16(1): mock_router.NewMockBatchConn(ctrl),
+					map[uint16]struct{}{
+						uint16(1): struct{}{},
 						// Interface 3 isn't in the external interfaces of this router
 						// another router has it.
 					},
@@ -1168,8 +1168,8 @@ func TestProcessPkt(t *testing.T) {
 		"astransit xover": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]router.BatchConn{
-						uint16(51): mock_router.NewMockBatchConn(ctrl),
+					map[uint16]struct{}{
+						uint16(51): struct{}{},
 					},
 					map[uint16]topology.LinkType{
 						51: topology.Child,
@@ -1365,8 +1365,8 @@ func TestProcessPkt(t *testing.T) {
 		"reversed onehop outbound": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]router.BatchConn{
-						uint16(1): mock_router.NewMockBatchConn(ctrl),
+					map[uint16]struct{}{
+						uint16(1): struct{}{},
 					},
 					nil,
 					mock_router.NewMockBatchConn(ctrl),
@@ -1425,8 +1425,8 @@ func TestProcessPkt(t *testing.T) {
 		"onehop outbound": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]router.BatchConn{
-						uint16(2): mock_router.NewMockBatchConn(ctrl),
+					map[uint16]struct{}{
+						uint16(2): struct{}{},
 					},
 					nil,
 					mock_router.NewMockBatchConn(ctrl),
diff --git a/router/export_test.go b/router/export_test.go
index 1c87bcce8b..9a8744f9e0 100644
--- a/router/export_test.go
+++ b/router/export_test.go
@@ -66,7 +66,7 @@ func NewPacket(raw []byte, src, dst *net.UDPAddr, ingress, egress uint16) *Packe
 }
 
 func NewDP(
-	external map[uint16]BatchConn,
+	external map[uint16]struct{},
 	linkTypes map[uint16]topology.LinkType,
 	internal BatchConn,
 	internalNextHops map[uint16]netip.AddrPort,

From d16ad11cac75077123820fc9057980b7aca7539b Mon Sep 17 00:00:00 2001
From: Jean-Christophe Hugly <jice@scion.org>
Date: Fri, 24 Jan 2025 17:10:20 +0100
Subject: [PATCH 02/11] Move a large part of underlay-specific code out of the
 main router.

The underlay code is now the udpip underlay provider.

The refactoring is not complete. The assumption that underlay addresses are
udp/ip addresses is still part of the router. Also, the main receiver and
sender tasks are still where they are and use the underlay connections via
a temporary API. The reason for that is to make the diff somewhat more
reviewable by not moving large swaths of code from one file to another.
---
 router/BUILD.bazel                   |   2 +
 router/cmd/router/BUILD.bazel        |   1 +
 router/cmd/router/main.go            |   1 +
 router/dataplane.go                  | 592 +++++++++------------------
 router/dataplane_internal_test.go    |  59 +--
 router/dataplane_test.go             |  82 +++-
 router/export_test.go                |  40 +-
 router/metrics.go                    |  15 +-
 router/underlay.go                   | 114 ++++++
 router/underlayproviders/BUILD.bazel |   9 +
 router/underlayproviders/udpip.go    | 330 +++++++++++++++
 11 files changed, 780 insertions(+), 465 deletions(-)
 create mode 100644 router/underlay.go
 create mode 100644 router/underlayproviders/BUILD.bazel
 create mode 100644 router/underlayproviders/udpip.go

diff --git a/router/BUILD.bazel b/router/BUILD.bazel
index b02a0a98e0..17a374f39d 100644
--- a/router/BUILD.bazel
+++ b/router/BUILD.bazel
@@ -10,6 +10,7 @@ go_library(
         "metrics.go",
         "serialize_proxy.go",
         "svc.go",
+        "underlay.go",
     ],
     importpath = "github.com/scionproto/scion/router",
     visibility = ["//visibility:public"],
@@ -69,6 +70,7 @@ go_test(
         "//private/underlay/conn:go_default_library",
         "//router/control:go_default_library",
         "//router/mock_router:go_default_library",
+        "//router/underlayproviders:go_default_library",
         "@com_github_golang_mock//gomock:go_default_library",
         "@com_github_google_gopacket//:go_default_library",
         "@com_github_google_gopacket//layers:go_default_library",
diff --git a/router/cmd/router/BUILD.bazel b/router/cmd/router/BUILD.bazel
index f1014ba346..20b4f28c45 100644
--- a/router/cmd/router/BUILD.bazel
+++ b/router/cmd/router/BUILD.bazel
@@ -17,6 +17,7 @@ go_library(
         "//router/config:go_default_library",
         "//router/control:go_default_library",
         "//router/mgmtapi:go_default_library",
+        "//router/underlayproviders:go_default_library",
         "@com_github_go_chi_chi_v5//:go_default_library",
         "@com_github_go_chi_cors//:go_default_library",
         "@org_golang_x_sync//errgroup:go_default_library",
diff --git a/router/cmd/router/main.go b/router/cmd/router/main.go
index df605f4d73..fb599391b5 100644
--- a/router/cmd/router/main.go
+++ b/router/cmd/router/main.go
@@ -37,6 +37,7 @@ import (
 	"github.com/scionproto/scion/router/config"
 	"github.com/scionproto/scion/router/control"
 	api "github.com/scionproto/scion/router/mgmtapi"
+	_ "github.com/scionproto/scion/router/underlayproviders"
 )
 
 var globalCfg config.Config
diff --git a/router/dataplane.go b/router/dataplane.go
index f690c3920f..8530ab8530 100644
--- a/router/dataplane.go
+++ b/router/dataplane.go
@@ -78,7 +78,7 @@ const (
 	is32bit = 1 - (ptrSize-4)/4
 )
 
-type bfdSession interface {
+type BfdSession interface {
 	Run(ctx context.Context) error
 	ReceiveMessage(*layers.BFD)
 	IsUp() bool
@@ -91,27 +91,15 @@ type BatchConn interface {
 	Close() error
 }
 
-// LinkScope describes the kind (or scope) of a link: internal, sibling, or external.
-type linkScope int
-
-const (
-	internal linkScope = iota // to/from end-hosts in the local AS
-	sibling                   // to/from (external interfaces owned by) a sibling router
-	external                  // to/from routers in another AS
-)
+// underlay is a pointer to our underlay provider.
+//
+// incremental refactoring: this allows for a single underlay. In the future, each link could be
+// via a different underlay. That would have to be supported by the configuration code and there
+// would likely be a registry of underlays. For now, That's the whole registry.
+var newUnderlay func() UnderlayProvider
 
-// link embodies the router's idea of an underlay connection. It associates the underlay connection,
-// with a bfdSession, a destination address, (and more in the future). It also allows the send
-// operation to be underlay-dependant. It can be (in the future) registered with the underlay
-// implementation so incoming packets can be easily associated to a link.
-// TODO(jiceatscion): use inheritence between implementations.
-type link interface {
-	getScope() linkScope
-	getBfdSession() bfdSession
-	isUp() bool
-	getRemote() netip.AddrPort
-	getIfID() uint16
-	send(p *packet) bool
+func AddUnderlay(newProvider func() UnderlayProvider) {
+	newUnderlay = newProvider
 }
 
 type disposition int
@@ -123,7 +111,7 @@ const (
 	pDone
 )
 
-// packet aggregates buffers and ancillary metadata related to one packet.
+// Packet aggregates buffers and ancillary metadata related to one packet.
 // That is everything we need to pass-around while processing a packet. The motivation is to save on
 // copy (pass everything via one reference) AND garbage collection (reuse everything).
 // The buffer is allocated in a separate location (but still reused) to keep the packet structures
@@ -133,7 +121,7 @@ const (
 // arch) until Slowpath request which is 6 bytes long. The rest is in decreasing order of size and
 // size-aligned. We want to fit neatly into cache lines, so we need to fit in 64 bytes. The padding
 // required to occupy exactly 64 bytes depends on the architecture.
-type packet struct {
+type Packet struct {
 	// The useful part of the raw packet at a point in time (i.e. a slice of the full buffer).
 	// It can be any portion of the full buffer; not necessarily the start.
 	rawPacket []byte
@@ -145,7 +133,7 @@ type packet struct {
 	srcAddr *net.UDPAddr
 	// The address to where we are forwarding the packet.
 	// Will be set by the processing routine; it is updated in-place.
-	dstAddr *net.UDPAddr
+	DstAddr *net.UDPAddr
 	// Additional metadata in case the packet is put on the slow path. Updated in-place.
 	slowPathRequest slowPathRequest
 	// The ingress on which this packet arrived. This is set by the receiver.
@@ -169,219 +157,40 @@ type slowPathRequest struct {
 }
 
 // Make sure that the packet structure has the size we expect.
-const _ uintptr = 64 - unsafe.Sizeof(packet{}) // assert 64 >= sizeof(packet)
-const _ uintptr = unsafe.Sizeof(packet{}) - 64 // assert sizeof(packet) >= 64
+const _ uintptr = 64 - unsafe.Sizeof(Packet{}) // assert 64 >= sizeof(Packet)
+const _ uintptr = unsafe.Sizeof(Packet{}) - 64 // assert sizeof(Packet) >= 64
 
 // initPacket configures the given blank packet (and returns it, for convenience).
-func (p *packet) init(buffer *[bufSize]byte) *packet {
+func (p *Packet) init(buffer *[bufSize]byte) *Packet {
 	p.buffer = buffer
 	p.rawPacket = p.buffer[:]
-	p.dstAddr = &net.UDPAddr{IP: make(net.IP, net.IPv6len)}
+	p.DstAddr = &net.UDPAddr{IP: make(net.IP, net.IPv6len)}
 	return p
 }
 
 // reset() makes the packet ready to receive a new underlay message.
 // A cleared dstAddr is represented with a zero-length IP so we keep reusing the IP storage bytes.
-func (p *packet) reset() {
-	p.dstAddr.IP = p.dstAddr.IP[0:0] // We're keeping the object, just blank it.
-	*p = packet{
+func (p *Packet) reset() {
+	p.DstAddr.IP = p.DstAddr.IP[0:0] // We're keeping the object, just blank it.
+	*p = Packet{
 		buffer:    p.buffer,    // keep the buffer
 		rawPacket: p.buffer[:], // restore the full packet capacity
-		dstAddr:   p.dstAddr,   // keep the dstAddr and so the IP slice and bytes
+		DstAddr:   p.DstAddr,   // keep the dstAddr and so the IP slice and bytes
 	}
 	// Everything else is reset to zero value.
 }
 
-// An external link is connection oriented. And is not shared. As a result, it has a defined
-// interfaceID and a fixed destination address.
-type externalLink struct {
-	// Each link has a queue, consumed by a forwarding task and fed by bfd senders
-	// and packet processors.
-	queue      chan *packet
-	bfdSession bfdSession
-	remote     netip.AddrPort
-	ifID       uint16
-}
-
-func newExternalLink(
-	queue chan *packet,
-	bfdSession bfdSession,
-	remote netip.AddrPort,
-	ifID uint16,
-) *externalLink {
-
-	return &externalLink{
-		queue:      queue,
-		bfdSession: bfdSession,
-		remote:     remote,
-		ifID:       ifID,
-	}
-}
-
-func (l *externalLink) getScope() linkScope {
-	return external
-}
-
-func (l *externalLink) getBfdSession() bfdSession {
-	return l.bfdSession
-}
-
-func (l *externalLink) isUp() bool {
-	return l.bfdSession == nil || l.bfdSession.IsUp()
-}
-
-func (l *externalLink) getRemote() netip.AddrPort {
-	return l.remote
-}
-
-func (l *externalLink) getIfID() uint16 {
-	return l.ifID
-}
-
-// The goal is to have send do those things that are dependent on the underlay connection type
-// such as updating the destination address of the pkt with the known bound one.
-// TODO(jiceatscion): indeed, do it.
-func (l *externalLink) send(p *packet) bool {
-	select {
-	case l.queue <- p:
-	default:
-		return false
-	}
-	return true
-}
-
-// A siblingLink is connection oriented. However, it may be shared between multiple interface IDs.
-// So, it has no defined interface ID but a fixed destination address.
-type siblingLink struct {
-	// Each link has a queue, consumed by a forwarding task and fed by bfd senders
-	// and packet processors.
-	queue      chan *packet
-	bfdSession bfdSession
-	remote     netip.AddrPort
-	ifID       uint16
-}
-
-func newSiblingLink(
-	queue chan *packet,
-	bfdSession bfdSession,
-	remote netip.AddrPort,
-) *siblingLink {
-
-	return &siblingLink{
-		queue:      queue,
-		bfdSession: bfdSession,
-		remote:     remote,
-	}
-}
-
-func (l *siblingLink) getScope() linkScope {
-	return sibling
-}
-func (l *siblingLink) getBfdSession() bfdSession {
-	return l.bfdSession
-}
-
-func (l *siblingLink) isUp() bool {
-	return l.bfdSession == nil || l.bfdSession.IsUp()
-}
-
-func (l *siblingLink) getRemote() netip.AddrPort {
-	return l.remote
-}
-
-func (l *siblingLink) getIfID() uint16 {
-	return 0
-}
-
-func (l *siblingLink) send(p *packet) bool {
-	select {
-	case l.queue <- p:
-	default:
-		return false
-	}
-	return true
-}
-
-// An internalLink is not connection oriented and it is not associated with an AS border interface.
-// So, it has no defined interface ID and no fixed destination address. It can't have a BFD session
-// either. So, it's a link only in name. May be, in the future, we find it more convenient to not
-// treat such connections as links. For now, it helps with code migration. Incoming packets are
-// associated with the link they came in through by way of the underlay source address.
-type internalLink struct {
-	// Each link has a queue, consumed by a forwarding task and fed by bfd senders
-	// and packet processors.
-	queue chan *packet
-}
-
-func newInternalLink(queue chan *packet) *internalLink {
-	return &internalLink{
-		queue: queue,
-	}
-}
-
-func (l *internalLink) getScope() linkScope {
-	return internal
-}
-
-func (l *internalLink) isUp() bool {
-	return true
-}
-
-func (l *internalLink) getBfdSession() bfdSession {
-	return nil
-}
-
-func (l *internalLink) getRemote() netip.AddrPort {
-	return netip.AddrPort{ /* fixme; we have a special value */ }
-}
-
-func (l *internalLink) getIfID() uint16 {
-	return 0
-}
-
-func (l *internalLink) send(p *packet) bool {
-	select {
-	case l.queue <- p:
-	default:
-		return false
-	}
-	return true
-}
-
-// Connections can be shared between links. So we keep a cannonical map.
-// Also, there must be one sending loop and one channel per connection. So that goes here.
-// This business should be moved to underlay implementations, but... one problem at a time.
-// So, this is a minimal implementation to support the single underlay we have today.
-type forwarder struct {
-	conn BatchConn
-	fwQ  chan *packet
-	ifID uint16 // non-zero only when not shared bw links.
-}
-
-func newForwarder(conn BatchConn, queueSize int, ifID uint16) *forwarder {
-	return &forwarder{
-		conn: conn,
-		fwQ:  make(chan *packet, queueSize),
-		ifID: ifID,
-	}
-}
-
 // DataPlane contains a SCION Border Router's forwarding logic. It reads packets
 // from multiple sockets, performs routing, and sends them to their destinations
 // (after updating the path, if that is needed).
 type DataPlane struct {
-	links               map[netip.AddrPort]link
-	forwarders          map[netip.AddrPort]*forwarder // We do this here as a refactoring step.
-	interfaces          map[uint16]link
-	external            map[uint16]struct{} // TODO: to be replaced w/ e.g. interfaces[x].getScope()
+	underlay            UnderlayProvider
+	interfaces          map[uint16]Link
 	linkTypes           map[uint16]topology.LinkType
 	neighborIAs         map[uint16]addr.IA
-	peerInterfaces      map[uint16]uint16
 	internalIP          netip.Addr
-	internalNextHops    map[uint16]netip.AddrPort
 	svc                 *services
 	macFactory          func() hash.Hash
-	bfdSessions         map[uint16]bfdSession
 	localIA             addr.IA
 	mtx                 sync.Mutex
 	running             atomic.Bool
@@ -401,7 +210,7 @@ type DataPlane struct {
 	// packet structure is fetched from the pool passed-around through the various channels and
 	// returned to the pool. To reduce the cost of copying, the packet structure is passed by
 	// reference.
-	packetPool chan *packet
+	packetPool chan *Packet
 }
 
 var (
@@ -535,18 +344,17 @@ func (d *DataPlane) AddInternalInterface(conn BatchConn, ip netip.Addr) error {
 	if conn == nil {
 		return emptyValue
 	}
+	if d.underlay == nil {
+		d.underlay = newUnderlay()
+	}
 	if d.interfaces == nil {
-		d.interfaces = make(map[uint16]link)
-		d.forwarders = make(map[netip.AddrPort]*forwarder)
+		d.interfaces = make(map[uint16]Link)
 	} else if d.interfaces[0] != nil {
 		return alreadySet
 	}
 
-	f := newForwarder(conn, d.RunConfig.BatchSize, 0)
-	d.forwarders[netip.AddrPort{}] = f
-	d.interfaces[0] = newInternalLink(f.fwQ)
-	// That is not a real link => not in the links map; should be?
-
+	l := d.underlay.NewInternalLink(conn, d.RunConfig.BatchSize)
+	d.interfaces[0] = l
 	d.internalIP = ip
 
 	return nil
@@ -567,31 +375,21 @@ func (d *DataPlane) AddExternalInterface(ifID uint16, conn BatchConn,
 	if conn == nil || !src.Addr.IsValid() || !dst.Addr.IsValid() {
 		return emptyValue
 	}
+	if d.underlay == nil {
+		d.underlay = newUnderlay()
+	}
 	bfd, err := d.addExternalInterfaceBFD(ifID, src, dst, cfg)
 	if err != nil {
 		return serrors.Wrap("adding external BFD", err, "if_id", ifID)
 	}
-	if d.external == nil {
-		d.external = make(map[uint16]struct{})
-	}
 	if d.interfaces == nil {
-		d.interfaces = make(map[uint16]link)
+		d.interfaces = make(map[uint16]Link)
 	}
-	if d.forwarders == nil {
-		d.forwarders = make(map[netip.AddrPort]*forwarder)
-	}
-	if _, exists := d.external[ifID]; exists {
+	if _, exists := d.interfaces[ifID]; exists {
 		return serrors.JoinNoStack(alreadySet, nil, "ifID", ifID)
 	}
-	if d.forwarders[dst.Addr] != nil {
-		return serrors.JoinNoStack(alreadySet, nil, "dst Addr", dst.Addr)
-	}
-	fw := newForwarder(conn, d.RunConfig.BatchSize, ifID)
-	lk := newExternalLink(fw.fwQ, bfd, dst.Addr, ifID)
-	d.forwarders[dst.Addr] = fw
-	d.links[dst.Addr] = lk
+	lk := d.underlay.NewExternalLink(conn, d.RunConfig.BatchSize, bfd, dst.Addr, ifID)
 	d.interfaces[ifID] = lk
-	d.external[ifID] = struct{}{}
 	return nil
 }
 
@@ -636,27 +434,9 @@ func (d *DataPlane) AddLinkType(ifID uint16, linkTo topology.LinkType) error {
 	return nil
 }
 
-// AddRemotePeer adds the remote peering interface ID for local
-// interface ID.  If the link type for the given ID is already set to
-// a different type, this method will return an error. This can only
-// be called on a not yet running dataplane.
-func (d *DataPlane) AddRemotePeer(local, remote uint16) error {
-	if t, ok := d.linkTypes[local]; ok && t != topology.Peer {
-		return serrors.JoinNoStack(unsupportedPathType, nil, "type", t)
-	}
-	if _, exists := d.peerInterfaces[local]; exists {
-		return serrors.JoinNoStack(alreadySet, nil, "local_interface", local)
-	}
-	if d.peerInterfaces == nil {
-		d.peerInterfaces = make(map[uint16]uint16)
-	}
-	d.peerInterfaces[local] = remote
-	return nil
-}
-
 // AddExternalInterfaceBFD adds the inter AS connection BFD session.
 func (d *DataPlane) addExternalInterfaceBFD(ifID uint16,
-	src, dst control.LinkEnd, cfg control.BFD) (bfdSession, error) {
+	src, dst control.LinkEnd, cfg control.BFD) (BfdSession, error) {
 
 	if *cfg.Disable {
 		return nil, nil
@@ -686,14 +466,14 @@ func (d *DataPlane) addExternalInterfaceBFD(ifID uint16,
 // returns InterfaceUp if the relevant bfdsession state is up, or if there is no BFD
 // session. Otherwise, it returns InterfaceDown.
 func (d *DataPlane) getInterfaceState(ifID uint16) control.InterfaceState {
-	if l := d.interfaces[ifID]; l != nil && !l.isUp() {
+	if l := d.interfaces[ifID]; l != nil && !l.IsUp() {
 		return control.InterfaceDown
 	}
 	return control.InterfaceUp
 }
 
 func (d *DataPlane) addBFDController(ifID uint16, s bfd.Sender, cfg control.BFD,
-	metrics bfd.Metrics) (bfdSession, error) {
+	metrics bfd.Metrics) (BfdSession, error) {
 
 	// Generate random discriminator. It can't be zero.
 	discInt, err := rand.Int(rand.Reader, big.NewInt(0xfffffffe))
@@ -767,27 +547,22 @@ func (d *DataPlane) AddNextHop(ifID uint16, src, dst netip.AddrPort, cfg control
 	if !dst.IsValid() || !src.IsValid() {
 		return emptyValue
 	}
+	if d.underlay == nil {
+		d.underlay = newUnderlay()
+	}
 	bfd, err := d.addNextHopBFD(ifID, src, dst, cfg, sibling)
 	if err != nil {
 		return serrors.Wrap("adding next hop BFD", err, "if_id", ifID)
 	}
-	if d.internalNextHops == nil {
-		d.internalNextHops = make(map[uint16]netip.AddrPort)
+	if d.interfaces == nil {
+		d.interfaces = make(map[uint16]Link)
 	}
-	if _, exists := d.internalNextHops[ifID]; exists {
+	if _, exists := d.interfaces[ifID]; exists {
 		return serrors.JoinNoStack(alreadySet, nil, "ifID", ifID)
 	}
 
-	// We have one link per unique dst address.
-	sib := d.links[dst]
-	if sib == nil {
-		// As a refactoring step, re-use the internal connection's forwarder, which is what we've
-		// been effectively doing so far.
-		sib := newSiblingLink(d.forwarders[netip.AddrPort{}].fwQ, bfd, dst)
-		d.links[dst] = sib
-	}
+	sib := d.underlay.NewSiblingLink(d.RunConfig.BatchSize, bfd, dst)
 	d.interfaces[ifID] = sib
-	d.internalNextHops[ifID] = dst
 	return nil
 }
 
@@ -795,7 +570,7 @@ func (d *DataPlane) AddNextHop(ifID uint16, src, dst netip.AddrPort, cfg control
 // If the remote ifID belongs to an existing address, the existing
 // BFD session will be re-used.
 func (d *DataPlane) addNextHopBFD(ifID uint16, src, dst netip.AddrPort, cfg control.BFD,
-	sibling string) (bfdSession, error) {
+	sibling string) (BfdSession, error) {
 
 	if *cfg.Disable {
 		return nil, nil
@@ -835,24 +610,28 @@ func (d *DataPlane) Run(ctx context.Context) error {
 	d.mtx.Lock()
 	d.initMetrics()
 
+	// incremental refactoring: we leave all the ingest work here for now, so we get the set of
+	// connections from the underlay.
+	underlayConnections := d.underlay.GetConnections()
 	processorQueueSize := max(
-		len(d.forwarders)*d.RunConfig.BatchSize/d.RunConfig.NumProcessors,
+		len(underlayConnections)*d.RunConfig.BatchSize/d.RunConfig.NumProcessors,
 		d.RunConfig.BatchSize)
 
 	d.initPacketPool(processorQueueSize)
 	procQs, slowQs := d.initQueues(processorQueueSize)
 
 	d.setRunning()
-	for _, f := range d.forwarders {
-		go func(f *forwarder) {
+	for _, c := range underlayConnections {
+		go func(c UnderlayConnection) {
 			defer log.HandlePanic()
-			d.runReceiver(f, procQs)
-		}(f)
-		go func(f *forwarder) {
+			d.runReceiver(c, procQs)
+		}(c)
+		go func(c UnderlayConnection) {
 			defer log.HandlePanic()
-			d.runForwarder(f)
-		}(f)
+			d.runForwarder(c)
+		}(c)
 	}
+
 	for i := 0; i < d.RunConfig.NumProcessors; i++ {
 		go func(i int) {
 			defer log.HandlePanic()
@@ -866,13 +645,17 @@ func (d *DataPlane) Run(ctx context.Context) error {
 		}(i)
 	}
 
-	for a, l := range d.links {
-		go func(a netip.AddrPort, s bfdSession) {
+	for a, l := range d.underlay.GetLinks() {
+		s := l.GetBfdSession()
+		if s == nil {
+			continue
+		}
+		go func() {
 			defer log.HandlePanic()
 			if err := s.Run(ctx); err != nil && err != bfd.AlreadyRunning {
 				log.Error("BFD session failed to start", "remote address", a, "err", err)
 			}
-		}(a, l.getBfdSession())
+		}()
 	}
 
 	d.mtx.Unlock()
@@ -888,32 +671,35 @@ func (d *DataPlane) initPacketPool(processorQueueSize int) {
 		len(d.interfaces)*(2*d.RunConfig.BatchSize)
 
 	log.Debug("Initialize packet pool of size", "poolSize", poolSize)
-	d.packetPool = make(chan *packet, poolSize)
+	d.packetPool = make(chan *Packet, poolSize)
 	pktBuffers := make([][bufSize]byte, poolSize)
-	pktStructs := make([]packet, poolSize)
+	pktStructs := make([]Packet, poolSize)
 	for i := 0; i < poolSize; i++ {
 		d.packetPool <- pktStructs[i].init(&pktBuffers[i])
 	}
 }
 
 // initializes the processing routines and queues
-func (d *DataPlane) initQueues(processorQueueSize int) ([]chan *packet, []chan *packet) {
+func (d *DataPlane) initQueues(processorQueueSize int) ([]chan *Packet, []chan *Packet) {
 
-	procQs := make([]chan *packet, d.RunConfig.NumProcessors)
+	procQs := make([]chan *Packet, d.RunConfig.NumProcessors)
 	for i := 0; i < d.RunConfig.NumProcessors; i++ {
-		procQs[i] = make(chan *packet, processorQueueSize)
+		procQs[i] = make(chan *Packet, processorQueueSize)
 	}
-	slowQs := make([]chan *packet, d.RunConfig.NumSlowPathProcessors)
+	slowQs := make([]chan *Packet, d.RunConfig.NumSlowPathProcessors)
 	for i := 0; i < d.RunConfig.NumSlowPathProcessors; i++ {
-		slowQs[i] = make(chan *packet, processorQueueSize)
+		slowQs[i] = make(chan *Packet, processorQueueSize)
 	}
 	return procQs, slowQs
 }
 
-func (d *DataPlane) runReceiver(f *forwarder, procQs []chan *packet) {
+// runReceiver handles incoming traffic from the given connection.
+//
+// Incremental refactoring: we should not have direct knowledge of connections. Later we will move
+// parts of this to the connection itself, so that we don't have to know.
+func (d *DataPlane) runReceiver(c UnderlayConnection, procQs []chan *Packet) {
 
-	// TODO(jiceatscion): ifID isn't veyr unique. It is zero for internal and all sibling links.
-	log.Debug("Run receiver for", "interface", f.ifID)
+	log.Debug("Run receiver", "connection", c.Name())
 
 	// Each receiver (therefore each input interface) has a unique random seed for the procID hash
 	// function.
@@ -922,8 +708,8 @@ func (d *DataPlane) runReceiver(f *forwarder, procQs []chan *packet) {
 	if _, err := rand.Read(randomBytes); err != nil {
 		panic("Error while generating random value")
 	}
-	for _, c := range randomBytes {
-		hashSeed = hashFNV1a(hashSeed, c)
+	for _, b := range randomBytes {
+		hashSeed = hashFNV1a(hashSeed, b)
 	}
 
 	// A collection of socket messages, as the readBatch API expects them. We keep using the same
@@ -932,12 +718,12 @@ func (d *DataPlane) runReceiver(f *forwarder, procQs []chan *packet) {
 
 	// An array of corresponding packet references. Each corresponds to one msg.
 	// The packet owns the buffer that we set in the matching msg, plus the metadata that we'll add.
-	packets := make([]*packet, d.RunConfig.BatchSize)
+	packets := make([]*Packet, d.RunConfig.BatchSize)
 
-	numReusable := 0                       // unused buffers from previous loop
-	metrics := d.forwardingMetrics[f.ifID] // If receiver exists, fw metrics exist too.
+	numReusable := 0                         // unused buffers from previous loop
+	metrics := d.forwardingMetrics[c.IfID()] // If receiver exists, fw metrics exist too.
 
-	enqueueForProcessing := func(size int, srcAddr *net.UDPAddr, pkt *packet) {
+	enqueueForProcessing := func(size int, srcAddr *net.UDPAddr, pkt *Packet) {
 		sc := classOfSize(size)
 		metrics[sc].InputPacketsTotal.Inc()
 		metrics[sc].InputBytesTotal.Add(float64(size))
@@ -951,7 +737,10 @@ func (d *DataPlane) runReceiver(f *forwarder, procQs []chan *packet) {
 		}
 
 		pkt.rawPacket = pkt.rawPacket[:size] // Update size; readBatch does not.
-		pkt.ingress = f.ifID
+		// Incremental refactoring: We should begin with finding the link and get the ifID
+		// from there. We will do that once we actually move these pre-processing tasks directly
+		// into the underlay.
+		pkt.ingress = c.IfID()
 		pkt.srcAddr = srcAddr
 		select {
 		case procQs[procID] <- pkt:
@@ -973,10 +762,10 @@ func (d *DataPlane) runReceiver(f *forwarder, procQs []chan *packet) {
 		}
 
 		// Fill the packets
-		numPkts, err := f.conn.ReadBatch(msgs)
+		numPkts, err := c.Conn().ReadBatch(msgs)
 		numReusable = len(msgs) - numPkts
 		if err != nil {
-			log.Debug("Error while reading batch", "interfaceID", f.ifID, "err", err)
+			log.Debug("Error while reading batch", "interfaceID", c.IfID(), "err", err)
 			continue
 		}
 		for i, msg := range msgs[:numPkts] {
@@ -1012,15 +801,15 @@ func computeProcID(data []byte, numProcRoutines int, hashSeed uint32) (uint32, e
 	return s % uint32(numProcRoutines), nil
 }
 
-func (d *DataPlane) getPacketFromPool() *packet {
+func (d *DataPlane) getPacketFromPool() *Packet {
 	return <-d.packetPool
 }
 
-func (d *DataPlane) returnPacketToPool(pkt *packet) {
+func (d *DataPlane) returnPacketToPool(pkt *Packet) {
 	d.packetPool <- pkt
 }
 
-func (d *DataPlane) runProcessor(id int, q <-chan *packet, slowQ chan<- *packet) {
+func (d *DataPlane) runProcessor(id int, q <-chan *Packet, slowQ chan<- *Packet) {
 
 	log.Debug("Initialize processor with", "id", id)
 	processor := newPacketProcessor(d)
@@ -1066,14 +855,14 @@ func (d *DataPlane) runProcessor(id int, q <-chan *packet, slowQ chan<- *packet)
 			d.returnPacketToPool(p)
 			continue
 		}
-		if !fwLink.send(p) {
+		if !fwLink.Send(p) {
 			d.returnPacketToPool(p)
 			metrics.DroppedPacketsBusyForwarder.Inc()
 		}
 	}
 }
 
-func (d *DataPlane) runSlowPathProcessor(id int, q <-chan *packet) {
+func (d *DataPlane) runSlowPathProcessor(id int, q <-chan *Packet) {
 
 	log.Debug("Initialize slow-path processor with", "id", id)
 	processor := newSlowPathProcessor(d)
@@ -1097,7 +886,7 @@ func (d *DataPlane) runSlowPathProcessor(id int, q <-chan *packet) {
 			d.returnPacketToPool(p)
 			continue
 		}
-		if !fwLink.send(p) {
+		if !fwLink.Send(p) {
 			d.returnPacketToPool(p)
 		}
 	}
@@ -1120,7 +909,7 @@ func newSlowPathProcessor(d *DataPlane) *slowPathPacketProcessor {
 
 type slowPathPacketProcessor struct {
 	d   *DataPlane
-	pkt *packet
+	pkt *Packet
 
 	scionLayer slayers.SCION
 	hbhLayer   slayers.HopByHopExtnSkipper
@@ -1147,7 +936,7 @@ func (p *slowPathPacketProcessor) reset() {
 	p.e2eLayer = slayers.EndToEndExtnSkipper{}
 }
 
-func (p *slowPathPacketProcessor) processPacket(pkt *packet) error {
+func (p *slowPathPacketProcessor) processPacket(pkt *Packet) error {
 	var err error
 	p.reset()
 	p.pkt = pkt
@@ -1205,7 +994,7 @@ func (p *slowPathPacketProcessor) processPacket(pkt *packet) error {
 	}
 }
 
-func updateOutputMetrics(metrics interfaceMetrics, packets []*packet) {
+func updateOutputMetrics(metrics interfaceMetrics, packets []*Packet) {
 	// We need to collect stats by traffic type and size class.
 	// Try to reduce the metrics lookup penalty by using some
 	// simpler staging data structure.
@@ -1228,14 +1017,28 @@ func updateOutputMetrics(metrics interfaceMetrics, packets []*packet) {
 	}
 }
 
-// It would make more sense for this to be part of Forwarder
-func (d *DataPlane) runForwarder(f *forwarder) {
+// Big issue with metrics and ifID. If an underlay connection must be shared between links
+// (for example, libling links), then we don't have a specific ifID in the connection per se. It
+// changes for each packet. As a result, in the shared case, either we account all metrics to
+// whatever placeholder ifID we have (i.e. 0), or we have to use pkt.egress and lookup the metrics
+// in the map for each packet. This is too expensive.
+//
+// Mitigations:
+//   - use ifID even if it is 0 for sibling links - no worse than before, since sibling links were
+//     already redirected to interface 0. Ok, until we have fully shared forwarders (like with an
+//     XDP underlay impl).
+//   - stage our own internal metrics map, sorted by ifID = pkt.egress, and batch update the
+//     metrics... might not be much cheaper than the naive way.
+//   - Use one fw queue per ifID in each connection... but then have to round-robin for fairness....
+//     smaller batches?
+//
+// For now, we do the first option.
+func (d *DataPlane) runForwarder(c UnderlayConnection) {
 
-	// TODO(jiceatscion): ifID isn't so informative; it is zero for internal and all sibling links.
-	log.Debug("Initialize forwarder for", "interface", f.ifID)
+	log.Debug("Run forwarder", "connection", c.Name())
 
 	// We use this somewhat like a ring buffer.
-	pkts := make([]*packet, d.RunConfig.BatchSize)
+	pkts := make([]*Packet, d.RunConfig.BatchSize)
 
 	// We use this as a temporary buffer, but allocate it just once
 	// to save on garbage handling.
@@ -1244,21 +1047,22 @@ func (d *DataPlane) runForwarder(f *forwarder) {
 		msgs[i].Buffers = make([][]byte, 1)
 	}
 
-	metrics := d.forwardingMetrics[f.ifID]
+	metrics := d.forwardingMetrics[c.IfID()]
 
 	toWrite := 0
 	for d.IsRunning() {
-		toWrite += readUpTo(f.fwQ, d.RunConfig.BatchSize-toWrite, toWrite == 0, pkts[toWrite:])
+		// Top-up our batch.
+		toWrite += readUpTo(c.Queue(), d.RunConfig.BatchSize-toWrite, toWrite == 0, pkts[toWrite:])
 
 		// Turn the packets into underlay messages that WriteBatch can send.
 		for i, p := range pkts[:toWrite] {
 			msgs[i].Buffers[0] = p.rawPacket
 			msgs[i].Addr = nil
-			if len(p.dstAddr.IP) != 0 {
-				msgs[i].Addr = p.dstAddr
+			if len(p.DstAddr.IP) != 0 {
+				msgs[i].Addr = p.DstAddr
 			}
 		}
-		written, _ := f.conn.WriteBatch(msgs[:toWrite], 0)
+		written, _ := c.Conn().WriteBatch(msgs[:toWrite], 0)
 		if written < 0 {
 			// WriteBatch returns -1 on error, we just consider this as
 			// 0 packets written
@@ -1288,7 +1092,7 @@ func (d *DataPlane) runForwarder(f *forwarder) {
 	}
 }
 
-func readUpTo(c <-chan *packet, n int, needsBlocking bool, pkts []*packet) int {
+func readUpTo(c <-chan *Packet, n int, needsBlocking bool, pkts []*Packet) int {
 	i := 0
 	if needsBlocking {
 		p, ok := <-c
@@ -1348,7 +1152,7 @@ func errorDiscard(ctx ...any) disposition {
 	return pDiscard
 }
 
-func (p *scionPacketProcessor) processPkt(pkt *packet) disposition {
+func (p *scionPacketProcessor) processPkt(pkt *Packet) disposition {
 	if err := p.reset(); err != nil {
 		return errorDiscard("error", err)
 	}
@@ -1390,26 +1194,33 @@ func (p *scionPacketProcessor) processPkt(pkt *packet) disposition {
 }
 
 func (p *scionPacketProcessor) processInterBFD(oh *onehop.Path, data []byte) disposition {
-	if len(p.d.bfdSessions) == 0 {
-		return errorDiscard("error", noBFDSessionConfigured)
-	}
 
+	// If this is an inter-AS BFD, it can via an interface we own. So the ifID matches one link
+	// and the ifID better be valid. In the future that will be checked upstream from here.
+	l, exists := p.d.interfaces[p.pkt.ingress]
+	if !exists {
+		return errorDiscard("error", noBFDSessionFound)
+	}
+	session := l.GetBfdSession()
+	if session == nil {
+		return errorDiscard("error", noBFDSessionFound)
+	}
 	bfd := &p.bfdLayer
 	if err := bfd.DecodeFromBytes(data, gopacket.NilDecodeFeedback); err != nil {
 		return errorDiscard("error", err)
 	}
-
-	if v, ok := p.d.bfdSessions[p.pkt.ingress]; ok {
-		v.ReceiveMessage(bfd)
-		return pDiscard // All's fine. That packet's journey ends here.
-	}
-
-	return errorDiscard("error", noBFDSessionFound)
+	session.ReceiveMessage(bfd)
+	return pDiscard // All's fine. That packet's journey ends here.
 }
 
 func (p *scionPacketProcessor) processIntraBFD(data []byte) disposition {
-	if len(p.d.bfdSessions) == 0 {
-		return errorDiscard("error", noBFDSessionConfigured)
+
+	// This packet came over a link that doesn't have a define ifID. We have to find it
+	// by srcAddress. We always find one. The internal link matches anything that is not known.
+	src := p.pkt.srcAddr.AddrPort() // POSSIBLY EXPENSIVE CONVERSION
+	session := p.d.underlay.GetLink(src).GetBfdSession()
+	if session == nil {
+		return errorDiscard("error", noBFDSessionFound)
 	}
 
 	bfd := &p.bfdLayer
@@ -1417,21 +1228,8 @@ func (p *scionPacketProcessor) processIntraBFD(data []byte) disposition {
 		return errorDiscard("error", err)
 	}
 
-	ifID := uint16(0)
-	src := p.pkt.srcAddr.AddrPort() // POSSIBLY EXPENSIVE CONVERSION
-	for k, v := range p.d.internalNextHops {
-		if src == v {
-			ifID = k
-			break
-		}
-	}
-
-	if v, ok := p.d.bfdSessions[ifID]; ok {
-		v.ReceiveMessage(bfd)
-		return pDiscard // All's fine. That packet's journey ends here.
-	}
-
-	return errorDiscard("error", noBFDSessionFound)
+	session.ReceiveMessage(bfd)
+	return pDiscard // All's fine. That packet's journey ends here.
 }
 
 func (p *scionPacketProcessor) processSCION() disposition {
@@ -1500,7 +1298,7 @@ type scionPacketProcessor struct {
 	// d is a reference to the dataplane instance that initiated this processor.
 	d *DataPlane
 	// pkt is the packet currently being processed by this processor.
-	pkt *packet
+	pkt *Packet
 	// mac is the hasher for the MAC computation.
 	mac hash.Hash
 
@@ -1568,7 +1366,7 @@ func (p *slowPathPacketProcessor) packSCMP(
 	// The original traffic type, if one had been set, no-longer applies.
 	p.pkt.trafficType = ttOther
 	p.pkt.egress = p.pkt.ingress
-	updateNetAddrFromNetAddr(p.pkt.dstAddr, p.pkt.srcAddr)
+	updateNetAddrFromNetAddr(p.pkt.DstAddr, p.pkt.srcAddr)
 	return nil
 }
 
@@ -1723,19 +1521,21 @@ func (p *scionPacketProcessor) respInvalidDstIA() disposition {
 // Provided that underlying network infrastructure prevents address spoofing,
 // this check prevents malicious end hosts in the local AS from bypassing the
 // SrcIA checks by disguising packets as transit traffic.
+//
+// Incremental refactoring: All or part of this check should move to the underlay.
 func (p *scionPacketProcessor) validateTransitUnderlaySrc() disposition {
 	if p.path.IsFirstHop() || p.pkt.ingress != 0 {
 		// not a transit packet, nothing to check
 		return pForward
 	}
 	pktIngressID := p.ingressInterface()
-	expectedSrc, okE := p.d.internalNextHops[pktIngressID]
-	if !okE {
+	ingressLink := p.d.interfaces[pktIngressID]
+	if ingressLink.GetScope() != Sibling {
 		// Drop
 		return errorDiscard("error", invalidSrcAddrForTransit)
 	}
 	src, okS := netip.AddrFromSlice(p.pkt.srcAddr.IP)
-	if !(okS && expectedSrc.Addr() == src) {
+	if !(okS && ingressLink.GetRemote().Addr() == src) {
 		// Drop
 		return errorDiscard("error", invalidSrcAddrForTransit)
 	}
@@ -1751,12 +1551,13 @@ func (p *scionPacketProcessor) validateTransitUnderlaySrc() disposition {
 // to another AS directly, or via a sibling router.
 func (p *scionPacketProcessor) validateEgressID() disposition {
 	egressID := p.pkt.egress
-	_, ih := p.d.internalNextHops[egressID]
-	_, eh := p.d.external[egressID]
+	link, found := p.d.interfaces[egressID]
+
 	// egress interface must be a known interface
+	// egress is never the internalInterface
 	// packet coming from internal interface, must go to an external interface
-	// packet coming from external interface can go to either internal or external interface
-	if !ih && !eh || (p.pkt.ingress == 0) && !eh {
+	// Note that, for now, ingress == 0 is also true for sibling interfaces. That might change.
+	if !found || (p.pkt.ingress == 0 && link.GetScope() == Sibling) {
 		errCode := slayers.SCMPCodeUnknownHopFieldEgress
 		if !p.infoField.ConsDir {
 			errCode = slayers.SCMPCodeUnknownHopFieldIngress
@@ -1874,7 +1675,7 @@ func (p *scionPacketProcessor) verifyCurrentMAC() disposition {
 }
 
 func (p *scionPacketProcessor) resolveInbound() disposition {
-	err := p.d.resolveLocalDst(p.pkt.dstAddr, p.scionLayer, p.lastLayer)
+	err := p.d.resolveLocalDst(p.pkt.DstAddr, p.scionLayer, p.lastLayer)
 
 	switch err {
 	case nil:
@@ -1965,22 +1766,21 @@ func (p *scionPacketProcessor) egressInterface() uint16 {
 
 func (p *scionPacketProcessor) validateEgressUp() disposition {
 	egressID := p.pkt.egress
-	if v, ok := p.d.bfdSessions[egressID]; ok {
-		if !v.IsUp() {
-			log.Debug("SCMP response", "cause", errBFDSessionDown)
-			if _, external := p.d.external[p.pkt.egress]; !external {
-				p.pkt.slowPathRequest = slowPathRequest{
-					scmpType: slayers.SCMPTypeInternalConnectivityDown,
-					code:     0,
-				}
-			} else {
-				p.pkt.slowPathRequest = slowPathRequest{
-					scmpType: slayers.SCMPTypeExternalInterfaceDown,
-					code:     0,
-				}
+	egressLink := p.d.interfaces[egressID]
+	if !egressLink.IsUp() {
+		log.Debug("SCMP response", "cause", errBFDSessionDown)
+		if egressLink.GetScope() != External {
+			p.pkt.slowPathRequest = slowPathRequest{
+				scmpType: slayers.SCMPTypeInternalConnectivityDown,
+				code:     0,
+			}
+		} else {
+			p.pkt.slowPathRequest = slowPathRequest{
+				scmpType: slayers.SCMPTypeExternalInterfaceDown,
+				code:     0,
 			}
-			return pSlowPath
 		}
+		return pSlowPath
 	}
 	return pForward
 }
@@ -2015,7 +1815,7 @@ func (p *scionPacketProcessor) handleEgressRouterAlert() disposition {
 	if !*alert {
 		return pForward
 	}
-	if _, ok := p.d.external[p.pkt.egress]; !ok {
+	if p.d.interfaces[p.pkt.egress].GetScope() != External {
 		// the egress router is not this one.
 		return pForward
 	}
@@ -2186,7 +1986,7 @@ func (p *scionPacketProcessor) process() disposition {
 	if disp := p.validateEgressUp(); disp != pForward {
 		return disp
 	}
-	if _, ok := p.d.external[egressID]; ok {
+	if p.d.interfaces[egressID].GetScope() == External {
 		// Not ASTransit in
 		if disp := p.processEgress(); disp != pForward {
 			return disp
@@ -2208,25 +2008,9 @@ func (p *scionPacketProcessor) process() disposition {
 	}
 
 	// ASTransit in: pkt leaving this AS through another BR.
-	if a, ok := p.d.internalNextHops[egressID]; ok {
-		p.pkt.trafficType = ttInTransit
-		updateNetAddrFromAddrPort(p.pkt.dstAddr, a)
-		// The packet must go to the other router via the internal interface.
-		p.pkt.egress = 0
-		return pForward
-	}
-
-	errCode := slayers.SCMPCodeUnknownHopFieldEgress
-	if !p.infoField.ConsDir {
-		errCode = slayers.SCMPCodeUnknownHopFieldIngress
-	}
-	log.Debug("SCMP response", "cause", cannotRoute)
-	p.pkt.slowPathRequest = slowPathRequest{
-		scmpType: slayers.SCMPTypeParameterProblem,
-		code:     errCode,
-		pointer:  p.currentHopPointer(),
-	}
-	return pSlowPath
+	// We already know the egressID is valid. The packet can go straight to forwarding.
+	p.pkt.trafficType = ttInTransit
+	return pForward
 }
 
 func (p *scionPacketProcessor) processOHP() disposition {
@@ -2291,7 +2075,7 @@ func (p *scionPacketProcessor) processOHP() disposition {
 	if err := updateSCIONLayer(p.pkt.rawPacket, s); err != nil {
 		return errorDiscard("error", err)
 	}
-	err := p.d.resolveLocalDst(p.pkt.dstAddr, s, p.lastLayer)
+	err := p.d.resolveLocalDst(p.pkt.DstAddr, s, p.lastLayer)
 	if err != nil {
 		return errorDiscard("error", err)
 	}
@@ -2322,7 +2106,7 @@ func (d *DataPlane) resolveLocalDst(
 		if a.Port() < d.dispatchedPortStart || a.Port() > d.dispatchedPortEnd {
 			updateNetAddrFromAddrAndPort(resolvedDst, a.Addr(), topology.EndhostPort)
 		} else {
-			updateNetAddrFromAddrPort(resolvedDst, a)
+			UpdateNetAddrFromAddrPort(resolvedDst, a)
 		}
 		return nil
 	case addr.HostTypeIP:
@@ -2617,10 +2401,10 @@ func (b *bfdSend) Send(bfd *layers.BFD) error {
 
 	if b.ifID == 0 {
 		// Using the internal interface: must specify the destination address
-		updateNetAddrFromAddrPort(p.dstAddr, b.dstAddr)
+		UpdateNetAddrFromAddrPort(p.DstAddr, b.dstAddr)
 	}
 	// No need to specify pkt.egress. It isn't used downstream from here.
-	if !fwLink.send(p) {
+	if !fwLink.Send(p) {
 		// We do not care if some BFD packets get bounced under high load. If it becomes a problem,
 		// the solution is do use BFD's demand-mode. To be considered in a future refactoring.
 		b.dataPlane.returnPacketToPool(p)
@@ -2687,8 +2471,8 @@ func (p *slowPathPacketProcessor) prepareSCMP(
 	}
 	// If the packet is sent to an external router, we need to increment the
 	// path to prepare it for the next hop.
-	_, external := p.d.external[p.pkt.ingress]
-	if external {
+	// This is an scmp response to pkt, so egress will be pkt.ingress.
+	if p.d.interfaces[p.pkt.ingress].GetScope() == External {
 		infoField := &revPath.InfoFields[revPath.PathMeta.CurrINF]
 		if infoField.ConsDir && !peering {
 			hopField := revPath.HopFields[revPath.PathMeta.CurrHF]
@@ -2940,9 +2724,9 @@ func nextHdr(layer gopacket.DecodingLayer) slayers.L4ProtocolType {
 // forwarding.
 func (d *DataPlane) initMetrics() {
 	d.forwardingMetrics = make(map[uint16]interfaceMetrics)
-	d.forwardingMetrics[0] = newInterfaceMetrics(d.Metrics, 0, d.localIA, d.neighborIAs)
-	for ifID := range d.external {
-		d.forwardingMetrics[ifID] = newInterfaceMetrics(d.Metrics, ifID, d.localIA, d.neighborIAs)
+	for ifID, link := range d.interfaces {
+		d.forwardingMetrics[ifID] = newInterfaceMetrics(
+			d.Metrics, ifID, d.localIA, link.GetScope(), d.neighborIAs)
 	}
 
 	// Start our custom /proc/pid/stat collector to export iowait time and (in the future) other
@@ -2959,7 +2743,7 @@ func (d *DataPlane) initMetrics() {
 // given netip.AddrPort. newDst.Addr() returns the IP by value. The compiler may or
 // may not inline the call and optimize out the copy. It is doubtful that manually inlining
 // increases the chances that the copy get elided. TODO(jiceatscion): experiment.
-func updateNetAddrFromAddrPort(netAddr *net.UDPAddr, newDst netip.AddrPort) {
+func UpdateNetAddrFromAddrPort(netAddr *net.UDPAddr, newDst netip.AddrPort) {
 	updateNetAddrFromAddrAndPort(netAddr, newDst.Addr(), newDst.Port())
 }
 
diff --git a/router/dataplane_internal_test.go b/router/dataplane_internal_test.go
index 1132f7518e..41900c95cd 100644
--- a/router/dataplane_internal_test.go
+++ b/router/dataplane_internal_test.go
@@ -52,7 +52,15 @@ var (
 func TestReceiver(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
-	dp := &DataPlane{Metrics: metrics}
+	dp := &DataPlane{
+		underlay:   newUnderlay(),
+		interfaces: make(map[uint16]Link),
+		Metrics:    metrics,
+		RunConfig: RunConfig{
+			NumProcessors: 1,
+			BatchSize:     64,
+		},
+	}
 	counter := 0
 	mInternal := mock_router.NewMockBatchConn(ctrl)
 	done := make(chan bool)
@@ -79,17 +87,13 @@ func TestReceiver(t *testing.T) {
 
 	_ = dp.AddInternalInterface(mInternal, netip.Addr{})
 
-	runConfig := &RunConfig{
-		NumProcessors: 1,
-		BatchSize:     64,
-	}
-	dp.initPacketPool(runConfig, 64)
-	procCh, _, _ := initQueues(runConfig, dp.interfaces, 64)
+	dp.initPacketPool(64)
+	procCh, _ := dp.initQueues(64)
 	initialPoolSize := len(dp.packetPool)
 	dp.setRunning()
 	dp.initMetrics()
 	go func() {
-		dp.runReceiver(0, dp.interfaces[0], runConfig, procCh)
+		dp.runReceiver(dp.underlay.GetConnections()[netip.AddrPort{}], procCh)
 	}()
 	ptrMap := make(map[uintptr]struct{})
 	for i := 0; i < 21; i++ {
@@ -115,7 +119,7 @@ func TestReceiver(t *testing.T) {
 	}
 	<-done
 	// make sure that the packet pool has the expected size after the test
-	assert.Equal(t, initialPoolSize-runConfig.BatchSize-20, len(dp.packetPool))
+	assert.Equal(t, initialPoolSize-dp.RunConfig.BatchSize-20, len(dp.packetPool))
 }
 
 // TestForwarder sets up a mocked batchConn, starts the forwarder that will write to
@@ -127,7 +131,15 @@ func TestForwarder(t *testing.T) {
 	defer ctrl.Finish()
 	done := make(chan struct{})
 	prepareDP := func(ctrl *gomock.Controller) *DataPlane {
-		ret := &DataPlane{Metrics: metrics}
+		ret := &DataPlane{
+			underlay: newUnderlay(),
+			Metrics:  metrics,
+			RunConfig: RunConfig{
+				NumProcessors:         20,
+				BatchSize:             64,
+				NumSlowPathProcessors: 1,
+			},
+		}
 
 		mInternal := mock_router.NewMockBatchConn(ctrl)
 		totalCount := 0
@@ -173,16 +185,14 @@ func TestForwarder(t *testing.T) {
 		return ret
 	}
 	dp := prepareDP(ctrl)
-	runConfig := &RunConfig{
-		NumProcessors: 20,
-		BatchSize:     64,
-	}
-	dp.initPacketPool(runConfig, 64)
-	_, fwCh, _ := initQueues(runConfig, dp.interfaces, 64)
+	dp.initPacketPool(64)
+	dp.initQueues(64)
+	conn := dp.underlay.GetConnections()[netip.AddrPort{}]
+	intf := dp.interfaces[0]
 	initialPoolSize := len(dp.packetPool)
 	dp.setRunning()
 	dp.initMetrics()
-	go dp.runForwarder(0, dp.interfaces[0], runConfig, fwCh[0])
+	go dp.runForwarder(conn)
 
 	dstAddr := &net.UDPAddr{IP: net.IP{10, 0, 200, 200}}
 	for i := 0; i < 255; i++ {
@@ -191,19 +201,18 @@ func TestForwarder(t *testing.T) {
 		pkt.rawPacket = pkt.rawPacket[:1]
 		pkt.rawPacket[0] = byte(i)
 		if i < 100 {
-			pkt.dstAddr.IP = pkt.dstAddr.IP[:4]
-			copy(pkt.dstAddr.IP, dstAddr.IP)
+			pkt.DstAddr.IP = pkt.DstAddr.IP[:4]
+			copy(pkt.DstAddr.IP, dstAddr.IP)
 		}
 		pkt.srcAddr = &net.UDPAddr{} // Receiver always sets this.
 		pkt.ingress = 0
 
 		assert.NotEqual(t, initialPoolSize, len(dp.packetPool))
 
-		select {
-		case fwCh[0] <- pkt:
-		case <-done:
-		}
-
+		// Normal use would be
+		// intf.Send(pkt):
+		// However we want to exclude queue overflow from the test. So we want a blocking send.
+		intf.BlockSend(pkt)
 	}
 	select {
 	case <-done:
@@ -619,7 +628,7 @@ func TestSlowPathProcessing(t *testing.T) {
 			dp.initMetrics()
 
 			rp := tc.mockMsg()
-			pkt := packet{}
+			pkt := Packet{}
 			pkt.init(&[bufSize]byte{})
 			pkt.reset()
 			pkt.ingress = tc.srcInterface
diff --git a/router/dataplane_test.go b/router/dataplane_test.go
index 9298aa23c6..8218380112 100644
--- a/router/dataplane_test.go
+++ b/router/dataplane_test.go
@@ -47,6 +47,7 @@ import (
 	"github.com/scionproto/scion/router"
 	"github.com/scionproto/scion/router/control"
 	"github.com/scionproto/scion/router/mock_router"
+	_ "github.com/scionproto/scion/router/underlayproviders"
 )
 
 var (
@@ -200,29 +201,45 @@ func TestDataPlaneAddNextHop(t *testing.T) {
 	l := netip.AddrPortFrom(netip.IPv4Unspecified(), 0)
 	r := netip.AddrPortFrom(netip.IPv4Unspecified(), 0)
 	nilAddrPort := netip.AddrPort{}
+	nilAddr := netip.Addr{}
 
 	nobfd := control.BFD{Disable: ptr.To(true)}
 
 	t.Run("fails after serve", func(t *testing.T) {
 		d := &router.DataPlane{}
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+		d.AddInternalInterface(mock_router.NewMockBatchConn(ctrl), nilAddr)
 		d.FakeStart()
 		assert.Error(t, d.AddNextHop(45, l, r, nobfd, ""))
 	})
 	t.Run("setting nil dst is not allowed", func(t *testing.T) {
 		d := &router.DataPlane{}
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+		d.AddInternalInterface(mock_router.NewMockBatchConn(ctrl), nilAddr)
 		assert.Error(t, d.AddNextHop(45, l, nilAddrPort, nobfd, ""))
 	})
 	t.Run("setting nil src is not allowed", func(t *testing.T) {
 		d := &router.DataPlane{}
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+		d.AddInternalInterface(mock_router.NewMockBatchConn(ctrl), nilAddr)
 		assert.Error(t, d.AddNextHop(45, nilAddrPort, r, nobfd, ""))
 	})
 	t.Run("normal add works", func(t *testing.T) {
 		d := &router.DataPlane{}
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+		d.AddInternalInterface(mock_router.NewMockBatchConn(ctrl), nilAddr)
 		assert.NoError(t, d.AddNextHop(45, l, r, nobfd, ""))
 		assert.NoError(t, d.AddNextHop(43, l, r, nobfd, ""))
 	})
 	t.Run("overwrite fails", func(t *testing.T) {
 		d := &router.DataPlane{}
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+		d.AddInternalInterface(mock_router.NewMockBatchConn(ctrl), nilAddr)
 		assert.NoError(t, d.AddNextHop(45, l, r, nobfd, ""))
 		assert.Error(t, d.AddNextHop(45, l, r, nobfd, ""))
 	})
@@ -238,7 +255,14 @@ func TestDataPlaneRun(t *testing.T) {
 	}{
 		"route 10 msg from external to internal": {
 			prepareDP: func(ctrl *gomock.Controller, done chan<- struct{}) *router.DataPlane {
-				ret := &router.DataPlane{Metrics: metrics}
+				ret := &router.DataPlane{
+					Metrics: metrics,
+					RunConfig: router.RunConfig{
+						NumProcessors:         8,
+						BatchSize:             256,
+						NumSlowPathProcessors: 1,
+					},
+				}
 				key := []byte("testkey_xxxxxxxx")
 				local := addr.MustParseIA("1-ff00:0:110")
 
@@ -326,7 +350,14 @@ func TestDataPlaneRun(t *testing.T) {
 		},
 		"bfd bootstrap internal session": {
 			prepareDP: func(ctrl *gomock.Controller, done chan<- struct{}) *router.DataPlane {
-				ret := &router.DataPlane{Metrics: metrics}
+				ret := &router.DataPlane{
+					Metrics: metrics,
+					RunConfig: router.RunConfig{
+						NumProcessors:         8,
+						BatchSize:             256,
+						NumSlowPathProcessors: 1,
+					},
+				}
 
 				postInternalBFD := func(id layers.BFDDiscriminator, src netip.AddrPort) []byte {
 					scn := &slayers.SCION{
@@ -405,7 +436,14 @@ func TestDataPlaneRun(t *testing.T) {
 		},
 		"bfd sender internal": {
 			prepareDP: func(ctrl *gomock.Controller, done chan<- struct{}) *router.DataPlane {
-				ret := &router.DataPlane{Metrics: metrics}
+				ret := &router.DataPlane{
+					Metrics: metrics,
+					RunConfig: router.RunConfig{
+						NumProcessors:         8,
+						BatchSize:             256,
+						NumSlowPathProcessors: 1,
+					},
+				}
 				localAddr := netip.MustParseAddrPort("10.0.200.100:0")
 				remoteAddr := netip.MustParseAddrPort("10.0.200.200:0")
 				mInternal := mock_router.NewMockBatchConn(ctrl)
@@ -456,7 +494,15 @@ func TestDataPlaneRun(t *testing.T) {
 		},
 		"bfd sender external": {
 			prepareDP: func(ctrl *gomock.Controller, done chan<- struct{}) *router.DataPlane {
-				ret := &router.DataPlane{Metrics: metrics}
+				ret := &router.DataPlane{
+					Metrics: metrics,
+					RunConfig: router.RunConfig{
+						NumProcessors:         8,
+						BatchSize:             256,
+						NumSlowPathProcessors: 1,
+					},
+				}
+
 				ifID := uint16(1)
 				mInternal := mock_router.NewMockBatchConn(ctrl)
 				mInternal.EXPECT().ReadBatch(gomock.Any()).Return(0, nil).AnyTimes()
@@ -508,7 +554,14 @@ func TestDataPlaneRun(t *testing.T) {
 		},
 		"bfd bootstrap external session": {
 			prepareDP: func(ctrl *gomock.Controller, done chan<- struct{}) *router.DataPlane {
-				ret := &router.DataPlane{Metrics: metrics}
+				ret := &router.DataPlane{
+					Metrics: metrics,
+					RunConfig: router.RunConfig{
+						NumProcessors:         8,
+						BatchSize:             256,
+						NumSlowPathProcessors: 1,
+					},
+				}
 
 				postExternalBFD := func(id layers.BFDDiscriminator, fromIfID uint16) []byte {
 					scn := &slayers.SCION{
@@ -589,18 +642,13 @@ func TestDataPlaneRun(t *testing.T) {
 		name, tc := name, tc
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
-			runConfig := &router.RunConfig{
-				NumProcessors:         8,
-				BatchSize:             256,
-				NumSlowPathProcessors: 1,
-			}
 			ch := make(chan struct{})
 			dp := tc.prepareDP(ctrl, ch)
 			errors := make(chan error)
 			ctx, cancelF := context.WithCancel(context.Background())
 			defer cancelF()
 			go func() {
-				errors <- dp.Run(ctx, runConfig)
+				errors <- dp.Run(ctx)
 			}()
 
 			for done := false; !done; {
@@ -1157,9 +1205,11 @@ func TestProcessPkt(t *testing.T) {
 				dpath.HopFields[1].Mac = computeMAC(t, key, dpath.InfoFields[0], dpath.HopFields[1])
 				var dstAddr *net.UDPAddr
 				ingress := uint16(1)
-				egress := uint16(0) // Internal forward to the egress router
+				egress := uint16(0) // To make sure it gets updated.
 				if afterProcessing {
-					dstAddr = &net.UDPAddr{IP: net.ParseIP("10.0.200.200").To4(), Port: 30043}
+					egress = uint16(3) // The sibling router is locally mapped to the egress ifID.
+					// The link is specific to the sibling. It has the address. So we don't expect:
+					// dstAddr = &net.UDPAddr{IP: net.ParseIP("10.0.200.200").To4(), Port: 30043}
 				}
 				return router.NewPacket(toBytes(t, spkt, dpath), nil, dstAddr, ingress, egress)
 			},
@@ -1210,11 +1260,13 @@ func TestProcessPkt(t *testing.T) {
 
 				var dstAddr *net.UDPAddr
 				ingress := uint16(51) // == consEgress, bc non-consdir
-				egress := uint16(0)   // Cross-over. The egress happens in the next segment.
+				egress := uint16(0)   // To check that it is updated
 				if afterProcessing {
 					dpath.PathMeta.CurrHF++
 					dpath.PathMeta.CurrINF++
-					dstAddr = &net.UDPAddr{IP: net.ParseIP("10.0.200.200").To4(), Port: 30043}
+					egress = uint16(3) // Internal hop => egress points at sibling router.
+					// The link is specific to the sibling. It has the address. So we don't expect:
+					// dstAddr = &net.UDPAddr{IP: net.ParseIP("10.0.200.200").To4(), Port: 30043}
 				} else {
 					dpath.InfoFields[0].UpdateSegID(dpath.HopFields[1].Mac)
 				}
diff --git a/router/export_test.go b/router/export_test.go
index 9a8744f9e0..4c3607eccb 100644
--- a/router/export_test.go
+++ b/router/export_test.go
@@ -36,30 +36,24 @@ func GetMetrics() *Metrics {
 
 var NewServices = newServices
 
-// Export the Packet struct so dataplane test can call ProcessPkt
-type Packet struct {
-	packet
-}
-
 type Disposition disposition
 
 const PDiscard = Disposition(pDiscard)
 
 func NewPacket(raw []byte, src, dst *net.UDPAddr, ingress, egress uint16) *Packet {
 	p := Packet{
-		packet: packet{
-			dstAddr:   &net.UDPAddr{IP: make(net.IP, 0, net.IPv6len)},
-			srcAddr:   &net.UDPAddr{IP: make(net.IP, 0, net.IPv6len)},
-			rawPacket: make([]byte, len(raw)),
-			ingress:   ingress,
-			egress:    egress,
-		},
+		DstAddr:   &net.UDPAddr{IP: make(net.IP, 0, net.IPv6len)},
+		srcAddr:   &net.UDPAddr{IP: make(net.IP, 0, net.IPv6len)},
+		rawPacket: make([]byte, len(raw)),
+		ingress:   ingress,
+		egress:    egress,
 	}
+
 	if src != nil {
 		p.srcAddr = src
 	}
 	if dst != nil {
-		p.dstAddr = dst
+		p.DstAddr = dst
 	}
 	copy(p.rawPacket, raw)
 	return &p
@@ -76,12 +70,11 @@ func NewDP(
 	key []byte) *DataPlane {
 
 	dp := &DataPlane{
-		interfaces:          map[uint16]BatchConn{0: internal},
+		underlay:            newUnderlay(),
+		interfaces:          make(map[uint16]Link),
 		localIA:             local,
-		external:            external,
 		linkTypes:           linkTypes,
 		neighborIAs:         neighbors,
-		internalNextHops:    internalNextHops,
 		dispatchedPortStart: uint16(dispatchedPortStart),
 		dispatchedPortEnd:   uint16(dispatchedPortEnd),
 		svc:                 &services{m: svc},
@@ -89,6 +82,19 @@ func NewDP(
 		Metrics:             metrics,
 	}
 
+	dp.interfaces[0] = dp.underlay.NewInternalLink(internal, 64)
+
+	// Make dummy external interfaces, as requested by the test. They are not actually used to send
+	// or receive. The blank address might cause issues, though.
+	for i, _ := range external {
+		dp.interfaces[i] = dp.underlay.NewExternalLink(nil, 64, nil, netip.AddrPort{}, i)
+	}
+
+	// Make dummy sibling interfaces, as requestes by the test.
+	for i, addr := range internalNextHops {
+		dp.interfaces[i] = dp.underlay.NewSiblingLink(64, nil, addr)
+	}
+
 	if err := dp.SetKey(key); err != nil {
 		panic(err)
 	}
@@ -103,7 +109,7 @@ func (d *DataPlane) FakeStart() {
 func (d *DataPlane) ProcessPkt(pkt *Packet) Disposition {
 
 	p := newPacketProcessor(d)
-	disp := p.processPkt(&(pkt.packet))
+	disp := p.processPkt(pkt)
 	// Erase trafficType; we don't set it in the expected results.
 	pkt.trafficType = ttOther
 	return Disposition(disp)
diff --git a/router/metrics.go b/router/metrics.go
index 5253bba765..7bbf1c8a85 100644
--- a/router/metrics.go
+++ b/router/metrics.go
@@ -282,9 +282,10 @@ func newInterfaceMetrics(
 	metrics *Metrics,
 	id uint16,
 	localIA addr.IA,
+	scope LinkScope,
 	neighbors map[uint16]addr.IA) interfaceMetrics {
 
-	ifLabels := interfaceLabels(id, localIA, neighbors)
+	ifLabels := interfaceLabels(id, localIA, scope, neighbors)
 	m := interfaceMetrics{}
 	for sc := minSizeClass; sc < maxSizeClass; sc++ {
 		scLabels := prometheus.Labels{"sizeclass": sc.String()}
@@ -355,7 +356,7 @@ func newOutputMetrics(
 	return om
 }
 
-func interfaceLabels(id uint16, localIA addr.IA, neighbors map[uint16]addr.IA) prometheus.Labels {
+func interfaceLabels(id uint16, localIA addr.IA, scope LinkScope, neighbors map[uint16]addr.IA) prometheus.Labels {
 	if id == 0 {
 		return prometheus.Labels{
 			"isd_as":          localIA.String(),
@@ -363,10 +364,16 @@ func interfaceLabels(id uint16, localIA addr.IA, neighbors map[uint16]addr.IA) p
 			"neighbor_isd_as": localIA.String(),
 		}
 	}
+	viaSibling := "->"
+	neighbor := "unknown"
+	if scope == External {
+		viaSibling = ""
+		neighbor = neighbors[id].String()
+	}
 	return prometheus.Labels{
 		"isd_as":          localIA.String(),
-		"interface":       strconv.FormatUint(uint64(id), 10),
-		"neighbor_isd_as": neighbors[id].String(),
+		"interface":       viaSibling + strconv.FormatUint(uint64(id), 10),
+		"neighbor_isd_as": neighbor,
 	}
 }
 
diff --git a/router/underlay.go b/router/underlay.go
new file mode 100644
index 0000000000..1c2aae49cd
--- /dev/null
+++ b/router/underlay.go
@@ -0,0 +1,114 @@
+// Copyright 2025 SCION Association
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// This module defines the interfaces between the router and the underlay network implementations.
+
+package router
+
+import (
+	"net/netip"
+)
+
+// LinkScope describes the kind (or scope) of a link: internal, sibling, or external.
+type LinkScope int
+
+const (
+	Internal LinkScope = iota // to/from end-hosts in the local AS
+	Sibling                   // to/from (external interfaces owned by) a sibling router
+	External                  // to/from routers in another AS
+)
+
+// Link embodies the router's idea of a point to point connection. A link associates the underlay
+// connection, with a bfdSession, a destination address, etc. It also allows the concrete send
+// operation to be delegated to different underlay implementations. The association between
+// link and underlay connection is a channel, on the sending side, and should be a demultiplexer on
+// the receiving side. The demultiplexer must have a src-addr:link map in all cases where links share
+// connections.
+//
+// Regardless of underlay, links come in three scopes: internal, sibling, and external. The
+// difference in behaviour is hidden from the rest of the router. The router only needs to
+// associate an interface ID with a link. If the interface ID belongs to a sibling router, then
+// the link is a sibling link. If the interface ID is zero, then the link is the internal link.
+type Link interface {
+	GetScope() LinkScope
+	GetBfdSession() BfdSession
+	IsUp() bool
+	GetIfID() uint16
+	GetRemote() netip.AddrPort // incremental refactoring: using code will move to underlay.
+	Send(p *Packet) bool
+	BlockSend(p *Packet)
+}
+
+// A provider of connectivity over some underlay implementation
+//
+// For any given underlay, there are three kinds of Link implementations to choose from.
+// The difference between them is the intent regarding addressing.
+//
+// Incremental refactoring: addresses are still explicitly IP/port. In the next step, we have to
+// make them opaque; to be interpreted only by the underlay implementation.
+type UnderlayProvider interface {
+
+	// NewExternalLink returns a link that addresses a single remote AS at a unique underlay
+	// address. So, it is given an ifID and a underlay remote address at creation. Outgoing packets
+	// do not need an underlay destination as metadata. Incoming packets have a defined ingress
+	// ifID.
+	NewExternalLink(
+		conn BatchConn, qSize int, bfd BfdSession, remote netip.AddrPort, ifID uint16,
+	) Link
+
+	// NewSinblingLink returns a link that addresses any number of remote ASes via a single sibling
+	// router. So, it is not given an ifID at creation, but it is given a remote underlay address:
+	// that of the sibling router. Outgoing packets do not need an underlay destination as metadata.
+	// Incoming packets have no defined ingress ifID.
+	NewSiblingLink(qSize int, bfd BfdSession, remote netip.AddrPort) Link
+
+	// NewIternalLink returns a link that addresses any host internal to the enclosing AS, so it is
+	// given neither ifID nor address. Outgoing packets need to have a destination address as
+	// metadata. Incoming packets have no defined ingress ifID.
+	NewInternalLink(conn BatchConn, qSize int) Link
+
+	// GetConnections returns the set of configured distinct connections in the provider.
+	//
+	// Incremental refactoring: this exists so most of the receiving code can stay in the main
+	// dataplane code for now. There may be fewer connections than links. For example, right now
+	// all sibling links and the internal link use a shared un-bound connection.
+	GetConnections() map[netip.AddrPort]UnderlayConnection
+
+	// GetLinks returns the set of configured distinct links in the provider.
+	//
+	// Incremental refactoring: this exists so most of the receiving code can stay in-here for now.
+	// There may be fewer links than ifIDs. For example, all interfaces owned by one given sibling
+	// router are connected via the same link because the remote address is the same.
+	GetLinks() map[netip.AddrPort]Link
+
+	// GetLink returns a link that matches the given source address. If the address is not that of
+	// a known link, then the internal link is returned.
+	//
+	// Increamental refactoring: This has to exist until incmoing packets are "demuxed" (i.e.
+	// matched with a link), on ingest by the underlay. That would imply moving a part of the
+	// runReceiver routine to the underlay. We will do that in the next step.
+	GetLink(netip.AddrPort) Link
+}
+
+// UnderlayConnection defines the minimum interface that the router expects from an underlay
+// connection.
+//
+// Incremental refactoring: this will eventually be reduced to nothing at all because the sender
+// receiver tasks will be part of the underlay.
+type UnderlayConnection interface {
+	Conn() BatchConn
+	Queue() <-chan *Packet
+	Name() string
+	IfID() uint16
+}
diff --git a/router/underlayproviders/BUILD.bazel b/router/underlayproviders/BUILD.bazel
new file mode 100644
index 0000000000..1f5ab23865
--- /dev/null
+++ b/router/underlayproviders/BUILD.bazel
@@ -0,0 +1,9 @@
+load("//tools/lint:go.bzl", "go_library")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["udpip.go"],
+    importpath = "github.com/scionproto/scion/router/underlayproviders",
+    visibility = ["//visibility:public"],
+    deps = ["//router:go_default_library"],
+)
diff --git a/router/underlayproviders/udpip.go b/router/underlayproviders/udpip.go
new file mode 100644
index 0000000000..6fbeb3a173
--- /dev/null
+++ b/router/underlayproviders/udpip.go
@@ -0,0 +1,330 @@
+// Copyright 2025 SCION Association
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package underlayproviders
+
+import (
+	"net/netip"
+
+	// Until we switch to Go 1.23 and can use slices.Collect(maps.Values())
+
+	"github.com/scionproto/scion/router"
+)
+
+// provider implements UnderlayProvider by making and returning Udp/Ip links.
+//
+// This is currently the only implementation. The goal of splitting out this code from the router
+// is to enable other implementations. However, as a first step, we continue assuming that the
+// batchConn is given to us and is a Udp socket and that, in the case of externalLink, it is bound.
+type provider struct {
+	allLinks       map[netip.AddrPort]router.Link
+	allConnections map[netip.AddrPort]*udpConnection
+}
+
+func init() {
+	// Register ourselves as an underlay provider. The registration consists of a constructor, not
+	// a provider object, because multiple router instances each must have their own underlay
+	// provider. The provider is not re-entrant.
+	router.AddUnderlay(newProvider)
+}
+
+// New instantiates a new instance of the provider for exclusive use by the caller.
+func newProvider() router.UnderlayProvider {
+	return &provider{
+		allLinks:       make(map[netip.AddrPort]router.Link),
+		allConnections: make(map[netip.AddrPort]*udpConnection),
+	}
+}
+
+func (u *provider) GetConnections() map[netip.AddrPort]router.UnderlayConnection {
+	// a map of interfaces and a map of concrete implementations aren't compatible.
+	// For the same reason, we cannot have the map of concrete as our return type; it
+	// does not satisfy the GetConnections interface (so much for the "don't return
+	// interfaces" rule)... Brilliant, Go.
+	// Since we do not want to store our own things as interfaces, we have to translate.
+	// Good thing it doesn't happen often.
+	m := make(map[netip.AddrPort]router.UnderlayConnection)
+	for a, c := range u.allConnections {
+		m[a] = c // Yeah that's exactly as stupid as it looks.
+	}
+	return m
+}
+
+func (u *provider) GetLinks() map[netip.AddrPort]router.Link {
+	return u.allLinks
+}
+
+func (u *provider) GetLink(addr netip.AddrPort) router.Link {
+	// There is one link for every address. The internal Link catches all.
+	l, found := u.allLinks[addr]
+	if found {
+		return l
+	}
+	return u.allLinks[netip.AddrPort{}]
+}
+
+// udpConnection is simply the combination of a BatchConn and sending queue (plus metadata for
+// logs and such). This allows udp connections to be shared between links. Bundling link and
+// connection together is possible and simpler for the code here, but leaks more refactoring changes
+// in the main router code. Specifically, either:
+//   - sibling links would each need an independent socket to the sibling router, which
+//     the router cannot provide at the moment.
+//   - the internal links and sibling links would be the same, which means the router needs to
+//     special case the sibling links: which we want to remove from the main code.
+type udpConnection struct {
+	conn  router.BatchConn
+	queue chan *router.Packet
+	ifID  uint16 // for metrics. All sibling links plus the internal link will be zero, though.
+	name  string // for logs. It's more informative than ifID.
+}
+
+// Incremental refactoring: The following implements UnderlayConnection so some of the code
+// that needs to interact with it can stay in the main router code. This will be removed in the
+// next step
+
+// Name returns the name (for logging) associated with a connection.
+func (u *udpConnection) Conn() router.BatchConn {
+	return u.conn
+}
+
+// Name returns the name (for logging) associated with a connection.
+func (u *udpConnection) Queue() <-chan *router.Packet {
+	return u.queue
+}
+
+// Name returns the name (for logging) associated with a connection.
+func (u *udpConnection) Name() string {
+	return u.name
+}
+
+// Name returns the name (for logging) associated with a connection.
+func (u *udpConnection) IfID() uint16 {
+	return u.ifID
+}
+
+// todo(jiceatscion): use inheritence between implementations?
+
+type externalLink struct {
+	queue      chan<- *router.Packet
+	bfdSession router.BfdSession
+	ifID       uint16
+	remote     netip.AddrPort // We keep this only for GetRemote
+}
+
+// NewExternalLink returns an external link over the UdpIpUnderlay.
+//
+// Incremental refactoring: we get the connection ready-made and require it to be bound. So, we
+// don't keep the remote address, but in the future, we will be making the connections, and
+// BatchConn will be gone.
+func (u *provider) NewExternalLink(
+	conn router.BatchConn,
+	qSize int,
+	bfd router.BfdSession,
+	remote netip.AddrPort,
+	ifID uint16,
+) router.Link {
+
+	queue := make(chan *router.Packet, qSize)
+	c := &udpConnection{
+		conn:  conn,
+		queue: queue,
+		ifID:  ifID,
+		name:  remote.String(),
+	}
+	u.allConnections[remote] = c
+	l := &externalLink{
+		queue:      queue,
+		bfdSession: bfd,
+		ifID:       ifID,
+	}
+	u.allLinks[remote] = l
+	return l
+}
+
+func (l *externalLink) GetScope() router.LinkScope {
+	return router.External
+}
+
+func (l *externalLink) GetBfdSession() router.BfdSession {
+	return l.bfdSession
+}
+
+func (l *externalLink) IsUp() bool {
+	return l.bfdSession == nil || l.bfdSession.IsUp()
+}
+
+func (l *externalLink) GetIfID() uint16 {
+	return l.ifID
+}
+
+func (l *externalLink) GetRemote() netip.AddrPort {
+	return l.remote
+}
+
+func (l *externalLink) Send(p *router.Packet) bool {
+	select {
+	case l.queue <- p:
+	default:
+		return false
+	}
+	return true
+}
+
+func (l *externalLink) BlockSend(p *router.Packet) {
+	l.queue <- p
+}
+
+type siblingLink struct {
+	queue      chan<- *router.Packet
+	bfdSession router.BfdSession
+	remote     netip.AddrPort
+}
+
+// newSiblingLink returns a sibling link over the UdpIpUnderlay.
+//
+// Incremental refactoring: this can only be an improvement over internalLink if we have a bound
+// batchConn with the sibling router. However, currently the caller doesn't have one to give us;
+// the main code has so far been reusing the internal connection. So, that's what we do for now.
+// As a result, we keep the remote address; as we need to supply it for every packet being sent
+// (something we will get rid of eventually).
+// In the future we will be making one connection per remote address and we might even be able
+// to erase the separation between link and connection for this implementation. Side effect
+// of moving the address:link here: the router does not know if there is an existing link. As
+// a result it has to give us a bfdSession in all cases and if we might throuw it away (there
+// are no permanent resources attached to it). This will be fixed by moving some bfd related code
+// in-here; but later.
+func (u *provider) NewSiblingLink(
+	qSize int, bfd router.BfdSession, remote netip.AddrPort) router.Link {
+
+	// There is exactly one sibling link per sibling router address.
+	l, exists := u.allLinks[remote]
+	if exists {
+		return l.(*siblingLink)
+	}
+
+	// All sibling links re-use the internal connection. This used to be a late binding (packets to
+	// siblings would get routed through the internal interface at run-time). But now this binding
+	// happens right now and it can't work if this is called before newInternalLink.
+	c, exists := u.allConnections[netip.AddrPort{}]
+	if !exists {
+		// That doesn't actually happen and we'll get rid of this soon.
+		panic("newSiblingLink called before newInternalLink")
+	}
+
+	s := &siblingLink{
+		queue:      c.queue,
+		bfdSession: bfd,
+		remote:     remote,
+	}
+	u.allLinks[remote] = s
+	return s
+}
+
+func (l *siblingLink) GetScope() router.LinkScope {
+	return router.Sibling
+}
+
+func (l *siblingLink) GetBfdSession() router.BfdSession {
+	return l.bfdSession
+}
+
+func (l *siblingLink) IsUp() bool {
+	return l.bfdSession == nil || l.bfdSession.IsUp()
+}
+
+func (l *siblingLink) GetIfID() uint16 {
+	return 0
+}
+
+func (l *siblingLink) GetRemote() netip.AddrPort {
+	return l.remote
+}
+
+func (l *siblingLink) Send(p *router.Packet) bool {
+	// We use an unbound connection but we offer a connected-oriented service. So, we need to
+	// supply the packet's destination address.
+	router.UpdateNetAddrFromAddrPort(p.DstAddr, l.remote)
+	select {
+	case l.queue <- p:
+	default:
+		return false
+	}
+	return true
+}
+
+func (l *siblingLink) BlockSend(p *router.Packet) {
+	// We use an unbound connection but we offer a connected-oriented service. So, we need to
+	// supply the packet's destination address.
+	router.UpdateNetAddrFromAddrPort(p.DstAddr, l.remote)
+	l.queue <- p
+}
+
+type internalLink struct {
+	queue chan *router.Packet
+}
+
+// newSiblingLink returns a sibling link over the UdpIpUnderlay.
+//
+// Incremental refactoring: we get the connection ready made. In the future we will be making it
+// and the conn argument will be gone.
+func (u *provider) NewInternalLink(conn router.BatchConn, qSize int) router.Link {
+	queue := make(chan *router.Packet, qSize)
+	c := &udpConnection{
+		conn:  conn,
+		queue: queue,
+		name:  "internal",
+		ifID:  0,
+	}
+	u.allConnections[netip.AddrPort{}] = c
+	l := &internalLink{
+		queue: queue,
+	}
+	u.allLinks[netip.AddrPort{}] = l
+	return l
+}
+
+func (l *internalLink) GetScope() router.LinkScope {
+	return router.Internal
+}
+
+func (l *internalLink) IsUp() bool {
+	return true
+}
+
+func (l *internalLink) GetBfdSession() router.BfdSession {
+	return nil
+}
+
+func (l *internalLink) GetIfID() uint16 {
+	return 0
+}
+
+func (l *internalLink) GetRemote() netip.AddrPort {
+	return netip.AddrPort{}
+}
+
+// The packet's destination is already in the packet's meta-data.
+func (l *internalLink) Send(p *router.Packet) bool {
+	select {
+	case l.queue <- p:
+	default:
+		return false
+	}
+	return true
+}
+
+// The packet's destination is already in the packet's meta-data.
+func (l *internalLink) BlockSend(p *router.Packet) {
+	l.queue <- p
+}

From 986158605fa979fb390cde559f12da6b3c1bac65 Mon Sep 17 00:00:00 2001
From: Jean-Christophe Hugly <jice@scion.org>
Date: Fri, 24 Jan 2025 18:01:22 +0100
Subject: [PATCH 03/11] Lintify.

---
 router/dataplane.go               | 26 ++++++-------
 router/dataplane_internal_test.go |  2 +-
 router/dataplane_test.go          | 64 ++++++++-----------------------
 router/export_test.go             |  4 +-
 router/metrics.go                 |  4 +-
 router/underlay.go                |  4 +-
 router/underlayproviders/udpip.go |  4 +-
 7 files changed, 39 insertions(+), 69 deletions(-)

diff --git a/router/dataplane.go b/router/dataplane.go
index 8530ab8530..25d572fb17 100644
--- a/router/dataplane.go
+++ b/router/dataplane.go
@@ -229,17 +229,17 @@ var (
 	unsupportedV4MappedV6Address  = errors.New("unsupported v4mapped IP v6 address")
 	unsupportedUnspecifiedAddress = errors.New("unsupported unspecified address")
 	noBFDSessionFound             = errors.New("no BFD session was found")
-	noBFDSessionConfigured        = errors.New("no BFD sessions have been configured")
-	errPeeringEmptySeg0           = errors.New("zero-length segment[0] in peering path")
-	errPeeringEmptySeg1           = errors.New("zero-length segment[1] in peering path")
-	errPeeringNonemptySeg2        = errors.New("non-zero-length segment[2] in peering path")
-	errShortPacket                = errors.New("Packet is too short")
-	errBFDSessionDown             = errors.New("bfd session down")
-	expiredHop                    = errors.New("expired hop")
-	ingressInterfaceInvalid       = errors.New("ingress interface invalid")
-	macVerificationFailed         = errors.New("MAC verification failed")
-	badPacketSize                 = errors.New("bad packet size")
-	notImplemented                = errors.New("Not Yet Implemented")
+	// noBFDSessionConfigured        = errors.New("no BFD sessions have been configured")
+	errPeeringEmptySeg0     = errors.New("zero-length segment[0] in peering path")
+	errPeeringEmptySeg1     = errors.New("zero-length segment[1] in peering path")
+	errPeeringNonemptySeg2  = errors.New("non-zero-length segment[2] in peering path")
+	errShortPacket          = errors.New("Packet is too short")
+	errBFDSessionDown       = errors.New("bfd session down")
+	expiredHop              = errors.New("expired hop")
+	ingressInterfaceInvalid = errors.New("ingress interface invalid")
+	macVerificationFailed   = errors.New("MAC verification failed")
+	badPacketSize           = errors.New("bad packet size")
+	//	notImplemented          = errors.New("Not Yet Implemented")
 
 	// zeroBuffer will be used to reset the Authenticator option in the
 	// scionPacketProcessor.OptAuth
@@ -650,12 +650,12 @@ func (d *DataPlane) Run(ctx context.Context) error {
 		if s == nil {
 			continue
 		}
-		go func() {
+		go func(a netip.AddrPort) {
 			defer log.HandlePanic()
 			if err := s.Run(ctx); err != nil && err != bfd.AlreadyRunning {
 				log.Error("BFD session failed to start", "remote address", a, "err", err)
 			}
-		}()
+		}(a)
 	}
 
 	d.mtx.Unlock()
diff --git a/router/dataplane_internal_test.go b/router/dataplane_internal_test.go
index 41900c95cd..f000fd70cc 100644
--- a/router/dataplane_internal_test.go
+++ b/router/dataplane_internal_test.go
@@ -441,7 +441,7 @@ func TestSlowPathProcessing(t *testing.T) {
 	// ProcessPacket assumes some pre-conditions:
 	// * The ingress interface has to exist. This fake map is good for the test cases we have.
 	// * InternalNextHops may not be nil. Empty is ok for all the test cases we have.
-	fakeExternalInterfaces := map[uint16]struct{}{1: struct{}{}}
+	fakeExternalInterfaces := []uint16{1}
 	fakeInternalNextHops := map[uint16]netip.AddrPort{}
 	fakeServices := map[addr.SVC][]netip.AddrPort{}
 
diff --git a/router/dataplane_test.go b/router/dataplane_test.go
index 8218380112..91eaa70683 100644
--- a/router/dataplane_test.go
+++ b/router/dataplane_test.go
@@ -209,7 +209,7 @@ func TestDataPlaneAddNextHop(t *testing.T) {
 		d := &router.DataPlane{}
 		ctrl := gomock.NewController(t)
 		defer ctrl.Finish()
-		d.AddInternalInterface(mock_router.NewMockBatchConn(ctrl), nilAddr)
+		assert.NoError(t, d.AddInternalInterface(mock_router.NewMockBatchConn(ctrl), nilAddr))
 		d.FakeStart()
 		assert.Error(t, d.AddNextHop(45, l, r, nobfd, ""))
 	})
@@ -217,21 +217,21 @@ func TestDataPlaneAddNextHop(t *testing.T) {
 		d := &router.DataPlane{}
 		ctrl := gomock.NewController(t)
 		defer ctrl.Finish()
-		d.AddInternalInterface(mock_router.NewMockBatchConn(ctrl), nilAddr)
+		assert.NoError(t, d.AddInternalInterface(mock_router.NewMockBatchConn(ctrl), nilAddr))
 		assert.Error(t, d.AddNextHop(45, l, nilAddrPort, nobfd, ""))
 	})
 	t.Run("setting nil src is not allowed", func(t *testing.T) {
 		d := &router.DataPlane{}
 		ctrl := gomock.NewController(t)
 		defer ctrl.Finish()
-		d.AddInternalInterface(mock_router.NewMockBatchConn(ctrl), nilAddr)
+		assert.NoError(t, d.AddInternalInterface(mock_router.NewMockBatchConn(ctrl), nilAddr))
 		assert.Error(t, d.AddNextHop(45, nilAddrPort, r, nobfd, ""))
 	})
 	t.Run("normal add works", func(t *testing.T) {
 		d := &router.DataPlane{}
 		ctrl := gomock.NewController(t)
 		defer ctrl.Finish()
-		d.AddInternalInterface(mock_router.NewMockBatchConn(ctrl), nilAddr)
+		assert.NoError(t, d.AddInternalInterface(mock_router.NewMockBatchConn(ctrl), nilAddr))
 		assert.NoError(t, d.AddNextHop(45, l, r, nobfd, ""))
 		assert.NoError(t, d.AddNextHop(43, l, r, nobfd, ""))
 	})
@@ -239,7 +239,7 @@ func TestDataPlaneAddNextHop(t *testing.T) {
 		d := &router.DataPlane{}
 		ctrl := gomock.NewController(t)
 		defer ctrl.Finish()
-		d.AddInternalInterface(mock_router.NewMockBatchConn(ctrl), nilAddr)
+		assert.NoError(t, d.AddInternalInterface(mock_router.NewMockBatchConn(ctrl), nilAddr))
 		assert.NoError(t, d.AddNextHop(45, l, r, nobfd, ""))
 		assert.Error(t, d.AddNextHop(45, l, r, nobfd, ""))
 	})
@@ -691,7 +691,7 @@ func TestProcessPkt(t *testing.T) {
 	// * The ingress interface has to exist. This fake map is good for most test cases.
 	//   Others need a custom one.
 	// * InternalNextHops may not be nil. Empty is ok (sufficient unless testing AS transit).
-	fakeExternalInterfaces := map[uint16]struct{}{1: struct{}{}, 2: struct{}{}, 3: struct{}{}}
+	fakeExternalInterfaces := []uint16{1, 2, 3}
 	fakeInternalNextHops := map[uint16]netip.AddrPort{}
 
 	testCases := map[string]struct {
@@ -773,9 +773,7 @@ func TestProcessPkt(t *testing.T) {
 		"outbound": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]struct{}{
-						uint16(1): struct{}{},
-					},
+					[]uint16{1},
 					map[uint16]topology.LinkType{
 						1: topology.Child,
 					},
@@ -807,10 +805,7 @@ func TestProcessPkt(t *testing.T) {
 		"brtransit": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]struct{}{
-						uint16(1): struct{}{},
-						uint16(2): struct{}{},
-					},
+					[]uint16{1, 2},
 					map[uint16]topology.LinkType{
 						1: topology.Parent,
 						2: topology.Child,
@@ -842,10 +837,7 @@ func TestProcessPkt(t *testing.T) {
 		"brtransit non consdir": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]struct{}{
-						uint16(1): struct{}{},
-						uint16(2): struct{}{},
-					},
+					[]uint16{1, 2},
 					map[uint16]topology.LinkType{
 						2: topology.Parent,
 						1: topology.Child,
@@ -878,10 +870,7 @@ func TestProcessPkt(t *testing.T) {
 		"brtransit peering consdir": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]struct{}{
-						uint16(1): struct{}{},
-						uint16(2): struct{}{},
-					},
+					[]uint16{1, 2},
 					map[uint16]topology.LinkType{
 						1: topology.Peer,
 						2: topology.Child,
@@ -948,10 +937,7 @@ func TestProcessPkt(t *testing.T) {
 		"brtransit peering non consdir": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]struct{}{
-						uint16(1): struct{}{},
-						uint16(2): struct{}{},
-					},
+					[]uint16{1, 2},
 					map[uint16]topology.LinkType{
 						1: topology.Peer,
 						2: topology.Child,
@@ -1025,10 +1011,7 @@ func TestProcessPkt(t *testing.T) {
 			// happens on the next hop.
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]struct{}{
-						uint16(1): struct{}{},
-						uint16(2): struct{}{},
-					},
+					[]uint16{1, 2},
 					map[uint16]topology.LinkType{
 						1: topology.Peer,
 						2: topology.Child,
@@ -1099,10 +1082,7 @@ func TestProcessPkt(t *testing.T) {
 		"peering non consdir upstream": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]struct{}{
-						uint16(1): struct{}{},
-						uint16(2): struct{}{},
-					},
+					[]uint16{1, 2},
 					map[uint16]topology.LinkType{
 						1: topology.Peer,
 						2: topology.Child,
@@ -1181,11 +1161,7 @@ func TestProcessPkt(t *testing.T) {
 		"astransit direct": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]struct{}{
-						uint16(1): struct{}{},
-						// Interface 3 isn't in the external interfaces of this router
-						// another router has it.
-					},
+					[]uint16{1}, // Interface 3 is in the external interfaces of a sibling router
 					map[uint16]topology.LinkType{
 						1: topology.Core,
 						3: topology.Core,
@@ -1218,9 +1194,7 @@ func TestProcessPkt(t *testing.T) {
 		"astransit xover": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]struct{}{
-						uint16(51): struct{}{},
-					},
+					[]uint16{51},
 					map[uint16]topology.LinkType{
 						51: topology.Child,
 						3:  topology.Core,
@@ -1417,9 +1391,7 @@ func TestProcessPkt(t *testing.T) {
 		"reversed onehop outbound": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]struct{}{
-						uint16(1): struct{}{},
-					},
+					[]uint16{1},
 					nil,
 					mock_router.NewMockBatchConn(ctrl),
 					fakeInternalNextHops,
@@ -1477,9 +1449,7 @@ func TestProcessPkt(t *testing.T) {
 		"onehop outbound": {
 			prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
 				return router.NewDP(
-					map[uint16]struct{}{
-						uint16(2): struct{}{},
-					},
+					[]uint16{2},
 					nil,
 					mock_router.NewMockBatchConn(ctrl),
 					fakeInternalNextHops, nil,
diff --git a/router/export_test.go b/router/export_test.go
index 4c3607eccb..c67f8effb4 100644
--- a/router/export_test.go
+++ b/router/export_test.go
@@ -60,7 +60,7 @@ func NewPacket(raw []byte, src, dst *net.UDPAddr, ingress, egress uint16) *Packe
 }
 
 func NewDP(
-	external map[uint16]struct{},
+	external []uint16,
 	linkTypes map[uint16]topology.LinkType,
 	internal BatchConn,
 	internalNextHops map[uint16]netip.AddrPort,
@@ -86,7 +86,7 @@ func NewDP(
 
 	// Make dummy external interfaces, as requested by the test. They are not actually used to send
 	// or receive. The blank address might cause issues, though.
-	for i, _ := range external {
+	for _, i := range external {
 		dp.interfaces[i] = dp.underlay.NewExternalLink(nil, 64, nil, netip.AddrPort{}, i)
 	}
 
diff --git a/router/metrics.go b/router/metrics.go
index 7bbf1c8a85..d26b2aa9ef 100644
--- a/router/metrics.go
+++ b/router/metrics.go
@@ -356,7 +356,9 @@ func newOutputMetrics(
 	return om
 }
 
-func interfaceLabels(id uint16, localIA addr.IA, scope LinkScope, neighbors map[uint16]addr.IA) prometheus.Labels {
+func interfaceLabels(
+	id uint16, localIA addr.IA, scope LinkScope, neighbors map[uint16]addr.IA) prometheus.Labels {
+
 	if id == 0 {
 		return prometheus.Labels{
 			"isd_as":          localIA.String(),
diff --git a/router/underlay.go b/router/underlay.go
index 1c2aae49cd..7f9338726b 100644
--- a/router/underlay.go
+++ b/router/underlay.go
@@ -33,8 +33,8 @@ const (
 // connection, with a bfdSession, a destination address, etc. It also allows the concrete send
 // operation to be delegated to different underlay implementations. The association between
 // link and underlay connection is a channel, on the sending side, and should be a demultiplexer on
-// the receiving side. The demultiplexer must have a src-addr:link map in all cases where links share
-// connections.
+// the receiving side. The demultiplexer must have a src-addr:link map in all cases where links
+// share connections.
 //
 // Regardless of underlay, links come in three scopes: internal, sibling, and external. The
 // difference in behaviour is hidden from the rest of the router. The router only needs to
diff --git a/router/underlayproviders/udpip.go b/router/underlayproviders/udpip.go
index 6fbeb3a173..010154b4c3 100644
--- a/router/underlayproviders/udpip.go
+++ b/router/underlayproviders/udpip.go
@@ -17,8 +17,6 @@ package underlayproviders
 import (
 	"net/netip"
 
-	// Until we switch to Go 1.23 and can use slices.Collect(maps.Values())
-
 	"github.com/scionproto/scion/router"
 )
 
@@ -113,7 +111,7 @@ func (u *udpConnection) IfID() uint16 {
 	return u.ifID
 }
 
-// todo(jiceatscion): use inheritence between implementations?
+// todo(jiceatscion): use inheritance between implementations?
 
 type externalLink struct {
 	queue      chan<- *router.Packet

From a2fb58c1186dbcbafc115163299028f4dc686833 Mon Sep 17 00:00:00 2001
From: Jean-Christophe Hugly <jice@scion.org>
Date: Fri, 24 Jan 2025 19:37:44 +0100
Subject: [PATCH 04/11] Remove a couple of interface method calls out of the
 critical path.

---
 router/dataplane.go | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/router/dataplane.go b/router/dataplane.go
index 25d572fb17..25368dd565 100644
--- a/router/dataplane.go
+++ b/router/dataplane.go
@@ -697,9 +697,9 @@ func (d *DataPlane) initQueues(processorQueueSize int) ([]chan *Packet, []chan *
 //
 // Incremental refactoring: we should not have direct knowledge of connections. Later we will move
 // parts of this to the connection itself, so that we don't have to know.
-func (d *DataPlane) runReceiver(c UnderlayConnection, procQs []chan *Packet) {
+func (d *DataPlane) runReceiver(u UnderlayConnection, procQs []chan *Packet) {
 
-	log.Debug("Run receiver", "connection", c.Name())
+	log.Debug("Run receiver", "connection", u.Name())
 
 	// Each receiver (therefore each input interface) has a unique random seed for the procID hash
 	// function.
@@ -708,8 +708,8 @@ func (d *DataPlane) runReceiver(c UnderlayConnection, procQs []chan *Packet) {
 	if _, err := rand.Read(randomBytes); err != nil {
 		panic("Error while generating random value")
 	}
-	for _, b := range randomBytes {
-		hashSeed = hashFNV1a(hashSeed, b)
+	for _, c := range randomBytes {
+		hashSeed = hashFNV1a(hashSeed, c)
 	}
 
 	// A collection of socket messages, as the readBatch API expects them. We keep using the same
@@ -720,8 +720,9 @@ func (d *DataPlane) runReceiver(c UnderlayConnection, procQs []chan *Packet) {
 	// The packet owns the buffer that we set in the matching msg, plus the metadata that we'll add.
 	packets := make([]*Packet, d.RunConfig.BatchSize)
 
-	numReusable := 0                         // unused buffers from previous loop
-	metrics := d.forwardingMetrics[c.IfID()] // If receiver exists, fw metrics exist too.
+	numReusable := 0 // unused buffers from previous loop
+	ifID := u.IfID()
+	metrics := d.forwardingMetrics[ifID] // If receiver exists, fw metrics exist too.
 
 	enqueueForProcessing := func(size int, srcAddr *net.UDPAddr, pkt *Packet) {
 		sc := classOfSize(size)
@@ -740,7 +741,7 @@ func (d *DataPlane) runReceiver(c UnderlayConnection, procQs []chan *Packet) {
 		// Incremental refactoring: We should begin with finding the link and get the ifID
 		// from there. We will do that once we actually move these pre-processing tasks directly
 		// into the underlay.
-		pkt.ingress = c.IfID()
+		pkt.ingress = ifID
 		pkt.srcAddr = srcAddr
 		select {
 		case procQs[procID] <- pkt:
@@ -750,6 +751,7 @@ func (d *DataPlane) runReceiver(c UnderlayConnection, procQs []chan *Packet) {
 		}
 	}
 
+	conn := u.Conn()
 	for d.IsRunning() {
 		// collect packets.
 
@@ -762,10 +764,10 @@ func (d *DataPlane) runReceiver(c UnderlayConnection, procQs []chan *Packet) {
 		}
 
 		// Fill the packets
-		numPkts, err := c.Conn().ReadBatch(msgs)
+		numPkts, err := conn.ReadBatch(msgs)
 		numReusable = len(msgs) - numPkts
 		if err != nil {
-			log.Debug("Error while reading batch", "interfaceID", c.IfID(), "err", err)
+			log.Debug("Error while reading batch", "interfaceID", ifID, "err", err)
 			continue
 		}
 		for i, msg := range msgs[:numPkts] {
@@ -1033,9 +1035,9 @@ func updateOutputMetrics(metrics interfaceMetrics, packets []*Packet) {
 //     smaller batches?
 //
 // For now, we do the first option.
-func (d *DataPlane) runForwarder(c UnderlayConnection) {
+func (d *DataPlane) runForwarder(u UnderlayConnection) {
 
-	log.Debug("Run forwarder", "connection", c.Name())
+	log.Debug("Run forwarder", "connection", u.Name())
 
 	// We use this somewhat like a ring buffer.
 	pkts := make([]*Packet, d.RunConfig.BatchSize)
@@ -1047,12 +1049,13 @@ func (d *DataPlane) runForwarder(c UnderlayConnection) {
 		msgs[i].Buffers = make([][]byte, 1)
 	}
 
-	metrics := d.forwardingMetrics[c.IfID()]
-
+	c := u.Queue()
+	conn := u.Conn()
+	metrics := d.forwardingMetrics[u.IfID()]
 	toWrite := 0
 	for d.IsRunning() {
 		// Top-up our batch.
-		toWrite += readUpTo(c.Queue(), d.RunConfig.BatchSize-toWrite, toWrite == 0, pkts[toWrite:])
+		toWrite += readUpTo(c, d.RunConfig.BatchSize-toWrite, toWrite == 0, pkts[toWrite:])
 
 		// Turn the packets into underlay messages that WriteBatch can send.
 		for i, p := range pkts[:toWrite] {
@@ -1062,7 +1065,7 @@ func (d *DataPlane) runForwarder(c UnderlayConnection) {
 				msgs[i].Addr = p.DstAddr
 			}
 		}
-		written, _ := c.Conn().WriteBatch(msgs[:toWrite], 0)
+		written, _ := conn.WriteBatch(msgs[:toWrite], 0)
 		if written < 0 {
 			// WriteBatch returns -1 on error, we just consider this as
 			// 0 packets written

From b1eb189406287bd334d477b55ce168c63069d1aa Mon Sep 17 00:00:00 2001
From: Jean-Christophe Hugly <jice@scion.org>
Date: Mon, 27 Jan 2025 16:31:53 +0100
Subject: [PATCH 05/11] Address reviewers concerns (TODO comments and variable
 names).

---
 router/dataplane.go               | 73 +++++++++++++++----------------
 router/underlay.go                | 14 +++---
 router/underlayproviders/udpip.go | 25 ++++++-----
 3 files changed, 55 insertions(+), 57 deletions(-)

diff --git a/router/dataplane.go b/router/dataplane.go
index 25368dd565..a38a50ff7e 100644
--- a/router/dataplane.go
+++ b/router/dataplane.go
@@ -93,7 +93,7 @@ type BatchConn interface {
 
 // underlay is a pointer to our underlay provider.
 //
-// incremental refactoring: this allows for a single underlay. In the future, each link could be
+// TODO(multi_underlay): this allows for a single underlay. In the future, each link could be
 // via a different underlay. That would have to be supported by the configuration code and there
 // would likely be a registry of underlays. For now, That's the whole registry.
 var newUnderlay func() UnderlayProvider
@@ -229,17 +229,15 @@ var (
 	unsupportedV4MappedV6Address  = errors.New("unsupported v4mapped IP v6 address")
 	unsupportedUnspecifiedAddress = errors.New("unsupported unspecified address")
 	noBFDSessionFound             = errors.New("no BFD session was found")
-	// noBFDSessionConfigured        = errors.New("no BFD sessions have been configured")
-	errPeeringEmptySeg0     = errors.New("zero-length segment[0] in peering path")
-	errPeeringEmptySeg1     = errors.New("zero-length segment[1] in peering path")
-	errPeeringNonemptySeg2  = errors.New("non-zero-length segment[2] in peering path")
-	errShortPacket          = errors.New("Packet is too short")
-	errBFDSessionDown       = errors.New("bfd session down")
-	expiredHop              = errors.New("expired hop")
-	ingressInterfaceInvalid = errors.New("ingress interface invalid")
-	macVerificationFailed   = errors.New("MAC verification failed")
-	badPacketSize           = errors.New("bad packet size")
-	//	notImplemented          = errors.New("Not Yet Implemented")
+	errPeeringEmptySeg0           = errors.New("zero-length segment[0] in peering path")
+	errPeeringEmptySeg1           = errors.New("zero-length segment[1] in peering path")
+	errPeeringNonemptySeg2        = errors.New("non-zero-length segment[2] in peering path")
+	errShortPacket                = errors.New("Packet is too short")
+	errBFDSessionDown             = errors.New("bfd session down")
+	expiredHop                    = errors.New("expired hop")
+	ingressInterfaceInvalid       = errors.New("ingress interface invalid")
+	macVerificationFailed         = errors.New("MAC verification failed")
+	badPacketSize                 = errors.New("bad packet size")
 
 	// zeroBuffer will be used to reset the Authenticator option in the
 	// scionPacketProcessor.OptAuth
@@ -353,8 +351,7 @@ func (d *DataPlane) AddInternalInterface(conn BatchConn, ip netip.Addr) error {
 		return alreadySet
 	}
 
-	l := d.underlay.NewInternalLink(conn, d.RunConfig.BatchSize)
-	d.interfaces[0] = l
+	d.interfaces[0] = d.underlay.NewInternalLink(conn, d.RunConfig.BatchSize)
 	d.internalIP = ip
 
 	return nil
@@ -388,8 +385,8 @@ func (d *DataPlane) AddExternalInterface(ifID uint16, conn BatchConn,
 	if _, exists := d.interfaces[ifID]; exists {
 		return serrors.JoinNoStack(alreadySet, nil, "ifID", ifID)
 	}
-	lk := d.underlay.NewExternalLink(conn, d.RunConfig.BatchSize, bfd, dst.Addr, ifID)
-	d.interfaces[ifID] = lk
+	d.interfaces[ifID] = d.underlay.NewExternalLink(
+		conn, d.RunConfig.BatchSize, bfd, dst.Addr, ifID)
 	return nil
 }
 
@@ -466,7 +463,7 @@ func (d *DataPlane) addExternalInterfaceBFD(ifID uint16,
 // returns InterfaceUp if the relevant bfdsession state is up, or if there is no BFD
 // session. Otherwise, it returns InterfaceDown.
 func (d *DataPlane) getInterfaceState(ifID uint16) control.InterfaceState {
-	if l := d.interfaces[ifID]; l != nil && !l.IsUp() {
+	if link := d.interfaces[ifID]; link != nil && !link.IsUp() {
 		return control.InterfaceDown
 	}
 	return control.InterfaceUp
@@ -561,8 +558,7 @@ func (d *DataPlane) AddNextHop(ifID uint16, src, dst netip.AddrPort, cfg control
 		return serrors.JoinNoStack(alreadySet, nil, "ifID", ifID)
 	}
 
-	sib := d.underlay.NewSiblingLink(d.RunConfig.BatchSize, bfd, dst)
-	d.interfaces[ifID] = sib
+	d.interfaces[ifID] = d.underlay.NewSiblingLink(d.RunConfig.BatchSize, bfd, dst)
 	return nil
 }
 
@@ -610,7 +606,7 @@ func (d *DataPlane) Run(ctx context.Context) error {
 	d.mtx.Lock()
 	d.initMetrics()
 
-	// incremental refactoring: we leave all the ingest work here for now, so we get the set of
+	// TODO(multi_underlay): we leave all the ingest work here for now, so we get the set of
 	// connections from the underlay.
 	underlayConnections := d.underlay.GetConnections()
 	processorQueueSize := max(
@@ -645,17 +641,17 @@ func (d *DataPlane) Run(ctx context.Context) error {
 		}(i)
 	}
 
-	for a, l := range d.underlay.GetLinks() {
-		s := l.GetBfdSession()
+	for addr, link := range d.underlay.GetLinks() {
+		s := link.GetBfdSession()
 		if s == nil {
 			continue
 		}
-		go func(a netip.AddrPort) {
+		go func(addr netip.AddrPort) {
 			defer log.HandlePanic()
 			if err := s.Run(ctx); err != nil && err != bfd.AlreadyRunning {
-				log.Error("BFD session failed to start", "remote address", a, "err", err)
+				log.Error("BFD session failed to start", "remote address", addr, "err", err)
 			}
-		}(a)
+		}(addr)
 	}
 
 	d.mtx.Unlock()
@@ -695,7 +691,7 @@ func (d *DataPlane) initQueues(processorQueueSize int) ([]chan *Packet, []chan *
 
 // runReceiver handles incoming traffic from the given connection.
 //
-// Incremental refactoring: we should not have direct knowledge of connections. Later we will move
+// TODO(multi_underlay): we should not have direct knowledge of connections. Later we will move
 // parts of this to the connection itself, so that we don't have to know.
 func (d *DataPlane) runReceiver(u UnderlayConnection, procQs []chan *Packet) {
 
@@ -738,7 +734,7 @@ func (d *DataPlane) runReceiver(u UnderlayConnection, procQs []chan *Packet) {
 		}
 
 		pkt.rawPacket = pkt.rawPacket[:size] // Update size; readBatch does not.
-		// Incremental refactoring: We should begin with finding the link and get the ifID
+		// TODO(multi_underlay): We should begin with finding the link and get the ifID
 		// from there. We will do that once we actually move these pre-processing tasks directly
 		// into the underlay.
 		pkt.ingress = ifID
@@ -1019,22 +1015,23 @@ func updateOutputMetrics(metrics interfaceMetrics, packets []*Packet) {
 	}
 }
 
-// Big issue with metrics and ifID. If an underlay connection must be shared between links
-// (for example, libling links), then we don't have a specific ifID in the connection per se. It
-// changes for each packet. As a result, in the shared case, either we account all metrics to
-// whatever placeholder ifID we have (i.e. 0), or we have to use pkt.egress and lookup the metrics
-// in the map for each packet. This is too expensive.
+// TODO(jiceatscion): There is a big issue with metrics and ifID. If an underlay connection must be
+// shared between links (for example, sibling links), then we don't have a specific ifID in the
+// connection per se. It changes for each packet. As a result, in the shared case, either we account
+// all metrics to whatever placeholder ifID we have (i.e. 0), or we have to use pkt.egress and
+// lookup the metrics in the map for each packet. This is too expensive.
 //
 // Mitigations:
 //   - use ifID even if it is 0 for sibling links - no worse than before, since sibling links were
-//     already redirected to interface 0. Ok, until we have fully shared forwarders (like with an
+//     already redirected to interface 0 (...until we have fully shared forwarders - like with an
 //     XDP underlay impl).
 //   - stage our own internal metrics map, sorted by ifID = pkt.egress, and batch update the
 //     metrics... might not be much cheaper than the naive way.
 //   - Use one fw queue per ifID in each connection... but then have to round-robin for fairness....
 //     smaller batches?
 //
-// For now, we do the first option.
+// For now, we do the first option. Whether that is good enough is still TBD.
+
 func (d *DataPlane) runForwarder(u UnderlayConnection) {
 
 	log.Debug("Run forwarder", "connection", u.Name())
@@ -1200,11 +1197,11 @@ func (p *scionPacketProcessor) processInterBFD(oh *onehop.Path, data []byte) dis
 
 	// If this is an inter-AS BFD, it can via an interface we own. So the ifID matches one link
 	// and the ifID better be valid. In the future that will be checked upstream from here.
-	l, exists := p.d.interfaces[p.pkt.ingress]
+	link, exists := p.d.interfaces[p.pkt.ingress]
 	if !exists {
 		return errorDiscard("error", noBFDSessionFound)
 	}
-	session := l.GetBfdSession()
+	session := link.GetBfdSession()
 	if session == nil {
 		return errorDiscard("error", noBFDSessionFound)
 	}
@@ -1525,7 +1522,7 @@ func (p *scionPacketProcessor) respInvalidDstIA() disposition {
 // this check prevents malicious end hosts in the local AS from bypassing the
 // SrcIA checks by disguising packets as transit traffic.
 //
-// Incremental refactoring: All or part of this check should move to the underlay.
+// TODO(multi_underlay): All or part of this check should move to the underlay.
 func (p *scionPacketProcessor) validateTransitUnderlaySrc() disposition {
 	if p.path.IsFirstHop() || p.pkt.ingress != 0 {
 		// not a transit packet, nothing to check
@@ -1545,7 +1542,7 @@ func (p *scionPacketProcessor) validateTransitUnderlaySrc() disposition {
 	return pForward
 }
 
-// Validates the egress interface referenced by the current hop. This  is not called for
+// Validates the egress interface referenced by the current hop. This is not called for
 // packets to be delivered to the local AS, so pkt.egress is never 0.
 // If pkt.ingress is zero, the packet can be coming from either a local end-host or a
 // sibling router. In either of these cases, it must be leaving via a locally owned external
diff --git a/router/underlay.go b/router/underlay.go
index 7f9338726b..0bdcbc6a44 100644
--- a/router/underlay.go
+++ b/router/underlay.go
@@ -30,7 +30,7 @@ const (
 )
 
 // Link embodies the router's idea of a point to point connection. A link associates the underlay
-// connection, with a bfdSession, a destination address, etc. It also allows the concrete send
+// connection with a bfdSession, a destination address, etc. It also allows the concrete send
 // operation to be delegated to different underlay implementations. The association between
 // link and underlay connection is a channel, on the sending side, and should be a demultiplexer on
 // the receiving side. The demultiplexer must have a src-addr:link map in all cases where links
@@ -45,7 +45,7 @@ type Link interface {
 	GetBfdSession() BfdSession
 	IsUp() bool
 	GetIfID() uint16
-	GetRemote() netip.AddrPort // incremental refactoring: using code will move to underlay.
+	GetRemote() netip.AddrPort // TODO(multi_underlay): using code will move to underlay.
 	Send(p *Packet) bool
 	BlockSend(p *Packet)
 }
@@ -55,7 +55,7 @@ type Link interface {
 // For any given underlay, there are three kinds of Link implementations to choose from.
 // The difference between them is the intent regarding addressing.
 //
-// Incremental refactoring: addresses are still explicitly IP/port. In the next step, we have to
+// TODO(multi_underlay): addresses are still explicitly IP/port. In the next step, we have to
 // make them opaque; to be interpreted only by the underlay implementation.
 type UnderlayProvider interface {
 
@@ -80,14 +80,14 @@ type UnderlayProvider interface {
 
 	// GetConnections returns the set of configured distinct connections in the provider.
 	//
-	// Incremental refactoring: this exists so most of the receiving code can stay in the main
+	// TODO(multi_underlay): this exists so most of the receiving code can stay in the main
 	// dataplane code for now. There may be fewer connections than links. For example, right now
 	// all sibling links and the internal link use a shared un-bound connection.
 	GetConnections() map[netip.AddrPort]UnderlayConnection
 
 	// GetLinks returns the set of configured distinct links in the provider.
 	//
-	// Incremental refactoring: this exists so most of the receiving code can stay in-here for now.
+	// TODO(multi_underlay): this exists so most of the receiving code can stay in-here for now.
 	// There may be fewer links than ifIDs. For example, all interfaces owned by one given sibling
 	// router are connected via the same link because the remote address is the same.
 	GetLinks() map[netip.AddrPort]Link
@@ -95,7 +95,7 @@ type UnderlayProvider interface {
 	// GetLink returns a link that matches the given source address. If the address is not that of
 	// a known link, then the internal link is returned.
 	//
-	// Increamental refactoring: This has to exist until incmoing packets are "demuxed" (i.e.
+	// TODO(multi_underlay): This has to exist until incmoing packets are "demuxed" (i.e.
 	// matched with a link), on ingest by the underlay. That would imply moving a part of the
 	// runReceiver routine to the underlay. We will do that in the next step.
 	GetLink(netip.AddrPort) Link
@@ -104,7 +104,7 @@ type UnderlayProvider interface {
 // UnderlayConnection defines the minimum interface that the router expects from an underlay
 // connection.
 //
-// Incremental refactoring: this will eventually be reduced to nothing at all because the sender
+// TODO(multi_underlay): this will eventually be reduced to nothing at all because the sender
 // receiver tasks will be part of the underlay.
 type UnderlayConnection interface {
 	Conn() BatchConn
diff --git a/router/underlayproviders/udpip.go b/router/underlayproviders/udpip.go
index 010154b4c3..af3d04e712 100644
--- a/router/underlayproviders/udpip.go
+++ b/router/underlayproviders/udpip.go
@@ -24,7 +24,7 @@ import (
 //
 // This is currently the only implementation. The goal of splitting out this code from the router
 // is to enable other implementations. However, as a first step, we continue assuming that the
-// batchConn is given to us and is a Udp socket and that, in the case of externalLink, it is bound.
+// batchConn is given to us and is a UDP socket and that, in the case of externalLink, it is bound.
 type provider struct {
 	allLinks       map[netip.AddrPort]router.Link
 	allConnections map[netip.AddrPort]*udpConnection
@@ -46,9 +46,9 @@ func newProvider() router.UnderlayProvider {
 }
 
 func (u *provider) GetConnections() map[netip.AddrPort]router.UnderlayConnection {
-	// a map of interfaces and a map of concrete implementations aren't compatible.
-	// For the same reason, we cannot have the map of concrete as our return type; it
-	// does not satisfy the GetConnections interface (so much for the "don't return
+	// A map of interfaces and a map of concrete implementations aren't compatible.
+	// For the same reason, we cannot have the map of concrete implementations as our return type;
+	// it does not satisfy the GetConnections interface (so much for the "don't return
 	// interfaces" rule)... Brilliant, Go.
 	// Since we do not want to store our own things as interfaces, we have to translate.
 	// Good thing it doesn't happen often.
@@ -73,7 +73,7 @@ func (u *provider) GetLink(addr netip.AddrPort) router.Link {
 }
 
 // udpConnection is simply the combination of a BatchConn and sending queue (plus metadata for
-// logs and such). This allows udp connections to be shared between links. Bundling link and
+// logs and such). This allows UDP connections to be shared between links. Bundling link and
 // connection together is possible and simpler for the code here, but leaks more refactoring changes
 // in the main router code. Specifically, either:
 //   - sibling links would each need an independent socket to the sibling router, which
@@ -87,7 +87,7 @@ type udpConnection struct {
 	name  string // for logs. It's more informative than ifID.
 }
 
-// Incremental refactoring: The following implements UnderlayConnection so some of the code
+// TODO(multi_underlay): The following implements UnderlayConnection so some of the code
 // that needs to interact with it can stay in the main router code. This will be removed in the
 // next step
 
@@ -122,7 +122,7 @@ type externalLink struct {
 
 // NewExternalLink returns an external link over the UdpIpUnderlay.
 //
-// Incremental refactoring: we get the connection ready-made and require it to be bound. So, we
+// TODO(multi_underlay): we get the connection ready-made and require it to be bound. So, we
 // don't keep the remote address, but in the future, we will be making the connections, and
 // BatchConn will be gone.
 func (u *provider) NewExternalLink(
@@ -191,7 +191,7 @@ type siblingLink struct {
 
 // newSiblingLink returns a sibling link over the UdpIpUnderlay.
 //
-// Incremental refactoring: this can only be an improvement over internalLink if we have a bound
+// TODO(multi_underlay): this can only be an improvement over internalLink if we have a bound
 // batchConn with the sibling router. However, currently the caller doesn't have one to give us;
 // the main code has so far been reusing the internal connection. So, that's what we do for now.
 // As a result, we keep the remote address; as we need to supply it for every packet being sent
@@ -199,9 +199,9 @@ type siblingLink struct {
 // In the future we will be making one connection per remote address and we might even be able
 // to erase the separation between link and connection for this implementation. Side effect
 // of moving the address:link here: the router does not know if there is an existing link. As
-// a result it has to give us a bfdSession in all cases and if we might throuw it away (there
+// a result it has to give us a bfdSession in all cases and if we might throw it away (there
 // are no permanent resources attached to it). This will be fixed by moving some bfd related code
-// in-here; but later.
+// in-here.
 func (u *provider) NewSiblingLink(
 	qSize int, bfd router.BfdSession, remote netip.AddrPort) router.Link {
 
@@ -216,7 +216,8 @@ func (u *provider) NewSiblingLink(
 	// happens right now and it can't work if this is called before newInternalLink.
 	c, exists := u.allConnections[netip.AddrPort{}]
 	if !exists {
-		// That doesn't actually happen and we'll get rid of this soon.
+		// TODO(multi_underlay):That doesn't actually happen.
+		// It is only required until we stop sharing the internal connection.
 		panic("newSiblingLink called before newInternalLink")
 	}
 
@@ -274,7 +275,7 @@ type internalLink struct {
 
 // newSiblingLink returns a sibling link over the UdpIpUnderlay.
 //
-// Incremental refactoring: we get the connection ready made. In the future we will be making it
+// TODO(multi_underlay): we get the connection ready made. In the future we will be making it
 // and the conn argument will be gone.
 func (u *provider) NewInternalLink(conn router.BatchConn, qSize int) router.Link {
 	queue := make(chan *router.Packet, qSize)

From 2df9f4dc82d5ea16849a63c4ea5d48bc519a3d6e Mon Sep 17 00:00:00 2001
From: Jean-Christophe Hugly <jice@scion.org>
Date: Mon, 27 Jan 2025 16:49:20 +0100
Subject: [PATCH 06/11] Adjust a couple more comment.

---
 router/underlayproviders/udpip.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/router/underlayproviders/udpip.go b/router/underlayproviders/udpip.go
index af3d04e712..f09253d5e2 100644
--- a/router/underlayproviders/udpip.go
+++ b/router/underlayproviders/udpip.go
@@ -124,7 +124,7 @@ type externalLink struct {
 //
 // TODO(multi_underlay): we get the connection ready-made and require it to be bound. So, we
 // don't keep the remote address, but in the future, we will be making the connections, and
-// BatchConn will be gone.
+// the conn argument will be gone.
 func (u *provider) NewExternalLink(
 	conn router.BatchConn,
 	qSize int,
@@ -199,7 +199,7 @@ type siblingLink struct {
 // In the future we will be making one connection per remote address and we might even be able
 // to erase the separation between link and connection for this implementation. Side effect
 // of moving the address:link here: the router does not know if there is an existing link. As
-// a result it has to give us a bfdSession in all cases and if we might throw it away (there
+// a result it has to give us a bfdSession in all cases and we might throw it away (there
 // are no permanent resources attached to it). This will be fixed by moving some bfd related code
 // in-here.
 func (u *provider) NewSiblingLink(

From 275a5d2a1b3a32770f3c8f90ea7ecdb37b71ea94 Mon Sep 17 00:00:00 2001
From: Jean-Christophe Hugly <jice@scion.org>
Date: Thu, 30 Jan 2025 13:03:03 +0100
Subject: [PATCH 07/11] Implement reviewer's suggestions.

---
 router/dataplane.go               | 62 +++++++++++++++----------------
 router/dataplane_internal_test.go |  4 +-
 router/underlay.go                | 32 ++++++++--------
 router/underlayproviders/udpip.go | 55 +++++++++++++--------------
 4 files changed, 77 insertions(+), 76 deletions(-)

diff --git a/router/dataplane.go b/router/dataplane.go
index a38a50ff7e..0298d652be 100644
--- a/router/dataplane.go
+++ b/router/dataplane.go
@@ -78,7 +78,7 @@ const (
 	is32bit = 1 - (ptrSize-4)/4
 )
 
-type BfdSession interface {
+type BFDSession interface {
 	Run(ctx context.Context) error
 	ReceiveMessage(*layers.BFD)
 	IsUp() bool
@@ -375,7 +375,7 @@ func (d *DataPlane) AddExternalInterface(ifID uint16, conn BatchConn,
 	if d.underlay == nil {
 		d.underlay = newUnderlay()
 	}
-	bfd, err := d.addExternalInterfaceBFD(ifID, src, dst, cfg)
+	bfd, err := d.newExternalInterfaceBFD(ifID, src, dst, cfg)
 	if err != nil {
 		return serrors.Wrap("adding external BFD", err, "if_id", ifID)
 	}
@@ -432,8 +432,8 @@ func (d *DataPlane) AddLinkType(ifID uint16, linkTo topology.LinkType) error {
 }
 
 // AddExternalInterfaceBFD adds the inter AS connection BFD session.
-func (d *DataPlane) addExternalInterfaceBFD(ifID uint16,
-	src, dst control.LinkEnd, cfg control.BFD) (BfdSession, error) {
+func (d *DataPlane) newExternalInterfaceBFD(ifID uint16,
+	src, dst control.LinkEnd, cfg control.BFD) (BFDSession, error) {
 
 	if *cfg.Disable {
 		return nil, nil
@@ -456,11 +456,11 @@ func (d *DataPlane) addExternalInterfaceBFD(ifID uint16,
 	if err != nil {
 		return nil, err
 	}
-	return d.addBFDController(ifID, s, cfg, m)
+	return newBFDController(ifID, s, cfg, m)
 }
 
 // getInterfaceState checks if there is a bfd session for the input interfaceID and
-// returns InterfaceUp if the relevant bfdsession state is up, or if there is no BFD
+// returns InterfaceUp if the relevant BFDSession state is up, or if there is no BFD
 // session. Otherwise, it returns InterfaceDown.
 func (d *DataPlane) getInterfaceState(ifID uint16) control.InterfaceState {
 	if link := d.interfaces[ifID]; link != nil && !link.IsUp() {
@@ -469,8 +469,8 @@ func (d *DataPlane) getInterfaceState(ifID uint16) control.InterfaceState {
 	return control.InterfaceUp
 }
 
-func (d *DataPlane) addBFDController(ifID uint16, s bfd.Sender, cfg control.BFD,
-	metrics bfd.Metrics) (BfdSession, error) {
+func newBFDController(ifID uint16, s bfd.Sender, cfg control.BFD,
+	metrics bfd.Metrics) (BFDSession, error) {
 
 	// Generate random discriminator. It can't be zero.
 	discInt, err := rand.Int(rand.Reader, big.NewInt(0xfffffffe))
@@ -547,7 +547,7 @@ func (d *DataPlane) AddNextHop(ifID uint16, src, dst netip.AddrPort, cfg control
 	if d.underlay == nil {
 		d.underlay = newUnderlay()
 	}
-	bfd, err := d.addNextHopBFD(ifID, src, dst, cfg, sibling)
+	bfd, err := d.newNextHopBFD(ifID, src, dst, cfg, sibling)
 	if err != nil {
 		return serrors.Wrap("adding next hop BFD", err, "if_id", ifID)
 	}
@@ -565,8 +565,8 @@ func (d *DataPlane) AddNextHop(ifID uint16, src, dst netip.AddrPort, cfg control
 // AddNextHopBFD adds the BFD session for the next hop address.
 // If the remote ifID belongs to an existing address, the existing
 // BFD session will be re-used.
-func (d *DataPlane) addNextHopBFD(ifID uint16, src, dst netip.AddrPort, cfg control.BFD,
-	sibling string) (BfdSession, error) {
+func (d *DataPlane) newNextHopBFD(ifID uint16, src, dst netip.AddrPort, cfg control.BFD,
+	sibling string) (BFDSession, error) {
 
 	if *cfg.Disable {
 		return nil, nil
@@ -586,7 +586,7 @@ func (d *DataPlane) addNextHopBFD(ifID uint16, src, dst netip.AddrPort, cfg cont
 	if err != nil {
 		return nil, err
 	}
-	return d.addBFDController(ifID, s, cfg, m)
+	return newBFDController(ifID, s, cfg, m)
 }
 
 func max(a int, b int) int {
@@ -608,7 +608,7 @@ func (d *DataPlane) Run(ctx context.Context) error {
 
 	// TODO(multi_underlay): we leave all the ingest work here for now, so we get the set of
 	// connections from the underlay.
-	underlayConnections := d.underlay.GetConnections()
+	underlayConnections := d.underlay.Connections()
 	processorQueueSize := max(
 		len(underlayConnections)*d.RunConfig.BatchSize/d.RunConfig.NumProcessors,
 		d.RunConfig.BatchSize)
@@ -618,11 +618,11 @@ func (d *DataPlane) Run(ctx context.Context) error {
 
 	d.setRunning()
 	for _, c := range underlayConnections {
-		go func(c UnderlayConnection) {
+		go func(c UnderlayConn) {
 			defer log.HandlePanic()
 			d.runReceiver(c, procQs)
 		}(c)
-		go func(c UnderlayConnection) {
+		go func(c UnderlayConn) {
 			defer log.HandlePanic()
 			d.runForwarder(c)
 		}(c)
@@ -641,8 +641,8 @@ func (d *DataPlane) Run(ctx context.Context) error {
 		}(i)
 	}
 
-	for addr, link := range d.underlay.GetLinks() {
-		s := link.GetBfdSession()
+	for addr, link := range d.underlay.Links() {
+		s := link.BFDSession()
 		if s == nil {
 			continue
 		}
@@ -693,7 +693,7 @@ func (d *DataPlane) initQueues(processorQueueSize int) ([]chan *Packet, []chan *
 //
 // TODO(multi_underlay): we should not have direct knowledge of connections. Later we will move
 // parts of this to the connection itself, so that we don't have to know.
-func (d *DataPlane) runReceiver(u UnderlayConnection, procQs []chan *Packet) {
+func (d *DataPlane) runReceiver(u UnderlayConn, procQs []chan *Packet) {
 
 	log.Debug("Run receiver", "connection", u.Name())
 
@@ -1032,7 +1032,7 @@ func updateOutputMetrics(metrics interfaceMetrics, packets []*Packet) {
 //
 // For now, we do the first option. Whether that is good enough is still TBD.
 
-func (d *DataPlane) runForwarder(u UnderlayConnection) {
+func (d *DataPlane) runForwarder(u UnderlayConn) {
 
 	log.Debug("Run forwarder", "connection", u.Name())
 
@@ -1201,7 +1201,7 @@ func (p *scionPacketProcessor) processInterBFD(oh *onehop.Path, data []byte) dis
 	if !exists {
 		return errorDiscard("error", noBFDSessionFound)
 	}
-	session := link.GetBfdSession()
+	session := link.BFDSession()
 	if session == nil {
 		return errorDiscard("error", noBFDSessionFound)
 	}
@@ -1218,7 +1218,7 @@ func (p *scionPacketProcessor) processIntraBFD(data []byte) disposition {
 	// This packet came over a link that doesn't have a define ifID. We have to find it
 	// by srcAddress. We always find one. The internal link matches anything that is not known.
 	src := p.pkt.srcAddr.AddrPort() // POSSIBLY EXPENSIVE CONVERSION
-	session := p.d.underlay.GetLink(src).GetBfdSession()
+	session := p.d.underlay.Link(src).BFDSession()
 	if session == nil {
 		return errorDiscard("error", noBFDSessionFound)
 	}
@@ -1530,12 +1530,12 @@ func (p *scionPacketProcessor) validateTransitUnderlaySrc() disposition {
 	}
 	pktIngressID := p.ingressInterface()
 	ingressLink := p.d.interfaces[pktIngressID]
-	if ingressLink.GetScope() != Sibling {
+	if ingressLink.Scope() != Sibling {
 		// Drop
 		return errorDiscard("error", invalidSrcAddrForTransit)
 	}
 	src, okS := netip.AddrFromSlice(p.pkt.srcAddr.IP)
-	if !(okS && ingressLink.GetRemote().Addr() == src) {
+	if !(okS && ingressLink.Remote().Addr() == src) {
 		// Drop
 		return errorDiscard("error", invalidSrcAddrForTransit)
 	}
@@ -1557,7 +1557,7 @@ func (p *scionPacketProcessor) validateEgressID() disposition {
 	// egress is never the internalInterface
 	// packet coming from internal interface, must go to an external interface
 	// Note that, for now, ingress == 0 is also true for sibling interfaces. That might change.
-	if !found || (p.pkt.ingress == 0 && link.GetScope() == Sibling) {
+	if !found || (p.pkt.ingress == 0 && link.Scope() == Sibling) {
 		errCode := slayers.SCMPCodeUnknownHopFieldEgress
 		if !p.infoField.ConsDir {
 			errCode = slayers.SCMPCodeUnknownHopFieldIngress
@@ -1769,7 +1769,7 @@ func (p *scionPacketProcessor) validateEgressUp() disposition {
 	egressLink := p.d.interfaces[egressID]
 	if !egressLink.IsUp() {
 		log.Debug("SCMP response", "cause", errBFDSessionDown)
-		if egressLink.GetScope() != External {
+		if egressLink.Scope() != External {
 			p.pkt.slowPathRequest = slowPathRequest{
 				scmpType: slayers.SCMPTypeInternalConnectivityDown,
 				code:     0,
@@ -1815,7 +1815,7 @@ func (p *scionPacketProcessor) handleEgressRouterAlert() disposition {
 	if !*alert {
 		return pForward
 	}
-	if p.d.interfaces[p.pkt.egress].GetScope() != External {
+	if p.d.interfaces[p.pkt.egress].Scope() != External {
 		// the egress router is not this one.
 		return pForward
 	}
@@ -1986,7 +1986,7 @@ func (p *scionPacketProcessor) process() disposition {
 	if disp := p.validateEgressUp(); disp != pForward {
 		return disp
 	}
-	if p.d.interfaces[egressID].GetScope() == External {
+	if p.d.interfaces[egressID].Scope() == External {
 		// Not ASTransit in
 		if disp := p.processEgress(); disp != pForward {
 			return disp
@@ -2471,8 +2471,8 @@ func (p *slowPathPacketProcessor) prepareSCMP(
 	}
 	// If the packet is sent to an external router, we need to increment the
 	// path to prepare it for the next hop.
-	// This is an scmp response to pkt, so egress will be pkt.ingress.
-	if p.d.interfaces[p.pkt.ingress].GetScope() == External {
+	// This is an SCMP response to pkt, so egress will be pkt.ingress.
+	if p.d.interfaces[p.pkt.ingress].Scope() == External {
 		infoField := &revPath.InfoFields[revPath.PathMeta.CurrINF]
 		if infoField.ConsDir && !peering {
 			hopField := revPath.HopFields[revPath.PathMeta.CurrHF]
@@ -2598,7 +2598,7 @@ func (p *slowPathPacketProcessor) prepareSCMP(
 	}
 	p.pkt.rawPacket = serBuf.Bytes()
 
-	log.Debug("scmp", "typecode", scmpH.TypeCode)
+	log.Debug("SCMP", "typecode", scmpH.TypeCode)
 	return nil
 }
 
@@ -2726,7 +2726,7 @@ func (d *DataPlane) initMetrics() {
 	d.forwardingMetrics = make(map[uint16]interfaceMetrics)
 	for ifID, link := range d.interfaces {
 		d.forwardingMetrics[ifID] = newInterfaceMetrics(
-			d.Metrics, ifID, d.localIA, link.GetScope(), d.neighborIAs)
+			d.Metrics, ifID, d.localIA, link.Scope(), d.neighborIAs)
 	}
 
 	// Start our custom /proc/pid/stat collector to export iowait time and (in the future) other
diff --git a/router/dataplane_internal_test.go b/router/dataplane_internal_test.go
index f000fd70cc..401e45f1d6 100644
--- a/router/dataplane_internal_test.go
+++ b/router/dataplane_internal_test.go
@@ -93,7 +93,7 @@ func TestReceiver(t *testing.T) {
 	dp.setRunning()
 	dp.initMetrics()
 	go func() {
-		dp.runReceiver(dp.underlay.GetConnections()[netip.AddrPort{}], procCh)
+		dp.runReceiver(dp.underlay.Connections()[netip.AddrPort{}], procCh)
 	}()
 	ptrMap := make(map[uintptr]struct{})
 	for i := 0; i < 21; i++ {
@@ -187,7 +187,7 @@ func TestForwarder(t *testing.T) {
 	dp := prepareDP(ctrl)
 	dp.initPacketPool(64)
 	dp.initQueues(64)
-	conn := dp.underlay.GetConnections()[netip.AddrPort{}]
+	conn := dp.underlay.Connections()[netip.AddrPort{}]
 	intf := dp.interfaces[0]
 	initialPoolSize := len(dp.packetPool)
 	dp.setRunning()
diff --git a/router/underlay.go b/router/underlay.go
index 0bdcbc6a44..25b3086733 100644
--- a/router/underlay.go
+++ b/router/underlay.go
@@ -30,7 +30,7 @@ const (
 )
 
 // Link embodies the router's idea of a point to point connection. A link associates the underlay
-// connection with a bfdSession, a destination address, etc. It also allows the concrete send
+// connection with a BFDSession, a destination address, etc. It also allows the concrete send
 // operation to be delegated to different underlay implementations. The association between
 // link and underlay connection is a channel, on the sending side, and should be a demultiplexer on
 // the receiving side. The demultiplexer must have a src-addr:link map in all cases where links
@@ -41,11 +41,11 @@ const (
 // associate an interface ID with a link. If the interface ID belongs to a sibling router, then
 // the link is a sibling link. If the interface ID is zero, then the link is the internal link.
 type Link interface {
-	GetScope() LinkScope
-	GetBfdSession() BfdSession
+	Scope() LinkScope
+	BFDSession() BFDSession
 	IsUp() bool
-	GetIfID() uint16
-	GetRemote() netip.AddrPort // TODO(multi_underlay): using code will move to underlay.
+	IfID() uint16
+	Remote() netip.AddrPort // TODO(multi_underlay): using code will move to underlay.
 	Send(p *Packet) bool
 	BlockSend(p *Packet)
 }
@@ -64,49 +64,49 @@ type UnderlayProvider interface {
 	// do not need an underlay destination as metadata. Incoming packets have a defined ingress
 	// ifID.
 	NewExternalLink(
-		conn BatchConn, qSize int, bfd BfdSession, remote netip.AddrPort, ifID uint16,
+		conn BatchConn, qSize int, bfd BFDSession, remote netip.AddrPort, ifID uint16,
 	) Link
 
 	// NewSinblingLink returns a link that addresses any number of remote ASes via a single sibling
 	// router. So, it is not given an ifID at creation, but it is given a remote underlay address:
 	// that of the sibling router. Outgoing packets do not need an underlay destination as metadata.
 	// Incoming packets have no defined ingress ifID.
-	NewSiblingLink(qSize int, bfd BfdSession, remote netip.AddrPort) Link
+	NewSiblingLink(qSize int, bfd BFDSession, remote netip.AddrPort) Link
 
 	// NewIternalLink returns a link that addresses any host internal to the enclosing AS, so it is
 	// given neither ifID nor address. Outgoing packets need to have a destination address as
 	// metadata. Incoming packets have no defined ingress ifID.
 	NewInternalLink(conn BatchConn, qSize int) Link
 
-	// GetConnections returns the set of configured distinct connections in the provider.
+	// Connections returns the set of configured distinct connections in the provider.
 	//
 	// TODO(multi_underlay): this exists so most of the receiving code can stay in the main
 	// dataplane code for now. There may be fewer connections than links. For example, right now
 	// all sibling links and the internal link use a shared un-bound connection.
-	GetConnections() map[netip.AddrPort]UnderlayConnection
+	Connections() map[netip.AddrPort]UnderlayConn
 
-	// GetLinks returns the set of configured distinct links in the provider.
+	// Links returns the set of configured distinct links in the provider.
 	//
 	// TODO(multi_underlay): this exists so most of the receiving code can stay in-here for now.
 	// There may be fewer links than ifIDs. For example, all interfaces owned by one given sibling
 	// router are connected via the same link because the remote address is the same.
-	GetLinks() map[netip.AddrPort]Link
+	Links() map[netip.AddrPort]Link
 
-	// GetLink returns a link that matches the given source address. If the address is not that of
+	// Link returns a link that matches the given source address. If the address is not that of
 	// a known link, then the internal link is returned.
 	//
-	// TODO(multi_underlay): This has to exist until incmoing packets are "demuxed" (i.e.
+	// TODO(multi_underlay): This has to exist until incoming packets are "demuxed" (i.e.
 	// matched with a link), on ingest by the underlay. That would imply moving a part of the
 	// runReceiver routine to the underlay. We will do that in the next step.
-	GetLink(netip.AddrPort) Link
+	Link(netip.AddrPort) Link
 }
 
-// UnderlayConnection defines the minimum interface that the router expects from an underlay
+// UnderlayConn defines the minimum interface that the router expects from an underlay
 // connection.
 //
 // TODO(multi_underlay): this will eventually be reduced to nothing at all because the sender
 // receiver tasks will be part of the underlay.
-type UnderlayConnection interface {
+type UnderlayConn interface {
 	Conn() BatchConn
 	Queue() <-chan *Packet
 	Name() string
diff --git a/router/underlayproviders/udpip.go b/router/underlayproviders/udpip.go
index f09253d5e2..25a17c20e0 100644
--- a/router/underlayproviders/udpip.go
+++ b/router/underlayproviders/udpip.go
@@ -15,6 +15,7 @@
 package underlayproviders
 
 import (
+	"maps"
 	"net/netip"
 
 	"github.com/scionproto/scion/router"
@@ -45,25 +46,25 @@ func newProvider() router.UnderlayProvider {
 	}
 }
 
-func (u *provider) GetConnections() map[netip.AddrPort]router.UnderlayConnection {
+func (u *provider) Connections() map[netip.AddrPort]router.UnderlayConn {
 	// A map of interfaces and a map of concrete implementations aren't compatible.
 	// For the same reason, we cannot have the map of concrete implementations as our return type;
-	// it does not satisfy the GetConnections interface (so much for the "don't return
+	// it does not satisfy the Connections() interface (so much for the "don't return
 	// interfaces" rule)... Brilliant, Go.
 	// Since we do not want to store our own things as interfaces, we have to translate.
 	// Good thing it doesn't happen often.
-	m := make(map[netip.AddrPort]router.UnderlayConnection)
+	m := make(map[netip.AddrPort]router.UnderlayConn)
 	for a, c := range u.allConnections {
 		m[a] = c // Yeah that's exactly as stupid as it looks.
 	}
 	return m
 }
 
-func (u *provider) GetLinks() map[netip.AddrPort]router.Link {
-	return u.allLinks
+func (u *provider) Links() map[netip.AddrPort]router.Link {
+	return maps.Clone(u.allLinks)
 }
 
-func (u *provider) GetLink(addr netip.AddrPort) router.Link {
+func (u *provider) Link(addr netip.AddrPort) router.Link {
 	// There is one link for every address. The internal Link catches all.
 	l, found := u.allLinks[addr]
 	if found {
@@ -87,7 +88,7 @@ type udpConnection struct {
 	name  string // for logs. It's more informative than ifID.
 }
 
-// TODO(multi_underlay): The following implements UnderlayConnection so some of the code
+// TODO(multi_underlay): The following implements UnderlayConn so some of the code
 // that needs to interact with it can stay in the main router code. This will be removed in the
 // next step
 
@@ -115,9 +116,9 @@ func (u *udpConnection) IfID() uint16 {
 
 type externalLink struct {
 	queue      chan<- *router.Packet
-	bfdSession router.BfdSession
+	bfdSession router.BFDSession
 	ifID       uint16
-	remote     netip.AddrPort // We keep this only for GetRemote
+	remote     netip.AddrPort // We keep this only for Remote()
 }
 
 // NewExternalLink returns an external link over the UdpIpUnderlay.
@@ -128,7 +129,7 @@ type externalLink struct {
 func (u *provider) NewExternalLink(
 	conn router.BatchConn,
 	qSize int,
-	bfd router.BfdSession,
+	bfd router.BFDSession,
 	remote netip.AddrPort,
 	ifID uint16,
 ) router.Link {
@@ -150,11 +151,11 @@ func (u *provider) NewExternalLink(
 	return l
 }
 
-func (l *externalLink) GetScope() router.LinkScope {
+func (l *externalLink) Scope() router.LinkScope {
 	return router.External
 }
 
-func (l *externalLink) GetBfdSession() router.BfdSession {
+func (l *externalLink) BFDSession() router.BFDSession {
 	return l.bfdSession
 }
 
@@ -162,11 +163,11 @@ func (l *externalLink) IsUp() bool {
 	return l.bfdSession == nil || l.bfdSession.IsUp()
 }
 
-func (l *externalLink) GetIfID() uint16 {
+func (l *externalLink) IfID() uint16 {
 	return l.ifID
 }
 
-func (l *externalLink) GetRemote() netip.AddrPort {
+func (l *externalLink) Remote() netip.AddrPort {
 	return l.remote
 }
 
@@ -185,7 +186,7 @@ func (l *externalLink) BlockSend(p *router.Packet) {
 
 type siblingLink struct {
 	queue      chan<- *router.Packet
-	bfdSession router.BfdSession
+	bfdSession router.BFDSession
 	remote     netip.AddrPort
 }
 
@@ -198,12 +199,12 @@ type siblingLink struct {
 // (something we will get rid of eventually).
 // In the future we will be making one connection per remote address and we might even be able
 // to erase the separation between link and connection for this implementation. Side effect
-// of moving the address:link here: the router does not know if there is an existing link. As
-// a result it has to give us a bfdSession in all cases and we might throw it away (there
-// are no permanent resources attached to it). This will be fixed by moving some bfd related code
+// of moving the address:link map here: the router does not know if there is an existing link. As
+// a result it has to give us a BFDSession in all cases and we might throw it away (there
+// are no permanent resources attached to it). This will be fixed by moving some BFD related code
 // in-here.
 func (u *provider) NewSiblingLink(
-	qSize int, bfd router.BfdSession, remote netip.AddrPort) router.Link {
+	qSize int, bfd router.BFDSession, remote netip.AddrPort) router.Link {
 
 	// There is exactly one sibling link per sibling router address.
 	l, exists := u.allLinks[remote]
@@ -230,11 +231,11 @@ func (u *provider) NewSiblingLink(
 	return s
 }
 
-func (l *siblingLink) GetScope() router.LinkScope {
+func (l *siblingLink) Scope() router.LinkScope {
 	return router.Sibling
 }
 
-func (l *siblingLink) GetBfdSession() router.BfdSession {
+func (l *siblingLink) BFDSession() router.BFDSession {
 	return l.bfdSession
 }
 
@@ -242,11 +243,11 @@ func (l *siblingLink) IsUp() bool {
 	return l.bfdSession == nil || l.bfdSession.IsUp()
 }
 
-func (l *siblingLink) GetIfID() uint16 {
+func (l *siblingLink) IfID() uint16 {
 	return 0
 }
 
-func (l *siblingLink) GetRemote() netip.AddrPort {
+func (l *siblingLink) Remote() netip.AddrPort {
 	return l.remote
 }
 
@@ -293,7 +294,7 @@ func (u *provider) NewInternalLink(conn router.BatchConn, qSize int) router.Link
 	return l
 }
 
-func (l *internalLink) GetScope() router.LinkScope {
+func (l *internalLink) Scope() router.LinkScope {
 	return router.Internal
 }
 
@@ -301,15 +302,15 @@ func (l *internalLink) IsUp() bool {
 	return true
 }
 
-func (l *internalLink) GetBfdSession() router.BfdSession {
+func (l *internalLink) BFDSession() router.BFDSession {
 	return nil
 }
 
-func (l *internalLink) GetIfID() uint16 {
+func (l *internalLink) IfID() uint16 {
 	return 0
 }
 
-func (l *internalLink) GetRemote() netip.AddrPort {
+func (l *internalLink) Remote() netip.AddrPort {
 	return netip.AddrPort{}
 }
 

From 98832415f16d3407baf862a08f4180a029be68bb Mon Sep 17 00:00:00 2001
From: Jean-Christophe Hugly <jice@scion.org>
Date: Fri, 31 Jan 2025 12:51:22 +0100
Subject: [PATCH 08/11] Move newBfdController to bfd/session.go (and renamed
 newBfdSession).

Both the name and the presence of the function in dataplane.go were relics
of the past.
---
 router/bfd/BUILD.bazel |  1 +
 router/bfd/session.go  | 23 +++++++++++++++++++++++
 router/dataplane.go    | 25 ++-----------------------
 3 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/router/bfd/BUILD.bazel b/router/bfd/BUILD.bazel
index 3903335562..1911f20091 100644
--- a/router/bfd/BUILD.bazel
+++ b/router/bfd/BUILD.bazel
@@ -14,6 +14,7 @@ go_library(
     deps = [
         "//pkg/log:go_default_library",
         "//pkg/private/serrors:go_default_library",
+        "//router/control:go_default_library",
         "@com_github_gopacket_gopacket//layers:go_default_library",
         "@com_github_prometheus_client_golang//prometheus:go_default_library",
     ],
diff --git a/router/bfd/session.go b/router/bfd/session.go
index 290a08177e..1637aaf346 100644
--- a/router/bfd/session.go
+++ b/router/bfd/session.go
@@ -16,8 +16,10 @@ package bfd
 
 import (
 	"context"
+	"crypto/rand"
 	"fmt"
 	"math"
+	"math/big"
 	"sync"
 	"time"
 
@@ -25,6 +27,7 @@ import (
 
 	"github.com/scionproto/scion/pkg/log"
 	"github.com/scionproto/scion/pkg/private/serrors"
+	"github.com/scionproto/scion/router/control"
 )
 
 const (
@@ -161,6 +164,26 @@ type Session struct {
 	testLogger log.Logger
 }
 
+func NewBFDSession(ifID uint16, s Sender, cfg control.BFD,
+	metrics Metrics) (*Session, error) {
+
+	// Generate random discriminator. It can't be zero.
+	discInt, err := rand.Int(rand.Reader, big.NewInt(0xfffffffe))
+	if err != nil {
+		return nil, err
+	}
+	disc := layers.BFDDiscriminator(uint32(discInt.Uint64()) + 1)
+	return &Session{
+		Sender:                s,
+		DetectMult:            layers.BFDDetectMultiplier(cfg.DetectMult),
+		DesiredMinTxInterval:  cfg.DesiredMinTxInterval,
+		RequiredMinRxInterval: cfg.RequiredMinRxInterval,
+		LocalDiscriminator:    disc,
+		ReceiveQueueSize:      10,
+		Metrics:               metrics,
+	}, nil
+}
+
 func (s *Session) String() string {
 	return fmt.Sprintf("local_disc %v, remote_disc %v, sender %v",
 		s.LocalDiscriminator, s.getRemoteDiscriminator(), s.Sender)
diff --git a/router/dataplane.go b/router/dataplane.go
index 33ebaf0c6d..fd5acb5017 100644
--- a/router/dataplane.go
+++ b/router/dataplane.go
@@ -24,7 +24,6 @@ import (
 	"errors"
 	"fmt"
 	"hash"
-	"math/big"
 	"net"
 	"net/netip"
 	"sync"
@@ -456,7 +455,7 @@ func (d *DataPlane) newExternalInterfaceBFD(ifID uint16,
 	if err != nil {
 		return nil, err
 	}
-	return newBFDController(ifID, s, cfg, m)
+	return bfd.NewBFDSession(ifID, s, cfg, m)
 }
 
 // getInterfaceState checks if there is a bfd session for the input interfaceID and
@@ -469,26 +468,6 @@ func (d *DataPlane) getInterfaceState(ifID uint16) control.InterfaceState {
 	return control.InterfaceUp
 }
 
-func newBFDController(ifID uint16, s bfd.Sender, cfg control.BFD,
-	metrics bfd.Metrics) (BFDSession, error) {
-
-	// Generate random discriminator. It can't be zero.
-	discInt, err := rand.Int(rand.Reader, big.NewInt(0xfffffffe))
-	if err != nil {
-		return nil, err
-	}
-	disc := layers.BFDDiscriminator(uint32(discInt.Uint64()) + 1)
-	return &bfd.Session{
-		Sender:                s,
-		DetectMult:            layers.BFDDetectMultiplier(cfg.DetectMult),
-		DesiredMinTxInterval:  cfg.DesiredMinTxInterval,
-		RequiredMinRxInterval: cfg.RequiredMinRxInterval,
-		LocalDiscriminator:    disc,
-		ReceiveQueueSize:      10,
-		Metrics:               metrics,
-	}, nil
-}
-
 // AddSvc adds the address for the given service. This can be called multiple
 // times for the same service, with the address added to the list of addresses
 // that provide the service.
@@ -586,7 +565,7 @@ func (d *DataPlane) newNextHopBFD(ifID uint16, src, dst netip.AddrPort, cfg cont
 	if err != nil {
 		return nil, err
 	}
-	return newBFDController(ifID, s, cfg, m)
+	return bfd.NewBFDSession(ifID, s, cfg, m)
 }
 
 func max(a int, b int) int {

From 6003805c8f13151ddc421775f198a5053a07477c Mon Sep 17 00:00:00 2001
From: Jean-Christophe Hugly <jice@scion.org>
Date: Fri, 31 Jan 2025 12:57:39 +0100
Subject: [PATCH 09/11] Rename BlcokSend to SendBlocking.

---
 router/dataplane_internal_test.go | 2 +-
 router/underlay.go                | 2 +-
 router/underlayproviders/udpip.go | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/router/dataplane_internal_test.go b/router/dataplane_internal_test.go
index b267b4d445..c645b7f4d5 100644
--- a/router/dataplane_internal_test.go
+++ b/router/dataplane_internal_test.go
@@ -212,7 +212,7 @@ func TestForwarder(t *testing.T) {
 		// Normal use would be
 		// intf.Send(pkt):
 		// However we want to exclude queue overflow from the test. So we want a blocking send.
-		intf.BlockSend(pkt)
+		intf.SendBlocking(pkt)
 	}
 	select {
 	case <-done:
diff --git a/router/underlay.go b/router/underlay.go
index 25b3086733..93abc8534b 100644
--- a/router/underlay.go
+++ b/router/underlay.go
@@ -47,7 +47,7 @@ type Link interface {
 	IfID() uint16
 	Remote() netip.AddrPort // TODO(multi_underlay): using code will move to underlay.
 	Send(p *Packet) bool
-	BlockSend(p *Packet)
+	SendBlocking(p *Packet)
 }
 
 // A provider of connectivity over some underlay implementation
diff --git a/router/underlayproviders/udpip.go b/router/underlayproviders/udpip.go
index 25a17c20e0..3b9c37a050 100644
--- a/router/underlayproviders/udpip.go
+++ b/router/underlayproviders/udpip.go
@@ -180,7 +180,7 @@ func (l *externalLink) Send(p *router.Packet) bool {
 	return true
 }
 
-func (l *externalLink) BlockSend(p *router.Packet) {
+func (l *externalLink) SendBlocking(p *router.Packet) {
 	l.queue <- p
 }
 
@@ -263,7 +263,7 @@ func (l *siblingLink) Send(p *router.Packet) bool {
 	return true
 }
 
-func (l *siblingLink) BlockSend(p *router.Packet) {
+func (l *siblingLink) SendBlocking(p *router.Packet) {
 	// We use an unbound connection but we offer a connected-oriented service. So, we need to
 	// supply the packet's destination address.
 	router.UpdateNetAddrFromAddrPort(p.DstAddr, l.remote)
@@ -325,6 +325,6 @@ func (l *internalLink) Send(p *router.Packet) bool {
 }
 
 // The packet's destination is already in the packet's meta-data.
-func (l *internalLink) BlockSend(p *router.Packet) {
+func (l *internalLink) SendBlocking(p *router.Packet) {
 	l.queue <- p
 }

From 69635ef4ab2c6823394e8280dd3db8acff571755 Mon Sep 17 00:00:00 2001
From: Jean-Christophe Hugly <jice@scion.org>
Date: Fri, 31 Jan 2025 13:25:47 +0100
Subject: [PATCH 10/11] Fix docstrings for UnderlayConn interface and other
 comments.

---
 router/underlay.go                | 4 ++++
 router/underlayproviders/udpip.go | 8 ++------
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/router/underlay.go b/router/underlay.go
index 93abc8534b..e073a49689 100644
--- a/router/underlay.go
+++ b/router/underlay.go
@@ -107,8 +107,12 @@ type UnderlayProvider interface {
 // TODO(multi_underlay): this will eventually be reduced to nothing at all because the sender
 // receiver tasks will be part of the underlay.
 type UnderlayConn interface {
+	// Conn returns the BatchConn associated with the connection.
 	Conn() BatchConn
+	// Queue returns the channel associated with the connection.
 	Queue() <-chan *Packet
+	// Name returns the name (for logging) associated with the connection.
 	Name() string
+	// IfID returns the IfID associated with the connection.
 	IfID() uint16
 }
diff --git a/router/underlayproviders/udpip.go b/router/underlayproviders/udpip.go
index 3b9c37a050..72e78717ed 100644
--- a/router/underlayproviders/udpip.go
+++ b/router/underlayproviders/udpip.go
@@ -92,22 +92,18 @@ type udpConnection struct {
 // that needs to interact with it can stay in the main router code. This will be removed in the
 // next step
 
-// Name returns the name (for logging) associated with a connection.
 func (u *udpConnection) Conn() router.BatchConn {
 	return u.conn
 }
 
-// Name returns the name (for logging) associated with a connection.
 func (u *udpConnection) Queue() <-chan *router.Packet {
 	return u.queue
 }
 
-// Name returns the name (for logging) associated with a connection.
 func (u *udpConnection) Name() string {
 	return u.name
 }
 
-// Name returns the name (for logging) associated with a connection.
 func (u *udpConnection) IfID() uint16 {
 	return u.ifID
 }
@@ -252,7 +248,7 @@ func (l *siblingLink) Remote() netip.AddrPort {
 }
 
 func (l *siblingLink) Send(p *router.Packet) bool {
-	// We use an unbound connection but we offer a connected-oriented service. So, we need to
+	// We use an unbound connection but we offer a connection-oriented service. So, we need to
 	// supply the packet's destination address.
 	router.UpdateNetAddrFromAddrPort(p.DstAddr, l.remote)
 	select {
@@ -264,7 +260,7 @@ func (l *siblingLink) Send(p *router.Packet) bool {
 }
 
 func (l *siblingLink) SendBlocking(p *router.Packet) {
-	// We use an unbound connection but we offer a connected-oriented service. So, we need to
+	// We use an unbound connection but we offer a connection-oriented service. So, we need to
 	// supply the packet's destination address.
 	router.UpdateNetAddrFromAddrPort(p.DstAddr, l.remote)
 	l.queue <- p

From 7edbdf79617681726b42a46d6d4b818e9ad5e194 Mon Sep 17 00:00:00 2001
From: Jean-Christophe Hugly <jice@scion.org>
Date: Mon, 3 Feb 2025 17:31:55 +0100
Subject: [PATCH 11/11] Fix name and signature of NewBFDSession.

New name NewSession() since BFD in the name was redundant.
Removed ifID argument, which wasn't needed.
Added a doc string.
---
 router/bfd/session.go | 11 +++++++++--
 router/dataplane.go   |  4 ++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/router/bfd/session.go b/router/bfd/session.go
index 1637aaf346..4fd850968e 100644
--- a/router/bfd/session.go
+++ b/router/bfd/session.go
@@ -164,8 +164,15 @@ type Session struct {
 	testLogger log.Logger
 }
 
-func NewBFDSession(ifID uint16, s Sender, cfg control.BFD,
-	metrics Metrics) (*Session, error) {
+// NewSession returns a new BFD session, configured as specified and updating the
+// given metrics. BFD packets are transmitted via the given Sender. Up to 10 incoming BFD packets
+// per session can be queued waiting for processing; excess traffic will be blocked.
+// A random discriminator is generated automatically. This can be used by the recipient to route
+// packets to the correct session.
+//
+// TODO(jiceatscion): blocking incoming traffic (*all of it*) when the BFD queue is full is
+// probably the wrong thing to do, but this is what we have been doing so far.
+func NewSession(s Sender, cfg control.BFD, metrics Metrics) (*Session, error) {
 
 	// Generate random discriminator. It can't be zero.
 	discInt, err := rand.Int(rand.Reader, big.NewInt(0xfffffffe))
diff --git a/router/dataplane.go b/router/dataplane.go
index fd5acb5017..f3cb2a1df9 100644
--- a/router/dataplane.go
+++ b/router/dataplane.go
@@ -455,7 +455,7 @@ func (d *DataPlane) newExternalInterfaceBFD(ifID uint16,
 	if err != nil {
 		return nil, err
 	}
-	return bfd.NewBFDSession(ifID, s, cfg, m)
+	return bfd.NewSession(s, cfg, m)
 }
 
 // getInterfaceState checks if there is a bfd session for the input interfaceID and
@@ -565,7 +565,7 @@ func (d *DataPlane) newNextHopBFD(ifID uint16, src, dst netip.AddrPort, cfg cont
 	if err != nil {
 		return nil, err
 	}
-	return bfd.NewBFDSession(ifID, s, cfg, m)
+	return bfd.NewSession(s, cfg, m)
 }
 
 func max(a int, b int) int {