From 7ae94a8c8ac6c6d3ac5278c628cc7e359dacb722 Mon Sep 17 00:00:00 2001
From: Alfonso Acosta <fons@weave.works>
Date: Fri, 27 Jan 2017 10:59:33 +0000
Subject: [PATCH 1/2] DNSSnooper: Support Dot1Q and limit decoding errors

---
 probe/endpoint/dns_snooper_linux_amd64.go | 38 +++++++++++++++++------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/probe/endpoint/dns_snooper_linux_amd64.go b/probe/endpoint/dns_snooper_linux_amd64.go
index d4f1be470..e18bf7348 100644
--- a/probe/endpoint/dns_snooper_linux_amd64.go
+++ b/probe/endpoint/dns_snooper_linux_amd64.go
@@ -15,15 +15,17 @@ import (
 )
 
 const (
-	bufSize              = 8 * 1024 * 1024 // 8MB
-	maxReverseDNSrecords = 10000
+	bufSize                 = 8 * 1024 * 1024 // 8MB
+	maxReverseDNSrecords    = 10000
+	maxLogsPerDecodingError = 4
 )
 
 // DNSSnooper is a snopper of DNS queries
 type DNSSnooper struct {
-	stop            chan struct{}
-	pcapHandle      *pcap.Handle
-	reverseDNSCache gcache.Cache
+	stop                chan struct{}
+	pcapHandle          *pcap.Handle
+	reverseDNSCache     gcache.Cache
+	decodingErrorCounts map[string]uint64 // for limiting
 }
 
 // NewDNSSnooper creates a new snooper of DNS queries
@@ -35,9 +37,10 @@ func NewDNSSnooper() (*DNSSnooper, error) {
 	reverseDNSCache := gcache.New(maxReverseDNSrecords).LRU().Build()
 
 	s := &DNSSnooper{
-		stop:            make(chan struct{}),
-		pcapHandle:      pcapHandle,
-		reverseDNSCache: reverseDNSCache,
+		stop:                make(chan struct{}),
+		pcapHandle:          pcapHandle,
+		reverseDNSCache:     reverseDNSCache,
+		decodingErrorCounts: map[string]uint64{},
 	}
 	go s.run()
 	return s, nil
@@ -163,11 +166,12 @@ func (s *DNSSnooper) run() {
 		ip4           layers.IPv4
 		ip6           layers.IPv6
 		eth           layers.Ethernet
+		dot1q         layers.Dot1Q
 		sll           layers.LinuxSLL
 	)
 
 	// assumes that the "any" interface is being used (see https://wiki.wireshark.org/SLL)
-	packetParser := gopacket.NewDecodingLayerParser(layers.LayerTypeLinuxSLL, &sll, &eth, &ip4, &ip6, &udp, &tcp, &dns)
+	packetParser := gopacket.NewDecodingLayerParser(layers.LayerTypeLinuxSLL, &sll, &dot1q, &eth, &ip4, &ip6, &udp, &tcp, &dns)
 
 	for {
 		select {
@@ -190,7 +194,7 @@ func (s *DNSSnooper) run() {
 		if err := packetParser.DecodeLayers(packet, &decodedLayers); err != nil {
 			// LayerTypePayload indicates the TCP payload has non-DNS data, which we are not interested in
 			if layer, ok := err.(gopacket.UnsupportedLayerType); !ok || gopacket.LayerType(layer) != gopacket.LayerTypePayload {
-				log.Errorf("DNSSnooper: error decoding packet: %s", err)
+				s.handleDecodingError(err)
 			}
 			continue
 		}
@@ -203,6 +207,20 @@ func (s *DNSSnooper) run() {
 	}
 }
 
+// handleDecodeError logs errors up to the maximum allowed count
+func (s *DNSSnooper) handleDecodingError(err error) {
+	str := err.Error()
+	count := s.decodingErrorCounts[str]
+	count++
+	s.decodingErrorCounts[str] = count
+	switch {
+	case count == maxLogsPerDecodingError:
+		log.Errorf("DNSSnooper: error decoding packet: %s (reached %d occurrences, silencing)", str, maxLogsPerDecodingError)
+	case count < maxLogsPerDecodingError:
+		log.Errorf("DNSSnooper: error decoding packet: %s", str)
+	}
+}
+
 func (s *DNSSnooper) processDNSMessage(dns *layers.DNS) {
 
 	// Only consider responses to singleton, A-record questions

From 6347238f10dca088cfd126d8b441ecee63f6134d Mon Sep 17 00:00:00 2001
From: Alfonso Acosta <fons@weave.works>
Date: Fri, 27 Jan 2017 13:05:50 +0000
Subject: [PATCH 2/2] Review feedback

---
 probe/endpoint/dns_snooper_linux_amd64.go | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/probe/endpoint/dns_snooper_linux_amd64.go b/probe/endpoint/dns_snooper_linux_amd64.go
index e18bf7348..6ebd99b5e 100644
--- a/probe/endpoint/dns_snooper_linux_amd64.go
+++ b/probe/endpoint/dns_snooper_linux_amd64.go
@@ -15,9 +15,10 @@ import (
 )
 
 const (
-	bufSize                 = 8 * 1024 * 1024 // 8MB
-	maxReverseDNSrecords    = 10000
-	maxLogsPerDecodingError = 4
+	bufSize                     = 8 * 1024 * 1024 // 8MB
+	maxReverseDNSrecords        = 10000
+	maxLogsPerDecodingError     = 4
+	maxDecodingErrorCardinality = 1000
 )
 
 // DNSSnooper is a snopper of DNS queries
@@ -209,6 +210,11 @@ func (s *DNSSnooper) run() {
 
 // handleDecodeError logs errors up to the maximum allowed count
 func (s *DNSSnooper) handleDecodingError(err error) {
+	// prevent potential memory leak
+	if len(s.decodingErrorCounts) > maxDecodingErrorCardinality {
+		return
+	}
+
 	str := err.Error()
 	count := s.decodingErrorCounts[str]
 	count++