You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "bneradt (via GitHub)" <gi...@apache.org> on 2024/07/18 20:05:19 UTC
[PR] Dispatch READ_COMPLETE when TLS handshake complete [trafficserver]
bneradt opened a new pull request, #11574:
URL: https://github.com/apache/trafficserver/pull/11574
The SSLNetVConnection.cc logic already does this, so this is a port over of that event dispatch functionality when ntodo is 0 for UnixNetVConnection.cc. @josiahwi and I investigated a periodic failure in proxy_protocol.test.py and we noticed that the issue was due to the handshake completing within a single call of sslStartHandshake. In those cases, the ConnectingEntry handler wasn't called back and the connection would hang without progressing with the rest of the request after the handshake completed. This ensures that the ConnectingEntry handler is called in these circumstances.
---
# Review Comment
For reference, here is the corresponding dispatch from SSLNetVConnection that does this callback as well:
https://github.com/apache/trafficserver/blob/5f29d55f3cff865f2178d597199d72eb747ac80b/src/iocore/net/SSLNetVConnection.cc#L690-L694
This patch simply pulls in that callback for UnixNetVConnection's TLS handshake handling logic.
--
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
Re: [PR] Dispatch READ_COMPLETE when TLS handshake complete [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #11574:
URL: https://github.com/apache/trafficserver/pull/11574#discussion_r1683542666
##########
src/iocore/net/UnixNetVConnection.cc:
##########
@@ -418,6 +418,12 @@ write_to_net_io(NetHandler *nh, UnixNetVConnection *vc, EThread *thread)
if (vc->write.enabled) {
nh->write_ready_list.in_or_enqueue(vc);
}
+ // If this was driven by a zero length read, signal complete when
+ // the handshake is complete. Otherwise set up for continuing read
+ // operations.
+ if (s->vio.ntodo() <= 0) {
+ vc->readSignalDone(VC_EVENT_READ_COMPLETE, nh);
Review Comment:
Great catch! I copied from the read operation of the SSLNetVConnection, but this is for the `do_io_write`. Let me try out the patch with WRITE_COMPLETE and verify that addresses the failure as well. It almost certainly will since the ConnectingEntry handles both the read and write complete events in the same switch case block.
--
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
Re: [PR] Dispatch WRITE_COMPLETE when TLS handshake completes [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt merged PR #11574:
URL: https://github.com/apache/trafficserver/pull/11574
--
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
Re: [PR] Dispatch WRITE_COMPLETE when TLS handshake completes [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on PR #11574:
URL: https://github.com/apache/trafficserver/pull/11574#issuecomment-2241796172
Cherry-picked to v10.0.x
--
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
Re: [PR] Dispatch READ_COMPLETE when TLS handshake complete [trafficserver]
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on code in PR #11574:
URL: https://github.com/apache/trafficserver/pull/11574#discussion_r1683525070
##########
src/iocore/net/UnixNetVConnection.cc:
##########
@@ -418,6 +418,12 @@ write_to_net_io(NetHandler *nh, UnixNetVConnection *vc, EThread *thread)
if (vc->write.enabled) {
nh->write_ready_list.in_or_enqueue(vc);
}
+ // If this was driven by a zero length read, signal complete when
+ // the handshake is complete. Otherwise set up for continuing read
+ // operations.
+ if (s->vio.ntodo() <= 0) {
+ vc->readSignalDone(VC_EVENT_READ_COMPLETE, nh);
Review Comment:
`VC_EVENT_READ_COMPLETE`? Not `VC_EVENT_WRITE_COMPLETE`?
--
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
Re: [PR] Dispatch READ_COMPLETE when TLS handshake complete [trafficserver]
Posted by "JosiahWI (via GitHub)" <gi...@apache.org>.
JosiahWI commented on PR #11574:
URL: https://github.com/apache/trafficserver/pull/11574#issuecomment-2237643535
Fixes #11552.
--
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