GH-1186: Remove Packet::hdr_size and uses of it.

This change also removes Packet::IP(), since Packet now contains an ip_hdr member
that points at the IP header if it exists.

src/IP.h

@@ -318,7 +318,7 @@ public:
 	 * already checked that the static IPv6 header is not truncated.  If
 	 * the packet contains extension headers and they are truncated, that can
 	 * be checked afterwards by comparing \a len with \a TotalLen.  E.g.
-	 * NetSessions::DoNextPacket does this to skip truncated packets.
+	 * The IP packet analyzer does this to skip truncated packets.
 	 * @param arg_ip6 pointer to memory containing an IPv6 packet.
 	 * @param arg_del whether to take ownership of \a arg_ip6 pointer's memory.
 	 * @param len the packet's length in bytes.

src/Sessions.cc

@@ -81,11 +81,10 @@ void NetSessions::NextPacket(double t, Packet* pkt)
 	packet_mgr->ProcessPacket(pkt);
 	}
 
-void NetSessions::DoNextPacket(double t, const Packet* pkt)
+void NetSessions::ProcessTransportLayer(double t, const Packet* pkt, size_t remaining)
 	{
 	const std::unique_ptr<IP_Hdr>& ip_hdr = pkt->ip_hdr;
 
-	uint32_t caplen = pkt->cap_len - pkt->hdr_size;
 	uint32_t len = ip_hdr->TotalLen();
 	uint16_t ip_hdr_len = ip_hdr->HdrLen();
 
@@ -95,18 +94,11 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt)
 		return;
 		}
 
-	if ( caplen < ip_hdr_len )
-		{
-		sessions->Weird("truncated_IP", pkt);
-		return;
-		}
-
 	len -= ip_hdr_len;	// remove IP header
-	caplen -= ip_hdr_len;	// remove IP header
 
 	int proto = ip_hdr->NextProto();
 
-	if ( CheckHeaderTrunc(proto, len, caplen, pkt) )
+	if ( CheckHeaderTrunc(proto, len, remaining, pkt) )
 		return;
 
 	const u_char* data = ip_hdr->Payload();
@@ -232,7 +224,7 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt)
 		conn->EnqueueEvent(new_packet, nullptr, conn->ConnVal(), pkt_hdr_val ?
 		                   std::move(pkt_hdr_val) : ip_hdr->ToPktHdrVal());
 
