git commit: [TS-3085] Large POSTs over (relatively) slower connections failing in ats5 Call ERR_get_error_line_data() via SSL_CLR_ERR_INCR_DYN_STAT to clean up the error queue after processing an SSL_
Repository: trafficserver Updated Branches: refs/heads/master 17bef772a - d12327d84 [TS-3085] Large POSTs over (relatively) slower connections failing in ats5 Call ERR_get_error_line_data() via SSL_CLR_ERR_INCR_DYN_STAT to clean up the error queue after processing an SSL_ERROR_SSL. Also, added consistent wrappers to SSL I/O functions and removed errno based looping on SSL_Write per James Peach's recommendations. Note that, this change removes the apparent logic of reattempting SSL_Write on transient error cases (such as ENOBUF), since openSSL documentation doesn't indicate that errno is set during SSL_Write. https://www.openssl.org/docs/ssl/SSL_write.html Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/d12327d8 Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/d12327d8 Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/d12327d8 Branch: refs/heads/master Commit: d12327d841d07bfe1fa772c42272fbc908a39324 Parents: 17bef77 Author: Sudheer Vinukonda sudhe...@yahoo-inc.com Authored: Wed Sep 24 13:32:56 2014 + Committer: Sudheer Vinukonda sudhe...@yahoo-inc.com Committed: Wed Sep 24 13:32:56 2014 + -- iocore/net/P_SSLUtils.h | 12 +++ iocore/net/SSLNetVConnection.cc | 69 iocore/net/SSLUtils.cc | 59 ++ 3 files changed, 93 insertions(+), 47 deletions(-) -- http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d12327d8/iocore/net/P_SSLUtils.h -- diff --git a/iocore/net/P_SSLUtils.h b/iocore/net/P_SSLUtils.h index 82c41b4..269f009 100644 --- a/iocore/net/P_SSLUtils.h +++ b/iocore/net/P_SSLUtils.h @@ -38,6 +38,8 @@ struct SSLCertLookup; class SSLNetVConnection; struct RecRawStatBlock; +typedef int ssl_error_t; + enum SSL_Stats { ssl_origin_server_expired_cert_stat, @@ -113,6 +115,12 @@ void SSLInitializeStatistics(); // Release SSL_CTX and the associated data void SSLReleaseContext(SSL_CTX* ctx); +// Wrapper functions to SSL I/O routines +ssl_error_t SSLWriteBuffer(SSL * ssl, const void * buf, size_t nbytes, size_t nwritten); +ssl_error_t SSLReadBuffer(SSL * ssl, void * buf, size_t nbytes, size_t nread); +ssl_error_t SSLAccept(SSL *ssl); +ssl_error_t SSLConnect(SSL * ssl); + // Log an SSL error. #define SSLError(fmt, ...) SSLDiagnostic(DiagsMakeLocation(), false, NULL, fmt, ##__VA_ARGS__) #define SSLErrorVC(vc,fmt, ...) SSLDiagnostic(DiagsMakeLocation(), false, vc, fmt, ##__VA_ARGS__) @@ -120,6 +128,10 @@ void SSLReleaseContext(SSL_CTX* ctx); #define SSLDebug(fmt, ...) SSLDiagnostic(DiagsMakeLocation(), true, NULL, fmt, ##__VA_ARGS__) #define SSLDebugVC(vc,fmt, ...) SSLDiagnostic(DiagsMakeLocation(), true, vc, fmt, ##__VA_ARGS__) +#define SSL_CLR_ERR_INCR_DYN_STAT(x, fmt, ...) \ + SSLDiagnostic(DiagsMakeLocation(), true, NULL, fmt, ##__VA_ARGS__); \ + RecIncrRawStat(ssl_rsb, NULL, (int) x, 1); + void SSLDiagnostic(const SrcLoc loc, bool debug, SSLNetVConnection * vc, const char * fmt, ...) TS_PRINTFLIKE(4, 5); // Return a static string name for a SSL_ERROR constant. http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d12327d8/iocore/net/SSLNetVConnection.cc -- diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc index b4269e7..0f26679 100644 --- a/iocore/net/SSLNetVConnection.cc +++ b/iocore/net/SSLNetVConnection.cc @@ -172,24 +172,6 @@ debug_certificate_name(const char * msg, X509_NAME * name) BIO_free(bio); } -static inline int -do_SSL_write(SSL * ssl, void *buf, int size) -{ - int r = 0; - do { -// need to check into SSL error handling -// to see if this is good enough. -r = SSL_write(ssl, (const char *) buf, size); -if (r = 0) - return r; -else - r = -errno; - } while (r == -EINTR || r == -ENOBUFS || r == -ENOMEM); - - return r; -} - - static int ssl_read_from_net(SSLNetVConnection * sslvc, EThread * lthread, int64_t ret) { @@ -199,7 +181,8 @@ ssl_read_from_net(SSLNetVConnection * sslvc, EThread * lthread, int64_t ret) int event = SSL_READ_ERROR_NONE; int64_t bytes_read; int64_t block_write_avail; - int sslErr = SSL_ERROR_NONE; + ssl_error_t sslErr = SSL_ERROR_NONE; + int nread = 0; for (bytes_read = 0; (b != 0) (sslErr == SSL_ERROR_NONE); b = b-next) { block_write_avail = b-write_avail(); @@ -209,23 +192,22 @@ ssl_read_from_net(SSLNetVConnection * sslvc, EThread * lthread, int64_t ret) int64_t offset = 0; // while can be replaced with if - need to test what works faster with openssl while (block_write_avail 0) { - int rres = SSL_read(sslvc-ssl, b-end() + offset,
Re: git commit: [TS-3085] Large POSTs over (relatively) slower connections failing in ats5 Call ERR_get_error_line_data() via SSL_CLR_ERR_INCR_DYN_STAT to clean up the error queue after processing an
On Sep 24, 2014, at 6:40 AM, sudhe...@apache.org wrote: Repository: trafficserver Updated Branches: refs/heads/master 17bef772a - d12327d84 [TS-3085] Large POSTs over (relatively) slower connections failing in ats5 Call ERR_get_error_line_data() via SSL_CLR_ERR_INCR_DYN_STAT to clean up the error queue after processing an SSL_ERROR_SSL. Also, added consistent wrappers to SSL I/O functions and removed errno based looping on SSL_Write per James Peach's recommendations. Note that, this change removes the apparent logic of reattempting SSL_Write on transient error cases (such as ENOBUF), since openSSL documentation doesn't indicate that errno is set during SSL_Write. https://www.openssl.org/docs/ssl/SSL_write.html Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/d12327d8 Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/d12327d8 Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/d12327d8 Branch: refs/heads/master Commit: d12327d841d07bfe1fa772c42272fbc908a39324 Parents: 17bef77 Author: Sudheer Vinukonda sudhe...@yahoo-inc.com Authored: Wed Sep 24 13:32:56 2014 + Committer: Sudheer Vinukonda sudhe...@yahoo-inc.com Committed: Wed Sep 24 13:32:56 2014 + -- iocore/net/P_SSLUtils.h | 12 +++ iocore/net/SSLNetVConnection.cc | 69 iocore/net/SSLUtils.cc | 59 ++ 3 files changed, 93 insertions(+), 47 deletions(-) -- http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d12327d8/iocore/net/P_SSLUtils.h -- diff --git a/iocore/net/P_SSLUtils.h b/iocore/net/P_SSLUtils.h index 82c41b4..269f009 100644 --- a/iocore/net/P_SSLUtils.h +++ b/iocore/net/P_SSLUtils.h @@ -38,6 +38,8 @@ struct SSLCertLookup; class SSLNetVConnection; struct RecRawStatBlock; +typedef int ssl_error_t; + enum SSL_Stats { ssl_origin_server_expired_cert_stat, @@ -113,6 +115,12 @@ void SSLInitializeStatistics(); // Release SSL_CTX and the associated data void SSLReleaseContext(SSL_CTX* ctx); +// Wrapper functions to SSL I/O routines +ssl_error_t SSLWriteBuffer(SSL * ssl, const void * buf, size_t nbytes, size_t nwritten); +ssl_error_t SSLReadBuffer(SSL * ssl, void * buf, size_t nbytes, size_t nread); +ssl_error_t SSLAccept(SSL *ssl); +ssl_error_t SSLConnect(SSL * ssl); + // Log an SSL error. #define SSLError(fmt, ...) SSLDiagnostic(DiagsMakeLocation(), false, NULL, fmt, ##__VA_ARGS__) #define SSLErrorVC(vc,fmt, ...) SSLDiagnostic(DiagsMakeLocation(), false, vc, fmt, ##__VA_ARGS__) @@ -120,6 +128,10 @@ void SSLReleaseContext(SSL_CTX* ctx); #define SSLDebug(fmt, ...) SSLDiagnostic(DiagsMakeLocation(), true, NULL, fmt, ##__VA_ARGS__) #define SSLDebugVC(vc,fmt, ...) SSLDiagnostic(DiagsMakeLocation(), true, vc, fmt, ##__VA_ARGS__) +#define SSL_CLR_ERR_INCR_DYN_STAT(x, fmt, ...) \ + SSLDiagnostic(DiagsMakeLocation(), true, NULL, fmt, ##__VA_ARGS__); \ + RecIncrRawStat(ssl_rsb, NULL, (int) x, 1); + Pleas wrap this in a do { } while(0) ... void SSLDiagnostic(const SrcLoc loc, bool debug, SSLNetVConnection * vc, const char * fmt, ...) TS_PRINTFLIKE(4, 5); // Return a static string name for a SSL_ERROR constant. http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d12327d8/iocore/net/SSLNetVConnection.cc -- diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc index b4269e7..0f26679 100644 --- a/iocore/net/SSLNetVConnection.cc +++ b/iocore/net/SSLNetVConnection.cc @@ -172,24 +172,6 @@ debug_certificate_name(const char * msg, X509_NAME * name) BIO_free(bio); } -static inline int -do_SSL_write(SSL * ssl, void *buf, int size) -{ - int r = 0; - do { -// need to check into SSL error handling -// to see if this is good enough. -r = SSL_write(ssl, (const char *) buf, size); -if (r = 0) - return r; -else - r = -errno; - } while (r == -EINTR || r == -ENOBUFS || r == -ENOMEM); - - return r; -} - - static int ssl_read_from_net(SSLNetVConnection * sslvc, EThread * lthread, int64_t ret) { @@ -199,7 +181,8 @@ ssl_read_from_net(SSLNetVConnection * sslvc, EThread * lthread, int64_t ret) int event = SSL_READ_ERROR_NONE; int64_t bytes_read; int64_t block_write_avail; - int sslErr = SSL_ERROR_NONE; + ssl_error_t sslErr = SSL_ERROR_NONE; + int nread = 0; for (bytes_read = 0; (b != 0) (sslErr == SSL_ERROR_NONE); b = b-next) { block_write_avail = b-write_avail(); @@ -209,23 +192,22 @@ ssl_read_from_net(SSLNetVConnection * sslvc, EThread * lthread, int64_t ret) int64_t