Skip to content

Commit

Permalink
topology: simplify local address configuration (scionproto#4489)
Browse files Browse the repository at this point in the history
The router interface section of the topology.json file previously had
two fields to specify the local address of the router:
- public: IP:port
- bind: IP

The idea was that normally only `public` is supplied. When behind NAT,
then `bind` could override the IP address on which the router binds
locally, and `public` would be the address that is visible to the remote
router. The only place where this could be relevant is the router BFD
keep-alive messages, as these contain the local address of the router in
the SCION address header. There is, however, no check that enforces that
this matches the underlay address.
In practice, if `bind` is supplied, the IP of `public` has simply been
ignored.

This change replaces `public` and `bind` with a new `local` field. The
old fields are deprecated, but still processed for backwards
compatibility.
The `local` support omitting the IP to bind on any local IP. This was
previously possible only by explicitly configuring a zero IP address.

Aside:
- change the internal representation from `net.UDPAddr` to
`netip.AddrPort`. Note that this changes the representation in the json
dump of the `/topology` status page.
- remove unused BRNames
  • Loading branch information
matzf authored Mar 21, 2024
1 parent 7405953 commit 2798312
Show file tree
Hide file tree
Showing 36 changed files with 415 additions and 484 deletions.
6 changes: 3 additions & 3 deletions acceptance/router_benchmark/conf/topology.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"interfaces": {
"2": {
"underlay": {
"public": "10.123.2.1:50000",
"local": "10.123.2.1:50000",
"remote": "10.123.2.2:50000"
},
"isd_as": "1-ff00:0:2",
Expand All @@ -19,7 +19,7 @@
},
"3": {
"underlay": {
"public": "10.123.3.1:50000",
"local": "10.123.3.1:50000",
"remote": "10.123.3.3:50000"
},
"isd_as": "1-ff00:0:3",
Expand All @@ -33,7 +33,7 @@
"interfaces": {
"4": {
"underlay": {
"public": "10.123.4.1:50000",
"local": "10.123.4.1:50000",
"remote": "10.123.4.4:50000"
},
"isd_as": "2-ff00:0:4",
Expand Down
14 changes: 7 additions & 7 deletions acceptance/router_multi/conf/topology.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"interfaces": {
"121": {
"underlay": {
"public": "192.168.12.2:50000",
"local": "192.168.12.2:50000",
"remote": "192.168.12.3:40000"
},
"isd_as": "1-ff00:0:2",
Expand All @@ -18,7 +18,7 @@
},
"131": {
"underlay": {
"public": "192.168.13.2:50000",
"local": "192.168.13.2:50000",
"remote": "192.168.13.3:40000"
},
"isd_as": "1-ff00:0:3",
Expand All @@ -27,7 +27,7 @@
},
"141": {
"underlay": {
"public": "192.168.14.2:50000",
"local": "192.168.14.2:50000",
"remote": "192.168.14.3:40000"
},
"isd_as": "1-ff00:0:4",
Expand All @@ -36,7 +36,7 @@
},
"151": {
"underlay": {
"public": "192.168.15.2:50000",
"local": "192.168.15.2:50000",
"remote": "192.168.15.3:40000"
},
"isd_as": "1-ff00:0:5",
Expand All @@ -51,7 +51,7 @@
"interfaces": {
"171": {
"underlay": {
"public": "192.168.17.2:50000",
"local": "192.168.17.2:50000",
"remote": "192.168.17.3:40000"
},
"isd_as": "2-ff00:0:7",
Expand All @@ -66,7 +66,7 @@
"interfaces": {
"181": {
"underlay": {
"public": "192.168.18.2:50000",
"local": "192.168.18.2:50000",
"remote": "192.168.18.3:40000"
},
"isd_as": "1-ff00:0:8",
Expand All @@ -81,7 +81,7 @@
"interfaces": {
"191": {
"underlay": {
"public": "192.168.19.2:50000",
"local": "192.168.19.2:50000",
"remote": "192.168.19.3:40000"
},
"isd_as": "1-ff00:0:9",
Expand Down
4 changes: 2 additions & 2 deletions acceptance/topo_common/topology.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"interfaces": {
"1": {
"underlay": {
"public": "127.0.0.4:50000",
"local": "127.0.0.4:50000",
"remote": "127.0.0.5:50000"
},
"isd_as": "1-ff00:0:111",
Expand All @@ -26,7 +26,7 @@
"interfaces": {
"2": {
"underlay": {
"public": "127.0.0.6:50000",
"local": "127.0.0.6:50000",
"remote": "127.0.0.7:50000"
},
"isd_as": "1-ff00:0:112",
Expand Down
4 changes: 2 additions & 2 deletions acceptance/topo_cs_reload/testdata/topology_reload.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"interfaces": {
"1": {
"underlay": {
"public": "127.0.1.4:50000",
"local": "127.0.1.4:50000",
"remote": "127.0.1.5:50000"
},
"isd_as": "1-ff00:0:111",
Expand All @@ -26,7 +26,7 @@
"interfaces": {
"2": {
"underlay": {
"public": "127.0.1.6:50000",
"local": "127.0.1.6:50000",
"remote": "127.0.1.7:50000"
},
"isd_as": "1-ff00:0:112",
Expand Down
4 changes: 2 additions & 2 deletions acceptance/topo_daemon_reload/testdata/topology_reload.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"interfaces": {
"1": {
"underlay": {
"public": "127.0.1.4:50000",
"local": "127.0.1.4:50000",
"remote": "127.0.1.5:50000"
},
"isd_as": "1-ff00:0:111",
Expand All @@ -26,7 +26,7 @@
"interfaces": {
"2": {
"underlay": {
"public": "127.0.1.6:50000",
"local": "127.0.1.6:50000",
"remote": "127.0.1.7:50000"
},
"isd_as": "1-ff00:0:112",
Expand Down
8 changes: 4 additions & 4 deletions control/beaconing/testdata/topology-core.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"interfaces": {
"1113": {
"underlay": {
"public": "127.0.0.5:0",
"local": "127.0.0.5:0",
"remote": "127.0.0.4:0"
},
"isd_as": "1-ff00:0:130",
Expand All @@ -20,7 +20,7 @@
},
"1121": {
"underlay": {
"public": "127.0.0.5:0",
"local": "127.0.0.5:0",
"remote": "127.0.0.4:0"
},
"isd_as": "2-ff00:0:210",
Expand All @@ -29,7 +29,7 @@
},
"1129": {
"underlay": {
"public": "127.0.0.5:0",
"local": "127.0.0.5:0",
"remote": "127.0.0.4:0"
},
"isd_as": "1-ff00:0:120",
Expand All @@ -38,7 +38,7 @@
},
"42": {
"underlay": {
"public": "127.0.0.5:0",
"local": "127.0.0.5:0",
"remote": "127.0.0.4:0"
},
"isd_as": "1-ff00:0:111",
Expand Down
10 changes: 5 additions & 5 deletions control/beaconing/testdata/topology.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"interfaces": {
"1417": {
"underlay": {
"public": "127.0.0.22:50000",
"local": "127.0.0.22:50000",
"remote": "127.0.0.23:50000"
},
"isd_as": "1-ff00:0:112",
Expand All @@ -18,7 +18,7 @@
},
"2712": {
"underlay": {
"public": "127.0.0.10:50000",
"local": "127.0.0.10:50000",
"remote": "127.0.0.11:50000"
},
"isd_as": "1-ff00:0:120",
Expand All @@ -27,7 +27,7 @@
},
"2723": {
"underlay": {
"public": "127.0.0.20:50000",
"local": "127.0.0.20:50000",
"remote": "127.0.0.21:50000"
},
"isd_as": "2-ff00:0:211",
Expand All @@ -36,7 +36,7 @@
},
"2815": {
"underlay": {
"public": "127.0.0.18:50000",
"local": "127.0.0.18:50000",
"remote": "127.0.0.19:50000"
},
"isd_as": "1-ff00:0:121",
Expand All @@ -45,7 +45,7 @@
},
"2823": {
"underlay": {
"public": "127.0.0.12:50000",
"local": "127.0.0.12:50000",
"remote": "127.0.0.13:50000"
},
"isd_as": "2-ff00:0:211",
Expand Down
1 change: 0 additions & 1 deletion control/cmd/control/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ go_library(
"@com_github_grpc_ecosystem_go_grpc_prometheus//:go_default_library",
"@com_github_spf13_cobra//:go_default_library",
"@in_gopkg_yaml_v2//:go_default_library",
"@org_go4_netipx//:go_default_library",
"@org_golang_google_grpc//:go_default_library",
"@org_golang_google_grpc//health:go_default_library",
"@org_golang_google_grpc//health/grpc_health_v1:go_default_library",
Expand Down
12 changes: 1 addition & 11 deletions control/cmd/control/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"crypto/x509"
"encoding/json"
"errors"
"fmt"
"net/http"
_ "net/http/pprof"
"path/filepath"
Expand All @@ -31,7 +30,6 @@ import (
"github.com/go-chi/cors"
promgrpc "github.com/grpc-ecosystem/go-grpc-prometheus"
"github.com/spf13/cobra"
"go4.org/netipx"
"golang.org/x/sync/errgroup"
"google.golang.org/grpc"
"google.golang.org/grpc/health"
Expand Down Expand Up @@ -843,19 +841,11 @@ func createBeaconStore(
func adaptInterfaceMap(in map[common.IFIDType]topology.IFInfo) map[uint16]ifstate.InterfaceInfo {
converted := make(map[uint16]ifstate.InterfaceInfo, len(in))
for id, info := range in {
addr, ok := netipx.FromStdAddr(
info.InternalAddr.IP,
info.InternalAddr.Port,
info.InternalAddr.Zone,
)
if !ok {
panic(fmt.Sprintf("failed to adapt the topology format. Input %s", info.InternalAddr))
}
converted[uint16(id)] = ifstate.InterfaceInfo{
ID: uint16(info.ID),
IA: info.IA,
LinkType: info.LinkType,
InternalAddr: addr,
InternalAddr: info.InternalAddr,
RemoteID: uint16(info.RemoteIFID),
MTU: uint16(info.MTU),
}
Expand Down
12 changes: 6 additions & 6 deletions control/ifstate/testdata/topology.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"interfaces": {
"101": {
"underlay": {
"public": "127.0.0.12:50000",
"local": "127.0.0.12:50000",
"remote": "127.0.0.13:50000"
},
"isd_as": "2-ff00:0:211",
Expand All @@ -18,7 +18,7 @@
},
"104": {
"underlay": {
"public": "127.0.0.10:50000",
"local": "127.0.0.10:50000",
"remote": "127.0.0.11:50000"
},
"isd_as": "1-ff00:0:120",
Expand All @@ -33,7 +33,7 @@
"interfaces": {
"103": {
"underlay": {
"public": "127.0.0.14:50000",
"local": "127.0.0.14:50000",
"remote": "127.0.0.15:50000"
},
"isd_as": "1-ff00:0:112",
Expand All @@ -42,7 +42,7 @@
},
"105": {
"underlay": {
"public": "127.0.0.16:50000",
"local": "127.0.0.16:50000",
"remote": "127.0.0.17:50000"
},
"isd_as": "1-ff00:0:130",
Expand All @@ -57,7 +57,7 @@
"interfaces": {
"100": {
"underlay": {
"public": "127.0.0.18:50000",
"local": "127.0.0.18:50000",
"remote": "127.0.0.19:50000"
},
"isd_as": "1-ff00:0:121",
Expand All @@ -66,7 +66,7 @@
},
"102": {
"underlay": {
"public": "127.0.0.20:50000",
"local": "127.0.0.20:50000",
"remote": "127.0.0.21:50000"
},
"isd_as": "2-ff00:0:211",
Expand Down
Loading

0 comments on commit 2798312

Please sign in to comment.