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/06/01 15:50:53 UTC
[GitHub] [trafficserver] brbzull0 opened a new pull request, #9765: UDP/QUIC: Make sure quiche gets the entire rx buffer from the udp packet.
brbzull0 opened a new pull request, #9765:
URL: https://github.com/apache/trafficserver/pull/9765
It seems that it was only getting the first slice of the entire chain. Currently each of the iov buffer size is hardcoded to 2k, if happens that we need to fit more than 2k inside the buffer the current code will only hand over the first part of the chain which will make QUICHE to complain as the data is incomplete.
This change makes sure that quiche gets the entire buffer.
FYI: For now this seems fine but as soon as we introduce GRO this will more likely to change.
--
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 #9765: UDP/QUIC: Make sure quiche gets the entire rx buffer from the udp packet.
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on code in PR #9765:
URL: https://github.com/apache/trafficserver/pull/9765#discussion_r1213380891
##########
iocore/net/UnixUDPNet.cc:
##########
@@ -134,9 +134,40 @@ UDPPacket::free()
if (p.conn)
p.conn->Release();
p.conn = nullptr;
+
+ if (this->_payload) {
+ _payload.release();
+ }
udpPacketAllocator.free(this);
}
+uint8_t *
+UDPPacket::get_entire_chain_buffer(size_t *buf_len)
+{
+ if (this->_payload == nullptr) {
+ IOBufferBlock *block = this->getIOBlockChain();
+
+ // No need to allocate, we will use the first slice. With the current slice size
+ // 2048 more likely we will using this block most of the time.
+ if (block && block->next.get() == nullptr) {
+ *buf_len = this->getPktLength();
+ return reinterpret_cast<uint8_t *>(block->buf());
+ }
+
+ // Store it. Should we try to avoid allocating here?
Review Comment:
> Should we try to avoid allocating here?
Yes, it's very unfortunate to alloc another buffer and copy data. But as we talked offline, let's fix the bug for now and improve it with a GRO friendly design.
--
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 #9765: UDP/QUIC: Make sure quiche gets the entire rx buffer from the udp packet.
Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on PR #9765:
URL: https://github.com/apache/trafficserver/pull/9765#issuecomment-1572350287
[approve ci Ubuntu]
--
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 #9765: UDP/QUIC: Make sure quiche gets the entire rx buffer from the udp packet.
Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on PR #9765:
URL: https://github.com/apache/trafficserver/pull/9765#issuecomment-1572375724
[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 #9765: UDP/QUIC: Make sure quiche gets the entire rx buffer from the udp packet.
Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on PR #9765:
URL: https://github.com/apache/trafficserver/pull/9765#issuecomment-1573580547
[approve ci ubuntu]
--
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 #9765: UDP/QUIC: Make sure quiche gets the entire rx buffer from the udp packet.
Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 merged PR #9765:
URL: https://github.com/apache/trafficserver/pull/9765
--
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 #9765: UDP/QUIC: Make sure quiche gets the entire rx buffer from the udp packet.
Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on PR #9765:
URL: https://github.com/apache/trafficserver/pull/9765#issuecomment-1572374909
[approve ci ubuntu]
--
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