Allow AttributeRegistry to signal treat-as-withdraw 52/78352/12
authorRobert Varga <robert.varga@pantheon.tech>
Wed, 5 Dec 2018 22:18:34 +0000 (23:18 +0100)
committerRobert Varga <nite@hq.sk>
Thu, 6 Dec 2018 10:21:33 +0000 (10:21 +0000)
When any attribute indicates we should be treating the resulting
message as withdraw, we need to report this indication upwards,
so that the it can be evaluated once the complete contents of
the message are known.

This can result in either a conversion of reachability to
unreachability, or, if we fail to parse the NLRI-bearing fields,
in a session teardown.

Change-Id: I8c3b7b9d9f9523b42a328820cc6b7c9168976aeb
JIRA: BGPCEP-359
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
bgp/extensions/mvpn/src/test/java/org/opendaylight/protocol/bgp/mvpn/impl/attributes/PEDistinguisherLabelsAttributeHandlerTest.java
bgp/extensions/mvpn/src/test/java/org/opendaylight/protocol/bgp/mvpn/impl/attributes/PMSITunnelAttributeHandlerTest.java
bgp/parser-impl/src/main/java/org/opendaylight/protocol/bgp/parser/impl/message/BGPUpdateMessageParser.java
bgp/parser-impl/src/test/java/org/opendaylight/protocol/bgp/parser/impl/PathAttributeParserTest.java
bgp/parser-impl/src/test/java/org/opendaylight/protocol/bgp/parser/impl/message/update/CommunitiesAttributeParserTest.java
bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/AttributeRegistry.java
bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/ParsedAttributes.java [new file with mode: 0644]
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/UnrecognizedAttributesTest.java

index 8831a686cd9053d1fe07c293b8ac8aaafb7960a2..da5309e7f50b96f777de7bae4fda95bcdd01cf5f 100644 (file)
@@ -68,7 +68,7 @@ public final class PEDistinguisherLabelsAttributeHandlerTest {
         this.handler.serializeAttribute(expected, actual);
         assertArrayEquals(PE_DISTINGUISHER_LABELS, ByteArray.readAllBytes(actual));
         final Attributes actualAttr = this.handler.parseAttributes(
-                Unpooled.wrappedBuffer(PE_DISTINGUISHER_LABELS), null);
+                Unpooled.wrappedBuffer(PE_DISTINGUISHER_LABELS), null).getAttributes();
         assertEquals(expected, actualAttr);
     }
 
index 482b4a4b74e5f11d1303c472f7e6ba56ac742789..20208954c2d3de2432877cbb2bc127283d769b78 100644 (file)
@@ -72,7 +72,7 @@ public final class PMSITunnelAttributeHandlerTest {
         assertArrayEquals(BIDIR_PIM_EXPECTED, ByteArray.readAllBytes(actual));
         final Attributes expected = buildBidirPimTreeAttribute();
         final Attributes actualAttr = this.handler.parseAttributes(
-                Unpooled.wrappedBuffer(BIDIR_PIM_EXPECTED), null);
+                Unpooled.wrappedBuffer(BIDIR_PIM_EXPECTED), null).getAttributes();
         assertEquals(expected, actualAttr);
     }
 
@@ -84,7 +84,7 @@ public final class PMSITunnelAttributeHandlerTest {
         assertArrayEquals(PIM_SM_TREE_EXPECTED, ByteArray.readAllBytes(actual));
         final Attributes expected = buildPimSMTreeAttribute();
         final Attributes actualAttr = this.handler.parseAttributes(
-                Unpooled.wrappedBuffer(PIM_SM_TREE_EXPECTED), null);
+                Unpooled.wrappedBuffer(PIM_SM_TREE_EXPECTED), null).getAttributes();
         assertEquals(expected, actualAttr);
     }
 
@@ -100,7 +100,7 @@ public final class PMSITunnelAttributeHandlerTest {
     public void parsePimSSMTree() throws Exception {
         final Attributes expected = buildPimSSMTreeAttribute();
         final Attributes actual = this.handler.parseAttributes(
-                Unpooled.wrappedBuffer(PIM_SSM_TREE_EXPECTED), null);
+                Unpooled.wrappedBuffer(PIM_SSM_TREE_EXPECTED), null).getAttributes();
         assertEquals(expected, actual);
     }
 
