mod_ssl's output buffering has been bothering me for a while.
1) it buffers the encrypted output stream (to some extent) coupled with
regular use of FLUSH buckets. This seems redundant/inefficient; the
core output filter should be doing this kind of thing optimally already.
2) it does /not/ do any buffering of the plaintext input brigades. This
leads to some pathological inefficiency as discussed here in the past:
http://marc.info/?l=apache-httpd-devm=125070085316400w=2
The one justification I know for (1) is in the ssl_engine_io.c comment:
/* the first two SSL_writes (of 1024 and 261 bytes)
* need to be in the same packet (vec[0].iov_base)
*/
but I see no evidence this will not happen if the core output filter is
allowed to operate as normal. Maybe that was not true in the psat.
So I'd propose to remove (1), which is simple and add (2), which is not
so simple. I've hacked up (yet another) coalescing filter for (2)
which works to coalesce chunked brigades as expected; proof of concept
attached. Would welcome some discussion before I work on this
further...
Regards, Joe
Index: ssl_engine_io.c
===
--- ssl_engine_io.c (revision 1031534)
+++ ssl_engine_io.c (working copy)
@@ -102,7 +102,6 @@
BIO*pbioWrite;
ap_filter_t*pInputFilter;
ap_filter_t*pOutputFilter;
-intnobuffer; /* non-zero to prevent buffering */
SSLConnRec *config;
} ssl_filter_ctx_t;
@@ -110,9 +109,6 @@
ssl_filter_ctx_t *filter_ctx;
conn_rec *c;
apr_bucket_brigade *bb;/* Brigade used as a buffer. */
-apr_size_t length; /* Number of bytes stored in -bb brigade. */
-char buffer[AP_IOBUFSIZE]; /* Fixed-size buffer */
-apr_size_t blen; /* Number of bytes of -buffer used. */
apr_status_t rc;
} bio_filter_out_ctx_t;
@@ -124,36 +120,13 @@
outctx-filter_ctx = filter_ctx;
outctx-c = c;
outctx-bb = apr_brigade_create(c-pool, c-bucket_alloc);
-outctx-blen = 0;
-outctx-length = 0;
return outctx;
}
-static int bio_filter_out_flush(BIO *bio)
+/* Pass the output filter brigade up the filter stack. */
+static int bio_filter_out_pass(bio_filter_out_ctx_t *outctx)
{
-bio_filter_out_ctx_t *outctx = (bio_filter_out_ctx_t *)(bio-ptr);
-apr_bucket *e;
-
-if (!(outctx-blen || outctx-length)) {
-outctx-rc = APR_SUCCESS;
-return 1;
-}
-
-if (outctx-blen) {
-e = apr_bucket_transient_create(outctx-buffer, outctx-blen,
-outctx-bb-bucket_alloc);
-/* we filled this buffer first so add it to the
- * head of the brigade
- */
-APR_BRIGADE_INSERT_HEAD(outctx-bb, e);
-outctx-blen = 0;
-}
-
-outctx-length = 0;
-e = apr_bucket_flush_create(outctx-bb-bucket_alloc);
-APR_BRIGADE_INSERT_TAIL(outctx-bb, e);
-
outctx-rc = ap_pass_brigade(outctx-filter_ctx-pOutputFilter-next,
outctx-bb);
/* Fail if the connection was reset: */
@@ -163,6 +136,17 @@
return (outctx-rc == APR_SUCCESS) ? 1 : -1;
}
+static int bio_filter_out_flush(BIO *bio)
+{
+bio_filter_out_ctx_t *outctx = (bio_filter_out_ctx_t *)(bio-ptr);
+apr_bucket *e;
+
+e = apr_bucket_flush_create(outctx-bb-bucket_alloc);
+APR_BRIGADE_INSERT_TAIL(outctx-bb, e);
+
+return bio_filter_out_pass(outctx);
+}
+
static int bio_filter_create(BIO *bio)
{
bio-shutdown = 1;
@@ -194,7 +178,8 @@
static int bio_filter_out_write(BIO *bio, const char *in, int inl)
{
bio_filter_out_ctx_t *outctx = (bio_filter_out_ctx_t *)(bio-ptr);
-
+apr_bucket *e;
+
/* Abort early if the client has initiated a renegotiation. */
if (outctx-filter_ctx-config-reneg_state == RENEG_ABORT) {
outctx-rc = APR_ECONNABORTED;
@@ -207,77 +192,48 @@
*/
BIO_clear_retry_flags(bio);
-if (!outctx-length (inl + outctx-blen sizeof(outctx-buffer))
-!outctx-filter_ctx-nobuffer) {
-/* the first two SSL_writes (of 1024 and 261 bytes)
- * need to be in the same packet (vec[0].iov_base)
- */
-/* XXX: could use apr_brigade_write() to make code look cleaner
- * but this way we avoid the malloc(APR_BUCKET_BUFF_SIZE)
- * and free() of it later
- */
-memcpy(outctx-buffer[outctx-blen], in, inl);
-outctx-blen += inl;
+/* pass along the encrypted data
+ * need to flush since we're using SSL's malloc-ed buffer
+ * which will be overwritten once we leave here
+ */
+e = apr_bucket_transient_create(in, inl, outctx-bb-bucket_alloc);
+APR_BRIGADE_INSERT_TAIL(outctx-bb, e);
+
+if (bio_filter_out_pass(outctx) 0) {
+return -1;
}
-else {
-/* pass along the encrypted data
- * need to flush since we're using SSL's
