implement reviewer's suggestions.

router/#connector.go#

@@ -0,0 +1,250 @@
+// Copyright 2020 Anapaya Systems
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package router
+
+import (
+	"net/netip"
+	"sync"
+
+	"github.com/scionproto/scion/pkg/addr"
+	"github.com/scionproto/scion/pkg/log"
+	"github.com/scionproto/scion/pkg/private/serrors"
+	"github.com/scionproto/scion/pkg/segment/iface"
+	"github.com/scionproto/scion/private/underlay/conn"
+	"github.com/scionproto/scion/router/config"
+	"github.com/scionproto/scion/router/control"
+)
+
+// Connector implements the Dataplane API of the router control process. It sets
+// up connections for the DataPlane.
+type Connector struct {
+	DataPlane DataPlane
+
+	ia                 addr.IA
+	mtx                sync.Mutex
+	internalInterfaces []control.InternalInterface
+	externalInterfaces map[uint16]control.ExternalInterface
+	siblingInterfaces  map[uint16]control.SiblingInterface
+	internalConnection conn.Conn
+
+	ReceiveBufferSize   int
+	SendBufferSize      int
+	BFD                 config.BFD
+	DispatchedPortStart *int
+	DispatchedPortEnd   *int
+}
+
+var errMultiIA = serrors.New("different IA not allowed")
+
+// CreateIACtx creates the context for ISD-AS.
+func (c *Connector) CreateIACtx(ia addr.IA) error {
+	c.mtx.Lock()
+	defer c.mtx.Unlock()
+	log.Debug("CreateIACtx", "isd_as", ia)
+	if !c.ia.IsZero() {
+		return serrors.JoinNoStack(errMultiIA, nil, "current", c.ia, "new", ia)
+	}
+	c.ia = ia
+	return c.DataPlane.SetIA(ia)
+}
+
+// AddInternalInterface adds the internal interface.
+func (c *Connector) AddInternalInterface(ia addr.IA, local netip.AddrPort) error {
+	c.mtx.Lock()
+	defer c.mtx.Unlock()
+	log.Debug("Adding internal interface", "isd_as", ia, "local", local)
+	if !c.ia.Equal(ia) {
+		return serrors.JoinNoStack(errMultiIA, nil, "current", c.ia, "new", ia)
+	}
+	connection, err := conn.New(local, netip.AddrPort{},
+		&conn.Config{ReceiveBufferSize: c.ReceiveBufferSize, SendBufferSize: c.SendBufferSize})
+	if err != nil {
+		return err
+	}
+	c.internalConnection = connection
+	c.internalInterfaces = append(c.internalInterfaces, control.InternalInterface{
+		IA:   ia,
+		Addr: local,
+	})
+	return c.DataPlane.AddInternalInterface(connection, local.Addr())
+}
+
+// AddExternalInterface adds a link between the local and remote address.
+// TODO(jiceatscion): do not depend on AddInternalInterface having been called first.
+func (c *Connector) AddExternalInterface(localIfID iface.ID, link control.LinkInfo,
+	owned bool) error {
+
+	c.mtx.Lock()
+	defer c.mtx.Unlock()
+	intf := uint16(localIfID)
+	log.Debug("Adding external interface", "interface", localIfID,
+		"local_isd_as", link.Local.IA, "local_addr", link.Local.Addr,
+		"remote_isd_as", link.Remote.IA, "remote_addr", link.Remote.Addr,
+		"owned", owned,
+		"link_bfd_configured", link.BFD.Disable != nil,
+		"link_bfd_enabled", link.BFD.Disable == nil || !*link.BFD.Disable,
+		"dataplane_bfd_enabled", !c.BFD.Disable)
+
+	if !c.ia.Equal(link.Local.IA) {
+		return serrors.JoinNoStack(errMultiIA, nil, "current", c.ia, "new", link.Local.IA)
+	}
+	if err := c.DataPlane.AddLinkType(intf, link.LinkTo); err != nil {
+		return serrors.Wrap("adding link type", err, "if_id", localIfID)
+	}
+	if err := c.DataPlane.AddNeighborIA(intf, link.Remote.IA); err != nil {
+		return serrors.Wrap("adding neighboring IA", err, "if_id", localIfID)
+	}
+
+	link.BFD = c.applyBFDDefaults(link.BFD)
+	if owned {
+		if len(c.externalInterfaces) == 0 {
+			c.externalInterfaces = make(map[uint16]control.ExternalInterface)
+		}
+		c.externalInterfaces[intf] = control.ExternalInterface{
+			IfID:  intf,
+			Link:  link,
+			State: control.InterfaceDown,
+		}
+		connection, err := conn.New(link.Local.Addr, link.Remote.Addr,
+			&conn.Config{ReceiveBufferSize: c.ReceiveBufferSize, SendBufferSize: c.SendBufferSize})
+		if err != nil {
+			return err
+		}
+
+		return c.DataPlane.AddExternalInterface(intf, connection, link.Local, link.Remote, link.BFD,
+			owned)
+	}
+
+	if len(c.siblingInterfaces) == 0 {
+		c.siblingInterfaces = make(map[uint16]control.SiblingInterface)
+	}
+	c.siblingInterfaces[intf] = control.SiblingInterface{
+		IfID:              intf,
+		InternalInterface: link.Remote.Addr,
+		Relationship:      link.LinkTo,
+		MTU:               link.MTU,
+		NeighborIA:        link.Remote.IA,
+		State:             control.InterfaceDown,
+	}
+	return c.DataPlane.AddNextHop(intf, link.Local.Addr, link.Remote.Addr,
+		link.BFD, link.Instance)
+}
+
+// AddSvc adds the service address for the given ISD-AS.
+func (c *Connector) AddSvc(ia addr.IA, svc addr.SVC, a netip.AddrPort) error {
+
+	c.mtx.Lock()
+	defer c.mtx.Unlock()
+	log.Debug("Adding service", "isd_as", ia, "svc", svc, "address", a)
+	if !c.ia.Equal(ia) {
+		return serrors.JoinNoStack(errMultiIA, nil, "current", c.ia, "new", a)
+	}
+	return c.DataPlane.AddSvc(svc, a)
+}
+
+// DelSvc deletes the service entry for the given ISD-AS and IP pair.
+func (c *Connector) DelSvc(ia addr.IA, svc addr.SVC, a netip.AddrPort) error {
+	c.mtx.Lock()
+	defer c.mtx.Unlock()
+	log.Debug("Deleting service", "isd_as", ia, "svc", svc, "address", a)
+	if !c.ia.Equal(ia) {
+		return serrors.JoinNoStack(errMultiIA, nil, "current", c.ia, "new", a)
+	}
+	return c.DataPlane.DelSvc(svc, a)
+}
+
+// SetKey sets the key for the given ISD-AS at the given index.
+func (c *Connector) SetKey(ia addr.IA, index int, key []byte) error {
+	c.mtx.Lock()
+	defer c.mtx.Unlock()
+	log.Debug("Setting key", "isd_as", ia, "index", index)
+	if !c.ia.Equal(ia) {
+		return serrors.JoinNoStack(errMultiIA, nil, "current", c.ia, "new", ia)
+	}
+	if index != 0 {
+		return serrors.New("currently only index 0 key is supported")
+	}
+	return c.DataPlane.SetKey(key)
+}
+
+func (c *Connector) ListInternalInterfaces() ([]control.InternalInterface, error) {
+	c.mtx.Lock()
+	defer c.mtx.Unlock()
+
+	if len(c.internalInterfaces) == 0 {
+		return nil, serrors.New("internal interface is not set")
+	}
+	return c.internalInterfaces, nil
+}
+
+func (c *Connector) ListExternalInterfaces() ([]control.ExternalInterface, error) {
+	c.mtx.Lock()
+	defer c.mtx.Unlock()
+
+	externalInterfaceList := make([]control.ExternalInterface, 0, len(c.externalInterfaces))
+	for _, externalInterface := range c.externalInterfaces {
+		externalInterface.State = c.DataPlane.getInterfaceState(externalInterface.IfID)
+		externalInterfaceList = append(externalInterfaceList, externalInterface)
+	}
+	return externalInterfaceList, nil
+}
+
+func (c *Connector) ListSiblingInterfaces() ([]control.SiblingInterface, error) {
+	c.mtx.Lock()
+	defer c.mtx.Unlock()
+
+	siblingInterfaceList := make([]control.SiblingInterface, 0, len(c.siblingInterfaces))
+	for _, siblingInterface := range c.siblingInterfaces {
+		siblingInterface.State = c.DataPlane.getInterfaceState(siblingInterface.IfID)
+		siblingInterfaceList = append(siblingInterfaceList, siblingInterface)
+	}
+	return siblingInterfaceList, nil
+}
+
+// applyBFDDefaults updates the given cfg object with the global default BFD settings.
+// Link-specific settings, if configured, remain unchanged.  IMPORTANT: cfg.Disable isn't a boolean
+// but a pointer to boolean, allowing a simple representation of the unconfigured state: nil. This
+// means that using a cfg object that hasn't been processed by this function may lead to a NPE.
+// In particular, "control.BFD{}" is invalid.
+func (c *Connector) applyBFDDefaults(cfg control.BFD) control.BFD {
+
+	if cfg.Disable == nil {
+		disable := c.BFD.Disable
+		cfg.Disable = &disable
+	}
+	if cfg.DetectMult == 0 {
+		cfg.DetectMult = c.BFD.DetectMult
+	}
+	if cfg.DesiredMinTxInterval == 0 {
+		cfg.DesiredMinTxInterval = c.BFD.DesiredMinTxInterval.Duration
+	}
+	if cfg.RequiredMinRxInterval == 0 {
+		cfg.RequiredMinRxInterval = c.BFD.RequiredMinRxInterval.Duration
+	}
+	return cfg
+}
+
+func (c *Connector) SetPortRange(start, end uint16) {
+	c.mtx.Lock()
+	defer c.mtx.Unlock()
+	if c.DispatchedPortStart != nil {
+		start = uint16(*c.DispatchedPortStart)
+	}
+	if c.DispatchedPortEnd != nil {
+		end = uint16(*c.DispatchedPortEnd)
+	}
+	log.Debug("Endhost port range configuration", "startPort", start, "endPort", end)
+	c.DataPlane.SetPortRange(start, end)
+}

