Hi Ilya,
On Sat, Feb 01, 2014 at 11:33:50AM -0800, Ilya Grigorik wrote:
> Hi Eric.
>
> 0001-MINOR-ssl-handshake-optimz-for-long-certificate-chai: works great!
> After applying this patch the full cert is sent in one RTT and without any
> extra pauses. [1]
Cool, I'm impressed to see that the SSL time has been divided by 3! Great
suggestion from you on this, thank you! I'll merge this one now.
> 0002-MINOR-ssl-Set-openssl-max_send_fragment-using-tune.s: I'm testing with
> / against openssl 1.0.1e, and it seems to work. Looking at the tcpdump, the
> packets look identical to previous runs without this patch. [2]
>
> Any thoughts on dynamic sizing? ;)
OK I've implemented it and tested it with good success. I'm seeing several
small packets at the beginning, then large ones.
However in order to do this I am not using Emeric's 0002 patch, because we
certainly don't want to change the fragment size from the SSL stack upon
every ssl_write() in dynamic mode, so I'm back to the initial principle of
just moderating the buffer size. By using tune.ssl.maxrecord 2859, I'm
seeing a few series of two segments of 1448 and 1437 bytes respectively,
then larger ones up to 14-15kB that are coalesced by TSO on the NIC.
It seems to do what we want :-)
I'm attaching the patches if you're interested in trying it. However you'll
have to revert patch 0002.
Thanks for your tests and suggestions!
Willy
>From 64a67a3acbea1c7e82f0736502f3b8114dba6a07 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w...@1wt.eu>
Date: Sun, 2 Feb 2014 01:44:13 +0100
Subject: BUG/MINOR: raw_sock: correctly set the MSG_MORE flag
Due to a typo, the MSG_MORE flag used to replace MSG_NOSIGNAL and
MSG_DONTWAIT. Fortunately, sockets are always marked non-blocking,
so the loss of MSG_DONTWAIT is harmless, and the NOSIGNAL is covered
by the interception of the SIGPIPE. So no issue could have been
caused by this bug.
---
src/raw_sock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/raw_sock.c b/src/raw_sock.c
index fda7de1..4b125ce 100644
--- a/src/raw_sock.c
+++ b/src/raw_sock.c
@@ -373,7 +373,7 @@ static int raw_sock_from_buf(struct connection *conn,
struct buffer *buf, int fl
send_flag = MSG_DONTWAIT | MSG_NOSIGNAL;
if (try < buf->o)
- send_flag = MSG_MORE;
+ send_flag |= MSG_MORE;
ret = send(conn->t.sock.fd, bo_ptr(buf), try, send_flag |
flags);
--
1.7.12.2.21.g234cd45.dirty
>From 92e526d5101dbe73f29ba6610a2a7092452277d1 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w...@1wt.eu>
Date: Sun, 2 Feb 2014 01:51:17 +0100
Subject: MEDIUM: connection: don't use real send() flags in snd_buf()
This prevents us from passing other useful info and requires the
upper levels to know these flags. Let's use a new flags category
instead : CO_SFL_*. For now, only MSG_MORE has been remapped.
---
include/types/connection.h | 4 ++++
src/checks.c | 4 ++--
src/raw_sock.c | 8 ++++----
src/ssl_sock.c | 4 ++--
src/stream_interface.c | 4 ++--
5 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/include/types/connection.h b/include/types/connection.h
index eb9307f..f8e526a 100644
--- a/include/types/connection.h
+++ b/include/types/connection.h
@@ -177,6 +177,10 @@ enum {
CO_SRC_BIND = 0x0008, /* bind to a specific source address
when connecting */
};
+/* flags that can be passed to xprt->snd_buf() */
+enum {
+ CO_SFL_MSG_MORE = 0x0001, /* More data to come afterwards */
+};
/* xprt_ops describes transport-layer operations for a connection. They
* generally run over a socket-based control layer, but not always. Some
diff --git a/src/checks.c b/src/checks.c
index 70b5a2e..a5273e7 100644
--- a/src/checks.c
+++ b/src/checks.c
@@ -962,7 +962,7 @@ static void event_srv_chk_w(struct connection *conn)
}
if (check->bo->o) {
- conn->xprt->snd_buf(conn, check->bo, MSG_DONTWAIT |
MSG_NOSIGNAL);
+ conn->xprt->snd_buf(conn, check->bo, 0);
if (conn->flags & CO_FL_ERROR) {
chk_report_conn_err(conn, errno, 0);
__conn_data_stop_both(conn);
@@ -2006,7 +2006,7 @@ static void tcpcheck_main(struct connection *conn)
check->current_step->action != TCPCHK_ACT_SEND ||
check->current_step->string_len >=
buffer_total_space(check->bo))) {
- if (conn->xprt->snd_buf(conn, check->bo, MSG_DONTWAIT |
MSG_NOSIGNAL) <= 0) {
+ if (conn->xprt->snd_buf(conn, check->bo, 0) <= 0) {
if (conn->flags & CO_FL_ERROR) {
chk_report_conn_err(conn, errno, 0);
__conn_data_stop_both(conn);
diff --git a/src/raw_sock.c b/src/raw_sock.c
index 4b125ce..3708b19 100644
--- a/src/raw_sock.c
+++ b/src/raw_sock.c
@@ -341,8 +341,8 @@ static int raw_sock_to_buf(struct connection *conn, struct
buffer *buf, int coun
/* Send all pending bytes from buffer <buf> to connection <conn>'s socket.
- * <flags> may contain MSG_MORE to make the system hold on without sending
- * data too fast.
+ * <flags> may contain some CO_SFL_* flags to hint the system about other
+ * pending data for example.
* Only one call to send() is performed, unless the buffer wraps, in which case
* a second call may be performed. The connection's flags are updated with
* whatever special event is detected (error, empty). The caller is responsible
@@ -372,10 +372,10 @@ static int raw_sock_from_buf(struct connection *conn,
struct buffer *buf, int fl
try = buf->data + try - buf->p;
send_flag = MSG_DONTWAIT | MSG_NOSIGNAL;
- if (try < buf->o)
+ if (try < buf->o || flags & CO_SFL_MSG_MORE)
send_flag |= MSG_MORE;
- ret = send(conn->t.sock.fd, bo_ptr(buf), try, send_flag |
flags);
+ ret = send(conn->t.sock.fd, bo_ptr(buf), try, send_flag);
if (ret > 0) {
buf->o -= ret;
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 45a6dd0..c35a180 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1482,8 +1482,8 @@ static int ssl_sock_to_buf(struct connection *conn,
struct buffer *buf, int coun
/* Send all pending bytes from buffer <buf> to connection <conn>'s socket.
- * <flags> may contain MSG_MORE to make the system hold on without sending
- * data too fast, but this flag is ignored at the moment.
+ * <flags> may contain some CO_SFL_* flags to hint the system about other
+ * pending data for example, but this flag is ignored at the moment.
* Only one call to send() is performed, unless the buffer wraps, in which case
* a second call may be performed. The connection's flags are updated with
* whatever special event is detected (error, empty). The caller is responsible
diff --git a/src/stream_interface.c b/src/stream_interface.c
index c4d2715..967f645 100644
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -693,13 +693,13 @@ static void si_conn_send(struct connection *conn)
* The test is arranged so that the most common case does only 2
* tests.
*/
- unsigned int send_flag = MSG_DONTWAIT | MSG_NOSIGNAL;
+ unsigned int send_flag = 0;
if ((!(chn->flags & (CF_NEVER_WAIT|CF_SEND_DONTWAIT)) &&
((chn->to_forward && chn->to_forward !=
CHN_INFINITE_FORWARD) ||
(chn->flags & CF_EXPECT_MORE))) ||
((chn->flags & (CF_SHUTW|CF_SHUTW_NOW)) == CF_SHUTW_NOW))
- send_flag |= MSG_MORE;
+ send_flag |= CO_SFL_MSG_MORE;
ret = conn->xprt->snd_buf(conn, chn->buf, send_flag);
if (ret > 0) {
--
1.7.12.2.21.g234cd45.dirty
>From f64e6058654f87b762bce1bc46b326cd2dbcdde0 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w...@1wt.eu>
Date: Sun, 2 Feb 2014 02:00:24 +0100
Subject: OPTIM: ssl: implement dynamic record size adjustment
By having the stream interface pass the CF_STREAMER flag to the
snd_buf() primitive, we're able to tell the send layer whether
we're sending large chunks or small ones.
We use this information in SSL to adjust the max record dynamically.
This results in small chunks respecting tune.ssl.maxrecord at the
beginning of a transfer or for small transfers, with an automatic
switch to full records if the exchanges last long. This allows the
receiver to parse HTML contents on the fly without having to retrieve
16kB of data, which is even more important with small initcwnd since
the receiver does not need to wait for round trips to start fetching
new objects. However, sending large files still produces large chunks.
For example, with tune.ssl.maxrecord = 2859, we see 5 write(2885)
sent in two segments each and 6 write(16421).
This idea was first proposed on the haproxy mailing list by Ilya Grigorik.
---
include/types/connection.h | 1 +
src/ssl_sock.c | 3 ++-
src/stream_interface.c | 3 +++
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/types/connection.h b/include/types/connection.h
index f8e526a..5341a86 100644
--- a/include/types/connection.h
+++ b/include/types/connection.h
@@ -180,6 +180,7 @@ enum {
/* flags that can be passed to xprt->snd_buf() */
enum {
CO_SFL_MSG_MORE = 0x0001, /* More data to come afterwards */
+ CO_SFL_STREAMER = 0x0002, /* Producer is continuously streaming
data */
};
/* xprt_ops describes transport-layer operations for a connection. They
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index c35a180..82ad719 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1511,7 +1511,8 @@ static int ssl_sock_from_buf(struct connection *conn,
struct buffer *buf, int fl
while (buf->o) {
try = bo_contig_data(buf);
- if (global.tune.ssl_max_record && try >
global.tune.ssl_max_record)
+ if (!(flags & CO_SFL_STREAMER) &&
+ global.tune.ssl_max_record && try >
global.tune.ssl_max_record)
try = global.tune.ssl_max_record;
ret = SSL_write(conn->xprt_ctx, bo_ptr(buf), try);
diff --git a/src/stream_interface.c b/src/stream_interface.c
index 967f645..b3364a3 100644
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -701,6 +701,9 @@ static void si_conn_send(struct connection *conn)
((chn->flags & (CF_SHUTW|CF_SHUTW_NOW)) == CF_SHUTW_NOW))
send_flag |= CO_SFL_MSG_MORE;
+ if (chn->flags & CF_STREAMER)
+ send_flag |= CO_SFL_STREAMER;
+
ret = conn->xprt->snd_buf(conn, chn->buf, send_flag);
if (ret > 0) {
chn->flags |= CF_WRITE_PARTIAL;
--
1.7.12.2.21.g234cd45.dirty