@@ -112,7 +112,7 @@ public final class PMSITunnelAttributeHandlerTest {
         assertArrayEquals(M_LDP_P2MP_LSP_EXPECTED_IPV4, ByteArray.readAllBytes(actualIpv4));
 
         final Attributes actualIpv4Attribute = this.handler.parseAttributes(
-                Unpooled.wrappedBuffer(M_LDP_P2MP_LSP_EXPECTED_IPV4_2), null);
+                Unpooled.wrappedBuffer(M_LDP_P2MP_LSP_EXPECTED_IPV4_2), null).getAttributes();
         assertEquals(expectedIpv4Att, actualIpv4Attribute);
 
         final Attributes expectedIpv6Att = buildMldpP2mpLspIpv6Attribute();
@@ -121,7 +121,7 @@ public final class PMSITunnelAttributeHandlerTest {
         assertArrayEquals(M_LDP_P2MP_LSP_EXPECTED_IPV6, ByteArray.readAllBytes(actualIpv6));
 
         final Attributes actualIpv6Attribute = this.handler.parseAttributes(
-                Unpooled.wrappedBuffer(M_LDP_P2MP_LSP_EXPECTED_IPV6), null);
+                Unpooled.wrappedBuffer(M_LDP_P2MP_LSP_EXPECTED_IPV6), null).getAttributes();
         assertEquals(expectedIpv6Att, actualIpv6Attribute);
 
         final ByteBuf actualL2vpn = Unpooled.buffer();
@@ -130,13 +130,13 @@ public final class PMSITunnelAttributeHandlerTest {
 
 
         final Attributes actualWrongFamily = this.handler.parseAttributes(
-                Unpooled.wrappedBuffer(M_LDP_P2MP_LSP_EXPECTED_WRONG_FAMILY), null);
+                Unpooled.wrappedBuffer(M_LDP_P2MP_LSP_EXPECTED_WRONG_FAMILY), null).getAttributes();
         assertEquals(buildWOTunnelInfAttribute(), actualWrongFamily);
 
         final Attributes expectedL2vpnAtt = buildMldpp2MPLspL2vpnAttribute();
 
         final Attributes actualL2vpnAttribute = this.handler.parseAttributes(
-                Unpooled.wrappedBuffer(M_LDP_P2MP_LSP_EXPECTED_L2VPN), null);
+                Unpooled.wrappedBuffer(M_LDP_P2MP_LSP_EXPECTED_L2VPN), null).getAttributes();
         assertEquals(expectedL2vpnAtt, actualL2vpnAttribute);
 
         final ByteBuf actualL2vp = Unpooled.buffer();
@@ -155,7 +155,7 @@ public final class PMSITunnelAttributeHandlerTest {
         this.handler.serializeAttribute(expected, actual);
         assertArrayEquals(RSVP_TE_P2MP_LSP_LSP_EXPECTED, ByteArray.readAllBytes(actual));
         final Attributes actualAttr = this.handler.parseAttributes(
-                Unpooled.wrappedBuffer(RSVP_TE_P2MP_LSP_LSP_EXPECTED), null);
+                Unpooled.wrappedBuffer(RSVP_TE_P2MP_LSP_LSP_EXPECTED), null).getAttributes();
         assertEquals(expected, actualAttr);
     }
 
@@ -166,7 +166,7 @@ public final class PMSITunnelAttributeHandlerTest {
         this.handler.serializeAttribute(expected, actual);
         assertArrayEquals(INGRESS_REPLICATION_EXPECTED, ByteArray.readAllBytes(actual));
         final Attributes actualAttr = this.handler.parseAttributes(
-                Unpooled.wrappedBuffer(INGRESS_REPLICATION_EXPECTED), null);
+                Unpooled.wrappedBuffer(INGRESS_REPLICATION_EXPECTED), null).getAttributes();
         assertEquals(expected, actualAttr);
     }
 
