mirror of
https://github.com/facebookincubator/mvfst.git
synced 2025-08-08 09:42:06 +03:00
Log an error when we write a packet larger than limit
Summary: as title Reviewed By: bschlinker Differential Revision: D22542572 fbshipit-source-id: 4f53a1c916b22e33f183208efb671c1d0883309c
This commit is contained in:
committed by
Facebook GitHub Bot
parent
6d443efc69
commit
a7dc8cb4f7
@@ -290,9 +290,10 @@ ssize_t GSOInplacePacketBatchWriter::write(
|
|||||||
(nextPacketSize_ && diffToEnd == nextPacketSize_))
|
(nextPacketSize_ && diffToEnd == nextPacketSize_))
|
||||||
<< "diffToEnd=" << diffToEnd << ", pktLimit=" << conn_.udpSendPacketLen
|
<< "diffToEnd=" << diffToEnd << ", pktLimit=" << conn_.udpSendPacketLen
|
||||||
<< ", nextPacketSize_=" << nextPacketSize_;
|
<< ", nextPacketSize_=" << nextPacketSize_;
|
||||||
CHECK_LT(diffToEnd, conn_.udpSendPacketLen + kPacketSizeViolationTolerance)
|
if (diffToEnd >= conn_.udpSendPacketLen + kPacketSizeViolationTolerance) {
|
||||||
<< "Remaining buffer contents larger than udpSendPacketLen by "
|
LOG(ERROR) << "Remaining buffer contents larger than udpSendPacketLen by "
|
||||||
<< (diffToEnd - conn_.udpSendPacketLen);
|
<< (diffToEnd - conn_.udpSendPacketLen);
|
||||||
|
}
|
||||||
uint64_t diffToStart = lastPacketEnd_ - buf->data();
|
uint64_t diffToStart = lastPacketEnd_ - buf->data();
|
||||||
buf->trimEnd(diffToEnd);
|
buf->trimEnd(diffToEnd);
|
||||||
auto bytesWritten = (numPackets_ > 1)
|
auto bytesWritten = (numPackets_ > 1)
|
||||||
|
@@ -267,6 +267,10 @@ DataPathResult continuousMemoryBuildScheduleEncrypt(
|
|||||||
// Include previous packets back.
|
// Include previous packets back.
|
||||||
packetBuf->prepend(prevSize);
|
packetBuf->prepend(prevSize);
|
||||||
connection.bufAccessor->release(std::move(packetBuf));
|
connection.bufAccessor->release(std::move(packetBuf));
|
||||||
|
if (encodedSize > connection.udpSendPacketLen) {
|
||||||
|
LOG(ERROR) << "Quic sending pkt larger than limit, encodedSize="
|
||||||
|
<< encodedSize;
|
||||||
|
}
|
||||||
// TODO: I think we should add an API that doesn't need a buffer.
|
// TODO: I think we should add an API that doesn't need a buffer.
|
||||||
bool ret = ioBufBatch.write(nullptr /* no need to pass buf */, encodedSize);
|
bool ret = ioBufBatch.write(nullptr /* no need to pass buf */, encodedSize);
|
||||||
// update stats and connection
|
// update stats and connection
|
||||||
@@ -338,6 +342,10 @@ DataPathResult iobufChainBasedBuildScheduleEncrypt(
|
|||||||
packetBuf->length() - headerLen,
|
packetBuf->length() - headerLen,
|
||||||
headerCipher);
|
headerCipher);
|
||||||
auto encodedSize = packetBuf->computeChainDataLength();
|
auto encodedSize = packetBuf->computeChainDataLength();
|
||||||
|
if (encodedSize > connection.udpSendPacketLen) {
|
||||||
|
LOG(ERROR) << "Quic sending pkt larger than limit, encodedSize="
|
||||||
|
<< encodedSize;
|
||||||
|
}
|
||||||
bool ret = ioBufBatch.write(std::move(packetBuf), encodedSize);
|
bool ret = ioBufBatch.write(std::move(packetBuf), encodedSize);
|
||||||
if (ret) {
|
if (ret) {
|
||||||
// update stats and connection
|
// update stats and connection
|
||||||
|
@@ -624,39 +624,6 @@ TEST_P(QuicBatchWriterTest, InplaceWriterBufResidueCheck) {
|
|||||||
EXPECT_EQ(0, rawBuf->headroom());
|
EXPECT_EQ(0, rawBuf->headroom());
|
||||||
}
|
}
|
||||||
|
|
||||||
TEST_P(QuicBatchWriterTest, InplaceWriterBufResidueTooBig) {
|
|
||||||
bool useThreadLocal = GetParam();
|
|
||||||
folly::EventBase evb;
|
|
||||||
folly::test::MockAsyncUDPSocket sock(&evb);
|
|
||||||
EXPECT_CALL(sock, getGSO()).WillRepeatedly(Return(1));
|
|
||||||
|
|
||||||
uint32_t batchSize = 20;
|
|
||||||
auto bufAccessor =
|
|
||||||
std::make_unique<SimpleBufAccessor>(conn_.udpSendPacketLen * batchSize);
|
|
||||||
conn_.bufAccessor = bufAccessor.get();
|
|
||||||
conn_.udpSendPacketLen = 1000;
|
|
||||||
auto batchWriter = quic::BatchWriterFactory::makeBatchWriter(
|
|
||||||
sock,
|
|
||||||
quic::QuicBatchingMode::BATCHING_MODE_GSO,
|
|
||||||
batchSize,
|
|
||||||
useThreadLocal,
|
|
||||||
quic::kDefaultThreadLocalDelay,
|
|
||||||
DataPathType::ContinuousMemory,
|
|
||||||
conn_);
|
|
||||||
auto buf = bufAccessor->obtain();
|
|
||||||
folly::IOBuf* rawBuf = buf.get();
|
|
||||||
bufAccessor->release(std::move(buf));
|
|
||||||
rawBuf->append(700);
|
|
||||||
ASSERT_FALSE(
|
|
||||||
batchWriter->append(nullptr, 700, folly::SocketAddress(), nullptr));
|
|
||||||
|
|
||||||
size_t packetSizeTooBig = 1090;
|
|
||||||
rawBuf->append(packetSizeTooBig);
|
|
||||||
EXPECT_TRUE(batchWriter->needsFlush(packetSizeTooBig));
|
|
||||||
|
|
||||||
EXPECT_DEATH(batchWriter->write(sock, folly::SocketAddress()), "");
|
|
||||||
}
|
|
||||||
|
|
||||||
INSTANTIATE_TEST_CASE_P(
|
INSTANTIATE_TEST_CASE_P(
|
||||||
QuicBatchWriterTest,
|
QuicBatchWriterTest,
|
||||||
QuicBatchWriterTest,
|
QuicBatchWriterTest,
|
||||||
|
Reference in New Issue
Block a user