Introduce BGPTreatAsWithdrawException 51/78351/12
authorRobert Varga <robert.varga@pantheon.tech>
Mon, 3 Dec 2018 00:10:55 +0000 (01:10 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Wed, 5 Dec 2018 22:17:55 +0000 (23:17 +0100)
Attribute parsers need to be able to report that the attribute
failed to parse and the UPDATE message needs to enter
treat-as-withdraw error recovery.

BGPTreatAsWithdrawException holds this information and can be
reported from individual attribute parsers.

Change-Id: I459eb70692cde1035ca03110f414acb60d302abd
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
bgp/parser-api/src/main/java/org/opendaylight/protocol/bgp/parser/BGPTreatAsWithdrawException.java [new file with mode: 0644]
bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/AttributeParser.java
bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/RevisedErrorHandling.java
bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/pojo/SimpleAttributeRegistry.java
bgp/parser-spi/src/test/java/org/opendaylight/protocol/bgp/parser/spi/pojo/BgpTestActivator.java
bgp/parser-spi/src/test/java/org/opendaylight/protocol/bgp/parser/spi/pojo/SimpleRegistryTest.java

diff --git a/bgp/parser-api/src/main/java/org/opendaylight/protocol/bgp/parser/BGPTreatAsWithdrawException.java b/bgp/parser-api/src/main/java/org/opendaylight/protocol/bgp/parser/BGPTreatAsWithdrawException.java
new file mode 100644 (file)
index 0000000..b626346
--- /dev/null
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2018 AT&T Intellectual Property. All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.protocol.bgp.parser;
+
+import static java.util.Objects.requireNonNull;
+
+import org.eclipse.jdt.annotation.NonNull;
+import org.eclipse.jdt.annotation.Nullable;
+
+/**
+ * An exception thrown when the parsing of an attribute results in treat-as-withdraw being applied to the UPDATE
+ * message, as per the rules in RFC7606 and related documents.
+ *
+ * <p>
+ * This exception must not be thrown when Revised Error Handling procedures are not in effect.
+ *
+ * @author Robert Varga
+ */
+public final class BGPTreatAsWithdrawException extends Exception {
+    private static final long serialVersionUID = 1L;
+
+    private final @NonNull BGPError error;
+
+    public BGPTreatAsWithdrawException(final @NonNull BGPError error, final @NonNull String format,
+            final Object... args) {
+        this(error, null, format, args);
+    }
+
+    public BGPTreatAsWithdrawException(final @NonNull BGPError error, @Nullable final Exception cause,
+            final @NonNull String format, final Object... args) {
+        super(String.format(format, args), cause);
+        this.error = requireNonNull(error);
+    }
+
+    public @NonNull BGPDocumentedException toDocumentedException() {
+        return new BGPDocumentedException(getMessage(), error, this);
+    }
+}
index 94931b7b2f486d5d52c970a58aacb54de8785741..5ccd5a164e1129f0ed8bb855f7edd32076c3c937 100644 (file)
@@ -13,6 +13,7 @@ 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.protocol.bgp.parser.BGPTreatAsWithdrawException;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev180329.path.attributes.AttributesBuilder;
 
 /**
@@ -26,10 +27,33 @@ public interface AttributeParser {
      * @param builder Path attributes builder. Guaranteed to contain all valid attributes whose type is numerically
      *        lower than this attribute's type.
      * @param constraint Peer specific constraints, may be null
+     * @throws BGPDocumentedException when an irrecoverable error occurred which has a {@link BGPError} assigned
+     * @throws BGPParsingException when a general unspecified parsing error occurs.
      */
     void parseAttribute(@Nonnull ByteBuf buffer, @Nonnull AttributesBuilder builder,
             @Nullable PeerSpecificParserConstraint constraint) throws BGPDocumentedException, BGPParsingException;
 
+    /**
+     * Parses attribute from ByteBuf buffer with the specified {@link RevisedErrorHandling}. Default implementation
+     * ignores error handling and defers to
+     * {@link #parseAttribute(ByteBuf, AttributesBuilder, PeerSpecificParserConstraint)}.
+     *
+     * @param buffer Encoded attribute body in ByteBuf.
+     * @param builder Path attributes builder. Guaranteed to contain all valid attributes whose type is numerically
+     *        lower than this attribute's type.
+     * @param errorHandling RFC7606 error handling type
+     * @param constraint Peer specific constraints, may be null
+     * @throws BGPDocumentedException when an irrecoverable error occurred which has a {@link BGPError} assigned
+     * @throws BGPParsingException when a general unspecified parsing error occurs.
+     * @throws BGPTreatAsWithdrawException when parsing according to revised error handling indicates the
+     *                                              message should be treated as withdraw.
+     */
+    default void parseAttribute(@Nonnull final ByteBuf buffer, @Nonnull final AttributesBuilder builder,
+            @Nonnull final RevisedErrorHandling errorHandling, @Nullable final PeerSpecificParserConstraint constraint)
+                    throws BGPDocumentedException, BGPParsingException, BGPTreatAsWithdrawException {
+        parseAttribute(buffer, builder, constraint);
+    }
+
     /**
      * 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
index ebb614813ebdbb1efe7bb051b55f85ca594cb4dc..f58f4a9903784d209d9ec9ca071fd4abd78b2e9b 100644 (file)
@@ -9,6 +9,9 @@ package org.opendaylight.protocol.bgp.parser.spi;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
+import org.opendaylight.protocol.bgp.parser.BGPDocumentedException;
+import org.opendaylight.protocol.bgp.parser.BGPError;
+import org.opendaylight.protocol.bgp.parser.BGPTreatAsWithdrawException;
 
 /**
  * Enumeration of possible treatments an UPDATE message and attributes can get based on the configuration of a peer.
@@ -20,7 +23,13 @@ public enum RevisedErrorHandling {
     /**
      * Do not use RFC7606 Revised Error Handling.
      */
-    NONE,
+    NONE {
+        @Override
+        public BGPDocumentedException reportError(final BGPError error, final @Nullable Exception cause,
+                final String format, final Object... args) throws BGPDocumentedException {
+            throw new BGPDocumentedException(String.format(format, args), error, cause);
+        }
+    },
     /**
      * Use RFC7606 Revised Error Handling, the peer is an internal neighbor.
      */
@@ -40,4 +49,37 @@ public enum RevisedErrorHandling {
         return constraint == null ? NONE : constraint.getPeerConstraint(RevisedErrorHandlingSupport.class)
                 .map(support -> support.isExternalPeer() ? EXTERNAL : INTERNAL).orElse(NONE);
     }
+
+    /**
+     * Report a failure to parse an attribute resulting either in treat-as-withdraw if RFC7606 is in effect, or
+     * connection teardown if it is not.
+     *
+     * @param error {@link BGPError} to report in case of a session teardown
+     * @param cause Parsing failure cause
+     * @param format Message format string
+     * @param args Message format arguments
+     * @return This method does not return
+     * @throws BGPTreatAsWithdrawException if Revised Error Handling is in effect
+     * @throws BGPDocumentedException if Revised Error Handling is in not effect
+     */
+    public BGPDocumentedException reportError(final BGPError error, final @Nullable Exception cause,
+            final String format, final Object... args) throws BGPDocumentedException, BGPTreatAsWithdrawException {
+        throw new BGPTreatAsWithdrawException(error, cause, format, args);
+    }
+
+    /**
+     * Report a failure to parse an attribute resulting either in treat-as-withdraw if RFC7606 is in effect, or
+     * connection teardown if it is not.
+     *
+     * @param error {@link BGPError} to report in case of a session teardown
+     * @param format Message format string
+     * @param args Message format arguments
+     * @return This method does not return
+     * @throws BGPTreatAsWithdrawException if Revised Error Handling is in effect
+     * @throws BGPDocumentedException if Revised Error Handling is in not effect
+     */
+    public BGPDocumentedException reportError(final BGPError error, final String format, final Object... args)
+            throws BGPDocumentedException, BGPTreatAsWithdrawException {
+        throw reportError(error, null, format, args);
+    }
 }
