moonchen commented on code in PR #13289:
URL: https://github.com/apache/trafficserver/pull/13289#discussion_r3444422083
##########
src/iocore/net/TLSBasicSupport.cc:
##########
@@ -319,6 +319,28 @@ TLSBasicSupport::_update_end_of_handshake_stats()
{
Metrics::Counter::increment(ssl_rsb.total_success_handshake_count_in);
+ // Only a full handshake runs the asymmetric signature (a resumed one skips
it); count those
+ // signatures by the server private-key type.
+ {
+ SSL *ssl = this->_get_ssl_object();
Review Comment:
Done for the new block. I left the other `_get_ssl_object()` locals as-is:
the group-name branch calls `SSLGetGroupName(SSL *)`, which is non-const
because the OpenSSL APIs it uses (`SSL_group_to_name`,
`SSL_get_negotiated_group`) take a non-const `SSL *` -- that's also why the
negotiated-group branch already needs a `const_cast`. Converting them all would
scatter `const_cast`s rather than remove them, so I'd rather leave those.
Marking resolved.
##########
src/iocore/net/SSLNetVConnection.cc:
##########
@@ -991,6 +991,7 @@ SSLNetVConnection::clear()
client_sess.reset();
if (ssl != nullptr) {
+ Metrics::Counter::increment(ssl_rsb.connections_closed);
Review Comment:
`clear()` is reached exactly once per VC, from `UnixNetVConnection::free()`
on recycle, so it's the single chokepoint where a connection's SSL object is
torn down. `do_io_close()` can be called more than once or skipped on abort
paths, so counting there would over/under-count. Blind-tunnel conversions free
their SSL earlier and continue as a tunnel (counted by the tunnel metrics), so
by the time `clear()` runs `ssl` is already null for them and they're excluded.
I added a comment at the increment spelling this out, since the concern is
readability for a future reader -- but happy to move it if you'd prefer.
--
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]
