Fix parsing issue of PcReport Objects 35/97435/1
authorOlivier Dugeon <olivier.dugeon@orange.com>
Wed, 21 Jul 2021 16:52:23 +0000 (18:52 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Fri, 10 Sep 2021 09:36:48 +0000 (11:36 +0200)
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
(cherry picked from commit ea833a352533c0a0643dc9ee79a87477e2c33be4)

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 ec7c93aba4041ec501470d19e6d667699e663bc2..c7454adca1a5f6971bc8f345505dbafda2898d1a 100644 (file)
@@ -185,6 +185,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) {
@@ -195,6 +215,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;
@@ -225,6 +246,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 79a740b63e29df7a371b58fcfdfe667565fc9c3c..fade330a4faa3e6eebf0524ede220f03f44b091f 100644 (file)
@@ -457,6 +457,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