Distinguish between ignored and erroring attributes

MP_REACH/MP_UNREACH attributes need to cause a BGPDocumentedException
to be thrown when in RFC7606 mode. This adds that capability and
makes parser report it.

Change-Id: Iabbd16e64332ad9ffd823e942262b7ba324ad328
JIRA: MDSAL-359
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/bgp/parser-impl/src/main/java/org/opendaylight/protocol/bgp/parser/impl/message/update/ReachAttributeParser.java b/bgp/parser-impl/src/main/java/org/opendaylight/protocol/bgp/parser/impl/message/update/ReachAttributeParser.java
index b4258d87d6f77a86771bce03e38ab1975b32002e..88d8106a320e83d5d27afe9ac56b4b347f4eb69c 100644 (file)
--- a/bgp/parser-impl/src/main/java/org/opendaylight/protocol/bgp/parser/impl/message/update/ReachAttributeParser.java
+++ b/bgp/parser-impl/src/main/java/org/opendaylight/protocol/bgp/parser/impl/message/update/ReachAttributeParser.java
@@ -9,7 +9,11 @@ package org.opendaylight.protocol.bgp.parser.impl.message.update;
 
 import org.opendaylight.protocol.bgp.parser.spi.AttributeParser;
 import org.opendaylight.protocol.bgp.parser.spi.AttributeSerializer;
+import org.opendaylight.protocol.bgp.parser.spi.RevisedErrorHandling;
 
 public abstract class ReachAttributeParser implements AttributeParser, AttributeSerializer {
-
+    @Override
+    public boolean ignoreDuplicates(final RevisedErrorHandling errorHandling) {
+        return errorHandling == RevisedErrorHandling.NONE;
+    }
 }

diff --git a/bgp/parser-impl/src/test/java/org/opendaylight/protocol/bgp/parser/impl/BGPParserTest.java b/bgp/parser-impl/src/test/java/org/opendaylight/protocol/bgp/parser/impl/BGPParserTest.java
index 13e42257a288657b1ab7ce74274c35c4db5cd90b..44ec82e5120e56e5f26685919f054e295fe553e3 100644 (file)
--- a/bgp/parser-impl/src/test/java/org/opendaylight/protocol/bgp/parser/impl/BGPParserTest.java
+++ b/bgp/parser-impl/src/test/java/org/opendaylight/protocol/bgp/parser/impl/BGPParserTest.java
@@ -120,7 +120,8 @@ public class BGPParserTest {
             multiPathHexFile));
         constraint = mock(PeerSpecificParserConstraint.class);
         mpSupport = mock(MultiPathSupport.class);
-        Mockito.doReturn(Optional.of(mpSupport)).when(constraint).getPeerConstraint(Mockito.any());
+        Mockito.doReturn(Optional.empty()).when(constraint).getPeerConstraint(Mockito.any());
+        Mockito.doReturn(Optional.of(mpSupport)).when(constraint).getPeerConstraint(MultiPathSupport.class);
         Mockito.doReturn(true).when(mpSupport).isTableTypeSupported(Mockito.any());
     }
 

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 22eefeb2c12178962c6eb7d8504c7a6ca0ee8d65..94931b7b2f486d5d52c970a58aacb54de8785741 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,7 @@ import io.netty.buffer.ByteBuf;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import org.opendaylight.protocol.bgp.parser.BGPDocumentedException;
+import org.opendaylight.protocol.bgp.parser.BGPError;
 import org.opendaylight.protocol.bgp.parser.BGPParsingException;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev180329.path.attributes.AttributesBuilder;
 
