Reorganize reassembly data structures

Started by factoring some details into a new DataBlockList class to at
least make it more clear where modifications occur.  More abstractions
likely to happen later as I experiment with alternate data structures
aimed at improving worse-case scenarios.

src/Frag.cc

@@ -185,15 +185,17 @@ void FragReassembler::Overlap(const u_char* b1, const u_char* b2, uint64_t n)
 		Weird("fragment_overlap");
 	}
 
-void FragReassembler::BlockInserted(DataBlock* /* start_block */)
+void FragReassembler::BlockInserted(const DataBlock* /* start_block */)
 	{
-	if ( blocks->seq > 0 || ! frag_size )
+	const DataBlock* b = block_list.Head();
+	// TODO: review all iteration here to see if it can be done better
+
+	if ( b->seq > 0 || ! frag_size )
 		// For sure don't have it all yet.
 		return;
 
 	// We might have it all - look for contiguous all the way.
-	DataBlock* b;
+	for ( ; b->next; b = b->next )
-	for ( b = blocks; b->next; b = b->next )
 		if ( b->upper != b->next->seq )
 			break;
 
@@ -219,13 +221,13 @@ void FragReassembler::BlockInserted(DataBlock* /* start_block */)
 			return;
 		}
 
-	else if ( last_block->upper > frag_size )
+	else if ( block_list.Tail()->upper > frag_size )
 		{
 		Weird("fragment_size_inconsistency");
-		frag_size = last_block->upper;
+		frag_size = block_list.Tail()->upper;
 		}
 
-	else if ( last_block->upper < frag_size )
+	else if ( block_list.Tail()->upper < frag_size )
 		// Missing the tail.
 		return;
 
@@ -246,7 +248,7 @@ void FragReassembler::BlockInserted(DataBlock* /* start_block */)
 
 	pkt += proto_hdr_len;
 
