diff --git a/go/lib/infra/modules/combinator/BUILD.bazel b/go/lib/infra/modules/combinator/BUILD.bazel index 2e36705215..96f05e8163 100644 --- a/go/lib/infra/modules/combinator/BUILD.bazel +++ b/go/lib/infra/modules/combinator/BUILD.bazel @@ -29,6 +29,7 @@ go_test( srcs = [ "combinator_test.go", "expiry_test.go", + "export_test.go", "staticinfo_accumulator_test.go", ], data = glob(["testdata/**"]), @@ -40,6 +41,7 @@ go_test( "//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/xtest:go_default_library", "//go/lib/xtest/graph:go_default_library", "@com_github_golang_mock//gomock:go_default_library", diff --git a/go/lib/infra/modules/combinator/combinator.go b/go/lib/infra/modules/combinator/combinator.go index 32c6d57105..4caa9faa37 100644 --- a/go/lib/infra/modules/combinator/combinator.go +++ b/go/lib/infra/modules/combinator/combinator.go @@ -21,6 +21,10 @@ package combinator import ( + "crypto/sha256" + "encoding/binary" + "sort" + "github.com/scionproto/scion/go/lib/addr" "github.com/scionproto/scion/go/lib/ctrl/seg" "github.com/scionproto/scion/go/lib/snet" @@ -29,20 +33,42 @@ import ( // Combine constructs paths between src and dst using the supplied // segments. All possible paths are first computed, and then filtered according -// to filterLongPaths. The remaining paths are returned sorted according to +// to filterLongPaths. +// +// Normally, with findAllIdentical=false, Combine returns one path for each +// unique sequence of path interfaces found. If there are multiple ways to +// construct the same sequence of path interfaces from the available path +// segments, the construction with latest expiration time will be returned. +// +// With findAllIdentical=true, Combine may return multiple paths with identical +// sequences of path interfaces, but constructed from different path segments. +// These forwarding paths can only be destinguished by the segment IDs and the +// hop field MACs. Typically, all of these will have exactly the same +// forwarding behaviour, but it is possible that an AS would misbehave and +// change behaviour based on the segment IDs or MACs. An application can use +// findAllIdentical=true in order to explore whether this may be the case. +// Note that this may return a large number of paths for wide network +// topologies. +// +// The remaining paths are returned sorted according to // weight (on equal weight, see pathSolutionList.Less for the tie-breaking // algorithm). // // If Combine cannot extract a hop field or info field from the segments, it // panics. -func Combine(src, dst addr.IA, ups, cores, downs []*seg.PathSegment) []Path { - solutions := newDMG(ups, cores, downs).GetPaths(vertexFromIA(src), vertexFromIA(dst)) +func Combine(src, dst addr.IA, ups, cores, downs []*seg.PathSegment, + findAllIdentical bool) []Path { - var pathSlice []Path - for _, solution := range solutions { - pathSlice = append(pathSlice, solution.Path()) + solutions := newDMG(ups, cores, downs).GetPaths(vertexFromIA(src), vertexFromIA(dst)) + paths := make([]Path, len(solutions)) + for i, solution := range solutions { + paths[i] = solution.Path() + } + paths = filterLongPaths(paths) + if !findAllIdentical { + paths = filterDuplicates(paths) } - return filterLongPaths(pathSlice) + return paths } type Path struct { @@ -73,3 +99,47 @@ func filterLongPaths(paths []Path) []Path { } return newPaths } + +// filterDuplicates removes paths with identical sequences of path interfaces, +// keeping only the one instance with latest expiry. +// Duplicates can arise when multiple combinations of different path segments +// result in the same "effective" path after applying short-cuts. +// XXX(matzf): the duplicates could/should be avoided directly by reducing the +// available options in the graph, as we could potentially create a large +// number of duplicates in wide network topologies. +func filterDuplicates(paths []Path) []Path { + // uniquePaths stores the index of the path with the latest expiry for every + // unique path interface sequence (== fingerprint). + uniquePaths := make(map[snet.PathFingerprint]int) + for i, p := range paths { + key := fingerprint(p.Metadata.Interfaces) + prev, dupe := uniquePaths[key] + if !dupe || p.Metadata.Expiry.After(paths[prev].Metadata.Expiry) { + uniquePaths[key] = i + } + } + + toKeep := make([]int, 0, len(uniquePaths)) + for _, idx := range uniquePaths { + toKeep = append(toKeep, idx) + } + sort.Ints(toKeep) + filtered := make([]Path, 0, len(toKeep)) + for _, i := range toKeep { + filtered = append(filtered, paths[i]) + } + return filtered +} + +// fingerprint uniquely identifies the path based on the sequence of +// ASes and BRs, i.e. by its PathInterfaces. +// XXX(matzf): copied from snet.Fingerprint. Perhaps snet.Fingerprint could be adapted to +// take []snet.PathInterface directly. +func fingerprint(interfaces []snet.PathInterface) snet.PathFingerprint { + h := sha256.New() + for _, intf := range interfaces { + binary.Write(h, binary.BigEndian, intf.IA.IAInt()) + binary.Write(h, binary.BigEndian, intf.ID) + } + return snet.PathFingerprint(h.Sum(nil)) +} diff --git a/go/lib/infra/modules/combinator/combinator_test.go b/go/lib/infra/modules/combinator/combinator_test.go index 9e8e292602..37b29f9403 100644 --- a/go/lib/infra/modules/combinator/combinator_test.go +++ b/go/lib/infra/modules/combinator/combinator_test.go @@ -12,15 +12,17 @@ // See the License for the specific language governing permissions and // limitations under the License. -package combinator +package combinator_test import ( "bytes" + "encoding/binary" "flag" "fmt" "io" "io/ioutil" "testing" + "time" "github.com/golang/mock/gomock" . "github.com/smartystreets/goconvey/convey" @@ -29,8 +31,11 @@ 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/infra/modules/combinator" "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/xtest" "github.com/scionproto/scion/go/lib/xtest/graph" ) @@ -80,7 +85,7 @@ func TestBadPeering(t *testing.T) { Convey("main", t, func() { for _, tc := range testCases { Convey(tc.Name, func() { - result := Combine(tc.SrcIA, tc.DstIA, tc.Ups, tc.Cores, tc.Downs) + result := combinator.Combine(tc.SrcIA, tc.DstIA, tc.Ups, tc.Cores, tc.Downs, false) txtResult := writePaths(result) if *update { err := ioutil.WriteFile(xtest.ExpandPath(tc.FileName), txtResult.Bytes(), 0644) @@ -127,7 +132,7 @@ func TestMultiPeering(t *testing.T) { for _, tc := range testCases { t.Run(tc.Name, func(t *testing.T) { - result := Combine(tc.SrcIA, tc.DstIA, tc.Ups, tc.Cores, tc.Downs) + result := combinator.Combine(tc.SrcIA, tc.DstIA, tc.Ups, tc.Cores, tc.Downs, false) txtResult := writePaths(result) if *update { err := ioutil.WriteFile(xtest.ExpandPath(tc.FileName), txtResult.Bytes(), 0644) @@ -171,7 +176,7 @@ func TestSameCoreParent(t *testing.T) { Convey("main", t, func() { for _, tc := range testCases { Convey(tc.Name, func() { - result := Combine(tc.SrcIA, tc.DstIA, tc.Ups, tc.Cores, tc.Downs) + result := combinator.Combine(tc.SrcIA, tc.DstIA, tc.Ups, tc.Cores, tc.Downs, false) txtResult := writePaths(result) if *update { err := ioutil.WriteFile(xtest.ExpandPath(tc.FileName), txtResult.Bytes(), 0644) @@ -223,7 +228,7 @@ func TestLoops(t *testing.T) { Convey("TestLoops", t, func() { for _, tc := range testCases { Convey(tc.Name, func() { - result := Combine(tc.SrcIA, tc.DstIA, tc.Ups, tc.Cores, tc.Downs) + result := combinator.Combine(tc.SrcIA, tc.DstIA, tc.Ups, tc.Cores, tc.Downs, false) txtResult := writePaths(result) if *update { err := ioutil.WriteFile(xtest.ExpandPath(tc.FileName), txtResult.Bytes(), 0644) @@ -532,7 +537,7 @@ func TestComputePath(t *testing.T) { Convey("main", t, func() { for _, tc := range testCases { Convey(tc.Name, func() { - result := Combine(tc.SrcIA, tc.DstIA, tc.Ups, tc.Cores, tc.Downs) + result := combinator.Combine(tc.SrcIA, tc.DstIA, tc.Ups, tc.Cores, tc.Downs, false) txtResult := writePaths(result) if *update { err := ioutil.WriteFile(xtest.ExpandPath(tc.FileName), txtResult.Bytes(), 0644) @@ -545,8 +550,144 @@ func TestComputePath(t *testing.T) { } }) } +func TestFilterDuplicates(t *testing.T) { + // Define three different path interface sequences for the test cases below. + // These look somewhat valid, but that doesn't matter at all -- we only look + // at the fingerprint anyway. + path0 := []snet.PathInterface{ + {IA: xtest.MustParseIA("1-ff00:0:110"), ID: common.IFIDType(10)}, + {IA: xtest.MustParseIA("1-ff00:0:111"), ID: common.IFIDType(10)}, + } + path1 := []snet.PathInterface{ + {IA: xtest.MustParseIA("1-ff00:0:110"), ID: common.IFIDType(11)}, + {IA: xtest.MustParseIA("1-ff00:0:112"), ID: common.IFIDType(11)}, + {IA: xtest.MustParseIA("1-ff00:0:112"), ID: common.IFIDType(12)}, + {IA: xtest.MustParseIA("1-ff00:0:111"), ID: common.IFIDType(12)}, + } + path2 := []snet.PathInterface{ + {IA: xtest.MustParseIA("1-ff00:0:110"), ID: common.IFIDType(11)}, + {IA: xtest.MustParseIA("1-ff00:0:112"), ID: common.IFIDType(11)}, + {IA: xtest.MustParseIA("1-ff00:0:112"), ID: common.IFIDType(22)}, + {IA: xtest.MustParseIA("1-ff00:0:111"), ID: common.IFIDType(22)}, + } + + // Define two expiry times for the paths: paths with latest expiry will be kept + timeEarly := time.Time{} + timeLater := timeEarly.Add(time.Hour) // just later than timeEarly + + testPath := func(id uint32, interfaces []snet.PathInterface, expiry time.Time) combinator.Path { + idBuf := make([]byte, 4) + binary.LittleEndian.PutUint32(idBuf, id) + return combinator.Path{ + // hide an id in the (otherwise unused) raw path + SPath: spath.Path{Raw: idBuf}, + Metadata: snet.PathMetadata{ + Interfaces: interfaces, + Expiry: expiry, + }, + } + } + + testCases := []struct { + Name string + Paths []combinator.Path + Expected []uint32 + }{ + { + Name: "nil slice", + Paths: nil, + Expected: []uint32{}, + }, + { + Name: "empty slice", + Paths: []combinator.Path{}, + Expected: []uint32{}, + }, + { + Name: "single path", + Paths: []combinator.Path{ + testPath(1, path0, timeEarly), + }, + Expected: []uint32{1}, + }, + { + Name: "different paths", + Paths: []combinator.Path{ + testPath(1, path0, timeEarly), + testPath(2, path1, timeEarly), + }, + Expected: []uint32{1, 2}, + }, + { + Name: "triple", + Paths: []combinator.Path{ + testPath(1, path0, timeEarly), + testPath(2, path0, timeLater), + testPath(3, path0, timeEarly), + }, + Expected: []uint32{2}, + }, + { + Name: "triple, same expiry", + Paths: []combinator.Path{ + testPath(1, path0, timeEarly), + testPath(2, path0, timeLater), + testPath(3, path0, timeLater), + }, + Expected: []uint32{2}, + }, + { + Name: "triple and double", + Paths: []combinator.Path{ + testPath(1, path0, timeEarly), + testPath(2, path0, timeLater), + testPath(3, path0, timeEarly), + testPath(5, path1, timeLater), + testPath(6, path1, timeEarly), + }, + Expected: []uint32{2, 5}, + }, + { + Name: "triple, double, single", + Paths: []combinator.Path{ + testPath(1, path0, timeEarly), + testPath(2, path0, timeLater), + testPath(3, path0, timeEarly), + testPath(5, path1, timeLater), + testPath(6, path1, timeEarly), + testPath(7, path2, timeEarly), + }, + Expected: []uint32{2, 5, 7}, + }, + { + Name: "triple, double, single, mixed", + Paths: []combinator.Path{ + testPath(1, path1, timeEarly), + testPath(2, path2, timeEarly), + testPath(3, path0, timeEarly), + testPath(4, path0, timeLater), + testPath(5, path1, timeLater), + testPath(6, path0, timeEarly), + }, + Expected: []uint32{2, 4, 5}, + }, + } + + for _, tc := range testCases { + t.Run(tc.Name, func(t *testing.T) { + + filtered := combinator.FilterDuplicates(tc.Paths) + // extract IDs hidden in the raw paths: + filteredIds := make([]uint32, len(filtered)) + for i, path := range filtered { + filteredIds[i] = binary.LittleEndian.Uint32(path.SPath.Raw) + } + assert.Equal(t, tc.Expected, filteredIds) + }) + } +} -func writePaths(paths []Path) *bytes.Buffer { +func writePaths(paths []combinator.Path) *bytes.Buffer { buffer := &bytes.Buffer{} for i, p := range paths { fmt.Fprintf(buffer, "Path #%d:\n", i) @@ -555,7 +696,7 @@ func writePaths(paths []Path) *bytes.Buffer { return buffer } -func writeTestString(p Path, w io.Writer) { +func writeTestString(p combinator.Path, w io.Writer) { fmt.Fprintf(w, " Weight: %d\n", p.Weight) sp := scion.Decoded{} diff --git a/go/lib/infra/modules/combinator/export_test.go b/go/lib/infra/modules/combinator/export_test.go new file mode 100644 index 0000000000..256b10af8c --- /dev/null +++ b/go/lib/infra/modules/combinator/export_test.go @@ -0,0 +1,19 @@ +// Copyright 2020 ETH Zurich +// +// 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 combinator + +var ( + FilterDuplicates = filterDuplicates +) diff --git a/go/lib/infra/modules/combinator/graph.go b/go/lib/infra/modules/combinator/graph.go index 4187c746f2..ec1abbd609 100644 --- a/go/lib/infra/modules/combinator/graph.go +++ b/go/lib/infra/modules/combinator/graph.go @@ -116,8 +116,7 @@ func (g *dmg) traverseSegment(segment *inputSegment) { // Whenever we add an edge that is not towards the first AS in the PCB, // we are creating a shortcut. We use the asEntryIndex to annotate the // edges as such, as we need the metadata during forwarding path - // construction when adding verify-only HFs and pruning unneeded pieces - // of the segment. + // construction when pruning unneeded pieces of the segment. currentIA := asEntries[asEntryIndex].Local @@ -275,13 +274,12 @@ type edgeMap map[*inputSegment]*edge type edge struct { Weight int // Shortcut is the ASEntry index on where the forwarding portion of this - // segment should end (for up-segments) or start (for down-segments). An - // additional V-only HF upstream of this ASEntry needs to be included for - // verification. This is also set when crossing peering links. If 0, the - // full segment is used. + // segment should end (for up-segments) or start (for down-segments). + // This is also set when crossing peering links. If 0, the full segment is + // used. Shortcut int - // Peer is the index in the hop entries array for this peer entry. If 0, - // the standard hop entry at index 0 is used (instead of a peer entry). + // Peer is the index + 1 in the peer entries array for ASEntry defined by the + // Shortcut index. This is 0 for non-peer shortcuts. Peer int } diff --git a/go/lib/infra/modules/combinator/testdata/00_loops.txt b/go/lib/infra/modules/combinator/testdata/00_loops.txt index ed94bce1ae..4b80312586 100644 --- a/go/lib/infra/modules/combinator/testdata/00_loops.txt +++ b/go/lib/infra/modules/combinator/testdata/00_loops.txt @@ -8,15 +8,6 @@ Path #0: 1-ff00:0:111#1417 1-ff00:0:112#1714 Path #1: - Weight: 1 - Fields: - IF C. - HF InIF=1432 OutIF=1417 - HF InIF=1714 OutIF=0 - Interfaces: - 1-ff00:0:111#1417 - 1-ff00:0:112#1714 -Path #2: Weight: 2 Fields: IF .. @@ -30,7 +21,7 @@ Path #2: 1-ff00:0:130#3214 1-ff00:0:130#1317 1-ff00:0:112#1713 -Path #3: +Path #2: Weight: 3 Fields: IF .. @@ -49,7 +40,7 @@ Path #3: 1-ff00:0:130#3229 1-ff00:0:130#1317 1-ff00:0:112#1713 -Path #4: +Path #3: Weight: 4 Fields: IF .. diff --git a/go/lib/infra/modules/segfetcher/pather.go b/go/lib/infra/modules/segfetcher/pather.go index 4e2bc85b8a..7369bfa89c 100644 --- a/go/lib/infra/modules/segfetcher/pather.go +++ b/go/lib/infra/modules/segfetcher/pather.go @@ -97,7 +97,7 @@ func (p *Pather) buildAllPaths(src, dst addr.IA, segs Segments) []combinator.Pat destinations := p.findDestinations(dst, up, core) var paths []combinator.Path for dst := range destinations { - paths = append(paths, combinator.Combine(src, dst, up, core, down)...) + paths = append(paths, combinator.Combine(src, dst, up, core, down, false)...) } // Filter expired paths now := time.Now()