Skip to content

Commit

Permalink
snet: replace PathInterface interface with a struct (#3879)
Browse files Browse the repository at this point in the history
Use a simple struct directly instead of an interface.
Contrary to what was implied by code comments, this does not appear to
introduce any problematic dependencies.
  • Loading branch information
matzf authored Sep 30, 2020
1 parent 0d449bc commit 962ca85
Show file tree
Hide file tree
Showing 26 changed files with 113 additions and 279 deletions.
20 changes: 5 additions & 15 deletions go/cs/segutil/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"fmt"
"strings"

"github.com/scionproto/scion/go/lib/addr"
"github.com/scionproto/scion/go/lib/common"
"github.com/scionproto/scion/go/lib/ctrl/seg"
"github.com/scionproto/scion/go/lib/pathpol"
Expand Down Expand Up @@ -58,7 +57,7 @@ func segsToPs(segs seg.Segments, dir Direction) pathpol.PathSet {
ps := make(pathpol.PathSet, len(segs))
for _, seg := range segs {
sw := wrap(seg, dir)
ps[sw.Fingerprint()] = sw
ps[sw.key] = sw
}
return ps
}
Expand Down Expand Up @@ -92,9 +91,9 @@ func wrap(seg *seg.PathSegment, dir Direction) segWrap {

for _, ifid := range interfaces {
if ifid != 0 {
intfs = append(intfs, pathInterface{
ia: asEntry.Local,
ifid: common.IFIDType(ifid),
intfs = append(intfs, snet.PathInterface{
IA: asEntry.Local,
ID: common.IFIDType(ifid),
})
keyParts = append(keyParts, fmt.Sprintf("%s#%d", asEntry.Local, ifid))
}
Expand All @@ -113,13 +112,4 @@ func wrap(seg *seg.PathSegment, dir Direction) segWrap {
}
}

func (s segWrap) Interfaces() []snet.PathInterface { return s.intfs }
func (s segWrap) Fingerprint() snet.PathFingerprint { return s.key }

type pathInterface struct {
ia addr.IA
ifid common.IFIDType
}

func (i pathInterface) IA() addr.IA { return i.ia }
func (i pathInterface) ID() common.IFIDType { return i.ifid }
func (s segWrap) Interfaces() []snet.PathInterface { return s.intfs }
20 changes: 9 additions & 11 deletions go/cs/segutil/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,28 +84,26 @@ func (r *Router) translate(comb *combinator.Path, dst addr.IA) (path, error) {
} else {
sp = spath.NewV2(buf.Bytes(), false)
}
nextHop, ok := r.Pather.TopoProvider.Get().UnderlayNextHop(comb.Interfaces[0].IfID)
nextHop, ok := r.Pather.TopoProvider.Get().UnderlayNextHop(comb.Interfaces[0].ID)
if !ok {
return path{}, serrors.New("Unable to find first-hop BR for path",
"ifid", comb.Interfaces[0].IfID)
"ifid", comb.Interfaces[0].ID)
}
p := path{
interfaces: make([]pathInterface, 0, len(comb.Interfaces)),
interfaces: make([]snet.PathInterface, len(comb.Interfaces)),
underlay: nextHop,
spath: sp,
metadata: pathMetadata{
mtu: comb.Mtu,
expiry: comb.ComputeExpTime(),
},
}
for _, intf := range comb.Interfaces {
p.interfaces = append(p.interfaces, pathInterface{ia: intf.IA(), ifid: intf.ID()})
}
copy(p.interfaces, comb.Interfaces)
return p, nil
}

type path struct {
interfaces []pathInterface
interfaces []snet.PathInterface
underlay *net.UDPAddr
spath *spath.Path
dst addr.IA
Expand Down Expand Up @@ -150,7 +148,7 @@ func (p path) Destination() addr.IA {
if len(p.interfaces) == 0 {
return p.dst
}
return p.interfaces[len(p.interfaces)-1].IA()
return p.interfaces[len(p.interfaces)-1].IA
}

func (p path) Metadata() snet.PathMetadata {
Expand Down Expand Up @@ -178,14 +176,14 @@ func (p path) fmtInterfaces() []string {
return hops
}
intf := p.interfaces[0]
hops = append(hops, fmt.Sprintf("%s %d", intf.IA(), intf.ID()))
hops = append(hops, fmt.Sprintf("%s %d", intf.IA, intf.ID))
for i := 1; i < len(p.interfaces)-1; i += 2 {
inIntf := p.interfaces[i]
outIntf := p.interfaces[i+1]
hops = append(hops, fmt.Sprintf("%d %s %d", inIntf.ID(), inIntf.IA(), outIntf.ID()))
hops = append(hops, fmt.Sprintf("%d %s %d", inIntf.ID, inIntf.IA, outIntf.ID))
}
intf = p.interfaces[len(p.interfaces)-1]
hops = append(hops, fmt.Sprintf("%d %s", intf.ID(), intf.IA()))
hops = append(hops, fmt.Sprintf("%d %s", intf.ID, intf.IA))
return hops
}

Expand Down
2 changes: 1 addition & 1 deletion go/lib/infra/modules/combinator/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ go_library(
"//go/lib/addr:go_default_library",
"//go/lib/common:go_default_library",
"//go/lib/ctrl/seg:go_default_library",
"//go/lib/sciond:go_default_library",
"//go/lib/serrors:go_default_library",
"//go/lib/slayers/path:go_default_library",
"//go/lib/slayers/path/scion:go_default_library",
"//go/lib/snet:go_default_library",
"//go/lib/spath:go_default_library",
"//go/lib/util:go_default_library",
"//go/proto:go_default_library",
Expand Down
12 changes: 6 additions & 6 deletions go/lib/infra/modules/combinator/combinator.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ import (
"github.com/scionproto/scion/go/lib/addr"
"github.com/scionproto/scion/go/lib/common"
"github.com/scionproto/scion/go/lib/ctrl/seg"
"github.com/scionproto/scion/go/lib/sciond"
"github.com/scionproto/scion/go/lib/serrors"
"github.com/scionproto/scion/go/lib/slayers/path"
"github.com/scionproto/scion/go/lib/slayers/path/scion"
"github.com/scionproto/scion/go/lib/snet"
"github.com/scionproto/scion/go/lib/spath"
"github.com/scionproto/scion/go/lib/util"
"github.com/scionproto/scion/go/proto"
Expand Down Expand Up @@ -95,7 +95,7 @@ type Path struct {
Segments []*Segment
Weight int
Mtu uint16
Interfaces []sciond.PathInterface
Interfaces []snet.PathInterface
StaticInfo *PathMetadata

HeaderV2 bool
Expand Down Expand Up @@ -124,7 +124,7 @@ func (p *Path) reverseDownSegment() {
}

func (p *Path) aggregateInterfaces() {
p.Interfaces = []sciond.PathInterface{}
p.Interfaces = []snet.PathInterface{}
for _, segment := range p.Segments {
p.Interfaces = append(p.Interfaces, segment.Interfaces...)
}
Expand Down Expand Up @@ -228,7 +228,7 @@ type Segment struct {
InfoField *InfoField
HopFields []*HopField
Type proto.PathSegType
Interfaces []sciond.PathInterface
Interfaces []snet.PathInterface
}

// initInfoFieldFrom copies the info field in pathSegment, and sets it as the
Expand Down Expand Up @@ -336,8 +336,8 @@ func FilterLongPaths(paths []*Path) []*Path {
long := false
iaCounts := make(map[addr.IA]int)
for _, iface := range path.Interfaces {
iaCounts[iface.IA()]++
if iaCounts[iface.IA()] > 2 {
iaCounts[iface.IA]++
if iaCounts[iface.IA] > 2 {
long = true
break
}
Expand Down
24 changes: 12 additions & 12 deletions go/lib/infra/modules/combinator/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"github.com/scionproto/scion/go/lib/addr"
"github.com/scionproto/scion/go/lib/common"
"github.com/scionproto/scion/go/lib/ctrl/seg"
"github.com/scionproto/scion/go/lib/sciond"
"github.com/scionproto/scion/go/lib/snet"
"github.com/scionproto/scion/go/proto"
)

Expand Down Expand Up @@ -288,7 +288,7 @@ func (solution *PathSolution) getFwdPathMetadata() *Path {
}
for _, solEdge := range solution.edges {
var hops []*HopField
var intfs []sciond.PathInterface
var intfs []snet.PathInterface

// Go through each ASEntry, starting from the last one, until we
// find a shortcut (which can be 0, meaning the end of the segment).
Expand Down Expand Up @@ -327,16 +327,16 @@ func (solution *PathSolution) getFwdPathMetadata() *Path {
// Segment is traversed in reverse construction direction.
// Only include non-zero interfaces.
if hopField.ConsEgress != 0 {
intfs = append(intfs, sciond.PathInterface{
RawIsdas: asEntry.Local.IAInt(),
IfID: common.IFIDType(hopField.ConsEgress),
intfs = append(intfs, snet.PathInterface{
IA: asEntry.Local,
ID: common.IFIDType(hopField.ConsEgress),
})
}
// In a non-peer shortcut the AS is not traversed completely.
if hopField.ConsIngress != 0 && (!isShortcut || isPeer) {
intfs = append(intfs, sciond.PathInterface{
RawIsdas: asEntry.Local.IAInt(),
IfID: common.IFIDType(hopField.ConsIngress),
intfs = append(intfs, snet.PathInterface{
IA: asEntry.Local,
ID: common.IFIDType(hopField.ConsIngress),
})
}
hops = append(hops, hopField)
Expand Down Expand Up @@ -549,15 +549,15 @@ func minUint16(x, y uint16) uint16 {
return y
}

func getPathInterfaces(ia addr.IA, inIFID, outIFID common.IFIDType) []sciond.PathInterface {
var result []sciond.PathInterface
func getPathInterfaces(ia addr.IA, inIFID, outIFID common.IFIDType) []snet.PathInterface {
var result []snet.PathInterface
if inIFID != 0 {
result = append(result,
sciond.PathInterface{RawIsdas: ia.IAInt(), IfID: inIFID})
snet.PathInterface{IA: ia, ID: inIFID})
}
if outIFID != 0 {
result = append(result,
sciond.PathInterface{RawIsdas: ia.IAInt(), IfID: outIFID})
snet.PathInterface{IA: ia, ID: outIFID})
}
return result
}
Expand Down
2 changes: 1 addition & 1 deletion go/lib/infra/modules/segfetcher/pather.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func (p *Pather) filterRevoked(ctx context.Context,
for _, iface := range path.Interfaces {
// cache automatically expires outdated revocations every second,
// so a cache hit implies revocation is still active.
revs, err := p.RevCache.Get(ctx, revcache.SingleKey(iface.IA(), iface.IfID))
revs, err := p.RevCache.Get(ctx, revcache.SingleKey(iface.IA, iface.ID))
if err != nil {
logger.Error("[segfetcher.Pather] Failed to get revocation", "err", err)
// continue, the client might still get some usable paths like this.
Expand Down
9 changes: 4 additions & 5 deletions go/lib/pathmgr/pathmgr.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,7 @@ func (r *resolver) Revoke(ctx context.Context, sRevInfo *path_mgmt.SignedRevInfo
}
// Each watcher contains a cache; purge paths matched by the revocation
// immediately from each cache.
pi := sciond.PathInterface{RawIsdas: revInfo.IA().IAInt(),
IfID: common.IFIDType(revInfo.IfID)}
pi := snet.PathInterface{IA: revInfo.IA(), ID: revInfo.IfID}
f := func(w *WatchRunner) {
pathsBeforeRev := w.sp.Load().APS
pathsAfterRev := dropRevoked(pathsBeforeRev, pi)
Expand All @@ -242,7 +241,7 @@ func (r *resolver) logger(ctx context.Context) log.Logger {
return log.FromCtx(ctx).New("lib", "PathResolver")
}

func dropRevoked(aps spathmeta.AppPathSet, pi sciond.PathInterface) spathmeta.AppPathSet {
func dropRevoked(aps spathmeta.AppPathSet, pi snet.PathInterface) spathmeta.AppPathSet {
other := make(spathmeta.AppPathSet)
for key, path := range aps {
if !matches(path, pi) {
Expand All @@ -252,9 +251,9 @@ func dropRevoked(aps spathmeta.AppPathSet, pi sciond.PathInterface) spathmeta.Ap
return other
}

func matches(path snet.Path, predicatePI sciond.PathInterface) bool {
func matches(path snet.Path, predicatePI snet.PathInterface) bool {
for _, pi := range path.Interfaces() {
if pi.IA().Equal(predicatePI.IA()) && pi.ID() == predicatePI.ID() {
if pi.IA.Equal(predicatePI.IA) && pi.ID == predicatePI.ID {
return true
}
}
Expand Down
2 changes: 1 addition & 1 deletion go/lib/pathmgr/pathmgr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ func TestWatchFilter(t *testing.T) {
replySet := make(pathpol.PathSet)
for key, v := range ps {
for _, intf := range v.Interfaces() {
if intf.IA().Equal(src) && intf.ID() == 105 {
if intf.IA.Equal(src) && intf.ID == 105 {
replySet[key] = v
break
}
Expand Down
15 changes: 3 additions & 12 deletions go/lib/pathmgr/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (

"github.com/golang/mock/gomock"

"github.com/scionproto/scion/go/lib/addr"
"github.com/scionproto/scion/go/lib/common"
"github.com/scionproto/scion/go/lib/snet"
"github.com/scionproto/scion/go/lib/snet/mock_snet"
Expand All @@ -46,9 +45,9 @@ func createPath(t testing.TB, ctrl *gomock.Controller, desc string) snet.Path {
if len(tokens) != 2 {
t.Fatalf("Invalid path description: %s", desc)
}
interfaces = append(interfaces, intf{
ia: xtest.MustParseIA(tokens[0]),
id: mustIfID(t, tokens[1]),
interfaces = append(interfaces, snet.PathInterface{
IA: xtest.MustParseIA(tokens[0]),
ID: mustIfID(t, tokens[1]),
})
}
path.EXPECT().Interfaces().Return(interfaces).AnyTimes()
Expand All @@ -62,11 +61,3 @@ func mustIfID(t testing.TB, s string) common.IFIDType {
}
return common.IFIDType(ifID)
}

type intf struct {
ia addr.IA
id common.IFIDType
}

func (i intf) IA() addr.IA { return i.ia }
func (i intf) ID() common.IFIDType { return i.id }
6 changes: 3 additions & 3 deletions go/lib/pathpol/hop_pred.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,10 @@ func HopPredicateFromString(str string) (*HopPredicate, error) {
// pathIFMatch takes a PathInterface and a bool indicating if the ingress
// interface needs to be matching. It returns true if the HopPredicate matches the PathInterface
func (hp *HopPredicate) pathIFMatch(pi snet.PathInterface, in bool) bool {
if hp.ISD != 0 && pi.IA().I != hp.ISD {
if hp.ISD != 0 && pi.IA.I != hp.ISD {
return false
}
if hp.AS != 0 && pi.IA().A != hp.AS {
if hp.AS != 0 && pi.IA.A != hp.AS {
return false
}
ifInd := 0
Expand All @@ -101,7 +101,7 @@ func (hp *HopPredicate) pathIFMatch(pi snet.PathInterface, in bool) bool {
if len(hp.IfIDs) == 2 && !in {
ifInd = 1
}
if hp.IfIDs[ifInd] != 0 && hp.IfIDs[ifInd] != pi.ID() {
if hp.IfIDs[ifInd] != 0 && hp.IfIDs[ifInd] != pi.ID {
return false
}
return true
Expand Down
11 changes: 1 addition & 10 deletions go/lib/pathpol/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"github.com/stretchr/testify/require"

"github.com/scionproto/scion/go/lib/addr"
"github.com/scionproto/scion/go/lib/common"
"github.com/scionproto/scion/go/lib/snet"
"github.com/scionproto/scion/go/lib/xtest"
"github.com/scionproto/scion/go/lib/xtest/graph"
Expand Down Expand Up @@ -634,7 +633,7 @@ func (p PathProvider) GetPaths(src, dst addr.IA) PathSet {
var key strings.Builder
for _, ifid := range ifids {
ia := p.g.GetParent(ifid)
pathIntfs = append(pathIntfs, testPathIntf{ia: ia, ifid: ifid})
pathIntfs = append(pathIntfs, snet.PathInterface{IA: ia, ID: ifid})
key.WriteString(fmt.Sprintf("%s-%d", ia, ifid))
}
result[snet.PathFingerprint(key.String())] = &testPath{
Expand All @@ -652,14 +651,6 @@ func (p *testPath) Interfaces() []snet.PathInterface {
return p.interfaces
}

type testPathIntf struct {
ia addr.IA
ifid common.IFIDType
}

func (i testPathIntf) ID() common.IFIDType { return i.ifid }
func (i testPathIntf) IA() addr.IA { return i.ia }

func mustHopPredicate(t *testing.T, str string) *HopPredicate {
hp, err := HopPredicateFromString(str)
xtest.FailOnErr(t, err)
Expand Down
10 changes: 5 additions & 5 deletions go/lib/pathpol/sequence.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,13 @@ func (s *Sequence) Eval(inputSet PathSet) PathSet {
// one element in form <IA>#<inbound-interface>,<outbound-interface>,
// e.g. 64-ff00:0:112#3,5. For the source AS, the inbound interface will be
// zero. For destination AS, outbound interface will be zero.
p := fmt.Sprintf("%s#0,%d ", ifaces[0].IA(), ifaces[0].ID())
p := fmt.Sprintf("%s#0,%d ", ifaces[0].IA, ifaces[0].ID)
for i := 1; i < len(ifaces)-1; i += 2 {
p += fmt.Sprintf("%s#%d,%d ", ifaces[i].IA(),
ifaces[i].ID(), ifaces[i+1].ID())
p += fmt.Sprintf("%s#%d,%d ", ifaces[i].IA,
ifaces[i].ID, ifaces[i+1].ID)
}
p += fmt.Sprintf("%s#%d,0 ", ifaces[len(ifaces)-1].IA(),
ifaces[len(ifaces)-1].ID())
p += fmt.Sprintf("%s#%d,0 ", ifaces[len(ifaces)-1].IA,
ifaces[len(ifaces)-1].ID)
// Check whether the string matches the sequence regexp.
//fmt.Printf("EVAL: %s\n", p)
if s.re.MatchString(p) {
Expand Down
Loading

0 comments on commit 962ca85

Please sign in to comment.