You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "brbzull0 (via GitHub)" <gi...@apache.org> on 2023/03/03 15:20:34 UTC

[GitHub] [trafficserver] brbzull0 opened a new pull request, #9486: QUIC: Add support to configure UDP max payload limit.

brbzull0 opened a new pull request, #9486:
URL: https://github.com/apache/trafficserver/pull/9486

   This commit also makes this configuration available for the quiche quic implementation.


-- 
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] ywkaras commented on a diff in pull request #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#discussion_r1130348783


##########
iocore/net/QUICNetProcessor_quiche.cc:
##########
@@ -90,8 +90,8 @@ QUICNetProcessor::start(int, size_t stacksize)
   }
 
   quiche_config_set_max_idle_timeout(this->_quiche_config, params->no_activity_timeout_in());
-  quiche_config_set_max_recv_udp_payload_size(this->_quiche_config, 16384);
-  quiche_config_set_max_send_udp_payload_size(this->_quiche_config, 16384);
+  quiche_config_set_max_recv_udp_payload_size(this->_quiche_config, params->get_max_recv_udp_payload_size_in());
+  quiche_config_set_max_send_udp_payload_size(this->_quiche_config, params->get_max_send_udp_payload_size_in());

Review Comment:
   Seems a bit odd that the default value for these config variables is (about) double the previous hard-coded value.



-- 
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 #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#issuecomment-1453797736

   I just found that `max_udp_payload_size` is actually a Transport Parameter of QUIC. It may be more appropriate to have the setting under `ts.config.quic`. And if so, we should have ones with `_in` and `_out` in the names to support outgoing QUIC connection in the future.
   
   > max_udp_payload_size (0x03):
   The maximum UDP payload size parameter is an integer value that limits the size of UDP payloads that the endpoint is willing to receive. UDP datagrams with payloads larger than this limit are not likely to be processed by the receiver.[¶](https://datatracker.ietf.org/doc/html/rfc9000#section-18.2-4.8.1)
   
   > The default for this parameter is the maximum permitted UDP payload of 65527. Values below 1200 are invalid.[¶](https://datatracker.ietf.org/doc/html/rfc9000#section-18.2-4.10.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


[GitHub] [trafficserver] ywkaras commented on a diff in pull request #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#discussion_r1130308866


##########
iocore/net/QUICNetProcessor_quiche.cc:
##########
@@ -90,8 +90,8 @@ QUICNetProcessor::start(int, size_t stacksize)
   }
 
   quiche_config_set_max_idle_timeout(this->_quiche_config, params->no_activity_timeout_in());
-  quiche_config_set_max_recv_udp_payload_size(this->_quiche_config, 16384);
-  quiche_config_set_max_send_udp_payload_size(this->_quiche_config, 16384);
+  quiche_config_set_max_recv_udp_payload_size(this->_quiche_config, params->get_max_recv_udp_payload_size_in());
+  quiche_config_set_max_send_udp_payload_size(this->_quiche_config, params->get_max_send_udp_payload_size_in());

Review Comment:
   in or out?



-- 
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] brbzull0 commented on pull request #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#issuecomment-1458444860

   > Ah, Quiche API uses similar names for send and recv, but only the one for recv is a transport parameter.
   > 
   > We should probably have the both recv and send under `ts.quic` because I think we need `in|out` for the both. `in|out` are for inbound|outbound _connections_, so having it under ts.udp doesn't make sense.
   > 
   > We could also have `ts.udp.send_payload_size` without `in|out`, but that'd be complicated. Either `ts.udp.send_payload_size` or `ts.quic.max_udp_send_payload_size_in|out` would have to override the other.
   
   grand, will update this PR then.


-- 
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] ywkaras commented on a diff in pull request #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#discussion_r1131265625


##########
iocore/net/QUICNetProcessor_quiche.cc:
##########
@@ -90,8 +90,8 @@ QUICNetProcessor::start(int, size_t stacksize)
   }
 
   quiche_config_set_max_idle_timeout(this->_quiche_config, params->no_activity_timeout_in());
-  quiche_config_set_max_recv_udp_payload_size(this->_quiche_config, 16384);
-  quiche_config_set_max_send_udp_payload_size(this->_quiche_config, 16384);
+  quiche_config_set_max_recv_udp_payload_size(this->_quiche_config, params->get_max_recv_udp_payload_size_in());
+  quiche_config_set_max_send_udp_payload_size(this->_quiche_config, params->get_max_send_udp_payload_size_in());

Review Comment:
   From the names of the the Quiche API functions, it doesn't look like you can set different max packet sizes when the near end is the server versus the client. 



