From a864a8c12e228cf0a891f6ab81ce5daf8f3673c8 Mon Sep 17 00:00:00 2001 From: jiceatscion <139873336+jiceatscion@users.noreply.github.com> Date: Mon, 9 Sep 2024 12:18:45 +0200 Subject: [PATCH 1/3] topology: allow peering links between core ASes (#4605) Fixes #4484 --- .buildkite/pipeline.yml | 6 ++--- control/beaconing/originator.go | 8 +++++- control/beaconing/originator_test.go | 3 +++ control/beaconing/testdata/topology-core.json | 10 +++++++ private/path/combinator/combinator_test.go | 27 ++++++++++++++++--- .../testdata/00_core_core_invalid_peering.txt | 25 +++++++++++++++++ .../combinator/testdata/00_multi_peering.txt | 22 +++++++++++++++ private/topology/topology.go | 2 +- tools/await-connectivity | 11 +++++--- 9 files changed, 102 insertions(+), 12 deletions(-) create mode 100644 private/path/combinator/testdata/00_core_core_invalid_peering.txt diff --git a/.buildkite/pipeline.yml b/.buildkite/pipeline.yml index 72c9c14cdb..766833d3ce 100644 --- a/.buildkite/pipeline.yml +++ b/.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 diff --git a/control/beaconing/originator.go b/control/beaconing/originator.go index c35b4e9038..3c7bf2067c 100644 --- a/control/beaconing/originator.go +++ b/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 diff --git a/control/beaconing/originator_test.go b/control/beaconing/originator_test.go index e7d64673a5..63d625bfe3 100644 --- a/control/beaconing/originator_test.go +++ b/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) diff --git a/control/beaconing/testdata/topology-core.json b/control/beaconing/testdata/topology-core.json index 11284ccd2f..eaae64d627 100644 --- a/control/beaconing/testdata/topology-core.json +++ b/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 } } } diff --git a/private/path/combinator/combinator_test.go b/private/path/combinator/combinator_test.go index 15bb1b6d4e..af67f0ebf4 100644 --- a/private/path/combinator/combinator_test.go +++ b/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) diff --git a/private/path/combinator/testdata/00_core_core_invalid_peering.txt b/private/path/combinator/testdata/00_core_core_invalid_peering.txt new file mode 100644 index 0000000000..6b712da657 --- /dev/null +++ b/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 diff --git a/private/path/combinator/testdata/00_multi_peering.txt b/private/path/combinator/testdata/00_multi_peering.txt index d0aafe0697..e6191e236c 100644 --- a/private/path/combinator/testdata/00_multi_peering.txt +++ b/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 .. diff --git a/private/topology/topology.go b/private/topology/topology.go index ef1234a602..4677a61592 100644 --- a/private/topology/topology.go +++ b/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) diff --git a/tools/await-connectivity b/tools/await-connectivity index 4f0d1f2aee..e0876f1509 100755 --- a/tools/await-connectivity +++ b/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,12 +88,12 @@ 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" @@ -101,3 +101,6 @@ main() { } main "$@" + +# that is not enough. Down segment registrations don't seem to happen fast. +sleep 10 From e88ddce159a03039b64907be17186aa3787021f5 Mon Sep 17 00:00:00 2001 From: jiceatscion <139873336+jiceatscion@users.noreply.github.com> Date: Mon, 9 Sep 2024 12:41:08 +0200 Subject: [PATCH 2/3] ci: udate version of upload artefact action (#4613) --- .github/workflows/gobra.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/gobra.yml b/.github/workflows/gobra.yml index 4dadbab9e8..5d544611d1 100644 --- a/.github/workflows/gobra.yml +++ b/.github/workflows/gobra.yml @@ -33,7 +33,7 @@ jobs: caching: '1' statsFile: ${{ env.statsFile }} - name: Upload the verification report - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: name: verification_stats.json path: ${{ env.statsFile }} From 79e7080b3f9471434c3b2e35026ae43dec794c9d Mon Sep 17 00:00:00 2001 From: Marten Gartner Date: Mon, 9 Sep 2024 12:59:16 +0200 Subject: [PATCH 3/3] underlay: support SockOptInt on all platforms incl Windows (#4610) There is an issue that `syscall.GetsockoptInt` and `syscall.SetsockoptInt` have different signatures on windows than on all other platforms. One fix is to have extra windows code to support this. In the end it would be great to have Windows support for all SCION services, too. The issue popped up when building the router, not sure if it affects other services. --- private/underlay/sockctrl/BUILD.bazel | 48 ++++++++++++++++++- private/underlay/sockctrl/sockctrl.go | 6 +-- private/underlay/sockctrl/sockctrl_windows.go | 43 +++++++++++++++++ private/underlay/sockctrl/sockopt.go | 3 ++ private/underlay/sockctrl/sockopt_windows.go | 39 +++++++++++++++ 5 files changed, 134 insertions(+), 5 deletions(-) create mode 100644 private/underlay/sockctrl/sockctrl_windows.go create mode 100644 private/underlay/sockctrl/sockopt_windows.go diff --git a/private/underlay/sockctrl/BUILD.bazel b/private/underlay/sockctrl/BUILD.bazel index 67e19ae27d..f1d0dfe0db 100644 --- a/private/underlay/sockctrl/BUILD.bazel +++ b/private/underlay/sockctrl/BUILD.bazel @@ -4,9 +4,55 @@ go_library( name = "go_default_library", srcs = [ "sockctrl.go", + "sockctrl_windows.go", "sockopt.go", + "sockopt_windows.go", ], importpath = "github.com/scionproto/scion/private/underlay/sockctrl", visibility = ["//visibility:public"], - deps = ["//pkg/private/serrors:go_default_library"], + deps = select({ + "@io_bazel_rules_go//go/platform:aix": [ + "//pkg/private/serrors:go_default_library", + ], + "@io_bazel_rules_go//go/platform:android": [ + "//pkg/private/serrors:go_default_library", + ], + "@io_bazel_rules_go//go/platform:darwin": [ + "//pkg/private/serrors:go_default_library", + ], + "@io_bazel_rules_go//go/platform:dragonfly": [ + "//pkg/private/serrors:go_default_library", + ], + "@io_bazel_rules_go//go/platform:freebsd": [ + "//pkg/private/serrors:go_default_library", + ], + "@io_bazel_rules_go//go/platform:illumos": [ + "//pkg/private/serrors:go_default_library", + ], + "@io_bazel_rules_go//go/platform:ios": [ + "//pkg/private/serrors:go_default_library", + ], + "@io_bazel_rules_go//go/platform:js": [ + "//pkg/private/serrors:go_default_library", + ], + "@io_bazel_rules_go//go/platform:linux": [ + "//pkg/private/serrors:go_default_library", + ], + "@io_bazel_rules_go//go/platform:netbsd": [ + "//pkg/private/serrors:go_default_library", + ], + "@io_bazel_rules_go//go/platform:openbsd": [ + "//pkg/private/serrors:go_default_library", + ], + "@io_bazel_rules_go//go/platform:plan9": [ + "//pkg/private/serrors:go_default_library", + ], + "@io_bazel_rules_go//go/platform:solaris": [ + "//pkg/private/serrors:go_default_library", + ], + "@io_bazel_rules_go//go/platform:windows": [ + "//pkg/private/serrors:go_default_library", + ], + "//conditions:default": [], + }), ) diff --git a/private/underlay/sockctrl/sockctrl.go b/private/underlay/sockctrl/sockctrl.go index 2faeef054a..af9dc41692 100644 --- a/private/underlay/sockctrl/sockctrl.go +++ b/private/underlay/sockctrl/sockctrl.go @@ -12,11 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:build go1.9 -// +build go1.9 +// In Windows, SetSockOptInt and GetSockOptInt require syscall.Handle instead of int. +//go:build !windows -// This version of sockctrl is for Go versions >= 1.9, where the socket FDs are -// accessible via RawConn.Control(). package sockctrl import ( diff --git a/private/underlay/sockctrl/sockctrl_windows.go b/private/underlay/sockctrl/sockctrl_windows.go new file mode 100644 index 0000000000..81fd84c7bb --- /dev/null +++ b/private/underlay/sockctrl/sockctrl_windows.go @@ -0,0 +1,43 @@ +// Copyright 2024 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. + +// In Windows, SetSockOptInt and GetSockOptInt require syscall.Handle instead of int. +//go:build windows + +package sockctrl + +import ( + "net" + "syscall" + + "github.com/scionproto/scion/pkg/private/serrors" +) + +func SockControl(c *net.UDPConn, f func(syscall.Handle) error) error { + rawConn, err := c.SyscallConn() + if err != nil { + return serrors.Wrap("sockctrl: error accessing raw connection", err) + } + var ctrlErr error + err = rawConn.Control(func(fd uintptr) { + ctrlErr = f(syscall.Handle(fd)) + }) + if err != nil { + return serrors.Wrap("sockctrl: RawConn.Control error", err) + } + if ctrlErr != nil { + return serrors.Wrap("sockctrl: control function error", ctrlErr) + } + return nil +} diff --git a/private/underlay/sockctrl/sockopt.go b/private/underlay/sockctrl/sockopt.go index cbf9339f06..d5b56e5cfc 100644 --- a/private/underlay/sockctrl/sockopt.go +++ b/private/underlay/sockctrl/sockopt.go @@ -12,6 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +// In Windows, SetSockOptInt and GetSockOptInt require syscall.Handle instead of int. +//go:build !windows + package sockctrl import ( diff --git a/private/underlay/sockctrl/sockopt_windows.go b/private/underlay/sockctrl/sockopt_windows.go new file mode 100644 index 0000000000..8a9ceb07b8 --- /dev/null +++ b/private/underlay/sockctrl/sockopt_windows.go @@ -0,0 +1,39 @@ +// Copyright 2024 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. + +// In Windows, SetSockOptInt and GetSockOptInt require syscall.Handle instead of int. +//go:build windows + +package sockctrl + +import ( + "net" + "syscall" +) + +func GetsockoptInt(c *net.UDPConn, level, opt int) (int, error) { + var val int + err := SockControl(c, func(fd syscall.Handle) error { + var err error + val, err = syscall.GetsockoptInt(fd, level, opt) + return err + }) + return val, err +} + +func SetsockoptInt(c *net.UDPConn, level, opt, value int) error { + return SockControl(c, func(fd syscall.Handle) error { + return syscall.SetsockoptInt(fd, level, opt, value) + }) +}