-	for ( b = blocks; b; b = b->next )
+	for ( b = block_list.Head(); b; b = b->next )
 		{
 		// If we're above a hole, stop.  This can happen because
 		// the logic above regarding a hole that's above the
@@ -299,13 +301,7 @@ void FragReassembler::BlockInserted(DataBlock* /* start_block */)
 
 void FragReassembler::Expire(double t)
 	{
-	while ( blocks )
+	block_list.Clear();
-		{
-		DataBlock* b = blocks->next;
-		delete blocks;
-		blocks = b;
-		}
-
 	expire_timer->ClearReassembler();
 	expire_timer = 0;	// timer manager will delete it
 

src/Frag.h

@@ -37,7 +37,7 @@ public:
 	const FragReassemblerKey& Key() const	{ return key; }
 
 protected:
-	void BlockInserted(DataBlock* start_block) override;
+	void BlockInserted(const DataBlock* start_block) override;
 	void Overlap(const u_char* b1, const u_char* b2, uint64_t n) override;
 	void Weird(const char* name) const;
 

src/Reassem.cc

@@ -7,7 +7,8 @@
 
 #include "Reassem.h"
 
-static const bool DEBUG_reassem = false;
+uint64_t Reassembler::total_size = 0;
+uint64_t Reassembler::sizes[REASSEM_NUM];
 
 DataBlock::DataBlock(Reassembler* reass, const u_char* data,
                      uint64_t size, uint64_t arg_seq, DataBlock* arg_prev,
@@ -27,6 +28,8 @@ DataBlock::DataBlock(Reassembler* reass, const u_char* data,
 	if ( next )
 		next->prev = this;
 
+	// TODO: could probably store this pointer and do book-keeping in
+	// DataBlockList instead
 	reassembler = reass;
 	reassembler->size_of_all_blocks += size;
 
@@ -34,36 +37,246 @@ DataBlock::DataBlock(Reassembler* reass, const u_char* data,
 	Reassembler::total_size += pad_size(size) + padded_sizeof(DataBlock);
 	}
 
-uint64_t Reassembler::total_size = 0;
+void DataBlockList::Size(uint64_t seq_cutoff, uint64_t* below, uint64_t* above) const
-uint64_t Reassembler::sizes[REASSEM_NUM];
+	{
+	// TODO: just have book-keeping to track this info and avoid iterating ?
+	for ( auto b = head; b; b = b->next )
+		{
+		if ( b->seq <= seq_cutoff )
+			*above += b->Size();
+		else
+			*below += b->Size();
+		}
+	}
+
+void DataBlockList::Clear()
+	{
+	while ( head )
+		{
+		auto next = head->next;
+		delete head;
+		head = next;
+		}
+	}
+
+void DataBlockList::Add(DataBlock* block, uint64_t limit)
+	{
+	++total_blocks;
+	block->next = nullptr;
+
+	if ( tail )
+		{
+		block->prev = tail;
+		tail->next = block;
+		}
+	else
+		{
+		block->prev = nullptr;
+		head = tail = block;
+		}
+
+	while ( head && total_blocks > limit )
+		{
+		auto next = head->next;
+		delete head;
+		head = next;
+		--total_blocks;
+		}
+	}
+
+DataBlock* DataBlockList::Insert(uint64_t seq, uint64_t upper,
+                                 const u_char* data, Reassembler* reass,
+                                 DataBlock* start)
+    {
+	// TODO: can probably do a lot better at finding the right insertion location
+
+	// Empty list.
+	if ( ! head )
+		{
+		head = tail = new DataBlock(reass, data, upper - seq, seq, 0, 0);
+		++total_blocks;
+		return head;
+		}
+
+	// Special check for the common case of appending to the end.
+	if ( tail && seq == tail->upper )
+		{
+		tail = new DataBlock(reass, data, upper - seq, seq, tail, 0);
+		++total_blocks;
+		return tail;
+		}
+
+	auto b = start ? start : head;
+
+	// Find the first block that doesn't come completely before the
+	// new data.
+	while ( b->next && b->upper <= seq )
+		b = b->next;
+
+	if ( b->upper <= seq )
+		{
+		// b is the last block, and it comes completely before
+		// the new block.
+		tail = new DataBlock(reass, data, upper - seq, seq, b, 0);
+		++total_blocks;
+		return tail;
+		}
+
+	DataBlock* new_b = 0;
+
+	if ( upper <= b->seq )
+		{
+		// The new block comes completely before b.
+		new_b = new DataBlock(reass, data, upper - seq, seq, b->prev, b);
+		++total_blocks;
+
+		if ( b == head )
+			head = new_b;
+
+		return new_b;
+		}
+
+	// The blocks overlap.
+	if ( seq < b->seq )
+		{
+		// The new block has a prefix that comes before b.
+		uint64_t prefix_len = b->seq - seq;
+		new_b = new DataBlock(reass, data, prefix_len, seq, b->prev, b);
+		++total_blocks;
+
+		if ( b == head )
+			head = new_b;
+
+		data += prefix_len;
+		seq += prefix_len;
+		}
+	else
+		new_b = b;
+
+	uint64_t overlap_start = seq;
+	uint64_t overlap_offset = overlap_start - b->seq;
+	uint64_t new_b_len = upper - seq;
+	uint64_t b_len = b->upper - overlap_start;
+	uint64_t overlap_len = min(new_b_len, b_len);
+
+	if ( overlap_len < new_b_len )
+		{
+		// Recurse to resolve remainder of the new data.
+		data += overlap_len;
+		seq += overlap_len;
+
+		if ( new_b == b )
+			new_b = Insert(seq, upper, data, reass, b);
+		else
+			Insert(seq, upper, data, reass, b);
+		}
+
+	if ( new_b->prev == tail )
+		tail = new_b;
+
+	return new_b;
+    }
+
+uint64_t DataBlockList::Trim(uint64_t seq, Reassembler* reass,
+                             uint64_t max_old, DataBlockList* old_list)
+	{
+	uint64_t num_missing = 0;
+
+	// Do this accounting before looking for Undelivered data,
+	// since that will alter last_reassem_seq.
+
+	if ( head )
+		{
+		if ( head->seq > reass->LastReassemSeq() )
+			// An initial hole.
+			num_missing += head->seq - reass->LastReassemSeq();
+		}
+
+	else if ( seq > reass->LastReassemSeq() )
+		{ // Trimming data we never delivered.
+		if ( ! head )
+			// We won't have any accounting based on blocks
+			// for this hole.
+			num_missing += seq - reass->LastReassemSeq();
+		}
+
+	if ( seq > reass->LastReassemSeq() )
+		{
+		// We're trimming data we never delivered.
+		reass->Undelivered(seq);
+		}
+
+	// TODO: better loop ?
+
+	while ( head && head->upper <= seq )
+		{
+		DataBlock* b = head->next;
+
+		if ( b && b->seq <= seq )
+			{
+			if ( head->upper != b->seq )
+				num_missing += b->seq - head->upper;
+			}
+		else
+			{
+			// No more blocks - did this one make it to seq?
+			// Second half of test is for acks of FINs, which
+			// don't get entered into the sequence space.
+			if ( head->upper != seq && head->upper != seq - 1 )
+				num_missing += seq - head->upper;
+			}
+
+		if ( max_old )
+			old_list->Add(head, max_old);
+		else
+			delete head;
+
+		head = b;
+		}
+
+	if ( head )
+		{
+		head->prev = 0;
+
+		// If we skipped over some undeliverable data, then
+		// it's possible that this block is now deliverable.
+		// Give it a try.
+		if ( head->seq == reass->LastReassemSeq() )
+			reass->BlockInserted(head);
+		}
+	else
+		tail = 0;
+
+	reass->SetTrimSeq(seq);
+	return num_missing;
+	}
 
 Reassembler::Reassembler(uint64_t init_seq, ReassemblerType reassem_type)
-	:  blocks(), last_block(), old_blocks(), last_old_block(),
+	: last_reassem_seq(init_seq), trim_seq(init_seq),
-	  last_reassem_seq(init_seq), trim_seq(init_seq),
+	  max_old_blocks(0), size_of_all_blocks(0),
-	  max_old_blocks(0), total_old_blocks(0), size_of_all_blocks(0),
 	  rtype(reassem_type)
 	{
 	}
 
-Reassembler::~Reassembler()
+void Reassembler::CheckOverlap(const DataBlockList& list,
+                               uint64_t seq, uint64_t len,
+                               const u_char* data)
 	{
-	ClearBlocks();
+	if ( list.Empty() )
-	ClearOldBlocks();
-	}
-
-void Reassembler::CheckOverlap(DataBlock *head, DataBlock *tail,
-					uint64_t seq, uint64_t len, const u_char* data)
-	{
-	if ( ! head || ! tail )
 		return;
 
+	auto head = list.Head();
+	auto tail = list.Tail();
+
+	// TODO: better way to iterate ?
+
 	if ( seq == tail->upper )
 		// Special case check for common case of appending to the end.
 		return;
 
 	uint64_t upper = (seq + len);
 
-	for ( DataBlock* b = head; b; b = b->next )
+	for ( auto b = head; b; b = b->next )
 		{
 		uint64_t nseq = seq;
 		uint64_t nupper = upper;
@@ -99,13 +312,13 @@ void Reassembler::NewBlock(double t, uint64_t seq, uint64_t len, const u_char* d
 
 	uint64_t upper_seq = seq + len;
 
-	CheckOverlap(old_blocks, last_old_block, seq, len, data);
+	CheckOverlap(old_block_list, seq, len, data);
 
 	if ( upper_seq <= trim_seq )
 		// Old data, don't do any work for it.
 		return;
 
-	CheckOverlap(blocks, last_block, seq, len, data);
+	CheckOverlap(block_list, seq, len, data);
 
 	if ( seq < trim_seq )
 		{ // Partially old data, just keep the good stuff.
@@ -116,139 +329,23 @@ void Reassembler::NewBlock(double t, uint64_t seq, uint64_t len, const u_char* d
 		len -= amount_old;
 		}
 
-	DataBlock* start_block;
+	auto start_block = block_list.Insert(seq, upper_seq, data, this);;
-
-	if ( ! blocks )
-		blocks = last_block = start_block =
-			new DataBlock(this, data, len, seq, 0, 0);
-	else
-		start_block = AddAndCheck(blocks, seq, upper_seq, data);
-
 	BlockInserted(start_block);
 	}
 
 uint64_t Reassembler::TrimToSeq(uint64_t seq)
 	{
-	uint64_t num_missing = 0;
+	return block_list.Trim(seq, this, max_old_blocks, &old_block_list);
-
-	// Do this accounting before looking for Undelivered data,
-	// since that will alter last_reassem_seq.
-
-	if ( blocks )
-		{
-		if ( blocks->seq > last_reassem_seq )
-			// An initial hole.
-			num_missing += blocks->seq - last_reassem_seq;
-		}
-
-	else if ( seq > last_reassem_seq )
-		{ // Trimming data we never delivered.
-		if ( ! blocks )
-			// We won't have any accounting based on blocks
-			// for this hole.
-			num_missing += seq - last_reassem_seq;
-		}
-
-	if ( seq > last_reassem_seq )
-		{
-		// We're trimming data we never delivered.
-		Undelivered(seq);
-		}
-
-	while ( blocks && blocks->upper <= seq )
-		{
-		DataBlock* b = blocks->next;
-
-		if ( b && b->seq <= seq )
-			{
-			if ( blocks->upper != b->seq )
-				num_missing += b->seq - blocks->upper;
-			}
-		else
-			{
-			// No more blocks - did this one make it to seq?
-			// Second half of test is for acks of FINs, which
-			// don't get entered into the sequence space.
-			if ( blocks->upper != seq && blocks->upper != seq - 1 )
-				num_missing += seq - blocks->upper;
-			}
-
-		if ( max_old_blocks )
-			{
-			// Move block over to old_blocks queue.
-			blocks->next = 0;
-
-			if ( last_old_block )
-				{
-				blocks->prev = last_old_block;
-				last_old_block->next = blocks;
-				}
-			else
-				{
-				blocks->prev = 0;
-				old_blocks = blocks;
-				}
-
-			last_old_block = blocks;
-			total_old_blocks++;
-
-			while ( old_blocks && total_old_blocks > max_old_blocks )
-				{
-				DataBlock* next = old_blocks->next;
-				delete old_blocks;
-				old_blocks = next;
-				total_old_blocks--;
-				}
-			}
-
-		else
-			delete blocks;
-
-		blocks = b;
-		}
-
-	if ( blocks )
-		{
-		blocks->prev = 0;
-
-		// If we skipped over some undeliverable data, then
-		// it's possible that this block is now deliverable.
-		// Give it a try.
-		if ( blocks->seq == last_reassem_seq )
-			BlockInserted(blocks);
-		}
-	else
-		last_block = 0;
-
-	if ( seq > trim_seq )
-		// seq is further ahead in the sequence space.
-		trim_seq = seq;
-
-	return num_missing;
 	}
 
 void Reassembler::ClearBlocks()
 	{
-	while ( blocks )
+	block_list.Clear();
-		{
-		DataBlock* b = blocks->next;
-		delete blocks;
-		blocks = b;
-		}
-
-	last_block = 0;
 	}
 
 void Reassembler::ClearOldBlocks()
 	{
-	while ( old_blocks )
+	old_block_list.Clear();
-		{
-		DataBlock* b = old_blocks->next;
-		delete old_blocks;
-		old_blocks = b;
-		}
-
-	last_old_block = 0;
 	}
 
 uint64_t Reassembler::TotalSize() const
@@ -267,89 +364,6 @@ void Reassembler::Undelivered(uint64_t up_to_seq)
 	last_reassem_seq = up_to_seq;
 	}
 
-DataBlock* Reassembler::AddAndCheck(DataBlock* b, uint64_t seq, uint64_t upper,
-					const u_char* data)
-	{
-	if ( DEBUG_reassem )
-		{
-		DEBUG_MSG("%.6f Reassembler::AddAndCheck seq=%" PRIu64", upper=%" PRIu64"\n",
-		          network_time, seq, upper);
-		}
-
-	// Special check for the common case of appending to the end.
-	if ( last_block && seq == last_block->upper )
-		{
-		last_block = new DataBlock(this, data, upper - seq,
-		                           seq, last_block, 0);
-		return last_block;
-		}
-
-	// Find the first block that doesn't come completely before the
-	// new data.
-	while ( b->next && b->upper <= seq )
-		b = b->next;
-
-	if ( b->upper <= seq )
-		{
-		// b is the last block, and it comes completely before
-		// the new block.
-		last_block = new DataBlock(this, data, upper - seq,
-		                           seq, b, 0);
-		return last_block;
-		}
-
-	DataBlock* new_b = 0;
-
-	if ( upper <= b->seq )
-		{
-		// The new block comes completely before b.
-		new_b = new DataBlock(this, data, upper - seq, seq,
-		                      b->prev, b);
-		if ( b == blocks )
-			blocks = new_b;
-		return new_b;
-		}
-
-	// The blocks overlap.
-	if ( seq < b->seq )
-		{
-		// The new block has a prefix that comes before b.
-		uint64_t prefix_len = b->seq - seq;
-		new_b = new DataBlock(this, data, prefix_len, seq,
-		                      b->prev, b);
-		if ( b == blocks )
-			blocks = new_b;
-
-		data += prefix_len;
-		seq += prefix_len;
-		}
-	else
-		new_b = b;
-
-	uint64_t overlap_start = seq;
-	uint64_t overlap_offset = overlap_start - b->seq;
-	uint64_t new_b_len = upper - seq;
-	uint64_t b_len = b->upper - overlap_start;
-	uint64_t overlap_len = min(new_b_len, b_len);
-
-	if ( overlap_len < new_b_len )
-		{
-		// Recurse to resolve remainder of the new data.
-		data += overlap_len;
-		seq += overlap_len;
-
-		if ( new_b == b )
-			new_b = AddAndCheck(b, seq, upper, data);
-		else
-			(void) AddAndCheck(b, seq, upper, data);
-		}
-
-	if ( new_b->prev == last_block )
-		last_block = new_b;
-
-	return new_b;
-	}
-
 uint64_t Reassembler::MemoryAllocation(ReassemblerType rtype)
 	{
 	return Reassembler::sizes[rtype];

src/Reassem.h

@@ -38,10 +38,48 @@ public:
 	Reassembler* reassembler; // Non-owning pointer back to parent.
 };
 
+// TODO: add comments
+class DataBlockList {
+public:
+
+	~DataBlockList()
+		{ Clear(); }
+
+	const DataBlock* Head() const
+		{ return head; }
+
+	const DataBlock* Tail() const
+		{ return tail; }
+
+	bool Empty() const
+		{ return head == nullptr; };
+
+	size_t NumBlocks() const
+		{ return total_blocks; };
+
+	void Size(uint64_t seq_cutoff, uint64_t* below, uint64_t* above) const;
+
+	void Clear();
+
+	DataBlock* Insert(uint64_t seq, uint64_t upper, const u_char* data,
+	                  Reassembler* reass, DataBlock* start = nullptr);
+
+	void Add(DataBlock* block, uint64_t limit);
+
+	uint64_t Trim(uint64_t seq, Reassembler* reass,
+	              uint64_t max_old, DataBlockList* old_list);
+
+private:
+
+	DataBlock* head = nullptr;
+	DataBlock* tail = nullptr;
+	size_t total_blocks = 0;
+};
+
 class Reassembler : public BroObj {
 public:
 	Reassembler(uint64_t init_seq, ReassemblerType reassem_type = REASSEM_UNKNOWN);
-	~Reassembler() override;
+	~Reassembler() override	{}
 
 	void NewBlock(double t, uint64_t seq, uint64_t len, const u_char* data);
 
@@ -53,9 +91,17 @@ public:
 	void ClearBlocks();
 	void ClearOldBlocks();
 
-	int HasBlocks() const		{ return blocks != 0; }
+	int HasBlocks() const
+		{ return ! block_list.Empty(); }
+
 	uint64_t LastReassemSeq() const	{ return last_reassem_seq; }
 
+	uint64_t TrimSeq() const
+		{ return trim_seq; }
+
+	void SetTrimSeq(uint64_t seq)
+		{ if ( seq > trim_seq ) trim_seq = seq; }
+
 	uint64_t TotalSize() const;	// number of bytes buffered up
 
 	void Describe(ODesc* d) const override;
@@ -72,28 +118,23 @@ protected:
 	Reassembler()	{ }
 
 	friend class DataBlock;
+	friend class DataBlockList;
 
 	virtual void Undelivered(uint64_t up_to_seq);
 
-	virtual void BlockInserted(DataBlock* b) = 0;
+	virtual void BlockInserted(const DataBlock* b) = 0;
 	virtual void Overlap(const u_char* b1, const u_char* b2, uint64_t n) = 0;
 
-	DataBlock* AddAndCheck(DataBlock* b, uint64_t seq,
+	void CheckOverlap(const DataBlockList& list,
-				uint64_t upper, const u_char* data);
-
-	void CheckOverlap(DataBlock *head, DataBlock *tail,
 				uint64_t seq, uint64_t len, const u_char* data);
 
-	DataBlock* blocks;
+	DataBlockList block_list;
-	DataBlock* last_block;
+	DataBlockList old_block_list;
-
-	DataBlock* old_blocks;
-	DataBlock* last_old_block;
 
+	// TODO: maybe roll some of these stats into DataBlockList ?
 	uint64_t last_reassem_seq;
 	uint64_t trim_seq;	// how far we've trimmed
 	uint32_t max_old_blocks;
-	uint32_t total_old_blocks;
 	uint64_t size_of_all_blocks;
 
 	ReassemblerType rtype;

src/analyzer/protocol/tcp/TCP_Reassembler.cc

@@ -66,7 +66,9 @@ void TCP_Reassembler::Done()
 
 	if ( record_contents_file )
 		{ // Record any undelivered data.
-		if ( blocks && last_reassem_seq < last_block->upper )
+		auto last_block = block_list.Tail();
+
+		if ( ! block_list.Empty() && last_reassem_seq < last_block->upper )
 			RecordToSeq(last_reassem_seq, last_block->upper,
 					record_contents_file);
 
@@ -78,15 +80,17 @@ void TCP_Reassembler::SizeBufferedData(uint64_t& waiting_on_hole,
 					uint64_t& waiting_on_ack) const
 	{
 	waiting_on_hole = waiting_on_ack = 0;
-	for ( DataBlock* b = blocks; b; b = b->next )
+	block_list.Size(last_reassem_seq, &waiting_on_ack, &waiting_on_hole);
-		{
+	}
-		if ( b->seq <= last_reassem_seq )
+
-			// We must have delivered this block, but
+uint64_t TCP_Reassembler::NumUndeliveredBytes() const
-			// haven't yet trimmed it.
+	{
-			waiting_on_ack += b->Size();
+	auto last_block = block_list.Tail();
-		else
+
-			waiting_on_hole += b->Size();
+	if ( last_block )
-		}
+		return last_block->upper - last_reassem_seq;
+
+	return 0;
 	}
 
 void TCP_Reassembler::SetContentsFile(BroFile* f)
@@ -102,8 +106,8 @@ void TCP_Reassembler::SetContentsFile(BroFile* f)
 		Unref(record_contents_file);
 	else
 		{
-		if ( blocks )
+		if ( ! block_list.Empty() )
-			RecordToSeq(blocks->seq, last_reassem_seq, f);
+			RecordToSeq(block_list.Head()->seq, last_reassem_seq, f);
 		}
 
 	Ref(f);
@@ -231,9 +235,10 @@ void TCP_Reassembler::Undelivered(uint64_t up_to_seq)
 		if ( ! skip_deliveries )
 			{
 			// If we have blocks that begin below up_to_seq, deliver them.
-			DataBlock* b = blocks;
+			auto b = block_list.Head();
 			while ( b )
 				{
+				// TODO: better way to do this iteration ?
 				if ( b->seq < last_reassem_seq )
 					{
 					// Already delivered this block.
@@ -253,7 +258,7 @@ void TCP_Reassembler::Undelivered(uint64_t up_to_seq)
 				BlockInserted(b);
 				// Inserting a block may cause trimming of what's buffered,
 				// so have to assume 'b' is invalid, hence re-assign to start.
-				b = blocks;
+				b = block_list.Head();
 				}
 
 			if ( up_to_seq > last_reassem_seq )
@@ -277,9 +282,10 @@ void TCP_Reassembler::Undelivered(uint64_t up_to_seq)
 
 void TCP_Reassembler::MatchUndelivered(uint64_t up_to_seq, bool use_last_upper)
 	{
-	if ( ! blocks || ! rule_matcher )
+	if ( block_list.Empty() || ! rule_matcher )
 		return;
 
+	auto last_block = block_list.Tail();
 	ASSERT(last_block);
 	if ( use_last_upper )
 		up_to_seq = last_block->upper;
@@ -298,8 +304,9 @@ void TCP_Reassembler::MatchUndelivered(uint64_t up_to_seq, bool use_last_upper)
 	// Skip blocks that are already delivered (but not ACK'ed).
 	// Question: shall we instead keep a pointer to the first undelivered
 	// block?
-	DataBlock* b;
+	// TODO: better way to iterate ?
-	for ( b = blocks; b && b->upper <= last_reassem_seq; b = b->next )
+	const DataBlock* b;
+	for ( b = block_list.Head(); b && b->upper <= last_reassem_seq; b = b->next )
 	      tcp_analyzer->Conn()->Match(Rule::PAYLOAD, b->block, b->Size(),
 						false, false, IsOrig(), false);
 
@@ -308,7 +315,8 @@ void TCP_Reassembler::MatchUndelivered(uint64_t up_to_seq, bool use_last_upper)
 
 void TCP_Reassembler::RecordToSeq(uint64_t start_seq, uint64_t stop_seq, BroFile* f)
 	{
-	DataBlock* b = blocks;
+	auto b = block_list.Head();
+	// TODO: better way to iterate ?
 	// Skip over blocks up to the start seq.
 	while ( b && b->upper <= start_seq )
 		b = b->next;
@@ -333,7 +341,7 @@ void TCP_Reassembler::RecordToSeq(uint64_t start_seq, uint64_t stop_seq, BroFile
 			RecordGap(last_seq, stop_seq, f);
 	}
 
-void TCP_Reassembler::RecordBlock(DataBlock* b, BroFile* f)
+void TCP_Reassembler::RecordBlock(const DataBlock* b, BroFile* f)
 	{
 	if ( f->Write((const char*) b->block, b->Size()) )
 		return;
@@ -367,7 +375,7 @@ void TCP_Reassembler::RecordGap(uint64_t start_seq, uint64_t upper_seq, BroFile*
 		}
 	}
 
-void TCP_Reassembler::BlockInserted(DataBlock* start_block)
+void TCP_Reassembler::BlockInserted(const DataBlock* start_block)
 	{
 	if ( start_block->seq > last_reassem_seq ||
 	     start_block->upper <= last_reassem_seq )
@@ -379,7 +387,8 @@ void TCP_Reassembler::BlockInserted(DataBlock* start_block)
 	// new stuff off into its own block(s), but in the following
 	// loop we have to take care not to deliver already-delivered
 	// data.
-	for ( DataBlock* b = start_block;
+	// TODO: better way to iterate ?
+	for ( auto b = start_block;
 	      b && b->seq <= last_reassem_seq; b = b->next )
 		{
 		if ( b->seq == last_reassem_seq )

src/analyzer/protocol/tcp/TCP_Reassembler.h

@@ -44,13 +44,7 @@ public:
 	// How much data is pending delivery since it's not yet reassembled.
 	// Includes the data due to holes (so this value is a bit different
 	// from waiting_on_hole above; and is computed in a different fashion).
-	uint64_t NumUndeliveredBytes() const
+	uint64_t NumUndeliveredBytes() const;
-		{
-		if ( last_block )
-			return last_block->upper - last_reassem_seq;
-		else
-			return 0;
-		}
 
 	void SetContentsFile(BroFile* f);
 	BroFile* GetContentsFile() const	{ return record_contents_file; }
@@ -93,10 +87,10 @@ private:
 	void Gap(uint64_t seq, uint64_t len);
 
 	void RecordToSeq(uint64_t start_seq, uint64_t stop_seq, BroFile* f);
-	void RecordBlock(DataBlock* b, BroFile* f);
+	void RecordBlock(const DataBlock* b, BroFile* f);
 	void RecordGap(uint64_t start_seq, uint64_t upper_seq, BroFile* f);
 
-	void BlockInserted(DataBlock* b) override;
+	void BlockInserted(const DataBlock* b) override;
 	void Overlap(const u_char* b1, const u_char* b2, uint64_t n) override;
 
 	TCP_Endpoint* endp;

src/file_analysis/FileReassembler.cc

@@ -26,6 +26,8 @@ uint64_t FileReassembler::Flush()
 	if ( flushing )
 		return 0;
 
+	auto last_block = block_list.Tail();
+
 	if ( last_block )
 		{
 		// This is expected to call back into FileReassembler::Undelivered().
@@ -50,13 +52,14 @@ uint64_t FileReassembler::FlushTo(uint64_t sequence)
 	return rval;
 	}
 
-void FileReassembler::BlockInserted(DataBlock* start_block)
+void FileReassembler::BlockInserted(const DataBlock* start_block)
 	{
 	if ( start_block->seq > last_reassem_seq ||
 	     start_block->upper <= last_reassem_seq )
 		return;
 
-	for ( DataBlock* b = start_block;
+	// TODO: better way to iterate ?
+	for ( auto b = start_block;
 	      b && b->seq <= last_reassem_seq; b = b->next )
 		{
 		if ( b->seq == last_reassem_seq )
@@ -74,8 +77,9 @@ void FileReassembler::BlockInserted(DataBlock* start_block)
 void FileReassembler::Undelivered(uint64_t up_to_seq)
 	{
 	// If we have blocks that begin below up_to_seq, deliver them.
-	DataBlock* b = blocks;
+	const DataBlock* b = block_list.Head();
 
+	// TODO: better way to iterate ?
 	while ( b )
 		{
 		if ( b->seq < last_reassem_seq )
@@ -96,7 +100,7 @@ void FileReassembler::Undelivered(uint64_t up_to_seq)
 		BlockInserted(b);
 		// Inserting a block may cause trimming of what's buffered,
 		// so have to assume 'b' is invalid, hence re-assign to start.
-		b = blocks;
+		b = block_list.Head();
 		}
 
 	if ( up_to_seq > last_reassem_seq )

src/file_analysis/FileReassembler.h

@@ -51,7 +51,7 @@ protected:
 	FileReassembler();
 
 	void Undelivered(uint64_t up_to_seq) override;
-	void BlockInserted(DataBlock* b) override;
+	void BlockInserted(const DataBlock* b) override;
 	void Overlap(const u_char* b1, const u_char* b2, uint64_t n) override;
 
 	File* the_file;