From 53988376db47afba84c3bae8a0c32d4ac0e73779 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Tue, 13 May 2014 19:42:04 +0900 Subject: [PATCH] Optimize SslHandler in an OpenSslEngine-friendly way Motivation: Previous fix for the OpenSslEngine compatibility issue (#2216 and 18b0e95659c057b126653bad2f018a8ce5385255) was to feed SSL records one by one to OpenSslEngine.unwrap(). It is not optimal because it will result in more JNI calls. Modifications: - Do not feed SSL records one by one. - Feed as many records as possible up to MAX_ENCRYPTED_PACKET_LENGTH - Deduplicate MAX_ENCRYPTED_PACKET_LENGTH definitions Result: - No allocation of intemediary arrays - Reduced number of calls to SSLEngine and thus its underlying JNI calls - A tad bit increase in throughput, probably reverting the tiny drop caused by 18b0e95659c057b126653bad2f018a8ce5385255 --- .../netty/handler/ssl/OpenSslEngine.java | 8 ++- .../netty/handler/ssl/SslBufferPool.java | 5 +- .../jboss/netty/handler/ssl/SslHandler.java | 70 ++++++------------- 3 files changed, 27 insertions(+), 56 deletions(-) diff --git a/src/main/java/org/jboss/netty/handler/ssl/OpenSslEngine.java b/src/main/java/org/jboss/netty/handler/ssl/OpenSslEngine.java index 0a370bfd2b..cfd27c9b8d 100644 --- a/src/main/java/org/jboss/netty/handler/ssl/OpenSslEngine.java +++ b/src/main/java/org/jboss/netty/handler/ssl/OpenSslEngine.java @@ -65,7 +65,9 @@ public final class OpenSslEngine extends SSLEngine { private static final int MAX_PLAINTEXT_LENGTH = 16 * 1024; // 2^14 private static final int MAX_COMPRESSED_LENGTH = MAX_PLAINTEXT_LENGTH + 1024; private static final int MAX_CIPHERTEXT_LENGTH = MAX_COMPRESSED_LENGTH + 1024; - private static final int MAX_ENCRYPTED_PACKET = MAX_CIPHERTEXT_LENGTH + 5 + 20 + 256; + + // Header (5) + Data (2^14) + Compression (1024) + Encryption (1024) + MAC (20) + Padding (256) + static final int MAX_ENCRYPTED_PACKET_LENGTH = MAX_CIPHERTEXT_LENGTH + 5 + 20 + 256; private static final AtomicIntegerFieldUpdater DESTROYED_UPDATER = AtomicIntegerFieldUpdater.newUpdater(OpenSslEngine.class, "destroyed"); @@ -432,7 +434,7 @@ public final class OpenSslEngine extends SSLEngine { } // protect against protocol overflow attack vector - if (src.remaining() > MAX_ENCRYPTED_PACKET) { + if (src.remaining() > MAX_ENCRYPTED_PACKET_LENGTH) { isInboundDone = true; isOutboundDone = true; engineClosed = true; @@ -689,7 +691,7 @@ public final class OpenSslEngine extends SSLEngine { } public int getPacketBufferSize() { - return MAX_ENCRYPTED_PACKET; + return MAX_ENCRYPTED_PACKET_LENGTH; } public int getApplicationBufferSize() { diff --git a/src/main/java/org/jboss/netty/handler/ssl/SslBufferPool.java b/src/main/java/org/jboss/netty/handler/ssl/SslBufferPool.java index 06d25dcbfb..f15529651a 100644 --- a/src/main/java/org/jboss/netty/handler/ssl/SslBufferPool.java +++ b/src/main/java/org/jboss/netty/handler/ssl/SslBufferPool.java @@ -38,8 +38,7 @@ import java.util.concurrent.atomic.AtomicInteger; public class SslBufferPool { // Add 1024 as a room for compressed data and another 1024 for Apache Harmony compatibility. - private static final int MAX_PACKET_SIZE = 16665 + 2048; - private static final int MAX_PACKET_SIZE_ALIGNED = (MAX_PACKET_SIZE / 128 + 1) * 128; + private static final int MAX_PACKET_SIZE_ALIGNED = (OpenSslEngine.MAX_ENCRYPTED_PACKET_LENGTH / 128 + 1) * 128; private static final int DEFAULT_POOL_SIZE = MAX_PACKET_SIZE_ALIGNED * 1024; @@ -156,7 +155,7 @@ public class SslBufferPool { // Note that we can allocate more buffers than maxBufferCount. // We will discard the buffers allocated after numAllocations reached maxBufferCount in releaseBuffer(). numAllocations.incrementAndGet(); - buf = allocate(MAX_PACKET_SIZE); + buf = allocate(OpenSslEngine.MAX_ENCRYPTED_PACKET_LENGTH); } } diff --git a/src/main/java/org/jboss/netty/handler/ssl/SslHandler.java b/src/main/java/org/jboss/netty/handler/ssl/SslHandler.java index 3628609562..83dfeb999b 100644 --- a/src/main/java/org/jboss/netty/handler/ssl/SslHandler.java +++ b/src/main/java/org/jboss/netty/handler/ssl/SslHandler.java @@ -182,8 +182,7 @@ import static org.jboss.netty.channel.Channels.*; public class SslHandler extends FrameDecoder implements ChannelDownstreamHandler { - private static final InternalLogger logger = - InternalLoggerFactory.getInstance(SslHandler.class); + private static final InternalLogger logger = InternalLoggerFactory.getInstance(SslHandler.class); private static final ByteBuffer EMPTY_BUFFER = ByteBuffer.allocate(0); @@ -851,31 +850,25 @@ public class SslHandler extends FrameDecoder protected Object decode( final ChannelHandlerContext ctx, Channel channel, ChannelBuffer in) throws Exception { - // Keeps the list of the length of every SSL record in the input buffer. - int[] recordLengths = null; - int nRecords = 0; - final int startOffset = in.readerIndex(); final int endOffset = in.writerIndex(); int offset = startOffset; + int totalLength = 0; // If we calculated the length of the current SSL record before, use that information. if (packetLength > 0) { if (endOffset - startOffset < packetLength) { return null; } else { - recordLengths = new int[4]; - recordLengths[0] = packetLength; - nRecords = 1; - offset += packetLength; + totalLength = packetLength; packetLength = 0; } } boolean nonSslRecord = false; - for (;;) { + while (totalLength < OpenSslEngine.MAX_ENCRYPTED_PACKET_LENGTH) { final int readableBytes = endOffset - offset; if (readableBytes < 5) { break; @@ -895,23 +888,18 @@ public class SslHandler extends FrameDecoder break; } - // We have a whole packet. - // Remember the offset and length of the current packet. - if (recordLengths == null) { - recordLengths = new int[4]; + int newTotalLength = totalLength + packetLength; + if (newTotalLength > OpenSslEngine.MAX_ENCRYPTED_PACKET_LENGTH) { + // Don't read too much. + break; } - if (nRecords == recordLengths.length) { - int[] newRecordLengths = new int[recordLengths.length << 1]; - System.arraycopy(recordLengths, 0, newRecordLengths, 0, recordLengths.length); - recordLengths = newRecordLengths; - } - recordLengths[nRecords ++] = packetLength; + // We have a whole packet. // Increment the offset to handle the next packet. offset += packetLength; + totalLength = newTotalLength; } - final int totalLength = offset - startOffset; ChannelBuffer unwrapped = null; if (totalLength > 0) { // The buffer contains one or more full SSL records. @@ -924,8 +912,10 @@ public class SslHandler extends FrameDecoder // 4) unwrapLater(...) calls decode(...) // // See https://github.com/netty/netty/issues/1534 - assert recordLengths != null; - unwrapped = unwrapMultiple(ctx, channel, in, totalLength, recordLengths, nRecords); + + final ByteBuffer inNetBuf = in.toByteBuffer(in.readerIndex(), totalLength); + unwrapped = unwrap(ctx, channel, in, inNetBuf, totalLength); + assert !inNetBuf.hasRemaining() || engine.isInboundDone(); } if (nonSslRecord) { @@ -1238,43 +1228,23 @@ public class SslHandler extends FrameDecoder * Calls {@link SSLEngine#unwrap(ByteBuffer, ByteBuffer)} with an empty buffer to handle handshakes, etc. */ private void unwrapNonAppData(ChannelHandlerContext ctx, Channel channel) throws SSLException { - unwrapSingle(ctx, channel, ChannelBuffers.EMPTY_BUFFER, EMPTY_BUFFER, null, -1); + unwrap(ctx, channel, ChannelBuffers.EMPTY_BUFFER, EMPTY_BUFFER, -1); } /** - * Unwraps multiple inbound SSL records. + * Unwraps inbound SSL records. */ - private ChannelBuffer unwrapMultiple( - ChannelHandlerContext ctx, Channel channel, - ChannelBuffer buffer, int totalLength, int[] recordLengths, int nRecords) throws SSLException { - - final ByteBuffer inNetBuf = buffer.toByteBuffer(buffer.readerIndex(), totalLength); - ChannelBuffer frame = null; - - for (int i = 0; i < nRecords; i ++) { - inNetBuf.limit(inNetBuf.position() + recordLengths[i]); - frame = unwrapSingle(ctx, channel, buffer, inNetBuf, frame, totalLength); - if (engine.isInboundDone()) { - break; - } - assert !inNetBuf.hasRemaining(); - } - - return frame; - } - - /** - * Unwraps a single SSL record. - */ - private ChannelBuffer unwrapSingle( + private ChannelBuffer unwrap( ChannelHandlerContext ctx, Channel channel, ChannelBuffer nettyInNetBuf, ByteBuffer nioInNetBuf, - ChannelBuffer nettyOutAppBuf, int initialNettyOutAppBufCapacity) throws SSLException { + int initialNettyOutAppBufCapacity) throws SSLException { final int nettyInNetBufStartOffset = nettyInNetBuf.readerIndex(); final int nioInNetBufStartOffset = nioInNetBuf.position(); final ByteBuffer nioOutAppBuf = bufferPool.acquireBuffer(); + ChannelBuffer nettyOutAppBuf = null; + try { boolean needsWrap = false; for (;;) {