-	conn->NextPacket(t, is_orig, ip_hdr.get(), len, caplen, data,
+	conn->NextPacket(t, is_orig, ip_hdr.get(), len, remaining, data,
 	                 record_packet, record_content, pkt);
 
 	// We skip this block for reassembled packets because the pointer

src/Sessions.h

@@ -93,8 +93,13 @@ public:
 	 * Main entry point for processing packets destined for session analyzers. This
 	 * method is called by the packet analysis manager when after it has processed
 	 * an IP-based packet, and shouldn't be called directly from other places.
+	 *
+	 * @param t The timestamp for this packet.
+	 * @param pkt The packet being processed.
+	 * @param len The number of bytes that haven't been processed yet by packet
+	 * analysis.
 	 */
-	void DoNextPacket(double t, const Packet *pkt);
+	void ProcessTransportLayer(double t, const Packet *pkt, size_t len);
 
 	/**
 	 * Returns a wrapper IP_Hdr object if \a pkt appears to be a valid IPv4

src/analyzer/protocol/vxlan/VXLAN.cc

@@ -75,25 +75,16 @@ void VXLAN_Analyzer::DeliverPacket(int len, const u_char* data, bool orig,
 	Packet pkt(DLT_EN10MB, &ts, caplen, len, data);
 	pkt.encap = outer;
 
-	packet_mgr->ProcessPacket(&pkt);
+	if ( ! packet_mgr->ProcessInnerPacket(&pkt) )
-
-	if ( ! pkt.l2_valid )
 		{
-		ProtocolViolation("VXLAN invalid inner ethernet frame",
+		ProtocolViolation("VXLAN invalid inner packet");
-		                  (const char*) data, len);
 		return;
 		}
 
-	data += pkt.hdr_size;
+	// This isn't really an error. It's just that the inner packet wasn't an IP packet (like ARP).
-	len -= pkt.hdr_size;
+	// Just return without reporting a violation.
-	caplen -= pkt.hdr_size;
-
 	if ( ! pkt.ip_hdr )
-		{
-		ProtocolViolation("Truncated VXLAN or invalid inner IP",
-		                  (const char*) data, len);
 		return;
-		}
 
 	ProtocolConfirmation();
 

src/iosource/Packet.cc

@@ -49,7 +49,6 @@ void Packet::Init(int arg_link_type, pkt_timeval *arg_ts, uint32_t arg_caplen,
 	dump_packet = false;
 
 	time = ts.tv_sec + double(ts.tv_usec) / 1e6;
-	hdr_size = 0;
 	eth_type = 0;
 	vlan = 0;
 	inner_vlan = 0;
@@ -84,11 +83,6 @@ Packet::~Packet()
 		delete [] data;
 	}
 
-const IP_Hdr Packet::IP() const
-	{
-	return IP_Hdr((struct ip *) (data + hdr_size), false);
-	}
-
 void Packet::Weird(const char* name)
 	{
 	sessions->Weird(name, this);

src/iosource/Packet.h

@@ -114,12 +114,6 @@ public:
 	          uint32_t len, const u_char *data, bool copy = false,
 	          std::string tag = "");
 
-	/**
-	 * Interprets the Layer 3 of the packet as IP and returns a
-	 * corresponding object.
-	 */
-	const IP_Hdr IP() const;
-
 	/**
 	 * Returns a \c raw_pkt_hdr RecordVal, which includes layer 2 and
 	 * also everything in IP_Hdr (i.e., IP4/6 + TCP/UDP/ICMP).
@@ -156,11 +150,6 @@ public:
 	// These are computed from Layer 2 data. These fields are only valid if
 	// l2_valid returns true.
 
-	/**
-	 * Layer 2 header size. Valid iff l2_valid is true.
-	 */
-	uint32_t hdr_size;
-
 	/**
 	 * Layer 3 protocol identified (if any). Valid iff l2_valid is true.
 	 */

src/packet_analysis/protocol/ip/IP.cc

@@ -39,10 +39,7 @@ bool IPAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet)
 		return false;
 		}
 
-	// TODO: i feel like this could be generated as we move along the header hierarchy.
+	int32_t hdr_size = static_cast<int32_t>(data - packet->data);
-	// TODO: the sessions code expects that the header size does not include the ip header. Should
-	// this change?
-	packet->hdr_size = static_cast<int32_t>(data - packet->data);
 
 	// Cast the current data pointer to an IP header pointer so we can use it to get some
 	// data about the header.
@@ -82,10 +79,10 @@ bool IPAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet)
 		sessions->Weird("ip_hdr_len_zero", packet);
 
 		// Cope with the zero'd out ip_len field by using the caplen.
-		total_len = packet->cap_len - packet->hdr_size;
+		total_len = packet->cap_len - hdr_size;
 		}
 
-	if ( packet->len < total_len + packet->hdr_size )
+	if ( packet->len < total_len + hdr_size )
 		{
 		sessions->Weird("truncated_IPv6", packet);
 		return false;
@@ -158,7 +155,7 @@ bool IPAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet)
 		else
 			{
 			f = detail::fragment_mgr->NextFragment(run_state::processing_start_time, packet->ip_hdr,
-			                                       packet->data + packet->hdr_size);
+			                                       packet->data + hdr_size);
 			std::unique_ptr<IP_Hdr> ih = f->ReassembledPkt();
 
 			if ( ! ih )
@@ -173,7 +170,7 @@ bool IPAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet)
 
 			len = total_len = packet->ip_hdr->TotalLen();
 			ip_hdr_len = packet->ip_hdr->HdrLen();