@@ -178,11 +178,11 @@ public final class PMSITunnelAttributeHandlerTest {
         assertArrayEquals(M_LDP_MP_2_MP_LSP_EXPECTED, ByteArray.readAllBytes(actual));
 
         final Attributes actualAttr = this.handler.parseAttributes(
-                Unpooled.wrappedBuffer(M_LDP_MP_2_MP_LSP_EXPECTED), null);
+                Unpooled.wrappedBuffer(M_LDP_MP_2_MP_LSP_EXPECTED), null).getAttributes();
         assertEquals(expected, actualAttr);
 
         final Attributes actualWrong = this.handler.parseAttributes(
-                Unpooled.wrappedBuffer(M_LDP_MP_2_MP_LSP_WRONG), null);
+                Unpooled.wrappedBuffer(M_LDP_MP_2_MP_LSP_WRONG), null).getAttributes();
         assertEquals(buildWOTunnelInfAttribute(), actualWrong);
 
         final Attributes wrongAttribute = buildMldpMP2mpLspWrongAttribute();
@@ -200,7 +200,7 @@ public final class PMSITunnelAttributeHandlerTest {
         assertArrayEquals(NO_TUNNEL_INFORMATION_PRESENT_EXPECTED, ByteArray.readAllBytes(actual));
         final Attributes expected = buildWOTunnelInfAttribute();
         final Attributes actualAttr = this.handler.parseAttributes(
-                Unpooled.wrappedBuffer(NO_TUNNEL_INFORMATION_PRESENT_EXPECTED), null);
+                Unpooled.wrappedBuffer(NO_TUNNEL_INFORMATION_PRESENT_EXPECTED), null).getAttributes();
         assertEquals(expected, actualAttr);
     }
 
index 42daa9f173a945609c117650aef58a07af078145..275489cdd74d80718af481c5e1804b3a2e6cc54a 100755 (executable)
@@ -14,9 +14,11 @@ import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
 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.BgpTableTypeImpl;
 import org.opendaylight.protocol.bgp.parser.impl.message.update.AsPathAttributeParser;
 import org.opendaylight.protocol.bgp.parser.impl.message.update.NextHopAttributeParser;
@@ -26,6 +28,7 @@ import org.opendaylight.protocol.bgp.parser.spi.MessageParser;
 import org.opendaylight.protocol.bgp.parser.spi.MessageSerializer;
 import org.opendaylight.protocol.bgp.parser.spi.MessageUtil;
 import org.opendaylight.protocol.bgp.parser.spi.MultiPathSupportUtil;
+import org.opendaylight.protocol.bgp.parser.spi.ParsedAttributes;
 import org.opendaylight.protocol.bgp.parser.spi.PathIdUtil;
 import org.opendaylight.protocol.bgp.parser.spi.PeerSpecificParserConstraint;
 import org.opendaylight.protocol.util.ByteBufWriteUtil;
@@ -97,7 +100,7 @@ public final class BGPUpdateMessageParser implements MessageParser, MessageSeria
         MessageUtil.formatMessage(TYPE, messageBody, bytes);
     }
 
