BIT-1854: fix the 'tcp_excessive_data_without_further_acks' option

This previously checked against the amount of out-of-sequence data
being buffered by the reassembler.  It now checks against the total
size of all blocks being buffered in the reassembler, which, by nature
of still being buffered there, means it's not been acked yet.
This commit is contained in:
Jon Siwek 2018-01-31 21:09:12 -06:00
parent 44175e0992
commit c2af3daa9f
3 changed files with 31 additions and 18 deletions

View file

@ -10,10 +10,11 @@
static const bool DEBUG_reassem = false; static const bool DEBUG_reassem = false;
DataBlock::DataBlock(const u_char* data, uint64 size, uint64 arg_seq, DataBlock::DataBlock(DataBlockListInfo* list_info, const u_char* data,
DataBlock* arg_prev, DataBlock* arg_next, uint64 size, uint64 arg_seq, DataBlock* arg_prev,
ReassemblerType reassem_type) DataBlock* arg_next, ReassemblerType reassem_type)
{ {
info = list_info;
seq = arg_seq; seq = arg_seq;
upper = seq + size; upper = seq + size;
block = new u_char[size]; block = new u_char[size];
@ -31,18 +32,19 @@ DataBlock::DataBlock(const u_char* data, uint64 size, uint64 arg_seq,
rtype = reassem_type; rtype = reassem_type;
Reassembler::sizes[rtype] += pad_size(size) + padded_sizeof(DataBlock); Reassembler::sizes[rtype] += pad_size(size) + padded_sizeof(DataBlock);
Reassembler::total_size += pad_size(size) + padded_sizeof(DataBlock); Reassembler::total_size += pad_size(size) + padded_sizeof(DataBlock);
info->totalSize += size;
} }
uint64 Reassembler::total_size = 0; uint64 Reassembler::total_size = 0;
uint64 Reassembler::sizes[REASSEM_NUM]; uint64 Reassembler::sizes[REASSEM_NUM];
Reassembler::Reassembler(uint64 init_seq, ReassemblerType reassem_type) Reassembler::Reassembler(uint64 init_seq, ReassemblerType reassem_type)
: block_list_info(),
blocks(), last_block(), old_blocks(), last_old_block(),
last_reassem_seq(init_seq), trim_seq(init_seq),
max_old_blocks(0), total_old_blocks(0),
rtype(reassem_type)
{ {
blocks = last_block = 0;
old_blocks = last_old_block = 0;
total_old_blocks = max_old_blocks = 0;
trim_seq = last_reassem_seq = init_seq;
rtype = reassem_type;
} }
Reassembler::~Reassembler() Reassembler::~Reassembler()
@ -116,7 +118,7 @@ void Reassembler::NewBlock(double t, uint64 seq, uint64 len, const u_char* data)
if ( ! blocks ) if ( ! blocks )
blocks = last_block = start_block = blocks = last_block = start_block =
new DataBlock(data, len, seq, 0, 0, rtype); new DataBlock(&block_list_info, data, len, seq, 0, 0, rtype);
else else
start_block = AddAndCheck(blocks, seq, upper_seq, data); start_block = AddAndCheck(blocks, seq, upper_seq, data);
@ -280,8 +282,8 @@ DataBlock* Reassembler::AddAndCheck(DataBlock* b, uint64 seq, uint64 upper,
// Special check for the common case of appending to the end. // Special check for the common case of appending to the end.
if ( last_block && seq == last_block->upper ) if ( last_block && seq == last_block->upper )
{ {
last_block = new DataBlock(data, upper - seq, seq, last_block = new DataBlock(&block_list_info, data, upper - seq,
last_block, 0, rtype); seq, last_block, 0, rtype);
return last_block; return last_block;
} }
@ -294,7 +296,8 @@ DataBlock* Reassembler::AddAndCheck(DataBlock* b, uint64 seq, uint64 upper,
{ {
// b is the last block, and it comes completely before // b is the last block, and it comes completely before
// the new block. // the new block.
last_block = new DataBlock(data, upper - seq, seq, b, 0, rtype); last_block = new DataBlock(&block_list_info, data, upper - seq,
seq, b, 0, rtype);
return last_block; return last_block;
} }
@ -303,7 +306,8 @@ DataBlock* Reassembler::AddAndCheck(DataBlock* b, uint64 seq, uint64 upper,
if ( upper <= b->seq ) if ( upper <= b->seq )
{ {
// The new block comes completely before b. // The new block comes completely before b.
new_b = new DataBlock(data, upper - seq, seq, b->prev, b, rtype); new_b = new DataBlock(&block_list_info, data, upper - seq, seq,
b->prev, b, rtype);
if ( b == blocks ) if ( b == blocks )
blocks = new_b; blocks = new_b;
return new_b; return new_b;
@ -314,7 +318,8 @@ DataBlock* Reassembler::AddAndCheck(DataBlock* b, uint64 seq, uint64 upper,
{ {
// The new block has a prefix that comes before b. // The new block has a prefix that comes before b.
uint64 prefix_len = b->seq - seq; uint64 prefix_len = b->seq - seq;
new_b = new DataBlock(data, prefix_len, seq, b->prev, b, rtype); new_b = new DataBlock(&block_list_info, data, prefix_len, seq,
b->prev, b, rtype);
if ( b == blocks ) if ( b == blocks )
blocks = new_b; blocks = new_b;

View file

@ -18,11 +18,16 @@ enum ReassemblerType {
REASSEM_NUM, REASSEM_NUM,
}; };
struct DataBlockListInfo {
uint64 totalSize;
};
class DataBlock { class DataBlock {
public: public:
DataBlock(const u_char* data, uint64 size, uint64 seq, DataBlock(DataBlockListInfo* list_info, const u_char* data,
DataBlock* prev, DataBlock* next, uint64 size, uint64 seq,
ReassemblerType reassem_type = REASSEM_UNKNOWN); DataBlock* prev, DataBlock* next,
ReassemblerType reassem_type = REASSEM_UNKNOWN);
~DataBlock(); ~DataBlock();
@ -33,6 +38,7 @@ public:
uint64 seq, upper; uint64 seq, upper;
u_char* block; u_char* block;
ReassemblerType rtype; ReassemblerType rtype;
DataBlockListInfo* info;
}; };
class Reassembler : public BroObj { class Reassembler : public BroObj {
@ -86,6 +92,7 @@ protected:
void CheckOverlap(DataBlock *head, DataBlock *tail, void CheckOverlap(DataBlock *head, DataBlock *tail,
uint64 seq, uint64 len, const u_char* data); uint64 seq, uint64 len, const u_char* data);
DataBlockListInfo block_list_info;
DataBlock* blocks; DataBlock* blocks;
DataBlock* last_block; DataBlock* last_block;
@ -105,6 +112,7 @@ protected:
inline DataBlock::~DataBlock() inline DataBlock::~DataBlock()
{ {
info->totalSize -= Size();
Reassembler::total_size -= pad_size(upper - seq) + padded_sizeof(DataBlock); Reassembler::total_size -= pad_size(upper - seq) + padded_sizeof(DataBlock);
Reassembler::sizes[rtype] -= pad_size(upper - seq) + padded_sizeof(DataBlock); Reassembler::sizes[rtype] -= pad_size(upper - seq) + padded_sizeof(DataBlock);
delete [] block; delete [] block;

View file

@ -501,7 +501,7 @@ int TCP_Reassembler::DataSent(double t, uint64 seq, int len,
} }
if ( tcp_excessive_data_without_further_acks && if ( tcp_excessive_data_without_further_acks &&
NumUndeliveredBytes() > static_cast<uint64>(tcp_excessive_data_without_further_acks) ) block_list_info.totalSize > static_cast<uint64>(tcp_excessive_data_without_further_acks) )
{ {
tcp_analyzer->Weird("excessive_data_without_further_acks"); tcp_analyzer->Weird("excessive_data_without_further_acks");
ClearBlocks(); ClearBlocks();