bneradt commented on code in PR #9348:
URL: https://github.com/apache/trafficserver/pull/9348#discussion_r1230118626


##########
doc/admin-guide/files/sni.yaml.en.rst:
##########
@@ -45,7 +45,7 @@ the user needs to enter the fqdn in the configuration with a 
``*.`` followed by
 For some settings, there is no guarantee that they will be applied to a 
connection under certain conditions.
 An established TLS connection may be reused for another server name if it’s 
used for HTTP/2. This also means that settings
 for server name A may affects requests for server name B as well. See 
https://daniel.haxx.se/blog/2016/08/18/http2-connection-coalescing/
-for a more detailed description of HTTP/2 connection coalescing.
+for a more detailed description of HTTP/2 connection coalescing. Similar thing 
can happen on a QUIC connection for HTTP/3 as well.

Review Comment:
   `Similar thing` -> `A similar thing`



##########
iocore/net/SNIActionPerformer.cc:
##########
@@ -28,6 +28,35 @@
 
 #include "P_SNIActionPerformer.h"
 
+#if TS_USE_QUIC == 1
+#include "P_QUICNetVConnection.h"
+#endif
+
+int
+ControlQUIC::SNIAction(TLSSNISupport *snis, const Context &ctx) const
+{
+#if TS_USE_QUIC == 1
+  if (enable_quic) {
+    return SSL_TLSEXT_ERR_OK;
+  }
+
+  // This action is only available for QUIC connections
+  auto *quic_vc = dynamic_cast<QUICNetVConnection *>(snis);
+  if (quic_vc == nullptr) {
+    return SSL_TLSEXT_ERR_OK;
+  }
+
+  if (is_debug_tag_set("ssl_sni")) {
+    const char *servername = quic_vc->get_server_name();
+    Debug("ssl_sni", "Rejecting handshake, fqdn [%s]", servername);

Review Comment:
   Probably good to say specifically that this is due to sni.yaml quic being 
disabled. Maybe:
   
   ```cpp
       Debug("ssl_sni", "Rejecting handshake due to QUIC being disabled for 
fqdn [%s]", servername);
   ```



##########
doc/admin-guide/files/sni.yaml.en.rst:
##########
@@ -174,6 +174,10 @@ http2_buffer_water_mark   Inbound   Specifies the high 
water mark for all HTTP/2
                                     By default this is 
:ts:cv:`proxy.config.http2.default_buffer_water_mark`.
                                     NOTE: Connection coalescing may prevent 
this taking effect.
 
+quic                      Inbound   Indicates whether QUIC connections should 
be accepted. The valid values are :code:`on` or
+                                    :code:`off`. Note that this is an 
additional setting to configure QUIC availability per server
+                                    name. You need to configure 
:ts:cv:`proxy.config.http.server_ports` to open ports for QUIC.

Review Comment:
   I suggest:
   
   ```
   Note that this is a more specific setting to configure QUIC availability per 
server
   name. More broadly, you will also need to configure 
:ts:cv:`proxy.config.http.server_ports` 
   to open ports for QUIC.



-- 
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]

Reply via email to