-- 
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] brbzull0 commented on pull request #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#issuecomment-1462443575

   Made a mess with my branches, this is missing the actual records. Will fix. Sorry again guys.


-- 
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 #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on code in PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#discussion_r1131387511


##########
iocore/net/QUICNetProcessor_quiche.cc:
##########
@@ -90,8 +90,8 @@ QUICNetProcessor::start(int, size_t stacksize)
   }
 
   quiche_config_set_max_idle_timeout(this->_quiche_config, params->no_activity_timeout_in());
-  quiche_config_set_max_recv_udp_payload_size(this->_quiche_config, 16384);
-  quiche_config_set_max_send_udp_payload_size(this->_quiche_config, 16384);
+  quiche_config_set_max_recv_udp_payload_size(this->_quiche_config, params->get_max_recv_udp_payload_size_in());
+  quiche_config_set_max_send_udp_payload_size(this->_quiche_config, params->get_max_send_udp_payload_size_in());

Review Comment:
   Oh you mean different packet sizes for inbound connection and outbound connection? We'll need another quiche_config object for outbound connections. That is why we use `_in` settings here. We'll have similar code that use `_out` settings somewhere else in the future.



-- 
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 #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on code in PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#discussion_r1131213628


##########
iocore/net/QUICNetProcessor_quiche.cc:
##########
@@ -90,8 +90,8 @@ QUICNetProcessor::start(int, size_t stacksize)
   }
 
   quiche_config_set_max_idle_timeout(this->_quiche_config, params->no_activity_timeout_in());
-  quiche_config_set_max_recv_udp_payload_size(this->_quiche_config, 16384);
-  quiche_config_set_max_send_udp_payload_size(this->_quiche_config, 16384);
+  quiche_config_set_max_recv_udp_payload_size(this->_quiche_config, params->get_max_recv_udp_payload_size_in());
+  quiche_config_set_max_send_udp_payload_size(this->_quiche_config, params->get_max_send_udp_payload_size_in());

Review Comment:
   And `_in` is correct for the both here. It's for inbound connections. The `_out` will be used when we support H*3* to  origin.



-- 
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] ywkaras commented on a diff in pull request #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#discussion_r1131366869


##########
iocore/net/QUICNetProcessor_quiche.cc:
##########
@@ -90,8 +90,8 @@ QUICNetProcessor::start(int, size_t stacksize)
   }
 
   quiche_config_set_max_idle_timeout(this->_quiche_config, params->no_activity_timeout_in());
-  quiche_config_set_max_recv_udp_payload_size(this->_quiche_config, 16384);
-  quiche_config_set_max_send_udp_payload_size(this->_quiche_config, 16384);
+  quiche_config_set_max_recv_udp_payload_size(this->_quiche_config, params->get_max_recv_udp_payload_size_in());
+  quiche_config_set_max_send_udp_payload_size(this->_quiche_config, params->get_max_send_udp_payload_size_in());

Review Comment:
   Yes, that's what I thought.  But I don't see how Quiche is allowing us to configure those values differently depending on whether the near end is the client or the server.  If we can't, we only need two new config variables, not four.



-- 
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 #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on code in PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#discussion_r1131340925


##########
iocore/net/QUICNetProcessor_quiche.cc:
##########
@@ -90,8 +90,8 @@ QUICNetProcessor::start(int, size_t stacksize)
   }
 
   quiche_config_set_max_idle_timeout(this->_quiche_config, params->no_activity_timeout_in());
-  quiche_config_set_max_recv_udp_payload_size(this->_quiche_config, 16384);
-  quiche_config_set_max_send_udp_payload_size(this->_quiche_config, 16384);
+  quiche_config_set_max_recv_udp_payload_size(this->_quiche_config, params->get_max_recv_udp_payload_size_in());
+  quiche_config_set_max_send_udp_payload_size(this->_quiche_config, params->get_max_send_udp_payload_size_in());

Review Comment:
   Although MTU always limits packet sizes on both direction, what these settings mean are:
   recv: Tell the peer "Don't send me packets bigger than this size"
   send: Think quietly "I'm going to send this size of packets at maximum if the peer allows"



-- 
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] brbzull0 closed pull request #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 closed pull request #9486: QUIC: Add support to configure UDP max payload limit. 
URL: https://github.com/apache/trafficserver/pull/9486


-- 
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] brbzull0 merged pull request #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 merged PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486


-- 
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] brbzull0 commented on pull request #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#issuecomment-1458444508

   grand
   
   


