Make sure attributes are parsed in order of their increasing type

This improvement of API specification allows later attributes to see the
previous ones. This is needed for linkstate, where the Linkstate
Attribute needs to see the Multiprotocol attribute.

Change-Id: I3b05cda27ae5277387881d6f34bf6359cae75a99
Signed-off-by: Robert Varga <rovarga@cisco.com>

diff --git a/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/AttributeParser.java b/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/AttributeParser.java
index 71b7ac6f1271001d5626550ceecd4cedd314fc59..d0971085f905e38981de587cb93196e3967be54c 100644 (file)
--- a/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/AttributeParser.java
+++ b/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/AttributeParser.java
@@ -11,6 +11,14 @@ import org.opendaylight.protocol.bgp.parser.BGPDocumentedException;
 import org.opendaylight.protocol.bgp.parser.BGPParsingException;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev130919.update.PathAttributesBuilder;
 
+/**
+ * Common interface for attribute parser implementation.
+ */
 public interface AttributeParser {
-       void parseAttribute(final byte[] bytes, PathAttributesBuilder builder) throws BGPDocumentedException, BGPParsingException;
+       /**
+        * @param body encoded attribute body
+        * @param builder Path attributes builder. Guaranteed to contain all valid attributes whose type
+        *                is numerically lower than this attribute's type.
+        */
+       void parseAttribute(final byte[] body, PathAttributesBuilder builder) throws BGPDocumentedException, BGPParsingException;
 }

diff --git a/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/pojo/SimpleAttributeRegistry.java b/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/pojo/SimpleAttributeRegistry.java
index 4cf2af119f9d3fe750eaf86b9cb8384d9c95af7b..ee3d3aa5d87de114d674530e4f0d0880f1aa773e 100644 (file)
--- a/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/pojo/SimpleAttributeRegistry.java
+++ b/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/pojo/SimpleAttributeRegistry.java
@@ -7,6 +7,10 @@
  */
 package org.opendaylight.protocol.bgp.parser.spi.pojo;
 
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.TreeMap;
+
 import org.opendaylight.protocol.bgp.parser.BGPDocumentedException;
 import org.opendaylight.protocol.bgp.parser.BGPError;
 import org.opendaylight.protocol.bgp.parser.BGPParsingException;
@@ -20,11 +24,28 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.mess
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev130919.update.PathAttributesBuilder;
 import org.opendaylight.yangtools.yang.binding.DataContainer;
 import org.opendaylight.yangtools.yang.binding.DataObject;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Preconditions;
 import com.google.common.primitives.UnsignedBytes;
 
 final class SimpleAttributeRegistry implements AttributeRegistry {
+       private static final class RawAttribute {
+               private final AttributeParser parser;
+               private final byte[] body;
+
+               public RawAttribute(final AttributeParser parser, final byte[] body) {
+                       this.parser = Preconditions.checkNotNull(parser);
+                       this.body = Preconditions.checkNotNull(body);
+               }
+       }
+
+       private static final Logger LOG = LoggerFactory.getLogger(SimpleAttributeRegistry.class);
+       private static final int OPTIONAL_BIT = 0;
+       private static final int TRANSITIVE_BIT = 1;
+       private static final int PARTIAL_BIT = 2;
+       private static final int EXTENDED_LENGTH_BIT = 3;
        private final HandlerRegistry<DataContainer, AttributeParser, AttributeSerializer> handlers = new HandlerRegistry<>();
 
        AutoCloseable registerAttributeParser(final int attributeType, final AttributeParser parser) {
@@ -36,13 +57,12 @@ final class SimpleAttributeRegistry implements AttributeRegistry {
                return this.handlers.registerSerializer(paramClass, serializer);
        }
 
-       private int parseAttribute(final byte[] bytes, final int offset, final PathAttributesBuilder builder) throws BGPDocumentedException,
-                       BGPParsingException {
+       private int addAttribute(final byte[] bytes, final int offset, final Map<Integer, RawAttribute> attributes) throws BGPDocumentedException {
                final boolean[] flags = ByteArray.parseBits(bytes[offset]);
-               final int type = UnsignedBytes.toInt(bytes[offset + 1]);
+               final Integer type = UnsignedBytes.toInt(bytes[offset + 1]);
                final int hdrlen;
                final int len;
-               if (flags[3]) {
+               if (flags[EXTENDED_LENGTH_BIT]) {
                        len = UnsignedBytes.toInt(bytes[offset + 2]) * 256 + UnsignedBytes.toInt(bytes[offset + 3]);
                        hdrlen = 4;
                } else {
@@ -50,25 +70,48 @@ final class SimpleAttributeRegistry implements AttributeRegistry {
                        hdrlen = 3;
                }
 
-               final AttributeParser parser = this.handlers.getParser(type);
-               if (parser == null) {
-                       if (!flags[0]) {
-                               throw new BGPDocumentedException("Well known attribute not recognized.", BGPError.WELL_KNOWN_ATTR_NOT_RECOGNIZED);
+               if (!attributes.containsKey(type)) {
+                       final AttributeParser parser = this.handlers.getParser(type);
+                       if (parser == null) {
+                               if (!flags[OPTIONAL_BIT]) {
+                                       throw new BGPDocumentedException("Well known attribute not recognized.", BGPError.WELL_KNOWN_ATTR_NOT_RECOGNIZED);
+                               }
+                               if (flags[TRANSITIVE_BIT]) {
+                                       // FIXME: transitive attributes need to be preserved
+                                       LOG.warn("Losing unrecognized transitive attribute {}", type);
+                               } else {
+                                       LOG.debug("Ignoring unrecognized attribute type {}", type);
+                               }
+                       } else {
+                               attributes.put(type, new RawAttribute(parser, ByteArray.subByte(bytes, offset + hdrlen, len)));
                        }
                } else {
-                       parser.parseAttribute(ByteArray.subByte(bytes, offset + hdrlen, len), builder);
+                       LOG.debug("Ignoring duplicate attribute type {}", type);
                }
-
                return hdrlen + len;
        }
 
        @Override
        public PathAttributes parseAttributes(final byte[] bytes) throws BGPDocumentedException, BGPParsingException {
                int byteOffset = 0;
-               final PathAttributesBuilder builder = new PathAttributesBuilder();
+
+               final TreeMap<Integer, RawAttribute> attributes = new TreeMap<>();
                while (byteOffset < bytes.length) {
-                       byteOffset += parseAttribute(bytes, byteOffset, builder);
+                       byteOffset += addAttribute(bytes, byteOffset, attributes);
+               }
+
+               /*
+                * TreeMap guarantees that we will be invoking the parser in the order
+                * of increasing attribute type.
+                */
+               final PathAttributesBuilder builder = new PathAttributesBuilder();
+               for (final Entry<Integer, RawAttribute> e : attributes.entrySet()) {
+                       LOG.debug("Parsing attribute type {}", e.getKey());
+
+                       final RawAttribute a = e.getValue();
+                       a.parser.parseAttribute(a.body, builder);
                }
+
                return builder.build();
        }