-			packet->cap_len = total_len + packet->hdr_size;
+			packet->cap_len = total_len + hdr_size;
 
 			if ( ip_hdr_len > total_len )
 				{
@@ -243,7 +240,7 @@ bool IPAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet)
 	case IPPROTO_ICMPV6:
 		DBG_LOG(DBG_PACKET_ANALYSIS, "Analysis in %s succeeded, next layer identifier is %#x.",
 		        GetAnalyzerName(), proto);
-		sessions->DoNextPacket(run_state::processing_start_time, packet);
+		sessions->ProcessTransportLayer(run_state::processing_start_time, packet, len);
 		break;
 	case IPPROTO_NONE:
 		// If the packet is encapsulated in Teredo, then it was a bubble and

src/packet_analysis/protocol/iptunnel/IPTunnel.cc

@@ -128,7 +128,8 @@ bool IPTunnelAnalyzer::ProcessEncapsulatedPacket(double t, const Packet* pkt,
 	auto outer = prev ? prev : std::make_shared<EncapsulationStack>();
 	outer->Add(ec);
 
-	// Construct fake packet for DoNextPacket
+	// Construct fake packet containing the inner packet so it can be processed
+	// like a normal one.
 	Packet p;
 	p.Init(DLT_RAW, &ts, caplen, len, data, false, "");
 	p.encap = outer;
@@ -164,7 +165,8 @@ bool IPTunnelAnalyzer::ProcessEncapsulatedPacket(double t, const Packet* pkt,
 	auto outer = prev ? prev : std::make_shared<EncapsulationStack>();
 	outer->Add(ec);
 
-	// Construct fake packet for DoNextPacket
+	// Construct fake packet containing the inner packet so it can be processed
+	// like a normal one.
 	Packet p;
 	p.Init(link_type, &ts, caplen, len, data, false, "");
 	p.encap = outer;

src/packet_analysis/protocol/iptunnel/IPTunnel.h

@@ -30,9 +30,9 @@ public:
 	 *
 	 * @param t Network time.
 	 * @param pkt If the outer pcap header is available, this pointer can be set
-	 *        so that the fake pcap header passed to DoNextPacket will use
+	 *        so the fake pcap header passed to the next analyzer will use the
-	 *        the same timeval.  The caplen and len fields of the fake pcap
+	 *        same timeval. The caplen and len fields of the fake pcap header
-	 *        header are always set to the TotalLength() of \a inner.
+	 *        are always set to the TotalLength() of \a inner.
 	 * @param inner Pointer to IP header wrapper of the inner packet, ownership
 	 *        of the pointer's memory is assumed by this function.
 	 * @param prev Any previous encapsulation stack of the caller, not including
@@ -49,13 +49,13 @@ public:
 	 * packet analysis.
 	 *
 	 * @param t Network time.
-	 * @param pkt  If the outer pcap header is available, this pointer can be
+	 * @param pkt If the outer pcap header is available, this pointer can be set
-	 *        set so that the fake pcap header passed to DoNextPacket will use
+	 *        so the fake pcap header passed to the next analyzer will use the
-	 *        the same timeval.
+	 *        same timeval.
-	 * @param caplen  number of captured bytes remaining
+	 * @param caplen Number of captured bytes remaining
-	 * @param len  number of bytes remaining as claimed by outer framing
+	 * @param len Number of bytes remaining as claimed by outer framing
-	 * @param data  the remaining packet data
+	 * @param data The remaining packet data
-	 * @param link_type  layer 2 link type used for initializing inner packet
+	 * @param link_type Layer 2 link type used for initializing inner packet
 	 * @param prev Any previous encapsulation stack of the caller, not
 	 *        including the most-recently found depth of encapsulation.
 	 * @param ec The most-recently found depth of encapsulation.

src/packet_analysis/protocol/wrapper/Wrapper.cc

@@ -155,8 +155,5 @@ bool WrapperAnalyzer::Analyze(Packet* packet, const uint8_t*& data)
 			}
 		}
 
-	// Calculate how much header we've used up.
-	packet->hdr_size = (data - packet->data);
-
 	return AnalyzeInnerPacket(packet, data, protocol);
 	}