More SCMP cleanup (#15)

---------

Co-authored-by: Tilmann Zäschke <tilmann.zaeschke@inf.ethz.ch>

CHANGELOG.md

@@ -12,7 +12,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - BREAKING CHANGE: Changed maven artifactId to "client"
   [#9](https://github.com/tzaeschke/phtree-cpp/pull/9)
 - BREAKING CHANGE: SCMP refactoring, renamed several SCMP related classes.
-  [#14](https://github.com/tzaeschke/phtree-cpp/pull/14)
+  [#14](https://github.com/tzaeschke/phtree-cpp/pull/14), 
+  [#15](https://github.com/tzaeschke/phtree-cpp/pull/15)
 
 ### Fixed
 - Fixed SCMP timeout and error handling (IOExceptions + SCMP errors).

TODO.md

@@ -76,8 +76,8 @@
   - BUG: Ping to iaOVGU causes 3 CS requests that have type UP,CORE,CORE....?
   - FIX: Ask why requesting an UP segment effectively returns a DOWN segment
     (it needs to be reversed + the SegID needs to be XORed)
-  - FIX: API ping takes 4-5 extra milliseconds
 - Docs:
+  Add docs for setting up an environment
   https://github.com/marcfrei/scion-time#setting-up-a-scion-test-environment
   https://github.com/netsec-ethz/lightning-filter#develop
 Discuss required for 0.1.0:

src/main/java/org/scion/AbstractDatagramChannel.java

@@ -191,16 +191,11 @@ protected ResponsePath receiveFromChannel(
       }
       buffer.flip();
 
-      String validationResult = ScionHeaderParser.validate(buffer.asReadOnlyBuffer());
-      if (validationResult != null) {
-        if (cfgReportFailedValidation) {
-          throw new ScionException(validationResult);
-        }
+      if (!validate(buffer.asReadOnlyBuffer())) {
         continue;
       }
 
       InternalConstants.HdrTypes hdrType = ScionHeaderParser.extractNextHeader(buffer);
-
       // From here on we use linear reading using the buffer's position() mechanism
       buffer.position(ScionHeaderParser.extractHeaderLength(buffer));
       // Check for extension headers.
@@ -215,7 +210,7 @@ protected ResponsePath receiveFromChannel(
     }
   }
 
-  private InternalConstants.HdrTypes receiveExtensionHeader(
+  protected InternalConstants.HdrTypes receiveExtensionHeader(
       ByteBuffer buffer, InternalConstants.HdrTypes hdrType) {
     if (hdrType == InternalConstants.HdrTypes.END_TO_END
         || hdrType == InternalConstants.HdrTypes.HOP_BY_HOP) {
@@ -229,9 +224,11 @@ private InternalConstants.HdrTypes receiveExtensionHeader(
     return hdrType;
   }
 
-  private void receiveScmp(ByteBuffer buffer, Path path) {
+  protected void receiveScmp(ByteBuffer buffer, Path path) {
     Scmp.ScmpType type = ScmpParser.extractType(buffer);
-    checkListeners(ScmpParser.consume(buffer, Scmp.createMessage(type, path)));
+    Scmp.Message msg = Scmp.createMessage(type, path);
+    ScmpParser.consume(buffer, msg);
+    checkListeners(msg);
   }
 
   protected void checkListeners(Scmp.Message scmpMsg) {
@@ -267,6 +264,10 @@ protected void checkConnected(boolean requiredState) {
   }
 
   public synchronized boolean isConnected() {
+    if (!channel.isOpen() || !channel.isConnected()) {
+      // This may happen when the channel gets disconnected due to being interrupted.
+      isConnected = false;
+    }
     return isConnected;
   }
 
@@ -346,7 +347,7 @@ protected void buildHeaderNoRefresh(
     } else {
       // For sending request path we need to have a valid local external address.
       // For a valid local external address we need to be connected.
-      if (!isConnected) {
+      if (!isConnected()) {
         isConnected = true;
         connection = path.getFirstHopAddress();
         channel.connect(connection);
@@ -395,4 +396,12 @@ private RequestPath updatePath(RequestPath path) throws IOException {
     }
     return newPath;
   }
+
+  protected boolean validate(ByteBuffer buffer) throws ScionException {
+    String validationResult = ScionHeaderParser.validate(buffer.asReadOnlyBuffer());
+    if (validationResult != null && cfgReportFailedValidation) {
+      throw new ScionException(validationResult);
+    }
+    return validationResult == null;
+  }
 }

src/main/java/org/scion/Scmp.java

@@ -217,11 +217,34 @@ public void setMessageArgs(ScmpTypeCode sc, int identifier, int sequenceNumber)
     }
   }
 
-  public static class EchoMessage extends Message {
-    private byte[] data;
+  public abstract static class TimedMessage extends Message {
     private long nanoSeconds;
     private boolean timedOut = false;
 
+    private TimedMessage(ScmpTypeCode typeCode, int identifier, int sequenceNumber, Path path) {
+      super(typeCode, identifier, sequenceNumber, path);
+    }
+
+    public void setNanoSeconds(long nanoSeconds) {
+      this.nanoSeconds = nanoSeconds;
+    }
+
+    public long getNanoSeconds() {
+      return nanoSeconds;
+    }
+
+    public void setTimedOut() {
+      this.timedOut = true;
+    }
+
+    public boolean isTimedOut() {
+      return timedOut;
+    }
+  }
+
+  public static class EchoMessage extends TimedMessage {
+    private byte[] data;
+
     private EchoMessage(
         ScmpTypeCode typeCode, int identifier, int sequenceNumber, Path path, byte[] data) {
       super(typeCode, identifier, sequenceNumber, path);
@@ -245,31 +268,12 @@ public byte[] getData() {
     public void setData(byte[] data) {
       this.data = data;
     }
-
-    public void setNanoSeconds(long nanoSeconds) {
-      this.nanoSeconds = nanoSeconds;
-    }
-
-    public long getNanoSeconds() {
-      return nanoSeconds;
-    }
-
-    public void setTimedOut(long nanoSeconds) {
-      this.nanoSeconds = nanoSeconds;
-      this.timedOut = true;
-    }
-
-    public boolean isTimedOut() {
-      return timedOut;
-    }
   }
 
-  public static class TracerouteMessage extends Message {
+  public static class TracerouteMessage extends TimedMessage {
 
     private long isdAs;
     private long ifID;
-    private long nanoSeconds;
-    private boolean timedOut = false;
 
     private TracerouteMessage(
         ScmpTypeCode typeCode, int identifier, int sequenceNumber, Path path) {
@@ -284,12 +288,6 @@ public static TracerouteMessage createRequest(int sequenceNumber, Path path) {
       return new TracerouteMessage(ScmpTypeCode.TYPE_130, -1, sequenceNumber, path);
     }
 
-    public static TracerouteMessage createTimedOut(long nanoSeconds) {
-      TracerouteMessage r = new TracerouteMessage(null, -1, -1, null);
-      r.setNanoSeconds(nanoSeconds);
-      return r;
-    }
-
     public TracerouteMessage(
         ScmpTypeCode typeCode,
         int identifier,
@@ -310,14 +308,6 @@ public long getIfID() {
       return ifID;
     }
 
-    public void setNanoSeconds(long nanoSeconds) {
-      this.nanoSeconds = nanoSeconds;
-    }
-
-    public long getNanoSeconds() {
-      return nanoSeconds;
-    }
-
     @Override
     public String toString() {
       String echoMsgStr = getTypeCode().getText();
@@ -330,15 +320,6 @@ public void setTracerouteArgs(long isdAs, long ifID) {
       this.isdAs = isdAs;
       this.ifID = ifID;
     }
-
-    public void setTimedOut(long nanoSeconds) {
-      this.nanoSeconds = nanoSeconds;
-      this.timedOut = true;
-    }
-
-    public boolean isTimedOut() {
-      return timedOut;
-    }
   }
 
   static Scmp.Message createMessage(Scmp.ScmpType type, Path path) {