topology: allow peering links between core ASes (scionproto#4605)

Fixes scionproto#4484

.buildkite/pipeline.yml

@@ -116,7 +116,7 @@ steps:
         - ./scion.sh run
         - tools/await-connectivity
         - ./bin/scion_integration || ( echo "^^^ +++" && false )
-        - ./bin/end2end_integration || ( echo "^^^ +++" && false )
+        - ./bin/end2end_integration --attempts=3 || ( echo "^^^ +++" && false )
       plugins: &scion-run-hooks
         - scionproto/metahook#v0.3.0:
             pre-command: .buildkite/cleanup-leftovers.sh
@@ -142,7 +142,7 @@ steps:
         - ./scion.sh topology -c topology/default-no-peers.topo
         - ./scion.sh run
         - tools/await-connectivity
-        - ./bin/end2end_integration || ( echo "^^^ +++" && false )
+        - ./bin/end2end_integration --attempts=3 || ( echo "^^^ +++" && false )
         - ./tools/integration/revocation_test.sh
       plugins: *scion-run-hooks
       artifact_paths: *scion-run-artifact-paths
@@ -158,7 +158,7 @@ steps:
         - ./scion.sh run
         - tools/await-connectivity
         - echo "--- run tests"
-        - ./bin/end2end_integration -d || ( echo "^^^ +++" && false )
+        - ./bin/end2end_integration -d --attempts=3 || ( echo "^^^ +++" && false )
       plugins: *scion-run-hooks
       artifact_paths: *scion-run-artifact-paths
       timeout_in_minutes: 15

control/beaconing/originator.go

@@ -32,6 +32,7 @@ import (
 	"github.com/scionproto/scion/pkg/private/serrors"
 	seg "github.com/scionproto/scion/pkg/segment"
 	"github.com/scionproto/scion/private/periodic"
+	"github.com/scionproto/scion/private/topology"
 )
 
 var _ periodic.Task = (*Originator)(nil)
@@ -95,6 +96,9 @@ func (o *Originator) originateBeacons(ctx context.Context) {
 		return
 	}
 
+	// Core ases can have peering too. Fetch the peering interfaces.
+	peers := sortedIntfs(o.AllInterfaces, topology.Peer)
+
 	// Only log on info and error level every propagation period to reduce
 	// noise. The offending logs events are redirected to debug level.
 	silent := !o.Tick.Passed()
@@ -109,6 +113,7 @@ func (o *Originator) originateBeacons(ctx context.Context) {
 			intf:       intf,
 			timestamp:  o.Tick.Now(),
 			summary:    s,
+			peers:      peers,
 		}
 		go func() {
 			defer log.HandlePanic()
@@ -154,6 +159,7 @@ type beaconOriginator struct {
 	intf      *ifstate.Interface
 	timestamp time.Time
 	summary   *summary
+	peers     []uint16
 }
 
 // originateBeacon originates a beacon on the given ifID.
@@ -206,7 +212,7 @@ func (o *beaconOriginator) createBeacon(ctx context.Context) (*seg.PathSegment,
 		return nil, serrors.Wrap("creating segment", err)
 	}
 
-	if err := o.Extender.Extend(ctx, beacon, 0, o.intf.TopoInfo().ID, nil); err != nil {
+	if err := o.Extender.Extend(ctx, beacon, 0, o.intf.TopoInfo().ID, o.peers); err != nil {
 		return nil, serrors.Wrap("extending segment", err)
 	}
 	return beacon, nil

control/beaconing/originator_test.go

@@ -104,6 +104,9 @@ func TestOriginatorRun(t *testing.T) {
 						hopF := b.ASEntries[b.MaxIdx()].HopEntry.HopField
 						// Check the interface matches.
 						assert.Equal(t, hopF.ConsEgress, egIfID)
+						// Check that the expected peering entry is there too.
+						peering := b.ASEntries[b.MaxIdx()].PeerEntries[0]
+						assert.Equal(t, peering.HopField.ConsIngress, uint16(4242))
 						// Check that the beacon is sent to the correct border router.
 						br := net.UDPAddrFromAddrPort(interfaceInfos(topo)[egIfID].InternalAddr)
 						assert.Equal(t, br, nextHop)

control/beaconing/testdata/topology-core.json

@@ -45,6 +45,16 @@
           "isd_as": "1-ff00:0:111",
           "link_to": "CHILD",
           "mtu": 1280
+        },
+        "4242": {
+          "underlay": {
+            "local": "127.0.0.5:0",
+            "remote": "127.0.0.4:0"
+          },
+          "isd_as": "2-ff00:0:220",
+          "link_to": "PEER",
+          "remote_interface_id": 2424,
+          "mtu": 1280
         }
       }
     }

private/path/combinator/combinator_test.go

@@ -96,10 +96,12 @@ func TestBadPeering(t *testing.T) {
 	}
 }
 
