Commit 12f0c056 authored by Jun Li's avatar Jun Li Committed by Alecs King

Make MIN_WRITE_SIZE configurable for AsyncSSLSocket.

Summary:
MIN_WRITE_SIZE is used to avoid small write calls to SSL_write. If there are
consecutive small buffers to write, then will be combined together(by being
copied to a local buffer) until total size exceeds MIN_WRITE_SIZE. This reduces
number of calls SSL_write, improving performance, and avoiding overhead in OpenSSL.

Currently, MIN_WRITE_SIZE is hard coded to be 1500 bytes.

Wormhole could benefit from this, as our average message size is several
hundreds of bytes. We could get even better throughput with larger
MIN_WRITE_SIZE.

As discussed with Adam and Alan, there is a good reason to make it
configurable, though default value is still 1500.

Test Plan: unit tests

Reviewed By: simpkins@fb.com

Subscribers: net-systems@, ssl-diffs@, folly-diffs@, yfeldblum, chalfant, thomasf

FB internal diff: D1996570

Tasks: 6784543

Signature: t1:1996570:1429667035:a661ef30a715dafec3e134a7f6af6f56ada2e8e0
parent 6546b7b9
......@@ -56,9 +56,6 @@ using folly::AsyncSocketException;
using folly::AsyncSSLSocket;
using folly::Optional;
/** Try to avoid calling SSL_write() for buffers smaller than this: */
size_t MIN_WRITE_SIZE = 1500;
// We have one single dummy SSL context so that we can implement attach
// and detach methods in a thread safe fashion without modifying opnessl.
static SSLContext *dummyCtx = nullptr;
......@@ -68,6 +65,10 @@ static SpinLock dummyCtxLock;
const uint8_t TASYNCSSLSOCKET_F_PERFORM_READ = 90;
const uint8_t TASYNCSSLSOCKET_F_PERFORM_WRITE = 91;
// If given min write size is less than this, buffer will be allocated on
// stack, otherwise it is allocated on heap
const size_t MAX_STACK_BUF_SIZE = 2048;
// This converts "illegal" shutdowns into ZERO_RETURN
inline bool zero_return(int error, int rc) {
return (error == SSL_ERROR_ZERO_RETURN || (rc == 0 && errno == 0));
......@@ -1174,6 +1175,17 @@ ssize_t AsyncSSLSocket::performWrite(const iovec* vec,
bool cork = isSet(flags, WriteFlags::CORK);
CorkGuard guard(fd_, count > 1, cork, &corked_);
// Declare a buffer used to hold small write requests. It could point to a
// memory block either on stack or on heap. If it is on heap, we release it
// manually when scope exits
char* combinedBuf{nullptr};
SCOPE_EXIT {
// Note, always keep this check consistent with what we do below
if (combinedBuf != nullptr && minWriteSize_ > MAX_STACK_BUF_SIZE) {
delete[] combinedBuf;
}
};
*countWritten = 0;
*partialWritten = 0;
ssize_t totalWritten = 0;
......@@ -1193,7 +1205,7 @@ ssize_t AsyncSSLSocket::performWrite(const iovec* vec,
ssize_t bytes;
errno = 0;
uint32_t buffersStolen = 0;
if ((len < MIN_WRITE_SIZE) && ((i + 1) < count)) {
if ((len < minWriteSize_) && ((i + 1) < count)) {
// Combine this buffer with part or all of the next buffers in
// order to avoid really small-grained calls to SSL_write().
// Each call to SSL_write() produces a separate record in
......@@ -1202,13 +1214,24 @@ ssize_t AsyncSSLSocket::performWrite(const iovec* vec,
// header and the first part of the response body in two
// separate SSL records (even if those two records are in
// the same TCP packet).
char combinedBuf[MIN_WRITE_SIZE];
if (combinedBuf == nullptr) {
if (minWriteSize_ > MAX_STACK_BUF_SIZE) {
// Allocate the buffer on heap
combinedBuf = new char[minWriteSize_];
} else {
// Allocate the buffer on stack
combinedBuf = (char*)alloca(minWriteSize_);
}
}
assert(combinedBuf != nullptr);
memcpy(combinedBuf, buf, len);
do {
// INVARIANT: i + buffersStolen == complete chunks serialized
uint32_t nextIndex = i + buffersStolen + 1;
bytesStolenFromNextBuffer = std::min(vec[nextIndex].iov_len,
MIN_WRITE_SIZE - len);
minWriteSize_ - len);
memcpy(combinedBuf + len, vec[nextIndex].iov_base,
bytesStolenFromNextBuffer);
len += bytesStolenFromNextBuffer;
......@@ -1219,7 +1242,7 @@ ssize_t AsyncSSLSocket::performWrite(const iovec* vec,
bytesStolenFromNextBuffer = 0;
buffersStolen++;
}
} while ((i + buffersStolen + 1) < count && (len < MIN_WRITE_SIZE));
} while ((i + buffersStolen + 1) < count && (len < minWriteSize_));
bytes = eorAwareSSLWrite(
ssl_, combinedBuf, len,
(isSet(flags, WriteFlags::EOR) && i + buffersStolen + 1 == count));
......
......@@ -640,6 +640,14 @@ class AsyncSSLSocket : public virtual AsyncSocket {
return clientHelloInfo_.get();
}
void setMinWriteSize(size_t minWriteSize) {
minWriteSize_ = minWriteSize;
}
size_t getMinWriteSize() {
return minWriteSize_;
}
protected:
/**
......@@ -730,6 +738,10 @@ class AsyncSSLSocket : public virtual AsyncSocket {
// Only one app EOR byte can be tracked.
size_t appEorByteNo_{0};
// Try to avoid calling SSL_write() for buffers smaller than this.
// It doesn't take effect when it is 0.
size_t minWriteSize_{1500};
// When openssl is about to sendmsg() across the minEorRawBytesNo_,
// it will pass MSG_EOR to sendmsg().
size_t minEorRawByteNo_{0};
......
......@@ -1206,6 +1206,24 @@ TEST(AsyncSSLSocketTest, NoClientCertHandshakeError) {
EXPECT_FALSE(server.handshakeSuccess_);
EXPECT_TRUE(server.handshakeError_);
}
TEST(AsyncSSLSocketTest, MinWriteSizeTest) {
EventBase eb;
// Set up SSL context.
auto sslContext = std::make_shared<SSLContext>();
sslContext->ciphers("ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH");
// create SSL socket
AsyncSSLSocket::UniquePtr socket(new AsyncSSLSocket(sslContext, &eb));
EXPECT_EQ(1500, socket->getMinWriteSize());
socket->setMinWriteSize(0);
EXPECT_EQ(0, socket->getMinWriteSize());
socket->setMinWriteSize(50000);
EXPECT_EQ(50000, socket->getMinWriteSize());
}
}
///////////////////////////////////////////////////////////////////////////
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment