You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2023/01/19 22:13:31 UTC
[GitHub] [trafficserver] maskit opened a new pull request, #9322: Add TLSCertSwitchSupport
maskit opened a new pull request, #9322:
URL: https://github.com/apache/trafficserver/pull/9322
This is preparation for enabling cert switching (or enabling SNI action) on QUIC connections. Like other TLSSomethingSupport, there should be no logic change.
Code change around QUICNetVC will be made on 10-Dev separately.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a diff in pull request #9322: Add TLSCertSwitchSupport
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on code in PR #9322:
URL: https://github.com/apache/trafficserver/pull/9322#discussion_r1122044758
##########
iocore/net/SSLUtils.cc:
##########
@@ -418,17 +306,19 @@ ssl_client_hello_callback(SSL *s, int *al, void *arg)
return SSL_CLIENT_HELLO_ERROR;
}
- SSLNetVConnection *netvc = SSLNetVCAccess(s);
- if (!netvc || netvc->ssl != s) {
- Debug("ssl.error", "ssl_client_hello_callback call back on stale netvc");
- return SSL_CLIENT_HELLO_ERROR;
- }
-
- bool reenabled = netvc->callHooks(TS_EVENT_SSL_CLIENT_HELLO);
+ SSLNetVConnection *netvc = dynamic_cast<SSLNetVConnection *>(snis);
Review Comment:
We talked about the use of dynamic_cast several months ago. I think Alan suggested having `T* NetVConnection::get_service<T>())`. The function would return a pointer for a specified service (SomethingSupport) if the netvc supports it. The downside is that we'd have to maintain the function in addition to having the name in class declaration. And I already stated I'm ok with having the function if dynamic_cast costs much. It just don't have enough priority on anybody's list.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit merged pull request #9322: Add TLSCertSwitchSupport
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit merged PR #9322:
URL: https://github.com/apache/trafficserver/pull/9322
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on a diff in pull request #9322: Add TLSCertSwitchSupport
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on code in PR #9322:
URL: https://github.com/apache/trafficserver/pull/9322#discussion_r1121994963
##########
iocore/net/SSLUtils.cc:
##########
@@ -418,17 +306,19 @@ ssl_client_hello_callback(SSL *s, int *al, void *arg)
return SSL_CLIENT_HELLO_ERROR;
}
- SSLNetVConnection *netvc = SSLNetVCAccess(s);
- if (!netvc || netvc->ssl != s) {
- Debug("ssl.error", "ssl_client_hello_callback call back on stale netvc");
- return SSL_CLIENT_HELLO_ERROR;
- }
-
- bool reenabled = netvc->callHooks(TS_EVENT_SSL_CLIENT_HELLO);
+ SSLNetVConnection *netvc = dynamic_cast<SSLNetVConnection *>(snis);
Review Comment:
Looking over the PR - I am concerned that we are adding 3 dynamic casts. Dynamic casts seem to be expensive and have shown up in perf top.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on pull request #9322: Add TLSCertSwitchSupport
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9322:
URL: https://github.com/apache/trafficserver/pull/9322#issuecomment-1468501026
We'll continue the discussion about dynamic_cast on #9482 and resolve performance issue on it at once.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org