@@ -28,4 +29,16 @@ public interface AttributeParser {
      */
     void parseAttribute(@Nonnull ByteBuf buffer, @Nonnull AttributesBuilder builder,
             @Nullable PeerSpecificParserConstraint constraint) throws BGPDocumentedException, BGPParsingException;
+
+    /**
+     * Determine whether a duplicate attribute should be ignored or {@link BGPError#MALFORMED_ATTR_LIST} should be
+     * raised. This is useful for MP_REACH/MP_UNREACH attributes, which need to emit a Notification under RFC7606
+     * rules.
+     *
+     * @param errorHandling Revised error handling type
+     * @return True if the duplicate attribute should be ignored, false if a BGPError should be raised.
+     */
+    default boolean ignoreDuplicates(final @Nonnull RevisedErrorHandling errorHandling) {
+        return true;
+    }
 }

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 ef9a782d9af6f594a182c2da1774b36472bf2d4c..d9c9b718cce15095131ee5efb139aced96951f3a 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
@@ -25,6 +25,7 @@ import org.opendaylight.protocol.bgp.parser.spi.AttributeParser;
 import org.opendaylight.protocol.bgp.parser.spi.AttributeRegistry;
 import org.opendaylight.protocol.bgp.parser.spi.AttributeSerializer;
 import org.opendaylight.protocol.bgp.parser.spi.PeerSpecificParserConstraint;
+import org.opendaylight.protocol.bgp.parser.spi.RevisedErrorHandling;
 import org.opendaylight.protocol.concepts.AbstractRegistration;
 import org.opendaylight.protocol.concepts.HandlerRegistry;
 import org.opendaylight.protocol.util.BitArray;