index d9c9b718cce15095131ee5efb139aced96951f3a..8d9dcefa28ab4eeb4294c82fa1eee03f99c3146a 100644 (file)
@@ -21,6 +21,7 @@ import java.util.concurrent.atomic.AtomicReference;
 import org.opendaylight.protocol.bgp.parser.BGPDocumentedException;
 import org.opendaylight.protocol.bgp.parser.BGPError;
 import org.opendaylight.protocol.bgp.parser.BGPParsingException;
+import org.opendaylight.protocol.bgp.parser.BGPTreatAsWithdrawException;
 import org.opendaylight.protocol.bgp.parser.spi.AttributeParser;
 import org.opendaylight.protocol.bgp.parser.spi.AttributeRegistry;
 import org.opendaylight.protocol.bgp.parser.spi.AttributeSerializer;
@@ -138,14 +139,33 @@ final class SimpleAttributeRegistry implements AttributeRegistry {
          * TreeMap guarantees that we will be invoking the parser in the order
          * of increasing attribute type.
          */
+        // We may have multiple attribute errors, each specifying a withdraw. We need to finish parsing the message
+        // all attributes before we can decide whether we can discard attributes, or whether we need to terminate
+        // the session.
         final AttributesBuilder builder = new AttributesBuilder();
-        for (final Entry<Integer, RawAttribute> e : attributes.entrySet()) {
-            LOG.debug("Parsing attribute type {}", e.getKey());
-
-            final RawAttribute a = e.getValue();
-            a.parser.parseAttribute(a.buffer, builder, constraint);
+        BGPTreatAsWithdrawException withdrawCause = null;
+        for (final Entry<Integer, RawAttribute> entry : attributes.entrySet()) {
+            LOG.debug("Parsing attribute type {}", entry.getKey());
+
+            final RawAttribute a = entry.getValue();
+            try {
+                a.parser.parseAttribute(a.buffer, builder, errorHandling, constraint);
+            } catch (BGPTreatAsWithdrawException e) {
+                LOG.info("Attribute {} indicated treat-as-withdraw", entry.getKey(), e);
+                if (withdrawCause == null) {
+                    withdrawCause = e;
+                } else {
+                    withdrawCause.addSuppressed(e);
+                }
+            }
         }
         builder.setUnrecognizedAttributes(this.unrecognizedAttributes);
+
+        // FIXME: BGPCEP-359 report withdrawCause upstream, so it can be handled properly
+        if (withdrawCause != null) {
+            throw withdrawCause.toDocumentedException();
+        }
+
         return builder.build();
     }
 
index c00ae9e1274d2742c7c6f128feb37a4574ae807a..fe3ac1e049bbd6b7793f3d3bd320eb4641819774 100644 (file)
@@ -21,6 +21,7 @@ import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
 import org.opendaylight.protocol.bgp.parser.BGPDocumentedException;
 import org.opendaylight.protocol.bgp.parser.BGPParsingException;
+import org.opendaylight.protocol.bgp.parser.BGPTreatAsWithdrawException;
 import org.opendaylight.protocol.bgp.parser.spi.AbstractBGPExtensionProviderActivator;
 import org.opendaylight.protocol.bgp.parser.spi.AttributeParser;
 import org.opendaylight.protocol.bgp.parser.spi.AttributeSerializer;
@@ -37,6 +38,7 @@ import org.opendaylight.protocol.bgp.parser.spi.NlriSerializer;
 import org.opendaylight.protocol.bgp.parser.spi.ParameterParser;
 import org.opendaylight.protocol.bgp.parser.spi.ParameterSerializer;
 import org.opendaylight.protocol.bgp.parser.spi.PeerSpecificParserConstraint;
+import org.opendaylight.protocol.bgp.parser.spi.RevisedErrorHandling;
 import org.opendaylight.protocol.bgp.parser.spi.extended.community.ExtendedCommunityParser;
 import org.opendaylight.protocol.bgp.parser.spi.extended.community.ExtendedCommunitySerializer;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Ipv4Address;
@@ -154,7 +156,7 @@ public class BgpTestActivator extends AbstractBGPExtensionProviderActivator {
         MockitoAnnotations.initMocks(this);
         try {
             Mockito.doNothing().when(this.attrParser).parseAttribute(any(ByteBuf.class), any(AttributesBuilder.class),
-                any(PeerSpecificParserConstraint.class));
+                any(RevisedErrorHandling.class), any(PeerSpecificParserConstraint.class));
             doReturn(EMPTY).when(this.attrParser).toString();
             Mockito.doNothing().when(this.attrSerializer).serializeAttribute(any(Attributes.class), any(ByteBuf.class));
             doReturn(EMPTY).when(this.attrSerializer).toString();
@@ -185,7 +187,7 @@ public class BgpTestActivator extends AbstractBGPExtensionProviderActivator {
             Mockito.doNothing().when(this.nlriParser).parseNlri(any(ByteBuf.class), any(MpReachNlriBuilder.class), any());
             doReturn(EMPTY).when(this.nlriParser).toString();
 
-        } catch (BGPDocumentedException | BGPParsingException e) {
+        } catch (BGPDocumentedException | BGPParsingException | BGPTreatAsWithdrawException e) {
             Assert.fail();
         }
     }
index aad908c8fda542bc39a3aad492438d61a5665b7f..7289e6717a9bb03b5696f68c37384b19a193ba34 100644 (file)
@@ -25,6 +25,7 @@ import org.junit.Test;
 import org.mockito.Mockito;
 import org.opendaylight.protocol.bgp.parser.BGPDocumentedException;
 import org.opendaylight.protocol.bgp.parser.BGPParsingException;
+import org.opendaylight.protocol.bgp.parser.BGPTreatAsWithdrawException;
 import org.opendaylight.protocol.bgp.parser.spi.AddressFamilyRegistry;
 import org.opendaylight.protocol.bgp.parser.spi.AttributeRegistry;
 import org.opendaylight.protocol.bgp.parser.spi.BGPExtensionProviderContext;
@@ -35,6 +36,7 @@ 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.PeerSpecificParserConstraint;
+import org.opendaylight.protocol.bgp.parser.spi.RevisedErrorHandling;
 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;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev180329.open.message.BgpParameters;
@@ -80,7 +82,7 @@ public class SimpleRegistryTest {
     }
 
     @Test
-    public void testSimpleAttribute() throws BGPDocumentedException, BGPParsingException {
+    public void testSimpleAttribute() throws BGPDocumentedException, BGPParsingException, BGPTreatAsWithdrawException {
         final AttributeRegistry attrReg = this.ctx.getAttributeRegistry();
         final byte[] attributeBytes = {
             0x00, 0x00, 0x00
@@ -89,7 +91,7 @@ public class SimpleRegistryTest {
         attrReg.serializeAttribute(mock(Attributes.class), byteAggregator);
         attrReg.parseAttributes(Unpooled.wrappedBuffer(attributeBytes), CONSTRAINT);
         verify(this.activator.attrParser, times(1)).parseAttribute(any(ByteBuf.class), any(AttributesBuilder.class),
-            any(PeerSpecificParserConstraint.class));
+            any(RevisedErrorHandling.class), any(PeerSpecificParserConstraint.class));
         verify(this.activator.attrSerializer, times(1)).serializeAttribute(any(Attributes.class), any(ByteBuf.class));
     }