You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "mattyw (via GitHub)" <gi...@apache.org> on 2024/08/13 07:51:21 UTC
[PR] Fatal error if no udp threads but using quic [trafficserver]
mattyw opened a new pull request, #11673:
URL: https://github.com/apache/trafficserver/pull/11673
If proxy.config.http.server_ports contains an entry to allow quic connections on a given port but there no udp threads ATS used to silently ignore these connections.
Given that this configuration isn't valid we raise a fatal error.
--
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] Fatal error if no udp threads but using quic [trafficserver]
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on code in PR #11673:
URL: https://github.com/apache/trafficserver/pull/11673#discussion_r1716367712
##########
src/traffic_server/traffic_server.cc:
##########
@@ -2215,6 +2215,21 @@ main(int /* argc ATS_UNUSED */, const char **argv)
REC_ReadConfigInteger(num_of_udp_threads, "proxy.config.udp.threads");
}
+ ats_scoped_str server_ports(255);
+ *server_ports = '\0';
+ REC_ReadConfigString(server_ports, "proxy.config.http.server_ports", 255);
+ bool quic_server_port = strstr(server_ports, "/quic/") != server_ports;
+
+#if TS_USE_QUIC == 0
+ if (quic_server_port) {
+ Fatal("proxy.config.http.server_ports listening for quic but traffic_server wasn't built with quic support.");
+ }
+#endif
+
+ if (quic_server_port && num_of_udp_threads == 0) {
+ Fatal("proxy.config.http.server_ports listening for quic but proxy.config.udp.threads is 0.");
Review Comment:
How you catch misconfiguration and how ATS behave while configuration may be wrong are different things. You could watch for a warning to catch misconfiguration, or you could also watch for the UDP ports. Ideally you should periodically send health check requests. Look at the code I mentioned above. You can try to enable mptcp where mptcp is unavailable on the host and that does not stop ATS. Lack of QUIC does not threaten the safety. QUIC can be unavailable even with proper configuration if middle boxes block it. I still think Fatal is too much.
--
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] Fatal error if no udp threads but using quic [trafficserver]
Posted by "mattyw (via GitHub)" <gi...@apache.org>.
mattyw commented on code in PR #11673:
URL: https://github.com/apache/trafficserver/pull/11673#discussion_r1716246995
##########
src/traffic_server/traffic_server.cc:
##########
@@ -2215,6 +2215,21 @@ main(int /* argc ATS_UNUSED */, const char **argv)
REC_ReadConfigInteger(num_of_udp_threads, "proxy.config.udp.threads");
}
+ ats_scoped_str server_ports(255);
+ *server_ports = '\0';
+ REC_ReadConfigString(server_ports, "proxy.config.http.server_ports", 255);
+ bool quic_server_port = strstr(server_ports, "/quic/") != server_ports;
+
+#if TS_USE_QUIC == 0
+ if (quic_server_port) {
+ Fatal("proxy.config.http.server_ports listening for quic but traffic_server wasn't built with quic support.");
+ }
+#endif
+
+ if (quic_server_port && num_of_udp_threads == 0) {
+ Fatal("proxy.config.http.server_ports listening for quic but proxy.config.udp.threads is 0.");
Review Comment:
@maskit I think you might be right w.r.t warning - I bounced between fatal and warning a few times before proposing.
> but QUIC being unavailable is not the end of the world.
Well, QUIC being unavailable when you've specifically asked ATS to serve quic could be considered to be the end of the world - it would certainly be surprising enough that I'd think we'd want to catch it sooner rather than later
--
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] Fatal error if no udp threads but using quic [trafficserver]
Posted by "mattyw (via GitHub)" <gi...@apache.org>.
mattyw commented on code in PR #11673:
URL: https://github.com/apache/trafficserver/pull/11673#discussion_r1714839788
##########
src/traffic_server/traffic_server.cc:
##########
@@ -2215,6 +2215,15 @@ main(int /* argc ATS_UNUSED */, const char **argv)
REC_ReadConfigInteger(num_of_udp_threads, "proxy.config.udp.threads");
}
+ ats_scoped_str server_ports(255);
Review Comment:
255 chosen arbitrarily - would appreciate guidance of a better number
--
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] Fatal error if no udp threads but using quic [trafficserver]
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on code in PR #11673:
URL: https://github.com/apache/trafficserver/pull/11673#discussion_r1715626770
##########
src/traffic_server/traffic_server.cc:
##########
@@ -2215,6 +2215,21 @@ main(int /* argc ATS_UNUSED */, const char **argv)
REC_ReadConfigInteger(num_of_udp_threads, "proxy.config.udp.threads");
}
+ ats_scoped_str server_ports(255);
+ *server_ports = '\0';
+ REC_ReadConfigString(server_ports, "proxy.config.http.server_ports", 255);
+ bool quic_server_port = strstr(server_ports, "/quic/") != server_ports;
+
+#if TS_USE_QUIC == 0
+ if (quic_server_port) {
+ Fatal("proxy.config.http.server_ports listening for quic but traffic_server wasn't built with quic support.");
Review Comment:
This check could be done as a part of this block, if we want to have a specialized message for QUIC (the block already has code for unrecognized options).
https://github.com/apache/trafficserver/blob/fc5918e5a8bb1eb88b98689c463c3574a64e8bef/src/records/RecHttp.cc#L367-L463
--
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] Fatal error if no udp threads but using quic [trafficserver]
Posted by "traeak (via GitHub)" <gi...@apache.org>.
traeak commented on PR #11673:
URL: https://github.com/apache/trafficserver/pull/11673#issuecomment-2488572345
[approve ci autest 1]
--
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] Fatal error if no udp threads but using quic [trafficserver]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11673:
URL: https://github.com/apache/trafficserver/pull/11673#issuecomment-2481779801
This pull request has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community.
--
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] Fatal error if no udp threads but using quic [trafficserver]
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on code in PR #11673:
URL: https://github.com/apache/trafficserver/pull/11673#discussion_r1715632103
##########
src/traffic_server/traffic_server.cc:
##########
@@ -2215,6 +2215,21 @@ main(int /* argc ATS_UNUSED */, const char **argv)
REC_ReadConfigInteger(num_of_udp_threads, "proxy.config.udp.threads");
}
+ ats_scoped_str server_ports(255);
+ *server_ports = '\0';
+ REC_ReadConfigString(server_ports, "proxy.config.http.server_ports", 255);
+ bool quic_server_port = strstr(server_ports, "/quic/") != server_ports;
+
+#if TS_USE_QUIC == 0
+ if (quic_server_port) {
+ Fatal("proxy.config.http.server_ports listening for quic but traffic_server wasn't built with quic support.");
+ }
+#endif
+
+ if (quic_server_port && num_of_udp_threads == 0) {
+ Fatal("proxy.config.http.server_ports listening for quic but proxy.config.udp.threads is 0.");
Review Comment:
I'm not sure if we should use `Fatal`. Even port configuration error is just a warning. It depends on the use case for sure, but QUIC being unavailable is not the end of the world.
--
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] Fatal error if no udp threads but using quic [trafficserver]
Posted by "JosiahWI (via GitHub)" <gi...@apache.org>.
JosiahWI commented on PR #11673:
URL: https://github.com/apache/trafficserver/pull/11673#issuecomment-2286421879
[approve ci]
--
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] Fatal error if no udp threads but using quic [trafficserver]
Posted by "mattyw (via GitHub)" <gi...@apache.org>.
mattyw commented on code in PR #11673:
URL: https://github.com/apache/trafficserver/pull/11673#discussion_r1716246257
##########
src/traffic_server/traffic_server.cc:
##########
@@ -2215,6 +2215,21 @@ main(int /* argc ATS_UNUSED */, const char **argv)
REC_ReadConfigInteger(num_of_udp_threads, "proxy.config.udp.threads");
}
+ ats_scoped_str server_ports(255);
+ *server_ports = '\0';
+ REC_ReadConfigString(server_ports, "proxy.config.http.server_ports", 255);
+ bool quic_server_port = strstr(server_ports, "/quic/") != server_ports;
+
+#if TS_USE_QUIC == 0
+ if (quic_server_port) {
+ Fatal("proxy.config.http.server_ports listening for quic but traffic_server wasn't built with quic support.");
Review Comment:
@maskit Yeah good suggestion! I tried looking into the records code to see a good place to put this logic in but couldn't find a good place. I see if I can make progress with your suggestion here.
--
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] Fatal error if no udp threads but using quic [trafficserver]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #11673: Fatal error if no udp threads but using quic
URL: https://github.com/apache/trafficserver/pull/11673
--
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