@@ -57,7 +58,9 @@ final class SimpleAttributeRegistry implements AttributeRegistry {
     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<>();
+
+    private final HandlerRegistry<DataContainer, AttributeParser, AttributeSerializer> handlers =
+            new HandlerRegistry<>();
     private final Map<AbstractRegistration, AttributeSerializer> serializers = new LinkedHashMap<>();
     private final AtomicReference<Iterable<AttributeSerializer>> roSerializers =
         new AtomicReference<>(this.serializers.values());
@@ -69,7 +72,8 @@ final class SimpleAttributeRegistry implements AttributeRegistry {
         return this.handlers.registerParser(attributeType, parser);
     }
 
-    synchronized AutoCloseable registerAttributeSerializer(final Class<? extends DataObject> paramClass, final AttributeSerializer serializer) {
+    synchronized AutoCloseable registerAttributeSerializer(final Class<? extends DataObject> paramClass,
+            final AttributeSerializer serializer) {
         final AbstractRegistration reg = this.handlers.registerSerializer(paramClass, serializer);
 
         this.serializers.put(reg, serializer);
@@ -85,27 +89,32 @@ final class SimpleAttributeRegistry implements AttributeRegistry {
         };
     }
 
-    private void addAttribute(final ByteBuf buffer, final Map<Integer, RawAttribute> attributes)
-            throws BGPDocumentedException {
+    private void addAttribute(final ByteBuf buffer, final RevisedErrorHandling errorHandling,
+            final Map<Integer, RawAttribute> attributes) throws BGPDocumentedException {
         final BitArray flags = BitArray.valueOf(buffer.readByte());
         final int type = buffer.readUnsignedByte();
         final int len = flags.get(EXTENDED_LENGTH_BIT) ? buffer.readUnsignedShort() : buffer.readUnsignedByte();
-        if (!attributes.containsKey(type)) {
-            final AttributeParser parser = this.handlers.getParser(type);
-            if (parser == null) {
-                processUnrecognized(flags, type, buffer, len);
-            } else {
-                attributes.put(type, new RawAttribute(parser, buffer.readSlice(len)));
+        final AttributeParser parser = this.handlers.getParser(type);
+        if (attributes.containsKey(type)) {
+            if (parser != null && !parser.ignoreDuplicates(errorHandling)) {
+                throw new BGPDocumentedException("Duplicate attribute " + type, BGPError.MALFORMED_ATTR_LIST);
             }
-        } else {
             LOG.debug("Ignoring duplicate attribute type {}", type);
+            return;
+        }
+
+        if (parser == null) {
+            processUnrecognized(flags, type, buffer, len);
+        } else {
+            attributes.put(type, new RawAttribute(parser, buffer.readSlice(len)));
         }
     }
 
     private void processUnrecognized(final BitArray flags, final int type, final ByteBuf buffer, final int len)
             throws BGPDocumentedException {
         if (!flags.get(OPTIONAL_BIT)) {
-            throw new BGPDocumentedException("Well known attribute not recognized.", BGPError.WELL_KNOWN_ATTR_NOT_RECOGNIZED);
+            throw new BGPDocumentedException("Well known attribute not recognized.",
+                BGPError.WELL_KNOWN_ATTR_NOT_RECOGNIZED);
         }
         final UnrecognizedAttributes unrecognizedAttribute = new UnrecognizedAttributesBuilder()
             .withKey(new UnrecognizedAttributesKey((short) type))
@@ -120,9 +129,10 @@ final class SimpleAttributeRegistry implements AttributeRegistry {
     @Override
     public Attributes parseAttributes(final ByteBuf buffer, final PeerSpecificParserConstraint constraint)
             throws BGPDocumentedException, BGPParsingException {
+        final RevisedErrorHandling errorHandling = RevisedErrorHandling.from(constraint);
         final Map<Integer, RawAttribute> attributes = new TreeMap<>();
         while (buffer.isReadable()) {
-            addAttribute(buffer, attributes);
+            addAttribute(buffer, errorHandling, attributes);
         }
         /*
          * TreeMap guarantees that we will be invoking the parser in the order

diff --git a/bgp/parser-spi/src/test/java/org/opendaylight/protocol/bgp/parser/spi/pojo/SimpleRegistryTest.java b/bgp/parser-spi/src/test/java/org/opendaylight/protocol/bgp/parser/spi/pojo/SimpleRegistryTest.java
index 006f5bb0256972679316f8fecedca2b4379fb4da..aad908c8fda542bc39a3aad492438d61a5665b7f 100644 (file)
--- a/bgp/parser-spi/src/test/java/org/opendaylight/protocol/bgp/parser/spi/pojo/SimpleRegistryTest.java
+++ b/bgp/parser-spi/src/test/java/org/opendaylight/protocol/bgp/parser/spi/pojo/SimpleRegistryTest.java
@@ -19,7 +19,6 @@ import static org.mockito.Mockito.verify;
 
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
-import java.util.Optional;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -35,7 +34,6 @@ import org.opendaylight.protocol.bgp.parser.spi.MessageRegistry;
 import org.opendaylight.protocol.bgp.parser.spi.MultiPathSupport;
 import org.opendaylight.protocol.bgp.parser.spi.NlriRegistry;
 import org.opendaylight.protocol.bgp.parser.spi.ParameterRegistry;
-import org.opendaylight.protocol.bgp.parser.spi.PeerConstraint;
 import org.opendaylight.protocol.bgp.parser.spi.PeerSpecificParserConstraint;
 import org.opendaylight.protocol.bgp.parser.spi.SubsequentAddressFamilyRegistry;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Ipv4Address;
@@ -58,12 +56,13 @@ public class SimpleRegistryTest {
 
     private static final MultiPathSupport ADD_PATH_SUPPORT = tableType -> true;
 
-    private static final PeerSpecificParserConstraint CONSTRAINT = new PeerSpecificParserConstraint() {
-        @Override
-        public <T extends PeerConstraint> Optional<T> getPeerConstraint(final Class<T> peerConstraintType) {
-            return (Optional<T>) Optional.of(ADD_PATH_SUPPORT);
-        }
-    };
+    private static final PeerSpecificParserConstraint CONSTRAINT;
+
+    static {
+        PeerSpecificParserConstraintImpl c = new PeerSpecificParserConstraintImpl();
+        c.addPeerConstraint(MultiPathSupport.class, ADD_PATH_SUPPORT);
+        CONSTRAINT = c;
+    }
 
     protected BGPExtensionProviderContext ctx;
     private BgpTestActivator activator;