combinator: avoid returning duplicate paths

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",

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))
+}

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{}

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
+)