From 834c7f32cc3e4836c64c5874ebaaec17e9dab899 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Wed, 5 Dec 2018 23:18:34 +0100 Subject: [PATCH] Allow AttributeRegistry to signal treat-as-withdraw 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 --- ...stinguisherLabelsAttributeHandlerTest.java | 2 +- .../PMSITunnelAttributeHandlerTest.java | 24 +++++------ .../impl/message/BGPUpdateMessageParser.java | 23 ++++++++--- .../parser/impl/PathAttributeParserTest.java | 4 +- .../CommunitiesAttributeParserTest.java | 2 +- .../bgp/parser/spi/AttributeRegistry.java | 9 +++-- .../bgp/parser/spi/ParsedAttributes.java | 40 +++++++++++++++++++ .../spi/pojo/SimpleAttributeRegistry.java | 12 ++---- .../spi/pojo/UnrecognizedAttributesTest.java | 2 +- 9 files changed, 84 insertions(+), 34 deletions(-) create mode 100644 bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/ParsedAttributes.java diff --git a/bgp/extensions/mvpn/src/test/java/org/opendaylight/protocol/bgp/mvpn/impl/attributes/PEDistinguisherLabelsAttributeHandlerTest.java b/bgp/extensions/mvpn/src/test/java/org/opendaylight/protocol/bgp/mvpn/impl/attributes/PEDistinguisherLabelsAttributeHandlerTest.java index 8831a686cd..da5309e7f5 100644 --- a/bgp/extensions/mvpn/src/test/java/org/opendaylight/protocol/bgp/mvpn/impl/attributes/PEDistinguisherLabelsAttributeHandlerTest.java +++ b/bgp/extensions/mvpn/src/test/java/org/opendaylight/protocol/bgp/mvpn/impl/attributes/PEDistinguisherLabelsAttributeHandlerTest.java @@ -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); } diff --git a/bgp/extensions/mvpn/src/test/java/org/opendaylight/protocol/bgp/mvpn/impl/attributes/PMSITunnelAttributeHandlerTest.java b/bgp/extensions/mvpn/src/test/java/org/opendaylight/protocol/bgp/mvpn/impl/attributes/PMSITunnelAttributeHandlerTest.java index 482b4a4b74..20208954c2 100644 --- a/bgp/extensions/mvpn/src/test/java/org/opendaylight/protocol/bgp/mvpn/impl/attributes/PMSITunnelAttributeHandlerTest.java +++ b/bgp/extensions/mvpn/src/test/java/org/opendaylight/protocol/bgp/mvpn/impl/attributes/PMSITunnelAttributeHandlerTest.java @@ -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); } diff --git a/bgp/parser-impl/src/main/java/org/opendaylight/protocol/bgp/parser/impl/message/BGPUpdateMessageParser.java b/bgp/parser-impl/src/main/java/org/opendaylight/protocol/bgp/parser/impl/message/BGPUpdateMessageParser.java index 42daa9f173..275489cdd7 100755 --- a/bgp/parser-impl/src/main/java/org/opendaylight/protocol/bgp/parser/impl/message/BGPUpdateMessageParser.java +++ b/bgp/parser-impl/src/main/java/org/opendaylight/protocol/bgp/parser/impl/message/BGPUpdateMessageParser.java @@ -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 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 = 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; } diff --git a/bgp/parser-impl/src/test/java/org/opendaylight/protocol/bgp/parser/impl/PathAttributeParserTest.java b/bgp/parser-impl/src/test/java/org/opendaylight/protocol/bgp/parser/impl/PathAttributeParserTest.java index e549185a04..1a4e320fe8 100644 --- a/bgp/parser-impl/src/test/java/org/opendaylight/protocol/bgp/parser/impl/PathAttributeParserTest.java +++ b/bgp/parser-impl/src/test/java/org/opendaylight/protocol/bgp/parser/impl/PathAttributeParserTest.java @@ -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(); diff --git a/bgp/parser-impl/src/test/java/org/opendaylight/protocol/bgp/parser/impl/message/update/CommunitiesAttributeParserTest.java b/bgp/parser-impl/src/test/java/org/opendaylight/protocol/bgp/parser/impl/message/update/CommunitiesAttributeParserTest.java index 68c467671b..16469850b7 100644 --- a/bgp/parser-impl/src/test/java/org/opendaylight/protocol/bgp/parser/impl/message/update/CommunitiesAttributeParserTest.java +++ b/bgp/parser-impl/src/test/java/org/opendaylight/protocol/bgp/parser/impl/message/update/CommunitiesAttributeParserTest.java @@ -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()); } diff --git a/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/AttributeRegistry.java b/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/AttributeRegistry.java index 4dc857cd19..f67f945d5e 100644 --- a/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/AttributeRegistry.java +++ b/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/AttributeRegistry.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.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 index 0000000000..b10d29d08e --- /dev/null +++ b/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/ParsedAttributes.java @@ -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 getWithdrawCause() { + return Optional.ofNullable(withdrawCause); + } +} 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 8d9dcefa28..1e87b046a6 100644 --- 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.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 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 diff --git a/bgp/parser-spi/src/test/java/org/opendaylight/protocol/bgp/parser/spi/pojo/UnrecognizedAttributesTest.java b/bgp/parser-spi/src/test/java/org/opendaylight/protocol/bgp/parser/spi/pojo/UnrecognizedAttributesTest.java index e7671d22a5..9ddc94b7ef 100644 --- a/bgp/parser-spi/src/test/java/org/opendaylight/protocol/bgp/parser/spi/pojo/UnrecognizedAttributesTest.java +++ b/bgp/parser-spi/src/test/java/org/opendaylight/protocol/bgp/parser/spi/pojo/UnrecognizedAttributesTest.java @@ -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 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 = -- 2.36.6