From 9a33d43e3516ad73e0ec74df82034f9fe23a5c28 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sun, 23 Apr 2023 23:23:06 +0200 Subject: [PATCH] Improve RouteDistinguisherUtil Do not muck with NormalizedNodes.findNode(), as we have the same functionlity directly available. Eliminate RDType -- it is a noble idea, but all we really need is just valid constants. This improves both paths, as the arguments are now known compile-time constants. Finally improve nullness annotations, eliminating the need for a requireNonNull() in flowspec. Change-Id: I176db96b783649de166797ee84bf919e68a57196 Signed-off-by: Robert Varga --- .../bgp/concepts/RouteDistinguisherUtil.java | 73 +++++++------------ .../AbstractFlowspecL3vpnNlriParser.java | 10 +-- 2 files changed, 31 insertions(+), 52 deletions(-) diff --git a/bgp/concepts/src/main/java/org/opendaylight/bgp/concepts/RouteDistinguisherUtil.java b/bgp/concepts/src/main/java/org/opendaylight/bgp/concepts/RouteDistinguisherUtil.java index 21a9c40db2..0d7510bd57 100644 --- a/bgp/concepts/src/main/java/org/opendaylight/bgp/concepts/RouteDistinguisherUtil.java +++ b/bgp/concepts/src/main/java/org/opendaylight/bgp/concepts/RouteDistinguisherUtil.java @@ -11,8 +11,9 @@ import static com.google.common.base.Preconditions.checkState; import static java.util.Objects.requireNonNull; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Preconditions; import io.netty.buffer.ByteBuf; +import org.eclipse.jdt.annotation.NonNull; +import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.protocol.util.Ipv4Util; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Ipv4AddressNoZone; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.types.rev200120.RdAs; @@ -22,8 +23,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.type import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.types.rev200120.RouteDistinguisherBuilder; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.DataContainerNode; -import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; -import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodes; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -38,6 +37,11 @@ public final class RouteDistinguisherUtil { @VisibleForTesting static final char SEPARATOR = ':'; + // Route descriptor types + private static final int RD_AS_2BYTE = 0; + private static final int RD_IPV4 = 1; + private static final int RD_AS_4BYTE = 2; + private RouteDistinguisherUtil() { // Hidden on purpose } @@ -66,7 +70,7 @@ public final class RouteDistinguisherUtil { final int first = findSeparator(value, 0); checkNoColon(value, first); - buf.writeShort(RDType.AS_4BYTE.value); + buf.writeShort(RD_AS_4BYTE); buf.writeInt(Integer.parseUnsignedInt(value.substring(0, first))); buf.writeShort(Integer.parseUnsignedInt(value.substring(first + 1))); } @@ -77,7 +81,7 @@ public final class RouteDistinguisherUtil { final int second = findSeparator(value, first); checkNoColon(value, second); - buf.writeShort(RDType.AS_2BYTE.value); + buf.writeShort(RD_AS_2BYTE); buf.writeShort(Integer.parseUnsignedInt(value.substring(first, second))); buf.writeInt(Integer.parseUnsignedInt(value.substring(second + 1))); } @@ -87,7 +91,7 @@ public final class RouteDistinguisherUtil { final int first = findSeparator(value, 0); checkNoColon(value, first); - buf.writeShort(RDType.IPV4.value); + buf.writeShort(RD_IPV4); buf.writeBytes(Ipv4Util.bytesForAddress(new Ipv4AddressNoZone(value.substring(0, first)))); buf.writeShort(Integer.parseUnsignedInt(value.substring(first + 1))); } @@ -105,13 +109,12 @@ public final class RouteDistinguisherUtil { /** * Parses three types of route distinguisher from given ByteBuf. */ - public static RouteDistinguisher parseRouteDistinguisher(final ByteBuf buffer) { - Preconditions.checkState(buffer != null && buffer.isReadable(RD_LENGTH), + public static @NonNull RouteDistinguisher parseRouteDistinguisher(final ByteBuf buffer) { + checkState(buffer != null && buffer.isReadable(RD_LENGTH), "Cannot read Route Distinguisher from provided buffer."); final int type = buffer.readUnsignedShort(); - final RDType rdType = RDType.valueOf(type); - switch (rdType) { - case AS_2BYTE: + switch (type) { + case RD_AS_2BYTE: return new RouteDistinguisher(new RdTwoOctetAs(new StringBuilder() .append(type) .append(SEPARATOR) @@ -119,13 +122,13 @@ public final class RouteDistinguisherUtil { .append(SEPARATOR) .append(buffer.readUnsignedInt()) .toString())); - case IPV4: + case RD_IPV4: return new RouteDistinguisher(new RdIpv4(new StringBuilder() .append(Ipv4Util.addressForByteBuf(buffer).getValue()) .append(SEPARATOR) .append(buffer.readUnsignedShort()) .toString())); - case AS_4BYTE: + case RD_AS_4BYTE: return new RouteDistinguisher(new RdAs(new StringBuilder() .append(buffer.readUnsignedInt()) .append(SEPARATOR) @@ -134,55 +137,33 @@ public final class RouteDistinguisherUtil { default: // now that this RD type is not supported, we want to read the remain 6 bytes // in order to get the byte index correct - final StringBuilder routeDistiguisher = new StringBuilder(); + final var sb = new StringBuilder(); for (int i = 0; i < 6; i++) { - routeDistiguisher.append("0x").append(Integer.toHexString(buffer.readByte() & 0xFF)).append(' '); + sb.append("0x").append(Integer.toHexString(buffer.readByte() & 0xFF)).append(' '); } - LOG.debug("Invalid Route Distinguisher: type={}, rawRouteDistinguisherValue={}", type, - routeDistiguisher); + LOG.debug("Invalid Route Distinguisher: type={}, rawRouteDistinguisherValue={}", type, sb); throw new IllegalArgumentException("Invalid Route Distinguisher type " + type); } } - public static RouteDistinguisher parseRouteDistinguisher(final String str) { + public static @Nullable RouteDistinguisher parseRouteDistinguisher(final String str) { return str == null ? null : RouteDistinguisherBuilder.getDefaultInstance(str); } - public static RouteDistinguisher parseRouteDistinguisher(final Object obj) { - if (obj instanceof String) { - return RouteDistinguisherBuilder.getDefaultInstance((String) obj); - } else if (obj instanceof RouteDistinguisher) { - return (RouteDistinguisher) obj; + public static @Nullable RouteDistinguisher parseRouteDistinguisher(final Object obj) { + if (obj instanceof String str) { + return RouteDistinguisherBuilder.getDefaultInstance(str); + } else if (obj instanceof RouteDistinguisher rd) { + return rd; } else { return null; } } - public static RouteDistinguisher extractRouteDistinguisher(final DataContainerNode route, + public static @Nullable RouteDistinguisher extractRouteDistinguisher(final DataContainerNode route, final NodeIdentifier rdNid) { - final NormalizedNode rdNode = NormalizedNodes.findNode(route, rdNid).orElse(null); + final var rdNode = route.childByArg(rdNid); return rdNode == null ? null : parseRouteDistinguisher(rdNode.body()); } - private enum RDType { - AS_2BYTE(0), - IPV4(1), - AS_4BYTE(2), - INVALID(-1); - - public final int value; - - RDType(final int val) { - this.value = val; - } - - static RDType valueOf(final int value) { - for (RDType type : values()) { - if (type.value == value) { - return type; - } - } - return INVALID; - } - } } diff --git a/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/l3vpn/AbstractFlowspecL3vpnNlriParser.java b/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/l3vpn/AbstractFlowspecL3vpnNlriParser.java index 394737dee2..a66b6e472f 100644 --- a/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/l3vpn/AbstractFlowspecL3vpnNlriParser.java +++ b/bgp/extensions/flowspec/src/main/java/org/opendaylight/protocol/bgp/flowspec/l3vpn/AbstractFlowspecL3vpnNlriParser.java @@ -7,7 +7,6 @@ */ package org.opendaylight.protocol.bgp.flowspec.l3vpn; -import static java.util.Objects.requireNonNull; import static org.opendaylight.bgp.concepts.RouteDistinguisherUtil.extractRouteDistinguisher; import com.google.common.annotations.VisibleForTesting; @@ -55,8 +54,8 @@ public abstract class AbstractFlowspecL3vpnNlriParser extends AbstractFlowspecNl /** * For flowspec-l3vpn, there is a route distinguisher field at the beginning of NLRI (8 bytes). */ - private static RouteDistinguisher readRouteDistinguisher(final ByteBuf nlri) { - final RouteDistinguisher rd = RouteDistinguisherUtil.parseRouteDistinguisher(nlri); + private static @NonNull RouteDistinguisher readRouteDistinguisher(final ByteBuf nlri) { + final var rd = RouteDistinguisherUtil.parseRouteDistinguisher(nlri); LOG.trace("Route Distinguisher read from NLRI: {}", rd); return rd; } @@ -74,7 +73,7 @@ public abstract class AbstractFlowspecL3vpnNlriParser extends AbstractFlowspecNl protected final DestinationType parseAdvertizedNlri(final ByteBuf nlri, final PathId pathId) throws BGPParsingException { readNlriLength(nlri); - return createAdvertizedRoutesDestinationType(requireNonNull(readRouteDistinguisher(nlri)), + return createAdvertizedRoutesDestinationType(readRouteDistinguisher(nlri), parseL3vpnNlriFlowspecList(nlri), pathId); } @@ -93,8 +92,7 @@ public abstract class AbstractFlowspecL3vpnNlriParser extends AbstractFlowspecNl protected final DestinationType parseWithdrawnNlri(final ByteBuf nlri, final PathId pathId) throws BGPParsingException { readNlriLength(nlri); - return createWithdrawnDestinationType(requireNonNull(readRouteDistinguisher(nlri)), - parseL3vpnNlriFlowspecList(nlri), pathId); + return createWithdrawnDestinationType(readRouteDistinguisher(nlri), parseL3vpnNlriFlowspecList(nlri), pathId); } /** -- 2.36.6