Skip to content

Commit

Permalink
SCMP echo problems, see #96
Browse files Browse the repository at this point in the history
  • Loading branch information
Tilmann Zäschke committed Jun 21, 2024
1 parent 369436f commit 60b8245
Show file tree
Hide file tree
Showing 10 changed files with 103 additions and 17 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
[#94](https://github.com/scionproto-contrib/jpan/pull/94)
- **BREAKING CHANGE**: removed `RequestPath` and `ScionAddress` from public API.
[#95](https://github.com/scionproto-contrib/jpan/pull/95)
- Better error message for SCMP echo in local AS
[#96](https://github.com/scionproto-contrib/jpan/issues/96)

### Fixed
- Fixed locking and resizing of buffers. [#68](https://github.com/scionproto-contrib/jpan/pull/68)
Expand Down
10 changes: 9 additions & 1 deletion src/main/java/org/scion/jpan/PathMetadata.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,17 @@ static PathMetadata create(Daemon.Path path, InetAddress dstIP, int dstPort) {
private PathMetadata(Daemon.Path path, InetAddress dstIP, int dstPort) {
this.pathProtoc = path;
this.pathRaw = path.getRaw().toByteArray();
// path length 0 means "local AS"
if (getRawPath().length == 0) {
// local AS has path length 0
// See issue https://github.com/scionproto-contrib/jpan/issues/96
// if (dstIP.isAnyLocalAddress()) {
// // This is an SCMP request
// ScionService service = Scion.defaultService();
// firstHop =
// ScionUtil.parseInetSocketAddress(service.getBorderRouterStrings().get(0));
// } else {
firstHop = new InetSocketAddress(dstIP, dstPort);
// }
} else {
firstHop = getFirstHopAddress(path);
}
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/org/scion/jpan/ScionService.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.util.*;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
import java.util.stream.Collectors;
import org.scion.jpan.internal.*;
import org.scion.jpan.proto.control_plane.SegmentLookupServiceGrpc;
import org.scion.jpan.proto.daemon.Daemon;
Expand Down Expand Up @@ -605,4 +606,14 @@ InetAddress getExternalIP(InetSocketAddress firstHopAddress) {
}
}
}

List<String> getBorderRouterStrings() {
if (daemonStub != null) {
return getInterfaces().values().stream()
.map(i -> i.getAddress().getAddress())
.collect(Collectors.toList());

Check warning on line 614 in src/main/java/org/scion/jpan/ScionService.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/scion/jpan/ScionService.java#L612-L614

Added lines #L612 - L614 were not covered by tests
} else {
return bootstrapper.getBorderRouterAddresses();

Check warning on line 616 in src/main/java/org/scion/jpan/ScionService.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/scion/jpan/ScionService.java#L616

Added line #L616 was not covered by tests
}
}
}
13 changes: 13 additions & 0 deletions src/main/java/org/scion/jpan/ScionUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@

package org.scion.jpan;

import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.UnknownHostException;
import org.scion.jpan.internal.PathRawParser;

/** Scion utility functions. */
Expand Down Expand Up @@ -190,5 +193,15 @@ public static long extractAs(long isdAs) {
return isdAs & MAX_AS;
}

static InetSocketAddress parseInetSocketAddress(String addrStr) {
try {
int posColon = addrStr.indexOf(':');
InetAddress inetAddress = InetAddress.getByName(addrStr.substring(0, posColon));
return new InetSocketAddress(inetAddress, Integer.parseInt(addrStr.substring(posColon + 1)));
} catch (UnknownHostException e) {
throw new IllegalArgumentException(e);

Check warning on line 202 in src/main/java/org/scion/jpan/ScionUtil.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/scion/jpan/ScionUtil.java#L198-L202

Added lines #L198 - L202 were not covered by tests
}
}

private ScionUtil() {}
}
5 changes: 5 additions & 0 deletions src/main/java/org/scion/jpan/ScmpChannel.java
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,11 @@ Scmp.TimedMessage sendEchoRequest(Scmp.EchoMessage request) throws IOException {
writeLock().lock();
try {
Path path = request.getPath();
if (path.getRawPath().length == 0
&& path.getFirstHopAddress().getAddress().isAnyLocalAddress()) {
throw new ScionException(
"Cannot ping service address in local AS: " + path.getFirstHopAddress());
}
super.channel().connect(path.getFirstHopAddress());
ByteBuffer buffer = getBufferSend(DEFAULT_BUFFER_SIZE);
// EchoHeader = 8 + data
Expand Down
8 changes: 8 additions & 0 deletions src/main/java/org/scion/jpan/internal/ScionBootstrapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,14 @@ public String getBorderRouterAddress(int interfaceId) {
throw new ScionRuntimeException("No router found with interface ID " + interfaceId);
}

public List<String> getBorderRouterAddresses() {
List<String> result = new ArrayList<>();

Check warning on line 201 in src/main/java/org/scion/jpan/internal/ScionBootstrapper.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/scion/jpan/internal/ScionBootstrapper.java#L201

Added line #L201 was not covered by tests
for (BorderRouter br : borderRouters) {
result.add(br.internalAddress);
}
return result;

Check warning on line 205 in src/main/java/org/scion/jpan/internal/ScionBootstrapper.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/scion/jpan/internal/ScionBootstrapper.java#L203-L205

Added lines #L203 - L205 were not covered by tests
}

public int getLocalMtu() {
return this.localMtu;
}
Expand Down
35 changes: 30 additions & 5 deletions src/test/java/org/scion/jpan/api/SCMPTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,13 @@ void echo() throws IOException {
}

@Test
void echo_localAS() throws IOException {
testEcho(this::getPathToLocalAS);
void echo_localAS_BR() throws IOException {
testEcho(this::getPathToLocalAS_BR);
}

@Test
void echo_localAS_SVC() {
assertThrows(ScionException.class, () -> testEcho(this::getPathToLocalAS_SVC));
}

private void testEcho(Supplier<Path> path) throws IOException {
Expand Down Expand Up @@ -194,8 +199,13 @@ void traceroute() throws IOException {
}

@Test
void traceroute_localAS() throws IOException {
testTraceroute(this::getPathToLocalAS, 0);
void traceroute_localAS_BR() throws IOException {
testTraceroute(this::getPathToLocalAS_BR, 0);
}

@Test
void traceroute_localAS_SVC() throws IOException {
testTraceroute(this::getPathToLocalAS_SVC, 0);
}

private void testTraceroute(Supplier<Path> path, int nHops) throws IOException {
Expand Down Expand Up @@ -307,10 +317,11 @@ private Path getPathTo112(InetAddress dstAddress) {
return service.getPaths(dstIA, dstAddress, Constants.SCMP_PORT).get(0);
}

private Path getPathToLocalAS() {
private Path getPathToLocalAS_BR() {
ScionService service = Scion.defaultService();
long dstIA = service.getLocalIsdAs();
try {
// Border router address
InetAddress addr = InetAddress.getByName(MockNetwork.BORDER_ROUTER_HOST);
int port = MockNetwork.BORDER_ROUTER_PORT1;
List<Path> paths = service.getPaths(dstIA, new InetSocketAddress(addr, port));
Expand All @@ -320,6 +331,20 @@ private Path getPathToLocalAS() {
}
}

private Path getPathToLocalAS_SVC() {
ScionService service = Scion.defaultService();
long dstIA = service.getLocalIsdAs();
try {
// Service address
InetAddress addr = InetAddress.getByAddress(new byte[] {0, 0, 0, 0});
int port = Constants.SCMP_PORT;
List<Path> paths = service.getPaths(dstIA, new InetSocketAddress(addr, port));
return paths.get(0);
} catch (UnknownHostException e) {
throw new RuntimeException(e);
}
}

@Test
void setUpScmpResponder_echo() throws IOException, InterruptedException {
MockNetwork.startTiny();
Expand Down
18 changes: 7 additions & 11 deletions src/test/java/org/scion/jpan/demo/ScmpEchoDemo.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,13 @@ public ScmpEchoDemo(int localPort) {
private static final Network network = Network.PRODUCTION;

public static void main(String[] args) throws IOException {
ScionService service = Scion.defaultService();
switch (network) {
case JUNIT_MOCK:
{
DemoTopology.configureMock();
MockDNS.install("1-ff00:0:112", "ip6-localhost", "::1");
ScmpEchoDemo demo = new ScmpEchoDemo();
demo.runDemo(service.getPaths(DemoConstants.ia112, serviceIP).get(0));
demo.runDemo(Scion.defaultService().getPaths(DemoConstants.ia112, serviceIP).get(0));
DemoTopology.shutDown();
break;
}
Expand All @@ -87,15 +86,18 @@ public static void main(String[] args) throws IOException {
// System.setProperty(Constants.PROPERTY_DAEMON, DemoConstants.daemon1111_minimal);

ScmpEchoDemo demo = new ScmpEchoDemo();
demo.runDemo(service.getPaths(DemoConstants.ia211, serviceIP).get(0));
// demo.runDemo(service.getPaths(DemoConstants.ia111, serviceIP).get(0));
// demo.runDemo(service.getPaths(DemoConstants.ia1111, serviceIP).get(0));
demo.runDemo(Scion.defaultService().getPaths(DemoConstants.ia211, serviceIP).get(0));
// Echo to local AS and on-path AS (111 is "on" the UP segment) is currently broken,
// see https://github.com/scionproto-contrib/jpan/issues/96
// demo.runDemo(Scion.defaultService().getPaths(DemoConstants.ia111, serviceIP).get(0));
// demo.runDemo(Scion.defaultService().getPaths(DemoConstants.ia1111, serviceIP).get(0));
break;
}
case PRODUCTION:
{
// Local port must be 30041 for networks that expect a dispatcher
ScmpEchoDemo demo = new ScmpEchoDemo(Constants.SCMP_PORT);
ScionService service = Scion.defaultService();
demo.runDemo(service.lookupAndGetPath("ethz.ch", Constants.SCMP_PORT, null));
demo.runDemo(service.getPaths(DemoConstants.iaOVGU, serviceIP).get(0));
break;
Expand Down Expand Up @@ -160,10 +162,4 @@ private static void println(String msg) {
System.out.println(msg);
}
}

private static InetSocketAddress toAddr(String addrString) throws UnknownHostException {
int posColon = addrString.indexOf(':');
InetAddress addr = InetAddress.getByName(addrString.substring(0, posColon));
return new InetSocketAddress(addr, Constants.SCMP_PORT);
}
}
2 changes: 2 additions & 0 deletions src/test/java/org/scion/jpan/demo/ScmpTracerouteDemo.java
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ public static void main(String[] args) throws IOException {
System.setProperty(Constants.PROPERTY_DAEMON, DemoConstants.daemon1111_minimal);
ScmpTracerouteDemo demo = new ScmpTracerouteDemo();
demo.runDemo(DemoConstants.ia211);
demo.runDemo(DemoConstants.ia111);
demo.runDemo(DemoConstants.ia1111);
break;
}
case PRODUCTION:
Expand Down
16 changes: 16 additions & 0 deletions src/test/java/org/scion/jpan/testutil/MockDaemon.java
Original file line number Diff line number Diff line change
Expand Up @@ -206,5 +206,21 @@ public void aS(Daemon.ASRequest req, StreamObserver<Daemon.ASResponse> responseO
responseObserver.onNext(replyBuilder.build());
responseObserver.onCompleted();
}

@Override
public void interfaces(
Daemon.InterfacesRequest req, StreamObserver<Daemon.InterfacesResponse> responseObserver) {
callCount.incrementAndGet();
Daemon.InterfacesResponse.Builder replyBuilder = Daemon.InterfacesResponse.newBuilder();
int i = -1;
for (String br : borderRouters) {
Daemon.Interface.Builder ifBuilder = Daemon.Interface.newBuilder();
ifBuilder.setAddress(Daemon.Underlay.newBuilder().setAddress(br).build());
// Interface IDs are currently not supported
replyBuilder.putInterfaces(i--, ifBuilder.build());
}
responseObserver.onNext(replyBuilder.build());
responseObserver.onCompleted();
}
}
}

0 comments on commit 60b8245

Please sign in to comment.