From: DanielCV <daniel.viswanathan@verizon.com>
Date: Tue, 19 Jul 2022 13:39:05 +0000 (+0530)
Subject: Allow NetconfChunkAggregator's maximum size to be adjusted
X-Git-Tag: v4.0.1~6
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=netconf.git;a=commitdiff_plain;h=b48e597dad44046da6af9ada1b22df0b51aa3a9a

Allow NetconfChunkAggregator's maximum size to be adjusted

There are devices which send out arbitrarily-large chunks, requiring
a potentially large buffer to hold the incoming message.

This patch allows each instance to have a the chunk size specified
as well as control over the default size via a system property.

JIRA: NETCONF-888
Change-Id: Iec041a4ba9c8886cceb44fa86d07320bb5ae942b
Signed-off-by: DanielCV <daniel.viswanathan@verizon.com>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
---

diff --git a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiator.java b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiator.java
index 3949db62d2..6ee1c8b57c 100644
--- a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiator.java
+++ b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiator.java
@@ -10,6 +10,7 @@ package org.opendaylight.netconf.nettyutil;
 import static com.google.common.base.Preconditions.checkState;
 import static java.util.Objects.requireNonNull;
 
+import com.google.common.annotations.Beta;
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelHandler;
 import io.netty.channel.ChannelHandlerContext;
@@ -20,6 +21,7 @@ import io.netty.util.Timer;
 import io.netty.util.concurrent.Promise;
 import java.util.Optional;
 import java.util.concurrent.TimeUnit;
+import org.checkerframework.checker.index.qual.NonNegative;
 import org.checkerframework.checker.lock.qual.GuardedBy;
 import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.netconf.api.NetconfDocumentedException;
@@ -50,6 +52,27 @@ public abstract class AbstractNetconfSessionNegotiator<S extends AbstractNetconf
 
     private static final Logger LOG = LoggerFactory.getLogger(AbstractNetconfSessionNegotiator.class);
     private static final String NAME_OF_EXCEPTION_HANDLER = "lastExceptionHandler";
+    private static final String DEFAULT_MAXIMUM_CHUNK_SIZE_PROP = "org.opendaylight.netconf.default.maximum.chunk.size";
+    private static final int DEFAULT_MAXIMUM_CHUNK_SIZE_DEFAULT = 16 * 1024 * 1024;
+
+    /**
+     * Default upper bound on the size of an individual chunk. This value can be controlled through
+     * {@value #DEFAULT_MAXIMUM_CHUNK_SIZE_PROP} system property and defaults to
+     * {@value #DEFAULT_MAXIMUM_CHUNK_SIZE_DEFAULT} bytes.
+     */
+    @Beta
+    public static final @NonNegative int DEFAULT_MAXIMUM_INCOMING_CHUNK_SIZE;
+
+    static {
+        final int propValue = Integer.getInteger(DEFAULT_MAXIMUM_CHUNK_SIZE_PROP, DEFAULT_MAXIMUM_CHUNK_SIZE_DEFAULT);
+        if (propValue <= 0) {
+            LOG.warn("Ignoring invalid {} value {}", DEFAULT_MAXIMUM_CHUNK_SIZE_PROP, propValue);
+            DEFAULT_MAXIMUM_INCOMING_CHUNK_SIZE = DEFAULT_MAXIMUM_CHUNK_SIZE_DEFAULT;
+        } else {
+            DEFAULT_MAXIMUM_INCOMING_CHUNK_SIZE = propValue;
+        }
+        LOG.debug("Default maximum incoming NETCONF chunk size is {} bytes", DEFAULT_MAXIMUM_INCOMING_CHUNK_SIZE);
+    }
 
     private final @NonNull NetconfHelloMessage localHello;
     protected final Channel channel;
diff --git a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/NetconfChunkAggregator.java b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/NetconfChunkAggregator.java
index 74bf492292..cab1d741b1 100644
--- a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/NetconfChunkAggregator.java
+++ b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/NetconfChunkAggregator.java
@@ -5,15 +5,18 @@
  * 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.netconf.nettyutil.handler;
 
+import static com.google.common.base.Preconditions.checkArgument;
+
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.CompositeByteBuf;
 import io.netty.buffer.Unpooled;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.handler.codec.ByteToMessageDecoder;
 import java.util.List;
+import org.checkerframework.checker.index.qual.NonNegative;
+import org.opendaylight.netconf.nettyutil.AbstractNetconfSessionNegotiator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -23,7 +26,9 @@ public class NetconfChunkAggregator extends ByteToMessageDecoder {
     private static final String GOT_PARAM_WHILE_WAITING_FOR_PARAM_PARAM = "Got byte {} while waiting for {}-{}";
     private static final String GOT_PARAM_WHILE_WAITING_FOR_PARAM_PARAM_PARAM =
         "Got byte {} while waiting for {}-{}-{}";
-    public static final int DEFAULT_MAXIMUM_CHUNK_SIZE = 16 * 1024 * 1024;
+
+    public static final @NonNegative int DEFAULT_MAXIMUM_CHUNK_SIZE =
+        AbstractNetconfSessionNegotiator.DEFAULT_MAXIMUM_INCOMING_CHUNK_SIZE;
 
     private enum State {
         HEADER_ONE, // \n
@@ -37,11 +42,29 @@ public class NetconfChunkAggregator extends ByteToMessageDecoder {
         FOOTER_FOUR, // \n
     }
 
-    private final int maxChunkSize = DEFAULT_MAXIMUM_CHUNK_SIZE;
+    private final int maxChunkSize;
     private State state = State.HEADER_ONE;
     private long chunkSize;
     private CompositeByteBuf chunk;
 
+    /**
+     * Construct an instance with maximum chunk size set to {@link #DEFAULT_MAXIMUM_CHUNK_SIZE}.
+     */
+    public NetconfChunkAggregator() {
+        this(DEFAULT_MAXIMUM_CHUNK_SIZE);
+    }
+
+    /**
+     * Construct an instance with specified maximum chunk size.
+     *
+     * @param maxChunkSize maximum chunk size
+     * @throws IllegalArgumentException if {@code maxChunkSize} is negative
+     */
+    public NetconfChunkAggregator(final @NonNegative int maxChunkSize) {
+        this.maxChunkSize = maxChunkSize;
+        checkArgument(maxChunkSize > 0, "Negative maximum chunk size %s", maxChunkSize);
+    }
+
     private static void checkNewLine(final byte byteToCheck, final String errorMessage) {
         if (byteToCheck != '\n') {
             LOG.debug(GOT_PARAM_WHILE_WAITING_FOR_PARAM, byteToCheck, (byte)'\n');
@@ -59,7 +82,7 @@ public class NetconfChunkAggregator extends ByteToMessageDecoder {
     private void checkChunkSize() {
         if (chunkSize > maxChunkSize) {
             LOG.debug("Parsed chunk size {}, maximum allowed is {}", chunkSize, maxChunkSize);
-            throw new IllegalStateException("Maximum chunk size exceeded");
+            throw new IllegalStateException("Chunk size " + chunkSize + " exceeds maximum " + maxChunkSize);
         }
     }