-- 
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 #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#issuecomment-1456918023

   Ah, Quiche API uses similar names for send and recv, but only the one for recv is a transport parameter.
   
   We should probably have the both recv and send under `ts.quic` because I think we need `in|out` for the both. `in|out` are for inbound|outbound *connections*, so having it under ts.udp doesn't make sense.
   
   We could also have `ts.udp.send_payload_size` without `in|out`, but that'd be complicated. Either `ts.udp.send_payload_size` or `ts.quic.max_udp_send_payload_size_in|out` would have to override the other.


-- 
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] ywkaras commented on a diff in pull request #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#discussion_r1130308100


##########
doc/admin-guide/files/records.yaml.en.rst:
##########
@@ -4667,6 +4667,23 @@ removed in the future without prior notice.
    that |TS| is willing to store. The value MUST be at least 2.
    Transport Parameter.
 
+.. ts:cv:: CONFIG proxy.config.quic.max_recv_udp_payload_size_in INT 65527
+
+   This value will be advertised as ``max_udp_payload_size`` Transport Parameter.
+
+.. ts:cv:: CONFIG proxy.config.quic.max_recv_udp_payload_size_out INT 65527
+
+   This value will be advertised as ``max_udp_payload_size`` Transport Parameter.
+
+.. ts:cv:: CONFIG proxy.config.quic.max_send_udp_payload_size_in INT 65527
+
+   Specified the maximum outgoing UDP payload size.
+
+.. ts:cv:: CONFIG proxy.config.quic.max_send_udp_payload_size_out INT 65527
+
+   Specified the maximum outgoing UDP payload size.

Review Comment:
   Ditto.



-- 
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] brbzull0 commented on a diff in pull request #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on code in PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#discussion_r1130817610


##########
iocore/net/QUICNetProcessor_quiche.cc:
##########
@@ -90,8 +90,8 @@ QUICNetProcessor::start(int, size_t stacksize)
   }
 
   quiche_config_set_max_idle_timeout(this->_quiche_config, params->no_activity_timeout_in());
-  quiche_config_set_max_recv_udp_payload_size(this->_quiche_config, 16384);
-  quiche_config_set_max_send_udp_payload_size(this->_quiche_config, 16384);
+  quiche_config_set_max_recv_udp_payload_size(this->_quiche_config, params->get_max_recv_udp_payload_size_in());
+  quiche_config_set_max_send_udp_payload_size(this->_quiche_config, params->get_max_send_udp_payload_size_in());

Review Comment:
   It seems to be the default for [this](https://www.rfc-editor.org/rfc/rfc9000.html#section-18.2-4.10.1) transport parameter.
   > The default for this parameter is the maximum permitted UDP payload of 65527. Values below 1200 are invalid.well, being hardcoded before means nothing different 
   



-- 
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] brbzull0 commented on a diff in pull request #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on code in PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#discussion_r1131338498


##########
src/records/RecordsConfig.cc:
##########
@@ -258,6 +258,10 @@ static const RecordElement RecordsConfig[] =
   ,
   {RECT_CONFIG, "proxy.config.udp.enable_gso", RECD_INT, "1", RECU_NULL, RR_NULL, RECC_NULL, nullptr, RECA_NULL}
   ,
+  {RECT_CONFIG, "proxy.config.udp.max_recv_payload_size", RECD_INT, "65527", RECU_DYNAMIC, RR_NULL, RECC_NULL, "^[0-9]+$", RECA_NULL}

Review Comment:
   yes, my bad, made a mess with names and all that stuff. Will fix.



-- 
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] ywkaras commented on a diff in pull request #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#discussion_r1131257078


##########
doc/admin-guide/files/records.yaml.en.rst:
##########
@@ -4667,6 +4667,23 @@ removed in the future without prior notice.
    that |TS| is willing to store. The value MUST be at least 2.
    Transport Parameter.
 
+.. ts:cv:: CONFIG proxy.config.quic.max_recv_udp_payload_size_in INT 65527
+
+   This value will be advertised as ``max_udp_payload_size`` Transport Parameter.
+
+.. ts:cv:: CONFIG proxy.config.quic.max_recv_udp_payload_size_out INT 65527
+
+   This value will be advertised as ``max_udp_payload_size`` Transport Parameter.

Review Comment:
   I suggest adding "for incoming/outgoing" packets.



