From 4ab3242da1b5ae5d99ff2086df935057880ca9ff Mon Sep 17 00:00:00 2001 From: Thuan Tran Date: Mon, 15 May 2023 19:57:38 +0700 Subject: Make bufferevent_set_max_single_read() effect --- bufferevent_ssl.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'bufferevent_ssl.c') diff --git a/bufferevent_ssl.c b/bufferevent_ssl.c index 17046b82..58945e62 100644 --- a/bufferevent_ssl.c +++ b/bufferevent_ssl.c @@ -407,8 +407,6 @@ do_write(struct bufferevent_ssl *bev_ssl, int atmost) #define WRITE_FRAME 15000 -#define READ_DEFAULT 4096 - /* Try to figure out how many bytes to read; return 0 if we shouldn't be * reading. */ static int @@ -416,7 +414,7 @@ bytes_to_read(struct bufferevent_ssl *bev) { struct evbuffer *input = bev->bev.bev.input; struct event_watermark *wm = &bev->bev.bev.wm_read; - int result = READ_DEFAULT; + int result = 0; ev_ssize_t limit; /* XXX 99% of this is generic code that nearly all bufferevents will * want. */ @@ -439,13 +437,11 @@ bytes_to_read(struct bufferevent_ssl *bev) } result = wm->high - evbuffer_get_length(input); - } else { - result = READ_DEFAULT; } /* Respect the rate limit */ limit = bufferevent_get_read_max_(&bev->bev); - if (result > limit) { + if (result == 0 || result > limit) { result = limit; } -- cgit v1.2.1 From ef51444f439b922c8fb00c7ef3f8482b6400b6ea Mon Sep 17 00:00:00 2001 From: Thuan Tran Date: Mon, 15 May 2023 10:18:38 +0700 Subject: More SSL_read() to fill big buffer Once SSL_read() only get max 16K bytes (one TLS record). In case of big buffer, should more SSL_read() to fill the buffer. Using sample https-client to measure max income MBit/s via nload tool. Note: set bufferevent_set_max_single_read() by 32K and add the chunk callback to read out each piece of data. The client sample do https request a data 900KB (the server don't use Transfer-Encoding: chunked) - With origin/master: max income is 2.26 MBit/s The chunk callback never get a piece of data > 16K. - With this PR: max income is 2.44 MBit/s The chunk callback can get some piece of data 32K or more. --- bufferevent_ssl.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) (limited to 'bufferevent_ssl.c') diff --git a/bufferevent_ssl.c b/bufferevent_ssl.c index 58945e62..c74fd283 100644 --- a/bufferevent_ssl.c +++ b/bufferevent_ssl.c @@ -253,9 +253,10 @@ do_read(struct bufferevent_ssl *bev_ssl, int n_to_read) { /* Requires lock */ struct bufferevent *bev = &bev_ssl->bev.bev; struct evbuffer *input = bev->input; - int r, n, i, n_used = 0, atmost; + int r, n, i = 0, atmost; struct evbuffer_iovec space[2]; int result = 0; + size_t len = 0; if (bev_ssl->bev.read_suspended) return 0; @@ -268,23 +269,30 @@ do_read(struct bufferevent_ssl *bev_ssl, int n_to_read) { if (n < 0) return OP_ERR; - for (i=0; ibev.read_suspended) break; bev_ssl->ssl_ops->clear_error(); - r = bev_ssl->ssl_ops->read(bev_ssl->ssl, space[i].iov_base, space[i].iov_len); - if (r>0) { + r = bev_ssl->ssl_ops->read( + bev_ssl->ssl, (unsigned char *)space[i].iov_base + len, space[i].iov_len - len); + if (r > 0) { result |= OP_MADE_PROGRESS; if (bev_ssl->read_blocked_on_write) if (clear_rbow(bev_ssl) < 0) return OP_ERR | result; - ++n_used; - space[i].iov_len = r; bev_ssl->ssl_ops->decrement_buckets(bev_ssl); + len += r; + if (space[i].iov_len - len > 0) { + continue; + } else { + space[i].iov_len = len; + len = 0; + ++i; + } } else { int err = bev_ssl->ssl_ops->get_error(bev_ssl->ssl, r); bev_ssl->ssl_ops->print_err(err); - if (bev_ssl->ssl_ops->err_is_ok(err) && result & OP_MADE_PROGRESS) { + if (result & OP_MADE_PROGRESS) { /* Process existing data */ break; } else if (bev_ssl->ssl_ops->err_is_want_read(err)) { @@ -292,7 +300,7 @@ do_read(struct bufferevent_ssl *bev_ssl, int n_to_read) { if (bev_ssl->read_blocked_on_write) if (clear_rbow(bev_ssl) < 0) return OP_ERR | result; - } else if(bev_ssl->ssl_ops->err_is_want_write(err)) { + } else if (bev_ssl->ssl_ops->err_is_want_write(err)) { /* This read operation requires a write, and the * underlying is full */ if (!bev_ssl->read_blocked_on_write) @@ -306,8 +314,13 @@ do_read(struct bufferevent_ssl *bev_ssl, int n_to_read) { } } - if (n_used) { - evbuffer_commit_space(input, space, n_used); + if (len > 0) { + space[i].iov_len = len; + ++i; + } + + if (i) { + evbuffer_commit_space(input, space, i); if (bev_ssl->underlying) BEV_RESET_GENERIC_READ_TIMEOUT(bev); } -- cgit v1.2.1 From 49a7ae4c5668ebad7b6c4618664224c1a0bc5079 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Tue, 16 May 2023 21:14:34 +0200 Subject: ssl: add some comments for lack of notify_close --- bufferevent_ssl.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'bufferevent_ssl.c') diff --git a/bufferevent_ssl.c b/bufferevent_ssl.c index c74fd283..1f1b4ed3 100644 --- a/bufferevent_ssl.c +++ b/bufferevent_ssl.c @@ -292,6 +292,13 @@ do_read(struct bufferevent_ssl *bev_ssl, int n_to_read) { } else { int err = bev_ssl->ssl_ops->get_error(bev_ssl->ssl, r); bev_ssl->ssl_ops->print_err(err); + /* NOTE: we ignore the error in case of some progress was done, + * because currently we do not send close_notify, and this will + * lead to error from SSL_read() (it will return 0, and + * SSL_get_error() will return SSL_ERROR_SSL), and this is because + * of lack of close_notify + * + * But AFAICS some code uses it the same way (i.e. nginx) */ if (result & OP_MADE_PROGRESS) { /* Process existing data */ break; @@ -922,6 +929,10 @@ be_ssl_destruct(struct bufferevent *bev) if (bev_ssl->bev.options & BEV_OPT_CLOSE_ON_FREE) { if (! bev_ssl->underlying) { evutil_socket_t fd = bev_ssl->ssl_ops->get_fd(bev_ssl); + /* NOTE: This is dirty shutdown, to send close_notify one of the + * following should be used: + * - SSL_shutdown() + * - mbedtls_ssl_close_notify() */ if (fd >= 0) evutil_closesocket(fd); } -- cgit v1.2.1