JosiahWI commented on code in PR #13289:
URL: https://github.com/apache/trafficserver/pull/13289#discussion_r3442866227
##########
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:
Can this be pointer to const? This is a bit of a nitpick and it's not
blocking, but the local on line 325 aliases locals in blocks below. If the
pointed-to type is const-qualified, it should be easier for a compiler to
determine that it can hoist all the `_get_ssl_object` calls and cache the
pointer in a register. It wouldn't be out of scope in my opinion to make the
same change to all instances in this method at the same time. If you don't want
to do this now, provide a justification and mark resolved; I'll re-review.
##########
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:
Can this happen at an earlier point where it would be clearer the TLS
connection is being terminated? The name of the method (`clear`) does not
suggest this codepath is only used for TLS termination, which could be
confusing to a future reader.
--
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]