-- 
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] brbzull0 commented on pull request #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#issuecomment-1455983908

   > I just found that `max_udp_payload_size` is actually a Transport Parameter of QUIC. It may be more appropriate to have the setting under `ts.config.quic`. And if so, we should have ones with `_in` and `_out` in the names to support outgoing QUIC connection in the future.
   > 
   > > max_udp_payload_size (0x03):
   > > The maximum UDP payload size parameter is an integer value that limits the size of UDP payloads that the endpoint is willing to receive. UDP datagrams with payloads larger than this limit are not likely to be processed by the receiver.[¶](https://datatracker.ietf.org/doc/html/rfc9000#section-18.2-4.8.1)
   > 
   > > The default for this parameter is the maximum permitted UDP payload of 65527. Values below 1200 are invalid.[¶](https://datatracker.ietf.org/doc/html/rfc9000#section-18.2-4.10.1)
   
   Ok, I saw that on the spcecs,
   
   So,
   
   `max_udp_payload_size_in|out` should be part of the `ts.quic` block, but `max_udp_send_payload_size` is not a transport param but rather a generic UDP, so the later should be part of the `ts.udp` , now as we set this on quiche, do we still need the `in|out` for the `send_payload_size`?


-- 
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] brbzull0 commented on pull request #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#issuecomment-1453767830

   [approve ci autest]


-- 
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] brbzull0 commented on pull request #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#issuecomment-1458555979

   [approve ci autest]


-- 
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] brbzull0 commented on a diff in pull request #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on code in PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#discussion_r1130817343


##########
doc/admin-guide/files/records.yaml.en.rst:
##########
@@ -4667,6 +4667,23 @@ removed in the future without prior notice.
    that |TS| is willing to store. The value MUST be at least 2.
    Transport Parameter.
 
+.. ts:cv:: CONFIG proxy.config.quic.max_recv_udp_payload_size_in INT 65527
+
+   This value will be advertised as ``max_udp_payload_size`` Transport Parameter.
+
+.. ts:cv:: CONFIG proxy.config.quic.max_recv_udp_payload_size_out INT 65527
+
+   This value will be advertised as ``max_udp_payload_size`` Transport Parameter.

Review Comment:
   Yes, they reference to the same transport parameter.  I think this eventually will contain more details, or even a link to the current rfc definition.



-- 
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] ywkaras commented on a diff in pull request #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#discussion_r1130307336


##########
doc/admin-guide/files/records.yaml.en.rst:
##########
@@ -4667,6 +4667,23 @@ removed in the future without prior notice.
    that |TS| is willing to store. The value MUST be at least 2.
    Transport Parameter.
 
+.. ts:cv:: CONFIG proxy.config.quic.max_recv_udp_payload_size_in INT 65527
+
+   This value will be advertised as ``max_udp_payload_size`` Transport Parameter.
+
+.. ts:cv:: CONFIG proxy.config.quic.max_recv_udp_payload_size_out INT 65527
+
+   This value will be advertised as ``max_udp_payload_size`` Transport Parameter.

Review Comment:
   Two configs with exactly the same description.



-- 
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 #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on code in PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#discussion_r1131220260


##########
src/records/RecordsConfig.cc:
##########
@@ -258,6 +258,10 @@ static const RecordElement RecordsConfig[] =
   ,
   {RECT_CONFIG, "proxy.config.udp.enable_gso", RECD_INT, "1", RECU_NULL, RR_NULL, RECC_NULL, nullptr, RECA_NULL}
   ,
+  {RECT_CONFIG, "proxy.config.udp.max_recv_payload_size", RECD_INT, "65527", RECU_DYNAMIC, RR_NULL, RECC_NULL, "^[0-9]+$", RECA_NULL}

Review Comment:
   We are going to have `proxy.config.quic.max_recv_udp_payload_size_in|out`, right?



-- 
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 #9486: QUIC: Add support to configure UDP max payload limit.

Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on code in PR #9486:
URL: https://github.com/apache/trafficserver/pull/9486#discussion_r1131206177


##########
iocore/net/QUICNetProcessor_quiche.cc:
##########
@@ -90,8 +90,8 @@ QUICNetProcessor::start(int, size_t stacksize)
   }
 
   quiche_config_set_max_idle_timeout(this->_quiche_config, params->no_activity_timeout_in());
-  quiche_config_set_max_recv_udp_payload_size(this->_quiche_config, 16384);
-  quiche_config_set_max_send_udp_payload_size(this->_quiche_config, 16384);
+  quiche_config_set_max_recv_udp_payload_size(this->_quiche_config, params->get_max_recv_udp_payload_size_in());
+  quiche_config_set_max_send_udp_payload_size(this->_quiche_config, params->get_max_send_udp_payload_size_in());

Review Comment:
   The hard-coded value means nothing. It could be 12000 or 32768 if I made the commit on a different day.



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