-func TestMultiPeering(t *testing.T) {
+func TestMiscPeering(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 	g := graph.NewDefaultGraph(ctrl)
+	// Add a core-core peering link. It can be used in some cases.
+	g.AddLink("1-ff00:0:130", 4001, "2-ff00:0:210", 4002, true)
 
 	testCases := []struct {
 		Name     string
@@ -111,7 +113,7 @@ func TestMultiPeering(t *testing.T) {
 		Downs    []*seg.PathSegment
 	}{
 		{
-			Name:     "two peerings between same ases",
+			Name:     "two peerings between same ases and core core peering",
 			FileName: "00_multi_peering.txt",
 			SrcIA:    addr.MustParseIA("1-ff00:0:112"),
 			DstIA:    addr.MustParseIA("2-ff00:0:212"),
@@ -125,8 +127,27 @@ func TestMultiPeering(t *testing.T) {
 				g.Beacon([]uint16{graph.If_210_X_211_A, graph.If_211_A_212_X}),
 			},
 		},
+		{
+			// In this case, the 130-210 peering link should not be used (the router would reject)
+			// because the hop through 210 would be assimilated to a valley path: one of the
+			// joined segments is a core segment, not a down segment.
+			Name:     "core to core peering forbidden",
+			FileName: "00_core_core_invalid_peering.txt",
+			SrcIA:    addr.MustParseIA("1-ff00:0:131"),
+			DstIA:    addr.MustParseIA("2-ff00:0:221"),
+			Ups: []*seg.PathSegment{
+				g.Beacon([]uint16{graph.If_130_A_131_X}),
+			},
+			Cores: []*seg.PathSegment{
+				g.Beacon([]uint16{
+					graph.If_220_X_210_X, graph.If_210_X_110_X, graph.If_110_X_130_A}),
+			},
+			Downs: []*seg.PathSegment{
+				g.Beacon([]uint16{graph.If_220_X_221_X}),
+			},
+		},
 	}
-	t.Log("TestMultiPeering")
+	t.Log("TestMiscPeering")
 	for _, tc := range testCases {
 		t.Run(tc.Name, func(t *testing.T) {
 			result := combinator.Combine(tc.SrcIA, tc.DstIA, tc.Ups, tc.Cores, tc.Downs, false)

private/path/combinator/testdata/00_core_core_invalid_peering.txt

@@ -0,0 +1,25 @@
+Path #0:
+  Weight: 5
+  Fields:
+    IF ..
+      HF InIF=1613 OutIF=0
+      HF InIF=0 OutIF=1316
+    IF ..
+      HF InIF=1311 OutIF=0
+      HF InIF=1121 OutIF=1113
+      HF InIF=2122 OutIF=2111
+      HF InIF=0 OutIF=2221
+    IF C.
+      HF InIF=0 OutIF=2224
+      HF InIF=2422 OutIF=0
+  Interfaces:
+    1-ff00:0:131#1613
+    1-ff00:0:130#1316
+    1-ff00:0:130#1311
+    1-ff00:0:110#1113
+    1-ff00:0:110#1121
+    2-ff00:0:210#2111
+    2-ff00:0:210#2122
+    2-ff00:0:220#2221
+    2-ff00:0:220#2224
+    2-ff00:0:221#2422

private/path/combinator/testdata/00_multi_peering.txt

@@ -31,6 +31,28 @@ Path #1:
     2-ff00:0:211#2325
     2-ff00:0:212#2523
 Path #2:
+  Weight: 5
+  Fields:
+    IF .P
+      HF InIF=1714 OutIF=0
+      HF InIF=1432 OutIF=1417
+      HF InIF=4001 OutIF=3214
+    IF CP
+      HF InIF=4002 OutIF=2123
+      HF InIF=2321 OutIF=2325
+      HF InIF=2523 OutIF=0
+  Interfaces:
+    1-ff00:0:112#1714
+    1-ff00:0:111#1417
+    1-ff00:0:111#1432
+    1-ff00:0:130#3214
+    1-ff00:0:130#4001
+    2-ff00:0:210#4002
+    2-ff00:0:210#2123
+    2-ff00:0:211#2321
+    2-ff00:0:211#2325
+    2-ff00:0:212#2523
+Path #3:
   Weight: 6
   Fields:
     IF ..

private/topology/topology.go

@@ -593,7 +593,7 @@ func (m IDAddrMap) copy() IDAddrMap {
 func (i IFInfo) CheckLinks(isCore bool, brName string) error {
 	if isCore {
 		switch i.LinkType {
-		case Core, Child:
+		case Core, Child, Peer:
 		default:
 			return serrors.New("Illegal link type for core AS",
 				"type", i.LinkType, "br", brName)

tools/await-connectivity

@@ -61,7 +61,7 @@ check_connectivity() {
   for as in $cores; do
     missing=$(comm -23 \
       <(printf "%s\n" $cores | grep -v $as | sort) \
-      <(curl -sfS $(cs_api_url_segments "$as") | jq '.[].start_isd_as' -r | sort -u)
+      <(curl --connect-timeout 5 -sfS $(cs_api_url_segments "$as") | jq '.[].start_isd_as' -r | sort -u)
     )
     if [ -n "$missing" ]; then
       echo "$as: waiting for" $missing
@@ -71,7 +71,7 @@ check_connectivity() {
 
   # non-core ASes: wait for at least one (up-)segment
   for as in $noncores; do
-    curl -sfS $(cs_api_url_segments "$as") | jq -e 'length > 0' > /dev/null || { 
+    curl --connect-timeout 5 -sfS $(cs_api_url_segments "$as") | jq -e 'length > 0' > /dev/null || { 
       echo "$as waiting for up segment"
       ret=1
     }
@@ -88,16 +88,19 @@ main() {
   local noncores=$(sed -n '/Non-core/,${s/^- //p}' gen/as_list.yml)
 
   for i in $(seq 1 "$QUIET"); do
-    check_connectivity "$cores" "$noncores" > /dev/null && exit 0
+    check_connectivity "$cores" "$noncores" > /dev/null && return 0
     sleep 1
   done
   for i in $(seq "$QUIET" $((TIMEOUT-1))); do
     echo "Check after ${i}s"
-    check_connectivity "$cores" "$noncores" && exit 0
+    check_connectivity "$cores" "$noncores" && return 0
     sleep 1
   done
   echo "Check after ${TIMEOUT}s"
   check_connectivity "$cores" "$noncores" || { echo "Timeout, giving up"; exit 1; }
 }
 
 main "$@"
+
+# that is not enough. Down segment registrations don't seem to happen fast.
+sleep 10