From 3ae027941be56a44cd2f2bc5c152e435c33cad1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilmann=20Z=C3=A4schke?= Date: Mon, 19 Aug 2024 13:50:57 +0200 Subject: [PATCH] Path duplicate detection --- CHANGELOG.md | 4 +- .../scion/jpan/internal/PathDeduplicator.java | 103 --------------- .../jpan/internal/PathDuplicationFilter.java | 117 ++++++++++++++++++ .../jpan/internal/PathRawParserLight.java | 65 ++++++++++ .../org/scion/jpan/internal/Segments.java | 19 ++- .../org/scion/jpan/demo/ScmpDemoDefault.java | 2 +- .../jpan/internal/SegmentsDefault112Test.java | 11 +- 7 files changed, 195 insertions(+), 126 deletions(-) delete mode 100644 src/main/java/org/scion/jpan/internal/PathDeduplicator.java create mode 100644 src/main/java/org/scion/jpan/internal/PathDuplicationFilter.java create mode 100644 src/main/java/org/scion/jpan/internal/PathRawParserLight.java diff --git a/CHANGELOG.md b/CHANGELOG.md index e70fbc41..e2e2d4b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,7 +42,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fixed some issues with IPv6 ASes - New option `EXPERIMENTAL_SCION_RESOLVER_MINIMIZE_REQUESTS` - "Integration" test for scionproto "default". - [#116](https://github.com/scionproto-contrib/jpan/pull/116) + [#114](https://github.com/scionproto-contrib/jpan/pull/114) +- Improved path duplication filtering. + [#117](https://github.com/scionproto-contrib/jpan/pull/117) ### Changed - Clean up TODO and deprecation info. [#100](https://github.com/scionproto-contrib/jpan/pull/100) diff --git a/src/main/java/org/scion/jpan/internal/PathDeduplicator.java b/src/main/java/org/scion/jpan/internal/PathDeduplicator.java deleted file mode 100644 index c6c691eb..00000000 --- a/src/main/java/org/scion/jpan/internal/PathDeduplicator.java +++ /dev/null @@ -1,103 +0,0 @@ -// Copyright 2024 ETH Zurich -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package org.scion.jpan.internal; - -import com.google.protobuf.ByteString; -import org.scion.jpan.proto.daemon.Daemon; - -import java.nio.ByteBuffer; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - -class PathDeduplicator { - - MultiMap paths = new MultiMap<>(); - - private static class Entry { - Daemon.Path path; - byte[] nullified; - - public Entry(Daemon.Path path, byte[] nullified) { - this.path = path; - this.nullified = nullified; - } - } - - void checkDuplicatePaths(Daemon.Path.Builder path) { - // Add, path to list, but avoid duplicates - ByteString raw = path.getRaw(); - // Create a hash code that ignores segment IDs and expiry date - byte[] nullifiedRaw = nullifySegmentIDsAndExpiry(path); - int hash = Arrays.hashCode(nullifiedRaw); - if (paths.contains(hash)) { - for (Entry otherPath : paths.get(hash)) { - byte[] otherNullifiedRaw = otherPath.nullified; - boolean equals = Arrays.equals(nullifiedRaw, otherNullifiedRaw); - if (equals) { - // duplicate! - // Which one doe we keep? Compare minimum expiration date. - - // TODO We should also nullify MAC and exp-date (not only time stamp) - // Better yet, we should just compare metadata! - // This would also create less objects. No need to nullify anything! - // We should only compare hopCount and ingress/egress IDs!! - if (true) throw new UnsupportedOperationException(); - - return; - } - } - } - // Add new path! - paths.put(hash, new Entry(path.build(), nullifiedRaw)); - } - - public List getPaths() { - List entries = paths.values(); - List result = new ArrayList<>(entries.size()); - for (Entry entry : entries) { - result.add(entry.path); - } - return result; - } - - private static byte[] nullifySegmentIDsAndExpiry(Daemon.Path.Builder path) { - // Nullify segment IDs and expiry date - - ByteString byteString = path.getRaw(); - ByteBuffer bb = byteString.asReadOnlyByteBuffer(); - int header = bb.getInt(); - - int hopCount0 = ByteUtil.readInt(header, 14, 6); - int hopCount1 = ByteUtil.readInt(header, 20, 6); - int hopCount2 = ByteUtil.readInt(header, 26, 6); - - if (hopCount0 > 0) { - // TODO we are also skipping the flags here, is that alright? - bb.getLong(); - } - if (hopCount1 > 0) { - bb.getLong(); - } - if (hopCount2 > 0) { - bb.getLong(); - } - - byte[] result = new byte[bb.remaining()]; - bb.get(result); - return result; - } - -} diff --git a/src/main/java/org/scion/jpan/internal/PathDuplicationFilter.java b/src/main/java/org/scion/jpan/internal/PathDuplicationFilter.java new file mode 100644 index 00000000..c90d6833 --- /dev/null +++ b/src/main/java/org/scion/jpan/internal/PathDuplicationFilter.java @@ -0,0 +1,117 @@ +// Copyright 2024 ETH Zurich +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package org.scion.jpan.internal; + +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import org.scion.jpan.proto.daemon.Daemon; + +class PathDuplicationFilter { + + MultiMap paths = new MultiMap<>(); + + private static class Entry { + Daemon.Path path; + int[] interfaces; + + public Entry(Daemon.Path path, int[] interfaces) { + this.path = path; + this.interfaces = interfaces; + } + + public void update(Daemon.Path path, int[] interfaces) { + this.path = path; + this.interfaces = interfaces; + } + } + + /** + * Add path to list, but avoid duplicates. A path is considered "duplicate" if it uses the same + * sequence of interface IDs. In case of a duplicate, we keep the path with the latest expiration + * date. + * + * @param path New path + */ + void checkDuplicatePaths(Daemon.Path.Builder path) { + // Create a hash code that uses only interface IDs + int[] interfaces = extractInterfaces(path); + int hash = Arrays.hashCode(interfaces); + if (paths.contains(hash)) { + for (Entry storedPath : paths.get(hash)) { + if (Arrays.equals(interfaces, storedPath.interfaces)) { + // Which one doe we keep? Compare minimum expiration date. + if (path.getExpiration().getSeconds() > storedPath.path.getExpiration().getSeconds()) { + storedPath.update(path.build(), interfaces); + } + return; + } + } + } + // Add new path! + paths.put(hash, new Entry(path.build(), interfaces)); + } + + public List getPaths() { + List entries = paths.values(); + List result = new ArrayList<>(entries.size()); + for (Entry entry : entries) { + result.add(entry.path); + } + return result; + } + + /** + * Extract used interfaces. Typically, for detecting duplicates, we could just compare the rwa + * byte[]. This would detect most duplicates. However, in some cases we get two paths that use + * identical interfaces but have different SegmentID, Expiration Date and different "unused" + * interfaces. For example, in the scionproto "default" topology, going from 1-ff00:0:111 to + * 1-ff00:0:112, we end up with two path that look externally like this: [494 > 103]. However, + * internally they look like this: + * + *

- segID=9858, timestamp=1723449803, [494, 0, 104, 103]
+ * - segID=9751, timestamp=1723449803, [494, 0, 105, 103]
+ * + *

The 104 vs 105 interface is not actually used and is an artifact of the path being + * shortened. The following methods considers these cases and only extracts interface IDs of + * interfaces that are actually used. + * + * @param path path + * @return interfaces + */ + private static int[] extractInterfaces(Daemon.Path.Builder path) { + ByteBuffer raw = path.getRaw().asReadOnlyByteBuffer(); + int[] segLen = PathRawParserLight.getSegments(raw); + int segCount = PathRawParserLight.calcSegmentCount(segLen); + int[] result = new int[PathRawParserLight.extractHopCount(segLen) * 2]; + int offset = 0; + int ifPos = 0; + for (int j = 0; j < segLen.length; j++) { + boolean flagC = PathRawParserLight.extractInfoFlagC(raw, j); + for (int i = offset; i < offset + segLen[j] - 1; i++) { + if (flagC) { + result[ifPos++] = PathRawParserLight.extractHopFieldEgress(raw, segCount, i); + result[ifPos++] = PathRawParserLight.extractHopFieldIngress(raw, segCount, i + 1); + } else { + result[ifPos++] = PathRawParserLight.extractHopFieldIngress(raw, segCount, i); + result[ifPos++] = PathRawParserLight.extractHopFieldEgress(raw, segCount, i + 1); + } + } + offset += segLen[j]; + } + return result; + } +} diff --git a/src/main/java/org/scion/jpan/internal/PathRawParserLight.java b/src/main/java/org/scion/jpan/internal/PathRawParserLight.java new file mode 100644 index 00000000..ecff9d5d --- /dev/null +++ b/src/main/java/org/scion/jpan/internal/PathRawParserLight.java @@ -0,0 +1,65 @@ +// 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.internal; + +import static org.scion.jpan.internal.ByteUtil.readBoolean; +import static org.scion.jpan.internal.ByteUtil.readInt; + +import java.nio.ByteBuffer; + +/** A lightweight stateless parser for raw paths. */ +public class PathRawParserLight { + private static final int PATH_META_LEN = 4; + private static final int PATH_INFO_LEN = 8; + private static final int HOP_FIELD_LEN = 12; + + private PathRawParserLight() {} + + public static int[] getSegments(ByteBuffer data) { + int i0 = data.getInt(); + int[] segLen = new int[3]; + segLen[0] = readInt(i0, 14, 6); + segLen[1] = readInt(i0, 20, 6); + segLen[2] = readInt(i0, 26, 6); + return segLen; + } + + public static boolean extractInfoFlagC(ByteBuffer data, int segID) { + int i0 = data.getInt(PATH_META_LEN + segID * PATH_INFO_LEN); + return readBoolean(i0, 7); + } + + public static int extractHopCount(int[] segLen) { + int nHops = segLen[0] + segLen[1] + segLen[2]; + return nHops - calcSegmentCount(segLen); + } + + public static int extractHopFieldIngress(ByteBuffer data, int segCount, int hopID) { + int hfOffset = PATH_META_LEN + segCount * PATH_INFO_LEN + hopID * HOP_FIELD_LEN; + return ByteUtil.toUnsigned(data.getShort(hfOffset + 2)); + } + + public static int extractHopFieldEgress(ByteBuffer data, int segCount, int hopID) { + int hfOffset = PATH_META_LEN + segCount * PATH_INFO_LEN + hopID * HOP_FIELD_LEN; + return ByteUtil.toUnsigned(data.getShort(hfOffset + 4)); + } + + public static int calcSegmentCount(int[] segLen) { + int nSegmentCount = 1; + nSegmentCount += segLen[1] > 0 ? 1 : 0; + nSegmentCount += segLen[2] > 0 ? 1 : 0; + return nSegmentCount; + } +} diff --git a/src/main/java/org/scion/jpan/internal/Segments.java b/src/main/java/org/scion/jpan/internal/Segments.java index f4cd7ad9..be7109f6 100644 --- a/src/main/java/org/scion/jpan/internal/Segments.java +++ b/src/main/java/org/scion/jpan/internal/Segments.java @@ -119,9 +119,9 @@ private static List getPaths( List directUp = filterForIsdAs(segmentsUp, dstIsdAs); if (!directUp.isEmpty()) { // DST is core or on-path - MultiMap paths = new MultiMap<>(); + PathDuplicationFilter paths = new PathDuplicationFilter(); combineSegment(paths, directUp, localAS, dstIsdAs); - return paths.values(); + return paths.getPaths(); } } } @@ -214,7 +214,7 @@ private static List combineSegments( int code = segmentsUp != null ? 4 : 0; code |= segmentsCore != null ? 2 : 0; code |= segmentsDown != null ? 1 : 0; - PathDeduplicator paths = new PathDeduplicator(); + PathDuplicationFilter paths = new PathDuplicationFilter(); switch (code) { case 7: combineThreeSegments( @@ -250,7 +250,7 @@ private static List combineSegments( } private static void combineSegment( - PathDeduplicator paths, + PathDuplicationFilter paths, List segments, LocalTopology localAS, long dstIsdAs) { @@ -269,7 +269,7 @@ private static void combineSegment( * @param localAS border router lookup resource */ private static void combineTwoSegments( - PathDeduplicator paths, + PathDuplicationFilter paths, List segments0, List segments1, long srcIsdAs, @@ -287,7 +287,7 @@ private static void combineTwoSegments( } private static void combineThreeSegments( - PathDeduplicator paths, + PathDuplicationFilter paths, List segmentsUp, List segmentsCore, List segmentsDown, @@ -321,7 +321,7 @@ private static void combineThreeSegments( } private static void buildPath( - PathDeduplicator paths, + PathDuplicationFilter paths, List segmentsUp, PathSegment segCore, List segmentsDown, @@ -335,10 +335,7 @@ private static void buildPath( } private static void buildPath( - PathDeduplicator paths, - LocalTopology localAS, - long dstIsdAs, - PathSegment... segments) { + PathDuplicationFilter paths, LocalTopology localAS, long dstIsdAs, PathSegment... segments) { Daemon.Path.Builder path = Daemon.Path.newBuilder(); ByteBuffer raw = ByteBuffer.allocate(1000); diff --git a/src/test/java/org/scion/jpan/demo/ScmpDemoDefault.java b/src/test/java/org/scion/jpan/demo/ScmpDemoDefault.java index c6b813e4..17c1b058 100644 --- a/src/test/java/org/scion/jpan/demo/ScmpDemoDefault.java +++ b/src/test/java/org/scion/jpan/demo/ScmpDemoDefault.java @@ -115,7 +115,7 @@ private static void runDemo(Path path) throws IOException { for (int i = 0; i < REPEAT; i++) { Scmp.EchoMessage msg = scmpChannel.sendEchoRequest(path, i, data); String millis = String.format("%.3f", msg.getNanoSeconds() / (double) 1_000_000); - String echoMsgStr = msg.getSizeReceived() + " bytes from "; + String echoMsgStr = " " + msg.getSizeReceived() + " bytes from "; InetAddress addr = msg.getPath().getRemoteAddress(); echoMsgStr += ScionUtil.toStringIA(path.getRemoteIsdAs()) + "," + addr.getHostAddress(); echoMsgStr += ": scmp_seq=" + msg.getSequenceNumber(); diff --git a/src/test/java/org/scion/jpan/internal/SegmentsDefault112Test.java b/src/test/java/org/scion/jpan/internal/SegmentsDefault112Test.java index f88998b1..6dca5dac 100644 --- a/src/test/java/org/scion/jpan/internal/SegmentsDefault112Test.java +++ b/src/test/java/org/scion/jpan/internal/SegmentsDefault112Test.java @@ -82,16 +82,7 @@ void removeDuplicatePaths() throws IOException { } } - // scionproto reports only 6 paths. The difference is that JPAN considers path different - // even if their hops are identical, as long their expiry date or SegmentId differs. - // For example, there are two paths that look like this: - // [494>103] - // However, they are not identical for JPAN: - // - segID=9858, timestamp=1723449803 - // - segID=9751, timestamp=1723449803 - - // assertEquals(6, paths.size()); - assertEquals(7, paths.size()); + assertEquals(6, paths.size()); } }