A performance issue on ConnectionId initialization was raised on #5494, that says generating randomized value in ConnectionId class's default constructor unnecessarily costs CPU, and there's no way to not initialize the value when you declare a variable for reading.
The use case was unexpected because ConnectionId has a constructor for reading that receives a pointer for a source buffer and its length. And in current design, it's assumed that you need a randomized id if you don't use the constructor for reading. ConnectionId has randomize() and some places call this however it's unnecessary now.
The problematic code was here:
|
QUICConnectionId cid_in_retry_token; |
|
if (!vc && params->stateless_retry() && QUICInvariants::is_long_header(buf)) { |
|
int ret = this->_stateless_retry(buf, buf_len, udp_packet->getConnection(), udp_packet->from, dcid, scid, &cid_in_retry_token); |
We could remove randomization logic from the default constructor and call randomize() when necessary. However, it can be a security issue if we forgot to call randomize(), since IDs are supposed to be random number when you send it.
Either of these happens only if we failed to use an appropriate constructor / initialization code. So it's foolproof. While we should avoid unnecessary random value generation, we should make sure that the value is randomized when necessary.
A performance issue on ConnectionId initialization was raised on #5494, that says generating randomized value in ConnectionId class's default constructor unnecessarily costs CPU, and there's no way to not initialize the value when you declare a variable for reading.
The use case was unexpected because
ConnectionIdhas a constructor for reading that receives a pointer for a source buffer and its length. And in current design, it's assumed that you need a randomized id if you don't use the constructor for reading.ConnectionIdhasrandomize()and some places call this however it's unnecessary now.The problematic code was here:
trafficserver/iocore/net/QUICPacketHandler.cc
Lines 289 to 291 in 0cb5634
We could remove randomization logic from the default constructor and call
randomize()when necessary. However, it can be a security issue if we forgot to callrandomize(), since IDs are supposed to be random number when you send it.Either of these happens only if we failed to use an appropriate constructor / initialization code. So it's foolproof. While we should avoid unnecessary random value generation, we should make sure that the value is randomized when necessary.