From 39c85cf1a20c4f05f4bfd5a056d80d55349fbd44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilmann=20Z=C3=A4schke?= Date: Wed, 5 Jun 2024 13:00:46 +0200 Subject: [PATCH 01/13] Metadata --- .../scion/jpan/AbstractDatagramChannel.java | 3 +- .../java/org/scion/jpan/PathMetadata.java | 264 ++++++++++++++++++ src/main/java/org/scion/jpan/PathPolicy.java | 10 +- src/main/java/org/scion/jpan/RequestPath.java | 235 +--------------- .../org/scion/jpan/ScionDatagramSocket.java | 3 +- src/main/java/org/scion/jpan/ScionUtil.java | 9 +- .../jpan/api/DatagramChannelApiTest.java | 17 +- .../scion/jpan/api/DatagramSocketApiTest.java | 7 +- .../org/scion/jpan/api/ScionServiceTest.java | 4 +- .../org/scion/jpan/demo/ScmpEchoDemo.java | 4 +- .../scion/jpan/demo/ScmpTracerouteDemo.java | 4 +- .../org/scion/jpan/demo/ShowpathsDemo.java | 4 +- 12 files changed, 305 insertions(+), 259 deletions(-) create mode 100644 src/main/java/org/scion/jpan/PathMetadata.java diff --git a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java index 3c3147bc2..4ccae991e 100644 --- a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java +++ b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java @@ -625,7 +625,8 @@ protected void buildHeader( protected RequestPath ensureUpToDate(RequestPath path) throws IOException { synchronized (stateLock) { - if (Instant.now().getEpochSecond() + cfgExpirationSafetyMargin <= path.getExpiration()) { + if (Instant.now().getEpochSecond() + cfgExpirationSafetyMargin + <= path.getMetadata().getExpiration()) { return path; } // expired, get new path diff --git a/src/main/java/org/scion/jpan/PathMetadata.java b/src/main/java/org/scion/jpan/PathMetadata.java new file mode 100644 index 000000000..7c436d7d4 --- /dev/null +++ b/src/main/java/org/scion/jpan/PathMetadata.java @@ -0,0 +1,264 @@ +// Copyright 2023 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 org.scion.jpan; + +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; +import org.scion.jpan.proto.daemon.Daemon; + +/** + * PathMetadata contains additional meta information such as bandwidth, latency or geo coordinates. + * PathMetadata is available from a RequestPaths are created/returned by the ScionService when + * requesting a new path from the control service. + */ +public class PathMetadata { + + private final Daemon.Path pathProtoc; + + static PathMetadata create(Daemon.Path path) { + return new PathMetadata(path); + } + + private PathMetadata(Daemon.Path path) { + this.pathProtoc = path; + } + + private Daemon.Path protoPath() { + if (pathProtoc == null) { + throw new IllegalStateException( + "Information is only available for paths that" + + " were retrieved directly from a path server."); + } + return pathProtoc; + } + + /** + * @return Interface for exiting the local AS using this path. + * @throws IllegalStateException if this is path is only a raw path + */ + public Interface getInterface() { + return new Interface(protoPath().getInterface()); + } + + /** + * @return The list of interfaces the path is composed of. + * @throws IllegalStateException if this is path is only a raw path + */ + public List getInterfacesList() { + return Collections.unmodifiableList( + protoPath().getInterfacesList().stream() + .map(PathInterface::new) + .collect(Collectors.toList())); + } + + /** + * @return The maximum transmission unit (MTU) on the path. + * @throws IllegalStateException if this is path is only a raw path + */ + public int getMtu() { + return protoPath().getMtu(); + } + + /** + * @return The point in time when this path expires. In seconds since UNIX epoch. + * @throws IllegalStateException if this is path is only a raw path + */ + public long getExpiration() { + return protoPath().getExpiration().getSeconds(); + } + + /** + * @return Latency lists the latencies between any two consecutive interfaces. Entry i describes + * the latency between interface i and i+1. Consequently, there are N-1 entries for N + * interfaces. A 0-value indicates that the AS did not announce a latency for this hop. + * @throws IllegalStateException if this is path is only a raw path + */ + public List getLatencyList() { + return Collections.unmodifiableList( + protoPath().getLatencyList().stream() + .map(time -> (int) (time.getSeconds() * 1_000 + time.getNanos() / 1_000_000)) + .collect(Collectors.toList())); + } + + /** + * @return Bandwidth lists the bandwidth between any two consecutive interfaces, in Kbit/s. Entry + * i describes the bandwidth between interfaces i and i+1. A 0-value indicates that the AS did + * not announce a bandwidth for this hop. + * @throws IllegalStateException if this is path is only a raw path + */ + public List getBandwidthList() { + return protoPath().getBandwidthList(); + } + + /** + * @return Geo lists the geographical position of the border routers along the path. Entry i + * describes the position of the router for interface i. A 0-value indicates that the AS did + * not announce a position for this router. + * @throws IllegalStateException if this is path is only a raw path + */ + public List getGeoList() { + return Collections.unmodifiableList( + protoPath().getGeoList().stream().map(GeoCoordinates::new).collect(Collectors.toList())); + } + + /** + * @return LinkType contains the announced link type of inter-domain links. Entry i describes the + * link between interfaces 2*i and 2*i+1. + * @throws IllegalStateException if this is path is only a raw path + */ + public List getLinkTypeList() { + return Collections.unmodifiableList( + protoPath().getLinkTypeList().stream() + .map(linkType -> LinkType.values()[linkType.getNumber()]) + .collect(Collectors.toList())); + } + + /** + * @return InternalHops lists the number of AS internal hops for the ASes on path. Entry i + * describes the hop between interfaces 2*i+1 and 2*i+2 in the same AS. Consequently, there + * are no entries for the first and last ASes, as these are not traversed completely by the + * path. + * @throws IllegalStateException if this is path is only a raw path + */ + public List getInternalHopsList() { + return protoPath().getInternalHopsList(); + } + + /** + * @return Notes contains the notes added by ASes on the path, in the order of occurrence. Entry i + * is the note of AS i on the path. + * @throws IllegalStateException if this is path is only a raw path + */ + public List getNotesList() { + return protoPath().getNotesList(); + } + + /** + * @return EpicAuths contains the EPIC authenticators used to calculate the PHVF and LHVF. + * @throws IllegalStateException if this is path is only a raw path + */ + public EpicAuths getEpicAuths() { + return new EpicAuths(protoPath().getEpicAuths()); + } + + public enum LinkType { + /** Unspecified link type. */ + LINK_TYPE_UNSPECIFIED, // = 0 + /** Direct physical connection. */ + LINK_TYPE_DIRECT, // = 1 + /** Connection with local routing/switching. */ + LINK_TYPE_MULTI_HOP, // = 2 + /** Connection overlayed over publicly routed Internet. */ + LINK_TYPE_OPEN_NET, // = 3 + } + + public static class EpicAuths { + private final byte[] authPhvf; + private final byte[] authLhvf; + + private EpicAuths(Daemon.EpicAuths epicAuths) { + this.authPhvf = epicAuths.getAuthPhvf().toByteArray(); + this.authLhvf = epicAuths.getAuthLhvf().toByteArray(); + } + + /** + * @return AuthPHVF is the authenticator use to calculate the PHVF. + */ + public byte[] getAuthPhvf() { + return authPhvf; + } + + /** + * @return AuthLHVF is the authenticator use to calculate the LHVF. + */ + public byte[] getAuthLhvf() { + return authLhvf; + } + } + + public static class Interface { + private final String address; + + private Interface(Daemon.Interface inter) { + this.address = inter.getAddress().getAddress(); + } + + /** + * @return Underlay address to exit through the interface. + */ + public String getAddress() { + return address; + } + } + + public static class PathInterface { + private final long isdAs; + + private final long id; + + private PathInterface(Daemon.PathInterface pathInterface) { + this.isdAs = pathInterface.getIsdAs(); + this.id = pathInterface.getId(); + } + + /** + * @return ISD-AS the interface belongs to. + */ + public long getIsdAs() { + return isdAs; + } + + /** + * @return ID of the interface in the AS. + */ + public long getId() { + return id; + } + } + + public static class GeoCoordinates { + private final float latitude; + private final float longitude; + private final String address; + + private GeoCoordinates(Daemon.GeoCoordinates geoCoordinates) { + this.latitude = geoCoordinates.getLatitude(); + this.longitude = geoCoordinates.getLongitude(); + this.address = geoCoordinates.getAddress(); + } + + /** + * @return Latitude of the geographic coordinate, in the WGS 84 datum. + */ + public float getLatitude() { + return latitude; + } + + /** + * @return Longitude of the geographic coordinate, in the WGS 84 datum. + */ + public float getLongitude() { + return longitude; + } + + /** + * @return Civic address of the location. + */ + public String getAddress() { + return address; + } + } +} diff --git a/src/main/java/org/scion/jpan/PathPolicy.java b/src/main/java/org/scion/jpan/PathPolicy.java index 454d5a839..bd27a593e 100644 --- a/src/main/java/org/scion/jpan/PathPolicy.java +++ b/src/main/java/org/scion/jpan/PathPolicy.java @@ -32,7 +32,7 @@ public RequestPath filter(List paths) { class MaxBandwith implements PathPolicy { public RequestPath filter(List paths) { return paths.stream() - .max(Comparator.comparing(path -> Collections.min(path.getBandwidthList()))) + .max(Comparator.comparing(path -> Collections.min(path.getMetadata().getBandwidthList()))) .orElseThrow(NoSuchElementException::new); } } @@ -45,7 +45,7 @@ public RequestPath filter(List paths) { .min( Comparator.comparing( path -> - path.getLatencyList().stream() + path.getMetadata().getLatencyList().stream() .mapToLong(l -> l > 0 ? l : Integer.MAX_VALUE) .reduce(0, Long::sum))) .orElseThrow(NoSuchElementException::new); @@ -55,7 +55,7 @@ public RequestPath filter(List paths) { class MinHopCount implements PathPolicy { public RequestPath filter(List paths) { return paths.stream() - .min(Comparator.comparing(path -> path.getInternalHopsList().size())) + .min(Comparator.comparing(path -> path.getMetadata().getInternalHopsList().size())) .orElseThrow(NoSuchElementException::new); } } @@ -76,7 +76,7 @@ public RequestPath filter(List paths) { } private boolean checkPath(RequestPath path) { - for (RequestPath.PathInterface pif : path.getInterfacesList()) { + for (PathMetadata.PathInterface pif : path.getMetadata().getInterfacesList()) { int isd = (int) (pif.getIsdAs() >>> 48); if (!allowedIsds.contains(isd)) { return false; @@ -102,7 +102,7 @@ public RequestPath filter(List paths) { } private boolean checkPath(RequestPath path) { - for (RequestPath.PathInterface pif : path.getInterfacesList()) { + for (PathMetadata.PathInterface pif : path.getMetadata().getInterfacesList()) { int isd = (int) (pif.getIsdAs() >>> 48); if (disallowedIsds.contains(isd)) { return false; diff --git a/src/main/java/org/scion/jpan/RequestPath.java b/src/main/java/org/scion/jpan/RequestPath.java index 9e6de75e0..6f11197e1 100644 --- a/src/main/java/org/scion/jpan/RequestPath.java +++ b/src/main/java/org/scion/jpan/RequestPath.java @@ -18,9 +18,6 @@ import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.UnknownHostException; -import java.util.Collections; -import java.util.List; -import java.util.stream.Collectors; import org.scion.jpan.internal.IPHelper; import org.scion.jpan.proto.daemon.Daemon; @@ -31,7 +28,7 @@ */ public class RequestPath extends Path { - private final Daemon.Path pathProtoc; + private final PathMetadata metadata; // We store the first hop separately to void creating unnecessary objects. private final InetSocketAddress firstHop; @@ -41,24 +38,15 @@ static RequestPath create(Daemon.Path path, long dstIsdAs, InetAddress dstIP, in private RequestPath(Daemon.Path path, long dstIsdAs, InetAddress dstIP, int dstPort) { super(path.getRaw().toByteArray(), dstIsdAs, dstIP, dstPort); - this.pathProtoc = path; + this.metadata = PathMetadata.create(path); if (getRawPath().length == 0) { // local AS has path length 0 firstHop = new InetSocketAddress(getRemoteAddress(), getRemotePort()); } else { - firstHop = getFirstHopAddress(pathProtoc); + firstHop = getFirstHopAddress(path); } } - private Daemon.Path protoPath() { - if (pathProtoc == null) { - throw new IllegalStateException( - "Information is only available for paths that" - + " were retrieved directly from a path server."); - } - return pathProtoc; - } - @Override public InetSocketAddress getFirstHopAddress() throws UnknownHostException { return firstHop; @@ -77,220 +65,7 @@ private InetSocketAddress getFirstHopAddress(Daemon.Path internalPath) { } } - /** - * @return Interface for exiting the local AS using this path. - * @throws IllegalStateException if this is path is only a raw path - */ - public Interface getInterface() { - return new Interface(protoPath().getInterface()); - } - - /** - * @return The list of interfaces the path is composed of. - * @throws IllegalStateException if this is path is only a raw path - */ - public List getInterfacesList() { - return Collections.unmodifiableList( - protoPath().getInterfacesList().stream() - .map(PathInterface::new) - .collect(Collectors.toList())); - } - - /** - * @return The maximum transmission unit (MTU) on the path. - * @throws IllegalStateException if this is path is only a raw path - */ - public int getMtu() { - return protoPath().getMtu(); - } - - /** - * @return The point in time when this path expires. In seconds since UNIX epoch. - * @throws IllegalStateException if this is path is only a raw path - */ - public long getExpiration() { - return protoPath().getExpiration().getSeconds(); - } - - /** - * @return Latency lists the latencies between any two consecutive interfaces. Entry i describes - * the latency between interface i and i+1. Consequently, there are N-1 entries for N - * interfaces. A 0-value indicates that the AS did not announce a latency for this hop. - * @throws IllegalStateException if this is path is only a raw path - */ - public List getLatencyList() { - return Collections.unmodifiableList( - protoPath().getLatencyList().stream() - .map(time -> (int) (time.getSeconds() * 1_000 + time.getNanos() / 1_000_000)) - .collect(Collectors.toList())); - } - - /** - * @return Bandwidth lists the bandwidth between any two consecutive interfaces, in Kbit/s. Entry - * i describes the bandwidth between interfaces i and i+1. A 0-value indicates that the AS did - * not announce a bandwidth for this hop. - * @throws IllegalStateException if this is path is only a raw path - */ - public List getBandwidthList() { - return protoPath().getBandwidthList(); - } - - /** - * @return Geo lists the geographical position of the border routers along the path. Entry i - * describes the position of the router for interface i. A 0-value indicates that the AS did - * not announce a position for this router. - * @throws IllegalStateException if this is path is only a raw path - */ - public List getGeoList() { - return Collections.unmodifiableList( - protoPath().getGeoList().stream().map(GeoCoordinates::new).collect(Collectors.toList())); - } - - /** - * @return LinkType contains the announced link type of inter-domain links. Entry i describes the - * link between interfaces 2*i and 2*i+1. - * @throws IllegalStateException if this is path is only a raw path - */ - public List getLinkTypeList() { - return Collections.unmodifiableList( - protoPath().getLinkTypeList().stream() - .map(linkType -> LinkType.values()[linkType.getNumber()]) - .collect(Collectors.toList())); - } - - /** - * @return InternalHops lists the number of AS internal hops for the ASes on path. Entry i - * describes the hop between interfaces 2*i+1 and 2*i+2 in the same AS. Consequently, there - * are no entries for the first and last ASes, as these are not traversed completely by the - * path. - * @throws IllegalStateException if this is path is only a raw path - */ - public List getInternalHopsList() { - return protoPath().getInternalHopsList(); - } - - /** - * @return Notes contains the notes added by ASes on the path, in the order of occurrence. Entry i - * is the note of AS i on the path. - * @throws IllegalStateException if this is path is only a raw path - */ - public List getNotesList() { - return protoPath().getNotesList(); - } - - /** - * @return EpicAuths contains the EPIC authenticators used to calculate the PHVF and LHVF. - * @throws IllegalStateException if this is path is only a raw path - */ - public EpicAuths getEpicAuths() { - return new EpicAuths(protoPath().getEpicAuths()); - } - - public enum LinkType { - /** Unspecified link type. */ - LINK_TYPE_UNSPECIFIED, // = 0 - /** Direct physical connection. */ - LINK_TYPE_DIRECT, // = 1 - /** Connection with local routing/switching. */ - LINK_TYPE_MULTI_HOP, // = 2 - /** Connection overlayed over publicly routed Internet. */ - LINK_TYPE_OPEN_NET, // = 3 - } - - public static class EpicAuths { - private final byte[] authPhvf; - private final byte[] authLhvf; - - private EpicAuths(Daemon.EpicAuths epicAuths) { - this.authPhvf = epicAuths.getAuthPhvf().toByteArray(); - this.authLhvf = epicAuths.getAuthLhvf().toByteArray(); - } - - /** - * @return AuthPHVF is the authenticator use to calculate the PHVF. - */ - public byte[] getAuthPhvf() { - return authPhvf; - } - - /** - * @return AuthLHVF is the authenticator use to calculate the LHVF. - */ - public byte[] getAuthLhvf() { - return authLhvf; - } - } - - public static class Interface { - private final String address; - - private Interface(Daemon.Interface inter) { - this.address = inter.getAddress().getAddress(); - } - - /** - * @return Underlay address to exit through the interface. - */ - public String getAddress() { - return address; - } - } - - public static class PathInterface { - private final long isdAs; - - private final long id; - - private PathInterface(Daemon.PathInterface pathInterface) { - this.isdAs = pathInterface.getIsdAs(); - this.id = pathInterface.getId(); - } - - /** - * @return ISD-AS the interface belongs to. - */ - public long getIsdAs() { - return isdAs; - } - - /** - * @return ID of the interface in the AS. - */ - public long getId() { - return id; - } - } - - public static class GeoCoordinates { - private final float latitude; - private final float longitude; - private final String address; - - private GeoCoordinates(Daemon.GeoCoordinates geoCoordinates) { - this.latitude = geoCoordinates.getLatitude(); - this.longitude = geoCoordinates.getLongitude(); - this.address = geoCoordinates.getAddress(); - } - - /** - * @return Latitude of the geographic coordinate, in the WGS 84 datum. - */ - public float getLatitude() { - return latitude; - } - - /** - * @return Longitude of the geographic coordinate, in the WGS 84 datum. - */ - public float getLongitude() { - return longitude; - } - - /** - * @return Civic address of the location. - */ - public String getAddress() { - return address; - } + public PathMetadata getMetadata() { + return metadata; } } diff --git a/src/main/java/org/scion/jpan/ScionDatagramSocket.java b/src/main/java/org/scion/jpan/ScionDatagramSocket.java index b4ccca3f1..c65483337 100644 --- a/src/main/java/org/scion/jpan/ScionDatagramSocket.java +++ b/src/main/java/org/scion/jpan/ScionDatagramSocket.java @@ -298,7 +298,8 @@ public void send(DatagramPacket packet) throws IOException { if (path == null) { path = channel.getPathPolicy().filter(channel.getOrCreateService2().getPaths(addr)); } else if (path instanceof RequestPath - && ((RequestPath) path).getExpiration() > Instant.now().getEpochSecond()) { + && ((RequestPath) path).getMetadata().getExpiration() + > Instant.now().getEpochSecond()) { // check expiration only for RequestPaths RequestPath request = (RequestPath) path; path = channel.getPathPolicy().filter(channel.getOrCreateService2().getPaths(request)); diff --git a/src/main/java/org/scion/jpan/ScionUtil.java b/src/main/java/org/scion/jpan/ScionUtil.java index 7d65f42f6..015e8454c 100644 --- a/src/main/java/org/scion/jpan/ScionUtil.java +++ b/src/main/java/org/scion/jpan/ScionUtil.java @@ -132,14 +132,15 @@ public static String toStringPath(byte[] raw) { * @return ISD/AS codes and border outer interface IDs along the path. */ public static String toStringPath(RequestPath path) { - if (path.getInterfacesList().isEmpty()) { + PathMetadata meta = path.getMetadata(); + if (meta.getInterfacesList().isEmpty()) { return "[]"; } StringBuilder sb = new StringBuilder(); sb.append("["); - int nInterfcaces = path.getInterfacesList().size(); + int nInterfcaces = meta.getInterfacesList().size(); for (int i = 0; i < nInterfcaces; i++) { - RequestPath.PathInterface pIf = path.getInterfacesList().get(i); + PathMetadata.PathInterface pIf = meta.getInterfacesList().get(i); if (i % 2 == 0) { sb.append(ScionUtil.toStringIA(pIf.getIsdAs())).append(" "); sb.append(pIf.getId()).append(">"); @@ -147,7 +148,7 @@ public static String toStringPath(RequestPath path) { sb.append(pIf.getId()).append(" "); } } - sb.append(ScionUtil.toStringIA(path.getInterfacesList().get(nInterfcaces - 1).getIsdAs())); + sb.append(ScionUtil.toStringIA(meta.getInterfacesList().get(nInterfcaces - 1).getIsdAs())); sb.append("]"); return sb.toString(); } diff --git a/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java b/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java index e174854e1..a010e4148 100644 --- a/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java +++ b/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java @@ -473,8 +473,9 @@ void send_disconnected_expiredRequestPath() throws IOException { ByteBuffer sendBuf = ByteBuffer.wrap(PingPongChannelHelper.MSG.getBytes()); try { RequestPath newPath = (RequestPath) channel.send(sendBuf, expiredPath); - assertTrue(newPath.getExpiration() > expiredPath.getExpiration()); - assertTrue(Instant.now().getEpochSecond() < newPath.getExpiration()); + assertTrue( + newPath.getMetadata().getExpiration() > expiredPath.getMetadata().getExpiration()); + assertTrue(Instant.now().getEpochSecond() < newPath.getMetadata().getExpiration()); assertNull(channel.getConnectionPath()); } catch (IOException e) { throw new RuntimeException(e); @@ -491,8 +492,9 @@ void send_connected_expiredRequestPath() throws IOException { ByteBuffer sendBuf = ByteBuffer.wrap(PingPongChannelHelper.MSG.getBytes()); try { RequestPath newPath = (RequestPath) channel.send(sendBuf, expiredPath); - assertTrue(newPath.getExpiration() > expiredPath.getExpiration()); - assertTrue(Instant.now().getEpochSecond() < newPath.getExpiration()); + assertTrue( + newPath.getMetadata().getExpiration() > expiredPath.getMetadata().getExpiration()); + assertTrue(Instant.now().getEpochSecond() < newPath.getMetadata().getExpiration()); assertEquals(newPath, channel.getConnectionPath()); } catch (IOException e) { throw new RuntimeException(e); @@ -510,8 +512,9 @@ void write_expiredRequestPath() throws IOException { try { channel.write(sendBuf); RequestPath newPath = (RequestPath) channel.getConnectionPath(); - assertTrue(newPath.getExpiration() > expiredPath.getExpiration()); - assertTrue(Instant.now().getEpochSecond() < newPath.getExpiration()); + assertTrue( + newPath.getMetadata().getExpiration() > expiredPath.getMetadata().getExpiration()); + assertTrue(Instant.now().getEpochSecond() < newPath.getMetadata().getExpiration()); } catch (IOException e) { throw new RuntimeException(e); } @@ -552,7 +555,7 @@ private RequestPath createExpiredPath(Path basePath) throws UnknownHostException basePath.getRemoteAddress(), basePath.getRemotePort(), basePath.getFirstHopAddress()); - assertTrue(Instant.now().getEpochSecond() > expiredPath.getExpiration()); + assertTrue(Instant.now().getEpochSecond() > expiredPath.getMetadata().getExpiration()); return expiredPath; } diff --git a/src/test/java/org/scion/jpan/api/DatagramSocketApiTest.java b/src/test/java/org/scion/jpan/api/DatagramSocketApiTest.java index f43564973..0d9888b95 100644 --- a/src/test/java/org/scion/jpan/api/DatagramSocketApiTest.java +++ b/src/test/java/org/scion/jpan/api/DatagramSocketApiTest.java @@ -618,8 +618,9 @@ void send_connected_expiredRequestPath() throws IOException { try { socket.send(packet); RequestPath newPath = socket.getConnectionPath(); - assertTrue(newPath.getExpiration() > expiredPath.getExpiration()); - assertTrue(Instant.now().getEpochSecond() < newPath.getExpiration()); + assertTrue( + newPath.getMetadata().getExpiration() > expiredPath.getMetadata().getExpiration()); + assertTrue(Instant.now().getEpochSecond() < newPath.getMetadata().getExpiration()); // assertNull(channel.getCurrentPath()); } catch (IOException e) { throw new RuntimeException(e); @@ -660,7 +661,7 @@ private RequestPath createExpiredPath(Path basePath) throws UnknownHostException basePath.getRemoteAddress(), basePath.getRemotePort(), basePath.getFirstHopAddress()); - assertTrue(Instant.now().getEpochSecond() > expiredPath.getExpiration()); + assertTrue(Instant.now().getEpochSecond() > expiredPath.getMetadata().getExpiration()); return expiredPath; } diff --git a/src/test/java/org/scion/jpan/api/ScionServiceTest.java b/src/test/java/org/scion/jpan/api/ScionServiceTest.java index 011127ab1..f952067dd 100644 --- a/src/test/java/org/scion/jpan/api/ScionServiceTest.java +++ b/src/test/java/org/scion/jpan/api/ScionServiceTest.java @@ -125,8 +125,8 @@ void getPath() throws IOException { assertEquals(dstIA, path.getRemoteIsdAs()); assertEquals(36, path.getRawPath().length); - assertEquals("127.0.0.10:31004", path.getInterface().getAddress()); - assertEquals(2, path.getInterfacesList().size()); + assertEquals("127.0.0.10:31004", path.getMetadata().getInterface().getAddress()); + assertEquals(2, path.getMetadata().getInterfacesList().size()); // assertEquals(1, viewer.getInternalHopsList().size()); // assertEquals(0, viewer.getMtu()); // assertEquals(0, viewer.getLinkTypeList().size()); diff --git a/src/test/java/org/scion/jpan/demo/ScmpEchoDemo.java b/src/test/java/org/scion/jpan/demo/ScmpEchoDemo.java index b85d2dcec..2f85bebe8 100644 --- a/src/test/java/org/scion/jpan/demo/ScmpEchoDemo.java +++ b/src/test/java/org/scion/jpan/demo/ScmpEchoDemo.java @@ -149,8 +149,8 @@ private void printPath(RequestPath path) { // ").append(channel.getLocalAddress().getAddress().getHostAddress()).append(nl); sb.append("Using path:").append(nl); sb.append(" Hops: ").append(ScionUtil.toStringPath(path)); - sb.append(" MTU: ").append(path.getMtu()); - sb.append(" NextHop: ").append(path.getInterface().getAddress()).append(nl); + sb.append(" MTU: ").append(path.getMetadata().getMtu()); + sb.append(" NextHop: ").append(path.getMetadata().getInterface().getAddress()).append(nl); println(sb.toString()); } diff --git a/src/test/java/org/scion/jpan/demo/ScmpTracerouteDemo.java b/src/test/java/org/scion/jpan/demo/ScmpTracerouteDemo.java index db8d230d9..19bb11d0a 100644 --- a/src/test/java/org/scion/jpan/demo/ScmpTracerouteDemo.java +++ b/src/test/java/org/scion/jpan/demo/ScmpTracerouteDemo.java @@ -129,8 +129,8 @@ private void printPath(RequestPath path) { // sb.append(" ").append(channel.getLocalAddress().getAddress().getHostAddress()).append(nl); sb.append("Using path:").append(nl); sb.append(" Hops: ").append(ScionUtil.toStringPath(path)); - sb.append(" MTU: ").append(path.getMtu()); - sb.append(" NextHop: ").append(path.getInterface().getAddress()).append(nl); + sb.append(" MTU: ").append(path.getMetadata().getMtu()); + sb.append(" NextHop: ").append(path.getMetadata().getInterface().getAddress()).append(nl); println(sb.toString()); } diff --git a/src/test/java/org/scion/jpan/demo/ShowpathsDemo.java b/src/test/java/org/scion/jpan/demo/ShowpathsDemo.java index 9510b54ca..1af0ea01e 100644 --- a/src/test/java/org/scion/jpan/demo/ShowpathsDemo.java +++ b/src/test/java/org/scion/jpan/demo/ShowpathsDemo.java @@ -114,9 +114,9 @@ private void runDemo(long destinationIA) throws IOException { + "] Hops: " + ScionUtil.toStringPath(path) + " MTU: " - + path.getMtu() + + path.getMetadata().getMtu() + " NextHop: " - + path.getInterface().getAddress() + + path.getMetadata().getInterface().getAddress() + " LocalIP: " + localIP; println(sb); From e2317169e2ff5400e753b19ab4179a4d341cfc94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilmann=20Z=C3=A4schke?= Date: Wed, 5 Jun 2024 13:48:51 +0200 Subject: [PATCH 02/13] Metadata --- CHANGELOG.md | 7 ++++ .../scion/jpan/AbstractDatagramChannel.java | 32 ++++++++-------- src/main/java/org/scion/jpan/Path.java | 38 +++++++++++++------ .../org/scion/jpan/ScionDatagramChannel.java | 4 +- .../org/scion/jpan/ScionDatagramSocket.java | 2 +- src/main/java/org/scion/jpan/ScmpChannel.java | 2 +- 6 files changed, 54 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 71e060ea7..91c1f6be9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Rename `DatagramSocket` to `ScionDatagramSopcketl` and move it to main package - Rename `ScionSocketOptions` starting with `SN_` to `SCION_` - SCMP API changes. [#71](https://github.com/scionproto-contrib/jpan/pull/71) +- Make `Path` inherit `InetSocketAddress` [#69](https://github.com/scionproto-contrib/jpan/pull/69) + +TODO +- Revert metadata being a separate class? go-SNET also has a separate class... +- Make Path fully immutable? +- Move design doc from Path javadoc to Design.md +- return value of send()? Separate PR? ### Fixed - Fixed locking and resizing of buffers. [#68](https://github.com/scionproto-contrib/jpan/pull/68) diff --git a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java index 4ccae991e..784010006 100644 --- a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java +++ b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java @@ -179,16 +179,14 @@ public InetSocketAddress getLocalAddress() throws IOException { /** * Returns the remote address. * + * @return The remote address or 'null' if this channel is not connected. * @see DatagramChannel#getRemoteAddress() - * @return The remote address. + * @see #connect(SocketAddress) + * @see #connect(RequestPath) * @throws IOException If an I/O error occurs */ - public InetSocketAddress getRemoteAddress() throws IOException { - Path path = getConnectionPath(); - if (path != null) { - return new InetSocketAddress(path.getRemoteAddress(), path.getRemotePort()); - } - return null; + public RequestPath getRemoteAddress() throws IOException { + return connectionPath; } public void disconnect() throws IOException { @@ -214,12 +212,15 @@ public void close() throws IOException { } /** - * Connect to a destination host. Note: - A SCION channel will internally connect to the next - * border router (first hop) instead of the remote host. + * Connect to a destination host. + * + *

NB: A SCION channel will internally connect to the next border router (first hop) instead of + * the remote host. * - *

NB: This method does internally no call {@link java.nio.channels.DatagramChannel}.connect(), - * instead it calls bind(). That means this method does NOT perform any additional security checks - * associated with connect(), only those associated with bind(). + *

NB: This method does internally not call {@link + * java.nio.channels.DatagramChannel}.connect(), instead it calls bind(). That means this method + * does NOT perform any additional security checks associated with connect(), only those + * associated with bind(). * * @param addr Address of remote host. * @return This channel. @@ -574,6 +575,7 @@ protected void buildHeader( srcIA = rPath.getLocalIsdAs(); srcAddress = rPath.getLocalAddress(); srcPort = rPath.getLocalPort(); + // TODO do we need to verify that the srcAddress is equal to the used local address? } else { srcIA = getOrCreateService().getLocalIsdAs(); if (overrideExternalAddress != null) { @@ -610,14 +612,14 @@ protected void buildHeader( rawPath.length, srcIA, srcAddress.getAddress(), - path.getRemoteIsdAs(), - path.getRemoteAddress().getAddress(), + path.getIsdAs(), + path.getAddress().getAddress(), hdrType, cfgTrafficClass); ScionHeaderParser.writePath(buffer, rawPath); if (hdrType == InternalConstants.HdrTypes.UDP) { - int dstPort = path.getRemotePort(); + int dstPort = path.getPort(); ScionHeaderParser.writeUdpOverlayHeader(buffer, payloadLength, srcPort, dstPort); } } diff --git a/src/main/java/org/scion/jpan/Path.java b/src/main/java/org/scion/jpan/Path.java index 898619cec..71f8ebcd8 100644 --- a/src/main/java/org/scion/jpan/Path.java +++ b/src/main/java/org/scion/jpan/Path.java @@ -21,18 +21,24 @@ * A Path is an InetSocketAddress/ISD/AS of a destination host plus a path to that host. * *

This class is thread safe. + * + *

Design considerations:
+ * - Having Path subclass InetSocketAddress may feel a bit awkward, not least because + * getAddress()/getPort() are not immediately clear to refer to the remote host. However, + * subclassing InetSocketAddress allows paths to be returned by DatagramChannel.receive(), which + * would otherwise not be possible.
+ * - Having two sublasses of Path ensures that RequestPaths and ResponsePath are never mixed up.
+ * - The design also allows immutability, and thus thread safety.
+ * - Having metadata in a separate class makes the API cleaner. */ -public abstract class Path { +public abstract class Path extends InetSocketAddress { private final byte[] pathRaw; private final long dstIsdAs; - private final InetAddress dstAddress; - private final int dstPort; protected Path(byte[] rawPath, long dstIsdAs, InetAddress dstIP, int dstPort) { + super(dstIP, dstPort); this.pathRaw = rawPath; this.dstIsdAs = dstIsdAs; - this.dstAddress = dstIP; - this.dstPort = dstPort; } public byte[] getRawPath() { @@ -41,27 +47,35 @@ public byte[] getRawPath() { public abstract InetSocketAddress getFirstHopAddress() throws UnknownHostException; + @Deprecated // Use getPort() instead public int getRemotePort() { - return dstPort; + return getPort(); } + @Deprecated // Use getAddress() instead public InetAddress getRemoteAddress() { - return dstAddress; + return getAddress(); } + @Deprecated // Use getIsdAs() instead public long getRemoteIsdAs() { return dstIsdAs; } + /** + * @return ISD/AS of the remote host. + */ + public long getIsdAs() { + return dstIsdAs; + } + @Override public String toString() { return "Path{" - + "rmtIsdAs=" + + "ISD/AS=" + ScionUtil.toStringIA(dstIsdAs) - + ", rmtAddress=" - + dstAddress - + ", rmtPort=" - + dstPort + + ", address=" + + super.toString() + ", pathRaw=" + Arrays.toString(pathRaw) + '}'; diff --git a/src/main/java/org/scion/jpan/ScionDatagramChannel.java b/src/main/java/org/scion/jpan/ScionDatagramChannel.java index 8156d4c22..12f6cae79 100644 --- a/src/main/java/org/scion/jpan/ScionDatagramChannel.java +++ b/src/main/java/org/scion/jpan/ScionDatagramChannel.java @@ -169,11 +169,11 @@ public int write(ByteBuffer src) throws IOException { ByteBuffer buffer = getBufferSend(src.remaining()); int len = src.remaining(); // + 8 for UDP overlay header length - checkPathAndBuildHeader(buffer, getConnectionPath(), len + 8, InternalConstants.HdrTypes.UDP); + checkPathAndBuildHeader(buffer, getRemoteAddress(), len + 8, InternalConstants.HdrTypes.UDP); buffer.put(src); buffer.flip(); - int sent = sendRaw(buffer, getConnectionPath().getFirstHopAddress(), getConnectionPath()); + int sent = sendRaw(buffer, getRemoteAddress().getFirstHopAddress(), getRemoteAddress()); if (sent < buffer.limit() || buffer.remaining() > 0) { throw new ScionException("Failed to send all data."); } diff --git a/src/main/java/org/scion/jpan/ScionDatagramSocket.java b/src/main/java/org/scion/jpan/ScionDatagramSocket.java index c65483337..ef0b1910b 100644 --- a/src/main/java/org/scion/jpan/ScionDatagramSocket.java +++ b/src/main/java/org/scion/jpan/ScionDatagramSocket.java @@ -290,7 +290,7 @@ public void send(DatagramPacket packet) throws IOException { Path path; if (channel.isConnected()) { - path = channel.getConnectionPath(); + path = channel.getRemoteAddress(); } else { InetSocketAddress addr = (InetSocketAddress) packet.getSocketAddress(); synchronized (pathCache) { diff --git a/src/main/java/org/scion/jpan/ScmpChannel.java b/src/main/java/org/scion/jpan/ScmpChannel.java index 458c63faa..cec5f7a0e 100644 --- a/src/main/java/org/scion/jpan/ScmpChannel.java +++ b/src/main/java/org/scion/jpan/ScmpChannel.java @@ -407,7 +407,7 @@ public InetSocketAddress getLocalAddress() throws IOException { return channel.getLocalAddress(); } - public InetSocketAddress getRemoteAddress() throws IOException { + public RequestPath getRemoteAddress() throws IOException { return channel.getRemoteAddress(); } From 64e12c3757f93a1553e5dc2d092480b6914c5d9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilmann=20Z=C3=A4schke?= Date: Wed, 5 Jun 2024 13:49:40 +0200 Subject: [PATCH 03/13] Metadata --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91c1f6be9..31c7a596e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] -## Added +### Added - SCMP echo responder [#78](https://github.com/scionproto-contrib/jpan/pull/78) - Maven Java executor [#80](https://github.com/scionproto-contrib/jpan/pull/80) - Dev environment setup hints doc [#82](https://github.com/scionproto-contrib/jpan/pull/82) From ecf0c03bfde27a58c13366424fd3f01532d37a96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilmann=20Z=C3=A4schke?= Date: Wed, 5 Jun 2024 15:14:55 +0200 Subject: [PATCH 04/13] getConnectionPath() clean up --- CHANGELOG.md | 2 ++ .../scion/jpan/AbstractDatagramChannel.java | 2 ++ .../org/scion/jpan/ScionDatagramSocket.java | 2 ++ src/main/java/org/scion/jpan/ScmpChannel.java | 2 ++ .../scion/jpan/api/DatagramChannelApiTest.java | 18 +++++++++--------- .../api/DatagramChannelErrorHandlingTest.java | 4 ++-- .../scion/jpan/demo/PingPongChannelClient.java | 2 +- 7 files changed, 20 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 31c7a596e..93ce07d11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,12 +20,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Rename `ScionSocketOptions` starting with `SN_` to `SCION_` - SCMP API changes. [#71](https://github.com/scionproto-contrib/jpan/pull/71) - Make `Path` inherit `InetSocketAddress` [#69](https://github.com/scionproto-contrib/jpan/pull/69) + **BREAKING CHANGE**: Access to path metadata has been moved to `Path.getMetadata()` TODO - Revert metadata being a separate class? go-SNET also has a separate class... - Make Path fully immutable? - Move design doc from Path javadoc to Design.md - return value of send()? Separate PR? +- deprecate/remove getConnectedPath() ### Fixed - Fixed locking and resizing of buffers. [#68](https://github.com/scionproto-contrib/jpan/pull/68) diff --git a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java index 784010006..e84598046 100644 --- a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java +++ b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java @@ -301,7 +301,9 @@ public C connect(RequestPath path) throws IOException { * #connect(RequestPath)} and may be refreshed when expired. * * @return the current Path or `null` if not path is connected. + * @deprecated Please use getRemoteAddress() instead */ + @Deprecated public Path getConnectionPath() { synchronized (stateLock) { return connectionPath; diff --git a/src/main/java/org/scion/jpan/ScionDatagramSocket.java b/src/main/java/org/scion/jpan/ScionDatagramSocket.java index ef0b1910b..5116c3d94 100644 --- a/src/main/java/org/scion/jpan/ScionDatagramSocket.java +++ b/src/main/java/org/scion/jpan/ScionDatagramSocket.java @@ -533,7 +533,9 @@ public void leaveGroup(SocketAddress mcastaddr, NetworkInterface netIf) { * * @return the current Path or `null` if not path is connected. * @see ScionDatagramChannel#getConnectionPath() + * @deprecated Please use getRemoteAddress() instead */ + @Deprecated public RequestPath getConnectionPath() { return (RequestPath) channel.getConnectionPath(); } diff --git a/src/main/java/org/scion/jpan/ScmpChannel.java b/src/main/java/org/scion/jpan/ScmpChannel.java index cec5f7a0e..6dd7b3c90 100644 --- a/src/main/java/org/scion/jpan/ScmpChannel.java +++ b/src/main/java/org/scion/jpan/ScmpChannel.java @@ -398,7 +398,9 @@ public void close() throws IOException { * * @return the current Path * @see ScionDatagramChannel#getConnectionPath() + * @deprecated Please use getRemoteAddress() instead */ + @Deprecated public RequestPath getConnectionPath() { return (RequestPath) channel.getConnectionPath(); } diff --git a/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java b/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java index a010e4148..1ecfd3ee5 100644 --- a/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java +++ b/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java @@ -133,7 +133,7 @@ void getLocalAddress_withSendResponsePath() throws IOException { InetSocketAddress local = channel.getLocalAddress(); assertTrue(local.getAddress().isAnyLocalAddress()); // double check that we used a responsePath - assertNull(channel.getConnectionPath()); + assertNull(channel.getRemoteAddress()); } } @@ -476,7 +476,7 @@ void send_disconnected_expiredRequestPath() throws IOException { assertTrue( newPath.getMetadata().getExpiration() > expiredPath.getMetadata().getExpiration()); assertTrue(Instant.now().getEpochSecond() < newPath.getMetadata().getExpiration()); - assertNull(channel.getConnectionPath()); + assertNull(channel.getRemoteAddress()); } catch (IOException e) { throw new RuntimeException(e); } @@ -495,7 +495,7 @@ void send_connected_expiredRequestPath() throws IOException { assertTrue( newPath.getMetadata().getExpiration() > expiredPath.getMetadata().getExpiration()); assertTrue(Instant.now().getEpochSecond() < newPath.getMetadata().getExpiration()); - assertEquals(newPath, channel.getConnectionPath()); + assertEquals(newPath, channel.getRemoteAddress()); } catch (IOException e) { throw new RuntimeException(e); } @@ -511,7 +511,7 @@ void write_expiredRequestPath() throws IOException { ByteBuffer sendBuf = ByteBuffer.wrap(PingPongChannelHelper.MSG.getBytes()); try { channel.write(sendBuf); - RequestPath newPath = (RequestPath) channel.getConnectionPath(); + RequestPath newPath = channel.getRemoteAddress(); assertTrue( newPath.getMetadata().getExpiration() > expiredPath.getMetadata().getExpiration()); assertTrue(Instant.now().getEpochSecond() < newPath.getMetadata().getExpiration()); @@ -564,22 +564,22 @@ void getConnectionPath() throws IOException { RequestPath addr = ExamplePacket.PATH; ByteBuffer buffer = ByteBuffer.allocate(50); try (ScionDatagramChannel channel = ScionDatagramChannel.open()) { - assertNull(channel.getConnectionPath()); + assertNull(channel.getRemoteAddress()); // send should NOT set a path channel.send(buffer, addr); - assertNull(channel.getConnectionPath()); + assertNull(channel.getRemoteAddress()); // connect should set a path channel.connect(addr); - assertNotNull(channel.getConnectionPath()); + assertNotNull(channel.getRemoteAddress()); channel.disconnect(); - assertNull(channel.getConnectionPath()); + assertNull(channel.getRemoteAddress()); // send should NOT set a path if (Util.getJavaMajorVersion() >= 14) { // This fails because of disconnect(), see https://bugs.openjdk.org/browse/JDK-8231880 channel.send(buffer, addr); - assertNull(channel.getConnectionPath()); + assertNull(channel.getRemoteAddress()); } } } diff --git a/src/test/java/org/scion/jpan/api/DatagramChannelErrorHandlingTest.java b/src/test/java/org/scion/jpan/api/DatagramChannelErrorHandlingTest.java index 3e2f85054..4c6064076 100644 --- a/src/test/java/org/scion/jpan/api/DatagramChannelErrorHandlingTest.java +++ b/src/test/java/org/scion/jpan/api/DatagramChannelErrorHandlingTest.java @@ -58,12 +58,12 @@ void testErrorHandling() throws IOException { RequestPath path1 = paths.get(0); channel.connect(path0); channel.write(ByteBuffer.allocate(0)); - assertEquals(path0, channel.getConnectionPath()); + assertEquals(path0, channel.getRemoteAddress()); // TODO Use mock instead of daemon? MockNetwork.returnScmpErrorOnNextPacket(Scmp.TypeCode.TYPE_5); channel.write(ByteBuffer.allocate(0)); - assertEquals(path0, channel.getConnectionPath()); + assertEquals(path0, channel.getRemoteAddress()); assertEquals(1, scmpReceived.get()); // mock.setSendCallback((byteBuffer,path) -> {}); diff --git a/src/test/java/org/scion/jpan/demo/PingPongChannelClient.java b/src/test/java/org/scion/jpan/demo/PingPongChannelClient.java index 765986ef4..904dde083 100644 --- a/src/test/java/org/scion/jpan/demo/PingPongChannelClient.java +++ b/src/test/java/org/scion/jpan/demo/PingPongChannelClient.java @@ -53,7 +53,7 @@ private static void run() throws IOException { channel.write(sendBuf); println( "Sent via " - + ScionUtil.toStringPath((RequestPath) channel.getConnectionPath()) + + ScionUtil.toStringPath((RequestPath) channel.getRemoteAddress()) + ": " + msg); From 7c463f6e4279ae77189e7ae557c6746082a8372f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilmann=20Z=C3=A4schke?= Date: Wed, 5 Jun 2024 17:33:21 +0200 Subject: [PATCH 05/13] getConnectionPath() clean up --- CHANGELOG.md | 8 +++- .../scion/jpan/AbstractDatagramChannel.java | 42 +++++++++++++++---- src/main/java/org/scion/jpan/RequestPath.java | 34 +++++++++++++++ .../org/scion/jpan/ScionDatagramChannel.java | 14 ++++--- 4 files changed, 82 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93ce07d11..9d9ab509e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,14 +20,18 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Rename `ScionSocketOptions` starting with `SN_` to `SCION_` - SCMP API changes. [#71](https://github.com/scionproto-contrib/jpan/pull/71) - Make `Path` inherit `InetSocketAddress` [#69](https://github.com/scionproto-contrib/jpan/pull/69) - **BREAKING CHANGE**: Access to path metadata has been moved to `Path.getMetadata()` + - **BREAKING CHANGE**: Access to path metadata has been moved to `Path.getMetadata()`. + - **BREAKING CHANGE**: ScionDatagramChannel.send(buffer, path) returns 'int'. TODO - Revert metadata being a separate class? go-SNET also has a separate class... - Make Path fully immutable? + --> How do we refresh Paths? How do we return refreshed paths from send()? + --> Maybe not return, but cache refreshed path, either in Path itself (weird?) or + in ScionService? +- Move expiryMargin to ScionService? - Move design doc from Path javadoc to Design.md - return value of send()? Separate PR? -- deprecate/remove getConnectedPath() ### Fixed - Fixed locking and resizing of buffers. [#68](https://github.com/scionproto-contrib/jpan/pull/68) diff --git a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java index e84598046..7082bb1de 100644 --- a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java +++ b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java @@ -629,17 +629,43 @@ protected void buildHeader( protected RequestPath ensureUpToDate(RequestPath path) throws IOException { synchronized (stateLock) { - if (Instant.now().getEpochSecond() + cfgExpirationSafetyMargin - <= path.getMetadata().getExpiration()) { - return path; - } - // expired, get new path - RequestPath newPath = pathPolicy.filter(getOrCreateService().getPaths(path)); - if (isConnected()) { - updateConnection(newPath, true); + // TODO remove +// if (Instant.now().getEpochSecond() + cfgExpirationSafetyMargin +// <= path.getMetadata().getExpiration()) { +// return path; +// } +// // expired, get new path +// RequestPath newPath = pathPolicy.filter(getOrCreateService().getPaths(path)); +// if (isConnected()) { +// updateConnection(newPath, true); +// } +// return newPath; + + // TODO ensure that refresh always heappens, e.g. in v1 we call in inside send().... + RequestPath newPath = refresh(path); + if (path != refresh(path) && isConnected()) { + // TODO move this `if` into refresh()? + updateConnection(path, true); } return newPath; + +// +// // TODO this assumes that the part modifies itself! +// if (path.refreshPath(getOrCreateService(), pathPolicy, cfgExpirationSafetyMargin)) { +// if (isConnected()) { +// updateConnection(path, true); +// } +// } + + } + } + + private RequestPath refresh(RequestPath path) { + if (Instant.now().getEpochSecond() + cfgExpirationSafetyMargin <= path.getMetadata().getExpiration()) { + return path; } + // expired, get new path + return pathPolicy.filter(service.getPaths(path)); } private void updateConnection(RequestPath newPath, boolean mustBeConnected) throws IOException { diff --git a/src/main/java/org/scion/jpan/RequestPath.java b/src/main/java/org/scion/jpan/RequestPath.java index 6f11197e1..a54327f87 100644 --- a/src/main/java/org/scion/jpan/RequestPath.java +++ b/src/main/java/org/scion/jpan/RequestPath.java @@ -14,10 +14,13 @@ package org.scion.jpan; +import java.io.IOException; import java.io.UncheckedIOException; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.UnknownHostException; +import java.time.Instant; + import org.scion.jpan.internal.IPHelper; import org.scion.jpan.proto.daemon.Daemon; @@ -68,4 +71,35 @@ private InetSocketAddress getFirstHopAddress(Daemon.Path internalPath) { public PathMetadata getMetadata() { return metadata; } + +// /** +// * @param service Service for obtaining new paths when required +// * @param pathPolicy PathPolicy for selecting a new path when required +// * @param expiryMargin Expiry margin, i.e. a path is considered "expired" if expiry is less than +// * expiryMargin seconds away. +// * @return `false` if the path is `null`, it it is a ResponsePath or if it didn't need updating. +// * Returns `true` only if the path was updated. +// */ +// synchronized boolean refreshPath(ScionService service, PathPolicy pathPolicy, int expiryMargin) +// throws IOException { +//// if (path == null) { +//// path = pathPolicy.filter(service.getPaths(this)); +//// if (path == null) { +//// throw new IOException("Address is not resolvable in SCION: " + super.toString()); +//// } +//// return true; +//// } +//// +//// if (!(path instanceof RequestPath)) { +//// return false; +//// } +//// +//// RequestPath requestPath = (RequestPath) path; +// if (Instant.now().getEpochSecond() + expiryMargin <= getMetadata().getExpiration()) { +// return false; +// } +// // expired, get new path +// path = pathPolicy.filter(service.getPaths(requestPath)); +// return true; +// } } diff --git a/src/main/java/org/scion/jpan/ScionDatagramChannel.java b/src/main/java/org/scion/jpan/ScionDatagramChannel.java index 12f6cae79..3ff1d8774 100644 --- a/src/main/java/org/scion/jpan/ScionDatagramChannel.java +++ b/src/main/java/org/scion/jpan/ScionDatagramChannel.java @@ -84,20 +84,23 @@ public ResponsePath receive(ByteBuffer userBuffer) throws IOException { * cannot be resolved to an ISD/AS. * @see java.nio.channels.DatagramChannel#send(ByteBuffer, SocketAddress) */ - public void send(ByteBuffer srcBuffer, SocketAddress destination) throws IOException { + public int send(ByteBuffer srcBuffer, SocketAddress destination) throws IOException { if (!(destination instanceof InetSocketAddress)) { throw new IllegalArgumentException("Address must be of type InetSocketAddress."); } + if (destination instanceof Path) { + return send(srcBuffer, (Path) destination); + } InetSocketAddress dst = (InetSocketAddress) destination; Path path = getPathPolicy().filter(getOrCreateService().getPaths(dst)); - send(srcBuffer, path); + return sendPath(srcBuffer, path); } /** * Attempts to send the content of the buffer to the destinationAddress. * * @param srcBuffer Data to send - * @param path Path to destination. If this is a Request path and it is expired then it will + * @param path Path to destination. If this is a RequestPath, and it is expired, then it will * automatically be replaced with a new path. Expiration of ResponsePaths is not checked * @return either the path argument or a new path if the path was an expired RequestPath. Note * that ResponsePaths are not checked for expiration. @@ -105,7 +108,7 @@ public void send(ByteBuffer srcBuffer, SocketAddress destination) throws IOExcep * cannot be resolved to an ISD/AS. * @see java.nio.channels.DatagramChannel#send(ByteBuffer, SocketAddress) */ - public Path send(ByteBuffer srcBuffer, Path path) throws IOException { + public int sendPath(ByteBuffer srcBuffer, Path path) throws IOException { writeLock().lock(); try { ByteBuffer buffer = getBufferSend(srcBuffer.remaining()); @@ -119,8 +122,7 @@ public Path send(ByteBuffer srcBuffer, Path path) throws IOException { throw new IOException("Packet is larger than max send buffer size."); } buffer.flip(); - sendRaw(buffer, actualPath.getFirstHopAddress(), actualPath); - return actualPath; + return sendRaw(buffer, actualPath.getFirstHopAddress(), actualPath); } finally { writeLock().unlock(); } From 0458d893b6d79487dbdce0617d1f5ef8c5a8eed1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilmann=20Z=C3=A4schke?= Date: Fri, 7 Jun 2024 16:59:35 +0200 Subject: [PATCH 06/13] getConnectionPath() clean up --- CHANGELOG.md | 9 ++- .../scion/jpan/AbstractDatagramChannel.java | 55 ++------------ src/main/java/org/scion/jpan/Path.java | 17 ++--- .../{PathMetadata.java => PathDetails.java} | 50 ++++++++++-- src/main/java/org/scion/jpan/PathPolicy.java | 10 +-- src/main/java/org/scion/jpan/RequestPath.java | 76 ++++--------------- .../java/org/scion/jpan/ResponsePath.java | 11 ++- .../org/scion/jpan/ScionDatagramChannel.java | 14 ++-- .../org/scion/jpan/ScionDatagramSocket.java | 2 +- .../java/org/scion/jpan/ScionService.java | 18 +++++ src/main/java/org/scion/jpan/ScionUtil.java | 4 +- .../api/DatagramChannelApiServerTest.java | 3 +- .../jpan/api/DatagramChannelApiTest.java | 32 ++++---- .../scion/jpan/api/DatagramSocketApiTest.java | 6 +- .../org/scion/jpan/api/ScionServiceTest.java | 4 +- .../org/scion/jpan/demo/ScmpEchoDemo.java | 4 +- .../scion/jpan/demo/ScmpTracerouteDemo.java | 4 +- .../org/scion/jpan/demo/ShowpathsDemo.java | 4 +- 18 files changed, 154 insertions(+), 169 deletions(-) rename src/main/java/org/scion/jpan/{PathMetadata.java => PathDetails.java} (80%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d9ab509e..da780cb8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,11 +20,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Rename `ScionSocketOptions` starting with `SN_` to `SCION_` - SCMP API changes. [#71](https://github.com/scionproto-contrib/jpan/pull/71) - Make `Path` inherit `InetSocketAddress` [#69](https://github.com/scionproto-contrib/jpan/pull/69) - - **BREAKING CHANGE**: Access to path metadata has been moved to `Path.getMetadata()`. + - **BREAKING CHANGE**: Access to path details such as metadata has been moved to + `Path.getDetails()`. - **BREAKING CHANGE**: ScionDatagramChannel.send(buffer, path) returns 'int'. TODO -- Revert metadata being a separate class? go-SNET also has a separate class... - Make Path fully immutable? --> How do we refresh Paths? How do we return refreshed paths from send()? --> Maybe not return, but cache refreshed path, either in Path itself (weird?) or @@ -33,6 +33,11 @@ TODO - Move design doc from Path javadoc to Design.md - return value of send()? Separate PR? +TODO general +- Why are these synchronized, they are only reading? isBlocking(), getPathPolicy(), ... + --> volatile? +- Why disconnect() inside disconnect()? + ### Fixed - Fixed locking and resizing of buffers. [#68](https://github.com/scionproto-contrib/jpan/pull/68) - Fixed unhelpful error & log message when with topofile has wrong permissions. diff --git a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java index 7082bb1de..4cb20fb9c 100644 --- a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java +++ b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java @@ -22,7 +22,6 @@ import java.nio.channels.ClosedChannelException; import java.nio.channels.DatagramChannel; import java.nio.channels.NotYetConnectedException; -import java.time.Instant; import java.util.Objects; import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; @@ -86,7 +85,7 @@ protected boolean isBlocking() { } public PathPolicy getPathPolicy() { - synchronized (stateLock) { + synchronized (stateLock) { // TODO why synchronized??? return this.pathPolicy; } } @@ -539,18 +538,21 @@ protected final ByteBuffer getBufferReceive(int requiredSize) { /** * @param path path * @param payloadLength payload length - * @return argument path or a new path if the argument path was expired * @throws IOException in case of IOException. */ - protected Path checkPathAndBuildHeader( + protected void checkPathAndBuildHeader( ByteBuffer buffer, Path path, int payloadLength, InternalConstants.HdrTypes hdrType) throws IOException { synchronized (stateLock) { if (path instanceof RequestPath) { - path = ensureUpToDate((RequestPath) path); + RequestPath requestPath = (RequestPath) path; + ScionService svc = getOrCreateService(); + if (svc.refreshPath(requestPath, getPathPolicy(), cfgExpirationSafetyMargin) + && isConnected()) { + updateConnection(requestPath, true); + } } buildHeader(buffer, path, payloadLength, hdrType); - return path; } } @@ -627,47 +629,6 @@ protected void buildHeader( } } - protected RequestPath ensureUpToDate(RequestPath path) throws IOException { - synchronized (stateLock) { - // TODO remove -// if (Instant.now().getEpochSecond() + cfgExpirationSafetyMargin -// <= path.getMetadata().getExpiration()) { -// return path; -// } -// // expired, get new path -// RequestPath newPath = pathPolicy.filter(getOrCreateService().getPaths(path)); -// if (isConnected()) { -// updateConnection(newPath, true); -// } -// return newPath; - - // TODO ensure that refresh always heappens, e.g. in v1 we call in inside send().... - RequestPath newPath = refresh(path); - if (path != refresh(path) && isConnected()) { - // TODO move this `if` into refresh()? - updateConnection(path, true); - } - return newPath; - -// -// // TODO this assumes that the part modifies itself! -// if (path.refreshPath(getOrCreateService(), pathPolicy, cfgExpirationSafetyMargin)) { -// if (isConnected()) { -// updateConnection(path, true); -// } -// } - - } - } - - private RequestPath refresh(RequestPath path) { - if (Instant.now().getEpochSecond() + cfgExpirationSafetyMargin <= path.getMetadata().getExpiration()) { - return path; - } - // expired, get new path - return pathPolicy.filter(service.getPaths(path)); - } - private void updateConnection(RequestPath newPath, boolean mustBeConnected) throws IOException { if (mustBeConnected && !isConnected()) { throw new IllegalStateException(); diff --git a/src/main/java/org/scion/jpan/Path.java b/src/main/java/org/scion/jpan/Path.java index 71f8ebcd8..b19a31a39 100644 --- a/src/main/java/org/scion/jpan/Path.java +++ b/src/main/java/org/scion/jpan/Path.java @@ -23,27 +23,24 @@ *

This class is thread safe. * *

Design considerations:
- * - Having Path subclass InetSocketAddress may feel a bit awkward, not least because + * - Having Path inherit InetSocketAddress may feel a bit awkward, not least because * getAddress()/getPort() are not immediately clear to refer to the remote host. However, - * subclassing InetSocketAddress allows paths to be returned by DatagramChannel.receive(), which + * subclassing InetSocketAddress allows paths to be returned from DatagramChannel.receive(), which * would otherwise not be possible.
- * - Having two sublasses of Path ensures that RequestPaths and ResponsePath are never mixed up.
+ * - Having two subclasses of Path ensures that RequestPaths and ResponsePath are never mixed up. + *
* - The design also allows immutability, and thus thread safety.
* - Having metadata in a separate class makes the API cleaner. */ public abstract class Path extends InetSocketAddress { - private final byte[] pathRaw; private final long dstIsdAs; - protected Path(byte[] rawPath, long dstIsdAs, InetAddress dstIP, int dstPort) { + protected Path(long dstIsdAs, InetAddress dstIP, int dstPort) { super(dstIP, dstPort); - this.pathRaw = rawPath; this.dstIsdAs = dstIsdAs; } - public byte[] getRawPath() { - return pathRaw; - } + public abstract byte[] getRawPath(); public abstract InetSocketAddress getFirstHopAddress() throws UnknownHostException; @@ -77,7 +74,7 @@ public String toString() { + ", address=" + super.toString() + ", pathRaw=" - + Arrays.toString(pathRaw) + + Arrays.toString(getRawPath()) + '}'; } } diff --git a/src/main/java/org/scion/jpan/PathMetadata.java b/src/main/java/org/scion/jpan/PathDetails.java similarity index 80% rename from src/main/java/org/scion/jpan/PathMetadata.java rename to src/main/java/org/scion/jpan/PathDetails.java index 7c436d7d4..2ff6ef8f7 100644 --- a/src/main/java/org/scion/jpan/PathMetadata.java +++ b/src/main/java/org/scion/jpan/PathDetails.java @@ -14,26 +14,54 @@ package org.scion.jpan; +import java.io.UncheckedIOException; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.UnknownHostException; import java.util.Collections; import java.util.List; import java.util.stream.Collectors; +import org.scion.jpan.internal.IPHelper; import org.scion.jpan.proto.daemon.Daemon; /** - * PathMetadata contains additional meta information such as bandwidth, latency or geo coordinates. - * PathMetadata is available from a RequestPaths are created/returned by the ScionService when - * requesting a new path from the control service. + * PathDetails contains the raw path and meta information such as bandwidth, latency or geo + * coordinates. PathDetails is available from RequestPaths that are created/returned by the + * ScionService when requesting a new path from the control service. */ -public class PathMetadata { +public class PathDetails { private final Daemon.Path pathProtoc; + private final byte[] pathRaw; + // We store the first hop separately to void creating unnecessary objects. + private final InetSocketAddress firstHop; - static PathMetadata create(Daemon.Path path) { - return new PathMetadata(path); + static PathDetails create(Daemon.Path path, InetAddress dstIP, int dstPort) { + return new PathDetails(path, dstIP, dstPort); } - private PathMetadata(Daemon.Path path) { + private PathDetails(Daemon.Path path, InetAddress dstIP, int dstPort) { this.pathProtoc = path; + this.pathRaw = path.getRaw().toByteArray(); + if (getRawPath().length == 0) { + // local AS has path length 0 + firstHop = new InetSocketAddress(dstIP, dstPort); + } else { + firstHop = getFirstHopAddress(path); + } + } + + private InetSocketAddress getFirstHopAddress(Daemon.Path internalPath) { + try { + String underlayAddressString = internalPath.getInterface().getAddress().getAddress(); + int splitIndex = underlayAddressString.indexOf(':'); + InetAddress ip = IPHelper.toInetAddress(underlayAddressString.substring(0, splitIndex)); + int port = Integer.parseUnsignedInt(underlayAddressString.substring(splitIndex + 1)); + return new InetSocketAddress(ip, port); + } catch (UnknownHostException e) { + // This really should never happen, the first hop is a literal IP address. + throw new UncheckedIOException(e); + } } private Daemon.Path protoPath() { @@ -45,6 +73,14 @@ private Daemon.Path protoPath() { return pathProtoc; } + public InetSocketAddress getFirstHopAddress() throws UnknownHostException { + return firstHop; + } + + public byte[] getRawPath() { + return pathRaw; + } + /** * @return Interface for exiting the local AS using this path. * @throws IllegalStateException if this is path is only a raw path diff --git a/src/main/java/org/scion/jpan/PathPolicy.java b/src/main/java/org/scion/jpan/PathPolicy.java index bd27a593e..29feb5e4a 100644 --- a/src/main/java/org/scion/jpan/PathPolicy.java +++ b/src/main/java/org/scion/jpan/PathPolicy.java @@ -32,7 +32,7 @@ public RequestPath filter(List paths) { class MaxBandwith implements PathPolicy { public RequestPath filter(List paths) { return paths.stream() - .max(Comparator.comparing(path -> Collections.min(path.getMetadata().getBandwidthList()))) + .max(Comparator.comparing(path -> Collections.min(path.getDetails().getBandwidthList()))) .orElseThrow(NoSuchElementException::new); } } @@ -45,7 +45,7 @@ public RequestPath filter(List paths) { .min( Comparator.comparing( path -> - path.getMetadata().getLatencyList().stream() + path.getDetails().getLatencyList().stream() .mapToLong(l -> l > 0 ? l : Integer.MAX_VALUE) .reduce(0, Long::sum))) .orElseThrow(NoSuchElementException::new); @@ -55,7 +55,7 @@ public RequestPath filter(List paths) { class MinHopCount implements PathPolicy { public RequestPath filter(List paths) { return paths.stream() - .min(Comparator.comparing(path -> path.getMetadata().getInternalHopsList().size())) + .min(Comparator.comparing(path -> path.getDetails().getInternalHopsList().size())) .orElseThrow(NoSuchElementException::new); } } @@ -76,7 +76,7 @@ public RequestPath filter(List paths) { } private boolean checkPath(RequestPath path) { - for (PathMetadata.PathInterface pif : path.getMetadata().getInterfacesList()) { + for (PathDetails.PathInterface pif : path.getDetails().getInterfacesList()) { int isd = (int) (pif.getIsdAs() >>> 48); if (!allowedIsds.contains(isd)) { return false; @@ -102,7 +102,7 @@ public RequestPath filter(List paths) { } private boolean checkPath(RequestPath path) { - for (PathMetadata.PathInterface pif : path.getMetadata().getInterfacesList()) { + for (PathDetails.PathInterface pif : path.getDetails().getInterfacesList()) { int isd = (int) (pif.getIsdAs() >>> 48); if (disallowedIsds.contains(isd)) { return false; diff --git a/src/main/java/org/scion/jpan/RequestPath.java b/src/main/java/org/scion/jpan/RequestPath.java index a54327f87..650adcb07 100644 --- a/src/main/java/org/scion/jpan/RequestPath.java +++ b/src/main/java/org/scion/jpan/RequestPath.java @@ -14,92 +14,48 @@ package org.scion.jpan; -import java.io.IOException; -import java.io.UncheckedIOException; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.UnknownHostException; -import java.time.Instant; - -import org.scion.jpan.internal.IPHelper; +import java.util.concurrent.atomic.AtomicReference; import org.scion.jpan.proto.daemon.Daemon; /** * A RequestPath is a Path with additional meta information such as bandwidth, latency or geo * coordinates. RequestPaths are created/returned by the ScionService when requesting a new path * from the control service. + * + *

A RequestPath is immutable except for an atomic reference to PathDetails (PathDetails are + * immutable). The reference may be updated, for example when a path expires. */ public class RequestPath extends Path { - private final PathMetadata metadata; - // We store the first hop separately to void creating unnecessary objects. - private final InetSocketAddress firstHop; + private final AtomicReference details = new AtomicReference<>(); static RequestPath create(Daemon.Path path, long dstIsdAs, InetAddress dstIP, int dstPort) { return new RequestPath(path, dstIsdAs, dstIP, dstPort); } private RequestPath(Daemon.Path path, long dstIsdAs, InetAddress dstIP, int dstPort) { - super(path.getRaw().toByteArray(), dstIsdAs, dstIP, dstPort); - this.metadata = PathMetadata.create(path); - if (getRawPath().length == 0) { - // local AS has path length 0 - firstHop = new InetSocketAddress(getRemoteAddress(), getRemotePort()); - } else { - firstHop = getFirstHopAddress(path); - } + super(dstIsdAs, dstIP, dstPort); + this.details.set(PathDetails.create(path, dstIP, dstPort)); } @Override public InetSocketAddress getFirstHopAddress() throws UnknownHostException { - return firstHop; + return getDetails().getFirstHopAddress(); } - private InetSocketAddress getFirstHopAddress(Daemon.Path internalPath) { - try { - String underlayAddressString = internalPath.getInterface().getAddress().getAddress(); - int splitIndex = underlayAddressString.indexOf(':'); - InetAddress ip = IPHelper.toInetAddress(underlayAddressString.substring(0, splitIndex)); - int port = Integer.parseUnsignedInt(underlayAddressString.substring(splitIndex + 1)); - return new InetSocketAddress(ip, port); - } catch (UnknownHostException e) { - // This really should never happen, the first hop is a literal IP address. - throw new UncheckedIOException(e); - } + @Override + public byte[] getRawPath() { + return getDetails().getRawPath(); } - public PathMetadata getMetadata() { - return metadata; + public PathDetails getDetails() { + return details.get(); } -// /** -// * @param service Service for obtaining new paths when required -// * @param pathPolicy PathPolicy for selecting a new path when required -// * @param expiryMargin Expiry margin, i.e. a path is considered "expired" if expiry is less than -// * expiryMargin seconds away. -// * @return `false` if the path is `null`, it it is a ResponsePath or if it didn't need updating. -// * Returns `true` only if the path was updated. -// */ -// synchronized boolean refreshPath(ScionService service, PathPolicy pathPolicy, int expiryMargin) -// throws IOException { -//// if (path == null) { -//// path = pathPolicy.filter(service.getPaths(this)); -//// if (path == null) { -//// throw new IOException("Address is not resolvable in SCION: " + super.toString()); -//// } -//// return true; -//// } -//// -//// if (!(path instanceof RequestPath)) { -//// return false; -//// } -//// -//// RequestPath requestPath = (RequestPath) path; -// if (Instant.now().getEpochSecond() + expiryMargin <= getMetadata().getExpiration()) { -// return false; -// } -// // expired, get new path -// path = pathPolicy.filter(service.getPaths(requestPath)); -// return true; -// } + void setDetails(PathDetails details) { + this.details.set(details); + } } diff --git a/src/main/java/org/scion/jpan/ResponsePath.java b/src/main/java/org/scion/jpan/ResponsePath.java index b53b4cc79..dee759e48 100644 --- a/src/main/java/org/scion/jpan/ResponsePath.java +++ b/src/main/java/org/scion/jpan/ResponsePath.java @@ -22,6 +22,8 @@ * ISD/AS, IP and port of the local host. This is mostly for convenience to avoid looking up this * information, but it also ensures that the return packet header contains the exact information * sent/expected by the client. + * + *

A ResponsePath is immutable and thus thread safe. */ public class ResponsePath extends Path { @@ -30,6 +32,7 @@ public class ResponsePath extends Path { private final long srcIsdAs; private final InetAddress srcAddress; private final int srcPort; + private final byte[] pathRaw; public static ResponsePath create( byte[] rawPath, @@ -53,11 +56,12 @@ private ResponsePath( InetAddress dstIP, int dstPort, InetSocketAddress firstHopAddress) { - super(rawPath, dstIsdAs, dstIP, dstPort); + super(dstIsdAs, dstIP, dstPort); this.firstHopAddress = firstHopAddress; this.srcIsdAs = srcIsdAs; this.srcAddress = srcIP; this.srcPort = srcPort; + this.pathRaw = rawPath; } @Override @@ -77,6 +81,11 @@ public int getLocalPort() { return srcPort; } + @Override + public byte[] getRawPath() { + return pathRaw; + } + @Override public String toString() { return "ResponsePath{" diff --git a/src/main/java/org/scion/jpan/ScionDatagramChannel.java b/src/main/java/org/scion/jpan/ScionDatagramChannel.java index 3ff1d8774..64bc0e045 100644 --- a/src/main/java/org/scion/jpan/ScionDatagramChannel.java +++ b/src/main/java/org/scion/jpan/ScionDatagramChannel.java @@ -89,7 +89,7 @@ public int send(ByteBuffer srcBuffer, SocketAddress destination) throws IOExcept throw new IllegalArgumentException("Address must be of type InetSocketAddress."); } if (destination instanceof Path) { - return send(srcBuffer, (Path) destination); + return sendPath(srcBuffer, (Path) destination); } InetSocketAddress dst = (InetSocketAddress) destination; Path path = getPathPolicy().filter(getOrCreateService().getPaths(dst)); @@ -113,16 +113,15 @@ public int sendPath(ByteBuffer srcBuffer, Path path) throws IOException { try { ByteBuffer buffer = getBufferSend(srcBuffer.remaining()); // + 8 for UDP overlay header length - Path actualPath = - checkPathAndBuildHeader( - buffer, path, srcBuffer.remaining() + 8, InternalConstants.HdrTypes.UDP); + checkPathAndBuildHeader( + buffer, path, srcBuffer.remaining() + 8, InternalConstants.HdrTypes.UDP); try { buffer.put(srcBuffer); } catch (BufferOverflowException e) { throw new IOException("Packet is larger than max send buffer size."); } buffer.flip(); - return sendRaw(buffer, actualPath.getFirstHopAddress(), actualPath); + return sendRaw(buffer, path.getFirstHopAddress(), path); } finally { writeLock().unlock(); } @@ -167,15 +166,16 @@ public int write(ByteBuffer src) throws IOException { try { checkOpen(); checkConnected(true); + Path path = getRemoteAddress(); ByteBuffer buffer = getBufferSend(src.remaining()); int len = src.remaining(); // + 8 for UDP overlay header length - checkPathAndBuildHeader(buffer, getRemoteAddress(), len + 8, InternalConstants.HdrTypes.UDP); + checkPathAndBuildHeader(buffer, path, len + 8, InternalConstants.HdrTypes.UDP); buffer.put(src); buffer.flip(); - int sent = sendRaw(buffer, getRemoteAddress().getFirstHopAddress(), getRemoteAddress()); + int sent = sendRaw(buffer, path.getFirstHopAddress(), path); if (sent < buffer.limit() || buffer.remaining() > 0) { throw new ScionException("Failed to send all data."); } diff --git a/src/main/java/org/scion/jpan/ScionDatagramSocket.java b/src/main/java/org/scion/jpan/ScionDatagramSocket.java index 5116c3d94..3f4229b35 100644 --- a/src/main/java/org/scion/jpan/ScionDatagramSocket.java +++ b/src/main/java/org/scion/jpan/ScionDatagramSocket.java @@ -298,7 +298,7 @@ public void send(DatagramPacket packet) throws IOException { if (path == null) { path = channel.getPathPolicy().filter(channel.getOrCreateService2().getPaths(addr)); } else if (path instanceof RequestPath - && ((RequestPath) path).getMetadata().getExpiration() + && ((RequestPath) path).getDetails().getExpiration() > Instant.now().getEpochSecond()) { // check expiration only for RequestPaths RequestPath request = (RequestPath) path; diff --git a/src/main/java/org/scion/jpan/ScionService.java b/src/main/java/org/scion/jpan/ScionService.java index f65e83290..741f97609 100644 --- a/src/main/java/org/scion/jpan/ScionService.java +++ b/src/main/java/org/scion/jpan/ScionService.java @@ -33,6 +33,7 @@ import java.net.InetSocketAddress; import java.net.SocketAddress; import java.nio.file.Paths; +import java.time.Instant; import java.util.*; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; @@ -555,4 +556,21 @@ InetAddress getExternalIP(InetSocketAddress firstHopAddress) { } } } + + /** + * @param path RequestPath that may need refreshing + * @param pathPolicy PathPolicy for selecting a new path when required + * @param expiryMargin Expiry margin, i.e. a path is considered "expired" if expiry is less than + * expiryMargin seconds away. + * @return `true` if the path was updated, otherwise `false`. + */ + synchronized boolean refreshPath(RequestPath path, PathPolicy pathPolicy, int expiryMargin) { + if (Instant.now().getEpochSecond() + expiryMargin <= path.getDetails().getExpiration()) { + return false; + } + // expired, get new path + RequestPath newPath = pathPolicy.filter(getPaths(path)); + path.setDetails(newPath.getDetails()); + return true; + } } diff --git a/src/main/java/org/scion/jpan/ScionUtil.java b/src/main/java/org/scion/jpan/ScionUtil.java index 015e8454c..44befb7ca 100644 --- a/src/main/java/org/scion/jpan/ScionUtil.java +++ b/src/main/java/org/scion/jpan/ScionUtil.java @@ -132,7 +132,7 @@ public static String toStringPath(byte[] raw) { * @return ISD/AS codes and border outer interface IDs along the path. */ public static String toStringPath(RequestPath path) { - PathMetadata meta = path.getMetadata(); + PathDetails meta = path.getDetails(); if (meta.getInterfacesList().isEmpty()) { return "[]"; } @@ -140,7 +140,7 @@ public static String toStringPath(RequestPath path) { sb.append("["); int nInterfcaces = meta.getInterfacesList().size(); for (int i = 0; i < nInterfcaces; i++) { - PathMetadata.PathInterface pIf = meta.getInterfacesList().get(i); + PathDetails.PathInterface pIf = meta.getInterfacesList().get(i); if (i % 2 == 0) { sb.append(ScionUtil.toStringIA(pIf.getIsdAs())).append(" "); sb.append(pIf.getId()).append(">"); diff --git a/src/test/java/org/scion/jpan/api/DatagramChannelApiServerTest.java b/src/test/java/org/scion/jpan/api/DatagramChannelApiServerTest.java index 50d00e1e4..8d0f0e53a 100644 --- a/src/test/java/org/scion/jpan/api/DatagramChannelApiServerTest.java +++ b/src/test/java/org/scion/jpan/api/DatagramChannelApiServerTest.java @@ -85,9 +85,8 @@ void send_withoutService() throws IOException { new byte[4], 1, new InetSocketAddress("127.0.0.1", 1)); - Path p2 = channel.send(ByteBuffer.allocate(0), path); + channel.send(ByteBuffer.allocate(0), path); assertNull(channel.getService()); - assertEquals(path, p2); } } diff --git a/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java b/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java index 1ecfd3ee5..6893619eb 100644 --- a/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java +++ b/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java @@ -469,13 +469,15 @@ void write_ChannelClosedFails() throws IOException { void send_disconnected_expiredRequestPath() throws IOException { // Expected behavior: expired paths should be replaced transparently. testExpired( - (channel, expiredPath) -> { + (channel, expiringPath) -> { ByteBuffer sendBuf = ByteBuffer.wrap(PingPongChannelHelper.MSG.getBytes()); try { - RequestPath newPath = (RequestPath) channel.send(sendBuf, expiredPath); - assertTrue( - newPath.getMetadata().getExpiration() > expiredPath.getMetadata().getExpiration()); - assertTrue(Instant.now().getEpochSecond() < newPath.getMetadata().getExpiration()); + long oldExpiration = expiringPath.getDetails().getExpiration(); + assertTrue(Instant.now().getEpochSecond() > oldExpiration); + channel.send(sendBuf, expiringPath); + long newExpiration = expiringPath.getDetails().getExpiration(); + assertTrue(newExpiration > oldExpiration); + assertTrue(Instant.now().getEpochSecond() < newExpiration); assertNull(channel.getRemoteAddress()); } catch (IOException e) { throw new RuntimeException(e); @@ -488,14 +490,16 @@ void send_disconnected_expiredRequestPath() throws IOException { void send_connected_expiredRequestPath() throws IOException { // Expected behavior: expired paths should be replaced transparently. testExpired( - (channel, expiredPath) -> { + (channel, expiringPath) -> { ByteBuffer sendBuf = ByteBuffer.wrap(PingPongChannelHelper.MSG.getBytes()); try { - RequestPath newPath = (RequestPath) channel.send(sendBuf, expiredPath); - assertTrue( - newPath.getMetadata().getExpiration() > expiredPath.getMetadata().getExpiration()); - assertTrue(Instant.now().getEpochSecond() < newPath.getMetadata().getExpiration()); - assertEquals(newPath, channel.getRemoteAddress()); + long oldExpiration = expiringPath.getDetails().getExpiration(); + assertTrue(Instant.now().getEpochSecond() > oldExpiration); + channel.send(sendBuf, expiringPath); + long newExpiration = expiringPath.getDetails().getExpiration(); + assertTrue(newExpiration > oldExpiration); + assertTrue(Instant.now().getEpochSecond() < newExpiration); + assertEquals(expiringPath, channel.getRemoteAddress()); } catch (IOException e) { throw new RuntimeException(e); } @@ -513,8 +517,8 @@ void write_expiredRequestPath() throws IOException { channel.write(sendBuf); RequestPath newPath = channel.getRemoteAddress(); assertTrue( - newPath.getMetadata().getExpiration() > expiredPath.getMetadata().getExpiration()); - assertTrue(Instant.now().getEpochSecond() < newPath.getMetadata().getExpiration()); + newPath.getDetails().getExpiration() > expiredPath.getDetails().getExpiration()); + assertTrue(Instant.now().getEpochSecond() < newPath.getDetails().getExpiration()); } catch (IOException e) { throw new RuntimeException(e); } @@ -555,7 +559,7 @@ private RequestPath createExpiredPath(Path basePath) throws UnknownHostException basePath.getRemoteAddress(), basePath.getRemotePort(), basePath.getFirstHopAddress()); - assertTrue(Instant.now().getEpochSecond() > expiredPath.getMetadata().getExpiration()); + assertTrue(Instant.now().getEpochSecond() > expiredPath.getDetails().getExpiration()); return expiredPath; } diff --git a/src/test/java/org/scion/jpan/api/DatagramSocketApiTest.java b/src/test/java/org/scion/jpan/api/DatagramSocketApiTest.java index 0d9888b95..11661e0d4 100644 --- a/src/test/java/org/scion/jpan/api/DatagramSocketApiTest.java +++ b/src/test/java/org/scion/jpan/api/DatagramSocketApiTest.java @@ -619,8 +619,8 @@ void send_connected_expiredRequestPath() throws IOException { socket.send(packet); RequestPath newPath = socket.getConnectionPath(); assertTrue( - newPath.getMetadata().getExpiration() > expiredPath.getMetadata().getExpiration()); - assertTrue(Instant.now().getEpochSecond() < newPath.getMetadata().getExpiration()); + newPath.getDetails().getExpiration() > expiredPath.getDetails().getExpiration()); + assertTrue(Instant.now().getEpochSecond() < newPath.getDetails().getExpiration()); // assertNull(channel.getCurrentPath()); } catch (IOException e) { throw new RuntimeException(e); @@ -661,7 +661,7 @@ private RequestPath createExpiredPath(Path basePath) throws UnknownHostException basePath.getRemoteAddress(), basePath.getRemotePort(), basePath.getFirstHopAddress()); - assertTrue(Instant.now().getEpochSecond() > expiredPath.getMetadata().getExpiration()); + assertTrue(Instant.now().getEpochSecond() > expiredPath.getDetails().getExpiration()); return expiredPath; } diff --git a/src/test/java/org/scion/jpan/api/ScionServiceTest.java b/src/test/java/org/scion/jpan/api/ScionServiceTest.java index f952067dd..1c7cb0eb1 100644 --- a/src/test/java/org/scion/jpan/api/ScionServiceTest.java +++ b/src/test/java/org/scion/jpan/api/ScionServiceTest.java @@ -125,8 +125,8 @@ void getPath() throws IOException { assertEquals(dstIA, path.getRemoteIsdAs()); assertEquals(36, path.getRawPath().length); - assertEquals("127.0.0.10:31004", path.getMetadata().getInterface().getAddress()); - assertEquals(2, path.getMetadata().getInterfacesList().size()); + assertEquals("127.0.0.10:31004", path.getDetails().getInterface().getAddress()); + assertEquals(2, path.getDetails().getInterfacesList().size()); // assertEquals(1, viewer.getInternalHopsList().size()); // assertEquals(0, viewer.getMtu()); // assertEquals(0, viewer.getLinkTypeList().size()); diff --git a/src/test/java/org/scion/jpan/demo/ScmpEchoDemo.java b/src/test/java/org/scion/jpan/demo/ScmpEchoDemo.java index 2f85bebe8..b79ccf58f 100644 --- a/src/test/java/org/scion/jpan/demo/ScmpEchoDemo.java +++ b/src/test/java/org/scion/jpan/demo/ScmpEchoDemo.java @@ -149,8 +149,8 @@ private void printPath(RequestPath path) { // ").append(channel.getLocalAddress().getAddress().getHostAddress()).append(nl); sb.append("Using path:").append(nl); sb.append(" Hops: ").append(ScionUtil.toStringPath(path)); - sb.append(" MTU: ").append(path.getMetadata().getMtu()); - sb.append(" NextHop: ").append(path.getMetadata().getInterface().getAddress()).append(nl); + sb.append(" MTU: ").append(path.getDetails().getMtu()); + sb.append(" NextHop: ").append(path.getDetails().getInterface().getAddress()).append(nl); println(sb.toString()); } diff --git a/src/test/java/org/scion/jpan/demo/ScmpTracerouteDemo.java b/src/test/java/org/scion/jpan/demo/ScmpTracerouteDemo.java index 19bb11d0a..911f489f0 100644 --- a/src/test/java/org/scion/jpan/demo/ScmpTracerouteDemo.java +++ b/src/test/java/org/scion/jpan/demo/ScmpTracerouteDemo.java @@ -129,8 +129,8 @@ private void printPath(RequestPath path) { // sb.append(" ").append(channel.getLocalAddress().getAddress().getHostAddress()).append(nl); sb.append("Using path:").append(nl); sb.append(" Hops: ").append(ScionUtil.toStringPath(path)); - sb.append(" MTU: ").append(path.getMetadata().getMtu()); - sb.append(" NextHop: ").append(path.getMetadata().getInterface().getAddress()).append(nl); + sb.append(" MTU: ").append(path.getDetails().getMtu()); + sb.append(" NextHop: ").append(path.getDetails().getInterface().getAddress()).append(nl); println(sb.toString()); } diff --git a/src/test/java/org/scion/jpan/demo/ShowpathsDemo.java b/src/test/java/org/scion/jpan/demo/ShowpathsDemo.java index 1af0ea01e..3ad8ccbc1 100644 --- a/src/test/java/org/scion/jpan/demo/ShowpathsDemo.java +++ b/src/test/java/org/scion/jpan/demo/ShowpathsDemo.java @@ -114,9 +114,9 @@ private void runDemo(long destinationIA) throws IOException { + "] Hops: " + ScionUtil.toStringPath(path) + " MTU: " - + path.getMetadata().getMtu() + + path.getDetails().getMtu() + " NextHop: " - + path.getMetadata().getInterface().getAddress() + + path.getDetails().getInterface().getAddress() + " LocalIP: " + localIP; println(sb); From abe39a64d12f0d22d5e75e94de4e5864295b2d38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilmann=20Z=C3=A4schke?= Date: Fri, 7 Jun 2024 17:05:02 +0200 Subject: [PATCH 07/13] getConnectionPath() clean up --- .../java/org/scion/jpan/AbstractDatagramChannel.java | 12 ++++-------- .../java/org/scion/jpan/ScionDatagramChannel.java | 4 ++-- src/main/java/org/scion/jpan/ScmpChannel.java | 10 +++++----- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java index 4cb20fb9c..ac1c87510 100644 --- a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java +++ b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java @@ -400,17 +400,13 @@ public void setOverrideSourceAddress(InetSocketAddress address) { this.overrideExternalAddress = address; } - protected int sendRaw(ByteBuffer buffer, InetSocketAddress address, Path path) + protected int sendRaw(ByteBuffer buffer, Path path) throws IOException { if (cfgRemoteDispatcher && path != null && path.getRawPath().length == 0) { - return channel.send( - buffer, new InetSocketAddress(address.getAddress(), Constants.DISPATCHER_PORT)); + InetAddress remoteHostIP = path.getFirstHopAddress().getAddress(); + return channel.send(buffer, new InetSocketAddress(remoteHostIP, Constants.DISPATCHER_PORT)); } - return channel.send(buffer, address); - } - - protected int sendRaw(ByteBuffer buffer, InetSocketAddress address) throws IOException { - return sendRaw(buffer, address, null); + return channel.send(buffer, path.getFirstHopAddress()); } public Consumer setScmpErrorListener(Consumer listener) { diff --git a/src/main/java/org/scion/jpan/ScionDatagramChannel.java b/src/main/java/org/scion/jpan/ScionDatagramChannel.java index 64bc0e045..45f455fe4 100644 --- a/src/main/java/org/scion/jpan/ScionDatagramChannel.java +++ b/src/main/java/org/scion/jpan/ScionDatagramChannel.java @@ -121,7 +121,7 @@ public int sendPath(ByteBuffer srcBuffer, Path path) throws IOException { throw new IOException("Packet is larger than max send buffer size."); } buffer.flip(); - return sendRaw(buffer, path.getFirstHopAddress(), path); + return sendRaw(buffer, path); } finally { writeLock().unlock(); } @@ -175,7 +175,7 @@ public int write(ByteBuffer src) throws IOException { buffer.put(src); buffer.flip(); - int sent = sendRaw(buffer, path.getFirstHopAddress(), path); + int sent = sendRaw(buffer, path); if (sent < buffer.limit() || buffer.remaining() > 0) { throw new ScionException("Failed to send all data."); } diff --git a/src/main/java/org/scion/jpan/ScmpChannel.java b/src/main/java/org/scion/jpan/ScmpChannel.java index 6dd7b3c90..0a36d0bb0 100644 --- a/src/main/java/org/scion/jpan/ScmpChannel.java +++ b/src/main/java/org/scion/jpan/ScmpChannel.java @@ -229,7 +229,7 @@ Scmp.TimedMessage sendEchoRequest(Scmp.EchoMessage request) throws IOException { buffer, Scmp.Type.INFO_128, localPort, request.getSequenceNumber(), request.getData()); buffer.flip(); request.setSizeSent(buffer.remaining()); - sendRaw(buffer, path.getFirstHopAddress()); + sendRaw(buffer, path); int sizeReceived = receive(request); request.setSizeReceived(sizeReceived); @@ -261,7 +261,7 @@ Scmp.TimedMessage sendTracerouteRequest( int posPath = ScionHeaderParser.extractPathHeaderPosition(buffer); buffer.put(posPath + node.posHopFlags, node.hopFlags); - sendRaw(buffer, path.getFirstHopAddress()); + sendRaw(buffer, path); receive(request); return request; @@ -356,10 +356,10 @@ void sendEchoResponses() throws IOException { buffer, Scmp.Type.INFO_129, port, msg.getSequenceNumber(), msg.getData()); buffer.flip(); msg.setSizeSent(buffer.remaining()); - sendRaw(buffer, path.getFirstHopAddress()); - log.info("Responded to SCMP {} from {}", type, path.getRemoteAddress()); + sendRaw(buffer, path); + log.info("Responded to SCMP {} from {}", type, path.getAddress()); } else { - log.info("Dropped SCMP message with type {} from {}", type, path.getRemoteAddress()); + log.info("Dropped SCMP message with type {} from {}", type, path.getAddress()); } } } finally { From b0d9e306558c5fd137e68fb31639ea7fbd9a3d89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilmann=20Z=C3=A4schke?= Date: Fri, 7 Jun 2024 17:29:31 +0200 Subject: [PATCH 08/13] getConnectionPath() clean up --- CHANGELOG.md | 7 +---- doc/Design.md | 30 +++++++++++++++---- .../scion/jpan/AbstractDatagramChannel.java | 5 ++-- src/main/java/org/scion/jpan/Path.java | 10 ------- src/main/java/org/scion/jpan/ScionUtil.java | 10 +++---- 5 files changed, 33 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da780cb8a..c74d1e14b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,13 +25,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - **BREAKING CHANGE**: ScionDatagramChannel.send(buffer, path) returns 'int'. TODO -- Make Path fully immutable? - --> How do we refresh Paths? How do we return refreshed paths from send()? - --> Maybe not return, but cache refreshed path, either in Path itself (weird?) or - in ScionService? - Move expiryMargin to ScionService? -- Move design doc from Path javadoc to Design.md -- return value of send()? Separate PR? +- return value of send()? Separate PR? -> test send() result TODO general - Why are these synchronized, they are only reading? isBlocking(), getPathPolicy(), ... diff --git a/doc/Design.md b/doc/Design.md index fdca73300..801145158 100644 --- a/doc/Design.md +++ b/doc/Design.md @@ -64,15 +64,35 @@ used or returned by `Scion.defaultService()`. ## Paths -There are two types of paths (both inherit `Path`: +`Path` contains a destination IA:IP:port, a raw path and a first hop IP:port. +There are two subclasses of paths (both inherit `Path`): + - `RequestPath` are used to send initial request. They are retrieved from a path service and contain meta information. -- `ResponsePath` are used to respond to a client request. They are extracted from SCION packets. +- `ResponsePath` are used to respond to a client request. They are extracted from SCION packets. + They also contains origin ISD/AS:IP:port. -`Path` contains a destination IA:IP:port, a raw path and a first hop IP:port. -`RequestPath` also contains path meta information. -`ResponsePath` also contains origin IA:IP:port. +### Design choices - Subclassing InetSocketAddress + +`Path` is a subclass of `InetSocketAddress`. This is useful for `DatagramChannel`'s `receive()` +(which returns an `InetScoketAddress`) and `send()` which requires an `InetSocketAddress` as +argument. + +The catch here is that `Path` has `getAddress()` and `getPort()` which are now contracted to +return the *remote* IP/port. Ideally they would be called `getRemoteAddress()` and`getRemotePort()` +but that is not possible. + +### Design choices - Mutable & separate "details" + +For concurrent usage, `Path`s should be thread safe. THis would ideally be achieved by making them +immutable. +The problem is that paths expire or can otherwise be invalidated. We could make this explicit and +force API users to either manage expiry/invalidation manually. + +The alternative is to move the path of `Path` that is mutable to a new class `PathDetails`. +`PathDetails` is itself immutable and is referenced atomically from `Path`, so thread safe +behaviour is still easily achievable. ## DatagramSocket diff --git a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java index ac1c87510..bdf2a5e37 100644 --- a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java +++ b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java @@ -400,9 +400,8 @@ public void setOverrideSourceAddress(InetSocketAddress address) { this.overrideExternalAddress = address; } - protected int sendRaw(ByteBuffer buffer, Path path) - throws IOException { - if (cfgRemoteDispatcher && path != null && path.getRawPath().length == 0) { + protected int sendRaw(ByteBuffer buffer, Path path) throws IOException { + if (cfgRemoteDispatcher && path.getRawPath().length == 0) { InetAddress remoteHostIP = path.getFirstHopAddress().getAddress(); return channel.send(buffer, new InetSocketAddress(remoteHostIP, Constants.DISPATCHER_PORT)); } diff --git a/src/main/java/org/scion/jpan/Path.java b/src/main/java/org/scion/jpan/Path.java index b19a31a39..889ab545f 100644 --- a/src/main/java/org/scion/jpan/Path.java +++ b/src/main/java/org/scion/jpan/Path.java @@ -21,16 +21,6 @@ * A Path is an InetSocketAddress/ISD/AS of a destination host plus a path to that host. * *

This class is thread safe. - * - *

Design considerations:
- * - Having Path inherit InetSocketAddress may feel a bit awkward, not least because - * getAddress()/getPort() are not immediately clear to refer to the remote host. However, - * subclassing InetSocketAddress allows paths to be returned from DatagramChannel.receive(), which - * would otherwise not be possible.
- * - Having two subclasses of Path ensures that RequestPaths and ResponsePath are never mixed up. - *
- * - The design also allows immutability, and thus thread safety.
- * - Having metadata in a separate class makes the API cleaner. */ public abstract class Path extends InetSocketAddress { private final long dstIsdAs; diff --git a/src/main/java/org/scion/jpan/ScionUtil.java b/src/main/java/org/scion/jpan/ScionUtil.java index 44befb7ca..637306a5b 100644 --- a/src/main/java/org/scion/jpan/ScionUtil.java +++ b/src/main/java/org/scion/jpan/ScionUtil.java @@ -132,15 +132,15 @@ public static String toStringPath(byte[] raw) { * @return ISD/AS codes and border outer interface IDs along the path. */ public static String toStringPath(RequestPath path) { - PathDetails meta = path.getDetails(); - if (meta.getInterfacesList().isEmpty()) { + PathDetails details = path.getDetails(); + if (details.getInterfacesList().isEmpty()) { return "[]"; } StringBuilder sb = new StringBuilder(); sb.append("["); - int nInterfcaces = meta.getInterfacesList().size(); + int nInterfcaces = details.getInterfacesList().size(); for (int i = 0; i < nInterfcaces; i++) { - PathDetails.PathInterface pIf = meta.getInterfacesList().get(i); + PathDetails.PathInterface pIf = details.getInterfacesList().get(i); if (i % 2 == 0) { sb.append(ScionUtil.toStringIA(pIf.getIsdAs())).append(" "); sb.append(pIf.getId()).append(">"); @@ -148,7 +148,7 @@ public static String toStringPath(RequestPath path) { sb.append(pIf.getId()).append(" "); } } - sb.append(ScionUtil.toStringIA(meta.getInterfacesList().get(nInterfcaces - 1).getIsdAs())); + sb.append(ScionUtil.toStringIA(details.getInterfacesList().get(nInterfcaces - 1).getIsdAs())); sb.append("]"); return sb.toString(); } From 0ddaabf73502c282bc724c4f580ac5e374f84ed2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilmann=20Z=C3=A4schke?= Date: Fri, 7 Jun 2024 17:39:15 +0200 Subject: [PATCH 09/13] getConnectionPath() clean up --- CHANGELOG.md | 2 -- .../java/org/scion/jpan/ScionDatagramChannel.java | 4 +++- .../org/scion/jpan/api/DatagramChannelApiTest.java | 11 +++++++++++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c74d1e14b..f36addd03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,10 +23,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - **BREAKING CHANGE**: Access to path details such as metadata has been moved to `Path.getDetails()`. - **BREAKING CHANGE**: ScionDatagramChannel.send(buffer, path) returns 'int'. - TODO - Move expiryMargin to ScionService? -- return value of send()? Separate PR? -> test send() result TODO general - Why are these synchronized, they are only reading? isBlocking(), getPathPolicy(), ... diff --git a/src/main/java/org/scion/jpan/ScionDatagramChannel.java b/src/main/java/org/scion/jpan/ScionDatagramChannel.java index 45f455fe4..84f49ac7c 100644 --- a/src/main/java/org/scion/jpan/ScionDatagramChannel.java +++ b/src/main/java/org/scion/jpan/ScionDatagramChannel.java @@ -115,13 +115,15 @@ public int sendPath(ByteBuffer srcBuffer, Path path) throws IOException { // + 8 for UDP overlay header length checkPathAndBuildHeader( buffer, path, srcBuffer.remaining() + 8, InternalConstants.HdrTypes.UDP); + int headerSize = buffer.position(); try { buffer.put(srcBuffer); } catch (BufferOverflowException e) { throw new IOException("Packet is larger than max send buffer size."); } buffer.flip(); - return sendRaw(buffer, path); + int size = sendRaw(buffer, path); + return size - headerSize; } finally { writeLock().unlock(); } diff --git a/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java b/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java index 6893619eb..dd19394ec 100644 --- a/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java +++ b/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java @@ -402,6 +402,17 @@ void getPathPolicy() throws IOException { } } + @Test + void send_bufferSize() throws IOException { + try (ScionDatagramChannel channel = ScionDatagramChannel.open()) { + int size0 = channel.send(ByteBuffer.allocate(0), ExamplePacket.PATH); + assertEquals(0, size0); + + int size100 = channel.send(ByteBuffer.wrap(new byte[100]), ExamplePacket.PATH); + assertEquals(100, size100); + } + } + @Test void send_bufferTooLarge() { RequestPath addr = ExamplePacket.PATH; From c211be3f659eff2c46c79c7988d992e75ca5eec1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilmann=20Z=C3=A4schke?= Date: Fri, 7 Jun 2024 17:51:58 +0200 Subject: [PATCH 10/13] getConnectionPath() clean up --- CHANGELOG.md | 7 +------ src/main/java/org/scion/jpan/AbstractDatagramChannel.java | 3 +-- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f36addd03..576ed1a24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,12 +24,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. `Path.getDetails()`. - **BREAKING CHANGE**: ScionDatagramChannel.send(buffer, path) returns 'int'. TODO -- Move expiryMargin to ScionService? - -TODO general -- Why are these synchronized, they are only reading? isBlocking(), getPathPolicy(), ... - --> volatile? -- Why disconnect() inside disconnect()? +- Move expiryMargin to ScionService? ### Fixed - Fixed locking and resizing of buffers. [#68](https://github.com/scionproto-contrib/jpan/pull/68) diff --git a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java index bdf2a5e37..d7db7f08c 100644 --- a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java +++ b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java @@ -85,7 +85,7 @@ protected boolean isBlocking() { } public PathPolicy getPathPolicy() { - synchronized (stateLock) { // TODO why synchronized??? + synchronized (stateLock) { return this.pathPolicy; } } @@ -190,7 +190,6 @@ public RequestPath getRemoteAddress() throws IOException { public void disconnect() throws IOException { synchronized (stateLock) { - channel.disconnect(); // TODO Why ? We shouldn´t do that...? connectionPath = null; } } From 18cf7e971ba85ec713629d722194d66bcad6be25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilmann=20Z=C3=A4schke?= Date: Mon, 10 Jun 2024 13:12:19 +0200 Subject: [PATCH 11/13] getConnectionPath() clean up --- CHANGELOG.md | 2 + .../java/org/scion/jpan/ScionService.java | 55 +++++++++++++++---- .../jpan/testutil/PingPongHelperBase.java | 4 +- 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 576ed1a24..83d4c6cce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - **BREAKING CHANGE**: ScionDatagramChannel.send(buffer, path) returns 'int'. TODO - Move expiryMargin to ScionService? +- remove ScionAddress? +- Remove getPaths(long dstIsdAs, InetSocketAddress dstScionAddress) -< ISD + Scion address!!! ### Fixed - Fixed locking and resizing of buffers. [#68](https://github.com/scionproto-contrib/jpan/pull/68) diff --git a/src/main/java/org/scion/jpan/ScionService.java b/src/main/java/org/scion/jpan/ScionService.java index 741f97609..41569f518 100644 --- a/src/main/java/org/scion/jpan/ScionService.java +++ b/src/main/java/org/scion/jpan/ScionService.java @@ -307,7 +307,9 @@ List getPathListDaemon(long srcIsdAs, long dstIsdAs) { * @return All paths returned by the path service. * @throws IOException if an errors occurs while querying paths. */ + @Deprecated // Please use lookUp() instead public List getPaths(InetSocketAddress dstAddress) throws IOException { + // TODO this is ambiguous .... InetSocketAddress argument // Use getHostString() to avoid DNS reverse lookup. ScionAddress sa = getScionAddress(dstAddress.getHostString()); return getPaths(sa.getIsdAs(), sa.getInetAddress(), dstAddress.getPort()); @@ -320,6 +322,13 @@ public List getPaths(InetSocketAddress dstAddress) throws IOExcepti * @param dstScionAddress Destination IP address. Must belong to a SCION enabled end host. * @return All paths returned by the path service. */ + // TODO this is bad!!! Why ISD + SCION-Address????? + // TODO this is bad!!! Why ISD + SCION-Address????? + // TODO this is bad!!! Why ISD + SCION-Address????? + // TODO this is bad!!! Why ISD + SCION-Address????? + // TODO this is bad!!! Why ISD + SCION-Address????? + // TODO this is bad!!! Why ISD + SCION-Address????? + // TODO this is bad!!! Why ISD + SCION-Address????? public List getPaths(long dstIsdAs, InetSocketAddress dstScionAddress) { // TODO change method API name to make clear that this requires a SCION IP. return getPaths(dstIsdAs, dstScionAddress.getAddress(), dstScionAddress.getPort()); @@ -332,7 +341,7 @@ public List getPaths(long dstIsdAs, InetSocketAddress dstScionAddre * @return All paths returned by the path service. */ public List getPaths(RequestPath path) { - return getPaths(path.getRemoteIsdAs(), path.getRemoteAddress(), path.getRemotePort()); + return getPaths(path.getIsdAs(), path.getAddress(), path.getPort()); } /** @@ -341,17 +350,24 @@ public List getPaths(RequestPath path) { * @param dstIsdAs Destination ISD/AS * @param dstAddress Destination IP address * @param dstPort Destination port - * @return All paths returned by the path service. + * @return All paths returned by the path service. Returns an empty list if no paths are found. */ public List getPaths(long dstIsdAs, InetAddress dstAddress, int dstPort) { + return getPaths(RequestPath.fromScionIP(dstIsdAs, dstAddress, dstPort)); + } + + /** + * Request paths from the local ISD/AS to the destination. + * + * @param dstAddress Destination SCION address + * @return All paths returned by the path service. + */ + public List getPaths(Path dstAddress) { long srcIsdAs = getLocalIsdAs(); - List paths = getPathList(srcIsdAs, dstIsdAs); - if (paths.isEmpty()) { - return Collections.emptyList(); - } + List paths = getPathList(srcIsdAs, dstAddress.getIsdAs()); List scionPaths = new ArrayList<>(paths.size()); for (int i = 0; i < paths.size(); i++) { - scionPaths.add(RequestPath.create(paths.get(i), dstIsdAs, dstAddress, dstPort)); + scionPaths.add(RequestPath.create(paths.get(i), dstAddress)); } return scionPaths; } @@ -378,7 +394,7 @@ public long getLocalIsdAs() { /** * @param hostName hostName of the host to resolve - * @return A ScionAddress + * @return The ISD/AS code for a hostname * @throws ScionException if the DNS/TXT lookup did not return a (valid) SCION address. */ public long getIsdAs(String hostName) throws ScionException { @@ -417,12 +433,31 @@ public long getIsdAs(String hostName) throws ScionException { throw new ScionException("No DNS TXT entry \"scion\" found for host: " + hostName); } + @Deprecated // Please use lookupScionAddress() instead. + public ScionAddress getScionAddress(String hostName) throws ScionException { + return lookupAddress(hostName); + } + /** + * Uses DNS and hostfiles to look up a SCIOn enabled IP address for a give host string. + * + * @param hostName hostName of the host to resolve + * @return A ScionSocketAddress + * @throws ScionException if the DNS/TXT lookup did not return a (valid) SCION address. + */ + public Path lookupSocketAddress(String hostName, int port) throws ScionException { + return RequestPath.fromScionAddress(lookupAddress(hostName), port); + } + + /** + * Uses DNS and hostfiles to look up a SCIOn enabled IP address for a give host string. + * * @param hostName hostName of the host to resolve * @return A ScionAddress * @throws ScionException if the DNS/TXT lookup did not return a (valid) SCION address. */ - public ScionAddress getScionAddress(String hostName) throws ScionException { + @Deprecated // TODO NOW: remove this lookupAddress() method in favor of lookupSocketAddress() + private ScionAddress lookupAddress(String hostName) throws ScionException { ScionAddress scionAddress = scionAddressCache.get(hostName); if (scionAddress != null) { return scionAddress; @@ -564,7 +599,7 @@ InetAddress getExternalIP(InetSocketAddress firstHopAddress) { * expiryMargin seconds away. * @return `true` if the path was updated, otherwise `false`. */ - synchronized boolean refreshPath(RequestPath path, PathPolicy pathPolicy, int expiryMargin) { + boolean refreshPath(RequestPath path, PathPolicy pathPolicy, int expiryMargin) { if (Instant.now().getEpochSecond() + expiryMargin <= path.getDetails().getExpiration()) { return false; } diff --git a/src/test/java/org/scion/jpan/testutil/PingPongHelperBase.java b/src/test/java/org/scion/jpan/testutil/PingPongHelperBase.java index 73448d320..264c02080 100644 --- a/src/test/java/org/scion/jpan/testutil/PingPongHelperBase.java +++ b/src/test/java/org/scion/jpan/testutil/PingPongHelperBase.java @@ -107,11 +107,11 @@ void runPingPong(ServerFactory serverFactory, ClientFactory clientFactory, boole } InetSocketAddress serverAddress = servers[0].getLocalAddress(); MockDNS.install(MockNetwork.TINY_SRV_ISD_AS, serverAddress.getAddress()); - RequestPath scionAddress = Scion.defaultService().getPaths(serverAddress).get(0); + RequestPath requestPath = Scion.defaultService().getPaths(serverAddress).get(0); Thread[] clients = new Thread[nClients]; for (int i = 0; i < clients.length; i++) { - clients[i] = clientFactory.create(i, scionAddress, nRounds); + clients[i] = clientFactory.create(i, requestPath, nRounds); clients[i].setName("Client-thread-" + i); clients[i].start(); } From 265d6bdbf7c38a6564ac159eb029984733c2d6eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilmann=20Z=C3=A4schke?= Date: Mon, 10 Jun 2024 14:47:12 +0200 Subject: [PATCH 12/13] Path interface --- CHANGELOG.md | 1 + .../scion/jpan/AbstractDatagramChannel.java | 17 +-- src/main/java/org/scion/jpan/Path.java | 43 ++------ src/main/java/org/scion/jpan/PathImpl.java | 70 ++++++++++++ src/main/java/org/scion/jpan/RequestPath.java | 32 +----- .../java/org/scion/jpan/RequestPathImpl.java | 61 +++++++++++ .../java/org/scion/jpan/ResponsePath.java | 67 +----------- .../java/org/scion/jpan/ResponsePathImpl.java | 103 ++++++++++++++++++ .../org/scion/jpan/ScionDatagramChannel.java | 10 +- .../org/scion/jpan/ScionDatagramSocket.java | 4 +- .../java/org/scion/jpan/ScionService.java | 57 +++++----- src/main/java/org/scion/jpan/ScmpChannel.java | 6 +- .../jpan/api/DatagramChannelApiTest.java | 2 +- .../jpan/testutil/PingPongHelperBase.java | 2 +- 14 files changed, 300 insertions(+), 175 deletions(-) create mode 100644 src/main/java/org/scion/jpan/PathImpl.java create mode 100644 src/main/java/org/scion/jpan/RequestPathImpl.java create mode 100644 src/main/java/org/scion/jpan/ResponsePathImpl.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 83d4c6cce..085644186 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ TODO - Move expiryMargin to ScionService? - remove ScionAddress? - Remove getPaths(long dstIsdAs, InetSocketAddress dstScionAddress) -< ISD + Scion address!!! +- Remove use of getHostName() -> InetAddress! ### Fixed - Fixed locking and resizing of buffers. [#68](https://github.com/scionproto-contrib/jpan/pull/68) diff --git a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java index d7db7f08c..aad70678f 100644 --- a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java +++ b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java @@ -184,8 +184,8 @@ public InetSocketAddress getLocalAddress() throws IOException { * @see #connect(RequestPath) * @throws IOException If an I/O error occurs */ - public RequestPath getRemoteAddress() throws IOException { - return connectionPath; + public InetSocketAddress getRemoteAddress() throws IOException { + return (InetSocketAddress) connectionPath; } public void disconnect() throws IOException { @@ -231,7 +231,10 @@ public C connect(SocketAddress addr) throws IOException { throw new IllegalArgumentException( "connect() requires an InetSocketAddress or a ScionSocketAddress."); } - return connect(pathPolicy.filter(getOrCreateService().getPaths((InetSocketAddress) addr))); + if (addr instanceof RequestPath) { + return connect(((RequestPath) addr)); + } + return connect(getOrCreateService().lookupAndGetPath((InetSocketAddress) addr, pathPolicy)); } } @@ -299,10 +302,8 @@ public C connect(RequestPath path) throws IOException { * #connect(RequestPath)} and may be refreshed when expired. * * @return the current Path or `null` if not path is connected. - * @deprecated Please use getRemoteAddress() instead */ - @Deprecated - public Path getConnectionPath() { + public RequestPath getConnectionPath() { synchronized (stateLock) { return connectionPath; } @@ -611,13 +612,13 @@ protected void buildHeader( srcIA, srcAddress.getAddress(), path.getIsdAs(), - path.getAddress().getAddress(), + path.getRemoteAddress().getAddress(), hdrType, cfgTrafficClass); ScionHeaderParser.writePath(buffer, rawPath); if (hdrType == InternalConstants.HdrTypes.UDP) { - int dstPort = path.getPort(); + int dstPort = path.getRemotePort(); ScionHeaderParser.writeUdpOverlayHeader(buffer, payloadLength, srcPort, dstPort); } } diff --git a/src/main/java/org/scion/jpan/Path.java b/src/main/java/org/scion/jpan/Path.java index 889ab545f..bf3ad6c63 100644 --- a/src/main/java/org/scion/jpan/Path.java +++ b/src/main/java/org/scion/jpan/Path.java @@ -15,56 +15,29 @@ package org.scion.jpan; import java.net.*; -import java.util.Arrays; /** * A Path is an InetSocketAddress/ISD/AS of a destination host plus a path to that host. * *

This class is thread safe. */ -public abstract class Path extends InetSocketAddress { - private final long dstIsdAs; +public interface Path { - protected Path(long dstIsdAs, InetAddress dstIP, int dstPort) { - super(dstIP, dstPort); - this.dstIsdAs = dstIsdAs; - } + byte[] getRawPath(); - public abstract byte[] getRawPath(); + InetSocketAddress getFirstHopAddress() throws UnknownHostException; - public abstract InetSocketAddress getFirstHopAddress() throws UnknownHostException; + int getRemotePort(); - @Deprecated // Use getPort() instead - public int getRemotePort() { - return getPort(); - } + InetAddress getRemoteAddress(); - @Deprecated // Use getAddress() instead - public InetAddress getRemoteAddress() { - return getAddress(); - } - - @Deprecated // Use getIsdAs() instead - public long getRemoteIsdAs() { - return dstIsdAs; - } + long getRemoteIsdAs(); /** * @return ISD/AS of the remote host. */ - public long getIsdAs() { - return dstIsdAs; - } + long getIsdAs(); @Override - public String toString() { - return "Path{" - + "ISD/AS=" - + ScionUtil.toStringIA(dstIsdAs) - + ", address=" - + super.toString() - + ", pathRaw=" - + Arrays.toString(getRawPath()) - + '}'; - } + String toString(); } diff --git a/src/main/java/org/scion/jpan/PathImpl.java b/src/main/java/org/scion/jpan/PathImpl.java new file mode 100644 index 000000000..a56ad2aef --- /dev/null +++ b/src/main/java/org/scion/jpan/PathImpl.java @@ -0,0 +1,70 @@ +// Copyright 2023 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 org.scion.jpan; + +import java.net.*; +import java.util.Arrays; + +/** + * A Path is an InetSocketAddress/ISD/AS of a destination host plus a path to that host. + * + *

This class is thread safe. + */ +abstract class PathImpl extends InetSocketAddress implements Path { + private final long dstIsdAs; + + protected PathImpl(long dstIsdAs, InetAddress dstIP, int dstPort) { + super(dstIP, dstPort); + this.dstIsdAs = dstIsdAs; + } + + public abstract byte[] getRawPath(); + + public abstract InetSocketAddress getFirstHopAddress() throws UnknownHostException; + + @Deprecated // Use getPort() instead + public int getRemotePort() { + return getPort(); + } + + @Deprecated // Use getAddress() instead + public InetAddress getRemoteAddress() { + return getAddress(); + } + + @Deprecated // Use getIsdAs() instead + public long getRemoteIsdAs() { + return dstIsdAs; + } + + /** + * @return ISD/AS of the remote host. + */ + public long getIsdAs() { + return dstIsdAs; + } + + @Override + public String toString() { + return "Path{" + + "ISD/AS=" + + ScionUtil.toStringIA(dstIsdAs) + + ", address=" + + super.toString() + + ", pathRaw=" + + Arrays.toString(getRawPath()) + + '}'; + } +} diff --git a/src/main/java/org/scion/jpan/RequestPath.java b/src/main/java/org/scion/jpan/RequestPath.java index 650adcb07..3cb4ef75f 100644 --- a/src/main/java/org/scion/jpan/RequestPath.java +++ b/src/main/java/org/scion/jpan/RequestPath.java @@ -15,9 +15,6 @@ package org.scion.jpan; import java.net.InetAddress; -import java.net.InetSocketAddress; -import java.net.UnknownHostException; -import java.util.concurrent.atomic.AtomicReference; import org.scion.jpan.proto.daemon.Daemon; /** @@ -28,34 +25,11 @@ *

A RequestPath is immutable except for an atomic reference to PathDetails (PathDetails are * immutable). The reference may be updated, for example when a path expires. */ -public class RequestPath extends Path { - - private final AtomicReference details = new AtomicReference<>(); +public interface RequestPath extends Path { static RequestPath create(Daemon.Path path, long dstIsdAs, InetAddress dstIP, int dstPort) { - return new RequestPath(path, dstIsdAs, dstIP, dstPort); - } - - private RequestPath(Daemon.Path path, long dstIsdAs, InetAddress dstIP, int dstPort) { - super(dstIsdAs, dstIP, dstPort); - this.details.set(PathDetails.create(path, dstIP, dstPort)); - } - - @Override - public InetSocketAddress getFirstHopAddress() throws UnknownHostException { - return getDetails().getFirstHopAddress(); + return RequestPathImpl.create(path, dstIsdAs, dstIP, dstPort); } - @Override - public byte[] getRawPath() { - return getDetails().getRawPath(); - } - - public PathDetails getDetails() { - return details.get(); - } - - void setDetails(PathDetails details) { - this.details.set(details); - } + PathDetails getDetails(); } diff --git a/src/main/java/org/scion/jpan/RequestPathImpl.java b/src/main/java/org/scion/jpan/RequestPathImpl.java new file mode 100644 index 000000000..cd0dc28b0 --- /dev/null +++ b/src/main/java/org/scion/jpan/RequestPathImpl.java @@ -0,0 +1,61 @@ +// Copyright 2023 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 org.scion.jpan; + +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.UnknownHostException; +import java.util.concurrent.atomic.AtomicReference; +import org.scion.jpan.proto.daemon.Daemon; + +/** + * A RequestPath is a Path with additional meta information such as bandwidth, latency or geo + * coordinates. RequestPaths are created/returned by the ScionService when requesting a new path + * from the control service. + * + *

A RequestPath is immutable except for an atomic reference to PathDetails (PathDetails are + * immutable). The reference may be updated, for example when a path expires. + */ +class RequestPathImpl extends PathImpl implements RequestPath { + + private final AtomicReference details = new AtomicReference<>(); + + static RequestPathImpl create(Daemon.Path path, long dstIsdAs, InetAddress dstIP, int dstPort) { + return new RequestPathImpl(path, dstIsdAs, dstIP, dstPort); + } + + private RequestPathImpl(Daemon.Path path, long dstIsdAs, InetAddress dstIP, int dstPort) { + super(dstIsdAs, dstIP, dstPort); + this.details.set(PathDetails.create(path, dstIP, dstPort)); + } + + @Override + public InetSocketAddress getFirstHopAddress() throws UnknownHostException { + return getDetails().getFirstHopAddress(); + } + + @Override + public byte[] getRawPath() { + return getDetails().getRawPath(); + } + + public PathDetails getDetails() { + return details.get(); + } + + void setDetails(PathDetails details) { + this.details.set(details); + } +} diff --git a/src/main/java/org/scion/jpan/ResponsePath.java b/src/main/java/org/scion/jpan/ResponsePath.java index dee759e48..8a3ffb451 100644 --- a/src/main/java/org/scion/jpan/ResponsePath.java +++ b/src/main/java/org/scion/jpan/ResponsePath.java @@ -25,16 +25,9 @@ * *

A ResponsePath is immutable and thus thread safe. */ -public class ResponsePath extends Path { +public interface ResponsePath extends Path { - private final InetSocketAddress firstHopAddress; - // The ResponsePath gets source information from the incoming packet. - private final long srcIsdAs; - private final InetAddress srcAddress; - private final int srcPort; - private final byte[] pathRaw; - - public static ResponsePath create( + static ResponsePath create( byte[] rawPath, long srcIsdAs, InetAddress srcIP, @@ -43,61 +36,13 @@ public static ResponsePath create( InetAddress dstIP, int dstPort, InetSocketAddress firstHopAddress) { - return new ResponsePath( + return ResponsePathImpl.create( rawPath, srcIsdAs, srcIP, srcPort, dstIsdAs, dstIP, dstPort, firstHopAddress); } - private ResponsePath( - byte[] rawPath, - long srcIsdAs, - InetAddress srcIP, - int srcPort, - long dstIsdAs, - InetAddress dstIP, - int dstPort, - InetSocketAddress firstHopAddress) { - super(dstIsdAs, dstIP, dstPort); - this.firstHopAddress = firstHopAddress; - this.srcIsdAs = srcIsdAs; - this.srcAddress = srcIP; - this.srcPort = srcPort; - this.pathRaw = rawPath; - } - - @Override - public InetSocketAddress getFirstHopAddress() { - return firstHopAddress; - } - - public long getLocalIsdAs() { - return srcIsdAs; - } - - public InetAddress getLocalAddress() { - return srcAddress; - } + long getLocalIsdAs(); - public int getLocalPort() { - return srcPort; - } + InetAddress getLocalAddress(); - @Override - public byte[] getRawPath() { - return pathRaw; - } - - @Override - public String toString() { - return "ResponsePath{" - + super.toString() - + ", firstHopAddress=" - + firstHopAddress - + ", localIsdAs=" - + srcIsdAs - + ", localAddress=" - + srcAddress - + ", localPort=" - + srcPort - + '}'; - } + int getLocalPort(); } diff --git a/src/main/java/org/scion/jpan/ResponsePathImpl.java b/src/main/java/org/scion/jpan/ResponsePathImpl.java new file mode 100644 index 000000000..1983efe60 --- /dev/null +++ b/src/main/java/org/scion/jpan/ResponsePathImpl.java @@ -0,0 +1,103 @@ +// Copyright 2023 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 org.scion.jpan; + +import java.net.InetAddress; +import java.net.InetSocketAddress; + +/** + * A ResponsePath is created/returned when receiving a packet. Besides being a Path, it contains + * ISD/AS, IP and port of the local host. This is mostly for convenience to avoid looking up this + * information, but it also ensures that the return packet header contains the exact information + * sent/expected by the client. + * + *

A ResponsePath is immutable and thus thread safe. + */ +class ResponsePathImpl extends PathImpl implements ResponsePath { + + private final InetSocketAddress firstHopAddress; + // The ResponsePath gets source information from the incoming packet. + private final long srcIsdAs; + private final InetAddress srcAddress; + private final int srcPort; + private final byte[] pathRaw; + + public static ResponsePathImpl create( + byte[] rawPath, + long srcIsdAs, + InetAddress srcIP, + int srcPort, + long dstIsdAs, + InetAddress dstIP, + int dstPort, + InetSocketAddress firstHopAddress) { + return new ResponsePathImpl( + rawPath, srcIsdAs, srcIP, srcPort, dstIsdAs, dstIP, dstPort, firstHopAddress); + } + + private ResponsePathImpl( + byte[] rawPath, + long srcIsdAs, + InetAddress srcIP, + int srcPort, + long dstIsdAs, + InetAddress dstIP, + int dstPort, + InetSocketAddress firstHopAddress) { + super(dstIsdAs, dstIP, dstPort); + this.firstHopAddress = firstHopAddress; + this.srcIsdAs = srcIsdAs; + this.srcAddress = srcIP; + this.srcPort = srcPort; + this.pathRaw = rawPath; + } + + @Override + public InetSocketAddress getFirstHopAddress() { + return firstHopAddress; + } + + public long getLocalIsdAs() { + return srcIsdAs; + } + + public InetAddress getLocalAddress() { + return srcAddress; + } + + public int getLocalPort() { + return srcPort; + } + + @Override + public byte[] getRawPath() { + return pathRaw; + } + + @Override + public String toString() { + return "ResponsePath{" + + super.toString() + + ", firstHopAddress=" + + firstHopAddress + + ", localIsdAs=" + + srcIsdAs + + ", localAddress=" + + srcAddress + + ", localPort=" + + srcPort + + '}'; + } +} diff --git a/src/main/java/org/scion/jpan/ScionDatagramChannel.java b/src/main/java/org/scion/jpan/ScionDatagramChannel.java index 84f49ac7c..e9f80ee83 100644 --- a/src/main/java/org/scion/jpan/ScionDatagramChannel.java +++ b/src/main/java/org/scion/jpan/ScionDatagramChannel.java @@ -89,11 +89,11 @@ public int send(ByteBuffer srcBuffer, SocketAddress destination) throws IOExcept throw new IllegalArgumentException("Address must be of type InetSocketAddress."); } if (destination instanceof Path) { - return sendPath(srcBuffer, (Path) destination); + return send(srcBuffer, (Path) destination); } InetSocketAddress dst = (InetSocketAddress) destination; - Path path = getPathPolicy().filter(getOrCreateService().getPaths(dst)); - return sendPath(srcBuffer, path); + Path path = getOrCreateService().lookupAndGetPath(dst, getPathPolicy()); + return send(srcBuffer, path); } /** @@ -108,7 +108,7 @@ public int send(ByteBuffer srcBuffer, SocketAddress destination) throws IOExcept * cannot be resolved to an ISD/AS. * @see java.nio.channels.DatagramChannel#send(ByteBuffer, SocketAddress) */ - public int sendPath(ByteBuffer srcBuffer, Path path) throws IOException { + public int send(ByteBuffer srcBuffer, Path path) throws IOException { writeLock().lock(); try { ByteBuffer buffer = getBufferSend(srcBuffer.remaining()); @@ -168,7 +168,7 @@ public int write(ByteBuffer src) throws IOException { try { checkOpen(); checkConnected(true); - Path path = getRemoteAddress(); + Path path = getConnectionPath(); ByteBuffer buffer = getBufferSend(src.remaining()); int len = src.remaining(); diff --git a/src/main/java/org/scion/jpan/ScionDatagramSocket.java b/src/main/java/org/scion/jpan/ScionDatagramSocket.java index 3f4229b35..6c2894ba5 100644 --- a/src/main/java/org/scion/jpan/ScionDatagramSocket.java +++ b/src/main/java/org/scion/jpan/ScionDatagramSocket.java @@ -290,13 +290,13 @@ public void send(DatagramPacket packet) throws IOException { Path path; if (channel.isConnected()) { - path = channel.getRemoteAddress(); + path = channel.getConnectionPath(); } else { InetSocketAddress addr = (InetSocketAddress) packet.getSocketAddress(); synchronized (pathCache) { path = pathCache.get(addr); if (path == null) { - path = channel.getPathPolicy().filter(channel.getOrCreateService2().getPaths(addr)); + path = channel.getOrCreateService().lookupAndGetPath(addr, channel.getPathPolicy()); } else if (path instanceof RequestPath && ((RequestPath) path).getDetails().getExpiration() > Instant.now().getEpochSecond()) { diff --git a/src/main/java/org/scion/jpan/ScionService.java b/src/main/java/org/scion/jpan/ScionService.java index 41569f518..120e55c75 100644 --- a/src/main/java/org/scion/jpan/ScionService.java +++ b/src/main/java/org/scion/jpan/ScionService.java @@ -307,10 +307,8 @@ List getPathListDaemon(long srcIsdAs, long dstIsdAs) { * @return All paths returned by the path service. * @throws IOException if an errors occurs while querying paths. */ - @Deprecated // Please use lookUp() instead + @Deprecated // Please use lookup() instead public List getPaths(InetSocketAddress dstAddress) throws IOException { - // TODO this is ambiguous .... InetSocketAddress argument - // Use getHostString() to avoid DNS reverse lookup. ScionAddress sa = getScionAddress(dstAddress.getHostString()); return getPaths(sa.getIsdAs(), sa.getInetAddress(), dstAddress.getPort()); } @@ -322,15 +320,7 @@ public List getPaths(InetSocketAddress dstAddress) throws IOExcepti * @param dstScionAddress Destination IP address. Must belong to a SCION enabled end host. * @return All paths returned by the path service. */ - // TODO this is bad!!! Why ISD + SCION-Address????? - // TODO this is bad!!! Why ISD + SCION-Address????? - // TODO this is bad!!! Why ISD + SCION-Address????? - // TODO this is bad!!! Why ISD + SCION-Address????? - // TODO this is bad!!! Why ISD + SCION-Address????? - // TODO this is bad!!! Why ISD + SCION-Address????? - // TODO this is bad!!! Why ISD + SCION-Address????? public List getPaths(long dstIsdAs, InetSocketAddress dstScionAddress) { - // TODO change method API name to make clear that this requires a SCION IP. return getPaths(dstIsdAs, dstScionAddress.getAddress(), dstScionAddress.getPort()); } @@ -341,19 +331,35 @@ public List getPaths(long dstIsdAs, InetSocketAddress dstScionAddre * @return All paths returned by the path service. */ public List getPaths(RequestPath path) { - return getPaths(path.getIsdAs(), path.getAddress(), path.getPort()); + return getPaths(path.getIsdAs(), path.getRemoteAddress(), path.getRemotePort()); } /** * Request paths from the local ISD/AS to the destination. * * @param dstIsdAs Destination ISD/AS - * @param dstAddress Destination IP address + * @param dstAddress A SCION-enabled Destination IP address * @param dstPort Destination port * @return All paths returned by the path service. Returns an empty list if no paths are found. */ public List getPaths(long dstIsdAs, InetAddress dstAddress, int dstPort) { - return getPaths(RequestPath.fromScionIP(dstIsdAs, dstAddress, dstPort)); + return getPaths(ScionAddress.create(dstIsdAs, dstAddress), dstPort); + } + + /** + * Resolves the address to a SCION address, request paths, and selects a path using the policy. + * + * @param dstAddr Destination address + * @param policy path policy + * @return All paths returned by the path service. + * @throws ScionException if the DNS/TXT lookup did not return a (valid) SCION address. + */ + public RequestPath lookupAndGetPath(InetSocketAddress dstAddr, PathPolicy policy) + throws ScionException { + if (policy == null) { + policy = PathPolicy.DEFAULT; + } + return policy.filter(getPaths(lookupAddress(dstAddr.getHostString()), dstAddr.getPort())); } /** @@ -362,12 +368,14 @@ public List getPaths(long dstIsdAs, InetAddress dstAddress, int dst * @param dstAddress Destination SCION address * @return All paths returned by the path service. */ - public List getPaths(Path dstAddress) { + public List getPaths(ScionAddress dstAddress, int dstPort) { long srcIsdAs = getLocalIsdAs(); List paths = getPathList(srcIsdAs, dstAddress.getIsdAs()); List scionPaths = new ArrayList<>(paths.size()); for (int i = 0; i < paths.size(); i++) { - scionPaths.add(RequestPath.create(paths.get(i), dstAddress)); + scionPaths.add( + RequestPath.create( + paths.get(i), dstAddress.getIsdAs(), dstAddress.getInetAddress(), dstPort)); } return scionPaths; } @@ -439,25 +447,14 @@ public ScionAddress getScionAddress(String hostName) throws ScionException { } /** - * Uses DNS and hostfiles to look up a SCIOn enabled IP address for a give host string. - * - * @param hostName hostName of the host to resolve - * @return A ScionSocketAddress - * @throws ScionException if the DNS/TXT lookup did not return a (valid) SCION address. - */ - public Path lookupSocketAddress(String hostName, int port) throws ScionException { - return RequestPath.fromScionAddress(lookupAddress(hostName), port); - } - - /** - * Uses DNS and hostfiles to look up a SCIOn enabled IP address for a give host string. + * Uses DNS and hostfiles to look up a SCION enabled IP address for a give host string. * * @param hostName hostName of the host to resolve * @return A ScionAddress * @throws ScionException if the DNS/TXT lookup did not return a (valid) SCION address. */ @Deprecated // TODO NOW: remove this lookupAddress() method in favor of lookupSocketAddress() - private ScionAddress lookupAddress(String hostName) throws ScionException { + ScionAddress lookupAddress(String hostName) throws ScionException { ScionAddress scionAddress = scionAddressCache.get(hostName); if (scionAddress != null) { return scionAddress; @@ -605,7 +602,7 @@ boolean refreshPath(RequestPath path, PathPolicy pathPolicy, int expiryMargin) { } // expired, get new path RequestPath newPath = pathPolicy.filter(getPaths(path)); - path.setDetails(newPath.getDetails()); + ((RequestPathImpl) path).setDetails(newPath.getDetails()); return true; } } diff --git a/src/main/java/org/scion/jpan/ScmpChannel.java b/src/main/java/org/scion/jpan/ScmpChannel.java index 0a36d0bb0..b03d44fa5 100644 --- a/src/main/java/org/scion/jpan/ScmpChannel.java +++ b/src/main/java/org/scion/jpan/ScmpChannel.java @@ -357,9 +357,9 @@ void sendEchoResponses() throws IOException { buffer.flip(); msg.setSizeSent(buffer.remaining()); sendRaw(buffer, path); - log.info("Responded to SCMP {} from {}", type, path.getAddress()); + log.info("Responded to SCMP {} from {}", type, path.getRemoteAddress()); } else { - log.info("Dropped SCMP message with type {} from {}", type, path.getAddress()); + log.info("Dropped SCMP message with type {} from {}", type, path.getRemoteAddress()); } } } finally { @@ -409,7 +409,7 @@ public InetSocketAddress getLocalAddress() throws IOException { return channel.getLocalAddress(); } - public RequestPath getRemoteAddress() throws IOException { + public InetSocketAddress getRemoteAddress() throws IOException { return channel.getRemoteAddress(); } diff --git a/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java b/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java index dd19394ec..4893f6e43 100644 --- a/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java +++ b/src/test/java/org/scion/jpan/api/DatagramChannelApiTest.java @@ -526,7 +526,7 @@ void write_expiredRequestPath() throws IOException { ByteBuffer sendBuf = ByteBuffer.wrap(PingPongChannelHelper.MSG.getBytes()); try { channel.write(sendBuf); - RequestPath newPath = channel.getRemoteAddress(); + RequestPath newPath = channel.getConnectionPath(); assertTrue( newPath.getDetails().getExpiration() > expiredPath.getDetails().getExpiration()); assertTrue(Instant.now().getEpochSecond() < newPath.getDetails().getExpiration()); diff --git a/src/test/java/org/scion/jpan/testutil/PingPongHelperBase.java b/src/test/java/org/scion/jpan/testutil/PingPongHelperBase.java index 264c02080..1b7b62556 100644 --- a/src/test/java/org/scion/jpan/testutil/PingPongHelperBase.java +++ b/src/test/java/org/scion/jpan/testutil/PingPongHelperBase.java @@ -107,7 +107,7 @@ void runPingPong(ServerFactory serverFactory, ClientFactory clientFactory, boole } InetSocketAddress serverAddress = servers[0].getLocalAddress(); MockDNS.install(MockNetwork.TINY_SRV_ISD_AS, serverAddress.getAddress()); - RequestPath requestPath = Scion.defaultService().getPaths(serverAddress).get(0); + RequestPath requestPath = Scion.defaultService().lookupAndGetPath(serverAddress, null); Thread[] clients = new Thread[nClients]; for (int i = 0; i < clients.length; i++) { From 993244ec236fd2279c829c3542f6547ac36a8e8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilmann=20Z=C3=A4schke?= Date: Mon, 10 Jun 2024 15:27:50 +0200 Subject: [PATCH 13/13] Path interface --- src/main/java/org/scion/jpan/AbstractDatagramChannel.java | 2 +- src/main/java/org/scion/jpan/Path.java | 5 ----- src/main/java/org/scion/jpan/ScionService.java | 2 +- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java index aad70678f..5b61d062f 100644 --- a/src/main/java/org/scion/jpan/AbstractDatagramChannel.java +++ b/src/main/java/org/scion/jpan/AbstractDatagramChannel.java @@ -611,7 +611,7 @@ protected void buildHeader( rawPath.length, srcIA, srcAddress.getAddress(), - path.getIsdAs(), + path.getRemoteIsdAs(), path.getRemoteAddress().getAddress(), hdrType, cfgTrafficClass); diff --git a/src/main/java/org/scion/jpan/Path.java b/src/main/java/org/scion/jpan/Path.java index bf3ad6c63..6a981c0c1 100644 --- a/src/main/java/org/scion/jpan/Path.java +++ b/src/main/java/org/scion/jpan/Path.java @@ -33,11 +33,6 @@ public interface Path { long getRemoteIsdAs(); - /** - * @return ISD/AS of the remote host. - */ - long getIsdAs(); - @Override String toString(); } diff --git a/src/main/java/org/scion/jpan/ScionService.java b/src/main/java/org/scion/jpan/ScionService.java index 120e55c75..84c7598c4 100644 --- a/src/main/java/org/scion/jpan/ScionService.java +++ b/src/main/java/org/scion/jpan/ScionService.java @@ -331,7 +331,7 @@ public List getPaths(long dstIsdAs, InetSocketAddress dstScionAddre * @return All paths returned by the path service. */ public List getPaths(RequestPath path) { - return getPaths(path.getIsdAs(), path.getRemoteAddress(), path.getRemotePort()); + return getPaths(path.getRemoteIsdAs(), path.getRemoteAddress(), path.getRemotePort()); } /**