router/BUILD.bazel

@@ -7,7 +7,7 @@ go_library(
         "dataplane.go",
         "fnv1aCheap.go",
         "metrics.go",
-        "serializeProxy.go",
+        "serialize_proxy.go",
         "svc.go",
     ],
     importpath = "github.com/scionproto/scion/router",

router/dataplane.go

@@ -2295,8 +2295,9 @@ func decodeSCMP(scmp *slayers.SCMP) ([]gopacket.SerializableLayer, error) {
 // afterwards (and the preceding headers, if any).
 func updateSCIONLayer(rawPkt []byte, s slayers.SCION) error {
 	payloadOffset := len(rawPkt) - len(s.LayerPayload())
-	serBuf := newSerializeProxy(rawPkt)
-	serBuf.clear(payloadOffset) // Prepends will go just before payload. (Appends will wreck it)
+
+	// Prepends must go just before payload. (and any Append will wreck it)
+	serBuf := newSerializeProxyOffset(rawPkt, payloadOffset)
 	return s.SerializeTo(&serBuf, gopacket.SerializeOptions{})
 }
 

router/serializeProxy.go → router/serialize_proxy.go

@@ -25,26 +25,38 @@ import (
 // panic. It is designed to be a local variable, so New() returns a value. The entire buffer
 // underpinning the given slice may be used; that is, from the start up to the remaining capacity.
 type serializeProxy struct {
-	initStart int // Initial starting point (where the first prepend or append occurs)
-	data      []byte
-	start     int // The slice's offset can't change (irreversible). So keep track separately.
-	layers    []gopacket.LayerType
+
+	// The slice's offset can't be changed as that is irreversible.
+	// So we keep track separately from the slice.
+
+	restart int // the value to reset start to during Clear().
+	start   int // current start of the useful data in the buffer.
+	data    []byte
+	layers  []gopacket.LayerType
 }
 
-// Initializes a new serializeProxy. The initial prepend/append point is set to the end of the
-// buffer in anticipation of AppendBytes never being used. This can be changed by calling clear()
-// if AppendBytes is to be used.
+// newSerializeProxy returns a new serializeProxy. The initial prepend/append point is set to the
+// end of the buffer in anticipation of AppendBytes never being used. The prepend/append point can
+// be changed when calling clear().
 func newSerializeProxy(buf []byte) serializeProxy {
+	return newSerializeProxyOffset(buf, cap(buf))
+}
+
+// newSerializeProxyOffset returns a new serializeProxy. The initial prepend/append point is set to
+// the given start value. This has the same effect as calling clear(statr).
+func newSerializeProxyOffset(buf []byte, start int) serializeProxy {
 	serBuf := serializeProxy{
 		data: buf,
 	}
-	serBuf.clear(cap(buf))
+	serBuf.clear(start)
 	return serBuf
 }
 
 // Resets the buffer to empty and sets the initial prepend/append point to the given position.
+// The next prepend will claim an area ending with index newStart - 1. The next append will claim an
+// area starting with index newStart.
 func (s *serializeProxy) clear(newStart int) {
-	s.initStart = newStart
+	s.restart = newStart
 	s.start = newStart
 	s.data = s.data[:newStart]
 	s.layers = s.layers[:0]
@@ -53,7 +65,7 @@ func (s *serializeProxy) clear(newStart int) {
 // Implements serializeBuffer.Clear(). This implementation never returns an error.
 // The initial prepend/append point is reset to that which was set by the last call to clear().
 func (s *serializeProxy) Clear() error {
-	s.clear(s.initStart)
+	s.clear(s.restart)
 	return nil
 }