Fix parsing issue of PcReport Objects 78/96978/6
authorOlivier Dugeon <olivier.dugeon@orange.com>
Wed, 21 Jul 2021 16:52:23 +0000 (18:52 +0200)
committerRobert Varga <nite@hq.sk>
Fri, 10 Sep 2021 09:33:18 +0000 (09:33 +0000)
StatefulPCReportMessageParser() class assumes that PCEP Objects in the
PcReport message are sent in a certain order. However, the order of
Object in the PcReport has changed between old draft version and final
RFC8231.

Indeed, as per RFC8231, the PcReport is composed of:
 - [SRP], <LSP>, <path> where:
   - <path> = <intended-path>
              [<actual-attribute-list><actual-path>]
              <intended-attribute-list>
   - <intended-path> = ERO
   - <actual-attribute-list> = BANDWIDTH, METRICS & <actual-path> = RRO
   - <intended-attribute-list> = LSPA, BANDWIDTH, METRICS, IRO

While in old draft version, <intended-attribute-list> was placed just
right after the <intended-path>.

Thus, the state machine should be flexible enough to accommodate to
PCCs that continue to use old draft and PCCs that are compliant to the
RFC8231.

JIRA: BGPCEP-974

Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
Change-Id: I97c4e339d8fe66d5e6a10fbb7055c91941351eda

pcep/ietf-stateful/src/main/java/org/opendaylight/protocol/pcep/ietf/stateful/StatefulPCReportMessageParser.java
pcep/ietf-stateful/src/test/java/org/opendaylight/protocol/pcep/ietf/PCEPValidatorTest.java
pcep/ietf-stateful/src/test/resources/PCRpt.4.bin [new file with mode: 0644]

index 5ddc8908d6456a8431a21b9d39fd8164924d6690..37832b7e33743fb47ea5601ff1bacc2dec22a061 100644 (file)
@@ -183,6 +183,26 @@ public class StatefulPCReportMessageParser extends AbstractMessageParser {
         }
     }
 
+    /**
+     * Determine the type of Object and insert it in the PathBuilder.
+     *
+     * <p>This method uses a state machine to check that Objects are seen only once and to speed up the browsing.
+     * However, the order of Object in the PcReport has changed between old draft version and final RFC8231.
+     * Indeed, as per RFC8231, the PcReport is composed of: ["SRP"], "LSP", "path"
+     * where "path" = "intended-path", ["actual-attribute-list", "actual-path"], "intended-attribute-list"
+     * and where "intended-path" = ERO, "actual-attribute-list" = BANDWIDTH, METRICS, "actual-path" = RRO
+     * and "intended-attribute-list" = LSPA, BANDWIDTH, METRICS, IRO
+     * In old draft version, "intended-attribute-list" was placed just right after the "intended-path".
+     * Thus, the state machine should be flexible enough to accommodate to PCCs that continue to use old draft and
+     * PCCs that are compliant to the RFC8231.</p>
+     *
+     * @param state         Current State of the state machine
+     * @param obj           Object to be identify and added to Path Builder
+     * @param builder       Path Builder to be fill with the Object
+     * @param pathMetrics   List of Metrics to be fill with Object when it is a Metrics
+     *
+     * @return              New State of the state machine
+     */
     private static State insertObject(final State state, final Object obj, final PathBuilder builder,
             final List<Metrics> pathMetrics) {
         switch (state) {
@@ -193,6 +213,7 @@ public class StatefulPCReportMessageParser extends AbstractMessageParser {
                 }
                 // fall through
             case LSPA_IN:
+                // Check presence for <intended-attribute-list> i.e LSPA, Bandwidth, Metrics, IRO ... as per old draft
                 if (obj instanceof Bandwidth) {
                     builder.setBandwidth((Bandwidth) obj);
                     return State.LSPA_IN;
@@ -223,6 +244,12 @@ public class StatefulPCReportMessageParser extends AbstractMessageParser {
                 }
                 // fall through
             case RRO_IN:
+                // Check presence for <intended-attribute-list> i.e LSPA, Bandwidth, Metrics, IRO ... as per RFC8231
+                if (obj instanceof Lspa) {
+                    builder.setLspa((Lspa) obj);
+                    return State.LSPA_IN;
+                }
+                // fall through
             case END:
                 return State.END;
             default:
index 2bffcfc4a8b8a9d2fa1504b54dd3ddf28204781d..585b7c5f43aa520a7d89b06b3335e20abadcb6d9 100644 (file)
@@ -440,6 +440,11 @@ public class PCEPValidatorTest {
         parser.serializeMessage(new PcrptBuilder().setPcrptMessage(builder.build()).build(), buf);
         assertArrayEquals(result.array(), buf.array());
 
+        result = Unpooled.wrappedBuffer(ByteArray.fileToBytes("src/test/resources/PCRpt.4.bin"));
+
+        assertEquals(new PcrptBuilder().setPcrptMessage(builder.build()).build(),
+                parser.parseMessage(result.slice(4, result.readableBytes() - 4), List.of()));
+
         result = Unpooled.wrappedBuffer(ByteArray.fileToBytes("src/test/resources/PCRpt.5.bin"));
 
         final List<Reports> reports3 = new ArrayList<>();
diff --git a/pcep/ietf-stateful/src/test/resources/PCRpt.4.bin b/pcep/ietf-stateful/src/test/resources/PCRpt.4.bin
new file mode 100644 (file)
index 0000000..9249c24
Binary files /dev/null and b/pcep/ietf-stateful/src/test/resources/PCRpt.4.bin differ