-    private void writePathIdPrefix(final ByteBuf byteBuf, final PathId pathId, final Ipv4Prefix ipv4Prefix) {
+    private static void writePathIdPrefix(final ByteBuf byteBuf, final PathId pathId, final Ipv4Prefix ipv4Prefix) {
         PathIdUtil.writePathId(pathId, byteBuf);
         ByteBufWriteUtil.writeMinimalPrefix(ipv4Prefix, byteBuf);
     }
@@ -141,15 +144,19 @@ public final class BGPUpdateMessageParser implements MessageParser, MessageSeria
         if (withdrawnRoutesLength == 0 && totalPathAttrLength == 0) {
             return builder.build();
         }
+        final Optional<BGPTreatAsWithdrawException> withdrawCause;
         if (totalPathAttrLength > 0) {
+            final ParsedAttributes attributes;
             try {
-                final Attributes attributes
-                        = this.reg.parseAttributes(buffer.readSlice(totalPathAttrLength), constraint);
-                builder.setAttributes(attributes);
+                attributes = this.reg.parseAttributes(buffer.readSlice(totalPathAttrLength), constraint);
             } catch (final RuntimeException | BGPParsingException e) {
                 // Catch everything else and turn it into a BGPDocumentedException
                 throw new BGPDocumentedException("Could not parse BGP attributes.", BGPError.MALFORMED_ATTR_LIST, e);
             }
+            builder.setAttributes(attributes.getAttributes());
+            withdrawCause = attributes.getWithdrawCause();
+        } else {
+            withdrawCause = Optional.empty();
         }
         final List<Nlri> nlri = new ArrayList<>();
         while (buffer.isReadable()) {
@@ -163,8 +170,14 @@ public final class BGPUpdateMessageParser implements MessageParser, MessageSeria
         if (!nlri.isEmpty()) {
             builder.setNlri(nlri);
         }
-        final Update msg = builder.build();
+        Update msg = builder.build();
         checkMandatoryAttributesPresence(msg);
+
+        if (withdrawCause.isPresent()) {
+            // FIXME: BGPCEP-359: check if we can treat the message as withdraw and convert the message
+            throw withdrawCause.get().toDocumentedException();
+        }
+
         LOG.debug("BGP Update message was parsed {}.", msg);
         return msg;
     }
index e549185a047fc2ea8278860463075a229c1e2356..1a4e320fe8cbe8833d07e94438660ddc27af5047 100644 (file)
@@ -58,7 +58,7 @@ public class PathAttributeParserTest {
         final BGPExtensionProviderContext providerContext = ServiceLoaderBGPExtensionProviderContext
             .getSingletonInstance();
         final Attributes pathAttributes = providerContext.getAttributeRegistry()
-            .parseAttributes(buffer, null);
+            .parseAttributes(buffer, null).getAttributes();
         final Aigp aigp = pathAttributes.getAigp();
         final AigpTlv tlv = aigp.getAigpTlv();
 
@@ -79,7 +79,7 @@ public class PathAttributeParserTest {
         final BGPExtensionProviderContext providerContext = ServiceLoaderBGPExtensionProviderContext
             .getSingletonInstance();
         final Attributes pathAttributes = providerContext.getAttributeRegistry()
-            .parseAttributes(inputData, null);
+            .parseAttributes(inputData, null).getAttributes();
         final Aigp aigp = pathAttributes.getAigp();
 
         final AttributesBuilder pathAttributesBuilder = new AttributesBuilder();
index 68c467671b746237c240be89ebd76dbc16ed65ad..16469850b7e91dbe90fb64de1b1b1357fa7029e7 100644 (file)
@@ -51,7 +51,7 @@ public class CommunitiesAttributeParserTest {
             .serializeAttribute(paBuilder.build(), actual);
         assertArrayEquals(CommunitiesBytes, ByteArray.getAllBytes(actual));
         final Attributes attributeOut = ServiceLoaderBGPExtensionProviderContext.getSingletonInstance()
-            .getAttributeRegistry().parseAttributes(actual, null);
+            .getAttributeRegistry().parseAttributes(actual, null).getAttributes();
         assertEquals(comms, attributeOut.getCommunities());
     }
 
index 4dc857cd191582e18f21a3e9aa809d3ccc474f8a..f67f945d5e50393a08802d01ddd5a429040baa80 100644 (file)
@@ -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.Attributes;
 
@@ -23,11 +24,11 @@ public interface AttributeRegistry {
      * @param buffer Input buffer.
      * @param constraints Peer specific constraint.
      * @return Decoded BGP Attribute.
-     * @throws BGPDocumentedException
-     * @throws BGPParsingException
+     * @throws BGPDocumentedException when an unrecoverable error occurs, which is documented via {@link BGPError}
+     * @throws BGPParsingException when a general unrecoverable parsing error occurs
      */
-    @Nonnull Attributes parseAttributes(@Nonnull ByteBuf buffer, @Nullable PeerSpecificParserConstraint constraints)
-            throws BGPDocumentedException, BGPParsingException;
+    @Nonnull ParsedAttributes parseAttributes(@Nonnull ByteBuf buffer,
+            @Nullable PeerSpecificParserConstraint constraints) throws BGPDocumentedException, BGPParsingException;
 
     /**
      * Serialize BGP Attribute to buffer.
diff --git a/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/ParsedAttributes.java b/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/ParsedAttributes.java
new file mode 100644 (file)
index 0000000..b10d29d
--- /dev/null
@@ -0,0 +1,40 @@
+/*
+ * 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.spi;
+
+import static java.util.Objects.requireNonNull;
+
+import java.util.Optional;
+import org.eclipse.jdt.annotation.NonNull;
+import org.eclipse.jdt.annotation.Nullable;
+import org.opendaylight.protocol.bgp.parser.BGPTreatAsWithdrawException;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev180329.path.attributes.Attributes;
+
+/**
+ * Parsed {@link Attributes}, potentially indicating withdraw.
+ *
+ * @author Robert Varga
+ */
+public final class ParsedAttributes {
+    private final @NonNull Attributes attributes;
+    private final @Nullable BGPTreatAsWithdrawException withdrawCause;
+
+    public ParsedAttributes(final @NonNull Attributes attributes,
+            final @Nullable BGPTreatAsWithdrawException withdrawCause) {
+        this.attributes = requireNonNull(attributes);
+        this.withdrawCause = withdrawCause;
+    }
+
+    public @NonNull Attributes getAttributes() {
+        return attributes;
+    }
+
+    public @NonNull Optional<BGPTreatAsWithdrawException> getWithdrawCause() {
+        return Optional.ofNullable(withdrawCause);
+    }
+}
index 8d9dcefa28ab4eeb4294c82fa1eee03f99c3146a..1e87b046a6d479e213ed46f180eaf9c74d874f9e 100644 (file)
@@ -25,6 +25,7 @@ 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;
+import org.opendaylight.protocol.bgp.parser.spi.ParsedAttributes;
 import org.opendaylight.protocol.bgp.parser.spi.PeerSpecificParserConstraint;
 import org.opendaylight.protocol.bgp.parser.spi.RevisedErrorHandling;
 import org.opendaylight.protocol.concepts.AbstractRegistration;
@@ -128,13 +129,14 @@ final class SimpleAttributeRegistry implements AttributeRegistry {
     }
 
     @Override
-    public Attributes parseAttributes(final ByteBuf buffer, final PeerSpecificParserConstraint constraint)
+    public ParsedAttributes 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, errorHandling, attributes);
         }
+
         /*
          * TreeMap guarantees that we will be invoking the parser in the order
          * of increasing attribute type.
@@ -160,13 +162,7 @@ final class SimpleAttributeRegistry implements AttributeRegistry {
             }
         }
         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();
+        return new ParsedAttributes(builder.build(), withdrawCause);
     }
 
     @Override
index e7671d22a58c766247f17344624de62b07e7b538..9ddc94b7ef58e2d9b917d9f2ce60541148fa81c5 100644 (file)
@@ -47,7 +47,7 @@ public class UnrecognizedAttributesTest {
     public void testUnrecognizedAttributes() throws BGPDocumentedException, BGPParsingException {
         final byte[] attributeBytes = { (byte)0xe0, 0x00, 0x05, 0x01, 0x02, 0x03, 0x04, 0x05 };
         final List<UnrecognizedAttributes> unrecogAttribs = simpleAttrReg
-            .parseAttributes(Unpooled.wrappedBuffer(attributeBytes), null).getUnrecognizedAttributes();
+            .parseAttributes(Unpooled.wrappedBuffer(attributeBytes), null).getAttributes().getUnrecognizedAttributes();
         assertEquals(UNRECOGNIZED_ATTRIBUTE_COUNT, unrecogAttribs.size());
         final UnrecognizedAttributes unrecogAttrib = unrecogAttribs.get(FIRST_ATTRIBUTE);
         final UnrecognizedAttributesKey expectedAttribKey =