Copilot commented on code in PR #13272:
URL: https://github.com/apache/trafficserver/pull/13272#discussion_r3416329621
##########
src/iocore/net/SSLNetVConnection.cc:
##########
@@ -991,6 +991,9 @@ SSLNetVConnection::clear()
client_sess.reset();
if (ssl != nullptr) {
+ // One SSL_free per VC that owned an SSL object -- the causal driver for
ssl:close
+ // (cert-chain X509 free dominates the teardown).
+ Metrics::Counter::increment(ssl_rsb.connections_closed);
Review Comment:
`connections_closed` is intended to be “one per SSL_free”, but the counter
is only incremented in `clear()`. There are other `SSL_free()` call sites in
this same file (e.g. tunnel/transparent paths) that will not be counted, so the
metric will under-report closes and the comment is misleading.
##########
src/iocore/net/TLSBasicSupport.cc:
##########
@@ -319,6 +319,34 @@ TLSBasicSupport::_update_end_of_handshake_stats()
{
Metrics::Counter::increment(ssl_rsb.total_success_handshake_count_in);
+ // Split inbound handshakes into full vs. resumed, and for full handshakes
+ // record the server private-key type. A full handshake runs an asymmetric
+ // signature (RSA being far heavier than ECDSA), while a resumed handshake
+ // skips it -- these are the causal workload drivers behind ssl:accept CPU.
+ {
+ SSL *ssl = this->_get_ssl_object();
+ if (ssl != nullptr) {
+ if (SSL_session_reused(ssl)) {
+
Metrics::Counter::increment(ssl_rsb.total_success_handshake_count_in_resumed);
+ } else {
+
Metrics::Counter::increment(ssl_rsb.total_success_handshake_count_in_full);
+ if (EVP_PKEY *pkey = SSL_get_privatekey(ssl); pkey != nullptr) {
+ switch (EVP_PKEY_id(pkey)) {
+ case EVP_PKEY_RSA:
+ Metrics::Counter::increment(ssl_rsb.handshake_sign_rsa);
+ break;
+ case EVP_PKEY_EC:
+ Metrics::Counter::increment(ssl_rsb.handshake_sign_ecdsa);
+ break;
+ default:
+ Metrics::Counter::increment(ssl_rsb.handshake_sign_other);
+ break;
+ }
+ }
Review Comment:
`handshake_sign_rsa` will miss RSA-PSS keys: `EVP_PKEY_RSA_PSS` will
currently fall into the default branch and be counted as `_other`, even though
it has RSA-like signing cost. Consider treating RSA-PSS as RSA (guarded for
older OpenSSL/BoringSSL builds).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
